F-030: fix(security): enforce loopback-only ollama_host#30
Open
Sephyi wants to merge 1 commit intodevelopmentfrom
Open
F-030: fix(security): enforce loopback-only ollama_host#30Sephyi wants to merge 1 commit intodevelopmentfrom
Sephyi wants to merge 1 commit intodevelopmentfrom
Conversation
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.
There was a problem hiding this comment.
Pull request overview
This PR addresses audit finding F-030 by enforcing that the configured ollama_host points only to a loopback address, preventing staged diff traffic from being redirected to arbitrary remote endpoints.
Changes:
- Add loopback-only validation for
ollama_hostduring config validation. - Introduce unit tests covering accepted/rejected loopback host cases.
- Add the
urlcrate dependency to robustly parse and inspect the configured URL.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/config.rs |
Adds ollama_host loopback enforcement via URL parsing and corresponding unit tests. |
Cargo.toml |
Adds url dependency needed for ollama_host validation. |
Cargo.lock |
Updates lockfile to include the new direct dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+748
to
+761
| 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(), | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fix(security): enforce loopback-only ollama_host.
Audit context
Closes audit entry F-030 from #3.
Verification
cargo fmt --checkcargo clippy --all-targets --all-features -- -D warningscargo test --all-targets