fix(core): keep API keys off other hosts and plain http, and time out silent endpoints

crates/core/src/providers.rs

@@ -71,6 +71,8 @@ struct ProviderConfigFile {
     models: Vec<ProviderModel>,
     #[serde(default)]
     compat: CompatFlagsFile,
+    #[serde(default)]
+    idle_timeout_secs: Option<u64>,
 }
 
 #[derive(Clone, Debug, Deserialize)]
@@ -88,6 +90,9 @@ pub struct ProviderConfig {
     pub headers: BTreeMap<String, String>,
     pub models: Vec<ProviderModel>,
     pub compat: CompatFlags,
+    /// Seconds the endpoint may send nothing before an attempt is given up;
+    /// unset means the client's default.
+    pub idle_timeout_secs: Option<u64>,
 }
 
 #[derive(Clone, Debug, Default, Deserialize)]
@@ -108,6 +113,13 @@ pub struct ActiveEndpoint {
     pub max_tokens: usize,
     pub temperature: Option<f32>,
     pub compat: CompatFlags,
+    /// The provider's `idle_timeout_secs`, when it sets one.
+    pub idle_timeout_secs: Option<u64>,
+    /// Set when a key is configured in settings but withheld from this
+    /// endpoint because it is another server: that the settings key was not
+    /// sent and how to give the provider its own, for the error a 401 from
+    /// it reports.
+    pub key_hint: Option<String>,
 }
 
 pub fn providers_path(data_dir: &Path) -> PathBuf {
@@ -196,6 +208,7 @@ fn parse_providers_file(text: &str) -> Result<BTreeMap<String, ProviderConfig>,
                     headers: raw.headers,
                     models: raw.models,
                     compat: raw.compat.into(),
+                    idle_timeout_secs: raw.idle_timeout_secs,
                 },
             )
         })
@@ -245,6 +258,9 @@ pub(crate) fn check_file(path: &Path) -> Option<Result<(usize, Vec<String>), Str
                 )))
             }
         }
+        if provider.idle_timeout_secs == Some(0) {
+            return Some(Err(format!("provider '{name}' has zero idle_timeout_secs")));
+        }
         for (header, value) in &provider.headers {
             if reqwest::header::HeaderName::from_bytes(header.as_bytes()).is_err() {
                 return Some(Err(format!(
@@ -303,7 +319,7 @@ fn unknown_key_warnings(text: &str) -> Vec<String> {
         let Some(provider) = provider.as_object() else { continue };
         unknown_keys(
             provider,
-            &["base_url", "api_key", "api_key_env", "headers", "models", "compat"],
+            &["base_url", "api_key", "api_key_env", "headers", "models", "compat", "idle_timeout_secs"],
             &format!("in provider '{name}'"),
             &mut out,
         );
@@ -516,11 +532,17 @@ pub fn resolve(settings: &Settings, data_dir: &Path) -> Result<ActiveEndpoint, R
         // Keep room for system + task history; never let max_tokens eat the window.
         let max_allowed = context_tokens.saturating_sub(2048).max(1);
         max_tokens = max_tokens.min(max_allowed);
-        let api_key = resolve_api_key(
-            p.api_key.as_deref(),
-            &p.api_key_env,
-            settings.api_key.as_deref(),
-        );
+        // The settings key was configured for settings.base_url: a provider
+        // with no key of its own inherits it only when it is that server.
+        // Sent anywhere else, it hands one server's credential to another.
+        let (api_key, key_hint) = match provider_key(p.api_key.as_deref(), &p.api_key_env) {
+            Some(key) => (Some(key), None),
+            None => match settings_key(settings.api_key.as_deref()) {
+                Some(key) if same_server(base_url, &settings.base_url) => (Some(key), None),
+                Some(_) => (None, Some(withheld_key_hint(name, p))),
+                None => (None, None),
+            },
+        };
         let headers = expand_headers(&p.headers);
         return Ok(ActiveEndpoint {
             provider: Some(name.clone()),
@@ -532,6 +554,8 @@ pub fn resolve(settings: &Settings, data_dir: &Path) -> Result<ActiveEndpoint, R
             max_tokens,
             temperature: settings.temperature,
             compat: p.compat.clone(),
+            idle_timeout_secs: p.idle_timeout_secs,
+            key_hint,
         });
     }
 
@@ -554,37 +578,43 @@ pub fn resolve(settings: &Settings, data_dir: &Path) -> Result<ActiveEndpoint, R
     Ok(ActiveEndpoint {
         provider: None,
         base_url: base_url.to_string(),
-        api_key: resolve_api_key(None, &[], settings.api_key.as_deref()),
+        api_key: settings_key(settings.api_key.as_deref()),
         headers: Vec::new(),
         model: model.to_string(),
         context_tokens,
         max_tokens,
         temperature: settings.temperature,
         compat: CompatFlags::defaults_for_missing(),
+        idle_timeout_secs: None,
+        key_hint: None,
     })
 }
 
-fn resolve_api_key(
-    provider_key: Option<&str>,
-    provider_env: &[String],
-    settings_key: Option<&str>,
-) -> Option<String> {
-    if let Some(k) = provider_key {
+/// A provider's own key: its `api_key`, else the first non-empty variable
+/// its `api_key_env` names.
+fn provider_key(key: Option<&str>, env: &[String]) -> Option<String> {
+    if let Some(k) = key {
         if let Some(v) = expand_secret(k) {
             if !v.is_empty() {
                 return Some(v);
             }
         }
     }
-    for name in provider_env {
+    for name in env {
         if let Ok(v) = std::env::var(name) {
             let v = v.trim().to_string();
             if !v.is_empty() {
                 return Some(v);
             }
         }
     }
-    if let Some(k) = settings_key {
+    None
+}
+
+/// The key configured for settings.base_url: settings.api_key, else
+/// `OPENMAX_API_KEY`.
+fn settings_key(key: Option<&str>) -> Option<String> {
+    if let Some(k) = key {
         if let Some(v) = expand_secret(k) {
             if !v.is_empty() {
                 return Some(v);
@@ -600,6 +630,31 @@ fn resolve_api_key(
     None
 }
 
+/// Whether two base URLs name the same server: scheme, host, and port
+/// (explicit or the scheme's default) all match. The path does not count.
+/// A URL that does not parse matches nothing.
+fn same_server(a: &str, b: &str) -> bool {
+    let server = |url: &str| {
+        let url = reqwest::Url::parse(url.trim()).ok()?;
+        Some((url.scheme().to_string(), url.host_str()?.to_string(), url.port_or_known_default()?))
+    };
+    server(a).is_some_and(|a| server(b) == Some(a))
+}
+
+/// What a 401 from provider `name` adds when the settings key was withheld
+/// from it: that the settings key was not sent, and how to give the provider
+/// its own. It must hold however the provider authenticates, since one that
+/// sends a credential through its own `headers` gets this hint too.
+fn withheld_key_hint(name: &str, p: &ProviderConfig) -> String {
+    let fix = match p.api_key_env.first() {
+        Some(var) => format!("export {var}, which provider '{name}' reads its key from"),
+        None => format!("give provider '{name}' its own key with api_key_env in ~/.openmax/providers.json"),
+    };
+    format!(
+        " (the key in settings.json or OPENMAX_API_KEY was not sent: it goes only to the server settings.base_url names, by scheme, host, and port; {fix})"
+    )
+}
+
 /// Expand secrets:
 /// - `$$...` → literal starting with `$` (escape)
 /// - `$ENV_VAR` → environment value
@@ -714,6 +769,23 @@ mod tests {
             .unwrap()
             .unwrap_err()
             .contains("invalid JSON"));
+
+        // An idle interval is a known key; zero would end every attempt at once.
+        std::fs::write(
+            &path,
+            r#"{"providers":{"slow":{"base_url":"http://localhost/v1","idle_timeout_secs":1800}}}"#,
+        )
+        .unwrap();
+        assert_eq!(check_file(&path).unwrap().unwrap(), (1, Vec::new()));
+        std::fs::write(
+            &path,
+            r#"{"providers":{"slow":{"base_url":"http://localhost/v1","idle_timeout_secs":0}}}"#,
+        )
+        .unwrap();
+        assert!(check_file(&path)
+            .unwrap()
+            .unwrap_err()
+            .contains("zero idle_timeout_secs"));
         let _ = std::fs::remove_dir_all(dir);
     }
 
@@ -889,6 +961,60 @@ mod tests {
         let _ = std::fs::remove_dir_all(dir);
     }
 
+    /// The bug this guards: a provider with no key of its own was handed the
+    /// key settings.json holds for settings.base_url, whatever host the
+    /// provider names, so one server's credential went to another. Only the
+    /// same scheme, host, and port inherits it (the path does not matter);
+    /// any other provider resolves with no key and a hint for the 401 it
+    /// will get, naming how to give it its own.
+    #[test]
+    fn a_settings_key_is_inherited_only_by_its_own_host() {
+        let dir = std::env::temp_dir().join(format!("openmax-prov-{}", uuid::Uuid::new_v4()));
+        let unset = format!("OPENMAX_TEST_UNSET_{}", uuid::Uuid::new_v4().simple());
+        let sized = r#""models": [{"id": "m", "context_tokens": 8192}]"#;
+        write_providers(
+            &dir,
+            &format!(
+                r#"{{"providers": {{
+                  "same": {{"base_url": "https://API.example.com:443/v2", {sized}}},
+                  "other_host": {{"base_url": "https://other.example.net/v1", {sized}}},
+                  "other_port": {{"base_url": "https://api.example.com:8443/v1", {sized}}},
+                  "plain": {{"base_url": "http://api.example.com/v1", {sized}}},
+                  "unset_env": {{"base_url": "https://other.example.net/v1", "api_key_env": "{unset}", {sized}}}
+                }}}}"#
+            ),
+        );
+        let mut s = Settings {
+            base_url: "https://api.example.com/v1".into(),
+            api_key: Some("sk-settings".into()),
+            model: "m".into(),
+            provider: Some("same".into()),
+            ..Default::default()
+        };
+        let ep = resolve(&s, &dir).unwrap();
+        assert_eq!(ep.api_key.as_deref(), Some("sk-settings"));
+        assert_eq!(ep.key_hint, None);
+
+        for name in ["other_host", "other_port", "plain"] {
+            s.provider = Some(name.into());
+            let ep = resolve(&s, &dir).unwrap();
+            assert_eq!(ep.api_key, None, "{name} is another server");
+            let hint = ep.key_hint.expect("a withheld key explains the 401 it causes");
+            assert!(hint.contains(&format!("provider '{name}'")) && hint.contains("api_key_env"), "{hint}");
+        }
+        // A provider whose own variable is unset is told to set that one.
+        s.provider = Some("unset_env".into());
+        let ep = resolve(&s, &dir).unwrap();
+        assert_eq!(ep.api_key, None);
+        assert!(ep.key_hint.is_some_and(|hint| hint.contains(&unset)));
+
+        // With no settings.base_url, the key belongs to no provider's host.
+        s.provider = Some("same".into());
+        s.base_url.clear();
+        assert_eq!(resolve(&s, &dir).unwrap().api_key, None);
+        let _ = std::fs::remove_dir_all(dir);
+    }
+
     #[test]
     fn unknown_provider_errors() {
         let dir = std::env::temp_dir().join(format!("openmax-prov-{}", uuid::Uuid::new_v4()));

crates/core/src/spec.rs

@@ -639,13 +639,31 @@ Shape: `{"providers": {"<name>": { ... }}}`. Per provider:
   `chat/completions` on it.
 - `api_key` (optional): literal secret. Prefer `api_key_env`.
 - `api_key_env` (optional): env var name, or list of names (first non-empty
-  wins).
+  wins). With neither (or both resolving empty), the provider uses
+  settings.json's `api_key` / `OPENMAX_API_KEY` only when its `base_url` has
+  the same scheme, host, and port as settings `base_url`; any other server
+  gets no key, and a 401 from it names how to give the provider its own.
 - `headers` (optional): extra HTTP headers as a string map.
 - `models` (optional): list of `{"id", "name"?, "context_tokens"?,
   "max_tokens"?}`. Ids are sent unchanged; order is preserved in `/model`.
 - `compat` (optional): wire quirks for picky servers:
   `{"use_max_completion_tokens": false, "send_stream_options": true}` are the
   defaults.
+- `idle_timeout_secs` (optional, default 600): a positive number of seconds
+  (`--check` rejects 0; it never disables the timeout) the endpoint may send
+  nothing at all (no headers, no bytes, no SSE keepalive comment) before the
+  attempt ends. If no response has arrived, or a stream goes silent before
+  any reply text, it is resent; a stream silent after reply text is reported
+  truncated. Raise it for a local server that works through a long prompt
+  silently for longer.
+
+A key, a credential header (one whose name contains auth, key, token,
+secret, password, credential, or cookie, such as Authorization or
+X-API-Key), or user:password in `base_url` never goes over plain http to
+another machine: such a request fails with an error before anything is
+sent. Use https, or a loopback address (127.0.0.1, ::1, localhost) for a
+server on this machine. A redirect is followed only on the same scheme,
+host, and port as `base_url`; one to another server fails the request.
 
 Select a provider with `"provider"` in settings.json, the `--provider` CLI
 option, or `/provider`; `/model` picks provider and model as one pair. This
@@ -703,6 +721,9 @@ Fields (all optional in JSON; an empty `base_url`/`model` or a missing
 - `base_url`: OpenAI-compatible endpoint root (the harness calls
   `chat/completions` on it). No default, never a localhost fallback.
 - `api_key`: literal, or `$ENV_VAR` indirection; `OPENMAX_API_KEY` also works.
+  Sent only to the server `base_url` names (a named provider without a key of
+  its own inherits it only on the same scheme, host, and port), and never over
+  plain http except to a loopback address.
 - `model`: model id sent with every request.
 - `approval_mode`: `auto` | `ask` | `readonly`; default ask. Used only when
   neither this project nor an enclosing one has a saved choice, such as a
@@ -922,9 +943,9 @@ here so a frontend can render what the model sees; `call_id` links it to the
 tool result it rode, or is empty for a note inserted before the next prompt
 like a turn-start receipt), `retry` (attempt, max_attempts, reason: the
 model request is being resent after a transport failure, a 429 or transient
-5xx, or, before any reply text, a stream that died or a stream or reply the
-provider failed with a rate limit, an overload, or a server fault; thinking
-already streamed for that attempt is void), `diff` (call_id,
+5xx, or, before any reply text, a stream that died or went silent, or a
+stream or reply the provider failed with a rate limit, an overload, or a
+server fault; thinking already streamed for that attempt is void), `diff` (call_id,
 path, diff, added, removed), `approval_request` (approval_id, name, summary,
 detail, reason, source_path, source_sha, and an optional `env`), `approval_settled` (approval_id,
 outcome), `refrozen` (tools, skills, changes: the refreeze receipt naming

crates/core/src/types.rs

@@ -164,10 +164,11 @@ pub enum AgentEvent {
     /// The model request is being resent: `attempt` of `max_attempts` goes
     /// out after a backoff, `reason` having ended the previous one (a
     /// transport failure, a 429 or transient 5xx, or, before any reply text,
-    /// a stream that died or a stream or reply the provider failed with a
-    /// rate limit, an overload, or a server fault). Emitted before the wait;
-    /// a cancel during it ends the turn and the attempt never goes out.
-    /// Thinking streamed for the failed attempt is void; no Token preceded it.
+    /// a stream that died or went silent, or a stream or reply the provider
+    /// failed with a rate limit, an overload, or a server fault). Emitted
+    /// before the wait; a cancel during it ends the turn and the attempt
+    /// never goes out. Thinking streamed for the failed attempt is void; no
+    /// Token preceded it.
     Retry { attempt: u32, max_attempts: u32, reason: String },
     Diff { call_id: String, path: String, diff: String, added: usize, removed: usize },
     /// Mutating tool waiting on the user. `detail` is a short args preview

docs/configuration.md

@@ -18,7 +18,16 @@ optional `~/.openmax/providers.json` for a catalog of named endpoints.
 
 `base_url` is the root of your model's HTTP API (the harness calls
 `chat/completions` on it). Set `model` to the id that server expects. Set
-`api_key` to a literal or `$ENV_VAR`, or export `OPENMAX_API_KEY`.
+`api_key` to a literal or `$ENV_VAR`, or export `OPENMAX_API_KEY`. A key never
+crosses plain http to another machine: with a key (or `user:password` in the
+`base_url`) configured and an `http://` `base_url` that is not a loopback
+address (`127.0.0.1`, `::1`, or `localhost`), every request fails with an
+error instead of sending it. Use https, or a
+loopback address for a server on this machine; a server that needs no key works
+over http once none is configured. A redirect is followed only on the same
+scheme, host, and port as `base_url`: one to another server fails the request
+with an error naming that server, since following it would hand the request
+and its headers to a server you did not configure.
 `max_parallel_tools` bounds concurrent read-only tool calls, defaults to 4, and
 is clamped to 1 through 32 at runtime. Mutating, approval-gated, and
 non-batchable calls remain serial.
@@ -146,6 +155,24 @@ Set `"provider"` in settings, use the provider CLI option, or use `/provider`
 when you only want to change the endpoint. Optional `compat` flags cover picky
 gateways (for example `max_completion_tokens` versus `max_tokens`).
 
+A provider takes its key from its own `api_key` or `api_key_env`. Without one,
+it uses the settings `api_key` (or `OPENMAX_API_KEY`) only when its `base_url`
+has the same scheme, host, and port as the settings `base_url`: that key was
+configured for that server. Any other provider gets no key, and a 401 from it
+says how to give that provider its own. A credential in a provider's `headers`
+(a header whose name contains `auth`, `key`, `token`, `secret`, `password`,
+`credential`, or `cookie`, such as `Authorization` or `X-API-Key`) follows the
+same plain-http rule as the key.
+
+A request has no overall deadline, since a local server can spend minutes on a
+long prompt, but an endpoint that sends nothing at all (no response headers, no
+reply bytes, not even an SSE keepalive comment) for 10 minutes ends the
+attempt. If no response arrives, or a stream goes silent before any reply
+text, the attempt is resent like a dropped connection; a stream that goes
+silent after reply text is reported truncated. A provider's
+`idle_timeout_secs` sets a different interval for that provider, in seconds
+(0 does not disable it, and `--check` rejects it).
+
 Open Max works with local servers (Ollama, LM Studio, vLLM, llama.cpp), cloud
 gateways (OpenRouter and similar), and private proxies.