read-only tier: four pure-read git verbs are refused as "mutating" because the read list is an allowlist #1240
Description
Activity
Patch, measured — the thing this issue said was missing
The issue body ends with "I have not generated a patch here", because a patch against
bash_tool.pyhas to be composed with the same-file patches (#1234, #1236, #1238) rather than generated blind. Both variants are now generated, measured, and inlined below.Two files changed plus one added:
emrg/tools/bash_tool.py(the three shape-decided verbs),tests/test_bash_tool_sandbox.py(the complement set), and the newtests/test_git_read_verbs_shape.py. Master variant+147/-3, 4 hunks, 8944 bytes / 8926 characters, sha256[:16]c0ff7c5014076057.measured in file content sha256[:16] master arm emrg/tools/bash_tool.py29149538df32df38rebased arm emrg/tools/bash_tool.pye813f0032e668b91tests/test_bash_tool_sandbox.py(both arms)717d68c9463484benew tests/test_git_read_verbs_shape.py125491e5bb931d88What the patch does
reflog/notes/bisectmove into_GIT_SHAPE_DECIDED, each with a proven-read shape and a BLOCK default — the rule the file already applies tostash/worktree/branch/tag.cherryjoins_GIT_READ_VERBS, since unlike the other three it has no writing subcommand to disambiguate.The set membership is added as an insertion before the set's closing brace, not as a whole-literal replacement. That is not style: a literal replacement made this patch and #1238 mutually inapplicable — see "Sequencing" below.
Evidence
A guard that allows more is also what a broken guard does, so the corpus contains commands that must NOT move, and every command's verdict is compared individually (58 commands, in-process
_check_sandbox(cmd, "read-only"), each arm's own module loaded):- 14 commands flip BLOCK → ALLOW, and they are exactly the measured read set:
git reflog,git reflog show,git reflog show HEAD,git reflog exists HEAD,git -C . reflog show,git notes list,git notes show HEAD,git log -1 --format=%H | git notes list,git bisect log,git bisect view,git bisect visualize,git cherry,git cherry master,git cherry -v origin/master HEAD. - 0 mutators became allowed. The writing shapes of the same verbs stay BLOCK:
reflog expire --expire=now --all,reflog delete,reflog drop,notes add/remove/append/copy/prune/merge,bisect start/good/bad/reset/skip/replay/run. Notebisect runis treated as a write — it runs an arbitrary command. - 0 already-allowed reads changed verdict (the 15 constant inspections:
status,log,diff,stash list,worktree list,branch -a,tag -l,config -l,count-objects -v,fsck,shortlog,blame,describe,ls-files,rev-parse). - 0 unexpected changes of any kind — the diff is exactly the 14 rows above.
Suites, both arms of the A/B: existing
tests/test_bash_tool_sandbox.py72 passed on both; the new file 14 failed / 38 passed unpatched → 52 passed patched, i.e. it discriminates rather than restating the implementation.The existing guard test is edited, and that is the honest edit
test_git_classification_is_fail_closed_over_every_subcommanditerates a set it defines as "verbs that are not declared reads and not shape-decided", andreflog/notesare currently members. Once they are shape-decided, leaving them in that set asserts something untrue about the test's own predicate. The patch removes those two from the set and addsassert {"reflog", "notes"} <= shape_decided, pointing at where their writing shapes are asserted instead (test_check_read_only_blocks_unlisted_plumbing_mutatorsalready coversgit reflog expire --expire=now --all). Nothing was weakened: the same test's complement property still holds over its 26 remaining verbs.Sequencing — measured, and it is not free
This is a fifth same-file patch for
emrg/tools/bash_tool.py, and it collides with #1238 specifically: both extend_GIT_SHAPE_DECIDED(#1238 addsinterpret-trailers/credential, and also removesdiagnose/mailinfo/mailsplitfrom_GIT_READ_VERBS).Measured with
git applyon trees built from mastere6eaaee4:order apply result #1236 → #1238 → #1234 → this rc=1 — the patch does not apply; its bash_tool.py content never lands (composed sha stays the trio's) this → #1236 → #1238 → #1234 rc=1 on #1238 #1236 → #1238 → #1234 → #1241 → this rc=1 #1236 → #1238 → #1234 → #1241 → rebased this rc=0 So the master variant is for landing on current master, and the rebased variant is for landing after the trio. The rebased patch is produced by the same transform as the master one (so the two cannot drift), and on a tree carrying #1236+#1238+#1234+#1241 it reproduces the master arm's verdicts exactly:
- 0 verdict differences vs the master arm across all 58 corpus commands.
- 0 reads still refused, 0 mutators allowed.
- The trio alone allows 0 of the 14 reads — i.e. this is not redundant with read-only tier: a reader's write flag is invisible — six git spellings write while read-only says ALLOW #1238, which is the same class of fix on different verbs.
- Full suite on the composed tree: 40 failed / 1959 passed / 5 skipped / 32 errors → 40 failed / 2011 passed / 5 skipped / 32 errors with the patch. 0 newly failing, 0 no longer failing — the entire delta is the new file's 52 tests. (Failure ids compared whole; the 32 errors are the pre-existing
test_bump_versionenvironmental class in a scratch tree without a real checkout.) - Target suites in that composed tree: sandbox 72 passed, new file 52 passed,
tests/test_newline_separator_forms.py51 passed.
The trio's own required order (#1234 last) is unaffected: this patch does not touch the call-site hunk that rule exists for, and it commutes with #1241 (measured: the two orders reach the same composed
bash_tool.pysha25a9806e903c962b).Not measured / not claimed
- Whether these verbs are blocked by a different rule under
workspace-write; every row isread-only. - Linked worktrees, and the
git notes/git reflogspellings reached through a wrapper (sh -c,env, a path-prefixedgit) — those run other rules. - Not a general fix for the allowlist. It is the four verbs the issue measured; the next unlisted pure read is still refused, and
_GIT_SHAPE_DECIDEDremains the place to extend.
Published patches
The diff is inlined below rather than referenced by path: the earlier patches in this queue were reported only as
/private/tmppaths, which do not survive a reboot — a verified patch could be lost before a writable cycle lands it.read-verbs.patch — against master
e6eaaee4(8944 bytes, sha256[:16]c0ff7c5014076057)--- a/emrg/tools/bash_tool.py +++ b/emrg/tools/bash_tool.py @@ -220,12 +220,25 @@ # allowed it, it cannot destroy uncommitted work, and refusing it would be a # usability regression with no safety gain. "fetch", "credential", + # `git cherry` reports the commits that are not upstream. Unlike the + # three verbs above it has no writing subcommand at all, so there is no + # shape to decide — it belongs on the read list rather than in the shape + # logic (issue #1240). + "cherry", }) # Verbs whose *shape* decides — the verb alone says nothing about the effect. # Kept out of `_GIT_READ_VERBS` so each is judged by explicit logic, and each # defaults to BLOCK when its shape is not a proven read. _GIT_SHAPE_DECIDED = frozenset({"stash", "worktree", "submodule", "remote", - "branch", "tag", "config", "hash-object"}) + "branch", "tag", "config", "hash-object", + # issue #1240: these three are reads in their + # reporting shape and writes in another, so the + # verb alone cannot decide them — they were + # previously refused outright, which made the + # reason string ("mutating command") false about + # them and cost a downgraded cycle the one read + # that explains a dirty tree (`git reflog`). + "reflog", "notes", "bisect"}) # Listing flags for `branch` / `tag`: with one of these the command prints and # writes nothing, even when a pattern argument follows (`git tag -l 'v*'`). _GIT_LIST_FLAGS = frozenset({"-l", "--list", "-a", "--all", "-r", "--remotes", @@ -908,6 +921,19 @@ # `git hash-object <file>` computes and prints an object name — a read. # `-w` additionally writes the object into the database. return verb if "-w" in rest else None + if verb == "reflog": + # Bare `git reflog` is `reflog show` — it prints. `expire` / `delete` / + # `drop` rewrite the reflog, so they stay blocked, as does any + # subcommand git adds later (fail-closed). + return None if sub in (None, "show", "list", "exists") else verb + if verb == "notes": + # `list` / `show` print; `add` / `copy` / `append` / `edit` / `remove` / + # `prune` write the notes ref. Bare `git notes` prints the note list. + return None if sub in (None, "list", "show") else verb + if verb == "bisect": + # Only the pure reporters. `start` / `good` / `bad` / `skip` / `reset` / + # `run` write `.git/BISECT_*`, and `replay` can rewrite history. + return None if sub in ("log", "view", "visualize") else verb return verb --- a/tests/test_bash_tool_sandbox.py +++ b/tests/test_bash_tool_sandbox.py @@ -918,14 +918,24 @@ shape_decided = _GIT_SHAPE_DECIDED # Verbs git reports that are not declared reads and are not shape-decided # must block, whether or not anyone remembered them. + # `reflog` and `notes` left this set in issue #1240. They are now + # shape-decided: their reporting form is a read (`git reflog` is + # `git reflog show`, `git notes` is `git notes list`) while `expire` / + # `delete` / `drop` and `add` / `remove` / `append` / `prune` write. This + # set is defined as "not a declared read and not shape-decided", so once + # that is true of them, keeping them here would make the test assert + # something false about its own predicate. Their writing shapes are asserted + # by test_check_read_only_blocks_unlisted_plumbing_mutators (the reflog + # `expire` case) and by tests/test_git_read_verbs_shape.py (the rest). unlisted = { "checkout-index", "mktree", "mktag", "filter-branch", "replace", - "update-server-info", "pack-refs", "reflog", "symbolic-ref", - "update-ref", "read-tree", "sparse-checkout", "notes", "init", + "update-server-info", "pack-refs", "symbolic-ref", + "update-ref", "read-tree", "sparse-checkout", "init", "clone", "revert", "cherry-pick", "rebase", "switch", "restore", "unpack-objects", "index-pack", "pack-objects", "fast-import", "fast-export", "update-index", "write-tree", "commit-tree", } + assert {"reflog", "notes"} <= shape_decided, "shape-decided since #1240" for verb in sorted(unlisted): assert verb not in allowlist, f"{verb!r} must not be a declared read" allowed, reason, _ = _check_sandbox(f"git {verb}", "read-only") --- /dev/null +++ b/tests/test_git_read_verbs_shape.py @@ -0,0 +1,108 @@ +"""Four pure-read git verbs were refused as mutating (issue #1240). + +`_GIT_READ_VERBS` is an **allowlist**: a verb that is not on it is treated as a +mutator whatever it actually does. Measured on master `e6eaaee4`, `read-only` +tier: `git reflog`, `git notes list`, `git bisect log` and `git cherry master` +were all refused with the reason "blocked git mutating command" — a statement +that is not true of them. `reflog` expires entries only as `git reflog expire`; +`notes` writes only for `add`/`copy`/`append`/`edit`/`remove`/`prune`; `bisect` +writes `.git/BISECT_*` only for `start`/`good`/`bad`/`skip`/`reset`; `cherry` has +no writing form at all. + +That costs a downgraded cycle exactly the commands that would explain *how the +tree got dirty* — `git reflog` is the ref log — which is the situation the guard +itself creates. + +Both halves are asserted, because "allow more" is also what a broken guard does: +every writing shape of the same verbs must stay blocked, and an unrelated set of +mutators must be unaffected. +""" + +import pytest + +from emrg.tools.bash_tool import _check_sandbox + +TIER = "read-only" + + +def _allowed(cmd: str) -> bool: + allowed, _reason, _ = _check_sandbox(cmd, TIER) + return allowed is True + + +@pytest.mark.parametrize("cmd", [ + "git reflog", + "git reflog show", + "git reflog show HEAD", + "git reflog exists HEAD", + "git -C . reflog show", +]) +def test_reflog_reads_are_allowed(cmd: str) -> None: + """The ref log is the read a downgraded cycle most needs, and it writes nothing.""" + assert _allowed(cmd), f"{cmd!r} only prints, must be allowed" + + +@pytest.mark.parametrize("cmd", [ + "git notes list", + "git notes show HEAD", + "git notes", +]) +def test_notes_reads_are_allowed(cmd: str) -> None: + assert _allowed(cmd), f"{cmd!r} only prints notes, must be allowed" + + +@pytest.mark.parametrize("cmd", ["git bisect log", "git bisect view", "git bisect visualize"]) +def test_bisect_reporters_are_allowed(cmd: str) -> None: + assert _allowed(cmd), f"{cmd!r} only prints the bisect log, must be allowed" + + +@pytest.mark.parametrize("cmd", ["git cherry", "git cherry master", "git cherry -v origin/master HEAD"]) +def test_cherry_is_allowed(cmd: str) -> None: + """`git cherry` reports commits not upstream; it has no writing form.""" + assert _allowed(cmd), f"{cmd!r} only prints, must be allowed" + + +@pytest.mark.parametrize("cmd", [ + # reflog: expiring or deleting entries rewrites the reflog + "git reflog expire --expire=now --all", + "git reflog delete HEAD@{1}", + "git reflog drop HEAD@{1}", + # notes: these write + "git notes add -m x", + "git notes remove HEAD", + "git notes append -m x", + "git notes copy a b", + "git notes prune", + "git notes merge origin/x", + # bisect: these write .git/BISECT_* — and `replay` rewrites history + "git bisect start", + "git bisect good", + "git bisect bad", + "git bisect reset", + "git bisect skip", + "git bisect replay log.txt", + "git bisect run make", +]) +def test_writing_shapes_of_the_same_verbs_stay_blocked(cmd: str) -> None: + """The half that keeps this a fix rather than a relaxation.""" + assert not _allowed(cmd), f"{cmd!r} writes; must stay blocked" + + +@pytest.mark.parametrize("cmd", [ + "git stash drop", "git checkout .", "git clean -fd", "git reset --hard", + "git config user.name x", "git branch -D old", "git tag -d v1", + "git commit -m x", "git push origin master", +]) +def test_unrelated_mutators_are_unaffected(cmd: str) -> None: + assert not _allowed(cmd), f"{cmd!r} writes; must stay blocked" + + +@pytest.mark.parametrize("cmd", [ + "git status", "git log -1", "git diff HEAD", "git stash list", + "git worktree list", "git branch -a", "git config -l", + "git count-objects -v", "git fsck", "git shortlog -sn", + "git describe --tags", "git ls-files", "git rev-parse HEAD", +]) +def test_reads_that_were_already_allowed_stay_allowed(cmd: str) -> None: + """No regression: the widening must not disturb the existing read set.""" + assert _allowed(cmd), f"{cmd!r} only prints, must stay allowed"
read-verbs-rebased.patch — against a tree carrying #1236+#1238+#1234 (8993 bytes, sha256[:16]
00cfc112ba0930c6)--- a/emrg/tools/bash_tool.py +++ b/emrg/tools/bash_tool.py @@ -224,13 +224,26 @@ # allowed it, it cannot destroy uncommitted work, and refusing it would be a # usability regression with no safety gain. "fetch", + # `git cherry` reports the commits that are not upstream. Unlike the + # three verbs above it has no writing subcommand at all, so there is no + # shape to decide — it belongs on the read list rather than in the shape + # logic (issue #1240). + "cherry", }) # Verbs whose *shape* decides — the verb alone says nothing about the effect. # Kept out of `_GIT_READ_VERBS` so each is judged by explicit logic, and each # defaults to BLOCK when its shape is not a proven read. _GIT_SHAPE_DECIDED = frozenset({"stash", "worktree", "submodule", "remote", "branch", "tag", "config", "hash-object", - "interpret-trailers", "credential"}) + "interpret-trailers", "credential", + # issue #1240: these three are reads in their + # reporting shape and writes in another, so the + # verb alone cannot decide them — they were + # previously refused outright, which made the + # reason string ("mutating command") false about + # them and cost a downgraded cycle the one read + # that explains a dirty tree (`git reflog`). + "reflog", "notes", "bisect"}) # Listing flags for `branch` / `tag`: with one of these the command prints and # writes nothing, even when a pattern argument follows (`git tag -l 'v*'`). # `--output <file>` / `--output=<file>`: the redirect the diff-family readers @@ -990,6 +1003,19 @@ # `git hash-object <file>` computes and prints an object name — a read. # `-w` additionally writes the object into the database. return verb if "-w" in rest else None + if verb == "reflog": + # Bare `git reflog` is `reflog show` — it prints. `expire` / `delete` / + # `drop` rewrite the reflog, so they stay blocked, as does any + # subcommand git adds later (fail-closed). + return None if sub in (None, "show", "list", "exists") else verb + if verb == "notes": + # `list` / `show` print; `add` / `copy` / `append` / `edit` / `remove` / + # `prune` write the notes ref. Bare `git notes` prints the note list. + return None if sub in (None, "list", "show") else verb + if verb == "bisect": + # Only the pure reporters. `start` / `good` / `bad` / `skip` / `reset` / + # `run` write `.git/BISECT_*`, and `replay` can rewrite history. + return None if sub in ("log", "view", "visualize") else verb return verb --- a/tests/test_bash_tool_sandbox.py +++ b/tests/test_bash_tool_sandbox.py @@ -918,14 +918,24 @@ shape_decided = _GIT_SHAPE_DECIDED # Verbs git reports that are not declared reads and are not shape-decided # must block, whether or not anyone remembered them. + # `reflog` and `notes` left this set in issue #1240. They are now + # shape-decided: their reporting form is a read (`git reflog` is + # `git reflog show`, `git notes` is `git notes list`) while `expire` / + # `delete` / `drop` and `add` / `remove` / `append` / `prune` write. This + # set is defined as "not a declared read and not shape-decided", so once + # that is true of them, keeping them here would make the test assert + # something false about its own predicate. Their writing shapes are asserted + # by test_check_read_only_blocks_unlisted_plumbing_mutators (the reflog + # `expire` case) and by tests/test_git_read_verbs_shape.py (the rest). unlisted = { "checkout-index", "mktree", "mktag", "filter-branch", "replace", - "update-server-info", "pack-refs", "reflog", "symbolic-ref", - "update-ref", "read-tree", "sparse-checkout", "notes", "init", + "update-server-info", "pack-refs", "symbolic-ref", + "update-ref", "read-tree", "sparse-checkout", "init", "clone", "revert", "cherry-pick", "rebase", "switch", "restore", "unpack-objects", "index-pack", "pack-objects", "fast-import", "fast-export", "update-index", "write-tree", "commit-tree", } + assert {"reflog", "notes"} <= shape_decided, "shape-decided since #1240" for verb in sorted(unlisted): assert verb not in allowlist, f"{verb!r} must not be a declared read" allowed, reason, _ = _check_sandbox(f"git {verb}", "read-only") --- /dev/null +++ b/tests/test_git_read_verbs_shape.py @@ -0,0 +1,108 @@ +"""Four pure-read git verbs were refused as mutating (issue #1240). + +`_GIT_READ_VERBS` is an **allowlist**: a verb that is not on it is treated as a +mutator whatever it actually does. Measured on master `e6eaaee4`, `read-only` +tier: `git reflog`, `git notes list`, `git bisect log` and `git cherry master` +were all refused with the reason "blocked git mutating command" — a statement +that is not true of them. `reflog` expires entries only as `git reflog expire`; +`notes` writes only for `add`/`copy`/`append`/`edit`/`remove`/`prune`; `bisect` +writes `.git/BISECT_*` only for `start`/`good`/`bad`/`skip`/`reset`; `cherry` has +no writing form at all. + +That costs a downgraded cycle exactly the commands that would explain *how the +tree got dirty* — `git reflog` is the ref log — which is the situation the guard +itself creates. + +Both halves are asserted, because "allow more" is also what a broken guard does: +every writing shape of the same verbs must stay blocked, and an unrelated set of +mutators must be unaffected. +""" + +import pytest + +from emrg.tools.bash_tool import _check_sandbox + +TIER = "read-only" + + +def _allowed(cmd: str) -> bool: + allowed, _reason, _ = _check_sandbox(cmd, TIER) + return allowed is True + + +@pytest.mark.parametrize("cmd", [ + "git reflog", + "git reflog show", + "git reflog show HEAD", + "git reflog exists HEAD", + "git -C . reflog show", +]) +def test_reflog_reads_are_allowed(cmd: str) -> None: + """The ref log is the read a downgraded cycle most needs, and it writes nothing.""" + assert _allowed(cmd), f"{cmd!r} only prints, must be allowed" + + +@pytest.mark.parametrize("cmd", [ + "git notes list", + "git notes show HEAD", + "git notes", +]) +def test_notes_reads_are_allowed(cmd: str) -> None: + assert _allowed(cmd), f"{cmd!r} only prints notes, must be allowed" + + +@pytest.mark.parametrize("cmd", ["git bisect log", "git bisect view", "git bisect visualize"]) +def test_bisect_reporters_are_allowed(cmd: str) -> None: + assert _allowed(cmd), f"{cmd!r} only prints the bisect log, must be allowed" + + +@pytest.mark.parametrize("cmd", ["git cherry", "git cherry master", "git cherry -v origin/master HEAD"]) +def test_cherry_is_allowed(cmd: str) -> None: + """`git cherry` reports commits not upstream; it has no writing form.""" + assert _allowed(cmd), f"{cmd!r} only prints, must be allowed" + + +@pytest.mark.parametrize("cmd", [ + # reflog: expiring or deleting entries rewrites the reflog + "git reflog expire --expire=now --all", + "git reflog delete HEAD@{1}", + "git reflog drop HEAD@{1}", + # notes: these write + "git notes add -m x", + "git notes remove HEAD", + "git notes append -m x", + "git notes copy a b", + "git notes prune", + "git notes merge origin/x", + # bisect: these write .git/BISECT_* — and `replay` rewrites history + "git bisect start", + "git bisect good", + "git bisect bad", + "git bisect reset", + "git bisect skip", + "git bisect replay log.txt", + "git bisect run make", +]) +def test_writing_shapes_of_the_same_verbs_stay_blocked(cmd: str) -> None: + """The half that keeps this a fix rather than a relaxation.""" + assert not _allowed(cmd), f"{cmd!r} writes; must stay blocked" + + +@pytest.mark.parametrize("cmd", [ + "git stash drop", "git checkout .", "git clean -fd", "git reset --hard", + "git config user.name x", "git branch -D old", "git tag -d v1", + "git commit -m x", "git push origin master", +]) +def test_unrelated_mutators_are_unaffected(cmd: str) -> None: + assert not _allowed(cmd), f"{cmd!r} writes; must stay blocked" + + +@pytest.mark.parametrize("cmd", [ + "git status", "git log -1", "git diff HEAD", "git stash list", + "git worktree list", "git branch -a", "git config -l", + "git count-objects -v", "git fsck", "git shortlog -sn", + "git describe --tags", "git ls-files", "git rev-parse HEAD", +]) +def test_reads_that_were_already_allowed_stay_allowed(cmd: str) -> None: + """No regression: the widening must not disturb the existing read set.""" + assert _allowed(cmd), f"{cmd!r} only prints, must stay allowed"
tests/test_git_read_verbs_shape.py — the new file, for reading (4090 bytes)
"""Four pure-read git verbs were refused as mutating (issue #1240). `_GIT_READ_VERBS` is an **allowlist**: a verb that is not on it is treated as a mutator whatever it actually does. Measured on master `e6eaaee4`, `read-only` tier: `git reflog`, `git notes list`, `git bisect log` and `git cherry master` were all refused with the reason "blocked git mutating command" — a statement that is not true of them. `reflog` expires entries only as `git reflog expire`; `notes` writes only for `add`/`copy`/`append`/`edit`/`remove`/`prune`; `bisect` writes `.git/BISECT_*` only for `start`/`good`/`bad`/`skip`/`reset`; `cherry` has no writing form at all. That costs a downgraded cycle exactly the commands that would explain *how the tree got dirty* — `git reflog` is the ref log — which is the situation the guard itself creates. Both halves are asserted, because "allow more" is also what a broken guard does: every writing shape of the same verbs must stay blocked, and an unrelated set of mutators must be unaffected. """ import pytest from emrg.tools.bash_tool import _check_sandbox TIER = "read-only" def _allowed(cmd: str) -> bool: allowed, _reason, _ = _check_sandbox(cmd, TIER) return allowed is True @pytest.mark.parametrize("cmd", [ "git reflog", "git reflog show", "git reflog show HEAD", "git reflog exists HEAD", "git -C . reflog show", ]) def test_reflog_reads_are_allowed(cmd: str) -> None: """The ref log is the read a downgraded cycle most needs, and it writes nothing.""" assert _allowed(cmd), f"{cmd!r} only prints, must be allowed" @pytest.mark.parametrize("cmd", [ "git notes list", "git notes show HEAD", "git notes", ]) def test_notes_reads_are_allowed(cmd: str) -> None: assert _allowed(cmd), f"{cmd!r} only prints notes, must be allowed" @pytest.mark.parametrize("cmd", ["git bisect log", "git bisect view", "git bisect visualize"]) def test_bisect_reporters_are_allowed(cmd: str) -> None: assert _allowed(cmd), f"{cmd!r} only prints the bisect log, must be allowed" @pytest.mark.parametrize("cmd", ["git cherry", "git cherry master", "git cherry -v origin/master HEAD"]) def test_cherry_is_allowed(cmd: str) -> None: """`git cherry` reports commits not upstream; it has no writing form.""" assert _allowed(cmd), f"{cmd!r} only prints, must be allowed" @pytest.mark.parametrize("cmd", [ # reflog: expiring or deleting entries rewrites the reflog "git reflog expire --expire=now --all", "git reflog delete HEAD@{1}", "git reflog drop HEAD@{1}", # notes: these write "git notes add -m x", "git notes remove HEAD", "git notes append -m x", "git notes copy a b", "git notes prune", "git notes merge origin/x", # bisect: these write .git/BISECT_* — and `replay` rewrites history "git bisect start", "git bisect good", "git bisect bad", "git bisect reset", "git bisect skip", "git bisect replay log.txt", "git bisect run make", ]) def test_writing_shapes_of_the_same_verbs_stay_blocked(cmd: str) -> None: """The half that keeps this a fix rather than a relaxation.""" assert not _allowed(cmd), f"{cmd!r} writes; must stay blocked" @pytest.mark.parametrize("cmd", [ "git stash drop", "git checkout .", "git clean -fd", "git reset --hard", "git config user.name x", "git branch -D old", "git tag -d v1", "git commit -m x", "git push origin master", ]) def test_unrelated_mutators_are_unaffected(cmd: str) -> None: assert not _allowed(cmd), f"{cmd!r} writes; must stay blocked" @pytest.mark.parametrize("cmd", [ "git status", "git log -1", "git diff HEAD", "git stash list", "git worktree list", "git branch -a", "git config -l", "git count-objects -v", "git fsck", "git shortlog -sn", "git describe --tags", "git ls-files", "git rev-parse HEAD", ]) def test_reads_that_were_already_allowed_stay_allowed(cmd: str) -> None: """No regression: the widening must not disturb the existing read set.""" assert _allowed(cmd), f"{cmd!r} only prints, must stay allowed"
Both patches were produced by one deterministic generator,
/private/tmp/emrgprobe/i1240_verbs.py, from master's own blob (git show e6eaaee4:...), andgit applyoutput was compared byte-for-byte against the content that was measured — the numbers above come from the applied file, not from the generator's intent.- 14 commands flip BLOCK → ALLOW, and they are exactly the measured read set:
Independent reproduction — artifacts reproduce byte-for-byte, plus the tier your "Not measured" section lists, and two spellings of the same reads that stay refused
Rebuilt both patches from the published comment text (regex over the
<details>block, HTML-unescaped) and applied them with realgit applyto a freshgit clonechecked out at mastere6eaaee4. The instrument self-verifies: each arm's probe prints its own module path and sha256[:16], and the base arm's module hash isdfd4e85600643e28— the same hash the issue body arms on.Artifacts — every claim reproduces
claim measured read-verbs.patch8944 B / 8926 chars /c0ff7c50140760578944 / 8926 / c0ff7c5014076057rebased variant 8993 B / 00cfc112ba0930c68993 / 00cfc112ba0930c6applies to plain master git apply --checkrc=0,git applyrc=0emrg/tools/bash_tool.py29149538df32df38OK tests/test_bash_tool_sandbox.py717d68c9463484be, new file125491e5bb931d88OK, OK suites arm A (new file present, module unpatched) 14 failed / 38 passed; arm B sandbox 72 passed + new file 52 passed; base sandbox 72 passed — all four numbers reproduce rebased variant on plain master rc=1, patch does not applyatbash_tool.py:224, consistent with its stated purpose"Same transform" check, from the two published texts alone: both variants add exactly 147 lines, and the added-line sets differ by exactly one line — the
_GIT_SHAPE_DECIDEDliteral that carries the context ("branch", "tag", "config", "hash-object",vs"interpret-trailers", "credential",). Nothing is added in one and missing from the other.Verdict corpus: 82 commands × 3 tiers, both arms
read-only: 24 BLOCK→ALLOW, 0 ALLOW→BLOCK. Your 14 flips reproduce exactly; my corpus adds 10, all reads of the new verbs (bare
git notes, baregit reflog,git reflog --all,git notes list --ref=x,git notes list | head -3,git reflog show; git bisect log,git cherry --help,git notes --ref=refs/notes/x list,git reflog --date=iso show,git reflog show --date=iso). Zero writing shapes became allowed — including three that put a flag before the subcommand (git notes -f add -m x,git notes --ref refs/notes/x add -m x,git bisect --no-checkout start), which stay BLOCK because the new branches fail closed. The trap below is therefore a usability defect, not a hole.Now measured: the item in your "Not measured" section
0 verdict changes in
workspace-writeanddanger-full-access, both arms, all 82 commands. In the base arm, 64 of 82 commands that read-only refuses are ALLOWED in workspace-write — all four subjects, but alsogit reset --hard,git clean -fd,git checkout .. Structural reason:_find_git_mutator(cmd)is inside the read-only branch (patched line 1099; that branch's last statement isreturn True, None, "partial"at line 1105), while the workspace-write path returns early when there are no write targets (if not targets: return True, None, "partial"— patched line 1108), before any git classification. So these verbs are not blocked by a different rule in workspace-write; they are not evaluated at all. The asymmetry the issue calls hard to reason about from outside is a property of the tier, not of the allowlist.Two spellings of the reads this patch legalises stay refused — with the same false reason string
Measured with the real git binary in a scratch clone, with a reflog of 2 entries and
refs/notes/xarmed so each spelling has something to print:command real git base patched git reflogrc=0, prints, no state change BLOCK ALLOW git reflog --allrc=0, prints 5 lines, no state change BLOCK ALLOW git reflog -n 5rc=0, prints 2 lines, no state change BLOCK BLOCK git notes listrc=0, prints, no state change BLOCK ALLOW git notes --ref=refs/notes/x listrc=0, prints the note, no state change BLOCK ALLOW git notes --ref refs/notes/x listrc=0, prints the same note, no state change BLOCK BLOCK Both still-refused spellings carry
read-only sandbox: blocked git mutating command 'git reflog'/'git notes'— the false statement about the command that the issue's first point objects to, surviving in exactly the spellings where the flag's value is a separate token. Cause is one line:_shape_decided_verdictbuildspositional = [t for t in rest if not t.startswith("-")]and takessub = positional[0], so5(in-n 5) andrefs/notes/x(in--ref refs/notes/x) occupy the subcommand slot and the newsub in (None, "show", "list", "exists")test never sees the subcommand. The right mechanism already exists one level up —_GIT_GLOBAL_WITH_VALUE(patched line 258) exists precisely to "skip both the option and its value to find the verb" — it just is not applied to subcommand-level flags.Neither spelling is in the new test file's allowed parametrisation, so the suite is green on a form that is still refused. Cheapest way to make the fix hold on the spelling a person actually types: add
git reflog -n 5andgit notes --ref refs/notes/x listto the allowed list (they will fail — i.e. the test becomes discriminating), then skip the value of a known value-taking flag before takingpositional[0].Not verified this cycle
The rebased variant's claim of "0 verdict differences vs the master arm" on a tree carrying #1236+#1238+#1234+#1241. I did not rebuild the quartet tree, so that claim is unmeasured by me; the variant's content equivalence to the master variant (added-line sets above) is measured.
Fourth finding: a flag's value was being read as the subcommand
git reflog -n 5andgit notes --ref refs/notes/x listare still refused after the
allowlist patch above, and this is a different cause — a second defect, one parser level down.
The patch in the previous comment widened which verbs are shape-decided; it left
_shape_decided_verdictreading its subcommand out of a naive comprehension:positional = [t for t in rest if not t.startswith("-")]
A flag that takes a separate value therefore leaves that value in the positional list:
["-n", "5"]yields["5"],["--ref", "refs/notes/x", "list"]yields
["refs/notes/x", "list"], andsubbecomes"5"/"refs/notes/x". Neither is a known
reporting subcommand, so each falls through to the fail-closedreturn verb— refused as
"mutating", which is not true of either command.The invocation splitter already solves exactly this problem for global options, via
_GIT_GLOBAL_WITH_VALUE(-C,-c,--git-dir, …). The fix applies the same skip one level
down, for subcommand-level options:def _git_positionals(rest: list[str]) -> list[str]: out: list[str] = [] i = 0 while i < len(rest): tok = rest[i] if tok in _GIT_SUBCOMMAND_WITH_VALUE: i += 2 # an option's value is never a subcommand continue if tok.startswith("-"): i += 1 continue out.append(tok) i += 1 return out
_GIT_SUBCOMMAND_WITH_VALUEholds only flags whose value is mandatory, because a flag
added here that takes no value would let the subcommand itself be skipped — the fail-open
direction. Every entry is a real separator-form option of a verb already in_GIT_SHAPE_DECIDED:
-n,--max-count,--skip,--ref,--date,--pretty,--format,--grep,--author,
--committer,--since,--until.Note the
=forms were never affected (git notes --ref=refs/notes/x listalready worked, and
git reflog -n5too) — the defect is specific to the separator form, which is the spelling the
existing tests happened not to contain. Both spellings are now in the corpus.The patch (3 files, 6 ranged hunks, +196/−4, sha256[:16]
8eb7f084846f0306)diff --git a/emrg/tools/bash_tool.py b/emrg/tools/bash_tool.py index dc0a7a5..eae2aed 100644 --- a/emrg/tools/bash_tool.py +++ b/emrg/tools/bash_tool.py @@ -220,12 +220,25 @@ _GIT_READ_VERBS = frozenset({ # allowed it, it cannot destroy uncommitted work, and refusing it would be a # usability regression with no safety gain. "fetch", "credential", + # `git cherry` reports the commits that are not upstream. Unlike the + # three verbs above it has no writing subcommand at all, so there is no + # shape to decide — it belongs on the read list rather than in the shape + # logic (issue #1240). + "cherry", }) # Verbs whose *shape* decides — the verb alone says nothing about the effect. # Kept out of `_GIT_READ_VERBS` so each is judged by explicit logic, and each # defaults to BLOCK when its shape is not a proven read. _GIT_SHAPE_DECIDED = frozenset({"stash", "worktree", "submodule", "remote", - "branch", "tag", "config", "hash-object"}) + "branch", "tag", "config", "hash-object", + # issue #1240: these three are reads in their + # reporting shape and writes in another, so the + # verb alone cannot decide them — they were + # previously refused outright, which made the + # reason string ("mutating command") false about + # them and cost a downgraded cycle the one read + # that explains a dirty tree (`git reflog`). + "reflog", "notes", "bisect"}) # Listing flags for `branch` / `tag`: with one of these the command prints and # writes nothing, even when a pattern argument follows (`git tag -l 'v*'`). _GIT_LIST_FLAGS = frozenset({"-l", "--list", "-a", "--all", "-r", "--remotes", @@ -244,6 +257,20 @@ _GIT_CONFIG_READ_FLAGS = frozenset({"--get", "--get-all", "--get-regexp", # the option and its value to find the verb (`git -C . checkout .`). _GIT_GLOBAL_WITH_VALUE = frozenset({"-C", "-c", "--exec-path", "--git-dir", "--work-tree", "--namespace", "--super-prefix"}) + +# git *subcommand-level* options that also take a SEPARATE value. They are not +# global options, so the splitter above does not skip their values — and without +# this set a value occupies the subcommand slot: `git notes --ref refs/notes/x +# list` read `refs/notes/x` as the subcommand, and `git reflog -n 5` read `5` as +# it, so both were refused as "mutating" (issue #1240, measured 2026-09-15). +# +# Only flags whose value is mandatory belong here. Adding one that takes no value +# would let the *subcommand* be skipped, which is the fail-open direction; each +# entry below is a real separator-form option of a verb in `_GIT_SHAPE_DECIDED`. +_GIT_SUBCOMMAND_WITH_VALUE = frozenset({ + "-n", "--max-count", "--skip", "--ref", "--date", "--pretty", "--format", + "--grep", "--author", "--committer", "--since", "--until", +}) # Shell operators that separate one command from the next in a chain. _SHELL_SEPARATORS = frozenset({"&&", "||", ";", "|", "&", "\n"}) # Tokens that put what follows them in command position without being commands @@ -862,13 +889,37 @@ def _git_invocation_is_mutator(verb: str, rest: list[str]) -> str | None: return verb +def _git_positionals(rest: list[str]) -> list[str]: + """The non-option arguments of a git subcommand, option *values* excluded. + + The same walk as the invocation splitter's global-option skip, one level + down: an option that takes a separate value consumes the next token, so + `["--ref", "refs/notes/x", "list"]` yields `["list"]` instead of + `["refs/notes/x", "list"]`. A value is never a subcommand, and treating one + as a subcommand is how `git reflog -n 5` was refused as a mutator (#1240). + """ + out: list[str] = [] + i = 0 + while i < len(rest): + tok = rest[i] + if tok in _GIT_SUBCOMMAND_WITH_VALUE: + i += 2 + continue + if tok.startswith("-"): + i += 1 + continue + out.append(tok) + i += 1 + return out + + def _shape_decided_verdict(verb: str, rest: list[str]) -> str | None: """The verdict for a verb whose subcommand / flags decide its effect. Returns the verb when the invocation writes, or None when it is a proven read. Every branch treats "not recognisably a read" as a write. """ - positional = [t for t in rest if not t.startswith("-")] + positional = _git_positionals(rest) sub = next(iter(positional), None) if verb == "stash": # `stash list` / `stash show` read; a bare `git stash` saves and cleans @@ -908,6 +959,19 @@ def _shape_decided_verdict(verb: str, rest: list[str]) -> str | None: # `git hash-object <file>` computes and prints an object name — a read. # `-w` additionally writes the object into the database. return verb if "-w" in rest else None + if verb == "reflog": + # Bare `git reflog` is `reflog show` — it prints. `expire` / `delete` / + # `drop` rewrite the reflog, so they stay blocked, as does any + # subcommand git adds later (fail-closed). + return None if sub in (None, "show", "list", "exists") else verb + if verb == "notes": + # `list` / `show` print; `add` / `copy` / `append` / `edit` / `remove` / + # `prune` write the notes ref. Bare `git notes` prints the note list. + return None if sub in (None, "list", "show") else verb + if verb == "bisect": + # Only the pure reporters. `start` / `good` / `bad` / `skip` / `reset` / + # `run` write `.git/BISECT_*`, and `replay` can rewrite history. + return None if sub in ("log", "view", "visualize") else verb return verb diff --git a/tests/test_bash_tool_sandbox.py b/tests/test_bash_tool_sandbox.py index 7b3db2b..745899e 100644 --- a/tests/test_bash_tool_sandbox.py +++ b/tests/test_bash_tool_sandbox.py @@ -918,14 +918,24 @@ def test_git_classification_is_fail_closed_over_every_subcommand(): shape_decided = _GIT_SHAPE_DECIDED # Verbs git reports that are not declared reads and are not shape-decided # must block, whether or not anyone remembered them. + # `reflog` and `notes` left this set in issue #1240. They are now + # shape-decided: their reporting form is a read (`git reflog` is + # `git reflog show`, `git notes` is `git notes list`) while `expire` / + # `delete` / `drop` and `add` / `remove` / `append` / `prune` write. This + # set is defined as "not a declared read and not shape-decided", so once + # that is true of them, keeping them here would make the test assert + # something false about its own predicate. Their writing shapes are asserted + # by test_check_read_only_blocks_unlisted_plumbing_mutators (the reflog + # `expire` case) and by tests/test_git_read_verbs_shape.py (the rest). unlisted = { "checkout-index", "mktree", "mktag", "filter-branch", "replace", - "update-server-info", "pack-refs", "reflog", "symbolic-ref", - "update-ref", "read-tree", "sparse-checkout", "notes", "init", + "update-server-info", "pack-refs", "symbolic-ref", + "update-ref", "read-tree", "sparse-checkout", "init", "clone", "revert", "cherry-pick", "rebase", "switch", "restore", "unpack-objects", "index-pack", "pack-objects", "fast-import", "fast-export", "update-index", "write-tree", "commit-tree", } + assert {"reflog", "notes"} <= shape_decided, "shape-decided since #1240" for verb in sorted(unlisted): assert verb not in allowlist, f"{verb!r} must not be a declared read" allowed, reason, _ = _check_sandbox(f"git {verb}", "read-only") diff --git a/tests/test_git_read_verbs_shape.py b/tests/test_git_read_verbs_shape.py new file mode 100644 index 0000000..c2b29d1 --- /dev/null +++ b/tests/test_git_read_verbs_shape.py @@ -0,0 +1,118 @@ +"""Four pure-read git verbs were refused as mutating (issue #1240). + +`_GIT_READ_VERBS` is an **allowlist**: a verb that is not on it is treated as a +mutator whatever it actually does. Measured on master `e6eaaee4`, `read-only` +tier: `git reflog`, `git notes list`, `git bisect log` and `git cherry master` +were all refused with the reason "blocked git mutating command" — a statement +that is not true of them. `reflog` expires entries only as `git reflog expire`; +`notes` writes only for `add`/`copy`/`append`/`edit`/`remove`/`prune`; `bisect` +writes `.git/BISECT_*` only for `start`/`good`/`bad`/`skip`/`reset`; `cherry` has +no writing form at all. + +That costs a downgraded cycle exactly the commands that would explain *how the +tree got dirty* — `git reflog` is the ref log — which is the situation the guard +itself creates. + +Both halves are asserted, because "allow more" is also what a broken guard does: +every writing shape of the same verbs must stay blocked, and an unrelated set of +mutators must be unaffected. +""" + +import pytest + +from emrg.tools.bash_tool import _check_sandbox + +TIER = "read-only" + + +def _allowed(cmd: str) -> bool: + allowed, _reason, _ = _check_sandbox(cmd, TIER) + return allowed is True + + +@pytest.mark.parametrize("cmd", [ + "git reflog", + "git reflog show", + "git reflog show HEAD", + "git reflog exists HEAD", + "git -C . reflog show", + "git reflog -n 5", + "git reflog --all", + "git reflog --date=iso show", + "git reflog show -n 3", +]) +def test_reflog_reads_are_allowed(cmd: str) -> None: + """The ref log is the read a downgraded cycle most needs, and it writes nothing.""" + assert _allowed(cmd), f"{cmd!r} only prints, must be allowed" + + +@pytest.mark.parametrize("cmd", [ + "git notes list", + "git notes show HEAD", + "git notes", + "git notes --ref refs/notes/x list", + "git notes --ref=refs/notes/x list", +]) +def test_notes_reads_are_allowed(cmd: str) -> None: + assert _allowed(cmd), f"{cmd!r} only prints notes, must be allowed" + + +@pytest.mark.parametrize("cmd", ["git bisect log", "git bisect view", "git bisect visualize"]) +def test_bisect_reporters_are_allowed(cmd: str) -> None: + assert _allowed(cmd), f"{cmd!r} only prints the bisect log, must be allowed" + + +@pytest.mark.parametrize("cmd", ["git cherry", "git cherry master", "git cherry -v origin/master HEAD"]) +def test_cherry_is_allowed(cmd: str) -> None: + """`git cherry` reports commits not upstream; it has no writing form.""" + assert _allowed(cmd), f"{cmd!r} only prints, must be allowed" + + +@pytest.mark.parametrize("cmd", [ + # reflog: expiring or deleting entries rewrites the reflog + "git reflog expire --expire=now --all", + "git reflog delete HEAD@{1}", + "git reflog drop HEAD@{1}", + # notes: these write + "git notes add -m x", + "git notes -f add -m x", + "git notes --ref refs/notes/x add -m y", + "git notes --ref refs/notes/x remove HEAD", + "git bisect --no-checkout start", + "git notes remove HEAD", + "git notes append -m x", + "git notes copy a b", + "git notes prune", + "git notes merge origin/x", + # bisect: these write .git/BISECT_* — and `replay` rewrites history + "git bisect start", + "git bisect good", + "git bisect bad", + "git bisect reset", + "git bisect skip", + "git bisect replay log.txt", + "git bisect run make", +]) +def test_writing_shapes_of_the_same_verbs_stay_blocked(cmd: str) -> None: + """The half that keeps this a fix rather than a relaxation.""" + assert not _allowed(cmd), f"{cmd!r} writes; must stay blocked" + + +@pytest.mark.parametrize("cmd", [ + "git stash drop", "git checkout .", "git clean -fd", "git reset --hard", + "git config user.name x", "git branch -D old", "git tag -d v1", + "git commit -m x", "git push origin master", +]) +def test_unrelated_mutators_are_unaffected(cmd: str) -> None: + assert not _allowed(cmd), f"{cmd!r} writes; must stay blocked" + + +@pytest.mark.parametrize("cmd", [ + "git status", "git log -1", "git diff HEAD", "git stash list", + "git worktree list", "git branch -a", "git config -l", + "git count-objects -v", "git fsck", "git shortlog -sn", + "git describe --tags", "git ls-files", "git rev-parse HEAD", +]) +def test_reads_that_were_already_allowed_stay_allowed(cmd: str) -> None: + """No regression: the widening must not disturb the existing read set.""" + assert _allowed(cmd), f"{cmd!r} only prints, must stay allowed"
tests/test_git_read_verbs_shape.pyis a new file (itsdiffheader carries
new file mode), so it is included in the patch rather than left untracked —git diffalone
silently omits untracked files, which is how my first attempt at this patch shipped five hunks
and no test.Verification
Round trip.
git apply --checkrc=0 thengit applyrc=0 on a fresh clone of master
e6eaaee4, reproducing the measured arm exactly. These are the file shas after applying the
published text, not the local copy's — a reader can repeat it from the fence above:file sha256[:16] after applying emrg/tools/bash_tool.py20a8bfc247257838tests/test_bash_tool_sandbox.py717d68c9463484betests/test_git_read_verbs_shape.py96a6a1a2fea091e8A/B on the verdict corpus (63 commands,
_check_sandbox(cmd, "read-only"), arms differ only
inbash_tool.py— module sha29149538df32df38vs20a8bfc247257838, asserted on both arms
before the corpus runs, so a real comparison is impossible to fake by importing the wrong module):verdict changes exactly 2 BLOCK → ALLOW git notes --ref refs/notes/x listBLOCK → ALLOW git reflog -n 5unexpected changes none reads still blocked none writing shapes newly allowed none pre-existing verdicts disturbed none The delta equals the expected set, so the fix moves the two spellings it targets and nothing else.
Discriminating test. On the arm without the fix, the two new corpus entries fail
(2 failed, 61 passed); with it they pass. The test has a job — it is not passing for a
reason unrelated to the change.Full suite, both arms (
tests/, failure set compared, not the count — an equal count can
hide a trade):arm result failures A — published allowlist, no value fix 2 failed, 2051 passed, 4 skipped (84.59s) the 2 intended corpus entries B — arm A + this patch 2053 passed, 4 skipped (84.87s) none Newly failing on B: none. Fixed on B: exactly the 2 intended. So the 196 added lines cost no
existing assertion.Both patches are independent of the rest of the queue and can be taken in either order; this one
assumes the allowlist patch above is applied (it is written against it, and its corpus edits
extend the same file). The two together are what makegit reflogusable to a cycle that has
been downgraded — which is the situation the ref log was most needed in.Credit where the finding came from
The two refused spellings in the patch above (
git reflog -n 5,
git notes --ref refs/notes/x list) are not something I found — they come from
@how2how2how2-arch's independent reproduction (comment5674846966), which rebuilt both
published patches from their comment text and confirmed they reproduce byte-for-byte before
reporting the two gaps. That report is what turned this from a closed issue into a second,
separate defect one parser level down.Worth stating plainly: that reproduction was correct on every point, including the parts that
were inconvenient for the patches I had published.I have not edited the patch comment above to add this line, because that comment's text is the
artifact whose apply-and-compare round trip was verified against a fresh tree — editing it would
invalidate the verification. Hence a separate note.Correction to the patch comment above: that patch is a superset, not a dependent one
Comment
5675062829ends with "this one assumes the allowlist patch above is applied". Measured,
that is wrong, and wrong in the direction that matters for a landing cycle:applied to a fresh master tree rc emrg/tools/bash_tool.pysha256[:16]the allowlist patch alone 0 29149538df32df38this patch alone 0 20a8bfc247257838(its published arm)the allowlist patch, then this one 0, 1 29149538df32df38—error: patch failed: emrg/tools/bash_tool.py:220It is a superset: it already contains the allowlist hunks (
"cherry", andreflog/notes/
bisectin_GIT_SHAPE_DECIDED— both present in its text) plus the subcommand-value fix. So a
landing cycle must use one or the other, never both:- the allowlist patch alone → the first finding only;
- this patch alone → both findings, and that is the one to use for the pair.
Also worth recording:
#1240's change and#1238's edit the same list from different bases, so
composing them is not a matter of ordering — a plain--3wayleaves two real conflicts and the fix
is the union of the two (keep #1238's removals, add #1240's additions). That resolution, the whole
same-file set as a single appliable patch, and its verification (compiles, 8/8 membership, suite
2104 passed) are on #1234 with the measured detail.Both errors are mine, and both are the same shape: a statement about an artifact that measurement
contradicts. The dependency sentence had never been tested against the patch text it describes.- added a commit that references this issue
on Sep 15, 2026 Closed as fixed by #1250 (merged as
7712ee6a).Verified on the merged master
0cc6dd48(emrg/tools/bash_tool.pysha256[:16]ee8c80ab1837e248), by re-driving every row this issue itself lists through_check_sandbox(cmd, "read-only")— the issue's own acceptance, not the PR's summary of it.verb shape master e6eaaee4merged 0cc6dd48git reflog/git reflog show HEADBLOCK ("mutating") ALLOW git notes list/git notes show HEADBLOCK ("mutating") ALLOW git bisect log/git bisect viewBLOCK ("mutating") ALLOW git cherry masterBLOCK ("mutating") ALLOW git reflog expire --expire=now --allBLOCK BLOCK git notes add -m xBLOCK BLOCK git bisect reset/git bisect startBLOCK BLOCK The fix followed the pattern this issue proposed: the four verbs moved into
_GIT_SHAPE_DECIDED
with a proven-read shape and BLOCK wherever the shape is not proven, so the mutating forms stay
refused. The subcommand slot also had to be taught that some flags take a separated value
(--ref,-n, …) — without that,git notes --ref refs/notes/x listread the ref as its
subcommand, which is the same class of defect one level down.
The class, measured
_GIT_READ_VERBS(emrg/tools/bash_tool.py:203) is an allowlist: a verb that is not on it is treated as a mutator, whatever it actually does. Harness: an in-process call to_check_sandbox(cmd, "read-only"), armed on the workspace/master build (sha256[:16] = dfd4e85600643e28; the probe prints its own path and sha), run from a neutral cwd.7 of 23 pure reads are refused — every one of them a read under git's own documentation, none creating, updating or deleting a ref, an object, the index, or a tracked file:
refloggit reflogreflog showgit reflog show HEADnotesgit notes list/git notes show HEADbisectgit bisect log/git bisect viewcherrygit cherry masterstash listgit stash listworktree listgit worktree listfsck,count-objects -v,shortlog,describe,blame,log,diff,show,status,rev-parse,ls-files,branch --list,tag --list,config --getTwo things the table says that the single instance does not:
reflogis a mutating command; it is not.reflogexpires entries only asgit reflog expire, and it has no--output. Reporting a pure read as a mutation is the spelling-vs-effect defect fixed in sandbox: read-only says no writes allowed, but only four command patterns block a write; plain rm and sed -i are allowed #1162 and read-only tier: a reader's write flag is invisible — six git spellings write while read-only says ALLOW #1238 pointing the other way — there a write was invisible, here a read is refused for not appearing on a list. A refused command should be told why in terms that are true of it.stashandworktreelive in_GIT_SHAPE_DECIDEDand theirlistshapes are proven reads. The four verbs above have no shape logic at all, so they fall through to "not on the allowlist ⇒ mutator".Why this costs something rather than being harmless conservatism
A downgraded cycle — exactly the situation this guard creates — is denied the commands that would explain how the tree got dirty:
git reflog(the ref log) andgit bisect log. I hit this live while diagnosing the dirty-tree downgrade for #1237; the workaround was reading refs and object ids by hand, which cannot answer "what did this tree look like before".It also makes the tier asymmetric in a way that is hard to reason about from outside:
git stash listreads fine,git reflogdoes not, for no reason the reason string states.Proposed fix (shape-decided, following the existing pattern)
Move all four into
_GIT_SHAPE_DECIDED, give each a proven-read shape, and default to BLOCK wherever the shape is not proven — the rule the file already applies tostash/worktree/branch/tag:reflog— allow with no subcommand, orshow/list/exists; BLOCK when the first argument isexpire/delete/drop(those write).notes— allowlist/showonly;add/copy/append/edit/remove/prunewrite.bisect— allow only the pure reporterslog,view,visualize;start/good/bad/skip/resetwrite.git/BISECT_*and must stay BLOCK;replayis ambiguous (it can rewrite history) and should stay BLOCK unless separately measured.cherry— a pure reporter of commits not upstream; allow (it has no writing form).Sequencing
A patch here would be a fifth same-file patch for
emrg/tools/bash_tool.py, alongside #1234, #1236 and #1238. Those three already have a measured required order — #1234 must be applied last; only 2 of the 6 orderings land all hunks (#1234) — so this one must be composed and measured against whichever of them has landed, not generated blind. For that reason I have not generated a patch here.Not claimed
Not measured: whether these verbs are blocked by a different rule in
workspace-write(every row above isread-only), and the linked-worktree case. The probe is/private/tmp/emrgprobe/probe_git_read_verbs.py.