diff --git a/actions/setup/js/add_comment.cjs b/actions/setup/js/add_comment.cjs index 167ee8baaa9..a98e4b54062 100644 --- a/actions/setup/js/add_comment.cjs +++ b/actions/setup/js/add_comment.cjs @@ -11,7 +11,7 @@ const { getRepositoryUrl } = require("./get_repository_url.cjs"); const { replaceTemporaryIdReferences, resolveSafeOutputIssueTarget } = require("./temporary_id.cjs"); const { getTrackerID } = require("./get_tracker_id.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); -const { parseBoolTemplatable } = require("./templatable.cjs"); +const { parseBoolTemplatable, parseIntTemplatable } = require("./templatable.cjs"); const { resolveTarget, isStagedMode } = require("./safe_output_helpers.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); @@ -507,6 +507,7 @@ async function main(config = {}) { const mentionsDisabled = config.mentions === false || config.mentions?.enabled === false; const preResolvedMentionAliases = !mentionsDisabled ? normalizeMentionAliases(config.allowedMentionAliases) : []; const configuredMentionAliases = !mentionsDisabled ? normalizeMentionAliases(config.mentions?.allowed) : []; + const maxMentions = mentionsDisabled ? undefined : parseIntTemplatable(config.mentions?.max, 50); // Create an authenticated GitHub client. Uses config["github-token"] when set // (for cross-repository operations), otherwise falls back to the step-level github. @@ -803,7 +804,7 @@ async function main(config = {}) { // Sanitize content to prevent injection attacks, allowing parent issue/PR/discussion authors // so they can be @mentioned in the generated comment. - processedBody = sanitizeContent(processedBody, { allowedAliases: allowedMentionAliases }); + processedBody = sanitizeContent(processedBody, { allowedAliases: allowedMentionAliases, maxMentions }); // Enforce max limits before processing (validates user-provided content) try { diff --git a/actions/setup/js/close_discussion.cjs b/actions/setup/js/close_discussion.cjs index 650a26c53c5..c540687c7a1 100644 --- a/actions/setup/js/close_discussion.cjs +++ b/actions/setup/js/close_discussion.cjs @@ -13,6 +13,7 @@ const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { ERR_NOT_FOUND } = require("./error_codes.cjs"); const { resolveNumberFromTemporaryId } = require("./temporary_id.cjs"); const { resolveAllowedMentionsFromPayload } = require("./resolve_mentions_from_payload.cjs"); +const { parseIntTemplatable } = require("./templatable.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); /** @@ -168,6 +169,7 @@ async function main(config = {}) { const maxCount = config.max || 10; const githubClient = await createAuthenticatedGitHubClient(config); const allowBody = config.allow_body !== false; // default true; false only when explicitly set to false + const maxMentions = parseIntTemplatable(config.mentions?.max, 50); let allowedMentionAliases = []; if (Array.isArray(config.allowedMentionAliases)) { allowedMentionAliases = config.allowedMentionAliases; @@ -307,7 +309,7 @@ async function main(config = {}) { core.info("close_discussion: allow-body is false — closing without a comment"); } } else if (item.body) { - const sanitizedBody = sanitizeContent(item.body, { allowedAliases: allowedMentionAliases }); + const sanitizedBody = sanitizeContent(item.body, { allowedAliases: allowedMentionAliases, maxMentions }); const comment = await addDiscussionComment(githubClient, discussion.id, sanitizedBody); core.info(`Added comment to discussion #${discussionNumber}: ${comment.url}`); commentUrl = comment.url; diff --git a/actions/setup/js/collect_ndjson_output.cjs b/actions/setup/js/collect_ndjson_output.cjs index 54070df3ff2..3e331b7202b 100644 --- a/actions/setup/js/collect_ndjson_output.cjs +++ b/actions/setup/js/collect_ndjson_output.cjs @@ -34,6 +34,7 @@ async function main() { // Extract mentions configuration from validation config const mentionsConfig = validationConfig?.mentions || null; + const maxMentions = parseIntTemplatable(mentionsConfig?.max, 50); // Resolve allowed mentions for the output collector // This determines which @mentions are allowed in the agent output @@ -43,7 +44,7 @@ async function main() { /** @type {number | undefined} */ let maxBotMentions; - function validateFieldWithInputSchema(value, fieldName, inputSchema, lineNum) { + function validateFieldWithInputSchema(value, fieldName, inputSchema, lineNum, allowedAliasesSeen) { if (inputSchema.required && (value === undefined || value === null)) { return { isValid: false, @@ -66,7 +67,7 @@ async function main() { error: `Line ${lineNum}: ${fieldName} must be a string`, }; } - normalizedValue = sanitizeContent(value, { allowedAliases: allowedMentions, maxBotMentions }); + normalizedValue = sanitizeContent(value, { allowedAliases: allowedMentions, maxMentions, maxBotMentions, allowedAliasesSeen }); break; case "boolean": if (typeof value !== "boolean") { @@ -97,11 +98,11 @@ async function main() { error: `Line ${lineNum}: ${fieldName} must be one of: ${inputSchema.options.join(", ")}`, }; } - normalizedValue = sanitizeContent(value, { allowedAliases: allowedMentions, maxBotMentions }); + normalizedValue = sanitizeContent(value, { allowedAliases: allowedMentions, maxMentions, maxBotMentions, allowedAliasesSeen }); break; default: if (typeof value === "string") { - normalizedValue = sanitizeContent(value, { allowedAliases: allowedMentions, maxBotMentions }); + normalizedValue = sanitizeContent(value, { allowedAliases: allowedMentions, maxMentions, maxBotMentions, allowedAliasesSeen }); } break; } @@ -120,9 +121,10 @@ async function main() { normalizedItem, }; } + const allowedAliasesSeen = new Set(); for (const [fieldName, inputSchema] of Object.entries(jobConfig.inputs)) { const fieldValue = item[fieldName]; - const validation = validateFieldWithInputSchema(fieldValue, fieldName, inputSchema, lineNum); + const validation = validateFieldWithInputSchema(fieldValue, fieldName, inputSchema, lineNum, allowedAliasesSeen); if (!validation.isValid && validation.error) { errors.push(validation.error); } else if (validation.normalizedValue !== undefined) { @@ -357,6 +359,7 @@ async function main() { if (hasValidationConfig(itemType)) { const validationResult = validateItem(item, itemType, i + 1, { allowedAliases: allowedMentions, + maxMentions, maxBotMentions, normalizeIssueClosingKeywords, dataEnabled: typeConfig !== null && typeof typeConfig === "object" && typeConfig.data_enabled === true, diff --git a/actions/setup/js/collect_ndjson_output.test.cjs b/actions/setup/js/collect_ndjson_output.test.cjs index 169e36f79ff..5f56b961804 100644 --- a/actions/setup/js/collect_ndjson_output.test.cjs +++ b/actions/setup/js/collect_ndjson_output.test.cjs @@ -1320,6 +1320,43 @@ describe("collect_ndjson_output.cjs", () => { parsedOutput = JSON.parse(outputCall[1]); expect(parsedOutput.items[0].body).toBe("Hey `@username` and `@org/team`, check this out! But preserve email@domain.com"); }), + it("should preserve allowed aliases after max when no more than max occur", async () => { + const allowed = Array.from({ length: 60 }, (_, i) => `user${i}`); + const validationPath = "/tmp/gh-aw/safeoutputs/validation.json"; + const validationConfig = JSON.parse(fs.readFileSync(validationPath, "utf8")); + validationConfig.mentions = { allowContext: false, allowed, max: 3 }; + fs.writeFileSync(validationPath, JSON.stringify(validationConfig)); + + const testFile = "/tmp/gh-aw/test-ndjson-output.txt"; + const ndjsonContent = '{"type":"create_issue","title":"Late allowlist entries","body":"Thanks @user57, @user58, and @user59"}'; + fs.writeFileSync(testFile, ndjsonContent); + process.env.GH_AW_SAFE_OUTPUTS = testFile; + fs.writeFileSync("/tmp/gh-aw/safeoutputs/config.json", '{"create_issue":true}'); + + await eval(`(async () => { ${collectScript}; await main(); })()`); + + const outputCall = mockCore.setOutput.mock.calls.find(call => call[0] === "output"); + const parsedOutput = JSON.parse(outputCall[1]); + expect(parsedOutput.items[0].body).toBe("Thanks @user57, @user58, and @user59"); + }), + it("should apply the mention limit across all fields in one item", async () => { + const validationPath = "/tmp/gh-aw/safeoutputs/validation.json"; + const validationConfig = JSON.parse(fs.readFileSync(validationPath, "utf8")); + validationConfig.mentions = { allowContext: false, allowed: ["user1", "user2", "user3", "user4"], max: 3 }; + fs.writeFileSync(validationPath, JSON.stringify(validationConfig)); + + const testFile = "/tmp/gh-aw/test-ndjson-output.txt"; + fs.writeFileSync(testFile, '{"type":"create_issue","title":"@user1 @user2","body":"@user3 @user4 @user1"}'); + process.env.GH_AW_SAFE_OUTPUTS = testFile; + fs.writeFileSync("/tmp/gh-aw/safeoutputs/config.json", '{"create_issue":true}'); + + await eval(`(async () => { ${collectScript}; await main(); })()`); + + const outputCall = mockCore.setOutput.mock.calls.find(call => call[0] === "output"); + const parsedOutput = JSON.parse(outputCall[1]); + expect(parsedOutput.items[0].title).toBe("@user1 @user2"); + expect(parsedOutput.items[0].body).toBe("@user3 `@user4` @user1"); + }), it("should neutralize bot trigger phrases", async () => { const testFile = "/tmp/gh-aw/test-ndjson-output.txt", ndjsonContent = '{"type": "create_issue", "title": "Bot Trigger Test", "body": "This fixes #123 and closes #456, also resolves #789"}'; diff --git a/actions/setup/js/create_discussion.cjs b/actions/setup/js/create_discussion.cjs index c24eb97f7b8..a77ecd22bfb 100644 --- a/actions/setup/js/create_discussion.cjs +++ b/actions/setup/js/create_discussion.cjs @@ -26,7 +26,7 @@ const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); const { isStagedMode } = require("./safe_output_helpers.cjs"); const { closeOlderDiscussions: closeOlderDiscussionsFunc } = require("./close_older_discussions.cjs"); -const { parseBoolTemplatable } = require("./templatable.cjs"); +const { parseBoolTemplatable, parseIntTemplatable } = require("./templatable.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); const { generateHistoryLink, generateHistoryUrl } = require("./generate_history_link.cjs"); const { MAX_LABELS } = require("./constants.cjs"); @@ -312,6 +312,7 @@ async function main(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 githubClient = await createAuthenticatedGitHubClient(config); + const maxMentions = parseIntTemplatable(config.mentions?.max, 50); let allowedMentionAliases = []; if (Array.isArray(config.allowedMentionAliases)) { allowedMentionAliases = config.allowedMentionAliases; @@ -502,7 +503,7 @@ async function main(config = {}) { const preSanitizeBodyLength = processedBody.trim().length; // Sanitize body content to neutralize @mentions, URLs, and other security risks - processedBody = sanitizeContent(processedBody, { allowedAliases: allowedMentionAliases }); + processedBody = sanitizeContent(processedBody, { allowedAliases: allowedMentionAliases, maxMentions }); if (minBodyLength > 0 && preSanitizeBodyLength < minBodyLength) { const error = `Discussion body length ${preSanitizeBodyLength} is below configured minimum ${minBodyLength}`; core.error(error); diff --git a/actions/setup/js/create_issue.cjs b/actions/setup/js/create_issue.cjs index e74e2d27451..95e91bdc3d2 100644 --- a/actions/setup/js/create_issue.cjs +++ b/actions/setup/js/create_issue.cjs @@ -20,7 +20,7 @@ const { renderTemplateFromFile } = require("./messages_core.cjs"); const { createExpirationLine, addExpirationToFooter } = require("./ephemerals.cjs"); const { MAX_SUB_ISSUES, getSubIssueCount, linkSubIssue } = require("./sub_issue_helpers.cjs"); const { closeOlderIssues, searchOlderIssues, addIssueComment } = require("./close_older_issues.cjs"); -const { parseBoolTemplatable } = require("./templatable.cjs"); +const { parseBoolTemplatable, parseIntTemplatable } = require("./templatable.cjs"); const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); const { isStagedMode } = require("./safe_output_helpers.cjs"); @@ -669,6 +669,7 @@ async function main(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 githubClient = await createAuthenticatedGitHubClient(config); + const maxMentions = parseIntTemplatable(config.mentions?.max, 50); let allowedMentionAliases = []; if (Array.isArray(config.allowedMentionAliases)) { allowedMentionAliases = config.allowedMentionAliases; @@ -915,7 +916,7 @@ async function main(config = {}) { processedBody = removeDuplicateTitleFromDescription(title, processedBody); // Sanitize body content to neutralize @mentions, URLs, and other security risks - processedBody = sanitizeContent(processedBody, { allowedAliases: allowedMentionAliases }); + processedBody = sanitizeContent(processedBody, { allowedAliases: allowedMentionAliases, maxMentions }); const bodyLines = processedBody.split("\n"); diff --git a/actions/setup/js/create_pr_review_comment.cjs b/actions/setup/js/create_pr_review_comment.cjs index 64360b5be2a..05882faf81e 100644 --- a/actions/setup/js/create_pr_review_comment.cjs +++ b/actions/setup/js/create_pr_review_comment.cjs @@ -12,6 +12,7 @@ const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); const { isTemplatableTrue, isStagedMode, logStagedPreviewInfo, checkRequiredFilter } = require("./safe_output_helpers.cjs"); const { resolveAllowedMentionsFromPayload } = require("./resolve_mentions_from_payload.cjs"); +const { parseIntTemplatable } = require("./templatable.cjs"); const { resolveInvocationContext } = require("./invocation_context_helpers.cjs"); const { ERR_VALIDATION } = require("./error_codes.cjs"); @@ -58,6 +59,7 @@ async function main(config = {}) { if (requiredLabels.length > 0) core.info(`Required labels (all): ${requiredLabels.join(", ")}`); if (requiredTitlePrefix) core.info(`Required title prefix: ${requiredTitlePrefix}`); + const maxMentions = parseIntTemplatable(config.mentions?.max, 50); let allowedMentionAliases = []; if (Array.isArray(config.allowedMentionAliases)) { allowedMentionAliases = config.allowedMentionAliases; @@ -376,7 +378,7 @@ async function main(config = {}) { const bufferedComment = { path: commentItem.path, line: line, - body: sanitizeContent(commentItem.body.trim(), { allowedAliases: allowedMentionAliases }), + body: sanitizeContent(commentItem.body.trim(), { allowedAliases: allowedMentionAliases, maxMentions }), side: side, }; diff --git a/actions/setup/js/create_project_status_update.cjs b/actions/setup/js/create_project_status_update.cjs index cf9a21a3562..754db86ade7 100644 --- a/actions/setup/js/create_project_status_update.cjs +++ b/actions/setup/js/create_project_status_update.cjs @@ -10,6 +10,7 @@ const { isTemporaryId, normalizeTemporaryId } = require("./temporary_id.cjs"); const { ERR_CONFIG, ERR_NOT_FOUND, ERR_PARSE, ERR_VALIDATION } = require("./error_codes.cjs"); const { logGraphQLError } = require("./github_api_helpers.cjs"); const { resolveAllowedMentionsFromPayload } = require("./resolve_mentions_from_payload.cjs"); +const { parseIntTemplatable } = require("./templatable.cjs"); /** * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction @@ -273,6 +274,7 @@ async function main(config = {}, githubClient = null) { if (!github) { throw new Error(`${ERR_CONFIG}: GitHub client is required but not provided. Either pass a github client to main() or ensure global.github is set by github-script action.`); } + const maxMentions = parseIntTemplatable(config.mentions?.max, 50); let allowedMentionAliases = []; if (Array.isArray(config.allowedMentionAliases)) { allowedMentionAliases = config.allowedMentionAliases; @@ -367,7 +369,7 @@ async function main(config = {}, githubClient = null) { const status = validateStatus(output.status); const startDate = formatDate(output.start_date); const targetDate = formatDate(output.target_date); - const body = sanitizeContent(String(output.body), { allowedAliases: allowedMentionAliases }); + const body = sanitizeContent(String(output.body), { allowedAliases: allowedMentionAliases, maxMentions }); core.info(`Creating status update: ${status} (${startDate} → ${targetDate})`); core.info(`Body preview: ${body.substring(0, 100)}${body.length > 100 ? "..." : ""}`); diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index f2a47731604..bc2a248ad0b 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -16,7 +16,7 @@ const { replaceTemporaryIdReferences, replaceTemporaryIdReferencesInPatch, getOr const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { addExpirationToFooter } = require("./ephemerals.cjs"); const { generateWorkflowIdMarker, generateWorkflowCallIdMarker, generateCloseKeyMarker, normalizeCloseOlderKey } = require("./generate_footer.cjs"); -const { parseBoolTemplatable } = require("./templatable.cjs"); +const { parseBoolTemplatable, parseIntTemplatable } = require("./templatable.cjs"); const { assembleMarkdownBodyParts } = require("./markdown_body_helpers.cjs"); const { getBodyHeader, getDisclosureHeader } = require("./messages_header.cjs"); const { generateHistoryUrl } = require("./generate_history_link.cjs"); @@ -782,6 +782,7 @@ async function main(config = {}) { // Tracks the pull requests created so far in this run so later messages can stack on top of them. const stackTracker = createStackTracker(); const githubClient = await createAuthenticatedGitHubClient(config); + const maxMentions = parseIntTemplatable(config.mentions?.max, 50); let allowedMentionAliases = []; if (Array.isArray(config.allowedMentionAliases)) { allowedMentionAliases = config.allowedMentionAliases; @@ -1504,7 +1505,7 @@ async function main(config = {}) { processedBody = removeDuplicateTitleFromDescription(title, processedBody); // Sanitize body content to neutralize @mentions, URLs, and other security risks - processedBody = sanitizeContent(processedBody, { allowedAliases: allowedMentionAliases }); + processedBody = sanitizeContent(processedBody, { allowedAliases: allowedMentionAliases, maxMentions }); // Auto-add "Fixes #N" closing keyword if triggered from an issue and not already present. // This ensures the triggering issue is auto-closed when the PR is merged. @@ -1898,7 +1899,7 @@ async function main(config = {}) { baseBranch, tempRef: createBundleTempRef(branchName), }); - const pushFailureMessage = sanitizeContent(neutralizeClosingKeywordsForIssueBody(getErrorMessage(pushError)), { allowedAliases: allowedMentionAliases }) + const pushFailureMessage = sanitizeContent(neutralizeClosingKeywordsForIssueBody(getErrorMessage(pushError)), { allowedAliases: allowedMentionAliases, maxMentions }) .replace(/\s+/g, " ") .trim(); const pushErrorSection = buildPushErrorSection(getErrorMessage(pushError), pushFailureMessage); @@ -2267,7 +2268,7 @@ gh pr create --title ${shellQuote(title)} --base ${shellQuote(baseBranch)} --hea branchName, baseBranch, }); - const pushFailureMessage = sanitizeContent(neutralizeClosingKeywordsForIssueBody(getErrorMessage(pushError)), { allowedAliases: allowedMentionAliases }) + const pushFailureMessage = sanitizeContent(neutralizeClosingKeywordsForIssueBody(getErrorMessage(pushError)), { allowedAliases: allowedMentionAliases, maxMentions }) .replace(/\s+/g, " ") .trim(); const pushErrorSection = buildPushErrorSection(getErrorMessage(pushError), pushFailureMessage); diff --git a/actions/setup/js/reply_to_pr_review_comment.cjs b/actions/setup/js/reply_to_pr_review_comment.cjs index 0d964eb561c..6fb33a01f1b 100644 --- a/actions/setup/js/reply_to_pr_review_comment.cjs +++ b/actions/setup/js/reply_to_pr_review_comment.cjs @@ -12,7 +12,7 @@ const { sanitizeContent } = require("./sanitize_content.cjs"); const { getPRNumber } = require("./update_context_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); const { isStagedMode, checkRequiredFilter } = require("./safe_output_helpers.cjs"); -const { parseBoolTemplatable } = require("./templatable.cjs"); +const { parseBoolTemplatable, parseIntTemplatable } = require("./templatable.cjs"); const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); const { resolveAllowedMentionsFromPayload } = require("./resolve_mentions_from_payload.cjs"); @@ -44,6 +44,7 @@ async function main(config = {}) { const requiredTitlePrefix = config.required_title_prefix || ""; if (requiredLabels.length > 0) core.info(`Required labels (all): ${requiredLabels.join(", ")}`); if (requiredTitlePrefix) core.info(`Required title prefix: ${requiredTitlePrefix}`); + const maxMentions = parseIntTemplatable(config.mentions?.max, 50); let allowedMentionAliases = []; if (Array.isArray(config.allowedMentionAliases)) { allowedMentionAliases = config.allowedMentionAliases; @@ -174,7 +175,7 @@ async function main(config = {}) { } // Inject CAUTION at top of body unconditionally if threat detection warning was raised - let finalBody = sanitizeContent(body, { allowedAliases: allowedMentionAliases }); + let finalBody = sanitizeContent(body, { allowedAliases: allowedMentionAliases, maxMentions }); const detectionCaution = getDetectionCautionAlert(workflowName, runUrl); if (detectionCaution) { finalBody = detectionCaution + "\n\n" + finalBody; diff --git a/actions/setup/js/resolve_mentions.cjs b/actions/setup/js/resolve_mentions.cjs index 81317aaa93c..4c265cc2dd2 100644 --- a/actions/setup/js/resolve_mentions.cjs +++ b/actions/setup/js/resolve_mentions.cjs @@ -22,7 +22,7 @@ function getRepoCacheKey(owner, repo) { * @property {string[]} allowedMentions - List of allowed mention usernames * @property {number} totalMentions - Total number of mentions found * @property {number} resolvedCount - Number of mentions resolved via API - * @property {boolean} limitExceeded - Whether the 50 mention limit was exceeded + * @property {boolean} limitExceeded - Whether the 50 unknown-mention resolution limit was exceeded */ /** @@ -172,12 +172,22 @@ async function resolveMentionsLazily(text, knownAuthors, owner, repo, github, co core.info(`Found ${totalMentions} unique mentions in text`); - // Limit to 50 mentions - filter out excess without API lookup - const limitExceeded = totalMentions > 50; - const mentionsToProcess = limitExceeded ? mentions.slice(0, 50) : mentions; + // Build set of known allowed authors (case-insensitive) + const knownAuthorsLowercase = new Set(knownAuthors.filter(a => a).map(a => a.toLowerCase())); + + // Limit unknown candidates to 50 while always preserving pre-authorized mentions. + let unknownMentionCount = 0; + const mentionsToProcess = mentions.filter(mention => { + if (knownAuthorsLowercase.has(mention.toLowerCase())) { + return true; + } + unknownMentionCount++; + return unknownMentionCount <= 50; + }); + const limitExceeded = unknownMentionCount > 50; if (limitExceeded) { - core.warning(`Mention limit exceeded: ${totalMentions} mentions found, processing only first 50`); + core.warning(`Mention limit exceeded: ${unknownMentionCount} unknown mentions found, processing only first 50`); } if (mentionsToProcess.length === 0) { @@ -190,9 +200,6 @@ async function resolveMentionsLazily(text, knownAuthors, owner, repo, github, co }; } - // Build set of known allowed authors (case-insensitive) - const knownAuthorsLowercase = new Set(knownAuthors.filter(a => a).map(a => a.toLowerCase())); - // Optimistically fetch recent collaborators (first page only) const collaboratorCache = await getRecentCollaborators(owner, repo, github, core); core.info(`Cached ${collaboratorCache.size} recent collaborators for optimistic resolution`); diff --git a/actions/setup/js/resolve_mentions.test.cjs b/actions/setup/js/resolve_mentions.test.cjs index 7b106b277f4..1be98f89b40 100644 --- a/actions/setup/js/resolve_mentions.test.cjs +++ b/actions/setup/js/resolve_mentions.test.cjs @@ -149,6 +149,12 @@ describe("resolve_mentions.cjs", () => { result = await resolveMentionsLazily(mentions, [], "owner", "repo", mockGithub, mockCore); (expect(result.totalMentions).toBe(60), expect(result.limitExceeded).toBe(!0), expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Mention limit exceeded"))); }), + it("should not count known authors toward the 50 mention resolution limit", async () => { + const knownAuthors = Array.from({ length: 60 }, (_, i) => `known${i}`), + mentions = knownAuthors.map(author => `@${author}`).join(" "), + result = await resolveMentionsLazily(mentions, knownAuthors, "owner", "repo", mockGithub, mockCore); + (expect(result.allowedMentions).toEqual(knownAuthors), expect(result.limitExceeded).toBe(!1)); + }), it("should preserve case in allowed mentions", async () => { mockGithub.rest.repos.listCollaborators.mockResolvedValue({ data: [{ login: "maintainer1", type: "User", permissions: { maintain: !0, admin: !1, push: !1 } }] }); const result = await resolveMentionsLazily("Hello @Maintainer1", [], "owner", "repo", mockGithub, mockCore); diff --git a/actions/setup/js/resolve_mentions_from_payload.cjs b/actions/setup/js/resolve_mentions_from_payload.cjs index 881b43f2cc5..1191248b939 100644 --- a/actions/setup/js/resolve_mentions_from_payload.cjs +++ b/actions/setup/js/resolve_mentions_from_payload.cjs @@ -191,7 +191,6 @@ async function resolveAllowedMentionsFromPayload(context, github, core, mentions const allowContext = mentionsConfig?.allowContext !== false; // default: true const allowedList = mentionsConfig?.allowed || []; const allowedTeams = mentionsConfig?.allowedTeams || []; - const maxMentions = mentionsConfig?.max || 50; try { const { owner, repo } = context.repo; @@ -234,23 +233,14 @@ async function resolveAllowedMentionsFromPayload(context, github, core, mentions // If collaborator mentions are disabled, only use known authors (context + allowed list) if (!allowCollaboratorMentions) { core.info(`[MENTIONS] Collaborator mentions disabled - only allowing context (${deduplicatedKnownAuthors.length} users)`); - if (deduplicatedKnownAuthors.length > maxMentions) { - core.warning(`[MENTIONS] Mention limit exceeded: ${deduplicatedKnownAuthors.length} mentions, limiting to ${maxMentions}`); - } - return deduplicatedKnownAuthors.slice(0, maxMentions); + return deduplicatedKnownAuthors; } // Build allowed mentions list from known authors and collaborators // We pass the known authors as fake mentions in text so they get processed const fakeText = deduplicatedKnownAuthors.map(author => `@${author}`).join(" "); const mentionResult = await resolveMentionsLazily(fakeText, deduplicatedKnownAuthors, owner, repo, github, core); - let allowedMentions = mentionResult.allowedMentions; - - // Apply max limit - if (allowedMentions.length > maxMentions) { - core.warning(`[MENTIONS] Mention limit exceeded: ${allowedMentions.length} mentions, limiting to ${maxMentions}`); - allowedMentions = allowedMentions.slice(0, maxMentions); - } + const allowedMentions = mentionResult.allowedMentions; if (allowedMentions.length > 0) { core.info(`[OUTPUT COLLECTOR] Allowed mentions: ${allowedMentions.join(", ")}`); diff --git a/actions/setup/js/resolve_mentions_from_payload.test.cjs b/actions/setup/js/resolve_mentions_from_payload.test.cjs index 44ab2584e9f..98894e6282e 100644 --- a/actions/setup/js/resolve_mentions_from_payload.test.cjs +++ b/actions/setup/js/resolve_mentions_from_payload.test.cjs @@ -321,7 +321,7 @@ describe("resolveAllowedMentionsFromPayload", () => { expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("extra known author")); }); - it("applies max limit when allowTeamMembers is false", async () => { + it("does not truncate known authors to max when allowTeamMembers is false", async () => { const context = { eventName: "issues", payload: { @@ -336,25 +336,22 @@ describe("resolveAllowedMentionsFromPayload", () => { allowTeamMembers: false, max: 3, }); - expect(result.length).toBeLessThanOrEqual(3); + expect(result).toEqual(["alice", "user0", "user1", "user2", "user3", "user4"]); }); - it("warns when mention limit is exceeded with team members disabled", async () => { + it("does not truncate resolved aliases to max", async () => { const context = { - eventName: "issues", - payload: { - issue: { - user: null, - assignees: Array.from({ length: 5 }, (_, i) => ({ login: `user${i}`, type: "User" })), - }, - }, + eventName: "workflow_dispatch", + payload: {}, repo: { owner: "o", repo: "r" }, }; - await resolveAllowedMentionsFromPayload(context, mockGithub, mockCore, { - allowTeamMembers: false, - max: 2, + const allowed = Array.from({ length: 60 }, (_, i) => `user${i}`); + const result = await resolveAllowedMentionsFromPayload(context, mockGithub, mockCore, { + allowContext: false, + allowed, + max: 3, }); - expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Mention limit exceeded")); + expect(result).toEqual(allowed); }); it("returns empty array and logs warning on error", async () => { diff --git a/actions/setup/js/safe_output_handler_manager.cjs b/actions/setup/js/safe_output_handler_manager.cjs index d6ea1c9660d..f28480a932c 100644 --- a/actions/setup/js/safe_output_handler_manager.cjs +++ b/actions/setup/js/safe_output_handler_manager.cjs @@ -21,6 +21,7 @@ const { getAssignToAgentAssigned, getAssignToAgentErrors, getAssignToAgentErrorC const { createPrReviewBufferRegistry } = require("./pr_review_buffer.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); const { resolveAllowedMentionsFromPayload } = require("./resolve_mentions_from_payload.cjs"); +const { parseIntTemplatable } = require("./templatable.cjs"); const { createManifestLogger, ensureManifestExists, extractCreatedItemFromResult, writeTemporaryIdMapFile, writeSafeOutputErrorReport } = require("./safe_output_manifest.cjs"); const { loadCustomSafeOutputJobTypes, loadCustomSafeOutputScriptHandlers, loadCustomSafeOutputActionHandlers, isStagedMode } = require("./safe_output_helpers.cjs"); const { emitSafeOutputActionOutputs } = require("./safe_outputs_action_outputs.cjs"); @@ -1365,9 +1366,11 @@ function getContentToCheck(messageType, message, result) { * @param {string} repo - Repository in "owner/repo" format * @param {number} issueNumber - Issue number to update * @param {string} updatedBody - Updated body content with resolved temp IDs + * @param {string[]} [allowedMentionAliases] - Mention aliases allowed by the workflow + * @param {number} [maxMentions] - Maximum distinct allowed mentions to preserve * @returns {Promise} */ -async function updateIssueBody(github, context, repo, issueNumber, updatedBody, allowedMentionAliases = []) { +async function updateIssueBody(github, context, repo, issueNumber, updatedBody, allowedMentionAliases = [], maxMentions = undefined) { const [owner, repoName] = repo.split("/"); core.info(`Updating issue ${repo}#${issueNumber} body with resolved temporary IDs`); @@ -1376,7 +1379,7 @@ async function updateIssueBody(github, context, repo, issueNumber, updatedBody, owner, repo: repoName, issue_number: issueNumber, - body: sanitizeContent(updatedBody, { allowedAliases: allowedMentionAliases }), + body: sanitizeContent(updatedBody, { allowedAliases: allowedMentionAliases, maxMentions }), }); core.info(`✓ Updated issue ${repo}#${issueNumber}`); @@ -1389,9 +1392,11 @@ async function updateIssueBody(github, context, repo, issueNumber, updatedBody, * @param {string} repo - Repository in "owner/repo" format * @param {number} prNumber - Pull request number to update * @param {string} updatedBody - Updated body content with resolved temp IDs + * @param {string[]} [allowedMentionAliases] - Mention aliases allowed by the workflow + * @param {number} [maxMentions] - Maximum distinct allowed mentions to preserve * @returns {Promise} */ -async function updatePullRequestBody(github, context, repo, prNumber, updatedBody, allowedMentionAliases = []) { +async function updatePullRequestBody(github, context, repo, prNumber, updatedBody, allowedMentionAliases = [], maxMentions = undefined) { const [owner, repoName] = repo.split("/"); core.info(`Updating pull request ${repo}#${prNumber} body with resolved temporary IDs`); @@ -1400,7 +1405,7 @@ async function updatePullRequestBody(github, context, repo, prNumber, updatedBod owner, repo: repoName, pull_number: prNumber, - body: sanitizeContent(updatedBody, { allowedAliases: allowedMentionAliases }), + body: sanitizeContent(updatedBody, { allowedAliases: allowedMentionAliases, maxMentions }), }); core.info(`✓ Updated pull request ${repo}#${prNumber}`); @@ -1413,9 +1418,11 @@ async function updatePullRequestBody(github, context, repo, prNumber, updatedBod * @param {string} repo - Repository in "owner/repo" format * @param {number} discussionNumber - Discussion number to update * @param {string} updatedBody - Updated body content with resolved temp IDs + * @param {string[]} [allowedMentionAliases] - Mention aliases allowed by the workflow + * @param {number} [maxMentions] - Maximum distinct allowed mentions to preserve * @returns {Promise} */ -async function updateDiscussionBody(github, context, repo, discussionNumber, updatedBody, allowedMentionAliases = []) { +async function updateDiscussionBody(github, context, repo, discussionNumber, updatedBody, allowedMentionAliases = [], maxMentions = undefined) { const [owner, repoName] = repo.split("/"); core.info(`Updating discussion ${repo}#${discussionNumber} body with resolved temporary IDs`); @@ -1453,7 +1460,7 @@ async function updateDiscussionBody(github, context, repo, discussionNumber, upd await github.graphql(mutation, { discussionId, - body: sanitizeContent(updatedBody, { allowedAliases: allowedMentionAliases }), + body: sanitizeContent(updatedBody, { allowedAliases: allowedMentionAliases, maxMentions }), }); core.info(`✓ Updated discussion ${repo}#${discussionNumber}`); @@ -1467,14 +1474,16 @@ async function updateDiscussionBody(github, context, repo, discussionNumber, upd * @param {number} commentId - Comment ID to update * @param {string} updatedBody - Updated body content with resolved temp IDs * @param {boolean} isDiscussion - Whether this is a discussion comment + * @param {string[]} [allowedMentionAliases] - Mention aliases allowed by the workflow + * @param {number} [maxMentions] - Maximum distinct allowed mentions to preserve * @returns {Promise} */ -async function updateCommentBody(github, context, repo, commentId, updatedBody, isDiscussion = false, allowedMentionAliases = []) { +async function updateCommentBody(github, context, repo, commentId, updatedBody, isDiscussion = false, allowedMentionAliases = [], maxMentions = undefined) { const [owner, repoName] = repo.split("/"); core.info(`Updating comment ${commentId} body with resolved temporary IDs`); - const sanitizedBody = sanitizeContent(updatedBody, { allowedAliases: allowedMentionAliases }); + const sanitizedBody = sanitizeContent(updatedBody, { allowedAliases: allowedMentionAliases, maxMentions }); if (isDiscussion) { // For discussion comments, we need to use GraphQL @@ -1514,9 +1523,11 @@ async function updateCommentBody(github, context, repo, commentId, updatedBody, * @param {Array<{type: string, message: any, result: any, originalTempIdMapSize: number}>} trackedOutputs - Outputs that need updating * @param {Map} temporaryIdMap - Current temporary ID map * @param {Map} [artifactUrlMap] - Optional artifact URL map for resolving artifact references + * @param {string[]} [allowedMentionAliases] - Mention aliases allowed by the workflow + * @param {number} [maxMentions] - Maximum distinct allowed mentions to preserve * @returns {Promise} Number of successful updates */ -async function processSyntheticUpdates(github, context, trackedOutputs, temporaryIdMap, artifactUrlMap, allowedMentionAliases = []) { +async function processSyntheticUpdates(github, context, trackedOutputs, temporaryIdMap, artifactUrlMap, allowedMentionAliases = [], maxMentions = undefined) { let updateCount = 0; core.info(`\n=== Processing Synthetic Updates ===`); @@ -1549,17 +1560,17 @@ async function processSyntheticUpdates(github, context, trackedOutputs, temporar // Update based on the original type switch (tracked.type) { case "create_issue": - await updateIssueBody(github, context, tracked.result.repo, tracked.result.number, updatedContent, allowedMentionAliases); + await updateIssueBody(github, context, tracked.result.repo, tracked.result.number, updatedContent, allowedMentionAliases, maxMentions); updateCount++; break; case "create_discussion": - await updateDiscussionBody(github, context, tracked.result.repo, tracked.result.number, updatedContent, allowedMentionAliases); + await updateDiscussionBody(github, context, tracked.result.repo, tracked.result.number, updatedContent, allowedMentionAliases, maxMentions); updateCount++; break; case "add_comment": // Update comment using the tracked comment ID if (tracked.result.commentId) { - await updateCommentBody(github, context, tracked.result.repo, tracked.result.commentId, updatedContent, tracked.result.isDiscussion, allowedMentionAliases); + await updateCommentBody(github, context, tracked.result.repo, tracked.result.commentId, updatedContent, tracked.result.isDiscussion, allowedMentionAliases, maxMentions); updateCount++; } else { core.debug(`Skipping synthetic update for comment - comment ID not tracked`); @@ -1567,14 +1578,14 @@ async function processSyntheticUpdates(github, context, trackedOutputs, temporar break; case "comment_memory": if (tracked.result.commentId) { - await updateCommentBody(github, context, tracked.result.repo, tracked.result.commentId, updatedContent, false, allowedMentionAliases); + await updateCommentBody(github, context, tracked.result.repo, tracked.result.commentId, updatedContent, false, allowedMentionAliases, maxMentions); updateCount++; } else { core.debug(`Skipping synthetic update for comment_memory - comment ID not tracked`); } break; case "create_pull_request": - await updatePullRequestBody(github, context, tracked.result.repo, tracked.result.number, updatedContent, allowedMentionAliases); + await updatePullRequestBody(github, context, tracked.result.repo, tracked.result.number, updatedContent, allowedMentionAliases, maxMentions); updateCount++; break; default: @@ -1679,6 +1690,7 @@ async function main() { } const allowedMentionAliases = config.mentions != null ? await resolveAllowedMentionsFromPayload(context, github, core, config.mentions) : []; + const maxMentions = parseIntTemplatable(config.mentions?.max, 50); // Load and initialize handlers based on configuration (factory pattern) const messageHandlers = await loadHandlers(config, prReviewBufferRegistry, allowedMentionAliases); @@ -1762,7 +1774,7 @@ async function main() { // Convert temp ID map back to Map const temporaryIdMap = new Map(Object.entries(processingResult.temporaryIdMap)); - syntheticUpdateCount = await processSyntheticUpdates(github, context, processingResult.outputsWithUnresolvedIds, temporaryIdMap, processingResult.artifactUrlMap, allowedMentionAliases); + syntheticUpdateCount = await processSyntheticUpdates(github, context, processingResult.outputsWithUnresolvedIds, temporaryIdMap, processingResult.artifactUrlMap, allowedMentionAliases, maxMentions); } // Write step summaries for all processed safe-outputs diff --git a/actions/setup/js/safe_output_type_validator.cjs b/actions/setup/js/safe_output_type_validator.cjs index 89c96f31ad6..40eef4da145 100644 --- a/actions/setup/js/safe_output_type_validator.cjs +++ b/actions/setup/js/safe_output_type_validator.cjs @@ -31,6 +31,8 @@ const ISSUE_INTENT_RATIONALE_MAX_LENGTH = 280; /** * @typedef {{ * allowedAliases?: string[], + * maxMentions?: number, + * allowedAliasesSeen?: Set, * maxBotMentions?: number, * normalizeIssueClosingKeywords?: boolean, * dataEnabled?: boolean, @@ -90,6 +92,8 @@ function normalizeIssueIntentRationale(rationale, options) { const sanitizedRationale = sanitizeContent(unfenceMarkdown(rationale), { maxLength: ISSUE_INTENT_RATIONALE_MAX_LENGTH, allowedAliases: options?.allowedAliases || [], + maxMentions: options?.maxMentions, + allowedAliasesSeen: options?.allowedAliasesSeen, maxBotMentions: options?.maxBotMentions, }).trim(); // sanitizeContent appends "\n[Content truncated due to length]" when it truncates, @@ -116,6 +120,8 @@ function validateIssueIntentLabels(value, lineNum, itemType, fieldName, options) const name = sanitizeContent(label, { maxLength: 128, allowedAliases: options?.allowedAliases || [], + maxMentions: options?.maxMentions, + allowedAliasesSeen: options?.allowedAliasesSeen, maxBotMentions: options?.maxBotMentions, }); if (!name) { @@ -149,6 +155,8 @@ function validateIssueIntentLabels(value, lineNum, itemType, fieldName, options) const name = sanitizeContent(label.name, { maxLength: 128, allowedAliases: options?.allowedAliases || [], + maxMentions: options?.maxMentions, + allowedAliasesSeen: options?.allowedAliasesSeen, maxBotMentions: options?.maxBotMentions, }); if (!name) { @@ -544,6 +552,8 @@ function validateField(value, fieldName, validation, itemType, lineNum, options) normalizedResult = sanitizeContent(normalizedResult, { maxLength: validation.maxLength, allowedAliases: options?.allowedAliases || [], + maxMentions: options?.maxMentions, + allowedAliasesSeen: options?.allowedAliasesSeen, maxBotMentions: options?.maxBotMentions, }); } @@ -557,6 +567,8 @@ function validateField(value, fieldName, validation, itemType, lineNum, options) finalValue = sanitizeContent(unfenceMarkdown(value), { maxLength: validation.maxLength || MAX_BODY_LENGTH, allowedAliases: options?.allowedAliases || [], + maxMentions: options?.maxMentions, + allowedAliasesSeen: options?.allowedAliasesSeen, maxBotMentions: options?.maxBotMentions, }); } @@ -635,6 +647,8 @@ function validateField(value, fieldName, validation, itemType, lineNum, options) ? sanitizeContent(item, { maxLength: validation.itemMaxLength || 128, allowedAliases: options?.allowedAliases || [], + maxMentions: options?.maxMentions, + allowedAliasesSeen: options?.allowedAliasesSeen, maxBotMentions: options?.maxBotMentions, }) : item @@ -753,10 +767,13 @@ function validateItem(item, itemType, lineNum, options) { } } + // Share mention state across every sanitized field in this output item. + const validationOptions = { ...options, allowedAliasesSeen: new Set() }; + // Validate each configured field for (const [fieldName, validation] of Object.entries(typeConfig.fields)) { const fieldValue = item[fieldName]; - const result = validateField(fieldValue, fieldName, validation, itemType, lineNum, options); + const result = validateField(fieldValue, fieldName, validation, itemType, lineNum, validationOptions); if (!result.isValid) { // When x-strip-on-error is set, strip the invalid optional field instead of rejecting the item. diff --git a/actions/setup/js/sanitize_content.cjs b/actions/setup/js/sanitize_content.cjs index 62bc5f359d6..ddf832a66f6 100644 --- a/actions/setup/js/sanitize_content.cjs +++ b/actions/setup/js/sanitize_content.cjs @@ -41,6 +41,8 @@ const RUNTIME_TO_MENTION_ALIAS_MAP = { * @typedef {Object} SanitizeOptions * @property {number} [maxLength] - Maximum length of content (default: 524288) * @property {string[]} [allowedAliases] - List of aliases (@mentions) that should not be neutralized + * @property {number} [maxMentions] - Maximum number of unique allowed aliases to preserve + * @property {Set} [allowedAliasesSeen] - Allowed aliases already preserved in this output item * @property {number} [maxBotMentions] - Maximum bot trigger references before filtering (default: 10) */ @@ -57,7 +59,11 @@ function sanitizeContent(content, maxLengthOrOptions) { /** @type {string[]} */ let allowedAliasesLowercase = []; /** @type {number | undefined} */ + let maxMentions; + /** @type {number | undefined} */ let maxBotMentions; + /** @type {Set | undefined} */ + let allowedAliasesSeen; if (typeof maxLengthOrOptions === "number") { maxLength = maxLengthOrOptions; @@ -66,7 +72,9 @@ function sanitizeContent(content, maxLengthOrOptions) { // Pre-process allowed aliases to lowercase for efficient comparison const normalizedAllowedAliases = normalizeAllowedAliases(maxLengthOrOptions.allowedAliases); allowedAliasesLowercase = expandAllowedAliases(normalizedAllowedAliases); + maxMentions = maxLengthOrOptions.maxMentions; maxBotMentions = maxLengthOrOptions.maxBotMentions; + allowedAliasesSeen = maxLengthOrOptions.allowedAliasesSeen; } // If no allowed aliases specified, use core sanitization (which neutralizes all mentions) @@ -124,7 +132,7 @@ function sanitizeContent(content, maxLengthOrOptions) { // Neutralize mentions after truncation so the length boundary cannot split an // inserted code-span delimiter and reactivate a mention. - sanitized = neutralizeMentions(sanitized, allowedAliasesLowercase); + sanitized = neutralizeMentions(sanitized, allowedAliasesLowercase, maxMentions, allowedAliasesSeen); // Neutralize GitHub references if restrictions are configured sanitized = neutralizeGitHubReferences(sanitized, allowedGitHubRefs); @@ -185,18 +193,24 @@ function sanitizeContent(content, maxLengthOrOptions) { * Neutralize @mentions with selective filtering * @param {string} s - The string to process * @param {string[]} allowedLowercase - List of allowed aliases (lowercase) + * @param {number | undefined} maxAllowed - Maximum number of unique allowed aliases to preserve + * @param {Set | undefined} seenAllowedAliases - Allowed aliases already preserved in this output item * @returns {string} Processed string */ - function neutralizeMentions(s, allowedLowercase) { + function neutralizeMentions(s, allowedLowercase, maxAllowed, seenAllowedAliases) { const wrapInCodeSpan = createRenderSafeCodeSpanWrapper(s); + const allowedAliasesSeen = seenAllowedAliases || new Set(); return applyToNonCodeRegions(s, (segment, regionBefore = "", regionAfter = "") => { return segment.replace(/(^|[^A-Za-z0-9])@([A-Za-z0-9](?:[A-Za-z0-9_-]{0,37}[A-Za-z0-9])?(?:\/[A-Za-z0-9._-]+)?)/g, (match, prefix, alias, offset) => { - const isAllowed = allowedLowercase.includes(alias.toLowerCase()); - if (isAllowed) { + const normalizedAlias = alias.toLowerCase(); + const isAllowed = allowedLowercase.includes(normalizedAlias); + const isWithinLimit = maxAllowed === undefined || allowedAliasesSeen.has(normalizedAlias) || allowedAliasesSeen.size < maxAllowed; + if (isAllowed && isWithinLimit) { + allowedAliasesSeen.add(normalizedAlias); return `${prefix}@${alias}`; } if (typeof core !== "undefined" && core.info) { - core.info(`Escaped mention: @${alias} (not in allowed list)`); + core.info(isAllowed ? `Escaped mention: @${alias} (mention limit exceeded)` : `Escaped mention: @${alias} (not in allowed list)`); } const before = prefix || (offset === 0 ? regionBefore : ""); const after = segment[offset + match.length] || (offset + match.length === segment.length ? regionAfter : ""); diff --git a/actions/setup/js/sanitize_content.test.cjs b/actions/setup/js/sanitize_content.test.cjs index 19664d207a3..8f9100432ec 100644 --- a/actions/setup/js/sanitize_content.test.cjs +++ b/actions/setup/js/sanitize_content.test.cjs @@ -284,6 +284,23 @@ describe("sanitize_content.cjs", () => { expect(result).toBe("Hello @user1 and @user2 and `@other`"); }); + it("should apply max only to allowed aliases present in the message", () => { + const allowedAliases = Array.from({ length: 10 }, (_, i) => `user${i}`); + const result = sanitizeContent("Hello @user7, @user8, and @user9", { + allowedAliases, + maxMentions: 3, + }); + expect(result).toBe("Hello @user7, @user8, and @user9"); + }); + + it("should neutralize additional distinct allowed aliases after max is reached", () => { + const result = sanitizeContent("@user1 @other @user2 @user3 @user1", { + allowedAliases: ["user1", "user2", "user3"], + maxMentions: 2, + }); + expect(result).toBe("@user1 `@other` @user2 `@user3` @user1"); + }); + it("should work with options object containing both maxLength and allowedAliases", () => { const result = sanitizeContent("Hello @author and @other", { maxLength: 524288, diff --git a/actions/setup/js/types/safe-output-script.d.ts b/actions/setup/js/types/safe-output-script.d.ts index 14cc1ff3e99..6b07e455a12 100644 --- a/actions/setup/js/types/safe-output-script.d.ts +++ b/actions/setup/js/types/safe-output-script.d.ts @@ -173,6 +173,8 @@ export interface SanitizeOptions { maxLength?: number; /** `@mention` aliases that should NOT be neutralized. */ allowedAliases?: string[]; + /** Maximum distinct allowed aliases to preserve. */ + maxMentions?: number; /** Maximum bot-trigger references before filtering (default: 10). */ maxBotMentions?: number; } diff --git a/pkg/workflow/safe_outputs_config_generation_test.go b/pkg/workflow/safe_outputs_config_generation_test.go index e3a91f895e0..1d4fa7c3ae2 100644 --- a/pkg/workflow/safe_outputs_config_generation_test.go +++ b/pkg/workflow/safe_outputs_config_generation_test.go @@ -280,7 +280,7 @@ func TestGenerateSafeOutputsConfigMissingToolWithIssue(t *testing.T) { func TestGenerateSafeOutputsConfigMentions(t *testing.T) { enabled := true allowedCollaborators := false - max := 5 + max := "5" data := &WorkflowData{ SafeOutputs: &SafeOutputsConfig{ @@ -307,6 +307,24 @@ func TestGenerateSafeOutputsConfigMentions(t *testing.T) { assert.InDelta(t, float64(5), mentions["max"], 0.0001, "max should be 5") } +func TestGenerateSafeOutputsConfigMentionsTemplatableMax(t *testing.T) { + max := "${{ inputs.max-mentions }}" + data := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + Mentions: &MentionsConfig{Max: &max}, + }, + } + + result, err := generateSafeOutputsConfig(data) + require.NoError(t, err) + + var parsed map[string]any + require.NoError(t, json.Unmarshal([]byte(result), &parsed)) + mentions, ok := parsed["mentions"].(map[string]any) + require.True(t, ok) + assert.Equal(t, max, mentions["max"]) +} + func TestGenerateSafeOutputsConfigNormalizeClosingKeywordsPerType(t *testing.T) { enabled := true data := &WorkflowData{ diff --git a/pkg/workflow/safe_outputs_config_runtime.go b/pkg/workflow/safe_outputs_config_runtime.go index aa882002d5e..f372c51452a 100644 --- a/pkg/workflow/safe_outputs_config_runtime.go +++ b/pkg/workflow/safe_outputs_config_runtime.go @@ -142,7 +142,11 @@ func buildMentionsHandlerConfig(m *MentionsConfig) map[string]any { cfg["allowedTeams"] = m.AllowedTeams } if m.Max != nil { - cfg["max"] = *m.Max + if n := templatableIntValue(m.Max); n > 0 { + cfg["max"] = n + } else { + cfg["max"] = *m.Max + } } return cfg } diff --git a/pkg/workflow/safe_outputs_config_types.go b/pkg/workflow/safe_outputs_config_types.go index 44acd0d6e48..ca70f2da580 100644 --- a/pkg/workflow/safe_outputs_config_types.go +++ b/pkg/workflow/safe_outputs_config_types.go @@ -173,8 +173,8 @@ type MentionsConfig struct { // but the workflow will not fail. AllowedTeams []string `yaml:"allowed-teams,omitempty" json:"allowedTeams,omitempty"` - // Max is the maximum number of mentions per message (default: 50) - Max *int `yaml:"max,omitempty" json:"max,omitempty"` + // Max is the maximum number of mentions per message (default: 50). Supports integer or GitHub Actions expression. + Max *string `yaml:"max,omitempty" json:"max,omitempty"` } // SecretMaskingConfig holds configuration for secret redaction behavior diff --git a/pkg/workflow/safe_outputs_mentions_test.go b/pkg/workflow/safe_outputs_mentions_test.go index c561f637e8b..49664a69bae 100644 --- a/pkg/workflow/safe_outputs_mentions_test.go +++ b/pkg/workflow/safe_outputs_mentions_test.go @@ -68,7 +68,7 @@ func TestParseMentionsConfig_Object(t *testing.T) { AllowedCollaborators: boolPtr(true), AllowContext: boolPtr(false), Allowed: []string{"bot1", "bot2"}, - Max: new(10), + Max: strPtr("10"), }, }, { @@ -79,7 +79,7 @@ func TestParseMentionsConfig_Object(t *testing.T) { }, expected: &MentionsConfig{ Allowed: []string{"bot1"}, - Max: new(5), + Max: strPtr("5"), }, }, { @@ -152,7 +152,7 @@ func TestParseMentionsConfig_Object(t *testing.T) { AllowContext: boolPtr(false), Allowed: []string{"bot1"}, AllowedTeams: []string{"myorg/eng"}, - Max: new(10), + Max: strPtr("10"), }, }, { @@ -161,7 +161,16 @@ func TestParseMentionsConfig_Object(t *testing.T) { "max": 10.5, }, expected: &MentionsConfig{ - Max: new(10), // should be truncated + Max: strPtr("10"), // should be truncated + }, + }, + { + name: "max as expression", + input: map[string]any{ + "max": "${{ inputs.max-mentions }}", + }, + expected: &MentionsConfig{ + Max: strPtr("${{ inputs.max-mentions }}"), }, }, { @@ -267,7 +276,7 @@ func TestGenerateSafeOutputsConfig_WithMentions(t *testing.T) { AllowedCollaborators: boolPtr(false), AllowContext: boolPtr(true), Allowed: []string{"bot1", "bot2"}, - Max: new(20), + Max: strPtr("20"), }, expected: map[string]any{ "allowedCollaborators": false, @@ -291,7 +300,7 @@ func TestGenerateSafeOutputsConfig_WithMentions(t *testing.T) { AllowedCollaborators: boolPtr(false), AllowedTeams: []string{"myorg/eng"}, Allowed: []string{"bot1"}, - Max: new(30), + Max: strPtr("30"), }, expected: map[string]any{ "allowedCollaborators": false, @@ -406,7 +415,7 @@ func TestExtractSafeOutputsConfig_WithMentions(t *testing.T) { AllowedCollaborators: boolPtr(false), AllowContext: boolPtr(true), Allowed: []string{"bot1"}, - Max: new(15), + Max: strPtr("15"), }, }, { diff --git a/pkg/workflow/safe_outputs_messages_config.go b/pkg/workflow/safe_outputs_messages_config.go index d449ecb063b..e0c66c12ef8 100644 --- a/pkg/workflow/safe_outputs_messages_config.go +++ b/pkg/workflow/safe_outputs_messages_config.go @@ -3,6 +3,7 @@ package workflow import ( "encoding/json" "fmt" + "strings" "github.com/github/gh-aw/pkg/logger" ) @@ -95,69 +96,20 @@ func parseMentionsConfig(mentions any) *MentionsConfig { } } - // Parse allowed list - if allowed, exists := mentionsMap["allowed"]; exists { - if allowedArray, ok := allowed.([]any); ok { - var allowedStrings []string - for _, item := range allowedArray { - if str, ok := item.(string); ok { - // Normalize username by removing '@' prefix if present - normalized := str - if str != "" && str[0] == '@' { - normalized = str[1:] - safeOutputMessagesLog.Printf("Normalized mention '%s' to '%s'", str, normalized) - } - allowedStrings = append(allowedStrings, normalized) - } - } - config.Allowed = allowedStrings - } - } + config.Allowed = parseMentionNames(mentionsMap["allowed"]) // Parse allowed-teams list - if allowedTeams, exists := mentionsMap["allowed-teams"]; exists { - if allowedTeamsArray, ok := allowedTeams.([]any); ok { - var allowedTeamsStrings []string - for _, item := range allowedTeamsArray { - if str, ok := item.(string); ok { - // Normalize team slug by removing '@' prefix if present - normalized := str - if str != "" && str[0] == '@' { - normalized = str[1:] - safeOutputMessagesLog.Printf("Normalized team mention '%s' to '%s'", str, normalized) - } - allowedTeamsStrings = append(allowedTeamsStrings, normalized) - } - } - config.AllowedTeams = allowedTeamsStrings - } - } - - // Parse max - if maxVal, exists := mentionsMap["max"]; exists { - switch v := maxVal.(type) { - case int: - if v >= 1 { - config.Max = &v - } - case int64: - intVal := int(v) - if intVal >= 1 { - config.Max = &intVal - } - case uint64: - intVal := int(v) - if intVal >= 1 { - config.Max = &intVal - } - case float64: - intVal := int(v) - // Warn if truncation occurs - if v != float64(intVal) { - safeOutputMessagesLog.Printf("mentions.max: float value %.2f truncated to integer %d", v, intVal) - } - if intVal >= 1 { - config.Max = &intVal + config.AllowedTeams = parseMentionNames(mentionsMap["allowed-teams"]) + + // Parse max as a templatable integer. + if err := preprocessIntFieldAsString(mentionsMap, "max", safeOutputMessagesLog); err != nil { + safeOutputMessagesLog.Printf("mentions.max: %v", err) + } else if maxValue, exists := mentionsMap["max"]; exists { + if max, ok := maxValue.(string); ok { + if templatableIntValue(&max) >= 1 || isExpression(max) { + config.Max = &max + } else { + safeOutputMessagesLog.Printf("mentions.max must be at least 1, got %q", max) } } } @@ -166,6 +118,26 @@ func parseMentionsConfig(mentions any) *MentionsConfig { return config } +func parseMentionNames(value any) []string { + names, ok := value.([]any) + if !ok { + return nil + } + result := make([]string, 0, len(names)) + for _, item := range names { + name, ok := item.(string) + if !ok { + continue + } + normalized := strings.TrimPrefix(name, "@") + if normalized != name { + safeOutputMessagesLog.Printf("Normalized mention '%s' to '%s'", name, normalized) + } + result = append(result, normalized) + } + return result +} + // serializeMessagesConfig converts SafeOutputMessagesConfig to JSON for passing as environment variable func serializeMessagesConfig(messages *SafeOutputMessagesConfig) (string, error) { if messages == nil {