From ca5c81afb97a1a8ac5e183ef0239799c50ed9b51 Mon Sep 17 00:00:00 2001 From: real-venus Date: Fri, 17 Jul 2026 02:37:17 -0700 Subject: [PATCH] fix(discovery-index): cap the untrusted response candidate list (#6774) normalizeDiscoveryIndexResponse looped over the response's `candidates` array with no length cap. The request side already clamps page size to MAX_PAGE_LIMIT (200), but the response comes from the OPTIONAL, only-partially-trusted hosted discovery-index service (this module's own header frames it as an external boundary). A misbehaving or compromised host could return an arbitrarily large `candidates` array and force unbounded client-side normalization. Caps the array at MAX_PAGE_LIMIT, dropping the overflow with a warning; the retained page and `nextCursor` still round-trip so forward pagination continues from the truncated page. Adds a regression test asserting an oversized (250-candidate) response is truncated to 200 with the warning and a preserved cursor. Closes #6774 --- .../loopover-engine/src/discovery-index-contract.ts | 11 ++++++++++- test/unit/discovery-index-contract.test.ts | 9 +++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/loopover-engine/src/discovery-index-contract.ts b/packages/loopover-engine/src/discovery-index-contract.ts index b51be3af9e..6ec310f2a6 100644 --- a/packages/loopover-engine/src/discovery-index-contract.ts +++ b/packages/loopover-engine/src/discovery-index-contract.ts @@ -224,8 +224,17 @@ export function normalizeDiscoveryIndexResponse(raw: unknown): ParsedDiscoveryIn warnings.push("DiscoveryIndexResponse must be a mapping; falling back to an empty candidate list."); } const rawCandidates = record && Array.isArray(record.candidates) ? record.candidates : []; + // #6774: the request side clamps page size to MAX_PAGE_LIMIT, but this response comes from the OPTIONAL, + // only-partially-trusted hosted index (see this module's header). A misbehaving or compromised host could + // otherwise return an arbitrarily large `candidates` array and force unbounded client-side normalization. Cap + // it, dropping the overflow with a warning; the retained page and `nextCursor` still round-trip so pagination + // continues from the truncated page. + const boundedCandidates = rawCandidates.length > MAX_PAGE_LIMIT ? rawCandidates.slice(0, MAX_PAGE_LIMIT) : rawCandidates; + if (boundedCandidates.length < rawCandidates.length) { + warnings.push(`DiscoveryIndexResponse returned ${rawCandidates.length} candidates; capping to ${MAX_PAGE_LIMIT} and dropping the rest.`); + } const candidates: DiscoveryIndexCandidate[] = []; - for (const entry of rawCandidates) { + for (const entry of boundedCandidates) { const normalized = normalizeDiscoveryIndexCandidate(entry); if (normalized === null) { warnings.push("DiscoveryIndexResponse dropped an invalid or boundary-violating candidate."); diff --git a/test/unit/discovery-index-contract.test.ts b/test/unit/discovery-index-contract.test.ts index 728d1b5cd4..61e776a032 100644 --- a/test/unit/discovery-index-contract.test.ts +++ b/test/unit/discovery-index-contract.test.ts @@ -173,5 +173,14 @@ describe("discovery-index API contract (#4300)", () => { expect(empty.warnings.join(" ")).toMatch(/must be a mapping/); expect(normalizeDiscoveryIndexResponse({ candidates: "nope", nextCursor: " " }).response).toMatchObject({ candidates: [], nextCursor: null }); }); + + it("caps an oversized candidate array at MAX_PAGE_LIMIT (200) with a warning, and still round-trips nextCursor for the truncated page (#6774)", () => { + // A misbehaving/compromised discovery-index host returns far more than the request-side page cap allows. + const oversized = Array.from({ length: 250 }, (_, i) => ({ ...VALID_CANDIDATE, issueNumber: i + 1 })); + const parsed = normalizeDiscoveryIndexResponse({ candidates: oversized, nextCursor: "page2==" }); + expect(parsed.response.candidates).toHaveLength(200); // dropped the overflow rather than processing 250 + expect(parsed.warnings.some((w) => /returned 250 candidates; capping to 200/.test(w))).toBe(true); + expect(parsed.response.nextCursor).toBe("page2=="); // truncation must not break forward pagination + }); }); });