diff --git a/docs/public/administration/server-configuration.mdx b/docs/public/administration/server-configuration.mdx index 3373f02a9..63da27729 100644 --- a/docs/public/administration/server-configuration.mdx +++ b/docs/public/administration/server-configuration.mdx @@ -328,7 +328,7 @@ Fabro no longer auto-loads `.env` files. Provider API keys are required for the ### LLM provider keys -Fabro's built-in provider access resolves these from `process env -> vault`. +Fabro's built-in provider access resolves these from the process environment first, then an exact-name vault token or OAuth entry. | Variable | Provider | |---|---| diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 55670aab0..4426ccb7f 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -10385,12 +10385,12 @@ components: example: ok SecretType: - description: The way a secret is consumed by the sandbox. + description: Schema of a stored secret. type: string enum: - - environment + - token + - oauth - file - - credential CreateSecretRequest: description: Request to store or update a secret. diff --git a/docs/public/changelog/2026-05-18.mdx b/docs/public/changelog/2026-05-18.mdx new file mode 100644 index 000000000..b952ca954 --- /dev/null +++ b/docs/public/changelog/2026-05-18.mdx @@ -0,0 +1,27 @@ +--- +title: "Explicit vault-backed provider credentials" +date: "2026-05-18" +--- + +## Explicit vault-backed provider credentials + +Fabro now separates process environment credentials from server-owned vault credentials. Provider auth refs use `env:` for process environment lookup and `vault:` for explicit vault lookup. API-key secrets are stored as raw `token` values, OAuth records are stored as typed `oauth` JSON, and file material remains `file`. + +```toml +[llm.providers.proxy.auth] +credentials = ["env:ACME_GATEWAY_API_KEY", "vault:ACME_GATEWAY_API_KEY"] +``` + +## Migration note + +Use `fabro secret set NAME value` for API keys and PAT-style secrets. The default secret type is now `token`; `environment` and `credential` secret schemas are no longer part of the API. + +Existing server-owned secrets using the removed `environment` or `credential` schemas must be re-created with the new `token`, `oauth`, or `file` schema. + +## More + + +- Removed the old `credential:` provider ref prefix in favor of explicit `vault:` +- OpenAI Codex OAuth credentials now live under `vault:OPENAI_CODEX` +- Provider catalogs now check process env first, then same-name vault entries + diff --git a/docs/public/core-concepts/models.mdx b/docs/public/core-concepts/models.mdx index 3fb7d611a..177f8eb77 100644 --- a/docs/public/core-concepts/models.mdx +++ b/docs/public/core-concepts/models.mdx @@ -37,7 +37,7 @@ No single model is best at everything. Fabro lets you assign the right model to | `minimax-m2.5` | minimax | `minimax` | 197K | $0.30 / $1.20 | 45 tok/s | | `mercury-2` | inception | `mercury` | 131K | $0.20 / $0.80 | 1000 tok/s | -Each provider requires its own API key set via environment variable (e.g. `ANTHROPIC_API_KEY`, `OPENAI_API_KEY`, `GEMINI_API_KEY`). See the [Quick Start](/getting-started/quick-start) for setup. +Each provider requires its own API key set via environment variable or matching vault token (e.g. `ANTHROPIC_API_KEY`, `OPENAI_API_KEY`, `GEMINI_API_KEY`). See the [Quick Start](/getting-started/quick-start) for setup. ## Configuring providers and models @@ -51,7 +51,7 @@ base_url = "https://llm-gateway.example.com/v1" aliases = ["gateway"] [llm.providers.proxy.auth] -credentials = ["env:ACME_GATEWAY_API_KEY"] +credentials = ["env:ACME_GATEWAY_API_KEY", "vault:ACME_GATEWAY_API_KEY"] [llm.providers.proxy.extra_headers] x-portkey-api-key = { env = "PORTKEY_API_KEY" } @@ -119,7 +119,7 @@ reasoning = false `api_id` is the model name sent to the provider API. Omit it when the Fabro model ID and provider model ID are the same. -Provider auth is declared in `[llm.providers..auth]` with ordered `env:` or `credential:` refs. The primary auth header defaults to `bearer`; override with `header = { custom = "Header-Name" }` for providers like Anthropic that use `x-api-key`. Omit the `[llm.providers..auth]` block entirely for providers that need no API key (e.g. Ollama). Custom headers for any provider — including providers that need only typed headers and no API-key auth — go in `extra_headers` as `{ env = "NAME" }`, `{ credential = "id" }`, or `{ literal = "value" }`. +Provider auth is declared in `[llm.providers..auth]` with ordered `env:` or `vault:` refs. The primary auth header defaults to `bearer`; override with `header = { custom = "Header-Name" }` for providers like Anthropic that use `x-api-key`. Omit the `[llm.providers..auth]` block entirely for providers that need no API key (e.g. Ollama). Custom headers for any provider — including providers that need only typed headers and no API-key auth — go in `extra_headers` as `{ env = "NAME" }`, `{ vault = "NAME" }`, or `{ literal = "value" }`. Provider `agent_profile` defaults from `adapter` and controls profile-specific behavior such as project-memory filenames, CLI/ACP command selection, and native session routing. Valid values are `anthropic`, `openai`, and `gemini`; model-level values override provider-level values. diff --git a/docs/public/integrations/litellm.mdx b/docs/public/integrations/litellm.mdx index f39712079..a19ac43e9 100644 --- a/docs/public/integrations/litellm.mdx +++ b/docs/public/integrations/litellm.mdx @@ -45,12 +45,12 @@ reasoning = false ## Configure credentials -The LiteLLM provider checks `credential:litellm` first, then `LITELLM_API_KEY` from the Fabro process environment. +The LiteLLM provider checks `LITELLM_API_KEY` from the Fabro process environment first, then the `vault:LITELLM_API_KEY` server secret. For a server-owned secret: ```bash -fabro secret set litellm sk-proxy-key +fabro secret set LITELLM_API_KEY sk-proxy-key ``` For a process environment variable: @@ -115,7 +115,7 @@ Only one model for a provider should set `default = true`. ## Troubleshooting -**"No API key configured"** — Set `credential:litellm` with `fabro secret set litellm ...` or export `LITELLM_API_KEY` in the Fabro process environment. +**"No API key configured"** — Set `vault:LITELLM_API_KEY` with `fabro secret set LITELLM_API_KEY ...` or export `LITELLM_API_KEY` in the Fabro process environment. **Connection refused** — Confirm the LiteLLM proxy is running and that `base_url` is reachable from the Fabro process. For Docker deployments, `localhost` means the Fabro container unless you point it at a host or service name. diff --git a/docs/public/reference/cli.mdx b/docs/public/reference/cli.mdx index 69283c4fa..0db3c3791 100644 --- a/docs/public/reference/cli.mdx +++ b/docs/public/reference/cli.mdx @@ -1168,7 +1168,7 @@ fabro secret set [OPTIONS] [VALUE] | Option | Description | | --- | --- | | `--description ` | Optional human-readable description | -| `--type ` | Secret storage type
Values: `environment`, `file`
Default: `environment` | +| `--type ` | Secret storage type
Values: `token`, `file`
Default: `token` | | `--value-stdin` | Read the secret value from stdin | ### `fabro server` diff --git a/docs/public/reference/user-configuration.mdx b/docs/public/reference/user-configuration.mdx index 8a9da1e73..abe17f584 100644 --- a/docs/public/reference/user-configuration.mdx +++ b/docs/public/reference/user-configuration.mdx @@ -91,7 +91,7 @@ base_url = "https://llm-gateway.example.com/v1" aliases = ["gateway"] [llm.providers.proxy.auth] -credentials = ["env:ACME_GATEWAY_API_KEY"] +credentials = ["env:ACME_GATEWAY_API_KEY", "vault:ACME_GATEWAY_API_KEY"] [llm.providers.proxy.extra_headers] x-portkey-api-key = { env = "PORTKEY_API_KEY" } @@ -182,12 +182,12 @@ enabled = true aliases = ["gateway"] [llm.providers.proxy.auth] -credentials = ["env:ACME_GATEWAY_API_KEY"] +credentials = ["env:ACME_GATEWAY_API_KEY", "vault:ACME_GATEWAY_API_KEY"] [llm.providers.proxy.extra_headers] x-portkey-api-key = { env = "PORTKEY_API_KEY" } x-portkey-config = { literal = "@bedrock-prod" } -x-team-secret = { credential = "gateway_team_secret" } +x-team-secret = { vault = "gateway_team_secret" } ``` | Key | Type / values | Default | Description | @@ -198,9 +198,9 @@ x-team-secret = { credential = "gateway_team_secret" } | `billing_policy` | `"openai"` \| `"anthropic"` \| `"gemini"` \| `"none"` | derived from `adapter` | Provider-owned billing algorithm for usage estimates. Override for exceptional providers such as local no-billing runtimes. | | `base_url` | string | built-in value or adapter runtime default | Provider API base URL. Required for most custom OpenAI-compatible providers. | | `auth` | table | omitted | API-key auth config. Omit the table entirely for providers that need no API key; any `extra_headers` are still attached. | -| `auth.credentials` | array | required when `auth` present | Ordered credential refs. Accepted forms are `credential:` and `env:`. Literal secret strings are rejected. | +| `auth.credentials` | array | required when `auth` present | Ordered credential refs. Accepted forms are `vault:` and `env:`. Literal secret strings are rejected. | | `auth.header` | `"bearer"` or `{ custom = "Header-Name" }` | `"bearer"` | Primary API-key header policy. Omit when the provider uses a standard bearer token. | -| `extra_headers` | table | `{}` | Additional headers attached to provider requests. Values must be typed refs: `{ literal = "..." }`, `{ env = "NAME" }`, or `{ credential = "id" }`. | +| `extra_headers` | table | `{}` | Additional headers attached to provider requests. Values must be typed refs: `{ literal = "..." }`, `{ env = "NAME" }`, or `{ vault = "NAME" }`. | | `priority` | integer | `0` | Higher-priority configured providers win default selection; ties use canonical provider ID. | | `enabled` | boolean | `true` | Set `false` to disable a provider after lower-precedence layers define it. | | `aliases` | array | `[]` | Additional provider names accepted by model routing and fallback config. | diff --git a/docs/superpowers/plans/2026-05-18-vault-and-credential-source-cleanup.md b/docs/superpowers/plans/2026-05-18-vault-and-credential-source-cleanup.md new file mode 100644 index 000000000..0143a4960 --- /dev/null +++ b/docs/superpowers/plans/2026-05-18-vault-and-credential-source-cleanup.md @@ -0,0 +1,1452 @@ +# Vault and Credential Source Cleanup Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Execution status:** Completed on 2026-05-18 on local `main` as requested. No worktree was created and no commits were made during implementation. The optional live provider auth spot-check was not run. + +**Goal:** Replace the conflated `credential:`/`env:` credential reference model with explicit `vault:`/`env:` prefixes, and store API-key secrets as raw tokens (no JSON envelope) while keeping OAuth credentials as a typed JSON schema in the vault. + +**Architecture:** The vault keeps three secret schemas: `token` (opaque token value), `oauth` (JSON `OAuthCredential`), and `file` (path-shaped). Provider catalog credential refs are explicit about source: `env:NAME` reads the process env only, `vault:NAME` reads the vault only. The credential resolver branches on the vault entry's schema to build the right `ApiCredential` (token → API-key header; oauth → refreshable bearer token); OpenAI/Codex-specific headers and base URLs live in resolver/catalog policy, not in the vault schema. The old `AuthCredential` enum, `credential_id_for`, `parse_credential_secret`, `Vault::snapshot()`, and the `credential:` prefix all go away. + +**Tech Stack:** Rust workspace (`cargo nextest`), `insta` snapshots, OpenAPI + `progenitor` for `fabro-api`, Bun + `openapi-generator` for the TS client. + +**Scope notes:** +- Greenfield app, no backwards compat. Rip-and-replace, do not parallel-implement. +- No higher-level alias prefix; `env:` and `vault:` are the only two. +- Keep `SecretType::File`. Possible future schemas such as `Cookie`, `PemCertificate`, and `SshKey` are out of scope. +- The order in `credentials = [...]` is the lookup order. Conventionally `env:NAME` precedes `vault:NAME`. +- Schema mismatch on a `vault:` reference (e.g. asking for an API key, finding a `file` entry) is a hard error surfaced from the resolver. +- Do not implicitly project vault token secrets into spawned process env. If workflow command env needs vault-backed values later, add an explicit mapping feature instead of a bulk export API. + +--- + +### Task 1: Create the feature branch and baseline + +**Files:** +- (none) + +- [ ] **Step 1: Confirm clean working state on `fix/provider-auth-headers`** + +Run: `git status` +Expected: existing modified files are all unrelated to vault/credential refactor (changelog, docs, provider catalog `.toml` files for the live-provider-cache fix, etc.). If any of those are partial work for *this* refactor, stop and resolve first. + +- [ ] **Step 2: Create the working branch** + +Run: `git switch -c refactor/vault-credential-schema` +Expected: switches off the parent branch with the existing modifications still in the working tree (cleanest base). + +- [ ] **Step 3: Sanity-check the baseline build** + +Run: `cargo build --workspace` +Expected: clean build (or only warnings). + +- [ ] **Step 4: Sanity-check the baseline tests** + +Run: `cargo nextest run --workspace` +Expected: all tests green. Note any pre-existing failures so you can distinguish them later. + +--- + +### Task 2: Rename `SecretType` variants in `fabro-types` + +**Files:** +- Modify: `lib/crates/fabro-types/src/secret.rs` + +This is the keystone rename. All compile errors that follow will be tracked down in later tasks; this task is just the type definition change. + +- [ ] **Step 1: Replace the `SecretType` enum** + +Edit `lib/crates/fabro-types/src/secret.rs` to: + +```rust +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; +use strum::Display; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Display, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +#[strum(serialize_all = "snake_case")] +pub enum SecretType { + /// Opaque API-key/PAT-style token value. + #[default] + Token, + /// JSON-encoded `OAuthCredential`. Refreshable; never projected into env. + Oauth, + /// Path-shaped secret materialized to the filesystem. + File, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SecretMetadata { + pub name: String, + #[serde(rename = "type")] + pub secret_type: SecretType, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub description: Option, + pub created_at: DateTime, + pub updated_at: DateTime, +} +``` + +Note: `Oauth` (not `OAuth`) so `serialize_all = "snake_case"` produces `oauth`. Confirm with `cargo expand` or a unit test if unsure. + +- [ ] **Step 2: Add a serialization round-trip test** + +Append to `lib/crates/fabro-types/src/secret.rs`: + +```rust +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn secret_type_serializes_to_snake_case() { + assert_eq!(serde_json::to_string(&SecretType::Token).unwrap(), "\"token\""); + assert_eq!(serde_json::to_string(&SecretType::Oauth).unwrap(), "\"oauth\""); + assert_eq!(serde_json::to_string(&SecretType::File).unwrap(), "\"file\""); + } + + #[test] + fn secret_type_default_is_token() { + assert_eq!(SecretType::default(), SecretType::Token); + } +} +``` + +- [ ] **Step 3: Run the new tests in isolation** + +Run: `cargo nextest run -p fabro-types` +Expected: passes (this crate has no callers of the old variants). + +- [ ] **Step 4: Commit** + +```bash +git add lib/crates/fabro-types/src/secret.rs +git commit -m "$(cat <<'EOF' +refactor(types): rename SecretType variants to schemas + +Environment→Token, Credential→Oauth. Schemas now describe the +shape of the stored secret rather than how it is consumed. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +The workspace will not compile after this commit. Subsequent tasks bring it back to green. + +--- + +### Task 3: Update `fabro-vault` for the new variants and remove bulk export + +**Files:** +- Modify: `lib/crates/fabro-vault/src/lib.rs` + +- [ ] **Step 1: Update `validate_name`** + +Find `validate_name` (around line 177) and replace the match arms: + +```rust +pub fn validate_name(name: &str, secret_type: SecretType) -> Result<(), Error> { + match secret_type { + SecretType::Token | SecretType::Oauth => Self::validate_env_name(name), + SecretType::File => Self::validate_file_name(name), + } +} +``` + +- [ ] **Step 2: Delete `snapshot()` and `credential_entries()`** + +Remove the `snapshot()` method entirely. It is unused in production and would be a foot gun because it bulk-exports vault secrets as env-shaped key/value pairs. + +Remove `credential_entries()` as well. The new resolver uses explicit `vault:NAME` lookups and schema checks; it should not scan all credential-like entries. + +Update `file_secrets` to use the new variant: `SecretType::File` (no change to the variant itself, just confirm it still compiles). + +- [ ] **Step 3: Update vault tests in `lib/crates/fabro-vault/src/lib.rs`** + +In the inline `mod tests`, replace all `SecretType::Environment` → `SecretType::Token` and `SecretType::Credential` → `SecretType::Oauth`. Delete tests for `snapshot()` and `credential_entries()`. Keep or add focused tests that prove `file_secrets()` still returns only `SecretType::File` entries and that token/oauth entries can still be read by name through `get_entry()`. + +- [ ] **Step 4: Run vault tests** + +Run: `cargo nextest run -p fabro-vault` +Expected: all green. + +- [ ] **Step 5: Commit** + +```bash +git add lib/crates/fabro-vault/ +git commit -m "$(cat <<'EOF' +refactor(vault): rename helpers for new SecretType schemas + +Token and Oauth keep env-var-shaped names; File keeps path-shaped names. +Remove snapshot() and credential_entries() so the vault no longer exposes +a bulk secret export/scanning API. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 4: Replace `AuthCredential`/`AuthDetails` with a single `OAuthCredential` + +**Files:** +- Modify: `lib/crates/fabro-auth/src/credential.rs` (major rewrite) +- Modify: `lib/crates/fabro-auth/src/lib.rs` (re-export changes) + +This is the big type-shape change. We delete the api-key envelope entirely and keep only the OAuth structure. + +- [ ] **Step 1: Rewrite `lib/crates/fabro-auth/src/credential.rs`** + +Replace the whole file with: + +```rust +use chrono::{DateTime, Duration, Utc}; +use fabro_redact::redact_string; +use serde::{Deserialize, Serialize}; + +/// JSON shape stored in the vault when `secret_type == Oauth`. +/// +/// There is no `provider` field: provider context comes from the catalog and +/// the auth strategy at resolve time. The current OpenAI Codex device-flow +/// login stores this shape, but the schema itself is generic OAuth. `account_id` +/// is a provider account identifier; today it is populated from OpenAI/ChatGPT +/// claims when available. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct OAuthCredential { + pub tokens: OAuthTokens, + pub config: OAuthConfig, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub account_id: Option, +} + +impl OAuthCredential { + #[must_use] + pub fn needs_refresh(&self) -> bool { + self.tokens.expires_at <= Utc::now() + Duration::minutes(5) + } +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct OAuthTokens { + pub access_token: String, + pub refresh_token: Option, + pub expires_at: DateTime, +} + +pub(crate) fn expires_at_from_now(expires_in: Option) -> DateTime { + let seconds = i64::try_from(expires_in.unwrap_or(3600)).unwrap_or(i64::MAX); + Utc::now() + Duration::seconds(seconds) +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct OAuthConfig { + pub auth_url: String, + pub token_url: String, + pub client_id: String, + pub scopes: Vec, + pub redirect_uri: Option, + pub use_pkce: bool, +} + +#[derive(Clone, PartialEq, Eq)] +pub enum ApiKeyHeader { + Bearer(String), + Custom { name: String, value: String }, +} + +fn redact_for_debug(value: &str) -> String { + let redacted = redact_string(value); + if redacted == value && !value.is_empty() { + "REDACTED".to_string() + } else { + redacted + } +} + +impl std::fmt::Debug for ApiKeyHeader { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Bearer(value) => f + .debug_tuple("Bearer") + .field(&redact_for_debug(value)) + .finish(), + Self::Custom { name, value } => f + .debug_struct("Custom") + .field("name", name) + .field("value", &redact_for_debug(value)) + .finish(), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn fixture(expires_at: DateTime) -> OAuthCredential { + OAuthCredential { + tokens: OAuthTokens { + access_token: "access".to_string(), + refresh_token: Some("refresh".to_string()), + expires_at, + }, + config: OAuthConfig { + auth_url: "https://auth.openai.com".to_string(), + token_url: "https://auth.openai.com/oauth/token".to_string(), + client_id: "client".to_string(), + scopes: vec!["openid".to_string()], + redirect_uri: Some("https://auth.openai.com/deviceauth/callback".to_string()), + use_pkce: true, + }, + account_id: Some("acct_123".to_string()), + } + } + + #[test] + fn round_trips_through_json() { + let credential = fixture(Utc::now() + Duration::hours(1)); + let json = serde_json::to_string(&credential).unwrap(); + let parsed: OAuthCredential = serde_json::from_str(&json).unwrap(); + assert_eq!(parsed, credential); + } + + #[test] + fn needs_refresh_uses_five_minute_buffer() { + assert!(fixture(Utc::now() + Duration::minutes(4)).needs_refresh()); + assert!(!fixture(Utc::now() + Duration::minutes(6)).needs_refresh()); + } + + #[test] + fn api_key_header_debug_redacts_secret_values() { + let header = ApiKeyHeader::Bearer("sk-test".to_string()); + let debug = format!("{header:?}"); + assert!(!debug.contains("sk-test")); + assert!(debug.contains("REDACTED")); + } +} +``` + +- [ ] **Step 2: Update `lib/crates/fabro-auth/src/lib.rs` re-exports** + +Replace the `pub use credential::{...}` block with: + +```rust +pub use credential::{ + ApiKeyHeader, OAuthCredential, OAuthConfig, OAuthTokens, +}; +``` + +Drop the `AuthCredential`, `AuthDetails`, `credential_id_for`, and `parse_credential_secret` re-exports — they no longer exist. + +- [ ] **Step 3: Verify the crate doesn't compile yet — that's expected** + +Run: `cargo build -p fabro-auth 2>&1 | head -40` +Expected: many errors in `resolve.rs`, `vault_ext.rs`, `vault_source.rs`, `strategies/*.rs`, `refresh.rs`. Each one will be fixed in subsequent tasks. + +- [ ] **Step 4: Commit the partial state** + +```bash +git add lib/crates/fabro-auth/src/credential.rs lib/crates/fabro-auth/src/lib.rs +git commit -m "$(cat <<'EOF' +refactor(auth): drop AuthCredential envelope, keep OAuthCredential only + +API-key secrets are no longer wrapped; they live in the vault as plain +token values. The OAuth credential keeps its struct form but loses the +`provider` field; provider-specific behavior stays in the resolver/catalog. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 5: Rewrite `vault_ext` for schema-typed lookups + +**Files:** +- Modify: `lib/crates/fabro-auth/src/vault_ext.rs` (full rewrite) +- Modify: `lib/crates/fabro-auth/src/lib.rs` (update re-exports) + +- [ ] **Step 1: Rewrite `lib/crates/fabro-auth/src/vault_ext.rs`** + +Replace the whole file with: + +```rust +use fabro_types::SecretMetadata; +use fabro_vault::{Error as VaultError, SecretType, Vault}; + +use crate::credential::OAuthCredential; + +/// Errors raised when a `vault:NAME` lookup finds an entry whose schema does +/// not match what the caller expected. +#[derive(Debug, thiserror::Error)] +pub enum VaultLookupError { + #[error("vault entry '{name}' has schema {actual:?}, expected {expected:?}")] + SchemaMismatch { + name: String, + expected: SecretType, + actual: SecretType, + }, + #[error("vault entry '{name}' is not valid {expected:?} JSON: {source}")] + DecodeFailed { + name: String, + expected: SecretType, + #[source] + source: serde_json::Error, + }, +} + +/// Reads the raw token value of a `Token`-typed secret. +/// +/// Returns `Ok(None)` if no entry exists. Returns `Err(SchemaMismatch)` if an +/// entry exists with a different schema. +pub fn vault_get_token(vault: &Vault, name: &str) -> Result, VaultLookupError> { + let Some(entry) = vault.get_entry(name) else { + return Ok(None); + }; + if entry.secret_type != SecretType::Token { + return Err(VaultLookupError::SchemaMismatch { + name: name.to_string(), + expected: SecretType::Token, + actual: entry.secret_type, + }); + } + Ok(Some(entry.value.clone())) +} + +/// Reads and decodes a `Oauth`-typed secret. +pub fn vault_get_oauth( + vault: &Vault, + name: &str, +) -> Result, VaultLookupError> { + let Some(entry) = vault.get_entry(name) else { + return Ok(None); + }; + if entry.secret_type != SecretType::Oauth { + return Err(VaultLookupError::SchemaMismatch { + name: name.to_string(), + expected: SecretType::Oauth, + actual: entry.secret_type, + }); + } + serde_json::from_str(&entry.value) + .map(Some) + .map_err(|source| VaultLookupError::DecodeFailed { + name: name.to_string(), + expected: SecretType::Oauth, + source, + }) +} + +pub fn vault_set_token( + vault: &mut Vault, + name: &str, + value: &str, +) -> Result { + vault.set(name, value, SecretType::Token, None) +} + +pub fn vault_set_oauth( + vault: &mut Vault, + name: &str, + credential: &OAuthCredential, +) -> Result { + let json = serde_json::to_string(credential)?; + vault.set(name, &json, SecretType::Oauth, None) +} + +#[cfg(test)] +mod tests { + use chrono::{Duration, Utc}; + + use super::*; + use crate::credential::{OAuthConfig, OAuthTokens}; + + fn temp_vault() -> Vault { + let dir = tempfile::tempdir().unwrap(); + Vault::open(dir.path()).unwrap() + } + + fn fixture() -> OAuthCredential { + OAuthCredential { + tokens: OAuthTokens { + access_token: "access".to_string(), + refresh_token: Some("refresh".to_string()), + expires_at: Utc::now() + Duration::hours(1), + }, + config: OAuthConfig { + auth_url: "https://auth.openai.com".to_string(), + token_url: "https://auth.openai.com/oauth/token".to_string(), + client_id: "client".to_string(), + scopes: vec!["openid".to_string()], + redirect_uri: None, + use_pkce: true, + }, + account_id: None, + } + } + + #[test] + fn vault_get_token_returns_none_when_absent() { + let vault = temp_vault(); + assert!(vault_get_token(&vault, "ANTHROPIC_API_KEY").unwrap().is_none()); + } + + #[test] + fn vault_get_token_returns_value_when_present() { + let mut vault = temp_vault(); + vault_set_token(&mut vault, "ANTHROPIC_API_KEY", "sk-test").unwrap(); + assert_eq!( + vault_get_token(&vault, "ANTHROPIC_API_KEY").unwrap().as_deref(), + Some("sk-test"), + ); + } + + #[test] + fn vault_get_token_errors_on_oauth_entry() { + let mut vault = temp_vault(); + vault_set_oauth(&mut vault, "OPENAI_CODEX", &fixture()).unwrap(); + let err = vault_get_token(&vault, "OPENAI_CODEX").unwrap_err(); + assert!(matches!(err, VaultLookupError::SchemaMismatch { .. })); + } + + #[test] + fn vault_get_oauth_round_trips() { + let mut vault = temp_vault(); + let credential = fixture(); + vault_set_oauth(&mut vault, "OPENAI_CODEX", &credential).unwrap(); + assert_eq!( + vault_get_oauth(&vault, "OPENAI_CODEX").unwrap().unwrap(), + credential, + ); + } +} +``` + +- [ ] **Step 2: Update `lib/crates/fabro-auth/src/lib.rs` re-exports** + +Replace the `pub use vault_ext::{...}` line with: + +```rust +pub use vault_ext::{ + VaultLookupError, vault_get_oauth, vault_get_token, vault_set_oauth, + vault_set_token, +}; +``` + +Drop the old `vault_credentials_for_provider`, `vault_get_credential`, `vault_set_credential` exports — they are gone. + +- [ ] **Step 3: Commit** + +```bash +git add lib/crates/fabro-auth/src/vault_ext.rs lib/crates/fabro-auth/src/lib.rs +git commit -m "$(cat <<'EOF' +refactor(auth): split vault credential helpers by schema + +vault_get_token and vault_get_oauth replace the polymorphic +vault_get_credential, with explicit SchemaMismatch errors when an +entry's stored type does not match what the caller asked for. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +The crate still won't compile — `resolve.rs`, `vault_source.rs`, `strategies/*.rs`, `refresh.rs`, and `strategy.rs` still reference the old types. That's the next task. + +--- + +### Task 6: Refactor `AuthStrategy` and the two strategy implementations + +**Files:** +- Modify: `lib/crates/fabro-auth/src/strategy.rs` +- Modify: `lib/crates/fabro-auth/src/strategies/api_key.rs` +- Modify: `lib/crates/fabro-auth/src/strategies/codex_device.rs` +- Modify: `lib/crates/fabro-auth/src/refresh.rs` +- Modify: `lib/crates/fabro-auth/src/lib.rs` (export `LoginResult`) + +- [ ] **Step 1: Introduce `LoginResult` in `strategy.rs`** + +Replace `lib/crates/fabro-auth/src/strategy.rs` with: + +```rust +use async_trait::async_trait; +use fabro_model::ProviderId; + +use crate::context::{AuthContextRequest, AuthContextResponse}; +use crate::credential::{OAuthCredential, OAuthConfig}; + +/// What a successful login produces. The login flow inspects this to decide +/// which vault schema to persist into. +#[derive(Debug, Clone)] +pub enum LoginResult { + /// Plain API-key token (will be stored as `SecretType::Token`). + ApiKey { + provider: ProviderId, + key: String, + }, + /// OAuth credential (will be stored as `SecretType::Oauth`). + OAuth { + provider: ProviderId, + credential: OAuthCredential, + }, +} + +#[async_trait] +pub trait AuthStrategy: Send { + async fn init(&mut self) -> anyhow::Result; + async fn complete(&mut self, response: AuthContextResponse) -> anyhow::Result; +} + +#[allow(dead_code)] +fn _config_marker(_: &OAuthConfig) {} +``` + +(The `_config_marker` line is just to silence the unused-import lint if `OAuthConfig` re-export remains needed; remove it once the build settles if clippy is happy.) + +- [ ] **Step 2: Update `strategies/api_key.rs` `complete()` to return `LoginResult::ApiKey`** + +In `lib/crates/fabro-auth/src/strategies/api_key.rs`: + +- Remove `use crate::credential::{AuthCredential, AuthDetails};` +- Add `use crate::strategy::{AuthStrategy, LoginResult};` +- Change the `complete` return type to `anyhow::Result` and the body to: + +```rust +async fn complete(&mut self, response: AuthContextResponse) -> anyhow::Result { + match response { + AuthContextResponse::ApiKey { key } => Ok(LoginResult::ApiKey { + provider: self.provider_id.clone(), + key, + }), + AuthContextResponse::DeviceCodeConfirmed => { + Err(anyhow::anyhow!("expected API key response")) + } + } +} +``` + +- Also fix the `CredentialRef::Credential(_) => None,` arm: it becomes `CredentialRef::Vault(_) => None,` once Task 8 lands. Leave it for now — this file will compile-error until then. + +- [ ] **Step 3: Update `strategies/codex_device.rs` to return `LoginResult::OAuth`** + +In `lib/crates/fabro-auth/src/strategies/codex_device.rs`: + +- Replace any `AuthCredential { provider, details: AuthDetails::CodexOAuth { tokens, config, account_id } }` construction with `LoginResult::OAuth { provider, credential: OAuthCredential { tokens, config, account_id } }`. +- Update the `complete` return type to `anyhow::Result`. +- Remove imports of `AuthCredential`/`AuthDetails`, add `OAuthCredential` and `LoginResult`. + +(Read the file before editing — the device-code flow has a refresh-on-completion path that may also need adjustment.) + +- [ ] **Step 4: Update `refresh.rs` to operate on `OAuthCredential`** + +In `lib/crates/fabro-auth/src/refresh.rs`, change the public signature from `refresh(credential: &AuthCredential, ...) -> Result` to: + +```rust +pub async fn refresh( + credential: &OAuthCredential, + http: &reqwest::Client, +) -> Result +``` + +Drop the `match &credential.details` — there is only the OAuth case now. The body that built a new `AuthCredential` should build a `OAuthCredential` directly. Keep `expires_at_from_now` import. + +- [ ] **Step 5: Re-export `LoginResult` from `lib.rs`** + +In `lib/crates/fabro-auth/src/lib.rs`, add `pub use strategy::{AuthStrategy, LoginResult};` (replace the existing `pub use strategy::AuthStrategy;` line). + +- [ ] **Step 6: Don't commit yet** + +The crate still doesn't compile — `resolve.rs` and `vault_source.rs` haven't been updated. We commit after the next task. + +--- + +### Task 7: Refactor `resolve.rs` and `vault_source.rs` (and `env_source.rs`) + +**Files:** +- Modify: `lib/crates/fabro-auth/src/resolve.rs` (significant changes) +- Modify: `lib/crates/fabro-auth/src/vault_source.rs` +- Modify: `lib/crates/fabro-auth/src/env_source.rs` + +This is the resolver core: `vault:NAME` reads the vault by schema; `env:NAME` reads only the process env. + +- [ ] **Step 1: Update `env_source.rs` — `env:` is now process-env-only** + +In `lib/crates/fabro-auth/src/env_source.rs`: + +- The existing logic that does `let CredentialRef::Env(name) = credential_ref else { return None; }` and then `self.lookup(name)` is already correct — it reads only from the env lookup, not the vault. No behavior change needed here. +- Update `CredentialRef::Credential(_)` arms in any iteration to `CredentialRef::Vault(_)` (the rename lands in Task 8; leave a `// FIXME(vault-rename)` for now if needed and revisit). +- Remove the doc string that says "facade for provider API-key process-env" if it's misleading; replace with "Resolves `env:NAME` references from the process environment only." + +- [ ] **Step 2: Rewrite `credential_from_ref` and related helpers in `resolve.rs`** + +In `lib/crates/fabro-auth/src/resolve.rs`: + +- Drop `lookup_env_or_vault`. Inline the two cases: + - `CredentialRef::Env(name)` → `(self.env_lookup)(name)` only. + - `CredentialRef::Vault(name)` → inspect `vault.get_entry(name)`, branch by `entry.secret_type`, and error on schema mismatch. +- The resolver no longer returns `Option`; it returns `Result, ResolveError>` where: + +```rust +pub(crate) enum ResolvedSecret { + ApiKey(String), + OAuth(OAuthCredential), +} +``` + +- For each `CredentialRef` in `credentials = [...]`, try in order: + - `Env(name)` → if `Some(value)`, return `ResolvedSecret::ApiKey(value)`; else continue. + - `Vault(name)` → look up the entry. If absent, continue. If present, branch on `secret_type`: + - `Token` → `ResolvedSecret::ApiKey(value)` + - `Oauth` → decode and return `ResolvedSecret::OAuth(credential)` + - `File` → return `ResolveError::SchemaMismatch` (a `vault:` ref can never resolve to a file secret). +- Update the OAuth refresh path that currently round-trips an `AuthCredential`: read `vault_get_oauth`, call `refresh::refresh(&credential, http)`, write back via `vault_set_oauth`. The vault name comes from the `CredentialRef::Vault(name)` that resolved to the OAuth secret in the first place; thread that name through. +- Replace every `AuthDetails::ApiKey { key }` / `AuthDetails::CodexOAuth { .. }` match with the new `ResolvedSecret` shape. + +- [ ] **Step 3: Update the `ApiCredential`-build paths** + +The functions that build an `ApiCredential` from an old `AuthCredential` (look near `build_api_key_header`, line ~335 and ~407) should now take `ResolvedSecret` instead. The branching is the same: api-key → `build_api_key_header(policy, key)`; oauth → bearer token. The OpenAI Codex-mode auto-configuration block (sets `base_url`, `codex_mode`, `originator`, `ChatGPT-Account-Id`) should fire only when the provider is OpenAI and the resolved secret is `ResolvedSecret::OAuth`. + +- [ ] **Step 4: Update `vault_source.rs`** + +In `lib/crates/fabro-auth/src/vault_source.rs`: + +- Update the test helpers: `api_key_credential` becomes a function that returns a `String` (or you inline it). `expired_openai_credential` returns `OAuthCredential`. +- Replace `vault.set(name, &serde_json::to_string(&api_key_credential).unwrap(), SecretType::Credential, None)` with `vault_set_token(&mut vault, name, &api_key)`. +- Replace `vault.set(name, &serde_json::to_string(&oauth_credential).unwrap(), SecretType::Credential, None)` with `vault_set_oauth(&mut vault, name, &oauth_credential)`. +- Update assertions to check `ResolvedSecret` variants. + +- [ ] **Step 5: Update `resolve.rs` tests** + +`lib/crates/fabro-auth/src/resolve.rs` has ~600 lines of tests using `AuthCredential`/`vault_set_credential`. Rewrite each test to use `vault_set_token` / `vault_set_oauth` and the new `CredentialRef::Vault(_)` variant. Notable tests to keep working: + +- `with_env_lookup_overrides_vault_settings` — semantics change: env now wins because `env:` is listed first in the test's catalog. Verify the test asserts the right precedence rule. +- The refresh-on-resolve test (around line 1062) that uses `vault_get_credential` — switch to `vault_get_oauth`. + +Delete tests that exercised the env-falls-back-to-vault behavior or the `credential_id_for` rule — they're testing removed code. + +- [ ] **Step 6: Run the auth crate tests** + +Run: `cargo nextest run -p fabro-auth` +Expected: green. If a test you deleted was load-bearing for a particular bug, add a replacement that exercises the new behavior (schema mismatch error, etc.). + +- [ ] **Step 7: Commit** + +```bash +git add lib/crates/fabro-auth/ +git commit -m "$(cat <<'EOF' +refactor(auth): resolve credentials via explicit env vs vault sources + +env:NAME reads only the process environment; vault:NAME reads only the +vault and branches on the entry's schema (Token → ApiKey, Oauth → +OAuth bearer). File entries cannot satisfy a credential ref. Removes +the old env-or-vault fallback that conflated the two sources. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 8: Rename `CredentialRef::Credential` → `CredentialRef::Vault`, change prefix + +**Files:** +- Modify: `lib/crates/fabro-model/src/catalog.rs` + +- [ ] **Step 1: Update the enum and its parse impl** + +In `lib/crates/fabro-model/src/catalog.rs` around line 154, rewrite: + +```rust +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(into = "String", try_from = "String")] +pub enum CredentialRef { + Vault(String), + Env(String), +} + +impl std::fmt::Display for CredentialRef { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Vault(name) => write!(f, "vault:{name}"), + Self::Env(name) => write!(f, "env:{name}"), + } + } +} + +impl FromStr for CredentialRef { + type Err = CredentialRefParseError; + + fn from_str(value: &str) -> Result { + if let Some(name) = value.strip_prefix("vault:") { + if name.is_empty() { + return Err(CredentialRefParseError::EmptyVault); + } + return Ok(Self::Vault(name.to_string())); + } + if let Some(name) = value.strip_prefix("env:") { + if name.is_empty() { + return Err(CredentialRefParseError::EmptyEnv); + } + return Ok(Self::Env(name.to_string())); + } + Err(CredentialRefParseError::Invalid) + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] +pub enum CredentialRefParseError { + #[error("credential reference must be `vault:` or `env:`")] + Invalid, + #[error("credential reference is missing a name after `vault:`")] + EmptyVault, + #[error("credential reference is missing a name after `env:`")] + EmptyEnv, +} +``` + +- [ ] **Step 2: Update inline tests in `catalog.rs`** + +Search the file for `"credential:"`, `CredentialRef::Credential(`, and `EmptyCredential` — update each to the new names. The inline test fixtures around line 2795 (`credentials = ["credential:bearer", "env:BEARER_API_KEY"]`) become `["env:BEARER_API_KEY", "vault:BEARER_API_KEY"]`. Likewise around 2839. + +- [ ] **Step 3: Find and update any other `CredentialRef::Credential` matches** + +Run: `rg -n 'CredentialRef::Credential\b' lib/` +Expected output sites (each must be updated to `Vault`): `strategies/api_key.rs`, `resolve.rs`, `cli/commands/install.rs`, `workflow/handler/llm/launch_env.rs`, `config/layers/llm.rs`. + +For each: rename the variant and (if the surrounding logic was reading from process env as a fallback for a `Credential` ref) align it with the new resolver semantics. + +- [ ] **Step 4: Run the workspace build** + +Run: `cargo build --workspace` +Expected: green. If TOML parsing tests fail, that's because the provider catalogs still use `credential:` — that's the next task. + +- [ ] **Step 5: Commit** + +```bash +git add -u +git commit -m "$(cat <<'EOF' +refactor(model): rename CredentialRef::Credential to Vault + +Prefix changes from `credential:` to `vault:` to match the resolver's +new semantics (vault-only, never falls back to env). + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 9: Update all provider catalog TOMLs + +**Files:** +- Modify: `lib/crates/fabro-model/src/catalog/providers/anthropic.toml` +- Modify: `lib/crates/fabro-model/src/catalog/providers/gemini.toml` +- Modify: `lib/crates/fabro-model/src/catalog/providers/inception.toml` +- Modify: `lib/crates/fabro-model/src/catalog/providers/kimi.toml` +- Modify: `lib/crates/fabro-model/src/catalog/providers/litellm.toml` +- Modify: `lib/crates/fabro-model/src/catalog/providers/minimax.toml` +- Modify: `lib/crates/fabro-model/src/catalog/providers/openai.toml` +- Modify: `lib/crates/fabro-model/src/catalog/providers/venice.toml` +- Modify: `lib/crates/fabro-model/src/catalog/providers/zai.toml` +- (Ollama has no auth — confirm `lib/crates/fabro-model/src/catalog/providers/ollama.toml` and skip if unchanged.) + +For each provider, the rule is: +- Reorder to put `env:` first (so process env wins). +- Use the same name on both sides (matches user convention). +- Drop the magic `credential:openai_codex` — replace with `vault:OPENAI_CODEX` and rely on the resolver branching on entry schema. + +- [ ] **Step 1: Update Anthropic** + +In `anthropic.toml`, change: + +```toml +credentials = ["credential:anthropic", "env:ANTHROPIC_API_KEY"] +``` + +to: + +```toml +credentials = ["env:ANTHROPIC_API_KEY", "vault:ANTHROPIC_API_KEY"] +``` + +- [ ] **Step 2: Update Gemini** + +```toml +credentials = ["env:GEMINI_API_KEY", "env:GOOGLE_API_KEY", "vault:GEMINI_API_KEY"] +``` + +- [ ] **Step 3: Update OpenAI** + +```toml +credentials = ["env:OPENAI_API_KEY", "vault:OPENAI_API_KEY", "vault:OPENAI_CODEX"] +``` + +- [ ] **Step 4: Update Kimi, Venice, Inception, LiteLLM, MiniMax, Zai, Ollama** + +Apply the same `["env:NAME", "vault:NAME"]` pattern to each remaining provider TOML. + +- [ ] **Step 5: Run catalog tests** + +Run: `cargo nextest run -p fabro-model` +Expected: green. If a snapshot fails because the TOML serialized representation changed, run `cargo insta pending-snapshots`, verify the diff is the expected rename, then `cargo insta accept --snapshot `. + +- [ ] **Step 6: Commit** + +```bash +git add lib/crates/fabro-model/src/catalog/providers/ +git commit -m "$(cat <<'EOF' +refactor(catalog): update provider credentials to vault: prefix + +env:NAME now precedes vault:NAME so process env wins. Names are +aligned across both sources. Drops the magic openai_codex id in +favor of vault:OPENAI_CODEX with schema-driven dispatch. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 10: Update OpenAPI schema and regenerate `fabro-api` types + +**Files:** +- Modify: `docs/public/api-reference/fabro-api.yaml` (SecretType enum) +- Affected (auto-regenerated): `lib/crates/fabro-api/` +- Modify: `lib/crates/fabro-api/tests/secret_type_round_trip.rs` +- Modify: `lib/crates/fabro-api/tests/secret_metadata_round_trip.rs` + +- [ ] **Step 1: Update the OpenAPI SecretType enum** + +In `docs/public/api-reference/fabro-api.yaml` around line 10387, change: + +```yaml +SecretType: + description: The way a secret is consumed by the sandbox. + type: string + enum: + - environment + - file + - credential +``` + +to: + +```yaml +SecretType: + description: Schema of a stored secret. + type: string + enum: + - token + - oauth + - file +``` + +Also update the `SecretMetadata.name` example if it still hints at the old terminology. + +- [ ] **Step 2: Rebuild `fabro-api`** + +Run: `cargo build -p fabro-api` +Expected: progenitor regenerates the Rust enum. The build will fail if any consumer references the old variants — those get fixed in later tasks. + +- [ ] **Step 3: Update the API round-trip tests** + +In `lib/crates/fabro-api/tests/secret_type_round_trip.rs`, update the test cases to cover `Token`, `Oauth`, `File`. Same for `secret_metadata_round_trip.rs` if it references variants by name. + +- [ ] **Step 4: Run the api crate tests** + +Run: `cargo nextest run -p fabro-api` +Expected: green. + +- [ ] **Step 5: Commit** + +```bash +git add docs/public/api-reference/fabro-api.yaml lib/crates/fabro-api/ +git commit -m "$(cat <<'EOF' +refactor(api): update SecretType enum to schema-shaped variants + +token | oauth | file. Regenerates fabro-api types via +progenitor. Tests updated for the new wire vocabulary. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 11: Update the server secrets handler and install flow + +**Files:** +- Modify: `lib/crates/fabro-server/src/server/handler/secrets.rs` +- Modify: `lib/crates/fabro-server/src/install.rs` +- Modify: `lib/crates/fabro-server/src/diagnostics.rs` +- Modify: `lib/crates/fabro-server/src/run_manifest.rs` +- Modify: `lib/crates/fabro-server/src/server.rs` (re-exports) +- Modify: `lib/crates/fabro-server/src/server/tests.rs` +- Modify: `lib/crates/fabro-server/src/test_support.rs` +- Modify: `lib/crates/fabro-server/tests/it/api/install.rs` + +- [ ] **Step 1: Update `secrets.rs` create handler** + +In `lib/crates/fabro-server/src/server/handler/secrets.rs`: + +- Drop the import and usage of `parse_credential_secret` — it no longer exists. +- Replace the `if secret_type == SecretType::Credential { ... parse_credential_secret ... }` block with the equivalent for `SecretType::Oauth`: validate that `value` deserializes to `OAuthCredential` and reject with `bad_request` on failure. +- Update `SecretType::Environment` → `SecretType::Token` in the Daytona validation block. + +Example replacement for the validation block: + +```rust +if secret_type == SecretType::Oauth { + if let Err(err) = serde_json::from_str::(&value) { + return ApiError::bad_request(format!("invalid oauth credential JSON: {err}")).into_response(); + } +} +if secret_type == SecretType::Token && name == EnvVars::DAYTONA_API_KEY { + // ... existing daytona check, unchanged body ... +} +``` + +Add an import for `fabro_auth::OAuthCredential` if not already present (you may need to thread it through the `server.rs` re-export hub at line 92). + +- [ ] **Step 2: Update `install.rs` provider-secret persistence** + +In `lib/crates/fabro-server/src/install.rs` around line 1537–1559, replace the `AuthCredential { ... details: AuthDetails::ApiKey { key } }` construction with: + +```rust +for provider in llm.providers { + let name = provider_secret_name(&provider.provider); + vault_secrets.push(VaultSecretWrite { + name, + value: provider.api_key, + secret_type: VaultSecretType::Token, + description: None, + }); +} +``` + +Where `provider_secret_name` returns e.g. `"ANTHROPIC_API_KEY"` for Anthropic — match the names used in the provider TOMLs from Task 9. The simplest implementation: a `match` on `ProviderId` that returns the conventional env-var name, or pull it from the catalog's first `CredentialRef::Vault` entry (preferred, single source of truth). + +- [ ] **Step 3: Update `install.rs` Daytona/GitHub blocks** + +Change `VaultSecretType::Environment` → `VaultSecretType::Token` for the Daytona and GitHub token writes (around lines 1533 and 1578). + +- [ ] **Step 4: Update `diagnostics.rs` and `run_manifest.rs`** + +In `lib/crates/fabro-server/src/diagnostics.rs` around line 747 and `lib/crates/fabro-server/src/run_manifest.rs` around line 2342, change `SecretType::Credential` references. Read the surrounding context to decide whether the right replacement is `Token` (it's exposing an api-key value) or `Oauth` (it's the OAuth record). Most likely `Token` for diagnostics, `Oauth` if it specifically references an OAuth secret. + +- [ ] **Step 5: Update `server.rs` re-export hub (around line 92)** + +Drop the `parse_credential_secret` from the use list. Update any `SecretType` variant references that flow through. + +- [ ] **Step 6: Update `server/tests.rs` and `tests/it/api/install.rs`** + +Rename all `SecretType::Environment` → `SecretType::Token` and `SecretType::Credential` → `SecretType::Oauth`. Where tests previously inserted a JSON `AuthCredential` blob under `SecretType::Credential`, replace with `vault_set_token`/`vault_set_oauth` helpers. + +The test at `server/tests.rs:6158` (`SecretType::Credential` for `GITHUB_TOKEN`) is suspect — `GITHUB_TOKEN` is a token secret, not OAuth. It should be `Token`. + +- [ ] **Step 7: Build and test** + +Run: `cargo nextest run -p fabro-server` +Expected: green. Accept any insta snapshots whose diffs are the schema rename (verify first with `cargo insta pending-snapshots`). + +- [ ] **Step 8: Commit** + +```bash +git add lib/crates/fabro-server/ +git commit -m "$(cat <<'EOF' +refactor(server): use schema-typed SecretType everywhere + +CreateSecret handler validates oauth JSON shape on write. Install +flow writes API keys as Token secrets (not the old JSON envelope). +Daytona and GitHub tokens move to SecretType::Token. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 12: Update CLI args, secret commands, and provider login + +**Files:** +- Modify: `lib/crates/fabro-cli/src/args.rs` +- Modify: `lib/crates/fabro-cli/src/commands/secret/set.rs` +- Modify: `lib/crates/fabro-cli/src/commands/provider/login.rs` +- Modify: `lib/crates/fabro-cli/src/commands/install.rs` +- Modify: `lib/crates/fabro-cli/src/commands/run/runner.rs` +- Modify: `lib/crates/fabro-cli/src/shared/provider_auth.rs` +- Modify: `lib/crates/fabro-cli/tests/it/cmd/doctor.rs` +- Modify: `lib/crates/fabro-cli/tests/it/cmd/install.rs` +- Modify: `lib/crates/fabro-cli/tests/it/cmd/run.rs` +- Modify: `lib/crates/fabro-cli/tests/it/workflow/acp.rs` +- Modify: `lib/crates/fabro-cli/tests/it/workflow/hooks.rs` + +- [ ] **Step 1: Update `SecretTypeArg` in `args.rs`** + +Around line 656: + +```rust +#[derive(Clone, Copy, Debug, ValueEnum)] +pub(crate) enum SecretTypeArg { + Token, + File, +} +``` + +Drop `Environment` (replaced by `Token`). Do **not** add `Oauth` here — oauth secrets are written by `fabro provider login`, never by `fabro secret set`. + +Update the default in `SecretSetArgs.r#type`: + +```rust +#[arg(long, value_enum, default_value = "token")] +pub(crate) r#type: SecretTypeArg, +``` + +- [ ] **Step 2: Update `commands/secret/set.rs`** + +```rust +fn api_secret_type(secret_type: SecretTypeArg) -> types::SecretType { + match secret_type { + SecretTypeArg::Token => types::SecretType::Token, + SecretTypeArg::File => types::SecretType::File, + } +} +``` + +- [ ] **Step 3: Rewrite `commands/provider/login.rs`** + +Replace the whole file with: + +```rust +use anyhow::Result; +use fabro_api::types; +use fabro_auth::LoginResult; +use fabro_util::terminal::Styles; + +use crate::args::ProviderLoginArgs; +use crate::command_context::CommandContext; +use crate::shared::provider_auth; + +pub(super) async fn login_command( + args: ProviderLoginArgs, + base_ctx: &CommandContext, +) -> Result<()> { + base_ctx.require_no_json_override()?; + let printer = base_ctx.printer(); + let s = Styles::detect_stderr(); + let ctx = base_ctx.with_target(&args.target)?; + let server = ctx.server().await?; + let result = if args.api_key_stdin { + provider_auth::authenticate_provider_with_api_key_source_and_catalog( + args.provider, + provider_auth::ApiKeySource::Stdin, + &s, + printer, + ctx.catalog()?, + ) + .await? + } else { + provider_auth::authenticate_provider_with_catalog( + args.provider, + &s, + printer, + ctx.catalog()?, + ) + .await? + }; + + let (name, value, type_) = match result { + LoginResult::ApiKey { provider, key } => { + let name = api_key_secret_name(&provider, ctx.catalog()?); + (name, key, types::SecretType::Token) + } + LoginResult::OAuth { credential, .. } => { + ("OPENAI_CODEX".to_string(), serde_json::to_string(&credential)?, types::SecretType::Oauth) + } + }; + + server + .create_secret(types::CreateSecretRequest { + name: name.clone(), + value, + type_, + description: None, + }) + .await?; + fabro_util::printerr!(printer, " {} Saved {}", s.green.apply_to("✔"), name); + Ok(()) +} + +/// Returns the first `vault:NAME` from the provider's catalog `credentials` +/// list, falling back to the provider id uppercased + `_API_KEY`. +fn api_key_secret_name(provider: &fabro_model::ProviderId, catalog: &fabro_model::Catalog) -> String { + use fabro_model::CredentialRef; + + catalog + .provider(provider) + .and_then(|p| p.auth.as_ref()) + .and_then(|auth| { + auth.credentials.iter().find_map(|r| match r { + CredentialRef::Vault(name) => Some(name.clone()), + CredentialRef::Env(_) => None, + }) + }) + .unwrap_or_else(|| format!("{}_API_KEY", provider.to_string().to_uppercase())) +} +``` + +(Adapt the imports and `catalog.provider(...).auth` access path to match the actual API of the catalog accessor. The point is: the secret name comes from the catalog, not a magic constant.) + +- [ ] **Step 4: Update `shared/provider_auth.rs`** + +Search for return types `AuthCredential` and change them to `LoginResult`. The function bodies that built `AuthCredential::ApiKey` should construct `LoginResult::ApiKey`; same for the OAuth path. + +- [ ] **Step 5: Update `commands/install.rs`** + +Around line 1180 and 2958, change `SecretType` references and `CredentialRef::Credential(_)` matches per the new vocabulary. Test fixtures that write JSON `AuthCredential` blobs become `Token` secrets. + +- [ ] **Step 6: Update CLI tests** + +For each of `tests/it/cmd/doctor.rs`, `cmd/install.rs`, `cmd/run.rs`, `workflow/acp.rs`, `workflow/hooks.rs`: + +- Replace `SecretType::Environment` → `SecretType::Token`, `SecretType::Credential` → `SecretType::Oauth`. +- Replace JSON `AuthCredential` writes with `vault_set_token`/`vault_set_oauth`. +- Accept insta snapshot diffs that are the rename only (verify first). + +- [ ] **Step 7: Build and test** + +Run: `cargo build --workspace && cargo nextest run -p fabro-cli` +Expected: green. Use `cargo insta pending-snapshots` and accept renames. + +- [ ] **Step 8: Commit** + +```bash +git add lib/crates/fabro-cli/ +git commit -m "$(cat <<'EOF' +refactor(cli): use LoginResult and schema-typed secrets + +fabro secret set --type now accepts token|file (default token). +fabro provider login picks the vault name from the catalog and writes +either a Token or Oauth secret based on the login result. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 13: Update remaining workflow/config references + +**Files:** +- Modify: `lib/crates/fabro-config/src/layers/llm.rs` +- Modify: `lib/crates/fabro-config/src/layers/combine.rs` +- Modify: `lib/crates/fabro-config/src/layers/mod.rs` +- Modify: `lib/crates/fabro-config/src/lib.rs` +- Modify: `lib/crates/fabro-workflow/src/handler/llm/api.rs` +- Modify: `lib/crates/fabro-workflow/src/handler/llm/launch_env.rs` +- Modify: `lib/crates/fabro-workflow/src/pipeline/initialize.rs` +- Modify: `lib/crates/fabro-workflow/src/pipeline/pull_request.rs` +- Modify: `lib/crates/fabro-workflow/tests/it/integration.rs` +- Modify: `lib/crates/fabro-install/src/lib.rs` + +- [ ] **Step 1: Sweep `CredentialRef::Credential` → `CredentialRef::Vault`** + +Run: `rg -n 'CredentialRef::Credential\b'` — should be empty if Task 8 was thorough. Any straggler: rename and recheck its surrounding logic for env-fallback assumptions. + +- [ ] **Step 2: Sweep `SecretType::Environment` and `SecretType::Credential`** + +Run: `rg -n 'SecretType::Environment|SecretType::Credential\b'` — should be empty. Rename any straggler to `Token`/`Oauth`. + +- [ ] **Step 3: Update `config/layers/llm.rs` tests** + +The tests at `config/layers/llm.rs:217`, `223`, `277`, `500`, `501`, `756`, `764` use `CredentialRef::Credential(...)` and `CredentialRef::Env(...)` literals. Rename the `Credential` ones to `Vault` and update the string under test (`"credential:openai_codex"` → `"vault:OPENAI_CODEX"`, etc.) to match the new prefix. + +- [ ] **Step 4: Update workflow llm/launch_env.rs** + +Look at line 84 — the `let CredentialRef::Env(name) = credential_ref else { return None; }` pattern. This is filtering for env vars when constructing the spawned-process env. Keep this site `env:`-only. `vault:` token secrets must not be implicitly projected into spawned process env; if a workflow needs vault-backed environment values later, add an explicit mapping feature rather than reintroducing bulk vault export. + +- [ ] **Step 5: Build and test the affected crates** + +Run: `cargo nextest run -p fabro-config -p fabro-workflow -p fabro-install` +Expected: green. + +- [ ] **Step 6: Commit** + +```bash +git add -u +git commit -m "$(cat <<'EOF' +refactor: update workflow/config/install for vault: prefix and schemas + +Final consumer updates: CredentialRef::Vault rename propagates, +SecretType variant references aligned with the new vocabulary. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 14: Regenerate TypeScript API client + +**Files:** +- Auto-regenerated: `lib/packages/fabro-api-client/` + +- [ ] **Step 1: Regenerate** + +Run: `cd lib/packages/fabro-api-client && bun run generate` +Expected: TS types now expose `'token' | 'oauth' | 'file'` for `SecretType`. + +- [ ] **Step 2: Build the package** + +Run: `cd lib/packages/fabro-api-client && bun run typecheck && bun run build` +Expected: green. + +- [ ] **Step 3: Build the web app** + +Run: `cd apps/fabro-web && bun run typecheck` +Expected: green. (The web UI doesn't reference `SecretType` by literal — verified during plan discovery — so no source changes should be needed.) + +- [ ] **Step 4: Commit** + +```bash +git add lib/packages/fabro-api-client/ +git commit -m "$(cat <<'EOF' +chore(api-client): regenerate TS client for new SecretType variants + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 15: Update documentation + +**Files:** +- Modify: `docs/public/reference/sdk.mdx` (if it documents the SecretType enum) +- Modify: `docs/public/reference/user-configuration.mdx` +- Modify: `docs/public/core-concepts/models.mdx` +- Modify: `docs/public/changelog/2026-05-13.mdx` (or create a new dated entry for today) +- Modify: `docs/internal/server-secrets-strategy.md` (if it describes the old schema) + +- [ ] **Step 1: Update docs that show TOML examples** + +Search docs for `credential:` literally and replace with `vault:`. Search for descriptions of `SecretType::Environment` / `SecretType::Credential` and update. + +Run: `rg -n 'credential:|SecretType::Environment|SecretType::Credential' docs/` +Expected after edits: only references inside the changelog entry describing the rename itself. + +- [ ] **Step 2: Add a changelog entry** + +Create `docs/public/changelog/2026-05-18.mdx` (or append to an existing 2026-05-18 entry if one exists from earlier work today): + +```mdx +--- +title: "Vault and credential source cleanup" +date: 2026-05-18 +--- + +The provider credential reference grammar is now explicit: +`env:NAME` reads only the process environment, `vault:NAME` reads only +the vault. Vault secrets carry a schema (`token`, `oauth`, or +`file`) describing what they hold; the resolver branches on that schema +to build the right credential. API keys are stored as plain tokens, +not JSON envelopes. +``` + +- [ ] **Step 3: Update `docs/internal/server-secrets-strategy.md` if needed** + +Read it. If it describes the old `Environment`/`Credential` schema, update to reflect the new `Token`/`Oauth`/`File` vocabulary. If it doesn't touch that, leave it alone. + +- [ ] **Step 4: Commit** + +```bash +git add docs/ +git commit -m "$(cat <<'EOF' +docs: update for vault: prefix and schema-typed SecretType + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 16: Final verification + +**Files:** +- (none — verification only) + +- [ ] **Step 1: Full workspace build** + +Run: `cargo build --workspace` +Expected: clean. + +- [ ] **Step 2: Full workspace tests** + +Run: `cargo nextest run --workspace` +Expected: all green. If any insta snapshots are still pending, check them with `cargo insta pending-snapshots` and accept the rename diffs only. + +- [ ] **Step 3: Format and lint** + +Run: `cargo +nightly-2026-04-14 fmt --all` +Run: `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings` +Expected: both green. + +- [ ] **Step 4: Search for residue** + +Run: `rg -n '"credential:"|AuthCredential\b|AuthDetails\b|credential_id_for|parse_credential_secret|vault_get_credential|vault_set_credential|vault_get_string|vault_set_string|vault_credentials_for_provider|credential_entries\b|SecretType::Environment\b|SecretType::Credential\b|SecretType::String\b|SecretType::CodexOauth\b|CredentialRef::Credential\b|codex_oauth' lib/ apps/ docs/` +Expected: zero hits in source code. Any hits in markdown changelogs describing the migration itself are fine. + +Run: `rg -n 'pub fn snapshot|\.snapshot\(\)' lib/crates/fabro-vault/` +Expected: zero hits. `Vault::snapshot()` should be gone. + +- [ ] **Step 5: TypeScript build** + +Run: `cd apps/fabro-web && bun run typecheck && bun run build` +Run: `cd lib/packages/fabro-api-client && bun run typecheck` +Expected: green. + +- [ ] **Step 6: Spot-check live provider auth (optional but recommended)** + +If you have credentials in `.env`: + +Run: `set -a && source .env && set +a && cargo nextest run -p fabro-llm --profile e2e --run-ignored only --test-threads 1` +Expected: live provider tests pass. This exercises the env: path with real keys. + +- [ ] **Step 7: Final commit if formatting changed anything** + +```bash +git status +# if dirty: +git add -u +git commit -m "$(cat <<'EOF' +style: cargo fmt after vault/credential cleanup + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Resolved Decisions and Remaining Notes + +1. **`OPENAI_CODEX` secret name.** Use `OPENAI_CODEX` as the canonical vault entry name for the OpenAI Codex OAuth credential. Task 9 uses it in the OpenAI provider TOML; Task 12 hardcodes the same in `provider/login.rs`. The validator in `vault.set` enforces env-var-shaped names, so uppercase is intentional. + +2. **`vault:` references to `File` secrets.** Treat this as a hard error (Task 7 step 2), not a silent miss. A schema mismatch means the catalog/vault configuration is wrong. + +3. **No bulk vault env projection.** `Vault::snapshot()` is removed. `Token` means opaque secret material, not "project this to env." If workflow commands later need vault-backed environment variables, add an explicit mapping feature. + +4. **`provider_auth::authenticate_provider_with_catalog` return type.** Task 12 step 4 assumes it returns the OAuth/api-key login result. If it currently returns a fully-built `AuthCredential` *with* a `provider` field that downstream code relies on, thread `provider` through `LoginResult` (already done in the enum definition) and adjust call sites accordingly. diff --git a/lib/crates/fabro-api/tests/secret_metadata_round_trip.rs b/lib/crates/fabro-api/tests/secret_metadata_round_trip.rs index d2cfe597a..284fe3a7f 100644 --- a/lib/crates/fabro-api/tests/secret_metadata_round_trip.rs +++ b/lib/crates/fabro-api/tests/secret_metadata_round_trip.rs @@ -13,7 +13,7 @@ fn secret_metadata_reuses_canonical_type() { fn secret_metadata_round_trips_representative_json() { let value = json!({ "name": "ANTHROPIC_API_KEY", - "type": "environment", + "type": "token", "description": "Anthropic API key", "created_at": "2026-04-29T12:34:56Z", "updated_at": "2026-04-29T12:40:00Z" @@ -21,7 +21,7 @@ fn secret_metadata_round_trips_representative_json() { let metadata: SecretMetadata = serde_json::from_value(value.clone()).unwrap(); assert_eq!(metadata.name, "ANTHROPIC_API_KEY"); - assert_eq!(metadata.secret_type, SecretType::Environment); + assert_eq!(metadata.secret_type, SecretType::Token); assert_eq!(metadata.description, Some("Anthropic API key".to_string())); assert_eq!(serde_json::to_value(metadata).unwrap(), value); } diff --git a/lib/crates/fabro-api/tests/secret_type_round_trip.rs b/lib/crates/fabro-api/tests/secret_type_round_trip.rs index d3ff0398b..1343b0f0d 100644 --- a/lib/crates/fabro-api/tests/secret_type_round_trip.rs +++ b/lib/crates/fabro-api/tests/secret_type_round_trip.rs @@ -12,27 +12,27 @@ fn secret_type_reuses_canonical_type() { #[test] fn secret_type_serializes_as_snake_case_strings() { assert_eq!( - serde_json::to_value(SecretType::Environment).unwrap(), - json!("environment") + serde_json::to_value(SecretType::Token).unwrap(), + json!("token") + ); + assert_eq!( + serde_json::to_value(SecretType::Oauth).unwrap(), + json!("oauth") ); assert_eq!( serde_json::to_value(SecretType::File).unwrap(), json!("file") ); - assert_eq!( - serde_json::to_value(SecretType::Credential).unwrap(), - json!("credential") - ); } #[test] fn secret_type_deserializes_each_variant() { - let env: SecretType = serde_json::from_value(json!("environment")).unwrap(); - assert_eq!(env, SecretType::Environment); + let token: SecretType = serde_json::from_value(json!("token")).unwrap(); + assert_eq!(token, SecretType::Token); + let oauth: SecretType = serde_json::from_value(json!("oauth")).unwrap(); + assert_eq!(oauth, SecretType::Oauth); let file: SecretType = serde_json::from_value(json!("file")).unwrap(); assert_eq!(file, SecretType::File); - let cred: SecretType = serde_json::from_value(json!("credential")).unwrap(); - assert_eq!(cred, SecretType::Credential); } fn assert_same_type() { diff --git a/lib/crates/fabro-auth/src/credential.rs b/lib/crates/fabro-auth/src/credential.rs index 7b512a7b9..c8b59ed8c 100644 --- a/lib/crates/fabro-auth/src/credential.rs +++ b/lib/crates/fabro-auth/src/credential.rs @@ -1,41 +1,25 @@ use chrono::{DateTime, Duration, Utc}; -use fabro_model::ProviderId; use fabro_redact::redact_string; use serde::{Deserialize, Serialize}; +/// JSON shape stored in the vault when `secret_type == Oauth`. +/// +/// Provider context comes from the catalog and auth strategy at resolve time. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -pub struct AuthCredential { - pub provider: ProviderId, - #[serde(flatten)] - pub details: AuthDetails, +pub struct OAuthCredential { + pub tokens: OAuthTokens, + pub config: OAuthConfig, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub account_id: Option, } -impl AuthCredential { +impl OAuthCredential { #[must_use] pub fn needs_refresh(&self) -> bool { - match &self.details { - AuthDetails::ApiKey { .. } => false, - AuthDetails::CodexOAuth { tokens, .. } => { - tokens.expires_at <= Utc::now() + Duration::minutes(5) - } - } + self.tokens.expires_at <= Utc::now() + Duration::minutes(5) } } -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -#[serde(tag = "type", rename_all = "snake_case")] -pub enum AuthDetails { - ApiKey { - key: String, - }, - CodexOAuth { - tokens: OAuthTokens, - config: OAuthConfig, - #[serde(default, skip_serializing_if = "Option::is_none")] - account_id: Option, - }, -} - #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct OAuthTokens { pub access_token: String, @@ -89,140 +73,47 @@ impl std::fmt::Debug for ApiKeyHeader { } } -pub fn credential_id_for(credential: &AuthCredential) -> Result { - match &credential.details { - AuthDetails::ApiKey { .. } => Ok(credential.provider.to_string()), - AuthDetails::CodexOAuth { .. } if credential.provider == ProviderId::openai() => { - Ok("openai_codex".to_string()) - } - AuthDetails::CodexOAuth { .. } => Err(format!( - "codex_oauth credentials are only valid for OpenAI, got {}", - credential.provider - )), - } -} - -pub fn parse_credential_secret(name: &str, value: &str) -> Result { - let credential: AuthCredential = - serde_json::from_str(value).map_err(|err| format!("invalid credential JSON: {err}"))?; - let expected_name = credential_id_for(&credential)?; - if name != expected_name { - return Err(format!( - "credential ID must be '{expected_name}' for this credential, got '{name}'" - )); - } - Ok(credential) -} - #[cfg(test)] mod tests { use super::*; - fn oauth_credential(expires_at: DateTime) -> AuthCredential { - AuthCredential { - provider: ProviderId::openai(), - details: AuthDetails::CodexOAuth { - tokens: OAuthTokens { - access_token: "access".to_string(), - refresh_token: Some("refresh".to_string()), - expires_at, - }, - config: OAuthConfig { - auth_url: "https://auth.openai.com".to_string(), - token_url: "https://auth.openai.com/oauth/token".to_string(), - client_id: "client".to_string(), - scopes: vec!["openid".to_string()], - redirect_uri: Some("https://auth.openai.com/deviceauth/callback".to_string()), - use_pkce: true, - }, - account_id: Some("acct_123".to_string()), + fn fixture(expires_at: DateTime) -> OAuthCredential { + OAuthCredential { + tokens: OAuthTokens { + access_token: "access".to_string(), + refresh_token: Some("refresh".to_string()), + expires_at, }, + config: OAuthConfig { + auth_url: "https://auth.openai.com".to_string(), + token_url: "https://auth.openai.com/oauth/token".to_string(), + client_id: "client".to_string(), + scopes: vec!["openid".to_string()], + redirect_uri: Some("https://auth.openai.com/deviceauth/callback".to_string()), + use_pkce: true, + }, + account_id: Some("acct_123".to_string()), } } #[test] - fn auth_credential_round_trips_through_json() { - let credential = oauth_credential(Utc::now() + Duration::hours(1)); + fn round_trips_through_json() { + let credential = fixture(Utc::now() + Duration::hours(1)); let json = serde_json::to_string(&credential).unwrap(); - let parsed: AuthCredential = serde_json::from_str(&json).unwrap(); + let parsed: OAuthCredential = serde_json::from_str(&json).unwrap(); assert_eq!(parsed, credential); } #[test] fn needs_refresh_uses_five_minute_buffer() { - assert!(oauth_credential(Utc::now() + Duration::minutes(4)).needs_refresh()); - assert!(!oauth_credential(Utc::now() + Duration::minutes(6)).needs_refresh()); - } - - #[test] - fn credential_id_for_openai_codex_oauth() { - let credential = oauth_credential(Utc::now() + Duration::hours(1)); - assert_eq!(credential_id_for(&credential).unwrap(), "openai_codex"); - } - - #[test] - fn credential_id_for_openai_api_key() { - let credential = AuthCredential { - provider: ProviderId::openai(), - details: AuthDetails::ApiKey { - key: "sk-test".to_string(), - }, - }; - assert_eq!(credential_id_for(&credential).unwrap(), "openai"); - } - - #[test] - fn credential_id_for_non_openai_codex_oauth_errors() { - let mut credential = oauth_credential(Utc::now() + Duration::hours(1)); - credential.provider = ProviderId::anthropic(); - assert!(credential_id_for(&credential).is_err()); - } - - #[test] - fn parse_credential_secret_validates_name_and_json() { - let credential = oauth_credential(Utc::now() + Duration::hours(1)); - let json = serde_json::to_string(&credential).unwrap(); - - assert!(parse_credential_secret("openai_codex", &json).is_ok()); - assert!(parse_credential_secret("openai", &json).is_err()); - assert!(parse_credential_secret("openai_codex", "{").is_err()); - } - - #[test] - fn credential_id_for_custom_api_key_uses_provider_id() { - let credential = AuthCredential { - provider: ProviderId::new("venice"), - details: AuthDetails::ApiKey { - key: "sk-test".to_string(), - }, - }; - - assert_eq!(credential_id_for(&credential).unwrap(), "venice"); - } - - #[test] - fn parse_credential_secret_accepts_custom_provider_api_key() { - let credential = AuthCredential { - provider: ProviderId::new("venice"), - details: AuthDetails::ApiKey { - key: "sk-test".to_string(), - }, - }; - let json = serde_json::to_string(&credential).unwrap(); - - assert_eq!( - parse_credential_secret("venice", &json).unwrap(), - credential - ); - assert!(parse_credential_secret("openai", &json).is_err()); + assert!(fixture(Utc::now() + Duration::minutes(4)).needs_refresh()); + assert!(!fixture(Utc::now() + Duration::minutes(6)).needs_refresh()); } #[test] fn api_key_header_debug_redacts_secret_values() { let header = ApiKeyHeader::Bearer("sk-test".to_string()); - let debug = format!("{header:?}"); - assert!(!debug.contains("sk-test")); assert!(debug.contains("REDACTED")); } diff --git a/lib/crates/fabro-auth/src/env_source.rs b/lib/crates/fabro-auth/src/env_source.rs index af2c32cca..ca01a2b3c 100644 --- a/lib/crates/fabro-auth/src/env_source.rs +++ b/lib/crates/fabro-auth/src/env_source.rs @@ -6,6 +6,7 @@ use fabro_model::{Catalog, CredentialRef, HeaderValueRef, ProviderId}; use fabro_static::EnvVars; 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}; #[derive(Clone)] @@ -64,15 +65,10 @@ impl EnvCredentialSource { project_id: None, }; if provider.id == ProviderId::openai() && cred.auth_header.is_some() { - cred.org_id = self.lookup(EnvVars::OPENAI_ORG_ID); - cred.project_id = self.lookup(EnvVars::OPENAI_PROJECT_ID); if let Some(account_id) = self.lookup(EnvVars::CHATGPT_ACCOUNT_ID) { - cred.base_url = Some("https://chatgpt.com/backend-api/codex".to_string()); - cred.codex_mode = true; - cred.extra_headers - .insert("ChatGPT-Account-Id".to_string(), account_id); - cred.extra_headers - .insert("originator".to_string(), "fabro".to_string()); + 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)) @@ -89,7 +85,7 @@ impl EnvCredentialSource { let value = match value_ref { HeaderValueRef::Literal(value) => Some(value.clone()), HeaderValueRef::Env(name) => self.lookup(name), - HeaderValueRef::Credential(_) => None, + HeaderValueRef::Vault(_) => None, } .ok_or_else(|| ResolveError::NotConfigured(provider.id.clone()))?; Ok((name.clone(), value)) diff --git a/lib/crates/fabro-auth/src/lib.rs b/lib/crates/fabro-auth/src/lib.rs index 9ba21b959..b04257035 100644 --- a/lib/crates/fabro-auth/src/lib.rs +++ b/lib/crates/fabro-auth/src/lib.rs @@ -11,10 +11,7 @@ mod vault_source; pub mod strategies; pub use context::{AuthContextRequest, AuthContextResponse}; -pub use credential::{ - ApiKeyHeader, AuthCredential, AuthDetails, OAuthConfig, OAuthTokens, credential_id_for, - parse_credential_secret, -}; +pub use credential::{ApiKeyHeader, OAuthConfig, OAuthCredential, OAuthTokens}; pub use credential_source::{CredentialSource, ResolvedCredentials}; pub use env_source::EnvCredentialSource; pub use refresh::refresh_oauth_credential; @@ -24,8 +21,12 @@ pub use resolve::{ configured_providers_from_process_env, }; pub use strategy::{ - AuthMethod, AuthStrategy, CODEX_AUTH_URL, CODEX_CLIENT_ID, CODEX_TOKEN_URL, codex_oauth_config, - strategy_for, + AuthMethod, AuthStrategy, CODEX_AUTH_URL, CODEX_CLIENT_ID, CODEX_TOKEN_URL, LoginResult, + codex_oauth_config, strategy_for, +}; +pub use vault_ext::{ + VaultLookupError, vault_get_oauth, vault_get_token, vault_set_oauth, vault_set_token, }; -pub use vault_ext::{vault_credentials_for_provider, vault_get_credential, vault_set_credential}; pub use vault_source::VaultCredentialSource; + +pub const OPENAI_CODEX_VAULT_SECRET_NAME: &str = "OPENAI_CODEX"; diff --git a/lib/crates/fabro-auth/src/refresh.rs b/lib/crates/fabro-auth/src/refresh.rs index 3392957c5..52df04579 100644 --- a/lib/crates/fabro-auth/src/refresh.rs +++ b/lib/crates/fabro-auth/src/refresh.rs @@ -1,42 +1,31 @@ -use crate::credential::{AuthCredential, AuthDetails, OAuthTokens, expires_at_from_now}; +use crate::credential::{OAuthCredential, OAuthTokens, expires_at_from_now}; pub async fn refresh_oauth_credential( - credential: &AuthCredential, -) -> anyhow::Result { - match &credential.details { - AuthDetails::ApiKey { .. } => Ok(credential.clone()), - AuthDetails::CodexOAuth { - tokens, - config, - account_id, - } => { - let refresh_token = tokens + credential: &OAuthCredential, +) -> anyhow::Result { + let refresh_token = credential + .tokens + .refresh_token + .as_deref() + .ok_or_else(|| anyhow::anyhow!("refresh token missing"))?; + let response = fabro_oauth::refresh_token( + fabro_oauth::OAuthEndpoint { + token_url: &credential.config.token_url, + client_id: &credential.config.client_id, + }, + refresh_token, + ) + .await + .map_err(anyhow::Error::msg)?; + Ok(OAuthCredential { + tokens: OAuthTokens { + access_token: response.access_token, + refresh_token: response .refresh_token - .as_deref() - .ok_or_else(|| anyhow::anyhow!("refresh token missing"))?; - let response = fabro_oauth::refresh_token( - fabro_oauth::OAuthEndpoint { - token_url: &config.token_url, - client_id: &config.client_id, - }, - refresh_token, - ) - .await - .map_err(anyhow::Error::msg)?; - Ok(AuthCredential { - provider: credential.provider.clone(), - details: AuthDetails::CodexOAuth { - tokens: OAuthTokens { - access_token: response.access_token, - refresh_token: response - .refresh_token - .or_else(|| tokens.refresh_token.clone()), - expires_at: expires_at_from_now(response.expires_in), - }, - config: config.clone(), - account_id: account_id.clone(), - }, - }) - } - } + .or_else(|| credential.tokens.refresh_token.clone()), + expires_at: expires_at_from_now(response.expires_in), + }, + config: credential.config.clone(), + account_id: credential.account_id.clone(), + }) } diff --git a/lib/crates/fabro-auth/src/resolve.rs b/lib/crates/fabro-auth/src/resolve.rs index d72a13a9d..3e08045f2 100644 --- a/lib/crates/fabro-auth/src/resolve.rs +++ b/lib/crates/fabro-auth/src/resolve.rs @@ -4,16 +4,16 @@ use std::sync::Arc; use fabro_model::catalog::CatalogProvider; use fabro_model::{ApiKeyHeaderPolicy, Catalog, CredentialRef, HeaderValueRef, ProviderId}; use fabro_static::EnvVars; -use fabro_vault::Vault; +use fabro_vault::{SecretType, Vault}; use shlex::try_quote; use tokio::sync::RwLock as AsyncRwLock; use tokio::task::spawn_blocking; -use crate::credential::{ApiKeyHeader, AuthCredential, AuthDetails, credential_id_for}; +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::{vault_get_credential, vault_set_credential}; +use crate::vault_ext::{VaultLookupError, vault_get_oauth, vault_get_token, vault_set_oauth}; pub type EnvLookup = Arc Option + Send + Sync>; @@ -30,6 +30,15 @@ pub enum CredentialUsage { CliAgent(CliAgentKind), } +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum ResolvedSecret { + ApiKey(String), + OAuth { + credential: Box, + vault_name: String, + }, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct ApiCredential { pub provider: ProviderId, @@ -66,6 +75,38 @@ impl ApiCredential { } } +const OPENAI_CODEX_BASE_URL: &str = "https://chatgpt.com/backend-api/codex"; +const CHATGPT_ACCOUNT_ID_HEADER: &str = "ChatGPT-Account-Id"; +const ORIGINATOR_HEADER: &str = "originator"; +const FABRO_ORIGINATOR: &str = "fabro"; + +pub(crate) fn apply_openai_api_env_context( + credential: &mut ApiCredential, + env_lookup: &(dyn Fn(&str) -> Option + Send + Sync), +) { + credential.org_id = env_lookup(EnvVars::OPENAI_ORG_ID); + credential.project_id = env_lookup(EnvVars::OPENAI_PROJECT_ID); +} + +pub(crate) fn apply_openai_codex_api_context( + credential: &mut ApiCredential, + account_id: Option<&str>, + env_lookup: &(dyn Fn(&str) -> Option + Send + Sync), +) { + apply_openai_api_env_context(credential, env_lookup); + if let Some(account_id) = account_id { + credential.extra_headers.insert( + CHATGPT_ACCOUNT_ID_HEADER.to_string(), + account_id.to_string(), + ); + } + credential + .extra_headers + .insert(ORIGINATOR_HEADER.to_string(), FABRO_ORIGINATOR.to_string()); + credential.base_url = Some(OPENAI_CODEX_BASE_URL.to_string()); + credential.codex_mode = true; +} + #[must_use] pub fn build_api_key_header(policy: ApiKeyHeaderPolicy, key: String) -> ApiKeyHeader { match policy { @@ -100,6 +141,19 @@ pub enum ResolvedCredential { pub enum ResolveError { #[error("{0} is not configured")] NotConfigured(ProviderId), + #[error("{provider} vault credential '{name}' has schema {actual:?}, expected Token or Oauth")] + VaultSchemaMismatch { + provider: ProviderId, + name: String, + actual: SecretType, + }, + #[error("{provider} vault credential '{name}' is not valid Oauth JSON: {source}")] + VaultDecodeFailed { + provider: ProviderId, + name: String, + #[source] + source: serde_json::Error, + }, #[error("{provider} requires re-authentication: {source}")] RefreshFailed { provider: ProviderId, @@ -117,6 +171,14 @@ pub fn auth_issue_message(provider: &ProviderId, err: &ResolveError) -> String { ResolveError::NotConfigured(_) => { format!("{provider_name} is not configured") } + ResolveError::VaultSchemaMismatch { name, actual, .. } => { + format!( + "{provider_name} vault credential '{name}' has schema {actual:?}, expected Token or Oauth" + ) + } + ResolveError::VaultDecodeFailed { name, source, .. } => { + format!("{provider_name} vault credential '{name}' is not valid OAuth JSON: {source}") + } ResolveError::RefreshFailed { source, .. } => { format!("{provider_name} requires re-authentication: {source}") } @@ -163,59 +225,61 @@ impl CredentialResolver { .api_credential_from_provider_auth(&vault, catalog_provider, catalog) .map(ResolvedCredential::Api); } - let initial_credential = { + let initial_secret = { let vault = self.vault.read().await; - self.find_credential(&vault, catalog_provider, usage)? + self.find_credential(&vault, catalog_provider)? }; - let credential = if initial_credential.needs_refresh() { - let AuthDetails::CodexOAuth { tokens, .. } = &initial_credential.details else { - unreachable!("only OAuth credentials can need refresh"); - }; - if tokens.refresh_token.is_none() { + let secret = if let ResolvedSecret::OAuth { + credential, + vault_name, + } = &initial_secret + { + if !credential.needs_refresh() { + initial_secret + } else if credential.tokens.refresh_token.is_none() { return Err(ResolveError::RefreshTokenMissing(provider_id.clone())); - } - - let refreshed = refresh_oauth_credential(&initial_credential) + } else { + let refreshed = refresh_oauth_credential(credential) + .await + .map_err(|source| ResolveError::RefreshFailed { + provider: provider_id.clone(), + source, + })?; + let refreshed_for_store = refreshed.clone(); + let vault_name_for_store = vault_name.clone(); + let vault = Arc::clone(&self.vault); + spawn_blocking(move || { + let mut vault = vault.blocking_write(); + vault_set_oauth(&mut vault, &vault_name_for_store, &refreshed_for_store) + .map(|_| ()) + .map_err(anyhow::Error::from) + }) .await + .map_err(|join_err| ResolveError::RefreshFailed { + provider: provider_id.clone(), + source: anyhow::Error::from(join_err), + })? .map_err(|source| ResolveError::RefreshFailed { provider: provider_id.clone(), source, })?; - let credential_id = - credential_id_for(&refreshed).map_err(|message| ResolveError::RefreshFailed { - provider: provider_id.clone(), - source: anyhow::anyhow!(message), - })?; - let refreshed_for_store = refreshed.clone(); - let vault = Arc::clone(&self.vault); - spawn_blocking(move || { - let mut vault = vault.blocking_write(); - vault_set_credential(&mut vault, &credential_id, &refreshed_for_store) - .map(|_| ()) - .map_err(anyhow::Error::from) - }) - .await - .map_err(|join_err| ResolveError::RefreshFailed { - provider: provider_id.clone(), - source: anyhow::Error::from(join_err), - })? - .map_err(|source| ResolveError::RefreshFailed { - provider: provider_id.clone(), - source, - })?; - refreshed + ResolvedSecret::OAuth { + credential: Box::new(refreshed), + vault_name: vault_name.clone(), + } + } } else { - initial_credential + initial_secret }; let vault = self.vault.read().await; match usage { CredentialUsage::ApiRequest => self - .to_api_credential(&vault, &credential, catalog) + .to_api_credential(&vault, &provider_id, &secret, catalog) .map(ResolvedCredential::Api), CredentialUsage::CliAgent(kind) => Ok(ResolvedCredential::Cli( - Self::to_cli_credential(&credential, kind, catalog), + Self::to_cli_credential(&provider_id, &secret, kind, catalog), )), } } @@ -234,24 +298,14 @@ impl CredentialResolver { &self, vault: &Vault, provider: &CatalogProvider, - usage: CredentialUsage, - ) -> Result { - if provider.id == ProviderId::openai() - && usage == CredentialUsage::CliAgent(CliAgentKind::Codex) - { - for credential_id in ["openai_codex", "openai"] { - if let Some(credential) = vault_get_credential(vault, credential_id) { - return Ok(credential); - } - } - } - + ) -> Result { let Some(auth) = &provider.auth else { return Err(ResolveError::NotConfigured(provider.id.clone())); }; for credential_ref in &auth.credentials { - if let Some(credential) = self.credential_from_ref(vault, &provider.id, credential_ref) + if let Some(credential) = + self.credential_from_ref(vault, &provider.id, credential_ref)? { return Ok(credential); } @@ -273,7 +327,7 @@ impl CredentialResolver { }; auth.credentials.iter().any(|credential_ref| { self.credential_from_ref(vault, &provider.id, credential_ref) - .is_some() + .is_ok_and(|credential| credential.is_some()) }) } @@ -282,21 +336,30 @@ impl CredentialResolver { vault: &Vault, provider: &ProviderId, credential_ref: &CredentialRef, - ) -> Option { + ) -> Result, ResolveError> { match credential_ref { - CredentialRef::Credential(id) => vault_get_credential(vault, id), - CredentialRef::Env(name) => { - self.lookup_env_or_vault(vault, name) - .map(|key| AuthCredential { - provider: provider.clone(), - details: AuthDetails::ApiKey { key }, + CredentialRef::Vault(name) => match vault_get_token(vault, name) { + Ok(Some(token)) => Ok(Some(ResolvedSecret::ApiKey(token))), + Ok(None) => Ok(None), + Err(VaultLookupError::SchemaMismatch { + actual: SecretType::Oauth, + .. + }) => vault_get_oauth(vault, name) + .map(|credential| { + credential.map(|credential| ResolvedSecret::OAuth { + credential: Box::new(credential), + vault_name: name.clone(), + }) }) - } + .map_err(|err| vault_lookup_error(provider, name, err)), + Err(err) => Err(vault_lookup_error(provider, name, err)), + }, + CredentialRef::Env(name) => Ok((self.env_lookup)(name).map(ResolvedSecret::ApiKey)), } } - fn lookup_env_or_vault(&self, vault: &Vault, name: &str) -> Option { - (self.env_lookup)(name).or_else(|| vault.get(name).map(str::to_string)) + fn lookup_env(&self, name: &str) -> Option { + (self.env_lookup)(name) } fn provider_base_url_for_catalog(provider: &ProviderId, catalog: &Catalog) -> Option { @@ -320,8 +383,8 @@ impl CredentialResolver { .map(|(name, value_ref)| { let value = match value_ref { HeaderValueRef::Literal(value) => Some(value.clone()), - HeaderValueRef::Env(name) => self.lookup_env_or_vault(vault, name), - HeaderValueRef::Credential(name) => vault.get(name).map(str::to_string), + HeaderValueRef::Env(name) => self.lookup_env(name), + HeaderValueRef::Vault(name) => vault.get(name).map(str::to_string), } .ok_or_else(|| ResolveError::NotConfigured(provider.clone()))?; Ok((name.clone(), value)) @@ -332,18 +395,19 @@ impl CredentialResolver { fn to_api_credential( &self, vault: &Vault, - credential: &AuthCredential, + provider_id: &ProviderId, + secret: &ResolvedSecret, catalog: &Catalog, ) -> Result { - let base_url = Self::provider_base_url_for_catalog(&credential.provider, catalog); - match &credential.details { - AuthDetails::ApiKey { key } => { + let base_url = Self::provider_base_url_for_catalog(provider_id, catalog); + match secret { + ResolvedSecret::ApiKey(key) => { let provider = catalog - .provider(&credential.provider) - .ok_or_else(|| ResolveError::NotConfigured(credential.provider.clone()))?; + .provider(provider_id) + .ok_or_else(|| ResolveError::NotConfigured(provider_id.clone()))?; let auth_header = auth_header_for_catalog_provider(provider, key.clone())?; let mut cred = ApiCredential { - provider: credential.provider.clone(), + provider: provider_id.clone(), auth_header: Some(auth_header), extra_headers: HashMap::new(), base_url: None, @@ -353,30 +417,32 @@ impl CredentialResolver { }; cred.base_url = base_url; cred.extra_headers = - self.resolved_extra_headers_for_catalog(vault, &credential.provider, catalog)?; - if credential.provider == ProviderId::openai() { - cred.org_id = self.lookup_env_or_vault(vault, EnvVars::OPENAI_ORG_ID); - cred.project_id = self.lookup_env_or_vault(vault, EnvVars::OPENAI_PROJECT_ID); + self.resolved_extra_headers_for_catalog(vault, provider_id, catalog)?; + if provider_id == &ProviderId::openai() { + apply_openai_api_env_context(&mut cred, &*self.env_lookup); } Ok(cred) } - AuthDetails::CodexOAuth { - tokens, account_id, .. - } => { - let mut extra_headers = HashMap::new(); - if let Some(account_id) = account_id { - extra_headers.insert("ChatGPT-Account-Id".to_string(), account_id.clone()); - extra_headers.insert("originator".to_string(), "fabro".to_string()); + ResolvedSecret::OAuth { credential, .. } => { + let mut extra_headers = + self.resolved_extra_headers_for_catalog(vault, provider_id, catalog)?; + let mut api_credential = ApiCredential { + provider: provider_id.clone(), + auth_header: Some(ApiKeyHeader::Bearer(credential.tokens.access_token.clone())), + extra_headers: std::mem::take(&mut extra_headers), + base_url, + codex_mode: false, + org_id: None, + project_id: None, + }; + if provider_id == &ProviderId::openai() { + apply_openai_codex_api_context( + &mut api_credential, + credential.account_id.as_deref(), + &*self.env_lookup, + ); } - Ok(ApiCredential { - provider: credential.provider.clone(), - auth_header: Some(ApiKeyHeader::Bearer(tokens.access_token.clone())), - extra_headers, - base_url: Some("https://chatgpt.com/backend-api/codex".to_string()), - codex_mode: true, - org_id: self.lookup_env_or_vault(vault, EnvVars::OPENAI_ORG_ID), - project_id: self.lookup_env_or_vault(vault, EnvVars::OPENAI_PROJECT_ID), - }) + Ok(api_credential) } } } @@ -404,44 +470,38 @@ impl CredentialResolver { } fn to_cli_credential( - credential: &AuthCredential, + provider_id: &ProviderId, + secret: &ResolvedSecret, kind: CliAgentKind, catalog: &Catalog, ) -> CliCredential { let mut env_vars = HashMap::new(); - let is_openai = credential.provider == ProviderId::openai(); - let login_command = match (is_openai, &credential.details, kind) { - (true, AuthDetails::ApiKey { key }, CliAgentKind::Codex) => { + let is_openai = provider_id == &ProviderId::openai(); + let login_command = match (is_openai, secret, kind) { + (true, ResolvedSecret::ApiKey(key), CliAgentKind::Codex) => { env_vars.insert(EnvVars::OPENAI_API_KEY.to_string(), key.clone()); Some(codex_login_command(key)) } - ( - true, - AuthDetails::CodexOAuth { - tokens, account_id, .. - }, - CliAgentKind::Codex, - ) => { + (true, ResolvedSecret::OAuth { credential, .. }, CliAgentKind::Codex) => { env_vars.insert( EnvVars::OPENAI_API_KEY.to_string(), - tokens.access_token.clone(), + credential.tokens.access_token.clone(), ); - if let Some(account_id) = account_id { + if let Some(account_id) = &credential.account_id { env_vars.insert(EnvVars::CHATGPT_ACCOUNT_ID.to_string(), account_id.clone()); } - Some(codex_login_command(&tokens.access_token)) + Some(codex_login_command(&credential.tokens.access_token)) } - (_, AuthDetails::ApiKey { key }, _) => { - if let Some(name) = primary_api_key_env_var(&credential.provider, catalog) { + (_, ResolvedSecret::ApiKey(key), _) => { + if let Some(name) = primary_api_key_env_var(provider_id, catalog) { env_vars.insert(name.to_string(), key.clone()); } None } - (_, AuthDetails::CodexOAuth { tokens, .. }, _) => { - env_vars.insert( - EnvVars::OPENAI_API_KEY.to_string(), - tokens.access_token.clone(), - ); + (_, ResolvedSecret::OAuth { credential, .. }, _) => { + if let Some(name) = primary_api_key_env_var(provider_id, catalog) { + env_vars.insert(name.to_string(), credential.tokens.access_token.clone()); + } None } }; @@ -453,6 +513,21 @@ impl CredentialResolver { } } +fn vault_lookup_error(provider: &ProviderId, name: &str, err: VaultLookupError) -> ResolveError { + match err { + VaultLookupError::SchemaMismatch { actual, .. } => ResolveError::VaultSchemaMismatch { + provider: provider.clone(), + name: name.to_string(), + actual, + }, + VaultLookupError::DecodeFailed { source, .. } => ResolveError::VaultDecodeFailed { + provider: provider.clone(), + name: name.to_string(), + source, + }, + } +} + pub async fn configured_providers_from_process_env( vault: Option<&Arc>>, catalog: &Catalog, @@ -470,7 +545,6 @@ pub async fn configured_providers_from_process_env( } } } - fn primary_api_key_env_var<'a>(provider: &ProviderId, catalog: &'a Catalog) -> Option<&'a str> { catalog .provider(provider)? @@ -480,7 +554,7 @@ fn primary_api_key_env_var<'a>(provider: &ProviderId, catalog: &'a Catalog) -> O .iter() .find_map(|credential_ref| match credential_ref { CredentialRef::Env(name) => Some(name.as_str()), - CredentialRef::Credential(_) => None, + CredentialRef::Vault(_) => None, }) } @@ -503,37 +577,25 @@ mod tests { use httpmock::MockServer; use super::*; - use crate::credential::{OAuthConfig, OAuthTokens}; - use crate::vault_ext::vault_get_credential; + use crate::credential::{OAuthConfig, OAuthCredential, OAuthTokens}; + use crate::vault_ext::{vault_get_oauth, vault_set_oauth, vault_set_token}; - fn api_key_credential(provider: ProviderId, key: &str) -> AuthCredential { - AuthCredential { - provider, - details: AuthDetails::ApiKey { - key: key.to_string(), + fn oauth_credential(token_url: String, expires_at: chrono::DateTime) -> OAuthCredential { + OAuthCredential { + tokens: OAuthTokens { + access_token: "expired-access".to_string(), + refresh_token: Some("refresh-token".to_string()), + expires_at, }, - } - } - - fn oauth_credential(token_url: String, expires_at: chrono::DateTime) -> AuthCredential { - AuthCredential { - provider: ProviderId::openai(), - details: AuthDetails::CodexOAuth { - tokens: OAuthTokens { - access_token: "expired-access".to_string(), - refresh_token: Some("refresh-token".to_string()), - expires_at, - }, - config: OAuthConfig { - auth_url: "https://auth.openai.com".to_string(), - token_url, - client_id: "test-client".to_string(), - scopes: vec!["openid".to_string()], - redirect_uri: Some("https://auth.openai.com/deviceauth/callback".to_string()), - use_pkce: true, - }, - account_id: Some("acct_123".to_string()), + config: OAuthConfig { + auth_url: "https://auth.openai.com".to_string(), + token_url, + client_id: "test-client".to_string(), + scopes: vec!["openid".to_string()], + redirect_uri: Some("https://auth.openai.com/deviceauth/callback".to_string()), + use_pkce: true, }, + account_id: Some("acct_123".to_string()), } } @@ -551,16 +613,14 @@ mod tests { } #[tokio::test] - async fn resolve_openai_api_request_prefers_typed_credential() { + async fn resolve_openai_api_request_prefers_env_when_listed_first() { let dir = tempfile::tempdir().unwrap(); let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); - vault_set_credential( - &mut vault, - "openai", - &api_key_credential(ProviderId::openai(), "vault-key"), - ) - .unwrap(); - let resolver = test_resolver(vault, Arc::new(|_| Some("env-key".to_string()))); + vault_set_token(&mut vault, "OPENAI_API_KEY", "vault-key").unwrap(); + let resolver = test_resolver( + vault, + Arc::new(|name| (name == "OPENAI_API_KEY").then(|| "env-key".to_string())), + ); let catalog = default_catalog(); let resolved = resolver @@ -573,7 +633,7 @@ mod tests { }; assert_eq!( api.auth_header, - Some(ApiKeyHeader::Bearer("vault-key".to_string())) + Some(ApiKeyHeader::Bearer("env-key".to_string())) ); } @@ -581,9 +641,9 @@ mod tests { async fn resolve_openai_api_request_falls_back_to_codex_oauth_credential() { let dir = tempfile::tempdir().unwrap(); let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); - vault_set_credential( + vault_set_oauth( &mut vault, - "openai_codex", + crate::OPENAI_CODEX_VAULT_SECRET_NAME, &oauth_credential( "https://auth.openai.com/oauth/token".to_string(), Utc::now() + Duration::hours(1), @@ -638,12 +698,7 @@ mod tests { async fn anthropic_api_credentials_use_x_api_key_header() { let dir = tempfile::tempdir().unwrap(); let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); - vault_set_credential( - &mut vault, - "anthropic", - &api_key_credential(ProviderId::anthropic(), "anthropic-key"), - ) - .unwrap(); + vault_set_token(&mut vault, "ANTHROPIC_API_KEY", "anthropic-key").unwrap(); let resolver = test_resolver(vault, Arc::new(|_| None)); let catalog = default_catalog(); @@ -679,7 +734,7 @@ agent_profile = "openai" base_url = "https://default.example.com/v1" [providers.acme.auth] -credentials = ["credential:acme"] +credentials = ["vault:acme"] [models."compat-model"] provider = "acme" @@ -698,12 +753,7 @@ reasoning = false ); let dir = tempfile::tempdir().unwrap(); let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); - vault_set_credential( - &mut vault, - "acme", - &api_key_credential(ProviderId::new("acme"), "compat-key"), - ) - .unwrap(); + vault_set_token(&mut vault, "acme", "compat-key").unwrap(); let resolver = test_resolver(vault, Arc::new(|_| None)); let resolved = resolver .resolve( @@ -731,9 +781,9 @@ reasoning = false async fn openai_codex_cli_credential_includes_login_command_and_account_id() { let dir = tempfile::tempdir().unwrap(); let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); - vault_set_credential( + vault_set_oauth( &mut vault, - "openai_codex", + crate::OPENAI_CODEX_VAULT_SECRET_NAME, &oauth_credential( "https://auth.openai.com/oauth/token".to_string(), Utc::now() + Duration::hours(1), @@ -774,12 +824,7 @@ reasoning = false async fn openai_api_key_cli_fallback_has_no_account_id() { let dir = tempfile::tempdir().unwrap(); let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); - vault_set_credential( - &mut vault, - "openai", - &api_key_credential(ProviderId::openai(), "openai-key"), - ) - .unwrap(); + vault_set_token(&mut vault, "OPENAI_API_KEY", "openai-key").unwrap(); let resolver = test_resolver(vault, Arc::new(|_| None)); let catalog = default_catalog(); @@ -826,12 +871,7 @@ reasoning = false std::fs::set_permissions(&codex_path, permissions).unwrap(); let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); - vault_set_credential( - &mut vault, - "openai", - &api_key_credential(ProviderId::openai(), "openai-key"), - ) - .unwrap(); + vault_set_token(&mut vault, "OPENAI_API_KEY", "openai-key").unwrap(); let resolver = test_resolver(vault, Arc::new(|_| None)); let catalog = default_catalog(); @@ -876,23 +916,22 @@ reasoning = false async fn with_env_lookup_overrides_vault_settings() { let dir = tempfile::tempdir().unwrap(); let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); - vault_set_credential( - &mut vault, - "openai", - &api_key_credential(ProviderId::openai(), "vault-key"), - ) - .unwrap(); + vault_set_token(&mut vault, "OPENAI_API_KEY", "vault-key").unwrap(); vault .set( "OPENAI_ORG_ID", "vault-org", - fabro_vault::SecretType::Environment, + fabro_vault::SecretType::Token, None, ) .unwrap(); let resolver = test_resolver( vault, - Arc::new(|name| (name == "OPENAI_ORG_ID").then(|| "env-org".to_string())), + Arc::new(|name| match name { + "OPENAI_API_KEY" => Some("env-key".to_string()), + "OPENAI_ORG_ID" => Some("env-org".to_string()), + _ => None, + }), ); let catalog = default_catalog(); @@ -911,12 +950,7 @@ reasoning = false async fn configured_providers_returns_vault_backed_provider() { let dir = tempfile::tempdir().unwrap(); let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); - vault_set_credential( - &mut vault, - "openai", - &api_key_credential(ProviderId::openai(), "vault-key"), - ) - .unwrap(); + vault_set_token(&mut vault, "OPENAI_API_KEY", "vault-key").unwrap(); let resolver = test_resolver(vault, Arc::new(|_| None)); let vault = resolver.vault.read().await; let catalog = default_catalog(); @@ -937,7 +971,7 @@ agent_profile = "openai" base_url = "https://api.acme.test/v1" [providers.acme.auth] -credentials = ["credential:acme"] +credentials = ["vault:acme"] [models."acme-large"] provider = "acme" @@ -956,13 +990,7 @@ reasoning = false ); let dir = tempfile::tempdir().unwrap(); let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); - vault_set_credential(&mut vault, "acme", &AuthCredential { - provider: ProviderId::new("acme"), - details: AuthDetails::ApiKey { - key: "acme-key".to_string(), - }, - }) - .unwrap(); + vault_set_token(&mut vault, "acme", "acme-key").unwrap(); let resolver = test_resolver(vault, Arc::new(|_| None)); let resolved = resolver @@ -1027,9 +1055,9 @@ reasoning = false let dir = tempfile::tempdir().unwrap(); let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); - vault_set_credential( + vault_set_oauth( &mut vault, - "openai_codex", + crate::OPENAI_CODEX_VAULT_SECRET_NAME, &oauth_credential( server.url("/oauth/token"), Utc::now() - Duration::minutes(1), @@ -1059,17 +1087,13 @@ reasoning = false let stored = { let vault = vault.read().await; - vault_get_credential(&vault, "openai_codex").unwrap() + vault_get_oauth(&vault, crate::OPENAI_CODEX_VAULT_SECRET_NAME) + .unwrap() + .unwrap() }; - let AuthDetails::CodexOAuth { - tokens, account_id, .. - } = stored.details - else { - panic!("expected codex oauth credential"); - }; - assert_eq!(tokens.access_token, "new-access"); - assert_eq!(tokens.refresh_token.as_deref(), Some("new-refresh")); - assert_eq!(account_id.as_deref(), Some("acct_123")); + assert_eq!(stored.tokens.access_token, "new-access"); + assert_eq!(stored.tokens.refresh_token.as_deref(), Some("new-refresh")); + assert_eq!(stored.account_id.as_deref(), Some("acct_123")); refresh_mock.assert_async().await; } @@ -1081,11 +1105,13 @@ reasoning = false "https://auth.openai.com/oauth/token".to_string(), Utc::now() - Duration::minutes(1), ); - let AuthDetails::CodexOAuth { tokens, .. } = &mut credential.details else { - unreachable!(); - }; - tokens.refresh_token = None; - vault_set_credential(&mut vault, "openai_codex", &credential).unwrap(); + credential.tokens.refresh_token = None; + vault_set_oauth( + &mut vault, + crate::OPENAI_CODEX_VAULT_SECRET_NAME, + &credential, + ) + .unwrap(); let resolver = test_resolver(vault, Arc::new(|_| None)); let catalog = default_catalog(); diff --git a/lib/crates/fabro-auth/src/strategies/api_key.rs b/lib/crates/fabro-auth/src/strategies/api_key.rs index 97815a668..6a815d1c6 100644 --- a/lib/crates/fabro-auth/src/strategies/api_key.rs +++ b/lib/crates/fabro-auth/src/strategies/api_key.rs @@ -3,8 +3,7 @@ use fabro_model::catalog::CatalogProvider; use fabro_model::{CredentialRef, ProviderId}; use crate::context::{AuthContextRequest, AuthContextResponse}; -use crate::credential::{AuthCredential, AuthDetails}; -use crate::strategy::AuthStrategy; +use crate::strategy::{AuthStrategy, LoginResult}; pub struct ApiKeyStrategy { provider_id: ProviderId, @@ -24,7 +23,7 @@ impl ApiKeyStrategy { .iter() .filter_map(|credential_ref| match credential_ref { CredentialRef::Env(name) => Some(name.clone()), - CredentialRef::Credential(_) => None, + CredentialRef::Vault(_) => None, }) .collect() }) @@ -49,11 +48,11 @@ impl AuthStrategy for ApiKeyStrategy { }) } - async fn complete(&mut self, response: AuthContextResponse) -> anyhow::Result { + async fn complete(&mut self, response: AuthContextResponse) -> anyhow::Result { match response { - AuthContextResponse::ApiKey { key } => Ok(AuthCredential { + AuthContextResponse::ApiKey { key } => Ok(LoginResult::ApiKey { provider: self.provider_id.clone(), - details: AuthDetails::ApiKey { key }, + key, }), AuthContextResponse::DeviceCodeConfirmed => { Err(anyhow::anyhow!("expected API key response")) diff --git a/lib/crates/fabro-auth/src/strategies/codex_device.rs b/lib/crates/fabro-auth/src/strategies/codex_device.rs index 9b6e9117b..94a3399c8 100644 --- a/lib/crates/fabro-auth/src/strategies/codex_device.rs +++ b/lib/crates/fabro-auth/src/strategies/codex_device.rs @@ -10,10 +10,8 @@ use serde_json::json; use tokio::time::sleep; use crate::context::{AuthContextRequest, AuthContextResponse}; -use crate::credential::{ - AuthCredential, AuthDetails, OAuthConfig, OAuthTokens, expires_at_from_now, -}; -use crate::strategy::AuthStrategy; +use crate::credential::{OAuthConfig, OAuthCredential, OAuthTokens, expires_at_from_now}; +use crate::strategy::{AuthStrategy, LoginResult}; const DEVICE_AUTH_POLL_INTERVAL: Duration = Duration::from_secs(2); const CODEX_DEVICE_VERIFICATION_URI: &str = "https://auth.openai.com/codex/device"; @@ -276,7 +274,7 @@ impl AuthStrategy for CodexDeviceStrategy { }) } - async fn complete(&mut self, response: AuthContextResponse) -> anyhow::Result { + async fn complete(&mut self, response: AuthContextResponse) -> anyhow::Result { match response { AuthContextResponse::ApiKey { .. } => Err(anyhow::anyhow!( "expected device code confirmation response" @@ -299,9 +297,9 @@ impl AuthStrategy for CodexDeviceStrategy { .await .map_err(anyhow::Error::msg)?; - Ok(AuthCredential { - provider: fabro_model::ProviderId::openai(), - details: AuthDetails::CodexOAuth { + Ok(LoginResult::OAuth { + provider: fabro_model::ProviderId::openai(), + credential: OAuthCredential { tokens: OAuthTokens { access_token: token_response.access_token, refresh_token: token_response.refresh_token, @@ -536,14 +534,14 @@ mod tests { assert!(pending_poll_mock.calls_async().await > 0); pending_poll_mock.delete_async().await; - let credential = complete.await.unwrap().unwrap(); + let result = complete.await.unwrap().unwrap(); - let AuthDetails::CodexOAuth { - tokens, account_id, .. - } = credential.details - else { + let LoginResult::OAuth { credential, .. } = result else { panic!("expected codex oauth credential"); }; + let OAuthCredential { + tokens, account_id, .. + } = credential; assert_eq!(tokens.access_token, "new-access-token"); assert_eq!(tokens.refresh_token.as_deref(), Some("new-refresh-token")); assert_eq!(account_id.as_deref(), Some("acct_123")); diff --git a/lib/crates/fabro-auth/src/strategy.rs b/lib/crates/fabro-auth/src/strategy.rs index b29d7aa35..b4e0ab058 100644 --- a/lib/crates/fabro-auth/src/strategy.rs +++ b/lib/crates/fabro-auth/src/strategy.rs @@ -2,7 +2,7 @@ use async_trait::async_trait; use fabro_model::{Catalog, ProviderId}; use crate::context::{AuthContextRequest, AuthContextResponse}; -use crate::credential::{AuthCredential, OAuthConfig}; +use crate::credential::{OAuthConfig, OAuthCredential}; use crate::strategies::api_key::ApiKeyStrategy; use crate::strategies::codex_device::CodexDeviceStrategy; @@ -10,10 +10,22 @@ pub const CODEX_CLIENT_ID: &str = "app_EMoamEEZ73f0CkXaXp7hrann"; pub const CODEX_AUTH_URL: &str = "https://auth.openai.com"; pub const CODEX_TOKEN_URL: &str = "https://auth.openai.com/oauth/token"; +#[derive(Debug, Clone)] +pub enum LoginResult { + ApiKey { + provider: ProviderId, + key: String, + }, + OAuth { + provider: ProviderId, + credential: OAuthCredential, + }, +} + #[async_trait] pub trait AuthStrategy: Send { async fn init(&mut self) -> anyhow::Result; - async fn complete(&mut self, response: AuthContextResponse) -> anyhow::Result; + async fn complete(&mut self, response: AuthContextResponse) -> anyhow::Result; } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/lib/crates/fabro-auth/src/vault_ext.rs b/lib/crates/fabro-auth/src/vault_ext.rs index 53dba827c..b08711e9e 100644 --- a/lib/crates/fabro-auth/src/vault_ext.rs +++ b/lib/crates/fabro-auth/src/vault_ext.rs @@ -1,105 +1,160 @@ -use fabro_model::ProviderId; use fabro_types::SecretMetadata; -use fabro_vault::{SecretType, Vault}; +use fabro_vault::{Error as VaultError, SecretType, Vault}; -use crate::credential::AuthCredential; +use crate::credential::OAuthCredential; -pub fn vault_set_credential( - vault: &mut Vault, - id: &str, - credential: &AuthCredential, -) -> Result { - let json = serde_json::to_string(credential)?; - vault.set(id, &json, SecretType::Credential, None) +#[derive(Debug, thiserror::Error)] +pub enum VaultLookupError { + #[error("vault entry '{name}' has schema {actual:?}, expected {expected:?}")] + SchemaMismatch { + name: String, + expected: SecretType, + actual: SecretType, + }, + #[error("vault entry '{name}' is not valid {expected:?} JSON: {source}")] + DecodeFailed { + name: String, + expected: SecretType, + #[source] + source: serde_json::Error, + }, } -#[must_use] -pub fn vault_get_credential(vault: &Vault, id: &str) -> Option { - let entry = vault.get_entry(id)?; - if entry.secret_type != SecretType::Credential { - return None; +pub fn vault_get_token(vault: &Vault, name: &str) -> Result, VaultLookupError> { + let Some(entry) = vault.get_entry(name) else { + return Ok(None); + }; + if entry.secret_type != SecretType::Token { + return Err(VaultLookupError::SchemaMismatch { + name: name.to_string(), + expected: SecretType::Token, + actual: entry.secret_type, + }); } - serde_json::from_str(&entry.value).ok() + Ok(Some(entry.value.clone())) } -#[must_use] -pub fn vault_credentials_for_provider( +pub fn vault_get_oauth( vault: &Vault, - provider: impl Into, -) -> Vec<(String, AuthCredential)> { - let provider = provider.into(); - vault - .credential_entries() - .into_iter() - .filter_map(|(name, entry)| { - serde_json::from_str::(&entry.value) - .ok() - .filter(|credential| credential.provider == provider) - .map(|credential| (name.to_string(), credential)) + name: &str, +) -> Result, VaultLookupError> { + let Some(entry) = vault.get_entry(name) else { + return Ok(None); + }; + if entry.secret_type != SecretType::Oauth { + return Err(VaultLookupError::SchemaMismatch { + name: name.to_string(), + expected: SecretType::Oauth, + actual: entry.secret_type, + }); + } + serde_json::from_str(&entry.value) + .map(Some) + .map_err(|source| VaultLookupError::DecodeFailed { + name: name.to_string(), + expected: SecretType::Oauth, + source, }) - .collect() +} + +pub fn vault_set_token( + vault: &mut Vault, + name: &str, + value: &str, +) -> Result { + vault.set(name, value, SecretType::Token, None) +} + +pub fn vault_set_oauth( + vault: &mut Vault, + name: &str, + credential: &OAuthCredential, +) -> Result { + let json = serde_json::to_string(credential)?; + vault.set(name, &json, SecretType::Oauth, None) } #[cfg(test)] mod tests { use chrono::{Duration, Utc}; - use fabro_model::ProviderId; use super::*; - use crate::credential::{AuthDetails, OAuthConfig, OAuthTokens}; + use crate::credential::{OAuthConfig, OAuthTokens}; - fn oauth_credential() -> AuthCredential { - AuthCredential { - provider: ProviderId::openai(), - details: AuthDetails::CodexOAuth { - tokens: OAuthTokens { - access_token: "access".to_string(), - refresh_token: Some("refresh".to_string()), - expires_at: Utc::now() + Duration::hours(1), - }, - config: OAuthConfig { - auth_url: "https://auth.openai.com".to_string(), - token_url: "https://auth.openai.com/oauth/token".to_string(), - client_id: "client".to_string(), - scopes: vec!["openid".to_string()], - redirect_uri: Some("https://auth.openai.com/deviceauth/callback".to_string()), - use_pkce: true, - }, - account_id: Some("acct_123".to_string()), + fn temp_vault() -> Vault { + let dir = tempfile::tempdir().unwrap(); + Vault::load(dir.path().join("secrets.json")).unwrap() + } + + fn fixture() -> OAuthCredential { + OAuthCredential { + tokens: OAuthTokens { + access_token: "access".to_string(), + refresh_token: Some("refresh".to_string()), + expires_at: Utc::now() + Duration::hours(1), }, + config: OAuthConfig { + auth_url: "https://auth.openai.com".to_string(), + token_url: "https://auth.openai.com/oauth/token".to_string(), + client_id: "client".to_string(), + scopes: vec!["openid".to_string()], + redirect_uri: None, + use_pkce: true, + }, + account_id: None, } } #[test] - fn vault_credential_round_trip() { - let dir = tempfile::tempdir().unwrap(); - let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); - let credential = oauth_credential(); - - vault_set_credential(&mut vault, "openai_codex", &credential).unwrap(); - - assert_eq!( - vault_get_credential(&vault, "openai_codex").unwrap(), - credential + fn vault_get_token_returns_none_when_absent() { + let vault = temp_vault(); + assert!( + vault_get_token(&vault, "ANTHROPIC_API_KEY") + .unwrap() + .is_none() ); } #[test] - fn vault_credentials_for_provider_filters_by_provider() { - let dir = tempfile::tempdir().unwrap(); - let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); - vault_set_credential(&mut vault, "openai_codex", &oauth_credential()).unwrap(); - vault_set_credential(&mut vault, "anthropic", &AuthCredential { - provider: ProviderId::anthropic(), - details: AuthDetails::ApiKey { - key: "anthropic-key".to_string(), - }, - }) + fn vault_get_token_returns_value_when_present() { + let mut vault = temp_vault(); + vault_set_token(&mut vault, "ANTHROPIC_API_KEY", "sk-test").unwrap(); + assert_eq!( + vault_get_token(&vault, "ANTHROPIC_API_KEY") + .unwrap() + .as_deref(), + Some("sk-test"), + ); + } + + #[test] + fn vault_get_token_errors_on_oauth_entry() { + let mut vault = temp_vault(); + vault_set_oauth( + &mut vault, + crate::OPENAI_CODEX_VAULT_SECRET_NAME, + &fixture(), + ) .unwrap(); + let err = vault_get_token(&vault, crate::OPENAI_CODEX_VAULT_SECRET_NAME).unwrap_err(); + assert!(matches!(err, VaultLookupError::SchemaMismatch { .. })); + } - let credentials = vault_credentials_for_provider(&vault, ProviderId::openai()); - - assert_eq!(credentials.len(), 1); - assert_eq!(credentials[0].0, "openai_codex"); + #[test] + fn vault_get_oauth_round_trips() { + let mut vault = temp_vault(); + let credential = fixture(); + vault_set_oauth( + &mut vault, + crate::OPENAI_CODEX_VAULT_SECRET_NAME, + &credential, + ) + .unwrap(); + assert_eq!( + vault_get_oauth(&vault, crate::OPENAI_CODEX_VAULT_SECRET_NAME) + .unwrap() + .unwrap(), + credential, + ); } } diff --git a/lib/crates/fabro-auth/src/vault_source.rs b/lib/crates/fabro-auth/src/vault_source.rs index 5f11bf800..16329ed0a 100644 --- a/lib/crates/fabro-auth/src/vault_source.rs +++ b/lib/crates/fabro-auth/src/vault_source.rs @@ -76,41 +76,30 @@ mod tests { use chrono::{Duration, Utc}; use fabro_model::{Catalog, ProviderId}; - use fabro_vault::{SecretType, Vault}; + use fabro_vault::Vault; use tokio::sync::RwLock as AsyncRwLock; use super::VaultCredentialSource; - use crate::credential::{AuthCredential, AuthDetails, OAuthConfig, OAuthTokens}; + use crate::credential::{OAuthConfig, OAuthCredential, OAuthTokens}; + use crate::vault_ext::{vault_set_oauth, vault_set_token}; use crate::{CredentialSource, ResolveError}; - fn api_key_credential(provider: ProviderId, key: &str) -> AuthCredential { - AuthCredential { - provider, - details: AuthDetails::ApiKey { - key: key.to_string(), + fn expired_openai_credential() -> OAuthCredential { + OAuthCredential { + tokens: OAuthTokens { + access_token: "expired-access".to_string(), + refresh_token: Some("refresh-token".to_string()), + expires_at: Utc::now() - Duration::hours(1), }, - } - } - - fn expired_openai_credential() -> AuthCredential { - AuthCredential { - provider: ProviderId::openai(), - details: AuthDetails::CodexOAuth { - tokens: OAuthTokens { - access_token: "expired-access".to_string(), - refresh_token: Some("refresh-token".to_string()), - expires_at: Utc::now() - Duration::hours(1), - }, - config: OAuthConfig { - auth_url: "https://auth.openai.com".to_string(), - token_url: "http://127.0.0.1:9/oauth/token".to_string(), - client_id: "client".to_string(), - scopes: vec!["openid".to_string()], - redirect_uri: Some("https://example.com/callback".to_string()), - use_pkce: true, - }, - account_id: Some("acct_123".to_string()), + config: OAuthConfig { + auth_url: "https://auth.openai.com".to_string(), + token_url: "http://127.0.0.1:9/oauth/token".to_string(), + client_id: "client".to_string(), + scopes: vec!["openid".to_string()], + redirect_uri: Some("https://example.com/callback".to_string()), + use_pkce: true, }, + account_id: Some("acct_123".to_string()), } } @@ -122,26 +111,13 @@ mod tests { async fn resolve_returns_credentials_and_auth_issues() { let dir = tempfile::tempdir().unwrap(); let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); - vault - .set( - "openai_codex", - &serde_json::to_string(&expired_openai_credential()).unwrap(), - SecretType::Credential, - None, - ) - .unwrap(); - vault - .set( - "anthropic", - &serde_json::to_string(&api_key_credential( - ProviderId::anthropic(), - "anthropic-key", - )) - .unwrap(), - SecretType::Credential, - None, - ) - .unwrap(); + vault_set_oauth( + &mut vault, + crate::OPENAI_CODEX_VAULT_SECRET_NAME, + &expired_openai_credential(), + ) + .unwrap(); + vault_set_token(&mut vault, "ANTHROPIC_API_KEY", "anthropic-key").unwrap(); let source = VaultCredentialSource::with_env_lookup(Arc::new(AsyncRwLock::new(vault)), |_| None); @@ -165,27 +141,8 @@ mod tests { async fn configured_providers_reads_from_vault_without_refreshing() { let dir = tempfile::tempdir().unwrap(); let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); - vault - .set( - "openai", - &serde_json::to_string(&api_key_credential(ProviderId::openai(), "openai-key")) - .unwrap(), - SecretType::Credential, - None, - ) - .unwrap(); - vault - .set( - "anthropic", - &serde_json::to_string(&api_key_credential( - ProviderId::anthropic(), - "anthropic-key", - )) - .unwrap(), - SecretType::Credential, - None, - ) - .unwrap(); + vault_set_token(&mut vault, "OPENAI_API_KEY", "openai-key").unwrap(); + vault_set_token(&mut vault, "ANTHROPIC_API_KEY", "anthropic-key").unwrap(); let source = VaultCredentialSource::with_env_lookup(Arc::new(AsyncRwLock::new(vault)), |_| None); let catalog = default_catalog(); diff --git a/lib/crates/fabro-cli/src/args.rs b/lib/crates/fabro-cli/src/args.rs index 57033439c..d0b0f85a4 100644 --- a/lib/crates/fabro-cli/src/args.rs +++ b/lib/crates/fabro-cli/src/args.rs @@ -654,7 +654,7 @@ pub(crate) struct SecretRmArgs { #[derive(Clone, Copy, Debug, ValueEnum)] pub(crate) enum SecretTypeArg { - Environment, + Token, File, } @@ -668,7 +668,7 @@ pub(crate) struct SecretSetArgs { #[arg(long, conflicts_with = "value")] pub(crate) value_stdin: bool, /// Secret storage type - #[arg(long, value_enum, default_value = "environment")] + #[arg(long, value_enum, default_value = "token")] pub(crate) r#type: SecretTypeArg, /// Optional human-readable description #[arg(long)] diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index e3af73d8d..5782a93f0 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -21,7 +21,7 @@ use dialoguer::console::Term; use dialoguer::theme::ColorfulTheme; use dialoguer::{MultiSelect, Select}; use fabro_api::types::{CreateSecretRequest, SecretType as ApiSecretType}; -use fabro_auth::{AuthCredential, AuthMethod, codex_oauth_config, credential_id_for}; +use fabro_auth::{AuthMethod, LoginResult, OPENAI_CODEX_VAULT_SECRET_NAME, codex_oauth_config}; use fabro_client::{AuthEntry, AuthStore, DevTokenEntry, ServerTarget}; use fabro_config::bind::Bind; use fabro_config::daemon::ServerDaemon; @@ -98,7 +98,7 @@ fn provider_env_var_label(provider: &ProviderId, catalog: &Catalog) -> String { .iter() .filter_map(|credential| match credential { CredentialRef::Env(name) => Some(name.as_str()), - CredentialRef::Credential(_) => None, + CredentialRef::Vault(_) => None, }) .collect::>() .join(" / ") @@ -107,6 +107,13 @@ fn provider_env_var_label(provider: &ProviderId, catalog: &Catalog) -> String { .unwrap_or_else(|| "API_KEY".to_string()) } +fn provider_vault_secret_name(provider: &ProviderId, catalog: &Catalog) -> String { + catalog.provider_vault_secret_name(provider).map_or_else( + || format!("{}_API_KEY", provider.to_string().to_uppercase()), + str::to_string, + ) +} + // --------------------------------------------------------------------------- // Auth status display // --------------------------------------------------------------------------- @@ -307,7 +314,7 @@ struct InstallFacts { #[derive(Debug)] struct LlmInstallSelection { - credentials: Vec, + credentials: Vec, } #[derive(Debug)] @@ -1212,13 +1219,21 @@ async fn persist_vault_secrets_with( result } -fn credential_secret_request(credential: &AuthCredential) -> Result { - Ok(CreateSecretRequest { - name: credential_id_for(credential).map_err(anyhow::Error::msg)?, - value: serde_json::to_string(credential)?, - type_: ApiSecretType::Credential, - description: None, - }) +fn credential_secret_request(result: &LoginResult) -> Result { + match result { + LoginResult::ApiKey { provider, key } => Ok(CreateSecretRequest { + name: provider_vault_secret_name(provider, &INSTALL_CATALOG), + value: key.clone(), + type_: ApiSecretType::Token, + description: None, + }), + LoginResult::OAuth { credential, .. } => Ok(CreateSecretRequest { + name: OPENAI_CODEX_VAULT_SECRET_NAME.to_string(), + value: serde_json::to_string(credential)?, + type_: ApiSecretType::Oauth, + description: None, + }), + } } fn server_env_updates(secrets: &[(String, String)]) -> Vec { @@ -1322,7 +1337,7 @@ fn persist_github_install_changes( } for (key, value) in &writes.vault_set { vault - .set(key, value, VaultSecretType::Environment, None) + .set(key, value, VaultSecretType::Token, None) .map_err(anyhow::Error::from)?; } @@ -1758,7 +1773,7 @@ async fn run_install_inner(args: &InstallArgs, ctx: &CommandContext) -> Result<( vault_secrets.push(CreateSecretRequest { name: "GITHUB_TOKEN".to_string(), value: token, - type_: ApiSecretType::Environment, + type_: ApiSecretType::Token, description: None, }); Some(PendingGitHubSettings::Token) @@ -2542,14 +2557,12 @@ client_id = "client-id" CreateSecretRequest { name: "GITHUB_TOKEN".to_string(), value: "gh-token".to_string(), - type_: ApiSecretType::Environment, + type_: ApiSecretType::Token, description: None, }, - credential_secret_request(&AuthCredential { + credential_secret_request(&LoginResult::ApiKey { provider: ProviderId::anthropic(), - details: fabro_auth::AuthDetails::ApiKey { - key: "anthropic-key".to_string(), - }, + key: "anthropic-key".to_string(), }) .unwrap(), ]; @@ -2562,7 +2575,7 @@ client_id = "client-id" .body( serde_json::json!({ "name": "persisted", - "type": "environment", + "type": "token", "created_at": "2026-01-01T00:00:00Z", "updated_at": "2026-01-01T00:00:00Z" }) @@ -2611,7 +2624,7 @@ client_id = "client-id" let vault_secrets = [CreateSecretRequest { name: "GITHUB_TOKEN".to_string(), value: "gh-token".to_string(), - type_: ApiSecretType::Environment, + type_: ApiSecretType::Token, description: None, }]; let server = MockServer::start_async().await; @@ -2623,7 +2636,7 @@ client_id = "client-id" .body( serde_json::json!({ "name": "persisted", - "type": "environment", + "type": "token", "created_at": "2026-01-01T00:00:00Z", "updated_at": "2026-01-01T00:00:00Z" }) @@ -2822,7 +2835,7 @@ client_id = "client-id" let vault_secrets = [CreateSecretRequest { name: "GITHUB_CLI_TOKEN".to_string(), value: "gh-token".to_string(), - type_: ApiSecretType::Environment, + type_: ApiSecretType::Token, description: None, }]; let settings_path = dir.path().join(SETTINGS_CONFIG_FILENAME); @@ -2870,7 +2883,7 @@ client_id = "client-id" let vault_secrets = [CreateSecretRequest { name: "GITHUB_CLI_TOKEN".to_string(), value: "gh-token".to_string(), - type_: ApiSecretType::Environment, + type_: ApiSecretType::Token, description: None, }]; let settings_path = dir.path().join(SETTINGS_CONFIG_FILENAME); @@ -2955,7 +2968,7 @@ client_id = "client-id" vault .get_entry(GITHUB_TOKEN_SECRET_KEY) .map(|entry| entry.secret_type), - Some(VaultSecretType::Environment) + Some(VaultSecretType::Token) ); assert_eq!(std::fs::read_to_string(&settings_path).unwrap(), "after"); } @@ -2976,7 +2989,7 @@ client_id = "client-id" .set( GITHUB_TOKEN_SECRET_KEY, "token", - VaultSecretType::Environment, + VaultSecretType::Token, None, ) .unwrap(); diff --git a/lib/crates/fabro-cli/src/commands/provider/login.rs b/lib/crates/fabro-cli/src/commands/provider/login.rs index 06dd8ec2b..7d92e20fb 100644 --- a/lib/crates/fabro-cli/src/commands/provider/login.rs +++ b/lib/crates/fabro-cli/src/commands/provider/login.rs @@ -1,6 +1,6 @@ -use anyhow::Result; +use anyhow::{Context, Result}; use fabro_api::types; -use fabro_auth::credential_id_for; +use fabro_auth::{LoginResult, OPENAI_CODEX_VAULT_SECRET_NAME}; use fabro_util::terminal::Styles; use crate::args::ProviderLoginArgs; @@ -16,7 +16,7 @@ pub(super) async fn login_command( let s = Styles::detect_stderr(); let ctx = base_ctx.with_target(&args.target)?; let server = ctx.server().await?; - let credential = if args.api_key_stdin { + let result = if args.api_key_stdin { provider_auth::authenticate_provider_with_api_key_source_and_catalog( args.provider, provider_auth::ApiKeySource::Stdin, @@ -34,22 +34,33 @@ pub(super) async fn login_command( ) .await? }; - let credential_id = credential_id_for(&credential).map_err(anyhow::Error::msg)?; - let value = serde_json::to_string(&credential)?; + + let (name, value, type_) = match result { + LoginResult::ApiKey { provider, key } => { + let name = ctx + .catalog()? + .provider_vault_secret_name(&provider) + .with_context(|| { + format!("provider '{provider}' does not define a vault credential path") + })? + .to_string(); + (name, key, types::SecretType::Token) + } + LoginResult::OAuth { credential, .. } => ( + OPENAI_CODEX_VAULT_SECRET_NAME.to_string(), + serde_json::to_string(&credential)?, + types::SecretType::Oauth, + ), + }; server .create_secret(types::CreateSecretRequest { - name: credential_id.clone(), + name: name.clone(), value, - type_: types::SecretType::Credential, + type_, description: None, }) .await?; - fabro_util::printerr!( - printer, - " {} Saved {}", - s.green.apply_to("✔"), - credential_id - ); + fabro_util::printerr!(printer, " {} Saved {}", s.green.apply_to("✔"), name); Ok(()) } diff --git a/lib/crates/fabro-cli/src/commands/run/runner.rs b/lib/crates/fabro-cli/src/commands/run/runner.rs index 3b9e2593c..651df7ee3 100644 --- a/lib/crates/fabro-cli/src/commands/run/runner.rs +++ b/lib/crates/fabro-cli/src/commands/run/runner.rs @@ -650,10 +650,8 @@ mod tests { use std::sync::Arc; use chrono::Utc; - use fabro_auth::{AuthCredential, AuthDetails}; use fabro_config::Storage; use fabro_interview::{AnswerValue, ControlInterviewer, Interviewer, Question}; - use fabro_model::ProviderId; use fabro_types::run_event::{ InterviewCompletedProps, InterviewStartedProps, RunCompletedProps, RunControlEffectProps, RunFailedProps, RunStatusTransitionProps, @@ -981,23 +979,12 @@ mod tests { let storage = Storage::new(temp.path()); let mut vault = Vault::load(storage.secrets_path()).unwrap(); vault - .set( - "anthropic", - &serde_json::to_string(&AuthCredential { - provider: ProviderId::anthropic(), - details: AuthDetails::ApiKey { - key: "vault-key".to_string(), - }, - }) - .unwrap(), - SecretType::Credential, - None, - ) + .set("ANTHROPIC_API_KEY", "vault-key", SecretType::Token, None) .unwrap(); let loaded = load_worker_vault(Some(temp.path())).unwrap().unwrap(); let guard = loaded.read().await; - let credential = guard.get("anthropic").unwrap(); + let credential = guard.get("ANTHROPIC_API_KEY").unwrap(); assert!(credential.contains("vault-key")); } diff --git a/lib/crates/fabro-cli/src/commands/secret/set.rs b/lib/crates/fabro-cli/src/commands/secret/set.rs index 2a361b875..60378c1b0 100644 --- a/lib/crates/fabro-cli/src/commands/secret/set.rs +++ b/lib/crates/fabro-cli/src/commands/secret/set.rs @@ -20,7 +20,7 @@ use crate::shared::provider_auth::prompt_password; fn api_secret_type(secret_type: SecretTypeArg) -> types::SecretType { match secret_type { - SecretTypeArg::Environment => types::SecretType::Environment, + SecretTypeArg::Token => types::SecretType::Token, SecretTypeArg::File => types::SecretType::File, } } diff --git a/lib/crates/fabro-cli/src/shared/provider_auth.rs b/lib/crates/fabro-cli/src/shared/provider_auth.rs index 9d84aa61e..71eaac652 100644 --- a/lib/crates/fabro-cli/src/shared/provider_auth.rs +++ b/lib/crates/fabro-cli/src/shared/provider_auth.rs @@ -15,7 +15,7 @@ use dialoguer::console::Term; use dialoguer::theme::ColorfulTheme; use dialoguer::{Confirm, Password}; use fabro_auth::{ - ApiCredential, AuthContextRequest, AuthContextResponse, AuthCredential, AuthMethod, + ApiCredential, AuthContextRequest, AuthContextResponse, AuthMethod, LoginResult, codex_oauth_config, strategy_for, }; use fabro_llm::client::Client as LlmClient; @@ -207,7 +207,7 @@ pub(crate) async fn authenticate_provider( provider: ProviderId, s: &Styles, printer: Printer, -) -> Result { +) -> Result { authenticate_provider_with_catalog(provider, s, printer, default_catalog_for_provider_auth()?) .await } @@ -217,7 +217,7 @@ pub(crate) async fn authenticate_provider_with_catalog( s: &Styles, printer: Printer, catalog: Arc, -) -> Result { +) -> Result { api_key_catalog_provider(&provider, catalog.as_ref())?; let method = pick_auth_method(&provider).await?; authenticate_provider_with_method_and_catalog(provider, method, s, printer, catalog).await @@ -228,7 +228,7 @@ pub(crate) async fn authenticate_provider_with_api_key_source( source: ApiKeySource, s: &Styles, printer: Printer, -) -> Result { +) -> Result { authenticate_provider_with_api_key_source_and_catalog( provider, source, @@ -245,7 +245,7 @@ pub(crate) async fn authenticate_provider_with_api_key_source_and_catalog( s: &Styles, printer: Printer, catalog: Arc, -) -> Result { +) -> Result { api_key_catalog_provider(&provider, catalog.as_ref())?; let mut strategy = strategy_for(&provider, AuthMethod::ApiKey, catalog.as_ref()); let request = strategy.init().await?; @@ -259,7 +259,7 @@ pub(crate) async fn authenticate_provider_with_method( method: AuthMethod, s: &Styles, printer: Printer, -) -> Result { +) -> Result { authenticate_provider_with_method_and_catalog( provider, method, @@ -276,7 +276,7 @@ pub(crate) async fn authenticate_provider_with_method_and_catalog( s: &Styles, printer: Printer, catalog: Arc, -) -> Result { +) -> Result { api_key_catalog_provider(&provider, catalog.as_ref())?; let mut strategy = strategy_for(&provider, method, catalog.as_ref()); let request = strategy.init().await?; diff --git a/lib/crates/fabro-cli/tests/it/cmd/doctor.rs b/lib/crates/fabro-cli/tests/it/cmd/doctor.rs index 8a9b19d61..45f0436e8 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/doctor.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/doctor.rs @@ -5,9 +5,7 @@ use std::process::Output; -use fabro_auth::{AuthCredential, AuthDetails}; use fabro_config::Storage; -use fabro_model::ProviderId; use fabro_test::{fabro_snapshot, test_context, twin_openai}; use fabro_vault::{SecretType, Vault}; @@ -24,26 +22,12 @@ fn toml_path(path: &std::path::Path) -> String { .replace('"', "\\\"") } -fn seed_openai_vault(storage_dir: &std::path::Path, base_url: &str, api_key: &str) { +fn seed_openai_vault(storage_dir: &std::path::Path, api_key: &str) { let mut vault = Vault::load(Storage::new(storage_dir).secrets_path()).expect("test vault should load"); vault - .set( - "openai", - &serde_json::to_string(&AuthCredential { - provider: ProviderId::openai(), - details: AuthDetails::ApiKey { - key: api_key.to_string(), - }, - }) - .expect("OpenAI test credential should serialize"), - SecretType::Credential, - None, - ) + .set("OPENAI_API_KEY", api_key, SecretType::Token, None) .expect("OpenAI credential should store in test vault"); - vault - .set("OPENAI_BASE_URL", base_url, SecretType::Environment, None) - .expect("OpenAI base URL should store in test vault"); } #[test] @@ -116,7 +100,7 @@ strategy = "app" toml_path(&storage_dir) ), ); - seed_openai_vault(&storage_dir, &twin.base_url, &namespace); + seed_openai_vault(&storage_dir, &namespace); context.isolated_server(); let mut cmd = context.doctor(); diff --git a/lib/crates/fabro-cli/tests/it/cmd/install.rs b/lib/crates/fabro-cli/tests/it/cmd/install.rs index cecc043ed..5a3ed6fdb 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/install.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/install.rs @@ -451,6 +451,6 @@ mode = "keep-me" vault .get_entry("GITHUB_TOKEN") .map(|entry| entry.secret_type), - Some(SecretType::Environment) + Some(SecretType::Token) ); } diff --git a/lib/crates/fabro-cli/tests/it/cmd/run.rs b/lib/crates/fabro-cli/tests/it/cmd/run.rs index 51959919f..c86788e4b 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/run.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/run.rs @@ -3,9 +3,7 @@ reason = "integration tests stage fixtures with sync std::fs; test infrastructure, not Tokio-hot path" )] -use fabro_auth::{AuthCredential, AuthDetails}; use fabro_config::Storage; -use fabro_model::ProviderId; use fabro_test::{fabro_json_snapshot, fabro_snapshot, test_context}; use fabro_vault::{SecretType, Vault}; use httpmock::MockServer; @@ -89,15 +87,9 @@ fn seed_anthropic_vault(storage_dir: &std::path::Path) { Vault::load(Storage::new(storage_dir).secrets_path()).expect("test vault should load"); vault .set( - "anthropic", - &serde_json::to_string(&AuthCredential { - provider: ProviderId::anthropic(), - details: AuthDetails::ApiKey { - key: "vault-anthropic-key".to_string(), - }, - }) - .expect("Anthropic test credential should serialize"), - SecretType::Credential, + "ANTHROPIC_API_KEY", + "vault-anthropic-key", + SecretType::Token, None, ) .expect("Anthropic credential should store in test vault"); diff --git a/lib/crates/fabro-cli/tests/it/cmd/secret.rs b/lib/crates/fabro-cli/tests/it/cmd/secret.rs index 6e7eca61d..2f7b2592a 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/secret.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/secret.rs @@ -52,7 +52,7 @@ fn test_secret_lifecycle() { secret(&["list"]) .success() .stdout(predicates::str::contains("FOO")) - .stdout(predicates::str::contains("environment")); + .stdout(predicates::str::contains("token")); // 3. update FOO secret(&["set", "FOO", "updated"]).success(); @@ -96,7 +96,7 @@ fn test_secret_list_alias_ls() { .assert() .success() .stdout(predicates::str::contains("X")) - .stdout(predicates::str::contains("environment")); + .stdout(predicates::str::contains("token")); } #[test] @@ -129,7 +129,7 @@ fn test_secret_value_with_equals() { .assert() .success() .stdout(predicates::str::contains("URL")) - .stdout(predicates::str::contains("environment")) + .stdout(predicates::str::contains("token")) .stdout(predicates::str::contains("https://x.com?a=1&b=2").not()); } diff --git a/lib/crates/fabro-cli/tests/it/cmd/secret_list.rs b/lib/crates/fabro-cli/tests/it/cmd/secret_list.rs index 531ca77e3..88e79771b 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/secret_list.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/secret_list.rs @@ -47,7 +47,7 @@ fn secret_list_json_returns_metadata_only() { .iter() .find(|entry| entry["name"] == "ANTHROPIC_API_KEY") .expect("secret list should include the saved key"); - assert_eq!(entry["type"], "environment"); + assert_eq!(entry["type"], "token"); assert!(entry.get("updated_at").is_some()); assert!(entry.get("value").is_none()); } diff --git a/lib/crates/fabro-cli/tests/it/cmd/secret_set.rs b/lib/crates/fabro-cli/tests/it/cmd/secret_set.rs index 511651fc9..d67459c36 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/secret_set.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/secret_set.rs @@ -21,7 +21,7 @@ fn help() { --json Output as JSON [env: FABRO_JSON=] --value-stdin Read the secret value from stdin --debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=] - --type Secret storage type [default: environment] [possible values: environment, file] + --type Secret storage type [default: token] [possible values: token, file] --description Optional human-readable description --no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true] --quiet Suppress non-essential output [env: FABRO_QUIET=] diff --git a/lib/crates/fabro-cli/tests/it/workflow/acp.rs b/lib/crates/fabro-cli/tests/it/workflow/acp.rs index 3a20fc63a..706a0729b 100644 --- a/lib/crates/fabro-cli/tests/it/workflow/acp.rs +++ b/lib/crates/fabro-cli/tests/it/workflow/acp.rs @@ -4,9 +4,7 @@ )] use fabro_acp::test_support::fake_acp_agent_script; -use fabro_auth::{AuthCredential, AuthDetails}; use fabro_config::Storage; -use fabro_model::ProviderId; use fabro_test::test_context; use fabro_types::EventBody; use fabro_vault::{SecretType, Vault}; @@ -161,18 +159,7 @@ fn seed_openai_vault(storage_dir: &std::path::Path) { let mut vault = Vault::load(Storage::new(storage_dir).secrets_path()).expect("test vault should load"); vault - .set( - "openai", - &serde_json::to_string(&AuthCredential { - provider: ProviderId::openai(), - details: AuthDetails::ApiKey { - key: "test-openai-key".to_string(), - }, - }) - .expect("OpenAI test credential should serialize"), - SecretType::Credential, - None, - ) + .set("OPENAI_API_KEY", "test-openai-key", SecretType::Token, None) .expect("OpenAI credential should store in test vault"); } diff --git a/lib/crates/fabro-cli/tests/it/workflow/hooks.rs b/lib/crates/fabro-cli/tests/it/workflow/hooks.rs index cc68abaf4..3561b254c 100644 --- a/lib/crates/fabro-cli/tests/it/workflow/hooks.rs +++ b/lib/crates/fabro-cli/tests/it/workflow/hooks.rs @@ -11,9 +11,7 @@ use std::process::Output; -use fabro_auth::{AuthCredential, AuthDetails}; use fabro_config::Storage; -use fabro_model::ProviderId; use fabro_test::{ TestMode, TwinOpenAi, TwinScenario, TwinScenarios, TwinToolCall, test_context, twin_openai, }; @@ -93,34 +91,20 @@ fn write_hook_settings(context: &fabro_test::TestContext, hook: &str) { context.write_home(".fabro/settings.toml", settings); } -fn seed_openai_vault(storage_dir: &std::path::Path, base_url: &str, api_key: &str) { +fn seed_openai_vault(storage_dir: &std::path::Path, api_key: &str) { let mut vault = Vault::load(Storage::new(storage_dir).secrets_path()).expect("test vault should load"); vault - .set( - "openai", - &serde_json::to_string(&AuthCredential { - provider: ProviderId::openai(), - details: AuthDetails::ApiKey { - key: api_key.to_string(), - }, - }) - .expect("OpenAI test credential should serialize"), - SecretType::Credential, - None, - ) + .set("OPENAI_API_KEY", api_key, SecretType::Token, None) .expect("OpenAI credential should store in test vault"); - vault - .set("OPENAI_BASE_URL", base_url, SecretType::Environment, None) - .expect("OpenAI base URL should store in test vault"); } fn configure_twin_server( context: &mut fabro_test::TestContext, - twin: &TwinOpenAi, + _twin: &TwinOpenAi, namespace: &str, ) { - seed_openai_vault(&twin_server_storage_dir(context), &twin.base_url, namespace); + seed_openai_vault(&twin_server_storage_dir(context), namespace); context.isolated_server(); } diff --git a/lib/crates/fabro-config/src/layers/llm.rs b/lib/crates/fabro-config/src/layers/llm.rs index 2d841eb8c..94b44ab2a 100644 --- a/lib/crates/fabro-config/src/layers/llm.rs +++ b/lib/crates/fabro-config/src/layers/llm.rs @@ -7,7 +7,7 @@ //! display_name = "Kimi" //! adapter = "openai_compatible" //! base_url = "https://api.moonshot.ai/v1" -//! auth = { credentials = ["credential:kimi", "env:KIMI_API_KEY"] } +//! auth = { credentials = ["env:KIMI_API_KEY", "vault:KIMI_API_KEY"] } //! priority = 60 //! enabled = true //! aliases = ["moonshot"] @@ -212,9 +212,9 @@ mod tests { // ---- CredentialRef ---------------------------------------------------- #[test] - fn credential_ref_parses_credential_form() { - let r = CredentialRef::from_str("credential:openai_codex").unwrap(); - assert_eq!(r, CredentialRef::Credential("openai_codex".to_string())); + fn credential_ref_parses_vault_form() { + let r = CredentialRef::from_str("vault:OPENAI_CODEX").unwrap(); + assert_eq!(r, CredentialRef::Vault("OPENAI_CODEX".to_string())); } #[test] @@ -225,7 +225,7 @@ mod tests { #[test] fn credential_ref_rejects_literal_secret() { - // A literal API key contains no `credential:` or `env:` prefix. + // A literal API key contains no `vault:` or `env:` prefix. let err = CredentialRef::from_str("sk-ant-1234").unwrap_err(); assert!(err.to_string().contains("must be")); assert!( @@ -235,8 +235,8 @@ mod tests { } #[test] - fn credential_ref_rejects_empty_credential_id() { - let err = CredentialRef::from_str("credential:").unwrap_err(); + fn credential_ref_rejects_empty_vault_name() { + let err = CredentialRef::from_str("vault:").unwrap_err(); assert!(err.to_string().contains("missing")); } @@ -248,8 +248,8 @@ mod tests { #[test] fn credential_ref_round_trips_through_string() { - let r = CredentialRef::Credential("kimi".to_string()); - assert_eq!(r.to_string(), "credential:kimi"); + let r = CredentialRef::Vault("kimi".to_string()); + assert_eq!(r.to_string(), "vault:kimi"); let back: CredentialRef = r.to_string().parse().unwrap(); assert_eq!(back, r); } @@ -263,7 +263,7 @@ mod tests { #[test] fn credential_ref_deserializes_from_toml_string() { - let parsed: CredentialRef = toml::from_str(r#"v = "credential:foo""#) + let parsed: CredentialRef = toml::from_str(r#"v = "vault:foo""#) .map(|v: toml::Value| { v.as_table() .unwrap() @@ -274,7 +274,7 @@ mod tests { .unwrap() }) .unwrap(); - assert_eq!(parsed, CredentialRef::Credential("foo".to_string())); + assert_eq!(parsed, CredentialRef::Vault("foo".to_string())); } #[test] @@ -367,8 +367,8 @@ agent_profile = "gemini" } #[test] - fn header_value_ref_parses_credential_form() { - let parsed: HeaderValueRef = toml::from_str(r#"value = { credential = "portkey_config" }"#) + fn header_value_ref_parses_vault_form() { + let parsed: HeaderValueRef = toml::from_str(r#"value = { vault = "portkey_config" }"#) .map(|v: toml::Value| { v.as_table() .unwrap() @@ -380,11 +380,8 @@ agent_profile = "gemini" }) .unwrap(); - assert_eq!( - parsed, - HeaderValueRef::Credential("portkey_config".to_string()) - ); - assert_eq!(parsed.to_string(), "credential:portkey_config"); + assert_eq!(parsed, HeaderValueRef::Vault("portkey_config".to_string())); + assert_eq!(parsed.to_string(), "vault:portkey_config"); } #[test] @@ -457,7 +454,7 @@ agent_profile = "gemini" for source in [ r#"value = { literal = "" }"#, r#"value = { env = "" }"#, - r#"value = { credential = "" }"#, + r#"value = { vault = "" }"#, ] { let err = toml::from_str::(source).unwrap_err(); assert!(err.to_string().contains("must not be empty")); @@ -479,7 +476,7 @@ enabled = true aliases = ["moonshot"] [providers.kimi.auth] -credentials = ["credential:kimi", "env:KIMI_API_KEY"] +credentials = ["env:KIMI_API_KEY", "vault:KIMI_API_KEY"] "#; let layer: LlmLayer = toml::from_str(toml).unwrap(); let kimi = layer.providers.get("kimi").unwrap(); @@ -489,8 +486,8 @@ credentials = ["credential:kimi", "env:KIMI_API_KEY"] let auth = kimi.auth.as_ref().expect("expected api_key auth"); assert_eq!(auth.header, ApiKeyHeaderPolicy::Bearer); assert_eq!(auth.credentials, vec![ - CredentialRef::Credential("kimi".to_string()), CredentialRef::Env("KIMI_API_KEY".to_string()), + CredentialRef::Vault("KIMI_API_KEY".to_string()), ]); assert_eq!(kimi.base_url.as_deref(), Some("https://api.moonshot.ai/v1")); assert_eq!(kimi.priority, Some(60)); @@ -509,7 +506,7 @@ base_url = "https://api.portkey.ai/v1" [providers.portkey.extra_headers] x-portkey-api-key = { env = "PORTKEY_API_KEY" } x-portkey-provider = { literal = "@bedrock-prod" } -x-portkey-config = { credential = "portkey_config" } +x-portkey-config = { vault = "portkey_config" } "#; let layer: LlmLayer = toml::from_str(toml).unwrap(); @@ -527,7 +524,7 @@ x-portkey-config = { credential = "portkey_config" } ); assert_eq!( headers.get("x-portkey-config"), - Some(&HeaderValueRef::Credential("portkey_config".to_string())), + Some(&HeaderValueRef::Vault("portkey_config".to_string())), ); } @@ -753,7 +750,7 @@ mystery = 1 let low = ProviderSettings { auth: Some(ProviderAuthConfig { credentials: vec![ - CredentialRef::Credential("bar".to_string()), + CredentialRef::Vault("bar".to_string()), CredentialRef::Env("BAZ".to_string()), ], header: ApiKeyHeaderPolicy::Custom { diff --git a/lib/crates/fabro-dev/src/commands/docs_options_reference.rs b/lib/crates/fabro-dev/src/commands/docs_options_reference.rs index 1301874e8..2eca08d75 100644 --- a/lib/crates/fabro-dev/src/commands/docs_options_reference.rs +++ b/lib/crates/fabro-dev/src/commands/docs_options_reference.rs @@ -227,12 +227,12 @@ enabled = true aliases = ["gateway"] [llm.providers.proxy.auth] -credentials = ["env:ACME_GATEWAY_API_KEY"] +credentials = ["env:ACME_GATEWAY_API_KEY", "vault:ACME_GATEWAY_API_KEY"] [llm.providers.proxy.extra_headers] x-portkey-api-key = { env = "PORTKEY_API_KEY" } x-portkey-config = { literal = "@bedrock-prod" } -x-team-secret = { credential = "gateway_team_secret" } +x-team-secret = { vault = "gateway_team_secret" } ``` | Key | Type / values | Default | Description | @@ -243,9 +243,9 @@ x-team-secret = { credential = "gateway_team_secret" } | `billing_policy` | `"openai"` \| `"anthropic"` \| `"gemini"` \| `"none"` | derived from `adapter` | Provider-owned billing algorithm for usage estimates. Override for exceptional providers such as local no-billing runtimes. | | `base_url` | string | built-in value or adapter runtime default | Provider API base URL. Required for most custom OpenAI-compatible providers. | | `auth` | table | omitted | API-key auth config. Omit the table entirely for providers that need no API key; any `extra_headers` are still attached. | -| `auth.credentials` | array | required when `auth` present | Ordered credential refs. Accepted forms are `credential:` and `env:`. Literal secret strings are rejected. | +| `auth.credentials` | array | required when `auth` present | Ordered credential refs. Accepted forms are `vault:` and `env:`. Literal secret strings are rejected. | | `auth.header` | `"bearer"` or `{ custom = "Header-Name" }` | `"bearer"` | Primary API-key header policy. Omit when the provider uses a standard bearer token. | -| `extra_headers` | table | `{}` | Additional headers attached to provider requests. Values must be typed refs: `{ literal = "..." }`, `{ env = "NAME" }`, or `{ credential = "id" }`. | +| `extra_headers` | table | `{}` | Additional headers attached to provider requests. Values must be typed refs: `{ literal = "..." }`, `{ env = "NAME" }`, or `{ vault = "NAME" }`. | | `priority` | integer | `0` | Higher-priority configured providers win default selection; ties use canonical provider ID. | | `enabled` | boolean | `true` | Set `false` to disable a provider after lower-precedence layers define it. | | `aliases` | array | `[]` | Additional provider names accepted by model routing and fallback config. | diff --git a/lib/crates/fabro-install/src/lib.rs b/lib/crates/fabro-install/src/lib.rs index cc2256482..4e10bde87 100644 --- a/lib/crates/fabro-install/src/lib.rs +++ b/lib/crates/fabro-install/src/lib.rs @@ -665,12 +665,7 @@ name = "custom" let vault_path = storage.secrets_path(); let mut vault = Vault::load(vault_path.clone()).unwrap(); vault - .set( - "EXISTING_SECRET", - "keep", - VaultSecretType::Environment, - None, - ) + .set("EXISTING_SECRET", "keep", VaultSecretType::Token, None) .unwrap(); let result = persist_install_outputs_direct( @@ -684,7 +679,7 @@ name = "custom" &[VaultSecretWrite { name: "bad-secret-name".to_string(), value: "boom".to_string(), - secret_type: VaultSecretType::Environment, + secret_type: VaultSecretType::Token, description: None, }], Some(&PendingSettingsWrite { diff --git a/lib/crates/fabro-model/src/catalog.rs b/lib/crates/fabro-model/src/catalog.rs index 7022bf692..966d9ee8a 100644 --- a/lib/crates/fabro-model/src/catalog.rs +++ b/lib/crates/fabro-model/src/catalog.rs @@ -154,14 +154,14 @@ pub struct CostRates { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(into = "String", try_from = "String")] pub enum CredentialRef { - Credential(String), + Vault(String), Env(String), } impl std::fmt::Display for CredentialRef { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::Credential(id) => write!(f, "credential:{id}"), + Self::Vault(name) => write!(f, "vault:{name}"), Self::Env(name) => write!(f, "env:{name}"), } } @@ -177,11 +177,11 @@ impl FromStr for CredentialRef { type Err = CredentialRefParseError; fn from_str(value: &str) -> Result { - if let Some(id) = value.strip_prefix("credential:") { - if id.is_empty() { - return Err(CredentialRefParseError::EmptyCredential); + if let Some(name) = value.strip_prefix("vault:") { + if name.is_empty() { + return Err(CredentialRefParseError::EmptyVault); } - return Ok(Self::Credential(id.to_string())); + return Ok(Self::Vault(name.to_string())); } if let Some(name) = value.strip_prefix("env:") { if name.is_empty() { @@ -203,10 +203,10 @@ impl TryFrom for CredentialRef { #[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] pub enum CredentialRefParseError { - #[error("credential reference must be `credential:` or `env:`")] + #[error("credential reference must be `vault:` or `env:`")] Invalid, - #[error("credential reference is missing an ID after `credential:`")] - EmptyCredential, + #[error("credential reference is missing a name after `vault:`")] + EmptyVault, #[error("credential reference is missing a name after `env:`")] EmptyEnv, } @@ -313,7 +313,7 @@ pub enum BillingPolicy { pub enum HeaderValueRef { Literal(String), Env(String), - Credential(String), + Vault(String), } impl Serialize for HeaderValueRef { @@ -327,7 +327,7 @@ impl Serialize for HeaderValueRef { match self { Self::Literal(value) => map.serialize_entry("literal", value)?, Self::Env(value) => map.serialize_entry("env", value)?, - Self::Credential(value) => map.serialize_entry("credential", value)?, + Self::Vault(value) => map.serialize_entry("vault", value)?, } map.end() } @@ -338,7 +338,7 @@ impl std::fmt::Display for HeaderValueRef { match self { Self::Literal(_) => f.write_str("literal:"), Self::Env(name) => write!(f, "env:{name}"), - Self::Credential(id) => write!(f, "credential:{id}"), + Self::Vault(id) => write!(f, "vault:{id}"), } } } @@ -354,11 +354,11 @@ enum HeaderValueRefInput { #[serde(deny_unknown_fields)] struct HeaderValueRefSerde { #[serde(default)] - literal: Option, + literal: Option, #[serde(default)] - env: Option, + env: Option, #[serde(default)] - credential: Option, + vault: Option, } impl<'de> Deserialize<'de> for HeaderValueRef { @@ -385,7 +385,7 @@ impl TryFrom for HeaderValueRef { let populated = [ value.literal.as_ref(), value.env.as_ref(), - value.credential.as_ref(), + value.vault.as_ref(), ] .into_iter() .flatten() @@ -399,8 +399,8 @@ impl TryFrom for HeaderValueRef { if let Some(value) = value.env { return non_empty_header_value(value).map(Self::Env); } - if let Some(value) = value.credential { - return non_empty_header_value(value).map(Self::Credential); + if let Some(value) = value.vault { + return non_empty_header_value(value).map(Self::Vault); } unreachable!("populated field count was already checked"); } @@ -416,7 +416,7 @@ fn non_empty_header_value(value: String) -> Result Option<&str> { + self.provider(id)? + .auth + .as_ref()? + .credentials + .iter() + .find_map(|credential_ref| match credential_ref { + CredentialRef::Vault(name) => Some(name.as_str()), + CredentialRef::Env(_) => None, + }) + } + #[must_use] pub fn model_settings(&self, id: &str) -> Option<&CatalogModelSettings> { let model = self.get(id)?; @@ -2713,7 +2726,7 @@ display_name = "Bearer" adapter = "openai" [providers.bearer.auth] -credentials = ["credential:bearer", "env:BEARER_API_KEY"] +credentials = ["env:BEARER_API_KEY", "vault:BEARER_API_KEY"] [providers.custom] display_name = "Custom" @@ -2744,8 +2757,8 @@ billing_policy = "none" bearer.auth, Some(ProviderAuthConfig { credentials: vec![ - CredentialRef::Credential("bearer".to_string()), CredentialRef::Env("BEARER_API_KEY".to_string()), + CredentialRef::Vault("BEARER_API_KEY".to_string()), ], header: ApiKeyHeaderPolicy::Bearer, }) diff --git a/lib/crates/fabro-model/src/catalog/providers/anthropic.toml b/lib/crates/fabro-model/src/catalog/providers/anthropic.toml index 5ea4b1284..ed3bd88ff 100644 --- a/lib/crates/fabro-model/src/catalog/providers/anthropic.toml +++ b/lib/crates/fabro-model/src/catalog/providers/anthropic.toml @@ -6,7 +6,7 @@ base_url = "https://api.anthropic.com/v1" priority = 100 [providers.anthropic.auth] -credentials = ["credential:anthropic", "env:ANTHROPIC_API_KEY"] +credentials = ["env:ANTHROPIC_API_KEY", "vault:ANTHROPIC_API_KEY"] header = { custom = "x-api-key" } [models."claude-opus-4-7"] diff --git a/lib/crates/fabro-model/src/catalog/providers/gemini.toml b/lib/crates/fabro-model/src/catalog/providers/gemini.toml index c41c7cb31..f91ca5c63 100644 --- a/lib/crates/fabro-model/src/catalog/providers/gemini.toml +++ b/lib/crates/fabro-model/src/catalog/providers/gemini.toml @@ -6,7 +6,7 @@ base_url = "https://generativelanguage.googleapis.com/v1beta" priority = 80 [providers.gemini.auth] -credentials = ["credential:gemini", "env:GEMINI_API_KEY", "env:GOOGLE_API_KEY"] +credentials = ["env:GEMINI_API_KEY", "env:GOOGLE_API_KEY", "vault:GEMINI_API_KEY"] header = { custom = "x-goog-api-key" } [models."gemini-3.1-pro-preview"] diff --git a/lib/crates/fabro-model/src/catalog/providers/inception.toml b/lib/crates/fabro-model/src/catalog/providers/inception.toml index 60a18b1bf..1d2b08a64 100644 --- a/lib/crates/fabro-model/src/catalog/providers/inception.toml +++ b/lib/crates/fabro-model/src/catalog/providers/inception.toml @@ -6,7 +6,7 @@ base_url = "https://api.inceptionlabs.ai/v1" priority = 40 [providers.inception.auth] -credentials = ["credential:inception", "env:INCEPTION_API_KEY"] +credentials = ["env:INCEPTION_API_KEY", "vault:INCEPTION_API_KEY"] [models."mercury-2"] provider = "inception" diff --git a/lib/crates/fabro-model/src/catalog/providers/kimi.toml b/lib/crates/fabro-model/src/catalog/providers/kimi.toml index bdf35f68e..4ea4b289c 100644 --- a/lib/crates/fabro-model/src/catalog/providers/kimi.toml +++ b/lib/crates/fabro-model/src/catalog/providers/kimi.toml @@ -6,7 +6,7 @@ base_url = "https://api.moonshot.ai/v1" priority = 70 [providers.kimi.auth] -credentials = ["credential:kimi", "env:KIMI_API_KEY"] +credentials = ["env:KIMI_API_KEY", "vault:KIMI_API_KEY"] [models."kimi-k2.5"] provider = "kimi" diff --git a/lib/crates/fabro-model/src/catalog/providers/litellm.toml b/lib/crates/fabro-model/src/catalog/providers/litellm.toml index 9519d67ef..55f5aef15 100644 --- a/lib/crates/fabro-model/src/catalog/providers/litellm.toml +++ b/lib/crates/fabro-model/src/catalog/providers/litellm.toml @@ -6,7 +6,7 @@ priority = 50 enabled = false [providers.litellm.auth] -credentials = ["credential:litellm", "env:LITELLM_API_KEY"] +credentials = ["env:LITELLM_API_KEY", "vault:LITELLM_API_KEY"] # To enable LiteLLM, add entries like these to settings.toml: # diff --git a/lib/crates/fabro-model/src/catalog/providers/minimax.toml b/lib/crates/fabro-model/src/catalog/providers/minimax.toml index f59371cfa..172a6fd6a 100644 --- a/lib/crates/fabro-model/src/catalog/providers/minimax.toml +++ b/lib/crates/fabro-model/src/catalog/providers/minimax.toml @@ -6,7 +6,7 @@ base_url = "https://api.minimax.io/v1" priority = 50 [providers.minimax.auth] -credentials = ["credential:minimax", "env:MINIMAX_API_KEY"] +credentials = ["env:MINIMAX_API_KEY", "vault:MINIMAX_API_KEY"] [models."minimax-m2.5"] provider = "minimax" diff --git a/lib/crates/fabro-model/src/catalog/providers/openai.toml b/lib/crates/fabro-model/src/catalog/providers/openai.toml index b29aea569..87dd2d8ac 100644 --- a/lib/crates/fabro-model/src/catalog/providers/openai.toml +++ b/lib/crates/fabro-model/src/catalog/providers/openai.toml @@ -6,7 +6,7 @@ base_url = "https://api.openai.com/v1" priority = 90 [providers.openai.auth] -credentials = ["credential:openai", "credential:openai_codex", "env:OPENAI_API_KEY"] +credentials = ["env:OPENAI_API_KEY", "vault:OPENAI_API_KEY", "vault:OPENAI_CODEX"] [models."gpt-5.2"] provider = "openai" diff --git a/lib/crates/fabro-model/src/catalog/providers/venice.toml b/lib/crates/fabro-model/src/catalog/providers/venice.toml index f4dc45b50..bb91aeada 100644 --- a/lib/crates/fabro-model/src/catalog/providers/venice.toml +++ b/lib/crates/fabro-model/src/catalog/providers/venice.toml @@ -6,7 +6,7 @@ priority = 35 aliases = ["venice-ai"] [providers.venice.auth] -credentials = ["credential:venice", "env:VENICE_API_KEY"] +credentials = ["env:VENICE_API_KEY", "vault:VENICE_API_KEY"] [models."venice-uncensored-1-2"] provider = "venice" diff --git a/lib/crates/fabro-model/src/catalog/providers/zai.toml b/lib/crates/fabro-model/src/catalog/providers/zai.toml index 54fe48134..1a8bf941f 100644 --- a/lib/crates/fabro-model/src/catalog/providers/zai.toml +++ b/lib/crates/fabro-model/src/catalog/providers/zai.toml @@ -6,7 +6,7 @@ base_url = "https://api.z.ai/api/coding/paas/v4" priority = 60 [providers.zai.auth] -credentials = ["credential:zai", "env:ZAI_API_KEY"] +credentials = ["env:ZAI_API_KEY", "vault:ZAI_API_KEY"] [models."glm-4.7"] provider = "zai" diff --git a/lib/crates/fabro-server/src/demo/mod.rs b/lib/crates/fabro-server/src/demo/mod.rs index 6e1496332..213df8cd1 100644 --- a/lib/crates/fabro-server/src/demo/mod.rs +++ b/lib/crates/fabro-server/src/demo/mod.rs @@ -592,13 +592,13 @@ pub(crate) async fn list_secrets( "data": [ { "name": "OPENAI_API_KEY", - "type": "environment", + "type": "token", "created_at": "2026-04-05T12:00:00Z", "updated_at": "2026-04-05T12:00:00Z" }, { "name": "GITHUB_APP_PRIVATE_KEY", - "type": "environment", + "type": "token", "created_at": "2026-04-05T12:05:00Z", "updated_at": "2026-04-05T12:05:00Z" } diff --git a/lib/crates/fabro-server/src/diagnostics.rs b/lib/crates/fabro-server/src/diagnostics.rs index e22009aef..8fd258ef4 100644 --- a/lib/crates/fabro-server/src/diagnostics.rs +++ b/lib/crates/fabro-server/src/diagnostics.rs @@ -681,7 +681,6 @@ fn check_crypto(state: &AppState) -> CheckResult { #[cfg(test)] mod tests { - use fabro_auth::{AuthCredential, AuthDetails}; use fabro_config::RunLayer; use fabro_vault::SecretType; use httpmock::Method::POST; @@ -731,20 +730,14 @@ mod tests { .max_concurrent_runs(5) .provider_base_url("openai", server.url("/v1")) .build(); - let credential = AuthCredential { - provider: ProviderId::openai(), - details: AuthDetails::ApiKey { - key: "vault-openai-key".to_string(), - }, - }; state .vault .write() .await .set( - "openai_codex", - &serde_json::to_string(&credential).unwrap(), - SecretType::Credential, + "OPENAI_API_KEY", + "vault-openai-key", + SecretType::Token, None, ) .unwrap(); diff --git a/lib/crates/fabro-server/src/install.rs b/lib/crates/fabro-server/src/install.rs index d9774935d..df6696732 100644 --- a/lib/crates/fabro-server/src/install.rs +++ b/lib/crates/fabro-server/src/install.rs @@ -13,7 +13,6 @@ use axum::routing::{get, post, put}; use axum::{Json, Router, middleware}; use base64::Engine as _; use base64::engine::general_purpose::{STANDARD as BASE64_STANDARD, URL_SAFE_NO_PAD}; -use fabro_auth::{AuthCredential, AuthDetails, credential_id_for}; use fabro_config::Storage; use fabro_config::bind::{Bind, BindRequest}; use fabro_config::envfile::EnvFileUpdate; @@ -838,6 +837,14 @@ fn install_catalog_provider(provider: &ProviderId) -> Result<&'static CatalogPro } } +fn provider_secret_name(provider: &ProviderId) -> Result { + install_catalog_provider(provider)?; + INSTALL_CATALOG + .provider_vault_secret_name(provider) + .map(str::to_string) + .ok_or_else(|| format!("provider '{provider}' does not define a vault credential path")) +} + async fn put_install_server( State(state): State, headers: HeaderMap, @@ -1530,31 +1537,19 @@ async fn post_install_finish( vault_secrets.push(VaultSecretWrite { name: EnvVars::DAYTONA_API_KEY.to_string(), value: api_key.expose_secret().to_string(), - secret_type: VaultSecretType::Environment, + secret_type: VaultSecretType::Token, description: None, }); } for provider in llm.providers { - let credential = AuthCredential { - provider: provider.provider, - details: AuthDetails::ApiKey { - key: provider.api_key, - }, - }; - let name = match credential_id_for(&credential) { + let name = match provider_secret_name(&provider.provider) { Ok(name) => name, Err(err) => return install_error_response(StatusCode::UNPROCESSABLE_ENTITY, err), }; - let value = match serde_json::to_string(&credential) { - Ok(value) => value, - Err(err) => { - return install_error_response(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()); - } - }; vault_secrets.push(VaultSecretWrite { name, - value, - secret_type: VaultSecretType::Credential, + value: provider.api_key, + secret_type: VaultSecretType::Token, description: None, }); } @@ -1575,7 +1570,7 @@ async fn post_install_finish( vault_secrets.push(VaultSecretWrite { name: EnvVars::GITHUB_TOKEN.to_string(), value: github.token, - secret_type: VaultSecretType::Environment, + secret_type: VaultSecretType::Token, description: None, }); let dev_token_path = Storage::new(state.storage_dir.as_ref()) diff --git a/lib/crates/fabro-server/src/run_manifest.rs b/lib/crates/fabro-server/src/run_manifest.rs index 88b6efbd9..4b8216a23 100644 --- a/lib/crates/fabro-server/src/run_manifest.rs +++ b/lib/crates/fabro-server/src/run_manifest.rs @@ -2331,15 +2331,9 @@ provider = "daytona" .write() .await .set( - "openai", - &serde_json::to_string(&fabro_auth::AuthCredential { - provider: ProviderId::openai(), - details: fabro_auth::AuthDetails::ApiKey { - key: "test-openai-key".to_string(), - }, - }) - .unwrap(), - fabro_vault::SecretType::Credential, + "OPENAI_API_KEY", + "test-openai-key", + fabro_vault::SecretType::Token, None, ) .unwrap(); diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index a52299126..2be58dfb5 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -40,9 +40,7 @@ pub use fabro_api::types::{ SystemRepairRunsResponse, SystemRunCounts, TimelineEntryResponse, VncPreviewResponse, WriteBlobResponse, }; -use fabro_auth::{ - CredentialSource, VaultCredentialSource, auth_issue_message, parse_credential_secret, -}; +use fabro_auth::{CredentialSource, VaultCredentialSource, auth_issue_message}; #[cfg(test)] use fabro_config::RunSettingsBuilder; use fabro_config::daemon::ServerDaemon; @@ -1558,7 +1556,9 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result = Arc::new(VaultCredentialSource::with_env_lookup( Arc::clone(&vault), { diff --git a/lib/crates/fabro-server/src/server/handler/secrets.rs b/lib/crates/fabro-server/src/server/handler/secrets.rs index ae87e4c9c..2d448ccff 100644 --- a/lib/crates/fabro-server/src/server/handler/secrets.rs +++ b/lib/crates/fabro-server/src/server/handler/secrets.rs @@ -1,11 +1,11 @@ use std::sync::Arc; +use fabro_auth::OAuthCredential; use fabro_static::EnvVars; use super::super::{ ApiError, AppState, CreateSecretRequest, DeleteSecretRequest, IntoResponse, Json, RequiredUser, - Response, Router, SecretType, State, StatusCode, VaultError, get, parse_credential_secret, - spawn_blocking, + Response, Router, SecretType, State, StatusCode, VaultError, get, spawn_blocking, }; pub(super) fn routes() -> Router> { @@ -31,12 +31,13 @@ async fn create_secret( let name = body.name; let value = body.value; let description = body.description; - if secret_type == SecretType::Credential { - if let Err(err) = parse_credential_secret(&name, &value) { - return ApiError::bad_request(err).into_response(); + if secret_type == SecretType::Oauth { + if let Err(err) = serde_json::from_str::(&value) { + return ApiError::bad_request(format!("invalid oauth credential JSON: {err}")) + .into_response(); } } - if secret_type == SecretType::Environment && name == EnvVars::DAYTONA_API_KEY { + if secret_type == SecretType::Token && name == EnvVars::DAYTONA_API_KEY { match state.check_daytona_api_key(value.clone()).await { Ok(check) if check.ok() => {} Ok(check) => { diff --git a/lib/crates/fabro-server/src/server/tests.rs b/lib/crates/fabro-server/src/server/tests.rs index 9f3925b81..60353d4b5 100644 --- a/lib/crates/fabro-server/src/server/tests.rs +++ b/lib/crates/fabro-server/src/server/tests.rs @@ -11,7 +11,6 @@ use axum::body::Body; use axum::http::{Method, Request, header}; use axum::response::sse::{Event as SseEvent, Sse}; use chrono::{Duration as ChronoDuration, Utc}; -use fabro_auth::{AuthCredential, AuthDetails}; use fabro_config::ServerSettingsBuilder; use fabro_config::bind::Bind; use fabro_interview::{ @@ -223,15 +222,29 @@ async fn mock_daytona_current_key<'a>( .await } -fn openai_api_key_credential(key: &str) -> AuthCredential { - AuthCredential { - provider: ProviderId::openai(), - details: AuthDetails::ApiKey { - key: key.to_string(), +fn openai_oauth_credential() -> fabro_auth::OAuthCredential { + fabro_auth::OAuthCredential { + tokens: fabro_auth::OAuthTokens { + access_token: "access".to_string(), + refresh_token: Some("refresh".to_string()), + expires_at: Utc::now() + ChronoDuration::hours(1), }, + config: fabro_auth::OAuthConfig { + auth_url: "https://auth.openai.com".to_string(), + token_url: "https://auth.openai.com/oauth/token".to_string(), + client_id: "client".to_string(), + scopes: vec!["openid".to_string()], + redirect_uri: Some("https://auth.openai.com/deviceauth/callback".to_string()), + use_pkce: true, + }, + account_id: Some("acct_123".to_string()), } } +fn openai_oauth_credential_json() -> String { + serde_json::to_string(&openai_oauth_credential()).unwrap() +} + fn openai_responses_payload(text: &str) -> serde_json::Value { json!({ "id": "resp_1", @@ -982,7 +995,7 @@ fn clone_sandbox_credentials_are_available_for_clone_based_providers() { } #[tokio::test] -async fn create_secret_stores_file_secret_and_excludes_it_from_snapshot() { +async fn create_secret_stores_file_secret_outside_token_lookups() { let state = test_app_state(); let app = crate::test_support::build_test_router(Arc::clone(&state)); let req = Request::builder() @@ -1007,7 +1020,10 @@ async fn create_secret_stores_file_secret_and_excludes_it_from_snapshot() { assert_eq!(body["description"], "Test certificate"); let vault = state.vault.read().await; - assert!(!vault.snapshot().contains_key("/tmp/test.pem")); + assert_eq!( + vault.get_entry("/tmp/test.pem").unwrap().secret_type, + SecretType::File + ); assert_eq!(vault.file_secrets(), vec![( "/tmp/test.pem".to_string(), "pem-data".to_string() @@ -1083,30 +1099,9 @@ async fn github_webhook_accepts_valid_signature_with_wrong_bearer_token() { } #[tokio::test] -async fn create_secret_stores_valid_credential_entries() { +async fn create_secret_stores_valid_oauth_entries() { let state = test_app_state(); let app = crate::test_support::build_test_router(Arc::clone(&state)); - let credential = fabro_auth::AuthCredential { - provider: ProviderId::openai(), - details: fabro_auth::AuthDetails::CodexOAuth { - tokens: fabro_auth::OAuthTokens { - access_token: "access".to_string(), - refresh_token: Some("refresh".to_string()), - expires_at: chrono::DateTime::parse_from_rfc3339("2030-01-01T00:00:00Z") - .unwrap() - .with_timezone(&chrono::Utc), - }, - config: fabro_auth::OAuthConfig { - auth_url: "https://auth.openai.com".to_string(), - token_url: "https://auth.openai.com/oauth/token".to_string(), - client_id: "client".to_string(), - scopes: vec!["openid".to_string()], - redirect_uri: Some("https://auth.openai.com/deviceauth/callback".to_string()), - use_pkce: true, - }, - account_id: Some("acct_123".to_string()), - }, - }; let req = Request::builder() .method("POST") @@ -1114,9 +1109,9 @@ async fn create_secret_stores_valid_credential_entries() { .header("content-type", "application/json") .body(Body::from( serde_json::to_string(&serde_json::json!({ - "name": "openai_codex", - "value": serde_json::to_string(&credential).unwrap(), - "type": "credential" + "name": "OPENAI_CODEX", + "value": openai_oauth_credential_json(), + "type": "oauth" })) .unwrap(), )) @@ -1126,9 +1121,9 @@ async fn create_secret_stores_valid_credential_entries() { assert_status!(response, StatusCode::OK).await; let listed = state.vault.read().await.list(); assert_eq!(listed.len(), 1); - assert_eq!(listed[0].name, "openai_codex"); - assert_eq!(listed[0].secret_type, SecretType::Credential); - assert!(state.vault.read().await.get("openai_codex").is_some()); + assert_eq!(listed[0].name, "OPENAI_CODEX"); + assert_eq!(listed[0].secret_type, SecretType::Oauth); + assert!(state.vault.read().await.get("OPENAI_CODEX").is_some()); } #[tokio::test] @@ -1158,7 +1153,7 @@ async fn create_secret_rejects_under_scoped_daytona_api_key_and_leaves_vault_unc .set( EnvVars::DAYTONA_API_KEY, "existing", - SecretType::Environment, + SecretType::Token, None, ) .unwrap(); @@ -1172,7 +1167,7 @@ async fn create_secret_rejects_under_scoped_daytona_api_key_and_leaves_vault_unc serde_json::to_string(&serde_json::json!({ "name": EnvVars::DAYTONA_API_KEY, "value": "dtn_test", - "type": "environment" + "type": "token" })) .unwrap(), )) @@ -1222,7 +1217,7 @@ async fn diagnostics_reports_under_scoped_daytona_api_key() { .set( EnvVars::DAYTONA_API_KEY, "dtn_test", - SecretType::Environment, + SecretType::Token, None, ) .unwrap(); @@ -1257,7 +1252,7 @@ async fn diagnostics_reports_under_scoped_daytona_api_key() { } #[tokio::test] -async fn resolve_llm_client_reads_openai_codex_credential_from_vault() { +async fn resolve_llm_client_reads_openai_token_from_vault() { let state = test_app_state_with_env_lookup( default_test_server_settings(), RunLayer::default(), @@ -1269,9 +1264,9 @@ async fn resolve_llm_client_reads_openai_codex_credential_from_vault() { .write() .await .set( - "openai_codex", - &serde_json::to_string(&openai_api_key_credential("vault-openai-key")).unwrap(), - SecretType::Credential, + "OPENAI_API_KEY", + "vault-openai-key", + SecretType::Token, None, ) .unwrap(); @@ -1325,7 +1320,7 @@ async fn resolve_llm_client_from_source_preserves_credential_source_chain() { } #[tokio::test] -async fn llm_source_configured_providers_reads_openai_codex_from_vault() { +async fn llm_source_configured_providers_reads_openai_token_from_vault() { let state = test_app_state_with_env_lookup( default_test_server_settings(), RunLayer::default(), @@ -1337,9 +1332,9 @@ async fn llm_source_configured_providers_reads_openai_codex_from_vault() { .write() .await .set( - "openai_codex", - &serde_json::to_string(&openai_api_key_credential("vault-openai-key")).unwrap(), - SecretType::Credential, + "OPENAI_API_KEY", + "vault-openai-key", + SecretType::Token, None, ) .unwrap(); @@ -1382,9 +1377,9 @@ async fn resolve_llm_client_uses_env_lookup_for_openai_settings() { .write() .await .set( - "openai_codex", - &serde_json::to_string(&openai_api_key_credential("vault-openai-key")).unwrap(), - SecretType::Credential, + "OPENAI_API_KEY", + "vault-openai-key", + SecretType::Token, None, ) .unwrap(); @@ -1416,15 +1411,15 @@ async fn resolve_llm_client_uses_env_lookup_for_openai_settings() { } #[tokio::test] -async fn list_secrets_includes_credential_metadata() { +async fn list_secrets_includes_oauth_metadata() { let state = test_app_state(); { let mut vault = state.vault.write().await; vault .set( - "anthropic", - "{\"provider\":\"anthropic\"}", - SecretType::Credential, + "OPENAI_CODEX", + &openai_oauth_credential_json(), + SecretType::Oauth, Some("saved auth"), ) .unwrap(); @@ -1446,16 +1441,16 @@ async fn list_secrets_includes_credential_metadata() { let data = body["data"].as_array().expect("data should be an array"); let entry = data .iter() - .find(|entry| entry["name"] == "anthropic") - .expect("credential metadata should be listed"); - assert_eq!(entry["type"], "credential"); + .find(|entry| entry["name"] == "OPENAI_CODEX") + .expect("oauth metadata should be listed"); + assert_eq!(entry["type"], "oauth"); assert_eq!(entry["description"], "saved auth"); assert!(entry.get("updated_at").is_some()); assert!(entry.get("value").is_none()); } #[tokio::test] -async fn create_secret_rejects_invalid_credential_json() { +async fn create_secret_rejects_invalid_oauth_json() { let state = test_app_state(); let app = crate::test_support::build_test_router(state); @@ -1465,9 +1460,9 @@ async fn create_secret_rejects_invalid_credential_json() { .header("content-type", "application/json") .body(Body::from( serde_json::to_string(&serde_json::json!({ - "name": "openai_codex", + "name": "OPENAI_CODEX", "value": "{not-json", - "type": "credential" + "type": "oauth" })) .unwrap(), )) @@ -1478,7 +1473,7 @@ async fn create_secret_rejects_invalid_credential_json() { } #[tokio::test] -async fn create_secret_rejects_wrong_credential_name() { +async fn create_secret_rejects_invalid_oauth_name() { let state = test_app_state(); let app = crate::test_support::build_test_router(state); @@ -1488,26 +1483,9 @@ async fn create_secret_rejects_wrong_credential_name() { .header("content-type", "application/json") .body(Body::from( serde_json::to_string(&serde_json::json!({ - "name": "openai", - "value": serde_json::to_string(&serde_json::json!({ - "provider": "openai", - "type": "codex_oauth", - "tokens": { - "access_token": "access", - "refresh_token": "refresh", - "expires_at": "2030-01-01T00:00:00Z" - }, - "config": { - "auth_url": "https://auth.openai.com", - "token_url": "https://auth.openai.com/oauth/token", - "client_id": "client", - "scopes": ["openid"], - "redirect_uri": "https://auth.openai.com/deviceauth/callback", - "use_pkce": true - } - })) - .unwrap(), - "type": "credential" + "name": "1OPENAI", + "value": openai_oauth_credential_json(), + "type": "oauth" })) .unwrap(), )) @@ -2593,12 +2571,7 @@ methods = ["dev-token"] .vault .write() .await - .set( - "openai_codex", - &serde_json::to_string(&openai_api_key_credential("openai-key")).unwrap(), - SecretType::Credential, - None, - ) + .set("OPENAI_API_KEY", "openai-key", SecretType::Token, None) .unwrap(); let app = crate::test_support::build_test_router(Arc::clone(&state)); let create_response = app @@ -2762,12 +2735,7 @@ methods = ["dev-token"] .vault .write() .await - .set( - "openai_codex", - &serde_json::to_string(&openai_api_key_credential("openai-key")).unwrap(), - SecretType::Credential, - None, - ) + .set("OPENAI_API_KEY", "openai-key", SecretType::Token, None) .unwrap(); let app = crate::test_support::build_test_router(Arc::clone(&state)); let create_response = app @@ -4523,7 +4491,7 @@ fn create_github_token_app_state_with_env_lookup_and_llm_catalog_settings( .vault .try_write() .expect("test vault should not already be locked") - .set("GITHUB_TOKEN", token, SecretType::Credential, None) + .set("GITHUB_TOKEN", token, SecretType::Token, None) .expect("test github token should be writable"); } state @@ -6210,12 +6178,7 @@ async fn create_run_pull_request_creates_and_persists_record() { .vault .write() .await - .set( - "openai_codex", - &serde_json::to_string(&openai_api_key_credential("openai-key")).unwrap(), - SecretType::Credential, - None, - ) + .set("OPENAI_API_KEY", "openai-key", SecretType::Token, None) .unwrap(); let app = crate::test_support::build_test_router(Arc::clone(&state)); let run_id = fixtures::RUN_1; diff --git a/lib/crates/fabro-server/tests/it/api/install.rs b/lib/crates/fabro-server/tests/it/api/install.rs index 553fa2a34..b2fe734ab 100644 --- a/lib/crates/fabro-server/tests/it/api/install.rs +++ b/lib/crates/fabro-server/tests/it/api/install.rs @@ -941,7 +941,7 @@ async fn token_install_finish_persists_settings_env_and_vault() { assert!(!server_env.contains("AWS_SECRET_ACCESS_KEY=")); let vault = Vault::load(fabro_config::Storage::new(temp_dir.path()).secrets_path()).unwrap(); - assert!(vault.get("anthropic").is_some()); + assert!(vault.get("ANTHROPIC_API_KEY").is_some()); assert_eq!(vault.get("GITHUB_TOKEN"), Some("ghp_test_token")); } @@ -1028,8 +1028,8 @@ async fn browser_install_finish_with_skipped_llm_persists_no_llm_credentials() { let vault = Vault::load(fabro_config::Storage::new(temp_dir.path()).secrets_path()).unwrap(); assert!( - vault.credential_entries().is_empty(), - "skipped LLM install should not write any credential vault entries" + vault.get("OPENAI_API_KEY").is_none() && vault.get("OPENAI_CODEX").is_none(), + "skipped LLM install should not write any OpenAI vault entries" ); assert_eq!( vault.get("GITHUB_TOKEN"), diff --git a/lib/crates/fabro-types/src/secret.rs b/lib/crates/fabro-types/src/secret.rs index b6edfc956..9ba64eafe 100644 --- a/lib/crates/fabro-types/src/secret.rs +++ b/lib/crates/fabro-types/src/secret.rs @@ -6,10 +6,13 @@ use strum::Display; #[serde(rename_all = "snake_case")] #[strum(serialize_all = "snake_case")] pub enum SecretType { + /// Opaque API-key/PAT-style token value. #[default] - Environment, + Token, + /// JSON-encoded OAuth credential. Refreshable; never projected into env. + Oauth, + /// Path-shaped secret materialized to the filesystem. File, - Credential, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -22,3 +25,29 @@ pub struct SecretMetadata { pub created_at: DateTime, pub updated_at: DateTime, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn secret_type_serializes_to_snake_case() { + assert_eq!( + serde_json::to_string(&SecretType::Token).unwrap(), + "\"token\"" + ); + assert_eq!( + serde_json::to_string(&SecretType::Oauth).unwrap(), + "\"oauth\"" + ); + assert_eq!( + serde_json::to_string(&SecretType::File).unwrap(), + "\"file\"" + ); + } + + #[test] + fn secret_type_default_is_token() { + assert_eq!(SecretType::default(), SecretType::Token); + } +} diff --git a/lib/crates/fabro-vault/src/lib.rs b/lib/crates/fabro-vault/src/lib.rs index a8c8f9561..d9256ea04 100644 --- a/lib/crates/fabro-vault/src/lib.rs +++ b/lib/crates/fabro-vault/src/lib.rs @@ -144,25 +144,6 @@ impl Vault { self.entries.get(name) } - pub fn snapshot(&self) -> HashMap { - self.entries - .iter() - .filter(|(_, entry)| entry.secret_type == SecretType::Environment) - .map(|(name, entry)| (name.clone(), entry.value.clone())) - .collect() - } - - pub fn credential_entries(&self) -> Vec<(&str, &SecretEntry)> { - let mut data = self - .entries - .iter() - .filter(|(_, entry)| entry.secret_type == SecretType::Credential) - .map(|(name, entry)| (name.as_str(), entry)) - .collect::>(); - data.sort_by(|a, b| a.0.cmp(b.0)); - data - } - pub fn file_secrets(&self) -> Vec<(String, String)> { let mut data = self .entries @@ -176,7 +157,7 @@ impl Vault { pub fn validate_name(name: &str, secret_type: SecretType) -> Result<(), Error> { match secret_type { - SecretType::Environment | SecretType::Credential => Self::validate_env_name(name), + SecretType::Token | SecretType::Oauth => Self::validate_env_name(name), SecretType::File => Self::validate_file_name(name), } } @@ -282,11 +263,11 @@ mod tests { let mut store = Vault::load(path.clone()).unwrap(); let meta = store - .set("OPENAI_API_KEY", "secret", SecretType::Environment, None) + .set("OPENAI_API_KEY", "secret", SecretType::Token, None) .unwrap(); assert_eq!(meta.name, "OPENAI_API_KEY"); - assert_eq!(meta.secret_type, SecretType::Environment); + assert_eq!(meta.secret_type, SecretType::Token); assert_eq!(store.get("OPENAI_API_KEY"), Some("secret")); assert!(path.exists()); } @@ -298,10 +279,10 @@ mod tests { let mut store = Vault::load(path).unwrap(); store - .set("OPENAI_API_KEY", "first", SecretType::Environment, None) + .set("OPENAI_API_KEY", "first", SecretType::Token, None) .unwrap(); store - .set("OPENAI_API_KEY", "second", SecretType::Environment, None) + .set("OPENAI_API_KEY", "second", SecretType::Token, None) .unwrap(); assert_eq!(store.get("OPENAI_API_KEY"), Some("second")); @@ -313,7 +294,7 @@ mod tests { let path = dir.path().join("secrets.json"); let mut store = Vault::load(path.clone()).unwrap(); store - .set("OPENAI_API_KEY", "secret", SecretType::Environment, None) + .set("OPENAI_API_KEY", "secret", SecretType::Token, None) .unwrap(); store.remove("OPENAI_API_KEY").unwrap(); @@ -322,19 +303,23 @@ mod tests { } #[test] - fn env_secret_snapshot_excludes_file_secrets() { + fn file_secrets_excludes_token_and_oauth_secrets() { let dir = tempfile::tempdir().unwrap(); let mut store = Vault::load(dir.path().join("secrets.json")).unwrap(); store - .set("OPENAI_API_KEY", "env", SecretType::Environment, None) + .set("OPENAI_API_KEY", "token", SecretType::Token, None) + .unwrap(); + store + .set("OPENAI_CODEX", "oauth-json", SecretType::Oauth, None) .unwrap(); store .set("/tmp/key.pem", "pem", SecretType::File, None) .unwrap(); - let snapshot = store.snapshot(); - assert_eq!(snapshot.get("OPENAI_API_KEY"), Some(&"env".to_string())); - assert!(!snapshot.contains_key("/tmp/key.pem")); + assert_eq!(store.file_secrets(), vec![( + "/tmp/key.pem".to_string(), + "pem".to_string() + )]); } #[test] @@ -354,21 +339,21 @@ mod tests { } #[test] - fn list_includes_credential_entries_loaded_from_disk() { + fn list_includes_schema_typed_entries_loaded_from_disk() { let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("secrets.json"); std::fs::write( &path, serde_json::json!({ "OPENAI_API_KEY": { - "value": "env", - "type": "environment", + "value": "token", + "type": "token", "created_at": "2026-04-12T00:00:00Z", "updated_at": "2026-04-12T00:00:00Z" }, - "openai_codex": { - "value": "{\"provider\":\"openai\"}", - "type": "credential", + "OPENAI_CODEX": { + "value": "{\"tokens\":{\"access_token\":\"access\",\"refresh_token\":\"refresh\",\"expires_at\":\"2026-04-12T01:00:00Z\"},\"config\":{\"auth_url\":\"https://auth.openai.com\",\"token_url\":\"https://auth.openai.com/oauth/token\",\"client_id\":\"client\",\"scopes\":[\"openid\"],\"redirect_uri\":null,\"use_pkce\":true}}", + "type": "oauth", "created_at": "2026-04-12T00:00:00Z", "updated_at": "2026-04-12T00:00:00Z" } @@ -379,11 +364,14 @@ mod tests { let store = Vault::load(path).unwrap(); - assert_eq!(store.list().len(), 2); - assert_eq!(store.list()[0].name, "OPENAI_API_KEY"); - assert_eq!(store.list()[1].name, "openai_codex"); - assert_eq!(store.list()[1].secret_type, SecretType::Credential); - assert_eq!(store.get("openai_codex"), Some("{\"provider\":\"openai\"}")); + let list = store.list(); + assert_eq!(list.len(), 2); + assert_eq!(list[0].name, "OPENAI_API_KEY"); + assert_eq!(list[0].secret_type, SecretType::Token); + assert_eq!(list[1].name, "OPENAI_CODEX"); + assert_eq!(list[1].secret_type, SecretType::Oauth); + assert_eq!(store.get("OPENAI_API_KEY"), Some("token")); + assert!(store.get("OPENAI_CODEX").is_some()); } #[test] @@ -392,38 +380,30 @@ mod tests { let mut store = Vault::load(dir.path().join("secrets.json")).unwrap(); store .set( - "openai_codex", - "credential-json", - SecretType::Credential, + "OPENAI_CODEX", + "oauth-json", + SecretType::Oauth, Some("saved auth"), ) .unwrap(); - let entry = store.get_entry("openai_codex").unwrap(); + let entry = store.get_entry("OPENAI_CODEX").unwrap(); - assert_eq!(entry.value, "credential-json"); - assert_eq!(entry.secret_type, SecretType::Credential); + assert_eq!(entry.value, "oauth-json"); + assert_eq!(entry.secret_type, SecretType::Oauth); assert_eq!(entry.description.as_deref(), Some("saved auth")); } #[test] - fn credential_entries_only_returns_credentials() { + fn get_entry_returns_token_entries_by_name() { let dir = tempfile::tempdir().unwrap(); let mut store = Vault::load(dir.path().join("secrets.json")).unwrap(); store - .set("OPENAI_API_KEY", "env", SecretType::Environment, None) - .unwrap(); - store - .set( - "openai_codex", - "credential-json", - SecretType::Credential, - None, - ) + .set("OPENAI_API_KEY", "token", SecretType::Token, None) .unwrap(); - assert_eq!(store.credential_entries().len(), 1); - assert_eq!(store.credential_entries()[0].0, "openai_codex"); - assert_eq!(store.credential_entries()[0].1.value, "credential-json"); + let entry = store.get_entry("OPENAI_API_KEY").unwrap(); + assert_eq!(entry.value, "token"); + assert_eq!(entry.secret_type, SecretType::Token); } } diff --git a/lib/crates/fabro-workflow/src/handler/llm/api.rs b/lib/crates/fabro-workflow/src/handler/llm/api.rs index 784708dd8..2473922f2 100644 --- a/lib/crates/fabro-workflow/src/handler/llm/api.rs +++ b/lib/crates/fabro-workflow/src/handler/llm/api.rs @@ -1148,7 +1148,7 @@ impl CompletionCoordinator for SteeringCompletionCoordinator { mod tests { use fabro_agent::subagent::SessionFactory; use fabro_agent::{AgentProfile, ToolRegistry}; - use fabro_auth::{AuthCredential, AuthDetails, EnvCredentialSource, VaultCredentialSource}; + use fabro_auth::{EnvCredentialSource, VaultCredentialSource}; use fabro_llm::provider::{ProviderAdapter, StreamEventStream}; use fabro_llm::{Error as LlmError, ProviderErrorDetail, ProviderErrorKind}; use fabro_vault::{SecretType, Vault}; @@ -1534,15 +1534,9 @@ reasoning = false let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); vault .set( - "anthropic", - &serde_json::to_string(&AuthCredential { - provider: ProviderId::anthropic(), - details: AuthDetails::ApiKey { - key: "anthropic-key".to_string(), - }, - }) - .unwrap(), - SecretType::Credential, + "ANTHROPIC_API_KEY", + "anthropic-key", + SecretType::Token, None, ) .unwrap(); diff --git a/lib/crates/fabro-workflow/src/pipeline/initialize.rs b/lib/crates/fabro-workflow/src/pipeline/initialize.rs index 74b5419bf..f1560d5dc 100644 --- a/lib/crates/fabro-workflow/src/pipeline/initialize.rs +++ b/lib/crates/fabro-workflow/src/pipeline/initialize.rs @@ -717,7 +717,6 @@ mod tests { use std::time::Duration; use fabro_acp::test_support::fake_acp_agent_script; - use fabro_auth::{AuthCredential, AuthDetails}; use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; use fabro_interview::AutoApproveInterviewer; use fabro_sandbox::SandboxSpec; @@ -1017,15 +1016,9 @@ mod tests { let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); vault .set( - "anthropic", - &serde_json::to_string(&AuthCredential { - provider: fabro_model::ProviderId::anthropic(), - details: AuthDetails::ApiKey { - key: "anthropic-key".to_string(), - }, - }) - .unwrap(), - SecretType::Credential, + "ANTHROPIC_API_KEY", + "anthropic-key", + SecretType::Token, None, ) .unwrap(); @@ -1124,18 +1117,7 @@ mod tests { let mut vault = Vault::load(temp.path().join("secrets.json")).unwrap(); vault - .set( - "openai", - &serde_json::to_string(&AuthCredential { - provider: fabro_model::ProviderId::openai(), - details: AuthDetails::ApiKey { - key: "openai-key".to_string(), - }, - }) - .unwrap(), - SecretType::Credential, - None, - ) + .set("OPENAI_API_KEY", "openai-key", SecretType::Token, None) .unwrap(); let vault = Arc::new(AsyncRwLock::new(vault)); diff --git a/lib/crates/fabro-workflow/src/pipeline/pull_request.rs b/lib/crates/fabro-workflow/src/pipeline/pull_request.rs index c178ce047..2f2518095 100644 --- a/lib/crates/fabro-workflow/src/pipeline/pull_request.rs +++ b/lib/crates/fabro-workflow/src/pipeline/pull_request.rs @@ -670,15 +670,12 @@ mod tests { use std::time::Duration; use chrono::Utc; - use fabro_auth::{ - AuthCredential, AuthDetails, CredentialSource, EnvCredentialSource, VaultCredentialSource, - }; + use fabro_auth::{CredentialSource, EnvCredentialSource, VaultCredentialSource}; use fabro_graphviz::graph::Graph; use fabro_llm::Error as LlmError; use fabro_llm::client::Client; use fabro_llm::provider::{ProviderAdapter, StreamEventStream}; use fabro_llm::types::{FinishReason, Message, Request, Response, StreamEvent, TokenCounts}; - use fabro_model::ProviderId; use fabro_model::catalog::{LlmCatalogSettings, ProviderCatalogSettings}; use fabro_store::Database; use fabro_types::{ @@ -835,15 +832,6 @@ mod tests { ) } - fn openai_api_key_credential(key: &str) -> AuthCredential { - AuthCredential { - provider: ProviderId::openai(), - details: AuthDetails::ApiKey { - key: key.to_string(), - }, - } - } - fn openai_responses_payload(text: &str) -> serde_json::Value { serde_json::json!({ "id": "resp_1", @@ -1344,9 +1332,9 @@ mod tests { let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap(); vault .set( - "openai_codex", - &serde_json::to_string(&openai_api_key_credential("vault-openai-key")).unwrap(), - SecretType::Credential, + "OPENAI_API_KEY", + "vault-openai-key", + SecretType::Token, None, ) .unwrap(); @@ -1840,9 +1828,9 @@ mod tests { let mut vault = Vault::load(vault_dir.path().join("secrets.json")).unwrap(); vault .set( - "openai_codex", - &serde_json::to_string(&openai_api_key_credential("vault-openai-key")).unwrap(), - SecretType::Credential, + "OPENAI_API_KEY", + "vault-openai-key", + SecretType::Token, None, ) .unwrap(); diff --git a/lib/crates/fabro-workflow/tests/it/integration.rs b/lib/crates/fabro-workflow/tests/it/integration.rs index be39fa3b3..36fff6272 100644 --- a/lib/crates/fabro-workflow/tests/it/integration.rs +++ b/lib/crates/fabro-workflow/tests/it/integration.rs @@ -6869,15 +6869,6 @@ mod real_llm { } } -fn openai_api_key_credential(key: &str) -> fabro_auth::AuthCredential { - fabro_auth::AuthCredential { - provider: ProviderId::openai(), - details: fabro_auth::AuthDetails::ApiKey { - key: key.to_string(), - }, - } -} - fn openai_responses_payload(text: &str) -> serde_json::Value { serde_json::json!({ "id": "resp_1", @@ -6960,9 +6951,9 @@ async fn workflow_run_with_vault_only_openai_codex_builds_pr_body() { let mut vault = Vault::load(vault_dir.path().join("secrets.json")).unwrap(); vault .set( - "openai_codex", - &serde_json::to_string(&openai_api_key_credential("vault-openai-key")).unwrap(), - SecretType::Credential, + "OPENAI_API_KEY", + "vault-openai-key", + SecretType::Token, None, ) .unwrap(); diff --git a/lib/packages/fabro-api-client/src/models/reasoning-effort-feature.ts b/lib/packages/fabro-api-client/src/models/reasoning-effort-feature.ts index b3c303299..33270e2db 100644 --- a/lib/packages/fabro-api-client/src/models/reasoning-effort-feature.ts +++ b/lib/packages/fabro-api-client/src/models/reasoning-effort-feature.ts @@ -15,7 +15,7 @@ /** - * Whether Fabro may expose reasoning effort levels for a model. + * Whether the model endpoint supports a native reasoning-effort parameter. */ export const ReasoningEffortFeature = { diff --git a/lib/packages/fabro-api-client/src/models/secret-type.ts b/lib/packages/fabro-api-client/src/models/secret-type.ts index b85844891..7359ac8aa 100644 --- a/lib/packages/fabro-api-client/src/models/secret-type.ts +++ b/lib/packages/fabro-api-client/src/models/secret-type.ts @@ -15,13 +15,13 @@ /** - * The way a secret is consumed by the sandbox. + * Schema of a stored secret. */ export const SecretType = { - ENVIRONMENT: 'environment', - FILE: 'file', - CREDENTIAL: 'credential' + TOKEN: 'token', + OAUTH: 'oauth', + FILE: 'file' } as const; export type SecretType = typeof SecretType[keyof typeof SecretType];