refactor: remove rules/ - detection rules are implementation artifacts#54
Merged
Merged
Conversation
All 59 files in rules/pattern/ carry a bawbel-namespaced rule_id,
implementation-specific detection code living in a repo whose entire
purpose is to be implementation-neutral. ARCHITECTURE.md's own diagram
confirms this: rules/pattern, rules/yara, rules/semgrep loaded directly
into PatternEngine, YARAEngine, SemgrepEngine, named components of one
specific scanner.
Investigated moving these into bawbel/scanner per the original plan,
but that repo already has its own independently-authored, more complete
detection logic (PATTERN_RULES embedded directly in pattern_engine.py,
covering 40 of 59 AVE ids in a different schema entirely; consolidated
yara/semgrep rule files covering 39 and 37). Its own 607-test suite
passes independent of anything in this repo. Copying ave's rules/pattern/
*.py files in verbatim would create dead, unwired files in the wrong
format rather than actually integrating with how that engine works, so
no scanner-side PR was opened -- rules/ is removed here without a
corresponding deposit anywhere, per fallback instruction.
rules/semgrep/ and rules/yara/ were empty scaffolding (README only, no
actual rule files); their READMEs were already generically worded
("AVE records," pointing to CONTRIBUTING.md), not bawbel-scanner-specific.
Companion changes required to keep this repo working without rules/:
- scripts/check_fixtures.py rewritten to key off records/ directly,
no longer off rules/pattern/ file contents
- scripts/check_rule_coverage.py removed (nothing left to check)
- scripts/generate-rules-and-fixtures.js removed (one-time generator
for rule files that no longer belong in this repo)
- tests/test_fixtures.py rewritten: no local rule modules to load and
run matches() against, so it now verifies fixture pairs exist, are
non-empty, and match a real record -- the conformance-fixture model,
not a detection-logic test
- .github/workflows/tests.yml: dropped the check_rule_coverage.py step
and the --cov=rules coverage gate (nothing left to measure coverage
of); pyproject.toml's [tool.coverage.run] source=["rules"] removed
- .github/workflows/notify-ave-site.yml: dropped the rules/** path
trigger. Also fixed while in this file: it still dispatched to
bawbel/ave-site and referenced bawbel/ave in its own comments, a
live automation bug left over from the org move, not just a doc issue
- CLAUDE.md: hard rule 4, "Adding a record," and the Local commands
block updated -- a record requires fixtures, not a local rule; rule
authorship happens via a coordinated PR in the implementing tool's
own repo (CONTRIBUTING.md Step 4 already said this; CLAUDE.md and
add-ave-record/SKILL.md contradicted it until now)
- .claude/skills/add-ave-record/SKILL.md: step 4 no longer says "write
the detection rule in rules/pattern/"; step 5 now points at the
coordinated cross-repo PR
- .claude/skills/handoff/SKILL.md: the template example line
referenced a rules/semgrep/ file that this workflow no longer
produces
- LANGUAGE.md: "Rule" definition no longer names rules/pattern/yara/
semgrep as where rules live in this repo (a direct consequence of
this deletion; LANGUAGE.md's other findings -- bawbel CLI names in
its detection-layer table, stale record counts -- are tracked
separately as Task D4, not folded in here)
All 59 records still validate; full fixture-conformance suite (236
tests) passes.
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
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.
Summary
All 59 files in
rules/pattern/carried abawbel-namespacedrule_id-- implementation-specific detection code living in a repo whose entire purpose is to be implementation-neutral.ARCHITECTURE.md's own diagram already documented this:rules/pattern,rules/yara,rules/semgreploading directly intoPatternEngine,YARAEngine,SemgrepEngine, named components of one specific scanner.Investigated moving these into
bawbel/scannerper the original plan, decided against it. That repo already has its own independently-authored, more complete detection logic:PATTERN_RULESembedded directly inpattern_engine.py(covering 40 of 59 AVE ids, in a completely different schema -- dicts with title/severity/aivss_score/owasp fields, not theRULE+matches()shape this repo's files used), plus consolidatedyara/semgreprule files covering 39 and 37 ids respectively. Its own 607-test suite passes independent of anything in this repo. Copyingrules/pattern/*.pyin verbatim would have created dead, unwired files in the wrong format rather than actually integrating with how that engine works -- so no scanner-side PR was opened.rules/is removed here without a corresponding deposit anywhere.rules/semgrep/andrules/yara/were empty scaffolding (README only, no actual rule files); their READMEs were already generically worded ("AVE records," pointing toCONTRIBUTING.md), notbawbel-scanner-specific.Companion changes (required, not optional -- CI would otherwise break)
scripts/check_fixtures.pyrewritten to key offrecords/directly, notrules/pattern/file contentsscripts/check_rule_coverage.pyremoved (nothing left to check)scripts/generate-rules-and-fixtures.jsremoved (one-time generator for rule files that no longer belong in this repo)tests/test_fixtures.pyrewritten: no local rule modules to load and runmatches()against, so it now verifies fixture pairs exist, are non-empty, and match a real record -- the conformance-fixture model, not a detection-logic test.github/workflows/tests.yml: dropped thecheck_rule_coverage.pystep and the--cov=rulescoverage gate;pyproject.toml's[tool.coverage.run] source = ["rules"]removed.github/workflows/notify-ave-site.yml: dropped therules/**path trigger. Also fixed while in this file: it still dispatched tobawbel/ave-siteand referencedbawbel/avein its own comments -- a live automation bug left over from the org move, not just a doc issueCLAUDE.md: hard rule 4, "Adding a record," and the Local commands block updated -- a record requires fixtures, not a local rule; rule authorship happens via a coordinated PR in the implementing tool's own repo (CONTRIBUTING.mdStep 4 already said this;CLAUDE.mdandadd-ave-record/SKILL.mdcontradicted it until now).claude/skills/add-ave-record/SKILL.md: step 4 no longer says "write the detection rule in rules/pattern/"; step 5 now points at the coordinated cross-repo PR.claude/skills/handoff/SKILL.md: the template example line referenced arules/semgrep/file this workflow no longer producesLANGUAGE.md: "Rule" definition no longer namesrules/pattern/yara/semgrepas where rules live in this repo (a direct consequence of this deletion;LANGUAGE.md's other findings --bawbelCLI names in its detection-layer table, stale record counts -- are tracked separately as Task D4, not folded in here)Test plan
python3 scripts/validate_records.py-- all 59 records still validpython3 scripts/check_fixtures.py-- all 59 records have positive/negative fixturespython3 -m pytest tests/ -x -q-- 236 passedbawbel/scanner's own test suite (607 tests) passes independent of this repo