Skip to content

4 sibling normalizeRepoFullName parsers still skip the path-safety guard #5831/#7525 already added to every other sibling #7795

Description

@JSONbored

Context

repo-clone.ts defines isValidRepoSegment/REPO_SEGMENT_PATTERN specifically to reject a ./../control-char owner or repo segment before it's persisted to SQLite or echoed through the CLI. Issue #5831 rolled this out to attempt-cli.ts, claim-ledger-cli.ts, event-ledger-cli.ts, claim-ledger.ts; issue #7525 explicitly extended it to governor-state.ts, governor-ledger.ts, governor-ledger-cli.ts, portfolio-queue.ts, portfolio-queue-cli.ts, worktree-allocator.ts — all ten of those files now call isValidRepoSegment.

Four more sibling parsers with the exact same body shape (and the exact same "persisted into SQLite" risk #7525's own issue text used as its rationale) were never touched by either issue and still only check "exactly one slash, both halves non-empty":

  • packages/loopover-miner/lib/contribution-profile-cache.ts:55-61
  • packages/loopover-miner/lib/prediction-ledger.ts:86-90
  • packages/loopover-miner/lib/replay-snapshot.ts:71-75
  • packages/loopover-miner/lib/run-state.ts:55-61
function normalizeRepoFullName(repoFullName: unknown): string {
  if (typeof repoFullName !== "string") throw new Error("invalid_repo_full_name");
  const [owner, repo, extra] = repoFullName.trim().split("/");
  if (!owner || !repo || extra !== undefined) throw new Error("invalid_repo_full_name");
  return `${owner}/${repo}`;   // no isValidRepoSegment(owner)/isValidRepoSegment(repo) check
}

Verified empirically that inputs like "owner/..", "../repo", and segments containing tab/newline characters pass these four unguarded parsers but are correctly rejected by the already-fixed sibling parsers (claim-ledger.ts et al.) that do call isValidRepoSegment.

These four functions back SQLite keys (contribution-profile-cache.sqlite3, prediction-ledger.sqlite3, replay-snapshot.sqlite3, run-state.sqlite3) and are echoed back through their sibling CLIs (e.g. run-state-cli.ts's state get/set <repo>) — matching exactly the rationale #5831/#7525 used to justify the fix elsewhere. Also confirmed untested for this specific case: each file's own test suite only exercises "no slash"/"extra segment" malformed cases, none test a ./../control-char segment, unlike test/unit/miner-claim-ledger.test.ts:139, which explicitly asserts a ../etc repo throws.

Requirements

⚠️ Required pattern - import and call isValidRepoSegment from ./repo-clone.js exactly as the ten already-fixed sibling files do (see claim-ledger.ts, governor-state.ts, portfolio-queue.ts, worktree-allocator.ts for the exact call shape). Do not invent a different validation approach.

Deliverables

Test Coverage Requirements

These files are under src/ via the packages/loopover-miner workspace - confirm current coverage.include scoping before assuming the top-level 99% patch gate applies exactly as src/** does; the new guard branch and its test must both be covered in all 4 files.

Expected Outcome

All normalizeRepoFullName implementations in packages/loopover-miner consistently reject a ./../control-char owner or repo segment, matching the safety guarantee #5831/#7525 already established for the rest of the package.

Links & Resources

packages/loopover-miner/lib/repo-clone.ts (isValidRepoSegment, the guard to reuse), packages/loopover-miner/lib/contribution-profile-cache.ts:55-61, prediction-ledger.ts:86-90, replay-snapshot.ts:71-75, run-state.ts:55-61 (the 4 files to fix), #5831 and #7525 (the precedent issues that fixed the other 10 siblings)

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions