From 51746f44eb0967d9155c85d8f455e9d080dba0c5 Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Wed, 12 Aug 2026 18:53:25 -0500 Subject: [PATCH] wrapper-guard: fail closed on an unreadable endpoint in every shape the scope gate admits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-9 review found the fail-closed rule narrower than the block it guards. The scope gate admits three shapes — a scheme URL, a schemeless /api/vN path, and a provider CLI's `api` subcommand — but the unreadable-endpoint test asked only for https?://. So a split endpoint token in the other two shapes was in scope to be blocked, produced no readable endpoint, and fell through to ALLOW, while the identical split behind a literal scheme blocked. Four writes reaching the provider unexamined, one of them a review verdict: p=repos/a/b/iss; q=ues; gh api -X POST ${p}${q} -f title=x p=repos/a/b/issues/1/comm; q=ents; gh api -X POST ${p}${q} -f body=x p=repos/a/b/pulls/1/rev; q=iews; gh api -X POST ${p}${q} -f event=APPROVED p=/api/v1/repos/a/b/iss; q=ues; curl -X POST -d x host${p}${q} This is the same defect class as the milestone arm one round earlier, one layer up: there the map claimed a span its wrapper did not cover, here a control claimed a surface it did not measure. A control is only as wide as its narrowest arm, and widening the scope gate without widening the fail-closed rule left the gap exactly where the gate had just been extended. Three arms now, one per admitted shape: a scheme URL token carrying an expansion; a schemeless token carrying both a forge fragment and an expansion, in either order; and the endpoint argument of a provider-CLI api call, read positionally. Limits are stated in the source rather than implied — a caller who splits the hostname as well, and an endpoint pushed past an option whose value contains whitespace, are both outside what this measures. An expansion in a BODY is explicitly not unreadable. Passing a payload in a variable is the safe practice and leaves the endpoint fully legible; blocking it would have been a control punishing the behaviour it wants. 101/101 fixtures, up from 92. Each arm is negative-controlled separately: removing the schemeless arm fails exactly the two schemeless fixtures, removing the provider-CLI arm fails exactly the four CLI fixtures, and neither disturbs any pre-existing fixture. One added fixture was rewritten after it passed for the wrong reason — its endpoint was readable, so it blocked on the endpoint map and never exercised the arm it was written for. Evidence: 101/101 locally and in ci-base; shellcheck clean at warning+; a 12-command sweep of ordinary forge work — reads with split endpoints, bodies in variables, unwrapped endpoints, an artifact PUT — blocks none of them; the 18-command sweep still blocks the same three round-six flips and nothing new. --- .../framework/tools/git/test-wrapper-guard.sh | 17 +++++ .../framework/tools/git/wrapper-guard.sh | 67 ++++++++++++++----- 2 files changed, 68 insertions(+), 16 deletions(-) diff --git a/packages/mosaic/framework/tools/git/test-wrapper-guard.sh b/packages/mosaic/framework/tools/git/test-wrapper-guard.sh index accbb444..9af08039 100755 --- a/packages/mosaic/framework/tools/git/test-wrapper-guard.sh +++ b/packages/mosaic/framework/tools/git/test-wrapper-guard.sh @@ -180,6 +180,23 @@ FIXTURES="$TMP/fixtures.tsv" # Requesting a reviewer is not submitting one; pr-review.sh files verdicts and # nothing in the tree adds a requested reviewer. printf '0\t{"tool_input":{"command":"gh api -X POST repos/a/b/pulls/1/requested_reviewers -f reviewers[]=u"}}\tno wrapper requests a reviewer, so it is not refused with pr-review.sh\n' + # SPAN, applied to the guard's OWN fail-closed rule rather than to a wrapper. + # The scope gate admits three shapes; the unreadable-endpoint rule asked only + # for `https?://`, so a split endpoint in the other two was in scope to block, + # produced no readable endpoint, and fell through to allow. Same defect class + # as the milestone arm, one layer up. Each shape gets its own fixture, because + # a single one would have passed on the arm that already worked. + printf '2\t{"tool_input":{"command":"p=repos/a/b/iss; q=ues; gh api -X POST ${p}${q} -f title=x"}}\ta split endpoint in a provider-CLI api call is unreadable, not absent\n' + printf '2\t{"tool_input":{"command":"p=repos/a/b/issues/1/comm; q=ents; gh api -X POST ${p}${q} -f body=x"}}\tsame, comments\n' + printf '2\t{"tool_input":{"command":"p=repos/a/b/pulls/1/rev; q=iews; gh api -X POST ${p}${q} -f event=APPROVED"}}\tsame, and a verdict is the costliest one to lose\n' + printf '2\t{"tool_input":{"command":"p=repos/a/b/iss; q=ues; tea api -X POST ${p}${q} -f title=x"}}\tevery CLI the scope gate admits, not just gh\n' + printf '2\t{"tool_input":{"command":"p=/api/v1/repos/a/b/iss; q=ues; curl -X POST -d x git.example.invalid${p}${q}"}}\ta schemeless forge host with a split path is unreadable too\n' + printf '2\t{"tool_input":{"command":"h=git.example.invalid; q=ues; curl -X POST -d x ${h}/api/v1/repos/a/b/iss${q}"}}\tthe expansion may come first; the token is what matters\n' + # And the reason this is not "any variable blocks a write": a payload in a + # variable is the SAFE way to pass one and leaves the endpoint fully legible. + printf '0\t{"tool_input":{"command":"gh api repos/a/b/git/refs -f sha=$SHA"}}\tan expansion in a body value leaves the endpoint readable\n' + printf '0\t{"tool_input":{"command":"curl -X POST -d \\"$BODY\\" https://git.example.invalid/api/v1/repos/a/b/git/refs"}}\tsame for a quoted body on an unwrapped endpoint\n' + printf '0\t{"tool_input":{"command":"gh api repos/${OWNER}/${REPO}/git/refs"}}\ta read with a split endpoint is still a read\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; only creating one does\n' printf '0\t{"tool_input":{"command":"curl -X POST -d @b https://git.example.invalid/api/v1/repos/a/b/issues/1/stopwatch/start"}}\tno wrapper owns a stopwatch, and none is invented for it\n' printf '0\t{"tool_input":{"command":"gh api -X POST repos/a/b/issues/1/times -f time=60"}}\tnor time tracking\n' diff --git a/packages/mosaic/framework/tools/git/wrapper-guard.sh b/packages/mosaic/framework/tools/git/wrapper-guard.sh index 4881fc6b..fd1e8d74 100755 --- a/packages/mosaic/framework/tools/git/wrapper-guard.sh +++ b/packages/mosaic/framework/tools/git/wrapper-guard.sh @@ -308,7 +308,7 @@ edit — that one is a real wrapper gap, and the override exists for it." ;; fi ;; esac - # A URL the guard cannot READ is a URL the guard must not CLEAR. + # An endpoint the guard cannot READ is an endpoint the guard must not CLEAR. # # Round one fixed one spelling of this and review immediately produced the # general form: split the endpoint token itself across two variables — @@ -318,25 +318,58 @@ edit — that one is a real wrapper gap, and the override exists for it." ;; # fragments is unwinnable: the endpoint does not exist until the shell # expands it, and this hook runs before that. # - # So stop pretending to read it. If a write's URL contains an expansion, + # So stop pretending to read it. If a write's endpoint contains an expansion, # the guard has no endpoint to judge, and "no endpoint" must not mean # "allowed" — that is the same absence-driven allow as the missing-wrapper # case, wearing different clothes. # - # Scoped to commands that are visibly forge-shaped, so an opaque webhook or - # artifact POST is untouched. A caller who splits `/api/` and the hostname - # as well does get through; that is no longer a mistake anyone makes by - # accident, and this hook stops mistakes. It is not a sandbox, and pretending - # otherwise is how you get a control nobody can trust the boundaries of. - if [ -z "$endpoint" ] \ - && printf '%s' "$CMD" | grep -Eq 'https?://[^[:space:]"'"'"'|;&)]*[$`]' \ - && printf '%s' "$CMD" | grep -Eq '/api/v[0-9]|/repos/|git\.|gitea|github\.com|gitlab|forgejo'; then - cat <