From 1c01ed7158d8233d284b513c657a81cccbbba373 Mon Sep 17 00:00:00 2001 From: kewton Date: Sat, 3 Oct 2026 01:55:23 +0900 Subject: [PATCH] fix(tools): refuse ANSI-C and locale quoting in the bash write guard Issue #575. The write-guard lexer did not know ANSI-C (`$'...'`) or locale (`$"..."`) quoting: it kept the `$` as an ordinary character and stripped the following quote as an ordinary quote, so `$'tee'` read as the word `$tee`. Bash expands those forms before running, so the program, its prefixes, and its options were read wrong and a write could leave the workspace while bash:verify auto-approval and secret detection were bypassed. New leaf module `bash_write_guard/ansi_c_quoting.rs` scans the whole command before the lexer and returns the first `$'` / `$"` outside a shell quote; quote state mirrors `shell_tokens` (outside / `'...'` / `"..."` / `\` escape), so the introducer inside a quote, after a backslash, or a lone `$` stays allowed. The refusal reuses the env -S shape: a new operation value "ANSI-C / locale quoting" is pushed to write_targets and rejected by confinement_rejection, so the write is recognized (has_recognized_mutation is true) and bash:verify auto-approval no longer applies. Event names/schema and the other lexical analyses are unchanged; `$'tee'`-style spellings are refused as unverifiable rather than expanded. Two-point measurement (report) - Before, develop@1e193ae7 with the new test and unmodified source: `cargo test --test issue575_bash_ansi_c_quoting` FAILED -- `ansi_c_quoting_rejects_program_and_option_spellings` ("expected rejection: $'tee' /tmp/f") and `ansi_c_quoting_rejection_names_the_ansi_c_operation` ("expected rejection"). The allow test passed. - After: 5 lib unit tests + 3 integration tests pass. Mutation testing (report; cargo-mutants 27.1.0, test filter `ansi_c_quoting` so both the lib unit tests and the integration tests run) - New module: `cargo mutants -f src/tools/bash_write_guard/ansi_c_quoting.rs --jobs 2 --timeout 120 -o /mutants-module2 -- ansi_c_quoting` => 13 mutants tested, 13 caught, 0 missed. - New wiring lines only: `cargo mutants -f src/tools/bash_write_guard.rs -F 'bash_write_guard\.rs:(33|130):' ...` => 3 mutants: 2 caught (the confinement_rejection operation check and the write_targets early return), 1 unviable (the `vec![Default::default()]` replacement does not compile), 0 missed. - Supplementary function-scope run (`-F 'write_targets|confinement_rejection'`) leaves 6 survivors, all on pre-existing lines this issue neither adds nor changes: the `/dev/null` skip (63/64), the `cd -` check (68), and the segment split (140). They survive because the second-layer `path_tokens` scan still rejects an outside path when the `/dev/null` skip is dropped, because the same assertions cover the changed conditions, and because the changed segment split yields an empty segment list that the fixtures do not distinguish. No survivor is on a line added by this change. Scope (report) - Declared and actual: src/tools/bash_write_guard.rs, src/tools/bash_write_guard/ansi_c_quoting.rs, tests/issue575_bash_ansi_c_quoting.rs. No other file changed; the referenced existing integration tests (issue566/567/428, bash_workspace_confinement) are untouched and pass. Gates (report; all exit 0) - cargo fmt --all -- --check - python3 scripts/validate_codex_skills.py --tracked-only - ruff check --isolated --select E4,E7,E9,F,I --ignore E402 - shellcheck scripts/*.sh - python3 -m pytest tests/test_codex_orchestrate.py -q - python3 -m unittest discover -s workspace/management/scripts -p 'test_*.py' - python3 tests/eval/test_acceptance_contract.py - python3 tests/eval/test_completion_contract_snapshots.py - python3 tests/eval/test_false_positive_regression.py - RUSTFLAGS='-D warnings' cargo clippy --all-targets -- -D warnings - RUSTFLAGS='-D warnings' cargo test --all-targets - ruff check --isolated --select E4,E7,E9,F,I --ignore E402 scripts/nextjs_domain_oracle.py tests/test_nextjs_domain_oracle.py - npm ci --ignore-scripts --include=dev --prefix tests/nextjs_domain && python3 -m pytest tests/test_nextjs_domain_oracle.py -q Report-note: the issue asks for a "report"; scope.allow permits only the three files above, so this report is carried in the commit message and no report file was added. Co-authored-by: CommandCodeBot --- src/tools/bash_write_guard.rs | 71 ++++++++++ src/tools/bash_write_guard/ansi_c_quoting.rs | 130 +++++++++++++++++++ tests/issue575_bash_ansi_c_quoting.rs | 115 ++++++++++++++++ 3 files changed, 316 insertions(+) create mode 100644 src/tools/bash_write_guard/ansi_c_quoting.rs create mode 100644 tests/issue575_bash_ansi_c_quoting.rs diff --git a/src/tools/bash_write_guard.rs b/src/tools/bash_write_guard.rs index 696dc692..c1bff07c 100644 --- a/src/tools/bash_write_guard.rs +++ b/src/tools/bash_write_guard.rs @@ -1,5 +1,6 @@ use std::path::Path; +mod ansi_c_quoting; mod command_prefix; #[derive(Debug, Clone, PartialEq, Eq)] @@ -29,6 +30,16 @@ pub(super) fn confinement_rejection( root: &Path, ) -> Option { for target in write_targets(command) { + if target.operation == ansi_c_quoting::OPERATION { + return Some(BashWriteConfinementRejection { + reason: format!( + "Bash command uses ANSI-C or locale quoting `{}`, whose expansion cannot be verified to remain in the Gate 1 workspace boundary; rewrite it with a literal escape (for example `printf 'a\\tb\\n'` or `grep -P '\\t'`)", + target.path + ), + path: target.path, + operation: target.operation, + }); + } if target.operation == command_prefix::UNVERIFIABLE_SPLIT_STRING_OPERATION { return Some(BashWriteConfinementRejection { reason: format!( @@ -116,6 +127,12 @@ fn path_matches(candidate: &Path, protected: &Path) -> bool { } fn write_targets(command: &str) -> Vec { + if let Some(kind) = ansi_c_quoting::outside_quotes(command) { + return vec![WriteTarget { + path: kind.introducer().to_string(), + operation: ansi_c_quoting::OPERATION.to_string(), + }]; + } let Some(tokens) = shell_tokens(command) else { return Vec::new(); }; @@ -984,4 +1001,58 @@ mod tests { ); assert!(protected_path_mutation("env FOO=1 cargo test", &root, &protected).is_none()); } + + #[test] + fn ansi_c_quoting_is_a_recognized_mutation() { + assert!(has_recognized_mutation("$'tee' a.txt")); + assert!(has_recognized_mutation("$\"tee\" a.txt")); + assert!(has_recognized_mutation("X=$'tee'")); + assert!(!has_recognized_mutation("echo \"$'x'\"")); + assert!(!has_recognized_mutation("echo '$'")); + assert!(!has_recognized_mutation("echo \\$'x'")); + } + + #[test] + fn ansi_c_quoting_write_targets_record_the_operation() { + assert_eq!( + write_target_pairs("$'tee' a.txt"), + expected_targets(&[("$'", ansi_c_quoting::OPERATION)]) + ); + assert_eq!( + write_target_pairs("$\"tee\" a.txt"), + expected_targets(&[("$\"", ansi_c_quoting::OPERATION)]) + ); + } + + #[test] + fn ansi_c_quoting_confinement_rejection_table() { + let fixture = tempfile::tempdir().unwrap(); + let root = fixture.path().join("workspace"); + std::fs::create_dir_all(&root).unwrap(); + + for command in [ + "$'tee' /tmp/f", + "env $'tee' /tmp/f", + "env $'-S' 'tee /tmp/f'", + "$\"tee\" /tmp/f", + "X=$'tee'", + ] { + assert!( + confinement_rejection(command, &root).is_some(), + "command must be rejected: {command}" + ); + } + + for command in [ + "echo \"$'x'\"", + "echo '$'", + "echo \\$'x'", + "printf 'a\\tb\\n' > out.txt", + ] { + assert!( + confinement_rejection(command, &root).is_none(), + "command must stay allowed: {command}" + ); + } + } } diff --git a/src/tools/bash_write_guard/ansi_c_quoting.rs b/src/tools/bash_write_guard/ansi_c_quoting.rs new file mode 100644 index 00000000..b040f4ae --- /dev/null +++ b/src/tools/bash_write_guard/ansi_c_quoting.rs @@ -0,0 +1,130 @@ +//! ANSI-C (`$'...'`) and locale (`$"..."`) quoting outside shell quotes +//! (Issue #575). +//! +//! The write-target inspector's lexical analysis in the parent module does not +//! know `$'...'` and `$"..."`: it keeps the `$` as an ordinary character and +//! strips the following quotes as ordinary quotes, so `$'tee'` becomes the word +//! `$tee` and the program, its prefixes, and its options are read wrong. The +//! shell expands those forms before the program runs, and the expansion cannot +//! be verified portably: the escape table differs between shell versions and +//! `$"..."` depends on the locale. Every `$'` or `$"` that sits outside a shell +//! quote is therefore refused as an unverifiable form. +//! +//! Quote handling mirrors the parent module's lexer (outside, `'...'`, `"..."`, +//! and `\` escaping), so a `$'` or `$"` inside a quote, after a backslash, or a +//! lone `$` stays allowed. The scan runs over the whole command before the +//! lexer so a mis-read `$'\''` cannot hide the commands that follow it. + +/// Operation recorded for a command that spells `$'` or `$"` outside a shell +/// quote, whose expansion cannot be verified. +pub(super) const OPERATION: &str = "ANSI-C / locale quoting"; + +/// The quoting introducer found outside a shell quote. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum Kind { + /// ANSI-C quoting, `$'...'`. + AnsiC, + /// Locale translation quoting, `$"..."`. + Locale, +} + +impl Kind { + /// The two-character introducer as it appears in the command. + pub(super) fn introducer(self) -> &'static str { + match self { + Kind::AnsiC => "$'", + Kind::Locale => "$\"", + } + } +} + +/// Returns the first `$'` / `$"` introducer that sits outside a shell quote. +pub(super) fn outside_quotes(command: &str) -> Option { + let mut chars = command.chars().peekable(); + let mut single_quoted = false; + let mut double_quoted = false; + while let Some(ch) = chars.next() { + if single_quoted { + if ch == '\'' { + single_quoted = false; + } + continue; + } + if double_quoted { + match ch { + '"' => double_quoted = false, + '\\' => { + chars.next(); + } + _ => {} + } + continue; + } + match ch { + '\'' => single_quoted = true, + '"' => double_quoted = true, + '\\' => { + chars.next(); + } + '$' => match chars.peek() { + Some('\'') => return Some(Kind::AnsiC), + Some('"') => return Some(Kind::Locale), + _ => {} + }, + _ => {} + } + } + None +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn ansi_c_quoting_outside_quotes_detection_table() { + let cases: &[(&str, Option)] = &[ + ("$'tee' /tmp/f", Some(Kind::AnsiC)), + ("env $'tee' /tmp/f", Some(Kind::AnsiC)), + ("env $'-S' 'tee /tmp/f'", Some(Kind::AnsiC)), + ("$'\\x74ee' /tmp/f", Some(Kind::AnsiC)), + ("$'t\\145e' /tmp/f", Some(Kind::AnsiC)), + ("x$'tee' /tmp/f", Some(Kind::AnsiC)), + ("echo $'\\'' ; tee /tmp/f", Some(Kind::AnsiC)), + ("echo $'\\''\ntee sub/link/f", Some(Kind::AnsiC)), + ("$'\\cX' /tmp/f", Some(Kind::AnsiC)), + ("$''tee /tmp/f", Some(Kind::AnsiC)), + ("printf $'a\\'b' > sub/link/f", Some(Kind::AnsiC)), + ("X=$'tee'", Some(Kind::AnsiC)), + ("cat $'.env'", Some(Kind::AnsiC)), + ("$\"tee\" /tmp/f", Some(Kind::Locale)), + ("$\"\"tee /tmp/f", Some(Kind::Locale)), + // A closed quote must be seen as closed, so the `$'` after it is + // outside; an escaped `"` inside a double quote must not close it. + ("echo 'a' $'tee'", Some(Kind::AnsiC)), + ("echo \"x\" $'tee'", Some(Kind::AnsiC)), + (r#"echo "a\"" $'tee'"#, Some(Kind::AnsiC)), + ("echo \"$'x'\"", None), + ("echo \"$\\'x'\"", None), + ("echo '$'", None), + ("echo 'a$'", None), + ("echo 'a' 'b'", None), + ("echo \\$'x'", None), + ("echo \"$\"", None), + ("printf '%s\\n' x", None), + ("printf 'a\\tb\\n'", None), + ("cargo test", None), + ("$", None), + ("", None), + ]; + for (command, expected) in cases { + assert_eq!(outside_quotes(command), *expected, "command: {command}"); + } + } + + #[test] + fn ansi_c_quoting_kind_reports_its_introducer() { + assert_eq!(Kind::AnsiC.introducer(), "$'"); + assert_eq!(Kind::Locale.introducer(), "$\""); + } +} diff --git a/tests/issue575_bash_ansi_c_quoting.rs b/tests/issue575_bash_ansi_c_quoting.rs new file mode 100644 index 00000000..c53abe8e --- /dev/null +++ b/tests/issue575_bash_ansi_c_quoting.rs @@ -0,0 +1,115 @@ +#![cfg(unix)] + +//! Issue #575: the Bash write-target guard's lexical analysis does not know +//! ANSI-C quoting (`$'...'`) or locale quoting (`$"..."`). It leaves the `$` as +//! an ordinary character and strips the following quotes as ordinary quotes, so +//! `$'tee'` becomes the word `$tee` and the program, its prefixes, and its +//! options are read wrong. The shell expands those forms before running, so +//! every command that spells `$'` or `$"` outside a shell quote is refused as an +//! unverifiable form. Every test name contains `ansi_c_quoting` so the +//! mutation-test filter selects these tests together with the unit tests in +//! `bash_write_guard/ansi_c_quoting.rs`. + +use std::path::PathBuf; + +use commandagent::tools::bash::path_confinement_rejection; + +struct Fixture { + _dir: tempfile::TempDir, + root: PathBuf, +} + +/// A workspace with an escaping `sub/link` symlink, an inside `a.txt`, and an +/// inside secret file. +fn fixture() -> Fixture { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("ws"); + std::fs::create_dir_all(root.join("sub")).unwrap(); + std::fs::write(root.join("a.txt"), "x").unwrap(); + std::fs::write(root.join(".env"), "workspace-secret").unwrap(); + let outside = dir.path().join("outside"); + std::fs::create_dir_all(&outside).unwrap(); + std::fs::write(outside.join("secret"), "outside-secret").unwrap(); + std::os::unix::fs::symlink(&outside, root.join("sub/link")).unwrap(); + let root = root.canonicalize().unwrap(); + Fixture { _dir: dir, root } +} + +#[test] +fn ansi_c_quoting_rejects_program_and_option_spellings() { + let fixture = fixture(); + let root = &fixture.root; + let cases = [ + // The problem table from the issue body: the program, a prefix, and an + // option are spelled with ANSI-C / locale quoting. + "$'tee' /tmp/f", + "env $'tee' /tmp/f", + "env $'-S' 'tee /tmp/f'", + "$'\\x74ee' /tmp/f", + "$'t\\145e' /tmp/f", + "$\"tee\" /tmp/f", + "x$'tee' /tmp/f", + "cat $'.env'", + "cat $'sub/link/secret'", + // The same spellings through an escaping symlink. + "$'tee' sub/link/f", + "env $'-C' sub tee link/f", + "cp $'-t' sub/link a.txt", + // A mis-read quote that would hide everything after it. + "echo $'\\'' ; tee /tmp/f", + // More program, option, and assignment spellings. + "$'\\cX' /tmp/f", + "$''tee /tmp/f", + "$\"\"tee /tmp/f", + "$'sudo' tee /tmp/f", + "$'rm' -rf sub/link/x", + "echo $'\\''\ntee sub/link/f", + "printf $'a\\'b' > sub/link/f", + "X=$'tee'", + ]; + for command in cases { + assert!( + path_confinement_rejection(command, root).is_some(), + "expected rejection: {command}" + ); + } +} + +#[test] +fn ansi_c_quoting_keeps_quoted_escaped_and_plain_forms_allowed() { + let fixture = fixture(); + let root = &fixture.root; + let cases = [ + // The `$'` / `$"` introducer is not expanded inside a quote, after a + // backslash, or as a lone `$`. + "echo \"$'x'\"", + "echo '$'", + "echo \\$'x'", + "echo \"$\"", + // Ordinary single-quoted escapes stay usable. + "printf '%s\\n' x > out.txt", + "printf 'a\\tb\\n' > out.txt", + // Everyday verification commands. + "cargo test", + "cargo fmt --all -- --check", + "cargo clippy --all-targets -- -D warnings", + "npm test", + "npm run build", + "python3 -m pytest", + "timeout 600 cargo test", + ]; + for command in cases { + assert!( + path_confinement_rejection(command, root).is_none(), + "expected allow: {command}" + ); + } +} + +#[test] +fn ansi_c_quoting_rejection_names_the_ansi_c_operation() { + let fixture = fixture(); + let rejection = + path_confinement_rejection("$'tee' /tmp/f", &fixture.root).expect("expected rejection"); + assert_eq!(rejection.operation, "ANSI-C / locale quoting"); +}