Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions packages/codeflow-mcp/src/invoke/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,12 @@ describe("TOOLS registry", () => {
* present. When `MCP_ALLOWED_ORIGIN` is set (strict mode), only allowlisted
* origins are echoed; untrusted or missing origins fall back to the first
* entry in the allowlist.
*
* The credential-bearing headers `authorization` and `x-api-key` are
* intentionally OMITTED from `Access-Control-Allow-Headers` so a cross-origin
* attacker cannot make the browser send them via preflight. Non-credential
* headers (`Content-Type`, `x-request-id`) are retained so legitimate clients
* can still issue preflight requests.
*/
describe("createHttpServer CORS handling", () => {
let server: Server;
Expand Down Expand Up @@ -358,7 +364,7 @@ describe("createHttpServer CORS handling", () => {
expect(res.allowOrigin).toBe(origin);
});

it("still includes authorization in Access-Control-Allow-Headers (echoing Origin does not strip credentials headers)", async () => {
it("omits authorization and x-api-key from Access-Control-Allow-Headers to prevent cross-origin credential exposure", async () => {
const res = await new Promise<{ allowHeaders: string | string[] | undefined }>((resolve, reject) => {
const req = httpRequest(
`${baseUrl}/`,
Expand All @@ -375,8 +381,16 @@ describe("createHttpServer CORS handling", () => {
req.end();
});
const allowHeaders = Array.isArray(res.allowHeaders) ? res.allowHeaders.join(",") : res.allowHeaders ?? "";
expect(allowHeaders).toContain("authorization");
expect(allowHeaders).toContain("x-api-key");
const normalized = allowHeaders.toLowerCase();
// Credential-bearing headers must NOT be advertised cross-origin: a wildcard
// (or echoed) Origin combined with these in Allow-Headers would let any
// malicious site make the browser send Authorization / X-API-Key to this
// endpoint via a preflight.
expect(normalized).not.toContain("authorization");
expect(normalized).not.toContain("x-api-key");
// Non-credential headers must still be present so legitimate preflights succeed.
expect(normalized).toContain("content-type");
expect(normalized).toContain("x-request-id");
});
});

Expand Down
9 changes: 7 additions & 2 deletions packages/codeflow-mcp/src/invoke/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,11 @@ export async function startStdioServer(): Promise<void> {
* `process.env` is read at call time so tests can stub the variable.
*/
function buildCorsHeaders(requestOrigin?: string): Record<string, string> {
// We omit `authorization` and `x-api-key` from Allow-Headers so the server
// does not advertise acceptance of credentialed headers. Combined with the
// explicit origin allowlist / echoed-Origin behaviour above, this prevents
// a cross-origin attacker from making the browser send Authorization /
// X-API-Key to this endpoint.
const allowedEnv = (process.env["MCP_ALLOWED_ORIGIN"] ?? "").trim();
if (allowedEnv.length > 0) {
const allowed = new Set(
Expand All @@ -222,14 +227,14 @@ function buildCorsHeaders(requestOrigin?: string): Record<string, string> {
return {
"Access-Control-Allow-Origin": allowOrigin,
"Access-Control-Allow-Methods": "GET, POST, OPTIONS",
"Access-Control-Allow-Headers": "Content-Type, authorization, x-api-key, x-request-id",
"Access-Control-Allow-Headers": "Content-Type, x-request-id",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Allow auth headers for allowlisted browser clients

When MCP_ALLOWED_ORIGIN is configured for a browser client, any Authorization or x-api-key request triggers a preflight whose requested header is now absent here, so the browser rejects the request before the JSON-RPC POST is sent. This breaks the documented browser-based HTTP transport and its invokeMcpTool(..., { "x-api-key": ... }) usage (docs/codeflow-mcp.md:61, docs/codeflow-mcp.md:89) even for explicitly allowlisted origins; retain these headers in strict mode (or make them configurable) rather than removing them globally.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Trusted browser clients lose access 🐞 Bug ≡ Correctness

buildCorsHeaders removes authorization and x-api-key even when MCP_ALLOWED_ORIGIN identifies
the requesting origin as trusted. Browser clients using the documented header forwarding trigger a
preflight that omits their requested credential header, so the browser blocks both tool listing and
invocation before their POST requests reach the server.
Agent Prompt
## Issue description
The strict CORS path removes `authorization` and `x-api-key` for explicitly allowlisted origins, breaking supported browser clients that pass those headers through `listMcpTools` or `invokeMcpTool`.

## Fix Focus Areas
- packages/codeflow-mcp/src/invoke/index.ts[214-237]
- packages/codeflow-mcp/src/invoke/index.test.ts[367-393]

## Recommended Fix
Retain `authorization` and `x-api-key` in `Access-Control-Allow-Headers` when the request origin matches `MCP_ALLOWED_ORIGIN`, while continuing to omit them in permissive mode. Add tests proving an allowlisted origin can preflight credential headers and an arbitrary origin cannot.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

};
}
const allowOrigin = requestOrigin && requestOrigin.length > 0 ? requestOrigin : "*";
return {
"Access-Control-Allow-Origin": allowOrigin,
"Access-Control-Allow-Methods": "GET, POST, OPTIONS",
"Access-Control-Allow-Headers": "Content-Type, authorization, x-api-key, x-request-id",
"Access-Control-Allow-Headers": "Content-Type, x-request-id",
};
}

Expand Down