From 1f7cd9cc2f84cdfce365a45605cf95bb18786593 Mon Sep 17 00:00:00 2001 From: Hunter Bown Date: Mon, 11 May 2026 22:12:19 -0500 Subject: [PATCH] feat(mcp): custom HTTP headers per server for authenticated gateways MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #1454. Harvested from PR #1456 by @Oliver-ZPLiu. Adds `pub headers: HashMap` to McpServerConfig, threaded through HttpTransport::new and StreamableHttpTransport so every outbound POST applies the user-configured headers after the fixed Accept / Content-Type framing. Mirrors the field shape that Claude Code, Codex, and OpenCode already accept in their MCP config formats — unblocks Hugging Face MCP, GitHub MCP, Atlassian Rovo MCP, and any other Streamable HTTP gateway that needs a Bearer token or API key. Defense-in-depth filter (`is_safe_custom_header`) drops: * empty / whitespace-only keys (would surface as a reqwest builder error mid-send and abort the connection); * `Accept` / `Content-Type` duplicates (the MCP Streamable HTTP protocol negotiates on these exact values; a stray override would silently break tool discovery); * values containing ASCII CR or LF (response-splitting defense against a misbehaving proxy). Skipped headers emit a `tracing::warn!` and the rest of the request still goes out, so a single bad entry can't take down a server. Scope-limited vs PR #1456: * SSE legacy transport intentionally not threaded (follow-up). The PR didn't cover it; modern MCP servers are Streamable HTTP. * No env-var interpolation in v0.8.31 — headers are sent literally, matching the PR. Documented in the field doc so users know tokens pasted directly into mcp.json live there as plain text. `${VAR}` substitution is a follow-up. 8 new tests (the original PR shipped without any): config round-trip with custom headers, empty headers omitted from serialized output, accept-normal-auth, reject empty key, reject CR/LF in value, reject Accept/Content-Type override (case- insensitive), and StreamableHttpTransport stores headers. Empty-headers MCP fixtures updated at 10 sites across mcp.rs and main.rs to match the new struct shape. --- CHANGELOG.md | 15 +++ crates/tui/src/main.rs | 4 + crates/tui/src/mcp.rs | 201 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 217 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b0a109e0..b157b589 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 a 50 KB file in 16 KB slices instead of dragging the whole thing into the conversation context on every turn. PDFs continue to use `pages`; `start_line` / `max_lines` apply to text files only. +- **MCP HTTP servers accept custom headers** for authentication + (#1454, harvested from PR #1456 by **@Oliver-ZPLiu**). Mirrors the + `headers` field that Claude Code, Codex, and OpenCode already + accept in their MCP config — add e.g. + `"headers": { "Authorization": "Bearer ${HF_TOKEN}" }` under any + HTTP server entry in `~/.deepseek/mcp.json` and the headers are + sent on every Streamable HTTP request. Headers are sent + literally — env-var interpolation is a follow-up, so tokens + pasted directly into mcp.json live there as plain text. The + Streamable HTTP transport filters out empty keys, framing + overrides (`Accept`, `Content-Type`), and CR/LF in values + (response-splitting defense) so a single bad entry can't break + protocol negotiation or smuggle a header through a misbehaving + proxy. Stdio servers (`command`-based) and the legacy SSE + transport ignore the field; SSE coverage is a follow-up. ## [0.8.30] - 2026-05-11 diff --git a/crates/tui/src/main.rs b/crates/tui/src/main.rs index 4be57411..75228022 100644 --- a/crates/tui/src/main.rs +++ b/crates/tui/src/main.rs @@ -940,6 +940,7 @@ fn mcp_template_json() -> Result { required: false, enabled_tools: Vec::new(), disabled_tools: Vec::new(), + headers: std::collections::HashMap::new(), }, ); serde_json::to_string_pretty(&cfg) @@ -3328,6 +3329,7 @@ async fn run_mcp_command(config: &Config, command: McpCommand) -> Result<()> { required: false, enabled_tools: Vec::new(), disabled_tools: Vec::new(), + headers: std::collections::HashMap::new(), }, ); save_mcp_config(&config_path, &cfg)?; @@ -3413,6 +3415,7 @@ async fn run_mcp_command(config: &Config, command: McpCommand) -> Result<()> { required: false, enabled_tools: Vec::new(), disabled_tools: Vec::new(), + headers: std::collections::HashMap::new(), }, ); save_mcp_config(&config_path, &cfg)?; @@ -5214,6 +5217,7 @@ mod doctor_mcp_tests { required: false, enabled_tools: Vec::new(), disabled_tools: Vec::new(), + headers: std::collections::HashMap::new(), } } diff --git a/crates/tui/src/mcp.rs b/crates/tui/src/mcp.rs index c8519f31..874016b8 100644 --- a/crates/tui/src/mcp.rs +++ b/crates/tui/src/mcp.rs @@ -42,6 +42,36 @@ fn validate_mcp_config_path(path: &Path) -> Result<()> { Ok(()) } +/// Predicate for [`StreamableHttpTransport::send`]'s custom-header pass. +/// +/// We accept whatever reqwest's `HeaderName::try_from` / +/// `HeaderValue::try_from` would accept, but with three extra rules: +/// +/// 1. Reject empty / whitespace-only keys — these would surface as a +/// request-builder error mid-send and abort the whole connection. +/// 2. Reject keys that duplicate the framing we already emit +/// (`Accept`, `Content-Type`). The MCP Streamable HTTP transport +/// relies on those exact values for protocol negotiation; a stray +/// user override could silently break tool discovery. +/// 3. Reject values containing ASCII CR or LF. reqwest already +/// rejects those, but the explicit check makes the failure path +/// visible (a `tracing::warn!` instead of an obscure +/// builder error) and documents the response-splitting +/// defense. +/// +/// Returning `false` means "skip this header"; the rest of the +/// request still goes out. +fn is_safe_custom_header(key: &str, value: &str) -> bool { + let trimmed = key.trim(); + if trimmed.is_empty() { + return false; + } + if trimmed.eq_ignore_ascii_case("accept") || trimmed.eq_ignore_ascii_case("content-type") { + return false; + } + !value.contains('\r') && !value.contains('\n') +} + /// Mask a URL so any embedded credentials in the userinfo portion (e.g. /// `https://user:secret@host`) are replaced with `***`. Failures fall back to /// the original string so we don't lose context — we never want masking to @@ -203,6 +233,30 @@ pub struct McpServerConfig { pub enabled_tools: Vec, #[serde(default)] pub disabled_tools: Vec, + /// Extra HTTP headers sent with every request to this MCP server. + /// Only the HTTP transports (streamable HTTP today; SSE in a + /// follow-up) honor this — `command`-based stdio servers ignore it. + /// + /// Mirrors the `headers` field that Claude Code, Codex, and + /// OpenCode already accept in their MCP config formats. Use it to + /// authenticate against gateways that require a Bearer token or + /// API key, e.g.: + /// + /// ```jsonc + /// "huggingface": { + /// "url": "https://huggingface.co/api/mcp", + /// "headers": { "Authorization": "Bearer ${HF_TOKEN}" } + /// } + /// ``` + /// + /// Header keys and values are passed through as-is — we do not + /// substitute environment variables in v0.8.31. If you store a + /// real token here, the value lives in plain text in + /// `~/.deepseek/mcp.json`; treat that file with the same care + /// as any other secret-bearing config. + #[serde(default)] + #[serde(skip_serializing_if = "HashMap::is_empty")] + pub headers: HashMap, } fn default_enabled() -> bool { @@ -496,6 +550,11 @@ enum HttpTransportMode { struct StreamableHttpTransport { client: reqwest::Client, url: String, + /// Extra headers applied to every outbound POST. Populated from + /// [`McpServerConfig::headers`]; an empty map is the no-auth + /// default. See `apply_custom_headers` for the filtering pass that + /// runs before each request. + headers: HashMap, pending_messages: VecDeque>, } @@ -693,6 +752,7 @@ impl HttpTransport { fn new( client: reqwest::Client, url: String, + headers: HashMap, cancel_token: tokio_util::sync::CancellationToken, endpoint_timeout: Duration, ) -> Self { @@ -700,6 +760,7 @@ impl HttpTransport { mode: HttpTransportMode::Streamable(StreamableHttpTransport::new( client.clone(), url.clone(), + headers, )), client, base_url: url, @@ -756,20 +817,44 @@ impl McpTransport for HttpTransport { } impl StreamableHttpTransport { - fn new(client: reqwest::Client, url: String) -> Self { + fn new(client: reqwest::Client, url: String, headers: HashMap) -> Self { Self { client, url, + headers, pending_messages: VecDeque::new(), } } async fn send(&mut self, msg: Vec) -> std::result::Result<(), StreamableSendError> { - let response = self + let mut request = self .client .post(&self.url) .header(ACCEPT, "application/json, text/event-stream") - .header(CONTENT_TYPE, "application/json") + .header(CONTENT_TYPE, "application/json"); + // Apply user-configured custom headers. Skip: + // * empty / whitespace-only keys (would produce reqwest builder + // errors mid-request and abort the whole connection); + // * keys that duplicate the framing we already set (`Accept`, + // `Content-Type`) so a stray entry can't break protocol + // negotiation; + // * values containing CR/LF, which would enable response- + // splitting style requests on a misbehaving proxy. + // reqwest itself rejects malformed header names/values; the + // duplicates and control-char filter is purely defense in + // depth. + for (key, value) in &self.headers { + if !is_safe_custom_header(key, value) { + tracing::warn!( + target: "mcp", + "skipping unsafe MCP header {:?} (empty/control-char/reserved)", + key + ); + continue; + } + request = request.header(key.as_str(), value.as_str()); + } + let response = request .body(msg) .send() .await @@ -1022,6 +1107,7 @@ impl McpConnection { Box::new(HttpTransport::new( client, url.clone(), + config.headers.clone(), cancel_token.clone(), Duration::from_secs(connect_timeout_secs), )) @@ -2256,6 +2342,7 @@ fn mcp_template_json() -> Result { required: false, enabled_tools: Vec::new(), disabled_tools: Vec::new(), + headers: HashMap::new(), }, ); serde_json::to_string_pretty(&cfg).context("Failed to render MCP template JSON") @@ -2307,6 +2394,7 @@ pub fn add_server_config( required: false, enabled_tools: Vec::new(), disabled_tools: Vec::new(), + headers: HashMap::new(), }, ); save_config(path, &cfg) @@ -2559,6 +2647,109 @@ mod tests { assert_eq!(server.env.get("FOO"), Some(&"bar".to_string())); } + #[test] + fn mcp_server_config_parses_custom_headers() { + let json = r#"{ + "servers": { + "hf": { + "url": "https://example.invalid/mcp", + "headers": { + "Authorization": "Bearer tok", + "X-Org": "anthropic" + } + } + } + }"#; + let cfg: McpConfig = serde_json::from_str(json).unwrap(); + let hf = cfg.servers.get("hf").expect("server present"); + assert_eq!( + hf.headers.get("Authorization"), + Some(&"Bearer tok".to_string()) + ); + assert_eq!(hf.headers.get("X-Org"), Some(&"anthropic".to_string())); + } + + #[test] + fn mcp_server_config_omits_headers_when_empty() { + // Empty headers map should not appear in the serialized output — + // older mcp.json files written before v0.8.31 must round-trip + // unchanged so a `mcp save` from a fresh install doesn't add + // dead keys. + let cfg = McpServerConfig { + command: Some("node".into()), + args: vec!["server.js".into()], + env: HashMap::new(), + url: None, + connect_timeout: None, + execute_timeout: None, + read_timeout: None, + disabled: false, + enabled: true, + required: false, + enabled_tools: Vec::new(), + disabled_tools: Vec::new(), + headers: HashMap::new(), + }; + let serialized = serde_json::to_string(&cfg).unwrap(); + assert!( + !serialized.contains("\"headers\""), + "empty headers must be omitted: {serialized}" + ); + } + + #[test] + fn is_safe_custom_header_accepts_normal_auth_pairs() { + assert!(is_safe_custom_header("Authorization", "Bearer tok")); + assert!(is_safe_custom_header("X-Api-Key", "deadbeef")); + assert!(is_safe_custom_header("x-org", "anthropic")); + } + + #[test] + fn is_safe_custom_header_rejects_empty_or_whitespace_key() { + assert!(!is_safe_custom_header("", "value")); + assert!(!is_safe_custom_header(" ", "value")); + } + + #[test] + fn is_safe_custom_header_rejects_response_splitting_values() { + assert!( + !is_safe_custom_header("X-Foo", "abc\r\nSet-Cookie: evil=1"), + "CRLF in value must reject — response-splitting defense" + ); + assert!( + !is_safe_custom_header("X-Foo", "abc\nbar"), + "bare LF in value must reject" + ); + assert!( + !is_safe_custom_header("X-Foo", "abc\rbar"), + "bare CR in value must reject" + ); + } + + #[test] + fn is_safe_custom_header_rejects_protocol_framing_overrides() { + // The MCP Streamable HTTP transport relies on its own + // Accept / Content-Type values for protocol negotiation; + // a stray user override would silently break tool discovery. + assert!(!is_safe_custom_header("Accept", "text/plain")); + assert!(!is_safe_custom_header("accept", "text/plain")); + assert!(!is_safe_custom_header("Content-Type", "text/plain")); + assert!(!is_safe_custom_header("CONTENT-TYPE", "x/y")); + } + + #[test] + fn streamable_http_transport_stores_headers() { + let client = reqwest::Client::new(); + let mut headers = HashMap::new(); + headers.insert("Authorization".to_string(), "Bearer xyz".to_string()); + let transport = StreamableHttpTransport::new( + client, + "https://example.invalid/mcp".to_string(), + headers.clone(), + ); + assert_eq!(transport.headers, headers); + } + #[test] fn test_mcp_config_parse_mcp_servers_alias_and_snapshot() { let dir = tempfile::tempdir().unwrap(); @@ -2646,6 +2837,7 @@ mod tests { required: false, enabled_tools: Vec::new(), disabled_tools: Vec::new(), + headers: HashMap::new(), }; assert_eq!(server_with_override.effective_connect_timeout(&global), 20); @@ -2755,6 +2947,7 @@ mod tests { required: false, enabled_tools: Vec::new(), disabled_tools: Vec::new(), + headers: HashMap::new(), } } @@ -2923,6 +3116,7 @@ mod tests { required: false, enabled_tools: Vec::new(), disabled_tools: Vec::new(), + headers: HashMap::new(), }, ); assert_ne!( @@ -3158,6 +3352,7 @@ mod tests { required: false, enabled_tools: Vec::new(), disabled_tools: Vec::new(), + headers: HashMap::new(), }; let conn = McpConnection::connect_with_policy(