Skip to content

Commit e5d1bbc

Browse files
committed
security(runtime): resolve CodeQL error-boundary findings
1 parent b40a9fc commit e5d1bbc

4 files changed

Lines changed: 211 additions & 14 deletions

File tree

open-sse/executors/lmarena/response.ts

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -68,17 +68,6 @@ export function missingCookieResult(
6868
};
6969
}
7070

71-
function parseArenaErrorBody(text: string | null | undefined, status: number): string {
72-
const fallback = `Arena API error: ${status}`;
73-
if (!text) return fallback;
74-
try {
75-
const errorJson = JSON.parse(text) as { error?: { message?: string }; message?: string };
76-
return errorJson.error?.message || errorJson.message || fallback;
77-
} catch {
78-
return text.slice(0, 500) || fallback;
79-
}
80-
}
81-
8271
function isBotOrChallenge(status: number, text: string | null | undefined): boolean {
8372
if (status === 403) return true;
8473
if (isCloudflareChallenge(text)) return true;
@@ -124,8 +113,10 @@ export function mapFailedTlsResult(opts: {
124113
markLMArenaCatalogModelDead(model);
125114
markLMArenaCatalogModelDead(arenaModelId);
126115
}
116+
// Fail closed: TLS error bodies can contain upstream stacks, causes, or internal identifiers.
117+
// Preserve the HTTP classification without projecting any body-derived text to the caller.
127118
return {
128-
response: errorResponse(status, parseArenaErrorBody(text, status), "api_error", String(status)),
119+
response: errorResponse(status, `Arena API error: ${status}`, "api_error", String(status)),
129120
url,
130121
headers,
131122
transformedBody,

open-sse/utils/errorSanitization.ts

Lines changed: 95 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,9 +358,102 @@ function redactPrivateKeyPemBlocks(value: string): string {
358358
return parts.join("");
359359
}
360360

361+
const DATA_URL_PREFIX = "data:";
362+
const BASE64_DATA_URL_MARKER = ";base64";
363+
const REDACTED_DATA_URL = "[REDACTED_DATA_URL]";
364+
365+
function matchesAsciiCaseInsensitiveAt(value: string, start: number, expected: string): boolean {
366+
if (start < 0 || start + expected.length > value.length) return false;
367+
for (let offset = 0; offset < expected.length; offset++) {
368+
const code = value.charCodeAt(start + offset);
369+
const foldedCode = code >= 0x41 && code <= 0x5a ? code + 0x20 : code;
370+
if (foldedCode !== expected.charCodeAt(offset)) return false;
371+
}
372+
return true;
373+
}
374+
375+
function isBase64DataUrlPayloadCode(code: number): boolean {
376+
return (
377+
isAsciiAlphaNumericCode(code) ||
378+
code === 0x2b ||
379+
code === 0x2f ||
380+
code === 0x3d ||
381+
code === 0x5f ||
382+
code === 0x2d
383+
);
384+
}
385+
386+
function isEcmaScriptWhitespaceCode(code: number): boolean {
387+
return (
388+
(code >= 0x09 && code <= 0x0d) ||
389+
code === 0x20 ||
390+
code === 0xa0 ||
391+
code === 0x1680 ||
392+
(code >= 0x2000 && code <= 0x200a) ||
393+
code === 0x2028 ||
394+
code === 0x2029 ||
395+
code === 0x202f ||
396+
code === 0x205f ||
397+
code === 0x3000 ||
398+
code === 0xfeff
399+
);
400+
}
401+
402+
/** Redact base64 data URLs in one pass, including input with many repeated `data:` prefixes. */
403+
function redactBase64DataUrls(value: string): string {
404+
const parts: string[] = [];
405+
let copyStart = 0;
406+
let index = 0;
407+
408+
while (index < value.length) {
409+
if (!matchesAsciiCaseInsensitiveAt(value, index, DATA_URL_PREFIX)) {
410+
index++;
411+
continue;
412+
}
413+
414+
const dataUrlStart = index;
415+
const mediaTypeStart = dataUrlStart + DATA_URL_PREFIX.length;
416+
let delimiter = mediaTypeStart;
417+
while (
418+
delimiter < value.length &&
419+
value[delimiter] !== "," &&
420+
!isEcmaScriptWhitespaceCode(value.charCodeAt(delimiter))
421+
) {
422+
delimiter++;
423+
}
424+
425+
const markerStart = delimiter - BASE64_DATA_URL_MARKER.length;
426+
const hasBase64Marker =
427+
delimiter < value.length &&
428+
value[delimiter] === "," &&
429+
markerStart >= mediaTypeStart &&
430+
matchesAsciiCaseInsensitiveAt(value, markerStart, BASE64_DATA_URL_MARKER);
431+
if (!hasBase64Marker) {
432+
index = delimiter < value.length ? delimiter + 1 : value.length;
433+
continue;
434+
}
435+
436+
let payloadEnd = delimiter + 1;
437+
while (payloadEnd < value.length && isBase64DataUrlPayloadCode(value.charCodeAt(payloadEnd))) {
438+
payloadEnd++;
439+
}
440+
if (payloadEnd === delimiter + 1) {
441+
index = delimiter + 1;
442+
continue;
443+
}
444+
445+
parts.push(value.slice(copyStart, dataUrlStart), REDACTED_DATA_URL);
446+
copyStart = payloadEnd;
447+
index = payloadEnd;
448+
}
449+
450+
if (parts.length === 0) return value;
451+
parts.push(value.slice(copyStart));
452+
return parts.join("");
453+
}
454+
361455
export function redactSensitiveErrorText(value: string): string {
362-
const commonCredentialsRedacted = redactPrivateKeyPemBlocks(value)
363-
.replace(/data:[^,\s]+;base64,[A-Za-z0-9+/=_-]+/gi, "[REDACTED_DATA_URL]")
456+
const commonCredentialsRedacted = redactBase64DataUrls(redactPrivateKeyPemBlocks(value))
364457
.replace(/\b(Bearer|Basic)\s+[A-Za-z0-9._~+/=-]+/gi, "$1 [REDACTED]")
365458
.replace(STRONG_CREDENTIAL_TOKEN_GLOBAL, "[REDACTED]");
366459
return redactLabeledCredentialAssignments(commonCredentialsRedacted);
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import assert from "node:assert/strict";
2+
import { performance } from "node:perf_hooks";
3+
import test from "node:test";
4+
5+
import { redactSensitiveErrorText } from "../../open-sse/utils/errorSanitization.ts";
6+
7+
test("redacts base64 data URLs without changing their surrounding text", () => {
8+
const input =
9+
"before DATA:image/svg+xml;charset=utf-8;BaSe64,PHN2Zz48L3N2Zz4= after " +
10+
"data:text/plain;base64,SGVsbG8! and data:;base64,U0VDUkVU.";
11+
12+
assert.equal(
13+
redactSensitiveErrorText(input),
14+
"before [REDACTED_DATA_URL] after [REDACTED_DATA_URL]! and [REDACTED_DATA_URL]."
15+
);
16+
});
17+
18+
test(
19+
"bounds work for adversarial repeated data prefixes while preserving an incomplete URL",
20+
{ timeout: 20_000 },
21+
() => {
22+
const input = `${"data:".repeat(30_000)}image/png;base64`;
23+
const startedAt = performance.now();
24+
25+
const output = redactSensitiveErrorText(input);
26+
const elapsedMs = performance.now() - startedAt;
27+
28+
assert.equal(output, input, "an incomplete data URL must remain unchanged");
29+
assert.ok(
30+
elapsedMs < 6_000,
31+
`repeated data prefixes must be processed in bounded time (took ${elapsedMs.toFixed(1)}ms)`
32+
);
33+
}
34+
);

tests/unit/lmarena-provider.test.ts

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,85 @@ describe("LMArena Executor", () => {
647647
}
648648
});
649649

650+
it("does not expose structured upstream error details while preserving classification", async () => {
651+
const executor = new LMArenaExecutor();
652+
__setTlsFetchOverrideForTesting(async () => ({
653+
status: 500,
654+
headers: new Headers({ "Content-Type": "application/json" }),
655+
text: JSON.stringify({
656+
error: {
657+
message:
658+
"SensitiveDatabaseAdapter failed\n" +
659+
" at loadSecret (/srv/private/lmarena/database.ts:46:7)",
660+
stack: "Error: database failure at /srv/private/lmarena/database.ts:46:7",
661+
cause: "postgresql://private-user:private-password@internal-db/arena",
662+
},
663+
}),
664+
body: null,
665+
}));
666+
667+
try {
668+
const result = await executor.execute({
669+
model: TEST_ARENA_MODEL_ID,
670+
body: { messages: [{ role: "user", content: "Hello" }] },
671+
credentials: { cookie: "session=test" },
672+
signal: new AbortController().signal,
673+
log: console,
674+
});
675+
676+
assert.equal(result.response.status, 500);
677+
const responseText = await result.response.text();
678+
const errorBody = JSON.parse(responseText);
679+
assert.deepEqual(errorBody.error, {
680+
message: "Arena API error: 500",
681+
type: "api_error",
682+
code: "500",
683+
});
684+
assert.doesNotMatch(
685+
responseText,
686+
/SensitiveDatabaseAdapter|loadSecret|database\.ts|private-password|stack|cause/i
687+
);
688+
} finally {
689+
__setTlsFetchOverrideForTesting(null);
690+
}
691+
});
692+
693+
it("does not expose plaintext upstream error details while preserving classification", async () => {
694+
__setTlsFetchOverrideForTesting(async () => ({
695+
status: 500,
696+
headers: new Headers({ "Content-Type": "text/plain" }),
697+
text:
698+
"SensitivePlaintextFailure: internal adapter failed\n" +
699+
" at loadSecret (/srv/private/lmarena/plaintext.ts:71:9)",
700+
body: null,
701+
}));
702+
703+
try {
704+
const result = await new LMArenaExecutor().execute({
705+
model: TEST_ARENA_MODEL_ID,
706+
body: { messages: [{ role: "user", content: "Hello" }] },
707+
credentials: { cookie: "session=test" },
708+
signal: new AbortController().signal,
709+
log: console,
710+
});
711+
712+
assert.equal(result.response.status, 500);
713+
const responseText = await result.response.text();
714+
const errorBody = JSON.parse(responseText);
715+
assert.deepEqual(errorBody.error, {
716+
message: "Arena API error: 500",
717+
type: "api_error",
718+
code: "500",
719+
});
720+
assert.doesNotMatch(
721+
responseText,
722+
/SensitivePlaintextFailure|internal adapter|loadSecret|plaintext\.ts/i
723+
);
724+
} finally {
725+
__setTlsFetchOverrideForTesting(null);
726+
}
727+
});
728+
650729
it("sanitizes network failure details before logging or responding", async () => {
651730
const errorLogs: string[] = [];
652731
__setTlsFetchOverrideForTesting(async () => {

0 commit comments

Comments
 (0)