mirror of
https://github.com/trailofbits/skills.git
synced 2026-09-14 14:28:48 +08:00
Eduard Milushi
ea5327d467
* fix(semgrep): stop non-rule YAML in a cloned repo killing the whole scan semgrep parses every .yaml/.yml under a --config directory as a rule, and one unparseable file aborts the entire scan with exit 7 — the rules that were fine produce nothing. Rule repos ship their own CI config next to their rules, and a workflow's `on: pull_request:` is a null value semgrep rejects outright. This silently zeroed two required third-party rulesets: trailofbits/semgrep-rules .github/workflows/semgrep-rules-format.yml elttam/semgrep-rules perf-templates/benchmark-tests.yml Both were reported as failed scans, so the run looked complete while two rule sources contributed nothing. A semgrep rule file always has a top-level `rules:` key and nothing else here does, so prune on that after cloning. It also drops `*.test.yaml` fixtures, which are rule test inputs rather than rules. Measured on the two repos above: keeps 118/145 and 80/94 files, losing no real rule. * fix(semgrep): prune join-mode rules that crash the scanner A `mode: join` rule crashes semgrep 1.173 with an AttributeError in join_rule.py. That is a hard process failure, not a rule-level error: the batch dies and writes no output at all, so every other rule in the same invocation is lost with it. Surfaces once the non-rule YAML prune lets elttam/semgrep-rules get far enough to load rules/generic/jsp-likely-xss.yaml. join is experimental and rare, so dropping those rules costs little next to losing the run that contains them. * fix(semgrep): keep results from a scan whose rules partly failed to compile Exit 2 was treated as "no scan happened", alongside exit 7. It is not: semgrep also returns 2 when individual rules fail to compile while the run completes and writes full JSON and SARIF. Two rulesets were discarded because of it. elttam/semgrep-rules has 12 Java rules current semgrep cannot parse, and ran 107 others fine over ts/php/js/yaml. apiiro/malicious-code-ruleset was filed as failed while its own log read "Scan completed successfully • Findings: 51" — 51 real findings dropped, with nothing in scans.json indicating a loss. Judge on the artifacts rather than the exit code: the existing `jq -e .results` check already proves semgrep produced a parseable result set. Adds `partial` and `exitCode` to each scan entry so a degraded run is visible — reporting it as an unqualified success would overstate coverage, but dropping it understated it far worse. * fix(semgrep): keep exit 7 fatal while still rescuing exit 2 The previous commit dropped the exit-code gate entirely, judging a scan purely on its artifacts. That went further than the problem needed and broke four upstream assertions under "execution, exit codes and finding counts", which pin exit 7 as a failure. Exit 2 still needs rescuing and is genuinely ambiguous: semgrep returns it both for a config that will not load, where it writes nothing, and for a run where some rules failed to compile while the rest completed and wrote full output. The artifact checks separate those two, so 2 is allowed through and flagged partial. Anything outside 0/1/2 is fatal regardless of what was written. Measured rather than assumed, because the exit code for an unloadable config turns out to depend on the OUTPUT FLAGS. semgrep 1.173, same rules directory and target, back to back: semgrep --config rules target -> 7, nothing written semgrep --config rules -o out.json --sarif-output=out.sarif -> 2, nothing written This script uses the second form, so an unloadable config arrives as exit 2 with no artifacts and the -s checks reject it unaided. The fatal branch is therefore belt-and-braces rather than the load-bearing part — but it costs nothing, it is what the suite pins, and it keeps a future semgrep that writes an empty result set alongside a hard failure from reading as a clean scan. Adds the coverage this branch never had for its own central behaviour: exit 2 with complete output is kept, counted and marked partial; exit 2 with no output still fails. Without it the next refactor reverts this silently, which is exactly what the previous commit did to exit 7. Bumps static-analysis to 1.3.3 in both plugin.json and marketplace.json, which the validator requires to agree, and rebases onto main since #258 has since touched this same script. * Bump static-analysis to 1.4.1 The bump this PR originally carried was lost when main moved to 1.4.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(semgrep): test the prunes, narrow join match, surface partial scans --------- Co-authored-by: kz-tob <kara.zaffarano@trailofbits.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>