From 9aad920916979dced45b6e5c32a1464726a7badc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 20:57:15 +0000 Subject: [PATCH 1/3] Initial plan From ba99ec9b7c8cc8f97e766c5121bb4dc7d0e47917 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 21:06:22 +0000 Subject: [PATCH 2/3] fix: allow resolve_pull_request_review_thread without triggering PR context when explicit thread_id is provided Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2e5ab53b-1616-429c-9017-f04d662c5d93 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/resolve_pr_review_thread.cjs | 14 +++----- .../js/resolve_pr_review_thread.test.cjs | 32 +++++++++++++++++-- 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/actions/setup/js/resolve_pr_review_thread.cjs b/actions/setup/js/resolve_pr_review_thread.cjs index 0f388a3b2df..1f8dccf229e 100644 --- a/actions/setup/js/resolve_pr_review_thread.cjs +++ b/actions/setup/js/resolve_pr_review_thread.cjs @@ -215,16 +215,12 @@ async function main(config = {}) { } // resolveTarget === "*": any PR in allowed repos — no further PR number check needed } else { - // Default (legacy) mode: scope to triggering PR only + // Default (legacy) mode: scope to triggering PR only when a triggering PR exists if (!triggeringPRNumber) { - core.warning("Cannot resolve review thread: not running in a pull request context"); - return { - success: false, - error: "Cannot resolve review threads outside of a pull request context", - }; - } - - if (threadPRNumber !== triggeringPRNumber) { + // No triggering PR (e.g. schedule/workflow_dispatch trigger) but an explicit thread_id + // was provided and resolved to a specific PR via the API — allow the resolution to proceed + core.info(`No triggering PR context; resolving thread ${threadId} via explicit thread_id (PR #${threadPRNumber})`); + } else if (threadPRNumber !== triggeringPRNumber) { core.warning(`Thread ${threadId} belongs to PR #${threadPRNumber}, not triggering PR #${triggeringPRNumber}`); return { success: false, diff --git a/actions/setup/js/resolve_pr_review_thread.test.cjs b/actions/setup/js/resolve_pr_review_thread.test.cjs index 69d5af7ef9f..112cadb9468 100644 --- a/actions/setup/js/resolve_pr_review_thread.test.cjs +++ b/actions/setup/js/resolve_pr_review_thread.test.cjs @@ -148,7 +148,7 @@ describe("resolve_pr_review_thread", () => { expect(result.error).toContain("not found"); }); - it("should reject when not in a pull request context", async () => { + it("should succeed when not in a pull request context but explicit thread_id is provided", async () => { // Override context to non-PR event (afterEach restores the original payload) global.context.payload = { repository: { html_url: "https://github.com/test-owner/test-repo" }, @@ -164,8 +164,34 @@ describe("resolve_pr_review_thread", () => { const result = await freshHandler(message, {}); - expect(result.success).toBe(false); - expect(result.error).toContain("pull request context"); + // Should succeed: thread_id was explicitly provided and resolved to a PR via the API + expect(result.success).toBe(true); + expect(result.thread_id).toBe("PRRT_kwDOABCD123456"); + expect(result.is_resolved).toBe(true); + }); + + it("should succeed when triggered by a schedule event (no PR context) with explicit thread_id", async () => { + // Simulate a schedule-triggered workflow (no pull_request in payload) + global.context.payload = {}; + + mockGraphqlForThread(77); + + const { main } = require("./resolve_pr_review_thread.cjs"); + const freshHandler = await main({ max: 10 }); + + const message = { + type: "resolve_pull_request_review_thread", + thread_id: "PRRT_kwDOSchedule77", + }; + + const result = await freshHandler(message, {}); + + expect(result.success).toBe(true); + expect(result.thread_id).toBe("PRRT_kwDOSchedule77"); + expect(result.is_resolved).toBe(true); + // Should have made two GraphQL calls: thread lookup + resolve mutation + expect(mockGraphql).toHaveBeenCalledTimes(2); + expect(mockGraphql).toHaveBeenCalledWith(expect.stringContaining("resolveReviewThread"), expect.objectContaining({ threadId: "PRRT_kwDOSchedule77" })); }); it("should fail when thread_id is missing", async () => { From 68d20154f9ea9a021db31baecf8bad22d2b5de4c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 21:43:06 +0000 Subject: [PATCH 3/3] fix: add repo validation in legacy mode for schedule-triggered resolve (security) Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8df6cfba-9708-4766-a959-df1fa83578e4 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/resolve_pr_review_thread.cjs | 27 ++++++++-- .../js/resolve_pr_review_thread.test.cjs | 53 ++++++++++++++++++- 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/actions/setup/js/resolve_pr_review_thread.cjs b/actions/setup/js/resolve_pr_review_thread.cjs index 1f8dccf229e..e31a8b55f0b 100644 --- a/actions/setup/js/resolve_pr_review_thread.cjs +++ b/actions/setup/js/resolve_pr_review_thread.cjs @@ -215,11 +215,30 @@ async function main(config = {}) { } // resolveTarget === "*": any PR in allowed repos — no further PR number check needed } else { - // Default (legacy) mode: scope to triggering PR only when a triggering PR exists + // Default (legacy) mode: always validate thread repo against defaultTargetRepo to stay + // least-privilege, even when there is no triggering PR (e.g. schedule/workflow_dispatch). + if (!threadRepo) { + core.warning(`Unable to determine repository for review thread ${threadId}; refusing to resolve in legacy mode`); + return { + success: false, + error: `Unable to determine repository for review thread ${threadId}`, + }; + } + + const legacyRepoValidation = validateTargetRepo(threadRepo, defaultTargetRepo, allowedRepos); + if (!legacyRepoValidation.valid) { + core.warning(`Thread ${threadId} repository ${threadRepo} is not allowed in legacy mode`); + return { + success: false, + error: legacyRepoValidation.error || `Repository ${threadRepo} is not allowed for this handler`, + }; + } + + // Scope to triggering PR only when a triggering PR exists if (!triggeringPRNumber) { - // No triggering PR (e.g. schedule/workflow_dispatch trigger) but an explicit thread_id - // was provided and resolved to a specific PR via the API — allow the resolution to proceed - core.info(`No triggering PR context; resolving thread ${threadId} via explicit thread_id (PR #${threadPRNumber})`); + // No triggering PR (e.g. schedule/workflow_dispatch trigger), but the thread has been + // resolved to a specific allowed repository via the API — allow the resolution to proceed + core.info(`No triggering PR context; resolving thread ${threadId} via explicit thread_id (PR #${threadPRNumber} in ${threadRepo})`); } else if (threadPRNumber !== triggeringPRNumber) { core.warning(`Thread ${threadId} belongs to PR #${threadPRNumber}, not triggering PR #${triggeringPRNumber}`); return { diff --git a/actions/setup/js/resolve_pr_review_thread.test.cjs b/actions/setup/js/resolve_pr_review_thread.test.cjs index 112cadb9468..b2971378700 100644 --- a/actions/setup/js/resolve_pr_review_thread.test.cjs +++ b/actions/setup/js/resolve_pr_review_thread.test.cjs @@ -194,6 +194,55 @@ describe("resolve_pr_review_thread", () => { expect(mockGraphql).toHaveBeenCalledWith(expect.stringContaining("resolveReviewThread"), expect.objectContaining({ threadId: "PRRT_kwDOSchedule77" })); }); + it("should fail in legacy mode when schedule-triggered and thread repo cannot be determined", async () => { + // Simulate a schedule-triggered workflow (no pull_request in payload) + global.context.payload = {}; + + // GraphQL returns thread with no repository info + mockGraphql.mockImplementation(query => { + if (query.includes("resolveReviewThread")) { + return Promise.resolve({ resolveReviewThread: { thread: { id: "PRRT_x", isResolved: true } } }); + } + return Promise.resolve({ + node: { pullRequest: { number: 10, repository: null } }, + }); + }); + + const { main } = require("./resolve_pr_review_thread.cjs"); + const freshHandler = await main({ max: 10 }); + + const message = { + type: "resolve_pull_request_review_thread", + thread_id: "PRRT_kwDONoRepo", + }; + + const result = await freshHandler(message, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("Unable to determine repository"); + }); + + it("should fail in legacy mode when schedule-triggered and thread belongs to a different repo", async () => { + // Simulate a schedule-triggered workflow (no pull_request in payload) + global.context.payload = {}; + + // Thread belongs to a different repo, not the default context repo + mockGraphqlForThread(10, "other-owner/other-repo"); + + const { main } = require("./resolve_pr_review_thread.cjs"); + const freshHandler = await main({ max: 10 }); + + const message = { + type: "resolve_pull_request_review_thread", + thread_id: "PRRT_kwDOOtherRepo", + }; + + const result = await freshHandler(message, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("other-owner/other-repo"); + }); + it("should fail when thread_id is missing", async () => { const message = { type: "resolve_pull_request_review_thread", @@ -286,9 +335,9 @@ describe("resolve_pr_review_thread", () => { }, }); } - // Lookup succeeds - thread is on triggering PR + // Lookup succeeds - thread is on triggering PR in the default repo return Promise.resolve({ - node: { pullRequest: { number: 42 } }, + node: { pullRequest: { number: 42, repository: { nameWithOwner: "test-owner/test-repo" } } }, }); });