diff --git a/packages/mosaic/framework/tools/git/test-wrapper-guard.sh b/packages/mosaic/framework/tools/git/test-wrapper-guard.sh index 150a5f9f..544f8313 100755 --- a/packages/mosaic/framework/tools/git/test-wrapper-guard.sh +++ b/packages/mosaic/framework/tools/git/test-wrapper-guard.sh @@ -137,6 +137,39 @@ FIXTURES="$TMP/fixtures.tsv" # detection: neither of these names curl at all. printf '2\t{"tool_input":{"command":"python3 -c '"'"'import requests; requests.post(\\"https://git.example.invalid/api/v1/repos/a/b/issues/1/comments\\", json={})'"'"'"}}\ta library call is a write with no flag and no curl\n' printf '2\t{"tool_input":{"command":"wget --post-data=x https://git.example.invalid/api/v1/repos/a/b/issues/1/comments"}}\twget spells its body differently and is still a write\n' + + # Round six scoped the guard on `https?://`, and review found the absence shape + # had simply moved to that new boundary: a raw provider CLI carries no scheme, + # so the guard never reached the write question. These are the reported repros. + printf '2\t{"tool_input":{"command":"gh api -X POST repos/a/b/issues -f title=x -f body=y"}}\tgh api is a raw write with no URL scheme at all\n' + printf '2\t{"tool_input":{"command":"gh api -X POST repos/a/b/pulls/1/reviews -f event=APPROVE"}}\tand it reaches the endpoint the review wrapper owns\n' + printf '2\t{"tool_input":{"command":"tea api -X POST repos/a/b/issues/1/comments -f body=x"}}\ttea api, same shape, different CLI\n' + printf '2\t{"tool_input":{"command":"curl -X POST -d x git.example.invalid/api/v1/repos/a/b/issues"}}\ta scheme-less host path is still an API write\n' + printf '2\t{"tool_input":{"command":"gh api repos/a/b/issues -f title=x"}}\tgh POSTs implicitly when handed a field, exactly as curl does with -d\n' + # ...and the boundary that stops a broader scope gate becoming block-everything. + printf '0\t{"tool_input":{"command":"gh api repos/a/b/pulls/1"}}\treading through a provider CLI stays untouched\n' + printf '0\t{"tool_input":{"command":"gh api -X POST repos/a/b/releases -f tag_name=v1"}}\tno wrapper owns releases, whoever calls it\n' + printf '0\t{"tool_input":{"command":"tea pulls create --title x --repo a/b"}}\tprovider PORCELAIN is out of scope by decision, not by accident\n' + printf '0\t{"tool_input":{"command":"rm -f /var/tmp/api/v1-issues-notes.txt"}}\t-f is only a body when it carries key=value\n' + printf '0\t{"tool_input":{"command":"grep -f patterns.txt /src/api/v1/repos/a/b/issues.log"}}\tsame, on the flag agents actually collide with\n' + + # Wrong remediation is its own defect: /issues/1/labels used to block with + # "use issue-create.sh", which is not the wrapper for that call. No wrapper + # owns an issue/PR subresource, so these flow through like every other + # unwrapped endpoint — and the two arms above them must keep blocking. + printf '0\t{"tool_input":{"command":"curl -X PATCH -d @b https://git.example.invalid/api/v1/repos/a/b/issues/1/labels"}}\tno wrapper owns issue labels, so there is nothing to enforce\n' + printf '0\t{"tool_input":{"command":"curl -X POST -d @b https://git.example.invalid/api/v1/repos/a/b/issues/1/assignees"}}\tnor assignees\n' + printf '0\t{"tool_input":{"command":"curl -X PATCH -d @b https://git.example.invalid/api/v1/repos/a/b/issues/comments/5"}}\tediting a comment has no wrapper either; only creating one does\n' + printf '2\t{"tool_input":{"command":"curl -X POST -d @b https://git.example.invalid/api/v1/repos/a/b/issues/1/comments"}}\tand the wrapped subresource must not fall through the arm above it\n' + printf '2\t{"tool_input":{"command":"curl -X POST -d @b https://git.example.invalid/api/v1/repos/a/b/issues"}}\tnor may issue creation\n' + printf '0\t{"tool_input":{"command":"curl -X PATCH -d @b https://git.example.invalid/api/v1/repos/a/b/pulls/1/labels"}}\tPR labels are unwrapped on the same reasoning as issue labels\n' + printf '2\t{"tool_input":{"command":"curl -X POST -d @b https://git.example.invalid/api/v1/repos/a/b/pulls"}}\tPR creation is wrapped and must not fall through with them\n' + + # The APPROVE trap, in the spelling a provider CLI uses, and the value that + # must never trip it. + printf '0\t{"tool_input":{"command":"curl -X POST -d {\\"event\\":\\"APPROVED\\"} https://git.example.invalid/api/v1/repos/a/b/releases"}}\tAPPROVED is the correct value and is never the trap\n' + # Documented over-block, pinned so it is a known boundary and not a surprise. + printf '2\t{"tool_input":{"command":"python3 -c '"'"'print(\\"https://git.example.invalid/api/v1/repos/a/b/issues/1/comments .post(\\")'"'"'"}}\tprose carrying .post( near a wrapped URL is refused, by the same payload rule\n' } > "$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 6e2370cd..b9cc393a 100755 --- a/packages/mosaic/framework/tools/git/wrapper-guard.sh +++ b/packages/mosaic/framework/tools/git/wrapper-guard.sh @@ -132,8 +132,28 @@ fi # mistakes; it is not a sandbox, and pretending otherwise is how you get a # control nobody can trust the boundaries of. # -# Scoped to commands carrying a URL, so nothing without one is even considered. -if printf '%s' "$CMD" | grep -Eq 'https?://'; then +# Scoped to commands that are provider-API-shaped, so nothing else is even +# considered. The first version of this scope gate asked only for `https?://`, +# and review found the absence shape had simply moved to the new boundary: +# gh api -X POST repos/a/b/pulls/1/reviews -f event=APPROVE +# tea api -X POST repos/a/b/issues/1/comments -f body=x +# curl -X POST -d x git.example.invalid/api/v1/repos/a/b/issues +# all carry a real write to a wrapped endpoint and none carries a scheme, so the +# guard never asked the write question at all. Gate 7 covers raw provider CLIs, +# so these are in scope and the gate now names the shapes they come in. +# +# Adding alternatives to a scope gate can only make it stricter — it cannot +# create a new allow — which is why this is a list of triggers rather than a +# model of any one caller. +# +# Boundary, deliberate and worth stating: this covers the `api` subcommand, +# which is a raw API call wearing a CLI. Provider PORCELAIN (`tea pulls create`, +# `gh pr merge`) is NOT covered — catching that means modelling every CLI's verb +# grammar, which is the parser mistake again in a new costume. Porcelain is a +# gate-7 gap for prose and review to hold, not this hook. +API_SHAPED='https?://|/api/v[0-9]' +API_SHAPED="$API_SHAPED"'|(^|[[:space:]|;&(])(gh|tea|glab|hub)[[:space:]]+api([[:space:]]|$)' +if printf '%s' "$CMD" | grep -Eq "$API_SHAPED"; then # Write detection, now client-agnostic. Every spelling curl accepts, because # the guard is defeated by the one spelling it does not know: `-d@body` (no @@ -151,7 +171,18 @@ if printf '%s' "$CMD" | grep -Eq 'https?://'; then # curl sends POST implicitly when handed a body, in any of these forms. printf '%s' "$CMD" | grep -Eq -- \ '(^|[[:space:]])(-d|-F|-T)|--data([-a-z]*)?[[:space:]=]|--json[[:space:]=]|--form|--upload-file|--post-(data|file)[[:space:]=]' && is_write=1 + # The provider CLIs POST implicitly the same way curl does, when handed a + # field. Matched only in `-f key=value` shape, so the far more common `rm -f` + # and `grep -f` cannot be read as a body. + printf '%s' "$CMD" | grep -Eq -- \ + '(^|[[:space:]])(-f|--field|--raw-field)[[:space:]]+[A-Za-z_][A-Za-z0-9_.-]*=|--input[[:space:]=]' && is_write=1 # ...and a library call is a write without any flag at all. + # + # Second documented over-block, and broader than the body flags because it + # needs no flag: any text carrying `.post(` near a wrapped URL is refused, + # including prose that merely quotes it. That follows from the same rule as + # the quoted-curl cost above — the payload is judged, not the caller — and it + # is stated here so it is a known boundary rather than a surprise. printf '%s' "$CMD" | grep -Eq -- \ '\.(post|put|patch|delete)\(' && is_write=1 @@ -162,6 +193,19 @@ if printf '%s' "$CMD" | grep -Eq 'https?://'; then endpoint="pull-request review"; wrapper="pr-review.sh" ;; *"/pulls/"*"/merge"*) endpoint="pull-request merge"; wrapper="pr-merge.sh" ;; *"/issues/"*"/comments"*) endpoint="issue comment"; wrapper="issue-comment.sh" ;; + # Any OTHER path UNDER a numbered issue or PR — labels, assignees, times, + # a comment edit at /issues/comments/{id}, a PATCH of the issue itself. + # No wrapper owns these, and this arm exists so the guard does not claim + # one does: before it, /issues/1/labels fell through to the generic arm + # below and was refused with "use issue-create.sh", which is the wrong + # call. Wrong remediation is worse than no remediation, because a block an + # agent cannot comply with teaches it that the hook is broken and the + # override is routine — and an override that is routine is a guard that is + # off. So these flow through, exactly as /releases and every other + # unwrapped endpoint already does. That is the standing rule here: this + # hook enforces "use the wrapper", and where there is no wrapper it has + # nothing to enforce. The gap belongs in the wrapper set, not in a block. + *"/issues/"?*|*"/pulls/"?*) : ;; *"/pulls"*) endpoint="pull request"; wrapper="pr-create.sh" ;; *"/issues"*) endpoint="issue"; wrapper="issue-create.sh" ;; *"/milestones"*) endpoint="milestone"; wrapper="milestone-create.sh" ;; @@ -251,7 +295,10 @@ EOF fi # ---- 3. the APPROVE/APPROVED trap, wherever it appears --------------------- -if printf '%s' "$CMD" | grep -Eq '"event"[[:space:]]*:[[:space:]]*"APPROVE"'; then +# Both spellings the trap arrives in: the JSON body `"event": "APPROVE"` and the +# provider-CLI field `-f event=APPROVE`. The trailing [^A-Z] is what keeps the +# correct value out of it — APPROVED must never match. +if printf '%s' "$CMD" | grep -Eq 'event"?[[:space:]]*[=:][[:space:]]*"?APPROVE([^A-Z]|$)'; then cat <