From efa36e33d4e0aabf23a794b701d66a5d09c0701f Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Sat, 20 Jun 2026 06:21:05 -0300 Subject: [PATCH] fix(security): scope OAuth callback postMessage to a trusted-origin allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The OAuth callback at `/callback` previously fell back to `window.opener.postMessage({ code, state, ... }, "*")` whenever the opener was cross-origin. The fallback was intended to support remote-OmniRoute + local-loopback callbacks (where opener and callback live on different origins), but the same code path also delivers the OAuth code/state to any hostile opener that pops the well-known callback URL — letting that attacker complete the OAuth flow as the user. Replace the wildcard fallback with iteration over a fixed allowlist: `window.location.origin` (same-origin parent — the popup-mode dashboard) plus Codex's fixed loopback helper (`http://localhost:1455` and the IPv4 literal `http://127.0.0.1:1455`). The browser drops `postMessage` to any opener whose actual origin is not in `targetOrigin`, so the message reaches only known parents and is silently dropped for any other. The same-origin fallback path is unchanged — methods 2 (`BroadcastChannel`) and 3 (`localStorage` storage event) still cover same-origin openers that COOP severed. The `openerSameOrigin` probe stays in place to drive the auto-close vs manual-copy UI decision (no behavior change for the success path). Adds a regression test (`tests/unit/ui/oauth-callback-postmessage-scope.test.tsx`) that mounts the page with a stubbed cross-origin opener and asserts no `postMessage` call ever uses `"*"` and every call lands on an allowlisted origin. The test failed against the pre-fix code (red), passes after the fix (green) — TDD per CLAUDE.md hard rule #18. Partial port of upstream decolua/9router#998: the upstream PR also re-enabled TLS verification on a DNS-bypass fetch in `open-sse/utils/proxyFetch.js`; that part is N/A here because OmniRoute's `proxyFetch.ts` never disabled TLS verification (no `rejectUnauthorized: false` anywhere in the file). Co-authored-by: aeonframework Inspired-by: https://github.com/decolua/9router/pull/998 --- CHANGELOG.md | 1 + src/app/callback/page.tsx | 39 +++--- .../oauth-callback-postmessage-scope.test.tsx | 111 ++++++++++++++++++ 3 files changed, 132 insertions(+), 19 deletions(-) create mode 100644 tests/unit/ui/oauth-callback-postmessage-scope.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index aaa3a1403ff..a8c3bab0280 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ _In development — bullets added per PR; finalized at release._ ### 🐛 Fixed - **fix(translator): Gemini accepts HTTP/HTTPS image URLs (no longer silently dropped)** — OpenAI-style `image_url` parts whose URL was `http://…` or `https://…` reached `convertOpenAIContentToParts` (the OpenAI→Gemini request helper) and were dropped with only a `console.warn`, because Gemini's `inlineData` requires base64 and the helper is synchronous (it cannot fetch + encode). Gemini's `Part` schema, however, natively accepts `fileData: { fileUri }` for remote URIs — the model fetches the asset itself. The helper now emits a `fileData` part (`mimeType: "image/*"`, inferred upstream on fetch) instead of dropping, so vision requests that pass a URL — not a data: URI — now reach Gemini intact. `data:` URIs still go through `inlineData` unchanged; unsupported schemes (e.g. `ftp:`) are still skipped. (thanks @East-rayyy) +- **fix(security): OAuth callback page no longer relays `code`/`state` to a wildcard `postMessage` target** — the OAuth callback at `/callback` posted `{ code, state, ... }` to `window.opener.postMessage(..., "*")` whenever the opener was cross-origin (a fallback for the legitimate remote-dashboard + local-loopback callback scenario). A hostile page that opened the callback URL in a popup against the well-known redirect URI would therefore receive the OAuth code+state and could complete the OAuth flow as the user. The wildcard fallback is replaced with an iteration over a fixed allowlist of trusted target origins (same-origin + Codex's loopback helper at `localhost:1455` / `127.0.0.1:1455`); the browser silently drops the message for any opener whose origin is not in the list. Methods 2 (`BroadcastChannel`) and 3 (`localStorage`) — already in the page — still cover same-origin parents when the opener was severed by COOP. (thanks @aeonframework) - **fix(compliance): startup cleanup honors the dashboard data-retention setting instead of always trimming to 7 days** — on every restart, `cleanupExpiredLogs()` (run at startup) read retention only from the `CALL_LOG_RETENTION_DAYS` / `APP_LOG_RETENTION_DAYS` env vars, which default to **7 days** when unset, and trimmed `usage_history` (the Usage Analysis data) before the dashboard-based `runAutoCleanup()` — which respects the configured retention — ever ran. So a dashboard "Data Retention" of 90 days was silently overridden and the Usage Analysis page only ever showed the last 7 days after a restart. Retention now follows the precedence **explicit env var → dashboard DB setting → 7-day default**, per table (`usage_history`→`usageHistory`, `call_logs`/`proxy_logs`/`request_detail_logs`→`callLogs`, `mcp_tool_audit`→`mcpAudit`); an operator who sets the env var still wins, and non-DB deployments still fall back to it. ([#4354](https://github.com/diegosouzapw/OmniRoute/issues/4354) — thanks @akbardwi) - **fix(providers): bailian-coding-plan static fallback catalog matches the registry (10 models)** — the provider-model sweep (#4324) added four current Model Studio coding-plan models (`qwen3.7-plus`, `qwen3-coder-plus`, `qwen3-coder-next`, `glm-4.7`) to the `bailian-coding-plan` registry entry but missed the static fallback mirror in `staticModels.ts`, which still listed only the older six. The static catalog (served when live discovery is unavailable) therefore diverged from the registry, and the existing static↔registry parity test went red on the release branch (only surfacing when test-impact analysis happened to select it). The static mirror now carries all ten models in registry order, restoring parity. ([#4324](https://github.com/diegosouzapw/OmniRoute/pull/4324)) - **fix(executors): ArenaLLM accepts LMArena's split Supabase SSR auth cookie** — LMArena migrated to `@supabase/ssr` chunked auth cookies: the single `arena-auth-prod-v1` cookie is now empty and the real session is split across `arena-auth-prod-v1.0`, `arena-auth-prod-v1.1`, … (ascending). A user who pasted the (now-empty) single cookie therefore sent an empty session and upstream rejected it as "invalid cookie". The LMArena executor now reconstructs the single cookie from its chunks — reading `.0`, `.1`, … in ascending numeric order until one is missing and concatenating their raw values (`@supabase/ssr`'s `combineChunks` rule: plain `join("")`, no base64-decode, no JSON-parse, the `base64-` prefix kept verbatim) — while preserving the rest of the pasted jar. A non-empty single cookie is still forwarded unchanged (back-compat). The credential UX now instructs pasting the **full Cookie header** and tracks the `.0`/`.1` storage keys. ([#4271](https://github.com/diegosouzapw/OmniRoute/issues/4271) — thanks @caussao) diff --git a/src/app/callback/page.tsx b/src/app/callback/page.tsx index 4a209b51da7..3f5dd678a1a 100644 --- a/src/app/callback/page.tsx +++ b/src/app/callback/page.tsx @@ -52,29 +52,30 @@ export default function CallbackPage() { // Method 1: postMessage to opener (popup mode). // May be null when Google OAuth's COOP header severs the opener reference. - // For remote OmniRoute + local loopback callbacks, the callback page origin - // is http://127.0.0.1: while the opener is the public OmniRoute origin. - // Use a wildcard fallback only for the opener that initiated this popup; the - // parent validates the OAuth state before accepting the callback. + // + // Only relay {code, state} to a known-trusted target origin. A wildcard "*" + // here would leak the OAuth code/state to a hostile opener — e.g. a page + // that opened this callback URL in a popup to phish the code. The browser + // delivers postMessage only when the opener's origin matches `targetOrigin`, + // so iterating over an allowlist lets the same-origin parent and Codex's + // fixed loopback helper receive it while silently dropping it for any other + // origin. Methods 2 (BroadcastChannel) and 3 (localStorage) cover the + // same-origin fallback when the opener was severed by COOP. + const trustedTargetOrigins = [ + window.location.origin, // Same origin (dashboard popup mode). + "http://localhost:1455", // Codex helper (fixed loopback port). + "http://127.0.0.1:1455", // Same Codex helper, IPv4 literal form. + ]; if (window.opener) { - try { - // Target this origin specifically — popup mode is only used when isTrueLocalhost, - // so the opener is always on the same origin as the callback page. - window.opener.postMessage( - { type: "oauth_callback", data: callbackData }, - window.location.origin - ); - sent = true; - } catch (e) { - console.log("postMessage failed:", e); - } - - if (!openerSameOrigin) { + for (const origin of trustedTargetOrigins) { try { - window.opener.postMessage({ type: "oauth_callback", data: callbackData }, "*"); + window.opener.postMessage( + { type: "oauth_callback", data: callbackData }, + origin + ); sent = true; } catch (e) { - console.log("cross-origin postMessage failed:", e); + console.log("postMessage failed:", e); } } } diff --git a/tests/unit/ui/oauth-callback-postmessage-scope.test.tsx b/tests/unit/ui/oauth-callback-postmessage-scope.test.tsx new file mode 100644 index 00000000000..0fcab8e8576 --- /dev/null +++ b/tests/unit/ui/oauth-callback-postmessage-scope.test.tsx @@ -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; + 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(); + }); + // 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(); + }); + 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(); + }); + 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", + }), + }); + }); +});