diff --git a/.shellcheckrc b/.shellcheckrc new file mode 100644 index 0000000..a6606c5 --- /dev/null +++ b/.shellcheckrc @@ -0,0 +1,15 @@ +# Repo-level shellcheck config. +# +# Every lib/*.sh sources its siblings through a variable path +# (`source "${WRAPPER_LIB}/logging.sh"`, `source "${TEST_DIR}/lib/op-guard.sh"`), +# which shellcheck cannot resolve on its own. The `# shellcheck source=...` +# directives at those call sites name the real target, but a directive is only +# followed when external sourcing is enabled — otherwise every one of them +# raises SC1091. +# +# Local runs passed anyway, because with no rc in the repo shellcheck fell +# back to the developer's ~/.shellcheckrc, which happens to set this. CI has +# no such file, so it saw the raw SC1091 and failed — a gap no local lint +# could catch. Setting it here makes both environments agree, and removes the +# need to remember `--external-sources` on every manual invocation. +external-sources=true diff --git a/CLAUDE.md b/CLAUDE.md index 9e533e1..e274b82 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -21,10 +21,12 @@ There is no test runner script — run individual test files directly. No `npm t ### Lint ```bash -shellcheck --external-sources lib/*.sh bin/claude-wrapper tests/*.sh +shellcheck -S info lib/*.sh bin/claude-wrapper tests/*.sh tests/lib/*.sh ``` -Use `--external-sources` because lib files source each other via variables (`${WRAPPER_LIB}/logging.sh`), and plain `shellcheck` emits false SC1091 warnings. +No `--external-sources` flag is needed: the repo-root `.shellcheckrc` sets `external-sources=true` for every run. Lib and test files source each other through variable paths (`${WRAPPER_LIB}/logging.sh`, `${TEST_DIR}/lib/op-guard.sh`), which shellcheck cannot resolve unless external sourcing is enabled — without it, the `# shellcheck source=...` directives at those call sites are ignored and each one raises a false SC1091. + +Keep that config in the repo rather than relying on a personal `~/.shellcheckrc`. CI (`standards-check`) runs bare `shellcheck -S info` with no flags and no user-level config, so a machine-local rc makes local lint pass while CI fails — see PR #122. ### Debug mode @@ -129,9 +131,9 @@ All secret files (`.op` files) must be owner-only permissions (no group/world). *~400 tokens/session saved* -- Run `shellcheck --external-sources ` (not plain `shellcheck`) for files that source other scripts via variables (e.g., `source "${WRAPPER_LIB}/logging.sh"`). Plain shellcheck produces SC1091 errors that are not real failures. -- `shellcheck` on `bin/claude-wrapper` produces SC1091 errors for sourced lib files; suppress with `--exclude=SC1091` or use `shellcheck --external-sources lib/file.sh` instead -- Always run `shellcheck` before committing shell script changes +- Run plain `shellcheck -S info `. The repo-root `.shellcheckrc` sets `external-sources=true`, so variable-path sources (`source "${WRAPPER_LIB}/logging.sh"`) resolve and the old `--external-sources` flag is redundant. +- Do not suppress SC1091 with `--exclude=SC1091` or a `# shellcheck disable` directive — the rc resolves it properly instead of hiding it. +- Always run `shellcheck` before committing shell script changes. ### Git Workflow diff --git a/lib/credentials.sh b/lib/credentials.sh index fec44dd..13011ac 100755 --- a/lib/credentials.sh +++ b/lib/credentials.sh @@ -2,8 +2,13 @@ # credentials.sh — inject CCCLI credentials from 1Password at wrapper launch # # Fetches OP_SERVICE_ACCOUNT_TOKEN from macOS Keychain and GH_TOKEN from the -# Automation vault. Both are scoped to the wrapper process lifetime only — -# they are not present in the interactive shell environment. +# Automation vault. Both are exported into the wrapper process and inherited +# by everything it launches, including the agent's own shell — they are not +# present in the user's login shell, but they ARE visible to child processes. +# +# If the vault fetch fails, GH_TOKEN is set to a deliberately invalid sentinel +# rather than left unset, so gh fails closed instead of falling back to the +# keyring OAuth token (which holds admin:org). See _load_gh_token. # # Requires: lib/logging.sh must be sourced first. # Must be sourced before lib/secrets-loader.sh (which needs OP_SERVICE_ACCOUNT_TOKEN). @@ -18,6 +23,11 @@ readonly _CREDENTIALS_SH_LOADED=1 readonly _CREDS_KEYCHAIN_SERVICE="op-service-account-claude-automation" readonly _CREDS_GH_TOKEN_REF="op://Automation/GitHub - CCCLI/Token" +# Sentinel exported when the vault fetch fails, so gh fails closed instead of +# falling back to the keyring OAuth token. Must be non-empty (an empty value +# would re-enable the keyring fallback) and must not be a valid credential. +readonly _CREDS_GH_TOKEN_FETCH_FAILED="invalid-cccli-token-vault-fetch-failed" + # Computed once so both credential-fetch functions below don't each re-run # `command -v timeout` on every invocation. _CREDS_HAS_TIMEOUT=false @@ -74,7 +84,9 @@ _load_service_account_token() { # GH_TOKEN is the restricted-scope CCCLI PAT, separate from the # personal token in gh's keyring. _load_gh_token() { - if [[ -n "${GH_TOKEN:-}" ]]; then + # A previously-exported failure sentinel is not a real token: treat it as + # unset so a retry can still reach the vault rather than being skipped. + if [[ -n "${GH_TOKEN:-}" && "${GH_TOKEN}" != "${_CREDS_GH_TOKEN_FETCH_FAILED}" ]]; then debug_log "GH_TOKEN already set, skipping vault lookup" return 0 fi @@ -112,7 +124,16 @@ _load_gh_token() { export GH_TOKEN="${token}" debug_log "GH_TOKEN loaded from Automation vault" else - log_warn "Failed to fetch GH_TOKEN from 1Password — gh CLI will use keyring fallback" + # Fail closed. Leaving GH_TOKEN unset makes gh fall back to the user's + # keyring OAuth token, which carries repo/workflow/admin:org/delete_repo — + # far wider than the restricted CCCLI PAT this function exists to inject. + # A transient op failure (locked vault, missing service account token, + # network blip, backoff exhausted) must not silently upgrade the agent's + # GitHub privileges. Export a deliberately invalid sentinel instead: gh + # then fails with a clear auth error rather than quietly succeeding with + # more access than intended. + export GH_TOKEN="${_CREDS_GH_TOKEN_FETCH_FAILED}" + log_warn "Failed to fetch GH_TOKEN from 1Password — gh disabled for this session (keyring fallback deliberately blocked)" fi unset token } diff --git a/tests/test-credentials.sh b/tests/test-credentials.sh index f39315c..56bf3d5 100755 --- a/tests/test-credentials.sh +++ b/tests/test-credentials.sh @@ -190,6 +190,50 @@ assert_equals "vault-gh-token" "${result}" \ assert_equals "1" "$(wc -l <"${call_log}" | tr -d ' ')" \ "op read succeeds on first attempt -> called exactly once" +# op read fails (all attempts) -> GH_TOKEN exported as the invalid sentinel, +# NOT left unset. Leaving it unset would let gh fall back to the user's keyring +# OAuth token (repo/workflow/admin:org), silently widening the agent's access +# on any transient vault failure. Fail closed instead. +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; source '${LIB_DIR}/logging.sh'; source '${LIB_DIR}/credentials.sh'; echo \"\${GH_TOKEN:-unset}\"" 2>/dev/null +)" +assert_equals "invalid-cccli-token-vault-fetch-failed" "${result}" \ + "op read fails -> GH_TOKEN set to invalid sentinel (fails closed, no keyring fallback)" + +# The same failure must warn, so a degraded session is visible rather than silent. +warn_output="$( + PATH="${stub_dir}" \ + OP_SERVICE_ACCOUNT_TOKEN="dummy" \ + bash -c "unset GH_TOKEN; source '${LIB_DIR}/logging.sh'; source '${LIB_DIR}/credentials.sh'" 2>&1 1>/dev/null +)" +assert_contains "gh disabled for this session" "${warn_output}" \ + "op read fails -> warns that gh is disabled" + +# A previously-set sentinel must not short-circuit a later retry: it is a +# failure marker, not a real token. +stub_dir="$(make_stub_dir env bash cat id security timeout)" +cat >"${stub_dir}/op" <<'EOF' +#!/usr/bin/env bash +echo "recovered-gh-token" +EOF +chmod +x "${stub_dir}/op" +result="$( + PATH="${stub_dir}" \ + OP_SERVICE_ACCOUNT_TOKEN="dummy" \ + GH_TOKEN="invalid-cccli-token-vault-fetch-failed" \ + bash -c "source '${LIB_DIR}/logging.sh'; source '${LIB_DIR}/credentials.sh'; echo \"\${GH_TOKEN:-unset}\"" 2>/dev/null +)" +assert_equals "recovered-gh-token" "${result}" \ + "sentinel present -> vault lookup still runs (sentinel is not a valid preset)" + # --- Summary --- echo ""