You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
packages/loopover-miner/lib/repo-clone.js validates owner/repo strings with an explicit path-safety pattern before ever joining them into a filesystem path:
// GitHub owner/repo names are restricted to alphanumerics, hyphens, underscores, and periods, and are never// exactly "." or ".." -- both are rejected here so a value like "../foo" can't make resolveRepoCloneDir's// join(cloneBaseDir, owner, repo) escape the intended clone directory (a real path-traversal finding).constREPO_SEGMENT_PATTERN=/^[A-Za-z0-9._-]+$/;functionisPathTraversalSegment(segment){returnsegment==="."||segment==="..";}functionnormalizeRepoFullName(repoFullName){const[owner,repo,extra]=repoFullName.trim().split("/");if(!owner||!repo||extra!==undefined)thrownewError("invalid_repo_full_name");if(!REPO_SEGMENT_PATTERN.test(owner)||!REPO_SEGMENT_PATTERN.test(repo))thrownewError("invalid_repo_full_name");if(isPathTraversalSegment(owner)||isPathTraversalSegment(repo))thrownewError("invalid_repo_full_name");
...
}
packages/loopover-miner/lib/cross-repo-evaluation.js independently duplicates the identical pattern and guard (own copies of REPO_SEGMENT_PATTERN/isPathTraversalSegment) for the same reason.
Several other CLI-facing owner/repo parsers in the same package only check "exactly one slash, both halves non-empty," with no character-set or ./..-segment restriction:
packages/loopover-miner/lib/attempt-cli.js (parseRepoTarget, ~line 46): const [owner, repo, extra] = trimmed.split("/"); if (!owner || !repo || extra !== undefined) return null;
packages/loopover-miner/lib/claim-ledger-cli.js (~line 14): same shape.
packages/loopover-miner/lib/event-ledger-cli.js (~line 10): same shape.
packages/loopover-miner/lib/claim-ledger.js (normalizeRepoFullName, ~line 28): same shape — this is the function that persists repoFullName into the claim-ledger SQLite store as a bound query parameter (not a filesystem path, so there is no traversal risk through this specific store), so an unusual value like foo/.. or foo/bar baz is silently accepted and persisted as a ledger key.
attempt-cli.js's own parsed.repoFullName does eventually reach repo-clone.js via prepareWorktree(...), so the filesystem path itself stays protected by repo-clone.js's own stricter check further down the call stack — but the CLI's own early parser accepts the invalid value first, producing a generic downstream error instead of the same clear, immediate Repository must be in owner/repo form: <value>-style rejection the CLI already gives for a missing slash. claim-ledger-cli.js and event-ledger-cli.js have no such downstream re-validation at all — an invalid repo identifier can be claimed/queried/persisted into the claim and event ledgers without ever being caught.
Requirements
Extract REPO_SEGMENT_PATTERN and isPathTraversalSegment (currently duplicated verbatim between repo-clone.js and cross-repo-evaluation.js) into one shared helper module (or export them from one of the two and import in the other) so there is a single source of truth.
Apply the shared validator's character-set + ./..-segment check to the owner/repo parsers in attempt-cli.js, claim-ledger-cli.js, event-ledger-cli.js, and claim-ledger.js's normalizeRepoFullName, so all of them reject the same class of invalid identifier that repo-clone.js/cross-repo-evaluation.js already reject, at the point of CLI argument parsing rather than only (or never) downstream.
Preserve each file's existing error-reporting convention (its own { error: "..." }/reportCliFailure shape) — this is about tightening what counts as valid, not changing how a rejection is reported.
Deliverables
A shared REPO_SEGMENT_PATTERN/isPathTraversalSegment (or equivalent single validator function) used by repo-clone.js, cross-repo-evaluation.js, attempt-cli.js, claim-ledger-cli.js, event-ledger-cli.js, and claim-ledger.js, replacing the two duplicated copies and the four weaker ad hoc checks.
Regression tests in test/unit/miner-attempt-cli.test.ts, test/unit/miner-claim-ledger-cli.test.ts, test/unit/miner-event-ledger-cli.test.ts, and test/unit/miner-claim-ledger.test.ts covering an invalid segment (e.g. owner/.., ../etc/repo, a value with a space or slash-adjacent invalid character) each returning the same clear rejection the "missing slash" case already gets.
Test Coverage Requirements
packages/loopover-miner/lib/** is in Codecov's coverage.include; target 99%+ patch coverage on every changed line and branch in the shared validator and each of the four updated call sites, plus the regression tests listed above.
Expected Outcome
Every CLI entry point that accepts an owner/repo argument rejects the same class of malformed/unsafe identifier at the point of parsing, consistent with the validation repo-clone.js and cross-repo-evaluation.js already apply — no CLI command silently accepts (and, for the claim/event ledgers, persists) a repo identifier that would fail validation one layer further down the stack, or that never gets validated at all.
Links & Resources
packages/loopover-miner/lib/repo-clone.js (REPO_SEGMENT_PATTERN, isPathTraversalSegment, normalizeRepoFullName — the precedent to reuse)
packages/loopover-miner/lib/cross-repo-evaluation.js (duplicate copy of the same pattern)
Context
packages/loopover-miner/lib/repo-clone.jsvalidatesowner/repostrings with an explicit path-safety pattern before ever joining them into a filesystem path:packages/loopover-miner/lib/cross-repo-evaluation.jsindependently duplicates the identical pattern and guard (own copies ofREPO_SEGMENT_PATTERN/isPathTraversalSegment) for the same reason.Several other CLI-facing
owner/repoparsers in the same package only check "exactly one slash, both halves non-empty," with no character-set or./..-segment restriction:packages/loopover-miner/lib/attempt-cli.js(parseRepoTarget, ~line 46):const [owner, repo, extra] = trimmed.split("/"); if (!owner || !repo || extra !== undefined) return null;packages/loopover-miner/lib/claim-ledger-cli.js(~line 14): same shape.packages/loopover-miner/lib/event-ledger-cli.js(~line 10): same shape.packages/loopover-miner/lib/claim-ledger.js(normalizeRepoFullName, ~line 28): same shape — this is the function that persistsrepoFullNameinto the claim-ledger SQLite store as a bound query parameter (not a filesystem path, so there is no traversal risk through this specific store), so an unusual value likefoo/..orfoo/bar bazis silently accepted and persisted as a ledger key.attempt-cli.js's ownparsed.repoFullNamedoes eventually reachrepo-clone.jsviaprepareWorktree(...), so the filesystem path itself stays protected byrepo-clone.js's own stricter check further down the call stack — but the CLI's own early parser accepts the invalid value first, producing a generic downstream error instead of the same clear, immediateRepository must be in owner/repo form: <value>-style rejection the CLI already gives for a missing slash.claim-ledger-cli.jsandevent-ledger-cli.jshave no such downstream re-validation at all — an invalid repo identifier can be claimed/queried/persisted into the claim and event ledgers without ever being caught.Requirements
REPO_SEGMENT_PATTERNandisPathTraversalSegment(currently duplicated verbatim betweenrepo-clone.jsandcross-repo-evaluation.js) into one shared helper module (or export them from one of the two and import in the other) so there is a single source of truth../..-segment check to theowner/repoparsers inattempt-cli.js,claim-ledger-cli.js,event-ledger-cli.js, andclaim-ledger.js'snormalizeRepoFullName, so all of them reject the same class of invalid identifier thatrepo-clone.js/cross-repo-evaluation.jsalready reject, at the point of CLI argument parsing rather than only (or never) downstream.{ error: "..." }/reportCliFailureshape) — this is about tightening what counts as valid, not changing how a rejection is reported.Deliverables
REPO_SEGMENT_PATTERN/isPathTraversalSegment(or equivalent single validator function) used byrepo-clone.js,cross-repo-evaluation.js,attempt-cli.js,claim-ledger-cli.js,event-ledger-cli.js, andclaim-ledger.js, replacing the two duplicated copies and the four weaker ad hoc checks.test/unit/miner-attempt-cli.test.ts,test/unit/miner-claim-ledger-cli.test.ts,test/unit/miner-event-ledger-cli.test.ts, andtest/unit/miner-claim-ledger.test.tscovering an invalid segment (e.g.owner/..,../etc/repo, a value with a space or slash-adjacent invalid character) each returning the same clear rejection the "missing slash" case already gets.Test Coverage Requirements
packages/loopover-miner/lib/**is in Codecov'scoverage.include; target 99%+ patch coverage on every changed line and branch in the shared validator and each of the four updated call sites, plus the regression tests listed above.Expected Outcome
Every CLI entry point that accepts an
owner/repoargument rejects the same class of malformed/unsafe identifier at the point of parsing, consistent with the validationrepo-clone.jsandcross-repo-evaluation.jsalready apply — no CLI command silently accepts (and, for the claim/event ledgers, persists) a repo identifier that would fail validation one layer further down the stack, or that never gets validated at all.Links & Resources
packages/loopover-miner/lib/repo-clone.js(REPO_SEGMENT_PATTERN,isPathTraversalSegment,normalizeRepoFullName— the precedent to reuse)packages/loopover-miner/lib/cross-repo-evaluation.js(duplicate copy of the same pattern)packages/loopover-miner/lib/attempt-cli.js(parseRepoTarget)packages/loopover-miner/lib/claim-ledger-cli.js,packages/loopover-miner/lib/event-ledger-cli.js(their own inlineowner/reposplit-and-check)packages/loopover-miner/lib/claim-ledger.js(normalizeRepoFullName)