Skip to content

Commit af5df47

Browse files
authored
SEC-004 conformance: sanitize close-issue comment body and add explicit handler exemptions (#27448)
1 parent 033b3a0 commit af5df47

File tree

6 files changed

+48
-3
lines changed

6 files changed

+48
-3
lines changed

actions/setup/js/close_agentic_workflows_issues.cjs

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22
/// <reference types="@actions/github-script" />
33

44
const { resolveExecutionOwnerRepo } = require("./repo_helpers.cjs");
5+
const { sanitizeContent } = require("./sanitize_content.cjs");
56

67
const TARGET_LABEL = "agentic-workflows";
8+
// Keep a conservative, explicit upper bound for comment sanitization.
9+
const MAX_COMMENT_BODY_LENGTH = 65536;
710
const NO_REPRO_MESSAGE = `Closing as no repro.
811
912
If this is still reproducible, please open a new issue with clear reproduction steps.`;
@@ -31,6 +34,23 @@ async function closeIssueAsNotPlanned(issueId) {
3134
});
3235
}
3336

37+
/**
38+
* Build the close-comment body with SEC-004 sanitization guarantees.
39+
*
40+
* @param {(content: string, options?: { maxLength?: number }) => string} [sanitize=sanitizeContent]
41+
* @returns {string}
42+
*/
43+
function getNoReproCommentBody(sanitize = sanitizeContent) {
44+
const body = sanitize(NO_REPRO_MESSAGE, { maxLength: MAX_COMMENT_BODY_LENGTH });
45+
if (typeof body !== "string") {
46+
throw new Error("Close comment body sanitization must return a string");
47+
}
48+
if (body.trim().length === 0) {
49+
throw new Error("Close comment body is empty after sanitization");
50+
}
51+
return body;
52+
}
53+
3454
/**
3555
* Close all open issues with the "agentic-workflows" label.
3656
* @returns {Promise<void>}
@@ -63,11 +83,11 @@ async function main() {
6383
owner,
6484
repo,
6585
issue_number: issue.number,
66-
body: NO_REPRO_MESSAGE,
86+
body: getNoReproCommentBody(),
6787
});
6888

6989
await closeIssueAsNotPlanned(issue.node_id);
7090
}
7191
}
7292

73-
module.exports = { main, closeIssueAsNotPlanned, CLOSE_ISSUE_MUTATION, NO_REPRO_MESSAGE };
93+
module.exports = { main, closeIssueAsNotPlanned, CLOSE_ISSUE_MUTATION, NO_REPRO_MESSAGE, MAX_COMMENT_BODY_LENGTH, getNoReproCommentBody };

actions/setup/js/close_agentic_workflows_issues.test.cjs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ describe("close_agentic_workflows_issues", () => {
5656
owner: "testowner",
5757
repo: "testrepo",
5858
issue_number: 101,
59-
body: module.NO_REPRO_MESSAGE,
59+
body: module.getNoReproCommentBody(),
6060
});
6161

6262
expect(mockGithub.graphql).toHaveBeenCalledTimes(1);
@@ -75,4 +75,18 @@ describe("close_agentic_workflows_issues", () => {
7575
expect(mockGithub.rest.issues.createComment).not.toHaveBeenCalled();
7676
expect(mockGithub.graphql).not.toHaveBeenCalled();
7777
});
78+
79+
it("sanitizes and validates the close comment body", async () => {
80+
const module = await import("./close_agentic_workflows_issues.cjs");
81+
const { sanitizeContent } = await import("./sanitize_content.cjs");
82+
const sanitize = vi.fn().mockReturnValue(module.NO_REPRO_MESSAGE);
83+
84+
expect(module.getNoReproCommentBody()).toBe(sanitizeContent(module.NO_REPRO_MESSAGE, { maxLength: module.MAX_COMMENT_BODY_LENGTH }));
85+
expect(module.getNoReproCommentBody(sanitize)).toBe(module.NO_REPRO_MESSAGE);
86+
expect(sanitize).toHaveBeenCalledWith(module.NO_REPRO_MESSAGE, { maxLength: module.MAX_COMMENT_BODY_LENGTH });
87+
expect(() => module.getNoReproCommentBody(() => "")).toThrow("Close comment body is empty after sanitization");
88+
expect(() => module.getNoReproCommentBody(() => " ")).toThrow("Close comment body is empty after sanitization");
89+
expect(() => module.getNoReproCommentBody(() => " \n\t ")).toThrow("Close comment body is empty after sanitization");
90+
expect(() => module.getNoReproCommentBody(() => 42)).toThrow("Close comment body sanitization must return a string");
91+
});
7892
});

actions/setup/js/mcp_cli_bridge.cjs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
/**
44
* mcp_cli_bridge.cjs
55
*
6+
* @safe-outputs-exempt SEC-004: "body" references are transport payloads/responses, not user-authored comment bodies
7+
*
68
* Node.js bridge that handles MCP session protocol for CLI wrapper scripts.
79
* Each CLI wrapper is a thin bash script that invokes this bridge with the
810
* server configuration and user-provided command + arguments.

actions/setup/js/mount_mcp_as_cli.cjs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
/**
55
* mount_mcp_as_cli.cjs
66
*
7+
* @safe-outputs-exempt SEC-004: "body" references are JSON-RPC transport payloads, not user-authored comment bodies
8+
*
79
* Mounts MCP servers as local CLI tools by reading the manifest written by
810
* start_mcp_gateway.cjs, querying each server for its tool list, and generating
911
* a standalone bash wrapper script per server in ${RUNNER_TEMP}/gh-aw/mcp-cli/bin/.

actions/setup/js/start_mcp_gateway.cjs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ require("./shim.cjs");
77
/**
88
* start_mcp_gateway.cjs
99
*
10+
* @safe-outputs-exempt SEC-004: "body" references are local healthcheck response payloads, not user-authored comment bodies
11+
*
1012
* Starts the MCP gateway process that proxies MCP servers through a unified HTTP endpoint.
1113
* Following the MCP Gateway Specification:
1214
* https://github.com/github/gh-aw/blob/main/docs/src/content/docs/reference/mcp-gateway.md

scripts/check-safe-outputs-conformance.sh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,11 @@ check_sanitization() {
138138
for handler in actions/setup/js/*.cjs; do
139139
# Skip test and utility files
140140
[[ "$handler" =~ (test|parse|buffer) ]] && continue
141+
142+
# Skip files with a documented SEC-004 exemption annotation
143+
if grep -q "@safe-outputs-exempt[[:space:]]\\+SEC-004" "$handler"; then
144+
continue
145+
fi
141146

142147
# Check if handler has body/content fields
143148
if grep -q "\"body\"\|body:" "$handler"; then

0 commit comments

Comments
 (0)