From 4f2d807379115bf874b0572bb665f6316938d5fe Mon Sep 17 00:00:00 2001 From: kewton Date: Sat, 10 Oct 2026 16:13:16 +0900 Subject: [PATCH 1/4] Read more working-directory option variants in the Bash write guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A table program's long option now matches a non-empty prefix (tar --dir), a `-`-prefixed option value is recorded and scanned again so a hidden `-C` is found (tar -cfC -C sub), git's value-taking global options skip their value word so a later `-C` is scanned (git -c k=v -C sub), and `npm -C` joins the table (Issue #645). The changed shapes become R=1; every other value is unchanged and the change only adds candidates. 判断: git の値をとる全体のオプションは本文の一覧どおり完全一致で扱い、接頭辞一致は加えていない（本文が名前を列挙しているため）。 判断: 値をとる全体オプションは Entry に欄を足して表し、git 以外は空にした（表を作業ディレクトリの解析の中だけに閉じるため）。 Co-authored-by: CommandCodeBot --- .../bash_write_guard/option_directory.rs | 157 +++++++++++-- ...issue645_bash_option_directory_variants.rs | 222 ++++++++++++++++++ 2 files changed, 358 insertions(+), 21 deletions(-) create mode 100644 tests/issue645_bash_option_directory_variants.rs diff --git a/src/tools/bash_write_guard/option_directory.rs b/src/tools/bash_write_guard/option_directory.rs index cdfa9234..b2b205e7 100644 --- a/src/tools/bash_write_guard/option_directory.rs +++ b/src/tools/bash_write_guard/option_directory.rs @@ -31,18 +31,28 @@ //! | `pnpm` | `-C` | `--dir` | //! | `poetry` | `-C` | `--directory` | //! | `yarn`, `bun` | | `--cwd` | -//! | `npm` | | `--prefix` | +//! | `npm` | `-C` | `--prefix` | //! | `uv` | | `--directory` | //! | `just` | `-d` | `--working-directory` | //! | `patch` | `-d` | `--directory` | //! +//! A long option that is a non-empty prefix of a table name is read as that +//! name — GNU and bsdtar accept an unambiguous abbreviation (`--dir` for +//! `--directory`) — in both the `--name=value` and `--name value` forms. An +//! unknown long option names no directory, as before. +//! //! # 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. +//! `--directory dir`, and a prefix of `--directory`. A value read from the next +//! word that itself begins with `-` (other than `--`) is recorded and then +//! scanned again, so the second `-C` in `tar -cfC -C sub` names `sub`. Nothing +//! after a `--` word is read. `git -C` is a global option only before the +//! subcommand, so `git commit -C HEAD` names no directory; `git`'s other global +//! options that take a value word (`-c`, `--git-dir`, `--work-tree`, +//! `--namespace`, `--super-prefix`, `--config-env`, `--attr-source`) skip that +//! value so the walk keeps scanning for a later `-C`. //! //! # Programs not in the table //! @@ -58,6 +68,12 @@ struct Entry { program: &'static str, short: Option, long: &'static [&'static str], + /// Global option characters that take their value from the next word and + /// name no directory (`git -c`). Empty for every program but `git`. + value_short: &'static [char], + /// Global option names whose value is a separate word and names no + /// directory (`git --git-dir DIR`). Empty for every program but `git`. + value_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, @@ -73,6 +89,8 @@ const fn entry( program, short, long, + value_short: &[], + value_long: &[], stop_at_subcommand, } } @@ -84,7 +102,21 @@ const TABLE: &[Entry] = &[ entry("bsdtar", Some('C'), &["directory"], false), entry("make", Some('C'), &["directory"], false), entry("gmake", Some('C'), &["directory"], false), - entry("git", Some('C'), &[], true), + Entry { + program: "git", + short: Some('C'), + long: &[], + value_short: &['c'], + value_long: &[ + "git-dir", + "work-tree", + "namespace", + "super-prefix", + "config-env", + "attr-source", + ], + stop_at_subcommand: true, + }, entry("env", Some('C'), &["chdir"], false), entry("sudo", Some('D'), &["chdir"], false), entry("ninja", Some('C'), &[], false), @@ -94,7 +126,7 @@ const TABLE: &[Entry] = &[ entry("poetry", Some('C'), &["directory"], false), entry("yarn", None, &["cwd"], false), entry("bun", None, &["cwd"], false), - entry("npm", None, &["prefix"], false), + entry("npm", Some('C'), &["prefix"], false), entry("uv", None, &["directory"], false), entry("just", Some('d'), &["working-directory"], false), entry("patch", Some('d'), &["directory"], false), @@ -127,23 +159,29 @@ fn collect(entry: &Entry, arguments: &[&str], values: &mut Vec) { } if let Some(rest) = word.strip_prefix("--") { let (name, inline) = split_long(rest); - if !entry.long.contains(&name.as_str()) { - index += 1; + if matches_long(entry.long, &name) { + 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 += value_step(value); + } + } 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; - } + // A value-taking global option (`git --git-dir DIR`) consumes its + // value word and scanning continues, so a following `-C` is read. + if inline.is_none() && entry.value_long.contains(&name.as_str()) { + index += 2; + continue; } + index += 1; continue; } if word.len() > 1 && word.starts_with('-') { @@ -160,7 +198,20 @@ fn collect(entry: &Entry, arguments: &[&str], values: &mut Vec) { break; }; values.push((*value).to_string()); - index += 2; + index += value_step(value); + continue; + } + // A value-taking global short option (`git -c KEY=VALUE`) consumes + // its value word and scanning continues. + if let Some(option) = entry + .value_short + .iter() + .copied() + .find(|option| word[1..].contains(*option)) + { + let position = word[1..].find(option).expect("found above"); + let remainder = &word[1 + position + option.len_utf8()..]; + index += if remainder.is_empty() { 2 } else { 1 }; continue; } index += 1; @@ -175,6 +226,23 @@ fn collect(entry: &Entry, arguments: &[&str], values: &mut Vec) { } } +/// Whether `name` is a non-empty prefix of one of `long`, so a GNU or bsdtar +/// abbreviation (`--dir` for `--directory`) is read as that option. +fn matches_long(long: &[&str], name: &str) -> bool { + !name.is_empty() && long.iter().any(|full| full.starts_with(name)) +} + +/// How far to advance after reading a value from the next word. A value that +/// begins with `-` (other than `--`) is itself scanned, so the walk advances one +/// word; anything else is consumed whole. +fn value_step(value: &str) -> usize { + if value.starts_with('-') && value != "--" { + 1 + } else { + 2 + } +} + /// Splits a long option body into its name and an attached `=value`. fn split_long(rest: &str) -> (String, Option) { match rest.split_once('=') { @@ -215,6 +283,53 @@ mod tests { assert_eq!(values(&["npm", "--prefix", "sub"]), vec!["sub"]); } + #[test] + fn reads_abbreviated_long_options() { + assert_eq!(values(&["tar", "--dir=sub"]), vec!["sub"]); + assert_eq!(values(&["tar", "--direc", "sub"]), vec!["sub"]); + assert_eq!(values(&["env", "--ch=sub"]), vec!["sub"]); + assert_eq!(values(&["just", "--working-dir", "sub"]), vec!["sub"]); + assert_eq!(values(&["npm", "--pref", "sub"]), vec!["sub"]); + // A prefix of nothing stays unknown. + assert_eq!( + values(&["tar", "--exclude=x", "-cf", "-", "f"]), + Vec::::new() + ); + } + + #[test] + fn rescans_a_dash_prefixed_value() { + assert_eq!(values(&["tar", "-cfC", "-C", "sub"]), vec!["-C", "sub"]); + assert_eq!( + values(&["tar", "-cfC", "--directory=sub"]), + vec!["--directory=sub", "sub"] + ); + assert_eq!(values(&["tar", "-fC", "-Csub"]), vec!["-Csub", "sub"]); + } + + #[test] + fn reads_npm_short_directory_option() { + assert_eq!(values(&["npm", "-C", "sub"]), vec!["sub"]); + assert_eq!(values(&["npm", "-Csub"]), vec!["sub"]); + } + + #[test] + fn git_skips_value_taking_global_options() { + assert_eq!(values(&["git", "-c", "k=v", "-C", "sub"]), vec!["sub"]); + assert_eq!(values(&["git", "--git-dir", "x", "-C", "sub"]), vec!["sub"]); + assert_eq!( + values(&["git", "--work-tree", "sub", "-C", "deep"]), + vec!["deep"] + ); + // The `=` form is one word and keeps its handling (the `-C` is still read). + assert_eq!(values(&["git", "--git-dir=x", "-C", "sub"]), vec!["sub"]); + // An unknown global option still ends the scan. + assert_eq!( + values(&["git", "--no-pager", "log", "-C", "sub"]), + Vec::::new() + ); + } + #[test] fn ignores_unknown_options_and_end_of_options() { // A cluster with no `C` names no directory. diff --git a/tests/issue645_bash_option_directory_variants.rs b/tests/issue645_bash_option_directory_variants.rs new file mode 100644 index 00000000..4791db1e --- /dev/null +++ b/tests/issue645_bash_option_directory_variants.rs @@ -0,0 +1,222 @@ +#![cfg(unix)] + +//! Issue #645: the working-directory option check (#637) missed four spellings, +//! so a relative `/`-less read after them was judged against the workspace root +//! alone and an outward symlink inside the option's directory escaped (`R=0`). +//! This file drives the public second-stage predicate `R` +//! (`path_confinement_rejection`) against a tempdir fixture: +//! +//! 1. a long-option abbreviation (`tar --dir=sub`, `env --ch sub`), which GNU +//! and bsdtar accept but the exact-name match missed; +//! 2. a cluster whose working-directory character is eaten as another option's +//! value (`tar -cfC -C sub`), which the cluster scan skipped; +//! 3. a `git` value-taking global option before `-C` (`git -c k=v -C sub`), +//! whose value the scan read as the subcommand and stopped; +//! 4. `npm -C`, the short spelling of `--prefix`, which the table lacked. +//! +//! ## False-rejection estimate (from the Issue body) +//! +//! Of the 436 distinct agent Bash commands recorded under `dev-reports/` and +//! `workspace/management/`, the H-17 prototype changed none. Of the ~59k +//! distinct Claude Code Bash commands in this repository it changed none: the +//! long-name prefix form appeared 0 times, a `git` command starting with a +//! value-taking global option 151 times, and `npm -C` 0 times. The change only +//! *adds* working-directory candidates (it never drops one), so a command is +//! newly refused only when the added destination holds an outward symlink or +//! when the value cannot be read from its spelling (a glob, `$`, `..`, empty). +//! +//! That monotonicity is also why no acceptance mark can loosen: this walk only +//! adds candidates, so `R`/`A`/`W`/`P`/`S` can only go `0 -> 1` and `V` can only +//! stay or tighten. The named issues' own test files plus +//! `cargo test --all-targets` pin that direction; this file pins the four new +//! shapes and the unchanged values of `meson -C`, `unzip -d`, and +//! `git --git-dir=x`. + +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"; + +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` holds the outward symlink `lf2` and the +/// regular file `sub/f`; 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"); + assert!( + !["/usr", "/bin", "/opt", "/etc", "/tmp"] + .iter() + .any(|prefix| dir.path().starts_with(prefix)), + "the fixture tempdir must not sit under a system prefix: {}", + dir.path().display() + ); + let root = dir.path().join("ws"); + std::fs::create_dir_all(root.join("sub")).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(); + std::os::unix::fs::symlink(&outside, root.join("sub/lf2")).unwrap(); + let root = root.canonicalize().unwrap(); + Fixture { _dir: dir, root } +} + +/// Asserts every command is refused by a read-side `path reference`, so the +/// relative read after the option escapes the workspace (`R=1`). +fn assert_read_rejected(root: &Path, commands: &[&str]) { + 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 + ); + } +} + +/// Table 1 rows 1 and 4: a long option that is a prefix of a table name +/// (`tar --dir`, `just --working-dir`, `npm --pref`) and `npm -C` now name +/// `sub`, so `lf2` under it escapes. +#[test] +fn abbreviated_and_short_npm_options_reject_relative_reads() { + let fixture = fixture(); + let root = &fixture.root; + assert_read_rejected( + root, + &[ + "tar --dir=sub -cf - lf2", + "tar --direc sub -cf - lf2", + "tar --dir sub -cf - lf2", + "make --dir=sub lf2", + "env --ch=sub cat lf2", + "env --chd sub cat lf2", + "sudo --chd=sub cat lf2", + "patch --dir=sub -i lf2", + "just --working-dir sub cat lf2", + "pnpm --di sub cat lf2", + "npm --pref sub x lf2", + "npm -C sub run x lf2", + "npm -C sub exec cat lf2", + ], + ); +} + +/// Table 1 row 2: a cluster whose working-directory character is read as another +/// option's value (`tar -cfC -C sub`). The `-`-prefixed value is recorded and +/// scanned again, so the real `-C sub` is found. +#[test] +fn clustered_values_that_hide_the_option_reject_relative_reads() { + let fixture = fixture(); + let root = &fixture.root; + assert_read_rejected( + root, + &[ + "tar -cfC -C sub lf2", + "tar -cfC --directory=sub lf2", + "tar -cfC -Csub lf2", + "tar -fC -Csub lf2", + "make -fC -C sub lf2", + "sudo -uD -D sub cat lf2", + ], + ); +} + +/// Table 1 row 3: a `git` value-taking global option before `-C` consumes its +/// value word, so the scan no longer mistakes it for the subcommand and stops. +#[test] +fn git_value_taking_global_options_reject_relative_reads() { + let fixture = fixture(); + let root = &fixture.root; + assert_read_rejected( + root, + &[ + "git -c k=v -C sub log lf2", + "git --git-dir x -C sub log lf2", + "git --work-tree sub -C sub log lf2", + "git --namespace n -C sub log lf2", + ], + ); +} + +/// Table 2: the workspace-internal reads and writes through these options are +/// not falsely refused; their value is unchanged. `git commit -C HEAD` stays +/// allowed because `git -C` is global only before the subcommand. +#[test] +fn inside_workspace_option_variants_stay_allowed() { + let fixture = fixture(); + let root = &fixture.root; + for command in [ + "git -C sub status", + "make -C sub", + "tar -C sub -cf - f", + "grep -C 3 x f", + "git log -C sub", + "git -c color.ui=never status", + "git -c k=v -C sub status", + "git --no-pager log -C", + "git commit -C HEAD", + "tar --exclude=x -cf - f", + "make --no-print-directory -C sub", + "make -f Makefile test", + "cargo test --no-fail-fast", + "cargo --manifest-path x/Cargo.toml test", + "npm --prefix sub run build", + "sudo -u root -D sub cat f", + "env --ignore-environment cat f", + "tar -xzf a.tar -C sub", + ] { + assert!( + path_confinement_rejection(command, root).is_none(), + "expected allow: {command}" + ); + } +} + +/// Table 3, unchanged-and-allowed: a program the design deliberately does not +/// add (`meson`, `gradle`, `tmux`, `docker`, `unzip`) names no candidate, and a +/// `git` object name after `-C sub` is not a file path, so `R` stays `0`. +#[test] +fn unlisted_programs_and_git_objects_keep_their_value() { + let fixture = fixture(); + let root = &fixture.root; + for command in [ + "git -c core.pager=cat -C sub show HEAD:lf2", + "meson -C sub lf2", + "gradle -p sub lf2", + "tmux -c sub cat lf2", + "docker -w sub cat lf2", + "unzip -d sub a.zip lf2", + ] { + assert!( + path_confinement_rejection(command, root).is_none(), + "expected allow: {command}" + ); + } +} + +/// Table 3, unchanged-and-refused: the `=`-attached global option and the +/// value-less `--no-pager`/`-P` keep the `-C` after them read, so `R` stays `1`. +#[test] +fn already_refused_git_shapes_stay_refused() { + let fixture = fixture(); + let root = &fixture.root; + assert_read_rejected( + root, + &[ + "git --no-pager -C sub log lf2", + "git -P -C sub log lf2", + "git --git-dir=x -C sub log lf2", + ], + ); +} From 2a98c9fe66d025655a16a55816e7422c5ab99737 Mon Sep 17 00:00:00 2001 From: kewton Date: Sat, 10 Oct 2026 18:12:54 +0900 Subject: [PATCH 2/4] Cover git -c not first in a cluster in the option-directory tests Add a `git -pc k=v -C sub log lf2` read case (Issue #645) to the variant integration test and the leaf unit test, so a cluster where the value-taking `-c` is not at position 0 pins that its value word is skipped and the later `-C` is still read. Product code is unchanged. Co-authored-by: CommandCodeBot --- src/tools/bash_write_guard/option_directory.rs | 2 ++ tests/issue645_bash_option_directory_variants.rs | 1 + 2 files changed, 3 insertions(+) diff --git a/src/tools/bash_write_guard/option_directory.rs b/src/tools/bash_write_guard/option_directory.rs index b2b205e7..5fef5beb 100644 --- a/src/tools/bash_write_guard/option_directory.rs +++ b/src/tools/bash_write_guard/option_directory.rs @@ -316,6 +316,8 @@ mod tests { #[test] fn git_skips_value_taking_global_options() { assert_eq!(values(&["git", "-c", "k=v", "-C", "sub"]), vec!["sub"]); + // `-c` need not be first in the cluster. + assert_eq!(values(&["git", "-pc", "k=v", "-C", "sub"]), vec!["sub"]); assert_eq!(values(&["git", "--git-dir", "x", "-C", "sub"]), vec!["sub"]); assert_eq!( values(&["git", "--work-tree", "sub", "-C", "deep"]), diff --git a/tests/issue645_bash_option_directory_variants.rs b/tests/issue645_bash_option_directory_variants.rs index 4791db1e..81f6e10a 100644 --- a/tests/issue645_bash_option_directory_variants.rs +++ b/tests/issue645_bash_option_directory_variants.rs @@ -142,6 +142,7 @@ fn git_value_taking_global_options_reject_relative_reads() { root, &[ "git -c k=v -C sub log lf2", + "git -pc k=v -C sub log lf2", "git --git-dir x -C sub log lf2", "git --work-tree sub -C sub log lf2", "git --namespace n -C sub log lf2", From 17443e4ae46a2928704b9afc0b03e474dd2068b0 Mon Sep 17 00:00:00 2001 From: kewton Date: Sun, 11 Oct 2026 02:26:44 +0900 Subject: [PATCH 3/4] Stop git -C from rescanning a value that looks like a global option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit git -C always takes exactly one literal path word. Rescanning its value made a value spelled like a value-taking global option (`--git-dir`, `-pc`, `-c`) swallow the following `-C sub`, so `sub/lf2` escaped (R=1 -> R=0). Now git's value is never rescanned (value_step returns 2 for stop_at_subcommand), while other table programs keep rescanning `-`-prefixed values (Issue #645). Tests: four new read rejections in git_c_value_named_like_a_global_option_still_reads_the_next_c, plus leaf unit assertions. The pre-fix code was observed to fail these. 判断: リーダーの差し戻しどおり、git の -C の値は値オプションとして読まず常に 2 語進め、git 以外の走査は変えない。 Co-authored-by: CommandCodeBot --- .../bash_write_guard/option_directory.rs | 37 +++++++++---- ...issue645_bash_option_directory_variants.rs | 52 +++++++++++++++++++ 2 files changed, 78 insertions(+), 11 deletions(-) diff --git a/src/tools/bash_write_guard/option_directory.rs b/src/tools/bash_write_guard/option_directory.rs index 5fef5beb..90d7b759 100644 --- a/src/tools/bash_write_guard/option_directory.rs +++ b/src/tools/bash_write_guard/option_directory.rs @@ -49,8 +49,10 @@ //! word that itself begins with `-` (other than `--`) is recorded and then //! scanned again, so the second `-C` in `tar -cfC -C sub` names `sub`. Nothing //! after a `--` word is read. `git -C` is a global option only before the -//! subcommand, so `git commit -C HEAD` names no directory; `git`'s other global -//! options that take a value word (`-c`, `--git-dir`, `--work-tree`, +//! subcommand, so `git commit -C HEAD` names no directory; because it always +//! takes exactly one literal path word, its value is never rescanned, so +//! `git -C --git-dir -C sub` reads `--git-dir` and then `sub`. `git`'s other +//! global options that take a value word (`-c`, `--git-dir`, `--work-tree`, //! `--namespace`, `--super-prefix`, `--config-env`, `--attr-source`) skip that //! value so the walk keeps scanning for a later `-C`. //! @@ -170,7 +172,7 @@ fn collect(entry: &Entry, arguments: &[&str], values: &mut Vec) { break; }; values.push((*value).to_string()); - index += value_step(value); + index += value_step(entry, value); } } continue; @@ -198,7 +200,7 @@ fn collect(entry: &Entry, arguments: &[&str], values: &mut Vec) { break; }; values.push((*value).to_string()); - index += value_step(value); + index += value_step(entry, value); continue; } // A value-taking global short option (`git -c KEY=VALUE`) consumes @@ -232,14 +234,17 @@ fn matches_long(long: &[&str], name: &str) -> bool { !name.is_empty() && long.iter().any(|full| full.starts_with(name)) } -/// How far to advance after reading a value from the next word. A value that -/// begins with `-` (other than `--`) is itself scanned, so the walk advances one -/// word; anything else is consumed whole. -fn value_step(value: &str) -> usize { - if value.starts_with('-') && value != "--" { - 1 - } else { +/// How far to advance after reading the working-directory value from the next +/// word. `git -C` always takes exactly the next word as its path, so its value +/// is never rescanned: a value spelled like a global option (`--git-dir`, +/// `-pc`, `-c`) must not swallow the `-C` after it. Every other table program +/// rescans a `-`-prefixed value (other than `--`) so a hidden option inside it +/// is still found. +fn value_step(entry: &Entry, value: &str) -> usize { + if entry.stop_at_subcommand || !(value.starts_with('-') && value != "--") { 2 + } else { + 1 } } @@ -319,6 +324,16 @@ mod tests { // `-c` need not be first in the cluster. assert_eq!(values(&["git", "-pc", "k=v", "-C", "sub"]), vec!["sub"]); assert_eq!(values(&["git", "--git-dir", "x", "-C", "sub"]), vec!["sub"]); + // A `-C` value spelled like a global option is one literal path word and + // is never rescanned, so the following `-C sub` is still read. + assert_eq!( + values(&["git", "-C", "--git-dir", "-C", "sub"]), + vec!["--git-dir", "sub"] + ); + assert_eq!( + values(&["git", "-C", "-pc", "-C", "sub"]), + vec!["-pc", "sub"] + ); assert_eq!( values(&["git", "--work-tree", "sub", "-C", "deep"]), vec!["deep"] diff --git a/tests/issue645_bash_option_directory_variants.rs b/tests/issue645_bash_option_directory_variants.rs index 81f6e10a..5e2121d3 100644 --- a/tests/issue645_bash_option_directory_variants.rs +++ b/tests/issue645_bash_option_directory_variants.rs @@ -71,6 +71,32 @@ fn fixture() -> Fixture { Fixture { _dir: dir, root } } +/// A workspace whose every `escape` directory (a nested path such as +/// `--git-dir/sub`) holds the outward symlink `lf2`, plus a root file `f`. Used +/// where the option value itself is a directory name that looks like an option. +fn fixture_with_escape_dirs(escape: &[&str]) -> Fixture { + let dir = tempfile::tempdir_in(env!("CARGO_TARGET_TMPDIR")).expect("create tempdir"); + assert!( + !["/usr", "/bin", "/opt", "/etc", "/tmp"] + .iter() + .any(|prefix| dir.path().starts_with(prefix)), + "the fixture tempdir must not sit under a system prefix: {}", + dir.path().display() + ); + let root = dir.path().join("ws"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::write(root.join("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 directory in escape { + std::fs::create_dir_all(root.join(directory)).unwrap(); + std::os::unix::fs::symlink(&outside, root.join(directory).join("lf2")).unwrap(); + } + let root = root.canonicalize().unwrap(); + Fixture { _dir: dir, root } +} + /// Asserts every command is refused by a read-side `path reference`, so the /// relative read after the option escapes the workspace (`R=1`). fn assert_read_rejected(root: &Path, commands: &[&str]) { @@ -150,6 +176,32 @@ fn git_value_taking_global_options_reject_relative_reads() { ); } +/// A `-C` value that is itself spelled like a value-taking global option +/// (`--git-dir`, `--namespace`, `-pc`, `-c`) must still be read as one literal +/// path word: `git -C --git-dir -C sub log lf2` chdirs to `--git-dir`, then to +/// `sub`, then reads `lf2` under `sub`. `git -C` never rescans its value, so the +/// following `-C sub` is not swallowed. The pre-fix code misread the value as a +/// global option and skipped `-C sub`, letting `sub/lf2` escape (`R=0`). +#[test] +fn git_c_value_named_like_a_global_option_still_reads_the_next_c() { + let fixture = fixture_with_escape_dirs(&["--git-dir/sub", "--namespace/sub", "-pc/sub"]); + let root = &fixture.root; + assert_read_rejected( + root, + &[ + "git -C --git-dir -C sub log lf2", + "git -C --namespace -C sub log lf2", + "git -C -pc -C sub log lf2", + ], + ); + + // The fourth form uses its own fixture that places `-c/sub/lf2` only and + // never `-c/lf2`, so the pre-fix code (which stops at `-c`) cannot reject it. + let fixture = fixture_with_escape_dirs(&["-c/sub"]); + let root = &fixture.root; + assert_read_rejected(root, &["git -C -c -C sub log lf2"]); +} + /// Table 2: the workspace-internal reads and writes through these options are /// not falsely refused; their value is unchanged. `git commit -C HEAD` stays /// allowed because `git -C` is global only before the subcommand. From 1ba873893e4ecb97565eb3439f61126893cff2fa Mon Sep 17 00:00:00 2001 From: kewton Date: Sun, 11 Oct 2026 03:27:17 +0900 Subject: [PATCH 4/4] Pin a `--` option value that must not end the option scan Add a leaf unit case (`values(["tar", "-C", "--", "-C", "sub"])`) and a read rejection (`tar -C -- -C sub -tf lf2` with `sub/lf2` an outward symlink) for a non-git table program whose `-C` value is literally `--` (Issue #645). The value is one path word, so the walk advances two words and still reads the following `-C sub`. Product code is unchanged; the two cases fail under the `&&` -> `||` mutation of value_step. Co-authored-by: CommandCodeBot --- src/tools/bash_write_guard/option_directory.rs | 7 +++++++ tests/issue645_bash_option_directory_variants.rs | 11 +++++++++++ 2 files changed, 18 insertions(+) diff --git a/src/tools/bash_write_guard/option_directory.rs b/src/tools/bash_write_guard/option_directory.rs index 90d7b759..28a4e8ab 100644 --- a/src/tools/bash_write_guard/option_directory.rs +++ b/src/tools/bash_write_guard/option_directory.rs @@ -312,6 +312,13 @@ mod tests { assert_eq!(values(&["tar", "-fC", "-Csub"]), vec!["-Csub", "sub"]); } + #[test] + fn a_double_dash_value_is_consumed_as_one_word() { + // `-C`'s value is `--`, one literal path word, not an end-of-options + // marker: the walk advances two words and still reads `-C sub`. + assert_eq!(values(&["tar", "-C", "--", "-C", "sub"]), vec!["--", "sub"]); + } + #[test] fn reads_npm_short_directory_option() { assert_eq!(values(&["npm", "-C", "sub"]), vec!["sub"]); diff --git a/tests/issue645_bash_option_directory_variants.rs b/tests/issue645_bash_option_directory_variants.rs index 5e2121d3..37e5588c 100644 --- a/tests/issue645_bash_option_directory_variants.rs +++ b/tests/issue645_bash_option_directory_variants.rs @@ -202,6 +202,17 @@ fn git_c_value_named_like_a_global_option_still_reads_the_next_c() { assert_read_rejected(root, &["git -C -c -C sub log lf2"]); } +/// A non-`git` table program's `-C` value may literally be `--` +/// (`tar -C -- -C sub`). It is one path word, not an end-of-options marker, so +/// the walk advances two words and still reads the following `-C sub`; `lf2` +/// under `sub` then escapes. A one-word advance would stop at `--` and miss it. +#[test] +fn option_value_double_dash_still_reads_the_next_option() { + let fixture = fixture_with_escape_dirs(&["sub"]); + let root = &fixture.root; + assert_read_rejected(root, &["tar -C -- -C sub -tf lf2"]); +} + /// Table 2: the workspace-internal reads and writes through these options are /// not falsely refused; their value is unchanged. `git commit -C HEAD` stays /// allowed because `git -C` is global only before the subcommand.