fix(destination): require fresh resolution on redirects - #92
Draft
seonghobae wants to merge 3 commits into
Draft
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Buyer/security gap
Stacked follow-up to #47. The prerequisite introduces bounded
FreshResolutionSnapshotauthority, but its existingRedirectGuard::authorize_redirectstill accepted the untimedResolutionSnapshot. That left redirect hops as a first-party bypass: a target resolution could be origin-matched yet used after its freshness window expired.Dependency
This Draft remains stacked on unchanged exact #47 head
6b5ed4dcea281b505f67db6180bb14c3bc95b392and must remain Draft while #47 is active. No prerequisite check/review is transferred to this branch.TDD / RCA sequence
98bb8ee99ec685f19091d6b6094ac7861872292badded a realistic regression that gives a redirect target a two-secondFreshResolutionSnapshot, attempts authorization exactly at the exclusive validity deadline, and requires the hop to fail without mutating redirect-chain state.31523936438, Rust contracts job93887508525, passed repository contracts and canonical formatting, then reached the intended production boundary:cargo check --locked --workspace --all-targetsfailed becauseauthorize_redirectstill accepted&ResolutionSnapshotwithout trusted current time (E0061) andRedirectError::ResolutionFreshnessDenieddid not exist (E0599). This is the valid RED.5516055bd70e687d6d76dc8660e90f003d3c0432added the narrow freshness composition and migrated the existing redirect tests. CI run31524429339then stopped at one canonical rustfmt hunk inredirect_policy.rs; workspace/tests were skipped, so that head is not counted as GREEN.b796564d059f7bcbd8177617b6fd46c6edc7dda1applies exactly the reported rustfmt delta without changing behavior.31524574783is now fully GREEN onb796564d059f7bcbd8177617b6fd46c6edc7dda1: Rust contracts job93889674164passed repository contracts, formatting, locked workspace/all-target check, full tests, strict Clippy and rustdoc; Production coverage job93889674208passed exact owned production function/line/region/branch enforcement. CodeRabbit exact-head commit status issuccess; GitHub reports the Draft mergeable, and no formal reviews or inline review threads are currently returned.No cancelled/predecessor job, synthetic merge result, or prerequisite evidence is promoted as current proof.
Implemented boundary
RedirectGuard::authorize_redirectnow:FreshResolutionSnapshotrather than an untimed resolution snapshot;current_timefrom the same clock domain as resolution approval;DestinationError::ResolutionUseBeforeApproval/ResolutionApprovalExpiredoutcomes asRedirectError::ResolutionFreshnessDenied, preserving the standard error source.Focused regressions prove exact-boundary expiry, pre-approval use rejection, no chain advancement on freshness denial, stable credential-free error text/source, and migration of every existing redirect authorization path through fresh resolution authority.
Truth boundary
This is deterministic per-hop destination authority only. It performs no DNS lookup, socket I/O, HTTP redirect following, wall-clock read, proxy/PAC execution, TLS/browser/model operation, persistence, or runtime clock attestation. Constructing a
FreshResolutionSnapshotdoes not prove the resolver or clock is trusted; the higher-layer resolver/network/browser adapter must supply independently validated resolution evidence and current time from the same trusted monotonic domain.The active
CHANGELOG.mdpath is currently touched by another OriginWeave writer lane, so this branch does not race that path. Canonical TM-002 already requires per-hop redirect authorization; no new trust domain or persistence owner is introduced by this bounded correction.This PR is a partial hardening slice under #28/#47 and does not close either issue. Keep Draft while #47 remains active; any prerequisite or current-head movement requires fresh exact-head/live-base validation.