feat(credentials): select GH_TOKEN by GitHub owner of launch directory - #123
Merged
Merged
Conversation
A fine-grained PAT is bound to exactly one resource owner at creation and cannot be repointed afterward. The 2026-09 org migration moved repos out from under the single personal-owner token, so gh lost access to every org repo while SSH-based git kept working — which is why the regression survived ~10 days unnoticed. Derive the owner from the launch directory's origin remote and select the matching vault item: smartwatermelon -> op://Automation/CCCLI-SWM/token nightowlstudiollc -> op://Automation/CCCLI-NOS/token otherwise -> op://Automation/GitHub - CCCLI/Token One op read per launch; the token is selected, not accumulated. Unknown or underivable owners fall back to the personal token, preserving pre-migration behavior for sessions launched outside a repo. Owner parsing uses prefix matching rather than the sed substitution sketched in #120. A non-matching sed expression passes its input through unchanged, so a non-GitHub remote yielded the entire URL as the "owner" — harmless at the case statement but wrong in the debug log. Explicit prefixes also handle ssh://git@github.com/owner/repo, which the scp-style pattern alone misses. Verified end-to-end through bin/claude-wrapper itself, not just a sourced library, with GH_TOKEN unset: claude-wrapper -> smartwatermelon -> CCCLI-SWM amelia-boone -> nightowlstudiollc -> CCCLI-NOS ~ -> <none> -> personal Both new tokens were verified against live GitHub before this change: org repo access (NOS private repos 0 -> 10), Administration:read (the branch-protection 403 that blocked the #122 merge), Actions:read, Pull requests:read, and Issues:write (scratch comment posted and deleted on each owner). Adds 10 tests covering both org owners, the personal owner, https/scp/ssh:// remote forms, unknown owners, non-GitHub remotes, a repo without an origin, a non-repo directory, and a subdirectory of a repo. All ten were confirmed to fail against broken code before being accepted. Also fixes make_stub_dir, a shared test helper: it used `command -v` to locate binaries, which returns the NAME rather than a path when the caller's shell has a function of that name. `git` is a function in this environment, so the helper created a broken symlink and git appeared absent inside every stub that requested it. Switched to `type -P`, which searches PATH only. This changes behavior for every test using that helper, not only the new ones. Docs: CLAUDE.md and README.md described a single hardcoded token ref, and README explicitly claimed the wrapper "does not perform per-org token routing", which this change makes false. Both now document the selection table. Also corrects two stale claims in CLAUDE.md's credentials.sh entry that #122 had already disproved: that the credentials are absent from child environments, and that a failed fetch falls back to the keyring. test-gh-token-permissions.sh was deliberately not run: it mutates a live sandbox repo. test-credentials 24/24, test-launch-dir-check 6/6, test-remote-session 26/26, test-wrapper 64/64, shellcheck -S info clean Closes #120 Claude-Session: https://claude.ai/code/session_011r87TBgqE338FGJn5zpZ52
This comment has been minimized.
This comment has been minimized.
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.
Problem
A fine-grained PAT is bound to exactly one GitHub resource owner at creation
and cannot be repointed. The 2026-09 org migration moved repos out from under
the single personal-owner token, so
ghlost access to every org repo — whileSSH-based git kept working, which is why it went unnoticed for ~10 days.
This is step 2 of the plan in #120. Step 1 (fail closed on vault failure)
shipped in #122.
Fix
Derive the GitHub owner from the launch directory's
originremote, and selectthe matching vault item:
smartwatermelonop://Automation/CCCLI-SWM/tokennightowlstudiollcop://Automation/CCCLI-NOS/tokenop://Automation/GitHub - CCCLI/TokenOne
op readper launch — the token is selected, not accumulated. Unknown orunderivable owners fall back to the personal token, preserving the previous
behavior for sessions launched outside a repo.
Deviation from the sketch in #120
The issue proposed a
sedsubstitution to extract the owner. A non-matchingsedpasses its input through unchanged, so a non-GitHub remote returned theentire URL as the owner:
That still lands in the
*)fallback, so the token choice was right byaccident — but a URL would have been printed where an owner name belongs.
Replaced with explicit prefix matching plus a charset check. This also fixes
ssh://git@github.com/owner/repo, a valid remote form the scp-style patterndoes not match.
Token verification (live, before this change)
Both new PATs were checked against real repos:
CCCLI-SWMCCCLI-NOSAdministration: readis the permission whose absence produced the HTTP 403that made the pre-merge hook treat every check as blocking during #122.
Issues: writeis what blocked posting a comment to #120 from this session.Both scratch comments were deleted and verified 404.
End-to-end verification
Through
bin/claude-wrapperitself — not a sourced library — withGH_TOKENunset:
And the selected tokens do real work: from
claude-wrapper, branch protectionreads
["claude-review / run-review"]; fromamelia-boone, 10 private NOSrepos are visible.
Tests
10 new cases: both org owners, the personal owner, https/scp/
ssh://remoteforms, unknown owners, non-GitHub remotes, a repo with no
origin, a non-repodirectory, and a subdirectory of a repo. Every one was confirmed to fail
against broken code before being accepted — they failed twice during
development, for two different real reasons.
test-credentials.sh24/24test-launch-dir-check.sh6/6test-remote-session.sh26/26test-wrapper.sh64/64shellcheck -S infocleantest-gh-token-permissions.shwas deliberately not run: it mutates a livesandbox repo.
Also in this PR
Shared test-helper bugfix.
make_stub_dirusedcommand -vto locatebinaries, which returns the name rather than a path when the caller's shell
has a function of that name.
gitis a function in this environment, so thehelper created a broken symlink and
gitappeared absent inside every stubthat requested it. Switched to
type -P. This changes behavior for every testusing the helper, not only the new ones.
Docs.
CLAUDE.mdandREADME.mdboth documented a single hardcoded ref,and README explicitly claimed the wrapper "does not perform per-org token
routing" — which this makes false. Also corrects two stale claims in CLAUDE.md
that #122 had already disproved: that these credentials are absent from child
process environments, and that a failed fetch falls back to the keyring.
Follow-up
The GitHub App migration (#120's "alternative considered") is deliberately out
of scope — the issue describes it as its own project. Filed separately so it is
not lost with #120 closed.
Closes #120
https://claude.ai/code/session_011r87TBgqE338FGJn5zpZ52