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:68load_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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingcoreCore team ownership not for students

    Type

    No type

    Projects

    Status
    📋 Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions