Reverse decisions for CORS and strict content type parsing.

[SamMorrowDrums]

#842 introduces to regressions to valid content types and applies erroneous CORS protections that are not the right security model for MCP servers, and just lock out web clients.

• SDK Regression: Re-allow browser-based MCP clients via CORS
  
• [Fix Content-Type rejection for application/json; charset=utf-8] (Fix Content-Type rejection for application/json; charset=utf-8 github/github-mcp-server#2362) ✔️ already fixed (also technically it's true the protocol itself does say only the json header is valid and nothing about charset 😅)

It would be great if you can confirm that the intention is to remove the CORS stuff, as we will use the temporary compatibility flag, but we expect that in hindsight once the flag is gone, it will be replaced with that being the default (whereas the release docs say the opposite).

Thanks so much, just want to ensure we are aligned on intention here 🙏

[maciej-kisiel]

@jba What do you think about reverting the default CORS protection enablement? Admittedly, it may have been a bit overeager change from my side to address a security advisory and I didn't realize the impact it would have on web clients.

[jba]

I'd like to understand this a little better.

You say "any client can try to connect to an MCP and the CORS headers don't matter"
(github/github-mcp-server#2359 (comment)). Are you saying what we have now (installing a zero http.CrossOriginProtection) is completely ineffective?

The CVE says, "a remote attacker can exploit this Cross-Site Request Forgery (CSRF) vulnerability... [b]y sending browser-generated cross-site POST requests to a local server without proper validation of the Origin header". What is erroneous about this?

You say you are using "the temporary compatibility flag," presumably the debug env var. But your PR is actually using the API as intended, by setting StreamableHTTPOptions.CrossOrigin.CrossOriginProtection.

In short, why aren't we working as intended, locking down the server as much as possible while allowing fully flexible overrides?

[SamMorrowDrums]

Thanks for looking into this.

I'd like to understand this a little better.

You say "any client can try to connect to an MCP and the CORS headers don't matter"
(github/github-mcp-server#2359 (comment)). Are you saying what we have now (installing a zero http.CrossOriginProtection) is completely ineffective?

Sorry if I was confusing, I meant to say it only blocks web based connection attempts (and only ones that have a concept of CORS at that).

It is effective at blocking web based attacks, it just shouldn't be auto-enabled in my opinion. It blocks valid web based MCP clients. Also web browsers do not distinguish between a local web page (like MCP Inspector) and a remote web page making cross origin requests.

The CVE says, "a remote attacker can exploit this Cross-Site Request Forgery (CSRF) vulnerability... [b]y sending browser-generated cross-site POST requests to a local server without proper validation of the Origin header". What is erroneous about this?

Erroneous might be overkill, it's possible. I just don't think it's a meaningful vulnerability. Browsers could already do this, and MCP isn't really special here.

The same could be said for any local service running with open ports. If the security model is that nothing external should ever hit the port then it's already risky.

The user also has to visit a malicious webpage for this to work too, while the server is running.

You say you are using "the temporary compatibility flag," presumably the debug env var. But your PR is actually using the API as intended, by setting StreamableHTTPOptions.CrossOrigin.CrossOriginProtection.

Apologies, we changed track after I posted this and decided it was impractical to inject env given the range of ways the server is run.

In short, why aren't we working as intended, locking down the server as much as possible while allowing fully flexible overrides?

I not certain this is a position the MCP SDK itself should be taking. I am fine with option to set it, but blocking all valid web clients still feels extreme to me.

[SamMorrowDrums]

I guess another way to put this is, I am not certain the SDK should be adding stuff outside the MCP spec to a transport it has been given without the caller asking it to do so.

[jba]

Thanks, that was very useful. We should (and must) leave the option and use it when it is provided, but the default should be to do nothing. We will need another MCPGODEBUG option, enableoriginverification, which should last at least one release.

[maciej-kisiel]

Default enforcement is what actually made me add it to the SDK API. If it's optional, the added benefit is just bringing more attention to the problem, because the users can equally easily do:

handler := mcp.NewStreamableHTTPHandler(...)
protection := http.NewCrossOriginProtection()
// ... configure protection in a desired way.
protectedHandler = protection.Handler(handler)
http.ListenAndServe(url, protectedHandler)

IMO that should be sufficient as long as the security concern is documented. Unfortunately, now the damage is done and we must support the option to pass the protection to the handler, but I will add appropriate paragraph to docs/rough_edges.md to recommend removing it in the next major version of the SDK.

We should also partially roll back 15e93a2 which adds this protection to the SSE transport as well.

[jba]

I think you should mark the field as deprecated, and mention the alternative you sketched above.