⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.
Context
Mirror this fix exactly: #8311 (fix(github): app.ts/comments.ts repoFullName parsing is missing the segment-count/whitespace guard every sibling GitHub-write module has) is the prior,
closed instance of this exact bug class. This issue applies the identical fix to three remaining
call sites in src/github/backfill.ts that #8311 did not cover (its scope was app.ts and
comments.ts only).
Several src/github/*.ts modules validate an incoming repoFullName: string before splitting it
into owner/repo, using this guard (quoting src/github/labels.ts:9-19, one of five existing
copies — pr-actions.ts, assignees.ts, labels.ts, issues.ts, milestones.ts, and, since
#8311, app.ts/comments.ts too):
function parseRepoFullName(repoFullName: string): { owner: string; repo: string } {
const parts = repoFullName.split("/");
const owner = parts[0];
const repo = parts[1];
if (parts.length !== 2 || !owner || !repo || /\s/.test(repoFullName)) {
throw new Error(`Invalid repository full name: ${repoFullName}`);
}
return { owner, repo };
}
This guards two failure modes: (1) "owner/repo/extra" — a naive two-variable destructure
silently drops the extra segment and issues a call against owner/repo instead of erroring on
malformed input; (2) "owner/ repo" / " owner/repo" — padded segments that pass a bare
truthiness check and get encodeURIComponent-ed straight into a GitHub API URL/GraphQL query
string.
Three functions in src/github/backfill.ts still do a bare two-variable destructure with only a
truthiness check, never adopting either guard:
fetchLiveCiAggregateViaGraphQl (src/github/backfill.ts:3298):
const [owner, name] = repoFullName.split("/"); if (!owner || !name) return null;
fetchLivePullRequestReviewDecision (:4061):
const [owner, name] = repoFullName.split("/"); if (!owner || !name) return undefined;
fetchLiveReviewThreadBlockers (:4133):
const [owner, name] = repoFullName.split("/"); if (!owner || !name) return [];
All three interpolate owner/name directly into a GraphQL query string (via
JSON.stringify(owner)/JSON.stringify(name)), the same class of downstream use #8311
hardened for app.ts/comments.ts.
Note: src/github/public.ts:204's repoFullName.split("/") call is not in scope — its
repoFullName is always produced by publicRepoFullName (src/github/public.ts), which already
validates owner/repo with a stricter regex and an allowlist check before this line ever runs,
so it is not part of this gap.
Requirements
- Each of the three functions listed above must reject a
repoFullName that is not exactly two
non-empty, whitespace-free segments — the identical guard already used by
pr-actions.ts/assignees.ts/labels.ts/issues.ts/milestones.ts/app.ts/comments.ts.
- Each call site's existing failure contract must be preserved exactly (do not change the
return-null/return-undefined/return-[] semantics per call site — only tighten the
validation each one already performs):
fetchLiveCiAggregateViaGraphQl returns null.
fetchLivePullRequestReviewDecision returns undefined.
fetchLiveReviewThreadBlockers returns [].
-
⚠️ Required pattern: follow this repo's established convention of a small,
per-module local copy of the parse/validate helper (see #8311's own required-pattern note,
and issues.ts:5-15's comment: "each GitHub-write module keeps its own copy rather than
importing a shared one, matching the existing house convention for this tiny pure check"). Add
a single local helper within src/github/backfill.ts reused by all three call sites (they are
in the same file) — do not introduce a new shared cross-file helper in client.ts or
elsewhere.
Deliverables
All of the above Deliverables are required in the same PR.
Test Coverage Requirements
99%+ Codecov patch coverage, branch-counted, on every changed line — both the newly-rejected
malformed-input branch and the existing valid-input branch at each of the three call sites must be
exercised by a real test.
Expected Outcome
src/github/backfill.ts rejects the same malformed repoFullName shapes (extra path segments,
embedded whitespace) that every sibling GitHub module in src/github/ already rejects since
#8311, closing the remaining inconsistency in this defense-in-depth boundary, with no behavior
change for any well-formed owner/repo value.
Links & Resources
#8311 (the prior, closed instance of this exact bug class, scoped to app.ts/comments.ts)
src/github/pr-actions.ts:23-32 (splitRepo), src/github/assignees.ts:7-19,
src/github/labels.ts:9-19, src/github/issues.ts:5-15, src/github/milestones.ts:5-15 (the
existing copies of this exact guard)
src/github/backfill.ts:3288-3305 (fetchLiveCiAggregateViaGraphQl), :4053-4069
(fetchLivePullRequestReviewDecision), :4125-4140 (fetchLiveReviewThreadBlockers) — the
three ungated call sites
Context
Mirror this fix exactly:
#8311(fix(github): app.ts/comments.ts repoFullName parsing is missing the segment-count/whitespace guard every sibling GitHub-write module has) is the prior,closed instance of this exact bug class. This issue applies the identical fix to three remaining
call sites in
src/github/backfill.tsthat#8311did not cover (its scope wasapp.tsandcomments.tsonly).Several
src/github/*.tsmodules validate an incomingrepoFullName: stringbefore splitting itinto
owner/repo, using this guard (quotingsrc/github/labels.ts:9-19, one of five existingcopies —
pr-actions.ts,assignees.ts,labels.ts,issues.ts,milestones.ts, and, since#8311,app.ts/comments.tstoo):This guards two failure modes: (1)
"owner/repo/extra"— a naive two-variable destructuresilently drops the extra segment and issues a call against
owner/repoinstead of erroring onmalformed input; (2)
"owner/ repo"/" owner/repo"— padded segments that pass a baretruthiness check and get
encodeURIComponent-ed straight into a GitHub API URL/GraphQL querystring.
Three functions in
src/github/backfill.tsstill do a bare two-variable destructure with only atruthiness check, never adopting either guard:
fetchLiveCiAggregateViaGraphQl(src/github/backfill.ts:3298):const [owner, name] = repoFullName.split("/"); if (!owner || !name) return null;fetchLivePullRequestReviewDecision(:4061):const [owner, name] = repoFullName.split("/"); if (!owner || !name) return undefined;fetchLiveReviewThreadBlockers(:4133):const [owner, name] = repoFullName.split("/"); if (!owner || !name) return [];All three interpolate
owner/namedirectly into a GraphQL query string (viaJSON.stringify(owner)/JSON.stringify(name)), the same class of downstream use#8311hardened for
app.ts/comments.ts.Note:
src/github/public.ts:204'srepoFullName.split("/")call is not in scope — itsrepoFullNameis always produced bypublicRepoFullName(src/github/public.ts), which alreadyvalidates
owner/repowith a stricter regex and an allowlist check before this line ever runs,so it is not part of this gap.
Requirements
repoFullNamethat is not exactly twonon-empty, whitespace-free segments — the identical guard already used by
pr-actions.ts/assignees.ts/labels.ts/issues.ts/milestones.ts/app.ts/comments.ts.return-
null/return-undefined/return-[]semantics per call site — only tighten thevalidation each one already performs):
fetchLiveCiAggregateViaGraphQlreturnsnull.fetchLivePullRequestReviewDecisionreturnsundefined.fetchLiveReviewThreadBlockersreturns[].Deliverables
src/github/backfill.ts: harden all three call sites listed above with the segment-count +whitespace guard, via a single local helper in this file reused by all three.
test/unit/backfill.test.ts,test/unit/backfill-2.test.ts, andtest/unit/graphql-status-rollup.test.ts; add each new case to whichever of those filesalready covers the corresponding function. Three cases total, one per call site, each
asserting that both
"owner/repo/extra"and"owner/ repo"are rejected the same way analready-malformed value (e.g.
"invalid", no-slash) is handled today, mirroring theexisting coverage pattern used for the sibling guard in
test/unit/github-pr-actions.test.ts/github-assignees.test.ts/github-labels.test.ts.All of the above Deliverables are required in the same PR.
Test Coverage Requirements
99%+ Codecov patch coverage, branch-counted, on every changed line — both the newly-rejected
malformed-input branch and the existing valid-input branch at each of the three call sites must be
exercised by a real test.
Expected Outcome
src/github/backfill.tsrejects the same malformedrepoFullNameshapes (extra path segments,embedded whitespace) that every sibling GitHub module in
src/github/already rejects since#8311, closing the remaining inconsistency in this defense-in-depth boundary, with no behaviorchange for any well-formed
owner/repovalue.Links & Resources
#8311(the prior, closed instance of this exact bug class, scoped toapp.ts/comments.ts)src/github/pr-actions.ts:23-32(splitRepo),src/github/assignees.ts:7-19,src/github/labels.ts:9-19,src/github/issues.ts:5-15,src/github/milestones.ts:5-15(theexisting copies of this exact guard)
src/github/backfill.ts:3288-3305(fetchLiveCiAggregateViaGraphQl),:4053-4069(
fetchLivePullRequestReviewDecision),:4125-4140(fetchLiveReviewThreadBlockers) — thethree ungated call sites