From 244d4c56768fcc60bd1b23fe45df7a57c7b293ec Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Sat, 16 May 2026 11:30:04 -0500 Subject: feat(gptel-tools): harden path validation with file-truename realpath Resolves PATH through file-truename before applying home-directory and read/write checks across the path-handling tools (git_status, git_log, git_diff, move_to_trash, read_text_file, update_text_file, write_text_file, list_directory_files, read_buffer, web_fetch). Without the resolve step, a symlink under HOME pointing outside HOME would pass the prefix check but the tool would act on the real target -- a symlink-escape. move_to_trash also tightens the trash-bin construction (treats empty file extensions correctly) and switches the "critical directories" list to truename-resolved canonical forms so a symlinked ~/.config can't be trashed via an aliased path. update_text_file fixes an off-by-one in the line-count derivation when the source content is empty. Each source change pairs with tests in tests/test-gptel-tools-*.el and tests/test-update-text-file.el covering the realpath escape paths, the empty-extension trash case, and the empty-content line- count edge. Combined coverage is now 100% across all ten gptel-tools source files: 516 / 516 executable lines, 217 tests. --- gptel-tools/update_text_file.el | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) (limited to 'gptel-tools/update_text_file.el') diff --git a/gptel-tools/update_text_file.el b/gptel-tools/update_text_file.el index 492ed554d..f8b58025c 100644 --- a/gptel-tools/update_text_file.el +++ b/gptel-tools/update_text_file.el @@ -40,20 +40,23 @@ PATH must resolve inside the user's home directory, must exist, must be a regular file, and must be readable and writable." - (let ((full (expand-file-name path "~"))) + (let* ((home (file-name-as-directory (file-truename (expand-file-name "~")))) + (full (expand-file-name path "~"))) (unless (string-prefix-p (expand-file-name "~") full) (error "Path must be within home directory: %s" path)) (unless (file-exists-p full) (error "File not found: %s" full)) - (when (file-directory-p full) - (error "Path is a directory, not a file: %s" full)) - (unless (file-readable-p full) - (error "No read permission for file: %s" full)) - (unless (file-writable-p full) - (error "No write permission for file: %s" full)) - (if (file-symlink-p full) - (file-truename full) - full))) + (let ((resolved (file-truename full))) + (unless (or (string= resolved (directory-file-name home)) + (string-prefix-p home resolved)) + (error "Resolved path must be within home directory: %s" path)) + (when (file-directory-p resolved) + (error "Path is a directory, not a file: %s" resolved)) + (unless (file-readable-p resolved) + (error "No read permission for file: %s" resolved)) + (unless (file-writable-p resolved) + (error "No write permission for file: %s" resolved)) + resolved))) (defun cj/update-text-file--backup-name (path) "Return a backup filename for PATH timestamped to the current second." @@ -113,9 +116,10 @@ on out-of-range LINE-NUM or empty TEXT." ;; extra empty element at the end. Trim it so the line count ;; matches what a human would say. (trailing-newline (string-suffix-p "\n" content)) - (line-count (if trailing-newline - (1- (length lines)) - (length lines)))) + (line-count (cond + ((string-empty-p content) 0) + (trailing-newline (1- (length lines))) + (t (length lines))))) (when (> line-num (1+ line-count)) (error "Line %d out of range (file has %d lines)" line-num line-count)) (let* ((to-insert (if (string-suffix-p "\n" text) -- cgit v1.2.3