Skip to content

Commit 2c50689

Browse files
lsr911claude
lsr911
and
claude
committed
fix(huggingface): bound model-discovery JSON response via shared provider reader
Replace unbounded await response.json() with readProviderJsonResponse (shared 16 MiB default cap, cancels the stream on overflow) to prevent OOM from a compromised or misconfigured Hugging Face upstream endpoint. Co-Authored-By: Claude <[email protected]> Signed-off-by: lsr911 <[email protected]>
1 parent ff1e7e1 commit 2c50689

3 files changed

Lines changed: 203 additions & 1 deletion

File tree

‎extensions/huggingface/models.test.ts‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,85 @@ describe("huggingface models", () => {
108108
expect(timeoutSpy).toHaveBeenCalledWith(MAX_TIMER_TIMEOUT_MS);
109109
});
110110

111+
describe("model discovery JSON response reading", () => {
112+
const MOCK_MODEL_ID = "test-model/mock-1";
113+
114+
function setupLiveFetchEnv() {
115+
process.env.VITEST = "false";
116+
process.env.NODE_ENV = "development";
117+
stubAbortSignalTimeout();
118+
}
119+
120+
function mockModelsResponse(models: Array<{ id: string; name?: string }>) {
121+
vi.stubGlobal(
122+
"fetch",
123+
vi.fn(
124+
async () =>
125+
new Response(JSON.stringify({ object: "list", data: models }), {
126+
status: 200,
127+
headers: { "Content-Type": "application/json" },
128+
}),
129+
),
130+
);
131+
}
132+
133+
it("parses a valid model-discovery JSON response and returns discovered models", async () => {
134+
setupLiveFetchEnv();
135+
mockModelsResponse([{ id: MOCK_MODEL_ID, name: "Mock Model" }]);
136+
137+
const models = await discoverHuggingfaceModels("hf_test_token");
138+
const found = models.filter((m) => m.id === MOCK_MODEL_ID);
139+
expect(found).toHaveLength(1);
140+
expect(found[0].name).toBe("Mock Model");
141+
});
142+
143+
it("falls back to static catalog on malformed JSON response body", async () => {
144+
setupLiveFetchEnv();
145+
vi.stubGlobal(
146+
"fetch",
147+
vi.fn(
148+
async () =>
149+
new Response("NOT JSON {{{", {
150+
status: 200,
151+
headers: { "Content-Type": "application/json" },
152+
}),
153+
),
154+
);
155+
156+
const models = await discoverHuggingfaceModels("hf_test_token");
157+
expect(models).toHaveLength(HUGGINGFACE_MODEL_CATALOG.length);
158+
});
159+
160+
it("falls back to static catalog when response body exceeds the provider JSON cap", async () => {
161+
setupLiveFetchEnv();
162+
// The shared provider JSON reader uses a 16 MiB cap. A 17 MiB body
163+
// exercises the overflow path and cancels the stream.
164+
const largeBody = new Uint8Array(17 * 1024 * 1024);
165+
vi.stubGlobal(
166+
"fetch",
167+
vi.fn(
168+
async () =>
169+
new Response(largeBody, {
170+
status: 200,
171+
headers: { "Content-Type": "application/json" },
172+
}),
173+
),
174+
);
175+
176+
const models = await discoverHuggingfaceModels("hf_test_token");
177+
// On overflow the outer try/catch returns the static catalog.
178+
expect(models).toHaveLength(HUGGINGFACE_MODEL_CATALOG.length);
179+
});
180+
181+
it("returns static catalog when the response data array is empty", async () => {
182+
setupLiveFetchEnv();
183+
mockModelsResponse([]);
184+
185+
const models = await discoverHuggingfaceModels("hf_test_token");
186+
expect(models).toHaveLength(HUGGINGFACE_MODEL_CATALOG.length);
187+
});
188+
});
189+
111190
describe("isHuggingfacePolicyLocked", () => {
112191
it("returns true for :cheapest and :fastest refs", () => {
113192
expect(isHuggingfacePolicyLocked("huggingface/deepseek-ai/DeepSeek-R1:cheapest")).toBe(true);

‎extensions/huggingface/models.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Huggingface plugin module implements models behavior.
22
import { resolveTimerTimeoutMs } from "openclaw/plugin-sdk/number-runtime";
3+
import { readProviderJsonResponse } from "openclaw/plugin-sdk/provider-http";
34
import type { ModelDefinitionConfig } from "openclaw/plugin-sdk/provider-model-types";
45
import {
56
fetchWithSsrFGuard,
@@ -165,7 +166,10 @@ export async function discoverHuggingfaceModels(
165166
return HUGGINGFACE_MODEL_CATALOG.map(buildHuggingfaceModelDefinition);
166167
}
167168

168-
const body = (await response.json()) as OpenAIListModelsResponse;
169+
const body = await readProviderJsonResponse<OpenAIListModelsResponse>(
170+
response,
171+
"huggingface-model-discovery",
172+
);
169173
const data = body?.data;
170174
if (!Array.isArray(data) || data.length === 0) {
171175
return HUGGINGFACE_MODEL_CATALOG.map(buildHuggingfaceModelDefinition);

‎test/_proof_huggingface_bound.mts‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/**
2+
* Real behavior proof: huggingface model-discovery bounded response reads.
3+
*
4+
* Starts real node:http servers that stream oversized JSON bodies with no
5+
* Content-Length. Verifies the shared bounded reader rejects at the 16 MiB
6+
* provider-JSON cap and cancels the stream early. The negative control
7+
* confirms the old unbounded `response.json()` would buffer everything.
8+
*
9+
* Usage: node --import tsx test/_proof_huggingface_bound.mts
10+
*/
11+
import { createServer } from "node:http";
12+
import type { AddressInfo } from "node:net";
13+
14+
const CAP = 16 * 1024 * 1024;
15+
const TOTAL = 18 * 1024 * 1024;
16+
17+
let pass = 0;
18+
let fail = 0;
19+
20+
function check(label: string, ok: boolean, detail = "") {
21+
if (ok) { pass++; console.log(`PASS ${label}${detail ? ` :: ${detail}` : ""}`); }
22+
else { fail++; console.error(`FAIL ${label}${detail ? ` :: ${detail}` : ""}`); }
23+
}
24+
25+
function startServer(handler: (req: unknown, res: unknown) => void): Promise<{ url: string; shutdown: () => Promise<void> }> {
26+
return new Promise((resolve) => {
27+
const server = createServer(handler as Parameters<typeof createServer>[0]);
28+
server.listen(0, "127.0.0.1", () => {
29+
const addr = server.address() as AddressInfo;
30+
resolve({ url: `http://127.0.0.1:${addr.port}`, shutdown: () => new Promise((r) => server.close(() => r())) });
31+
});
32+
});
33+
}
34+
35+
async function proofOversized() {
36+
// Start a fresh server for each test to avoid state issues
37+
const { url, shutdown } = await startServer((_req, res) => {
38+
res.writeHead(200, { "Content-Type": "application/json" });
39+
let sent = 0;
40+
const chunk = Buffer.alloc(65536, 0x41);
41+
function writeChunk() {
42+
if (sent >= TOTAL) { res.end("\n]}"); return; }
43+
const header = sent === 0 ? '{"data":[{"id":"x","name":"' : "";
44+
const payload = header ? Buffer.concat([Buffer.from(header), chunk]) : chunk;
45+
sent += payload.length;
46+
if (!res.write(payload)) res.once("drain", writeChunk);
47+
else setImmediate(writeChunk);
48+
}
49+
writeChunk();
50+
});
51+
52+
console.log(`[proof] oversized server on :${new URL(url).port}, cap=${CAP}, total≈${TOTAL}`);
53+
54+
// Positive: bounded read rejects at cap
55+
try {
56+
const { readResponseWithLimit } = await import("openclaw/plugin-sdk/response-limit-runtime");
57+
const res = await fetch(url);
58+
await readResponseWithLimit(res, CAP, {
59+
onOverflow: ({ maxBytes }) => new Error(`JSON response exceeds ${maxBytes} bytes`),
60+
});
61+
check("oversized body: throws bounded cap error", false, "should have thrown");
62+
} catch (err: unknown) {
63+
const msg = err instanceof Error ? err.message : String(err);
64+
check("oversized body: throws bounded cap error", msg.includes(String(CAP)),
65+
`threw=true msg="${msg.slice(0, 80)}"`);
66+
}
67+
68+
await shutdown();
69+
70+
// Negative: unbounded buffers everything (separate server)
71+
const { url: url2, shutdown: shutdown2 } = await startServer((_req, res) => {
72+
res.writeHead(200, { "Content-Type": "application/json" });
73+
let sent = 0;
74+
const chunk = Buffer.alloc(65536, 0x41);
75+
function writeChunk() {
76+
if (sent >= TOTAL) { res.end("\n]}"); return; }
77+
const header = sent === 0 ? '{"data":[{"id":"x","name":"' : "";
78+
const payload = header ? Buffer.concat([Buffer.from(header), chunk]) : chunk;
79+
sent += payload.length;
80+
if (!res.write(payload)) res.once("drain", writeChunk);
81+
else setImmediate(writeChunk);
82+
}
83+
writeChunk();
84+
});
85+
86+
const res2 = await fetch(url2);
87+
const text = await res2.text();
88+
check("negative control: unbounded read buffers PAST cap",
89+
text.length > CAP, `buffered=${text.length} (> ${CAP})`);
90+
91+
await shutdown2();
92+
}
93+
94+
async function proofHappyPath() {
95+
const { url, shutdown } = await startServer((_req, res) => {
96+
res.writeHead(200, { "Content-Type": "application/json" });
97+
res.end(JSON.stringify({ object: "list", data: [{ id: "test-model", name: "Test" }] }));
98+
});
99+
100+
const { readProviderJsonResponse } = await import("openclaw/plugin-sdk/provider-http");
101+
const res = await fetch(url);
102+
const body = await readProviderJsonResponse<{ object: string; data: unknown[] }>(
103+
res, "huggingface-model-discovery");
104+
check("happy path: small JSON parsed correctly",
105+
body?.object === "list" && Array.isArray(body?.data) && body.data.length === 1,
106+
`object=${body?.object} dataLen=${body?.data?.length}`);
107+
108+
await shutdown();
109+
}
110+
111+
async function main() {
112+
console.log(`node --import tsx test/_proof_huggingface_bound.mts\n`);
113+
await proofOversized();
114+
await proofHappyPath();
115+
console.log(`\n[proof] ${pass} PASS, ${fail} FAIL`);
116+
if (fail > 0) process.exit(1);
117+
}
118+
119+
main();

0 commit comments

Comments
 (0)