diff --git a/src/vpcopilot/apply.py b/src/vpcopilot/apply.py index eb9a354..eee2a32 100644 --- a/src/vpcopilot/apply.py +++ b/src/vpcopilot/apply.py @@ -103,6 +103,16 @@ def normalize_service_policy_spec(spec: dict) -> dict: return spec +def artifact_spec(spec: dict) -> dict: + """Unwrap a generated artifact to the bare XC spec. + + Generated artifacts vary: some are a full create body `{metadata, spec}`, some a bare spec — + `apply_from_scan` already handles both. The linter has to agree, or a perfectly good policy is + reported as "no DENY rule" purely because `rule_list` sits one level further down.""" + inner = spec.get("spec") if isinstance(spec, dict) else None + return inner if isinstance(inner, dict) and inner else spec + + def lint_service_policy(spec: dict, exploit: dict | None) -> list[str]: """Deterministic pre-apply lint — catch a service_policy that won't actually block the exploit, BEFORE any live LB round-trip. Under FIRST_MATCH the FIRST rule whose path+method match the @@ -110,6 +120,7 @@ def lint_service_policy(spec: dict, exploit: dict | None) -> list[str]: guess so an allow-all catches the exploit first), the exploit sails through. Returns issue strings ([] = looks correct).""" import re + spec = artifact_spec(spec) # accept a {metadata, spec} artifact, exactly as apply_from_scan does rules = (spec.get("rule_list") or {}).get("rules") or [] if not any((r.get("spec") or {}).get("action") == "DENY" for r in rules): return ["no DENY rule"] diff --git a/tests/test_lint.py b/tests/test_lint.py index 5195a1a..13310ed 100644 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -60,3 +60,34 @@ def test_lint_generated_spec_dispatches_by_control(): assert lint_generated_spec("service_policy", sp, EXPLOIT) == [] assert lint_generated_spec("api_schema", {"paths": {}}, None) # caught assert lint_generated_spec("rate_limit", {"anything": 1}, None) == [] # parameterized -> advisory + + +# ---- A3: the linter must read the same shape apply_from_scan consumes ---- +def test_lint_unwraps_a_metadata_spec_artifact(): + """generate emits either a bare spec or a full XC create body. apply_from_scan unwraps + {metadata, spec}; when the linter did not, every wrapped policy was reported 'no DENY rule' + even though it had one — a false positive on a perfectly good band-aid.""" + inner = _policy([ + {"action": "DENY", "path": {"prefix_values": ["/users/v1/register"]}, "http_method": {"methods": ["POST"]}}, + {"action": "ALLOW", "path": {"prefix_values": ["/"]}}]) + wrapped = {"metadata": {"name": "deny-register"}, "spec": inner} + assert lint_service_policy(wrapped, EXPLOIT) == [] + assert lint_generated_spec("service_policy", wrapped, EXPLOIT) == [] + + +def test_lint_still_catches_a_wrapped_policy_with_no_deny(): + wrapped = {"metadata": {"name": "x"}, "spec": _policy([{"action": "ALLOW", "path": {"prefix_values": ["/"]}}])} + assert lint_service_policy(wrapped, EXPLOIT) == ["no DENY rule"] + + +def test_lint_still_catches_a_wrapped_allow_before_deny(): + wrapped = {"metadata": {"name": "x"}, "spec": _policy([ + {"action": "ALLOW", "path": {"prefix_values": ["/users/v1/register"]}, "http_method": {"methods": ["POST"]}}, + {"action": "DENY", "path": {"prefix_values": ["/users/v1/register"]}, "http_method": {"methods": ["POST"]}}])} + assert lint_service_policy(wrapped, EXPLOIT) + + +def test_lint_leaves_a_bare_spec_alone(): + bare = _policy([{"action": "DENY", "path": {"prefix_values": ["/users/v1/register"]}, + "http_method": {"methods": ["POST"]}}]) + assert lint_service_policy(bare, EXPLOIT) == []