From a567e8d86a07d991f206e81a41e541b24a5046ea Mon Sep 17 00:00:00 2001 From: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Date: Wed, 2 Sep 2026 08:35:07 -0300 Subject: [PATCH] fix(security): redact dashboard failure events --- .../dashboard-request-failed-redaction.md | 1 + open-sse/handlers/chatCore/attemptLogging.ts | 5 +- ...ashboard-request-failed-redaction-probe.ts | 125 +++++++++++++++++ ...dashboard-request-failed-redaction.test.ts | 129 ++++++++++++++++++ 4 files changed, 259 insertions(+), 1 deletion(-) create mode 100644 changelog.d/fixes/dashboard-request-failed-redaction.md create mode 100644 tests/fixtures/dashboard-request-failed-redaction-probe.ts create mode 100644 tests/unit/dashboard-request-failed-redaction.test.ts diff --git a/changelog.d/fixes/dashboard-request-failed-redaction.md b/changelog.d/fixes/dashboard-request-failed-redaction.md new file mode 100644 index 0000000000..a9efc53dc7 --- /dev/null +++ b/changelog.d/fixes/dashboard-request-failed-redaction.md @@ -0,0 +1 @@ +- **fix(security):** sanitize `request.failed` diagnostics before publishing them to live dashboard listeners and replay history, while keeping status, model, provider, latency, and internal call-log diagnostics intact. diff --git a/open-sse/handlers/chatCore/attemptLogging.ts b/open-sse/handlers/chatCore/attemptLogging.ts index 25d8033480..e4df6a3054 100644 --- a/open-sse/handlers/chatCore/attemptLogging.ts +++ b/open-sse/handlers/chatCore/attemptLogging.ts @@ -17,6 +17,7 @@ import type { RequestCompletedPayload, RequestFailedPayload } from "@/lib/events import { saveCallLog } from "@/lib/usageDb"; import { FORMATS } from "../../translator/formats.ts"; import { takeEarlyKeepaliveBytes } from "../../utils/earlyKeepaliveByteBuffer.ts"; +import { sanitizeErrorMessage } from "../../utils/error.ts"; import { cloneBoundedChatLogPayload, truncateForLog } from "./logTruncation.ts"; import { attachLogMeta } from "./cacheUsageMeta.ts"; @@ -157,7 +158,9 @@ export function resolveRequestLifecycleEvent(input: { name: "request.failed", payload: { id: traceId, - error: error || `HTTP ${status}`, + // Dashboard listeners and event history cross a public WebSocket boundary. Keep the raw + // diagnostic in the call log/pipeline above, but expose only the canonical safe projection. + error: sanitizeErrorMessage(error || `HTTP ${status}`), statusCode: typeof status === "number" ? status : undefined, latencyMs, model: model || undefined, diff --git a/tests/fixtures/dashboard-request-failed-redaction-probe.ts b/tests/fixtures/dashboard-request-failed-redaction-probe.ts new file mode 100644 index 0000000000..bf740c2220 --- /dev/null +++ b/tests/fixtures/dashboard-request-failed-redaction-probe.ts @@ -0,0 +1,125 @@ +import assert from "node:assert/strict"; + +import type { RequestFailedPayload } from "../../src/lib/events/types.ts"; + +const RESULT_PREFIX = "DASHBOARD_FAILURE_PROBE_RESULT="; + +async function main(): Promise { + assert.ok(process.env.DATA_DIR, "probe requires an isolated DATA_DIR"); + assert.ok(process.env.OMNIROUTE_PLUGINS_DIR, "probe requires an isolated plugins directory"); + assert.ok(process.env.API_KEY_SECRET, "probe requires a synthetic API_KEY_SECRET"); + + const { persistAttemptLogs } = await import("../../open-sse/handlers/chatCore/attemptLogging.ts"); + const eventBus = await import("../../src/lib/events/eventBus.ts"); + const dbCore = await import("../../src/lib/db/core.ts"); + const callLogs = await import("../../src/lib/usage/callLogs.ts"); + + let unsubscribe: (() => void) | undefined; + try { + globalThis.__omnirouteEventBus = undefined; + const hostileError = new Error( + "Provider failed in /srv/omniroute/src/private/provider.ts:42:7 with " + + "api_key='sk-live-dashboard-secret'" + ); + hostileError.stack = + `${hostileError.name}: ${hostileError.message}\n` + + " at dispatch (/srv/omniroute/src/private/transport.ts:91:3)"; + const rawDiagnostic = hostileError.stack; + const traceId = "trace-dashboard-redaction"; + const callLogId = "call-log-dashboard-redaction"; + + const deliveredPromise = new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + unsubscribe?.(); + reject(new Error("timed out waiting for persistAttemptLogs request.failed event")); + }, 10_000); + unsubscribe = eventBus.on("request.failed", (payload) => { + if (payload.id !== traceId) return; + clearTimeout(timeout); + unsubscribe?.(); + unsubscribe = undefined; + resolve(payload); + }); + }); + + persistAttemptLogs( + { + status: 502, + tokens: {}, + responseBody: null, + error: rawDiagnostic, + }, + { + traceId, + provider: "private-provider", + connectionId: null, + model: "private-model", + skillRequestId: "skill-dashboard-redaction", + detailedLoggingEnabled: false, + reqLogger: null, + pendingRequestId: callLogId, + clientRawRequest: { endpoint: "/v1/chat/completions" }, + requestedModel: "private-model", + credentials: null, + startTime: Date.now() - 37, + body: { model: "private-model", messages: [] }, + sourceFormat: "openai", + targetFormat: "openai", + comboName: null, + comboStepId: null, + comboExecutionKey: null, + tokensCompressed: null, + apiKeyInfo: null, + noLogEnabled: false, + correlationId: null, + modelPinned: false, + sessionTag: null, + } + ); + + const delivered = await deliveredPromise; + assert.equal(delivered.id, traceId); + assert.equal(delivered.statusCode, 502); + assert.equal(delivered.model, "private-model"); + assert.equal(delivered.provider, "private-provider"); + assert.ok(delivered.latencyMs >= 0); + assert.equal(delivered.error, "Error: Provider failed in with api_key='[REDACTED]'"); + assert.doesNotMatch(delivered.error, /sk-live-dashboard-secret|\/srv\/omniroute|\n/); + + const replayed = eventBus + .getEventHistory(undefined, 10) + .find( + (entry) => + entry.event === "request.failed" && + (entry.payload as RequestFailedPayload | undefined)?.id === traceId + ); + assert.ok(replayed, "late subscribers must have the safe request.failed history entry"); + assert.deepEqual(replayed.payload, delivered); + + const writerDrained = await callLogs.waitForCallLogSaves(10_000); + assert.equal(writerDrained, true, "call-log write must drain"); + const persisted = await callLogs.getCallLogById(callLogId); + assert.ok(persisted, "failed attempt must still be available to internal diagnostics"); + assert.equal(persisted.error, rawDiagnostic); + + console.log( + RESULT_PREFIX + + JSON.stringify({ + delivered, + replayMatches: JSON.stringify(replayed.payload) === JSON.stringify(delivered), + internalRawPreserved: persisted.error === rawDiagnostic, + writerDrained, + }) + ); + } finally { + unsubscribe?.(); + try { + await callLogs.waitForCallLogSaves(10_000); + await callLogs.closeCallLogSaves(10_000); + } finally { + dbCore.resetDbInstance(); + } + } +} + +await main(); diff --git a/tests/unit/dashboard-request-failed-redaction.test.ts b/tests/unit/dashboard-request-failed-redaction.test.ts new file mode 100644 index 0000000000..e039f7c0dc --- /dev/null +++ b/tests/unit/dashboard-request-failed-redaction.test.ts @@ -0,0 +1,129 @@ +import assert from "node:assert/strict"; +import { execFile } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { test } from "node:test"; +import { fileURLToPath } from "node:url"; + +import type { RequestFailedPayload } from "../../src/lib/events/types.ts"; + +const RESULT_PREFIX = "DASHBOARD_FAILURE_PROBE_RESULT="; +const repoRoot = fileURLToPath(new URL("../../", import.meta.url)); +const probePath = fileURLToPath( + new URL("../fixtures/dashboard-request-failed-redaction-probe.ts", import.meta.url) +); + +type ProbeResult = { + delivered: RequestFailedPayload; + replayMatches: boolean; + internalRawPreserved: boolean; + writerDrained: boolean; +}; + +function runProbe(env: NodeJS.ProcessEnv): Promise<{ stdout: string; stderr: string }> { + return new Promise((resolve, reject) => { + execFile( + process.execPath, + ["--import", "tsx/esm", probePath], + { + cwd: repoRoot, + env, + timeout: 30_000, + maxBuffer: 8 * 1024 * 1024, + }, + (error, stdout, stderr) => { + if (error) { + reject( + new Error( + `dashboard failure probe exited unsuccessfully: ${error.message}\n` + + `stdout:\n${stdout}\nstderr:\n${stderr}` + ) + ); + return; + } + resolve({ stdout, stderr }); + } + ); + }); +} + +test("persistAttemptLogs redacts request.failed delivery/replay but keeps its internal log", async () => { + const isolationRoot = fs.mkdtempSync( + path.join(os.tmpdir(), "omniroute-dashboard-failure-redaction-") + ); + const dataDir = path.join(isolationRoot, "data"); + const pluginsDir = path.join(isolationRoot, "plugins"); + fs.mkdirSync(dataDir, { recursive: true }); + fs.mkdirSync(pluginsDir, { recursive: true }); + + try { + // The subprocess receives only process/runtime basics plus synthetic OmniRoute settings: no + // provider credentials are inherited and no parent singleton/env/global state is mutated. + const { stdout, stderr } = await runProbe({ + PATH: process.env.PATH, + NODE_PATH: process.env.NODE_PATH, + LANG: process.env.LANG, + LC_ALL: process.env.LC_ALL, + TZ: process.env.TZ, + TMPDIR: process.env.TMPDIR, + NODE_ENV: "test", + DATA_DIR: dataDir, + OMNIROUTE_PLUGINS_DIR: pluginsDir, + API_KEY_SECRET: "test-dashboard-failure-redaction-secret", + PII_RESPONSE_SANITIZATION: "false", + OMNIROUTE_ENABLE_LIVE_WS: "0", + }); + + assert.doesNotMatch(stderr, /sk-live-dashboard-secret|\/srv\/omniroute/); + const resultLine = stdout.split(/\r?\n/).find((line) => line.startsWith(RESULT_PREFIX)); + assert.ok(resultLine, `probe did not emit its result marker; stdout:\n${stdout}`); + const result = JSON.parse(resultLine.slice(RESULT_PREFIX.length)) as ProbeResult; + + assert.equal(result.delivered.id, "trace-dashboard-redaction"); + assert.equal(result.delivered.statusCode, 502); + assert.equal(result.delivered.model, "private-model"); + assert.equal(result.delivered.provider, "private-provider"); + assert.equal( + result.delivered.error, + "Error: Provider failed in with api_key='[REDACTED]'" + ); + assert.equal(result.replayMatches, true); + assert.equal(result.internalRawPreserved, true); + assert.equal(result.writerDrained, true); + } finally { + // The probe exits only after draining/closing its writer and resetting its DB singleton. + fs.rmSync(isolationRoot, { + recursive: true, + force: true, + maxRetries: 5, + retryDelay: 100, + }); + } +}); + +test("the private LiveWS bridge forwards the already-safe event into its backlog unchanged", () => { + const source = fs.readFileSync( + fileURLToPath(new URL("../../src/server/ws/liveServer.ts", import.meta.url)), + "utf8" + ); + + // publishDashboardEvent/eventHistoryBacklog are module-private. This bounded source-chain + // assertion avoids opening a server while proving the bus payload is what live delivery and + // welcome/backlog replay store. The behavioral safety assertion lives in the subprocess above. + assert.match( + source, + /eventHistoryBacklog\.push\(\{ event, payload, timestamp \}\)/, + "the LiveWS backlog must store the event-bus payload" + ); + assert.match( + source, + /data:\s*h\.payload/, + "welcome replay must forward the stored backlog payload" + ); + assert.match( + source, + /onAny\(\(event:[^\n]+payload:[^\n]+\)\s*=>\s*\{\s*publishDashboardEvent\(event, payload\)/, + "the LiveWS bridge must publish the same event-bus payload" + ); +});