From 8f7282066e37ec2657f340f122027aa176599d88 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 26 Jun 2026 06:05:57 -0500 Subject: [PATCH] Add workspace file downloads Add first-class workspace file downloads, broader attachment content-type support, and the stream-lifetime limiter fix from PR review. --- .../src/types/workspace-file-resource.ts | 8 +- .../src/validators/workspace-file-resource.ts | 2 +- server/src/__tests__/file-resources.test.ts | 151 +++++++++++++++++- .../__tests__/issue-attachment-routes.test.ts | 35 +++- server/src/routes/file-resources.ts | 67 +++++++- server/src/routes/issues.ts | 5 - .../src/services/workspace-file-resources.ts | 60 ++++++- ui/src/api/file-resources.ts | 8 + .../components/FileViewerSheet.copy.test.tsx | 2 +- ui/src/components/FileViewerSheet.test.tsx | 2 +- ui/src/components/FileViewerSheet.tsx | 23 +++ .../components/WorkspaceFileBrowser.test.tsx | 6 +- ui/src/components/WorkspaceFileBrowser.tsx | 33 +++- .../workspace-file-browser.stories.tsx | 2 +- 14 files changed, 373 insertions(+), 31 deletions(-) diff --git a/packages/shared/src/types/workspace-file-resource.ts b/packages/shared/src/types/workspace-file-resource.ts index 8f6aaed423..6ca345b247 100644 --- a/packages/shared/src/types/workspace-file-resource.ts +++ b/packages/shared/src/types/workspace-file-resource.ts @@ -34,7 +34,7 @@ export interface ResolvedWorkspaceResource { denialReason?: string | null; capabilities: { preview: boolean; - download: false; + download: boolean; listChildren: boolean; }; } @@ -61,10 +61,10 @@ export interface WorkspaceFileListFileItem { contentType?: string | null; byteSize?: number | null; modifiedAt?: string | null; - previewKind: Exclude; + previewKind: WorkspaceFilePreviewKind; capabilities: { - preview: true; - download: false; + preview: boolean; + download: true; listChildren: false; }; } diff --git a/packages/shared/src/validators/workspace-file-resource.ts b/packages/shared/src/validators/workspace-file-resource.ts index 1eea721547..9ec9c6d51b 100644 --- a/packages/shared/src/validators/workspace-file-resource.ts +++ b/packages/shared/src/validators/workspace-file-resource.ts @@ -90,7 +90,7 @@ export const resolvedWorkspaceResourceSchema = z.object({ denialReason: z.string().nullable().optional(), capabilities: z.object({ preview: z.boolean(), - download: z.literal(false), + download: z.boolean(), listChildren: z.boolean(), }), }); diff --git a/server/src/__tests__/file-resources.test.ts b/server/src/__tests__/file-resources.test.ts index 1f0516aab7..8064f4bcf0 100644 --- a/server/src/__tests__/file-resources.test.ts +++ b/server/src/__tests__/file-resources.test.ts @@ -231,6 +231,53 @@ describeEmbeddedPostgres("workspace file resources", () => { expect(res.body.content.data).toContain("export const ok"); }); + it("lists and downloads non-previewable workspace files", async () => { + const { root, projectRoot, executionRoot } = await makeWorkspace(); + const graph = await seedGraph(db, { projectRoot, executionRoot }); + const relativePath = "artifacts/archive.bin"; + const bytes = Buffer.from([0, 1, 2, 3, 4, 255]); + await fs.mkdir(path.join(projectRoot, "artifacts"), { recursive: true }); + await fs.writeFile(path.join(projectRoot, relativePath), bytes); + + const app = createApp(db, { + type: "board", + userId: "board-user", + companyIds: [graph.companyId], + source: "session", + isInstanceAdmin: false, + }); + + const listed = await request(app) + .get(`/api/issues/${graph.issueId}/file-resources/list`) + .query({ workspace: "project", mode: "all", path: "artifacts" }); + + expect(listed.status).toBe(200); + expect(listed.body.items).toEqual(expect.arrayContaining([ + expect.objectContaining({ + kind: "file", + relativePath, + previewKind: "unsupported", + capabilities: { preview: false, download: true, listChildren: false }, + }), + ])); + expect(JSON.stringify(listed.body)).not.toContain(root); + + const downloaded = await request(app) + .get(`/api/issues/${graph.issueId}/file-resources/content`) + .query({ workspace: "project", path: relativePath, download: "1" }) + .buffer(true) + .parse((res, callback) => { + const chunks: Buffer[] = []; + res.on("data", (chunk) => chunks.push(Buffer.from(chunk))); + res.on("end", () => callback(null, Buffer.concat(chunks))); + }); + + expect(downloaded.status).toBe(200); + expect(downloaded.headers["content-disposition"]).toBe('attachment; filename="archive.bin"'); + expect(downloaded.headers["x-content-type-options"]).toBe("nosniff"); + expect(Buffer.compare(downloaded.body as Buffer, bytes)).toBe(0); + }); + it("falls back from an execution workspace miss to the project workspace", async () => { const { projectRoot, executionRoot } = await makeWorkspace(); const graph = await seedGraph(db, { projectRoot, executionRoot }); @@ -1125,12 +1172,15 @@ describeEmbeddedPostgres("workspace file resources", () => { workspaceKind: "project_workspace", workspaceId: "11111111-1111-4111-8111-111111111111", previewKind: "text", - capabilities: { preview: true, download: false, listChildren: false }, + capabilities: { preview: true, download: true, listChildren: false }, }; }), readContent: vi.fn(async () => { throw new Error("not used"); }), + prepareDownload: vi.fn(async () => { + throw new Error("not used"); + }), }; const resolveLimitedApp = createApp( db, @@ -1187,11 +1237,14 @@ describeEmbeddedPostgres("workspace file resources", () => { workspaceKind: "project_workspace", workspaceId: "11111111-1111-4111-8111-111111111111", previewKind: "text", - capabilities: { preview: true, download: false, listChildren: false }, + capabilities: { preview: true, download: true, listChildren: false }, }, content: { encoding: "utf8", data: "# Visible\n" }, }; }), + prepareDownload: vi.fn(async () => { + throw new Error("not used"); + }), }; const contentLimitedApp = createApp( db, @@ -1225,6 +1278,92 @@ describeEmbeddedPostgres("workspace file resources", () => { expect(JSON.stringify(rowsAfterLimits.map((row) => row.details))).not.toContain(projectRoot); }); + it("holds download concurrency slots until the file stream completes", async () => { + if (process.platform === "win32") return; + + const { projectRoot, executionRoot } = await makeWorkspace(); + const graph = await seedGraph(db, { projectRoot, executionRoot }); + const fifoPath = path.join(projectRoot, "slow-download.bin"); + await execFileAsync("mkfifo", [fifoPath]); + let slowDownloadStarted: (() => void) | null = null; + const downloadStarted = new Promise((resolve) => { + slowDownloadStarted = resolve; + }); + const service: WorkspaceFileResourceService = { + getIssue: vi.fn(async () => ({ companyId: graph.companyId })), + list: vi.fn(async () => { + throw new Error("not used"); + }), + resolve: vi.fn(async () => { + throw new Error("not used"); + }), + readContent: vi.fn(async () => { + throw new Error("not used"); + }), + prepareDownload: vi.fn(async () => { + slowDownloadStarted?.(); + return { + resource: { + kind: "file", + provider: "local_fs", + title: "slow-download.bin", + displayPath: "slow-download.bin", + workspaceLabel: "Workspace", + workspaceKind: "project_workspace", + workspaceId: "11111111-1111-4111-8111-111111111111", + previewKind: "unsupported", + contentType: "application/octet-stream", + byteSize: null, + capabilities: { preview: false, download: true, listChildren: false }, + }, + realPath: fifoPath, + }; + }), + }; + const app = createApp( + db, + { + type: "board", + userId: "board-user", + companyIds: [graph.companyId], + source: "session", + isInstanceAdmin: false, + }, + { + service, + limiter: createFileResourceLimiter({ maxConcurrent: 1, maxRequests: 100, windowMs: 60_000 }), + }, + ); + const firstDownload = request(app) + .get(`/api/issues/${graph.issueId}/file-resources/content`) + .query({ path: "slow-download.bin", download: "1" }) + .buffer(true) + .parse((res, callback) => { + const chunks: Buffer[] = []; + res.on("data", (chunk) => chunks.push(Buffer.from(chunk))); + res.on("end", () => callback(null, Buffer.concat(chunks))); + }); + const firstDownloadResponse = firstDownload.then((res) => res); + + await downloadStarted; + await new Promise((resolve) => setImmediate(resolve)); + + const secondDownload = await request(app) + .get(`/api/issues/${graph.issueId}/file-resources/content`) + .query({ path: "slow-download.bin", download: "1" }); + expect(secondDownload.status).toBe(429); + + const writer = await fs.open(fifoPath, "w"); + await writer.write(Buffer.from("slow")); + await writer.close(); + + const first = await firstDownloadResponse; + expect(first.status).toBe(200); + expect(first.headers["content-length"]).toBeUndefined(); + expect(first.headers["content-disposition"]).toBe('attachment; filename="slow-download.bin"'); + expect(Buffer.compare(first.body as Buffer, Buffer.from("slow"))).toBe(0); + }); + it("uses tighter list-specific rate and concurrency limits", async () => { const { projectRoot, executionRoot } = await makeWorkspace(); const graph = await seedGraph(db, { projectRoot, executionRoot }); @@ -1267,6 +1406,9 @@ describeEmbeddedPostgres("workspace file resources", () => { readContent: vi.fn(async () => { throw new Error("not used"); }), + prepareDownload: vi.fn(async () => { + throw new Error("not used"); + }), }; const app = createApp( db, @@ -1340,12 +1482,15 @@ describeEmbeddedPostgres("file resource route guards", () => { workspaceKind: "project_workspace", workspaceId: "11111111-1111-4111-8111-111111111111", previewKind: "text", - capabilities: { preview: true, download: false, listChildren: false }, + capabilities: { preview: true, download: true, listChildren: false }, }; }), readContent: vi.fn(async () => { throw new Error("not used"); }), + prepareDownload: vi.fn(async () => { + throw new Error("not used"); + }), }; const app = express(); app.use((req, _res, next) => { diff --git a/server/src/__tests__/issue-attachment-routes.test.ts b/server/src/__tests__/issue-attachment-routes.test.ts index 3b47cabe3f..b7f6fb7379 100644 --- a/server/src/__tests__/issue-attachment-routes.test.ts +++ b/server/src/__tests__/issue-attachment-routes.test.ts @@ -302,23 +302,32 @@ describe("issue attachment routes", () => { }); }); - it("rejects unsupported upload content types before storing the file", async () => { + it("accepts arbitrary upload content types while preserving the stored MIME type", async () => { const storage = createStorageService(); mockIssueService.getById.mockResolvedValue({ id: "11111111-1111-4111-8111-111111111111", companyId: "company-1", identifier: "PAP-1", }); + mockIssueService.createAttachment.mockResolvedValue(makeAttachment("application/x-msdownload", "payload.exe")); const app = await createApp(storage); const res = await request(app) .post("/api/companies/company-1/issues/11111111-1111-4111-8111-111111111111/attachments") .attach("file", Buffer.from("exe"), { filename: "payload.exe", contentType: "application/x-msdownload" }); - expect(res.status).toBe(422); - expect(res.body.error).toBe("Unsupported attachment content type: application/x-msdownload"); - expect(storage.__calls.putFile).toBeUndefined(); - expect(mockIssueService.createAttachment).not.toHaveBeenCalled(); + expect(res.status).toBe(201); + expect(storage.__calls.putFile).toMatchObject({ + contentType: "application/x-msdownload", + originalFilename: "payload.exe", + }); + expect(mockIssueService.createAttachment).toHaveBeenCalledWith( + expect.objectContaining({ + contentType: "application/x-msdownload", + originalFilename: "payload.exe", + }), + ); + expect(res.body.contentType).toBe("application/x-msdownload"); }); it("enforces the process-level issue attachment limit even when the company limit allows more", async () => { @@ -383,6 +392,22 @@ describe("issue attachment routes", () => { expect(res.headers["x-content-type-options"]).toBe("nosniff"); }); + it("serves arbitrary binary attachments as downloads with nosniff", async () => { + const storage = createStorageService(); + mockIssueService.getAttachmentById.mockResolvedValue(makeAttachment("application/x-msdownload", "payload.exe")); + + const app = await createApp(storage); + const res = await request(app) + .get("/api/attachments/attachment-1/content") + .buffer(true) + .parse(parseBinaryResponse); + + expect(res.status).toBe(200); + expect(res.headers["content-type"]).toContain("application/x-msdownload"); + expect(res.headers["content-disposition"]).toBe('attachment; filename="payload.exe"'); + expect(res.headers["x-content-type-options"]).toBe("nosniff"); + }); + it("keeps image attachments inline for previews", async () => { const storage = createStorageService(); mockIssueService.getAttachmentById.mockResolvedValue(makeAttachment("image/png", "preview.png")); diff --git a/server/src/routes/file-resources.ts b/server/src/routes/file-resources.ts index 2f2ca638af..0d4e8aacd4 100644 --- a/server/src/routes/file-resources.ts +++ b/server/src/routes/file-resources.ts @@ -1,3 +1,5 @@ +import { createReadStream } from "node:fs"; +import { pipeline } from "node:stream/promises"; import { Router } from "express"; import { ZodError } from "zod"; import type { Db } from "@paperclipai/db"; @@ -35,6 +37,11 @@ export type WorkspaceFileResourceService = { input: { path: string; workspace?: "auto" | "execution" | "project" | null; projectId?: string | null; workspaceId?: string | null }, opts?: { issue?: Awaited> }, ): Promise; + prepareDownload( + issueId: string, + input: { path: string; workspace?: "auto" | "execution" | "project" | null; projectId?: string | null; workspaceId?: string | null }, + opts?: { issue?: Awaited> }, + ): Promise<{ resource: ResolvedWorkspaceResource; realPath: string }>; }; type FileResourceLimiter = { @@ -104,6 +111,14 @@ function limiterKey(companyId: string, actorId: string, issueId: string) { return `${companyId}:${actorId}:${issueId}`; } +function parseBooleanQuery(value: unknown) { + return value === true || value === "true" || value === "1"; +} + +function safeAttachmentFilename(value: string) { + return value.replaceAll("\"", "").replace(/[\\/\r\n]/g, "_") || "workspace-file"; +} + function readQuery(query: unknown) { let parsed; try { @@ -267,7 +282,7 @@ export function fileResourceRoutes(db: Db, opts: { projectId?: string | null; workspaceId?: string | null; error: unknown; - action?: "issue.file_resource_content_denied" | "issue.file_resource_resolve_denied"; + action?: "issue.file_resource_content_denied" | "issue.file_resource_resolve_denied" | "issue.file_resource_download_denied"; }) { await logActivity(db, { companyId: input.companyId, @@ -595,6 +610,56 @@ export function fileResourceRoutes(db: Db, opts: { throw error; } try { + if (parseBooleanQuery(req.query.download)) { + let result: Awaited> | null = null; + try { + result = await svc.prepareDownload(req.params.issueId, query, { issue }); + } catch (error) { + await logDeniedAttempt({ + companyId: issue.companyId, + actor, + issueId: req.params.issueId, + displayPath: query.path, + projectId: query.projectId, + workspaceId: query.workspaceId, + error, + action: "issue.file_resource_download_denied", + }); + throw error; + } + + await logActivity(db, { + companyId: issue.companyId, + actorType: actor.actorType, + actorId: actor.actorId, + action: "issue.file_resource_download", + entityType: "issue", + entityId: req.params.issueId, + agentId: actor.agentId, + runId: actor.runId, + details: activityDetails({ + outcome: "success", + workspaceKind: result.resource.workspaceKind, + workspaceId: result.resource.workspaceId, + projectId: result.resource.projectId ?? null, + projectName: result.resource.projectName ?? null, + displayPath: result.resource.displayPath, + byteSize: result.resource.byteSize ?? null, + contentType: result.resource.contentType ?? null, + }), + }); + + res.setHeader("Content-Type", result.resource.contentType ?? "application/octet-stream"); + if (result.resource.byteSize != null) { + res.setHeader("Content-Length", String(result.resource.byteSize)); + } + res.setHeader("Cache-Control", "private, max-age=60"); + res.setHeader("X-Content-Type-Options", "nosniff"); + res.setHeader("Content-Disposition", `attachment; filename="${safeAttachmentFilename(result.resource.title)}"`); + await pipeline(createReadStream(result.realPath), res); + return; + } + let result: WorkspaceFileContent | null = null; try { result = await svc.readContent(req.params.issueId, query, { issue }); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 2c2e6b32d6..fdc3c19816 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -109,7 +109,6 @@ import { import { shouldWakeAssigneeOnCheckout } from "./issues-checkout-wakeup.js"; import { isInlineAttachmentContentType, - isAllowedContentType, normalizeIssueAttachmentMaxBytes, normalizeContentType, SVG_CONTENT_TYPE, @@ -8198,10 +8197,6 @@ export function issueRoutes( res.status(422).json({ error: "Attachment is empty" }); return; } - if (!isAllowedContentType(contentType)) { - res.status(422).json({ error: `Unsupported attachment content type: ${contentType}` }); - return; - } const parsedMeta = createIssueAttachmentMetadataSchema.safeParse(req.body ?? {}); if (!parsedMeta.success) { diff --git a/server/src/services/workspace-file-resources.ts b/server/src/services/workspace-file-resources.ts index 805f3a1328..d7a3965dd1 100644 --- a/server/src/services/workspace-file-resources.ts +++ b/server/src/services/workspace-file-resources.ts @@ -242,9 +242,8 @@ function listItemFromStat(input: { stat: { size: number; mtime: Date }; }): WorkspaceFileListItem | null { const contentType = contentTypeForPath(input.relativePath); - const previewKind = previewKindForKnownContentType(contentType); - if (!previewKind || previewKind === "unsupported") return null; - if (input.stat.size > previewCapForKind(previewKind)) return null; + const previewKind = previewKindForKnownContentType(contentType) ?? "unsupported"; + const previewable = previewKind !== "unsupported" && input.stat.size <= previewCapForKind(previewKind); return { kind: "file", @@ -262,8 +261,8 @@ function listItemFromStat(input: { modifiedAt: input.stat.mtime.toISOString(), previewKind, capabilities: { - preview: true, - download: false, + preview: previewable, + download: true, listChildren: false, }, }; @@ -471,7 +470,7 @@ async function statLocalCandidate(candidate: WorkspaceCandidate, normalized: Nor denialReason, capabilities: { preview: !tooLarge && !unsupported, - download: false, + download: true, listChildren: false, }, }, @@ -1394,10 +1393,59 @@ export function workspaceFileResourceService(db: Db) { throw unprocessable("No local-readable workspace is available for this issue", { code: "no_local_workspace" }); } + async function prepareDownload(issueId: string, input: { + path: string; + workspace?: WorkspaceFileSelector | null; + projectId?: string | null; + workspaceId?: string | null; + }, opts: { issue?: IssueRow } = {}): Promise { + const issue = opts.issue ?? await getIssue(issueId); + const selector = input.workspace ?? "auto"; + const explicitTarget = Boolean(input.projectId || input.workspaceId); + const normalized = normalizeWorkspaceRelativePath(input.path); + const candidates = await listCandidates(issue, selector, input); + if (candidates.length === 0) { + throw unprocessable("No workspace is available for this issue", { code: "no_workspace" }); + } + + let lastNotFound: unknown = null; + for (const candidate of candidates) { + if (candidate.remote) { + if (explicitTarget || selector !== "auto") { + throw unprocessable("Remote workspaces cannot be downloaded by the server", { code: "remote_workspace" }); + } + continue; + } + try { + return await statLocalCandidate(candidate, normalized); + } catch (error) { + if (!explicitTarget && selector === "auto" && isHttpStatus(error, 404)) { + lastNotFound = error; + continue; + } + throw error; + } + } + + if (lastNotFound && !explicitTarget && selector === "auto") { + const discovered = await discoverUniqueProjectWorkspaceMatch( + issue, + new Set(candidates.map((candidate) => candidate.workspaceId)), + async (candidate) => statLocalCandidate(candidate, normalized), + ); + if (discovered.state === "ambiguous") throwAmbiguousWorkspacePath(discovered.count); + if (discovered.state === "one") return discovered.value; + } + + if (lastNotFound) throw lastNotFound; + throw unprocessable("No local-readable workspace is available for this issue", { code: "no_local_workspace" }); + } + return { getIssue, list, resolve, readContent, + prepareDownload, }; } diff --git a/ui/src/api/file-resources.ts b/ui/src/api/file-resources.ts index affc5f77a1..b2e10dd16b 100644 --- a/ui/src/api/file-resources.ts +++ b/ui/src/api/file-resources.ts @@ -42,6 +42,12 @@ function buildQuery(query: FileResourceQuery | FileResourceListQuery): string { return params.toString(); } +export function buildFileResourceDownloadUrl(issueId: string, query: FileResourceQuery): string { + const params = new URLSearchParams(buildQuery(query)); + params.set("download", "1"); + return `/api/issues/${encodeURIComponent(issueId)}/file-resources/content?${params.toString()}`; +} + export const fileResourcesApi = { list(issueId: string, query: FileResourceListQuery = {}): Promise { const search = buildQuery(query); @@ -62,4 +68,6 @@ export const fileResourcesApi = { `/issues/${encodeURIComponent(issueId)}/file-resources/content?${buildQuery(query)}`, ); }, + + downloadUrl: buildFileResourceDownloadUrl, }; diff --git a/ui/src/components/FileViewerSheet.copy.test.tsx b/ui/src/components/FileViewerSheet.copy.test.tsx index 2f3493b5f5..1eb5127b67 100644 --- a/ui/src/components/FileViewerSheet.copy.test.tsx +++ b/ui/src/components/FileViewerSheet.copy.test.tsx @@ -60,7 +60,7 @@ const resolvedResource: ResolvedWorkspaceResource = { contentType: "text/markdown; charset=utf-8", byteSize: 42, previewKind: "text", - capabilities: { preview: true, download: false, listChildren: false }, + capabilities: { preview: true, download: true, listChildren: false }, }; const content: WorkspaceFileContent = { diff --git a/ui/src/components/FileViewerSheet.test.tsx b/ui/src/components/FileViewerSheet.test.tsx index cede832805..c01eb2a42b 100644 --- a/ui/src/components/FileViewerSheet.test.tsx +++ b/ui/src/components/FileViewerSheet.test.tsx @@ -61,7 +61,7 @@ describe("FileContentViewer", () => { contentType: "text/plain; charset=utf-8", byteSize: 18, previewKind: "text", - capabilities: { preview: true, download: false, listChildren: false }, + capabilities: { preview: true, download: true, listChildren: false }, }, content: { encoding: "utf8", diff --git a/ui/src/components/FileViewerSheet.tsx b/ui/src/components/FileViewerSheet.tsx index 554e082c23..5706bda1ec 100644 --- a/ui/src/components/FileViewerSheet.tsx +++ b/ui/src/components/FileViewerSheet.tsx @@ -16,6 +16,7 @@ import { Check, Cloud, Copy, + Download, Eye, FileCode2, FileSearch, @@ -529,6 +530,9 @@ export function FileViewerSheet({ const resolvedResource: ResolvedWorkspaceResource | undefined = resolveQuery.data; const canPreview = resolvedResource?.capabilities.preview ?? false; + const downloadUrl = state && resolvedResource?.capabilities.download + ? fileResourcesApi.downloadUrl(issueId, state) + : null; const contentQuery = useQuery({ queryKey: state @@ -780,6 +784,25 @@ export function FileViewerSheet({ Back to files ) : null} + {state ? ( + downloadUrl ? ( + + ) : null + ) : null} {state ? (