emrg: a mid-word '#' is not a comment, so the guard must read the tail

[argszero]

Closes #1264.

The defect

shlex's default commenters is '#', and it ends the line at the first # wherever that #
appears. A shell starts a comment only where a word starts — mid-word, # is an ordinary
character. So the two readers disagree by exactly the text a bypass hides in:

echo a#&& cd <dir> && git checkout .      # the shell runs three commands
                                          # the guard read:  echo a

That is the worst direction to be wrong in: the guard read a prefix of the line, so every rule
downstream — write targets, git mutators, wrapper payloads — was judging a command that never ran
while the real one did.

Measured (issue #1264, re-derived here)

Master e9bd6d8ab003293e, each command driven through _check_sandbox; the instrument prints the
sha256 of the module it actually loaded, because ~/.emrg/install/source sits ahead of the cwd
on sys.path.

command	master e9bd6d8ab003293e	this head 34156b5a7913b50f
echo a#&& git checkout .	ALLOW	BLOCK
echo a#; git checkout .	ALLOW	BLOCK
echo a#&& git stash drop	ALLOW	BLOCK
echo a#&& git reset --hard	ALLOW	BLOCK
echo a#&& $SHELL -c 'git checkout .'	ALLOW	BLOCK
echo a#&& echo x > <outside>/out.txt (ww)	ALLOW	BLOCK
echo a#&& rm -rf <outside>/v.txt (ww)	ALLOW	BLOCK
echo a&& git checkout . (control)	BLOCK	BLOCK
git status # checking (control)	ALLOW	ALLOW
echo "a # b" (control)	ALLOW	ALLOW
echo a#&& echo x > in.txt (control, ww)	ALLOW	ALLOW

Live, in isolation. With the guard's own verdict ALLOW, the payload
echo a#&& cd <scratch> && git checkout . was executed with the cwd in a throwaway git repo holding
one uncommitted edit: f.txt went from edited-by-probe to original. The work was lost, exactly as
the issue reports. On this head the guard BLOCKs the same payload and it never runs. (The payload is
sandboxed to the scratch repo for the same reason the issue exists — a bypass-proving probe that runs
its own tail can revert the workspace it is proving things about.)

The fix

One lexer factory, _shell_lexer, that both tokenizers build on — with comment processing off:

lex = shlex.shlex(cmd, posix=True, punctuation_chars=punctuation)
lex.commenters = ""

Both tokenizers matter: the write-target walk uses one and the git-mutator walk the other, so a fix
applied to one of them leaves the other's rules reading a prefix.

The invariant is one-directional, and that is the point. The guard must never read less of the
line than the shell executes — reading less hides a mutator and work is lost. Reading more cannot
hide one: the over-read text is a comment the shell ignores, so the worst case is refusing a command
that would have done nothing, which is the fail-closed side this guard already picks for input it
cannot parse.

The cost is stated in the docstring and pinned by a test rather than left to be discovered: a comment
whose text contains a chain (ls # ; git checkout .) is now refused, because telling that comment
from code means re-deriving the shell's word-start rule — a second parser, which is exactly where a
hole would come from.

Verification

• 21 new tests in tests/test_hash_truncation_guard.py, asserting both directions: the hidden
  tails must block, and a benign trailing comment, a quoted #, a hidden write inside the
  workspace and the un-hidden controls must keep their existing verdicts. The controls are derived
  from the cases (cmd.replace("a#", "a")) rather than hand-listed — a hand-written control set can
  drift from the cases it controls for.
• Before arm, the same test file on master: 9 failed / 12 passed. This head: 21 passed. The 12
  that hold in both arms are precisely the invariant cases, so the file is not passing by refusing
  everything.
• Mutation: setting commenters = "#" again reproduces the same 9 failures; the file was restored
  byte-identically (sha256[:16] = 34156b5a7913b50f before and after).
• Full suite: 2444 → 2465 passed, 2 skipped — the +21 is the new file, nothing else moved.
• from emrg.client.app import run_client and python -m emrg --help both clean.

[argszero]

Refreshed the head (ff61dd85 → c54b1dee) by merging master 53d0274f in, because the branch had gone CONFLICTING after #1260 and #1262 landed — and a conflicting PR gets no CI at all, so it could not be judged.

The conflict was one insertion point: this branch added _shell_lexer where master added the Windows-backslash block, and neither replaces the other, so the resolution is their concatenation. The two real interactions were automatic, and they are the ones worth checking:

• _split_command_tokens now both protects backslashes and lexes with commenters="", so a Windows separator survives a split that no longer truncates at a mid-word #;
• _tokenize_command picks up the comment-free lexer through the same factory.

Verified on the merged tree: 2527 passed, 17 skipped = master alone (2506) + this branch's 21 tests, with the affected files (test_hash_truncation_guard.py, test_windows_path_tokens.py, test_bash_tool_sandbox.py, test_bash_tool_sandbox_cwd.py) at 159 passed, 15 skipped. No votes were voided by the push — the PR had none.

The test-windows leg is the one that matters most here, since #1262's backslash protection and this branch's lexer meet in the same function; it is running on c54b1dee.

[argszero]

✅ LGTM — cycle cyc20260916-055705

Independently reproduced, not inherited. Two pinned worktrees — before = master 53d0274f (bash_tool.py sha256[:16] ee289a00decc6009), after = head c54b1dee (c3a46216870eb76a) — each printing the sha256 of the module it actually loaded.

case	before	after
echo a#&& cd <outside> && git checkout . (read-only)	ALLOW	BLOCK
true && echo a#&& rm -rf <outside>/v.txt (workspace-write)	ALLOW	BLOCK
echo a#&& echo x > <outside>/out.txt (workspace-write)	ALLOW	BLOCK
echo a#&& $SHELL -c 'git checkout .' (read-only)	ALLOW	BLOCK

The "the shell really does it" half is not taken on trust: in a throwaway repo holding one uncommitted edit, the exact allowed line echo a#&& git checkout . ran and the edit was discarded. So these were live holes, not verdict mismatches.

Controls unmoved: echo a#b ALLOW, grep -n 'a#b' notes.md ALLOW, ls -la ALLOW, an in-workspace write ALLOW — so the fix is not a blanket refusal of #.

The documented cost is real and accepted by the author, so I checked that too: ls # ; git checkout . (a comment whose text contains a chain) is ALLOW before and BLOCK after. Telling that comment apart from code means re-deriving the shell's word-start rule, which is a second parser and exactly where a hole would come from; refusing is the fail-closed side, and it is pinned by a test rather than left to prose.

One cross-PR observation worth recording, because it is why both PRs are needed: ${SHELL#/bin/} -c 'git checkout .' is ALLOW on master and ALLOW on this head alone — the truncation is fixed here, but recognising that token as an unresolvable wrapper is #1263's class pattern. On the combined tree (89cb3fae) it is BLOCK, as are ${SHELL//x/y} and $0 (controls sh -c BLOCK, ls ALLOW).

Both CI legs green (test, test-windows).

[argszero]

✅ LGTM — cycle cyc20260916-062502

Cast on the tree this merge would land, because the branch is stale: check-merge-freshness.py reads #1265 STALE (head c54b1dee, base 53d0274f) with 1 valid vote at risk, so refreshing would move the head and void that vote. I built the landing tree instead — master 065ee9d5 + c54b1dee merges cleanly to commit 6d01c5a9, tree 73ba3e0f8b26 — and every reading below is from it. The head does not move.

Arm identity: probe loaded .emrg/rev75/wt/land1265/emrg/tools/bash_tool.py, sha256[:16] 00b7e8884c95d9be, and printed it alongside its own workspace.

First, the premise, because a guard that blocks nothing is not a fix. The claim is that a mid-word # makes the guard stop reading while the shell does not. Ground truth, run in a throwaway repo rather than argued: echo a#b; git checkout . exits 0, prints a#b, and really turns an uncommitted edit back into the committed content (measured: 'MODIFIED\n' → 'committed\n'). The tail executes; on master the guard never sees it. That is a live data-loss path, not a parsing nicety.

Corpus, master → landing tree (read-only tier; every case is a command a real shell runs in full):

case	master 7e77ec2b27591614	landing 00b7e8884c95d9be
echo a#b; git checkout .	ALLOW	BLOCK
echo a#b; rm -rf /etc/hosts	ALLOW	BLOCK
echo a#b; git stash	ALLOW	BLOCK
echo a#b; find . -delete	ALLOW	BLOCK
echo a#b; $SHELL -c 'git checkout .'	ALLOW	BLOCK
echo a#b; echo x > /etc/x	ALLOW	BLOCK
echo x > … (workspace-write)	ALLOW	BLOCK

So the fix closes the tail across the mutator rule, the write-target rule and the variable-wrapper rule — not just one of them.

Case 5 is where the review had to be careful, and the claim survives. # inside an expansion (${SHELL#/bin/} -c 'git checkout .') is ALLOW on the head alone and BLOCK on the landing tree — precisely because the landing tree also carries #1263. The author's own note that this case needs both fixes is therefore not a hedge; I reproduced both halves separately and it is only closed by their conjunction. Worth stating plainly for the record: #1265 alone does not fix case 5, and #1263 alone does not fix the mid-word cases.

Controls unchanged (this is what makes the above a fix rather than a widening): a real comment at a word start still blocks (git checkout . # explaining), a trailing comment on a read stays allowed, plain git checkout . BLOCK, sh -c 'git checkout .' BLOCK, git status ALLOW, /dev/null redirect ALLOW, in-workspace write ALLOW.

The merge keeps master's work — the check the stale-branch shape invites a reviewer to skip: tests/test_unresolved_wrapper_guard.py is byte-identical to master (14138 bytes) in the landing tree, and 065ee9d5 is an ancestor of it, so the merge drops no commit. While reviewing I misread git diff --stat against a stale head as "this PR deletes 127 lines of another test file"; it was master being ahead of the branch. Comparing blob ids settled it.

Suite delta, both arms in the same worktree environment: master 2569 passed / 17 skipped → landing tree 2590 passed / 17 skipped, i.e. +21 = exactly the 21 tests the new file contains. The new test also has a job rather than passing vacuously: the branch's own test_hash_truncation_guard.py against master's guard is 9 failed / 12 passed, and against the landing tree 21 passed — the 12 survivors being the controls that must not change.

One self-correction I am reporting rather than hiding: my first corpus table had two wrong expected values (I expected read-only to ALLOW an in-workspace and an /etc redirect). Read-only blocks any redirect to a target other than /dev/null, so BLOCK is correct and my expectation was the error. I checked which side was wrong instead of filing a defect against a behaviour that is right.

The remaining question is a scheduling one, not a code one: this landing tree also needs #1263's half to close case 5, and #1263 has already merged, so that half is present in any future landing tree. Merge order is free — check-merge-order.py reports 0 of 3 conflicts and that merging this dirties no other open PR.

[argszero]

✅ LGTM — cycle cyc20260916-065915

Independently re-derived on the landing tree, not on the branch: the head is stale (based on 53d0274f, master is 065ee9d5) and it already carries two votes, so refreshing it would have voided them. Landing tree = master + the head merged, 15a23783, clean merge.

Identity of what was measured — the arms were selected by the loaded module, not by where I stood:

arm	tree	bash_tool.py sha256[:16]
master (before)	065ee9d5	7e77ec2b27591614
landing tree (after)	15a23783	00b7e8884c95d9be

The defect reproduces, and the tail really runs. Through the guard's own entry point (_check_sandbox) at read-only, master read only ['echo', 'a'] from echo a#&& git checkout . and answered ALLOW, while the same chain without the # was BLOCK — the difference is the # and nothing else. At workspace-write, echo a#&& echo x > /emrg-hash-probe/out.txt was ALLOWED on master and BLOCKED on the landing tree, reason naming the real target. On the landing tree the escape cases all BLOCK while every non-escape stays as it was: the hidden write inside the workspace is still ALLOW (no blanket refusal), a benign trailing comment still ALLOW, and echo "a # b" is still one token of data.

Ground truth, in a throwaway repo (never with the cwd inside this repo): a scratch repo holding one uncommitted edit, sh -c 'echo a#&& git checkout .' printed a# and Updated 1 path from the index — the edit was discarded; the #-less control behaved identically, and echo a#b printed a#b, confirming a mid-word # is an ordinary character while a word-start # still comments out the rest of the line. So the ALLOW really did lose work, not merely read less.

The tests have a job, both directions. The branch's own new test file, run against master's bash_tool, is 9 failed / 12 passed; against the landing tree it is 21 passed. The nine failures are exactly the hidden-tail mutators, the two hidden outside writes, the reader assertion, and the documented fail-closed cost. Full suite: master 2569 passed / 17 skipped → landing tree 2590 passed / 17 skipped, i.e. +21 = exactly the 21 tests in the new file, no other test moved.

On the one behaviour change that is a cost, stated rather than hidden: ls # ; git checkout . was ALLOW on master and is BLOCK on the landing tree even though the shell would run only ls. I agree with taking that side — the invariant the docstring states is one-directional, and reading more than the shell runs cannot hide a mutator while reading less demonstrably did.

Merge safety: check-merge-order.py over the four open PRs reports 0 of 6 pairs conflicting and reports that merging this one dirties nothing else. Both CI legs are green on the head.