-
Notifications
You must be signed in to change notification settings - Fork 1k
feat: Add CORS configuration for browser-based MCP clients #713
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add cors middleware to example servers with Mcp-Session-Id exposed - Add CORS documentation section to README - Configure minimal CORS settings (only expose required headers) This enables browser-based clients to connect to MCP servers by properly exposing the Mcp-Session-Id header required for session management. Reported-by: Jerome
6c23ed6
to
bda811a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just need to merge README as there is already a warning about CORS
@@ -584,6 +584,26 @@ app.listen(3000); | |||
> ); | |||
> ``` | |||
|
|||
|
|||
#### CORS Configuration for Browser-Based Clients | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was added in #633
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah up - that's just above this, I've merged them
@@ -96,6 +97,12 @@ const getServer = () => { | |||
const app = express(); | |||
app.use(express.json()); | |||
|
|||
// Configure CORS to expose Mcp-Session-Id header for browser-based clients | |||
app.use(cors({ | |||
origin: '*', // Allow all origins - adjust as needed for production |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd probably add a very big "WARNING" here not to include "*"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If your MCP server will be accessible by browsers, then you might actually want this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary
Mcp-Session-Id
headerProblem
Browser-based MCP clients cannot access the
Mcp-Session-Id
header from initialization responses due to CORS restrictions. Without this header, they cannot establish sessions with MCP servers.Solution
This PR adds the
cors
npm package to example servers and configures it to expose theMcp-Session-Id
header viaAccess-Control-Expose-Headers
. The configuration is minimal, only exposing what's necessary for MCP protocol operation.Changes
cors
import and middleware to:sseAndStreamableHttpCompatibleServer.ts
simpleStatelessStreamableHttp.ts
jsonResponseStreamableHttp.ts
Test plan
Mcp-Session-Id
header from responsesReported-by: Jerome