Add CORS preflight FIX - #214
Merged
Jagadeeshftw merged 1 commit intoJul 28, 2026
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
================================================================================
PULL REQUEST
Title:
Add CORS preflight (OPTIONS) tests verifying the corsOrigins allowlist
Base: main
Branch: add-cors-preflight-tests
Type: test-only (no production code changes)
Closes: the CORS preflight test-coverage issue
PROBLEM
app.ts mounts the cors middleware with config.corsOrigins (or unrestricted, if
unset):
However, no test exercised an actual OPTIONS preflight request against a
mutating route (e.g. POST /api/v1/anchors) to confirm the configured
corsOrigins allowlist is actually honored by the cors package's preflight
response headers.
Verified before this change:
HTTP-layer behavior.
Origin header) - never an OPTIONS preflight.
A misconfigured or silently-ignored CORS setting is a real cross-origin
exposure risk, and before this change it would not have failed any test.
WHAT CHANGED
Test-only change: one new file, src/cors.test.ts, containing three tests.
Each test boots createApp() under a specific CORS_ORIGIN configuration and
issues a supertest OPTIONS request to /api/v1/anchors - the preflight a
browser would send before the mutating route POST /api/v1/anchors - with an
Access-Control-Request-Method: POST header:
Allowlisted origin is reflected
CORS_ORIGIN=https://allowed.example, request Origin matches it.
Asserts: status 204 and
Access-Control-Allow-Origin: https://allowed.example (reflected, not a
wildcard), plus Access-Control-Allow-Methods permitting POST.
Disallowed origin is not reflected
CORS_ORIGIN=https://allowed.example, request Origin=https://evil.example.
Asserts: status 204 and the Access-Control-Allow-Origin header is ABSENT
(and explicitly not the disallowed origin).
Unrestricted default still holds when CORS_ORIGIN is unset
Asserts: status 204 and Access-Control-Allow-Origin: *,
matching the behavior documented in config.ts and README.md
("If unset, every origin is permitted").
Conventions followed:
process.env.CORS_ORIGIN is captured up front and restored in afterEach,
so the suite is hermetic regardless of the ambient environment.
is the canonical mutating route already used throughout the app-level
suites (maintenanceMode.test.ts, bodyLimit.test.ts, etc.).
middleware terminates any OPTIONS request itself (preflightContinue
defaults to false, optionsSuccessStatus 204), an array allowlist uses
exact-string matching and reflects a matching request origin, a
non-matching origin gets no Access-Control-Allow-Origin header, and the
default origin '*' is emitted only when no allowlist is configured.
VALIDATION
Full CI pipeline (lint -> build -> test) green, run locally on Node 20 to
match CI:
Mutation check (simulated and reverted; not part of this diff):
This is exactly the silently-ignored-allowlist risk described in the issue,
so the new tests demonstrably guard against it. app.ts was restored after the
check; git diff confirms it is byte-identical to HEAD, and git status shows
the only change in this PR is the new test file.
REVIEWER NOTE
No production code was changed, so this PR cannot alter runtime behavior on
its own; it only locks the current behavior in. If the cors mount in app.ts
ever evolves (e.g. credentials support, dynamic origin callbacks), extend
these preflight tests alongside it so the allowlist stays verified at the
HTTP layer rather than only in config.ts's parsing logic.
FILES CHANGED
Total: 1 file created, 0 files modified.
Closes #151