fix(cloudflare): bound control-plane responses during streaming - #618
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughCloudflare 제어면 응답 처리를 공통 bounded streaming reader로 통합했습니다. reader는 chunk 수신 중 1 MiB 한도를 적용하고, oversize·취소·인코딩·JSON·HTTP·provider 오류를 실패-폐쇄 방식으로 처리합니다. 관련 스크립트, 테스트, 문서를 갱신했습니다. ChangesCloudflare transport hardening
Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant CloudflareScript
participant readBoundedCloudflareJsonResponse
participant CloudflareResponseStream
CloudflareScript->>readBoundedCloudflareJsonResponse: 응답과 1 MiB ceiling 전달
readBoundedCloudflareJsonResponse->>CloudflareResponseStream: chunk 읽기
CloudflareResponseStream-->>readBoundedCloudflareJsonResponse: 응답 chunk 반환
readBoundedCloudflareJsonResponse-->>CloudflareScript: result 또는 실패 반환
✨ Finishing Touches📝 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 |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review on 973129832a43aa862646e4e505efc8492e52b2a0: the source/test repair is coherent with the Noema-owned deployment/recovery transport boundary. The bounded reader enforces the ceiling while streaming, preserves the byte-ceiling failure if cancellation cleanup fails, strictly decodes UTF-8/JSON, and the three Cloudflare callers no longer perform post-hoc response.text() sizing. Exact-head application CI, reviewer-ci, required Security Scan, and patch-validator-image are terminal GREEN, including repository-wide 100% coverage for the new production helper.
One valid completion finding remains: canonical CHANGELOG/operability documentation does not yet record this production control-plane resource-bound change. Keep Draft; do not promote source GREEN to merge/release authority until that documentation is code-current and the resulting final exact head independently re-passes the gates.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review on 9233a5cb5d706a4e8bef98fcf3622c92497c30b3: reviewed the shared bounded Cloudflare response reader, all three deploy/status/recovery call sites, hostile regressions, coverage inclusion, and the canonical CHANGELOG/OPERABILITY convergence. The three callers retain their 120s AbortSignal wall-clock bounds, so the new byte ceiling does not create an unbounded stalled-body path. The unrelated CHANGELOG wording drift introduced during documentation convergence was restored before this exact head; the current CHANGELOG diff is limited to the intended #618 entry. I found no additional source/test/DDD/owner-boundary/documentation finding on this exact head. This is a COMMENT review only, not self-approval; merge still requires terminal exact-head gates plus fresh base/head/thread verification.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Final exact-head review on fb166ec6ecc769a19fd4ae4502f6d0994fc89e33: reviewed the shared bounded response reader, all three production Cloudflare deploy/status/recovery call sites, 120-second AbortSignal bounds, hostile byte/stream/UTF-8/JSON/provider-error tests, 100% coverage inclusion, CHANGELOG, OPERABILITY, and product/technical gap baseline convergence. The baseline correctly separates protected main@2434a1b9deca91e0ce09713479839e7e4042585a / #617 from active #618 and keeps ADR-0018 / production rehearsal / immutable release evidence open. The unrelated CHANGELOG drift was repaired and the current patch is scoped. No additional source/test/DDD/owner-boundary/documentation finding remains on this exact head, and current inline-thread inventory is empty. COMMENT only; this is not self-approval.
…ded-response-protected-20260911 docs(cloudflare): classify #618 as protected authority
1 participant
Problem
The production Cloudflare deploy/status/recovery clients define a 1 MiB response ceiling but called
response.text()before checkingBuffer.byteLength(...). The check therefore ran only after the entire provider body had already been buffered in memory, so an unexpectedly large or hostile control-plane response could exceed the intended resource boundary before rejection.This is a Noema-owned deployment/recovery transport concern. It does not move Cloudflare provider truth, routing, credentials, or outbound authority into another bounded context.
Test-first reality RED
Test-only exact
51ddaf8988e3678b2010a8ef8d6f7e0af88637a9added an executable regression requiring all three production Cloudflare control-plane callers to stop using post-hocresponse.text()sizing and consume a bounded streaming JSON response helper instead.Hosted application CI
34584814994checked out that exact head, passed checkout/base/toolchain/install/typecheck, then failed atrelease tests. This is the reality RED for the protected-source defect; downstream application-CI steps were correctly skipped.Causal repair
The source repair introduces
scripts/lib/cloudflare-response.mjs, which readsresponse.bodyincrementally and refuses bytes beyond the configured ceiling before accepting the over-limit chunk; keeps cancellation failure subordinate to the authoritative byte-ceiling failure and always releases the stream reader lock; uses fatal UTF-8 decoding and fail-closed JSON/provider-error admission; and replaces the duplicated post-hoc parser in deploy/status/recovery without changing Cloudflare mutation, routing, identity, or credential authority.Hostile tests cover exact-boundary success, missing/non-stream body, malformed chunk, oversized body, cancellation failure, invalid UTF-8, non-JSON, provider-declared errors and HTTP errors. The production helper is included in the repository-wide 100% statements/branches/functions/lines coverage gate. All three production callers retain a 120-second
AbortSignal.timeout(...), and the regression now pins that wall-clock contract as well as the streaming byte bound.Documentation convergence
Current candidate exact
fb166ec6ecc769a19fd4ae4502f6d0994fc89e33records the streaming-time resource bound inCHANGELOG.md,docs/OPERABILITY.md, anddocs/product-technical-gap-baseline.md. The baseline now distinguishes protectedmain@2434a1b9deca91e0ce09713479839e7e4042585a/ protected #617 history from active #618 candidate authority and keeps ADR-0018Proposeduntil immutable release plus controlled production recovery evidence exists. The executable transport regression also binds these canonical documentation surfaces to the source contract.During documentation convergence, an unrelated historical CHANGELOG wording drift introduced by a full-file update was detected in review and restored before the current exact; the current CHANGELOG patch is limited to the intended #618 entry.
Current gate
Predecessor exact-head GREEN is not merge authority after the documentation/test mutations. Final exact
fb166ec6ecc769a19fd4ae4502f6d0994fc89e33must independently receive terminal application CI, reviewer-ci, required Security Scan, and patch-validator-image success plus fresh COMMENT-only review, zero unresolved threads, unchanged base/head, and normal merge eligibility.Keep this PR Draft until those final-exact conditions are satisfied. Source-level integration is not deployment, recovery-rehearsal, SLO, or immutable-release evidence.
Summary by CodeRabbit
개선 사항
문서