feat: exclude generic OpenAI rule for semgrep - #89
Merged
Conversation
Signed-off-by: Scott Schreckengaust <345885+scottschreckengaust@users.noreply.github.com>
scottschreckengaust
enabled auto-merge
March 11, 2026 19:19
krokoko
approved these changes
Mar 11, 2026
icarthick
added a commit
to icarthick/agent-plugins
that referenced
this pull request
Mar 30, 2026
* upstream/main: docs: Add Japanese README (awslabs#106) feat(databases-on-aws): add initial Aurora DSQL skill for Databases on AWS Plugin pt. 1 (awslabs#66) chore(deps): update github-actions: Bump jdx/mise-action (awslabs#96) feat(aws-serverless): Add Amazon API gateway skill (awslabs#94) chore(deps): update github-actions: Bump actions/download-artifact (awslabs#93) feat(aws-amplify): add AWS Amplify Gen 2 plugin (awslabs#18) fix: formatting in CODEOWNERS for amazon-location-service (awslabs#90) feat: exlcude generic OpenAI rule for semgrep (awslabs#89) feat(tools): add reference integrity, size validation, and skill scaffolding (awslabs#87) docs: add aws-serverless plugin documentation to README (awslabs#86) feat(migration-to-aws): Migration plugin (awslabs#73) # Conflicts: # .claude-plugin/marketplace.json # .github/CODEOWNERS # AGENTS.md # README.md
scottschreckengaust
added a commit
that referenced
this pull request
Jul 7, 2026
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>
This was referenced Jul 7, 2026
Morlej
pushed a commit
to Morlej/agent-plugins
that referenced
this pull request
Jul 8, 2026
…wslabs#220) * feat(security): vendor Semgrep r/all ruleset with tracked exclusions Implements RFC awslabs#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> * chore(security): record PR#220 in Semgrep exclusions tracking Replace the PR#TBD placeholders in exclusions.toml and the generated EXCLUSIONS.md with this PR's number (awslabs#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> * fix(security): exclude broken r/all rule bbp-pattern-inject Rebased onto main, which added an exclusion for `bbp-pattern-inject` (PR awslabs#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> * feat(security): auto-exclude bug-bounty probe rules from r/all by source 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 awslabs#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> * refactor(security): drop redundant dprint exclude for rule-state.json 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> * refactor(security): emit dprint-aligned tables, drop last dprint exclude 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> * fix(security): sort vendored rules by id for deterministic output 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> * fix(security): remediate GHAS findings in update.py (no suppressions) 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> * refactor(security): list all exclusions explicitly; fix PRs; dedup doc 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 awslabs#11 (2026-02-07), OpenAI was PR awslabs#89, bbp-pattern-inject was PR awslabs#200. Only the 78 new probe exclusions are PR awslabs#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> * chore(security): align semgrep verbosity/log flags across all invocations 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> * refactor(security): remove dead source parsing from update.py 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> * refactor(security): rename to rules-* prefix; drop derived files; add 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> * fix(security): allowlist former vendored-file name for gitleaks history 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> * docs(security): add tools/semgrep/README.md Add a short directory README so the vendored-ruleset setup is legible in the GitHub folder view without opening the files: what each file is (and which is human-edited vs generated), how to refresh it (`mise run semgrep:update`), and a glossary of the excluded/active/new/orphaned states. Deliberately carries no volatile figures (rule counts, ids) so it can't go stale; the rules-update.py docstring remains the authoritative design reference. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(security): address PR review nits on vendored-semgrep tooling Three minor, non-blocking review items: - rules-status.toml header named the old files (exclusions.toml, r-all.active.yaml, EXCLUSIONS.md, update.py). It's the file humans edit, so fix the comments to name the files that actually exist (rules-status.toml, rules-vendored.yaml, rules-update.py) and drop the removed EXCLUSIONS.md references — new/orphaned are reported on the console by rules-update.py. - Make the deterministic sort total: tie-break active blocks on their text (rb[0], rb[1]) so duplicate ids (r/all reuses some) also order stably instead of falling back to the download's nondeterministic order. Latent today (0 active dups — the dups are all excluded); byte-identical output verified, so no change to the vendored file now. - Document split_rules' assumption that rule content never begins a column-0 "- " line; if it ever did, the mis-split block fails rule_id_of and aborts loudly rather than silently corrupting the ruleset. (The reviewer's third note — security:semgrep --quiet -> --verbose — was the intentional flag alignment; no change needed.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Sphia Sadek <isadeks@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
References to generic OpenAI is expected.
Related
Changes
Adding
--exclude-rule='ai.generic.detect-generic-ai-oai.detect-generic-ai-oai'to semgrepAcknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.