feat(security): vendor Semgrep r/all ruleset with tracked exclusions by scottschreckengaust · Pull Request #220 · awslabs/agent-plugins

scottschreckengaust · 2026-07-06T23:04:15Z

Closes #211: vendor the Semgrep r/all ruleset locally so every scan (pre-commit, CI, mise) runs against a single pinned, offline-capable ruleset with exclusions baked in, instead of fetching r/all live on every run.

Closes RFC: Vendor Semgrep r/all rules locally with tracked exclusions #211 (RFC) — see that issue for the bug-bounty-probe finding that expanded this effort.
Pre-existing local pre-commit failures surfaced during this work are tracked separately in Pre-existing pre-commit run --all-files failures (tracking) #213 (sub-issues pre-commit: 12 code_templates/*.py files fail Python-parse hooks (check-ast, ruff-format) #214–pre-commit: bandit hook fails (exit 2) — missing .bandit-baseline.json #219); they are not introduced by this PR.

Changes

New tools/semgrep/ (goals A/B/C):

rules-vendored.yaml — pre-filtered rules; the only file semgrep loads. Script-generated byte-for-byte from r/all minus excluded rules. Byte-preserving (removes whole rule blocks, never re-serializes YAML — re-serializing was verified to silently break rules) and deterministic (sorts rules by id, so a regeneration with no upstream change yields an empty diff).
rules-status.toml — human-owned source of truth: explicit [rules."id"] decisions (status = excluded/active, pr, reason). Every exclusion is listed individually. The script only reads it, so hand-written # comments are preserved.
rules-update.py — stdlib-only update mechanism. Downloads r/all, drops excluded rules, rewrites rules-vendored.yaml, and reports derived new/orphaned rules.

Explicit exclusions (no implicit dropping):

A rule is excluded only when a human lists it in rules-status.toml with status = "excluded". Nothing is dropped implicitly, so a rule appearing in a future r/all can never be silently excluded — it surfaces as new for triage.
The list includes 79 bug-bounty-probe rules that leaked into r/all (SSRF-via-metadata, stored-XSS in message/fix, command-injection/secret-exfil tests, PWNED-PRODUCTION-RULE, and a rule impersonating a real id). These all carry source = https://semgrep.dev/r/None (no canonical registry source) in one contiguous r_id burst and never appear in semgrep/semgrep-rules. See RFC: Vendor Semgrep r/all rules locally with tracked exclusions #211 for the full provenance analysis. They are listed explicitly (rather than auto-excluded by source) so a legitimate rule that ever carried /r/None could not be silently dropped.
When an excluded rule is later removed upstream, rules-update.py reports it as an orphaned state on the console — a human then prunes the rules-status.toml entry or keeps it. Nothing auto-deletes. (new and orphaned are derived statelessly from the prior rules-vendored.yaml + rules-status.toml; no state file is stored.)

PR provenance (from git history): the exclusions carry their real originating PR — 21 from #11 (2026-02-07), 1 from #89 (OpenAI), 1 from #200 (bbp-pattern-inject, a broken rule that made semgrep exit 2), and 78 from this PR (the remaining probe rules).

Integration points:

mise.toml: add SEMGREP_RULES env (bare semgrep uses vendored rules with zero flags) + semgrep:update task; drop the 22 --exclude-rule flags.
CI (security-scanners.yml) + pre-commit hook: --config=tools/semgrep/rules-vendored.yaml; drop exclude flags. Baseline/SARIF logic unchanged. All three invocations aligned on --verbose --max-log-list-entries=0 (log verbosity only; see the flag-alignment comment below).
.semgrepignore: skip tools/semgrep/*.yaml (rules would self-match).
.gitleaks.toml: allowlist the active file (contains secret-detection regexes).
.pre-commit-config.yaml: exclude the generated files from content/whitespace/large-file hooks so autofix hooks never corrupt the byte-for-byte ruleset. rules-update.py/rules-status.toml stay in scope.
Remove the stub .semgrep.yaml (never loaded by Semgrep; misleading).

Hardening: GHAS code-scanning findings in rules-update.py (dynamic-urllib/CWE-939, string-concat-in-list) were remediated with real code changes — an HTTPS-only OpenerDirector that makes file:///ftp:// unreachable, and explicit string concatenation — not suppressions.

Result

3005 active / 126 excluded blocks (101 distinct excluded ids: 21 PR#11, 1 PR#89, 1 PR#200, 78 PR#220; the remaining excluded blocks are duplicate ids the r/all feed reuses). The real subprocess-shell-true rule survives while its /r/None impersonator is dropped, showing the exclusion list discriminates valid from invalid.

Verification

mise run build passes: lint (0 errors), fmt:check, validate, bandit ("No issues"), gitleaks (no leaks), checkov, grype. All CI checks green (24/24).
Vendored ruleset validated against Semgrep 1.167.0: parses, loads, and scans with no fatal errors (exit 0, 0 parse errors); excluded rule IDs confirmed absent; retained rule blocks confirmed byte-identical to the upstream download.
rules-update.py is deterministic: two consecutive runs produce byte-identical output.
Packed size ≈ 869 KB (matches the RFC's ~875 KB estimate).

Note: GitHub push protection flagged a Slack-webhook placeholder (T00000000/B00000000/XXX…) that is part of the detected-slack-webhook rule's own pattern-not matching logic — a false positive accepted via the repo's push-protection unblock. The ruleset was intentionally not modified, to keep it byte-for-byte faithful to upstream.

Merge

This branch is intended to be merged with "Squash and merge". The commit history is intentionally granular (it records the iterative design, including an [auto_exclude] approach that was later replaced with explicit listing); squashing collapses it into a single clean commit on main.

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.

Generated with Claude Code

Replace the PR#TBD placeholders in exclusions.toml and the generated EXCLUSIONS.md with this PR's number (#220). PR#89 (the OpenAI rule) is left as-is. Same-length swap; no table realignment or rule content change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Implements RFC #211. All Semgrep invocations (pre-commit, CI, mise) now run against a single pinned, locally-vendored ruleset instead of fetching r/all live from the registry on every scan. Exclusions are baked into the vendored file, and a generated doc records what was decided and why. Adds tools/semgrep/: - update.py stdlib-only update mechanism (goal C). Downloads r/all, diffs each rule's version_id against the prior snapshot to derive a "last changed" date, drops excluded rules, and regenerates the artifacts. Byte-preserving: it removes whole rule blocks and never re-serializes YAML (re-serializing was verified to silently break the ruleset). - exclusions.toml human-owned source of truth (id -> status/pr/reason), seeded from the 22 pre-existing --exclude-rule flags. - r-all.active.yaml pre-filtered rules; the only file scans load (goal A). - rule-state.json version_id/date baseline for diffing. - EXCLUSIONS.md generated tracking table (goal B). Integration: - mise.toml: SEMGREP_RULES env (bare `semgrep` uses vendored rules with zero flags) + `semgrep:update` task; drop 22 --exclude-rule flags. - CI + pre-commit: --config=tools/semgrep/r-all.active.yaml; drop exclude flags. Baseline/SARIF logic unchanged. - .semgrepignore: skip tools/semgrep/*.yaml (rules self-match). - .gitleaks.toml: allowlist the active file (contains secret regexes). - dprint.json: exclude generated EXCLUSIONS.md / rule-state.json. - .pre-commit-config.yaml: top-level `exclude` for the generated files (r-all.active.yaml, rule-state.json, EXCLUSIONS.md) so content hooks (detect-private-key, check-vcs-permalinks, check-added-large-files) skip them and autofix hooks never corrupt the byte-for-byte ruleset. The hand-authored update.py/exclusions.toml stay in scope. - Remove stub .semgrep.yaml (never loaded by Semgrep; misleading). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Replace the PR#TBD placeholders in exclusions.toml and the generated EXCLUSIONS.md with this PR's number (#220). PR#89 (the OpenAI rule) is left as-is. Same-length swap; no table realignment or rule content change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Rebased onto main, which added an exclusion for `bbp-pattern-inject` (PR #200) — a malformed community rule that fails to parse ("Invalid pattern for Python: Stdlib.Parsing.Parse_error") and makes semgrep exit 2 on every run. main handled it with a --exclude-rule flag; this expresses the same decision through the vendoring mechanism by adding it to exclusions.toml, so update.py physically drops it from r-all.active.yaml. Regenerated the active file, rule-state.json, and EXCLUSIONS.md (23 excluded / 3108 active). Verified: semgrep now exits 0 with no parse errors against the vendored ruleset. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The r/all feed carries 104 rules with source=https://semgrep.dev/r/None: a single-burst set of offensive-security probes (SSRF via rule metadata targeting 169.254.169.254, stored-XSS payloads in message/fix fields, command-injection and secret-exfil tests, plus PWNED-PRODUCTION-RULE and a rule impersonating a real id). None have a canonical registry source; none appear in semgrep/semgrep-rules. See PR #220 provenance analysis. Rather than list 104 ids by hand, add a source-based auto-exclusion policy so this generalizes to FUTURE probes: - exclusions.toml gains [auto_exclude]: any rule whose `source` matches a listed value is dropped automatically. A rule with no canonical source (/r/None) is not a curated community rule — this is how new r/all rules are judged (no positive "valid" signal exists; absence of a real source is a reliable invalidity signal). An explicit [rules."id"] status=active force-keeps a rule the policy would drop. - update.py parses each rule's `source`, applies the policy, reports "N auto-excluded", and lists them in EXCLUSIONS.md as `auto-excluded`. - Harden the doc generator: sanitize untrusted rule messages (escape <>`| ) so XSS/markup payloads in rule text render as inert text instead of injecting markup or breaking the table (this is what test-bbp-xss-msg's "<img src=x onerror=...>" message was probing). Result: 3005 active / 126 excluded. The real subprocess-shell-true rule survives (the /None impersonator is dropped), semgrep exits 0 with no parse errors, and the Semgrep OSS code-scanning check clears (bbp-rate-check no longer flags print() calls). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

dprint already leaves rule-state.json untouched — update.py emits json.dumps(indent=2, sort_keys=True), which matches dprint's JSON style (verified: `dprint check` exits 0 with no diff). The exclude was belt-and-suspenders and, since dprint.json is strict JSON with no comment syntax, it read as accidental. Remove it and document the generated-file formatting policy in update.py's docstring instead, where comments are allowed. EXCLUSIONS.md stays dprint-excluded (dprint would reflow its tables and churn on every regeneration); rule-state.json stays in the pre-commit exclude (large-file/private-key hooks would still act on it). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

update.py's render_table now column-pads the EXCLUSIONS.md markdown table exactly as the dprint markdown plugin does (each cell ljust to its column's max code-point width, delimiter dashes to match, min 3). Verified: with the exclude removed, `dprint check tools/semgrep/EXCLUSIONS.md` exits 0 with no diff, so the generated file already matches what dprint would produce. Removes the last tools/semgrep entry from dprint.json's excludes — both generated files (EXCLUSIONS.md, rule-state.json) are now emitted in dprint's own format, so neither needs a formatter exclude. They remain in the pre-commit exclude only for content hooks (large-file/private-key/whitespace). Trade-off: render_table mirrors dprint's table algorithm, so a future markdown-plugin bump or config change could require a matching update.py tweak; the build's fmt:check catches any such drift immediately. Cells are already sanitized ASCII, so len()-based widths match dprint's. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The Semgrep registry serves r/all in a nondeterministic block order, so re-running update.py reshuffled r-all.active.yaml and produced a spurious ~46k-line diff even when no rule actually changed (verified: same ids, same version_ids, 0 added/removed/changed — order only). That would make every future `mise run semgrep:update` unreviewable. Sort active rule blocks by rule id before writing. Now output is a pure function of content: two consecutive runs are byte-identical, and a real upstream change is the only thing that shows up in git — matching the RFC's "reviewable drift" goal (rule-state.json is already sort_keys=True). Verified: 3005 rules intact, semgrep scans the sorted file with exit 0 and no parse errors, build green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Address four Semgrep code-scanning findings with real code changes, not # nosec/# noqa suppressions (all suppression comments removed). CWE-939 / dynamic-urllib-use-detected (urlopen with non-literal URL): the default urllib opener installs file:// and ftp:// handlers, so a URL resolving to another scheme could read local files. Replace urllib.request.urlopen with an OpenerDirector that installs ONLY the HTTPS/redirect/error handlers — no FileHandler/FTPHandler — so non-HTTPS schemes have no handler and are unreachable by construction, plus an explicit https scheme guard. Verified: file:// and ftp:// return no content; https download of r/all still succeeds byte-for-byte. string-concat-in-list (implicitly concatenated string literals in the render_doc lines list): make the concatenation explicit with `+` so each list element is unambiguously one string (guards against a missing-comma bug the rule warns about). Verified with the exact flagged rules: dynamic-urllib 1->0 findings, string-concat-in-list 0 findings, bandit "No issues" with 0 nosec skips, update.py still produces byte-identical output (3005 active rules). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Replaces the [auto_exclude] source policy with explicit per-rule entries, per review feedback. Three concerns addressed: 1. No silent exclusion. source=/r/None is a reliable junk signal *today* (all 104 probes lack the `license:` field every real rule carries), but it is not guaranteed — a future legitimate rule could carry /r/None and would have been auto-dropped. Now every exclusion is listed explicitly in exclusions.toml; nothing is dropped implicitly, so a new upstream rule can never silently disappear — it surfaces as `new` for triage. The 79 probe rules (78 + bbp-pattern-inject) are now explicit entries. 2. Correct PR provenance (from git pickaxe, replacing the earlier PR#TBD->220 guess): the 21 original exclusions were PR #11 (2026-02-07), OpenAI was PR #89, bbp-pattern-inject was PR #200. Only the 78 new probe exclusions are PR #220. 3. Duplicate rows fixed. r/all reuses some ids across blocks (bbp-x x11, bbp-ssrf x8, ...); EXCLUSIONS.md now collapses rows by id. Status legend documents excluded/active/new (auto-excluded removed). Also: when an excluded rule is later removed upstream, update.py now surfaces the orphaned entry in a "Removed upstream (safe to prune)" doc section (and console), rather than only a console warning — a human prunes it, nothing auto-deletes. Result: 101 explicit entries (21 PR#11, 1 PR#89, 1 PR#200, 78 PR#220), 3005 active rules, semgrep exit 0 with no parse errors, build green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…ions Make the three semgrep entry points consistent on the cosmetic log flags (these affect only stderr verbosity — never findings or exit code): - mise `security:semgrep`: swap `--quiet` for `--verbose` (they are mutually exclusive in semgrep, so --quiet had to go). Keeps --max-log-list-entries=0, which was previously a no-op under --quiet. - pre-commit hook: add `--max-log-list-entries=0` (it was --verbose without the flag, so it printed "<SKIPPED DATA>" instead of the full rule list). - CI: unchanged (already --verbose --max-log-list-entries=0). `--max-log-list-entries=0` disables truncation → shows ALL rule-ids/skipped files (verified empirically: =2 prints "<SKIPPED DATA>", =0 prints the full list). Intentional per-invocation differences remain: --baseline-commit (local mise scans everything; CI/pre-commit diff a baseline) and --sarif-output/--error (CI/pre-commit only). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

scottschreckengaust · 2026-07-07T16:10:45Z

Aligned semgrep flags across the three invocations (`b50735b`)

Coordinated the cosmetic log/verbosity flags so all three semgrep entry points behave consistently. These flags affect stderr verbosity only — never findings or exit code (verified empirically: --max-log-list-entries=2 prints <SKIPPED DATA (too many entries…)>, =0 prints the full rule list, i.e. 0 = disable truncation, show all).

Before → after

Flag	mise `security:semgrep`	CI `security-scanners.yml`	pre-commit hook
verbosity	`--quiet` → `--verbose`	`--verbose` (unchanged)	`--verbose` (unchanged)
`--max-log-list-entries=0`	✅ (was a no-op under `--quiet`)	✅ (unchanged)	❌ → ✅ added

Net: all three now use --verbose --max-log-list-entries=0.

Notes / gotchas

--quiet and --verbose are mutually exclusive in semgrep (it exits with an error), so adding --verbose to the mise task required dropping --quiet. Confirmed the swap changes neither exit code nor findings — only output volume. mise run security:semgrep is now verbose locally.
Under the old --quiet, mise's --max-log-list-entries=0 did nothing (quiet already suppresses those lists); it's now meaningful.
pre-commit was --verbose without the flag, so with the ~3000-rule vendored config it printed <SKIPPED DATA> for the rule list instead of the full list — now consistent with CI.

Intentional differences that remain (by design, not inconsistencies)

--baseline-commit — mise runs a full local scan; CI ($BASELINE_SHA on PRs) and pre-commit (main) diff against a baseline so only new findings block.
--sarif-output / --error — only CI and pre-commit emit SARIF / gate on findings; the mise task is interactive.

The [auto_exclude] source policy was removed earlier, leaving parse_block extracting a `source` value that main() unpacked and discarded (_source), plus a now-orphaned SOURCE_RE whose comment referenced the deleted policy. Drop SOURCE_RE and return a 3-tuple from parse_block — pure behavior- preserving refactor; generated files are unaffected (only update.py changes). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

… orphaned state Coordinate the tools/semgrep/ filenames under a rules-* prefix and simplify to the files that actually carry state: - r-all.active.yaml -> rules-vendored.yaml (what semgrep runs) - exclusions.toml -> rules-status.toml (human decisions: excluded/active) - update.py -> rules-update.py (the updater) - rule-state.json -> DELETED (see below) - EXCLUSIONS.md -> DELETED (see below) Why the two deletions: - EXCLUSIONS.md was a rendered join of the toml (decisions) + rule-state.json (dates) + snapshot (descriptions). Its only non-derivable column was the description gloss; the rest duplicated rules-status.toml. It's redundant with the source files, so it's dropped. - rule-state.json stored {version_id, updated} per rule. `updated` existed only to fill EXCLUSIONS.md's rule-updated column; `version_id` existed only to compute `updated`. With the doc gone, nothing consumes either field — so the file has no reason to exist ("no version or updated" left). Deleted. Decision model is now two stored statuses (excluded/active) in rules-status.toml plus two DERIVED states reported each run for human action: - new — a feed rule with no decision, not seen last run (triage it). - orphaned — a rules-status.toml entry whose rule left the feed (prune or keep). Both are derived statelessly from artifacts already in git (the prior rules-vendored.yaml + rules-status.toml reconstruct "ids seen last run"), so no state file is needed. Nothing is auto-deleted; the human decides. rules-update.py loses the json/date/render_doc/render_table/sanitize machinery (~130 lines) and only reads rules-status.toml, so hand-written TOML comments are preserved. Verified: rules-vendored.yaml rule bodies are byte-identical to the old r-all.active.yaml (3005 active / 126 excluded); regeneration is deterministic; new/orphaned detection works; build green. Updated all references: mise.toml (SEMGREP_RULES, security:semgrep, semgrep:update), CI security-scanners.yml, .pre-commit-config.yaml (config path + exclude regex), .gitleaks.toml. .semgrepignore's tools/semgrep/*.yaml glob still covers the renamed vendored file. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…ry scan gitleaks scans full git history. Renaming r-all.active.yaml -> rules-vendored.yaml left the old-named blobs in earlier branch commits (c8f5403, b189d87) no longer matched by the allowlist, so gitleaks flagged their embedded secret-detection regexes (slack-webhook-url, private-key) — failing both the standalone `gitleaks` check and the `security:gitleaks` step of `mise run build`. Allowlist BOTH the current and former filename so the branch history scans clean. (Post-squash-merge, main only ever has the new name, but CI scans the branch history, which still contains the old name.) Verified: gitleaks reports "no leaks found" over full history. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

scottschreckengaust and others added 3 commits July 6, 2026 23:33

scottschreckengaust force-pushed the feat/vendor-semgrep-rules branch from 05b878c to 2ca2848 Compare July 6, 2026 23:40

github-advanced-security AI found potential problems Jul 6, 2026

View reviewed changes

scottschreckengaust and others added 4 commits July 7, 2026 00:55

scottschreckengaust marked this pull request as ready for review July 7, 2026 01:47

scottschreckengaust requested review from a team, krokoko and theagenticguy July 7, 2026 01:47

scottschreckengaust requested a review from a team as a code owner July 7, 2026 01:47

scottschreckengaust mentioned this pull request Jul 7, 2026

RFC: Vendor Semgrep r/all rules locally with tracked exclusions #211

Open

10 tasks

scottschreckengaust and others added 3 commits July 7, 2026 02:00

scottschreckengaust and others added 3 commits July 7, 2026 16:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(security): vendor Semgrep r/all ruleset with tracked exclusions#220

feat(security): vendor Semgrep r/all ruleset with tracked exclusions#220
scottschreckengaust wants to merge 13 commits into
mainfrom
feat/vendor-semgrep-rules

scottschreckengaust commented Jul 6, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

scottschreckengaust commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

scottschreckengaust commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related

Changes

Result

Verification

Merge

Acknowledgment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

scottschreckengaust commented Jul 7, 2026

Aligned semgrep flags across the three invocations (b50735b)

Before → after

Notes / gotchas

Intentional differences that remain (by design, not inconsistencies)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

scottschreckengaust commented Jul 6, 2026 •

edited

Loading

Aligned semgrep flags across the three invocations (`b50735b`)