feat: inject CCCLI credentials at wrapper launch only

[smartwatermelon]

Summary

• New lib/credentials.sh fetches OP_SERVICE_ACCOUNT_TOKEN from macOS Keychain and GH_TOKEN from op://Automation/GitHub - CCCLI/Token, scoped to the wrapper process lifetime only
• Sourced from bin/claude-wrapper between git-identity.sh and secrets-loader.sh (which depends on OP_SERVICE_ACCOUNT_TOKEN)
• Removes reliance on the interactive shell having these credentials exported; they no longer leak into the user's shell environment
• Keychain lookup is guarded by a 3s timeout to prevent stalls; both loads are no-ops if the target variable is already set

Test plan

• bash tests/test-wrapper.sh — 58/58 pass
• shellcheck --external-sources lib/credentials.sh bin/claude-wrapper clean
• Pre-commit + pre-push reviews PASS (code-reviewer + adversarial-reviewer + codebase review)
• Manual smoke test: launch wrapper, confirm op inject resolves references and gh commands authenticate

🤖 Generated with Claude Code

[claude]

PR #39 Review — feat: inject CCCLI credentials at wrapper launch only

Summary

The PR adds lib/credentials.sh, a new module that fetches OP_SERVICE_ACCOUNT_TOKEN from macOS Keychain and GH_TOKEN from 1Password, and sources it from bin/claude-wrapper between git-identity.sh and secrets-loader.sh. The design is sound: idempotent guards, silent failure on missing credentials, and correct source ordering.

No blocking issues found.

---

Non-blocking observations

1. _load_gh_token lacks a timeout guard (maintenance impact: medium)

lib/credentials.sh:73: op read is called with no timeout, unlike the Keychain lookup which has a 3s timeout guard. If the 1Password CLI hangs (network issue, socket not ready, daemon restart), wrapper startup stalls indefinitely. The asymmetry is surprising given the explicit comment in _load_service_account_token about "Keychain hangs from stalling CCCLI startup" — the same risk applies here.

Suggested fix — mirror the Keychain guard:

if command -v timeout &>/dev/null; then
  token="$(timeout 5 op read "${_CREDS_GH_TOKEN_REF}" 2>/dev/null || true)"
else
  token="$(op read "${_CREDS_GH_TOKEN_REF}" 2>/dev/null || true)"
fi

2. No source guard — readonly declarations will error on re-source (maintenance impact: low-medium)

lib/credentials.sh:14-15: The two readonly constants have no source guard. If any test scaffolding sources credentials.sh twice (or if a future refactor sources it conditionally), bash will emit bash: _CREDS_KEYCHAIN_SERVICE: readonly variable and abort. The test plan shows 58/58 passing so this is not currently broken, but as the test suite grows it is a latent trap. A standard guard at the top would eliminate the risk:

[[ -n "${_CREDENTIALS_SH_LOADED:-}" ]] && return 0
readonly _CREDENTIALS_SH_LOADED=1

3. security binary is macOS-only — Linux CI silently skips Keychain (informational)

lib/credentials.sh:31-39: security find-generic-password does not exist on Linux. The 2>/dev/null || true silences the error cleanly, so CI will hit the log_warn path at line 48 on every run. This is expected given the macOS Keychain design, but worth confirming that warn output does not pollute test output or mask real failures.

---

Strengths

• Idempotent guards ([[ -n "${VAR:-}" ]]) on both loaders prevent double-fetches when the caller's shell already has the tokens set.
• unset token after export is correct hygiene.
• Source order in bin/claude-wrapper is correct: credentials.sh (provides OP_SERVICE_ACCOUNT_TOKEN) before secrets-loader.sh (consumes it).
• id -un instead of $USER is the right call for Keychain account lookup.

VERDICT: PASS

[claude]

PR #39 Review: feat: inject CCCLI credentials at wrapper launch only

Summary

This PR adds lib/credentials.sh, a new module that fetches OP_SERVICE_ACCOUNT_TOKEN from macOS Keychain and GH_TOKEN from the 1Password Automation vault at wrapper launch time, scoped to the wrapper process only. It is sourced in bin/claude-wrapper between git-identity.sh and secrets-loader.sh.

---

Findings

No Blocking Issues

Source ordering is correct.
credentials.sh is inserted in bin/claude-wrapper after git-identity.sh and before secrets-loader.sh, satisfying the documented dependency: secrets-loader.sh needs OP_SERVICE_ACCOUNT_TOKEN already set.

Fail-safe token handling is correct.
Both _load_service_account_token and _load_gh_token use 2>/dev/null || true so failures degrade gracefully. unset token after export prevents the plaintext token from lingering in the local variable. The guard in _load_gh_token correctly skips the vault call when the service account token could not be fetched.

No double-injection risk.
Both functions return early if the target variable is already set, so shells that already export these via ~/.config/bash/1password.sh are unaffected.

Timeout guards are appropriate.
3 s for Keychain and 5 s for op read are reasonable upper bounds that prevent startup stalls.

---

Non-blocking Observations

1. timeout availability check is duplicated (credentials.sh lines 34 and 67)
Each function independently checks command -v timeout. Since both run at module load time, extracting the check into a module-level flag would eliminate the duplication and make adding future credential fetches easier. Low urgency, but worth noting before the pattern spreads.

2. security command is silently unavailable on Linux
_load_service_account_token calls security find-generic-password without a command -v security guard. On Linux the command silently fails (handled correctly by || true), but the resulting log_warn message mentioning Keychain is misleading on non-macOS hosts. A command -v security pre-check returning early with a debug_log would be clearer.

3. Library file has executable bit (100755)
Shell libraries meant to be sourced rather than executed are conventionally 644. The executable bit does no harm but is inconsistent with the sourced-only usage.

VERDICT: PASS