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
15 changes: 11 additions & 4 deletions actions/setup/js/generate_footer.cjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// @ts-check
/// <reference types="@actions/github-script" />

const { getDetectionReasonText, getThreatDetectedMarker } = require("./threat_detection_warning.cjs");
const { getDetectionReasonText, getThreatDetectedMarker, isToolingFailureReason } = require("./threat_detection_warning.cjs");

/**
* Generates a standalone workflow-id XML comment marker for searchability.
Expand Down Expand Up @@ -105,9 +105,13 @@ function generateXMLMarker(workflowName, runUrl) {
}

/**
* Get the detection caution alert for expired entity closing comments.
* Get the detection alert for expired entity closing comments.
* Reads GH_AW_DETECTION_CONCLUSION and GH_AW_DETECTION_REASON from environment variables.
* Returns the caution alert markdown when conclusion is "warning", or empty string otherwise.
* Returns alert markdown when conclusion is "warning", or empty string otherwise.
*
* When the reason indicates a tooling failure (agent_failure or parse_error) a [!WARNING]
* admonition is used so reviewers can distinguish "detection engine crashed" from "detection
* engine found something". Actual threat findings (threat_detected) keep [!CAUTION].
*
* Note: This function is intentionally kept inline (not imported from messages_footer.cjs)
* because importing messages_footer.cjs here would cause the bundler to inline
Expand All @@ -119,7 +123,7 @@ function generateXMLMarker(workflowName, runUrl) {
*
* @param {string} workflowName - Name of the workflow
* @param {string} runUrl - URL of the workflow run
* @returns {string} Caution alert markdown or empty string
* @returns {string} Alert markdown or empty string
*/
function getExpiredEntityCautionAlert(workflowName, runUrl) {
const detectionConclusion = process.env.GH_AW_DETECTION_CONCLUSION;
Expand All @@ -128,6 +132,9 @@ function getExpiredEntityCautionAlert(workflowName, runUrl) {
}
const detectionReason = process.env.GH_AW_DETECTION_REASON || "";
const reasonText = getDetectionReasonText(detectionReason);
if (isToolingFailureReason(detectionReason)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/codebase-design] The inline template string here is a verbatim copy of the one in messages_run_status.cjs. Any future wording change to the warning banner must be applied in two places. Since threat_detection_warning.cjs is already imported by both files, extracting the shared template there would keep the wording in sync automatically.

💡 Suggested approach

In threat_detection_warning.cjs, export a constant:

const TOOLING_FAILURE_WARNING_TEMPLATE =
  `> [!WARNING]
> threat detection engine error
> The threat detection engine encountered an error...`;
module.exports = { ..., TOOLING_FAILURE_WARNING_TEMPLATE };

Both callers import and interpolate it, removing the duplication.

@copilot please address this.

return `> [!WARNING]\n> threat detection engine error\n> The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.\n> ${getThreatDetectedMarker(detectionReason)}\n>\n> <details>\n> <summary>Details</summary>\n>\n> ${reasonText}\n>\n> Review the [workflow run logs](${runUrl}) for details.\n> </details>`;
}
return `> [!CAUTION]\n> agentic threat detected\n> Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.\n> ${getThreatDetectedMarker(detectionReason)}\n>\n> <details>\n> <summary>Details</summary>\n>\n> ${reasonText}\n>\n> Review the [workflow run logs](${runUrl}) for details.\n> </details>`;
}

Expand Down
27 changes: 27 additions & 0 deletions actions/setup/js/generate_footer.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,33 @@ describe("generate_footer.cjs", () => {
expect(result).toContain("Potential security threats were detected");
});

it("should return warning alert for agent_failure (tooling failure, not security finding)", () => {
process.env.GH_AW_DETECTION_CONCLUSION = "warning";
process.env.GH_AW_DETECTION_REASON = "agent_failure";

const result = getExpiredEntityCautionAlert("Test Workflow", "https://github.com/test/repo/actions/runs/123");

expect(result).toContain("> [!WARNING]");
expect(result).toContain("threat detection engine error");
expect(result).toContain("<!-- gh-aw-threat-detected -->");
expect(result).not.toContain("> [!CAUTION]");
expect(result).not.toContain("agentic threat detected");
expect(result).toContain("failed to produce results");
});

it("should return warning alert for parse_error (tooling failure, not security finding)", () => {
process.env.GH_AW_DETECTION_CONCLUSION = "warning";
process.env.GH_AW_DETECTION_REASON = "parse_error";

const result = getExpiredEntityCautionAlert("Test Workflow", "https://github.com/test/repo/actions/runs/123");

expect(result).toContain("> [!WARNING]");
expect(result).toContain("threat detection engine error");
expect(result).not.toContain("> [!CAUTION]");
expect(result).not.toContain("agentic threat detected");
expect(result).toContain("could not be parsed");
});

it("should return empty string when detection conclusion is not warning", () => {
process.env.GH_AW_DETECTION_CONCLUSION = "success";

Expand Down
9 changes: 6 additions & 3 deletions actions/setup/js/messages.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1402,19 +1402,22 @@ describe("messages.cjs", () => {
expect(result).toContain("Potential security threats were detected");
});

it("should return caution alert with agent_failure reason", async () => {
it("should return warning alert with agent_failure reason (tooling failure, not security finding)", async () => {
process.env.GH_AW_DETECTION_CONCLUSION = "warning";
process.env.GH_AW_DETECTION_REASON = "agent_failure";

const { getDetectionCautionAlert } = await import("./messages.cjs");

const result = getDetectionCautionAlert("Test Workflow", "https://github.com/test/repo/actions/runs/123");

expect(result).toContain("> [!CAUTION]");
expect(result).toContain("> [!WARNING]");
expect(result).toContain("threat detection engine error");
expect(result).not.toContain("> [!CAUTION]");
expect(result).not.toContain("agentic threat detected");
expect(result).toContain("threat detection engine failed");
});

it("should return caution alert with default reason when reason is empty", async () => {
it("should return caution alert with default reason when reason is empty (unknown, not tooling failure)", async () => {
process.env.GH_AW_DETECTION_CONCLUSION = "warning";

const { getDetectionCautionAlert } = await import("./messages.cjs");
Expand Down
15 changes: 13 additions & 2 deletions actions/setup/js/messages_run_status.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*/

const { getMessages, renderTemplate, toSnakeCase } = require("./messages_core.cjs");
const { getDetectionReasonText, getThreatDetectedMarkerTemplate, normalizeThreatKinds } = require("./threat_detection_warning.cjs");
const { getDetectionReasonText, getThreatDetectedMarkerTemplate, normalizeThreatKinds, isToolingFailureReason } = require("./threat_detection_warning.cjs");

/**
* Renders a message using a custom template from config or a default template.
Expand Down Expand Up @@ -141,11 +141,22 @@ function getCommitPushedMessage(ctx) {
/**
* Get the detection-warning message with progressive disclosure via details/summary.
* Used when continue-on-error is true (default) instead of false.
*
* When the reason indicates a tooling failure (agent_failure or parse_error) the
* message uses a [!WARNING] admonition so reviewers can distinguish "detection
* engine crashed" from "detection engine found something". Actual threat findings
* (threat_detected) keep the [!CAUTION] admonition.
*
* @param {DetectionWarningContext} ctx - Context for detection-warning message generation
* @returns {string} Detection-warning message with caution admonition
* @returns {string} Detection-warning message with admonition
*/
function getDetectionWarningMessage(ctx) {
const reasonText = getDetectionReasonText(ctx.reason);
const isEngineError = isToolingFailureReason(ctx.reason);
if (isEngineError) {
const defaultTemplate = `> [!WARNING]\n> threat detection engine error\n> The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.\n> ${getThreatDetectedMarkerTemplate()}\n>\n> <details>\n> <summary>Details</summary>\n>\n> {reason_text}\n>\n> Review the [workflow run logs]({run_url}) for details.\n> </details>`;
return renderConfiguredMessage("detectionEngineError", defaultTemplate, { ...ctx, reasonText, threatKinds: normalizeThreatKinds(ctx.reason) });
}
const defaultTemplate = `> [!CAUTION]\n> agentic threat detected\n> Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.\n> ${getThreatDetectedMarkerTemplate()}\n>\n> <details>\n> <summary>Details</summary>\n>\n> {reason_text}\n>\n> Review the [workflow run logs]({run_url}) for details.\n> </details>`;
return renderConfiguredMessage("detectionWarning", defaultTemplate, { ...ctx, reasonText, threatKinds: normalizeThreatKinds(ctx.reason) });
}
Expand Down
15 changes: 15 additions & 0 deletions actions/setup/js/threat_detection_warning.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,24 @@ function getDetectionReasonText(reason) {
return reasonDescriptions[normalizedReason] || "The threat detection analysis could not be completed.";
}

/**
* Returns true when the reason indicates a tooling failure rather than an actual
* security finding. Tooling failures (agent_failure, parse_error) mean the
* detection engine itself crashed or could not produce a verdict — they should be
* surfaced as a distinct infrastructure error, not as a security threat.
*
* @param {string | undefined | null} reason
* @returns {boolean}
*/
function isToolingFailureReason(reason) {
const normalized = String(reason || "").trim();
return normalized === "agent_failure" || normalized === "parse_error";
Comment on lines +67 to +69
}

module.exports = {
normalizeThreatKinds,
getThreatDetectedMarker,
getThreatDetectedMarkerTemplate,
getDetectionReasonText,
isToolingFailureReason,
};
26 changes: 25 additions & 1 deletion actions/setup/js/threat_detection_warning.test.cjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, it, expect } from "vitest";
import { normalizeThreatKinds, getThreatDetectedMarker, getThreatDetectedMarkerTemplate, getDetectionReasonText } from "./threat_detection_warning.cjs";
import { normalizeThreatKinds, getThreatDetectedMarker, getThreatDetectedMarkerTemplate, getDetectionReasonText, isToolingFailureReason } from "./threat_detection_warning.cjs";

describe("threat_detection_warning", () => {
describe("normalizeThreatKinds", () => {
Expand Down Expand Up @@ -30,4 +30,28 @@ describe("threat_detection_warning", () => {
expect(getDetectionReasonText("new_reason")).toBe("The threat detection analysis could not be completed.");
});
});

describe("isToolingFailureReason", () => {
it("returns true for agent_failure", () => {
expect(isToolingFailureReason("agent_failure")).toBe(true);
});

it("returns true for parse_error", () => {
expect(isToolingFailureReason("parse_error")).toBe(true);
});

it("returns false for threat_detected", () => {
expect(isToolingFailureReason("threat_detected")).toBe(false);
});

it("returns false for empty/null/undefined", () => {
expect(isToolingFailureReason("")).toBe(false);
expect(isToolingFailureReason(null)).toBe(false);
expect(isToolingFailureReason(undefined)).toBe(false);
});

it("returns false for unknown reason", () => {
expect(isToolingFailureReason("some_new_reason")).toBe(false);
});
});
});
Loading