fix: guard MCP streamable-HTTP client redirects against SSRF (loopback/link-local/metadata) (#11501)
Some checks failed
CI / Check GDK API Reference is Up-to-Date (push) Waiting to run
CI / Test and Lint Electron Desktop App (push) Blocked by required conditions
CI / Test Desktop Updater Install (ubuntu-latest) (push) Blocked by required conditions
CI / Test Desktop Updater Install (windows-latest) (push) Blocked by required conditions
Prepare GDK release / Create or update GDK release PR (push) Waiting to run
MCP Conformance / Build Conformance Binaries (push) Waiting to run
MCP Conformance / Conformance 2025-11-25 / 0.1.16 (push) Blocked by required conditions
MCP Conformance / Conformance 2025-11-25 / 0.2.0-alpha.10 (push) Blocked by required conditions
MCP Conformance / Conformance 2026-07-28 / 0.2.0-alpha.10 (push) Blocked by required conditions
Create Minor Release PR / check-version-bump-pr (push) Waiting to run
Create Minor Release PR / release (push) Blocked by required conditions
Live Provider Tests / check-fork (push) Waiting to run
Live Provider Tests / changes (push) Blocked by required conditions
Canary / Prepare Version (push) Waiting to run
Canary / build-cli-linux (push) Blocked by required conditions
Canary / Upload Install Script (push) Waiting to run
Canary / bundle-macos-arm64 (push) Blocked by required conditions
Canary / bundle-macos-x64 (push) Blocked by required conditions
Canary / bundle-windows (push) Blocked by required conditions
Canary / bundle-windows-cuda (push) Blocked by required conditions
Canary / Release (push) Blocked by required conditions
Cargo Deny / deny (push) Waiting to run
Unused Dependencies / machete (push) Waiting to run
CI / changes (push) Waiting to run
CI / goose ACP npm packages (push) Blocked by required conditions
CI / Check Rust Code Format (push) Blocked by required conditions
CI / Build and Test Rust Project (push) Blocked by required conditions
CI / Build Lean ACP Binary (push) Blocked by required conditions
CI / Check goose-sdk UniFFI (push) Blocked by required conditions
CI / Test Kotlin GDK package (push) Blocked by required conditions
CI / Build and Test TLS Backend (native-tls) (push) Blocked by required conditions
CI / Build and Test TLS Backend (rustls-tls) (push) Blocked by required conditions
CI / Build and Test Roaming Feature (push) Blocked by required conditions
CI / Build Rust Project on Windows (push) Waiting to run
CI / Check MSRV (push) Blocked by required conditions
CI / Lint Rust Code (push) Blocked by required conditions
CI / Check Generated Schemas are Up-to-Date (push) Blocked by required conditions
CI / Check GDK Release Configuration (push) Waiting to run
Live Provider Tests / Build Binary (push) Blocked by required conditions
Live Provider Tests / Smoke Tests (push) Blocked by required conditions
Live Provider Tests / Smoke Tests (Code Execution) (push) Blocked by required conditions
Live Provider Tests / Compaction Tests (push) Blocked by required conditions
Publish Docker Image / docker (push) Waiting to run
Scorecard supply-chain security / Scorecard analysis (push) Waiting to run
Buzz automation / test (push) Has been cancelled

Co-authored-by: trickyfalcon <trickyfalcon@users.noreply.github.com>
Co-authored-by: Jasper Hugo <jasper@jasperhugo.com>
This commit is contained in:
Mo 2026-09-23 19:00:58 +00:00 • committed by GitHub
parent ac6222029f
commit 6a2c3dadf6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 116 additions and 7 deletions

View file

@ -216,7 +216,9 @@ fn http_client(
timeout: Duration,
) -> ExtensionResult<reqwest::Client> {
#[allow(unused_mut)]
let mut builder = reqwest::Client::builder().default_headers(header_map(headers)?);
let mut builder = reqwest::Client::builder()
.redirect(reqwest::redirect::Policy::none())
.default_headers(header_map(headers)?);
#[cfg(target_os = "linux")]
{
builder = builder.tcp_user_timeout(Some(timeout));
@ -953,6 +955,116 @@ mod tests {
);
}
/// Option-B verification (unauthenticated client): a server 3xx must be
/// surfaced to the transport and the redirect target must never be contacted.
#[tokio::test]
async fn test_redirect_not_followed_unauthenticated() {
use wiremock::matchers::any;
use wiremock::{Mock, MockServer, ResponseTemplate};
let target = MockServer::start().await;
Mock::given(any())
.respond_with(ResponseTemplate::new(200))
.mount(&target)
.await;
let redirector = MockServer::start().await;
Mock::given(any())
.respond_with(ResponseTemplate::new(307).insert_header("location", target.uri()))
.mount(&redirector)
.await;
let temp_dir = tempdir().unwrap();
let result = connect(
test_params(&redirector.uri(), HashMap::new(), temp_dir.path()),
None,
Box::new(InMemoryCredentialStore::new()),
)
.await;
// The 3xx is surfaced (connection fails); it is not silently followed.
assert!(result.is_err(), "expected 3xx to be surfaced as an error");
// The redirect target must never have been contacted.
let target_requests = target.received_requests().await.unwrap();
assert!(
target_requests.is_empty(),
"redirect target was contacted: {target_requests:?}"
);
// The redirector itself did receive the request.
assert!(!redirector.received_requests().await.unwrap().is_empty());
}
/// Option-B verification (authenticated client): same guarantee on the
/// `connect_with_auth` path, and the custom auth header is sent to the
/// redirector — never forwarded to the redirect target.
#[tokio::test]
async fn test_redirect_not_followed_with_auth_headers() {
use rmcp::transport::auth::{
InMemoryCredentialStore, OAuthTokenResponse, StoredCredentials,
};
use wiremock::matchers::any;
use wiremock::{Mock, MockServer, ResponseTemplate};
let target = MockServer::start().await;
Mock::given(any())
.respond_with(ResponseTemplate::new(200))
.mount(&target)
.await;
let redirector = MockServer::start().await;
Mock::given(any())
.respond_with(ResponseTemplate::new(307).insert_header("location", target.uri()))
.mount(&redirector)
.await;
let mut headers = HashMap::new();
headers.insert("x-api-key".to_string(), "test-secret-redirect".to_string());
let token_response: OAuthTokenResponse = serde_json::from_value(serde_json::json!({
"access_token": "fake-test-token",
"token_type": "bearer",
}))
.expect("valid fake token JSON");
let creds = StoredCredentials::new(
"test-client".to_string(),
Some(token_response),
vec![],
None,
);
let store = InMemoryCredentialStore::new();
store.save(creds).await.unwrap();
let mut auth_manager = rmcp::transport::AuthorizationManager::new(redirector.uri())
.await
.expect("AuthorizationManager::new should not make network calls");
auth_manager.set_credential_store(store);
let temp_dir = tempdir().unwrap();
let result = connect_with_auth(
auth_manager,
&redirector.uri(),
&headers,
test_ctx(temp_dir.path()),
)
.await;
assert!(result.is_err(), "expected 3xx to be surfaced as an error");
let target_requests = target.received_requests().await.unwrap();
assert!(
target_requests.is_empty(),
"redirect target was contacted: {target_requests:?}"
);
// The auth header reached the redirector only.
let redirect_requests = redirector.received_requests().await.unwrap();
assert!(!redirect_requests.is_empty());
assert!(redirect_requests.iter().any(|req| {
req.headers
.get("x-api-key")
.map(|v| v == "test-secret-redirect")
.unwrap_or(false)
}));
}
mod static_oauth_client {
use super::*;

View file

@ -454,14 +454,11 @@ mod tests {
#[cfg(unix)]
#[tokio::test]
async fn command_runs_with_configured_cwd_and_resolves_relative_path() {
use std::os::unix::fs::PermissionsExt;
let dir = tempfile::tempdir().unwrap();
let script_path = dir.path().join("get-token.sh");
std::fs::write(&script_path, "#!/bin/sh\npwd\n").unwrap();
std::fs::set_permissions(&script_path, std::fs::Permissions::from_mode(0o755)).unwrap();
// Avoid ETXTBSY when parallel subprocesses inherit a freshly written executable.
std::os::unix::fs::symlink("/bin/pwd", dir.path().join("get-token")).unwrap();
let mut config = auth_config("./get-token.sh", Vec::<&str>::new(), 3600);
let mut config = auth_config("./get-token", Vec::<&str>::new(), 3600);
config.cwd = Some(dir.path().to_string_lossy().to_string());
let provider = CommandAuthProvider::new(&config, "Authorization", "Bearer ");