Skip to content

comments.ts repo full-name parser silently truncates a malformed 3-segment repoFullName #6612

Description

@JSONbored

Context

createOrUpdateIssueCommentWithMarker in src/github/comments.ts:44-54 validates its repoFullName
argument like this:

const [owner, repo] = repoFullName.split("/");
if (!owner || !repo) throw new Error(`Invalid repository full name: ${repoFullName}`);

"owner/repo/extra" passes this check: .split("/") produces ["owner", "repo", "extra"], the
destructure takes only the first two elements, both are truthy, so the extra segment is silently dropped
instead of the malformed input being rejected. The resulting GitHub API call is made against owner/repo
even though the caller actually supplied a three-segment string.

Two sibling files in the same directory guard against exactly this case with a shared validation shape,
parseRepoFullName, which explicitly checks the segment count:

  • src/github/assignees.ts:9-19:
    function parseRepoFullName(repoFullName: string): { owner: string; repo: string } {
      const parts = repoFullName.split("/");
      const owner = parts[0];
      const repo = parts[1];
      if (parts.length !== 2 || !owner || !repo || /\s/.test(repoFullName)) {
        throw new Error(`Invalid repository full name: ${repoFullName}`);
      }
      return { owner, repo };
    }
  • src/github/labels.ts:9-19 (identical implementation).

test/unit/github-assignees.test.ts:13-15 already proves this matters in practice: it explicitly asserts
ensurePullRequestAssignee(..., "owner/repo/extra", ...) is rejected with /Invalid repository full name/.
comments.ts has no equivalent assertion — test/unit/github-comments.test.ts:379-381
("rejects invalid repository names before calling GitHub") only exercises the no-slash case ("invalid"),
never the three-segment case, so the gap is untested as well as unfixed.

Requirements

  • createOrUpdateIssueCommentWithMarker in src/github/comments.ts MUST reject any repoFullName that does
    not split into exactly two non-empty segments on "/" — i.e. it must throw
    Invalid repository full name: ${repoFullName} for "owner/repo/extra" exactly as it already does for
    "invalid" (no slash) and "owner/" / "/repo" (an empty segment).
  • Implement this by adding a segment-count check (parts.length !== 2) to the existing validation in
    src/github/comments.ts:53-54, matching the check already present in parseRepoFullName
    (src/github/assignees.ts:9-19 / src/github/labels.ts:9-19). You do not need to import or reuse
    parseRepoFullName itself (it is a module-private, unexported function in both files) — replicating the
    same validation shape inline in comments.ts, consistent with how comments.ts already parses this field
    inline, is sufficient.
  • Do not change the whitespace-handling behavior of comments.ts's parser in this issue — only the missing
    segment-count check described above is in scope. (A separate issue covers per-segment whitespace padding
    in a different file.)
  • The two call sites of createOrUpdateIssueCommentWithMarker
    createOrUpdatePrIntelligenceComment (src/github/comments.ts:22-31) and
    createOrUpdateAgentCommandComment (src/github/comments.ts:33-42) — must continue to propagate this
    thrown error unchanged (no new try/catch around the validation).

Deliverables

  • src/github/comments.ts's repo-name validation (currently src/github/comments.ts:53-54) rejects any
    input that does not split into exactly two non-empty "/"-separated segments.
  • test/unit/github-comments.test.ts extended with a case asserting "owner/repo/extra" is rejected via
    createOrUpdatePrIntelligenceComment(..., "owner/repo/extra", ...), mirroring the existing assertion
    pattern at test/unit/github-comments.test.ts:379-381 and the precedent test at
    test/unit/github-assignees.test.ts:13-15.

Test Coverage Requirements

This change touches src/github/comments.ts and test/unit/github-comments.test.ts, both under this
repo's src/** Codecov patch gate (99%+ patch coverage, hard gate). The new segment-count branch must be
exercised by the new test case so the patch is fully covered, not merely inspected.

Expected Outcome

createOrUpdateIssueCommentWithMarker (and therefore both of its exported wrappers) rejects a malformed
three-segment repoFullName the same way ensurePullRequestAssignee and ensurePullRequestLabel already
do, instead of silently truncating it to the first two segments and issuing a GitHub API call against a repo
the caller never actually specified.

Links & Resources

  • src/github/comments.ts:44-54 (the parser to fix)
  • src/github/assignees.ts:9-19 and src/github/labels.ts:9-19 (parseRepoFullName, the validation shape to match)
  • test/unit/github-assignees.test.ts:13-15 (existing precedent test proving "owner/repo/extra" must be rejected)
  • test/unit/github-comments.test.ts:379-381 (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