From 879894a5bdd0c01baba3623fe2c5bebface5890d Mon Sep 17 00:00:00 2001 From: Ayaan Zaidi Date: Thu, 30 Jul 2026 02:12:32 +0200 Subject: [PATCH] fix(proxy): redact sensitive headers in standalone debug proxy captures The standalone debug proxy wrote raw request and response headers into capture rows while the patched-fetch runtime redacted at the parallel call sites, so a capture taken through the proxy could persist Authorization, Cookie, and API-key values to disk. Move the runtime's redaction policy into a leaf module both writers import instead of adding a second copy, so the two capture paths cannot drift. The shared helper also flattens node's array-valued headers, which the standalone proxy passes in directly, and keeps value-level registered-secret redaction for header names that are not themselves sensitive. Reported by SebTardif in #90009; supersedes #82951, which redacted only by header name and predates the proxy-server rewrite. --- src/proxy-capture/header-redaction.test.ts | 63 +++++++++++++++++++ src/proxy-capture/header-redaction.ts | 70 ++++++++++++++++++++++ src/proxy-capture/proxy-server.ts | 7 ++- src/proxy-capture/runtime.ts | 54 +---------------- 4 files changed, 138 insertions(+), 56 deletions(-) create mode 100644 src/proxy-capture/header-redaction.test.ts create mode 100644 src/proxy-capture/header-redaction.ts diff --git a/src/proxy-capture/header-redaction.test.ts b/src/proxy-capture/header-redaction.test.ts new file mode 100644 index 000000000000..3e2574953fe5 --- /dev/null +++ b/src/proxy-capture/header-redaction.test.ts @@ -0,0 +1,63 @@ +/** Canonical debug-proxy capture header redaction. */ +import { afterEach, describe, expect, it } from "vitest"; +import { registerSecretValueForRedaction } from "../logging/secret-redaction-registry.js"; +import { resetSecretRedactionRegistryForTest } from "../logging/secret-redaction-registry.test-support.js"; +import { isSensitiveCaptureHeaderName, redactedCaptureHeaders } from "./header-redaction.js"; + +afterEach(() => { + resetSecretRedactionRegistryForTest(); +}); + +describe("redactedCaptureHeaders", () => { + it("redacts credential-bearing header names regardless of case", () => { + const redacted = redactedCaptureHeaders({ + Authorization: "Bearer live-token", + COOKIE: "session=abc", + "X-Api-Key": "sk-live", + "content-type": "application/json", + }); + expect(redacted).toEqual({ + Authorization: "[REDACTED]", + COOKIE: "[REDACTED]", + "X-Api-Key": "[REDACTED]", + "content-type": "application/json", + }); + }); + + it("redacts a registered secret pasted into an otherwise innocuous header", () => { + // The name check alone would pass this through; value redaction is what + // keeps a leaked token out of the capture. + registerSecretValueForRedaction("super-secret-value"); + const redacted = redactedCaptureHeaders({ "x-trace-note": "ctx super-secret-value end" }); + expect(redacted?.["x-trace-note"]).not.toContain("super-secret-value"); + }); + + it("flattens node's array-valued headers instead of dropping them", () => { + // node:http exposes repeated headers as arrays; the standalone proxy feeds + // those in directly. + const redacted = redactedCaptureHeaders({ + "set-cookie": ["a=1", "b=2"], + via: ["1.1 a", "1.1 b"], + }); + expect(redacted?.["set-cookie"]).toBe("[REDACTED]"); + expect(redacted?.via).toBe("1.1 a, 1.1 b"); + }); + + it("accepts a Headers instance", () => { + const redacted = redactedCaptureHeaders( + new Headers({ authorization: "Bearer x", accept: "text/plain" }), + ); + expect(redacted?.authorization).toBe("[REDACTED]"); + expect(redacted?.accept).toBe("text/plain"); + }); + + it("returns undefined when there are no headers", () => { + expect(redactedCaptureHeaders(undefined)).toBeUndefined(); + }); + + it("treats token-ish name fragments as sensitive", () => { + expect(isSensitiveCaptureHeaderName("x-vendor-access-token")).toBe(true); + expect(isSensitiveCaptureHeaderName("x-session-id")).toBe(true); + expect(isSensitiveCaptureHeaderName("accept-language")).toBe(false); + }); +}); diff --git a/src/proxy-capture/header-redaction.ts b/src/proxy-capture/header-redaction.ts new file mode 100644 index 000000000000..e0078f163fd0 --- /dev/null +++ b/src/proxy-capture/header-redaction.ts @@ -0,0 +1,70 @@ +/** + * Canonical header redaction for debug proxy captures. + * + * Both capture writers — the patched-fetch runtime and the standalone proxy + * server — must redact identically. A capture that leaks credentials is worse + * than no capture, and the standalone path previously stored raw headers while + * the runtime path redacted, so this policy lives in one leaf module that both + * import rather than being duplicated per writer. + */ +import { redactRegisteredSecretValues } from "../logging/secret-redaction-registry.js"; + +export const REDACTED_CAPTURE_HEADER_VALUE = "[REDACTED]"; + +const SENSITIVE_CAPTURE_HEADER_NAMES = new Set([ + "authorization", + "proxy-authorization", + "cookie", + "set-cookie", + "x-api-key", + "api-key", + "apikey", + "x-auth-token", + "auth-token", + "x-access-token", + "access-token", +]); +const SENSITIVE_CAPTURE_HEADER_NAME_FRAGMENTS = [ + "api-key", + "apikey", + "token", + "secret", + "password", + "credential", + "session", +]; + +export function isSensitiveCaptureHeaderName(name: string): boolean { + const normalized = name.trim().toLowerCase(); + if (!normalized) { + return false; + } + if (SENSITIVE_CAPTURE_HEADER_NAMES.has(normalized)) { + return true; + } + return SENSITIVE_CAPTURE_HEADER_NAME_FRAGMENTS.some((fragment) => normalized.includes(fragment)); +} + +export function redactedCaptureHeaders( + headers: Headers | Record | undefined, +): Record | undefined { + if (!headers) { + return undefined; + } + const entries = + headers instanceof Headers ? Array.from(headers.entries()) : Object.entries(headers); + const redacted: Record = {}; + for (const [name, value] of entries) { + // Header names are matched exactly and by sensitive fragments because + // providers use many token/key naming variants. Names that pass the check + // still run through value redaction so a registered secret pasted into an + // innocuous header does not survive. + if (isSensitiveCaptureHeaderName(name)) { + redacted[name] = REDACTED_CAPTURE_HEADER_VALUE; + continue; + } + const flattened = Array.isArray(value) ? value.join(", ") : (value ?? ""); + redacted[name] = redactRegisteredSecretValues(flattened, () => REDACTED_CAPTURE_HEADER_VALUE); + } + return redacted; +} diff --git a/src/proxy-capture/proxy-server.ts b/src/proxy-capture/proxy-server.ts index daa6e40f0922..42db7ac47d21 100644 --- a/src/proxy-capture/proxy-server.ts +++ b/src/proxy-capture/proxy-server.ts @@ -8,6 +8,7 @@ import { StringDecoder } from "node:string_decoder"; import { URL } from "node:url"; import { ensureDebugProxyCa } from "./ca.js"; import type { DebugProxySettings } from "./env.js"; +import { redactedCaptureHeaders } from "./header-redaction.js"; import { getDebugProxyCaptureStore } from "./store.sqlite.js"; import type { CaptureEventRecord } from "./types.js"; @@ -282,7 +283,7 @@ export async function startDebugProxyServer(params: { direction: "inbound", kind: "response", status: upstreamRes.statusCode ?? undefined, - headersJson: JSON.stringify(upstreamRes.headers), + headersJson: JSON.stringify(redactedCaptureHeaders(upstreamRes.headers)), ...finishBodyPreviewCapture(responseCapture), }); }); @@ -337,7 +338,7 @@ export async function startDebugProxyServer(params: { recordTargetEvent({ direction: "outbound", kind: "request", - headersJson: JSON.stringify(req.headers), + headersJson: JSON.stringify(redactedCaptureHeaders(req.headers)), ...finishBodyPreviewCapture(requestCapture), }); }); @@ -389,7 +390,7 @@ export async function startDebugProxyServer(params: { flowId, host: hostname, path: req.url ?? "", - headersJson: JSON.stringify(req.headers), + headersJson: JSON.stringify(redactedCaptureHeaders(req.headers)), }); try { assertDebugProxyDirectUpstreamAllowed(); diff --git a/src/proxy-capture/runtime.ts b/src/proxy-capture/runtime.ts index 0eaba9d45c4e..1ce29c17107d 100644 --- a/src/proxy-capture/runtime.ts +++ b/src/proxy-capture/runtime.ts @@ -8,6 +8,7 @@ import { redactRegisteredSecretValues, } from "../logging/secret-redaction-registry.js"; import { resolveDebugProxySettings, type DebugProxySettings } from "./env.js"; +import { redactedCaptureHeaders, REDACTED_CAPTURE_HEADER_VALUE } from "./header-redaction.js"; import { closeDebugProxyCaptureStore, getDebugProxyCaptureStore, @@ -22,7 +23,6 @@ import type { } from "./types.js"; const DEBUG_PROXY_FETCH_PATCH_KEY = Symbol.for("openclaw.debugProxy.fetchPatch"); -const REDACTED_CAPTURE_HEADER_VALUE = "[REDACTED]"; const REDACTED_CAPTURE_BINARY_PAYLOAD = Buffer.from("[REDACTED BINARY PAYLOAD]", "utf8"); // Cap captured response bodies so debug proxy capture cannot be turned into an // out-of-memory vector. The patched global fetch tees every outbound response @@ -91,28 +91,6 @@ async function readCapturedResponseBodyBounded( ? { status: "too-large" } : { status: "captured", buffer: Buffer.concat(chunks, total) }; } -const SENSITIVE_CAPTURE_HEADER_NAMES = new Set([ - "authorization", - "proxy-authorization", - "cookie", - "set-cookie", - "x-api-key", - "api-key", - "apikey", - "x-auth-token", - "auth-token", - "x-access-token", - "access-token", -]); -const SENSITIVE_CAPTURE_HEADER_NAME_FRAGMENTS = [ - "api-key", - "apikey", - "token", - "secret", - "password", - "credential", - "session", -]; function parseDeclaredCaptureContentLength(raw: string | null | undefined): bigint | undefined { if (raw === null || raw === undefined) { @@ -195,36 +173,6 @@ function resolveUrlString(input: RequestInfo | URL): string | null { return null; } -function isSensitiveCaptureHeaderName(name: string): boolean { - const normalized = name.trim().toLowerCase(); - if (!normalized) { - return false; - } - if (SENSITIVE_CAPTURE_HEADER_NAMES.has(normalized)) { - return true; - } - return SENSITIVE_CAPTURE_HEADER_NAME_FRAGMENTS.some((fragment) => normalized.includes(fragment)); -} - -function redactedCaptureHeaders( - headers: Headers | Record | undefined, -): Record | undefined { - if (!headers) { - return undefined; - } - const entries = - headers instanceof Headers ? Array.from(headers.entries()) : Object.entries(headers); - const redacted: Record = {}; - for (const [name, value] of entries) { - // Header names are matched exactly and by sensitive fragments because - // providers use many token/key naming variants. - redacted[name] = isSensitiveCaptureHeaderName(name) - ? REDACTED_CAPTURE_HEADER_VALUE - : redactRegisteredSecretValues(value, () => REDACTED_CAPTURE_HEADER_VALUE); - } - return redacted; -} - function redactCaptureUrl(rawUrl: string): string { let url: URL; try {