From ba066393d06ff6af061ecb9ddbed919a9d32779b Mon Sep 17 00:00:00 2001 From: RealDiligent Date: Sun, 26 Jul 2026 21:38:24 +0800 Subject: [PATCH] fix(miner): reject a portfolio-queue identifier containing the '::' composite-id separator portfolio-queue-manager's queueItemId joins apiBaseUrl/repoFullName/identifier on '::' (ITEM_ID_SEPARATOR). repoFullName is '::'-free via isValidRepoSegment, but normalizeIdentifier had no such guard, so an identifier like 'issue::5' silently corrupted the parsed triple (e.g. queueItemId(base, 'acme/widgets', 'issue::5') parsed back as repoFullName 'issue', identifier '5'). Reject a '::'-containing identifier at construction (normalizeIdentifier) rather than encode-and-hope. Tests: round-trip a valid (single-colon) identifier; enqueue with 'issue::5' throws invalid_identifier while 'issue:5' is still accepted. --- packages/loopover-miner/lib/portfolio-queue.ts | 5 +++++ test/unit/miner-portfolio-queue-manager.test.ts | 12 ++++++++++++ 2 files changed, 17 insertions(+) diff --git a/packages/loopover-miner/lib/portfolio-queue.ts b/packages/loopover-miner/lib/portfolio-queue.ts index bbc3fdf385..b28a1d9be7 100644 --- a/packages/loopover-miner/lib/portfolio-queue.ts +++ b/packages/loopover-miner/lib/portfolio-queue.ts @@ -113,6 +113,11 @@ function normalizeIdentifier(identifier: unknown): string { if (typeof identifier !== "string") throw new Error("invalid_identifier"); const trimmed = identifier.trim(); if (!trimmed) throw new Error("invalid_identifier"); + // portfolio-queue-manager's composite queueItemId joins apiBaseUrl/repoFullName/identifier on "::" + // (ITEM_ID_SEPARATOR). repoFullName is "::"-free by isValidRepoSegment, but identifier had no such guard: an + // identifier containing "::" would silently corrupt the parsed apiBaseUrl/repoFullName/identifier triple. + // Reject it at construction rather than encode-and-hope (#8857). + if (trimmed.includes("::")) throw new Error("invalid_identifier"); return trimmed; } diff --git a/test/unit/miner-portfolio-queue-manager.test.ts b/test/unit/miner-portfolio-queue-manager.test.ts index 2ecb880e50..140e477b32 100644 --- a/test/unit/miner-portfolio-queue-manager.test.ts +++ b/test/unit/miner-portfolio-queue-manager.test.ts @@ -86,6 +86,18 @@ describe("entriesToPortfolioQueue() / selectEligibleBatch() (#4285)", () => { expect(() => parseQueueItemId("https://api.github.com::::issue:7")).toThrow("invalid_queue_item_id"); }); + it("queueItemId/parseQueueItemId round-trip an identifier that does NOT contain the '::' separator (#8857)", () => { + const id = queueItemId("https://api.github.com", "acme/widgets", "issue:5"); + expect(parseQueueItemId(id)).toEqual({ apiBaseUrl: "https://api.github.com", repoFullName: "acme/widgets", identifier: "issue:5" }); + }); + + it("rejects an identifier containing the '::' separator at enqueue time, preventing silent id corruption (#8857)", () => { + const manager = memoryManager({ globalWipCap: 4, perRepoWipCap: 2 }); + expect(() => manager.enqueue({ repoFullName: "acme/widgets", identifier: "issue::5", apiBaseUrl: "https://api.github.com" })).toThrow("invalid_identifier"); + // A single-colon identifier is still accepted — the invariant only forbids the "::" join sequence itself. + expect(manager.enqueue({ repoFullName: "acme/widgets", identifier: "issue:5", apiBaseUrl: "https://api.github.com" }).identifier).toBe("issue:5"); + }); + it("entriesToPortfolioQueue falls back to the github.com default when a row's apiBaseUrl is missing (#5563)", () => { const entries = [ { repoFullName: "acme/alpha", identifier: "x", priority: 0, status: "queued", enqueuedAt: "t1" },