Check that a document id is one path segment before local media is deleted #426

Summary

On a server that stores editor media on local disk, deleteByPrefix joins the document id into a path and deletes it recursively. It never checks that the id is one plain path segment. A WebSocket first edit or PUT /api/documents/:docId can store a row under any id. Every purge later passes that stored id to deleteByPrefix. So a crafted id can delete another document's media folder, or a folder outside the storage root.

• Severity: Medium on local storage. Not applicable on S3, where keys are literal.
• Area: apps/hocuspocus.server local media storage
• Who is exposed: self-hosters. .env.example sets PERSIST_TO_LOCAL_STORAGE=true. The production value is not checked yet; it is on the review's production-check list.
• Source: security review of 2026-10-06, finding M13. Verified with the real Hono router, which decodes %2F in params.

Where

• deleteByPrefix runs rm(path.join(storageRoot(), documentId), { recursive: true, force: true }) with no check: apps/hocuspocus.server/src/lib/storage/storage.local.ts:134-142.
• The same file already checks containment in get (:50-56) and copyObject (:102-116). A plain containment check is not enough here. An id such as a/../<other id> resolves inside the root, to another document's folder.
• upload is safe in practice. Its route checks the id against ^[A-Za-z0-9_-]+$ (apps/hocuspocus.server/src/schemas/hypermultimedia.schema.ts:5-9) and needs a live row.
• Rows under any id:
  • WebSocket: onAuthenticate admits any room name (apps/hocuspocus.server/src/hocuspocus.server.ts:464-592). The first edit then writes the row through ensureDraftDocumentMetadata (:429-434), and the worker's create backstop can write it too.
  • REST: PUT /api/documents/:docId has no param schema (apps/hocuspocus.server/src/api/routers/documents.router.ts:57-62). updateDocument upserts the row (apps/hocuspocus.server/src/api/services/documents.service.ts:615-628).
• Purge path: deleteDocumentMedia (apps/hocuspocus.server/src/api/services/media.service.ts:20-33), called from apps/hocuspocus.server/src/api/services/documentPurge.service.ts:55.

Fix plan

1. The required fix, in deleteByPrefix (storage.local.ts):
   • Resolve path.resolve(root, documentId).
   • Refuse unless its parent is exactly root and its last segment equals documentId. That rejects ., .. and any id that contains /. On Linux, \ is a plain file-name character, so an id with \ stays inside its own folder.
   • On refusal, log a warning with storageLocalLogger and return without deleting.
2. Defense in depth: refuse bad ids before a row exists. Use the pattern upload already enforces, so every document with media still passes.
   • Export documentIdField from apps/hocuspocus.server/src/schemas/hypermultimedia.schema.ts.
   • In onAuthenticate, throw before the metadata lookup when documentName fails it.
   • On PUT /:docId, add zValidator('param', z.object({ docId: documentIdField }), houseEnvelopeHook).
3. Before step 2 ships, count legacy rows that fail the pattern on the hocuspocus database:
   select count(*) from "DocumentMetadata" where "documentId" !~ '^[A-Za-z0-9_-]{1,100}$';
   If the count is not zero, list those ids in the PR and ask the maintainer before you merge step 2.

Layer: the containment check belongs in the storage adapter (storage.local.ts), like the checks in get and copyObject. Do not add path logic to media.service.ts or the purge service. Use documentIdField, not the stricter 19-character documentIdSchema from modules/document-content/http/schema.ts. Older documents keep their random ids forever (apps/hocuspocus.server/CLAUDE.md §Hocuspocus Server).

Out of scope

• S3 deleteByPrefix. Keys are literal there, and s3Prefix ends in /.
• Cleaning up rows that already hold a bad id. Step 3 only counts them.

Acceptance criteria

• deleteByPrefix with ../x, a/../b, ., or an empty string deletes nothing and logs a warning.
• A normal purge still removes the document's media folder.
• A WebSocket room name or PUT id that contains /, \ or .. is refused before any row is written.
• Existing documents still open and still accept uploads.

Verify

• Add cases to apps/hocuspocus.server/tests/unit/storage.local.test.ts. Set LOCAL_STORAGE_PATH to a temp folder, and create a sibling folder and a victim document folder. Call deleteByPrefix with each bad id, and check both folders still exist. Then check a valid id removes its own folder.
• cd apps/hocuspocus.server && bun test tests/unit/storage.local.test.ts.

Related

• Keep a purged document id gone after a PUT or a reconnect #407 (purged id comes back, fixed)
• Stop serving editor media after a document moves to Trash #408 (media after Private or Trash)

Generated by Claude Code