From e1157fbd34af54cd8b1f3bfcd9d7db54547e92b5 Mon Sep 17 00:00:00 2001 From: kewton Date: Fri, 2 Oct 2026 01:13:15 +0900 Subject: [PATCH 1/6] fix(tools): peel command prefixes before the Bash write-target check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bash の書き込み検査が segment の先頭語だけで program を決めていたため、 env・sudo・timeout・nice・予約語などの前置きの後ろの書き込みを ワークスペース外と判定できなかった（#566）。新しい葉 module `bash_write_guard/command_prefix.rs` に前置きの表と剥がし方を置き、 不動点まで剥がしてから program を決めるようにした。剥がし方が決まらない形は 残りの語を走査し、書き込み program か別の前置きがあれば拒否に倒す。 判断: `sudoedit` の operand を書き込み先として扱う（Issue 設計の「sudoedit は operand が書き込み先」に対応） 判断: `env -S` の operand は語を空白で分割してから走査する（引用された `tee /tmp/f` を検出するため） 読み替え: 「残りの語の basename を走査」は、決められなくなった前置きの直後以降の語を対象とした 本文に無い指摘: cargo mutants を --jobs 3 で回すと共有 target dir の競合で偽の生存変異が出る（記録した missed の `index - 1` / `offset - 1` は直接再適用すると overflow panic で kill される）；report に記録 Co-authored-by: CommandCodeBot --- src/tools/bash_write_guard.rs | 325 ++++- src/tools/bash_write_guard/command_prefix.rs | 1204 +++++++++++++++++ ...ue566_bash_command_prefix_write_targets.rs | 156 +++ 3 files changed, 1619 insertions(+), 66 deletions(-) create mode 100644 src/tools/bash_write_guard/command_prefix.rs create mode 100644 tests/issue566_bash_command_prefix_write_targets.rs diff --git a/src/tools/bash_write_guard.rs b/src/tools/bash_write_guard.rs index 96783119..be3df1f5 100644 --- a/src/tools/bash_write_guard.rs +++ b/src/tools/bash_write_guard.rs @@ -1,5 +1,7 @@ use std::path::Path; +mod command_prefix; + #[derive(Debug, Clone, PartialEq, Eq)] pub(super) struct BashWriteConfinementRejection { pub path: String, @@ -27,6 +29,16 @@ pub(super) fn confinement_rejection( root: &Path, ) -> Option { for target in write_targets(command) { + if target.operation == command_prefix::UNRESOLVED_OPERATION { + return Some(BashWriteConfinementRejection { + reason: format!( + "Bash command prefix `{}` cannot be resolved, so the write targets it launches cannot be proven to remain in the Gate 1 workspace boundary", + target.path + ), + path: target.path, + operation: target.operation, + }); + } if target.path == "/dev/null" && matches!(target.operation.as_str(), "output redirection" | "tee") { @@ -100,86 +112,120 @@ fn write_targets(command: &str) -> Vec { let mut targets = redirect_targets(&tokens); for segment in tokens.split(|token| *token == ShellToken::SegmentEnd) { let words = command_words(segment); - let Some((program, arguments)) = words.split_first() else { + if words.is_empty() { continue; - }; - let program = Path::new(program) - .file_name() - .and_then(|name| name.to_str()) - .unwrap_or(program); - let operands = positional_operands(arguments); - match program { - "ln" => { - if let Some(target_directory) = target_directory(arguments) { - targets.push(WriteTarget { - path: target_directory, - operation: program.to_string(), - }); - if symbolic_link_requested(arguments) { - targets.extend(operands.iter().map(|path| WriteTarget { - path: (*path).to_string(), - operation: "symlink target".to_string(), - })); - } - } else if let Some(destination) = operands.get(1..).and_then(|items| items.last()) { - targets.push(WriteTarget { - path: (*destination).to_string(), - operation: program.to_string(), - }); - if symbolic_link_requested(arguments) { - targets.extend(operands[..operands.len() - 1].iter().map(|path| { - WriteTarget { - path: (*path).to_string(), - operation: "symlink target".to_string(), - } - })); - } - } + } + match command_prefix::resolve(&words) { + command_prefix::Resolution::Program { + program, + arguments, + writes, + } => { + push_prefix_writes(&mut targets, writes); + let arguments: Vec<&str> = arguments.iter().map(String::as_str).collect(); + collect_program_targets(&program, &arguments, &mut targets); } - "cp" | "mv" | "install" => { - if let Some(target_directory) = target_directory(arguments) { - targets.push(WriteTarget { - path: target_directory, - operation: program.to_string(), - }); - } else if program == "install" - && arguments - .iter() - .any(|argument| matches!(*argument, "-d" | "--directory")) - { - targets.extend(operands.into_iter().map(|path| WriteTarget { - path: path.to_string(), - operation: program.to_string(), + command_prefix::Resolution::NoExecution { writes } => { + push_prefix_writes(&mut targets, writes); + } + command_prefix::Resolution::Undecidable { prefix } => { + targets.push(WriteTarget { + path: prefix, + operation: command_prefix::UNRESOLVED_OPERATION.to_string(), + }); + } + } + } + targets +} + +fn push_prefix_writes(targets: &mut Vec, writes: Vec) { + targets.extend(writes.into_iter().map(|write| WriteTarget { + path: write.path, + operation: write.operation, + })); +} + +fn collect_program_targets(program: &str, arguments: &[&str], targets: &mut Vec) { + let operands = positional_operands(arguments); + match program { + "ln" => { + if let Some(target_directory) = target_directory(arguments) { + targets.push(WriteTarget { + path: target_directory, + operation: program.to_string(), + }); + if symbolic_link_requested(arguments) { + targets.extend(operands.iter().map(|path| WriteTarget { + path: (*path).to_string(), + operation: "symlink target".to_string(), })); - } else if let Some(destination) = operands.last() { - targets.push(WriteTarget { - path: (*destination).to_string(), - operation: program.to_string(), - }); } - } - "tee" | "mkdir" | "rm" | "touch" | "truncate" => { - targets.extend(operands.into_iter().map(|path| WriteTarget { - path: path.to_string(), + } else if let Some(destination) = operands.get(1..).and_then(|items| items.last()) { + targets.push(WriteTarget { + path: (*destination).to_string(), operation: program.to_string(), - })); + }); + if symbolic_link_requested(arguments) { + targets.extend( + operands[..operands.len() - 1] + .iter() + .map(|path| WriteTarget { + path: (*path).to_string(), + operation: "symlink target".to_string(), + }), + ); + } } - "chmod" | "chown" => { - targets.extend(operands.into_iter().skip(1).map(|path| WriteTarget { + } + "cp" | "mv" | "install" => { + if let Some(target_directory) = target_directory(arguments) { + targets.push(WriteTarget { + path: target_directory, + operation: program.to_string(), + }); + } else if program == "install" + && arguments + .iter() + .any(|argument| matches!(*argument, "-d" | "--directory")) + { + targets.extend(operands.into_iter().map(|path| WriteTarget { path: path.to_string(), operation: program.to_string(), })); - } - "cd" => { + } else if let Some(destination) = operands.last() { targets.push(WriteTarget { - path: operands.first().copied().unwrap_or("~").to_string(), - operation: "working directory".to_string(), + path: (*destination).to_string(), + operation: program.to_string(), }); } - _ => {} } + "tee" | "mkdir" | "rm" | "touch" | "truncate" => { + targets.extend(operands.into_iter().map(|path| WriteTarget { + path: path.to_string(), + operation: program.to_string(), + })); + } + "chmod" | "chown" => { + targets.extend(operands.into_iter().skip(1).map(|path| WriteTarget { + path: path.to_string(), + operation: program.to_string(), + })); + } + "cd" => { + targets.push(WriteTarget { + path: operands.first().copied().unwrap_or("~").to_string(), + operation: "working directory".to_string(), + }); + } + "sudoedit" => { + targets.extend(operands.into_iter().map(|path| WriteTarget { + path: path.to_string(), + operation: program.to_string(), + })); + } + _ => {} } - targets } fn redirect_targets(tokens: &[ShellToken]) -> Vec { @@ -775,4 +821,151 @@ mod tests { ); } } + + #[test] + fn command_prefix_write_targets_table() { + let cases: &[(&str, &[(&str, &str)])] = &[ + ("env tee /tmp/f", &[("/tmp/f", "tee")]), + ("env FOO=1 cp a.txt /tmp/f", &[("/tmp/f", "cp")]), + ("sudo cp a.txt /tmp/f", &[("/tmp/f", "cp")]), + ("command cp a.txt /tmp/f", &[("/tmp/f", "cp")]), + ("nohup tee /tmp/f", &[("/tmp/f", "tee")]), + ("timeout 5 cp a.txt /tmp/f", &[("/tmp/f", "cp")]), + ("{ tee /tmp/f; }", &[("/tmp/f", "tee")]), + ("! tee /tmp/f", &[("/tmp/f", "tee")]), + ("if true; then tee /tmp/f; fi", &[("/tmp/f", "tee")]), + ("while tee /tmp/f; do :; done", &[("/tmp/f", "tee")]), + ("env -i tee /tmp/f", &[("/tmp/f", "tee")]), + ("env -u HOME tee /tmp/f", &[("/tmp/f", "tee")]), + ("env -u HOME FOO=1 tee /tmp/f", &[("/tmp/f", "tee")]), + ("env -- tee /tmp/f", &[("/tmp/f", "tee")]), + ("/usr/bin/env tee /tmp/f", &[("/tmp/f", "tee")]), + ("command -p tee /tmp/f", &[("/tmp/f", "tee")]), + ("exec tee /tmp/f", &[("/tmp/f", "tee")]), + ("exec -a x tee /tmp/f", &[("/tmp/f", "tee")]), + ("sudo -u root tee /tmp/f", &[("/tmp/f", "tee")]), + ("sudo -E tee /tmp/f", &[("/tmp/f", "tee")]), + ("sudo -- tee /tmp/f", &[("/tmp/f", "tee")]), + ("doas -u root tee /tmp/f", &[("/tmp/f", "tee")]), + ("timeout -s KILL 5 tee /tmp/f", &[("/tmp/f", "tee")]), + ("timeout -k 1 5 tee /tmp/f", &[("/tmp/f", "tee")]), + ( + "timeout --preserve-status 5s tee /tmp/f", + &[("/tmp/f", "tee")], + ), + ("nice -n 5 tee /tmp/f", &[("/tmp/f", "tee")]), + ("nice -5 tee /tmp/f", &[("/tmp/f", "tee")]), + ("ionice -c 3 tee /tmp/f", &[("/tmp/f", "tee")]), + ("stdbuf -oL tee /tmp/f", &[("/tmp/f", "tee")]), + ("time -p tee /tmp/f", &[("/tmp/f", "tee")]), + ("/usr/bin/time -o /tmp/f true", &[("/tmp/f", "time")]), + ("sudo -e /tmp/f", &[("/tmp/f", "sudo")]), + ("for x in a; do tee /tmp/f; done", &[("/tmp/f", "tee")]), + ("true && { tee /tmp/f; }", &[("/tmp/f", "tee")]), + ("f() { tee /tmp/f; }", &[("/tmp/f", "tee")]), + ("coproc tee /tmp/f", &[("/tmp/f", "tee")]), + ("env sudo timeout 5 tee /tmp/f", &[("/tmp/f", "tee")]), + ( + "sudo -X u tee /tmp/f", + &[("sudo", command_prefix::UNRESOLVED_OPERATION)], + ), + ( + "sudo -s tee /tmp/f", + &[("sudo", command_prefix::UNRESOLVED_OPERATION)], + ), + ( + "env -S \"tee /tmp/f\"", + &[("env", command_prefix::UNRESOLVED_OPERATION)], + ), + ]; + for (command, expected) in cases { + assert_eq!( + write_target_pairs(command), + expected_targets(expected), + "command: {command}" + ); + } + } + + #[test] + fn command_prefix_confinement_rejection_table() { + let fixture = tempfile::tempdir().unwrap(); + let root = fixture.path().join("workspace"); + std::fs::create_dir_all(&root).unwrap(); + + for command in [ + "env tee /tmp/f", + "sudo cp a.txt /tmp/f", + "command cp a.txt /tmp/f", + "nohup tee /tmp/f", + "timeout 5 cp a.txt /tmp/f", + "{ tee /tmp/f; }", + "! tee /tmp/f", + "if true; then tee /tmp/f; fi", + "env -S \"tee /tmp/f\"", + "sudo -s tee /tmp/f", + "sudo -X u tee /tmp/f", + "sudo -e /tmp/f", + "doas -u root tee /tmp/f", + "nice -5 tee /tmp/f", + "time -p tee /tmp/f", + "/usr/bin/time -o /tmp/f true", + ] { + assert!( + confinement_rejection(command, &root).is_some(), + "command must be rejected: {command}" + ); + } + + for command in [ + "env FOO=1 tee out.txt", + "printf x | env tee out.txt", + "timeout 5 cp a.txt b.txt", + "env tee /dev/null", + "env FOO=1 cargo test", + "timeout 600 cargo test", + "timeout --foreground 600 cargo test", + "timeout --unknown 600 cargo test", + "nice cargo build", + "nohup cargo build", + "time cargo test", + "{ cargo test; }", + "command -v cargo", + "command -v tee /tmp/f", + "sudo -l tee /tmp/f", + ] { + assert!( + confinement_rejection(command, &root).is_none(), + "command must stay allowed: {command}" + ); + } + } + + #[test] + fn command_prefix_has_recognized_mutation_table() { + assert!(has_recognized_mutation("env tee out.txt")); + assert!(has_recognized_mutation("timeout 5 tee out.txt")); + assert!(!has_recognized_mutation("timeout 600 cargo test")); + assert!(!has_recognized_mutation("env FOO=1 cargo test")); + assert!(!has_recognized_mutation("command -v tee /tmp/f")); + assert!(has_recognized_mutation("sudo -X u tee /tmp/f")); + } + + #[test] + fn command_prefix_protected_path_mutation_table() { + let fixture = tempfile::tempdir().unwrap(); + let root = fixture.path().join("workspace"); + std::fs::create_dir_all(&root).unwrap(); + let protected = vec!["build/spec.ts".to_string()]; + + assert_eq!( + protected_path_mutation("env FOO=1 tee build/spec.ts", &root, &protected), + Some("build/spec.ts".to_string()) + ); + assert_eq!( + protected_path_mutation("sudo -u root rm build/spec.ts", &root, &protected), + Some("build/spec.ts".to_string()) + ); + assert!(protected_path_mutation("env FOO=1 cargo test", &root, &protected).is_none()); + } } diff --git a/src/tools/bash_write_guard/command_prefix.rs b/src/tools/bash_write_guard/command_prefix.rs new file mode 100644 index 00000000..35c40cac --- /dev/null +++ b/src/tools/bash_write_guard/command_prefix.rs @@ -0,0 +1,1204 @@ +//! Command-prefix peeling for the Bash write-target inspector (Issue #566). +//! +//! The write target of a shell segment belongs to the program that actually +//! runs, not to the first word of the segment. Prefix programs (`env`, `sudo`, +//! `timeout`, `nice`, reserved words such as `{` and `if`, ...) sit in front of +//! that program, so the inspector peels them before it decides which program's +//! write table applies. +//! +//! [`resolve`] applies the table repeatedly until a fixpoint and returns one of +//! three outcomes: a resolved program, "nothing executes", or "the chain cannot +//! be resolved". An unresolvable chain fails closed: [`resolve`] scans the +//! remaining words and reports the chain as unresolvable only when that scan +//! finds a write program or another prefix. An ordinary verification command +//! such as `timeout 600 cargo test` therefore stays allowed. + +use std::path::Path; + +use super::is_environment_assignment; + +/// Upper bound on how many prefixes are peeled before the chain is treated as +/// unresolvable. A deeper chain is a fail-closed case, never an unbounded walk. +const MAX_DEPTH: usize = 16; + +/// Operation recorded for a segment whose prefix chain could not be resolved +/// while a write program or another prefix remained in the words. +pub(super) const UNRESOLVED_OPERATION: &str = "unresolved command prefix"; + +/// A write destination named by a prefix itself rather than by the program it +/// launches, e.g. `time -o FILE` or `sudo -e FILE`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) struct PrefixWrite { + pub path: String, + pub operation: String, +} + +/// Outcome of peeling the prefixes of a single segment's words. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) enum Resolution { + /// A program runs; `arguments` are the words after it and `writes` are the + /// destinations the prefix chain named itself. + Program { + program: String, + arguments: Vec, + writes: Vec, + }, + /// The words do not launch a program (e.g. `command -v`, `sudo -l`). + NoExecution { writes: Vec }, + /// The chain is unresolvable and the remaining words name a write program + /// or another prefix, so the caller must reject the segment. + Undecidable { prefix: String }, +} + +/// Peels the command prefixes of one segment and decides what runs. +pub(super) fn resolve(words: &[&str]) -> Resolution { + let mut writes: Vec = Vec::new(); + let mut cursor = 0usize; + let mut depth = 0usize; + loop { + let Some(word) = words.get(cursor) else { + return Resolution::NoExecution { writes }; + }; + let name = basename(word); + let Some(prefix) = prefix_for(name) else { + return Resolution::Program { + program: name.to_string(), + arguments: words[cursor + 1..] + .iter() + .map(|word| (*word).to_string()) + .collect(), + writes, + }; + }; + if depth >= MAX_DEPTH { + return unresolvable(words, cursor, writes); + } + match step(prefix, cursor, &words[cursor + 1..], &mut writes) { + Step::Continue { cursor: next } => { + cursor = next; + depth += 1; + } + Step::Stop => return Resolution::NoExecution { writes }, + Step::Undecidable => return unresolvable(words, cursor, writes), + } + } +} + +/// A prefix program or reserved word that may sit in front of the real program. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Prefix { + Env, + Command, + Builtin, + Exec, + Nohup, + Sudo, + Doas, + Timeout, + Nice, + Ionice, + Stdbuf, + Time, + Setsid, + Caffeinate, + Arch, + Bang, + BraceOpen, + BraceClose, + If, + Then, + Elif, + Else, + While, + Until, + Do, + Coproc, + Function, +} + +fn prefix_for(name: &str) -> Option { + Some(match name { + "env" => Prefix::Env, + "command" => Prefix::Command, + "builtin" => Prefix::Builtin, + "exec" => Prefix::Exec, + "nohup" => Prefix::Nohup, + "sudo" => Prefix::Sudo, + "doas" => Prefix::Doas, + "timeout" | "gtimeout" => Prefix::Timeout, + "nice" => Prefix::Nice, + "ionice" => Prefix::Ionice, + "stdbuf" => Prefix::Stdbuf, + "time" => Prefix::Time, + "setsid" => Prefix::Setsid, + "caffeinate" => Prefix::Caffeinate, + "arch" => Prefix::Arch, + "!" => Prefix::Bang, + "{" => Prefix::BraceOpen, + "}" => Prefix::BraceClose, + "if" => Prefix::If, + "then" => Prefix::Then, + "elif" => Prefix::Elif, + "else" => Prefix::Else, + "while" => Prefix::While, + "until" => Prefix::Until, + "do" => Prefix::Do, + "coproc" => Prefix::Coproc, + "function" => Prefix::Function, + _ => return None, + }) +} + +/// How a prefix consumes the words that follow it. +enum Step { + /// Peeling may continue at `cursor`. + Continue { cursor: usize }, + /// The words launch nothing; the segment has no program. + Stop, + /// The prefix cannot be peeled without ambiguity. + Undecidable, +} + +fn step(prefix: Prefix, index: usize, args: &[&str], writes: &mut Vec) -> Step { + match prefix { + Prefix::Env => step_env(index, args), + Prefix::Command => step_command(index, args), + Prefix::Exec => step_exec(index, args), + Prefix::Sudo => step_sudo(index, args, writes), + Prefix::Timeout => step_timeout(index, args), + Prefix::Nice => step_nice(index, args), + Prefix::Stdbuf => step_stdbuf(index, args), + Prefix::Time => step_time(index, args, writes), + Prefix::Doas => step_options(index, args, &["-n", "-L"], &["-u", "-C"], &[]), + Prefix::Ionice => step_options( + index, + args, + &["-t", "--ignore"], + &["-c", "--class", "-n", "--classdata", "-p", "--pid"], + &["--help", "--version"], + ), + Prefix::Setsid => step_options( + index, + args, + &["-c", "--ctty", "-f", "--fork", "-w", "--wait"], + &[], + &["--help", "--version"], + ), + Prefix::Caffeinate => step_options( + index, + args, + &["-d", "-i", "-m", "-s", "-u"], + &["-t", "-w"], + &[], + ), + Prefix::Arch => step_options( + index, + args, + &["-arm64", "-i386", "-x86_64"], + &["-arch"], + &["-h"], + ), + Prefix::Builtin + | Prefix::Nohup + | Prefix::Bang + | Prefix::BraceOpen + | Prefix::BraceClose + | Prefix::If + | Prefix::Then + | Prefix::Elif + | Prefix::Else + | Prefix::While + | Prefix::Until + | Prefix::Do + | Prefix::Coproc => step_bare(index, args), + Prefix::Function => step_function(index, args), + } +} + +/// Peels a part-of-speech prefix that consumes no option words of its own. +fn step_bare(index: usize, args: &[&str]) -> Step { + if args.is_empty() { + Step::Stop + } else { + Step::Continue { cursor: index + 1 } + } +} + +/// `function NAME` consumes the function name before the body. +fn step_function(index: usize, args: &[&str]) -> Step { + if args.is_empty() { + Step::Stop + } else { + Step::Continue { cursor: index + 2 } + } +} + +/// Peels a prefix whose option grammar is a flag/value/stop table. +fn step_options( + index: usize, + args: &[&str], + flags: &[&str], + value_options: &[&str], + stop_options: &[&str], +) -> Step { + let mut offset = 0; + while let Some(&word) = args.get(offset) { + if word == "--" { + return Step::Continue { + cursor: index + 1 + offset + 1, + }; + } + if stop_options.contains(&word) { + return Step::Stop; + } + if flags.contains(&word) { + offset += 1; + continue; + } + if value_options.contains(&word) { + if args.get(offset + 1).is_none() { + return Step::Undecidable; + } + offset += 2; + continue; + } + if let Some((name, _)) = word.split_once('=') + && value_options.contains(&name) + { + offset += 1; + continue; + } + if word.starts_with('-') && word.len() > 1 { + return Step::Undecidable; + } + return Step::Continue { + cursor: index + 1 + offset, + }; + } + Step::Stop +} + +fn step_env(index: usize, args: &[&str]) -> Step { + let mut offset = 0; + while let Some(&word) = args.get(offset) { + if word == "--" { + return Step::Continue { + cursor: index + 1 + offset + 1, + }; + } + if matches!( + word, + "-i" | "-" | "-0" | "-v" | "--ignore-environment" | "--null" | "--debug" + ) { + offset += 1; + continue; + } + if matches!(word, "-S" | "--split-string" | "-C" | "--chdir") + || word.starts_with("--split-string=") + || word.starts_with("--chdir=") + { + return Step::Undecidable; + } + if matches!(word, "-u" | "--unset" | "-P" | "--path") { + if args.get(offset + 1).is_none() { + return Step::Undecidable; + } + offset += 2; + continue; + } + if word.starts_with("--unset=") || word.starts_with("--path=") { + offset += 1; + continue; + } + if is_environment_assignment(word) { + offset += 1; + continue; + } + if word.starts_with('-') && word.len() > 1 { + return Step::Undecidable; + } + return Step::Continue { + cursor: index + 1 + offset, + }; + } + Step::Stop +} + +fn step_command(index: usize, args: &[&str]) -> Step { + let mut offset = 0; + while let Some(&word) = args.get(offset) { + match word { + "--" => { + return Step::Continue { + cursor: index + 1 + offset + 1, + }; + } + "-v" | "-V" => return Step::Stop, + "-p" => offset += 1, + w if w.starts_with('-') && w.len() > 1 => return Step::Undecidable, + _ => { + return Step::Continue { + cursor: index + 1 + offset, + }; + } + } + } + Step::Stop +} + +fn step_exec(index: usize, args: &[&str]) -> Step { + let mut offset = 0; + while let Some(&word) = args.get(offset) { + match word { + "--" => { + return Step::Continue { + cursor: index + 1 + offset + 1, + }; + } + "-c" | "-l" => offset += 1, + "-a" => { + if args.get(offset + 1).is_none() { + return Step::Undecidable; + } + offset += 2; + } + w if w.starts_with('-') && w.len() > 1 => return Step::Undecidable, + _ => { + return Step::Continue { + cursor: index + 1 + offset, + }; + } + } + } + Step::Stop +} + +fn step_sudo(index: usize, args: &[&str], writes: &mut Vec) -> Step { + const FLAGS: &[&str] = &[ + "-E", + "--preserve-env", + "-H", + "--set-home", + "-n", + "--non-interactive", + "-A", + "--askpass", + "-b", + "--background", + "-P", + "--preserve-groups", + "-S", + "--stdin", + ]; + const VALUE_OPTIONS: &[&str] = &[ + "-u", + "--user", + "-g", + "--group", + "-h", + "--host", + "-p", + "--prompt", + "-C", + "--close-from", + "-T", + "--command-timeout", + "-r", + "--role", + "-t", + "--type", + ]; + const TERMINAL: &[&str] = &[ + "-l", + "--list", + "-v", + "--validate", + "-k", + "--reset-timestamp", + "-K", + "--remove-timestamp", + "-V", + "--version", + ]; + let mut offset = 0; + while let Some(&word) = args.get(offset) { + if word == "--" { + return Step::Continue { + cursor: index + 1 + offset + 1, + }; + } + if TERMINAL.contains(&word) { + return Step::Stop; + } + if matches!(word, "-i" | "--login" | "-s" | "--shell") + || matches!(word, "-D" | "--chdir" | "-R" | "--chroot") + || word.starts_with("--chdir=") + || word.starts_with("--chroot=") + { + return Step::Undecidable; + } + if matches!(word, "-e" | "--edit") { + let Some(file) = args.get(offset + 1) else { + return Step::Undecidable; + }; + writes.push(PrefixWrite { + path: (*file).to_string(), + operation: "sudo".to_string(), + }); + return Step::Stop; + } + if FLAGS.contains(&word) { + offset += 1; + continue; + } + if let Some((name, _)) = word.split_once('=') + && VALUE_OPTIONS.contains(&name) + { + offset += 1; + continue; + } + if VALUE_OPTIONS.contains(&word) { + if args.get(offset + 1).is_none() { + return Step::Undecidable; + } + offset += 2; + continue; + } + if let Some((option, _)) = split_attached(word) + && VALUE_OPTIONS.contains(&option.as_str()) + { + offset += 1; + continue; + } + if word.starts_with('-') && word.len() > 1 { + return Step::Undecidable; + } + return Step::Continue { + cursor: index + 1 + offset, + }; + } + Step::Stop +} + +fn step_timeout(index: usize, args: &[&str]) -> Step { + let mut offset = 0; + while let Some(&word) = args.get(offset) { + match word { + "--" => { + offset += 1; + break; + } + "--help" | "--version" => return Step::Stop, + "--preserve-status" | "--foreground" | "--verbose" | "-v" => offset += 1, + "-s" | "--signal" | "-k" | "--kill-after" => { + if args.get(offset + 1).is_none() { + return Step::Undecidable; + } + offset += 2; + } + w if w.starts_with("--signal=") || w.starts_with("--kill-after=") => offset += 1, + w if w.starts_with('-') && w.len() > 1 => return Step::Undecidable, + _ => break, + } + } + if args.get(offset).is_none() { + return Step::Stop; + } + offset += 1; + if args.get(offset).is_none() { + return Step::Stop; + } + Step::Continue { + cursor: index + 1 + offset, + } +} + +fn step_nice(index: usize, args: &[&str]) -> Step { + let mut offset = 0; + while let Some(&word) = args.get(offset) { + if word == "--" { + return Step::Continue { + cursor: index + 1 + offset + 1, + }; + } + if matches!(word, "--help" | "--version") { + return Step::Stop; + } + if matches!(word, "-n" | "--adjustment") { + if args.get(offset + 1).is_none() { + return Step::Undecidable; + } + offset += 2; + continue; + } + if word.starts_with("--adjustment=") { + offset += 1; + continue; + } + if is_numeric_adjustment(word) { + offset += 1; + continue; + } + if word.starts_with('-') && word.len() > 1 { + return Step::Undecidable; + } + return Step::Continue { + cursor: index + 1 + offset, + }; + } + Step::Stop +} + +fn step_stdbuf(index: usize, args: &[&str]) -> Step { + const VALUE_OPTIONS: &[&str] = &["-i", "-o", "-e", "--input", "--output", "--error"]; + let mut offset = 0; + while let Some(&word) = args.get(offset) { + if word == "--" { + return Step::Continue { + cursor: index + 1 + offset + 1, + }; + } + if matches!(word, "--help" | "--version") { + return Step::Stop; + } + if VALUE_OPTIONS.contains(&word) { + if args.get(offset + 1).is_none() { + return Step::Undecidable; + } + offset += 2; + continue; + } + if let Some((name, _)) = word.split_once('=') + && VALUE_OPTIONS.contains(&name) + { + offset += 1; + continue; + } + if let Some((option, _)) = split_attached(word) + && VALUE_OPTIONS.contains(&option.as_str()) + { + offset += 1; + continue; + } + if word.starts_with('-') && word.len() > 1 { + return Step::Undecidable; + } + return Step::Continue { + cursor: index + 1 + offset, + }; + } + Step::Stop +} + +fn step_time(index: usize, args: &[&str], writes: &mut Vec) -> Step { + const FLAGS: &[&str] = &[ + "-p", + "--portability", + "-v", + "--verbose", + "-a", + "--append", + "-q", + "--quiet", + ]; + const FORMAT_OPTIONS: &[&str] = &["-f", "--format"]; + const OUTPUT_OPTIONS: &[&str] = &["-o", "--output"]; + let mut offset = 0; + while let Some(&word) = args.get(offset) { + if word == "--" { + return Step::Continue { + cursor: index + 1 + offset + 1, + }; + } + if matches!(word, "--help" | "--version" | "-V") { + return Step::Stop; + } + if FLAGS.contains(&word) { + offset += 1; + continue; + } + if FORMAT_OPTIONS.contains(&word) { + if args.get(offset + 1).is_none() { + return Step::Undecidable; + } + offset += 2; + continue; + } + if OUTPUT_OPTIONS.contains(&word) { + let Some(path) = args.get(offset + 1) else { + return Step::Undecidable; + }; + writes.push(PrefixWrite { + path: (*path).to_string(), + operation: "time".to_string(), + }); + offset += 2; + continue; + } + if let Some((name, value)) = word.split_once('=') { + if FORMAT_OPTIONS.contains(&name) { + offset += 1; + continue; + } + if OUTPUT_OPTIONS.contains(&name) { + writes.push(PrefixWrite { + path: value.to_string(), + operation: "time".to_string(), + }); + offset += 1; + continue; + } + } + if let Some((option, value)) = split_attached(word) { + if FORMAT_OPTIONS.contains(&option.as_str()) { + offset += 1; + continue; + } + if OUTPUT_OPTIONS.contains(&option.as_str()) { + writes.push(PrefixWrite { + path: value, + operation: "time".to_string(), + }); + offset += 1; + continue; + } + } + if word.starts_with('-') && word.len() > 1 { + return Step::Undecidable; + } + return Step::Continue { + cursor: index + 1 + offset, + }; + } + Step::Stop +} + +/// Fails closed for an unresolvable chain: if any remaining word (or a +/// whitespace-separated fragment of a quoted word, as `env -S 'CMD'` produces) +/// is a write program or another prefix, the chain is reported unresolvable. +fn unresolvable(words: &[&str], stuck: usize, writes: Vec) -> Resolution { + for word in &words[stuck + 1..] { + for piece in word.split_whitespace() { + let name = basename(piece); + if is_write_program(name) || prefix_for(name).is_some() { + return Resolution::Undecidable { + prefix: words[stuck].to_string(), + }; + } + } + } + Resolution::NoExecution { writes } +} + +fn is_write_program(name: &str) -> bool { + matches!( + name, + "tee" + | "cp" + | "mv" + | "install" + | "ln" + | "mkdir" + | "rm" + | "touch" + | "truncate" + | "chmod" + | "chown" + | "cd" + ) +} + +fn is_numeric_adjustment(word: &str) -> bool { + let Some(digits) = word.strip_prefix('-') else { + return false; + }; + !digits.is_empty() && digits.chars().all(|ch| ch.is_ascii_digit()) +} + +/// Splits a short option with an attached value (`-uroot`, `-oL`, `-oFILE`). +fn split_attached(word: &str) -> Option<(String, String)> { + let mut chars = word.chars(); + if chars.next()? != '-' { + return None; + } + let option = chars.next()?; + if option == '-' { + return None; + } + Some((format!("-{option}"), chars.as_str().to_string())) +} + +fn basename(word: &str) -> &str { + Path::new(word) + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or(word) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn program(program: &str, arguments: &[&str]) -> Resolution { + Resolution::Program { + program: program.to_string(), + arguments: arguments.iter().map(|item| (*item).to_string()).collect(), + writes: Vec::new(), + } + } + + fn no_execution() -> Resolution { + Resolution::NoExecution { writes: Vec::new() } + } + + fn write(path: &str, operation: &str) -> PrefixWrite { + PrefixWrite { + path: path.to_string(), + operation: operation.to_string(), + } + } + + #[test] + fn command_prefix_resolves_known_prefix_chains() { + assert_eq!(resolve(&["tee", "/tmp/f"]), program("tee", &["/tmp/f"])); + assert_eq!( + resolve(&["env", "tee", "/tmp/f"]), + program("tee", &["/tmp/f"]) + ); + assert_eq!( + resolve(&["env", "FOO=1", "tee", "out.txt"]), + program("tee", &["out.txt"]) + ); + assert_eq!( + resolve(&["/usr/bin/env", "-u", "HOME", "cp", "a", "b"]), + program("cp", &["a", "b"]) + ); + assert_eq!( + resolve(&["sudo", "-u", "root", "tee", "/tmp/f"]), + program("tee", &["/tmp/f"]) + ); + assert_eq!( + resolve(&["timeout", "-k", "1", "5", "tee", "/tmp/f"]), + program("tee", &["/tmp/f"]) + ); + assert_eq!( + resolve(&["nice", "-5", "tee", "/tmp/f"]), + program("tee", &["/tmp/f"]) + ); + assert_eq!( + resolve(&["stdbuf", "-oL", "tee", "/tmp/f"]), + program("tee", &["/tmp/f"]) + ); + assert_eq!( + resolve(&["{", "tee", "/tmp/f", "}"]), + program("tee", &["/tmp/f", "}"]) + ); + assert_eq!( + resolve(&["command", "env", "nohup", "tee", "/tmp/f"]), + program("tee", &["/tmp/f"]) + ); + } + + #[test] + fn command_prefix_records_prefix_owned_write_targets() { + assert_eq!( + resolve(&["/usr/bin/time", "-o", "/tmp/f", "true"]), + Resolution::Program { + program: "true".to_string(), + arguments: Vec::new(), + writes: vec![write("/tmp/f", "time")], + } + ); + assert_eq!( + resolve(&["sudo", "-e", "/tmp/f"]), + Resolution::NoExecution { + writes: vec![write("/tmp/f", "sudo")], + } + ); + } + + #[test] + fn command_prefix_marks_non_executing_modes() { + assert_eq!(resolve(&["command", "-v", "cargo"]), no_execution()); + assert_eq!(resolve(&["sudo", "-l", "tee", "/tmp/f"]), no_execution()); + } + + #[test] + fn command_prefix_marks_unresolvable_chains() { + assert_eq!( + resolve(&["sudo", "-X", "u", "tee", "/tmp/f"]), + Resolution::Undecidable { + prefix: "sudo".to_string(), + } + ); + assert_eq!( + resolve(&["env", "-S", "tee /tmp/f"]), + Resolution::Undecidable { + prefix: "env".to_string(), + } + ); + assert_eq!( + resolve(&["env", "-C", "sub", "tee", "f"]), + Resolution::Undecidable { + prefix: "env".to_string(), + } + ); + assert_eq!( + resolve(&["sudo", "-s", "tee", "/tmp/f"]), + Resolution::Undecidable { + prefix: "sudo".to_string(), + } + ); + } + + #[test] + fn command_prefix_keeps_unidentified_verification_chains_allowed() { + assert_eq!( + resolve(&["timeout", "--unknown", "600", "cargo", "test"]), + no_execution() + ); + assert_eq!( + resolve(&["env", "FOO=1", "cargo", "test"]), + program("cargo", &["test"]) + ); + } + + #[test] + fn command_prefix_rejects_chains_past_the_depth_limit() { + let mut words: Vec<&str> = std::iter::repeat_n("env", MAX_DEPTH).collect(); + words.extend(["tee", "/tmp/f"]); + assert_eq!( + resolve(&words), + program("tee", &["/tmp/f"]), + "exactly the depth limit still resolves" + ); + + words.insert(0, "env"); + assert_eq!( + resolve(&words), + Resolution::Undecidable { + prefix: "env".to_string(), + }, + "one prefix past the depth limit fails closed" + ); + } + + #[test] + fn command_prefix_splits_attached_short_options() { + assert_eq!( + split_attached("-uroot"), + Some(("-u".to_string(), "root".to_string())) + ); + assert_eq!( + split_attached("-oL"), + Some(("-o".to_string(), "L".to_string())) + ); + assert_eq!(split_attached("--user"), None); + assert_eq!(split_attached("-"), None); + } + + #[test] + fn command_prefix_recognises_every_table_entry() { + for name in [ + "env", + "command", + "builtin", + "exec", + "nohup", + "sudo", + "doas", + "timeout", + "gtimeout", + "nice", + "ionice", + "stdbuf", + "time", + "setsid", + "caffeinate", + "arch", + "!", + "{", + "}", + "if", + "then", + "elif", + "else", + "while", + "until", + "do", + "coproc", + "function", + ] { + assert!(prefix_for(name).is_some(), "table must include `{name}`"); + } + for name in ["tee", "cargo", "sh", "xargs", "watch", "sudoedit", ""] { + assert!(prefix_for(name).is_none(), "`{name}` is not a prefix"); + } + } + + #[test] + fn command_prefix_resolution_table() { + let undecidable = |prefix: &str| Resolution::Undecidable { + prefix: prefix.to_string(), + }; + let cases: &[(&[&str], Resolution)] = &[ + // env flags, value options, assignments and `--`. + (&["env", "-i", "tee", "f"], program("tee", &["f"])), + (&["env", "-", "tee", "f"], program("tee", &["f"])), + (&["env", "-0", "tee", "f"], program("tee", &["f"])), + (&["env", "-v", "tee", "f"], program("tee", &["f"])), + ( + &["env", "--ignore-environment", "tee", "f"], + program("tee", &["f"]), + ), + (&["env", "--null", "tee", "f"], program("tee", &["f"])), + (&["env", "--debug", "tee", "f"], program("tee", &["f"])), + (&["env", "-u", "HOME", "tee", "f"], program("tee", &["f"])), + ( + &["env", "--unset", "HOME", "tee", "f"], + program("tee", &["f"]), + ), + ( + &["env", "-P", "/usr/bin", "tee", "f"], + program("tee", &["f"]), + ), + ( + &["env", "--path", "/usr/bin", "tee", "f"], + program("tee", &["f"]), + ), + (&["env", "--unset=HOME", "tee", "f"], program("tee", &["f"])), + (&["env", "--path=/x", "tee", "f"], program("tee", &["f"])), + (&["env", "--", "tee", "f"], program("tee", &["f"])), + ( + &["env", "A=1", "-i", "B=2", "tee", "f"], + program("tee", &["f"]), + ), + // command. + (&["command", "-p", "tee", "f"], program("tee", &["f"])), + (&["command", "--", "tee", "f"], program("tee", &["f"])), + (&["command", "-v", "cargo"], no_execution()), + (&["command", "-V", "cargo"], no_execution()), + // builtin / exec / nohup. + (&["builtin", "cd", "/tmp"], program("cd", &["/tmp"])), + ( + &["exec", "-c", "-l", "-a", "x", "tee", "f"], + program("tee", &["f"]), + ), + (&["exec", "--", "tee", "f"], program("tee", &["f"])), + (&["nohup", "tee", "f"], program("tee", &["f"])), + // sudo. + (&["sudo", "-E", "tee", "f"], program("tee", &["f"])), + ( + &["sudo", "-H", "-n", "-A", "-b", "-P", "-S", "tee", "f"], + program("tee", &["f"]), + ), + (&["sudo", "-u", "root", "tee", "f"], program("tee", &["f"])), + (&["sudo", "-uroot", "tee", "f"], program("tee", &["f"])), + (&["sudo", "--user=root", "tee", "f"], program("tee", &["f"])), + ( + &["sudo", "-g", "g", "-h", "host", "tee", "f"], + program("tee", &["f"]), + ), + ( + &["sudo", "-p", "p", "-C", "9", "-T", "5", "tee", "f"], + program("tee", &["f"]), + ), + ( + &["sudo", "-r", "r", "-t", "t", "tee", "f"], + program("tee", &["f"]), + ), + (&["sudo", "--", "tee", "f"], program("tee", &["f"])), + (&["sudo", "-l", "tee", "f"], no_execution()), + (&["sudo", "--version", "tee", "f"], no_execution()), + // doas. + (&["doas", "-n", "-L", "tee", "f"], program("tee", &["f"])), + (&["doas", "-u", "root", "tee", "f"], program("tee", &["f"])), + ( + &["doas", "-C", "/etc/doas.conf", "tee", "f"], + program("tee", &["f"]), + ), + (&["doas", "--", "tee", "f"], program("tee", &["f"])), + // timeout / gtimeout. + (&["timeout", "5", "tee", "f"], program("tee", &["f"])), + (&["gtimeout", "5", "tee", "f"], program("tee", &["f"])), + ( + &["timeout", "--preserve-status", "5", "tee", "f"], + program("tee", &["f"]), + ), + ( + &["timeout", "--foreground", "5", "tee", "f"], + program("tee", &["f"]), + ), + ( + &["timeout", "--verbose", "5", "tee", "f"], + program("tee", &["f"]), + ), + (&["timeout", "-v", "5", "tee", "f"], program("tee", &["f"])), + ( + &["timeout", "-s", "KILL", "5", "tee", "f"], + program("tee", &["f"]), + ), + ( + &["timeout", "--signal", "KILL", "5", "tee", "f"], + program("tee", &["f"]), + ), + ( + &["timeout", "--signal=KILL", "5", "tee", "f"], + program("tee", &["f"]), + ), + ( + &["timeout", "-k", "1", "5", "tee", "f"], + program("tee", &["f"]), + ), + ( + &["timeout", "--kill-after=1", "5", "tee", "f"], + program("tee", &["f"]), + ), + (&["timeout", "--", "5", "tee", "f"], program("tee", &["f"])), + (&["timeout", "--help"], no_execution()), + // nice. + (&["nice", "-n", "5", "tee", "f"], program("tee", &["f"])), + ( + &["nice", "--adjustment", "5", "tee", "f"], + program("tee", &["f"]), + ), + ( + &["nice", "--adjustment=5", "tee", "f"], + program("tee", &["f"]), + ), + (&["nice", "-5", "tee", "f"], program("tee", &["f"])), + (&["nice", "--", "tee", "f"], program("tee", &["f"])), + (&["nice", "--version"], no_execution()), + // ionice. + (&["ionice", "-t", "tee", "f"], program("tee", &["f"])), + (&["ionice", "--ignore", "tee", "f"], program("tee", &["f"])), + (&["ionice", "-c", "3", "tee", "f"], program("tee", &["f"])), + ( + &["ionice", "--class", "3", "tee", "f"], + program("tee", &["f"]), + ), + (&["ionice", "--class=3", "tee", "f"], program("tee", &["f"])), + (&["ionice", "-n", "7", "tee", "f"], program("tee", &["f"])), + ( + &["ionice", "--classdata", "7", "tee", "f"], + program("tee", &["f"]), + ), + (&["ionice", "-p", "123", "tee", "f"], program("tee", &["f"])), + ( + &["ionice", "--pid", "123", "tee", "f"], + program("tee", &["f"]), + ), + (&["ionice", "--", "tee", "f"], program("tee", &["f"])), + (&["ionice", "--help"], no_execution()), + // stdbuf. + (&["stdbuf", "-i", "L", "tee", "f"], program("tee", &["f"])), + (&["stdbuf", "-o", "L", "tee", "f"], program("tee", &["f"])), + (&["stdbuf", "-e", "L", "tee", "f"], program("tee", &["f"])), + (&["stdbuf", "-oL", "tee", "f"], program("tee", &["f"])), + (&["stdbuf", "--input=L", "tee", "f"], program("tee", &["f"])), + ( + &["stdbuf", "--output=L", "tee", "f"], + program("tee", &["f"]), + ), + (&["stdbuf", "--error=L", "tee", "f"], program("tee", &["f"])), + (&["stdbuf", "--", "tee", "f"], program("tee", &["f"])), + (&["stdbuf", "--help"], no_execution()), + // time. + (&["time", "-p", "tee", "f"], program("tee", &["f"])), + (&["time", "-v", "tee", "f"], program("tee", &["f"])), + (&["time", "-a", "tee", "f"], program("tee", &["f"])), + (&["time", "-q", "tee", "f"], program("tee", &["f"])), + ( + &["time", "--portability", "tee", "f"], + program("tee", &["f"]), + ), + (&["time", "--verbose", "tee", "f"], program("tee", &["f"])), + (&["time", "--append", "tee", "f"], program("tee", &["f"])), + (&["time", "--quiet", "tee", "f"], program("tee", &["f"])), + (&["time", "-f", "%e", "tee", "f"], program("tee", &["f"])), + (&["time", "--format=%e", "tee", "f"], program("tee", &["f"])), + (&["time", "--", "tee", "f"], program("tee", &["f"])), + (&["time", "--version"], no_execution()), + // setsid / caffeinate / arch. + (&["setsid", "-f", "-w", "tee", "f"], program("tee", &["f"])), + (&["setsid", "--fork", "tee", "f"], program("tee", &["f"])), + (&["setsid", "--wait", "tee", "f"], program("tee", &["f"])), + ( + &["caffeinate", "-d", "-i", "-m", "-s", "-u", "tee", "f"], + program("tee", &["f"]), + ), + ( + &["caffeinate", "-t", "5", "tee", "f"], + program("tee", &["f"]), + ), + ( + &["caffeinate", "-w", "9", "tee", "f"], + program("tee", &["f"]), + ), + (&["arch", "-arm64", "tee", "f"], program("tee", &["f"])), + (&["arch", "-i386", "tee", "f"], program("tee", &["f"])), + (&["arch", "-x86_64", "tee", "f"], program("tee", &["f"])), + ( + &["arch", "-arch", "arm64", "tee", "f"], + program("tee", &["f"]), + ), + (&["arch", "-h"], no_execution()), + // flag/value option-table edges: unknown options fail closed, + // values that follow flags still resolve, and nested prefixes use + // an offset that is not zero. + (&["doas", "-X", "tee", "f"], undecidable("doas")), + (&["doas", "-u"], no_execution()), + (&["ionice", "-X", "tee", "f"], undecidable("ionice")), + (&["ionice", "-c"], no_execution()), + ( + &["ionice", "-t", "-c", "3", "tee", "f"], + program("tee", &["f"]), + ), + (&["setsid", "-X", "tee", "f"], undecidable("setsid")), + (&["caffeinate", "-X", "tee", "f"], undecidable("caffeinate")), + (&["arch", "-bogus", "tee", "f"], undecidable("arch")), + (&["env", "ionice", "--", "tee", "f"], program("tee", &["f"])), + ( + &["env", "doas", "-u", "root", "tee", "f"], + program("tee", &["f"]), + ), + ( + &["env", "arch", "-arm64", "tee", "f"], + program("tee", &["f"]), + ), + // shell prefixes and reserved words. + (&["!", "tee", "f"], program("tee", &["f"])), + (&["{", "tee", "f"], program("tee", &["f"])), + (&["}", "tee", "f"], program("tee", &["f"])), + (&["if", "tee", "f"], program("tee", &["f"])), + (&["then", "tee", "f"], program("tee", &["f"])), + (&["elif", "tee", "f"], program("tee", &["f"])), + (&["else", "tee", "f"], program("tee", &["f"])), + (&["while", "tee", "f"], program("tee", &["f"])), + (&["until", "tee", "f"], program("tee", &["f"])), + (&["do", "tee", "f"], program("tee", &["f"])), + (&["coproc", "tee", "f"], program("tee", &["f"])), + (&["function", "f", "tee", "x"], program("tee", &["x"])), + // prefix only, without a command. + (&["env"], no_execution()), + (&["timeout", "5"], no_execution()), + (&["command"], no_execution()), + (&["sudo", "-l"], no_execution()), + // unresolvable forms, including nested prefixes. + (&["env", "-C", "sub", "tee", "f"], undecidable("env")), + (&["env", "--chdir=sub", "tee", "f"], undecidable("env")), + (&["env", "-S", "tee f"], undecidable("env")), + (&["env", "-X", "tee", "f"], undecidable("env")), + (&["sudo", "-i", "tee", "f"], undecidable("sudo")), + (&["sudo", "-s", "tee", "f"], undecidable("sudo")), + (&["sudo", "-D", "/tmp", "tee", "f"], undecidable("sudo")), + (&["sudo", "-R", "/tmp", "tee", "f"], undecidable("sudo")), + (&["sudo", "--chdir=/tmp", "tee", "f"], undecidable("sudo")), + (&["sudo", "--chroot=/tmp", "tee", "f"], undecidable("sudo")), + (&["env", "sudo", "-s", "tee", "f"], undecidable("sudo")), + ]; + for (words, expected) in cases { + assert_eq!(resolve(words), *expected, "words: {words:?}"); + } + } +} diff --git a/tests/issue566_bash_command_prefix_write_targets.rs b/tests/issue566_bash_command_prefix_write_targets.rs new file mode 100644 index 00000000..3606228f --- /dev/null +++ b/tests/issue566_bash_command_prefix_write_targets.rs @@ -0,0 +1,156 @@ +#![cfg(unix)] + +//! Issue #566: the Bash write-target guard must decide the program after +//! peeling command prefixes (`env`, `sudo`, `timeout`, `nice`, reserved words, +//! ...) rather than from the first word of the segment, so a write launched +//! through a prefix is judged against the workspace boundary. Every test name +//! contains `command_prefix` so the mutation-test filter selects these tests +//! together with the unit tests in `bash_write_guard/command_prefix.rs`. + +use std::path::{Path, PathBuf}; + +use commandagent::tools::bash::path_confinement_rejection; + +struct Fixture { + _dir: tempfile::TempDir, + root: PathBuf, +} + +/// A workspace with an escaping `sub/link` symlink and an inside `a.txt`. +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(); + 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 command_prefix_rejects_writes_through_every_prefix() { + let fixture = fixture(); + let root = &fixture.root; + let cases = [ + // The problem table from the issue body. + "env tee /tmp/f", + "env FOO=1 cp a.txt /tmp/f", + "sudo cp a.txt /tmp/f", + "command cp a.txt /tmp/f", + "nohup tee /tmp/f", + "timeout 5 cp a.txt /tmp/f", + "{ tee /tmp/f; }", + "! tee /tmp/f", + "if true; then tee /tmp/f; fi", + "while tee /tmp/f; do :; done", + // env options. + "env -i tee /tmp/f", + "env -u HOME tee /tmp/f", + "env -u HOME FOO=1 tee /tmp/f", + "env -- tee /tmp/f", + "/usr/bin/env tee /tmp/f", + "env -S \"tee /tmp/f\"", + // command / exec. + "command -p tee /tmp/f", + "exec tee /tmp/f", + "exec -a x tee /tmp/f", + // sudo / doas. + "sudo -u root tee /tmp/f", + "sudo -E tee /tmp/f", + "sudo -- tee /tmp/f", + "sudo -s tee /tmp/f", + "sudo -e /tmp/f", + "doas -u root tee /tmp/f", + // timeout / nice / ionice / stdbuf / time. + "timeout -s KILL 5 tee /tmp/f", + "timeout -k 1 5 tee /tmp/f", + "timeout --preserve-status 5s tee /tmp/f", + "nice -n 5 tee /tmp/f", + "nice -5 tee /tmp/f", + "ionice -c 3 tee /tmp/f", + "stdbuf -oL tee /tmp/f", + "time -p tee /tmp/f", + "/usr/bin/time -o /tmp/f true", + // reserved words / grouping. + "for x in a; do tee /tmp/f; done", + "true && { tee /tmp/f; }", + "f() { tee /tmp/f; }", + "coproc tee /tmp/f", + // nested. + "env sudo timeout 5 tee /tmp/f", + // undecidable option. + "sudo -X u tee /tmp/f", + // The same writes through the escaping `sub/link` symlink. + "env tee sub/link/f", + "timeout 5 cp a.txt sub/link/", + "{ tee sub/link/f; }", + ]; + for command in cases { + assert!( + path_confinement_rejection(command, root).is_some(), + "expected rejection: {command}" + ); + } +} + +#[test] +fn command_prefix_rejection_names_the_resolved_write_operation() { + let fixture = fixture(); + let root = &fixture.root; + for (command, operation) in [ + ("env tee /tmp/f", "tee"), + ("timeout 5 cp a.txt /tmp/f", "cp"), + ("sudo -u root tee /tmp/f", "tee"), + ("command cp a.txt /tmp/f", "cp"), + ("timeout 5 cp a.txt sub/link/", "cp"), + ] { + let rejection = path_confinement_rejection(command, root) + .unwrap_or_else(|| panic!("expected rejection: {command}")); + assert_eq!(rejection.operation, operation, "command: {command}"); + } +} + +#[test] +fn command_prefix_keeps_inside_and_verification_writes_allowed() { + let fixture = fixture(); + let root = &fixture.root; + let cases = [ + "env FOO=1 tee out.txt", + "printf x | env tee out.txt", + "timeout 5 cp a.txt b.txt", + "env tee /dev/null", + "env FOO=1 cargo test", + "env RUST_LOG=debug cargo test", + "timeout 600 cargo test", + "timeout --foreground 600 cargo test", + "timeout --unknown 600 cargo test", + "nice cargo build", + "nohup cargo build", + "time cargo test", + "{ cargo test; }", + "command -v cargo", + "command -v tee /tmp/f", + "sudo -l tee /tmp/f", + ]; + for command in cases { + assert!( + path_confinement_rejection(command, root).is_none(), + "expected allow: {command}" + ); + } +} + +#[test] +fn command_prefix_rejection_reports_the_workspace_relative_retry() { + let fixture = fixture(); + let root = &fixture.root; + let rejection = path_confinement_rejection("env tee /tmp/f", root).expect("rejected"); + assert!( + rejection.operation == "tee" || !rejection.nearest_relative.is_empty(), + "a rejection must keep the existing report shape" + ); + assert!(Path::new(&rejection.root).is_absolute()); +} From eddc6620ca82ce0492a71c9a851298d9412aa194 Mon Sep 17 00:00:00 2001 From: kewton Date: Fri, 2 Oct 2026 13:24:53 +0900 Subject: [PATCH 2/6] fix(tools): close command-prefix write-target gaps found in review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit レビューで見つかった前置きの表の抜け 4 件をふさぐ。 - env -S / --split-string / -iS の文字列 operand を取り出して空白で分け、書き込み program か前置きがあれば拒否する - nohup・builtin は直後の -- を飛ばしてから program を決める - sudo の -k / --reset-timestamp を「実行しない」の表から flag の表へ移す - 走査の書き込み program の一覧に sudoedit を足す テストの不足（env -C / --chdir / sudo -D / env -i -- / exec -- / exec -X、env -S の各形、深さの境目 16/17、command -）も追加。 判断: 設計 5（exec の前置きと shell の前置きの区別）は使う側がないため今回は実装しない（#568 で入れる） 判断: env -S の値は空白分割した各語の basename が書き込み program か前置きのとき拒否に倒す 本文に無い指摘: リーダーが cargo mutants を回すため、ワーカー側では実行していない Co-authored-by: CommandCodeBot --- src/tools/bash_write_guard/command_prefix.rs | 130 +++++++++++++++--- ...ue566_bash_command_prefix_write_targets.rs | 55 ++++++++ 2 files changed, 167 insertions(+), 18 deletions(-) diff --git a/src/tools/bash_write_guard/command_prefix.rs b/src/tools/bash_write_guard/command_prefix.rs index 35c40cac..1c7a32f4 100644 --- a/src/tools/bash_write_guard/command_prefix.rs +++ b/src/tools/bash_write_guard/command_prefix.rs @@ -80,6 +80,11 @@ pub(super) fn resolve(words: &[&str]) -> Resolution { } Step::Stop => return Resolution::NoExecution { writes }, Step::Undecidable => return unresolvable(words, cursor, writes), + Step::Reject => { + return Resolution::Undecidable { + prefix: words[cursor].to_string(), + }; + } } } } @@ -157,6 +162,8 @@ enum Step { Stop, /// The prefix cannot be peeled without ambiguity. Undecidable, + /// The prefix is proven to carry a write program or another prefix. + Reject, } fn step(prefix: Prefix, index: usize, args: &[&str], writes: &mut Vec) -> Step { @@ -198,9 +205,8 @@ fn step(prefix: Prefix, index: usize, args: &[&str], writes: &mut Vec step_bare_end_of_options(index, args), + Prefix::Bang | Prefix::BraceOpen | Prefix::BraceClose | Prefix::If @@ -224,6 +230,16 @@ fn step_bare(index: usize, args: &[&str]) -> Step { } } +/// Like [`step_bare`], but skips a `--` end-of-options marker so the program +/// that follows it is still resolved (`nohup --`, `builtin --`). +fn step_bare_end_of_options(index: usize, args: &[&str]) -> Step { + match args.first() { + None => Step::Stop, + Some(&"--") => Step::Continue { cursor: index + 2 }, + Some(_) => Step::Continue { cursor: index + 1 }, + } +} + /// `function NAME` consumes the function name before the body. fn step_function(index: usize, args: &[&str]) -> Step { if args.is_empty() { @@ -293,12 +309,31 @@ fn step_env(index: usize, args: &[&str]) -> Step { offset += 1; continue; } - if matches!(word, "-S" | "--split-string" | "-C" | "--chdir") - || word.starts_with("--split-string=") - || word.starts_with("--chdir=") - { + if matches!(word, "-C" | "--chdir") || word.starts_with("--chdir=") { return Step::Undecidable; } + if word == "-S" || word == "--split-string" { + let Some(value) = args.get(offset + 1) else { + return Step::Undecidable; + }; + if contains_write_or_prefix(value) { + return Step::Reject; + } + offset += 2; + continue; + } + if let Some(value) = word.strip_prefix("--split-string=") { + if contains_write_or_prefix(value) { + return Step::Reject; + } + offset += 1; + continue; + } + if let Some(value) = short_cluster_split_string(word) + && contains_write_or_prefix(value) + { + return Step::Reject; + } if matches!(word, "-u" | "--unset" | "-P" | "--path") { if args.get(offset + 1).is_none() { return Step::Undecidable; @@ -389,6 +424,8 @@ fn step_sudo(index: usize, args: &[&str], writes: &mut Vec) -> Step "--preserve-groups", "-S", "--stdin", + "-k", + "--reset-timestamp", ]; const VALUE_OPTIONS: &[&str] = &[ "-u", @@ -413,8 +450,6 @@ fn step_sudo(index: usize, args: &[&str], writes: &mut Vec) -> Step "--list", "-v", "--validate", - "-k", - "--reset-timestamp", "-K", "--remove-timestamp", "-V", @@ -677,19 +712,41 @@ fn step_time(index: usize, args: &[&str], writes: &mut Vec) -> Step /// whitespace-separated fragment of a quoted word, as `env -S 'CMD'` produces) /// is a write program or another prefix, the chain is reported unresolvable. fn unresolvable(words: &[&str], stuck: usize, writes: Vec) -> Resolution { - for word in &words[stuck + 1..] { - for piece in word.split_whitespace() { - let name = basename(piece); - if is_write_program(name) || prefix_for(name).is_some() { - return Resolution::Undecidable { - prefix: words[stuck].to_string(), - }; - } - } + if words[stuck + 1..] + .iter() + .any(|word| contains_write_or_prefix(word)) + { + return Resolution::Undecidable { + prefix: words[stuck].to_string(), + }; } Resolution::NoExecution { writes } } +/// Whether a text names a write program or another prefix when split the way a +/// shell splits `env -S`'s string operand. +fn contains_write_or_prefix(text: &str) -> bool { + text.split_whitespace().any(|piece| { + let name = basename(piece); + is_write_program(name) || prefix_for(name).is_some() + }) +} + +/// The `env -S`/`-iS` short form: returns the string operand attached after the +/// `S` letter of a single-dash option cluster. A value-taking option (`-u`, +/// `-P`) before the `S` means the remainder belongs to that option instead. +fn short_cluster_split_string(word: &str) -> Option<&str> { + let rest = word.strip_prefix('-')?; + if rest.is_empty() || rest.starts_with('-') { + return None; + } + let index = rest.find('S')?; + if rest[..index].chars().any(|ch| ch == 'u' || ch == 'P') { + return None; + } + rest.get(index + 1..) +} + fn is_write_program(name: &str) -> bool { matches!( name, @@ -705,6 +762,7 @@ fn is_write_program(name: &str) -> bool { | "chmod" | "chown" | "cd" + | "sudoedit" ) } @@ -881,6 +939,22 @@ mod tests { }, "one prefix past the depth limit fails closed" ); + + // The same boundary with a non-writing program: the scan finds nothing + // to reject, so the chain stays allowed. + let mut cargo_words: Vec<&str> = std::iter::repeat_n("env", MAX_DEPTH).collect(); + cargo_words.extend(["cargo", "test"]); + assert_eq!( + resolve(&cargo_words), + program("cargo", &["test"]), + "exactly the depth limit still resolves a verification command" + ); + cargo_words.insert(0, "env"); + assert_eq!( + resolve(&cargo_words), + no_execution(), + "past the depth limit a verification command stays allowed" + ); } #[test] @@ -1166,6 +1240,26 @@ mod tests { &["env", "arch", "-arm64", "tee", "f"], program("tee", &["f"]), ), + // B1: env -S / --split-string string operands are scanned. + (&["env", "-Stee /tmp/f"], undecidable("env")), + (&["env", "-iStee /tmp/f"], undecidable("env")), + (&["env", "--split-string=tee /tmp/f"], undecidable("env")), + (&["env", "-Scp a.txt /tmp/f"], undecidable("env")), + (&["env", "-S", "tee /tmp/f"], undecidable("env")), + (&["env", "-Scargo test"], no_execution()), + // B2/B3: nohup and builtin skip a leading `--`. + (&["nohup", "--", "tee", "f"], program("tee", &["f"])), + (&["builtin", "--", "cd", "x"], program("cd", &["x"])), + // B4: sudo -k still runs the command. + (&["sudo", "-k", "tee", "f"], program("tee", &["f"])), + (&["sudo", "-n", "-k", "tee", "f"], program("tee", &["f"])), + (&["sudo", "-k"], no_execution()), + (&["sudo", "-K", "tee", "f"], no_execution()), + // sudoedit is a write program for the unresolvable scan. + (&["env", "-C", "sub", "sudoedit", "f"], undecidable("env")), + (&["sudo", "-X", "u", "sudoedit", "f"], undecidable("sudo")), + // `command -` runs nothing in sh, so `-` is not treated as the program. + (&["command", "-", "tee", "f"], program("-", &["tee", "f"])), // shell prefixes and reserved words. (&["!", "tee", "f"], program("tee", &["f"])), (&["{", "tee", "f"], program("tee", &["f"])), diff --git a/tests/issue566_bash_command_prefix_write_targets.rs b/tests/issue566_bash_command_prefix_write_targets.rs index 3606228f..e81ec859 100644 --- a/tests/issue566_bash_command_prefix_write_targets.rs +++ b/tests/issue566_bash_command_prefix_write_targets.rs @@ -26,6 +26,7 @@ fn fixture() -> Fixture { 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(); + std::os::unix::fs::symlink(&outside, root.join("linked-outside")).unwrap(); let root = root.canonicalize().unwrap(); Fixture { _dir: dir, root } } @@ -83,6 +84,28 @@ fn command_prefix_rejects_writes_through_every_prefix() { "env sudo timeout 5 tee /tmp/f", // undecidable option. "sudo -X u tee /tmp/f", + // Review round-1 holes: cwd-changing and end-of-options prefixes. + "env -C sub tee link/f", + "env --chdir=sub tee link/f", + "sudo -D sub tee link/f", + "env -i -- tee /tmp/f", + "exec -- tee /tmp/f", + "exec -X tee /tmp/f", + // B1: the env -S string operand is scanned. + "env -S'tee /tmp/f'", + "env -iS'tee /tmp/f'", + "env --split-string='tee /tmp/f'", + "env -S\"cp a.txt /tmp/f\"", + // B2/B3: nohup and builtin skip a leading `--`. + "nohup -- tee /tmp/f", + "builtin -- cd linked-outside && tee f", + // B4: sudo -k still runs the command. + "sudo -k tee /tmp/f", + "sudo -k -u root cp a.txt /tmp/f", + "sudo -n -k tee /tmp/f", + // sudoedit is a write program for the unresolved scan. + "env -C sub sudoedit link/f", + "sudo -X u sudoedit /tmp/f", // The same writes through the escaping `sub/link` symlink. "env tee sub/link/f", "timeout 5 cp a.txt sub/link/", @@ -134,6 +157,14 @@ fn command_prefix_keeps_inside_and_verification_writes_allowed() { "command -v cargo", "command -v tee /tmp/f", "sudo -l tee /tmp/f", + // `command -` runs nothing in sh, so `-` is not a program. + "command - tee /tmp/f", + // B1: an env -S string with no write program stays allowed. + "env -S'cargo test'", + // B4: `sudo -k` without a command, and the terminal timestamp options. + "sudo -k", + "sudo -K", + "sudo -v tee /tmp/f", ]; for command in cases { assert!( @@ -143,6 +174,30 @@ fn command_prefix_keeps_inside_and_verification_writes_allowed() { } } +#[test] +fn command_prefix_depth_limit_boundary() { + let fixture = fixture(); + let root = &fixture.root; + + let past_limit_write = format!("{}tee /tmp/f", "env ".repeat(17)); + assert!( + path_confinement_rejection(&past_limit_write, root).is_some(), + "17 nested prefixes before a write must still reject" + ); + + let past_limit_cargo = format!("{}cargo test", "env ".repeat(17)); + assert!( + path_confinement_rejection(&past_limit_cargo, root).is_none(), + "17 nested prefixes before a verification command must stay allowed" + ); + + let within_limit_write = format!("{}tee out.txt", "env ".repeat(16)); + assert!( + path_confinement_rejection(&within_limit_write, root).is_none(), + "16 nested prefixes still peel to the write program" + ); +} + #[test] fn command_prefix_rejection_reports_the_workspace_relative_retry() { let fixture = fixture(); From 68c744d68f45400c576b097a39e1ce0e1548a3ea Mon Sep 17 00:00:00 2001 From: kewton Date: Fri, 2 Oct 2026 17:47:49 +0900 Subject: [PATCH 3/6] test(tools): pin command-prefix offsets with inside-write detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cargo mutants の生存 34 件のうち、offset・cursor のずれが観測できる形を足す。 値を取る option の後ろに workspace 内の書き込み program を置き、閉じ込め許可・ has_recognized_mutation true・保護 path 検出が path を返す の 3 点を固定する。 unknown option と bare `-` の境目、short_cluster_split_string の境目も直接固定。 本文に無い指摘: `env -Sx tee out.txt` と `env -iS x tee out.txt` は -S の値に書き込みが無いのに拒否される。short_cluster 経路が -S の値を使い切らず continue しないためで、fail-closed の安全側だが設計の「誤拒否を絞る」より厳しい。製品は凍結のため未修正、報告のみ 判断: 352:48・469:13・470:13・529:36・563:32・600:32 は等価変異（到達しない len==1、fall-through も同じ Undecidable、args.get(offset) が常に Some）と判断しテストで殺さない 判断: 312:45・315:25 は指示どおり等価として扱い無視する Co-authored-by: CommandCodeBot --- src/tools/bash_write_guard/command_prefix.rs | 146 ++++++++++++++++++ ...ue566_bash_command_prefix_write_targets.rs | 53 +++++++ 2 files changed, 199 insertions(+) diff --git a/src/tools/bash_write_guard/command_prefix.rs b/src/tools/bash_write_guard/command_prefix.rs index 1c7a32f4..f746af58 100644 --- a/src/tools/bash_write_guard/command_prefix.rs +++ b/src/tools/bash_write_guard/command_prefix.rs @@ -1295,4 +1295,150 @@ mod tests { assert_eq!(resolve(words), *expected, "words: {words:?}"); } } + + fn command_prefix_inside_fixture() -> tempfile::TempDir { + let fixture = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(fixture.path().join("ws")).unwrap(); + fixture + } + + /// An inside write must stay allowed *and* stay recognized. A cursor that + /// drops the write program (or picks an option value as the program) makes + /// either the mutation flag or the protected-path lookup disappear, so the + /// three assertions below pin the offset arithmetic. + fn assert_command_prefix_inside_write(command: &str, target: &str) { + let fixture = command_prefix_inside_fixture(); + let root = fixture.path().join("ws"); + assert!( + super::super::confinement_rejection(command, &root).is_none(), + "must stay allowed: {command}" + ); + assert!( + super::super::has_recognized_mutation(command), + "must stay a recognized mutation: {command}" + ); + assert_eq!( + super::super::protected_path_mutation(command, &root, &[target.to_string()]), + Some(target.to_string()), + "must protect the path: {command}" + ); + } + + fn assert_command_prefix_no_write(command: &str) { + let fixture = command_prefix_inside_fixture(); + let root = fixture.path().join("ws"); + assert!( + super::super::confinement_rejection(command, &root).is_none(), + "must stay allowed: {command}" + ); + assert!( + !super::super::has_recognized_mutation(command), + "must not be a recognized mutation: {command}" + ); + assert_eq!( + super::super::protected_path_mutation(command, &root, &["out.txt".to_string()]), + None, + "must not protect a path: {command}" + ); + } + + fn assert_command_prefix_rejected(command: &str) { + let fixture = command_prefix_inside_fixture(); + let root = fixture.path().join("ws"); + assert!( + super::super::confinement_rejection(command, &root).is_some(), + "must be rejected: {command}" + ); + } + + #[test] + fn command_prefix_option_values_keep_inside_writes_detected() { + let cases: &[(&str, &str)] = &[ + ("env -S x tee out.txt", "out.txt"), + ("env -S 'x' tee out.txt", "out.txt"), + ("env --split-string x tee out.txt", "out.txt"), + ("env --split-string=x tee out.txt", "out.txt"), + ("env -i -- tee out.txt", "out.txt"), + ("env -u V tee out.txt", "out.txt"), + ("env - tee out.txt", "out.txt"), + ("command -p tee out.txt", "out.txt"), + ("exec -a n tee out.txt", "out.txt"), + ("exec -c tee out.txt", "out.txt"), + ("exec -l tee out.txt", "out.txt"), + ("exec -- tee out.txt", "out.txt"), + ("exec -c -- tee out.txt", "out.txt"), + ("sudo -u root tee out.txt", "out.txt"), + ("sudo -g g tee out.txt", "out.txt"), + ("sudo -n tee out.txt", "out.txt"), + ("sudo -- tee out.txt", "out.txt"), + ("timeout 5 tee out.txt", "out.txt"), + ("timeout -s TERM 5 tee out.txt", "out.txt"), + ("timeout -k 1 5 tee out.txt", "out.txt"), + ("timeout --signal=TERM 5 tee out.txt", "out.txt"), + ("timeout - tee out.txt", "out.txt"), + ("nice -n 5 tee out.txt", "out.txt"), + ("nice -5 tee out.txt", "out.txt"), + ("nice --adjustment=5 tee out.txt", "out.txt"), + ("nice -n -5 tee out.txt", "out.txt"), + ("nice -n 5 -- tee out.txt", "out.txt"), + ("stdbuf -o L tee out.txt", "out.txt"), + ("stdbuf -oL tee out.txt", "out.txt"), + ("stdbuf -i0 -o0 tee out.txt", "out.txt"), + ("stdbuf --output=L tee out.txt", "out.txt"), + ("time -p tee out.txt", "out.txt"), + ("/usr/bin/time -f F tee out.txt", "out.txt"), + ("/usr/bin/time -o log tee out.txt", "out.txt"), + ("/usr/bin/time -v tee out.txt", "out.txt"), + ("/usr/bin/time -a -o log tee out.txt", "out.txt"), + ("/usr/bin/time --output=log tee out.txt", "out.txt"), + ("/usr/bin/time -fF tee out.txt", "out.txt"), + ("/usr/bin/time -olog tee out.txt", "out.txt"), + ]; + for (command, target) in cases { + assert_command_prefix_inside_write(command, target); + } + } + + #[test] + fn command_prefix_unknown_options_and_bare_dash_boundaries() { + for command in [ + "command - tee out.txt", + "command -v tee out.txt", + "exec - tee out.txt", + "nice - tee out.txt", + "stdbuf - tee out.txt", + "/usr/bin/time - tee out.txt", + ] { + assert_command_prefix_no_write(command); + } + for command in [ + "command -X tee /tmp/f", + "exec -X tee /tmp/f", + "nice -X tee /tmp/f", + "stdbuf -X tee /tmp/f", + "/usr/bin/time -X tee /tmp/f", + "sudo -i tee /tmp/f", + "sudo -s tee /tmp/f", + "sudo -D sub tee /tmp/f", + "sudo -R sub tee /tmp/f", + ] { + assert_command_prefix_rejected(command); + } + } + + #[test] + fn command_prefix_short_cluster_split_string_table() { + assert_eq!(short_cluster_split_string("-S"), Some("")); + assert_eq!(short_cluster_split_string("-iS"), Some("")); + assert_eq!(short_cluster_split_string("-iSx"), Some("x")); + assert_eq!(short_cluster_split_string("-Sx"), Some("x")); + assert_eq!(short_cluster_split_string("-i"), None); + assert_eq!(short_cluster_split_string("-ab"), None); + assert_eq!(short_cluster_split_string("-x"), None); + assert_eq!(short_cluster_split_string("-"), None); + assert_eq!(short_cluster_split_string("--Sfoo"), None); + assert_eq!(short_cluster_split_string("-uSx"), None); + assert_eq!(short_cluster_split_string("-PSx"), None); + assert_eq!(short_cluster_split_string(""), None); + } } diff --git a/tests/issue566_bash_command_prefix_write_targets.rs b/tests/issue566_bash_command_prefix_write_targets.rs index e81ec859..0592bf10 100644 --- a/tests/issue566_bash_command_prefix_write_targets.rs +++ b/tests/issue566_bash_command_prefix_write_targets.rs @@ -106,6 +106,14 @@ fn command_prefix_rejects_writes_through_every_prefix() { // sudoedit is a write program for the unresolved scan. "env -C sub sudoedit link/f", "sudo -X u sudoedit /tmp/f", + // Unknown options on the value-taking prefixes stay fail-closed. + "command -X tee /tmp/f", + "nice -X tee /tmp/f", + "stdbuf -X tee /tmp/f", + "/usr/bin/time -X tee /tmp/f", + "sudo -i tee /tmp/f", + "sudo -R sub tee /tmp/f", + "timeout -f 5 tee /tmp/f", // The same writes through the escaping `sub/link` symlink. "env tee sub/link/f", "timeout 5 cp a.txt sub/link/", @@ -165,6 +173,51 @@ fn command_prefix_keeps_inside_and_verification_writes_allowed() { "sudo -k", "sudo -K", "sudo -v tee /tmp/f", + // A program reached after an option value must still be seen; the + // write is inside the workspace, so confinement stays allowed. + "env -S x tee out.txt", + "env --split-string x tee out.txt", + "env --split-string=x tee out.txt", + "env -i -- tee out.txt", + "env -u V tee out.txt", + "env - tee out.txt", + "command -p tee out.txt", + "command - tee out.txt", + "command -v tee out.txt", + "exec -a n tee out.txt", + "exec -c tee out.txt", + "exec -l tee out.txt", + "exec -- tee out.txt", + "exec - tee out.txt", + "sudo -u root tee out.txt", + "sudo -g g tee out.txt", + "sudo -n tee out.txt", + "sudo -- tee out.txt", + "timeout 5 tee out.txt", + "timeout -s TERM 5 tee out.txt", + "timeout -k 1 5 tee out.txt", + "timeout --signal=TERM 5 tee out.txt", + "timeout - tee out.txt", + "nice -n 5 tee out.txt", + "nice -5 tee out.txt", + "nice --adjustment=5 tee out.txt", + "nice -n -5 tee out.txt", + "nice -n 5 -- tee out.txt", + "nice - tee out.txt", + "stdbuf -o L tee out.txt", + "stdbuf -oL tee out.txt", + "stdbuf -i0 -o0 tee out.txt", + "stdbuf --output=L tee out.txt", + "stdbuf - tee out.txt", + "time -p tee out.txt", + "/usr/bin/time -f F tee out.txt", + "/usr/bin/time -o log tee out.txt", + "/usr/bin/time -v tee out.txt", + "/usr/bin/time -a -o log tee out.txt", + "/usr/bin/time --output=log tee out.txt", + "/usr/bin/time -fF tee out.txt", + "/usr/bin/time -olog tee out.txt", + "/usr/bin/time - tee out.txt", ]; for command in cases { assert!( From 3d658779b7840d5f0ac7bd5b8900dcac3e0f8d03 Mon Sep 17 00:00:00 2001 From: kewton Date: Fri, 2 Oct 2026 17:59:52 +0900 Subject: [PATCH 4/6] fix(tools): extract env -S in the unresolved scan and refuse unverifiable operands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R1: 決められない形の走査（unresolvable → contains_write_or_prefix）でも env の -S の値を 取り出す。attached（-Stee）・flag をまとめた -iuX -S…・--split-string=…・-S ＋次の語の すべてを対象にし、走査が tee・cp などを見落とさないようにする。 R2: -S の値に " ' \ $ のいずれかが含まれていたら、確かめられない書式として拒否する （安全側）。含まれなければ今と同じく空白で分けて走査する。 結合テストに R1 拒否 5 行・R2 拒否 7 行・許可 2 行を追加。直す前（eddc6620 相当）では `env -S'"tee" /tmp/f'` と `env -uX -S'tee /tmp/f'` がいずれも拒否されず失敗し、直した後で 通ることを実測した（二点測定）。 判断: R2 の「確かめられない」判定は " ' \ $ の 4 文字を含む場合とする（指示どおり） 本文に無い指摘: 先に報告した `env -Sx tee out.txt` / `env -iS x tee out.txt` の誤拒否は今回の R1/R2 では変わらない（step_env の short_cluster 経路が値を使い切らず continue しないため）。拒否側で安全 Co-authored-by: CommandCodeBot --- src/tools/bash_write_guard/command_prefix.rs | 70 ++++++++++++++++--- ...ue566_bash_command_prefix_write_targets.rs | 16 +++++ 2 files changed, 76 insertions(+), 10 deletions(-) diff --git a/src/tools/bash_write_guard/command_prefix.rs b/src/tools/bash_write_guard/command_prefix.rs index f746af58..0393ae47 100644 --- a/src/tools/bash_write_guard/command_prefix.rs +++ b/src/tools/bash_write_guard/command_prefix.rs @@ -316,21 +316,21 @@ fn step_env(index: usize, args: &[&str]) -> Step { let Some(value) = args.get(offset + 1) else { return Step::Undecidable; }; - if contains_write_or_prefix(value) { + if env_split_string_unsafe(value) { return Step::Reject; } offset += 2; continue; } if let Some(value) = word.strip_prefix("--split-string=") { - if contains_write_or_prefix(value) { + if env_split_string_unsafe(value) { return Step::Reject; } offset += 1; continue; } if let Some(value) = short_cluster_split_string(word) - && contains_write_or_prefix(value) + && env_split_string_unsafe(value) { return Step::Reject; } @@ -708,14 +708,11 @@ fn step_time(index: usize, args: &[&str], writes: &mut Vec) -> Step Step::Stop } -/// Fails closed for an unresolvable chain: if any remaining word (or a -/// whitespace-separated fragment of a quoted word, as `env -S 'CMD'` produces) -/// is a write program or another prefix, the chain is reported unresolvable. +/// Fails closed for an unresolvable chain: if any remaining word (or the +/// string operand of an `env -S` in it) names a write program or another +/// prefix, the chain is reported unresolvable. fn unresolvable(words: &[&str], stuck: usize, writes: Vec) -> Resolution { - if words[stuck + 1..] - .iter() - .any(|word| contains_write_or_prefix(word)) - { + if unresolvable_scan_rejects(&words[stuck + 1..]) { return Resolution::Undecidable { prefix: words[stuck].to_string(), }; @@ -723,6 +720,35 @@ fn unresolvable(words: &[&str], stuck: usize, writes: Vec) -> Resol Resolution::NoExecution { writes } } +/// Scans the words after an unresolvable prefix. An `env -S` string operand is +/// extracted exactly as `step_env` does, so an attached (`-Stee`), clustered +/// (`-iStee`), `=`-joined (`--split-string=tee`), or following-word +/// (`-S tee`) operand is inspected instead of being split as an option word. +fn unresolvable_scan_rejects(words: &[&str]) -> bool { + let mut index = 0; + while let Some(&word) = words.get(index) { + if let Some(value) = word.strip_prefix("--split-string=") { + if env_split_string_unsafe(value) { + return true; + } + } else if word == "-S" || word == "--split-string" { + if let Some(value) = words.get(index + 1) + && env_split_string_unsafe(value) + { + return true; + } + } else if let Some(value) = short_cluster_split_string(word) { + if env_split_string_unsafe(value) { + return true; + } + } else if contains_write_or_prefix(word) { + return true; + } + index += 1; + } + false +} + /// Whether a text names a write program or another prefix when split the way a /// shell splits `env -S`'s string operand. fn contains_write_or_prefix(text: &str) -> bool { @@ -732,6 +758,16 @@ fn contains_write_or_prefix(text: &str) -> bool { }) } +/// Whether an `env -S` string operand must be refused. Quotes, escapes, and +/// expansions are not re-split the way the shell would, so the operand is not +/// verifiable; otherwise it is refused when it names a write program or prefix. +fn env_split_string_unsafe(value: &str) -> bool { + let unverifiable = value + .chars() + .any(|ch| matches!(ch, '"' | '\'' | '\\' | '$')); + unverifiable || contains_write_or_prefix(value) +} + /// The `env -S`/`-iS` short form: returns the string operand attached after the /// `S` letter of a single-dash option cluster. A value-taking option (`-u`, /// `-P`) before the `S` means the remainder belongs to that option instead. @@ -1247,6 +1283,20 @@ mod tests { (&["env", "-Scp a.txt /tmp/f"], undecidable("env")), (&["env", "-S", "tee /tmp/f"], undecidable("env")), (&["env", "-Scargo test"], no_execution()), + // R1: the env -S operand is extracted in the unresolved scan too. + (&["env", "-uX", "-Stee /tmp/f"], undecidable("env")), + (&["env", "-iuX", "-Scp a.txt /tmp/f"], undecidable("env")), + (&["env", "-C", "sub", "-Stee /tmp/f"], undecidable("env")), + (&["env", "--chdir=sub", "-Stee link/f"], undecidable("env")), + (&["env", "--unknown", "-Stee /tmp/f"], undecidable("env")), + (&["env", "-S"], no_execution()), + (&["env", "--split-string"], no_execution()), + // R2: a quoted, escaped, or expanded -S operand is unverifiable. + (&["env", "-S\"tee\" /tmp/f"], undecidable("env")), + (&["env", "-S'tee' /tmp/f"], undecidable("env")), + (&["env", "-Ste\"\"e /tmp/f"], undecidable("env")), + (&["env", "-Stee\\_/tmp/f"], undecidable("env")), + (&["env", "-S${X} /tmp/f"], undecidable("env")), // B2/B3: nohup and builtin skip a leading `--`. (&["nohup", "--", "tee", "f"], program("tee", &["f"])), (&["builtin", "--", "cd", "x"], program("cd", &["x"])), diff --git a/tests/issue566_bash_command_prefix_write_targets.rs b/tests/issue566_bash_command_prefix_write_targets.rs index 0592bf10..454d2978 100644 --- a/tests/issue566_bash_command_prefix_write_targets.rs +++ b/tests/issue566_bash_command_prefix_write_targets.rs @@ -114,6 +114,20 @@ fn command_prefix_rejects_writes_through_every_prefix() { "sudo -i tee /tmp/f", "sudo -R sub tee /tmp/f", "timeout -f 5 tee /tmp/f", + // R2: a quoted, backslashed, or expanded -S value is unverifiable. + "env -S'\"tee\" /tmp/f'", + "env -S\"'tee' /tmp/f\"", + "env -S'te\"\"e /tmp/f'", + "env -S'tee\\_/tmp/f'", + "env -S'cp\\_a.txt\\_/tmp/f'", + "X=tee env -S'${X} /tmp/f'", + "env -S'\"cd\" linked-outside'", + // R1: the env -S value is extracted in the unresolved scan too. + "env -uX -S'tee /tmp/f'", + "env -iuX -S'cp a.txt /tmp/f'", + "env -C sub -S'tee /tmp/f'", + "env --chdir=sub -S'tee link/f'", + "env --unknown -S'tee /tmp/f'", // The same writes through the escaping `sub/link` symlink. "env tee sub/link/f", "timeout 5 cp a.txt sub/link/", @@ -169,6 +183,8 @@ fn command_prefix_keeps_inside_and_verification_writes_allowed() { "command - tee /tmp/f", // B1: an env -S string with no write program stays allowed. "env -S'cargo test'", + "env -S", + "env --split-string", // B4: `sudo -k` without a command, and the terminal timestamp options. "sudo -k", "sudo -K", From 3024b7ab3211a185b4684fdd183bc3d341327bb3 Mon Sep 17 00:00:00 2001 From: kewton Date: Fri, 2 Oct 2026 19:40:08 +0900 Subject: [PATCH 5/6] test(tools): cover the -S following-word extraction in the unresolved scan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit unresolvable_scan_rejects の 2 生存（734:32 の ||→&&、735:50 の index + 1→index * 1）を 殺す行を足す。決められない前置きの後ろで -S / --split-string が独立した語になり、その次の語が R2 の対象（"・'・\・$ を含む）で write の語を含まない形を拒否の表に追加した。次の語を通常の語と して走査しても tee 等が見つからないため、次語の取り出し経路だけが拒否に到達する。 拒否: X=tee env --unknown -S '${X} /tmp/f' / X=tee env --unknown --split-string '${X} /tmp/f' / env -C sub -S '${X} /tmp/f' / env --unknown -S 'a\_b' 許可: env --unknown -S（-S が最後の語で値なし） Co-authored-by: CommandCodeBot --- tests/issue566_bash_command_prefix_write_targets.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/issue566_bash_command_prefix_write_targets.rs b/tests/issue566_bash_command_prefix_write_targets.rs index 454d2978..db0c477b 100644 --- a/tests/issue566_bash_command_prefix_write_targets.rs +++ b/tests/issue566_bash_command_prefix_write_targets.rs @@ -128,6 +128,13 @@ fn command_prefix_rejects_writes_through_every_prefix() { "env -C sub -S'tee /tmp/f'", "env --chdir=sub -S'tee link/f'", "env --unknown -S'tee /tmp/f'", + // An `-S`/`--split-string` written as its own word after an unresolved + // prefix: only the following-word extraction reaches the unverifiable + // operand, which carries no write word of its own. + "X=tee env --unknown -S '${X} /tmp/f'", + "X=tee env --unknown --split-string '${X} /tmp/f'", + "env -C sub -S '${X} /tmp/f'", + "env --unknown -S 'a\\_b'", // The same writes through the escaping `sub/link` symlink. "env tee sub/link/f", "timeout 5 cp a.txt sub/link/", @@ -185,6 +192,7 @@ fn command_prefix_keeps_inside_and_verification_writes_allowed() { "env -S'cargo test'", "env -S", "env --split-string", + "env --unknown -S", // B4: `sudo -k` without a command, and the terminal timestamp options. "sudo -k", "sudo -K", From dbc19a3f32d4c7ff733765096f46a8d76f344d8b Mon Sep 17 00:00:00 2001 From: kewton Date: Fri, 2 Oct 2026 23:09:24 +0900 Subject: [PATCH 6/6] fix(tools): refuse env -S / --split-string in every spelling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit env -S の綴りを 1 つずつ塞ぐ方針をやめ、値を問わず一律に拒否する。H-04 で E1〜E3 の blocker が出たため。 - step_env: env_requests_split_string で、-S を含む短い option のまとまり（-S・-iS・-vS・ -0S・-iuXS…）と --split-string の接頭辞（--s…--split-string、= の有無を問わず）を値を見ずに Step::Reject にする。値をとる option（-u・-C・-P）が先頭なら残りはその値として扱い対象外 （-uSOMETHING・-u NAME・-u S）。 - 走査（unresolvable_scan_rejects）は値の取り出しをやめ、語ごとの一律判定と contains_write_or_prefix だけにする（env -uX --unknown -S… でも拒否）。 - 新 Resolution::UnverifiableSplitString を追加し、理由を「env -S / --split-string」として 拒否する（Undecidable とは別扱い）。 - 消した関数: env_split_string_unsafe、short_cluster_split_string。Step::Reject は該当の option 語を持ち、bash_write_guard の write_targets/confinement_rejection に配線した。 変えた期待の行: - 結合 許可→拒否へ移動: env -S'cargo test' / env -S / env --split-string / env --unknown -S / env -S x tee out.txt / env --split-string x tee out.txt / env --split-string=x tee out.txt。 結合 拒否に追加: env -iS '"tee" /tmp/f' / env -vS 'tee\_/tmp/f' / env -0S '${X} /tmp/f' / env -S'-Stee /tmp/f' / env -S'--split-string=tee /tmp/f' / env --split='tee /tmp/f' / env --s='tee /tmp/f' / env --split-str='cp a.txt /tmp/f' / env -iuXS 'x' / env -uX --unknown -S 'cargo test' / sudo env -S'x y' / timeout 5 env -S'x y'。 結合 許可に追加: env -i PATH=/usr/bin cargo test / env -u HOME cargo test / env -uSOMETHING cargo test / env -u S cargo test。 - 単体 resolution_table: env -Scargo test・env -S・env --split-string を no_execution から UnverifiableSplitString へ、-S を含む他の行も同じく更新。command_prefix_marks_unresolvable_chains の env -S 行も UnverifiableSplitString へ。 - 単体 short_cluster_split_string_table を env_requests_split_string_table に置換。 - bash_write_guard の write_targets table の env -S 行を ("-S", UNVERIFIABLE_SPLIT_STRING_OPERATION) へ。 Co-authored-by: CommandCodeBot --- src/tools/bash_write_guard.rs | 18 +- src/tools/bash_write_guard/command_prefix.rs | 263 ++++++++++-------- ...ue566_bash_command_prefix_write_targets.rs | 32 ++- 3 files changed, 189 insertions(+), 124 deletions(-) diff --git a/src/tools/bash_write_guard.rs b/src/tools/bash_write_guard.rs index be3df1f5..696dc692 100644 --- a/src/tools/bash_write_guard.rs +++ b/src/tools/bash_write_guard.rs @@ -29,6 +29,16 @@ pub(super) fn confinement_rejection( root: &Path, ) -> Option { for target in write_targets(command) { + if target.operation == command_prefix::UNVERIFIABLE_SPLIT_STRING_OPERATION { + return Some(BashWriteConfinementRejection { + reason: format!( + "Bash command `{}` selects env -S / --split-string, whose command cannot be verified to remain in the Gate 1 workspace boundary", + target.path + ), + path: target.path, + operation: target.operation, + }); + } if target.operation == command_prefix::UNRESOLVED_OPERATION { return Some(BashWriteConfinementRejection { reason: format!( @@ -134,6 +144,12 @@ fn write_targets(command: &str) -> Vec { operation: command_prefix::UNRESOLVED_OPERATION.to_string(), }); } + command_prefix::Resolution::UnverifiableSplitString { word } => { + targets.push(WriteTarget { + path: word, + operation: command_prefix::UNVERIFIABLE_SPLIT_STRING_OPERATION.to_string(), + }); + } } } targets @@ -875,7 +891,7 @@ mod tests { ), ( "env -S \"tee /tmp/f\"", - &[("env", command_prefix::UNRESOLVED_OPERATION)], + &[("-S", command_prefix::UNVERIFIABLE_SPLIT_STRING_OPERATION)], ), ]; for (command, expected) in cases { diff --git a/src/tools/bash_write_guard/command_prefix.rs b/src/tools/bash_write_guard/command_prefix.rs index 0393ae47..c4036326 100644 --- a/src/tools/bash_write_guard/command_prefix.rs +++ b/src/tools/bash_write_guard/command_prefix.rs @@ -25,6 +25,10 @@ const MAX_DEPTH: usize = 16; /// while a write program or another prefix remained in the words. pub(super) const UNRESOLVED_OPERATION: &str = "unresolved command prefix"; +/// Operation recorded for a segment whose `env` option selects `-S` / +/// `--split-string`, whose command string cannot be verified. +pub(super) const UNVERIFIABLE_SPLIT_STRING_OPERATION: &str = "env -S / --split-string"; + /// A write destination named by a prefix itself rather than by the program it /// launches, e.g. `time -o FILE` or `sudo -e FILE`. #[derive(Debug, Clone, PartialEq, Eq)] @@ -48,6 +52,9 @@ pub(super) enum Resolution { /// The chain is unresolvable and the remaining words name a write program /// or another prefix, so the caller must reject the segment. Undecidable { prefix: String }, + /// The segment carries an `env` `-S` / `--split-string` spelling, whose + /// command string cannot be verified, so the caller must reject it. + UnverifiableSplitString { word: String }, } /// Peels the command prefixes of one segment and decides what runs. @@ -80,11 +87,7 @@ pub(super) fn resolve(words: &[&str]) -> Resolution { } Step::Stop => return Resolution::NoExecution { writes }, Step::Undecidable => return unresolvable(words, cursor, writes), - Step::Reject => { - return Resolution::Undecidable { - prefix: words[cursor].to_string(), - }; - } + Step::Reject { word } => return Resolution::UnverifiableSplitString { word }, } } } @@ -162,8 +165,9 @@ enum Step { Stop, /// The prefix cannot be peeled without ambiguity. Undecidable, - /// The prefix is proven to carry a write program or another prefix. - Reject, + /// The prefix carries an `env` `-S` / `--split-string` spelling, which + /// cannot be verified. Carries the offending option word. + Reject { word: String }, } fn step(prefix: Prefix, index: usize, args: &[&str], writes: &mut Vec) -> Step { @@ -302,6 +306,14 @@ fn step_env(index: usize, args: &[&str]) -> Step { cursor: index + 1 + offset + 1, }; } + // `env -S` / `--split-string` builds the command from a string operand + // that cannot be re-split the way the shell would, so every spelling is + // refused before its value is looked at. + if env_requests_split_string(word) { + return Step::Reject { + word: word.to_string(), + }; + } if matches!( word, "-i" | "-" | "-0" | "-v" | "--ignore-environment" | "--null" | "--debug" @@ -312,28 +324,6 @@ fn step_env(index: usize, args: &[&str]) -> Step { if matches!(word, "-C" | "--chdir") || word.starts_with("--chdir=") { return Step::Undecidable; } - if word == "-S" || word == "--split-string" { - let Some(value) = args.get(offset + 1) else { - return Step::Undecidable; - }; - if env_split_string_unsafe(value) { - return Step::Reject; - } - offset += 2; - continue; - } - if let Some(value) = word.strip_prefix("--split-string=") { - if env_split_string_unsafe(value) { - return Step::Reject; - } - offset += 1; - continue; - } - if let Some(value) = short_cluster_split_string(word) - && env_split_string_unsafe(value) - { - return Step::Reject; - } if matches!(word, "-u" | "--unset" | "-P" | "--path") { if args.get(offset + 1).is_none() { return Step::Undecidable; @@ -708,9 +698,9 @@ fn step_time(index: usize, args: &[&str], writes: &mut Vec) -> Step Step::Stop } -/// Fails closed for an unresolvable chain: if any remaining word (or the -/// string operand of an `env -S` in it) names a write program or another -/// prefix, the chain is reported unresolvable. +/// Fails closed for an unresolvable chain: if any remaining word selects +/// `env -S` / `--split-string`, or names a write program or another prefix, +/// the chain is reported unresolvable. fn unresolvable(words: &[&str], stuck: usize, writes: Vec) -> Resolution { if unresolvable_scan_rejects(&words[stuck + 1..]) { return Resolution::Undecidable { @@ -720,33 +710,13 @@ fn unresolvable(words: &[&str], stuck: usize, writes: Vec) -> Resol Resolution::NoExecution { writes } } -/// Scans the words after an unresolvable prefix. An `env -S` string operand is -/// extracted exactly as `step_env` does, so an attached (`-Stee`), clustered -/// (`-iStee`), `=`-joined (`--split-string=tee`), or following-word -/// (`-S tee`) operand is inspected instead of being split as an option word. +/// Scans the words after an unresolvable prefix with the same per-word rules: +/// an `env` `-S` / `--split-string` option is refused in any spelling, and a +/// write program or another prefix is refused. fn unresolvable_scan_rejects(words: &[&str]) -> bool { - let mut index = 0; - while let Some(&word) = words.get(index) { - if let Some(value) = word.strip_prefix("--split-string=") { - if env_split_string_unsafe(value) { - return true; - } - } else if word == "-S" || word == "--split-string" { - if let Some(value) = words.get(index + 1) - && env_split_string_unsafe(value) - { - return true; - } - } else if let Some(value) = short_cluster_split_string(word) { - if env_split_string_unsafe(value) { - return true; - } - } else if contains_write_or_prefix(word) { - return true; - } - index += 1; - } - false + words + .iter() + .any(|word| env_requests_split_string(word) || contains_write_or_prefix(word)) } /// Whether a text names a write program or another prefix when split the way a @@ -758,29 +728,26 @@ fn contains_write_or_prefix(text: &str) -> bool { }) } -/// Whether an `env -S` string operand must be refused. Quotes, escapes, and -/// expansions are not re-split the way the shell would, so the operand is not -/// verifiable; otherwise it is refused when it names a write program or prefix. -fn env_split_string_unsafe(value: &str) -> bool { - let unverifiable = value - .chars() - .any(|ch| matches!(ch, '"' | '\'' | '\\' | '$')); - unverifiable || contains_write_or_prefix(value) -} - -/// The `env -S`/`-iS` short form: returns the string operand attached after the -/// `S` letter of a single-dash option cluster. A value-taking option (`-u`, -/// `-P`) before the `S` means the remainder belongs to that option instead. -fn short_cluster_split_string(word: &str) -> Option<&str> { - let rest = word.strip_prefix('-')?; - if rest.is_empty() || rest.starts_with('-') { - return None; +/// Whether an `env` option word selects `-S` / `--split-string`, in any +/// spelling, so its command string cannot be verified. +/// +/// Long form: any non-empty prefix of `--split-string` (`--s` … `--split-string`), +/// with or without an attached `=`. Short form: a single-dash option cluster +/// that contains an `S` anywhere, unless its first letter is a value-taking +/// option (`-u`, `-C`, `-P`), whose remainder is that option's value. +fn env_requests_split_string(word: &str) -> bool { + if let Some(rest) = word.strip_prefix("--") { + let name = rest.split_once('=').map_or(rest, |(name, _)| name); + let name = format!("--{name}"); + return name.len() > 2 && "--split-string".starts_with(&name); } - let index = rest.find('S')?; - if rest[..index].chars().any(|ch| ch == 'u' || ch == 'P') { - return None; + let Some(rest) = word.strip_prefix('-') else { + return false; + }; + if rest.is_empty() || matches!(rest.chars().next(), Some('u' | 'C' | 'P')) { + return false; } - rest.get(index + 1..) + rest.contains('S') } fn is_write_program(name: &str) -> bool { @@ -927,8 +894,8 @@ mod tests { ); assert_eq!( resolve(&["env", "-S", "tee /tmp/f"]), - Resolution::Undecidable { - prefix: "env".to_string(), + Resolution::UnverifiableSplitString { + word: "-S".to_string(), } ); assert_eq!( @@ -1051,6 +1018,9 @@ mod tests { let undecidable = |prefix: &str| Resolution::Undecidable { prefix: prefix.to_string(), }; + let split_string = |word: &str| Resolution::UnverifiableSplitString { + word: word.to_string(), + }; let cases: &[(&[&str], Resolution)] = &[ // env flags, value options, assignments and `--`. (&["env", "-i", "tee", "f"], program("tee", &["f"])), @@ -1276,27 +1246,38 @@ mod tests { &["env", "arch", "-arm64", "tee", "f"], program("tee", &["f"]), ), - // B1: env -S / --split-string string operands are scanned. - (&["env", "-Stee /tmp/f"], undecidable("env")), - (&["env", "-iStee /tmp/f"], undecidable("env")), - (&["env", "--split-string=tee /tmp/f"], undecidable("env")), - (&["env", "-Scp a.txt /tmp/f"], undecidable("env")), - (&["env", "-S", "tee /tmp/f"], undecidable("env")), - (&["env", "-Scargo test"], no_execution()), - // R1: the env -S operand is extracted in the unresolved scan too. + // env -S / --split-string is refused in every spelling. + (&["env", "-Stee /tmp/f"], split_string("-Stee /tmp/f")), + (&["env", "-iStee /tmp/f"], split_string("-iStee /tmp/f")), + ( + &["env", "--split-string=tee /tmp/f"], + split_string("--split-string=tee /tmp/f"), + ), + ( + &["env", "-Scp a.txt /tmp/f"], + split_string("-Scp a.txt /tmp/f"), + ), + (&["env", "-S", "tee /tmp/f"], split_string("-S")), + (&["env", "-Scargo test"], split_string("-Scargo test")), + (&["env", "-S"], split_string("-S")), + (&["env", "--split-string"], split_string("--split-string")), + ( + &["env", "-S\"tee\" /tmp/f"], + split_string("-S\"tee\" /tmp/f"), + ), + (&["env", "-S'tee' /tmp/f"], split_string("-S'tee' /tmp/f")), + ( + &["env", "-Ste\"\"e /tmp/f"], + split_string("-Ste\"\"e /tmp/f"), + ), + (&["env", "-Stee\\_/tmp/f"], split_string("-Stee\\_/tmp/f")), + (&["env", "-S${X} /tmp/f"], split_string("-S${X} /tmp/f")), + // Behind an undecidable option the scan applies the same refusal. (&["env", "-uX", "-Stee /tmp/f"], undecidable("env")), (&["env", "-iuX", "-Scp a.txt /tmp/f"], undecidable("env")), (&["env", "-C", "sub", "-Stee /tmp/f"], undecidable("env")), (&["env", "--chdir=sub", "-Stee link/f"], undecidable("env")), (&["env", "--unknown", "-Stee /tmp/f"], undecidable("env")), - (&["env", "-S"], no_execution()), - (&["env", "--split-string"], no_execution()), - // R2: a quoted, escaped, or expanded -S operand is unverifiable. - (&["env", "-S\"tee\" /tmp/f"], undecidable("env")), - (&["env", "-S'tee' /tmp/f"], undecidable("env")), - (&["env", "-Ste\"\"e /tmp/f"], undecidable("env")), - (&["env", "-Stee\\_/tmp/f"], undecidable("env")), - (&["env", "-S${X} /tmp/f"], undecidable("env")), // B2/B3: nohup and builtin skip a leading `--`. (&["nohup", "--", "tee", "f"], program("tee", &["f"])), (&["builtin", "--", "cd", "x"], program("cd", &["x"])), @@ -1331,7 +1312,7 @@ mod tests { // unresolvable forms, including nested prefixes. (&["env", "-C", "sub", "tee", "f"], undecidable("env")), (&["env", "--chdir=sub", "tee", "f"], undecidable("env")), - (&["env", "-S", "tee f"], undecidable("env")), + (&["env", "-S", "tee f"], split_string("-S")), (&["env", "-X", "tee", "f"], undecidable("env")), (&["sudo", "-i", "tee", "f"], undecidable("sudo")), (&["sudo", "-s", "tee", "f"], undecidable("sudo")), @@ -1404,10 +1385,6 @@ mod tests { #[test] fn command_prefix_option_values_keep_inside_writes_detected() { let cases: &[(&str, &str)] = &[ - ("env -S x tee out.txt", "out.txt"), - ("env -S 'x' tee out.txt", "out.txt"), - ("env --split-string x tee out.txt", "out.txt"), - ("env --split-string=x tee out.txt", "out.txt"), ("env -i -- tee out.txt", "out.txt"), ("env -u V tee out.txt", "out.txt"), ("env - tee out.txt", "out.txt"), @@ -1477,18 +1454,74 @@ mod tests { } #[test] - fn command_prefix_short_cluster_split_string_table() { - assert_eq!(short_cluster_split_string("-S"), Some("")); - assert_eq!(short_cluster_split_string("-iS"), Some("")); - assert_eq!(short_cluster_split_string("-iSx"), Some("x")); - assert_eq!(short_cluster_split_string("-Sx"), Some("x")); - assert_eq!(short_cluster_split_string("-i"), None); - assert_eq!(short_cluster_split_string("-ab"), None); - assert_eq!(short_cluster_split_string("-x"), None); - assert_eq!(short_cluster_split_string("-"), None); - assert_eq!(short_cluster_split_string("--Sfoo"), None); - assert_eq!(short_cluster_split_string("-uSx"), None); - assert_eq!(short_cluster_split_string("-PSx"), None); - assert_eq!(short_cluster_split_string(""), None); + fn command_prefix_env_requests_split_string_table() { + // Short clusters with an S anywhere are refused. + for word in [ + "-S", + "-Sx", + "-iS", + "-vS", + "-0S", + "-iuXS", + "-SCARGO", + "-iStee /tmp/f", + ] { + assert!( + env_requests_split_string(word), + "must request split-string: {word}" + ); + } + // A value-taking option first owns the rest of the word. + for word in [ + "-u", + "-uroot", + "-uSOMETHING", + "-P/usr/bin", + "-PS", + "-Csub", + "-CS", + "-i", + "-", + "-0", + "-x", + "-X", + "S", + "tee", + "", + ] { + assert!( + !env_requests_split_string(word), + "must not request split-string: {word}" + ); + } + // Every prefix of --split-string, with or without `=`. + for word in [ + "--s", + "--sp", + "--split", + "--split-str", + "--split-string", + "--split-string=tee /tmp/f", + "--s='tee /tmp/f'", + ] { + assert!( + env_requests_split_string(word), + "must request split-string: {word}" + ); + } + for word in [ + "--", + "--ignore-environment", + "--null", + "--debug", + "--chdir", + "--unset=x", + "--split-stringx", + ] { + assert!( + !env_requests_split_string(word), + "must not request split-string: {word}" + ); + } } } diff --git a/tests/issue566_bash_command_prefix_write_targets.rs b/tests/issue566_bash_command_prefix_write_targets.rs index db0c477b..045171c4 100644 --- a/tests/issue566_bash_command_prefix_write_targets.rs +++ b/tests/issue566_bash_command_prefix_write_targets.rs @@ -135,6 +135,26 @@ fn command_prefix_rejects_writes_through_every_prefix() { "X=tee env --unknown --split-string '${X} /tmp/f'", "env -C sub -S '${X} /tmp/f'", "env --unknown -S 'a\\_b'", + // env -S / --split-string is refused in every spelling, even without a + // write word and even with no value at all. + "env -S'cargo test'", + "env -S", + "env --split-string", + "env -S x tee out.txt", + "env --split-string x tee out.txt", + "env --split-string=x tee out.txt", + "env -iS '\"tee\" /tmp/f'", + "env -vS 'tee\\_/tmp/f'", + "env -0S '${X} /tmp/f'", + "env -S'-Stee /tmp/f'", + "env -S'--split-string=tee /tmp/f'", + "env --split='tee /tmp/f'", + "env --s='tee /tmp/f'", + "env --split-str='cp a.txt /tmp/f'", + "env -iuXS 'x'", + "env -uX --unknown -S 'cargo test'", + "sudo env -S'x y'", + "timeout 5 env -S'x y'", // The same writes through the escaping `sub/link` symlink. "env tee sub/link/f", "timeout 5 cp a.txt sub/link/", @@ -176,6 +196,10 @@ fn command_prefix_keeps_inside_and_verification_writes_allowed() { "env tee /dev/null", "env FOO=1 cargo test", "env RUST_LOG=debug cargo test", + "env -i PATH=/usr/bin cargo test", + "env -u HOME cargo test", + "env -uSOMETHING cargo test", + "env -u S cargo test", "timeout 600 cargo test", "timeout --foreground 600 cargo test", "timeout --unknown 600 cargo test", @@ -188,20 +212,12 @@ fn command_prefix_keeps_inside_and_verification_writes_allowed() { "sudo -l tee /tmp/f", // `command -` runs nothing in sh, so `-` is not a program. "command - tee /tmp/f", - // B1: an env -S string with no write program stays allowed. - "env -S'cargo test'", - "env -S", - "env --split-string", - "env --unknown -S", // B4: `sudo -k` without a command, and the terminal timestamp options. "sudo -k", "sudo -K", "sudo -v tee /tmp/f", // A program reached after an option value must still be seen; the // write is inside the workspace, so confinement stays allowed. - "env -S x tee out.txt", - "env --split-string x tee out.txt", - "env --split-string=x tee out.txt", "env -i -- tee out.txt", "env -u V tee out.txt", "env - tee out.txt",