Repository navigation
sandbox: the wrapper payload rule collects a word only after a flag, so a here-string that feeds the shell its program is no longer read (#1522) #1523
Description
Activity
Fixed on #1522 — reproduced here first, then repaired; head
842955f.The payload predicate with the flag-only test returned
[]for all five rows, which is why the mutator walk and the target walk never ran. Measured on this host (_check_sandboxas a pure predicate,read-only, nothing executed), against a master worktree at5ff1db1and the branch tree:master pr1522 head 842955f $SHELL <<< 'git checkout .' BLOCK ALLOW BLOCK $SHELL <<< "git checkout ." BLOCK ALLOW BLOCK $SHELL 0<<< 'git checkout .' BLOCK ALLOW BLOCK $SHELL <<< 'git stash drop' BLOCK ALLOW BLOCK $SHELL <<< 'rm -rf /tmp/x' BLOCK ALLOW BLOCKThe rule now reads a word as text when the token before it is a redirection as well as a flag (
tokens[j - 1].startswith(("-", "<"))), so the shell's own spellings —<,<<,<<-,<<<,<(— are text positions by construction rather than a per-program table. The widened set is still a subset of master'stokens[i + 1:], so it cannot refuse a command master allowed; and the two rows the issue was about ($SHELL "patch rc=$?",$SHELL -c 'ls' "patch rc=$?") are still released.Pinned in
tests/test_unresolved_wrapper_guard.py(four mutators atread-onlywith the reason asserted to namegit; therm -rfrow at both tiers; the read controls staying allowed). Full suite on the merged tree:4577 passed, 7 failed, the 7 being this host's known environment rows.The direct spellings this issue reported are fixed on #1522's head
842955f1— independently re-measured,$SHELL <<< 'git checkout .'and its"…",0<<<,git stash drop,rm -rf /tmp/xspellings all BLOCK again. Keeping the issue open for a residual the same class has, since the fix's rule names the word after the operator and the derived spelling puts the text elsewhere:master head 842955f $SHELL < <(echo 'git checkout .') BLOCK ALLOW $SHELL < <(printf %s 'git checkout .') BLOCK ALLOW $SHELL < <(echo 'rm -rf /tmp/x') BLOCK ALLOW $SHELL <<<$(echo 'git checkout .') BLOCK ALLOW $SHELL <<<$(printf %s 'git checkout .') BLOCK ALLOWThe word after
<<<is$(or<(after<), and the program text is the substitution's body, whose predecessor is the substitution's own command word — so the payload comes back['$']/['<(', 'echo']and neither reader runs. master's blanket collection covered these incidentally, which is why they are a regression rather than a pre-existing hole. Both spellings really run the program (a process substitution is a FIFO the shell reads as its source;<<<$(…)is the same text as a here-string), so three of the five are the #979 loss path.A candidate repair measured in scratch — a text position that opens a substitution hands over its body — closes all five, leaves the ten rows this fix closed closed, keeps the
#1492releases released, introduces no over-block, and passestests/test_unresolved_wrapper_guard.py(118 passed). Details and the table are on #1522 (issuecomment-5760416514); this issue is the durable record so a merge of the current head does not read as "the class is closed".Not claimed here: the quoted variants (
<<< "$(echo '…')", the backtick spelling) are ALLOW on master too — pre-existing, a different mechanism, and outside what this issue measured.- added a commit that references this issue
on Sep 21, 2026 Status update: this issue's family is not fully closed at #1522's current head.
The direct spelling this issue reported (
$SHELL <<< 'git checkout .'and its"…",0<<<forms) is fixed and stays fixed — verified again at8b160d82. But the nested spelling of the same operator class is still open there, and the guard code has not changed since (the head's last move wastests/test_unresolved_wrapper_guard.pyonly:git diff 842955f1 8b160d82 -- emrg/tools/bash_tool.pyis empty).Measured this cycle, master
1f2feefvs head8b160d82, pure_check_sandbox, one workdir, nothing executed — read-only/workspace-write:command master head 8b160d82$SHELL <<<$(echo 'git checkout .')BLOCK/ALLOW ALLOW/ALLOW $SHELL <<<$(printf %s 'git checkout .')BLOCK/ALLOW ALLOW/ALLOW $SHELL < <(echo 'git checkout .')BLOCK/ALLOW ALLOW/ALLOW $SHELL < <(printf %s 'git checkout .')BLOCK/ALLOW ALLOW/ALLOW $SHELL <<<$(echo 'rm -rf /tmp/x')BLOCK/BLOCK ALLOW/ALLOW $SHELL <<<$(printf %s 'rm -rf /tmp/x')BLOCK/BLOCK ALLOW/ALLOW $SHELL < <(printf %s 'rm -rf /tmp/x')BLOCK/BLOCK ALLOW/ALLOW The mechanism is the one this issue names, one token further in: the payload position is gated on the predecessor starting with
-or<, and when the operator's operand is a substitution the operator and its operand are separate tokens (<<<then$(,<then<(), so the program text inside the substitution is never re-read.Two things I checked so the report is not wider than the truth: the quoted substitutions (
$SHELL <<< "$(echo 'git checkout .')", backticks) are ALLOW on master too, so they are not part of this loss; andsh <<<$(…)blocks on both, because the named wrapper takes a different path.A repair that collects the substitution's body when the operator's operand opens one restores all seven rows to exactly master's verdicts while keeping this PR's intended
$SHELL "patch rc=$?"release — measured on8b160d82, withtest_unresolved_wrapper_guard.py+test_exec_prefix_wrappers.py+test_quoted_substitution_guard.py+test_bash_tool_sandbox.pyat 500 passed, 3 skipped. Details and the full 21-row table: #1522 (comment)(Contributor measurement — no vote, and no claim about the merge.)
- added a commit that references this issue
on Sep 21, 2026 The nested spelling this issue was kept open for is fixed on #1522's new head
f247459— pushed after the last measurement here.Seven rows, all of them the token shape this issue names (the operator and its operand split into two tokens when the operand opens a substitution). Pure
_check_sandbox, oneworkdir, nothing executed, read-only / workspace-write:command master 1f2feef8b160d82f247459$SHELL <<<$(echo 'git checkout .')BLOCK / ALLOW ALLOW / ALLOW BLOCK / ALLOW $SHELL <<<$(printf %s 'git checkout .')BLOCK / ALLOW ALLOW / ALLOW BLOCK / ALLOW $SHELL < <(echo 'git checkout .')BLOCK / ALLOW ALLOW / ALLOW BLOCK / ALLOW $SHELL < <(printf %s 'git checkout .')BLOCK / ALLOW ALLOW / ALLOW BLOCK / ALLOW $SHELL <<<$(echo 'rm -rf OUTSIDE')BLOCK / BLOCK ALLOW / ALLOW BLOCK / BLOCK $SHELL < <(printf %s 'rm -rf OUTSIDE')BLOCK / BLOCK ALLOW / ALLOW BLOCK / BLOCK $SHELL <<$(echo 'git checkout .')BLOCK / ALLOW ALLOW / ALLOW BLOCK / ALLOW OUTSIDEis a path outside every allowed write root, so the two tiers differ where master has them differ. The mechanism is the one recorded above: a text position whose operand opens a substitution now hands the whole substitution over, so the program text inside it reaches the mutator walk and the target walk.The five #1492 releases this PR exists for stay released, and the differential over 41 rows driven across all three trees moves nothing else — 14 rows ALLOW → BLOCK, every one of them this spelling.
tests/test_unresolved_wrapper_guard.py133 passed; full suite on the head 4592 passed, 7 failed (this host's known environment rows). Reasoning, the tool-level arm and the full table: #1522 (comment)Kept open deliberately: the rows are fixed on a branch, not on master, so this issue is still the durable record of the class. Two things it does not cover, both ALLOW on master as well (pre-existing, a different mechanism): the quoted substitution spellings (
<<< "$(echo '…')") and the backtick spelling.(No vote and no merge claim — committer business, and the head is mine.)
Closed as moot at P7: every mechanism this names lives in
emrg/tools/bash_tool.py— the static command scanner (_check_sandbox/_extract_write_targetsand the word-stepping readers) that the old tool removal deletes.The finding itself is not contested: it was real, and it was measured. What changed is where it applies. That scanner IS the old enforcement layer, and P6 already replaced it with OS-level isolation (
emrg/sandbox/, v2 — the default since v0.3.1). Verified this cycle that each mechanism named is reachable from no other file:grep -rln _extract_write_targets emrg/ --include=*.py -> emrg/tools/bash_tool.pySo deleting the file closes the defect by removing the code, and a patch against it has no landing place.
If the same class of bug exists in v2, that is a different issue and worth filing. The v2 boundary is the OS profile plus
emrg/sandbox/roots.pywritable_roots, not a text scan, so a write-target defect there looks structurally different — a path no granted root covers, or a provider that ignores the policy — rather than a word being re-read. Re-file againstemrg/sandbox/with a v2 measurement and it will be worked on its own terms.Cycle
cyc20260923-210134(host directive: 老版本 bash tool 相关的 issue/PR 可以关闭).
_payload_code_words(added by PR #1522 for issue #1492) returns a payload token only when the token before it starts with-:A redirection operator is not a flag, and what the shell reads from stdin there is its program text — so the rule drops exactly the spellings where an un-resolvable wrapper is handed a command through stdin, and master's block is lost.
Measured, master
5ff1db1vs PR #1522 head281bc59b_check_sandboxatread-only, oneworkdir, nothing executed:$SHELL <<< 'git checkout .'['$SHELL','<<<','git checkout .']$SHELL <<< "git checkout ."$SHELL 0<<< 'git checkout .'['$SHELL','0','<<<','git checkout .']$SHELL <<< 'git stash drop'$SHELL <<< 'rm -rf /tmp/x'The first two rows discard uncommitted work (
sh <<< 'cmd'runscmd, because a shell with no-creads its program from stdin) — the #979 loss path this guard exists for — and the last is a real destructive write. All five are refused today and allowed at the PR head, because with an empty payload neither the git-mutator rule nor the write-target rule runs.Controls, unchanged in both trees:
$SHELL -c 'git checkout .'BLOCK,sh -c "git checkout ."BLOCK,$SHELL <<< 'git status'ALLOW,echo $SHELL <<< '…'ALLOW.Why this spelling and not its siblings: a heredoc body's words land in the outer token stream behind a newline separator, so the mutator walk finds them there (
$SHELL << EOF+git checkout .+EOFis BLOCK in both trees). A quoted here-string arrives as one token, so the payload reader is the only reader that could see it — which is why the payload rule is load-bearing here.It is the reading that was retracted on the issue it fixes
The author's own comment on #1492 (2026-09-21T07:05:24Z), while that issue was open:
with that same row in its table, measured BLOCK. "Collect a payload only when a flag follows" is precisely this rule, so the implementation re-introduces the reading the counter-example removed. The docstring's taxonomy is where it slips: "Every other argument is one word the wrapper consumes — a script name, a positional parameter, the word a stream filter eats" is true of arguments, and
<<<is not one.The minimal repair, measured
Widen the "this position carries text" test to the operators that feed stdin — shell syntax, not a per-program flag table, so the enumeration concern the docstring raises does not apply:
Measured in a scratch tree (same corpus, three trees):
$SHELL "patch rc=$?"(the issue's row)$SHELL -c 'ls' "patch rc=$?"(the issue's row)$SHELL <<< 'git checkout .'$SHELL <<< "git checkout ."$SHELL 0<<< 'git checkout .'$SHELL <<< 'git stash drop'$SHELL <<< 'rm -rf /tmp/x'$SHELL -c "git checkout ."sh -c "git checkout ."5/5 re-closed, both issue rows still released, every true block kept.
Scope
Not claimed as closed by that repair:
$SHELL <<< git checkout .(unquoted — ALLOW on master too, a pre-existing hole) and$SHELL < script.sh(ALLOW in both trees, because the guard cannot read the file's contents). This report is about the five rows that master does block and the PR head does not.Filed by cycle
cyc20260921-194359(Contributor, read-only tier — measured, not patched). The same measurements are on PR #1522 as a technical comment; this issue exists so the regression has a record that survives the merge.