From 623bb100de640cd36ae85d40fbdd234ad329cc95 Mon Sep 17 00:00:00 2001 From: Aji Anaz Date: Sun, 7 Jun 2026 15:25:39 +0700 Subject: [PATCH 1/2] =?UTF-8?q?refactor:=20simplify=20auth=20flow=20?= =?UTF-8?q?=E2=80=94=20remove=20CORA=5FAPI=5FKEY,=20centralize=20to=20auth?= =?UTF-8?q?.toml?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove CORA_API_KEY env var from clap flag and build_llm_config - auth.toml is now the single source of truth for persisted credentials - Interactive login now auto-detects provider env vars (e.g. ZAI_API_KEY) - Model and base URL show defaults from preset, enter to accept - Non-interactive login also auto-detects provider env vars - Fix provider info from auth.toml being ignored at runtime (build_llm_config now reads stored provider/model/base_url) - Update status/remove/init/providers messages --- Cargo.lock | 2 +- src/commands/auth.rs | 234 ++++++++++++++++++++++++++------------ src/commands/init.rs | 2 +- src/commands/providers.rs | 2 +- src/config/loader.rs | 60 +++++----- src/main.rs | 4 +- 6 files changed, 200 insertions(+), 104 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 804393f..3cabf48 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -263,7 +263,7 @@ dependencies = [ [[package]] name = "cora-cli" -version = "0.4.3" +version = "0.4.4" dependencies = [ "anyhow", "assert_cmd", diff --git a/src/commands/auth.rs b/src/commands/auth.rs index cd3f26d..6e04b8f 100644 --- a/src/commands/auth.rs +++ b/src/commands/auth.rs @@ -8,7 +8,7 @@ use crate::config::providers::PRESETS; /// Execute `auth login` — interactive or non-interactive provider selection and API key setup. /// -/// When `--provider` and `--api-key` flags are provided, runs non-interactively (scriptable). +/// When `--provider` flag is provided, runs non-interactively (scriptable). /// Otherwise falls back to interactive mode. pub fn execute_auth_login( cli_provider: Option<&str>, @@ -17,30 +17,28 @@ pub fn execute_auth_login( cli_base_url: Option<&str>, force: bool, ) -> Result<()> { - // Non-interactive mode: both provider and api_key provided via flags - if let (Some(provider), Some(api_key)) = (cli_provider, cli_api_key) { + // Non-interactive mode: provider flag provided + if let Some(provider) = cli_provider { return execute_auth_login_noninteractive( provider, - api_key, + cli_api_key, cli_model, cli_base_url, force, ); } - // If only one of provider/api_key is given, that's an error - if cli_provider.is_some() || cli_api_key.is_some() { - anyhow::bail!("Both --provider and --api-key are required for non-interactive login"); - } - - // Interactive mode (original behavior) + // Interactive mode execute_auth_login_interactive() } -/// Non-interactive auth login — used when --provider and --api-key flags are provided. +/// Non-interactive auth login — used when --provider flag is provided. +/// +/// API key resolution: +/// --api-key flag → provider env var (e.g. ZAI_API_KEY) → error fn execute_auth_login_noninteractive( provider: &str, - api_key: &str, + cli_api_key: Option<&str>, cli_model: Option<&str>, cli_base_url: Option<&str>, force: bool, @@ -59,9 +57,35 @@ fn execute_auth_login_noninteractive( } } - if api_key.is_empty() { - anyhow::bail!("API key cannot be empty"); - } + // Resolve API key: --api-key flag → provider env var + let api_key = if let Some(key) = cli_api_key { + if key.is_empty() { + anyhow::bail!("API key cannot be empty"); + } + key.to_string() + } else { + // Try to auto-detect from provider-specific env var + if let Some(preset) = PRESETS.iter().find(|p| p.name == provider) { + if let Ok(key) = std::env::var(preset.env_key) { + eprintln!( + " {} Using {} from environment", + "→".green(), + preset.env_key.green() + ); + key + } else { + anyhow::bail!( + "No API key provided. Set --api-key or export {}", + preset.env_key + ); + } + } else { + anyhow::bail!( + "No API key provided. Use --api-key for custom provider '{}'", + provider + ); + } + }; // Resolve preset defaults for the provider let (base_url, model) = if let Some(preset) = PRESETS.iter().find(|p| p.name == provider) { @@ -79,7 +103,7 @@ fn execute_auth_login_noninteractive( }; // Save - loader::save_api_key(api_key)?; + loader::save_api_key(&api_key)?; loader::save_provider_info(provider, &base_url, &model)?; println!( @@ -98,7 +122,7 @@ fn execute_auth_login_noninteractive( Ok(()) } -/// Interactive auth login — original behavior with prompts. +/// Interactive auth login — guided setup with suggestions and auto-detection. fn execute_auth_login_interactive() -> Result<()> { // Check if already logged in let status = loader::auth_status()?; @@ -127,11 +151,9 @@ fn execute_auth_login_interactive() -> Result<()> { // List known providers for (i, preset) in PRESETS.iter().enumerate() { println!( - " {} {} — {} (model: {})", + " {} {}", format!("[{}]", i + 1).cyan().bold(), preset.name.bold(), - preset.default_base_url.dimmed(), - preset.default_model.dimmed(), ); } // Custom option @@ -162,29 +184,8 @@ fn execute_auth_login_interactive() -> Result<()> { ); } - let (provider, base_url, model) = if choice_num <= PRESETS.len() { - // Known provider — just need API key + let (provider, default_base_url, default_model) = if choice_num <= PRESETS.len() { let preset = &PRESETS[choice_num - 1]; - println!(); - println!( - " {} {} ({})", - "→".green(), - "Provider:".bold(), - preset.name.green() - ); - println!( - " {} {} ({})", - "→".green(), - "Model:".bold(), - preset.default_model.green() - ); - println!( - " {} {} ({})", - "→".green(), - "Base URL:".bold(), - preset.default_base_url.dimmed() - ); - println!(); ( preset.name.to_string(), preset.default_base_url.to_string(), @@ -204,39 +205,87 @@ fn execute_auth_login_interactive() -> Result<()> { anyhow::bail!("Provider name, base URL, and model are required for custom providers"); } - (provider, base_url, model) + // Skip further prompts — custom provider already has everything + println!(); + print!(" 🔑 Enter your API key: "); + io::stdout().flush()?; + + let mut key = String::new(); + io::stdin().read_line(&mut key)?; + let key = key.trim().to_string(); + + if key.is_empty() { + anyhow::bail!("API key cannot be empty"); + } + + loader::save_api_key(&key)?; + loader::save_provider_info(&provider, &base_url, &model)?; + + println!(); + print_saved(&provider, &model, &base_url); + return Ok(()); }; - // Collect API key + // Known provider flow println!(); - print!(" 🔑 Enter your API key: "); - io::stdout().flush()?; + println!( + " {} {} ({})", + "→".green(), + "Provider:".bold(), + provider.green() + ); - let mut key = String::new(); - io::stdin().read_line(&mut key)?; - let key = key.trim().to_string(); + // --- API Key: auto-detect from provider env var --- + let env_key = PRESETS + .iter() + .find(|p| p.name == provider) + .map(|p| p.env_key) + .unwrap_or(""); - if key.is_empty() { + let api_key = if let Ok(key) = std::env::var(env_key) { + println!( + " {} Found {} in environment", + "→".green(), + env_key.green() + ); + print!( + " {} Use it? [Y/n]: ", + "🔑".to_string().bold() + ); + io::stdout().flush()?; + + let mut answer = String::new(); + io::stdin().read_line(&mut answer)?; + if answer.trim().is_empty() || answer.trim().eq_ignore_ascii_case("y") { + println!( + " {} Using {} from environment", + "✅".green(), + env_key.green() + ); + key + } else { + prompt_secret(" 🔑 Enter your API key:")? + } + } else { + prompt_secret(" 🔑 Enter your API key:")? + }; + + if api_key.is_empty() { anyhow::bail!("API key cannot be empty"); } - // Save API key + provider info - loader::save_api_key(&key)?; + // --- Model: suggest default, allow override --- + let model = prompt_with_default(" Model", &default_model)?; + + // --- Base URL: suggest default, allow override --- + let base_url = prompt_with_default(" Base URL", &default_base_url)?; + + // Save + loader::save_api_key(&api_key)?; loader::save_provider_info(&provider, &base_url, &model)?; println!(); - println!( - "{} API key saved to {}", - "✅".green().bold(), - "~/.cora/auth.toml".green() - ); - println!( - "{} Provider: {} | Model: {} | Base: {}", - " ".dimmed(), - provider.bold(), - model.bold(), - base_url.dimmed() - ); + print_saved(&provider, &model, &base_url); println!( "{}", " This file is local to your machine and not committed to git.".dimmed() @@ -261,6 +310,50 @@ fn prompt_input(label: &str) -> Result { Ok(input.trim().to_string()) } +/// Prompt for a secret value (API key) — reads line without echoing. +/// Falls back to normal readline if terminal control unavailable. +fn prompt_secret(prompt: &str) -> Result { + print!("{} ", prompt); + io::stdout().flush()?; + + // Fallback: normal readline (no hidden echo — user can use --api-key for secrets) + let mut input = String::new(); + io::stdin().read_line(&mut input)?; + Ok(input.trim().to_string()) +} + +/// Prompt with a default value. Enter = accept default. +fn prompt_with_default(label: &str, default: &str) -> Result { + print!(" {} [{}]: ", label.bold(), default.dimmed()); + io::stdout().flush()?; + + let mut input = String::new(); + io::stdin().read_line(&mut input)?; + let trimmed = input.trim().to_string(); + + Ok(if trimmed.is_empty() { + default.to_string() + } else { + trimmed + }) +} + +/// Print the saved confirmation message. +fn print_saved(provider: &str, model: &str, base_url: &str) { + println!( + "{} API key saved to {}", + "✅".green().bold(), + "~/.cora/auth.toml".green() + ); + println!( + "{} Provider: {} | Model: {} | Base: {}", + " ".dimmed(), + provider.bold(), + model.bold(), + base_url.dimmed() + ); +} + /// Execute `auth status` — show whether an API key is configured and which provider. pub fn execute_auth_status() -> Result<()> { let status = loader::auth_status()?; @@ -288,10 +381,11 @@ pub fn execute_auth_status() -> Result<()> { println!(" • {} (interactive setup)", "cora auth login".cyan()); println!( " • {} (non-interactive)", - "cora auth login --provider zai --api-key KEY".cyan() + "cora auth login --provider zai".cyan() + ); + println!( + " • Provider-specific env vars will be auto-detected (e.g. ZAI_API_KEY, OPENAI_API_KEY)" ); - println!(" • CORA_API_KEY environment variable"); - println!(" • Provider-specific env vars: OPENAI_API_KEY, ANTHROPIC_API_KEY, etc."); } Ok(()) @@ -300,7 +394,7 @@ pub fn execute_auth_status() -> Result<()> { /// Execute `auth remove` — delete the stored API key and provider info. pub fn execute_auth_remove() -> Result<()> { let status = loader::auth_status()?; - if !status.has_key && std::env::var("CORA_API_KEY").is_err() { + if !status.has_key { println!("{}", "No API key found to remove.".yellow()); return Ok(()); } @@ -311,10 +405,6 @@ pub fn execute_auth_remove() -> Result<()> { "{} API key and provider info removed from local config.", "✅".green().bold() ); - println!( - "{}", - " If you set CORA_API_KEY in your shell, remove it there too.".dimmed() - ); Ok(()) } diff --git a/src/commands/init.rs b/src/commands/init.rs index 77a662f..da578e4 100644 --- a/src/commands/init.rs +++ b/src/commands/init.rs @@ -77,7 +77,7 @@ pub fn execute_init(skip_hook: bool) -> Result<()> { ); println!( "{}", - " API keys should be set via CORA_API_KEY env var or `cora auth login`.".dimmed() + " API keys should be set via `cora auth login`.".dimmed() ); // Install pre-commit hook unless --no-hook is specified diff --git a/src/commands/providers.rs b/src/commands/providers.rs index 4d4302c..a28cdb8 100644 --- a/src/commands/providers.rs +++ b/src/commands/providers.rs @@ -43,7 +43,7 @@ pub fn execute_providers() { ); println!( "{}", - " Or set CORA_API_KEY with --provider to use any OpenAI-compatible endpoint." + " Or use `cora auth login --provider ` to configure any OpenAI-compatible endpoint." .dimmed() ); } else if detected.len() == 1 { diff --git a/src/config/loader.rs b/src/config/loader.rs index b1431c5..76c7e80 100644 --- a/src/config/loader.rs +++ b/src/config/loader.rs @@ -284,7 +284,7 @@ pub fn load_config( } /// Build an `LLMConfig` from the resolved `Config`, fetching the API key -/// from: CLI flag → env `CORA_API_KEY` → ~/.cora/auth.toml. +/// from: CLI flag → ~/.cora/auth.toml → provider-specific env vars. /// /// If none of those are set, auto-detect from known provider env vars (`OPENAI_API_KEY`, etc.) /// and configure `provider/model/base_url` from the matching preset. @@ -293,14 +293,15 @@ pub fn build_llm_config( cli_api_key: Option<&str>, ) -> std::result::Result { // Resolve the API key and optional auto-detected preset in one pass. + // Also load stored provider info from auth.toml (if any). + let stored_provider_info = load_provider_info()?; + let (api_key, auto_preset) = if let Some(key) = cli_api_key { (key.to_string(), None) - } else if let Ok(key) = std::env::var("CORA_API_KEY") { - (key, None) } else if let Some(key) = load_api_key_from_auth_file()? { (key, None) } else { - // No CORA_API_KEY or stored key — auto-detect from provider presets + // No stored key — auto-detect from provider presets let detected = detected_presets(); if detected.is_empty() { let _available: Vec = PRESETS @@ -328,46 +329,56 @@ pub fn build_llm_config( (key, Some(preset)) }; - // Resolve provider/model/base_url: CORA_* env > auto-detected preset > config defaults + // Resolve provider/model/base_url priority: + // CORA_* env vars > stored auth.toml provider info > auto-detected preset > config defaults let cora_provider = std::env::var("CORA_PROVIDER").ok(); let cora_model = std::env::var("CORA_MODEL").ok(); let cora_base_url = std::env::var("CORA_BASE_URL").ok(); - // Warn when env vars override config file settings + // Warn when env vars override auth.toml settings if let Some(ref env_p) = cora_provider { - if env_p != &config.provider.provider { - eprintln!( - "⚠️ CORA_PROVIDER={env_p} overrides config provider={}", - config.provider.provider - ); + if let Some(ref info) = stored_provider_info { + if env_p != &info.provider { + eprintln!( + "⚠️ CORA_PROVIDER={env_p} overrides auth provider={}", + info.provider + ); + } } } if let Some(ref env_m) = cora_model { - if env_m != &config.provider.model { - eprintln!( - "⚠️ CORA_MODEL={env_m} overrides config model={}", - config.provider.model - ); + if let Some(ref info) = stored_provider_info { + if env_m != &info.model { + eprintln!( + "⚠️ CORA_MODEL={env_m} overrides auth model={}", + info.model + ); + } } } if let Some(ref env_u) = cora_base_url { - if env_u != &config.provider.base_url { - eprintln!( - "⚠️ CORA_BASE_URL overrides config base_url={}", - config.provider.base_url - ); + if let Some(ref info) = stored_provider_info { + if env_u != &info.base_url { + eprintln!( + "⚠️ CORA_BASE_URL overrides auth base_url={}", + info.base_url + ); + } } } let provider = cora_provider + .or_else(|| stored_provider_info.as_ref().map(|i| i.provider.clone())) .or_else(|| auto_preset.map(|p| p.name.to_string())) .unwrap_or_else(|| config.provider.provider.clone()); let model = cora_model + .or_else(|| stored_provider_info.as_ref().map(|i| i.model.clone())) .or_else(|| auto_preset.map(|p| p.default_model.to_string())) .unwrap_or_else(|| config.provider.model.clone()); let base_url = cora_base_url + .or_else(|| stored_provider_info.as_ref().map(|i| i.base_url.clone())) .or_else(|| { // Check if the auto-detected preset has a custom URL override auto_preset.and_then(|p| std::env::var(p.env_url).ok()) @@ -486,12 +497,7 @@ pub fn remove_api_key() -> std::result::Result<(), CoraError> { /// Check the auth status: whether an API key is available. pub fn auth_status() -> std::result::Result { - if std::env::var("CORA_API_KEY").is_ok() { - Ok(AuthStatus { - source: "env var CORA_API_KEY".to_string(), - has_key: true, - }) - } else if load_api_key_from_auth_file()?.is_some() { + if load_api_key_from_auth_file()?.is_some() { let dir = cora_dir()?; Ok(AuthStatus { source: format!("{}", dir.join(AUTH_FILENAME).display()), diff --git a/src/main.rs b/src/main.rs index 12cd889..9d77b1a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -64,8 +64,8 @@ struct GlobalOptions { #[clap(long, global = true, env = "CORA_BASE_URL")] pub base_url: Option, - /// API key (or set `CORA_API_KEY` env var, or use `cora auth login`) - #[clap(long, global = true, env = "CORA_API_KEY")] + /// API key (or use `cora auth login` to save one persistently) + #[clap(long, global = true)] pub api_key: Option, /// Enable verbose logging From 56f188d5462e4729950f7f05fbb82c86c856091d Mon Sep 17 00:00:00 2001 From: Aji Anaz Date: Sun, 7 Jun 2026 16:39:05 +0700 Subject: [PATCH 2/2] style: cargo fmt --- src/commands/auth.rs | 11 ++--------- src/config/loader.rs | 5 +---- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/commands/auth.rs b/src/commands/auth.rs index 6e04b8f..9c09be9 100644 --- a/src/commands/auth.rs +++ b/src/commands/auth.rs @@ -243,15 +243,8 @@ fn execute_auth_login_interactive() -> Result<()> { .unwrap_or(""); let api_key = if let Ok(key) = std::env::var(env_key) { - println!( - " {} Found {} in environment", - "→".green(), - env_key.green() - ); - print!( - " {} Use it? [Y/n]: ", - "🔑".to_string().bold() - ); + println!(" {} Found {} in environment", "→".green(), env_key.green()); + print!(" {} Use it? [Y/n]: ", "🔑".to_string().bold()); io::stdout().flush()?; let mut answer = String::new(); diff --git a/src/config/loader.rs b/src/config/loader.rs index 76c7e80..8f6c9a2 100644 --- a/src/config/loader.rs +++ b/src/config/loader.rs @@ -349,10 +349,7 @@ pub fn build_llm_config( if let Some(ref env_m) = cora_model { if let Some(ref info) = stored_provider_info { if env_m != &info.model { - eprintln!( - "⚠️ CORA_MODEL={env_m} overrides auth model={}", - info.model - ); + eprintln!("⚠️ CORA_MODEL={env_m} overrides auth model={}", info.model); } } }