fix(miner): reject path-traversal repo segments in 4 remaining normalizeRepoFullName parsers - #8058
Conversation
…izeRepoFullName parsers contribution-profile-cache.ts, prediction-ledger.ts, replay-snapshot.ts and run-state.ts each defined normalizeRepoFullName but only checked "one slash, both halves non-empty" — a `.`/`..`/control-char owner or repo segment (e.g. "owner/..", "../repo", tab/newline in a segment) passed through and was persisted as a SQLite key / echoed back through the sibling CLIs. JSONbored#5831 and JSONbored#7525 already rolled isValidRepoSegment out to the other ten normalizeRepoFullName siblings for exactly this reason; these four were missed. Import and call isValidRepoSegment on both segments, matching the established pattern (claim-ledger.ts, governor-state.ts, portfolio-queue.ts, ...). Add a per-store regression test covering a `.`/`..`/control-char segment. Closes JSONbored#7795
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Caution 🛑 LoopOver review result - reject/close recommendedReview updated: 2026-07-22 17:24:03 UTC
Review summary Nits — 5 non-blocking
Why this is blocked
📋 Copy for AI agents — paste into your coding agentDecision 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.
|
|
LoopOver is closing this pull request on the maintainer's behalf (Linked issue overlaps another open PR; duplicate of open PR #7999). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed. |
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (25.00%) is below the target coverage (99.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #8058 +/- ##
===========================================
- Coverage 91.93% 38.18% -53.76%
===========================================
Files 742 4 -738
Lines 76035 275 -75760
Branches 23079 59 -23020
===========================================
- Hits 69904 105 -69799
+ Misses 5037 170 -4867
+ Partials 1094 0 -1094
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Summary
normalizeRepoFullNameincontribution-profile-cache.ts,prediction-ledger.ts,replay-snapshot.tsandrun-state.tsonly validated "exactly one slash, both halves non-empty" — so a./../control-char owner or repo segment (owner/..,../repo, a tab/newline inside a segment) passed through and was persisted as a SQLite key and echoed back through the sibling CLIs.#5831 and #7525 already extended
isValidRepoSegment(fromrepo-clone.ts) to the other tennormalizeRepoFullNamesiblings for exactly this "persisted into SQLite" reason; these four were never touched.Changes
isValidRepoSegmenton bothownerandrepoin all four parsers, matching the exact call shape used by the already-fixed siblings (claim-ledger.ts,governor-state.ts,portfolio-queue.ts,worktree-allocator.ts). No new validation mechanism../../control-char segment, mirroringminer-claim-ledger.test.ts's existing../etcassertion.Verification
All four affected test suites pass locally (81 tests), including the new path-traversal assertions.
Closes #7795