Skip to content

Secure opaque-origin viewer and editor preview - #68

Draft
erseco wants to merge 47 commits into
mainfrom
feature/secure-iframe-sandbox
Draft

Secure opaque-origin viewer and editor preview#68
erseco wants to merge 47 commits into
mainfrom
feature/secure-iframe-sandbox

Conversation

@erseco

@erseco erseco commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

What this does

Renders untrusted .elpx content in an opaque origin — a sandbox without
allow-same-origin — so author scripts cannot reach the Nextcloud session, and
gives the embedded editor a place to publish its opaque preview.

The preview contract

The editor sends the whole project as one ZIP and gets back an unguessable
capability id; the app serves that tree from an authless route under a sandbox
CSP.

Request
Management (authenticated, CSRF on, owner-scoped) POST /api/preview-session · DELETE /api/preview-session/{previewId}
Serving (authless capability URL) GET /preview/{previewId}/{path}

This app previously emitted a preview contract the editor no longer reads, which
left the opaque preview unreachable: it failed closed and silently stayed
filtered. Rebuilding the editor from its current branch confirmed which contract
it reads before any code moved.

Bounds kept, because one Nextcloud instance serves many users off one
filesystem: 30-min idle TTL, 4 snapshots/user and a 2 GiB global budget, both
LRU-evicting. Archives are vetted entry by entry before anything is written, and
the zip-bomb cap is measured on real decompressed bytes.

External video

An opaque origin fails YouTube's and Vimeo's embedder check (Error 153). A shim
inside the iframe demotes provider iframes to geometry placeholders and a relay
on the trusted parent overlays the real player over each one.

Also drops the parent half of the interactive-video media bridge: its child
script is not distributed, so it could never attach to anything.

Docs

docs/preview-serving-contract.md.

erseco added 2 commits July 6, 2026 13:19
…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.
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Preview this PR in the Nextcloud Playground

Open this PR in the Nextcloud Playground

A fresh Nextcloud boots in your browser with this branch's exelearning app installed and enabled (log in as admin / admin). Two sample .elpx are seeded under exelearning-samples/ in Files — click one to open the viewer.

eXeLearning editor: v4.0.2 (overlaid at boot from the upstream release).

erseco added 8 commits July 6, 2026 23:13
Single source of truth for the secure opaque-iframe policy shared with the other
plugins: secure sandbox tokens ('allow-scripts allow-popups allow-forms', never
allow-same-origin), the strict/compatible published CSP, the SVG/XML locked CSP,
Permissions-Policy, provider whitelist, and the dev-only legacy escape hatch.
Stateless HMAC-SHA256 token binding a fileId + expiry under the instance secret,
minted at view-open (authenticated) and verified by the cookieless opaque
content route. Mirrors Moodle's tokenpluginfile capability model. Secret injected
as a string to keep the service OCP-free and unit-testable.
…SP + shim)

ContentController serves published .elpx entries into an opaque-origin iframe over
a cookieless capability URL: verify the fileId-bound token, resolve the file
system-wide, read the entry, emit the strict published CSP on HTML (svgCsp on
SVG/XML) + hardening headers, and inline the eXe embed shim into HTML documents.
Adds EmbedShimInjector + PackageMimeService (OCP-free, tested) and refactors
AssetController onto the shared MIME service (single source of truth).
When a package resolves and secure mode is active, provide a fileId-bound
capability token (contentToken) + the secureIframe flag as initial state, so the
Vue viewer can load the package from the opaque /content/{token}/… route. The
read permission was already checked by getForUser*; legacy mode leaves the token
null and keeps the Service-Worker path.
…ilder

iframe-renderer now defaults to the secure opaque sandbox
('allow-scripts allow-popups allow-forms', no allow-same-origin) and adds
createContentIframe for the /content/{token} route; the legacy same-origin
Service-Worker path (createPackageIframe) keeps allow-same-origin + link rewiring
behind opaque=false. paths.ts gains buildContentUrl (token-addressed).
Mirror the eXe-core embed/media bridge (exe_embed_shim/relay + exe_media_policy/host)
into src/embed/ and drive them from the Viewer: the opaque content iframe's shim
promotes cross-origin/PDF players to placeholders, and relay-host.ts overlays real
players (Channel A) + hosts the interactive-video bridge (Channel B), pinging on
load and clearing/reflowing overlays on hide/resize. ElpxViewer now defaults to the
opaque /content/{token} path when a token is present, keeping the Service-Worker
path only under the legacy escape hatch. Mirror JS excluded from eslint/biome (kept
byte-synced with core).
Add IframeSandbox::embedMode() (strict default, open opt-in via
EXELEARNING_EMBED_OPEN) and pass {mode, whitelist} to the relay via initial
state, so the published viewer overlays only maintained providers unless opened
up. relay-host.startRelay now accepts the config (defaults to open).
@erseco erseco changed the title Add host-served opaque HTTP preview serving (eXe core preview contract) Add secure (opaque-origin) iframe mode: published viewer + editor preview Jul 6, 2026
@erseco
erseco marked this pull request as draft July 6, 2026 23:08
erseco added 16 commits July 7, 2026 00:14
The cookieless #[PublicPage] content route has no session, so IRootFolder::getById
resolved nothing and every /content/{token}/... returned 404. Bind the user id into
the token and resolve via getForUserById(uid, fileId) at serve time, which mounts
the user's storage and re-checks the read permission.
…he Playground

The php-wasm Playground has no real HTTP server (a Service Worker can't serve an
opaque iframe), so its published-viewer demo needs the same-origin path. The
nextcloud-playground blueprint schema can only reach PHP via config:app:set, so wire
IframeSandbox's injected envReader (in Application.php, keeping IframeSandbox OCP-free)
to fall back to a Nextcloud app-config value when the process env is unset — process
env still wins on real hosts. blueprint.json sets it with a setConfig step (loud
DEV-ONLY comment). ApplicationTest (7 tests) proves the precedence + secure-by-default.
Implement the eXeLearning editor-preview serving contract v2 as a
Nextcloud-native host: an authless, cookieless capability URL that serves
untrusted author HTML/JS from an opaque origin, plus an authenticated,
owner-scoped management API.

Protocol logic lives in OCP-free seams under lib/Service/Preview so every
rule is unit-testable without a server:

- PreviewPolicy: single source of truth for the sandbox-first CSP
  (byte-identical to core previewCspHeader), the scriptable-document set,
  the Permissions-Policy, served MIME resolution and traversal-safe path
  normalization.
- PreviewSessionStore: file-backed three-layer store (generated documents,
  session assets, fixed refs) with atomic revisions (staging + pointer
  swap), asset immutability, per-session/per-user/global budgets with LRU
  eviction, and a 30-min idle TTL.
- FixedResourceManifest: exact-key lookup into the installed editor
  distribution's bundles/preview-fixed-resources.json with distribution-root
  containment; absent manifest disables the layer (422, client demotes).
- PreviewServer: serving HTTP policy (hardening headers on every response
  incl. 404, tiered Cache-Control, ETag/304, single-range 206/416, sandbox
  CSP on every scriptable type from any layer).
- PreviewSessionApi: management HTTP policy (ownership gate, 409/422/413
  wire bodies, protocol-version negotiation).

Thin Nextcloud adapters on top: PreviewController (#[PublicPage] serving),
PreviewSessionController (#[NoAdminRequired], CSRF on, mirroring editor#save),
and PreviewCleanupJob (TimedJob sweeping expired sessions). Routes and the
background job are registered; the store root is resolved from the Nextcloud
data directory (the only OCP seam).

The editor is intentionally not wired to select the http transport yet.
Cover the serving-contract v2 seams without a Nextcloud server:

- PreviewPolicyTest: CSP byte-identity with core, scriptable classification,
  MIME resolution, path normalization (literal/encoded traversal, backslash,
  NUL), id/key validation.
- PreviewSessionStoreTest: lifecycle, per-user LRU cap, asset immutability
  (original bytes keep serving), atomic revision ordering, the full
  validation ladder (409/400/422/413), incremental deltas/deletes,
  three-layer resolution, and idle-TTL drop-on-access + sweep.
- FixedResourceManifestTest: manifest lookup, containment, and graceful
  disable when the manifest is absent/malformed.
- PreviewServerTest and PreviewSessionApiTest: header/CSP/cache/ETag/Range
  policy and the management status/body mapping.
- PreviewContractConformanceTest: replays tests/fixtures/preview-contract/
  vectors.json (vendored verbatim from eXe core) against the API + server,
  porting the harness interpretation so protocol semantics stay aligned with
  every other host.
Replace the v1 (manifest + content-addressed blob) host-side companion with
the v2 three-layer model: management API, authless serving route, tiered
Cache-Control, the byte-identical sandbox CSP, the fixed-resource manifest,
file-backed storage and cleanup. Document the app's two-CSP situation
(PreviewPolicy preview CSP vs IframeSandbox published CSP) and why they are
deliberately not unified.
Sync the byte-synced eXe-core embed bridge mirrors with core commit
054abfd0 (external-media relay stayed frozen when the exported page's nav
drawer / a class-flip reflow moved the content iframe with no scroll/resize
event).

exe_embed_shim.js init(): report(force) with a JSON dirty-check (force on
initial run, load and parent 'request' pings; observer-driven reports skip
identical geometry so attribute-noisy pages cannot spam the parent). The
MutationObserver now also watches attributes (class/style/hidden/open), and
transitionend/animationend + a documentElement/body ResizeObserver feed the
reflow schedule.

exe_embed_relay.js: positionOverlay records entry.lastRect; a new
checkDrift() re-pins any overlay whose content-iframe box moved without an
event and returns the moved count; it is exposed on the relay and run from a
300ms interval in init().

Changed regions are byte-identical to core (only the export wrapper differs).
Port core's checkDrift unit test (tests/js/exe-embed-relay.test.ts) driving
the raw mirror via createRelay, kept deterministic/offline by disabling
happy-dom child-frame navigation and file loading.
Authenticated disk-fill DoS: publishRevision wrote each revision's unique
content-addressed document blobs under the session tree but never pruned
superseded revisions, while the per-session (200 MiB) and global (2 GiB)
budgets count only the ACTIVE revision's documentBytes + assetBytes. An
authenticated user looping publishRevision with ~199 MiB of unique-content
documents per revision passed every budget check yet left each revision's
bytes on disk, so N revisions = N x ~199 MiB physical, unbounded — filling
the Nextcloud data volume.

Fix: after the atomic `current` pointer swap, prune to a single active
revision — delete superseded revision manifests and any document blob no
longer referenced by the active revision. Pruning runs under the same
exclusive publish flock (single writer) and AFTER the swap, so new readers
already resolve the new revision; a GET still pinned to the just-superseded
revision may 404 and re-sync (the designed recovery). Assets live in a
separate directory and are never pruned — they are shared and immutable
across revisions. A failed publish returns before any blob is written
(validation precedes the publish block), so it leaves no staging behind.

Physical document disk is now bounded by the active-revision byte budget.

TDD: new store test publishes several large unique-content revisions and
asserts only the active revision's manifest + single blob remain, the asset
survives every revision, and on-disk bytes stay bounded (not N x docSize).
…adyStored

storeAssets() reported a blob-write failure (disk full, unwritable assets
dir, link/rename failure) as `alreadyStored`. Per the contract's recovery
flow `alreadyStored` means "the server already holds these bytes", so the
client marked the key uploaded — the asset then 404s forever and the
missing-assets recovery loop could never repair it.

Distinguish the two false returns of writeBlobAtomic(): when the target now
exists the key appeared concurrently (bytes present → alreadyStored); when it
does not, the write genuinely failed (bytes absent → rejected with reason
'write-failed'). A rejected key is left un-uploaded, so a later revision
referencing it returns 422 missing-assets and the client re-uploads it.

TDD: new store test forces a deterministic, root-independent write failure
(replaces assets/ with a regular file → ENOTDIR) and asserts the key is
rejected 'write-failed' (not stored/alreadyStored), the blob is absent, and a
revision referencing it returns 422 missing-assets.
Sync exe_embed_relay.js with core: the 300ms checkDrift interval and the
window listeners installed by init() were left running for the page lifetime
with no teardown, and a second init() on the same relay stacked a duplicate
interval + duplicate listeners.

createRelay() now tracks driftTimer + started; dispose() runs clear() then
clears the drift interval and removes the message/resize/scroll/load
listeners (idempotent, safe before init or called twice), exposed on the
relay; init() early-returns when already started and stores the interval
handle in driftTimer.

Changed region is byte-identical to core (only the export wrapper differs).
Port core's dispose unit test into tests/js/exe-embed-relay.test.ts (tears
down overlays like clear(), safe before init and when called twice).
Inject a previewHttp block into window.__EXE_EMBEDDING_CONFIG__ so the
embedded editor can drive the HTTP editor-preview transport (serving
contract v2): protocolVersion 2, a management base URL, a serving base
URL, and the current Nextcloud CSRF token as the requesttoken header.

Both URLs are generated server-side through IURLGenerator::linkToRoute so
they carry the correct webroot and front-controller prefix under a
sub-path install (mirroring @nextcloud/router generateUrl on the client);
the serving base is derived by generating the bare capability-root URL for
a placeholder id and stripping the id segment. The requesttoken is the
same encrypted value the standard template layer exposes as
data-requesttoken (CsrfTokenManager::getToken()->getEncryptedValue()),
which the management routes require because they keep CSRF on.

The bundled v4.0.2 editor predates the HTTP preview client and simply
ignores the block; a capable build activates it with no server change.
Two serving-policy corrections mandated by the normalized contract (v2.1
section 4), both in the OCP-free PreviewServer so they stay unit-testable
and vector-replayable:

- Range: a malformed, multi-range or non-bytes header is now IGNORED and
  the server returns a normal 200 full body. 416 is reserved for a
  syntactically valid single range that is unsatisfiable (e.g. bytes=99-
  past EOF). parseRange now distinguishes 'ignore' (null) from
  'unsatisfiable' so the caller can tell the two apart; the old behaviour
  of 416-on-any-parse-failure was wrong.

- Bare capability root: GET {servingBase}/{previewId} now 302-redirects to
  {previewId}/index.html instead of serving index.html bytes inline. Served
  inline, the document's relative subresource references resolve against
  preview/ (dropping the id segment) and every asset 404s. The Location is
  relative so it is correct under any webroot.
A missing/unreadable .accessed marker made isExpired() return false, so the
session became immortal: it counted against the global byte budget forever
and sweepExpired could never reclaim it. Fall back to meta.json createdAt as
the age clock when the marker is gone; if meta.json is missing/corrupt too,
the directory is unusable and is treated as expired so the sweep reclaims it.
globalBytes() is recomputed from the surviving session directories on every
call, so removing the directory reconciles the accounting automatically.

Tests: a session whose .accessed was deleted is swept after TTL via the
createdAt fallback; a directory with neither marker nor meta.json is
reclaimed.
A dropped/failed files[] part became a silently-empty document (sha1(''),
size 0) and the revision published anyway. The controller now aligns every
declared write with a healthy uploaded part (present, UPLOAD_ERR_OK,
readable); any missing/failed/unreadable part — or a part/write count
mismatch — rejects the whole batch with 400 before any staging, so the
revision pointer never advances. uploadedFileParts() carries per-part upload
health (a genuinely empty but successfully uploaded file stays valid);
uploadedFileBytes() keeps the asset path unchanged by delegating to it.

The store mirrors this rigor: a genuine document blob-write failure aborts
applyRevision with 500 before the manifest write and pointer swap, so the
active revision is never advanced onto a missing blob (an 'already present'
content-addressed blob is not a failure). PreviewSessionApi maps 500 through.

Test: a document write failure returns 500 and leaves the previous revision
active.
…iptions

- docs/preview-serving-contract.md: replace the dead previewTransport /
  previewBasePath vocabulary with the normalized previewHttp two-URL model,
  document that editor activation is now wired plus its editor-build
  dependency, and note the bare-root 302 and malformed-Range->200 CHANGES
  with a vectors re-vendor-pending caveat (vectors are not forked here).
- README.md + appinfo/info.xml: the viewer is served over the opaque
  /content HTTP route by default (a sandbox without allow-same-origin), not
  purely the Service Worker; note the HTTP editor-preview path and that only
  the legacy opt-in viewer needs Service Workers.
- CHANGELOG.md: correct the stale claim that CI 'verifies the Service Worker
  route responds' to what CI actually does.
erseco added 6 commits July 11, 2026 12:41
Add a CI job that boots a real Nextcloud, enables the app, serves it over
HTTP and drives the editor-preview management and serving routes end to end
(tests/e2e/preview-api-e2e.sh). Cookie/session auth is used on purpose so
the CSRF middleware is genuinely exercised. It asserts: management rejects a
create without a requesttoken (412, CSRF proof); create returns 201 with
protocolVersion 2; asset upload + revision publish round-trip; the authless
serving response is 200 with the opaque sandbox CSP and no allow-same-origin;
the bare root 302-redirects; a second user's management call is 403; a
dropped multipart part is rejected (400) without advancing the revision; and
after DELETE the serving route 404s.

Full editor-iframe browser E2E stays blocked on a capable editor build and
is documented as such rather than faked.
The built-in PHP server's document root is the Nextcloud tree, so the
top-level status.php is served directly; /index.php/status.php routes
through the front controller and 404s. Probe /status.php (and assert
installed:true) so the readiness gate reflects a genuinely ready server.
Per the frozen contract (v2.1 section 4 / RFC 9110 section 14.1.2), a range
whose last-byte-pos is less than its first-byte-pos (bytes=5-2) is an INVALID
spec and must be ignored (200 full body), not answered 416. parseRange now
returns null for that case; 416 stays reserved for a valid but unsatisfiable
single range — first-byte-pos at/after EOF (bytes=99-) or a zero-length
suffix (bytes=-0). Adds bytes=5-2 (ignored) and bytes=-0 (416) tests.
Record the CSRF-TTL audit conclusion: a Nextcloud request token is bound to a
per-session secret, so the value injected into previewHttp.managementHeaders
stays valid for the whole editing session (every getEncryptedValue() encoding
validates against the same secret). The secret regenerates only on session-id
regeneration (login/logout/re-auth), which reloads the parent page and re-injects
a fresh token; the parent's keepalive heartbeat keeps the session alive
meanwhile. The injected-token approach is durable — no postMessage refresh path
is required now.
The login helper died silently before the first assertion: under set -euo
pipefail, scrape_token's grep returns non-zero on no match, which tripped
set -e inside the RT1=$(login ...) command substitution and aborted with no
output. Guard scrape_token with '|| true' so an empty result is handled by
the caller's check instead of killing the script, capture the login pages to
files and dump a diagnostic (status + snippet) when the requesttoken is
absent, and follow post-login redirects with -L. Also tail data/nextcloud.log
on failure (occ has no log:tail command).
The post-login step scraped /apps/files/ HTML for data-requesttoken, which
came back empty under php -S (the failure the last run surfaced). Replace it
with Nextcloud's canonical /csrftoken JSON endpoint — the same source
OC.requestToken refreshes from — parsed with jq. It requires a live
authenticated session, so it doubles as a login check: an unauthenticated hit
returns a login redirect (not JSON) and yields an empty token with a
diagnostic dump, instead of masking a login failure with a guest token.
@erseco erseco changed the title Add secure (opaque-origin) iframe mode: published viewer + editor preview Secure opaque-origin iframe: published viewer + external-media relay + HTTP editor preview (serving contract v2) Jul 11, 2026
erseco added 4 commits July 11, 2026 13:50
The management create returned 401 (unauthenticated), not 412: the cookie
session was never authenticated. Root cause is almost certainly Secure session
cookies — behind CI's proxy Nextcloud defaults to https, marks the session
cookie Secure, and curl refuses to send it over the plain-http php -S server,
so every authenticated request falls through to 401 (while /csrftoken still
hands a guest token back, masking it). Force overwriteprotocol=http so the
session cookie is not Secure.

Also make login() verify authentication explicitly via the OCS cloud/user
endpoint (which returns the authenticated user id, unlike /csrftoken) and dump
the login POST status + cookie-jar names on failure, and dump the create body
(asserting it mentions CSRF) when the 412 assertion misses — so any remaining
auth issue is diagnosable in one run instead of opaque.
The login POST returns 200 (not the 303 of a successful login) and the
session stays unauthenticated: the login form's own strict-cookie/CSRF check
fails because the SameSite/session cookies aren't round-tripping over php -S
plain HTTP. Complete the plain-HTTP config: overwrite.cli.url without the
/index.php suffix (so cookies get Path=/), overwriteprotocol=http, and delete
forcessl/overwritehost so nothing forces https (Secure cookies).

Make the failure fully diagnosable in one run: dump the login flow's
Set-Cookie headers, the raw cookie jar, and the login POST body (which carries
NC's error) so the exact cookie attribute or login error is visible instead of
inferred.
The browser login form's SameSite/session cookies do not round-trip under the
built-in php -S server (the raw cookie jar stayed empty and the login POST
re-rendered the form), so a cookie session can't be established there. Switch
the E2E to HTTP Basic auth, which authenticates per request via the
Authorization header with no cookies.

CSRF is still exercised honestly using Nextcloud's own rule
(Request::passesCSRFCheck): the positive calls send the OCS-APIRequest header
(which makes passesCSRFCheck pass without a session/token), and an
AUTHENTICATED management POST WITHOUT that header is rejected 412 'CSRF check
failed' — the real proof that the management routes are not #[NoCSRFRequired].
The full round-trip, authless opaque serving (200 + sandbox CSP, no
allow-same-origin), bare-root 302, ownership 403, dropped-part 400 (revision
unchanged) and post-delete 404 all run cookielessly against real Nextcloud.

No product code changes; harness only.
@erseco erseco changed the title Secure opaque-origin iframe: published viewer + external-media relay + HTTP editor preview (serving contract v2) Secure opaque-origin viewer and HTTP editor preview v2 Jul 12, 2026
erseco added 6 commits July 12, 2026 10:35
The embedded editor stopped speaking protocol v2 months ago. This app kept
emitting `previewHttp` with `protocolVersion: 2` — a key the current editor
bundle has zero references to — and never emitted `previewSnapshot`, the one it
does read. The opaque preview was therefore unreachable: it failed closed and
silently stayed filtered.

Verified rather than assumed: a bundle rebuilt from the editor's current branch
contains `previewSnapshot`, `managementUrl`, `servingBaseUrl` and
`deleteUrlTemplate`, and no `previewHttp` or `protocolVersion` at all.

The editor now sends the whole project as one ZIP per opaque refresh, so the
layered store (immutable asset keys, incremental revisions, a fixed-resource
manifest resolved out of the editor distribution) has nothing left to optimise:

- PreviewSnapshotStore + SnapshotArchive replace PreviewSessionStore,
  PreviewSessionApi, FixedResourceManifest and ApiResult. Archives are vetted
  entry by entry before anything is written, and the zip-bomb cap is enforced on
  the real decompressed bytes as they stream out — the sizes in a ZIP's central
  directory are supplied by whoever built the archive.
- The four management operations collapse to POST create/replace and DELETE.
- PreviewServer keeps its header, cache, ETag and range policy; it only loses
  the fixed layer.

What survives the simplification, because Nextcloud is the one adapter serving
many users off one filesystem: the per-user snapshot cap and the global byte
budget, both LRU-evicting, plus the idle TTL.

Also drops the parent half of the interactive-video media bridge. Its child
(`exe_media_bridge.js`) is not distributed, so `exe_media_host.js` and
`exe_media_policy.js` could never attach to anything. The embed relay that makes
external video work inside the opaque iframe is untouched.
@erseco erseco changed the title Secure opaque-origin viewer and HTTP editor preview v2 Secure opaque-origin viewer and editor preview Jul 25, 2026
erseco added 4 commits July 25, 2026 17:33
The end-to-end check still drove the retired four-operation management API:
it POSTed an empty body to create a session, then uploaded assets and published
a revision. Against the snapshot endpoint that first call is a 400 ("Missing
snapshot upload"), so the job failed.

This is the one consumer the migration missed: a shell script, so it never
turned up in the PHP and TypeScript greps used to find dangling references.

Rewritten around the two real operations, keeping every security assertion it
already made — CSRF 412, authless serving with the sandbox CSP and no
allow-same-origin, the bare-root 302, owner scoping and 404 after delete — and
adding three the snapshot contract makes worth checking:

- replacing in place keeps the capability id and swaps the WHOLE tree, so a path
  the new archive omits stops resolving;
- a rejected archive leaves the live snapshot untouched, so a bad upload can
  never destroy a preview that was working;
- a non-owner may not replace someone else's capability, not just not delete it.

Archives are built with python3 rather than a zip binary so the contents are
exact and the script depends on nothing extra on the runner.
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.

The distinction is drawn in the controller: 404 for an unknown capability, 403
for one owned by somebody else. Covered by the API E2E against a real Nextcloud,
which asserts both the cross-user publish and the cross-user delete.
… branches

Review pass over the snapshot migration.

The 403-vs-404 fix landed one level too low. The store already produced a full
authorization verdict for the publish path, 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).
guardExistingCapability() becomes the public authorize(), deleteOwned() runs
through it, and the controller is now a verdict-to-JSON map on both verbs.
exists() goes: it said nothing ownerOf() did not.

PreviewServer carried three CSP branches that could never decide anything:
resolve() labels a file a document exactly when its type is scriptable, so the
two checks inside serveAsset() were unreachable and the one in serveBytes() was
always true. They read as security policy while being dead. The kind now IS the
scriptability — one field instead of two that could drift — and each tier states
plainly why it does or does not carry the CSP. serveAsset's four near-identical
header blocks collapse to one, which also stops the 304 from silently omitting
Content-Type and Accept-Ranges.

SnapshotArchive::extract() counted every byte it wrote and threw the total away,
so the store walked the whole tree again to recount it on every publish. It now
returns the number; treeBytes() is gone.

Also deletes ASSET_KEY_RE, isValidAssetKey() and limits(), which described and
validated a wire format the snapshot contract no longer has.
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.

The content directory's inode joins the tag: every publish extracts into a fresh
directory and renames it in, so it always turns over. Where a filesystem does
not report one it reads 0 and the tag degrades to the previous form, which is no
worse. The regression test fails against that previous form.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant