From faf31f942d55623aec9946e08eb51f373692ebab Mon Sep 17 00:00:00 2001 From: Jen Garcia Date: Mon, 22 Jun 2026 19:28:44 -0400 Subject: [PATCH 1/7] fix: POST /attachments stores bytes only, preventing duplicate _attachment@1 records When the adapter-api calls POST /attachments, the server was calling ScopedStack.putAttachment() which stores the bytes AND creates an _attachment@1 metadata record. The client-side ScopedStack.putAttachment() then also created a second _attachment@1 record via POST /records, because it treats adapter.putAttachment() as bytes-only per the StackAdapter interface contract. This produced two _attachment@1 records per upload: - Record #1: created by the server route, with mimeType hardcoded to application/octet-stream (APIAdapter always sends that) and no filename - Record #2: created by the client ScopedStack, with correct mimeType and filename Fix: the POST /attachments route now calls adapter.putAttachment(data) directly (bytes only), matching the StackAdapter interface intent. The metadata record is the caller's responsibility, which ScopedStack handles automatically via its subsequent POST /records call. Downstream changes: - GET /attachments/:fileId: falls back to application/octet-stream when no _attachment@1 record exists (instead of 404), since the file may exist without a metadata record immediately after upload - DELETE /attachments/:fileId: falls back to checking bytes existence when no _attachment@1 record is found, so orphaned bytes are deletable --- src/routes/attachments.ts | 117 +++++++------------------------ tests/routes/attachments.test.ts | 54 +++++++++++++- 2 files changed, 77 insertions(+), 94 deletions(-) diff --git a/src/routes/attachments.ts b/src/routes/attachments.ts index 4b009ab..2f8962e 100644 --- a/src/routes/attachments.ts +++ b/src/routes/attachments.ts @@ -10,7 +10,10 @@ export function attachmentRoutes(ctx: StackContext, maxAttachmentBytes: number): const { adapter, stack } = ctx; const { ownerEntityId } = stack; - // POST /attachments — upload raw binary, Content-Type = MIME type + // POST /attachments — upload raw binary; stores bytes only. + // Callers should follow up with POST /records to create the _attachment@1 + // metadata record (mimeType, size, filename). The SDK's ScopedStack.putAttachment() + // does this automatically. app.post('/', requireAuth(), async (c) => { const contentLength = Number(c.req.header('Content-Length') ?? 0); if (contentLength > maxAttachmentBytes) { @@ -22,13 +25,7 @@ export function attachmentRoutes(ctx: StackContext, maxAttachmentBytes: number): return c.json({ error: 'Attachment too large' }, 413); } - const filename = parseFilename(c.req.header('Content-Disposition')); - const mimeType = sanitizeMimeType( - resolveMimeType(c.req.header('Content-Type') ?? 'application/octet-stream', filename), - ); - - const auth = c.get('auth'); - const fileId = await stack.asEntity(auth!.entityId).putAttachment(data, mimeType, filename); + const fileId = await adapter.putAttachment(data); return c.json({ fileId }, 201); }); @@ -40,16 +37,6 @@ export function attachmentRoutes(ctx: StackContext, maxAttachmentBytes: number): const accessible = await isAttachmentAccessible(fileId, auth?.entityId ?? null, ctx); if (!accessible) return c.json({ error: 'Unauthorized' }, 401); - // Owner-level query for all _attachment@1 records for this file. MimeType and - // size are safe to expose at owner scope since they're inferrable from the file - // bytes, which the requester already has permission to access. - const metaResult = await stack.query({ - filter: { typeId: `${SYSTEM_TYPES.ATTACHMENT}@1`, content: { fileId } }, - }); - const attachmentContent = (metaResult.records[0]?.content as AttachmentContent) ?? null; - - if (!attachmentContent) return c.json({ error: 'Attachment not found' }, 404); - let data: Uint8Array; try { data = await adapter.getAttachment(fileId); @@ -57,7 +44,14 @@ export function attachmentRoutes(ctx: StackContext, maxAttachmentBytes: number): return c.json({ error: 'Attachment not found' }, 404); } - // Filename is pure metadata — only expose it from the requester's own upload record. + // _attachment@1 metadata record is optional — falls back to safe defaults + // when absent (e.g. right after upload before the caller has created the record). + const metaResult = await stack.query({ + filter: { typeId: `${SYSTEM_TYPES.ATTACHMENT}@1`, content: { fileId } }, + }); + const attachmentContent = (metaResult.records[0]?.content as AttachmentContent) ?? null; + + // Filename is pure metadata — only expose it from the requester's own record. const ownRecord = metaResult.records.find((r) => r.entityId === auth?.entityId); const visibleFilename = (ownRecord?.content as AttachmentContent | undefined)?.filename; @@ -66,8 +60,8 @@ export function attachmentRoutes(ctx: StackContext, maxAttachmentBytes: number): : 'attachment'; const headers: Record = { - 'Content-Type': attachmentContent.mimeType, - 'Content-Length': String(attachmentContent.size), + 'Content-Type': attachmentContent?.mimeType ?? 'application/octet-stream', + 'Content-Length': String(attachmentContent?.size ?? data.byteLength), 'Content-Disposition': disposition, 'X-Content-Type-Options': 'nosniff', }; @@ -79,11 +73,19 @@ export function attachmentRoutes(ctx: StackContext, maxAttachmentBytes: number): app.delete('/:fileId', requireOwner(ownerEntityId), async (c) => { const fileId = c.req.param('fileId'); - // Find the _attachment@1 metadata record(s) for this file + // Find any _attachment@1 metadata record(s) for this file const metaResult = await stack.query({ filter: { typeId: `${SYSTEM_TYPES.ATTACHMENT}@1`, content: { fileId } }, }); - if (!metaResult.records.length) return c.json({ error: 'Attachment not found' }, 404); + + // If no metadata record, verify the bytes actually exist before proceeding + if (!metaResult.records.length) { + try { + await adapter.getAttachment(fileId); + } catch { + return c.json({ error: 'Attachment not found' }, 404); + } + } // Refuse if any record in the stack still references this file const refResult = await stack.query({ filter: { attachmentFileId: fileId }, limit: 1 }); @@ -102,75 +104,6 @@ export function attachmentRoutes(ctx: StackContext, maxAttachmentBytes: number): return app; } -function parseFilename(disposition: string | undefined): string | undefined { - if (!disposition) return undefined; - // RFC 5987 form takes priority: filename*=UTF-8'' - const rfc5987 = disposition.match(/filename\*=UTF-8''([^\s;]+)/i); - if (rfc5987) return decodeURIComponent(rfc5987[1]); - // Plain quoted form: filename="..." (used by curl and other HTTP clients) - const quoted = disposition.match(/filename="([^"]+)"/); - return quoted?.[1]; -} - -// MIME types that browsers can use to execute scripts or parse as markup. -// Uploaders supplying one of these get application/octet-stream instead. -const BLOCKED_MIME_TYPES = new Set([ - 'text/html', - 'text/javascript', - 'application/javascript', - 'application/x-javascript', - 'application/xhtml+xml', - 'image/svg+xml', - 'text/xml', - 'application/xml', -]); - -function sanitizeMimeType(mimeType: string): string { - const base = mimeType.split(';')[0].trim().toLowerCase(); - return BLOCKED_MIME_TYPES.has(base) ? 'application/octet-stream' : mimeType; -} - -// Map of common file extensions to MIME types. Used to upgrade -// application/octet-stream when the client omits a specific Content-Type -// but provided a filename with a recognisable extension. -// Omits types in BLOCKED_MIME_TYPES — those are sanitized to -// application/octet-stream anyway, so inferring them from extension -// serves no purpose. -const EXTENSION_MIME: Record = { - // Images - jpg: 'image/jpeg', - jpeg: 'image/jpeg', - png: 'image/png', - gif: 'image/gif', - webp: 'image/webp', - ico: 'image/x-icon', - // Documents - pdf: 'application/pdf', - // Text - txt: 'text/plain', - md: 'text/markdown', - csv: 'text/csv', - json: 'application/json', - // Video - mp4: 'video/mp4', - webm: 'video/webm', - mov: 'video/quicktime', - // Audio - mp3: 'audio/mpeg', - wav: 'audio/wav', - ogg: 'audio/ogg', - m4a: 'audio/mp4', - // Archives - zip: 'application/zip', - gz: 'application/gzip', -}; - -function resolveMimeType(declared: string, filename: string | undefined): string { - if (declared !== 'application/octet-stream' || !filename) return declared; - const ext = filename.split('.').pop()?.toLowerCase(); - return (ext && EXTENSION_MIME[ext]) || declared; -} - /** * An attachment is accessible if: * 1. The requester is the stack owner, OR diff --git a/tests/routes/attachments.test.ts b/tests/routes/attachments.test.ts index 580abd1..2bd2170 100644 --- a/tests/routes/attachments.test.ts +++ b/tests/routes/attachments.test.ts @@ -148,24 +148,74 @@ describe('POST /attachments', () => { await t.cleanup(); }); - it('preserves filename from Content-Disposition on upload and returns it on download', async () => { + it('stores bytes only — does not create an _attachment@1 record', async () => { + const content = new TextEncoder().encode('file content'); + const uploadRes = await t.app.request('/attachments', { + method: 'POST', + headers: { + Authorization: `Bearer ${TEST_TOKEN}`, + 'Content-Type': 'text/plain', + 'Content-Disposition': 'attachment; filename="ignored.txt"', + }, + body: content, + }); + expect(uploadRes.status).toBe(201); + const { fileId } = (await uploadRes.json()) as { fileId: string }; + + const metaResult = await t.ctx.stack.query({ + filter: { typeId: '_attachment@1', content: { fileId } }, + }); + expect(metaResult.records).toHaveLength(0); + }); + + it('serves file with application/octet-stream fallback when no _attachment@1 record exists', async () => { + const content = new TextEncoder().encode('raw bytes'); + const uploadRes = await t.app.request('/attachments', { + method: 'POST', + headers: { + Authorization: `Bearer ${TEST_TOKEN}`, + 'Content-Type': 'image/png', + }, + body: content, + }); + expect(uploadRes.status).toBe(201); + const { fileId } = (await uploadRes.json()) as { fileId: string }; + + const downloadRes = await t.app.request(`/attachments/${fileId}`, { + headers: { Authorization: `Bearer ${TEST_TOKEN}` }, + }); + expect(downloadRes.status).toBe(200); + expect(downloadRes.headers.get('Content-Type')).toBe('application/octet-stream'); + expect(downloadRes.headers.get('Content-Disposition')).toBe('attachment'); + }); + + it('returns mimeType and filename from _attachment@1 record created after upload', async () => { const content = new TextEncoder().encode('PDF content'); const uploadRes = await t.app.request('/attachments', { method: 'POST', headers: { Authorization: `Bearer ${TEST_TOKEN}`, 'Content-Type': 'application/octet-stream', - 'Content-Disposition': 'attachment; filename="test.pdf"', + 'Content-Disposition': 'attachment; filename="ignored.pdf"', }, body: content, }); expect(uploadRes.status).toBe(201); const { fileId } = (await uploadRes.json()) as { fileId: string }; + // Create the metadata record (what ScopedStack.putAttachment does via POST /records) + await t.ctx.stack.create('_attachment@1', { + fileId, + mimeType: 'application/pdf', + size: content.byteLength, + filename: 'test.pdf', + }); + const downloadRes = await t.app.request(`/attachments/${fileId}`, { headers: { Authorization: `Bearer ${TEST_TOKEN}` }, }); expect(downloadRes.status).toBe(200); + expect(downloadRes.headers.get('Content-Type')).toBe('application/pdf'); expect(downloadRes.headers.get('Content-Disposition')).toBe( "attachment; filename*=UTF-8''test.pdf", ); From 3f30d57709e4010e7ba55d39da22882ccc952404 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 23 Jun 2026 11:06:31 +0000 Subject: [PATCH 2/7] docs: update attachment upload description to reflect bytes-only POST /attachments --- docs/api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api.md b/docs/api.md index c50bb63..069ce0d 100644 --- a/docs/api.md +++ b/docs/api.md @@ -40,7 +40,7 @@ All routes are prefixed by the base URL. Requests are authenticated with a `Bear | Method | Path | Auth | Description | | ------ | ---------------------- | ---------- | --------------- | -| POST | `/attachments` | Required | Upload a file | +| POST | `/attachments` | Required | Store raw file bytes (bytes only — no metadata record created) | | GET | `/attachments/:fileId` | Optional | Download a file | | DELETE | `/attachments/:fileId` | Owner only | Delete a file | From b4de50863eed7acb513901f9bac75365f33bc293 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 23 Jun 2026 11:25:39 +0000 Subject: [PATCH 3/7] fix: set entityId on _attachment@1 record in test; fix Prettier formatting in api.md --- docs/api.md | 8 ++++---- tests/routes/attachments.test.ts | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/api.md b/docs/api.md index 069ce0d..a593a74 100644 --- a/docs/api.md +++ b/docs/api.md @@ -38,11 +38,11 @@ All routes are prefixed by the base URL. Requests are authenticated with a `Bear ## Attachments -| Method | Path | Auth | Description | -| ------ | ---------------------- | ---------- | --------------- | +| Method | Path | Auth | Description | +| ------ | ---------------------- | ---------- | -------------------------------------------------------------- | | POST | `/attachments` | Required | Store raw file bytes (bytes only — no metadata record created) | -| GET | `/attachments/:fileId` | Optional | Download a file | -| DELETE | `/attachments/:fileId` | Owner only | Delete a file | +| GET | `/attachments/:fileId` | Optional | Download a file | +| DELETE | `/attachments/:fileId` | Owner only | Delete a file | ## Entity & Tokens diff --git a/tests/routes/attachments.test.ts b/tests/routes/attachments.test.ts index 2bd2170..d01d294 100644 --- a/tests/routes/attachments.test.ts +++ b/tests/routes/attachments.test.ts @@ -5,6 +5,7 @@ import { testConfig, logger, TEST_TOKEN, + TEST_ENTITY_ID, OTHER_ENTITY_ID, type TestApp, } from '../setup.js'; @@ -204,12 +205,11 @@ describe('POST /attachments', () => { const { fileId } = (await uploadRes.json()) as { fileId: string }; // Create the metadata record (what ScopedStack.putAttachment does via POST /records) - await t.ctx.stack.create('_attachment@1', { - fileId, - mimeType: 'application/pdf', - size: content.byteLength, - filename: 'test.pdf', - }); + await t.ctx.stack.create( + '_attachment@1', + { fileId, mimeType: 'application/pdf', size: content.byteLength, filename: 'test.pdf' }, + { entityId: TEST_ENTITY_ID }, + ); const downloadRes = await t.app.request(`/attachments/${fileId}`, { headers: { Authorization: `Bearer ${TEST_TOKEN}` }, From 3ea5ff6e87089643d662ae6fb35dc6e4ef51b58f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 23 Jun 2026 11:54:24 +0000 Subject: [PATCH 4/7] fix: sanitize blocked MIME types on download, not just upload --- src/routes/attachments.ts | 20 ++++++++++++++++++- tests/routes/attachments.test.ts | 34 ++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/routes/attachments.ts b/src/routes/attachments.ts index 2f8962e..a00b9c9 100644 --- a/src/routes/attachments.ts +++ b/src/routes/attachments.ts @@ -60,7 +60,7 @@ export function attachmentRoutes(ctx: StackContext, maxAttachmentBytes: number): : 'attachment'; const headers: Record = { - 'Content-Type': attachmentContent?.mimeType ?? 'application/octet-stream', + 'Content-Type': sanitizeMimeType(attachmentContent?.mimeType ?? 'application/octet-stream'), 'Content-Length': String(attachmentContent?.size ?? data.byteLength), 'Content-Disposition': disposition, 'X-Content-Type-Options': 'nosniff', @@ -104,6 +104,24 @@ export function attachmentRoutes(ctx: StackContext, maxAttachmentBytes: number): return app; } +// MIME types that browsers can use to execute scripts or parse as markup. +// Callers requesting one of these as Content-Type receive application/octet-stream instead. +const BLOCKED_MIME_TYPES = new Set([ + 'text/html', + 'text/javascript', + 'application/javascript', + 'application/x-javascript', + 'application/xhtml+xml', + 'image/svg+xml', + 'text/xml', + 'application/xml', +]); + +function sanitizeMimeType(mimeType: string): string { + const base = mimeType.split(';')[0].trim().toLowerCase(); + return BLOCKED_MIME_TYPES.has(base) ? 'application/octet-stream' : mimeType; +} + /** * An attachment is accessible if: * 1. The requester is the stack owner, OR diff --git a/tests/routes/attachments.test.ts b/tests/routes/attachments.test.ts index d01d294..4c21fe7 100644 --- a/tests/routes/attachments.test.ts +++ b/tests/routes/attachments.test.ts @@ -221,6 +221,40 @@ describe('POST /attachments', () => { ); }); + it('serves blocked mimeTypes as application/octet-stream', async () => { + const content = new TextEncoder().encode(''); + const uploadRes = await t.app.request('/attachments', { + method: 'POST', + headers: { Authorization: `Bearer ${TEST_TOKEN}` }, + body: content, + }); + const { fileId } = (await uploadRes.json()) as { fileId: string }; + + for (const blocked of [ + 'text/html', + 'image/svg+xml', + 'text/javascript', + 'application/javascript', + ]) { + await t.ctx.stack.create( + '_attachment@1', + { fileId, mimeType: blocked, size: content.byteLength }, + { entityId: TEST_ENTITY_ID }, + ); + const res = await t.app.request(`/attachments/${fileId}`, { + headers: { Authorization: `Bearer ${TEST_TOKEN}` }, + }); + expect(res.headers.get('Content-Type'), `blocked type ${blocked}`).toBe( + 'application/octet-stream', + ); + // Clean up so the next iteration gets a fresh record + const meta = await t.ctx.stack.query({ + filter: { typeId: '_attachment@1', content: { fileId } }, + }); + for (const r of meta.records) await t.ctx.stack.delete(r.id, { hard: true }); + } + }); + it('returns 413 when Content-Length exceeds the limit (pre-check)', async () => { const smallConfig = { ...testConfig(t.dbPath), maxAttachmentBytes: 10 }; const smallApp = createApp(t.ctx, smallConfig, logger); From 5cda6bf04e08608207f64bb4d165cd808b5a551d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 23 Jun 2026 12:02:14 +0000 Subject: [PATCH 5/7] feat: support ?contentType and ?filename query params on GET /attachments/:fileId --- src/routes/attachments.ts | 37 ++++++++++++++++++++------------ tests/routes/attachments.test.ts | 27 +++++++++++++++++++++++ 2 files changed, 50 insertions(+), 14 deletions(-) diff --git a/src/routes/attachments.ts b/src/routes/attachments.ts index a00b9c9..7d5095f 100644 --- a/src/routes/attachments.ts +++ b/src/routes/attachments.ts @@ -44,24 +44,33 @@ export function attachmentRoutes(ctx: StackContext, maxAttachmentBytes: number): return c.json({ error: 'Attachment not found' }, 404); } - // _attachment@1 metadata record is optional — falls back to safe defaults - // when absent (e.g. right after upload before the caller has created the record). - const metaResult = await stack.query({ - filter: { typeId: `${SYSTEM_TYPES.ATTACHMENT}@1`, content: { fileId } }, - }); - const attachmentContent = (metaResult.records[0]?.content as AttachmentContent) ?? null; - - // Filename is pure metadata — only expose it from the requester's own record. - const ownRecord = metaResult.records.find((r) => r.entityId === auth?.entityId); - const visibleFilename = (ownRecord?.content as AttachmentContent | undefined)?.filename; + const contentTypeParam = c.req.query('contentType'); + const filenameParam = c.req.query('filename'); + + // Skip the _attachment@1 lookup entirely when the caller supplies both values. + // Otherwise query once and extract whatever is still needed. + let recordMimeType: string | undefined; + let recordFilename: string | undefined; + if (!contentTypeParam || !filenameParam) { + const metaResult = await stack.query({ + filter: { typeId: `${SYSTEM_TYPES.ATTACHMENT}@1`, content: { fileId } }, + }); + recordMimeType = (metaResult.records[0]?.content as AttachmentContent | undefined)?.mimeType; + // Filename is pure metadata — only expose it from the requester's own record. + const ownRecord = metaResult.records.find((r) => r.entityId === auth?.entityId); + recordFilename = (ownRecord?.content as AttachmentContent | undefined)?.filename; + } - const disposition = visibleFilename - ? `attachment; filename*=UTF-8''${encodeURIComponent(visibleFilename)}` + const resolvedFilename = filenameParam ?? recordFilename; + const disposition = resolvedFilename + ? `attachment; filename*=UTF-8''${encodeURIComponent(resolvedFilename)}` : 'attachment'; const headers: Record = { - 'Content-Type': sanitizeMimeType(attachmentContent?.mimeType ?? 'application/octet-stream'), - 'Content-Length': String(attachmentContent?.size ?? data.byteLength), + 'Content-Type': sanitizeMimeType( + contentTypeParam ?? recordMimeType ?? 'application/octet-stream', + ), + 'Content-Length': String(data.byteLength), 'Content-Disposition': disposition, 'X-Content-Type-Options': 'nosniff', }; diff --git a/tests/routes/attachments.test.ts b/tests/routes/attachments.test.ts index 4c21fe7..a56ff12 100644 --- a/tests/routes/attachments.test.ts +++ b/tests/routes/attachments.test.ts @@ -138,6 +138,33 @@ describe('GET /attachments/:fileId', () => { const { status } = await req(t.app, 'GET', `/attachments/${fileId}`, { token }); expect(status).toBe(401); }); + + it('uses ?contentType param as Content-Type without a metadata record', async () => { + const fileId = await putFile(t.ctx); + const res = await t.app.request(`/attachments/${fileId}?contentType=image/png`, { + headers: { Authorization: `Bearer ${TEST_TOKEN}` }, + }); + expect(res.status).toBe(200); + expect(res.headers.get('Content-Type')).toBe('image/png'); + }); + + it('uses ?filename param in Content-Disposition without a metadata record', async () => { + const fileId = await putFile(t.ctx); + const res = await t.app.request(`/attachments/${fileId}?filename=photo.png`, { + headers: { Authorization: `Bearer ${TEST_TOKEN}` }, + }); + expect(res.status).toBe(200); + expect(res.headers.get('Content-Disposition')).toBe("attachment; filename*=UTF-8''photo.png"); + }); + + it('sanitizes a blocked ?contentType param to application/octet-stream', async () => { + const fileId = await putFile(t.ctx); + const res = await t.app.request(`/attachments/${fileId}?contentType=text/html`, { + headers: { Authorization: `Bearer ${TEST_TOKEN}` }, + }); + expect(res.status).toBe(200); + expect(res.headers.get('Content-Type')).toBe('application/octet-stream'); + }); }); describe('POST /attachments', () => { From 037af386c9e0292a7ae2e7cbbe4e187e192048b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 23 Jun 2026 12:06:58 +0000 Subject: [PATCH 6/7] feat: infer Content-Type from ?filename extension, skipping DB query when filename is provided --- src/routes/attachments.ts | 47 ++++++++++++++++++++++++++++---- tests/routes/attachments.test.ts | 9 ++++++ 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/src/routes/attachments.ts b/src/routes/attachments.ts index 7d5095f..0cbea67 100644 --- a/src/routes/attachments.ts +++ b/src/routes/attachments.ts @@ -47,15 +47,20 @@ export function attachmentRoutes(ctx: StackContext, maxAttachmentBytes: number): const contentTypeParam = c.req.query('contentType'); const filenameParam = c.req.query('filename'); - // Skip the _attachment@1 lookup entirely when the caller supplies both values. - // Otherwise query once and extract whatever is still needed. + // When the caller provides a filename we can infer the MIME type from its + // extension, so no DB lookup is needed. Without a filename we must query + // for the requester's own _attachment@1 record (and for mimeType if + // contentType was not supplied). let recordMimeType: string | undefined; let recordFilename: string | undefined; - if (!contentTypeParam || !filenameParam) { + if (filenameParam === undefined) { const metaResult = await stack.query({ filter: { typeId: `${SYSTEM_TYPES.ATTACHMENT}@1`, content: { fileId } }, }); - recordMimeType = (metaResult.records[0]?.content as AttachmentContent | undefined)?.mimeType; + if (!contentTypeParam) { + recordMimeType = (metaResult.records[0]?.content as AttachmentContent | undefined) + ?.mimeType; + } // Filename is pure metadata — only expose it from the requester's own record. const ownRecord = metaResult.records.find((r) => r.entityId === auth?.entityId); recordFilename = (ownRecord?.content as AttachmentContent | undefined)?.filename; @@ -68,7 +73,8 @@ export function attachmentRoutes(ctx: StackContext, maxAttachmentBytes: number): const headers: Record = { 'Content-Type': sanitizeMimeType( - contentTypeParam ?? recordMimeType ?? 'application/octet-stream', + contentTypeParam ?? + resolveMimeType(recordMimeType ?? 'application/octet-stream', filenameParam), ), 'Content-Length': String(data.byteLength), 'Content-Disposition': disposition, @@ -131,6 +137,37 @@ function sanitizeMimeType(mimeType: string): string { return BLOCKED_MIME_TYPES.has(base) ? 'application/octet-stream' : mimeType; } +// Extension-to-MIME map used to infer Content-Type from a ?filename param. +// Omits types in BLOCKED_MIME_TYPES — they would be sanitized away regardless. +const EXTENSION_MIME: Record = { + jpg: 'image/jpeg', + jpeg: 'image/jpeg', + png: 'image/png', + gif: 'image/gif', + webp: 'image/webp', + ico: 'image/x-icon', + pdf: 'application/pdf', + txt: 'text/plain', + md: 'text/markdown', + csv: 'text/csv', + json: 'application/json', + mp4: 'video/mp4', + webm: 'video/webm', + mov: 'video/quicktime', + mp3: 'audio/mpeg', + wav: 'audio/wav', + ogg: 'audio/ogg', + m4a: 'audio/mp4', + zip: 'application/zip', + gz: 'application/gzip', +}; + +function resolveMimeType(declared: string, filename: string | undefined): string { + if (declared !== 'application/octet-stream' || !filename) return declared; + const ext = filename.split('.').pop()?.toLowerCase(); + return (ext && EXTENSION_MIME[ext]) || declared; +} + /** * An attachment is accessible if: * 1. The requester is the stack owner, OR diff --git a/tests/routes/attachments.test.ts b/tests/routes/attachments.test.ts index a56ff12..3615add 100644 --- a/tests/routes/attachments.test.ts +++ b/tests/routes/attachments.test.ts @@ -157,6 +157,15 @@ describe('GET /attachments/:fileId', () => { expect(res.headers.get('Content-Disposition')).toBe("attachment; filename*=UTF-8''photo.png"); }); + it('infers Content-Type from ?filename extension when no ?contentType is given', async () => { + const fileId = await putFile(t.ctx); + const res = await t.app.request(`/attachments/${fileId}?filename=photo.png`, { + headers: { Authorization: `Bearer ${TEST_TOKEN}` }, + }); + expect(res.status).toBe(200); + expect(res.headers.get('Content-Type')).toBe('image/png'); + }); + it('sanitizes a blocked ?contentType param to application/octet-stream', async () => { const fileId = await putFile(t.ctx); const res = await t.app.request(`/attachments/${fileId}?contentType=text/html`, { From 4602935ed84845b42dc6a0712546ddfe5d908675 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 23 Jun 2026 12:27:46 +0000 Subject: [PATCH 7/7] =?UTF-8?q?refactor:=20remove=20=5Fattachment@1=20DB?= =?UTF-8?q?=20query=20from=20GET=20/attachments=20=E2=80=94=20callers=20su?= =?UTF-8?q?pply=20metadata=20via=20query=20params?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GET handler no longer queries the database for metadata. Callers pass ?contentType and/or ?filename as query params; without them the response falls back to application/octet-stream with no Content-Disposition filename. Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01HteEBim64NucEifnFH1yqQ --- src/routes/attachments.ts | 34 ++++----------------------- tests/routes/attachments.test.ts | 40 ++++++++------------------------ 2 files changed, 15 insertions(+), 59 deletions(-) diff --git a/src/routes/attachments.ts b/src/routes/attachments.ts index 0cbea67..160272d 100644 --- a/src/routes/attachments.ts +++ b/src/routes/attachments.ts @@ -1,6 +1,5 @@ import { Hono } from 'hono'; import { SYSTEM_TYPES } from '@haverstack/core'; -import type { AttachmentContent } from '@haverstack/core'; import type { AppEnv } from '../types.js'; import type { StackContext } from '../stack.js'; import { requireAuth, requireOwner } from '../middleware/auth.js'; @@ -47,41 +46,18 @@ export function attachmentRoutes(ctx: StackContext, maxAttachmentBytes: number): const contentTypeParam = c.req.query('contentType'); const filenameParam = c.req.query('filename'); - // When the caller provides a filename we can infer the MIME type from its - // extension, so no DB lookup is needed. Without a filename we must query - // for the requester's own _attachment@1 record (and for mimeType if - // contentType was not supplied). - let recordMimeType: string | undefined; - let recordFilename: string | undefined; - if (filenameParam === undefined) { - const metaResult = await stack.query({ - filter: { typeId: `${SYSTEM_TYPES.ATTACHMENT}@1`, content: { fileId } }, - }); - if (!contentTypeParam) { - recordMimeType = (metaResult.records[0]?.content as AttachmentContent | undefined) - ?.mimeType; - } - // Filename is pure metadata — only expose it from the requester's own record. - const ownRecord = metaResult.records.find((r) => r.entityId === auth?.entityId); - recordFilename = (ownRecord?.content as AttachmentContent | undefined)?.filename; - } - - const resolvedFilename = filenameParam ?? recordFilename; - const disposition = resolvedFilename - ? `attachment; filename*=UTF-8''${encodeURIComponent(resolvedFilename)}` + const disposition = filenameParam + ? `attachment; filename*=UTF-8''${encodeURIComponent(filenameParam)}` : 'attachment'; - const headers: Record = { + return c.newResponse(data as unknown as Uint8Array, 200, { 'Content-Type': sanitizeMimeType( - contentTypeParam ?? - resolveMimeType(recordMimeType ?? 'application/octet-stream', filenameParam), + contentTypeParam ?? resolveMimeType('application/octet-stream', filenameParam), ), 'Content-Length': String(data.byteLength), 'Content-Disposition': disposition, 'X-Content-Type-Options': 'nosniff', - }; - - return c.newResponse(data as unknown as Uint8Array, 200, headers); + }); }); // DELETE /attachments/:fileId diff --git a/tests/routes/attachments.test.ts b/tests/routes/attachments.test.ts index 3615add..2e04f2f 100644 --- a/tests/routes/attachments.test.ts +++ b/tests/routes/attachments.test.ts @@ -5,7 +5,6 @@ import { testConfig, logger, TEST_TOKEN, - TEST_ENTITY_ID, OTHER_ENTITY_ID, type TestApp, } from '../setup.js'; @@ -205,7 +204,7 @@ describe('POST /attachments', () => { expect(metaResult.records).toHaveLength(0); }); - it('serves file with application/octet-stream fallback when no _attachment@1 record exists', async () => { + it('serves file with application/octet-stream when no query params are given', async () => { const content = new TextEncoder().encode('raw bytes'); const uploadRes = await t.app.request('/attachments', { method: 'POST', @@ -226,30 +225,20 @@ describe('POST /attachments', () => { expect(downloadRes.headers.get('Content-Disposition')).toBe('attachment'); }); - it('returns mimeType and filename from _attachment@1 record created after upload', async () => { + it('uses ?contentType and ?filename query params for Content-Type and Content-Disposition', async () => { const content = new TextEncoder().encode('PDF content'); const uploadRes = await t.app.request('/attachments', { method: 'POST', - headers: { - Authorization: `Bearer ${TEST_TOKEN}`, - 'Content-Type': 'application/octet-stream', - 'Content-Disposition': 'attachment; filename="ignored.pdf"', - }, + headers: { Authorization: `Bearer ${TEST_TOKEN}` }, body: content, }); expect(uploadRes.status).toBe(201); const { fileId } = (await uploadRes.json()) as { fileId: string }; - // Create the metadata record (what ScopedStack.putAttachment does via POST /records) - await t.ctx.stack.create( - '_attachment@1', - { fileId, mimeType: 'application/pdf', size: content.byteLength, filename: 'test.pdf' }, - { entityId: TEST_ENTITY_ID }, + const downloadRes = await t.app.request( + `/attachments/${fileId}?contentType=application/pdf&filename=test.pdf`, + { headers: { Authorization: `Bearer ${TEST_TOKEN}` } }, ); - - const downloadRes = await t.app.request(`/attachments/${fileId}`, { - headers: { Authorization: `Bearer ${TEST_TOKEN}` }, - }); expect(downloadRes.status).toBe(200); expect(downloadRes.headers.get('Content-Type')).toBe('application/pdf'); expect(downloadRes.headers.get('Content-Disposition')).toBe( @@ -257,7 +246,7 @@ describe('POST /attachments', () => { ); }); - it('serves blocked mimeTypes as application/octet-stream', async () => { + it('serves blocked ?contentType values as application/octet-stream', async () => { const content = new TextEncoder().encode(''); const uploadRes = await t.app.request('/attachments', { method: 'POST', @@ -272,22 +261,13 @@ describe('POST /attachments', () => { 'text/javascript', 'application/javascript', ]) { - await t.ctx.stack.create( - '_attachment@1', - { fileId, mimeType: blocked, size: content.byteLength }, - { entityId: TEST_ENTITY_ID }, + const res = await t.app.request( + `/attachments/${fileId}?contentType=${encodeURIComponent(blocked)}`, + { headers: { Authorization: `Bearer ${TEST_TOKEN}` } }, ); - const res = await t.app.request(`/attachments/${fileId}`, { - headers: { Authorization: `Bearer ${TEST_TOKEN}` }, - }); expect(res.headers.get('Content-Type'), `blocked type ${blocked}`).toBe( 'application/octet-stream', ); - // Clean up so the next iteration gets a fresh record - const meta = await t.ctx.stack.query({ - filter: { typeId: '_attachment@1', content: { fileId } }, - }); - for (const r of meta.records) await t.ctx.stack.delete(r.id, { hard: true }); } });