Skip to content

miner: eight more owner/repo parsers still skip isValidRepoSegment (round 2) #10326

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 exports a shared REPO_SEGMENT_PATTERN /
isValidRepoSegment(segment: unknown): boolean (lines 72-79) specifically so that "every
owner/repo parser in this package shares this one definition instead of duplicating it or skipping
it entirely." isValidRepoSegment rejects path-traversal segments (., ..) and anything
outside [A-Za-z0-9._-]+. A prior gap-audit round already fixed seven such parsers that skipped
this validator; a fresh audit of packages/loopover-miner/lib/** found eight more call sites,
across seven files
, that still hand-roll their own owner/repo shape check (only "non-empty
and exactly one /") instead of importing and calling isValidRepoSegment:

  1. packages/loopover-miner/lib/portfolio-discovery.ts, normalizeRankedIssue (around line 43):
    const [owner, repo, extra] = repoFullName.split("/");
    if (!owner || !repo || extra !== undefined) return null;
  2. packages/loopover-miner/lib/rejection-signal.ts, parseRepoFullName (around line 68):
    same shape, feeding fetchPolicyDoc's raw.githubusercontent.com URL and
    fetchPullRequestPayload's api.github.com URL (both built with encodeURIComponent, which does
    not escape ., so a value like owner="a", repo=".." produces a path-altering URL segment).
  3. packages/loopover-miner/lib/ci-poller.ts, parseRepoFullName (around line 99).
  4. packages/loopover-miner/lib/pr-disposition-poller.ts, parseRepoFullName (around line 101) —
    byte-for-byte the same function as feat(scoring): add situational score projections #3, independently duplicated.
  5. packages/loopover-miner/lib/manage-poll.ts, the inline check inside exported
    recordManagePollSnapshot (around line 195) — notably, parseRepoArg two functions above it in
    the SAME file already calls isValidRepoSegment correctly, so this is an in-file inconsistency,
    not just a cross-file one.
  6. packages/loopover-miner/lib/chat-miner-ops-actions.ts, isQueueTargetParams (around line 36):
    typeof repoFullName === "string" && repoFullName.includes("/") — accepts "a/b/c", "/a/b",
    or a value containing .. as a segment.
  7. packages/loopover-miner/lib/chat-miner-ops-actions.ts, isDenyHookDecisionParams (around line
    42) — same weak .includes("/") check.
  8. packages/loopover-miner/lib/contribution-profile-extract.ts, its own module-local
    parseRepoFullName (around line 80) — feeds three GitHub API URLs built by raw string
    interpolation (see the companion issue on this same file's URL construction).

All eight are independently reachable: several are exported functions with no guarantee their
current caller pre-validates (e.g. rejection-signal.ts's three exported functions take
repoFullName as a plain string), and the two chat-miner-ops-actions.ts validators gate real MCP
chat-action dispatch, where malformed input from a live chat surface is exactly the kind of input
isValidRepoSegment exists to catch before it reaches a URL or SQL LIKE pattern built from it.

Requirements

  • In each of the 8 call sites listed above, import isValidRepoSegment from
    ./repo-clone.js (relative import path already used elsewhere in the package) and use it to
    validate the owner and repo segments, in addition to (not instead of) each function's existing
    non-empty / single-slash / type checks.
  • Preserve each function's existing return/throw contract exactly (e.g. ci-poller.ts and
    pr-disposition-poller.ts's parseRepoFullName throw Error("invalid_repo_full_name");
    portfolio-discovery.ts, rejection-signal.ts, manage-poll.ts, and
    contribution-profile-extract.ts return null or throw as they already do — only add the
    validation, do not change the success/failure signaling shape).
  • For chat-miner-ops-actions.ts's two validators, replace the .includes("/") check with a real
    two-segment split + isValidRepoSegment check on both segments (matching the shape check every
    other parser in this issue uses), not just an additional isValidRepoSegment call bolted onto the
    existing loose check.
  • Do not touch manage-poll.ts's existing parseRepoArg function — it already calls
    isValidRepoSegment correctly and is not in scope.

Deliverables

  • All 8 call sites listed in Context validate both owner and repo with
    isValidRepoSegment before treating the parsed value as valid.
  • A new regression test for each of the 8 call sites, asserting that a path-traversal or
    otherwise-invalid segment (e.g. "acme/..", "../etc", or a segment containing a control
    character) is now rejected — added to the existing test file that already covers that
    function (e.g. test/unit/miner-portfolio-discovery.test.ts,
    test/unit/miner-rejection-signal.test.ts, test/unit/miner-ci-poller.test.ts,
    test/unit/miner-pr-disposition-poller.test.ts, the manage-poll test file, the
    chat-miner-ops-actions / MCP-governor-gating test file, and the
    contribution-profile-extract test file). Do not add a single combined test file for all 8 —
    each fix's regression test lives next to the existing tests for that function.

All 8 fixes and all 8 regression tests are required in this one PR — this is a mechanical,
repetitive fix applied consistently across every site; there is no reason to split it, and a PR
that fixes only some of the 8 sites does not resolve this issue.

Test Coverage Requirements

This repo's Codecov patch gate is 99%+ (branch-counted) on packages/loopover-miner/lib/**. Every
new validation branch you add (the new isValidRepoSegment check succeeding and failing) must be
exercised by both a passing-value test and a rejecting-value test. Tests must be added under this
repo's actual test root, test/unit/** (this package's tests live in the shared root test/
directory, not a packages/loopover-miner/test/** directory — confirm the existing sibling test
files' location before adding new ones, and mirror it exactly).

Expected Outcome

Every owner/repo parser under packages/loopover-miner/lib/** — not just the majority that
already do — rejects a path-traversal or otherwise-invalid repo segment before it reaches a URL,
SQL LIKE pattern, or downstream call. No parser in the package is a silent outlier anymore.

Links & Resources

  • packages/loopover-miner/lib/repo-clone.ts — the canonical isValidRepoSegment /
    REPO_SEGMENT_PATTERN to reuse.
  • packages/loopover-miner/lib/manage-poll.ts's own parseRepoArg — the correct in-file sibling
    pattern to mirror for that file's fix.

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