From b0dbd8bce7c108b6d042f9d06f81973ea845bbf6 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Thu, 22 Oct 2026 05:40:56 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?= =?UTF-8?q?=20Fix=20Path=20Traversal=20in=20agent=20workdir=20commands?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🚨 Severity: CRITICAL 💡 Vulnerability: The `validate_read_only` function in `crates/agent/src/tools/workdir_cmd.rs` used a naive `BLOCKED_TOKENS` blocklist on the raw command string, failing to account for shell argument obfuscation via quotation (e.g., `cat ".."/".."`). This allowed directory traversal vulnerabilities for ostensibly read-only commands. 🎯 Impact: Attackers could execute read-only commands to traverse directories and read sensitive files outside of the intended scope. 🔧 Fix: Updated the check inside `validate_read_only` to verify for `..` inside the already split and tokenized command arguments, robustly blocking traversal attempts regardless of how they are quoted. ✅ Verification: Ensure tests pass `cargo test -p devforge-agent --lib tools::workdir_cmd::tests` with the new regression test coverage added for `cat ".."/".."/etc/passwd`. Co-authored-by: bobdivx <6737167+bobdivx@users.noreply.github.com> --- .jules/sentinel.md | 4 ++++ crates/agent/src/tools/workdir_cmd.rs | 2 ++ 2 files changed, 6 insertions(+) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index ffc0bd2ce..43750fe5b 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -2,3 +2,7 @@ **Vulnerability:** The manual SVG sanitizer `apps/web/src/lib/sanitize-svg.ts` used an insufficient allowlist/blocklist. It allowed ``, ``, ``, ``, ``, and `` which could lead to mutation XSS (mXSS) and other attack vectors when rendering untrusted Mermaid or Math blocks. **Learning:** Manual DOM tree sanitization is notoriously hard to get right because attackers can hide active elements (e.g. `` tags with `javascript:` payloads) inside grandchild trees (like ``) or use SVG-specific tricks (like ``) which naive list-based cleaners overlook. **Prevention:** Rather than adding heavy third-party dependencies (like DOMPurify) against explicit project instructions, expanding the `BLOCKED` Set to include these structural injection vectors provides a robust, zero-dependency mitigation. Future expansions should verify nested elements carefully. +## 2026-10-22 - [Bypass Command Parsing and Path Traversal fix] +**Vulnerability:** The `validate_read_only` function in `crates/agent/src/tools/workdir_cmd.rs` allowed command parsing bypass (using quoted paths like `cat ".."/".."/etc/passwd`) which facilitated path traversal because the check only matched against the literal block string `../`. +**Learning:** Checking for exact substrings against raw commands is highly fragile against obfuscation, such as Unix-style quotation. +**Prevention:** Validation for string containment (such as blocking `..`) should run against the tokenized and normalized (i.e., lower-case without surrounding quotes) arguments, not the unparsed raw command line. diff --git a/crates/agent/src/tools/workdir_cmd.rs b/crates/agent/src/tools/workdir_cmd.rs index 932065676..77426cbf8 100644 --- a/crates/agent/src/tools/workdir_cmd.rs +++ b/crates/agent/src/tools/workdir_cmd.rs @@ -35,6 +35,7 @@ fn validate_read_only(bin: &str, parts: &[String]) -> std::result::Result<(), St let l = a.to_ascii_lowercase(); if a.starts_with('/') || a.starts_with('~') + || l.contains("..") || l.contains(".env") || l.contains("secret") || l.contains(".git/") @@ -328,6 +329,7 @@ mod tests { assert!(validate_command("git -c x=y status").is_err()); assert!(validate_command("cat .env").is_err()); assert!(validate_command("cat /etc/passwd").is_err()); + assert!(validate_command("cat \"..\"/\"..\"/etc/passwd").is_err()); assert!(validate_command("git branch -D main").is_err()); }