Skip to content

engine: fleet-run-manifest's normalizeRepoFullName skips the #7525/#8350 path-safety guard #10311

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-engine/src/fleet-run-manifest.ts's local normalizeRepoFullName (lines 67-72)
only checks for exactly one slash and two non-empty halves:

function normalizeRepoFullName(value: unknown): string | null {
  if (typeof value !== "string") return null;
  const [owner, repo, extra] = value.trim().split("/");
  if (!owner || !repo || extra !== undefined) return null;
  return `${owner}/${repo}`;
}

Its own doc comment claims parity with "the goal-spec / portfolio-queue validators," but it never
calls isValidRepoSegment from repo-segment.ts — unlike every other owner/repo normalizer in
this package: governor-ledger.ts's normalizeOptionalRepoFullName (explicitly citing #7525/#8350),
discovery-index-contract.ts's normalizeRepoFullName, discovery-soft-claim.ts's
normalizeRepoFullName, results-payload.ts, and miner/deny-hook-synthesis.ts. This makes
fleet-run-manifest.ts the only owner/repo-splitting normalizer in the package that never applies
the path-traversal guard: a value like "../evilrepo" or "acme/.." passes through unchanged into
FleetRunManifestRepo.repoFullName — the exact value class #7525/#8350 exist to reject.
FleetRunManifest/FleetRunManifestRepo is exported via index.ts and paired with a per-repo
worktree budget (MAX_MANIFEST_REPOS), so this is live, consumed code, not scaffolding.
test/fleet-run-manifest-parser.test.ts has no test exercising a ./.. segment.

Requirements

  • In packages/loopover-engine/src/fleet-run-manifest.ts, import isValidRepoSegment from
    ./repo-segment.js (mirror the exact import used in governor-ledger.ts).
  • Update normalizeRepoFullName so that, after the existing single-slash/non-empty-halves check, it
    additionally rejects the value (returns null) when !isValidRepoSegment(owner) || !isValidRepoSegment(repo) — mirror governor-ledger.ts's normalizeOptionalRepoFullName
    exactly in behavior (same guard, same order of checks).
  • Do not change the function's signature, its null-on-invalid contract, or any other normalizer in
    this file.

Deliverables

  • normalizeRepoFullName in fleet-run-manifest.ts rejects any owner/repo segment that
    fails isValidRepoSegment (covers ., .., and any character outside
    isValidRepoSegment's allowed set), returning null exactly as it does today for the
    existing malformed-input cases.
  • A legitimate owner/repo value (e.g. "acme/widgets", "acme-co/my_widget.js") still
    normalizes successfully, unchanged.

Both Deliverables are required in this one PR — there is no narrower scope for this issue.

Test Coverage Requirements

packages/loopover-engine/src/** is measured by Codecov via two separate uploads whose hits are
unioned
— root test/** AND packages/loopover-engine/test/**. Add the regression test(s) to
packages/loopover-engine/test/fleet-run-manifest-parser.test.ts (not only a root-level test/**
file), asserting that a manifest repo entry with a repoFullName (or bare-string entry, whichever
this function's callers pass) containing "../evilrepo", "acme/..", "./acme", or a
disallowed-character segment (e.g. "ac me/widgets") is rejected the same way the existing
malformed-input tests already assert, and that legitimate slugs ("acme/widgets",
"acme-co/my_widget.js") still pass. Target 100% branch coverage of the new guard.

Expected Outcome

Every owner/repo-splitting normalizer in packages/loopover-engine/src/** applies the same
path-traversal guard, closing the one remaining gap where a manifest-supplied repo full name could
carry a ./.. traversal segment into FleetRunManifestRepo.repoFullName unrejected.

Links & Resources

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