Context
Discovered while implementing #4740/#4741/#4824 (epic #4737's REES/deterministic-tier work and its
fetch-helper follow-ups) — flagged independently by two separate implementation passes. Non-urgent,
mechanical, repo-wide hardening pass.
The problem
In review-enrichment/src/analyzers/, 17 of 19 files that validate a GitHub owner/repo slug before
building a Contents-API URL use the weak pattern const SLUG_RE = /^[A-Za-z0-9._-]+$/;. This allows a
slug segment made entirely of dots (e.g. owner="..") to pass validation, because every character in
".." is individually in the allowed class.
Only 2 files already use the hardened version: review-enrichment/src/analyzers/duplication-delta.ts
(const SLUG_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/;) and review-enrichment/src/analyzers/codeowners.ts
(same pattern, with an explicit comment: "rejects .. and other path-traversal segments").
duplication-delta.ts's own comment explains the risk precisely: a bare [A-Za-z0-9._-]+ class lets a
segment of exactly ".." or "." satisfy it, and since these owner/repo values get spliced into a
https://github.com/ghapi/repos/{owner}/{repo}/contents/... URL string passed to fetch()/new URL(),
the WHATWG URL parser's dot-segment removal (applied to special schemes like https) can normalize
repos/../something down to just /something — sending the request to a different, unintended path on
github.com/ghapi than the one the code thinks it's calling, with the real auth token attached. This is a
confused-deputy / request-smuggling-adjacent risk, not classic filesystem traversal (there's no local
disk access), but it's still a real correctness/security gap: input validation is weaker than the two
files that already guard against it.
The 17 files still using the weak pattern: approval-integrity.ts, blame-link.ts, caller-impact.ts,
churn-hotspot.ts, commit-hygiene.ts, commit-lint.ts, commit-signature.ts, complexity-delta.ts,
coverage-delta.ts, doc-comment-drift.ts, exhaustiveness-drift.ts, flaky-test.ts,
pending-review-requests.ts, revert-recurrence.ts, stale-branch.ts, undocumented-export.ts,
unused-export.ts.
Fix
Change each of these 17 files' SLUG_RE to the hardened pattern /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/
(matching duplication-delta.ts/codeowners.ts exactly), add a short comment explaining why (mirror
duplication-delta.ts's own comment), and add/extend each file's test suite with a case confirming a
leading-dot or all-dot owner/repo segment (e.g. "..", ".") is now rejected (fails safe, returns no
finding) where an existing "invalid slug" test doesn't already cover it.
This is a repo-wide, mechanical, low-risk hardening pass — no behavior change for any legitimate
owner/repo slug (which always starts with an alphanumeric character on GitHub).
Acceptance criteria
Context
Discovered while implementing #4740/#4741/#4824 (epic #4737's REES/deterministic-tier work and its
fetch-helper follow-ups) — flagged independently by two separate implementation passes. Non-urgent,
mechanical, repo-wide hardening pass.
The problem
In
review-enrichment/src/analyzers/, 17 of 19 files that validate a GitHub owner/repo slug beforebuilding a Contents-API URL use the weak pattern
const SLUG_RE = /^[A-Za-z0-9._-]+$/;. This allows aslug segment made entirely of dots (e.g.
owner="..") to pass validation, because every character in".."is individually in the allowed class.Only 2 files already use the hardened version:
review-enrichment/src/analyzers/duplication-delta.ts(
const SLUG_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/;) andreview-enrichment/src/analyzers/codeowners.ts(same pattern, with an explicit comment: "rejects
..and other path-traversal segments").duplication-delta.ts's own comment explains the risk precisely: a bare[A-Za-z0-9._-]+class lets asegment of exactly
".."or"."satisfy it, and since these owner/repo values get spliced into ahttps://github.com/ghapi/repos/{owner}/{repo}/contents/...URL string passed tofetch()/new URL(),the WHATWG URL parser's dot-segment removal (applied to special schemes like
https) can normalizerepos/../somethingdown to just/something— sending the request to a different, unintended path ongithub.laiyagushi.com/ghapithan the one the code thinks it's calling, with the real auth token attached. This is aconfused-deputy / request-smuggling-adjacent risk, not classic filesystem traversal (there's no local
disk access), but it's still a real correctness/security gap: input validation is weaker than the two
files that already guard against it.
The 17 files still using the weak pattern:
approval-integrity.ts,blame-link.ts,caller-impact.ts,churn-hotspot.ts,commit-hygiene.ts,commit-lint.ts,commit-signature.ts,complexity-delta.ts,coverage-delta.ts,doc-comment-drift.ts,exhaustiveness-drift.ts,flaky-test.ts,pending-review-requests.ts,revert-recurrence.ts,stale-branch.ts,undocumented-export.ts,unused-export.ts.Fix
Change each of these 17 files'
SLUG_REto the hardened pattern/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/(matching
duplication-delta.ts/codeowners.tsexactly), add a short comment explaining why (mirrorduplication-delta.ts's own comment), and add/extend each file's test suite with a case confirming aleading-dot or all-dot owner/repo segment (e.g.
"..",".") is now rejected (fails safe, returns nofinding) where an existing "invalid slug" test doesn't already cover it.
This is a repo-wide, mechanical, low-risk hardening pass — no behavior change for any legitimate
owner/repo slug (which always starts with an alphanumeric character on GitHub).
Acceptance criteria
SLUG_REpattern, with a comment explaining the dot-segment risk.".."/"."slug segment is rejected (fails safe, no finding),added where not already covered.
deliberately deferred from refactor(rees): migrate three analyzers off hand-rolled bounded-fetch onto boundedFetchText #4821/PR refactor(rees): migrate three analyzers off hand-rolled bounded-fetch onto boundedFetchText #4821 and Migrate 3 more REES analyzers onto boundedFetchText #4824/PR refactor(rees): migrate three more analyzers off hand-rolled bounded-fetch onto boundedFetchText #4947, which each touched 3 of these same
files but left this fix for one consistent pass instead of a partial, inconsistent one).