Secure opaque-origin viewer and opt-in opaque editor preview - #21
Draft
erseco wants to merge 56 commits into
Draft
Secure opaque-origin viewer and opt-in opaque editor preview#21erseco wants to merge 56 commits into
erseco wants to merge 56 commits into
Conversation
The preview iframe shows arbitrary author HTML/JS from an .elpx. In the two show partials it was served same-origin with allow-same-origin, so the content could read the Omeka page's cookies/DOM and reach window.parent. Add an exelearning_iframe_mode setting (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 same-origin behaviour for environments that need it (e.g. the php-wasm Playground, whose service worker only serves same-origin documents). The media file renderer was already opaque; this centralizes the sandbox tokens in a single IframeSandbox helper consumed by the renderer and both show partials, and makes the two partials follow the setting (they were hardcoded to legacy). 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). The teacher-mode toggle is already hidden server-side by the content proxy, so no parent-to-iframe DOM access is involved. Adds unit tests for the helper, the setting and the renderer config.
Contributor
omeka-s Playground Preview
Try this PR in your browser
|
…secure mode
Review feedback: in secure mode the .elpx content could still run as the Omeka origin
when loaded OUTSIDE the sandboxed iframe — via the "Open in new tab" link, an escaped
popup (allow-popups-to-escape-sandbox), or by navigating to the raw
/exelearning/content/{hash}/index.html URL. The iframe sandbox only protects the
embedded case; the raw route stayed same-origin with an executable CSP.
- ContentController emits a `sandbox allow-scripts allow-popups` CSP directive for HTML
in secure mode, so the document keeps an opaque origin however it is loaded (iframe,
new tab, popup, direct URL). Legacy mode is unchanged. This makes the "Open in new
tab" link safe and neutralizes any popup escape at the source.
- IframeSandbox secure tokens drop `allow-popups-to-escape-sandbox` (kept only in
legacy), so a popup the content opens cannot reopen unsandboxed and same-origin.
- normalizeMode() uses a strict comparison so non-string values fail safe without
warnings; getConfig() catches \Throwable for consistency.
Verified in a browser: a top-level document served with `sandbox allow-scripts` reports
window.origin === 'null' and document.cookie throws SecurityError, while scripts and
same-host subresources still load.
Tests: secure tokens omit same-origin AND popup-escape; secure CSP carries the sandbox
directive while legacy does not; normalizeMode handles arrays/objects/booleans.
Collaborator
Author
Live verification (secure mode, default)Brought this branch up via Docker (
By the symmetry of the same-origin policy, the author content cannot read the Omeka DOM, cookies or CSRF token, nor reach The content controller's CSP backs this up: External-embed compatibility (worth a docs note). Omeka's CSP is the strictest of the three eXeLearning integrations: Minor: |
Omeka's content CSP was the strictest of the eXeLearning integrations (img/media/frame-src 'self'), so external images, HTML5 video/audio, PDFs and iframe embeds were blocked in BOTH modes — unlike wp-exelearning and mod_exelearning, which already allow https:. Widen img-src/media-src/frame-src to include https: so authors can embed external resources. connect-src stays 'self' (no exfiltration) and the document stays opaque-sandboxed in secure mode (it cannot reach the host). Note: third-party video players (YouTube/Vimeo) need their own origin, which an opaque-origin sandbox denies, so they render blank in secure mode and require legacy. This is an inherent trade-off of opaque isolation; serving the content from a separate origin (subdomain) would be the only way to keep isolation AND functional embeds. Verified live in Omeka: the content CSP now carries img/media/frame-src ... https:. Test asserts external embeds are allowed while connect-src stays locked.
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: ContentController injects a shim that replaces whitelisted-video / .pdf iframes with placeholders and reports their geometry via postMessage; a relay enqueued on the item/media views + renderer validates + rebuilds the URL and overlays the real player inline over each placeholder. - asset/js/exe-embed-shim.js / exe-embed-relay.js: the shared shim + relay. - ContentController::injectEmbedShim(): inject the shim into served HTML (secure only). - IframeSandbox::embedWhitelist() + enqueueEmbedRelay(). - item-show.phtml / media-show.phtml / ExeLearningRenderer.php: enqueue the relay. 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 ContentControllerTest + IframeSandboxTest. Verified live (:8080): the shim, whitelist and relay are served with the secure CSP; the JS is byte-identical to the wp plugin, where YouTube/Vimeo and remote + local PDF render inline.
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 + wp shim.
Add a self-contained Playwright Firefox e2e that loads the real exe-embed-shim and exe-embed-relay against a static harness, verifying whitelisted video and local PDF iframes are promoted to inline parent players while non-whitelisted hosts are dropped. Also document the secure-mode external embeds flow in the README.
…mbed e2e Parse new URL().hostname instead of matching a URL substring, keeping parity with the mod_exelearning spec.
…av fix Bring the Omeka S 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, ContentController and renderer tests for the new tokens and hosts.
erseco
marked this pull request as draft
June 14, 2026 12:21
…) + embed policy
Bring the Omeka S 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 Omeka 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 setting (open default, fail-safe to strict) to IframeSandbox
+ the module config form, inject {mode, whitelist} into the relay config, and update the
tests. Mirrors mod's logic (tools/check-embed-sync.mjs reports no drift).
- 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). - ContentController: emit Referrer-Policy: no-referrer on every served file in secure mode (PDFs/CSS were getting none); drop the unused window.__exeEmbedWhitelist injection. - Add a Vitest relay unit suite (happy-dom) covering the structural gate, including the trailing-dot cases.
Resolve teacher-mode conflict in ContentController. main (#23) retired the host-side teacher CSS injection in favour of the package's own ?exe-teacher=1 URL parameter (appended by ExeLearningRenderer::buildContentPath), so drop injectTeacherModeCss() and its call here. Keep the secure-mode external-embed shim (injectEmbedShim), which is independent of teacher mode. The param works in secure (opaque-origin) mode too because the package reads its own location.search, so no host CSS injection is needed.
…067) Mirror the id-only channel into the embed shim/relay. Add asset/js/exe-media-policy.js + exe-media-host.js and IframeSandbox::enqueueMediaHost(), called from ExeLearningRenderer + admin/public views: 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 preview iframe is now always opaque-origin: the same-origin ConfigForm option was removed. A dev-only escape hatch (EXELEARNING_UNSAFE_LEGACY_IFRAME constant/env, default off, never in the ConfigForm) restores same-origin only where an opaque subframe cannot be served (the php-wasm Playground). - IframeSandbox: normalizeMode() is always secure unless the unsafe escape hatch is set; embedMode() defaults to strict (open is opt-in); add a CSP profile (strict default, EXELEARNING_CSP_PROFILE=compatible to opt in). - ContentController: response 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. - ConfigForm: drop the secure/legacy selector; embed policy defaults to strict. The renderer and views default embed_mode to strict. - Tests: legacy option/arg ignored (still secure + sandboxed), strict embed/CSP defaults, compatible profile via env, escape hatch off by default, form has no mode element. Add testing-the-sandbox.elpx fixture. NOTE: validated by php -l + vitest (28 JS tests). PHPUnit + phpcs/phpmd run in PR CI (no composer/Laminas harness here).
…tants The omeka-s-playground now exposes a generic top-level phpConstants blueprint property. Use it to declare the module's dev-only EXELEARNING_UNSAFE_LEGACY_IFRAME constant, which IframeSandbox::isUnsafeLegacy() reads: the Playground's php-wasm service worker only serves same-origin documents, so the always-opaque content iframe cannot load its CSS there. The constant renders the demo same-origin in the Playground only; real Omeka never defines it.
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).
…nto feature/secure-iframe-sandbox
Collaborator
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.
PreviewController.php lacked the trailing newline PSR2 requires (phpcs PSR2.Files.EndFileNewline). No other changes.
Add PreviewControllerTest (16 tests) covering the security-critical logic of the reference preview endpoint: the capability-UUID gate and traversal -> 404 with the exact hardening headers, the serve path emitting the sandbox CSP byte-identical to core previewCspHeader() on scriptable types (text/html, image/svg+xml) and NOT on passive ones (text/css, image/png), plus isScriptableDocument()/normalizePath()/ mimeFor(). PreviewController reaches 100% line coverage, restoring the suite above the 90% CI gate (was 87.5% after the untested reference was added). Minimal source change: lookupPreviewFile private -> protected so a Testable subclass can override the store seam to exercise the serve/CSP path (PHP private methods are not polymorphic), mirroring the existing TestableStylesController pattern.
…o feature/secure-iframe-sandbox
Implement the storage half of the eXeLearning preview serving contract v2 for
the Omeka S adapter. PHP is request-scoped, so the in-memory reference manager
in eXe core is replaced by a file-backed store under the Omeka file store.
PreviewSessionStore holds the three layers with distinct lifecycles:
- assets/ — author media, immutable per assetKey (opaque validated token, never
hashed by the server);
- rev/{n}/documents/ — a full self-contained document snapshot per revision
(unchanged files hardlinked from the previous revision);
- current — the active-revision pointer, swapped atomically (rename) after the
revision materializes, under an exclusive lock, so a concurrent GET observes
revision N or N+1, never a mixture.
Full v2 semantics: create/storeAssets/applyRevision/deleteSession with owner
scoping (403/404), 409 revision-conflict, path validation (traversal, encoded,
double-encoded, backslash, NUL), assetKey regex, immutability, 422
missing-assets / unknown-fixed-resources, per-asset/session/global byte and file
budgets (declared-then-actual), 30-min idle TTL with an opportunistic swept.
PreviewFixedResources resolves layer 1 through the installed static editor's
bundles/preview-fixed-resources.json by exact id lookup with distribution-root
containment; an absent/invalid manifest disables the fixed layer (never fatal).
…t API
Wire the serving half of the preview serving contract v2 and register the
routes so the controllers are reachable.
PreviewController::serveAction evolves to three-layer resolution (documents →
assetRefs→assets → fixedRefs→manifest → 404) with tiered Cache-Control
(no-store / no-cache+ETag / private,max-age), Accept-Ranges + single-range
206/416 and If-None-Match → 304 on assets, and the byte-identical sandbox CSP on
every scriptable document type from any layer. It is the authless capability URL
twin of ContentController; base hardening headers are emitted on every response,
404s included.
PreviewSessionController is the only authenticated surface: create / assets /
revisions / delete, gated by a logged-in identity + mandatory CSRF
(CsrfValidationTrait), owner-scoped via the store.
config/module.config.php registers both controllers and both services, the
authless Regex serving route (/exelearning/preview/{previewId}/*), and the four
authenticated child routes under /api/exelearning/preview-session.
Tests: the evolved PreviewControllerTest keeps the byte-exact CSP drift
assertion and adds the tiered-cache, ETag/Range/304 and per-layer CSP coverage;
new store, management and conformance-vector-replay tests; the shared
test/fixtures/preview-contract vectors are vendored and replayed in PHPUnit.
…buffering The contract enforces the session byte budget twice: on declared sizes before buffering and on actual bytes while buffering. The management controller only did the second (collectFileBytes read every part into memory first). Add the first stage: PreviewSessionStore::remainingSessionBudget() returns an owned session's spare capacity (null when unknown/not owned), and assetsAction now 413s when the declared-size total already exceeds it, before reading any part.
Mirror the final embed-bridge change from eXe core. The relay's checkDrift interval (and its window listeners) previously ran for the page lifetime with no teardown, and a second init() on the same relay would stack a duplicate interval and duplicate listeners. createRelay() now tracks driftTimer + started. A new clear() removes every overlay and its players; dispose() calls clear() then (browser-only) clears the drift interval and removes the message/resize/scroll/load listeners, and resets started so a reused relay can init() again. init() is idempotent (returns early when already started) and keeps the interval handle in driftTimer. clear and dispose are exposed on the relay object. Ports the dispose() unit test; the mirror keeps its auto-running IIFE + module.exports export wrapper.
Expose the normalized previewHttp block (serving contract v2 §1) in window.__EXE_EMBEDDING_CONFIG__ so the embedded editor selects the opaque HttpPreviewProvider instead of failing closed. managementBaseUrl and servingBaseUrl are derived from the same serverUrl+basePath origin as saveEndpoint, matching the two distinct Omeka prefixes (/api/exelearning/preview-session and /exelearning/preview) that a single previewBasePath could not express. The management CSRF token rides managementHeaders['X-CSRF-Token'] on every publish. Laminas' default form token is unfit for this lifecycle: it is not single-use (isValid() is a pure comparison, never consumed or rotated), but initCsrfToken() stamps an absolute, container-global EXPIRE (default 300s) that is never refreshed on validation, so a token minted at editor load stops validating ~5 minutes into an editing session and every later publish 403s. PreviewCsrf mints a dedicated, session-lifetime token (timeout => null) under its own namespace, and PreviewSessionController validates against that same namespace, so previewing survives a long editing session. The anonymous export bootstrap passes previewHttp: null (never previews).
storeAssets() called evictOthersForGlobal() once per asset entry, and each call did a full allSessions() pass (scandir + per-session metadata reads) — O(M·N) filesystem operations for a batch of M assets over N sessions. Scan the session tree at most once per request instead: lazily, on the first entry that reaches the global-budget check, then reuse (and mutate via evictions) that snapshot for every later entry. A batch whose entries all fail a cheap pre-check never scans at all. The revision publish path keeps its single evictOthersForGlobal() call; both paths now share evictUntilFits(). Per-entry decisions are byte-for-byte unchanged: the snapshot keeps this session at its pre-batch size (in-batch growth is only committed to session.json after the loop, mirroring the existing mid-batch counter staleness), and evictions persist across entries. allSessions() is now protected and the class non-final so a spy subclass can assert the one-scan-per-batch invariant.
Bare capability URL: a GET to …/{previewId} or …/{previewId}/ (no file) now
302-redirects to …/{previewId}/index.html instead of serving index.html bytes
at the bare URL. The decision reads the actual request path — not the route's
file=index.html default — so an explicit …/{previewId}/index.html still serves
200. The original query string is preserved on the redirect.
Range handling: a malformed, multi-range, or non-bytes Range is now IGNORED and
answered with a normal 200 full body (a Range the server does not understand is
not an error); 416 is reserved for a syntactically valid but unsatisfiable
single range (first-byte-pos at/after the end, or a zero-length suffix). A
last-byte-pos < first-byte-pos spec is invalid (RFC 9110 §14.1.2), hence 200.
The conformance harness now drives serving requests with the real URI path +
query so the re-vendored bare-root/Range vectors replay without forking the
JSON (TODO(re-vendor) marks the pending core vector update).
…vation
The preview session store lives at {file_store}/exelearning-preview/. Its Apache
.htaccess deny guard is invisible to nginx, and the -preview/ prefix is not
covered by the existing /files/exelearning/ rule, so a direct GET could serve
untrusted author HTML same-origin without the sandbox CSP. Add an explicit
`location ^~ /files/exelearning-preview/ { return 403; }` to both shipped nginx
samples and the README, with a note that non-Apache deployments MUST deny it.
Rewrite the serving-contract activation section: replace the aspirational single
previewBasePath with the real normalized previewHttp block (two prefixes +
X-CSRF-Token), document the preview-scoped long-lived CSRF token, the §4
bare-URL/Range deltas, and the editor-build dependency (dormant until a build
with HttpPreviewProvider + preview-fixed-resources.json). Correct the srcdoc
section: srcdoc is removed as an authored transport; static/PWA uses the gated
static-service-worker, and the php-wasm playground editor preview fails closed
(intended dev-only behavior, never an admin setting).
Add a closed-range-past-end vector (bytes=99-100 -> 416) and a request whose getUri() reports no path (the bare-URL redirect's defensive guard, serve normally), so every new serving-delta branch is exercised.
erseco
changed the title
…symmetric Adversarial review flagged the "survives the whole session" claim as asserted but unverified: mint() used timeout=>null while csrfTokenIsValid() built its validator without it (default 300s), and both paths were coverage-ignored. - Add PreviewCsrf::validator() as the single source of truth for the validator options (namespace + timeout=>null); mint() and PreviewSessionController's validation both build through it, so they can never drift into different namespaces or a finite timeout. - Add a REAL test (no coverage-ignore) driving the actual Laminas\Validator\Csrf against a clock-controlled session container that faithfully models Laminas' absolute container-global expiry: a preview token still validates after 6 simulated minutes and validates repeatedly (not single-use), while the control (default 300s form validator) expires past its TTL. Covers validator()/mint() and the controller's csrfTokenIsValid (unknown-token rejection).
…Location
Re-vendor test/fixtures/preview-contract/vectors.json verbatim from core
(commit 5675623a): adds the bare-root 302 step and the Range cases
(bytes=-0 -> 416; bytes=5-2 / 15-2 inverted, multi-range, non-bytes, garbage
-> 200 full body, no Content-Range).
The new bare-root vector expects a RELATIVE Location "{previewId}/index.html"
(base-path / origin / app:// safe), not the absolute path this adapter emitted.
Change PreviewController to emit the relative form: no trailing slash ->
"{previewId}/index.html" (replaces the last URL segment), trailing slash ->
"index.html" (appends); query string preserved. The Range behavior already
conformed.
Drop the TODO(re-vendor) harness note (vectors now include the deltas); the
conformance harness substitutes {previewId} in expected header matchers so the
relative Location asserts exactly. Docs updated for the relative Location.
Adversarial review noted Laminas Csrf's initCsrfToken could (in some versions) call setExpirationHops(1), which would kill the token after ONE subsequent request regardless of timeout. Verified every reachable Laminas validator (module vendor + all Omeka builds) calls only setExpirationSeconds — no hops — and locked it with tests: - ClockedSessionContainer now models hop-based expiry AND records whether a validator armed a seconds- or hop-expiry. - The preview token still validates across 4 sequential request cycles (hops), and the mint arms NEITHER a seconds nor a hop expiry on its namespace; a container self-test proves the hop model actually expires (non-vacuous).
…d (§5) The store discarded write return values: storeAssets indexed an asset (stored[] + byte counter) after file_put_contents without checking it, and publishRevisionLocked materialized documents and swapped the `current` pointer without checking any write — so a failed doc write could leave `current` pointing at a half-materialized revision, violating contract §5. - Add a shared atomicWrite() (temp + full-length-verified write + rename) used by writeAsset, writeJsonAtomic, swapPointer and revision document writes; it returns false on any failure and never leaves a partial file. - storeAssets reports a failed write as `write-failed` and never indexes it. - publishRevisionLocked aborts with 500 and discards the partial rev BEFORE the pointer swap if materialization, revision.json, or the swap fails — `current` stays on the old revision, so a reader never sees a partial one. Tests force real write failures (assets dir replaced by a file; document write made to fail; rename onto a directory) and assert nothing is indexed and the pointer never moves. Unreachable I/O-defense branches are marked.
erseco
changed the title
Three files conflicted, all around the runtime editor installer main removed. Module.php and config/module.config.php drop StaticEditorInstaller and its two install-editor routes, whose controller actions went with it, and keep this branch's IframeSandbox and preview-session routes. The v2 preview routes stay for now — the snapshot migration retires them next, and folding that into a merge would make both unreviewable. EditorControllerTest kept this branch's preview-config tests and dropped the install-status ones, which asserted against a class that no longer exists. Two things did not conflict and still broke: - PreviewFixedResourcesFactory called StaticEditorInstaller::getEditorPath(). EditorBundle::getPath() resolves the same dist/static root. - main's controller derives the install sub-path from $request->getUri()->getPath() before falling back to getBasePath(), and four editAction tests stub a URI object without it. The stubs now provide it. The suite ends with the same 3 errors and 1 failure as the pre-merge baseline — OMEKA_PATH undefined, the StylesService icon test, and a DownloadFormats test that only fails in a full run. None of them are the merge's.
Rebuilt the bundled editor from the trust-boundary branch to check rather than assume, and the module is in the same position Moodle was: the editor reads previewSnapshot / managementUrl / servingBaseUrl / deleteUrlTemplate, and has no reference to previewHttp or protocolVersion at all — which is the only thing editor-bootstrap.phtml hands it. The opaque preview is unreachable here today. This is the first piece of the replacement, and it is additive: nothing calls it yet, so the module keeps building and testing as before. PreviewSnapshotStore keeps a whole project under one capability id, replacing the layered protocol-v2 store. Content sits in its own content/ subdirectory so an author path can never collide with the store's own files, and a write is staged beside the live tree and swapped in. It does NOT bring its own archive inspector: ZipSafety already guards uploads here, rejects unsafe and forbidden entries atomically before writing, and caps zip bombs on the REAL decompressed bytes rather than the attacker-declared sizes in the central directory — stricter than the inspectors the WordPress and Moodle ports needed. Reused as-is. Thirteen tests cover ownership on replace and delete, the whole-tree swap, idle expiry and the sweep, the idle clock being pushed back on serving, and the five archive rejections — no index, traversal, a forbidden PHP-capable name, the entry count and the byte cap — plus the absence of a leftover staging directory.
The module handed the editor a previewHttp block for a protocol the current build does not read, and never handed it the previewSnapshot one it does. The opaque preview was unreachable in Omeka: enabling external scripts found no route it understood and fell back to the filtered preview. EditorController now builds previewSnapshot — management URL, serving base and an explicit deleteUrlTemplate — from the same origin as saveEndpoint, and editor-bootstrap.phtml emits it. PreviewSessionController drops the four-operation dispatcher for the two the contract has: POST one whole ZIP (minting or replacing a capability), DELETE by id. Identity, the long-lived preview CSRF namespace and owner scoping are unchanged. PreviewController keeps everything worth keeping — the capability gate, the bare root 302, traversal-safe normalisation, the hardening headers, the sandbox CSP on scriptable types and the ETag/Range tier. Only the resolution changed: it reads from the snapshot directory and confirms the resolved realpath sits under the snapshot root, so a symlink cannot aim the response outside it. normalizePath moved verbatim onto the new store. Out: PreviewSessionStore, its factory, PreviewFixedResources and its factory, two of the four routes, and the v2 conformance fixtures whose test went with them. Net 3996 lines removed for 425 added. One thing the tests could not have caught, found by grepping for the deleted class rather than trusting a green suite: PreviewSessionControllerFactory still injected PreviewSessionStore. The unit tests build the controller directly, so they stayed green while the service manager would have fataled on the first real request. 570 tests pass, leaving the two failures the pre-merge baseline already had.
enqueueMediaHost() put exe-media-policy.js and exe-media-host.js on every secure page and attached the host to every content iframe. That is the parent half of the interactive-video bridge, and its child — exe_media_bridge.js — is not in asset/js/, is injected nowhere, and does not ship inside an extracted package. The host was loading, scanning the DOM and registering a listener per iframe for a handshake that could never arrive. The embed bridge is a different pair and stays exactly as it was: the shim IS injected into extracted content by ContentController and the relay IS enqueued by IframeSandbox, which is what makes external video play inside the opaque iframe. The two mirrored files stay under asset/js/. They are canonical copies shared with the other host plugins, and the interactive-video idevice may ship its child later; nothing loads them today.
erseco
changed the title
PreviewController still documented three-layer resolution (documents → assetRefs→assets → fixedRefs→manifest) and a cache tier per layer. Those layers went with the protocol they served; the class now resolves a normalized path inside one snapshot directory, confirmed with realpath().
The store's delete() collapses "does not exist" and "not yours" into a single false, so the controller answered 404 for both. Publishing over another author's capability answers 403 for the very same condition, which left the two halves of the management API disagreeing about what owner scoping looks like — and the serving contract documents 403/404. Adds PreviewSnapshotStore::ownerOf(), which keeps the two cases apart (null for an unknown capability, the owner id otherwise), and reuses it in delete() so the ownership rule lives in one place. Matches the same fix in the Nextcloud adapter, where an API end-to-end test against a real server caught the divergence.
…fixed tier Review pass over the snapshot migration. The 403-vs-404 fix landed one level too low. replace() already computed a full authorization verdict, so adding a thinner ownerOf() and rebuilding the verdict in the controller wrote the ownership rule twice — and the two verbs still disagreed on a malformed id (400 on publish, 404 on delete). That ladder becomes the public authorize(), replace() and deleteOwned() both run through it, and deleteAction is now a verdict-to-JSON map. PreviewController still branched on a 'fixed' kind for an immutable cache tier. The layer that produced it went with protocol v2; lookupPreviewFile() only ever returns document or asset, so the branch was unreachable — kept alive by two tests that fabricated descriptors the store can no longer emit. Branch, tests and the surrounding three-layer prose are gone. The guarantee those tests really protected — a scriptable SVG carries the sandbox CSP — is still covered from the tier that can actually serve one.
…nd ranges The serving route read the entire file and MD5'd it to build the ETag, before deciding anything. A conditional GET that ends as a 304 with an empty body, and a Range seek into a large video, both pulled the whole file into PHP memory and hashed it first — and scrubbing a video issues exactly those requests, over and over. The store lookup now describes an asset (path, size, identity-based ETag) rather than reading it, and the tier reads only once it knows what it is sending: a 304 and a 416 read nothing, a 206 seeks and reads its slice. The ETag becomes sha1(path|mtime|size), matching the Nextcloud adapter; a replace swaps the whole tree, so mtimes change and the tag still turns over on every refresh. A scriptable document is still read up front: it is always sent whole. The test seam keeps its readable ['kind' => 'asset', 'bytes' => '…'] form — the helper materialises those bytes into a temp file, so the cases stay as they were while the real read path is exercised.
Deriving the ETag from path, mtime and size traded a content hash for identity, but mtime has one-second granularity. An author who refreshes twice inside the same second with an edit that keeps a file the same length — a colour in a stylesheet — produced a byte-identical tag, so the browser was handed a 304 for the previous bytes and the change appeared not to take. Reproduced before fixing: after a replace with same-size content in the same second, mtime and size are unchanged while the content directory's inode is not, because every publish extracts into a fresh directory and renames it in. The inode joins the tag; where a filesystem does not report one it reads 0 and the tag degrades to the previous form, which is no worse.
…shake Enqueue the vendorized exe_external_media bundle from IframeSandbox, add a hello/welcome handshake between exe-embed-shim.js and exe-embed-relay.js, and cover the new behavior with controller/service tests and an e2e no-host fixture. CI gains a verify.mjs check step.
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.
Published viewer
An
.elpxrenders in a sandboxed, opaque-origin iframe under a response-levelsandbox CSP, so author HTML and JavaScript cannot reach the Omeka page, its
cookies or the session.
External video would blank out under that sandbox, so the shared eXe shim is
injected into the extracted package and demotes each provider embed to a
placeholder; a relay on the Omeka page overlays the real player in its place. No
separate subdomain needed.
Editor preview
Filtered by default. Enabling active content POSTs the whole project as one ZIP
to an authenticated endpoint and loads the capability URL it gets back, served
authless under the same sandbox CSP with a 30-minute idle TTL:
Snapshots live outside the web root, keyed per site. Archives are vetted by the
module's existing
ZipSafety, which rejects unsafe and PHP-capable entriesatomically before writing and caps zip bombs on the real decompressed bytes
rather than the sizes declared in the archive.
Notes for review
This module previously emitted a preview contract the editor no longer reads,
which left the opaque preview unreachable here: enabling external scripts
found no route the editor understood and silently fell back to the filtered
preview. Rebuilding the bundled editor from the current branch confirmed it
before any code moved. Aligning the module accounts for the size of this diff —
about 3 800 lines out for 425 in.
Two things worth a reviewer's attention:
PreviewSessionControllerFactorystill injected the deleted store after theport. The unit tests build the controller directly, so the suite stayed green
while the service manager would have fataled on the first real request. Found
by grepping for the removed class, not by the tests.
enqueueMediaHost()loaded the parent half of the interactive-video bridge onevery secure page and attached it to every iframe, but its child ships nowhere
in this module — a handshake that could never arrive. Removed; the mirrored
files stay for when the idevice ships its child.
Tests
PreviewSnapshotStoreTest(store + archive guards),PreviewSessionControllerTest(both management actions, identity/CSRF/method guards, owner scoping) and
PreviewControllerTest(serving contract). 570 pass, leaving two pre-existinglocal failures unrelated to this branch.