diff --git a/changelog.d/fixes/13488-pii-sanitizer-splices-openrouter-metadata.md b/changelog.d/fixes/13488-pii-sanitizer-splices-openrouter-metadata.md new file mode 100644 index 0000000000..2b270f4adf --- /dev/null +++ b/changelog.d/fixes/13488-pii-sanitizer-splices-openrouter-metadata.md @@ -0,0 +1 @@ +- **fix(sse):** stop the streaming PII sanitizer from splicing OpenRouter metadata (`provider`, `native_finish_reason`, `reasoning_details[].format`) into the answer text buffer (#13488) — thanks @Xore diff --git a/src/lib/sseTextTransform.ts b/src/lib/sseTextTransform.ts index d33ed8d6b5..d9d0275980 100644 --- a/src/lib/sseTextTransform.ts +++ b/src/lib/sseTextTransform.ts @@ -1,6 +1,7 @@ export type FieldCategory = "content" | "reasoning" | "toolArgs" | "partialJson"; -const CATEGORY_MAP: Record = { +// Keys that always map to a fixed category, regardless of where they appear in the chunk. +const FIXED_CATEGORY_MAP: Record = { reasoning: "reasoning", thinking: "reasoning", reasoning_content: "reasoning", @@ -8,8 +9,69 @@ const CATEGORY_MAP: Record = { partial_json: "partialJson", }; +// System/protocol metadata keys that must never be routed through the PII processor or +// buffer, no matter which JSON shape they appear in. Shared by the real-time sanitizeObject +// pass (below) and streamingPiiTransform.ts's onFlush generic-fallback branch — the two +// copies of this list had drifted (see issue #13488): neither one listed `provider`, +// `native_finish_reason`, or the `reasoning_details[].format` field, so those OpenRouter +// metadata strings fell through to the default "content" category and got spliced into the +// same sliding-window buffer as the actual answer text. +export const METADATA_KEYS = new Set([ + "id", + "model", + "object", + "created", + "finish_reason", + "finishReason", + "native_finish_reason", + "role", + "type", + "index", + "stop_reason", + "stop_sequence", + "system_fingerprint", + "service_tier", + "usage", + "prompt_tokens", + "completion_tokens", + "total_tokens", + "input_tokens", + "output_tokens", + "logprobs", + "refusal", + "name", + "event", + "provider", + "format", +]); + +/** + * Classify a string field as a PII-processed category, or `null` when it is metadata that + * must pass through untouched. `parentKey` is the key of the object that directly contains + * `key` (empty string at the JSON root) — it disambiguates `text`, which means the answer + * everywhere except inside a `reasoning_details[]` item, where it is reasoning text (and + * `format` alongside it is metadata, not content, even though it is not disambiguated by + * parent elsewhere). Any string field not explicitly recognized as metadata defaults to + * "content" — this keeps non-standard/unrecognized stream shapes (arbitrary JSON keys that + * match none of the known provider formats) from silently losing their text. + */ +export function classifyField(key: string, parentKey = ""): FieldCategory | null { + if (FIXED_CATEGORY_MAP[key]) { + return FIXED_CATEGORY_MAP[key]; + } + if (key === "text" && parentKey === "reasoning_details") { + return "reasoning"; + } + if (METADATA_KEYS.has(key)) { + return null; + } + return "content"; +} + +// Back-compat helper kept for any external caller expecting a category rather than `null` +// for metadata; internal call sites use `classifyField` so they can skip metadata entirely. export function getFieldCategory(key: string): FieldCategory { - return CATEGORY_MAP[key] || "content"; + return classifyField(key) ?? "content"; } const STOP_EVENT_TYPES = new Set([ @@ -123,34 +185,18 @@ export function createSseTextTransform( const isStopSignal = checkIfStopSignal(json); const isSnapshot = checkIfSnapshot(json); - const METADATA_KEYS = [ - "id", - "model", - "object", - "created", - "finish_reason", - "finishReason", - "role", - "type", - "index", - "stop_reason", - "stop_sequence", - "system_fingerprint", - "service_tier", - "usage", - "prompt_tokens", - "completion_tokens", - "total_tokens", - "input_tokens", - "output_tokens", - "logprobs", - "refusal", - "name", - "event", - ]; - - // Recursively sanitize all string properties (except system metadata) - const sanitizeObject = (obj: any, currentChoiceIdx = 0, currentToolIdx = 0) => { + // Recursively sanitize string properties, skipping recognized system metadata + // (`classifyField` returns null for METADATA_KEYS — `provider`, + // `native_finish_reason`, `reasoning_details[].format`, etc.). `parentKey` is the + // key of the enclosing object (unchanged across array-index recursion) so + // `classifyField` can tell `reasoning_details[].text` apart from ordinary content + // text. See issue #13488. + const sanitizeObject = ( + obj: any, + currentChoiceIdx = 0, + currentToolIdx = 0, + parentKey = "" + ) => { if (!obj || typeof obj !== "object") return; let choiceIdx = currentChoiceIdx; @@ -167,14 +213,15 @@ export function createSseTextTransform( } const compositeKey = `${choiceIdx}_${toolIdx}`; + const isArray = Array.isArray(obj); for (const key of Object.keys(obj)) { - if (METADATA_KEYS.includes(key)) { - continue; - } if (typeof obj[key] === "string") { const val = obj[key]; - const field: FieldCategory = getFieldCategory(key); + const field = classifyField(key, parentKey); + if (field === null) { + continue; + } if (field === "toolArgs" || field === "partialJson") { obj[key] = val; matched = true; @@ -183,7 +230,7 @@ export function createSseTextTransform( obj[key] = processor(val, field, isStopSignal, compositeKey, isSnapshot); matched = true; } else if (typeof obj[key] === "object") { - sanitizeObject(obj[key], choiceIdx, toolIdx); + sanitizeObject(obj[key], choiceIdx, toolIdx, isArray ? parentKey : key); } } }; diff --git a/src/lib/streamingPiiTransform.ts b/src/lib/streamingPiiTransform.ts index a26eb3fa2e..72018bb975 100644 --- a/src/lib/streamingPiiTransform.ts +++ b/src/lib/streamingPiiTransform.ts @@ -1,4 +1,4 @@ -import { createSseTextTransform, FieldCategory, getFieldCategory } from "./sseTextTransform"; +import { createSseTextTransform, FieldCategory, classifyField } from "./sseTextTransform"; import { sanitizePII } from "./piiSanitizer"; export interface PiiTransformOptions { @@ -116,33 +116,6 @@ export function createPiiSseTransform(options?: PiiTransformOptions): TransformS return null; } - // Explicitly target formats to prevent metadata corruption and leakage - const METADATA_KEYS = [ - "id", - "model", - "object", - "created", - "finish_reason", - "finishReason", - "role", - "type", - "index", - "stop_reason", - "stop_sequence", - "system_fingerprint", - "service_tier", - "usage", - "prompt_tokens", - "completion_tokens", - "total_tokens", - "input_tokens", - "output_tokens", - "logprobs", - "refusal", - "name", - "event", - ]; - // 1. Claude format if ( typeof lastJson.type === "string" && @@ -283,22 +256,31 @@ export function createPiiSseTransform(options?: PiiTransformOptions): TransformS // 5. Generic fallback const templateJson = lastContentJson || lastJson; const finalJson = JSON.parse(JSON.stringify(templateJson)); - const clearDeltas = (obj: any) => { + // Skip recognized system metadata (same `classifyField`/METADATA_KEYS as sanitizeObject + // in sseTextTransform.ts) — fields like `provider` or `native_finish_reason` must never + // be cleared to "" or refilled with buffered answer text here either. See #13488. + const clearDeltas = (obj: any, parentKey = "") => { if (!obj || typeof obj !== "object") return; + const isArray = Array.isArray(obj); for (const key of Object.keys(obj)) { - if (METADATA_KEYS.includes(key)) { - continue; - } if (typeof obj[key] === "string") { + if (classifyField(key, parentKey) === null) { + continue; + } obj[key] = ""; } else if (typeof obj[key] === "object") { - clearDeltas(obj[key]); + clearDeltas(obj[key], isArray ? parentKey : key); } } }; clearDeltas(finalJson); - const populateRemaining = (obj: any, currentChoiceIdx = 0, currentToolIdx = 0) => { + const populateRemaining = ( + obj: any, + currentChoiceIdx = 0, + currentToolIdx = 0, + parentKey = "" + ) => { if (!obj || typeof obj !== "object") return; let choiceIdx = currentChoiceIdx; @@ -315,20 +297,21 @@ export function createPiiSseTransform(options?: PiiTransformOptions): TransformS } const compositeKey = `${choiceIdx}_${toolIdx}`; + const isArray = Array.isArray(obj); for (const key of Object.keys(obj)) { - if (METADATA_KEYS.includes(key)) { - continue; - } if (typeof obj[key] === "string") { - const field: FieldCategory = getFieldCategory(key); + const field = classifyField(key, parentKey); + if (field === null) { + continue; + } const choiceBuf = getBuffers(compositeKey); if (choiceBuf[field]) { obj[key] = (obj[key] || "") + choiceBuf[field]; choiceBuf[field] = ""; } } else if (typeof obj[key] === "object") { - populateRemaining(obj[key], choiceIdx, toolIdx); + populateRemaining(obj[key], choiceIdx, toolIdx, isArray ? parentKey : key); } } }; diff --git a/tests/unit/issue-13488-pii-openrouter-metadata-splice.test.ts b/tests/unit/issue-13488-pii-openrouter-metadata-splice.test.ts new file mode 100644 index 0000000000..6f9de4064a --- /dev/null +++ b/tests/unit/issue-13488-pii-openrouter-metadata-splice.test.ts @@ -0,0 +1,227 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +// Isolate DB state +const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-test-issue-13488-")); +process.env.DATA_DIR = tmpDir; + +// Enable the feature flag for tests (mode "warn" per the issue repro steps — "nothing is +// supposed to be modified", yet the windowed re-emission still runs and scrambles output). +const originalEnv = process.env.PII_RESPONSE_SANITIZATION; +const originalMode = process.env.PII_RESPONSE_SANITIZATION_MODE; +process.env.PII_RESPONSE_SANITIZATION = "true"; +process.env.PII_RESPONSE_SANITIZATION_MODE = "warn"; +process.env.PII_TEST_BYPASS_MIN_WINDOW = "true"; + +import { createPiiSseTransform } from "../../src/lib/streamingPiiTransform.ts"; + +async function testTransform(transform: TransformStream, inputChunks: string[]): Promise { + const writer = transform.writable.getWriter(); + const reader = transform.readable.getReader(); + + const writePromise = (async () => { + for (const chunk of inputChunks) { + await writer.write(new TextEncoder().encode(chunk)); + } + await writer.close(); + })(); + + const outputChunks: string[] = []; + let res = await reader.read(); + while (!res.done) { + outputChunks.push(new TextDecoder().decode(res.value)); + res = await reader.read(); + } + + await writePromise; + return outputChunks.join(""); +} + +function extractContentAndMetadata(output: string, metadataKey: string) { + const dataLines = output + .split("\n") + .filter((l) => l.startsWith("data: ") && l !== "data: [DONE]"); + + let reassembledContent = ""; + const metadataValues = new Set(); + for (const line of dataLines) { + const json = JSON.parse(line.slice("data: ".length)); + const delta = json.choices?.[0]?.delta; + if (delta?.content) reassembledContent += delta.content; + if (typeof json[metadataKey] === "string") metadataValues.add(json[metadataKey]); + } + return { reassembledContent, metadataValues }; +} + +// Reproduces the exact shape from the issue: OpenRouter SSE chunks carry a top-level +// "provider" string and delta.reasoning_details[].text, alongside delta.content — all on +// the same choice. windowSize is kept small (5) to force windowed re-emission on every +// chunk, matching the reporter's observation that scrambling happens even in mode=warn. +test("issue #13488: OpenRouter top-level `provider` field must not share the content buffer", async () => { + const transform = createPiiSseTransform({ windowSize: 5 }); + + const makeChunk = (provider: string, content: string, reasoningText = "") => + `data: ${JSON.stringify({ + id: "gen-1", + model: "z-ai/glm-5.3-flash", + provider, + choices: [ + { + index: 0, + delta: { + content, + role: "assistant", + reasoning: "", + reasoning_details: reasoningText + ? [{ type: "reasoning.text", text: reasoningText, format: "" }] + : [], + }, + }, + ], + })}\n\n`; + + const chunks = [ + makeChunk("Together", "Lake"), + makeChunk("Together", "Saimaa "), + makeChunk("Together", "is the largest "), + makeChunk("Together", "lake in Finland."), + ]; + const done = `data: [DONE]\n\n`; + + const output = await testTransform(transform, [...chunks, done]); + + const { reassembledContent, metadataValues: providerValues } = extractContentAndMetadata( + output, + "provider" + ); + + const expectedContent = "LakeSaimaa is the largest lake in Finland."; + + assert.equal( + reassembledContent, + expectedContent, + `content must reassemble byte-identical to input even with PII sanitization enabled; ` + + `got ${JSON.stringify(reassembledContent)}` + ); + + assert.deepEqual( + [...providerValues], + ["Together"], + `the "provider" field must stay constant across every chunk (metadata, not answer text); ` + + `got ${JSON.stringify([...providerValues])}` + ); + + // None of the answer text should ever have leaked into the provider field. + for (const p of providerValues) { + assert.ok( + !/Lake|Saimaa|largest|Finland/.test(p), + `provider field must never contain spliced-in answer text; got "${p}"` + ); + } +}); + +// Same shape but with mode=redact, per the issue's Validation Plan — the splice bug must +// also be gone when redaction (not just pass-through warn mode) is active. +test("issue #13488: mode=redact must not splice `provider` and `content` either", async () => { + const originalModeLocal = process.env.PII_RESPONSE_SANITIZATION_MODE; + process.env.PII_RESPONSE_SANITIZATION_MODE = "redact"; + try { + const transform = createPiiSseTransform({ windowSize: 5 }); + + const makeChunk = (provider: string, content: string) => + `data: ${JSON.stringify({ + id: "gen-2", + model: "z-ai/glm-5.3-flash", + provider, + choices: [{ index: 0, delta: { content, role: "assistant" } }], + })}\n\n`; + + const chunks = [ + makeChunk("Together", "The "), + makeChunk("Together", "capital "), + makeChunk("Together", "of France "), + makeChunk("Together", "is Paris."), + ]; + const done = `data: [DONE]\n\n`; + + const output = await testTransform(transform, [...chunks, done]); + const { reassembledContent, metadataValues: providerValues } = extractContentAndMetadata( + output, + "provider" + ); + + assert.equal( + reassembledContent, + "The capital of France is Paris.", + `content must reassemble byte-identical even under redact mode; got ${JSON.stringify(reassembledContent)}` + ); + assert.deepEqual( + [...providerValues], + ["Together"], + `provider must stay constant under redact mode; got ${JSON.stringify([...providerValues])}` + ); + } finally { + if (originalModeLocal !== undefined) { + process.env.PII_RESPONSE_SANITIZATION_MODE = originalModeLocal; + } else { + delete process.env.PII_RESPONSE_SANITIZATION_MODE; + } + } +}); + +// A second concurrently-streamed metadata field (native_finish_reason) alongside `provider` +// and `content` — closes the family of "any recognized metadata field shares the buffer", +// not just the single field named in the report. +test("issue #13488: a second metadata field (native_finish_reason) must not share buffers either", async () => { + const transform = createPiiSseTransform({ windowSize: 5 }); + + const makeChunk = (provider: string, finishReason: string, content: string) => + `data: ${JSON.stringify({ + id: "gen-3", + model: "z-ai/glm-5.3-flash", + provider, + native_finish_reason: finishReason, + choices: [{ index: 0, delta: { content, role: "assistant" } }], + })}\n\n`; + + const chunks = [ + makeChunk("Together", "in_progress", "Hello "), + makeChunk("Together", "in_progress", "there, "), + makeChunk("Together", "in_progress", "world!"), + ]; + const done = `data: [DONE]\n\n`; + + const output = await testTransform(transform, [...chunks, done]); + const { reassembledContent, metadataValues: providerValues } = extractContentAndMetadata( + output, + "provider" + ); + const { metadataValues: finishReasonValues } = extractContentAndMetadata( + output, + "native_finish_reason" + ); + + assert.equal(reassembledContent, "Hello there, world!"); + assert.deepEqual([...providerValues], ["Together"]); + assert.deepEqual([...finishReasonValues], ["in_progress"]); +}); + +test.after(async () => { + if (originalEnv !== undefined) { + process.env.PII_RESPONSE_SANITIZATION = originalEnv; + } else { + delete process.env.PII_RESPONSE_SANITIZATION; + } + if (originalMode !== undefined) { + process.env.PII_RESPONSE_SANITIZATION_MODE = originalMode; + } else { + delete process.env.PII_RESPONSE_SANITIZATION_MODE; + } + + const coreDb = await import("../../src/lib/db/core.ts"); + coreDb.resetDbInstance(); + fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +});