fix(codeflow): [CI/rebase] CORS exposes x-api-key and authorization headers cross-origin with wildcard - #44
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoPrevent cross-origin credential headers in CodeFlow MCP
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02f5e7047f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "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", |
There was a problem hiding this comment.
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 👍 / 👎.
Code Review by Qodo
1. Trusted browser clients lose access
|
| "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", |
There was a problem hiding this comment.
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
Original task: task_audit_1780457435798_004
PR branch: claude-lane/task_audit_1780457435798_004-20260603120220
CI failure: merge-conflict
Strategy: rebase
Hint: PR has merge conflicts with main. Run: git fetch origin && git rebase origin/main && git push --force-with-lease. The watcher's open_pr_for_task will detect the existing PR.
Failure log (last 3000 chars)
Workflow
git log origin/main..claude-lane/task_audit_1780457435798_004-20260603120220)claude-lane/task_audit_1780457435798_004-20260603120220) — the worktree is already set up by the dispatcherfix(ci): <one-line summary>(single line)Automated by DevPulse dispatcher.