diff --git a/docs/07-configuration.md b/docs/07-configuration.md index 3a2ac004..417d5dd8 100644 --- a/docs/07-configuration.md +++ b/docs/07-configuration.md @@ -168,6 +168,24 @@ awman config set yoloDisallowedTools "" # set an empty list An empty repo list actively overrides a non-empty global list. To stop overriding, remove the field from the repo config file. See [Yolo Mode](06-yolo-mode.md). +### Control credential injection (`auth` mode) + +By default awman injects host keychain credentials into agent containers +(`keychain` mode). Two alternatives are available for harnesses that supply +credentials through other means: + +```json +{ "auth": "passthrough" } +``` + +| Value | Behaviour | +|-------|-----------| +| `keychain` (default) | Inject host keychain credentials. When the repo also declares `env(ANTHROPIC_API_KEY)` (or another credential that covers the same provider) **and that variable is set on the host**, the keychain OAuth token for that provider is automatically suppressed at injection time — the container receives exactly one set of credentials per provider. If the declared passthrough var is not set on the host, the keychain credential is retained so the container is not left with zero credentials for that provider. | +| `passthrough` | No KEYCHAIN credential injection; declared `env()` overlays still apply. Supply credentials via `env(VAR)` overlays. | +| `none` | No KEYCHAIN credential injection; declared `env()` overlays still apply. | + +Set `auth` in `.awman/config.json` directly (it is not settable via `config set`). The field is per-repo only — cloud harnesses that do not declare an anthropic env var remain on the default `keychain` path and continue to receive keychain OAuth unaffected. + --- ## Runtimes @@ -220,6 +238,7 @@ awman keeps global config and data (workflows, skills, worktrees, API state) und | `agentStuckTimeout` | integer (seconds) | 30 | Inactivity period before an agent is flagged as stuck | yes | | `baseImage` | string | (unset → global) | Image tag for workflow setup/teardown containers — see [Workflows](05-workflows.md) | no (edit file) | | `dockerfile` | string | `Dockerfile.dev` | Path to the project base Dockerfile, relative to repo root or absolute | no (edit file) | +| `auth` | `"keychain"` \| `"passthrough"` \| `"none"` | `"keychain"` | Credential injection mode — see [Control credential injection](#control-credential-injection-auth-mode) | no (edit file) | ### Global config fields (`$HOME/.awman/config.json`) diff --git a/src/data/config/effective.rs b/src/data/config/effective.rs index c31d0319..159153e8 100644 --- a/src/data/config/effective.rs +++ b/src/data/config/effective.rs @@ -9,7 +9,7 @@ use std::time::Duration; use crate::data::config::env::EnvSnapshot; use crate::data::config::flags::FlagConfig; use crate::data::config::global::GlobalConfig; -use crate::data::config::repo::RepoConfig; +use crate::data::config::repo::{AgentAuthMode, RepoConfig}; use crate::data::config::{DEFAULT_AGENT_STUCK_TIMEOUT_SECS, DEFAULT_SCROLLBACK_LINES}; /// Merged view of every configuration source, in precedence order. @@ -206,6 +206,20 @@ impl EffectiveConfig { } self.global.base_image.clone() } + + /// Effective credential injection mode (repo > built-in default `keychain`). + /// + /// `passthrough` skips keychain injection entirely; `none` disables keychain + /// injection; `keychain` (default) injects host keychain creds, subject to + /// injection-time dedup of any repo-declared env vars. + /// + /// Single-layer resolution (repo only, no global flag override) is a + /// deliberate conservative choice: a credential toggle must be an explicit + /// per-repo decision, not silently inherited from a global default that the + /// operator may not realize is set. + pub fn auth_mode(&self) -> AgentAuthMode { + self.repo.auth.unwrap_or_default() + } } #[cfg(test)] @@ -214,7 +228,7 @@ mod tests { use crate::data::config::env::{ EnvSnapshot, AWMAN_API_KEY, AWMAN_REMOTE_ADDR, AWMAN_REMOTE_SESSION, }; - use crate::data::config::repo::{ApiConfig, RemoteConfig}; + use crate::data::config::repo::{AgentAuthMode, ApiConfig, RemoteConfig}; use std::time::Duration; fn make_effective( @@ -803,4 +817,74 @@ mod tests { ); assert_eq!(ec4.scrollback_lines(), DEFAULT_SCROLLBACK_LINES); } + + // ── Part B: auth_mode ───────────────────────────────────────────────────── + + #[test] + fn auth_mode_default_is_keychain() { + let ec = make_effective( + FlagConfig::default(), + EnvSnapshot::empty(), + RepoConfig::default(), + GlobalConfig::default(), + ); + assert_eq!( + ec.auth_mode(), + AgentAuthMode::Keychain, + "default auth mode must be keychain" + ); + } + + #[test] + fn auth_mode_passthrough_from_repo() { + let repo = RepoConfig { + auth: Some(AgentAuthMode::Passthrough), + ..Default::default() + }; + let ec = make_effective( + FlagConfig::default(), + EnvSnapshot::empty(), + repo, + GlobalConfig::default(), + ); + assert_eq!( + ec.auth_mode(), + AgentAuthMode::Passthrough, + "passthrough set in repo config must be effective" + ); + } + + #[test] + fn auth_mode_none_from_repo() { + let repo = RepoConfig { + auth: Some(AgentAuthMode::None), + ..Default::default() + }; + let ec = make_effective( + FlagConfig::default(), + EnvSnapshot::empty(), + repo, + GlobalConfig::default(), + ); + assert_eq!( + ec.auth_mode(), + AgentAuthMode::None, + "none set in repo config must be effective" + ); + } + + #[test] + fn auth_mode_keychain_explicitly_set_in_repo() { + let repo = RepoConfig { + auth: Some(AgentAuthMode::Keychain), + ..Default::default() + }; + let ec = make_effective( + FlagConfig::default(), + EnvSnapshot::empty(), + repo, + GlobalConfig::default(), + ); + assert_eq!(ec.auth_mode(), AgentAuthMode::Keychain); + } } diff --git a/src/data/config/mod.rs b/src/data/config/mod.rs index c53e85e7..75da6d05 100644 --- a/src/data/config/mod.rs +++ b/src/data/config/mod.rs @@ -12,7 +12,8 @@ pub use env::{Env, EnvSnapshot}; pub use flags::FlagConfig; pub use global::GlobalConfig; pub use repo::{ - ApiConfig, RemoteConfig, RepoConfig, WorkItemsConfig, REPO_CONFIG_FILENAME, REPO_CONFIG_SUBDIR, + AgentAuthMode, ApiConfig, RemoteConfig, RepoConfig, WorkItemsConfig, REPO_CONFIG_FILENAME, + REPO_CONFIG_SUBDIR, }; /// Built-in default number of scrollback lines for the container terminal emulator. diff --git a/src/data/config/repo.rs b/src/data/config/repo.rs index eb205253..fe3e1a6f 100644 --- a/src/data/config/repo.rs +++ b/src/data/config/repo.rs @@ -42,6 +42,29 @@ pub struct ApiConfig { pub always_non_interactive: Option, } +/// Per-repo credential injection mode. +/// +/// Controls how awman injects credentials into agent containers. The default +/// (`keychain`) injects whatever the host keychain resolves, subject to +/// injection-time deduplication (Part A). `passthrough` skips keychain +/// injection entirely — the harness must supply credentials via `env(VAR)` +/// overlays. `none` disables KEYCHAIN credential injection; declared `env()` +/// overlays still apply. +/// +/// Resolved through `EffectiveConfig::auth_mode()`. +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum AgentAuthMode { + /// Inject host keychain credentials (with injection-time dedup). Default. + #[default] + Keychain, + /// Skip KEYCHAIN injection; rely on repo-declared `env()` overlays. + /// Declared `env()` overlays still apply. + Passthrough, + /// No KEYCHAIN credential injection; declared `env()` overlays still apply. + None, +} + /// Work-items configuration nested within `RepoConfig`. #[derive(Debug, Default, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct WorkItemsConfig { @@ -79,6 +102,10 @@ pub struct RepoConfig { pub base_image: Option, #[serde(rename = "dockerfile", skip_serializing_if = "Option::is_none")] pub dockerfile: Option, + /// Credential injection mode for agent containers. See [`AgentAuthMode`]. + /// Resolved per-repo; `keychain` (default) injects host keychain creds. + #[serde(rename = "auth", skip_serializing_if = "Option::is_none")] + pub auth: Option, } impl RepoConfig { diff --git a/src/engine/auth/mod.rs b/src/engine/auth/mod.rs index 75d31156..991379c0 100644 --- a/src/engine/auth/mod.rs +++ b/src/engine/auth/mod.rs @@ -9,6 +9,7 @@ use ring::digest; use ring::rand::{SecureRandom, SystemRandom}; use subtle::ConstantTimeEq; +use crate::data::config::repo::AgentAuthMode; use crate::data::fs::api_paths::ApiPaths; use crate::data::fs::auth_paths::AuthPathResolver; use crate::data::session::{AgentName, Session}; @@ -16,6 +17,35 @@ use crate::engine::error::EngineError; pub mod keychain; +/// Map an awman credential env-var name to its well-known service name, or +/// `None` when awman has no mapping for it. +/// +/// This is the canonical source of the credential→service table. The Docker +/// Sandbox driver ([`crate::engine::sandbox::dsbx::auth`]) re-exports this +/// function rather than maintaining its own copy, so all runtimes share one +/// definition. Callers that need to detect "does this declared env var cover +/// the same auth service as a keychain credential?" use this function to +/// compare services by name rather than by hardcoded var pairs. +/// +/// Service names match the Docker Sandboxes well-known service table at +/// docs.docker.com/ai/sandboxes/security/credentials/ (June 2026). +pub fn service_for_credential(key: &str) -> Option<&'static str> { + match key { + // Anthropic — API key (all runtimes) and OAuth token (container runtimes; + // the sbx driver skips the OAuth token for a separate reason: sbx has no + // OAuth support yet, tracked at docker/sbx-releases#11). Both vars + // authenticate the same provider so both map to the same service here. + "ANTHROPIC_API_KEY" | "CLAUDE_CODE_OAUTH_TOKEN" => Some("anthropic"), + "OPENAI_API_KEY" => Some("openai"), + "GH_TOKEN" | "GITHUB_TOKEN" => Some("github"), + "GEMINI_API_KEY" | "GOOGLE_API_KEY" => Some("google"), + "AWS_ACCESS_KEY_ID" | "AWS_SECRET_ACCESS_KEY" => Some("aws"), + "GROQ_API_KEY" => Some("groq"), + "MISTRAL_API_KEY" => Some("mistral"), + _ => None, + } +} + /// Status of an agent's host-side credential discovery. #[derive(Debug, Clone, PartialEq, Eq)] pub struct AgentCredentialStatus { @@ -137,17 +167,27 @@ impl AuthEngine { }) } - /// Composite resolver: keychain credentials scoped to the per-repo config. + /// Composite resolver: keychain credentials gated on the repo `auth` mode. /// - /// The decision to *use* keychain credentials silently vs prompting is a - /// Layer 2 concern (governed by `auto_agent_auth_accepted`). This method - /// only resolves the credentials. + /// - `keychain` (default) — inject host keychain credentials. Part-A dedup + /// is applied later at `ResolvedContainerOptions::resolve` time. + /// - `passthrough` — skip keychain injection entirely; the repo must + /// supply credentials via `env(VAR)` overlays. Declared `env()` overlays + /// still apply. + /// - `none` — no KEYCHAIN credential injection; declared `env()` overlays + /// still apply. + /// + /// The Layer-2 `auto_agent_auth_accepted` consent flag is a separate concern; + /// this method only resolves *which* credentials to inject. pub fn resolve_agent_auth( &self, - _session: &Session, + session: &Session, agent: &AgentName, ) -> Result { - self.agent_keychain_credentials(agent) + match session.effective_config().auth_mode() { + AgentAuthMode::Keychain => self.agent_keychain_credentials(agent), + AgentAuthMode::Passthrough | AgentAuthMode::None => Ok(AgentCredentials::default()), + } } // ── API-key lifecycle ────────────────────────────────────────────────── @@ -632,4 +672,131 @@ mod tests { let outcome = e.verify_api_key(&key).unwrap(); assert_eq!(outcome, AuthOutcome::Authorized); } + + // ── Part B: resolve_agent_auth auth-mode gating ─────────────────────────── + + use crate::data::config::env::AWMAN_CONFIG_HOME; + use crate::data::config::repo::{ + AgentAuthMode, RepoConfig, REPO_CONFIG_FILENAME, REPO_CONFIG_SUBDIR, + }; + use crate::data::config::EnvSnapshot; + use crate::data::session::{Session, SessionOpenOptions}; + + /// Build an isolated session with the given `RepoConfig` written to disk. + /// + /// Uses a temp home dir (via `AWMAN_CONFIG_HOME`) so no process-global env + /// is read or mutated. + fn session_with_repo_config( + git_root: &std::path::Path, + home_dir: &std::path::Path, + repo_cfg: &RepoConfig, + ) -> Session { + // Write repo config. + let awman_dir = git_root.join(REPO_CONFIG_SUBDIR); + std::fs::create_dir_all(&awman_dir).unwrap(); + let cfg_json = serde_json::to_string_pretty(repo_cfg).unwrap(); + std::fs::write(awman_dir.join(REPO_CONFIG_FILENAME), cfg_json).unwrap(); + + let env = EnvSnapshot::with_overrides([(AWMAN_CONFIG_HOME, home_dir.to_str().unwrap())]); + + let resolver = crate::data::session::StaticGitRootResolver::new(git_root); + Session::open( + git_root.to_path_buf(), + &resolver, + SessionOpenOptions { + env: Some(env), + ..Default::default() + }, + ) + .unwrap() + } + + /// `passthrough` mode: `resolve_agent_auth` returns an empty credential set + /// without touching the host keychain (hermetic even on macOS). + #[test] + fn passthrough_mode_yields_empty_credentials() { + let git_tmp = tempfile::tempdir().unwrap(); + let home_tmp = tempfile::tempdir().unwrap(); + let session = session_with_repo_config( + git_tmp.path(), + home_tmp.path(), + &RepoConfig { + auth: Some(AgentAuthMode::Passthrough), + ..Default::default() + }, + ); + let engine = engine_with(home_tmp.path(), home_tmp.path()); + let agent = crate::data::session::AgentName::new("claude").unwrap(); + let creds = engine.resolve_agent_auth(&session, &agent).unwrap(); + assert!( + creds.env_vars.is_empty(), + "passthrough mode must return no keychain credentials; got: {:?}", + creds.env_vars + ); + } + + /// `none` mode: same result — no credential injection. + #[test] + fn none_mode_yields_empty_credentials() { + let git_tmp = tempfile::tempdir().unwrap(); + let home_tmp = tempfile::tempdir().unwrap(); + let session = session_with_repo_config( + git_tmp.path(), + home_tmp.path(), + &RepoConfig { + auth: Some(AgentAuthMode::None), + ..Default::default() + }, + ); + let engine = engine_with(home_tmp.path(), home_tmp.path()); + let agent = crate::data::session::AgentName::new("claude").unwrap(); + let creds = engine.resolve_agent_auth(&session, &agent).unwrap(); + assert!( + creds.env_vars.is_empty(), + "none mode must return no keychain credentials; got: {:?}", + creds.env_vars + ); + } + + /// `keychain` explicitly set: same as default — attempt keychain lookup + /// (may be empty on CI without a keychain; the important thing is that it + /// does NOT return early with empty credentials the way passthrough/none do). + /// We verify via the successful `Ok` return, not the credential count. + #[test] + fn keychain_mode_attempts_keychain_lookup() { + let git_tmp = tempfile::tempdir().unwrap(); + let home_tmp = tempfile::tempdir().unwrap(); + let session = session_with_repo_config( + git_tmp.path(), + home_tmp.path(), + &RepoConfig { + auth: Some(AgentAuthMode::Keychain), + ..Default::default() + }, + ); + let engine = engine_with(home_tmp.path(), home_tmp.path()); + let agent = crate::data::session::AgentName::new("claude").unwrap(); + // Must not error regardless of whether the keychain has credentials. + let result = engine.resolve_agent_auth(&session, &agent); + assert!( + result.is_ok(), + "keychain mode must not error when keychain is absent; got: {result:?}" + ); + } + + /// Default (no `auth` field in repo config): same as `keychain`. + #[test] + fn default_mode_behaves_like_keychain() { + let git_tmp = tempfile::tempdir().unwrap(); + let home_tmp = tempfile::tempdir().unwrap(); + let session = + session_with_repo_config(git_tmp.path(), home_tmp.path(), &RepoConfig::default()); + let engine = engine_with(home_tmp.path(), home_tmp.path()); + let agent = crate::data::session::AgentName::new("claude").unwrap(); + let result = engine.resolve_agent_auth(&session, &agent); + assert!( + result.is_ok(), + "default mode must not error; got: {result:?}" + ); + } } diff --git a/src/engine/container/options.rs b/src/engine/container/options.rs index 54db5cf6..34c9f511 100644 --- a/src/engine/container/options.rs +++ b/src/engine/container/options.rs @@ -196,6 +196,93 @@ pub enum ContainerOption { }, } +/// Injection-time dedup: drop any entry from `agent_credentials` whose +/// credential key maps to the same provider service as a key already declared +/// in `env_passthrough` or `env_literal`, **and whose host value is actually +/// resolvable**. +/// +/// Mirrors the rationale of the sbx path's `CLAUDE_CODE_OAUTH_TOKEN` silent +/// skip ([`crate::engine::sandbox::dsbx::auth::inject_credentials`]): +/// when the harness has **declared** an env var (via `env(VAR)`) that already +/// authenticates the same provider, the keychain OAuth token is redundant and +/// its presence causes the container to receive two conflicting credentials for +/// the same service. +/// +/// An `env_passthrough` entry only counts as "covering" a service when its host +/// value is actually set — mirroring `build_run_argv`'s own emission gate +/// (`if let Ok(value) = std::env::var(name)`). A declared but unset passthrough +/// var does NOT suppress a keychain credential, because the passthrough will emit +/// nothing and the container would otherwise receive zero credentials for that +/// service. `env_literal` entries always carry a value and always count. +/// +/// `lookup_env` abstracts the host env lookup so that callers in tests can +/// supply a hermetic closure instead of reading `std::env::var` directly. +/// +/// If dedup would drop the **last** remaining credential for a service, a +/// `log::warn!` is emitted so the situation is never silent. +/// +/// Example: harness declares `env(ANTHROPIC_API_KEY)` → service "anthropic"; +/// keychain resolves `CLAUDE_CODE_OAUTH_TOKEN` → also service "anthropic"; +/// host has `ANTHROPIC_API_KEY` set → the passthrough will be emitted → +/// `CLAUDE_CODE_OAUTH_TOKEN` is dropped from `agent_credentials`. +/// +/// Counter-example: same declaration but `ANTHROPIC_API_KEY` is NOT set on the +/// host → the passthrough emits nothing → `CLAUDE_CODE_OAUTH_TOKEN` is retained. +pub(crate) fn dedup_credentials_by_declared_env( + agent_credentials: &mut Vec<(String, String)>, + env_passthrough: &[EnvVar], + env_literal: &[EnvLiteral], + lookup_env: &dyn Fn(&str) -> Option, +) { + // Collect the set of provider services already covered by declared env vars + // that will actually be emitted to the container: + // - env_passthrough: only when the host value is set (and non-empty). + // - env_literal: always (they carry an explicit value). + let covered_services: Vec<&'static str> = env_passthrough + .iter() + .filter(|v| lookup_env(v.0.as_str()).is_some_and(|val| !val.is_empty())) + .map(|v| v.0.as_str()) + .chain(env_literal.iter().map(|l| l.key.as_str())) + .filter_map(crate::engine::auth::service_for_credential) + .collect(); + + if covered_services.is_empty() { + return; + } + + // Before dropping, check whether this dedup would leave any service with + // zero credentials. If so, warn — the outcome is intentional (the literal + // or resolvable passthrough will cover it), but it should never be silent. + for service in &covered_services { + let service_creds_before: Vec<_> = agent_credentials + .iter() + .filter(|(k, _)| crate::engine::auth::service_for_credential(k) == Some(service)) + .collect(); + if !service_creds_before.is_empty() { + tracing::warn!( + dropped_keys = ?service_creds_before + .iter() + .map(|(k, _)| k.as_str()) + .collect::>(), + service = service, + "awman: dropping keychain credential(s) for service because the repo \ + declared an env var that covers the same provider; the container will \ + receive credentials via the declared env overlay.", + ); + } + } + + // Retain only credentials whose service is NOT already covered by a + // harness-declared env var that will be emitted to the container. + agent_credentials.retain(|(key, _)| { + match crate::engine::auth::service_for_credential(key) { + Some(service) => !covered_services.contains(&service), + // Credential with no known service mapping — retain unconditionally. + None => true, + } + }); +} + /// Resolved option bag — all options merged into a single struct that the /// backend consumes. Conflicting options are detected here. #[derive(Debug, Clone, Default)] @@ -247,6 +334,16 @@ impl ResolvedContainerOptions { for opt in options { r.ingest(opt)?; } + // Part A: drop agent_credentials that duplicate a service already covered + // by a harness-declared env var. Applies to ALL container runtimes. + // Production callers pass the real host-env lookup; tests inject a + // hermetic closure to avoid mutating process-global state. + dedup_credentials_by_declared_env( + &mut r.agent_credentials, + &r.env_passthrough, + &r.env_literal, + &|name| std::env::var(name).ok(), + ); r.validate()?; Ok(r) } @@ -397,4 +494,217 @@ mod tests { // Multiple overlay entries accumulate — dedup is caller's responsibility. assert_eq!(resolved.overlays.len(), 3); } + + // ── Part A: injection-time credential dedup (resolve-level, env_literal path) ── + + /// `env_literal` (always-valued) coverage → same-service keychain credential + /// is dropped. This goes through `resolve()` because env_literal entries + /// always have a value — no host lookup needed. + #[test] + fn declared_anthropic_env_literal_drops_oauth_token_from_agent_credentials() { + let resolved = ResolvedContainerOptions::resolve([ + ContainerOption::EnvLiteral(EnvLiteral { + key: "ANTHROPIC_API_KEY".into(), + value: "sk-ant-key-literal".into(), + }), + ContainerOption::AgentCredentials { + env_vars: vec![("CLAUDE_CODE_OAUTH_TOKEN".into(), "sk-ant-oat-secret".into())], + }, + ]) + .expect("resolve must succeed"); + + assert!( + resolved.agent_credentials.is_empty(), + "ANTHROPIC_API_KEY via env_literal must still trigger dedup; \ + got: {:?}", + resolved.agent_credentials + ); + } + + /// When agent_credentials is empty the dedup is a no-op (no panic, no error). + #[test] + fn dedup_with_empty_agent_credentials_is_noop() { + // Use env_literal so the test is hermetic (no host-env lookup). + let resolved = + ResolvedContainerOptions::resolve([ContainerOption::EnvLiteral(EnvLiteral { + key: "ANTHROPIC_API_KEY".into(), + value: "sk-key".into(), + })]) + .expect("resolve must succeed"); + assert!(resolved.agent_credentials.is_empty()); + } + + // ── Part A: dedup_credentials_by_declared_env unit tests (injectable lookup) ── + // + // All of these pass a hermetic closure as `lookup_env` so no process-global + // std::env mutation is needed. The closure mimics the subset of env vars + // that would be set on the host in each scenario. + + /// declared env(ANTHROPIC_API_KEY) that IS set on host + keychain OAuth → + /// OAuth dropped (the passthrough will be emitted; no need for two creds). + #[test] + fn dedup_fn_drops_oauth_when_anthropic_passthrough_set_on_host() { + let mut creds = vec![("CLAUDE_CODE_OAUTH_TOKEN".into(), "tok".into())]; + let pt = vec![EnvVar("ANTHROPIC_API_KEY".into())]; + // Simulate: ANTHROPIC_API_KEY is set on the host. + let lookup = |name: &str| -> Option { + if name == "ANTHROPIC_API_KEY" { + Some("sk-ant-api-key".into()) + } else { + None + } + }; + dedup_credentials_by_declared_env(&mut creds, &pt, &[], &lookup); + assert!( + creds.is_empty(), + "OAuth must be dropped when ANTHROPIC_API_KEY is set on host; got: {creds:?}" + ); + } + + /// declared env(ANTHROPIC_API_KEY) that is NOT set on host + keychain OAuth → + /// OAuth retained (passthrough emits nothing; dropping it would leave zero + /// credentials for the 'anthropic' service). + #[test] + fn dedup_fn_retains_oauth_when_anthropic_passthrough_declared_but_unset() { + let mut creds = vec![("CLAUDE_CODE_OAUTH_TOKEN".into(), "tok".into())]; + let pt = vec![EnvVar("ANTHROPIC_API_KEY".into())]; + // Simulate: ANTHROPIC_API_KEY is NOT set on the host. + let lookup = |_name: &str| -> Option { None }; + dedup_credentials_by_declared_env(&mut creds, &pt, &[], &lookup); + assert_eq!( + creds.len(), + 1, + "OAuth must be retained when the declared passthrough var is unset on host; \ + got: {creds:?}" + ); + } + + /// No declared anthropic var → cloud harness path — keychain OAuth is + /// retained regardless of host env. + #[test] + fn dedup_fn_retains_oauth_when_no_anthropic_declared() { + let mut creds = vec![("CLAUDE_CODE_OAUTH_TOKEN".into(), "tok".into())]; + let pt = vec![EnvVar("OPENAI_API_KEY".into())]; // covers openai, not anthropic + // Even if OPENAI_API_KEY is set, it doesn't cover the anthropic service. + let lookup = |name: &str| -> Option { + if name == "OPENAI_API_KEY" { + Some("sk-openai".into()) + } else { + None + } + }; + dedup_credentials_by_declared_env(&mut creds, &pt, &[], &lookup); + assert_eq!( + creds.len(), + 1, + "no anthropic declared → OAuth must be retained" + ); + } + + /// env_literal (always-valued) coverage → same-service credential dropped. + #[test] + fn dedup_fn_handles_env_literal_source() { + let mut creds = vec![("CLAUDE_CODE_OAUTH_TOKEN".into(), "tok".into())]; + let lit = vec![EnvLiteral { + key: "ANTHROPIC_API_KEY".into(), + value: "literal-key".into(), + }]; + // lookup_env is irrelevant for literals but must be provided. + let lookup = |_: &str| -> Option { None }; + dedup_credentials_by_declared_env(&mut creds, &[], &lit, &lookup); + assert!( + creds.is_empty(), + "env_literal coverage must also trigger dedup" + ); + } + + /// No declared vars → no dedup, regardless of host env. + #[test] + fn dedup_fn_retains_credential_when_no_declared_vars() { + let mut creds = vec![("CLAUDE_CODE_OAUTH_TOKEN".into(), "tok".into())]; + let lookup = |_: &str| -> Option { None }; + dedup_credentials_by_declared_env(&mut creds, &[], &[], &lookup); + assert_eq!(creds.len(), 1, "no declared vars → no dedup"); + } + + /// A credential with no known service mapping is never dropped by the dedup. + #[test] + fn dedup_fn_unmapped_credential_is_never_dropped() { + let mut creds = vec![ + ("CLAUDE_CODE_OAUTH_TOKEN".into(), "tok".into()), + ("MY_CUSTOM_INTERNAL_TOKEN".into(), "custom".into()), + ]; + let pt = vec![EnvVar("ANTHROPIC_API_KEY".into())]; + let lookup = |name: &str| -> Option { + if name == "ANTHROPIC_API_KEY" { + Some("sk-ant".into()) + } else { + None + } + }; + dedup_credentials_by_declared_env(&mut creds, &pt, &[], &lookup); + // OAuth (anthropic) dropped; custom (no mapping) retained. + assert!( + !creds.iter().any(|(k, _)| k == "CLAUDE_CODE_OAUTH_TOKEN"), + "OAuth must be dropped when anthropic passthrough is set" + ); + assert!( + creds.iter().any(|(k, _)| k == "MY_CUSTOM_INTERNAL_TOKEN"), + "unmapped credential must survive dedup; got: {creds:?}" + ); + } + + /// When keychain credential's OWN name equals the declared passthrough var + /// (e.g. declared env(ANTHROPIC_API_KEY) + keychain returns ANTHROPIC_API_KEY + /// itself), the keychain entry is dropped because both map to service + /// "anthropic" and the passthrough is set on host. + #[test] + fn dedup_fn_drops_when_credential_name_equals_declared_var() { + let mut creds = vec![("ANTHROPIC_API_KEY".into(), "sk-ant-from-keychain".into())]; + let pt = vec![EnvVar("ANTHROPIC_API_KEY".into())]; + // Passthrough is set on host (perhaps to a different value). + let lookup = |name: &str| -> Option { + if name == "ANTHROPIC_API_KEY" { + Some("sk-ant-from-env".into()) + } else { + None + } + }; + dedup_credentials_by_declared_env(&mut creds, &pt, &[], &lookup); + assert!( + creds.is_empty(), + "credential whose own name equals the declared var must be dropped; \ + got: {creds:?}" + ); + } + + /// Multi-service: declared var covers service X (anthropic), credential for + /// service Y (openai) is retained. + #[test] + fn dedup_fn_multi_service_declared_x_retains_credential_for_y() { + let mut creds = vec![ + ("CLAUDE_CODE_OAUTH_TOKEN".into(), "tok-anthropic".into()), + ("OPENAI_API_KEY".into(), "tok-openai".into()), + ]; + let pt = vec![EnvVar("ANTHROPIC_API_KEY".into())]; + // Only anthropic passthrough is set on host. + let lookup = |name: &str| -> Option { + if name == "ANTHROPIC_API_KEY" { + Some("sk-ant".into()) + } else { + None + } + }; + dedup_credentials_by_declared_env(&mut creds, &pt, &[], &lookup); + // Anthropic OAuth dropped; OpenAI retained (different service, not declared). + assert!( + !creds.iter().any(|(k, _)| k == "CLAUDE_CODE_OAUTH_TOKEN"), + "anthropic OAuth must be dropped" + ); + assert!( + creds.iter().any(|(k, _)| k == "OPENAI_API_KEY"), + "openai credential must be retained (covers service 'openai', not declared); \ + got: {creds:?}" + ); + } } diff --git a/src/engine/sandbox/dsbx/auth.rs b/src/engine/sandbox/dsbx/auth.rs index 4b1a5792..805c4876 100644 --- a/src/engine/sandbox/dsbx/auth.rs +++ b/src/engine/sandbox/dsbx/auth.rs @@ -17,22 +17,10 @@ use crate::engine::container::options::{EnvLiteral, EnvVar}; use crate::engine::error::EngineError; use crate::engine::sandbox::dsbx::spawn::SbxCommand; -/// Map an awman credential env-var name to its `sbx` well-known service name, -/// or `None` when awman has no mapping for it. Matches the service table in -/// the Docker Sandboxes credentials docs -/// (docs.docker.com/ai/sandboxes/security/credentials/, June 2026). -pub(super) fn service_for_credential(key: &str) -> Option<&'static str> { - match key { - "ANTHROPIC_API_KEY" => Some("anthropic"), - "OPENAI_API_KEY" => Some("openai"), - "GH_TOKEN" | "GITHUB_TOKEN" => Some("github"), - "GEMINI_API_KEY" | "GOOGLE_API_KEY" => Some("google"), - "AWS_ACCESS_KEY_ID" | "AWS_SECRET_ACCESS_KEY" => Some("aws"), - "GROQ_API_KEY" => Some("groq"), - "MISTRAL_API_KEY" => Some("mistral"), - _ => None, - } -} +/// Re-export the shared service→credential mapping so the dsbx module (and its +/// tests via `use super::*`) can use it without a long path. The canonical +/// definition lives in [`crate::engine::auth::service_for_credential`]. +pub(super) use crate::engine::auth::service_for_credential; /// Allowlist of provider auth env vars accepted via `env(VAR)` overlays for /// launch-time auto-auth, per agent. Only mixin-kit agents participate — @@ -368,9 +356,12 @@ fn listing_mentions_service(stdout: &str, service: &str) -> bool { /// `CLAUDE_CODE_OAUTH_TOKEN` is the one silent exception: the keychain /// resolver always surfaces it when the user is logged in to Claude Code, but /// sbx has no OAuth-token support yet (docker/sbx-releases#11), so it can -/// never be injected. Auth still works without it — via an `ANTHROPIC_API_KEY` -/// env() overlay ([`auto_auth_env_overlays`]) or `/login` inside the sandbox — -/// and the missing-auth case already gets its own warning from +/// never be injected. The service mapping in [`service_for_credential`] maps +/// it to "anthropic" for dedup purposes on other runtimes — but the sbx +/// driver skips it unconditionally (key-name check before the service lookup). +/// Auth still works without it — via an `ANTHROPIC_API_KEY` env() overlay +/// ([`auto_auth_env_overlays`]) or `/login` inside the sandbox — and the +/// missing-auth case already gets its own warning from /// [`auto_auth_env_overlays`], so warning here on every launch is pure noise. pub(super) fn inject_credentials( creds: &[(String, String)], @@ -378,13 +369,18 @@ pub(super) fn inject_credentials( sink: &mut dyn UserMessageSink, ) -> Result<(), EngineError> { for (key, value) in creds { + // Sbx has no OAuth-token support yet; skip unconditionally. See the + // doc comment above. Key-name check takes precedence over the service + // mapping so this remains correct even though service_for_credential + // now maps CLAUDE_CODE_OAUTH_TOKEN → "anthropic" for dedup use by + // other runtimes. + if key == "CLAUDE_CODE_OAUTH_TOKEN" { + continue; + } match service_for_credential(key) { Some(service) => { set_secret(service, value, sandbox, sink)?; } - None if key == "CLAUDE_CODE_OAUTH_TOKEN" => { - // Silently skipped — see the doc comment above. - } None => { sink.write_message(crate::data::message::UserMessage { level: crate::data::message::MessageLevel::Warning, @@ -411,6 +407,13 @@ mod tests { service_for_credential("ANTHROPIC_API_KEY"), Some("anthropic") ); + // OAuth token maps to anthropic for injection-time dedup on container + // runtimes. The sbx driver skips it via a key-name guard before + // reaching this table (sbx has no OAuth support yet). + assert_eq!( + service_for_credential("CLAUDE_CODE_OAUTH_TOKEN"), + Some("anthropic") + ); assert_eq!(service_for_credential("OPENAI_API_KEY"), Some("openai")); assert_eq!(service_for_credential("GH_TOKEN"), Some("github")); assert_eq!(service_for_credential("GITHUB_TOKEN"), Some("github")); @@ -534,6 +537,9 @@ mod tests { fn all_credential_table_entries_have_expected_service_names() { let table: &[(&str, &str)] = &[ ("ANTHROPIC_API_KEY", "anthropic"), + // OAuth token: maps to anthropic for dedup; sbx skips it via + // key-name guard before reaching this table. + ("CLAUDE_CODE_OAUTH_TOKEN", "anthropic"), ("OPENAI_API_KEY", "openai"), ("GH_TOKEN", "github"), ("GITHUB_TOKEN", "github"), diff --git a/src/frontend/tui/render.rs b/src/frontend/tui/render.rs index b328e4de..5c56483e 100644 --- a/src/frontend/tui/render.rs +++ b/src/frontend/tui/render.rs @@ -1187,7 +1187,7 @@ fn render_dialog(dialog: &dialogs::Dialog, area: Rect, frame: &mut Frame) { let visible = list_h as usize; let start = selected .saturating_sub(visible.saturating_sub(1)) - .min(items.len().saturating_sub(visible).max(0)); + .min(items.len().saturating_sub(visible)); let lines: Vec = items .iter() .enumerate()