From 4f2eb482083c2f722e28ad2b587612bc70ca6fe7 Mon Sep 17 00:00:00 2001 From: Gunther Schulz Date: Wed, 29 Jul 2026 12:17:26 +0200 Subject: [PATCH] =?UTF-8?q?fix(server):=20forward=20extension-mutated=20he?= =?UTF-8?q?aders=20=E2=80=94=20they=20never=20reached=20the=20wire?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit preForward builds reqCtx.headers = { ...clientReq.headers } for the extension pipeline to read and mutate, but only reqCtx.body was ever serialized back into the outbound request: forwardRequest still read the ORIGINAL clientReq.headers, so added, changed and deleted header keys were silently discarded. auto-1m-guard's strip mode is the standing in-tree victim — its unit test asserts ctx.headers is mutated correctly, and nothing proved the mutation reached the wire (it did not). preForward now returns the mutated header object, and handleMessages/ handleBootstrap forward a minimal { url, method, headers } wrapper — forwardRequest only reads those three fields, so upstream.mjs's signature is untouched. handlePassthrough runs no extension pipeline and is deliberately unchanged. Returning the object itself (not a copy) keeps deletions visible: plain object semantics carry add, change and delete alike. Wire-level regression test: a real proxy instance through the real pipeline to a local upstream that records what it received — a synthetic extension exercising add/change/delete, plus auto-1m-guard's strip contract end-to-end. Against the unfixed server the suite fails 4 of 5; with the fix 5 of 5 pass. Co-Authored-By: Claude Fable 5 --- proxy/server.mjs | 31 +++- test/proxy-server-header-propagation.test.mjs | 161 ++++++++++++++++++ 2 files changed, 187 insertions(+), 5 deletions(-) create mode 100644 test/proxy-server-header-propagation.test.mjs diff --git a/proxy/server.mjs b/proxy/server.mjs index 0162694f..b6786f48 100644 --- a/proxy/server.mjs +++ b/proxy/server.mjs @@ -66,6 +66,19 @@ function collectBody(req) { // `routeName` is stashed on ctx.meta.route so route-aware extensions // (bootstrap-defense, env-flag-detector) can discriminate without each // route needing its own pipeline hook. +// +// Returns `headers`: the (possibly extension-mutated) outbound header +// object. Extensions read/mutate `reqCtx.headers` — added, changed, AND +// deleted keys — expecting those mutations to reach the real outbound +// request (auto-1m-guard's strip mode is the standing example). Prior to +// this fix only `reqCtx.body` was serialized back into `forwardBody`; +// `reqCtx.headers` was built for extensions to read/mutate but the mutated +// object was discarded — forwardRequest still read the ORIGINAL +// `clientReq.headers`, so header mutations never reached the wire. +// Returning the object itself (not a copy) is what makes deletions visible +// to the caller too: `{ ...x }` followed by `delete copy.k` naturally drops +// `k` from the copy, so no special-casing is needed for add/change/delete — +// plain object semantics carry all three. async function preForward(clientReq, clientRes, _abortController, extSnapshot, routeName, baseMeta = {}) { const rawBody = await collectBody(clientReq); @@ -77,6 +90,7 @@ async function preForward(clientReq, clientRes, _abortController, extSnapshot, r } let forwardBody = rawBody; + let headers = clientReq.headers; // baseMeta lets routes pre-populate audit scalars (e.g. resolved upstream // hostname, request_id) so they're available to onRequest hooks BEFORE the // upstream call — block-mode short-circuits in onRequest, so a post-call @@ -99,9 +113,10 @@ async function preForward(clientReq, clientRes, _abortController, extSnapshot, r if (parsed) { forwardBody = Buffer.from(JSON.stringify(reqCtx.body)); } + headers = reqCtx.headers; } - return { handled: false, parsed, forwardBody, meta }; + return { handled: false, parsed, forwardBody, headers, meta }; } async function handleMessages(clientReq, clientRes) { @@ -124,15 +139,19 @@ async function handleMessages(clientReq, clientRes) { "response headers:", redactHeaders(clientRes.getHeaders())); return; } - const { parsed, forwardBody, meta } = pre; + const { parsed, forwardBody, headers, meta } = pre; const requestedModel = parsed?.model || null; let upstreamRes, responseHeaders, statusCode, upstreamConnectionId; try { + // Forward the (possibly extension-mutated) headers, not clientReq + // directly — forwardRequest only reads .url/.method/.headers off its + // first argument, so a minimal wrapper carrying the mutated headers is + // sufficient and avoids touching upstream.mjs's signature. ({ upstreamRes, responseHeaders, statusCode, upstreamConnectionId } = await forwardRequest( - clientReq, + { url: clientReq.url, method: clientReq.method, headers }, forwardBody, abortController.signal )); @@ -234,13 +253,15 @@ async function handleBootstrap(clientReq, clientRes) { const pre = await preForward(clientReq, clientRes, abortController, extSnapshot, "bootstrap", baseMeta); if (pre.handled) return; - const { forwardBody, meta } = pre; + const { forwardBody, headers, meta } = pre; let upstreamRes, responseHeaders, statusCode, upstreamConnectionId; try { + // See handleMessages' matching comment: forward the (possibly + // extension-mutated) headers rather than clientReq.headers directly. ({ upstreamRes, responseHeaders, statusCode, upstreamConnectionId } = await forwardRequest( - clientReq, + { url: clientReq.url, method: clientReq.method, headers }, forwardBody, abortController.signal, )); diff --git a/test/proxy-server-header-propagation.test.mjs b/test/proxy-server-header-propagation.test.mjs new file mode 100644 index 00000000..4244fca4 --- /dev/null +++ b/test/proxy-server-header-propagation.test.mjs @@ -0,0 +1,161 @@ +// Regression coverage for the header-mutation plumbing gap surfaced by the +// deferred-tool-rewrite unit (see that file's header comment, and the prior +// closing report's gap #2): `preForward` (proxy/server.mjs) built +// `reqCtx.headers = { ...clientReq.headers }` for extensions to read/mutate, +// but only `reqCtx.body` was serialized back into the real outbound request +// — header mutations (added, changed, OR deleted keys) never reached +// `forwardRequest`, which still read the ORIGINAL `clientReq.headers`. +// +// This file exercises the fix at the wire level: a real proxy instance, +// through the real extension pipeline, forwarding to a real (local) upstream +// that records exactly what it received. Two cases: +// 1. A synthetic test extension exercising add/change/delete generically. +// 2. auto-1m-guard's existing strip-mode contract, end-to-end — this is +// the "tighten, don't just re-assert" case: auto-1m-guard.test.mjs +// already asserted ctx.headers is mutated correctly (not vacuous for +// what it tests), but nothing previously proved that mutation reached +// the wire. Before this fix, this exact test would have failed: the +// upstream would have received the UN-stripped header. + +import { describe, it, before, after } from "node:test"; +import assert from "node:assert/strict"; +import http from "node:http"; +import { mkdtemp, rm, writeFile, mkdir } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { startProxy } from "../proxy/server.mjs"; + +function clientRequest(port, body, headers = {}) { + return new Promise((resolve, reject) => { + const data = JSON.stringify(body); + const req = http.request( + { + hostname: "127.0.0.1", + port, + path: "/v1/messages", + method: "POST", + headers: { "content-type": "application/json", ...headers }, + }, + (res) => { + const chunks = []; + res.on("data", (c) => chunks.push(c)); + res.on("end", () => resolve({ status: res.statusCode, body: Buffer.concat(chunks).toString() })); + }, + ); + req.on("error", reject); + req.end(data); + }); +} + +function fakeSseUpstream(onRequest) { + return http.createServer((req, res) => { + const chunks = []; + req.on("data", (c) => chunks.push(c)); + req.on("end", () => { + onRequest(req, Buffer.concat(chunks).toString()); + res.writeHead(200, { "content-type": "text/event-stream" }); + res.write('data: {"type":"message_start","message":{"model":"claude-opus-4-20250514","usage":{}}}\n\n'); + res.write("data: [DONE]\n\n"); + res.end(); + }); + }); +} + +describe("preForward header propagation (generic add/change/delete via a synthetic extension)", () => { + let handle, upstream, upstreamPort, extDir, lastUpstreamHeaders; + + before(async () => { + extDir = await mkdtemp(join(tmpdir(), "header-propagation-ext-")); + await writeFile(join(extDir, "extensions.json"), JSON.stringify({})); + // Synthetic extension: adds a new header, changes an existing one, and + // deletes a third — exercises all three mutation kinds in one pass. + await writeFile( + join(extDir, "header-mutator.mjs"), + `export default { + name: "header-mutator", + order: 100, + onRequest(ctx) { + ctx.headers["x-added-by-extension"] = "added"; + ctx.headers["x-to-change"] = "changed-value"; + delete ctx.headers["x-to-delete"]; + }, + };`, + ); + + upstream = fakeSseUpstream((req) => { + lastUpstreamHeaders = req.headers; + }); + await new Promise((r) => upstream.listen(0, "127.0.0.1", r)); + upstreamPort = upstream.address().port; + + process.env.CACHE_FIX_PROXY_UPSTREAM = `http://127.0.0.1:${upstreamPort}`; + handle = await startProxy({ port: 0, watch: false, extensionsDir: extDir, extensionsConfig: join(extDir, "extensions.json") }); + }); + + after(async () => { + await handle.close(); + await new Promise((r) => upstream.close(r)); + delete process.env.CACHE_FIX_PROXY_UPSTREAM; + await rm(extDir, { recursive: true, force: true }); + }); + + it("an added header reaches the outbound request", async () => { + await clientRequest(handle.port, { model: "test", messages: [] }); + assert.equal(lastUpstreamHeaders["x-added-by-extension"], "added"); + }); + + it("a changed header's new value reaches the outbound request", async () => { + await clientRequest(handle.port, { model: "test", messages: [] }, { "x-to-change": "original-value" }); + assert.equal(lastUpstreamHeaders["x-to-change"], "changed-value"); + }); + + it("a deleted header is ABSENT from the outbound request", async () => { + await clientRequest(handle.port, { model: "test", messages: [] }, { "x-to-delete": "should-not-survive" }); + assert.equal("x-to-delete" in lastUpstreamHeaders, false); + }); + + it("an untouched header still passes through unchanged (no regression to full-passthrough behavior)", async () => { + await clientRequest(handle.port, { model: "test", messages: [] }, { "x-untouched": "still-here" }); + assert.equal(lastUpstreamHeaders["x-untouched"], "still-here"); + }); +}); + +describe("preForward header propagation — auto-1m-guard strip mode reaches the wire (tightened, not vacuous)", () => { + let handle, upstream, upstreamPort, lastUpstreamHeaders; + const ONEM = "context-1m-2025-08-07"; + + before(async () => { + upstream = fakeSseUpstream((req) => { + lastUpstreamHeaders = req.headers; + }); + await new Promise((r) => upstream.listen(0, "127.0.0.1", r)); + upstreamPort = upstream.address().port; + + process.env.CACHE_FIX_PROXY_UPSTREAM = `http://127.0.0.1:${upstreamPort}`; + process.env.CACHE_FIX_AUTO_1M_GUARD = "strip"; + // Real extensionsDir/config (default), so auto-1m-guard runs for real + // alongside the rest of the always-loaded pipeline. + handle = await startProxy({ port: 0, watch: false }); + }); + + after(async () => { + await handle.close(); + await new Promise((r) => upstream.close(r)); + delete process.env.CACHE_FIX_PROXY_UPSTREAM; + delete process.env.CACHE_FIX_AUTO_1M_GUARD; + }); + + it("strip mode: context-1m-2025-08-07 is ABSENT from the outbound anthropic-beta header on the real wire", async () => { + await clientRequest( + handle.port, + { model: "test", messages: [] }, + { "anthropic-beta": `claude-code-20250219, oauth_auth, ${ONEM}, context-management-2025-06-27` }, + ); + const outboundBeta = lastUpstreamHeaders["anthropic-beta"] || ""; + assert.ok( + !outboundBeta.includes(ONEM), + `expected ${ONEM} stripped from outbound anthropic-beta, got: ${JSON.stringify(outboundBeta)}`, + ); + assert.ok(outboundBeta.includes("oauth_auth"), "other beta tokens must survive the strip"); + }); +});