diff --git a/bash/gh-wrapper.sh b/bash/gh-wrapper.sh index eb2f36a..57d8b8b 100755 --- a/bash/gh-wrapper.sh +++ b/bash/gh-wrapper.sh @@ -179,21 +179,107 @@ _gh_wrapper_is_beacon_context() { return 1 } +# TEMPORARY until the 2026-09 rename lands (dev-env +# docs/superpowers/specs/2026-09-03-org-migration-design.md, Step 2). Remove +# in the Step 6 follow-up together with every test case that names it. Both +# logins are the same person: the personal account is renamed from +# smartwatermelon to twistedmelonman, and until that happens every token +# still reports the old name. Format: desired=alias[,desired=alias...]. +_GH_WRAPPER_LOGIN_ALIASES="${_GH_WRAPPER_LOGIN_ALIASES:-twistedmelonman=smartwatermelon}" + +# True when `actual` is `desired` or one of desired's aliases, case- +# insensitively. One-directional: an alias never stands in for its own +# desired value as a `desired` argument. +_gh_wrapper_logins_equal() { + local desired="${1,,}" actual="${2,,}" + [[ "${desired}" == "${actual}" ]] && return 0 + local IFS=',' + local pair + for pair in ${_GH_WRAPPER_LOGIN_ALIASES}; do + pair="${pair,,}" + if [[ "${pair%%=*}" == "${desired}" && "${pair#*=}" == "${actual}" ]]; then + return 0 + fi + done + return 1 +} + +# The keyring login gh will use: the `user:` under `github.com:` in hosts.yml. +# Empty when no host entry exists. +_gh_wrapper_keyring_login() { + awk '/^github\.com:/{f=1} f && /^ *user:/{print $2; exit}' "${HOME}/.config/gh/hosts.yml" 2>/dev/null | tr -d "\"'" +} + +# Every login the keyring holds for github.com, one per line: the keys of the +# `users:` block. Those keys sit one indent level deeper than `users:` itself, +# which is what separates them from siblings like `user:` and `git_protocol:`. +# Empty when the block is absent (older hosts.yml files omit it). +_gh_wrapper_keyring_users() { + awk ' + /^github\.com:/ { host = 1; next } + /^[^[:space:]]/ { host = 0; users = 0; login_indent = 0; next } + host && match($0, /^[[:space:]]*users:[[:space:]]*$/) { + users = 1 + login_indent = 0 + users_indent = index($0, "u") - 1 + next + } + users && match($0, /^[[:space:]]*[^[:space:]#][^:]*:/) { + indent = match($0, /[^[:space:]]/) - 1 + if (indent <= users_indent) { users = 0; next } + # A login key sits at the first indent level inside the block. Anything + # deeper is that login`s own settings (oauth_token:, git_protocol:), not + # another account. + if (login_indent == 0) { login_indent = indent } + if (indent != login_indent) { next } + key = $0 + sub(/^[[:space:]]*/, "", key) + sub(/:.*$/, "", key) + print key + } + ' "${HOME}/.config/gh/hosts.yml" 2>/dev/null | tr -d "\"'" +} + +# The login to hand `gh auth switch`. `desired` may not exist in the keyring +# yet — during the rename window the account is still named by its alias — and +# switching to an account gh does not have fails outright. Prefer `desired` +# when held, else the first held login the alias table equates to it, else +# `desired` unchanged so the caller still fails closed with its own message. +_gh_wrapper_resolve_switch_target() { + local desired="$1" + local held_logins + held_logins="$(_gh_wrapper_keyring_users)" || held_logins="" + + local held alias_match="" + while IFS= read -r held; do + [[ -z "${held}" ]] && continue + if [[ "${held,,}" == "${desired,,}" ]]; then + printf '%s' "${held}" + return 0 + fi + if [[ -z "${alias_match}" ]] && _gh_wrapper_logins_equal "${desired}" "${held}"; then + alias_match="${held}" + fi + done <<<"${held_logins}" + + printf '%s' "${alias_match:-${desired}}" +} + # gh has one active account per host (not per repo), unlike git+SSH which # already resolves the right identity per remote via ~/.ssh/config host # aliases. This keeps gh in sync with that same per-repo intent. # # Mapping, in precedence order: # 1. Owners explicitly claimed by an identity win outright, in BOTH -# directions — smartwatermelon/nightowlstudiollc -> smartwatermelon, -# beacon-biosignals/andrewmrich -> andrewmrich. An explicitly-owned repo -# means the same thing no matter which directory you invoke gh from, -# preserving the cwd-independence established in +# directions — smartwatermelon/nightowlstudiollc/twistedmelonman -> +# twistedmelonman, beacon-biosignals/andrewmrich -> andrewmrich. An +# explicitly-owned repo means the same thing no matter which directory +# you invoke gh from, preserving the cwd-independence established in # smartwatermelon/dotfiles#135. # 2. Otherwise (an owner claimed by neither — a third-party org, an # upstream you've been added to), consult the Beacon-context heuristic: # checkout under the beacon dir, or forked from beacon-biosignals. -# 3. Otherwise, default to smartwatermelon. This is the personal-default +# 3. Otherwise, default to twistedmelonman. This is the personal-default # environment; Beacon work is the specifically-marked exception. # # Local-only (reads/writes gh's config file, no network), so it's cheap to @@ -206,20 +292,22 @@ _gh_wrapper_sync_identity() { owner="$(_gh_wrapper_resolve_owner "$@")" [[ -z "${owner}" ]] && return 0 - # NOTE: the nightowlstudiollc -> smartwatermelon mapping below is asserted, - # not verified — nothing here confirms the smartwatermelon gh account is - # actually authorized against nightowlstudiollc repos. A `gh auth status` - # check (cross-referencing the authorized orgs for the current account) - # would be the way to confirm this mapping is still correct; that's left - # as a future enhancement rather than added here to avoid scope creep. + # smartwatermelon is the ORG (2026-09 migration); nightowlstudiollc is the + # other org; twistedmelonman is the personal account that owns both and + # keeps the archived repos and forks. All three resolve to the person. + # + # Still asserted, not verified: nothing here confirms the twistedmelonman + # gh account is actually authorized against either org's repos. A `gh auth + # status` check cross-referencing the account's authorized orgs would + # confirm it; left as a future enhancement rather than scope creep here. case "${owner,,}" in - smartwatermelon | nightowlstudiollc) desired="smartwatermelon" ;; + smartwatermelon | nightowlstudiollc | twistedmelonman) desired="twistedmelonman" ;; beacon-biosignals | andrewmrich) desired="andrewmrich" ;; *) if _gh_wrapper_is_beacon_context; then desired="andrewmrich" else - desired="smartwatermelon" + desired="twistedmelonman" fi ;; esac @@ -275,7 +363,7 @@ _gh_wrapper_sync_identity() { return 1 fi - if [[ "${token_login,,}" != "${desired,,}" ]]; then + if ! _gh_wrapper_logins_equal "${desired}" "${token_login}"; then echo "[gh] ERROR: GH_TOKEN authenticates as '${token_login}' but repo owner '${owner}' requires '${desired}'" >&2 echo "[gh] GH_TOKEN takes precedence over 'gh auth switch', so this would" >&2 echo "[gh] run as the wrong identity. Failing closed." >&2 @@ -284,12 +372,17 @@ _gh_wrapper_sync_identity() { fi fi - current=$(awk '/^github\.com:/{f=1} f && /^ *user:/{print $2; exit}' "${HOME}/.config/gh/hosts.yml" 2>/dev/null | tr -d "\"'") - - if [[ -n "${current}" && "${current}" != "${desired}" ]]; then - if ! command gh auth switch --hostname github.com --user "${desired}" >/dev/null 2>&1; then - echo "[gh] ERROR: failed to switch identity to '${desired}' (repo owner: '${owner}') — refusing to run as '${current}' instead" >&2 - echo "[gh] If '${desired}' is not authenticated on this machine, run: gh auth login --hostname github.com" >&2 + current="$(_gh_wrapper_keyring_login)" + + if [[ -n "${current}" ]] && ! _gh_wrapper_logins_equal "${desired}" "${current}"; then + # Not `desired` verbatim: during the rename window the keyring still holds + # the pre-rename login, and `gh auth switch` to an account it does not have + # fails. Resolve to a login gh actually holds. + local target + target="$(_gh_wrapper_resolve_switch_target "${desired}")" + if ! command gh auth switch --hostname github.com --user "${target}" >/dev/null 2>&1; then + echo "[gh] ERROR: failed to switch identity to '${target}' (repo owner: '${owner}') — refusing to run as '${current}' instead" >&2 + echo "[gh] If '${target}' is not authenticated on this machine, run: gh auth login --hostname github.com" >&2 echo "[gh] Failing closed rather than acting on '${owner}' as the wrong identity." >&2 return 1 fi @@ -452,7 +545,7 @@ _gh_wrapper_force_draft_for_off_org() { owner="$(_gh_wrapper_resolve_owner "$@")" if [[ -n "${owner}" ]]; then case "${owner,,}" in - smartwatermelon | nightowlstudiollc) ;; # in-org: no change + smartwatermelon | nightowlstudiollc | twistedmelonman) ;; # in-org: no change *) # Off-org target: force --draft. Don't bother deduplicating if the # caller already passed --draft (or --draft=false, which gh doesn't @@ -473,6 +566,209 @@ _gh_wrapper_force_draft_for_off_org() { return 0 } +# --- F4: scope-error hint ------------------------------------------------------ +# GH_TOKEN (the CCCLI PAT) and the keyring token are the same login; only the +# scopes differ. When gh fails because the active token lacks a scope, say +# exactly how to re-run the one command with the other token. Detection is +# gh's own ScopesSuggestion text, so there is no command classifier to get +# wrong. Design: dev-env docs/superpowers/specs/2026-09-03-org-migration-design.md. +# +# Never widen the PAT: it is exported into every session, so a scope added +# there applies to every call rather than the one that needed it. + +# Print the scope named in a captured stderr file, or nothing. The character +# class accepts either quote style, so the hint keeps working whichever one gh +# emits; pinning a single one would silently disable it if that ever changed. +# Measured against gh 2.x (2026-09), which uses "...". +_gh_wrapper_scope_from_file() { + grep -oE "needs the [\"'][A-Za-z0-9:_]+[\"'] scope" "$1" 2>/dev/null \ + | head -1 | sed -E "s/needs the [\"']([^\"']+)[\"'] scope/\1/" +} + +# Re-quote argv for display, replacing the VALUE of any flag that routinely +# carries a secret with . The hint is printed to stderr, which lands +# in terminal scrollback, CI logs and transcripts — echoing `--body ghp_...` +# back would copy a live credential into all three. Redaction is by flag name, +# not by pattern-matching the value: guessing what a secret looks like fails +# open on every format not anticipated, whereas the flag says outright that +# whatever follows it is a value the caller chose to pass secretly. +# +# Flags covered (gh's real pairings: -b/--body, -F/--field, -f/--raw-field, +# -H/--header) in all three spellings pflag accepts: `--body VALUE` and +# `-b VALUE` (value in the next argument), `--body=VALUE` (value after `=`), +# and the stuck short form `-bVALUE` (value glued to the flag letter). A +# literal `--` ends flag parsing, as in the other argv scanners in this file. +# +# For the key=value flags (-f/-F/--field/--raw-field) only the part after the +# first `=` is redacted, so `-f body=SECRET` prints as `-f body=`: +# the key names the API field and is not secret, and keeping it leaves the +# suggested command recognizable. A value with no `=` is redacted whole. +_gh_wrapper_redact_argv() { + local out="" arg redact_next="" past_dashdash=0 + for arg in "$@"; do + if [[ -n "${redact_next}" ]]; then + out+="$(_gh_wrapper_redact_value "${redact_next}" "${arg}") " + redact_next="" + continue + fi + if [[ "${past_dashdash}" == "1" ]]; then + out+="$(printf '%q ' "${arg}")" + continue + fi + case "${arg}" in + --) + past_dashdash=1 + out+="-- " + ;; + -b | --body | -H | --header) + redact_next=whole + out+="$(printf '%q ' "${arg}")" + ;; + -f | -F | --field | --raw-field) + redact_next=keyed + out+="$(printf '%q ' "${arg}")" + ;; + --body=* | --header=*) + # %q with no trailing space: the `=` must abut the flag name, and + # `printf '%q '` would wedge a space in between. + out+="$(printf '%q' "${arg%%=*}")=$(_gh_wrapper_redact_value whole "${arg#*=}") " + ;; + --field=* | --raw-field=*) + out+="$(printf '%q' "${arg%%=*}")=$(_gh_wrapper_redact_value keyed "${arg#*=}") " + ;; + -b?* | -H?*) + out+="${arg:0:2}$(_gh_wrapper_redact_value whole "${arg:2}") " + ;; + -f?* | -F?*) + out+="${arg:0:2}$(_gh_wrapper_redact_value keyed "${arg:2}") " + ;; + *) out+="$(printf '%q ' "${arg}")" ;; + esac + done + printf '%s' "${out% }" +} + +# Redact one flag value. mode=whole replaces all of it; mode=keyed keeps a +# leading `key=` and replaces what follows (or the whole value if it has no +# `=`). The kept key is re-quoted so the result stays pasteable. +_gh_wrapper_redact_value() { + local mode="$1" value="$2" + if [[ "${mode}" == "keyed" && "${value}" == *=* ]]; then + printf '%q=' "${value%%=*}" + else + printf '' + fi +} + +# Print the hint for `scope`, quoting the original argv back so the suggested +# command can be pasted verbatim (secret-bearing flag values excepted — see +# _gh_wrapper_redact_argv). +_gh_wrapper_print_scope_hint() { + local scope="$1" + shift + local cmd + cmd="$(_gh_wrapper_redact_argv "$@")" + # gh reads GH_TOKEN first, then GITHUB_TOKEN. Name whichever is actually set: + # telling someone to unset GH_TOKEN when GITHUB_TOKEN is what authenticated + # them is advice that cannot work, and `gh auth refresh` is equally useless + # here — it rewrites the keyring token, which an env-var token overrides. + # The caller only reaches this function when one of the two is set. + local token_var="GH_TOKEN" + if [[ -z "${GH_TOKEN:-}" ]]; then + token_var="GITHUB_TOKEN" + fi + local keyring + keyring="$(_gh_wrapper_keyring_login)" + # The login stays on the same line as its label: a reader grepping the + # hint for the account name should find it next to the word naming it, + # not wrapped onto the following line. + echo "[gh] ${token_var} is set and lacks the '${scope}' scope." >&2 + echo "[gh] The keyring identity for ${keyring:-} has it." >&2 + echo "[gh] Re-run this one command without ${token_var}:" >&2 + echo "[gh] env -u ${token_var} gh ${cmd}" >&2 + echo "[gh] (Do not add the scope to the CCCLI PAT — it is exported into every session.)" >&2 +} + +# Run the real gh. stderr goes to the terminal live AND to a temp file; stdout +# is untouched. On non-zero exit, a scope error in the file triggers the hint. +# The real exit code is returned. +# +# Not `exec`: the hint needs gh's exit status and stderr after it returns. +# Only called when an env-var token is set (see the standalone branch): the +# tee pipe costs gh's stderr its TTY and can reorder stderr against stdout, +# which is acceptable in an agent session and not at a human's terminal. +_gh_wrapper_run_with_scope_hint() { + local real_gh="$1" + shift + local errfile rc=0 + # No temp file means no detection, but the command itself must still run — + # degrade to a plain passthrough rather than failing the call. + if ! errfile="$(mktemp "${TMPDIR:-/tmp}/gh-wrapper-stderr.XXXXXX")"; then + "${real_gh}" "$@" + return $? + fi + # A signal that kills the wrapper mid-run skips the rm -f at the bottom and + # leaves one temp file behind per interrupted run. + # + # RETURN alone does NOT fix that: measured, a RETURN trap does not fire when + # the shell is killed by SIGTERM — the process dies without unwinding the + # function. An explicit signal trap is required. RETURN is kept for the + # ordinary paths (including the `return` below), and the signal handler + # re-raises after cleanup so the caller still sees a signal death (128+n) + # rather than a normal exit. + # + # The handler also forwards the signal to gh. Before F4 the wrapper exec'd + # gh, so a signal aimed at "the gh process" hit gh. Now that pid is the + # wrapper's; without forwarding, gh would run on as an orphan after the + # wrapper died. Ctrl-C already reaches the whole foreground process group, + # so this matters for a targeted kill (`timeout gh ...`, `kill `). + # + # gh runs in the BACKGROUND and the wrapper `wait`s for it. This is not + # optional: bash defers a trapped signal until a foreground command + # finishes (measured — a SIGTERM to the wrapper during a foreground + # `gh | tee` pipeline was not acted on until gh exited on its own, so the + # handler could neither forward nor clean up). Only `wait` returns at once + # on a trapped signal, running the handler immediately. The explicit `<&0` + # is defensive: only a job-control (interactive) shell gives a background + # child /dev/null as stdin, and this path runs non-interactively — but the + # function is exported, so it is kept correct for a sourced caller too. + # Non-interactive bash starts background children with SIGINT ignored (Go + # leaves an ignored signal ignored), so INT is forwarded as TERM. `kill` is + # guarded because `set -e` applies inside the handler. + # + # stderr goes through a process substitution running tee: gh writes to a + # dynamically allocated fd ({errfd}, so a repeat call in one shell cannot + # clobber a still-open fd and orphan the previous tee), tee copies to the + # file and the real stderr. gh gets the fd closed so it does not inherit + # the write end. After gh exits the fd is closed and tee is waited for, so + # the file is complete before it is read. tee buffers, so a PARTIAL-line + # write to stderr — gh's interactive prompts, which deliberately omit the + # trailing newline — can surface after stdout that was written later. + # Whole lines are unaffected, and stdout bypasses the pipe entirely. + local gh_pid="" tee_pid="" errfd="" + trap 'rm -f "${errfile}"' RETURN + trap '[[ -n "${gh_pid}" ]] && kill -TERM "${gh_pid}" 2>/dev/null; rm -f "${errfile}"; trap - TERM HUP INT; kill -s TERM $$' TERM HUP INT + exec {errfd}> >(tee "${errfile}" >&2 || true) + tee_pid=$! + "${real_gh}" "$@" 2>&"${errfd}" {errfd}>&- <&0 & + gh_pid=$! + # `|| rc=$?` keeps `set -e` (standalone mode) from aborting on a failing gh + # before rc is read. + wait "${gh_pid}" || rc=$? + gh_pid="" + exec {errfd}>&- + wait "${tee_pid}" 2>/dev/null || true + if [[ "${rc}" -ne 0 ]]; then + local scope + scope="$(_gh_wrapper_scope_from_file "${errfile}")" + if [[ -n "${scope}" ]]; then + _gh_wrapper_print_scope_hint "${scope}" "$@" + fi + fi + rm -f "${errfile}" + return "${rc}" +} + if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then # --- Standalone-wrapper mode (executed directly, e.g. via the # ~/.local/bin/gh symlink) --- @@ -530,15 +826,15 @@ if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then fi fi - # Only needed for the final exec below — computed here (after the + # Only needed for the final gh invocation below — computed here (after the # _GH_REVIEW_DONE-guarded checks above) rather than unconditionally at the # top of this block, so we don't do a needless PATH scan before knowing # this call is going to pass those checks. REAL_GH="$(_gh_wrapper_find_real_gh)" # Defensive: _gh_wrapper_find_real_gh currently fails hard on lookup # failure (and set -e aborts the assignment), but if it ever returns 0 - # with empty output we'd otherwise exec "" "$@" and produce a confusing - # low-level exec error. Check explicitly instead. + # with empty output we'd otherwise run "" "$@" and produce a confusing + # low-level "command not found" error. Check explicitly instead. if [[ -z "${REAL_GH}" ]]; then echo "[gh] ERROR: could not locate real gh binary on PATH" >&2 exit 1 @@ -550,7 +846,27 @@ if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then source "${CLAUDE_GH_TOKEN_ROUTER}" "$@" fi - exec "${REAL_GH}" "$@" + # The F4 scope hint exists for one situation: an env-var token (GH_TOKEN, or + # GITHUB_TOKEN as gh's fallback) is overriding the keyring and lacks a scope + # the keyring token has. Only then is there anything to say beyond what gh + # already prints (`gh auth refresh -s `). And the hint is for an + # agent session, which cannot act on gh's own message: a human reading a + # terminal can. So stderr is captured only when BOTH hold — an env token is + # set AND stderr is not a terminal. Capturing costs gh its stderr TTY (colors, + # and the interactive prompts of `gh auth login` / `gh pr create`, which + # render on stderr) and lets tee reorder stderr against stdout; a human with + # GH_TOKEN exported in an interactive shell must not pay that on every call. + # + # Otherwise, exec as before: gh owns the terminal outright and the wrapper + # process is gone. Only this standalone path runs the hint — function mode + # reaches it through `command gh`, which lands right here. If ~/.local/bin/gh + # is not on PATH, no hint is printed; that is the documented limit. + if [[ -z "${GH_TOKEN:-}" && -z "${GITHUB_TOKEN:-}" ]] || [[ -t 2 ]]; then + exec "${REAL_GH}" "$@" + fi + _gh_wrapper_rc=0 + _gh_wrapper_run_with_scope_hint "${REAL_GH}" "$@" || _gh_wrapper_rc=$? + exit "${_gh_wrapper_rc}" else # --- Function-definition mode (sourced from functions.sh) --- gh() { @@ -620,6 +936,6 @@ else # its own body into subshells, not functions it calls. Without exporting # these too, gh() would break in any subshell that inherits the exported # gh but didn't source this file (e.g. BASH_ENV unset/overridden there). - export -f gh sugh _gh_wrapper_block_bypass _gh_wrapper_maybe_review _gh_wrapper_review_script_path _gh_wrapper_sync_identity _gh_wrapper_find_real_gh _gh_wrapper_resolve_owner _gh_wrapper_force_draft_for_off_org _gh_wrapper_is_beacon_context _gh_wrapper_beacon_dir_is_explicit - export _gh_wrapper_review_script GH_WRAPPER_BEACON_DIR _GH_WRAPPER_BEACON_DIR_DEFAULT + export -f gh sugh _gh_wrapper_block_bypass _gh_wrapper_maybe_review _gh_wrapper_review_script_path _gh_wrapper_sync_identity _gh_wrapper_find_real_gh _gh_wrapper_resolve_owner _gh_wrapper_force_draft_for_off_org _gh_wrapper_is_beacon_context _gh_wrapper_beacon_dir_is_explicit _gh_wrapper_logins_equal _gh_wrapper_keyring_login _gh_wrapper_keyring_users _gh_wrapper_resolve_switch_target _gh_wrapper_run_with_scope_hint _gh_wrapper_scope_from_file _gh_wrapper_print_scope_hint _gh_wrapper_redact_argv _gh_wrapper_redact_value + export _gh_wrapper_review_script GH_WRAPPER_BEACON_DIR _GH_WRAPPER_BEACON_DIR_DEFAULT _GH_WRAPPER_LOGIN_ALIASES fi diff --git a/bash/tests/test-gh-wrapper-draft-off-org.sh b/bash/tests/test-gh-wrapper-draft-off-org.sh index 856ed10..9dad2cf 100755 --- a/bash/tests/test-gh-wrapper-draft-off-org.sh +++ b/bash/tests/test-gh-wrapper-draft-off-org.sh @@ -60,6 +60,10 @@ assert_args "off-org explicit --repo=" 1 pr create --repo=someoutsideorg/foo --t # nightowlstudiollc counts as in-org: no draft forced. assert_args "nightowlstudiollc explicit -R" 0 pr create -R nightowlstudiollc/somerepo --title x +# twistedmelonman is the personal account that keeps the archived repos and +# forks after the 2026-09 org migration: in-org, no draft forced. +assert_args "twistedmelonman explicit --repo" 0 pr create --repo twistedmelonman/old-archived --title x + # Case-insensitive owner match, mirroring _gh_wrapper_sync_identity's # regression coverage for smartwatermelon/dotfiles#159. assert_args "mixed-case in-org owner" 0 pr create --repo SmartWatermelon/dotfiles --title x diff --git a/bash/tests/test-gh-wrapper-gh-token-precedence.sh b/bash/tests/test-gh-wrapper-gh-token-precedence.sh index ca80d39..d958812 100755 --- a/bash/tests/test-gh-wrapper-gh-token-precedence.sh +++ b/bash/tests/test-gh-wrapper-gh-token-precedence.sh @@ -28,7 +28,7 @@ export HOME="${WORKDIR}/home" mkdir -p "${HOME}/.config/gh" cat >"${HOME}/.config/gh/hosts.yml" <<'YAML' github.com: - user: smartwatermelon + user: twistedmelonman oauth_token: fake YAML @@ -72,13 +72,23 @@ else fi # Case 3: GH_TOKEN matching the resolved identity -> proceeds. -if _sync_under_env GH_TOKEN="fake-token-for-smartwatermelon" \ - CLAUDE_GH_TOKEN_LOGIN="smartwatermelon"; then +if _sync_under_env GH_TOKEN="fake-token-for-twistedmelonman" \ + CLAUDE_GH_TOKEN_LOGIN="twistedmelonman"; then _pass "matching GH_TOKEN: proceeds" else _fail "matching GH_TOKEN: should proceed" fi +# Case 3b: GH_TOKEN still reporting the pre-rename login. Accepted through the +# temporary alias (dev-env org-migration design, "Temporary login alias"). +# Delete this case together with the alias. +if _sync_under_env GH_TOKEN="fake-token-for-smartwatermelon" \ + CLAUDE_GH_TOKEN_LOGIN="smartwatermelon"; then + _pass "aliased GH_TOKEN (smartwatermelon): proceeds" +else + _fail "aliased GH_TOKEN (smartwatermelon): should proceed via alias" +fi + # Case 4: expired or revoked GH_TOKEN, with CLAUDE_GH_TOKEN_LOGIN unset so the # `gh api user` path runs. Must fail closed AND say the identity could not be # resolved — not that it mismatched. diff --git a/bash/tests/test-gh-wrapper-identity.sh b/bash/tests/test-gh-wrapper-identity.sh index 9d1a7c1..4bc0ecb 100755 --- a/bash/tests/test-gh-wrapper-identity.sh +++ b/bash/tests/test-gh-wrapper-identity.sh @@ -8,7 +8,8 @@ # (regression: smartwatermelon/dotfiles#159). # 2. The Beacon-context heuristic for owners claimed by neither identity # (checkout under the beacon dir, or forked from beacon-biosignals). -# 3. The smartwatermelon default for everything else. +# 3. The twistedmelonman default for everything else (smartwatermelon is a +# temporary alias). set -euo pipefail unset CDPATH @@ -33,7 +34,7 @@ mkdir -p "${HOME}/.config/gh" # fixture owner each case resolves. That guard is correct; inheriting the # ambient token here is not. Sandbox it the same way HOME is sandboxed, so the # cases exercise the hosts.yml path they are written to test. -unset GH_TOKEN CLAUDE_GH_TOKEN_LOGIN +unset GH_TOKEN GITHUB_TOKEN CLAUDE_GH_TOKEN_LOGIN # git init inside the sandboxed HOME must not pick up interactive prompts. export GIT_CONFIG_GLOBAL="${HOME}/.gitconfig" @@ -131,26 +132,153 @@ mkdir -p "${HOME}/neutral-cwd" cd "${HOME}/neutral-cwd" # --- Tier 1: explicitly-claimed owners ------------------------------------- -# These win in both directions and must never depend on cwd. -assert_desired "lowercase smartwatermelon" "smartwatermelon" "smartwatermelon/dotfiles" "smartwatermelon" -assert_desired "lowercase nightowlstudiollc" "andrewmrich" "nightowlstudiollc/kebab-tax" "smartwatermelon" -assert_desired "beacon-biosignals org" "smartwatermelon" "beacon-biosignals/somerepo" "andrewmrich" +# These win in both directions and must never depend on cwd. "Wrong current" +# fixtures use andrewmrich, not smartwatermelon: smartwatermelon is an alias +# of twistedmelonman until the 2026-09 rename lands, so it never triggers a +# switch (see the alias block below). +assert_desired "lowercase smartwatermelon" "andrewmrich" "smartwatermelon/dotfiles" "twistedmelonman" +assert_desired "lowercase nightowlstudiollc" "andrewmrich" "nightowlstudiollc/kebab-tax" "twistedmelonman" +assert_desired "lowercase twistedmelonman" "andrewmrich" "twistedmelonman/old-archived" "twistedmelonman" +assert_desired "already twistedmelonman stays" "twistedmelonman" "smartwatermelon/dotfiles" "twistedmelonman" +assert_desired "beacon-biosignals org" "twistedmelonman" "beacon-biosignals/somerepo" "andrewmrich" # The git-pkgs-proxy case: a fork created during Beacon work, owned by # andrewmrich rather than the beacon-biosignals org. -assert_desired "andrewmrich personal fork" "smartwatermelon" "andrewmrich/git-pkgs-proxy" "andrewmrich" +assert_desired "andrewmrich personal fork" "twistedmelonman" "andrewmrich/git-pkgs-proxy" "andrewmrich" -# Case-insensitivity across all four claimed owners +# Case-insensitivity across all claimed owners # (regression: smartwatermelon/dotfiles#159). -assert_desired "mixed-case SmartWatermelon" "andrewmrich" "SmartWatermelon/dotfiles" "smartwatermelon" -assert_desired "upper-case NIGHTOWLSTUDIOLLC" "andrewmrich" "NIGHTOWLSTUDIOLLC/kebab-tax" "smartwatermelon" -assert_desired "mixed-case Beacon-BioSignals" "smartwatermelon" "Beacon-BioSignals/somerepo" "andrewmrich" -assert_desired "mixed-case AndrewMRich" "smartwatermelon" "AndrewMRich/git-pkgs-proxy" "andrewmrich" +assert_desired "mixed-case SmartWatermelon" "andrewmrich" "SmartWatermelon/dotfiles" "twistedmelonman" +assert_desired "upper-case NIGHTOWLSTUDIOLLC" "andrewmrich" "NIGHTOWLSTUDIOLLC/kebab-tax" "twistedmelonman" +assert_desired "mixed-case TwistedMelonMan" "andrewmrich" "TwistedMelonMan/old-archived" "twistedmelonman" +assert_desired "mixed-case Beacon-BioSignals" "twistedmelonman" "Beacon-BioSignals/somerepo" "andrewmrich" +assert_desired "mixed-case AndrewMRich" "twistedmelonman" "AndrewMRich/git-pkgs-proxy" "andrewmrich" + +# --- Temporary alias (remove with the alias, dev-env org-migration Step 6) --- +# Before the rename, both tokens still report smartwatermelon. That must be +# accepted as twistedmelonman, in both directions the wrapper compares +# (hosts.yml here; GH_TOKEN in test-gh-wrapper-gh-token-precedence.sh). +assert_no_switch() { + local label="$1" current_user="$2" repo_arg="$3" + rm -f "${switch_log}" + cat >"${HOME}/.config/gh/hosts.yml" </dev/null || true)" + if [[ -z "${got}" ]]; then + echo "PASS: ${label} (no switch, ${current_user} accepted)" + else + echo "FAIL: ${label} — unexpected switch attempted to '${got}'" + fail=1 + fi +} +assert_no_switch "alias: smartwatermelon accepted for twistedmelonman" "smartwatermelon" "smartwatermelon/dotfiles" +assert_no_switch "alias: SmartWatermelon accepted case-insensitively" "SmartWatermelon" "nightowlstudiollc/kebab-tax" + +if _gh_wrapper_logins_equal twistedmelonman smartwatermelon; then + echo "PASS: logins_equal accepts the alias" +else + echo "FAIL: logins_equal rejects the alias" + fail=1 +fi +if _gh_wrapper_logins_equal smartwatermelon twistedmelonman; then + echo "FAIL: logins_equal is not one-directional" + fail=1 +else + echo "PASS: logins_equal is one-directional" +fi +if _gh_wrapper_logins_equal twistedmelonman andrewmrich; then + echo "FAIL: logins_equal accepted an unrelated login" + fail=1 +else + echo "PASS: logins_equal rejects an unrelated login" +fi + +# --- Alias resolves the SWITCH TARGET, not just the comparison --------------- +# The window this pins: pre-rename, the keyring holds andrewmrich and +# smartwatermelon but NOT twistedmelonman. A personal repo resolves +# desired=twistedmelonman, so switching to `desired` verbatim asks gh for an +# account it does not have and the wrapper hard-fails — on the very calls the +# alias exists to keep working. The target must be resolved to a login the +# keyring actually holds. Remove with the alias (org-migration Step 6). +# +# This case needs a STRICTER stub than the shared one above: the shared stub +# always exits 0, which would let a switch to a non-existent account look like +# success. This one fails unless the requested user is really held, which is +# what `gh auth switch` does. +# Redefined via eval for the same reason the shared stub above is: only the +# `command` override reaches it, which the linter cannot trace. +_test_switch_strict_users="" +eval '_test_command_stub() { + if [[ "$1" == "gh" && "$2" == "auth" && "$3" == "switch" ]]; then + local requested="${*: -1}" + printf "%s" "${requested}" >"${switch_log}" + local held + for held in ${_test_switch_strict_users}; do + [[ "${held}" == "${requested}" ]] && return 0 + done + return 1 + fi + builtin command "$@" +}' + +assert_switch_target() { + local label="$1" current_user="$2" held_users="$3" repo_arg="$4" expected="$5" + rm -f "${switch_log}" + _test_switch_strict_users="${held_users}" + { + echo "github.com:" + echo " users:" + local u + for u in ${held_users}; do + echo " ${u}:" + done + echo " user: ${current_user}" + } >"${HOME}/.config/gh/hosts.yml" + if ! _gh_wrapper_sync_identity --repo "${repo_arg}" pr list 2>/dev/null; then + echo "FAIL: ${label} — sync returned non-zero (switch to a login the keyring lacks)" + fail=1 + return + fi + local got + got="$(cat "${switch_log}" 2>/dev/null || true)" + if [[ "${got}" == "${expected}" ]]; then + echo "PASS: ${label} (switched to ${expected})" + else + echo "FAIL: ${label} — expected switch to '${expected}', got '${got}'" + fail=1 + fi +} + +# Keyring holds andrewmrich + smartwatermelon; twistedmelonman does not exist +# yet. A personal-org repo must switch to the held alias, not to the +# not-yet-existent desired login. +assert_switch_target "alias resolves switch target to a held login" \ + "andrewmrich" "andrewmrich smartwatermelon" "smartwatermelon/dotfiles" "smartwatermelon" + +# Post-rename shape: once twistedmelonman exists it is preferred over the alias. +assert_switch_target "held desired login wins over its alias" \ + "andrewmrich" "andrewmrich smartwatermelon twistedmelonman" "smartwatermelon/dotfiles" "twistedmelonman" + +# Restore the permissive shared stub for the cases that follow. +eval '_test_command_stub() { + if [[ "$1" == "gh" && "$2" == "auth" && "$3" == "switch" ]]; then + printf "%s" "${*: -1}" >"${switch_log}" + return 0 + fi + builtin command "$@" +}' # --- Tier 3: default --------------------------------------------------------- # An owner claimed by neither identity, with no Beacon context, defaults to -# smartwatermelon. This is the inversion of the old behavior, which defaulted -# unclaimed owners to andrewmrich. -assert_desired "unclaimed owner defaults to smartwatermelon" "andrewmrich" "someotherorg/somerepo" "smartwatermelon" +# twistedmelonman. +assert_desired "unclaimed owner defaults to twistedmelonman" "andrewmrich" "someotherorg/somerepo" "twistedmelonman" # --- Tier 2: Beacon-context heuristic ---------------------------------------- # Only consulted for owners claimed by neither identity. @@ -160,14 +288,14 @@ beacon_repo="${GH_WRAPPER_BEACON_DIR}/thirdparty-tool" mkdir -p "${beacon_repo}" git -C "${beacon_repo}" init -q assert_desired_in "${beacon_repo}" "unclaimed owner, checkout under beacon dir" \ - "smartwatermelon" "someotherorg/thirdparty-tool" "andrewmrich" + "twistedmelonman" "someotherorg/thirdparty-tool" "andrewmrich" # A sibling dir sharing the prefix must NOT match. sibling_repo="${GH_WRAPPER_BEACON_DIR}-scratch/thirdparty-tool" mkdir -p "${sibling_repo}" git -C "${sibling_repo}" init -q assert_desired_in "${sibling_repo}" "prefix-sibling dir does not count as beacon" \ - "andrewmrich" "someotherorg/thirdparty-tool" "smartwatermelon" + "andrewmrich" "someotherorg/thirdparty-tool" "twistedmelonman" # Signal 2: forked from the beacon-biosignals org, checkout anywhere. fork_repo="${HOME}/elsewhere/forked-tool" @@ -175,7 +303,7 @@ mkdir -p "${fork_repo}" git -C "${fork_repo}" init -q git -C "${fork_repo}" remote add upstream "git@github.com:beacon-biosignals/forked-tool.git" assert_desired_in "${fork_repo}" "unclaimed owner, upstream is beacon-biosignals" \ - "smartwatermelon" "someotherorg/forked-tool" "andrewmrich" + "twistedmelonman" "someotherorg/forked-tool" "andrewmrich" # An upstream pointing somewhere else must NOT match. other_fork="${HOME}/elsewhere/other-fork" @@ -183,7 +311,7 @@ mkdir -p "${other_fork}" git -C "${other_fork}" init -q git -C "${other_fork}" remote add upstream "git@github.com:unrelated/other-fork.git" assert_desired_in "${other_fork}" "unrelated upstream does not count as beacon" \ - "andrewmrich" "someotherorg/other-fork" "smartwatermelon" + "andrewmrich" "someotherorg/other-fork" "twistedmelonman" # --- Tier 1 beats Tier 2 ----------------------------------------------------- # An explicitly-claimed owner is authoritative even from inside a beacon @@ -191,7 +319,7 @@ assert_desired_in "${other_fork}" "unrelated upstream does not count as beacon" # `gh -R smartwatermelon/dotfiles ...` meaning the same thing from any # directory (smartwatermelon/dotfiles#135). assert_desired_in "${beacon_repo}" "claimed owner beats beacon cwd" \ - "andrewmrich" "smartwatermelon/dotfiles" "smartwatermelon" + "andrewmrich" "smartwatermelon/dotfiles" "twistedmelonman" cd "${HOME}/neutral-cwd" @@ -204,7 +332,7 @@ assert_warns() { local out cat >"${HOME}/.config/gh/hosts.yml" <"${HOME}/.config/gh/hosts.yml" <<'YAML' +github.com: + user: twistedmelonman + oauth_token: fake +YAML +# GH_TOKEN and GITHUB_TOKEN both authenticate gh (in that precedence order) and +# GH_HOST redirects which host it talks to; any of the three leaking in from the +# caller's environment would change what the hint says. Clear all three so each +# case below controls them explicitly. +unset GH_TOKEN GH_HOST GITHUB_TOKEN +# The wrapper's F3 guard resolves GH_TOKEN's login through `gh api user` when +# this is unset; that would hit the stub and fail. Pin it so no case here +# depends on identity resolution — the scope hint is the thing under test. +export CLAUDE_GH_TOKEN_LOGIN="twistedmelonman" + +fail=0 +_pass() { echo " PASS: $1"; } +_fail() { + echo " FAIL: $1" >&2 + fail=1 +} + +# Three stub gh binaries. Each is the only `gh` on PATH after the wrapper +# itself, which _gh_wrapper_find_real_gh skips. +STUBS="${WORKDIR}/stubs" +mkdir -p "${STUBS}/scope" "${STUBS}/plain403" "${STUBS}/ok" + +# Text measured against the real gh binary (gh 2.x, 2026-09-03), via +# gh api orgs/nightowlstudiollc/actions/secrets +# whose stderr was, verbatim: +# gh: You must be an org admin or have the actions secrets fine-grained permission. (HTTP 403) +# gh: This API operation needs the "admin:org" scope. To request it, run: gh auth refresh -h github.com -s admin:org +# The real exit status there is 1; the stub uses 4 so the pass-through +# assertion cannot be satisfied by the wrapper's own generic failure code. +cat >"${STUBS}/scope/gh" <<'STUB' +#!/usr/bin/env bash +echo '{"message":"You must be an org admin or have the actions secrets fine-grained permission.","status":"403"}' +echo 'gh: You must be an org admin or have the actions secrets fine-grained permission. (HTTP 403)' >&2 +echo 'gh: This API operation needs the "admin:org" scope. To request it, run: gh auth refresh -h github.com -s admin:org' >&2 +exit 4 +STUB +cat >"${STUBS}/plain403/gh" <<'STUB' +#!/usr/bin/env bash +echo 'gh: Must have admin rights to Repository. (HTTP 403)' >&2 +exit 1 +STUB +cat >"${STUBS}/ok/gh" <<'STUB' +#!/usr/bin/env bash +echo 'stdout-from-gh' +echo 'stderr-from-gh' >&2 +exit 0 +STUB +chmod +x "${STUBS}"/*/gh + +# The wrapper's stderr temp files land in ${TMPDIR}. Point that at a sandbox so +# the leak assertions below observe only this test's files, and cannot be +# fooled by an unrelated process's leftovers in the shared /tmp. +export TMPDIR="${WORKDIR}/tmp" +mkdir -p "${TMPDIR}" + +# Count the wrapper's stderr temp files currently in the sandboxed TMPDIR. +_errfile_count() { + find "${TMPDIR}" -maxdepth 1 -name 'gh-wrapper-stderr.*' 2>/dev/null | wc -l | tr -d ' ' +} + +# Run the wrapper in standalone mode from a non-repo cwd (owner resolution +# yields nothing, so no identity switch is attempted) with the given stub +# first on PATH after the wrapper. Captures stdout and stderr separately. +# +# The wrapper is invoked as `bash "${WRAPPER}"` — the repo file directly, NOT +# ~/.local/bin/gh. Going through the installed symlink makes +# _gh_wrapper_find_real_gh resolve `gh` back to the wrapper itself, which +# re-enters and never reaches the code under test. +_run() { + local stub="$1" + shift + ( + cd "${HOME}/neutral-cwd" || exit 99 + PATH="${STUBS}/${stub}:${PATH}" bash "${WRAPPER}" "$@" \ + >"${WORKDIR}/out" 2>"${WORKDIR}/err" + ) +} + +# --- Case 1: scope error with GH_TOKEN set -> env -u hint --------------------- +GH_TOKEN="fixture-token" _run scope secret set CLAUDE_CODE_OAUTH_TOKEN --org smartwatermelon --visibility all +rc=$? +err="$(cat "${WORKDIR}/err")" +out="$(cat "${WORKDIR}/out")" + +if [[ "${rc}" -eq 4 ]]; then + _pass "scope error: exit code passes through (4)" +else + _fail "scope error: expected exit 4, got ${rc}" +fi +if [[ "${err}" == *'needs the "admin:org" scope'* ]]; then + _pass "scope error: original stderr preserved" +else + _fail "scope error: original stderr missing, got: ${err}" +fi +if [[ "${err}" == *"[gh] GH_TOKEN is set and lacks the 'admin:org' scope"* ]]; then + _pass "scope error: hint names the scope" +else + _fail "scope error: hint missing, got: ${err}" +fi +if [[ "${err}" == *"env -u GH_TOKEN gh secret set CLAUDE_CODE_OAUTH_TOKEN --org smartwatermelon --visibility all"* ]]; then + _pass "scope error: hint carries the exact re-run command" +else + _fail "scope error: re-run command missing, got: ${err}" +fi +if [[ "${err}" == *"keyring identity for twistedmelonman"* ]]; then + _pass "scope error: hint names the keyring login from hosts.yml" +else + _fail "scope error: keyring login missing, got: ${err}" +fi +if [[ "${out}" == *'You must be an org admin'* ]]; then + _pass "scope error: stdout passes through untouched" +else + _fail "scope error: stdout lost, got: ${out}" +fi + +# --- Case 2: scope error WITHOUT an env token -> plain passthrough, no hint --- +# With no GH_TOKEN/GITHUB_TOKEN there is no escape hatch to suggest: gh's own +# stderr already says `gh auth refresh -s admin:org`. The wrapper must exec gh +# untouched so a human at a terminal keeps gh's stderr TTY and ordering. +_run scope secret set CLAUDE_CODE_OAUTH_TOKEN --org smartwatermelon +rc=$? +err="$(cat "${WORKDIR}/err")" +if [[ "${rc}" -eq 4 && "${err}" == *'needs the "admin:org" scope'* && "${err}" != *"[gh]"* ]]; then + _pass "scope error, no env token: gh's stderr passes through, no wrapper hint" +else + _fail "scope error, no env token: expected untouched stderr and rc 4, got rc=${rc}: ${err}" +fi +# The no-token path must exec, not fork-and-wait: the stub reports whether its +# parent is still the wrapper (bash reading gh-wrapper.sh) or the test's +# subshell. A stderr TTY check would be the direct test, but the suite has no +# pty; the exec check proves the same thing one level up. +mkdir -p "${STUBS}/ppid" +cat >"${STUBS}/ppid/gh" <<'STUB' +#!/usr/bin/env bash +ps -o command= -p "${PPID}" 2>/dev/null +exit 0 +STUB +chmod +x "${STUBS}/ppid/gh" +_run ppid api user +out="$(cat "${WORKDIR}/out")" +if [[ "${out}" != *"gh-wrapper.sh"* ]]; then + _pass "no env token: wrapper execs gh (parent is not the wrapper)" +else + _fail "no env token: wrapper did not exec, parent is: ${out}" +fi +GH_TOKEN="fixture-token" _run ppid api user +out="$(cat "${WORKDIR}/out")" +if [[ "${out}" == *"gh-wrapper.sh"* ]]; then + _pass "env token set: wrapper stays resident to read gh's exit (control)" +else + _fail "env token set: expected the wrapper as parent, got: ${out}" +fi +# Env token set BUT stderr is a terminal -> still exec. The hint is for an +# agent session; a human with GH_TOKEN exported in an interactive shell would +# otherwise lose gh's stderr TTY on every call, and `gh auth login` / `gh pr +# create` render their prompts on stderr. The pty comes from script(1): BSD +# script runs the command directly on a fresh pty (the suite runs on macOS — +# run-tests.sh and CI both require it). The stub reports what gh sees. +mkdir -p "${STUBS}/tty2" +cat >"${STUBS}/tty2/gh" <<'STUB' +#!/usr/bin/env bash +if [[ -t 2 ]]; then echo STDERR_IS_TTY; else echo STDERR_NOT_TTY; fi +exit 0 +STUB +chmod +x "${STUBS}/tty2/gh" +host_os="$(uname)" +if [[ "${host_os}" == "Darwin" ]] && command -v script >/dev/null 2>&1; then + out="$( + cd "${HOME}/neutral-cwd" || exit 99 + GH_TOKEN="fixture-token" PATH="${STUBS}/tty2:${PATH}" \ + script -q /dev/null bash "${WRAPPER}" api user /dev/null | tr -d '\r' + )" + if [[ "${out}" == *"STDERR_IS_TTY"* ]]; then + _pass "env token set, stderr is a tty: wrapper execs gh (gh keeps its stderr tty)" + else + _fail "env token set, stderr is a tty: gh lost its stderr tty, stub saw: ${out}" + fi +else + _fail "env token set, stderr is a tty: no BSD script(1) available to allocate a pty" +fi + +# --- Case 3: negative control, non-scope 403 -> no hint ----------------------- +GH_TOKEN="fixture-token" _run plain403 pr list --repo smartwatermelon/dotfiles +rc=$? +err="$(cat "${WORKDIR}/err")" +if [[ "${rc}" -eq 1 && "${err}" == *"Must have admin rights"* && "${err}" != *"[gh]"* ]]; then + _pass "non-scope 403: stderr preserved, no hint, exit 1" +else + _fail "non-scope 403: expected no hint, got rc=${rc} err=${err}" +fi + +# --- Case 4: negative control, success -> nothing added ----------------------- +GH_TOKEN="fixture-token" _run ok repo view smartwatermelon/dotfiles +rc=$? +out="$(cat "${WORKDIR}/out")" +err="$(cat "${WORKDIR}/err")" +if [[ "${rc}" -eq 0 && "${out}" == "stdout-from-gh" && "${err}" == "stderr-from-gh" ]]; then + _pass "success: stdout and stderr untouched, exit 0" +else + _fail "success: output altered, rc=${rc} out=${out} err=${err}" +fi + +# --- Case 5: no stray temp files -------------------------------------------- +leftover="$(_errfile_count)" +if [[ "${leftover}" == "0" ]]; then + _pass "temp stderr files cleaned up" +else + _fail "temp stderr files left behind: ${leftover}" +fi + +# --- Case 6: secret-bearing flag values are redacted from the hint ----------- +# The hint goes to stderr, which lands in scrollback, CI logs and transcripts. +# Echoing back `--body ` would copy a live credential into all three. +SECRET="totally-not-a-real-secret-value-9271" +GH_TOKEN="fixture-token" _run scope secret set FOO --body "${SECRET}" --org smartwatermelon +err="$(cat "${WORKDIR}/err")" +if [[ "${err}" == *""* ]]; then + _pass "redaction: hint shows for --body" +else + _fail "redaction: expected , got: ${err}" +fi +if [[ "${err}" == *"${SECRET}"* ]]; then + _fail "redaction: SECRET LEAKED into the hint: ${err}" +else + _pass "redaction: secret value never appears in the hint" +fi +# The non-secret parts must survive, or the suggestion is not runnable. +if [[ "${err}" == *"gh secret set FOO"* && "${err}" == *"--org smartwatermelon"* ]]; then + _pass "redaction: non-secret arguments preserved" +else + _fail "redaction: non-secret arguments mangled, got: ${err}" +fi + +# --with-equals spelling must redact too; a key=value field keeps its key. +GH_TOKEN="fixture-token" _run scope api -X POST /x --field="body=${SECRET}" +err="$(cat "${WORKDIR}/err")" +if [[ "${err}" != *"${SECRET}"* && "${err}" == *"--field=body="* ]]; then + _pass "redaction: --field=key=VALUE keeps the key, redacts the value" +else + _fail "redaction: --field=key=VALUE not redacted as expected, got: ${err}" +fi + +# Stuck short flags (-bVALUE, -fkey=VALUE) are valid pflag syntax and were the +# leak the first fix round missed: an exact-token `case` never saw them. +GH_TOKEN="fixture-token" _run scope secret set FOO "-b${SECRET}" --org smartwatermelon +err="$(cat "${WORKDIR}/err")" +if [[ "${err}" != *"${SECRET}"* && "${err}" == *" -b "* ]]; then + _pass "redaction: stuck -bVALUE redacted" +else + _fail "redaction: stuck -bVALUE leaked or mangled, got: ${err}" +fi +GH_TOKEN="fixture-token" _run scope api -X POST /x "-fquery=${SECRET}" +err="$(cat "${WORKDIR}/err")" +if [[ "${err}" != *"${SECRET}"* && "${err}" == *" -fquery="* ]]; then + _pass "redaction: stuck -fkey=VALUE keeps the key, redacts the value" +else + _fail "redaction: stuck -fkey=VALUE leaked or mangled, got: ${err}" +fi + +# A positional after `--` is never a flag, even if it spells one. +GH_TOKEN="fixture-token" _run scope api /x -- --body +err="$(cat "${WORKDIR}/err")" +if [[ "${err}" == *"gh api /x -- --body"* && "${err}" != *""* ]]; then + _pass "redaction: -- ends flag parsing" +else + _fail "redaction: -- not honored, got: ${err}" +fi + +# --- Case 7: GITHUB_TOKEN (no GH_TOKEN) names the right variable ------------- +# gh reads GH_TOKEN first, then GITHUB_TOKEN. Suggesting `gh auth refresh` here +# would be useless: an env-var token overrides the keyring token it rewrites. +GITHUB_TOKEN="fixture-token" _run scope secret set FOO --org smartwatermelon +err="$(cat "${WORKDIR}/err")" +if [[ "${err}" == *"env -u GITHUB_TOKEN"* ]]; then + _pass "GITHUB_TOKEN: hint names GITHUB_TOKEN in the re-run" +else + _fail "GITHUB_TOKEN: expected 'env -u GITHUB_TOKEN', got: ${err}" +fi +if [[ "${err}" == *"env -u GH_TOKEN "* ]]; then + _fail "GITHUB_TOKEN: wrongly told the user to unset GH_TOKEN" +else + _pass "GITHUB_TOKEN: does not name the unset GH_TOKEN" +fi + +# --- Case 8: no temp-file leak when the wrapper is signalled mid-run --------- +# SIGTERM/SIGHUP skip the function's trailing rm -f; only a RETURN trap cleans +# up. SIGINT already unwound correctly, so TERM is the case that regressed. +# +# The stub records its own pid so the test can also prove the wrapper forwarded +# the signal: before F4 the wrapper exec'd gh, so "kill the gh process" killed +# gh. Now that pid is the wrapper's, and gh must not survive it as an orphan. +mkdir -p "${STUBS}/sleeper" +cat >"${STUBS}/sleeper/gh" <<'STUB' +#!/usr/bin/env bash +echo "$$" >"${OM_SLEEPER_PIDFILE:?}" +echo 'starting' >&2 +sleep 30 +STUB +chmod +x "${STUBS}/sleeper/gh" +export OM_SLEEPER_PIDFILE="${WORKDIR}/sleeper.pid" +rm -f "${OM_SLEEPER_PIDFILE}" + +leak_before="$(_errfile_count)" +# `exec` so the backgrounded pid IS the wrapper. Without it the pid belongs to +# the subshell, and signalling that leaves the wrapper running untouched — the +# trap never fires and the test reports a leak that is really a mis-aimed kill. +# GH_TOKEN is set because only the env-token path captures stderr at all. +( + cd "${HOME}/neutral-cwd" || exit 99 + PATH="${STUBS}/sleeper:${PATH}" GH_TOKEN="fixture-token" exec bash "${WRAPPER}" api user \ + >/dev/null 2>&1 +) & +sleeper_pid=$! +# Wait for the errfile to actually exist before signalling. Without this the +# test can kill the wrapper before mktemp runs and pass for the wrong reason — +# a clean result would then prove nothing. +saw_errfile=0 +for _ in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do + now="$(_errfile_count)" + if [[ "${now}" != "${leak_before}" ]]; then + saw_errfile=1 + break + fi + sleep 0.25 +done +kill -TERM "${sleeper_pid}" 2>/dev/null +# Bounded: a plain `wait` here passes for the wrong reason. bash defers a +# trapped signal while a FOREGROUND command runs, so a wrapper that runs gh in +# the foreground only acts on the SIGTERM after gh's own 30s exit — by which +# time the cleanup and "forward" look fine. Measured against 42a5502: every +# assertion below passed, 30s late. The wrapper must be gone within 3s. +wrapper_gone=0 +for _ in 1 2 3 4 5 6 7 8 9 10 11 12; do + if ! kill -0 "${sleeper_pid}" 2>/dev/null; then + wrapper_gone=1 + break + fi + sleep 0.25 +done +if [[ "${wrapper_gone}" == "1" ]]; then + _pass "SIGTERM prompt: wrapper acted on the signal within 3s" +else + _fail "SIGTERM prompt: wrapper still alive 3s after SIGTERM (trap deferred behind a foreground gh)" + kill -KILL "${sleeper_pid}" 2>/dev/null +fi +wait "${sleeper_pid}" 2>/dev/null +sleep 0.3 + +if [[ "${saw_errfile}" == "1" ]]; then + _pass "SIGTERM leak: errfile observed mid-run (assertion is meaningful)" +else + _fail "SIGTERM leak: errfile never appeared; the check below proves nothing" +fi +leak_after="$(_errfile_count)" +if [[ "${leak_after}" == "0" ]]; then + _pass "SIGTERM leak: no temp file left behind after SIGTERM" +else + _fail "SIGTERM leak: ${leak_after} temp file(s) left behind after SIGTERM" +fi +gh_pid="$(cat "${OM_SLEEPER_PIDFILE}" 2>/dev/null)" +if [[ -n "${gh_pid}" ]] && ! kill -0 "${gh_pid}" 2>/dev/null; then + _pass "SIGTERM forward: gh did not survive the wrapper as an orphan" +else + _fail "SIGTERM forward: gh (pid ${gh_pid:-unknown}) still running after the wrapper died" + kill -TERM "${gh_pid}" 2>/dev/null +fi + +if [[ ${fail} -eq 0 ]]; then + echo "test-gh-wrapper-scope-hint.sh: all assertions passed" + exit 0 +fi +exit 1