Skip to content

miner: seven owner/repo parsers still skip isValidRepoSegment #9684

Description

@JSONbored

⚠️ 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

packages/loopover-miner/lib/repo-clone.ts:66-79 defines isValidRepoSegment to reject a ./../control-char/whitespace owner or repo segment. Issues #5831, #7525, #7795 and #8350 rolled it out across the package; the comment above the constant records which files were behind at the time:

// Exported so every other owner/repo parser in this package (#5831) shares this one definition instead of
// duplicating it (cross-repo-evaluation.js) or skipping it entirely (attempt-cli.js, claim-ledger-cli.js,
// event-ledger-cli.js, claim-ledger.js).

Every one of those five statements is now false: cross-repo-evaluation.ts:123 imports and calls it, and attempt-cli.ts:195, claim-ledger-cli.ts:51, event-ledger-cli.ts:27 and claim-ledger.ts:108 all enforce it. A reader auditing which parsers are unguarded is pointed at the wrong five files.

Meanwhile seven parsers with the identical body shape were never touched:

CLI argument parsersdiscover-cli.ts:202-207 (parseRepoTarget), loop-cli.ts:117-122 (parseRepoTarget, byte-identical to attempt-cli.ts:191-197 except for the guard), manage-poll.ts:51-58 (parseRepoArg), purge-cli.ts:142-150 (parseRepoArg, byte-identical to portfolio-queue-cli.ts:49-62 except for the guard), run-state-cli.ts:31-39 (parseRepoArg).

Store write-path parsersevent-ledger.ts:81-87 (normalizeOptionalRepoFullName): this is the write path appendEvent uses, so appendEvent({ repoFullName: "../evil" }) persists "../evil" into miner_event_ledger.repo_full_name. Its own CLI rejects such a value on read (event-ledger-cli.ts:27), so the row becomes unreachable via ledger list --repo and un-purgeable via purge --repo, while still being echoed by unfiltered readers. And ranked-candidates.ts:109-114 (normalizeRepoFullName) — the snapshot store discover replaces on every run and purge --repo targets.

Concretely today: loopover-miner purge --repo ../x parses successfully, then each store's own purgeByRepo diverges — the guarded stores throw invalid_repo_full_name (reported as a per-store error, exit 2, outcome partial), while ranked-candidates accepts it and reports a clean 0. That mixed disposition is the observable symptom.

Requirements

  • All seven parsers listed above must call isValidRepoSegment(owner) and isValidRepoSegment(repo), imported from ./repo-clone.js.
  • Each must preserve its own existing rejection convention: the five CLI parsers return their current { error: "Repository must be in owner/repo form." } / null shape; event-ledger.ts and ranked-candidates.ts throw their existing error names (invalid_repo_full_name and invalid_ranked_candidate respectively). Do not introduce new error strings or new exit codes.
  • event-ledger.ts's parser must keep returning null for an omitted/nullish repoFullName (the guard applies only to the string path).
  • The stale comment at packages/loopover-miner/lib/repo-clone.ts:69-71 must be rewritten to describe the current state: the guard is exported and shared by every owner/repo parser in the package, with no list of files that skip it.

⚠️ Required pattern: import and call isValidRepoSegment from ./repo-clone.js exactly as packages/loopover-miner/lib/portfolio-queue-cli.ts:56-60 and packages/loopover-miner/lib/claim-ledger.ts:104-110 already do. It does NOT satisfy this issue to re-derive the regex locally, to add a new shared "parseRepoFullName" helper and refactor call sites onto it, to fix only the five CLI parsers and leave the two store parsers, or to leave the stale comment in place.

Deliverables

  • parseRepoTarget in discover-cli.ts, parseRepoTarget in loop-cli.ts, parseRepoArg in manage-poll.ts, parseRepoArg in purge-cli.ts, and parseRepoArg in run-state-cli.ts all call isValidRepoSegment on both segments and reject via their existing error shape.
  • normalizeOptionalRepoFullName in event-ledger.ts and normalizeRepoFullName in ranked-candidates.ts both call isValidRepoSegment on both segments and throw their existing error names.
  • The comment at repo-clone.ts:69-71 no longer names cross-repo-evaluation.js, attempt-cli.js, claim-ledger-cli.js, event-ledger-cli.js or claim-ledger.js as unguarded/duplicating.
  • A regression test per file (seven cases total) asserting a .. segment is rejected — mirroring the existing assertion at test/unit/miner-claim-ledger.test.ts:139 — in the corresponding test/unit/miner-*.test.ts suites.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example fixing the five CLI parsers and skipping event-ledger.ts, which is the one that actually persists an unreachable row — does not resolve this issue.

Test Coverage Requirements

packages/loopover-miner/lib/**/*.ts IS inside Codecov's coverage.include in vitest.config.ts, so the 99%+ branch-counted codecov/patch gate applies exactly as for src/**. Each new guard is a new branch and needs both arms covered (valid segments accepted, .. segment rejected) in all seven files, plus the nullish-passthrough arm in event-ledger.ts. Each rejection case must be a named regression test that fails against the current code.

Expected Outcome

Every owner/repo parser in packages/loopover-miner rejects the same class of unsafe identifier, so purge --repo, ledger list --repo, discover, loop, manage poll and state get/set all reject a ./../control-char segment at parse time instead of producing a mixed per-store disposition or an un-purgeable ledger row.

Links & Resources

packages/loopover-miner/lib/repo-clone.ts:66-79, packages/loopover-miner/lib/portfolio-queue-cli.ts:49-62, packages/loopover-miner/lib/claim-ledger.ts:104-110, test/unit/miner-claim-ledger.test.ts:139. Precedents: #5831, #7525, #7795, #8350.

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