diff --git a/lib/credentials.sh b/lib/credentials.sh index ef46fb0..c207e8a 100755 --- a/lib/credentials.sh +++ b/lib/credentials.sh @@ -136,6 +136,41 @@ _creds_gh_token_ref_for_owner() { esac } +# Read one vault reference, with bounded retries. Prints the token on stdout +# and returns 1 if every attempt failed. Callers decide what a failure means; +# this function never exports anything and never logs the value. +_creds_read_token_ref() { + local token_ref="$1" + local token + local -a backoff=(2 4 8) + local wait_secs + + if "${_CREDS_HAS_TIMEOUT}"; then + for wait_secs in "${backoff[@]}"; do + token="$(timeout "${wait_secs}" op read "${token_ref}" 2>/dev/null || true)" + if [[ -n "${token}" ]]; then + printf '%s' "${token}" + return 0 + fi + debug_log "op read failed (timeout ${wait_secs}s)" + done + else + # No timeout command available to bound each attempt, so retrying would + # only multiply the hang risk (an unbounded op read blocks forever on + # the first try, making retries unreachable) with no upside. Attempt + # exactly once instead of the usual backoff loop. + debug_log "timeout command unavailable, skipping retries (single attempt only)" + token="$(op read "${token_ref}" 2>/dev/null || true)" + if [[ -n "${token}" ]]; then + printf '%s' "${token}" + return 0 + fi + debug_log "op read failed (no timeout available)" + fi + + return 1 +} + # Fetch GH_TOKEN from Automation vault via service account. # Only runs if OP_SERVICE_ACCOUNT_TOKEN is available. # GH_TOKEN is the restricted-scope CCCLI PAT, separate from the @@ -153,40 +188,16 @@ _load_gh_token() { return 0 fi - local token - local -a backoff=(2 4 8) - local wait_secs - local owner token_ref + local token owner token_ref # ${PWD} is the user's launch directory: bin/claude-wrapper never changes # directory before sourcing this file, so the cwd it inherits is the one the - # session was started from. One op read per launch, not three — the token is - # selected, not accumulated. + # session was started from. owner="$(_creds_github_owner_for_dir "${PWD}")" token_ref="$(_creds_gh_token_ref_for_owner "${owner}")" debug_log "GitHub owner: ${owner:-} -> token ref: ${token_ref}" - if "${_CREDS_HAS_TIMEOUT}"; then - for wait_secs in "${backoff[@]}"; do - token="$(timeout "${wait_secs}" op read "${token_ref}" 2>/dev/null || true)" - if [[ -n "${token}" ]]; then - break - fi - debug_log "op read failed (timeout ${wait_secs}s)" - done - else - # No timeout command available to bound each attempt, so retrying would - # only multiply the hang risk (an unbounded op read blocks forever on - # the first try, making retries unreachable) with no upside. Attempt - # exactly once instead of the usual backoff loop. - debug_log "timeout command unavailable, skipping retries (single attempt only)" - token="$(op read "${token_ref}" 2>/dev/null || true)" - if [[ -z "${token}" ]]; then - debug_log "op read failed (no timeout available)" - fi - fi - - if [[ -n "${token}" ]]; then + if token="$(_creds_read_token_ref "${token_ref}")"; then export GH_TOKEN="${token}" debug_log "GH_TOKEN loaded from Automation vault (${token_ref})" else @@ -204,8 +215,68 @@ _load_gh_token() { unset token } +# Load every owner's token as GH_TOKEN_, so a session can act for an +# owner other than the one its launch directory happens to name. +# +# A fine-grained PAT is bound to one resource owner at creation, so no single +# token can cover the fleet: reading an org's repos with the personal token +# fails 403 on protection and 404 on the repo itself, whether that repo is +# public or private — the boundary is ownership, not visibility. Selecting one +# token per session (above) therefore guarantees the wrong credential for any +# cross-owner work, which fleet probes and rollouts are by definition. +# +# Fetched once at launch rather than per use. A 42-repo probe would otherwise +# mean dozens of vault reads, making the correct path slower than the keyring +# fallback it replaces — which is how workarounds get entrenched. +# +# These are additive: GH_TOKEN keeps its launch-directory selection, so nothing +# that reads it changes. See claude-wrapper#126 for the eventual target, where +# gh-wrapper.sh selects among these per invocation from the target repo's owner. +_load_owner_gh_tokens() { + if [[ -z "${OP_SERVICE_ACCOUNT_TOKEN:-}" ]]; then + debug_log "Skipping per-owner token fetch: OP_SERVICE_ACCOUNT_TOKEN not available" + return 0 + fi + + local spec var token_ref token loaded=0 + # var:ref pairs. The refs are the same constants _load_gh_token selects from. + local -a specs=( + "GH_TOKEN_SWM:${_CREDS_GH_TOKEN_REF_SMARTWATERMELON}" + "GH_TOKEN_NOS:${_CREDS_GH_TOKEN_REF_NIGHTOWLSTUDIOLLC}" + "GH_TOKEN_TWM:${_CREDS_GH_TOKEN_REF_PERSONAL}" + ) + + for spec in "${specs[@]}"; do + var="${spec%%:*}" + token_ref="${spec#*:}" + + # Respect a value already in the environment, matching _load_gh_token. + if [[ -n "${!var:-}" && "${!var}" != "${_CREDS_GH_TOKEN_FETCH_FAILED}" ]]; then + debug_log "${var} already set, skipping vault lookup" + ((loaded += 1)) + continue + fi + + if token="$(_creds_read_token_ref "${token_ref}")"; then + export "${var}=${token}" + debug_log "${var} loaded from Automation vault (${token_ref})" + ((loaded += 1)) + else + # Same fail-closed reasoning as GH_TOKEN: a caller that substitutes an + # empty value would fall through to the keyring OAuth token, silently + # widening scope. The sentinel makes gh fail with an auth error instead. + export "${var}=${_CREDS_GH_TOKEN_FETCH_FAILED}" + log_warn "Failed to fetch ${var} from 1Password — cross-owner gh calls for that owner will fail closed" + fi + unset token + done + + debug_log "Per-owner tokens available: ${loaded}/${#specs[@]}" +} + # ========================================================= # MAIN # ========================================================= _load_service_account_token _load_gh_token +_load_owner_gh_tokens diff --git a/tests/test-credentials.sh b/tests/test-credentials.sh index 7f309c8..7607e50 100755 --- a/tests/test-credentials.sh +++ b/tests/test-credentials.sh @@ -175,6 +175,13 @@ assert_equals "" "$([[ -f "${call_log}" ]] && cat "${call_log}" || true)" \ # OP_SERVICE_ACCOUNT_TOKEN available, op read succeeds on first try (with # timeout available) -> GH_TOKEN exported, single call. +# +# The per-owner vars are preset so _load_owner_gh_tokens takes its already-set +# path and reads nothing: this assertion counts _load_gh_token's calls, and a +# shared call log would otherwise attribute all four reads to it. Presetting +# rather than filtering keeps the count exact instead of merely plausible. +# They must be set explicitly here — inheriting them from the developer's live +# environment is what let this pass locally while failing in CI. stub_dir="$(make_stub_dir env bash cat id security timeout)" call_log="${stub_dir}/op-calls.log" cat >"${stub_dir}/op" </dev/null )" assert_equals "vault-gh-token" "${result}" \ @@ -334,6 +342,136 @@ mkdir -p "${repo_dir}/nested/deeper" assert_equals "op://Automation/CCCLI-SWM/token" "$(ref_selected_from "${repo_dir}/nested/deeper")" \ "subdirectory of a repo -> enclosing repo's owner" +# --- Per-owner token loading (_load_owner_gh_tokens) --- +# +# These assert only against the fixed sentinel literal and stub-supplied +# values. No assertion prints a real credential, and the stub `op` never +# returns one — a test that must echo a live token to prove itself is the +# test that leaks it. + +# KNOWN-BAD GATE. Before asserting that the three vars get set, prove the +# assertion can fail: with no OP_SERVICE_ACCOUNT_TOKEN, nothing is exported +# and op is never called. A suite that only ever sees the success path cannot +# distinguish "loaded correctly" from "assertion never ran". +stub_dir="$(make_stub_dir env bash cat id timeout)" +call_log="${stub_dir}/op-calls.log" +cat >"${stub_dir}/op" <>"${call_log}" +echo "should-never-be-reached" +EOF +chmod +x "${stub_dir}/op" +result="$( + PATH="${stub_dir}" \ + bash -c "unset OP_SERVICE_ACCOUNT_TOKEN GH_TOKEN GH_TOKEN_SWM GH_TOKEN_NOS GH_TOKEN_TWM; source '${LIB_DIR}/logging.sh'; source '${LIB_DIR}/credentials.sh'; echo \"\${GH_TOKEN_SWM:-unset}/\${GH_TOKEN_NOS:-unset}/\${GH_TOKEN_TWM:-unset}\"" 2>/dev/null +)" +assert_equals "unset/unset/unset" "${result}" \ + "no OP_SERVICE_ACCOUNT_TOKEN -> per-owner tokens not exported" +assert_equals "" "$([[ -f "${call_log}" ]] && cat "${call_log}" || true)" \ + "no OP_SERVICE_ACCOUNT_TOKEN -> op never invoked for per-owner tokens" + +# All three refs resolve -> all three vars exported, each from its own ref. +# The stub echoes the ref it was asked for, so a var populated from the wrong +# ref is visible rather than merely non-empty. +stub_dir="$(make_stub_dir env bash cat id security timeout)" +cat >"${stub_dir}/op" <<'EOF' +#!/usr/bin/env bash +# args: read +case "$2" in + "op://Automation/CCCLI-SWM/token") echo "tok-swm" ;; + "op://Automation/CCCLI-NOS/token") echo "tok-nos" ;; + "op://Automation/GitHub - CCCLI/Token") echo "tok-twm" ;; + *) exit 1 ;; +esac +EOF +chmod +x "${stub_dir}/op" +result="$( + PATH="${stub_dir}" \ + OP_SERVICE_ACCOUNT_TOKEN="dummy" \ + bash -c "unset GH_TOKEN GH_TOKEN_SWM GH_TOKEN_NOS GH_TOKEN_TWM; source '${LIB_DIR}/logging.sh'; source '${LIB_DIR}/credentials.sh'; echo \"\${GH_TOKEN_SWM}/\${GH_TOKEN_NOS}/\${GH_TOKEN_TWM}\"" 2>/dev/null +)" +assert_equals "tok-swm/tok-nos/tok-twm" "${result}" \ + "all refs resolve -> each per-owner var loaded from its own ref" + +# FAIL-CLOSED PATH. Every ref fails -> each var holds the invalid sentinel, +# never an empty string. An empty value would let gh fall through to the +# keyring OAuth token (repo/workflow/admin:org), silently widening scope on a +# transient vault failure. This is the assertion the export flagged as +# unverified; it is safe because the sentinel is a fixed literal, not a token. +stub_dir="$(make_stub_dir env bash cat id security timeout)" +cat >"${stub_dir}/op" <<'EOF' +#!/usr/bin/env bash +exit 1 +EOF +chmod +x "${stub_dir}/op" +result="$( + PATH="${stub_dir}" \ + OP_SERVICE_ACCOUNT_TOKEN="dummy" \ + bash -c "unset GH_TOKEN GH_TOKEN_SWM GH_TOKEN_NOS GH_TOKEN_TWM; source '${LIB_DIR}/logging.sh'; source '${LIB_DIR}/credentials.sh'; echo \"\${GH_TOKEN_SWM}/\${GH_TOKEN_NOS}/\${GH_TOKEN_TWM}\"" 2>/dev/null +)" +assert_equals "invalid-cccli-token-vault-fetch-failed/invalid-cccli-token-vault-fetch-failed/invalid-cccli-token-vault-fetch-failed" "${result}" \ + "all refs fail -> every per-owner var set to the invalid sentinel (fails closed)" + +# A partial failure must not poison the refs that did resolve: one bad ref +# fails closed on its own var only. Mixed outcomes are the realistic vault +# failure, and the dangerous version is one failure zeroing the others. +stub_dir="$(make_stub_dir env bash cat id security timeout)" +cat >"${stub_dir}/op" <<'EOF' +#!/usr/bin/env bash +case "$2" in + "op://Automation/CCCLI-NOS/token") exit 1 ;; + "op://Automation/CCCLI-SWM/token") echo "tok-swm" ;; + "op://Automation/GitHub - CCCLI/Token") echo "tok-twm" ;; + *) exit 1 ;; +esac +EOF +chmod +x "${stub_dir}/op" +result="$( + PATH="${stub_dir}" \ + OP_SERVICE_ACCOUNT_TOKEN="dummy" \ + bash -c "unset GH_TOKEN GH_TOKEN_SWM GH_TOKEN_NOS GH_TOKEN_TWM; source '${LIB_DIR}/logging.sh'; source '${LIB_DIR}/credentials.sh'; echo \"\${GH_TOKEN_SWM}/\${GH_TOKEN_NOS}/\${GH_TOKEN_TWM}\"" 2>/dev/null +)" +assert_equals "tok-swm/invalid-cccli-token-vault-fetch-failed/tok-twm" "${result}" \ + "one ref fails -> only that var gets the sentinel, others keep their tokens" + +# The fail-closed path must warn, so a degraded session is visible. A silent +# sentinel looks identical to a working token until a gh call fails oddly. +stub_dir="$(make_stub_dir env bash cat id security timeout)" +cat >"${stub_dir}/op" <<'EOF' +#!/usr/bin/env bash +exit 1 +EOF +chmod +x "${stub_dir}/op" +warn_output="$( + PATH="${stub_dir}" \ + OP_SERVICE_ACCOUNT_TOKEN="dummy" \ + bash -c "unset GH_TOKEN GH_TOKEN_SWM GH_TOKEN_NOS GH_TOKEN_TWM; source '${LIB_DIR}/logging.sh'; source '${LIB_DIR}/credentials.sh'" 2>&1 1>/dev/null +)" +assert_contains "Failed to fetch GH_TOKEN_SWM from 1Password" "${warn_output}" \ + "per-owner fetch failure warns rather than failing silently" + +# An already-set var is respected and costs no vault read — matching +# _load_gh_token. The stub exits non-zero, so a lookup would overwrite the +# pre-set value with the sentinel and fail this assertion. +stub_dir="$(make_stub_dir env bash cat id security timeout)" +call_log="${stub_dir}/op-calls.log" +cat >"${stub_dir}/op" <>"${call_log}" +exit 1 +EOF +chmod +x "${stub_dir}/op" +result="$( + PATH="${stub_dir}" \ + OP_SERVICE_ACCOUNT_TOKEN="dummy" \ + GH_TOKEN_SWM="preset-swm" \ + bash -c "unset GH_TOKEN GH_TOKEN_NOS GH_TOKEN_TWM; source '${LIB_DIR}/logging.sh'; source '${LIB_DIR}/credentials.sh'; echo \"\${GH_TOKEN_SWM}\"" 2>/dev/null +)" +assert_equals "preset-swm" "${result}" \ + "already-set per-owner var is preserved, not overwritten" +assert_equals "" "$(grep -F "op://Automation/CCCLI-SWM/token" "${call_log}" 2>/dev/null || true)" \ + "already-set per-owner var -> its ref is never read from the vault" + # --- Summary --- echo "" diff --git a/tests/test-wrapper.sh b/tests/test-wrapper.sh index e589ae6..1b92149 100755 --- a/tests/test-wrapper.sh +++ b/tests/test-wrapper.sh @@ -606,10 +606,17 @@ EOF [[ -n "${real_bin}" ]] && ln -s "${real_bin}" "${stub_dir}/${bin}" done + # The per-owner vars are preset so _load_owner_gh_tokens takes its + # already-set path and reads nothing. This assertion counts _load_gh_token's + # attempts, and the call log is shared by everything the sourced file does, + # so without this the three per-owner reads are attributed here too. They + # must be set explicitly rather than inherited: the developer environment + # exports them, which would make this pass locally and fail in CI. local debug_output debug_output="$( PATH="${stub_dir}" \ OP_SERVICE_ACCOUNT_TOKEN="dummy-token-for-test" \ + GH_TOKEN_SWM="preset" GH_TOKEN_NOS="preset" GH_TOKEN_TWM="preset" \ CLAUDE_DEBUG=true \ bash -c "unset GH_TOKEN; source '${LIB_DIR}/logging.sh'; source '${LIB_DIR}/credentials.sh'" 2>&1 1>/dev/null )"