Skip to content

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

Open
Ty-Robb wants to merge 3 commits intosinewaveai:mainfrom
Ty-Robb:fix/67-fail-closed-scan-results
Open

fix(action): fail closed when security scan output is invalid#71
Ty-Robb wants to merge 3 commits intosinewaveai:mainfrom
Ty-Robb:fix/67-fail-closed-scan-results

Conversation

@Ty-Robb
Copy link
Copy Markdown

@Ty-Robb Ty-Robb commented Apr 16, 2026

Summary

This PR hardens the composite security-scan action to fail closed when scanner output is malformed or schema-invalid, instead of silently reporting zero findings.

Changes

  • Capture scanner JSON on stdout and logs on stderr (scan-results.stderr)
  • Add strict JSON parsing/validation for scan-results.json
  • Fail the job when:
    • scanner output is empty
    • JSON is invalid
    • output contains error
    • output schema is not recognized
  • Preserve issue counting for valid outputs

Why

Current behavior can fail open if JSON parsing fails due to mixed stderr content, resulting in 0 issues and a false pass.

Fixes #67

@Ty-Robb
Copy link
Copy Markdown
Author

Ty-Robb commented Apr 16, 2026

Validation notes from local verification:

  • Added regression coverage for fail-closed parsing behavior.
  • Ran targeted tests and checks:
    • npx vitest run tests/parse-scan-results.test.js (pass)
    • npx vitest run tests/sarif-output.test.js tests/scan-project.test.js tests/scan-diff.test.js (pass)
    • YAML parse check for .github/actions/security-scan/action.yml (pass)
  • In one earlier broad/parallel run, we observed a timeout in a SARIF test (environment/resource-sensitive). Isolated reruns passed.

Recommendation: merge on green CI.

@Ty-Robb
Copy link
Copy Markdown
Author

Ty-Robb commented Apr 16, 2026

Follow-up hardening added before merge:

  • Expanded parser regression coverage for additional fail-closed edge cases:
    • empty JSON file
    • non-object JSON root
    • invalid numeric total
    • combined issues[] + files[].issues[] severity counting

Validation rerun after this test expansion:

  • npx vitest run tests/parse-scan-results.test.js (pass)
  • npx vitest run tests/sarif-output.test.js tests/scan-project.test.js tests/scan-diff.test.js (pass)
  • manual smoke with real scan-project output + parser script (pass)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Full-project SARIF conversion appears incompatible with current scan-project output

1 participant