From e5d1bbc9a982f3d8938822a2886af8482fef7fb4 Mon Sep 17 00:00:00 2001 From: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:56:00 -0300 Subject: [PATCH] security(runtime): resolve CodeQL error-boundary findings --- open-sse/executors/lmarena/response.ts | 15 +-- open-sse/utils/errorSanitization.ts | 97 ++++++++++++++++++- .../unit/error-data-url-sanitization.test.ts | 34 +++++++ tests/unit/lmarena-provider.test.ts | 79 +++++++++++++++ 4 files changed, 211 insertions(+), 14 deletions(-) create mode 100644 tests/unit/error-data-url-sanitization.test.ts diff --git a/open-sse/executors/lmarena/response.ts b/open-sse/executors/lmarena/response.ts index f99218fd47..78575722b5 100644 --- a/open-sse/executors/lmarena/response.ts +++ b/open-sse/executors/lmarena/response.ts @@ -68,17 +68,6 @@ export function missingCookieResult( }; } -function parseArenaErrorBody(text: string | null | undefined, status: number): string { - const fallback = `Arena API error: ${status}`; - if (!text) return fallback; - try { - const errorJson = JSON.parse(text) as { error?: { message?: string }; message?: string }; - return errorJson.error?.message || errorJson.message || fallback; - } catch { - return text.slice(0, 500) || fallback; - } -} - function isBotOrChallenge(status: number, text: string | null | undefined): boolean { if (status === 403) return true; if (isCloudflareChallenge(text)) return true; @@ -124,8 +113,10 @@ export function mapFailedTlsResult(opts: { markLMArenaCatalogModelDead(model); markLMArenaCatalogModelDead(arenaModelId); } + // Fail closed: TLS error bodies can contain upstream stacks, causes, or internal identifiers. + // Preserve the HTTP classification without projecting any body-derived text to the caller. return { - response: errorResponse(status, parseArenaErrorBody(text, status), "api_error", String(status)), + response: errorResponse(status, `Arena API error: ${status}`, "api_error", String(status)), url, headers, transformedBody, diff --git a/open-sse/utils/errorSanitization.ts b/open-sse/utils/errorSanitization.ts index 5b101e32a1..e1805bcf3a 100644 --- a/open-sse/utils/errorSanitization.ts +++ b/open-sse/utils/errorSanitization.ts @@ -358,9 +358,102 @@ function redactPrivateKeyPemBlocks(value: string): string { return parts.join(""); } +const DATA_URL_PREFIX = "data:"; +const BASE64_DATA_URL_MARKER = ";base64"; +const REDACTED_DATA_URL = "[REDACTED_DATA_URL]"; + +function matchesAsciiCaseInsensitiveAt(value: string, start: number, expected: string): boolean { + if (start < 0 || start + expected.length > value.length) return false; + for (let offset = 0; offset < expected.length; offset++) { + const code = value.charCodeAt(start + offset); + const foldedCode = code >= 0x41 && code <= 0x5a ? code + 0x20 : code; + if (foldedCode !== expected.charCodeAt(offset)) return false; + } + return true; +} + +function isBase64DataUrlPayloadCode(code: number): boolean { + return ( + isAsciiAlphaNumericCode(code) || + code === 0x2b || + code === 0x2f || + code === 0x3d || + code === 0x5f || + code === 0x2d + ); +} + +function isEcmaScriptWhitespaceCode(code: number): boolean { + return ( + (code >= 0x09 && code <= 0x0d) || + code === 0x20 || + code === 0xa0 || + code === 0x1680 || + (code >= 0x2000 && code <= 0x200a) || + code === 0x2028 || + code === 0x2029 || + code === 0x202f || + code === 0x205f || + code === 0x3000 || + code === 0xfeff + ); +} + +/** Redact base64 data URLs in one pass, including input with many repeated `data:` prefixes. */ +function redactBase64DataUrls(value: string): string { + const parts: string[] = []; + let copyStart = 0; + let index = 0; + + while (index < value.length) { + if (!matchesAsciiCaseInsensitiveAt(value, index, DATA_URL_PREFIX)) { + index++; + continue; + } + + const dataUrlStart = index; + const mediaTypeStart = dataUrlStart + DATA_URL_PREFIX.length; + let delimiter = mediaTypeStart; + while ( + delimiter < value.length && + value[delimiter] !== "," && + !isEcmaScriptWhitespaceCode(value.charCodeAt(delimiter)) + ) { + delimiter++; + } + + const markerStart = delimiter - BASE64_DATA_URL_MARKER.length; + const hasBase64Marker = + delimiter < value.length && + value[delimiter] === "," && + markerStart >= mediaTypeStart && + matchesAsciiCaseInsensitiveAt(value, markerStart, BASE64_DATA_URL_MARKER); + if (!hasBase64Marker) { + index = delimiter < value.length ? delimiter + 1 : value.length; + continue; + } + + let payloadEnd = delimiter + 1; + while (payloadEnd < value.length && isBase64DataUrlPayloadCode(value.charCodeAt(payloadEnd))) { + payloadEnd++; + } + if (payloadEnd === delimiter + 1) { + index = delimiter + 1; + continue; + } + + parts.push(value.slice(copyStart, dataUrlStart), REDACTED_DATA_URL); + copyStart = payloadEnd; + index = payloadEnd; + } + + if (parts.length === 0) return value; + parts.push(value.slice(copyStart)); + return parts.join(""); +} + export function redactSensitiveErrorText(value: string): string { - const commonCredentialsRedacted = redactPrivateKeyPemBlocks(value) - .replace(/data:[^,\s]+;base64,[A-Za-z0-9+/=_-]+/gi, "[REDACTED_DATA_URL]") + const commonCredentialsRedacted = redactBase64DataUrls(redactPrivateKeyPemBlocks(value)) .replace(/\b(Bearer|Basic)\s+[A-Za-z0-9._~+/=-]+/gi, "$1 [REDACTED]") .replace(STRONG_CREDENTIAL_TOKEN_GLOBAL, "[REDACTED]"); return redactLabeledCredentialAssignments(commonCredentialsRedacted); diff --git a/tests/unit/error-data-url-sanitization.test.ts b/tests/unit/error-data-url-sanitization.test.ts new file mode 100644 index 0000000000..34fe7d439e --- /dev/null +++ b/tests/unit/error-data-url-sanitization.test.ts @@ -0,0 +1,34 @@ +import assert from "node:assert/strict"; +import { performance } from "node:perf_hooks"; +import test from "node:test"; + +import { redactSensitiveErrorText } from "../../open-sse/utils/errorSanitization.ts"; + +test("redacts base64 data URLs without changing their surrounding text", () => { + const input = + "before DATA:image/svg+xml;charset=utf-8;BaSe64,PHN2Zz48L3N2Zz4= after " + + "data:text/plain;base64,SGVsbG8! and data:;base64,U0VDUkVU."; + + assert.equal( + redactSensitiveErrorText(input), + "before [REDACTED_DATA_URL] after [REDACTED_DATA_URL]! and [REDACTED_DATA_URL]." + ); +}); + +test( + "bounds work for adversarial repeated data prefixes while preserving an incomplete URL", + { timeout: 20_000 }, + () => { + const input = `${"data:".repeat(30_000)}image/png;base64`; + const startedAt = performance.now(); + + const output = redactSensitiveErrorText(input); + const elapsedMs = performance.now() - startedAt; + + assert.equal(output, input, "an incomplete data URL must remain unchanged"); + assert.ok( + elapsedMs < 6_000, + `repeated data prefixes must be processed in bounded time (took ${elapsedMs.toFixed(1)}ms)` + ); + } +); diff --git a/tests/unit/lmarena-provider.test.ts b/tests/unit/lmarena-provider.test.ts index dfbe01ae5a..7afa04187e 100644 --- a/tests/unit/lmarena-provider.test.ts +++ b/tests/unit/lmarena-provider.test.ts @@ -647,6 +647,85 @@ describe("LMArena Executor", () => { } }); + it("does not expose structured upstream error details while preserving classification", async () => { + const executor = new LMArenaExecutor(); + __setTlsFetchOverrideForTesting(async () => ({ + status: 500, + headers: new Headers({ "Content-Type": "application/json" }), + text: JSON.stringify({ + error: { + message: + "SensitiveDatabaseAdapter failed\n" + + " at loadSecret (/srv/private/lmarena/database.ts:46:7)", + stack: "Error: database failure at /srv/private/lmarena/database.ts:46:7", + cause: "postgresql://private-user:private-password@internal-db/arena", + }, + }), + body: null, + })); + + try { + const result = await executor.execute({ + model: TEST_ARENA_MODEL_ID, + body: { messages: [{ role: "user", content: "Hello" }] }, + credentials: { cookie: "session=test" }, + signal: new AbortController().signal, + log: console, + }); + + assert.equal(result.response.status, 500); + const responseText = await result.response.text(); + const errorBody = JSON.parse(responseText); + assert.deepEqual(errorBody.error, { + message: "Arena API error: 500", + type: "api_error", + code: "500", + }); + assert.doesNotMatch( + responseText, + /SensitiveDatabaseAdapter|loadSecret|database\.ts|private-password|stack|cause/i + ); + } finally { + __setTlsFetchOverrideForTesting(null); + } + }); + + it("does not expose plaintext upstream error details while preserving classification", async () => { + __setTlsFetchOverrideForTesting(async () => ({ + status: 500, + headers: new Headers({ "Content-Type": "text/plain" }), + text: + "SensitivePlaintextFailure: internal adapter failed\n" + + " at loadSecret (/srv/private/lmarena/plaintext.ts:71:9)", + body: null, + })); + + try { + const result = await new LMArenaExecutor().execute({ + model: TEST_ARENA_MODEL_ID, + body: { messages: [{ role: "user", content: "Hello" }] }, + credentials: { cookie: "session=test" }, + signal: new AbortController().signal, + log: console, + }); + + assert.equal(result.response.status, 500); + const responseText = await result.response.text(); + const errorBody = JSON.parse(responseText); + assert.deepEqual(errorBody.error, { + message: "Arena API error: 500", + type: "api_error", + code: "500", + }); + assert.doesNotMatch( + responseText, + /SensitivePlaintextFailure|internal adapter|loadSecret|plaintext\.ts/i + ); + } finally { + __setTlsFetchOverrideForTesting(null); + } + }); + it("sanitizes network failure details before logging or responding", async () => { const errorLogs: string[] = []; __setTlsFetchOverrideForTesting(async () => {