fix(engine): share governor-ledger's repo-segment guard with the three normalizers that skipped it

[rsnetworkinginc]

Closes #9610

Summary

fix(engine): share governor-ledger's repo-segment guard with the three normalizers that skipped it

Three owner/repo normalizers in packages/loopover-engine performed the same "exactly one slash, both halves non-empty" check but skipped the "."/".." path-safety segment guard that closed #5831 → #7525 → #8350:

1. idea-intake.ts — validateIdeaSubmission({ targetRepo: "../evilrepo" }) returned ok: true (regex has the right character class but no traversal rejection), and buildClaimPlan copied that value verbatim into every ClaimStep.targetRepo. This is the renter-supplied freeform intake path reachable over HTTP (POST /v1/loop/intake-idea, POST /v1/loop/plan-idea-claims), whose zod schema is deliberately loose because the engine validator "owns the real bounds/format checks".
2. discovery-index-contract.ts:normalizeRepoFullName — data from the "OPTIONAL, only-partially-trusted hosted index" flowed into owner/repo slicing with no segment guard.
3. discovery-soft-claim.ts:normalizeRepoFullName — an identical unchecked copy whose JSDoc claimed it "mirrors the … claim-ledger repo validation" (which does call isValidRepoSegment).

Root cause

The engine-side copy of the guard lived privately inside governor-ledger.ts (deliberately not importable from the miner package), so each sibling normalizer hand-rolled its own slash check without the traversal rejection.

Fix

• New module packages/loopover-engine/src/repo-segment.ts exporting exactly two symbols — REPO_SEGMENT_PATTERN (/^[A-Za-z0-9._-]+$/) and isValidRepoSegment — a straight lift of governor-ledger.ts:43-47, byte-for-byte semantics. Both re-exported from packages/loopover-engine/src/index.ts in the existing explicit named-export barrel style.
• governor-ledger.ts deletes its local copy and imports from ./repo-segment.js; externally observable behaviour unchanged (its suite passes unmodified).
• idea-intake.ts targetRepo string branch now follows repo-clone.ts:87-93's shape — split on /, reject empty/extra segments, then isValidRepoSegment on both halves — rejecting with the existing target_repo_malformed code. No new error code.
• discovery-index-contract.ts normalizeRepoFullName returns null when either segment fails the guard; a rejected candidate is still dropped with the existing "DiscoveryIndexResponse dropped an invalid or boundary-violating candidate." warning. No new warning string.
• discovery-soft-claim.ts normalizeRepoFullName likewise returns null, so buildSoftClaimRequest returns null for such a claim, exactly as it already does for a slashless value.
• packages/loopover-miner/lib/repo-clone.ts untouched (engine must not import from the miner package). No widened character class, no new regex, no new error/warning strings.

Tests (RED → GREEN)

New named regression tests, run against the unfixed source first:

Test Files  4 failed (4)
     Tests  8 failed | 62 passed (70)

(the traversal-rejection tests fail pre-fix; the round-trip/still-accepts tests pass, showing no behavior regression). After the fix, including the untouched governor-ledger suite and both intake routes suites:

Test Files  7 passed (7)
     Tests  90 passed (90)

• validateIdeaSubmission({ …, targetRepo: "../evilrepo" }) → { ok: false, errors: ["target_repo_malformed"] }; "./x", "a/..", "../.." covered in the same block; "acme/widgets" still resolves to { kind: "existing", repo: "acme/widgets" }.
• normalizeDiscoveryIndexCandidate({ repoFullName: "../evil", issueNumber: 1, title: "x" }) → null; normalizeDiscoveryIndexResponse drops that candidate and emits the existing invalid-candidate warning.
• buildSoftClaimRequest({ …, repoFullName: "../evil" }) → null.
• A valid "owner/repo" round-trips unchanged through all three normalizers, asserted in each suite.
• New dedicated test/unit/repo-segment.test.ts covers the barrel re-export plus a passing slug, a pattern-failing segment, ".", and "..".
• New packages/loopover-engine/test/repo-segment.test.ts mirrors all four rejection classes and both accept paths inside the engine package's own node:test suite (801 tests, 801 pass), so the change is behavior-tested by the suite that actually grades engine source — not only by a vitest twin.

Coverage

Engine source is graded under two Codecov flags, and this diff is covered under both flags' own coverage runs:

• engine flag (c8 over the engine package's own node:test suite — node --experimental-strip-types scripts/engine-coverage.ts, CI's exact recipe): cross-referencing the produced packages/loopover-engine/coverage/lcov.info against this PR's diff hunks — every changed line covered, zero partial branches on changed lines, and the new repo-segment.ts at 100% lines/branches:

OK: packages/loopover-engine/src/repo-segment.ts             | uncovered - | partial-branch-lines -
OK: packages/loopover-engine/src/governor-ledger.ts          | changed-lines 2 | uncovered - | partial-branch-lines -
OK: packages/loopover-engine/src/idea-intake.ts              | changed-lines 9 | uncovered - | partial-branch-lines -
OK: packages/loopover-engine/src/discovery-index-contract.ts | changed-lines 5 | uncovered - | partial-branch-lines -
OK: packages/loopover-engine/src/discovery-soft-claim.ts     | changed-lines 5 | uncovered - | partial-branch-lines -
OK: packages/loopover-engine/src/index.ts                    | changed-lines 1 | uncovered - | partial-branch-lines -

• backend flag (root vitest v8 coverage, packages/loopover-engine/src/**/*.ts is inside coverage.include): same cross-reference against the vitest lcov — all changed lines covered, no partial branches on changed lines; repo-segment.ts and idea-intake.ts at 100% stmts/branch/funcs/lines.

Both arms of every new conditional are exercised: isValidRepoSegment pass / pattern-fail / "." / "..", and each of the three call sites with accepted and rejected values (empty halves, extra segments, non-slug characters, traversal segments).

Validation commands (repo root)

git diff --check                                  # clean
npm run build --workspace @loopover/engine        # clean tsc build
npm run test --workspace @loopover/engine         # 801 tests, 801 pass, 0 fail
node --experimental-strip-types scripts/engine-coverage.ts   # engine-flag lcov: every patch line covered, no partial branches
npx vitest run test/unit                          # green (sole exception: miner-attempt-cli #8808 timeout, which reproduces identically on unmodified origin/main in this environment)
npx vitest run --coverage test/unit/idea-intake-bridge.test.ts test/unit/discovery-index-contract.test.ts test/unit/discovery-soft-claim.test.ts test/unit/repo-segment.test.ts test/unit/governor-ledger.test.ts test/unit/routes-intake-idea.test.ts test/unit/routes-plan-idea-claims.test.ts
npm run typecheck                                 # clean

[loopover-orb]

Warning

⏸️ LoopOver review result - manual review recommended

_{Review updated: 2026-07-29 01:25:55 UTC}

11 files · 1 AI reviewer · no blockers · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
This PR extracts governor-ledger's `isValidRepoSegment` traversal guard into a new shared `repo-segment.ts` module and applies it to three previously-unguarded `owner/repo` normalizers (idea-intake, discovery-index-contract, discovery-soft-claim), closing issue #9610. The trace is verifiable end-to-end: each normalizer's split-and-check logic previously accepted a `.`/`..` segment (e.g. `"../evilrepo"` splits into owner `".."`, repo `"evilrepo"`, both passing the old regex-only check) and now correctly rejects it via the shared guard, matching the exact fix pattern already proven in governor-ledger.ts. Tests are real and exercise the actual reachable normalizer functions with concrete traversal payloads, not fabricated states, and the barrel export addition is wired correctly.

Nits — 4 non-blocking

• packages/loopover-engine/src/idea-intake.ts's inline comment says 'Same split-and-guard shape as repo-clone.ts's normalizeRepoFullName' but repo-clone.ts lives in the miner package and isn't imported here — consider rewording to avoid implying a direct code relationship.
• The new packages/loopover-engine/test/repo-segment.test.ts imports from '../dist/index.js' rather than source, so it only validates the built artifact — confirm this matches the existing convention for that test directory rather than being an oversight.
• Nice touch keeping REPO_SEGMENT_PATTERN and isValidRepoSegment as the only two exports from repo-segment.ts, matching the PR description's stated scope.
• Consider a follow-up doc note in governor-ledger.ts's write-path comment (line ~43) referencing the new shared module location, since the historical fix(miner): several owner/repo CLI parsers skip repo-clone.js's path-safety validation #5831/fix(engine): extend #5831's repo-segment path-safety validation to governor/portfolio/worktree stores #7525/governor-ledger.ts's engine-side normalizeOptionalRepoFullName skips the #7525 path-safety guard its own miner-lib sibling has #8350 comment trail now spans two files.

Decision drivers

• ✅ Code review — No blockers (1 reviewer)
• ✅ Gate result — Passing (No configured blocker found.)

Context & advisory signals — never blocks the verdict

Signal	Result	Evidence
Linked issue	✅ Linked	#9610, #5831
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (2 linked issues).
Validation posture	❌ 5/25	Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload	✅ 10/10	Author activity: 52 registered-repo PR(s), 27 merged, 3 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	rsnetworkinginc; Gittensor profile; 52 PR(s), 3 issue(s).
Improvement	✅ Minor	risk: clean · value: minor · LLM: moderate

Linked issue satisfaction

Addressed
The PR adds the shared repo-segment.ts module with the exact byte-for-byte lift from governor-ledger.ts, re-exports it from index.ts, and applies isValidRepoSegment via the split-and-guard pattern to all three sibling normalizers (idea-intake.ts, discovery-index-contract.ts, discovery-soft-claim.ts) plus removes the duplicate copy from governor-ledger.ts, matching every stated requirement.

Review context

• Author: rsnetworkinginc
• Role context: outside_contributor
• Public audience mode: oss maintainer
• Lane context: Repository is registered but has no active allocation in the current snapshot.
• Public profile languages: TypeScript
• Official Gittensor activity: 52 PR(s), 3 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Start here: Await review-lane availability.
• Then work through the remaining 2 steps in the Signals table above.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
• Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
• Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

• @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
• @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
• A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

---

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

• Re-run LoopOver review

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.08%. Comparing base (bd139a5) to head (d2f9a8e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9625      +/-   ##
==========================================
+ Coverage   90.02%   90.08%   +0.05%     
==========================================
  Files         888      889       +1     
  Lines      111983   112001      +18     
  Branches    26570    26586      +16     
==========================================
+ Hits       100810   100891      +81     
+ Misses       9843     9776      -67     
- Partials     1330     1334       +4     

Flag	Coverage Δ
backend	95.56% <100.00%> (-0.01%)	⬇️
engine	66.87% <100.00%> (+0.31%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...es/loopover-engine/src/discovery-index-contract.ts	92.77% <100.00%> (+11.69%)	⬆️
...ckages/loopover-engine/src/discovery-soft-claim.ts	100.00% <100.00%> (+16.84%)	⬆️
packages/loopover-engine/src/governor-ledger.ts	100.00% <100.00%> (ø)
packages/loopover-engine/src/idea-intake.ts	84.21% <100.00%> (+7.44%)	⬆️
packages/loopover-engine/src/index.ts	100.00% <100.00%> (ø)
packages/loopover-engine/src/repo-segment.ts	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

[loopover-orb]

LoopOver approves — the gate is satisfied and CI is green.