Skip to content

review_checks: fail closed on scanner output, guard dash filenames, batch semgrep - #1978

Merged
remigiusz-antczak merged 2 commits into
mainfrom
land/sec-folds4
Oct 24, 2026
Merged

remigiusz-antczak merged 2 commits into
mainfrom
land/sec-folds4

Conversation

@remigiusz-antczak

Copy link
Copy Markdown
Collaborator

Supersedes #1975, rebuilt as one squashed commit. Same change, plus the fix for the failed size-ratchet check: the SKILL.md line is shorter (+9 bytes) and has a size-budget-raise line in changelog.d/security-folds.md.

Local CI repro on this commit: size-ratchet ok, mustload ok, all scripts/ci-tests.d pass. test-ci-gates.sh shows 293 passed and 9 failed; the 9 are the known sandbox-only failures (dcr-gates opt-in, peer pains, reap_own pgrep, tracker socket bind).

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
remigiusz-antczak merged commit 4c3fcbc into main Oct 24, 2026
1 check passed
@remigiusz-antczak
remigiusz-antczak deleted the land/sec-folds4 branch October 24, 2026 10:38
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