From 488bb3ca8f221e71676e79e5c5035c11684b5fe4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 16:13:52 +0000 Subject: [PATCH 1/6] Initial plan From 373af5d0c8cf7940202c0f00e56a702880a4e854 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 16:22:22 +0000 Subject: [PATCH 2/6] Add sanitized_logging.cjs with safe logging functions and apply to high-risk files Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../js/add_reaction_and_edit_comment.cjs | 3 +- actions/setup/js/check_command_position.cjs | 5 +- actions/setup/js/display_file_helpers.cjs | 3 +- actions/setup/js/sanitized_logging.cjs | 71 ++++++ actions/setup/js/sanitized_logging.test.cjs | 202 ++++++++++++++++++ 5 files changed, 280 insertions(+), 4 deletions(-) create mode 100644 actions/setup/js/sanitized_logging.cjs create mode 100644 actions/setup/js/sanitized_logging.test.cjs diff --git a/actions/setup/js/add_reaction_and_edit_comment.cjs b/actions/setup/js/add_reaction_and_edit_comment.cjs index 830daaa5a71..309ee15f42d 100644 --- a/actions/setup/js/add_reaction_and_edit_comment.cjs +++ b/actions/setup/js/add_reaction_and_edit_comment.cjs @@ -4,6 +4,7 @@ const { getRunStartedMessage } = require("./messages_run_status.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { generateWorkflowIdMarker } = require("./generate_footer.cjs"); +const { safeInfo } = require("./sanitized_logging.cjs"); async function main() { // Read inputs from environment variables @@ -14,7 +15,7 @@ async function main() { const runUrl = context.payload.repository ? `${context.payload.repository.html_url}/actions/runs/${runId}` : `${githubServer}/${context.repo.owner}/${context.repo.repo}/actions/runs/${runId}`; core.info(`Reaction type: ${reaction}`); - core.info(`Command name: ${command || "none"}`); + safeInfo(`Command name: ${command || "none"}`); core.info(`Run ID: ${runId}`); core.info(`Run URL: ${runUrl}`); diff --git a/actions/setup/js/check_command_position.cjs b/actions/setup/js/check_command_position.cjs index 90787871aa0..b740bab7f6e 100644 --- a/actions/setup/js/check_command_position.cjs +++ b/actions/setup/js/check_command_position.cjs @@ -10,6 +10,7 @@ async function main() { const commandsJSON = process.env.GH_AW_COMMANDS; const { getErrorMessage } = require("./error_helpers.cjs"); + const { safeInfo } = require("./sanitized_logging.cjs"); if (!commandsJSON) { core.setFailed("Configuration error: GH_AW_COMMANDS not specified."); @@ -63,7 +64,7 @@ async function main() { const trimmedText = text.trim(); const firstWord = trimmedText.split(/\s+/)[0]; - core.info(`Checking command position. First word in text: ${firstWord}`); + safeInfo(`Checking command position. First word in text: ${firstWord}`); core.info(`Looking for commands: ${commands.map(c => `/${c}`).join(", ")}`); // Check if any of the commands match @@ -78,7 +79,7 @@ async function main() { } if (matchedCommand) { - core.info(`✓ Command '/${matchedCommand}' matched at the start of the text`); + safeInfo(`✓ Command '/${matchedCommand}' matched at the start of the text`); core.setOutput("command_position_ok", "true"); core.setOutput("matched_command", matchedCommand); } else { diff --git a/actions/setup/js/display_file_helpers.cjs b/actions/setup/js/display_file_helpers.cjs index cf96906de89..6c73144b702 100644 --- a/actions/setup/js/display_file_helpers.cjs +++ b/actions/setup/js/display_file_helpers.cjs @@ -9,6 +9,7 @@ */ const fs = require("fs"); +const { safeInfo } = require("./sanitized_logging.cjs"); /** * Display a single file's content in a collapsible group @@ -56,7 +57,7 @@ function displayFileContent(filePath, fileName, maxBytes = 64 * 1024) { core.startGroup(`${fileName} (${stats.size} bytes)`); const lines = contentToDisplay.split("\n"); for (const line of lines) { - core.info(line); + safeInfo(line); } if (wasTruncated) { core.info(`...`); diff --git a/actions/setup/js/sanitized_logging.cjs b/actions/setup/js/sanitized_logging.cjs new file mode 100644 index 00000000000..103094ce7f0 --- /dev/null +++ b/actions/setup/js/sanitized_logging.cjs @@ -0,0 +1,71 @@ +// @ts-check +/// + +/** + * Sanitized Logging Helpers + * + * This module provides safe logging functions that neutralize GitHub Actions + * workflow commands (::command::) at the start of lines to prevent workflow + * command injection when logging user-generated content. + * + * GitHub Actions interprets lines starting with "::" as workflow commands. + * For example: "::set-output name=x::value" or "::error::message" + * + * When logging user-controlled strings, these must be sanitized to prevent + * injection attacks where malicious input could trigger unintended workflow commands. + */ + +/** + * Neutralizes GitHub Actions workflow commands by replacing line-start "::" + * @param {string} message - The message to neutralize + * @returns {string} The neutralized message + */ +function neutralizeWorkflowCommands(message) { + if (typeof message !== "string") { + return message; + } + + // Replace "::" at the start of any line with ": :" (space inserted) + // The 'm' flag makes ^ match at the start of each line + return message.replace(/^::/gm, ": :"); +} + +/** + * Safe wrapper for core.info that neutralizes workflow commands + * @param {string} message - The message to log + */ +function safeInfo(message) { + core.info(neutralizeWorkflowCommands(message)); +} + +/** + * Safe wrapper for core.debug that neutralizes workflow commands + * @param {string} message - The message to log + */ +function safeDebug(message) { + core.debug(neutralizeWorkflowCommands(message)); +} + +/** + * Safe wrapper for core.warning that neutralizes workflow commands + * @param {string} message - The message to log + */ +function safeWarning(message) { + core.warning(neutralizeWorkflowCommands(message)); +} + +/** + * Safe wrapper for core.error that neutralizes workflow commands + * @param {string} message - The message to log + */ +function safeError(message) { + core.error(neutralizeWorkflowCommands(message)); +} + +module.exports = { + neutralizeWorkflowCommands, + safeInfo, + safeDebug, + safeWarning, + safeError, +}; diff --git a/actions/setup/js/sanitized_logging.test.cjs b/actions/setup/js/sanitized_logging.test.cjs new file mode 100644 index 00000000000..27369a4afd7 --- /dev/null +++ b/actions/setup/js/sanitized_logging.test.cjs @@ -0,0 +1,202 @@ +// @ts-check +const { neutralizeWorkflowCommands, safeInfo, safeDebug, safeWarning, safeError } = require("./sanitized_logging.cjs"); + +// Mock core object +global.core = { + info: vi.fn(), + debug: vi.fn(), + warning: vi.fn(), + error: vi.fn(), +}; + +describe("sanitized_logging", () => { + beforeEach(() => { + // Reset mocks before each test + vi.clearAllMocks(); + }); + + describe("neutralizeWorkflowCommands", () => { + it("should neutralize workflow commands at the start of a line", () => { + const input = "::set-output name=test::value"; + const expected = ": :set-output name=test::value"; + expect(neutralizeWorkflowCommands(input)).toBe(expected); + }); + + it("should neutralize multiple workflow commands on different lines", () => { + const input = "::error::Something failed\n::warning::Be careful\n::debug::Details here"; + const expected = ": :error::Something failed\n: :warning::Be careful\n: :debug::Details here"; + expect(neutralizeWorkflowCommands(input)).toBe(expected); + }); + + it("should not neutralize :: in the middle of a line", () => { + const input = "This has :: in the middle"; + expect(neutralizeWorkflowCommands(input)).toBe(input); + }); + + it("should not neutralize :: that is not at line start", () => { + const input = "namespace::function"; + expect(neutralizeWorkflowCommands(input)).toBe(input); + }); + + it("should handle :: in various positions correctly", () => { + const input = "Time 12:30 PM, ratio 3:1, IPv6 ::1, namespace::function"; + expect(neutralizeWorkflowCommands(input)).toBe(input); + }); + + it("should neutralize workflow command after newline", () => { + const input = "Normal text\n::set-output name=x::y"; + const expected = "Normal text\n: :set-output name=x::y"; + expect(neutralizeWorkflowCommands(input)).toBe(expected); + }); + + it("should handle empty string", () => { + expect(neutralizeWorkflowCommands("")).toBe(""); + }); + + it("should handle string with only ::", () => { + expect(neutralizeWorkflowCommands("::")).toBe(": :"); + }); + + it("should handle multiple :: at line start", () => { + const input = "::::test"; + const expected = ": :::test"; + expect(neutralizeWorkflowCommands(input)).toBe(expected); + }); + + it("should preserve :: after spaces", () => { + const input = " ::command"; + expect(neutralizeWorkflowCommands(input)).toBe(input); + }); + + it("should handle multiline with mixed patterns", () => { + const input = "First line\n::error::Bad\nmiddle::text\n::warning::Watch out"; + const expected = "First line\n: :error::Bad\nmiddle::text\n: :warning::Watch out"; + expect(neutralizeWorkflowCommands(input)).toBe(expected); + }); + + it("should handle non-string input gracefully", () => { + // @ts-expect-error - Testing non-string input + expect(neutralizeWorkflowCommands(null)).toBe(null); + // @ts-expect-error - Testing non-string input + expect(neutralizeWorkflowCommands(undefined)).toBe(undefined); + // @ts-expect-error - Testing non-string input + expect(neutralizeWorkflowCommands(123)).toBe(123); + }); + + it("should neutralize real workflow command examples", () => { + const commands = [ + { input: "::add-mask::secret", expected: ": :add-mask::secret" }, + { input: "::stop-commands::token", expected: ": :stop-commands::token" }, + { input: "::group::My Group", expected: ": :group::My Group" }, + { input: "::endgroup::", expected: ": :endgroup::" }, + { input: "::save-state name=foo::bar", expected: ": :save-state name=foo::bar" }, + ]; + + for (const { input, expected } of commands) { + expect(neutralizeWorkflowCommands(input)).toBe(expected); + } + }); + + it("should handle file content with potential workflow commands", () => { + const fileContent = ` +Some text here +::error::This is in the file +More content +::set-output name=test::value +End of file`; + const expected = ` +Some text here +: :error::This is in the file +More content +: :set-output name=test::value +End of file`; + expect(neutralizeWorkflowCommands(fileContent)).toBe(expected); + }); + }); + + describe("safeInfo", () => { + it("should call core.info with neutralized message", () => { + const message = "::error::test"; + safeInfo(message); + expect(core.info).toHaveBeenCalledWith(": :error::test"); + }); + + it("should handle safe messages without modification", () => { + const message = "This is a safe message"; + safeInfo(message); + expect(core.info).toHaveBeenCalledWith(message); + }); + + it("should neutralize multiline messages", () => { + const message = "Line 1\n::error::Line 2"; + safeInfo(message); + expect(core.info).toHaveBeenCalledWith("Line 1\n: :error::Line 2"); + }); + }); + + describe("safeDebug", () => { + it("should call core.debug with neutralized message", () => { + const message = "::debug::test"; + safeDebug(message); + expect(core.debug).toHaveBeenCalledWith(": :debug::test"); + }); + + it("should handle safe messages without modification", () => { + const message = "Debug info"; + safeDebug(message); + expect(core.debug).toHaveBeenCalledWith(message); + }); + }); + + describe("safeWarning", () => { + it("should call core.warning with neutralized message", () => { + const message = "::warning::test"; + safeWarning(message); + expect(core.warning).toHaveBeenCalledWith(": :warning::test"); + }); + + it("should handle safe messages without modification", () => { + const message = "Warning message"; + safeWarning(message); + expect(core.warning).toHaveBeenCalledWith(message); + }); + }); + + describe("safeError", () => { + it("should call core.error with neutralized message", () => { + const message = "::error::test"; + safeError(message); + expect(core.error).toHaveBeenCalledWith(": :error::test"); + }); + + it("should handle safe messages without modification", () => { + const message = "Error message"; + safeError(message); + expect(core.error).toHaveBeenCalledWith(message); + }); + }); + + describe("integration tests", () => { + it("should prevent workflow command injection from user input", () => { + // Simulate user input that tries to inject workflow commands + const userInput = "User message\n::set-output name=admin::true"; + safeInfo(userInput); + expect(core.info).toHaveBeenCalledWith("User message\n: :set-output name=admin::true"); + }); + + it("should handle command names from comment body", () => { + const commandName = "::stop-commands::token"; + safeInfo(`Command: ${commandName}`); + expect(core.info).toHaveBeenCalledWith("Command: ::stop-commands::token"); + }); + + it("should protect file content logging", () => { + const fileLines = ["::add-mask::password123", "normal line", "::error::fake error"]; + fileLines.forEach((line) => safeInfo(line)); + + expect(core.info).toHaveBeenNthCalledWith(1, ": :add-mask::password123"); + expect(core.info).toHaveBeenNthCalledWith(2, "normal line"); + expect(core.info).toHaveBeenNthCalledWith(3, ": :error::fake error"); + }); + }); +}); From 3419e6506c94c28337b1546003041b47bd29ad0c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 16:25:11 +0000 Subject: [PATCH 3/6] Format JavaScript test files and verify all tests pass Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/sanitized_logging.test.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actions/setup/js/sanitized_logging.test.cjs b/actions/setup/js/sanitized_logging.test.cjs index 27369a4afd7..3eb30815283 100644 --- a/actions/setup/js/sanitized_logging.test.cjs +++ b/actions/setup/js/sanitized_logging.test.cjs @@ -192,7 +192,7 @@ End of file`; it("should protect file content logging", () => { const fileLines = ["::add-mask::password123", "normal line", "::error::fake error"]; - fileLines.forEach((line) => safeInfo(line)); + fileLines.forEach(line => safeInfo(line)); expect(core.info).toHaveBeenNthCalledWith(1, ": :add-mask::password123"); expect(core.info).toHaveBeenNthCalledWith(2, "normal line"); From 28e8d18ba5027f554e8ec68d073d9e17d6fd4686 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 12 Feb 2026 16:48:40 +0000 Subject: [PATCH 4/6] Add changeset [skip-ci] --- .changeset/patch-sanitize-core-info-logging.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/patch-sanitize-core-info-logging.md diff --git a/.changeset/patch-sanitize-core-info-logging.md b/.changeset/patch-sanitize-core-info-logging.md new file mode 100644 index 00000000000..2032f482479 --- /dev/null +++ b/.changeset/patch-sanitize-core-info-logging.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Introduce `sanitized_logging.cjs` wrappers (`safeInfo`, `safeDebug`, `safeWarning`, `safeError`) and apply them to previously vulnerable `core.info()` calls so user-controlled strings can't inject workflow commands. From 2d65dc69803c6364a1bdfaf12e5b535838c1ef15 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 16:57:20 +0000 Subject: [PATCH 5/6] Remove unnecessary safeInfo usage for non-user-input variables Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/add_reaction_and_edit_comment.cjs | 3 +-- actions/setup/js/check_command_position.cjs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/actions/setup/js/add_reaction_and_edit_comment.cjs b/actions/setup/js/add_reaction_and_edit_comment.cjs index 309ee15f42d..830daaa5a71 100644 --- a/actions/setup/js/add_reaction_and_edit_comment.cjs +++ b/actions/setup/js/add_reaction_and_edit_comment.cjs @@ -4,7 +4,6 @@ const { getRunStartedMessage } = require("./messages_run_status.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { generateWorkflowIdMarker } = require("./generate_footer.cjs"); -const { safeInfo } = require("./sanitized_logging.cjs"); async function main() { // Read inputs from environment variables @@ -15,7 +14,7 @@ async function main() { const runUrl = context.payload.repository ? `${context.payload.repository.html_url}/actions/runs/${runId}` : `${githubServer}/${context.repo.owner}/${context.repo.repo}/actions/runs/${runId}`; core.info(`Reaction type: ${reaction}`); - safeInfo(`Command name: ${command || "none"}`); + core.info(`Command name: ${command || "none"}`); core.info(`Run ID: ${runId}`); core.info(`Run URL: ${runUrl}`); diff --git a/actions/setup/js/check_command_position.cjs b/actions/setup/js/check_command_position.cjs index b740bab7f6e..e010adbcafc 100644 --- a/actions/setup/js/check_command_position.cjs +++ b/actions/setup/js/check_command_position.cjs @@ -79,7 +79,7 @@ async function main() { } if (matchedCommand) { - safeInfo(`✓ Command '/${matchedCommand}' matched at the start of the text`); + core.info(`✓ Command '/${matchedCommand}' matched at the start of the text`); core.setOutput("command_position_ok", "true"); core.setOutput("matched_command", matchedCommand); } else { From a6153a41f72bc243047d034c8f5a9a7f55a8f6ae Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 17:19:09 +0000 Subject: [PATCH 6/6] Changes before error encountered Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/check_command_position.cjs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/actions/setup/js/check_command_position.cjs b/actions/setup/js/check_command_position.cjs index e010adbcafc..90787871aa0 100644 --- a/actions/setup/js/check_command_position.cjs +++ b/actions/setup/js/check_command_position.cjs @@ -10,7 +10,6 @@ async function main() { const commandsJSON = process.env.GH_AW_COMMANDS; const { getErrorMessage } = require("./error_helpers.cjs"); - const { safeInfo } = require("./sanitized_logging.cjs"); if (!commandsJSON) { core.setFailed("Configuration error: GH_AW_COMMANDS not specified."); @@ -64,7 +63,7 @@ async function main() { const trimmedText = text.trim(); const firstWord = trimmedText.split(/\s+/)[0]; - safeInfo(`Checking command position. First word in text: ${firstWord}`); + core.info(`Checking command position. First word in text: ${firstWord}`); core.info(`Looking for commands: ${commands.map(c => `/${c}`).join(", ")}`); // Check if any of the commands match