diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fe49dcb30..8c59d6bb40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ _Living section — bullets land here as PRs merge into `release/v3.8.47` (paral - **Chaos Mode**: multi-model parallel/collaborative task execution — dispatches a task to every active provider connection at once (parallel) or chains outputs sequentially so each model builds on the previous one's answer (collaborative), configurable via Dashboard → Chaos Mode (`GET`/`PUT`/`DELETE /api/chaos/config`) and gated per-API-key via a new `chaosModeEnabled` permission (opt-in — disabled by default globally and per key). `POST /api/chaos/run` (dashboard session) and `POST /api/skills/collect/chaos` (external Bearer-token) delegate to a shared `executeChaosRun()` engine (`src/lib/chaos/chaosExecutor.ts`) that dispatches in-process via the established synthetic-Request/route-handler pattern (no network hop, no hardcoded port), with a concurrency cap (max 10 parallel), configurable `max_tokens` (256–128k), a clear error when `stream` is requested, and collaborative-chain info (provider order + input size). Fixes external Bearer-auth bypass and stale config-cache leakage. Regression guard: `tests/unit/chaos-config.test.ts`, `tests/unit/chaos-executor.test.ts`, `tests/unit/chaos-api-routes.test.ts`. ([#6728](https://github.com/diegosouzapw/OmniRoute/pull/6728) — thanks @Moseyuh333) - **feat(cli):** 2 new CLI tool integrations on Dashboard → CLI Tools — **omp** (Oh My Pi) and **letta** — each with binary detection, config apply/reset, and a settings card following the existing tool-card pattern. Both settings routes shell out to `which omp`/`which letta` to detect the local install, so they're loopback-gated (`LOCAL_ONLY_API_PREFIXES`, Hard Rules #15/#17) in addition to the shared `requireCliToolsAuth()` management-auth guard every cli-tools route requires, and route errors through `sanitizeErrorMessage()`; `src/lib/db/omp.ts` isolates the `omp` CLI's own local SQLite reads behind parameterized queries. (Note: the original PR also proposed **pi**, **codewhale**, and **jcode** integrations — those three had already shipped via a separate PR by the time this one was reconciled, so only omp+letta landed here.) Regression guard: `tests/unit/db/omp.test.ts`, `tests/unit/cli-tools-auth-hardening.test.ts`, `tests/integration/cli-settings-omp.test.ts`, `tests/integration/cli-settings-letta.test.ts`. ([#6318](https://github.com/diegosouzapw/OmniRoute/pull/6318) — thanks @hamsa0x7) - **fix(providers):** register OpenRouter as a rerank provider so `openrouter/cohere/rerank-*` models resolve instead of erroring `Invalid rerank model` (#6574 — thanks @rafpigna) +- **fix(api):** `HEAD` requests no longer hang until client timeout on any route — valid, unknown, authed, or unauthed ([#6400](https://github.com/diegosouzapw/OmniRoute/issues/6400)), broader follow-up to the route-specific #6517 (`/v1/models`). Root cause: Next.js 16's App Router _route-handler_ pipeline (`next/dist/server/send-response.js`) correctly skips piping a `Response` body for `HEAD`, but its _page_-rendering pipeline (`next/dist/server/pipe-readable.js` → `pipeToNodeResponse`, used for every app-router page/layout render — including the `not-found` boundary any unmatched path falls through to) has no such check and always streams the full rendered body regardless of method; combined with Node's default keep-alive framing this left some clients unsure whether the (implicitly bodyless) `HEAD` response had actually finished. A new `scripts/dev/head-response-guard.cjs`, wired into both the dev/start custom server (`scripts/dev/run-next.mjs`) and the packaged standalone server (`scripts/dev/standalone-server-ws.mjs`) at the same tier as the existing `http-method-guard.cjs`/`peer-stamp.mjs` wrappers, discards any body bytes written for a `HEAD` request and forces `Connection: close` once `.end()` is called — independent of route existence or auth state, satisfying RFC 9110 §9.3.2. Regression guard: `tests/unit/head-request-closes-6400.test.ts`. ### 🐛 Bug Fixes diff --git a/scripts/dev/head-response-guard.cjs b/scripts/dev/head-response-guard.cjs new file mode 100644 index 0000000000..7ecb460439 --- /dev/null +++ b/scripts/dev/head-response-guard.cjs @@ -0,0 +1,104 @@ +"use strict"; + +/** + * HEAD response guard (#6400). + * + * RFC 9110 §9.3.2 requires a HEAD response to carry the same headers/status a + * GET would, with ZERO body, and the connection should not leave the client + * guessing about when the (bodyless) response is actually finished. + * + * Next.js 16 handles this correctly for App Router *route handlers* + * (`route.ts` exporting `GET`) — `next/dist/server/send-response.js` explicitly + * skips piping the `Response.body` when `req.method === 'HEAD'`. But Next's + * *page*-rendering pipeline (`next/dist/server/pipe-readable.js` -> + * `pipeToNodeResponse`, used for every app-router page/layout render — the + * root page, the `not-found` boundary that unmatched paths fall through to, + * dashboard pages, etc.) has NO such check: it always pipes the fully + * rendered body to the HTTP response regardless of method. Combined with + * Node's default keep-alive framing, a HEAD request to any page-rendered path + * ends up with the socket only settling once that render finishes — on a + * client that doesn't special-case a HEAD response's implicit zero-length + * body (observed on Windows/curl in #6400), this reads as "headers arrive, + * then it hangs" instead of the RFC-mandated "closes immediately". + * + * Fix: for every inbound HEAD request, before Next ever sees it, wrap the + * Node `ServerResponse` so: + * - Any body bytes written by Next (route handler OR page render) are + * discarded — status code and headers Next computed (auth 401s, 404s, + * 200s, etc.) are preserved untouched. + * - The connection is force-closed right after headers flush + * (`Connection: close`), removing any keep-alive ambiguity a client could + * have about whether more bytes are coming. + * + * This applies globally (valid routes, unmatched/404 paths, authed and + * unauthed) because it operates at the Node HTTP transport layer shared by + * every request — the same tier as the existing `http-method-guard.cjs` / + * `peer-stamp.mjs` wrappers — never inside Next's per-route code. + * See: https://github.com/diegosouzapw/OmniRoute/issues/6400 + */ + +function isHeadRequest(req) { + return typeof req?.method === "string" && req.method.toUpperCase() === "HEAD"; +} + +/** + * Mutates `res` in place so any body write is discarded and the response + * ends (closing the connection) as soon as `.end()` is called, regardless of + * what body argument was passed to it. + * + * @param {import("node:http").ServerResponse} res + */ +function suppressBodyAndForceClose(res) { + try { + // Never leave the client guessing whether the (bodyless) response has + // more bytes coming — closing the socket is the unambiguous signal. + res.setHeader("Connection", "close"); + } catch { + // Headers may already be flushed in rare re-entrant cases — the write/end + // overrides below still guarantee an empty, prompt HEAD response. + } + + const originalEnd = res.end.bind(res); + let ended = false; + + res.write = function headSuppressedWrite(_chunk, encodingOrCb, cb) { + // Discard the body but keep the writable-stream contract: report the + // write as flushed (no backpressure) so callers like Next's + // `pipeToNodeResponse` never block waiting on a `drain` that would + // otherwise never fire, and invoke whichever callback form was passed. + if (typeof encodingOrCb === "function") encodingOrCb(); + else if (typeof cb === "function") cb(); + return true; + }; + + res.end = function headSuppressedEnd(chunk, encoding, cb) { + if (ended) return res; + ended = true; + if (typeof chunk === "function") return originalEnd(chunk); + if (typeof encoding === "function") return originalEnd(encoding); + if (typeof cb === "function") return originalEnd(cb); + return originalEnd(); + }; +} + +/** + * Wrap a Node request listener so every inbound HEAD request gets the + * body-suppression + forced-close treatment before the wrapped listener + * (eventually Next.js) runs. + * + * @param {(req: import("node:http").IncomingMessage, res: import("node:http").ServerResponse) => unknown} listener + */ +function wrapRequestListenerWithHeadResponseGuard(listener) { + return function headResponseGuardRequestHandler(req, res) { + if (isHeadRequest(req)) { + suppressBodyAndForceClose(res); + } + return listener.call(this, req, res); + }; +} + +module.exports = { + isHeadRequest, + suppressBodyAndForceClose, + wrapRequestListenerWithHeadResponseGuard, +}; diff --git a/scripts/dev/run-next.mjs b/scripts/dev/run-next.mjs index 4a402650a8..0ff02837f0 100644 --- a/scripts/dev/run-next.mjs +++ b/scripts/dev/run-next.mjs @@ -10,6 +10,7 @@ import { createOmnirouteWsBridge } from "./v1-ws-bridge.mjs"; import { createResponsesWsProxy } from "./responses-ws-proxy.mjs"; import { ensurePeerStampToken, stampPeerIp } from "./peer-stamp.mjs"; import methodGuard from "./http-method-guard.cjs"; +import headResponseGuard from "./head-response-guard.cjs"; import { ensureNativeSqlite } from "./ensure-native-sqlite.mjs"; import { isTurbopackCacheCorruption, @@ -18,6 +19,7 @@ import { import { randomUUID } from "node:crypto"; const { maybeHandleDisallowedMethod } = methodGuard; +const { wrapRequestListenerWithHeadResponseGuard } = headResponseGuard; // Pre-read DATA_DIR from local .env before bootstrap resolves paths if (!process.env.DATA_DIR) { @@ -143,13 +145,15 @@ async function start() { baseUrl: `http://127.0.0.1:${dashboardPort}`, }); - const server = http.createServer((req, res) => { - if (maybeHandleDisallowedMethod(req, res)) return; - // Stamp the real TCP peer IP before Next sees the request, so the authz - // middleware can decide LOCAL_ONLY locality without trusting the Host header. - stampPeerIp(req); - return requestHandler(req, res); - }); + const server = http.createServer( + wrapRequestListenerWithHeadResponseGuard((req, res) => { + if (maybeHandleDisallowedMethod(req, res)) return; + // Stamp the real TCP peer IP before Next sees the request, so the authz + // middleware can decide LOCAL_ONLY locality without trusting the Host header. + stampPeerIp(req); + return requestHandler(req, res); + }) + ); server.on("upgrade", async (req, socket, head) => { try { const responsesWsHandled = await responsesWsProxy.handleUpgrade(req, socket, head); diff --git a/scripts/dev/standalone-server-ws.mjs b/scripts/dev/standalone-server-ws.mjs index 890caee40f..eb59d9d50a 100644 --- a/scripts/dev/standalone-server-ws.mjs +++ b/scripts/dev/standalone-server-ws.mjs @@ -5,11 +5,13 @@ import { createResponsesWsProxy } from "./responses-ws-proxy.mjs"; import { ensurePeerStampToken, wrapRequestListenerWithPeerStamp } from "./peer-stamp.mjs"; import { maybeHandleWebdav } from "./webdav-handler.mjs"; import methodGuard from "./http-method-guard.cjs"; +import headResponseGuard from "./head-response-guard.cjs"; import { resolveTlsOptions, createServerListener } from "./tls-options.mjs"; const originalCreateServer = http.createServer.bind(http); const proxiesByPort = new Map(); const { wrapRequestListenerWithMethodGuard } = methodGuard; +const { wrapRequestListenerWithHeadResponseGuard } = headResponseGuard; // Opt-in native HTTPS (#5242). Resolved once at boot: when both OMNIROUTE_TLS_CERT // and OMNIROUTE_TLS_KEY point at readable files we terminate TLS on the same @@ -114,8 +116,12 @@ http.createServer = function createServerWithResponsesWs(...args) { const lastFnIdx = args.map((a) => typeof a === "function").lastIndexOf(true); if (lastFnIdx >= 0) { // Method guard runs before Next because Next 16 rejects TRACE while constructing requests. - args[lastFnIdx] = wrapRequestListenerWithMethodGuard( - wrapRequestListenerWithWebdav(wrapRequestListenerWithPeerStamp(args[lastFnIdx])) + // Head-response guard wraps outermost so it sees (and can force-close) every + // HEAD request regardless of which inner layer ends up handling it (#6400). + args[lastFnIdx] = wrapRequestListenerWithHeadResponseGuard( + wrapRequestListenerWithMethodGuard( + wrapRequestListenerWithWebdav(wrapRequestListenerWithPeerStamp(args[lastFnIdx])) + ) ); } @@ -134,8 +140,10 @@ http.createServer = function createServerWithResponsesWs(...args) { if (eventName === "request" && typeof listener === "function") { return originalOn( eventName, - wrapRequestListenerWithMethodGuard( - wrapRequestListenerWithWebdav(wrapRequestListenerWithPeerStamp(listener)) + wrapRequestListenerWithHeadResponseGuard( + wrapRequestListenerWithMethodGuard( + wrapRequestListenerWithWebdav(wrapRequestListenerWithPeerStamp(listener)) + ) ) ); } @@ -149,8 +157,10 @@ http.createServer = function createServerWithResponsesWs(...args) { if (eventName === "request" && typeof listener === "function") { return originalAddListener( eventName, - wrapRequestListenerWithMethodGuard( - wrapRequestListenerWithWebdav(wrapRequestListenerWithPeerStamp(listener)) + wrapRequestListenerWithHeadResponseGuard( + wrapRequestListenerWithMethodGuard( + wrapRequestListenerWithWebdav(wrapRequestListenerWithPeerStamp(listener)) + ) ) ); } diff --git a/tests/unit/head-request-closes-6400.test.ts b/tests/unit/head-request-closes-6400.test.ts new file mode 100644 index 0000000000..f10180911c --- /dev/null +++ b/tests/unit/head-request-closes-6400.test.ts @@ -0,0 +1,237 @@ +import { describe, it, after } from "node:test"; +import assert from "node:assert/strict"; +import http from "node:http"; +import { EventEmitter } from "node:events"; +import { createRequire } from "node:module"; +import type { AddressInfo } from "node:net"; + +const require = createRequire(import.meta.url); +const headResponseGuard = require("../../scripts/dev/head-response-guard.cjs") as { + wrapRequestListenerWithHeadResponseGuard: ( + listener: (req: http.IncomingMessage, res: http.ServerResponse) => unknown + ) => (req: http.IncomingMessage, res: http.ServerResponse) => unknown; + suppressBodyAndForceClose: (res: http.ServerResponse) => void; +}; + +const { wrapRequestListenerWithHeadResponseGuard, suppressBodyAndForceClose } = headResponseGuard; + +/** + * Regression test for issue #6400 — "HEAD requests hang ~6s — response never + * closes after headers", reported across EVERY route (valid, unknown, authed, + * unauthed). + * + * Root cause: Next.js 16's App Router route-handler pipeline + * (`next/dist/server/send-response.js`) correctly skips piping a `Response` + * body for HEAD requests, but its *page*-rendering pipeline + * (`next/dist/server/pipe-readable.js` -> `pipeToNodeResponse`, used for every + * app-router page/layout render — including the `not-found` boundary that + * unmatched paths fall through to) has NO such check: it always streams the + * full rendered body to the HTTP response regardless of method. Combined with + * Node's default keep-alive framing this leaves clients (observed on + * Windows/curl) unsure whether the — implicitly bodyless — HEAD response has + * actually finished. + * + * Fix: `scripts/dev/head-response-guard.cjs` wraps the Node request listener + * (wired into both the dev/start custom server `scripts/dev/run-next.mjs` and + * the packaged standalone server `scripts/dev/standalone-server-ws.mjs`) so + * that, for every inbound HEAD request, any body bytes the inner handler + * tries to write are discarded (never blocking on backpressure) and the + * connection is force-closed (`Connection: close`) as soon as `.end()` is + * called — independent of route existence or auth state. + */ +describe("issue #6400 — HEAD response guard (unit)", () => { + function makeMockResponse() { + const emitter = new EventEmitter() as EventEmitter & { + headers: Record; + ended: boolean; + writeCalls: unknown[][]; + endCalls: unknown[][]; + write: (...args: unknown[]) => boolean; + end: (...args: unknown[]) => unknown; + setHeader: (name: string, value: string) => void; + }; + emitter.headers = {}; + emitter.ended = false; + emitter.writeCalls = []; + emitter.endCalls = []; + emitter.setHeader = (name: string, value: string) => { + emitter.headers[name.toLowerCase()] = value; + }; + emitter.write = (...args: unknown[]) => { + emitter.writeCalls.push(args); + return true; + }; + emitter.end = (...args: unknown[]) => { + emitter.endCalls.push(args); + emitter.ended = true; + return emitter; + }; + return emitter; + } + + it("sets Connection: close on the response", () => { + const res = makeMockResponse(); + suppressBodyAndForceClose(res as unknown as http.ServerResponse); + assert.equal(res.headers.connection, "close"); + }); + + it("discards any body written via res.write() but still reports success (no backpressure stall)", () => { + const res = makeMockResponse(); + const originalWrite = res.write; + suppressBodyAndForceClose(res as unknown as http.ServerResponse); + + const ok = res.write("this body must never reach the socket"); + assert.equal(ok, true, "write must report success so callers never block on a drain event"); + assert.equal( + res.writeCalls.length, + 0, + "the original write() must never be called — the body must be fully discarded" + ); + assert.notEqual(res.write, originalWrite); + }); + + it("res.end() forwards to the original end with NO body argument, and is idempotent", () => { + const res = makeMockResponse(); + suppressBodyAndForceClose(res as unknown as http.ServerResponse); + + res.end("this must be dropped"); + res.end("second call must be a no-op"); + + assert.equal(res.endCalls.length, 1, "end() must only forward to the original once"); + assert.deepEqual( + res.endCalls[0], + [], + "the discarded body must never be forwarded to the real end()" + ); + assert.equal(res.ended, true); + }); + + it("wrapRequestListenerWithHeadResponseGuard only guards HEAD requests, GET/POST pass through untouched", () => { + let receivedReq: { method?: string } | null = null; + let receivedRes: unknown = null; + const listener = (req: { method?: string }, res: unknown) => { + receivedReq = req; + receivedRes = res; + }; + const guarded = wrapRequestListenerWithHeadResponseGuard( + listener as unknown as (req: http.IncomingMessage, res: http.ServerResponse) => unknown + ); + + const getRes = makeMockResponse(); + guarded({ method: "GET" } as unknown as http.IncomingMessage, getRes as unknown as http.ServerResponse); + assert.equal(getRes.headers.connection, undefined, "GET must not be forced to close"); + + const headRes = makeMockResponse(); + guarded( + { method: "HEAD" } as unknown as http.IncomingMessage, + headRes as unknown as http.ServerResponse + ); + assert.equal(headRes.headers.connection, "close", "HEAD must be force-closed"); + + // Sanity: the inner listener was actually invoked in both cases (guard + // must not swallow the request — routing/auth/status-code logic still + // runs, only the body write path is intercepted). + assert.ok(receivedReq); + assert.ok(receivedRes); + }); +}); + +/** + * End-to-end confirmation over a real TCP socket: a HEAD request through the + * guarded listener gets an empty body and a closed connection even when the + * underlying handler writes a large body synchronously (the shape that, on + * an un-guarded pipe, streams the full page/RSC payload for HEAD exactly as + * Next's `pipeToNodeResponse` does today). + */ +describe("issue #6400 — HEAD response guard (integration, real socket)", () => { + const servers: http.Server[] = []; + + after(() => { + for (const server of servers) server.close(); + }); + + function startGuardedServer( + handler: (req: http.IncomingMessage, res: http.ServerResponse) => void + ): Promise<{ port: number }> { + const server = http.createServer(wrapRequestListenerWithHeadResponseGuard(handler)); + servers.push(server); + return new Promise((resolve) => { + server.listen(0, "127.0.0.1", () => resolve({ port: (server.address() as AddressInfo).port })); + }); + } + + function rawRequest( + port: number, + method: string + ): Promise<{ statusCode: number; headers: Record; body: string }> { + return new Promise((resolvePromise, reject) => { + const req = http.request( + { host: "127.0.0.1", port, method, path: "/", headers: { Connection: "keep-alive" } }, + (res) => { + const chunks: Buffer[] = []; + res.on("data", (chunk) => chunks.push(chunk)); + res.on("end", () => { + resolvePromise({ + statusCode: res.statusCode ?? 0, + headers: Object.fromEntries( + Object.entries(res.headers).map(([k, v]) => [k, String(v)]) + ), + body: Buffer.concat(chunks).toString("utf8"), + }); + }); + } + ); + req.on("error", reject); + req.end(); + }); + } + + it("HEAD to a handler that writes a large body synchronously still returns an empty body + Connection: close", async () => { + const largeBody = "x".repeat(1_000_000); + const { port } = await startGuardedServer((req, res) => { + res.writeHead(200, { "Content-Type": "text/html" }); + res.write(largeBody); + res.end(); + }); + + const result = await rawRequest(port, "HEAD"); + + assert.equal(result.statusCode, 200); + assert.equal(result.body, "", "HEAD body must be empty per RFC 9110 §9.3.2"); + assert.equal( + result.headers.connection, + "close", + "HEAD response must force Connection: close so the client never has to guess" + ); + }); + + it("GET through the SAME guarded listener is unaffected (still streams the full body)", async () => { + const { port } = await startGuardedServer((req, res) => { + res.writeHead(200, { "Content-Type": "text/plain" }); + res.end("hello world"); + }); + + const result = await rawRequest(port, "GET"); + + assert.equal(result.statusCode, 200); + assert.equal(result.body, "hello world"); + assert.notEqual( + result.headers.connection, + "close", + "GET requests must not be forced to close — only HEAD is guarded" + ); + }); + + it("HEAD to a 404/unknown-route-shaped handler also closes immediately with an empty body", async () => { + const { port } = await startGuardedServer((req, res) => { + res.writeHead(404, { "Content-Type": "text/html" }); + res.end("not found"); + }); + + const result = await rawRequest(port, "HEAD"); + + assert.equal(result.statusCode, 404); + assert.equal(result.body, ""); + assert.equal(result.headers.connection, "close"); + }); +});