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
4 changes: 4 additions & 0 deletions .changeset/patch-enforce-safe-output-max-limits.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions actions/setup/js/add_labels.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ const HANDLER_TYPE = "add_labels";
const { validateLabels } = require("./safe_output_validator.cjs");
const { getErrorMessage } = require("./error_helpers.cjs");
const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs");
const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs");
Copy link
Contributor

Choose a reason for hiding this comment

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

Good practice: importing repo helpers for cross-repository validation.


/**
* Maximum limits for label parameters to prevent resource exhaustion.
* These limits align with GitHub's API constraints and security best practices.
*/
/** @type {number} Maximum number of labels allowed per operation */
const MAX_LABELS = 10;

/**
* Main handler factory for add_labels
Expand Down Expand Up @@ -89,6 +97,13 @@ async function main(config = {}) {
return { success: false, error };
}

// Enforce max limits on labels before validation
const limitResult = tryEnforceArrayLimit(requestedLabels, MAX_LABELS, "labels");
if (!limitResult.success) {
core.warning(`Label limit exceeded: ${limitResult.error}`);
return { success: false, error: limitResult.error };
}

// Use validation helper to sanitize and validate labels
const labelsResult = validateLabels(requestedLabels, allowedLabels, maxCount);
if (!labelsResult.valid) {
Expand Down
30 changes: 30 additions & 0 deletions actions/setup/js/add_labels.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -478,5 +478,35 @@ describe("add_labels", () => {
expect(addLabelsCalls[0].owner).toBe("github");
expect(addLabelsCalls[0].repo).toBe("gh-aw");
});

it("should enforce max limit on labels per operation", async () => {
const handler = await main({ max: 10 });

// Try to add more than MAX_LABELS (10)
const result = await handler(
{
item_number: 100,
labels: [
"label1",
"label2",
"label3",
"label4",
"label5",
"label6",
"label7",
"label8",
"label9",
"label10",
"label11", // 11th label exceeds limit
],
},
{}
);

expect(result.success).toBe(false);
expect(result.error).toContain("E003");
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

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

The test expects error code E003, but according to the Safe Outputs Specification (section 9.5 Error Code Catalog), E002 should be used for LIMIT_EXCEEDED errors. Please update the test expectations to check for E002 instead of E003.

Suggested change
expect(result.error).toContain("E003");
expect(result.error).toContain("E002");

Copilot uses AI. Check for mistakes.
expect(result.error).toContain("Cannot add more than 10 labels");
expect(result.error).toContain("received 11");
});
});
});
15 changes: 15 additions & 0 deletions actions/setup/js/create_discussion.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ const { getErrorMessage } = require("./error_helpers.cjs");
const { createExpirationLine, generateFooterWithExpiration } = require("./ephemerals.cjs");
const { generateWorkflowIdMarker } = require("./generate_footer.cjs");
const { sanitizeLabelContent } = require("./sanitize_label_content.cjs");
const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs");

/**
* Maximum limits for discussion parameters to prevent resource exhaustion.
* These limits align with GitHub's API constraints and security best practices.
*/
/** @type {number} Maximum number of labels allowed per discussion */
const MAX_LABELS = 10;

/**
* Fetch repository ID and discussion categories for a repository
Expand Down Expand Up @@ -483,6 +491,13 @@ async function main(config = {}) {
.map(label => (label.length > 64 ? label.substring(0, 64) : label))
.filter((label, index, arr) => arr.indexOf(label) === index);

// Enforce max limits on labels before API calls
const limitResult = tryEnforceArrayLimit(discussionLabels, MAX_LABELS, "labels");
if (!limitResult.success) {
core.warning(`Discussion limit exceeded: ${limitResult.error}`);
return { success: false, error: limitResult.error };
}

// Build title
let title = item.title ? item.title.trim() : "";
let processedBody = replaceTemporaryIdReferences(item.body || "", temporaryIdMap, qualifiedItemRepo);
Expand Down
36 changes: 36 additions & 0 deletions actions/setup/js/create_discussion_labels.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -392,4 +392,40 @@ describe("create_discussion with labels", () => {
expect(labelsApplyCall).toBeDefined();
expect(labelsApplyCall[1].labelIds).toEqual(["LA_label1", "LA_label2"]);
});

it("should enforce max labels limit (SEC-003)", async () => {
process.env.GH_AW_WORKFLOW_ID = "test-workflow";
process.env.GH_AW_WORKFLOW_NAME = "Test Workflow";
process.env.GH_AW_WORKFLOW_SOURCE = "test.md";

const handler = await createDiscussionMain({
category: "General",
});

// Try to create discussion with more than MAX_LABELS (10)
const message = {
title: "Test Discussion",
body: "This is a test discussion body",
labels: [
"label1",
"label2",
"label3",
"label4",
"label5",
"label6",
"label7",
"label8",
"label9",
"label10",
"label11", // 11th label exceeds limit
],
};

const result = await handler(message, {});

expect(result.success).toBe(false);
expect(result.error).toContain("E003");
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

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

The test expects error code E003, but according to the Safe Outputs Specification (section 9.5 Error Code Catalog), E002 should be used for LIMIT_EXCEEDED errors. Please update the test expectations to check for E002 instead of E003.

Suggested change
expect(result.error).toContain("E003");
expect(result.error).toContain("E002");

Copilot uses AI. Check for mistakes.
expect(result.error).toContain("Cannot add more than 10 labels");
expect(result.error).toContain("received 11");
});
});
24 changes: 24 additions & 0 deletions actions/setup/js/create_issue.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const { renderTemplate } = require("./messages_core.cjs");
const { createExpirationLine, addExpirationToFooter } = require("./ephemerals.cjs");
const { MAX_SUB_ISSUES, getSubIssueCount } = require("./sub_issue_helpers.cjs");
const { closeOlderIssues } = require("./close_older_issues.cjs");
const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs");
const fs = require("fs");

/**
Expand All @@ -53,6 +54,16 @@ const MAX_SUB_ISSUES_PER_PARENT = MAX_SUB_ISSUES;
/** @type {number} Maximum number of parent issues to check when searching */
const MAX_PARENT_ISSUES_TO_CHECK = 10;

/**
* Maximum limits for issue parameters to prevent resource exhaustion.
* These limits align with GitHub's API constraints and security best practices.
*/
/** @type {number} Maximum number of labels allowed per issue */
const MAX_LABELS = 10;

/** @type {number} Maximum number of assignees allowed per issue */
const MAX_ASSIGNEES = 5;

/**
* Searches for an existing parent issue that can accept more sub-issues
* @param {string} owner - Repository owner
Expand Down Expand Up @@ -385,6 +396,19 @@ async function main(config = {}) {
// Copilot is not a valid GitHub user and must be assigned via the agent assignment API
assignees = assignees.filter(assignee => assignee !== "copilot");

// Enforce max limits on labels and assignees before API calls
const labelsLimitResult = tryEnforceArrayLimit(labels, MAX_LABELS, "labels");
if (!labelsLimitResult.success) {
core.warning(`Issue limit exceeded: ${labelsLimitResult.error}`);
return { success: false, error: labelsLimitResult.error };
}

const assigneesLimitResult = tryEnforceArrayLimit(assignees, MAX_ASSIGNEES, "assignees");
if (!assigneesLimitResult.success) {
core.warning(`Issue limit exceeded: ${assigneesLimitResult.error}`);
return { success: false, error: assigneesLimitResult.error };
}

let title = message.title?.trim() ?? "";

// Replace temporary ID references in the body using already-created issues
Expand Down
44 changes: 44 additions & 0 deletions actions/setup/js/create_issue.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -390,4 +390,48 @@ describe("create_issue", () => {
expect(createCall.body).toContain("Related to #456");
});
});

describe("max limit enforcement", () => {
it("should enforce max limit on labels", async () => {
const handler = await main({});

const result = await handler({
title: "Test Issue",
body: "Test body",
labels: [
"label1",
"label2",
"label3",
"label4",
"label5",
"label6",
"label7",
"label8",
"label9",
"label10",
"label11", // 11th label exceeds limit
],
});

expect(result.success).toBe(false);
expect(result.error).toContain("E003");
expect(result.error).toContain("Cannot add more than 10 labels");
expect(result.error).toContain("received 11");
Comment on lines +417 to +419
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

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

The test expects error code E003, but according to the Safe Outputs Specification (section 9.5 Error Code Catalog), E002 should be used for LIMIT_EXCEEDED errors. Please update the test expectations to check for E002 instead of E003.

This issue also appears on line 432 of the same file.

Copilot uses AI. Check for mistakes.
});

it("should enforce max limit on assignees", async () => {
const handler = await main({});

const result = await handler({
title: "Test Issue",
body: "Test body",
assignees: ["user1", "user2", "user3", "user4", "user5", "user6"], // 6 assignees exceeds limit of 5
});

expect(result.success).toBe(false);
expect(result.error).toContain("E003");
expect(result.error).toContain("Cannot add more than 5 assignees");
expect(result.error).toContain("received 6");
});
});
});
39 changes: 38 additions & 1 deletion actions/setup/js/create_pull_request.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,34 @@ const { generateWorkflowIdMarker } = require("./generate_footer.cjs");
/** @type {string} Safe output type handled by this module */
const HANDLER_TYPE = "create_pull_request";

/**
* Maximum limits for pull request parameters to prevent resource exhaustion.
* These limits align with GitHub's API constraints and security best practices.
*/
/** @type {number} Maximum number of files allowed per pull request */
const MAX_FILES = 100;

/**
* Enforces maximum limits on pull request parameters to prevent resource exhaustion attacks.
* Per Safe Outputs specification requirement SEC-003, limits must be enforced before API calls.
*
* @param {string} patchContent - Patch content to validate
* @throws {Error} When any limit is exceeded, with error code E003 and details
Comment on lines +34 to +38
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

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

The JSDoc comment references "error code E003" but should reference "error code E002" to match the correct error code for LIMIT_EXCEEDED according to the Safe Outputs Specification.

This issue also appears on line 51 of the same file.

Copilot uses AI. Check for mistakes.
*/
function enforcePullRequestLimits(patchContent) {
if (!patchContent || !patchContent.trim()) {
return;
}

// Count files in patch by looking for "diff --git" lines
const fileMatches = patchContent.match(/^diff --git /gm);
const fileCount = fileMatches ? fileMatches.length : 0;

// Check file count - max limit exceeded check
if (fileCount > MAX_FILES) {
throw new Error(`E003: Cannot create pull request with more than ${MAX_FILES} files (received ${fileCount})`);
}
}
/**
* Generate a patch preview with max 500 lines and 2000 chars for issue body
* @param {string} patchContent - The full patch content
Expand Down Expand Up @@ -219,6 +247,15 @@ async function main(config = {}) {
isEmpty = !patchContent || !patchContent.trim();
}

// Enforce max limits on patch before processing
try {
enforcePullRequestLimits(patchContent);
} catch (error) {
const errorMessage = getErrorMessage(error);
core.warning(`Pull request limit exceeded: ${errorMessage}`);
return { success: false, error: errorMessage };
}

// Check for actual error conditions (but allow empty patches as valid noop)
if (patchContent.includes("Failed to generate patch")) {
// If allow-empty is enabled, ignore patch errors and proceed
Expand Down Expand Up @@ -859,4 +896,4 @@ You can manually create a pull request from the branch if needed.${patchPreview}
}; // End of handleCreatePullRequest
} // End of main

module.exports = { main };
module.exports = { main, enforcePullRequestLimits };
60 changes: 59 additions & 1 deletion actions/setup/js/create_pull_request.test.cjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// @ts-check
import { describe, it, expect } from "vitest";
import { describe, it, expect, beforeEach, vi } from "vitest";
import { createRequire } from "module";

const require = createRequire(import.meta.url);

describe("create_pull_request - fallback-as-issue configuration", () => {
describe("configuration parsing", () => {
Expand Down Expand Up @@ -46,3 +49,58 @@ describe("create_pull_request - fallback-as-issue configuration", () => {
});
});
});

describe("create_pull_request - max limit enforcement", () => {
let mockFs;

beforeEach(() => {
// Mock fs module for patch reading
mockFs = {
existsSync: vi.fn().mockReturnValue(true),
readFileSync: vi.fn(),
};
});

it("should enforce max file limit on patch content", () => {
// Create a patch with more than MAX_FILES (100) files
const patchLines = [];
for (let i = 0; i < 101; i++) {
patchLines.push(`diff --git a/file${i}.txt b/file${i}.txt`);
patchLines.push("index 1234567..abcdefg 100644");
patchLines.push("--- a/file${i}.txt");
patchLines.push("+++ b/file${i}.txt");
Comment on lines +68 to +71
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

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

The strings in lines 68-71 use double quotes instead of backticks, which prevents variable interpolation. All instances of "...\${i}..." should be changed to template literals using backticks: `...${i}...`. Without this, the test generates 101 identical "diff --git a/file${i}.txt b/file${i}.txt" lines instead of 101 unique files, which means the file counting logic may not be properly tested. The regex pattern ^diff --git /gm would still match 101 times, so the test might pass, but it's testing the wrong thing.

This issue also appears on line 93 of the same file.

Copilot uses AI. Check for mistakes.
patchLines.push("@@ -1,1 +1,1 @@");
patchLines.push("-old content");
patchLines.push("+new content");
}
const patchContent = patchLines.join("\n");

// Import the enforcement function
const { enforcePullRequestLimits } = require("./create_pull_request.cjs");

// Should throw E003 error
expect(() => enforcePullRequestLimits(patchContent)).toThrow("E003");
Comment on lines +81 to +82
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

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

The test expects error code E003, but according to the Safe Outputs Specification (section 9.5 Error Code Catalog), E002 should be used for LIMIT_EXCEEDED errors. Please update the test expectations to check for E002 instead of E003.

Suggested change
// Should throw E003 error
expect(() => enforcePullRequestLimits(patchContent)).toThrow("E003");
// Should throw E002 error
expect(() => enforcePullRequestLimits(patchContent)).toThrow("E002");

Copilot uses AI. Check for mistakes.
expect(() => enforcePullRequestLimits(patchContent)).toThrow("Cannot create pull request with more than 100 files");
expect(() => enforcePullRequestLimits(patchContent)).toThrow("received 101");
});

it("should allow patches under the file limit", () => {
// Create a patch with exactly MAX_FILES (100) files
const patchLines = [];
for (let i = 0; i < 100; i++) {
patchLines.push(`diff --git a/file${i}.txt b/file${i}.txt`);
patchLines.push("index 1234567..abcdefg 100644");
patchLines.push("--- a/file${i}.txt");
patchLines.push("+++ b/file${i}.txt");
patchLines.push("@@ -1,1 +1,1 @@");
patchLines.push("-old content");
patchLines.push("+new content");
}
const patchContent = patchLines.join("\n");

const { enforcePullRequestLimits } = require("./create_pull_request.cjs");

// Should not throw
expect(() => enforcePullRequestLimits(patchContent)).not.toThrow();
});
});
Loading