diff --git a/AGENTS.md b/AGENTS.md index b5798f4..49bc988 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -40,8 +40,8 @@ protocol, the iframe boot HTML pattern). Remove everything Drive-specific. - **Do not add a non-Nextcloud backend.** Anything server-side runs as a Nextcloud controller, service, or preview provider. CSRF protection, authentication and permissions go through Nextcloud's APIs. -- **Use `@nextcloud/viewer`, `@nextcloud/files`, `@nextcloud/axios`, - `@nextcloud/router`, `@nextcloud/l10n`**. Do not introduce React, Angular, +- **Use `@nextcloud/files`, `@nextcloud/axios`, `@nextcloud/router`, + `@nextcloud/l10n`**. Do not introduce React, Angular, or alternative HTTP clients. - **Service Worker scope must stay narrow.** Only `/apps/exelearning/runtime/` may be intercepted. The SW must never see @@ -109,7 +109,7 @@ that matches before starting: | Skill | When | |---|---| | `architecture-records` | Writing or reviewing an ADR or a change document | -| `nextcloud-app` | Touching `lib/` — controllers, services, DI, routes, preview | +| `nextcloud-app-development` | Touching `lib/` — controllers, services, DI, routes, preview | | `elpx-package-safety` | Touching ZIP entry handling, entry-path validation or the Service Worker | | `testing` | Adding or fixing tests in `tests/js/` or `tests/Unit/` | @@ -147,13 +147,16 @@ seen them pass. ## Where to look -- `src/main.ts` — registers the Viewer handler and Files actions. -- `src/viewer/ElpxViewer.vue` — the Viewer modal component. +- `src/main.ts` — registers the Files actions and New-menu entry. +- `src/view/*` — the full-page view (`ElpxViewPage.vue`) with its preview + and embedded-editor (`EditorEmbed.vue`) modes. +- `src/viewer/ElpxViewer.vue` — the package preview component. - `src/elpx/*` — pure-TS extraction, validation, session, SW client, paths. - `src/files/*` — Files-app integration: MIME helpers and actions. - `src/editor/*` — optional editor scaffold using the upstream embedding protocol; safe to ignore unless touching the editor. -- `js/exelearning-sw.js` — the Service Worker itself, hand-written. +- `src/sw/exelearning-sw.js` — the Service Worker itself, hand-written and + served as-is by `SwController`. - `lib/AppInfo/Application.php` — app bootstrap, init script, preview provider registration. - `lib/Controller/*` — HTTP boundary, one controller per concern. diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index bf22d83..18739ff 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -160,7 +160,7 @@ elpx/package-validator.ts → require index.html ↓ elpx/viewer-session.ts → session id + per-entry map ↓ -elpx/service-worker-client.ts → POST session into js/exelearning-sw.js +elpx/service-worker-client.ts → POST session into src/sw/exelearning-sw.js ↓ elpx/iframe-renderer.ts → sandboxed iframe at /apps/exelearning/runtime/{session}/index.html @@ -202,8 +202,9 @@ appear in the Files row menu. Save flow: 1. The editor iframe posts `SAVE_FILE` with the new bytes. -2. `editor-page.ts` POSTs them to `/apps/exelearning/editor/save` with the - open-file `If-Match` ETag. +2. `EditorEmbed.vue` POSTs them to `/apps/exelearning/editor/save` with the + open-file `If-Match` ETag. The Save button and Ctrl/Cmd+S inside the + editor both go through this path. 3. The PHP controller refuses the write if the file changed on disk (`HTTP 412`). @@ -211,7 +212,9 @@ Save flow: - `.elpx` HTML never runs in the parent Nextcloud window. - The iframe uses - `sandbox="allow-scripts allow-same-origin allow-forms allow-popups allow-downloads"`. + `sandbox="allow-scripts allow-same-origin allow-forms allow-popups allow-downloads allow-popups-to-escape-sandbox"`. +- The server-side asset fallback refuses top-level navigations + (`Sec-Fetch-Dest: document`), so package HTML only renders framed. - The Service Worker only intercepts URLs under `/apps/exelearning/runtime/`. - All ZIP paths are normalized; `..`, absolute paths and NUL-tainted entries diff --git a/Makefile b/Makefile index 46826dc..3b77743 100644 --- a/Makefile +++ b/Makefile @@ -395,7 +395,7 @@ up: check-docker echo "ERROR: js/$(APP_NAME)-main.mjs missing after build."; exit 1; \ fi @# Make sure the optional eXeLearning static editor is present so the - @# /apps/exelearning/editor route works inside the container. Skip the + @# editor mode of /apps/exelearning/view works in the container. Skip the @# download when js/editor/index.html already exists; refresh the @# bundle explicitly with `make download-editor` (optionally pinning @# EXELEARNING_EDITOR_REF=vX.Y.Z) when a new upstream tag ships. diff --git a/appinfo/routes.php b/appinfo/routes.php index ded2661..bf094eb 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -19,7 +19,7 @@ 'name' => 'asset#fetch', 'url' => '/asset/{sessionId}/{path}', 'verb' => 'GET', - 'requirements' => ['path' => '.+'], + 'requirements' => ['sessionId' => '\d+', 'path' => '.+'], ], [ 'name' => 'thumbnail#byFileId', diff --git a/biome.json b/biome.json index 48e4ede..3f4d06f 100644 --- a/biome.json +++ b/biome.json @@ -1,5 +1,5 @@ { - "$schema": "https://biomejs.dev/schemas/2.4.15/schema.json", + "$schema": "https://biomejs.dev/schemas/2.5.14/schema.json", "vcs": { "enabled": true, "clientKind": "git", diff --git a/codecov.yml b/codecov.yml index 3cdf0ac..3f7250c 100644 --- a/codecov.yml +++ b/codecov.yml @@ -16,9 +16,6 @@ ignore: - "tests/**/*" - "vendor/**/*" - "src/main.ts" - - "src/editor/editor-frame.ts" - - "src/elpx/elpx-loader.ts" - - "src/elpx/service-worker-client.ts" - "src/files/actions.ts" - "src/files/new-menu.ts" - "src/sw/**/*" diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index d7a6019..9322d32 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -60,9 +60,9 @@ public function register(IRegistrationContext $context): void { } public function boot(IBootContext $context): void { - // Register the Viewer handler as an init script. Init scripts are - // emitted by the server in , so the handler is available before - // the Viewer app probes for MIME associations. + // Register the Files actions as an init script. Init scripts are + // emitted by the server in , so they exist before the Files app + // collects actions. unset($context); Util::addInitScript(self::APP_ID, 'exelearning-main'); } diff --git a/src/files/actions.ts b/src/files/actions.ts index 2db2a7b..0baee78 100644 --- a/src/files/actions.ts +++ b/src/files/actions.ts @@ -159,7 +159,7 @@ const downloadAction: IFileAction = { */ export function registerFileActions(): void { registerFileAction(viewAction) // default — opens /apps/exelearning/view - registerFileAction(editAction) // kebab — opens /apps/exelearning/editor + registerFileAction(editAction) // kebab — opens /apps/exelearning/view?mode=editor registerFileAction(downloadAction) // kebab — native download registerFileAction(openAsExeLearningAction) // kebab on plain .zip } diff --git a/src/sw/exelearning-sw.js b/src/sw/exelearning-sw.js index 3c559e2..19943a9 100644 --- a/src/sw/exelearning-sw.js +++ b/src/sw/exelearning-sw.js @@ -13,7 +13,7 @@ * Sessions are kept only in this worker's memory. Closing the tab (and the * subsequent SW termination) drops them. The page is the source of truth. * - * This file is shipped as-is — not bundled by webpack — and served by + * This file is shipped as-is — not bundled by Vite — and served by * SwController so the response can set Content-Type and Service-Worker-Allowed. */ @@ -64,8 +64,8 @@ self.addEventListener('message', (event) => { /** * Stores a new session in the in-memory map. Files arrive as a list of - * `{ path, mime, bytes }`; the path is normalised here so request matching - * never has to deal with `..`/`.` segments or backslashes. + * `{ path, mime, bytes }`; non-canonical paths are rejected here, so request + * matching never has to deal with `..`/`.` segments or backslashes. * @param {{ sessionId: string, files: Array<{ path: string, mime?: string, bytes: ArrayBuffer | null }>, indexEntry?: string, filename?: string }} data * `EXELEARNING_REGISTER_SESSION` message payload from the page. */ @@ -178,7 +178,7 @@ function safeDecode(value) { /** * SW-side mirror of `normalizeEntryPath` from src/elpx/paths.ts. Kept * inline because the SW must not import from the bundled application - * code (it is loaded out-of-band by the browser, not by webpack). + * code (it is loaded out-of-band by the browser, not by Vite). * * Validates and returns the input unchanged, or null. A path is accepted * only when it is non-empty, free of NUL bytes and backslashes, and made diff --git a/src/viewer/ElpxViewer.vue b/src/viewer/ElpxViewer.vue index 15939cb..ee802a2 100644 --- a/src/viewer/ElpxViewer.vue +++ b/src/viewer/ElpxViewer.vue @@ -63,7 +63,7 @@ export default defineComponent({ name: 'ElpxViewer', components: { ViewerError }, props: { - // @nextcloud/viewer passes these for any registered handler. + // File identity, passed by ElpxViewPage. filename: { type: String, default: '' }, basename: { type: String, default: '' }, source: { type: String, default: '' }, diff --git a/tests/js/editor-frame.test.ts b/tests/js/editor-frame.test.ts new file mode 100644 index 0000000..447ae4e --- /dev/null +++ b/tests/js/editor-frame.test.ts @@ -0,0 +1,128 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { EditorFrame } from '../../src/editor/editor-frame' + +let container: HTMLElement +let frame: EditorFrame + +function iframeWindow(): Window { + const iframe = container.querySelector('iframe') + if (!iframe?.contentWindow) throw new Error('iframe not attached') + return iframe.contentWindow +} + +/** Delivers a message to the page as if the editor iframe had posted it. */ +function fromEditor(data: unknown, source: Window | null = iframeWindow()): void { + window.dispatchEvent(new MessageEvent('message', { data, source })) +} + +/** Captures what the page posts into the editor iframe. */ +function spyOnPosts() { + return vi.spyOn(iframeWindow(), 'postMessage').mockImplementation(() => undefined) +} + +async function readyFrame(): Promise { + const loading = frame.load() + fromEditor({ type: 'EXELEARNING_READY' }) + await loading +} + +beforeEach(() => { + container = document.createElement('div') + document.body.appendChild(container) + frame = new EditorFrame(container, { editorIframeUrl: 'about:blank' }) +}) + +afterEach(() => { + frame.destroy() + container.remove() + vi.useRealTimers() +}) + +describe('EditorFrame', () => { + it('refuses to talk to the editor before it reports ready', async () => { + await expect(frame.requestSave()).rejects.toThrow('not ready') + }) + + it('ignores messages that do not come from its own iframe', async () => { + vi.useFakeTimers() + const loading = frame.load() + fromEditor({ type: 'EXELEARNING_READY' }, window) + vi.advanceTimersByTime(60_000) + + await expect(loading).rejects.toThrow('Timed out waiting for EXELEARNING_READY') + }) + + it('resolves a save only with the reply to its own request id', async () => { + await readyFrame() + const posts = spyOnPosts() + + const saving = frame.requestSave() + const requestId = (posts.mock.calls[0][0] as { requestId: string }).requestId + fromEditor({ type: 'SAVE_FILE', requestId: 'someone-else', bytes: new Uint8Array([9]) }) + fromEditor({ type: 'SAVE_FILE', requestId, bytes: new Uint8Array([1, 2]), filename: 'a.elpx' }) + + const saved = await saving + expect(new Uint8Array(saved.bytes)).toEqual(new Uint8Array([1, 2])) + expect(saved.filename).toBe('a.elpx') + }) + + it('rejects a save when the editor reports an error', async () => { + await readyFrame() + const posts = spyOnPosts() + + const saving = frame.requestSave() + const requestId = (posts.mock.calls[0][0] as { requestId: string }).requestId + fromEditor({ type: 'REQUEST_SAVE_ERROR', requestId, error: 'disk full' }) + + await expect(saving).rejects.toThrow('disk full') + }) + + it('transfers the package bytes when opening a file', async () => { + await readyFrame() + const posts = spyOnPosts() + const bytes = new ArrayBuffer(4) + + const opening = frame.openFile({ bytes, filename: 'lesson.elpx' }) + const [message, , transfer] = posts.mock.calls[0] as unknown as [{ requestId: string }, string, Transferable[]] + expect(transfer).toEqual([bytes]) + fromEditor({ type: 'OPEN_FILE_SUCCESS', requestId: message.requestId }) + + await expect(opening).resolves.toBeUndefined() + }) + + it('times out a save the editor never answers', async () => { + await readyFrame() + spyOnPosts() + vi.useFakeTimers() + + const saving = frame.requestSave() + vi.advanceTimersByTime(60_000) + + await expect(saving).rejects.toThrow('Timed out waiting for the eXeLearning editor') + }) + + it('reports an error type without a message and a detached iframe', async () => { + await readyFrame() + const posts = spyOnPosts() + + const opening = frame.openFile({ bytes: new ArrayBuffer(1), filename: 'x.elpx' }) + const requestId = (posts.mock.calls[0][0] as { requestId: string }).requestId + fromEditor({ type: 'OPEN_FILE_ERROR', requestId }) + await expect(opening).rejects.toThrow('OPEN_FILE_ERROR') + + container.querySelector('iframe')?.remove() + await expect(frame.requestSave()).rejects.toThrow('not available') + }) + + it('forwards unsolicited editor messages to subscribers until they unsubscribe', () => { + const seen: string[] = [] + const unsubscribe = frame.onMessage((message) => seen.push(message.type)) + + fromEditor({ type: 'REQUEST_SAVE' }) + unsubscribe() + fromEditor({ type: 'REQUEST_SAVE' }) + fromEditor('not an editor message') + + expect(seen).toEqual(['REQUEST_SAVE']) + }) +}) diff --git a/tests/js/elpx-loader.test.ts b/tests/js/elpx-loader.test.ts new file mode 100644 index 0000000..4750a39 --- /dev/null +++ b/tests/js/elpx-loader.test.ts @@ -0,0 +1,61 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const get = vi.fn() +vi.mock('@nextcloud/axios', () => ({ default: { get } })) +vi.mock('@nextcloud/router', () => ({ + generateUrl: (path: string, params?: Record) => + path.replace(/\{(\w+)\}/g, (_, key: string) => String(params?.[key])), +})) + +const { loadElpx } = await import('../../src/elpx/elpx-loader') + +function respond(headers: Record = {}) { + get.mockResolvedValueOnce({ data: new ArrayBuffer(3), headers }) +} + +describe('loadElpx', () => { + beforeEach(() => get.mockReset()) + + it('requires a file id or a path', async () => { + await expect(loadElpx({})).rejects.toThrow('Either fileId or path') + await expect(loadElpx({ path: '' })).rejects.toThrow('Either fileId or path') + }) + + it('loads by file id and reads filename and etag from the headers', async () => { + respond({ 'content-disposition': 'attachment; filename*=UTF-8\'\'Lecci%C3%B3n%201.elpx', etag: '"abc"' }) + + const loaded = await loadElpx({ fileId: 7 }) + + expect(get).toHaveBeenCalledWith('/apps/exelearning/package/by-file-id/7', { responseType: 'arraybuffer' }) + expect(loaded).toMatchObject({ filename: 'Lección 1.elpx', etag: '"abc"' }) + }) + + it('loads by path and accepts a plain quoted filename', async () => { + respond({ 'content-disposition': 'attachment; filename="plain.elpx"' }) + + const loaded = await loadElpx({ path: 'Docs/x.elpx' }) + + expect(get).toHaveBeenCalledWith('/apps/exelearning/package/by-path', { + responseType: 'arraybuffer', + params: { path: 'Docs/x.elpx' }, + }) + expect(loaded.filename).toBe('plain.elpx') + expect(loaded).not.toHaveProperty('etag') + }) + + it('keeps an undecodable filename verbatim', async () => { + respond({ 'content-disposition': 'attachment; filename*=UTF-8\'\'bad%E0.elpx' }) + + expect((await loadElpx({ fileId: 1 })).filename).toBe('bad%E0.elpx') + }) + + it('derives a filename when the server sends no Content-Disposition', async () => { + respond() + respond() + respond({ 'content-disposition': 'inline' }) + + expect((await loadElpx({ path: 'Docs/lesson.elpx' })).filename).toBe('lesson.elpx') + expect((await loadElpx({ path: 'top.elpx' })).filename).toBe('top.elpx') + expect((await loadElpx({ fileId: 9 })).filename).toBe('package-9.elpx') + }) +}) diff --git a/tests/js/service-worker-client.test.ts b/tests/js/service-worker-client.test.ts index ce43491..c86590b 100644 --- a/tests/js/service-worker-client.test.ts +++ b/tests/js/service-worker-client.test.ts @@ -9,11 +9,11 @@ function fakeWorker(state: ServiceWorkerState) { return worker } -async function loadClient(installing: ServiceWorker) { +async function loadClient(installing: ServiceWorker | null, active: ServiceWorker | null = null, controller: object | null = null) { vi.resetModules() - const registration = { installing, waiting: null, active: null } + const registration = { installing, waiting: null, active } vi.stubGlobal('navigator', { - serviceWorker: { controller: null, register: vi.fn().mockResolvedValue(registration) }, + serviceWorker: { controller, register: vi.fn().mockResolvedValue(registration) }, }) return import('../../src/elpx/service-worker-client') } @@ -35,6 +35,28 @@ describe('ensureRuntimeWorker', () => { await expect(pending).resolves.toMatchObject({ scope: '/apps/exelearning/runtime/' }) }) + it('resolves at once for an already active or controlling worker and caches it', async () => { + const active = fakeWorker('activated') + const controlled = await loadClient(null, active, {}) + const first = await controlled.ensureRuntimeWorker() + expect(await controlled.ensureRuntimeWorker()).toBe(first) + + const plain = await loadClient(null, active) + await expect(plain.ensureRuntimeWorker()).resolves.toBeDefined() + + const none = await loadClient(null) + await expect(none.ensureRuntimeWorker()).resolves.toBeDefined() + }) + + it('shares one registration between concurrent callers', async () => { + const { ensureRuntimeWorker } = await loadClient(null, fakeWorker('activated')) + + const [a, b] = await Promise.all([ensureRuntimeWorker(), ensureRuntimeWorker()]) + + expect(a).toBe(b) + expect(navigator.serviceWorker.register).toHaveBeenCalledTimes(1) + }) + it('rejects when the worker becomes redundant so the viewer can fall back', async () => { const worker = fakeWorker('installing') const { ensureRuntimeWorker } = await loadClient(worker) @@ -47,3 +69,84 @@ describe('ensureRuntimeWorker', () => { await expect(pending).rejects.toThrow('failed to activate') }) }) + +describe('session messages', () => { + afterEach(() => { + vi.unstubAllGlobals() + }) + + /** Active worker that answers every message on the transferred port. */ + function replyingWorker(reply: unknown) { + const posted: Array> = [] + const worker = Object.assign(fakeWorker('activated'), { + postMessage: (message: Record, [port]: MessagePort[]) => { + posted.push(message) + port.postMessage(reply) + }, + }) + const runtime = { + registration: { active: worker, waiting: null, installing: null } as unknown as ServiceWorkerRegistration, + scriptUrl: '', scope: '', runtimeBase: '', + } + return { runtime, posted } + } + + const session = { + id: 's1', + indexEntry: 'index.html', + filename: 'x.elpx', + data: { files: new Map([['index.html', { mime: 'text/html', bytes: new Uint8Array([1, 2, 3]).subarray(1) }]]) }, + } + + it('sends each entry as its own ArrayBuffer and resolves on ok', async () => { + const { registerSession } = await loadClient(fakeWorker('activated')) + const { runtime, posted } = replyingWorker({ ok: true }) + + await registerSession(runtime, session as never) + + expect(posted[0]).toMatchObject({ type: 'EXELEARNING_REGISTER_SESSION', sessionId: 's1', indexEntry: 'index.html' }) + const [file] = posted[0].files as Array<{ bytes: ArrayBuffer }> + expect(new Uint8Array(file.bytes)).toEqual(new Uint8Array([2, 3])) + }) + + it('surfaces a rejection from the worker', async () => { + const { registerSession } = await loadClient(fakeWorker('activated')) + const { runtime } = replyingWorker({ ok: false, error: 'quota' }) + + await expect(registerSession(runtime, session as never)).rejects.toThrow('quota') + }) + + it('rejects when there is no worker or posting fails', async () => { + const { registerSession } = await loadClient(fakeWorker('activated')) + const { runtime } = replyingWorker({}) + const gone = { ...runtime, registration: { active: null, waiting: null, installing: null } as unknown as ServiceWorkerRegistration } + const broken = Object.assign(fakeWorker('activated'), { + postMessage: () => { + throw new Error('DataCloneError') + }, + }) + const throwing = { ...runtime, registration: { active: broken } as unknown as ServiceWorkerRegistration } + + await expect(registerSession(gone, session as never)).rejects.toThrow('not active') + await expect(registerSession(runtime, session as never)).rejects.toThrow('rejected the message') + await expect(registerSession(throwing, session as never)).rejects.toThrow('DataCloneError') + }) + + it('swallows unregister failures and skips a missing worker', async () => { + const { unregisterSession } = await loadClient(fakeWorker('activated')) + const { runtime, posted } = replyingWorker({ ok: false }) + const gone = { ...runtime, registration: { active: null, waiting: null, installing: null } as unknown as ServiceWorkerRegistration } + + await expect(unregisterSession(runtime, 's1')).resolves.toBeUndefined() + await expect(unregisterSession(gone, 's1')).resolves.toBeUndefined() + expect(posted).toEqual([{ type: 'EXELEARNING_UNREGISTER_SESSION', sessionId: 's1' }]) + }) + + it('throws when no Service Worker API exists', async () => { + vi.resetModules() + vi.stubGlobal('navigator', {}) + const { ensureRuntimeWorker } = await import('../../src/elpx/service-worker-client') + + await expect(ensureRuntimeWorker()).rejects.toThrow('not available') + }) +}) diff --git a/vitest.config.ts b/vitest.config.ts index 33a928b..0433cd0 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -11,11 +11,14 @@ export default defineConfig({ reporter: ['text', 'lcov'], reportsDirectory: 'coverage/js', include: [ + 'src/editor/editor-frame.ts', 'src/editor/editor-messages.ts', 'src/elpx/asset-map.ts', + 'src/elpx/elpx-loader.ts', 'src/elpx/iframe-renderer.ts', 'src/elpx/package-validator.ts', 'src/elpx/paths.ts', + 'src/elpx/service-worker-client.ts', 'src/elpx/viewer-session.ts', 'src/elpx/zip-reader.ts', 'src/files/mime.ts',