Skip to content

security scan: fail closed on scanner crashes, malformed output and dash filenames - #1975

Closed
remigiusz-antczak wants to merge 1 commit into
mainfrom
land/sec-folds3
Closed

remigiusz-antczak wants to merge 1 commit into
mainfrom
land/sec-folds3

Conversation

@remigiusz-antczak

Copy link
Copy Markdown
Collaborator

Hardens the whole-app security scan (review_checks.sh, from #1970) so that a scanner that crashed, timed out or returned malformed output can no longer read as a clean result.

  • JSON parsing fails closed. Malformed output, or valid JSON of the wrong shape, gives a not run: <tool> line instead of zero findings.
  • semgrep: entries in its errors array are surfaced as a partial-scan line next to the findings. Files go to semgrep in chunks of 200 per call, each with its own timeout and --.
  • Exit codes: each tool's crash code, as distinct from its findings code, is surfaced for gitleaks, osv-scanner, semgrep, shellcheck, ruff, pyflakes, tsc and go vet.
  • Files whose names start with a dash are passed as ./name or after --, so a tool cannot read them as options.
  • The gitleaks report is truncated before each file, so one file's findings are never attributed to the next. An empty report counts as no findings.
  • SKILL.md now says the security-review-mode reference is loaded only on that path.

Tests: 32/32 (19 new in scripts/test-review-checks-security-folds.sh plus 13 existing). 13 of the 19 new checks fail on the pre-change script, run through the REVIEW_CHECKS override. The other 6 check that the new code does not over-block. Independent review found no blocking issues and 6 warnings; all 6 are fixed here.

🤖 Generated with Claude Code

https://claude.ai/code/session_014QNFFPWVcPUbeuvwM5fk2Z

…atch semgrep

Scanner JSON that is missing, empty, malformed or the wrong shape now
produces a "not run" line instead of reading as zero findings, a semgrep
`errors` array is reported as a partial scan, and crash exit codes from
gitleaks, osv-scanner, semgrep, shellcheck, ruff, pyflakes, tsc and go vet
are surfaced rather than swallowed (findings codes such as shellcheck 1 or
osv-scanner 1 are not crashes). Changed files reach the linters, gitleaks
and osv-scanner as ./path so a leading-dash name is never parsed as an
option, the gitleaks report is cleared before every run, and semgrep runs
once per 200 changed files after `--` instead of once per file. SKILL.md
routes the whole-app security reference lazily.

Tests: 32/32 (19 in test-review-checks-security-folds.sh, 13 existing)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014QNFFPWVcPUbeuvwM5fk2Z
@remigiusz-antczak

Copy link
Copy Markdown
Collaborator Author

Superseded by #1978.

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.

1 participant