From 2ca56f2cd364588d9b658e67c1b9ee8d84127c71 Mon Sep 17 00:00:00 2001 From: Sephyi Date: Sun, 19 Apr 2026 18:51:05 +0200 Subject: [PATCH] fix(security): enforce loopback-only ollama_host per PRD SR-001 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous validation only checked that `ollama_host` started with an `http://` or `https://` scheme. A malicious or compromised config could point it at any remote URL, redirecting staged diff traffic (source code, potentially sensitive context) to an attacker-controlled host. Parse the configured URL with the `url` crate and reject any host that is not: - `127.0.0.0/8` IPv4 loopback range (via `Ipv4Addr::is_loopback`) - `::1` IPv6 loopback (via `Ipv6Addr::is_loopback`) - the literal string `localhost` (ASCII case-insensitive) DNS is intentionally *not* resolved — a hostname other than `localhost` is rejected even if it would resolve to a loopback address, because the resolver cannot be trusted at config time and a local DNS hijack could otherwise bypass the check. Existing integration tests that build URLs from `wiremock::MockServer` continue to pass because mock servers bind to `127.0.0.1`, and those tests construct `Config` directly without going through `Config::load`. Add 11 unit tests covering localhost (mixed case), 127.0.0.1, 127.x.x.x, [::1], https scheme, public IPv4 (8.8.8.8), RFC1918 (192.168.x.x), public hostname, public IPv6 (2001:db8::1), and malformed URL rejection. Closes audit entry F-030 from #3. --- Cargo.lock | 1 + Cargo.toml | 3 ++ src/config.rs | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 8aa0e1e..1416452 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -453,6 +453,7 @@ dependencies = [ "tree-sitter-ruby", "tree-sitter-rust", "tree-sitter-typescript", + "url", "wiremock", ] diff --git a/Cargo.toml b/Cargo.toml index 5ecc876..152a183 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,6 +52,9 @@ figment = { version = "0.10", features = ["toml", "env"] } # HTTP client reqwest = { version = "0.13", features = ["json", "stream"] } +# URL parsing (loopback enforcement for ollama_host per SR-001) +url = "2.5" + # Git (pure Rust) - minimal features gix = { version = "0.80", default-features = false, features = ["revision"] } diff --git a/src/config.rs b/src/config.rs index 69693bb..75fbf7a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -8,8 +8,10 @@ use figment::providers::{Env, Format, Serialized, Toml}; use secrecy::SecretString; use serde::{Deserialize, Serialize}; use std::fs; +use std::net::IpAddr; use std::path::PathBuf; use tracing::warn; +use url::{Host, Url}; use crate::cli::Cli; use crate::error::{Error, Result}; @@ -453,6 +455,8 @@ impl Config { ))); } + validate_ollama_host_is_loopback(&self.ollama_host)?; + // Validate cloud provider base URLs if let Some(ref url) = self.openai_base_url && !url.starts_with("http://") @@ -725,3 +729,113 @@ impl Config { out } } + +/// Enforce loopback-only `ollama_host` per PRD SR-001. +/// +/// The Ollama server is expected to run locally; allowing an arbitrary +/// remote URL would let a malicious config redirect staged diff traffic +/// (which can include source code and sensitive content) to an attacker +/// host. Reject any URL whose host is not: +/// +/// - `127.0.0.0/8` IPv4 loopback range +/// - `::1` IPv6 loopback +/// - the literal string `localhost` +/// +/// Note: this does not resolve DNS — a hostname other than `localhost` +/// is rejected even if it would resolve to `127.0.0.1`, because the +/// resolver (and its answers) cannot be trusted at config time. +fn validate_ollama_host_is_loopback(raw: &str) -> Result<()> { + let url = Url::parse(raw) + .map_err(|e| Error::Config(format!("ollama_host is not a valid URL: {e} (got '{raw}')")))?; + + let host = url.host().ok_or_else(|| { + Error::Config(format!( + "ollama_host must include a host component, got '{raw}'" + )) + })?; + + let is_loopback = match host { + Host::Domain(name) => name.eq_ignore_ascii_case("localhost"), + Host::Ipv4(addr) => IpAddr::V4(addr).is_loopback(), + Host::Ipv6(addr) => IpAddr::V6(addr).is_loopback(), + }; + + if !is_loopback { + return Err(Error::Config(format!( + "ollama_host must point to a loopback address (127.x.x.x, ::1, or localhost); \ + got '{host}'" + ))); + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn loopback_accepts_localhost() { + assert!(validate_ollama_host_is_loopback("http://localhost:11434").is_ok()); + } + + #[test] + fn loopback_accepts_localhost_uppercase() { + // Host matching must be case-insensitive to match DNS semantics. + assert!(validate_ollama_host_is_loopback("http://LocalHost:11434").is_ok()); + } + + #[test] + fn loopback_accepts_ipv4_loopback() { + assert!(validate_ollama_host_is_loopback("http://127.0.0.1:11434").is_ok()); + } + + #[test] + fn loopback_accepts_ipv4_loopback_range() { + // Any 127.x.x.x address is loopback per RFC 1122. + assert!(validate_ollama_host_is_loopback("http://127.1.2.3:11434").is_ok()); + } + + #[test] + fn loopback_accepts_ipv6_loopback() { + assert!(validate_ollama_host_is_loopback("http://[::1]:11434").is_ok()); + } + + #[test] + fn loopback_accepts_https_scheme() { + assert!(validate_ollama_host_is_loopback("https://localhost:11434").is_ok()); + } + + #[test] + fn loopback_rejects_public_ipv4() { + let err = validate_ollama_host_is_loopback("http://8.8.8.8:11434").unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("loopback"), "unexpected error: {msg}"); + assert!(msg.contains("8.8.8.8"), "host missing from error: {msg}"); + } + + #[test] + fn loopback_rejects_private_rfc1918() { + // 192.168.x.x is not loopback even though it is a private range. + let err = validate_ollama_host_is_loopback("http://192.168.1.10:11434").unwrap_err(); + assert!(err.to_string().contains("loopback")); + } + + #[test] + fn loopback_rejects_public_hostname() { + let err = validate_ollama_host_is_loopback("http://evil.example.com:11434").unwrap_err(); + assert!(err.to_string().contains("loopback")); + } + + #[test] + fn loopback_rejects_public_ipv6() { + let err = validate_ollama_host_is_loopback("http://[2001:db8::1]:11434").unwrap_err(); + assert!(err.to_string().contains("loopback")); + } + + #[test] + fn loopback_rejects_malformed_url() { + let err = validate_ollama_host_is_loopback("http://").unwrap_err(); + assert!(matches!(err, Error::Config(_))); + } +}