Security audit: fix 32 confirmed vulnerabilities (10 critical / 16 high / 5 medium / 1 low)

[erseco]

Security audit: fix 32 confirmed vulnerabilities (10 critical / 16 high / 5 medium / 1 low)

Fixes every issue from a multi-agent security audit that survived independent adversarial verification. Organized into themed commits grouped by root cause so the change is reviewable and bisectable. Every fix ships with colocated tests in the same change.

Summary

• 68 files changed, 12 commits.
• make fix, make test-unit (incl. the per-file 90% line-coverage gate), make test-integration, and make test-e2e all green locally.
• New .ts files have colocated *.spec.ts; new/changed public/app JS has colocated *.test.js; user-visible flows covered by existing/updated E2E specs.

Fixes by theme

1. Path traversal / arbitrary file write — shared safe-path foundation + sinks

New src/utils/safe-path.ts (single source of truth: isSafePathSegment / safeJoin / sanitizeFileExtension) plus a fix to file-helper.isPathSafe, which used a naive startsWith(base) that treated /data/assets-evil as inside /data/assets. Applied to every sink building an on-disk path from untrusted input:

• C1 (critical) — clientId used directly as the on-disk filename across asset upload/finalize/sync/stream (assets.ts), batch upload (upload-session.ts), and API v1 (api/v1/assets.ts) → arbitrary file write/overwrite (webshell / DB / cross-tenant). The repo already had isPathSafe/sanitizeFolderPath but applied them to the wrong field.
• C2 (critical) — odeSessionId path param in export download (GET/POST) → arbitrary file write + directory read (export.ts).
• C3 (critical) — themeName/themeId in theme bundle/download/metadata endpoints → unauthenticated arbitrary file/dir read, incl. .env/DB (themes.ts).
• H10 (high) — resumableIdentifier chunk-directory traversal (assets.ts).
• H14 (high) — ideviceId metadata endpoint traversal (idevices.ts).

No auth added to the public theme/idevice resource endpoints (they are legitimately fetched unauthenticated by preview/embedding/static builds); the traversal validation is the fix.

2. Broken authorization / IDOR

• C4 (critical) — POST /projects/uuid/:uuid/duplicate had no auth/access check; any caller could clone an arbitrary private project into their account. Now requires auth + checkProjectAccess on the source.
• H4 (high) — GET /api/games/:odeSessionId/idevices returned full iDevice content of any project by UUID, unauthenticated. Now withJwtAuth + enforceProjectAccess. Verified the editor/preview caller is same-origin and sends the auth cookie; exported static packages never call the live endpoint.
• M1 (medium) — project sharing endpoints leaked owner/collaborator emails for any project (numeric variant unauthenticated + enumerable). Now require auth + access.
• M2 (medium) — set_platform_new_ode exported any project by UUID with no ownership binding. Now requires the project's platform_id to match the validated JWT cmid.

3. SSRF

New src/utils/ssrf-guard.ts: safeFetch allows only http(s), resolves the host and rejects private/loopback/link-local/CGNAT/unspecified addresses, and follows redirects manually re-validating each hop (DNS + fetch injectable for hermetic tests).

• H2 (high) — link-validator.validateLink fetched arbitrary URLs with redirect: 'follow' and no egress filter. Now routes through safeFetch; a blocked target is reported as broken without leaking the resolved IP.
• H1 (high) — /brokenlinks reimplemented the same unguarded fetch inline and was unauthenticated. Deleted the duplicate, calls the shared SSRF-safe service, and requires auth on all three brokenlinks endpoints.
• M3 (medium) — isAllowedProviderUrl failed open when PROVIDER_URLS was empty (the shipped default). Now fails closed; returnurls with internal-IP/non-http hosts are rejected.

4. Yjs persistence

• C7 (critical) — the document mutex woke all waiters on release, so several callers believed they held the lock (reproduced: 4–5 of 5 concurrent). Replaced with a real FIFO mutex; release is idempotent.
• C6 (critical) — REST API v1 edits were saved to yjs_updates with hardcoded version '1', always older than the Date.now() snapshot version, so they were filtered out on load (silent, unrecoverable loss). Now uses a monotonic timestamp version.
• H6 (high) — saveFullState did a non-transactional delete-then-insert; a failed insert wiped state. Now wrapped in a transaction.
• H5 (high) — the client load endpoint read only the snapshot table; a project whose state lived in yjs_updates loaded empty/404. Now reconstructs snapshot + updates.
• M6 (medium) — CAST(version AS INTEGER) is rejected by MySQL (needs SIGNED) and overflows 32-bit on PostgreSQL for the millisecond-timestamp versions already in use. Now dialect-aware (INTEGER/BIGINT/SIGNED).

5. Stored / DOM XSS

• C5 (critical) — a remote collaborator's iDevice htmlContent was assigned to innerHTML with no sanitization (zero-click JS in every collaborator's session). New public/app/utils/sanitizeHtml.js (DOMPurify-backed, with a fail-safe fallback) sanitizes remote content before innerHTML; DOMPurify was wired into the workarea (it had only been loaded for the equation editor). loadInitScriptIdevice re-attaches legitimate iDevice behavior, so interactivity is preserved.
• H12 (high) — the File Manager list view interpolated the asset filename raw into innerHTML in the reference-count branch. Now escaped with the existing escapeHtml helper.

6. Resource exhaustion / DoS

• C9 (critical) — ELP import called unzipSync with no decompressed-size cap (zip bomb → OOM). Now enforces cumulative / per-entry / entry-count caps via fflate's central-directory filter, aborting before inflation, on the main, nested-ELP, and zip-contents paths.
• H8 (high) — client-controlled awareness availableAssets grew server memory unbounded. Now capped per message, ids validated, distinct entries and pending requests bounded per project, orphaned keys pruned on disconnect; explicit 8MB maxPayloadLength on the Yjs WS route.
• H9 (high) — abandoned chunked uploads never expired (disk + memory leak). Added per-chunk / total-chunk caps and a TTL sweeper (wired into server startup/shutdown).
• M5 (medium) — upload-session-manager.cleanupExpired() was defined but never scheduled. Now scheduled (with serverPriorityQueue.cleanupStaleSlots), wired into startup/shutdown.
• L1 (low) — batch upload buffered all files before checking the size cap. Now enforces the cap by declared size before buffering, with incremental-abort fallback.

7. Client-side data loss

• H7 (high) — AssetManager._putToCache only fell back to IndexedDB on scheme errors, so a QuotaExceededError was swallowed and the only in-memory blob copy was then evicted — losing it entirely. Now any cache write failure falls back to IndexedDB and only resolves after a confirmed persistent write; otherwise it re-throws so the caller keeps the blob in memory.

8. Auth secret / EPUB / DB / parser

• H3 (high) — admin/user/pages/project routes and admin-route-helpers each used a divergent local getJwtSecret (JWT_SECRET || APP_SECRET || <public default>) to verify tokens, while auth.ts signs with API_JWT_SECRET || JWT_SECRET || <default>. With the shipped .env (JWT_SECRET unset) this mismatched and broke admin auth; and if APP_SECRET were unset it fell back to a public in-repo constant, enabling token forgery. All now use the single canonical resolver; APP_SECRET stays only for platform JWTs.
• H11 (high) — EPUB export used blind global regexes that broke external .html links, mangled prose, and corrupted void elements with > in an attribute. Replaced with a real xmldom parse + XHTML serialize, rewriting only internal page links.
• M4 (medium) — transferOwnership ran 3 writes non-transactionally. Now atomic.
• H13 (high) — legacy XML import resolved each <reference> via a full-document scan (O(n²); ~21.8s on a crafted file). Now a Map built once per parse (sub-second).

Re-verified, not changed

• C8 ("Ctrl+S skips assets") — re-verified against the live code: YjsProjectManagerMixin.save() overrides project.save() and routes through SaveManager, which uploads pending assets before markClean. Not a bug; no change.
• 4 audit findings were rejected by the verifiers as false positives and are not included.

Testing

• make test-unit: 7280 pass, 0 fail; coverage gate ✓ (all 219 files ≥90% line coverage). Two files I touched near the gate were brought back over 90% with real behavior tests rather than excluded.
• make test-integration: green (admin integration suite's JWT env aligned with the unified resolver).
• make test-e2e (Playwright/chromium): green.

Notes for reviewers

• The dual-table Yjs model (yjs_documents snapshot + yjs_updates) is preserved; this PR makes its two write paths and version schemes consistent and the load path reconstruct from both. Collapsing to a single table is a larger refactor left for follow-up.
• set_platform_new_ode_browser has a residual (non-exfiltrating) platform-link gap noted during M2; worth a follow-up.