feat(s5-4d): fetch opaque-origin isolation via broker SecurityOrigin type-unification#448
Merged
Conversation
…type-unification
Route the fetch request origin through the engine's origin type so a sandboxed
document's opaque origin is carried faithfully (identity-stable) into the broker,
where it strips credentials/cookies and serializes the Origin header as "null".
- elidex-net broker contract: `Request.origin: Option<url::Origin>` →
`Option<SecurityOrigin>` (elidex-net already depends on elidex-plugin; zero new DAG
edges). `should_attach_cookies` / `should_store_set_cookie_from` SameOrigin arm ⇒
attach iff `*source == SecurityOrigin::from_url(&url)` — opaque ≠ tuple BY TYPE, so
credentials strip structurally (not a scheme branch); `None` embedder carve-out
unchanged. Preflight/Origin-header serialize via `SecurityOrigin::serialize()`
(opaque → "null") replacing `url::Origin::ascii_serialization`.
- VM fetch (elidex-js): re-key the lone settings-origin hold-out `origin_for_request`
→ `fetch_request_origin` = `VmInner::document_origin()` (every sibling reader —
postMessage/WS/SSE/storage/history — already routes through it). `FetchCorsMeta`,
`reject_same_origin_cross_origin`, `attach_default_origin`, `cors_check_passes` follow.
- Redirect-taint (E7): existing `redirect_tainted` stays the OUTER clause; opaque makes
the inner same-origin equalities false too. Two producers preserved, not collapsed.
- worker/SW construction sites convert via `from_url` (tuple script origins →
behavior-neutral). boa unchanged (`..Default::default()` → `origin: None`).
- §4.5 storage-home-neutral: zero new per-VM side-store; predicates are pure fns over
the origin value → survive the B1 HostData→component migration verbatim.
Tests: net `should_attach_cookies` matrix {tuple-same/tuple-cross/opaque/None} +
opaque-never-stores + opaque-preflight-null; VM sandboxed-doc fetch = opaque origin,
identity-stable across two fetches, SameOrigin-from-opaque rejected, E7 tuple-taint "null".
Plan-memo §4.4/§5.4 (docs/plans/2026-07-s5-4-sandbox-enforcement.md).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…op redundant dispatch clone - Extract the repeated `*origin == SecurityOrigin::from_url(url)` compare shape into `SecurityOrigin::same_origin_with_url(&self, &url::Url)` (elidex-plugin origin.rs) and adopt it at all 7 compare sites (cookie attach/store, preflight is_same_origin, CORS classify, fetch reject/attach). Same-origin equality now lives in exactly one place (One-issue-one-way); `&& !redirect_tainted` compositions stay at the call sites. - fetch dispatch: drop the redundant second `SecurityOrigin` clone (2→1) in both paths — move the origin into `Request.origin`, clone once for `FetchCorsMeta.request_origin`, and read `request.origin.as_ref()` in `attach_default_origin` (param dropped). - Correct the `fetch_request_origin` doc: `document_origin()` is the canonical resolver for the Window + Service Worker realms, but a dedicated worker's fetch carries the about:blank opaque "null" origin (run_worker_with_source doesn't seed current_url) — pre-existing, deferred to slot #11-dedicated-worker-settings-origin-seed. No behavior change (elidex-plugin 357 / elidex-net 301 / elidex-js 6263 tests pass). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ility
Pre-push gate (/elidex-review + /code-review high) findings, all doc/comment —
no behavior change (elidex-plugin 357 / elidex-net 301 / elidex-js 6263 unchanged):
- elidex-review Spec MIN: cite HTML §7.1.1 ("same origin") on `same_origin_with_url`;
fix a stale non-existent "Fetch §3.1.7" in `fetch_request_origin` → the correct
Fetch §4.6 "HTTP-network-or-cache fetch" step 21.1 (webref-verified).
- elidex-review Project-context: mark plan-memo §5.4 (S5-4d) ✅ LANDED; register the
new carve D5 `#11-dedicated-worker-settings-origin-seed` in §8 with full audit.
- code-review F2 (PLAUSIBLE, dormant): note that re-keying fetch onto document_origin()
shares the load-time-override navigation-staleness concern with every settings-origin
reader (postMessage/WS/SSE/storage/history) — deferred #11-vm-navigation-origin-resync;
the re-key makes fetch CONSISTENT, not fetch-unique. Dormant until the S5-6 flip.
- code-review F1 (CONFIRMED): note that an opaque-origin doc sending Origin: null still
emits a tuple Referer (compute_default_referer derives from source URL) — NOT an
S5-4d regression (Referer leaked identically pre-S5-4d; Origin isolation is a strict
improvement); opaque-origin Referer suppression is the referrer-policy surface →
deferred #11-referrer-policy.
Both code-review findings = valid defers to EXISTING slots (genuine cross-surface /
cross-subsystem boundaries), not "edge" — traceability citations added at each site.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…view TERMINAL fix-delta The opaque-initiator test body was migrated to SecurityOrigin::Opaque / serialize() (S5-4d) but its lead comment still named the pre-migration url::Origin API (Origin::Opaque, ascii_serialization). Mechanical token propagation caught by the TERMINAL /elidex-review Axis-4 sweep; comment-only, no behavior change (cargo check -p elidex-js --all-features clean). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7debf4535d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ight cache key PreflightCacheKey.origin was request.origin.serialize(), collapsing every opaque origin to "null" — so two distinct sandboxed / data: documents sharing a NetClient shared one preflight cache entry, letting one document's OPTIONS result satisfy another's. S5-4d had just made Request.origin identity-stable (SecurityOrigin::Opaque(u64)); the cache key was the one place still discarding that identity. Fix: key on the SecurityOrigin itself (Eq + Hash) rather than its serialization. Distinct opaque origins now get distinct keys (isolated by construction); the same opaque document's identity-stable origin still shares its own entry; tuple origins are unaffected (equal tuples stay Eq). Spec-permissible: Fetch §4.9 keys the cache on a byte-serialized origin (null for opaque) + a network partition key which elidex does not model — the cache is an optimization, so extra preflights (never a wrong result) are always allowed. Regression tests: distinct_opaque_origins_do_not_collide / same_opaque_origin_shares_cache_key. elidex-net --all-features green, clippy clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
send
added a commit
that referenced
this pull request
Jul 4, 2026
…-commit; defer task-queued model Codex R4 surfaced 3 P2 on content/navigation.rs — the 4th consecutive round on the history-drain-then-navigate mechanism, findings in mutual tension with R2/R3. This fired the #396 self-root-check. An adversarial root-cause re-derivation (+ webref §7.4 verification) confirmed the mechanism is an edifice: a traversal is a QUEUED task in the spec (WHATWG HTML §7.4.3 traverse the history by a delta appends to the traversable's session-history traversal queue; §7.4.6.1 apply the history step runs within it; §7.4.4 URL and history update steps run synchronously in the current task), but elidex drains all same-turn intents in ONE synchronous pass, treating a traversal as a synchronous pipeline-rebuild mid-FIFO-drain. R1/R2/R3 + #259/#283/#448 + the carved E7 race are 7 clauses approximating that one missing task boundary. Disposition = minimize 5a + defer the model (not collapse-in-5a: the task-queued model is edge-dense, needs its own plan-review; not patch-the-3: that is more edifice). Reachability (code-traced): boa's single-slot back-channel makes the drain ≤1 action/turn, so #259 (multi-action FIFO drop) is post-flip-only and #448 (reentrant restore clobber) needs the dead SW path; #283 is the ONE live-reachable finding and is 5a's own regression. - #283 FIXED: break → return true on a superseding traversal, so a successful traversal returns immediately and never falls through to take_pending_navigation() (which would drain a location.* the freshly-rebuilt page's initial scripts queued). Preserves E7 "traversal wins", restores pre-5a symmetry. Both drivers (content + app). - Cursor: R3's eager-move + restore_index replaced by peek-then-commit (NavigationController::peek_back/peek_forward/peek_go + commit_index; the cursor moves iff the load succeeds — atomic by construction, no speculative move for #448 to clobber). current_index/restore_index retired. go_back/go_forward/go collapse to thin peek+commit wrappers (One-issue-one-way). commit_index debug_asserts the in-range peek-then-commit invariant (elidex-review Axis 2). - #259 + #448 deferred to the reframed §8-D5 slot #11-session-history-task-queue-model (from #11-traversal-navigation-same-turn-race): implement the spec's task-queued traversal (apply the history step as a deferred post-drain content-thread task). One slot subsumes #259/#283/#448 + E7; lands in a plan-reviewed 5c/5d (edge-dense). §4.1 amended: the collapse correctly drops the multi-navigable serialization but must KEEP the single-navigable task boundary (its omission is the root). - boa relative-nav base stays deferred to the S5-6 flip (unchanged, §3.2). #283 success-path (successful traversal → fresh page's nav un-drained) is not unit-producible in the disconnected harness (load always Errs); the complement (failed traversal → drain continues → nav drains) IS pinned, and the discriminating success assertion is registered as connected/flip integration coverage (S5-4b precedent). 3 peek-then-commit unit tests added. mise run ci green. /elidex-review 5-axis design re-gate 0 CRIT / 0 IMP (2 borderline-FP MINs: commit_index assert → fixed; #283 success-path coverage → registered integration deliverable). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
send
added a commit
that referenced
this pull request
Jul 5, 2026
…toryCursorOp)
Codex R5: in a successful JS traversal, handle_navigate rebuilt the pipeline and
called notify_navigation (which ships ContentToBrowser::NavigationState —
can_go_back/can_go_forward from nav_controller) BEFORE the caller's commit_index
moved the cursor, so history.back() reported stale chrome state. This is a
regression R4's peek-then-commit introduced (R3's eager-move committed before
handle_navigate; peek-then-commit moved the commit after). A fresh nav already
push()es INSIDE handle_navigate before notify → the asymmetry was the bug.
Fix: thread the cursor op INTO handle_navigate so it commits before notify,
symmetric with the push.
- New enum HistoryCursorOp { Push, Commit(usize), Keep }; handle_navigate takes
it instead of `is_history_nav: bool`. In the Ok branch, before notify:
match { Push => push(url), Commit(i) => commit_index(i), Keep => {} }.
- Callers: fresh nav → Push; JS traversal (handle_history_action) → Commit(target)
via peek, caller-side commit_index removed; chrome GoBack/GoForward (already
eager-moved via go_*) → Keep; Reload → Keep. go_back/go_forward/go retained
(chrome + tests). iframe-local handle_navigate is a different fn, untouched.
- App/inline path LEFT AS-IS (no bug): it sends no nav_controller-derived
NavigationState IPC; the inline chrome reads can_go_back/forward LIVE at egui
render time, after the synchronous caller-side commit_index — verified, so the
stale-IPC vector cannot exist there.
Step 4.5 independent re-check (given R4→R5, the fix itself was re-screened) — 5/6
CLEAN + validated the app decision; 1 MIN: the "atomic by construction" claim
overstated — handle_navigate's SW-fetch synchronous message pump could stale the
held peek index before commit (same SW-reentrancy root as #448; unreachable today
— SW controller path dead; debug_assert-backstopped). Dispositioned doc-only:
scoped the navigation.rs claim to the non-reentrant path + folded the SW-pump
held-peek vector AND the pre-existing chrome-eager failed-load atomicity gap into
the deferred slot #11-session-history-task-queue-model (§8-D5). Both re-land at
the SW-interception / async-event-loop wiring (M4-10); the task-queued model
unifies all traversal cursor commits.
R5 success-path (post-commit NavigationState on a successful traversal) is not
unit-producible in the disconnected harness (same boundary as #283); the
failed-load complement + the enum success-gating are pinned; the success
assertion is registered as connected/flip integration coverage.
mise run ci green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
send
added a commit
that referenced
this pull request
Jul 5, 2026
…tion (#449) Drain same-turn history actions before the navigation drain (history-first) + canonicalize the drained history to a Vec, so `pushState('/a'); location.href='/b'` commits `/a` instead of stranding it. Converged via Codex /external-converge (7 rounds → TERMINAL). Fixes on the live-reachable surface: - #283: a superseding traversal returns immediately, so it never falls through to drain a `location.*` the freshly-loaded page's initial scripts queued. - peek-then-commit cursor atomicity (by construction; retires the R3 restore_index rollback that Codex #448 flagged). - HistoryCursorOp (Push/Commit/Keep): commit the session-history cursor INSIDE handle_navigate, before notify_navigation, so a successful traversal ships the moved can_go_back/can_go_forward to the chrome (Codex R5). The R1-R4 history-drain churn was diagnosed (root-cause re-derivation + webref §7.4) as a #396 edifice approximating the spec's task-queued traversal (a traversal is a queued task — HTML §7.4.3/§7.4.6.1 — not a synchronous mid-drain rebuild). Rather than accrete a 5th/6th reconciliation clause, 5a fixes only the live-reachable facets and defers the faithful task-queued-traversal model to a plan-reviewed slice: slot #11-session-history-task-queue-model (§8-D5), which subsumes #259 (post-flip multi-action FIFO), #448 + the SW-fetch message-pump reentrancy vector, the pre-existing chrome-button atomicity gap, and the E7 race. The boa relative-nav base stays deferred to the S5-6 flip (a one-order fix regresses the other same-turn order; the VM is correct by construction). Plan-memo docs/plans/2026-07-s5-5-navigation-history-enforcement.md rides this PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Route the fetch request origin through the engine's origin type so a sandboxed document's opaque origin is carried faithfully (identity-stable) into the broker, where it strips credentials/cookies and serializes
Origin: null. The last S5-4 sandbox slice.Request.origin: Option<url::Origin>→Option<elidex_plugin::SecurityOrigin>(zero new DAG edges).should_attach_cookies/should_store_set_cookie_fromSameOrigin ⇒ attach ifforigin.same_origin_with_url(url)— opaque ≠ tuple BY TYPE (credentials strip structurally);Noneembedder carve-out unchanged. Origin/preflight serialize viaSecurityOrigin::serialize()(opaque → "null").origin_for_request→fetch_request_origin=document_origin()(every sibling reader already routes through it). Redirect-taint E7 outer clause preserved.SecurityOrigin::same_origin_with_urlpredicate centralizes same-origin equality (one home); dispatch clone 2→1. boa unchanged (origin: None).Closes deferred slot
#11-sandbox-fetch-opaque-origin-isolation. Plan-memo §4.4/§5.4.Deferred (both valid cross-surface/subsystem defers to existing slots, not regressions — traceability citations in-code):
#11-referrer-policy: an opaque-origin doc now sendsOrigin: nullbut the Referer path still derives from the tuple source URL. NOT an S5-4d regression (Referer leaked identically pre-S5-4d; Origin isolation is a strict improvement); opaque-origin Referer suppression is the referrer-policy surface.#11-vm-navigation-origin-resync: shared with every settings-origin reader; dormant until the S5-6 flip drives current_url on a persistent VM. The re-key makes fetch consistent, not fetch-unique.#11-dedicated-worker-settings-origin-seed(plan-memo §8 D5): dedicated-worker fetch carries about:blank opaque origin (run_worker_with_source doesn't seed current_url); pre-existing, cross-concern.Pre-push gate: fmt + mise run ci (green ×3) + /simplify + /code-review high + /elidex-review 5-axis (code design-clean; all findings doc/citation/slot, fixed).
🤖 Generated with Claude Code