From 7e8f1efa9543af17336a8838e2d907e1f1a3c5f4 Mon Sep 17 00:00:00 2001 From: kewton Date: Fri, 9 Oct 2026 20:44:30 +0900 Subject: [PATCH 1/2] Confine option-named working directories in the Bash guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Read a table program's working-directory option value (tar -C, git -C, make -C, env -C, sudo -D, ninja/go/cargo/pnpm/poetry/yarn/bun/npm/uv/just/ patch, and the tar/gmake aliases) as a working-directory candidate, exactly like a cd destination, through the new leaf module src/tools/bash_write_guard/option_directory.rs. A glob/brace or otherwise unreadable value raises the #635 glob-destination mark so the relative words after it are still refused. Read the word after `pushd --` as a destination even when it starts with `+`/`-`, and name the destination word (option value included) as the undecidable refusal path instead of the following command word. 判断: option values are read from each segment's raw words rather than the resolved program, because env -C / sudo -D never resolve to a Program and a table program can sit behind a prefix (`nice env -C DIR`). 判断: an option value that cannot be read from its spelling (a glob or brace, `$`, a backtick, `~`, a `..` component, or an empty value) raises the #635 mark and adds no candidate, per the issue's design; the mark only ever adds refusals. 本文に無い指摘: `find -execdir` and the options of unlisted programs stay out of scope and keep their existing handling, as the module doc records. Co-authored-by: CommandCodeBot --- src/tools/bash.rs | 9 +- src/tools/bash_write_guard.rs | 1 + .../bash_write_guard/option_directory.rs | 259 ++++++++++++++++++ .../bash_write_guard/working_directory.rs | 79 +++++- .../issue637_bash_option_working_directory.rs | 227 +++++++++++++++ 5 files changed, 564 insertions(+), 11 deletions(-) create mode 100644 src/tools/bash_write_guard/option_directory.rs create mode 100644 tests/issue637_bash_option_working_directory.rs diff --git a/src/tools/bash.rs b/src/tools/bash.rs index 2fcaf573..4b246f18 100644 --- a/src/tools/bash.rs +++ b/src/tools/bash.rs @@ -834,8 +834,15 @@ pub fn path_confinement_rejection( // existing handling. if inspection.has_working_directory() || inspection.glob_destination { if inspection.undecidable && !inspection.relative_words.is_empty() { + // Issue #637: name the destination that raised the mark (the + // `cd`/`pushd` target or option value) rather than the following + // command word, so the message points at the word to fix. + let word = inspection + .glob_destination_word + .as_deref() + .unwrap_or(&inspection.relative_words[0]); return Some(path_reference_rejection( - &inspection.relative_words[0], + word, &root, Some("follows a working directory change that cannot be determined"), )); diff --git a/src/tools/bash_write_guard.rs b/src/tools/bash_write_guard.rs index a338e1c4..ee937b74 100644 --- a/src/tools/bash_write_guard.rs +++ b/src/tools/bash_write_guard.rs @@ -5,6 +5,7 @@ use super::shell_lexical; mod ansi_c_quoting; mod command_prefix; +mod option_directory; mod working_directory; pub(super) use working_directory::Inspection; diff --git a/src/tools/bash_write_guard/option_directory.rs b/src/tools/bash_write_guard/option_directory.rs new file mode 100644 index 00000000..cdfa9234 --- /dev/null +++ b/src/tools/bash_write_guard/option_directory.rs @@ -0,0 +1,259 @@ +//! Programs that name a working directory in an option value (Issue #637). +//! +//! The working-directory walk in [`super::working_directory`] reads the `cd`, +//! `pushd`, and `popd` destinations, but a program can change its own working +//! directory through an option: `tar -C DIR`, `git -C DIR`, `make -C DIR`, +//! `env -C DIR`, `sudo -D DIR`, and others. Those segments resolved to no +//! candidate, so a relative path after them (`tar -C sub -cf - lf2`) was judged +//! against the workspace root alone and an outward symlink inside the option's +//! directory escaped. +//! +//! This leaf module holds only the table and the value extraction; the walk +//! wires each value into the same candidate/target/mark handling a `cd` +//! destination gets. It reads the raw segment words rather than the resolved +//! program, because `env -C` and `sudo -D` never resolve to a program (the +//! prefix peel fails closed on them) and because a table program can sit behind +//! a prefix (`nice env -C DIR`, `sudo nice -C DIR`). +//! +//! # Table +//! +//! Each program is matched by the basename of a word, with its short option +//! character and long option names (without the leading `--`): +//! +//! | program | short | long | +//! |---|---|---| +//! | `tar`, `gtar`, `bsdtar` | `-C` | `--directory` | +//! | `make`, `gmake` | `-C` | `--directory` | +//! | `git` | `-C` | (global options before the subcommand only) | +//! | `env` | `-C` | `--chdir` | +//! | `sudo` | `-D` | `--chdir` | +//! | `ninja`, `go`, `cargo` | `-C` | | +//! | `pnpm` | `-C` | `--dir` | +//! | `poetry` | `-C` | `--directory` | +//! | `yarn`, `bun` | | `--cwd` | +//! | `npm` | | `--prefix` | +//! | `uv` | | `--directory` | +//! | `just` | `-d` | `--working-directory` | +//! | `patch` | `-d` | `--directory` | +//! +//! # Value forms +//! +//! `-C dir`, `-Cdir` (a short-option cluster: the value is the rest of the +//! word when the option is not last, otherwise the next word), `--directory=dir`, +//! and `--directory dir`. Nothing after a `--` word is read. `git -C` is a +//! global option only before the subcommand, so `git commit -C HEAD` names no +//! directory. +//! +//! # Programs not in the table +//! +//! A program that is not listed is read exactly as before: its option values +//! are not treated as working directories. `find -execdir`, whose working +//! directory is a different mechanism, and the options of unlisted programs are +//! out of this issue's scope. + +use std::path::Path; + +/// One program's option spellings that carry a working-directory value. +struct Entry { + program: &'static str, + short: Option, + long: &'static [&'static str], + /// When true, only the options before the first operand (the subcommand) + /// are global (`git -C`); a later `-C` belongs to the subcommand. + stop_at_subcommand: bool, +} + +const fn entry( + program: &'static str, + short: Option, + long: &'static [&'static str], + stop_at_subcommand: bool, +) -> Entry { + Entry { + program, + short, + long, + stop_at_subcommand, + } +} + +/// The table of programs whose option value names a working directory. +const TABLE: &[Entry] = &[ + entry("tar", Some('C'), &["directory"], false), + entry("gtar", Some('C'), &["directory"], false), + entry("bsdtar", Some('C'), &["directory"], false), + entry("make", Some('C'), &["directory"], false), + entry("gmake", Some('C'), &["directory"], false), + entry("git", Some('C'), &[], true), + entry("env", Some('C'), &["chdir"], false), + entry("sudo", Some('D'), &["chdir"], false), + entry("ninja", Some('C'), &[], false), + entry("go", Some('C'), &[], false), + entry("cargo", Some('C'), &[], false), + entry("pnpm", Some('C'), &["dir"], false), + entry("poetry", Some('C'), &["directory"], false), + entry("yarn", None, &["cwd"], false), + entry("bun", None, &["cwd"], false), + entry("npm", None, &["prefix"], false), + entry("uv", None, &["directory"], false), + entry("just", Some('d'), &["working-directory"], false), + entry("patch", Some('d'), &["directory"], false), +]; + +/// The working-directory values the table programs name in one segment's words, +/// in the order they appear. A word whose basename matches a table program is +/// scanned; a table program behind a prefix (`nice env -C DIR`) is found the +/// same way. Values are returned with their spelling as the shell builds the +/// word (quotes removed, escapes resolved). +pub(super) fn directories(words: &[&str]) -> Vec { + let mut values = Vec::new(); + for (index, word) in words.iter().enumerate() { + let name = basename(word); + let Some(entry) = TABLE.iter().find(|entry| entry.program == name) else { + continue; + }; + collect(entry, &words[index + 1..], &mut values); + } + values +} + +/// Reads the values `entry`'s options name from its argument words. +fn collect(entry: &Entry, arguments: &[&str], values: &mut Vec) { + let mut index = 0usize; + while index < arguments.len() { + let word = arguments[index]; + if word == "--" { + break; + } + if let Some(rest) = word.strip_prefix("--") { + let (name, inline) = split_long(rest); + if !entry.long.contains(&name.as_str()) { + index += 1; + continue; + } + match inline { + Some(value) => { + values.push(value); + index += 1; + } + None => { + let Some(value) = arguments.get(index + 1) else { + break; + }; + values.push((*value).to_string()); + index += 2; + } + } + continue; + } + if word.len() > 1 && word.starts_with('-') { + if let Some(option) = entry.short + && let Some(position) = word[1..].find(option) + { + let remainder = &word[1 + position + option.len_utf8()..]; + if !remainder.is_empty() { + values.push(remainder.to_string()); + index += 1; + continue; + } + let Some(value) = arguments.get(index + 1) else { + break; + }; + values.push((*value).to_string()); + index += 2; + continue; + } + index += 1; + continue; + } + // A non-option word. Only `git`'s global options end at the subcommand; + // other table programs keep scanning their operands for an option. + if entry.stop_at_subcommand { + break; + } + index += 1; + } +} + +/// Splits a long option body into its name and an attached `=value`. +fn split_long(rest: &str) -> (String, Option) { + match rest.split_once('=') { + Some((name, value)) => (name.to_string(), Some(value.to_string())), + None => (rest.to_string(), None), + } +} + +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 values(words: &[&str]) -> Vec { + directories(words) + } + + #[test] + fn reads_short_values_in_both_forms() { + assert_eq!(values(&["tar", "-C", "sub"]), vec!["sub"]); + assert_eq!(values(&["tar", "-Csub"]), vec!["sub"]); + // The option is not first in the cluster: the remainder is the value. + assert_eq!(values(&["tar", "-xCsub"]), vec!["sub"]); + } + + #[test] + fn reads_long_values_in_both_forms() { + assert_eq!(values(&["tar", "--directory=sub"]), vec!["sub"]); + assert_eq!(values(&["tar", "--directory", "sub"]), vec!["sub"]); + assert_eq!(values(&["pnpm", "--dir", "sub"]), vec!["sub"]); + assert_eq!(values(&["yarn", "--cwd", "sub"]), vec!["sub"]); + assert_eq!(values(&["npm", "--prefix", "sub"]), vec!["sub"]); + } + + #[test] + fn ignores_unknown_options_and_end_of_options() { + // A cluster with no `C` names no directory. + assert_eq!(values(&["tar", "-cf", "-", "f"]), Vec::::new()); + // Nothing after `--` is read. + assert_eq!(values(&["tar", "--", "-C", "sub"]), Vec::::new()); + assert_eq!(values(&["tar", "-C"]), Vec::::new()); + } + + #[test] + fn git_reads_only_global_options_before_the_subcommand() { + assert_eq!(values(&["git", "-C", "sub", "status"]), vec!["sub"]); + assert_eq!( + values(&["git", "-C", "sub", "-C", "deep", "log"]), + vec!["sub".to_string(), "deep".to_string()] + ); + // `-C` after the subcommand belongs to the subcommand. + assert_eq!( + values(&["git", "commit", "-C", "HEAD"]), + Vec::::new() + ); + } + + #[test] + fn finds_a_table_program_behind_a_prefix() { + assert_eq!(values(&["nice", "env", "-C", "sub", "cat"]), vec!["sub"]); + assert_eq!(values(&["sudo", "make", "-C", "sub"]), vec!["sub"]); + // `sudo -D` is read even though the prefix peel fails closed on it. + assert_eq!(values(&["sudo", "-D", "sub", "cat"]), vec!["sub"]); + } + + #[test] + fn ignores_programs_not_in_the_table() { + assert_eq!(values(&["fltr", "-C", "sub"]), Vec::::new()); + assert_eq!(values(&["python3", "-C", "sub"]), Vec::::new()); + } + + #[test] + fn matches_a_program_by_its_basename() { + assert_eq!(values(&["/usr/bin/tar", "-C", "sub"]), vec!["sub"]); + } +} diff --git a/src/tools/bash_write_guard/working_directory.rs b/src/tools/bash_write_guard/working_directory.rs index e792115f..2db1398c 100644 --- a/src/tools/bash_write_guard/working_directory.rs +++ b/src/tools/bash_write_guard/working_directory.rs @@ -7,8 +7,11 @@ //! escapes. //! //! [`inspect`] walks the lexed tokens and builds the set of working directories -//! the command could run in. It starts at the workspace root and *adds* each -//! determinable `cd`/`pushd` destination instead of replacing, so a subshell, a +//! the command could run in. A table program's option value that names a +//! working directory (`tar -C DIR`, `git -C DIR`, `env -C DIR`, ...) is read by +//! [`super::option_directory`] and added the same way. It starts at the +//! workspace root and *adds* each determinable `cd`/`pushd` destination instead +//! of replacing, so a subshell, a //! pipeline, or a `||` branch that keeps the old directory cannot hide the root //! candidate. A destination whose value cannot be read statically (`$`, //! backquotes, `~`, a glob, `..`, `-`, or an empty `cd`) adds nothing. A command @@ -25,7 +28,7 @@ use std::collections::HashSet; use std::path::Path; use super::command_prefix::{self, Resolution}; -use super::{ShellToken, command_words, positional_operands}; +use super::{ShellToken, command_words, option_directory, positional_operands}; use crate::tools::bash::path_tokens::CrMode; /// Upper bound on the working-directory candidates. Consecutive distinct `cd` @@ -54,6 +57,12 @@ pub(crate) struct Inspection { /// #635). Relative paths after it must be refused even though the workspace /// root is the only candidate. pub(crate) glob_destination: bool, + /// The word that raised [`Self::glob_destination`] (a `cd`/`pushd` + /// destination, or a table program's option value whose value cannot be + /// determined). It is the reason path for the `/`-less refusal below, so the + /// message names the destination rather than the following command word + /// (Issue #637). + pub(crate) glob_destination_word: Option, /// Literal relative read words (from segments that are not a working /// directory change). The second stage of the inspector joins these onto /// every candidate. @@ -69,6 +78,7 @@ impl Inspection { targets: Vec::new(), undecidable: false, glob_destination: false, + glob_destination_word: None, relative_words: Vec::new(), } } @@ -87,6 +97,9 @@ impl Inspection { self.relative_words.extend(other.relative_words); self.undecidable |= other.undecidable; self.glob_destination |= other.glob_destination; + if self.glob_destination_word.is_none() { + self.glob_destination_word = other.glob_destination_word; + } } /// Whether a `cd`/`pushd` added a candidate beyond the workspace root. @@ -148,7 +161,7 @@ fn walk(text: &str, ambiguous: bool, cr_mode: CrMode) -> Inspection { }; let mut has_cd = false; - let mut glob_destination = false; + let mut glob_destination: Option = None; let mut cdpath = false; let mut dirstack = false; let mut loop_keyword = false; @@ -177,6 +190,17 @@ fn walk(text: &str, ambiguous: bool, cr_mode: CrMode) -> Inspection { if words.is_empty() { continue; } + // Issue #637: a table program's working-directory option value + // (`tar -C DIR`, `git -C DIR`, `env -C DIR`, `sudo -D DIR`, ...) is a + // candidate exactly like a `cd` destination. The raw segment words are + // read because `env -C`/`sudo -D` never resolve to a `Program` and a + // table program can sit behind a prefix. + collect_option_directories( + &words, + &mut inspection, + &mut glob_destination, + &mut overflow, + ); let resolution = command_prefix::resolve(&words); // A working-directory change is consumed by the candidate walk below; // its program and operands are not reads, so they do not feed the @@ -205,7 +229,7 @@ fn walk(text: &str, ambiguous: bool, cr_mode: CrMode) -> Inspection { has_cd = true; if let Some(target) = positional_operands(&arguments).first().copied() { if is_glob_destination(target) { - glob_destination = true; + mark_glob_destination(&mut glob_destination, target); } if !overflow && let Some(joined) = extend(&mut inspection.bases, target) { inspection.targets.extend(joined); @@ -220,12 +244,16 @@ fn walk(text: &str, ambiguous: bool, cr_mode: CrMode) -> Inspection { }; // `pushd` with no operand and `pushd ±N` only rotate the // directory stack, which holds nothing beyond the candidates, - // so they add no candidate. - if target.starts_with('+') || target.starts_with('-') { + // so they add no candidate. A `pushd` that names `--` reads the + // word after it as a destination, so a leading `+`/`-` is a + // directory name, not a stack position (Issue #637). + let stack_rotation = !arguments.contains(&"--") + && (target.starts_with('+') || target.starts_with('-')); + if stack_rotation { continue; } if is_glob_destination(target) { - glob_destination = true; + mark_glob_destination(&mut glob_destination, target); } if !overflow && let Some(joined) = extend(&mut inspection.bases, target) { inspection.targets.extend(joined); @@ -239,16 +267,47 @@ fn walk(text: &str, ambiguous: bool, cr_mode: CrMode) -> Inspection { } } - inspection.glob_destination = glob_destination; + inspection.glob_destination = glob_destination.is_some(); + inspection.glob_destination_word = glob_destination; inspection.undecidable = ambiguous || cdpath || dirstack || overflow - || glob_destination + || inspection.glob_destination || (has_cd && (loop_keyword || function_keyword)); inspection } +/// Records the first word that raises the glob-destination mark, so the +/// `/`-less refusal can name the destination instead of the next command word. +fn mark_glob_destination(slot: &mut Option, word: &str) { + if slot.is_none() { + *slot = Some(word.to_string()); + } +} + +/// Adds the candidates a table program's option value produces (Issue #637), +/// exactly like a `cd` destination: a value that cannot be read from its +/// spelling (a glob or brace, `$`, a backtick, `~`, a `..` component, an empty +/// value) adds no candidate and raises the glob-destination mark, so the +/// relative words after it are still refused. +fn collect_option_directories( + words: &[&str], + inspection: &mut Inspection, + glob_destination: &mut Option, + overflow: &mut bool, +) { + for value in option_directory::directories(words) { + if !is_determinable(&value) { + mark_glob_destination(glob_destination, &value); + } + if !*overflow && let Some(joined) = extend(&mut inspection.bases, &value) { + inspection.targets.extend(joined); + *overflow = inspection.bases.len() > MAX_CANDIDATES; + } + } +} + fn word_token(token: &ShellToken) -> Option<&str> { match token { ShellToken::Word(word) => Some(word.as_str()), diff --git a/tests/issue637_bash_option_working_directory.rs b/tests/issue637_bash_option_working_directory.rs new file mode 100644 index 00000000..dfe85a74 --- /dev/null +++ b/tests/issue637_bash_option_working_directory.rs @@ -0,0 +1,227 @@ +#![cfg(unix)] + +//! Issue #637: a program's option can change the working directory +//! (`tar -C DIR`, `git -C DIR`, `make -C DIR`, `env -C DIR`, `sudo -D DIR`, ...) +//! and a `pushd` after `--` can name a `-`-prefixed directory. Neither was a +//! working-directory candidate, so a following relative `/`-less path was judged +//! against the workspace root alone and an outward symlink inside the option's +//! directory escaped. This file fixes the second-stage decision (`R`), the +//! write-side decision (`W`) for the `pushd --` shape, and the refusal reason +//! path, which must name the destination word (including an option value) rather +//! than the following command word. +//! +//! False-rejection estimate (from the issue, recorded here as evidence): of the +//! 436 distinct agent Bash commands in `dev-reports` and `workspace/management`, +//! zero use a table option or `pushd --`, so no recorded command changes. Of the +//! ~59k distinct commands in this repository, the added candidates are `git -C` +//! 1,745, `tar -C`/`--directory` 108, `env -C` 28, `sudo -D` 11, npm-family 54, +//! `cargo -C` 8, `make -C` 1, and `pushd --` 0; a candidate is added only, so a +//! command is refused only when the added destination holds an outward symlink +//! or when the value holds a glob/`$`. The one newly refused command kept inside +//! the workspace was `git -C sub status; cat lf2` (the candidate stays for the +//! later segment), and the inside commands listed in +//! `inside_workspace_option_directories_are_allowed` are unchanged. + +use std::path::{Path, PathBuf}; + +use commandagent::tools::bash::path_confinement_rejection; + +/// The fixed operation a read-side refusal carries. +const READ_OPERATION: &str = "path reference"; +/// The existing write-side working-directory reason. +const CWD_REASON: &str = "follows a working directory change that cannot be determined"; + +struct Fixture { + _dir: tempfile::TempDir, + root: PathBuf, +} + +/// A tempdir outside the prefix `is_system_prefix_allowed` accepts (`/usr`, +/// `/bin`, `/opt`, `/etc`, `/tmp`), so an escaping symlink target is never +/// admitted as a system path. `sub` is an ordinary directory holding the +/// outward symlink `lf2` and the sub-directory `deep/lf3` (also outward); +/// `s2` holds `lf4`, `s[2]` holds `lf5`, and the `-`-named directory `-s2` holds +/// `lf7`. The workspace root holds the regular files `f` and `a.tar`. +fn fixture() -> Fixture { + let dir = tempfile::tempdir_in(env!("CARGO_TARGET_TMPDIR")).expect("create tempdir"); + let root = dir.path().join("ws"); + for directory in ["sub", "sub/deep", "s2", "s[2]", "-s2"] { + std::fs::create_dir_all(root.join(directory)).unwrap(); + } + std::fs::write(root.join("f"), "x").unwrap(); + std::fs::write(root.join("a.tar"), "x").unwrap(); + std::fs::write(root.join("sub/f"), "x").unwrap(); + let outside = dir.path().join("outside"); + std::fs::create_dir_all(&outside).unwrap(); + std::fs::write(outside.join("secret"), "outside-secret").unwrap(); + for link in ["sub/lf2", "sub/deep/lf3", "s2/lf4", "s[2]/lf5", "-s2/lf7"] { + std::os::unix::fs::symlink(&outside, root.join(link)).unwrap(); + } + let root = root.canonicalize().unwrap(); + Fixture { _dir: dir, root } +} + +/// Asserts every command is refused by a read-side `path reference` that names +/// the working-directory reason where the destination cannot be read. +fn assert_option_read_rejected(root: &Path, commands: &[String]) { + for command in commands { + let rejection = path_confinement_rejection(command, root) + .unwrap_or_else(|| panic!("expected rejection: {command}")); + assert_eq!( + rejection.operation, READ_OPERATION, + "command: {command} ({})", + rejection.reason + ); + } +} + +/// The issue table: every shape that changes the working directory through an +/// option must make the following relative read escape, so `R=1`. The value +/// forms (`-C dir`, `-Cdir`, `--directory=dir`, `--directory dir`), the table +/// programs, the glob/brace values, and the `cd`-then-option shapes are all +/// covered. +#[test] +fn option_directory_rejects_relative_reads() { + let fixture = fixture(); + let root = &fixture.root; + let cases: Vec = [ + // tar value forms. + "tar -C sub -cf - lf2", + "tar -cf - -C sub lf2", + "tar -Csub -cf - lf2", + "tar --directory=sub -cf - lf2", + "tar --directory sub -cf - lf2", + "tar -xf a.tar -C sub lf2", + // git / make. + "git -C sub log lf2", + "git -C sub -C deep log lf3", + "make -C sub lf2", + "make --directory=sub lf2", + // prefixes. + "env -C sub cat lf2", + "env --chdir=sub cat lf2", + "nice env -C sub cat lf2", + "sudo -D sub cat lf2", + "sudo --chdir=sub cat lf2", + // the wider table. + "ninja -C sub lf2", + "pnpm -C sub exec cat lf2", + "yarn --cwd sub cat lf2", + "npm --prefix sub exec cat lf2", + "go -C sub run lf2", + "uv --directory sub run cat lf2", + "poetry -C sub run cat lf2", + "just -d sub cat lf2", + "cargo -C sub run lf2", + "patch -d sub lf2 < x.patch", + // glob and brace values add no candidate but still refuse. + "tar -C s2* -cf - lf4", + "git -C s? log lf4", + "make -C {s2,x} lf4", + // a `cd` candidate combined with an option candidate. + "cd sub && tar -C deep -cf - lf3", + "tar -C sub -cf - deep/lf3", + ] + .into_iter() + .map(str::to_string) + .collect(); + assert_option_read_rejected(root, &cases); +} + +/// The issue problem 2, reads: `pushd --` reads a `-`-prefixed word as a +/// destination, so the relative read after it escapes the `-s2` directory. +#[test] +fn pushd_double_dash_rejects_relative_reads() { + let fixture = fixture(); + let root = &fixture.root; + let cases = [ + "pushd -- -s2 && cat lf7".to_string(), + "pushd -- -s* && cat lf7".to_string(), + "pushd -n -- -s2 && cat lf7".to_string(), + ]; + assert_option_read_rejected(root, &cases); +} + +/// The issue problem 2, writes: `pushd -- -s2` then a relative write is refused +/// by the write side (`W=1`) with the write operation, before the read stage. +#[test] +fn pushd_double_dash_rejects_relative_writes() { + let fixture = fixture(); + let root = &fixture.root; + let rejection = path_confinement_rejection("pushd -- -s2 && echo x > lf7", root) + .expect("expected a write rejection"); + assert_eq!(rejection.operation, "output redirection"); + + // Without `--`, a `-`-prefixed operand is still a stack rotation, not a + // directory, so `lf7` is judged at the root alone and stays allowed. + for command in ["pushd -n +1 && cat lf7", "pushd -s2 && cat lf7"] { + assert!( + path_confinement_rejection(command, root).is_none(), + "expected allow: {command}" + ); + } +} + +/// The issue problem 3: the refusal reason names the destination word (the +/// option value included), not the command word that follows it. +#[test] +fn refusal_reason_names_the_destination_word() { + let fixture = fixture(); + let root = &fixture.root; + for (command, word) in [ + ("cd 's2*' && cat lf4", "s2*"), + ("cd \"s[2]\" && ls", "s[2]"), + ("tar -C 's2*' -cf - lf4", "s2*"), + ("make -C {s2,x} lf4", "{s2,x}"), + ("git -C s? log lf4", "s?"), + ] { + let rejection = path_confinement_rejection(command, root) + .unwrap_or_else(|| panic!("expected rejection: {command}")); + assert_eq!(rejection.path, word, "command: {command}"); + assert_eq!(rejection.operation, READ_OPERATION, "command: {command}"); + assert!( + rejection.reason.contains(CWD_REASON), + "command: {command}: {}", + rejection.reason + ); + } +} + +/// The issue acceptance: workspace-internal reads and writes through a table +/// option are not falsely refused. `git commit -C HEAD` must stay allowed because +/// `git -C` is a global option only before the subcommand. +#[test] +fn inside_workspace_option_directories_are_allowed() { + let fixture = fixture(); + let root = &fixture.root; + for command in [ + "git -C sub status", + "make -C sub", + "tar -C sub -cf - f", + "tar -C sub -xf a.tar", + "env -C sub cat f", + "git commit -C HEAD", + "git -C sub -C deep status", + "npm --prefix sub test", + "git -C . status", + ] { + assert!( + path_confinement_rejection(command, root).is_none(), + "expected allow: {command}" + ); + } +} + +/// A program that is not in the table keeps its existing handling: its option +/// value is not a working-directory candidate, so a `/`-less path reachable only +/// through it is not newly refused (the `find -execdir`-style gap stays out of +/// this issue's scope). +#[test] +fn programs_not_in_the_table_are_unchanged() { + let fixture = fixture(); + let root = &fixture.root; + assert!( + path_confinement_rejection("fltr -C sub lf2", root).is_none(), + "an unlisted program must keep its existing handling" + ); +} From 72e99ad3dc0fee6a00eee94748106e61b4e8a745 Mon Sep 17 00:00:00 2001 From: kewton Date: Fri, 9 Oct 2026 23:00:35 +0900 Subject: [PATCH 2/2] Test the option-value candidate cap skips the bound MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add working_directory_caps_an_option_growth_that_skips_the_bound, the option-value analogue of working_directory_caps_a_growth_that_skips_the_bound (#635): forty `git -C sub` values add one candidate each, then a single `git -C r` doubles the set past MAX_CANDIDATES without landing on it, so the inspection must still be undecidable. This catches the `>` → `==` mutation in collect_option_directories' cap check, which would leave the mark clear and allow more. Test-only; no production decision changes. Co-authored-by: CommandCodeBot --- .../bash_write_guard/working_directory.rs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/tools/bash_write_guard/working_directory.rs b/src/tools/bash_write_guard/working_directory.rs index 2db1398c..a26187aa 100644 --- a/src/tools/bash_write_guard/working_directory.rs +++ b/src/tools/bash_write_guard/working_directory.rs @@ -508,6 +508,26 @@ mod tests { ); } + /// Issue #637: the option-value walk shares the cap check with `cd`. Forty + /// `git -C sub` values add exactly one candidate each (lengths 2..=41), then + /// a single `git -C r` doubles the set to 82, which skips 64; the value past + /// the cap must still be undecidable. A `>` read as `==` never sees 64 and + /// leaves the mark clear. + #[test] + fn working_directory_caps_an_option_growth_that_skips_the_bound() { + let command = format!("{}git -C r status", "git -C sub status; ".repeat(40)); + let inspection = inspect(&command); + assert!( + inspection.bases.len() > MAX_CANDIDATES, + "the fixture must pass the bound: {}", + inspection.bases.len() + ); + assert!( + inspection.undecidable, + "an option candidate set that passed the bound without landing on it must be undecidable" + ); + } + #[test] fn working_directory_keeps_cwd_words_out_of_reads() { let inspection = inspect("cd sub && cat secret");