Skip to content

pr-actions.ts splitRepo misses per-segment whitespace padding (owner/ repo slips through) #6613

Description

@JSONbored

Context

splitRepo in src/github/pr-actions.ts:23-32 validates the repoFullName shared by every write primitive
in that file (createPullRequestReview, mergePullRequest, dismissLatestBotApproval, closePullRequest,
closeIssue, etc.):

function splitRepo(repoFullName: string): { owner: string; repo: string } {
  if (repoFullName !== repoFullName.trim()) {
    throw new Error(`Invalid repository full name: ${repoFullName}`);
  }
  const parts = repoFullName.split("/");
  if (parts.length !== 2 || !parts[0] || !parts[1]) {
    throw new Error(`Invalid repository full name: ${repoFullName}`);
  }
  return { owner: parts[0], repo: parts[1] };
}

The repoFullName !== repoFullName.trim() check only catches leading/trailing whitespace on the WHOLE
string. It does not catch whitespace padding INSIDE one segment. For input "owner/ repo": the whole-string
trim check passes (no leading/trailing space on the full string), .split("/") yields
["owner", " repo"], parts.length === 2 and both elements are truthy (" repo" is a non-empty string) —
so validation passes and " repo" (with its leading space) is sent to GitHub as the repo path parameter.

The sibling validator in the same subsystem, parseRepoFullName in src/github/assignees.ts:9-19 /
src/github/labels.ts:9-19, was hardened against exactly this case. Its comment names the scenario directly:

// Reject any whitespace (leading, trailing, or per-segment like `owner/ repo`) so a padded slug can never
// reach a GitHub call — a valid owner/repo name never contains spaces.
if (parts.length !== 2 || !owner || !repo || /\s/.test(repoFullName)) {

test/unit/github-assignees.test.ts:21-23 proves the fix works there — it iterates
[" owner/repo ", "owner/ repo", "owner /repo"] and asserts every one throws. pr-actions.ts's own test,
test/unit/github-pr-actions.test.ts:16-33, only covers "invalid", "owner/repo/extra", and
" owner/repo " (whole-string padding) — it never exercises "owner/ repo" or "owner /repo"
(per-segment padding), so the gap has no regression coverage today.

Requirements

  • splitRepo in src/github/pr-actions.ts:23-32 MUST reject any repoFullName containing whitespace
    ANYWHERE in the string (leading, trailing, or internal to either segment), not just leading/trailing on the
    whole string. Replace or supplement the current repoFullName !== repoFullName.trim() check with a
    whitespace test covering the whole string, matching parseRepoFullName's /\s/.test(repoFullName) check
    in src/github/assignees.ts:9-19 / src/github/labels.ts:9-19.
  • Every one of splitRepo's existing callers in src/github/pr-actions.ts (createPullRequestReview,
    createPullRequestReviewComments, mergePullRequest, dismissLatestBotApproval, updatePullRequestBranch,
    listPullRequestCommitMessages, createIssueComment, closePullRequest, reopenPullRequest, closeIssue,
    getLastActorForEvent) must continue to throw the same Invalid repository full name: ${repoFullName}
    message shape unchanged — this is a stricter input-rejection fix, not a message-format change.

Deliverables

  • splitRepo in src/github/pr-actions.ts rejects "owner/ repo", "owner /repo", and any other
    internally-whitespace-padded repoFullName, not just whole-string leading/trailing padding.
  • test/unit/github-pr-actions.test.ts's "validates the repo name before any GitHub call" test
    (currently test/unit/github-pr-actions.test.ts:16-33) extended to assert "owner/ repo" and
    "owner /repo" are rejected, mirroring the precedent list already used in
    test/unit/github-assignees.test.ts:21-23.

Test Coverage Requirements

This change touches src/github/pr-actions.ts and test/unit/github-pr-actions.test.ts, both under this
repo's src/** Codecov patch gate (99%+ patch coverage, hard gate). The new/expanded whitespace check must
be exercised by the extended test cases so the patch is fully covered.

Expected Outcome

Every write primitive in src/github/pr-actions.ts rejects an internally-whitespace-padded repoFullName
before it reaches a GitHub API call, closing the same class of bug parseRepoFullName
(src/github/assignees.ts / src/github/labels.ts) was already hardened against — a padded segment can
never again be sent to GitHub as part of an owner/repo path parameter from this file.

Links & Resources

  • src/github/pr-actions.ts:23-32 (splitRepo, the function to fix)
  • src/github/assignees.ts:9-19 / src/github/labels.ts:9-19 (parseRepoFullName, the whitespace-rejection
    pattern to mirror, with its explicit owner/ repo comment)
  • test/unit/github-assignees.test.ts:21-23 (existing precedent test covering this exact padding shape)
  • test/unit/github-pr-actions.test.ts:16-33 (the existing, incomplete test to extend)

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