From c5ddaec71e39ece21a50de977c996d3ca7a0eb06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Tue, 21 Jul 2026 01:18:46 -0400 Subject: [PATCH] Add per-provider auth.json keys; make auth methods registry-generic (P0b) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Platform API keys now live in auth.json under the platform-id scope (the per-provider auth.json key contract), resolved env > auth.json > legacy [platforms.*] config.toml (read-only fallback). The TUI login picker, paste box, auth-method advertising, and authenticate handler are all registry-generic: a new PlatformSpec row appears in the login UI and authenticates with zero UI changes. Spec rows gained vendor/console_host/ login_label display fields (moonshot strings byte-identical, pinned by tests). Adversarial review caught that auth.json keys were validated at login but never stamped onto catalog entries (completions would 401; restart lost eager auth). Fixed red-green: resolve_model_list/resolve_model_catalog now take a resolved PlatformApiKeys snapshot consumed by the credential- stamping layer (auth.json beats stale config.toml, matching the login validator), with production callers resolving fresh per catalog build. Also from review: the new auth.json writer takes the manager's cross- process flock (bounded retry — an unlocked RMW racing a token refresh could revert a rotated refresh token); the oauth-401 wiremock test is hermetic (KIGI_SHARE_DIR tempdir; it could read a dev's real auth.json and hit live moonshot); cli_models resolves real keys; auth.json is read once per registry sweep; caller-less lock_config_writes deleted; catalog resolvers tightened to pub(crate); stale config.toml doc comments and the no-credentials error copy updated. --- AGENTS.md | 16 + crates/codegen/kigi-models/src/lib.rs | 69 ++- .../kigi-shell/src/agent/auth_method.rs | 136 +++--- crates/codegen/kigi-shell/src/agent/config.rs | 440 ++++++++++-------- .../src/agent/config_model_override_parse.rs | 2 +- crates/codegen/kigi-shell/src/agent/models.rs | 130 ++++-- .../kigi-shell/src/agent/models_fetch.rs | 5 +- .../src/agent/mvp_agent/acp_agent.rs | 30 +- .../src/agent/mvp_agent/agent_ops.rs | 30 +- crates/codegen/kigi-shell/src/auth/manager.rs | 1 + crates/codegen/kigi-shell/src/auth/mod.rs | 3 +- crates/codegen/kigi-shell/src/auth/storage.rs | 145 +++++- crates/codegen/kigi-shell/src/cli_models.rs | 6 +- .../kigi-shell/src/util/config/persist.rs | 7 - crates/codegen/kigi-tui/src/app/actions.rs | 8 +- crates/codegen/kigi-tui/src/app/app_view.rs | 52 +-- .../codegen/kigi-tui/src/app/dispatch/auth.rs | 2 +- .../kigi-tui/src/app/dispatch/tests/auth.rs | 15 +- .../codegen/kigi-tui/src/views/welcome/mod.rs | 9 +- 19 files changed, 731 insertions(+), 375 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d2a543c..fe268ca 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -150,6 +150,22 @@ edges stay deterministic Rust. The harness appends a terminal the replan cap; `{"ops": []}` is a respected free no-op; failures degrade. +## Provider registry & API-key auth (post-0.1.3 expansion) + +- The platform registry is compiled-in spec rows in `kigi-models` + (`PlatformSpec`; adding a platform = enum variant + `ALL` entry + `spec()` + arm + row; registry tests enforce completeness/uniqueness/row shape). +- API-key resolution precedence, per platform: platform env var(s) > + `auth.json` scope named by the platform id (`moonshot-cn`, …) > + legacy `[platforms.]` in config.toml (read-only fallback). +- The TUI login picker persists pasted keys to `auth.json` (platform-id + scope, `api_key` mode) — never to config.toml. The keyring holds ONLY the + OAuth session scope; platform keys are file-only. +- Auth method ids over ACP equal the platform ids; interactive picker rows + are built generically from advertised methods (`AuthMethodKind:: + ApiKeyPlatform`), so new registry rows appear in the picker with no TUI + changes. + ## Milestones (PRD §8.3) - M0 (done): rename, deletions (voice/telemetry/announcements/marketplace/ diff --git a/crates/codegen/kigi-models/src/lib.rs b/crates/codegen/kigi-models/src/lib.rs index 3bf3a20..6d7ab96 100644 --- a/crates/codegen/kigi-models/src/lib.rs +++ b/crates/codegen/kigi-models/src/lib.rs @@ -60,7 +60,8 @@ enum BaseUrlSource { /// `all_covers_every_variant` test — a variant missing from `ALL` would /// otherwise be silently unparseable and excluded from model sync. struct PlatformSpec { - /// Wire id (auth method id, managed-model-key prefix, config key). + /// Wire id (auth method id, managed-model-key prefix, config key, and — + /// for API-key platforms — the auth.json scope the key is stored under). id: &'static str, display_name: &'static str, base_url: BaseUrlSource, @@ -74,6 +75,13 @@ struct PlatformSpec { /// /// SECURITY: the *values* behind these names must never be logged. api_key_envs: &'static [&'static str], + /// Short vendor word for login copy ("Paste your {vendor} API key"). + vendor: &'static str, + /// Where the user gets an API key (login copy + key-validation errors). + /// `None` for OAuth channels. + console_host: Option<&'static str>, + /// Interactive login-picker label. `None` = fall back to `display_name`. + login_label: Option<&'static str>, } const KIMI_CODE_SPEC: PlatformSpec = PlatformSpec { @@ -83,6 +91,9 @@ const KIMI_CODE_SPEC: PlatformSpec = PlatformSpec { uses_oauth: true, allowed_model_prefixes: None, api_key_envs: &[], + vendor: "Kimi", + console_host: None, + login_label: None, }; const MOONSHOT_CN_SPEC: PlatformSpec = PlatformSpec { @@ -95,6 +106,9 @@ const MOONSHOT_CN_SPEC: PlatformSpec = PlatformSpec { uses_oauth: false, allowed_model_prefixes: Some(&["kimi-k"]), api_key_envs: &[MOONSHOT_CN_API_KEY_ENV, MOONSHOT_API_KEY_ENV], + vendor: "Moonshot", + console_host: Some("platform.moonshot.cn"), + login_label: Some("Moonshot Open Platform (API key \u{b7} moonshot.cn)"), }; const MOONSHOT_AI_SPEC: PlatformSpec = PlatformSpec { @@ -107,6 +121,9 @@ const MOONSHOT_AI_SPEC: PlatformSpec = PlatformSpec { uses_oauth: false, allowed_model_prefixes: Some(&["kimi-k"]), api_key_envs: &[MOONSHOT_AI_API_KEY_ENV, MOONSHOT_API_KEY_ENV], + vendor: "Moonshot", + console_host: Some("platform.moonshot.ai"), + login_label: Some("Moonshot Open Platform (API key \u{b7} moonshot.ai)"), }; /// The platform registry. Platforms are compiled-in spec rows; there is no @@ -183,6 +200,23 @@ impl PlatformId { pub fn managed_model_key(self, model_id: &str) -> String { format!("{}/{model_id}", self.as_str()) } + + /// Short vendor word for login copy ("Paste your {vendor} API key"). + pub fn vendor(self) -> &'static str { + self.spec().vendor + } + + /// Console host where the user obtains an API key, for login copy and + /// key-validation errors. `None` for OAuth channels. + pub fn console_host(self) -> Option<&'static str> { + self.spec().console_host + } + + /// Label for the interactive login picker (falls back to the display + /// name when the row doesn't override it). + pub fn login_label(self) -> &'static str { + self.spec().login_label.unwrap_or(self.spec().display_name) + } } /// Split a managed catalog key `{platform_id}/{model_id}` back into its @@ -531,6 +565,39 @@ mod tests { ); } + /// Row-shape invariants the login UI and key resolution rely on: + /// API-key platforms carry a console host (paste-box copy) and at least + /// one key env var (missing-key error names it); OAuth channels carry + /// neither key envs nor a console host requirement. + #[test] + fn api_key_rows_carry_console_host_and_env_names() { + for p in PlatformId::ALL { + if p.uses_oauth() { + assert!( + p.api_key_env_names().is_empty(), + "{}: OAuth platforms take no key envs", + p.as_str() + ); + } else { + assert!( + p.console_host().is_some(), + "{}: API-key platforms must name their console host", + p.as_str() + ); + assert!( + !p.api_key_env_names().is_empty(), + "{}: API-key platforms must name at least one key env", + p.as_str() + ); + assert!( + !p.vendor().is_empty(), + "{}: API-key platforms must set a vendor word", + p.as_str() + ); + } + } + } + /// `parse` resolves by scanning spec rows, so duplicate ids would /// silently shadow a platform. Pin uniqueness as rows are added. #[test] diff --git a/crates/codegen/kigi-shell/src/agent/auth_method.rs b/crates/codegen/kigi-shell/src/agent/auth_method.rs index e7d5ad6..23a7935 100644 --- a/crates/codegen/kigi-shell/src/agent/auth_method.rs +++ b/crates/codegen/kigi-shell/src/agent/auth_method.rs @@ -102,13 +102,13 @@ pub struct BuiltAuthMethods { /// 1. `xai.api_key` (if `has_external_api_key`) /// 2. `cached_token` (if `has_cached_token`) /// 3. `kimi-code` (the Kimi Code device login) -/// 4. `moonshot-cn` (Moonshot Open Platform API-key login, always) -/// 5. `moonshot-ai` (Moonshot Open Platform API-key login, always) +/// 4. every API-key registry platform, in `PlatformId::ALL` order +/// (`moonshot-cn`, `moonshot-ai`, …), always advertised /// -/// The moonshot methods are for the INTERACTIVE login picker only: they come +/// The platform methods are for the INTERACTIVE login picker only: they come /// after `kimi-code` so they can never become `auth_methods.first()` (the /// pager's startup metadata / eager-auth fallback reads `first()`), and they -/// are never the `default_auth_method_id` (a configured moonshot key already +/// are never the `default_auth_method_id` (a configured platform key already /// authenticates eagerly via `xai.api_key` — the catalog entries it stamps /// satisfy `should_advertise_xai_api_key`). /// @@ -150,8 +150,11 @@ pub fn build_auth_methods(inputs: AuthMethodsBuildInputs<'_>) -> BuiltAuthMethod } methods.push(kimi_code_auth_method(login_label)); - methods.push(moonshot_auth_method(kigi_models::PlatformId::MoonshotCn)); - methods.push(moonshot_auth_method(kigi_models::PlatformId::MoonshotAi)); + for platform in kigi_models::PlatformId::ALL { + if !platform.uses_oauth() { + methods.push(platform_auth_method(platform)); + } + } BuiltAuthMethods { methods, @@ -165,10 +168,8 @@ pub enum AuthMethodKind { XaiApiKey, CachedToken, KimiCode, - /// Moonshot Open Platform API-key login (moonshot.cn). - MoonshotCn, - /// Moonshot Open Platform API-key login (moonshot.ai). - MoonshotAi, + /// Registry API-key platform login (method id = the platform id). + ApiKeyPlatform(kigi_models::PlatformId), Unknown, } @@ -178,17 +179,18 @@ impl AuthMethodKind { XAI_API_KEY_METHOD_ID => Self::XaiApiKey, CACHED_TOKEN_AUTH_METHOD_ID => Self::CachedToken, KIMI_CODE_METHOD_ID => Self::KimiCode, - MOONSHOT_CN_METHOD_ID => Self::MoonshotCn, - MOONSHOT_AI_METHOD_ID => Self::MoonshotAi, - _ => Self::Unknown, + other => match platform_for_method_id_str(other) { + Some(platform) => Self::ApiKeyPlatform(platform), + None => Self::Unknown, + }, } } /// API key auth: no auth.json session, no refresh, no browser round-trip. - /// The moonshot methods qualify — they validate a configured platform key - /// and then behave exactly like an external-API-key session. + /// The registry platform methods qualify — they validate a configured + /// platform key and then behave exactly like an external-API-key session. pub fn is_api_key(self) -> bool { - matches!(self, Self::XaiApiKey | Self::MoonshotCn | Self::MoonshotAi) + matches!(self, Self::XaiApiKey | Self::ApiKeyPlatform(_)) } /// `true` for session-based methods (cached_token, interactive login). @@ -323,65 +325,56 @@ pub fn kimi_code_auth_method(label: Option<&str>) -> acp::AuthMethod { ) } -/// Interactive API-key login for the Moonshot open platforms. Method ids -/// equal [`kigi_models::PlatformId::as_str`] (`moonshot-cn` / `moonshot-ai`), -/// which is also the `[platforms.]` config-table name — one id everywhere. +/// Interactive API-key login method ids equal +/// [`kigi_models::PlatformId::as_str`] (`moonshot-cn` / `moonshot-ai` / …), +/// which is also the `[platforms.]` config-table name and the auth.json +/// scope — one id everywhere. pub const MOONSHOT_CN_METHOD_ID: &str = "moonshot-cn"; pub const MOONSHOT_AI_METHOD_ID: &str = "moonshot-ai"; -/// The open platform behind an interactive moonshot method id. `None` for -/// every other id (including `kimi-code`, whose platform uses OAuth). -pub fn moonshot_platform_for_method_id(id: &acp::AuthMethodId) -> Option { - match id.0.as_ref() { - MOONSHOT_CN_METHOD_ID => Some(kigi_models::PlatformId::MoonshotCn), - MOONSHOT_AI_METHOD_ID => Some(kigi_models::PlatformId::MoonshotAi), - _ => None, - } +/// The API-key registry platform behind an interactive method id. `None` +/// for every other id (including `kimi-code`, whose platform uses OAuth). +pub fn platform_for_method_id(id: &acp::AuthMethodId) -> Option { + platform_for_method_id_str(id.0.as_ref()) } -/// Console host for an open platform, used in method descriptions and login -/// copy ("platform.moonshot.cn" / "platform.moonshot.ai"). -pub fn moonshot_console_host(platform: kigi_models::PlatformId) -> &'static str { - match platform { - kigi_models::PlatformId::MoonshotCn => "platform.moonshot.cn", - _ => "platform.moonshot.ai", - } +fn platform_for_method_id_str(id: &str) -> Option { + kigi_models::PlatformId::parse(id).filter(|p| !p.uses_oauth()) } -/// A Moonshot Open Platform API-key login method. -pub fn moonshot_auth_method(platform: kigi_models::PlatformId) -> acp::AuthMethod { - let host_suffix = match platform { - kigi_models::PlatformId::MoonshotCn => "moonshot.cn", - _ => "moonshot.ai", +/// An API-key registry platform's login method (picker label + description +/// from the platform's spec row). +pub fn platform_auth_method(platform: kigi_models::PlatformId) -> acp::AuthMethod { + let description = match platform.console_host() { + Some(host) => format!("API key from {host}"), + None => format!("API key for {}", platform.display_name()), }; acp::AuthMethod::Agent( acp::AuthMethodAgent::new( acp::AuthMethodId::new(platform.as_str()), - format!("Moonshot Open Platform (API key \u{b7} {host_suffix})"), + platform.login_label().to_string(), ) - .description(Some(format!( - "API key from {}", - moonshot_console_host(platform) - ))), + .description(Some(description)), ) } -/// Actionable error for a moonshot `authenticate` with no key configured. -pub fn missing_moonshot_key_error(platform: kigi_models::PlatformId) -> String { - let env_var = platform - .api_key_env_names() - .first() - .copied() - .unwrap_or(kigi_models::MOONSHOT_API_KEY_ENV); - format!( - "No API key configured for {} \u{2014} paste one in the login screen or set {env_var}", - platform.as_str(), - ) +/// Actionable error for a platform `authenticate` with no key configured. +pub fn missing_platform_key_error(platform: kigi_models::PlatformId) -> String { + match platform.api_key_env_names().first() { + Some(env_var) => format!( + "No API key configured for {} \u{2014} paste one in the login screen or set {env_var}", + platform.as_str(), + ), + None => format!( + "No API key configured for {} \u{2014} paste one in the login screen", + platform.as_str(), + ), + } } -/// Validate + accept a Moonshot open-platform API key for `authenticate`. +/// Validate + accept an API-key platform's key for `authenticate`. /// -/// `key` is the caller-resolved credential (env > config; see +/// `key` is the caller-resolved credential (env > auth.json > config; see /// `resolve_platform_api_key`) — `None` fails with the actionable /// missing-key message. A present key is validated with /// `GET {platform_base}/models` (the same endpoint the catalog fetch uses): @@ -398,7 +391,7 @@ pub(crate) async fn authenticate_platform_api_key( err }; let Some(key) = key else { - return Err(auth_err(missing_moonshot_key_error(platform))); + return Err(auth_err(missing_platform_key_error(platform))); }; let url = format!("{}/models", platform.base_url().trim_end_matches('/')); let response = crate::http::shared_client() @@ -412,7 +405,7 @@ pub(crate) async fn authenticate_platform_api_key( return Err(auth_err(format!( "Invalid API key for {} \u{2014} check your key on {}", platform.as_str(), - moonshot_console_host(platform), + platform.console_host().unwrap_or("the provider console"), ))); } if !status.is_success() { @@ -464,10 +457,14 @@ mod tests { assert!(api.is_api_key()); assert!(!api.is_session_based()); assert!(!api.needs_interactive_login()); - // Moonshot methods are API-key shaped: NOT session-based (no token - // refresh may ever run for them) and no browser round-trip. + // Registry platform methods are API-key shaped: NOT session-based (no + // token refresh may ever run for them) and no browser round-trip. for id in [MOONSHOT_CN_METHOD_ID, MOONSHOT_AI_METHOD_ID] { let kind = AuthMethodKind::from_id(&acp::AuthMethodId::new(id)); + assert!( + matches!(kind, AuthMethodKind::ApiKeyPlatform(p) if p.as_str() == id), + "{id} must classify as its ApiKeyPlatform" + ); assert!(kind.is_api_key(), "{id} must classify as api-key"); assert!(!kind.is_session_based(), "{id} must not be session-based"); assert!( @@ -493,6 +490,21 @@ mod tests { ); } + /// The OAuth platform id must never resolve as an API-key platform + /// method — `platform_for_method_id`'s `uses_oauth` filter is what keeps + /// the generic `authenticate` arm from hijacking the device login. + #[test] + fn oauth_platform_id_is_not_an_api_key_method() { + assert_eq!( + platform_for_method_id(&acp::AuthMethodId::new(KIMI_CODE_METHOD_ID)), + None + ); + assert_eq!( + AuthMethodKind::from_id(&acp::AuthMethodId::new(KIMI_CODE_METHOD_ID)), + AuthMethodKind::KimiCode + ); + } + #[test] fn session_token_auth_gate_matrix() { // Session method + NotByok → refresh. @@ -673,7 +685,7 @@ mod tests { fn global_external_api_key_advertises_xai_api_key_first() { let _set = EnvGuard::set(XAI_API_KEY_ENV_VAR, "xai-external-key"); let cfg = Config::default(); - let models = resolve_model_list(&cfg, None); + let models = resolve_model_list(&cfg, None, &Default::default()); let has_external_api_key = should_advertise_xai_api_key(models.values()); assert!(has_external_api_key); let built = build_auth_methods(AuthMethodsBuildInputs { diff --git a/crates/codegen/kigi-shell/src/agent/config.rs b/crates/codegen/kigi-shell/src/agent/config.rs index 2bad164..60b2885 100644 --- a/crates/codegen/kigi-shell/src/agent/config.rs +++ b/crates/codegen/kigi-shell/src/agent/config.rs @@ -794,22 +794,17 @@ pub struct PlatformCredentialConfig { pub api_key: Option, } -/// Resolve the API key for an open-platform registry entry: -/// platform-scoped env > generic `KIGI_MOONSHOT_API_KEY` env > config file. -/// `None` for the OAuth platform and when nothing is configured. -/// The returned value must never be logged. -pub(crate) fn resolve_platform_api_key( - platform: kigi_models::PlatformId, - platforms: &PlatformsConfig, -) -> Option { - resolve_platform_api_key_with(platform, platforms, |name| std::env::var(name).ok()) -} - -/// Testable core of [`resolve_platform_api_key`] with an injected getenv. +/// Resolve the API key for an API-key registry platform with injected env +/// and auth.json readers. Precedence: env > auth.json > config file — the +/// same "env always wins" rule the config-file layer already follows. +/// `None` for OAuth platforms and when nothing is configured. The batch +/// production caller is `PlatformApiKeys::resolve` (reads auth.json once +/// for the whole registry sweep). The returned value must never be logged. pub(crate) fn resolve_platform_api_key_with( platform: kigi_models::PlatformId, platforms: &PlatformsConfig, mut getenv: impl FnMut(&str) -> Option, + stored: impl FnOnce(kigi_models::PlatformId) -> Option, ) -> Option { for name in platform.api_key_env_names() { if let Some(value) = getenv(name) @@ -818,15 +813,19 @@ pub(crate) fn resolve_platform_api_key_with( return Some(value); } } + if let Some(value) = stored(platform) + && !value.trim().is_empty() + { + return Some(value); + } platforms.config_api_key(platform) } -/// Persist `[platforms.].api_key` into `~/.kigi/config.toml` — the exact -/// table [`resolve_platform_api_key`] reads back (env vars still win over the -/// file). Shared writer for the CLI and the TUI login screen; same in-process -/// pattern as the `kigi mcp add` writer (whole-file toml round-trip, atomic -/// tmp+rename), taken under the config write lock so it can't interleave with -/// a settings save. +/// Persist a platform API key into `~/.kigi/auth.json` under the platform-id +/// scope — the per-provider auth.json key contract that +/// [`resolve_platform_api_key`] reads back (env vars still win). Shared +/// writer for the CLI and the TUI login screen. Legacy `[platforms.*]` +/// config.toml keys remain a read-only fallback source. /// /// SECURITY: the key lands in the file by design; it must never be logged, /// and errors carry only path/IO context — never the key. @@ -834,21 +833,15 @@ pub async fn save_platform_api_key( platform: kigi_models::PlatformId, api_key: &str, ) -> anyhow::Result<()> { - let _guard = crate::util::config::lock_config_writes().await; - save_platform_api_key_at(&crate::util::config::user_config_path(), platform, api_key).await + save_platform_api_key_in(&crate::util::kigi_home::kigi_home(), platform, api_key) } -/// Path-injectable core of [`save_platform_api_key`] (tests use a tempdir). -/// Does NOT take the config write lock — production callers go through -/// [`save_platform_api_key`]. -pub async fn save_platform_api_key_at( - path: &std::path::Path, +/// Home-injectable core of [`save_platform_api_key`] (tests use a tempdir). +pub fn save_platform_api_key_in( + kigi_home: &std::path::Path, platform: kigi_models::PlatformId, api_key: &str, ) -> anyhow::Result<()> { - use toml::Value as TomlValue; - use toml::map::Map as TomlMap; - anyhow::ensure!( !platform.uses_oauth(), "{} authenticates via OAuth and takes no API key", @@ -856,38 +849,13 @@ pub async fn save_platform_api_key_at( ); let api_key = api_key.trim(); anyhow::ensure!(!api_key.is_empty(), "API key must not be empty"); - - let mut root: TomlValue = match tokio::fs::read_to_string(path).await { - Ok(s) => toml::from_str(&s).map_err(|e| { - // Refuse to overwrite an unparseable config — a silent fallback - // to an empty table would drop every other section. - anyhow::anyhow!("refusing to overwrite unparseable {}: {e}", path.display()) - })?, - Err(_) => TomlValue::Table(TomlMap::new()), - }; - let table = root - .as_table_mut() - .ok_or_else(|| anyhow::anyhow!("config root is not a table"))?; - let platforms = table - .entry("platforms") - .or_insert_with(|| TomlValue::Table(TomlMap::new())) - .as_table_mut() - .ok_or_else(|| anyhow::anyhow!("[platforms] is not a table"))?; - let entry = platforms - .entry(platform.as_str().to_string()) - .or_insert_with(|| TomlValue::Table(TomlMap::new())) - .as_table_mut() - .ok_or_else(|| anyhow::anyhow!("[platforms.{}] is not a table", platform.as_str()))?; - entry.insert( - "api_key".to_string(), - TomlValue::String(api_key.to_string()), - ); - - let toml_str = toml::to_string_pretty(&root)?; - // Mode-preserving atomic write: a 0600 config must not widen while - // receiving a secret. - crate::util::config::atomic_write_string(path, &toml_str)?; - Ok(()) + crate::auth::store_platform_api_key(kigi_home, platform, api_key).map_err(|e| { + anyhow::anyhow!( + "saving {} API key to auth.json in {}: {e}", + platform.as_str(), + kigi_home.display() + ) + }) } #[derive(Clone, Debug, Default, Serialize, Deserialize)] @@ -2565,9 +2533,10 @@ fn managed_settings_env_flag(key: &str) -> Option { } /// Assemble the final model map. Priority (highest wins): /// config.toml `[model.*]` > prefetched (remote) > hardcoded defaults. -pub fn resolve_model_list( +pub(crate) fn resolve_model_list( cfg: &Config, prefetched: Option>, + platform_keys: &crate::agent::models::PlatformApiKeys, ) -> IndexMap { let mut resolved: IndexMap = IndexMap::new(); if cfg.endpoints.has_custom_endpoint() { @@ -2681,7 +2650,7 @@ pub fn resolve_model_list( } apply_global_extra_headers(&mut resolved, &cfg.models); apply_global_scalar_defaults(&mut resolved, &cfg.models); - apply_platform_credentials(&mut resolved, &cfg.platforms); + apply_platform_credentials(&mut resolved, &cfg.platforms, platform_keys); for entry in resolved.values_mut() { entry.info.derive_reasoning_effort_fields(); } @@ -2704,6 +2673,7 @@ pub fn resolve_model_list( fn apply_platform_credentials( resolved: &mut IndexMap, platforms: &PlatformsConfig, + platform_keys: &crate::agent::models::PlatformApiKeys, ) { for (key, entry) in resolved.iter_mut() { let id = entry.info.id.as_deref().unwrap_or(key.as_str()); @@ -2720,15 +2690,23 @@ fn apply_platform_credentials( .env_key .as_ref() .is_some_and(|k| k.resolve_value().is_some()); - if entry.api_key.is_none() - && !env_resolves - && let Some(config_key) = platforms.config_api_key(platform) - { - tracing::debug!( - model_key = %key, platform = platform.as_str(), - "stamped [platforms] config api_key onto open-platform entry" - ); - entry.api_key = Some(config_key); + if entry.api_key.is_none() && !env_resolves { + // The resolved snapshot (env > auth.json > config, minus env + // which stays live via env_key above) wins over a raw config.toml + // read, so a key rotated via the TUI login can never lose to a + // stale `[platforms.*]` entry. The config fallback keeps callers + // that pass an empty snapshot (tests, pure-config paths) working. + let stamped = platform_keys + .key_for(platform) + .map(str::to_owned) + .or_else(|| platforms.config_api_key(platform)); + if let Some(stamped) = stamped { + tracing::debug!( + model_key = %key, platform = platform.as_str(), + "stamped resolved platform api_key onto open-platform entry" + ); + entry.api_key = Some(stamped); + } } // A credentialed open-platform entry is usable by API-key users. if entry.has_own_credentials() { @@ -3877,7 +3855,7 @@ pub fn try_resolve_model_credentials( let cfg = Config::new_from_toml_cfg(&raw) .map_err(|e| tracing::warn!(error = % e, "config parse failed for credential resolution")) .ok()?; - let models = resolve_model_list(&cfg, None); + let models = resolve_model_list(&cfg, None, &Default::default()); let entry = find_model_by_id(&models, model_id)?; let credentials = resolve_credentials(entry, session_key); Some(credentials) @@ -3936,7 +3914,7 @@ fn with_resolved_model(model_id: &str, f: impl FnOnce(ModelLookup) -> T) -> T else { return f(ModelLookup::ConfigUnavailable); }; - let models = resolve_model_list(&cfg, None); + let models = resolve_model_list(&cfg, None, &Default::default()); f(ModelLookup::Loaded(find_model_by_id(&models, model_id))) } /// Resolve a standalone `SamplerConfig` for an auxiliary model slug (image @@ -4616,7 +4594,7 @@ reasoning_effort = "low" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let model = resolved.get("my-custom-model").expect("model should exist"); assert_eq!(model.info.model, "kigi-4.5"); assert_eq!(model.info.base_url, "https://api.example.com/v1"); @@ -5131,7 +5109,7 @@ reasoning_effort = "low" )) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let model = resolved.get(dm).expect("model should exist"); assert_eq!(model.api_key, Some("user-custom-api-key".to_string())); assert_eq!(model.info.model, dm); @@ -5184,7 +5162,7 @@ reasoning_effort = "low" )) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let model = resolve_model_list(&cfg, None) + let model = resolve_model_list(&cfg, None, &Default::default()) .get(dm) .expect("model should exist") .clone(); @@ -5200,7 +5178,7 @@ reasoning_effort = "low" )) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let model = resolve_model_list(&cfg, None) + let model = resolve_model_list(&cfg, None, &Default::default()) .get(dm) .expect("model should exist") .clone(); @@ -5221,7 +5199,7 @@ reasoning_effort = "low" )) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let model = resolve_model_list(&cfg, None) + let model = resolve_model_list(&cfg, None, &Default::default()) .get(dm) .expect("model should exist") .clone(); @@ -5237,7 +5215,7 @@ reasoning_effort = "low" )) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let model = resolve_model_list(&cfg, None) + let model = resolve_model_list(&cfg, None, &Default::default()) .get(dm) .expect("model should exist") .clone(); @@ -5253,7 +5231,7 @@ reasoning_effort = "low" )) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let model = resolve_model_list(&cfg, None) + let model = resolve_model_list(&cfg, None, &Default::default()) .get(dm) .expect("model should exist") .clone(); @@ -5367,7 +5345,7 @@ reasoning_effort = "low" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let model = resolved.get("my-custom-model").expect("model should exist"); assert_eq!(model.info.context_window, NonZeroU64::new(256_000).unwrap()); } @@ -5394,7 +5372,7 @@ reasoning_effort = "low" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let model = resolved .get("my-responses-model") .expect("model should exist"); @@ -5413,7 +5391,7 @@ reasoning_effort = "low" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let model = resolved.get("my-chat-model").expect("model should exist"); assert_eq!(model.info.api_backend, ApiBackend::ChatCompletions); } @@ -5433,7 +5411,7 @@ reasoning_effort = "low" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let model = resolved.get("my-claude").expect("model should exist"); assert!( model.info.supports_reasoning_effort, @@ -5456,7 +5434,7 @@ reasoning_effort = "low" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let model = resolved.get("my-claude").expect("model should exist"); assert!( !model.info.supports_reasoning_effort, @@ -5478,7 +5456,7 @@ reasoning_effort = "low" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let model = resolved.get("my-openai").expect("model should exist"); assert!( !model.info.supports_reasoning_effort, @@ -5497,7 +5475,7 @@ reasoning_effort = "low" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let model = resolved.get("my-model").expect("model should exist"); assert_eq!(model.info.api_backend, ApiBackend::ChatCompletions); } @@ -5523,7 +5501,7 @@ reasoning_effort = "low" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let model = resolved .get("my-concise-model") .expect("model should exist"); @@ -5541,7 +5519,7 @@ reasoning_effort = "low" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let model = resolved.get("my-model").expect("model should exist"); assert!(!model.info.use_concise); } @@ -5600,7 +5578,7 @@ reasoning_effort = "low" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let model = resolved.get("my-model").expect("model should exist"); assert!( !model.info.use_concise, @@ -5685,7 +5663,7 @@ reasoning_effort = "low" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let model = resolved.get("my-agent-model").expect("model should exist"); assert_eq!(model.info.agent_type, "codex"); } @@ -5701,7 +5679,7 @@ reasoning_effort = "low" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let model = resolved.get("my-model").expect("model should exist"); assert_eq!(model.info.agent_type, DEFAULT_AGENT_TYPE); } @@ -5962,7 +5940,7 @@ reasoning_effort = "low" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).unwrap(); - let catalog = resolve_model_catalog(&cfg, None); + let catalog = resolve_model_catalog(&cfg, None, &Default::default()); let available = available_models(&catalog, true); assert!( catalog.contains_key("visible-model"), @@ -5995,7 +5973,11 @@ reasoning_effort = "low" "#, ) .unwrap(); - let catalog = resolve_model_catalog(&Config::new_from_toml_cfg(&raw).unwrap(), None); + let catalog = resolve_model_catalog( + &Config::new_from_toml_cfg(&raw).unwrap(), + None, + &Default::default(), + ); assert!(!catalog.contains_key("to-disable")); } #[test] @@ -6012,7 +5994,11 @@ reasoning_effort = "low" "#, ) .unwrap(); - let catalog = resolve_model_catalog(&Config::new_from_toml_cfg(&raw).unwrap(), None); + let catalog = resolve_model_catalog( + &Config::new_from_toml_cfg(&raw).unwrap(), + None, + &Default::default(), + ); let available = available_models(&catalog, true); assert!(catalog.contains_key("to-hide")); assert!(catalog["to-hide"].info.hidden); @@ -6040,7 +6026,11 @@ reasoning_effort = "low" "#, ) .unwrap(); - let catalog = resolve_model_catalog(&Config::new_from_toml_cfg(&raw).unwrap(), None); + let catalog = resolve_model_catalog( + &Config::new_from_toml_cfg(&raw).unwrap(), + None, + &Default::default(), + ); assert!(catalog["keep-one"].info.user_selectable, "wildcard match"); assert!( catalog["explicit-key"].info.user_selectable, @@ -6065,7 +6055,11 @@ reasoning_effort = "low" "#, ) .unwrap(); - let catalog = resolve_model_catalog(&Config::new_from_toml_cfg(&raw).unwrap(), None); + let catalog = resolve_model_catalog( + &Config::new_from_toml_cfg(&raw).unwrap(), + None, + &Default::default(), + ); assert!( catalog["foo"].info.user_selectable, "empty allowed_models must not restrict" @@ -6110,7 +6104,7 @@ reasoning_effort = "low" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw).unwrap(); - let catalog = resolve_model_catalog(&cfg, None); + let catalog = resolve_model_catalog(&cfg, None, &Default::default()); assert!(catalog.contains_key("oauth-only-model")); assert!(catalog.contains_key("public-model")); let api_available = available_models(&catalog, false); @@ -6137,7 +6131,7 @@ reasoning_effort = "low" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let model = resolved.get("slow-model").expect("model should exist"); assert_eq!(model.info.inference_idle_timeout_secs, Some(600)); } @@ -6153,7 +6147,7 @@ reasoning_effort = "low" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let model = resolved.get("default-model").expect("model should exist"); assert_eq!(model.info.inference_idle_timeout_secs, None); } @@ -6214,7 +6208,7 @@ reasoning_effort = "low" ) -> (Config, IndexMap) { let raw: toml::Value = toml::from_str(toml_str).expect("test TOML should parse"); let cfg = Config::new_from_toml_cfg(&raw).expect("config should parse"); - let resolved = resolve_model_list(&cfg, prefetched); + let resolved = resolve_model_list(&cfg, prefetched, &Default::default()); (cfg, resolved) } fn resolve_sampling(model: &ModelEntry, session_key: Option<&str>) -> SamplerConfig { @@ -6539,7 +6533,7 @@ reasoning_effort = "low" None, ), ); - let resolved = resolve_model_list(&cfg, Some(prefetched)); + let resolved = resolve_model_list(&cfg, Some(prefetched), &Default::default()); assert!( resolved.contains_key("acme-model"), "enterprise model should be present" @@ -6553,7 +6547,7 @@ reasoning_effort = "low" #[test] fn e2e_default_endpoint_still_injects_defaults() { let cfg = Config::default(); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); assert!( resolved.contains_key(BUNDLED_DEFAULT_KEY), "default model should be present when using default endpoint" @@ -8703,7 +8697,7 @@ default = "kigi-4.5" ); entry.info.context_window = NonZeroU64::new(default_cw).unwrap(); prefetched.insert("kigi-4.5".to_owned(), entry); - let resolved = resolve_model_list(&cfg, Some(prefetched)); + let resolved = resolve_model_list(&cfg, Some(prefetched), &Default::default()); let by_key = resolved .get("kigi-build") .expect("kigi-build key must exist"); @@ -8740,7 +8734,7 @@ default = "kigi-4.5" entry.info.agent_type = default_agent_type(); entry.info.api_backend = ApiBackend::default(); prefetched.insert("kigi-4.5".to_owned(), entry); - let resolved = resolve_model_list(&cfg, Some(prefetched)); + let resolved = resolve_model_list(&cfg, Some(prefetched), &Default::default()); let latest = resolved.get("kigi-4.5").unwrap(); assert_eq!( latest.info.agent_type, @@ -8772,7 +8766,7 @@ default = "kigi-4.5" test_model_entry("kigi-4.5", "https://test.example.com/v1", None, None, None); entry.info.context_window = NonZeroU64::new(65_536).unwrap(); prefetched.insert("kigi-4.5".to_owned(), entry); - let resolved = resolve_model_list(&cfg, Some(prefetched)); + let resolved = resolve_model_list(&cfg, Some(prefetched), &Default::default()); let latest = resolved.get("kigi-4.5").unwrap(); assert_eq!( latest.info.context_window.get(), @@ -8795,7 +8789,7 @@ default = "kigi-4.5" ); entry.info.context_window = NonZeroU64::new(default_cw).unwrap(); prefetched.insert("some-unknown-model".to_owned(), entry); - let resolved = resolve_model_list(&cfg, Some(prefetched)); + let resolved = resolve_model_list(&cfg, Some(prefetched), &Default::default()); let model = resolved.get("some-unknown-model").unwrap(); assert_eq!( model.info.context_window.get(), @@ -8944,7 +8938,7 @@ default = "kigi-4.5" let entry = prefetch_model_entry("remote-only-model", 200_000, ApiBackend::default()); let mut prefetched = IndexMap::new(); prefetched.insert("remote-only-model".to_owned(), entry); - let resolved = resolve_model_list(&cfg, Some(prefetched)); + let resolved = resolve_model_list(&cfg, Some(prefetched), &Default::default()); let model = resolved .get("remote-only-model") .expect("prefetched model should exist"); @@ -8970,7 +8964,7 @@ default = "kigi-4.5" let entry = prefetch_model_entry("remote-only-model", 200_000, ApiBackend::default()); let mut prefetched = IndexMap::new(); prefetched.insert("remote-only-model".to_owned(), entry); - let resolved = resolve_model_list(&cfg, Some(prefetched)); + let resolved = resolve_model_list(&cfg, Some(prefetched), &Default::default()); let info = &resolved .get("remote-only-model") .expect("prefetched model should exist") @@ -8997,7 +8991,7 @@ default = "kigi-4.5" let entry = prefetch_model_entry("remote-only-model", 200_000, ApiBackend::default()); let mut prefetched = IndexMap::new(); prefetched.insert("remote-only-model".to_owned(), entry); - let resolved = resolve_model_list(&cfg, Some(prefetched)); + let resolved = resolve_model_list(&cfg, Some(prefetched), &Default::default()); let model = resolved .get("remote-only-model") .expect("model should exist"); @@ -9021,7 +9015,7 @@ default = "kigi-4.5" entry.info.max_retries = Some(3); let mut prefetched = IndexMap::new(); prefetched.insert("remote-only-model".to_owned(), entry); - let resolved = resolve_model_list(&cfg, Some(prefetched)); + let resolved = resolve_model_list(&cfg, Some(prefetched), &Default::default()); let model = resolved .get("remote-only-model") .expect("prefetched model should exist"); @@ -9058,7 +9052,7 @@ default = "kigi-4.5" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw_config).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let custom = &resolved.get("custom").expect("custom model").info; assert_eq!(custom.reasoning_efforts.len(), 2); assert_eq!(custom.reasoning_efforts[0].label, "High"); @@ -9094,7 +9088,7 @@ default = "kigi-4.5" }]; let mut prefetched = IndexMap::new(); prefetched.insert("kigi-x".to_owned(), entry); - let resolved = resolve_model_list(&cfg, Some(prefetched)); + let resolved = resolve_model_list(&cfg, Some(prefetched), &Default::default()); let efforts = &resolved .get("kigi-x") .expect("kigi-x") @@ -9113,7 +9107,7 @@ default = "kigi-4.5" let entry = prefetch_model_entry(BUNDLED_DEFAULT_KEY, default_cw, ApiBackend::default()); let mut prefetched = IndexMap::new(); prefetched.insert(BUNDLED_DEFAULT_KEY.to_owned(), entry); - let resolved = resolve_model_list(&cfg, Some(prefetched)); + let resolved = resolve_model_list(&cfg, Some(prefetched), &Default::default()); let entry = resolved.get(BUNDLED_DEFAULT_KEY).expect("model must exist"); assert_ne!( entry.info.context_window.get(), @@ -9128,7 +9122,7 @@ default = "kigi-4.5" let entry = prefetch_model_entry(BUNDLED_DEFAULT_KEY, explicit_cw, ApiBackend::default()); let mut prefetched = IndexMap::new(); prefetched.insert(BUNDLED_DEFAULT_KEY.to_owned(), entry); - let resolved = resolve_model_list(&cfg, Some(prefetched)); + let resolved = resolve_model_list(&cfg, Some(prefetched), &Default::default()); let entry = resolved.get(BUNDLED_DEFAULT_KEY).expect("model must exist"); assert_eq!( entry.info.context_window.get(), @@ -9143,7 +9137,7 @@ default = "kigi-4.5" let entry = prefetch_model_entry("kigi", default_cw, ApiBackend::default()); let mut prefetched = IndexMap::new(); prefetched.insert("kigi".to_owned(), entry); - let resolved = resolve_model_list(&cfg, Some(prefetched)); + let resolved = resolve_model_list(&cfg, Some(prefetched), &Default::default()); let entry = resolved.get("kigi").expect("model must exist"); let defaults = default_model_entries(&EndpointsConfig::default()); if let Some(default) = defaults.get("kigi") { @@ -9169,9 +9163,9 @@ default = "kigi-4.5" if let Some(e) = defs.shift_remove(BUNDLED_DEFAULT_KEY) { p.insert(BUNDLED_DEFAULT_KEY.to_string(), e); } - let resolved = resolve_model_list(&cfg, Some(p)); + let resolved = resolve_model_list(&cfg, Some(p), &Default::default()); assert!(resolved.contains_key(BUNDLED_DEFAULT_KEY)); - let no_p = resolve_model_list(&cfg, None); + let no_p = resolve_model_list(&cfg, None, &Default::default()); assert!(no_p.contains_key(BUNDLED_DEFAULT_KEY)); } #[test] @@ -9182,7 +9176,7 @@ default = "kigi-4.5" if let Some(e) = defs.shift_remove(BUNDLED_DEFAULT_KEY) { p.insert(BUNDLED_DEFAULT_KEY.to_string(), e); } - let resolved = resolve_model_list(&cfg, Some(p)); + let resolved = resolve_model_list(&cfg, Some(p), &Default::default()); let sess: Vec<_> = resolved .values() .filter(|e| e.visible_for_auth(true)) @@ -9203,7 +9197,7 @@ default = "kigi-4.5" let mut p = IndexMap::new(); let e = prefetch_model_entry("secret-xyz", 200000, ApiBackend::default()); p.insert("secret-xyz".to_string(), e); - let resolved = resolve_model_list(&cfg, Some(p)); + let resolved = resolve_model_list(&cfg, Some(p), &Default::default()); assert!(resolved.contains_key("secret-xyz")); assert!(!resolved.contains_key(BUNDLED_DEFAULT_KEY)); } @@ -9213,14 +9207,14 @@ default = "kigi-4.5" let mut p = IndexMap::new(); let e = prefetch_model_entry("kimi-fresh", 500_000, ApiBackend::Responses); p.insert("kimi-fresh".to_string(), e); - let resolved = resolve_model_list(&cfg, Some(p)); + let resolved = resolve_model_list(&cfg, Some(p), &Default::default()); assert!(resolved.contains_key("kimi-fresh")); assert!(!resolved.contains_key(BUNDLED_DEFAULT_KEY)); } #[test] fn resolve_model_list_empty_prefetch_yields_empty_base() { let cfg = Config::default(); - let resolved = resolve_model_list(&cfg, Some(IndexMap::new())); + let resolved = resolve_model_list(&cfg, Some(IndexMap::new()), &Default::default()); assert!(resolved.is_empty()); } /// Regression: enterprise managed config aliases the bundled subscription @@ -9239,7 +9233,7 @@ default = "kigi-4.5" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let entry = resolved .get(BUNDLED_DEFAULT_KEY) .expect("bundled default must exist"); @@ -9261,7 +9255,7 @@ default = "kigi-4.5" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let entry = resolved .get(BUNDLED_DEFAULT_KEY) .expect("bundled default must exist"); @@ -9287,7 +9281,7 @@ default = "kigi-4.5" ) .unwrap(); let cfg = Config::new_from_toml_cfg(&raw).expect("config should parse"); - let resolved = resolve_model_list(&cfg, None); + let resolved = resolve_model_list(&cfg, None, &Default::default()); let cn = resolved .get("moonshot-cn/kimi-k2-turbo-preview") @@ -9318,6 +9312,76 @@ default = "kigi-4.5" "the OAuth platform takes no API key" ); } + /// A key that resolves ONLY via auth.json (the TUI-paste storage) must be + /// stamped onto the platform's catalog entries exactly like a config.toml + /// key — otherwise login validates the key but every completion goes out + /// keyless (401), and restart falls back to the login screen. + #[test] + #[serial] + fn auth_json_resolved_key_stamps_matching_open_platform_entries() { + let _cn = EnvGuard::unset(kigi_models::MOONSHOT_CN_API_KEY_ENV); + let _ai = EnvGuard::unset(kigi_models::MOONSHOT_AI_API_KEY_ENV); + let _gen = EnvGuard::unset(kigi_models::MOONSHOT_API_KEY_ENV); + let cfg = Config::default(); + // The resolved snapshot as PlatformApiKeys::resolve would build it + // from an auth.json `moonshot-cn` scope (no env, no config.toml). + let keys = + crate::agent::models::PlatformApiKeys::test_keys(Some("sk-from-auth-json"), None); + let resolved = resolve_model_list(&cfg, None, &keys); + + let cn = resolved + .get("moonshot-cn/kimi-k2-turbo-preview") + .expect("bundled moonshot-cn entry"); + assert_eq!( + cn.api_key.as_deref(), + Some("sk-from-auth-json"), + "auth.json-resolved key must be stamped onto the entry" + ); + assert!( + cn.visible_for_auth(false), + "credentialed open-platform entry must be visible to API-key users" + ); + assert!( + crate::agent::auth_method::should_advertise_xai_api_key(resolved.values()), + "a stamped auth.json key alone must advertise the API-key auth \ + method on restart (no login screen)" + ); + let ai = resolved + .get("moonshot-ai/kimi-k2-turbo-preview") + .expect("bundled moonshot-ai entry"); + assert!( + ai.api_key.is_none(), + "the cn key must not leak onto the ai platform" + ); + } + /// When auth.json and config.toml disagree, the resolved snapshot + /// (auth.json) wins — the same precedence the login validator uses, so a + /// key rotated via the TUI can never lose to a stale config.toml key. + #[test] + #[serial] + fn auth_json_key_beats_stale_config_key_when_stamping() { + let _cn = EnvGuard::unset(kigi_models::MOONSHOT_CN_API_KEY_ENV); + let _ai = EnvGuard::unset(kigi_models::MOONSHOT_AI_API_KEY_ENV); + let _gen = EnvGuard::unset(kigi_models::MOONSHOT_API_KEY_ENV); + let raw: toml::Value = toml::from_str( + r#" + [platforms.moonshot-cn] + api_key = "sk-stale-config" + "#, + ) + .unwrap(); + let cfg = Config::new_from_toml_cfg(&raw).expect("config should parse"); + let keys = crate::agent::models::PlatformApiKeys::test_keys(Some("sk-rotated"), None); + let resolved = resolve_model_list(&cfg, None, &keys); + let cn = resolved + .get("moonshot-cn/kimi-k2-turbo-preview") + .expect("bundled moonshot-cn entry"); + assert_eq!( + cn.api_key.as_deref(), + Some("sk-rotated"), + "the resolved snapshot must beat the stale config.toml key" + ); + } /// F2 acceptance: with ONLY a moonshot key (env), the api-key auth method /// is advertised (no login screen) because the catalog has a credentialed /// entry. @@ -9326,84 +9390,96 @@ default = "kigi-4.5" fn moonshot_env_key_advertises_api_key_auth_method() { let _gen = EnvGuard::set(kigi_models::MOONSHOT_API_KEY_ENV, "sk-only-moonshot"); let cfg = Config::default(); - let models = resolve_model_list(&cfg, None); + let models = resolve_model_list(&cfg, None, &Default::default()); assert!( crate::agent::auth_method::should_advertise_xai_api_key(models.values()), "a moonshot env key alone must advertise the API-key auth method" ); } - /// The login-screen writer persists `[platforms.].api_key` into the - /// exact table `resolve_platform_api_key` reads back, preserving sibling - /// tables and never leaking onto the other platform. - #[tokio::test] - async fn save_platform_api_key_round_trips_through_resolver() { + /// The login-screen writer persists the key into auth.json under the + /// platform-id scope — the exact scope `resolve_platform_api_key` reads + /// back — trimming whitespace and never leaking onto the other platform. + #[test] + fn save_platform_api_key_round_trips_through_resolver() { let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("config.toml"); - std::fs::write(&path, "[ui]\ncompact_mode = true\n").unwrap(); + let home = dir.path(); - save_platform_api_key_at(&path, kigi_models::PlatformId::MoonshotCn, "sk-from-tui") - .await + save_platform_api_key_in(home, kigi_models::PlatformId::MoonshotCn, " sk-from-tui ") .expect("write must succeed"); - let raw: toml::Value = toml::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); - let platforms: PlatformsConfig = raw - .get("platforms") - .cloned() - .expect("[platforms] written") - .try_into() - .expect("PlatformsConfig parses"); - // Env unset in this resolve (injected getenv) → config file wins. - let resolved = - resolve_platform_api_key_with(kigi_models::PlatformId::MoonshotCn, &platforms, |_| { - None - }); - assert_eq!(resolved.as_deref(), Some("sk-from-tui")); - assert!( - platforms - .config_api_key(kigi_models::PlatformId::MoonshotAi) - .is_none(), + // Env unset in this resolve (injected getenv) → auth.json wins. + let platforms = PlatformsConfig::default(); + let resolved = resolve_platform_api_key_with( + kigi_models::PlatformId::MoonshotCn, + &platforms, + |_| None, + |p| crate::auth::read_platform_api_key(home, p), + ); + assert_eq!( + resolved.as_deref(), + Some("sk-from-tui"), + "trimmed key round-trips" + ); + assert_eq!( + crate::auth::read_platform_api_key(home, kigi_models::PlatformId::MoonshotAi), + None, "the cn key must not leak onto the ai platform" ); - assert!( - raw.get("ui") - .and_then(|ui| ui.get("compact_mode")) - .and_then(|v| v.as_bool()) - .unwrap_or(false), - "sibling [ui] table must be preserved" - ); } - /// Writer guardrails: the OAuth platform takes no key, empty keys are - /// rejected, and an unparseable config is refused (never clobbered). - #[tokio::test] - async fn save_platform_api_key_rejects_invalid_inputs() { + /// Precedence: env var > auth.json scope > `[platforms.*]` config file. + #[test] + fn platform_key_precedence_env_then_auth_json_then_config() { let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("config.toml"); + let home = dir.path(); + save_platform_api_key_in(home, kigi_models::PlatformId::MoonshotCn, "sk-auth-json") + .expect("write must succeed"); + let platforms: PlatformsConfig = + toml::from_str("[moonshot-cn]\napi_key = \"sk-config\"\n").unwrap(); + + let stored = |p| crate::auth::read_platform_api_key(home, p); + // Env wins over both files. + let resolved = resolve_platform_api_key_with( + kigi_models::PlatformId::MoonshotCn, + &platforms, + |name| (name == kigi_models::MOONSHOT_CN_API_KEY_ENV).then(|| "sk-env".to_owned()), + stored, + ); + assert_eq!(resolved.as_deref(), Some("sk-env")); + // auth.json wins over config.toml. + let resolved = resolve_platform_api_key_with( + kigi_models::PlatformId::MoonshotCn, + &platforms, + |_| None, + stored, + ); + assert_eq!(resolved.as_deref(), Some("sk-auth-json")); + // config.toml is the last fallback. + let resolved = resolve_platform_api_key_with( + kigi_models::PlatformId::MoonshotCn, + &platforms, + |_| None, + |_| None, + ); + assert_eq!(resolved.as_deref(), Some("sk-config")); + } + /// Writer guardrails: the OAuth platform takes no key and empty keys are + /// rejected — and a rejected write never creates auth.json. + #[test] + fn save_platform_api_key_rejects_invalid_inputs() { + let dir = tempfile::tempdir().unwrap(); + let home = dir.path(); assert!( - save_platform_api_key_at(&path, kigi_models::PlatformId::KimiCode, "sk-x") - .await - .is_err(), + save_platform_api_key_in(home, kigi_models::PlatformId::KimiCode, "sk-x").is_err(), "kimi-code authenticates via OAuth and must reject an API key" ); assert!( - save_platform_api_key_at(&path, kigi_models::PlatformId::MoonshotCn, " ") - .await - .is_err(), + save_platform_api_key_in(home, kigi_models::PlatformId::MoonshotCn, " ").is_err(), "blank keys must be rejected" ); - - let bad = "this is [not valid toml\n"; - std::fs::write(&path, bad).unwrap(); assert!( - save_platform_api_key_at(&path, kigi_models::PlatformId::MoonshotCn, "sk-x") - .await - .is_err(), - "unparseable config must be refused" - ); - assert_eq!( - std::fs::read_to_string(&path).unwrap(), - bad, - "unparseable config must be left untouched" + !home.join("auth.json").exists(), + "rejected writes must not create auth.json" ); } #[test] diff --git a/crates/codegen/kigi-shell/src/agent/config_model_override_parse.rs b/crates/codegen/kigi-shell/src/agent/config_model_override_parse.rs index 2e55671..9db08f8 100644 --- a/crates/codegen/kigi-shell/src/agent/config_model_override_parse.rs +++ b/crates/codegen/kigi-shell/src/agent/config_model_override_parse.rs @@ -311,7 +311,7 @@ mod tests { w.kind == ModelOverrideWarningKind::DuplicateAlias && w.field.as_deref() == Some("send_compactions_remaining") })); - let resolved = crate::agent::config::resolve_model_list(&cfg, None); + let resolved = crate::agent::config::resolve_model_list(&cfg, None, &Default::default()); assert!(resolved.contains_key("kigi-4.5")); } diff --git a/crates/codegen/kigi-shell/src/agent/models.rs b/crates/codegen/kigi-shell/src/agent/models.rs index 79d238b..2c24a91 100644 --- a/crates/codegen/kigi-shell/src/agent/models.rs +++ b/crates/codegen/kigi-shell/src/agent/models.rs @@ -55,38 +55,57 @@ enum CacheAuthMethod { Platforms, } -/// Resolved open-platform API keys (PRD F2): platform-scoped env > -/// generic `KIGI_MOONSHOT_API_KEY` env > `[platforms.*]` config. +/// Resolved API-key platform credentials (PRD F2), one entry per registry +/// platform with a usable key: platform env var(s) > auth.json platform +/// scope > `[platforms.*]` config. /// /// SECURITY: values are secrets — the manual `Debug` impl prints presence /// only, and nothing here may be logged or persisted. #[derive(Clone, Default)] pub(crate) struct PlatformApiKeys { - moonshot_cn: Option, - moonshot_ai: Option, + keys: std::collections::BTreeMap, } impl std::fmt::Debug for PlatformApiKeys { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("PlatformApiKeys") - .field("moonshot_cn", &self.moonshot_cn.is_some()) - .field("moonshot_ai", &self.moonshot_ai.is_some()) - .finish() + let mut s = f.debug_struct("PlatformApiKeys"); + for platform in kigi_models::PlatformId::ALL { + if !platform.uses_oauth() { + s.field(platform.as_str(), &self.keys.contains_key(&platform)); + } + } + s.finish() } } impl PlatformApiKeys { pub(crate) fn resolve(platforms: &config::PlatformsConfig) -> Self { - Self { - moonshot_cn: config::resolve_platform_api_key( - kigi_models::PlatformId::MoonshotCn, + // Read auth.json ONCE for the whole registry sweep — per-platform + // re-reads would mean one file parse per provider on every resolve. + let stored = + crate::auth::read_auth_json(&crate::util::kigi_home::kigi_home().join("auth.json")) + .ok(); + let mut keys = std::collections::BTreeMap::new(); + for platform in kigi_models::PlatformId::ALL { + if platform.uses_oauth() { + continue; + } + let key = config::resolve_platform_api_key_with( + platform, platforms, - ), - moonshot_ai: config::resolve_platform_api_key( - kigi_models::PlatformId::MoonshotAi, - platforms, - ), + |name| std::env::var(name).ok(), + |p| { + stored + .as_ref() + .and_then(|m| m.get(p.as_str())) + .map(|a| a.key.clone()) + }, + ); + if let Some(key) = key { + keys.insert(platform, key); + } } + Self { keys } } /// Resolve from the effective on-disk config (startup paths that have no @@ -101,26 +120,26 @@ impl PlatformApiKeys { } pub(crate) fn key_for(&self, platform: kigi_models::PlatformId) -> Option<&str> { - match platform { - kigi_models::PlatformId::KimiCode => None, - kigi_models::PlatformId::MoonshotCn => self.moonshot_cn.as_deref(), - kigi_models::PlatformId::MoonshotAi => self.moonshot_ai.as_deref(), - } + self.keys.get(&platform).map(String::as_str) } - /// Any open-platform key configured? Drives "should we prefetch without a - /// session" and the F2 acceptance path (moonshot key only, no login). + /// Any API-key platform credentialed? Drives "should we prefetch without + /// a session" and the F2 acceptance path (platform key only, no login). pub(crate) fn any(&self) -> bool { - self.moonshot_cn.is_some() || self.moonshot_ai.is_some() + !self.keys.is_empty() } /// Test-only constructor (fields are private to this module). #[cfg(test)] pub(crate) fn test_keys(cn: Option<&str>, ai: Option<&str>) -> Self { - Self { - moonshot_cn: cn.map(str::to_owned), - moonshot_ai: ai.map(str::to_owned), + let mut keys = std::collections::BTreeMap::new(); + if let Some(k) = cn { + keys.insert(kigi_models::PlatformId::MoonshotCn, k.to_owned()); } + if let Some(k) = ai { + keys.insert(kigi_models::PlatformId::MoonshotAi, k.to_owned()); + } + Self { keys } } } @@ -300,7 +319,11 @@ impl ModelsManager { .map(|c| c.models) }); let has_prefetched = prefetched_models.is_some(); - let catalog = resolve_model_catalog(cfg, prefetched_models.clone()); + let catalog = resolve_model_catalog( + cfg, + prefetched_models.clone(), + &PlatformApiKeys::resolve(&cfg.platforms), + ); // Validate only against a real catalog; a bundled-only first run defers // to the async fetch (`apply_refresh_result`). @@ -348,7 +371,11 @@ impl ModelsManager { return; } let prefetched = self.inner.prefetched.read().clone(); - let new_catalog = resolve_model_catalog(&new_config, prefetched); + let new_catalog = resolve_model_catalog( + &new_config, + prefetched, + &PlatformApiKeys::resolve(&new_config.platforms), + ); let has_real_catalog = *self.inner.has_fetched_real_catalog.read(); if has_real_catalog && let Err(e) = validate_selectable(&new_config, &new_catalog) { tracing::error!(error = %e, "ignoring config reload: allowed_models excludes all models"); @@ -636,7 +663,8 @@ impl ModelsManager { // ── Mutations ─────────────────────────────────────────────────── fn rebuild(&self, cfg: &config::Config, prefetched: Option>) { - *self.inner.models.write() = resolve_model_catalog(cfg, prefetched); + *self.inner.models.write() = + resolve_model_catalog(cfg, prefetched, &PlatformApiKeys::resolve(&cfg.platforms)); } /// Refresh models when the etag changes. @@ -2015,11 +2043,13 @@ impl ModelGlobSet { /// `find_model_by_id`/`models()` and ignore `user_selectable`, so they need no /// exemption. Globs are validated at load (`Config::validate_model_filters`); /// the arms here fail closed if one slips through. -pub fn resolve_model_catalog( +pub(crate) fn resolve_model_catalog( cfg: &config::Config, prefetched: Option>, + platform_keys: &PlatformApiKeys, ) -> IndexMap { - let mut catalog: IndexMap = config::resolve_model_list(cfg, prefetched); + let mut catalog: IndexMap = + config::resolve_model_list(cfg, prefetched, platform_keys); if let Ok(Some(disabled)) = ModelGlobSet::compile(cfg.models.disabled_models.as_ref()) { let before = catalog.len(); @@ -2271,7 +2301,7 @@ mod tests { context_window = 256000 "#, ); - let catalog = resolve_model_catalog(&cfg, None); + let catalog = resolve_model_catalog(&cfg, None, &Default::default()); let (_key, entry, _src) = resolve_default_model(&cfg, &catalog, true); assert!( entry.info.user_selectable, @@ -2298,7 +2328,7 @@ mod tests { context_window = 256000 "#, ); - let catalog = resolve_model_catalog(&excluded, None); + let catalog = resolve_model_catalog(&excluded, None, &Default::default()); assert!( validate_selectable(&excluded, &catalog) .unwrap_err() @@ -2316,7 +2346,7 @@ mod tests { context_window = 256000 "#, ); - let catalog = resolve_model_catalog(&zero, None); + let catalog = resolve_model_catalog(&zero, None, &Default::default()); assert!(validate_selectable(&zero, &catalog).is_err()); } @@ -2463,7 +2493,7 @@ mod tests { reasoning_entry.info.supports_reasoning_effort = true; prefetched.insert("reasoning-model".to_string(), reasoning_entry); - let catalog = resolve_model_catalog(&cfg, Some(prefetched)); + let catalog = resolve_model_catalog(&cfg, Some(prefetched), &Default::default()); assert_eq!( catalog["reasoning-model"].info.reasoning_effort, Some(ReasoningEffort::High), @@ -2484,7 +2514,7 @@ mod tests { }; prefetched.insert("plain-model".to_string(), plain_entry); - let catalog = resolve_model_catalog(&cfg, Some(prefetched)); + let catalog = resolve_model_catalog(&cfg, Some(prefetched), &Default::default()); assert_eq!( catalog["plain-model"].info.reasoning_effort, None, "non-reasoning default model must NOT be stamped with persisted effort", @@ -2537,7 +2567,7 @@ mod tests { }]; prefetched.insert("legacy-none".to_string(), with_none); - let catalog = resolve_model_catalog(&cfg, Some(prefetched)); + let catalog = resolve_model_catalog(&cfg, Some(prefetched), &Default::default()); assert_eq!( catalog["kigi-4.5"].info.reasoning_effort, Some(ReasoningEffort::High), @@ -2583,7 +2613,7 @@ mod tests { cfg.config_models .insert("plain".to_string(), config::ConfigModelOverride::default()); - let catalog = resolve_model_catalog(&cfg, None); + let catalog = resolve_model_catalog(&cfg, None, &Default::default()); let info = &catalog["menu-only"].info; assert!( info.supports_reasoning_effort, @@ -2644,7 +2674,7 @@ mod tests { }; prefetched.insert("plain-model".to_string(), plain_entry); - let catalog = resolve_model_catalog(&cfg, Some(prefetched)); + let catalog = resolve_model_catalog(&cfg, Some(prefetched), &Default::default()); assert_eq!( catalog["reasoning-model"].info.reasoning_effort, Some(ReasoningEffort::High), @@ -3314,10 +3344,7 @@ mod tests { use serial_test::serial; fn keys(cn: Option<&str>, ai: Option<&str>) -> PlatformApiKeys { - PlatformApiKeys { - moonshot_cn: cn.map(str::to_owned), - moonshot_ai: ai.map(str::to_owned), - } + PlatformApiKeys::test_keys(cn, ai) } #[test] @@ -3366,6 +3393,7 @@ mod tests { kigi_models::PlatformId::MoonshotCn, &platforms, getenv, + |_| None, ) .as_deref(), Some("env-cn"), @@ -3376,6 +3404,7 @@ mod tests { kigi_models::PlatformId::MoonshotAi, &platforms, getenv, + |_| None, ) .as_deref(), Some("env-generic"), @@ -3387,6 +3416,7 @@ mod tests { kigi_models::PlatformId::MoonshotAi, &platforms, |_| None, + |_| None, ) .as_deref(), Some("cfg-ai"), @@ -3397,6 +3427,7 @@ mod tests { kigi_models::PlatformId::KimiCode, &platforms, getenv, + |_| None, ), None, ); @@ -3962,6 +3993,15 @@ mod tests { .with_env_filter(tracing_subscriber::EnvFilter::from_default_env()) .with_test_writer() .try_init(); + // Hermetic home + no platform keys: this path resolves PlatformApiKeys + // (env + auth.json). A dev machine's real ~/.kigi/auth.json platform + // scope or moonshot env var would enable a LIVE moonshot fetch here, + // short-circuiting the refresh-retry under test. + let hermetic_home = tempfile::tempdir().unwrap(); + let _home = EnvGuard::set("KIGI_SHARE_DIR", hermetic_home.path()); + let _cn = EnvGuard::unset(kigi_models::MOONSHOT_CN_API_KEY_ENV); + let _ai = EnvGuard::unset(kigi_models::MOONSHOT_AI_API_KEY_ENV); + let _gen = EnvGuard::unset(kigi_models::MOONSHOT_API_KEY_ENV); let server = MockServer::start().await; Mock::given(method("GET")) .and(path("/models")) @@ -4063,7 +4103,7 @@ mod tests { true, ); assert!(outcome.models.is_none(), "no cache and no network → None"); - let bundled = resolve_model_catalog(&config::Config::default(), None); + let bundled = resolve_model_catalog(&config::Config::default(), None, &Default::default()); assert!(bundled.contains_key("kimi-code/kimi-for-coding")); assert!(bundled.contains_key("moonshot-cn/kimi-k2-thinking-turbo")); assert!(bundled.contains_key("moonshot-ai/kimi-k2-turbo-preview")); diff --git a/crates/codegen/kigi-shell/src/agent/models_fetch.rs b/crates/codegen/kigi-shell/src/agent/models_fetch.rs index 6e6c883..3b343c8 100644 --- a/crates/codegen/kigi-shell/src/agent/models_fetch.rs +++ b/crates/codegen/kigi-shell/src/agent/models_fetch.rs @@ -176,8 +176,9 @@ fn fetch_platform_models_blocking( let enabled = enabled_platforms(auth.is_some(), platform_keys); if enabled.is_empty() { return Err(BackendError::Auth( - "No platform credentials: log in with `kigi login` or configure a moonshot API key \ - (KIGI_MOONSHOT_API_KEY or [platforms.*] in ~/.kigi/config.toml)." + "No platform credentials: log in with `kigi login`, paste a platform API key in \ + the login screen (stored in ~/.kigi/auth.json), or set a platform env var such \ + as KIGI_MOONSHOT_API_KEY." .into(), )); } diff --git a/crates/codegen/kigi-shell/src/agent/mvp_agent/acp_agent.rs b/crates/codegen/kigi-shell/src/agent/mvp_agent/acp_agent.rs index 96e1333..cbd6e0a 100644 --- a/crates/codegen/kigi-shell/src/agent/mvp_agent/acp_agent.rs +++ b/crates/codegen/kigi-shell/src/agent/mvp_agent/acp_agent.rs @@ -508,21 +508,23 @@ impl acp::Agent for MvpAgent { ); Ok(self.auth_response_with_meta()) } - auth_method::MOONSHOT_CN_METHOD_ID | auth_method::MOONSHOT_AI_METHOD_ID => { - let platform = auth_method::moonshot_platform_for_method_id( - &arguments.method_id, - ) - .expect("match arm guarantees a moonshot method id"); - self.authenticate_moonshot(platform, arguments.method_id.clone()) - .await - } _ => { - Err( - acp::Error::invalid_params() - .data( - format!("unsupported auth method: {}", arguments.method_id.0), - ), - ) + if let Some(platform) = + auth_method::platform_for_method_id(&arguments.method_id) + { + self.authenticate_api_key_platform( + platform, + arguments.method_id.clone(), + ) + .await + } else { + Err( + acp::Error::invalid_params() + .data( + format!("unsupported auth method: {}", arguments.method_id.0), + ), + ) + } } } } diff --git a/crates/codegen/kigi-shell/src/agent/mvp_agent/agent_ops.rs b/crates/codegen/kigi-shell/src/agent/mvp_agent/agent_ops.rs index 8508e78..f17ff46 100644 --- a/crates/codegen/kigi-shell/src/agent/mvp_agent/agent_ops.rs +++ b/crates/codegen/kigi-shell/src/agent/mvp_agent/agent_ops.rs @@ -452,18 +452,19 @@ impl MvpAgent { ) .await } - /// `authenticate(moonshot-cn / moonshot-ai)`: interactive open-platform - /// API-key login from the welcome picker. + /// `authenticate()`: interactive API-key login from + /// the welcome picker for any non-OAuth registry platform. /// /// Reloads the platform keys from disk+env (the TUI persists the pasted - /// key to `[platforms.]` in config.toml immediately before this call), - /// fails with an actionable error when none is configured, validates the - /// key against `GET {platform_base}/models`, then marks the session - /// authenticated exactly like an external API key: publish the method id - /// (NOT session-based — no token refresh), swap the freshly-stamped config - /// into the models manager, and trigger the model sync so the catalog - /// gains the platform's entries. The key itself is never logged. - pub(super) async fn authenticate_moonshot( + /// key to auth.json under the platform-id scope immediately before this + /// call), fails with an actionable error when none is configured, + /// validates the key against `GET {platform_base}/models`, then marks the + /// session authenticated exactly like an external API key: publish the + /// method id (NOT session-based — no token refresh), swap the + /// freshly-stamped config into the models manager, and trigger the model + /// sync so the catalog gains the platform's entries. The key itself is + /// never logged. + pub(super) async fn authenticate_api_key_platform( &self, platform: kigi_models::PlatformId, method_id: acp::AuthMethodId, @@ -480,10 +481,11 @@ impl MvpAgent { Some("platform_key_invalid_or_missing"), ); })?; - // Swap the on-disk config (now carrying the key) into the models - // manager so `apply_platform_credentials` stamps the platform's - // catalog entries; a parse failure keeps the last-known-good config - // (`on_auth_changed` below still re-resolves keys from disk itself). + // Rebuild the catalog from the on-disk config: the rebuild freshly + // resolves platform keys (env > auth.json > config), so the key just + // persisted to auth.json is stamped onto the platform's entries; a + // parse failure keeps the last-known-good config (`on_auth_changed` + // below still re-resolves keys from disk itself). match crate::config::load_effective_config() .map_err(|e| e.to_string()) .and_then(|raw| crate::agent::config::Config::new_from_toml_cfg(&raw)) diff --git a/crates/codegen/kigi-shell/src/auth/manager.rs b/crates/codegen/kigi-shell/src/auth/manager.rs index 9ea253c..cfdfc46 100644 --- a/crates/codegen/kigi-shell/src/auth/manager.rs +++ b/crates/codegen/kigi-shell/src/auth/manager.rs @@ -12,6 +12,7 @@ use tokio_util::sync::CancellationToken; #[path = "manager/lock.rs"] mod lock; +pub(crate) use lock::try_lock_auth_file_nonblocking; #[path = "manager/sleep_gate.rs"] mod sleep_gate; diff --git a/crates/codegen/kigi-shell/src/auth/mod.rs b/crates/codegen/kigi-shell/src/auth/mod.rs index 2342621..9615afa 100644 --- a/crates/codegen/kigi-shell/src/auth/mod.rs +++ b/crates/codegen/kigi-shell/src/auth/mod.rs @@ -27,5 +27,6 @@ pub use meta::AuthMeta; pub use model::{AuthMode, KimiAuth, lookup_auth}; pub(crate) use model::{TOKEN_TTL, is_expired, token_suffix}; pub use storage::{ - clear_api_key, read_api_key, read_auth_json, read_token_by_scope, store_api_key, + clear_api_key, read_api_key, read_auth_json, read_platform_api_key, read_token_by_scope, + store_api_key, store_platform_api_key, }; diff --git a/crates/codegen/kigi-shell/src/auth/storage.rs b/crates/codegen/kigi-shell/src/auth/storage.rs index 1157791..81eb3ae 100644 --- a/crates/codegen/kigi-shell/src/auth/storage.rs +++ b/crates/codegen/kigi-shell/src/auth/storage.rs @@ -96,9 +96,9 @@ pub(crate) fn disable_mock_keyring_for_test() { fn keyring_entry() -> Result<&'static keyring::Entry, keyring::Error> { static ENTRY: std::sync::OnceLock> = std::sync::OnceLock::new(); - match ENTRY.get_or_init(|| { - keyring::Entry::new(KEYRING_SERVICE, crate::auth::config::KIMI_CODE_OAUTH_SCOPE) - }) { + match ENTRY + .get_or_init(|| keyring::Entry::new(KEYRING_SERVICE, crate::auth::KIMI_CODE_OAUTH_SCOPE)) + { Ok(entry) => Ok(entry), // `keyring::Error` is not `Clone`; surface a stable equivalent. Err(e) => { @@ -536,6 +536,71 @@ pub fn store_api_key(kigi_home: &Path, api_key: &str) -> std::io::Result<()> { write_auth_json(&path, &map) } +/// Read an API-key platform's key from auth.json. The scope is the platform +/// id itself (`anthropic`, `moonshot-cn`, …) — the stable per-provider +/// auth.json key contract. `None` when absent or unreadable. +pub fn read_platform_api_key( + kigi_home: &Path, + platform: kigi_models::PlatformId, +) -> Option { + let path = kigi_home.join("auth.json"); + let map = read_auth_json(&path).ok()?; + map.get(platform.as_str()).map(|a| a.key.clone()) +} + +/// Store an API-key platform's key in auth.json under its platform-id scope. +/// Same corrupt-recovery + atomic-write path as [`store_api_key`]; all other +/// scopes (OAuth session, other platforms) are preserved. +/// +/// SECURITY: the key must never be logged; errors carry only IO context. +pub fn store_platform_api_key( + kigi_home: &Path, + platform: kigi_models::PlatformId, + api_key: &str, +) -> std::io::Result<()> { + if platform.uses_oauth() { + // Real error, not debug_assert: an OAuth scope written here would + // shadow the session entry in release builds too. + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!( + "{} authenticates via OAuth and takes no API key", + platform.as_str() + ), + )); + } + let path = kigi_home.join("auth.json"); + // Serialize with the manager's cross-process auth.json writers (token + // refresh holds the same flock): an unlocked read-modify-write here + // could write back a pre-refresh map and revert a rotated refresh + // token (token-family revocation → forced re-login). Bounded retry; + // a sustained holder fails loudly rather than racing. + let mut lock = None; + for _ in 0..20 { + lock = super::manager::try_lock_auth_file_nonblocking(&path); + if lock.is_some() { + break; + } + std::thread::sleep(std::time::Duration::from_millis(25)); + } + let Some(_lock) = lock else { + return Err(std::io::Error::new( + std::io::ErrorKind::WouldBlock, + "auth.json is locked by another kigi process; try again", + )); + }; + let mut map = read_auth_json_or_empty_recovering_corrupt(&path)?; + map.insert( + platform.as_str().to_owned(), + KimiAuth { + key: api_key.to_owned(), + auth_mode: AuthMode::ApiKey, + ..Default::default() + }, + ); + write_auth_json(&path, &map) +} + /// Remove the `kigi::api_key` scope from auth.json. pub fn clear_api_key(kigi_home: &Path) -> std::io::Result<()> { let path = kigi_home.join("auth.json"); @@ -550,6 +615,80 @@ pub fn clear_api_key(kigi_home: &Path) -> std::io::Result<()> { Ok(()) } +#[cfg(test)] +mod platform_key_tests { + use super::*; + + /// Store/read round-trip under the platform-id scope; the OAuth session + /// scope and other platform scopes in the same file are preserved. + #[test] + fn platform_key_round_trip_preserves_other_scopes() { + let dir = tempfile::tempdir().unwrap(); + let home = dir.path(); + // Pre-existing OAuth session entry must survive platform-key writes. + let path = home.join("auth.json"); + let mut map = AuthStore::new(); + map.insert( + crate::auth::KIMI_CODE_OAUTH_SCOPE.to_owned(), + KimiAuth { + key: "oauth-token".to_owned(), + auth_mode: AuthMode::OAuth, + ..Default::default() + }, + ); + write_auth_json(&path, &map).unwrap(); + + store_platform_api_key(home, kigi_models::PlatformId::MoonshotCn, "sk-cn").unwrap(); + store_platform_api_key(home, kigi_models::PlatformId::MoonshotAi, "sk-ai").unwrap(); + + assert_eq!( + read_platform_api_key(home, kigi_models::PlatformId::MoonshotCn).as_deref(), + Some("sk-cn") + ); + assert_eq!( + read_platform_api_key(home, kigi_models::PlatformId::MoonshotAi).as_deref(), + Some("sk-ai") + ); + let stored = read_auth_json(&path).unwrap(); + assert_eq!( + stored + .get(crate::auth::KIMI_CODE_OAUTH_SCOPE) + .map(|a| a.key.as_str()), + Some("oauth-token"), + "platform-key writes must not clobber the OAuth session scope" + ); + assert_eq!( + stored.get("moonshot-cn").map(|a| a.auth_mode.clone()), + Some(AuthMode::ApiKey), + "platform keys are stored as api_key mode under the platform id" + ); + } + + /// The OAuth platform takes no API key — a real error in release builds. + #[test] + fn storing_key_for_oauth_platform_is_invalid_input() { + let dir = tempfile::tempdir().unwrap(); + let err = store_platform_api_key(dir.path(), kigi_models::PlatformId::KimiCode, "sk-x") + .expect_err("oauth platform must reject api keys"); + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert!( + !dir.path().join("auth.json").exists(), + "rejected write must not create auth.json" + ); + } + + /// Missing file reads as None (not an error) — resolution treats absent + /// auth.json as "no stored key". + #[test] + fn reading_platform_key_without_auth_json_is_none() { + let dir = tempfile::tempdir().unwrap(); + assert_eq!( + read_platform_api_key(dir.path(), kigi_models::PlatformId::MoonshotCn), + None + ); + } +} + #[cfg(test)] mod write_fallback_tests { use super::*; diff --git a/crates/codegen/kigi-shell/src/cli_models.rs b/crates/codegen/kigi-shell/src/cli_models.rs index ae0d13a..7508299 100644 --- a/crates/codegen/kigi-shell/src/cli_models.rs +++ b/crates/codegen/kigi-shell/src/cli_models.rs @@ -35,7 +35,11 @@ impl AuthStatus { .unwrap_or(&origin); return Self::LoggedIn(host.to_owned()); } - let models = crate::agent::config::resolve_model_list(agent_config, None); + let models = crate::agent::config::resolve_model_list( + agent_config, + None, + &crate::agent::models::PlatformApiKeys::resolve(&agent_config.platforms), + ); if crate::agent::auth_method::should_advertise_xai_api_key(models.values()) && let Some(name) = models .iter() diff --git a/crates/codegen/kigi-shell/src/util/config/persist.rs b/crates/codegen/kigi-shell/src/util/config/persist.rs index 625200d..f7cd61b 100644 --- a/crates/codegen/kigi-shell/src/util/config/persist.rs +++ b/crates/codegen/kigi-shell/src/util/config/persist.rs @@ -92,13 +92,6 @@ pub async fn save_config(config: &Config) -> Result<()> { Ok(()) } -/// Acquire the `config.toml` write lock used by [`save_config`], so callers that -/// mutate the file directly (marketplace add/remove) can't interleave with a -/// settings save and clobber it. -pub(crate) async fn lock_config_writes() -> tokio::sync::MutexGuard<'static, ()> { - SAVE_LOCK.lock().await -} - /// Read a file, treating only `NotFound` as empty. Hard read errors (EACCES, /// EIO) propagate so callers don't clobber an unreadable file on the next write. pub(crate) fn read_to_string_or_empty(path: &std::path::Path) -> std::io::Result { diff --git a/crates/codegen/kigi-tui/src/app/actions.rs b/crates/codegen/kigi-tui/src/app/actions.rs index 5ccc249..1e6dcc1 100644 --- a/crates/codegen/kigi-tui/src/app/actions.rs +++ b/crates/codegen/kigi-tui/src/app/actions.rs @@ -574,9 +574,9 @@ pub enum Action { BeginPlatformKeyEntry(crate::app::app_view::PlatformLogin), /// Esc from the API-key entry box: return to the login picker. CancelPlatformKeyEntry, - /// User submitted a pasted Moonshot API key: persist it to - /// `[platforms.]` in config.toml, then authenticate with the - /// platform's method id. The key must never be logged. + /// User submitted a pasted platform API key: persist it to auth.json + /// under the platform-id scope, then authenticate with the platform's + /// method id. The key must never be logged. SubmitPlatformApiKey(String), /// Copy the auth URL to the clipboard during authentication. CopyAuthUrl, @@ -1612,7 +1612,7 @@ pub enum Effect { PollAuthUrl { request_seq: u64 }, /// Submit a manually-pasted auth code (ext request). SubmitAuthCode { request_seq: u64, code: String }, - /// Persist a Moonshot API key to `[platforms.]` in config.toml, then + /// Persist a platform API key to auth.json (platform-id scope), then /// send AuthenticateRequest with the platform's method id. SECURITY: the /// key must never appear in logs or errors. PersistPlatformApiKeyAndAuthenticate { diff --git a/crates/codegen/kigi-tui/src/app/app_view.rs b/crates/codegen/kigi-tui/src/app/app_view.rs index e740ea3..ade199f 100644 --- a/crates/codegen/kigi-tui/src/app/app_view.rs +++ b/crates/codegen/kigi-tui/src/app/app_view.rs @@ -264,41 +264,35 @@ pub enum AuthMode { /// from the welcome login picker. Esc returns to the picker (no quit). ApiKeyEntry(PlatformLogin), } -/// Open-platform API-key login target, selected from the welcome picker. -/// Mirrors the shell's `moonshot-cn` / `moonshot-ai` interactive auth -/// methods ([`kigi_shell::agent::auth_method`]). +/// API-key platform login target, selected from the welcome picker. Wraps a +/// non-OAuth registry platform ([`kigi_shell::models::PlatformId`]); +/// [`Self::from_method_id`] — the production entry point — guarantees the +/// non-OAuth invariant. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum PlatformLogin { - MoonshotCn, - MoonshotAi, -} +pub struct PlatformLogin(pub kigi_shell::models::PlatformId); impl PlatformLogin { /// The picker target behind an advertised ACP method id; `None` for every - /// non-moonshot method. + /// method that isn't an API-key registry platform. pub fn from_method_id(id: &acp::AuthMethodId) -> Option { - match id.0.as_ref() { - kigi_shell::agent::auth_method::MOONSHOT_CN_METHOD_ID => Some(Self::MoonshotCn), - kigi_shell::agent::auth_method::MOONSHOT_AI_METHOD_ID => Some(Self::MoonshotAi), - _ => None, - } + kigi_shell::agent::auth_method::platform_for_method_id(id).map(Self) } - /// The registry platform whose `[platforms.]` table stores the key. + /// The registry platform whose auth.json scope stores the key. pub fn platform_id(self) -> kigi_shell::models::PlatformId { - match self { - Self::MoonshotCn => kigi_shell::models::PlatformId::MoonshotCn, - Self::MoonshotAi => kigi_shell::models::PlatformId::MoonshotAi, - } + self.0 } /// The ACP auth method id to `authenticate` with after persisting the key. pub fn method_id(self) -> acp::AuthMethodId { acp::AuthMethodId::new(self.platform_id().as_str()) } - /// Console host shown in the paste-box copy. + /// Vendor word for the paste-box copy ("Paste your {vendor} API key"). + pub fn vendor(self) -> &'static str { + self.0.vendor() + } + /// Console host shown in the paste-box copy. Every API-key registry row + /// carries one (pinned by a kigi-models registry test); the fallback is + /// unreachable copy, not control flow. pub fn console_host(self) -> &'static str { - match self { - Self::MoonshotCn => "platform.moonshot.cn", - Self::MoonshotAi => "platform.moonshot.ai", - } + self.0.console_host().unwrap_or("the provider console") } } /// One row of the unauthenticated welcome menu (the login picker). @@ -6909,14 +6903,14 @@ pub(crate) mod tests { assert_eq!( items[1], PendingMenuItem::ApiKey { - target: PlatformLogin::MoonshotCn, + target: PlatformLogin(kigi_shell::models::PlatformId::MoonshotCn), label: "Moonshot Open Platform (API key \u{b7} moonshot.cn)".into(), } ); assert_eq!( items[2], PendingMenuItem::ApiKey { - target: PlatformLogin::MoonshotAi, + target: PlatformLogin(kigi_shell::models::PlatformId::MoonshotAi), label: "Moonshot Open Platform (API key \u{b7} moonshot.ai)".into(), } ); @@ -6960,7 +6954,9 @@ pub(crate) mod tests { assert!( matches!( outcome, - InputOutcome::Action(Action::BeginPlatformKeyEntry(PlatformLogin::MoonshotCn)) + InputOutcome::Action(Action::BeginPlatformKeyEntry(PlatformLogin( + kigi_shell::models::PlatformId::MoonshotCn + ))) ), "Enter on row 1 must open moonshot-cn key entry, got {outcome:?}" ); @@ -6976,7 +6972,7 @@ pub(crate) mod tests { request_seq: 1, handle: None, auth_url: None, - mode: AuthMode::ApiKeyEntry(PlatformLogin::MoonshotCn), + mode: AuthMode::ApiKeyEntry(PlatformLogin(kigi_shell::models::PlatformId::MoonshotCn)), }; let outcome = app.handle_input(&key_event(KeyCode::Esc, KeyModifiers::NONE)); assert!( @@ -6994,7 +6990,7 @@ pub(crate) mod tests { request_seq: 1, handle: None, auth_url: None, - mode: AuthMode::ApiKeyEntry(PlatformLogin::MoonshotAi), + mode: AuthMode::ApiKeyEntry(PlatformLogin(kigi_shell::models::PlatformId::MoonshotAi)), }; // Empty input: Enter is a no-op. let outcome = app.handle_input(&key_event(KeyCode::Enter, KeyModifiers::NONE)); diff --git a/crates/codegen/kigi-tui/src/app/dispatch/auth.rs b/crates/codegen/kigi-tui/src/app/dispatch/auth.rs index 0abd647..4fb685d 100644 --- a/crates/codegen/kigi-tui/src/app/dispatch/auth.rs +++ b/crates/codegen/kigi-tui/src/app/dispatch/auth.rs @@ -290,7 +290,7 @@ pub(super) fn dispatch_cancel_platform_key_entry(app: &mut AppView) -> Vec]` in config.toml, then authenticate with the platform's +/// auth.json (platform-id scope), then authenticate with the platform's /// method id (one sequential background task — see the effect handler). /// The screen shows the connecting state while the key is validated; a /// failure lands back on the picker with the error line (`AuthFailed`). diff --git a/crates/codegen/kigi-tui/src/app/dispatch/tests/auth.rs b/crates/codegen/kigi-tui/src/app/dispatch/tests/auth.rs index eb1bf1a..e0a322f 100644 --- a/crates/codegen/kigi-tui/src/app/dispatch/tests/auth.rs +++ b/crates/codegen/kigi-tui/src/app/dispatch/tests/auth.rs @@ -112,14 +112,14 @@ fn submit_platform_api_key_dispatches_persist_then_authenticate() { app.auth_state = AuthState::Pending { error: None }; let effects = dispatch( - Action::BeginPlatformKeyEntry(PlatformLogin::MoonshotCn), + Action::BeginPlatformKeyEntry(PlatformLogin(kigi_shell::models::PlatformId::MoonshotCn)), &mut app, ); assert!(effects.is_empty(), "entering key entry is UI-only"); let seq = match &app.auth_state { AuthState::Authenticating { request_seq, - mode: AuthMode::ApiKeyEntry(PlatformLogin::MoonshotCn), + mode: AuthMode::ApiKeyEntry(PlatformLogin(kigi_shell::models::PlatformId::MoonshotCn)), .. } => *request_seq, other => panic!("expected ApiKeyEntry(MoonshotCn), got {other:?}"), @@ -135,7 +135,10 @@ fn submit_platform_api_key_dispatches_persist_then_authenticate() { }, ] => { assert_eq!(*request_seq, seq); - assert_eq!(*target, PlatformLogin::MoonshotCn); + assert_eq!( + *target, + PlatformLogin(kigi_shell::models::PlatformId::MoonshotCn) + ); assert_eq!(key, "sk-test-key"); assert_eq!( target.method_id().0.as_ref(), @@ -179,7 +182,7 @@ fn cancel_platform_key_entry_returns_to_picker() { let mut app = test_app(); app.auth_state = AuthState::Pending { error: None }; dispatch( - Action::BeginPlatformKeyEntry(PlatformLogin::MoonshotAi), + Action::BeginPlatformKeyEntry(PlatformLogin(kigi_shell::models::PlatformId::MoonshotAi)), &mut app, ); app.auth_code_input = "sk-half-typed".into(); @@ -200,7 +203,7 @@ fn submit_platform_api_key_ignores_blank_key() { let mut app = test_app(); app.auth_state = AuthState::Pending { error: None }; dispatch( - Action::BeginPlatformKeyEntry(PlatformLogin::MoonshotCn), + Action::BeginPlatformKeyEntry(PlatformLogin(kigi_shell::models::PlatformId::MoonshotCn)), &mut app, ); let effects = dispatch(Action::SubmitPlatformApiKey(" ".into()), &mut app); @@ -208,7 +211,7 @@ fn submit_platform_api_key_ignores_blank_key() { assert!(matches!( app.auth_state, AuthState::Authenticating { - mode: AuthMode::ApiKeyEntry(PlatformLogin::MoonshotCn), + mode: AuthMode::ApiKeyEntry(PlatformLogin(kigi_shell::models::PlatformId::MoonshotCn)), .. } )); diff --git a/crates/codegen/kigi-tui/src/views/welcome/mod.rs b/crates/codegen/kigi-tui/src/views/welcome/mod.rs index 4996f4b..b3be894 100644 --- a/crates/codegen/kigi-tui/src/views/welcome/mod.rs +++ b/crates/codegen/kigi-tui/src/views/welcome/mod.rs @@ -1218,12 +1218,13 @@ fn render_welcome_authenticating( } AuthMode::ApiKeyEntry(target) => { - // Moonshot API-key paste box: instruction + input + hints. No + // Platform API-key paste box: instruction + input + hints. No // auth-URL machinery — the key comes from the platform console. let h_pad: u16 = content_area.width / 6; let inner_width = content_area.width.saturating_sub(h_pad * 2).max(1); let instruction = format!( - "Paste your Moonshot API key (from {})", + "Paste your {} API key (from {})", + target.vendor(), target.console_host() ); let msg_height = (instruction.len() as u16).div_ceil(inner_width); @@ -2136,7 +2137,9 @@ mod tests { &theme, logo_line_count(area.height), None, // auth_url — none in key-entry mode - AuthMode::ApiKeyEntry(crate::app::app_view::PlatformLogin::MoonshotCn), + AuthMode::ApiKeyEntry(crate::app::app_view::PlatformLogin( + kigi_shell::models::PlatformId::MoonshotCn, + )), "", // auth_code_input false, // clipboard_copied false, // show_raw_url