fix(mcp): allow browser MCP clients through CORS preflight (#179)
The OPTIONS preflight hardcoded Access-Control-Allow-Headers to just Content-Type. Any browser MCP client sending mcp-protocol-version (per the MCP spec) or Authorization / X-API-Key (typical for hosted deployments) failed the preflight and the browser blocked the actual request. Add those three to the allow-list and a 24h Max-Age so browsers don't re-preflight every call. Adds an integration test that drives the server with a real preflight request. Closes #175.
This commit is contained in:
1 parent
a3b21bbb70
commit
d4d749d850
2 files changed
+92
-1
No files matched your search
@@ -83,10 +83,15 @@ async fn handle_connection(
|
||||
|
||||
match method {
|
||||
"OPTIONS" => {
|
||||
// mcp-protocol-version is part of the MCP spec, Authorization /
|
||||
// X-API-Key are common for hosted deployments. Without these
|
||||
// listed the browser preflight check fails and blocks the actual
|
||||
// request.
|
||||
let hdr = "HTTP/1.1 204 No Content\r\n\
|
||||
Access-Control-Allow-Origin: *\r\n\
|
||||
Access-Control-Allow-Methods: GET, POST, OPTIONS\r\n\
|
||||
Access-Control-Allow-Headers: Content-Type\r\n\
|
||||
Access-Control-Allow-Headers: Content-Type, Authorization, X-API-Key, mcp-protocol-version\r\n\
|
||||
Access-Control-Max-Age: 86400\r\n\
|
||||
\r\n";
|
||||
writer.write_all(hdr.as_bytes()).await?;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,86 @@
|
||||
//! Regression test for issue #175: the MCP HTTP server's OPTIONS preflight
|
||||
//! response must list every header a browser MCP client may send, including
|
||||
//! `mcp-protocol-version` (from the MCP spec) and `Authorization` /
|
||||
//! `X-API-Key` (common in hosted deployments). Otherwise the browser blocks
|
||||
//! the actual request with a CORS error.
|
||||
|
||||
use std::net::TcpListener as StdListener;
|
||||
use std::time::Duration;
|
||||
use tokio::io::{AsyncReadExt, AsyncWriteExt};
|
||||
use tokio::net::TcpStream;
|
||||
use tokio::task::LocalSet;
|
||||
use tokio::time::{sleep, timeout};
|
||||
|
||||
fn pick_free_port() -> u16 {
|
||||
let l = StdListener::bind("127.0.0.1:0").unwrap();
|
||||
let p = l.local_addr().unwrap().port();
|
||||
drop(l);
|
||||
p
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn options_preflight_lists_required_browser_headers() {
|
||||
let port = pick_free_port();
|
||||
let local = LocalSet::new();
|
||||
|
||||
// Spawn the MCP HTTP server. It loops forever; we abort the task at the
|
||||
// end of the test. `current_thread` + LocalSet is required because the
|
||||
// browser state is `!Send` (Page holds V8 handles).
|
||||
let server = local.spawn_local(async move {
|
||||
let _ = obscura_mcp::http::run(port, None, None, false).await;
|
||||
});
|
||||
|
||||
local.run_until(async {
|
||||
// Wait for the listener to bind.
|
||||
for _ in 0..40 {
|
||||
if TcpStream::connect(("127.0.0.1", port)).await.is_ok() {
|
||||
break;
|
||||
}
|
||||
sleep(Duration::from_millis(50)).await;
|
||||
}
|
||||
|
||||
let mut stream = TcpStream::connect(("127.0.0.1", port))
|
||||
.await
|
||||
.expect("MCP server did not come up");
|
||||
let req = b"OPTIONS /mcp HTTP/1.1\r\n\
|
||||
Host: 127.0.0.1\r\n\
|
||||
Origin: https://dashboard.example.com\r\n\
|
||||
Access-Control-Request-Method: POST\r\n\
|
||||
Access-Control-Request-Headers: Content-Type, mcp-protocol-version, Authorization\r\n\
|
||||
\r\n";
|
||||
stream.write_all(req).await.unwrap();
|
||||
stream.flush().await.unwrap();
|
||||
|
||||
let mut buf = [0u8; 4096];
|
||||
let n = timeout(Duration::from_secs(2), stream.read(&mut buf))
|
||||
.await
|
||||
.expect("read timed out")
|
||||
.expect("read failed");
|
||||
let response = String::from_utf8_lossy(&buf[..n]).to_string();
|
||||
|
||||
server.abort();
|
||||
|
||||
assert!(
|
||||
response.starts_with("HTTP/1.1 204"),
|
||||
"expected 204 No Content preflight, got:\n{response}"
|
||||
);
|
||||
let lc = response.to_lowercase();
|
||||
assert!(
|
||||
lc.contains("access-control-allow-headers:"),
|
||||
"preflight must include Access-Control-Allow-Headers; got:\n{response}"
|
||||
);
|
||||
assert!(
|
||||
lc.contains("mcp-protocol-version"),
|
||||
"ACAH must list mcp-protocol-version (per MCP spec); got:\n{response}"
|
||||
);
|
||||
assert!(
|
||||
lc.contains("authorization"),
|
||||
"ACAH must list Authorization for hosted deployments; got:\n{response}"
|
||||
);
|
||||
assert!(
|
||||
lc.contains("x-api-key"),
|
||||
"ACAH must list X-API-Key for hosted deployments; got:\n{response}"
|
||||
);
|
||||
})
|
||||
.await;
|
||||
}
|
||||
Reference in new issue
Block a user