Add max limit enforcement to add_comment handler (SEC-003)

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,
+};

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();
+  });
+});