feat(s5-4a): canonical sandbox predicate home (elidex_plugin::sandbox) + §8.1.8.1 event-handler processing step-1 gate#444
Conversation
…ewed) Q2 resolved: 5-slice sub-split (4a predicate home + §8.1.8.1 gate / 4b iframe origin ordering / 4c method gates + window.open / 4d fetch opaque-origin / 4e worker MessageEvent origin) under one plan-reviewed memo. Predicate home = elidex-plugin (DAG-derived); Request.origin unifies on SecurityOrigin; window.open routes through an engine-indep WindowOpenDisposition fn with a call-time aux-nav snapshot verdict gating the shell named-miss promotion. /elidex-plan-review converged: R1 5-agent (1C/2I/11M) -> fixes -> focused re-checks (1I/6M) -> fixes -> final verify (1M) -> fix. All findings fixed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…) + §8.1.8.1 event-handler processing step-1 gate
Predicate unification (One issue, one way): the three duplicate
`sandbox_flags.is_none_or(contains(X))` bodies — VM HostData
(scripts/forms/popups_allowed), boa iframe_bridge (same trio;
modals_allowed stays boa-only until S5-4c/S5-6), and the session-trait
surface (doc-pointer update; the trait has declarations only) — now
delegate to one engine-independent home, `elidex_plugin::sandbox`
(§7.1.5 content-sandbox capability predicates + the §8.1.3.4
settings-level `scripting_enabled` composition), co-resident with the
existing process-sandbox policy types (module doc separates the two
planes). Truth-table unit tests pin the None / Some(empty) / per-bit
contract.
New INVOKE gate (the missing half of §8.1.8.1): "the event handler
processing algorithm" step 1 ("If scripting is disabled for
eventTarget, then return") now gates every handler-derived invocation
path — the dispatch-walk callable-resolve seam
(`DispatchTarget::resolve_callable`), the session UA-dispatch
(`ScriptEngine::call_listener`), and the `unhandledrejection` UA-fire —
via `VmInner::scripting_disabled_for_platform_object` /
`event_handler_invocation_suppressed` (marshal-scale composition;
settings-level rule from elidex-plugin). §9-Q3 resolved
"representable now": clause (b) ships via the bound-document proxy
(node document ≠ bound `document_entity` ⇒ null browsing context in the
single-BC VM model; Document-node and non-Node targets handled;
unresolvable owner fails open). Clause (c) documented never-fires while
bound. §8-D3 NOT carved. The existing COMPILE gate (get-current-value
step 3.2) keeps its flag-only predicate — two gates, two predicates
(memo §2.7-C1 / E1); plain addEventListener listeners are never
suppressed. Step-1 suppression preserves the IDL getter's callable
value.
Closes slot #11-scripting-disabled-eventhandler-processing-step1
(marker comment replaced by the spec-cited gate). Memo updated: §9-Q3
RESOLVED / §8-D3 NOT CREATED / §5.0 host_data.rs touch-time seam
assessment = no split (single cohesive struct+impl; delegation reduces
lines).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ms, is_handler threading Reviewed /simplify fixes on top of the S5-4a predicate-home commit: 1. Module relocation: OS process-sandbox types (SandboxPolicy / PlatformSandbox / SandboxError / Filesystem+NetworkAccess) move to elidex_plugin::process_sandbox; sandbox.rs becomes the cohesive content-sandbox module — IframeSandboxFlags + parse_sandbox_attribute (moved from origin.rs, tests included) alongside the §7.1.5 capability predicates. Root re-export paths unchanged. 2. Doc trims: scripting_enabled §8.1.3.4 transcription → 5 lines; step-1 gate narration stated once on the canonical predicate, call sites reduced to one-line pointers; scripting_disabled_for_platform_object docstring trimmed to composition + fail-open rationale + one-line slot-cited caveats. 3. Hot path: is_handler threaded through ScriptEngine::call_listener from the dispatch plan's ListenerPlanEntry, replacing the per-listener EventListeners re-fetch; boa impl takes _is_handler (no invoke gate). 4. unhandledrejection loop captures is_handler into PendingListener at collection and checks the settings-level predicate directly (target = bound document ⇒ clause (b) cannot fire). event_handler_invocation_suppressed left with zero callers → deleted; the canonical comment lives on scripting_disabled_for_platform_object. 5. Plan-memo §4.1 Shape block notes the kickoff naming resolution (process_sandbox rename + flags/parser move). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…fied; pre-flip oracle substitution) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…1 gate precedes compile, inferred brand-check, canonical predicate convergence - parse_sandbox_attribute matches allow-* tokens ASCII case-insensitively (WHATWG HTML attr-iframe-sandbox: tokens are ASCII case-insensitive); truth-table test added - $8.1.8.1 processing step 1 (scripting-disabled gate) hoisted ABOVE the step-2 'getting the current value' compile at both gated dispatch sites (engine.rs call_listener / dispatch_target.rs resolve_callable), so a suppressed target's raw inline source never compiles during dispatch; pinned by suppressed_target_dispatch_does_not_compile_raw_inline_handler - scripting_disabled_for_platform_object brand-checks via node_kind_inferred (crate convention), closing the fail-open hole for NodeKind-less nodes of a null-BC document - unhandledrejection dispatch routes through the canonical scripting_disabled_for_platform_object predicate (one-issue-one-way), gated before its compile likewise - sandbox module doc names the open-coded S5-4c migration targets instead of implying no consumers exist Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…not a dangling future Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! 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". |
|
@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: 1d78c5ba77
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d78c5ba77
ℹ️ 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".
…e home + adopt-equivalent clause (b) Two adjudicated external-review findings: - F1 (Layering, Codex P1): the HTML §8.1.3.4 platform-object composition moved out of vm/host/ into engine-independent elidex_script_session::scripting::scripting_disabled_for_platform_object (pure fn over &EcsDom + bound document + sandbox flags, unit-tested). VmInner::scripting_disabled_for_platform_object is now a thin marshal wrapper (HostData reads + delegate; unbound-DOM guard keeps the VmObject-only dispatch path settings-level, as before). Call sites unchanged in shape. - F2 (over-suppression bug, Codex P2): elidex's insertion path lacks DOM §4.2.3 pre-insert adoption (append_child relinks without re-homing AssociatedDocument), so a DOMParser-parsed node appended into the bound document kept a stale owner and clause (b) wrongly suppressed its handlers. Clause (b) now resolves the node document adopt-equivalently: composed tree root == bound document (the isConnected query) => node document IS the bound document => not suppressed; only out-of-tree nodes fall through to the stale-owner comparison. Missing insertion-adoption carved as defer slot #11-cross-document-adopt-on-insert. VM regression tests: appended DOMParser node handler RUNS (fails pre-fix, verified); appended-then-removed suppressed again. Memo §5.1.2 carries the correction note. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1a8a1376f
ℹ️ 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".
…memo plan-review date F-R4a: replace the directional adopt-equivalent clause (b) with a single effective-node-document rule resolved from the composed tree root. The R2 directional fix only handled a foreign node appended INTO the bound tree; the mirror case (a bound-created node appended INTO a DOMParser/null-BC document, keeping a stale bound-document `AssociatedDocument`) was NOT suppressed though §8.1.3.4 clause (b) requires it. New rule: if the composed tree root IS a Document that root is the effective node document (connected), else fall back to `owner_document` (detached); suppress iff the effective node document != bound document; unresolvable → fail open. Both directions are symptoms of the missing DOM §4.2.3 insertion-adoption (`#11-cross-document-adopt-on-insert`). Adds the reverse-quadrant unit + VM tests (falsified against the directional rule). F-R4b: reword the §0 plan-review resolution to assert the ordering (plan-review converged BEFORE the first S5-4a implementation commit) and use 2026-07-02 (UTC commit date) so it no longer reads as future-dated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa6aa1dcd9
ℹ️ 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".
…dge (defers to #11-cross-document-adopt-on-insert) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35515cf82e
ℹ️ 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".
…sert in §8 + sweep future-dated provenance Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3fb4552bfe
ℹ️ 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".
|
@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". |
…e (b) is §8.1.3.4 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: edd5ec82e9
ℹ️ 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".
…e cross-document-move facet class (#11-cross-document-adopt-on-insert) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
1 similar comment
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! 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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. 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". |
1 participant
Summary
S5-4a — first slice of the S5-4 sandbox/security enforcement cluster (plan-memo
docs/plans/2026-07-s5-4-sandbox-enforcement.md§5.1, bundled in this PR; plan-reviewed via /elidex-plan-review 2026-07-03). Closes defer slot#11-scripting-disabled-eventhandler-processing-step1.elidex_plugin::sandbox= content-sandbox module (HTML §7.1.5) holdingIframeSandboxFlags+parse_sandbox_attribute+ capability predicates (scripts_allowed/forms_allowed/popups_allowed/scripting_enabled) — data + its laws in one module. The pre-existing OS process-sandbox types moved toprocess_sandbox.rs; all crate-root re-export paths unchanged.host_data.rs, boaiframe_bridge.rs, session-trait docs) — no behavior change, one-issue-one-way convergence.VmInner::scripting_disabled_for_platform_object= settings-levelscripting_enabled(flags)∧ platform-object clause (b), applied at the three handler-derived dispatch chokepoints (resolve_callable,ScriptEngine::call_listener,unhandledrejectionloop), before the step-2 compile per spec order.addEventListenerlisteners are never gated (step 1 is handler-specific).owner_document(node) != document_entityunder the single-browsing-context VM model (theis_connectedquery shape). D3 was NOT carved. Caveats documented in-code: detached-iframe docs unreachable (moot); template-contents false-negative rides#11-template-contents-owner-document.parse_sandbox_attributewas case-sensitive; tokens are ASCII case-insensitive perhtml#attr-iframe-sandbox(sandbox="ALLOW-SCRIPTS"now grants scripts).is_handlerthreaded throughScriptEngine::call_listener(plan-entry bit, no per-listener ECS re-fetch); boa impl = mechanical_is_handler(no invoke gate — boa is the deletion-bound parity baseline, D-26 PR7).S5-4c pointers (not in this PR, plan-reviewed split)
modals_allowed+ 2-flagtop_navigation_allowed(flags, activation)land in S5-4c WITH their consumers (VM natives + shell link-gate re-key); the module doc names the three open-coded migration-target sites.Gate
/pre-push 6-stage complete: fmt /
mise run ci×4 green / /simplify (4-agent, module-home + doc-trim + hot-path fixes applied) / /code-review high (workflow, 5 findings adjudicated & fixed: case-insensitive tokens, gate-before-compile hoist,node_kind_inferredbrand-check, canonical-predicate convergence, doc precision) / /review (7 angles, 0 actionable) / /elidex-review (5-axis: 0 CRIT / 0 IMP / 2 MIN both fixed).Also included: two plan-memo CORRECTION notes recorded at S5-4b impl contact (§3.3 OOP-template claim falsified; §5.2 pre-flip oracle substitution) — S5-4b/4e branches are staged separately.
Tests: elidex-plugin 353 / elidex-js 6229 / elidex-js-boa 330 / elidex-script-session 135, workspace clippy
-D warningsclean.🤖 Generated with Claude Code