Repository navigation
fix(gh-wrapper): read the gh api endpoint past value-taking flags - #442
Merged
Merged
Conversation
added 2 commits
October 19, 2026 15:59
The approval gate's argument walk treated every -* argument as a flag with no value. In `gh api -X POST repos/o/r/pulls/5/comments`, POST became the endpoint, so: - _gh_wrapper_cap_kind gave pr-comment (140) instead of line-comment (280), and blocked an approved inline comment. - _gh_wrapper_gate_destination found no repos/ endpoint, so the body was routed by $PWD alone. From an exempt checkout it passed with no check, even when the endpoint named a gated repository. Reproduced in the suite before this fix. After `api`, the walk now skips the value of each value-taking gh api flag (-X/--method, -H/--header, -f/--raw-field, -F/--field, -q/--jq, -t/--template, --input, -p/--preview, --cache, --hostname). The flag list is shared with _gh_wrapper_api_body_fields through a new _gh_wrapper_api_value_flag, so the two walks cannot drift. It also counts the positionals after `api`. gh api takes exactly one. None, or more than one (an unknown flag left its value behind), now blocks with "cannot find the gh api endpoint" instead of routing by $PWD. Endpoints with no repos/ part (gists, user) still route by the checkout, as the claude-config hook does. Why this and not the hook's regex (first word matching repos/ or graphql): the regex cannot tell "not a repo endpoint" from "endpoint not found", and a flag value such as an --input path under /repos/ could match it. The skip walk yields gh's own endpoint, and the positional count catches a flag this walk does not know. Advances #383 (its _gh_wrapper_cap_kind item). Closes #434
The api_pos check blocks on zero or any count above one, not only two. The approval-gate walk header now says where it differs from the force-draft walk, and why that walk does not need the change. Advances #434
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The approval gate's argument walk in
bash/gh-wrapper.shtreated every-*argument as a flag with no value. Ingh api -X POST repos/o/r/pulls/5/comments,POSTbecame the endpoint. That caused two faults:_gh_wrapper_cap_kindreturnedpr-comment(140) instead ofline-comment(280)._gh_wrapper_gate_destinationfound norepos/endpoint, so the body routed by$PWDalone. From an exempt checkout, a body for a gated repo passed with no check. The issue had not reproduced this in isolation; the new suite case reproduced it on unmodified code (rc=0 where 1 was expected).Change
api, the walk skips the value of each value-takinggh apiflag:-X/--method,-H/--header,-f/--raw-field,-F/--field,-q/--jq,-t/--template,--input,-p/--preview,--cache,--hostname. The=and attached spellings are one argv word already. The new_gh_wrapper_api_value_flagholds the list, and_gh_wrapper_api_body_fieldsuses it too, so the two walks cannot drift. It is in theexport -flist.api.gh apitakes exactly one. Zero, or more than one (an unknown flag left its value behind), now blocks with "cannot find the gh api endpoint" and does not route by$PWD.repos/part (gists,user) still route by the checkout. The claude-config hook does the same.Why not the hook's regex
The hook takes the first word that matches
^(https?://[^/]+)?/?repos/orgraphql. That regex cannot tell "not a repo endpoint" from "endpoint not found". Also, a flag value such as an--inputpath under/repos/could match it. The skip walk gives the endpoint that gh itself uses, and the positional count catches a flag that the walk does not know.Tests
New cases in
bash/tests/test-gh-wrapper-approval-gate.sh. Before the fix, 9 of them failed on unmodified code:line-commentfor-X POST,-H 'Accept: x'and-f x=ybeforerepos/o/r/pulls/5/comments-X POST repos/beacon-biosignals/x/...from a visual checkout, no record: blocked-X POST repos/smartwatermelon/x/...from a gated checkout: passes-X POST repos/nightowlstudiollc/x/...from an exempt checkout: blocked (the Effect 2 case)-H 'Accept: x' repos/nightowlstudiollc/x/issues/1/commentsfrom an exempt checkout: blockedRegression guards that passed before and after:
--method=POSTfor kind and for route, andrepos/{owner}/{repo}from an exempt checkout (passes).All 11
test-gh-wrapper-*.shfiles pass.shellcheck -S inforeports no new findings compared withorigin/main.shfmt -dis clean.Advances #383 (its
_gh_wrapper_cap_kinditem).Closes #434