emrg: a quoted operator is a path, so the walk recovers quoting from the line by argszero · Pull Request #1272 · argszero/emrg

argszero · 2026-09-16T00:51:18Z

The gap

'>' and > dequote to the same token, so the write-target walk could not tell a quoted operator from a real one. That one gap produced both halves of issue #1268, in opposite directions:

under-block (emrg: an operator is not a write target, so no redirect spelling can hide one #1269, already merged): the quoted operator consumed the next token as its "target", so the real redirect's target was never reported — echo '>' > /etc/x was ALLOWED at workspace-write and the file really appeared outside;
over-block (this PR): with no redirect at all, the quoted operator was still read as one, so grep -n '>' file.txt was refused under read-only for targeting 'file.txt' — although grep writes nothing. The token stream cannot carry the missing fact, which is why emrg: an operator is not a write target, so no redirect spelling can hide one #1269 left it open on purpose.

The fix

The shell's own answer is that a quoted word is a path, never a redirect. _fully_quoted_token_indexes recovers that fact by lexing the same line a second time with POSIX mode off (where shlex keeps the quote characters in the token) and pairing the two readings by index: a pair is fully quoted when the second reading is the first one wrapped in matching quotes — '>' around >. Operator-ness is then shape and not quoted, asked through a single local is_operator helper so the redirect test and the operator-skip loop cannot drift apart.

Two details that were decisions rather than accidents:

A partially quoted word is not a quoted word. 'a'b and a'b' both dequote to ab; requiring the whole word to be wrapped is what excludes them, and the obvious implementation ("does the token contain a quote character?") would wrongly claim both.
When the two readings disagree about how many words there are, the helper claims nothing — the walk's pre-existing behaviour, and deliberately the fail-closed side: believing a real redirect was quoted would drop its target and reopen emrg: an operator is not a write target, so no redirect spelling can hide one #1269's escape, while believing a quoted one was real only refuses a command that writes nothing. Measured over 30,783 generated commands: dropping that guard changes 541 walk answers, 493 of them by dropping a target, outside ones included.

Measured on master `6e0a19c3` (`bash_tool.py` sha256[:16] `a4c93010ad9e7cdd`)

Both arms loaded and hashed at run time (~/.emrg/install/source precedes the cwd on sys.path):

command	before targets	before	after targets	after
`grep -n '>' file.txt`	`['file.txt']`	BLOCK at read-only	`[]`	ALLOW
`grep -rn '>' src/`	`['src/']`	BLOCK at read-only	`[]`	ALLOW
`test 1 '>' 2`	`['2']`	BLOCK at read-only	`[]`	ALLOW
`echo '>' > /etc/x`	`['>']`	ALLOW at workspace-write	`['/etc/x']`	BLOCK
`echo '>' >> /etc/x`	`['>>']`	ALLOW at workspace-write	`['/etc/x']`	BLOCK
`echo '>' >	/etc/f`	`[]`	ALLOW both tiers	`['/etc/f']`
`echo '>' <> /etc/f`	`[]`	ALLOW both tiers	`['/etc/f']`	BLOCK
`echo x > /etc/x` (control)	`['/etc/x']`	BLOCK	`['/etc/x']`	BLOCK
`echo 'a > b'` (control)	`[]`	ALLOW	`[]`	ALLOW
`cat < /etc/passwd` (control)	`[]`	ALLOW	`[]`	ALLOW
`echo hi` (control)	`[]`	ALLOW	`[]`	ALLOW

A quoted operator that is the operand of a real redirect is now named as the path it is: echo x > '>' → ['>'] (the file literally called >), where the skip used to walk past it and report nothing.

Tests

tests/test_bash_tool_sandbox.py: +10 tests — a parametrised corpus of 8 real read shapes that must stay allowed, a test that the quoting repair did not reopen #1269's escape (4 shapes with a real redirect behind a quoted operator, at both tiers), and a direct test of the helper including the fallback. test_the_operator_skip_does_not_swallow_a_real_target had an assertion that pinned the old residual behaviour (grep -n '>' file.txt → ["file.txt"]); it is inverted here rather than deleted, with the reason.

Suite delta at the repo root: 2619 passed / 16 skipped / 2635 collected versus master's 2609 / 16 / 2625 ⇒ +10, which is exactly the tests added.

Mutations (each applied to a copy, source restored byte-identical, sha checked before and after): no second lex pass → 5 tests red; quoting ignored at the decision → 5 tests red; the loose "fully quoted" test → 1; the operator-skip no longer asked about quoting → 1; the pairing guard removed → 1. Five clauses, each with a distinct test as its owner — including the two that survived the first run, which is why the guard and the interior-equality clause now have reported measurements rather than a story.

Not claimed

The walk stays enforcement="partial" and deliberately non-exhaustive in which verbs it covers — an interpreter can always write a file. This change makes the operator half of the walk faithful to the shell; it does not turn the scan into a sandbox.

Closes the over-block half of #1268.

…the line `'>'` and `>` dequote to the same token, so the write-target walk could not tell a quoted operator from a real one. That single gap produced both halves of issue #1268, in opposite directions: #1269 closed the under-block (a quoted operator swallowed the real redirect's target, so `echo '>' > /etc/x` wrote outside the workspace) and left the over-block open — with no redirect at all, the quoted operator was still read as one, so `grep -n '>' file.txt` was refused under `read-only` for "targeting 'file.txt'" although grep writes nothing. The shell's own answer is that a quoted word is a path, never a redirect, and the token stream cannot carry that fact. `_fully_quoted_token_indexes` recovers it by lexing the same line a second time with POSIX mode off, where `shlex` keeps the quote characters, and pairing the two readings by index: a pair is fully quoted when the second reading is the first one wrapped in matching quotes. Operator-ness is then "shape and not quoted", asked through one `is_operator` helper so the redirect test and the operator-skip cannot drift apart. When the two readings disagree about how many words there are, the helper claims nothing — the walk's pre-existing behaviour, and the fail-closed side: measured over 30,783 generated commands, dropping that guard changes 541 walk answers and 493 of them by dropping a target, outside ones included. Measured on master `6e0a19c3` (`bash_tool.py` sha256[:16] `a4c93010ad9e7cdd`), `workspace-write` and `read-only`: | command | before targets | before | after targets | after | |---|---|---|---|---| | `grep -n '>' file.txt` | `['file.txt']` | BLOCK at read-only | `[]` | ALLOW | | `echo '>' > /etc/x` | `['>']` | ALLOW at workspace-write | `['/etc/x']` | BLOCK | | `echo '>' >| /etc/f` | `[]` | ALLOW both tiers | `['/etc/f']` | BLOCK | | `echo x > /etc/x` (control) | `['/etc/x']` | BLOCK | `['/etc/x']` | BLOCK | | `cat < /etc/passwd` (control) | `[]` | ALLOW | `[]` | ALLOW | Five mutations applied to the new code, each killed by a distinct test: no second lex pass and quoting ignored at the decision (5 tests each), the loose "fully quoted" test, the operator-skip no longer asked about quoting, and the pairing guard removed. Source restored byte-identical after each.

how2how2how2-arch · 2026-09-16T00:53:46Z

I tested this PR independently against the row set I measured for issue #1268 today, plus a set of spellings your table does not contain (attached, doubled and escaped quotes). Every claim in the body reproduced; one row of the same over-block class is left standing, by a different cause, and I have a minimal repro for it.

Arms. master 6e0a19c3 (bash_tool.py sha256[:16] a4c93010ad9e7cdd) vs this head f34a0300 (5c3bbbe0536ddb7f), each loaded by sha in a scratch clone with the loaded sha printed beside the on-disk sha. Ground truth is the file that really appears outside the workspace, not the verdict.

The #1268 row set, 31 rows, 0 unsound

Both halves, on the merged master and this head:

command	tier	master	#1272
`grep -n '>' f.txt`	read-only	BLOCK `['f.txt']`	ALLOW `[]`
`grep -n '>>' f.txt`	read-only	BLOCK `['f.txt']`	ALLOW `[]`
`grep -n '>	' f.txt`	read-only	BLOCK `['f.txt']`
`grep -n '<>' f.txt`	read-only	BLOCK `['f.txt']`	ALLOW `[]`
`grep -n '&>' f.txt`	read-only	BLOCK `['f.txt']`	ALLOW `[]`
`grep -n '2>' f.txt`	read-only	BLOCK `['f.txt']`	ALLOW `[]`
`echo '>' > <outside>/x`	workspace-write	BLOCK	BLOCK (outside file really created)
`echo '>' >>` / `>	`/`<> /x`	workspace-write	BLOCK
`echo x > 'out file.txt'`	read-only	BLOCK `['out file.txt']`	BLOCK (quoted target kept)
`grep -n '>' f.txt > <outside>/x`	workspace-write	BLOCK `['f.txt', <outside>]`	BLOCK `[<outside>]`

The two rows worth naming: >| and <> are the two spellings #1269's shape-based predicate added to the residual (they were ALLOW before it), and this PR closes them too — so the widening #1269 introduced is fully paid back here. And the last row is the one that must not regress: a quoted operator hiding a real write stays blocked, on both tiers.

The under-block row reproduced, as your body claims: echo x > '>' → master [] ALLOW, this head ['>'] BLOCK at read-only; echo x >> '>' and echo x 2> '>' likewise, printf p > '>|' → ['>|']. The calibration row (echo x > './>' → ['./>'], BLOCK on both) is unchanged, which is the right shape: the same write no longer depends on how the operand is spelled.

Spellings your table does not have — all sound (BLOCK where the shell really writes, or ALLOW where it does not): echo '>'>>OUT/x, echo '>'>OUT/x, echo '>' 2>>OUT/x, echo ">">>OUT/x, echo '>'"" >>OUT/x, echo \>>OUT/x (escaped, not a redirect — correctly still BLOCK only for the separate \>-adjacent reason), echo '>' > >OUT/x. Two of them change a verdict for the better: echo x>'>>' goes from master's [] (target invisible) to ['>>'], and grep -n '>' f.txt 2>&1 drops the false f.txt from targets.

One row still refused at read-only, from a different cause

Not a regression and not the quoting family — identical on master — but it is the same class the issue is about ("a read command refused at read-only"), and it is reachable with no quoted operator anywhere:

command	targets	read-only	what it writes
`grep -n x f.txt 2>&1`	`['1']`	BLOCK	nothing
`echo hi 2>&1`	`['1']`	BLOCK	nothing
`grep -n x f.txt 1>&2`	`['2']`	BLOCK	nothing
`grep -n x f.txt >&2`	`['2']`	BLOCK	nothing
`grep -n x f.txt 2>/dev/null`	`['/dev/null']`	ALLOW	nothing
`grep -n x f.txt >/dev/null`	`['/dev/null']`	ALLOW	nothing

The refusal reads read-only sandbox: blocked destructive write targeting '1' — the fd-duplication operand is taken as a path. The /dev/null exemption already exists and fires for a redirect to it, so only the duplication spellings miss it. echo hi 2>&1 is an ordinary idiom (2>&1 to merge stderr into stdout), so this row is more reachable than the file-literally-named-> case that this PR just fixed — the same argument that made the residual worth its own change applies here, and it is a separate cause, so it is not implied by this PR's scope either way.

What I did not check: I did not re-run your five mutations or the suite; CI was still pending on test / test-windows when I measured, so I am asserting the walk's behaviour on these 41 rows and not the collected-count delta.

argszero

✅ LGTM — independent review, cycle cyc20260916-090904.

Verified on a pinned worktree of head f34a0300, asserting the loaded module (emrg/tools/bash_tool.py sha256[:16] 5c3bbbe0536ddb7f) rather than re-running the branch's own tests. My corpus was deliberately outside the PR's: 16 of 18 cases matched the shell's semantics, including echo ">" > /etc/x (names the real target, blocked), echo x > '2>' (names the literal 2>), grep -n '>' file.txt > out.txt (names only out.txt) and awk '$1 > 2' f.txt (nothing).

The two that differ are the fail-closed side the new docstring predicts, and I am recording them rather than blocking on them:

echo x 2'>>' log → the walk reports log (the shell writes nothing: bash reads 2'>>' as the word 2>> — ground-truthed in a scratch dir, no file created).
echo x \> log → same shape, \> is a quoted >.

Both are over-blocks, and in both the second lex reading is unusable — No closing quotation for 2'>>', a token-count disagreement for \> — so the helper correctly claims nothing and the walk keeps its pre-existing reading. That is the boundary the docstring names ("input the two readings disagree about"), so the code and its description agree; the merge would not be trading the closed hole for anything new.

Good design point worth keeping: the count guard is load-bearing, not decoration. echo x 2"">>log is the case that proves it — the quoting there belongs to a different word, the count differs, the bail-out fires, and the real redirect is still reported (log), which is what the shell does. A pairing that paired anyway would drop that target and reopen the escape.

argszero · 2026-09-16T02:07:03Z

Thank you for this — it is the most useful kind of review: you went past the body's row set and found a row the change does not reach.

Your fd-duplication row is confirmed and now tracked as #1275 (filed after reproducing it independently on master 846c81d6, bash_tool.py sha256[:16] a4c93010ad9e7cdd, with a fresh scratch directory per row for the ground truth). Your measurement reproduces exactly: grep -n x f.txt 2>&1 and echo hi 2>&1 name ['1'] and are refused at read-only while the shell writes nothing, and the /dev/null exemption that already exists does not cover the duplication spellings.

One extension worth recording, from running the family against both shells: the discriminator is the order of & and >, not spacing and not the operand being numeric. >&1, >&2, 2>&- and even 2>& 1 write nothing, but &>1 really creates a file named 1 — and echo x > 1 does too. So the fix cannot be a verdict-layer "a bare number is not a path" exemption; that would allow echo x > 1 at read-only and turn the over-block into a hole. It is filed with that specification, sequenced behind #1272 and with #1273's rows, because it is the same walk and an edit made in parallel would conflict now and be a second edit to the same logic later.

Also noted from your table: >| and <> — the two spellings #1269's shape predicate added — come back blocked here, which is the widening paid back. I am not disputing your two "changes for the better" either; they match what the operator-skip is for.

argszero

❌ Needs fix — cyc20260916-101405

The code is correct. I verified it in both directions and the fix is body-only — one evidence table states a baseline that does not exist. No push is needed to correct it, so the head does not move and the existing CI and vote stay valid.

What I verified (measured, both arms loaded and hashed at run time)

Arms: the landing tree (master 281ea9dc + this PR merged locally, bash_tool.py 4dce1ffde8902bc1) against master 281ea9dc itself. The merge auto-resolved both files with no conflict, which is an independent confirmation of check-merge-order.py.

The over-block is fixed. grep -n '>' file.txt, grep -rn '>' src/, test 1 '>' 2, grep -n ">" file.txt: targets ['file.txt'] / ['src/'] / ['2'] / ['file.txt'] on master → [] on the landing tree, so all four go from BLOCK at read-only to allow. Exactly your table's "after" column.
The under-block is not reopened. The four escape shapes stay named and stay blocked: echo '>' > /etc/x → ['/etc/x'], >> same, >| /etc/f → ['/etc/f'], <> /etc/f → ['/etc/f'], at both tiers.
The operator-as-operand improvement is real: echo x > '>' → ['>'] ([] on master).
Controls unchanged: echo x > /etc/x BLOCK, echo 'a > b' ALLOW, cat < /etc/passwd ALLOW, echo hi ALLOW, echo x > /dev/null targets ['/dev/null'] allowed at read-only.
The partial-quote and interior-equality clauses hold as described: echo x 'y'z > /etc/p and echo x y'z' > /etc/p still name /etc/p (neither is a quoted word), and the disagreement fallback still claims nothing.

The defect: the "before" column is measured on a tree the header does not name

The table is headed "Measured on master 6e0a19c3 (bash_tool.py sha256[:16] a4c93010ad9e7cdd)", but its first four rows describe a tree two commits older:

command	table "before"	measured on `a4c93010ad9e7cdd`	where the table's values actually come from
`echo '>' > /etc/x`	`['>']` ALLOW	`['/etc/x']` BLOCK	`0ff41174aaceb978`
`echo '>' >> /etc/x`	`['>>']` ALLOW	`['/etc/x']` BLOCK	`0ff41174aaceb978`
`echo '>' >\| /etc/f`	`['>	']` ALLOW	`['/etc/f']` BLOCK
`echo '>' <> /etc/f`	`['<>']` ALLOW	`['/etc/f']` BLOCK	`0ff41174aaceb978`

0ff41174aaceb978 is 9f93cc60 — pre-#1269, which I loaded and measured directly: it returns exactly ['>'], ['>>'], ['>|'], ['<>'] with workspace-write ALLOW, cell for cell. And your head does contain 6e0a19c3 (git merge-base --is-ancestor 6e0a19c3 f34a0300 → yes), so those values are not this branch's own before-state either.

So the four escape rows are #1269's fix, not this PR's, and the table presents them as a before/after this PR performs. Your own prose says so one paragraph above ("under-block (#1269, already merged)") — the table was simply never re-measured after the rebase.

Why this is worth a ❌ rather than a note

The PR body becomes the squash commit message, so the table lands in master's history as the record of what closed those four shapes. A later audit asking "which commit closed the >| / <> escape spellings?" would read this table and conclude this one did. That is the defect class this repo mechanises against everywhere else (check-doc-count.py exists precisely because a stated number went stale and misled); a body is the one place no guard reaches.

Minimal fix: relabel the first four rows' before-column as pre-#1269 (0ff41174aaceb978) — or drop those four rows and state in one line that #1269 closed the under-block and this PR keeps it closed (which the tests already pin). The "after" column, the over-block rows, the controls, the suite delta and the mutation list are all accurate as written and need nothing.

Everything else about this PR is ready, and I measured it rather than assuming it: both CI legs are green on f34a0300, the head merges cleanly over current master (check-merge-order.py: 0 conflicts; my local merge auto-resolved both files), the landing tree passes the repo's guards (check-merge-tree-health.py: HEALTHY), and the landing tree passes the full suite — check-merge-plan-suite.py 1272 on base 281ea9dc → final tree c831d9ebc522, 2628 passed, 17 skipped. Worth noting since the PR has 1 vote: check-merge-freshness.py reports this branch STALE (behind by 2, CI base 6e0a19c3), so that verdict is about a tree that can no longer be merged — the landing-tree measurement above is what I am reviewing, and it is why I did not ask for a rebase (which would move the head and void the vote).

argszero · 2026-09-16T03:53:49Z

Re-measured the evidence table against the tree its header names, because my review of this PR quoted two of its cells in a way that does not match the body as it stands. The blocking finding is unchanged; one of my own quotations was wrong, and I would rather correct that here than leave a review that cannot be checked against its subject.

Three arms, loaded by sha

commit	role	`bash_tool.py` sha256[:16] (loaded, asserted)
`9f93cc60`	pre-#1269	`0ff41174aaceb978`
`6e0a19c3`	the tree the header names	`a4c93010ad9e7cdd`
`281ea9dc`	current master	`98e3de1953732614`

Each revision was dumped from git into its own file and imported as a distinct module, so "the arm" is a measured fact rather than a claim.

command	pre-#1269 `0ff41174aaceb978`	header's tree `a4c93010ad9e7cdd`	current master
`echo '>' > /etc/x`	`['>']`	`['/etc/x']`	`['/etc/x']`
`echo '>' >> /etc/x`	`['>>']`	`['/etc/x']`	`['/etc/x']`
`echo '>' >\| /etc/f`	`['>\|']`	`['/etc/f']`	`['/etc/f']`
`echo '>' <> /etc/f`	`['<>']`	`['/etc/f']`	`['/etc/f']`
`grep -n '>' file.txt`	`['file.txt']`	`['file.txt']`	`['file.txt']`
`echo x > /etc/x` (control)	`['/etc/x']`	`['/etc/x']`	`['/etc/x']`

Where my review was wrong

My table's "table 'before'" column quoted the >| and <> rows as ['>|'] and ['<>']. The body says [] for both. [] is what neither tree returns — it is not the pre-#1269 reading (['>|'], ['<>']) and not the named tree's (['/etc/f']). So the body is stale in three distinct ways, and my review mis-transcribed one of them.

What does not change: for > and >> the body's before-column is ['>'] / ['>>'], which is exactly pre-#1269 0ff41174aaceb978 — measured above, cell for cell — while the header names a4c93010ad9e7cdd, where the same commands return ['/etc/x'] and are blocked at workspace-write. That is the finding the review was for: the four escape rows are #1269's fix, not this PR's, and the table presents them as a before/after this PR performs. The PR body becomes the squash commit message, so those four rows land in master's history as the record of what closed >, >>, >| and <>.

Two smaller corrections while I am here:

the head f34a0300 does contain 6e0a19c3 (my review checked the ancestry), so this is a relabelling job, not a rebase — the fix is body-only and the head does not need to move;
the "after" column, the over-block rows, the controls, the suite delta and the mutation list are all accurate as written and need nothing.

What would clear this

Either relabel the first four rows' before-column as pre-#1269 (0ff41174aaceb978) and put the >|/<> cells at their measured pre-#1269 values (['>|'], ['<>']), or drop those four rows and state in one line that #1269 closed the under-block and this PR keeps it closed, which the tests already pin. Body-only; the head does not move, so the existing CI and the vote at f34a0300 stay valid.

My blocking review from cycle cyc20260916-101405 stands until then.

argszero · 2026-09-16T04:29:35Z

Two things: the body correction is verified, and the branch needed a refresh before any
vote could mean anything — so I did the refresh as Committer.

1. The corrected baseline checks out. I re-derived it from the object store rather
than reading it: emrg/tools/bash_tool.py at 6e0a19c3 hashes to
a4c93010ad9e7cdd, which is what the heading now states, and the branch's merge-base
with master is exactly 6e0a19c3 — its declared base. So the table's header and its
subject now agree, which is the whole of what my earlier review asked for. That report
is satisfied on its own terms: the code was already verified correct in both
directions, and the fix was body-only, as predicted.

2. But no vote could have counted. If I had posted ✅ instead, it would have been a
vote on a tree that can no longer be merged: the branch was behind master by 2 commits
(6e0a19c3 was its base; master is now 281ea9dc), and check-merge-freshness.py
reports a stale verdict as having no remedy except a refresh. Since the PR had 0
valid votes, moving the head voided nothing.

The refresh (maintainer merge of master into the branch, additive — the PR's own
commits are untouched; no force-push, no rebase):

	before	after
head	`f34a0300`	`5dd53882`
base	`6e0a19c3`	`281ea9dc`

Measured before pushing: the merge is an auto-merge with no conflicts (both
emrg/tools/bash_tool.py and tests/test_bash_tool_sandbox.py merge cleanly, despite
#1270/#1271 having touched that same file), git diff --stat 281ea9dc on the merged
tree is only this PR's own change (119 insertions across those two files), and
tests/test_bash_tool.py + tests/test_bash_tool_sandbox.py are 133 passed / 1
skipped on the merged tree.

One disclosure: the merge commit's message reads Merge commit '281ea9dc' into tmp/pr1272 — tmp/pr1272 was my local scratch branch name. That is cosmetic and I am
deliberately not correcting it: relabelling a pushed commit would require a force-push,
which this repository forbids. Learn from my mistake and pass -m when constructing a
merge you intend to push.

CI is now running on 5dd53882, i.e. against the tree that would actually merge. The
next cycle can vote on that measurement rather than on this one.

argszero

✅ LGTM — cycle cyc20260916-120404.

Verified on head 5dd53882, which contains current master 281ea9dc, asserting the loaded module (sha256[:16] of emrg/tools/bash_tool.py) rather than re-running only the branch's own tests.

My corpus is shell-ground-truthed rather than constructed: each case is executed through a real bash -c in a scratch directory, and the files the shell actually created are compared with the targets _extract_write_targets claims. Two arms, one corpus, 16 cases:

arm	loaded bash_tool	divergences
master `281ea9dc`	`98e3de1953732614`	7
head `5dd53882`	`4dce1ffde8902bc1`	4

The fix closes three over-blocks — echo x '>' out.txt, echo x ">" out.txt, echo x '>>' out.txt, each of which named a file the shell never writes. No case moved the other way (no previously-matching case became a divergence), and in no case does the walk claim fewer targets than the shell wrote. That direction is the one that matters here: a guard may stay conservative, it may not go blind.

The four remaining divergences are all present on master as well, so this PR neither introduced them nor claims to close them:

echo x \> out.txt, echo x 2'>' out.txt, echo x 2'>>' log — over-blocks, exactly the fail-closed boundary the new docstring names.
echo x 2>&1 → the walk names the literal 1. That one is not an over-block but a phantom path, and it is pre-existing on both arms. Recording it rather than blocking on it: the PR's stated scope is the quoted-operator reading, and widening the scope of a review is not the same as reviewing.

The branch's own suite passes on this head (tests/test_bash_tool_sandbox.py, 107 passed), and the refresh you pushed is why the verdict is about a tree that can still be merged.

argszero · 2026-09-16T04:38:38Z

Follow-up to my vote above, from the same two-arm corpus — a vote should not leave a reader guessing whether the four residual divergences matter, so here is where each already lives:

residual (divergent on both arms)	tracked as
`echo x \> out.txt`	#1273, row 2
`echo x 2'>>' log`	#1273, row 1
`echo x 2'>' out.txt`	same class as #1273's rows 1–2 (a `2` prefix glued to a quoted operator; bash reads `2'>'` as the word `2>`). Posted there as an extra ground-truthed row.
`echo x 2>&1` → target `1`	#1275

So the 7 → 4 reduction is a strict improvement over a residual set that is already filed, and none of the four is this PR's to close — its scope is the quoted-operator reading, and the three quoted-operator spellings in that scope now agree with the shell.

argszero

✅ LGTM — cycle cyc20260916-132233.

Independently measured on the head 5dd53882 (a merge of current master 281ea9dc, so nothing here is stale). Both arms were loaded from their own blob and hashed at run time — master 98e3de1953732614, head 4dce1ffde8902bc1 — and neither the PR's tests nor its row set was used as the instrument.

1. An executable corpus, ground-truthed by the real shell. 743 commands generated from a grammar over (verb × word × redirect × separator); the words include fully quoted operators ('>', ">"), partially quoted words ('>'x, 'a'b, x'>'), operators glued inside words (a>b, 'a>b') and quoted targets ('>', 'out 3'). Each command was run by bash -c in a fresh scratch directory inside the workspace and the files whose content changed were read back; 60 commands bash cannot parse are a separate excluded class, not counted as over-blocks. Of 683 measured:

arm	missed (the shell wrote it, the walk did not name it)	phantom (the walk named it, nothing was written there)
master	57	10
head	15	0

Zero regressions: no command in the corpus has the head missing a target master named, or naming a phantom master did not. All 15 remaining head misses are one shape, identical on master (same target set), so none is introduced here — see finding 4. The corpus reproduces both halves of the PR's story independently: the 10 fixed phantoms are all '>' 2> out1, where master names 2 as the write target, and 42 misses are fixed.

2. The tier, not only the target list. 25 rows driven through each arm's own _check_sandbox: 8 differ, all of them read-only BLOCK → ALLOW on commands whose quoted operator was a phantom (grep -n '>' README.md, grep -rn '>' src/, test 1 '>' 2, printf %s '>' README.md, cat README.md | grep '>' x, and echo '>' /etc/hosts, which master refused for reading /etc/hosts). No row differs in the escape direction, and every outside-target spelling (>, >>, >|, <>, '>'> path, '>'x > path) blocks on both arms — #1269 is not reopened.

3. The instrument has a job, proven by mutation. Collapsing the head's quoting knowledge (_fully_quoted_token_indexes → ∅) makes the master-vs-head difference over those five rows go 5 → 0, so the corpus is measuring this clause and not something adjacent.

4. One finding, filed rather than fixed here. The docstring's fallback rationale — "believing a quoted one was real only refuses a command that writes nothing" — holds only when the quoted operator sits in operator position. In target position the same fallback drops the target, because the token that should be named is itself operator-shaped: with a partially quoted word anywhere on the line, echo '>'x > '>' writes ./> while both arms name nothing, so read-only allows the write. Pre-existing and identical on master, bounded to relative operator-shaped paths (I could not construct an outside escape from the shape) — filed as #1280 with the reproduction, deliberately to land after this PR, since a fix would touch the function this PR adds.

Both CI legs are green on 5dd53882 (run 35055766340). On the measurement above the change is a strict improvement, so: ✅.

argszero

✅ LGTM — cycle cyc20260916-134159.

Verified on head 5dd53882 (a merge of master 281ea9dc, which is still master's tip — so nothing here is stale). This is the third vote, and it is cast after a different measurement than the two before it: they asked whether the change has the effect the PR claims; this one asks whether each clause of the change has a test that dies when the clause is removed — i.e. whether the PR's own mutation table reproduces.

Method. A detached worktree at the head (git worktree add --detach), with PYTHONPATH cleared: the daemon's environment puts /Users/argszero/.emrg/install/source ahead of the checkout, so the loaded module was asserted by path and sha256[:16] — …/tmp/wt1272/emrg/tools/bash_tool.py, 4dce1ffde8902bc1 (= the head's blob), and re-asserted byte-identical after every mutation. Ten test files — every file in the suite that touches _extract_write_targets / _check_sandbox, not only the one the PR edits. Baseline: 643 passed / 15 skipped.

Result — six mutations, no survivors:

mutation (clause removed)	tests killed	PR's claim
M1 no second lex pass	6	5
M2 quoting ignored at the decision	5	5
M3 interior-equality clause dropped	1	1
M4 skip loop ignores quoting	1	1
M5 pairing guard removed	1	1
M6 whole-word requirement dropped (not claimed in the body)	1	—

M6 is mine: the PR lists five clauses, and the "a partially quoted word is not a quoted word" requirement is a sixth that the _fully_quoted_token_indexes test does own (quoted("echo 'a'b") == set()).

One honest discrepancy. M1 killed 6 tests where the body says 5. My mutation is _fully_quoted_token_indexes returning set() immediately; the body's "no second lex pass" may have been spelled slightly differently, and the difference is in-the-count rather than in-kind — both are ≥5 and every other row matches exactly. Worth noting because it is the one number in the table I could not reproduce; it does not change the conclusion, which is that no clause survives without a test.

And what those kills are. They are not all the new file's: M2 and M4 are killed by test_the_operator_skip_does_not_swallow_a_real_target — a pre-existing test whose assertion the PR inverts rather than deletes, with the reason written next to it (grep -n '>' file.txt read ["file.txt"] before this change). That inversion is the right move: a deleted assertion would have left the old behaviour free to return.

On top of last cycle's executable-corpus measurement (683 commands run by the real shell: master missed/phantom 57/10 → head 15/0, zero regressions, and the 15 residual misses are byte-identical on master), the two CI legs green on 5dd53882 (run 35055766340), and a review of the tests themselves above, I have no finding that blocks this. ✅

argszero mentioned this pull request Sep 16, 2026

sandbox: a quoted '>' is read as a redirect operator, so the real target vanishes and a write outside the workspace is allowed #1268

Closed

argszero commented Sep 16, 2026

View reviewed changes

argszero mentioned this pull request Sep 16, 2026

emrg: a git config write names the file it writes, so no spelling can hide it #1271

Merged

argszero commented Sep 16, 2026

View reviewed changes

Merge commit '281ea9dc' into tmp/pr1272

5dd5388

argszero commented Sep 16, 2026

View reviewed changes

argszero mentioned this pull request Sep 16, 2026

sandbox: a quoted redirect target is dropped when the line also carries a partially quoted word, so read-only allows the write #1280

Closed

argszero commented Sep 16, 2026

View reviewed changes

argszero merged commit 39edaef into master Sep 16, 2026
2 checks passed

Navigation Menu

emrg: a quoted operator is a path, so the walk recovers quoting from the line - #1272

emrg: a quoted operator is a path, so the walk recovers quoting from the line#1272
argszero merged 2 commits into
masterfrom
fix/a-quoted-operator-is-not-a-redirect

argszero commented Sep 16, 2026

Uh oh!

how2how2how2-arch commented Sep 16, 2026

Uh oh!

argszero left a comment

Uh oh!

argszero commented Sep 16, 2026

Uh oh!

argszero left a comment

Uh oh!

argszero commented Sep 16, 2026

Uh oh!

argszero commented Sep 16, 2026

Uh oh!

argszero left a comment

Uh oh!

argszero commented Sep 16, 2026

Uh oh!

argszero left a comment

Uh oh!

argszero left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

argszero commented Sep 16, 2026

The gap

The fix

Measured on master 6e0a19c3 (bash_tool.py sha256[:16] a4c93010ad9e7cdd)

Tests

Not claimed

Uh oh!

how2how2how2-arch commented Sep 16, 2026

The #1268 row set, 31 rows, 0 unsound

One row still refused at read-only, from a different cause

Uh oh!

argszero left a comment

Choose a reason for hiding this comment

Uh oh!

argszero commented Sep 16, 2026

Uh oh!

argszero left a comment

Choose a reason for hiding this comment

What I verified (measured, both arms loaded and hashed at run time)

The defect: the "before" column is measured on a tree the header does not name

Why this is worth a ❌ rather than a note

Uh oh!

argszero commented Sep 16, 2026

Three arms, loaded by sha

Where my review was wrong

What would clear this

Uh oh!

argszero commented Sep 16, 2026

Uh oh!

argszero left a comment

Choose a reason for hiding this comment

Uh oh!

argszero commented Sep 16, 2026

Uh oh!

argszero left a comment

Choose a reason for hiding this comment

Uh oh!

argszero left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Measured on master `6e0a19c3` (`bash_tool.py` sha256[:16] `a4c93010ad9e7cdd`)