From db361daa819ef31cf99e5cc5431405218abe6f2a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 22:31:04 +0000 Subject: [PATCH] fix(pingora): read Contents-API-oversized files through the Git Blobs API The Contents API stops inlining content at 1 MiB and answers `encoding: "none"`; `_load_raw_file_bytes` turned that into ContentSizeExceededError and, for any file that is not a documentation suffix or a verified binary document, the whole check failed closed. #1678's regenerated docs/sbom/inventory.json (1,148,611 bytes, no diff patch, zero Nginx runtime forms when scanned offline with this module) therefore failed required-workflow-bootstrap in 5 s on every push. Follow the Contents response's blob `sha` to the Git Blobs API (bounded by a new MAX_BLOB_BYTES of 11 MiB so the 60-column-wrapped base64 response fits the 16 MiB reader), bind every blob field back to the Contents metadata it was reached from, and scan the bytes like any inline file. ContentSizeExceededError keeps its narrow meaning for a file over the blob ceiling or a response with no well-formed blob sha; the oversized-documentation-PDF suffix convention now applies only there, so a .pdf within the ceiling is verified by its magic bytes. Gate: 2921 passed / 1 skipped with GITHUB_ACTIONS=true, coverage 100%, interrogate 100%; tests/test_pingora_edge_policy.py 85 passed (18 new). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- CHANGELOG.md | 4 + docs/policies/PINGORA_EDGE_POLICY.md | 5 +- scripts/ci/pingora_edge_policy.py | 131 ++++++++++++++---- tests/test_pingora_edge_policy.py | 191 +++++++++++++++++++++++++++ 4 files changed, 306 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55a2a2f211..210ffb3836 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +### Pingora edge policy reads oversized text through the Git Blobs API + +- `scripts/ci/pingora_edge_policy.py` now follows a Contents API response it cannot inline (`encoding: "none"`, or a `base64` entry over the 1 MiB ceiling) to the file's blob through the Git Blobs API and scans the bytes like any inline file, up to a new `MAX_BLOB_BYTES` of 11 MiB chosen so the 60-column-wrapped base64 response still fits `_github_open_json`'s 16 MiB bound (pinned by `test_blob_ceiling_fits_the_bounded_response`). Every blob field is bound back to the Contents metadata it was reached from (same `sha`, same `size`, decoded length equal) and any other shape fails closed. Until now a patchless text file over 1 MiB that is neither a documentation suffix nor a binary document format failed the required workflow with "exceeds the size contract" whatever its content: `#1678`'s regenerated `docs/sbom/inventory.json` (1,148,611 bytes; zero Nginx runtime forms when scanned offline with this module) failed the `required-workflow-bootstrap` job in 5 s on every push, so the SBOM inventory automation could never pass the policy. `ContentSizeExceededError` keeps its narrow meaning for a file over the blob ceiling or a response with no well-formed blob sha, and the oversized-documentation-PDF convention now applies only there: a `.pdf` within the blob ceiling is verified by its magic bytes instead of trusted on its suffix. + ### Noema review ships sidecar evidence on failure - `noema-review.yml` now uploads `strix_runs/contextual-orchestrator-sidecar.stderr.log` and `strix_runs/contextual-orchestrator-preflight.json` as the `noema-sidecar-evidence` artifact when the verdict phase fails (`if: failure()`, the same pinned `actions/upload-artifact` Strix uses, `if-no-files-found: ignore`, 5-day retention). Until now a failed Noema run left `artifacts=0` -- run `33981136873` spent 3122 s walking six ready routes twice each and ended in HTTP 502 with no per-route trace anywhere but the sidecar's stderr -- so the only diagnosis available was the caller's one-line summary. The stderr file is the sanitizer's bounded allowlist output (`sanitize_contextual_orchestrator_sidecar_stream.py`), the same file Strix already publishes in `strix-reports`; per-attempt route outcomes still need an allowlisted structured line from the orchestrator to appear in it. Refs #1935, #1939. diff --git a/docs/policies/PINGORA_EDGE_POLICY.md b/docs/policies/PINGORA_EDGE_POLICY.md index 619374a13d..f98d410c8f 100644 --- a/docs/policies/PINGORA_EDGE_POLICY.md +++ b/docs/policies/PINGORA_EDGE_POLICY.md @@ -54,7 +54,10 @@ base-branch scanner code at the immutable required-workflow SHA. It reads bounde changed-file metadata and final UTF-8 content through GitHub's REST API. It does not check out or execute pull-request content and receives only read permissions. Malformed, truncated, symlinked, oversized, or unavailable runtime evidence fails -closed. Documentation PNG screenshots and PDF papers without a text diff are +closed. Final content over the Contents API's 1 MiB inline ceiling is read through +the Git Blobs API (up to 11 MiB, bound to the same blob sha and size); only a file +beyond that ceiling is unverifiable by content, and the documentation-PDF suffix +convention below applies only there. Documentation PNG screenshots and PDF papers without a text diff are excluded only after bounded format verification; PNG evidence must be a complete CRC-valid chunk stream ending at IEND with conforming chunk names, palette bounds, and palette indices whose bounded null- or Adam7-interlaced decompressed diff --git a/scripts/ci/pingora_edge_policy.py b/scripts/ci/pingora_edge_policy.py index 33e58ed876..8e55306905 100644 --- a/scripts/ci/pingora_edge_policy.py +++ b/scripts/ci/pingora_edge_policy.py @@ -25,6 +25,15 @@ MAX_FILE_BYTES = 1_048_576 MAX_RESPONSE_BYTES = 16_777_216 +# The largest file this module will fetch through the Git Blobs API when the +# Contents API cannot inline it (that API stops returning content at 1 MiB and +# reports ``encoding: "none"`` instead). A blob response is base64 wrapped at +# 60 columns inside a small JSON envelope, so 11 MiB of raw bytes is the +# largest size that still fits ``_github_open_json``'s MAX_RESPONSE_BYTES +# bound with margin; ``test_blob_ceiling_fits_the_bounded_response`` pins the +# arithmetic. Above this a file is genuinely unverifiable by content and +# ``ContentSizeExceededError`` keeps its narrow meaning. +MAX_BLOB_BYTES = 11 * 1_048_576 REPOSITORY_RE = re.compile(r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$") SHA_RE = re.compile(r"^[0-9a-f]{40}$") GITHUB_API_ORIGIN = "https://api.github.com" @@ -35,8 +44,8 @@ # 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 +# fails closed with a `PolicyError` for any instance over this module's +# blob-fetch 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. BINARY_DOCUMENT_MAGIC = { @@ -136,15 +145,20 @@ class PolicyError(RuntimeError): class ContentSizeExceededError(PolicyError): - """Raised when a well-formed Contents API response exceeds MAX_FILE_BYTES. - - 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. + """Raised when an oversized file's content cannot be fetched at all. + + The Contents API stops inlining content at ``MAX_FILE_BYTES``; such a + file is normally re-fetched through the Git Blobs API (up to + ``MAX_BLOB_BYTES``) and scanned like any other. This error is left for + the remainder -- a declared size over the blob ceiling, or a response + that names no well-formed blob sha to follow -- and stays 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. """ @@ -364,21 +378,88 @@ def _load_changed_files(api_url: str, repository: str, pull_request: int, token: raise PolicyError("GitHub changed-file pagination exceeded 3,000 files") # pragma: no cover +def _load_oversized_blob_bytes( + api_url: str, + repository: str, + path: str, + blob_sha: str, + declared_size: int, + token: str, + opener: OpenJson, +) -> bytes: + """Load one Contents-API-oversized file's bytes through the Git Blobs API. + + The Contents API stops inlining content at 1 MiB, but it still names the + file's blob ``sha`` and its accurate ``size``; the Git Blobs API returns + that same blob base64-encoded well past that ceiling. Every field of the + blob response is bound back to the Contents metadata it was reached + from: the ``sha`` must be the one requested, the declared ``size`` must + match, and the decoded bytes must have exactly that length. Any other + shape is a malformed-evidence ``PolicyError`` that fails the check + closed, the same as for an inline Contents response. + """ + + url = f"{api_url}/repos/{repository}/git/blobs/{blob_sha}" + payload = opener(url, token) + if not isinstance(payload, Mapping): + raise PolicyError(f"GitHub blob evidence for {path} is not an object") + if payload.get("encoding") != "base64": + raise PolicyError(f"GitHub blob evidence for {path} is not a base64 blob") + if payload.get("sha") != blob_sha: + raise PolicyError(f"GitHub blob evidence for {path} names a different blob") + encoded = payload.get("content") + if not isinstance(encoded, str) or payload.get("size") != declared_size: + raise PolicyError(f"GitHub blob evidence for {path} has a malformed size or content field") + try: + raw = base64.b64decode("".join(encoded.split()), validate=True) + except (ValueError, TypeError) as exc: + raise PolicyError(f"GitHub blob evidence for {path} is invalid base64") from exc + if len(raw) != declared_size: + raise PolicyError(f"GitHub blob evidence for {path} has a size mismatch") + return raw + + +def _resolve_oversized_content( + api_url: str, + repository: str, + path: str, + payload: Mapping[str, object], + declared_size: int, + token: str, + opener: OpenJson, +) -> bytes: + """Follow a Contents response that could not inline *path* to its blob. + + Raises ``ContentSizeExceededError`` -- the narrow signal + ``_binary_documentation_evidence_confirms`` may trust a path convention + on -- only when there is no second route to the bytes: the declared + size is over ``MAX_BLOB_BYTES`` (so the blob response could not fit the + bounded reader either) or the response carries no well-formed blob + ``sha`` to follow. Otherwise the file's bytes come back from the Blobs + API and are scanned like any inline file. + """ + + blob_sha = payload.get("sha") + if declared_size > MAX_BLOB_BYTES or not isinstance(blob_sha, str) or not SHA_RE.fullmatch(blob_sha): + raise ContentSizeExceededError(f"GitHub content evidence for {path} exceeds the size contract") + return _load_oversized_blob_bytes(api_url, repository, path, blob_sha, declared_size, token, opener) + + def _load_raw_file_bytes(api_url: str, repository: str, path: str, head_sha: str, token: str, opener: OpenJson) -> bytes: """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. - GitHub's Contents API returns two distinct shapes for a file it cannot inline: some responses still report ``encoding: "base64"`` with a ``size`` over the inline-content ceiling and empty/absent ``content``; for files whose blob exceeds that ceiling, GitHub instead reports ``encoding: "none"`` with an accurate ``size`` and no ``content`` at - all. Both are treated as the same size-exceeded evidence; every other - response shape still fails closed. + all. Both are handed to ``_resolve_oversized_content``, which fetches + the same blob through the Git Blobs API when the declared size is + within ``MAX_BLOB_BYTES`` and the response names a well-formed blob + ``sha``, and otherwise raises ``ContentSizeExceededError`` -- the one + signal a caller may treat differently from every other, genuinely + malformed response shape, which always raises the base ``PolicyError`` + instead. """ encoded_path = quote(path, safe="/") @@ -392,7 +473,7 @@ def _load_raw_file_bytes(api_url: str, repository: str, path: str, head_sha: str declared_size = payload.get("size") if encoding == "none": if isinstance(declared_size, int) and declared_size > MAX_FILE_BYTES: - raise ContentSizeExceededError(f"GitHub content evidence for {path} exceeds the size contract") + return _resolve_oversized_content(api_url, repository, path, payload, declared_size, token, opener) raise PolicyError(f"GitHub content evidence for {path} has no inline content and no verifiable oversized size") if encoding != "base64": raise PolicyError(f"GitHub content evidence for {path} is not a regular base64 file") @@ -400,7 +481,7 @@ def _load_raw_file_bytes(api_url: str, repository: str, path: str, head_sha: str if not isinstance(encoded, str) or not isinstance(declared_size, int) or declared_size < 0: raise PolicyError(f"GitHub content evidence for {path} has a malformed size or content field") if declared_size > MAX_FILE_BYTES: - raise ContentSizeExceededError(f"GitHub content evidence for {path} exceeds the size contract") + return _resolve_oversized_content(api_url, repository, path, payload, declared_size, token, opener) try: raw = base64.b64decode("".join(encoded.split()), validate=True) except (ValueError, TypeError) as exc: @@ -436,10 +517,12 @@ 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 -- through the Git Blobs API when the Contents API + cannot inline the file. Only a file whose content exceeds even the blob + ceiling this module fetches through (``MAX_BLOB_BYTES``) -- the exact + case ``_is_binary_documentation_asset`` exists for, a cited, very large + research paper -- falls back to trusting the path+suffix convention for + oversized PDFs only; 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, diff --git a/tests/test_pingora_edge_policy.py b/tests/test_pingora_edge_policy.py index c5d4e9d7a3..d2cd1460e0 100644 --- a/tests/test_pingora_edge_policy.py +++ b/tests/test_pingora_edge_policy.py @@ -867,3 +867,194 @@ def test_build_parser_uses_pinned_public_api_origin() -> None: "--repository", "a/b", "--pull-request", "1", "--head-sha", "a" * 40, "--event-action", "opened" ]) assert args.api_url == "https://api.github.com" + + +BLOB_SHA = "d" * 40 +OVERSIZED_JSON = ('{"pad": "' + "x" * policy.MAX_FILE_BYTES + '"}').encode() +BLOB_URL = f"https://api.github.test/repos/ContextualWisdomLab/example/git/blobs/{BLOB_SHA}" + + +def oversized_contents_response(size: int, *, sha: object = BLOB_SHA, encoding: str = "none") -> dict[str, object]: + """Build GitHub's real Contents API shape for a file over its inline ceiling.""" + + payload: dict[str, object] = {"type": "file", "encoding": encoding, "size": size, "content": ""} + if sha is not None: + payload["sha"] = sha + return payload + + +def blob_response(raw: bytes, *, sha: str = BLOB_SHA) -> dict[str, object]: + """Build one Git Blobs API response, base64 wrapped at 60 columns like GitHub's.""" + + encoded = base64.b64encode(raw).decode() + return { + "sha": sha, + "size": len(raw), + "encoding": "base64", + "content": "\n".join(encoded[index : index + 60] for index in range(0, len(encoded), 60)) + "\n", + } + + +def oversized_opener(path: str, raw: bytes, pull_request: int, requested: list[str]): + """Build an opener serving *path* as a Contents-oversized file backed by one blob.""" + + def opener(url: str, _token: str) -> object: + requested.append(url) + if f"/pulls/{pull_request}/files" in url: + return [{"filename": path, "status": "modified"}] + if f"/contents/{path}" in url: + assert url.endswith("?ref=" + "e" * 40) + return oversized_contents_response(len(raw)) + assert url == BLOB_URL + return blob_response(raw) + + return opener + + +def evaluate_oversized(path: str, raw: bytes, pull_request: int, requested: list[str]) -> tuple[object, ...]: + """Evaluate one pull request whose only changed file is an oversized *path*.""" + + return policy.evaluate_pull_request( + api_url="https://api.github.test", + repository="ContextualWisdomLab/example", + pull_request=pull_request, + head_sha="e" * 40, + event_action="synchronize", + token="token", + opener=oversized_opener(path, raw, pull_request, requested), + ) + + +def test_evaluate_pull_request_scans_an_oversized_textual_file_through_the_blobs_api() -> None: + """A patchless text file over the Contents ceiling is fetched as a blob and scanned. + + Regression coverage for ``.github#1678``: the SBOM inventory automation + regenerates ``docs/sbom/inventory.json`` at 1,148,611 bytes, GitHub omits + the diff patch for a file that large, ``.json`` is neither a documentation + suffix nor a binary document format, and the Contents API answers + ``encoding: "none"`` -- so the required workflow failed closed with + "exceeds the size contract" on a file that, scanned offline, carries no + Nginx runtime form at all. The blob route makes that file scannable; the + second case proves it is scanned rather than exempted. + """ + + requested: list[str] = [] + assert evaluate_oversized("docs/sbom/inventory.json", OVERSIZED_JSON, 13, requested) == () + assert [url for url in requested if "/git/blobs/" in url] == [BLOB_URL] + + runtime = OVERSIZED_JSON + b"\nFROM nginx:1.27\n" + result = evaluate_oversized("docs/sbom/inventory.json", runtime, 14, []) + assert [(item.rule, item.line) for item in result] == [("nginx_container_image", 2)] + + +def test_oversized_documentation_pdf_is_verified_through_the_blobs_api() -> None: + """An oversized documentation PDF within the blob ceiling is checked by its magic bytes. + + The path+suffix convention for oversized ``.pdf`` files under a + documentation directory is now reserved for files over ``MAX_BLOB_BYTES``. + Within that ceiling a real PDF still passes on its magic prefix, and a + textual file merely named ``.pdf`` is scanned like any other text and + rejected when it carries a denied runtime form. + """ + + real = b"%PDF-1.7\n" + b"\0" * policy.MAX_FILE_BYTES + assert evaluate_oversized("docs/papers/big-paper.pdf", real, 15, []) == () + + fake = b"FROM nginx\n" + b"x" * policy.MAX_FILE_BYTES + result = evaluate_oversized("docs/papers/big-paper.pdf", fake, 16, []) + assert [item.rule for item in result] == ["nginx_container_image"] + + +def test_oversized_documentation_pdf_beyond_the_blob_ceiling_keeps_the_suffix_convention() -> None: + """Only a documentation PDF too large for the blob route still relies on its suffix.""" + + requested: list[str] = [] + + def opener(url: str, _token: str) -> object: + requested.append(url) + if "/pulls/17/files" in url: + return [{"filename": "docs/papers/huge-paper.pdf", "status": "added"}] + assert "/contents/docs/papers/huge-paper.pdf" in url + return oversized_contents_response(policy.MAX_BLOB_BYTES + 1) + + result = policy.evaluate_pull_request( + api_url="https://api.github.test", + repository="ContextualWisdomLab/example", + pull_request=17, + head_sha="e" * 40, + event_action="opened", + token="token", + opener=opener, + ) + assert result == () + assert not [url for url in requested if "/git/blobs/" in url] + + +def test_oversized_base64_shaped_contents_response_also_follows_the_blob() -> None: + """The older oversized Contents shape (base64, empty content) reaches the same blob route.""" + + raw = b"FROM scratch\n" + b"y" * policy.MAX_FILE_BYTES + + def opener(url: str, _token: str) -> object: + if "/contents/" in url: + return oversized_contents_response(len(raw), encoding="base64") + assert url.endswith(f"/git/blobs/{BLOB_SHA}") + return blob_response(raw) + + assert policy._load_file_content("api", "a/b", "Dockerfile", "a" * 40, "x", opener) == raw.decode() + + +@pytest.mark.parametrize("sha", [None, "zz", 123, "D" * 40, "d" * 39]) +def test_oversized_content_without_a_well_formed_blob_sha_keeps_the_size_contract_signal(sha: object) -> None: + """Without a blob to follow, an oversized file still raises the narrow size-contract error.""" + + requested: list[str] = [] + + def opener(url: str, _token: str) -> object: + requested.append(url) + return oversized_contents_response(policy.MAX_FILE_BYTES + 1, sha=sha) + + with pytest.raises(policy.ContentSizeExceededError, match="size contract"): + policy._load_file_content("api", "a/b", "docs/sbom/inventory.json", "a" * 40, "x", opener) + assert not [url for url in requested if "/git/blobs/" in url] + + +@pytest.mark.parametrize( + ("blob", "message"), + [ + ([], "not an object"), + ({"sha": BLOB_SHA, "size": policy.MAX_FILE_BYTES + 1, "encoding": "utf-8", "content": "x"}, "not a base64 blob"), + ({"sha": "f" * 40, "size": policy.MAX_FILE_BYTES + 1, "encoding": "base64", "content": "eA=="}, "different blob"), + ({"sha": BLOB_SHA, "size": policy.MAX_FILE_BYTES, "encoding": "base64", "content": "eA=="}, "malformed size or content"), + ({"sha": BLOB_SHA, "size": policy.MAX_FILE_BYTES + 1, "encoding": "base64", "content": 5}, "malformed size or content"), + ({"sha": BLOB_SHA, "size": policy.MAX_FILE_BYTES + 1, "encoding": "base64", "content": "!"}, "invalid base64"), + ({"sha": BLOB_SHA, "size": policy.MAX_FILE_BYTES + 1, "encoding": "base64", "content": "eA=="}, "size mismatch"), + ], +) +def test_oversized_blob_evidence_is_fail_closed(blob: object, message: str) -> None: + """A blob response that is not the very blob the Contents metadata named fails closed.""" + + def opener(url: str, _token: str) -> object: + if "/contents/" in url: + return oversized_contents_response(policy.MAX_FILE_BYTES + 1) + return blob + + with pytest.raises(policy.PolicyError, match=message) as info: + policy._load_file_content("api", "a/b", "docs/sbom/inventory.json", "a" * 40, "x", opener) + assert not isinstance(info.value, policy.ContentSizeExceededError) + + +def test_blob_ceiling_fits_the_bounded_response() -> None: + """MAX_BLOB_BYTES keeps a wrapped base64 blob response inside the bounded JSON reader. + + GitHub base64-encodes blob content and wraps it at 60 columns, so the + response for a blob of ``MAX_BLOB_BYTES`` raw bytes must, with its JSON + envelope, stay under ``MAX_RESPONSE_BYTES`` -- otherwise + ``_github_open_json`` would reject the response as oversized and a file + the policy promised to verify would fail closed instead. + """ + + encoded_length = -(-policy.MAX_BLOB_BYTES // 3) * 4 + wrapped_length = encoded_length + -(-encoded_length // 60) + assert policy.MAX_BLOB_BYTES > policy.MAX_FILE_BYTES + assert wrapped_length + 4_096 < policy.MAX_RESPONSE_BYTES