diff --git a/src/github/backfill.ts b/src/github/backfill.ts index 8573942dd1..278e79d5f8 100644 --- a/src/github/backfill.ts +++ b/src/github/backfill.ts @@ -3276,6 +3276,19 @@ export function isStatusRollupGraphQlEnabled(env: { GITHUB_STATUS_ROLLUP_GRAPHQL return /^(1|true|yes|on)$/i.test(env.GITHUB_STATUS_ROLLUP_GRAPHQL ?? ""); } +// Mirrors parseRepoFullName in labels.ts / assignees.ts (#8311): backfill keeps its own local copy rather than +// importing a shared one. Returns null for malformed input so each GraphQL read helper preserves its existing +// fail-soft return contract (null / undefined / []). +function parseBackfillRepoFullName(repoFullName: string): { owner: string; name: string } | null { + const parts = repoFullName.split("/"); + const owner = parts[0]; + const name = parts[1]; + if (parts.length !== 2 || !owner || !name || /\s/.test(repoFullName)) { + return null; + } + return { owner, name }; +} + /** * GraphQL equivalent of {@link fetchLiveCiAggregate}: ONE bounded query returns the head commit's statusCheckRollup * (check-runs AND classic statuses, unified) plus its check-suites — replacing the paginated /check-runs + /status @@ -3295,8 +3308,9 @@ export async function fetchLiveCiAggregateViaGraphQl( advisoryCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null, ): Promise { if (!headSha || !token) return null; - const [owner, name] = repoFullName.split("/"); - if (!owner || !name) return null; + const parsed = parseBackfillRepoFullName(repoFullName); + if (!parsed) return null; + const { owner, name } = parsed; const query = `query LoopOverLiveCiRollup { repository(owner: ${JSON.stringify(owner)}, name: ${JSON.stringify(name)}) { object(oid: ${JSON.stringify(headSha)}) { ... on Commit { statusCheckRollup { contexts(first: 100) { nodes { __typename ... on CheckRun { name conclusion status startedAt detailsUrl title summary checkSuite { databaseId app { slug } } } ... on StatusContext { context state description targetUrl } } pageInfo { hasNextPage } } } checkSuites(first: 100) { nodes { status app { slug } } pageInfo { hasNextPage } } } } } }`; const result = await githubGraphQl<{ data?: { @@ -4058,8 +4072,9 @@ export async function fetchLivePullRequestReviewDecision( admissionKey?: GitHubRateLimitAdmissionKey, ): Promise { if (!token) return undefined; - const [owner, name] = repoFullName.split("/"); - if (!owner || !name) return undefined; + const parsed = parseBackfillRepoFullName(repoFullName); + if (!parsed) return undefined; + const { owner, name } = parsed; const query = `query { repository(owner: ${JSON.stringify(owner)}, name: ${JSON.stringify(name)}) { pullRequest(number: ${prNumber}) { reviewDecision } } }`; const result = await githubGraphQl<{ data?: { repository?: { pullRequest?: { reviewDecision?: string | null } | null } | null }; errors?: unknown[] }>( env, @@ -4130,8 +4145,9 @@ export async function fetchLiveReviewThreadBlockers( admissionKey?: GitHubRateLimitAdmissionKey, ): Promise { if (!token) return []; - const [owner, name] = repoFullName.split("/"); - if (!owner || !name) return []; + const parsed = parseBackfillRepoFullName(repoFullName); + if (!parsed) return []; + const { owner, name } = parsed; const threads: Array = []; let cursor: string | null = null; const seenCursors = new Set(); diff --git a/test/unit/backfill-2.test.ts b/test/unit/backfill-2.test.ts index 7b71d851d8..32fe1827cf 100644 --- a/test/unit/backfill-2.test.ts +++ b/test/unit/backfill-2.test.ts @@ -1660,6 +1660,18 @@ describe("GitHub backfill", () => { // (`liveReviewDecision ?? pr.reviewDecision`), so a PR approved at backfill time and later flipped to // CHANGES_REQUESTED still read as APPROVED -> approvalsSatisfied -> merge. The sentinel is a real VALUE // precisely so it survives that `??` and the stale stored decision can never be substituted. + describe("fetchLivePullRequestReviewDecision — repoFullName guard (#9317)", () => { + it("returns undefined for extra-segment and whitespace-padded slugs before any GraphQL call", async () => { + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + const fetchSpy = vi.fn(async () => Response.json({ data: { repository: { pullRequest: { reviewDecision: "APPROVED" } } } })); + vi.stubGlobal("fetch", fetchSpy); + for (const repoFullName of ["owner/repo/extra", "owner/ repo", " owner/repo"]) { + expect(await fetchLivePullRequestReviewDecision(env, repoFullName, 7, "public-token")).toBeUndefined(); + } + expect(fetchSpy).not.toHaveBeenCalled(); + }); + }); + describe("fetchLivePullRequestReviewDecision — partial-response guard (#9052)", () => { it("returns the unreadable sentinel on a 200-with-errors response, so the stale stored decision cannot win the ?? fallback", async () => { const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); @@ -2528,6 +2540,10 @@ describe("GitHub backfill", () => { expect(fetchSpy).not.toHaveBeenCalled(); await expect(fetchLiveReviewThreadBlockers(env, "malformed", 1, "public-token")).resolves.toEqual([]); expect(fetchSpy).not.toHaveBeenCalled(); + for (const repoFullName of ["owner/repo/extra", "owner/ repo", " owner/repo"]) { + await expect(fetchLiveReviewThreadBlockers(env, repoFullName, 1, "public-token")).resolves.toEqual([]); + } + expect(fetchSpy).not.toHaveBeenCalled(); await expect(fetchLiveReviewThreadBlockers(env, "JSONbored/gittensory", 1, "public-token")).resolves.toEqual([]); expect(fetchSpy).toHaveBeenCalledTimes(1); }); diff --git a/test/unit/graphql-status-rollup.test.ts b/test/unit/graphql-status-rollup.test.ts index 3da5a3caf5..9789ac1beb 100644 --- a/test/unit/graphql-status-rollup.test.ts +++ b/test/unit/graphql-status-rollup.test.ts @@ -76,6 +76,16 @@ describe("fetchLiveCiAggregateViaGraphQl — verdicts", () => { expect(await fetchLiveCiAggregateViaGraphQl(env, "no-slash", SHA, TOKEN)).toBeNull(); }); + // #9317: segment-count + whitespace guard, matching pr-actions.ts/assignees.ts/labels.ts (#8311). + it("returns null for extra-segment and whitespace-padded repo slugs before any GraphQL call (#9317)", async () => { + const fetchSpy = vi.fn(async () => Response.json(graphqlBody({ runs: [{ name: "build", conclusion: "SUCCESS", status: "COMPLETED" }] }))); + vi.stubGlobal("fetch", fetchSpy); + for (const repoFullName of ["owner/repo/extra", "owner/ repo", " owner/repo"]) { + expect(await fetchLiveCiAggregateViaGraphQl(env, repoFullName, SHA, TOKEN)).toBeNull(); + } + expect(fetchSpy).not.toHaveBeenCalled(); + }); + it("returns null on a GraphQL error or an unexpected/absent commit (→ REST fallback)", async () => { stubGraphql(graphqlBody(), { status: 500 }); expect(await fetchLiveCiAggregateViaGraphQl(env, REPO, SHA, TOKEN)).toBeNull();