From 21debc0819ef90eccb3ff6be3b1596020962ba6e Mon Sep 17 00:00:00 2001 From: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Date: Thu, 3 Sep 2026 07:47:37 -0300 Subject: [PATCH] refactor(video): extract the guarded client-snapshot log call to keep chatCore within its size budget Fast Quality Gates check:file-size flagged chatCore.ts growing past its frozen ceiling (5985 > 5976) from the P2a wiring. Move the guarded logClientRawRequest call into logClientRawRequestRedacted (new export in videoBridgeSnapshotRedaction.ts, which already owns the redaction), collapsing the inline if-block at the chatCore.ts call site to a single call. Net -4 lines vs the pre-P2a base. Behavior unchanged: non-observed still logs the exact same clientRawRequest.body reference; observed still logs the redacted clone. --- open-sse/handlers/chatCore.ts | 21 ++-------- .../videoBridgeSnapshotRedaction.ts | 33 ++++++++++++++++ tests/unit/video-bridge-log-redaction.test.ts | 39 ++++++++----------- 3 files changed, 53 insertions(+), 40 deletions(-) diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index 1243074ad5..d28711ce12 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -360,7 +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 { redactVideoTranscriptFieldsForLog } from "@/lib/guardrails/videoBridgeSnapshotRedaction"; +import { logClientRawRequestRedacted } from "@/lib/guardrails/videoBridgeSnapshotRedaction"; import { shouldPreserveCacheControl, resolveConnectionCacheOverride, @@ -1212,22 +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, - // #12150 P2 surface 1 (the dominant transcript-retention leak): this snapshot is - // captured BEFORE the guardrail chain runs, so it still carries the client's raw - // video transcript cue text untouched by the video-bridge guardrail's own - // description redaction. Redact it in this LOGGED copy only when the guardrail - // observed a video part on this request — clientRawRequest.body itself is never - // mutated and keeps flowing to every other consumer unchanged. - videoBridgeObserved - ? redactVideoTranscriptFieldsForLog(clientRawRequest.body) - : 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 index 0af6d5416e..8738b47cfc 100644 --- a/src/lib/guardrails/videoBridgeSnapshotRedaction.ts +++ b/src/lib/guardrails/videoBridgeSnapshotRedaction.ts @@ -100,3 +100,36 @@ export function redactVideoTranscriptFieldsForLog(body: unknown): unknown { } 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/video-bridge-log-redaction.test.ts b/tests/unit/video-bridge-log-redaction.test.ts index 06fbb6f2fc..268102d246 100644 --- a/tests/unit/video-bridge-log-redaction.test.ts +++ b/tests/unit/video-bridge-log-redaction.test.ts @@ -26,7 +26,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { redactVideoTranscriptFieldsForLog } from "../../src/lib/guardrails/videoBridgeSnapshotRedaction.ts"; +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; @@ -275,11 +275,11 @@ test("Scenario A (adversarial review): a message prepended AFTER the guardrail b // `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 ternary a few hundred lines into -// a 5900+ line handler, for no additional proof beyond what's below — so this exercises -// the EXACT call-site pattern -// (`videoBridgeObserved ? redactVideoTranscriptFieldsForLog(body) : body`) against a fake -// logClientRawRequest. The pure helper itself has its own thorough suite in +// 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[] = []; @@ -291,21 +291,6 @@ function fakeReqLogger() { }; } -/** Mirrors chatCore.ts's guarded logClientRawRequest call site verbatim. */ -function callLogClientRawRequestAsChatCoreDoes( - reqLogger: ReturnType, - clientRawRequest: { endpoint: string; body: unknown; headers: unknown }, - videoBridgeObserved: boolean -) { - reqLogger.logClientRawRequest( - clientRawRequest.endpoint, - videoBridgeObserved - ? redactVideoTranscriptFieldsForLog(clientRawRequest.body) - : clientRawRequest.body, - clientRawRequest.headers - ); -} - test("surface 2 (raw snapshot): the fake logClientRawRequest receives a redacted snapshot only when videoBridgeObserved is true", () => { const rawBody = { model: "openai/gpt-x", @@ -326,7 +311,7 @@ test("surface 2 (raw snapshot): the fake logClientRawRequest receives a redacted const clientRawRequest = { endpoint: "/v1/chat/completions", body: rawBody, headers: {} }; const observedLogger = fakeReqLogger(); - callLogClientRawRequestAsChatCoreDoes(observedLogger, clientRawRequest, true); + logClientRawRequestRedacted(observedLogger, clientRawRequest, true); const observedSnapshot = observedLogger.calls[0]; assert.ok( !JSON.stringify(observedSnapshot).includes(SECRET), @@ -343,10 +328,18 @@ test("surface 2 (raw snapshot): the fake logClientRawRequest receives a redacted ); const nonObservedLogger = fakeReqLogger(); - callLogClientRawRequestAsChatCoreDoes(nonObservedLogger, clientRawRequest, false); + 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)" + ); });