feat(http): rebuild bounded HTTP/1.1 authority on current main - #37
feat(http): rebuild bounded HTTP/1.1 authority on current main#37seonghobae wants to merge 230 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesBounded HTTP/1.1 교환
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Commit-Message-Assisted-by: Claude (via Claude Code) Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Exact-head repair and verification for |
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head framing finding on 357fa97dc3cd4a3afff78c29151392b50be6f50a: the new 205 repair currently conflates representation/content semantics with HTTP/1.1 message-body framing.
RFC 9110 §15.3.6 says a server MUST NOT generate content in a 205 response. However RFC 9112 §6.3's framing precedence lists only HEAD, 1xx, 204, and 304 as responses terminated at the header boundary regardless of framing fields; 205 is not in that exception. For 205, Transfer-Encoding / Content-Length / close-delimiting still determine the HTTP/1.1 message-body boundary, after which the semantic rule can reject any non-empty content.
Current determine_body_framing() instead places 205 beside 204/304 and returns BodyFraming::NoContent before applying a valid Content-Length/Transfer-Encoding framing result. read_network_response() then checks only body_prefix and returns success immediately without another read. A malicious/non-conforming peer can therefore send 205 + Content-Length: 1, flush the headers, delay one byte, and have OriginWeave return a successful zero-content response before the prohibited byte arrives. Because evidence then records NoContent, this is an evidence-integrity defect even though the connection object is single-use.
Please make the next step a realistic TLS loopback RED: send HTTP/1.1 205 Reset Content\r\nContent-Length: 1...\r\n\r\n, flush only the head, delay, then send one byte. The exact current implementation should demonstrate the premature-success failure before production repair. Minimal causal direction: keep RFC 9112 wire framing separate from a 205 forbids content semantic invariant; read the declared/chunked/close-delimited message correctly, then reject non-empty 205 content rather than treating 205 as the same framing class as 204/304. Also correct docs/doctoring/http11-security-evidence.md, whose current statement that 205 status semantics determine no wire content ahead of RFC 9112 framing is not accurate for HTTP/1.1.
Do not weaken the existing HEAD/1xx/204/304 framing protections or surplus-byte fail-closed behavior. Current hosted checks are non-terminal, so this is a repair finding, not a request to close the valid HTTP delta.
Read Reset Content responses according to HTTP/1.1 wire framing, then reject non-empty content so segmented payloads cannot produce premature success. Commit-Message-Assisted-by: Claude (via Claude Code) Signed-off-by: Seongho Bae <me@seonghobae.me>
|
현재 head |
Add a realistic TLS loopback regression proving a complete Content-Length response finishes without waiting for HTTP/1.1 transport closure. Signed-off-by: Seongho Bae <me@seonghobae.me>
Record the RFC 9112 framing and persistence evidence behind the realistic keep-alive regression so the Ready synchronize generation receives repository-native verification. Signed-off-by: Seongho Bae <me@seonghobae.me>
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head review for e645df91f1f591f462a8ec09e2f4d57b5fb0def9: valid framing finding remains open. RFC 9112 §6.2/§6.3 makes a valid Content-Length the response-message boundary; §9.3 permits the transport to remain persistent after a self-delimited message. Current read_exact_content() still performs an extra terminator read after receiving the declared octets, so a normal persistent response can time out after it is already complete. The new TLS loopback regression holds the connection open beyond the HTTP deadline to exercise that exact behavior. First test commit ran under Draft and native Rust/coverage jobs were skipped; this Ready synchronize generation has fresh CI/Security/Semgrep/CodeQL queued. Do not repair production or claim RED/GREEN until repository-native execution is terminal. Minimal eventual fix: return after exact Content-Length bytes, retain incomplete-body and already-buffered surplus rejection, never reinterpret trailing bytes as a second response.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head review for fa34a696e4ed9b2529a65a7d6a45ac8b72ecefb3: the Content-Length persistence repair is source-minimal and preserves the reviewed framing/security boundaries. read_exact_content() no longer waits for TLS EOF after the exact declared octet count; incomplete bodies and already-buffered surplus still fail closed, and 205 semantic rejection remains downstream of ordinary wire framing. The realistic TLS keep-alive regression remains the causal acceptance test. No GREEN is claimed until repository-native CI/coverage/security materialize and finish on this unchanged head. CHANGELOG.md still needs a narrow Unreleased persistence note before documentation completion.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head review for 5f339088c652e51b630d05f2a352c983d9e5fb32: the stale segmented-surplus contract has been reconciled with the same RFC 9112 self-delimiting message boundary as the production repair. The realistic TLS server still sends Content-Length: 1, the exact x body, delays, and attempts a later y; the client must complete the single-use response at the declared boundary and must never reinterpret later bytes as current content or a second response. This does not weaken already-buffered surplus rejection or incomplete-body handling. Current exact CI/security runs are non-terminal, so no GREEN is claimed. CHANGELOG.md still needs the narrow Unreleased persistence note.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head review for 28721edcc67bb5379ffb11a259d746396ac7ae03: the missing Content-Length persistence release-note boundary is now guarded test-first by tests/test_http_content_length_persistence_documentation_contract.py and recorded under Unreleased. The contract remains tied to RFC 9112 §§6.2/6.3 and the existing real TLS regression (100 ms HTTP deadline, 350 ms peer hold, BodyFraming::ContentLength(5)). This documentation generation does not change HTTP runtime semantics or authority. Fresh CI 33940886121, Security 33940886172, Semgrep 33940886124, and CodeQL 33940886119 are non-terminal, so no GREEN/merge claim transfers from predecessor heads.
Commit-Message-Assisted-by: Claude (via Claude Code) Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Exact-head repair and verification: The current head reproduced a Rust 1.97.1 Hosted exact-head checks and an eligible approval remain required; this is not merge authorization. |
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head review for 1e2f41072854edcdbaf0f9ecf14697a3bfd62195: the delta from 28721ed... is formatting-only in content_length_segmented_surplus.rs; it does not alter HTTP framing, timeout, policy, browser authority, or workflow semantics. The Content-Length persistence acceptance remains exact declared-byte completion without transport-EOF dependence, while short bodies and already-buffered surplus remain fail closed. Current CI/Security/Semgrep/CodeQL are non-terminal, so predecessor evidence does not transfer and no GREEN/approval claim is made.
|
Scheduled review-feedback autofix for this PR head.
|
|
Scheduled review-feedback autofix for this PR head.
|
Purpose
Canonical non-destructive reconstruction of issue #9 / historical PR #11: one bounded reusable
originweave-httpHTTP/1.1 authority over an existing authenticated TLS stream, with strict framing, resource budgets, integrity/MIME/disposition evidence, realistic loopback tests, ADRs, and repository quality contracts.Current protected-base topology
Protected base is
main@87c4daa1830bac5a5228b6036752ad5633232085(#286). Current exact head is1e2f41072854edcdbaf0f9ecf14697a3bfd62195, open, Ready, mergeable, and directly based on current protected main. This branch contains no scheduled-writer.github/**, ruleset, secret, provider/model, tag, release, or publication mutation.Content-Length persistence repair
RFC 9112 §§6.2–6.3 make a valid
Content-Lengthself-delimiting for the current HTTP/1.1 message: closure/timeout makes the response incomplete only when it occurs before the declared octet count. The realistic TLS regressionpersistent_content_length.rsreturnsContent-Length: 5, exactlyhello, then keeps TLS open for 350 ms while the exchange deadline is 100 ms. Test-onlyf70a81d8da15ab418c7c667db9c727dd089bd472exposed the prior post-length sentinel wait. Minimal production repairb21bfd6b7b766df31dc1e43733a183249fc6230dremoves only that sentinel read. Fewer-than-declared bytes, already-buffered surplus, Transfer-Encoding/Content-Length ambiguity, bounded content decoding, and no-content semantic checks remain fail closed.The older segmented-surplus test had depended on the removed sentinel after the current response was already complete.
5f339088c652e51b630d05f2a352c983d9e5fb32retains the realistic delayed-byte TLS scenario but asserts the RFC-correct current-message result and then disposes the single-use connection; OriginWeave does not pool, cache, forward, or parse the delayed byte as another response.docs/doctoring/http-content-length-persistence.mdrecords the RFC decision and evidence boundary.6e2c7b9d26a38341247c141354f44e15f96d99e8established the changelog documentation RED,28721edcc67bb5379ffb11a259d746396ac7ae03added the Unreleased record, and current1e2f41072854edcdbaf0f9ecf14697a3bfd62195is the formatting-only repair. All currently returned inline review threads are resolved.Exact-current hosted evidence
The formerly queued exact-current repository lanes are now terminal on the unchanged head:
33946243438: success;33946243410: success;33946243423: success;33946243437: failure.The CodeQL failure is the organization central-dispatch/verdict control-plane pattern rather than an observed HTTP source finding. Detect-languages job
101252742547succeeded. Actions101285593221, python101285593227, and javascript-typescript101285593246each successfully completedRequest current-head CodeQL scan dispatchand then failed atRelease runner or enforce current-head CodeQL verdict. This recurrence, together with #50 and #229, is handed to canonical organization issueContextualWisdomLab/.github#712in comment5555411125. Keep the repository leaf fail-closed; do not duplicate central scanning or blind-rerun the unchanged head.Exact-current eligible independent approval and live ruleset satisfaction remain separate. No predecessor, queued, skipped, absent, model-only, author-only, or status-only evidence is promoted to passing acceptance.
Authority boundary
This PR owns bounded HTTP/1.1 semantics over an already authenticated TLS stream; it does not own destination authorization, DNS freshness, TLS authentication, browser control, workflow control, or release authority. Keep Ready but unmerged until the central required verdict and live review/ruleset gates are satisfied. No self-approval, bypass, force-push, destructive rebase, workflow/ruleset/secret mutation, tag, release, or publication is authorized. Issue #9 remains open until this canonical HTTP authority reaches protected main through normal governance.