diff --git a/docs/security/GUARDRAILS.md b/docs/security/GUARDRAILS.md index e4b956e43f..6d2e21c61e 100644 --- a/docs/security/GUARDRAILS.md +++ b/docs/security/GUARDRAILS.md @@ -476,23 +476,32 @@ fusion counters. The default Video Bridge path does not invoke speech-to-text or download a second media copy; without that explicit track, it remains video-only. -**Transcript retention (opt-in feature, #12150 P1).** When a request renders any -transcript cue (a caller-declared `transcript` or a fused `audioTranscript`), the -guardrail marks it `videoBridgeObserved` and produces a redacted shadow of the -video description — an identical rendering in which every cue's free-text body is -replaced by `[redacted-video-transcript]`, built by substituting the structured -cue field before the string is assembled (never by parsing the flattened text, so -no cue content — adversarial or ordinary, including bodies containing `]` such as +**Transcript retention (#12150 P1).** This applies automatically whenever the +Video Bridge (itself opt-in) renders a transcript cue — there is no separate +retention flag. When a request renders any transcript cue (a caller-declared +`transcript` or a fused `audioTranscript`), the guardrail marks it +`videoBridgeObserved` and produces a redacted shadow of the video description — +an identical rendering in which every cue's free-text body is replaced by +`[redacted-video-transcript]`, built by substituting the structured cue field +before the string is assembled (never by parsing the flattened text, so no cue +content — adversarial or ordinary, including bodies containing `]` such as `[inaudible]`/`[music]` — can survive). The persisted call-log request body swaps each video-derived text part for that redacted shadow, matched by content -equality (so it stays correct even after system-prompt/handoff/memory injection -reshapes the message array); the body sent upstream to the model is unchanged. -An observed request also populates no durable Memory (both request- and -response-derived extraction are skipped), so the model's own reply cannot echo -transcript text into Memory. Two further retention surfaces — the raw -pre-guardrail client-request snapshot in the detailed-log artifact and -`previous_response_id` continuation fail-closed — are tracked for a follow-up -(P2) and are not yet closed. +equality; the `fullText` anchor is re-read from the finished pre-call guardrail +payload, so the match still succeeds after later chain guardrails (the PII and +credential maskers, priorities 10/95) rewrite the description text in place and +after system-prompt/handoff/memory injection reshapes the message array. The +body sent upstream to the model is unchanged. An observed request also populates +no durable Memory (both request- and response-derived extraction are skipped), +so the model's own reply cannot echo transcript text into Memory. + +Retention surfaces still open, tracked for a follow-up (**P2**, #12430): the raw +pre-guardrail client-request snapshot in the detailed-log artifact; +`previous_response_id` continuation fail-closed; derived-prompt internal +dispatches that embed the transcript inside a synthesized string prompt +(pipeline stages, context-handoff); and the response body / semantic-cache copy +of a model reply that quotes the transcript. These are raw/response-class or +opt-in surfaces outside P1's persisted-request-body + Memory scope. The internal `/api/modality-bridge/video/drilldown` lifecycle is a separate, loopback/token-authenticated cache substrate. Every operation also requires a diff --git a/src/lib/guardrails/videoBridge.ts b/src/lib/guardrails/videoBridge.ts index f8425a188c..216c525365 100644 --- a/src/lib/guardrails/videoBridge.ts +++ b/src/lib/guardrails/videoBridge.ts @@ -66,6 +66,38 @@ export interface VideoBridgeLogRedactionEntry { redactedText: string; } +/** + * #12150 P1 final-review fix: re-anchor each redaction entry's `fullText` from + * the FINAL pre-call guardrail payload. The video-bridge guardrail runs at + * priority 7, but the PII masker (10) and credential masker (95) rewrite the + * SAME chained payload afterward, in place — so by the end of the chain the + * replaced part's text may differ from what video-bridge recorded, and the log + * sink's content-match (`part.text === fullText`) would miss (fail open). Chain + * guardrails only rewrite text in place — they never splice the message array — + * so the advisory `(container, messageIndex, partIndex)` still resolves inside + * the finished chain payload; reading the part text there yields the true + * post-chain text the log sink will see. Falls back to the original `fullText` + * when the index no longer resolves. Returns new entries; never mutates the + * shared guardrail `meta` array. `redactedText` is unchanged (it is rendered + * from the structured cues, independent of any masker rewrite). + */ +export function reanchorVideoBridgeRedaction( + entries: readonly VideoBridgeLogRedactionEntry[], + finalBody: unknown +): VideoBridgeLogRedactionEntry[] { + const body = finalBody as Record | null | undefined; + return entries.map((entry) => { + const container = body?.[entry.container]; + if (!Array.isArray(container)) return { ...entry }; + const message = container[entry.messageIndex] as { content?: unknown } | undefined; + const content = message?.content; + if (!Array.isArray(content)) return { ...entry }; + const part = content[entry.partIndex] as { text?: unknown } | undefined; + if (!part || typeof part.text !== "string") return { ...entry }; + return { ...entry, fullText: part.text }; + }); +} + type VideoBridgeBody = { model?: string; messages?: Array<{ role?: string; content?: unknown }>; diff --git a/src/sse/handlers/chat.ts b/src/sse/handlers/chat.ts index 7747bc00c1..6c426bcfce 100644 --- a/src/sse/handlers/chat.ts +++ b/src/sse/handlers/chat.ts @@ -104,6 +104,7 @@ import { } from "./chatHelpers"; import { buildModalityBridgeHeader } from "@/lib/guardrails/modalityBridge/bridgeStats"; import type { VideoBridgeLogRedactionEntry } from "@/lib/guardrails/videoBridge"; +import { reanchorVideoBridgeRedaction } from "@/lib/guardrails/videoBridge"; import { resolveConversationId } from "@omniroute/open-sse/services/conversationTracker.ts"; import { classifyProviderBreakerResult, @@ -315,11 +316,18 @@ type VideoBridgeLog = { observed: boolean; redaction: VideoBridgeLogRedactionEnt /** * #12150 P1b: derive the video-bridge log/Memory shadow from - * preCallGuardrails.results. Returns undefined when the video-bridge - * guardrail did not run (disabled, no video parts) or ran but rendered no - * transcript cue (ordinary video, or the request was blocked/failed before - * meta was set) — so every non-video request threads `undefined` through the - * dispatch chain, byte-identical to before this param existed. + * preCallGuardrails.results. Returns undefined only when the video-bridge + * guardrail did not run (disabled, no video parts, or the request was + * blocked/failed before meta was set); a replaced ordinary video returns + * `{ observed: false, redaction: [] }`. So every non-video request threads + * `undefined` through the dispatch chain, byte-identical to before this param + * existed. + * + * `finalBody` is the payload AFTER the whole pre-call chain + * (`preCallGuardrails.payload`): #12150 P1 final-review fix re-anchors each + * redaction entry's `fullText` from it so the log sink's content-match still + * finds the part after the PII/credential maskers (priorities 10/95) rewrote + * the description text in place. * * `results` is typed as a structural subset of GuardrailExecutionResult * (src/lib/guardrails/base.ts), the same "no type dependency on the @@ -327,14 +335,16 @@ type VideoBridgeLog = { observed: boolean; redaction: VideoBridgeLogRedactionEnt * (modalityBridge/bridgeStats.ts). */ function deriveVideoBridgeLog( - results: Array<{ guardrail: string; meta?: Record | null }> + results: Array<{ guardrail: string; meta?: Record | null }>, + finalBody: unknown ): VideoBridgeLog | undefined { const entry = results.find((r) => r.guardrail === "video-bridge"); const meta = entry?.meta; if (!meta || typeof meta.videoBridgeObserved !== "boolean") return undefined; - const redaction = Array.isArray(meta.videoBridgeLogRedaction) + const rawRedaction = Array.isArray(meta.videoBridgeLogRedaction) ? (meta.videoBridgeLogRedaction as VideoBridgeLogRedactionEntry[]) : []; + const redaction = reanchorVideoBridgeRedaction(rawRedaction, finalBody); return { observed: meta.videoBridgeObserved, redaction }; } @@ -774,7 +784,7 @@ async function handleChatImplementation( // #12150 P1b: video-bridge log/Memory shadow — undefined on every // non-video request. Threaded through handleSingleModelChat's // runtimeOptions -> executeChatWithBreaker -> handleChatCore. - const videoBridgeLog = deriveVideoBridgeLog(preCallGuardrails.results); + const videoBridgeLog = deriveVideoBridgeLog(preCallGuardrails.results, body); telemetry.endPhase(); // Agentic conversation tracking (X-ConversationId): resolved once per diff --git a/tests/unit/guardrails/videoBridge.test.ts b/tests/unit/guardrails/videoBridge.test.ts index fa62495eba..0aaafbffe6 100644 --- a/tests/unit/guardrails/videoBridge.test.ts +++ b/tests/unit/guardrails/videoBridge.test.ts @@ -1,7 +1,10 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { VideoBridgeGuardrail } from "../../../src/lib/guardrails/videoBridge.ts"; +import { + VideoBridgeGuardrail, + reanchorVideoBridgeRedaction, +} from "../../../src/lib/guardrails/videoBridge.ts"; import { callVisionModel } from "../../../src/lib/guardrails/visionBridgeHelpers.ts"; import { buildModalityBridgeHeader, @@ -821,3 +824,59 @@ test("audio/video fusion telemetry reaches guardrail meta, bridge stats, and cac assert.equal(after.fusionRuns - before.fusionRuns, 2); assert.equal(after.fusionPartials - before.fusionPartials, 2); }); + +test("reanchorVideoBridgeRedaction re-reads fullText from the post-guardrail body (PII/credential masker interaction)", () => { + // A later chain guardrail (PII masker @10, credential masker @95) rewrote the + // description text IN PLACE after video-bridge@7 built the redaction map, so + // the map's fullText is stale. Re-anchoring at the advisory indices must pick + // up the post-masker text so the log sink's content-match still finds the part. + const entries = [ + { + container: "messages" as const, + messageIndex: 0, + partIndex: 1, + fullText: + "[Video description: transcript[source=client;confidence=0.90;interval=00:01.000-00:02.000] my name is Alice]", + redactedText: + "[Video description: transcript[source=client;confidence=0.90;interval=00:01.000-00:02.000] [redacted-video-transcript]]", + }, + ]; + const finalBody = { + messages: [ + { + role: "user", + content: [ + { type: "text", text: "hello" }, + { + type: "text", + // PII masker replaced "Alice" with a token in place: + text: "[Video description: transcript[source=client;confidence=0.90;interval=00:01.000-00:02.000] my name is [NAME_1]]", + }, + ], + }, + ], + }; + const reanchored = reanchorVideoBridgeRedaction(entries, finalBody); + assert.equal( + reanchored[0].fullText, + (finalBody.messages[0].content[1] as { text: string }).text, + "fullText must equal the post-masker part text so the sink match succeeds" + ); + assert.equal(reanchored[0].redactedText, entries[0].redactedText, "redactedText is unchanged"); + // Original entries object is not mutated (meta is shared). + assert.equal(entries[0].fullText.includes("Alice"), true); +}); + +test("reanchorVideoBridgeRedaction keeps original fullText when the advisory index no longer resolves", () => { + const entries = [ + { + container: "messages" as const, + messageIndex: 5, + partIndex: 9, + fullText: "[Video description: original]", + redactedText: "[Video description: [redacted-video-transcript]]", + }, + ]; + const reanchored = reanchorVideoBridgeRedaction(entries, { messages: [] }); + assert.equal(reanchored[0].fullText, "[Video description: original]"); +});