diff --git a/scripts/ci/pingora_edge_policy.py b/scripts/ci/pingora_edge_policy.py index eb0e3a741a..4718d80f5b 100644 --- a/scripts/ci/pingora_edge_policy.py +++ b/scripts/ci/pingora_edge_policy.py @@ -32,15 +32,9 @@ GITHUB_API_ORIGIN = "https://api.github.com" DOCUMENT_SUFFIXES = frozenset({".md", ".mdx", ".rst", ".adoc", ".txt"}) -# Opaque binary document formats that cannot embed an interpretable, active -# Nginx runtime artifact (unlike a text config, script, or container image -# reference). Without this, any such file placed under a documentation -# directory still falls through to `_needs_content_scan` -> `True` (binary -# files never carry a GitHub diff `patch`), and then `_load_file_content` -# fails closed with a `PolicyError` for any instance over the Contents API's -# 1 MiB base64 ceiling -- rejecting a legitimate research-paper citation -# (this org's own "attach the relevant paper PDF" convention) for a reason -# that has nothing to do with the Nginx runtime policy this module enforces. +# Opaque binary document formats are checked by bounded magic/format evidence; +# content over the Contents API's 1 MiB ceiling remains fail-closed because its +# bytes cannot be inspected for an active runtime artifact. BINARY_DOCUMENT_MAGIC = { ".hwpx": (b"PK\x03\x04",), ".pdf": (b"%PDF-",), @@ -50,6 +44,7 @@ SOURCE_TEST_SUFFIXES = frozenset({".py", ".pyi", ".js", ".mjs", ".cjs", ".ts", ".tsx", ".rs"}) LICENSE_NAMES = frozenset({"license", "license.md", "copying", "copyrights", "notice"}) DOCUMENTATION_DIRECTORIES = frozenset({"doc", "docs", "documentation"}) +PUBLICATION_BINARY_DIRECTORIES = frozenset({"evidence", "figures"}) DOCUMENTATION_ROOT_NAMES = frozenset({"readme", "changelog", "changes"}) RUNTIME_PATH_NAMES = frozenset({ @@ -62,22 +57,28 @@ r"(?:-(?:u|g|h|C|p|R|T)|--(?:user|group|host|close-from|prompt|chroot|command-timeout))" ) SUDO_OPTION_RE = ( - rf"(?:{SUDO_ARGUMENT_OPTION_RE}(?:=|\s+)\S+|" + rf"(?:{SUDO_ARGUMENT_OPTION_RE}(?:=|[ \t]+)\S+|" rf"(?!(?:{SUDO_ARGUMENT_OPTION_RE})(?:=|\s|$))--?\S+|--)" ) -SUDO_PREFIX_RE = rf"(?:sudo\s+(?:{SUDO_OPTION_RE}\s+)*|)" +SUDO_PREFIX_RE = rf"(?:sudo[ \t]+(?:{SUDO_OPTION_RE}[ \t]+)*|)" NGINX_RUNTIME_IMAGE_RE = ( r"(?:nginx|nginx-(?!prometheus-exporter(?:[:@\s]|$))[A-Za-z0-9._-]+)" ) +NGINX_COMMAND_RE = r"(?:nginx|/(?:[A-Za-z0-9._-]+/)*nginx|(?:\.\.?/)*(?:[A-Za-z0-9._-]+/)*nginx)" +PACKAGE_OPTION_RE = ( + r"(?:--[A-Za-z0-9][A-Za-z0-9-]*(?:=[^\s#\\]+|[ \t]+[^\s#\\]+)?|" + r"-[A-Za-z0-9](?:=[^\s#\\]+|[ \t]+[^\s#\\]+)?)[ \t]+" +) CONTENT_RULES: tuple[tuple[str, re.Pattern[str]], ...] = ( ( "nginx_container_image", re.compile( - r"(?im)^\s*(?:-\s*)?(?:FROM|image:)\s+" + r"(?im)^[ \t]*(?:-[ \t]*)?(?:FROM|image:)[ \t]+" + r"[\"']?" r"(?:[A-Za-z0-9._-]+(?::[0-9]+)?/)*" rf"{NGINX_RUNTIME_IMAGE_RE}" - r"(?:[:@]\S+|\s|$)" + r"(?:[:@]\S+|[\"']?\s|[\"']?$)" ), ), ( @@ -91,9 +92,9 @@ ( "nginx_runtime_command", re.compile( - r"(?im)(?:^\s*(?:systemctl|service)\s+(?:--\S+\s+)*(?:\S+\s+)*nginx\b|" - rf"^\s*{SUDO_PREFIX_RE}nginx(?=\s|$|[;&|])|" - r"(?:CMD|ENTRYPOINT)\s*\[[^\n]*[\"']nginx[\"']|" + rf"(?im)(?:^[ \t]*{SUDO_PREFIX_RE}(?:systemctl|service)[ \t]+(?:--\S+[ \t]+)*(?:\S+[ \t]+)*nginx\b|" + rf"^[ \t]*{SUDO_PREFIX_RE}{NGINX_COMMAND_RE}(?=[ \t]|$|[;&|])|" + rf"(?:CMD|ENTRYPOINT)\s*\[[^\n]*[\"'](?:[A-Za-z0-9._/-]*?/)?nginx[\"']|" r"\bnginx\s+-g\s+[\"']daemon\s+off;)" ), ), @@ -107,8 +108,9 @@ ( "nginx_package_install", re.compile( - rf"(?im)^\s*(?:RUN\s+)?{SUDO_PREFIX_RE}(?:apk\s+add|apt(?:-get)?\s+install|" - r"dnf\s+install|yum\s+install)\b(?:[^\n#]*\\\s*\n\s*)*[^\n#]*\bnginx\b" + rf"(?im)^[ \t]*(?:RUN[ \t]+)?{SUDO_PREFIX_RE}(?:apk|apt(?:-get)?|dnf|yum)[ \t]+" + rf"(?:{PACKAGE_OPTION_RE})*(?:add|install)" + r"\b(?:[^\n#\\]*\\[ \t]*\r?\n[ \t]*)*[^\n#\\]*\bnginx\b" ), ), ) @@ -143,11 +145,8 @@ class ContentSizeExceededError(PolicyError): Distinct from every other ``PolicyError`` cause (a malformed response, a non-file/non-base64 entry, corrupt base64, a declared size that does not - match the decoded bytes) so a caller can choose to trust a narrow, - path-scoped convention -- a genuinely oversized documentation PDF, the - one case this module cannot verify by content at all -- instead of - failing the whole check closed. Every other content-evidence failure - still fails closed exactly as before. + match the decoded bytes) so callers can report the precise fail-closed + reason. Oversized files cannot be admitted without format evidence. """ @@ -178,11 +177,8 @@ def _is_known_documentation_path(pure: PurePosixPath) -> bool: def _is_documentation_or_source_fixture(path: str) -> bool: """Return whether *path* is prose, license text, or scanner source fixture. - Textual suffixes only: a ``.pdf`` is handled separately by - ``_is_binary_documentation_asset`` and gated on GitHub reporting no diff - ``patch`` for it, so a textual file merely named with a ``.pdf`` suffix - (one GitHub *can* diff, meaning it could carry inspectable content) is - never exempted here. + Textual suffixes only: a binary document is handled separately by + ``_is_binary_documentation_asset`` and verified through its content. ``tests/test_pingora_edge_policy.py`` is exempted the same way this module's own source is: a scanner's regression suite necessarily @@ -232,7 +228,7 @@ def _is_binary_documentation_asset(changed: ChangedFile) -> bool: pure = PurePosixPath(changed.path) return ( pure.suffix.lower() in BINARY_DOCUMENT_MAGIC - and (_is_known_documentation_path(pure) or (pure.suffix.lower() == ".hwpx" and "evidence" in (part.lower() for part in pure.parts))) + and (_is_known_documentation_path(pure) or any(part.lower() in PUBLICATION_BINARY_DIRECTORIES for part in pure.parts)) and _runtime_path_rule(changed.path) is None ) @@ -261,12 +257,12 @@ def _line_number(content: str, start: int) -> int: def scan_content(path: str, content: str) -> tuple[Violation, ...]: """Return all Pingora policy violations found in one final file version.""" - if _is_documentation_or_source_fixture(path): - return () violations: list[Violation] = [] path_rule = _runtime_path_rule(path) if path_rule is not None: violations.append(Violation(path, path_rule, 1, "active Nginx runtime artifact path")) + if _is_documentation_or_source_fixture(path): + return tuple(violations) for rule, pattern in CONTENT_RULES: for match in pattern.finditer(content): excerpt = " ".join(match.group(0).strip().split())[:160] @@ -371,9 +367,9 @@ def _load_raw_file_bytes(api_url: str, repository: str, path: str, head_sha: str """Load one final head file's raw decoded bytes from the Contents API. Raises ``ContentSizeExceededError`` specifically when the declared size - is a well-formed positive integer over ``MAX_FILE_BYTES`` -- a signal a - caller may treat differently from every other, genuinely malformed - response shape, which always raises the base ``PolicyError`` instead. + is a well-formed positive integer over ``MAX_FILE_BYTES``. This remains + distinct from every other malformed response shape so the caller can + report the precise fail-closed reason. GitHub's Contents API returns two distinct shapes for a file it cannot inline: some responses still report ``encoding: "base64"`` with a @@ -439,10 +435,9 @@ def _binary_documentation_evidence_confirms( limit, well under this module's ``MAX_FILE_BYTES`` content-fetch ceiling. Whenever the file's raw bytes can be fetched at all, this verifies the declared format's magic prefix instead of trusting - patch-presence alone. Only a file whose content evidently exceeds the - Contents API's size ceiling -- the exact case ``_is_binary_documentation_asset`` - exists for, a cited, large research paper -- falls back to trusting the - path+suffix convention for oversized PDFs only; every other + patch-presence alone. A file whose content exceeds the Contents API's + size ceiling cannot be verified and therefore remains subject to the + fail-closed content path; every other content-evidence failure (a malformed API response, corrupt base64, a declared size that does not match the decoded bytes) propagates and fails the whole check closed, @@ -452,12 +447,14 @@ def _binary_documentation_evidence_confirms( try: raw = _load_raw_file_bytes(api_url, repository, changed.path, head_sha, token, opener) except ContentSizeExceededError: - return PurePosixPath(changed.path).suffix.lower() == ".pdf" + return False suffix = PurePosixPath(changed.path).suffix.lower() if suffix == ".png": return _is_complete_png(raw) if suffix == ".hwpx": return _is_complete_hwpx(raw) + # PDF parsing is intentionally out of scope; its magic prefix is the + # bounded evidence available for this opaque documentation format. return raw.startswith(BINARY_DOCUMENT_MAGIC[suffix]) @@ -651,14 +648,16 @@ def _needs_content_scan(changed: ChangedFile) -> bool: for that case before this function is even consulted. """ - if changed.status == "removed" or _is_documentation_or_source_fixture(changed.path): + if changed.status == "removed": + return False + if _runtime_path_rule(changed.path) is not None: + return True + if _is_documentation_or_source_fixture(changed.path): return False if _is_binary_documentation_asset(changed): return False if not changed.patch_available: return True - if _runtime_path_rule(changed.path) is not None: - return True lower_path = changed.path.lower() if PurePosixPath(lower_path).name in {"dockerfile", "containerfile", "docker-compose.yml", "docker-compose.yaml", "compose.yml", "compose.yaml"}: return True @@ -697,8 +696,8 @@ def evaluate_pull_request( # a missing patch does not by itself prove binary content (GitHub # also omits one for an oversized textual diff), so this confirms # the format's magic prefix whenever the bytes can be fetched at - # all, falling back to the path+suffix convention only when the - # content genuinely exceeds the Contents API's size ceiling. A + # all. Content that exceeds the Contents API's size ceiling cannot + # be verified and therefore fails closed. A # removed file has no head content to fetch at all -- _needs_content_scan # already special-cases this the same way for every other file. if changed.status != "removed" and _is_binary_documentation_asset(changed): diff --git a/tests/test_pingora_edge_policy.py b/tests/test_pingora_edge_policy.py index c5d4e9d7a3..6da4b0fbbe 100644 --- a/tests/test_pingora_edge_policy.py +++ b/tests/test_pingora_edge_policy.py @@ -98,6 +98,13 @@ def test_nested_documentation_path_allows_prose_samples() -> None: assert policy.scan_content("packages/component/docs/migration.md", fixture_text()) == () +def test_figures_prose_is_scanned_while_publication_binary_paths_are_verified() -> None: + """Only binary publication assets receive the figures/evidence exemption.""" + + assert policy.scan_content("figures/migration.md", "nginx install nginx\n") + assert policy.scan_content("evidence/migration.md", "nginx install nginx\n") + + def test_needs_content_scan_exempts_documentation_pdfs() -> None: """A cited research-paper PDF under docs/ never reaches content scanning. @@ -230,6 +237,91 @@ def test_sudo_argument_options_do_not_reinterpret_their_values() -> None: } +@pytest.mark.parametrize( + "content", + [ + "RUN apt-get -y install nginx\n", + "RUN apt -y install nginx\n", + "RUN apk --no-cache add nginx\n", + "RUN dnf -y install nginx\n", + "RUN yum --assumeyes install nginx\n", + ], +) +def test_package_manager_options_cannot_hide_nginx_install(content: str) -> None: + """Options between a package manager and its verb remain bounded and denied.""" + + assert [item.rule for item in policy.scan_content("Dockerfile", content)] == [ + "nginx_package_install" + ] + + +@pytest.mark.parametrize( + "content", + [ + "RUN apt-get -o Debug::pkgProblemResolver=yes install nginx\n", + "RUN apt-get --option Debug::pkgProblemResolver=yes install nginx\n", + "RUN apt-get -o ./relative.conf install nginx\n", + ], +) +def test_valued_package_manager_options_cannot_hide_nginx_install(content: str) -> None: + """Short, long, and relative valued options remain denied.""" + + assert [item.rule for item in policy.scan_content("Dockerfile", content)] == [ + "nginx_package_install" + ] + + +@pytest.mark.parametrize("manager", ["dnf", "yum"]) +def test_package_install_crlf_continuations_cannot_hide_nginx(manager: str) -> None: + """CRLF Dockerfile continuations remain covered by the package rule.""" + + content = f"RUN {manager} install \\\r\n nginx\r\n" + assert [item.rule for item in policy.scan_content("Dockerfile", content)] == [ + "nginx_package_install" + ] + + +@pytest.mark.parametrize("command", ["./objs/nginx -s reload\n", "../../objs/nginx -s reload\n"]) +def test_relative_nginx_command_cannot_bypass_runtime_rule(command: str) -> None: + """A relative executable path still identifies an active Nginx command.""" + + assert [item.rule for item in policy.scan_content("scripts/start.sh", command)] == [ + "nginx_runtime_command" + ] + + +@pytest.mark.parametrize( + "content", + [ + 'FROM "nginx:1.25-alpine"\n', + "sudo systemctl restart nginx\n", + 'CMD ["/usr/sbin/nginx", "-g", "daemon off;"]\n', + ], +) +def test_canonical_runtime_forms_cannot_bypass_content_rules(content: str) -> None: + """Quoted, namespaced, sudo, and absolute-path forms remain denied.""" + + assert policy.scan_content("deploy/runtime.yaml", content) + + +def test_package_install_continuations_have_bounded_matching_time() -> None: + """Continuation matching stays close to linear for attacker-controlled input.""" + + import time + + def elapsed(lines: int) -> float: + content = "RUN apt-get install " + (chr(92) + "\n").join( + " " + ("x" * 100) for _ in range(lines) + ) + " nginx\n" + started = time.perf_counter() + policy.scan_content("Dockerfile", content) + return time.perf_counter() - started + + small = elapsed(64) + large = elapsed(256) + assert large < max(0.25, small * 8) + + def test_untrusted_document_suffix_does_not_bypass_runtime_scan() -> None: """A runtime-looking file cannot evade policy checks by using a prose suffix.""" @@ -237,12 +329,41 @@ def test_untrusted_document_suffix_does_not_bypass_runtime_scan() -> None: assert [item.rule for item in violations] == ["nginx_container_image"] +def test_runtime_path_is_checked_before_documentation_fixture_exemption() -> None: + """An active Nginx filename cannot hide beneath a documentation directory.""" + + changed = policy.ChangedFile + assert policy._needs_content_scan(changed("docs/nginx.conf", "modified", "")) + violations = policy.scan_content("docs/nginx.conf", "migration history\n") + assert [item.rule for item in violations] == ["nginx_runtime_artifact"] + + +def test_evaluate_scans_active_runtime_path_under_documentation() -> None: + """The network evaluation path preserves the path-level guard.""" + + def opener(url: str, _token: str) -> object: + if "/pulls/13/files" in url: + return [{"filename": "docs/nginx.conf", "status": "modified", "patch": "+server {}"}] + return encoded_file("server {}\n") + + result = policy.evaluate_pull_request( + api_url="https://api.github.test", + repository="ContextualWisdomLab/example", + pull_request=13, + head_sha="e" * 40, + event_action="opened", + token="token", + opener=opener, + ) + assert [item.rule for item in result] == ["nginx_runtime_artifact"] + + def test_evaluate_pull_request_reads_pagination_and_final_content() -> None: """The checker uses every file page and scans final head content, not removed lines.""" calls: list[str] = [] first_page = [ - {"filename": f"docs/file-{index}.md", "status": "modified", "patch": "+Nginx"} + {"filename": f"docs/file-{index}.md", "status": "modified", "patch": "+documentation"} for index in range(100) ] second_page = [ @@ -298,17 +419,16 @@ def opener(url: str, _token: str) -> object: assert [item.rule for item in result] == ["nginx_container_image"] -def test_evaluate_pull_request_exempts_an_oversized_documentation_pdf() -> None: - """A genuinely oversized documentation PDF still cannot be verified by content. +def test_evaluate_pull_request_rejects_an_unverifiable_oversized_pdf() -> None: + """An oversized PDF cannot bypass the policy on a suffix claim alone. GitHub's real Contents API response for a file whose blob exceeds the inline-content ceiling reports ``encoding: "none"`` with an accurate ``size`` and no ``content`` at all (not a ``base64``-encoded entry with an oversized declared size) -- this is that real shape, not a synthetic one, per Devin Review's finding that the earlier version of this test - used a response shape GitHub never actually returns. This is the one - case that still falls back to the path+suffix convention -- the real - research-paper-citation use case this whole exemption exists for. + used a response shape GitHub never actually returns. The checker fails + closed because no format evidence is available. """ def opener(url: str, _token: str) -> object: @@ -319,16 +439,16 @@ def opener(url: str, _token: str) -> object: assert "/contents/docs/papers/big-paper.pdf" in url return {"type": "file", "encoding": "none", "size": policy.MAX_FILE_BYTES + 1, "content": ""} - result = policy.evaluate_pull_request( - api_url="https://api.github.test", - repository="ContextualWisdomLab/example", - pull_request=11, - head_sha="c" * 40, - event_action="opened", - token="token", - opener=opener, - ) - assert result == () + with pytest.raises(policy.ContentSizeExceededError): + policy.evaluate_pull_request( + api_url="https://api.github.test", + repository="ContextualWisdomLab/example", + pull_request=11, + head_sha="c" * 40, + event_action="opened", + token="token", + opener=opener, + ) def test_evaluate_pull_request_scans_a_disguised_textual_pdf_without_a_patch() -> None: @@ -412,6 +532,50 @@ def opener(url: str, _token: str) -> object: ) == () +def test_evaluate_pull_request_exempts_a_real_publication_figure_png() -> None: + """Publication figures use the same bounded PNG evidence as screenshots.""" + + def opener(url: str, _token: str) -> object: + if "/pulls/150/files" in url: + return [{"filename": "figures/result.png", "status": "added"}] + assert "/contents/figures/result.png" in url + raw = base64.b64decode( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=" + ) + return { + "type": "file", "encoding": "base64", "size": len(raw), + "content": base64.b64encode(raw).decode("ascii"), + } + + assert policy.evaluate_pull_request( + api_url="https://api.github.test", + repository="ContextualWisdomLab/example", + pull_request=150, + head_sha="d" * 40, + event_action="opened", + token="token", + opener=opener, + ) == () + + +def test_evaluate_pull_request_exempts_a_real_evidence_png() -> None: + """Publication evidence PNGs are validated as binary artifacts.""" + + def opener(url: str, _token: str) -> object: + if "/pulls/151/files" in url: + return [{"filename": "evidence/manuscript_revision_pages/page_001.png", "status": "added"}] + assert "/contents/evidence/manuscript_revision_pages/page_001.png" in url + raw = base64.b64decode( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=" + ) + return {"type": "file", "encoding": "base64", "size": len(raw), "content": base64.b64encode(raw).decode("ascii")} + + assert policy.evaluate_pull_request( + api_url="https://api.github.test", repository="ContextualWisdomLab/example", + pull_request=151, head_sha="e" * 40, event_action="opened", token="token", opener=opener, + ) == () + + def test_evaluate_pull_request_rejects_a_fake_documentation_png() -> None: """A PNG suffix without PNG magic remains runtime-content evidence."""