feat(gh-wrapper): org-migration identity guard (F3) and token-scope escape hatch (F4) - #306
Merged
twistedmelonman merged 7 commits intoSep 4, 2026
Conversation
added 7 commits
September 3, 2026 13:27
Prepares for the 2026-09 org migration: the smartwatermelon user is renamed to twistedmelonman and the name is re-claimed as an org. The owner table now maps smartwatermelon, nightowlstudiollc, and twistedmelonman to the person, and the force-draft in-org list gains twistedmelonman. A single dated alias (twistedmelonman=smartwatermelon) keeps every wrapped call working between this change landing and the rename. It is removed in a follow-up once all machines have re-logged in. Claude-Session: https://claude.ai/code/session_01RUgidKkV54aNnH1rRNfUq6
The alias suppressed the identity COMPARISON but not the switch TARGET. Pre-rename the keyring holds smartwatermelon, not twistedmelonman, so a personal repo resolving desired=twistedmelonman made the wrapper run `gh auth switch --user twistedmelonman` against an account gh does not have. That hard-failed on the first personal-repo call after any Beacon-repo call — precisely the window the alias exists to cover, on all three machines. Resolve the target against the logins the keyring actually holds: use `desired` when present, else the first held login the alias table equates to it, else `desired` unchanged so the caller still fails closed. No reverse alias is introduced. _gh_wrapper_keyring_users parses the `users:` block by indent depth so a login's own nested settings (oauth_token:) are not mistaken for additional accounts, and so a second host cannot leak into the list. Claude-Session: https://claude.ai/code/session_01RUgidKkV54aNnH1rRNfUq6
GH_TOKEN and the keyring token are the same login; only the scopes differ. When gh fails with its own "needs the ... scope" text, the standalone wrapper now prints the exact re-run command with GH_TOKEN unset (or `gh auth refresh -s <scope>` when no GH_TOKEN is set). stderr is mirrored live through tee; stdout and the exit code pass through. Detection keys off gh's ScopesSuggestion output rather than classifying commands, so there is no list of scope-needing subcommands to keep current. The quote character is not pinned: gh has emitted both '...' and "..." across versions, and matching one silently disables the hint. The final exec becomes a run-then-exit, because the hint needs gh's exit status and stderr after it returns. stdout is routed around the tee pipe on fd 3, so gh's TTY detection there -- which drives color, paging and prompts -- is unchanged; only stderr becomes a pipe. Verified: stdin, interactive stderr prompts, live (unbuffered) stderr, exit-code fidelity and temp-file cleanup all hold. Claude-Session: https://claude.ai/code/session_01RUgidKkV54aNnH1rRNfUq6
…e signals in the scope hint Four findings from the Task 2 review of a383a5c: - The hint re-quoted argv verbatim, so `gh secret set FOO --body <token>` echoed the token into stderr (scrollback, CI logs, transcripts). Values following -b/--body, -f/--field, -F/--raw-field, -H/--header, in both the `--flag VALUE` and `--flag=VALUE` spellings, now print as <redacted>. Redaction is by flag, not by value pattern, so it cannot fail open on an unanticipated token format. - gh reads GH_TOKEN then GITHUB_TOKEN. When only GITHUB_TOKEN was set the hint told the user to `gh auth refresh`, which rewrites a keyring token the env var overrides. The hint now names whichever variable is set. - A SIGTERM/SIGHUP mid-run skipped the trailing rm -f and left one temp file per interrupted run; a RETURN trap alone does not fire on signal death (measured). An explicit TERM/HUP/INT trap cleans up and re-raises. - The test clears GH_HOST and GITHUB_TOKEN as well as GH_TOKEN, and sandboxes TMPDIR so the leak assertions see only this run's files. New assertions (redaction x4, GITHUB_TOKEN x2, SIGTERM leak x2) all fail against a383a5c and pass here. Claude-Session: https://claude.ai/code/session_01B7KFdvsQq7eLUGEi4pRQmX
The first redaction pass matched flags as exact tokens, so pflag's stuck short form (`-bSECRET`, `-fkey=SECRET`) fell through to the verbatim branch and printed the secret. Handle all three spellings pflag accepts. Also from the re-review of c695838: - Pair the flags as gh defines them (-F/--field, -f/--raw-field). - For key=value flags keep the key and redact only the value, so the suggested command still says which field carried the secret. - Stop flag parsing at a literal `--`, matching the file's other scanners. Three new assertions fail against c695838 and pass here; the `--` case guards a path that did not leak before but had no test. Claude-Session: https://claude.ai/code/session_01B7KFdvsQq7eLUGEi4pRQmX
…ground so signals reach it Whole-branch review found three findings with one root cause: the stderr tee pipe interposed on every gh call, costing gh its stderr TTY, reordering stderr against stdout, and orphaning gh on a targeted SIGTERM. - Standalone mode now execs the real gh unless GH_TOKEN or GITHUB_TOKEN is set; the hint only matters when an env token overrides the keyring. - _gh_wrapper_run_with_scope_hint runs gh in the background and `wait`s. Measured: bash defers a trapped signal behind a foreground pipeline, so the old handler could neither forward nor clean up until gh exited on its own. `wait` returns at once, the handler kills gh, removes the temp file, and re-raises. Explicit `<&0` keeps stdin; INT is forwarded as TERM because background children start with SIGINT ignored. - stderr goes through `exec 4> >(tee ...)`; the fd is closed and tee waited for before the file is read, so no partial-file race. - Hint names whichever env var is set; the `gh auth refresh` branch is gone (unreachable once capture is gated). - Tests: no-token path asserts exec via parent-process check and untouched stderr; SIGTERM case asserts the wrapper dies within 3s (a plain `wait` passed 30s late against 42a5502), no temp file leak, and no orphaned gh. All five new assertions fail against 42a5502. Identity test unsets GITHUB_TOKEN too, so an ambient token cannot flip the exec path. Claude-Session: https://claude.ai/code/session_01B7KFdvsQq7eLUGEi4pRQmX
…apture Re-review of f27a340 found two real gaps: - The capture gate was "env token set", but the rationale is "an agent session cannot act on gh's own message". A human with GH_TOKEN exported in an interactive shell took the capture path on every call and lost gh's stderr TTY — the fd `gh auth login` and `gh pr create` render their prompts on. The gate is now: exec unless an env token is set AND stderr is not a terminal. New test allocates a pty via BSD script(1) and asserts the stub sees a stderr tty with GH_TOKEN set; fails against f27a340. - `exec 4> >(tee ...)` used a fixed fd. The function is exported, so a repeat call in one shell would clobber a still-open fd 4 and orphan the first tee. Now `exec {errfd}> >(...)` with the fd closed for gh and after wait. Verified: two sourced calls leave no extra fds and no tee children. Comment fix: `<&0` is defensive — only a job-control shell gives a background child /dev/null as stdin, and this path is non-interactive. Claude-Session: https://claude.ai/code/session_01B7KFdvsQq7eLUGEi4pRQmX
|
Scope: Signal handling in F4, identity resolution and alias logic in F3, secret redaction on hint output. F3: Identity resolution with login aliases
F4: Signal handling in background stderr capture
Secret redaction
Exit codes
No bugs, regressions, security issues, or data-loss risks identified. VERDICT: PASS |
twistedmelonman
deleted the
claude/feat-gh-wrapper-org-migration-01RUgidK
branch
September 4, 2026 02:45
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
Plan tasks 1–2 of the org migration (spec + plan: smartwatermelon/dev-env#80, tooling: smartwatermelon/dev-env#81).
twistedmelonman(the post-rename login), withsmartwatermelonkept as a temporary alias until the GitHub account is renamed; the alias is removed by plan Task 12. Auth-switch target resolves through the alias.GH_TOKEN, orGITHUB_TOKENas gh's fallback) lacks a scope and gh says so, the wrapper prints one pasteable escape hatch:env -u GH_TOKEN gh <argv>. Secret-bearing flag values (-b/--body,-f/-F/--field/--raw-field,-H/--header, stuck short forms,--flag=value) print as<redacted>;--is honored.Capture design (F4)
stderr is captured only when an env token is set and stderr is not a terminal. Otherwise the wrapper execs gh as before, so a human with
GH_TOKENexported keeps gh's stderr TTY (gh auth login/gh pr createrender prompts there). In the capture path gh runs in the background and the wrapperwaits — measured: bash defers a trapped signal behind a foreground pipeline, so onlywaitlets the handler forward SIGTERM to gh, remove the temp file, and re-raise. stderr goes throughexec {errfd}> >(tee ...); the fd is closed and tee waited for before the file is read.Verification
bash/tests/test-gh-wrapper-identity.sh,bash/tests/test-gh-wrapper-scope-hint.sh(26 assertions incl. redaction, exec-vs-resident via parent-process check, pty via BSDscript(1), SIGTERM within 3s / no temp-file leak / no orphaned gh)Parked
wait "${tee_pid}" || truehides a dead tee (worst case: no hint, gh's own message still prints)https://claude.ai/code/session_01B7KFdvsQq7eLUGEi4pRQmX