Skip to content

fix(security): preserve private-reporting oversize failure through cleanup faults - #673

Merged
seonghobae merged 2 commits into
mainfrom
fix/private-vuln-audit-cancel-cleanup-20260912
Sep 12, 2026
Merged

fix(security): preserve private-reporting oversize failure through cleanup faults#673
seonghobae merged 2 commits into
mainfrom
fix/private-vuln-audit-cancel-cleanup-20260912

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Problem

readBoundedJson correctly treats cancellation as cleanup after the 16 KiB GitHub response ceiling is exceeded, but void reader.cancel().catch(...) still let a synchronously throwing cancel() replace the already-decided size-limit failure. That contradicted the cleanup-authority invariant and made failure classification depend on cleanup behavior.

This lane stays inside Noema's security/read-only operational evidence boundary. It does not alter private-vulnerability-reporting setting authority, GitHub identity, provider routing, quarantine, outbound policy, release, deployment, or production evidence.

RED → minimum GREEN

  • RED-only exact 03e71f503d21eb633a9429eb454e012fa48fd953 added a hostile reader whose first chunk exceeds 16 KiB, whose cancel() throws synchronously, and whose releaseLock() is observable.
  • Hosted CI run 34689976768 reproduced the intended failure after exact checkout/install/typecheck: release tests failed on the hostile cleanup path and later release steps were skipped.
  • GREEN exact 6f9dc4ca047817e4c9ef5ae8a55816493b1c558d wraps only synchronous cancellation cleanup in try/catch. The primary oversize error remains authoritative; asynchronous rejection handling and reader-lock release remain unchanged.

Exact-head checks

For 6f9dc4ca047817e4c9ef5ae8a55816493b1c558d:

  • ci 34690213329: SUCCESS; release tests, security step, KPI verification, dependency-license inventory, acquisition manifest/integrity, and base-drift guard all completed.
  • reviewer-ci 34690213314: SUCCESS.
  • patch-validator-image 34690213322: SUCCESS.
  • Security Scan 34690213336: SUCCESS.
  • CodeRabbit reviewed this exact and generated no actionable comments; merge risk was reported minimal.
  • current inline review-thread count: 0.
  • exact-head maintainer review is COMMENT-only, not self-approval.

Acceptance

  • retain the 16 KiB streamed byte ceiling, media-type gate, duplicate-key rejection, fatal UTF-8 and bounded diagnostics;
  • cleanup must not replace the primary oversize decision whether cancellation rejects asynchronously or throws synchronously;
  • reader lock must still be released on terminal paths;
  • normal merge only from this exact head after a fresh base/head/mergeability check;
  • no self-approval, force push, destructive rebase or gate weakening.

Summary by CodeRabbit

  • 버그 수정

    • 응답 데이터가 허용된 크기를 초과할 때 스트림 정리 과정에서 오류가 발생해도, 원래의 크기 제한 오류가 일관되게 표시됩니다.
    • 초과 응답 처리 후 리더 잠금이 정상적으로 해제됩니다.
  • 테스트

    • 스트림 취소 오류와 리더 잠금 해제 동작을 검증하는 테스트를 추가했습니다.

@coderabbitai

coderabbitai Bot commented Sep 12, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: e8046b10-aa4e-49e8-86d9-05a8de3d4db6

📥 Commits

Reviewing files that changed from the base of the PR and between 31c5ca8 and 6f9dc4c.

📒 Files selected for processing (2)
  • scripts/private-vulnerability-reporting-audit.mjs
  • test/private-vulnerability-reporting-adapter.test.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

응답 본문이 크기 제한을 초과하면 동기적인 reader.cancel() 오류가 발생해도 크기 제한 오류를 유지합니다. 테스트는 오류 우선순위와 releaseLock() 호출을 검증합니다.

Changes

취약점 보고 응답 처리

Layer / File(s) Summary
크기 제한 오류 보존
scripts/private-vulnerability-reporting-audit.mjs, test/private-vulnerability-reporting-adapter.test.ts
크기 제한 초과 경로에서 reader.cancel()의 동기 오류를 처리합니다. 테스트는 크기 제한 오류가 전달되고 리더 잠금이 해제되는지 검증합니다.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 6f9dc

The bounded-response cleanup path now reports the intended size-limit failure even when cancellation itself fails. The relevant regression is covered and the change is ready for normal final checks.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 readBoundedJson의 정리 오류가 응답 크기 제한 오류를 대체하지 않도록 수정한 핵심 변경을 정확하고 간결하게 설명합니다.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/private-vuln-audit-cancel-cleanup-20260912

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head review: hosted RED 34689976768 proved the synchronous cleanup fault replaced the already-decided oversize rejection. The smallest GREEN candidate catches only a synchronous cancel() cleanup fault; it does not weaken the 16 KiB limit or other parser gates, and releaseLock() remains in the existing terminal cleanup. The new exact-head gates are queued, so this is evidence commentary—not approval or completion.

@seonghobae
seonghobae marked this pull request as ready for review September 12, 2026 11:15

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head review for 6f9dc4ca047817e4c9ef5ae8a55816493b1c558d: the hosted RED at 03e71f503d21eb633a9429eb454e012fa48fd953 demonstrates the synchronous cancel() fault replacing the stable 16 KiB oversize decision; this head contains the minimum causal containment. The source patch preserves the byte/media-type/UTF-8/duplicate-key admission boundaries and finally lock release, while the hostile regression verifies primary-error authority plus cleanup. CodeRabbit reports no actionable finding on this exact and current inline-thread count is zero. Application CI, reviewer-ci, and patch-validator-image are GREEN; required Security Scan is still pending, so this COMMENT is not an approval or merge claim.

@seonghobae
seonghobae merged commit 185dcc2 into main Sep 12, 2026
18 checks passed
@seonghobae
seonghobae deleted the fix/private-vuln-audit-cancel-cleanup-20260912 branch September 12, 2026 11:21
seonghobae added a commit that referenced this pull request Sep 12, 2026
…ncel-authority-20260912

docs(noema): converge #673 private-reporting cancellation authority
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority: high High-priority or P1 work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant