fix(github): reject a malformed three-segment repoFullName in comments.ts#6640
Conversation
…s.ts
`createOrUpdateIssueCommentWithMarker` validated `repoFullName` with
`const [owner, repo] = repoFullName.split("/")` + a truthiness check, so
"owner/repo/extra" passed: the destructure silently dropped the extra segment
and the GitHub call was issued against "owner/repo" — a repo the caller never
specified.
Add the missing `parts.length !== 2` segment-count check inline, matching the
guard already present in parseRepoFullName (assignees.ts / labels.ts). No-slash
and empty-segment inputs are still rejected exactly as before; whitespace
handling is intentionally left unchanged (a separate issue covers that in a
different file).
Extends test/unit/github-comments.test.ts with a case asserting
"owner/repo/extra" is rejected via createOrUpdatePrIntelligenceComment,
mirroring the existing precedent in github-assignees.test.ts.
Closes JSONbored#6612
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6640 +/- ##
=======================================
Coverage 93.64% 93.64%
=======================================
Files 680 680
Lines 68026 68028 +2
Branches 18673 18673
=======================================
+ Hits 63704 63706 +2
Misses 3347 3347
Partials 975 975
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-16 17:13:49 UTC
Review summary Nits — 3 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
What
createOrUpdateIssueCommentWithMarkerinsrc/github/comments.tsvalidatedrepoFullNamewithconst [owner, repo] = repoFullName.split("/")+ a truthiness check."owner/repo/extra"passed it — the destructure silently dropped the extra segment, and the GitHub API call was issued againstowner/repo, a repo the caller never specified.Resolves #6612.
Fix
Add the missing
parts.length !== 2segment-count check inline, matching the guard already inparseRepoFullName(assignees.ts/labels.ts):"invalid") and empty-segment ("owner/","/repo") inputs are still rejected exactly as before.createOrUpdatePrIntelligenceComment,createOrUpdateAgentCommandComment) continue to propagate the thrown error unchanged.Test
test/unit/github-comments.test.tsgains a case asserting"owner/repo/extra"is rejected viacreateOrUpdatePrIntelligenceComment, mirroring the precedent ingithub-assignees.test.ts. The newparts.length !== 2branch is exercised by it.Locally green:
npx vitest run test/unit/github-comments.test.ts→ 14/14 pass; the new branch is covered (diff patch-coverage complete).