diff --git a/actions/setup/js/add_comment.cjs b/actions/setup/js/add_comment.cjs index db73e96e62..a789bed2a7 100644 --- a/actions/setup/js/add_comment.cjs +++ b/actions/setup/js/add_comment.cjs @@ -18,6 +18,45 @@ const { getMessages } = require("./messages_core.cjs"); /** @type {string} Safe output type handled by this module */ const HANDLER_TYPE = "add_comment"; +/** + * Maximum limits for comment parameters to prevent resource exhaustion. + * These limits align with GitHub's API constraints and security best practices. + */ +/** @type {number} Maximum comment body length (GitHub's limit) */ +const MAX_COMMENT_LENGTH = 65536; + +/** @type {number} Maximum number of mentions allowed per comment */ +const MAX_MENTIONS = 10; + +/** @type {number} Maximum number of links allowed per comment */ +const MAX_LINKS = 50; + +/** + * Enforces maximum limits on comment parameters to prevent resource exhaustion attacks. + * Per Safe Outputs specification requirement MR3, limits must be enforced before API calls. + * + * @param {string} body - Comment body to validate + * @throws {Error} When any limit is exceeded, with error code and details + */ +function enforceCommentLimits(body) { + // Check body length - max limit exceeded check + if (body.length > MAX_COMMENT_LENGTH) { + throw new Error(`E006: Comment body exceeds maximum length of ${MAX_COMMENT_LENGTH} characters (got ${body.length})`); + } + + // Count mentions (@username pattern) - max limit exceeded check + const mentions = (body.match(/@\w+/g) || []).length; + if (mentions > MAX_MENTIONS) { + throw new Error(`E007: Comment contains ${mentions} mentions, maximum is ${MAX_MENTIONS}`); + } + + // Count links (http:// and https:// URLs) - max limit exceeded check + const links = (body.match(/https?:\/\/[^\s]+/g) || []).length; + if (links > MAX_LINKS) { + throw new Error(`E008: Comment contains ${links} links, maximum is ${MAX_LINKS}`); + } +} + // Copy helper functions from original file async function minimizeComment(github, nodeId, reason = "outdated") { const query = /* GraphQL */ ` @@ -424,6 +463,18 @@ async function main(config = {}) { // Replace temporary ID references in body let processedBody = replaceTemporaryIdReferences(item.body || "", temporaryIdMap, itemRepo); + // Enforce max limits before processing (validates user-provided content) + try { + enforceCommentLimits(processedBody); + } catch (error) { + const errorMessage = getErrorMessage(error); + core.warning(`Comment validation failed: ${errorMessage}`); + return { + success: false, + error: errorMessage, + }; + } + // Add tracker ID and footer const trackerIDComment = getTrackerID("markdown"); if (trackerIDComment) { @@ -441,6 +492,19 @@ async function main(config = {}) { processedBody += missingInfoSections; } + // Enforce max limits again after adding footer and metadata + // This ensures the final body (including generated content) doesn't exceed limits + try { + enforceCommentLimits(processedBody); + } catch (error) { + const errorMessage = getErrorMessage(error); + core.warning(`Final comment body validation failed: ${errorMessage}`); + return { + success: false, + error: errorMessage, + }; + } + core.info(`Adding comment to ${isDiscussion ? "discussion" : "issue/PR"} #${itemNumber} in ${itemRepo}`); // If in staged mode, preview the comment without creating it @@ -632,4 +696,11 @@ async function main(config = {}) { }; } -module.exports = { main }; +module.exports = { + main, + // Export constants and functions for testing + MAX_COMMENT_LENGTH, + MAX_MENTIONS, + MAX_LINKS, + enforceCommentLimits, +}; diff --git a/actions/setup/js/add_comment.test.cjs b/actions/setup/js/add_comment.test.cjs index e13e57dcc2..7e9e84cc78 100644 --- a/actions/setup/js/add_comment.test.cjs +++ b/actions/setup/js/add_comment.test.cjs @@ -1099,3 +1099,127 @@ describe("add_comment", () => { }); }); }); + +describe("enforceCommentLimits", () => { + let enforceCommentLimits; + let MAX_COMMENT_LENGTH; + let MAX_MENTIONS; + let MAX_LINKS; + + beforeEach(async () => { + const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8"); + const exports = await eval(`(async () => { ${addCommentScript}; return { enforceCommentLimits, MAX_COMMENT_LENGTH, MAX_MENTIONS, MAX_LINKS }; })()`); + enforceCommentLimits = exports.enforceCommentLimits; + MAX_COMMENT_LENGTH = exports.MAX_COMMENT_LENGTH; + MAX_MENTIONS = exports.MAX_MENTIONS; + MAX_LINKS = exports.MAX_LINKS; + }); + + it("should accept comment within all limits", () => { + const validBody = "This is a valid comment with @user1 and https://github.com"; + expect(() => enforceCommentLimits(validBody)).not.toThrow(); + }); + + it("should reject comment exceeding MAX_COMMENT_LENGTH", () => { + const longBody = "a".repeat(MAX_COMMENT_LENGTH + 1); + expect(() => enforceCommentLimits(longBody)).toThrow(/E006.*maximum length/i); + }); + + it("should accept comment at exactly MAX_COMMENT_LENGTH", () => { + const exactBody = "a".repeat(MAX_COMMENT_LENGTH); + expect(() => enforceCommentLimits(exactBody)).not.toThrow(); + }); + + it("should reject comment with too many mentions", () => { + const mentions = Array.from({ length: MAX_MENTIONS + 1 }, (_, i) => `@user${i}`).join(" "); + const bodyWithMentions = `Comment with mentions: ${mentions}`; + expect(() => enforceCommentLimits(bodyWithMentions)).toThrow(/E007.*mentions/i); + }); + + it("should accept comment at exactly MAX_MENTIONS", () => { + const mentions = Array.from({ length: MAX_MENTIONS }, (_, i) => `@user${i}`).join(" "); + const bodyWithMentions = `Comment with mentions: ${mentions}`; + expect(() => enforceCommentLimits(bodyWithMentions)).not.toThrow(); + }); + + it("should reject comment with too many links", () => { + const links = Array.from({ length: MAX_LINKS + 1 }, (_, i) => `https://example.com/${i}`).join(" "); + const bodyWithLinks = `Comment with links: ${links}`; + expect(() => enforceCommentLimits(bodyWithLinks)).toThrow(/E008.*links/i); + }); + + it("should accept comment at exactly MAX_LINKS", () => { + const links = Array.from({ length: MAX_LINKS }, (_, i) => `https://example.com/${i}`).join(" "); + const bodyWithLinks = `Comment with links: ${links}`; + expect(() => enforceCommentLimits(bodyWithLinks)).not.toThrow(); + }); + + it("should count both http and https links", () => { + const httpLinks = Array.from({ length: 26 }, (_, i) => `http://example.com/${i}`).join(" "); + const httpsLinks = Array.from({ length: 25 }, (_, i) => `https://example.com/${i}`).join(" "); + const bodyWithMixedLinks = `Comment with mixed: ${httpLinks} ${httpsLinks}`; + expect(() => enforceCommentLimits(bodyWithMixedLinks)).toThrow(/E008.*links/i); + }); + + it("should provide detailed error message for length violation", () => { + const longBody = "a".repeat(MAX_COMMENT_LENGTH + 100); + try { + enforceCommentLimits(longBody); + throw new Error("Should have thrown"); + } catch (error) { + expect(error.message).toMatch(/E006/); + expect(error.message).toMatch(/65536/); + expect(error.message).toMatch(/65636/); + } + }); + + it("should provide detailed error message for mentions violation", () => { + const mentions = Array.from({ length: 15 }, (_, i) => `@user${i}`).join(" "); + const bodyWithMentions = `Comment: ${mentions}`; + try { + enforceCommentLimits(bodyWithMentions); + throw new Error("Should have thrown"); + } catch (error) { + expect(error.message).toMatch(/E007/); + expect(error.message).toMatch(/15 mentions/); + expect(error.message).toMatch(/maximum is 10/); + } + }); + + it("should provide detailed error message for links violation", () => { + const links = Array.from({ length: 60 }, (_, i) => `https://example.com/${i}`).join(" "); + const bodyWithLinks = `Comment: ${links}`; + try { + enforceCommentLimits(bodyWithLinks); + throw new Error("Should have thrown"); + } catch (error) { + expect(error.message).toMatch(/E008/); + expect(error.message).toMatch(/60 links/); + expect(error.message).toMatch(/maximum is 50/); + } + }); + + it("should handle empty comment body", () => { + expect(() => enforceCommentLimits("")).not.toThrow(); + }); + + it("should handle comment with no mentions", () => { + const body = "This is a comment without any mentions at all"; + expect(() => enforceCommentLimits(body)).not.toThrow(); + }); + + it("should handle comment with no links", () => { + const body = "This is a comment without any links at all"; + expect(() => enforceCommentLimits(body)).not.toThrow(); + }); + + it("should not count incomplete mention patterns", () => { + const body = "@ not a mention, @ also not, @123 is not a mention"; + expect(() => enforceCommentLimits(body)).not.toThrow(); + }); + + it("should count valid mention patterns only", () => { + const body = "Valid: @user1 @user2. Invalid: @ @123 email@example.com"; + expect(() => enforceCommentLimits(body)).not.toThrow(); + }); +});