Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .shellcheckrc
Original file line number Diff line number Diff line change
@@ -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
12 changes: 7 additions & 5 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 <file>` (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 <file>`. 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

Expand Down
29 changes: 25 additions & 4 deletions lib/credentials.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
44 changes: 44 additions & 0 deletions tests/test-credentials.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 ""
Expand Down
Loading