fix(files): serve mothership chat attachments stored under a workspace key - #6789
Conversation
…e key
A mothership chat attachment is minted with the same storage key shape as a
workspace file — `resolveUploadStorage` calls `generateWorkspaceFileKey` for
`mothership_attachment` — but its row is written with `context = 'mothership'`.
The serve route branched on the key prefix alone, so every attachment entered
the workspace-file use case, which matches on `context = 'workspace'` and
resolved it to nothing: `{"error":"FileNotFoundError","message":"File not
found"}` for every chat thumbnail and click-through. The `context=mothership`
query param on the serve URL is decorative; the route never read it.
Serve now resolves the storage context from the key's stored binding, which is
server-authored at upload and the only thing that separates the two, and passes
it into the cloud and local handlers instead of re-inferring. Genuine workspace
files still go through the authorized use case.
`verifyWorkspaceFileAccess` takes the context too, so an attachment authorizes
from its database row rather than falling through to storage-object metadata.
A soft-deleted attachment is now denied, matching workspace files.
The route tests are what let this ship: they mocked `inferContextFromKey` to
return 'mothership' for a `workspace/…` key, which it never does. With the mock
made honest, nine of them fail against the old route.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview The serve route now uses Authorization threads Tests cover Reviewed by Cursor Bugbot for commit 26ac161. Configure here. |
Greptile SummaryThe PR fixes serving mothership chat attachments whose workspace-shaped storage keys previously routed them through the workspace-files use case.
Confidence Score: 5/5The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking issues identified. The active metadata binding is unique per key, and the resolved context is consistently applied across route selection, authorization, deletion checks, and storage reads.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/uploads/server/metadata.ts | Adds deterministic context resolution from the unique active metadata binding, with prefix inference retained for unbound and non-workspace keys. |
| apps/sim/app/api/files/serve/[...path]/route.ts | Uses the resolved storage context consistently to select authentication and drive local or cloud authorization and reads. |
| apps/sim/app/api/files/authorization.ts | Extends workspace-scoped metadata authorization to mothership bindings while preserving deleted-record denial and workspace permission checks. |
| apps/sim/app/api/files/serve/[...path]/route.test.ts | Adds regression coverage proving workspace-prefixed mothership attachments bypass the workspace-file use case and are served under the mothership context. |
| apps/sim/lib/uploads/server/metadata.test.ts | Covers stored mothership and workspace bindings, unbound workspace keys, and non-workspace inference without a database lookup. |
Sequence Diagram
sequenceDiagram
participant Client
participant Serve as File Serve Route
participant Metadata
participant Auth
participant Storage
Client->>Serve: GET /api/files/serve/workspace/...
Serve->>Metadata: resolveStoredFileContext(key)
Metadata-->>Serve: mothership or workspace
alt Workspace file
Serve->>Auth: internalWorkspaceFileServeAuth
else Mothership attachment
Serve->>Auth: session/internal authentication
end
Serve->>Auth: verifyFileAccess(key, resolvedContext)
Auth-->>Serve: authorized
Serve->>Storage: read key using resolvedContext
Storage-->>Client: attachment bytes
Reviews (1): Last reviewed commit: "fix(files): serve mothership chat attach..." | Re-trigger Greptile
Summary
{"error":"FileNotFoundError","message":"File not found"}. A mothership attachment is minted with the same key shape as a workspace file (resolveUploadStoragecallsgenerateWorkspaceFileKeyformothership_attachment) but its row carriescontext = 'mothership', so the serve route's key-prefix branch sent it into the workspace-file use case, which matches oncontext = 'workspace'and resolved it to nothing. Regressed in fix(files): preserve principals when serving generated documents #6654.resolveStoredFileContext) rather than the prefix alone, and threads it into the cloud/local handlers instead of re-inferring. Genuine workspace files still go through the authorized use case.verifyWorkspaceFileAccesstakes the context, so an attachment authorizes from its DB row instead of falling back to storage-object metadata — a soft-deleted attachment is now denied, matching workspace files.Type of Change
Testing
The serve route tests mocked
inferContextFromKeyto return'mothership'for aworkspace/…key, which it never does — that fiction is what kept this green. Made the mock honest and added a regression test; nine of the existing tests fail against the pre-fix route, confirming the reproduction.bun run test app/api/files lib/uploads— 725 pass (53 files)type-check,lint,check:audits(29 audits),check:api-validation, block-registry audit — all cleanChecklist