diff --git a/.changeset/patch-fix-safe-output-cross-repo-auth.md b/.changeset/patch-fix-safe-output-cross-repo-auth.md new file mode 100644 index 0000000000..5315fd1d39 --- /dev/null +++ b/.changeset/patch-fix-safe-output-cross-repo-auth.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Fixed safe-output handler schemas and wiring so cross-repo properties (`target-repo`, `allowed-repos`) and handler auth config (including `github-token`) are available everywhere. diff --git a/.github/smoke-test-claude-22526414043.md b/.github/smoke-test-claude-22526414043.md new file mode 100644 index 0000000000..638f942f45 --- /dev/null +++ b/.github/smoke-test-claude-22526414043.md @@ -0,0 +1,3 @@ +# Smoke Test File + +Created by smoke test run 22526414043 for Claude engine validation. diff --git a/actions/setup/js/add_comment.cjs b/actions/setup/js/add_comment.cjs index e9d17ebaab..2cf3b0fa46 100644 --- a/actions/setup/js/add_comment.cjs +++ b/actions/setup/js/add_comment.cjs @@ -13,6 +13,7 @@ const { getErrorMessage } = require("./error_helpers.cjs"); const { parseBoolTemplatable } = require("./templatable.cjs"); const { resolveTarget } = require("./safe_output_helpers.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { getMissingInfoSections } = require("./missing_messages_helper.cjs"); const { getMessages } = require("./messages_core.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); @@ -302,6 +303,10 @@ async function main(config = {}) { const maxCount = config.max || 20; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); + // Create an authenticated GitHub client. Uses config["github-token"] when set + // (for cross-repository operations), otherwise falls back to the step-level github. + const authClient = await createAuthenticatedGitHubClient(config); + // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; @@ -453,7 +458,7 @@ async function main(config = {}) { if (item.item_number !== undefined && item.item_number !== null) { // Explicit item_number: fetch the issue/PR to get its author try { - const { data: issueData } = await github.rest.issues.get({ + const { data: issueData } = await authClient.rest.issues.get({ owner: repoParts.owner, repo: repoParts.repo, issue_number: itemNumber, @@ -556,7 +561,7 @@ async function main(config = {}) { // Hide older comments if enabled AND append-only-comments is not enabled // When append-only-comments is true, we want to keep all comments visible if (hideOlderCommentsEnabled && !appendOnlyComments && workflowId) { - await hideOlderComments(github, repoParts.owner, repoParts.repo, itemNumber, workflowId, isDiscussion); + await hideOlderComments(authClient, repoParts.owner, repoParts.repo, itemNumber, workflowId, isDiscussion); } else if (hideOlderCommentsEnabled && appendOnlyComments) { core.info("Skipping hide-older-comments because append-only-comments is enabled"); } @@ -574,7 +579,7 @@ async function main(config = {}) { } } `; - const queryResult = await github.graphql(discussionQuery, { + const queryResult = await authClient.graphql(discussionQuery, { owner: repoParts.owner, repo: repoParts.repo, number: itemNumber, @@ -585,10 +590,10 @@ async function main(config = {}) { throw new Error(`${ERR_NOT_FOUND}: Discussion #${itemNumber} not found in ${itemRepo}`); } - comment = await commentOnDiscussion(github, repoParts.owner, repoParts.repo, itemNumber, processedBody, null); + comment = await commentOnDiscussion(authClient, repoParts.owner, repoParts.repo, itemNumber, processedBody, null); } else { // Use REST API for issues/PRs - const { data } = await github.rest.issues.createComment({ + const { data } = await authClient.rest.issues.createComment({ owner: repoParts.owner, repo: repoParts.repo, issue_number: itemNumber, @@ -644,7 +649,7 @@ async function main(config = {}) { } } `; - const queryResult = await github.graphql(discussionQuery, { + const queryResult = await authClient.graphql(discussionQuery, { owner: repoParts.owner, repo: repoParts.repo, number: itemNumber, @@ -656,7 +661,7 @@ async function main(config = {}) { } core.info(`Found discussion #${itemNumber}, adding comment...`); - const comment = await commentOnDiscussion(github, repoParts.owner, repoParts.repo, itemNumber, processedBody, null); + const comment = await commentOnDiscussion(authClient, repoParts.owner, repoParts.repo, itemNumber, processedBody, null); core.info(`Created comment on discussion: ${comment.html_url}`); diff --git a/actions/setup/js/add_labels.cjs b/actions/setup/js/add_labels.cjs index a136c20859..5124014daa 100644 --- a/actions/setup/js/add_labels.cjs +++ b/actions/setup/js/add_labels.cjs @@ -13,6 +13,7 @@ const { getErrorMessage } = require("./error_helpers.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); /** * Maximum limits for label parameters to prevent resource exhaustion. @@ -32,6 +33,7 @@ async function main(config = {}) { const blockedPatterns = config.blocked || []; const maxCount = config.max || 10; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); + const authClient = await createAuthenticatedGitHubClient(config); // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; @@ -161,7 +163,7 @@ async function main(config = {}) { } try { - await github.rest.issues.addLabels({ + await authClient.rest.issues.addLabels({ owner: repoParts.owner, repo: repoParts.repo, issue_number: itemNumber, diff --git a/actions/setup/js/add_reviewer.cjs b/actions/setup/js/add_reviewer.cjs index aecc015f98..23c9d33654 100644 --- a/actions/setup/js/add_reviewer.cjs +++ b/actions/setup/js/add_reviewer.cjs @@ -9,6 +9,7 @@ const { processItems } = require("./safe_output_processor.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { getPullRequestNumber } = require("./pr_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); // GitHub Copilot reviewer bot username const COPILOT_REVIEWER_BOT = "copilot-pull-request-reviewer[bot]"; @@ -25,6 +26,7 @@ async function main(config = {}) { // Extract configuration const allowedReviewers = config.allowed || []; const maxCount = config.max || 10; + const authClient = await createAuthenticatedGitHubClient(config); // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; @@ -106,7 +108,7 @@ async function main(config = {}) { // Add non-copilot reviewers first if (otherReviewers.length > 0) { - await github.rest.pulls.requestReviewers({ + await authClient.rest.pulls.requestReviewers({ owner: context.repo.owner, repo: context.repo.repo, pull_number: prNumber, @@ -118,7 +120,7 @@ async function main(config = {}) { // Add copilot reviewer separately if requested if (hasCopilot) { try { - await github.rest.pulls.requestReviewers({ + await authClient.rest.pulls.requestReviewers({ owner: context.repo.owner, repo: context.repo.repo, pull_number: prNumber, diff --git a/actions/setup/js/assign_milestone.cjs b/actions/setup/js/assign_milestone.cjs index 0d4ecd8ff4..f48267869d 100644 --- a/actions/setup/js/assign_milestone.cjs +++ b/actions/setup/js/assign_milestone.cjs @@ -7,6 +7,7 @@ const { getErrorMessage } = require("./error_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); /** @type {string} Safe output type handled by this module */ const HANDLER_TYPE = "assign_milestone"; @@ -20,6 +21,7 @@ async function main(config = {}) { // Extract configuration const allowedMilestones = config.allowed || []; const maxCount = config.max || 10; + const authClient = await createAuthenticatedGitHubClient(config); // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; @@ -77,7 +79,7 @@ async function main(config = {}) { // Fetch milestones if needed and not already cached if (allowedMilestones.length > 0 && allMilestones === null) { try { - const milestonesResponse = await github.rest.issues.listMilestones({ + const milestonesResponse = await authClient.rest.issues.listMilestones({ owner: context.repo.owner, repo: context.repo.repo, state: "all", @@ -133,7 +135,7 @@ async function main(config = {}) { }; } - await github.rest.issues.update({ + await authClient.rest.issues.update({ owner: context.repo.owner, repo: context.repo.repo, issue_number: issueNumber, diff --git a/actions/setup/js/assign_to_user.cjs b/actions/setup/js/assign_to_user.cjs index b785d46113..ed569038fa 100644 --- a/actions/setup/js/assign_to_user.cjs +++ b/actions/setup/js/assign_to_user.cjs @@ -11,6 +11,7 @@ const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_help const { resolveIssueNumber, extractAssignees } = require("./safe_output_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); const { parseBoolTemplatable } = require("./templatable.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); /** @type {string} Safe output type handled by this module */ const HANDLER_TYPE = "assign_to_user"; @@ -27,6 +28,7 @@ async function main(config = {}) { const maxCount = config.max || 10; const unassignFirst = parseBoolTemplatable(config.unassign_first, false); const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); + const authClient = await createAuthenticatedGitHubClient(config); // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; @@ -131,7 +133,7 @@ async function main(config = {}) { // If unassign_first is enabled, get current assignees and remove them first if (unassignFirst) { core.info(`Fetching current assignees for issue #${issueNumber} to unassign them first`); - const issue = await github.rest.issues.get({ + const issue = await authClient.rest.issues.get({ owner: repoParts.owner, repo: repoParts.repo, issue_number: issueNumber, @@ -140,7 +142,7 @@ async function main(config = {}) { const currentAssignees = issue.data.assignees?.map(a => a.login) || []; if (currentAssignees.length > 0) { core.info(`Unassigning ${currentAssignees.length} current assignee(s): ${JSON.stringify(currentAssignees)}`); - await github.rest.issues.removeAssignees({ + await authClient.rest.issues.removeAssignees({ owner: repoParts.owner, repo: repoParts.repo, issue_number: issueNumber, @@ -153,7 +155,7 @@ async function main(config = {}) { } // Add assignees to the issue - await github.rest.issues.addAssignees({ + await authClient.rest.issues.addAssignees({ owner: repoParts.owner, repo: repoParts.repo, issue_number: issueNumber, diff --git a/actions/setup/js/close_discussion.cjs b/actions/setup/js/close_discussion.cjs index 76d37d62c0..982c43dd3c 100644 --- a/actions/setup/js/close_discussion.cjs +++ b/actions/setup/js/close_discussion.cjs @@ -8,6 +8,7 @@ const { getErrorMessage } = require("./error_helpers.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { ERR_NOT_FOUND } = require("./error_codes.cjs"); /** @@ -159,6 +160,7 @@ async function main(config = {}) { const requiredLabels = config.required_labels || []; const requiredTitlePrefix = config.required_title_prefix || ""; const maxCount = config.max || 10; + const authClient = await createAuthenticatedGitHubClient(config); // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; @@ -218,7 +220,7 @@ async function main(config = {}) { try { // Fetch discussion details - const discussion = await getDiscussionDetails(github, context.repo.owner, context.repo.repo, discussionNumber); + const discussion = await getDiscussionDetails(authClient, context.repo.owner, context.repo.repo, discussionNumber); // Validate required labels if configured if (requiredLabels.length > 0) { @@ -266,7 +268,7 @@ async function main(config = {}) { let commentUrl; if (item.body) { const sanitizedBody = sanitizeContent(item.body); - const comment = await addDiscussionComment(github, discussion.id, sanitizedBody); + const comment = await addDiscussionComment(authClient, discussion.id, sanitizedBody); core.info(`Added comment to discussion #${discussionNumber}: ${comment.url}`); commentUrl = comment.url; } @@ -277,7 +279,7 @@ async function main(config = {}) { } else { const reason = item.reason || undefined; core.info(`Closing discussion #${discussionNumber} with reason: ${reason || "none"}`); - const closedDiscussion = await closeDiscussion(github, discussion.id, reason); + const closedDiscussion = await closeDiscussion(authClient, discussion.id, reason); core.info(`Closed discussion #${discussionNumber}: ${closedDiscussion.url}`); } diff --git a/actions/setup/js/close_issue.cjs b/actions/setup/js/close_issue.cjs index 4294858ad0..cfb3ad2673 100644 --- a/actions/setup/js/close_issue.cjs +++ b/actions/setup/js/close_issue.cjs @@ -9,6 +9,7 @@ const { getErrorMessage } = require("./error_helpers.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { ERR_NOT_FOUND } = require("./error_codes.cjs"); /** @@ -87,6 +88,7 @@ async function main(config = {}) { const comment = config.comment || ""; const configStateReason = config.state_reason || "COMPLETED"; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); + const authClient = await createAuthenticatedGitHubClient(config); // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; @@ -200,7 +202,7 @@ async function main(config = {}) { try { // Fetch issue details core.info(`Fetching issue details for #${issueNumber} in ${repoParts.owner}/${repoParts.repo}`); - const issue = await getIssueDetails(github, repoParts.owner, repoParts.repo, issueNumber); + const issue = await getIssueDetails(authClient, repoParts.owner, repoParts.repo, issueNumber); core.info(`Issue #${issueNumber} fetched: state=${issue.state}, title="${issue.title}", labels=[${issue.labels.map(l => l.name || l).join(", ")}]`); // Check if already closed - but still add comment @@ -252,7 +254,7 @@ async function main(config = {}) { // Add comment with the body from the message core.info(`Adding comment to issue #${issueNumber}: length=${commentToPost.length}`); - const commentResult = await addIssueComment(github, repoParts.owner, repoParts.repo, issueNumber, commentToPost); + const commentResult = await addIssueComment(authClient, repoParts.owner, repoParts.repo, issueNumber, commentToPost); core.info(`✓ Comment posted to issue #${issueNumber}: ${commentResult.html_url}`); core.info(`Comment details: id=${commentResult.id}, body_length=${commentToPost.length}`); @@ -265,7 +267,7 @@ async function main(config = {}) { // Use item-level state_reason if provided, otherwise fall back to config-level default const stateReason = item.state_reason || configStateReason; core.info(`Closing issue #${issueNumber} in ${itemRepo} with state_reason=${stateReason}`); - closedIssue = await closeIssue(github, repoParts.owner, repoParts.repo, issueNumber, stateReason); + closedIssue = await closeIssue(authClient, repoParts.owner, repoParts.repo, issueNumber, stateReason); core.info(`✓ Issue #${issueNumber} closed successfully: ${closedIssue.html_url}`); } diff --git a/actions/setup/js/close_pull_request.cjs b/actions/setup/js/close_pull_request.cjs index 4a64ba512c..89709d7233 100644 --- a/actions/setup/js/close_pull_request.cjs +++ b/actions/setup/js/close_pull_request.cjs @@ -6,6 +6,7 @@ const { getTrackerID } = require("./get_tracker_id.cjs"); const { generateFooterWithMessages } = require("./messages_footer.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { ERR_NOT_FOUND } = require("./error_codes.cjs"); /** @@ -85,6 +86,7 @@ async function main(config = {}) { const requiredTitlePrefix = config.required_title_prefix || ""; const maxCount = config.max || 10; const comment = config.comment || ""; + const authClient = await createAuthenticatedGitHubClient(config); // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; @@ -185,7 +187,7 @@ async function main(config = {}) { let pr; try { core.info(`Fetching PR details for #${prNumber} in ${owner}/${repo}`); - pr = await getPullRequestDetails(github, owner, repo, prNumber); + pr = await getPullRequestDetails(authClient, owner, repo, prNumber); core.info(`PR #${prNumber} fetched: state=${pr.state}, title="${pr.title}", labels=[${pr.labels.map(l => l.name || l).join(", ")}]`); } catch (error) { const errorMsg = getErrorMessage(error); @@ -247,7 +249,7 @@ async function main(config = {}) { const triggeringIssueNumber = context.payload?.issue?.number; const commentBody = buildCommentBody(commentToPost, triggeringIssueNumber, triggeringPRNumber); core.info(`Adding comment to PR #${prNumber}: length=${commentBody.length}`); - await addPullRequestComment(github, owner, repo, prNumber, commentBody); + await addPullRequestComment(authClient, owner, repo, prNumber, commentBody); commentPosted = true; core.info(`✓ Comment posted to PR #${prNumber}`); core.info(`Comment details: body_length=${commentBody.length}`); @@ -273,7 +275,7 @@ async function main(config = {}) { } else { try { core.info(`Closing PR #${prNumber}`); - closedPR = await closePullRequest(github, owner, repo, prNumber); + closedPR = await closePullRequest(authClient, owner, repo, prNumber); core.info(`✓ PR #${prNumber} closed successfully: ${closedPR.title}`); } catch (error) { const errorMsg = getErrorMessage(error); diff --git a/actions/setup/js/create_discussion.cjs b/actions/setup/js/create_discussion.cjs index 643a2fbb34..3f2299e801 100644 --- a/actions/setup/js/create_discussion.cjs +++ b/actions/setup/js/create_discussion.cjs @@ -12,6 +12,7 @@ const { getTrackerID } = require("./get_tracker_id.cjs"); const { sanitizeTitle, applyTitlePrefix } = require("./sanitize_title.cjs"); const { generateTemporaryId, isTemporaryId, normalizeTemporaryId, getOrGenerateTemporaryId, replaceTemporaryIdReferences } = require("./temporary_id.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { removeDuplicateTitleFromDescription } = require("./remove_duplicate_title.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { createExpirationLine, generateFooterWithExpiration } = require("./ephemerals.cjs"); @@ -34,7 +35,7 @@ const MAX_LABELS = 10; * @param {string} repo - Repository name * @returns {Promise<{repositoryId: string, discussionCategories: Array<{id: string, name: string, slug: string, description: string}>}|null>} */ -async function fetchRepoDiscussionInfo(owner, repo) { +async function fetchRepoDiscussionInfo(githubClient, owner, repo) { const repositoryQuery = ` query($owner: String!, $repo: String!) { repository(owner: $owner, name: $repo) { @@ -50,7 +51,7 @@ async function fetchRepoDiscussionInfo(owner, repo) { } } `; - const queryResult = await github.graphql(repositoryQuery, { + const queryResult = await githubClient.graphql(repositoryQuery, { owner: owner, repo: repo, }); @@ -129,7 +130,7 @@ function resolveCategoryId(categoryConfig, itemCategory, categories) { * @param {string[]} labelNames - Array of label names to fetch IDs for * @returns {Promise>} Array of label objects with name and ID */ -async function fetchLabelIds(owner, repo, labelNames) { +async function fetchLabelIds(githubClient, owner, repo, labelNames) { if (!labelNames || labelNames.length === 0) { return []; } @@ -149,7 +150,7 @@ async function fetchLabelIds(owner, repo, labelNames) { } `; - const queryResult = await github.graphql(labelsQuery, { + const queryResult = await githubClient.graphql(labelsQuery, { owner: owner, repo: repo, }); @@ -189,7 +190,7 @@ async function fetchLabelIds(owner, repo, labelNames) { * @param {string[]} labelIds - Array of label node IDs to add * @returns {Promise} True if labels were applied successfully */ -async function applyLabelsToDiscussion(discussionId, labelIds) { +async function applyLabelsToDiscussion(githubClient, discussionId, labelIds) { if (!labelIds || labelIds.length === 0) { return true; // Nothing to do } @@ -215,7 +216,7 @@ async function applyLabelsToDiscussion(discussionId, labelIds) { } `; - const mutationResult = await github.graphql(addLabelsMutation, { + const mutationResult = await githubClient.graphql(addLabelsMutation, { labelableId: discussionId, labelIds: labelIds, }); @@ -310,6 +311,10 @@ async function main(config = {}) { const closeOlderDiscussions = parseBoolTemplatable(config.close_older_discussions, false); const includeFooter = parseBoolTemplatable(config.footer, true); + // Create an authenticated GitHub client. Uses config["github-token"] when set + // (for cross-repository operations), otherwise falls back to the step-level github. + const authClient = await createAuthenticatedGitHubClient(config); + // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; @@ -397,7 +402,7 @@ async function main(config = {}) { let repoInfo = repoInfoCache.get(qualifiedItemRepo); if (!repoInfo) { try { - const fetchedInfo = await fetchRepoDiscussionInfo(repoParts.owner, repoParts.repo); + const fetchedInfo = await fetchRepoDiscussionInfo(authClient, repoParts.owner, repoParts.repo); if (!fetchedInfo) { const error = `Failed to fetch repository information for '${qualifiedItemRepo}'`; core.warning(error); @@ -564,7 +569,7 @@ async function main(config = {}) { } `; - const mutationResult = await github.graphql(createDiscussionMutation, { + const mutationResult = await authClient.graphql(createDiscussionMutation, { repositoryId: repoInfo.repositoryId, categoryId: categoryId, title: title, @@ -586,10 +591,10 @@ async function main(config = {}) { // Apply labels if configured if (discussionLabels.length > 0) { core.info(`Applying ${discussionLabels.length} labels to discussion: ${discussionLabels.join(", ")}`); - const labelIdsData = await fetchLabelIds(repoParts.owner, repoParts.repo, discussionLabels); + const labelIdsData = await fetchLabelIds(authClient, repoParts.owner, repoParts.repo, discussionLabels); if (labelIdsData.length > 0) { const labelIds = labelIdsData.map(l => l.id); - const labelsApplied = await applyLabelsToDiscussion(discussion.id, labelIds); + const labelsApplied = await applyLabelsToDiscussion(authClient, discussion.id, labelIds); if (labelsApplied) { core.info(`✓ Applied labels: ${labelIdsData.map(l => l.name).join(", ")}`); } diff --git a/actions/setup/js/create_issue.cjs b/actions/setup/js/create_issue.cjs index 79b75c53bb..ffae1f4ec3 100644 --- a/actions/setup/js/create_issue.cjs +++ b/actions/setup/js/create_issue.cjs @@ -32,6 +32,7 @@ const { generateWorkflowIdMarker } = require("./generate_footer.cjs"); const { getTrackerID } = require("./get_tracker_id.cjs"); const { generateTemporaryId, isTemporaryId, normalizeTemporaryId, getOrGenerateTemporaryId, replaceTemporaryIdReferences } = require("./temporary_id.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { removeDuplicateTitleFromDescription } = require("./remove_duplicate_title.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { renderTemplate } = require("./messages_core.cjs"); @@ -73,10 +74,10 @@ const MAX_ASSIGNEES = 5; * @param {string} markerComment - The HTML comment marker to search for * @returns {Promise} - Parent issue number or null if none found */ -async function searchForExistingParent(owner, repo, markerComment) { +async function searchForExistingParent(githubClient, owner, repo, markerComment) { try { const searchQuery = `repo:${owner}/${repo} is:issue "${markerComment}" in:body`; - const searchResults = await github.rest.search.issuesAndPullRequests({ + const searchResults = await githubClient.rest.search.issuesAndPullRequests({ q: searchQuery, per_page: MAX_PARENT_ISSUES_TO_CHECK, sort: "created", @@ -119,6 +120,7 @@ async function searchForExistingParent(owner, repo, markerComment) { /** * Finds an existing parent issue for a group, or creates a new one if needed * @param {object} params - Parameters for finding/creating parent issue + * @param {object} params.githubClient - Authenticated GitHub client * @param {string} params.groupId - The group identifier * @param {string} params.owner - Repository owner * @param {string} params.repo - Repository name @@ -129,12 +131,12 @@ async function searchForExistingParent(owner, repo, markerComment) { * @param {number} [params.expiresHours=0] - Hours until expiration (0 means no expiration) * @returns {Promise} - Parent issue number or null if creation failed */ -async function findOrCreateParentIssue({ groupId, owner, repo, titlePrefix, labels, workflowName, workflowSourceURL, expiresHours = 0 }) { +async function findOrCreateParentIssue({ githubClient, groupId, owner, repo, titlePrefix, labels, workflowName, workflowSourceURL, expiresHours = 0 }) { const markerComment = ``; // Search for existing parent issue with the group marker core.info(`Searching for existing parent issue for group: ${groupId}`); - const existingParent = await searchForExistingParent(owner, repo, markerComment); + const existingParent = await searchForExistingParent(githubClient, owner, repo, markerComment); if (existingParent) { return existingParent; } @@ -143,7 +145,7 @@ async function findOrCreateParentIssue({ groupId, owner, repo, titlePrefix, labe core.info(`Creating new parent issue for group: ${groupId}`); try { const template = createParentIssueTemplate(groupId, titlePrefix, workflowName, workflowSourceURL, expiresHours); - const { data: parentIssue } = await github.rest.issues.create({ + const { data: parentIssue } = await githubClient.rest.issues.create({ owner, repo, title: template.title, @@ -214,6 +216,10 @@ async function main(config = {}) { const closeOlderIssuesEnabled = parseBoolTemplatable(config.close_older_issues, false); const includeFooter = parseBoolTemplatable(config.footer, true); + // Create an authenticated GitHub client. Uses config["github-token"] when set + // (for cross-repository operations), otherwise falls back to the step-level github. + const authClient = await createAuthenticatedGitHubClient(config); + // Check if copilot assignment is enabled const assignCopilot = process.env.GH_AW_ASSIGN_COPILOT === "true"; @@ -477,7 +483,7 @@ async function main(config = {}) { } try { - const { data: issue } = await github.rest.issues.create({ + const { data: issue } = await authClient.rest.issues.create({ owner: repoParts.owner, repo: repoParts.repo, title, @@ -533,6 +539,7 @@ async function main(config = {}) { // Parent issue expires 1 day (24 hours) after sub-issues const parentExpiresHours = expiresHours > 0 ? expiresHours + 24 : 0; groupParentNumber = await findOrCreateParentIssue({ + githubClient: authClient, groupId, owner: repoParts.owner, repo: repoParts.repo, @@ -575,7 +582,7 @@ async function main(config = {}) { `; // Get parent issue node ID - const parentResult = await github.graphql(getIssueNodeIdQuery, { + const parentResult = await authClient.graphql(getIssueNodeIdQuery, { owner: repoParts.owner, repo: repoParts.repo, issueNumber: effectiveParentIssueNumber, @@ -585,7 +592,7 @@ async function main(config = {}) { // Get child issue node ID core.info(`Fetching node ID for child issue #${issue.number}...`); - const childResult = await github.graphql(getIssueNodeIdQuery, { + const childResult = await authClient.graphql(getIssueNodeIdQuery, { owner: repoParts.owner, repo: repoParts.repo, issueNumber: issue.number, @@ -609,7 +616,7 @@ async function main(config = {}) { } `; - await github.graphql(addSubIssueMutation, { + await authClient.graphql(addSubIssueMutation, { issueId: parentNodeId, subIssueId: childNodeId, }); @@ -621,7 +628,7 @@ async function main(config = {}) { // Fallback: add a comment if sub-issue linking fails try { core.info(`Attempting fallback: adding comment to parent issue #${effectiveParentIssueNumber}...`); - await github.rest.issues.createComment({ + await authClient.rest.issues.createComment({ owner: repoParts.owner, repo: repoParts.repo, issue_number: effectiveParentIssueNumber, diff --git a/actions/setup/js/create_issue_group.test.cjs b/actions/setup/js/create_issue_group.test.cjs index 38bca262df..b04041974c 100644 --- a/actions/setup/js/create_issue_group.test.cjs +++ b/actions/setup/js/create_issue_group.test.cjs @@ -47,7 +47,7 @@ describe("searchForExistingParent", () => { }); it("should return null when no parent issues found", async () => { - const result = await searchForExistingParent("owner", "repo", ""); + const result = await searchForExistingParent(mockGithub, "owner", "repo", ""); expect(result).toBeNull(); }); @@ -76,7 +76,7 @@ describe("searchForExistingParent", () => { }, }); - const result = await searchForExistingParent("owner", "repo", ""); + const result = await searchForExistingParent(mockGithub, "owner", "repo", ""); expect(result).toBe(42); }); @@ -95,7 +95,7 @@ describe("searchForExistingParent", () => { }, }); - const result = await searchForExistingParent("owner", "repo", ""); + const result = await searchForExistingParent(mockGithub, "owner", "repo", ""); expect(result).toBeNull(); }); @@ -124,7 +124,7 @@ describe("searchForExistingParent", () => { }, }); - const result = await searchForExistingParent("owner", "repo", ""); + const result = await searchForExistingParent(mockGithub, "owner", "repo", ""); expect(result).toBeNull(); }); @@ -155,7 +155,7 @@ describe("searchForExistingParent", () => { }); }); - const result = await searchForExistingParent("owner", "repo", ""); + const result = await searchForExistingParent(mockGithub, "owner", "repo", ""); expect(result).toBe(2); // Should skip closed parent and return first open one }); diff --git a/actions/setup/js/create_pr_review_comment.cjs b/actions/setup/js/create_pr_review_comment.cjs index 651dbddeee..e8970e7887 100644 --- a/actions/setup/js/create_pr_review_comment.cjs +++ b/actions/setup/js/create_pr_review_comment.cjs @@ -8,6 +8,7 @@ const { getErrorMessage } = require("./error_helpers.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); /** @type {string} Safe output type handled by this module */ const HANDLER_TYPE = "create_pull_request_review_comment"; @@ -27,6 +28,7 @@ async function main(config = {}) { const maxCount = config.max || 10; const buffer = config._prReviewBuffer; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); + const authClient = await createAuthenticatedGitHubClient(config); if (!buffer) { core.warning("create_pull_request_review_comment: No PR review buffer provided in config"); @@ -203,7 +205,7 @@ async function main(config = {}) { // If we don't have the full PR details yet, fetch them if (!pullRequest || !pullRequest.head || !pullRequest.head.sha) { try { - const { data: fullPR } = await github.rest.pulls.get({ + const { data: fullPR } = await authClient.rest.pulls.get({ owner: repoParts.owner, repo: repoParts.repo, pull_number: pullRequestNumber, diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index 96840a47e1..49ebdaad06 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -19,6 +19,7 @@ const { generateFooterWithMessages } = require("./messages_footer.cjs"); const { normalizeBranchName } = require("./normalize_branch_name.cjs"); const { pushExtraEmptyCommit } = require("./extra_empty_commit.cjs"); const { getBaseBranch } = require("./get_base_branch.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); /** * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction @@ -102,6 +103,7 @@ async function main(config = {}) { const maxCount = config.max || 1; // PRs are typically limited to 1 const maxSizeKb = config.max_patch_size ? parseInt(String(config.max_patch_size), 10) : 1024; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); + const authClient = await createAuthenticatedGitHubClient(config); // Base branch from config (if set) - validated at factory level if explicit // Dynamic base branch resolution happens per-message after resolving the actual target repo @@ -716,7 +718,7 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${branchName} --repo ${patchPreview}`; try { - const { data: issue } = await github.rest.issues.create({ + const { data: issue } = await authClient.rest.issues.create({ owner: repoParts.owner, repo: repoParts.repo, title: title, @@ -727,7 +729,7 @@ ${patchPreview}`; core.info(`Created fallback issue #${issue.number}: ${issue.html_url}`); // Update the activation comment with issue link (if a comment was created) - await updateActivationComment(github, context, core, issue.html_url, issue.number, "issue"); + await updateActivationComment(authClient, context, core, issue.html_url, issue.number, "issue"); // Write summary to GitHub Actions summary await core.summary @@ -836,7 +838,7 @@ ${patchPreview}`; // Try to create the pull request, with fallback to issue creation try { - const { data: pullRequest } = await github.rest.pulls.create({ + const { data: pullRequest } = await authClient.rest.pulls.create({ owner: repoParts.owner, repo: repoParts.repo, title: title, @@ -850,7 +852,7 @@ ${patchPreview}`; // Add labels if specified if (labels.length > 0) { - await github.rest.issues.addLabels({ + await authClient.rest.issues.addLabels({ owner: repoParts.owner, repo: repoParts.repo, issue_number: pullRequest.number, @@ -862,7 +864,7 @@ ${patchPreview}`; // Enable auto-merge if configured if (autoMerge) { try { - await github.graphql( + await authClient.graphql( `mutation($prId: ID!) { enablePullRequestAutoMerge(input: {pullRequestId: $prId}) { pullRequest { @@ -881,7 +883,7 @@ ${patchPreview}`; } // Update the activation comment with PR link (if a comment was created) - await updateActivationComment(github, context, core, pullRequest.html_url, pullRequest.number); + await updateActivationComment(authClient, context, core, pullRequest.html_url, pullRequest.number); // Write summary to GitHub Actions summary await core.summary @@ -978,7 +980,7 @@ gh pr create --title "${title}" --base ${baseBranch} --head ${branchName} --repo ${patchPreview}`; try { - const { data: issue } = await github.rest.issues.create({ + const { data: issue } = await authClient.rest.issues.create({ owner: repoParts.owner, repo: repoParts.repo, title: title, @@ -989,7 +991,7 @@ ${patchPreview}`; core.info(`Created fallback issue #${issue.number}: ${issue.html_url}`); // Update the activation comment with issue link (if a comment was created) - await updateActivationComment(github, context, core, issue.html_url, issue.number, "issue"); + await updateActivationComment(authClient, context, core, issue.html_url, issue.number, "issue"); // Return success with fallback flag return { diff --git a/actions/setup/js/dispatch_workflow.cjs b/actions/setup/js/dispatch_workflow.cjs index 625df9469b..6dec8d416c 100644 --- a/actions/setup/js/dispatch_workflow.cjs +++ b/actions/setup/js/dispatch_workflow.cjs @@ -9,6 +9,7 @@ const HANDLER_TYPE = "dispatch_workflow"; const { getErrorMessage } = require("./error_helpers.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); /** * Main handler factory for dispatch_workflow @@ -20,6 +21,7 @@ async function main(config = {}) { const allowedWorkflows = config.workflows || []; const maxCount = config.max || 1; const workflowFiles = config.workflow_files || {}; // Map of workflow name to file extension + const authClient = await createAuthenticatedGitHubClient(config); core.info(`Dispatch workflow configuration: max=${maxCount}`); if (allowedWorkflows.length > 0) { @@ -45,7 +47,7 @@ async function main(config = {}) { // Fall back to querying the repository try { - const { data: repoData } = await github.rest.repos.get({ + const { data: repoData } = await authClient.rest.repos.get({ owner: repo.owner, repo: repo.repo, }); @@ -154,7 +156,7 @@ async function main(config = {}) { core.info(`Dispatching workflow: ${workflowFile}`); // Dispatch the workflow using the resolved file - await github.rest.actions.createWorkflowDispatch({ + await authClient.rest.actions.createWorkflowDispatch({ owner: repo.owner, repo: repo.repo, workflow_id: workflowFile, diff --git a/actions/setup/js/handler_auth.cjs b/actions/setup/js/handler_auth.cjs new file mode 100644 index 0000000000..fe5698b712 --- /dev/null +++ b/actions/setup/js/handler_auth.cjs @@ -0,0 +1,53 @@ +// @ts-check +/// + +/** + * handler_auth.cjs + * + * Shared authentication helper for safe-output handlers. + * Provides a consistent way to create authenticated GitHub clients, + * supporting both the step-level token and per-handler tokens for + * cross-repository operations. + * + * Token precedence: + * 1. config["github-token"] — per-handler PAT (for cross-repo operations) + * 2. global github — step-level token set in the github-script with.github-token + * + * The step-level token itself follows (as set by the Go compiler): + * project token > global safe-outputs.github-token > magic secrets + */ + +/** + * Creates an authenticated GitHub client from the handler configuration. + * + * If the handler config contains a "github-token" field, this function creates + * a new Octokit instance authenticated with that token. This enables cross-repository + * operations where a PAT with access to the target repo is required. + * + * If no per-handler token is configured, the global github object is returned + * unchanged (preserving the step-level token from with.github-token). + * + * Usage in handlers: + * const authClient = await createAuthenticatedGitHubClient(config); + * // Use authClient for all GitHub API calls instead of the global github + * const { data } = await authClient.rest.issues.create({ ... }); + * + * @param {Object} config - Handler config object, optionally containing "github-token" + * @returns {Promise} Authenticated GitHub client — an Octokit instance created via getOctokit() + * when a per-handler token is configured, or the global github object (step-level token) otherwise. + * Both return values expose the same API surface (rest, graphql, etc.). + */ +async function createAuthenticatedGitHubClient(config) { + // Note: bracket notation is required because "github-token" contains a hyphen + // (not a valid JavaScript identifier). This is consistent with other hyphenated + // config keys like "target-repo" and "allowed-repos". + const token = config["github-token"]; + if (!token) { + return github; + } + core.info("Using per-handler github-token for cross-repository authentication"); + const { getOctokit } = await import("@actions/github"); + return getOctokit(token); +} + +module.exports = { createAuthenticatedGitHubClient }; diff --git a/actions/setup/js/handler_auth.test.cjs b/actions/setup/js/handler_auth.test.cjs new file mode 100644 index 0000000000..166bc66601 --- /dev/null +++ b/actions/setup/js/handler_auth.test.cjs @@ -0,0 +1,93 @@ +// @ts-check +/// + +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; + +// Mock @actions/github so we can control getOctokit +vi.mock("@actions/github", () => ({ + getOctokit: vi.fn(token => ({ + _token: token, + rest: {}, + graphql: vi.fn(), + })), +})); + +import { createAuthenticatedGitHubClient } from "./handler_auth.cjs"; + +describe("createAuthenticatedGitHubClient", () => { + let mockCore; + let mockGithub; + let originalGlobals; + + beforeEach(() => { + originalGlobals = { + core: global.core, + github: global.github, + }; + + mockCore = { + info: vi.fn(), + warning: vi.fn(), + error: vi.fn(), + }; + + // The global github object (step-level token) + mockGithub = { + _token: "step-level-token", + rest: {}, + graphql: vi.fn(), + }; + + global.core = mockCore; + global.github = mockGithub; + }); + + afterEach(() => { + global.core = originalGlobals.core; + global.github = originalGlobals.github; + vi.clearAllMocks(); + }); + + it("returns the global github when no github-token in config", async () => { + const client = await createAuthenticatedGitHubClient({}); + expect(client).toBe(mockGithub); + expect(mockCore.info).not.toHaveBeenCalled(); + }); + + it("returns the global github when config is empty object", async () => { + const client = await createAuthenticatedGitHubClient({ max: 10, target: "triggering" }); + expect(client).toBe(mockGithub); + }); + + it("creates a new Octokit when github-token is set in config", async () => { + const client = await createAuthenticatedGitHubClient({ "github-token": "my-pat-token" }); + + // Should NOT be the global github + expect(client).not.toBe(mockGithub); + // Should have the token from config + expect(client._token).toBe("my-pat-token"); + }); + + it("logs a message when using per-handler token", async () => { + await createAuthenticatedGitHubClient({ "github-token": "my-pat-token" }); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("per-handler github-token")); + }); + + it("creates distinct Octokit instances for different tokens", async () => { + const client1 = await createAuthenticatedGitHubClient({ "github-token": "token-1" }); + const client2 = await createAuthenticatedGitHubClient({ "github-token": "token-2" }); + + expect(client1._token).toBe("token-1"); + expect(client2._token).toBe("token-2"); + expect(client1).not.toBe(client2); + }); + + it("does not mutate the global github object", async () => { + const originalGithub = global.github; + await createAuthenticatedGitHubClient({ "github-token": "my-pat-token" }); + + // Global should be unchanged + expect(global.github).toBe(originalGithub); + expect(global.github._token).toBe("step-level-token"); + }); +}); diff --git a/actions/setup/js/hide_comment.cjs b/actions/setup/js/hide_comment.cjs index f22bc4be8a..1ea3103340 100644 --- a/actions/setup/js/hide_comment.cjs +++ b/actions/setup/js/hide_comment.cjs @@ -7,6 +7,7 @@ const { getErrorMessage } = require("./error_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); /** * Type constant for handler identification @@ -48,6 +49,7 @@ async function main(config = {}) { // Extract configuration const allowedReasons = config.allowed_reasons || []; const maxCount = config.max || 5; + const authClient = await createAuthenticatedGitHubClient(config); // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; @@ -122,7 +124,7 @@ async function main(config = {}) { }; } - const hideResult = await hideCommentAPI(github, commentId, normalizedReason); + const hideResult = await hideCommentAPI(authClient, commentId, normalizedReason); if (hideResult.isMinimized) { core.info(`Successfully hidden comment: ${commentId}`); diff --git a/actions/setup/js/link_sub_issue.cjs b/actions/setup/js/link_sub_issue.cjs index 16114e395e..4abc332a67 100644 --- a/actions/setup/js/link_sub_issue.cjs +++ b/actions/setup/js/link_sub_issue.cjs @@ -4,6 +4,7 @@ const { loadTemporaryIdMapFromResolved, resolveRepoIssueTarget } = require("./temporary_id.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); /** * Main handler factory for link_sub_issue @@ -18,6 +19,7 @@ async function main(config = {}) { const subRequiredLabels = config.sub_required_labels || []; const subTitlePrefix = config.sub_title_prefix || ""; const maxCount = config.max || 5; + const authClient = await createAuthenticatedGitHubClient(config); // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; @@ -154,7 +156,7 @@ async function main(config = {}) { // Fetch parent issue to validate filters let parentIssue; try { - const parentResponse = await github.rest.issues.get({ + const parentResponse = await authClient.rest.issues.get({ owner, repo, issue_number: parentIssueNumber, @@ -199,7 +201,7 @@ async function main(config = {}) { // Fetch sub-issue to validate filters let subIssue; try { - const subResponse = await github.rest.issues.get({ + const subResponse = await authClient.rest.issues.get({ owner, repo, issue_number: subIssueNumber, @@ -230,7 +232,7 @@ async function main(config = {}) { } } `; - const parentCheckResult = await github.graphql(parentCheckQuery, { + const parentCheckResult = await authClient.graphql(parentCheckQuery, { owner, repo, number: subIssueNumber, @@ -297,7 +299,7 @@ async function main(config = {}) { } // Use GraphQL mutation to add sub-issue - await github.graphql( + await authClient.graphql( ` mutation AddSubIssue($parentId: ID!, $subIssueId: ID!) { addSubIssue(input: { issueId: $parentId, subIssueId: $subIssueId }) { diff --git a/actions/setup/js/mark_pull_request_as_ready_for_review.cjs b/actions/setup/js/mark_pull_request_as_ready_for_review.cjs index 4b2fdfda40..b417cf11f5 100644 --- a/actions/setup/js/mark_pull_request_as_ready_for_review.cjs +++ b/actions/setup/js/mark_pull_request_as_ready_for_review.cjs @@ -10,6 +10,7 @@ const { sanitizeContent } = require("./sanitize_content.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); const { ERR_NOT_FOUND } = require("./error_codes.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); /** @type {string} Safe output type handled by this module */ const HANDLER_TYPE = "mark_pull_request_as_ready_for_review"; @@ -83,6 +84,7 @@ async function markPullRequestAsReadyForReview(github, owner, repo, prNumber) { async function main(config = {}) { // Extract configuration const maxCount = config.max || 10; + const authClient = await createAuthenticatedGitHubClient(config); // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; @@ -147,7 +149,7 @@ async function main(config = {}) { try { // First, get the current PR to check if it's a draft - const currentPR = await getPullRequestDetails(github, context.repo.owner, context.repo.repo, prNumber); + const currentPR = await getPullRequestDetails(authClient, context.repo.owner, context.repo.repo, prNumber); // Check if it's already not a draft if (!currentPR.draft) { @@ -176,7 +178,7 @@ async function main(config = {}) { } // Update the PR to mark as ready for review - const pr = await markPullRequestAsReadyForReview(github, context.repo.owner, context.repo.repo, prNumber); + const pr = await markPullRequestAsReadyForReview(authClient, context.repo.owner, context.repo.repo, prNumber); // Add comment with reason const workflowName = process.env.GH_AW_WORKFLOW_NAME || "GitHub Agentic Workflow"; @@ -193,7 +195,7 @@ async function main(config = {}) { const footer = generateFooterWithMessages(workflowName, runUrl, workflowSource, workflowSourceURL, triggeringIssueNumber, triggeringPRNumber, triggeringDiscussionNumber); const commentBody = `${sanitizedReason}\n\n${footer}`; - await addPullRequestComment(github, context.repo.owner, context.repo.repo, prNumber, commentBody); + await addPullRequestComment(authClient, context.repo.owner, context.repo.repo, prNumber, commentBody); core.info(`✓ Marked PR #${prNumber} as ready for review and added comment: ${pr.html_url}`); diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index 0510ed1622..76e2261a7a 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -10,6 +10,7 @@ const { normalizeBranchName } = require("./normalize_branch_name.cjs"); const { pushExtraEmptyCommit } = require("./extra_empty_commit.cjs"); const { detectForkPR } = require("./pr_helpers.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); /** * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction @@ -36,6 +37,7 @@ async function main(config = {}) { // Cross-repo support: resolve target repository from config // This allows pushing to PRs in a different repository than the workflow const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); + const authClient = await createAuthenticatedGitHubClient(config); // Base branch from config (if set) - used only for logging at factory level // Dynamic base branch resolution happens per-message after resolving the actual target repo @@ -225,7 +227,7 @@ async function main(config = {}) { // Fetch the specific PR to get its head branch, title, and labels let pullRequest; try { - const response = await github.rest.pulls.get({ + const response = await authClient.rest.pulls.get({ owner: repoParts.owner, repo: repoParts.repo, pull_number: pullNumber, diff --git a/actions/setup/js/remove_labels.cjs b/actions/setup/js/remove_labels.cjs index 059d6733d5..abbe3cd64b 100644 --- a/actions/setup/js/remove_labels.cjs +++ b/actions/setup/js/remove_labels.cjs @@ -12,6 +12,7 @@ const { validateLabels } = require("./safe_output_validator.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); /** * Main handler factory for remove_labels @@ -24,6 +25,7 @@ async function main(config = {}) { const blockedPatterns = config.blocked || []; const maxCount = config.max || 10; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); + const authClient = await createAuthenticatedGitHubClient(config); // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; @@ -161,7 +163,7 @@ async function main(config = {}) { // Remove labels one at a time (GitHub API doesn't have a bulk remove endpoint) for (const label of uniqueLabels) { try { - await github.rest.issues.removeLabel({ + await authClient.rest.issues.removeLabel({ owner: repoParts.owner, repo: repoParts.repo, issue_number: itemNumber, diff --git a/actions/setup/js/reply_to_pr_review_comment.cjs b/actions/setup/js/reply_to_pr_review_comment.cjs index f47aeed333..366515a8eb 100644 --- a/actions/setup/js/reply_to_pr_review_comment.cjs +++ b/actions/setup/js/reply_to_pr_review_comment.cjs @@ -12,6 +12,7 @@ const { sanitizeContent } = require("./sanitize_content.cjs"); const { getPRNumber } = require("./update_context_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); const { parseBoolTemplatable } = require("./templatable.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); /** * Type constant for handler identification @@ -34,6 +35,7 @@ async function main(config = {}) { const includeFooter = parseBoolTemplatable(config.footer, true); const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); + const authClient = await createAuthenticatedGitHubClient(config); // Determine the triggering PR number from context const triggeringPRNumber = getPRNumber(context.payload); @@ -162,7 +164,7 @@ async function main(config = {}) { core.info(`Replying to review comment ${numericCommentId} on PR #${targetPRNumber} (${owner}/${repo})`); - const result = await github.rest.pulls.createReplyForReviewComment({ + const result = await authClient.rest.pulls.createReplyForReviewComment({ owner, repo, pull_number: targetPRNumber, diff --git a/actions/setup/js/resolve_pr_review_thread.cjs b/actions/setup/js/resolve_pr_review_thread.cjs index 4a5509fbac..870eed6370 100644 --- a/actions/setup/js/resolve_pr_review_thread.cjs +++ b/actions/setup/js/resolve_pr_review_thread.cjs @@ -8,6 +8,7 @@ const { getErrorMessage } = require("./error_helpers.cjs"); const { getPRNumber } = require("./update_context_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); /** * Type constant for handler identification @@ -77,6 +78,7 @@ async function resolveReviewThreadAPI(github, threadId) { async function main(config = {}) { // Extract configuration const maxCount = config.max || 10; + const authClient = await createAuthenticatedGitHubClient(config); // Determine the triggering PR number from context const triggeringPRNumber = getPRNumber(context.payload); @@ -130,7 +132,7 @@ async function main(config = {}) { } // Look up the thread to validate it belongs to the triggering PR - const threadPRNumber = await getThreadPullRequestNumber(github, threadId); + const threadPRNumber = await getThreadPullRequestNumber(authClient, threadId); if (threadPRNumber === null) { core.warning(`Review thread not found or not a PullRequestReviewThread: ${threadId}`); return { @@ -162,7 +164,7 @@ async function main(config = {}) { }; } - const resolveResult = await resolveReviewThreadAPI(github, threadId); + const resolveResult = await resolveReviewThreadAPI(authClient, threadId); if (resolveResult.isResolved) { core.info(`Successfully resolved review thread: ${threadId}`); diff --git a/actions/setup/js/submit_pr_review.cjs b/actions/setup/js/submit_pr_review.cjs index c8083dac0b..d6616d68d9 100644 --- a/actions/setup/js/submit_pr_review.cjs +++ b/actions/setup/js/submit_pr_review.cjs @@ -6,6 +6,7 @@ */ const { resolveTarget } = require("./safe_output_helpers.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); /** @type {string} Safe output type handled by this module */ @@ -32,6 +33,7 @@ async function main(config = {}) { const maxCount = config.max || 1; const targetConfig = config.target || "triggering"; const buffer = config._prReviewBuffer; + const authClient = await createAuthenticatedGitHubClient(config); if (!buffer) { core.warning("submit_pull_request_review: No PR review buffer provided in config"); @@ -122,7 +124,7 @@ async function main(config = {}) { core.info(`Set review context from triggering PR: ${repo}#${payloadPR.number}`); } else { try { - const { data: fetchedPR } = await github.rest.pulls.get({ + const { data: fetchedPR } = await authClient.rest.pulls.get({ owner: context.repo.owner, repo: context.repo.repo, pull_number: prNum, diff --git a/actions/setup/js/submit_pr_review.test.cjs b/actions/setup/js/submit_pr_review.test.cjs index 7b7fc67589..d27c4b601c 100644 --- a/actions/setup/js/submit_pr_review.test.cjs +++ b/actions/setup/js/submit_pr_review.test.cjs @@ -29,6 +29,14 @@ const mockContext = { global.core = mockCore; global.context = mockContext; +global.github = { + rest: { + pulls: { + get: vi.fn().mockResolvedValue({ data: {} }), + }, + }, + graphql: vi.fn().mockResolvedValue({}), +}; const { createReviewBuffer } = require("./pr_review_buffer.cjs"); @@ -54,6 +62,16 @@ describe("submit_pr_review (Handler Factory Architecture)", () => { }, }; + // Reset github to default mock for each test (some tests delete global.github) + global.github = { + rest: { + pulls: { + get: vi.fn().mockResolvedValue({ data: {} }), + }, + }, + graphql: vi.fn().mockResolvedValue({}), + }; + // Create a fresh buffer for each test (factory pattern, no global state) buffer = createReviewBuffer(); diff --git a/actions/setup/js/unassign_from_user.cjs b/actions/setup/js/unassign_from_user.cjs index b1d3b9c0ee..cbd2462d80 100644 --- a/actions/setup/js/unassign_from_user.cjs +++ b/actions/setup/js/unassign_from_user.cjs @@ -10,6 +10,7 @@ const { getErrorMessage } = require("./error_helpers.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { resolveIssueNumber, extractAssignees } = require("./safe_output_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); /** @type {string} Safe output type handled by this module */ const HANDLER_TYPE = "unassign_from_user"; @@ -27,6 +28,7 @@ async function main(config = {}) { // Resolve target repository configuration const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); + const authClient = await createAuthenticatedGitHubClient(config); // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; @@ -127,7 +129,7 @@ async function main(config = {}) { try { // Remove assignees from the issue - await github.rest.issues.removeAssignees({ + await authClient.rest.issues.removeAssignees({ owner: repoParts.owner, repo: repoParts.repo, issue_number: issueNumber, diff --git a/actions/setup/js/update_handler_factory.cjs b/actions/setup/js/update_handler_factory.cjs index 92b50c9d9e..83bdbcda97 100644 --- a/actions/setup/js/update_handler_factory.cjs +++ b/actions/setup/js/update_handler_factory.cjs @@ -8,6 +8,8 @@ const { getErrorMessage } = require("./error_helpers.cjs"); const { resolveTarget } = require("./safe_output_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); +const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); /** * @typedef {Object} UpdateHandlerConfig @@ -101,6 +103,14 @@ function createUpdateHandlerFactory(handlerConfig) { const updateTarget = config.target || "triggering"; const maxCount = config.max || 10; + // Create an authenticated GitHub client. Uses config["github-token"] when set + // (for cross-repository operations), otherwise falls back to the step-level github. + const authClient = await createAuthenticatedGitHubClient(config); + + // Resolve default target repo and allowed repos for cross-repository routing. + // If no target-repo is configured, defaults to the current repository. + const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); + // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; @@ -139,8 +149,24 @@ function createUpdateHandlerFactory(handlerConfig) { const item = message; + // Resolve cross-repo target: if message has a "repo" field, validate it against + // the allowed repos and use it as the effective context. This enables updating items + // in a different repository when github-token is configured with the required permissions. + // Using {any} type to allow partial context override (effectiveContext.repo may differ from context.repo). + /** @type {any} */ + let effectiveContext = context; + if (item.repo) { + const repoResult = resolveAndValidateRepo(item, defaultTargetRepo, allowedRepos, itemTypeName); + if (!repoResult.success) { + core.warning(repoResult.error); + return { success: false, error: repoResult.error }; + } + effectiveContext = { ...context, repo: repoResult.repoParts }; + core.info(`Cross-repo update: targeting ${repoResult.repo}`); + } + // Resolve item number (may use custom logic) - const itemNumberResult = resolveItemNumber(item, updateTarget, context); + const itemNumberResult = resolveItemNumber(item, updateTarget, effectiveContext); if (!itemNumberResult.success) { core.warning(itemNumberResult.error); @@ -207,9 +233,11 @@ function createUpdateHandlerFactory(handlerConfig) { }; } - // Execute the update + // Execute the update using the authenticated client and effective context. + // authClient uses config["github-token"] when set (for cross-repo), otherwise global github. + // effectiveContext.repo contains the target repo owner/name for cross-repo routing. try { - const updatedItem = await executeUpdate(github, context, itemNumber, updateData); + const updatedItem = await executeUpdate(authClient, effectiveContext, itemNumber, updateData); core.info(`Successfully updated ${itemTypeName} #${itemNumber}: ${updatedItem.html_url || updatedItem.url}`); // Format and return success result diff --git a/actions/setup/js/update_handler_factory.test.cjs b/actions/setup/js/update_handler_factory.test.cjs index ba6e23b51f..10c7235632 100644 --- a/actions/setup/js/update_handler_factory.test.cjs +++ b/actions/setup/js/update_handler_factory.test.cjs @@ -484,4 +484,110 @@ describe("update_handler_factory.cjs", () => { }); }); }); + + describe("authentication: github-token and cross-repo routing", () => { + it("should use the global github client when no github-token in config", async () => { + // Capture which github client is passed to executeUpdate + let capturedClient = null; + const mockExecuteUpdate = vi.fn().mockImplementation(async (githubClient, context, num, data) => { + capturedClient = githubClient; + return { html_url: "https://example.com", title: "Updated" }; + }); + + const handlerFactory = factoryModule.createUpdateHandlerFactory({ + itemType: "update_test", + itemTypeName: "test item", + supportsPR: false, + resolveItemNumber: vi.fn().mockReturnValue({ success: true, number: 42 }), + buildUpdateData: vi.fn().mockReturnValue({ success: true, data: { title: "Test" } }), + executeUpdate: mockExecuteUpdate, + formatSuccessResult: vi.fn().mockReturnValue({ success: true }), + }); + + // No github-token in config + const handler = await handlerFactory({}); + await handler({ title: "Test" }); + + // The global github client should be used + expect(capturedClient).toBe(mockGithub); + }); + + it("should pass the correct context.repo when no message.repo", async () => { + let capturedContext = null; + const mockExecuteUpdate = vi.fn().mockImplementation(async (githubClient, context, num, data) => { + capturedContext = context; + return { html_url: "https://example.com", title: "Updated" }; + }); + + const handlerFactory = factoryModule.createUpdateHandlerFactory({ + itemType: "update_test", + itemTypeName: "test item", + supportsPR: false, + resolveItemNumber: vi.fn().mockReturnValue({ success: true, number: 42 }), + buildUpdateData: vi.fn().mockReturnValue({ success: true, data: { title: "Test" } }), + executeUpdate: mockExecuteUpdate, + formatSuccessResult: vi.fn().mockReturnValue({ success: true }), + }); + + const handler = await handlerFactory({ "target-repo": "owner/myrepo" }); + // Message without repo field — should use default context.repo + await handler({ title: "Test" }); + + expect(capturedContext.repo.owner).toBe("testowner"); + expect(capturedContext.repo.repo).toBe("testrepo"); + }); + + it("should route to message.repo when it matches the configured target-repo", async () => { + let capturedContext = null; + const mockExecuteUpdate = vi.fn().mockImplementation(async (githubClient, context, num, data) => { + capturedContext = context; + return { html_url: "https://example.com", title: "Updated" }; + }); + + const handlerFactory = factoryModule.createUpdateHandlerFactory({ + itemType: "update_test", + itemTypeName: "test item", + supportsPR: false, + resolveItemNumber: vi.fn().mockReturnValue({ success: true, number: 99 }), + buildUpdateData: vi.fn().mockReturnValue({ success: true, data: { title: "Test" } }), + executeUpdate: mockExecuteUpdate, + formatSuccessResult: vi.fn().mockReturnValue({ success: true }), + }); + + const handler = await handlerFactory({ "target-repo": "other-owner/side-repo" }); + // Message specifies a cross-repo target + await handler({ issue_number: 99, repo: "other-owner/side-repo" }); + + // effectiveContext.repo should be the target repo + expect(capturedContext.repo.owner).toBe("other-owner"); + expect(capturedContext.repo.repo).toBe("side-repo"); + }); + + it("should reject message.repo when it is not in allowed-repos", async () => { + const mockExecuteUpdate = vi.fn().mockResolvedValue({ html_url: "https://example.com", title: "Updated" }); + + const handlerFactory = factoryModule.createUpdateHandlerFactory({ + itemType: "update_test", + itemTypeName: "test item", + supportsPR: false, + resolveItemNumber: vi.fn().mockReturnValue({ success: true, number: 42 }), + buildUpdateData: vi.fn().mockReturnValue({ success: true, data: { title: "Test" } }), + executeUpdate: mockExecuteUpdate, + formatSuccessResult: vi.fn().mockReturnValue({ success: true }), + }); + + const handler = await handlerFactory({ + "target-repo": "allowed-owner/allowed-repo", + allowed_repos: ["allowed-owner/allowed-repo"], + }); + + // This repo is not in allowed-repos + const result = await handler({ issue_number: 42, repo: "malicious-owner/other-repo" }); + + expect(result.success).toBe(false); + expect(result.error).toBeDefined(); + // executeUpdate should not have been called + expect(mockExecuteUpdate).not.toHaveBeenCalled(); + }); + }); }); diff --git a/actions/setup/js/update_release.cjs b/actions/setup/js/update_release.cjs index 2462b20817..8fbfc2571f 100644 --- a/actions/setup/js/update_release.cjs +++ b/actions/setup/js/update_release.cjs @@ -13,6 +13,7 @@ const { updateBody } = require("./update_pr_description_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); const { ERR_API, ERR_CONFIG, ERR_VALIDATION } = require("./error_codes.cjs"); const { parseBoolTemplatable } = require("./templatable.cjs"); +const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); // Content sanitization: message.body is sanitized by updateBody() helper /** @@ -29,6 +30,7 @@ async function main(config = {}) { const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; const workflowName = process.env.GH_AW_WORKFLOW_NAME || "GitHub Agentic Workflow"; const includeFooter = parseBoolTemplatable(config.footer, true); + const authClient = await createAuthenticatedGitHubClient(config); /** * Process a single update-release message @@ -67,7 +69,7 @@ async function main(config = {}) { if (!releaseTag && context.payload.inputs.release_id) { const releaseId = context.payload.inputs.release_id; core.info(`Fetching release with ID: ${releaseId}`); - const { data: release } = await github.rest.repos.getRelease({ + const { data: release } = await authClient.rest.repos.getRelease({ owner: context.repo.owner, repo: context.repo.repo, release_id: parseInt(releaseId, 10), @@ -84,7 +86,7 @@ async function main(config = {}) { // Get the release by tag core.info(`Fetching release with tag: ${releaseTag}`); - const { data: release } = await github.rest.repos.getReleaseByTag({ + const { data: release } = await authClient.rest.repos.getReleaseByTag({ owner: context.repo.owner, repo: context.repo.repo, tag: releaseTag, @@ -108,7 +110,7 @@ async function main(config = {}) { }); // Update the release - const { data: updatedRelease } = await github.rest.repos.updateRelease({ + const { data: updatedRelease } = await authClient.rest.repos.updateRelease({ owner: context.repo.owner, repo: context.repo.repo, release_id: release.id, diff --git a/actions/setup/setup.sh b/actions/setup/setup.sh index 4a8e81e05d..557ade5046 100755 --- a/actions/setup/setup.sh +++ b/actions/setup/setup.sh @@ -231,6 +231,7 @@ SAFE_OUTPUTS_FILES=( "shim.cjs" "repo_helpers.cjs" "glob_pattern_helpers.cjs" + "handler_auth.cjs" ) SAFE_OUTPUTS_COUNT=0 diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index b18826631f..3820d2242e 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -4105,6 +4105,10 @@ "type": "boolean", "description": "Controls whether AI-generated footer is added to the issue. When false, the visible footer content is omitted but XML markers (workflow-id, tracker-id, metadata) are still included for searchability. Defaults to true.", "default": true + }, + "github-token": { + "$ref": "#/$defs/github_token", + "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." } }, "additionalProperties": false, @@ -4606,6 +4610,10 @@ ], "default": 7, "description": "Time until the discussion expires and should be automatically closed. Supports integer (days), relative time format like '2h' (2 hours), '7d' (7 days), '2w' (2 weeks), '1m' (1 month), '1y' (1 year), or false to disable expiration. Minimum duration: 2 hours. When set, a maintenance workflow will be generated. Defaults to 7 days if not specified." + }, + "github-token": { + "$ref": "#/$defs/github_token", + "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." } }, "additionalProperties": false, @@ -5032,6 +5040,10 @@ "pull-requests": { "type": "boolean", "description": "Controls whether the workflow requests pull-requests:write permission for add-comment and includes pull requests in the event trigger condition. Default: true (includes pull-requests:write). Set to false to disable pull request commenting." + }, + "github-token": { + "$ref": "#/$defs/github_token", + "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." } }, "additionalProperties": false, @@ -5427,6 +5439,17 @@ "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + }, + "target-repo": { + "type": "string", + "description": "Target repository in format 'owner/repo' for cross-repository code scanning alert creation. Takes precedence over trial target repo settings." + }, + "allowed-repos": { + "type": "array", + "items": { + "type": "string" + }, + "description": "List of additional repositories in format 'owner/repo' that code scanning alerts can be created in. When specified, the agent can use a 'repo' field in the output to specify which repository to create the alert in. The target repository (current or target-repo) is always implicitly allowed." } }, "additionalProperties": false @@ -6035,6 +6058,17 @@ "title-prefix": { "type": "string", "description": "Required prefix for issue title. Only issues with this title prefix can be updated." + }, + "allowed-repos": { + "type": "array", + "items": { + "type": "string" + }, + "description": "List of additional repositories in format 'owner/repo' that issues can be updated in. When specified, the agent can use a 'repo' field in the output to specify which repository to update the issue in. The target repository (current or target-repo) is always implicitly allowed." + }, + "github-token": { + "$ref": "#/$defs/github_token", + "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." } }, "additionalProperties": false @@ -6173,6 +6207,17 @@ "github-token-for-extra-empty-commit": { "type": "string", "description": "Token used to push an empty commit after pushing changes to trigger CI events. Works around the GITHUB_TOKEN limitation where pushes don't trigger workflow runs. Defaults to the magic secret GH_AW_CI_TRIGGER_TOKEN if set in the repository. Use a secret expression (e.g. '${{ secrets.CI_TOKEN }}') for a custom token, or 'app' for GitHub App auth." + }, + "target-repo": { + "type": "string", + "description": "Target repository in format 'owner/repo' for cross-repository push to pull request branch. Takes precedence over trial target repo settings." + }, + "allowed-repos": { + "type": "array", + "items": { + "type": "string" + }, + "description": "List of additional repositories in format 'owner/repo' that push to pull request branch can target. When specified, the agent can use a 'repo' field in the output to specify which repository to push to. The target repository (current or target-repo) is always implicitly allowed." } }, "additionalProperties": false @@ -6770,6 +6815,24 @@ "items": { "$ref": "#/$defs/githubActionsStep" } + }, + "runner": { + "description": "Runner specification for this job (alias for runs-on)", + "oneOf": [ + { + "type": "string" + }, + { + "type": "array", + "items": { + "type": "string" + } + } + ] + }, + "agent-output": { + "type": "string", + "description": "Agent output field to use as input for this safe job (alias for output)" } }, "additionalProperties": false diff --git a/pkg/workflow/aw_info_versions_test.go b/pkg/workflow/aw_info_versions_test.go index 37494d2ab1..5f25b082a2 100644 --- a/pkg/workflow/aw_info_versions_test.go +++ b/pkg/workflow/aw_info_versions_test.go @@ -140,7 +140,7 @@ func TestCLIVersionInAwInfo(t *testing.T) { compiler.generateCreateAwInfo(&yaml, workflowData, engine) output := yaml.String() - expectedLine := `cli_version: "` + tt.cliVersion + `"` + expectedLine := `GH_AW_INFO_CLI_VERSION: "` + tt.cliVersion + `"` containsVersion := strings.Contains(output, expectedLine) if tt.shouldInclude { @@ -150,8 +150,8 @@ func TestCLIVersionInAwInfo(t *testing.T) { } } else { // For dev builds, cli_version should not appear at all - if strings.Contains(output, "cli_version:") { - t.Errorf("%s: Expected output to NOT contain 'cli_version:' field, got:\n%s", + if strings.Contains(output, "GH_AW_INFO_CLI_VERSION:") { + t.Errorf("%s: Expected output to NOT contain 'GH_AW_INFO_CLI_VERSION:' field, got:\n%s", tt.description, output) } } @@ -216,7 +216,7 @@ func TestAwfVersionInAwInfo(t *testing.T) { compiler.generateCreateAwInfo(&yaml, workflowData, engine) output := yaml.String() - expectedLine := `awf_version: "` + tt.expectedAwfVersion + `"` + expectedLine := `GH_AW_INFO_AWF_VERSION: "` + tt.expectedAwfVersion + `"` if !strings.Contains(output, expectedLine) { t.Errorf("%s: Expected output to contain '%s', got:\n%s", tt.description, expectedLine, output) @@ -256,13 +256,13 @@ func TestBothVersionsInAwInfo(t *testing.T) { output := yaml.String() // Check for cli_version - expectedCLILine := `cli_version: "2.0.0-beta.5"` + expectedCLILine := `GH_AW_INFO_CLI_VERSION: "2.0.0-beta.5"` if !strings.Contains(output, expectedCLILine) { t.Errorf("Expected output to contain cli_version '%s', got:\n%s", expectedCLILine, output) } // Check for awf_version - expectedAwfLine := `awf_version: "v0.5.0"` + expectedAwfLine := `GH_AW_INFO_AWF_VERSION: "v0.5.0"` if !strings.Contains(output, expectedAwfLine) { t.Errorf("Expected output to contain awf_version '%s', got:\n%s", expectedAwfLine, output) } @@ -320,7 +320,7 @@ func TestAwmgVersionInAwInfo(t *testing.T) { compiler.generateCreateAwInfo(&yaml, workflowData, engine) output := yaml.String() - expectedLine := `awmg_version: "` + tt.expectedAwmgVersion + `"` + expectedLine := `GH_AW_INFO_AWMG_VERSION: "` + tt.expectedAwmgVersion + `"` if !strings.Contains(output, expectedLine) { t.Errorf("%s: Expected output to contain '%s', got:\n%s", tt.description, expectedLine, output) @@ -365,19 +365,19 @@ func TestAllVersionsInAwInfo(t *testing.T) { output := yaml.String() // Check for cli_version - expectedCLILine := `cli_version: "2.0.0-beta.5"` + expectedCLILine := `GH_AW_INFO_CLI_VERSION: "2.0.0-beta.5"` if !strings.Contains(output, expectedCLILine) { t.Errorf("Expected output to contain cli_version '%s', got:\n%s", expectedCLILine, output) } // Check for awf_version - expectedAwfLine := `awf_version: "v0.5.0"` + expectedAwfLine := `GH_AW_INFO_AWF_VERSION: "v0.5.0"` if !strings.Contains(output, expectedAwfLine) { t.Errorf("Expected output to contain awf_version '%s', got:\n%s", expectedAwfLine, output) } // Check for awmg_version - expectedAwmgLine := `awmg_version: "v0.0.12"` + expectedAwmgLine := `GH_AW_INFO_AWMG_VERSION: "v0.0.12"` if !strings.Contains(output, expectedAwmgLine) { t.Errorf("Expected output to contain awmg_version '%s', got:\n%s", expectedAwmgLine, output) } diff --git a/pkg/workflow/compiler_safe_outputs_config.go b/pkg/workflow/compiler_safe_outputs_config.go index 476ed54955..2d4299898e 100644 --- a/pkg/workflow/compiler_safe_outputs_config.go +++ b/pkg/workflow/compiler_safe_outputs_config.go @@ -142,6 +142,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddTemplatableBool("group", c.Group). AddTemplatableBool("close_older_issues", c.CloseOlderIssues). AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, "add_comment": func(cfg *SafeOutputsConfig) map[string]any { @@ -155,6 +156,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddTemplatableBool("hide_older_comments", c.HideOlderComments). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, "create_discussion": func(cfg *SafeOutputsConfig) map[string]any { @@ -175,6 +177,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddBoolPtr("fallback_to_issue", c.FallbackToIssue). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, "close_issue": func(cfg *SafeOutputsConfig) map[string]any { @@ -218,6 +221,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("target", c.Target). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). Build() // If config is empty, it means add_labels was explicitly configured with no options // (null config), which means "allow any labels". Return non-nil empty map to @@ -240,6 +244,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("target", c.Target). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, "add_reviewer": func(cfg *SafeOutputsConfig) map[string]any { @@ -253,6 +258,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("target", c.Target). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, "assign_milestone": func(cfg *SafeOutputsConfig) map[string]any { @@ -266,6 +272,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("target", c.Target). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, "mark_pull_request_as_ready_for_review": func(cfg *SafeOutputsConfig) map[string]any { @@ -280,6 +287,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, "create_code_scanning_alert": func(cfg *SafeOutputsConfig) map[string]any { @@ -290,6 +298,9 @@ var handlerRegistry = map[string]handlerBuilder{ return newHandlerConfigBuilder(). AddTemplatableInt("max", c.Max). AddIfNotEmpty("driver", c.Driver). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, "create_agent_session": func(cfg *SafeOutputsConfig) map[string]any { @@ -325,6 +336,7 @@ var handlerRegistry = map[string]handlerBuilder{ return builder. AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). Build() }, @@ -350,6 +362,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddStringSlice("allowed_labels", c.AllowedLabels). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). Build() }, @@ -366,6 +379,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("sub_title_prefix", c.SubTitlePrefix). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, "update_release": func(cfg *SafeOutputsConfig) map[string]any { @@ -375,6 +389,7 @@ var handlerRegistry = map[string]handlerBuilder{ c := cfg.UpdateRelease return newHandlerConfigBuilder(). AddTemplatableInt("max", c.Max). + AddIfNotEmpty("github-token", c.GitHubToken). AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). Build() }, @@ -389,6 +404,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("target", c.Target). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, "submit_pull_request_review": func(cfg *SafeOutputsConfig) map[string]any { @@ -399,6 +415,7 @@ var handlerRegistry = map[string]handlerBuilder{ return newHandlerConfigBuilder(). AddTemplatableInt("max", c.Max). AddIfNotEmpty("target", c.Target). + AddIfNotEmpty("github-token", c.GitHubToken). AddStringPtr("footer", getEffectiveFooterString(c.Footer, cfg.Footer)). Build() }, @@ -412,6 +429,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("target", c.Target). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). Build() }, @@ -422,6 +440,7 @@ var handlerRegistry = map[string]handlerBuilder{ c := cfg.ResolvePullRequestReviewThread return newHandlerConfigBuilder(). AddTemplatableInt("max", c.Max). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, "create_pull_request": func(cfg *SafeOutputsConfig) map[string]any { @@ -446,6 +465,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). AddDefault("max_patch_size", maxPatchSize). + AddIfNotEmpty("github-token", c.GitHubToken). AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). AddBoolPtr("fallback_as_issue", c.FallbackAsIssue). AddIfNotEmpty("base_branch", c.BaseBranch) @@ -468,6 +488,9 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("if_no_changes", c.IfNoChanges). AddIfNotEmpty("commit_title_suffix", c.CommitTitleSuffix). AddDefault("max_patch_size", maxPatchSize). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, "update_pull_request": func(cfg *SafeOutputsConfig) map[string]any { @@ -484,6 +507,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, "close_pull_request": func(cfg *SafeOutputsConfig) map[string]any { @@ -498,6 +522,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, "hide_comment": func(cfg *SafeOutputsConfig) map[string]any { @@ -510,6 +535,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddStringSlice("allowed_reasons", c.AllowedReasons). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, "dispatch_workflow": func(cfg *SafeOutputsConfig) map[string]any { @@ -526,6 +552,7 @@ var handlerRegistry = map[string]handlerBuilder{ builder.AddDefault("workflow_files", c.WorkflowFiles) } + builder.AddIfNotEmpty("github-token", c.GitHubToken) return builder.Build() }, "missing_tool": func(cfg *SafeOutputsConfig) map[string]any { @@ -535,6 +562,7 @@ var handlerRegistry = map[string]handlerBuilder{ c := cfg.MissingTool return newHandlerConfigBuilder(). AddTemplatableInt("max", c.Max). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, "missing_data": func(cfg *SafeOutputsConfig) map[string]any { @@ -544,6 +572,7 @@ var handlerRegistry = map[string]handlerBuilder{ c := cfg.MissingData return newHandlerConfigBuilder(). AddTemplatableInt("max", c.Max). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, // Note: "noop" is intentionally NOT included here because it is always processed @@ -608,6 +637,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("target", c.Target). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). AddTemplatableBool("unassign_first", c.UnassignFirst). Build() }, @@ -623,6 +653,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("target", c.Target). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). Build() }, "create_project_status_update": func(cfg *SafeOutputsConfig) map[string]any { diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index c330ebc672..83d2b08040 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -190,8 +190,6 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa outputs["assign_to_agent_assigned"] = "${{ steps.assign_to_agent.outputs.assigned }}" outputs["assign_to_agent_assignment_errors"] = "${{ steps.assign_to_agent.outputs.assignment_errors }}" outputs["assign_to_agent_assignment_error_count"] = "${{ steps.assign_to_agent.outputs.assignment_error_count }}" - - // Note: Permissions are computed centrally by ComputePermissionsForSafeOutputs() } // 4. Create Agent Session step @@ -203,57 +201,23 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa outputs["create_agent_session_session_number"] = "${{ steps.create_agent_session.outputs.session_number }}" outputs["create_agent_session_session_url"] = "${{ steps.create_agent_session.outputs.session_url }}" - - // Note: Permissions are computed centrally by ComputePermissionsForSafeOutputs() } - // Note: Create Pull Request is now handled by the handler manager - // The outputs and permissions are configured in the handler manager section above - - // Note: Mark Pull Request as Ready for Review is now handled by the handler manager - // The permissions are configured in the handler manager section above - - // Note: Create Code Scanning Alert is now handled by the handler manager - // The permissions are configured in the handler manager section above - // Note: Permissions are computed centrally by ComputePermissionsForSafeOutputs() - - // Note: Create Project Status Update is now handled by the handler manager - // The permissions are configured in the handler manager section above - // Note: Permissions are computed centrally by ComputePermissionsForSafeOutputs() - - // Note: Add Reviewer is now handled by the handler manager // The outputs and permissions are configured in the handler manager section above if data.SafeOutputs.AddReviewer != nil { outputs["add_reviewer_reviewers_added"] = "${{ steps.process_safe_outputs.outputs.reviewers_added }}" - // Note: Permissions are computed centrally by ComputePermissionsForSafeOutputs() } - // Note: Assign Milestone is now handled by the handler manager // The outputs and permissions are configured in the handler manager section above if data.SafeOutputs.AssignMilestone != nil { outputs["assign_milestone_milestone_assigned"] = "${{ steps.process_safe_outputs.outputs.milestone_assigned }}" - // Note: Permissions are computed centrally by ComputePermissionsForSafeOutputs() } - // Note: Assign To User is now handled by the handler manager // The outputs and permissions are configured in the handler manager section above if data.SafeOutputs.AssignToUser != nil { outputs["assign_to_user_assigned"] = "${{ steps.process_safe_outputs.outputs.assigned }}" - // Note: Permissions are computed centrally by ComputePermissionsForSafeOutputs() } - // Note: Update Pull Request step - now handled by handler manager - - // Note: Push To Pull Request Branch step - now handled by handler manager - - // Note: Upload Assets - now handled as a separate job (see buildSafeOutputsJobs) - // This was moved out of the consolidated job to allow proper git configuration - // for pushing to orphaned branches - - // Note: Update Release step - now handled by handler manager - // Note: Link Sub Issue step - now handled by handler manager - // Note: Hide Comment step - now handled by handler manager - // If no steps were added, return nil if len(safeOutputStepNames) == 0 { consolidatedSafeOutputsJobLog.Print("No safe output steps were added") diff --git a/pkg/workflow/compiler_safe_outputs_steps.go b/pkg/workflow/compiler_safe_outputs_steps.go index 493c712b28..127c1659bb 100644 --- a/pkg/workflow/compiler_safe_outputs_steps.go +++ b/pkg/workflow/compiler_safe_outputs_steps.go @@ -9,6 +9,56 @@ import ( var consolidatedSafeOutputsStepsLog = logger.New("workflow:compiler_safe_outputs_steps") +// computeEffectiveProjectToken computes the effective project token using the precedence: +// 1. Per-config token (e.g., from update-project, create-project-status-update) +// 2. Safe-outputs level token +// 3. Magic secret fallback via getEffectiveProjectGitHubToken() +func computeEffectiveProjectToken(perConfigToken string, safeOutputsToken string) string { + configToken := perConfigToken + if configToken == "" && safeOutputsToken != "" { + configToken = safeOutputsToken + } + return getEffectiveProjectGitHubToken(configToken) +} + +// computeProjectURLAndToken computes the project URL and token from the various project-related +// safe-output configurations. Priority order: update-project > create-project-status-update > create-project. +// Returns the project URL (may be empty for create-project) and the effective token. +func computeProjectURLAndToken(safeOutputs *SafeOutputsConfig) (projectURL, projectToken string) { + if safeOutputs == nil { + return "", "" + } + + safeOutputsToken := safeOutputs.GitHubToken + + // Check update-project first (highest priority) + if safeOutputs.UpdateProjects != nil && safeOutputs.UpdateProjects.Project != "" { + projectURL = safeOutputs.UpdateProjects.Project + projectToken = computeEffectiveProjectToken(safeOutputs.UpdateProjects.GitHubToken, safeOutputsToken) + consolidatedSafeOutputsStepsLog.Printf("Setting GH_AW_PROJECT_URL from update-project config: %s", projectURL) + consolidatedSafeOutputsStepsLog.Printf("Setting GH_AW_PROJECT_GITHUB_TOKEN from update-project config") + return + } + + // Check create-project-status-update second + if safeOutputs.CreateProjectStatusUpdates != nil && safeOutputs.CreateProjectStatusUpdates.Project != "" { + projectURL = safeOutputs.CreateProjectStatusUpdates.Project + projectToken = computeEffectiveProjectToken(safeOutputs.CreateProjectStatusUpdates.GitHubToken, safeOutputsToken) + consolidatedSafeOutputsStepsLog.Printf("Setting GH_AW_PROJECT_URL from create-project-status-update config: %s", projectURL) + consolidatedSafeOutputsStepsLog.Printf("Setting GH_AW_PROJECT_GITHUB_TOKEN from create-project-status-update config") + return + } + + // Check create-project for token even if no URL is set (create-project doesn't have a project URL field) + // This ensures GH_AW_PROJECT_GITHUB_TOKEN is set when create-project is configured + if safeOutputs.CreateProjects != nil { + projectToken = computeEffectiveProjectToken(safeOutputs.CreateProjects.GitHubToken, safeOutputsToken) + consolidatedSafeOutputsStepsLog.Printf("Setting GH_AW_PROJECT_GITHUB_TOKEN from create-project config") + } + + return +} + // buildConsolidatedSafeOutputStep builds a single step for a safe output operation // within the consolidated safe-outputs job. This function handles both inline script // mode and file mode (requiring from local filesystem). @@ -309,43 +359,7 @@ func (c *Compiler) buildHandlerManagerStep(data *WorkflowData) []string { // // Note: If multiple project configs are present, we prefer update-project > create-project-status-update > create-project // This is only relevant for the environment variables - each configuration must explicitly specify its own settings - var projectURL string - var projectToken string - - // Check update-project first (highest priority) - if data.SafeOutputs.UpdateProjects != nil && data.SafeOutputs.UpdateProjects.Project != "" { - projectURL = data.SafeOutputs.UpdateProjects.Project - // Use per-config token, fallback to safe-outputs level token, then default - configToken := data.SafeOutputs.UpdateProjects.GitHubToken - if configToken == "" && data.SafeOutputs.GitHubToken != "" { - configToken = data.SafeOutputs.GitHubToken - } - projectToken = getEffectiveProjectGitHubToken(configToken) - consolidatedSafeOutputsStepsLog.Printf("Setting GH_AW_PROJECT_URL from update-project config: %s", projectURL) - consolidatedSafeOutputsStepsLog.Printf("Setting GH_AW_PROJECT_GITHUB_TOKEN from update-project config") - } else if data.SafeOutputs.CreateProjectStatusUpdates != nil && data.SafeOutputs.CreateProjectStatusUpdates.Project != "" { - projectURL = data.SafeOutputs.CreateProjectStatusUpdates.Project - // Use per-config token, fallback to safe-outputs level token, then default - configToken := data.SafeOutputs.CreateProjectStatusUpdates.GitHubToken - if configToken == "" && data.SafeOutputs.GitHubToken != "" { - configToken = data.SafeOutputs.GitHubToken - } - projectToken = getEffectiveProjectGitHubToken(configToken) - consolidatedSafeOutputsStepsLog.Printf("Setting GH_AW_PROJECT_URL from create-project-status-update config: %s", projectURL) - consolidatedSafeOutputsStepsLog.Printf("Setting GH_AW_PROJECT_GITHUB_TOKEN from create-project-status-update config") - } - - // Check create-project for token even if no URL is set (create-project doesn't have a project URL field) - // This ensures GH_AW_PROJECT_GITHUB_TOKEN is set when create-project is configured - if projectToken == "" && data.SafeOutputs.CreateProjects != nil { - // Use per-config token, fallback to safe-outputs level token, then default - configToken := data.SafeOutputs.CreateProjects.GitHubToken - if configToken == "" && data.SafeOutputs.GitHubToken != "" { - configToken = data.SafeOutputs.GitHubToken - } - projectToken = getEffectiveProjectGitHubToken(configToken) - consolidatedSafeOutputsStepsLog.Printf("Setting GH_AW_PROJECT_GITHUB_TOKEN from create-project config") - } + projectURL, projectToken := computeProjectURLAndToken(data.SafeOutputs) if projectURL != "" { steps = append(steps, fmt.Sprintf(" GH_AW_PROJECT_URL: %q\n", projectURL)) @@ -362,9 +376,19 @@ func (c *Compiler) buildHandlerManagerStep(data *WorkflowData) []string { // cannot be accessed with the default GITHUB_TOKEN. GH_AW_PROJECT_GITHUB_TOKEN is the required // token for Projects v2 operations. steps = append(steps, " with:\n") + // Token precedence for the handler manager step: + // 1. Project token (if project operations are configured) - already set above + // 2. Safe-outputs level token (so.GitHubToken) + // 3. Magic secret fallback via getEffectiveSafeOutputGitHubToken() + // + // Note: We do NOT fall back to per-output tokens (add-comment, create-issue, etc.) + // because those are specific to their operations. The handler manager needs a + // general-purpose token for the github-script client. configToken := "" if projectToken != "" { configToken = projectToken + } else if data.SafeOutputs != nil && data.SafeOutputs.GitHubToken != "" { + configToken = data.SafeOutputs.GitHubToken } c.addSafeOutputGitHubTokenForConfig(&steps, data, configToken) diff --git a/pkg/workflow/config_helpers.go b/pkg/workflow/config_helpers.go index ca48123c42..98ea4e9018 100644 --- a/pkg/workflow/config_helpers.go +++ b/pkg/workflow/config_helpers.go @@ -164,6 +164,14 @@ func parseAllowedLabelsFromConfig(configMap map[string]any) []string { return ParseStringArrayFromConfig(configMap, "allowed-labels", configHelpersLog) } +// parseAllowedReposFromConfig extracts and validates allowed-repos from a config map. +// Returns a slice of repository slugs in "owner/repo" format. +// Returns nil when the key is not present or the value is not a valid array type. +// Returns an empty slice when the key exists but contains no valid strings. +func parseAllowedReposFromConfig(configMap map[string]any) []string { + return ParseStringArrayFromConfig(configMap, "allowed-repos", configHelpersLog) +} + // NOTE: parseExpiresFromConfig and parseRelativeTimeSpec have been moved to time_delta.go // to consolidate all time parsing logic in a single location. These functions are used // for parsing expiration configurations in safe output jobs. diff --git a/pkg/workflow/create_code_scanning_alert.go b/pkg/workflow/create_code_scanning_alert.go index 920fa56e08..783fe2ccc3 100644 --- a/pkg/workflow/create_code_scanning_alert.go +++ b/pkg/workflow/create_code_scanning_alert.go @@ -12,7 +12,9 @@ var createCodeScanningAlertLog = logger.New("workflow:create_code_scanning_alert // CreateCodeScanningAlertsConfig holds configuration for creating repository security advisories (SARIF format) from agent output type CreateCodeScanningAlertsConfig struct { BaseSafeOutputConfig `yaml:",inline"` - Driver string `yaml:"driver,omitempty"` // Driver name for SARIF tool.driver.name field (default: "GitHub Agentic Workflows Security Scanner") + Driver string `yaml:"driver,omitempty"` // Driver name for SARIF tool.driver.name field (default: "GitHub Agentic Workflows Security Scanner") + TargetRepoSlug string `yaml:"target-repo,omitempty"` // Target repository in format "owner/repo" for cross-repository code scanning alert creation + AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories in format "owner/repo" that code scanning alerts can be created in } // buildCreateOutputCodeScanningAlertJob creates the create_code_scanning_alert job @@ -106,6 +108,12 @@ func (c *Compiler) parseCodeScanningAlertsConfig(outputMap map[string]any) *Crea } } + // Parse target-repo + securityReportsConfig.TargetRepoSlug = parseTargetRepoFromConfig(configMap) + + // Parse allowed-repos + securityReportsConfig.AllowedRepos = parseAllowedReposFromConfig(configMap) + // Parse common base fields with default max of 0 (unlimited) c.parseBaseSafeOutputConfig(configMap, &securityReportsConfig.BaseSafeOutputConfig, 0) } else { diff --git a/pkg/workflow/push_to_pull_request_branch.go b/pkg/workflow/push_to_pull_request_branch.go index 0a0b9f99ad..aebf511e4b 100644 --- a/pkg/workflow/push_to_pull_request_branch.go +++ b/pkg/workflow/push_to_pull_request_branch.go @@ -18,6 +18,8 @@ type PushToPullRequestBranchConfig struct { IfNoChanges string `yaml:"if-no-changes,omitempty"` // Behavior when no changes to push: "warn", "error", or "ignore" (default: "warn") CommitTitleSuffix string `yaml:"commit-title-suffix,omitempty"` // Optional suffix to append to generated commit titles GithubTokenForExtraEmptyCommit string `yaml:"github-token-for-extra-empty-commit,omitempty"` // Token used to push an empty commit to trigger CI events. Use a PAT or "app" for GitHub App auth. + TargetRepoSlug string `yaml:"target-repo,omitempty"` // Target repository in format "owner/repo" for cross-repository push to pull request branch + AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories in format "owner/repo" that push to pull request branch can target } // buildCheckoutRepository generates a checkout step with optional target repository and custom token @@ -126,6 +128,12 @@ func (c *Compiler) parsePushToPullRequestBranchConfig(outputMap map[string]any) } } + // Parse target-repo for cross-repository push + pushToBranchConfig.TargetRepoSlug = parseTargetRepoFromConfig(configMap) + + // Parse allowed-repos for cross-repository push + pushToBranchConfig.AllowedRepos = parseAllowedReposFromConfig(configMap) + // Parse common base fields with default max of 0 (no limit) c.parseBaseSafeOutputConfig(configMap, &pushToBranchConfig.BaseSafeOutputConfig, 0) } diff --git a/pkg/workflow/safe_jobs.go b/pkg/workflow/safe_jobs.go index b367c2e783..881d5715af 100644 --- a/pkg/workflow/safe_jobs.go +++ b/pkg/workflow/safe_jobs.go @@ -68,9 +68,11 @@ func (c *Compiler) parseSafeJobsConfig(jobsMap map[string]any) map[string]*SafeJ } } - // Parse runs-on + // Parse runs-on (also accept "runner" as alias) if runsOn, exists := jobConfig["runs-on"]; exists { safeJob.RunsOn = runsOn + } else if runner, exists := jobConfig["runner"]; exists { + safeJob.RunsOn = runner } // Parse if condition @@ -131,11 +133,15 @@ func (c *Compiler) parseSafeJobsConfig(jobsMap map[string]any) map[string]*SafeJ } } - // Parse output + // Parse output (also accept "agent-output" as alias) if output, exists := jobConfig["output"]; exists { if outputStr, ok := output.(string); ok { safeJob.Output = outputStr } + } else if agentOutput, exists := jobConfig["agent-output"]; exists { + if agentOutputStr, ok := agentOutput.(string); ok { + safeJob.Output = agentOutputStr + } } // Parse inputs using the unified parsing function diff --git a/pkg/workflow/safe_output_parser.go b/pkg/workflow/safe_output_parser.go index 9ed7ad4723..941e186f77 100644 --- a/pkg/workflow/safe_output_parser.go +++ b/pkg/workflow/safe_output_parser.go @@ -56,6 +56,9 @@ func ParseTargetConfig(configMap map[string]any) (SafeOutputTargetConfig, bool) // Parse target-repo; wildcard "*" is allowed and means "any repository" config.TargetRepoSlug = parseTargetRepoFromConfig(configMap) + // Parse allowed-repos + config.AllowedRepos = parseAllowedReposFromConfig(configMap) + return config, false } diff --git a/pkg/workflow/safe_outputs_cross_repo_config_test.go b/pkg/workflow/safe_outputs_cross_repo_config_test.go new file mode 100644 index 0000000000..cb7098f186 --- /dev/null +++ b/pkg/workflow/safe_outputs_cross_repo_config_test.go @@ -0,0 +1,593 @@ +//go:build !integration + +package workflow + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestCreateCodeScanningAlertConfigTargetRepo verifies that create-code-scanning-alert +// correctly parses target-repo and allowed-repos fields. +func TestCreateCodeScanningAlertConfigTargetRepo(t *testing.T) { + compiler := NewCompiler() + + tests := []struct { + name string + configMap map[string]any + expectedRepo string + expectedRepos []string + expectedToken string + expectedDriver string + }{ + { + name: "target-repo and allowed-repos configured", + configMap: map[string]any{ + "create-code-scanning-alert": map[string]any{ + "max": 10, + "target-repo": "githubnext/gh-aw-side-repo", + "allowed-repos": []any{"githubnext/gh-aw-side-repo"}, + "github-token": "${{ secrets.TEMP_USER_PAT }}", + }, + }, + expectedRepo: "githubnext/gh-aw-side-repo", + expectedRepos: []string{"githubnext/gh-aw-side-repo"}, + expectedToken: "${{ secrets.TEMP_USER_PAT }}", + expectedDriver: "", + }, + { + name: "driver and target-repo configured", + configMap: map[string]any{ + "create-code-scanning-alert": map[string]any{ + "driver": "My Scanner", + "target-repo": "owner/other-repo", + }, + }, + expectedRepo: "owner/other-repo", + expectedRepos: nil, + expectedToken: "", + expectedDriver: "My Scanner", + }, + { + name: "no cross-repo config", + configMap: map[string]any{ + "create-code-scanning-alert": map[string]any{ + "max": 5, + }, + }, + expectedRepo: "", + expectedRepos: nil, + expectedToken: "", + }, + { + name: "nil config value", + configMap: map[string]any{ + "create-code-scanning-alert": nil, + }, + expectedRepo: "", + expectedRepos: nil, + expectedToken: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := compiler.parseCodeScanningAlertsConfig(tt.configMap) + + require.NotNil(t, cfg, "config should not be nil") + assert.Equal(t, tt.expectedRepo, cfg.TargetRepoSlug, "TargetRepoSlug mismatch") + assert.Equal(t, tt.expectedRepos, cfg.AllowedRepos, "AllowedRepos mismatch") + assert.Equal(t, tt.expectedToken, cfg.GitHubToken, "GitHubToken mismatch") + if tt.expectedDriver != "" { + assert.Equal(t, tt.expectedDriver, cfg.Driver, "Driver mismatch") + } + }) + } +} + +// TestPushToPullRequestBranchConfigTargetRepo verifies that push-to-pull-request-branch +// correctly parses target-repo and allowed-repos fields. +func TestPushToPullRequestBranchConfigTargetRepo(t *testing.T) { + compiler := NewCompiler() + + tests := []struct { + name string + configMap map[string]any + expectedRepo string + expectedRepos []string + expectedToken string + }{ + { + name: "target-repo and allowed-repos configured", + configMap: map[string]any{ + "push-to-pull-request-branch": map[string]any{ + "target-repo": "githubnext/gh-aw-side-repo", + "allowed-repos": []any{"githubnext/gh-aw-side-repo"}, + "github-token": "${{ secrets.TEMP_USER_PAT }}", + }, + }, + expectedRepo: "githubnext/gh-aw-side-repo", + expectedRepos: []string{"githubnext/gh-aw-side-repo"}, + expectedToken: "${{ secrets.TEMP_USER_PAT }}", + }, + { + name: "multiple allowed repos", + configMap: map[string]any{ + "push-to-pull-request-branch": map[string]any{ + "target-repo": "org/primary-repo", + "allowed-repos": []any{"org/primary-repo", "org/secondary-repo"}, + }, + }, + expectedRepo: "org/primary-repo", + expectedRepos: []string{"org/primary-repo", "org/secondary-repo"}, + expectedToken: "", + }, + { + name: "no cross-repo config", + configMap: map[string]any{ + "push-to-pull-request-branch": map[string]any{ + "target": "triggering", + }, + }, + expectedRepo: "", + expectedRepos: nil, + expectedToken: "", + }, + { + name: "nil push-to-pull-request-branch config", + configMap: map[string]any{ + "push-to-pull-request-branch": nil, + }, + expectedRepo: "", + expectedRepos: nil, + expectedToken: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := compiler.parsePushToPullRequestBranchConfig(tt.configMap) + + require.NotNil(t, cfg, "config should not be nil") + assert.Equal(t, tt.expectedRepo, cfg.TargetRepoSlug, "TargetRepoSlug mismatch") + assert.Equal(t, tt.expectedRepos, cfg.AllowedRepos, "AllowedRepos mismatch") + assert.Equal(t, tt.expectedToken, cfg.GitHubToken, "GitHubToken mismatch") + }) + } +} + +// TestUpdateIssueConfigGitHubToken verifies that update-issue correctly parses the github-token field. +func TestUpdateIssueConfigGitHubToken(t *testing.T) { + compiler := NewCompiler() + + tests := []struct { + name string + configMap map[string]any + expectedToken string + expectedRepo string + expectedRepos []string + }{ + { + name: "github-token and allowed-repos configured", + configMap: map[string]any{ + "update-issue": map[string]any{ + "target-repo": "githubnext/gh-aw-side-repo", + "allowed-repos": []any{"githubnext/gh-aw-side-repo"}, + "github-token": "${{ secrets.TEMP_USER_PAT }}", + }, + }, + expectedToken: "${{ secrets.TEMP_USER_PAT }}", + expectedRepo: "githubnext/gh-aw-side-repo", + expectedRepos: []string{"githubnext/gh-aw-side-repo"}, + }, + { + name: "no token or cross-repo", + configMap: map[string]any{ + "update-issue": map[string]any{ + "body": true, + }, + }, + expectedToken: "", + expectedRepo: "", + expectedRepos: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := compiler.parseUpdateIssuesConfig(tt.configMap) + + require.NotNil(t, cfg, "config should not be nil") + assert.Equal(t, tt.expectedToken, cfg.GitHubToken, "GitHubToken mismatch") + assert.Equal(t, tt.expectedRepo, cfg.TargetRepoSlug, "TargetRepoSlug mismatch") + assert.Equal(t, tt.expectedRepos, cfg.AllowedRepos, "AllowedRepos mismatch") + }) + } +} + +// TestAddCommentGitHubTokenInHandlerConfig verifies that github-token is included in +// the handler manager config JSON for add-comment. +func TestAddCommentGitHubTokenInHandlerConfig(t *testing.T) { + compiler := NewCompiler() + + workflowData := &WorkflowData{ + Name: "Test", + SafeOutputs: &SafeOutputsConfig{ + AddComments: &AddCommentsConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + GitHubToken: "${{ secrets.TEMP_USER_PAT }}", + }, + TargetRepoSlug: "githubnext/gh-aw-side-repo", + AllowedRepos: []string{"githubnext/gh-aw-side-repo"}, + }, + }, + } + + var steps []string + compiler.addHandlerManagerConfigEnvVar(&steps, workflowData) + + require.NotEmpty(t, steps, "steps should not be empty") + stepsContent := strings.Join(steps, "") + + // Extract and parse the handler config JSON + handlerConfig := extractHandlerConfig(t, stepsContent) + + addComment, ok := handlerConfig["add_comment"] + require.True(t, ok, "add_comment config should be present") + + assert.Equal(t, "${{ secrets.TEMP_USER_PAT }}", addComment["github-token"], "github-token should be in handler config") + assert.Equal(t, "githubnext/gh-aw-side-repo", addComment["target-repo"], "target-repo should be in handler config") + + allowedRepos, ok := addComment["allowed_repos"] + require.True(t, ok, "allowed_repos should be present") + assert.Contains(t, allowedRepos, "githubnext/gh-aw-side-repo", "allowed_repos should contain the repo") +} + +// TestCreateIssueGitHubTokenInHandlerConfig verifies that github-token is included in +// the handler manager config JSON for create-issue. +func TestCreateIssueGitHubTokenInHandlerConfig(t *testing.T) { + compiler := NewCompiler() + + workflowData := &WorkflowData{ + Name: "Test", + SafeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + GitHubToken: "${{ secrets.TEMP_USER_PAT }}", + }, + TargetRepoSlug: "githubnext/gh-aw-side-repo", + AllowedRepos: []string{"githubnext/gh-aw-side-repo"}, + }, + }, + } + + var steps []string + compiler.addHandlerManagerConfigEnvVar(&steps, workflowData) + + require.NotEmpty(t, steps) + handlerConfig := extractHandlerConfig(t, strings.Join(steps, "")) + + createIssue, ok := handlerConfig["create_issue"] + require.True(t, ok, "create_issue config should be present") + + assert.Equal(t, "${{ secrets.TEMP_USER_PAT }}", createIssue["github-token"], "github-token should be in handler config") + assert.Equal(t, "githubnext/gh-aw-side-repo", createIssue["target-repo"], "target-repo should be in handler config") +} + +// TestCreateDiscussionGitHubTokenInHandlerConfig verifies that github-token is included in +// the handler manager config JSON for create-discussion. +func TestCreateDiscussionGitHubTokenInHandlerConfig(t *testing.T) { + compiler := NewCompiler() + + workflowData := &WorkflowData{ + Name: "Test", + SafeOutputs: &SafeOutputsConfig{ + CreateDiscussions: &CreateDiscussionsConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + GitHubToken: "${{ secrets.TEMP_USER_PAT }}", + }, + TargetRepoSlug: "githubnext/gh-aw-side-repo", + AllowedRepos: []string{"githubnext/gh-aw-side-repo"}, + }, + }, + } + + var steps []string + compiler.addHandlerManagerConfigEnvVar(&steps, workflowData) + + require.NotEmpty(t, steps) + handlerConfig := extractHandlerConfig(t, strings.Join(steps, "")) + + createDiscussion, ok := handlerConfig["create_discussion"] + require.True(t, ok, "create_discussion config should be present") + + assert.Equal(t, "${{ secrets.TEMP_USER_PAT }}", createDiscussion["github-token"], "github-token should be in handler config") +} + +// TestCreateCodeScanningAlertCrossRepoInHandlerConfig verifies that target-repo, allowed-repos, +// and github-token are included in the handler manager config for create-code-scanning-alert. +func TestCreateCodeScanningAlertCrossRepoInHandlerConfig(t *testing.T) { + compiler := NewCompiler() + + workflowData := &WorkflowData{ + Name: "Test", + SafeOutputs: &SafeOutputsConfig{ + CreateCodeScanningAlerts: &CreateCodeScanningAlertsConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + GitHubToken: "${{ secrets.TEMP_USER_PAT }}", + }, + TargetRepoSlug: "githubnext/gh-aw-side-repo", + AllowedRepos: []string{"githubnext/gh-aw-side-repo"}, + Driver: "test-scanner", + }, + }, + } + + var steps []string + compiler.addHandlerManagerConfigEnvVar(&steps, workflowData) + + require.NotEmpty(t, steps) + handlerConfig := extractHandlerConfig(t, strings.Join(steps, "")) + + alert, ok := handlerConfig["create_code_scanning_alert"] + require.True(t, ok, "create_code_scanning_alert config should be present") + + assert.Equal(t, "${{ secrets.TEMP_USER_PAT }}", alert["github-token"], "github-token should be in handler config") + assert.Equal(t, "githubnext/gh-aw-side-repo", alert["target-repo"], "target-repo should be in handler config") + assert.Equal(t, "test-scanner", alert["driver"], "driver should be in handler config") + + allowedRepos, ok := alert["allowed_repos"] + require.True(t, ok, "allowed_repos should be present") + assert.Contains(t, allowedRepos, "githubnext/gh-aw-side-repo", "allowed_repos should contain the repo") +} + +// TestUpdateIssueGitHubTokenInHandlerConfig verifies that github-token is included in +// the handler manager config JSON for update-issue. +func TestUpdateIssueGitHubTokenInHandlerConfig(t *testing.T) { + compiler := NewCompiler() + + bodyVal := true + workflowData := &WorkflowData{ + Name: "Test", + SafeOutputs: &SafeOutputsConfig{ + UpdateIssues: &UpdateIssuesConfig{ + UpdateEntityConfig: UpdateEntityConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + GitHubToken: "${{ secrets.TEMP_USER_PAT }}", + }, + SafeOutputTargetConfig: SafeOutputTargetConfig{ + TargetRepoSlug: "githubnext/gh-aw-side-repo", + AllowedRepos: []string{"githubnext/gh-aw-side-repo"}, + }, + }, + Body: &bodyVal, + }, + }, + } + + var steps []string + compiler.addHandlerManagerConfigEnvVar(&steps, workflowData) + + require.NotEmpty(t, steps) + handlerConfig := extractHandlerConfig(t, strings.Join(steps, "")) + + updateIssue, ok := handlerConfig["update_issue"] + require.True(t, ok, "update_issue config should be present") + + assert.Equal(t, "${{ secrets.TEMP_USER_PAT }}", updateIssue["github-token"], "github-token should be in handler config") + assert.Equal(t, "githubnext/gh-aw-side-repo", updateIssue["target-repo"], "target-repo should be in handler config") + + allowedRepos, ok := updateIssue["allowed_repos"] + require.True(t, ok, "allowed_repos should be present") + assert.Contains(t, allowedRepos, "githubnext/gh-aw-side-repo", "allowed_repos should contain the repo") +} + +// TestPushToPullRequestBranchCrossRepoInHandlerConfig verifies that target-repo and allowed-repos +// are included in the handler manager config JSON for push-to-pull-request-branch. +func TestPushToPullRequestBranchCrossRepoInHandlerConfig(t *testing.T) { + compiler := NewCompiler() + + workflowData := &WorkflowData{ + Name: "Test", + SafeOutputs: &SafeOutputsConfig{ + PushToPullRequestBranch: &PushToPullRequestBranchConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + GitHubToken: "${{ secrets.TEMP_USER_PAT }}", + }, + TargetRepoSlug: "githubnext/gh-aw-side-repo", + AllowedRepos: []string{"githubnext/gh-aw-side-repo"}, + }, + }, + } + + var steps []string + compiler.addHandlerManagerConfigEnvVar(&steps, workflowData) + + require.NotEmpty(t, steps) + handlerConfig := extractHandlerConfig(t, strings.Join(steps, "")) + + pushBranch, ok := handlerConfig["push_to_pull_request_branch"] + require.True(t, ok, "push_to_pull_request_branch config should be present") + + assert.Equal(t, "githubnext/gh-aw-side-repo", pushBranch["target-repo"], "target-repo should be in handler config") + + allowedRepos, ok := pushBranch["allowed_repos"] + require.True(t, ok, "allowed_repos should be present") + assert.Contains(t, allowedRepos, "githubnext/gh-aw-side-repo", "allowed_repos should contain the repo") +} + +// TestHandlerManagerStepPerOutputTokenInHandlerConfig verifies that per-output tokens +// (e.g., add-comment.github-token) are wired into the handler config JSON (GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG) +// but NOT used as the step-level with.github-token. The step-level token follows the same +// precedence as github_token.go: project token > global safe-outputs token > magic secrets. +func TestHandlerManagerStepPerOutputTokenInHandlerConfig(t *testing.T) { + compiler := NewCompiler() + + tests := []struct { + name string + safeOutputs *SafeOutputsConfig + expectedInHandlerConfig []string // tokens that should appear in handler config JSON + expectedNotInWithToken string // token that should NOT be in with.github-token (per-output tokens) + expectedStepLevelFallback string // step-level token should use this instead + }{ + { + name: "add-comment token appears in handler config JSON, not step-level", + safeOutputs: &SafeOutputsConfig{ + AddComments: &AddCommentsConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + GitHubToken: "${{ secrets.TEMP_USER_PAT }}", + }, + TargetRepoSlug: "githubnext/gh-aw-side-repo", + AllowedRepos: []string{"githubnext/gh-aw-side-repo"}, + }, + }, + expectedInHandlerConfig: []string{"TEMP_USER_PAT"}, + expectedNotInWithToken: "github-token: ${{ secrets.TEMP_USER_PAT }}", + expectedStepLevelFallback: "GH_AW_GITHUB_TOKEN", + }, + { + name: "create-issue token appears in handler config JSON, not step-level", + safeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + GitHubToken: "${{ secrets.TEMP_USER_PAT }}", + }, + TargetRepoSlug: "githubnext/gh-aw-side-repo", + AllowedRepos: []string{"githubnext/gh-aw-side-repo"}, + }, + }, + expectedInHandlerConfig: []string{"TEMP_USER_PAT"}, + expectedNotInWithToken: "github-token: ${{ secrets.TEMP_USER_PAT }}", + expectedStepLevelFallback: "GH_AW_GITHUB_TOKEN", + }, + { + name: "global safe-outputs token is used for step-level with.github-token", + safeOutputs: &SafeOutputsConfig{ + GitHubToken: "${{ secrets.GLOBAL_PAT }}", + AddComments: &AddCommentsConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + GitHubToken: "${{ secrets.PER_OUTPUT_PAT }}", + }, + }, + }, + // Both tokens should appear in step content + expectedInHandlerConfig: []string{"GLOBAL_PAT", "PER_OUTPUT_PAT"}, + // Step-level should use global, not per-output + expectedNotInWithToken: "github-token: ${{ secrets.PER_OUTPUT_PAT }}", + expectedStepLevelFallback: "GLOBAL_PAT", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + workflowData := &WorkflowData{ + Name: "Test", + SafeOutputs: tt.safeOutputs, + } + + steps := compiler.buildHandlerManagerStep(workflowData) + stepsContent := strings.Join(steps, "") + + // Verify tokens appear somewhere in the step content (handler config JSON) + for _, token := range tt.expectedInHandlerConfig { + assert.Contains(t, stepsContent, token, + "handler manager step should include token %q in step content", token) + } + + // Verify per-output token is NOT used as the step-level with.github-token + if tt.expectedNotInWithToken != "" { + assert.NotContains(t, stepsContent, tt.expectedNotInWithToken, + "per-output token should not be used directly as step-level with.github-token") + } + + // Verify the step-level token uses the expected fallback + if tt.expectedStepLevelFallback != "" { + // Extract just the "with:" section to check step-level token + withIdx := strings.Index(stepsContent, " with:\n") + if withIdx >= 0 { + withSection := stepsContent[withIdx : withIdx+200] + assert.Contains(t, withSection, tt.expectedStepLevelFallback, + "step-level with.github-token should use %q", tt.expectedStepLevelFallback) + } + } + }) + } +} + +// TestParseAllowedReposFromConfig verifies the parseAllowedReposFromConfig helper function. +func TestParseAllowedReposFromConfig(t *testing.T) { + tests := []struct { + name string + input map[string]any + expected []string + }{ + { + name: "single repo as array", + input: map[string]any{ + "allowed-repos": []any{"owner/repo"}, + }, + expected: []string{"owner/repo"}, + }, + { + name: "multiple repos", + input: map[string]any{ + "allowed-repos": []any{"owner/repo1", "owner/repo2", "other-owner/repo3"}, + }, + expected: []string{"owner/repo1", "owner/repo2", "other-owner/repo3"}, + }, + { + name: "no allowed-repos key", + input: map[string]any{}, + expected: nil, + }, + { + name: "empty allowed-repos array", + input: map[string]any{ + "allowed-repos": []any{}, + }, + expected: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := parseAllowedReposFromConfig(tt.input) + if tt.expected == nil { + assert.Emptyf(t, result, "parseAllowedReposFromConfig should return nil or empty for: %s", tt.name) + } else { + assert.Equal(t, tt.expected, result, "parseAllowedReposFromConfig mismatch") + } + }) + } +} + +// extractHandlerConfig is a helper that parses the GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG +// JSON from the rendered step strings. +func extractHandlerConfig(t *testing.T, stepsContent string) map[string]map[string]any { + t.Helper() + + var configJSON string + for line := range strings.SplitSeq(stepsContent, "\n") { + if strings.Contains(line, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") { + parts := strings.SplitN(line, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: ", 2) + if len(parts) == 2 { + configJSON = strings.TrimSpace(parts[1]) + configJSON = strings.Trim(configJSON, "\"") + configJSON = strings.ReplaceAll(configJSON, "\\\"", "\"") + break + } + } + } + + require.NotEmpty(t, configJSON, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG env var not found in steps") + + var result map[string]map[string]any + err := json.Unmarshal([]byte(configJSON), &result) + require.NoError(t, err, "Handler config JSON should be valid: %s", configJSON) + + return result +}