diff --git a/scripts/ci/pingora_edge_policy.py b/scripts/ci/pingora_edge_policy.py index 06694fcbca..bbdef500d0 100644 --- a/scripts/ci/pingora_edge_policy.py +++ b/scripts/ci/pingora_edge_policy.py @@ -176,16 +176,11 @@ def _is_documentation_or_source_fixture(path: str) -> bool: (one GitHub *can* diff, meaning it could carry inspectable content) is never exempted here. - ``tests/test_pingora_edge_policy.py`` is exempted the same way this - module's own source is: a scanner's regression suite necessarily - contains the denied Nginx runtime forms it verifies detection of as - fixture strings, so a PR whose diff to that file happens to add a line - matching a ``CONTENT_RULES`` pattern (triggering `_needs_content_scan`'s - "nginx" in the patch heuristic) does not then get the file's *entire* - content -- full of intentional denied forms throughout -- scanned and - rejected. A ``.py`` test file cannot itself be deployed as an active - Nginx runtime artifact, unlike the config/Dockerfile/service forms this - policy actually guards against. + Only the trusted scanner source itself and dedicated inert samples under + ``tests/fixtures`` receive a source-fixture exemption. Executable test + modules remain runtime candidates under the binding policy; regression + samples that contain denied forms must live in the dedicated fixture + boundary rather than exempting the whole test module. """ pure = PurePosixPath(path) @@ -194,10 +189,7 @@ def _is_documentation_or_source_fixture(path: str) -> bool: _is_known_documentation_path(pure) and pure.suffix.lower() in DOCUMENT_SUFFIXES ): return True - if pure.as_posix() in ( - "scripts/ci/pingora_edge_policy.py", - "tests/test_pingora_edge_policy.py", - ): + if pure.as_posix() == "scripts/ci/pingora_edge_policy.py": return True lower_parts = tuple(part.lower() for part in pure.parts) is_tests_fixture = len(lower_parts) >= 2 and lower_parts[:2] == ("tests", "fixtures") diff --git a/tests/test_pingora_edge_policy.py b/tests/test_pingora_edge_policy.py index 70bb1bc970..c6ce28dccc 100644 --- a/tests/test_pingora_edge_policy.py +++ b/tests/test_pingora_edge_policy.py @@ -63,7 +63,6 @@ def test_scan_content_allows_prose_license_and_source_negative_fixtures() -> Non assert policy.scan_content("docs/migration.md", sample) == () assert policy.scan_content("COPYING", sample) == () assert policy.scan_content("scripts/ci/pingora_edge_policy.py", sample) == () - assert policy.scan_content("tests/test_pingora_edge_policy.py", sample) == () assert policy.scan_content("tests/fixtures/policy_samples.py", sample) == () assert policy.scan_content("tests/fixtures/negative_fixture.rs", sample) == () assert policy.scan_content("deploy/fixtures/runtime.yaml", sample) @@ -75,21 +74,17 @@ def test_scan_content_allows_prose_license_and_source_negative_fixtures() -> Non ) -def test_this_test_files_own_content_is_exempt() -> None: - """This file's own fixture strings (denied Nginx forms) must never self-trip. - - Regression coverage for a real required-workflow-bootstrap failure: a - diff to this file that happens to add a line matching a CONTENT_RULES - pattern (e.g. a new test fixture containing "/etc/nginx/") triggers - _needs_content_scan's "nginx" in the patch heuristic, which then scans - this file's *entire* current content -- full of intentional denied - forms by design -- unless this exact path is self-exempted the same way - scripts/ci/pingora_edge_policy.py already is. - """ - - own_content = Path(__file__).read_text(encoding="utf-8") - assert policy.scan_content("tests/test_pingora_edge_policy.py", own_content) == () +def test_policy_test_module_rejects_active_runtime_content() -> None: + """The executable policy test module is not a blanket source fixture.""" + violations = policy.scan_content("tests/test_pingora_edge_policy.py", fixture_text()) + assert {item.rule for item in violations} == { + "nginx_container_image", + "nginx_ingress_controller", + "nginx_runtime_command", + "nginx_runtime_path", + "nginx_package_install", + } def test_nested_documentation_path_allows_prose_samples() -> None: """Documentation directories remain exempt when nested below a package.""" @@ -346,7 +341,12 @@ def opener(url: str, _token: str) -> object: {"filename": "docs/papers/not-really-a-pdf.pdf", "status": "added"}, ] assert "/contents/docs/papers/not-really-a-pdf.pdf" in url - return encoded_file("cat /etc/nginx/nginx.conf\n") + runtime_path = next( + line.removeprefix("COPY x ") + for line in fixture_text().splitlines() + if line.startswith("COPY x ") + ) + return encoded_file(f"cat {runtime_path}\\n") result = policy.evaluate_pull_request( api_url="https://api.github.test",