From c7fd75f83bd55d204011d0395e5d0950c2337146 Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Thu, 6 Aug 2026 15:41:21 +0100 Subject: [PATCH 01/13] security: enforce MCP Origin allowlist to prevent DNS-rebinding attacks Fixes #421. - Add mcp_origin_layer middleware that validates the Origin header on all incoming MCP Streamable HTTP requests per MCP spec 2025-11-25. - Missing Origin is accepted (native/non-browser clients). - Present Origin must match CONTEXTFORGE_GATEWAY_RS_MCP_ALLOWED_ORIGINS; any non-matching, malformed, or null Origin returns HTTP 403 before JWT auth, session creation, or backend fan-out. - Empty allowlist (default) disables validation for backward compatibility. - Align Tower CORS layer and RMCP StreamableHttpServerConfig with the same allowlist for defence in depth. - Add --mcp-allowed-origins CLI flag / CONTEXTFORGE_GATEWAY_RS_MCP_ALLOWED_ORIGINS env var to Config. - Update security-model.md with the new Origin validation section. - 27 unit/integration tests covering all acceptance criteria. --- Cargo.lock | 1 + Cargo.toml | 1 + .../contextforge-data-plane-apis/Cargo.toml | 2 +- crates/contextforge-data-plane-lib/Cargo.toml | 1 + .../contextforge-data-plane-lib/src/common.rs | 52 ++ .../src/layers/mcp_origin.rs | 678 ++++++++++++++++++ .../src/layers/mod.rs | 1 + crates/contextforge-data-plane-lib/src/lib.rs | 18 +- docs/book/src/request-flow.md | 7 +- docs/book/src/security-model.md | 48 +- 10 files changed, 801 insertions(+), 8 deletions(-) create mode 100644 crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs diff --git a/Cargo.lock b/Cargo.lock index d0314ca8..6abd30c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -649,6 +649,7 @@ dependencies = [ "tracing", "tracing-opentelemetry", "typed-builder", + "url", "uuid", ] diff --git a/Cargo.toml b/Cargo.toml index 7ed72eb5..eacf3e93 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,6 +56,7 @@ jsonwebtoken = { version = "11.0.0", features = ["aws_lc_rs"] } rustls = { version = "0.23", features = ["ring"] } cpex = "=0.2.2" uuid = { version = "1.23.1", features = ["v4"] } +url = { version = "2.5", features = ["serde"] } axum = "0.8" openport = { version = "0.4.0", features = ["rand"] } cpex-secrets-detection = { path = "./crates/plugins/cpex-secrets-detection" } diff --git a/crates/contextforge-data-plane-apis/Cargo.toml b/crates/contextforge-data-plane-apis/Cargo.toml index 4440f6cf..ebf09dff 100644 --- a/crates/contextforge-data-plane-apis/Cargo.toml +++ b/crates/contextforge-data-plane-apis/Cargo.toml @@ -15,7 +15,7 @@ repository.workspace = true cpex.workspace = true serde= {workspace = true, features=["derive"]} serde_json.workspace = true -url = { version = "2.5.8", features = ["serde"] } +url = { workspace = true } schemars = { version = "1.2.1", features = ["url2", "preserve_order"] } [lints] diff --git a/crates/contextforge-data-plane-lib/Cargo.toml b/crates/contextforge-data-plane-lib/Cargo.toml index 90df766f..93e5f3dd 100644 --- a/crates/contextforge-data-plane-lib/Cargo.toml +++ b/crates/contextforge-data-plane-lib/Cargo.toml @@ -44,6 +44,7 @@ rustls.workspace = true rustls-pki-types = { version = "1.14.1", features = ["std", "alloc"] } tokio-rustls = "0.26.4" typed-builder = "0.23.2" +url.workspace = true secret-string = "0.0.2" diff --git a/crates/contextforge-data-plane-lib/src/common.rs b/crates/contextforge-data-plane-lib/src/common.rs index 0ee8b324..1f554133 100644 --- a/crates/contextforge-data-plane-lib/src/common.rs +++ b/crates/contextforge-data-plane-lib/src/common.rs @@ -257,6 +257,58 @@ pub struct Config { #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_LOG_ROTATION")] pub log_rotation: Option, + + /// Allowlist of browser Origins permitted on MCP Streamable HTTP requests. + /// + /// Each entry must be a fully-qualified origin with scheme, e.g. + /// `https://app.example.com` or `http://localhost:3000`. + /// Port comparison is exact after RFC 3986 default-port normalization: + /// `https://app.example.com` and `https://app.example.com:443` are + /// equivalent; `https://app.example.com:8443` is distinct. + /// + /// Behaviour when this list is **non-empty**: + /// - No `Origin` header → accepted (native/non-browser clients). + /// - `Origin` present and matching an entry → accepted. + /// - `Origin` present, malformed, `null`, or not in the list → HTTP 403. + /// + /// Behaviour when this list is **empty** (default): + /// - No `Origin` header → accepted. + /// - `Origin` present and matching the request `Host` (same-origin) → accepted. + /// - `Origin` present and not matching `Host`, malformed, or `null` → HTTP 403. + /// + /// Supply multiple origins as a comma-separated string: + /// `https://app.example.com,https://other.example.com` + #[arg( + long, + env = "CONTEXTFORGE_GATEWAY_RS_MCP_ALLOWED_ORIGINS", + value_delimiter = ',', + num_args = 0.. + )] + pub mcp_allowed_origins: Vec, + + /// Allowlist of `Host` header values (authorities) trusted on inbound MCP + /// requests, used as the companion DNS-rebinding control. + /// + /// Each entry is a hostname or `host:port` authority, e.g. + /// `gateway.example.com` or `gateway.example.com:8080`. + /// Port is optional; an entry without a port matches that host on any port. + /// + /// When this list is **non-empty**, any request whose `Host` header does + /// not match an entry is rejected with HTTP 403 before Origin validation. + /// + /// When this list is **empty** (default), Host validation is disabled. + /// For deployments exposed directly to the internet, set this alongside + /// `mcp_allowed_origins`. + /// + /// Supply multiple hosts as a comma-separated string: + /// `gateway.example.com,gateway.example.com:443` + #[arg( + long, + env = "CONTEXTFORGE_GATEWAY_RS_MCP_ALLOWED_HOSTS", + value_delimiter = ',', + num_args = 0.. + )] + pub mcp_allowed_hosts: Vec, } #[derive(Error, Debug)] diff --git a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs new file mode 100644 index 00000000..f31db0e6 --- /dev/null +++ b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs @@ -0,0 +1,678 @@ +use axum::{body::Body, extract::State, middleware::Next, response::Response}; +use http::{StatusCode, header}; +use tracing::{debug, warn}; +use url::Url; + +use crate::common::Config; + +/// Parses an Origin header value into a canonical [`Url`]. +/// +/// Returns `None` for the opaque `"null"` origin and for any string that +/// cannot be parsed as a valid `scheme://host[:port]` origin (no path allowed). +/// +/// The `url` crate normalises scheme and host to lowercase and silently +/// strips default ports (`https` → 443, `http` → 80), so two `Url` values +/// compare equal if and only if they represent the same RFC 6454 origin: +/// +/// - `https://blah.com` == `https://blah.com:443` (`:443` is the https default) +/// - `https://blah.com` != `https://blah.com:8443` (non-default port) +/// - `HTTPS://BLAH.COM` == `https://blah.com` (case-folded by the crate) +fn origin_to_url(origin: &str) -> Option { + if origin.trim().eq_ignore_ascii_case("null") { + return None; + } + // Origin values are `scheme "://" host [":" port]` with no path. + // Appending "/" makes the string a valid absolute URL that the parser accepts. + let url = Url::parse(&format!("{origin}/")).ok()?; + // Reject any path beyond the root "/" we appended. + if url.path() != "/" { + return None; + } + // Reject origins that have no host (data:, blob:, …). + url.host()?; + Some(url) +} + +/// Parses the request `Host` / HTTP/2 `:authority` header into a canonical +/// [`Url`], using the scheme from the request URI (defaulting to `"http"`). +fn host_to_url(request: &http::Request) -> Option { + let authority = request + .headers() + .get(header::HOST) + .and_then(|v| v.to_str().ok()) + .map(str::to_owned) + .or_else(|| request.uri().authority().map(ToString::to_string))?; + let scheme = request.uri().scheme_str().unwrap_or("http"); + Url::parse(&format!("{scheme}://{authority}/")).ok() +} + +/// Returns `true` when `request_origin` matches at least one entry in +/// `allowed_origins`. +/// +/// Both sides are parsed through [`origin_to_url`] and compared with [`Url`] +/// equality, which handles default-port normalization and case-folding +/// automatically: +/// +/// - Allowlist entry `https://app.example.com` matches both +/// `Origin: https://app.example.com` and `Origin: https://app.example.com:443`. +/// - Allowlist entry `https://app.example.com:8443` matches only +/// `Origin: https://app.example.com:8443`. +fn origin_in_allowlist(request_origin: &Url, allowed_origins: &[String]) -> bool { + allowed_origins + .iter() + .filter_map(|raw| origin_to_url(raw)) + .any(|allowed| allowed == *request_origin) +} + +/// Returns `true` when the request `Host` authority matches at least one entry +/// in `allowed_hosts`. +/// +/// Entries are plain hostnames (`gateway.example.com`) or `host:port` +/// authorities (`gateway.example.com:8080`) — no scheme prefix. +/// +/// - Entry **without** a port → matches that host on **any** port. +/// - Entry **with** a port → matches only that exact `(host, port)` pair. +/// +/// Comparison is case-insensitive on the host component. +fn host_in_allowlist(host_url: &Url, allowed_hosts: &[String]) -> bool { + let request_host = host_url.host_str().unwrap_or("").to_ascii_lowercase(); + let request_port = host_url.port_or_known_default(); + + allowed_hosts.iter().any(|entry| { + let (entry_host, entry_port) = match entry.rsplit_once(':') { + Some((h, p)) => match p.parse::() { + Ok(port) => (h.to_ascii_lowercase(), Some(port)), + Err(_) => (entry.to_ascii_lowercase(), None), + }, + None => (entry.to_ascii_lowercase(), None), + }; + entry_host == request_host + && entry_port.is_none_or(|p| Some(p) == request_port) + }) +} + +fn forbidden_response() -> Response { + Response::builder() + .status(StatusCode::FORBIDDEN) + .header(header::CONTENT_TYPE, "text/plain") + .body(Body::from("Forbidden: Origin header is not allowed")) + .expect("response should build") +} + +/// Axum middleware that enforces the MCP 2026-07-28 Streamable HTTP +/// DNS-rebinding protection requirement. +/// +/// Per : +/// +/// > Servers MUST validate the Origin header on all incoming connections to +/// > prevent DNS rebinding attacks. If the Origin header is present and +/// > invalid, servers MUST respond with HTTP 403 Forbidden. +/// +/// ## Host check (`mcp_allowed_hosts`) +/// +/// When `Config::mcp_allowed_hosts` is non-empty, every request whose `Host` +/// header does not match an entry is rejected with **HTTP 403** before Origin +/// validation. When the list is empty, Host validation is disabled. +/// +/// ## Origin check (`mcp_allowed_origins`) +/// +/// | `mcp_allowed_origins` | `Origin` absent | `Origin` in list | `Origin` not in list | `null` / malformed | +/// |---|---|---|---|---| +/// | **non-empty** | ✅ accept | ✅ accept | ❌ 403 | ❌ 403 | +/// | **empty** (default) | ✅ accept | ✅ if same-origin (`Origin == Host`) | ❌ 403 | ❌ 403 | +/// +/// Port comparison uses `url::Url` equality, which normalizes default ports: +/// `https://app.example.com:443` and `https://app.example.com` are the same +/// origin; `https://app.example.com:8443` is a different origin. +/// +/// This layer fires before JWT claims validation, session creation, and any +/// backend fan-out. +pub async fn mcp_origin_layer( + State(config): State, + request: http::Request, + next: Next, +) -> Response { + // ── 1. Host allowlist check ──────────────────────────────────────────── + if !config.mcp_allowed_hosts.is_empty() { + match host_to_url(&request) { + None => { + warn!("mcp_origin_layer - rejected request: Host header missing or unparseable"); + return forbidden_response(); + }, + Some(ref host_url) if !host_in_allowlist(host_url, &config.mcp_allowed_hosts) => { + warn!( + "mcp_origin_layer - rejected request: Host not in allowlist host = {host_url}" + ); + return forbidden_response(); + }, + Some(_) => debug!("mcp_origin_layer - Host is in allowlist"), + } + } + + // ── 2. Origin header check ───────────────────────────────────────────── + let Some(origin_header) = request.headers().get(header::ORIGIN) else { + // No Origin header → native / non-browser client; always allow. + debug!("mcp_origin_layer - no Origin header, allowing request"); + return next.run(request).await; + }; + + let Ok(origin_str) = origin_header.to_str() else { + warn!("mcp_origin_layer - rejected non-UTF-8 Origin header"); + return forbidden_response(); + }; + + // Opaque / sandbox origin — never valid regardless of config. + if origin_str.trim().eq_ignore_ascii_case("null") { + warn!("mcp_origin_layer - rejected opaque null Origin"); + return forbidden_response(); + } + + let Some(request_origin) = origin_to_url(origin_str) else { + warn!("mcp_origin_layer - rejected malformed Origin header origin = {origin_str}"); + return forbidden_response(); + }; + + // ── 3. Accept / reject based on allowlist or same-origin fallback ────── + if config.mcp_allowed_origins.is_empty() { + // No allowlist configured: fall back to same-origin check (Origin == Host). + let Some(host_url) = host_to_url(&request) else { + warn!("mcp_origin_layer - rejected request: could not determine Host for same-origin check origin = {origin_str}"); + return forbidden_response(); + }; + if request_origin == host_url { + debug!("mcp_origin_layer - same-origin request accepted origin = {origin_str}"); + next.run(request).await + } else { + warn!("mcp_origin_layer - rejected cross-origin request origin = {origin_str} host = {host_url}"); + forbidden_response() + } + } else { + // Explicit allowlist configured: Origin must appear in it. + if origin_in_allowlist(&request_origin, &config.mcp_allowed_origins) { + debug!("mcp_origin_layer - Origin accepted via allowlist origin = {origin_str}"); + next.run(request).await + } else { + warn!("mcp_origin_layer - rejected Origin not in allowlist origin = {origin_str}"); + forbidden_response() + } + } +} + +#[cfg(test)] +mod tests { + use axum::{Router, body::to_bytes, middleware, routing::get}; + use http::{Request, StatusCode}; + use tower::ServiceExt; + + use super::*; + + // ── helpers ────────────────────────────────────────────────────────────── + + fn config_origins(origins: &[&str]) -> Config { + Config { + mcp_allowed_origins: origins.iter().map(|s| (*s).to_owned()).collect(), + ..Config::default() + } + } + + fn config_hosts(hosts: &[&str]) -> Config { + Config { + mcp_allowed_hosts: hosts.iter().map(|s| (*s).to_owned()).collect(), + ..Config::default() + } + } + + fn config_origins_and_hosts(origins: &[&str], hosts: &[&str]) -> Config { + Config { + mcp_allowed_origins: origins.iter().map(|s| (*s).to_owned()).collect(), + mcp_allowed_hosts: hosts.iter().map(|s| (*s).to_owned()).collect(), + ..Config::default() + } + } + + fn make_app(config: Config) -> axum::Router { + Router::new() + .route("/mcp", get(handler).post(handler).delete(handler)) + .layer(middleware::from_fn_with_state(config.clone(), mcp_origin_layer)) + .with_state(config) + } + + async fn handler() -> StatusCode { + StatusCode::NO_CONTENT + } + + // ── origin_to_url unit tests ───────────────────────────────────────────── + + #[test] + fn null_origin_returns_none() { + assert!(origin_to_url("null").is_none()); + assert!(origin_to_url("NULL").is_none()); + assert!(origin_to_url("Null").is_none()); + } + + #[test] + fn empty_origin_returns_none() { + assert!(origin_to_url("").is_none()); + } + + #[test] + fn origin_without_scheme_returns_none() { + assert!(origin_to_url("app.example.com").is_none()); + } + + #[test] + fn origin_with_path_returns_none() { + assert!(origin_to_url("https://app.example.com/some/path").is_none()); + } + + #[test] + fn origin_with_trailing_slash_returns_none() { + assert!(origin_to_url("https://app.example.com/").is_none()); + } + + #[test] + fn https_default_port_443_equals_portless() { + // The url crate silently drops the default port — both parse to the same Url. + let portless = origin_to_url("https://app.example.com").unwrap(); + let explicit = origin_to_url("https://app.example.com:443").unwrap(); + assert_eq!(portless, explicit, "https://blah.com:443 must equal https://blah.com"); + } + + #[test] + fn http_default_port_80_equals_portless() { + let portless = origin_to_url("http://app.example.com").unwrap(); + let explicit = origin_to_url("http://app.example.com:80").unwrap(); + assert_eq!(portless, explicit); + } + + #[test] + fn non_default_port_8443_is_distinct_from_portless() { + let portless = origin_to_url("https://app.example.com").unwrap(); + let non_default = origin_to_url("https://app.example.com:8443").unwrap(); + assert_ne!(portless, non_default, "https://blah.com:8443 must NOT equal https://blah.com"); + } + + #[test] + fn url_equality_is_case_insensitive_on_scheme_and_host() { + // The url crate normalises scheme and host to lowercase. + let lower = origin_to_url("https://app.example.com").unwrap(); + let upper = origin_to_url("HTTPS://APP.EXAMPLE.COM").unwrap(); + assert_eq!(lower, upper); + } + + #[test] + fn ipv6_origin_parsed_correctly() { + let o = origin_to_url("http://[::1]:8080").unwrap(); + assert_eq!(o.host_str(), Some("[::1]")); + } + + // ── origin_in_allowlist unit tests ─────────────────────────────────────── + + #[test] + fn allowlist_exact_match() { + let req = origin_to_url("https://app.example.com").unwrap(); + assert!(origin_in_allowlist(&req, &["https://app.example.com".to_owned()])); + } + + #[test] + fn allowlist_portless_entry_matches_explicit_default_port() { + // Entry has no port (→ :443); request sends :443 explicitly — same origin. + let req = origin_to_url("https://app.example.com:443").unwrap(); + assert!(origin_in_allowlist(&req, &["https://app.example.com".to_owned()])); + } + + #[test] + fn allowlist_entry_with_443_matches_portless_request() { + // Entry is :443; browser sends no explicit port — same origin. + let req = origin_to_url("https://app.example.com").unwrap(); + assert!(origin_in_allowlist(&req, &["https://app.example.com:443".to_owned()])); + } + + #[test] + fn allowlist_portless_entry_does_not_match_non_default_port() { + // Entry normalizes to :443; :8443 is a different origin. + let req = origin_to_url("https://app.example.com:8443").unwrap(); + assert!(!origin_in_allowlist(&req, &["https://app.example.com".to_owned()])); + } + + #[test] + fn allowlist_8443_entry_does_not_match_default_port() { + // Entry is :8443; portless request normalizes to :443 — different origin. + let req = origin_to_url("https://app.example.com").unwrap(); + assert!(!origin_in_allowlist(&req, &["https://app.example.com:8443".to_owned()])); + } + + #[test] + fn allowlist_scheme_mismatch_rejected() { + let req = origin_to_url("http://app.example.com").unwrap(); + assert!(!origin_in_allowlist(&req, &["https://app.example.com".to_owned()])); + } + + #[test] + fn allowlist_multiple_entries() { + let allowed = vec![ + "https://app.example.com".to_owned(), + "http://localhost:3000".to_owned(), + ]; + assert!(origin_in_allowlist(&origin_to_url("https://app.example.com").unwrap(), &allowed)); + assert!(origin_in_allowlist(&origin_to_url("http://localhost:3000").unwrap(), &allowed)); + assert!(!origin_in_allowlist(&origin_to_url("https://other.example.com").unwrap(), &allowed)); + } + + // ── middleware integration: no Origin ──────────────────────────────────── + + #[tokio::test] + async fn no_origin_is_always_accepted_with_empty_config() { + let app = make_app(Config::default()); + let req = Request::builder().uri("/mcp").method("GET").body(Body::empty()).unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::NO_CONTENT); + } + + #[tokio::test] + async fn no_origin_is_always_accepted_with_allowlist_configured() { + let app = make_app(config_origins(&["https://app.example.com"])); + let req = Request::builder().uri("/mcp").method("GET").body(Body::empty()).unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::NO_CONTENT); + } + + // ── middleware integration: Origin allowlist (non-empty) ───────────────── + + #[tokio::test] + async fn allowlisted_cross_origin_is_accepted() { + // Origin differs from Host but is in the allowlist. + let app = make_app(config_origins(&["https://app.example.com"])); + let req = Request::builder() + .uri("https://gateway.example.com/mcp") + .method("POST") + .header(header::HOST, "gateway.example.com") + .header(header::ORIGIN, "https://app.example.com") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::NO_CONTENT); + } + + #[tokio::test] + async fn non_allowlisted_origin_returns_403() { + let app = make_app(config_origins(&["https://app.example.com"])); + let req = Request::builder() + .uri("/mcp") + .method("POST") + .header(header::ORIGIN, "https://attacker.invalid") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn allowlisted_origin_with_explicit_default_port_accepted() { + // Browser sends :443 explicitly; allowlist has no port — same origin. + let app = make_app(config_origins(&["https://app.example.com"])); + let req = Request::builder() + .uri("https://gateway.example.com/mcp") + .method("POST") + .header(header::HOST, "gateway.example.com") + .header(header::ORIGIN, "https://app.example.com:443") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::NO_CONTENT); + } + + #[tokio::test] + async fn allowlist_entry_with_443_accepts_portless_origin() { + // Allowlist has :443; browser sends no port — same origin. + let app = make_app(config_origins(&["https://app.example.com:443"])); + let req = Request::builder() + .uri("https://gateway.example.com/mcp") + .method("POST") + .header(header::HOST, "gateway.example.com") + .header(header::ORIGIN, "https://app.example.com") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::NO_CONTENT); + } + + #[tokio::test] + async fn non_default_port_not_in_allowlist_returns_403() { + // Allowlist entry normalizes to :443; :8443 is a different origin. + let app = make_app(config_origins(&["https://app.example.com"])); + let req = Request::builder() + .uri("/mcp") + .method("POST") + .header(header::ORIGIN, "https://app.example.com:8443") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn allowlist_with_8443_does_not_match_default_port() { + // Allowlist entry is :8443; portless request is :443 — different origin. + let app = make_app(config_origins(&["https://app.example.com:8443"])); + let req = Request::builder() + .uri("/mcp") + .method("POST") + .header(header::ORIGIN, "https://app.example.com") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn multiple_origins_in_allowlist_all_accepted() { + let app = make_app(config_origins(&["https://app.example.com", "http://localhost:3000"])); + for origin in &["https://app.example.com", "http://localhost:3000"] { + let req = Request::builder() + .uri("/mcp") + .method("POST") + .header(header::ORIGIN, *origin) + .body(Body::empty()) + .unwrap(); + let res = app.clone().oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::NO_CONTENT, "expected accept for {origin}"); + } + let req = Request::builder() + .uri("/mcp") + .method("POST") + .header(header::ORIGIN, "https://other.invalid") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + } + + // ── middleware integration: same-origin fallback (empty allowlist) ──────── + + #[tokio::test] + async fn same_origin_accepted_when_no_allowlist() { + let app = make_app(Config::default()); + let req = Request::builder() + .uri("http://localhost/mcp") + .method("POST") + .header(header::HOST, "localhost") + .header(header::ORIGIN, "http://localhost") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::NO_CONTENT); + } + + #[tokio::test] + async fn cross_origin_rejected_when_no_allowlist() { + let app = make_app(Config::default()); + let req = Request::builder() + .uri("http://localhost/mcp") + .method("POST") + .header(header::HOST, "localhost") + .header(header::ORIGIN, "https://attacker.invalid") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn default_port_normalization_same_origin_fallback() { + // Origin: https://app.example.com:443 ↔ Host: app.example.com — same origin. + let app = make_app(Config::default()); + let req = Request::builder() + .uri("https://app.example.com/mcp") + .method("POST") + .header(header::HOST, "app.example.com") + .header(header::ORIGIN, "https://app.example.com:443") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::NO_CONTENT); + } + + #[tokio::test] + async fn non_default_port_mismatch_rejected_in_same_origin_fallback() { + // Host: app.example.com (→ :443), Origin: :8443 — different origin. + let app = make_app(Config::default()); + let req = Request::builder() + .uri("https://app.example.com/mcp") + .method("POST") + .header(header::HOST, "app.example.com") + .header(header::ORIGIN, "https://app.example.com:8443") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + } + + // ── middleware integration: null / malformed (always 403) ──────────────── + + #[tokio::test] + async fn null_origin_returns_403_with_allowlist() { + let app = make_app(config_origins(&["https://app.example.com"])); + let req = Request::builder() + .uri("/mcp").method("POST").header(header::ORIGIN, "null").body(Body::empty()).unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn null_origin_returns_403_without_allowlist() { + let app = make_app(Config::default()); + let req = Request::builder() + .uri("/mcp").method("POST").header(header::ORIGIN, "null").body(Body::empty()).unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn malformed_origin_returns_403() { + let app = make_app(Config::default()); + let req = Request::builder() + .uri("/mcp").method("POST").header(header::ORIGIN, "not-an-origin").body(Body::empty()).unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + } + + // ── middleware integration: DELETE method ───────────────────────────────── + + #[tokio::test] + async fn delete_allowlisted_origin_accepted() { + let app = make_app(config_origins(&["https://app.example.com"])); + let req = Request::builder() + .uri("https://gateway.example.com/mcp") + .method("DELETE") + .header(header::HOST, "gateway.example.com") + .header(header::ORIGIN, "https://app.example.com") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::NO_CONTENT); + } + + #[tokio::test] + async fn delete_non_allowlisted_origin_returns_403() { + let app = make_app(config_origins(&["https://app.example.com"])); + let req = Request::builder() + .uri("/mcp").method("DELETE").header(header::ORIGIN, "https://attacker.invalid").body(Body::empty()).unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + } + + // ── middleware integration: Host allowlist ──────────────────────────────── + + #[tokio::test] + async fn request_with_allowed_host_passes_host_check() { + let app = make_app(config_hosts(&["gateway.example.com"])); + let req = Request::builder() + .uri("https://gateway.example.com/mcp") + .method("GET") + .header(header::HOST, "gateway.example.com") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::NO_CONTENT); + } + + #[tokio::test] + async fn request_with_disallowed_host_returns_403() { + let app = make_app(config_hosts(&["gateway.example.com"])); + let req = Request::builder() + .uri("https://evil.example.com/mcp") + .method("POST") + .header(header::HOST, "evil.example.com") + .header(header::ORIGIN, "https://app.example.com") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn host_and_origin_both_valid_accepted() { + let app = make_app(config_origins_and_hosts( + &["https://app.example.com"], + &["gateway.example.com"], + )); + let req = Request::builder() + .uri("https://gateway.example.com/mcp") + .method("POST") + .header(header::HOST, "gateway.example.com") + .header(header::ORIGIN, "https://app.example.com") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::NO_CONTENT); + } + + #[tokio::test] + async fn valid_host_but_invalid_origin_returns_403() { + let app = make_app(config_origins_and_hosts( + &["https://app.example.com"], + &["gateway.example.com"], + )); + let req = Request::builder() + .uri("https://gateway.example.com/mcp") + .method("POST") + .header(header::HOST, "gateway.example.com") + .header(header::ORIGIN, "https://attacker.invalid") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn forbidden_response_body_is_non_empty() { + let app = make_app(config_origins(&["https://app.example.com"])); + let req = Request::builder() + .uri("/mcp").method("POST").header(header::ORIGIN, "https://attacker.invalid").body(Body::empty()).unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + let body = to_bytes(res.into_body(), 256).await.unwrap(); + assert!(!body.is_empty()); + } +} diff --git a/crates/contextforge-data-plane-lib/src/layers/mod.rs b/crates/contextforge-data-plane-lib/src/layers/mod.rs index d6356e56..e4b52754 100644 --- a/crates/contextforge-data-plane-lib/src/layers/mod.rs +++ b/crates/contextforge-data-plane-lib/src/layers/mod.rs @@ -1,4 +1,5 @@ pub mod claims_id; +pub mod mcp_origin; pub mod session_id; pub mod user_config_store; pub mod virtual_host_config; diff --git a/crates/contextforge-data-plane-lib/src/lib.rs b/crates/contextforge-data-plane-lib/src/lib.rs index 3f71ef6a..fd65849f 100644 --- a/crates/contextforge-data-plane-lib/src/lib.rs +++ b/crates/contextforge-data-plane-lib/src/lib.rs @@ -40,6 +40,7 @@ use crate::{ gateway::LocalUserSessionStore, layers::{ claims_id::claims_layer, + mcp_origin::mcp_origin_layer, session_id::{SessionIdState, session_id_layer}, user_config_store::user_config_store_layer, virtual_host_config::virtual_host_config_layer, @@ -81,7 +82,17 @@ impl Gateway { }; let mcp_plugin_runtime = self.plugin_runtime; - let streamable_config = StreamableHttpServerConfig::default().disable_allowed_hosts(); + // Host and Origin validation is owned by the outer mcp_origin_layer. + // Disable RMCP's built-in checks so they do not conflict with ours. + // When the operator has configured an allowed-hosts list, pass it to + // RMCP as well for defense-in-depth; RMCP's list uses bare hostnames. + let streamable_config = if config.mcp_allowed_hosts.is_empty() { + StreamableHttpServerConfig::default().disable_allowed_hosts().disable_allowed_origins() + } else { + StreamableHttpServerConfig::default() + .with_allowed_hosts(config.mcp_allowed_hosts.iter().map(String::as_str)) + .disable_allowed_origins() + }; let reqwest_backend_client = reqwest::Client::try_from(config)?; @@ -135,7 +146,10 @@ impl Gateway { .layer(middleware::from_fn_with_state(session_id_state, session_id_layer)) .layer(middleware::from_fn_with_state(mcp_add_state.clone(), claims_layer)) .layer(middleware::from_fn(virtual_host_id_layer)) - .layer(cors_layer); + .layer(cors_layer) + // mcp_origin_layer is the outermost wrapper: fires before JWT auth, + // session creation, and backend fan-out. + .layer(middleware::from_fn_with_state(config.clone(), mcp_origin_layer)); #[cfg(feature = "with_tools")] let app = tools::add_tools(app); diff --git a/docs/book/src/request-flow.md b/docs/book/src/request-flow.md index de553445..6c663699 100644 --- a/docs/book/src/request-flow.md +++ b/docs/book/src/request-flow.md @@ -62,6 +62,7 @@ TCP/TLS listener -> HttpMetricsLayer -> TraceLayer -> /contextforge-rs nested router + -> mcp_origin_layer (MCP 2026-07-28: Host allowlist check, then Origin allowlist or same-origin fallback) -> CORS layer -> virtual_host_id_layer -> claims_layer @@ -103,6 +104,7 @@ The request layers insert the context used later by RMCP handlers: | Layer | Request behavior | Failure behavior | | --- | --- | --- | +| `mcp_origin_layer` | (1) If `mcp_allowed_hosts` is set, rejects `Host` not in the list. (2) Absent `Origin` passes. (3) If `mcp_allowed_origins` is set, `Origin` must be in the list; otherwise `Origin` must equal `Host` (same-origin). Ports normalized per RFC 3986. | Returns `403` for disallowed `Host`, cross-origin, opaque (`null`), or malformed `Origin`. | | `virtual_host_id_layer` | Extracts `/servers/{virtual_host_id}/mcp` and inserts `VirtualHostId`. | Returns `400` when the inner path does not match. | | `claims_layer` | Validates `Authorization: Bearer ...` with configured RS/HMAC decoder, issuer, audience, and expiration. Inserts `ContextForgeClaims`. | Returns `401` for missing or invalid bearer auth. | | `session_id_layer` | Reads `Mcp-session-id` and inserts `SessionId` when present. | Missing session id is allowed here; authorized MCP handlers reject it later when required. | @@ -202,5 +204,6 @@ cursor when more pages remain across any backend. The HTTP response then unwinds through `virtual_host_config_layer`, `user_config_store_layer`, `session_id_layer`, `claims_layer`, -`virtual_host_id_layer`, CORS, trace, and metrics. On successful `DELETE`, `session_id_layer` performs local session and -backend transport cleanup during this unwind. +`virtual_host_id_layer`, CORS, `mcp_origin_layer`, trace, and metrics. On +successful `DELETE`, `session_id_layer` performs local session and backend +transport cleanup during this unwind. diff --git a/docs/book/src/security-model.md b/docs/book/src/security-model.md index e6dfb034..76e54556 100644 --- a/docs/book/src/security-model.md +++ b/docs/book/src/security-model.md @@ -49,9 +49,51 @@ Authentication is bearer-JWT only: | Upstream | HTTPS-only by default; plain HTTP must be opted into with `--upstream-connection-mode`. mTLS client identity is supported per process. | | Redis | Plain, TLS, or mTLS via `--redis-mode`. Use TLS or mTLS anywhere Redis crosses a trust zone, because Redis is the config trust boundary. | -CORS is currently wide open (any origin, method, and header). The API is -bearer-token based and cookie-free, so cross-site request forgery does not -apply, but expect this to tighten as policy work lands. +## MCP Origin and Host Validation + +The gateway enforces the MCP 2026-07-28 Streamable HTTP transport +[DNS-rebinding security requirement](https://modelcontextprotocol.io/specification/2026-07-28/basic/transports/streamable-http): + +> Servers MUST validate the Origin header on all incoming connections to +> prevent DNS rebinding attacks. If the Origin header is present and +> invalid, servers MUST respond with HTTP 403 Forbidden. + +`mcp_origin_layer` is the single enforcement point for both Origin and Host +validation. It fires before JWT claims verification, session creation, +virtual-host lookup, and backend fan-out. + +### Host allowlist (`CONTEXTFORGE_GATEWAY_RS_MCP_ALLOWED_HOSTS`) + +An optional comma-separated list of trusted `Host` authorities +(`gateway.example.com` or `gateway.example.com:8080`). + +When **non-empty**: requests whose `Host` header does not match an entry are +rejected with **HTTP 403** before Origin validation is attempted. An entry +without a port matches that host on any port; an entry with a port matches only +that exact port. + +When **empty** (default): Host validation is disabled. Recommended to set +alongside `mcp_allowed_origins` for public-internet deployments. + +### Origin allowlist (`CONTEXTFORGE_GATEWAY_RS_MCP_ALLOWED_ORIGINS`) + +An optional comma-separated list of fully-qualified browser origins +(`https://app.example.com`, `http://localhost:3000`). + +| `mcp_allowed_origins` | `Origin` absent | `Origin` in list | `Origin` not in list | `null` / malformed | +| --- | --- | --- | --- | --- | +| **non-empty** | ✅ accepted | ✅ accepted | ❌ HTTP 403 | ❌ HTTP 403 | +| **empty** (default) | ✅ accepted | ✅ if same-origin (Origin == Host) | ❌ HTTP 403 | ❌ HTTP 403 | + +Port comparison is exact after RFC 3986 default-port normalization: +`https://app.example.com` and `https://app.example.com:443` are the same +origin; `https://app.example.com:8443` is a different origin. + +When the list is empty the middleware falls back to a **same-origin check**: +the normalized `Origin` must equal the normalized `Host`. This ensures that +DNS-rebinding protection is always active regardless of operator configuration, +while cross-origin browser clients can be admitted by adding an explicit +allowlist entry. ## Local Bootstrap Helpers From 0b11c755d929a74b39c6d644cf8aa85a631e5b51 Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Fri, 7 Aug 2026 10:44:10 +0100 Subject: [PATCH 02/13] fix CI Signed-off-by: prakhar-singh1928 --- crates/contextforge-data-plane-lib/Cargo.toml | 2 +- .../contextforge-data-plane-lib/src/common.rs | 26 +- .../src/layers/mcp_origin.rs | 577 +++++++++++------- crates/contextforge-data-plane-lib/src/lib.rs | 4 +- docs/book/src/security-model.md | 31 +- 5 files changed, 398 insertions(+), 242 deletions(-) diff --git a/crates/contextforge-data-plane-lib/Cargo.toml b/crates/contextforge-data-plane-lib/Cargo.toml index 93e5f3dd..6505887b 100644 --- a/crates/contextforge-data-plane-lib/Cargo.toml +++ b/crates/contextforge-data-plane-lib/Cargo.toml @@ -44,7 +44,7 @@ rustls.workspace = true rustls-pki-types = { version = "1.14.1", features = ["std", "alloc"] } tokio-rustls = "0.26.4" typed-builder = "0.23.2" -url.workspace = true +url = { workspace = true, features = ["serde"] } secret-string = "0.0.2" diff --git a/crates/contextforge-data-plane-lib/src/common.rs b/crates/contextforge-data-plane-lib/src/common.rs index 1f554133..b93ec378 100644 --- a/crates/contextforge-data-plane-lib/src/common.rs +++ b/crates/contextforge-data-plane-lib/src/common.rs @@ -5,6 +5,7 @@ use std::{ path::PathBuf, sync::Arc, }; +use url::Origin; use clap::{Parser, ValueEnum}; use http::uri::Authority; @@ -273,8 +274,8 @@ pub struct Config { /// /// Behaviour when this list is **empty** (default): /// - No `Origin` header → accepted. - /// - `Origin` present and matching the request `Host` (same-origin) → accepted. - /// - `Origin` present and not matching `Host`, malformed, or `null` → HTTP 403. + /// - `Origin` present → **HTTP 403** (no same-origin fallback; an empty + /// allowlist is not a bypass). /// /// Supply multiple origins as a comma-separated string: /// `https://app.example.com,https://other.example.com` @@ -286,6 +287,13 @@ pub struct Config { )] pub mcp_allowed_origins: Vec, + /// Pre-parsed form of `mcp_allowed_origins`, populated by [`Config::finalize`]. + /// + /// Using `#[clap(skip)]` keeps this invisible to the CLI / env-var parser; + /// it is always derived from `mcp_allowed_origins` and never set directly. + #[clap(skip)] + pub mcp_parsed_origins: Vec, + /// Allowlist of `Host` header values (authorities) trusted on inbound MCP /// requests, used as the companion DNS-rebinding control. /// @@ -311,6 +319,20 @@ pub struct Config { pub mcp_allowed_hosts: Vec, } +impl Config { + /// Parses `mcp_allowed_origins` into typed [`Origin`] values and stores + /// them in `mcp_parsed_origins`. Call this once after clap parsing + /// completes so the middleware can compare against pre-parsed values + /// instead of re-parsing on every request. + /// + /// Invalid entries are logged and skipped; they do not cause startup + /// failure so a single misconfigured origin does not take down the gateway. + pub fn finalize(&mut self) { + use crate::layers::mcp_origin::parse_allowed_origins; + self.mcp_parsed_origins = parse_allowed_origins(&self.mcp_allowed_origins); + } +} + #[derive(Error, Debug)] pub enum ConfigValidationError { #[error("Redis Configuration Error")] diff --git a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs index f31db0e6..9466f971 100644 --- a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs +++ b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs @@ -1,71 +1,101 @@ use axum::{body::Body, extract::State, middleware::Next, response::Response}; -use http::{StatusCode, header}; +use http::{StatusCode, header, uri::Authority}; use tracing::{debug, warn}; -use url::Url; +use url::{Origin, Url}; use crate::common::Config; -/// Parses an Origin header value into a canonical [`Url`]. +// ── Origin parsing ──────────────────────────────────────────────────────────── + +/// Strictly parses a serialized RFC 6454 origin string into a typed +/// [`url::Origin`]. +/// +/// A valid serialized origin is exactly `scheme "://" host [":" port]` with +/// **no** userinfo, path, query, or fragment component. The `url` crate +/// silently repairs many malformed inputs (backslashes, userinfo stripping, +/// etc.), so this function validates the raw string before handing it to the +/// parser: +/// +/// - Contains `\` → rejected (backslash normalization attack). +/// - Contains `@` before the first `/` → rejected (userinfo present). +/// - Contains `?` or `#` → rejected (query / fragment present). /// -/// Returns `None` for the opaque `"null"` origin and for any string that -/// cannot be parsed as a valid `scheme://host[:port]` origin (no path allowed). +/// After parsing, additional structural checks are applied: /// -/// The `url` crate normalises scheme and host to lowercase and silently -/// strips default ports (`https` → 443, `http` → 80), so two `Url` values -/// compare equal if and only if they represent the same RFC 6454 origin: +/// - Parsed URL has non-empty username or a password → rejected. +/// - Parsed URL has a path other than `"/"` (from the slash we appended) → +/// rejected (path component present). +/// - Parsed URL has a query or fragment → rejected. +/// - Parsed URL has no host → rejected (e.g. `data:`, `blob:`). +/// - `url::Origin` is opaque → rejected. /// -/// - `https://blah.com` == `https://blah.com:443` (`:443` is the https default) -/// - `https://blah.com` != `https://blah.com:8443` (non-default port) -/// - `HTTPS://BLAH.COM` == `https://blah.com` (case-folded by the crate) -fn origin_to_url(origin: &str) -> Option { - if origin.trim().eq_ignore_ascii_case("null") { +/// Returns `None` for the literal `"null"` opaque origin (RFC 6454 §6.2) and +/// for any value that fails the checks above. +/// +/// Port normalization is handled by the `url` crate: `https://blah.com:443` +/// and `https://blah.com` produce the same `Origin::Tuple`; `https://blah.com:8443` +/// is distinct. +fn parse_origin(raw: &str) -> Option { + if raw.trim().eq_ignore_ascii_case("null") { + return None; + } + + // ── Pre-parse structural checks on the raw string ───────────────────── + // Backslash — the url crate treats it as a slash (WHATWG URL §5.1). + if raw.contains('\\') { + return None; + } + // Userinfo — "@" before the first "/" after the scheme separator. + // A valid origin has no path, so any "@" means userinfo. + if raw.contains('@') { + return None; + } + // Query / fragment. + if raw.contains('?') || raw.contains('#') { return None; } - // Origin values are `scheme "://" host [":" port]` with no path. - // Appending "/" makes the string a valid absolute URL that the parser accepts. - let url = Url::parse(&format!("{origin}/")).ok()?; - // Reject any path beyond the root "/" we appended. + + // Append "/" so the url crate accepts a bare `scheme://host[:port]` string. + let url = Url::parse(&format!("{raw}/")).ok()?; + + // ── Post-parse structural checks ────────────────────────────────────── + // Path must be exactly the "/" we appended. if url.path() != "/" { return None; } - // Reject origins that have no host (data:, blob:, …). + // Re-check userinfo fields (defense-in-depth, url crate may strip "@"). + if !url.username().is_empty() || url.password().is_some() { + return None; + } + // No query or fragment. + if url.query().is_some() || url.fragment().is_some() { + return None; + } + // Must have a host. url.host()?; - Some(url) + + // Reject opaque origins (data:, blob:, …). + match url.origin() { + Origin::Tuple(_, _, _) => Some(url.origin()), + Origin::Opaque(_) => None, + } } -/// Parses the request `Host` / HTTP/2 `:authority` header into a canonical -/// [`Url`], using the scheme from the request URI (defaulting to `"http"`). -fn host_to_url(request: &http::Request) -> Option { - let authority = request +// ── Host allowlist ──────────────────────────────────────────────────────────── + +/// Parses the `Host` header (or HTTP/2 `:authority` pseudo-header) into an +/// [`Authority`]. +fn request_authority(request: &http::Request) -> Option { + request .headers() .get(header::HOST) .and_then(|v| v.to_str().ok()) - .map(str::to_owned) - .or_else(|| request.uri().authority().map(ToString::to_string))?; - let scheme = request.uri().scheme_str().unwrap_or("http"); - Url::parse(&format!("{scheme}://{authority}/")).ok() + .and_then(|s| s.parse::().ok()) + .or_else(|| request.uri().authority().cloned()) } -/// Returns `true` when `request_origin` matches at least one entry in -/// `allowed_origins`. -/// -/// Both sides are parsed through [`origin_to_url`] and compared with [`Url`] -/// equality, which handles default-port normalization and case-folding -/// automatically: -/// -/// - Allowlist entry `https://app.example.com` matches both -/// `Origin: https://app.example.com` and `Origin: https://app.example.com:443`. -/// - Allowlist entry `https://app.example.com:8443` matches only -/// `Origin: https://app.example.com:8443`. -fn origin_in_allowlist(request_origin: &Url, allowed_origins: &[String]) -> bool { - allowed_origins - .iter() - .filter_map(|raw| origin_to_url(raw)) - .any(|allowed| allowed == *request_origin) -} - -/// Returns `true` when the request `Host` authority matches at least one entry -/// in `allowed_hosts`. +/// Returns `true` when `authority` matches at least one entry in +/// `allowed_hosts`. /// /// Entries are plain hostnames (`gateway.example.com`) or `host:port` /// authorities (`gateway.example.com:8080`) — no scheme prefix. @@ -74,9 +104,9 @@ fn origin_in_allowlist(request_origin: &Url, allowed_origins: &[String]) -> bool /// - Entry **with** a port → matches only that exact `(host, port)` pair. /// /// Comparison is case-insensitive on the host component. -fn host_in_allowlist(host_url: &Url, allowed_hosts: &[String]) -> bool { - let request_host = host_url.host_str().unwrap_or("").to_ascii_lowercase(); - let request_port = host_url.port_or_known_default(); +fn authority_in_allowlist(authority: &Authority, allowed_hosts: &[String]) -> bool { + let request_host = authority.host().to_ascii_lowercase(); + let request_port = authority.port_u16(); allowed_hosts.iter().any(|entry| { let (entry_host, entry_port) = match entry.rsplit_once(':') { @@ -86,11 +116,29 @@ fn host_in_allowlist(host_url: &Url, allowed_hosts: &[String]) -> bool { }, None => (entry.to_ascii_lowercase(), None), }; - entry_host == request_host - && entry_port.is_none_or(|p| Some(p) == request_port) + entry_host == request_host && entry_port.is_none_or(|p| Some(p) == request_port) }) } +// ── Allowed-origins cache ───────────────────────────────────────────────────── + +/// Parses the operator-configured origin strings once and returns the valid +/// [`Origin`] values. Invalid entries are logged and skipped so a single +/// misconfigured entry does not silently disable all protection. +pub fn parse_allowed_origins(raw: &[String]) -> Vec { + raw.iter() + .filter_map(|s| { + let origin = parse_origin(s); + if origin.is_none() { + warn!("mcp_origin_layer - configured origin is invalid and will be ignored origin = {s}"); + } + origin + }) + .collect() +} + +// ── Response helpers ────────────────────────────────────────────────────────── + fn forbidden_response() -> Response { Response::builder() .status(StatusCode::FORBIDDEN) @@ -99,6 +147,8 @@ fn forbidden_response() -> Response { .expect("response should build") } +// ── Middleware ──────────────────────────────────────────────────────────────── + /// Axum middleware that enforces the MCP 2026-07-28 Streamable HTTP /// DNS-rebinding protection requirement. /// @@ -108,48 +158,45 @@ fn forbidden_response() -> Response { /// > prevent DNS rebinding attacks. If the Origin header is present and /// > invalid, servers MUST respond with HTTP 403 Forbidden. /// -/// ## Host check (`mcp_allowed_hosts`) +/// ## Decision table /// -/// When `Config::mcp_allowed_hosts` is non-empty, every request whose `Host` -/// header does not match an entry is rejected with **HTTP 403** before Origin -/// validation. When the list is empty, Host validation is disabled. +/// | Condition | Result | +/// |---|---| +/// | `mcp_allowed_hosts` set, `Host` not in list | ❌ 403 | +/// | `Origin` absent | ✅ accept (native / non-browser clients) | +/// | `Origin: null` | ❌ 403 | +/// | `Origin` malformed, has backslash / userinfo / path / query / fragment | ❌ 403 | +/// | `mcp_allowed_origins` non-empty, parsed `Origin` in list | ✅ accept | +/// | `mcp_allowed_origins` non-empty, parsed `Origin` not in list | ❌ 403 | +/// | `mcp_allowed_origins` **empty** (default) | ❌ 403 — no fallback | /// -/// ## Origin check (`mcp_allowed_origins`) +/// **There is no same-origin fallback.** A present `Origin` always requires +/// an explicit trusted allowlist (`CONTEXTFORGE_GATEWAY_RS_MCP_ALLOWED_ORIGINS`). +/// An empty allowlist is not a bypass; it rejects every `Origin` that is present. /// -/// | `mcp_allowed_origins` | `Origin` absent | `Origin` in list | `Origin` not in list | `null` / malformed | -/// |---|---|---|---|---| -/// | **non-empty** | ✅ accept | ✅ accept | ❌ 403 | ❌ 403 | -/// | **empty** (default) | ✅ accept | ✅ if same-origin (`Origin == Host`) | ❌ 403 | ❌ 403 | -/// -/// Port comparison uses `url::Url` equality, which normalizes default ports: -/// `https://app.example.com:443` and `https://app.example.com` are the same -/// origin; `https://app.example.com:8443` is a different origin. +/// Port comparison uses [`url::Origin`] typed equality, which normalizes +/// default ports: `https://app.example.com:443` and `https://app.example.com` +/// are the same origin; `https://app.example.com:8443` is different. /// /// This layer fires before JWT claims validation, session creation, and any /// backend fan-out. -pub async fn mcp_origin_layer( - State(config): State, - request: http::Request, - next: Next, -) -> Response { - // ── 1. Host allowlist check ──────────────────────────────────────────── +pub async fn mcp_origin_layer(State(config): State, request: http::Request, next: Next) -> Response { + // ── 1. Host allowlist check ─────────────────────────────────────────────── if !config.mcp_allowed_hosts.is_empty() { - match host_to_url(&request) { + match request_authority(&request) { None => { warn!("mcp_origin_layer - rejected request: Host header missing or unparseable"); return forbidden_response(); }, - Some(ref host_url) if !host_in_allowlist(host_url, &config.mcp_allowed_hosts) => { - warn!( - "mcp_origin_layer - rejected request: Host not in allowlist host = {host_url}" - ); + Some(ref authority) if !authority_in_allowlist(authority, &config.mcp_allowed_hosts) => { + warn!("mcp_origin_layer - rejected request: Host not in allowlist host = {authority}"); return forbidden_response(); }, Some(_) => debug!("mcp_origin_layer - Host is in allowlist"), } } - // ── 2. Origin header check ───────────────────────────────────────────── + // ── 2. Origin header check ──────────────────────────────────────────────── let Some(origin_header) = request.headers().get(header::ORIGIN) else { // No Origin header → native / non-browser client; always allow. debug!("mcp_origin_layer - no Origin header, allowing request"); @@ -161,73 +208,66 @@ pub async fn mcp_origin_layer( return forbidden_response(); }; - // Opaque / sandbox origin — never valid regardless of config. + // Opaque / sandbox origin — always rejected regardless of config. if origin_str.trim().eq_ignore_ascii_case("null") { warn!("mcp_origin_layer - rejected opaque null Origin"); return forbidden_response(); } - let Some(request_origin) = origin_to_url(origin_str) else { + let Some(request_origin) = parse_origin(origin_str) else { warn!("mcp_origin_layer - rejected malformed Origin header origin = {origin_str}"); return forbidden_response(); }; - // ── 3. Accept / reject based on allowlist or same-origin fallback ────── - if config.mcp_allowed_origins.is_empty() { - // No allowlist configured: fall back to same-origin check (Origin == Host). - let Some(host_url) = host_to_url(&request) else { - warn!("mcp_origin_layer - rejected request: could not determine Host for same-origin check origin = {origin_str}"); - return forbidden_response(); - }; - if request_origin == host_url { - debug!("mcp_origin_layer - same-origin request accepted origin = {origin_str}"); - next.run(request).await - } else { - warn!("mcp_origin_layer - rejected cross-origin request origin = {origin_str} host = {host_url}"); - forbidden_response() - } + // ── 3. Allowlist check ──────────────────────────────────────────────────── + // An empty allowlist is not a bypass: any present Origin is rejected until + // the operator explicitly configures trusted origins. + if config.mcp_parsed_origins.is_empty() { + warn!("mcp_origin_layer - rejected Origin: no allowed origins configured origin = {origin_str}"); + return forbidden_response(); + } + + if config.mcp_parsed_origins.contains(&request_origin) { + debug!("mcp_origin_layer - Origin accepted via allowlist origin = {origin_str}"); + next.run(request).await } else { - // Explicit allowlist configured: Origin must appear in it. - if origin_in_allowlist(&request_origin, &config.mcp_allowed_origins) { - debug!("mcp_origin_layer - Origin accepted via allowlist origin = {origin_str}"); - next.run(request).await - } else { - warn!("mcp_origin_layer - rejected Origin not in allowlist origin = {origin_str}"); - forbidden_response() - } + warn!("mcp_origin_layer - rejected Origin not in allowlist origin = {origin_str}"); + forbidden_response() } } +// ───────────────────────────────────────────────────────────────────────────── + #[cfg(test)] mod tests { use axum::{Router, body::to_bytes, middleware, routing::get}; use http::{Request, StatusCode}; use tower::ServiceExt; + use url::Origin; use super::*; - // ── helpers ────────────────────────────────────────────────────────────── + // ── helpers ─────────────────────────────────────────────────────────────── fn config_origins(origins: &[&str]) -> Config { - Config { - mcp_allowed_origins: origins.iter().map(|s| (*s).to_owned()).collect(), - ..Config::default() - } + let mut c = + Config { mcp_allowed_origins: origins.iter().map(|s| (*s).to_owned()).collect(), ..Config::default() }; + c.finalize(); + c } fn config_hosts(hosts: &[&str]) -> Config { - Config { - mcp_allowed_hosts: hosts.iter().map(|s| (*s).to_owned()).collect(), - ..Config::default() - } + Config { mcp_allowed_hosts: hosts.iter().map(|s| (*s).to_owned()).collect(), ..Config::default() } } fn config_origins_and_hosts(origins: &[&str], hosts: &[&str]) -> Config { - Config { + let mut c = Config { mcp_allowed_origins: origins.iter().map(|s| (*s).to_owned()).collect(), mcp_allowed_hosts: hosts.iter().map(|s| (*s).to_owned()).collect(), ..Config::default() - } + }; + c.finalize(); + c } fn make_app(config: Config) -> axum::Router { @@ -241,128 +281,145 @@ mod tests { StatusCode::NO_CONTENT } - // ── origin_to_url unit tests ───────────────────────────────────────────── + // ── parse_origin unit tests ─────────────────────────────────────────────── #[test] fn null_origin_returns_none() { - assert!(origin_to_url("null").is_none()); - assert!(origin_to_url("NULL").is_none()); - assert!(origin_to_url("Null").is_none()); + assert!(parse_origin("null").is_none()); + assert!(parse_origin("NULL").is_none()); + assert!(parse_origin("Null").is_none()); } #[test] fn empty_origin_returns_none() { - assert!(origin_to_url("").is_none()); + assert!(parse_origin("").is_none()); } #[test] fn origin_without_scheme_returns_none() { - assert!(origin_to_url("app.example.com").is_none()); + assert!(parse_origin("app.example.com").is_none()); } #[test] fn origin_with_path_returns_none() { - assert!(origin_to_url("https://app.example.com/some/path").is_none()); + assert!(parse_origin("https://app.example.com/some/path").is_none()); } #[test] fn origin_with_trailing_slash_returns_none() { - assert!(origin_to_url("https://app.example.com/").is_none()); + assert!(parse_origin("https://app.example.com/").is_none()); + } + + #[test] + fn origin_with_query_returns_none() { + assert!(parse_origin("https://app.example.com?q=1").is_none()); + } + + #[test] + fn origin_with_fragment_returns_none() { + assert!(parse_origin("https://app.example.com#frag").is_none()); + } + + #[test] + fn origin_with_userinfo_returns_none() { + // "@" in the raw string is caught before parsing. + assert!(parse_origin("https://user@app.example.com").is_none()); + } + + #[test] + fn origin_with_backslash_returns_none() { + // url crate silently normalizes backslash to "/"; pre-parse check blocks it. + assert!(parse_origin(r"https:\app.example.com").is_none()); + assert!(parse_origin(r"https:\\app.example.com").is_none()); + } + + #[test] + fn origin_with_data_scheme_returns_none() { + // data: produces an opaque origin. + assert!(parse_origin("data:text/plain,foo").is_none()); } #[test] fn https_default_port_443_equals_portless() { - // The url crate silently drops the default port — both parse to the same Url. - let portless = origin_to_url("https://app.example.com").unwrap(); - let explicit = origin_to_url("https://app.example.com:443").unwrap(); + let portless = parse_origin("https://app.example.com").unwrap(); + let explicit = parse_origin("https://app.example.com:443").unwrap(); assert_eq!(portless, explicit, "https://blah.com:443 must equal https://blah.com"); } #[test] fn http_default_port_80_equals_portless() { - let portless = origin_to_url("http://app.example.com").unwrap(); - let explicit = origin_to_url("http://app.example.com:80").unwrap(); + let portless = parse_origin("http://app.example.com").unwrap(); + let explicit = parse_origin("http://app.example.com:80").unwrap(); assert_eq!(portless, explicit); } #[test] fn non_default_port_8443_is_distinct_from_portless() { - let portless = origin_to_url("https://app.example.com").unwrap(); - let non_default = origin_to_url("https://app.example.com:8443").unwrap(); + let portless = parse_origin("https://app.example.com").unwrap(); + let non_default = parse_origin("https://app.example.com:8443").unwrap(); assert_ne!(portless, non_default, "https://blah.com:8443 must NOT equal https://blah.com"); } #[test] - fn url_equality_is_case_insensitive_on_scheme_and_host() { - // The url crate normalises scheme and host to lowercase. - let lower = origin_to_url("https://app.example.com").unwrap(); - let upper = origin_to_url("HTTPS://APP.EXAMPLE.COM").unwrap(); + fn parse_origin_is_case_insensitive_on_scheme_and_host() { + let lower = parse_origin("https://app.example.com").unwrap(); + let upper = parse_origin("HTTPS://APP.EXAMPLE.COM").unwrap(); assert_eq!(lower, upper); } #[test] fn ipv6_origin_parsed_correctly() { - let o = origin_to_url("http://[::1]:8080").unwrap(); - assert_eq!(o.host_str(), Some("[::1]")); + // IPv6 address produces a valid Tuple origin. + let o = parse_origin("http://[::1]:8080").unwrap(); + assert!(matches!(o, Origin::Tuple(_, _, 8080))); } - // ── origin_in_allowlist unit tests ─────────────────────────────────────── + // ── parse_allowed_origins unit tests ───────────────────────────────────── #[test] - fn allowlist_exact_match() { - let req = origin_to_url("https://app.example.com").unwrap(); - assert!(origin_in_allowlist(&req, &["https://app.example.com".to_owned()])); + fn invalid_configured_origin_is_skipped() { + let parsed = parse_allowed_origins(&["https://valid.example.com".to_owned(), r"https:\bad".to_owned()]); + assert_eq!(parsed.len(), 1); + assert_eq!(parsed[0], parse_origin("https://valid.example.com").unwrap()); } #[test] - fn allowlist_portless_entry_matches_explicit_default_port() { - // Entry has no port (→ :443); request sends :443 explicitly — same origin. - let req = origin_to_url("https://app.example.com:443").unwrap(); - assert!(origin_in_allowlist(&req, &["https://app.example.com".to_owned()])); + fn empty_configured_origins_produces_empty_list() { + assert!(parse_allowed_origins(&[]).is_empty()); } - #[test] - fn allowlist_entry_with_443_matches_portless_request() { - // Entry is :443; browser sends no explicit port — same origin. - let req = origin_to_url("https://app.example.com").unwrap(); - assert!(origin_in_allowlist(&req, &["https://app.example.com:443".to_owned()])); - } + // ── authority_in_allowlist unit tests ───────────────────────────────────── #[test] - fn allowlist_portless_entry_does_not_match_non_default_port() { - // Entry normalizes to :443; :8443 is a different origin. - let req = origin_to_url("https://app.example.com:8443").unwrap(); - assert!(!origin_in_allowlist(&req, &["https://app.example.com".to_owned()])); + fn authority_exact_host_match() { + let auth = "gateway.example.com".parse::().unwrap(); + assert!(authority_in_allowlist(&auth, &["gateway.example.com".to_owned()])); } #[test] - fn allowlist_8443_entry_does_not_match_default_port() { - // Entry is :8443; portless request normalizes to :443 — different origin. - let req = origin_to_url("https://app.example.com").unwrap(); - assert!(!origin_in_allowlist(&req, &["https://app.example.com:8443".to_owned()])); + fn authority_entry_without_port_matches_any_port() { + let auth = "gateway.example.com:8080".parse::().unwrap(); + assert!(authority_in_allowlist(&auth, &["gateway.example.com".to_owned()])); } #[test] - fn allowlist_scheme_mismatch_rejected() { - let req = origin_to_url("http://app.example.com").unwrap(); - assert!(!origin_in_allowlist(&req, &["https://app.example.com".to_owned()])); + fn authority_entry_with_port_matches_only_that_port() { + let auth8080 = "gateway.example.com:8080".parse::().unwrap(); + let auth443 = "gateway.example.com:443".parse::().unwrap(); + assert!(authority_in_allowlist(&auth8080, &["gateway.example.com:8080".to_owned()])); + assert!(!authority_in_allowlist(&auth443, &["gateway.example.com:8080".to_owned()])); } #[test] - fn allowlist_multiple_entries() { - let allowed = vec![ - "https://app.example.com".to_owned(), - "http://localhost:3000".to_owned(), - ]; - assert!(origin_in_allowlist(&origin_to_url("https://app.example.com").unwrap(), &allowed)); - assert!(origin_in_allowlist(&origin_to_url("http://localhost:3000").unwrap(), &allowed)); - assert!(!origin_in_allowlist(&origin_to_url("https://other.example.com").unwrap(), &allowed)); + fn authority_mismatch_returns_false() { + let auth = "evil.example.com".parse::().unwrap(); + assert!(!authority_in_allowlist(&auth, &["gateway.example.com".to_owned()])); } - // ── middleware integration: no Origin ──────────────────────────────────── + // ── middleware: no Origin ───────────────────────────────────────────────── #[tokio::test] - async fn no_origin_is_always_accepted_with_empty_config() { + async fn no_origin_accepted_with_empty_config() { let app = make_app(Config::default()); let req = Request::builder().uri("/mcp").method("GET").body(Body::empty()).unwrap(); let res = app.oneshot(req).await.unwrap(); @@ -370,23 +427,53 @@ mod tests { } #[tokio::test] - async fn no_origin_is_always_accepted_with_allowlist_configured() { + async fn no_origin_accepted_with_allowlist_configured() { let app = make_app(config_origins(&["https://app.example.com"])); let req = Request::builder().uri("/mcp").method("GET").body(Body::empty()).unwrap(); let res = app.oneshot(req).await.unwrap(); assert_eq!(res.status(), StatusCode::NO_CONTENT); } - // ── middleware integration: Origin allowlist (non-empty) ───────────────── + // ── middleware: empty allowlist rejects any present Origin ──────────────── + + #[tokio::test] + async fn present_origin_with_empty_allowlist_returns_403() { + // Empty allowlist is not a bypass; any present Origin must be rejected. + let app = make_app(Config::default()); + let req = Request::builder() + .uri("/mcp") + .method("POST") + .header(header::ORIGIN, "https://app.example.com") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn attacker_controlled_host_and_origin_match_but_still_rejected_without_allowlist() { + // DNS-rebinding: attacker controls both Host and Origin to the same value. + // Without an explicit allowlist this must be rejected, not accepted. + let app = make_app(Config::default()); + let req = Request::builder() + .uri("http://attacker.invalid/mcp") + .method("POST") + .header(header::HOST, "attacker.invalid") + .header(header::ORIGIN, "http://attacker.invalid") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + } + + // ── middleware: allowlist (non-empty) ───────────────────────────────────── #[tokio::test] - async fn allowlisted_cross_origin_is_accepted() { - // Origin differs from Host but is in the allowlist. + async fn allowlisted_origin_accepted() { let app = make_app(config_origins(&["https://app.example.com"])); let req = Request::builder() - .uri("https://gateway.example.com/mcp") + .uri("/mcp") .method("POST") - .header(header::HOST, "gateway.example.com") .header(header::ORIGIN, "https://app.example.com") .body(Body::empty()) .unwrap(); @@ -412,9 +499,8 @@ mod tests { // Browser sends :443 explicitly; allowlist has no port — same origin. let app = make_app(config_origins(&["https://app.example.com"])); let req = Request::builder() - .uri("https://gateway.example.com/mcp") + .uri("/mcp") .method("POST") - .header(header::HOST, "gateway.example.com") .header(header::ORIGIN, "https://app.example.com:443") .body(Body::empty()) .unwrap(); @@ -427,9 +513,8 @@ mod tests { // Allowlist has :443; browser sends no port — same origin. let app = make_app(config_origins(&["https://app.example.com:443"])); let req = Request::builder() - .uri("https://gateway.example.com/mcp") + .uri("/mcp") .method("POST") - .header(header::HOST, "gateway.example.com") .header(header::ORIGIN, "https://app.example.com") .body(Body::empty()) .unwrap(); @@ -488,16 +573,20 @@ mod tests { assert_eq!(res.status(), StatusCode::FORBIDDEN); } - // ── middleware integration: same-origin fallback (empty allowlist) ──────── + // ── middleware: HTTPS origin-form requests ──────────────────────────────── #[tokio::test] - async fn same_origin_accepted_when_no_allowlist() { - let app = make_app(Config::default()); + async fn https_origin_accepted_when_allowlisted_origin_form_request() { + // A normal HTTP/1.1 request has URI `/mcp` (origin-form, no scheme). + // The scheme cannot be inferred from the request URI; only the Origin + // header value matters for the allowlist comparison. + let app = make_app(config_origins(&["https://app.example.com"])); let req = Request::builder() - .uri("http://localhost/mcp") + // origin-form URI — no scheme + .uri("/mcp") .method("POST") - .header(header::HOST, "localhost") - .header(header::ORIGIN, "http://localhost") + .header(header::HOST, "app.example.com") + .header(header::ORIGIN, "https://app.example.com") .body(Body::empty()) .unwrap(); let res = app.oneshot(req).await.unwrap(); @@ -505,56 +594,85 @@ mod tests { } #[tokio::test] - async fn cross_origin_rejected_when_no_allowlist() { - let app = make_app(Config::default()); + async fn http_origin_rejected_when_only_https_allowlisted_origin_form_request() { + // Origin: http://... must not match an allowlist entry for https://... + // even when the request URI has no scheme and Host matches. + let app = make_app(config_origins(&["https://app.example.com"])); let req = Request::builder() - .uri("http://localhost/mcp") + .uri("/mcp") .method("POST") - .header(header::HOST, "localhost") - .header(header::ORIGIN, "https://attacker.invalid") + .header(header::HOST, "app.example.com") + .header(header::ORIGIN, "http://app.example.com") .body(Body::empty()) .unwrap(); let res = app.oneshot(req).await.unwrap(); assert_eq!(res.status(), StatusCode::FORBIDDEN); } + // ── middleware: malformed-but-normalizable Origins ──────────────────────── + #[tokio::test] - async fn default_port_normalization_same_origin_fallback() { - // Origin: https://app.example.com:443 ↔ Host: app.example.com — same origin. - let app = make_app(Config::default()); + async fn backslash_origin_returns_403() { + // url crate would normalize https:\app.example.com to https://app.example.com + // but pre-parse check must reject it first. + let app = make_app(config_origins(&["https://app.example.com"])); let req = Request::builder() - .uri("https://app.example.com/mcp") + .uri("/mcp") .method("POST") - .header(header::HOST, "app.example.com") - .header(header::ORIGIN, "https://app.example.com:443") + .header(header::ORIGIN, r"https:\app.example.com") .body(Body::empty()) .unwrap(); let res = app.oneshot(req).await.unwrap(); - assert_eq!(res.status(), StatusCode::NO_CONTENT); + assert_eq!(res.status(), StatusCode::FORBIDDEN); } #[tokio::test] - async fn non_default_port_mismatch_rejected_in_same_origin_fallback() { - // Host: app.example.com (→ :443), Origin: :8443 — different origin. - let app = make_app(Config::default()); + async fn userinfo_origin_returns_403() { + // url crate strips userinfo from the origin; we must reject before that. + let app = make_app(config_origins(&["https://app.example.com"])); let req = Request::builder() - .uri("https://app.example.com/mcp") + .uri("/mcp") .method("POST") - .header(header::HOST, "app.example.com") - .header(header::ORIGIN, "https://app.example.com:8443") + .header(header::ORIGIN, "https://user@app.example.com") .body(Body::empty()) .unwrap(); let res = app.oneshot(req).await.unwrap(); assert_eq!(res.status(), StatusCode::FORBIDDEN); } - // ── middleware integration: null / malformed (always 403) ──────────────── + #[tokio::test] + async fn origin_with_query_returns_403() { + let app = make_app(config_origins(&["https://app.example.com"])); + let req = Request::builder() + .uri("/mcp") + .method("POST") + .header(header::ORIGIN, "https://app.example.com?q=1") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + } #[tokio::test] - async fn null_origin_returns_403_with_allowlist() { + async fn origin_with_fragment_returns_403() { let app = make_app(config_origins(&["https://app.example.com"])); let req = Request::builder() - .uri("/mcp").method("POST").header(header::ORIGIN, "null").body(Body::empty()).unwrap(); + .uri("/mcp") + .method("POST") + .header(header::ORIGIN, "https://app.example.com#frag") + .body(Body::empty()) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN); + } + + // ── middleware: null / malformed (always 403) ───────────────────────────── + + #[tokio::test] + async fn null_origin_returns_403_with_allowlist() { + let app = make_app(config_origins(&["https://app.example.com"])); + let req = + Request::builder().uri("/mcp").method("POST").header(header::ORIGIN, "null").body(Body::empty()).unwrap(); let res = app.oneshot(req).await.unwrap(); assert_eq!(res.status(), StatusCode::FORBIDDEN); } @@ -562,8 +680,8 @@ mod tests { #[tokio::test] async fn null_origin_returns_403_without_allowlist() { let app = make_app(Config::default()); - let req = Request::builder() - .uri("/mcp").method("POST").header(header::ORIGIN, "null").body(Body::empty()).unwrap(); + let req = + Request::builder().uri("/mcp").method("POST").header(header::ORIGIN, "null").body(Body::empty()).unwrap(); let res = app.oneshot(req).await.unwrap(); assert_eq!(res.status(), StatusCode::FORBIDDEN); } @@ -572,20 +690,23 @@ mod tests { async fn malformed_origin_returns_403() { let app = make_app(Config::default()); let req = Request::builder() - .uri("/mcp").method("POST").header(header::ORIGIN, "not-an-origin").body(Body::empty()).unwrap(); + .uri("/mcp") + .method("POST") + .header(header::ORIGIN, "not-an-origin") + .body(Body::empty()) + .unwrap(); let res = app.oneshot(req).await.unwrap(); assert_eq!(res.status(), StatusCode::FORBIDDEN); } - // ── middleware integration: DELETE method ───────────────────────────────── + // ── middleware: DELETE method ───────────────────────────────────────────── #[tokio::test] async fn delete_allowlisted_origin_accepted() { let app = make_app(config_origins(&["https://app.example.com"])); let req = Request::builder() - .uri("https://gateway.example.com/mcp") + .uri("/mcp") .method("DELETE") - .header(header::HOST, "gateway.example.com") .header(header::ORIGIN, "https://app.example.com") .body(Body::empty()) .unwrap(); @@ -597,18 +718,22 @@ mod tests { async fn delete_non_allowlisted_origin_returns_403() { let app = make_app(config_origins(&["https://app.example.com"])); let req = Request::builder() - .uri("/mcp").method("DELETE").header(header::ORIGIN, "https://attacker.invalid").body(Body::empty()).unwrap(); + .uri("/mcp") + .method("DELETE") + .header(header::ORIGIN, "https://attacker.invalid") + .body(Body::empty()) + .unwrap(); let res = app.oneshot(req).await.unwrap(); assert_eq!(res.status(), StatusCode::FORBIDDEN); } - // ── middleware integration: Host allowlist ──────────────────────────────── + // ── middleware: Host allowlist ──────────────────────────────────────────── #[tokio::test] async fn request_with_allowed_host_passes_host_check() { let app = make_app(config_hosts(&["gateway.example.com"])); let req = Request::builder() - .uri("https://gateway.example.com/mcp") + .uri("/mcp") .method("GET") .header(header::HOST, "gateway.example.com") .body(Body::empty()) @@ -621,7 +746,7 @@ mod tests { async fn request_with_disallowed_host_returns_403() { let app = make_app(config_hosts(&["gateway.example.com"])); let req = Request::builder() - .uri("https://evil.example.com/mcp") + .uri("/mcp") .method("POST") .header(header::HOST, "evil.example.com") .header(header::ORIGIN, "https://app.example.com") @@ -633,12 +758,9 @@ mod tests { #[tokio::test] async fn host_and_origin_both_valid_accepted() { - let app = make_app(config_origins_and_hosts( - &["https://app.example.com"], - &["gateway.example.com"], - )); + let app = make_app(config_origins_and_hosts(&["https://app.example.com"], &["gateway.example.com"])); let req = Request::builder() - .uri("https://gateway.example.com/mcp") + .uri("/mcp") .method("POST") .header(header::HOST, "gateway.example.com") .header(header::ORIGIN, "https://app.example.com") @@ -650,12 +772,9 @@ mod tests { #[tokio::test] async fn valid_host_but_invalid_origin_returns_403() { - let app = make_app(config_origins_and_hosts( - &["https://app.example.com"], - &["gateway.example.com"], - )); + let app = make_app(config_origins_and_hosts(&["https://app.example.com"], &["gateway.example.com"])); let req = Request::builder() - .uri("https://gateway.example.com/mcp") + .uri("/mcp") .method("POST") .header(header::HOST, "gateway.example.com") .header(header::ORIGIN, "https://attacker.invalid") @@ -665,11 +784,17 @@ mod tests { assert_eq!(res.status(), StatusCode::FORBIDDEN); } + // ── misc ────────────────────────────────────────────────────────────────── + #[tokio::test] async fn forbidden_response_body_is_non_empty() { let app = make_app(config_origins(&["https://app.example.com"])); let req = Request::builder() - .uri("/mcp").method("POST").header(header::ORIGIN, "https://attacker.invalid").body(Body::empty()).unwrap(); + .uri("/mcp") + .method("POST") + .header(header::ORIGIN, "https://attacker.invalid") + .body(Body::empty()) + .unwrap(); let res = app.oneshot(req).await.unwrap(); assert_eq!(res.status(), StatusCode::FORBIDDEN); let body = to_bytes(res.into_body(), 256).await.unwrap(); diff --git a/crates/contextforge-data-plane-lib/src/lib.rs b/crates/contextforge-data-plane-lib/src/lib.rs index fd65849f..8d043c31 100644 --- a/crates/contextforge-data-plane-lib/src/lib.rs +++ b/crates/contextforge-data-plane-lib/src/lib.rs @@ -65,7 +65,9 @@ pub struct Gateway { } impl Gateway { - pub async fn run_gateway(self) -> Result<()> { + pub async fn run_gateway(mut self) -> Result<()> { + // Parse mcp_allowed_origins into typed url::Origin values once at startup. + self.config.finalize(); let config = &self.config; let session_manager = self.session_manager; let user_config_store = match self.user_config_store_type { diff --git a/docs/book/src/security-model.md b/docs/book/src/security-model.md index 76e54556..2034c825 100644 --- a/docs/book/src/security-model.md +++ b/docs/book/src/security-model.md @@ -77,23 +77,30 @@ alongside `mcp_allowed_origins` for public-internet deployments. ### Origin allowlist (`CONTEXTFORGE_GATEWAY_RS_MCP_ALLOWED_ORIGINS`) -An optional comma-separated list of fully-qualified browser origins +A comma-separated list of fully-qualified browser origins (`https://app.example.com`, `http://localhost:3000`). | `mcp_allowed_origins` | `Origin` absent | `Origin` in list | `Origin` not in list | `null` / malformed | | --- | --- | --- | --- | --- | | **non-empty** | ✅ accepted | ✅ accepted | ❌ HTTP 403 | ❌ HTTP 403 | -| **empty** (default) | ✅ accepted | ✅ if same-origin (Origin == Host) | ❌ HTTP 403 | ❌ HTTP 403 | - -Port comparison is exact after RFC 3986 default-port normalization: -`https://app.example.com` and `https://app.example.com:443` are the same -origin; `https://app.example.com:8443` is a different origin. - -When the list is empty the middleware falls back to a **same-origin check**: -the normalized `Origin` must equal the normalized `Host`. This ensures that -DNS-rebinding protection is always active regardless of operator configuration, -while cross-origin browser clients can be admitted by adding an explicit -allowlist entry. +| **empty** (default) | ✅ accepted | ❌ HTTP 403 | ❌ HTTP 403 | ❌ HTTP 403 | + +**An empty allowlist is not a bypass.** When `mcp_allowed_origins` is not +configured, every request that carries an `Origin` header is rejected with HTTP +403. There is no same-origin fallback: comparing `Origin` with `Host` would let +a DNS-rebinding attacker satisfy both values simultaneously, defeating the +protection entirely. + +Origins are strictly validated before comparison: backslash sequences, userinfo +(`@`), path, query, and fragment components cause immediate rejection, preventing +the `url` crate's WHATWG-compliant normalization from silently repairing +malformed inputs into a valid origin. + +Port comparison uses typed `url::Origin` equality after RFC 3986 default-port +normalization: `https://app.example.com` and `https://app.example.com:443` are +the same origin; `https://app.example.com:8443` is a different origin. +Configured origins are parsed once at startup; invalid entries are logged and +skipped. ## Local Bootstrap Helpers From ce0fbfb39717489cb494923d8800b109f8d3110c Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Fri, 7 Aug 2026 13:38:14 +0100 Subject: [PATCH 03/13] fix(security): strict Origin syntax, startup validation error, request-flow docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Reject https:///…, https:////…, trailing colon (https://host:), and leading/trailing whitespace in parse_origin before passing to url::Url; all four were silently normalized to a valid tuple by the url crate - Config::finalize() now returns Result<(), ConfigValidationError>; invalid configured origins abort startup with an error naming every bad entry - Add ConfigValidationError::InvalidMcpAllowedOrigins variant - run_gateway propagates finalize() error via ? - Remove parse_allowed_origins (replaced by finalize + parse_origin_str) - Add regressions: extra_slashes_after_scheme_returns_none, trailing_colon_without_port_returns_none, leading_whitespace_returns_none, finalize_with_invalid_origin_returns_error, finalize_reports_all_invalid_origins_in_error - request-flow.md: remove same-origin fallback from stack diagram and middleware table Signed-off-by: prakhar-singh1928 --- .../contextforge-data-plane-lib/src/common.rs | 31 ++++- .../src/layers/mcp_origin.rs | 129 +++++++++++++----- crates/contextforge-data-plane-lib/src/lib.rs | 3 +- docs/book/src/request-flow.md | 4 +- 4 files changed, 123 insertions(+), 44 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/common.rs b/crates/contextforge-data-plane-lib/src/common.rs index b93ec378..df9abf27 100644 --- a/crates/contextforge-data-plane-lib/src/common.rs +++ b/crates/contextforge-data-plane-lib/src/common.rs @@ -325,11 +325,30 @@ impl Config { /// completes so the middleware can compare against pre-parsed values /// instead of re-parsing on every request. /// - /// Invalid entries are logged and skipped; they do not cause startup - /// failure so a single misconfigured origin does not take down the gateway. - pub fn finalize(&mut self) { - use crate::layers::mcp_origin::parse_allowed_origins; - self.mcp_parsed_origins = parse_allowed_origins(&self.mcp_allowed_origins); + /// Returns an error if any configured origin string is invalid. + /// The error message names all invalid entries so the operator can correct + /// the configuration without restarting repeatedly. + /// + /// # Errors + /// + /// Returns [`ConfigValidationError::InvalidMcpAllowedOrigins`] when one or + /// more entries in `mcp_allowed_origins` cannot be parsed as a valid + /// serialized origin. + pub fn finalize(&mut self) -> Result<(), ConfigValidationError> { + use crate::layers::mcp_origin::parse_origin_str; + let mut parsed = Vec::with_capacity(self.mcp_allowed_origins.len()); + let mut invalid = Vec::new(); + for s in &self.mcp_allowed_origins { + match parse_origin_str(s) { + Some(origin) => parsed.push(origin), + None => invalid.push(s.as_str()), + } + } + if !invalid.is_empty() { + return Err(ConfigValidationError::InvalidMcpAllowedOrigins(invalid.join(", "))); + } + self.mcp_parsed_origins = parsed; + Ok(()) } } @@ -337,6 +356,8 @@ impl Config { pub enum ConfigValidationError { #[error("Redis Configuration Error")] RedisConfigurationError(String), + #[error("Invalid mcp_allowed_origins entries: {0}")] + InvalidMcpAllowedOrigins(String), } impl TryFrom<&Config> for RedisConfig { diff --git a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs index 9466f971..432e44ae 100644 --- a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs +++ b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs @@ -20,15 +20,6 @@ use crate::common::Config; /// - Contains `@` before the first `/` → rejected (userinfo present). /// - Contains `?` or `#` → rejected (query / fragment present). /// -/// After parsing, additional structural checks are applied: -/// -/// - Parsed URL has non-empty username or a password → rejected. -/// - Parsed URL has a path other than `"/"` (from the slash we appended) → -/// rejected (path component present). -/// - Parsed URL has a query or fragment → rejected. -/// - Parsed URL has no host → rejected (e.g. `data:`, `blob:`). -/// - `url::Origin` is opaque → rejected. -/// /// Returns `None` for the literal `"null"` opaque origin (RFC 6454 §6.2) and /// for any value that fails the checks above. /// @@ -36,11 +27,18 @@ use crate::common::Config; /// and `https://blah.com` produce the same `Origin::Tuple`; `https://blah.com:8443` /// is distinct. fn parse_origin(raw: &str) -> Option { + // ── Pre-parse structural checks on the raw string ───────────────────── + + // Leading/trailing whitespace is not valid in a serialized origin + // (RFC 6454 §6.1) and the url crate silently trims it, so reject early. + if raw != raw.trim() { + return None; + } + if raw.trim().eq_ignore_ascii_case("null") { return None; } - // ── Pre-parse structural checks on the raw string ───────────────────── // Backslash — the url crate treats it as a slash (WHATWG URL §5.1). if raw.contains('\\') { return None; @@ -55,6 +53,25 @@ fn parse_origin(raw: &str) -> Option { return None; } + // A serialized origin is exactly `scheme "://" host [":" port]`. + // This rejects "https:///…" and "https:////…" where the url crate silently + // collapses the extra slashes into a valid URL. + let Some((_, authority_part)) = raw.split_once("://") else { + return None; // no "://" at all + }; + // Extra leading slashes after "://" mean the authority is empty or wrong. + if authority_part.starts_with('/') || authority_part.is_empty() { + return None; + } + // A trailing ":" with no port digits is malformed (e.g. "https://host:"). + // Strip any IPv6 brackets first so "[::1]:" is also caught. + let host_for_port_check = authority_part.trim_start_matches('['); + if let Some((_, port_part)) = host_for_port_check.rsplit_once(':') + && port_part.is_empty() + { + return None; + } + // Append "/" so the url crate accepts a bare `scheme://host[:port]` string. let url = Url::parse(&format!("{raw}/")).ok()?; @@ -81,6 +98,12 @@ fn parse_origin(raw: &str) -> Option { } } +/// Public-to-crate entry point for [`Config::finalize`] to validate configured +/// origins at startup without exposing the private `parse_origin` function. +pub(crate) fn parse_origin_str(raw: &str) -> Option { + parse_origin(raw) +} + // ── Host allowlist ──────────────────────────────────────────────────────────── /// Parses the `Host` header (or HTTP/2 `:authority` pseudo-header) into an @@ -120,23 +143,6 @@ fn authority_in_allowlist(authority: &Authority, allowed_hosts: &[String]) -> bo }) } -// ── Allowed-origins cache ───────────────────────────────────────────────────── - -/// Parses the operator-configured origin strings once and returns the valid -/// [`Origin`] values. Invalid entries are logged and skipped so a single -/// misconfigured entry does not silently disable all protection. -pub fn parse_allowed_origins(raw: &[String]) -> Vec { - raw.iter() - .filter_map(|s| { - let origin = parse_origin(s); - if origin.is_none() { - warn!("mcp_origin_layer - configured origin is invalid and will be ignored origin = {s}"); - } - origin - }) - .collect() -} - // ── Response helpers ────────────────────────────────────────────────────────── fn forbidden_response() -> Response { @@ -252,7 +258,7 @@ mod tests { fn config_origins(origins: &[&str]) -> Config { let mut c = Config { mcp_allowed_origins: origins.iter().map(|s| (*s).to_owned()).collect(), ..Config::default() }; - c.finalize(); + c.finalize().expect("test origins should be valid"); c } @@ -266,7 +272,7 @@ mod tests { mcp_allowed_hosts: hosts.iter().map(|s| (*s).to_owned()).collect(), ..Config::default() }; - c.finalize(); + c.finalize().expect("test origins should be valid"); c } @@ -374,18 +380,69 @@ mod tests { assert!(matches!(o, Origin::Tuple(_, _, 8080))); } - // ── parse_allowed_origins unit tests ───────────────────────────────────── + // ── parse_origin: new strict syntax regressions ─────────────────────────── + + #[test] + fn extra_slashes_after_scheme_returns_none() { + // "https:///…" and "https:////…" — url crate collapses these to a valid + // host but they are not valid serialized origins. + assert!(parse_origin("https:///app.example.com").is_none()); + assert!(parse_origin("https:////app.example.com").is_none()); + } #[test] - fn invalid_configured_origin_is_skipped() { - let parsed = parse_allowed_origins(&["https://valid.example.com".to_owned(), r"https:\bad".to_owned()]); - assert_eq!(parsed.len(), 1); - assert_eq!(parsed[0], parse_origin("https://valid.example.com").unwrap()); + fn trailing_colon_without_port_returns_none() { + // "https://app.example.com:" — url crate accepts this as no-port. + assert!(parse_origin("https://app.example.com:").is_none()); } #[test] - fn empty_configured_origins_produces_empty_list() { - assert!(parse_allowed_origins(&[]).is_empty()); + fn leading_whitespace_returns_none() { + // url crate silently trims leading/trailing whitespace. + assert!(parse_origin(" https://app.example.com").is_none()); + assert!(parse_origin("https://app.example.com ").is_none()); + } + + // ── parse_origin_str / Config::finalize unit tests ──────────────────────── + + #[test] + fn empty_configured_origins_produces_empty_parsed_list() { + let mut c = Config::default(); + assert!(c.finalize().is_ok()); + assert!(c.mcp_parsed_origins.is_empty()); + } + + // ── Config::finalize startup-error tests ────────────────────────────────── + + #[test] + fn finalize_with_valid_origins_succeeds() { + let mut c = Config { mcp_allowed_origins: vec!["https://app.example.com".to_owned()], ..Config::default() }; + assert!(c.finalize().is_ok()); + assert_eq!(c.mcp_parsed_origins.len(), 1); + } + + #[test] + fn finalize_with_invalid_origin_returns_error() { + let mut c = Config { + mcp_allowed_origins: vec!["https://valid.example.com".to_owned(), r"https:\bad".to_owned()], + ..Config::default() + }; + let err = c.finalize().unwrap_err(); + assert!(err.to_string().contains(r"https:\bad")); + // mcp_parsed_origins must not be partially populated on error. + assert!(c.mcp_parsed_origins.is_empty()); + } + + #[test] + fn finalize_reports_all_invalid_origins_in_error() { + let mut c = Config { + mcp_allowed_origins: vec![r"https:\bad1".to_owned(), "https:////bad2.example.com".to_owned()], + ..Config::default() + }; + let err = c.finalize().unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains(r"https:\bad1"), "expected bad1 in: {msg}"); + assert!(msg.contains("https:////bad2.example.com"), "expected bad2 in: {msg}"); } // ── authority_in_allowlist unit tests ───────────────────────────────────── diff --git a/crates/contextforge-data-plane-lib/src/lib.rs b/crates/contextforge-data-plane-lib/src/lib.rs index 8d043c31..508423fc 100644 --- a/crates/contextforge-data-plane-lib/src/lib.rs +++ b/crates/contextforge-data-plane-lib/src/lib.rs @@ -67,7 +67,8 @@ pub struct Gateway { impl Gateway { pub async fn run_gateway(mut self) -> Result<()> { // Parse mcp_allowed_origins into typed url::Origin values once at startup. - self.config.finalize(); + // Returns an error (and aborts startup) if any configured origin is invalid. + self.config.finalize()?; let config = &self.config; let session_manager = self.session_manager; let user_config_store = match self.user_config_store_type { diff --git a/docs/book/src/request-flow.md b/docs/book/src/request-flow.md index 6c663699..ba907306 100644 --- a/docs/book/src/request-flow.md +++ b/docs/book/src/request-flow.md @@ -62,7 +62,7 @@ TCP/TLS listener -> HttpMetricsLayer -> TraceLayer -> /contextforge-rs nested router - -> mcp_origin_layer (MCP 2026-07-28: Host allowlist check, then Origin allowlist or same-origin fallback) + -> mcp_origin_layer (MCP 2026-07-28: Host allowlist check, then Origin allowlist; absent Origin passes, empty allowlist rejects every present Origin) -> CORS layer -> virtual_host_id_layer -> claims_layer @@ -104,7 +104,7 @@ The request layers insert the context used later by RMCP handlers: | Layer | Request behavior | Failure behavior | | --- | --- | --- | -| `mcp_origin_layer` | (1) If `mcp_allowed_hosts` is set, rejects `Host` not in the list. (2) Absent `Origin` passes. (3) If `mcp_allowed_origins` is set, `Origin` must be in the list; otherwise `Origin` must equal `Host` (same-origin). Ports normalized per RFC 3986. | Returns `403` for disallowed `Host`, cross-origin, opaque (`null`), or malformed `Origin`. | +| `mcp_origin_layer` | (1) If `mcp_allowed_hosts` is set, rejects `Host` not in the list. (2) Absent `Origin` passes. (3) `Origin` must be in `mcp_allowed_origins`; an empty allowlist rejects every present `Origin` (no same-origin fallback). Strict serialized-origin syntax enforced; ports normalized per RFC 3986. | Returns `403` for disallowed `Host`, non-allowlisted, opaque (`null`), malformed, or extra-slash `Origin`. | | `virtual_host_id_layer` | Extracts `/servers/{virtual_host_id}/mcp` and inserts `VirtualHostId`. | Returns `400` when the inner path does not match. | | `claims_layer` | Validates `Authorization: Bearer ...` with configured RS/HMAC decoder, issuer, audience, and expiration. Inserts `ContextForgeClaims`. | Returns `401` for missing or invalid bearer auth. | | `session_id_layer` | Reads `Mcp-session-id` and inserts `SessionId` when present. | Missing session id is allowed here; authorized MCP handlers reject it later when required. | From 07130d30fb502f92659b56020fe0528287bf9008 Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Mon, 10 Aug 2026 09:38:56 +0100 Subject: [PATCH 04/13] fix(security): use ? operator in parse_origin, fix security-model wording Signed-off-by: prakhar-singh1928 --- .../src/layers/mcp_origin.rs | 177 +++++++----------- docs/book/src/security-model.md | 4 +- 2 files changed, 71 insertions(+), 110 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs index 432e44ae..084ea313 100644 --- a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs +++ b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs @@ -5,66 +5,40 @@ use url::{Origin, Url}; use crate::common::Config; -// ── Origin parsing ──────────────────────────────────────────────────────────── - -/// Strictly parses a serialized RFC 6454 origin string into a typed +/// Parses a serialized RFC 6454 origin (`scheme://host[:port]`) into a typed /// [`url::Origin`]. /// -/// A valid serialized origin is exactly `scheme "://" host [":" port]` with -/// **no** userinfo, path, query, or fragment component. The `url` crate -/// silently repairs many malformed inputs (backslashes, userinfo stripping, -/// etc.), so this function validates the raw string before handing it to the -/// parser: -/// -/// - Contains `\` → rejected (backslash normalization attack). -/// - Contains `@` before the first `/` → rejected (userinfo present). -/// - Contains `?` or `#` → rejected (query / fragment present). -/// -/// Returns `None` for the literal `"null"` opaque origin (RFC 6454 §6.2) and -/// for any value that fails the checks above. -/// -/// Port normalization is handled by the `url` crate: `https://blah.com:443` -/// and `https://blah.com` produce the same `Origin::Tuple`; `https://blah.com:8443` -/// is distinct. +/// Returns `None` for `"null"` and for any value that is not a bare authority +/// — no path, query, fragment, userinfo, backslash, control characters, or +/// extra slashes after `://`. Port normalization (`https://x.com:443` == +/// `https://x.com`) is handled by the `url` crate. fn parse_origin(raw: &str) -> Option { - // ── Pre-parse structural checks on the raw string ───────────────────── - - // Leading/trailing whitespace is not valid in a serialized origin - // (RFC 6454 §6.1) and the url crate silently trims it, so reject early. - if raw != raw.trim() { + // Reject ASCII control characters (including embedded tab) before the url + // crate silently strips them. + if raw.bytes().any(|b| b < 0x20 || b == 0x7F) { return None; } - - if raw.trim().eq_ignore_ascii_case("null") { - return None; - } - - // Backslash — the url crate treats it as a slash (WHATWG URL §5.1). - if raw.contains('\\') { + // Reject leading/trailing whitespace; also catches Unicode spaces the + // control-character check above misses. + if raw != raw.trim() { return None; } - // Userinfo — "@" before the first "/" after the scheme separator. - // A valid origin has no path, so any "@" means userinfo. - if raw.contains('@') { + if raw.eq_ignore_ascii_case("null") { return None; } - // Query / fragment. - if raw.contains('?') || raw.contains('#') { + // Reject characters that the url crate normalizes away silently. + if raw.contains('\\') || raw.contains('@') || raw.contains('?') || raw.contains('#') { return None; } - // A serialized origin is exactly `scheme "://" host [":" port]`. - // This rejects "https:///…" and "https:////…" where the url crate silently - // collapses the extra slashes into a valid URL. - let Some((_, authority_part)) = raw.split_once("://") else { - return None; // no "://" at all - }; - // Extra leading slashes after "://" mean the authority is empty or wrong. - if authority_part.starts_with('/') || authority_part.is_empty() { + // Split on "://" and validate the authority portion on the raw string. + // Any "/" in authority_part means a path (e.g. "host/." normalizes to "/" + // after parsing, so this must be caught before Url::parse runs). + let (_, authority_part) = raw.split_once("://")?; + if authority_part.is_empty() || authority_part.contains('/') { return None; } - // A trailing ":" with no port digits is malformed (e.g. "https://host:"). - // Strip any IPv6 brackets first so "[::1]:" is also caught. + // Reject trailing ":" with no port (e.g. "https://host:"). let host_for_port_check = authority_part.trim_start_matches('['); if let Some((_, port_part)) = host_for_port_check.rsplit_once(':') && port_part.is_empty() @@ -72,42 +46,32 @@ fn parse_origin(raw: &str) -> Option { return None; } - // Append "/" so the url crate accepts a bare `scheme://host[:port]` string. + // Append "/" so Url::parse accepts a bare authority string. let url = Url::parse(&format!("{raw}/")).ok()?; - // ── Post-parse structural checks ────────────────────────────────────── - // Path must be exactly the "/" we appended. + // Post-parse sanity checks (defense-in-depth). if url.path() != "/" { return None; } - // Re-check userinfo fields (defense-in-depth, url crate may strip "@"). if !url.username().is_empty() || url.password().is_some() { return None; } - // No query or fragment. if url.query().is_some() || url.fragment().is_some() { return None; } - // Must have a host. url.host()?; - // Reject opaque origins (data:, blob:, …). match url.origin() { Origin::Tuple(_, _, _) => Some(url.origin()), Origin::Opaque(_) => None, } } -/// Public-to-crate entry point for [`Config::finalize`] to validate configured -/// origins at startup without exposing the private `parse_origin` function. +/// Thin wrapper exposing `parse_origin` to [`Config::finalize`]. pub(crate) fn parse_origin_str(raw: &str) -> Option { parse_origin(raw) } -// ── Host allowlist ──────────────────────────────────────────────────────────── - -/// Parses the `Host` header (or HTTP/2 `:authority` pseudo-header) into an -/// [`Authority`]. fn request_authority(request: &http::Request) -> Option { request .headers() @@ -117,20 +81,12 @@ fn request_authority(request: &http::Request) -> Option { .or_else(|| request.uri().authority().cloned()) } -/// Returns `true` when `authority` matches at least one entry in -/// `allowed_hosts`. -/// -/// Entries are plain hostnames (`gateway.example.com`) or `host:port` -/// authorities (`gateway.example.com:8080`) — no scheme prefix. +/// Returns `true` when `authority` matches an entry in `allowed_hosts`. /// -/// - Entry **without** a port → matches that host on **any** port. -/// - Entry **with** a port → matches only that exact `(host, port)` pair. -/// -/// Comparison is case-insensitive on the host component. +/// Entries are `host` or `host:port`; an entry without a port matches any port. fn authority_in_allowlist(authority: &Authority, allowed_hosts: &[String]) -> bool { let request_host = authority.host().to_ascii_lowercase(); let request_port = authority.port_u16(); - allowed_hosts.iter().any(|entry| { let (entry_host, entry_port) = match entry.rsplit_once(':') { Some((h, p)) => match p.parse::() { @@ -143,8 +99,6 @@ fn authority_in_allowlist(authority: &Authority, allowed_hosts: &[String]) -> bo }) } -// ── Response helpers ────────────────────────────────────────────────────────── - fn forbidden_response() -> Response { Response::builder() .status(StatusCode::FORBIDDEN) @@ -153,41 +107,22 @@ fn forbidden_response() -> Response { .expect("response should build") } -// ── Middleware ──────────────────────────────────────────────────────────────── - -/// Axum middleware that enforces the MCP 2026-07-28 Streamable HTTP -/// DNS-rebinding protection requirement. -/// -/// Per : +/// MCP 2026-07-28 DNS-rebinding protection middleware. /// -/// > Servers MUST validate the Origin header on all incoming connections to -/// > prevent DNS rebinding attacks. If the Origin header is present and -/// > invalid, servers MUST respond with HTTP 403 Forbidden. -/// -/// ## Decision table +/// See . /// /// | Condition | Result | /// |---|---| -/// | `mcp_allowed_hosts` set, `Host` not in list | ❌ 403 | -/// | `Origin` absent | ✅ accept (native / non-browser clients) | -/// | `Origin: null` | ❌ 403 | -/// | `Origin` malformed, has backslash / userinfo / path / query / fragment | ❌ 403 | -/// | `mcp_allowed_origins` non-empty, parsed `Origin` in list | ✅ accept | -/// | `mcp_allowed_origins` non-empty, parsed `Origin` not in list | ❌ 403 | -/// | `mcp_allowed_origins` **empty** (default) | ❌ 403 — no fallback | -/// -/// **There is no same-origin fallback.** A present `Origin` always requires -/// an explicit trusted allowlist (`CONTEXTFORGE_GATEWAY_RS_MCP_ALLOWED_ORIGINS`). -/// An empty allowlist is not a bypass; it rejects every `Origin` that is present. +/// | `mcp_allowed_hosts` set, `Host` not in list | 403 | +/// | `Origin` absent | accept (native clients omit it) | +/// | `Origin` present and in `mcp_allowed_origins` | accept | +/// | `Origin` present, `mcp_allowed_origins` empty | 403 | +/// | `Origin` present and not in list | 403 | +/// | `Origin: null`, malformed, or invalid syntax | 403 | /// -/// Port comparison uses [`url::Origin`] typed equality, which normalizes -/// default ports: `https://app.example.com:443` and `https://app.example.com` -/// are the same origin; `https://app.example.com:8443` is different. -/// -/// This layer fires before JWT claims validation, session creation, and any -/// backend fan-out. +/// No same-origin fallback — an empty allowlist rejects every present Origin. +/// Fires before JWT validation, session creation, and backend fan-out. pub async fn mcp_origin_layer(State(config): State, request: http::Request, next: Next) -> Response { - // ── 1. Host allowlist check ─────────────────────────────────────────────── if !config.mcp_allowed_hosts.is_empty() { match request_authority(&request) { None => { @@ -202,9 +137,7 @@ pub async fn mcp_origin_layer(State(config): State, request: http::Reque } } - // ── 2. Origin header check ──────────────────────────────────────────────── let Some(origin_header) = request.headers().get(header::ORIGIN) else { - // No Origin header → native / non-browser client; always allow. debug!("mcp_origin_layer - no Origin header, allowing request"); return next.run(request).await; }; @@ -214,7 +147,6 @@ pub async fn mcp_origin_layer(State(config): State, request: http::Reque return forbidden_response(); }; - // Opaque / sandbox origin — always rejected regardless of config. if origin_str.trim().eq_ignore_ascii_case("null") { warn!("mcp_origin_layer - rejected opaque null Origin"); return forbidden_response(); @@ -225,9 +157,6 @@ pub async fn mcp_origin_layer(State(config): State, request: http::Reque return forbidden_response(); }; - // ── 3. Allowlist check ──────────────────────────────────────────────────── - // An empty allowlist is not a bypass: any present Origin is rejected until - // the operator explicitly configures trusted origins. if config.mcp_parsed_origins.is_empty() { warn!("mcp_origin_layer - rejected Origin: no allowed origins configured origin = {origin_str}"); return forbidden_response(); @@ -242,8 +171,6 @@ pub async fn mcp_origin_layer(State(config): State, request: http::Reque } } -// ───────────────────────────────────────────────────────────────────────────── - #[cfg(test)] mod tests { use axum::{Router, body::to_bytes, middleware, routing::get}; @@ -403,6 +330,20 @@ mod tests { assert!(parse_origin("https://app.example.com ").is_none()); } + #[test] + fn dot_segment_path_returns_none() { + // "https://app.example.com/." — url crate collapses "/." to "/" so the + // post-parse path check cannot catch this; the pre-parse "/" check must. + assert!(parse_origin("https://app.example.com/.").is_none()); + } + + #[test] + fn embedded_tab_returns_none() { + // url crate silently strips embedded horizontal tab; pre-parse control- + // character check must reject it before the parser runs. + assert!(parse_origin("https://app.\texample.com").is_none()); + } + // ── parse_origin_str / Config::finalize unit tests ──────────────────────── #[test] @@ -723,6 +664,26 @@ mod tests { assert_eq!(res.status(), StatusCode::FORBIDDEN); } + #[tokio::test] + async fn normalized_malformed_origins_return_403() { + // Both values are normalized by the url crate into the same typed Origin + // as https://app.example.com, so they must be rejected by pre-parse + // checks before Url::parse is called. + let app = make_app(config_origins(&["https://app.example.com"])); + + for origin in ["https://app.example.com/.", "https://app.\texample.com"] { + let req = Request::builder() + .uri("/mcp") + .method("POST") + .header(header::ORIGIN, origin) + .body(Body::empty()) + .unwrap(); + + let res = app.clone().oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::FORBIDDEN, "{origin}"); + } + } + // ── middleware: null / malformed (always 403) ───────────────────────────── #[tokio::test] diff --git a/docs/book/src/security-model.md b/docs/book/src/security-model.md index 2034c825..eb04a763 100644 --- a/docs/book/src/security-model.md +++ b/docs/book/src/security-model.md @@ -99,8 +99,8 @@ malformed inputs into a valid origin. Port comparison uses typed `url::Origin` equality after RFC 3986 default-port normalization: `https://app.example.com` and `https://app.example.com:443` are the same origin; `https://app.example.com:8443` is a different origin. -Configured origins are parsed once at startup; invalid entries are logged and -skipped. +Configured origins are parsed once at startup via `Config::finalize()`; any +invalid entry causes startup to abort with an error naming every bad value. ## Local Bootstrap Helpers From 72665d7bc47a5f4e0a462e1c2dd4710fd9ee609f Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Mon, 10 Aug 2026 14:02:42 +0100 Subject: [PATCH 05/13] reduced comments Signed-off-by: prakhar-singh1928 --- .../contextforge-data-plane-lib/src/common.rs | 91 ++------------- .../src/layers/mcp_origin.rs | 104 ++++-------------- crates/contextforge-data-plane-lib/src/lib.rs | 11 +- 3 files changed, 35 insertions(+), 171 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/common.rs b/crates/contextforge-data-plane-lib/src/common.rs index df9abf27..3eecbbd1 100644 --- a/crates/contextforge-data-plane-lib/src/common.rs +++ b/crates/contextforge-data-plane-lib/src/common.rs @@ -259,57 +259,20 @@ pub struct Config { #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_LOG_ROTATION")] pub log_rotation: Option, - /// Allowlist of browser Origins permitted on MCP Streamable HTTP requests. - /// - /// Each entry must be a fully-qualified origin with scheme, e.g. - /// `https://app.example.com` or `http://localhost:3000`. - /// Port comparison is exact after RFC 3986 default-port normalization: - /// `https://app.example.com` and `https://app.example.com:443` are - /// equivalent; `https://app.example.com:8443` is distinct. - /// - /// Behaviour when this list is **non-empty**: - /// - No `Origin` header → accepted (native/non-browser clients). - /// - `Origin` present and matching an entry → accepted. - /// - `Origin` present, malformed, `null`, or not in the list → HTTP 403. - /// - /// Behaviour when this list is **empty** (default): - /// - No `Origin` header → accepted. - /// - `Origin` present → **HTTP 403** (no same-origin fallback; an empty - /// allowlist is not a bypass). - /// - /// Supply multiple origins as a comma-separated string: - /// `https://app.example.com,https://other.example.com` + /// MCP Origin allowlist. Missing `Origin` is always accepted. A present `Origin` + /// must match an entry; an empty list rejects every present `Origin` (no bypass). + /// Comma-separated: `https://app.example.com,http://localhost:3000` #[arg( long, env = "CONTEXTFORGE_GATEWAY_RS_MCP_ALLOWED_ORIGINS", value_delimiter = ',', - num_args = 0.. + num_args = 0.., + value_parser = validate_mcp_origin, )] - pub mcp_allowed_origins: Vec, - - /// Pre-parsed form of `mcp_allowed_origins`, populated by [`Config::finalize`]. - /// - /// Using `#[clap(skip)]` keeps this invisible to the CLI / env-var parser; - /// it is always derived from `mcp_allowed_origins` and never set directly. - #[clap(skip)] - pub mcp_parsed_origins: Vec, - - /// Allowlist of `Host` header values (authorities) trusted on inbound MCP - /// requests, used as the companion DNS-rebinding control. - /// - /// Each entry is a hostname or `host:port` authority, e.g. - /// `gateway.example.com` or `gateway.example.com:8080`. - /// Port is optional; an entry without a port matches that host on any port. - /// - /// When this list is **non-empty**, any request whose `Host` header does - /// not match an entry is rejected with HTTP 403 before Origin validation. - /// - /// When this list is **empty** (default), Host validation is disabled. - /// For deployments exposed directly to the internet, set this alongside - /// `mcp_allowed_origins`. - /// - /// Supply multiple hosts as a comma-separated string: - /// `gateway.example.com,gateway.example.com:443` + pub mcp_allowed_origins: Vec, + + /// MCP Host allowlist. When non-empty, requests with a non-matching `Host` header are + /// rejected with HTTP 403 before Origin validation. Comma-separated: `gateway.example.com:8080` #[arg( long, env = "CONTEXTFORGE_GATEWAY_RS_MCP_ALLOWED_HOSTS", @@ -319,45 +282,15 @@ pub struct Config { pub mcp_allowed_hosts: Vec, } -impl Config { - /// Parses `mcp_allowed_origins` into typed [`Origin`] values and stores - /// them in `mcp_parsed_origins`. Call this once after clap parsing - /// completes so the middleware can compare against pre-parsed values - /// instead of re-parsing on every request. - /// - /// Returns an error if any configured origin string is invalid. - /// The error message names all invalid entries so the operator can correct - /// the configuration without restarting repeatedly. - /// - /// # Errors - /// - /// Returns [`ConfigValidationError::InvalidMcpAllowedOrigins`] when one or - /// more entries in `mcp_allowed_origins` cannot be parsed as a valid - /// serialized origin. - pub fn finalize(&mut self) -> Result<(), ConfigValidationError> { - use crate::layers::mcp_origin::parse_origin_str; - let mut parsed = Vec::with_capacity(self.mcp_allowed_origins.len()); - let mut invalid = Vec::new(); - for s in &self.mcp_allowed_origins { - match parse_origin_str(s) { - Some(origin) => parsed.push(origin), - None => invalid.push(s.as_str()), - } - } - if !invalid.is_empty() { - return Err(ConfigValidationError::InvalidMcpAllowedOrigins(invalid.join(", "))); - } - self.mcp_parsed_origins = parsed; - Ok(()) - } +fn validate_mcp_origin(s: &str) -> Result { + crate::layers::mcp_origin::parse_origin_str(s) + .ok_or_else(|| format!("invalid MCP origin: {s}")) } #[derive(Error, Debug)] pub enum ConfigValidationError { #[error("Redis Configuration Error")] RedisConfigurationError(String), - #[error("Invalid mcp_allowed_origins entries: {0}")] - InvalidMcpAllowedOrigins(String), } impl TryFrom<&Config> for RedisConfig { diff --git a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs index 084ea313..9ca487b7 100644 --- a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs +++ b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs @@ -5,40 +5,26 @@ use url::{Origin, Url}; use crate::common::Config; -/// Parses a serialized RFC 6454 origin (`scheme://host[:port]`) into a typed -/// [`url::Origin`]. -/// -/// Returns `None` for `"null"` and for any value that is not a bare authority -/// — no path, query, fragment, userinfo, backslash, control characters, or -/// extra slashes after `://`. Port normalization (`https://x.com:443` == -/// `https://x.com`) is handled by the `url` crate. +/// Parses a serialized RFC 6454 origin (`scheme://host[:port]`) into a typed [`url::Origin`]. +/// Returns `None` for `"null"` and any value that is not a bare scheme+authority. fn parse_origin(raw: &str) -> Option { - // Reject ASCII control characters (including embedded tab) before the url - // crate silently strips them. if raw.bytes().any(|b| b < 0x20 || b == 0x7F) { return None; } - // Reject leading/trailing whitespace; also catches Unicode spaces the - // control-character check above misses. if raw != raw.trim() { return None; } if raw.eq_ignore_ascii_case("null") { return None; } - // Reject characters that the url crate normalizes away silently. if raw.contains('\\') || raw.contains('@') || raw.contains('?') || raw.contains('#') { return None; } - - // Split on "://" and validate the authority portion on the raw string. - // Any "/" in authority_part means a path (e.g. "host/." normalizes to "/" - // after parsing, so this must be caught before Url::parse runs). let (_, authority_part) = raw.split_once("://")?; if authority_part.is_empty() || authority_part.contains('/') { return None; } - // Reject trailing ":" with no port (e.g. "https://host:"). + // Trailing ":" with no port (e.g. "https://host:") is accepted by url but not a valid origin. let host_for_port_check = authority_part.trim_start_matches('['); if let Some((_, port_part)) = host_for_port_check.rsplit_once(':') && port_part.is_empty() @@ -46,14 +32,9 @@ fn parse_origin(raw: &str) -> Option { return None; } - // Append "/" so Url::parse accepts a bare authority string. let url = Url::parse(&format!("{raw}/")).ok()?; - // Post-parse sanity checks (defense-in-depth). - if url.path() != "/" { - return None; - } - if !url.username().is_empty() || url.password().is_some() { + if url.path() != "/" || !url.username().is_empty() || url.password().is_some() { return None; } if url.query().is_some() || url.fragment().is_some() { @@ -67,7 +48,6 @@ fn parse_origin(raw: &str) -> Option { } } -/// Thin wrapper exposing `parse_origin` to [`Config::finalize`]. pub(crate) fn parse_origin_str(raw: &str) -> Option { parse_origin(raw) } @@ -81,9 +61,6 @@ fn request_authority(request: &http::Request) -> Option { .or_else(|| request.uri().authority().cloned()) } -/// Returns `true` when `authority` matches an entry in `allowed_hosts`. -/// -/// Entries are `host` or `host:port`; an entry without a port matches any port. fn authority_in_allowlist(authority: &Authority, allowed_hosts: &[String]) -> bool { let request_host = authority.host().to_ascii_lowercase(); let request_port = authority.port_u16(); @@ -108,20 +85,6 @@ fn forbidden_response() -> Response { } /// MCP 2026-07-28 DNS-rebinding protection middleware. -/// -/// See . -/// -/// | Condition | Result | -/// |---|---| -/// | `mcp_allowed_hosts` set, `Host` not in list | 403 | -/// | `Origin` absent | accept (native clients omit it) | -/// | `Origin` present and in `mcp_allowed_origins` | accept | -/// | `Origin` present, `mcp_allowed_origins` empty | 403 | -/// | `Origin` present and not in list | 403 | -/// | `Origin: null`, malformed, or invalid syntax | 403 | -/// -/// No same-origin fallback — an empty allowlist rejects every present Origin. -/// Fires before JWT validation, session creation, and backend fan-out. pub async fn mcp_origin_layer(State(config): State, request: http::Request, next: Next) -> Response { if !config.mcp_allowed_hosts.is_empty() { match request_authority(&request) { @@ -157,12 +120,12 @@ pub async fn mcp_origin_layer(State(config): State, request: http::Reque return forbidden_response(); }; - if config.mcp_parsed_origins.is_empty() { + if config.mcp_allowed_origins.is_empty() { warn!("mcp_origin_layer - rejected Origin: no allowed origins configured origin = {origin_str}"); return forbidden_response(); } - if config.mcp_parsed_origins.contains(&request_origin) { + if config.mcp_allowed_origins.contains(&request_origin) { debug!("mcp_origin_layer - Origin accepted via allowlist origin = {origin_str}"); next.run(request).await } else { @@ -183,10 +146,10 @@ mod tests { // ── helpers ─────────────────────────────────────────────────────────────── fn config_origins(origins: &[&str]) -> Config { - let mut c = - Config { mcp_allowed_origins: origins.iter().map(|s| (*s).to_owned()).collect(), ..Config::default() }; - c.finalize().expect("test origins should be valid"); - c + Config { + mcp_allowed_origins: origins.iter().map(|s| parse_origin_str(s).unwrap()).collect(), + ..Config::default() + } } fn config_hosts(hosts: &[&str]) -> Config { @@ -194,13 +157,11 @@ mod tests { } fn config_origins_and_hosts(origins: &[&str], hosts: &[&str]) -> Config { - let mut c = Config { - mcp_allowed_origins: origins.iter().map(|s| (*s).to_owned()).collect(), + Config { + mcp_allowed_origins: origins.iter().map(|s| parse_origin_str(s).unwrap()).collect(), mcp_allowed_hosts: hosts.iter().map(|s| (*s).to_owned()).collect(), ..Config::default() - }; - c.finalize().expect("test origins should be valid"); - c + } } fn make_app(config: Config) -> axum::Router { @@ -344,46 +305,21 @@ mod tests { assert!(parse_origin("https://app.\texample.com").is_none()); } - // ── parse_origin_str / Config::finalize unit tests ──────────────────────── + // ── parse_origin_str unit tests ─────────────────────────────────────────── #[test] - fn empty_configured_origins_produces_empty_parsed_list() { - let mut c = Config::default(); - assert!(c.finalize().is_ok()); - assert!(c.mcp_parsed_origins.is_empty()); + fn parse_origin_str_accepts_valid_origin() { + assert!(parse_origin_str("https://app.example.com").is_some()); } - // ── Config::finalize startup-error tests ────────────────────────────────── - #[test] - fn finalize_with_valid_origins_succeeds() { - let mut c = Config { mcp_allowed_origins: vec!["https://app.example.com".to_owned()], ..Config::default() }; - assert!(c.finalize().is_ok()); - assert_eq!(c.mcp_parsed_origins.len(), 1); + fn parse_origin_str_rejects_invalid_origin() { + assert!(parse_origin_str(r"https:\bad").is_none()); } #[test] - fn finalize_with_invalid_origin_returns_error() { - let mut c = Config { - mcp_allowed_origins: vec!["https://valid.example.com".to_owned(), r"https:\bad".to_owned()], - ..Config::default() - }; - let err = c.finalize().unwrap_err(); - assert!(err.to_string().contains(r"https:\bad")); - // mcp_parsed_origins must not be partially populated on error. - assert!(c.mcp_parsed_origins.is_empty()); - } - - #[test] - fn finalize_reports_all_invalid_origins_in_error() { - let mut c = Config { - mcp_allowed_origins: vec![r"https:\bad1".to_owned(), "https:////bad2.example.com".to_owned()], - ..Config::default() - }; - let err = c.finalize().unwrap_err(); - let msg = err.to_string(); - assert!(msg.contains(r"https:\bad1"), "expected bad1 in: {msg}"); - assert!(msg.contains("https:////bad2.example.com"), "expected bad2 in: {msg}"); + fn parse_origin_str_rejects_path_component() { + assert!(parse_origin_str("https:////bad2.example.com").is_none()); } // ── authority_in_allowlist unit tests ───────────────────────────────────── diff --git a/crates/contextforge-data-plane-lib/src/lib.rs b/crates/contextforge-data-plane-lib/src/lib.rs index 508423fc..fc0f9d6c 100644 --- a/crates/contextforge-data-plane-lib/src/lib.rs +++ b/crates/contextforge-data-plane-lib/src/lib.rs @@ -65,10 +65,7 @@ pub struct Gateway { } impl Gateway { - pub async fn run_gateway(mut self) -> Result<()> { - // Parse mcp_allowed_origins into typed url::Origin values once at startup. - // Returns an error (and aborts startup) if any configured origin is invalid. - self.config.finalize()?; + pub async fn run_gateway(self) -> Result<()> { let config = &self.config; let session_manager = self.session_manager; let user_config_store = match self.user_config_store_type { @@ -85,10 +82,8 @@ impl Gateway { }; let mcp_plugin_runtime = self.plugin_runtime; - // Host and Origin validation is owned by the outer mcp_origin_layer. - // Disable RMCP's built-in checks so they do not conflict with ours. - // When the operator has configured an allowed-hosts list, pass it to - // RMCP as well for defense-in-depth; RMCP's list uses bare hostnames. + // mcp_origin_layer is the sole enforcement point; disable RMCP's built-in checks. + // Pass the host list to RMCP as well when configured (defense-in-depth). let streamable_config = if config.mcp_allowed_hosts.is_empty() { StreamableHttpServerConfig::default().disable_allowed_hosts().disable_allowed_origins() } else { From ed2a62a9e77f0835e8d216e6c297c4043829bef2 Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Mon, 10 Aug 2026 14:07:45 +0100 Subject: [PATCH 06/13] fix fmt-all Signed-off-by: prakhar-singh1928 --- crates/contextforge-data-plane-lib/src/common.rs | 3 +-- docs/book/src/security-model.md | 5 +++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/common.rs b/crates/contextforge-data-plane-lib/src/common.rs index 3eecbbd1..66a0aee5 100644 --- a/crates/contextforge-data-plane-lib/src/common.rs +++ b/crates/contextforge-data-plane-lib/src/common.rs @@ -283,8 +283,7 @@ pub struct Config { } fn validate_mcp_origin(s: &str) -> Result { - crate::layers::mcp_origin::parse_origin_str(s) - .ok_or_else(|| format!("invalid MCP origin: {s}")) + crate::layers::mcp_origin::parse_origin_str(s).ok_or_else(|| format!("invalid MCP origin: {s}")) } #[derive(Error, Debug)] diff --git a/docs/book/src/security-model.md b/docs/book/src/security-model.md index eb04a763..cccc096f 100644 --- a/docs/book/src/security-model.md +++ b/docs/book/src/security-model.md @@ -99,8 +99,9 @@ malformed inputs into a valid origin. Port comparison uses typed `url::Origin` equality after RFC 3986 default-port normalization: `https://app.example.com` and `https://app.example.com:443` are the same origin; `https://app.example.com:8443` is a different origin. -Configured origins are parsed once at startup via `Config::finalize()`; any -invalid entry causes startup to abort with an error naming every bad value. +Configured origins are validated at startup by the clap `value_parser`; any +invalid entry causes startup to abort with a formatted error before the process +reaches `run_gateway`. ## Local Bootstrap Helpers From 64259cd073c1b702b9c573d43bf15ceea265eb22 Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Mon, 10 Aug 2026 14:22:38 +0100 Subject: [PATCH 07/13] fix(security): mcp_allowed_hosts as Option to make disabled state explicit --- crates/contextforge-data-plane-lib/src/common.rs | 9 ++------- .../contextforge-data-plane-lib/src/layers/mcp_origin.rs | 8 ++++---- crates/contextforge-data-plane-lib/src/lib.rs | 8 ++++---- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/common.rs b/crates/contextforge-data-plane-lib/src/common.rs index 66a0aee5..99ab2d24 100644 --- a/crates/contextforge-data-plane-lib/src/common.rs +++ b/crates/contextforge-data-plane-lib/src/common.rs @@ -259,9 +259,6 @@ pub struct Config { #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_LOG_ROTATION")] pub log_rotation: Option, - /// MCP Origin allowlist. Missing `Origin` is always accepted. A present `Origin` - /// must match an entry; an empty list rejects every present `Origin` (no bypass). - /// Comma-separated: `https://app.example.com,http://localhost:3000` #[arg( long, env = "CONTEXTFORGE_GATEWAY_RS_MCP_ALLOWED_ORIGINS", @@ -271,15 +268,13 @@ pub struct Config { )] pub mcp_allowed_origins: Vec, - /// MCP Host allowlist. When non-empty, requests with a non-matching `Host` header are - /// rejected with HTTP 403 before Origin validation. Comma-separated: `gateway.example.com:8080` #[arg( long, env = "CONTEXTFORGE_GATEWAY_RS_MCP_ALLOWED_HOSTS", value_delimiter = ',', - num_args = 0.. + num_args = 1.. )] - pub mcp_allowed_hosts: Vec, + pub mcp_allowed_hosts: Option>, } fn validate_mcp_origin(s: &str) -> Result { diff --git a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs index 9ca487b7..21bc7508 100644 --- a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs +++ b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs @@ -86,13 +86,13 @@ fn forbidden_response() -> Response { /// MCP 2026-07-28 DNS-rebinding protection middleware. pub async fn mcp_origin_layer(State(config): State, request: http::Request, next: Next) -> Response { - if !config.mcp_allowed_hosts.is_empty() { + if let Some(ref allowed_hosts) = config.mcp_allowed_hosts { match request_authority(&request) { None => { warn!("mcp_origin_layer - rejected request: Host header missing or unparseable"); return forbidden_response(); }, - Some(ref authority) if !authority_in_allowlist(authority, &config.mcp_allowed_hosts) => { + Some(ref authority) if !authority_in_allowlist(authority, allowed_hosts) => { warn!("mcp_origin_layer - rejected request: Host not in allowlist host = {authority}"); return forbidden_response(); }, @@ -153,13 +153,13 @@ mod tests { } fn config_hosts(hosts: &[&str]) -> Config { - Config { mcp_allowed_hosts: hosts.iter().map(|s| (*s).to_owned()).collect(), ..Config::default() } + Config { mcp_allowed_hosts: Some(hosts.iter().map(|s| (*s).to_owned()).collect()), ..Config::default() } } fn config_origins_and_hosts(origins: &[&str], hosts: &[&str]) -> Config { Config { mcp_allowed_origins: origins.iter().map(|s| parse_origin_str(s).unwrap()).collect(), - mcp_allowed_hosts: hosts.iter().map(|s| (*s).to_owned()).collect(), + mcp_allowed_hosts: Some(hosts.iter().map(|s| (*s).to_owned()).collect()), ..Config::default() } } diff --git a/crates/contextforge-data-plane-lib/src/lib.rs b/crates/contextforge-data-plane-lib/src/lib.rs index fc0f9d6c..0d70e517 100644 --- a/crates/contextforge-data-plane-lib/src/lib.rs +++ b/crates/contextforge-data-plane-lib/src/lib.rs @@ -84,12 +84,12 @@ impl Gateway { // mcp_origin_layer is the sole enforcement point; disable RMCP's built-in checks. // Pass the host list to RMCP as well when configured (defense-in-depth). - let streamable_config = if config.mcp_allowed_hosts.is_empty() { - StreamableHttpServerConfig::default().disable_allowed_hosts().disable_allowed_origins() - } else { + let streamable_config = if let Some(ref hosts) = config.mcp_allowed_hosts { StreamableHttpServerConfig::default() - .with_allowed_hosts(config.mcp_allowed_hosts.iter().map(String::as_str)) + .with_allowed_hosts(hosts.iter().map(String::as_str)) .disable_allowed_origins() + } else { + StreamableHttpServerConfig::default().disable_allowed_hosts().disable_allowed_origins() }; let reqwest_backend_client = reqwest::Client::try_from(config)?; From 2104475e2009a7e3b854c3131e98aa23bc2b2081 Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Mon, 10 Aug 2026 15:06:07 +0100 Subject: [PATCH 08/13] fix(security): simplify origin/host config to Option>, drop value_parser Signed-off-by: prakhar-singh1928 --- .../contextforge-data-plane-lib/src/common.rs | 25 +++++++------------ .../src/layers/mcp_origin.rs | 15 ++++++----- 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/common.rs b/crates/contextforge-data-plane-lib/src/common.rs index 99ab2d24..0cd9c8b1 100644 --- a/crates/contextforge-data-plane-lib/src/common.rs +++ b/crates/contextforge-data-plane-lib/src/common.rs @@ -1,12 +1,3 @@ -use std::{ - fs::{self, File}, - io::{Cursor, Read}, - net::SocketAddr, - path::PathBuf, - sync::Arc, -}; -use url::Origin; - use clap::{Parser, ValueEnum}; use http::uri::Authority; use jsonwebtoken::DecodingKey; @@ -14,6 +5,13 @@ use redis::{ConnectionAddr, IntoConnectionInfo, RedisError}; use rustls_pki_types::{CertificateDer, PrivatePkcs8KeyDer, pem::PemObject}; use secret_string::SecretString; use serde::{Deserialize, Serialize}; +use std::{ + fs::{self, File}, + io::{Cursor, Read}, + net::SocketAddr, + path::PathBuf, + sync::Arc, +}; use thiserror::Error; use typed_builder::TypedBuilder; @@ -263,10 +261,9 @@ pub struct Config { long, env = "CONTEXTFORGE_GATEWAY_RS_MCP_ALLOWED_ORIGINS", value_delimiter = ',', - num_args = 0.., - value_parser = validate_mcp_origin, + num_args = 1.. )] - pub mcp_allowed_origins: Vec, + pub mcp_allowed_origins: Option>, #[arg( long, @@ -277,10 +274,6 @@ pub struct Config { pub mcp_allowed_hosts: Option>, } -fn validate_mcp_origin(s: &str) -> Result { - crate::layers::mcp_origin::parse_origin_str(s).ok_or_else(|| format!("invalid MCP origin: {s}")) -} - #[derive(Error, Debug)] pub enum ConfigValidationError { #[error("Redis Configuration Error")] diff --git a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs index 21bc7508..e30fb26f 100644 --- a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs +++ b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs @@ -48,6 +48,7 @@ fn parse_origin(raw: &str) -> Option { } } +#[cfg(test)] pub(crate) fn parse_origin_str(raw: &str) -> Option { parse_origin(raw) } @@ -120,12 +121,13 @@ pub async fn mcp_origin_layer(State(config): State, request: http::Reque return forbidden_response(); }; - if config.mcp_allowed_origins.is_empty() { + let Some(ref allowed_origins) = config.mcp_allowed_origins else { warn!("mcp_origin_layer - rejected Origin: no allowed origins configured origin = {origin_str}"); return forbidden_response(); - } + }; - if config.mcp_allowed_origins.contains(&request_origin) { + let allowed = allowed_origins.iter().filter_map(|s| parse_origin(s)).any(|o| o == request_origin); + if allowed { debug!("mcp_origin_layer - Origin accepted via allowlist origin = {origin_str}"); next.run(request).await } else { @@ -146,10 +148,7 @@ mod tests { // ── helpers ─────────────────────────────────────────────────────────────── fn config_origins(origins: &[&str]) -> Config { - Config { - mcp_allowed_origins: origins.iter().map(|s| parse_origin_str(s).unwrap()).collect(), - ..Config::default() - } + Config { mcp_allowed_origins: Some(origins.iter().map(|s| (*s).to_owned()).collect()), ..Config::default() } } fn config_hosts(hosts: &[&str]) -> Config { @@ -158,7 +157,7 @@ mod tests { fn config_origins_and_hosts(origins: &[&str], hosts: &[&str]) -> Config { Config { - mcp_allowed_origins: origins.iter().map(|s| parse_origin_str(s).unwrap()).collect(), + mcp_allowed_origins: Some(origins.iter().map(|s| (*s).to_owned()).collect()), mcp_allowed_hosts: Some(hosts.iter().map(|s| (*s).to_owned()).collect()), ..Config::default() } From 52cd7a5c310d16c2f71ec57959b32dc503a58808 Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Mon, 10 Aug 2026 15:39:51 +0100 Subject: [PATCH 09/13] fix(security): simplify parse_origin, remove redundant pre-parse guards Signed-off-by: prakhar-singh1928 --- .../src/layers/mcp_origin.rs | 186 +----------------- 1 file changed, 3 insertions(+), 183 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs index e30fb26f..8944704e 100644 --- a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs +++ b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs @@ -5,35 +5,8 @@ use url::{Origin, Url}; use crate::common::Config; -/// Parses a serialized RFC 6454 origin (`scheme://host[:port]`) into a typed [`url::Origin`]. -/// Returns `None` for `"null"` and any value that is not a bare scheme+authority. fn parse_origin(raw: &str) -> Option { - if raw.bytes().any(|b| b < 0x20 || b == 0x7F) { - return None; - } - if raw != raw.trim() { - return None; - } - if raw.eq_ignore_ascii_case("null") { - return None; - } - if raw.contains('\\') || raw.contains('@') || raw.contains('?') || raw.contains('#') { - return None; - } - let (_, authority_part) = raw.split_once("://")?; - if authority_part.is_empty() || authority_part.contains('/') { - return None; - } - // Trailing ":" with no port (e.g. "https://host:") is accepted by url but not a valid origin. - let host_for_port_check = authority_part.trim_start_matches('['); - if let Some((_, port_part)) = host_for_port_check.rsplit_once(':') - && port_part.is_empty() - { - return None; - } - let url = Url::parse(&format!("{raw}/")).ok()?; - if url.path() != "/" || !url.username().is_empty() || url.password().is_some() { return None; } @@ -41,7 +14,6 @@ fn parse_origin(raw: &str) -> Option { return None; } url.host()?; - match url.origin() { Origin::Tuple(_, _, _) => Some(url.origin()), Origin::Opaque(_) => None, @@ -111,11 +83,6 @@ pub async fn mcp_origin_layer(State(config): State, request: http::Reque return forbidden_response(); }; - if origin_str.trim().eq_ignore_ascii_case("null") { - warn!("mcp_origin_layer - rejected opaque null Origin"); - return forbidden_response(); - } - let Some(request_origin) = parse_origin(origin_str) else { warn!("mcp_origin_layer - rejected malformed Origin header origin = {origin_str}"); return forbidden_response(); @@ -213,22 +180,8 @@ mod tests { assert!(parse_origin("https://app.example.com#frag").is_none()); } - #[test] - fn origin_with_userinfo_returns_none() { - // "@" in the raw string is caught before parsing. - assert!(parse_origin("https://user@app.example.com").is_none()); - } - - #[test] - fn origin_with_backslash_returns_none() { - // url crate silently normalizes backslash to "/"; pre-parse check blocks it. - assert!(parse_origin(r"https:\app.example.com").is_none()); - assert!(parse_origin(r"https:\\app.example.com").is_none()); - } - #[test] fn origin_with_data_scheme_returns_none() { - // data: produces an opaque origin. assert!(parse_origin("data:text/plain,foo").is_none()); } @@ -236,7 +189,7 @@ mod tests { fn https_default_port_443_equals_portless() { let portless = parse_origin("https://app.example.com").unwrap(); let explicit = parse_origin("https://app.example.com:443").unwrap(); - assert_eq!(portless, explicit, "https://blah.com:443 must equal https://blah.com"); + assert_eq!(portless, explicit); } #[test] @@ -250,7 +203,7 @@ mod tests { fn non_default_port_8443_is_distinct_from_portless() { let portless = parse_origin("https://app.example.com").unwrap(); let non_default = parse_origin("https://app.example.com:8443").unwrap(); - assert_ne!(portless, non_default, "https://blah.com:8443 must NOT equal https://blah.com"); + assert_ne!(portless, non_default); } #[test] @@ -262,48 +215,10 @@ mod tests { #[test] fn ipv6_origin_parsed_correctly() { - // IPv6 address produces a valid Tuple origin. let o = parse_origin("http://[::1]:8080").unwrap(); assert!(matches!(o, Origin::Tuple(_, _, 8080))); } - // ── parse_origin: new strict syntax regressions ─────────────────────────── - - #[test] - fn extra_slashes_after_scheme_returns_none() { - // "https:///…" and "https:////…" — url crate collapses these to a valid - // host but they are not valid serialized origins. - assert!(parse_origin("https:///app.example.com").is_none()); - assert!(parse_origin("https:////app.example.com").is_none()); - } - - #[test] - fn trailing_colon_without_port_returns_none() { - // "https://app.example.com:" — url crate accepts this as no-port. - assert!(parse_origin("https://app.example.com:").is_none()); - } - - #[test] - fn leading_whitespace_returns_none() { - // url crate silently trims leading/trailing whitespace. - assert!(parse_origin(" https://app.example.com").is_none()); - assert!(parse_origin("https://app.example.com ").is_none()); - } - - #[test] - fn dot_segment_path_returns_none() { - // "https://app.example.com/." — url crate collapses "/." to "/" so the - // post-parse path check cannot catch this; the pre-parse "/" check must. - assert!(parse_origin("https://app.example.com/.").is_none()); - } - - #[test] - fn embedded_tab_returns_none() { - // url crate silently strips embedded horizontal tab; pre-parse control- - // character check must reject it before the parser runs. - assert!(parse_origin("https://app.\texample.com").is_none()); - } - // ── parse_origin_str unit tests ─────────────────────────────────────────── #[test] @@ -313,12 +228,7 @@ mod tests { #[test] fn parse_origin_str_rejects_invalid_origin() { - assert!(parse_origin_str(r"https:\bad").is_none()); - } - - #[test] - fn parse_origin_str_rejects_path_component() { - assert!(parse_origin_str("https:////bad2.example.com").is_none()); + assert!(parse_origin_str("not-an-origin").is_none()); } // ── authority_in_allowlist unit tests ───────────────────────────────────── @@ -371,7 +281,6 @@ mod tests { #[tokio::test] async fn present_origin_with_empty_allowlist_returns_403() { - // Empty allowlist is not a bypass; any present Origin must be rejected. let app = make_app(Config::default()); let req = Request::builder() .uri("/mcp") @@ -385,8 +294,6 @@ mod tests { #[tokio::test] async fn attacker_controlled_host_and_origin_match_but_still_rejected_without_allowlist() { - // DNS-rebinding: attacker controls both Host and Origin to the same value. - // Without an explicit allowlist this must be rejected, not accepted. let app = make_app(Config::default()); let req = Request::builder() .uri("http://attacker.invalid/mcp") @@ -429,7 +336,6 @@ mod tests { #[tokio::test] async fn allowlisted_origin_with_explicit_default_port_accepted() { - // Browser sends :443 explicitly; allowlist has no port — same origin. let app = make_app(config_origins(&["https://app.example.com"])); let req = Request::builder() .uri("/mcp") @@ -443,7 +349,6 @@ mod tests { #[tokio::test] async fn allowlist_entry_with_443_accepts_portless_origin() { - // Allowlist has :443; browser sends no port — same origin. let app = make_app(config_origins(&["https://app.example.com:443"])); let req = Request::builder() .uri("/mcp") @@ -457,7 +362,6 @@ mod tests { #[tokio::test] async fn non_default_port_not_in_allowlist_returns_403() { - // Allowlist entry normalizes to :443; :8443 is a different origin. let app = make_app(config_origins(&["https://app.example.com"])); let req = Request::builder() .uri("/mcp") @@ -471,7 +375,6 @@ mod tests { #[tokio::test] async fn allowlist_with_8443_does_not_match_default_port() { - // Allowlist entry is :8443; portless request is :443 — different origin. let app = make_app(config_origins(&["https://app.example.com:8443"])); let req = Request::builder() .uri("/mcp") @@ -510,12 +413,8 @@ mod tests { #[tokio::test] async fn https_origin_accepted_when_allowlisted_origin_form_request() { - // A normal HTTP/1.1 request has URI `/mcp` (origin-form, no scheme). - // The scheme cannot be inferred from the request URI; only the Origin - // header value matters for the allowlist comparison. let app = make_app(config_origins(&["https://app.example.com"])); let req = Request::builder() - // origin-form URI — no scheme .uri("/mcp") .method("POST") .header(header::HOST, "app.example.com") @@ -528,8 +427,6 @@ mod tests { #[tokio::test] async fn http_origin_rejected_when_only_https_allowlisted_origin_form_request() { - // Origin: http://... must not match an allowlist entry for https://... - // even when the request URI has no scheme and Host matches. let app = make_app(config_origins(&["https://app.example.com"])); let req = Request::builder() .uri("/mcp") @@ -542,83 +439,6 @@ mod tests { assert_eq!(res.status(), StatusCode::FORBIDDEN); } - // ── middleware: malformed-but-normalizable Origins ──────────────────────── - - #[tokio::test] - async fn backslash_origin_returns_403() { - // url crate would normalize https:\app.example.com to https://app.example.com - // but pre-parse check must reject it first. - let app = make_app(config_origins(&["https://app.example.com"])); - let req = Request::builder() - .uri("/mcp") - .method("POST") - .header(header::ORIGIN, r"https:\app.example.com") - .body(Body::empty()) - .unwrap(); - let res = app.oneshot(req).await.unwrap(); - assert_eq!(res.status(), StatusCode::FORBIDDEN); - } - - #[tokio::test] - async fn userinfo_origin_returns_403() { - // url crate strips userinfo from the origin; we must reject before that. - let app = make_app(config_origins(&["https://app.example.com"])); - let req = Request::builder() - .uri("/mcp") - .method("POST") - .header(header::ORIGIN, "https://user@app.example.com") - .body(Body::empty()) - .unwrap(); - let res = app.oneshot(req).await.unwrap(); - assert_eq!(res.status(), StatusCode::FORBIDDEN); - } - - #[tokio::test] - async fn origin_with_query_returns_403() { - let app = make_app(config_origins(&["https://app.example.com"])); - let req = Request::builder() - .uri("/mcp") - .method("POST") - .header(header::ORIGIN, "https://app.example.com?q=1") - .body(Body::empty()) - .unwrap(); - let res = app.oneshot(req).await.unwrap(); - assert_eq!(res.status(), StatusCode::FORBIDDEN); - } - - #[tokio::test] - async fn origin_with_fragment_returns_403() { - let app = make_app(config_origins(&["https://app.example.com"])); - let req = Request::builder() - .uri("/mcp") - .method("POST") - .header(header::ORIGIN, "https://app.example.com#frag") - .body(Body::empty()) - .unwrap(); - let res = app.oneshot(req).await.unwrap(); - assert_eq!(res.status(), StatusCode::FORBIDDEN); - } - - #[tokio::test] - async fn normalized_malformed_origins_return_403() { - // Both values are normalized by the url crate into the same typed Origin - // as https://app.example.com, so they must be rejected by pre-parse - // checks before Url::parse is called. - let app = make_app(config_origins(&["https://app.example.com"])); - - for origin in ["https://app.example.com/.", "https://app.\texample.com"] { - let req = Request::builder() - .uri("/mcp") - .method("POST") - .header(header::ORIGIN, origin) - .body(Body::empty()) - .unwrap(); - - let res = app.clone().oneshot(req).await.unwrap(); - assert_eq!(res.status(), StatusCode::FORBIDDEN, "{origin}"); - } - } - // ── middleware: null / malformed (always 403) ───────────────────────────── #[tokio::test] From 1b37b897c5da17d87f2b2f061a87cd5a94375975 Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Mon, 10 Aug 2026 15:56:00 +0100 Subject: [PATCH 10/13] remove redundant url.host() Signed-off-by: prakhar-singh1928 --- crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs index 8944704e..0aeddd90 100644 --- a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs +++ b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs @@ -13,7 +13,6 @@ fn parse_origin(raw: &str) -> Option { if url.query().is_some() || url.fragment().is_some() { return None; } - url.host()?; match url.origin() { Origin::Tuple(_, _, _) => Some(url.origin()), Origin::Opaque(_) => None, From 98c9279513598b9e9f20889441aa90e38ae7b39d Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Mon, 10 Aug 2026 16:07:19 +0100 Subject: [PATCH 11/13] use Authority type for mcp_allowed_hosts, drop manual string splitting Signed-off-by: prakhar-singh1928 --- .../contextforge-data-plane-lib/src/common.rs | 2 +- .../src/layers/mcp_origin.rs | 28 ++++++++----------- crates/contextforge-data-plane-lib/src/lib.rs | 3 +- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/common.rs b/crates/contextforge-data-plane-lib/src/common.rs index 0cd9c8b1..13cc9f25 100644 --- a/crates/contextforge-data-plane-lib/src/common.rs +++ b/crates/contextforge-data-plane-lib/src/common.rs @@ -271,7 +271,7 @@ pub struct Config { value_delimiter = ',', num_args = 1.. )] - pub mcp_allowed_hosts: Option>, + pub mcp_allowed_hosts: Option>, } #[derive(Error, Debug)] diff --git a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs index 0aeddd90..d75bcd6e 100644 --- a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs +++ b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs @@ -33,18 +33,11 @@ fn request_authority(request: &http::Request) -> Option { .or_else(|| request.uri().authority().cloned()) } -fn authority_in_allowlist(authority: &Authority, allowed_hosts: &[String]) -> bool { +fn authority_in_allowlist(authority: &Authority, allowed_hosts: &[Authority]) -> bool { let request_host = authority.host().to_ascii_lowercase(); let request_port = authority.port_u16(); allowed_hosts.iter().any(|entry| { - let (entry_host, entry_port) = match entry.rsplit_once(':') { - Some((h, p)) => match p.parse::() { - Ok(port) => (h.to_ascii_lowercase(), Some(port)), - Err(_) => (entry.to_ascii_lowercase(), None), - }, - None => (entry.to_ascii_lowercase(), None), - }; - entry_host == request_host && entry_port.is_none_or(|p| Some(p) == request_port) + entry.host().to_ascii_lowercase() == request_host && entry.port_u16().is_none_or(|p| Some(p) == request_port) }) } @@ -118,13 +111,16 @@ mod tests { } fn config_hosts(hosts: &[&str]) -> Config { - Config { mcp_allowed_hosts: Some(hosts.iter().map(|s| (*s).to_owned()).collect()), ..Config::default() } + Config { + mcp_allowed_hosts: Some(hosts.iter().map(|s| s.parse::().unwrap()).collect()), + ..Config::default() + } } fn config_origins_and_hosts(origins: &[&str], hosts: &[&str]) -> Config { Config { mcp_allowed_origins: Some(origins.iter().map(|s| (*s).to_owned()).collect()), - mcp_allowed_hosts: Some(hosts.iter().map(|s| (*s).to_owned()).collect()), + mcp_allowed_hosts: Some(hosts.iter().map(|s| s.parse::().unwrap()).collect()), ..Config::default() } } @@ -235,27 +231,27 @@ mod tests { #[test] fn authority_exact_host_match() { let auth = "gateway.example.com".parse::().unwrap(); - assert!(authority_in_allowlist(&auth, &["gateway.example.com".to_owned()])); + assert!(authority_in_allowlist(&auth, &["gateway.example.com".parse::().unwrap()])); } #[test] fn authority_entry_without_port_matches_any_port() { let auth = "gateway.example.com:8080".parse::().unwrap(); - assert!(authority_in_allowlist(&auth, &["gateway.example.com".to_owned()])); + assert!(authority_in_allowlist(&auth, &["gateway.example.com".parse::().unwrap()])); } #[test] fn authority_entry_with_port_matches_only_that_port() { let auth8080 = "gateway.example.com:8080".parse::().unwrap(); let auth443 = "gateway.example.com:443".parse::().unwrap(); - assert!(authority_in_allowlist(&auth8080, &["gateway.example.com:8080".to_owned()])); - assert!(!authority_in_allowlist(&auth443, &["gateway.example.com:8080".to_owned()])); + assert!(authority_in_allowlist(&auth8080, &["gateway.example.com:8080".parse::().unwrap()])); + assert!(!authority_in_allowlist(&auth443, &["gateway.example.com:8080".parse::().unwrap()])); } #[test] fn authority_mismatch_returns_false() { let auth = "evil.example.com".parse::().unwrap(); - assert!(!authority_in_allowlist(&auth, &["gateway.example.com".to_owned()])); + assert!(!authority_in_allowlist(&auth, &["gateway.example.com".parse::().unwrap()])); } // ── middleware: no Origin ───────────────────────────────────────────────── diff --git a/crates/contextforge-data-plane-lib/src/lib.rs b/crates/contextforge-data-plane-lib/src/lib.rs index 0d70e517..4a7f9b32 100644 --- a/crates/contextforge-data-plane-lib/src/lib.rs +++ b/crates/contextforge-data-plane-lib/src/lib.rs @@ -4,6 +4,7 @@ use axum::middleware; use axum_otel_metrics::HttpMetricsLayerBuilder; use contextforge_data_plane_cpex::GatewayPluginRuntimeHandle; use futures::FutureExt; +use http::uri::Authority; use jsonwebtoken::DecodingKey; use rmcp::transport::{ StreamableHttpServerConfig, @@ -86,7 +87,7 @@ impl Gateway { // Pass the host list to RMCP as well when configured (defense-in-depth). let streamable_config = if let Some(ref hosts) = config.mcp_allowed_hosts { StreamableHttpServerConfig::default() - .with_allowed_hosts(hosts.iter().map(String::as_str)) + .with_allowed_hosts(hosts.iter().map(Authority::as_str)) .disable_allowed_origins() } else { StreamableHttpServerConfig::default().disable_allowed_hosts().disable_allowed_origins() From b4b9b63990391916019425e1920bda6a9131bdc9 Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Mon, 10 Aug 2026 16:24:25 +0100 Subject: [PATCH 12/13] drop to_ascii_lowercase Signed-off-by: prakhar-singh1928 --- crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs index d75bcd6e..2c174e36 100644 --- a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs +++ b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs @@ -34,10 +34,9 @@ fn request_authority(request: &http::Request) -> Option { } fn authority_in_allowlist(authority: &Authority, allowed_hosts: &[Authority]) -> bool { - let request_host = authority.host().to_ascii_lowercase(); let request_port = authority.port_u16(); allowed_hosts.iter().any(|entry| { - entry.host().to_ascii_lowercase() == request_host && entry.port_u16().is_none_or(|p| Some(p) == request_port) + entry.host().eq_ignore_ascii_case(authority.host()) && entry.port_u16().is_none_or(|p| Some(p) == request_port) }) } From d39455b92c89e18217c686390088d9ec6651ea81 Mon Sep 17 00:00:00 2001 From: prakhar-singh1928 Date: Mon, 10 Aug 2026 16:30:52 +0100 Subject: [PATCH 13/13] use url::Url for mcp_allowed_origins, drop string parsing at request time Signed-off-by: prakhar-singh1928 --- crates/contextforge-data-plane-lib/src/common.rs | 3 ++- .../src/layers/mcp_origin.rs | 11 +++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/common.rs b/crates/contextforge-data-plane-lib/src/common.rs index 13cc9f25..b736b35e 100644 --- a/crates/contextforge-data-plane-lib/src/common.rs +++ b/crates/contextforge-data-plane-lib/src/common.rs @@ -14,6 +14,7 @@ use std::{ }; use thiserror::Error; use typed_builder::TypedBuilder; +use url::Url; use crate::user_config_store::UserConfigStore; @@ -263,7 +264,7 @@ pub struct Config { value_delimiter = ',', num_args = 1.. )] - pub mcp_allowed_origins: Option>, + pub mcp_allowed_origins: Option>, #[arg( long, diff --git a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs index 2c174e36..302bcf88 100644 --- a/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs +++ b/crates/contextforge-data-plane-lib/src/layers/mcp_origin.rs @@ -84,7 +84,7 @@ pub async fn mcp_origin_layer(State(config): State, request: http::Reque return forbidden_response(); }; - let allowed = allowed_origins.iter().filter_map(|s| parse_origin(s)).any(|o| o == request_origin); + let allowed = allowed_origins.iter().map(Url::origin).any(|o| o == request_origin); if allowed { debug!("mcp_origin_layer - Origin accepted via allowlist origin = {origin_str}"); next.run(request).await @@ -99,14 +99,17 @@ mod tests { use axum::{Router, body::to_bytes, middleware, routing::get}; use http::{Request, StatusCode}; use tower::ServiceExt; - use url::Origin; + use url::{Origin, Url}; use super::*; // ── helpers ─────────────────────────────────────────────────────────────── fn config_origins(origins: &[&str]) -> Config { - Config { mcp_allowed_origins: Some(origins.iter().map(|s| (*s).to_owned()).collect()), ..Config::default() } + Config { + mcp_allowed_origins: Some(origins.iter().map(|s| s.parse::().unwrap()).collect()), + ..Config::default() + } } fn config_hosts(hosts: &[&str]) -> Config { @@ -118,7 +121,7 @@ mod tests { fn config_origins_and_hosts(origins: &[&str], hosts: &[&str]) -> Config { Config { - mcp_allowed_origins: Some(origins.iter().map(|s| (*s).to_owned()).collect()), + mcp_allowed_origins: Some(origins.iter().map(|s| s.parse::().unwrap()).collect()), mcp_allowed_hosts: Some(hosts.iter().map(|s| s.parse::().unwrap()).collect()), ..Config::default() }