Skip to content

bug: scanner engine loads rule modules that CI validation and doc counts never see #244

Description

@parthrohit22

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

  1. Add scanner/rules/scratch_test.py containing a scan() function and no RULE_ID.
  2. Run a scan — load_rules() imports and executes it.
  3. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingcoreCore team ownership not for students

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions