diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index 8dbac0018b..d28711ce12 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -360,6 +360,7 @@ import { deleteSessionAccountAffinity } from "@/lib/db/sessionAccountAffinity"; import { getCacheControlSettings } from "@/lib/cacheControlSettings"; import { guardrailRegistry } from "@/lib/guardrails"; import type { VideoBridgeLogRedactionEntry } from "@/lib/guardrails/videoBridge"; +import { logClientRawRequestRedacted } from "@/lib/guardrails/videoBridgeSnapshotRedaction"; import { shouldPreserveCacheControl, resolveConnectionCacheOverride, @@ -1211,14 +1212,9 @@ export async function handleChatCore({ }); const pendingScope = { id: pendingRequestId, model, provider, connectionId: pendingConnId }; const providerRequestCapture = createPreparedRequestLogger(reqLogger, pendingScope); - // 0. Log client raw request (before format conversion) - if (clientRawRequest) { - reqLogger.logClientRawRequest( - clientRawRequest.endpoint, - clientRawRequest.body, - clientRawRequest.headers - ); - } + // 0. Log client raw request (before format conversion) — redacts video transcript + // cues in the logged copy only; see videoBridgeSnapshotRedaction.ts. + logClientRawRequestRedacted(reqLogger, clientRawRequest, videoBridgeObserved); const reasoningRouteDecision = body && typeof body === "object" ? (body as Record)._omnirouteReasoningRouteTrace diff --git a/src/lib/guardrails/videoBridgeSnapshotRedaction.ts b/src/lib/guardrails/videoBridgeSnapshotRedaction.ts new file mode 100644 index 0000000000..8738b47cfc --- /dev/null +++ b/src/lib/guardrails/videoBridgeSnapshotRedaction.ts @@ -0,0 +1,135 @@ +/** + * #12150 P2 surface 1 (the dominant transcript-retention leak): structured redaction of + * video transcript fields on the CLIENT-REQUEST SNAPSHOT that lands in the detailed-log + * artifact. + * + * `clientRawRequest.body` (src/sse/handlers/chat/clientRawRequest.ts::buildClientRawRequest) + * is a bounded clone of the client's ORIGINAL request, captured BEFORE the guardrail chain + * runs, and persisted verbatim by `reqLogger.logClientRawRequest` + * (open-sse/handlers/chatCore.ts). Because it predates the video-bridge guardrail's own + * description redaction (#12150 P1 — see `describeVideoPart`'s `descriptionRedacted` in + * videoBridgeHelpers.ts), it still carries the client's raw `transcript` / `audioTranscript` + * cue text on any video part. This module redacts THAT COPY ONLY: the body sent to the + * provider and the response returned to the client are never touched here. + * + * Deliberately a standalone, dependency-light module — NOT part of videoBridgeHelpers.ts, + * which pulls in the frame-extraction broker client, audio/video fusion, contact-sheet + * composition and `sharp` for real video processing. The chat request hot path statically + * imports whatever module owns the `logClientRawRequest` call site on every request + * (video or not), so keeping this redaction free of that dependency chain matters for cold + * start and blast radius. + * + * The field walk mirrors `extractVideoParts` (videoBridgeHelpers.ts): for each content part, + * the candidate objects are the part itself, its `video_url` sub-object, and its `source` + * sub-object (the same three checked there) — but this walk is deliberately WIDER: any of + * those objects carrying a `transcript`/`audioTranscript` key gets redacted regardless of + * the part's `type`/shape. Those two field names are video-cue-only in this codebase's + * request contract, so matching on field presence rather than a shape allowlist is strictly + * safer (fails closed on an unusual or future video shape instead of silently skipping it). + * Redaction is a structured field substitution, not a scan over rendered text, so it cannot + * be bypassed by adversarial cue content (see the discarded regex approach recorded in the + * #12150 design doc, `_tasks/superpowers/specs/2026-09-01-video-transcript-retention-design.md`). + */ + +// Kept as a local literal (not imported from videoBridgeHelpers.ts) for the reason in the +// file header above. Equality with the canonical `VIDEO_TRANSCRIPT_REDACTION_PLACEHOLDER` +// export is enforced by a drift test in +// tests/unit/guardrails/videoBridgeSnapshotRedaction.test.ts. +const REDACTION_PLACEHOLDER = "[redacted-video-transcript]"; + +const TRANSCRIPT_FIELD_NAMES = ["transcript", "audioTranscript"] as const; +const NESTED_SUBOBJECT_KEYS = ["video_url", "source"] as const; +const CONTAINER_KEYS = ["messages", "input"] as const; + +type UnknownRecord = Record; + +function isPlainRecord(value: unknown): value is UnknownRecord { + return Boolean(value) && typeof value === "object" && !Array.isArray(value); +} + +/** + * Overwrites transcript field VALUES in place on `part` and its `video_url`/`source` + * sub-objects. Only ever called on a part that already lives inside the function's own + * `structuredClone`, never on caller-owned data. Keys are overwritten, never deleted, so + * downstream shape/observability (e.g. "this part had a transcript") is preserved. + */ +function redactTranscriptFieldsOnPart(part: unknown): void { + if (!isPlainRecord(part)) return; + const candidates: UnknownRecord[] = [part]; + for (const key of NESTED_SUBOBJECT_KEYS) { + const nested = part[key]; + if (isPlainRecord(nested)) candidates.push(nested); + } + for (const candidate of candidates) { + for (const field of TRANSCRIPT_FIELD_NAMES) { + if (candidate[field] !== undefined) { + candidate[field] = REDACTION_PLACEHOLDER; + } + } + } +} + +function redactContentArray(content: unknown): void { + if (!Array.isArray(content)) return; + for (const part of content) { + redactTranscriptFieldsOnPart(part); + } +} + +/** `messages` (Chat Completions) or `input` (Responses API) — either container shape. */ +function redactContainer(container: unknown): void { + if (!Array.isArray(container)) return; + for (const message of container) { + if (!isPlainRecord(message)) continue; + redactContentArray(message.content); + } +} + +/** + * Returns a NEW structure with every video transcript cue field value replaced by the + * redaction placeholder. Never mutates `body` — the caller (chatCore.ts) must keep passing + * the untouched original to translation/dispatch/response. A non-object `body`, or one with + * neither `messages` nor `input`, or with video parts that carry no transcript field, is + * returned as an equivalent (cloned) structure with nothing to change. + */ +export function redactVideoTranscriptFieldsForLog(body: unknown): unknown { + if (!isPlainRecord(body)) return body; + const cloned = structuredClone(body) as UnknownRecord; + for (const key of CONTAINER_KEYS) { + redactContainer(cloned[key]); + } + return cloned; +} + +interface ClientRawRequestLike { + endpoint: unknown; + body: unknown; + headers?: unknown; +} + +interface RequestLoggerLike { + logClientRawRequest: (endpoint: unknown, body: unknown, headers?: unknown) => void; +} + +/** + * Call-site wrapper for `reqLogger.logClientRawRequest` (chatCore.ts's "0. Log client raw + * request" step): keeps the null-check and the observed/redacted guard out of chatCore.ts, + * which is a size-frozen file (`config/quality/file-size-baseline.json`) — this owns the + * redaction, so it owns the one guarded call site that applies it. Behavior is identical to + * the inline block it replaces: a non-observed request logs `clientRawRequest.body` by the + * exact same reference (no clone); an observed one logs the redacted clone. + */ +export function logClientRawRequestRedacted( + reqLogger: RequestLoggerLike, + clientRawRequest: ClientRawRequestLike | null | undefined, + videoBridgeObserved: boolean +): void { + if (!clientRawRequest) return; + reqLogger.logClientRawRequest( + clientRawRequest.endpoint, + videoBridgeObserved + ? redactVideoTranscriptFieldsForLog(clientRawRequest.body) + : clientRawRequest.body, + clientRawRequest.headers + ); +} diff --git a/tests/unit/guardrails/videoBridgeSnapshotRedaction.test.ts b/tests/unit/guardrails/videoBridgeSnapshotRedaction.test.ts new file mode 100644 index 0000000000..3f48bee7dc --- /dev/null +++ b/tests/unit/guardrails/videoBridgeSnapshotRedaction.test.ts @@ -0,0 +1,207 @@ +// #12150 P2 surface 1 (the dominant transcript-retention leak): pure-helper coverage for +// redactVideoTranscriptFieldsForLog — the structured redaction applied to the RAW +// client-request snapshot (clientRawRequest.body) before it is persisted by +// reqLogger.logClientRawRequest (open-sse/handlers/chatCore.ts). See +// src/lib/guardrails/videoBridgeSnapshotRedaction.ts for the full design rationale +// (deliberately dependency-light; field-presence match rather than a shape allowlist). +import assert from "node:assert/strict"; +import test from "node:test"; + +import { redactVideoTranscriptFieldsForLog } from "../../../src/lib/guardrails/videoBridgeSnapshotRedaction.ts"; +// Heavy import is fine here (test only, never in the production module under test) — used +// solely to prove the local placeholder literal never drifts from the canonical P1 constant. +import { VIDEO_TRANSCRIPT_REDACTION_PLACEHOLDER } from "../../../src/lib/guardrails/videoBridgeHelpers.ts"; + +type JsonRecord = Record; + +function asRecord(value: unknown): JsonRecord { + return value as JsonRecord; +} + +function contentAt( + body: unknown, + container: "messages" | "input", + messageIndex: number +): JsonRecord[] { + const messages = asRecord(body)[container] as JsonRecord[]; + return messages[messageIndex].content as JsonRecord[]; +} + +test("redacts transcript and audioTranscript directly on a video part (messages container)", () => { + const body = { + model: "gpt-x", + messages: [ + { role: "system", content: "sys" }, + { + role: "user", + content: [ + { type: "text", text: "look at this video" }, + { + type: "input_video", + video_url: "https://example.com/clip.mp4", + transcript: { cues: [{ text: "secret words", startSeconds: 0, endSeconds: 2 }] }, + audioTranscript: { cues: [{ text: "audio secret", startSeconds: 0, endSeconds: 1 }] }, + }, + ], + }, + ], + }; + + const result = redactVideoTranscriptFieldsForLog(body); + assert.notEqual(result, body, "must return a new structure, not the same reference"); + + const videoPart = contentAt(result, "messages", 1)[1]; + assert.equal(videoPart.transcript, "[redacted-video-transcript]"); + assert.equal(videoPart.audioTranscript, "[redacted-video-transcript]"); + // The video ref itself and the sibling non-video part must survive untouched. + assert.equal(videoPart.video_url, "https://example.com/clip.mp4"); + assert.equal(contentAt(result, "messages", 1)[0].text, "look at this video"); + assert.equal(asRecord(result).messages, asRecord(result).messages); // sanity: still an array + + const serialized = JSON.stringify(result); + assert.ok(!serialized.includes("secret words"), "raw video transcript must not survive"); + assert.ok(!serialized.includes("audio secret"), "raw audio transcript must not survive"); +}); + +test("redacts a transcript nested under the video_url sub-object", () => { + const body = { + messages: [ + { + role: "user", + content: [ + { + type: "video_url", + video_url: { + url: "https://example.com/nested.mp4", + transcript: { cues: [{ text: "nested secret" }] }, + }, + }, + ], + }, + ], + }; + + const result = redactVideoTranscriptFieldsForLog(body); + const part = contentAt(result, "messages", 0)[0]; + const videoUrl = part.video_url as JsonRecord; + assert.equal(videoUrl.transcript, "[redacted-video-transcript]"); + assert.equal(videoUrl.url, "https://example.com/nested.mp4"); + assert.ok(!JSON.stringify(result).includes("nested secret")); +}); + +test("redacts a transcript nested under the source sub-object (video_source shape)", () => { + const body = { + messages: [ + { + role: "user", + content: [ + { + type: "video_source", + source: { + type: "url", + url: "https://example.com/source.mp4", + audioTranscript: { cues: [{ text: "source secret" }] }, + }, + }, + ], + }, + ], + }; + + const result = redactVideoTranscriptFieldsForLog(body); + const part = contentAt(result, "messages", 0)[0]; + const source = part.source as JsonRecord; + assert.equal(source.audioTranscript, "[redacted-video-transcript]"); + assert.equal(source.url, "https://example.com/source.mp4"); + assert.ok(!JSON.stringify(result).includes("source secret")); +}); + +test("covers the input container (Responses API shape)", () => { + const body = { + model: "gpt-x", + input: [ + { + role: "user", + content: [ + { + type: "input_video", + video_url: "https://example.com/input.mp4", + transcript: { cues: [{ text: "input secret" }] }, + }, + ], + }, + ], + }; + + const result = redactVideoTranscriptFieldsForLog(body); + const part = contentAt(result, "input", 0)[0]; + assert.equal(part.transcript, "[redacted-video-transcript]"); + assert.ok(!JSON.stringify(result).includes("input secret")); +}); + +test("does not mutate the input", () => { + const body = { + messages: [ + { + role: "user", + content: [ + { + type: "input_video", + video_url: "https://example.com/clip.mp4", + transcript: { cues: [{ text: "secret words" }] }, + audioTranscript: { cues: [{ text: "audio secret" }] }, + }, + ], + }, + ], + }; + const before = JSON.parse(JSON.stringify(body)); + + redactVideoTranscriptFieldsForLog(body); + + assert.deepEqual(body, before, "input object must be byte-identical after the call"); +}); + +test("a non-video body is returned unchanged", () => { + const body = { + model: "gpt-x", + messages: [ + { role: "system", content: "sys" }, + { role: "user", content: "hello, no video here" }, + ], + }; + + const result = redactVideoTranscriptFieldsForLog(body); + assert.deepEqual(result, body); +}); + +test("a body with a video part but no transcript field is unchanged", () => { + const body = { + messages: [ + { + role: "user", + content: [{ type: "input_video", video_url: "https://example.com/no-transcript.mp4" }], + }, + ], + }; + + const result = redactVideoTranscriptFieldsForLog(body); + assert.deepEqual(result, body); +}); + +test("the redaction placeholder matches the canonical P1 constant (no drift)", () => { + const body = { + messages: [ + { + role: "user", + content: [ + { type: "input_video", video_url: "https://example.com/clip.mp4", transcript: "raw" }, + ], + }, + ], + }; + + const result = redactVideoTranscriptFieldsForLog(body); + const part = contentAt(result, "messages", 0)[0]; + assert.equal(part.transcript, VIDEO_TRANSCRIPT_REDACTION_PLACEHOLDER); +}); diff --git a/tests/unit/video-bridge-log-redaction.test.ts b/tests/unit/video-bridge-log-redaction.test.ts index d190db80c4..268102d246 100644 --- a/tests/unit/video-bridge-log-redaction.test.ts +++ b/tests/unit/video-bridge-log-redaction.test.ts @@ -26,6 +26,8 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { logClientRawRequestRedacted } from "../../src/lib/guardrails/videoBridgeSnapshotRedaction.ts"; + const testDataDir = fs.mkdtempSync(path.join(os.tmpdir(), "omni-video-log-redaction-test-")); process.env.DATA_DIR = testDataDir; @@ -265,3 +267,79 @@ test("Scenario A (adversarial review): a message prepended AFTER the guardrail b "the prepended system message must be untouched" ); }); + +// #12150 P2 surface 1 (the dominant transcript-retention leak): the RAW client-request +// snapshot passed to reqLogger.logClientRawRequest (open-sse/handlers/chatCore.ts's +// "0. Log client raw request" step) is a DIFFERENT sink from persistAttemptLogs above — +// it is captured before the guardrail chain even runs, so it carries the client's raw +// `transcript`/`audioTranscript` FIELDS on a structured video part, not a flattened +// description string. Importing the real chatCore.ts here would pull the full +// request-pipeline dependency graph (executors, providers, combo routing, DB-backed +// settings, ...) into the test just to reach one guarded call a few hundred lines into +// a 5900+ line handler, for no additional proof beyond what's below — so this calls the +// REAL exported `logClientRawRequestRedacted` (the exact function chatCore.ts's call site +// invokes, post file-size-refactor) against a fake logClientRawRequest. The pure redaction +// helper itself has its own thorough suite in +// tests/unit/guardrails/videoBridgeSnapshotRedaction.test.ts. +function fakeReqLogger() { + const calls: unknown[] = []; + return { + calls, + logClientRawRequest(_endpoint: unknown, body: unknown, _headers?: unknown) { + calls.push(body); + }, + }; +} + +test("surface 2 (raw snapshot): the fake logClientRawRequest receives a redacted snapshot only when videoBridgeObserved is true", () => { + const rawBody = { + model: "openai/gpt-x", + messages: [ + { + role: "user", + content: [ + { type: "text", text: "look at this video" }, + { + type: "input_video", + video_url: "https://example.com/clip.mp4", + transcript: { cues: [{ text: SECRET, startSeconds: 0, endSeconds: 2 }] }, + }, + ], + }, + ], + }; + const clientRawRequest = { endpoint: "/v1/chat/completions", body: rawBody, headers: {} }; + + const observedLogger = fakeReqLogger(); + logClientRawRequestRedacted(observedLogger, clientRawRequest, true); + const observedSnapshot = observedLogger.calls[0]; + assert.ok( + !JSON.stringify(observedSnapshot).includes(SECRET), + "an observed request must not log the raw transcript" + ); + assert.notEqual( + observedSnapshot, + rawBody, + "the observed path must log a redacted CLONE, not the original reference" + ); + assert.ok( + JSON.stringify(rawBody).includes(SECRET), + "clientRawRequest.body itself must stay untouched for every other consumer (translation/dispatch)" + ); + + const nonObservedLogger = fakeReqLogger(); + logClientRawRequestRedacted(nonObservedLogger, clientRawRequest, false); + assert.equal( + nonObservedLogger.calls[0], + rawBody, + "the non-observed path must log the exact same object reference — byte-identical, no clone" + ); + + const skippedLogger = fakeReqLogger(); + logClientRawRequestRedacted(skippedLogger, null, true); + assert.equal( + skippedLogger.calls.length, + 0, + "a missing clientRawRequest must not call logClientRawRequest at all (mirrors the old if-guard)" + ); +});