Skip to content

fix(ui): reject malformed owner/repo strings in splitRepoFullName - #7818

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
jaytbarimbao-collab:fix-splitrepo-guard-7783
Jul 21, 2026
Merged

fix(ui): reject malformed owner/repo strings in splitRepoFullName#7818
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
jaytbarimbao-collab:fix-splitrepo-guard-7783

Conversation

@jaytbarimbao-collab

Copy link
Copy Markdown
Contributor

Closes #7783

splitRepoFullName is the sole validity gate for the free-text Repository input across 8 panels. It destructured [owner, repo, extra] and rejected only when extra was truthy — which inspects just the 3rd /-segment. Any input whose 3rd segment is empty slipped through as a truncated pair:

splitRepoFullName("acme/repo/")         -> { owner: 'acme', repo: 'repo' }   // trailing slash accepted
splitRepoFullName("acme/repo//x")       -> { owner: 'acme', repo: 'repo' }   // 4th segment dropped
splitRepoFullName("owner/repo//stale")  -> { owner: 'owner', repo: 'repo' }  // pasted stale-copy accepted

In ams-miner-cohort-card.tsx and ai-review-settings.tsx the result feeds straight into encodeURIComponent(owner)/encodeURIComponent(repo) to build an API path, so a pasted owner/repo//stale-copy silently queried the wrong repo instead of showing the "Enter a repository as owner/repo" validation message.

Fix: validate the segment count explicitly (parts.length !== 2) so anything with more or fewer than two non-empty segments is rejected. The return shape and every caller are unchanged.

Tests: adds maintainer-settings-preview.test.ts (the file had no direct tests despite 7 panel importers) — splitRepoFullName's valid / trailing-slash / double-slash / empty-segment / stale-copy cases, plus baseline coverage for the file's other exported helpers (splitReviewabilityPr, extractPreviewRepoOptions, parsePreviewLabels, parseLinkedIssues, findPreviewScenario, buildSettingsPreviewRequest). Verified locally: eslint + prettier clean, tsc --noEmit clean, 20/20 tests pass. (apps/loopover-ui is outside the src/** 99% patch gate; the test file is this issue's own deliverable.)

splitRepoFullName gated the free-text Repository input across 8 panels by
destructuring `[owner, repo, extra]` and rejecting only when `extra` was
truthy. That inspected just the 3rd `/`-segment, so any input whose 3rd
segment was empty slipped through as a truncated pair -- "owner/repo/",
"owner/repo//x", and pastes like "owner/repo//stale-copy" all resolved to
{owner, repo} and were silently queried instead of being rejected with the
usual "owner/repo" validation message.

Validate the segment count explicitly (`parts.length !== 2`) so anything
with more or fewer than two non-empty segments is rejected. Return shape
and all callers are unchanged.

Adds maintainer-settings-preview.test.ts (the file had no direct tests):
splitRepoFullName's valid/trailing-slash/double-slash/empty-segment cases
plus baseline coverage for the file's other exported helpers.

Closes JSONbored#7783
@superagent-security

Copy link
Copy Markdown
Contributor

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

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 21, 2026
@loopover-orb

loopover-orb Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-21 13:34:18 UTC

2 files · 1 AI reviewer · no blockers · readiness 95/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
Fixes a real validation gap in splitRepoFullName: the old code only checked truthiness of the 3rd split segment, so inputs like "acme/repo/" or "owner/repo//stale" split to a 3rd segment of "" and slipped through as valid. The new parts.length !== 2 check correctly rejects any input with more or fewer than two segments, verified against all the cases in the PR description. The fix is minimal, in the right layer (the sole validity gate), and the return shape is unchanged so callers are unaffected.

Nits — 4 non-blocking
  • The PR description claims maintainer-settings-preview.ts "had no direct tests despite 7 panel importers," but test/unit/maintainer-settings-preview-ui.test.ts already imports and tests splitRepoFullName (and the other exported helpers) with near-identical cases — the new apps/loopover-ui/src/lib/maintainer-settings-preview.test.ts largely duplicates that coverage rather than filling a real gap.
  • The inline comment above splitRepoFullName in maintainer-settings-preview.ts is a bit long for a one-line explanation of a straightforward length check — consider trimming it to the key insight (old code only inspected the 3rd segment) and dropping the enumerated examples, which are already in the PR description.
  • Since test/unit/maintainer-settings-preview-ui.test.ts already exercises this module end-to-end, consider consolidating rather than maintaining two near-duplicate test files for the same source file going forward.
  • Confirm the two test files aren't both counted toward coverage in a way that overstates new coverage added by this PR.

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 #7783
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 (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 151 registered-repo PR(s), 73 merged, 16 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jaytbarimbao-collab; Gittensor profile; 151 PR(s), 16 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The diff replaces the destructuring check with an explicit `parts.length !== 2` validation as required, correctly rejecting trailing slashes, double slashes, and empty segments while preserving the return shape and callers, and adds the requested test file with coverage of the specified cases plus the other exported helpers.

Review context
  • Author: jaytbarimbao-collab
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Rust
  • Official Gittensor activity: 151 PR(s), 16 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
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

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@loopover-orb
loopover-orb Bot merged commit ead55ca into JSONbored:main Jul 21, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

splitRepoFullName silently accepts malformed owner/repo strings with extra path segments

1 participant