-
-
Notifications
You must be signed in to change notification settings - Fork 10.1k
fix(security): scope OAuth callback postMessage to trusted-origin allowlist #4372
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
diegosouzapw
merged 1 commit into
release/v3.8.31
from
feat/port-pr-998-callback-postmessage
Jun 20, 2026
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
111 changes: 111 additions & 0 deletions
111
tests/unit/ui/oauth-callback-postmessage-scope.test.tsx
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| // @vitest-environment jsdom | ||
| import React from "react"; | ||
| import { act } from "react"; | ||
| import { createRoot, type Root } from "react-dom/client"; | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; | ||
|
|
||
| // Mock next-intl translations (the page imports useTranslations("auth")). | ||
| vi.mock("next-intl", () => ({ | ||
| useTranslations: () => (key: string) => key, | ||
| })); | ||
|
|
||
| import CallbackPage from "@/app/callback/page"; | ||
|
|
||
| /** | ||
| * Regression guard for ported upstream PR decolua/9router#998 (security): | ||
| * the OAuth callback page must never relay {code, state} to a wildcard | ||
| * postMessage target ("*"), as a hostile opener can read the code/state and | ||
| * complete the OAuth flow as the user. Only the same-origin parent and | ||
| * Codex's fixed loopback helper (127.0.0.1:1455) are trusted targets. | ||
| */ | ||
| describe("OAuth callback page — postMessage target origin scope (#998)", () => { | ||
| let container: HTMLDivElement; | ||
| let root: Root; | ||
| let postMessageSpy: ReturnType<typeof vi.fn>; | ||
| let originalOpener: typeof window.opener; | ||
|
|
||
| beforeEach(() => { | ||
| container = document.createElement("div"); | ||
| document.body.appendChild(container); | ||
| root = createRoot(container); | ||
| postMessageSpy = vi.fn(); | ||
| originalOpener = window.opener; | ||
|
|
||
| // Set the callback URL with OAuth params (triggers the postMessage send). | ||
| window.history.replaceState({}, "", "/callback?code=test_code_abc123&state=test_state_xyz789"); | ||
|
|
||
| // Stub window.opener as a CROSS-ORIGIN opener: same-origin probe must throw | ||
| // (mimics a real cross-origin window.opener), which means the page falls into | ||
| // the fallback path that previously used a wildcard "*" target origin. | ||
| Object.defineProperty(window, "opener", { | ||
| configurable: true, | ||
| writable: true, | ||
| value: { | ||
| postMessage: postMessageSpy, | ||
| get location(): never { | ||
| throw new Error("cross-origin access blocked"); | ||
| }, | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| act(() => root.unmount()); | ||
| container.remove(); | ||
| Object.defineProperty(window, "opener", { | ||
| configurable: true, | ||
| writable: true, | ||
| value: originalOpener, | ||
| }); | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it("never targets the wildcard '*' origin even when opener is cross-origin", async () => { | ||
| await act(async () => { | ||
| root.render(<CallbackPage />); | ||
| }); | ||
| // Give useEffect a microtask to flush. | ||
| await act(async () => { | ||
| await Promise.resolve(); | ||
| }); | ||
|
|
||
| const targetOrigins = postMessageSpy.mock.calls.map((call) => call[1]); | ||
| expect(targetOrigins).not.toContain("*"); | ||
| }); | ||
|
|
||
| it("only targets trusted origins (same-origin + Codex 127.0.0.1:1455)", async () => { | ||
| await act(async () => { | ||
| root.render(<CallbackPage />); | ||
| }); | ||
| await act(async () => { | ||
| await Promise.resolve(); | ||
| }); | ||
|
|
||
| const trusted = new Set([window.location.origin, "http://localhost:1455", "http://127.0.0.1:1455"]); | ||
| const targetOrigins = postMessageSpy.mock.calls.map((call) => call[1]); | ||
| expect(targetOrigins.length).toBeGreaterThan(0); | ||
| for (const origin of targetOrigins) { | ||
| expect(trusted.has(origin)).toBe(true); | ||
| } | ||
| }); | ||
|
|
||
| it("delivers the OAuth code/state payload at least once via postMessage", async () => { | ||
| await act(async () => { | ||
| root.render(<CallbackPage />); | ||
| }); | ||
| await act(async () => { | ||
| await Promise.resolve(); | ||
| }); | ||
|
|
||
| // Sanity: the scoped postMessage path still actually attempts delivery. | ||
| expect(postMessageSpy).toHaveBeenCalled(); | ||
| const firstCall = postMessageSpy.mock.calls[0]; | ||
| expect(firstCall[0]).toMatchObject({ | ||
| type: "oauth_callback", | ||
| data: expect.objectContaining({ | ||
| code: "test_code_abc123", | ||
| state: "test_state_xyz789", | ||
| }), | ||
| }); | ||
| }); | ||
| }); |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To prevent sending duplicate
postMessageevents to the opener (which can trigger duplicate event handlers or race conditions in the parent window), we should deduplicate thetrustedTargetOriginsarray. This is especially important if the application is running on one of the hardcoded loopback origins (e.g., during local development or testing), wherewindow.location.originwould match one of the other entries.