security: fix mentions command injection and @file credential leak (P0)
Jaime Fournier <jaimef@linbsd.org>
b2d9ec93fd31cc830d3ed515027741270b139025
diff --git a/src/jcode/core/mentions.ss b/src/jcode/core/mentions.ss
index 284451e..0ea1df5 100644
--- a/src/jcode/core/mentions.ss
+++ b/src/jcode/core/mentions.ss
@@ -15,12 +15,17 @@
;;; email addresses survive intact.
(export expand-mentions
- any-mentions?)
+ any-mentions?
+ resolve-file
+ resolve-diff
+ resolve-ls
+ resolve-symbol)
(import :std/os/shell
:std/misc/string
:std/misc/ports
- :jcode/core/log)
+ :jcode/core/log
+ :jcode/core/path-policy)
(def logger (make-logger "mentions"))
@@ -132,14 +137,15 @@
(else #f))))
(def (resolve-file path)
+ ;; Route reads through the descriptor-relative workspace policy so an
+ ;; @file: mention cannot inline credentials (e.g. ~/.ssh/id_rsa) or escape
+ ;; the workspace via absolute/tilde/symlink paths.
(cond
((string=? path "") #f)
- ((not (file-exists? path))
- (format "[@file:~a] not found" path))
- ((file-directory? path)
+ ((secure-path-directory? path)
(resolve-ls path))
(else
- (let ((content (try (read-file-string path)
+ (let ((content (try (secure-read-file-string path)
(catch (e) (err->string e)))))
(format "[@file:~a]\n```\n~a\n```" path content)))))
@@ -147,7 +153,7 @@
(let ((cmd (cond
((string=? arg "") "git diff --no-color")
((string=? arg "staged") "git diff --staged --no-color")
- (else (format "git diff --no-color -- ~a" arg)))))
+ (else (format "git diff --no-color -- ~a" (shell-escape arg))))))
(let-values (((out err code) (try (shell/status cmd #f)
(catch (e) (values "" (err->string e) 1)))))
(cond
@@ -169,19 +175,22 @@
(format "[@ls:~a] not found" p))
((not (file-directory? p))
(resolve-file p))
- (else
- (let-values (((out _err _rc) (try (shell/status (format "ls -1 ~a" p) #f)
- (catch (e) (values "" "" 1)))))
- (format "[@ls:~a]\n```\n~a\n```" p (or out "")))))))
+ (else
+ (let-values (((out _err _rc) (try (shell/status (format "ls -1 ~a" (shell-escape p)) #f)
+ (catch (e) (values "" "" 1)))))
+ (format "[@ls:~a]\n```\n~a\n```" p (or out "")))))))
(def (resolve-symbol name)
(cond
((string=? name "") #f)
(else
- (let* ((q (shell-escape name))
+ ;; Shell-escape the entire -e argument once. Escaping NAME alone and
+ ;; embedding it inside an outer single-quoted pattern let an embedded
+ ;; quote terminate the outer quoting and inject commands.
+ (let* ((pattern (format "(def(struct| |/)[[:space:]]*\\(?~a\\b" name))
(cmd (format
- "rg -n --no-heading -e '(def(struct| |/)[[:space:]]*\\(?~a\\b' . 2>/dev/null | head -20"
- q)))
+ "rg -n --no-heading -e ~a . 2>/dev/null | head -20"
+ (shell-escape pattern))))
(let-values (((out _err _rc) (try (shell/status cmd #f)
(catch (e) (values "" "" 1)))))
(cond
diff --git a/test/security-regression.ss b/test/security-regression.ss
index 7b9f7c9..df78fe1 100644
--- a/test/security-regression.ss
+++ b/test/security-regression.ss
@@ -6,6 +6,7 @@
(jcode core debug-repl)
(jcode core remote-auth)
(jcode core plugin)
+ (jcode core mentions)
(std misc ports)
(std misc string)
(std net tcp))
@@ -169,6 +170,41 @@
(when (file-exists? plugin-marker) (delete-file plugin-marker))
(system (string-append "rm -rf '" plugin-home "'"))
+;; ── (b) mentions command injection + policy bypass ────────────────────
+(define diff-probe "/tmp/jcode-sec-diff-probe")
+(when (file-exists? diff-probe) (delete-file diff-probe))
+(resolve-diff (string-append "x; touch " diff-probe))
+(check "@diff ';' injection does not execute"
+ (not (file-exists? diff-probe)))
+(resolve-diff (string-append "$(touch " diff-probe ")"))
+(check "@diff '$()' injection does not execute"
+ (not (file-exists? diff-probe)))
+
+(define ls-probe "jcode-sec-ls-probe")
+(define ls-dir "jcode-sec-ls-dir;touch jcode-sec-ls-probe")
+(when (file-exists? ls-probe) (delete-file ls-probe))
+(system (string-append "rm -rf '" ls-dir "'"))
+(mkdir ls-dir)
+(resolve-ls ls-dir)
+(check "@ls injection via crafted dir name does not execute"
+ (not (file-exists? ls-probe)))
+(system (string-append "rm -rf '" ls-dir "'"))
+(when (file-exists? ls-probe) (delete-file ls-probe))
+
+(define sym-probe "/tmp/jcode-sec-sym-probe")
+(when (file-exists? sym-probe) (delete-file sym-probe))
+(resolve-symbol (string-append "x'$(touch " sym-probe ")'"))
+(check "@symbol quote-breakout injection does not execute"
+ (not (file-exists? sym-probe)))
+
+(define mention-saved-home (getenv "HOME"))
+(putenv "HOME" (string-append (current-directory) "/.security-path-test/home"))
+(check "@file refuses to inline a sensitive credential path"
+ (not (string-contains
+ (resolve-file ".security-path-test/home/.ssh/id_test")
+ "credential-sentinel")))
+(putenv "HOME" mention-saved-home)
+
(when (> failures 0)
(error 'security-regression (format "~a security regression test(s) failed" failures)))
(printf "Security regressions passed~n")