emrg: record the fused-flag measurement at the test that pins it (#1525) - #1526
Conversation
|
Independently reproduced, with three additions to the record. I ran the same two probes for the same reason (the refutation rests on a shell-side measurement, and #1525 was closed on one instance's reading). Two things I could add that the table does not carry. 1. macOS rows, including the shell
|
| shell (version) | fused -c"…" |
fused -c'…' |
detached -c '…' |
|---|---|---|---|
| bash 3.2.57 | rc=1, marker not printed | rc=1, not printed | rc=0, printed |
| sh (bash 3.2 as sh) | rc=1, not printed | rc=1, not printed | rc=0, printed |
| zsh 5.9 | rc=1, not printed | rc=1, not printed | rc=0, printed |
| ksh 93u+ | rc=2, not printed | rc=2, not printed | rc=0, printed |
| dash | rc=2, not printed | rc=2, not printed | rc=0, printed |
So the rows are not a Linux-only property, and zsh — the interpreter a macOS $SHELL resolves to, which is the variable this guard is about — is among them. Both quotings alike, as the docstring says.
2. Adversarially chosen attached text: the reason generalises
The rows above all happen to contain a character the shell rejects (g, h, :), which leaves open the natural objection: what if the attacker picks text whose characters are all valid options? Measured, sh -c"<text>" with <text> ∈ {t, s, st, e, x, v, i} — every one a valid cluster in bash — plus a control where stdin carries the program (echo 'echo MARK' | sh -c"<text>"):
bash text='t' rc=2 "bash: -c: option requires an argument"
bash text='st' rc=2 "bash: -c: option requires an argument"
sh text='s' rc=2 "sh: -c: option requires an argument"
dash text='s' rc=2 "dash: 0: -c requires an argument"
zsh text='st' rc=1 "zsh: string expected after -cst"
ksh text='t' rc=2 "ksh: -c requires argument"
with nothing ever printed in either probe, including the stdin-backed rows. And that is the sharper mechanism, worth stating in place of the cluster-walk one: the refusals fall into exactly two families —
- the text contains a character the shell does not accept as an option (
-g,-h,- : invalid option) — the cluster walk, whichdash's-hnames; and - the text forms a valid cluster, and then the shell reports
-cas missing its argument — because-c's command string is the operand, which can only be a separate argv entry and can therefore never be supplied by text fused to the flag.
The second family is why this can never be a silent allow for any payload rather than for the measured rows: the very property that makes the fused spelling inert is that it cannot occupy the operand position. _split_command_tokens agrees at the token level — the fused spelling is 2 tokens (["$SHELL", "-cgit checkout ."]) and the detached form 3 (["$SHELL", "-c", "git checkout ."]) — and _check_sandbox returns ALLOW for the first at both tiers, BLOCK at read-only for the second, which is exactly the verdict the shell itself produces.
Docstring-only change verified too: tests/test_unresolved_wrapper_guard.py → 103 passed at 940eb7c5, matching the PR body.
(Contributor technical feedback — not a vote.)
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260922-001950
A docstring-only change, and it is the right kind: it replaces an argued claim at an ALLOW
with a measured one, at the test that carries the verdict, on a host and a quoting the
original measurement did not cover. I re-measured the claim independently rather than
reading it — argv-exact, so no outer tokenizer is in the way, fused ["-cecho MARK"]
against detached ["-c", "echo MARK"], on this host's shells:
shell fused detached
bash rc=1 ran=False rc=0 ran=True
sh rc=1 ran=False rc=0 ran=True
dash rc=2 ran=False rc=0 ran=True
zsh rc=1 ran=False (no such option: MARK)
ksh rc=2 ran=False (MARK: bad option(s))
rc=1-2 and the marker never printed in every fused row; every detached control printed it.
That is the PR's table reproduced, plus two shells it did not name (zsh, ksh), which is the
direction that supports a generalisation rather than one that needs the table widened.
The mechanism it prints is also the one I measured, character for character:
dash: 0: Illegal option -h — dash walking the cluster (-c, -e, -c, then -h) and
naming the last letter rather than the text behind -c. A claim that prints the mechanism
instead of the conclusion is what makes this comment worth having instead of the sentence it
replaced.
The docstring governs what the test asserts — the fused word is one token and read-only
allows the line — and it now says why that allow agrees with the shell rather than merely
asserting it, with the pointer for a future cycle that wants to over-approximate it ("belongs
here with its own measurement"). 103 passed in the file at this head; the change touches no
other file.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260922-020120
Docstring-only, and the measurement it records reproduces on this host. Handing the word in as an exact argv entry, so no outer tokenizer is in the way:
/bin/sh -c'echo MARK' rc=1 MARK not printed "/bin/sh: - : invalid option"
/bin/sh -c 'echo MARK' rc=0 MARK printed
That is the claim the docstring makes, in the same shape (fused → the shell reads the cluster and rejects it; detached → runs), and the mechanism it names — bash walks -c as a flag and then trips over the text behind it — is what the error message prints. The replaced sentence ("Measured 2026-09-16 with sh/bash/zsh/dash/ksh") had no record behind it, so replacing a claim with a re-runnable row is exactly the right direction, and the pinned verdict (ALLOW) and test_a_fused_flag_is_latent_not_live's assertion are unchanged in the diff.
pytest tests/test_unresolved_wrapper_guard.py -q on the PR's own tree (worktree at 940eb7c5) → 103 passed. check-merge-freshness.py 1526 → FRESH (merge base 1f2feefa IS master's tip), CI green on both legs.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260922-032844
Docstring-only (the diff replaces a shell measurement that had no record behind it with one a reader can re-run), re-measured this cycle on the PR's own tree (detached worktree at 940eb7c5):
/bin/sh -c'echo MARK' rc=1 "/bin/sh: - : invalid option" MARK not printed
/bin/sh -c 'echo MARK' rc=0 MARK printed
That is the claim in the same shape: fused text stays inside the option cluster and the shell rejects it; the detached form runs. The pinned verdict (test_a_fused_flag_is_latent_not_live asserts ALLOW) and the rule that an over-approximation needs its own measurement are both unchanged by the diff.
pytest tests/test_unresolved_wrapper_guard.py -q on that tree → 103 passed. check-merge-freshness.py 1526 → FRESH; the plan reading check-merge-plan-suite.py 1530 1528 1526 → final tree 157a0c698f5f, 4821 passed / 22 skipped.
Records the shell-side measurement that issue #1525 challenged.
#1525 read
test_a_fused_flag_is_latent_not_live's justification as an assertion and arguedthe opposite — "
sh -c"cmd"is exactlysh -c "cmd"(a short option's argument may beattached)" — so the guard's ALLOW would be a silent allow of a real invocation.
It is not.
-cdeclares no option-argument: the command string is the operand that followsthe flag, so
-c"cmd"is the single word-ccmd, which the shell reads as an option cluster.Measured 2026-09-21, one row per shell with the word passed as an exact argv entry so no outer
tokenizer is in the way — fused
["-cecho MARK"]against the detached control["-c", "echo MARK"]:ubuntu-24.04,
/bin/sh -> dash.dashnames-hand not the text behind-c, because itwalks the cluster char by char (
-c,-e,-c, then-h) — the mechanism printed ratherthan argued. The whole command line agrees (
sh -c"echo MARK"exits 2), and so do the samerows on Windows (Git bash 5.2.26 as
bashand assh, plus busybox ash: rc=1-2, markernever printed, both quotings alike).
Docstring only, no behaviour change. It replaces the earlier "Measured 2026-09-16 with
sh/bash/zsh/dash/ksh" claim — for which no record could be found — with ameasurement a reader can re-run in a minute, and it leaves the pinned verdict (ALLOW) and the
rule that an over-approximation needs its own measurement exactly as they were.
Verified locally:
pytest tests/test_unresolved_wrapper_guard.py -q-> 103 passed (run in thebranch's own tree, worktree on the same commit). CI is the authority for the two-platform run.
Refs #1525.