Repository navigation
Conversation
asto18089
left a comment
There was a problem hiding this comment.
Fresh review — verdict: CHANGES_REQUESTED (2 BLOCKER, 4 MAJOR)
Reviewed from scratch at head faae44b7b (5 DCO commits, 10 files, +960/−78). The branch already rides the pinvou3-clean tip 6e1ee88ee (merge-base == base), so no rebase was needed. CI is green on this head. Twelve independent review lanes plus my own reproductions; every BLOCKER/MAJOR below was reproduced by execution (not inferred), with HEAD-vs-base comparisons where "new vs pre-existing" matters.
The three fixes are real and root-caused: the stdin-EOF diagnosis is correct and both pins bite red/green; compound probes were a genuine live pain point; the test role contract was dishonest and is now honest. But the compound grammar opens two workspace-boundary content reads that the PR's own cd-symlink hardening does not cover, the posture re-keying changes more than the body claims, and one hunk is out of scope.
BLOCKER-1 — cd as a pipeline stage escapes the operand gate (arbitrary file read under ShellPolicy::ReadOnly)
enforce_readonly_segment_operands (crates/tui/src/tools/shell.rs:4156-4174) returns early in its cd branch after checking only argv[1]. The gate splits on ;/&&/|| but never applies the | split the classifier applies, and is_agent_readonly_segment admits cd as any pipeline stage (command_safety.rs:689) — contrary to its own doc, which says "standalone segment or sequence prefix" (command_safety.rs:565). So for cd sub | cat /etc/passwd the gate sees ["cd","sub","|","cat","/etc/passwd"], resolves sub, returns Ok, and /etc/passwd is never inspected; the shell runs the pipeline with cd in a subshell.
Reproduced end-to-end through LowercaseBashTool with ShellPolicy::ReadOnly (HEAD): cd sub | cat /etc/passwd → success=true, content ## # User Database …. Same for cd sub | cat <ws>/link (symlink → outside file, content returned). Base refuses the whole shape at the classifier. The body's claim that the gate "splits with the same shared splitter and judges every segment" is false for pipelines.
Fix: enforce the documented position rule — refuse cd as a pipeline stage (in the classifier and/or the gate) — or make the cd branch fall through to per-token checking of the remaining stage(s). Add a pin for cd sub | cat /etc/passwd and cd sub | cat; cat link.
BLOCKER-2 — glob operands expand past containment; the sequence/cd forms are new
The operand loop checks the literal spelling; * never exists on disk, so joined.exists() (shell.rs:4239-4240) skips it, and the shell lane then expands the glob — including symlinks that point outside. Reproduced (HEAD): cd sub; cat * and cd sub && cat * with sub/link → /tmp/outside/secret.txt → success=true, content inner\nTOPSECRET-OUTSIDE. Also cd sub; head -c 100 l*, cd sub; od -c *, and metadata oracles (wc -c *, stat *, ls -la *, file *).
Honest attribution: the pipeline form (cat * | head) leaks at base too (same gate skip, same shell lane), so the core gap pre-exists. This PR (a) newly makes it reachable through every sequence and post-cd form, and (b) adds a doc comment that is false as written — command_safety.rs:572-573: "glob * arguments, expanded by the shell only against workspace paths the operand gate already confines". Since the PR's stated review-round MAJOR was exactly the sibling escape (cd sub; cat link), this one belongs in the same change.
Fix: refuse operands containing */? at the gate (or expand and verify each match), and correct the doc comment.
MAJOR-1 — gh network policy is bypassed by any compound
enforce_readonly_github_network_policy (shell.rs:3887-3910) decides via is_github_readonly_command(command) on the whole command, which requires is_parallel_readonly_command and therefore returns false for any compound → return Ok(()) with the decider never consulted. Reproduced under a Deny-all NetworkPolicyDecider: bare gh repo view → blocked with the policy message; pwd; gh repo view and pwd && gh repo view → gh executed. Pre-existing via pipelines (gh repo view | head hits the same whole-command keying), but the PR newly admits the ;/&&/|| vectors and rewrote exactly this gh exemption per-segment in the operand gate (shell.rs:4146-4149) while leaving its twin network seam whole-command. Fix: evaluate the network policy per segment with the same splitter (and per pipeline stage), i.e. if any segment is a gh read, consult the decider.
MAJOR-2 — the posture re-keying changes capability for six roles, not one; the truth-table claim is false
posture_permits_tool's Required branch (mod.rs:15291-15293) and shell_is_denied (mod.rs:15445-15447) now key on role_posture_permits(role, Required), which is true for Worker, Scout, Reviewer, Builder, Verifier (all for_role().shell == Full, worker_profile.rs:212-237) and unconditionally for Custom (mod.rs:14512). The old key was runtime.shell == Full. So every one of those roles flips whenever its runtime shell is ReadOnly, not just the Verifier.
Executed matrix at base and HEAD (private worktrees, same scratch harness): for runtime shell ReadOnly, shell_is_denied and the Required posture flip true→false / false→true for general, implement, custom, test, explore and reviewer; the Run catalog flips false→true for general/implement/custom/test (explore/reviewer stay catalog- and dispatch-blocked by bounded_inspection). Concrete reachable path — the plain agent-tool nesting (Scout parent → Builder or read-only-start Custom child, derive_child → write=false, shell=ReadOnly, no ChildAuthority deny list): Run {"action":"tests","args":"--lib x"} at HEAD → OK … exit_code 101 … could not find Cargo.toml (cargo was forked); at base → [role.posture.denied]. Run {"action":"verifiers"} likewise executes at HEAD (gate scan ran). Custom commands, automation, raw shell and writes still refuse (envelope write_denied/unbounded), and the Fleet path stays closed because ChildAuthority::clamp installs the verification deny list + sentinel (pinned by a_read_only_parents_delegation_never_widens_authority). Scout/Reviewer remain blocked by bounded_inspection. The (write=true, shell=ReadOnly) cell — where the new envelope would admit the whole Executes class (MCP/automation/plugins) — is not reachable by any operator path (traced: presets, manifests, spawn schema, engine turn clamp only reaches ToolContext), so that is latent, not a merge risk.
Net: the body's "an eight-role truth table was verified pre/post … the only delta is Verifier's bounded bash lane" is false by execution, and the "Checked and left alone" paragraph's "stays bounded and fail-closed" understates it — a read-only parent's Builder/Custom child now starts the workspace test runner that the parent cannot. That is an authority widening through delegation on the plain path, unpinned. Fix: grant the carve-out only to the Verifier (e.g. matches!(runtime.shell, Full) || (matches!(agent_type, Verifier) && runtime.shell != None) in both seams), which makes the claim true and keeps the intended Verifier surface; pin the Builder/Custom-under-read-only-parent cells. If the broader re-key is actually intended, say so in the body and pin it.
MAJOR-3 — unconditional foreground stdin close breaks interact after a foreground→background detach
execute_foreground_via_background now closes the write end unconditionally (shell.rs:4466-4472) before the poll loop that services detach requests (take_foreground_background_request, shell.rs:4487; UI trigger request_active_foreground_shell_background, tui/ui.rs:1095-1106). Probed: foreground sleep 30 → detach → write_stdin("hello\n") → Err(stdin is not available for task …). Pre-PR a no-payload foreground run kept the writer (pipe or PTY), so feeding input after detach worked. The new comment "no later interact call" (shell.rs:4455-4459) is false for this path; no pin covers it (detach tests at tests.rs:3340/3404 never interact). Fix: close the write end only when the command finishes without a detach, or re-open/keep the writer when a detach request is observed; pin detach-then-interact.
MAJOR-4 — out-of-scope hunk: the reviewer-prompt sentence
REVIEW_AGENT_INTRO gains a line-number-discipline sentence (mod.rs:16588) plus two assertions in verifier_and_reviewer_intros_state_the_evidence_contract. It edits the reviewer's prompt, which is none of the three fixes, and the body's enabler ("wc -l — now actually available to read-only roles") is false for the reviewer: Scout/Reviewer/Planner already had the bounded bash lane at base (allows_bounded_readonly_bash at 6e1ee88ee). Disclosed, one sentence + test, trivially separable — please move it to its own PR.
MINOR
set -o pipefailprefix on compounds with a pipe (shell.rs:2193-2194) fails on dash/fish hosts ($SHELLor/bin/shis used verbatim,shell_dispatcher.rs:487-550); pre-existing for bare pipelines, newly reachable fora; b | c. Fail-closed, host-dependent. Source-derived, not reproduced (host is zsh).- Git compounds take the shell lane and lose
hardened_readonly_argv's--no-ext-diff --no-textconv --no-show-signaturesplice (shell.rs:2198vs:2200); env hardening still applies. Pre-existing forgit log | wc. readonly_sequence_segmentsdoc (command_safety.rs:622-628) says the charset gate "has already refused every metacharacter that could make a quoted separator behave differently" — quotes and backslash are not refused; the conclusion (mis-split → seam refusal) holds and was verified (cat "x;cat y"admitted by classifier, refused by seam), but the stated reason is wrong.- The cd fold is O(depth²) in nested-
cdchains (cd a;cd a;…: 125 → 15 ms, 250 → 83 ms, 400 → 308 ms measured, debug), PATH_MAX-bounded and fail-closed, with no command-length cap upstream. Flat chains are linear (10k segments → 72 ms). Cheap hardening: cap segment count or command bytes at the tool boundary. - Attached short-option values are refused for advertised families:
du -sh .,cut -d, -f1 f,comm -12 a b,od -cv frefuse; the separated spellings admit. A model pays one refusal + rewrite on the most common spelling. - Stale copy the PR missed:
docs/SUBAGENTS.md:281-283(testwhen-to-use bullet still describes only the verification surface);docs/zh_hans/FLEET.md:124("只保留…有界验证工具" is now false for the verifier);fleet/executor.rs:241-246(3-role list + comment "Verifiers keep the dedicated Run surface" now inaccurate);fleet/role.rs:644retains legacyBashonly for S/R/P while the registry path carves the verifier via a second mechanism;worker_runtime.rs:1588label pin still lists three roles (the PR's declared follow-up). - The guidance sentence (
guidance.rs:88-92) is unconditional, but foregroundcombined_output:trueruns under a PTY (shell.rs:5496). On macOS the PTY writer drop still delivers EOF (verified:catexits in ~57 ms); on Linux the reader's dup may keep the master open — unproven here. - Four enumeration sites now list
Scout | Reviewer | Planner | Verifierwhilerole_requires_read_only_shellalready exists and is imported inmod.rs; two sibling sites already diverged (above).
NIT
assert!(!custom.to_string().is_empty()) (subagent/tests.rs:8341) is vacuous after expect_err; prose-matching pins at :8319/:8354 ("not available" || "not allowed") vs the tag-based pin used for the lane refusal; bash_lane_role reads as "is lane role" but means "must not be granted here" (the literal "bash" argument is load-bearing — passing name would let Bash re-enter); two naive splitters now coexist (readonly_sequence_segments :629 and split_command_segments :1587) with different empty-segment/& contracts and no cross-reference; od re-implements the option-loop scaffolding of agent_text_filter_options_match; refusal copy lists ; unconditionally while cmd.exe refuses it at the seam; gh … reads is a family summary (gh repo list, gh release download refuse).
Verified OK (by execution)
- Fix 1 root cause confirmed from base:
execute_sync_sandboxedinherited stdin (hunk is in that function,shell.rs:2368-2377); foreground kept the pipe. Payload path still writes+flushes before close (shell.rs:2887-2892); manager close is idempotent on PTY/persist/completed lanes. Backgroundinteractpins green. Red/green: removing theStdio::null()branch reddens exactly the sync pin; restoring theif stdin_data.is_some()guard reddens exactly the foreground pin. - All 8 claimed mutations redden real pins (no inert pins); m8 reddens exactly the drift pin; m5 reddens only the classifier-level routing pin on macOS (
/usr/bin/cdexists), confirming the body's host-independence rationale. Three extra mutations show the Verifier re-key andbash_lane_roleare load-bearing. - Verifier lane: canonical
bashprobe runs (wc -c→ 10);Bash/BASH/exec_shellrefuse atrole_blocks_unhardened_process_toolbefore any normalization;rm -rfrefuses with[shell.readonly.command]; Run default/selection admitted; custom Run refused;is_parallel_readonly_commandbyte-identical to base. - Classifier probe matrix (65 inputs) fails closed on every tricky separator/quote shape; the PR's own pinned escapes (
cd sub; cat link,cd sub && od -c link) are refused end-to-end. - Every family in the new refusal copy is actually admitted as described; nothing promised is refused. en/zh doc edits are faithful.
- Smuggle audit: all hunks map to Fix 1/2/3 except MAJOR-4; DCO 5/5; merge-order claims exact (
git merge-tree: one string-literal conflict vs #123, clean vs #124/#125). - Gates (macOS arm64, stable 1.99.1):
cargo fmt --checkclean; clippy (CONTRIBUTING all-targets form) → only the pre-existingrepo_law.rs:638 cloned_ref_to_slice_refs; targeted counts exactly as claimed (79/0, 148/0/1, 606/0, 361/0); full--libtwice: 12287/1 and 12286/2, failures = the named flake class, each 1/1 alone. - Windows cfg block: names exist (
ShellKind::Cmd,global_dispatcher().kind()); force-compiled on macOS by flipping the cfg — compiles clean.
Net
Direction is right and the fixes are real, but BLOCKER-1/2 are workspace-boundary content reads introduced (or newly reached) by this change, MAJOR-1 bypasses a network policy on the same new surface, MAJOR-2 contradicts the body's central claim, MAJOR-3 regresses detach+interact, and MAJOR-4 should be its own PR. Please fix and re-request; a fix wave that refuses cd as a pipeline stage, refuses glob operands at the gate, applies the network policy per segment, and narrows the posture carve-out to the Verifier would close everything above the MINOR line.
Fix wave for review 5669186887 — head
|
| Finding | Fix | Commit | Pins (all red under reverse mutation) |
|---|---|---|---|
BLOCKER-1 cd as a pipeline stage skipped every operand after the | |
Classifier refuses cd as a pipeline stage (a stage runs in a subshell and persists nowhere the gate could fold); the gate judges every stage on its own. While doing this I found the same shape via gh: gh issue list | cat /etc/passwd skipped the gate too because the gh exemption covered the whole segment — that one predates this PR and is closed by the same per-stage walk (gh now exempts only its own stage). |
c5e2b48a6 |
agent_readonly_shell_rejects_unsafe_compound_cd_and_od_shapes (classifier), readonly_operand_gate_judges_every_pipeline_stage_on_its_own (gate, cd + gh cases), readonly_shell_refuses_pipeline_cd_and_glob_escapes_end_to_end |
| BLOCKER-2 glob operands expanded over outside symlinks | * operands (the only wildcard the charset admits) are expanded against the live cwd the way the shell does — component by component, hidden entries only when the component asks, symlinked dirs followed — and every match is judged by resolved location. Closes the pre-existing cat * | head form as well; the false "already confines" doc comment is corrected. No glob library: with ?/[/{ charset-refused, a *-only matcher is ~40 lines. |
c5e2b48a6 |
readonly_operand_gate_judges_glob_expansions_by_resolved_location, the end-to-end pin above |
| MAJOR-1 gh network policy bypassed by compounds | The guard judges every sequence segment and pipeline stage with the classifier's splits; a gh read anywhere consults the decider. Also closes the pre-existing gh … | head form. |
c5e2b48a6 |
readonly_github_shell_calls_obey_the_host_network_policy_before_spawn (compound cases) |
| MAJOR-2 posture re-key flipped six roles | holds_shell_authority keys on the held shell policy plus one named exception (Verifier with a non-None shell); role_posture_permits is no longer consulted for shell authority. Builder/Worker/Custom children of a read-only parent are back to refusing Run at posture, envelope and catalog. allows_bounded_readonly_bash and the catalog branch read the role set from role_requires_read_only_shell instead of enumerating it; bash_lane_role renamed to say what it does. |
e52aa813c |
read_only_shell_children_of_other_roles_hold_no_shell_authority (new), the existing Verifier pins stay green |
MAJOR-3 detach-then-interact regression |
Kept the EOF-at-spawn contract (reverting it reintroduces the $SHELL wedge; a pipe cannot deliver EOF and later accept input). The false "no later interact call" comment is replaced by the real contract, interact on a detached foreground command now says the stdin is at EOF and points at background=true, and the behaviour is pinned instead of accidental. |
3a297ded4 |
detached_foreground_command_reports_its_closed_stdin_on_interact |
| MAJOR-4 reviewer-intro sentence out of scope | Dropped from this PR (sentence + its two assertions); can be proposed on its own. | 22fbe0bfb |
— |
MINORs taken: clustered short options in the text-filter grammar (cut -d, -f1, comm -12, sort -rn; expansion only when every letter is admitted — 09aab42fe, pinned by agent_text_filters_admit_clustered_short_options); 32-segment cap on sequences for the quadratic nested-cd fold (agent_readonly_shell_bounds_the_number_of_sequence_segments); splitter doc now states why quoted separators mis-split instead of claiming they cannot; verifier added to the narrowed-shell label pin; vacuous !is_empty() assert replaced by one naming the two walls that own the custom-Run refusal (the dispatch refusal comes from the approval wall, not the unbounded guard, which the vacuous assert had hidden); docs/SUBAGENTS.md test bullet, docs/zh_hans/FLEET.md, and the fleet/executor.rs comment updated; refusal copy names the two newly refused shapes.
Deliberately not changed, disclosed in the body's "Checked and left alone": find -L/-follow outside-filename listing and the git compounds' missing --no-ext-diff --no-textconv --no-show-signature splice (both predate this PR; env hardening still applies to git); set -o pipefail on dash/fish hosts (same lane bare pipelines already used); the Linux PTY/combined_output corner of the guidance sentence (true on macOS, unproven on Linux here); the executor-path ToolShellAuthority list still excludes the Verifier (comment now says so — a behaviour change there deserves its own look).
Gates at 3a297ded4 (macOS arm64, stable 1.99.1): cargo fmt --check clean; clippy (CONTRIBUTING all-targets form) exit 0 with the documented repo_law.rs:638 baseline allowed; command_safety:: 81/0, tools::shell:: 152/0/1 ignored, tools::subagent:: 607/0, fleet:: 361/0; full --lib 12292/3, the three in runtime_threads::tests (known load-timing class, module untouched, each 1/1 alone, 183/0 at --test-threads=2). Merge-order note unchanged: still exactly one string-literal conflict with #123 (the refusal copy), clean against #124/#125.
asto18089
left a comment
There was a problem hiding this comment.
Fresh review (round 3, from scratch) — head 3dbb1f44c on pinvou3-clean 6e1ee88ee
Verdict: CHANGES REQUESTED (posted as a comment because author == reviewer). No blocker, 6 MAJOR (4 new, 2 pre-existing-but-inside-the-PR's-claimed-closure), 2 pre-existing escapes in the same gate that should ride along, and a set of minors.
Setup. Merge-base == pinvou3-clean tip, so the rebase is a no-op; CI green at head. 15 independent review lanes (classifier adversarial, operand-gate live probes under three shells, authority matrix at base and head, two mutation batteries, base-vs-head defect reproduction, design, docs, scope, gates, perf). Every finding below was re-executed by me, not just reported by a lane. Earlier rounds were not consulted.
What holds up (verified)
- All three defects reproduce at base and are fixed at head with independent probes identical in both trees: foreground
cat/$SHELLwedge 3071 ms → 30 ms; sync laneTimedOuton a held-open pipe →Completedin 11 ms;wc -c f; wc -l f/cd sub && …/od -c f/cut -d, -f1 frefused → admitted and executed; verifierbashabsent from catalog and refused → admitted and executes, Run surface permitted in both trees. - Gates:
cargo fmtclean; strict all-targets clippy reports exactly the recordedrepo_law.rs:638baseline and nothing else;command_safety::81/0,tools::shell::152/0/1,tools::subagent::607/0,fleet::361/0 — digit-for-digit as the body claims; full lib 12295/0/13 (with the repo-requiredRUST_MIN_STACK=16777216; without it the suite aborts in a pre-existingruntime_apitest — environment, not this PR); rest of workspace 2311/0. - Mutations: 31 of 35 reverse mutations redden exactly their named pins (17/20 classifier+gate+network, 14/15 stdin+authority), including the three authority-widening variants and both mutations that equal base behavior. The 4 survivors are pin-strength gaps listed under minors, none a live hole.
- No smuggling: every hunk attributes to F1/F2/F3; 11 commits DCO-signed, conventional, messages match diffs; the
secret→outside_pathrename commit is pure test churn. Merge-tree claims exact: #123 conflicts on precisely the[shell.readonly.command]literal, #124/#125 clean;is_parallel_readonly_commandbyte-identical (same md5 base/head); #119 is in base. - Hardening parity on the widened shell lane was probed, not assumed: with a repo-local
diff.<driver>.textconv/diff.external/core.fsmonitorhook, the shared env (GIT_EXTERNAL_DIFF=<inert>,GIT_CONFIG_COUNT core.fsmonitor=false …) stops the driver from executing on both lanes (shell lane fails loud with "external diff died" instead of showing the raw diff — nit only). - Quoted-separator mis-splits are fail-closed as documented: every
cat "a;b"-family shape either refuses at the classifier or dies at the seam'sshlex; I could not construct an input where both halves classify,shlexaccepts, and the shell runs a non-admitted command.
MAJOR — must change before merge
M1 — NEW — cd <glob> is judged as a literal while the shell cd's into the first match → outside read.
is_agent_readonly_cd (command_safety.rs:1019-1034) admits *; the seam's cd branch (shell.rs:4194-4213) canonicalizes the spelling cwd/cdt*. With a literal dir cdt* (inside) and a sibling cdt -> .. (outside, sorts first), bash/sh glob cdt* → [cdt, cdt*], cd takes the first and ignores the rest. Executed under SHELL=/bin/bash, /bin/sh, and $SHELL unset: cd cdt*; cat secret.txt | head -5 → success=true, stdout = the outside secret. zsh fails closed. Base refuses all ; compounds, so this is introduced by the PR's own surface. Fix: refuse * in the cd operand at the classifier (the doc already says "one relative path"); optionally also judge a glob cd operand through the walker and refuse multi-match.
M2 — PRE-EXISTING leak, but inside the closure this PR claims — .* expands to ./.. on bash/sh; the walker never sees ...
collect_readonly_glob_matches (shell.rs:4352-4395) enumerates with read_dir (:4374), which never yields ./..; bash and /bin/sh (macOS fallback when $SHELL is unset) expand .* to . .. .h. Executed: cat .*/secret.txt | head -5, pwd; cat .*/secret.txt | head -5, sed -n '1,5p' .*/secret.txt | head -5, od -c .*/secret.txt | head -4 all return the outside content under ReadOnly on bash/sh (the od route is new at head). The body's "expands each one … the way the shell expands it" is false for this spelling. Fix: when a component starts with ., synthesize . and .. as candidates (readonly_star_match(".*", "..") already returns true) so they are location-judged; zsh hosts then over-refuse .*, which is fine.
M3 — PRE-EXISTING for the bare form, NEW over-claim — a * in a gh stage makes the network guard skip the policy.
enforce_readonly_github_network_policy (shell.rs:3900-3905) keys each stage on is_github_readonly_command, which calls is_parallel_readonly_command first (command_safety.rs:1086-1089) whose charset refuses * (:482) — while the agent classifier admits * and a gh segment falls through readonly_tokens_admitted with no charset gate. Executed: gh pr list --search '*' and pwd; gh repo view * → classifier true, guard sees no gh stage → a Deny network policy is never consulted; the gh operand exemption also skips the *. Contradicts "a gh read anywhere in a compound consults the decider exactly like a bare one". Fix: recognize gh stages on tokens (shell_words + GITHUB_READONLY_PREFIXES + option/host tables), not via the parallel gate; pin pwd; gh pr list --search '*' under Deny.
M4 — NEW — the glob walker is an unbounded, symlink-cycling, in-host walk with no cap, no visited-set, no timeout, and no spawn_blocking.
collect_readonly_glob_matches follows symlinked dirs via is_dir() (:4369/:4391), collects all matches before judging (:4335-4336), canonicalizes each, and runs inside async fn execute (:5163 → :5375; grep -c spawn_blocking shell.rs = 0). The child has a watchdog; the gate has none. Measured on a 30-file workspace with two -> . symlinks (test profile): depth 6 → 23 ms, 8 → 108 ms, 10 → 507 ms, 12 → 2.3 s, 14 → 10.4 s (×4.5 per +2; minutes at depth ≥ 16, multi-GB match vector by depth 20). Reachable by a read-only child's command choice (cat */*/*/*/*/*/*/*/*/*/*/*) in any workspace holding an ancestor-pointing symlink (pnpm/Bazel link farms, docs -> . mirrors). MAX_READONLY_SEQUENCE_SEGMENTS does not bound this (32 × cat * over a 1k-entry dir is already 309 ms). Fix: (dev, ino) visited-set, hard match cap (fail closed), judge-while-walking with early return, and spawn_blocking around the gate.
M5 — NEW, deliberate but undisclosed as delegation widening — a read-only role parent now obtains test-runner execution via agent(type=test).
holds_shell_authority (subagent/mod.rs:15430-15434) = Full || (Verifier && shell != None). On the in-process path a Verifier child of a Scout/Reviewer/Planner lands at ShellPolicy::ReadOnly and keeps Run. Executed at head: parent ∈ {Scout, Reviewer, Planner} → test child: posture_run=true, envelope_denies=false, delegation_refusal=None, catalog offers Run and bash, and Run {"action":"tests"} reached RunTool and ran cargo (exit 101 "could not find Cargo.toml" — execution-stage, i.e. the whole permission stack passed). Same probe at base: [role.posture.denied]. Walls that do hold: shell-less sessions (allow_shell=false → None, refused both trees), Builder/Worker/Custom children, the exact-fleet sentinel, reject_unbounded_verification. So this is bounded (default/selection gates + classifier bash) and the operator ceiling is respected — but it is a child gaining a capability its parent lacks, and the PR edits the very paragraphs that still say the opposite: docs/SUBAGENTS.md:93-96 ("delegating to obtain shell is therefore mechanically useless … the only shell path a read-only parent has"), docs/zh_hans/SUBAGENTS.md:40, and the comment in fleet/exact.rs:1374-1376. Decide one: (a) keep the invariant — carve out only when the parent's live shell was Full (i.e. the ReadOnly came from the role's own narrowing, not from the clamp), or (b) accept the exception and state it explicitly in those three places. I recommend (a); (b) is acceptable if sign-off is explicit.
M6 — NEW — the roster and docs/zh_hans/FLEET.md:124 promise fleet members a bash lane the fleet-executor path never grants.
authority_envelope_for_worker (fleet/executor.rs:238-255) gives the Verifier ToolShellAuthority::None (the shell branch names only Scout/Reviewer/Planner — the in-code comment says so), and enforce_tool_authority (tools/registry.rs:565-588) refuses every bash spelling unless shell == ReadOnly. So a TOML-fleet verifier/test member gets Run and no bash, while FleetRole::Verifier.description() (fleet/role.rs:196) and the new zh FLEET.md sentence ("以及分类器门控的只读取证探针(规范的 bash…)") promise it. For an "honest test role" PR this is the one dishonest sentence. Fix: extend the executor's shell branch to the Verifier (mirrors the registry) or qualify both texts with the path.
PRE-EXISTING escapes in the same gate — recommend fixing here (each is a few lines)
P1 — sed -n <range>p -e w/<abs> f: everything after tokens[2] is unchecked. is_agent_readonly_sed (command_safety.rs:984-1002) reads only -n and the first script; the operand gate treats w/tmp/owned as a nonexistent relative path and skips it. Executed: classifier true, gate Ok. GNU sed (getopt permutation) runs the second script → write (w), read (r), or exec (e) outside the workspace on Linux; BSD sed fails closed (executed on this host). The rewritten doc bullet (:581-582, "no script verbs … can appear in a two-token range script") re-asserts the false claim. Fix: tokens.len() == 3.
P2 — zsh =cmd operands pass the gate and read absolute paths on the shell lane. The =-split (shell.rs:4237-4240) turns =ls into value ls, nonexistent → skipped (:4300); zsh's default EQUALS expands =ls to /bin/ls. Executed: cat =ls | wc -c → 252512 (bytes of /bin/ls) under ReadOnly with zsh. Base leaked via pipes; the PR adds pwd; cat =ls and cd . && cat =ls. Fix: refuse operands whose spelling starts with =.
MINOR
shell.rs:5005tool description still says a pipeline "is the one exception" that runs through the shell; sequences and cd-prefixed commands do now too, and a standalonecd subaffects only that one call — the model is not told.guidance.rs:88-91foreground-stdin sentence over-generalizes: a foregroundcombined_outputrun is on the PTY lane where the close is a no-op (stdin stays a live terminal → still wedges), and the non-Unixinteractive: trueforeground lane inherits the console. Also the sentence has no pin (mutation M4 of the stdin battery stayed green).docs/zh_hans/SUBAGENTS.md:123verifier bullet did not receive the shell-posture sentences the English bullet got.- Pin-strength gaps (mutation survivors): the gate-side pipeline-stage
cdarm (shell.rs:4179-4183) is masked by a disjunctive assertion (cwd.pipeline_stage || operand.outside_workspace) — add a case whose only defect is the piped cd; the cd resolution base is unpinned because every gate test usescwd == roots[0]— add one with a nested cwd; the hidden-entry rule (:4381-4384) is unpinned in the drop direction; the classifier-lane gate's only pin lives insidescout_shell_respects_parent_shell_and_network_ceilings. verifier_child_executes_a_bounded_readonly_bash_probe_for_evidencerunswc -cwithout#[cfg(unix)]while its shell-side sibling is gated; the 3000 ms / 2900 ms foreground EOF margin is 100 ms; the e2e refusal test pins content absence but no refusal tag;[shell.readonly.operand.unresolved](dangling symlink / non-UTF-8 match),odend-to-end, and the Windows[shell.readonly.sequence.unsupported_shell]branch have no tests.- Direct-argv lane validates globs then execs the literal
*— single-command globs (wc -c *.md) have never actually worked; the gate's expansion is inert there. Follow-up: expand in Rust on that lane. - Windows:
set -o pipefail;is still prepended for cmd/PowerShell pipelines (pre-existing; the new cmd;refusal states the principle this violates);Stdio::null()in the sync lane conflicts with the project's ownhooks/executor.rs:2179-2181note that NUL is insufficient on Windows (unpinned there). od -A d/-N 8values are judged as path operands (false refusal if a same-named outside symlink exists);odbypasses the cluster expander its siblings use;-his BSD-only.- Comment at
shell.rs:4630says the PTY lane "carries no pipe handle" — it does (StdinWriter::Pty), the drop is merely inert.piped(shell.rs:2196) now holds non-piped commands — rename.MAX_READONLY_SEQUENCE_SEGMENTSneeds nopub(crate). Refusal copy omits the 32-segment cap. gh-qalias missing. Doc bullet under-reportsfindprimaries (-truncate).
Disposition
Fixes 1 and 3 are sound in mechanism; fix 2's compound/glob surface needs M1–M4 closed and the two pre-existing seam holes (P1, P2) should go with them since this PR is the hardening of exactly that gate. M5 needs a decision (my vote: keep the invariant), M6 a one-branch or one-sentence fix. Re-review after the fix wave; all repro probes are in my worktrees (~/pinvou-worktrees/cw-pr126-{L2,L4,L7,L13,L14}) and can be turned into pins.
|
Fix wave for the fresh-review comment (5673056115): all six MAJORs, both pre-existing escapes, and the valuable minors are fixed and pushed as 9 DCO commits ( MAJORs
Pre-existing escapes (fixed here, as recommended)
Minors ( Verification: fmt clean; strict all-targets clippy clean with only the recorded Not done (deliberately): the walker node/match caps refuse huge-but-legitimate expansions instead of paging them (fail-closed trade, documented in the tag copy); teaching the gate od/filter option-value consumption (false-refusal polish, follow-up); Windows runtime verification of the sync-lane |
asto18089
left a comment
There was a problem hiding this comment.
Fresh review (round 4) — head 5b3eec687 on pinvou3-clean@6e1ee88ee
Reviewed from scratch by 13 independent lanes (stdin lane, classifier grammar, execution seam, operand gate + glob walker, gh network guard, verifier carve-out, docs/copy parity, claim audit, smuggling/hygiene, full gates, design quality, adversarial escape hunt, and an independent blocker-reproduction lane that re-derived the two escapes below without reading the reports that found them). Base pinvou3-clean has not moved since the PR was cut (merge-base = base tip), so no rebase was needed.
Verdict: changes requested — 2 BLOCKER, 2 MAJOR, ~14 MINOR, plus pre-existing items for follow-up. Fix 1 (stdin EOF) and fix 3 (verifier authority) hold completely, mutation-verified. Fix 2's grammar and gate are well-built but the new compound surface still has two deterministic one-call workspace escapes and one unbounded in-host cost, all reproduced end-to-end under ShellPolicy::ReadOnly through BashTool::execute on both /bin/bash and /bin/zsh.
BLOCKER
B1 — cd <regular file>; cat link reads outside the workspace (introduced by this PR).
crates/tui/src/tools/shell.rs:4226-4244 canonicalizes the cd operand and folds the live cwd onto the result with no is_dir() check, so an existing regular file is accepted as the destination. Every later segment then resolves its relative operands under …/notes.txt/…, where nothing exists, and the if joined.exists() skip at shell.rs:4341-4353 passes them unjudged (the glob walk's read_dir failure at 4530-4532 is likewise inert). At runtime cd notes.txt fails with "not a directory", ; and || continue in the original cwd, and the unjudged operand resolves at the workspace root where the planted outside symlink lives. Reproduced with success=true and the outside marker returned for cd notes.txt; cat link, cd notes.txt || cat link, and cd notes.txt; head -c 40 link *; cat link alone is correctly refused, so the fold is the load-bearing bypass. Base refused every ;/|| sequence outright, so this surface is entirely new. The body's "later segments resolve … exactly where the shell resolves them" is false whenever the runtime cd fails. Fix: require resolved.is_dir() in the cd arm (refuse otherwise), and consider making a later-segment operand that fails to exist under a folded cwd refuse rather than skip.
B2 — the operand gate judges only the text after the first =; the shell opens the whole word (root pre-existing; coverage claim and new reach introduced by this PR).
shell.rs:4279-4282: token.split_once('=').map_or(token, |(_, value)| value) — for a non-flag operand this judges the wrong path. With an in-workspace symlink named ax=b.out → outside file: cat ax=b.out is judged as b.out (nonexistent → skipped) and leaks via the direct-argv lane with success=true — on base as well as here. cat x=nope, cat x=y (RHS decoy in-workspace), cat x=y=z, sed -n 1p x=nope all leak the same way. The PR's new glob walker (4336-4338) walks only the RHS pattern, so cat a*=b* | head -c 40 admits (walker pattern b* matches nothing) while bash/zsh pathname-expand the whole word to ax=b.out — the body's "expanded the way the shell expands it … every match judged by its resolved location" fails for any =-bearing token. The round-3 =cmd refusal at 4269-4277 covers only a leading =; the sequence admission newly reaches pwd; cat a*=b* and cd sub; cat x=nope, both refused on base. Fix: split on = only for --prefixed tokens (--opt=path); judge a bare operand as the whole word. The split sits in the exact loop this PR edits, which is why this is blocking rather than a follow-up.
MAJOR
M1 — glob caps are per operand; the aggregate in-host walk is unbounded and untimed (introduced by this PR).
ReadonlyGlobWalk is rebuilt per operand (shell.rs:4392-4411), and the gate's spawn_blocking at 5540 has no timeout, so the round-3 4096/50 000 caps bound one walk only. Measured: cat fan/f* × 1000 over a 2000-dir fan → 19.7 s of host work, admitted; 500 * × 2000 entries → 24.5 s per ~1 KB command; 300 globs end-to-end → 5.8 s wall even though the child then failed exec with E2BIG. A ~20 KB command over a few thousand files is minutes of deterministic host CPU before any watchdog starts. Fix: one shared node/match budget per command (counter threaded through all operands), or a small cap on glob operands per command, and/or a timeout around the gate.
M2 — the "Checked and left alone" disclosure understates a code-execution hole, and its compensating-control claim does not hold (escape pre-existing).
The body says git compounds take the shell lane "without the direct-argv lane's --no-ext-diff --no-textconv --no-show-signature splice (env hardening still applies)". With a repo-local diff.<driver>.textconv plus a .gitattributes diff=driver line, git show HEAD | cat executes the textconv program — a marker file was written outside the workspace — despite GIT_CONFIG_GLOBAL=/dev/null, GIT_EXTERNAL_DIFF, GIT_ATTR_NOSYSTEM and the GIT_CONFIG_COUNT pins (shell.rs:5581-5622); the direct lane's splice is what makes bare git show HEAD safe, and the shell lane has none. Reproduced on base too, so pre-existing — but this PR's sequence admission routes pwd; git show HEAD to the same lane, and a body that otherwise proves every escape end-to-end should not describe an arbitrary-command surface as hardened. Either splice the flags into shell-lane git stages (token-level stage rewrite, same place the gh guard already walks stages), refuse git as a compound stage, or correct the disclosure to say textconv execution remains reachable and defer explicitly. Realistic-threat note: .git/config is not cloned, so the planted config is the same class of workspace-content threat as the planted symlinks the round-1 fix targets.
MINOR
- Copy —
cdis admitted as any whole non-piped segment (cat f; cd sub,pwd; cd sub && ls— the latter pinned admitted), not only "standalone or as a sequence prefix" (command_safety.rs:572-573, body). Behaviour is safe (fold is left-to-right); wording understates. - Copy — the cmd.exe
;refusal is a runtimeShellKinddecision (shell.rs:2164-2171,3888), not "cfg-gated" as the body says twice. - Copy — the model-facing routing description (
shell.rs:5163) says;/&&/||sequences "run through the detected shell" without qualifying that cmd.exe;sequences are refused at the seam; its pipefail half is qualified, the;half is not. - Copy — the refusal copy hardcodes "32" (
tools/subagent/mod.rs:15777) separately fromMAX_READONLY_SEQUENCE_SEGMENTS. - Copy — the Verifier roster/schema/intro strings promise the bounded read-only evidence shell unconditionally, but fleet durable-exec verifiers get
shell: None(fleet/executor.rs:238-263, disclosed only in a code comment). - Body — the pinned-behaviour list ends with "wider EOF margin"; no commit or pin matches (the EOF test's 2900/3000 ms margin dates from
eac3379dd). - Body — the Testing section attributes the same "12283/5 and 12286/2" earlier runs to two disjoint flake memberships.
- Design — the stage
|split is inlined at three judge sites (command_safety.rs:637;shell.rs:3935,4206); the shared sequence splitter is exactly what closed the round-2/3 divergence, and the stage split deserves the same single helper. - Design — the classifier over-admits quoted separators (
cat "x;cat y"classifies true viashell_words' quote-trim fallback,command_safety.rs:1735-1742); safety rests entirely on the seam's fail-closed shlex re-parse (verified). Worth a comment and a pin on that invariant. - Test — the match-cap pin sizes its fixture from the constant it pins (
shell/tests.rs:1551), so mutating the constant hangs the test instead of reddening it. - Test — no pin for the sub-subagent chain (carve-out Verifier parent → Builder/
testchild); the capture point atsubagent/mod.rs:14666is correct by code path only. - Test polish — the symlink fixture is repeated across ~10 gate tests; several refusal pins accept an OR of tags; the glob-bounds test takes ~15 s wall.
- History — three in-range self-correction pairs (
eac3379dd→3a297ded4/e271e10a5,e52aa813c→8d35ba1af,4e06ac862→22fbe0bfb) ande271e10a5is a six-file grab-bag; disclosed, bisectability only. - Design note — the walker implements full shell parity (dot synthesis, symlink follow, dedup); a final-component-only glob contract would be roughly half the code. The measured 10.4 s blowup motivates the choice; the declined trade-off is not recorded anywhere.
Pre-existing, for follow-up (not introduced here)
npm view|show|info(command_safety.rs:1018-1023, byte-identical to base) checks only the subcommand:npm view x --registry http://host/…and--userconfig <file-in-workspace>are admitted, invisible to the gh-only network guard, and every run writes~/.npm. One lane misattributed this as new; it is not — but the compound admission now reaches it from any sequence position.reject_network_reaching_input(tools/subagent/mod.rs:16161-16167) still uses the whole-command gh predicate; a profile-network-denied subagent passes it for glob/compound gh reads, leaving the execute-time guard as the last bash-specific layer. The stage predicate this PR added is a drop-in.- Fleet envelope gh pre-filter (
tools/registry.rs:576) keeps whole-command granularity; harmless only because a deny-all decider is always installed whennetwork_access == Some(false). - Pipeline stage count is uncapped (the 32-cap counts sequence segments only); an admitted relative fifo operand blocks to the 120 s watchdog; the PTY foreground lane still wedges stdin readers (disclosed; guidance correctly excludes it).
docs/SUBAGENTS.md:70,72,218-220(zh mirror) label Scout/Reviewer "bounded verify", butRunis unreachable for them (bounded_inspection); the zhSUBAGENTS.mdsync marker points at an August revision.
What holds
- Fix 1 — root cause verified against base (
spawn_background_sandboxedkeeps the writer; close onlyif stdin_data.is_some(); sync lane inherits). Three mutations each redden exactly the claimed pin (restore conditional close → 3000 ms timeout; dropStdio::null()→TimedOut; drop the background writer →test_write_stdin_streams_output). PTY/interactive lanes byte-identical. Detach-interact message pinned. - Fix 3 — full authority matrix (parent role/shell × child role × nesting path) shows no child holding more process authority than its parent. Forcing
parent_shell_authority = truereddens exactlyclamped_verifier_child_of_read_only_parent_holds_no_run_surface; restoring therole_posture_permitsre-key reddens exactly the two round-2/3 pins while the direct-case pin stays green; the two legacy-Bash walls are independent (each alone still denies). Receipts labelread_only; restore re-derives from the single spawn chokepoint. - Fix 2 structure —
is_parallel_readonly_commandbyte-identical to base and pinned; one shared sequence splitter with six call sites; the round-2 first-program early return is structurally gone;./..synthesis, cd-wildcard refusal, pipeline-stage cd refusal, cwd fold on a real directory, the token-level gh stage predicate, sed third-token pin and the leading-=refusal all re-probed and hold; mutations on the cd-pipeline-stage term, dot synthesis, match cap and the stage predicate each bite. ~110-input grammar matrix: no admitted write path, redirects/&/prefix wrappers/?/{}all refuse. - gh guard — every stage judged;
pwd; gh repo view *andgh pr list --search '*'consult a Deny decider; bare-command behaviour identical to base. - Claims — 40 hold, 1 partial (item 6), 2 fail (the glob "every match judged" sentence per B2; the git "env hardening still applies" sentence per M2). #123 conflict is exactly the one
[shell.readonly.command]literal; #124/#125 merge-tree clean; #124 really refactors the three deferred functions; #123 really fixes the followup-on-interrupted contention. - Gates at head — fmt clean; clippy under the CONTRIBUTING allow list = only the recorded
repo_law.rs:638baseline;command_safety::82/0,tools::shell::158/0/1 ignored (pre-existing),tools::subagent::608/0,fleet::361/0; full libRUST_MIN_STACK=1677721612303/0/13 (338 s); workspace-minus-tui 2311/0. - Smuggling — none; every hunk attributes to a claimed fix; 20/20 commits DCO-signed and conventional; no secrets; VERSION/changelog untouched; en/zh docs parity 1:1.
Lane reports: ~/pinvou-worktrees/reports/cw-pr126-r4/ (L1–L15).
Round-4 disposition — fix wave pushed, head
|
| Finding | Fix | Commit | Pin |
|---|---|---|---|
B1 cd notes.txt; cat link (regular file as cd destination) |
destination must be a directory, [shell.readonly.cwd.not_a_directory] |
9c53f92fc |
readonly_operand_gate_refuses_cd_onto_a_regular_file; e2e cases |
B2 gate judged only the RHS of the first = |
split only --prefixed tokens; bare operands judged whole (globs too) |
9c53f92fc |
readonly_operand_gate_judges_equals_bearing_operands_as_whole_words; e2e |
| M1 glob caps per operand | one ReadonlyGlobBudget per command across segments/stages/operands |
9c53f92fc |
readonly_glob_budget_is_shared_across_operands |
| M2 shell-lane git stages ran repo textconv (body claim "env hardening still applies" wrong) | harden_readonly_shell_git_stages re-emits every log/show/diff stage with the direct lane's splice; one table splice_git_read_hardening; unquotable token refuses |
4c7a9e2ef |
readonly_shell_lane_hardens_git_diff_rendering_stages; e2e readonly_shell_lane_git_show_does_not_run_repository_textconv (fixture proven live first) |
re-probe: gate trimmed spellings, cat ' x ' leaked on both lanes (pre-existing) |
judge operands byte-for-byte as parsed | c28f94102 |
readonly_operand_gate_judges_the_parsed_spelling_without_trimming; e2e |
re-probe: git blame applies textconv by default, no splice on either lane (pre-existing) |
blame → --no-textconv in the shared table |
f5f235419 |
blame cases in both git pins, second raw-shell control |
re-probe: zero-match globs consumed no budget (8000 zz* = 8 s) |
third counter: 500 000 examined entries per command | 10f3b1311 |
readonly_glob_budget_counts_examined_entries |
pre-existing: reject_network_reaching_input + fleet pre-filter whole-command gh predicate |
command_reaches_github_readonly per stage at all three ceilings; whole-command predicate deleted; one readonly_pipeline_stages replaces three inlined `split(' |
')` | 7ecc376f5 |
minors 1–5, 8–10 (cd wording, cmd ; qualification, hardcoded 32, Verifier roster scope, stage-split helper, quoted-separator invariant pin, match-cap fixture literal) |
as listed | a0fc5435f, 7ecc376f5, 9c53f92fc |
readonly_operand_gate_fails_closed_on_quoted_separators |
| minors 6–7, 14 (body: phantom "wider EOF margin", flake narrative, walker trade-off note) | body rewritten; design note in the walker doc | body / 9c53f92fc |
— |
Deferred with disclosure (now in "Checked and left alone"): npm view registry/userconfig redirect — a flag pin alone is false security because a project .npmrc redirects just the same; needs a pinned-registry splice plus a network-decider consult (separate topic). Sub-subagent chain pin (minor 11): invariant holds by code path, dedicated test deferred. Minors 12–13 (fixture repetition, history churn) not acted on.
Gates at 10f3b1311: fmt clean; clippy under the CONTRIBUTING allow list = only the recorded repo_law.rs:638 baseline; command_safety:: 82/0, tools::shell:: 166/0/1 ignored, tools::subagent:: 608/0, fleet:: 361/0; full lib (12311 tests, RUST_MIN_STACK=16777216) three runs 12310/1, 12309/2, 12308/3 — every failure in the known load-timing flake set (forkguard_model_operation_auxiliary_retry_keys, omitted_runtime_models_use_the_active_provider_default, pdf_tests::contract_read_*), each passes alone, none of those modules in the diff; workspace-minus-tui 2311/0.
Merge order: #125 and #114 merged mid-wave; merge-tree against the advanced base is clean, so the branch stays on its base per CONTRIBUTING. Against #123 there is now a second mechanical conflict in the git hardening table (its rev-parse|branch|worktree no-splice additions vs this branch's blame splice) — resolution spelled out in the body; note #123's comment that blame spawns no helper is the misconception round 4 disproved.
Lane reports: ~/pinvou-worktrees/reports/cw-pr126-r4/ (V1-mutations.md, V2-adversarial-reprobe.md).
10f3b13 to
6fe3760
Compare
asto18089
left a comment
There was a problem hiding this comment.
Round 5 — fresh from-scratch review at 6fe376008 (rebased)
Rebase: the branch was rebased from 6e1ee88ee onto the current pinvou3-clean = f1eb10efc (#125, incl. #114). Pure replay — git range-diff shows all 27 commits =; no conflicts; CI on 6fe376008 is fully green (Analyze ×4, DCO, CodeQL, Gitleaks, Type Check, check 19m41s, gate, link); mergeable: MERGEABLE, mergeStateStatus: CLEAN.
Method: 17 independent review lanes starting from zero (prior rounds used only as a list of claims to re-verify, never as evidence): 7 claim-verification lanes (fix 1 / classifier / execution seam / glob walker + budget / Verifier authority / rounds 1–3 / round 4), smuggling + history, engineering quality, shell-semantics + path-containment bug hunt, test quality, docs/i18n, merge topology, and a code-executing red team that drove every candidate end-to-end through LowercaseBashTool under ShellPolicy::ReadOnly with planted outside symlinks, under /bin/zsh, /bin/bash, /bin/sh and /bin/dash separately. Gates re-run locally on the rebased head: cargo fmt clean; command_safety:: 82/0, tools::shell:: 166/0 (1 pre-existing ignore), tools::subagent:: 608/0, fleet:: 361/0; clippy with the CI allow-list flags clean for every file this PR touches (the only local hit is collapsible_if at core/engine/turn_loop.rs:5499, base code the PR does not touch; CI's clippy on this head is green).
Verdict: changes requested (posted as a comment — self-authored PR)
The three fixes are real, root-caused, and complete for the shapes they name; every one of the fifteen round 1–4 escapes is genuinely closed and honestly pinned; there are zero smuggled hunks; the pins are unusually mutation-aware; the engineering is justified rather than reinvented. But the red team confirmed five new workspace-boundary escapes through the tool of exactly the classes this PR declares load-bearing (judged-vs-executed name mismatch, cd-fold-lies-when-cd-fails, walker-vs-shell glob parity) — one of them with a git-shippable fixture that leaks on all four shells. By this PR's own round-1–4 standard they must be fixed and pinned here before merge.
Answers to the four review questions:
- Useful / complete / root-caused — yes for the stated defects (stdin EOF root cause confirmed on base at
shell.rsbase:2732/2758/4363 and base:2338; classifier/seam/authority claims verified line by line; see "Verified" below). Not complete as a boundary: see B1, M1–M4. - Elegant / no reinvention / not over-engineered — yes. The hand walker was checked against
globset,glob,ignore::WalkBuilder,globwalk,walkdir(all present in the lock): none supplies budget + bash./..synthesis + pattern-position-aware cycle dedup + fail-closed per-match judging;readonly_shell_wordvsshlex::try_quoteis a cross-shell requirement shlex cannot meet; the three budget counters are each separately load-bearing and pinned. One real design defect: thecdoperand shape rules live in two places with different code (classifieris_agent_readonly_cdvs the seam cd arm) and have already diverged (M1). - Smuggled changes — none. Every hunk in the 14-file diff maps to a body claim or a named round item; the single unclaimed hunk is the
-truncatedoc-comment bullet (command_safety.rs:584, comment-only, already inMUTATING_PRIMARIESon base). All 27 commits are conventional + DCO-signed, no merges/fixups. Splitting into stdin / grammar+seam / Verifier would be cleaner for revertability but the bundle is one live-session topic with shared files and copy — recommendation, not a requirement. - Bug-free — no. Five confirmed escapes + one availability break below.
Confirmed escapes (red team, literal OUTSIDE_MARKER_R5 in tool output)
B1 — BLOCKER — \\?\ is stripped from the text the gate judges but not from the text the shell runs; the raw ? reaches the shell as a live glob.
command_safety.rs:422 command.replace(r"\\?\", "") runs inside is_agent_readonly_shell_command (:596-597) before the charset gate (:600-608, which refuses ?) and before every shlex parse (shell.rs:4044, :4160, :4368); but the shell lane hands the shell the raw text — harden_readonly_shell_git_stages copies non-git stages byte for byte from the original string (shell.rs:4146-4154). Fixture: ln -s <outside> 'a\?b' (4-char name; a symlink git can ship). Tool input pwd; cat a\\?\b → gate judges pwd; cat ab (ab nonexistent → skipped), shell parses a\?b = literal \ + single-char glob → LEAK on zsh, bash, sh, dash. Same for cat a\\?\b; pwd, cat Cargo.toml | cat a\\?\b (zsh/bash/sh), and word-start pwd; cat \\?\secret with a symlink named \?secret (gate judges secret). The direct lane is safe only by asymmetry (hardened_readonly_argv execs the stripped ["ab"]). The strip is pinned as desirable platform-independent behaviour by agent_readonly_shell_admits_windows_verbatim_paths (command_safety.rs:2375-2388), which is why nothing caught it. Fix: run the charset gate (and classification) on the raw command, or apply normalize_windows_command_paths only for Windows shell kinds; and make the seam judge exactly the bytes the shell receives. Pin red/green on all lanes.
M1 — MAJOR — cd =ls; cat link on zsh. is_agent_readonly_cd (command_safety.rs:1071-1089) checks -/\/*/absolute/~/.. but not =; the seam's cd arm (shell.rs:4330-4362) returns before enforce_readonly_argv_operands, so the round-3 =-prefix refusal at shell.rs:4392-4396 never applies to a cd operand. With a planted directory literally named =ls and link → outside: gate folds cwd onto W/=ls, judges =ls/link (missing → skipped); zsh EQUALS-expands =ls → /bin/ls, cd fails, ;/|| continue at W → LEAK (zsh): zsh:cd:1: not a directory: /bin/ls + marker. bash/sh enter the literal dir (safe). Three independent lanes converged on this. Fix: refuse =-leading cd operands in both layers (or route the cd arm through the same operand-shape function the argv gate uses, so there is one rule). Pin cd =ls; cat link and cd =ls || cat link.
M2 — MAJOR — cd <unsearchable-dir>; cat link on every shell. The round-4 is_dir() check (shell.rs:4356) and canonicalize both succeed for a directory with no search bit (stat needs x only on the ancestors); the shell's chdir needs x on the leaf and fails deterministically, so ;/|| continue in the original cwd while the gate folded every later operand under the destination (exists() miss → skipped, shell.rs:4471-4483). Fixture chmod 000 vault + link → outside: cd vault; cat link → LEAK on zsh, bash, sh, dash; cd vault || cat link leaks on zsh/bash/sh. Also cd vault; cat * (the walker's read_dir fails → zero matches → inert, the shell expands * at W). This falsifies commit c42312a76's "a failing one is a TOCTOU race rather than a one-call escape" — EACCES is deterministic. Precondition needs a non-repo actor (git cannot ship a mode), which is inside the stated threat model (attached roots are agent-writable, shell.rs:4211-4214). Fix: prove the shell's chdir will succeed before folding (std::fs::read_dir(&resolved).is_ok() needs the same x bit; or access(X_OK)), refuse otherwise under the cwd.* family; and judge the || branch in the original cwd (its RHS runs only when cd failed). Pin.
M3 — MAJOR — cat \\.\secret (direct lane) on every shell. The operand-level verbatim-prefix strip shell.rs:4419-4422 (strip_prefix(r"\\?\").or_else(strip_prefix(r"\\.\"))) fires on unix too; \ and . are admitted by the charset, so a POSIX file literally named \\.\secret is judged as secret (missing → skipped) while hardened_readonly_argv execs the real 10-char name → LEAK on zsh, bash, sh, dash; wc -c \\.\secret discloses the outside byte count. The comment at shell.rs:4413-4418 ("On unix hosts such spellings stay fail-closed below") is false. Fix: apply both strips only under cfg!(windows) (or only when the remainder parses as a Windows path) so on unix the \-bearing spelling hits the existing shape refusal. Pin.
M4 — MAJOR — zsh recursive **/. The walker has no ** case (shell.rs:4660-4747): ** reduces to a single-level *, and a non-final component only descends one level. zsh (macOS default, the PR's own evidence platform) expands **/ to every depth. Fixture sub/deep/evil.txt → outside: pwd; cat **/*.txt, grep -n OUTSIDE **/*.txt | head -n 5, head -n 2 **/evil.txt | cat → LEAK (zsh); bash/sh/dash treat ** as * (safe). No test mentions **. Fix (fail-closed, zero cost on bash/dash): refuse any ** component when a later component exists; or implement real recursion under the existing budget. Pin.
Other MAJOR
M5 — availability — set -o pipefail aborts every admitted pipeline and every ||-sequence on dash hosts. readonly_shell_bound_command (shell.rs:3905-3920) prepends the prefix for every non-Windows kind including Custom (= any $SHELL, shell_dispatcher.rs:525-550), and binds whenever the raw text contains | — which || does. /bin/dash -c 'set -o pipefail; …' → set: Illegal option -o pipefail, exit 2, nothing runs (dash exits on a failed special builtin; verified live and through the tool: Failed to execute tool: /bin/dash: 1: set: Illegal option -o pipefail). On Debian/Ubuntu hosts where the resolved shell is dash, every admitted pipeline and || compound fails. Pre-existing on base (the base prefixed unconditionally), but the round-3 bullet says the prefix was "scoped … into pure per-shell-kind decisions" and the comment at shell.rs:3902-3904 claims "every non-Windows kind honors it" — both overstated. Fix: key the prefix on the resolved binary's basename (sh/bash/zsh/ksh yes; dash/ash no) or probe capability once at detection; and don't let || trigger it. (Note: once scoped, dash becomes exposed to the M2 || / M4 pipeline spellings that the abort currently masks — fix those first.)
M6 — Windows cmd.exe only, not executable here — %VAR% operands. The charset (command_safety.rs:602-608) admits %; cmd.exe expands %USERPROFILE%\… before exec; the gate judges the literal as a nonexistent relative path and skips it. Pipelines already reached cmd on base; this PR additionally routes &&-compounds there (;-sequences are refused for cmd, && is not — shell.rs:3892-3898). Analog of the zsh =cmd hole this PR refused outright. readonly_shell_word's bare set also keeps % and ^ (shell.rs:4186-4197) — cmd metacharacters. Fix: refuse %-bearing operands (matching the =cmd precedent) and drop %/^ from the bare set.
MINOR
- Third network ceiling unpinned for compounds —
registry.rs:578(fleet envelope pre-filter) is exercised only with bare gh commands (registry/tests.rs:818, 885); reverting that one line to a whole-command predicate stays green. Addpwd; gh repo view→ deny to the envelope test. The seam (shell/tests.rs:2140) and no-network pre-check (subagent/tests.rs:20015) sites are pinned. - The rewritten
[shell.readonly.command]copy is unpinned beyond the tag (subagent/tests.rs:8226, 8925) — yet the body names it as the #123 hand-union hazard. Pin "at most 32 segments", "must be a directory", and the admitted-families lead-in so a bad union reddens. - Legacy worker record without a stored
runtime_profileregains Run on resume —normalize_worker_record(subagent/mod.rs:1451-1455) rebuildsfor_role(Verifier)= Full shell with no knowledge of the original parent clamp;resume_from_checkpoint_with_policypreserves it (:5946-5949, 6018), soparent_shell_authorityrecomputestruein any Full-Access session for atestchild whose original parent was read-only. Records created after43cfb6bcdare unaffected. Reconstruct clamped, or refuse the carve-out for synthesized profiles. --prefixed token containing=and*: the gate walks only the post-=text (shell.rs:4406-4412) while the shell expands the whole word —git ls-files --exclude-from=zz*vs a planted--exclude-from=zzq(only path-consuming--opt=option in the tables). Refuse or judge both.- Glob recursion depth = pattern component count, no cap (
shell.rs:4682-4702: the..candidate is entered unjudged when non-leaf); a ~20 KB.*/.*/…command recurses ~10 000 frames on a blocking thread — crash/DoS risk, threshold UNVERIFIED. Cap components (32–64). interactrefusal attribution (shell.rs:1381-1384): "its stdin is already at end-of-file (foreground commands run that way…)" also fires for PTY (combined_output) foreground tasks,persist:trueservices (Stdio::null()), and any completed task — the cause is wrong there, the advice still right. Vary by lane or drop the parenthetical.- Verifier gate lane holds an open stdin pipe forever (
verifier.rs:571-581→ piped branch, nothing closes it) — the same disease fix 1 cures for foreground; pre-existing, bounded by the 600 s gate timeout. One-line close after spawn. - Doc drift:
agent_readonly_command_needs_shell_for_cddoc says "leading segment" (command_safety.rs:723-724) but routes any segment-leading cd (:729-737); body +docs/SUBAGENTS.md:100+ zh:40 say cd is "standalone or as a sequence prefix" while the classifier admits it at any sequence position;docs/SUBAGENTS.md:93/ zh:40 "(only bounded-inspection roles keep the classified read-only shell)" is now false (Verifier joined); new[shell.readonly.operand.shape]/.outside_workspacemessages say "Read-only Scout shell" to a Verifier child;shell.rs:4177"characters no shell interprets (*stays bare…)" is self-contradictory. - Body: the Testing paragraph cites "the recorded
cloned_ref_to_slice_refsbaseline atrepo_law.rs:638" — that lint name exists nowhere in the tree at HEAD or base, andrepo_law.rs:638is inside an unrelated test; delete or re-anchor. "Fifteen workspace-boundary gaps … all are fixed" should read "fifteen known gaps". The Merge-order closing sentence ("the branch stays on its original base") is stale after this rebase. - Test coverage asymmetries: every walker/compound/textconv/cd-fold/sync-stdin pin is
#[cfg(unix)]; the WindowsStdio::null()sync lane and theentered_pathsdedup stand-in never execute; cmd.exe;refusal is pinned only as a pure function. No test runs one shape under two shells (the./..and=cmddivergence claims are reasoned, not demonstrated — the gate-level pins are at the right layer, so this is a note, not a defect). Dedup and node-budget regressions turn the cycle pin into a ~600 s slow-red rather than a fast one, and neither is in the 29-mutation list. - Visibility:
is_github_readonly_tokens/is_github_readonly_stagearepub(crate)with no callers outsidecommand_safety.rs. - Pre-existing, noted for follow-up:
#comments (cat #fwedges on stdin on the background lane); hard link inside W to an outside inode reads outside (needs a writer);ls -l danglingdiscloses the outside target name; zshNOMATCHaborts the whole compound opaquely (safe direction; copy could say so).
Merge order — the #123 union is load-bearing, not cosmetic
Re-verified at the new head against the current heads: #123 f8f204b13 conflicts in exactly the two regions the body names (shell.rs splice match; subagent/mod.rs refusal literal); #124 moved to 97a266091 (force-pushed) — still merge-tree clean both directions; #127/#128/#87/#85/#82/#81 share zero files. One addition to the body's prescription: post-merge, #123's classifier admits git rev-parse / branch / worktree list, and those reach splice_git_read_hardening's _ => Err(...) arm (shell.rs:4126-4129) — so a bare git rev-parse --show-toplevel on the direct lane would be refused despite admission, and any compound containing such a stage dies in harden_readonly_shell_git_stages. The arm "status" | "ls-files" | "grep" | "rev-parse" | "branch" | "worktree" => Ok(false) beside the spliced blame arm fixes both lanes (verified correct and complete: #123's only shell.rs change is that match; git branch --edit-description is refused by #123's grammar). #123's tests pin three prose fragments of the refusal copy ("Admitted:", "git branch (bare/flag-only", "git worktree list (flags only)") — the union text must keep them; a ready-made union is in the review artifacts.
Verified true (no need to re-litigate)
Fix 1 root cause and completeness (every spawn path enumerated; PTY/interactive/background/persist lanes behave as stated); is_parallel_readonly_command byte-identical to base; sequence splitter semantics incl. |&, ;;, &> fail-closed; od/sed/cluster grammars tight (sort -o, uniq second operand, tail -f, grep -f/-r, sed -f/--expression/-s/-i, script verbs w/r/e all unrepresentable); gh table is read-only (no api/create/clone/download), no wrapper commands admitted; every round 1–4 escape existed pre-fix and is closed with a non-vacuous pin (29/29 claimed mutations traced to a specific failing assertion by reading); dedup key (dev, ino, remaining-length) is sound (no under-match constructible); budget threaded once per command across segments/stages/operands with all three counters refusing; gate on spawn_blocking; splice order safe (no overriding git flag is admittable; git -c/--git-dir/--exec-path refused); env hardening identical on both lanes; Verifier authority model: order clamp → capture → narrowing verified, round-2 regression closed, both legacy-Bash walls independent, fleet envelope XOR, sub-subagent re-clamps; EN↔zh docs semantically identical; refusal copy enumerates exactly the admitted families; static test counts 82/608/361 exact. Free-form red-team hunt: 53 further shapes (cd sub || cat link, cat sub/../link, cat -- -*, cat ' x ', find . -name '*', 33 segments, cd out; …, gh writes, …) — zero leaks, every refusal with the expected tag.
Artifacts
Lane reports L1–L17 and raw per-shell logs: ~/pinvou-worktrees/reports/cw-pr126-r5/. Reproducible red probes (not part of the PR) left in a scratch worktree at crates/tui/src/tools/shell/probe_r5.rs; run SHELL=/bin/zsh PROBE_R5_EXPECT_SHELL=/bin/zsh cargo test -p codewhale-tui --lib -- probe_r5 (repeat per shell) — probe_r5_c1_*/c3/c4 red on all four shells, c2/c5 red under zsh.
|
Round-5 fix wave is pushed — head is now Confirmed escapes
Other MAJOR
MINORs
Left alone, disclosed in the bodyLegacy worker records without a stored Verification
|
asto18089
left a comment
There was a problem hiding this comment.
Fresh review (round 6, from scratch) — head d4dbe8f55 on pinvou3-clean@f1eb10efc
Verdict: CHANGES_REQUESTED (posted as a comment because author == reviewer). One BLOCKER. Everything else the eleven lanes probed held up: the claims audit found every auditable body claim true (all 39 named tests exist, the five targeted-suite counts are exact, the merge-order/union notes re-verified against the current heads of #123/#124/#127/#128, CI green on this head, DCO 35/35, the round-5 rebase proven a byte-identical replay by range-diff), the red team found no second escape, and the local gates were re-run clean (cargo fmt clean; command_safety 82, tools::shell 173+1 ignored, tools::subagent 608, fleet 361, registry 48 — each matching the body exactly).
Rebase: none needed — merge-base == origin/pinvou3-clean == f1eb10efc, so the branch already rides the latest tip.
BLOCKER — the cwd fold ignores &&/|| short-circuit: a cd the shell never executes still folds, and later operands are then skipped unjudged
readonly_sequence_segments (command_safety.rs:711-737) returns bare segments and discards the connector; the gate's fold loop (tools/shell.rs:4333-4337) folds on every admitted cd segment unconditionally; and the cd arm's own hardening (not_a_directory, unsearchable) reasons only about the cd failing — never about it not running. The round-4/5 closures are exactly the fail cases; the never-runs case is open:
cat nope && cd sub; cat link # cat nope fails → && short-circuits the cd
true || cd sub; cat link # true succeeds → || short-circuits the cd
First-hand reproduction (not traced — executed):
- Gate: driving the real
enforce_readonly_workspace_operands(workspace withsub/and a root symlinklink → <outside>/secret.txt) returns Ok/admitted for both commands — the gate folds ontosub/, judgessub/link(does not exist) at theexists()skip, and admits. - Shell:
bash -c 'cat nope && cd sub; cat link'andsh -c '…'both print the outside file's contents (TOPSECRET-OUTSIDE); the fold-truthful controlcd sub; cat linkcorrectly errors.
So an admitted read-only command reads outside-workspace content — the third member of the "the fold lies about the shell's cwd" family this PR claims closed, and a direct falsification of the round-1 claim that later segments resolve "exactly where the shell resolves them" (true only while the cd runs).
Suggested fix: make the splitter retain each segment's preceding connector and fold only a cd whose predecessor is ; or sequence-start; a cd right of &&/|| either refuses or — strictly safer — judges every later operand under both the pre-cd and post-cd working directories (fail-closed union). Neither the advertised cd dir && wc -c f (cd first) nor the pinned cd sub; cat link drift test is affected. The git-stage hardener, network guard, and router consume the same splitter but do not fold cwd, so they are untouched by the fix.
MINOR
- M1 — cmd.exe
^: judged-vs-run divergence survives on the shell lane (tools/shell.rs:3900-3928,command_safety.rs:620-627).readonly_compound_refused_by_shell_kindrefuses%and;-sequences forCmdbut not^, and the charset admits^: on a cmd host,cat a^b | wc -croutes to the shell lane, cmd consumes the^, andcatopensab.txt— a spelling the operand gate skipped as nonexistent, plantable as an outside symlink. Pre-existing for pipes on base, but this PR extends the same lane (;-sequences, cd compounds) without closing the sibling, and the body's Windows disclosure names only\\?\spellings. Same family: the re-quoter double-quotes a%NAME%-bearing git-stage token, and cmd expands%inside double quotes, soreadonly_shell_word's "reads back as the same single word" invariant is false there. Fix: refuse^-bearing (and keep%) compounds forShellKind::Cmdexactly like the%arm, and extend the disclosure. - M2 — "Parent-side parallel auto-approve classifier is byte-for-byte unchanged" is false at the behavior level. The source is identical, but the round-3 gh
-qadmission edited the sharedreadonly_options_are_allowedgh table (command_safety.rs:1214-1230), whichis_parallel_readonly_command→readonly_tokens_admittedalso serves — so parent auto-approve now admitsgh … -q …reads base refused. Safe direction (-q=--jq), but neither pin covers gh. Scope the claim or pin the delta. - M3 — the interact stdin refusal misstates the combined-output lane (
tools/shell.rs:~1398). "interactive, combined-output, and persistent commands never expose one" — a backgroundcombined_outputtask is routed to the PTY branch (tty = tty || (combined_output && background),shell.rs:5690) and does expose aPtystdin writer while running. The EOF variant likewise attributes closure to "foreground commands", though verifier gate jobs set the same flag. Copy accuracy only. - M4 — the
./..dot-glob attribution to bash is wrong. "bash expands.*to.and.." (commentsshell.rs:4603/4813-4815, test doctests.rs:1515-1518, body round 3) is false on bash ≥4 (probed 5.2: no dot-entries,..*stays literal); it is dash/sh that produce the full cross-product (probed). The synthesis is required for the dash lane, so the code is right and conservative elsewhere — only the attribution needs rewording. - M5 — the no-network blind spot on admitted npm reads is unlisted. A child with
network_tool = falsecan run the admitted, URL-freenpm view left-pad:reject_network_reaching_inputand the envelope pre-filter consult onlycommand_reaches_github_readonly. Pre-existing on base; but the body's "left alone" bullet covers only the--registry/.npmrcredirect angle, not the plain no-network-child angle — add it to the disclosure or to the follow-up. - M6 — one unclaimed hunk (doc-only, verified accurate):
-truncateadded to find's mutating-primary doc list (command_safety.rs:602) — pre-existing refusal the doc omitted. Mention it in the body or drop the line; behavior unchanged.
NIT ledger (no action required to merge; carry as follow-ups)
Unpinned verifier gate-job stdin close (feasible via a cat gate fixture); grandchild carve-out chain (Verifier→test) unpinned; VERIFIER_AGENT_INTRO statically promises selections a parent-clamped Verifier cannot run; reject_unbounded_verification's Unbounded copy overstates when shell=false (pre-existing); genuine multi-hop fold (cd a; cd b; cat f) unpinned — the positive control uses a non-moving second cd; unresolvable cd destinations report outside_workspace though they may be inside-but-unresolvable; the gate's own piped arm admits an empty stage (unreachable today, one-line tighten); documented-but-worth-listing safe refusals (cut -d-, sort -t-, sort -rk2, od attached-value forms); cmd refusal copy advises a shell the model cannot select; zh FLEET doubled bracket );(; UI-facing detach refusal speaks model-ese; generic stdin-refusal parenthetical loose in two states (model-closed tty, detached combined_output); gh …; cat <outside> sequence form untracked by an operand-gate pin (pipeline form is); runtime-reachable pwd; cat 50%off.txt shape of the cmd % refusal unpinned (the pin uses a single command that never reaches the function); PowerShell flattens a bare , in re-quoted git tokens (degrades to git errors, safe); ZDOTDIR/GLOBIGNORE not in the cleared-env set (operator-side only); the fsmonitor/hooks env override rides GIT_CONFIG_COUNT (git ≥ 2.31 — worth one comment sentence); git pathspec magic :(glob)** dodges the pattern-shaped recursive-glob check (git clamps pathspecs to the worktree, currently inert); walker micro-costs (per-entry OsString, double stat per candidate dir, exists()+canonicalize() double resolution).
Claims verification (high level)
Fix 1 (stdin EOF: root cause, foreground close, sync Stdio::null, kept background pipe, PTY untouched, flag + accurate refusal, gate jobs, guidance): VERIFIED, including a systematic stdin-pipe sweep — no leaked pipe anywhere in the touched code. Fix 2 (splitter/cap/cd/od/clusters; per-stage operand gate, cwd fold, glob walker + shared budget + dot synthesis + ** refusal + component cap; =cmd; no-trim/whole-word; quoted-separator fail-closed; Windows-only normalization; git splice incl. blame — --no-ext-diff proven load-bearing by live probe; binary-keyed pipefail — dash Illegal option reproduced; cmd ;/%; gh per-stage predicate at all three consumers with gh api/mutations correctly excluded): VERIFIED — with B1 the one hole. Fix 3 (Verifier joins, parent-grant-keyed single carve-out, other-roles-hold-none, three independent legacy-Bash walls, receipts read_only, docs en/zh parity, legacy runtime_profile base-equivalence): VERIFIED. Performance: all previously measured exploits now bounded two orders of magnitude lower, counters checked before the work they bound, gate on the blocking pool. Smuggling: none beyond M6. The merge-order section re-verified against current heads: #123 conflicts still exactly the two claimed regions (copy union + the Ok(false) arm — both still required, and #123's "blame spawns no configured helper" comment is still present and must not survive), #124 clean at 97a266091, #127/#128 zero shared files.
Disposition
One BLOCKER (short-circuit cwd fold) must be fixed and pinned red/green (mutation-verified like the others) before merge; M1/M2/M3/M4 are small enough to ride the same fix wave; M5/M6 are body/copy-level. Once B1 lands, this is approve-grade: the surrounding engineering — root-causing, red/green + mutation discipline, claims honesty, merge-order preparation — is exemplary.
Round-6 fix wave — head
|
asto18089
left a comment
There was a problem hiding this comment.
Round-7 fresh review — CHANGES_REQUESTED (1 BLOCKER, 1 MAJOR)
Fresh 12-lane review at head e03fd90bc on base f1eb10efc. Rebase status: none needed — the base tip still equals the merge-base, git merge-tree --write-tree origin/pinvou3-clean e03fd90bc is clean, and all CI legs are green at exactly this head. Lanes: stdin-EOF, classifier grammar soundness (~150 adversarial shapes), seam/execution, glob walker against a live bash/sh/dash oracle, the round-6 connector fix re-verification plus a fold-family hunt, network guard, verifier-role authority, a code-executing red team, smuggling/scope (hunk-by-hunk), performance (measured), a claims-vs-reality audit, and an independent mutation re-run of this head's "2/2 red" claim. The two boundary findings below are proven end to end first-hand; the minors were verified in code (and where cheap, live).
BLOCKER B1 — repository-configured filter.<driver>.clean executes under admitted git status / git diff / git blame: outside content crosses the boundary and attacker-chosen commands run
Round 4 closed four siblings of the "repo config turns nominal reads into programs" family (textconv via argv splice, ext-diff, fsmonitor/hooksPath via env pins) and stated the invariant: "Repository config and attributes are attacker-controlled evidence inputs; a nominal git diff/log/show must not turn them into programs." The fifth sibling is missed: content filters. A worktree .gitattributes (* filter=x) plus repo .git/config ([filter "x"] clean = <command>) makes git run <command> — with file content on stdin — whenever a nominal read converts worktree content. The GIT_CONFIG_COUNT block (shell.rs:5973-5983) pins only core.fsmonitor, core.hooksPath, log.showSignature; the round-4 splice (--no-textconv --no-ext-diff) addresses a different mechanism and does not help; git status/ls-files/grep are in the no-splice arm and rely on env pins only.
First-hand reproduction, with the exact child environment from shell.rs:5917-5983 (all GIT_*/GH_* pins, sanitized PATH, GIT_CONFIG_NOSYSTEM, GIT_CONFIG_GLOBAL=/dev/null, GIT_ATTR_NOSYSTEM=1) applied to a fixture repo whose .gitattributes is * filter=redsq and whose repo config defines filter.redsq.clean = sh -c 'cat <outside-file> >&2; touch <outside-marker>; cat':
git status -s(admitted, operand-free) → the outside file's content appears on the command's stderr and the marker file is created (arbitrary command execution).git diff --stat,git diff HEAD→ same; the filter fires once per converted input.git blame note.txt→ fires even with the round-4 splice applied (git blame --no-textconv note.txt: marker read, command executed), and through the shell lane asgit blame note.txt | wc -l.- Compound form
pwd; git status -s→ same on the shell lane.
All commands are in the admitted table and every operand is inside the workspace — the operand gate has no visibility into this mechanism at all. Under SandboxPolicy::ReadOnly the child keeps full-disk read by design, and on hosts where the sandbox backend is unavailable the child is unsandboxed, so the write/exec proof stands.
Fix direction: git has no env/argv switch that disables worktree attribute filters (GIT_ATTR_NOSYSTEM is system-only; core.attributesFile does not shadow the worktree .gitattributes; filter.* cannot be wildcard-pinned). Realistic walls: refuse the git stage when the repository's config/attributes declare any filter (pre-scan, same shape as the unquotable-stage refusal), or pin filter.<discovered-name>.clean/smudge to cat per discovered name via additional GIT_CONFIG_KEY_n entries with fail-closed on unresolvable names. The same pre-scan should note merge.<driver> (unreachable from the admitted subcommands today) and gpg.* (already closed by log.showSignature=false + the splice).
MAJOR M1 — gh stages are exempt from the operand gate's glob expansion, so an admitted compound redirects gh to a planted host past a host-scoped network policy
The operand gate exempts gh stages from all operand judgment (Some("gh") => {} shell.rs:4385, early return shell.rs:4405) on the stated rationale that gh reads consume no local path operands and their host is judged separately. But gh read verbs do accept HOST/OWNER/REPO operands, and the classifier refuses exactly those spellings (github_command_targets_unsupported_host, command_safety.rs:1345) — the seam's exemption means a glob hides the host from that check while the shell expands it at runtime:
pwd | gh repo view */*/*,pwd; gh repo view */*/*,cd fixtures; gh pr list -R */*/*are all admitted (probe-verified against the real classifier).- bash/sh/dash expand
*/*/*against the folded cwd; a plantedexample.invalid/o/rdirectory tree yields exactly that token (live-verified;set -- */*/*). - The guard evaluates only the hardcoded host
api.github.com(shell.rs:3994), so a host-scoped policy (allowgithub.com, default-deny) passes, andGH_HOST=github.compinning does not help: livegh repo view example.invalid/o/runderGH_HOST=github.comattemptshttps://example.invalid/...(verified; no external contact made —.invalid). - Effect: host-scoped network-policy bypass plus attacker-chosen response content injected into a read-only agent's context. Mitigations that keep this at MAJOR rather than BLOCKER: Deny-default policies still refuse (the predicate over-recognizes gh stages); the fleet envelope and the no-network pre-check refuse all predicate-true gh reads regardless of host; no credentials are sent to the foreign host (unauthenticated). Undisclosed and untested by the PR — the gh-exemption pins all use literal operands.
One-line fix: in the operand gate's gh arms, refuse a gh stage whose post-verb tokens contain * (gh reads never legitimately name workspace files) — or expand and run each expansion through github_command_targets_unsupported_host.
Minors
- M2 — docs/SUBAGENTS.md:100-101 garbled by the head commit. The round-6
cd-sentence edit produced "— and a workspace-confined / a workspace-confinedcd <one relative path>in any sequence position…" (duplicated phrase; the zh twin indocs/zh_hans/SUBAGENTS.mdis clean). One-line fix: delete the dangling fragment. - M3 —
cd +Nis the zsh twin of the round-5=lscase.readonly_cd_operand_admitted(command_safety.rs:1157-1175) refuses-/=/~-prefixed cd operands but not+;cd +2; pwdclassifies. On zsh,cd +Nselects the Nth directory-stack entry — empty in a non-interactive-cshell — so the cd fails and a;-sequence continues at the original cwd while the seam folds onto<cwd>/+2(the cd-NEVER-RUNS lie again). Exploitation needs a workspace directory literally named+2and a zsh host (not live-provable here — no zsh). One-line fix: refuse+-prefixed cd operands next to-/=/~. - M4 — combined_output foreground stdin contract. The PR drops the PTY writer at spawn, which removes the base behavior where a detached combined_output command could still be fed via
interact, while combined_output foreground stdin-readers still wedge to the watchdog; the guidance sentence implies combined_output foreground commands have a usable terminal stdin, but theirs is one nobody can write to and that never EOFs. State the combined_output foreground stdin contract in the guidance copy. - M5 — the seam's own hardening flags are classifier-refused if spelled explicitly.
git diff --no-ext-diff HEADrefuses (--no-ext-diffabsent from the switch table) while the bare form is admitted and spliced. Fail-safe, usability only: add--no-ext-diff --no-textconvto thegit diff/git log/git showswitch lists. - M6 — gh
-R ../fooadmits where-R ../../xrefuses. The host heuristic requires ≥2 path parts; a single-suffix relative repo slips it. Not a boundary break (still api.github.com), but arbitrary; treat any..-leading-Rvalue as unsupported. - M7 — od
-j/-Nhuge decimals admitted. Digit-only check admits-j 999…(200 digits); GNU od fails safe at runtime. Parse tou64and bound. - M8 — glob-walker over-approximations (all fail-closed, fine to keep if documented). Trailing-slash globs (
wc */) judge regular files the shell would not expand (shell.rs:4895-4899); quoted stars are walked (cat '*'over-refuses);.*synthesis is unconditional on hosts where./..are never produced; dangling glob matches refuse where the shell's open would merely ENOENT (pinned semantics). Each is safe-direction; a sentence in the copy or docs would make them deliberate. - M9 — the gate's glob walk has no deadline or cancellation. Measured worst case before refusal: ≈3 s warm host CPU (500k-entry cap; 239k entries / 1.48 s on a
/usr-scale zero-match probe), unkillable by the cancel token and not covered by the heavy-command permit (which keys on cargo/rustc only), so a looping agent burns host seconds per dispatch indefinitely. Bounded, hence MINOR; a per-command deadline checked inexamine()/enter()is a two-line hardening. - M10 — no command-length cap. A 10 MB
a;a;a;…allocates ~80 MB of transient slices before the 32-segment refusal (linear, input-bounded; ~8 linear whole-command passes per dispatch otherwise). An upstream length cap would close it.
Nits
fleet/executor.rs:246-249comment rewrite is unclaimed by the body (accurate; one sentence in the body would cover it).- The second legacy-
Bashwall (posture_permits_tool'sbash_must_use_classifier_lane) is pinned only through a Scout fixture; a one-line Verifier-labeled assertion would label it. - No direct agent-grammar classifier pin for
?/[/{spellings (charset refusal; one-look confirmation). interact/cancel/waitdispatch before theShellPolicy::ReadOnlymatch insideBashTool::execute— not exploitable today (the posture gate refuses read-only inputs shaped for those actions), but hoisting the policy match above the action dispatch is a one-line hardening.- The interact EOF message's parenthetical ("foreground commands run that way…") is slightly off when the flag was set by an operator
interact --close-stdin; neutral wording would cover both. - The two-cd pin's doc comment misstates the fold-discard case (the gate refuses earlier at
cd deep's canonicalize; the admit-instead outcome belongs to the partial-fold mutations). Pin still bites; doc only.
What verified clean this round
- Round-6 fix (head commit) is sound. Classifier and seam split with the same connector-carrying splitter and cannot disagree on any command the seam accepts — every mis-split vector (quoted
;, escaped\;,& &,|||, empty segments, trailing backslashes) either refuses at the classifier or is caught by the seam's strict shlex, and shlex 2.0.1 proved POSIX-faithful on every dequoting nuance checked against live shells. No new BLOCKER member of the fold-lies family found (CDPATH is cleared; physical-vs-logical cwd,+Naside, checked; 32-segment fold accumulation correct; UTF-8 bytes cannot fake separators). - stdin-EOF fix verified end to end, including verifier gate jobs, the
stdin_closed_at_eoflifecycle per task state, the write-then-close ordering, kill/cleanup paths, and the background lane's kept pipe; 6/6 named pins green and mutation-re-derived red by this review (shared-target provenance caveat handled). - Classifier grammar: no command form found that both layers accept and that writes, executes, or reads outside (redirects/heredocs/process-substitution refused at the charset;
sort -oand every cluster form refused; sed's script-pinning survives all-e/-f/script-verb smugglings; find's ten mutating primaries refused; exec wrappers absent;#-comment semantics shlex-exact vs live shells). - Seam/execution: routing is a pure host-independent predicate with no judge/route divergence; pipefail keying and the cmd
%/^/;refusals are pure per-shell-kind decisions pinned on every host (dash rationale reproduced live); the git re-quoter is exactly closed over the POSIX double-quote special set (11 nasty tokens byte-identical through bash/dash/sh); no git read reaches the shell lane unhardened (nothing outside the seven admitted subcommands classifies); no alternate entry point bypasses the operand gate under ReadOnly. - Glob walker: a faithful model of bash/sh/dash expansion (hidden-entry, symlink-following with resolution-at-judgment, multi-star ordering, trailing
**,.*/..synthesis at every level on sh/dash, GLOBIGNORE cleared both sides), single choke point on the blocking pool, budget genuinely shared and refuse-don't-truncate; two of the six load-bearing pins mutation-proven to bite in a throwaway harness. - Verifier authority (fix 3): the carve-out is Verifier-only on every path (spawn, nested chain, resume, fleet), the role set is single-sourced, receipts say
read_only, legacyBashstays triple-walled, and the disclosed residual holes (legacy record reconstruction, sub-subagent pin absence) are accurately disclosed. - Network guard architecture: one shared stage splitter, token-level per-stage predicate, all three consumers converted,
is_github_readonly_commanddeleted with zero stale references; the parallel-classifier delta is exactly the gh-qtable addition as claimed; npm disclosure accurate. - Claims audit: 43/43 cited tests exist verbatim, all five suite counts match digit-for-digit (84/175+1/608/361/48; fmt clean), the #123 merge-tree still yields exactly the two claimed conflict regions with the described mechanical resolution, #124 is clean at the cited head, #127/#128 disjoint, 36 commits / zero merges / all DCO-signed, CI green at this exact head.
- Mutation re-run of the head's claims: both claimed mutations redden (seam-tautology → the
[shell.readonly.cwd.conditional]gate pin; classifier-drop → its classifier pin plus the end-to-end pin), and a full no-fold mutation reddens both fold pins, so the fold-coverage claim is real. Two precision notes for the body's Testing section: (1) the seam-tautology mutation reddens only the gate pin — the end-to-end pin stays green because the classifier wall refuses first, so the seam check is observable solely via the gate pin's tag assertion; (2) under the classifier mutation the end-to-end pin reddens for a wording technicality — its boundary marker is the case-sensitivecontains("read-only"), which cannot attribute a refusal to a layer. Neither is a boundary weakness; consider an attribute-on-tag assertion in the end-to-end pin.
Verdict
CHANGES_REQUESTED: the PR's substance is strong and honestly reported — but B1 leaves the read-only lane's core claim ("a nominal read must not become a program") broken by a mechanism the PR's own round-4 invariant already condemns, and M1 leaves an undisclosed host-scoped policy bypass one glob away from an admitted compound. Both fixes are small, local to splice_git_read_hardening/the gh arms of the operand gate, and naturally pin red/green with the PR's existing harness. The remaining minors/nits are follow-up sized.
9ca48b0 to
2fada9a
Compare
Round-7 fix wave landed (
|
asto18089
left a comment
There was a problem hiding this comment.
Round 8 — fresh 14-lane re-review (12 static + test-execution + mutation re-derivation)
Head 2fada9a31 (unchanged since the round-7 fix wave), base f1eb10efc = origin/pinvou3-clean tip = merge-base, so the required rebase onto the latest base is a no-op — the branch is already on top of the latest pinvou3-clean (upstream main is a different line; this fork PR targets pinvou3-clean as in rounds 1–7). CI: both workflow_dispatch runs are queued at the exact head in a runner-congestion window; in the meantime this review re-ran the claimed gates locally (exact counts + full suite + 4/4 mutations, below).
Verdict: CHANGES_REQUESTED. Everything outside the round-7 filter-discovery code re-verified merge-grade: no smuggled changes, fixes 1+3 sound, round-6 fold intact, the gh-glob fix sound, docs/claims accurate, counts exact, mutations bite. But the round-7 BLOCKER fix's central claim — "discovers every filter the repositories a stage can reach declare" — fails five independent ways, each first-hand proven to restore the original defect (attacker-chosen command executing under an admitted git status/git diff, with the fix's own env in place).
BLOCKER-class — the round-7 filter discovery is incomplete (B-1 … B-5)
B-1. Filter names merged case-insensitively; git matches them case-sensitively. shell.rs:4640 (within-config dedupe) and :4484 (cross-repo merge) use eq_ignore_ascii_case, but git config subsections and the attribute→driver lookup are byte-sensitive (re-probed: attr filter=mix runs [filter "mix"], filter=Mix runs [filter "Mix"]). Repo config declaring [filter "Mix"] clean = cat plus [filter "mix"] clean = sh evil.sh (attr * filter=mix) yields a single pin spelled filter.Mix.*; under the tool's exact env the lowercase driver still executes (marker written). Reproduced end-to-end by three independent lanes, single- and cross-repo. The same fold also over-pins across cases: a case-variant declaring only process arms a process pin for the first-seen name and breaks a working clean/smudge filter — the exact failure the only-declared-keys design says it avoids. Fix: dedupe byte-exact (or refuse case-colliding names, mirroring the unspellable-name refusal); pin a two-case red/green fixture.
B-2. Same-line section keys are invisible to the scan. armed_git_filter_names continues after a […] header (shell.rs:4581-4601), discarding the rest of the line; git parses [filter "sl"] clean = <cmd> on one line (probed). Proven end-to-end with the fix's exact env: the repo's clean filter runs (marker written). Blast radius is wider: same-line [include] path = … and [extensions] worktreeconfig = … are likewise read by git but unseen, so the same spelling also defeats the two fail-closed refusals (included-config filter fired in the probe). Fix: parse the post-] remainder through the key parser, or refuse a header line with trailing content.
B-3. Continuation handling diverges from git — silently (no refusal). continuation = raw.trim_end().ends_with('\\') is set before the comment check and without backslash counting (shell.rs:4570-4577). Byte-probed on git 2.43: (a) a comment line ending in \ does NOT continue (comments end at the newline) yet the walker skips the next line — a [filter "evil"] section hidden behind # note \ arms and runs (HIDDEN_EVIL marker, zero pins, no refusal); (b) an unquoted value ending in \\ is git-escaped (value\, line ends) yet the walker swallows the next line (BSLASH_EVIL). Both violate the body's own contract — "anything the scan cannot confidently enumerate … refuses" — without refusing. Fix: continuation only inside unquoted values, only for an odd trailing-backslash count, never after a comment line; or fail closed on any \-terminated comment/value ambiguity.
B-4. A non-UTF-8 byte in the .git gitdir file silently reports "no repository". read_to_string(&candidate).ok()? (shell.rs:4507); git parses the file byte-wise. Proven end-to-end: a linked worktree whose gitdir path contains one 0xE9 byte runs the main repo's clean filter under git status (WORKTREE_EVIL) while the scan returns Ok with no pins and no error — fail-open where this code's convention is refuse. Fix: read bytes (or map the decode error to the filter_config refusal).
B-5. Submodules are never enumerated — the superproject's admitted reads execute the submodule's filters. This lane dispute was settled with a correctly-armed fixture on git 2.43 (an earlier probe's zero-result was fixture poisoning — tee MARKER >/dev/null is not an identity filter; git reads the filter's stdout, so the redirect hashes every file as the empty blob). Setup: submodule's .gitattributes * filter=evil + config armed only in super/.git/modules/sub/config (super config has zero filter lines). Results: superproject git status -s, git status, git diff, git diff --ignore-submodules=none, --porcelain=v2 (plain and --ignore-submodules=none) all execute the submodule's clean filter whenever a submodule file is stat-dirty with preserved size — including mtime-only touch while git reports "nothing to commit, working tree clean" — and git diff --submodule=diff fires even for size-differing dirt (it runs a second in-submodule child). GIT_TRACE/strace show the forked child (cd sub; unset GIT_PREFIX; GIT_DIR=.git git status --porcelain=2) and that it inherits GIT_CONFIG_KEY_n/VALUE_n/COUNT intact — so pins derived from a correct enumeration would suppress it; the single-repo scan never reads .gitmodules or .git/modules/*//config, while the doc comment (shell.rs:4331-4345) claims completeness ("every repository a stage can reach"). Fix: enumerate registered submodules' gitdirs (.gitmodules + .git/modules/*), or refuse stages in repositories with initialized submodules.
Fixture-methodology note for the fixes (bites every red/green pin in this family): git status fires clean only for stat-dirty-with-matching-size (racy) entries — size-differing dirt is decided by stat alone without conversion, and a settled index silences the fire until the next touch; git diff converts the worktree side unconditionally. Control sizes carefully and re-touch before each run.
M-1 (same family, pre-existing on base — fix or disclose). core.worktree in repo config redirects admitted reads outside the workspace: git status --porcelain lists outside entries, git ls-files -o lists them, and with an index entry seeded under a matching name git diff displays the outside file's content (all probed; no hunks touch this on base or head). This is the next member of the same "repo config is attacker-controlled" family rounds 4/7 close, and unlike filters it needs no enumeration — a core.worktree= env pin (same GIT_CONFIG mechanism, outranks repo config, probed) or a config-scan refusal would close it cheaply. If deferred, give it a body bullet with the npm bullet's rigor.
M-2 (load-bearing refusal, undocumented). Command-line -c key=value outranks the entire GIT_CONFIG_* env family — probed for filter pins and the core.hooksPath pin, under three placements including --no-pager -c and -C . -c. Unreachable today only because the classifier refuses every pre-subcommand token except --no-pager/-C (command_safety.rs:1063-1074). That refusal is now load-bearing for the whole round-4+round-7 pin wall and nothing records the dependency; a future admission of -c would silently neutralize both fixes. Add a comment at the pin construction and a pin asserting -c-bearing git stages refuse.
HIGH — tests/CI
- T-1. Dead fixture: the worktree
gitdir:-link/commondirbranches have zero red coverage.readonly_filter_discovery_pins_declared_filters's worktree sub-case (shell/tests.rs:1329-1341) writesrepo/.git-file, but the scan readsrepo/.git— a directory — first, so the link branch (candidate.is_file()) and thecommondirjoin are never executed; deleting both production branches leaves the suite green. Use a realgit worktree addcheckout whose.gitis a file. - T-2. The filter E2E breaks the required Windows CI leg.
readonly_shell_lane_git_status_does_not_run_repository_content_filters(shell/tests.rs:1541) hard-depends onsh(positive control and filter command) with no#[cfg(unix)], unlike its siblingreadonly_shell_lane_git_show_does_not_run_repository_textconv(cfg-gated) and ~62 other sites in the file; windows-latest runscargo nextest run --workspacewith no platform filter. Gate it before merge.
Performance (new code; the rest of the lane found the read-only lane clean — sub-millisecond common case, nothing on the full-shell/background paths)
- P-1. Quadratic filter-name accumulation, cap deferred. The per-config
findover an unboundednamesvec (shell.rs:4638-4646) and the cross-repo merge (:4481-4491) run before the >8-pin refusal (:4420): a crafted 256 KiB config (~13k names) costs ~0.3–0.8 s per admittedgit status, and a 32-nested-repo compound minutes — on the blocking pool, with no child to watchdog. The same "admitted command buys host work" class round 4 closed for globs. Fix: refuse insidearmed_git_filter_namesonce names exceed the cap (behavior-preserving) and/or HashSet dedupe. - P-2. Unbounded reads before size checks: the
.gitlink file (shell.rs:4507),commondir(:4527), and the config itself (:4540-4548 reads fully, checks 256 KiB only after) — a planted multi-GB.git/commondirbuys a full-size transient per scanned repo per command.metadata().len()first, or a capped buffer.
Minors
- The code comment (shell.rs:6439-6440) and body claim an empty
filter.<name>.processoverride is ignored and the attacker command ran — inverted on git 2.43: the empty override takes (silent disarm, rc=0, repo command not run; probed 3/3 + strace). The/usr/bin/falsehelper design is still correct; the stated justification is factually wrong and should not be cited. - git < 2.31 silently ignores the whole
GIT_CONFIG_COUNTblock (comment shell.rs:6415-6419): on old-git hosts the filter wall is absent forstatus/ls-files/grepand env is the only vehicle (-cis refused). Consider a version probe/refusal or a louder disclosure. - Cross-repo merge ORs armed flags across repos sharing a name (shell.rs:4481-4492): a clean-only repo's working filter degrades to a loud handshake failure when a sibling repo declares
processfor the same name (availability, fail-closed direction). - The 256 KiB config scan-cap refusal is untested (deleting the check leaves the suite green).
- Deadline insertion points in
judge/enterare individually unpinned — only removingexamine's check reddens the deadline test; a multi-component pattern would cover the others. - The E2E's
git status -sarm has no raw-shell positive control (its own comment concedes status may decide from stat); only the diff arms are proven live. Add agit status -scontrol with a size-preserving-dirty fixture. - Windows-only: the 64 KiB cap is checked after the allocating normalize (the cap comment's "bounds both layers" is untrue on that cfg), and glob re-entry keys store full path text on non-unix (~100–400 MB worst-case transient at the node cap). Cheap bounds exist for both.
- Pre-existing literal-host residual:
gh repo view myhost/o/r(single-label, no dot/colon) classifies as OWNER/REPO while gh routes HOST/OWNER/REPO — the wildcard-free remainder of the round-7 gh MAJOR; fold into the disclosed npm follow-up. gh pr list --search '*'is now seam-refused everywhere (quoted star over-refusal — safe direction) butcommand_safety.rs:712/:1239still cite it as the canonical admitted shape; stale as end-to-end descriptions.- The full-suite flaky declaration is stale at this head:
remote_control::tests::separate_predispatch_crashes_on_one_run_get_distinct_recovery_turn_idsfailed 3/3 full-suite runs here (plus tworemote_control/runtime_threadssiblings once each, all passing alone); the branch touches neither file (pre-existing load flakiness), but merge-queue CI will churn — extend the body's flaky list or fix.
Nits: the refusal copy interpolates the 32-segment cap but not the new 64 KiB bound; the discovery has_git_stage pre-scan duplicates the shlex parse its loop repeats; the deadline comment's "bounds its wall time" holds only between operations; en FLEET.md has no twin of the changed zh member-permissions paragraph (pre-existing divergence); 20/37 commit subjects exceed the parent repo's 50-char guideline (convention doc absent from this repo — informational).
Verified sound this round (re-derived first-hand; do not re-litigate)
- Smuggling: none. All 37 commits touch exactly the 16 claimed files (4878/233), every hunk in scope, DCO 37/37, #124's topic functions untouched, no secrets/
unsafe/new deps; both disclosed out-of-lane deltas (gh-q/--jqclassifier admission, Windows-only normalization) match the body. Deletions all map to disclosed replacements. - Merge order: re-verified fresh today — #123 conflicts confined to exactly the two described regions with the described content (the mechanical union and the pinned-phrase copy union are correct and load-bearing; #123's "blame spawns no configured helper" comment must indeed not survive), #124 clean at
97a266091, #127/#128 share zero files. - Fixes 1 and 3: every stdin wiring site verified (foreground unconditional close, sync
null(), background keeps the pipe, PTY spawn untouched, gate-job EOF,stdin_closed_at_eoflifecycle and messages); verifier carve-out ordering (clamp → capture → narrow) and Verifier-only-ness re-derived, including a behavior-equivalence argument for the dropped posture term; receipts truthful; en/zh copy consistent; every "checked and left alone" claim matches the code. - Round-6 integrity: the connector-aware fold is intact and all three consumers (classifier, seam, discovery walk) share the one splitter; discovery runs strictly behind the gate in the same closure; prior-round pins intact (one E2E wall assertion widened read-only-lane-wide; the leak guard remains the load-bearing assert — acceptable).
- Round-7 MAJOR (gh glob): both arms refuse post-verb wildcards, keyed per gh stage, literal exemption kept, fail-closed through all network pre-checks; the parent classifier's charset refuses
*independently. - Round-7 minors: 13/13 verified present, correct, honestly claimed (incl.
cd +N, spliced flags admitted, od u64, 64 KiB cap before allocation on unix, policy-before-task-actions, en SUBAGENTS dedup). - Claims audit: all 21 checkable body claims hold, incl. the 15 test-count numbers (89 / 182+1i / 608 / 361 / 48 — executed here, exact), five E2E/pin tests run live and passing, and the 12.3k full suite green at this head modulo the pre-existing flake above.
- Mutations: the four claimed round-7 mutations re-derived and applied in a dedicated worktree — 4/4 red with exact isolation (discovery no-op → E2E + both discovery pins; env-pin drop → E2E only; gh-glob no-op → gate pin only, classifier pin stays green;
-Cignored → the-Cpin only), each reverted to green, worktree clean.
Required before merge
B-1 … B-5 (each with a red/green pin per shape — mind the size/racy fixture gotchas above), T-1, T-2; strongly recommended in the same wave since they are one-to-three-line fixes: P-1, P-2, the M-2 comment+pin; M-1 fix or disclose with follow-up rigor. The mechanism itself (gate-then-discovery ordering, env-pin transport incl. into git's forked submodule children, fail-closed refusals, only-declared-keys rule within one repo, cat//usr/bin/false inert values) is sound — the gaps are enumeration completeness and parser fidelity, not architecture.
Round-8 fix wave — all review findings addressedLanded as one commit ( Per-finding resolution:
Deliberately not changed (from the review, with reasons):
Verification at the new head: targeted PR body updated: round-8 bullet in Review rounds, round-8-head counts, the flaky-set note, and the 8/8 mutation list. |
asto18089
left a comment
There was a problem hiding this comment.
Round 9 — fresh 14-lane re-review (12 static lanes, one test-execution lane, one mutation re-derivation lane)
Verdict: CHANGES_REQUESTED. The round-8 fix wave's pins are real — 8/8 mutations re-derived red with exact isolation, the quoted-form [filter "x"] defense verified end-to-end, fixes 1 and 3 sound, counts exact, hygiene clean. But the hand-rolled config parser/enumerator still diverges from git's actual grammar in seven first-hand-proven ways (three proven end-to-end through the tool), so the headline claim — content filters discovered from every reachable repository and pinned inert — does not hold; plus one chained -C boundary leak, one unsanitized exec in the version gate, and a merge-order section that no longer matches the forced-pushed #123.
Rebase: no-op — origin/pinvou3-clean tip f1eb10efc == merge-base == PR base (re-verified 2026-10-30); MERGEABLE, base/head OIDs unchanged.
BLOCKERs — filter discovery still incomplete (each restores the round-7/8 defect class; all first-hand verified against git 2.43 with probe repos; B1/B2/B3 additionally driven end-to-end through the tool)
-
Dotted section headers
[filter.x]arm a live driver the parser treats as inert. Git accepts[filter.x],[FILTER.EV](dotted-form subsections are lowercased), and[filter.a.b](subsection = everything after the first dot; attributefilter=a.bmatches) as three/four-part keys — probed: with repo config[filter.x] clean = tee markerand.gitattributes*.s filter=x, a committed, size-preserving-dirty file makes plaingit statuswrite the marker. The parser requires a quoted subsection: an unquoted header yields(base = "filter.x", subsection = None), andarmed_git_filter_namesskips it as inert (setting.base != "filter",tools/shell.rs:4951) — zero pins, and the E2E through the tool admits the status and the filter runs (raw-shell control fired, tool fired). The same shape in.gitmodules([submodule.s]) is honored by real git and invisible togitmodules_submodule_paths(setting.base != "submodule",tools/shell.rs:5058) — the round-8 submodule gap restored through a first-class spelling. Every E2E fixture uses only the quoted form, so the suite stays green. Fix nuance: dotted-form subsections are case-folded by git; quoted-form subsections are byte-exact — the round-8 byte-exact dedupe must not be regressed while adding the folded dotted path. -
Two section headers on one physical line: git re-targets the section, the parser pins the decoy.
[filter "decoy"] [filter "evil"]+ next lineclean = …— git's char-based parser starts a new section after the first](probed viagit config --list), socleanarmsfilter "evil". The parser takes the whole remainder as a key line underdecoyand armsfilter "decoy"— with the lane's pins active the E2E still writes the marker: the pins name the wrong driver andevilruns unpinned. The round-8 "same-line key" fix (tools/shell.rs:4898) introduced exactly this remainder-as-key handling; a remainder beginning with[must re-enter header parsing. -
A value followed by a trailing comment that ends in an odd backslash over-continues.
clean = cat ; note \+smudge = evil: git ends the value at the mid-line comment (comments end at the newline; the backslash inside the comment is dead), so BOTH keys are live. The parser computesgit_config_line_continues(raw)on the raw line without stripping the trailing comment (tools/shell.rs:4936), consumessmudge = evilas continuation text, and pins onlysmudge— the realcleanruns unpinned (E2E: marker written with pins active). The round-8 parity fix is correct for full-line comments; this value-then-comment shape was missed. -
Duplicate
core.worktree: git honors the LAST value, the validator checks the FIRST.validated_core_worktreeuses.find(...)(tools/shell.rs:5020). Probed in the absorbed-submodule form the walk scans:worktree = ../../../subfollowed byworktree = /outside→git rev-parse --show-toplevel=/outside, while the validator resolves the first value back to the discovered worktree and admits. A crafted config restores the out-of-workspace read class. Validate the last value. -
Submodule fork-in does not require
.gitmodules; enumeration is.gitmodules-driven only. Git forks its dirty-check child into any index gitlink whose path holds a repository — registration only refines the label. Probed: submodule with hostile clean filter armed in its repo config,.gitmodulesdeleted from the worktree (index still has it) →git status -sfires the filter. The scanner returns early on.gitmodulesNotFound (tools/shell.rs:4614), so nothing is scanned and no pins are armed — unpinned execution under the fix's own env. Enumeration must be index-driven (gitlink entries viagit ls-files --stage/index walk), not registration-driven. -
Config values keep inline comments; git strips them.
path = sub ; notedecodes tosubin git (fork happens) while the scanner joins a nonexistent directory and silently dedup-skips — fail-open mis-scan on the.gitmodulespath case (tools/shell.rs:4915,:5067). The repo-config direction over-arms pins (benign — pin values are fixed literals);core.worktreeover-refuses (fail-closed). -
A UTF-8 BOM before the first header hides that whole section. Git skips the BOM (probed: config readable, filters fire);
str::trimdoes not strip U+FEFF, so the line never matchesstarts_with('[')and — before any header — is skipped, taking the section's keys with it. A BOM-prefixed[filter "evil"](or[submodule "…"]) runs unpinned (probed). Mid-file BOM makes git fatal, so only the leading-BOM shape matters.
MAJORs
-
Chained
git -C A -C escape statusdefeats the operand gate's flat judging. Every relative operand is judged against the same effective cwd (tools/shell.rs:5432), but git applies-Ccumulatively: probed,git -C <ws>/A -C escape status -swithA/escapea symlink to an outside repo lands there and lists outside file names — while the flat form of the same target,git -C A/escape status -s, is refused at[shell.readonly.operand.outside_workspace]. That inconsistency proves the flat judgment is the bug (E2E through the tool: chained form admitted, outside name crossed). Filter conversion stays covered (discovery follows the same chain), so the leak is boundary/names — butgit diffin the chained repo would cross converted outside content. The doc comment attools/shell.rs:4536-4538("every-Coperand was already canonicalized inside the workspace") is materially false for this shape; resolution must chain-Cvalues the way git does (or refuse a-Cwhose accumulated path escapes). -
The git version probe execs under the host env/PATH, not the child's sanitized env.
ensure_git_supports_env_config_pins(tools/shell.rs:4487) runsCommand::new("git").arg("--version")with no env or PATH handling, uncached, whenever pins are armed — while the child lane runs a sanitized PATH and refuses workspace-root binaries under the lane's own invariant that attached roots are untrusted for programs and PATH. If the operator's host PATH carries an agent-writable directory ahead of the system git, a repo that declares any filter plus a plantedgitshim in that directory executes attacker code under the full tool-process env. The probe should resolve git the way the child lane does. (Secondary: it may version-check a different binary than the child runs.) -
The merge-order section is stale against the force-pushed #123 (
2071e94cd). Freshgit merge-treefrom the common base conflicts in three files, not the two described regions: the refusal-copy string and the hardening table are exactly as described (verified #123's side still carries "Not admitted: cd, &&, ;" and the blame-spawns-no-helper comment), plustools/shell/tests.rs— #123's kill-path test block vs this branch's round-5 seam pins at the same anchor (merged-tree tests.rs:7139-7614). "#127/#128 share zero files" is also stale: #128 (7fe08d55a) now sharessubagent/mod.rs+subagent/tests.rs(still auto-merges). #124 (97a266091) and #129 (f83e8cd84) are clean. Anyone unioning per the current text will miss tests.rs; the resolution is mechanical (keep both blocks). The load-bearing note itself verifies against head code (splice_git_read_hardening's_ => Errwould refuse #123's admittedrev-parse/branch/worktree liston both lanes).
Claim-accuracy fixes needed before merge
- The clippy claim is false at this head under the invocation the body quotes.
cargo clippy --workspace --all-targets --all-features --lockedwith ci.yml's allow list exits 101 with exactly twocollapsible_iferrors —core/engine/turn_loop.rs:5499(disclosed, pre-existing, untouched) andtools/shell.rs:4461-4465, which blame shows was introduced by this branch's own commit2fada9a31in a touched file. The fork CI leg passes becausefork-ci.ymlstill allows the lint and omits--all-targets(both invocations verified locally first-hand). Upstream deliberately removed that allowance (ci.yml:353-356: the allowances "were hiding exactly the class of problem that reached the gate"). One mechanical collapse fixes it; the Testing bullet needs correcting either way. - Full-suite bookkeeping: the 12 336/3/13 tui-lib claim reproduced exactly and everything passes alone, but this round's third flake was
tui::clipboard::tests::wl_paste_helper_reads_text_from_stdout— outside the documented rotating set (passes alone in 0.00 s;clipboard.rsuntouched by the diff). Update the body's flaky list.
Structural recommendation (fourth consecutive round of parser divergence)
Round 7 found one gap in this parser, round 8 five, round 9 seven — each a new dialect shape in the same ~200 hand-rolled lines, each restoring arbitrary command execution under an admitted read. The quoted-form defense now stands (mutation re-derivation 8/8 red with exact isolation; the env-pin mechanism verified end-to-end including into git's forked submodule children and nested submodules; the version gate uniform across all seven admitted subcommands; single-value core.worktree validation correct in every discovery form). The convergent fix is to stop hand-parsing and enumerate through git itself: git config --list -z --local in each discovered repository (git's own parser; every dialect shape above collapses into "git said so"; includes become enumerable instead of unscanable) and git config --blob :gitmodules --list -z or git ls-files --stage gitlink enumeration for submodules — keeping the hand parser only as the refusal path when git cannot be invoked. That converts an unbounded dialect chase into one bounded integration.
Verified sound this round (for the record)
Fix 1 holds end-to-end (foreground close with no reachable early-error leak; sync Stdio::null(); background pipe kept; PTY untouched; stdin_closed_at_eof recorded/consumed correctly; gate jobs EOF at spawn — the two named red/green tests pass). Routing, pipefail keying, per-shell-kind refusals, cfg-gated Windows transforms, and child-env construction all verified with live probes (dash pipefail abort, missing /usr/bin/cd, with_env_var override semantics — no hostile GIT_CONFIG_* collision is possible). The cd fold, connector semantics, and cd shape rules are sound with no new bypass (classifier and seam share one splitter and one shape predicate; the residual pre-spawn TOCTOU is honestly documented). The operand gate and glob walker are sound in the boundary direction (every walker match judged by resolved location; over-approximations only refuse; ./.. synthesis matches dash parity). The classifier held against ~80 adversarial spellings with real-binary ground truth (git grep -O/--open-files-in-pager provably exec a pager and are refused; sed w/r/e verbs including glued w/tmp/x and 2wp refused; sort -o/-rno, uniq's second operand, wc --files0-from refused; -c precedence probe confirms the refusal is the only wall for the env-pin family); the parallel classifier is byte-identical to base with exactly the pinned deltas. Fix 3's capture point is exactly where claimed, the carve-out is literally Verifier-typed, delegation cannot launder authority, receipts are truthful, and the copy matches the classifier. Targeted counts 89/185+1i/608/361/48 exact; fmt clean; full suite green modulo the flaky set; 38/38 DCO; exactly the 16 claimed files; the −328 removals all accounted for; docs en/zh consistent; the left-alone list intact (nothing half-fixed, nothing smuggled).
MINORs (fix or disclose)
- The version gate fires only when filter pins are armed;
core.fsmonitor/hooksPathpins ride the same ≥ 2.31 floor (tools/shell.rs:6907) — a filter-free repo declaring fsmonitor on a pre-2.31 git runs the hook under admittedstatus/ls-files/grep, the arms where env pins are the only wall (fsmonitor exec probed live on 2.43; the gate should key on any emittedGIT_CONFIG_KEY_n). - Unpinned walls (this repo's own history: refactors drop walls and only tests notice): the 32-repository cap, the total 8-pin cap, commondir undecodable/oversize refusals,
.gitmodulesoversize, the non-unix glob key bound, the agent grammar's\n/\rcharset refusal, and the cwd fold through an in-workspace symlinkedcddestination (the exact case the canonicalize fold exists for). - Fail-closed availability with no pinned off-ramp: >32 resolvable submodule registrations or >9 declared filters refuse every git evidence command under that repo; a linked submodule worktree (
git worktree addof a submodule) refuses becausecore.worktreeis validated against a worktree git ignores there (probed). .gitpointer targets are trimmed while git consumes the line byte-exactly — a crafted sibling directory named with trailing spaces can get the scanner to pin repo A while git runs repoA(contrived, fail-open).ZDOTDIR=""does not stop zsh sourcing the default~/.zshenv, where operatorEXTENDED_GLOB/DOT_GLOBcould widen admitted literal operands (a^b, dotfiles) past the walker — zsh-only, operator opt-in, unprobed here (no zsh on this host); worth azsh -f-style suppression or a probe on a zsh host.
NITs
sed ;-refusal comment names the charset gate as the wall (real walls: mis-split + seam shlex, command_safety.rs:1134); gh wildcard refusal copy says "the shell would expand" where quoted spellings are refused by the guard's quote-stripped over-approximation (shell.rs:5156); od -An glued spelling refuses while -A n admits; quoted-separator filenames (cat 'a;b') are unreadable through the lane; a dangling symlink inside a glob match refuses the whole probe; Windows-looking POSIX filenames (C:x, any \) refuse on unix; the refusal copy's family enumeration omits the pre-existing codewhale --version family; VERIFIER_AGENT_INTRO's read-only-shell sentence lacks the "for in-session delegations" qualifier its siblings carry; the foreground stdin close is sequenced after the (currently infallible) attach_heavy_permit.
Pre-existing (base behavior, unchanged here — follow-up material)
The parent-side parallel auto-approve classifier's charset admits ( and ), so zsh process substitution cat =(cmd) passes the parent's read-only/auto-approve path on a macOS-zsh host (the agent grammar this PR hardens refuses parens; flagged by two lanes); inline stdin payloads > 64 KiB against a non-reading live child can stall the executor thread and shell-manager lock watchdog-blind; the fleet envelope projects a read-only Bash schema to shell-less verifiers whose every call refuses.
CI state
Every Pinvou fork CI workflow_dispatch on this branch for 24+ hours is zombie-queued with zero jobs (the stale ones return 409 and cannot even be cancelled); a fresh dispatch at this head (run 40992820404) is zombie-queued identically — this needs repo-admin attention (runner policy/Actions health), and until it resolves there is no green required-gate signal at head. Local gates stand in as reported above. MERGEABLE/BLOCKED-on-pending-checks, base f1eb10efc.
Round-9 fix wave — all review findings addressedLanded as one DCO commit ( The seven proven parser/enumerator divergences (BLOCKERs)All first-hand reproduced against git 2.43 in the review, each now mirrored and pinned:
MAJORs
Also fixed (minors/nits from round 9)
New pins: Verification
|
Round-10 fix wave — all review findings addressedLanded as one DCO commit ( The BLOCKER: discovery children executed repository config
The MAJORs
Also fixed (minors/nits from round 10)
Verification
Not taken, deliberately: the PowerShell |
Round-11 fresh review — CHANGES_REQUESTED (2 BLOCKERs, 1 MAJOR)Fresh 11-lane review (9 static/probe lanes, one test-execution lane, one mutation re-derivation lane) at The good news first, because it matters: the round-9/10 fix waves hold up. 6/6 sampled mutations re-derived RED with exactly their own pins and reverted green (discovery fsmonitor argv pins both call sites, sequence splitter, cd cwd-fold, filter-discovery E2E on the leak itself, version-gate comparator, blame splice row — blame is doubly pinned). All targeted suites green (command_safety 94, tools::shell 207, subagent 608, fleet 361, registry 48, workspace libs 2150, doctests green, fmt clean, fork-CI clippy clean with its exact allow list); full tui suite 12364/2/13 with exactly your documented But round 11 found two new first-hand-proven escape classes, both in code this PR introduced or extends, and one false gate claim. BLOCKER 1 — multi-line
|
Round-11 fix wave landed (
|
Round 12 (fresh 12-lane review at
|
5178856 to
db75f65
Compare
|
Round-12 fix wave landed at What was fixedBLOCKER 1 — the strip model behind the round-11 mirror was one byte short. git consumes a BLOCKER 2 — glob MAJORs. (1) The pre-spawn discovery git child is bounded: it opens Minors and nits. The config parser carries the continuation state across a pre-section key's backslash (a following header is swallowed the way git swallows it — probe-pinned — instead of arming a driver git never runs); the parallel classifier's SHRINK with the shared gh host heuristic is disclosed and pinned ( Verification
The round-12 review's structural recommendation stands: the scheduled |
Round 13 — fresh 13-lane review at
|
Second slice of the #126 split: the bounded read-only probe lane. - Sequences (`a; b`, `a && b`, `a || b`) admit iff every segment individually passes the read-only gate, via one shared splitter; `cd <one relative path>` is admitted inside a sequence with its operand shape shared by classifier and execution seam, and routed to the shell lane (it is a builtin); read-only `od` dumps and clustered short text-filter options join the grammar. - Every admitted operand is judged at the execution seam: the cwd folds left-to-right through admitted `cd`s and connector conditionals, glob operands are expanded and judged match by match under shared caps and a wall-clock budget (dot/dot-dot synthesis, `**` refusal, absolute-spelling walks included), and `=cmd`, tilde, verbatim and post-dash wildcard spellings refuse. - Admitted git reads are hardened: repository `filter.<name>` / textconv conversions are pinned to inert helpers via argv config, discovery enumerates registered submodules and linked worktrees byte-exactly like git (multiline/NUL/CRLF/BOM values, padded links, quoted gitlink spellings), under bounded scans, a watchdog, and a git-version floor for env-config pins. `core.worktree` values carrying quotes or backslashes refuse: git escape-decodes quoted values, so the raw spelling validated here is not the string git applies (round-19 review). - gh reads judge every stage (compound and `*`-bearing stages obey the host network policy and the worker envelope); the pipefail prefix keys on the shell kind, and cmd.exe/PowerShell delimiter walls are stated in refusals. - The `[shell.readonly.command]` refusal enumerates admitted families, the every-segment rule, and refused shapes; the worker catalog's read-only gh pre-filter uses the same per-stage predicate. Tests: 114 new pins across classifier, operand gate, glob walker, git discovery/parser differential (incl. the escape-decode worktree wall, mutation-verified 2/2 red), network guards, and lane routing; every listed escape was reproduced end-to-end against the unfixed code before fixing (18 review rounds). Targeted: `tools::shell::` + `command_safety::` + `tools::subagent::` + `tools::registry::` + `tests::verifier::` 1010 passed / 0 failed / 1 ignored (pre-existing); fmt clean; fork-CI clippy flags clean. Split of #126, rebased onto fix/shell-stdin-eof @6337b0e0a (the stdin-review round); full pre-split content preserved on archive/p126-pre-split-8705ccd85. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
e1e2458 to
456babf
Compare
There was a problem hiding this comment.
Round 19 — from-zero re-review of the split slice (8 lanes), rebase + 1×P1 fixed
Re-review conducted from scratch after the split, per the four review goals (usefulness/completeness, elegance, no smuggling, bug-freedom). Rebase: the parent #139 moved to 6337b0e0a mid-review; this slice is rebased onto it at head 3406aeb30 (tree d83ee5291a8d — built and verified locally, landed via the Git Data API during a git-443 outage, and finally force-pushed as plain git once the wall lifted). The conflict resolutions are byte-identical to the independent concurrent rebase (e1e2458dd) — the two rebases disagreed on nothing; my head adds only the round-19 wall + pins.
The one P1 — found and fixed in this wave
validated_core_worktree validated the RAW core.worktree spelling, but git escape-decodes quoted values and strips the quotes: probe-pinned against git 2.43, a config value spelled "w\nx" chdir's to a name containing a real newline (unquoted values keep backslashes literal — the divergence is quote-scoped). With a decoy link named .git/"w\nx" (quote characters are legal filename bytes) canonicalizing the raw spelling back to the discovered worktree, the raw-vs-applied divergence passed validation while the child relocated through the decoded name to an outside symlink — git status -uall lists outside files (raw-shell control reproduced the relocation live). Same raw-vs-applied class the round-17 continuation fix closed, one decode step deeper.
Fix: any \ or " in the raw value refuses; unquoted escape-free spellings keep admitting (admission twin pins this with the decoy links present, so the wall keys on the divergence, not on the links). Pins: readonly_filter_discovery_refuses_an_escaped_core_worktree_value + end-to-end leak-flip readonly_shell_lane_refuses_an_escaped_core_worktree_relocation. Mutations 2/2 red on exactly these pins, restore green.
The other seven lanes — verdicts
- command_safety grammar — sound; fail-closed held in every adversarial shape constructed; 3×P3 nits (doc wording, comment drift, one undisclosed fail-closed parallel-surface shrink).
- Operand gate / seam / routing — CLEAN; judged-vs-executed equivalence verified against dash/bash/zsh ground truth including the
||-fold semantics, env allowlist (noBASH_ENV/ZDOTDIR/GLOBIGNORE/CDPATHreach the child), pipefail keying, cmd/PowerShell walls. P3: compounds assume sh-family semantics forCustom$SHELL(follow-up: key compound admission on the resolved binary like pipefail already does); two disclosed over-refusals. - Git hardening — 40 ground-truth fixture classes probed against the real git binary (gitfile strips, commondir, walk-up predicate decoys ×13, config dialect table, fsmonitor/hooks/textconv/process-filter firing and pinning, version floor); all match except the P1 above. Disclosed residuals re-graded: all P3/acceptable.
- Split fidelity / smuggling — SPLIT FAITHFUL: slice 3's head tree is bit-for-bit the pre-split archive tree; every pre-split test absent from this slice verified present on a sibling slice (dropped coverage: empty); every behavioral hunk of all 6 files maps to a body claim. No smuggled changes.
- Test quality — no fake pins, no weakened coverage (571 deletions in
subagent/tests.rsare pure blank-line churn; 0 base tests deleted inshell/tests.rs); 22 named pins sampled: all exist, all bind to named production entry points, all plausible-red-on-revert. - Executing red team — ~190 probes through the real gates (separator/quoting, cd shapes, walker, program flags incl. every write path, env, caps): 0 new escapes; near-misses all safe-direction and now documented.
- Performance — ~0.2–0.5 ms per admitted probe; walker 4096/50k/500k + 10 s deadline; discovery 32-repo cap + per-child watchdog + 10 s aggregate budget; nothing on the non-read-only path. The Follow-up section's stale "≈5-minute worst case" figure corrected in the body (honest worst ≈10–20 s, fail-closed).
Receipts at 456babf28b
- Targeted:
tools::shell::262/0,command_safety::100/0,tools::subagent::601/0,tools::registry::48/0,tools::verifier::9/0 — 1019 passed / 0 failed / 1 ignored (the ignored exporter pre-existing;tools::verifier::is the current filter spelling — the pre-splittests::verifier::name no longer matches, which is why the first draft of this receipt mis-tallied the split). cargo fmt --all -- --checkclean; fork-CI clippy clean under the exact workflow flags.- Merge-order re-sweep at this head: #123 (
6c40e3bb21) conflicts in FOUR files (addssubagent/tests.rs, #123-side insertions, keep-both resolution); #136 (f989796f1f, moved again) now conflicts inshell.rs+verifier.rs; direct main-tip merge textually clean but stack-order forbids it. Body updated accordingly (also:#P3NUM→ #140, checklist docs item re-scoped to slice 3, round-16 stale CI/zombie note updated — the fork CI ran green onf5aaaafd408).
Known residuals unchanged and disclosed in the body (TOCTOU between the enterability probe and the shell's cd, hardlink invisibility, find -L name disclosure, Windows-lane verbatim divergence unexercised — no Windows host).
CI state at the final head
The rebase landed while the git-443 wall was up (Git Data API route, tree-verified byte-equal), then force-pushed as plain git once it lifted. Two process facts worth recording for the queue:
- On the stacked base (
fix/shell-stdin-eof),Pinvou fork CI'spull_request: branches:[pinvou3-clean]filter cannot fire; the dispatched run (41845678082) showscheck(fmt + clippy + full workspace test job) green on this tree, and the tree-scan Gitleaks leg "no leaks found". - The same dispatch run's Gitleaks "introduced commits" leg failed only because
workflow_dispatchhas nopull_request.base.sha— the workflow's documented fallback scans ALL 7406 reachable commits and trips 8 pre-existing findings (generic-api-key/jwtinnative_memory-era commits,config/persistence.rs, extension-host fixtures — none in files this PR touches). Reproduced locally with the workflow-pinned gitleaks 8.30.1 (sha256-verified): the introduced range6337b0e0a..3406aeb30scans 0 leaks, the full-HEAD scan reproduces exactly the same 8. The Merge Queue's integration run (well-defined base) is the authoritative gate, per convention.
Second slice of the #126 split: the bounded read-only probe lane. - Sequences (`a; b`, `a && b`, `a || b`) admit iff every segment individually passes the read-only gate, via one shared splitter; `cd <one relative path>` is admitted inside a sequence with its operand shape shared by classifier and execution seam, and routed to the shell lane (it is a builtin); read-only `od` dumps and clustered short text-filter options join the grammar. - Every admitted operand is judged at the execution seam: the cwd folds left-to-right through admitted `cd`s and connector conditionals, glob operands are expanded and judged match by match under shared caps and a wall-clock budget (dot/dot-dot synthesis, `**` refusal, absolute-spelling walks included), and `=cmd`, tilde, verbatim and post-dash wildcard spellings refuse. - Admitted git reads are hardened: repository `filter.<name>` / textconv conversions are pinned to inert helpers via argv config, discovery enumerates registered submodules and linked worktrees byte-exactly like git (multiline/NUL/CRLF/BOM values, padded links, quoted gitlink spellings), under bounded scans, a watchdog, and a git-version floor for env-config pins. `core.worktree` values carrying quotes or backslashes refuse: git escape-decodes quoted values, so the raw spelling validated here is not the string git applies (round-19 review). - gh reads judge every stage (compound and `*`-bearing stages obey the host network policy and the worker envelope); the pipefail prefix keys on the shell kind, and cmd.exe/PowerShell delimiter walls are stated in refusals. - The `[shell.readonly.command]` refusal enumerates admitted families, the every-segment rule, and refused shapes; the worker catalog's read-only gh pre-filter uses the same per-stage predicate. Tests: 114 new pins across classifier, operand gate, glob walker, git discovery/parser differential (incl. the escape-decode worktree wall, mutation-verified 2/2 red), network guards, and lane routing; every listed escape was reproduced end-to-end against the unfixed code before fixing (18 review rounds). Targeted: `tools::shell::` + `command_safety::` + `tools::subagent::` + `tools::registry::` + `tools::verifier::` 1019 passed / 0 failed / 1 ignored (the ignored test pre-existing; `tools::verifier::` is the current filter name — the pre-split `tests::verifier::` spelling no longer matches); fmt clean; fork-CI clippy flags clean. Split of #126, rebased onto fix/shell-stdin-eof @6337b0e0a (the stdin-review round); full pre-split content preserved on archive/p126-pre-split-8705ccd85. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
456babf to
945c98a
Compare
Second slice of the #126 split: the bounded read-only probe lane. - Sequences (`a; b`, `a && b`, `a || b`) admit iff every segment individually passes the read-only gate, via one shared splitter; `cd <one relative path>` is admitted inside a sequence with its operand shape shared by classifier and execution seam, and routed to the shell lane (it is a builtin); read-only `od` dumps and clustered short text-filter options join the grammar. - Every admitted operand is judged at the execution seam: the cwd folds left-to-right through admitted `cd`s and connector conditionals, glob operands are expanded and judged match by match under shared caps and a wall-clock budget (dot/dot-dot synthesis, `**` refusal, absolute-spelling walks included), and `=cmd`, tilde, verbatim and post-dash wildcard spellings refuse. - Admitted git reads are hardened: repository `filter.<name>` / textconv conversions are pinned to inert helpers via argv config, discovery enumerates registered submodules and linked worktrees byte-exactly like git (multiline/NUL/CRLF/BOM values, padded links, quoted gitlink spellings), under bounded scans, a watchdog, and a git-version floor for env-config pins. `core.worktree` values carrying quotes or backslashes refuse: git escape-decodes quoted values, so the raw spelling validated here is not the string git applies (round-19 review). - gh reads judge every stage (compound and `*`-bearing stages obey the host network policy and the worker envelope); the pipefail prefix keys on the shell kind, and cmd.exe/PowerShell delimiter walls are stated in refusals. - The `[shell.readonly.command]` refusal enumerates admitted families, the every-segment rule, and refused shapes; the worker catalog's read-only gh pre-filter uses the same per-stage predicate. Tests: 114 new pins across classifier, operand gate, glob walker, git discovery/parser differential (incl. the escape-decode worktree wall, mutation-verified 2/2 red), network guards, and lane routing; every listed escape was reproduced end-to-end against the unfixed code before fixing (18 review rounds). Targeted: `tools::shell::` + `command_safety::` + `tools::subagent::` + `tools::registry::` + `tools::verifier::` 1019 passed / 0 failed / 1 ignored (the ignored test pre-existing; `tools::verifier::` is the current filter name — the pre-split `tests::verifier::` spelling no longer matches); fmt clean; fork-CI clippy flags clean. Split of #126, rebased onto fix/shell-stdin-eof @6337b0e0a (the stdin-review round); full pre-split content preserved on archive/p126-pre-split-8705ccd85. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
945c98a to
3406aeb
Compare
Third slice of the #126 split: the `test` role's honest surface. - The roster description, the type schema, VERIFIER_AGENT_INTRO, and both SUBAGENTS role tables state the real contract: built-in verification gates with test-selection arguments, plus the classifier-bounded read-only shell for evidence gathering; custom commands stay refused. - The Verifier joins `role_requires_read_only_shell` and `allows_bounded_readonly_bash` (the latter now reads that one predicate), gaining the same bounded probe lane as the inspection roles; the carve-out is Verifier-only and keys on a recorded parent-shell grant, so a read-only-parent's child never regains authority through the role name alone (chain pins cover the sub-subagent path). - Shell authority (`holds_shell_authority`) keys on the held policy plus the grant sentinel; fleet receipts report the verifier's effective shell as `read_only`; the worker catalog omits bash from envelopes without read-only shell authority and pins the refusal copy's load-bearing claims. Tests: verifier contract/authority/carve-out pins incl. the sub-subagent chain, catalog and receipts labels; the deleted no-shell verifier catalog test is replaced by the probe-lane variant. Targeted: `tools::subagent::` + `tools::registry::` + `fleet::` suites green; tree of this commit equals the reviewed #126 head tree byte-exact (8705ccd). Split of #126 (stacks on fix/live-test-findings-subagent-shell after fix/shell-stdin-eof); full content preserved on archive/p126-pre-split-8705ccd85. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
Second slice of the #126 split: the bounded read-only probe lane. - Sequences (`a; b`, `a && b`, `a || b`) admit iff every segment individually passes the read-only gate, via one shared splitter; `cd <one relative path>` is admitted inside a sequence with its operand shape shared by classifier and execution seam, and routed to the shell lane (it is a builtin); read-only `od` dumps and clustered short text-filter options join the grammar. - Every admitted operand is judged at the execution seam: the cwd folds left-to-right through admitted `cd`s and connector conditionals, glob operands are expanded and judged match by match under shared caps and a wall-clock budget (dot/dot-dot synthesis, `**` refusal, absolute-spelling walks included), and `=cmd`, tilde, verbatim and post-dash wildcard spellings refuse. - Admitted git reads are hardened: repository `filter.<name>` / textconv conversions are pinned to inert helpers via argv config, discovery enumerates registered submodules and linked worktrees byte-exactly like git (multiline/NUL/CRLF/BOM values, padded links, quoted gitlink spellings), under bounded scans, a watchdog, and a git-version floor for env-config pins. `core.worktree` values carrying quotes or backslashes refuse: git escape-decodes quoted values, so the raw spelling validated here is not the string git applies (round-19 review). - gh reads judge every stage (compound and `*`-bearing stages obey the host network policy and the worker envelope); the pipefail prefix keys on the shell kind, and cmd.exe/PowerShell delimiter walls are stated in refusals. - The `[shell.readonly.command]` refusal enumerates admitted families, the every-segment rule, and refused shapes; the worker catalog's read-only gh pre-filter uses the same per-stage predicate. Tests: 114 new pins across classifier, operand gate, glob walker, git discovery/parser differential (incl. the escape-decode worktree wall, mutation-verified 2/2 red), network guards, and lane routing; every listed escape was reproduced end-to-end against the unfixed code before fixing (18 review rounds). Targeted: `tools::shell::` + `command_safety::` + `tools::subagent::` + `tools::registry::` + `tools::verifier::` 1019 passed / 0 failed / 1 ignored (the ignored test pre-existing; `tools::verifier::` is the current filter name — the pre-split `tests::verifier::` spelling no longer matches); fmt clean; fork-CI clippy flags clean. Split of #126, rebased onto fix/shell-stdin-eof @6337b0e0a (the stdin-review round); full pre-split content preserved on archive/p126-pre-split-8705ccd85. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
3406aeb to
f063d51
Compare
Third slice of the #126 split: the `test` role's honest surface. - The roster description, the type schema, VERIFIER_AGENT_INTRO, and both SUBAGENTS role tables state the real contract: built-in verification gates with test-selection arguments, plus the classifier-bounded read-only shell for evidence gathering; custom commands stay refused. - The Verifier joins `role_requires_read_only_shell` and `allows_bounded_readonly_bash` (the latter now reads that one predicate), gaining the same bounded probe lane as the inspection roles; the carve-out is Verifier-only and keys on a recorded parent-shell grant, so a read-only-parent's child never regains authority through the role name alone (chain pins cover the sub-subagent path). - Shell authority (`holds_shell_authority`) keys on the held policy plus the grant sentinel; fleet receipts report the verifier's effective shell as `read_only`; the worker catalog omits bash from envelopes without read-only shell authority and pins the refusal copy's load-bearing claims. Tests: verifier contract/authority/carve-out pins incl. the sub-subagent chain, catalog and receipts labels; the deleted no-shell verifier catalog test is replaced by the probe-lane variant. Targeted: `tools::subagent::` + `tools::registry::` + `fleet::` suites green; tree of this commit equals the reviewed #126 head tree byte-exact (8705ccd). Split of #126 (stacks on fix/live-test-findings-subagent-shell after fix/shell-stdin-eof); full content preserved on archive/p126-pre-split-8705ccd85. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
|
Re-aligned again onto #139's wave 2 (a2ec04f — the round-1 Option B decision: explicit interact close_stdin now writes \n+VEOF through the PTY dup): this branch is now f063d51. Note there was a push race between two sessions (your 3406aeb was based on wave 1); this rebase preserved your conflict resolutions and your 2 new tests verbatim — |
First slice of the #126 split: the foreground/sync stdin lifecycle. - Foreground and sync shell commands without a stdin payload no longer hold the write end open for the process lifetime: foreground closes it right after spawn, the sync lane spawns with `Stdio::null()`, so a stdin-reading probe (`cat`, a bare `$SHELL`) gets end-of-file and exits instead of idling until the 120 s watchdog. - A foreground command later detached to the background reports its closed stdin on `interact` instead of an opaque refusal. - stdin writes are bounded everywhere (64 KiB inline cap, owned-writer fallback for Windows/ohos, 5 s deadline with a stall flag), a PTY writer no longer injects EOF on drop (dup'd master fd), and the verifier's background gate jobs are handed EOF at spawn instead of idling on the held-open pipe. - bash guidance and the schema describe the foreground stdin contract; the model-facing description carries the same sentence. Tests: 11 new pins (foreground EOF, sync-lane EOF, detached-foreground interact receipt, PTY interact surface, stalled-tty bound, interact/ inline caps, bounded owned write) + the verifier gate-job EOF pin. Targeted: `tools::shell::` 152 passed / 0 failed / 1 ignored (pre-existing), `tests::verifier::` green; fmt clean; fork-CI clippy flags clean; upstream all-targets clippy reports only the pre-existing core/engine/turn_loop.rs:5499 collapsible_if (base code). Split of #126; full content preserved on archive/p126-pre-split-8705ccd85. Stacks: none (base f1eb10e). Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
a2ec04f to
9148c17
Compare
Second slice of the #126 split: the bounded read-only probe lane. - Sequences (`a; b`, `a && b`, `a || b`) admit iff every segment individually passes the read-only gate, via one shared splitter; `cd <one relative path>` is admitted inside a sequence with its operand shape shared by classifier and execution seam, and routed to the shell lane (it is a builtin); read-only `od` dumps and clustered short text-filter options join the grammar. - Every admitted operand is judged at the execution seam: the cwd folds left-to-right through admitted `cd`s and connector conditionals, glob operands are expanded and judged match by match under shared caps and a wall-clock budget (dot/dot-dot synthesis, `**` refusal, absolute-spelling walks included), and `=cmd`, tilde, verbatim and post-dash wildcard spellings refuse. - Admitted git reads are hardened: repository `filter.<name>` / textconv conversions are pinned to inert helpers via argv config, discovery enumerates registered submodules and linked worktrees byte-exactly like git (multiline/NUL/CRLF/BOM values, padded links, quoted gitlink spellings), under bounded scans, a watchdog, and a git-version floor for env-config pins. `core.worktree` values carrying quotes or backslashes refuse: git escape-decodes quoted values, so the raw spelling validated here is not the string git applies (round-19 review). - gh reads judge every stage (compound and `*`-bearing stages obey the host network policy and the worker envelope); the pipefail prefix keys on the shell kind, and cmd.exe/PowerShell delimiter walls are stated in refusals. - The `[shell.readonly.command]` refusal enumerates admitted families, the every-segment rule, and refused shapes; the worker catalog's read-only gh pre-filter uses the same per-stage predicate. Tests: 114 new pins across classifier, operand gate, glob walker, git discovery/parser differential (incl. the escape-decode worktree wall, mutation-verified 2/2 red), network guards, and lane routing; every listed escape was reproduced end-to-end against the unfixed code before fixing (18 review rounds). Targeted: `tools::shell::` + `command_safety::` + `tools::subagent::` + `tools::registry::` + `tools::verifier::` 1019 passed / 0 failed / 1 ignored (the ignored test pre-existing; `tools::verifier::` is the current filter name — the pre-split `tests::verifier::` spelling no longer matches); fmt clean; fork-CI clippy flags clean. Split of #126, rebased onto fix/shell-stdin-eof @6337b0e0a (the stdin-review round); full pre-split content preserved on archive/p126-pre-split-8705ccd85. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
f063d51 to
d6d4143
Compare
Third slice of the #126 split: the `test` role's honest surface. - The roster description, the type schema, VERIFIER_AGENT_INTRO, and both SUBAGENTS role tables state the real contract: built-in verification gates with test-selection arguments, plus the classifier-bounded read-only shell for evidence gathering; custom commands stay refused. - The Verifier joins `role_requires_read_only_shell` and `allows_bounded_readonly_bash` (the latter now reads that one predicate), gaining the same bounded probe lane as the inspection roles; the carve-out is Verifier-only and keys on a recorded parent-shell grant, so a read-only-parent's child never regains authority through the role name alone (chain pins cover the sub-subagent path). - Shell authority (`holds_shell_authority`) keys on the held policy plus the grant sentinel; fleet receipts report the verifier's effective shell as `read_only`; the worker catalog omits bash from envelopes without read-only shell authority and pins the refusal copy's load-bearing claims. Tests: verifier contract/authority/carve-out pins incl. the sub-subagent chain, catalog and receipts labels; the deleted no-shell verifier catalog test is replaced by the probe-lane variant. Targeted: `tools::subagent::` + `tools::registry::` + `fleet::` suites green; tree of this commit equals the reviewed #126 head tree byte-exact (8705ccd). Split of #126 (stacks on fix/live-test-findings-subagent-shell after fix/shell-stdin-eof); full content preserved on archive/p126-pre-split-8705ccd85. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
|
Re-aligned onto the rebased #139 (9148c17, now on main 449edbf after #123/#124 landed): this branch is d6d4143. Your resolutions are preserved verbatim; the #123 collisions were resolved to your state (your hardener call site + your refusal copy), with two semantic reconciliations where main's newer content demanded it: (1) the git-read hardener now also passes rev-parse/branch/worktree unspliced (main's #123/#124 widening — main's new orientation probes in shell/tests.rs pin it), while blame KEEPS its --no-textconv splice (this slice's empirical textconv pin); (2) main's new refusal-enumeration subagent test was not carried into this branch (its expectations describe main's grammar state) — that reconciliation is #126 re-sweep material. Gates at d6d4143: |
asto18089
left a comment
There was a problem hiding this comment.
Round 20 — from-zero re-review of the rebased slice (9 lanes), rebase verified, 2×P2 fixed in-wave
Re-review conducted from scratch per the four review goals (usefulness/completeness, elegance, no smuggling, bug-freedom), with one complication handled mid-flight: the branch was rewritten under us by a concurrent session — the stack was rebased onto pinvou3-clean with #123/#124 merged (slice 1 re-split into four commits, #139 now 9148c1785f; slice 2 re-headed d6d41439d8), and the round-19-era head 3406aeb30 I started from is no longer the PR head. Both states were reviewed; the rebase was itself treated as new review surface.
Rebase fidelity (the new surface)
A dedicated lane compared every slice-2-added function between 3406aeb30 (tree d83ee5291a8d) and d6d41439d8 (tree 6dc3c39c10): 239 total — 238 byte-identical; the one delta is the splice_git_read_hardening arm extension (#123's classifier admits git rev-parse/branch --list/worktree list, so the no-splice arm must list them or every such probe dies fail-closed at the seam), pinned both directions by readonly_argv_executes_the_git_orientation_probes_without_a_shell. The #123/#124 hunks land as insertion-only keep-both blocks exactly as the body's merge-order section predicted; hunk-attribution ledger: 135 hunks, zero slice-2 behavior drift, registry.rs blob-identical. All eight round-19-critical elements verified byte-intact at the new head (escape-decode wall + both its pins, refusal copy with {max_segments}, per-stage gh predicate and all three consumers, spawn walls, caps, per-kind walls, splice table, sequence splitter). One rebase artifact found and fixed: a duplicated bounded-stdin doc comment landed above the cd-operand test; the copy is removed in the wave.
Findings and dispositions
- P2 (fixed in-wave) —
BASHOPTS/SHELLOPTSwere not inreadonly_spawn_env_overrides; bash honors an inheritedBASHOPTS=dotglobat startup (probed on this host's bash 5.2), defeating the round-6GLOBIGNORE=""wall and widening*over dotfiles. Operator-env precondition (same class as the existing ZDOTDIR/GLOBIGNORE pins), so not attacker-reachable through the grammar — but the pin family exists precisely for this class. Fixed in8f98b3e895: both keys cleared, table pin widened toreadonly_spawn_env_overrides_carry_the_six_fixed_keys. The round-15 follow-up note claiming "the child-env allowlist already drops both" was wrong and is corrected in the body. - P2 (fixed in-wave) — the three PR-introduced mode-000 root-runner skip notes used
eprintln!, tripping the crate-level#![deny(clippy::print_stderr)]under--all-targetsclippy (the repo's own ci.yml gate form; the fork-CI lib-only form did not compile tests, which is why earlier receipts stayed green). Fixed: one shared#[allow]ed skip-note helper; the strict clippy inventory now shows zero warnings pointing into slice-2's six files. Remaining warnings are three pre-existingcollapsible_if(turn_loop.rs:5499,verifier.rs:587, andsubagent/mod.rs:8036carried in with #123, in #123's own hunks) plus CI-allowedtoo_many_argumentsin untouched files. - P2 → P3 (downgraded after repro) — the registry-layer per-stage gh predicate swap (
enforce_tool_authority) has no test that reddens if broken; repro confirmed the mutation-kill gap, but the layer is fail-closed one branch earlier (compounds refuse at the arbitrary-execution wall before the gh branch is reached), and the predicate's load-bearing consumers (the seam's network guard and the subagent pre-check) carry this PR's own per-stage pins (readonly_github_shell_calls_obey_the_host_network_policy_before_spawn,a_network_denied_child_cannot_address_a_remote_location_through_any_tool). Coverage note, not a hole; a small registry closure pin remains nice-to-have. - P3 ledger (no action required this round, disclosed in the body) — discovery aggregate wall-clock is
deadline + one child watchdog(~20 s honest worst, comment wording);.gitwalk-up has no per-level deadline (single-syscall residual, same disclosed class);objects/info/alternatescontent-crossing residual deserves one disclosure sentence; two doc-comment inaccuracies (GIT_DISCOVERY_ACROSS_FILESYSTEMmechanism claim, padded-gitfile probe narrative); refusal copy promisestr/uniqoperand shapes their caps refuse; quoted-separator over-recognition can mislabel a refusal reason for no-network children (outcome-safe, seam fails closed); copy-only pins are brittle to copy edits; subagent/tests.rs 571-blank-line churn. - Body corrections (applied in this update, 19 edits) — Summary enumeration completed through rounds 14–19 and scoped to this slice; round-3 authority bullet and the sub-subagent-chain bullet re-anchored to #140; Split row 3 + Merge order corrected (#140's branch is stale on the superseded round-18-era stack and must be rebased onto this head before it merges — merging it as-is would resurrect the pre-round-19 slice-2 state and drop the escape-decode wall); clippy claim updated (three pre-existing
collapsible_ifat the new head); round-16 zombie-CI sentence qualified as historical; receipt deltas annotated; phantomreadonly_sequence_refused_by_shell_kindname marked; the spliced round-15/16 follow-up sentence repaired; theBASHOPTS/SHELLOPTSitem moved to implemented.
Red team and performance (fresh, on the new head's content)
- Executing red team: 187 tool-path probes, 0 escapes, 0 unexpected false refusals, under bash and dash children, against hostile forests (outside-symlink lattices, cd drift chains, glob farms, textconv/blame repos, include/includeIf, gitfile pads/CRLF/NUL, commondir chains, quoted-escape
core.worktree) with raw-shell positive controls proving every fixture fires. The round-19 P1 closure re-verified live at both the pin and E2E levels, with the unquoted admission twin still admitting. zsh unavailable on this host — zsh-specific shapes probed at the shell-agnostic gate level. - Performance: all bounds re-measured — worst admitted pathological glob compound 10.4 ms (round-4's 25 s blowup stays dead; cycle dedup holds at depth 30), 1M-entry single dir refuses at the entry cap in ~140 ms, worst discovery 18 ms (240 KiB config) with the 300 KiB read bound refusing before parse, classifier linear at the 61 KiB/32-segment bound (2.6 ms), common-path pre-spawn delta vs parent <0.1 ms p50. Timing pins all ≥3x margin; two pre-existing <2x windows noted for a follow-up.
Gates (local reproduction at d6d41439d8 + 8f98b3e895, tree c7c17bfff)
tools::shell::270/0/1,tools::subagent::629/0,command_safety::100/0,tools::registry::48/0,tools::verifier::9/0 — 1056 passed / 0 failed / 1 ignored, receipt-exact before and after the wave.cargo fmt --all -- --checkclean; strict--all-targets --all-featuresclippy inventory clean for slice-2's six files (see ledger above).- gitleaks 8.30.1: the introduced-range scan is clean; the full-worktree scan's 8 findings are all pre-existing main content outside every introduced hunk (verified hunk-by-hunk, including the
lib.rs:15034fakesk-live-…marker inside a redaction test). As before, the fork-CI dispatch's introduced-scan leg needs a well-defined base — the Merge Queue integration run is the authoritative gate.
Verdict
Content: all four review goals hold — the PR does exactly what it claims (compound read-only probes admitted with the operand gate, glob walker, git hardening and discovery walls all enforced and pinned), elegantly (shared splitters, one budget, one splice table, no reinvention), with zero smuggling (the rebase's 36-file delta is fully attributed #123/#124 content), and no known functional or performance defect. No P0/P1 open in this slice. The one open action is on the sibling slice: rebase #140 onto this head (8f98b3e895 or later) before it merges, then the stack can proceed to the Merge Queue. This round's fixes are landed as 8f98b3e895; the body now matches reality at that head.
Third slice of the #126 split: the `test` role's honest surface. - The roster description, the type schema, VERIFIER_AGENT_INTRO, and both SUBAGENTS role tables state the real contract: built-in verification gates with test-selection arguments, plus the classifier-bounded read-only shell for evidence gathering; custom commands stay refused. - The Verifier joins `role_requires_read_only_shell` and `allows_bounded_readonly_bash` (the latter now reads that one predicate), gaining the same bounded probe lane as the inspection roles; the carve-out is Verifier-only and keys on a recorded parent-shell grant, so a read-only-parent's child never regains authority through the role name alone (chain pins cover the sub-subagent path). - Shell authority (`holds_shell_authority`) keys on the held policy plus the grant sentinel; fleet receipts report the verifier's effective shell as `read_only`; the worker catalog omits bash from envelopes without read-only shell authority and pins the refusal copy's load-bearing claims. Tests: verifier contract/authority/carve-out pins incl. the sub-subagent chain, catalog and receipts labels; the deleted no-shell verifier catalog test is replaced by the probe-lane variant. Targeted: `tools::subagent::` + `tools::registry::` + `fleet::` suites green; tree of this commit equals the reviewed #126 head tree byte-exact (8705ccd). Split of #126 (stacks on fix/live-test-findings-subagent-shell after fix/shell-stdin-eof); full content preserved on archive/p126-pre-split-8705ccd85. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
Second slice of the #126 split: the bounded read-only probe lane. - Sequences (`a; b`, `a && b`, `a || b`) admit iff every segment individually passes the read-only gate, via one shared splitter; `cd <one relative path>` is admitted inside a sequence with its operand shape shared by classifier and execution seam, and routed to the shell lane (it is a builtin); read-only `od` dumps and clustered short text-filter options join the grammar. - Every admitted operand is judged at the execution seam: the cwd folds left-to-right through admitted `cd`s and connector conditionals, glob operands are expanded and judged match by match under shared caps and a wall-clock budget (dot/dot-dot synthesis, `**` refusal, absolute-spelling walks included), and `=cmd`, tilde, verbatim and post-dash wildcard spellings refuse. - Admitted git reads are hardened: repository `filter.<name>` / textconv conversions are pinned to inert helpers via argv config, discovery enumerates registered submodules and linked worktrees byte-exactly like git (multiline/NUL/CRLF/BOM values, padded links, quoted gitlink spellings), under bounded scans, a watchdog, and a git-version floor for env-config pins. `core.worktree` values carrying quotes or backslashes refuse: git escape-decodes quoted values, so the raw spelling validated here is not the string git applies (round-19 review). - gh reads judge every stage (compound and `*`-bearing stages obey the host network policy and the worker envelope); the pipefail prefix keys on the shell kind, and cmd.exe/PowerShell delimiter walls are stated in refusals. - The `[shell.readonly.command]` refusal enumerates admitted families, the every-segment rule, and refused shapes; the worker catalog's read-only gh pre-filter uses the same per-stage predicate. Tests: 114 new pins across classifier, operand gate, glob walker, git discovery/parser differential (incl. the escape-decode worktree wall, mutation-verified 2/2 red), network guards, and lane routing; every listed escape was reproduced end-to-end against the unfixed code before fixing (18 review rounds). Targeted: `tools::shell::` + `command_safety::` + `tools::subagent::` + `tools::registry::` + `tools::verifier::` 1019 passed / 0 failed / 1 ignored (the ignored test pre-existing; `tools::verifier::` is the current filter name — the pre-split `tests::verifier::` spelling no longer matches); fmt clean; fork-CI clippy flags clean. Split of #126, rebased onto fix/shell-stdin-eof @6337b0e0a (the stdin-review round); full pre-split content preserved on archive/p126-pre-split-8705ccd85. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
Round-20 review wave on the compound read-only probe slice: - the fixed spawn-env overrides now clear BASHOPTS/SHELLOPTS: bash honors both at startup from the inherited process environment, so an operator-exported BASHOPTS=dotglob would widen * over the dotfiles the GLOBIGNORE wall keeps cold (same class as the round-6 ZDOTDIR and GLOBIGNORE pins); the table pin now covers six fixed keys - the three mode-000 root-runner skip notes route through one shared helper with a single targeted clippy::print_stderr exemption, keeping the crate-level deny lint clean under --all-targets clippy - rebase repair: a duplicated bounded-stdin doc comment block landed above the cd-operand test in the #123/#124 rebase; the copy is gone and the separating blank line restored Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
First slice of the #126 split: the foreground/sync stdin lifecycle. - Foreground and sync shell commands without a stdin payload no longer hold the write end open for the process lifetime: foreground closes it right after spawn, the sync lane spawns with `Stdio::null()`, so a stdin-reading probe (`cat`, a bare `$SHELL`) gets end-of-file and exits instead of idling until the 120 s watchdog. - A foreground command later detached to the background reports its closed stdin on `interact` instead of an opaque refusal. - stdin writes are bounded everywhere (64 KiB inline cap, owned-writer fallback for Windows/ohos, 5 s deadline with a stall flag), a PTY writer no longer injects EOF on drop (dup'd master fd), and the verifier's background gate jobs are handed EOF at spawn instead of idling on the held-open pipe. - bash guidance and the schema describe the foreground stdin contract; the model-facing description carries the same sentence. Tests: 11 new pins (foreground EOF, sync-lane EOF, detached-foreground interact receipt, PTY interact surface, stalled-tty bound, interact/ inline caps, bounded owned write) + the verifier gate-job EOF pin. Targeted: `tools::shell::` 152 passed / 0 failed / 1 ignored (pre-existing), `tests::verifier::` green; fmt clean; fork-CI clippy flags clean; upstream all-targets clippy reports only the pre-existing core/engine/turn_loop.rs:5499 collapsible_if (base code). Split of #126; full content preserved on archive/p126-pre-split-8705ccd85. Stacks: none (base f1eb10e). Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
9148c17 to
8c6c00d
Compare
8f98b3e to
1f9635a
Compare
|
Heads-up from the #140 round-2 review: the
So it is a parallelism-dependent race, invisible under the fork-CI invocation ( Attribution (all runs on this board, 20 hardware threads):
Given slices 1/2 own the touched surfaces (slice-1's new |
Third slice of the #126 split: the `test` role's honest surface. - The roster description, the type schema, VERIFIER_AGENT_INTRO, and both SUBAGENTS role tables state the real contract: built-in verification gates with test-selection arguments, plus the classifier-bounded read-only shell for evidence gathering; custom commands stay refused. - The Verifier joins `role_requires_read_only_shell` and `allows_bounded_readonly_bash` (the latter now reads that one predicate), gaining the same bounded probe lane as the inspection roles; the carve-out is Verifier-only and keys on a recorded parent-shell grant, so a read-only-parent's child never regains authority through the role name alone (chain pins cover the sub-subagent path). - Shell authority (`holds_shell_authority`) keys on the held policy plus the grant sentinel; fleet receipts report the verifier's effective shell as `read_only`; the worker catalog omits bash from envelopes without read-only shell authority and pins the refusal copy's load-bearing claims. Tests: verifier contract/authority/carve-out pins incl. the sub-subagent chain, catalog and receipts labels; the deleted no-shell verifier catalog test is replaced by the probe-lane variant. Targeted: `tools::subagent::` + `tools::registry::` + `fleet::` suites green; tree of this commit equals the reviewed #126 (stacks on fix/live-test-findings-subagent-shell after fix/shell-stdin-eof); full content preserved on archive/p126-pre-split-8705ccd85. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
…nd close round-21 review findings The round-20 rebase of this branch onto the #123/#124-merged main resolved subagent/tests.rs by dropping six merged #123/#124 test fns and the `mod identity_selection;` declaration, silently orphaning the 11 forkguard_agent_identity_* tests in the still-present tests/identity_selection.rs (26 KB, byte-identical, no longer compiled). The dropped fns exercise surfaces this slice does not touch (same-worktree peer gating, ancestor walk-up non-adoption, claim-path normalization, scoped mutation paths) and all 18 tests pass restored, so the deletion was pure coverage loss, not a forced adaptation. This restores them union-keep-both — the resolution the body already documents for this file. Also: pin the "Do not retry variations" guidance fragment in the replacement enumeration test (the one fragment the deleted shell_readonly_refusal_enumerates_the_admitted_families pin carried that the replacement dropped), and correct the readonly_shell_bound_command doc comment, which said the pipefail prefix keys on "a pipeline" while the predicate is a plain `contains('|')` (an || sequence matches and receives the harmless no-op prefix). Receipts at this head: readonly_ 150/0, command_safety 101/0, forkguard 285/0 (--test-threads=2), tools::subagent:: 648/0, rustfmt clean on both touched files. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
asto18089
left a comment
There was a problem hiding this comment.
Round 21 — fresh 9-lane re-review at the re-aligned stack (head 6386b876b, tree bc3415f3c9, base 8c6c00dc0c)
Rebase performed first (per review protocol)
pinvou3-clean moved five commits past the round-20 base (449edbf0b → 11cba403dc: #129, #131, #133, #134, #135 — the last four touch tools/subagent/, overlapping this slice's files). The stack was re-aligned: #139's four commits re-based onto 11cba403dc as 8c6c00dc0c (delta bit-identical, verified against the true merge-base 449edbf0b), then this slice's two commits replayed as 7b754c7df + 1f9635a894. Fidelity: the four non-overlap files are blob-identical to the round-20 head; subagent/mod.rs delta content-identical; subagent/tests.rs delta differs by exactly one cosmetic blank-line-removal hunk that #131's inserted test fn makes unpreservable — union kept both sides. rustfmt parse-checks ruled out union-brace breakage before push.
The one P1 — found by two independent lanes, verified, fixed in-wave (6386b876b)
The round-20 rebase resolution of tools/subagent/tests.rs silently dropped coverage instead of keeping both sides: six merged-#123/#124 test fns (same_worktree_disjoint_sibling_blocks_unbounded_shell, same_worktree_ancestor_chain_keeps_unbounded_shell, same_worktree_gate_ignores_settled_and_foreign_worktree_peers, worktree_discovery_never_adopts_or_enumerates_an_ancestors_sibling_repos, normalize_claim_path_folds_backslashes_into_the_ledger_frame, scoped_mutation_paths_accept_absolute_targets_inside_the_workspace) plus the mod identity_selection; declaration — orphaning the still-present tests/identity_selection.rs (11 forkguard_agent_identity_* tests, byte-identical 26 KB file, no longer compiled anywhere in the stack; also absent from slice 3's tree, verified at its head). Traced: absent from the pre-split archive because the archive predates #123/#124, present at base and on main, deleted by this branch's own delta since the round-20 rewrite. A fn-set audit (base vs head) confirms these seven are the only deletions. All six fns exercise surfaces this slice does not touch (same-worktree peer gating, ancestor walk-up non-adoption, claim-path normalization, scoped mutation paths). The fix wave restores them union-keep-both — the resolution the body already documents — plus re-pins the Do not retry variations guidance fragment (the one fragment the deleted enumeration pin carried that the replacement dropped) and corrects the readonly_shell_bound_command doc comment (the pipefail prefix keys on a plain contains('|'), so an || sequence receives the harmless no-op prefix; doc said "a pipeline"). All 18 restored tests pass — pure coverage loss, not forced adaptation.
Lane results
| Lane | Verdict |
|---|---|
| L1 classifier grammar | PASS — 65 hand-traced inputs through a compiled harness; seam walls verified for every deliberate classifier admission; 3×P3 |
| L2 execution seam | PASS — env wall complete (BASH_ENV/ENV/ZDOTDIR/GLOBIGNORE/CDPATH/RIPGREP_CONFIG_PATH/BASHOPTS/SHELLOPTS + child_env allowlist double-wall + sanitized PATH); splice re-quoting sound; cd fold/containment verified; 108 readonly E2Es green; 3×P3 |
| L3 authority wiring | PASS — predicate swap uniform at all 3 consumers, 0 dead refs; every route judges pre-spawn; child shell min-clamped (min_with(parent)), posture denials inheritable; no delegation leak |
| L4 split fidelity + smuggling | SMUGGLING: none — 595 hunks classified; registry.rs/mod.rs deltas are exactly the claimed wiring; blank-line churn carries zero content; the P1 above is its one finding |
| L5 test quality + mutations | new pins high quality; 6/6 production mutations turned their pins red (escape-decode wall, BASHOPTS env pin, compound .any flip, empty-segment flip, cd * flip, gh predicate swap); conflict regions compile and pass |
| L6 executing red team | 0 escapes — 222 tool-entry + 49 classifier + 12 spawn-env probes on bash, dash, and the unset-SHELL fallback; quoted-escape/real-newline core.worktree, -C wildcards/symlink chains, filter/textconv/fsmonitor fixtures (raw-shell positive controls fire, lane never does), per-stage gh Deny-policy compounds, hostile parent env incl. workspace-internal startup files: all held |
| L7 performance | PASS — no regression vs base on common paths; new costs bounded (glob caps + 10 s deadline; discovery 32-repo cap + watchdog + 10 s aggregate wall); one claim-precision nit (below) |
| L8 body accuracy | all substantive section-2 claims TRUE in the delta; stale anchors + two body-vs-tree contradictions — corrected in the body this round |
| L9 static gates | PASS — receipts below |
P3 ledger (disclosed in the body, not changed)
- GNU
+Ncount spellings (tail -n +5) newly refuse — digit-validation class round 18 disclosed didn't name this family (base admitted it). - Availability refusals:
git status -uall,grep -r/-rn/-e/--include,du -sh(alternatives cover the need:du -s -h,git status --porcelain,rg -g,git grep -e). git worktree listprints operator-registered linked-worktree paths — the documented name-only disclosure class.command_reaches_github_readonlyruns pre-admission on unbounded input (linear, ~7 µs typical — bounding note only).- The
0.2–0.5 ms per admitted probefigure understates git-stage probes: ≈3.4 ms/repo warm discovery (32-repo cap ≈ 111 ms; walls bound the pathological case) — body corrected. - Pre-existing, unchanged (flagged for the record, out of this delta): a network-denied Full-shell child can reach a gh read through
task_shell_start/task_shell_wait(not in the precheck's name scope) unless anetwork_policydecider is configured; theforkguardsuite's global test-env locks convoy at high thread counts.
Receipts at 6386b876b (tree bc3415f3c9)
readonly_150/0 ·command_safety::101/0 ·forkguard_285/0 (--test-threads=2) ·tools::subagent::648/0 ·readonly_operand_gate23/0 ·registry202/0/2 pre-existing ignores.cargo check -p codewhale-tui --all-targetsclean under workspacedeny(warnings); rustfmt clean on all touched files; fork-CI clippy clean; upstream-CI clippy variant reports only pre-existing sites (identical inventory at base, none in this PR's files).- gitleaks over the introduced range
8c6c00dc0c..6386b876b: no leaks. - PR body refreshed: round-21 bullet + receipt, split-time note round-21 update (stack anchors
8c6c00dc0c/7b754c7df/1f9635a894/6386b876b/treebc3415f3c9), the two keep-both claims corrected, #140 row/split-table updated, merge-order section marked historical, clippy anchors refreshed.
Open items
- #140 must follow the round-21 wave head (
6386b876b) — it re-based onto1f9635a894mid-review (round-3 fixes, head6e2ed34fb8) and is now behind by exactly the fix commit; one mechanical base-follow before merge, else the combined tree loses this round's restored coverage. - DCO advisory:
1f9635a894author email (@126.com) ≠ itsSigned-off-by(@users.noreply.github.com) — gate passes; normalize at squash.
Verdict: with the P1 fixed in-wave, the slice is useful, root-cause-complete against its claims, elegant (one splitter family serving classifier/gate/hardening/network-guard/discovery), smuggling-free, and bug-free per all nine lanes — COMMENTED (no outstanding blocker in this slice's own delta).
Summary
No-Issue: issues are disabled on this repository; the defects below were reproduced from a live multi-agent capability-test session transcript (2026-10-27, parent and children on
deepseek/deepseek-flash, macOS arm64 / zsh; 33 test cases, 10 subagent instances; every material claim in the transcript was re-verified parent-side with independent reads and command runs). Each fix carries red/green tests; the load-bearing pins are mutation-verified. Eighteen adversarial review rounds found and closed the workspace-boundary gaps enumerated below — acdsymlink escape, acd/ghpipeline-stage escape, glob operands expanding over outside symlinks, a gh network-policy bypass, acdwildcard the shell expands differently from the gate,.*expanding through.., an unbounded in-host glob walk, a secondsed -escript, a zsh=cmdoperand, and (round 4) acdonto a regular file,=-bearing and whitespace-bearing operands judged under the wrong name, a glob budget that was per operand and blind to zero-match scans, repository textconv executed by shell-lanegitstages and bygit blameon both lanes, and two network ceilings keyed on the whole command, and (round 5, a fresh 17-lane review with a code-executing red team on zsh/bash/sh/dash) a Windows path normalization that ran on unix and let a\\?\spelling carry a live?glob past the charset gate, the operand gate's own verbatim-prefix strip on unix, acd =cmdspelling zsh reinterprets, acdonto a directory the shell cannot enter, zsh's recursive**/, cmd.exe%NAME%expansion, and option words whose glob the shell expands whole — and two authority gaps in fix 3 (an over-wide re-key, and gate execution obtained through a read-only role parent via delegation), and (round 7, a fresh 12-lane review with a code-executing red team) a repository-declared git content filter executing under admittedgit status/git diff/git blame— outside content crossing and attacker-chosen commands running under the branch's own child env — and a gh stage whose glob the shell expands into the invocation'sHOST/OWNER/REPOposition at runtime, past a host-scoped network policy, and (round 11, a fresh 11-lane review) a.gitgitfile consumed as one line while git reads the whole value — interior newlines naming a gitdir whose content filters executed under an admitted read — and agit -Coperand judged as a literal spelling while the shell expands it, the chainedgit -C x -C *shape binding the second-Cto an outside symlink and rendering an outside repository's committed content — and (round 12, a fresh 12-lane review) a strip model one byte short of git's — the.git/commondirlink value's trailing\r/\nrun is stripped in any order and count, not one\nplus one\r, so a second trailing newline sent discovery to a decoy-or-nothing scan while the hostile repository's content filter ran under the admitted read — and a glob..synthesis judged at the lexical parent while dash resolves the expanded..physically through a followed directory symlink, reading an outside file the gate called inert — and (round 13, a fresh 13-lane review) a.gitDIRECTORY that returned from the walk-up unvalidated while git's own discovery skips a directory failingis_git_directoryand keeps walking — one empty directory between the stage root and a hostile repository sent discovery to a scan-nothing admit while the ANCESTOR's content filter executed under the admitted read (including a bogus dir at the workspace root with the hostile repository above it, reachable by a workspace-scoped writer alone) — and a gitdir validity check approximating git's predicate in both directions — admitting gitdirs git fatals on (garbage HEAD text, a missing refs/) while refusing gitdirs git runs (a dangling-HEAD symlink) under a false message — and a.gitlink value read past the first NUL byte where git's C-string read ends it; and (round 18, a fresh 11-lane review with an executing red team) an absolute glob operand whose literal spelling canonicalized inside the workspace while the shell's expansion matched an outside symlink (executed leak), a#-comment tail the gate's tokenizer dropped while the cmd.exe lane — which has no comment syntax — passed it through as live unjudged argv, an ungated test helper that went dead-code on the windows test lane under-Dwarnings(required check red), and the Windows/ohos interact write that could wedge the session's shell lock indefinitely — and (rounds 14–17, spanning the pre-split rounds) the walk-up-symlinked-gitfile /ref:-control-char / commondir-NUL escapes, the post----a=bre-arm and the planted--ext-diffre-arm, the git-stage-glob bypass and the cmd,/=delimiters, and the continued-core.worktreerelocation — and (round 19) the quoted-escape-decodecore.worktreerelocation — every known one is fixed and pinned in this slice's tree (the fix-3 authority pair and the docs hunks are slice 3's — see Split), see "Review rounds" below.Topic-disjoint from #119 (merged), #123, #124, #125. Shared-file hunk positions against the other open PRs are re-swept every round and recorded under "Merge order" below (as of round 20: #123 and #124 have merged to
pinvou3-cleanand this branch's stack has been rebased onto them — the round-20 lane verified the rebase faithful, see the Split-time note; #136's conflict shapes stay under "Merge order").Split
This PR is a slice of the original #126, split along its three named fixes (2026-11-04; the full pre-split content is preserved branch-exact on
archive/p126-pre-split-8705ccd85, head tree8705ccd852052):pinvou3-cleantestrole's honest contract + probe laneThe 18 review rounds below reviewed the combined tree (
f1eb10efc..baf9ce81c); every finding is re-anchored to the slice that owns it, and each slice re-ran its own gates (receipts under Testing). The three slices cannot be reordered: 3 depends on 2's probe-lane admission for the Verifier, and 2's seams build on 1's stdin lifecycle contracts.1. stdin EOF for foreground and sync commands — moved to slice 1 (#139): the fix, its pins, the guidance sentence, and the verifier gate-job EOF live there; this PR's seams assume its stdin contracts.
2. The bounded read-only grammar refused compound probes (
;,&&,cd … && …,od)Live evidence: read-only children were refused
wc -c f; wc -l f(sequence),cd dir && wc -c f, andod -c f, with[shell.readonly.command]; planner and explore both burned steps rewriting probes around the refusals (the transcript records the rewrites; #123's transcript separately recorded a 13-retry loop against the old one-line refusal).Fix (classifier,
command_safety.rs):a; b,a && b,a || b— admitted iff every segment individually passes the sameis_agent_readonly_segmentgate, via a new shared splitterreadonly_sequence_segments(Noneon a bare&, which still refuses; empty segments refuse; the split is deliberately naive like the existing|split, and a quoted-separator mis-split that still classifies is refused at the seam by the operand gate's stricter parse);cd <one relative path>— at any position in the sequence, never a pipeline stage (a stage runs in a subshell and persists nowhere the gate could follow), no flags/../absolute/~/=/wildcard, with one shape rule (readonly_cd_operand_admitted) shared by the classifier and the execution seam; the operand is additionally canonicalized against the workspace at the execution seam (same[shell.readonly.cwd.outside_workspace]refusal family); at most 32 sequence segments, since the seam resolves onecdper segment against the filesystem;oddumps — display radices-b -c -d -o -s -x -h,-v,-Arestricted tod|o|x|n|none, numeric-j/-Nonly,--handled; piped in either direction;cut -d, -f1,comm -12,sort -rn,sort -k2) — expanded before the exact match, and only when every letter is an admitted switch or the first letter an admitted value option; the parent's parallel classifier does not use this grammar.Execution (
shell.rs): compounds route through the shell lane pipelines already use (pipefail prefix only when a pipe is present); a command whose leading segment iscdalso takes the shell lane —cdis a builtin and most Linux hosts ship no/usr/bin/cdbinary, so the direct-argv lane would have failed an admitted probe with a not-found exec error (macOS happens to ship one, which is why the router is pinned host-independently). On Windows,cmd.exetreats;as an argument delimiter, so the seam refuses;-sequences for a resolvedcmdshell ([shell.readonly.sequence.unsupported_shell], a pure per-shell-kind decision pinned on every host; PowerShell runs them;&&/||parse on PowerShell 7+, and the Windows PowerShell 5.1 parse error is a safe nothing-ran failure — precision fixed in round 13, whose predicate's own doc concedes the 5.1 case). The operand-containment gate splits with the same shared splitter and judges every sequence segment and, inside a segment, every pipeline stage on its own: theghexemption applies to a gh stage only (gh …; ls /etcandgh … | cat /etc/passwdare operand-checked where the old whole-command parse skipped everything), acdstage is refused ([shell.readonly.cwd.pipeline_stage]), and a*operand — the only wildcard the charset admits — is expanded against the live working directory the way the shell expands it (component by component, hidden entries only when asked for, symlinked directories followed) and every match is judged by its resolved location. The gh network-policy guard judges the same stages, so a gh read anywhere in a compound consults the decider exactly like a bare one — and since round 4 the no-network subagent pre-check and the fleet envelope pre-filter consult that same per-stage predicate (command_reaches_github_readonly). Gitlog/show/diff/blamestages on the shell lane receive the same in-process hardening flags the direct-argv lane splices (--no-ext-diff --no-textconv [--no-show-signature],--no-textconvforblame), re-emitted from the argv the gate parsed with the separators copied through byte for byte (round 4).Review rounds (each escape proven end-to-end under
ShellPolicy::ReadOnlyagainst the unfixed code, then fixed and pinned red/green):Round 1 —
cd sub; cat link, withsub/linka symlink to an outside file, read the file because every segment's operands resolved against the dispatch-time cwd. The gate now folds the drift left-to-right: after an admittedcd, later segments resolve against the canonicalized destination, exactly where the shell resolves them (readonly_operand_gate_refuses_relative_operands_after_cd_cwd_drift,readonly_operand_gate_keeps_admitting_operands_under_cd_destination).Round 2 —
cd sub | cat /etc/passwdreturned the file: the gate judged a segment by its first program, and acd(orgh, a gap that predates this PR) stage returned early with every operand after the|unchecked, while the shell ran the stage in a subshell. The classifier now refusescdas a pipeline stage and the gate judges stages one by one (agent_readonly_shell_rejects_unsafe_compound_cd_and_od_shapes,readonly_operand_gate_judges_every_pipeline_stage_on_its_own).Round 2 —
cd sub; cat *returned an outside file through an in-workspace symlink: the literal spelling*exists on no disk, so the operand check skipped it (the pipeline formcat * | headleaked the same way before this PR). Glob operands are now expanded and judged match by match (readonly_operand_gate_judges_glob_expansions_by_resolved_location); both rounds are also pinned through the tool (readonly_shell_refuses_pipeline_cd_and_glob_escapes_end_to_end).Round 2 —
pwd; gh repo viewnever consulted a Deny network policy because the guard classified the whole command as "not a gh read". It now judges every stage (readonly_github_shell_calls_obey_the_host_network_policy_before_spawn).Round 3 —
cd cdt*; cat secret.txtread outside on bash/sh: the seam canonicalized the operand spelling (a literalcdt*directory) while the shell entered the first glob match, a sibling symlink pointing out.cdis now admitted only as one relative path — a wildcard in the operand refuses at the classifier (agent_readonly_shell_rejects_unsafe_compound_cd_and_od_shapes).Round 3 —
cat .*/secret.txtread outside on bash/sh: sh/dash expand.*to.and..(modern bash and zsh do not), andread_dirnever yields them, so the walker never saw the parent directory. It now synthesizes both candidates at every level a dot-component matches and judges them by resolved location (readonly_glob_walker_judges_the_dot_and_dotdot_candidates).Round 3 — the glob walk ran in the host process before any child existed to time out, and an ancestor-pointing directory symlink made a deep pattern re-enter the same directory exponentially (10.4 s measured at pattern depth 14). Matches are judged as they are produced, re-entries deduplicate by (directory identity, remaining pattern length), and match/node caps (4096 / 50 000) refuse instead of exploring; the gate runs on the blocking pool (
readonly_glob_walk_is_bounded_by_the_match_and_node_caps).Round 3 —
gh pr list --search '*'andpwd; gh repo view *skipped a Deny network policy: the guard's per-stage predicate reused the whole-command parallel classifier, whose charset refuses*while the agent grammar admits it. The stage check is now token-level (github_stage_predicate_sees_glob_operand_stages, plus the compound loop inreadonly_github_shell_calls_obey_the_host_network_policy_before_spawn).Round 3 —
sed -n 5p -e w/tmp/owned Cargo.tomlwas admitted end to end: GNU sed parses a later-eas a second script (write/read/exec outside the workspace); BSD sed fails closed. Every token after the script must now be a plain input file (agent_readonly_shell_rejects_mutation_and_injection); multi-file reads stay admitted.Round 3 —
cat =ls | wc -lread the resolved absolute path oflson zsh hosts (default EQUALS expansion) while the operand gate skipped the spelling as a nonexistent relative value after its=-split.=cmdoperands now refuse outright, before any shell runs (readonly_operands_are_workspace_bounded_and_symlink_aware).Round 3 (authority — moved to slice 3 (fix(fleet): the test role states its real contract #140): the registry carve-out fix and both pins named below live there; this slice's tree contains none of it) — a Verifier child of a read-only role parent (Scout/Reviewer/Planner) kept the built-in Run surface and executed cargo test where the base refused it at
[role.posture.denied]: the carve-out keyed on the role alone, so the read-only shell a parent clamped onto the child still unlocked the gates. The registry now records whether the child's shell was full after the parent clamp but before the role's own narrowing, and only such a Verifier keeps the gates — delegation moves work, never authority (clamped_verifier_child_of_read_only_parent_holds_no_run_surface;verifier_child_keeps_the_default_and_selection_bounded_run_surfacestill pins the direct case, and the fleet ceilings and shell-less sessions are unchanged).Round 3 — the zh FLEET copy promised fleet members the classifier-gated bash probe lane; the fleet envelope defines bounded verification and Bash authority as mutually exclusive (
normalized()), so the sentence now scopes the lane to theagentnesting path. Round 3 also aligned the model-facing routing description (sequences and cd-prefixed commands take the shell lane; a standalonecdaffects only its own command), qualified the foreground-stdin guidance sentence to the non-tty lanes, scoped the POSIXpipefailprefix and the cmd.exe;refusal into pure per-shell-kind decisions (cmd/PowerShell no longer receive the prefix at all), admitted gh-qspellings, added the 32-segment cap to the refusal copy, restored the zh verifier bullet the English update left behind, and pinned the previously-masked behaviors (piped-cd tag, live-cwd fold, hidden-entry exclusion, dangling glob matches, guidance sentence, unix gating).Round 4 —
cd notes.txt; cat link(a regular file as thecddestination) read an outside symlink at the root on bash and zsh: the seam canonicalized the file and folded later segments' cwd onto it, every later relative operand skipped judgement at theexists()miss undernotes.txt/…, while the shell'scdfailed and;/||continued in the original directory. The destination must now be a directory ([shell.readonly.cwd.not_a_directory];readonly_operand_gate_refuses_cd_onto_a_regular_file).Round 4 — the gate judged only the text after an operand's first
=, socat ax=b.out(an outside symlink by that name) was judged as the inside decoyb.outon the direct lane too, and the globa*=b*was walked asb*while the shell matchedax=b.out. Only--prefixed tokens split now; a bare operand is judged as the whole word (readonly_operand_gate_judges_equals_bearing_operands_as_whole_words). The gate also trimmed every spelling while the program opened it untrimmed —cat ' x 'read an outside symlink namedx— so operands are now judged byte for byte as parsed (readonly_operand_gate_judges_the_parsed_spelling_without_trimming).Round 4 — the glob caps were per operand: a ~1 KB command of 500
*tokens over a 2000-entry directory bought ~25 s of admitted host CPU, and a zero-match pattern (zz*) moved neither counter while still scanning every entry (8000 of them: 8 s). OneReadonlyGlobBudgetper command now counts matches, entered nodes, and examined entries (4096 / 50 000 / 500 000) across every segment, stage, and operand (readonly_glob_budget_is_shared_across_operands,readonly_glob_budget_counts_examined_entries).Round 4 — the shell lane handed the shell the raw compound, so
git show HEAD | catandpwd; git show HEADexecuted a repository-configureddiff.<driver>.textconv(a marker was written outside the workspace) while baregit show HEADwas spliced on the direct lane; theGIT_*environment hardening pins global and system configuration, not the repository's. Andgit blameapplies textconv by default yet was in the no-splice set on both lanes. One splice table (splice_git_read_hardening) now serves both lanes, andharden_readonly_shell_git_stagesrewrites every Gitlog/show/diff/blamestage before the shell binds the compound, refusing a stage whose token cannot be re-quoted portably ([shell.readonly.git.unquotable_stage]) rather than running it unhardened (readonly_shell_lane_hardens_git_diff_rendering_stages; end-to-endreadonly_shell_lane_git_show_does_not_run_repository_textconv, whose fixture is proven live by raw-shell controls for bothshowandblamefirst).Round 4 —
reject_network_reaching_input(a child withnetwork_tool = false) and the fleet envelope pre-filter still used the whole-command gh predicate and passedpwd; gh repo viewandgh pr list --search '*'; both now consult the per-stagecommand_reaches_github_readonly, the whole-commandis_github_readonly_commandis deleted, and the three inlined|splits are onereadonly_pipeline_stages(compound cases added toa_network_denied_child_cannot_address_a_remote_location_through_any_tool;command_reaches_github_readonly_recognizes_gh_reads_per_stage). Round 4 also pinned the seam's fail-closed handling of quoted separators (readonly_operand_gate_fails_closed_on_quoted_separators), sized the match-cap fixture by a literal so a raised cap reddens the pin instead of hanging it, qualified the routing description for cmd.exe;-sequences, interpolated the segment cap into the refusal copy, and scoped the Verifier roster promise to in-session delegations (the fleet durable-execution path grants that role no bash lane).Round 5 —
pwd; cat a\\?\bread a planted outside symlink nameda\?bon zsh, bash, sh, and dash:normalize_windows_command_pathsstripped\\?\before the charset gate refused?and doubled backslashes of Windows-looking words, on every host, so the gate judgedpwd; cat ab(a nonexistent name, skipped) while the POSIX shell parsed the raw text and expandeda\?bas a one-character glob; the operand gate's own verbatim-prefix strip had the same shape (cat \\.\secretjudged assecret, the program opened the real ten-character name — the direct lane too). Both transforms now run only when compiled for Windows, wherePath::canonicalizegenuinely yields those spellings; on unix the spellings are plain POSIX file names and fail closed at the charset and shape gates (windows_path_normalization_stays_off_unix_lanes,agent_readonly_shell_refuses_verbatim_spellings_unix_lanes; the Windows-side admission pin is now#[cfg(windows)]). The Windows lane itself is unchanged and its judged-vs-run divergence for verbatim spellings was not exercised here (no Windows host).Round 5 —
cd =ls; cat linkon zsh: the cd rule refused flags,\,*,~, absolute and..but not=, and the seam's cd arm never ran the operand gate's=refusal, so zsh's EQUALS expansion made the shell'scd /bin/lsfail and;/||continue at the root while the gate had folded onto a literal=lsdirectory. The cd operand shape rule is now one function shared by both layers, refusing=-prefixed and empty operands (readonly_operand_gate_refuses_cd_operands_the_shell_reinterprets,[shell.readonly.cwd.shape]).Round 5 —
cd vault; cat linkwith a mode-000 directory, on every POSIX shell:canonicalizeandis_dirneed search permission only on the ancestors whilechdirneeds it on the destination, so the round-4 directory check passed and the fold lied again (commit c42312a's "TOCTOU race" framing was wrong — a permission-deniedcdis deterministic). The seam now proves the destination enterable by resolving<dir>/., exactly the permissionchdirneeds, without listing it ([shell.readonly.cwd.unsearchable];readonly_operand_gate_refuses_cd_destinations_the_shell_cannot_enter, end-to-endreadonly_shell_refuses_unsearchable_cd_destinations_end_to_end). The race between that probe and the shell'scdis the residual no pre-spawn check can close.Round 5 —
pwd; cat **/*.txton zsh read a depth-two outside symlink: the walker consumes one directory level per component (bash withoutglobstar, sh, dash agree) while zsh expands**/to every depth. A**/***component followed by more pattern is refused; a trailing**is a plain*everywhere and stays admitted. The walk is also capped at 32 components, since every.*level enters the synthesized..and the recursion depth was otherwise unbounded (readonly_operand_gate_refuses_recursive_glob_components).Round 5 — the shell pathname-expands an option word whole, so
--exclude-from=zz*matched a planted--exclude-from=zzqwhile the gate walked onlyzz*; an option carrying a wildcard is now judged by its full spelling as well (readonly_operand_gate_judges_dash_prefixed_glob_operands_as_whole_words; the shape is not reachable through the current grammar, which admits no--opt=valueoption — defense in depth at the seam).Round 5 — the
set -o pipefailprefix was prepended for every non-Windows kind includingCustom(any$SHELL) andSh;/bin/shis dash on Debian/Ubuntu and dash exits the whole-cpayload on it (set: Illegal option -o pipefail, status 2), so every admitted pipeline and — because||contains|— every||sequence ran nothing there. The prefix is now keyed on the executable's file name (bash, zsh, ksh family); other binaries run unprefixed and report the last stage's status, which is reporting fidelity, not a boundary. cmd.exe additionally refuses any%-bearing compound (it expands%NAME%before any program sees the word — the twin of zsh's=cmd), and%/^moved out of the Git stage re-quoter's bare set (readonly_shell_lane_decisions_per_shell_kindnow covers/bin/sh, dash, bash, zsh, and a%compound on cmd). The round-3 claim that the prefix had been "scoped into pure per-shell-kind decisions" was keyed on the wrong thing.Round 5 also: the
interactrefusal said "already at end-of-file" for every task without a writable stdin, which is true only when a stdin pipe was closed (not for PTY/combined_output, persistent, or finished tasks) — the task now records whether a pipe was really closed and states the accurate reason; the verifier's background gate jobs get end-of-file right after spawn instead of idling on an open pipe until the 600 s gate timeout (no dedicated pin — the gate harness has no stdin-reading gate fixture); the operand-gate refusals name the "read-only shell" rather than a Scout shell the Verifier does not hold; the[shell.readonly.command]copy states the enter rule and the new refused shapes and is now pinned on its load-bearing claims; the fleet envelope's Bash admission is the parallel classifier, so compound gh reads fail closed there with or without a network grant (pinned — compound probes are an in-sessionagentcapability, not a fleet one);agent_readonly_command_needs_shell_for_cd's doc and both SUBAGENTS docs saycdis admitted at any sequence position; two gh predicates lost an unneededpub(crate).Round 6 (fresh 11-lane review) — the cwd fold ignored sequence connectors: an admitted
cdfolds unconditionally, but acdjoined by&&/||runs only when the predecessor's exit status allows it, socat nope && cd sub; cat link(andtrue || cd sub; cat link) left the shell at the original directory — where a planted root symlink awaited — while the gate judgedlinkundersub/and skipped it at theexists()miss (proven live on bash and sh through the tool; the third member of the fold-lies family, the cd-NEVER-RUNS twin of round 4's cd-FAILS cases). The splitter now carries each segment's connector (readonly_sequence_segments_with_connectors); the classifier refuses a conditionally-gatedcdstatically (agent_readonly_shell_rejects_conditionally_gated_cd) and the seam re-checks it at the fold ([shell.readonly.cwd.conditional];readonly_operand_gate_refuses_a_cd_its_connector_may_not_run, end-to-endreadonly_shell_refuses_short_circuit_cd_escape_end_to_end); the refusal copy names the shape. Acdits connector always runs (first segment, or joined by;) still folds, so the advertisedcd dir && wc -c fand the;-drift pin are unchanged. Also from round 6: cmd.exe consumes a bare^as an escape before any program sees the word (cat a^b | wc -copensab, a spelling the gate skipped as nonexistent), soreadonly_compound_refused_by_shell_kindnow refuses^-bearing compounds forCmdnext to%and drops the un-actionable PowerShell advice from that copy; the generic stdin refusal states accurate reasons per state (a terminal command exposes a writable stdin only while it runs and until its stdin is closed — the old copy claimed combined-output commands never expose one); the dot-glob./..synthesis is attributed to sh/dash (modern bash does not expand them — comments and the walker test doc); the genuine two-cdfold gained a discriminating pin (cd sub; cd deep; cat link— the positive control was previously non-moving and did not bite); the gh stage's sequence-form operand exemption is pinned (gh issue list; cat <outside>); the runtime-reachablepwd; cat 50%off.txtshape of the cmd%refusal is pinned (the old pin used a single command that routes direct-argv); the gate's own piped pass refuses an empty stage;ZDOTDIR/GLOBIGNOREjoin the cleared startup env; the GIT_CONFIG_COUNT block documents its git >= 2.31 floor; the-truncatedoc line on find's mutating-primary list is now claimed (pre-existing refusal, doc-only); the zh FLEET doubled bracket is fixed; the SUBAGENTScdsentence (en/zh) scopes "any position" to positions the connector always runs.Round 7 (fresh 12-lane review) — two boundary gaps found and closed here. (1) Repository content filters: a worktree
.gitattributesassigningfilter=<name>plus repo config definingfilter.<name>.cleanmakes that command run (file content on stdin) whenever an admittedgit status/git diff/git blameconverts worktree content — under the branch's own child env, outside content crossed onto the command's output and attacker-chosen commands executed (first-hand reproduced; the round-4--no-textconv/--no-ext-diffsplice addresses a different mechanism and theGIT_CONFIG_COUNTblock pinned no filter;git statusis in the no-splice arm). The lane now discovers every filter the repositories a stage can reach declare — mirroring the operand gate'scdfold, so a nested repo entered bycdis scanned too, and resolving each stage's-Cpreamble exactly as git applies it (git -C sub statusruns againstsub's repository without the shell moving;readonly_filter_discovery_follows_the_cd_fold_into_nested_repositoriespins the fold, the-Credirect, and the interleaved--no-pager/-Cwalk) — and pins each declared conversion key to an inert helper:filter.<name>.clean/smudge→cat,filter.<name>.process→ the inert helper, whose protocol handshake then fails loudly instead of running the configured command. Only keys the repository itself declared are pinned (introducing aprocesskey git would not otherwise see breaks a working clean/smudge filter), and anything the scan cannot confidently enumerate — configincludedirectives, the worktree-config extension, unspellable filter names, more than eight names — refuses ([shell.readonly.git.filter_config]). An attribute naming an undefined filter is inert (git skips the conversion, probed), so the config scan is complete; aliases cannot shadow the admitted subcommands (probed). End-to-endreadonly_shell_lane_git_status_does_not_run_repository_content_filterswith a raw-shell positive control and unit pins for the discovery walk (readonly_filter_discovery_pins_declared_filters,readonly_filter_discovery_refuses_configs_it_cannot_enumerate,readonly_filter_discovery_follows_the_cd_fold_into_nested_repositories). (2) The operand gate's gh stage exemption was glob-blind:pwd | gh repo view */*/*is admitted, and the shell expands a planted workspace entry into the invocation'sHOST/OWNER/REPOposition at runtime — a host-scoped network-policy bypass, since the guard evaluates only the hardcodedapi.github.comand gh honors-R/positional hosts over the pinnedGH_HOST(proven live without external contact via a.invalidhost). A gh stage now refuses any wildcard after the program word (readonly_operand_gate_refuses_glob_operands_on_gh_stages; the exemption keeps holding for literal operands). Also from round 7:cd +Nrefuses (zsh's directory-stack selection reinterprets the spelling while the fold assumes entry — the+twin of round 5's=); the flags the git hardening splices (--no-ext-diff,--no-textconv,--no-show-signature) are admitted in the option tables they are spliced into, so a defensive spelling no longer loses the whole probe;gh -R ../foorefuses like../../xalready did; od-j/-Nvalues must parse as u64 (a 200-digit spelling used to admit); the read-only grammar bounds command text at 64 KiB before its linear passes allocate; the glob walk gained a 10 s wall-clock deadline ([shell.readonly.operand.glob_deadline]) so a cold or network-backed filesystem cannot stretch one expansion past any watchdog (no child exists yet for the timeout to end);interact/cancel/waitare policy-checked before the action dispatch — a read-only shell holds no task surface, and a task addressed under the policy could belong to another agent in the session (readonly_shell_refuses_task_actions_at_the_policy); the guidance states the combined_output foreground stdin contract; the interact EOF message states the closed-pipe reason neutrally (an operatorclose_stdinproduced the foreground wording); the en SUBAGENTScdsentence lost a duplicated phrase (introduced by round 6's edit; the zh twin was already clean); the two-cdfold pin's doc now describes its discriminating cases accurately; the Verifier's legacy-Bashposture wall is pinned Verifier-labeled; and the fleet executor comment distinguishing the Run-only executor path from the registry's bounded lane is claimed here.Round 8 (fresh 14-lane review: 12 static lanes, one test-execution lane, one mutation re-derivation lane) — the round-7 filter discovery was incomplete; five gaps each restored the original defect under the fix's own env, probe-verified on git 2.43 before fixing: (1) filter names merged case-insensitively (
eq_ignore_ascii_case) while git matches an attribute value against a config subsection byte for byte, so a case-variant driver ran unpinned — names now dedupe byte-exactly and each spelling gets its own pins; (2) keys on a section-header line ([filter "x"] clean = vas one physical line) were skipped while git parses them, and the same spelling carriedinclude/worktreeConfigpast the header-level walls — the header remainder now parses as a key; (3) value continuation keyed on any trailing backslash silently swallowed whole[filter]sections — behind a backslash-terminated comment (git ends comments at the newline) and behind an escaped-backslash value (\\ends a value, an odd count continues) — continuation now keys on the odd trailing-backslash count and comments never continue; (4) a.gitlink orcommondirpointer holding an undecodable byte silently reported "no repository" while git parses bytes and still runs that repository's filters — both now refuse, and every discovery read (link, pointer, config,.gitmodules) is size-bounded before allocation; (5) submodules were never enumerated: git forks its owngit status/git diffchild into each initialized submodule and that child converts content under the submodule's own repository config — probe-pinned it fires on a mtime-only touch while the superproject reports a clean tree, and the env pins travel into that child intact, so enumeration is the whole fix —.gitmodules-registered submodule repositories now scan (recursively through nested registrations, bounded by a repository cap), and registrations the lane cannot resolve byte-faithfully refuse. Also from round 8:core.worktree— the next member of the same repo-config family — admits only when it resolves back to the worktree the repository was discovered in (the absorbed-submodule spelling git itself writes:worktree = ../../../subrelative to the absorbed gitdir) and refuses redirects; filter pins are gated on a git >= 2.31 version check exactly when they are armed (on older git the env block silently does not exist, and forstatus/ls-files/grepthose pins are the only wall); the per-config 8-pin cap now fires during the parse instead of after every repository is scanned, bounding a crafted config to linear work;ghreads a three-component operand asHOST/OWNER/REPO, so single-label hosts (gh repo view myhost/o/r, resolvable through a DNS search domain) refuse like dotted ones already did; the 64 KiB command bound fires before the Windows path normalization allocates, and non-unix glob identity keys are byte-bounded; the-c-outranks-env-pins precedence — the classifier's refusal of-c-bearing git stages is load-bearing for the whole env-pin family — is recorded at the pin site with its own refusal pin; the emptyprocess-override comment now states the probed behavior (it does take on current git, silently disarming the filter — the/usr/bin/falsehelper stays the honest wall); the filter E2E is unix-gated like its textconv sibling and gained a raw-shellgit statuscontrol on a size-preserving fixture (size-differing dirt is decided from stat alone and never converts — the old fixture's status arm was vacuously green), and the worktree fixture now follows a real gitdir-link +commondirlayout (the old one never executed the branches it claimed); the submodule twin E2E pins the new enumeration end to end, with the raw-shell control proving the submodule filter fires from the superproject before the pins hold.Round 9 (fresh 14-lane review: 12 static lanes, one test-execution lane, one mutation re-derivation lane) — the round-8 parser still diverged from git's real config grammar in seven first-hand-proven shapes, each restoring unpinned filter execution under an admitted read; every one is fixed here against git 2.43 probes: (1) dotted section headers (
[filter.dotted],[FILTER.EV],[filter.a.b]) armed real drivers the parser treated as inert two-part keys — the dotted form case-folds the subsection and splits at the first dot, and[filter.x "y"]concatenates; headers now parse in all four git shapes; (2) two section headers on one physical line re-target the section at the second[while the parser pinned the decoy — the remainder after]now loops; (3) a value followed by a trailing comment ending in an odd backslash over-continued (git ends the value at the unquoted comment; the backslash inside it is dead) — comments are now cut quote-aware with quote state carried across continuation lines before backslash parity is computed; (4) duplicatecore.worktreevalidated the first value while git applies the last — the last is validated; (5) submodule enumeration keyed on.gitmodulesregistration although git forks its dirty-check child into ANY index gitlink holding a repository (probed: with.gitmodulesdeleted from the worktree but still in the index,git statusfired the submodule's filter) — enumeration now runs the hardenedgit ls-files --stage -z(cleared env + the same sanitized PATH the child gets, capped stdout, unresolvable paths refuse) and.gitmodulesis no longer consulted; (6) inline;/#value comments were kept where git strips them (.gitmodulespath = sub ; notedecoded tosubupstream while the scanner joined a nonexistent directory) — subsumed by the comment-cut rewrite; (7) a leading UTF-8 BOM (which git skips) hid the entire first section — now stripped. Also from round 9:core.worktreevalidation is skipped for linked worktrees (acommondirgitdir resolves its worktree from the back-pointer and git ignores the key there — probed, so the old unconditional validation refused legitimate submodule worktrees);.gitpointer files parse byte-exactly (literalgitdir:+ exactly one space, byte-exact first-line value; the shapes git fatals on refuse); the operand gate judgedgit -Coperands flat against the command cwd, so a chainedgit -C A -C escape statusthrough a workspace symlink landed in an outside repository the flat spelling refuses — gate and discovery now share one cumulative-Croot resolver (readonly_git_stage_root), and the false "every-Coperand was already canonicalized" comment is gone; the git version probe ran under the host environment and PATH — it now uses the child's cleared env + sanitized PATH (a planted shim in an agent-writable PATH directory is never the probed binary; pinned), and because the fixedcore.fsmonitor/hooksPathenv pins are written for every read-only git stage, the git >= 2.31 gate now fires for every git stage, not only filter-armed ones;ZDOTDIRis pinned to/(an empty value lets zsh fall back to~/.zshenv, whose operator options can widen admitted operands); the foreground stdin EOF write precedes the heavy-permit attach so no future fallible step can leak an open write end; od admits the glued-An/-j<N>/-N<N>spellings; the gh wildcard refusal copy and the sed separator comment state their real reasons;VERIFIER_AGENT_INTROscopes the read-only shell to in-session delegations; the newline-in-command and symlinked-cd-fold behaviors gained pins; and the money E2E now declares a second filter through the dotted spelling (the raw-shell control proves git arms it; the tool leg proves the lane pins it). The round-10 wave (fresh 12-lane review: 10 static/probe lanes, one test-execution lane, one mutation re-derivation lane) — the discovery machinery itself ran repository config unpinned: a hostile repo'score.fsmonitorcommand executes inside the enumeration's owngit ls-files --stage -zon git 2.43 (probe-pinned; an empty index suffices), on a child spawned directly by the tool process — full host authority, pre-admission, and a command later refused at bind time has already run its discovery execs. Every discovery git invocation now carries the fixed argv pins (-c core.fsmonitor=false -c core.hooksPath=<null> -c log.showSignature=false), which outrank every config file and need no version floor — so the version gate no longer has to run before the first discovery exec; the raw-shell control proves the hook fires under a plain git invocation and stays silent under the lane, at the discovery level and through the tool. Also from round 10: a-C ~operand was judged by its literal spelling while the shell expands~against$HOME(with a planted literal~directory the resolver admitted a stage git ran outside the workspace — a PR-introduced regression; base judged every token and refused); the cumulative root resolver now refuses tilde spellings like the plain-operand and cd rules, and proves each-Cdestination enterable the way the cd fold does (a mode-000 destination passes canonicalize+is_dir and fails the child'schdir). A FIFO at a discovered metadata path (statreports size 0, so a size cap never fires) blockedopenuntil a writer appeared inside the pre-spawn discovery, where no watchdog bounds anything — pointer files and configs are now read through one bounded reader: non-blocking open on unix, the regular-file check and size cap on the opened handle's own metadata, refusal for anything else (the scan reads each repository's config before spawning its enumeration child, so the git-side block on a FIFO config is unreachable too). The.gitlink strips exactly the one trailing\rgit strips at the newline (a CRLF-terminated link resolves and runs there — probed — while every other padded spelling still refuses); thecommondirvalue is byte-exact with the same strip and must resolve (a padded spelling refuses instead of silently reading a config git never opens). Index gitlink paths carrying"/\\are ordinary filename bytes consumed in-process on unix — scanned and pinned now, where the Windows-quoting reasoning refused them; Windows keeps the refusal (separator syntax there). A single glob-bearing command takes the shell lane — the direct-argv lane handed the program the literal*, so the expansion the operand gate walked could never happen; glob-free singles keep the direct lane. Inline stdin payloads past one pipe buffer (64 KiB) refuse before spawn: delivery is a blocking write under the shell-manager lock, and a non-reading child wedged the executor past any watchdog (the disclosed >64 KiB stall, now closed at the boundary — the schema advertisesmaxLengthand the refusal points at backgroundinteractstreaming; the read-only grammar keeps rejecting astdinfield outright). od-j/-Naccept GNU's multiplicative suffixes with checked arithmetic (-N4k), and head/tail admit GNU's glued numeric value spellings (-n5,-c3) through the row-shared option tables — the parallel auto-approve classifier shares those tables, so its admitted set grows by exactly these spellings, pinned deliberately (the round-4 git hardening flags--no-ext-diff/--no-textconv/--no-show-signatureride the same shared tables; that earlier undisclosed growth is now pinned too). The version-gate refusal copy states what actually fires it (every read-only git stage, not only filter declarations); the version probe is memoized per sanitized PATH (one fork per process, not per command — spawn failures re-probe, deterministic outcomes cache); each config text parses once for both consumers (core.worktreevalidation and the filter accumulation re-parsed the same text twice); the discovered-filter name bound is a named constant; the schema'stest:sentence scopes the read-only shell to in-session delegations like its roster and intro siblings; the glob-budget wall-clock bounds are loosened so a loaded runner cannot redden them; the discovery cd-fold's silent skip on a failed canonicalize carries its cannot-fire-here comment (the gate refuses anycddestination it could not resolve before discovery runs). Disclosed, unchanged:git ls-files --stage -zstill runs once per scanned repository even with zero gitlinks (one discovery fork per admitted git command warm — the version probe is memoized) and a repository whose index listing exceeds the 16 MiB scan bound refuses every git evidence command — both fail-closed costs now stated here. The review's structural recommendation — replace the hand-rolled config parser withgit config --list -zenumeration, keeping the hand parser only as the refusal fallback — is scheduled as the committed follow-up below.Round 11 (fresh 11-lane review: 8 static/probe lanes, one test-execution lane, one mutation re-derivation lane, one merge-order lane) — two new escapes, both reproduced end to end before fixing, plus gate and copy drift. (1) The
.gitgitfile andcommondirreaders cut the link value at the first newline while git consumes the WHOLE file minus one trailing newline — interior newlines are ordinary path bytes, and a directory named with one is legal on POSIX. Proven against git 2.43: a link likegitdir: decoy/.gitplus a second line resolves a gitdir literally named with that newline and runs THAT repository'sfilter.<name>.cleanunder an admitted read-onlygit status/git diff(marker-file proof), while the first-line cut scanned the nonexistent decoy, found nothing, and admitted the read. The readers now consume the whole value — the byte-exact mirror, so discovery pins the filters of exactly the repository git runs (the round-10 padding/CRLF refusals keep their honest pins; the old first-line pin whose justification claimed git refuses such links is rewritten). End-to-endreadonly_shell_lane_git_diff_pins_filters_behind_a_multi_line_gitdir_linkwith a raw-shell control;readonly_git_commondir_follows_the_multiline_value_git_resolvespins the shared-directory twin. (2)git -C <value>operands were judged as literal spellings while the shell expands wildcard (and zsh EQUALS) spellings against the stage's working directory — and-Cvalues are sliced off before the plain-operand walker, so nothing expansion-judged them. Proven end to end with the chained shapegit -C x -C * show HEAD: a literal*directory planted INSIDEx/canonicalizes fine, the expansion binds the second-Cto the first-sorted outside symlink, the leftover entries land in git's subcommand and rev slots (attacker-named tags), and the outside repository's committed content renders; the flatgit -C *shape had only survived-refused by subcommand-slot poisoning, a collation accident. The root resolver now refuses any wildcard and any=-prefixed-Cvalue on the spelling itself (tilde already refused), like thecdand gh-stage rules —readonly_operand_gate_refuses_shell_expanded_git_c_operandspins all four shapes — and the two comments asserting the breached invariant as fact are rewritten. Also from round 11: the env-config version floor moves to 2.32 — theGIT_CONFIG_COUNTfeed needs 2.31, butGIT_CONFIG_GLOBAL(the wall that keeps a repo.gitattributesfrom referencing a global-only filter) arrived in 2.32, so 2.31.x admitted under protection that partially did not take (comparator pin renamedgit_env_config_support_needs_two_thirty_twoand re-bounded); the upstream-CI clippy gate was found RED at the round-10 head (the glued-numeric helper trippedcollapsible_ifatcommand_safety.rs:1397; the body's "only the pre-existing turn_loop.rs:5499" claim was false) and is collapsed with a let-chain — the upstream invocation now reports onlyturn_loop.rs:5499, verified with the exact ci.yml flags; the 64 KiB inline-stdin cap's never-wedge invariant is stated as the Linux fact it is (Windows/macOS pipe buffers may be smaller; the cap stays a strict improvement) with its off-by-one comment fixed and theinput/dataschema aliases advertising the maxLength the runtime enforces; the guidance stdin sentence namesttyalongsideinteractiveandcombined_output; the[shell.readonly.command]copy names the glued/cluster spellings it admits, the gh-stage glob refusal, and the new-Cwalls; refusal messages bound echoed filter names andcore.worktreevalues at 200 bytes (a crafted config could otherwise buy unbounded error text through the symlink-following config reads); thegit ls-filesper-repository fail-closed cost is stated at its call site;splice_git_read_hardeningreuses the one preamble walk instead of a private copy; the verifier background gate jobs' at-spawn stdin EOF gained its pin (run_verifiers_background_gate_jobs_get_stdin_eof_at_spawn— acatgate completes instead of idling to the gate timeout); and the verifier-catalog test whose name denied the shell surface the fix-3 contract grants is renamed.Round 12 (fresh 12-lane review: nine static/probe lanes, one live-E2E lane, one test-execution lane, one mutation lane; both escapes reproduced end to end by the reviewer with raw-git marker discriminators before fixing) — the strip model behind the round-11 mirror was itself an approximation: git consumes a
.gitgitfile/commondirvalue with an UNBOUNDED run of trailing\r/\nbytes stripped, in any order (probe-pinned with a marker-firing decoy discriminator: a value ending in\n\n,\r\r, or\n\rresolves the fully stripped target and runs THAT repository's filter while the armed decoy one byte longer stays cold), while the readers stripped exactly one\nplus one\r— a link one byte past the round-11 fixture sent discovery to a decoy-or-nothing scan while the child git ran the hostile repository. Both readers now strip the run; the padded space/tab refusals keep honest comments (git resolves and runs a repository found at a padded path — probe-pinned — so the refusal is the fail-closed side of a mirror the lane declines to follow); and the consumed value is validated (git fatals on a non-repository, so the lane refuses the same instead of admitting a read whose scan finds nothing and whose child dies at git's fatal). Six comments asserting the false one-strip/first-line model — two inside the round-11 fix itself, one on the rewritten pin's own doc header, one on the scan-cap constant — are rewritten to the probe-true model. (2) The glob..synthesis judged the LEXICAL parent while the kernel resolves the shell's expanded..PHYSICALLY through a followed directory symlink:cat sub/..*/secret_outwithsuba symlink to an outside directory read the outside file on dash (probe-pinned; modern bash leaves..*unexpanded, so the scope is exactly the sh/dash family the synthesis exists to cover) while the gate judged a nonexistent lexical path, called the operand inert, and admitted. The candidate is now the textualsub/..under the walked directory, which canonicalizes exactly where the kernel resolves the shell's word (the round-3 dot-synthesis pin stayed green under the reverted mutation — its fixtures sit where lexical and physical coincide, which is why the shape survived). Also from round 12: the pre-spawn discovery git child is bounded — the enumeration child opens repository files itself, and a FIFO at.git/indexor.git/HEADblocked its own open forever (probe-pinned, timeout-kill exit 124) in the one pre-spawn region no watchdog bounds, where the bounded-reader wall covers only the files the tool opens; a watchdog thread kills the child at a 10 s bound and the command refuses. The interact stdin write is bounded the way the spawn payload is capped: a command that stops consuming stdin filled its pipe and held the shell-manager lock — every other shell in the session with it — forever, and the 64 KiB cap's own refusal copy steered straight into that wall; the write now runs on a worker thread with a 5 s deadline, abandons the payload to the background writer (the bytes still land if the child resumes reading), marks the task's stdin stalled, and refuses further writes (two writers could interleave);interactpayloads also carry the 64 KiB per-call cap; Windows keeps a blocking write under the cap (disclosed — and corrected in round 13: the handle IS cloneable there viaChildStdin::try_clone, so this is a scope choice, not a platform necessity). The round-11 refusal-echo bound was unpinned — a neutering mutation left the whole suite green — and is pinned now with file-sizedcore.worktreeand filter-name values whose position-marked content discriminates a truncated echo. Discovery consumed raw (non-canonicalized) roots and cwd while the gate canonicalized: a declared root spelled through a symlink flipped every-C-bearing git read to a refusal, and a divergent symlink inside the cwd spelling could send git's physical.gitwalk-up somewhere the lane's lexical one did not look; the root set and cwd are canonicalized once before both consumers, pinned through an aliased-workspace tool leg. The config parser carries the continuation state across a pre-section key's trailing backslash so the following section header is swallowed the way git swallows it (probe-pinned) instead of arming a driver git never runs. The parallel classifier's admitted set SHRANK with the shared gh host heuristic — base parallel-admittedgh repo view myhost/o/randgh pr list -R ../secretfor parent auto-approve; both refuse now — a behavior delta the byte-identical source hides, disclosed here and pinned. The absolute-outside-gitdir link leg (git runs that repository's filters; the lane scans and pins it by design) gained its own scan pin; the warm discovery-fork count in this body was one, not two (the version probe is memoized); the[shell.readonly.command]refusal copy now attributes seam-refused shapes to the execution seam instead of the grammar; the internal dispatch message no longer names a Scout shell no verifier holds; the tool description's pipefail list covers the ksh family the predicate admits; and the EOF-ordering comment states the real (infallible) close path.Round 13 (fresh 13-lane review: two boundary red-team lanes with code-executing probes, discovery-subprocess, glob-walker, stdin/interact, a 104-shape config-parser differential, claims-vs-implementation, smuggling audit, pin quality, performance/economy, copy accuracy, merge-order + gates, and a fresh-mutation lane) — one BLOCKER, two MAJORs, fixed here. (1) The walk-up returned on ANY
.gitDIRECTORY while git'ssetup_git_directory_gently_1treats a directory failingis_git_directoryas non-fatal and CONTINUES upward: one emptysub/.gitbetween the stage root and a hostile repository blinded the round-7 filter discovery, and an admittedgit -C sub diffexecuted the ancestor'sfilter.<name>.clean— attacker command execution under ReadOnly with outside content crossing into the output — reproduced end to end through the tool by two independent reviewers and re-run by the coordinator, including the cross-boundary shape (a bogus.gitat the workspace root, hostile repository ABOVE it: a workspace-scoped writer alone triggers it; raw control fires, marker proves execution). The walk-up now mirrors the discovery loop per level: a.gitdirectory is a repository only when the SAME predicateread_gitfileapplies — anis_git_directorymirror built from git 2.43's source (validate_headref: arefs/-prefixed symlink target validates WITHOUT existence,ref:plus anisspaceskip plus arefs/-prefixed name, or a 40-hex detached HEAD; thecommondirpointer resolved with the trailing-run strip and real-pathed;access(X_OK)onobjectsandrefs) — and anything else continues upward, with a.gitsymlink treated directory-transparent like git's lstat-firstread_gitfile. The mirror must be exact because both divergence directions leak: a directory the lane accepts while git rejects stops the walk where git continues to an ancestor whose filters then run unpinned, and a directory it rejects while git accepts walks past a repository the child runs. The old gitfile-branch validity check was an approximation in both directions — admitting garbage-HEAD-text and missing-refs gitdirs git fatals on, refusing a dangling-HEAD-symlink gitdir (git accepts; probe exit 0 with the filter firing) under a false(git itself fatals there)message — replaced by the shared predicate, and the link value now truncates at the first NUL like git's C-string read (gitdir: <target>\0junkruns the prefix repository — probed). Pins: the predicate unit with ten check-by-check legs and a raw-git control on the dangling-symlink acceptance, the bogus-dir walk-up variants (empty, HEAD-only, symlinked, garbage-HEAD — every one resolves the ANCESTOR), both end-to-end leak flips with raw controls, the NUL truncation, and a rename of the padded-commondir pin to the probe-true model plus a follows-a-resolving-pointer twin (a padded value that RESOLVES is followed exactly where git resolves). The remaining walk divergences are comment-disclosed and fail-safe: the bare-repository check (a bare repo converts no worktree content, so no filter can fire), filesystem boundaries (the child env clears GIT_DISCOVERY_ACROSS_FILESYSTEM, so the child may stop where the walk continues — those repositories are scanned, never run), and safe.directory (the child fatals after the scan pinned, failing the command honestly). (2) The Follow-up section's enumeration spelling was include-blind:git config --list -z --locallists onlyinclude.path=..., never the includedfilter.*records (probed;--localalso never readsconfig.worktree), so the follow-up executed as written would have converted today's fail-closed include walls into silent under-scans — the round-7 class behind a green-looking scan. The section now spells the UNscoped--list -zunder the hardened discovery env with origin filtering, and drops the incoherent keep-the-parser-as-fallback clause (when git cannot be invoked, every git evidence command refuses anyway). Also from round 13: the discovery watchdog poll quantum drops 25 ms to 2 ms (measured ~25 ms of join tax on every admitted git command) and is pinned as a production constant, with the bounded-child pin's elapsed window tightened to [9, 15] s;read_bounded_regular_filerefuses a file that grew past the cap between fstat and read instead of parsing a silently truncated text (the one pre-spawn race family member whose failure direction was admission); the gitlink-path refusal echoes go through the 200-byte echo bound — a crafted index gitlink path is bounded only by the 16 MiB listing cap (probed: git accepts a 100 KB invalid-UTF-8 gitlink path andls-files --stage -zemits it verbatim; the unbounded echo bought a ~300 KB refusal) — pinned with the crafted-index fixture;[core "sub"] worktree(git lists it as the inert keycore.sub.worktree) no longer trips the relocation refusal; the glob walker documents and pins the two containment-load-bearing mechanisms fresh mutations exposed — the re-entry dedup's remaining-pattern-length component (dropping it admitscat s/*/z*over a workspace self-link on bash AND dash with the whole suite green) and the.half of the dot-synthesis (dropping it admitscat .*/..*/o.txtone level above the workspace on dash) — and discloses the judged-vs-run symlink-swap race the cd fold already disclosed; the canonical-roots drop direction is pinned; the tool description's routing sentence names glob-bearing singles (the round-10 routing change missed by three rewrites), the guidance stdin sentence states where tty commands actually run and who can write a combined_output terminal's stdin, the[shell.readonly.command]copy scopes the conditional-cdrefusal to later segments (a first-segmentcdunder&&/||is admitted — the classifier's own test says so), the cwd shape copy names the+/-prefixes it refuses, the "run this Scout" remnant is de-roled, the parser doc states its one-directional mirror honestly, and the sub-subagent chain (carve-out Verifier parent → Builder child) gains its no-extra-authority ceiling pin. The body's own numbers were corrected here too: the round-12 head ran 216 shell tests with TEN new pins (not 215/nine), the parallel-classifier growth is-q-only (base already carried--jq), and the fix(subagent): 固定任务身份与完整输出恢复 #124 shared-file count is five.Round 14 (fresh 15-lane review: 13 static/probe lanes — classifier, stdin lifecycle, operand gate + glob walker, git splice + routing, a ~40-construct config-parser differential against git 2.43 probes, walk-up/discovery with git probes, test-role + fleet, test-quality audit, claims-vs-implementation, merge-order, performance/economy, docs/copy plus a per-hunk smuggling audit of all 13 non-big-three files — and two code-executing red-team lanes, 60 compound/cd/glob probes × bash and dash and 26 content-filter topology probes, all with raw-git positive controls, zero escapes outside the findings below, plus coordinator re-verification of every BLOCKER-class claim): two BLOCKER-class walk-up escapes, one more escape the review first failed to reproduce, and two MAJORs, fixed here. (1) The walk-up keyed a
.gitentry on its LSTAT kind: a symlink to a plain gitfile hit neither the directory arm nor thesymlink_metadata().is_file()arm, so the walk resolved NO repository while git 2.43 stats through the link and applies the gitfile — probe-proven end to end (a rawgit statusin a directory whose.gitis a symlink to a one-line gitfile resolved the target repository and executed ITSfilter.<name>.clean, marker proof; the whole fixture fits inside the workspace, the round-13 BLOCKER's own reach) — the walk now keys on the FOLLOWED kind, and the false "a symlink to a FILE is never a gitfile" comment is rewritten. (2) Theref:skip accepted\f(Rustis_ascii_whitespace) where git 2.43 rejects it: a decoy.gitdirectory with HEADref:\frefs/heads/mainpassed the lane's predicate — walk stopped, scan found nothing, read admitted — while the child's discovery rejected the decoy and resolved the hostile ANCESTOR (raw control:git rev-parse --git-dirfrom the decoy names the ancestor); the cross-boundary shape of the round-13 BLOCKER, one byte wide. The skip set is now exactly {SP, TAB, LF, CR}, and the "git skips isspace" comments (wrong in both directions — git also rejects\v) are rewritten, pin doc included. (3) The discovery shard's commondir claim — first reported "unconfirmed" by the review because its debunking fixtures never reached the pointer read (no HEAD, no index, no worktree content) — is CONFIRMED with complete fixtures and fixed: git consumes the value as a C string (an interior NUL resolves and runs the prefix repository, filter fired under a plaingit status) and paths are bytes (a non-UTF-8 byte path names a repository git runs, filter fired), while the lane's whole-value canonicalize failed on the NUL and itsfrom_utf8declined the byte path; both commondir consumers now share one byte-exact pointer reader (strip the trailing run, truncate at the NUL, build the path from raw bytes on unix; Windows keeps the UTF-8 wall, where git itself fatals on such values, so declining stays fail-safe). (4) MAJOR: the unix PTY interact write now rides the 5 s bounded worker like a pipe write — the writer is a dup of the master fd, andtake_writeris NOT called on unix: probe-pinned that dropping its box injects newline+VEOF into the master, an instant EOF for a terminal command that never receives an interact (Windows keeps the blocking write for both handle kinds, disclosed at the site); a close after a stall no longer claims an EOF the abandoned writer has not delivered — the flag stays down and the refusal names the stall. (5) MAJOR: the foreground stdin-EOF close is deliberately pipe-only — a terminal (combined_output) foreground command keeps its PTY writer attached, restoring the base interact surface a session member can write to while the command runs, which the unconditional close had silently destroyed against the guidance sentence that describes it; the detached-foreground EOF contract is unchanged (pipe lane, still pinned). Also here: the pre-spawn canonicalizations moved onto the blocking pool with the gate they feed (cold-cache realpath walks no longer stall an executor thread), the discovery reap pollstry_waitso the watchdog can deliver its kill after stdout EOF, the cmd compound refusal names every refused connector family instead of ";"-separated, the classifier's quoted-separator comment states the seam-rests invariant, the fleet-envelope compound-gh leg names the wall that fires, and the od---handling plus the read-only stdin-field rejection gain their pins. Mutations 3/3 red — walk-arm revert,is_ascii_whitespacerestore, NUL-truncation drop — each reddening exactly its own pins, including the two leak-flip E2Es.Round 15 (fresh 12-lane re-review from scratch after a rebase-freshness check — merge-base == pinvou3-clean tip == base
f1eb10efc, rebase a no-op; lanes: claims, shell core, git area against live git 2.43 binary probes, command_safety, subagent/verifier authority, PTY/stdin lifecycle, test quality, per-hunk smuggling, one code-executing red-team lane, performance, docs/copy, cross-cutting/merge-order; the coordinator re-verified every blocker by direct trace and ran all gates): three BLOCKERs, one MAJOR copy-placement fix, and the missed pins, fixed here. (1)StdinWriter::flush's newPtyFilearm kept the base#[cfg(not(target_env = "ohos"))]while the variant andwrite_all's arm are#[cfg(unix)]: on Windows the arm names a variant that does not exist (E0599 — probe-proven with rustc at the exact attributes forx86_64-pc-windows-msvc) and on the ohos unix target both arms compile out leaving a non-exhaustive match (E0004);ci.ymlrunswindows-latestas a required check andohos.ymlruns a targetcargo checkon everycrates/**PR, so the branch's Linux-only gates structurally could not see either. One-token fix (#[cfg(unix)]), and the two comments claiming the dup's drop "delivers EOF to the slave" / "closes the last master handle" are rewritten (the reader thread's clone is the live master handle; the dup's drop delivers nothing — the pipe-only EOF claim was already honest). (2) The operand gate judged post---tokens with the option grammar's=-suffix rule while the classifier deliberately admits everything after--as a plain operand:cat -- -a=bjudgedbwhile the program opened-a=b— a planted outside symlink spelled-a=bcrossed the boundary under a name the gate never judged (raw-shell control fires, refused now;cat -- filestays admitted; unit + E2E leak-flip pins). (3) The glob walker judged matches by resolved location only: an expansion entry named like a flag lands in an option slot the gate never judged, and because the shell expands*inside the ALREADY-hardened command text, a planted--ext-difffile re-arms the external diffsplice_git_read_hardeningremoved — git's last-flag-wins, and the attribute driver behinddiff.<name>.commandoutranks theGIT_EXTERNAL_DIFFenv pin (control-proven end to end with the exact hardened text under the pin set); a planted--show-signaturere-armsgpg.programthe same way. The walker now refuses dash-named matches outright (gh stages never reach it — their globs refuse earlier; the round-11 option-word union test pins the stronger refusal, and a plain-name outside match still refuses by location). Also fixed: the aligned read-only routing and stdin-contract copy moved onto the model-visiblebashdescription — the round-3 alignment had landed only on the hidden uppercaseBash(model_visible() == false), the model-visible tool still carried the "only pipelines still go through the shell" sentence this lane's own routing falsified, and the catalog pin now pins the visible tool; PowerShell shell-lane compounds refuse,-bearing commands (PowerShell argument mode splits a bare comma into several argv words the gate judged as one —readonly_shell_word's "reads back unchanged on PowerShell alike" claim corrected; per-kind decisions pinned); raw NUL bytes refuse at the charset gate (the gate'sexists()silently skipped them and spawn failed closed with a generic error — the honest answer is the in-policy refusal; pinned); the ZDOTDIR/GLOBIGNORE startup walls pin at a pure tablereadonly_shell_startup_env_walls(the child env is not observable through the admitted grammar — env-reading commands refuse — so the pin lives at the source; the round-14 "gained their pins" od---claim was wrong:od -- fhad no pin anywhere and the stdin-field rejection half was pinned by a pre-existing row — od--admission pinned now and the claim corrected); the post-stall close contract pins (a close after a stall must leave the stall refusal, never an EOF claim — thestdin_closed_at_eofstall conjunct is defense-in-depth under the stall-first check order and the pin reddens a reorder that would surface the false claim); the inline-stdin boundary anchors assert exact equality with the constant (thecontains("65536")anchor survives a 640 KiB cap); the gh host heuristic's..refusal scopes to path-like spellings so a--jq '..'recursive-descent filter admits (pinned;(/[-bearing filters stay charset-refused and a quoted pipe fails closed at the stage split); the round-14 receipt "upstream--all-targetsclippy reports only the pre-existingturn_loop.rs:5499" was FALSE again at the round-14 head —&[outer.clone()]intests.rstrippedclippy::unnecessary_cloneunder the exact ci.yml flags (two errors) — fixed tostd::slice::from_refand the receipt true at the new head; the E2E missing its#[cfg(unix)]gate (..._behind_a_nul_commondir_link, red-by-construction on Windows) carries it now; two DCO trailer/name drifts (db75f6526,dc889a214rewrote as09c405817/de3edb420), a third on6f9b1f271(nowa6df582da), and a glued trailer one03fd90bc(nowafe5aa35b) are repaired by message-only rewrites with byte-identical trees. REFUTED this round, no change made: the shell-core lane's Windows\\.\device-namespace MAJOR — the strip prefixr"\\."is three bytes, every device-namespace spelling keeps a leading\, and the shape gate refuses it fail-closed at head (the refuting implementation attempt was reverted). The red-team lane proved 0 new escapes with executed candidates: hostileBASHOPTS/SHELLOPTSenv againstcat *over a planted dot-symlink (raw control leaks; the child-env allowlist drops both variables independent of the startup pins), a raw NUL inside an operand (admit → spawn fails closed, manager mutex not poisoned), and a workspace-plantedgitshim against the pre-spawn discovery children (std resolves bare program names through the CHILD envp PATH, which discovery sanitizes). Mutations 7/7 red — post---context drop, dash-named wall removal, PowerShell comma wall removal, ZDOTDIR pin emptied, NUL charset drop, visible-copy revert,..heuristic widened — each reddening exactly its own pins.Round 16 (fresh 13-lane review from scratch, rebase-freshness re-verified first — merge-base still == pinvou3-clean tip == base
f1eb10efc, rebase a no-op, and no open fork PR head is an ancestor of the branch so no parent-PR stacking; lanes: claims-vs-implementation (58 sampled claims TRUE / 0 FALSE / 0 PARTIAL), shell execution core, classifier + operand gate + glob walker with a fresh adversarial sweep, a git-area differential against the live git 2.43 binary across ~120 shapes (walk-up predicate, gitfile/commondir readers, a 42-shape config-grammar differential, submodule enumeration,-Cresolution, pin precedence), subagent/verifier/fleet authority, cfg-surface + PTY lifecycle with rustc probes on the ohos and windows targets, test-quality (196 test fns audited), per-hunk smuggling (97/97 hunks), performance/economy, docs/copy, cross-cutting (freshgit merge-treeagainst all 12 open PR heads, DCO 46/46, gate receipts reproduced), one code-executing red team (81 candidates executed end to end through the real tool entry on bash/sh/dash: 1 leak, 1 pre-disclosed find -L residual, 30 refused-at-wall, 25 classifier-blocked, 23 inert, 1 admitted-with-Deny-policy — zero other escapes), and a fresh-mutation lane re-proving 8 load-bearing walls red): four BLOCKERs, fixed in the same wave. (1) The operand gate resolved a git stage's wildcard against the accumulated-Cstage root while the SHELL expands it against its own working directory:git -C .r diff *over a hidden clean repository judged.r's entries while the workspace-root--textconvexpansion re-armed the repository's attributediff.evil.textconvdriver past the splice — git is last-flag-wins anddiff.<driver>.textconvis not env-pinnable — executing attacker-chosen code under ReadOnly with outside content in the output (executed proof, marker + secret-in-output; the round-15 dash-named wall is bypassed purely by retargeting the stage). The walker now judges a git stage's globs at BOTH bases (stage root for git's pathspec resolution, shell cwd for what the shell splices), at both call sites, sharing the one command glob budget, with an E2E leak-flip + raw-shell control and piped AND unpiped unit shapes so neither site regresses alone (both single-site mutations red). (2) cmd.exe tokenizes,and=as argument delimiters when it passes a line to a program (mkdir a,bcreates two directories), socat a,b | wc -lran with argv words the gate judged as one word — planted outside symlinks spelledaandbread unjudged on a cmd host (documented cmd semantics; no Windows host here, the round-5 verbatim-path precedent). The per-kind wall refuses both spellings for the Cmd kind (the admitted grammar carries no delimiter-bearing option, so nothing admitted is lost; quoted occurrences refuse too, mirroring the%wall), the multi-segment refusal copy now states the exact refusal surface instead of the overstated "gets no compound probes", thereadonly_shell_worddoc's false "reads back unchanged on cmd.exe alike" claim is corrected, and the round-15 comma pin — whose cmd leg asserted the false premise (Cmd "cat a,b" → is_none()) — is rewritten for both walls. (3) The round-15 cfg fix leftStdinWriter::PtyFilealive on ohos with its only constructor inside thenot(target_env = "ohos")PTY block: a never-constructeddead_codevariant, fatal under the workspacedeny(warnings)and ohos.yml's-Dwarnings(rustc probe on the realaarch64-unknown-linux-ohostarget — the round-15 red replaced by a new one). The variant, both match arms, and the bounded-write site now carry the sameall(unix, not(target_env = "ohos"))predicate as the constructor; probe-clean on ohos, windows, and the host under-D warnings. (4) Five new tests were red-by-construction off Linux (the round-15 class): two ungated symlink-planting tests fail the windows test-target COMPILE (std::os::unix::fs::symlinkat fn level, E0433 probe-proven), one asserts unix-only gitlink resolution against the Windows refusal its own#[cfg(windows)]twin pins, one executeswc -cthrough the sanitized PATH (absent on windows runners), and one exercises the unix-only bounded stdin write against Windows blocking semantics (30 s guard wedge). All#[cfg(unix)]now, reasons at each site. Also in the wave:od -j/-Nadmit GNU's strtoll base-0 spellings (0x10hex,010octal) the decimal-only parse refused, and the post---corner admissions (a second--as a plain operand, a bare-, an empty operand) are pinned. Verified TRUE and left alone this round: the PowerShell comma wall is complete against every charset-surviving PowerShell shape; post---judgment survives every attack shape tried (second--, quoted--, mid-pipeline/sequence, cd/gh/git post---, dangling--); the dash-named wall is sound against re-arm (its false-refusal families —wc -c ./*refusing./-file— are safe-direction, follow-up); the git-area differential found ZERO escape shapes — its six fail-safe false-refusal findings are the disclosed deliberate walls (padded gitfile targets, the non-UTF-8 gitfile wall vs the byte-faithful commondir reader asymmetry, include/includeIf, worktreeConfig, core.worktree without repositoryformatversion, unspellable filter names);find -Loutside-name listing re-confirmed as the pre-disclosed residual; authority (clamp→capture→narrow ordering, chains, receipts, per-stage gh predicate in both pre-checks) clean; smuggling CLEAN; DCO 46/46; all gate receipts reproduce (fmt clean, fork-CI clippy clean under its exact flags, upstream--all-targetsclippy reporting only the pre-existingturn_loop.rs:5499). Disclosed to the follow-up list below: the discovery read loop never re-checks its watchdog flag (a grandchild holding the killed child's stdout could out-wait it — unreachable under the pinned config today), the version probe runs unwatchdogged (trusted binary, memoized), discovery has no per-command aggregate budget (32 repositories × 10 s watchdog ≈ 5 min fail-closed worst case), andBackgroundShell::drop's PTY arm waits unbounded where the Process arm is bounded. Mutations 3/3 red (piped git site → unit piped shapes; unpiped git site → unit + E2E leak-flip; cmd delimiter wall → the rewritten per-kind pin), each reddening exactly its own pins.Round 17 (fresh 13-lane review from scratch, rebase-freshness re-verified first — merge-base ==
pinvou3-cleantip == basef1eb10efc, rebase a no-op, base unmoved, fix(tui): repair residual model-facing copy (2026-10-07 audit batch) #136/chore(deps): bump undici 7.30.1, msal-node 7.1.2 #135 open and not in main; lanes: shell execution core, classifier/operand-gate/glob walker, a git-area differential against the live git 2.43 binary, subagent/verifier/fleet authority, cfg-surface with rustc probes on the installedaarch64-unknown-linux-ohosandx86_64-pc-windows-msvctargets, claims-vs-implementation (~55 claims sampled), test-quality, per-hunk smuggling (98 hunks), performance/economy, docs/copy + zh/en parity, cross-cutting (merge-tree vs all 17 open PR heads, DCO, live CI state), one code-executing red team (~96 candidates executed end to end through the real tool entry), and a fresh-mutation lane (10/10 red)): two BLOCKERs and one MAJOR, fixed in the same wave. (1)BackgroundShell::write_stdin's ohos fallback was the ELSE ARM of the#[cfg(all(unix, not(target_env = "ohos")))]bounded-write statement, and the second fallback was#[cfg(not(unix))]— both strip on ohos (unix = true,target_env = "ohos"), leaving NO write on ohos: thestdinbinding unused, thePipefield unread, theStdinWritermethods uncalled (all three fatal under the workspacedeny(warnings)on ohos.yml'scargo checklane — probe-verified on the real target with three diagnostics; the round-16 audit covered the variant sites, one level above the stripped statement), while interact/spawn stdin payloads were silently discarded withOk(())— the in-code claim that ohos "falls through to the blocking write below" was false. The fallback now carries#[cfg(any(target_env = "ohos", not(unix)))]; spliced-verbatim probes compile clean on ohos, windows-msvc, and the host under-D warnings. (2)validated_core_worktreevalidated the RAW first physical line of acore.worktreevalue while git applies the continuation-joined, escape-decoded value (probe-proven on git 2.43): a workspace-internal hostile repository declaringworktree = w\continued over a second line — with a planted.git/w\symlink making the raw value canonicalize back to the discovered worktree — relocated the child's worktree outside the workspace past the wall, and an admittedgit statuslisted the outside victim's files under ReadOnly (coordinator-replayed). The parser now flags continued settings (continuedonParsedGitConfigSetting) and the validation refuses them; pinned by a parser unit, a plain refusal case, a unix discriminating discovery fixture whose admission twin keys on the continuation itself (not the link spelling), and an end-to-end leak-flip with a raw relocation control. (3) MAJOR: the round-16 git-stage-glob E2E's raw-shell control ran under the ambient locale — under a strcoll locale bash sortsseed.txtbefore the planted--textconvandgit diff <path> --textconvfatals ("must come before non-option arguments"; probe-proven), reddening the control on any host whoseshis bash (macOS runners); the control now pinsLC_ALL=C. Also fixed here: the attached-path option wall no longer fires past--(cat -- -a/bis a plain post-terminator operand judged by resolved location — the classifier admits it; both directions pinned); the 10 s glob-deadline magnitude is pinned like the sibling watchdog quantum; the quoted-glob-value over-walk (option-table-blind seam, safe direction) is documented at the walk site; the hidden read-onlyBashdescription now names the cmd,/=walls and scopes the glob-expansion sentence; the PowerShell/cmd delimiter refusal messages hedge the quoted-occurrence fail-closed shape. Verified TRUE and left alone: zero new boundary escapes (round-16 fixes re-verified live across 16 dual-base shapes incl. cd-fold variants), smuggling CLEAN (98/98 hunks, one comment-only nit now claimed here), authority clean (no path builds the carve-out without the clamp), performance clean (dual-base walks fire only when-Cmoves the base, sharing the one budget; canonicalizations all in the blocking pool), all five gate-receipt counts reproduce exactly (the lanes' staticforkguard_243 over-count REFUTED by measurement: 240/0), ~55 sampled claims TRUE with no overclaim, merge-order recipes still byte-correct at today's heads (fix(subagent): release interrupted claims and inherit worktree isolation #123's head moved to6c40e3bb21f0with the identical three-file conflict shape; fix(tui): repair residual model-facing copy (2026-10-07 audit batch) #136's single-literal conflict unchanged at7da8df2a5f6f; fix(tui): widen local-endpoint SSE open timeout to 180s #130–chore(deps): bump undici 7.30.1, msal-node 7.1.2 #135 and upstream refactor(skills): export install safety primitives #81/feat(memory): refuse sensitive notes at write time #82/feat(automation): archive terminal runs on delete #85/feat(execpolicy): builtin sensitive-path safety ruleset #87 all merge clean), zh/en parity holds, and the CI zombie-queue disclosure stands (still zero runs at head). New follow-ups disclosed: the fleet-durable Verifier catalog listsbashwhile the envelope refuses every call (pre-existing, fail-closed); the sub-subagent chain pin models the derivation rather than executing the spawn seam; the network-guard spawn ordering has no tool-entry-level pin; the stalled-tty bounded-write test's 15 s upper guard has 3× headroom over the 5 s deadline and flaked once under a heavily loaded host (passes 3/3 alone); the reviewed state's head commit carried aSigned-off-byidentity differing from its author (advisory gate passes; normalize at squash). Mutations: neutering the continued-value wall reddens exactly its two fixtures; removing the attached-path guard reddens the post---admission legs; the fixedwrite_stdinregion probe-compiles clean on all three targets (the unfixed text failed ohos with three diagnostics).Round 17b (follow-up wave on the same review, triaging the deferred minors): the valuable ones landed. (1) The classifier-bounded probe lane admits two shapes real tools accept that the row tables refused — POSIX getopt clusters for wc's all-switch row (
wc -clis exactlywc -c -l; a letter that is not an admitted switch still refuses exactly as the tool itself rejects it) and GNU grep's numeric context shorthand (grep -3 pat fis exactlygrep -C 3 pat f, rewritten to the exact-match form in the option context only — past--a-3is a file name and stays an operand; rg does not accept the spelling and its row does not gain it). Both ride the shared row table, so the parallel auto-approve classifier's admitted set grows by exactly these spellings — that delta is disclosed here and pinned on both classifiers. (2) The evidence catalog no longer advertises thebashlane to a worker whose envelope grants no shell authority (enforce_tool_authorityrefuses every bash call forshell != ReadOnly, so the fleet-durable verifier was shown a surface it could never use); a read-only-shell envelope keeps the lane, pinned both directions. (3) The network-guard's before-spawn ordering gained its tool-entry leg: under a Deny policy an admitted gh read refuses with the policy error insideexecutewhile a non-gh read runs untouched; the gh shape parses--limitclient-side, so even the dropped-guard mutant path fails at argument parsing and can never reach the network. (4) The round-16 git-stage-glob E2E now drives BOTH call sites at tool level (unpiped and piped-with-wc), so neither site can regress behind the unit pins alone. (5) The bounded stdin-write tests' upper guards were REMOVED rather than widened — the load-bearing assertions are that the deadline fired (elapsed ≥ deadline) and further writes are refused; a fixed upper bound proved load-fragile at any multiple (3× flaked once under a 13-lane-loaded host, 6× starved twice under back-to-back saturated full-suite runs), while a broken-mechanism mutant never returns and trips the 60 s outer hang guard instead. (6) The79d72fb78head's mismatchedSigned-off-byidentity was repaired by a message-only rewrite with byte-identical trees (the round-15 trailer-repair precedent): the branch is now1f26458c1(round-16 wave, tree82002b12a, trailerasto18089 <asto18089@users.noreply.github.com>matching its author) →24e67da67(round-17 wave, tree4e565f224) → the round-17b wave; the pinned round-16/17 review receipts remain content-valid, with all SHAs they cite now superseded by the tree-identical rewrites. Deferred with reasons, recorded for follow-up: the test-suite dedup and the legacy cfg-gate reason comments (churn in the fix(subagent): release interrupted claims and inherit worktree isolation #123 conflict surface), the discovery double-parse and the per-entry star-match allocation (bounded economy below the noise floor),head --lines=3(the no-equals grammar is a documented deliberate shape), the empty-named[filter ""]wall (exotic, fail-closed), the sub-subagent chain pin through the real spawn seam (the threaded runtime is unobservable without a real grandchild spawn — follow-up), and the 19 long commit subjects (no gate exists; the parent squash collapses them). The 7-lineagent_readonly_bash_inputcomment translating legacyjustification/sandbox_permissionscalls into a deliberately-refused shape is claimed here (review-round-17 minor 11: real and desirable, previously unclaimed).Round 18 (fresh 11-lane review from scratch, rebase-freshness re-verified — merge-base
f1eb10efc== base tip, no parent stacking; an executing red team ran 61 tool-boundary probes, 60 fail-safe): three BLOCKERs and one MAJOR, fixed here. (1) The#5595absolute branch canonicalized an absolute operand's LITERAL spelling and skipped the glob walk, socat <ws>/x*with a plantedx*-named entry (glob bytes are legal filename bytes) was admitted while the shell expanded the word onto an outside symlink — executed leak (raw controls read the outside secret throughcat,head, andod); without the planted literal the spelling still refuses unresolved (the literal is load-bearing) and a plain absolute glob stays admitted. Absolute glob-bearing operands now walk the glob-bearing suffix from the literal's canonical parent through the same judge — resolved location, dash-named wall, shared command budget — a relative operand's matches get (readonly_operand_gate_judges_absolute_glob_expansions, end-to-endreadonly_shell_lane_refuses_an_absolute_glob_whose_expansion_reaches_outside). (2) shlex — the tokenizer the classifier and the seam share — drops a#-word-start run to end of line as a POSIX comment, socat f # x | wc -lwas judged as the wordscatandfalone while cmd.exe, which has no comment syntax, passes#andxas live argv words the gate never judged (a planted outside symlink named#opens; the mirror image of the,/=delimiter wall this branch already carries). The cmd lane now refuses any occurrence of#; the POSIX and PowerShell lanes honor#comments and run LESS than the gate judged, so no wall there (readonly_compound_refusal_names_the_cmd_comment_wall— refuse-on-cmd and admit-on-POSIX both pinned). (3) the shell tests'git_helperwas not cfg-gated while all four of its callers are#[cfg(unix)]fixtures — dead code on the windows test lane, whichRUSTFLAGS: -Dwarningscompiles (Test (windows-latest)is a required check; probe-proven on x86_64-pc-windows-msvc). One#[cfg(unix)]. (4) MAJOR: the Windows/ohos interact fallback wrote deadline-free whileexecute_interactheld the session shell-manager mutex, so an interact payload above the ConPTY input buffer against a task that stops consuming wedged cancel/kill/wait process-wide. Those platforms now bound by ownership — the portable-pty writer box exposes no cloneable fd, so the worker takes the writer, finishes the payload, and hands it back; a deadline timeout leaves the writer with the abandoned worker (the bytes still land if the command resumes), marks the stall, and returns with the lock released — probe-compiled-Dwarnings-clean on all three CI-gated targets and exercised on the host with consuming and non-reading children (bounded_stdin_write_owned_returns_the_writer_and_times_out_clean). Also here: the unix bounded write distinguishes a panicked worker (a writer failure, reported honestly) from a stall (the flag refuses further writes); git-filter discovery gains the aggregate wall-clock budget mirroring the glob walk's (the per-repo count and per-child watchdog bounded scans individually but not together — 32 slow repositories were minutes of pre-spawn host work; pinned with an injectable deadline); count options validate their separated values like their glued spellings (head -n foorefuses,head -n 10admits; range and string-valued options unchanged); the parallel classifier's charset refuses NUL like the agent charset's; the four fixed spawn-env overrides come from one pinned fn (a dropped insertion site is dead code, a changed value reds the pin); the inline-payload guidance scopes end-of-file delivery to non-combined_outputforeground runs; the verifier intro and the en/zh docs state the probe lane's grant conditionality (the reviewer row drops a bounded-verify promise no surface grants); the FIFO discovery test drops its load-fragile upper bound (round-17b precedent); the chmod-000 fixtures skip under a root runner; the verifier probe asserts wc's count as the first field, not a substring; stale comments updated (the posture carve-out includes the verifier, the discovery env arming applies to children that run before the version gate) and the walker threat model names the hardlink residual it shares with the CONFIGURATION.md disclosure. Mutations 4/4 red — the absolute-glob walk neutered, the#wall dropped, the aggregate check dropped, the separated-value validation dropped — each reddening exactly its own pin, and the review's leaked shapes now refuse at the tool boundary.Round 19 (2026-11-04, a from-zero re-review of the SPLIT slice with 8 lanes: command_safety grammar, operand-gate/seam/routing, git hardening, split-fidelity+smuggling, test quality, an executing red team (~190 probes, 0 escapes), performance, body/merge-order/CI): one P1 and a set of body corrections, all fixed here. (1) P1:
validated_core_worktreevalidated the RAWcore.worktreespelling while git escape-decodes QUOTED values ("w\nx"chdir's to a name containing a real newline — probe-pinned against git 2.43; unquoted values keep backslashes literal) and strips the quotes, so a quoted-escaped value plus a decoy link named with the quote characters itself canonicalized the raw spelling back to the discovered worktree while git relocated through the decoded name to an outside symlink andgit status -ualllisted outside files — the same raw-vs-applied divergence class the round-17 continuation fix closed, one decode step deeper. Any\or"in the raw value now refuses (readonly_filter_discovery_refuses_an_escaped_core_worktree_value+ end-to-end leak-flipreadonly_shell_lane_refuses_an_escaped_core_worktree_relocation, whose raw-shell control proves the relocation live before the wall stands; mutations 2/2 red on exactly these pins; the unquoted escape-free sibling keeps admitting with the decoy links present). (2) Split fidelity: slice 3's head tree is bit-for-bit the pre-split archive tree and every pre-split test absent from this slice lives on a sibling slice — nothing dropped, nothing smuggled (all 6 files' behavioral hunks map to body claims; the only unclaimed items are the 571 blank-line separators this slice strips and restores at composition, plus one orphaned section banner). (3) Test audit found no fake pins, no weakened coverage, no lost tests; the receipts' "1 ignored (pre-existing)" is accurate (the Linux-visible ignored exporter exists on main). (4) Performance: ~0.2–0.5 ms per admitted probe, pre-spawn scans bounded and fail-closed (the walk-up deadline residual stays disclosed under Follow-up), nothing on the non-read-only path; the Follow-up section's stale "≈5-minute worst case" figure is corrected below (the aggregate budget it predates is in this tree; honest worst ≈10–20 s). (5) Body corrections below: the fix(subagent): release interrupted claims and inherit worktree isolation #123/fix(tui): repair residual model-facing copy (2026-10-07 audit batch) #136 conflict shapes refreshed at the round-19 heads, the checklist docs item re-scoped to slice 3, the fix(fleet): the test role states its real contract #140 placeholder filled, the CI receipt updated (the fork CI ran green on the pre-round-19 head after the round-16 zombie-queue note, and re-runs on this head).Round 20 (2026-11-05, a from-zero 9-lane re-review of the rebased slice: classifier grammar, execution seam, test quality, an executing red team (187 probes, 0 escapes, bash + dash children against hostile git/filter/textconv/worktree/commondir forests with raw-shell controls), performance, body-vs-tree, authority surface + refusal copy, gates, and rebase fidelity after the stack moved onto fix(subagent): release interrupted claims and inherit worktree isolation #123/fix(subagent): 固定任务身份与完整输出恢复 #124-included main): two P2s found and fixed in the wave — (1)
BASHOPTS/SHELLOPTSwere not in the spawn-env overrides while bash honors an inheritedBASHOPTS=dotglobat startup, defeating the round-6GLOBIGNORE=""wall (operator-env precondition, same class as the existing pins); (2) the three PR-introduced mode-000 skip notes tripped the crate-leveldeny(clippy::print_stderr)under--all-targetsclippy — both fixed in commit8f98b3e895with the table pin widened to six keys and one shared#[allow]ed skip-note helper, plus the rebase's duplicated bounded-stdin doc comment removed. Verified good: every prior round's closure held (the round-19 escape-decode wall re-verified live at pin and E2E level with its admission twin); the registry-side per-stage gh predicate swap is confirmed defense-in-depth (compounds refuse one branch earlier at the arbitrary-execution wall, and the load-bearing consumers carry this PR's own per-stage pins); quoted-separator mis-splits remain classifier-admit/seam-refuse with parity proven; perf bounds re-measured (worst admitted pathological glob 10.4 ms, worst discovery 18 ms, common-path delta <0.1 ms); no fake pins, no dropped coverage, receipts exact. Open items: fix(fleet): the test role states its real contract #140 must be rebased onto this head before it merges; the body corrections above are applied in this update.Round 21 (2026-11-05, a from-zero 9-lane re-review at the re-aligned stack: classifier grammar, execution seam, authority wiring (registry envelope + subagent precheck + delegation clamps), split-fidelity + smuggling, test quality with a fresh mutation battery, an executing red team (283 fresh probes across bash/dash/
Sh-fallback against hostile git/filter/textconv/worktree forests with raw-shell controls: 0 escapes), performance (head-vs-base measurements), body accuracy, static gates): one P1 and body corrections, all fixed in the round-21 wave6386b876b. (1) P1 — silent coverage loss; this corrects round-19 item (3) and the split-time note above: the round-20 rebase resolution oftools/subagent/tests.rsdropped six merged-fix(subagent): release interrupted claims and inherit worktree isolation #123/fix(subagent): 固定任务身份与完整输出恢复 #124 test fns (same_worktree_disjoint_sibling_blocks_unbounded_shell,same_worktree_ancestor_chain_keeps_unbounded_shell,same_worktree_gate_ignores_settled_and_foreign_worktree_peers,worktree_discovery_never_adopts_or_enumerates_an_ancestors_sibling_repos,normalize_claim_path_folds_backslashes_into_the_ledger_frame,scoped_mutation_paths_accept_absolute_targets_inside_the_workspace) plus themod identity_selection;declaration — orphaning the still-presenttests/identity_selection.rs(11forkguard_agent_identity_*tests, byte-identical file, no longer compiled anywhere in the stack). All six exercise surfaces this slice does not touch; the wave restores them union-keep-both (the resolution this body already documents for this file) and all 18 restored tests pass, proving pure coverage loss rather than forced adaptation. The wave also re-pins theDo not retry variationsguidance fragment (the one fragment the deleted enumeration pin carried that the replacement dropped) and corrects thereadonly_shell_bound_commanddoc comment (the pipefail prefix keys on a plaincontains('|'), so an||sequence receives the harmless no-op prefix too). (2) Performance re-measured head-vs-base: the round-19~0.2–0.5 ms per admitted probefigure holds for non-git probes (13–20 µs pre-spawn measured — better than claimed) but understates git-stage probes: discovery spawns onegit ls-files --stagechild per distinct repository, ≈3.4 ms/repo warm (32-repo cap ≈ 111 ms; the 10 s aggregate wall and per-child watchdog bound the pathological case). (3) Safe-direction P3s disclosed, not changed: GNU+Ncount spellings (tail -n +5) now refuse under the digit-validation class round 18 disclosed;git status -uall,grep -r/-rn/-e/--include, anddu -shrefuse (availability;du -s -h,git status --porcelain,rg -g,git grep -ecover the need);git worktree listprints operator-registered linked-worktree paths (the documented name-only disclosure class). (4) The fresh mutation battery (6 production mutants) all turn their pins red. Receipts below.The
[shell.readonly.command]refusal copy (tools/subagent/mod.rs) is rewritten to enumerate the admitted families, the every-segment-must-admit rule for sequences (with the segment cap interpolated from its constant), the refused shapes, and the no-retry instruction.The parent-side parallel auto-approve classifier (
is_parallel_readonly_command) is the same classifier shape and pinned (parallel_classifier_still_refuses_compound_cd_and_od_forms); its admitted set did grow by gh-qreads — the shared gh option table already carried--jqat base, so only-qis new (precision fixed in round 13) — that behavior delta is deliberate and pinned byparallel_classifier_admits_gh_jq_reads_through_the_shared_gh_table(round 6); the classifier's complete admitted-set delta across this branch is the one disclosed where it landed — the glued head/tail spellings that grew it (round 10, pinned), the gh host heuristic that shrank it (round 12, pinned), and the round-5 unix removal of the verbatim-prefix strip that shrank it for\\?\-bearing words (the transform is acfg!(windows)no-op on unix, so base's parallel-admittedcat \\?\secretnow refuses at the parallel charset gate; disclosed here after the round-17 lane found the summary sentence omitting it) — and this sentence defers to those disclosures rather than repeating a stale summary of it.3. the
testrole's honest contract — moved to slice 3 (#140): the role predicates, registry carve-out, receipts label, and role docs live there.Merge order
(Historical — #123/#124/#129 are merged into
pinvou3-clean; the recipes below were applied in the round-20/21 re-bases and verified in-tree.) Against #123 (head71cc26218fat the round-13 wave head; it has moved repeatedly —71221288dat round 10, force-pushed past2071e94cd, re-verified each wave, latestgit merge-treere-run at the round-13 wave head): every shared file auto-merges except THREE files. (1) One string literal — the[shell.readonly.command]refusal copy, which #123 also rewrites. The two rewrites are semantically contradictory (its copy says "Not admitted: cd, &&, ;"; this branch admits all three and adds od/gh to the enumeration, and interpolates the segment cap and states the cd-is-a-directory and connector rules). Whichever PR merges second must hand-union the copy with the other's families — #123's tests pin SIX fragments (Admitted:,git branch (bare/flag-only,git worktree list (flags only),find (no -delete/-exec/-execdir,comm, npm view,Do not retry variations) of which this branch's copy satisfies only the last, and this branch's pins (Admitted families:,at most 32 segments,must be a directory the shell can enter,recursive \/`, "joined by&&/||",every segment of a sequence must individually be admitted) sit insubagent/tests.rs, which auto-merges and judges whichever copy wins — picking a side provably reddens the other side's pins. (2) **The Git hardening table** inshell.rs: #123 addsrev-parse | branch | worktreeto the no-splice arm of the old inlinematch, while this branch extracts thatmatchintosplice_git_read_hardening(returning whether anything was spliced) and movesblameto the spliced set with--no-textconv. Resolution is mechanical:"status" | "ls-files" | "grep" | "rev-parse" | "branch" | "worktree" => Ok(false)next to theblamearm. Note that #123's accompanying comment saysblamespawns no configured helper — round 4 proved otherwise (git blameapplies textconv by default), so the comment must not survive the union. (3) **tools/shell/tests.rs** — #123's kill-path/claim-liveness test block and this branch's round-5 operand-seam pins are inserted at the same anchor; resolution is mechanical, keep both blocks. Against #124 (97a2660, five shared files, all auto-merging), #127 (f88448b), #128 (7fe08d5), and #129 (f83e8cd): merge-tree clean at the round-11 wave head, re-verified 2026-10-31 against every head OID quoted above, and re-verified again at the round-12 wave head (db75f65) and at the round-13 wave head (7cb6ece), and at the round-14 wave head (d3d2262) — #123 still conflicts in exactly the same three files (shell.rs, shell/tests.rs, subagent/mod.rs; its head has since moved forward to3949470with the identical conflict shape and resolution recipes, merge-tree re-verified fresh at the round-14 wave head), #124/#127/#128/#129 still merge clean (at97a2660#124 shares FOUR files with this branch, all auto-merging — the FIVE figure this sentence once carried was itself the stale one; the closing clause was already right) (the mid-review corrections that said #128/#127 sharesubagent/mod.rs+subagent/tests.rswent stale themselves — at the cited heads #127 touches onlysession_manager.rsand #128 shares zero files with this branch, and #128 is a draft on a fork branch; #124 shares four files, all auto-merging; #129 shares none). Two notes on the #123 union: it is load-bearing, not cosmetic — #123's classifier admitsgit rev-parse/branch/worktree list, and without the extendedOk(false)arm those reachsplice_git_read_hardening's_ => Errand are refused on both lanes despite admission; and the refusal-copy union must keep the phrases this branch now pins (Admitted families:,at most 32 segments,must be a directory the shell can enter,recursive `/`, and "joined by&&/||") alongside the three fragments #123's tests pin. And as of the round-16 wave (2026-11-03), **#136** (fix/model-facing-copy-residue, opened 2026-11-02, after this section's last sweep) conflicts with this branch intools/shell.rs— both PRs rewrite the model-visiblebashtool description literal; a hand-union of the two copy rewrites, simpler than the #123 recipe (and whoever unions must take this branch's routing sentences: #136's side still carries the stale "(only pipelines still go through the shell)" and drops the stdin-EOF sentence). Re-swept at the round-17 wave head (landed as64abd6c, content-identical tree4e565f224to the localc5d898383, via the Git Data API during a git-443 outage): #123's head has since moved to6c40e3bwith the identical three-file conflict shape — both recipes re-verified byte-correct there, including the disputed blame-spawns-no-helper comment still present on #123's side — #136's conflict is unchanged at7da8df2(tools/shell/tests.rs` auto-merges), and #130–#135 plus upstream #81/#82/#85/#87 all merge clean with no shared conflicted region.Split-time note (2026-11-04, updated at round 20): during the round-20 review the stack was rebased onto
pinvou3-cleanwith #123 and #124 merged in: slice 1 (#139,fix/shell-stdin-eof) is now FOUR commits ending at9148c1785f, and this PR's head moved from the round-19 head3406aeb30(treed83ee5291a8d) tod6d41439d8, then the round-20 fix wave landed8f98b3e895on top. The round-20 fidelity lane compared every slice-2-added function between the two heads: 239 total — 238 byte-identical, the one delta thesplice_git_read_hardeningarm extension #123's classifier mechanically requires (#123 admitsgit rev-parse/branch --list/worktree list; the no-splice arm must list them or every such probe dies fail-closed at the seam), pinned both directions byreadonly_argv_executes_the_git_orientation_probes_without_a_shell; the #123/#124 hunks land as insertion-only keep-both blocks, exactly as this section predicted. All eight round-19-verified critical elements (escape-decode wall, refusal copy, per-stage gh predicate, spawn walls, caps, per-kind walls, splice table, sequence splitter) are byte-intact at the new head. Merge order is slice 1 → this PR → slice 3 (#140, based on this branch). Round-19 merge-tree sweep at the round-19 head: #123 (6c40e3bb21) conflicts in FOUR files —shell.rs,shell/tests.rs,subagent/mod.rs, and now alsotools/subagent/tests.rs(the three-file shape quoted below predates the split; the fourth file's hunks are #123-side insertions of comment+test blocks against empty slice-2-side lines — resolution keeps both sides); #136 (f989796f1f, moved again) conflicts intools/shell.rsAND nowtools/verifier.rs(its head moved past7da8df2a5/a4ac4ab7058and picked up a verifier.rs hunk — its union recipe below still applies for the shell.rs copy, and its verifier.rs hunk needs the same hand-union treatment); the direct merge into thepinvou3-cleantip is textually clean but violates the declared stack (slice 2 assumes slice 1's stdin contracts). Slice 1 and slice 3 re-sweep their own shared-file hunks as their round-1 step. Round-20 correction: #140's branch has NOT followed the rebase — it still sits on the superseded round-18-era stack, so its "based on this branch" claim is false until it is rebased onto this PR's head (8f98b3e895or later); merging it as-is would resurrect the pre-round-19 slice-2 state and drop the escape-decode wall from the combined tree. This was resolved mid-round-21: #140 re-based onto this PR's round-20 head (1f9635a894, its head6e2ed34fb8). Round-21 update: the whole stack was then re-aligned ontopinvou3-clean11cba403dc(five main commits landed since the round-20 base: #129, #131, #133, #134, #135 — the last four touchtools/subagent/, overlapping this slice's files): slice 1 re-based as FOUR commits ending at8c6c00dc0c(delta bit-identical), and this slice re-based as7b754c7df+ the round-20 wave1f9635a894(delta content-identical to the round-20 stack — the only divergence is one cosmetic blank-line-removal hunk intools/subagent/tests.rsthat #131's inserted test fn makes unpreservable; union kept both sides). The round-21 fix wave then landed6386b876b(treebc3415f3c9) — see Round 21 below. The round-20 fidelity sentence above ("the #123/#124 hunks land as insertion-only keep-both blocks") is corrected by round 21: the round-20 resolution dropped seven merged-#123/#124 test fns plus themod identity_selection;line (see Round 21), and #140 must follow the round-21 wave head before it merges — the one open stack action, again.Checked and left alone (from the same transcript)
messageto an interrupted child ("only running children accept messages") andfollowupto a completed child ("cannot resume"): real gaps, deliberately not fixed here — fix(subagent): 固定任务身份与完整输出恢复 #124 is actively refactoring exactly these functions (queue_running_parent_message,followup_child,resume_from_checkpoint_with_policy→_by_idwrappers) and a colliding change would force a painful rebase. Design sketch for the fix(subagent): 固定任务身份与完整输出恢复 #124 follow-up: admitmessageforInterrupted(_)children with a continuable checkpoint; drainqueued_mail[old_id]into the resume atresume_from_checkpoint_with_policy(resume re-dispatches under a new id) and append the notes tobuild_resume_promptafter the conversation tail, oldest first, followup text last; keep terminal statuses refused. The related P1 the transcript did reproduce —followupon an interrupted child contending with its own parked write claim — is already fixed by fix(subagent): release interrupted claims and inherit worktree isolation #123 and is not re-fixed here.write_rootsare disjoint: by design; the contention receipt already names the three recoveries.find -L/find -followlist outside filenames (never content) through an in-workspace directory symlink: a single admitted command that predates this PR, left for a follow-up. (The git-compound splice gap that used to sit in this bullet was closed in round 4, above.)npm view|show|info(admitted on base, byte-identical here) checks only its subcommand:--registry <url>or--userconfig <file>— and equally a project-level.npmrcinside the workspace — redirect the registry, and the gh network guard does not see npm — so anetwork_tool = falsechild can also run the URL-freenpm viewat all (the round-4 network ceilings key on gh reads only; round 6 review). A flag pin alone would be false security given.npmrc, so this is left for a follow-up that splices a pinned registry and userconfig into npm reads, consults the network decider for the registry host, and gates the arm on the caller's network posture.runtime_profile(pre-43cfb6bcd) is reconstructed from the role default on load, so atestchild whose original parent was read-only regains the Run surface when resumed in a Full Access session. This equals the base behaviour (a Verifier with a Full shell always held Run there) and every record written since the field existed stores the clamped profile; left alone as base-equivalent and shrinking, noted for a follow-up that reconstructs under the resumer's ceiling.normalize_windows_command_pathsand the verbatim-prefix strip; its judged-vs-run divergence for\\?\spellings (cmd.exe/PowerShell wildcard semantics) was not exercised — no Windows host in this review.ls -l dangling(round 14; inherent to expansion-based judgment, left alone).#comments pass the charset (cat #fwedges on stdin on the background lane), and its admitted stdin-reading siblings wedge the same way —cat -, a baregrep pattern, a baresed -n 5p, a bareodwith no operand all block on the background lane's held-open stdin until the 120 s watchdog; a hard link inside the workspace to an outside inode reads outside (needs a writer to create);ls -l danglingdiscloses an outside target name; zshNOMATCHaborts a zero-match compound opaquely (safe direction).testchild) holds no extra authority by code path — moved to slice 3 (fix(fleet): the test role states its real contract #140): thederive_childre-clamp mechanism and thesub_subagent_chain_of_a_carveout_verifier_holds_no_extra_authoritypin (round 13; the stale "no dedicated pin" half corrected round 15) live there, not in this slice's tree.Testing
Local toolchain stable; warm target reused via the same
Cargo.lock.cargo fmt --all -- --check— clean.cargo clippy --workspace --all-features --lockedwith the fork-CI allow list — clean, andcargo clippy --workspace --all-targets --all-features --lockedwith the upstream CI allow list now reports ONLY the pre-existingcollapsible_ifatcore/engine/turn_loop.rs:5499(base code this branch does not touch; round-20 update: at the rebased head two further pre-existingcollapsible_if—tools/verifier.rs:587, andtools/subagent/mod.rs:8036carried in with the fix(subagent): release interrupted claims and inherit worktree isolation #123 rebase — plus, before the round-20 wave, a PR-introducedclippy::print_stderrtrio inshell/tests.rs, fixed there with one shared#[allow]ed skip-note helper; no warning points into slice-2-owned logic). The round-9 review caught this branch's own cwd-foldif letpair tripping that lint under the upstream invocation (whose allow list deliberately excludes it); collapsed in the round-9 wave. The round-11 review caught the claim being false again at the round-10 wave head — the glued-numeric helper tripped the same lint atcommand_safety.rs:1397— collapsed with a let-chain in the round-11 wave; at the wave head (f65a0d1ac) the upstream invocation reports onlyturn_loop.rs:5499, verified with the exact ci.yml flags; re-verified at the round-12 wave head (db75f6526) with the exact upstream flags (only the pre-existingturn_loop.rs:5499) and the exact fork-CI flags (clean). The round-14 head's receipt was false again —&[outer.clone()]intests.rstrippedclippy::unnecessary_cloneunder the exact upstream flags (the round-15 review caught it) — fixed tostd::slice::from_ref; at the round-15 wave head (922c286ec) the upstream invocation reports only the pre-existingturn_loop.rs:5499and the fork-CI invocation is clean, both re-verified. Round-21 update at the re-aligned head (6386b876b): contents unchanged; main-side insertions shift the anchors (core/engine/turn_loop.rs:5611,tools/subagent/mod.rs:8058,tools/verifier.rs:587unchanged), fork-CI flags clean, and the bare upstream invocation inventory is 9 pre-existing sites (approval.rs:183, verifier.rs:587, subagent/mod.rs:8058, remote_control.rs:2188, lib.rs:12445, turn_loop.rs:909, turn_loop.rs:5611, engine.rs:2019, context_report.rs:422), identical at base — none in this PR's files.command_safety::82 passed / 0 failed;tools::shell::173 passed / 0 failed / 1 ignored (pre-existing);tools::subagent::608 / 0;fleet::361 / 0;tools::registry::48 / 0. At the round-6 head:command_safety::84 / 0;tools::shell::175 / 0 / 1 ignored;tools::subagent::608 / 0;fleet::361 / 0;tools::registry::48 / 0. At the round-7 head:command_safety::89 / 0;tools::shell::182 / 0 / 1 ignored;tools::subagent::608 / 0;fleet::361 / 0;tools::registry::48 / 0. At the round-8 head:command_safety::89 / 0;tools::shell::185 / 0 / 1 ignored;tools::subagent::608 / 0;fleet::361 / 0;tools::registry::48 / 0. At the round-9 head (93a28bde6):command_safety::91 / 0;tools::shell::193 / 0 / 1 ignored;tools::subagent::608 / 0;fleet::361 / 0;tools::registry::48 / 0. At the round-10 wave head (dc889a214):command_safety::94 / 0;tools::shell::203 / 0 / 1 ignored;tools::subagent::608 / 0;fleet::361 / 0;tools::registry::48 / 0. At the round-11 wave head (f65a0d1ac):command_safety::94 / 0;tools::shell::206 / 0 / 1 ignored;tools::subagent::608 / 0;tools::verifier::9 / 0;fleet::361 / 0;tools::registry::48 / 0 (the round-10 body said 203 shell tests; the isolated count at that head was 207 — the final commit's tests postdated the number, and one env-gated member is host-dependent). At the round-12 wave head (db75f6526):command_safety::94 / 0;tools::shell::216 / 0 / 1 ignored (ten new pins);tools::subagent::608 / 0;tools::verifier::9 / 0 +fleet::361 / 0 +tools::registry::48 / 0 (418 combined). At the round-13 wave head (7cb6ecea1):command_safety::94 / 0;tools::shell::228 / 0 / 1 ignored (twelve new pins);tools::subagent::609 / 0;tools::verifier::9 / 0;fleet::361 / 0;tools::registry::48 / 0 (1349 passed combined). At the round-14 wave head (d3d22629b, on top of ddcf5e8's two round-13 pin-gap tests):command_safety::94 / 0;tools::shell::239 / 0 / 1 ignored (nine new pins: two walk-up units, the skip-set unit, two commondir units, two leak-flip filter E2Es, the terminal-interact pin, the bounded-tty-write pin);tools::subagent::609 / 0;tools::registry::48 / 0;tools::verifier::9 / 0;fleet::361 / 0.cargo test -p codewhale-tui --locked --lib(with the repo-requiredRUST_MIN_STACK=16777216) at the round-5 head: 12325 passed / 2 failed / 13 ignored, the two failures beingclient::operation_tests::forkguard_model_operation_auxiliary_retry_keysandtools::file::pdf_tests::contract_read_reports_huge_first_line_with_exact_bash_fallback— the known load-timing flake set (the run overlapped a per-commit compile check); both pass alone (2/2 in 0.13 s) andgit diff origin/pinvou3-clean...HEADtouches neitherclient/nortools/file. Earlier heads: 12310/1, 12309/2, 12308/3 (round 4), 12303/0/13 on a quiet machine (round 3), always with the same rotating set. At the round-8 head the rotating set grew:remote_control::tests::separate_predispatch_crashes_on_one_run_get_distinct_recovery_turn_idsfailed 3/3 full-suite runs (withruntime_chat_backlog_refreshes_immutable_config_before_new_catalog_or_prompt,restart_rehydrates_runtime_chat_ownership_until_terminal_cursor, andruntime_threads::tests::restart_reconciles_unresolved_dynamic_call_after_existing_turn_completionappearing in individual runs) — all 77remote_controltests pass alone; none of those files is touched by this branch. The suite hard-aborts without the 16 MiBRUST_MIN_STACKthe CI config sets — environment, not code. Every one of the eight round-5 commits builds on its own (cargo check --lib --testsper commit). At the round-9 head: 12347 passed / 2 failed / 13 ignored, the failures beingremote_control::tests::separate_predispatch_crashes_on_one_run_get_distinct_recovery_turn_idsandremote_control::tests::restart_rehydrates_runtime_chat_ownership_until_terminal_cursor— both in the documented rotating set, each passing alone; the workspace lib tests (--exclude codewhale-tui) and doc tests are green. At the round-10 wave head: 12360 passed / 2 failed / 13 ignored — the same documentedremote_controlpair, all 83remote_controltests passing alone (3.56 s); one earlier full-suite run on a loaded machine rotated a wider set (runtime_threads::tests::agent_mail_queue_is_durable_idempotent_redacted_and_workspace_scoped, a secondtools::pdfmember) — all pass alone in untouched files; fmt clean, fork-CI clippy clean under its exact allow list, upstream--all-targetsclippy reporting only the pre-existingturn_loop.rs:5499, workspace libs (2150/0) and doctests green. At the round-11 wave head: 12363 passed / 3 failed / 13 ignored — one documentedremote_controlmember plusremote_control::tests::runtime_chat_same_session_reconfigure_releases_then_reacquires_owner_lockandtui::ui::tests::missing_named_custom_provider_resume_leaves_current_session_wholly_unchanged, all three passing alone (3/3 in 0.09 s) in files this branch does not touch; workspace libs 2150/0 and doctests green at this head too. At the round-12 wave head: 12372 passed / 3 failed / 13 ignored — the same documentedremote_controlrotating set (restart_rehydrates_runtime_chat_ownership_until_terminal_cursor,runtime_chat_same_session_reconfigure_releases_then_reacquires_owner_lock,separate_predispatch_crashes_on_one_run_get_distinct_recovery_turn_ids), each passing alone (3/3 in 0.12 s); one earlier full-suite run on a loaded machine rotated 5 members of the same family; workspace libs (2150/0) and doctests green at this head. At the round-13 wave head (7cb6ecea1): 12384 passed / 5 failed / 13 ignored — the documentedremote_controlrotating set (four members) plusmodel_inventory::tests::ollama_default_prefers_live_local_tags_over_the_unresolved_marker, all five passing alone (5/5 in 0.13 s) in files this branch does not touch; workspace libs (exit 0) and doctests (exit 0) green.. At the round-14 wave head (d3d22629b): 12397 passed / 3 failed / 13 ignored — the documentedremote_controlrotating set (restart_rehydrates_runtime_chat_ownership_until_terminal_cursor,separate_predispatch_crashes_on_one_run_get_distinct_recovery_turn_ids,runtime_chat_same_session_reconfigure_releases_then_reacquires_owner_lock), all in untouched files and each passing alone in earlier runs of the same set; workspace libs and doctests green at this head;cargo fmt --all -- --checkclean; fork-CI clippy (exactfork-ci.ymlflags) clean; upstream--all-targetsclippy (exactci.ymlflags) reporting only the pre-existingturn_loop.rs:5499. At the round-15 wave head (922c286ec):tools::shell::245/0/1-ignored,command_safety::95/0,forkguard_240/0,tools::subagent::609/0,tools::registry::48/0, full--lib12404/3/13 (the documented rotating set:forkguard_model_operation_auxiliary_retry_keysplus tworemote_controlmembers, all in untouched files and each passing alone in earlier runs), fmt clean, both clippy shapes as above; all seven round-15 mutations red.Pinvou fork CIdispatches on this branch have been zombie-queuing with zero jobs GitHub-side for 24+ hours (round 9 disclosed the same; 409 on cancel) — if this run stalls identically, the repo needs an admin look at Actions health; the local gates above stand in.cdParentDircheck; skipping the cd operand containment; admitting anyod -Avalue; forcing the cd shell-routing term false; restoring sync-laneStdio::inherit(); restoring foreground open-stdin; discarding the operand-gate cwd fold; dropping thecd-as-pipeline-stage refusal; restoring the whole-segmentgh/cdearly return in the gate; disabling glob expansion; keying the network guard on the whole command; restoring therole_posture_permitsre-key; disabling cluster expansion; round 3: dropping the cd wildcard check; disabling the./..synthesis; disabling the match cap; reverting the network guard to the whole-command predicate; disabling the sed trailing-flag refusal; disabling the=cmdrefusal; forcingparent_shell_authoritytrue; round 4: deleting the cdis_dircheck; splitting bare operands on=again; re-adding the operand.trim(); giving each operand a fresh glob budget; making the entry counter a no-op; making the shell-lane git rewrite a no-op; movingblameback to the no-splice set; keying the no-network pre-check on the whole command. round 5: making the Windows path normalization unconditional again; making the operand verbatim-prefix strip unconditional; dropping the cd=refusal; dropping the cd enterability probe; dropping the**refusal; dropping the glob component cap; dropping the whole-word option glob walk; keying pipefail on the kind again; dropping the cmd.exe%refusal; restoring%/^to the bare re-quote set; never recording the stdin-EOF flag; dropping the enter rule from the refusal copy — 12/12 red, each reddening exactly its own pins, and the first, third, fourth and fifth reproduce the outside-content leak itself through the tool; round 6: making the seam's connector check a tautology (the gate pin and its fold discriminations redden); dropping the classifier's connector check (the classifier pin reddens; the seam wall still holds, as designed) — 2/2 red; round 7: disabling the filter discovery (the E2E filter pin reddens on the outside-content leak itself); dropping the discovered pins fromGIT_CONFIG_COUNT(same pin, same leak); making the gh-stage wildcard refusal a no-op (the gh-glob pin reddens); ignoring the-Cpreamble in filter discovery (the fold/-Cpin reddens) — 4/4 red, each reddening exactly its own pin. Round 8 (all applied → expected pins redden → reverted → green, each reddening exactly its own pin): case-folding the name dedupe (the two-case pin reddens); skipping the header-line remainder (the same-line pins redden); inverting the continuation parity (the escaped-backslash pin reddens); reporting an undecodable.gitlink as no repository (its refusal pin reddens); skipping submodule enumeration (both submodule pins redden, the E2E on the leak itself); no-op'ing thecore.worktreevalidation (its refusal pin reddens); forcing the version gate true (its comparator pin reddens); reverting the gh host heuristic (the single-label pin reddens) — 8/8 red. Each round-4 mutation reddened exactly its own pin(s); three of them reproduce the outside-content leak itself and both textconv mutations show the repository's filter executing under the tool. The first cut's two inert pins (od-A, standalone-cd routing on macOS) were found by this process and replaced with pins that bite. Round 9 (all applied → expected pins redden → reverted → green): the dotted-header split skipped (the dotted pin reddens); same-line re-targeting removed (the two-headers assertion reddens); the comment cut disabled (the trailing-comment assertion reddens — the escaped-backslash assertions sit later in the same test and stop executing under it);core.worktreefirst-value restored (the last-wins refusal reddens);readonly_gitlink_pathsenumerate-nothing variant applied (the submodule unit pinreadonly_filter_discovery_scans_registered_submodulesand the submodule E2E twin redden on the leak itself, the repository-content E2E stays green as designed; a validation-bypass variant would only redden the direct unit pin — the call site itself is covered end-to-end by the E2E); the-Cchain flattened to the base cwd (the chained--Cpin reddens at its admission control, and the shared discovery fold test reddens with it — the resolver is deliberately one function serving both callers); the version probe made env-inheriting (its shim pin stays green because the pin injects PATH through the test seam rather than the live environment — the guarded property is that the SUPPLIED PATH is sanitized — and the unsanitized-PATH variant of the same mutation reddens it) — 7/7 red with the stated isolation. Round 10 (all applied → expected pins redden → reverted → green, baseline 97 targeted pins green, each reddening exactly its own pin with one unrelated pin in the area confirmed green): the discovery argv pins dropped from the enumeration child (both fsmonitor pins redden on the marker leak — unit and tool); the tilde-Crefusal deleted (the tilde pin reddens on the admitted escape); the-Centerability probe dropped (the mode-000 pin reddens); the bounded reader's regular-file check neutered (the FIFO pin reddens on its hang guard — the O_NONBLOCK open succeeds and the lane's own git child then blocks on the FIFO config, exactly the hang the wall exists to prevent); thecommondirrestore totrim()with the empty-refusal dropped (the padded pointer pin reddens); the.gitlink\rstrip deleted (the CRLF pin reddens); the gitlink"/\\refusal made unconditional again (the unix-bytes scan pin reddens, and the updated unit pin reddens with it); the stdin cap disabled (the oversized-payload pin reddens on the executed payload); the glob routing clause dropped (the single-glob expansion pin reddens — the literal spelling fails on the direct lane); the glued head/tail branch removed (its pin reddens); the od suffix parse reverted to plain u64 (its pin reddens) — 11/11 red. Round 11 (all applied → expected pins redden → reverted → green, against the committed wave): the gitfile reader restored to the first-line cut (the multi-line-gitdir E2E reddens on the hostile-filter leak); the-Cwildcard wall neutered (its pin reddens at the chained admission); the-C=wall neutered (its pin reddens ongit -C =pwd); the version floor restored to 2.31 (the comparator pin reddens on the 2.31.9 refusal); the gate-job stdin close deleted (thecat-gate pin reddens after the completion wait finds it still running); the commondir reader restored to the first-line cut (the whole-value pin reddens) — 6/6 red. Round 12 (all applied → expected pins redden → reverted → green, on the committed wavedb75f6526, 10/10 red): the gitfile loop-strip restored to one\n+one\r(the multi-strip unit reddens on its one-strip decoys AND the two-newline E2E checkout leg reddens on the[shell.readonly.git.filter_config]refusal itself); the commondir loop-strip restored to one strip (its twin reddens on the decoy filter namedecoylf); the..synthesis restored tobase.parent()(the symlinked-dotdot pin reddens; the round-3 dot-synthesis pin stays green — its fixtures sit where lexical and physical coincide, exactly why the shape survived); the discovery watchdog neutered (the FIFO-index pin reddens on its 30 s hang guard); the stdin deadline removed (recv()unbounded; the bounded-write pin reddens on its 30 s hang guard); the stall flag never set (the same pin reddens at the second-write 500 ms bound after a second full deadline — 10.03 s total confirms the mechanism); the echo cap removed (the echo pin reddens on the full 400-byte value echoed); the gitdir validity check removed (the dangling-link assertion returns Ok where it demands a refusal); the continuation reorder reverted (the swallowed-header pin reddens — thefiltersection armed where git swallows it — and all ten parser-family pins stay green, matching the prediction); the canonical-roots sharing reverted at the execute entry (the aliased-root tool leg reddens on the-Ccontainment refusal where it demands success). Round 13 (all applied → expected pins redden → reverted → green, on the committed wave7cb6ecea1, 11/11 red, each in its own worktree and private target): the directory branch reverted to unconditional return (the bogus-dir walk-up pin and both leak-flip E2Es redden — the cross-boundary one too); the predicate forced always-true (the predicate unit's negative legs, the padded-commondir-names-nothing refusal, the strip test's dangling leg, and the three walk pins — coherent, the walk shares the predicate); the predicate forced always-false (25 red, every walk-resolving pin); the NUL truncation removed (its pin alone); the poll quantum set to 1000 ms (the NEW constant pin reddens — the elapsed window deliberately still passes because the sleep-then-check loop lands the kill at ~10 s for any quantum up to the bound, which is exactly why the latency constant is pinned separately); the gitlink echo unbounded (its pin alone, refusal 300,241 bytes); the subsection filter dropped (its pin alone); the dedup remaining-length component removed (its pin alone — the round-3 dot pin STAYS GREEN, the documented blind spot); the.candidate removed (its pin alone — the round-3 dot pin STAYS GREEN); andsession_shell_ceilingwidened on the session flag (the chain pin alone).Round-16 receipt at the wave head (
79d72fb78):tools::shell::247 passed / 0 failed / 1 ignored;command_safety::96 / 0;forkguard_240 / 0;tools::subagent::609 / 0;tools::registry::48 / 0; fmt clean; fork-CI clippy clean under its exact flags; upstream--all-targetsclippy under the exact ci.yml flags reporting only the pre-existingcore/engine/turn_loop.rs:5499; mutations 3/3 red (piped git site, unpiped git site, cmd delimiter wall) with the round-15 mutation set still standing (the round-16 lane re-proved 8 walls red: post---flip, dash-named wall, PowerShell comma wall, visible copy, NUL charset, ZDOTDIR pin, walk-up predicate,ref:skip set — each reddening exactly its own pins). Local ohos/windows cfg verification by rustc probes on the installed targets (as of round 16 no completed CI run existed for any head of this branch — the fork CI dispatches were zombie-queued, disclosed above; superseded by the round-19/round-20 receipts below).Round-17b follow-up receipt at the wave head (
565049c0b, content-identical tree721d29136to the localfbd0ed25f):tools::shell::252 / 0 / 1-ignored (the new network-guard tool-entry pin),command_safety::98 / 0 (the two grammar pins),tools::subagent::609 / 0,tools::registry::49 / 0 (the catalog pin),tools::verifier::9 / 0,forkguard_240 / 0; fmt clean; fork-CI clippy clean under its exact flags; upstream--all-targetsclippy under the exact ci.yml flags reporting only the pre-existingcore/engine/turn_loop.rs:5499; mutations 4/4 red (wc cluster neutered, grep rewrite neutered, catalog bash-drop reverted, network guard dropped — the last failing locally at gh's argument parsing, never touching the network).Round-18 wave receipt at the wave head (
3fab32959):tools::shell::258 passed / 0 failed / 1 ignored (six new pins: the absolute-glob unit + tool E2E, the cmd comment wall, the owned-writer bounded stdin helper, the aggregate discovery budget, the spawn-env overrides),command_safety::100 / 0 (two new pins: the separated count-value validation, the parallel-charset NUL refusal),tools::subagent::609 / 0,tools::registry::49 / 0,tools::verifier::9 / 0,forkguard_240 / 0;cargo fmt --all -- --checkclean; fork-CI clippy clean under the exact workflow flags; the cfg-surface probe compiles-Dwarnings-clean onaarch64-unknown-linux-gnu,aarch64-unknown-linux-ohos, andx86_64-pc-windows-msvc; fullcargo test -p codewhale-tui --locked --lib(repo-requiredRUST_MIN_STACK=16777216) 12422 passed / 4 failed / 13 ignored, the four failures all in the documented rotatingremote_controlflake family (untouched files; both spot-checked green in isolation); mutations 4/4 red as itemized in the round-18 entry. Rebase freshness re-verified at the wave:origin/pinvou3-cleanstillf1eb10efc(merge-base == base tip), the push fast-forwarded565049c0b..3fab32959.Round-19 receipt at the slice head (rebased onto #139's
6337b0e0a, head3406aeb30, treed83ee5291a8d— built and verified locally, then landed through the Git Data API during a git-443 outage and finally force-pushed as plain git once the wall lifted):tools::shell::262 / 0,command_safety::100 / 0,tools::subagent::601 / 0,tools::registry::48 / 0,tools::verifier::9 / 0 — 1019 passed / 0 failed / 1 ignored (the ignored exporter pre-existing;tools::verifier::is the current filter spelling — the pre-splittests::verifier::name no longer matches, which is why an earlier draft of this receipt mis-tallied the split; the registry 49→48 and subagent 609→601 drops vs the round-16 combined-tree receipt are the split's test moves — the role/registry pins live on slice 3);cargo fmt --all -- --checkclean; fork-CI clippy clean under the exact workflow flags; the escape-decode wall mutated off → both its pins redden (2/2), restored → green. CI: the fork CI ran green on the round-18 headf5aaaafd408(run 41816238004,check15m29s incl. the full workspace test job). At the round-19 head thecheckjob is green under dispatch (run 41845678082) with the Gitleaks tree leg clean; the dispatch's introduced-scan leg trips 8 PRE-EXISTING full-history findings (dispatch has nopull_request.base.sha, so the workflow's documented fallback scans all 7406 reachable commits — findings innative_memory-era tests,config/persistence.rs, extension-host fixtures, none in files this slice touches), reproduced locally with the pinned gitleaks 8.30.1: the introduced range scans 0 leaks; the Merge Queue's integration run with a well-defined base is the authoritative gate.Round-21 receipt at the re-aligned stack + wave (
7b754c7df+1f9635a894+6386b876b, treebc3415f3c9):readonly_150 / 0,command_safety::101 / 0,forkguard_285 / 0 (--test-threads=2; at the harness-default 20 threads the suite's global test-env locks convoy past 12 min — a pre-existing suite property, every stalled test passes in <0.1 s isolated),tools::subagent::648 / 0;cargo check -p codewhale-tui --all-targetsclean under the workspacedeny(warnings); gitleaks over the introduced range8c6c00dc0c..6386b876b: no leaks. DCO note:1f9635a894's author email (asto18089@126.com) differs from itsSigned-off-by(asto18089@users.noreply.github.com) — the gate passes; normalize at squash.Round-20 receipt at the rebased slice head + fix wave (
d6d41439d8+8f98b3e895, treec7c17bfff):tools::shell::270 passed / 0 failed / 1 ignored,tools::subagent::629 / 0,command_safety::100 / 0,tools::registry::48 / 0,tools::verifier::9 / 0 — 1056 passed / 0 failed / 1 ignored;cargo fmt --all -- --checkclean;cargo clippy -p codewhale-tui --all-targets --all-features --lockedinventory: zero warnings pointing into slice-2's six files (remaining: the three pre-existingcollapsible_ifand CI-allowedtoo_many_argumentsin untouched files); gitleaks 8.30.1: the introduced-range scan clean, and the full-worktree scan's 8 findings are all pre-existing main content outside every introduced hunk (verified hunk-by-hunk, incl. thelib.rs:15034fakesk-live-…marker inside a redaction test); the widened six-key pin reddens on a droppedBASHOPTSentry, and the renamed four-key pin no longer exists to go stale.Round-17 wave receipt at the wave head (
64abd6c66e, superseded by the tree-identical rewrite24e67da67):tools::shell::251 passed / 0 failed / 1 ignored (four new pins),command_safety::96 / 0,forkguard_240 / 0,tools::subagent::609 / 0,tools::registry::48 / 0,tools::verifier::9 / 0; fmt clean; fork-CI clippy clean under its exact flags; upstream--all-targetsclippy under the exact ci.yml flags reporting only the pre-existingcore/engine/turn_loop.rs:5499; mutations 2/2 red (continued-value wall neutered → its two fixtures; attached-path guard removed → the post---admission legs); spliced-verbatimwrite_stdinprobe-compiles clean on ohos, windows-msvc, and the host under-D warnings.Checklist
*-only glob walker incommand_safety.rs/shell.rs—globsetis already a dependency but only matches, so enumeration plus the shell dotfile/..rules still need the small hand walker; the charset refuses every other wildcard; the Verifier joins two existing role predicates; round 4 adds one shared stage splitterreadonly_pipeline_stagesreplacing three inlined splits, one gh-reach predicatecommand_reaches_github_readonlyreplacing the deleted whole-commandis_github_readonly_command, oneReadonlyGlobBudgetstruct, and one shell-lane Git stage rewriter that reuses the direct lane's splice tablesplice_git_read_hardening; round 5 adds one shared cd operand shape rulereadonly_cd_operand_admittedreplacing the classifier's inline checks, one per-kind compound refusalreadonly_compound_refused_by_shell_kind(thereadonly_sequence_refused_by_shell_kindspelling quoted in earlier drafts was a phantom name — no such fn existed in any tree), one binary-keyedshell_binary_speaks_pipefail, oneMAX_READONLY_GLOB_COMPONENTScap, and onestdin_closed_at_eofbool onBackgroundShell;normalize_windows_command_pathsbecomes a Windows-only wrapper over the unconditionalprotected_windows_command_paths); round 12 adds one bounded discovery child (watchdog thread + wall-clock kill over the shared child lock), one bounded stdin write (worker thread + deadline + thestdin_write_stalledflag onBackgroundShell), the textual..synthesis in the existing walker, and the loop-strip + gitdir-validation mirror in the existing readersdocs/SUBAGENTS.md,docs/zh_hans/SUBAGENTS.md,docs/zh_hans/FLEET.md, bash guidance, roster/schema/intro strings, refusal copy) — slice-scoped at the split: the three docs/ files, roster/schema/intro strings, and the Verifier intro live in slice 3 (fix(fleet): the test role states its real contract #140, verified present there); this slice carries the bash tool-description rewrite and the[shell.readonly.command]refusal copy, both in this diffFollow-up (this repository, scheduled by the round-10 review)
Round-15 disclosed follow-ups (reviewed, deliberate not-fixed): the repository-discovery walk-up has no wall-clock deadline — the one pre-spawn scan without the glob-deadline/discovery-watchdog treatment, so a hostile deep tree (or NFS workspace) stretches every git stage's pre-spawn cost on the blocking pool. Round-16 adds to the same list (all reviewed, deliberate not-fixed): the discovery read loop never re-checks its watchdog flag, so a grandchild holding the killed child's stdout could out-wait the bound (unreachable under the pinned config today — defense-in-depth); the version probe runs unwatchdogged (trusted binary resolving through the sanitized PATH, memoized per PATH); discovery has no per-command aggregate budget (32 scanned repositories × the 10 s watchdog ≈ a 5-minute fail-closed worst case on a crafted multi-submodule FIFO workspace) — STALE as written at round 16: round 18 added the aggregate budget (
MAX_READONLY_DISCOVERY_WALL_CLOCK, 10 s) this item predates, and the round-19 performance lane measured the honest worst at ≈10–20 s fail-closed (FIFO variant ≈10 s; slow-children variant ≈10 s aggregate + one ≤10 s in-flight overshoot), so the item is closed as implemented — kept here only to correct the figure;BackgroundShell::drop's PTY arm waits unbounded where the Process arm uses the bounded helper; the dash-named match wall is position-blind (a final match starting with-refuses even where the expansion word could never parse as an option —wc -c ./*,cat -- *— safe direction; refining requires per-slot expansion knowledge worth its own change); the.gitgitfile reader keeps the UTF-8 wall for non-UTF-8 byte targets while thecommondirreader is byte-faithful on unix — an internal asymmetry, both fail-safe; the sanitized PATH is recomputed per discovery child and before the version-gate memo lookup (~2 redundant full canonicalization passes per admitted git command); two canonicalization sites remain on the executor thread under the manager lock (shell.rsenv block and program resolution — pre-existing structure; the pooled roots could thread through); the discovery reap's watchdog-fired branch ends in an unbounded blockingwait()(D-state child only, disclosed at the site;wait_child_boundedexists); Windows stdin writers remain blocking under the manager lock (disclosed since round 12;ChildStdin::try_clonemakes the pipe lane servable by the unix bounded worker); the twocommondirconsumers share the value reader but sit on different file-reading layers with different failure mappings (predicate declines, scan refuses — fail-safe both ways; one resolved-reader helper would unify); the git preamble skip (--no-pager/-C) is hand-rolled in three places;!refuses outright where a mid-word!is inert on every supported shell (fidelity polish); raw-git test controls bake git-2.43 semantics without the version gate the filter pins use and inherit ambientGIT_*env (fail-loud hygiene);BASHOPTS/SHELLOPTSbelt-and-braces startup pins — implemented in the round-20 wave: the round-15 claim that the child-env allowlist already drops both was wrong (the spawn path inherits the operator's environment; only the listed overrides are applied), so both keys are now cleared inreadonly_spawn_env_overridesand pinned byreadonly_spawn_env_overrides_carry_the_six_fixed_keys.Replace
parse_git_config_settingsand its two consumers with enumeration through git itself: the UNscopedgit config --list -zin each discovered repository, run under the same hardened discovery environment the argv config pins now arm (the argv-cpins outrank every config file, so the enumeration inherits the fsmonitor/hooks walls without a version floor), with origin filtering over the listing. NOT the scoped--localspelling an earlier revision of this section named:--locallists onlyinclude.path=..., never the includedfilter.*records, and never readsconfig.worktree, so that spelling would convert today's fail-closed include walls into silent under-scans — the round-7 class behind a green-looking scan. The enumeration route dissolves theinclude/includeIf/worktreeConfigrefusal walls into enumerated coverage (a functional improvement — repositories with includes stop refusing every git evidence command), inherits git's grammar permanently instead of chasing dialect shapes (rounds 7→8→9 found 1→5→7 escapes in successive approximations of the same ~200-line parser), and costs one extra child per scanned repository the lane already pays; when git cannot be invoked at all, every git evidence command refuses anyway, so there is no parser fallback to keep. Probe-c-entry visibility in the listing and includes-of-includes before deleting the parser; the 17-shape differential this branch's probes cover becomes the regression suite.Follow-up (parent repository)
pinvou-agentpicks this up when its CodeWhale gitlink advances to the squash of this PR onpinvou3-clean, together with the outstandingfork-guard.shregister/fingerprint updates already noted by #115–#125.