Skip to content

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

Description

@JSONbored

Context

splitRepoFullName (apps/loopover-ui/src/lib/maintainer-settings-preview.ts:118-122) is the sole validity gate for the free-text "Repository" input across 8 call sites (owner-panel.tsx:45, onboarding-preview-card.tsx:30,69, maintainer-settings.tsx:121, ams-miner-cohort-card.tsx:26, ai-review-settings.tsx:34, maintainer-panel.tsx:511,529, activation-preview.tsx:44, and registration-workspace.ts:2 re-export):

export function splitRepoFullName(repoFullName: string): { owner: string; repo: string } | null {
  const [owner, repo, extra] = repoFullName.trim().split("/");
  if (!owner || !repo || extra) return null;
  return { owner, repo };
}

Destructuring only inspects the 3rd segment. Verified in Node:

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

Any input whose 3rd /-segment happens to be empty (two consecutive slashes) bypasses the extra truthiness check regardless of what follows. In ams-miner-cohort-card.tsx:28 and ai-review-settings.tsx:36, the result feeds directly into encodeURIComponent(target.owner)}/${encodeURIComponent(target.repo)} to build an API path - so a pasted repo string like owner/repo//stale-copy is treated as the valid repo owner/repo instead of being rejected, silently querying the wrong thing instead of showing the "Enter a repository as owner/repo" validation message the callers otherwise rely on.

Additionally, the whole file (182 lines: splitRepoFullName, splitReviewabilityPr, parsePreviewLabels, parseLinkedIssues, buildSettingsPreviewRequest, extractPreviewRepoOptions) has zero direct test coverage, despite being imported by 7 different panel components. Its sibling maintainer-settings-editable.ts does have maintainer-settings-editable.test.ts.

Requirements

Replace the destructuring-based check with an explicit parts.length !== 2 validation, so any input with more than 2 /-separated segments (including ones with an empty middle segment) is correctly rejected. Do not change the function's return shape or any caller.

Deliverables

  • splitRepoFullName in apps/loopover-ui/src/lib/maintainer-settings-preview.ts rejects any input that doesn't split into exactly 2 non-empty segments.
  • New test file maintainer-settings-preview.test.ts (none exists today) covering splitRepoFullName at minimum: valid owner/repo, trailing slash, double-slash, and empty-segment cases - plus baseline coverage for the file's other exported functions if practical.

Test Coverage Requirements

apps/loopover-ui is not covered by the src/** 99% patch gate - the new test file is this issue's own deliverable, not a separate requirement.

Expected Outcome

A malformed repo string with extra or empty path segments is rejected with the existing validation message instead of silently resolving to a truncated/wrong owner+repo pair.

Links & Resources

apps/loopover-ui/src/lib/maintainer-settings-preview.ts:118-122, call sites in owner-panel.tsx, onboarding-preview-card.tsx, maintainer-settings.tsx, ams-miner-cohort-card.tsx, ai-review-settings.tsx, maintainer-panel.tsx, activation-preview.tsx

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