diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index d28711ce12..a50df1069b 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -360,7 +360,10 @@ 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 { + logClientRawRequestRedacted, + redactPendingBody, +} from "@/lib/guardrails/videoBridgeSnapshotRedaction"; import { shouldPreserveCacheControl, resolveConnectionCacheOverride, @@ -928,7 +931,7 @@ export async function handleChatCore({ const pendingRequestId = trackPendingRequest(model, provider, pendingConnId, true, { clientEndpoint: clientRawRequest?.endpoint || "/v1/chat/completions", - clientRequest: clientRawRequest?.body ?? body, + clientRequest: redactPendingBody(clientRawRequest?.body ?? body, videoBridgeObserved), providerRequest: initialProviderRequest, stage: "registered", correlationId, diff --git a/open-sse/handlers/chatCore/attemptLogging.ts b/open-sse/handlers/chatCore/attemptLogging.ts index 2a583896a7..65a492a541 100644 --- a/open-sse/handlers/chatCore/attemptLogging.ts +++ b/open-sse/handlers/chatCore/attemptLogging.ts @@ -50,6 +50,16 @@ import { attachLogMeta } from "./cacheUsageMeta.ts"; * never touches a part whose text differs — see * `tests/unit/video-bridge-log-redaction.test.ts`'s "Scenario A" test for the * reproduction this fixes. + * + * #12430 item 4 (P2c): a message's `content` can also be a plain STRING that + * embeds `fullText` as a SUBSTRING rather than an exact array part — derived + * dispatches (pipeline-strategy stages, smart-auto-pipeline, context-handoff + * summaries) all interpolate the transcript blob into a larger rendered + * prompt string before calling `handleSingleModel`. That string branch is + * mutually exclusive with the array branch (a message's `content` is one or + * the other, never both) and uses `String.prototype.replaceAll` against the + * trusted `fullText` literal to swap every occurrence — see + * `tests/unit/video-bridge-derived-prompt-redaction.test.ts`. */ export function applyVideoBridgeLogRedaction( body: unknown, @@ -78,6 +88,47 @@ export function applyVideoBridgeLogRedaction( const originalMessage = originalContainer[messageIndex]; if (!originalMessage || typeof originalMessage !== "object") continue; const originalContent = (originalMessage as Record).content; + + // Derived-prompt dispatches (pipeline-strategy stages, smart-auto-pipeline, + // context-handoff summaries — #12430 item 4) embed the transcript as a + // SUBSTRING of a plain string `content`, e.g. a rendered stage prompt or a + // `{HISTORY}`-interpolated handoff summary, never as an exact array part. + // Mutually exclusive with the array branch below: a message's `content` + // is either a string or an array, never both, so this and the + // `Array.isArray` check never both match the same message. + if (typeof originalContent === "string") { + if (!originalContent.includes(fullText)) continue; + + // Same lazy clone-on-write as the array branch: root -> container + // array -> this message. Siblings keep referencing the originals. + if (!rootClone) rootClone = { ...source }; + let containerClone = clonedContainers.get(container); + if (!containerClone) { + containerClone = [...originalContainer]; + clonedContainers.set(container, containerClone); + rootClone[container] = containerClone; + } + + const messageKey = `${container}:${messageIndex}`; + let messageClone = clonedMessages.get(messageKey); + if (!messageClone) { + messageClone = { ...(originalMessage as Record) }; + clonedMessages.set(messageKey, messageClone); + containerClone[messageIndex] = messageClone; + } + + // Re-read from the (possibly already-cloned) message so a second + // redaction entry matching the same string content composes with the + // first instead of clobbering it. `fullText` is a trusted literal + // (the `[Video description:...]` blob), so replaceAll(string, string) + // needs no regex and is safe. replaceAll (not replace): a stage/summary + // prompt can quote the transcript back more than once. + const currentText = + typeof messageClone.content === "string" ? messageClone.content : originalContent; + messageClone.content = currentText.replaceAll(fullText, redactedText); + redacted = true; + continue; + } if (!Array.isArray(originalContent)) continue; for (let partIndex = 0; partIndex < originalContent.length; partIndex++) { diff --git a/src/lib/guardrails/videoBridgeSnapshotRedaction.ts b/src/lib/guardrails/videoBridgeSnapshotRedaction.ts index 8738b47cfc..8324b4be90 100644 --- a/src/lib/guardrails/videoBridgeSnapshotRedaction.ts +++ b/src/lib/guardrails/videoBridgeSnapshotRedaction.ts @@ -133,3 +133,17 @@ export function logClientRawRequestRedacted( clientRawRequest.headers ); } + +/** + * Call-site wrapper for the `clientRequest` field stored by `trackPendingRequest` + * (open-sse/handlers/chatCore.ts): the sibling in-memory leak to + * `logClientRawRequestRedacted` above — same raw body, but live-exposed via + * /api/usage/call-logs (pendingDetails), /api/logs/[id] and /api/conversations + * while the request is in-flight, not just in the persisted detailed-log + * snapshot. Identical observed/non-observed branching: a non-observed request + * keeps the exact same reference (no clone); an observed one gets the redacted + * clone. + */ +export function redactPendingBody(clientRequest: unknown, videoBridgeObserved: boolean): unknown { + return videoBridgeObserved ? redactVideoTranscriptFieldsForLog(clientRequest) : clientRequest; +} diff --git a/tests/unit/guardrails/videoBridgeSnapshotRedaction.test.ts b/tests/unit/guardrails/videoBridgeSnapshotRedaction.test.ts index 3f48bee7dc..8ca1dbab9f 100644 --- a/tests/unit/guardrails/videoBridgeSnapshotRedaction.test.ts +++ b/tests/unit/guardrails/videoBridgeSnapshotRedaction.test.ts @@ -7,7 +7,10 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { redactVideoTranscriptFieldsForLog } from "../../../src/lib/guardrails/videoBridgeSnapshotRedaction.ts"; +import { + redactVideoTranscriptFieldsForLog, + redactPendingBody, +} 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"; @@ -205,3 +208,57 @@ test("the redaction placeholder matches the canonical P1 constant (no drift)", ( const part = contentAt(result, "messages", 0)[0]; assert.equal(part.transcript, VIDEO_TRANSCRIPT_REDACTION_PLACEHOLDER); }); + +// #12430 item 6 (P2c): the sibling in-memory leak. `trackPendingRequest` +// (open-sse/handlers/chatCore.ts) stores the same raw client body under +// `clientRequest`, live-exposed via /api/usage/call-logs (pendingDetails), +// /api/logs/[id] and /api/conversations while the request is in-flight. This +// helper is the guarded call-site wrapper chatCore.ts uses, mirroring +// logClientRawRequestRedacted's observed/non-observed branching. +test("redactPendingBody: observed=true delegates to redactVideoTranscriptFieldsForLog", () => { + const body = { + messages: [ + { + role: "user", + content: [ + { + type: "input_video", + video_url: "https://example.com/clip.mp4", + transcript: { cues: [{ text: "pending secret" }] }, + }, + ], + }, + ], + }; + + const result = redactPendingBody(body, true); + assert.notEqual( + result, + body, + "observed path must return a new structure, not the same reference" + ); + const part = contentAt(result, "messages", 0)[0]; + assert.equal(part.transcript, VIDEO_TRANSCRIPT_REDACTION_PLACEHOLDER); + assert.ok(!JSON.stringify(result).includes("pending secret")); + assert.deepEqual(result, redactVideoTranscriptFieldsForLog(body)); +}); + +test("redactPendingBody: observed=false returns the SAME reference unchanged", () => { + const body = { + messages: [ + { + role: "user", + content: [ + { + type: "input_video", + video_url: "https://example.com/clip.mp4", + transcript: { cues: [{ text: "not observed" }] }, + }, + ], + }, + ], + }; + + const result = redactPendingBody(body, false); + assert.equal(result, body, "non-observed path must return the exact same reference"); +}); diff --git a/tests/unit/video-bridge-derived-prompt-redaction.test.ts b/tests/unit/video-bridge-derived-prompt-redaction.test.ts new file mode 100644 index 0000000000..1e31e75ea1 --- /dev/null +++ b/tests/unit/video-bridge-derived-prompt-redaction.test.ts @@ -0,0 +1,184 @@ +// tests/unit/video-bridge-derived-prompt-redaction.test.ts +// P2c of #12150/#12430 (Video Bridge transcript retention — derived-prompt +// dispatch logs, item 4). +// +// Seam trace finding (decisive): videoBridgeLog is ALREADY threaded end-to-end +// to every nested handleChatCore — pipeline-strategy stages +// (src/domain/pipeline.ts::executeStage), smart-auto-pipeline, and +// context-handoff summaries (open-sse/services/contextHandoff.ts) — because +// all of them dispatch through the single P1b `handleSingleModel` closure and +// terminate in the SAME handleChatCore -> persistAttemptLogs -> +// applyVideoBridgeLogRedaction logging path. No plumbing/param changes were +// needed anywhere. +// +// The gap this file proves closed: those derived dispatches embed the +// transcript as a SUBSTRING of a plain STRING `content` message — +// `{ role: "user", content: }` — built by executeStage() +// (pipeline.ts:196-199 via prompts.ts interpolation) and by the +// context-handoff summary builders (contextHandoff.ts:415/729, `{HISTORY}` +// template substitution). Before this fix, applyVideoBridgeLogRedaction only +// matched ARRAY-content parts by exact text (`part.text === fullText`), so it +// silently skipped these string-content messages and the raw transcript +// persisted in the stage/summary sub-request call logs. +// +// This suite calls the real, already-exported `applyVideoBridgeLogRedaction` +// (open-sse/handlers/chatCore/attemptLogging.ts) directly — it is a pure +// function (no DB), so no persistAttemptLogs/DB harness is needed here; that +// integration-level proof already lives in +// tests/unit/video-bridge-log-redaction.test.ts. +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { applyVideoBridgeLogRedaction } from "../../open-sse/handlers/chatCore/attemptLogging.ts"; +import type { VideoBridgeLogRedactionEntry } from "../../src/lib/guardrails/videoBridge.ts"; + +const SECRET = "secret words"; +const FULL_TEXT = `[Video description: transcript[source=client] ${SECRET}]`; +const REDACTED_TEXT = "[Video description: transcript[source=client] [redacted-video-transcript]]"; + +function entry( + overrides: Partial = {} +): VideoBridgeLogRedactionEntry { + return { + container: "messages", + messageIndex: 0, + partIndex: 0, + fullText: FULL_TEXT, + redactedText: REDACTED_TEXT, + ...overrides, + }; +} + +test("derived-prompt (pipeline stage): a string-content message with the transcript embedded as a substring is redacted, secret absent, surrounding prompt text intact", () => { + const body = { + model: "openai/gpt-x", + messages: [ + { role: "system", content: "You are a summarization stage." }, + { + role: "user", + content: `Summarize the following context.\n\n${FULL_TEXT}\n\nEnd of context.`, + }, + ], + }; + + const result = applyVideoBridgeLogRedaction(body, [ + entry({ messageIndex: 1, partIndex: 0 }), + ]) as typeof body; + + const redactedContent = result.messages[1].content; + assert.equal( + redactedContent, + `Summarize the following context.\n\n${REDACTED_TEXT}\n\nEnd of context.` + ); + assert.ok(!redactedContent.includes(SECRET), "the raw transcript must not survive redaction"); + assert.ok( + redactedContent.startsWith("Summarize the following context.\n\n"), + "surrounding prompt text before the blob must stay intact" + ); + assert.ok( + redactedContent.endsWith("\n\nEnd of context."), + "surrounding prompt text after the blob must stay intact" + ); + assert.equal(JSON.stringify(result).includes(SECRET), false); +}); + +test("derived-prompt (context-handoff summary): input container string content is redacted the same way as messages", () => { + const body = { + model: "openai/gpt-x", + input: [ + { + role: "user", + content: `Continue the conversation given this history.\n\n${FULL_TEXT}\n\nContinue now.`, + }, + ], + }; + + const result = applyVideoBridgeLogRedaction(body, [ + entry({ container: "input", messageIndex: 0, partIndex: 0 }), + ]) as typeof body; + + const redactedContent = result.input[0].content; + assert.equal( + redactedContent, + `Continue the conversation given this history.\n\n${REDACTED_TEXT}\n\nContinue now.` + ); + assert.ok(!redactedContent.includes(SECRET)); + assert.equal(JSON.stringify(result).includes(SECRET), false); +}); + +test("multiple occurrences of fullText within the same string are ALL replaced (replaceAll, not replace)", () => { + const body = { + messages: [ + { + role: "user", + content: `First mention: ${FULL_TEXT}\n\nQuoted back for grounding: ${FULL_TEXT}\n\nDone.`, + }, + ], + }; + + const result = applyVideoBridgeLogRedaction(body, [entry({ messageIndex: 0, partIndex: 0 })]) as { + messages: Array<{ content: string }>; + }; + + const redactedContent = result.messages[0].content; + assert.equal( + redactedContent, + `First mention: ${REDACTED_TEXT}\n\nQuoted back for grounding: ${REDACTED_TEXT}\n\nDone.` + ); + assert.equal( + redactedContent.split(REDACTED_TEXT).length - 1, + 2, + "both occurrences must be replaced" + ); + assert.ok(!redactedContent.includes(SECRET)); +}); + +test("regression: the existing ARRAY-content exact-part-match path still redacts (no regression from the new string branch)", () => { + const body = { + messages: [ + { role: "system", content: "sys" }, + { + role: "user", + content: [ + { type: "text", text: "look at this video" }, + { type: "text", text: FULL_TEXT }, + ], + }, + ], + }; + + const result = applyVideoBridgeLogRedaction(body, [entry({ messageIndex: 1, partIndex: 1 })]) as { + messages: Array<{ content: unknown }>; + }; + + const content = result.messages[1].content as Array<{ text: string }>; + assert.equal(content[1].text, REDACTED_TEXT); + assert.ok(!content[1].text.includes(SECRET)); + assert.equal(content[0].text, "look at this video", "sibling part must stay untouched"); +}); + +test("no mutation of the input object: the caller's body is byte-identical after redaction (string-content path)", () => { + const body = { + messages: [{ role: "user", content: `before ${FULL_TEXT} after` }], + }; + const snapshotBefore = JSON.parse(JSON.stringify(body)); + + applyVideoBridgeLogRedaction(body, [entry({ messageIndex: 0, partIndex: 0 })]); + + assert.deepEqual(body, snapshotBefore, "the original body must never be mutated"); +}); + +test("non-matching string content is returned unchanged, with the SAME root reference (nothing redacted -> no clone allocated)", () => { + const body = { + messages: [{ role: "user", content: "nothing to see here, no transcript blob at all" }], + }; + + const result = applyVideoBridgeLogRedaction(body, [entry({ messageIndex: 0, partIndex: 0 })]); + + assert.equal( + result, + body, + "when no fullText matches, the exact same object reference is returned" + ); +});