diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ed863721f..3dc2a3b77f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,8 @@ ### 🔧 Bug Fixes +- **api (proxy header hygiene):** upstream `x-middleware-*` control headers (emitted by providers hosted behind Next.js, e.g. synthetic.new) are now stripped from proxied responses instead of forwarded verbatim — forwarding `x-middleware-rewrite` made Next 16 throw `NextResponse.rewrite() was used in a app route handler` and return 500 despite a successful upstream call. Applies to both streaming and JSON paths. Regression guard: `tests/unit/middleware-header-strip-5849.test.ts`. ([#5849](https://github.com/diegosouzapw/OmniRoute/issues/5849)) + - **docs (pnpm global install):** replaced the unsupported `pnpm approve-builds -g` step with the install-time `pnpm add -g omniroute@latest --allow-build=better-sqlite3` flag across README + Setup Guide (and i18n mirrors), fixing native-build approval for pnpm v11 global installs. ([#5554](https://github.com/diegosouzapw/OmniRoute/issues/5554)) - **dashboard (token badge):** the red "Token Expired" connection badge no longer flashes for OAuth refresh-capable providers (Antigravity/Gemini) whose access token merely lapsed but is auto-refreshed — it now shows only when the connection is terminally expired (`testStatus === "expired"`). Continuation of #5326. Regression guard: `tests/unit/ui/connection-row-token-badge-5836.test.tsx`. ([#5836](https://github.com/diegosouzapw/OmniRoute/issues/5836)) diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index 4a1f6aef39..4535402de7 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -37,6 +37,7 @@ import { import { buildStreamingResponseHeaders, materializeDeduplicatedExecutionResult, + stripNextMiddlewareControlHeaders, stripStaleForwardingHeaders, } from "./chatCore/responseHeaders.ts"; import { @@ -2618,6 +2619,7 @@ export async function handleChatCore({ const headersObj = normalizeHeaders(rawResult.response.headers); const responseHeaders = new Headers(headersObj); stripStaleForwardingHeaders(responseHeaders); + stripNextMiddlewareControlHeaders(responseHeaders); const contentType = (responseHeaders.get("content-type") || "").toLowerCase(); const payload = await readNonStreamingResponseBody( rawResult.response, diff --git a/open-sse/handlers/chatCore/responseHeaders.ts b/open-sse/handlers/chatCore/responseHeaders.ts index 3d490babc6..3c09538716 100644 --- a/open-sse/handlers/chatCore/responseHeaders.ts +++ b/open-sse/handlers/chatCore/responseHeaders.ts @@ -11,13 +11,61 @@ const STREAMING_RESPONSE_HEADER_DENYLIST = new Set([ "transfer-encoding", ]); +/** + * Prefix of Next.js internal middleware control headers. + * + * When an upstream provider is itself hosted behind a Next.js middleware + * (e.g. synthetic.new), a perfectly successful `200 OK` response can still + * carry Next's own control headers such as `x-middleware-rewrite`, + * `x-middleware-next`, `x-middleware-override-headers`, + * `x-middleware-set-cookie`, and the `x-middleware-request-*` family. + * + * OmniRoute forwards upstream response headers verbatim. If we re-emit those + * headers from an App Router route handler, Next 16's `app-route` runtime + * interprets `x-middleware-rewrite` as a `NextResponse.rewrite()` call and + * throws `NextResponse.rewrite() was used in a app route handler` — turning a + * successful upstream call into a 500. This is provider-agnostic proxy + * hygiene: any upstream behind Next middleware can leak these headers. + * + * See issue #5849. + */ +const NEXTJS_MIDDLEWARE_HEADER_PREFIX = "x-middleware-"; + +/** + * True when `headerName` is a Next.js internal middleware control header that + * must never be forwarded from a proxied upstream response. + */ +export function isNextMiddlewareControlHeader(headerName: string): boolean { + return headerName.toLowerCase().startsWith(NEXTJS_MIDDLEWARE_HEADER_PREFIX); +} + +/** + * Strip the whole `x-middleware-*` family (see {@link isNextMiddlewareControlHeader}) + * from a `Headers` instance. Used on the non-streaming JSON path alongside + * {@link stripStaleForwardingHeaders}. + */ +export function stripNextMiddlewareControlHeaders(headers: Headers): void { + const toDelete: string[] = []; + headers.forEach((_value, key) => { + if (isNextMiddlewareControlHeader(key)) { + toDelete.push(key); + } + }); + for (const key of toDelete) { + headers.delete(key); + } +} + export function buildStreamingResponseHeaders( providerHeaders: Headers, meta: Parameters[0] ): Record { const forwardedHeaders: [string, string][] = []; providerHeaders.forEach((value, key) => { - if (!STREAMING_RESPONSE_HEADER_DENYLIST.has(key.toLowerCase())) { + if ( + !STREAMING_RESPONSE_HEADER_DENYLIST.has(key.toLowerCase()) && + !isNextMiddlewareControlHeader(key) + ) { forwardedHeaders.push([key, value]); } }); diff --git a/tests/unit/middleware-header-strip-5849.test.ts b/tests/unit/middleware-header-strip-5849.test.ts new file mode 100644 index 0000000000..3e087c77bb --- /dev/null +++ b/tests/unit/middleware-header-strip-5849.test.ts @@ -0,0 +1,67 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { + buildStreamingResponseHeaders, + isNextMiddlewareControlHeader, + stripNextMiddlewareControlHeaders, +} from "@omniroute/open-sse/handlers/chatCore/responseHeaders.ts"; + +// Regression guard for issue #5849: +// Providers hosted behind a Next.js middleware (e.g. synthetic.new) leak Next's +// internal `x-middleware-*` control headers on a successful 200 response. +// Forwarding `x-middleware-rewrite` verbatim from an App Router route handler +// makes Next 16 throw `NextResponse.rewrite() was used in a app route handler` +// and return 500. Both proxy paths (streaming + JSON) must strip the family. + +const MIDDLEWARE_HEADERS: [string, string][] = [ + ["x-middleware-rewrite", "/internal/rewrite"], + ["x-middleware-next", "1"], + ["x-middleware-override-headers", "x-foo"], + ["x-middleware-set-cookie", "a=b"], + ["x-middleware-request-foo", "bar"], +]; + +test("isNextMiddlewareControlHeader matches the whole x-middleware-* family (case-insensitive)", () => { + for (const [name] of MIDDLEWARE_HEADERS) { + assert.equal(isNextMiddlewareControlHeader(name), true, name); + assert.equal(isNextMiddlewareControlHeader(name.toUpperCase()), true, name); + } + assert.equal(isNextMiddlewareControlHeader("x-request-id"), false); + assert.equal(isNextMiddlewareControlHeader("content-type"), false); +}); + +test("streaming path: buildStreamingResponseHeaders strips x-middleware-* and preserves normal headers", () => { + const upstream = new Headers(); + for (const [k, v] of MIDDLEWARE_HEADERS) upstream.append(k, v); + upstream.append("x-request-id", "req-123"); + + const out = buildStreamingResponseHeaders(upstream, {}); + + const lowerKeys = Object.keys(out).map((k) => k.toLowerCase()); + for (const [name] of MIDDLEWARE_HEADERS) { + assert.ok( + !lowerKeys.includes(name.toLowerCase()), + `expected ${name} to be stripped, got: ${lowerKeys.join(", ")}` + ); + } + // Normal upstream header preserved. + const requestIdKey = Object.keys(out).find((k) => k.toLowerCase() === "x-request-id"); + assert.ok(requestIdKey, "x-request-id must be preserved"); + assert.equal(out[requestIdKey as string], "req-123"); +}); + +test("non-streaming JSON path: stripNextMiddlewareControlHeaders removes the family, keeps the rest", () => { + const headers = new Headers(); + for (const [k, v] of MIDDLEWARE_HEADERS) headers.append(k, v); + headers.append("x-request-id", "req-456"); + headers.append("content-type", "application/json"); + + stripNextMiddlewareControlHeaders(headers); + + for (const [name] of MIDDLEWARE_HEADERS) { + assert.equal(headers.get(name), null, `${name} must be stripped`); + } + assert.equal(headers.get("x-request-id"), "req-456"); + assert.equal(headers.get("content-type"), "application/json"); +});