fix(cors): warn on credentials with `null` origin by pi0x · Pull Request #1464 · h3js/h3

pi0x · 2026-07-14T12:34:56Z

origin: "null" combined with credentials: true is dangerous: sandboxed iframes, data:/file: documents, and other opaque origins all send Origin: null, so credentials would be shared across untrusted contexts. This adds a warn-once message in resolveCorsOptions, mirroring the existing wildcard-origin and wildcard-exposeHeaders warnings. The hazard is also documented on the origin option JSDoc, and a unit test asserts the warning fires.

🤖 Generated with Claude Code

Summary by CodeRabbit

Documentation
- Clarified the security risks of combining credentialed CORS requests with origin: "null".
Bug Fixes
- Added warnings when credentialed CORS configuration allows opaque origins, including direct or list-based "null" origins.
- Warnings are deduplicated to avoid repeated notifications.
Tests
- Added coverage verifying warnings for both supported "null" origin configurations.

`origin: "null"` combined with `credentials: true` is dangerous: sandboxed iframes, `data:`/`file:` documents, and other opaque origins all send `Origin: null`, so credentials would be shared across untrusted contexts. Emit a warn-once message (mirroring the existing wildcard warnings) and document the hazard on the `origin` JSDoc. Co-Authored-By: Claude Fable 5 <[email protected]>

codecov · 2026-07-14T12:36:40Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

pi0x

Independent review (did not author this change). Tests pass (test/unit/cors.test.ts, 44 passed) and pnpm lint is clean.

LGTM — well-scoped DX warning with no behavior change to actual CORS resolution:

Trigger resolved.credentials && resolved.origin === "null" is correct and properly gated.
Reuses the existing warnOnce helper, so dedup semantics match the wildcard warnings; the distinct message string dedupes independently.
Test is well-isolated: exposeHeaders: ["X-Custom"] + non-wildcard origin ensures exactly one warning fires, so toHaveBeenCalledOnce() is meaningful.
JSDoc is accurate — sandboxed iframes and data:/file:/opaque origins do serialize Origin: null.

Non-blocking suggestion: the warning only fires for the top-level string origin: "null", not for an array containing it (e.g. origin: ["null"]). Unlike the wildcard case — where ["*"] is inert because no real request sends Origin: * — an array containing "null" is a live hazard: isCorsOriginAllowed does origin === _origin over the array, so Origin: null matches and gets reflected with credentials. Consider also covering Array.isArray(resolved.origin) && resolved.origin.includes("null"). Fine to defer since it matches the current wildcard-warning pattern and is warn-only.

🤖 Generated with Claude Code

The array path in `isCorsOriginAllowed` does an exact string comparison, so `origin: ["null"]` matches `Origin: null` requests and is just as hazardous with credentials as the top-level `origin: "null"` string. Co-Authored-By: Claude Fable 5 <[email protected]>

pi0x · 2026-07-14T12:38:46Z

Addressed the review gap in e66906d: the warning now also fires when the origin array contains "null" (the array path does an exact string match, so origin: ["null"] is a live hazard with credentials). Added a unit test for the array case; tests and lint pass.

coderabbitai · 2026-07-14T12:38:47Z

📝 Walkthrough

Walkthrough

Changes

Credentialed null-origin CORS handling

Layer / File(s)	Summary
Document and validate null-origin warnings `src/utils/cors.ts`, `src/utils/internal/cors.ts`, `test/unit/cors.test.ts`	The CORS documentation warns against credentialed `"null"` origins; `resolveCorsOptions` emits a deduplicated warning for direct and array-based `"null"` origins, with unit tests covering both cases.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

h3js/h3#1323: Updates the same CORS credential-warning logic and related unit tests.

Suggested reviewers: pi0

Poem

I’m a rabbit guarding the CORS gate,
Warning when null origins congregate.
Credentials hop, but the warning rings,
Tests check both arrays and single strings.
Documentation keeps the burrow bright!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: warning when CORS credentials are used with a null origin.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/cors-null-origin-warn

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/unit/cors.test.ts`:
- Around line 97-109: Reset the module-level warnOnce state between the
direct-origin and array-origin tests so each case independently observes its
expected warning. Update the test setup around resolveCorsOptions and warnSpy,
or isolate the module per case, while preserving the global warn-once behavior
in production.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 18a6666f-9cd9-4293-8bc5-9ae6d4293cf7

📥 Commits

Reviewing files that changed from the base of the PR and between 8ed97e4 and e66906d.

📒 Files selected for processing (3)

src/utils/cors.ts
src/utils/internal/cors.ts
test/unit/cors.test.ts

coderabbitai · 2026-07-14T12:41:04Z

+      it('warns when credentials is used with an origin array containing `"null"`', () => {
+        const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+
+        // `"null"` in an array is a live hazard too: the array path does an
+        // exact string comparison, so an `Origin: null` request matches and
+        // is reflected with credentials.
+        resolveCorsOptions({
+          credentials: true,
+          origin: ["https://example.com", "null"],
+          exposeHeaders: ["X-Custom"],
+        });
+        expect(warnSpy).toHaveBeenCalledOnce();
+        expect(warnSpy.mock.calls[0][0]).toContain("null");


🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reset warning state between these test cases.

warnOnce keeps a module-level set keyed by the warning text, so the preceding direct-origin test already consumes this exact warning. When this array-origin test runs second, warnSpy receives zero calls and toHaveBeenCalledOnce() fails. Isolate the module state between cases or restructure the tests without changing the intended global warn-once behavior.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/unit/cors.test.ts` around lines 97 - 109, Reset the module-level warnOnce state between the direct-origin and array-origin tests so each case independently observes its expected warning. Update the test setup around resolveCorsOptions and warnSpy, or isolate the module per case, while preserving the global warn-once behavior in production.

pi0x requested a review from pi0 as a code owner July 14, 2026 12:34

pi0x commented Jul 14, 2026

View reviewed changes

coderabbitai Bot reviewed Jul 14, 2026

View reviewed changes

pi0 approved these changes Jul 14, 2026

View reviewed changes

pi0 merged commit 029fde0 into main Jul 14, 2026
8 of 9 checks passed

pi0 deleted the fix/cors-null-origin-warn branch July 14, 2026 12:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(cors): warn on credentials with `null` origin#1464

fix(cors): warn on credentials with `null` origin#1464
pi0 merged 2 commits into
mainfrom
fix/cors-null-origin-warn

pi0x commented Jul 14, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

codecov Bot commented Jul 14, 2026

Uh oh!

pi0x left a comment

Uh oh!

pi0x commented Jul 14, 2026

Uh oh!

coderabbitai Bot commented Jul 14, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Jul 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

pi0x commented Jul 14, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

codecov Bot commented Jul 14, 2026

Codecov Report

Uh oh!

pi0x left a comment

Choose a reason for hiding this comment

Uh oh!

pi0x commented Jul 14, 2026

Uh oh!

coderabbitai Bot commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pi0x commented Jul 14, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 14, 2026 •

edited

Loading