fix(setup): remove redundant LLM config and API keys from bootstrap .env (#1448)

* fix(setup): remove redundant LLM vars and API keys from bootstrap .env Only true chicken-and-egg vars belong in ~/.ironclaw/.env — things needed to connect to the DB or decrypt secrets (DATABASE_BACKEND, DATABASE_URL, LIBSQL_PATH, SECRETS_MASTER_KEY, ONBOARD_COMPLETED). LLM settings (LLM_BACKEND, LLM_BASE_URL, OLLAMA_BASE_URL, model name, provider-specific URLs) are persisted to the DB via persist_settings() and loaded by Config::from_db_with_toml() after connection. API keys are stored encrypted in the secrets DB and injected via inject_llm_keys_from_secrets(). Writing them as plaintext to .env was redundant and a security regression. Also fixes for_model_discovery() and build_nearai_model_fetch_config() to use env_or_override() instead of std::env::var(), so they can read NEARAI_API_KEY from the thread-safe overlay during the onboarding wizard (where inject_single_var() sets the key after the user enters it). Also fixes incorrect secret names in README (anthropic_api_key → llm_anthropic_api_key, openai_api_key → llm_openai_api_key). Supersedes #266 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: add missing fallback_deliverable field to job_monitor tests Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * docs: address review comments on bootstrap .env and README - Update write_bootstrap_env() docstring to reflect current behavior (no LLM vars, no credentials) - Fix Layer 1 .env examples in README to remove LLM_BACKEND/LLM_BASE_URL - Fix legacy secret name in README example (anthropic_api_key → llm_anthropic_api_key) - Document channel/sandbox vars in bootstrap vars list - Add cleanup comment in test explaining empty-value-as-unset behavior Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
2026-08-25 14:53:34 +00:00 · 2026-03-20 14:07:19 -07:00
parent d3b69e7be3
commit 9603fefd01
4 changed files with 68 additions and 92 deletions
@@ -246,8 +246,8 @@ impl NearAiConfig {
        } else {
            "https://private.near.ai"
        };
-        let base_url =
-            std::env::var("NEARAI_BASE_URL").unwrap_or_else(|_| default_base.to_string());
+        let base_url = crate::config::helpers::env_or_override("NEARAI_BASE_URL")
+            .unwrap_or_else(|| default_base.to_string());

        Self {
            model: String::new(),
@@ -332,8 +332,8 @@ pub(crate) async fn fetch_openai_compatible_models(
 /// Uses [`NearAiConfig::for_model_discovery()`] to construct a minimal NEAR AI
 /// config, then wraps it in an `LlmConfig` with session config for auth.
 pub(crate) fn build_nearai_model_fetch_config() -> crate::config::LlmConfig {
-    let auth_base_url =
-        std::env::var("NEARAI_AUTH_URL").unwrap_or_else(|_| "https://private.near.ai".to_string());
+    let auth_base_url = crate::config::helpers::env_or_override("NEARAI_AUTH_URL")
+        .unwrap_or_else(|| "https://private.near.ai".to_string());

    crate::config::LlmConfig {
        backend: "nearai".to_string(),
@@ -216,8 +216,8 @@ env-var mode or skipped secrets.
 |----------|-------------|-------------|---------|
 | NEAR AI Chat | Browser OAuth or session token | - | `NEARAI_SESSION_TOKEN` |
 | NEAR AI Cloud | API key | `llm_nearai_api_key` | `NEARAI_API_KEY` |
-| Anthropic | API key | `anthropic_api_key` | `ANTHROPIC_API_KEY` |
-| OpenAI | API key | `openai_api_key` | `OPENAI_API_KEY` |
+| Anthropic | API key | `llm_anthropic_api_key` | `ANTHROPIC_API_KEY` |
+| OpenAI | API key | `llm_openai_api_key` | `OPENAI_API_KEY` |
 | Ollama | None | - | - |
 | OpenRouter | API key | `llm_openrouter_api_key` | `OPENROUTER_API_KEY` |
 | OpenAI-compatible | Optional API key | `llm_compatible_api_key` | `LLM_API_KEY` |
@@ -406,26 +406,24 @@ Contains only the settings needed BEFORE database connection. Written by
 ```env
 DATABASE_BACKEND="libsql"
 LIBSQL_PATH="/Users/name/.ironclaw/ironclaw.db"
-LLM_BACKEND="openai_compatible"
-LLM_BASE_URL="http://my-vllm:8000/v1"
+SECRETS_MASTER_KEY="..."   # only if env key source selected
+ONBOARD_COMPLETED="true"
 ```

-Or for PostgreSQL + NEAR AI:
+Or for PostgreSQL:
 ```env
 DATABASE_BACKEND="postgres"
 DATABASE_URL="postgres://user:pass@localhost/ironclaw"
-LLM_BACKEND="nearai"
-```
-
-Or for Ollama:
-```env
-LLM_BACKEND="ollama"
-OLLAMA_BASE_URL="http://localhost:11434"
+SECRETS_MASTER_KEY="..."
+ONBOARD_COMPLETED="true"
 ```

 **Why separate?** Chicken-and-egg: you need `DATABASE_BACKEND` to know
-which database to connect to, and `LLM_BACKEND` to know whether to
-attempt NEAR AI session auth -- neither can be stored in the database.
+which database to connect to, and `SECRETS_MASTER_KEY` to decrypt the
+secrets store — neither can be stored in the database. LLM settings
+(`LLM_BACKEND`, base URLs, model names) are persisted to the DB via
+`persist_settings()` and loaded after connection. API keys are stored
+encrypted in the secrets DB.

 **Layer 2: Database settings table** (everything else)

@@ -487,16 +485,20 @@ Final step of the wizard:
 4. Print configuration summary
 ```

-Bootstrap vars written to `~/.ironclaw/.env`:
+Bootstrap vars written to `~/.ironclaw/.env` (only true chicken-and-egg vars
+that are needed before the DB is connected):
 - `DATABASE_BACKEND` (always)
 - `DATABASE_URL` (if postgres)
 - `LIBSQL_PATH` (if libsql)
 - `LIBSQL_URL` (if turso sync)
- `LLM_BACKEND` (always, when set)
- `LLM_BASE_URL` (if openai_compatible)
- `OLLAMA_BASE_URL` (if ollama)
- `NEARAI_API_KEY` (if API key auth path)
+- `SECRETS_MASTER_KEY` (if env key source selected in Step 2)
 - `ONBOARD_COMPLETED` (always, "true")
+- Channel/sandbox vars: `CLAUDE_CODE_ENABLED`, `SIGNAL_HTTP_URL`, `SIGNAL_ACCOUNT`, etc. (channel init may precede DB)
+
+LLM settings (`LLM_BACKEND`, `LLM_BASE_URL`, model, API keys) are persisted
+to the DB via `persist_settings()` and loaded by `Config::from_db_with_toml()`
+after connection. API keys are stored encrypted in the secrets DB and injected
+via `inject_llm_keys_from_secrets()`.

 **Invariant:** Both Layer 1 and Layer 2 must be written. If the database
 write fails, the wizard returns an error and the `.env` file is not written.
@@ -586,7 +588,7 @@ in the database `secrets` table. The wizard writes secrets like:
 ```
 telegram_bot_token    → encrypted bot token
 telegram_webhook_secret → encrypted webhook HMAC secret
-anthropic_api_key     → encrypted API key
+llm_anthropic_api_key → encrypted API key
 ```

 ---
@@ -2654,16 +2654,17 @@ impl SetupWizard {

    /// Write bootstrap environment variables to `~/.ironclaw/.env`.
    ///
-    /// These are the chicken-and-egg settings needed before the database is
-    /// connected (DATABASE_BACKEND, DATABASE_URL, LLM_BACKEND, etc.).
+    /// Only true chicken-and-egg settings are written here — things needed
+    /// before the database is connected: `DATABASE_BACKEND`, `DATABASE_URL`,
+    /// `LIBSQL_PATH`, `SECRETS_MASTER_KEY`, `ONBOARD_COMPLETED`, and
+    /// channel config vars (Signal, Claude Code sandbox).
    ///
-    /// **Credentials are NOT written here.** API keys and OAuth tokens live
-    /// only in the encrypted secrets DB. `LlmConfig::resolve()` defers
-    /// gracefully when credentials are missing during early startup, and the
-    /// re-resolution in `AppBuilder::build_all()` fills them in after
-    /// `inject_llm_keys_from_secrets()` loads from encrypted storage.
+    /// **LLM settings and credentials are NOT written here.** `LLM_BACKEND`,
+    /// base URLs, and model names are persisted to the DB via
+    /// `persist_settings()` and loaded by `Config::from_db_with_toml()`.
+    /// API keys live only in the encrypted secrets DB and are injected via
+    /// `inject_llm_keys_from_secrets()` after DB init.
    fn write_bootstrap_env(&self) -> Result<(), SetupError> {
-        let registry = crate::llm::ProviderRegistry::load();
        let mut env_vars: Vec<(String, String)> = Vec::new();

        if let Some(ref backend) = self.settings.database_backend {
@@ -2679,66 +2680,6 @@ impl SetupWizard {
            env_vars.push(("LIBSQL_URL".to_string(), url.clone()));
        }

-        // LLM bootstrap vars: same chicken-and-egg problem as DATABASE_BACKEND.
-        // Config::from_env() needs the backend before the DB is connected.
-        if let Some(ref backend) = self.settings.llm_backend {
-            env_vars.push(("LLM_BACKEND".to_string(), backend.clone()));
-        }
-        if let Some(ref url) = self.settings.openai_compatible_base_url {
-            env_vars.push(("LLM_BASE_URL".to_string(), url.clone()));
-        }
-        if let Some(ref url) = self.settings.ollama_base_url {
-            env_vars.push(("OLLAMA_BASE_URL".to_string(), url.clone()));
-        }
-        if let Some(ref region) = self.settings.bedrock_region {
-            env_vars.push(("BEDROCK_REGION".to_string(), region.clone()));
-        }
-        if self.settings.llm_backend.as_deref() == Some("bedrock") {
-            if let Some(ref model) = self.settings.selected_model {
-                env_vars.push(("BEDROCK_MODEL".to_string(), model.clone()));
-            }
-            if let Some(ref cross) = self.settings.bedrock_cross_region {
-                env_vars.push(("BEDROCK_CROSS_REGION".to_string(), cross.clone()));
-            }
-            if let Some(ref profile) = self.settings.bedrock_profile {
-                env_vars.push(("AWS_PROFILE".to_string(), profile.clone()));
-            }
-        }
-
-        // Model name: same chicken-and-egg — Config::from_env() resolves the
-        // model before the DB is connected, so we must persist it to .env.
-        // Write the backend-specific env var so the correct resolution path
-        // picks it up (looked up from the provider registry).
-        // Bedrock model is already written above as BEDROCK_MODEL, skip here.
-        if self.settings.llm_backend.as_deref() != Some("bedrock")
-            && let Some(ref model) = self.settings.selected_model
-        {
-            let backend_str = self.settings.llm_backend.as_deref().unwrap_or("nearai");
-            let model_env = registry.model_env_var(backend_str);
-            env_vars.push((model_env.to_string(), model.clone()));
-        }
-
-        // Also write provider-specific base URL env var if the provider
-        // defines one (e.g., GROQ doesn't need LLM_BASE_URL since its
-        // default is compiled in, but it doesn't hurt to be explicit).
-        if let Some(ref backend) = self.settings.llm_backend
-            && let Some(def) = registry.find(backend)
-            && let Some(ref base_url_env) = def.base_url_env
-            && let Some(ref base_url) = def.default_base_url
-            && base_url_env != "LLM_BASE_URL"
-            && base_url_env != "OLLAMA_BASE_URL"
-        {
-            env_vars.push((base_url_env.clone(), base_url.clone()));
-        }
-
-        // Preserve NEARAI_API_KEY if present (set by API key auth flow
-        // via the thread-safe runtime env overlay).
-        if let Some(api_key) = crate::config::helpers::env_or_override("NEARAI_API_KEY")
-            && !api_key.is_empty()
-        {
-            env_vars.push(("NEARAI_API_KEY".to_string(), api_key));
-        }
-
        // Secrets master key (env var mode): write to .env so it's available
        // on next startup before the DB is connected.
        if let Some(ref key_hex) = self.settings.secrets_master_key_hex {
@@ -3924,6 +3865,39 @@ mod tests {
        );
    }

+    /// Regression: API key set via inject_single_var (the path used by
+    /// setup_api_key_provider during onboarding) must be picked up by
+    /// for_model_discovery() so model listing uses cloud-api auth
+    /// instead of falling back to session-token auth.
+    #[test]
+    fn test_model_discovery_picks_up_injected_var() {
+        use secrecy::ExposeSecret;
+
+        let _lock = ENV_MUTEX.lock().unwrap();
+        let _guard = EnvGuard::clear("NEARAI_API_KEY");
+        let _guard2 = EnvGuard::clear("NEARAI_BASE_URL");
+
+        crate::config::inject_single_var("NEARAI_API_KEY", "injected-wizard-key");
+        let config = build_nearai_model_fetch_config();
+
+        // Clean up: empty values are treated as unset by env_or_override()
+        // at every layer (real env, runtime overrides, INJECTED_VARS).
+        crate::config::inject_single_var("NEARAI_API_KEY", "");
+
+        assert!(
+            config.nearai.api_key.is_some(),
+            "for_model_discovery must read NEARAI_API_KEY from inject_single_var overlay"
+        );
+        assert_eq!(
+            config.nearai.api_key.as_ref().unwrap().expose_secret(),
+            "injected-wizard-key"
+        );
+        assert_eq!(
+            config.nearai.base_url, "https://cloud-api.near.ai",
+            "API key from overlay must select cloud-api base URL"
+        );
+    }
+
    /// Regression: API key set via set_runtime_env (interactive api_key_login
    /// path) must be picked up by build_nearai_model_fetch_config so that
    /// model listing doesn't fall back to session-token auth and re-trigger