From fcbe3a511f3018669c92cd5da963c0a37c55f691 Mon Sep 17 00:00:00 2001 From: ultrahighsuper Date: Fri, 17 Jul 2026 01:45:33 +0900 Subject: [PATCH] fix(github): reject a malformed three-segment repoFullName in comments.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `createOrUpdateIssueCommentWithMarker` validated `repoFullName` with `const [owner, repo] = repoFullName.split("/")` + a truthiness check, so "owner/repo/extra" passed: the destructure silently dropped the extra segment and the GitHub call was issued against "owner/repo" — a repo the caller never specified. Add the missing `parts.length !== 2` segment-count check inline, matching the guard already present in parseRepoFullName (assignees.ts / labels.ts). No-slash and empty-segment inputs are still rejected exactly as before; whitespace handling is intentionally left unchanged (a separate issue covers that in a different file). Extends test/unit/github-comments.test.ts with a case asserting "owner/repo/extra" is rejected via createOrUpdatePrIntelligenceComment, mirroring the existing precedent in github-assignees.test.ts. Closes #6612 --- src/github/comments.ts | 9 +++++++-- test/unit/github-comments.test.ts | 8 ++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/github/comments.ts b/src/github/comments.ts index 779637c4cf..95c22d3441 100644 --- a/src/github/comments.ts +++ b/src/github/comments.ts @@ -50,8 +50,13 @@ async function createOrUpdateIssueCommentWithMarker( marker: string, options: { createIfMissing?: boolean | undefined; mode?: AgentActionMode } = {}, ): Promise<{ id: number; html_url?: string } | null> { - const [owner, repo] = repoFullName.split("/"); - if (!owner || !repo) throw new Error(`Invalid repository full name: ${repoFullName}`); + const parts = repoFullName.split("/"); + const owner = parts[0]; + const repo = parts[1]; + // Reject anything that is not exactly two non-empty segments -- "owner/repo/extra" would otherwise pass + // (the destructure silently drops the extra segment), issuing a call against a repo the caller never + // specified. Matches the segment-count guard in parseRepoFullName (assignees.ts / labels.ts). + if (parts.length !== 2 || !owner || !repo) throw new Error(`Invalid repository full name: ${repoFullName}`); return await withInstallationTokenRetry(env, installationId, async (token) => { // Non-live mode suppresses the comment create/update writes; the GET marker-search probe below still runs. diff --git a/test/unit/github-comments.test.ts b/test/unit/github-comments.test.ts index dc98b246de..21b7d99afd 100644 --- a/test/unit/github-comments.test.ts +++ b/test/unit/github-comments.test.ts @@ -379,6 +379,14 @@ describe("GitHub PR intelligence comments", () => { it("rejects invalid repository names before calling GitHub", async () => { await expect(createOrUpdatePrIntelligenceComment(createTestEnv(), 123, "invalid", 12, "body")).rejects.toThrow(/Invalid repository full name/); }); + + it("rejects a malformed three-segment repository name instead of silently truncating it", async () => { + // "owner/repo/extra" splits into three segments; the extra one must be rejected, not dropped, so no + // GitHub call is ever made against the truncated "owner/repo". + await expect(createOrUpdatePrIntelligenceComment(createTestEnv(), 123, "owner/repo/extra", 12, "body")).rejects.toThrow( + /Invalid repository full name/, + ); + }); }); async function generatePrivateKeyPem(): Promise {