feat(files): files as a channel-scoped root entity - #302
Merged
Conversation
Introduce files as a first-class, channel-scoped root entity that messages
reference via fileIds, instead of metadata embedded in the message body.
- new file/ core domain: register (draft on upload), attach (link on message
send), remove (owner-only, ref-counted storage cleanup), getChannel query
- FileEntry type + FileRepo (batch-by-ids, attached-channel listing)
- channel-scoped upload/list/delete routes under /api/channels/:id/files
- message:create accepts fileIds; reads resolve fileIds -> files (batched);
message:remove cascades file removal
- storage: compute image resolution at upload, expose stat(), sweep
{id}-* thumbnail variants on remove (fs/gcs/memory)
- additive backfill migration for plaintext channels (DMs forward-only);
legacy embedded attachments are never stripped
- upload via POST /api/channels/:id/files; store file entity id + storageId - messages reference files via fileIds (kept plaintext through encryption so encrypted DMs attach files server-side); files no longer embedded on send - render legacy embedded attachments and new resolved files through one path (permanent dual-read); optimistic ghost shows files immediately - FilesAPI.list/remove for the per-channel files view
- deno fmt normalization across migration files and BACKLOG.md - remove stale .planning/pr-full/FINAL-REPORT.md
raaymax
commented
Jun 18, 2026
| toJSON(): Array<{ id?: string; clientId: string; fileName: string; fileSize: number; contentType: string }> { | ||
| return this.list.map((f) => ({ | ||
| id: f.id, | ||
| id: f.storageId, |
Owner
Author
There was a problem hiding this comment.
Can we do something about this? can we refactor the old code to make this obsolete?
Owner
Author
There was a problem hiding this comment.
Resolved in b9c6a7c by removing the root cause: /avatar and /emoji were the only consumers of that storage-id-as-attachment shape, so they're gone and FilesModel.toJSON() with them. Commands are now text-only.
Avatars/emojis are profile/workspace assets, not channel attachments — they'll be reintroduced as first-class uploads (dedicated endpoints + UI, no channel File entity) in a follow-up. Tracked in v4 scope §6. Existing avatars/emojis keep rendering; only setting new ones pauses until the follow-up.
These slash-commands attached a channel-scoped File entity just to carry a storage id (the reason the frontend leaked storageId via FilesModel.toJSON). Avatars and emojis are profile/workspace assets, not channel message attachments, so they will be reintroduced as first-class uploads with dedicated endpoints + UI (tracked in v4 scope). - delete EmojiCommand/AvatarCommand + registry entries - commands are now text-only (drop attachments/fileIds from the command body, route schema, Command type, and execute resolution) - remove FilesModel.toJSON; the command path no longer sends files - update command/emoji/profile/users tests; seed the emoji-listing test via repo instead of the removed /emoji command
New messages carry files only via fileIds (File entities). The legacy attachments array is no longer accepted or persisted on create/update. The read path is unchanged: old plaintext rows keep their stored attachments, old encrypted messages still carry attachments inside the decrypted body, and the client renders both legacy attachments and new files. Editing an old message preserves its stored attachments (updates only $set the provided fields).
WithImages now uses resolved File entities (message.files) instead of the legacy embedded attachments; add WithLegacyAttachments to document the permanent dual-read path for old/encrypted messages.
storageId is a @quack/storage implementation detail; the frontend used it to build file URLs, leaking it. New files are now addressed by their File entity id: - new GET /api/channels/:channelId/files/:fileId resolves the entity -> storageId and streams (with thumbnail + download support) - toClientFile() strips storageId from every client-facing payload (message.files, channel files list, upload response); MessageFile drops the field and the frontend File model no longer carries it - frontend builds URLs via getFileUrl(channelId, fileId) Legacy /api/files/:storageId stays (deprecated) to serve old plaintext attachments, decrypted DM attachments, and avatars/emojis until a follow-up migrates encrypted attachments and removes it.
Match the message convention (userId = the user who created the row). The files collection is new/unreleased so no data migration is needed; the backfill migration is updated in place.
message:update now accepts fileIds: it persists them, runs file:attach for newly-referenced files, and re-resolves files for the response and SSE broadcast. This is the building block the future client-side migration of encrypted DM attachments depends on (register File rows for embedded attachments, then point the existing message at them). Add-only for now: detaching files removed on edit (delete + storage sweep) and the migration's cross-author attach (message:update is owner-only) are deferred.
…dening - assert storageId is absent from message.files and the channel files list - assert /avatar and /emoji now 404 (removed) and aren't listed by /help - download endpoint: thumbnail (?w&h), download=true disposition, and 403 for a non-member of a private channel
…url helpers
Regression fix: Files.tsx was rebuilding image/download URLs from file.id
via the legacy /api/files/:storageId hooks and ignoring the url from
toAttachments. After files moved to entity-id addressing this produced
/api/files/{entityId} (404) for new attachments and always showed 0 B.
- toAttachments now carries size and the correct url (new channel-scoped
endpoint for resolved files, legacy storageId endpoint for embedded
attachments)
- Files.tsx consumes that url (thumbnail via ?h, download via
?download=true) and the real size instead of rebuilding from file.id
- remove the now-unused getThumbnailUrl/getDownloadUrl hooks and the
client.api.getThumbnail/getDownloadUrl methods they wrapped
- mock getFileUrl in the Message story so the image renders
Fetch request-body streaming (duplex: half) requires HTTP/2; the Vite dev server is HTTP/1.1, so the streaming upload failed ALPN negotiation and froze at 0%. Select the upload strategy via a build-time flag: - prod (HTTP/2 behind the proxy): ReadableStream streaming, no buffering — important for large files - dev (BUFFERED_UPLOAD=true): buffer to a Blob and POST via XHR (works over HTTP/1.1, real upload progress), also the fallback for browsers without request-stream support Also fixes two latent bugs in the buffered path: it sent multipart FormData (the route expects a raw body + Content-Disposition) and streamToBlob pushed chunk.value (undefined) instead of chunk.
- pnpm with minimumReleaseAge (1wk cooldown), blockExoticSubdeps, default build-script blocking (esbuild allowlisted) - route install paths through Socket Firewall (sfw) in CI + Docker - restore dev HTTPS via @vitejs/plugin-basic-ssl, fixing the crypto.subtle login crash from the prior cert-free dev server - least-privilege permissions on CI test jobs
- storage: after merging dev's off-event-loop resize pool, route image resolution reads through the worker too (merge had left readResolution calling the now-removed PhotonImage import) and copy bytes into the Blob before transferring the buffer to the worker - storage: clamp resize dimensions to 2048 to prevent unbounded-allocation DoS - download: send X-Content-Type-Options: nosniff and default to attachment (inline only for safe raster types) to prevent stored XSS via svg/html; sanitize the Content-Disposition filename - upload: parse content-disposition safely (no 500 on a missing header) and sanitize the stored filename - attach: only attach draft files, preventing an already-attached file from being re-pointed to another message - migration down: drop only the indexes it created instead of the whole files collection (which would destroy live data on rollback)
This was referenced Jul 24, 2026
Merged
Merged
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.
Summary
Turns files from message attachments (metadata embedded inside the message body) into a first-class, channel-scoped root entity that messages reference via
fileIds. This finishes the long-standing// TODO make this a separate entityand lays the groundwork for a per-channel Files view. Fully backward compatible — old messages keep rendering via legacy embedded attachments and DM history is untouched.Design doc:
docs/files-design.md.Changes
Backend — new
file/domainregister(draft on upload),attach(link on message send),remove(owner-only, ref-counted storage cleanup),getChannelqueryFileEntrytype +FileRepo(batch-by-ids, attached-channel listing)/api/channels/:id/files(upload / list / delete)message:createacceptsfileIds; reads resolvefileIds → files(batched, no N+1);message:removecascades file removalStorage
resolutionat upload, exposesstat()remove()sweeps{id}-*thumbnail variants by prefix (fs / gcs / memory) — fixes a pre-existing thumbnail leakMigration
Filerows from embedded attachments in plaintext channels (DMs are forward-only; the server can't read encrypted attachment metadata)attachmentsare never strippedFrontend — data path
fileIds, kept plaintext through encryption so encrypted DMs still attach files server-sideE2E posture
Public/private channel file metadata is server-readable; for DMs the channel-scoped upload means filename/content-type now reach the server in plaintext (accepted for now — encryption is a documented later step). DM history remains fully readable via the permanent legacy path.
Testing
files.test.ts: register → attach via message → channel list (REST) → message-remove cascade + storage sweep; owner-only direct delete@quack/apitype-check clean; frontend production build passesNot included (follow-up)