What happened?
Three places in the repo discover scanner rule modules, and they do not agree on what counts as a rule.
| Location |
Discovery pattern |
scanner/engine.py:68 — load_rules(), executes rules at scan time |
RULES_DIR.glob("*.py"), skipping names starting with _ |
.github/workflows/ci.yml:68 and :207 — structure validation |
scanner/rules/az_*.py |
.github/scripts/update_learn_page.py — doc statistics (added in #230) |
az_*.py with a RULE_ID |
The engine is the loosest of the three. Any .py file in scanner/rules/ that does not start with
_ is imported and executed, whether or not it is named az_*.py and whether or not it declares a
RULE_ID.
All three currently report 66 because scanner/rules/ happens to contain nothing outside the
az_*.py and _*_common.py conventions. Nothing enforces that.
The practical consequence: a leftover, experimental or misnamed .py file with a scan() function
would be loaded and run against a real subscription in production, while CI's structure validation
would never check it and the Learn page and README would never count it. A rule can execute without
being validated.
What did you expect?
The code that executes rules and the code that validates and counts them should agree on what a rule
is, so that a file cannot run in production without also being checked by CI.
Steps to reproduce
- Add
scanner/rules/scratch_test.py containing a scan() function and no RULE_ID.
- Run a scan —
load_rules() imports and executes it.
- Run the CI structure-validation job and
.github/scripts/update_learn_page.py — neither sees the file.
Environment
- OpenShield version:
dev @ f1a7e45
- Python version: 3.11
- OS: n/a
Logs or screenshots
# scanner/engine.py:68
for rule_path in sorted(RULES_DIR.glob("*.py")):
# .github/workflows/ci.yml:68
files=(scanner/rules/az_*.py)
Suggested fix
Narrow load_rules() to az_*.py and require a RULE_ID, matching what CI already enforces, so the
engine cannot execute anything CI has not validated. If loading non-conforming files is intentional,
the CI job and the stats script should be widened to match instead — but the three should not
disagree silently.
Adding a CI guard that fails when a .py file in scanner/rules/ matches neither az_*.py nor
_*_common.py would stop the two conventions drifting apart again.
Noted while reviewing #230; flagged there as out of scope for that PR.
What happened?
Three places in the repo discover scanner rule modules, and they do not agree on what counts as a rule.
scanner/engine.py:68—load_rules(), executes rules at scan timeRULES_DIR.glob("*.py"), skipping names starting with_.github/workflows/ci.yml:68and:207— structure validationscanner/rules/az_*.py.github/scripts/update_learn_page.py— doc statistics (added in #230)az_*.pywith aRULE_IDThe engine is the loosest of the three. Any
.pyfile inscanner/rules/that does not start with_is imported and executed, whether or not it is namedaz_*.pyand whether or not it declares aRULE_ID.All three currently report 66 because
scanner/rules/happens to contain nothing outside theaz_*.pyand_*_common.pyconventions. Nothing enforces that.The practical consequence: a leftover, experimental or misnamed
.pyfile with ascan()functionwould be loaded and run against a real subscription in production, while CI's structure validation
would never check it and the Learn page and README would never count it. A rule can execute without
being validated.
What did you expect?
The code that executes rules and the code that validates and counts them should agree on what a rule
is, so that a file cannot run in production without also being checked by CI.
Steps to reproduce
scanner/rules/scratch_test.pycontaining ascan()function and noRULE_ID.load_rules()imports and executes it..github/scripts/update_learn_page.py— neither sees the file.Environment
dev@ f1a7e45Logs or screenshots
Suggested fix
Narrow
load_rules()toaz_*.pyand require aRULE_ID, matching what CI already enforces, so theengine cannot execute anything CI has not validated. If loading non-conforming files is intentional,
the CI job and the stats script should be widened to match instead — but the three should not
disagree silently.
Adding a CI guard that fails when a
.pyfile inscanner/rules/matches neitheraz_*.pynor_*_common.pywould stop the two conventions drifting apart again.Noted while reviewing #230; flagged there as out of scope for that PR.