From caeace8b524913b62c08c2ad326cb6944a693af7 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Wed, 22 Apr 2026 19:13:57 -0400 Subject: [PATCH] fix: harden diagnostics export redaction --- .../diagnostic-stability-bundle.test.ts | 14 +++ src/logging/diagnostic-stability-bundle.ts | 9 +- src/logging/diagnostic-support-export.test.ts | 53 +++++++++ .../diagnostic-support-log-redaction.ts | 14 ++- src/logging/diagnostic-support-redaction.ts | 104 +++++++++++++++--- 5 files changed, 177 insertions(+), 17 deletions(-) diff --git a/src/logging/diagnostic-stability-bundle.test.ts b/src/logging/diagnostic-stability-bundle.test.ts index 432443ae944..5b76aa00586 100644 --- a/src/logging/diagnostic-stability-bundle.test.ts +++ b/src/logging/diagnostic-stability-bundle.test.ts @@ -6,6 +6,7 @@ import { emitDiagnosticEvent, resetDiagnosticEventsForTest } from "../infra/diag import { resetFatalErrorHooksForTest, runFatalErrorHooks } from "../infra/fatal-error-hooks.js"; import { installDiagnosticStabilityFatalHook, + MAX_DIAGNOSTIC_STABILITY_BUNDLE_BYTES, readDiagnosticStabilityBundleFileSync, readLatestDiagnosticStabilityBundleSync, resetDiagnosticStabilityBundleForTest, @@ -218,6 +219,19 @@ describe("diagnostic stability bundles", () => { ); }); + it("rejects oversized bundle files before reading them", () => { + const file = path.join(tempDir, "oversized.json"); + fs.closeSync(fs.openSync(file, "w")); + fs.truncateSync(file, MAX_DIAGNOSTIC_STABILITY_BUNDLE_BYTES + 1); + + const result = readDiagnosticStabilityBundleFileSync(file); + + expect(result.status).toBe("failed"); + expect(result.status === "failed" ? String(result.error) : "").toContain( + "Stability bundle is too large", + ); + }); + it("rejects malformed bundle snapshots before returning them", () => { const baseBundle = { version: 1, diff --git a/src/logging/diagnostic-stability-bundle.ts b/src/logging/diagnostic-stability-bundle.ts index a020d5cd68f..281058a5f57 100644 --- a/src/logging/diagnostic-stability-bundle.ts +++ b/src/logging/diagnostic-stability-bundle.ts @@ -12,6 +12,7 @@ import { export const DIAGNOSTIC_STABILITY_BUNDLE_VERSION = 1; export const DEFAULT_DIAGNOSTIC_STABILITY_BUNDLE_LIMIT = MAX_DIAGNOSTIC_STABILITY_LIMIT; export const DEFAULT_DIAGNOSTIC_STABILITY_BUNDLE_RETENTION = 20; +export const MAX_DIAGNOSTIC_STABILITY_BUNDLE_BYTES = 5 * 1024 * 1024; const SAFE_REASON_CODE = /^[A-Za-z0-9_.:-]{1,120}$/u; const BUNDLE_PREFIX = "openclaw-stability-"; @@ -246,12 +247,18 @@ export function readDiagnosticStabilityBundleFileSync( file: string, ): ReadDiagnosticStabilityBundleResult { try { + const stat = fs.statSync(file); + if (stat.size > MAX_DIAGNOSTIC_STABILITY_BUNDLE_BYTES) { + throw new Error( + `Stability bundle is too large: ${stat.size} bytes exceeds ${MAX_DIAGNOSTIC_STABILITY_BUNDLE_BYTES}`, + ); + } const raw = fs.readFileSync(file, "utf8"); const bundle = parseDiagnosticStabilityBundle(JSON.parse(raw)); return { status: "found", path: file, - mtimeMs: fs.statSync(file).mtimeMs, + mtimeMs: stat.mtimeMs, bundle, }; } catch (error) { diff --git a/src/logging/diagnostic-support-export.test.ts b/src/logging/diagnostic-support-export.test.ts index 843dd8e15c8..01219617409 100644 --- a/src/logging/diagnostic-support-export.test.ts +++ b/src/logging/diagnostic-support-export.test.ts @@ -134,6 +134,8 @@ describe("diagnostic support export", () => { subsystem: "gateway", component: "gateway/server", channel: "telegram", + sessionId: "gateway-session-15555551212", + sessionKey: "matrix:!supportRoom:matrix.example.com:$supportEventSecret", msg: `gateway websocket listening at ${credentialUrl} Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ== ${fakeAwsKey} ${fakeJwt} Cookie: sid=secret`, hostname: "support-host", message: privateChat, @@ -263,6 +265,8 @@ describe("diagnostic support export", () => { expect(combined).not.toContain("QWxhZGRpbjpvcGVuIHNlc2FtZQ=="); expect(combined).not.toContain("sid=secret"); expect(combined).not.toContain("structured secret payload"); + expect(combined).not.toContain("gateway-session-15555551212"); + expect(combined).not.toContain("supportEventSecret"); expect(combined).not.toContain(fakeAwsKey); expect(combined).not.toContain(fakeJwt); expect(combined).toContain("payload.large"); @@ -277,6 +281,8 @@ describe("diagnostic support export", () => { expect(sanitizedLogs).toContain('"subsystem":"gateway"'); expect(sanitizedLogs).toContain('"component":"gateway/server"'); expect(sanitizedLogs).toContain('"channel":"telegram"'); + expect(sanitizedLogs).not.toContain("sessionId"); + expect(sanitizedLogs).not.toContain("sessionKey"); expect(sanitizedLogs).toContain("gateway websocket listening"); expect(sanitizedLogs).toContain( "wss://:@gateway.example/ws?token=", @@ -402,6 +408,53 @@ describe("diagnostic support export", () => { expect(sanitizeSupportConfigValue(18789, redaction, "port")).toBe(18789); }); + it("blocks prototype keys and caps support sanitizer width", () => { + const redaction = { + env: { + HOME: tempDir, + OPENCLAW_STATE_DIR: tempDir, + }, + stateDir: tempDir, + }; + const wideSnapshot: Record = { + ["__proto__"]: "polluted", + constructor: "polluted", + prototype: "polluted", + }; + for (let index = 0; index < 1005; index += 1) { + wideSnapshot[`field${String(index).padStart(4, "0")}`] = index; + } + + const snapshot = sanitizeSupportSnapshotValue(wideSnapshot, redaction) as Record< + string, + unknown + >; + + expect(Object.getPrototypeOf(snapshot)).toBe(null); + expect(snapshot.__proto__).toBeUndefined(); + expect(snapshot.constructor).toBeUndefined(); + expect(snapshot.prototype).toBeUndefined(); + expect(snapshot.field0000).toBe(0); + expect(snapshot.field0999).toBe(999); + expect(snapshot.field1000).toBeUndefined(); + expect(snapshot[""]).toEqual({ + truncated: true, + count: 1008, + limit: 1000, + }); + + const array = sanitizeSupportConfigValue( + Array.from({ length: 1005 }, (_entry, index) => ({ name: `item-${index}` })), + redaction, + ) as Record; + + expect(Array.isArray(array)).toBe(false); + expect((array.items as unknown[]).length).toBe(1000); + expect(array.truncated).toBe(true); + expect(array.count).toBe(1005); + expect(array.limit).toBe(1000); + }); + it("redacts support text identifiers without hiding useful URL hosts", () => { const fakeAwsKey = ["ASIA", "IOSFODNN7EXAMPLE"].join(""); const fakeJwt = [ diff --git a/src/logging/diagnostic-support-log-redaction.ts b/src/logging/diagnostic-support-log-redaction.ts index 976a330dbd0..5eadd1df1f2 100644 --- a/src/logging/diagnostic-support-log-redaction.ts +++ b/src/logging/diagnostic-support-log-redaction.ts @@ -1,14 +1,15 @@ +import { isBlockedObjectKey } from "../infra/prototype-keys.js"; import { redactSupportString, type SupportRedactionContext, } from "./diagnostic-support-redaction.js"; const LOG_STRING_FIELD_RE = - /^(?:action|channel|code|component|endpoint|event|handshake|kind|level|localAddr|logger|method|model|module|msg|name|outcome|phase|pluginId|provider|reason|remoteAddr|requestId|runId|service|sessionId|sessionKey|source|status|subsystem|surface|target|time|traceId|type)$/iu; + /^(?:action|channel|code|component|endpoint|event|handshake|kind|level|localAddr|logger|method|model|module|msg|name|outcome|phase|pluginId|provider|reason|remoteAddr|requestId|runId|service|source|status|subsystem|surface|target|time|traceId|type)$/iu; const LOG_SCALAR_FIELD_RE = /^(?:active|attempt|bytes|count|durationMs|enabled|exitCode|intervalMs|jobs|limitBytes|localPort|nextWakeAtMs|pid|port|queueDepth|queued|remotePort|statusCode|waitMs|waiting)$/iu; const OMITTED_LOG_FIELD_RE = - /(?:authorization|body|chat|content|cookie|credential|detail|error|header|instruction|message|password|payload|prompt|result|secret|text|token|tool|transcript|url)/iu; + /(?:authorization|body|chat|content|cookie|credential|detail|error|header|instruction|message|password|payload|prompt|result|secret|session[-_]?id|session[-_]?key|text|token|tool|transcript|url)/iu; const UNSAFE_LOG_MESSAGE_RE = /(?:\b(?:ai response|assistant said|chat text|message contents|prompt|raw webhook body|tool output|tool result|transcript|user said|webhook body)\b|auto-responding\b.*:\s*["']|partial for\b.*:)/iu; const MAX_LOG_STRING_LENGTH = 240; @@ -31,6 +32,10 @@ function asRecord(value: unknown): Record | undefined { return value as Record; } +function createLogRecord(): Record { + return Object.create(null) as Record; +} + export function sanitizeSupportLogRecord( line: string, redaction: SupportRedactionContext, @@ -53,7 +58,7 @@ export function sanitizeSupportLogRecord( }; } - const sanitized: Record = {}; + const sanitized = createLogRecord(); addNamedLogFields(sanitized, source, redaction); addLogTapeMetaFields(sanitized, source, redaction); addLogTapeArgFields(sanitized, source, redaction); @@ -183,6 +188,9 @@ function addSafeLogField( if (OMITTED_LOG_FIELD_RE.test(key)) { return; } + if (isBlockedObjectKey(key)) { + return; + } if (!isSafeLogField(key, value)) { return; } diff --git a/src/logging/diagnostic-support-redaction.ts b/src/logging/diagnostic-support-redaction.ts index 43e553abe17..ee7e41d25b2 100644 --- a/src/logging/diagnostic-support-redaction.ts +++ b/src/logging/diagnostic-support-redaction.ts @@ -1,5 +1,6 @@ import path from "node:path"; import { isSecretRefShape } from "../config/redact-snapshot.secret-ref.js"; +import { isBlockedObjectKey } from "../infra/prototype-keys.js"; import { isSensitiveUrlQueryParamName } from "../shared/net/redact-sensitive-url.js"; import { redactSensitiveText } from "./redact.js"; @@ -28,7 +29,10 @@ const HANDLE_RE = /(^|[^\w:/])@[A-Za-z0-9_]{5,}\b(?!\.)/gu; const LONG_DECIMAL_ID_RE = /\b\d{9,}\b/gu; const MAX_SUPPORT_STRING_LENGTH = 2000; const MAX_SUPPORT_SNAPSHOT_DEPTH = 10; +const MAX_SUPPORT_ARRAY_ITEMS = 1000; +const MAX_SUPPORT_OBJECT_ENTRIES = 1000; const DEFAULT_TRUNCATION_SUFFIX = "..."; +const TRUNCATED_SUPPORT_FIELD = ""; export type SupportRedactionContext = { env: NodeJS.ProcessEnv; @@ -46,6 +50,11 @@ type PathRedactionPrefix = { caseInsensitive: boolean; }; +type SupportObjectEntry = { + key: string; + value: unknown; +}; + function asRecord(value: unknown): Record | undefined { if (!value || typeof value !== "object" || Array.isArray(value)) { return undefined; @@ -66,7 +75,7 @@ function isPrivateConfigField(key: string): boolean { } function sanitizeSecretRefForSupport(value: Record): Record { - const sanitized: Record = {}; + const sanitized = createSupportRecord(); if (typeof value.source === "string") { sanitized.source = value.source; } @@ -82,6 +91,62 @@ function privateMapEntryLabel(key: string): string { return normalized.endsWith("s") ? normalized.slice(0, -1) : normalized; } +function createSupportRecord(): Record { + return Object.create(null) as Record; +} + +function countOwnObjectEntries(record: Record): number { + let count = 0; + for (const key in record) { + if (Object.prototype.hasOwnProperty.call(record, key)) { + count += 1; + } + } + return count; +} + +function limitedSupportObjectEntries(record: Record): { + count: number; + entries: SupportObjectEntry[]; +} { + let count = 0; + const entries: SupportObjectEntry[] = []; + for (const key in record) { + if (!Object.prototype.hasOwnProperty.call(record, key)) { + continue; + } + count += 1; + if (isBlockedObjectKey(key) || entries.length >= MAX_SUPPORT_OBJECT_ENTRIES) { + continue; + } + entries.push({ key, value: record[key] }); + } + entries.sort((a, b) => a.key.localeCompare(b.key)); + return { count, entries }; +} + +function addTruncationMetadata(sanitized: Record, count: number): void { + if (count > MAX_SUPPORT_OBJECT_ENTRIES) { + sanitized[TRUNCATED_SUPPORT_FIELD] = { + truncated: true, + count, + limit: MAX_SUPPORT_OBJECT_ENTRIES, + }; + } +} + +function supportArrayResult(items: unknown[], count: number): unknown[] | Record { + if (count <= MAX_SUPPORT_ARRAY_ITEMS) { + return items; + } + return { + items, + truncated: true, + count, + limit: MAX_SUPPORT_ARRAY_ITEMS, + }; +} + function isWindowsAbsolutePath(value: string): boolean { return /^(?:[A-Za-z]:[\\/]|\\\\)/u.test(value); } @@ -303,25 +368,33 @@ export function sanitizeSupportSnapshotValue( } if (Array.isArray(value)) { if (key === "programArguments") { - return sanitizeCommandArguments(value, redaction); + return supportArrayResult( + sanitizeCommandArguments(value.slice(0, MAX_SUPPORT_ARRAY_ITEMS), redaction), + value.length, + ); } - return value.map((entry) => sanitizeSupportSnapshotValue(entry, redaction, key, depth + 1)); + return supportArrayResult( + value + .slice(0, MAX_SUPPORT_ARRAY_ITEMS) + .map((entry) => sanitizeSupportSnapshotValue(entry, redaction, key, depth + 1)), + value.length, + ); } const record = asRecord(value); if (!record) { return ""; } if (PRIVATE_MAP_SUPPORT_FIELD_RE.test(key)) { - return { count: Object.keys(record).length }; + return { count: countOwnObjectEntries(record) }; } - const sanitized: Record = {}; - for (const [entryKey, entryValue] of Object.entries(record).toSorted((a, b) => - a[0].localeCompare(b[0]), - )) { + const sanitized = createSupportRecord(); + const { count, entries } = limitedSupportObjectEntries(record); + for (const { key: entryKey, value: entryValue } of entries) { sanitized[entryKey] = isPrivateSupportField(entryKey) ? "" : sanitizeSupportSnapshotValue(entryValue, redaction, entryKey, depth + 1); } + addTruncationMetadata(sanitized, count); return sanitized; } @@ -350,7 +423,12 @@ export function sanitizeSupportConfigValue( count: value.length, }; } - return value.map((entry) => sanitizeSupportConfigValue(entry, redaction, key, depth + 1)); + return supportArrayResult( + value + .slice(0, MAX_SUPPORT_ARRAY_ITEMS) + .map((entry) => sanitizeSupportConfigValue(entry, redaction, key, depth + 1)), + value.length, + ); } const record = asRecord(value); if (!record) { @@ -360,13 +438,12 @@ export function sanitizeSupportConfigValue( return isSecretRefShape(record) ? sanitizeSecretRefForSupport(record) : ""; } - const sanitized: Record = {}; + const sanitized = createSupportRecord(); let privateEntryIndex = 0; const redactEntryKeys = PRIVATE_MAP_SUPPORT_FIELD_RE.test(key); const privateEntryLabel = redactEntryKeys ? privateMapEntryLabel(key) : ""; - for (const [entryKey, entryValue] of Object.entries(record).toSorted((a, b) => - a[0].localeCompare(b[0]), - )) { + const { count, entries } = limitedSupportObjectEntries(record); + for (const { key: entryKey, value: entryValue } of entries) { let outputKey = entryKey; if (redactEntryKeys) { privateEntryIndex += 1; @@ -374,5 +451,6 @@ export function sanitizeSupportConfigValue( } sanitized[outputKey] = sanitizeSupportConfigValue(entryValue, redaction, entryKey, depth + 1); } + addTruncationMetadata(sanitized, count); return sanitized; }