fix(pingora): exempt documentation raster evidence - #1420
Conversation
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Warning Review limit reachedNext included review available in 47 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough정책 스캐너가 문서 디렉터리의 PNG 최종 바이트를 구조적으로 검증합니다. GIF, JPEG, WebP 예외를 제거합니다. 정책 문서와 PNG, PDF, 페이지네이션 회귀 테스트를 갱신합니다. ChangesPingora 래스터 증거 검증
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🟡 Moderate · up to The PR allows validated documentation PNGs to bypass runtime scanning, but the validator can allocate unbounded decompressed data before rejecting a crafted image, potentially disrupting the required policy gate; malformed transparency metadata may also be accepted. Merge should wait for bounded finalization and complete PNG validation. Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant GitHubContentsAPI
participant evaluate_pull_request
participant PNGValidator
GitHubContentsAPI->>evaluate_pull_request: 변경 파일 메타데이터와 최종 raw bytes
evaluate_pull_request->>PNGValidator: 문서 PNG 검증 요청
PNGValidator-->>evaluate_pull_request: 유효한 PNG 또는 PolicyError
evaluate_pull_request-->>GitHubContentsAPI: 스캔 통과 또는 정책 실패
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 93.10% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 2 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Signed-off-by: Codex <codex@localhost>
…260830' into HEAD # Conflicts: # docs/adr/0019-cloudflare-pingora-edge-standard.md # docs/policies/PINGORA_EDGE_POLICY.md # scripts/ci/pingora_edge_policy.py # tests/test_pingora_edge_policy.py
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
# Conflicts: # CHANGELOG.md
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Signed-off-by: Seongho Bae <me@seonghobae.me>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Signed-off-by: Seongho Bae <me@seonghobae.me>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Signed-off-by: Seongho Bae <me@seonghobae.me>
| valid_ancillary = ( | ||
| 1 <= separator <= 79 | ||
| and separator + 2 < length | ||
| and chunk_data[separator + 1] == 0 | ||
| ) |
There was a problem hiding this comment.
🟡 Malformed profile names receive exemptions
An iCCP name with forbidden bytes or spacing passes validation. Malformed PNG evidence then receives the documentation exemption.
| valid_ancillary = ( | |
| 1 <= separator <= 79 | |
| and separator + 2 < length | |
| and chunk_data[separator + 1] == 0 | |
| ) | |
| valid_ancillary = ( | |
| 1 <= separator <= 79 | |
| and all( | |
| 32 <= byte <= 126 or 161 <= byte <= 255 | |
| for byte in chunk_data[:separator] | |
| ) | |
| and chunk_data[0] != 32 | |
| and chunk_data[separator - 1] != 32 | |
| and b" " not in chunk_data[:separator] | |
| and separator + 2 < length | |
| and chunk_data[separator + 1] == 0 | |
| ) |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
📝 Info: Image size remains deliberately bounded
_load_raw_file_bytes still rejects PNGs above one megabyte. The policy explicitly requires bounded evidence, so larger screenshots remain unsupported by contract.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| if width is None or saw_trns or saw_idat or not valid_length: | ||
| return False | ||
| saw_trns = True | ||
| elif chunk_type in {b"cHRM", b"gAMA", b"iCCP", b"sBIT", b"sRGB"}: |
There was a problem hiding this comment.
🟡 Malformed PNG metadata passes validation
A PNG with malformed zTXt or pHYs data passes _is_recognized_documentation_image. The gate grants an exception without its required complete chunk validation.
Prompt for agents
The PNG validator in scripts/ci/pingora_edge_policy.py validates only five named ancillary chunks. Other standard ancillary chunks pass after CRC checking without their required payload, ordering, cardinality, or compressed-stream contracts. For example, a zTXt chunk containing an invalid zlib stream and a pHYs chunk with the wrong length both pass _is_recognized_documentation_image. Define the supported ancillary-chunk set explicitly and validate each supported chunk completely, including every compressed stream; reject unsupported or malformed chunks so the implementation matches the documented complete PNG validation contract. Add regression tests for malformed zTXt/iTXt payloads and fixed-length ancillary chunks.
Was this helpful? React with 👍 or 👎 to provide feedback.
| def _png_rows_are_valid( | ||
| pixels: bytes, | ||
| pass_layouts: list[tuple[int, int, int, int]], | ||
| *, | ||
| bytes_per_pixel: int, | ||
| bit_depth: int, | ||
| palette_entries: int | None, | ||
| ) -> bool: | ||
| """Validate filters and indexed samples for every decoded PNG pass.""" | ||
|
|
||
| for start, stride, rows, pass_width in pass_layouts: | ||
| previous = bytes(stride - 1) | ||
| for row in range(rows): | ||
| row_start = start + row * stride | ||
| filter_type = pixels[row_start] | ||
| if filter_type > 4: | ||
| return False | ||
| encoded = pixels[row_start + 1 : row_start + stride] | ||
| reconstructed = bytearray(len(encoded)) | ||
| for index, value in enumerate(encoded): | ||
| left = reconstructed[index - bytes_per_pixel] if index >= bytes_per_pixel else 0 | ||
| above = previous[index] | ||
| upper_left = previous[index - bytes_per_pixel] if index >= bytes_per_pixel else 0 | ||
| predictor = (0, left, above, (left + above) // 2, _paeth_predictor(left, above, upper_left))[filter_type] | ||
| reconstructed[index] = (value + predictor) & 0xFF | ||
| previous = bytes(reconstructed) | ||
| if palette_entries is None: | ||
| continue | ||
| samples_seen = 0 | ||
| for byte in reconstructed: | ||
| for shift in range(8 - bit_depth, -1, -bit_depth): | ||
| if samples_seen == pass_width: | ||
| break | ||
| if ((byte >> shift) & ((1 << bit_depth) - 1)) >= palette_entries: | ||
| return False | ||
| samples_seen += 1 | ||
| return True |
There was a problem hiding this comment.
📝 Info: Interlaced scanlines use correct geometry
_png_rows_are_valid resets prior rows per Adam7 pass and uses the defined filter byte distance. Indexed checks exclude only padding samples.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Outcome
Keep the Pingora required gate fail-closed for runtime candidates while allowing non-executable raster acceptance evidence beneath documentation directories.
Root cause
GitHub omits patches for binary files. The scanner treated every patchless file as a runtime candidate, fetched a PNG screenshot as UTF-8, and blocked LineageWeave PR 780 before the actual review workflow could run.
Changes
Validation
Cross-repo trigger: ContextualWisdomLab/LineageWeave#780.
Summary by CodeRabbit
정책 및 보안 개선
문서화
Active repair ownership
Codex resumed exact-head review repair on 2026-08-30 KST; current scope is the unresolved bounded PNG validation findings and protected delivery.