fix(serve-http): gate CORS preflight headers behind the Origin allowlist by yashkot007 · Pull Request #983 · garrytan/gbrain

yashkot007 · 2026-05-14T05:59:29Z

Summary

corsPreflightHeaders() in src/mcp/http-transport.ts returns

Access-Control-Allow-Methods: POST, GET, OPTIONS
Access-Control-Allow-Headers: Content-Type, Authorization, Accept

unconditionally on every OPTIONS request, regardless of whether the request Origin is in GBRAIN_HTTP_CORS_ORIGIN. Only Access-Control-Allow-Origin and Vary: Origin get the allowlist gate.

Compare to the actual-request path (corsHeaders(), lines 132-139), which gates everything behind the allowlist:

function corsHeaders(origin: string | null, extra: Record<string, string> = {}): Record<string, string> {
  const headers: Record<string, string> = { ...extra };
  if (corsAllowlist && origin && corsAllowlist.has(origin)) {
    headers['Access-Control-Allow-Origin'] = origin;
    headers['Vary'] = 'Origin';
  }
  return headers;
}

The asymmetry: a CORS preflight from a non-allowlisted origin (Origin: https://evil.example) still receives the full method + header surface in the response, even though the actual request that would follow gets blocked by the missing Access-Control-Allow-Origin. The data path stays safe — browsers refuse to read a response without matching Access-Control-Allow-Origin — but the API surface is leaked to anyone probing the preflight, which is asymmetric to the default-deny posture documented in SECURITY.md:

Default-deny: no Access-Control-Allow-Origin header is sent unless an allowlist is configured.

This fix applies the same default-deny posture symmetrically to the preflight surface.

Fix

 function corsPreflightHeaders(origin: string | null): Record<string, string> {
-  const headers: Record<string, string> = {
+  if (!corsAllowlist || !origin || !corsAllowlist.has(origin)) {
+    return { 'Vary': 'Origin' };
+  }
+  return {
+    'Access-Control-Allow-Origin': origin,
     'Access-Control-Allow-Methods': 'POST, GET, OPTIONS',
     'Access-Control-Allow-Headers': 'Content-Type, Authorization, Accept',
+    'Vary': 'Origin',
   };
-  if (corsAllowlist && origin && corsAllowlist.has(origin)) {
-    headers['Access-Control-Allow-Origin'] = origin;
-    headers['Vary'] = 'Origin';
-  }
-  return headers;
 }

Behavior

`GBRAIN_HTTP_CORS_ORIGIN`	Request `Origin`	Pre-fix	Post-fix
unset	any	Allow-Methods + Allow-Headers leak	`Vary: Origin` only
set	matches allowlist	full preflight headers	full preflight headers (unchanged)
set	doesn't match	Allow-Methods + Allow-Headers leak	`Vary: Origin` only

Vary: Origin is retained even on the deny path so cached preflights stay cache-correct if the same path is later re-fetched from an allowlisted Origin.

Test plan

Three new test cases in test/http-transport.test.ts (slotted into the existing describe('http-transport: CORS', ...) block right after test 12):

12a — no GBRAIN_HTTP_CORS_ORIGIN + OPTIONS request: asserts no Allow-Origin, no Allow-Methods, no Allow-Headers
12b — env set + matching Origin: asserts full preflight headers including Vary: Origin
12c — env set + non-matching Origin: asserts no Allow-Methods / Allow-Headers leak, but Vary: Origin still present (cache-correctness)

Verification:

bun test test/http-transport.test.ts — 27 pass / 0 fail (24 existing + 3 new)
bun run verify — all 11 invariant gates pass (privacy, jsonb, source-id projection, progress, test-isolation, wasm, admin-build, admin-scope-drift, cli-exec, system-of-record, eval-glossary, typecheck)
No new dependencies, no breaking changes for existing allowlisted clients

🤖 Generated with Claude Code

`corsPreflightHeaders()` (http-transport.ts) returned `Access-Control-Allow-Methods: POST, GET, OPTIONS` and `Access-Control-Allow-Headers: Content-Type, Authorization, Accept` unconditionally on every OPTIONS request, regardless of whether the request `Origin` was in `GBRAIN_HTTP_CORS_ORIGIN`. Only `Access-Control-Allow-Origin` and `Vary: Origin` were gated. Result: an unallowlisted caller doing a CORS preflight learns the API surface (allowed methods + headers) even though the actual request would be blocked by the missing `Allow-Origin`. The data path stays safe — browser refuses to read the actual response — but the preflight itself leaks the surface, asymmetric to how `corsHeaders()` (lines 132-139) already gates everything correctly for the actual-request path. Fix: default-deny posture applied to the preflight surface too. When the Origin isn't in the allowlist, return only `Vary: Origin` (so cached preflights stay cache-correct if the same path is later re-fetched from an allowlisted Origin) and zero CORS-permission headers. When the Origin matches the allowlist, return the full set (Allow-Origin + Allow-Methods + Allow-Headers + Vary). Test plan (3 new cases in test/http-transport.test.ts, all passing): - 12a: no GBRAIN_HTTP_CORS_ORIGIN → OPTIONS returns no permission headers - 12b: env set + matching Origin → full preflight headers, Vary: Origin - 12c: env set + non-matching Origin → no Allow-Methods/Headers leak, Vary: Origin still present Also verified: - `bun test test/http-transport.test.ts` — 27 pass / 0 fail (24 existing + 3 new) - `bun run verify` — all 11 invariant gates pass (privacy, jsonb, source-id projection, progress, test-isolation, wasm, admin-build, admin-scope-drift, cli-exec, system-of-record, eval-glossary, typecheck) - No new dependencies, no breaking changes 🤖 Generated with [Claude Code](https://claude.com/claude-code)

…ut DCR + validator surface (#1403) * v0.41.3.0 fix(security/mcp): OAuth CORS lockdown, pre-register without DCR, validator surface Three expanded cherry-picks plus codex-surfaced live-CORS fix, parser rewrite, atomicity fix, DCR validator gate, SECURITY.md reconciliation. What ships - gbrain auth register-client gets --redirect-uri (repeatable) and --token-endpoint-auth-method flags so the SECURITY.md-recommended "pre-register without --enable-dcr" path actually works for claude.ai and ChatGPT custom connectors. - ALLOWED_TOKEN_ENDPOINT_AUTH_METHODS = {client_secret_post, client_secret_basic, none} validator gates all three registration entry points (CLI, admin endpoint, DCR /register) so --enable-dcr is no longer the looser path. - Live Express OAuth server (/mcp, /token, /authorize, /register, /revoke) was using default-wide-open cors() middleware — every origin could complete a token exchange from a logged-in operator's browser. Now default-deny; allowlist via GBRAIN_HTTP_CORS_ORIGIN. - GBRAIN_HTTP_TRUST_PROXY env var on Express server with the same semantics as the legacy bearer transport already had. Default 'loopback' preserved. SECURITY.md doc rewritten to match reality (was lying that trust proxy was "disabled by default" while code hardcoded 'loopback'). - Admin endpoint registration now atomic — INSERT-then-UPDATE for public clients replaced with single INSERT via the new registerClientManual(..., tokenEndpointAuthMethod) parameter (codex outside-voice F4 catch). - Legacy transport corsHeaders + corsPreflightHeaders consolidated into one function gated on the allowlist for BOTH Allow-Origin and Allow-Methods/Headers (codex F1; #983 thematically). Surfaced by D7 codex outside-voice review on the v0.41.3 plan: F1 (live Express CORS wide-open), F2 (indexOf parser couldn't do repeatable flags), F3 (client_secret_basic missing from validator), F4 (admin endpoint INSERT-then-UPDATE atomicity), F5 (DCR path bypassed validator), F6 (env var already existed on legacy transport), F7 (SECURITY.md vs impl doc disagreement). Tests: 183 directly-touched cases green. Three new test files (test/serve-http-trust-proxy.test.ts, test/serve-http-cors.test.ts, test/auth-register-client-args.test.ts) + 18 new oauth.test.ts cases + 4 IRON RULE CORS preflight regressions. Plan: ~/.claude/plans/system-instruction-you-are-working-wise-piglet.md (D1-D11 captured, codex outside-voice integrated, GSTACK REVIEW REPORT verdict CLEARED). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(test): audit-writer readRecent calendar-boundary flake writer.log() uses real `new Date()` for filename computation, but the test mocked `now` to 2026-05-22. When CI runs on a date in a different ISO week (e.g. 2026-05-25 W22 vs the mocked W21), log() writes to one file but readRecent(now) reads a different one — zero events overlap, expect(2).toBe(0) fails. Fix: write events directly to the file matching the test's mocked `now` via writer.computeFilename(now), same pattern the cross-week straddle test (line 234+) already used for the previous-week event. Pre-existing test bug, surfaced when CI rolled past the week boundary the original author wrote against. Not introduced by v0.41.3.0; fix included here because /ship found it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

…ut DCR + validator surface (garrytan#1403) * v0.41.3.0 fix(security/mcp): OAuth CORS lockdown, pre-register without DCR, validator surface Three expanded cherry-picks plus codex-surfaced live-CORS fix, parser rewrite, atomicity fix, DCR validator gate, SECURITY.md reconciliation. What ships - gbrain auth register-client gets --redirect-uri (repeatable) and --token-endpoint-auth-method flags so the SECURITY.md-recommended "pre-register without --enable-dcr" path actually works for claude.ai and ChatGPT custom connectors. - ALLOWED_TOKEN_ENDPOINT_AUTH_METHODS = {client_secret_post, client_secret_basic, none} validator gates all three registration entry points (CLI, admin endpoint, DCR /register) so --enable-dcr is no longer the looser path. - Live Express OAuth server (/mcp, /token, /authorize, /register, /revoke) was using default-wide-open cors() middleware — every origin could complete a token exchange from a logged-in operator's browser. Now default-deny; allowlist via GBRAIN_HTTP_CORS_ORIGIN. - GBRAIN_HTTP_TRUST_PROXY env var on Express server with the same semantics as the legacy bearer transport already had. Default 'loopback' preserved. SECURITY.md doc rewritten to match reality (was lying that trust proxy was "disabled by default" while code hardcoded 'loopback'). - Admin endpoint registration now atomic — INSERT-then-UPDATE for public clients replaced with single INSERT via the new registerClientManual(..., tokenEndpointAuthMethod) parameter (codex outside-voice F4 catch). - Legacy transport corsHeaders + corsPreflightHeaders consolidated into one function gated on the allowlist for BOTH Allow-Origin and Allow-Methods/Headers (codex F1; garrytan#983 thematically). Surfaced by D7 codex outside-voice review on the v0.41.3 plan: F1 (live Express CORS wide-open), F2 (indexOf parser couldn't do repeatable flags), F3 (client_secret_basic missing from validator), F4 (admin endpoint INSERT-then-UPDATE atomicity), F5 (DCR path bypassed validator), F6 (env var already existed on legacy transport), F7 (SECURITY.md vs impl doc disagreement). Tests: 183 directly-touched cases green. Three new test files (test/serve-http-trust-proxy.test.ts, test/serve-http-cors.test.ts, test/auth-register-client-args.test.ts) + 18 new oauth.test.ts cases + 4 IRON RULE CORS preflight regressions. Plan: ~/.claude/plans/system-instruction-you-are-working-wise-piglet.md (D1-D11 captured, codex outside-voice integrated, GSTACK REVIEW REPORT verdict CLEARED). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(test): audit-writer readRecent calendar-boundary flake writer.log() uses real `new Date()` for filename computation, but the test mocked `now` to 2026-05-22. When CI runs on a date in a different ISO week (e.g. 2026-05-25 W22 vs the mocked W21), log() writes to one file but readRecent(now) reads a different one — zero events overlap, expect(2).toBe(0) fails. Fix: write events directly to the file matching the test's mocked `now` via writer.computeFilename(now), same pattern the cross-week straddle test (line 234+) already used for the previous-week event. Pre-existing test bug, surfaced when CI rolled past the week boundary the original author wrote against. Not introduced by v0.41.3.0; fix included here because /ship found it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

This was referenced May 18, 2026

v0.36.1.1 fix-wave: community PR triage + 28 atomic fixes #1182

Merged

v0.41.3.0 fix(security/mcp): OAuth CORS lockdown + pre-register without DCR + validator surface #1403

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(serve-http): gate CORS preflight headers behind the Origin allowlist#983

fix(serve-http): gate CORS preflight headers behind the Origin allowlist#983
yashkot007 wants to merge 1 commit into
garrytan:masterfrom
yashkot007:fix/cors-preflight-gating

yashkot007 commented May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

yashkot007 commented May 14, 2026

Summary

Fix

Behavior

Test plan

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant