Skip to content

Commit 5326fc7

Browse files
authored
fix(enrichment): bound doc drift file reads (#1892)
1 parent 5ff3794 commit 5326fc7

2 files changed

Lines changed: 69 additions & 3 deletions

File tree

review-enrichment/src/analyzers/doc-comment-drift.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import type { EnrichRequest, DocCommentDriftFinding } from "../types.js";
1010
const MAX_FILES = 20;
1111
const MAX_FINDINGS = 50;
1212
const MAX_SIGNATURE_LINES = 40;
13+
const MAX_FETCH_BYTES = 1_000_000;
1314
const SOURCE_RE = /\.(?:ts|tsx|js|jsx|mjs|cjs)$/;
1415
const SKIP_RE = /(?:\.d\.ts$|\.min\.|\.test\.|\.spec\.|__tests__\/|(?:^|\/)tests?\/)/;
1516
const SLUG_RE = /^[A-Za-z0-9._-]+$/;
@@ -22,6 +23,34 @@ interface ScanOptions {
2223
signal?: AbortSignal;
2324
}
2425

26+
async function readBoundedText(resp: Response, signal?: AbortSignal): Promise<string | null> {
27+
const length = Number(resp.headers.get("content-length"));
28+
if (Number.isFinite(length) && length > MAX_FETCH_BYTES) return null;
29+
if (!resp.body) return null;
30+
31+
const reader = resp.body.getReader();
32+
const decoder = new TextDecoder();
33+
let size = 0;
34+
let text = "";
35+
try {
36+
while (true) {
37+
if (signal?.aborted) return null;
38+
const { done, value } = await reader.read();
39+
if (done) break;
40+
size += value.byteLength;
41+
if (size > MAX_FETCH_BYTES) {
42+
await reader.cancel();
43+
return null;
44+
}
45+
text += decoder.decode(value, { stream: true });
46+
}
47+
text += decoder.decode();
48+
return text;
49+
} finally {
50+
reader.releaseLock();
51+
}
52+
}
53+
2554
/** Reconstruct the pre-PR content of a file by reverse-applying its unified `patch` to the post-PR `newContent`:
2655
* context and removed (`-`) lines rebuild the old text; added (`+`) lines are dropped. Returns null if a hunk's
2756
* position runs past the content (so the caller falls back to "no old parameters" and reports nothing). Pure. */
@@ -325,7 +354,7 @@ export async function scanDocCommentDrift(
325354
`https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/contents/${path}?ref=${encodeURIComponent(headSha)}`,
326355
{ headers, signal: options.signal },
327356
);
328-
if (resp.ok) content = await resp.text();
357+
if (resp.ok) content = await readBoundedText(resp, options.signal);
329358
} catch {
330359
content = null;
331360
}

review-enrichment/test/doc-comment-drift.test.ts

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ const baseReq = (files) => ({
1919
githubToken: "ght",
2020
files,
2121
});
22-
const fileWith = (content) => async () => ({ ok: true, text: async () => content });
23-
const status = (code) => async () => ({ ok: code >= 200 && code < 300, status: code, text: async () => "" });
22+
const fileWith = (content, init) => async () => new Response(content, init);
23+
const status = (code) => async () => new Response("", { status: code });
2424
const oldParams = (entries) => new Map(entries.map(([name, ids]) => [name, new Set(ids)]));
2525

2626
const DRIFTED = `/**\n * @param oldName the old one\n */\nexport function doThing(newName) {\n return newName;\n}\n`;
@@ -227,6 +227,43 @@ test("scanDocCommentDrift: fetches the file at headSha and reports drift", async
227227
assert.deepEqual(findings[0].staleParams, ["oldName"]);
228228
});
229229

230+
test("scanDocCommentDrift: skips oversized file responses before reading the body", async () => {
231+
let bodyAccessed = false;
232+
const out = await scanDocCommentDrift(
233+
baseReq([{ path: "src/a.ts", patch: DRIFT_PATCH }]),
234+
async () => ({
235+
ok: true,
236+
headers: new Headers({ "content-length": "1000001" }),
237+
get body() {
238+
bodyAccessed = true;
239+
return new Response(DRIFTED).body;
240+
},
241+
}),
242+
);
243+
assert.deepEqual(out, []);
244+
assert.equal(bodyAccessed, false);
245+
});
246+
247+
test("scanDocCommentDrift: cancels streamed file responses that exceed the byte cap", async () => {
248+
let canceled = false;
249+
const chunk = new Uint8Array(500_001);
250+
const stream = new ReadableStream({
251+
start(controller) {
252+
controller.enqueue(chunk);
253+
controller.enqueue(chunk);
254+
},
255+
cancel() {
256+
canceled = true;
257+
},
258+
});
259+
const out = await scanDocCommentDrift(
260+
baseReq([{ path: "src/a.ts", patch: DRIFT_PATCH }]),
261+
async () => new Response(stream),
262+
);
263+
assert.deepEqual(out, []);
264+
assert.equal(canceled, true);
265+
});
266+
230267
test("scanDocCommentDrift: requires a github token and a head sha", async () => {
231268
assert.deepEqual(await scanDocCommentDrift({ repoFullName: "o/r", prNumber: 1, headSha: "x", files: [{ path: "src/a.ts", patch: DRIFT_PATCH }] }, fileWith(DRIFTED)), []);
232269
assert.deepEqual(await scanDocCommentDrift({ repoFullName: "o/r", prNumber: 1, githubToken: "t", files: [{ path: "src/a.ts", patch: DRIFT_PATCH }] }, fileWith(DRIFTED)), []);

0 commit comments

Comments
 (0)