diff --git a/packages/mosaic/framework/tools/git/test-wrapper-guard.sh b/packages/mosaic/framework/tools/git/test-wrapper-guard.sh index 52dd6d21..150a5f9f 100755 --- a/packages/mosaic/framework/tools/git/test-wrapper-guard.sh +++ b/packages/mosaic/framework/tools/git/test-wrapper-guard.sh @@ -59,58 +59,84 @@ FIXTURES="$TMP/fixtures.tsv" printf '2\t{"tool_input":{"command":"a=/api/v1/repos/a/b/iss; b=ues/1/comments; curl -d@body https://git.example.invalid${a}${b}"}}\tan endpoint token split across variables is unreadable, not absent\n' printf '2\t{"tool_input":{"command":"a=/api/v1/repos/a/b/pu; b=lls/1/reviews; curl -d@body https://git.example.invalid${a}${b}"}}\tsame split, review endpoint\n' printf '0\t{"tool_input":{"command":"curl -X POST -d @payload https://hooks.example.invalid/services/${WEBHOOK_ID}"}}\tan opaque URL that is not forge-shaped stays allowed\n' - # And the other direction, which is the failure mode that gets a hook deleted: - # discussing a call is not making one. In each of these the client sits behind - # a quote, never at command position. - printf '0\t{"tool_input":{"command":"grep -R \\"curl -d https://git.example.invalid/api/v1/repos/a/b/issues\\" docs/"}}\tgrepping for an example is not calling it\n' - printf '0\t{"tool_input":{"command":"echo \\"curl -d https://git.example.invalid/api/v1/repos/a/b/pulls\\" > note.txt"}}\twriting an example into a file is not calling it\n' - printf '0\t{"tool_input":{"command":"python3 -c '"'"'print(\\"curl -d https://git.example.invalid/api/v1/repos/a/b/issues\\")'"'"'"}}\tprinting an example is not calling it\n' + # --- round six changed the contract in this direction, and these fixtures are + # where it shows. They used to assert that discussing a call is not making one. + # Five rounds proved there is no textual way to tell a quoted example from a + # quoted command, so the guard stopped trying: it judges the payload, and a + # payload inside quotes is still a payload. Quoting one of these on a Bash + # command line is now refused, and the way to write the example is a + # file-writing tool. This is the deliberate cost of the mechanism change. + printf '2\t{"tool_input":{"command":"grep -R \\"curl -d https://git.example.invalid/api/v1/repos/a/b/issues\\" docs/"}}\tquoting a wrapped write is refused even in a grep\n' + printf '2\t{"tool_input":{"command":"echo \\"curl -d https://git.example.invalid/api/v1/repos/a/b/pulls\\" > note.txt"}}\t...and when written into a file\n' + printf '2\t{"tool_input":{"command":"python3 -c '"'"'print(\\"curl -d https://git.example.invalid/api/v1/repos/a/b/issues\\")'"'"'"}}\t...and when printed from another language\n' + # The boundary that keeps this from being "block everything": what is refused + # is a WRITE to a WRAPPED endpoint. Mentioning either alone still passes, and + # these are asserted as hard as the blocks above. + printf '0\t{"tool_input":{"command":"grep -R \\"curl -s https://git.example.invalid/api/v1/repos/a/b/issues/1/comments\\" docs/"}}\tquoting a READ example is untouched\n' + printf '0\t{"tool_input":{"command":"echo \\"the wrapped endpoint is https://git.example.invalid/api/v1/repos/a/b/issues/1/comments\\" >> notes.md"}}\tnaming the endpoint without a body flag is untouched\n' + printf '0\t{"tool_input":{"command":"grep -R \\"curl -d@b https://git.example.invalid/api/v1/repos/a/b/releases\\" docs/"}}\tquoting a write to an UNWRAPPED endpoint is untouched\n' + printf '0\t{"tool_input":{"command":"issue-comment.sh --repo a/b --issue 1 --body @msg.md"}}\tthe wrapper itself carries a body flag and must never trip its own guard\n' # Command position must still catch the real thing behind operators and env. printf '2\t{"tool_input":{"command":"cd /tmp && GITEA_TOKEN=$T curl -d@b https://git.example.invalid/api/v1/repos/a/b/pulls/1/merge"}}\ta real call behind && and an assignment is still a call\n' - # --- and the case the AUTHOR hit, one level in from the reported one: an - # operator INSIDE a quoted string is not an operator. This blocked a message - # that merely quoted the fixture above. Position is judged on the skeleton. - printf '0\t{"tool_input":{"command":"send.sh -m \\"repro was: cd /tmp && curl -d@b https://git.example.invalid/api/v1/repos/a/b/pulls/1/merge\\""}}\tan operator inside a quoted string is not an operator\n' - printf '0\t{"tool_input":{"command":"cat >> notes.md <> notes.md <> notes.md"}}\tnaming a command after echo is not running it\n' + printf '2\t{"tool_input":{"command":"echo timeout 10 curl -d@b https://git.example.invalid/api/v1/repos/a/b/issues/1/comments >> notes.md"}}\tnaming the call after echo carries the payload, so it is refused\n' # A shell standing between quoted data and execution makes that data code, # and the pipe is the form agents actually use. Filing it as data allowed the # call to vanish from the skeleton while still running. printf '2\t{"tool_input":{"command":"printf '"'"'%%s\\\\n'"'"' '"'"'curl -d@b https://git.example.invalid/api/v1/repos/a/b/issues/1/comments'"'"' | sh"}}\tquoted code piped to a shell is code\n' printf '2\t{"tool_input":{"command":"cat <> notes.md"}}\tan unrelated shell on another line does not promote quoted prose to code\n' + # ...and the questions that used to follow — is the pipe target a shell, does a + # shell on one line execute a string on another — no longer have to be answered + # at all. Both of these carry the payload, both are refused, and neither + # outcome depends on parsing what the pipe or the other line does. + printf '2\t{"tool_input":{"command":"grep -R \\"curl -d@b https://git.example.invalid/api/v1/repos/a/b/issues/1/comments\\" docs/ | wc -l"}}\tpiping the payload to wc is refused without asking what wc is\n' + printf '2\t{"tool_input":{"command":"docker run --rm alpine sh -c '"'"'echo hi'"'"'\\necho \\"example: curl -d@b https://git.example.invalid/api/v1/repos/a/b/issues/1/comments\\" >> notes.md"}}\tan unrelated shell on another line no longer changes the answer either way\n' # --- round four. The guard was still reading the command as typed rather than # as the shell will run it: a backslash before a newline is removed before # anything else happens, so the endpoint token can be split across the join. printf '2\t{"tool_input":{"command":"curl -d@b https://git.example.invalid/api/v1/repos/a/b/iss\\\\\\nues/1/comments"}}\ta line continuation inside the endpoint token is still that endpoint\n' printf '2\t{"tool_input":{"command":"curl -d@b https://git.example.invalid/api/v1/repos/a/b/pu\\\\\\nlls/1/reviews"}}\tsame join, review endpoint\n' - printf '0\t{"tool_input":{"command":"cat >> notes.md <> notes.md < "$FIXTURES" fail=0 n=0 diff --git a/packages/mosaic/framework/tools/git/wrapper-guard.sh b/packages/mosaic/framework/tools/git/wrapper-guard.sh index cd539736..6e2370cd 100755 --- a/packages/mosaic/framework/tools/git/wrapper-guard.sh +++ b/packages/mosaic/framework/tools/git/wrapper-guard.sh @@ -22,6 +22,10 @@ # curl for reads, for registry/manifest calls, and for endpoints with no # wrapper (there are many) all pass untouched. # +# One consequence is worth knowing before it surprises you: it judges the +# payload, not the caller, so a command that merely QUOTES such a write is +# refused as well. See the long note at section 2 for why that trade was made. +# # Break-glass, for a genuine gap where no wrapper can express the call: # MOSAIC_WRAPPER_OVERRIDE=1 # Using it means "no wrapper covers this" — if that is wrong, the fix is to @@ -83,103 +87,73 @@ EOF fi # ---- 2/3. provider API writes --------------------------------------------- -# A raw provider write is four things at once: an HTTP client, a URL, a mutating -# verb or a request body, and a path fragment naming an endpoint a wrapper -# already owns. All four are required, which is what keeps reads and unwrapped -# endpoints flowing. +# A raw provider write this guard cares about is two things: a WRITE, and a URL +# naming an endpoint a Mosaic wrapper already owns. Reads are untouched — they +# are how you gather evidence — and the many endpoints with no wrapper flow +# through. # -# Deliberately NOT gated on the literal "/api/v1/repos/". An independent reviewer -# broke that version in one line: build the path in shell variables -# p=/api/v1/repo; q=s/a/b/pulls/1/reviews; curl -d@body "https://host${p}${q}" -# and the host-anchored literal never appears, so the check read clean while the -# write went through. The endpoint fragments below survive it, because the -# fragment has to appear somewhere for the URL to be constructible at all. -# The client must be at COMMAND POSITION, and that has to be judged against the -# CODE in the command, not against its text. Review caught the text version -# blocking ordinary work: -# grep -R "curl -d https://host/api/v1/repos/a/b/issues" docs/ -# echo "curl -d https://host/api/v1/repos/a/b/pulls" > note.txt -# Talking about a call is not making one, and over-blocking is not the safe -# direction: a guard that blocks ordinary work gets switched off, and a guard -# that is off permits everything. +# It deliberately does NOT ask which program makes the call, or whether that +# program sits at shell command position. It used to, and that is the whole +# history of this file. Answering "is this code or is this data" from the text +# of a shell command required a skeleton with quoted spans and heredoc bodies +# removed, an invoker list for the forms where a shell executes quoted text, a +# prefix list for `env`/`sudo`/`timeout`, option-value skipping, and +# backslash-newline joining. Five rounds of adversarial review put nineteen +# writes straight through it, and every one had the same shape: the client was +# ABSENT from the skeleton, so the guard allowed. Variables, line continuations, +# command prefixes, option values, pipes into a shell, and finally command +# substitution inside the very quotes the skeleton was discarding: +# echo "$(curl -d@b .../issues/1/comments)" +# msg="$(curl -d@b .../issues/1/comments)" +# Classifying code against data in shell text with sed and awk is not a hard +# problem, it is the wrong problem. It was not even portable: under CI's busybox +# awk the quote-stripping silently failed, the skeleton kept every quoted span, +# and the guard started refusing ordinary prose instead — which is the other way +# a control like this dies. # -# A first fix required the client to follow a shell operator. That lasted until -# the author sent a message quoting one of these fixtures — the quoted text -# contained `... && GITEA_TOKEN=$T curl -d@b .../merge`, so an operator appeared -# INSIDE the quotes and the guard blocked the message. Same defect, one level -# in: an operator inside a string is not an operator. +# So the client detection is gone, and with it that entire failure class: what +# is left cannot fail open by hiding the caller, because it never looks for one. +# It looks for the payload. Something that names a wrapped endpoint and carries +# a body is refused however it is spelled — curl, wget, `python -c`, or a form +# nobody has thought of yet. # -# So the position test runs against a SKELETON — the command with its data spans -# (quoted strings, heredoc bodies) removed. Endpoint, URL and body detection all -# still run against the FULL text, because real calls quote their URLs and a -# skeleton would be blind to them. +# The cost is real and belongs in the open, because over-blocking is how a hook +# gets switched off: QUOTING one of these calls on a Bash command line now +# blocks too. `grep -R "curl -d .../issues" docs/` is refused, and so is echoing +# an example into a file. There is no textual way to tell a quoted example from +# a quoted command — that is exactly the finding above — so the rule is the one +# an agent can hold in mind without a parser: # -# The exception is the reason quotes are data at all: if something is about to -# EXECUTE the quoted text, then the quotes hold code and the skeleton is the -# full text again. The first version of this list named only `bash -c`, `sh <<` -# and `eval`, and review immediately produced the spellings it did not know: -# printf '%s\n' 'curl -d@b .../comments' | sh -# cat <