Skip to content

Commit a941d3a

Browse files
tzy-17Patrick-Erichsensteipete
authored
fix(shared): reject HTTP status codes outside the 100-599 range (#110554)
* fix(shared): reject HTTP status codes outside the 100-599 range extractHttpStatusMatch only checked Number.isFinite, so 3-digit sequences such as "000", "099", "600", and "999" were accepted as valid HTTP statuses. Downstream code then surfaced them as "HTTP 0" in user-visible messages and fed them into retry/classification branches (e.g. isCloudflareOrHtmlErrorPage checks `code < 500` and `code < 600`). Restrict the extracted code to the IANA HTTP status code range (100-599). Co-authored-by: Patrick Erichsen <[email protected]> * fix(shared): validate all parsed HTTP status prefixes --------- Co-authored-by: Patrick Erichsen <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>
1 parent 95d632e commit a941d3a

2 files changed

Lines changed: 95 additions & 10 deletions

File tree

‎src/shared/assistant-error-format.test.ts‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import { describe, expect, it } from "vitest";
2+
import {
3+
extractLeadingHttpStatus,
4+
extractProviderWrappedHttpStatus,
5+
formatRawAssistantErrorForUi,
6+
parseApiErrorInfo,
7+
} from "./assistant-error-format.js";
8+
9+
describe("extractLeadingHttpStatus", () => {
10+
it("accepts status codes in the valid HTTP range 100-599", () => {
11+
expect(extractLeadingHttpStatus("100 everything is fine")).toEqual({
12+
code: 100,
13+
rest: "everything is fine",
14+
});
15+
expect(extractLeadingHttpStatus("500 internal error")).toEqual({
16+
code: 500,
17+
rest: "internal error",
18+
});
19+
expect(extractLeadingHttpStatus("599 something rest")).toEqual({
20+
code: 599,
21+
rest: "something rest",
22+
});
23+
});
24+
25+
it("rejects 3-digit sequences outside the HTTP status code range", () => {
26+
// 000 / 099 / 999 / 600 — the regex would capture these as 3-digit
27+
// numbers, but they are not valid HTTP statuses and should not be
28+
// surfaced as "HTTP <code>" in user-visible messages or in retry
29+
// classification.
30+
expect(extractLeadingHttpStatus("000 something")).toBeNull();
31+
expect(extractLeadingHttpStatus("099 something")).toBeNull();
32+
expect(extractLeadingHttpStatus("600 something")).toBeNull();
33+
expect(extractLeadingHttpStatus("999 something")).toBeNull();
34+
});
35+
36+
it("rejects strings that do not start with a 3-digit HTTP status", () => {
37+
expect(extractLeadingHttpStatus("no status here")).toBeNull();
38+
expect(extractLeadingHttpStatus("")).toBeNull();
39+
});
40+
});
41+
42+
describe("extractProviderWrappedHttpStatus", () => {
43+
it("accepts provider-wrapped statuses inside the valid HTTP range", () => {
44+
expect(extractProviderWrappedHttpStatus("OpenAI API error (503): service down")).toEqual({
45+
code: 503,
46+
rest: "service down",
47+
});
48+
expect(extractProviderWrappedHttpStatus("API error (429): rate limited")).toEqual({
49+
code: 429,
50+
rest: "rate limited",
51+
});
52+
});
53+
54+
it("rejects provider-wrapped statuses outside the valid HTTP range", () => {
55+
expect(extractProviderWrappedHttpStatus("API error (000): something")).toBeNull();
56+
expect(extractProviderWrappedHttpStatus("API error (999): something")).toBeNull();
57+
expect(extractProviderWrappedHttpStatus("API error (600): something")).toBeNull();
58+
});
59+
});
60+
61+
describe("HTTP status consumers", () => {
62+
it("formats only status lines inside the HTTP range", () => {
63+
expect(formatRawAssistantErrorForUi("100 Continue")).toBe("HTTP 100: Continue");
64+
expect(formatRawAssistantErrorForUi("599 Provider Error")).toBe("HTTP 599: Provider Error");
65+
expect(formatRawAssistantErrorForUi("000 Invalid")).toBe("000 Invalid");
66+
expect(formatRawAssistantErrorForUi("600 Invalid")).toBe("600 Invalid");
67+
expect(formatRawAssistantErrorForUi("999 Invalid")).toBe("999 Invalid");
68+
});
69+
70+
it("does not attach invalid status prefixes to API payloads", () => {
71+
const payload = '{"type":"error","error":{"type":"server_error","message":"Provider failed."}}';
72+
73+
expect(parseApiErrorInfo(`599 ${payload}`)).toMatchObject({
74+
httpCode: "599",
75+
type: "server_error",
76+
message: "Provider failed.",
77+
});
78+
expect(formatRawAssistantErrorForUi(`599 ${payload}`)).toBe(
79+
"HTTP 599 server_error: Provider failed.",
80+
);
81+
82+
for (const code of ["000", "600", "999"]) {
83+
expect(parseApiErrorInfo(`${code} ${payload}`)).toBeNull();
84+
expect(formatRawAssistantErrorForUi(`${code} ${payload}`)).toBe(`${code} ${payload}`);
85+
}
86+
});
87+
});

‎src/shared/assistant-error-format.ts‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { expectDefined } from "@openclaw/normalization-core";
21
// Assistant error formatting helpers normalize assistant-visible error payloads.
32
import { truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice";
43
const ERROR_PAYLOAD_PREFIX_RE =
@@ -107,7 +106,7 @@ function extractHttpStatusMatch(
107106
return null;
108107
}
109108
const code = Number(match[1]);
110-
if (!Number.isFinite(code)) {
109+
if (!Number.isInteger(code) || code < 100 || code > 599) {
111110
return null;
112111
}
113112
return { code, rest: (match[2] ?? "").trim() };
@@ -174,10 +173,10 @@ export function parseApiErrorInfo(raw?: string): ApiErrorInfo | null {
174173
let httpCode: string | undefined;
175174
let candidate = trimmed;
176175

177-
const httpPrefixMatch = candidate.match(/^(\d{3})\s+(.+)$/s);
178-
if (httpPrefixMatch) {
179-
httpCode = httpPrefixMatch[1];
180-
candidate = expectDefined(httpPrefixMatch[2], "http prefix match capture group 2").trim();
176+
const httpPrefix = extractHttpStatusMatch(candidate.match(/^(\d{3})\s+(.+)$/s));
177+
if (httpPrefix) {
178+
httpCode = String(httpPrefix.code);
179+
candidate = httpPrefix.rest;
181180
}
182181

183182
const payload = parseApiErrorPayload(candidate);
@@ -249,11 +248,10 @@ export function formatRawAssistantErrorForUi(raw?: string): string {
249248
);
250249
}
251250

252-
const httpMatch = trimmed.match(HTTP_STATUS_PREFIX_RE);
251+
const httpMatch = extractHttpStatusMatch(trimmed.match(HTTP_STATUS_PREFIX_RE));
253252
if (httpMatch) {
254-
const rest = expectDefined(httpMatch[2], "http match capture group 2").trim();
255-
if (!rest.startsWith("{")) {
256-
return `HTTP ${httpMatch[1]}: ${rest}`;
253+
if (!httpMatch.rest.startsWith("{")) {
254+
return `HTTP ${httpMatch.code}: ${httpMatch.rest}`;
257255
}
258256
}
259257

0 commit comments

Comments
 (0)