diff --git a/CHANGELOG.md b/CHANGELOG.md index 75a4109c9d..9d8521ed12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ ### Contextual-orchestrator pin advance fixes orchestrator/free retry-stacking - Advanced the central sidecar's pinned immutable CO revision from `2e414d15` to protected `main@414f22973658c4ddc3d4320fcf7acd9b4e8ba991`, carrying contextual-orchestrator#1081's fix into Strix, OpenCode, and Noema. Root cause: `TaskOrchestrator._invoke`'s own retry-then-failover decision for a retryable 5xx (budgeted `1 + tool_retry_attempts` real tries per candidate) was getting multiplied by `ModelClient._send_with_retry`'s independent transient-retry-with-backoff underneath it (`max_retries + 1` further tries per call) -- up to 6 real network attempts against one already-flagged-flaky `orchestrator/free` agent before `_invoke` ever tried the next ranked candidate. Confirmed as the cause of independently observed incidents in #1912, #1231, #1503, and #1198, each spending 9-57+ minutes on one escalated route and surfacing that same route's model in its final error, never reaching a cleanly-ready sibling preflight had already found. The fix (`ModelClient.single_attempt_transport()`) changes only which agent gets tried next; no per-attempt timeout changed. Reproduced the bug directly against unmodified contextual-orchestrator `main` before the fix (6 real attempts) and confirmed the fix resolves it (<=2) before advancing this pin. `docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md`'s 2026-09-06 amendment and `tests/test_contextual_orchestrator_review_sidecar_contract.py`'s `ORCH_PIN_SHA` were updated alongside this pin. All callers still consume an exact SHA; no branch or tag is introduced. +### 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. ### Review sidecar preflight keeps transient-rejected routes as deferred failover 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