From ac4aa2047a4fd8a18308d91dc790f8f5277da632 Mon Sep 17 00:00:00 2001 From: David Slater Date: Wed, 25 Mar 2026 05:13:36 +0000 Subject: [PATCH 1/6] fix: parse threat detection results from detection.log instead of agent_output.json MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The threat detection parser was reading agent_output.json (the main agent's structured output) instead of detection.log (the detection copilot's stdout piped via tee). Since agent_output.json never contains THREAT_DETECTION_RESULT, the verdict always defaulted to clean — silently ignoring real detections. Changes: - Read from detection.log (DETECTION_LOG_FILENAME constant) instead of agent_output.json - Support both stream-json format (type:result envelope) and raw text format - Error on missing result, multiple results, invalid JSON, and file I/O failures instead of silently defaulting to clean - Extract only from type:result envelopes (not type:assistant) to avoid double-counting in stream-json mode - Add comprehensive tests for both output formats --- actions/setup/js/constants.cjs | 9 + actions/setup/js/constants.test.cjs | 19 +- .../js/parse_threat_detection_results.cjs | 178 +++++++-- .../parse_threat_detection_results.test.cjs | 350 ++++++++++++++++++ 4 files changed, 520 insertions(+), 36 deletions(-) create mode 100644 actions/setup/js/parse_threat_detection_results.test.cjs diff --git a/actions/setup/js/constants.cjs b/actions/setup/js/constants.cjs index 1d377c70b3..6adcb33b0d 100644 --- a/actions/setup/js/constants.cjs +++ b/actions/setup/js/constants.cjs @@ -80,6 +80,14 @@ const RPC_MESSAGES_PATH = `${TMP_GH_AW_PATH}/mcp-logs/rpc-messages.jsonl`; */ const MANIFEST_FILE_PATH = `${TMP_GH_AW_PATH}/safe-output-items.jsonl`; +/** + * Filename of the threat detection log written by the detection engine via tee. + * The detection copilot's stdout (containing THREAT_DETECTION_RESULT) is piped + * through `tee -a` to this file inside the threat-detection directory. + * @type {string} + */ +const DETECTION_LOG_FILENAME = "detection.log"; + module.exports = { AGENT_OUTPUT_FILENAME, TMP_GH_AW_PATH, @@ -90,4 +98,5 @@ module.exports = { GATEWAY_JSONL_PATH, RPC_MESSAGES_PATH, MANIFEST_FILE_PATH, + DETECTION_LOG_FILENAME, }; diff --git a/actions/setup/js/constants.test.cjs b/actions/setup/js/constants.test.cjs index e29cf1bb70..0369d3174a 100644 --- a/actions/setup/js/constants.test.cjs +++ b/actions/setup/js/constants.test.cjs @@ -1,12 +1,16 @@ // @ts-check import { describe, it, expect } from "vitest"; -const { AGENT_OUTPUT_FILENAME, TMP_GH_AW_PATH, COPILOT_REVIEWER_BOT, FAQ_CREATE_PR_PERMISSIONS_URL, MAX_LABELS, MAX_ASSIGNEES, GATEWAY_JSONL_PATH, RPC_MESSAGES_PATH, MANIFEST_FILE_PATH } = require("./constants.cjs"); +const { AGENT_OUTPUT_FILENAME, TMP_GH_AW_PATH, COPILOT_REVIEWER_BOT, FAQ_CREATE_PR_PERMISSIONS_URL, MAX_LABELS, MAX_ASSIGNEES, GATEWAY_JSONL_PATH, RPC_MESSAGES_PATH, MANIFEST_FILE_PATH, DETECTION_LOG_FILENAME } = require("./constants.cjs"); describe("constants", () => { describe("file names", () => { it("should export AGENT_OUTPUT_FILENAME", () => { expect(AGENT_OUTPUT_FILENAME).toBe("agent_output.json"); }); + + it("should export DETECTION_LOG_FILENAME", () => { + expect(DETECTION_LOG_FILENAME).toBe("detection.log"); + }); }); describe("paths", () => { @@ -60,7 +64,18 @@ describe("constants", () => { describe("module exports", () => { it("should export all expected constants", () => { const exported = require("./constants.cjs"); - const expectedKeys = ["AGENT_OUTPUT_FILENAME", "TMP_GH_AW_PATH", "COPILOT_REVIEWER_BOT", "FAQ_CREATE_PR_PERMISSIONS_URL", "MAX_LABELS", "MAX_ASSIGNEES", "GATEWAY_JSONL_PATH", "RPC_MESSAGES_PATH", "MANIFEST_FILE_PATH"]; + const expectedKeys = [ + "AGENT_OUTPUT_FILENAME", + "TMP_GH_AW_PATH", + "COPILOT_REVIEWER_BOT", + "FAQ_CREATE_PR_PERMISSIONS_URL", + "MAX_LABELS", + "MAX_ASSIGNEES", + "GATEWAY_JSONL_PATH", + "RPC_MESSAGES_PATH", + "MANIFEST_FILE_PATH", + "DETECTION_LOG_FILENAME", + ]; for (const key of expectedKeys) { expect(exported).toHaveProperty(key); } diff --git a/actions/setup/js/parse_threat_detection_results.cjs b/actions/setup/js/parse_threat_detection_results.cjs index e064c35e4b..28d9634489 100644 --- a/actions/setup/js/parse_threat_detection_results.cjs +++ b/actions/setup/js/parse_threat_detection_results.cjs @@ -4,36 +4,138 @@ /** * Parse Threat Detection Results * - * This module parses the threat detection results from the agent output file - * and determines whether any security threats were detected (prompt injection, - * secret leak, malicious patch). It sets the appropriate output and fails the - * workflow if threats are detected. + * This module parses the threat detection results from the detection log file + * (written by the detection copilot via tee) and determines whether any + * security threats were detected (prompt injection, secret leak, malicious + * patch). It sets the appropriate output and fails the workflow if threats + * are detected. + * + * The detection copilot writes its verdict to stdout which is piped through + * `tee -a` to detection.log. This parser reads that file — NOT agent_output.json + * (which is the main agent's structured output used as *input* to the detection + * copilot). */ const fs = require("fs"); const path = require("path"); const { getErrorMessage } = require("./error_helpers.cjs"); const { listFilesRecursively } = require("./file_helpers.cjs"); -const { AGENT_OUTPUT_FILENAME } = require("./constants.cjs"); -const { ERR_SYSTEM, ERR_VALIDATION } = require("./error_codes.cjs"); +const { DETECTION_LOG_FILENAME } = require("./constants.cjs"); +const { ERR_SYSTEM, ERR_PARSE, ERR_VALIDATION } = require("./error_codes.cjs"); + +const RESULT_PREFIX = "THREAT_DETECTION_RESULT:"; + +/** + * Try to extract a THREAT_DETECTION_RESULT value from a stream-json line. + * Stream-json output from Claude wraps the result in JSON envelopes like: + * {"type":"result","result":"THREAT_DETECTION_RESULT:{\"prompt_injection\":...}"} + * + * The same result also appears in {"type":"assistant"} messages, but we only + * extract from "type":"result" which is the authoritative final summary. + * + * @param {string} line - A single line from the detection log + * @returns {string|null} The raw THREAT_DETECTION_RESULT:... string if found, null otherwise + */ +function extractFromStreamJson(line) { + const trimmed = line.trim(); + if (!trimmed.startsWith("{")) return null; + + try { + const obj = JSON.parse(trimmed); + // Only extract from the authoritative "result" summary, not "assistant" messages. + // In stream-json mode, the same content appears in both; using only "result" + // avoids double-counting. + if (obj.type === "result" && typeof obj.result === "string") { + const resultStr = obj.result.trim(); + if (resultStr.startsWith(RESULT_PREFIX)) { + return resultStr; + } + } + } catch { + // Not valid JSON — not a stream-json line + } + return null; +} + +/** + * Parse the detection log and extract the THREAT_DETECTION_RESULT. + * + * Supports two output formats: + * 1. **Stream-json** (--output-format stream-json): The result is embedded inside + * a JSON envelope: {"type":"result","result":"THREAT_DETECTION_RESULT:{...}"} + * 2. **Raw/text** (--output-format text or --print): The result appears as a + * bare line: THREAT_DETECTION_RESULT:{...} + * + * Strategy: extract from stream-json "type":"result" lines first (authoritative). + * If none found, fall back to raw line matching. This avoids double-counting + * since stream-json mode produces both "assistant" and "result" envelopes + * containing the same string. + * + * @param {string} content - The raw detection log content + * @returns {{ verdict?: { prompt_injection: boolean, secret_leak: boolean, malicious_patch: boolean, reasons: string[] }, error?: string }} + */ +function parseDetectionLog(content) { + const lines = content.split("\n"); + + // Phase 1: Try stream-json extraction (from "type":"result" lines only) + const streamMatches = []; + for (const line of lines) { + const extracted = extractFromStreamJson(line); + if (extracted) { + streamMatches.push(extracted); + } + } + + // Phase 2: If no stream-json results, try raw line matching + const rawMatches = []; + if (streamMatches.length === 0) { + for (const line of lines) { + const trimmed = line.trim(); + if (trimmed.startsWith(RESULT_PREFIX)) { + rawMatches.push(trimmed); + } + } + } + + const matches = streamMatches.length > 0 ? streamMatches : rawMatches; + + if (matches.length === 0) { + return { error: "No THREAT_DETECTION_RESULT found in detection log. The detection model may have failed to follow the output format." }; + } + + if (matches.length > 1) { + return { error: `Multiple THREAT_DETECTION_RESULT entries found (${matches.length}) in detection log. Expected exactly one. Entries: ${matches.map((m, i) => `\n [${i + 1}] ${m}`).join("")}` }; + } + + const jsonPart = matches[0].substring(RESULT_PREFIX.length); + try { + const parsed = JSON.parse(jsonPart); + const verdict = { + prompt_injection: Boolean(parsed.prompt_injection), + secret_leak: Boolean(parsed.secret_leak), + malicious_patch: Boolean(parsed.malicious_patch), + reasons: Array.isArray(parsed.reasons) ? parsed.reasons : [], + }; + return { verdict }; + } catch (/** @type {any} */ parseError) { + return { error: `Failed to parse JSON from THREAT_DETECTION_RESULT: ${getErrorMessage(parseError)}\nRaw value: ${matches[0]}` }; + } +} /** * Main entry point for parsing threat detection results * @returns {Promise} */ async function main() { - // Parse threat detection results - let verdict = { prompt_injection: false, secret_leak: false, malicious_patch: false, reasons: [] }; + const threatDetectionDir = "/tmp/gh-aw/threat-detection"; + const logPath = path.join(threatDetectionDir, DETECTION_LOG_FILENAME); - try { - // Agent output artifact is downloaded to /tmp/gh-aw/threat-detection/ - // GitHub Actions places single-file artifacts directly in the target directory - const threatDetectionDir = "/tmp/gh-aw/threat-detection"; - const outputPath = path.join(threatDetectionDir, AGENT_OUTPUT_FILENAME); - if (!fs.existsSync(outputPath)) { - core.error("❌ Agent output file not found at: " + outputPath); - // List all files in artifact directory for debugging - core.info("📁 Listing all files in artifact directory: " + threatDetectionDir); + // Check that the detection log exists + if (!fs.existsSync(logPath)) { + core.error("❌ Detection log file not found at: " + logPath); + // List all files in artifact directory for debugging + core.info("📁 Listing all files in artifact directory: " + threatDetectionDir); + try { const files = listFilesRecursively(threatDetectionDir, threatDetectionDir); if (files.length === 0) { core.warning(" No files found in " + threatDetectionDir); @@ -41,22 +143,32 @@ async function main() { core.info(" Found " + files.length + " file(s):"); files.forEach(file => core.info(" - " + file)); } - core.setFailed(`${ERR_SYSTEM}: ❌ Agent output file not found at: ${outputPath}`); - return; + } catch { + core.warning(" Could not list files in " + threatDetectionDir); } - const outputContent = fs.readFileSync(outputPath, "utf8"); - const lines = outputContent.split("\n"); + core.setOutput("success", "false"); + core.setFailed(`${ERR_SYSTEM}: ❌ Detection log file not found at: ${logPath}`); + return; + } - for (const line of lines) { - const trimmedLine = line.trim(); - if (trimmedLine.startsWith("THREAT_DETECTION_RESULT:")) { - const jsonPart = trimmedLine.substring("THREAT_DETECTION_RESULT:".length); - verdict = { ...verdict, ...JSON.parse(jsonPart) }; - break; - } - } - } catch (error) { - core.warning("Failed to parse threat detection results: " + getErrorMessage(error)); + // Read the detection log + let logContent; + try { + logContent = fs.readFileSync(logPath, "utf8"); + } catch (/** @type {any} */ readError) { + core.setOutput("success", "false"); + core.setFailed(`${ERR_SYSTEM}: ❌ Failed to read detection log: ${getErrorMessage(readError)}`); + return; + } + + // Parse the detection result + const { verdict, error } = parseDetectionLog(logContent); + + if (error) { + core.error("❌ " + error); + core.setOutput("success", "false"); + core.setFailed(`${ERR_PARSE}: ❌ ${error}`); + return; } core.info("Threat detection verdict: " + JSON.stringify(verdict)); @@ -70,14 +182,12 @@ async function main() { const reasonsText = verdict.reasons && verdict.reasons.length > 0 ? "\nReasons: " + verdict.reasons.join("; ") : ""; - // Set success output to false before failing core.setOutput("success", "false"); core.setFailed(`${ERR_VALIDATION}: ❌ Security threats detected: ${threats.join(", ")}${reasonsText}`); } else { core.info("✅ No security threats detected. Safe outputs may proceed."); - // Set success output to true when no threats detected core.setOutput("success", "true"); } } -module.exports = { main }; +module.exports = { main, parseDetectionLog, extractFromStreamJson }; diff --git a/actions/setup/js/parse_threat_detection_results.test.cjs b/actions/setup/js/parse_threat_detection_results.test.cjs new file mode 100644 index 0000000000..8a3e275109 --- /dev/null +++ b/actions/setup/js/parse_threat_detection_results.test.cjs @@ -0,0 +1,350 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; + +// Create mock functions +const mockExistsSync = vi.fn(() => false); +const mockReadFileSync = vi.fn(() => ""); + +// Mock fs module +vi.mock("fs", () => { + return { + existsSync: mockExistsSync, + readFileSync: mockReadFileSync, + default: { + existsSync: mockExistsSync, + readFileSync: mockReadFileSync, + }, + }; +}); + +// Mock file_helpers to avoid transitive fs issues +vi.mock("./file_helpers.cjs", () => { + return { + listFilesRecursively: vi.fn(() => []), + }; +}); + +// Mock the global core object +const mockCore = { + info: vi.fn(), + warning: vi.fn(), + error: vi.fn(), + setFailed: vi.fn(), + setOutput: vi.fn(), +}; +global.core = mockCore; + +const { parseDetectionLog, extractFromStreamJson } = require("./parse_threat_detection_results.cjs"); + +describe("extractFromStreamJson", () => { + it("should extract result from type:result JSON envelope", () => { + const line = '{"type":"result","subtype":"success","is_error":false,"result":"THREAT_DETECTION_RESULT:{\\"prompt_injection\\":false,\\"secret_leak\\":false,\\"malicious_patch\\":false,\\"reasons\\":[]}","stop_reason":"end_turn"}'; + const result = extractFromStreamJson(line); + expect(result).toContain("THREAT_DETECTION_RESULT:"); + }); + + it("should return null for type:assistant JSON (not authoritative)", () => { + const line = '{"type":"assistant","message":{"content":[{"type":"text","text":"THREAT_DETECTION_RESULT:{\\"prompt_injection\\":false}"}]}}'; + const result = extractFromStreamJson(line); + expect(result).toBeNull(); + }); + + it("should return null for non-JSON lines", () => { + expect(extractFromStreamJson("just plain text")).toBeNull(); + expect(extractFromStreamJson("THREAT_DETECTION_RESULT:{...}")).toBeNull(); + }); + + it("should return null for JSON without result field", () => { + const line = '{"type":"result","subtype":"success"}'; + expect(extractFromStreamJson(line)).toBeNull(); + }); + + it("should return null for type:result where result does not start with prefix", () => { + const line = '{"type":"result","result":"some other output"}'; + expect(extractFromStreamJson(line)).toBeNull(); + }); + + it("should return null for malformed JSON", () => { + expect(extractFromStreamJson("{not valid json}")).toBeNull(); + }); +}); + +describe("parseDetectionLog", () => { + describe("valid results", () => { + it("should parse a clean verdict (no threats)", () => { + const content = 'Some debug output\nTHREAT_DETECTION_RESULT:{"prompt_injection":false,"secret_leak":false,"malicious_patch":false,"reasons":[]}\nMore output'; + const { verdict, error } = parseDetectionLog(content); + + expect(error).toBeUndefined(); + expect(verdict).toEqual({ + prompt_injection: false, + secret_leak: false, + malicious_patch: false, + reasons: [], + }); + }); + + it("should parse a verdict with threats detected", () => { + const content = 'THREAT_DETECTION_RESULT:{"prompt_injection":true,"secret_leak":false,"malicious_patch":true,"reasons":["found backdoor","injected prompt"]}'; + const { verdict, error } = parseDetectionLog(content); + + expect(error).toBeUndefined(); + expect(verdict.prompt_injection).toBe(true); + expect(verdict.secret_leak).toBe(false); + expect(verdict.malicious_patch).toBe(true); + expect(verdict.reasons).toEqual(["found backdoor", "injected prompt"]); + }); + + it("should handle leading/trailing whitespace on the result line", () => { + const content = ' THREAT_DETECTION_RESULT:{"prompt_injection":false,"secret_leak":false,"malicious_patch":false,"reasons":[]} '; + const { verdict, error } = parseDetectionLog(content); + + expect(error).toBeUndefined(); + expect(verdict.prompt_injection).toBe(false); + }); + + it("should coerce non-boolean values to booleans", () => { + const content = 'THREAT_DETECTION_RESULT:{"prompt_injection":1,"secret_leak":0,"malicious_patch":"yes","reasons":"not-array"}'; + const { verdict, error } = parseDetectionLog(content); + + expect(error).toBeUndefined(); + expect(verdict.prompt_injection).toBe(true); + expect(verdict.secret_leak).toBe(false); + expect(verdict.malicious_patch).toBe(true); + expect(verdict.reasons).toEqual([]); + }); + }); + + describe("no result line", () => { + it("should return error when no THREAT_DETECTION_RESULT line exists", () => { + const content = "Some debug output\nNo result here\nMore output"; + const { verdict, error } = parseDetectionLog(content); + + expect(verdict).toBeUndefined(); + expect(error).toContain("No THREAT_DETECTION_RESULT found"); + }); + + it("should return error for empty content", () => { + const { verdict, error } = parseDetectionLog(""); + + expect(verdict).toBeUndefined(); + expect(error).toContain("No THREAT_DETECTION_RESULT found"); + }); + + it("should return error when content has only whitespace", () => { + const { verdict, error } = parseDetectionLog(" \n \n "); + + expect(verdict).toBeUndefined(); + expect(error).toContain("No THREAT_DETECTION_RESULT found"); + }); + }); + + describe("multiple result lines", () => { + it("should return error when multiple THREAT_DETECTION_RESULT lines found", () => { + const content = [ + 'THREAT_DETECTION_RESULT:{"prompt_injection":false,"secret_leak":false,"malicious_patch":false,"reasons":[]}', + 'THREAT_DETECTION_RESULT:{"prompt_injection":true,"secret_leak":false,"malicious_patch":false,"reasons":["injection"]}', + ].join("\n"); + const { verdict, error } = parseDetectionLog(content); + + expect(verdict).toBeUndefined(); + expect(error).toContain("Multiple THREAT_DETECTION_RESULT entries found (2)"); + }); + + it("should include raw lines in error for debugging", () => { + const content = [ + 'THREAT_DETECTION_RESULT:{"prompt_injection":false,"secret_leak":false,"malicious_patch":false,"reasons":[]}', + "some other output", + 'THREAT_DETECTION_RESULT:{"prompt_injection":true,"secret_leak":false,"malicious_patch":false,"reasons":[]}', + ].join("\n"); + const { error } = parseDetectionLog(content); + + expect(error).toContain("[1]"); + expect(error).toContain("[2]"); + }); + }); + + describe("invalid JSON", () => { + it("should return error when JSON is malformed", () => { + const content = "THREAT_DETECTION_RESULT:{not valid json}"; + const { verdict, error } = parseDetectionLog(content); + + expect(verdict).toBeUndefined(); + expect(error).toContain("Failed to parse JSON from THREAT_DETECTION_RESULT"); + expect(error).toContain("Raw value:"); + }); + + it("should return error when JSON is empty", () => { + const content = "THREAT_DETECTION_RESULT:"; + const { verdict, error } = parseDetectionLog(content); + + expect(verdict).toBeUndefined(); + expect(error).toContain("Failed to parse JSON"); + }); + + it("should return error when JSON is truncated", () => { + const content = 'THREAT_DETECTION_RESULT:{"prompt_injection":true,"secret_leak":'; + const { verdict, error } = parseDetectionLog(content); + + expect(verdict).toBeUndefined(); + expect(error).toContain("Failed to parse JSON"); + }); + }); + + describe("stream-json format (--output-format stream-json)", () => { + it("should extract result from type:result JSON envelope", () => { + const content = [ + "2026-03-23T00:04:39.809Z [DEBUG] Fast mode unavailable", + '{"type":"assistant","message":{"model":"claude-sonnet-4-6","content":[{"type":"text","text":"THREAT_DETECTION_RESULT:{\\"prompt_injection\\":false,\\"secret_leak\\":false,\\"malicious_patch\\":false,\\"reasons\\":[]}"}]}}', + '{"type":"result","subtype":"success","is_error":false,"result":"THREAT_DETECTION_RESULT:{\\"prompt_injection\\":false,\\"secret_leak\\":false,\\"malicious_patch\\":false,\\"reasons\\":[]}","stop_reason":"end_turn"}', + "2026-03-23T00:04:42.251Z [DEBUG] LSP server manager shut down successfully", + ].join("\n"); + const { verdict, error } = parseDetectionLog(content); + + expect(error).toBeUndefined(); + expect(verdict).toEqual({ + prompt_injection: false, + secret_leak: false, + malicious_patch: false, + reasons: [], + }); + }); + + it("should extract threats from stream-json format", () => { + const content = [ + '{"type":"result","subtype":"success","result":"THREAT_DETECTION_RESULT:{\\"prompt_injection\\":true,\\"secret_leak\\":false,\\"malicious_patch\\":false,\\"reasons\\":[\\"Injected JSON payload in prompt.txt\\"]}","stop_reason":"end_turn"}', + ].join("\n"); + const { verdict, error } = parseDetectionLog(content); + + expect(error).toBeUndefined(); + expect(verdict.prompt_injection).toBe(true); + expect(verdict.reasons).toEqual(["Injected JSON payload in prompt.txt"]); + }); + + it("should not double-count from assistant and result envelopes", () => { + // Both assistant and result contain the same THREAT_DETECTION_RESULT + // The parser should only extract from type:result (authoritative) + const content = [ + '{"type":"assistant","message":{"content":[{"type":"text","text":"THREAT_DETECTION_RESULT:{\\"prompt_injection\\":false,\\"secret_leak\\":false,\\"malicious_patch\\":false,\\"reasons\\":[]}"}]}}', + '{"type":"result","result":"THREAT_DETECTION_RESULT:{\\"prompt_injection\\":false,\\"secret_leak\\":false,\\"malicious_patch\\":false,\\"reasons\\":[]}"}', + ].join("\n"); + const { verdict, error } = parseDetectionLog(content); + + expect(error).toBeUndefined(); + expect(verdict).toBeDefined(); + }); + }); + + describe("non-stream format (--print / --output-format text)", () => { + it("should extract from a realistic non-stream detection log", () => { + const content = [ + "● Read workflow prompt and agent output files (shell)", + " │ cat /tmp/gh-aw/threat-detection/aw-prompts/prompt.txt", + " └ 195 lines...", + "", + "Looking at the content carefully, I notice a classic prompt injection pattern.", + "", + 'THREAT_DETECTION_RESULT:{"prompt_injection":true,"secret_leak":false,"malicious_patch":false,"reasons":["Injected JSON payload in prompt.txt"]}', + "", + "Total usage est: 1 Premium request", + ].join("\n"); + const { verdict, error } = parseDetectionLog(content); + + expect(error).toBeUndefined(); + expect(verdict.prompt_injection).toBe(true); + expect(verdict.secret_leak).toBe(false); + expect(verdict.malicious_patch).toBe(false); + expect(verdict.reasons).toEqual(["Injected JSON payload in prompt.txt"]); + }); + }); +}); + +describe("main", () => { + let mod; + + beforeEach(async () => { + vi.clearAllMocks(); + mockExistsSync.mockReturnValue(false); + mockReadFileSync.mockReturnValue(""); + // Re-import to get fresh module with mocks + mod = await import("./parse_threat_detection_results.cjs"); + }); + + it("should fail when detection log file does not exist", async () => { + mockExistsSync.mockReturnValue(false); + + await mod.main(); + + expect(mockCore.setOutput).toHaveBeenCalledWith("success", "false"); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Detection log file not found")); + }); + + // Note: The following tests are skipped because mocking fs for CJS modules + // is difficult in vitest (same issue as safe_output_validator.test.cjs). + // The core parsing logic is thoroughly tested via parseDetectionLog above. + // These tests document the expected behavior of main() for each scenario. + describe.skip("with detection log file present (CJS fs mock limitation)", () => { + it("should fail when detection log has no result line", async () => { + mockExistsSync.mockReturnValue(true); + mockReadFileSync.mockReturnValue("just some debug output\nno result here\n"); + + await mod.main(); + + expect(mockCore.setOutput).toHaveBeenCalledWith("success", "false"); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("No THREAT_DETECTION_RESULT line found")); + }); + + it("should fail when detection log has multiple result lines", async () => { + mockExistsSync.mockReturnValue(true); + mockReadFileSync.mockReturnValue( + ['THREAT_DETECTION_RESULT:{"prompt_injection":false,"secret_leak":false,"malicious_patch":false,"reasons":[]}', 'THREAT_DETECTION_RESULT:{"prompt_injection":true,"secret_leak":false,"malicious_patch":false,"reasons":[]}'].join("\n") + ); + + await mod.main(); + + expect(mockCore.setOutput).toHaveBeenCalledWith("success", "false"); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Multiple THREAT_DETECTION_RESULT lines found")); + }); + + it("should fail when result JSON is invalid", async () => { + mockExistsSync.mockReturnValue(true); + mockReadFileSync.mockReturnValue("THREAT_DETECTION_RESULT:{bad json}"); + + await mod.main(); + + expect(mockCore.setOutput).toHaveBeenCalledWith("success", "false"); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Failed to parse JSON")); + }); + + it("should succeed with clean verdict", async () => { + mockExistsSync.mockReturnValue(true); + mockReadFileSync.mockReturnValue('debug output\nTHREAT_DETECTION_RESULT:{"prompt_injection":false,"secret_leak":false,"malicious_patch":false,"reasons":[]}\n'); + + await mod.main(); + + expect(mockCore.setOutput).toHaveBeenCalledWith("success", "true"); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + }); + + it("should fail when threats are detected", async () => { + mockExistsSync.mockReturnValue(true); + mockReadFileSync.mockReturnValue('THREAT_DETECTION_RESULT:{"prompt_injection":true,"secret_leak":false,"malicious_patch":false,"reasons":["found injection"]}'); + + await mod.main(); + + expect(mockCore.setOutput).toHaveBeenCalledWith("success", "false"); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Security threats detected: prompt injection")); + }); + + it("should fail when readFileSync throws", async () => { + mockExistsSync.mockReturnValue(true); + mockReadFileSync.mockImplementation(() => { + throw new Error("EACCES: permission denied"); + }); + + await mod.main(); + + expect(mockCore.setOutput).toHaveBeenCalledWith("success", "false"); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Failed to read detection log")); + }); + }); +}); From 12f3d7e3754666942cd52c9d49fbbd8809827c6e Mon Sep 17 00:00:00 2001 From: David Slater Date: Wed, 25 Mar 2026 05:17:47 +0000 Subject: [PATCH 2/6] fix: narrow verdict type to satisfy TypeScript --- actions/setup/js/parse_threat_detection_results.cjs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/actions/setup/js/parse_threat_detection_results.cjs b/actions/setup/js/parse_threat_detection_results.cjs index 28d9634489..f0b14d85be 100644 --- a/actions/setup/js/parse_threat_detection_results.cjs +++ b/actions/setup/js/parse_threat_detection_results.cjs @@ -164,10 +164,10 @@ async function main() { // Parse the detection result const { verdict, error } = parseDetectionLog(logContent); - if (error) { - core.error("❌ " + error); + if (error || !verdict) { + core.error("❌ " + (error || "No verdict returned from detection log parser")); core.setOutput("success", "false"); - core.setFailed(`${ERR_PARSE}: ❌ ${error}`); + core.setFailed(`${ERR_PARSE}: ❌ ${error || "No verdict returned from detection log parser"}`); return; } From 2afc128b80c23a8779768dfdb8f4f2211f3174f4 Mon Sep 17 00:00:00 2001 From: David Slater Date: Wed, 25 Mar 2026 05:28:59 +0000 Subject: [PATCH 3/6] fix: strict boolean validation for threat flags, fix skipped test messages, update prompt - Reject non-boolean threat flag types (string "false" would have been misclassified as true via Boolean coercion) - Add regression tests for string/number flag values - Fix skipped main() test expectations to match current error messages - Update threat_detection.md prompt to require JSON boolean types --- .../js/parse_threat_detection_results.cjs | 16 ++++++++-- .../parse_threat_detection_results.test.cjs | 31 ++++++++++++++----- actions/setup/md/threat_detection.md | 1 + pkg/workflow/prompts/threat_detection.md | 1 + 4 files changed, 38 insertions(+), 11 deletions(-) diff --git a/actions/setup/js/parse_threat_detection_results.cjs b/actions/setup/js/parse_threat_detection_results.cjs index f0b14d85be..3880d71ed6 100644 --- a/actions/setup/js/parse_threat_detection_results.cjs +++ b/actions/setup/js/parse_threat_detection_results.cjs @@ -110,10 +110,20 @@ function parseDetectionLog(content) { const jsonPart = matches[0].substring(RESULT_PREFIX.length); try { const parsed = JSON.parse(jsonPart); + + // Validate that threat flags are actual booleans. + // Boolean("false") === true, so accepting non-boolean types would cause + // false positives (string "false" treated as a detection). + for (const field of ["prompt_injection", "secret_leak", "malicious_patch"]) { + if (typeof parsed[field] !== "boolean") { + return { error: `Invalid type for "${field}": expected boolean, got ${typeof parsed[field]} (${JSON.stringify(parsed[field])}). Raw value: ${matches[0]}` }; + } + } + const verdict = { - prompt_injection: Boolean(parsed.prompt_injection), - secret_leak: Boolean(parsed.secret_leak), - malicious_patch: Boolean(parsed.malicious_patch), + prompt_injection: parsed.prompt_injection, + secret_leak: parsed.secret_leak, + malicious_patch: parsed.malicious_patch, reasons: Array.isArray(parsed.reasons) ? parsed.reasons : [], }; return { verdict }; diff --git a/actions/setup/js/parse_threat_detection_results.test.cjs b/actions/setup/js/parse_threat_detection_results.test.cjs index 8a3e275109..8a90150afa 100644 --- a/actions/setup/js/parse_threat_detection_results.test.cjs +++ b/actions/setup/js/parse_threat_detection_results.test.cjs @@ -102,15 +102,30 @@ describe("parseDetectionLog", () => { expect(verdict.prompt_injection).toBe(false); }); - it("should coerce non-boolean values to booleans", () => { + it("should reject non-boolean values with a type error", () => { const content = 'THREAT_DETECTION_RESULT:{"prompt_injection":1,"secret_leak":0,"malicious_patch":"yes","reasons":"not-array"}'; const { verdict, error } = parseDetectionLog(content); - expect(error).toBeUndefined(); - expect(verdict.prompt_injection).toBe(true); - expect(verdict.secret_leak).toBe(false); - expect(verdict.malicious_patch).toBe(true); - expect(verdict.reasons).toEqual([]); + expect(verdict).toBeUndefined(); + expect(error).toContain('Invalid type for "prompt_injection"'); + expect(error).toContain("expected boolean"); + }); + + it('should reject string "false" as non-boolean', () => { + const content = 'THREAT_DETECTION_RESULT:{"prompt_injection":"false","secret_leak":false,"malicious_patch":false,"reasons":[]}'; + const { verdict, error } = parseDetectionLog(content); + + expect(verdict).toBeUndefined(); + expect(error).toContain('Invalid type for "prompt_injection"'); + expect(error).toContain("got string"); + }); + + it('should reject string "true" as non-boolean', () => { + const content = 'THREAT_DETECTION_RESULT:{"prompt_injection":"true","secret_leak":false,"malicious_patch":false,"reasons":[]}'; + const { verdict, error } = parseDetectionLog(content); + + expect(verdict).toBeUndefined(); + expect(error).toContain('Invalid type for "prompt_injection"'); }); }); @@ -290,7 +305,7 @@ describe("main", () => { await mod.main(); expect(mockCore.setOutput).toHaveBeenCalledWith("success", "false"); - expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("No THREAT_DETECTION_RESULT line found")); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("No THREAT_DETECTION_RESULT found")); }); it("should fail when detection log has multiple result lines", async () => { @@ -302,7 +317,7 @@ describe("main", () => { await mod.main(); expect(mockCore.setOutput).toHaveBeenCalledWith("success", "false"); - expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Multiple THREAT_DETECTION_RESULT lines found")); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Multiple THREAT_DETECTION_RESULT entries found")); }); it("should fail when result JSON is invalid", async () => { diff --git a/actions/setup/md/threat_detection.md b/actions/setup/md/threat_detection.md index 3fcc5f61e5..29dc5d9162 100644 --- a/actions/setup/md/threat_detection.md +++ b/actions/setup/md/threat_detection.md @@ -52,6 +52,7 @@ Output format: THREAT_DETECTION_RESULT:{"prompt_injection":false,"secret_leak":false,"malicious_patch":false,"reasons":[]} Replace the boolean values with `true` if you detect that type of threat, `false` otherwise. +The `prompt_injection`, `secret_leak`, and `malicious_patch` fields **must** be JSON booleans (`true` or `false`), not strings, numbers, or other types. Using `"false"` (a string) instead of `false` (a boolean) will cause a parsing error. Include detailed reasons in the `reasons` array explaining any threats detected. ## Security Guidelines diff --git a/pkg/workflow/prompts/threat_detection.md b/pkg/workflow/prompts/threat_detection.md index 3fcc5f61e5..29dc5d9162 100644 --- a/pkg/workflow/prompts/threat_detection.md +++ b/pkg/workflow/prompts/threat_detection.md @@ -52,6 +52,7 @@ Output format: THREAT_DETECTION_RESULT:{"prompt_injection":false,"secret_leak":false,"malicious_patch":false,"reasons":[]} Replace the boolean values with `true` if you detect that type of threat, `false` otherwise. +The `prompt_injection`, `secret_leak`, and `malicious_patch` fields **must** be JSON booleans (`true` or `false`), not strings, numbers, or other types. Using `"false"` (a string) instead of `false` (a boolean) will cause a parsing error. Include detailed reasons in the `reasons` array explaining any threats detected. ## Security Guidelines From 908ca1326f1b30e3d670795b62f1c6c47562bce0 Mon Sep 17 00:00:00 2001 From: David Slater Date: Wed, 25 Mar 2026 05:31:23 +0000 Subject: [PATCH 4/6] fix: validate THREAT_DETECTION_RESULT JSON is a plain object Reject null, arrays, strings, and numbers with a clear error message instead of falling through to the boolean field check with a confusing 'expected boolean, got undefined' error. --- .../js/parse_threat_detection_results.cjs | 5 +++ .../parse_threat_detection_results.test.cjs | 36 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/actions/setup/js/parse_threat_detection_results.cjs b/actions/setup/js/parse_threat_detection_results.cjs index 3880d71ed6..a508a040a6 100644 --- a/actions/setup/js/parse_threat_detection_results.cjs +++ b/actions/setup/js/parse_threat_detection_results.cjs @@ -111,6 +111,11 @@ function parseDetectionLog(content) { try { const parsed = JSON.parse(jsonPart); + // The result must be a plain object, not null, an array, or a primitive. + if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) { + return { error: `THREAT_DETECTION_RESULT JSON must be an object, got ${parsed === null ? "null" : Array.isArray(parsed) ? "array" : typeof parsed}. Raw value: ${matches[0]}` }; + } + // Validate that threat flags are actual booleans. // Boolean("false") === true, so accepting non-boolean types would cause // false positives (string "false" treated as a detection). diff --git a/actions/setup/js/parse_threat_detection_results.test.cjs b/actions/setup/js/parse_threat_detection_results.test.cjs index 8a90150afa..de088bb542 100644 --- a/actions/setup/js/parse_threat_detection_results.test.cjs +++ b/actions/setup/js/parse_threat_detection_results.test.cjs @@ -203,6 +203,42 @@ describe("parseDetectionLog", () => { expect(verdict).toBeUndefined(); expect(error).toContain("Failed to parse JSON"); }); + + it("should return error when JSON is null", () => { + const content = "THREAT_DETECTION_RESULT:null"; + const { verdict, error } = parseDetectionLog(content); + + expect(verdict).toBeUndefined(); + expect(error).toContain("must be an object"); + expect(error).toContain("got null"); + }); + + it("should return error when JSON is an array", () => { + const content = "THREAT_DETECTION_RESULT:[]"; + const { verdict, error } = parseDetectionLog(content); + + expect(verdict).toBeUndefined(); + expect(error).toContain("must be an object"); + expect(error).toContain("got array"); + }); + + it("should return error when JSON is a string", () => { + const content = 'THREAT_DETECTION_RESULT:"clean"'; + const { verdict, error } = parseDetectionLog(content); + + expect(verdict).toBeUndefined(); + expect(error).toContain("must be an object"); + expect(error).toContain("got string"); + }); + + it("should return error when JSON is a number", () => { + const content = "THREAT_DETECTION_RESULT:42"; + const { verdict, error } = parseDetectionLog(content); + + expect(verdict).toBeUndefined(); + expect(error).toContain("must be an object"); + expect(error).toContain("got number"); + }); }); describe("stream-json format (--output-format stream-json)", () => { From 6a56590402cf6939d2a0ac36314bf95de8dafcc0 Mon Sep 17 00:00:00 2001 From: David Slater Date: Wed, 25 Mar 2026 05:52:47 +0000 Subject: [PATCH 5/6] fix: deduplicate identical THREAT_DETECTION_RESULT entries The detection command writes to the same file via both --debug-file and tee -a, so the same THREAT_DETECTION_RESULT line appears 2-3 times. Deduplicate identical entries instead of erroring. Only error when entries conflict (different verdicts). --- .../js/parse_threat_detection_results.cjs | 11 +++++--- .../parse_threat_detection_results.test.cjs | 26 ++++++++++++++++--- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/actions/setup/js/parse_threat_detection_results.cjs b/actions/setup/js/parse_threat_detection_results.cjs index a508a040a6..b4f76d1a46 100644 --- a/actions/setup/js/parse_threat_detection_results.cjs +++ b/actions/setup/js/parse_threat_detection_results.cjs @@ -103,11 +103,16 @@ function parseDetectionLog(content) { return { error: "No THREAT_DETECTION_RESULT found in detection log. The detection model may have failed to follow the output format." }; } - if (matches.length > 1) { - return { error: `Multiple THREAT_DETECTION_RESULT entries found (${matches.length}) in detection log. Expected exactly one. Entries: ${matches.map((m, i) => `\n [${i + 1}] ${m}`).join("")}` }; + // Deduplicate identical results. The detection command writes to the same file + // via both --debug-file and tee, so the same line often appears 2-3 times. + // Only error if the entries actually disagree (different verdicts). + const uniqueMatches = [...new Set(matches)]; + + if (uniqueMatches.length > 1) { + return { error: `Multiple conflicting THREAT_DETECTION_RESULT entries found (${uniqueMatches.length} unique out of ${matches.length} total) in detection log. Expected one consistent verdict. Entries: ${uniqueMatches.map((m, i) => `\n [${i + 1}] ${m}`).join("")}` }; } - const jsonPart = matches[0].substring(RESULT_PREFIX.length); + const jsonPart = uniqueMatches[0].substring(RESULT_PREFIX.length); try { const parsed = JSON.parse(jsonPart); diff --git a/actions/setup/js/parse_threat_detection_results.test.cjs b/actions/setup/js/parse_threat_detection_results.test.cjs index de088bb542..d094f6157f 100644 --- a/actions/setup/js/parse_threat_detection_results.test.cjs +++ b/actions/setup/js/parse_threat_detection_results.test.cjs @@ -154,7 +154,25 @@ describe("parseDetectionLog", () => { }); describe("multiple result lines", () => { - it("should return error when multiple THREAT_DETECTION_RESULT lines found", () => { + it("should deduplicate identical THREAT_DETECTION_RESULT lines", () => { + // --debug-file and tee both write to the same file, causing duplicates + const content = [ + 'THREAT_DETECTION_RESULT:{"prompt_injection":false,"secret_leak":false,"malicious_patch":false,"reasons":[]}', + 'THREAT_DETECTION_RESULT:{"prompt_injection":false,"secret_leak":false,"malicious_patch":false,"reasons":[]}', + 'THREAT_DETECTION_RESULT:{"prompt_injection":false,"secret_leak":false,"malicious_patch":false,"reasons":[]}', + ].join("\n"); + const { verdict, error } = parseDetectionLog(content); + + expect(error).toBeUndefined(); + expect(verdict).toEqual({ + prompt_injection: false, + secret_leak: false, + malicious_patch: false, + reasons: [], + }); + }); + + it("should error when conflicting THREAT_DETECTION_RESULT lines found", () => { const content = [ 'THREAT_DETECTION_RESULT:{"prompt_injection":false,"secret_leak":false,"malicious_patch":false,"reasons":[]}', 'THREAT_DETECTION_RESULT:{"prompt_injection":true,"secret_leak":false,"malicious_patch":false,"reasons":["injection"]}', @@ -162,10 +180,10 @@ describe("parseDetectionLog", () => { const { verdict, error } = parseDetectionLog(content); expect(verdict).toBeUndefined(); - expect(error).toContain("Multiple THREAT_DETECTION_RESULT entries found (2)"); + expect(error).toContain("Multiple conflicting THREAT_DETECTION_RESULT entries"); }); - it("should include raw lines in error for debugging", () => { + it("should include unique lines in error for debugging", () => { const content = [ 'THREAT_DETECTION_RESULT:{"prompt_injection":false,"secret_leak":false,"malicious_patch":false,"reasons":[]}', "some other output", @@ -353,7 +371,7 @@ describe("main", () => { await mod.main(); expect(mockCore.setOutput).toHaveBeenCalledWith("success", "false"); - expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Multiple THREAT_DETECTION_RESULT entries found")); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Multiple conflicting THREAT_DETECTION_RESULT entries")); }); it("should fail when result JSON is invalid", async () => { From 48490a8356341a8d80039d0e249e0c345adfe4f7 Mon Sep 17 00:00:00 2001 From: David Slater Date: Wed, 25 Mar 2026 05:57:13 +0000 Subject: [PATCH 6/6] style: fix Prettier formatting --- actions/setup/js/parse_threat_detection_results.cjs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/actions/setup/js/parse_threat_detection_results.cjs b/actions/setup/js/parse_threat_detection_results.cjs index b4f76d1a46..b7d69bec6b 100644 --- a/actions/setup/js/parse_threat_detection_results.cjs +++ b/actions/setup/js/parse_threat_detection_results.cjs @@ -109,7 +109,9 @@ function parseDetectionLog(content) { const uniqueMatches = [...new Set(matches)]; if (uniqueMatches.length > 1) { - return { error: `Multiple conflicting THREAT_DETECTION_RESULT entries found (${uniqueMatches.length} unique out of ${matches.length} total) in detection log. Expected one consistent verdict. Entries: ${uniqueMatches.map((m, i) => `\n [${i + 1}] ${m}`).join("")}` }; + return { + error: `Multiple conflicting THREAT_DETECTION_RESULT entries found (${uniqueMatches.length} unique out of ${matches.length} total) in detection log. Expected one consistent verdict. Entries: ${uniqueMatches.map((m, i) => `\n [${i + 1}] ${m}`).join("")}`, + }; } const jsonPart = uniqueMatches[0].substring(RESULT_PREFIX.length);