Skip to content

Commit adfc80f

Browse files
authored
Apply mention limits to output content instead of allowlists (#57747)
1 parent e8150b9 commit adfc80f

24 files changed

Lines changed: 263 additions & 147 deletions

actions/setup/js/add_comment.cjs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const { getRepositoryUrl } = require("./get_repository_url.cjs");
1111
const { replaceTemporaryIdReferences, resolveSafeOutputIssueTarget } = require("./temporary_id.cjs");
1212
const { getTrackerID } = require("./get_tracker_id.cjs");
1313
const { getErrorMessage } = require("./error_helpers.cjs");
14-
const { parseBoolTemplatable } = require("./templatable.cjs");
14+
const { parseBoolTemplatable, parseIntTemplatable } = require("./templatable.cjs");
1515
const { resolveTarget, isStagedMode } = require("./safe_output_helpers.cjs");
1616
const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs");
1717
const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs");
@@ -507,6 +507,7 @@ async function main(config = {}) {
507507
const mentionsDisabled = config.mentions === false || config.mentions?.enabled === false;
508508
const preResolvedMentionAliases = !mentionsDisabled ? normalizeMentionAliases(config.allowedMentionAliases) : [];
509509
const configuredMentionAliases = !mentionsDisabled ? normalizeMentionAliases(config.mentions?.allowed) : [];
510+
const maxMentions = mentionsDisabled ? undefined : parseIntTemplatable(config.mentions?.max, 50);
510511

511512
// Create an authenticated GitHub client. Uses config["github-token"] when set
512513
// (for cross-repository operations), otherwise falls back to the step-level github.
@@ -803,7 +804,7 @@ async function main(config = {}) {
803804

804805
// Sanitize content to prevent injection attacks, allowing parent issue/PR/discussion authors
805806
// so they can be @mentioned in the generated comment.
806-
processedBody = sanitizeContent(processedBody, { allowedAliases: allowedMentionAliases });
807+
processedBody = sanitizeContent(processedBody, { allowedAliases: allowedMentionAliases, maxMentions });
807808

808809
// Enforce max limits before processing (validates user-provided content)
809810
try {

actions/setup/js/close_discussion.cjs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs");
1313
const { ERR_NOT_FOUND } = require("./error_codes.cjs");
1414
const { resolveNumberFromTemporaryId } = require("./temporary_id.cjs");
1515
const { resolveAllowedMentionsFromPayload } = require("./resolve_mentions_from_payload.cjs");
16+
const { parseIntTemplatable } = require("./templatable.cjs");
1617
const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs");
1718

1819
/**
@@ -168,6 +169,7 @@ async function main(config = {}) {
168169
const maxCount = config.max || 10;
169170
const githubClient = await createAuthenticatedGitHubClient(config);
170171
const allowBody = config.allow_body !== false; // default true; false only when explicitly set to false
172+
const maxMentions = parseIntTemplatable(config.mentions?.max, 50);
171173
let allowedMentionAliases = [];
172174
if (Array.isArray(config.allowedMentionAliases)) {
173175
allowedMentionAliases = config.allowedMentionAliases;
@@ -307,7 +309,7 @@ async function main(config = {}) {
307309
core.info("close_discussion: allow-body is false — closing without a comment");
308310
}
309311
} else if (item.body) {
310-
const sanitizedBody = sanitizeContent(item.body, { allowedAliases: allowedMentionAliases });
312+
const sanitizedBody = sanitizeContent(item.body, { allowedAliases: allowedMentionAliases, maxMentions });
311313
const comment = await addDiscussionComment(githubClient, discussion.id, sanitizedBody);
312314
core.info(`Added comment to discussion #${discussionNumber}: ${comment.url}`);
313315
commentUrl = comment.url;

actions/setup/js/collect_ndjson_output.cjs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ async function main() {
3434

3535
// Extract mentions configuration from validation config
3636
const mentionsConfig = validationConfig?.mentions || null;
37+
const maxMentions = parseIntTemplatable(mentionsConfig?.max, 50);
3738

3839
// Resolve allowed mentions for the output collector
3940
// This determines which @mentions are allowed in the agent output
@@ -43,7 +44,7 @@ async function main() {
4344
/** @type {number | undefined} */
4445
let maxBotMentions;
4546

46-
function validateFieldWithInputSchema(value, fieldName, inputSchema, lineNum) {
47+
function validateFieldWithInputSchema(value, fieldName, inputSchema, lineNum, allowedAliasesSeen) {
4748
if (inputSchema.required && (value === undefined || value === null)) {
4849
return {
4950
isValid: false,
@@ -66,7 +67,7 @@ async function main() {
6667
error: `Line ${lineNum}: ${fieldName} must be a string`,
6768
};
6869
}
69-
normalizedValue = sanitizeContent(value, { allowedAliases: allowedMentions, maxBotMentions });
70+
normalizedValue = sanitizeContent(value, { allowedAliases: allowedMentions, maxMentions, maxBotMentions, allowedAliasesSeen });
7071
break;
7172
case "boolean":
7273
if (typeof value !== "boolean") {
@@ -97,11 +98,11 @@ async function main() {
9798
error: `Line ${lineNum}: ${fieldName} must be one of: ${inputSchema.options.join(", ")}`,
9899
};
99100
}
100-
normalizedValue = sanitizeContent(value, { allowedAliases: allowedMentions, maxBotMentions });
101+
normalizedValue = sanitizeContent(value, { allowedAliases: allowedMentions, maxMentions, maxBotMentions, allowedAliasesSeen });
101102
break;
102103
default:
103104
if (typeof value === "string") {
104-
normalizedValue = sanitizeContent(value, { allowedAliases: allowedMentions, maxBotMentions });
105+
normalizedValue = sanitizeContent(value, { allowedAliases: allowedMentions, maxMentions, maxBotMentions, allowedAliasesSeen });
105106
}
106107
break;
107108
}
@@ -120,9 +121,10 @@ async function main() {
120121
normalizedItem,
121122
};
122123
}
124+
const allowedAliasesSeen = new Set();
123125
for (const [fieldName, inputSchema] of Object.entries(jobConfig.inputs)) {
124126
const fieldValue = item[fieldName];
125-
const validation = validateFieldWithInputSchema(fieldValue, fieldName, inputSchema, lineNum);
127+
const validation = validateFieldWithInputSchema(fieldValue, fieldName, inputSchema, lineNum, allowedAliasesSeen);
126128
if (!validation.isValid && validation.error) {
127129
errors.push(validation.error);
128130
} else if (validation.normalizedValue !== undefined) {
@@ -357,6 +359,7 @@ async function main() {
357359
if (hasValidationConfig(itemType)) {
358360
const validationResult = validateItem(item, itemType, i + 1, {
359361
allowedAliases: allowedMentions,
362+
maxMentions,
360363
maxBotMentions,
361364
normalizeIssueClosingKeywords,
362365
dataEnabled: typeConfig !== null && typeof typeConfig === "object" && typeConfig.data_enabled === true,

actions/setup/js/collect_ndjson_output.test.cjs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1320,6 +1320,43 @@ describe("collect_ndjson_output.cjs", () => {
13201320
parsedOutput = JSON.parse(outputCall[1]);
13211321
expect(parsedOutput.items[0].body).toBe("Hey `@username` and `@org/team`, check this out! But preserve email@domain.com");
13221322
}),
1323+
it("should preserve allowed aliases after max when no more than max occur", async () => {
1324+
const allowed = Array.from({ length: 60 }, (_, i) => `user${i}`);
1325+
const validationPath = "/tmp/gh-aw/safeoutputs/validation.json";
1326+
const validationConfig = JSON.parse(fs.readFileSync(validationPath, "utf8"));
1327+
validationConfig.mentions = { allowContext: false, allowed, max: 3 };
1328+
fs.writeFileSync(validationPath, JSON.stringify(validationConfig));
1329+
1330+
const testFile = "/tmp/gh-aw/test-ndjson-output.txt";
1331+
const ndjsonContent = '{"type":"create_issue","title":"Late allowlist entries","body":"Thanks @user57, @user58, and @user59"}';
1332+
fs.writeFileSync(testFile, ndjsonContent);
1333+
process.env.GH_AW_SAFE_OUTPUTS = testFile;
1334+
fs.writeFileSync("/tmp/gh-aw/safeoutputs/config.json", '{"create_issue":true}');
1335+
1336+
await eval(`(async () => { ${collectScript}; await main(); })()`);
1337+
1338+
const outputCall = mockCore.setOutput.mock.calls.find(call => call[0] === "output");
1339+
const parsedOutput = JSON.parse(outputCall[1]);
1340+
expect(parsedOutput.items[0].body).toBe("Thanks @user57, @user58, and @user59");
1341+
}),
1342+
it("should apply the mention limit across all fields in one item", async () => {
1343+
const validationPath = "/tmp/gh-aw/safeoutputs/validation.json";
1344+
const validationConfig = JSON.parse(fs.readFileSync(validationPath, "utf8"));
1345+
validationConfig.mentions = { allowContext: false, allowed: ["user1", "user2", "user3", "user4"], max: 3 };
1346+
fs.writeFileSync(validationPath, JSON.stringify(validationConfig));
1347+
1348+
const testFile = "/tmp/gh-aw/test-ndjson-output.txt";
1349+
fs.writeFileSync(testFile, '{"type":"create_issue","title":"@user1 @user2","body":"@user3 @user4 @user1"}');
1350+
process.env.GH_AW_SAFE_OUTPUTS = testFile;
1351+
fs.writeFileSync("/tmp/gh-aw/safeoutputs/config.json", '{"create_issue":true}');
1352+
1353+
await eval(`(async () => { ${collectScript}; await main(); })()`);
1354+
1355+
const outputCall = mockCore.setOutput.mock.calls.find(call => call[0] === "output");
1356+
const parsedOutput = JSON.parse(outputCall[1]);
1357+
expect(parsedOutput.items[0].title).toBe("@user1 @user2");
1358+
expect(parsedOutput.items[0].body).toBe("@user3 `@user4` @user1");
1359+
}),
13231360
it("should neutralize bot trigger phrases", async () => {
13241361
const testFile = "/tmp/gh-aw/test-ndjson-output.txt",
13251362
ndjsonContent = '{"type": "create_issue", "title": "Bot Trigger Test", "body": "This fixes #123 and closes #456, also resolves #789"}';

actions/setup/js/create_discussion.cjs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs");
2626
const { logStagedPreviewInfo } = require("./staged_preview.cjs");
2727
const { isStagedMode } = require("./safe_output_helpers.cjs");
2828
const { closeOlderDiscussions: closeOlderDiscussionsFunc } = require("./close_older_discussions.cjs");
29-
const { parseBoolTemplatable } = require("./templatable.cjs");
29+
const { parseBoolTemplatable, parseIntTemplatable } = require("./templatable.cjs");
3030
const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs");
3131
const { generateHistoryLink, generateHistoryUrl } = require("./generate_history_link.cjs");
3232
const { MAX_LABELS } = require("./constants.cjs");
@@ -312,6 +312,7 @@ async function main(config = {}) {
312312
// Create an authenticated GitHub client. Uses config["github-token"] when set
313313
// (for cross-repository operations), otherwise falls back to the step-level github.
314314
const githubClient = await createAuthenticatedGitHubClient(config);
315+
const maxMentions = parseIntTemplatable(config.mentions?.max, 50);
315316
let allowedMentionAliases = [];
316317
if (Array.isArray(config.allowedMentionAliases)) {
317318
allowedMentionAliases = config.allowedMentionAliases;
@@ -502,7 +503,7 @@ async function main(config = {}) {
502503
const preSanitizeBodyLength = processedBody.trim().length;
503504

504505
// Sanitize body content to neutralize @mentions, URLs, and other security risks
505-
processedBody = sanitizeContent(processedBody, { allowedAliases: allowedMentionAliases });
506+
processedBody = sanitizeContent(processedBody, { allowedAliases: allowedMentionAliases, maxMentions });
506507
if (minBodyLength > 0 && preSanitizeBodyLength < minBodyLength) {
507508
const error = `Discussion body length ${preSanitizeBodyLength} is below configured minimum ${minBodyLength}`;
508509
core.error(error);

actions/setup/js/create_issue.cjs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ const { renderTemplateFromFile } = require("./messages_core.cjs");
2020
const { createExpirationLine, addExpirationToFooter } = require("./ephemerals.cjs");
2121
const { MAX_SUB_ISSUES, getSubIssueCount, linkSubIssue } = require("./sub_issue_helpers.cjs");
2222
const { closeOlderIssues, searchOlderIssues, addIssueComment } = require("./close_older_issues.cjs");
23-
const { parseBoolTemplatable } = require("./templatable.cjs");
23+
const { parseBoolTemplatable, parseIntTemplatable } = require("./templatable.cjs");
2424
const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs");
2525
const { logStagedPreviewInfo } = require("./staged_preview.cjs");
2626
const { isStagedMode } = require("./safe_output_helpers.cjs");
@@ -669,6 +669,7 @@ async function main(config = {}) {
669669
// Create an authenticated GitHub client. Uses config["github-token"] when set
670670
// (for cross-repository operations), otherwise falls back to the step-level github.
671671
const githubClient = await createAuthenticatedGitHubClient(config);
672+
const maxMentions = parseIntTemplatable(config.mentions?.max, 50);
672673
let allowedMentionAliases = [];
673674
if (Array.isArray(config.allowedMentionAliases)) {
674675
allowedMentionAliases = config.allowedMentionAliases;
@@ -915,7 +916,7 @@ async function main(config = {}) {
915916
processedBody = removeDuplicateTitleFromDescription(title, processedBody);
916917

917918
// Sanitize body content to neutralize @mentions, URLs, and other security risks
918-
processedBody = sanitizeContent(processedBody, { allowedAliases: allowedMentionAliases });
919+
processedBody = sanitizeContent(processedBody, { allowedAliases: allowedMentionAliases, maxMentions });
919920

920921
const bodyLines = processedBody.split("\n");
921922

actions/setup/js/create_pr_review_comment.cjs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs");
1212
const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs");
1313
const { isTemplatableTrue, isStagedMode, logStagedPreviewInfo, checkRequiredFilter } = require("./safe_output_helpers.cjs");
1414
const { resolveAllowedMentionsFromPayload } = require("./resolve_mentions_from_payload.cjs");
15+
const { parseIntTemplatable } = require("./templatable.cjs");
1516
const { resolveInvocationContext } = require("./invocation_context_helpers.cjs");
1617
const { ERR_VALIDATION } = require("./error_codes.cjs");
1718

@@ -58,6 +59,7 @@ async function main(config = {}) {
5859
if (requiredLabels.length > 0) core.info(`Required labels (all): ${requiredLabels.join(", ")}`);
5960
if (requiredTitlePrefix) core.info(`Required title prefix: ${requiredTitlePrefix}`);
6061

62+
const maxMentions = parseIntTemplatable(config.mentions?.max, 50);
6163
let allowedMentionAliases = [];
6264
if (Array.isArray(config.allowedMentionAliases)) {
6365
allowedMentionAliases = config.allowedMentionAliases;
@@ -376,7 +378,7 @@ async function main(config = {}) {
376378
const bufferedComment = {
377379
path: commentItem.path,
378380
line: line,
379-
body: sanitizeContent(commentItem.body.trim(), { allowedAliases: allowedMentionAliases }),
381+
body: sanitizeContent(commentItem.body.trim(), { allowedAliases: allowedMentionAliases, maxMentions }),
380382
side: side,
381383
};
382384

actions/setup/js/create_project_status_update.cjs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const { isTemporaryId, normalizeTemporaryId } = require("./temporary_id.cjs");
1010
const { ERR_CONFIG, ERR_NOT_FOUND, ERR_PARSE, ERR_VALIDATION } = require("./error_codes.cjs");
1111
const { logGraphQLError } = require("./github_api_helpers.cjs");
1212
const { resolveAllowedMentionsFromPayload } = require("./resolve_mentions_from_payload.cjs");
13+
const { parseIntTemplatable } = require("./templatable.cjs");
1314

1415
/**
1516
* @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction
@@ -273,6 +274,7 @@ async function main(config = {}, githubClient = null) {
273274
if (!github) {
274275
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.`);
275276
}
277+
const maxMentions = parseIntTemplatable(config.mentions?.max, 50);
276278
let allowedMentionAliases = [];
277279
if (Array.isArray(config.allowedMentionAliases)) {
278280
allowedMentionAliases = config.allowedMentionAliases;
@@ -367,7 +369,7 @@ async function main(config = {}, githubClient = null) {
367369
const status = validateStatus(output.status);
368370
const startDate = formatDate(output.start_date);
369371
const targetDate = formatDate(output.target_date);
370-
const body = sanitizeContent(String(output.body), { allowedAliases: allowedMentionAliases });
372+
const body = sanitizeContent(String(output.body), { allowedAliases: allowedMentionAliases, maxMentions });
371373

372374
core.info(`Creating status update: ${status} (${startDate}${targetDate})`);
373375
core.info(`Body preview: ${body.substring(0, 100)}${body.length > 100 ? "..." : ""}`);

actions/setup/js/create_pull_request.cjs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ const { replaceTemporaryIdReferences, replaceTemporaryIdReferencesInPatch, getOr
1616
const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs");
1717
const { addExpirationToFooter } = require("./ephemerals.cjs");
1818
const { generateWorkflowIdMarker, generateWorkflowCallIdMarker, generateCloseKeyMarker, normalizeCloseOlderKey } = require("./generate_footer.cjs");
19-
const { parseBoolTemplatable } = require("./templatable.cjs");
19+
const { parseBoolTemplatable, parseIntTemplatable } = require("./templatable.cjs");
2020
const { assembleMarkdownBodyParts } = require("./markdown_body_helpers.cjs");
2121
const { getBodyHeader, getDisclosureHeader } = require("./messages_header.cjs");
2222
const { generateHistoryUrl } = require("./generate_history_link.cjs");
@@ -782,6 +782,7 @@ async function main(config = {}) {
782782
// Tracks the pull requests created so far in this run so later messages can stack on top of them.
783783
const stackTracker = createStackTracker();
784784
const githubClient = await createAuthenticatedGitHubClient(config);
785+
const maxMentions = parseIntTemplatable(config.mentions?.max, 50);
785786
let allowedMentionAliases = [];
786787
if (Array.isArray(config.allowedMentionAliases)) {
787788
allowedMentionAliases = config.allowedMentionAliases;
@@ -1504,7 +1505,7 @@ async function main(config = {}) {
15041505
processedBody = removeDuplicateTitleFromDescription(title, processedBody);
15051506

15061507
// Sanitize body content to neutralize @mentions, URLs, and other security risks
1507-
processedBody = sanitizeContent(processedBody, { allowedAliases: allowedMentionAliases });
1508+
processedBody = sanitizeContent(processedBody, { allowedAliases: allowedMentionAliases, maxMentions });
15081509

15091510
// Auto-add "Fixes #N" closing keyword if triggered from an issue and not already present.
15101511
// This ensures the triggering issue is auto-closed when the PR is merged.
@@ -1898,7 +1899,7 @@ async function main(config = {}) {
18981899
baseBranch,
18991900
tempRef: createBundleTempRef(branchName),
19001901
});
1901-
const pushFailureMessage = sanitizeContent(neutralizeClosingKeywordsForIssueBody(getErrorMessage(pushError)), { allowedAliases: allowedMentionAliases })
1902+
const pushFailureMessage = sanitizeContent(neutralizeClosingKeywordsForIssueBody(getErrorMessage(pushError)), { allowedAliases: allowedMentionAliases, maxMentions })
19021903
.replace(/\s+/g, " ")
19031904
.trim();
19041905
const pushErrorSection = buildPushErrorSection(getErrorMessage(pushError), pushFailureMessage);
@@ -2267,7 +2268,7 @@ gh pr create --title ${shellQuote(title)} --base ${shellQuote(baseBranch)} --hea
22672268
branchName,
22682269
baseBranch,
22692270
});
2270-
const pushFailureMessage = sanitizeContent(neutralizeClosingKeywordsForIssueBody(getErrorMessage(pushError)), { allowedAliases: allowedMentionAliases })
2271+
const pushFailureMessage = sanitizeContent(neutralizeClosingKeywordsForIssueBody(getErrorMessage(pushError)), { allowedAliases: allowedMentionAliases, maxMentions })
22712272
.replace(/\s+/g, " ")
22722273
.trim();
22732274
const pushErrorSection = buildPushErrorSection(getErrorMessage(pushError), pushFailureMessage);

0 commit comments

Comments
 (0)