diff --git a/lib/apps/fabro-cli/src/commands/run/runner.rs b/lib/apps/fabro-cli/src/commands/run/runner.rs index 4e6b72b8b..45c2dcecf 100644 --- a/lib/apps/fabro-cli/src/commands/run/runner.rs +++ b/lib/apps/fabro-cli/src/commands/run/runner.rs @@ -137,13 +137,10 @@ pub(crate) async fn execute( if let Some(control_manager) = &mut control_manager { control_manager.wait_for_first_connection().await?; } - let vault = load_worker_vault(storage_dir.as_deref()).await?; + let vault = load_worker_vault(storage_dir.as_deref(), &run_dir).await?; let github_app = { - let vault_guard = match &vault { - Some(arc) => Some(arc.read().await), - None => None, - }; - maybe_build_github_credentials(&run_spec.settings, vault_guard.as_deref())? + let vault_guard = vault.read().await; + maybe_build_github_credentials(&run_spec.settings, &vault_guard)? }; let services = StartServices { run_id, @@ -272,10 +269,23 @@ impl fabro_tool::RunManifestBuilder for WorkerRunManifestBuilder { } } -async fn load_worker_vault(storage_dir: Option<&Path>) -> Result>>> { - let Some(storage_dir) = storage_dir else { - return Ok(None); - }; +/// Load the worker's secret vault from the run's storage root. +/// +/// A worker always runs against a server-created run, which lives under +/// `/scratch/`, so the storage root is always resolvable. Failing +/// here is better than continuing without a vault: credentials would silently +/// fall back to whatever the worker process happens to have in its environment. +async fn load_worker_vault( + storage_dir: Option<&Path>, + run_dir: &Path, +) -> Result>> { + let storage_dir = storage_dir.with_context(|| { + format!( + "run worker for {} was spawned without --storage-dir; it needs the storage root to \ + load its secret vault", + run_dir.display() + ) + })?; let storage = Storage::new(storage_dir); let vault = SecretStore::open_snapshot(storage.sqlite_path(), storage.secrets_path()) @@ -287,7 +297,7 @@ async fn load_worker_vault(storage_dir: Option<&Path>) -> Result RunEvent { fn maybe_build_github_credentials( settings: &WorkflowSettings, - vault: Option<&fabro_vault::Vault>, + vault: &fabro_vault::Vault, ) -> Result> { let resolved_run = &settings.run; let resolved_server = ServerSettingsBuilder::load_default().ok(); @@ -1747,7 +1757,9 @@ mod tests { .set("ANTHROPIC_API_KEY", "vault-key", SecretType::Token, None) .unwrap(); - let loaded = load_worker_vault(Some(temp.path())).await.unwrap().unwrap(); + let loaded = load_worker_vault(Some(temp.path()), temp.path()) + .await + .unwrap(); let guard = loaded.read().await; let credential = guard.get("ANTHROPIC_API_KEY").unwrap(); diff --git a/lib/apps/fabro-cli/src/shared/github.rs b/lib/apps/fabro-cli/src/shared/github.rs index 731467d1d..f6e6ec9f0 100644 --- a/lib/apps/fabro-cli/src/shared/github.rs +++ b/lib/apps/fabro-cli/src/shared/github.rs @@ -8,7 +8,7 @@ pub(crate) fn build_github_credentials( strategy: GithubIntegrationStrategy, app_id: Option<&str>, app_slug: Option<&str>, - vault: Option<&Vault>, + vault: &Vault, ) -> anyhow::Result> { match strategy { GithubIntegrationStrategy::App => { @@ -31,7 +31,7 @@ pub(crate) fn build_github_credentials( /// Look up GitHub token: GITHUB_TOKEN env -> vault GITHUB_TOKEN -> GH_TOKEN env /// -> vault GH_TOKEN -fn lookup_github_token(vault: Option<&Vault>) -> Option { +fn lookup_github_token(vault: &Vault) -> Option { lookup_env_or_vault(EnvVars::GITHUB_TOKEN, vault) .or_else(|| lookup_env_or_vault(EnvVars::GH_TOKEN, vault)) } @@ -40,10 +40,10 @@ fn lookup_github_token(vault: Option<&Vault>) -> Option { clippy::disallowed_methods, reason = "GitHub credential resolution intentionally falls back from vault to documented process-env names." )] -fn lookup_env_or_vault(name: &str, vault: Option<&Vault>) -> Option { +fn lookup_env_or_vault(name: &str, vault: &Vault) -> Option { std::env::var(name) .ok() - .or_else(|| vault.and_then(|v| v.get(name).map(str::to_string))) + .or_else(|| vault.get(name).map(str::to_string)) .map(|t| t.trim().to_string()) .filter(|t| !t.is_empty()) } diff --git a/lib/apps/fabro-cli/tests/it/cmd/runner.rs b/lib/apps/fabro-cli/tests/it/cmd/runner.rs index 6828b6251..5bce373f8 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/runner.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/runner.rs @@ -71,6 +71,11 @@ fn spawn_worker_process( ); cmd.args([ "__run-worker", + "--storage-dir", + context + .storage_dir + .to_str() + .expect("storage directory path should be valid UTF-8"), "--server", server, "--run-dir", @@ -215,6 +220,11 @@ fn worker_requires_fabro_worker_token_env() { .command() .args([ "__run-worker", + "--storage-dir", + context + .storage_dir + .to_str() + .expect("storage directory path should be valid UTF-8"), "--server", "http://127.0.0.1:32276", "--run-dir", @@ -273,6 +283,11 @@ digraph CachedGraph { let output = worker_command(&context, run_id.as_str()) .args([ "__run-worker", + "--storage-dir", + context + .storage_dir + .to_str() + .expect("storage directory path should be valid UTF-8"), "--server", server.as_str(), "--run-dir", @@ -342,6 +357,11 @@ digraph GitHubApp { cmd.env("GITHUB_APP_PRIVATE_KEY", "%%%not-base64%%%"); cmd.args([ "__run-worker", + "--storage-dir", + context + .storage_dir + .to_str() + .expect("storage directory path should be valid UTF-8"), "--server", server.as_str(), "--run-dir", @@ -391,6 +411,11 @@ digraph DetachedStoreOnly { let output = worker_command(&context, run_id.as_str()) .args([ "__run-worker", + "--storage-dir", + context + .storage_dir + .to_str() + .expect("storage directory path should be valid UTF-8"), "--server", server.as_str(), "--run-dir", @@ -609,6 +634,11 @@ digraph Test { let mut cmd = worker_command(&context, &run_id); cmd.args([ "__run-worker", + "--storage-dir", + context + .storage_dir + .to_str() + .expect("storage directory path should be valid UTF-8"), "--server", &server, "--run-dir", @@ -674,6 +704,11 @@ fn runner_reports_malformed_run_state_without_prefetching_events() { let output = worker_command(&context, &run_id) .args([ "__run-worker", + "--storage-dir", + context + .storage_dir + .to_str() + .expect("storage directory path should be valid UTF-8"), "--server", &format!("{}/api/v1", server.base_url()), "--run-dir", diff --git a/lib/apps/fabro-server/Cargo.toml b/lib/apps/fabro-server/Cargo.toml index 1f2c5bb61..e47243f32 100644 --- a/lib/apps/fabro-server/Cargo.toml +++ b/lib/apps/fabro-server/Cargo.toml @@ -108,6 +108,7 @@ fabro-build-support = { path = "../../foundation/build-support" } chrono = { workspace = true } [dev-dependencies] +fabro-auth = { path = "../../foundation/fabro-auth", features = ["test-support"] } tokio = { workspace = true, features = ["test-util", "macros"] } tower = "0.5" http-body-util = "0.1" diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index 11a9a0948..daeadaea2 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -4115,7 +4115,7 @@ async fn execute_run_in_process(state: Arc, run_id: RunId) { run_control: None, github_app, github_permissions, - vault: Some(Arc::new(AsyncRwLock::new(vault.into_vault()))), + vault: Arc::new(AsyncRwLock::new(vault.into_vault())), catalog: state.catalog(), on_node: None, registry_override, diff --git a/lib/apps/fabro-server/tests/it/scenario/run_completion.rs b/lib/apps/fabro-server/tests/it/scenario/run_completion.rs index a0e40e06c..8c7e5aeb1 100644 --- a/lib/apps/fabro-server/tests/it/scenario/run_completion.rs +++ b/lib/apps/fabro-server/tests/it/scenario/run_completion.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use axum::body::Body; use axum::http::{Request, StatusCode}; -use fabro_auth::EnvCredentialSource; +use fabro_auth::test_support; use fabro_model::{Catalog, ProviderId}; use fabro_static::EnvVars; use fabro_test::{TwinScenario, TwinScenarios, twin_openai}; @@ -43,12 +43,11 @@ fn test_app_with_openai_agent_backend(openai_base_url: String, api_key: String) ); let source_api_key = api_key.clone(); let env_api_key = api_key.clone(); - let llm_source: Arc = Arc::new( - EnvCredentialSource::with_env_lookup(Arc::new(move |name| match name { + let llm_source: Arc = + test_support::env_credential_source(move |name| match name { "OPENAI_API_KEY" => Some(source_api_key.clone()), _ => None, - })), - ); + }); let state = fabro_server::test_support::TestAppStateBuilder::new() .runtime_settings(settings.server_settings, settings.manifest_run_defaults) .max_concurrent_runs(5) diff --git a/lib/components/fabro-agent/Cargo.toml b/lib/components/fabro-agent/Cargo.toml index 07f8497fc..2e9a299b6 100644 --- a/lib/components/fabro-agent/Cargo.toml +++ b/lib/components/fabro-agent/Cargo.toml @@ -59,6 +59,7 @@ htmd = "0.5" libc = "0.2" [dev-dependencies] +fabro-auth = { path = "../../foundation/fabro-auth", features = ["test-support"] } insta.workspace = true tokio = { workspace = true, features = ["test-util", "macros"] } tempfile = "3" diff --git a/lib/components/fabro-agent/tests/it/parity_matrix.rs b/lib/components/fabro-agent/tests/it/parity_matrix.rs index e37a29003..385bd5275 100644 --- a/lib/components/fabro-agent/tests/it/parity_matrix.rs +++ b/lib/components/fabro-agent/tests/it/parity_matrix.rs @@ -13,7 +13,7 @@ use fabro_agent::{ AgentEvent, AgentProfile, AgentProfileBuilder, LocalSandbox, OpenAiProfile, Session, SessionOptions, SubAgentSupervisor, ToolSecrets, WebFetchSummarizer, }; -use fabro_auth::EnvCredentialSource; +use fabro_auth::test_support; use fabro_llm::client::Client; use fabro_llm::provider::ProviderAdapter; use fabro_llm::providers::{OpenAiAdapter, OpenAiCompatibleAdapter}; @@ -132,7 +132,7 @@ async fn make_client(provider: &Provider, twin: Option<&OpenAiTwinOptions>) -> C return make_twin_client(twin.expect("openai twin config should be provided")); } - let source = EnvCredentialSource::new(); + let source = test_support::StubCredentialSource; let catalog = Arc::new(Catalog::from_builtin().expect("default catalog should build")); Client::from_source(&source, catalog) .await diff --git a/lib/components/fabro-hooks/Cargo.toml b/lib/components/fabro-hooks/Cargo.toml index 02f25fdad..2c1a68422 100644 --- a/lib/components/fabro-hooks/Cargo.toml +++ b/lib/components/fabro-hooks/Cargo.toml @@ -30,6 +30,7 @@ tracing.workspace = true tokio-util.workspace = true [dev-dependencies] +fabro-auth = { path = "../../foundation/fabro-auth", features = ["test-support"] } httpmock = "0.8" tokio = { workspace = true, features = ["test-util", "macros"] } toml.workspace = true diff --git a/lib/components/fabro-hooks/src/executor.rs b/lib/components/fabro-hooks/src/executor.rs index 08b39b723..d92f73c8e 100644 --- a/lib/components/fabro-hooks/src/executor.rs +++ b/lib/components/fabro-hooks/src/executor.rs @@ -836,7 +836,7 @@ impl HookExecutor for HookExecutorImpl { #[cfg(test)] mod tests { - use fabro_auth::{CredentialSource, EnvCredentialSource}; + use fabro_auth::{CredentialSource, test_support}; use fabro_types::fixtures; use fabro_util::env::TestEnv; @@ -855,7 +855,7 @@ mod tests { } fn test_llm_source() -> Arc { - Arc::new(EnvCredentialSource::new()) + test_support::vault_only_credential_source() } fn test_catalog() -> Arc { diff --git a/lib/components/fabro-hooks/src/runner.rs b/lib/components/fabro-hooks/src/runner.rs index 9435dba9a..c3ec53987 100644 --- a/lib/components/fabro-hooks/src/runner.rs +++ b/lib/components/fabro-hooks/src/runner.rs @@ -4,7 +4,7 @@ use std::sync::Arc; use fabro_agent::Sandbox; use fabro_auth::CredentialSource; #[cfg(test)] -use fabro_auth::EnvCredentialSource; +use fabro_auth::test_support; use fabro_model::Catalog; use crate::config::{HookDefinition, HookSettings}; @@ -46,7 +46,7 @@ impl HookRunner { Self { config, executor, - llm_source: Arc::new(EnvCredentialSource::new()), + llm_source: test_support::vault_only_credential_source(), catalog: Arc::new(Catalog::from_builtin().expect("default catalog should build")), compiled_matchers, } @@ -238,7 +238,6 @@ impl HookRunner { #[cfg(test)] mod tests { - use fabro_auth::EnvCredentialSource; use fabro_types::fixtures; use super::*; @@ -279,7 +278,7 @@ mod tests { } fn test_llm_source() -> Arc { - Arc::new(EnvCredentialSource::new()) + test_support::vault_only_credential_source() } fn test_catalog() -> Arc { diff --git a/lib/components/fabro-hooks/tests/host_command_hooks.rs b/lib/components/fabro-hooks/tests/host_command_hooks.rs index 4bbe0c5a3..fe73371dd 100644 --- a/lib/components/fabro-hooks/tests/host_command_hooks.rs +++ b/lib/components/fabro-hooks/tests/host_command_hooks.rs @@ -2,7 +2,7 @@ use std::path::Path; use std::sync::Arc; use fabro_agent::{LocalSandbox, Sandbox}; -use fabro_auth::{CredentialSource, EnvCredentialSource}; +use fabro_auth::{CredentialSource, test_support}; use fabro_hooks::{ HookContext, HookDecision, HookDefinition, HookEvent, HookExecutionContext, HookRunner, HookSettings, InterpString, @@ -12,7 +12,7 @@ use fabro_types::RunId; use tokio::fs; fn test_llm_source() -> Arc { - Arc::new(EnvCredentialSource::new()) + test_support::vault_only_credential_source() } fn test_catalog() -> Arc { diff --git a/lib/components/fabro-workflow/Cargo.toml b/lib/components/fabro-workflow/Cargo.toml index 932bb0071..5ce0c0cac 100644 --- a/lib/components/fabro-workflow/Cargo.toml +++ b/lib/components/fabro-workflow/Cargo.toml @@ -14,7 +14,7 @@ readme = "README.md" doctest = false [features] -test-support = [] +test-support = ["fabro-auth/test-support"] [lints] workspace = true @@ -74,6 +74,7 @@ tempfile = "3" toml.workspace = true fabro-vault = { path = "../../foundation/fabro-vault" } [dev-dependencies] +fabro-auth = { path = "../../foundation/fabro-auth", features = ["test-support"] } base64.workspace = true fabro-acp = { path = "../fabro-acp", features = ["test-support"] } fabro-workflow = { path = ".", features = ["test-support"] } diff --git a/lib/components/fabro-workflow/src/handler/llm/api.rs b/lib/components/fabro-workflow/src/handler/llm/api.rs index c4ce3737e..def8369ce 100644 --- a/lib/components/fabro-workflow/src/handler/llm/api.rs +++ b/lib/components/fabro-workflow/src/handler/llm/api.rs @@ -10,7 +10,7 @@ use fabro_agent::{ Sandbox, Session, SessionOptions, SessionShutdownReason, StaticEnvProvider, ToolEnvProvider, ToolSecrets, canonical_tool_name, register_question_tools, }; -use fabro_auth::{CredentialSource, EnvCredentialSource}; +use fabro_auth::CredentialSource; use fabro_graphviz::graph::{AttrValue, Node}; use fabro_llm::client::Client; use fabro_llm::types::{ @@ -695,22 +695,6 @@ impl AgentApiBackend { } } - #[must_use] - pub fn new_from_env( - model: String, - provider_id: impl Into, - fallback_chain: Vec, - steering_hub: Arc, - ) -> Self { - Self::new( - model, - provider_id, - fallback_chain, - Arc::new(EnvCredentialSource::new()), - steering_hub, - ) - } - #[must_use] pub fn with_env(mut self, env: HashMap) -> Self { self.tool_env = Some(Arc::new(StaticEnvProvider(env))); @@ -1718,7 +1702,7 @@ mod tests { use fabro_agent::subagent::SessionFactory; use fabro_agent::{AgentProfile, LocalSandbox, ToolRegistry}; use fabro_api::types; - use fabro_auth::{EnvCredentialSource, VaultCredentialSource}; + use fabro_auth::{VaultCredentialSource, test_support as auth_test_support}; use fabro_llm::provider::{ProviderAdapter, StreamEventStream}; use fabro_llm::{Error as LlmError, ProviderErrorDetail, ProviderErrorKind}; use fabro_tool::FabroToolBackend; @@ -1895,18 +1879,18 @@ reasoning = false } fn mock_api_backend(server: &MockServer) -> AgentApiBackend { - let source = EnvCredentialSource::with_env_lookup(Arc::new(|name| { + let source = auth_test_support::env_credential_source(|name| { if name == "MOCK_API_KEY" { Some("sk-test".to_string()) } else { None } - })); + }); AgentApiBackend::new_with_catalog( "mock-model".to_string(), ProviderId::from("mock"), Vec::new(), - Arc::new(source), + source, SteeringHub::for_tests(), mock_llm_catalog(server), ) @@ -2004,10 +1988,11 @@ reasoning = false #[test] fn agent_backend_stores_config() { - let backend = AgentApiBackend::new_from_env( + let backend = AgentApiBackend::new( "claude-opus-4-6".to_string(), ProviderId::openai(), Vec::new(), + auth_test_support::vault_only_credential_source(), SteeringHub::for_tests(), ); assert_eq!(backend.model, "claude-opus-4-6"); @@ -2016,10 +2001,11 @@ reasoning = false #[test] fn agent_backend_initializes_empty_sessions() { - let backend = AgentApiBackend::new_from_env( + let backend = AgentApiBackend::new( "claude-opus-4-6".to_string(), ProviderId::anthropic(), Vec::new(), + auth_test_support::vault_only_credential_source(), SteeringHub::for_tests(), ); assert!(backend.sessions.lock().unwrap().is_empty()); @@ -2722,7 +2708,7 @@ enabled = true "gpt-5.4".to_string(), ProviderId::from("openrouter"), Vec::new(), - Arc::new(EnvCredentialSource::new()), + auth_test_support::vault_only_credential_source(), SteeringHub::for_tests(), Arc::new(Catalog::from_builtin_with_overrides(&settings).unwrap()), ); @@ -2738,7 +2724,7 @@ enabled = true "gpt-5.4".to_string(), ProviderId::from("openrouter"), Vec::new(), - Arc::new(EnvCredentialSource::new()), + auth_test_support::vault_only_credential_source(), SteeringHub::for_tests(), Arc::new(Catalog::from_builtin().unwrap()), ); @@ -2785,7 +2771,7 @@ reasoning = false "acme-llama".to_string(), ProviderId::from("acme"), Vec::new(), - Arc::new(EnvCredentialSource::new()), + auth_test_support::vault_only_credential_source(), SteeringHub::for_tests(), catalog, ); @@ -2832,7 +2818,7 @@ reasoning = false "acme-claude".to_string(), ProviderId::from("acme"), Vec::new(), - Arc::new(EnvCredentialSource::new()), + auth_test_support::vault_only_credential_source(), SteeringHub::for_tests(), catalog, ); @@ -2857,7 +2843,7 @@ enabled = true "openai/gpt-5.4".to_string(), ProviderId::from("openrouter"), Vec::new(), - Arc::new(EnvCredentialSource::new()), + auth_test_support::vault_only_credential_source(), SteeringHub::for_tests(), catalog, ); @@ -2872,10 +2858,11 @@ enabled = true #[test] fn run_model_controls_apply_when_node_omits_controls() { - let backend = AgentApiBackend::new_from_env( + let backend = AgentApiBackend::new( "gpt-5.4".to_string(), ProviderId::openai(), Vec::new(), + auth_test_support::vault_only_credential_source(), SteeringHub::for_tests(), ) .with_run_model_controls(fabro_types::settings::run::RunModelControls { @@ -2892,10 +2879,11 @@ enabled = true #[test] fn node_controls_override_run_model_controls() { - let backend = AgentApiBackend::new_from_env( + let backend = AgentApiBackend::new( "gpt-5.4".to_string(), ProviderId::openai(), Vec::new(), + auth_test_support::vault_only_credential_source(), SteeringHub::for_tests(), ) .with_run_model_controls(fabro_types::settings::run::RunModelControls { @@ -2920,10 +2908,11 @@ enabled = true #[test] fn omitted_reasoning_effort_stays_unset() { - let backend = AgentApiBackend::new_from_env( + let backend = AgentApiBackend::new( "gpt-5.4".to_string(), ProviderId::openai(), Vec::new(), + auth_test_support::vault_only_credential_source(), SteeringHub::for_tests(), ); let node = Node::new("work"); @@ -2969,10 +2958,11 @@ enabled = true provider: "openai".to_string(), model: "gpt-5.5".to_string(), }]; - let backend = AgentApiBackend::new_from_env( + let backend = AgentApiBackend::new( "claude-fable-5".to_string(), ProviderId::anthropic(), fallback_chain.clone(), + auth_test_support::vault_only_credential_source(), SteeringHub::for_tests(), ); let mut providers = HashMap::new(); @@ -3242,10 +3232,11 @@ enabled = true #[tokio::test] async fn api_backend_shutdown_closes_cached_sessions_once() { - let backend = AgentApiBackend::new_from_env( + let backend = AgentApiBackend::new( "gpt-5.4".to_string(), ProviderId::openai(), Vec::new(), + auth_test_support::vault_only_credential_source(), SteeringHub::for_tests(), ); let emitter = Arc::new(Emitter::new(fabro_types::RunId::new())); diff --git a/lib/components/fabro-workflow/src/lifecycle/git.rs b/lib/components/fabro-workflow/src/lifecycle/git.rs index 4db74c904..21402ec27 100644 --- a/lib/components/fabro-workflow/src/lifecycle/git.rs +++ b/lib/components/fabro-workflow/src/lifecycle/git.rs @@ -593,6 +593,7 @@ mod tests { use anyhow::Result; use async_trait::async_trait; use bytes::Bytes; + use fabro_auth::test_support as auth_test_support; use fabro_core::graph::Graph as CoreGraph; use fabro_core::lifecycle::RunLifecycle; use fabro_core::state::ExecutionState; @@ -1261,7 +1262,7 @@ mod tests { tokio_util::sync::CancellationToken::new(), fabro_model::ProviderId::anthropic(), "claude-sonnet-4-6".to_string(), - Arc::new(fabro_auth::EnvCredentialSource::new()), + auth_test_support::vault_only_credential_source(), Arc::new(Catalog::from_builtin().expect("default catalog should build")), Arc::new(SandboxGitRuntime::new()), Arc::clone(&lifecycle.metadata_runtime), diff --git a/lib/components/fabro-workflow/src/operations/start.rs b/lib/components/fabro-workflow/src/operations/start.rs index 3b560c7cb..eb3d49218 100644 --- a/lib/components/fabro-workflow/src/operations/start.rs +++ b/lib/components/fabro-workflow/src/operations/start.rs @@ -3,7 +3,7 @@ use std::path::Path; use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; -use fabro_auth::{CredentialSource, EnvCredentialSource, VaultCredentialSource}; +use fabro_auth::{CredentialSource, VaultCredentialSource}; use fabro_interview::{AutoApproveInterviewer, Interviewer}; use fabro_llm::client::Client as LlmClient; use fabro_mcp::config::McpServerSettings; @@ -81,7 +81,7 @@ struct RunSession { workflow_path: Option, workflow_bundle: Option>, run_control: Option>, - vault: Option>>, + vault: Arc>, catalog: Arc, fabro_run_tools: Option, } @@ -106,7 +106,7 @@ pub struct StartServices { /// Server-resolved GitHub integration permissions to inject into the /// sandbox env. Empty when github integration has no permissions. pub github_permissions: HashMap, - pub vault: Option>>, + pub vault: Arc>, pub catalog: Arc, pub on_node: crate::OnNodeCallback, pub registry_override: Option>, @@ -374,7 +374,7 @@ impl RunSession { resolve_sandbox_provider(resolved).effective_for(resolved.execution.mode); let catalog = Arc::clone(&services.catalog); let configured = - configured_providers_for_start(services.vault.as_ref(), Arc::clone(&catalog)).await; + configured_providers_for_start(&services.vault, Arc::clone(&catalog)).await; #[cfg(feature = "test-support")] let configured = workflow_test_support::test_configured_provider_ids( catalog.as_ref(), @@ -383,14 +383,11 @@ impl RunSession { .is_some_and(|value| !matches!(value.as_str(), "" | "0" | "false" | "no")), ); let llm = resolve_start_llm(catalog.as_ref(), &configured, resolved)?; - let vault_guard = match services.vault.as_ref() { - Some(vault) => Some(vault.read().await), - None => None, - }; + let vault_guard = services.vault.read().await; // Token-only secrets lookup over the vault read guard, shared across // every run-boundary resolver. A missing or non-Token secret becomes // `None`, so resolution fails closed with a secret error. - let secret_lookup = |name: &str| vault_token_lookup(vault_guard.as_deref(), name); + let secret_lookup = |name: &str| vault_token_lookup(&vault_guard, name); let mcp_servers = resolved .agent .mcps @@ -436,10 +433,9 @@ impl RunSession { clone_branch: record.base_branch().map(str::to_string), }, SandboxProviderKind::Daytona => { - let api_key = match vault_guard.as_deref() { - Some(vault) => vault.get(EnvVars::DAYTONA_API_KEY).map(str::to_string), - None => None, - }; + let api_key = vault_guard + .get(EnvVars::DAYTONA_API_KEY) + .map(str::to_string); SandboxSpec::Daytona { config: Box::new(resolve_daytona_config(resolved)), github_app: services.github_app.clone(), @@ -522,16 +518,13 @@ impl RunSession { } async fn configured_providers_for_start( - vault: Option<&Arc>>, + vault: &Arc>, catalog: Arc, ) -> Vec { - let source: Arc = match vault { - Some(vault) => Arc::new(VaultCredentialSource::with_env_lookup( - Arc::clone(vault), - process_env_var, - )), - None => Arc::new(EnvCredentialSource::new()), - }; + let source: Arc = Arc::new(VaultCredentialSource::with_env_lookup( + Arc::clone(vault), + process_env_var, + )); match LlmClient::from_source_report(source.as_ref(), catalog).await { Ok(report) => report .client @@ -572,8 +565,8 @@ fn process_env_var(name: &str) -> Option { std::env::var(name).ok() } -fn vault_token_lookup(vault: Option<&Vault>, name: &str) -> Option { - vault.and_then(|vault| fabro_auth::vault_get_token(vault, name).ok().flatten()) +fn vault_token_lookup(vault: &Vault, name: &str) -> Option { + fabro_auth::vault_get_token(vault, name).ok().flatten() } async fn load_accepted_run_definition( @@ -1780,7 +1773,7 @@ reasoning = false )]))); let session = RunSession::new(&persisted, StartServices { - vault: Some(vault), + vault, ..test_start_services(&store, &storage_root, emitter, registry).await }) .await @@ -1838,7 +1831,7 @@ reasoning = false let vault = Arc::new(AsyncRwLock::new(start_vault(&[]))); let Err(err) = RunSession::new(&persisted, StartServices { - vault: Some(vault), + vault, ..test_start_services(&store, &storage_root, emitter, registry).await }) .await @@ -2004,7 +1997,7 @@ reasoning = false run_control: None, github_app: None, github_permissions: HashMap::new(), - vault: Some(Arc::new(AsyncRwLock::new(start_vault(&[])))), + vault: Arc::new(AsyncRwLock::new(start_vault(&[]))), catalog: test_catalog(), on_node: None, registry_override: Some(registry), @@ -2032,7 +2025,7 @@ reasoning = false } fn vault_secret_lookup(vault: &Vault) -> impl FnMut(&str) -> Option + '_ { - move |name| vault_token_lookup(Some(vault), name) + move |name| vault_token_lookup(vault, name) } fn prepare_with_step(step: PreparedStep) -> RunPrepareSettings { diff --git a/lib/components/fabro-workflow/src/pipeline/execute/tests.rs b/lib/components/fabro-workflow/src/pipeline/execute/tests.rs index 2322e6b71..98a1feabb 100644 --- a/lib/components/fabro-workflow/src/pipeline/execute/tests.rs +++ b/lib/components/fabro-workflow/src/pipeline/execute/tests.rs @@ -12,6 +12,7 @@ use std::time::Duration; use async_trait::async_trait; use fabro_agent::Sandbox; +use fabro_auth::test_support as auth_test_support; use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; use fabro_hooks::HookSettings; use fabro_interview::AutoApproveInterviewer; @@ -286,7 +287,7 @@ async fn execute_test_run_with_options( github_permissions: None, origin_url: None, }, - vault: None, + vault: auth_test_support::empty_vault(), git: git_options, run_control: None, registry_override, @@ -348,7 +349,7 @@ async fn execute_runs_start_to_exit_and_returns_final_context() { github_permissions: None, origin_url: None, }, - vault: None, + vault: auth_test_support::empty_vault(), git: None, run_control: None, registry_override: None, @@ -487,7 +488,7 @@ async fn resumed_in_flight_node_starts_a_new_stage_execution() { github_permissions: None, origin_url: None, }, - vault: None, + vault: auth_test_support::empty_vault(), git: None, run_control: None, registry_override: Some(Arc::new(make_registry())), @@ -598,7 +599,7 @@ async fn run_with_lifecycle( github_permissions: None, origin_url: None, }, - vault: None, + vault: auth_test_support::empty_vault(), git: None, run_control: None, registry_override: Some(Arc::new(registry)), diff --git a/lib/components/fabro-workflow/src/pipeline/finalize.rs b/lib/components/fabro-workflow/src/pipeline/finalize.rs index 949578880..a6d6581ec 100644 --- a/lib/components/fabro-workflow/src/pipeline/finalize.rs +++ b/lib/components/fabro-workflow/src/pipeline/finalize.rs @@ -645,6 +645,7 @@ mod tests { use anyhow::Result; use async_trait::async_trait; use bytes::Bytes; + use fabro_auth::test_support as auth_test_support; use fabro_graphviz::graph::Graph; use fabro_model::Catalog; use fabro_sandbox::test_support::MockSandbox; @@ -1020,7 +1021,7 @@ mod tests { tokio_util::sync::CancellationToken::new(), fabro_model::ProviderId::anthropic(), "claude-sonnet-4-6".to_string(), - Arc::new(fabro_auth::EnvCredentialSource::new()), + auth_test_support::vault_only_credential_source(), Arc::new(Catalog::from_builtin().expect("default catalog should build")), Arc::new(SandboxGitRuntime::new()), metadata_runtime, @@ -1053,7 +1054,7 @@ mod tests { tokio_util::sync::CancellationToken::new(), fabro_model::ProviderId::anthropic(), "claude-sonnet-4-6".to_string(), - Arc::new(fabro_auth::EnvCredentialSource::new()), + auth_test_support::vault_only_credential_source(), Arc::new(Catalog::from_builtin().expect("default catalog should build")), Arc::new(SandboxGitRuntime::new()), Arc::new(RunMetadataRuntime::new()), diff --git a/lib/components/fabro-workflow/src/pipeline/initialize.rs b/lib/components/fabro-workflow/src/pipeline/initialize.rs index 149a9ee59..0b776944b 100644 --- a/lib/components/fabro-workflow/src/pipeline/initialize.rs +++ b/lib/components/fabro-workflow/src/pipeline/initialize.rs @@ -5,8 +5,7 @@ use std::time::Instant; use fabro_agent::{Sandbox, ToolSecrets}; use fabro_auth::{ - CredentialSource, EnvCredentialSource, ExtraHeadersCredentialSource, VaultCredentialSource, - auth_issue_message, + CredentialSource, ExtraHeadersCredentialSource, VaultCredentialSource, auth_issue_message, }; use fabro_graphviz::graph; use fabro_hooks::{HookContext, HookDecision, HookEvent, HookExecutionContext, HookRunner}; @@ -237,21 +236,12 @@ async fn build_registry( } } -#[expect( - clippy::disallowed_methods, - reason = "CLI/library workflow runs without a vault explicitly pass the Brave Search process-env credential into tool configuration; server runs pass a vault." -)] -async fn tool_secrets_from_configured_sources( - vault: Option<&Arc>>, -) -> ToolSecrets { - let brave_search_api_key = match vault { - Some(vault) => vault - .read() - .await - .get(EnvVars::BRAVE_SEARCH_API_KEY) - .map(str::to_string), - None => std::env::var(EnvVars::BRAVE_SEARCH_API_KEY).ok(), - }; +async fn tool_secrets_from_configured_sources(vault: &Arc>) -> ToolSecrets { + let brave_search_api_key = vault + .read() + .await + .get(EnvVars::BRAVE_SEARCH_API_KEY) + .map(str::to_string); ToolSecrets { brave_search_api_key, } @@ -267,15 +257,11 @@ fn graph_needs_api_backend(graph: &graph::Graph) -> bool { const SESSION_ID_HEADER: &str = "x-session-id"; fn build_llm_source( - vault: Option>>, + vault: Arc>, run_id: fabro_types::RunId, ) -> Arc { - let inner: Arc = match vault { - Some(vault) => Arc::new(VaultCredentialSource::new(vault)), - None => Arc::new(EnvCredentialSource::new()), - }; Arc::new(ExtraHeadersCredentialSource::new( - inner, + Arc::new(VaultCredentialSource::new(vault)), HashMap::from([(SESSION_ID_HEADER.to_string(), run_id.to_string())]), )) } @@ -298,7 +284,7 @@ pub async fn initialize( options.run_options.git = options.git.clone(); let llm_source = build_llm_source(options.vault.clone(), options.run_options.run_id); - let tool_secrets = tool_secrets_from_configured_sources(options.vault.as_ref()).await; + let tool_secrets = tool_secrets_from_configured_sources(&options.vault).await; let catalog = Arc::clone(&options.catalog); let sandbox_git = Arc::new(SandboxGitRuntime::new()); let metadata_runtime = Arc::new(RunMetadataRuntime::new()); @@ -356,14 +342,12 @@ pub async fn initialize( let instance = record.instance().ok_or_else(|| { Error::Precondition("cannot resume run: run sandbox was not initialized".to_string()) })?; - let daytona_api_key = match &options.vault { - Some(vault) => vault - .read() - .await - .get(EnvVars::DAYTONA_API_KEY) - .map(str::to_string), - None => None, - }; + let daytona_api_key = options + .vault + .read() + .await + .get(EnvVars::DAYTONA_API_KEY) + .map(str::to_string); let sandbox = reconnect_for_run_with_callback( instance, daytona_api_key, @@ -663,6 +647,7 @@ mod tests { use std::time::Duration; use fabro_acp::test_support::fake_acp_agent_script; + use fabro_auth::test_support as auth_test_support; use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; use fabro_interview::AutoApproveInterviewer; use fabro_sandbox::SandboxSpec; @@ -864,7 +849,7 @@ mod tests { github_permissions: None, origin_url: None, }, - vault: None, + vault: auth_test_support::empty_vault(), git: None, run_control: None, registry_override: None, @@ -945,7 +930,7 @@ mod tests { github_permissions: None, origin_url: None, }, - vault: None, + vault: auth_test_support::empty_vault(), git: None, run_control: None, registry_override: None, @@ -1053,7 +1038,7 @@ mod tests { let run_id = test_run_id(); let expected_session_id = run_id.to_string(); - let source = build_llm_source(Some(vault), run_id); + let source = build_llm_source(vault, run_id); let resolved = source.resolve(test_catalog().as_ref()).await.unwrap(); assert!(!resolved.credentials.is_empty()); @@ -1136,13 +1121,13 @@ mod tests { let store = memory_store(); let run_store = store.create_run(&test_run_id()).await.unwrap(); let initialized = initialize(test_persisted(graph, source, &run_dir), InitOptions { - run_store: run_store.into(), - dry_run: false, - emitter: emitter.clone(), - sandbox: SandboxSpec::Local { + run_store: run_store.into(), + dry_run: false, + emitter: emitter.clone(), + sandbox: SandboxSpec::Local { working_directory: temp.path().to_path_buf(), }, - llm: LlmSpec { + llm: LlmSpec { model: "fake-acp".to_string(), provider_id: fabro_model::ProviderId::openai(), fallback_chain: Vec::new(), @@ -1150,30 +1135,30 @@ mod tests { model_controls: RunModelControls::default(), dry_run: false, }, - interviewer: Arc::new(AutoApproveInterviewer::engine()), - steering_hub: Arc::new(crate::steering_hub::SteeringHub::new(emitter)), - catalog: test_catalog(), - lifecycle: crate::run_options::LifecycleOptions { + interviewer: Arc::new(AutoApproveInterviewer::engine()), + steering_hub: Arc::new(crate::steering_hub::SteeringHub::new(emitter)), + catalog: test_catalog(), + lifecycle: crate::run_options::LifecycleOptions { setup_commands: Vec::new(), setup_command_timeout_ms: 1_000, }, - run_options: test_settings(&run_dir), - workflow_path: None, - workflow_bundle: None, - hooks: fabro_hooks::HookSettings { hooks: vec![] }, - sandbox_env: SandboxEnvSpec { + run_options: test_settings(&run_dir), + workflow_path: None, + workflow_bundle: None, + hooks: fabro_hooks::HookSettings { hooks: vec![] }, + sandbox_env: SandboxEnvSpec { toml_env: HashMap::new(), github_permissions: None, origin_url: None, }, - vault: Some(vault), - git: None, - run_control: None, + vault, + git: None, + run_control: None, registry_override: None, - artifact_sink: None, - resume: None, - seed_context: None, - fabro_run_tools: None, + artifact_sink: None, + resume: None, + seed_context: None, + fabro_run_tools: None, }) .await .unwrap(); @@ -1261,7 +1246,7 @@ mod tests { github_permissions: None, origin_url: None, }, - vault: None, + vault: auth_test_support::empty_vault(), git: None, run_control: None, registry_override: None, @@ -1403,7 +1388,7 @@ mod tests { github_permissions: None, origin_url: None, }, - vault: None, + vault: auth_test_support::empty_vault(), git: None, run_control: None, registry_override: None, diff --git a/lib/components/fabro-workflow/src/pipeline/pull_request.rs b/lib/components/fabro-workflow/src/pipeline/pull_request.rs index 0dcc7745c..1266e0f41 100644 --- a/lib/components/fabro-workflow/src/pipeline/pull_request.rs +++ b/lib/components/fabro-workflow/src/pipeline/pull_request.rs @@ -677,7 +677,7 @@ mod tests { use std::time::Duration; use chrono::Utc; - use fabro_auth::{CredentialSource, EnvCredentialSource, VaultCredentialSource}; + use fabro_auth::{CredentialSource, VaultCredentialSource, test_support as auth_test_support}; use fabro_graphviz::graph::Graph; use fabro_llm::Error as LlmError; use fabro_llm::client::Client; @@ -819,7 +819,7 @@ mod tests { } fn test_llm_source() -> Arc { - Arc::new(EnvCredentialSource::new()) + auth_test_support::vault_only_credential_source() } fn test_projection() -> RunProjection { diff --git a/lib/components/fabro-workflow/src/pipeline/types.rs b/lib/components/fabro-workflow/src/pipeline/types.rs index 425cba9aa..288826686 100644 --- a/lib/components/fabro-workflow/src/pipeline/types.rs +++ b/lib/components/fabro-workflow/src/pipeline/types.rs @@ -299,7 +299,7 @@ pub struct InitOptions { pub workflow_bundle: Option>, pub hooks: fabro_hooks::HookSettings, pub sandbox_env: SandboxEnvSpec, - pub vault: Option>>, + pub vault: Arc>, pub git: Option, pub registry_override: Option>, pub artifact_sink: Option, diff --git a/lib/components/fabro-workflow/src/test_support.rs b/lib/components/fabro-workflow/src/test_support.rs index 5ece3320c..afc17a213 100644 --- a/lib/components/fabro-workflow/src/test_support.rs +++ b/lib/components/fabro-workflow/src/test_support.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use std::time::Duration; use fabro_agent::Sandbox; -use fabro_auth::{CredentialSource, EnvCredentialSource}; +use fabro_auth::{CredentialSource, test_support as auth_test_support}; use fabro_graphviz::graph::Graph as GvGraph; use fabro_interview::AutoApproveInterviewer; use fabro_model::Catalog; @@ -249,7 +249,7 @@ async fn initialized( "claude-sonnet-4-6".to_string(), options .llm_source - .unwrap_or_else(|| Arc::new(EnvCredentialSource::new())), + .unwrap_or_else(auth_test_support::vault_only_credential_source), Arc::new(Catalog::from_builtin().expect("default catalog should build")), Arc::new(SandboxGitRuntime::new()), Arc::new(RunMetadataRuntime::new()), diff --git a/lib/components/fabro-workflow/tests/it/integration.rs b/lib/components/fabro-workflow/tests/it/integration.rs index dc13bcbf8..46d189825 100644 --- a/lib/components/fabro-workflow/tests/it/integration.rs +++ b/lib/components/fabro-workflow/tests/it/integration.rs @@ -2165,7 +2165,6 @@ async fn smoke_test_with_mock_codergen_backend() { #[tokio::test] async fn shared_thread_compaction_before_routing_audit_succeeds() { - use fabro_auth::EnvCredentialSource; use fabro_workflow::steering_hub::SteeringHub; use httpmock::Method::POST; use httpmock::MockServer; @@ -2292,9 +2291,9 @@ reasoning = false )) .expect("test catalog should parse"); let catalog = Arc::new(Catalog::from_builtin_with_overrides(&settings).unwrap()); - let source = Arc::new(EnvCredentialSource::with_env_lookup(Arc::new(|name| { + let source = auth_test_support::env_credential_source(|name| { (name == "COMPACT_API_KEY").then(|| "sk-test".to_string()) - }))); + }); let backend = AgentApiBackend::new_with_catalog( "compact-model".to_string(), ProviderId::from("compact"), @@ -2397,7 +2396,6 @@ reasoning = false #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn workflow_persists_authoritative_openrouter_cost_for_agent_stage() { - use fabro_auth::EnvCredentialSource; use fabro_workflow::steering_hub::SteeringHub; use httpmock::Method::POST; use httpmock::MockServer; @@ -2448,9 +2446,9 @@ base_url = "{}" )) .expect("test catalog should parse"); let catalog = Arc::new(Catalog::from_builtin_with_overrides(&settings).unwrap()); - let source = Arc::new(EnvCredentialSource::with_env_lookup(Arc::new(|name| { + let source = auth_test_support::env_credential_source(|name| { (name == "OPENROUTER_API_KEY").then(|| "sk-test".to_string()) - }))); + }); let backend = AgentApiBackend::new_with_catalog( "openai/gpt-5.4".to_string(), ProviderId::from("openrouter"), @@ -6915,6 +6913,7 @@ mod real_llm { use std::sync::Arc; use async_trait::async_trait; + use fabro_auth::test_support as auth_test_support; use fabro_graphviz::graph::Node; use fabro_llm::client::Client; use fabro_llm::providers::OpenAiAdapter; @@ -7008,7 +7007,7 @@ mod real_llm { } fabro_test::require_env("ANTHROPIC_API_KEY")?; - let source = fabro_auth::EnvCredentialSource::new(); + let source = auth_test_support::StubCredentialSource; Some(Arc::new( Client::from_source(&source, super::default_catalog()) .await @@ -8418,7 +8417,7 @@ fn subgraph_without_label_no_class_derived() { fn hook_runner_from_defs(hooks: Vec) -> Arc { Arc::new(fabro_hooks::HookRunner::new( fabro_hooks::HookSettings { hooks }, - Arc::new(fabro_auth::EnvCredentialSource::new()), + auth_test_support::vault_only_credential_source(), default_catalog(), )) } @@ -10329,6 +10328,7 @@ async fn node_dir_uses_visit_count_on_revisit() { // Git checkpoint e2e (Local) // --------------------------------------------------------------------------- +use fabro_auth::test_support as auth_test_support; use fabro_workflow::handler::fan_in::FanInHandler; use fabro_workflow::handler::parallel::ParallelHandler; diff --git a/lib/foundation/fabro-auth/Cargo.toml b/lib/foundation/fabro-auth/Cargo.toml index 90ad9891e..8fcfa13b0 100644 --- a/lib/foundation/fabro-auth/Cargo.toml +++ b/lib/foundation/fabro-auth/Cargo.toml @@ -9,6 +9,9 @@ description = "Typed provider credential storage and resolution for Fabro" [lints] workspace = true +[features] +test-support = [] + [dependencies] anyhow.workspace = true async-trait.workspace = true diff --git a/lib/foundation/fabro-auth/src/env_source.rs b/lib/foundation/fabro-auth/src/env_source.rs deleted file mode 100644 index 8a2d7cc53..000000000 --- a/lib/foundation/fabro-auth/src/env_source.rs +++ /dev/null @@ -1,381 +0,0 @@ -use std::collections::HashMap; -use std::sync::Arc; - -use async_trait::async_trait; -use fabro_model::catalog::CatalogProvider; -use fabro_model::{Catalog, CredentialRef, ProviderId}; -use fabro_static::EnvVars; -use fabro_types::settings::ResolveCtx; - -use crate::credential_source::{CredentialSource, ResolvedCredentials}; -use crate::resolve::{apply_openai_api_env_context, apply_openai_codex_api_context}; -use crate::{ApiCredential, EnvLookup, ResolveError, build_api_key_header, resolve}; - -#[derive(Clone)] -pub struct EnvCredentialSource { - env_lookup: EnvLookup, -} - -impl EnvCredentialSource { - #[must_use] - #[expect( - clippy::disallowed_methods, - reason = "EnvCredentialSource is the provider API-key process-env facade." - )] - pub fn new() -> Self { - Self::with_env_lookup(Arc::new(|name| std::env::var(name).ok())) - } - - #[must_use] - pub fn with_env_lookup(env_lookup: EnvLookup) -> Self { - Self { env_lookup } - } - - fn lookup(&self, name: &str) -> Option { - (self.env_lookup)(name) - } - - fn credential_for( - &self, - provider: &CatalogProvider, - ) -> Result, ResolveError> { - let (auth_header, extra_headers) = match &provider.auth { - Some(auth) => { - let Some(key) = auth.credentials.iter().find_map(|credential_ref| { - let CredentialRef::Env(name) = credential_ref else { - return None; - }; - self.lookup(name) - }) else { - return Ok(None); - }; - ( - Some(build_api_key_header(auth.header.clone(), key)), - self.resolved_extra_headers(provider)?, - ) - } - None => (None, self.resolved_extra_headers(provider)?), - }; - - let mut cred = ApiCredential { - provider: provider.id.clone(), - auth_header, - extra_headers, - base_url: provider.base_url.clone(), - codex_mode: false, - org_id: None, - project_id: None, - }; - if provider.id == ProviderId::openai() && cred.auth_header.is_some() { - if let Some(account_id) = self.lookup(EnvVars::CHATGPT_ACCOUNT_ID) { - apply_openai_codex_api_context(&mut cred, Some(&account_id), &*self.env_lookup); - } else { - apply_openai_api_env_context(&mut cred, &*self.env_lookup); - } - } - Ok(Some(cred)) - } - - fn resolved_extra_headers( - &self, - provider: &CatalogProvider, - ) -> Result, ResolveError> { - let mut ctx = ResolveCtx::new().with_env(|env_name| self.lookup(env_name)); - resolve::resolve_extra_headers(&provider.id, &provider.extra_headers, &mut ctx) - } -} - -impl std::fmt::Debug for EnvCredentialSource { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("EnvCredentialSource") - .finish_non_exhaustive() - } -} - -impl Default for EnvCredentialSource { - fn default() -> Self { - Self::new() - } -} - -#[async_trait] -impl CredentialSource for EnvCredentialSource { - async fn resolve(&self, catalog: &Catalog) -> anyhow::Result { - let mut credentials = Vec::new(); - let mut auth_issues = Vec::new(); - - for provider in catalog.providers() { - match self.credential_for(provider) { - Ok(Some(credential)) => credentials.push(credential), - Ok(None) => {} - Err(ResolveError::NotConfigured(_) | ResolveError::Interpolation { .. }) - if provider.auth.is_some() => {} - Err(err) => auth_issues.push((provider.id.clone(), err)), - } - } - - Ok(ResolvedCredentials { - credentials, - auth_issues, - }) - } - - async fn configured_providers(&self, catalog: &Catalog) -> Vec { - catalog - .providers() - .iter() - .filter(|provider| match &provider.auth { - Some(auth) => auth.credentials.iter().any(|credential_ref| { - matches!(credential_ref, CredentialRef::Env(name) if self.lookup(name).is_some()) - }), - None => self.resolved_extra_headers(provider).is_ok(), - }) - .map(|provider| provider.id.clone()) - .collect() - } -} - -#[cfg(test)] -mod tests { - use std::collections::HashMap; - use std::sync::Arc; - - use fabro_model::catalog::LlmCatalogSettings; - use fabro_model::{Catalog, ProviderId}; - - use super::EnvCredentialSource; - use crate::CredentialSource; - - fn test_source(entries: &[(&str, &str)]) -> EnvCredentialSource { - let entries: HashMap = entries - .iter() - .map(|(key, value)| ((*key).to_string(), (*value).to_string())) - .collect(); - EnvCredentialSource::with_env_lookup(Arc::new(move |name| entries.get(name).cloned())) - } - - fn catalog_with(overrides: &str) -> Catalog { - let settings: LlmCatalogSettings = toml::from_str(overrides).unwrap(); - Catalog::from_builtin_with_overrides(&settings).unwrap() - } - - fn default_catalog() -> Catalog { - catalog_with("") - } - - /// A no-auth portkey provider whose only variation is its `extra_headers` - /// TOML lines. - fn portkey_catalog(extra_headers: &str) -> Catalog { - catalog_with(&format!( - r#" -[providers.portkey] -display_name = "Portkey Bedrock" -adapter = "anthropic" -agent_profile = "anthropic" -base_url = "https://api.portkey.ai/v1" - -[providers.portkey.extra_headers] -{extra_headers} - -[models."portkey-claude"] -provider = "portkey" -display_name = "Portkey Claude" -family = "claude" -default = true - -[models."portkey-claude".limits] -context_window = 200000 - -[models."portkey-claude".features] -tools = true -vision = true -reasoning = true -reasoning_effort = "levels" -"# - )) - } - - #[tokio::test] - async fn configured_providers_reads_injected_env() { - let source = test_source(&[("ANTHROPIC_API_KEY", "anthropic-key")]); - let catalog = default_catalog(); - - assert_eq!(source.configured_providers(&catalog).await, vec![ - ProviderId::anthropic() - ]); - } - - #[tokio::test] - async fn resolve_returns_empty_when_no_keys_are_configured() { - let source = test_source(&[]); - let catalog = default_catalog(); - - let resolved = source.resolve(&catalog).await.unwrap(); - - assert!(resolved.credentials.is_empty()); - assert!(resolved.auth_issues.is_empty()); - } - - #[tokio::test] - async fn resolve_builds_openai_codex_env_credential() { - let source = test_source(&[ - ("OPENAI_API_KEY", "openai-key"), - ("CHATGPT_ACCOUNT_ID", "acct_123"), - ("OPENAI_PROJECT_ID", "project_123"), - ]); - let catalog = default_catalog(); - - let resolved = source.resolve(&catalog).await.unwrap(); - let credential = resolved.credentials.first().unwrap(); - - assert_eq!(credential.provider, ProviderId::openai()); - assert!(credential.codex_mode); - assert_eq!( - credential.base_url.as_deref(), - Some("https://chatgpt.com/backend-api/codex") - ); - assert_eq!( - credential.extra_headers.get("ChatGPT-Account-Id"), - Some(&"acct_123".to_string()) - ); - assert_eq!(credential.project_id.as_deref(), Some("project_123")); - } - - #[tokio::test] - async fn resolve_uses_catalog_credentials_and_base_url_for_openai_compatible_providers() { - let source = test_source(&[("KIMI_API_KEY", "kimi-key")]); - let catalog = default_catalog(); - - let resolved = source.resolve(&catalog).await.unwrap(); - let credential = resolved.credentials.first().unwrap(); - - assert_eq!(credential.provider, ProviderId::new("kimi")); - assert_eq!( - credential.base_url.as_deref(), - Some("https://api.moonshot.ai/v1") - ); - } - - #[tokio::test] - async fn resolve_registers_custom_env_backed_provider() { - let catalog = catalog_with( - r#" -[providers.acme] -display_name = "Acme" -adapter = "openai_compatible" -agent_profile = "openai" -base_url = "https://api.acme.test/v1" - -[providers.acme.auth] -credentials = ["env:ACME_API_KEY"] - -[models."acme-large"] -provider = "acme" -display_name = "Acme Large" -family = "acme" -default = true - -[models."acme-large".limits] -context_window = 128000 - -[models."acme-large".features] -tools = true -vision = false -reasoning = false -"#, - ); - let source = test_source(&[("ACME_API_KEY", "acme-key")]); - - let resolved = source.resolve(&catalog).await.unwrap(); - let credential = resolved - .credentials - .iter() - .find(|credential| credential.provider == ProviderId::new("acme")) - .expect("custom provider should resolve from the supplied catalog"); - - assert_eq!( - credential.auth_header.as_ref().unwrap(), - &crate::ApiKeyHeader::Bearer("acme-key".to_string(),) - ); - assert_eq!( - credential.base_url.as_deref(), - Some("https://api.acme.test/v1") - ); - } - - #[tokio::test] - async fn env_source_resolves_literal_and_env_header_tokens() { - let catalog = portkey_catalog( - r#" -x-portkey-api-key = "{{ env.PORTKEY_API_KEY }}" -x-portkey-provider = "@bedrock-prod" -"#, - ); - let source = test_source(&[("PORTKEY_API_KEY", "pk-live")]); - - let resolved = source.resolve(&catalog).await.unwrap(); - let credential = resolved - .credentials - .iter() - .find(|credential| credential.provider == ProviderId::new("portkey")) - .expect("no-auth provider should register when extra headers resolve"); - - assert!(credential.auth_header.is_none()); - assert_eq!( - credential.extra_headers.get("x-portkey-api-key"), - Some(&"pk-live".to_string()) - ); - assert_eq!( - credential.extra_headers.get("x-portkey-provider"), - Some(&"@bedrock-prod".to_string()) - ); - } - - #[tokio::test] - async fn env_source_secrets_header_token_is_unavailable() { - let catalog = portkey_catalog(r#"x-team-secret = "{{ secrets.gateway_team_secret }}""#); - let source = test_source(&[]); - - let resolved = source.resolve(&catalog).await.unwrap(); - - assert!( - !resolved - .credentials - .iter() - .any(|credential| credential.provider == ProviderId::new("portkey")) - ); - let (_, issue) = resolved - .auth_issues - .iter() - .find(|(provider, _)| provider == &ProviderId::new("portkey")) - .expect("secrets token should surface as an auth issue"); - assert!(matches!( - issue, - crate::ResolveError::Interpolation { provider, .. } - if provider == &ProviderId::new("portkey") - )); - assert!(issue.to_string().contains("gateway_team_secret")); - } - - #[tokio::test] - async fn env_source_reports_missing_env_header_for_no_auth_provider() { - let catalog = portkey_catalog(r#"x-portkey-api-key = "{{ env.PORTKEY_API_KEY }}""#); - let source = test_source(&[]); - - let resolved = source.resolve(&catalog).await.unwrap(); - - assert!( - !resolved - .credentials - .iter() - .any(|credential| credential.provider == ProviderId::new("portkey")) - ); - let (_, issue) = resolved - .auth_issues - .iter() - .find(|(provider, _)| provider == &ProviderId::new("portkey")) - .expect("missing env header should surface as an auth issue"); - assert!(matches!(issue, crate::ResolveError::Interpolation { .. })); - assert!(issue.to_string().contains("PORTKEY_API_KEY")); - } -} diff --git a/lib/foundation/fabro-auth/src/lib.rs b/lib/foundation/fabro-auth/src/lib.rs index 77c217317..b57075718 100644 --- a/lib/foundation/fabro-auth/src/lib.rs +++ b/lib/foundation/fabro-auth/src/lib.rs @@ -1,12 +1,13 @@ mod context; mod credential; mod credential_source; -mod env_source; mod extra_headers_source; mod refresh; mod resolve; mod sql_vault_source; mod strategy; +#[cfg(any(test, feature = "test-support"))] +pub mod test_support; mod vault_ext; mod vault_source; @@ -15,13 +16,11 @@ pub mod strategies; pub use context::{AuthContextRequest, AuthContextResponse}; pub use credential::{ApiKeyHeader, OAuthConfig, OAuthCredential, OAuthTokens}; pub use credential_source::{CredentialSource, ResolvedCredentials}; -pub use env_source::EnvCredentialSource; pub use extra_headers_source::ExtraHeadersCredentialSource; pub use refresh::refresh_oauth_credential; pub use resolve::{ ApiCredential, CredentialResolver, CredentialUsage, EnvLookup, ResolveError, ResolvedCredential, auth_issue_message, build_api_key_header, - configured_providers_from_process_env, }; pub use sql_vault_source::SqlVaultCredentialSource; pub use strategy::{ diff --git a/lib/foundation/fabro-auth/src/resolve.rs b/lib/foundation/fabro-auth/src/resolve.rs index 60a4576b7..55afb8a30 100644 --- a/lib/foundation/fabro-auth/src/resolve.rs +++ b/lib/foundation/fabro-auth/src/resolve.rs @@ -10,8 +10,6 @@ use tokio::sync::RwLock as AsyncRwLock; use tokio::task::spawn_blocking; use crate::credential::{ApiKeyHeader, OAuthCredential}; -use crate::credential_source::CredentialSource; -use crate::env_source::EnvCredentialSource; use crate::refresh::refresh_oauth_credential; use crate::vault_ext::{ VaultLookupError, vault_get_oauth, vault_get_token, vault_set_oauth, vault_token_lookup, @@ -515,23 +513,6 @@ fn vault_lookup_error(provider: &ProviderId, name: &str, err: VaultLookupError) } } -pub async fn configured_providers_from_process_env( - vault: Option<&Arc>>, - catalog: &Catalog, -) -> Vec { - match vault { - Some(vault_arc) => { - let resolver = CredentialResolver::new(Arc::clone(vault_arc)); - let guard = vault_arc.read().await; - resolver.configured_providers(&guard, catalog) - } - None => { - EnvCredentialSource::new() - .configured_providers(catalog) - .await - } - } -} #[cfg(test)] mod tests { use std::error::Error as _; diff --git a/lib/foundation/fabro-auth/src/test_support.rs b/lib/foundation/fabro-auth/src/test_support.rs new file mode 100644 index 000000000..944f83342 --- /dev/null +++ b/lib/foundation/fabro-auth/src/test_support.rs @@ -0,0 +1,62 @@ +//! Test-only credential sources. +//! +//! Feature-gated so they never link into production builds. Production code +//! resolves credentials through [`VaultCredentialSource`] over a real vault; +//! these helpers exist so tests can supply a source without one. + +use std::collections::HashMap; +use std::sync::Arc; + +use async_trait::async_trait; +use fabro_model::{Catalog, ProviderId}; +use fabro_vault::Vault; +use tokio::sync::RwLock as AsyncRwLock; + +use crate::credential_source::{CredentialSource, ResolvedCredentials}; +use crate::vault_source::VaultCredentialSource; + +/// A credential source that resolves nothing, for tests that need a source but +/// never make a provider request. +#[derive(Debug, Default, Clone, Copy)] +pub struct StubCredentialSource; + +#[async_trait] +impl CredentialSource for StubCredentialSource { + async fn resolve(&self, _catalog: &Catalog) -> anyhow::Result { + Ok(ResolvedCredentials { + credentials: Vec::new(), + auth_issues: Vec::new(), + }) + } + + async fn configured_providers(&self, _catalog: &Catalog) -> Vec { + Vec::new() + } +} + +/// A detached in-memory vault holding no secrets. +#[must_use] +pub fn empty_vault() -> Arc> { + Arc::new(AsyncRwLock::new(Vault::from_entries(HashMap::new()))) +} + +/// A vault-backed source whose credentials come only from `env_lookup`. +/// +/// Tests that inject fake provider keys use this instead of reading the real +/// process environment, which would make them order-dependent. +#[must_use] +pub fn env_credential_source(env_lookup: F) -> Arc +where + F: Fn(&str) -> Option + Send + Sync + 'static, +{ + Arc::new(VaultCredentialSource::with_env_lookup( + empty_vault(), + env_lookup, + )) +} + +/// A vault-backed source over an empty vault with no process-env fallback. +#[must_use] +pub fn vault_only_credential_source() -> Arc { + Arc::new(VaultCredentialSource::vault_only(empty_vault())) +}