fix(security): close unauthenticated MCP host execution and proxy bypass - #50
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus 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 |
reprewindai-dev
left a comment
There was a problem hiding this comment.
Security containment: keep this PR draft. The source direction is appropriate (admin-gated MCP registry, production local-process denial, no full process.env inheritance, fail-closed direct proxy auth), but exact-head cAPI CI run 32644995839 concludes failure. Before promotion, add/confirm focused negative tests for unauthenticated GET/POST registry access, production local-process denial, missing/wrong X-API-Key on every proxy method, and proof the internal/admin credentials are stripped upstream. Also audit remote MCP/OpenAPI registration URLs for SSRF/egress policy: authentication alone does not make caller-selected remote endpoints safe. Runtime remains NOT_VERIFIED until the deployed SHA, HTTP/protocol identity, listener 3003, and Traefik routing agree; public ingress exposure must be independently checked.
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Uh oh!
There was an error while loading. Please reload this page.
Incident
Production cAPI exposes
/api/mcp/serversand the direct/api/proxy/{serverId}/{...path}forwarder without an effective authentication gate. The MCP registry acceptslocal-processdescriptors, and the stdio transport inherited the complete cAPI process environment.Fix
/api/mcp/serverslocal-processMCP in production; permit it only in non-production whenCAPI_ALLOW_LOCAL_PROCESS_MCP=true...process.envinheritance from spawned stdio MCP processes; only a minimal non-secret runtime allowlist plus explicitly supplied descriptor env is passedBYOS_INTERNAL_API_KEYon every direct proxy methodTruth boundary
This is source remediation. It does not by itself remove
capi.veklom.comfrom public ingress or prove the deployed runtime is fixed. Production ingress should still be closed/restricted immediately, then this commit deployed and verified with unauthenticated negative tests.Required deployment checks
GET /api/mcp/servers=> 401/503, never inventoryPOST /api/mcp/servers=> 401/503, never process spawnlocal-processregistration => 403X-API-Key=> 401/503