Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions actions/setup/js/add_comment.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 3 additions & 1 deletion actions/setup/js/close_discussion.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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");

/**
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
13 changes: 8 additions & 5 deletions actions/setup/js/collect_ndjson_output.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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") {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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) {
Expand Down Expand Up @@ -357,6 +359,7 @@ async function main() {
if (hasValidationConfig(itemType)) {
const validationResult = validateItem(item, itemType, i + 1, {
allowedAliases: allowedMentions,
maxMentions,
maxBotMentions,
Comment on lines 360 to 363

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed: this is already addressed by 92deabd. validateItem now creates one allowedAliasesSeen set for the output item and supplies it to each field sanitizer; the collector’s safe-job path does the same. The regression test verifies the cap spans title and body while allowing a repeated approved identity.

normalizeIssueClosingKeywords,
dataEnabled: typeConfig !== null && typeof typeConfig === "object" && typeConfig.data_enabled === true,
Expand Down
37 changes: 37 additions & 0 deletions actions/setup/js/collect_ndjson_output.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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"}';
Expand Down
5 changes: 3 additions & 2 deletions actions/setup/js/create_discussion.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions actions/setup/js/create_issue.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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");

Expand Down
4 changes: 3 additions & 1 deletion actions/setup/js/create_pr_review_comment.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
};

Expand Down
4 changes: 3 additions & 1 deletion actions/setup/js/create_project_status_update.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 ? "..." : ""}`);
Expand Down
9 changes: 5 additions & 4 deletions actions/setup/js/create_pull_request.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
Loading
Loading