fix(ws): codex Responses-over-WebSocket upgrade — clean handshake + bridge-secret auth

Two bugs made `wscat ws://host/v1/responses` fail with
"Transfer-Encoding can't be present with Content-Length":

1. authz/management policy 401'd the proxy's own internal authenticate/prepare
   loopback call to /api/internal/codex-responses-ws (MANAGEMENT-classified, the
   per-process bridge secret wasn't recognized one layer up). Added a tightly-scoped
   carve-out: isValidWsBridgeRequest() honors a timing-safe sha256 match of
   OMNIROUTE_WS_BRIDGE_SECRET (x-omniroute-ws-bridge-secret header) for that exact
   internal path; the route still re-validates the secret. → auth now succeeds → 101.

2. On auth failure the proxy spread the internal fetch's response headers onto the
   raw upgrade socket — a chunked Transfer-Encoding + Next CSP/route-class headers
   collided with writeHttpError's Content-Length framing (and duplicated Content-Type
   via a case-mismatched spread). writeHttpError now strips framing + pipeline/security
   headers (case-insensitive), and the auth-fail callsite no longer forwards them.

Regression test: tests/unit/responses-ws-proxy-headers.test.mjs (exports writeHttpError;
asserts no TE+CL, single Content-Type, no CSP/route-class leak, safe headers forwarded).
This commit is contained in:
diegosouzapw
2026-06-02 06:02:49 -03:00
parent 512e980a9e
commit 8086d2878b
3 changed files with 138 additions and 9 deletions

View File

@@ -150,16 +150,47 @@ export function decodeClientFrames(
};
}
function writeHttpError(socket, status, body, headers = {}) {
const WRITE_ERROR_RESERVED_HEADERS = new Set([
// Framing — must never collide with our Content-Length default.
"transfer-encoding",
"content-length",
"content-type",
"connection",
"keep-alive",
// Next pipeline / security headers are meaningless on a raw JSON error socket
// and must not leak from a forwarded internal-fetch response.
"content-security-policy",
"x-frame-options",
"x-content-type-options",
"referrer-policy",
"permissions-policy",
"strict-transport-security",
"x-omniroute-route-class",
"x-request-id",
"date",
]);
export function writeHttpError(socket, status, body, headers = {}) {
if (!socket.writable || socket.destroyed) return;
const bodyBuffer = Buffer.from(body || "", "utf8");
const statusText = STATUS_CODES[status] || "Error";
// Strip any caller-supplied framing / duplicate-prone headers (case-insensitive)
// so our Content-Length/Connection/Content-Type defaults always win. Forwarding
// an upstream fetch's chunked Transfer-Encoding here would collide with
// Content-Length ("Transfer-Encoding can't be present with Content-Length") and
// break the client's HTTP parser on a raw upgrade socket.
const safeHeaders = {};
for (const [name, value] of Object.entries(headers || {})) {
if (!WRITE_ERROR_RESERVED_HEADERS.has(String(name).toLowerCase())) {
safeHeaders[name] = value;
}
}
const responseHeaders = {
Connection: "close",
"Content-Length": String(bodyBuffer.length),
"Content-Type": "application/json; charset=utf-8",
...headers,
...safeHeaders,
};
const head = [
@@ -637,12 +668,11 @@ export function createResponsesWsProxy({
headers: getAuthHeaders(req.url || pathname, req.headers),
});
if (!auth.ok) {
writeHttpError(
socket,
auth.status,
auth.text || "{}",
Object.fromEntries(auth.headers.entries())
);
// Do NOT forward the internal fetch's response headers onto the raw
// upgrade socket — they carry chunked transfer-encoding + Next security
// headers that collide with writeHttpError's Content-Length framing.
// The sanitized JSON body alone is enough for the client.
writeHttpError(socket, auth.status, auth.text || "{}");
return true;
}

View File

@@ -1,4 +1,4 @@
import { timingSafeEqual } from "node:crypto";
import { createHash, timingSafeEqual } from "node:crypto";
import { isModelSyncInternalRequest } from "../../../shared/services/modelSyncScheduler";
import { isAuthRequired, isDashboardSessionAuthenticated } from "../../../shared/utils/apiAuth";
import { getLegacyCliTokenSync, getMachineTokenSync } from "../../../lib/machineToken";
@@ -69,11 +69,38 @@ function isInternalModelSyncRequest(ctx: PolicyContext): boolean {
return isModelSyncInternalRequest(ctx.request);
}
const WS_BRIDGE_INTERNAL_PATH = "/api/internal/codex-responses-ws";
const WS_BRIDGE_SECRET_HEADER = "x-omniroute-ws-bridge-secret";
// The in-process codex Responses-over-WebSocket proxy authenticates its internal
// authenticate/prepare calls with a per-process, unguessable secret minted by
// server-ws.mjs (OMNIROUTE_WS_BRIDGE_SECRET). Without this carve-out the MANAGEMENT
// classification 401s that loopback call, which then leaks chunked/security headers
// back onto the upgrade socket. The internal route re-validates the secret timing-safe
// (bridgeSecretMatches), so this is the same trust boundary, surfaced one layer up.
function isValidWsBridgeRequest(ctx: PolicyContext): boolean {
if (ctx.classification.normalizedPath !== WS_BRIDGE_INTERNAL_PATH) return false;
const expected = process.env.OMNIROUTE_WS_BRIDGE_SECRET || "";
if (!expected) return false;
const provided = ctx.request.headers?.get?.(WS_BRIDGE_SECRET_HEADER) ?? "";
if (!provided) return false;
const expectedHash = createHash("sha256").update(expected).digest();
const providedHash = createHash("sha256").update(provided).digest();
return timingSafeEqual(expectedHash, providedHash);
}
export const managementPolicy: RoutePolicy = {
routeClass: "MANAGEMENT",
async evaluate(ctx: PolicyContext): Promise<AuthOutcome> {
const path = ctx.classification.normalizedPath;
// Codex Responses-over-WS bridge: honor the per-process bridge secret before
// the loopback/auth gates so the proxy's internal calls aren't 401'd (which
// would corrupt the WS upgrade response). The internal route re-checks it.
if (isValidWsBridgeRequest(ctx)) {
return allow({ kind: "management_key", id: "ws-bridge", label: "codex-ws-bridge-secret" });
}
// Tier 1: local-only gate — block spawn-capable routes from non-loopback.
//
// Carve-out: a small allow-list of LOCAL_ONLY paths (see

View File

@@ -0,0 +1,72 @@
/**
* tests/unit/responses-ws-proxy-headers.test.mjs
*
* Regression for the codex Responses-over-WebSocket upgrade bug:
* writeHttpError used to spread the internal fetch's response headers onto the
* raw upgrade socket. Those headers include a chunked `transfer-encoding` (the
* internal 401 has no Content-Length) plus Next security headers, which collide
* with writeHttpError's own `Content-Length` framing → the client's HTTP parser
* fails with "Transfer-Encoding can't be present with Content-Length".
*
* writeHttpError must now strip framing / duplicate-prone headers (case-insensitive)
* so its Content-Length/Connection/Content-Type defaults always win.
*/
import test from "node:test";
import assert from "node:assert/strict";
const { writeHttpError } = await import("../../scripts/dev/responses-ws-proxy.mjs");
function fakeSocket() {
return {
writable: true,
destroyed: false,
_head: "",
_body: null,
write(chunk) {
this._head += String(chunk);
},
end(chunk) {
if (chunk !== undefined) this._body = chunk;
},
};
}
test("writeHttpError strips chunked transfer-encoding + leaked pipeline headers from the caller", () => {
const sock = fakeSocket();
// Simulate the exact offending input: undici Object.fromEntries of a chunked
// Next 401 (no content-length) with security + pipeline headers.
writeHttpError(sock, 401, JSON.stringify({ error: { message: "ws_auth_required" } }), {
"transfer-encoding": "chunked",
connection: "keep-alive",
"content-type": "application/json",
"content-security-policy": "default-src 'self'",
"x-frame-options": "DENY",
"x-omniroute-route-class": "MANAGEMENT",
"x-request-id": "abc",
});
const head = sock._head;
const lower = head.toLowerCase();
// The single most important invariant: never both framing headers.
assert.ok(lower.includes("content-length:"), "must emit Content-Length");
assert.ok(!lower.includes("transfer-encoding"), "must NOT emit Transfer-Encoding alongside Content-Length");
assert.ok(!lower.includes("keep-alive"), "must not forward the upstream keep-alive Connection");
// Exactly one Content-Type (no duplicate from a case-mismatched spread).
assert.equal((lower.match(/content-type:/g) || []).length, 1, "exactly one Content-Type header");
// Pipeline / security headers must not leak onto the raw upgrade socket.
assert.ok(!lower.includes("content-security-policy"), "must not leak CSP");
assert.ok(!lower.includes("x-omniroute-route-class"), "must not leak route-class");
// Our own framing defaults win.
assert.ok(head.startsWith("HTTP/1.1 401 "), "status line preserved");
assert.ok(lower.includes("connection: close"), "Connection: close default wins");
});
test("writeHttpError still forwards safe non-framing headers (e.g. retry-after)", () => {
const sock = fakeSocket();
writeHttpError(sock, 429, "{}", { "retry-after": "5", "transfer-encoding": "chunked" });
const lower = sock._head.toLowerCase();
assert.ok(lower.includes("retry-after: 5"), "safe header forwarded");
assert.ok(!lower.includes("transfer-encoding"), "framing header still stripped");
});