fix(action): fail closed when security scan output is invalid

.github/actions/security-scan/action.yml

@@ -125,76 +125,30 @@ runs:
           # Use the merge base so we only see issues introduced in this PR.
           BASE_REF="${{ github.event.pull_request.base.sha || 'HEAD~1' }}"
           agent-security-scanner-mcp scan-diff "$BASE_REF" HEAD \
-            --verbosity full > scan-results.json 2>&1
+            --verbosity full > scan-results.json 2> scan-results.stderr
           SCAN_EXIT=$?
           echo "::endgroup::"
         else
           echo "::group::Running full project security scan"
           agent-security-scanner-mcp scan-project . \
-            --verbosity full > scan-results.json 2>&1
+            --verbosity full > scan-results.json 2> scan-results.stderr
           SCAN_EXIT=$?
           echo "::endgroup::"
         fi
 
-        # Parse issue counts from the JSON output.
-        # The scanner outputs JSON with issues_count, or structured issue arrays.
-        if [ -f scan-results.json ] && python3 -c "import json; json.load(open('scan-results.json'))" 2>/dev/null; then
-          ISSUES_COUNT=$(python3 -c "
-        import json, sys
-        try:
-            data = json.load(open('scan-results.json'))
-            total = data.get('issues_count', data.get('total', 0))
-            print(total)
-        except Exception:
-            print(0)
-        ")
-
-          CRITICAL_COUNT=$(python3 -c "
-        import json, sys
-        try:
-            data = json.load(open('scan-results.json'))
-            issues = data.get('issues', data.get('files', []))
-            count = 0
-            if isinstance(issues, list):
-                for item in issues:
-                    if isinstance(item, dict):
-                        sev = item.get('severity', '').upper()
-                        if sev == 'ERROR':
-                            count += 1
-                        # Handle nested issues in project scan
-                        for nested in item.get('issues', []):
-                            if isinstance(nested, dict) and nested.get('severity', '').upper() == 'ERROR':
-                                count += 1
-            print(count)
-        except Exception:
-            print(0)
-        ")
-
-          WARNING_COUNT=$(python3 -c "
-        import json, sys
-        try:
-            data = json.load(open('scan-results.json'))
-            issues = data.get('issues', data.get('files', []))
-            count = 0
-            if isinstance(issues, list):
-                for item in issues:
-                    if isinstance(item, dict):
-                        sev = item.get('severity', '').upper()
-                        if sev == 'WARNING':
-                            count += 1
-                        for nested in item.get('issues', []):
-                            if isinstance(nested, dict) and nested.get('severity', '').upper() == 'WARNING':
-                                count += 1
-            print(count)
-        except Exception:
-            print(0)
-        ")
-        else
-          ISSUES_COUNT=0
-          CRITICAL_COUNT=0
-          WARNING_COUNT=0
+        # Parse issue counts from JSON output.
+        # Fail closed on malformed/invalid schema output instead of silently reporting 0 issues.
+        COUNTS=$(python3 "${{ github.action_path }}/../../../scripts/parse_scan_results.py" scan-results.json 2>&1)
+        PARSE_EXIT=$?
+        if [ "$PARSE_EXIT" -ne 0 ]; then
+          echo "::error::Failed to parse security scan output safely (fail-closed)."
+          echo "$COUNTS"
+          [ -s scan-results.stderr ] && cat scan-results.stderr
+          exit 1
         fi
 
+        IFS=$'\t' read -r ISSUES_COUNT CRITICAL_COUNT WARNING_COUNT <<< "$COUNTS"
+
         echo "issues_count=$ISSUES_COUNT" >> "$GITHUB_OUTPUT"
         echo "critical_count=$CRITICAL_COUNT" >> "$GITHUB_OUTPUT"
         echo "warning_count=$WARNING_COUNT" >> "$GITHUB_OUTPUT"

scripts/parse_scan_results.py

@@ -0,0 +1,97 @@
+#!/usr/bin/env python3
+"""
+Parse security scanner JSON output and emit counts as:
+<total>\t<critical>\t<warning>
+
+Exits non-zero when output is malformed or schema-invalid (fail-closed).
+"""
+
+from __future__ import annotations
+
+import json
+import sys
+from pathlib import Path
+
+
+def _count_severity(data: dict, level: str) -> int:
+    level = level.upper()
+    count = 0
+
+    issues = data.get("issues", [])
+    if isinstance(issues, list):
+        for item in issues:
+            if not isinstance(item, dict):
+                continue
+            if str(item.get("severity", "")).upper() == level:
+                count += 1
+            nested = item.get("issues", [])
+            if isinstance(nested, list):
+                for nested_item in nested:
+                    if isinstance(nested_item, dict) and str(nested_item.get("severity", "")).upper() == level:
+                        count += 1
+
+    files = data.get("files", [])
+    if isinstance(files, list):
+        for file_item in files:
+            if not isinstance(file_item, dict):
+                continue
+            nested = file_item.get("issues", [])
+            if isinstance(nested, list):
+                for nested_item in nested:
+                    if isinstance(nested_item, dict) and str(nested_item.get("severity", "")).upper() == level:
+                        count += 1
+
+    return count
+
+
+def parse_counts(path: Path) -> tuple[int, int, int]:
+    if not path.exists() or path.stat().st_size == 0:
+        raise ValueError("security scanner produced no JSON output")
+
+    try:
+        data = json.loads(path.read_text(encoding="utf-8"))
+    except Exception as exc:  # noqa: BLE001
+        raise ValueError(f"invalid JSON output: {exc}") from exc
+
+    if not isinstance(data, dict):
+        raise ValueError("scanner output must be a JSON object")
+
+    if data.get("error"):
+        raise ValueError(f"scanner returned error: {data.get('error')}")
+
+    if not any(
+        key in data
+        for key in ("issues_count", "total", "issues", "files", "by_severity", "grade")
+    ):
+        raise ValueError("scanner output schema not recognized")
+
+    total = data.get("issues_count", data.get("total", 0))
+    try:
+        total_int = int(total)
+    except Exception as exc:  # noqa: BLE001
+        raise ValueError(f"invalid total count: {total}") from exc
+
+    critical = _count_severity(data, "ERROR")
+    warning = _count_severity(data, "WARNING")
+
+    return total_int, critical, warning
+
+
+def main() -> int:
+    if len(sys.argv) != 2:
+        print("usage: parse_scan_results.py <scan-results.json>", file=sys.stderr)
+        return 2
+
+    input_path = Path(sys.argv[1])
+    try:
+        total, critical, warning = parse_counts(input_path)
+    except ValueError as exc:
+        print(str(exc), file=sys.stderr)
+        return 1
+
+    print(f"{total}\t{critical}\t{warning}")
+    return 0
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())

tests/parse-scan-results.test.js

@@ -0,0 +1,147 @@
+import { describe, it, expect } from 'vitest';
+import { execFileSync } from 'child_process';
+import { mkdtempSync, writeFileSync, rmSync } from 'fs';
+import { join } from 'path';
+import { tmpdir } from 'os';
+
+const SCRIPT = join(process.cwd(), 'scripts', 'parse_scan_results.py');
+
+function runParser(filePath) {
+  return execFileSync('python3', [SCRIPT, filePath], { encoding: 'utf-8' }).trim();
+}
+
+describe('parse_scan_results.py', () => {
+  it('parses valid compact/flat issue output', () => {
+    const dir = mkdtempSync(join(tmpdir(), 'scan-parse-'));
+    const file = join(dir, 'scan-results.json');
+    try {
+      writeFileSync(file, JSON.stringify({
+        issues_count: 3,
+        issues: [
+          { severity: 'error' },
+          { severity: 'WARNING' },
+          { severity: 'info' },
+        ],
+      }), 'utf-8');
+
+      const out = runParser(file);
+      expect(out).toBe('3\t1\t1');
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  it('counts nested file issues when files[] schema is used', () => {
+    const dir = mkdtempSync(join(tmpdir(), 'scan-parse-'));
+    const file = join(dir, 'scan-results.json');
+    try {
+      writeFileSync(file, JSON.stringify({
+        total: 2,
+        files: [
+          {
+            file: 'a.js',
+            issues: [{ severity: 'ERROR' }, { severity: 'warning' }],
+          },
+        ],
+      }), 'utf-8');
+
+      const out = runParser(file);
+      expect(out).toBe('2\t1\t1');
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  it('fails closed on invalid JSON', () => {
+    const dir = mkdtempSync(join(tmpdir(), 'scan-parse-'));
+    const file = join(dir, 'scan-results.json');
+    try {
+      writeFileSync(file, 'not-json', 'utf-8');
+      expect(() => runParser(file)).toThrow();
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  it('fails closed on empty JSON file', () => {
+    const dir = mkdtempSync(join(tmpdir(), 'scan-parse-'));
+    const file = join(dir, 'scan-results.json');
+    try {
+      writeFileSync(file, '', 'utf-8');
+      expect(() => runParser(file)).toThrow();
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  it('fails closed when JSON root is not an object', () => {
+    const dir = mkdtempSync(join(tmpdir(), 'scan-parse-'));
+    const file = join(dir, 'scan-results.json');
+    try {
+      writeFileSync(file, JSON.stringify(['not', 'an', 'object']), 'utf-8');
+      expect(() => runParser(file)).toThrow();
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  it('fails closed when scanner returns error object', () => {
+    const dir = mkdtempSync(join(tmpdir(), 'scan-parse-'));
+    const file = join(dir, 'scan-results.json');
+    try {
+      writeFileSync(file, JSON.stringify({ error: 'boom' }), 'utf-8');
+      expect(() => runParser(file)).toThrow();
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  it('fails closed when schema is not recognized', () => {
+    const dir = mkdtempSync(join(tmpdir(), 'scan-parse-'));
+    const file = join(dir, 'scan-results.json');
+    try {
+      writeFileSync(file, JSON.stringify({ hello: 'world' }), 'utf-8');
+      expect(() => runParser(file)).toThrow();
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  it('fails closed when total count is not numeric', () => {
+    const dir = mkdtempSync(join(tmpdir(), 'scan-parse-'));
+    const file = join(dir, 'scan-results.json');
+    try {
+      writeFileSync(file, JSON.stringify({
+        total: 'not-a-number',
+        issues: [{ severity: 'ERROR' }],
+      }), 'utf-8');
+      expect(() => runParser(file)).toThrow();
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  it('counts severities across both top-level issues and nested files issues', () => {
+    const dir = mkdtempSync(join(tmpdir(), 'scan-parse-'));
+    const file = join(dir, 'scan-results.json');
+    try {
+      writeFileSync(file, JSON.stringify({
+        issues_count: 4,
+        issues: [
+          { severity: 'ERROR' },
+          { severity: 'warning' },
+        ],
+        files: [
+          {
+            file: 'a.py',
+            issues: [{ severity: 'ERROR' }, { severity: 'WARNING' }],
+          },
+        ],
+      }), 'utf-8');
+      const out = runParser(file);
+      expect(out).toBe('4\t2\t2');
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+});