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.Second commit states the contract the fix depends on, so the next reader doesn't re-derive module ownership from the prefix:
workspace_files.contextis server-authored like the key, but unlike the key it is mutable —materialize_filepromotes an attachment to a workspace file by flipping that column. Encoding a mutable fact in an immutable key would mean copying the bytes on every transition just to restate them, which is why the prefix can't own this andexecuteSavestays a metadata-only flip.resolveTrustedFileContextclaimed the prefix was flatly "authoritative" — the claim that made routing on it look safe. Now scoped to what it actually defends: a caller-supplied context still can never relabel a private key.resolveStoredFileContextis documented as the sanctioned reader, not a workaround.verifyWorkspaceFileAccessfiltered its binding lookup tocontext = 'workspace', so attachments missed the row and fell through to object metadata, which can't see a soft delete. It now matches either workspace-scoped context — which is what callers already wanted, since the LLM-attachment and presigned-URL paths pass'workspace'for attachment keys today. A soft-deleted attachment is now denied on all of them.context = 'workspace'because they mean the Files module, not the bucket.Type of Change
Testing
Both regressions are pinned by tests that fail against the pre-fix code, and in both cases the reason they shipped green was a mock asserting something the real function never does:
inferContextFromKeyto return'mothership'for aworkspace/…key. Made honest — 9 of them fail against the old route.getFileMetadataByKeymock that honors thecontext/includeDeletedarguments, so they actually exercise the filter. 2 fail against the old authorizer; the workspace-context cases still pass, showing no Files-module regression.bun run test app/api/files lib/uploads lib/workspace-files— 861 pass (77 files).type-check,lint, and all 29 audits clean. Not exercised against a running app.Checklist