#567 [security][tools] Bash の書き込み先の glob・brace を展開前の文字列で判定するため、中の symlink を通ってワークスペース外へ書き込める（#509 の調査で発見） - #573
Merged
Kewton merged 4 commits intoOct 1, 2026
Conversation
Bash write targets that use glob (`* ? [`) or brace (`{a,b}`, `{1..9}`)
syntax were judged on the pre-expansion word. That name does not exist, so
the literal proof fell back to the nearest existing parent -- the workspace
root -- and allowed writes that the shell resolves onto an intermediate
symlink leaving the workspace, or onto a brace-produced `..`.
Expand the word the way the shell would -- braces first (nested, `,`,
ranges), then globs one component at a time without following symlinks --
and re-run the literal proof on every result. Fail closed once the brace
(256) or glob (4096) expansion limit is exceeded, and never read a directory
outside the workspace root.
- src/tools/path_guard.rs: split the literal proof into ensure_single_target
and call the new child module from ensure_bash_write_target.
- src/tools/path_guard/bash_pattern.rs: new child module with the expansion.
- tests/issue567_bash_glob_write_targets.rs: reject/allow/cap/loop coverage.
判断: the brace cap 256 and glob match cap 4096 are the issue's suggested values; tests pin only that an overshoot is rejected, not the exact boundary.
読み替え: `tee */f` is allowed only in a normal workspace whose root holds no escaping symlink for `*` to match; the issue's "許可のまま" list is read that way.
本文に無い指摘: a glob used as a directory component must drop non-directory matches, otherwise `tee */f` is over-rejected whenever the root also holds a regular file.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
…on up front Follow-up to #567 (review H-02). Two blockers and three robustness gaps: - POSIX bracket expressions (`[[:class:]]`, `[[=c=]]`, `[[.coll.]]`) are refused as unprovable: globset reads them as an ordinary class, so the shell would expand a word the guard silently passed through literally. - A leftover `{`, `}`, `,`, or `\` is rewritten to a character-class literal (`[{]`, `[}]`, `[,]`, `[\\]`) before globset, because globset reads those as alternation/escaping while the shell reads them as ordinary characters. Without this, `{x}*` matched nothing and fell back to allowing the literal. - Brace expansion is counted before expanding, so an over-limit word (256) is rejected without growing a queue; a range is sized before building a vector and its loop uses checked_add, so `{1..100000000}` and an i64-spanning range fail fast instead of allocating or overflowing. - `{01..02}` is proven both zero-padded (`01`, bash 4+) and stripped (`1`), including when only one endpoint carries the leading zero. - `dotglob` is documented with extglob/GLOBIGNORE/nocaseglob as a known limit. 判断: the requested cargo-mutants command (`--lib --test`, no filter) cannot complete inside the 30-minute cap on this machine: the lib suite is 2854 tests / ~45s / ~670s CPU each, so 172 mutants exceed the budget. Ran the integration selection to completion and cross-checked every survivor against the module's own unit tests. 読み替え: `{9223372036854775806..9223372036854775807}` is two safe inside values, so it is allowed, not rejected; the test pins no-panic + a time bound, and a genuinely overflowing span is asserted rejected. 本文に無い指摘: the asymmetric padding case `{01..2}`/`{1..02}` was not covered by the symmetric `{01..02}` test; added both. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Follow-up to #567 review L-02. The leader's mutant run left 12 survivors in the new numeric-range code. Add direct unit coverage and an integration case: - RangeSpec::values: full-list assertions for ascending/descending, step, endpoints, a single value, zero-padded (both forms) and character ranges. - RangeSpec::bounded_count: counts for the same shapes, a span no step divides exactly, zero-padded doubling, a zero step, and the limit / limit+1 cap. - Integration: `{k..m}`, `{1..3}` (with `2` -> outside) and `{1..5..2}` (with `3` -> outside) are rejected only because the range expands. Manual per-mutant classification (apply mutation, run tests; not cargo-mutants): 10 of 12 killed. Two survive and are provably equivalent: - bounded_count `limit as u128 + 1` -> `* 1`: `count > limit + 1` and `count > limit` agree at every count (<= limit -> count; == limit+1 and >= limit+2 -> limit+1), so no test can distinguish them. - values `direction < 0` -> `<= 0`: direction is only ever +1 or -1, never 0, so the two conditions are identical. 判断: no production change needed. The `values -> vec![]` survivor did not reproduce: with it a range expands to nothing and the guard allows, which makes the reject-asserting tests fail, so the suite kills it (lib and integration). 本文に無い指摘: the reported 174:9 survivor is a stale-build artifact, not a test hole. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
…e range step Follow-up to #567 review (round 3). - globset_pattern rewrote a literal `{`, `}`, `,`, or `\` into a class literal even inside an existing bracket, nesting it (`[,l]` -> `[[,]l]`). globset then read a different set, matched nothing, and fell back to the literal proof, so `[,l]inked-outside/` was allowed. It now returns None when the component contains `[` together with any of `{ } , \`, and the caller refuses the word as unprovable (same treatment as `[:`/`[=`/`[.`). - range_spec used `step.abs()`, which panics in debug for i64::MIN (`{1..5..-9223372036854775808}`); it now uses `checked_abs()` and the word stays literal. Tests: unit coverage for globset_pattern's None cases and the overflowing step; an integration table rejecting `[,l]inked-outside/`, `[{l]inked-outside/secret`, `[!,]inked-outside/secret`, `[l}]`, `[^{]`, `[!{]*`, `[k-m,]`, plus rm/chmod forms; and allow cases confirming brackets without `{ } , \` and braces without a bracket (quoted `[id]`, `cat src/{main,lib}.rs`, `ls src/*.rs`, `tee src/*.rs`, `cp a.txt "src/[id]/x"`) still pass. 判断: a bracket mixed with `{ } , \` is refused rather than expanding both spellings, because any rewrite nests the brackets and cannot be proven to match the shell. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
7 tasks
Kewton
deleted the
feature/issue-567-security-tools-bash-glob-brace-symlink-509
branch
October 1, 2026 11:48
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
src/tools/path_guard.rs:ensure_bash_write_targetnow calls the new expansion check before its existing proof (a few lines).src/tools/path_guard/bash_pattern.rs(new):..reached through.*/.?/{.,}..[:...:],[=,[.). Leftover literal{,},,and\are escaped before matching.tests/issue567_bash_glob_write_targets.rs(new): integration tests throughpath_confinement_rejection.ls src/*.rsandcat src/{main,lib}.rs, and quoted Next.js routes such as"src/app/[id]/page.tsx", stay allowed.Verification
commandmate verifyon the final head)..., brace, POSIX class, literal brace, zero-padded range) fail. With this change, they pass. The resource-cap tests hung before the fix and now return quickly.tests/issue428_bash_path_tokens.rsandtests/bash_workspace_confinement.rs.bash_pattern.rs: all 172 mutants were measured, and unstable results were re-measured one at a time. The only survivors are equivalent boundary mutants inRangeSpec::bounded_count(the result staysmin(count, limit + 1)). Timeouts are loop mutants that never finish, so they count as detected.Notes
extglob,GLOBIGNORE,nocaseglob,dotglob, and a symlink swapped between the check and the run. Conservative false rejections are possible, for examplelin\*and a quoted[id]wheniordis a symlink pointing outside.cat lin*/secret, etc.) are out of scope and tracked in [security][tools] Bash の 2 段目の読み取りの判定が、引用符のない glob・動的な値・glob を含む絶対パスで抜け、ワークスペース外を読める（#567 の調査で発見） #571.>|と数字でない>&を redirect として読まず、ワークスペース外へ書き込める（#509 の調査で発見） #565 (merged, PR #565 [security][tools] Bash の書き込み検査が>|と数字でない>&を redirect として読まず、ワークスペース外へ書き込める（#509 の調査で発見） #572), [security][tools] Bash の書き込み検査が env・sudo・timeout などの前置きや{ … }の中の書き込みを認識せず、ワークスペース外へ書き込める（#509 の調査で発見） #566, [security][tools] Bash の書き込み検査が同じコマンドの cd・pushd の後の cwd を追わず、symlink を通ってワークスペース外へ書き込める（#509 の調査で発見） #568, [security][tools] Bash の字句検査では、プログラム経由の間接書き込みや検証コマンドの副作用を閉じ込められない #502.Developed with the CommandMate parallel-dev harness (PM: Claude Code, leader: Claude_Dev, workers: Command Code).
🤖 Generated with Claude Code