Secure opaque-origin content + HTTP editor preview v2#56
Draft
erseco wants to merge 61 commits into
Draft
Conversation
The frontend shortcode and Gutenberg block embedded .elpx content with sandbox="allow-scripts allow-same-origin allow-popups", served same-origin. With allow-same-origin the arbitrary author HTML/JS can read the WordPress page's cookies/DOM and reach window.parent. Add an exelearning_iframe_sandbox_mode option (secure default | legacy): secure drops allow-same-origin so the content runs in an opaque origin and is isolated from the page; legacy restores the previous behaviour for environments that need it (e.g. WordPress Playground, whose service worker only serves same-origin documents). A single ExeLearning_Iframe_Sandbox helper owns the option and the per-mode sandbox tokens, consumed by both the shortcode and the block. Teacher mode used same-origin contentDocument access, which cannot run against an opaque iframe; in secure mode the desired state is carried on the iframe src (exe-teacher / exe-teacher-toggler) and the content proxy applies it server-side (hide-toggler style and mode-teacher class), so no parent-to-iframe DOM access is involved. The legacy contentDocument path is kept for legacy mode. No CSP change is needed: the proxy's existing default-src 'self' resolves to the serving origin and loads same-host subresources under the opaque origin (verified in a browser). An admin setting under Settings -> eXeLearning lets admins switch modes, and the Playground blueprint forces legacy. Adds unit tests for the helper, the shortcode and block sandbox tokens per mode, the secure teacher-mode src params, and the proxy's server-side teacher-mode injection.
Contributor
Test in WordPress PlaygroundTest the plugin with the code from this branch:
|
Adds the new "Security" settings card strings to the POT and translates them in all maintained locales (ca, ca_valencia, de_DE, eo, es_ES, eu, gl_ES, it_IT, pt_PT, ro_RO) so the untranslated-strings check passes. Also drops the redundant card subtitle and shortens the field help text.
… hardening)
Even with the secure iframe, the proxied content is served same-origin with an
executable CSP, so opening the raw /wp-json/exelearning/v1/content/{hash}/index.html URL
top-level (e.g. manual navigation) would run author JS as the WordPress origin. Add a
`sandbox allow-scripts allow-popups` CSP directive for HTML in secure mode so the
document keeps an opaque origin however it is loaded; legacy is unchanged. Extracted a
testable build_html_csp() helper.
Verified in wp-env: opening the raw content URL top-level now reports
window.origin === 'null' and document.cookie throws SecurityError, while the content
still renders.
Contributor
Author
Live verification (secure mode, default)Brought this branch up with
By the symmetry of the same-origin policy, the author content cannot read the WordPress DOM, cookies or nonce, nor reach The backing content-proxy hardening checks out: HTML responses carry External-embed compatibility (worth a docs note). Because the content runs in an opaque origin and
This is an inherent trade-off of opaque-origin isolation, not a bug — just worth documenting for authors. Note on #54 (asset proxy): both PRs touch |
In secure mode the .elpx content runs in an opaque-origin sandbox, so cross-origin players (YouTube/Vimeo) and PDFs render blank (the sandbox flag propagates to nested iframes; Chrome also blocks its PDF viewer without allow-same-origin). Promote those embeds to the embedding page: the content proxy injects a shim that replaces whitelisted-video / .pdf iframes with placeholders and reports their geometry via postMessage; a relay enqueued on the shortcode/block page validates + rebuilds the URL and overlays the real player inline over each placeholder. - assets/js/exe-embed-shim.js / exe-embed-relay.js: the shared shim + relay. - class-content-proxy.php: inject the shim into served HTML (secure only). - class-iframe-sandbox.php: embed_whitelist() + enqueue_embed_relay(). - class-shortcodes.php / class-elp-upload-block.php: enqueue the relay (secure). PDFs: local package PDFs always render; any https .pdf renders; same-origin PDFs must belong to this package (served as application/pdf, never executable HTML). Tests in ContentProxyTest + IframeSandboxTest. Verified live in wp-env: YouTube, Vimeo and remote + local PDF render inline; a non-whitelisted iframe is not promoted.
The shim runs inside the content, so it resolves each iframe src against the content location and reports the ABSOLUTE URL. The parent relay resolves URLs against the host page, so a relative src (e.g. a locally-packaged PDF) would otherwise be rejected. Keeps parity with the mod_exelearning + omeka shim.
Add a self-contained Playwright/Firefox end-to-end test that loads the real exe-embed shim and relay against a static harness (no WordPress runtime needed) and verifies that whitelisted video and local PDF embeds are promoted to inline players on the parent page while other origins are rejected. Document the external-embed flow and how to run the test in the README.
…ywright config The dedicated playwright-embed.config.cjs runs the external-embed e2e in Firefox against its own static harness; the main config (chromium + wp-env) was auto-discovering it and failing. Exclude it via testIgnore. Also assert the non-whitelisted host with an exact hostname check (new URL().hostname) instead of a URL substring match.
…av fix Bring the WordPress embed relay/shim in line with the canonical mod_exelearning source: - Add Dailymotion and EducaMadrid/Mediateca de Madrid external-embed providers (allowlist hosts + per-provider canonical-URL validators in the relay). - Clamp the relayed player overlay to the placeholder box (clickjacking defence in depth; the overlay already clips with overflow:hidden). - Add allow-forms to the secure sandbox tokens, including the response-level CSP sandbox directive, so the form-based eXeLearning iDevices can submit inside the opaque sandbox. Align the legacy tokens with the canonical set. - Fix a lingering external embed when the eXe content pages to another view: the in-iframe shim restarts its embed-id counter per page, so a reused id could keep the previous page's player; tag each player with its URL and replace it when a reused id maps to a different URL. Update the IframeSandbox and ContentProxy unit tests for the new tokens and hosts.
…) + embed policy
Bring the WordPress embed relay/shim in line with the canonical mod_exelearning DEC-0061
change: drop the host allowlist for the default 'open' policy and promote any iframe whose
src is https AND cross-origin to the WordPress host (rejecting same-origin, sub/superdomains,
IP/loopback/local hosts and userinfo); a 'strict' policy keeps the allowlist + per-provider
reconstruction. The promoted video player is sandboxed (allow-scripts allow-same-origin
allow-popups allow-forms allow-presentation; no top-navigation/modals) so an arbitrary embed
cannot redirect the tab while the cross-origin provider still renders; PDFs stay unsandboxed.
Add the D1 same-origin-landing guard and the D2 forged-message defence (promoted players
tagged data-exe-embed-player, excluded from the content-source lookup).
Add an exelearning_embed_mode option (open default, fail-safe to strict) + an "External embed
policy" select in the admin Security settings, inject {mode, whitelist} into the relay config,
and update the IframeSandbox tests. Mirrors mod's logic (no drift).
…pen default + exelearning_embed_mode option The 10 maintained .po files require every string translated; the new select labels were untranslated and no canonical pot regeneration is available here. The embed policy still resolves from the exelearning_embed_mode option (default open), so the feature is unchanged; a localized UI can follow.
- Relay/shim: port the trailing-dot FQDN-root host normalization (normalizeHost) so the served host in 'host.' form is treated as same-host and not promoted as a cross-origin player. - Relay: hoist the content-iframe rect read out of the per-embed loop (one reflow per sync) and adopt the canonical dual-export tail (Node-requireable for tests; browser auto-run unchanged). - Content proxy: drop the unused window.__exeEmbedWhitelist injection; Referrer-Policy same-origin -> no-referrer on served files. - Sandbox: ship the embed whitelist only in strict mode. - Add a Vitest relay unit suite (happy-dom) covering the structural gate, including the trailing-dot cases.
…-sandbox # Conflicts: # languages/exelearning-ca.mo # languages/exelearning-ca.po # languages/exelearning-ca_valencia.mo # languages/exelearning-ca_valencia.po # languages/exelearning-de_DE.mo # languages/exelearning-de_DE.po # languages/exelearning-eo.mo # languages/exelearning-eo.po # languages/exelearning-es_ES.mo # languages/exelearning-es_ES.po # languages/exelearning-eu.mo # languages/exelearning-eu.po # languages/exelearning-gl_ES.mo # languages/exelearning-gl_ES.po # languages/exelearning-it_IT.mo # languages/exelearning-it_IT.po # languages/exelearning-pt_PT.mo # languages/exelearning-pt_PT.po # languages/exelearning-ro_RO.mo # languages/exelearning-ro_RO.po # languages/exelearning.pot # tests/unit/ContentProxyTest.php
Reconcile teacher-mode handling with main (#58). main retired host-side CSS/JS injection in favour of the package's own ?exe-teacher=1 URL parameter, so the secure-iframe branch is aligned to the same contract: - public/class-shortcodes.php, includes/class-elp-upload-block.php: keep the secure-mode embed-relay enqueue and opaque-iframe rendering, but drop the legacy teacher injection. The selector is offered by appending ?exe-teacher=1 to the iframe src when teacher_mode_visible (or the legacy teacher_mode attr) is on. This rides through the secure-mode content proxy too: the package reads its own location.search even under the opaque origin, so no host injection is needed. - includes/class-content-proxy.php: retire inject_teacher_mode() and the exe-teacher-toggler / exe-teacher server-side rewriting. It injected the exact #teacher-mode-toggler-wrapper CSS that #58 retired and, worse, auto-activated mode-teacher on ?exe-teacher=1 — which contradicts core (the parameter only offers the selector, it never auto-reveals). Keep inject_embed_shim(). - Tests: update ShortcodesTest / ElpUploadBlockTest secure-mode cases to assert the param-based contract (exe-teacher=1, no exe-teacher-toggler, no contentDocument) and drop the ContentProxyTest inject_teacher_mode cases. Other conflicts: - package.json: union — main's @playwright/test and @wordpress/env bumps plus the branch's vitest + happy-dom (used by tests/js). Lockfile regenerated. - languages: msgcat union of both sides (main's strings authoritative, the branch's secure-mode admin strings preserved), .mo recompiled, JED .json taken from main (elp-upload.js is byte-identical to main), .pot unioned. CI regenerates references. The admin block-editor live preview (assets/js/elp-upload.js) keeps main's same-origin preview behaviour unchanged.
…067) Mirror the id-only channel (extractProvider/reconstructProvider) into the embed shim/relay. Add exe-media-policy.js + exe-media-host.js (vendored from mod_exelearning) and enqueue_media_host() in ExeLearning_Iframe_Sandbox, called from the block + shortcode renderers: parent-side host for the interactive-video iDevice via raw postMessage (no YouTube IFrame API/Vimeo SDK). No-op in legacy.
… on re-open M-3: makePlayer() rendered a cross-origin .pdf in a fully unsandboxed iframe, so an author-supplied https://evil/x.pdf serving HTML could top-navigate the host tab to a phishing page (reachable in both open and strict mode). Mirror the canonical Moodle 3-way branch: a same-origin package PDF stays unsandboxed (the browser PDF viewer needs it), a cross-origin PDF gets sandbox="allow-same-origin" (no allow-scripts, no allow-top-navigation). L-2: the modal media host appended a new <dialog> and overwrote session.adapter on every 'open' without tearing down the previous one, so repeated 'open' commands could stack modals and orphan provider players. openMedia() now discards the prior media/poll-timer/dialog first (single active media per session). Tests: cross-origin vs same-origin PDF sandbox; media-host single-active teardown.
…nly escape hatch
The content iframe is now always opaque-origin: the same-origin admin mode was removed.
A dev-only escape hatch (EXELEARNING_UNSAFE_LEGACY_IFRAME constant, default off, never in
the admin UI) restores same-origin only for environments that cannot serve opaque subframes
(the php-wasm WordPress Playground). It surfaces a loud admin warning when active.
- class-iframe-sandbox: mode() is always secure unless the unsafe constant is defined;
embed_mode() defaults to strict (open is opt-in); add a CSP profile (strict default,
exelearning_csp_profile filter -> compatible).
- content-proxy: build_html_csp() strict by default (no bare https: in script/img/media-src;
frame-src limited to the maintained providers) with a documented-weaker compatible profile.
- admin settings: replace the secure/legacy selector with a read-only status + escape-hatch
warning. blueprint.json defines the unsafe constant instead of update_option('...','legacy').
- Tests: legacy option ignored, strict embed/CSP defaults, compatible profile, escape hatch
off by default. Add testing-the-sandbox.elpx fixture.
NOTE: validated locally by php -l + vitest (28 JS tests). The PHPUnit + phpcs run in PR CI
(no composer/WP test harness here).
…ways-secure settings card The php-wasm Playground service worker only serves same-origin documents, so the opaque content iframe could not load its CSS/JS. The escape hatch is now delivered by a Playground-only mu-plugin (loads before all plugins, defines a real boolean EXELEARNING_UNSAFE_LEGACY_IFRAME) instead of defineWpConfigConsts, and is_unsafe_legacy() accepts any truthy value (filter_var). Also removes the read-only "always secure" Security card from the admin settings (it only stated the obvious); the dev-only escape-hatch warning now renders only when the hatch is active.
…u-plugin The admin-settings warning for the EXELEARNING_UNSAFE_LEGACY_IFRAME escape hatch was the only new translatable string in this PR, and the check-untranslated CI gate requires every languages/*.po to be fully translated — so it failed. The warning only ever shows when the dev-only hatch is active, which in practice is the WordPress Playground. Move it into the Playground-only mu-plugin (plain English, not scanned by make-pot) as an admin_notices banner, and drop render_security_section() from the shipped plugin. The hatch itself (is_unsafe_legacy) is unchanged and still covered by IframeSandboxTest.
…yground comment Two PHPUnit tests still drove the same-origin sandbox via the removed admin option (update_option(OPTION, MODE_LEGACY)) and asserted allow-same-origin. Always-opaque ignores that option, so they failed once the untranslated gate (which masked them) passed. Rewrite both as regression guards: the ignored legacy option must keep the content/block iframe opaque (no allow-same-origin); the dev-only constant hatch is the only same-origin path (covered by IframeSandboxTest). Also trim the Playground mu-plugin comment — the full php-wasm rationale now lives in the PR description.
eXeLearning core (public/app/common/exe_embed_bridge/) is now the canonical source for the promote-to-parent embed relay/shim; this copy mirrors it. Header comment only — no logic change (verified by core's scripts/check-embed-sync.mjs: no drift).
Contributor
Author
|
The external-embed bridge ( |
…rence) Mirror of the eXeLearning core canonical contract (exelearning/doc/development/preview-serving-contract.md): serve the editor preview of untrusted author content over an authless capability URL in an opaque origin, via this host's own cookieless serving primitive, so the preview gets real per-page URLs (working navigation + open-in-new-tab) instead of the srcdoc fallback. The sandbox-first CSP is emitted verbatim from core's previewCspHeader() on every scriptable document type (text/html, image/svg+xml, application/xml, application/xhtml+xml). Reference endpoint + docs; the session store, management API and tests are follow-up per repo.
Remove stray markdown (a code fence + wiring-notes prose) accidentally left after the class closing brace, convert the aligned route-list inline comment to a block comment, capitalize a docblock short description, and end the file with a single newline. phpcs --standard=.phpcs.xml.dist now clean.
Follow core commit 054abfd0 (canonical exe_embed_bridge) in the plugin mirrors: - shim: report(force) with a JSON dirty-check so observer-driven syncs skip identical geometry while initial/load/parent-request pings always post; MutationObserver now also watches class/style/hidden/open attributes; add transitionend/animationend + a ResizeObserver on documentElement/body so a class-toggled (animated) layout change re-measures the settled geometry. - relay: positionOverlay records entry.lastRect; new checkDrift() re-pins any overlay whose content iframe box moved with no scroll/resize (sidebar/panel toggles), exposed on the relay and run from a 300 ms interval in init(). Port the core checkDrift() unit test. 28 JS tests green.
The store lives under wp-content/uploads (web-servable). Untrusted author HTML
written at uploads/exelearning-preview/{uuid}/revisions/{n}/documents/*.html
could be fetched directly, served SAME-ORIGIN WITHOUT the sandbox CSP that the
REST serving route adds, defeating the opaque-origin isolation (the CSP-bypass
the published-content path already guards against via the uploads .htaccess).
On first use the store now drops a deny guard into its base dir (idempotent,
self-healing), mirroring WordPress core's protected-upload-subdir pattern:
- .htaccess: Require all denied / Deny from all (cascades to every session
subdirectory on Apache);
- empty index.php: no directory listing.
.htaccess is the Apache guard; nginx must place the store outside the web root
or deny the location (documented in the serving-contract mirror). Add TDD tests
asserting both guard files exist with deny content and self-heal, plus explicit
per-session ownership (403) tests for the revision and delete routes.
A files[] part whose $_FILES error is not UPLOAD_ERR_OK (truncated, oversized, partial) arrives with an empty temp file. Previously it slipped through: apply_revision would copy the empty path (crashing on PHP 8's "Path cannot be empty") or publish a 0-byte document, and store_assets silently reported it rejected while returning 200 — a silent-empty-document bug (parity with the Moodle/Omeka #14 class). check_uploaded_parts() now validates every part BEFORE the store runs and rejects the whole batch, naming the offending #index (filename): 413 for UPLOAD_ERR_INI_SIZE / UPLOAD_ERR_FORM_SIZE, 400 for any other error or an absent/unreadable temp file. An empty file is never substituted; index alignment (writes[i] <-> files[i]) is preserved. TDD tests: an oversized (INI_SIZE) revision part -> 413 with the store untouched (revision unchanged, no revisions/1 dir, no 0-byte document); a partial part -> 400; an errored asset part -> 400 with nothing stored.
…rdown) Follow core (canonical exe_embed_bridge): the drift-check interval was left running for the page lifetime. createRelay() now tracks driftTimer + started; init() is idempotent (a second init() no longer stacks a duplicate interval and window listeners) and stores the interval handle; dispose() clears the overlays and, in the browser, clears the drift timer and removes the message/resize/ scroll/load listeners, then resets started so a reused relay can init() again. Adds an internal clear() (the mirror never exposed core's clear()/reflow()) so dispose() tears down overlays; only dispose is exposed, per the minimal mirror surface. Port the dispose() unit test (tears down overlays like clear, safe before init and when called twice). 29 JS tests green.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
The preview v2 WP-Cron feature added one user-facing string, the 15-minute cleanup interval label __( 'Every 15 minutes (eXeLearning preview cleanup)' ), which was untranslated in all 10 locales and turned the check-untranslated CI gate red. Add the msgid to the .pot and each locale .po with its translation and recompile the .mo (via the plugin's wp i18n toolchain). Minimal, per-string edit — no reflow of unrelated entries.
Pure structural refactor — identical status codes, error bodies and side effects (the 66-test preview suite is unchanged and green). Extract cohesive private helpers so every flagged method drops below the CC=15 / NPath=500 thresholds: - proxy: upload_assets -> validate_asset_entries / check_declared_asset_budget / build_asset_store_entries; publish_revision -> validate_revision_meta (+ validate_revision_collections) / buffer_revision_writes; build_serve_response -> build_asset_response. - store: apply_revision -> validate_revision_order / normalize_revision_paths / check_asset_refs / check_fixed_refs / compute_revision_budget; store_assets -> classify_asset_entry (+ classify_new_asset); serve_lookup -> resolve_served_file. Guard-clause flattening collapses the multiplicative NPath (e.g. apply_revision 2519425 -> under 500). For the two class-level ExcessiveClassComplexity findings (WMC, which method extraction does not lower): both classes are the single cohesive implementation of the serving-contract-v2 protocol; splitting would only relocate WMC into a tightly-coupled collaborator and fragment one state machine / break the tested build_serve_response surface, so each carries a justified @SuppressWarnings(PHPMD.ExcessiveClassComplexity). Method-level complexity — the meaningful gate — is now under threshold everywhere.
The preview classes tripped the CI phpcs step (the repo's own `/exelearning/*` exclude-pattern matches any local checkout path containing a bare `exelearning` segment, so local `make phpcs`/wp-env runs silently skip the whole plugin; CI's `wp-exelearning` checkout path is scanned normally). Verified by linting the files at a neutral path with the same standard. Auto-fixes (phpcbf): @param docblock alignment, array double-arrow alignment, equals-sign alignment. Manual: - proxy: Yoda condition in is_intish() (hoist the float, `floor($value) === $value`); phpcs:ignore for the range-streaming fread(). - store: hoist count() out of the enforce_per_user_cap() loop condition (decrement in-loop, behavior identical); capitalize the ASSET_KEY_REGEX docblock; reword the normalize_content_path comment that read as commented-out code; fold the touch() ignore onto one directive so both sniffs are suppressed. - fixed-resources: rename the reserved-keyword `$parent` param to `$root`. phpcs (neutral path, full standard) clean on every touched PHP file; PHPMD still clean; preview PHPUnit 66 tests green.
…alse-green The bare `<exclude-pattern>/exelearning/*</exclude-pattern>` matched the FULL path, so any checkout under a directory containing an `exelearning/` segment (e.g. .../ate/exelearning/wp-exelearning/, or the wp-env mount .../plugins/exelearning/) excluded the ENTIRE plugin — local `make phpcs` "passed" in 61ms by scanning nothing, while CI (checkout .../wp-exelearning/) scanned normally and failed. Anchor it to the intended target only, the WordPress uploads extraction dir. Verified from the real checkout path: `phpcs -q .` now scans the plugin (3.5s, was 61ms), is clean, a probe violation under includes/ is caught, and a probe under wp-content/uploads/exelearning/ stays excluded. No other ruleset behavior changed.
Run the canonical pipeline (wp i18n make-pot -> pot-remove-ctime -> update-po -> make-mo) so the committed catalog matches the current branch source: refresh every `#:` source reference, drop strings removed by earlier work, and add the ones added, re-sorted to the extractor's order. The preview-cleanup cron label translations are preserved by update-po in all 10 locales. composer untranslated is clean and `msgfmt -c` passes on all 10 .po.
…ollers Splits ExeLearning_Preview_Proxy (which carried an @SuppressWarnings PHPMD.ExcessiveClassComplexity) into two focused classes over a small shared header helper, and lands the contract v2.1 serving deltas the split made natural to place: - ExeLearning_Preview_Http_Headers: the byte-identical sandbox CSP, the scriptable-type set and the extension->MIME map. Adds text/xml to the scriptable set to match eXe core's isScriptableDocumentType, closing a CSP coverage gap (a stray text/xml document is as scriptable as application/xml). - ExeLearning_Preview_Serving_Controller: the authless read side. Serves a bare capability URL as a 302 to index.html (so a page's relative subresources resolve against the index base), and treats a malformed / multi-range / non-bytes Range as "ignore -> 200 full body" instead of 416 (416 stays only for a syntactically valid, single, unsatisfiable range). - ExeLearning_Preview_Management_Controller: the owner-scoped write side. - ExeLearning_Preview_Proxy: now a thin registrar wiring both controllers and the cleanup cron over one shared store + resolver. The PHPMD suppression is gone naturally, not moved. The store keeps its own suppression (its cohesion is real). Behaviour is guarded by the shared conformance vectors (replayed unchanged) and the split unit tests.
…ker viewer
Wires the opaque HTTP preview transport (serving contract v2) into the editor
bootstrap and removes the legacy Service Worker /viewer/ machinery the plugin's
own contract forbids on the WordPress origin.
- editor-bootstrap.php now emits `previewHttp` in __EXE_EMBEDDING_CONFIG__
(protocolVersion 2, the two management/serving base URLs, X-WP-Nonce header),
and no longer monkey-patches navigator.serviceWorker or rewrites the preview
iframe /viewer/ src. The CSS/idevice 404 shim and UI hiding stay.
- previewHttp is emitted only under pretty permalinks; under plain permalinks
rest_url() yields the ?rest_route= form that cannot carry a /{id}/{path}
suffix, so the bootstrap omits previewHttp (the editor fails closed, no silent
same-origin fallback) and ExeLearning_Editor::maybe_warn_preview_permalinks
surfaces a WP admin notice. The gate + config live in
build_preview_http_config()/pretty_permalinks_enabled() (single source).
- The Playground legacy hatch (EXELEARNING_UNSAFE_LEGACY_IFRAME + mu-plugin) is
untouched; it targets the published viewer, a separate system.
The bundled editor (v4.0.2) predates HttpPreviewProvider, so the wiring has no
in-editor consumer until a newer build ships; it is wired now and covered
headlessly by EditorTest. The Playwright spec asserts the injection when a
capable build is present and skips honestly otherwise.
- Rewrites the "Editor activation" section to the normalized previewHttp two-URL model (drops the dead previewTransport + previewBasePath single-base vocabulary) and documents the pretty-permalink requirement, the fail-closed behaviour and the editor-build dependency (endpoints dormant until a build with HttpPreviewProvider + bundles/preview-fixed-resources.json ships). - Documents the serving deltas: the bare-capability-URL 302 to index.html and the Range fallback (malformed / multi-range / non-bytes -> 200 full). - Adds text/xml to the scriptable-CSP list and rewrites the implementation bullets for the serving/management/header split. - Fixes the vectors README path mismatch (the WordPress mirror lives at docs/preview-serving-contract.md; vectors.json is vendored from core) and notes the Range/bare-root deltas are covered by unit tests pending a re-vendor of vectors.json from core (do not fork the JSON).
Adds translations for the two new admin-notice strings introduced by the opaque HTTP preview wiring (the pretty-permalink requirement warning and its "Change Permalink Settings" link) across all ten locales, and refreshes the POT/PO source references. Isolated in its own commit per repo convention.
The managementHeaders X-WP-Nonce is a standard wp_rest nonce: valid for nonce_life (default 24h) across the current + previous 12h tick, so it survives at least 12h and up to 24h — well beyond an editing session, and unlike a fixed short CSRF-token expiry it will not break a preview mid-session. Documents the window and the reload-to-re-mint fallback.
erseco
changed the title
… re-vendor vectors
Adversarial review (Agent K) found parse_range checked satisfiability
($start >= $total) BEFORE structural invalidity ($end < $start), so bytes=15-2
on a 10-byte body returned 416 instead of being ignored. RFC 9110: an inverted
byte-range-spec (last-byte-pos < first-byte-pos) is invalid and MUST be ignored,
regardless of whether the first-byte-pos is also past the end.
- Reorder parse_range: the last<first invalidity check now precedes the
satisfiability check, so bytes=5-2 AND bytes=15-2 → null → 200 full body.
- Bare-root redirect now emits a RELATIVE Location `{previewId}/index.html`
(byte-identical to eXe core) instead of an absolute rest_url(); it resolves
against the trailing-slash-free `.../preview/{previewId}` and stays correct
under BASE_PATH / app://.
- Re-vendor tests/fixtures/preview-contract/vectors.json verbatim from core
(adds serve-bare-root-redirect + zero-suffix/inverted/inverted-oob/multi/
non-bytes/garbage Range cases); adapt the conformance harness only to
substitute {previewId} in expected header values (the bare-root Location).
Conformance replay green (120 assertions); serving unit tests cover bytes=5-2,
bytes=15-2 and the relative Location.
…ity with core Adversarial review: SANDBOX_CSP ended with a trailing ';' so it was NOT byte-identical to core's previewCspHeader() (which joins its directive array with '; ' and ends at "frame-ancestors 'self'"), contradicting the byte-identity the docs claim. The drift gate only checks directive substrings, so it missed this. Functionally inert as a header value, but the contract is byte-identity — dropped the trailing ';' and adjusted the two test literals. Verified equal to core's reconstructed output.
…TP-v2 interim Adversarial review: removing the /viewer/ monkey-patch left NOTHING stopping the BUNDLED pre-v2 editor (v4.0.2, which still calls loadPreviewFromServiceWorker() -> /viewer/index.html) from registering preview-sw.js on the WordPress origin — a same-origin, un-sandboxed preview of untrusted author content, exactly what the opaque HTTP transport avoids. Add ExeLearning_Editor::service_worker_guard_script(): a first-thing IIFE (injected right after <head>, before any editor script) that stubs navigator.serviceWorker.register to a resolved no-op registration, mirroring the Nextcloud/Moodle resilience shim. NOT a path rewrite. Interim protection until an HTTP-v2 editor build ships. Unit-tested for presence + shape.
Adversarial review [HIGH]: the store lives under wp-content/uploads/exelearning-preview/ guarded only by the Apache .htaccess deny the store self-heals. nginx ignores .htaccess, so on a default nginx WordPress a direct GET to a materialized preview document serves untrusted author HTML SAME-ORIGIN and WITHOUT the sandbox CSP — a second, un-sandboxed serving path that defeats the opaque-origin isolation. Ship nginx-exelearning-preview.conf (location ^~ .../exelearning-preview/ -> 403), mirroring Omeka's data/nginx-exelearning.conf, to `include` from the site server block. Unit test asserts the snippet ships with the deny rule.
…erim
- Bare-root redirect documented as a RELATIVE Location {previewId}/index.html
(BASE_PATH / app:// safe) and Range as invalidity-before-satisfiability
(bytes=5-2 and bytes=15-2 -> 200-ignore, never 416).
- Drop the trailing ';' from the documented CSP so it matches core byte-for-byte.
- Point the nginx storage guard at the shipped nginx-exelearning-preview.conf;
add a README "Server configuration (nginx)" note (pretty-permalink requirement
+ preview-store deny include).
Comment on lines
+351
to
+395
| private function parse_range( $value, $total ) { | ||
| if ( empty( $value ) ) { | ||
| return null; | ||
| } | ||
| // Malformed / multi-range / non-bytes unit: ignore -> 200 full body. | ||
| if ( ! preg_match( '/^bytes=(\d*)-(\d*)$/', trim( $value ), $m ) ) { | ||
| return null; | ||
| } | ||
| $raw_start = $m[1]; | ||
| $raw_end = $m[2]; | ||
| if ( '' === $raw_start && '' === $raw_end ) { | ||
| return null; | ||
| } | ||
| if ( '' === $raw_start ) { | ||
| $suffix = (int) $raw_end; | ||
| if ( 0 === $suffix || 0 === $total ) { | ||
| return 'unsatisfiable'; | ||
| } | ||
| return array( | ||
| 'start' => max( 0, $total - $suffix ), | ||
| 'end' => $total - 1, | ||
| ); | ||
| } | ||
| $start = (int) $raw_start; | ||
| // Structural invalidity (last-byte-pos < first-byte-pos) is checked | ||
| // BEFORE satisfiability: an inverted spec is IGNORED (-> 200 full body), | ||
| // even when the first-byte-pos is ALSO past the end. bytes=15-2 on a | ||
| // 10-byte body must be 200, never 416. | ||
| if ( '' !== $raw_end && (int) $raw_end < $start ) { | ||
| return null; | ||
| } | ||
| if ( $start >= $total ) { | ||
| return 'unsatisfiable'; | ||
| } | ||
| if ( '' === $raw_end ) { | ||
| return array( | ||
| 'start' => $start, | ||
| 'end' => $total - 1, | ||
| ); | ||
| } | ||
| return array( | ||
| 'start' => $start, | ||
| 'end' => min( (int) $raw_end, $total - 1 ), | ||
| ); | ||
| } |
Adversarial review (K, conflicting reports — verified against the code): the
store DISCARDED its write return values. copy_file() ignored copy(); atomic_write()
ignored file_put_contents()/rename(). So:
- store_assets() bumped assetBytes and reported the key in `stored` even when
the asset copy failed, and
- stage_and_publish() swapped the `current` pointer even when a document copy
or the publish rename failed,
violating §5 ("failed writes are never indexed" / "abort before the pointer
swap"). The earlier "integrity OK" review saw the validation + staging structure
but missed the discarded returns.
Fix, matching Moodle/NC:
- copy_file()/atomic_write() now throw \RuntimeException on any failed write
(atomic_write also cleans its orphaned temp).
- store_assets() catches it per entry -> `rejected: write-failed`, never
indexing the key or bytes.
- stage_and_publish() return-checks the publish rename, removes the partial
staging on any failure, and only swaps the pointer once the full revision is
staged and renamed (the commit point).
- apply_revision() maps a write failure to a clean 500 with the active revision
unchanged.
Regression tests force real write failures (copy into a directory target;
rename into a non-empty target) and assert write-failed rejection / no assetBytes
bump, and 500 + unswapped pointer + no stray staging.
…ntract §5) Follow-up to the §5 write-return fix (643da31), addressing the reviewer's short-write note and the reference pattern Omeka landed (ce538b1): - atomic_write() now compares file_put_contents()'s return against the payload length, not just `=== false`. A disk-full SHORT write returns a byte count below the length (not false), so without this a truncated meta.json / refs.json / pointer could be renamed into place and served. A failed/short write drops the temp file (new drop_temp helper) and throws. - Regression test: a revision-2 document copy is forced to fail (write points at a directory) AFTER a good revision 1 — asserts 500, the pointer stays on revision 1, revision 1's document is still served intact, and revisions/2 was never created (prior revision survives, no partial publish). copy_file() already fails closed on partial copies (copy() returns false on any failure). WP uses throw/catch where Omeka returns bool; the §5 semantics — never index a failed write, abort before the pointer swap — are identical.
erseco
changed the title
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
2 participants
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.
Summary
This branch delivers two related security features:
The iframe never receives
allow-same-originin production. Author HTML and JavaScript therefore cannot reach the WordPress page, cookies, REST nonce, storage, or editor APIs.Published-content viewer
EXELEARNING_UNSAFE_LEGACY_IFRAMEescape hatch for the php-wasm Playground published viewer. It is not an administration setting and is not used by editor preview.HTTP editor preview: protocol v2
The plugin exposes two deliberately separate surfaces:
Management is authenticated with the WordPress cookie/REST path, requires
X-WP-Nonce, checksupload_files, and enforces session ownership. Serving is an authless, cookieless capability URL backed only by an unguessable preview UUID and TTL.Protocol v2 separates:
Private session storage
The production proxy now creates one shared session store in a site-scoped private system-temporary directory:
It no longer uses
wp-content/uploadsas the production default. This makes authored HTML, SVG, XML, CSS and JavaScript private by construction and prevents a direct web-server path from bypassing the serving controller's sandbox CSP.exelearning_preview_store_dirmay select another private directory. Apache guards,index.php, and nginx guidance remain defense in depth, not the primary isolation boundary.The management and serving controllers share the same store and fixed-resource resolver, reducing duplicate lazy construction.
Serving behavior
text/xml, and XHTML from every layer.nosniff,no-referrer, restrictive Permissions-Policy and appropriate CORS headers, including errors.302from the bare capability root to{previewId}/index.html.206for a valid satisfiable single range.416for a valid unsatisfiable range, includingbytes=-0.200response when Range is malformed, multi-range, uses another unit, or is inverted.Editor configuration
The bootstrap injects:
{ "previewHttp": { "protocolVersion": 2, "managementBaseUrl": "{rest_url}/exelearning/v1/preview-session", "servingBaseUrl": "{rest_url}/exelearning/v1/preview", "managementHeaders": { "X-WP-Nonce": "..." } } }Pretty permalinks are required because the plain
?rest_route=form cannot provide a stable capability subtree. When they are disabled,previewHttpis omitted and the editor fails closed with an administration notice.Activation status
The currently bundled released editor predates
HttpPreviewProvider. The host routes, storage, bootstrap wiring, contract tests and security headers are implemented, but a released editor does not drive the HTTP preview yet.Production activation depends on a core release containing:
HttpPreviewProvider;StaticServiceWorkerPreviewProvider;bundles/preview-fixed-resources.json.No full browser activation is claimed until that editor is installed and tested inside WordPress.
Tests
Canonical source
eXeLearning core is the canonical source for the serving contract, scriptable MIME policy, CSP, bridge files, and conformance vectors. WordPress maintains a host adapter and synchronized mirrors rather than a protocol fork.
Known limitations
exelearning_preview_store_dirmust remain outside any publicly served tree.