-
Notifications
You must be signed in to change notification settings - Fork 307
feat: mount custom GitHub Actions as safe output tools via safe-outputs.actions
#21752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b3565d0
ce8b3ef
b550238
1b7f9bb
44a547d
df8c250
84df56f
6e6b35e
398b143
6a9c487
ead4e55
bdc20c1
7a554f9
67d4dd5
29c9973
5aad3d9
b3559c9
8cc1b77
a588b34
ec1e666
31250a1
f2ee097
de5f3ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,10 @@ | ||
| { | ||
| "entries": { | ||
| "actions-ecosystem/action-add-labels@v1": { | ||
| "repo": "actions-ecosystem/action-add-labels", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding a comment explaining the |
||
| "version": "v1", | ||
| "sha": "18f1af5e3544586314bbe15c0273249c770b2daf" | ||
| }, | ||
| "actions/ai-inference@v2.0.7": { | ||
| "repo": "actions/ai-inference", | ||
| "version": "v2.0.7", | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| // @ts-check | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Smoke test review comment #2: New JS handler file — consider adding JSDoc comments at the top for IDE tooling support. |
||
| /// <reference types="@actions/github-script" /> | ||
|
|
||
| /** | ||
| * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction | ||
| */ | ||
|
|
||
| const { replaceTemporaryIdReferences } = require("./temporary_id.cjs"); | ||
| const { getErrorMessage } = require("./error_helpers.cjs"); | ||
| const { sanitizeContent } = require("./sanitize_content.cjs"); | ||
|
|
||
| /** | ||
| * Internal safe-output message fields that should not be forwarded as action inputs. | ||
| * These fields are part of the safe-output messaging protocol and are not user-defined. | ||
| * Maintained as an explicit set so future protocol fields can be added without silently | ||
| * forwarding them to external action `with:` inputs. | ||
| * @type {Set<string>} | ||
| */ | ||
| const INTERNAL_MESSAGE_FIELDS = new Set(["type"]); | ||
|
|
||
| /** | ||
| * Main handler factory for a custom safe output action. | ||
| * | ||
| * Each configured safe-output action gets its own instance of this factory function, | ||
| * invoked with a config that includes `action_name` (the normalized tool name). | ||
| * | ||
| * The handler: | ||
| * 1. Enforces that the action is called at most once (per the spec). | ||
| * 2. Applies temporary ID substitutions to all string-valued fields in the payload. | ||
| * 3. Exports the processed payload as a step output named `action_<name>_payload`. | ||
| * | ||
| * The compiler generates a corresponding GitHub Actions step with: | ||
| * if: steps.process_safe_outputs.outputs.action_<name>_payload != '' | ||
| * uses: <resolved-action-ref> | ||
| * with: | ||
| * <input>: ${{ fromJSON(steps.process_safe_outputs.outputs.action_<name>_payload).<input> }} | ||
| * | ||
| * @type {HandlerFactoryFunction} | ||
| */ | ||
| async function main(config = {}) { | ||
| const actionName = config.action_name || "unknown_action"; | ||
| const outputKey = `action_${actionName}_payload`; | ||
|
|
||
| core.info(`Custom action handler initialized: action_name=${actionName}, output_key=${outputKey}`); | ||
|
|
||
| // Track whether this action has been called (enforces once-only constraint) | ||
| let called = false; | ||
|
|
||
| /** | ||
| * Handler function that processes a single tool call for this action. | ||
| * Applies temporary ID substitutions and exports the payload as a step output. | ||
| * | ||
| * @param {Object} message - The tool call message from the agent output | ||
| * @param {Object} resolvedTemporaryIds - Map of temp IDs to resolved values (plain object) | ||
| * @param {Map<string, Object>} temporaryIdMap - Live map of temporary IDs (for substitution) | ||
| * @returns {Promise<Object>} Result with success/error status | ||
| */ | ||
| return async function handleCustomAction(message, resolvedTemporaryIds, temporaryIdMap = new Map()) { | ||
| // Enforce once-only constraint | ||
| if (called) { | ||
| const error = `Action "${actionName}" can only be called once per workflow run`; | ||
| core.warning(error); | ||
| return { | ||
| success: false, | ||
| error, | ||
| }; | ||
| } | ||
| called = true; | ||
|
|
||
| try { | ||
| core.info(`Processing custom action: ${actionName}`); | ||
|
|
||
| // Build the processed payload by: | ||
| // 1. Applying temporary ID reference substitutions to string fields | ||
| // 2. Redacting (sanitizing) all string fields via sanitizeContent() before export. | ||
| // This prevents prompt-injected content from leaking URLs, mentions, or harmful | ||
| // content into external action inputs. | ||
| const processedInputs = {}; | ||
| for (const [key, value] of Object.entries(message)) { | ||
| // Skip internal safe-output messaging fields that are not action inputs. | ||
| // Maintained as an explicit set to allow future additions without silently | ||
| // forwarding new internal fields to external action steps. | ||
| if (INTERNAL_MESSAGE_FIELDS.has(key)) { | ||
| continue; | ||
| } | ||
|
|
||
| if (typeof value === "string") { | ||
| // Apply temporary ID reference substitution (e.g., "aw_abc1" → "42"), then | ||
| // sanitize to redact malicious URLs, neutralize bot-trigger phrases, and | ||
| // escape @mentions that could cause unintended notifications in the action. | ||
| const substituted = replaceTemporaryIdReferences(value, temporaryIdMap); | ||
| processedInputs[key] = sanitizeContent(substituted); | ||
| } else { | ||
| processedInputs[key] = value; | ||
| } | ||
| } | ||
|
|
||
| // Export the processed payload as a step output | ||
| const payloadJSON = JSON.stringify(processedInputs); | ||
| core.setOutput(outputKey, payloadJSON); | ||
| core.info(`✓ Custom action "${actionName}": exported payload as "${outputKey}" (${payloadJSON.length} bytes)`); | ||
|
|
||
| return { | ||
| success: true, | ||
| action_name: actionName, | ||
| payload: payloadJSON, | ||
| }; | ||
| } catch (error) { | ||
| const errorMessage = getErrorMessage(error); | ||
| core.error(`Failed to process custom action "${actionName}": ${errorMessage}`); | ||
| return { | ||
| success: false, | ||
| error: `Failed to process custom action "${actionName}": ${errorMessage}`, | ||
| }; | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| module.exports = { main }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| // @ts-check | ||
|
|
||
| import { describe, it, expect, beforeEach, vi } from "vitest"; | ||
| import { main } from "./safe_output_action_handler.cjs"; | ||
|
|
||
| describe("safe_output_action_handler", () => { | ||
| beforeEach(() => { | ||
| // Provide a mock global `core` matching the @actions/core API surface used by the handler. | ||
| global.core = { | ||
| info: vi.fn(), | ||
| debug: vi.fn(), | ||
| warning: vi.fn(), | ||
| error: vi.fn(), | ||
| setOutput: vi.fn(), | ||
| setFailed: vi.fn(), | ||
| }; | ||
| }); | ||
|
|
||
| describe("main() — factory", () => { | ||
| it("should return a function (the handler) when called", async () => { | ||
| const handler = await main({ action_name: "my_action" }); | ||
| expect(typeof handler).toBe("function"); | ||
| }); | ||
|
|
||
| it("should use 'unknown_action' when config is not provided", async () => { | ||
| const handler = await main(); | ||
| expect(typeof handler).toBe("function"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("handler — basic payload export", () => { | ||
| it("should export the payload as a step output and return success", async () => { | ||
| const handler = await main({ action_name: "add_label" }); | ||
|
|
||
| const message = { type: "add_label", labels: "bug" }; | ||
| const result = await handler(message, {}, new Map()); | ||
|
|
||
| expect(result.success).toBe(true); | ||
| expect(result.action_name).toBe("add_label"); | ||
|
|
||
| // 'type' (an INTERNAL_MESSAGE_FIELDS member) must be stripped from the exported payload | ||
| const exported = JSON.parse(global.core.setOutput.mock.calls[0][1]); | ||
| expect(exported).toHaveProperty("labels", "bug"); | ||
| expect(exported).not.toHaveProperty("type"); | ||
| }); | ||
|
|
||
| it("should use the correct output key format action_<name>_payload", async () => { | ||
| const handler = await main({ action_name: "my_action" }); | ||
| await handler({ type: "my_action", key: "value" }, {}, new Map()); | ||
|
|
||
| expect(global.core.setOutput).toHaveBeenCalledWith("action_my_action_payload", expect.any(String)); | ||
| }); | ||
|
|
||
| it("should pass through non-string values without sanitization", async () => { | ||
| const handler = await main({ action_name: "my_action" }); | ||
| const message = { type: "my_action", count: 42, active: true }; | ||
|
|
||
| await handler(message, {}, new Map()); | ||
|
|
||
| const exported = JSON.parse(global.core.setOutput.mock.calls[0][1]); | ||
| expect(exported.count).toBe(42); | ||
| expect(exported.active).toBe(true); | ||
| }); | ||
| }); | ||
|
|
||
| describe("handler — once-only enforcement", () => { | ||
| it("should return an error on the second call", async () => { | ||
| const handler = await main({ action_name: "my_action" }); | ||
| const message = { labels: "bug" }; | ||
|
|
||
| const first = await handler(message, {}, new Map()); | ||
| expect(first.success).toBe(true); | ||
|
|
||
| const second = await handler(message, {}, new Map()); | ||
| expect(second.success).toBe(false); | ||
| expect(second.error).toContain("can only be called once"); | ||
|
|
||
| // setOutput should only be called once (for the first invocation) | ||
| expect(global.core.setOutput).toHaveBeenCalledTimes(1); | ||
| }); | ||
| }); | ||
|
|
||
| describe("handler — INTERNAL_MESSAGE_FIELDS filtering", () => { | ||
| it("should strip 'type' from the exported payload", async () => { | ||
| const handler = await main({ action_name: "my_action" }); | ||
| await handler({ type: "my_action", title: "hello" }, {}, new Map()); | ||
|
|
||
| const exported = JSON.parse(global.core.setOutput.mock.calls[0][1]); | ||
| expect(exported).not.toHaveProperty("type"); | ||
| expect(exported).toHaveProperty("title", "hello"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("handler — temporaryIdMap substitution", () => { | ||
| it("should substitute temporary ID references in string values", async () => { | ||
| const handler = await main({ action_name: "my_action" }); | ||
| // The temporary ID pattern is "#aw_XXXX" (3-12 alphanumeric chars with #aw_ prefix) | ||
| const temporaryIdMap = new Map([["aw_abc1", { repo: "owner/repo", number: 99 }]]); | ||
| await handler({ type: "my_action", body: "Fixes #aw_abc1" }, {}, temporaryIdMap); | ||
|
|
||
| const exported = JSON.parse(global.core.setOutput.mock.calls[0][1]); | ||
| // replaceTemporaryIdReferences should replace '#aw_abc1' with the issue reference | ||
| expect(exported.body).not.toContain("#aw_abc1"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("handler — empty payload", () => { | ||
| it("should handle an empty message (only internal fields)", async () => { | ||
| const handler = await main({ action_name: "my_action" }); | ||
| const result = await handler({ type: "my_action" }, {}, new Map()); | ||
|
|
||
| expect(result.success).toBe(true); | ||
| const exported = JSON.parse(global.core.setOutput.mock.calls[0][1]); | ||
| expect(exported).toEqual({}); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New action pin entry added. Consider verifying the SHA corresponds to the correct version tag to ensure reproducibility and security.