Skip to content

feat(scanner): evaluate() contract foundation - fallible inventory, shared helpers, single engine path #369

Description

@parthrohit22

What problem does this solve?

scanner/evaluation.py defines the per-resource coverage contract from #263: a rule exposes evaluate() and reports PASS / FAIL / UNKNOWN / ERROR / NOT_APPLICABLE for every resource it inspected. Only AZ-KV-006 implements it today. The engine records the other 143 rules as UNKNOWN / LEGACY_RULE_NOT_MIGRATED, and because compliance reports became evaluation-derived in #310, most mapped controls in the CIS, NIST CSF, ISO 27001 and SOC 2 reports read UNKNOWN.

Before rules can be migrated at scale, three gaps in the foundation need closing:

  1. Inventory failure is invisible. AzureClient.get_storage_accounts() and get_key_vaults() return [] both when the subscription has no resources and when the list call fails (permissions, throttling, network). evaluate() cannot tell these apart, so AZ-KV-006 reports a permissions failure as NOT_APPLICABLE instead of ERROR.
  2. No shared helpers. Every migrated rule would otherwise hand-build the same subscription-scope ERROR / NOT_APPLICABLE evaluations and reason codes.
  3. Two code paths per rule. The engine calls both scan() and evaluate() on a migrated rule. That doubles the ARM list calls and lets the two paths drift apart.

Describe the solution

scanner/azure_client.py

  • Add list_storage_accounts() and list_key_vaults() returning Optional[List], where None means the call failed. This matches the existing convention in get_route_tables(), get_managed_clusters(), get_cosmos_accounts() and others.
  • Keep get_storage_accounts() / get_key_vaults() as backward-compatible wrappers (return self.list_...() or []) so no legacy scan() changes behaviour.

scanner/evaluation.py

  • Add inventory_unavailable(rule_id, resource_type, subscription_id), which returns an ERROR evaluation with INVENTORY_UNAVAILABLE at the subscription scope.
  • Add no_resources_found(rule_id, resource_type, subscription_id), which returns a NOT_APPLICABLE evaluation with NO_RESOURCES_FOUND.
  • Document the standard reason codes used by family migrations (EVIDENCE_UNAVAILABLE, MISSING_PROPERTIES, POLICY_NOT_REQUIRED, APPROVED_EXCEPTION).

scanner/engine.py

  • When a rule exposes evaluate(), take its findings from the FAIL evaluations and do not call scan().
  • Legacy rules without evaluate() keep the current scan() path and the UNKNOWN / LEGACY_RULE_NOT_MIGRATED placeholder.

scanner/rules/az_kv_006.py

  • Use list_key_vaults(). A failed call returns inventory_unavailable(...) and an empty list returns no_resources_found(...).
  • Make scan() a thin wrapper over the FAIL evaluations.

Tests

  • Add tests/test_rule_evaluation_contract.py, parametrized over every rule module that exposes evaluate():
    • an inventory failure yields ERROR, never PASS or NOT_APPLICABLE;
    • an empty inventory yields NOT_APPLICABLE;
    • every FAIL carries a finding with the module's RULE_ID, SEVERITY and FRAMEWORKS;
    • scan() returns exactly the FAIL findings.
  • Extend tests/helpers/mock_azure.py so storage-account and Key Vault inventories can be set to None (failure).
  • Update the engine tests in tests/test_rule_evaluations.py for the "evaluate() supersedes scan()" path.

Docs

Alternatives considered

  • Change get_storage_accounts() / get_key_vaults() to return None directly. Rejected: every legacy scan() that iterates the result would raise TypeError until it is migrated.
  • Keep calling both scan() and evaluate() and rely on a parity test. Rejected as the long-term design because it doubles ARM calls per migrated rule. The parity test is still added as a safety net.

Additional context

Activity

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

Metadata

Metadata

Assignees

Labels

coreCore team ownership not for studentsenhancementNew feature or requestpriority: highImportant, should be fixed in the current sprintpythonPull requests that update python coderoadmapPlanned feature track, not a current bug

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions