diff --git a/docs/api.md b/docs/api.md index c50bb63..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 | -| ------ | ---------------------- | ---------- | --------------- | -| POST | `/attachments` | Required | Upload a file | -| GET | `/attachments/:fileId` | Optional | Download a file | -| DELETE | `/attachments/:fileId` | Owner only | Delete a file | +| 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 | ## Entity & Tokens diff --git a/src/routes/attachments.ts b/src/routes/attachments.ts index 4b009ab..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'; @@ -10,7 +9,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 +24,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 +36,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,33 +43,40 @@ 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. - 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'); - const disposition = visibleFilename - ? `attachment; filename*=UTF-8''${encodeURIComponent(visibleFilename)}` + const disposition = filenameParam + ? `attachment; filename*=UTF-8''${encodeURIComponent(filenameParam)}` : 'attachment'; - const headers: Record = { - 'Content-Type': attachmentContent.mimeType, - 'Content-Length': String(attachmentContent.size), + return c.newResponse(data as unknown as Uint8Array, 200, { + 'Content-Type': sanitizeMimeType( + 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 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,18 +95,8 @@ 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. +// Callers requesting one of these as Content-Type receive application/octet-stream instead. const BLOCKED_MIME_TYPES = new Set([ 'text/html', 'text/javascript', @@ -130,37 +113,27 @@ function sanitizeMimeType(mimeType: string): string { 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. +// 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 = { - // 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', }; diff --git a/tests/routes/attachments.test.ts b/tests/routes/attachments.test.ts index 580abd1..2e04f2f 100644 --- a/tests/routes/attachments.test.ts +++ b/tests/routes/attachments.test.ts @@ -137,6 +137,42 @@ 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('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`, { + headers: { Authorization: `Bearer ${TEST_TOKEN}` }, + }); + expect(res.status).toBe(200); + expect(res.headers.get('Content-Type')).toBe('application/octet-stream'); + }); }); describe('POST /attachments', () => { @@ -148,14 +184,33 @@ describe('POST /attachments', () => { await t.cleanup(); }); - it('preserves filename from Content-Disposition on upload and returns it on download', async () => { - const content = new TextEncoder().encode('PDF content'); + 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': 'application/octet-stream', - 'Content-Disposition': 'attachment; filename="test.pdf"', + '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 when no query params are given', 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, }); @@ -166,11 +221,56 @@ describe('POST /attachments', () => { 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('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}` }, + body: content, + }); + expect(uploadRes.status).toBe(201); + const { fileId } = (await uploadRes.json()) as { fileId: string }; + + const downloadRes = await t.app.request( + `/attachments/${fileId}?contentType=application/pdf&filename=test.pdf`, + { 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", ); }); + it('serves blocked ?contentType values 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', + ]) { + const res = await t.app.request( + `/attachments/${fileId}?contentType=${encodeURIComponent(blocked)}`, + { headers: { Authorization: `Bearer ${TEST_TOKEN}` } }, + ); + expect(res.headers.get('Content-Type'), `blocked type ${blocked}`).toBe( + 'application/octet-stream', + ); + } + }); + 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);