diff --git a/.changeset/patch-fix-cross-repo-safe-output-handlers.md b/.changeset/patch-fix-cross-repo-safe-output-handlers.md new file mode 100644 index 0000000000..eaaeea2347 --- /dev/null +++ b/.changeset/patch-fix-cross-repo-safe-output-handlers.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Ensure remove_labels and assign_to_user safe-output handlers honor target-repo and allowed_repos configs. diff --git a/actions/setup/js/assign_to_user.cjs b/actions/setup/js/assign_to_user.cjs index 63bc4b96de..0a7226137e 100644 --- a/actions/setup/js/assign_to_user.cjs +++ b/actions/setup/js/assign_to_user.cjs @@ -7,6 +7,7 @@ const { processItems } = require("./safe_output_processor.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); +const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); /** @type {string} Safe output type handled by this module */ const HANDLER_TYPE = "assign_to_user"; @@ -20,11 +21,16 @@ async function main(config = {}) { // Extract configuration const allowedAssignees = config.allowed || []; const maxCount = config.max || 10; + const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); core.info(`Assign to user configuration: max=${maxCount}`); if (allowedAssignees.length > 0) { core.info(`Allowed assignees: ${allowedAssignees.join(", ")}`); } + core.info(`Default target repo: ${defaultTargetRepo}`); + if (allowedRepos.size > 0) { + core.info(`Allowed repos: ${Array.from(allowedRepos).join(", ")}`); + } // Track how many items we've processed for max limit let processedCount = 0; @@ -47,6 +53,18 @@ async function main(config = {}) { processedCount++; + // Resolve and validate target repository + const repoResult = resolveAndValidateRepo(message, defaultTargetRepo, allowedRepos, "assignee"); + if (!repoResult.success) { + core.warning(`Skipping assign_to_user: ${repoResult.error}`); + return { + success: false, + error: repoResult.error, + }; + } + const { repo: itemRepo, repoParts } = repoResult; + core.info(`Target repository: ${itemRepo}`); + const assignItem = message; // Determine issue number @@ -96,18 +114,18 @@ async function main(config = {}) { }; } - core.info(`Assigning ${uniqueAssignees.length} users to issue #${issueNumber}: ${JSON.stringify(uniqueAssignees)}`); + core.info(`Assigning ${uniqueAssignees.length} users to issue #${issueNumber} in ${itemRepo}: ${JSON.stringify(uniqueAssignees)}`); try { // Add assignees to the issue await github.rest.issues.addAssignees({ - owner: context.repo.owner, - repo: context.repo.repo, + owner: repoParts.owner, + repo: repoParts.repo, issue_number: issueNumber, assignees: uniqueAssignees, }); - core.info(`Successfully assigned ${uniqueAssignees.length} user(s) to issue #${issueNumber}`); + core.info(`Successfully assigned ${uniqueAssignees.length} user(s) to issue #${issueNumber} in ${itemRepo}`); return { success: true, diff --git a/actions/setup/js/assign_to_user.test.cjs b/actions/setup/js/assign_to_user.test.cjs index 45a83f30c1..9485164aa4 100644 --- a/actions/setup/js/assign_to_user.test.cjs +++ b/actions/setup/js/assign_to_user.test.cjs @@ -238,4 +238,111 @@ describe("assign_to_user (Handler Factory Architecture)", () => { assignees: ["user1", "user2"], }); }); + + it("should support target-repo from config", async () => { + vi.clearAllMocks(); + const { main } = require("./assign_to_user.cjs"); + const targetRepoHandler = await main({ + max: 10, + "target-repo": "external-org/external-repo", + }); + const addAssigneesCalls = []; + + mockGithub.rest.issues.addAssignees = async params => { + addAssigneesCalls.push(params); + return {}; + }; + + const result = await targetRepoHandler( + { + issue_number: 100, + assignees: ["user1"], + }, + {} + ); + + expect(result.success).toBe(true); + expect(addAssigneesCalls[0].owner).toBe("external-org"); + expect(addAssigneesCalls[0].repo).toBe("external-repo"); + }); + + it("should support repo field in message for cross-repository operations", async () => { + vi.clearAllMocks(); + const { main } = require("./assign_to_user.cjs"); + const crossRepoHandler = await main({ + max: 10, + "target-repo": "default-org/default-repo", + allowed_repos: ["cross-org/cross-repo"], + }); + const addAssigneesCalls = []; + + mockGithub.rest.issues.addAssignees = async params => { + addAssigneesCalls.push(params); + return {}; + }; + + const result = await crossRepoHandler( + { + issue_number: 456, + assignees: ["user1"], + repo: "cross-org/cross-repo", + }, + {} + ); + + expect(result.success).toBe(true); + expect(addAssigneesCalls[0].owner).toBe("cross-org"); + expect(addAssigneesCalls[0].repo).toBe("cross-repo"); + }); + + it("should reject repo not in allowed-repos list", async () => { + vi.clearAllMocks(); + const { main } = require("./assign_to_user.cjs"); + const handler = await main({ + max: 10, + "target-repo": "default-org/default-repo", + allowed_repos: ["allowed-org/allowed-repo"], + }); + + const result = await handler( + { + issue_number: 100, + assignees: ["user1"], + repo: "unauthorized-org/unauthorized-repo", + }, + {} + ); + + expect(result.success).toBe(false); + expect(result.error).toContain("not in the allowed-repos list"); + }); + + it("should qualify bare repo name with default repo org", async () => { + vi.clearAllMocks(); + const { main } = require("./assign_to_user.cjs"); + const handler = await main({ + max: 10, + "target-repo": "github/default-repo", + allowed_repos: ["github/gh-aw"], + }); + const addAssigneesCalls = []; + + mockGithub.rest.issues.addAssignees = async params => { + addAssigneesCalls.push(params); + return {}; + }; + + const result = await handler( + { + issue_number: 100, + assignees: ["user1"], + repo: "gh-aw", // Bare repo name + }, + {} + ); + + expect(result.success).toBe(true); + expect(addAssigneesCalls[0].owner).toBe("github"); + expect(addAssigneesCalls[0].repo).toBe("gh-aw"); + }); }); diff --git a/actions/setup/js/remove_labels.cjs b/actions/setup/js/remove_labels.cjs index 294970b8a6..ef8118f9e1 100644 --- a/actions/setup/js/remove_labels.cjs +++ b/actions/setup/js/remove_labels.cjs @@ -10,6 +10,7 @@ const HANDLER_TYPE = "remove_labels"; const { validateLabels } = require("./safe_output_validator.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); +const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); /** * Main handler factory for remove_labels @@ -20,11 +21,16 @@ async function main(config = {}) { // Extract configuration const allowedLabels = config.allowed || []; const maxCount = config.max || 10; + const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); core.info(`Remove labels configuration: max=${maxCount}`); if (allowedLabels.length > 0) { core.info(`Allowed labels to remove: ${allowedLabels.join(", ")}`); } + core.info(`Default target repo: ${defaultTargetRepo}`); + if (allowedRepos.size > 0) { + core.info(`Allowed repos: ${Array.from(allowedRepos).join(", ")}`); + } // Track how many items we've processed for max limit let processedCount = 0; @@ -47,6 +53,18 @@ async function main(config = {}) { processedCount++; + // Resolve and validate target repository + const repoResult = resolveAndValidateRepo(message, defaultTargetRepo, allowedRepos, "label"); + if (!repoResult.success) { + core.warning(`Skipping remove_labels: ${repoResult.error}`); + return { + success: false, + error: repoResult.error, + }; + } + const { repo: itemRepo, repoParts } = repoResult; + core.info(`Target repository: ${itemRepo}`); + // Determine target issue/PR number const itemNumber = message.item_number !== undefined ? parseInt(String(message.item_number), 10) : context.payload?.issue?.number || context.payload?.pull_request?.number; @@ -111,7 +129,7 @@ async function main(config = {}) { }; } - core.info(`Removing ${uniqueLabels.length} labels from ${contextType} #${itemNumber}: ${JSON.stringify(uniqueLabels)}`); + core.info(`Removing ${uniqueLabels.length} labels from ${contextType} #${itemNumber} in ${itemRepo}: ${JSON.stringify(uniqueLabels)}`); // Track successfully removed labels const removedLabels = []; @@ -121,17 +139,18 @@ async function main(config = {}) { for (const label of uniqueLabels) { try { await github.rest.issues.removeLabel({ - ...context.repo, + owner: repoParts.owner, + repo: repoParts.repo, issue_number: itemNumber, name: label, }); removedLabels.push(label); - core.info(`Removed label "${label}" from ${contextType} #${itemNumber}`); + core.info(`Removed label "${label}" from ${contextType} #${itemNumber} in ${itemRepo}`); } catch (error) { // Label might not exist on the issue/PR - this is not a failure const errorMessage = getErrorMessage(error); if (errorMessage.includes("Label does not exist") || errorMessage.includes("404")) { - core.info(`Label "${label}" was not present on ${contextType} #${itemNumber}, skipping`); + core.info(`Label "${label}" was not present on ${contextType} #${itemNumber} in ${itemRepo}, skipping`); } else { core.warning(`Failed to remove label "${label}": ${errorMessage}`); failedLabels.push({ label, error: errorMessage }); @@ -140,7 +159,7 @@ async function main(config = {}) { } if (removedLabels.length > 0) { - core.info(`Successfully removed ${removedLabels.length} labels from ${contextType} #${itemNumber}`); + core.info(`Successfully removed ${removedLabels.length} labels from ${contextType} #${itemNumber} in ${itemRepo}`); } return { diff --git a/actions/setup/js/remove_labels.test.cjs b/actions/setup/js/remove_labels.test.cjs index c8b89711b2..0776578f57 100644 --- a/actions/setup/js/remove_labels.test.cjs +++ b/actions/setup/js/remove_labels.test.cjs @@ -430,5 +430,104 @@ describe("remove_labels", () => { expect(removeLabelCalls[0].owner).toBe("test-owner"); expect(removeLabelCalls[0].repo).toBe("test-repo"); }); + + it("should support target-repo from config", async () => { + const handler = await main({ + max: 10, + "target-repo": "external-org/external-repo", + }); + const removeLabelCalls = []; + + mockGithub.rest.issues.removeLabel = async params => { + removeLabelCalls.push(params); + return {}; + }; + + const result = await handler( + { + item_number: 100, + labels: ["bug"], + }, + {} + ); + + expect(result.success).toBe(true); + expect(removeLabelCalls[0].owner).toBe("external-org"); + expect(removeLabelCalls[0].repo).toBe("external-repo"); + }); + + it("should support repo field in message for cross-repository operations", async () => { + const handler = await main({ + max: 10, + "target-repo": "default-org/default-repo", + allowed_repos: ["cross-org/cross-repo"], + }); + const removeLabelCalls = []; + + mockGithub.rest.issues.removeLabel = async params => { + removeLabelCalls.push(params); + return {}; + }; + + const result = await handler( + { + item_number: 456, + labels: ["bug"], + repo: "cross-org/cross-repo", + }, + {} + ); + + expect(result.success).toBe(true); + expect(removeLabelCalls[0].owner).toBe("cross-org"); + expect(removeLabelCalls[0].repo).toBe("cross-repo"); + }); + + it("should reject repo not in allowed-repos list", async () => { + const handler = await main({ + max: 10, + "target-repo": "default-org/default-repo", + allowed_repos: ["allowed-org/allowed-repo"], + }); + + const result = await handler( + { + item_number: 100, + labels: ["bug"], + repo: "unauthorized-org/unauthorized-repo", + }, + {} + ); + + expect(result.success).toBe(false); + expect(result.error).toContain("not in the allowed-repos list"); + }); + + it("should qualify bare repo name with default repo org", async () => { + const handler = await main({ + max: 10, + "target-repo": "github/default-repo", + allowed_repos: ["github/gh-aw"], + }); + const removeLabelCalls = []; + + mockGithub.rest.issues.removeLabel = async params => { + removeLabelCalls.push(params); + return {}; + }; + + const result = await handler( + { + item_number: 100, + labels: ["bug"], + repo: "gh-aw", // Bare repo name + }, + {} + ); + + expect(result.success).toBe(true); + expect(removeLabelCalls[0].owner).toBe("github"); + expect(removeLabelCalls[0].repo).toBe("gh-aw"); + }); }); });