mirror of
https://github.com/trailofbits/skills.git
synced 2026-09-14 14:28:48 +08:00
kz-tob
f9950784a7
* static-analysis: detect every language run-scans.sh can scan
Step 1 of the Semgrep scan globbed for 14 extensions while
`includes_for` in scripts/run-scans.sh carries globs for 41. Detection
is the only input to ruleset selection, so a category Step 1 never
reports is one Step 2 never selects and the scan never runs: YAML and
its Kubernetes, GitHub Actions, and CloudFormation variants, plus C#,
Kotlin, Scala, Swift, Elixir, Apex, and Solidity had rulesets in
references/rulesets.md that could not be reached. A header-only C++
tree (.h/.hh/.hpp/.hxx), a .cc-only one, and an ESM-only .mjs package
matched nothing either. The report read clean rather than incomplete.
The detection list is now the union of the `includes_for` globs, with a
note on the invariant and on the five types deliberately absent from
both because semgrep does not parse them. The category table gained the
rows the new patterns resolve to, so no glob dangles without a
destination; every category name is one `canonical_lang` folds. YAML
feeds four categories, so it carries disambiguation rules rather than a
single guess.
The framework markers were written without the `**/` prefix every
language pattern had, so they matched only the target root and a
monorepo keeping package.json in packages/*/ got no framework rulesets
at all. They are prefixed now, and pick up composer.json and
requirements.txt, which rulesets.md keys on but the list omitted.
workflows/semgrep-scan.js carried the same gap in its detect-phase
prompt. SKILL.md advertises it as the same scan without the approval
gate, so it gets the same category list, extension guidance, and YAML
disambiguation.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* static-analysis: drop generic JSON from language detection
Globbing **/*.json detected a "json" category on essentially every
target: package.json, tsconfig.json, lockfiles and editor settings all
match. The only ruleset keyed to that category is r/json.aws, which
covers AWS IAM policy misconfigurations, so the effect was to attach an
IAM ruleset to every scan and to report a JSON language for projects
with no JSON worth scanning.
The detection list is therefore the union of the includes_for globs
minus that one glob, and the note says so rather than leaving the
"union" claim false. IAM policies and JSON-format CloudFormation
templates are still reachable by naming the json or cloudformation
category explicitly, which is the case where selecting r/json.aws is
what the user actually wants.
The dynamic workflow's detect prompt drops json from its category list
for the same reason, and says not to glob *.json, since a bare list
would invite the model to add it back.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* static-analysis: assign JSON by content instead of dropping it
Review P2 on #281: removing **/*.json in 822f5da left r/json.aws and
JSON-format CloudFormation unreachable, and the remedy the note offered
— "name the json or cloudformation category explicitly" — has no input
path. parseArgs accepts only target, out, mode, jobs and skill; Select
sees only what Detect reported; and the gated plan lists only detected
categories, so nothing prompts the user either. A repo whose only IaC is
infra/template.json got neither ruleset, and because the category was
never planned it cannot appear in coveredNothing, failed or skipped. The
report reads clean, which is the failure mode this PR exists to remove.
So .json is globbed again, but the category is assigned from content
rather than from the extension, exactly as .yaml/.yml already is. That
keeps the reason it was dropped: package.json, tsconfig.json, lockfiles
and editor settings match no marker and yield no category, so an IAM
ruleset is not attached to every scan. AWSTemplateFormatVersion, or
Resources with a "Type": "AWS::" member, is cloudformation; a Statement
array whose elements have Effect is json. Sampling is steered at paths
that suggest infrastructure, since build config outnumbers policies.
Both extensions are now described as content-assigned in one place
rather than JSON being an exception to a union claim, so the detection
list is the full union of includes_for again — verified 41 = 41 with no
drift in either direction.
The dynamic workflow's detect prompt gets the same treatment; the review
noted it had the identical gap.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* static-analysis: tell the detect phase to report cpp, not c
Review P3 on #281: the detect prompt names both `c` and `cpp` as
categories but gives their extensions as a single group, with nothing
saying which takes which. `includes_for c` is only `*.c *.h` while
`includes_for cpp` covers `.c .cc .cpp .cxx .h .hh .hpp .hxx`, so
reporting `c` for a .cpp/.hpp tree scans with --include=*.c
--include=*.h, opens zero files and exits 0. That lands in
coveredNothing rather than failing, so the C/C++ rules never read the
source and nothing says the scan was empty for the wrong reason.
cpp is a strict superset of c, so the rule is to report the superset.
The same relation holds for javascript and typescript — javascript
already carries the .ts/.tsx globs, which run-scans.sh notes at the
includes_for comment — so both pairs are stated together rather than as
one special case.
The gated path needs no change: scan-workflow.md's table folds all eight
extensions into one `C/C++` row, and canonical_lang maps `c/c++` to cpp,
so it already resolves to the superset. Verified rather than assumed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>