Skip to content

fix(auth): dedup container credentials at injection time; add auth mode#18

Draft
vinnie357 wants to merge 2 commits into
prettysmartdev:mainfrom
vinnie357:fix/anthropic-credential-dedup-and-auth-mode
Draft

fix(auth): dedup container credentials at injection time; add auth mode#18
vinnie357 wants to merge 2 commits into
prettysmartdev:mainfrom
vinnie357:fix/anthropic-credential-dedup-and-auth-mode

Conversation

@vinnie357

@vinnie357 vinnie357 commented Jun 16, 2026

Copy link
Copy Markdown

Problem

The container/docker runtime diverged from the sbx (Docker Sandbox) path in a way that caused containers to receive two conflicting Anthropic credentials when users supply env(ANTHROPIC_API_KEY) via an overlay:

  • The sbx path already silently skips CLAUDE_CODE_OAUTH_TOKEN (src/engine/sandbox/dsbx/auth.rs) because sbx has no OAuth-token support yet.
  • The container/docker path emitted both env_passthrough (the harness-declared ANTHROPIC_API_KEY) and agent_credentials (the keychain CLAUDE_CODE_OAUTH_TOKEN) as -e flags with no dedup.

Our previous fix (PR #17) attempted to detect this at the keychain-resolution layer by reading std::env::var("ANTHROPIC_API_KEY"). That approach was wrong: it reads the ambient process environment, not the harness-declared env, regressing users who have ANTHROPIC_API_KEY in their process env but not declared as an env() overlay (they'd lose OAuth auth).

Changes

Part A — injection-time credential dedup (all container runtimes)

ResolvedContainerOptions::resolve() now calls dedup_credentials_by_declared_env() after ingesting all options. When a var in env_passthrough or env_literal maps to a provider service via service_for_credential, any agent_credentials entry mapping to the same service is dropped before the options bag is used by any backend.

  • Keyed on declared env only — never std::env::var.
  • Applies to docker AND apple-containers (shared ResolvedContainerOptions path).
  • Cloud harnesses that do not declare an anthropic env var receive keychain OAuth unaffected.

Part B — explicit per-harness auth mode

A new auth field in .awman/config.json (per-repo) selects the credential injection policy:

Value Behaviour
keychain (default) Inject keychain creds; Part-A dedup applied.
passthrough Skip keychain injection entirely; rely on env() overlays.
none No credential injection.

Resolved via EffectiveConfig::auth_mode() and gated in AuthEngine::resolve_agent_auth.

Shared infrastructure

service_for_credential is now the canonical table in engine::auth (public). The dsbx module re-exports it rather than maintaining its own copy. CLAUDE_CODE_OAUTH_TOKEN is added to the table (→ "anthropic") so the container-side dedup can match it against ANTHROPIC_API_KEY by service. The sbx inject_credentials is updated to skip CLAUDE_CODE_OAUTH_TOKEN via an unconditional key-name guard (before the service lookup), preserving sbx behaviour.

Supersedes

PR #17 (env-sniff approach). This PR replaces it — the new branch is a clean cherry off upstream/main with no Approach-1 guard.

Follow-up fixes (Forge review)

Part A gate on resolvable declared env: dedup_credentials_by_declared_env now only counts an env_passthrough entry as "covering" its service when the host value is actually set (lookup_env returns Some(non-empty)), mirroring build_run_argv's own emission gate. A declared but unset passthrough var no longer suppresses a keychain credential — the passthrough would emit nothing and the container would be left with zero credentials for that service. env_literal entries still always count (they carry an explicit value). A tracing::warn! is emitted whenever a keychain credential is dropped so the situation is never silent.

Injectable lookup_env closure: The coverage check takes a &dyn Fn(&str) -> Option<String> rather than calling std::env::var directly, so tests stay hermetic without mutating global env state.

Doc precision: "Per-harness" → "Per-repo" in AgentAuthMode, resolve_agent_auth, and EffectiveConfig::auth_mode() doc comments. none/passthrough docs now correctly say "No KEYCHAIN credential injection; declared env() overlays still apply" rather than "No credential injection of any kind." auth_mode() gains a note that single-layer (repo-only) is a deliberate conservative choice.

Incidental: src/frontend/tui/render.rs contains a pre-existing unnecessary_min_or_max clippy lint (line 1190) kept in this PR for clippy-clean CI; it is unrelated to auth and was present before these changes.

Test summary

All tests pass except one pre-existing infra-dependent test (real_git_run_poll_ci_loop_* requires a GitHub remote — no git remotes in the test working directory).

New tests added (all hermetic, injectable closure):

  • dedup_fn_drops_oauth_when_anthropic_passthrough_set_on_host — passthrough declared AND set → OAuth dropped
  • dedup_fn_retains_oauth_when_anthropic_passthrough_declared_but_unset — passthrough declared but UNSET → OAuth retained (core fix)
  • dedup_fn_retains_oauth_when_no_anthropic_declared — no anthropic var → OAuth retained
  • dedup_fn_handles_env_literal_source — env_literal always counts
  • dedup_fn_retains_credential_when_no_declared_vars — no-op path
  • dedup_fn_unmapped_credential_is_never_dropped — custom token retained
  • dedup_fn_drops_when_credential_name_equals_declared_var — keychain var name == declared var → dropped
  • dedup_fn_multi_service_declared_x_retains_credential_for_y — multi-service: X dropped, Y retained
  • declared_anthropic_env_literal_drops_oauth_token_from_agent_credentials — env_literal through resolve()
  • engine::container::options — Part A dedup via resolve() and dedup_credentials_by_declared_env unit tests (env_passthrough, env_literal, unmapped credentials, no-op paths)
  • data::config::effective — Part B auth_mode() precedence tests (default, passthrough, none, explicit keychain)
  • engine::auth — Part B resolve_agent_auth gating tests (passthrough → empty, none → empty, keychain → Ok)

Part A: when the harness declares an env(VAR) overlay that covers a
provider service (e.g. env(ANTHROPIC_API_KEY) → service "anthropic"),
drop the matching keychain credential from agent_credentials at
ResolvedContainerOptions::resolve time.  This prevents containers from
receiving two conflicting Anthropic credentials when the user has both
ANTHROPIC_API_KEY declared as an overlay and a CLAUDE_CODE_OAUTH_TOKEN
in the keychain.  The dedup is keyed on declared env only — never on
std::env::var — so cloud harnesses without a declared anthropic var
continue to receive keychain OAuth unaffected.

Part B: add a per-harness `auth` field to RepoConfig with three modes:
`keychain` (default, inject keychain creds with Part-A dedup applied),
`passthrough` (never inject keychain creds; rely on env() overlays),
`none` (no credential injection).  Gated in AuthEngine::resolve_agent_auth
via session.effective_config().auth_mode(), which is resolved through the
existing EffectiveConfig precedence layer.

Shared infrastructure: service_for_credential is now the canonical table
in engine::auth and is re-exported into the dsbx module rather than
duplicated.  CLAUDE_CODE_OAUTH_TOKEN is added to the table (→ "anthropic")
so the dedup can match it against ANTHROPIC_API_KEY by service name; the
sbx driver's existing key-name guard for the OAuth token is updated
to be an unconditional continue-before-service-lookup so sbx behaviour
is unchanged.

Also apply the pre-existing clippy::unnecessary_min_or_max fix in
src/frontend/tui/render.rs required to pass the lint gate.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant