From e790ff708b75726700a82613586e47a24c0e91d2 Mon Sep 17 00:00:00 2001 From: Matt Date: Sun, 10 May 2026 16:50:16 +0200 Subject: [PATCH] feat(documents): path-style download URLs for rep-facing readability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Storage paths stay UUID-flat per the established CRM pattern (every other content type — brochures, berth PDFs, invoices, reports, templates, expense receipts — uses the same shape). The new catch-all /api/v1/documents/[id]/download/[...slug] route serves files keyed on doc id but rebuilds the slug from current state and 404s on mismatch — a hand-edited or stale link can't render the wrong filename or fold a wrong-folder path into a forwarded URL. URLs in shared links / browser tabs read like 'Deals 2026/Q1/contract.pdf' even though storage keys remain UUIDs. listDocuments + getDocumentById now hydrate a `downloadUrl` field per row (null when no file is attached yet) so UI consumers don't reconstruct paths. Filename is batch-fetched via files-table join to keep the query builder shape unchanged. Tests: 5 integration cases — happy-path stream, wrong-folder slug, wrong-filename slug, orphaned doc (no fileId), cross-port (tenancy isolation). Storage backend swapped to a real FilesystemBackend in a tempdir so the byte-streaming path is exercised end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../[id]/download/[...slug]/handlers.ts | 98 ++++++++ .../[id]/download/[...slug]/route.ts | 5 + src/lib/services/documents.service.ts | 105 ++++++++- .../document-path-style-download.test.ts | 213 ++++++++++++++++++ 4 files changed, 418 insertions(+), 3 deletions(-) create mode 100644 src/app/api/v1/documents/[id]/download/[...slug]/handlers.ts create mode 100644 src/app/api/v1/documents/[id]/download/[...slug]/route.ts create mode 100644 tests/integration/document-path-style-download.test.ts diff --git a/src/app/api/v1/documents/[id]/download/[...slug]/handlers.ts b/src/app/api/v1/documents/[id]/download/[...slug]/handlers.ts new file mode 100644 index 00000000..2cc988fa --- /dev/null +++ b/src/app/api/v1/documents/[id]/download/[...slug]/handlers.ts @@ -0,0 +1,98 @@ +/** + * Route handler for `GET /api/v1/documents/[id]/download/[...slug]`. + * + * Lives in handlers.ts (not route.ts) so integration tests can call it + * directly, bypassing auth/permission middleware (per CLAUDE.md "Route + * handler exports" convention). + * + * Lookup is keyed off the doc id; the slug embeds the current folder path + + * filename so a forwarded link reads like `Deals 2026/Q1/contract.pdf` even + * though the underlying storage key is a UUID. The slug is rebuilt from + * current state and compared with the supplied path — a stale or + * hand-edited URL 404s rather than silently serving the wrong file. + */ + +import { Readable } from 'node:stream'; + +import { NextResponse } from 'next/server'; +import { and, eq } from 'drizzle-orm'; + +import { type RouteHandler } from '@/lib/api/helpers'; +import { db } from '@/lib/db'; +import { documents, files } from '@/lib/db/schema/documents'; +import { errorResponse, NotFoundError } from '@/lib/errors'; +import { getStorageBackend } from '@/lib/storage'; +import { listTree, type FolderNode } from '@/lib/services/document-folders.service'; +import { findFolderPath } from '@/lib/services/documents.service'; + +type DownloadParams = { id: string; slug: string[] }; + +export const downloadHandler: RouteHandler = async (_req, ctx, params) => { + try { + const docId = params.id; + if (!docId) throw new NotFoundError('Document'); + + const doc = await db + .select({ + id: documents.id, + folderId: documents.folderId, + fileId: documents.fileId, + fileStoragePath: files.storagePath, + fileMimeType: files.mimeType, + fileFilename: files.filename, + }) + .from(documents) + .leftJoin(files, eq(files.id, documents.fileId)) + .where(and(eq(documents.id, docId), eq(documents.portId, ctx.portId))) + .limit(1) + .then((r) => r[0]); + + if (!doc) throw new NotFoundError('Document'); + if (!doc.fileStoragePath || !doc.fileFilename) { + throw new NotFoundError('Document file'); + } + + const tree = await listTree(ctx.portId); + const expected = buildExpectedSlug(doc.folderId, doc.fileFilename, tree); + const supplied = (params.slug ?? []).map(decodeURIComponent).join('/'); + if (supplied !== expected) throw new NotFoundError('Document at this path'); + + const backend = await getStorageBackend(); + const nodeStream = await backend.get(doc.fileStoragePath); + const webStream = Readable.toWeb(nodeStream as Readable) as ReadableStream; + + return new NextResponse(webStream, { + status: 200, + headers: { + 'content-type': doc.fileMimeType ?? 'application/octet-stream', + 'content-disposition': `inline; filename="${sanitizeFilenameForHeader(doc.fileFilename)}"`, + }, + }); + } catch (error) { + return errorResponse(error); + } +}; + +function buildExpectedSlug( + folderId: string | null, + filename: string, + tree: FolderNode[], +): string { + const segments: string[] = []; + if (folderId) { + const path = findFolderPath(tree, folderId); + for (const node of path) segments.push(node.name); + } + segments.push(filename); + return segments.join('/'); +} + +/** + * Strip control + quote characters from a filename before placing it in the + * Content-Disposition header value. The legacy `inline; filename="..."` form + * is what the rest of the CRM emits today; this is the same minimal + * sanitization the other download endpoints rely on. + */ +function sanitizeFilenameForHeader(filename: string): string { + return filename.replace(/["\\\r\n]/g, '_'); +} diff --git a/src/app/api/v1/documents/[id]/download/[...slug]/route.ts b/src/app/api/v1/documents/[id]/download/[...slug]/route.ts new file mode 100644 index 00000000..85151f43 --- /dev/null +++ b/src/app/api/v1/documents/[id]/download/[...slug]/route.ts @@ -0,0 +1,5 @@ +import { withAuth, withPermission } from '@/lib/api/helpers'; + +import { downloadHandler } from './handlers'; + +export const GET = withAuth(withPermission('documents', 'view', downloadHandler)); diff --git a/src/lib/services/documents.service.ts b/src/lib/services/documents.service.ts index 8f3f173d..18563d90 100644 --- a/src/lib/services/documents.service.ts +++ b/src/lib/services/documents.service.ts @@ -34,7 +34,11 @@ import { voidDocument as documensoVoid, } from '@/lib/services/documenso-client'; import { getPortEoiSigners } from '@/lib/services/documenso-payload'; -import { listTree, collectDescendantIds } from '@/lib/services/document-folders.service'; +import { + listTree, + collectDescendantIds, + type FolderNode, +} from '@/lib/services/document-folders.service'; import type { CreateDocumentInput, UpdateDocumentInput, @@ -217,7 +221,7 @@ export async function listDocuments( ? documents.documentType : documents.createdAt; - return buildListQuery({ + const result = await buildListQuery({ table: documents, portIdColumn: documents.portId, portId, @@ -230,6 +234,85 @@ export async function listDocuments( page, pageSize: limit, }); + + const hydrated = await hydrateDocumentsWithDownloadUrl(portId, result.data); + return { ...result, data: hydrated }; +} + +// ─── Download URL helpers ───────────────────────────────────────────────────── + +/** + * Resolve the rep-facing download URL for a document. The URL embeds the + * folder path + filename for browser-tab / shared-link readability, but the + * route handler keys lookup off the doc id and validates the slug for truth + * — a hand-edited URL with the wrong path 404s instead of silently serving + * the wrong file. + * + * Pass the resolved folder tree once per request and call this for each doc + * so we don't refetch the tree per row. Returns `null` when the document has + * no attached file (signature-only docs pre-completion); UI consumers branch + * on that to decide whether to render the download affordance. + */ +export function buildDocumentDownloadUrl( + doc: { id: string; folderId: string | null; filename: string | null }, + folderTree: readonly FolderNode[], +): string | null { + if (!doc.filename) return null; + const segments: string[] = []; + if (doc.folderId) { + const path = findFolderPath(folderTree, doc.folderId); + for (const node of path) segments.push(encodeURIComponent(node.name)); + } + segments.push(encodeURIComponent(doc.filename)); + return `/api/v1/documents/${doc.id}/download/${segments.join('/')}`; +} + +/** + * Walk the folder tree to materialize the ancestor chain that ends at + * `id`. Returns roots-first; empty when the id is missing (orphan + * `folder_id` pointer — see listTree's intentional silent drop). + */ +export function findFolderPath(tree: readonly FolderNode[], id: string): FolderNode[] { + for (const node of tree) { + if (node.id === id) return [node]; + const inChild = findFolderPath(node.children, id); + if (inChild.length > 0) return [node, ...inChild]; + } + return []; +} + +type DocumentRow = typeof documents.$inferSelect; +type DocumentRowWithDownload = DocumentRow & { downloadUrl: string | null }; + +async function hydrateDocumentsWithDownloadUrl( + portId: string, + rows: DocumentRow[], +): Promise { + if (rows.length === 0) return []; + + const fileIds = Array.from( + new Set(rows.map((r) => r.fileId).filter((v): v is string => v !== null)), + ); + const filenameById = new Map(); + if (fileIds.length > 0) { + const fileRows = await db + .select({ id: files.id, filename: files.filename }) + .from(files) + .where(and(inArray(files.id, fileIds), eq(files.portId, portId))); + for (const f of fileRows) filenameById.set(f.id, f.filename); + } + + const tree = await listTree(portId); + return rows.map((row) => { + const filename = row.fileId ? (filenameById.get(row.fileId) ?? null) : null; + return { + ...row, + downloadUrl: buildDocumentDownloadUrl( + { id: row.id, folderId: row.folderId, filename }, + tree, + ), + }; + }); } // ─── Deal docs for a berth ──────────────────────────────────────────────────── @@ -344,7 +427,23 @@ export async function getDocumentById(id: string, portId: string) { }); if (!doc) throw new NotFoundError('Document'); - return doc; + + let filename: string | null = null; + if (doc.fileId) { + const fileRow = await db + .select({ filename: files.filename }) + .from(files) + .where(and(eq(files.id, doc.fileId), eq(files.portId, portId))) + .limit(1) + .then((r) => r[0]); + filename = fileRow?.filename ?? null; + } + const tree = await listTree(portId); + const downloadUrl = buildDocumentDownloadUrl( + { id: doc.id, folderId: doc.folderId, filename }, + tree, + ); + return { ...doc, downloadUrl }; } /** diff --git a/tests/integration/document-path-style-download.test.ts b/tests/integration/document-path-style-download.test.ts new file mode 100644 index 00000000..9879858b --- /dev/null +++ b/tests/integration/document-path-style-download.test.ts @@ -0,0 +1,213 @@ +/** + * Integration test: GET /api/v1/documents/[id]/download/[...slug] + * + * Verifies the slug truth-check and tenancy isolation on the path-style + * download route. The storage backend is swapped for a real FilesystemBackend + * rooted in a tempdir so the byte-streaming path is exercised end-to-end. + */ + +import { mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import * as path from 'node:path'; + +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { db } from '@/lib/db'; +import { documents, files } from '@/lib/db/schema/documents'; +import { user } from '@/lib/db/schema/users'; +import { createFolder } from '@/lib/services/document-folders.service'; +import { makeMockCtx } from '../helpers/route-tester'; +import { makeFullPermissions, makePort } from '../helpers/factories'; + +let testUserId: string; + +beforeAll(async () => { + const [u] = await db.select({ id: user.id }).from(user).limit(1); + testUserId = u!.id; +}); + +describe('GET /api/v1/documents/[id]/download/[...slug]', () => { + let storageRoot: string; + let backend: import('@/lib/storage/filesystem').FilesystemBackend; + + beforeEach(async () => { + storageRoot = await mkdtemp(path.join(tmpdir(), 'pn-doc-dl-')); + const { FilesystemBackend } = await import('@/lib/storage/filesystem'); + backend = await FilesystemBackend.create({ + root: storageRoot, + proxyHmacSecretEncrypted: null, + }); + vi.doMock('@/lib/storage', async () => { + const real = await vi.importActual('@/lib/storage'); + return { ...real, getStorageBackend: vi.fn(async () => backend) }; + }); + }); + + afterEach(async () => { + vi.doUnmock('@/lib/storage'); + await rm(storageRoot, { recursive: true, force: true }); + }); + + async function setupDoc(opts: { + portId: string; + folderId: string | null; + filename: string; + body: Buffer; + storagePath: string; + }) { + await backend.put(opts.storagePath, opts.body, { + contentType: 'application/pdf', + sizeBytes: opts.body.length, + }); + const [fileRow] = await db + .insert(files) + .values({ + portId: opts.portId, + filename: opts.filename, + originalName: opts.filename, + mimeType: 'application/pdf', + sizeBytes: String(opts.body.length), + storagePath: opts.storagePath, + uploadedBy: testUserId, + }) + .returning(); + const [docRow] = await db + .insert(documents) + .values({ + portId: opts.portId, + documentType: 'other', + title: opts.filename, + createdBy: testUserId, + folderId: opts.folderId, + fileId: fileRow!.id, + }) + .returning(); + return { fileRow: fileRow!, docRow: docRow! }; + } + + it('streams the file when the slug matches the current folder path + filename', async () => { + const port = await makePort(); + const folder = await createFolder(port.id, testUserId, { name: 'Deals 2026', parentId: null }); + const sub = await createFolder(port.id, testUserId, { name: 'Q1', parentId: folder.id }); + const body = Buffer.from('hello world'); + const { docRow } = await setupDoc({ + portId: port.id, + folderId: sub.id, + filename: 'contract.pdf', + body, + storagePath: 'test/contract.pdf', + }); + + const { downloadHandler } = await import( + '@/app/api/v1/documents/[id]/download/[...slug]/handlers' + ); + const ctx = makeMockCtx({ portId: port.id, permissions: makeFullPermissions() }); + const req = new Request('http://localhost/api/v1/documents/x/download/whatever') as never; + const res = await downloadHandler(req, ctx, { + id: docRow.id, + slug: ['Deals 2026', 'Q1', 'contract.pdf'], + }); + expect(res.status).toBe(200); + const buf = Buffer.from(await res.arrayBuffer()); + expect(buf.toString()).toBe('hello world'); + expect(res.headers.get('content-type')).toBe('application/pdf'); + }); + + it('404s when the folder-path segments do not match current state', async () => { + const port = await makePort(); + const folder = await createFolder(port.id, testUserId, { name: 'Real Folder', parentId: null }); + const { docRow } = await setupDoc({ + portId: port.id, + folderId: folder.id, + filename: 'spec.pdf', + body: Buffer.from('x'), + storagePath: 'test/spec.pdf', + }); + + const { downloadHandler } = await import( + '@/app/api/v1/documents/[id]/download/[...slug]/handlers' + ); + const ctx = makeMockCtx({ portId: port.id, permissions: makeFullPermissions() }); + const req = new Request('http://localhost/x') as never; + const res = await downloadHandler(req, ctx, { + id: docRow.id, + slug: ['Wrong Folder', 'spec.pdf'], + }); + expect(res.status).toBe(404); + }); + + it('404s when the filename segment does not match current state', async () => { + const port = await makePort(); + const folder = await createFolder(port.id, testUserId, { name: 'F', parentId: null }); + const { docRow } = await setupDoc({ + portId: port.id, + folderId: folder.id, + filename: 'real.pdf', + body: Buffer.from('x'), + storagePath: 'test/real.pdf', + }); + + const { downloadHandler } = await import( + '@/app/api/v1/documents/[id]/download/[...slug]/handlers' + ); + const ctx = makeMockCtx({ portId: port.id, permissions: makeFullPermissions() }); + const req = new Request('http://localhost/x') as never; + const res = await downloadHandler(req, ctx, { + id: docRow.id, + slug: ['F', 'wrong.pdf'], + }); + expect(res.status).toBe(404); + }); + + it('404s when the document has no attached file (orphaned doc)', async () => { + const port = await makePort(); + const [docRow] = await db + .insert(documents) + .values({ + portId: port.id, + documentType: 'other', + title: 'No file', + createdBy: testUserId, + folderId: null, + }) + .returning(); + + const { downloadHandler } = await import( + '@/app/api/v1/documents/[id]/download/[...slug]/handlers' + ); + const ctx = makeMockCtx({ portId: port.id, permissions: makeFullPermissions() }); + const req = new Request('http://localhost/x') as never; + const res = await downloadHandler(req, ctx, { + id: docRow!.id, + slug: ['anything.pdf'], + }); + expect(res.status).toBe(404); + }); + + it('404s when the document belongs to another port (tenant isolation)', async () => { + const portA = await makePort(); + const portB = await makePort(); + const folder = await createFolder(portA.id, testUserId, { + name: 'Cross', + parentId: null, + }); + const { docRow } = await setupDoc({ + portId: portA.id, + folderId: folder.id, + filename: 'a.pdf', + body: Buffer.from('x'), + storagePath: 'test/a.pdf', + }); + + const { downloadHandler } = await import( + '@/app/api/v1/documents/[id]/download/[...slug]/handlers' + ); + const ctx = makeMockCtx({ portId: portB.id, permissions: makeFullPermissions() }); + const req = new Request('http://localhost/x') as never; + const res = await downloadHandler(req, ctx, { + id: docRow.id, + slug: ['Cross', 'a.pdf'], + }); + expect(res.status).toBe(404); + }); +});