Skip to content

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

Merged
cohix merged 1 commit into
prettysmartdev:mainfrom
vinnie357:fix/anthropic-credential-dedup-and-auth-mode
Jul 16, 2026
Merged

fix(auth): dedup container credentials at injection time; add auth mode#18
cohix merged 1 commit into
prettysmartdev:mainfrom
vinnie357:fix/anthropic-credential-dedup-and-auth-mode

Conversation

@vinnie357

@vinnie357 vinnie357 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

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 (post-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: the render.rs clippy one-liner is no longer part of this PR — upstream's render module moved to render/dialog.rs and clippy on the pinned 1.94.0 toolchain is clean without it.

Test summary

make test-fast: 2247 passed, 0 failed. the full cargo test run has 2 failures in real_git_run_poll_ci_loop_* tests that need a live github remote (environment-only; upstream main fails a superset in the same clone).

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)

@vinnie357
vinnie357 force-pushed the fix/anthropic-credential-dedup-and-auth-mode branch from d4865ec to 7860912 Compare July 16, 2026 15:36
@vinnie357

vinnie357 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

rebased onto main (ef1ed17) and squashed to one commit — conflicts resolved and github shows it mergeable. kept the DynamicWorkflows / maxConcurrentAgents merge additive so both your WI-0096 changes and the auth change are intact (your tests pass alongside the new auth ones). dropped the unrelated render.rs clippy one-liner to keep this scoped to auth — the site moved to render/dialog.rs upstream and clippy on the pinned 1.94.0 toolchain is clean without it. fmt, clippy -D warnings, and make test-fast (2247 tests) all green on 1.94.0; the two real_git tests need a live github remote so they're excluded, same as upstream main here. lmk if you want anything tweaked 🙏

@vinnie357
vinnie357 marked this pull request as ready for review July 16, 2026 15:57
@cohix
cohix merged commit 77c99ab into prettysmartdev:main Jul 16, 2026
2 of 3 checks passed
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.

2 participants