diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index 5c5af615..ce94ec6a 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -392,6 +392,12 @@ Operational policy: - `issue_id` uuid fk not null - `asset_id` uuid fk not null - `issue_comment_id` uuid fk null +- V1 attachment serving contract: + - Default upload allowlist includes common images, PDF, plain text/markdown/JSON/CSV/HTML, ZIP, and video artifacts (`video/mp4`, `video/webm`, `video/quicktime`). + - Attachment reads are company-scoped and expose stable path metadata: `contentPath`/`openPath` for inline-safe viewing and `downloadPath` for forced download. + - Inline-safe responses use `Content-Disposition: inline`; unsafe types and explicit download requests use `attachment`. + - Video attachments are inline-safe and support single `Range: bytes=start-end` requests with `206`, `Content-Range`, and `Accept-Ranges: bytes` for browser playback/seeking. +- Attachment-backed artifact work products use `type: "artifact"`, `provider: "paperclip"`, and metadata with `attachmentId`, `contentType`, `byteSize`, `contentPath`, `openPath`, `downloadPath`, and optional `originalFilename`. ## 7.15 `documents` + `document_revisions` + `issue_documents` diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index ab8cf59a..51e7f20d 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -411,6 +411,7 @@ export type { DocumentTextProjection, DocumentTextRange, UpdateDocumentAnnotationThreadRequest, + AttachmentArtifactWorkProductMetadata, Issue, IssueAssigneeAdapterOverrides, IssueBlockerAttention, @@ -921,6 +922,7 @@ export { createIssueAttachmentMetadataSchema, createIssueWorkProductSchema, updateIssueWorkProductSchema, + attachmentArtifactWorkProductMetadataSchema, issueWorkProductTypeSchema, issueWorkProductStatusSchema, issueWorkProductReviewStateSchema, diff --git a/packages/shared/src/types/index.ts b/packages/shared/src/types/index.ts index 8da1aba1..e3c80954 100644 --- a/packages/shared/src/types/index.ts +++ b/packages/shared/src/types/index.ts @@ -172,6 +172,7 @@ export type { IssueWorkProductProvider, IssueWorkProductStatus, IssueWorkProductReviewState, + AttachmentArtifactWorkProductMetadata, } from "./work-product.js"; export type { Issue, diff --git a/packages/shared/src/types/issue.ts b/packages/shared/src/types/issue.ts index 9637d977..46ef3010 100644 --- a/packages/shared/src/types/issue.ts +++ b/packages/shared/src/types/issue.ts @@ -845,4 +845,6 @@ export interface IssueAttachment { createdAt: Date; updatedAt: Date; contentPath: string; + openPath?: string; + downloadPath?: string; } diff --git a/packages/shared/src/types/work-product.ts b/packages/shared/src/types/work-product.ts index 297a2463..21ad3675 100644 --- a/packages/shared/src/types/work-product.ts +++ b/packages/shared/src/types/work-product.ts @@ -53,3 +53,13 @@ export interface IssueWorkProduct { createdAt: Date; updatedAt: Date; } + +export interface AttachmentArtifactWorkProductMetadata { + attachmentId: string; + contentType: string; + byteSize: number; + contentPath: string; + openPath: string; + downloadPath: string; + originalFilename?: string | null; +} diff --git a/packages/shared/src/validators/index.ts b/packages/shared/src/validators/index.ts index fb4838cf..c424e711 100644 --- a/packages/shared/src/validators/index.ts +++ b/packages/shared/src/validators/index.ts @@ -282,6 +282,7 @@ export { export { createIssueWorkProductSchema, updateIssueWorkProductSchema, + attachmentArtifactWorkProductMetadataSchema, issueWorkProductTypeSchema, issueWorkProductStatusSchema, issueWorkProductReviewStateSchema, diff --git a/packages/shared/src/validators/work-product.test.ts b/packages/shared/src/validators/work-product.test.ts new file mode 100644 index 00000000..214a9637 --- /dev/null +++ b/packages/shared/src/validators/work-product.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from "vitest"; +import { attachmentArtifactWorkProductMetadataSchema } from "./work-product.js"; + +describe("attachmentArtifactWorkProductMetadataSchema", () => { + it("accepts the attachment-backed artifact metadata contract", () => { + const parsed = attachmentArtifactWorkProductMetadataSchema.parse({ + attachmentId: "11111111-1111-4111-8111-111111111111", + contentType: "video/mp4", + byteSize: 1234, + contentPath: "/api/attachments/11111111-1111-4111-8111-111111111111/content", + openPath: "/api/attachments/11111111-1111-4111-8111-111111111111/content", + downloadPath: "/api/attachments/11111111-1111-4111-8111-111111111111/content?download=1", + originalFilename: "demo.mp4", + }); + + expect(parsed.contentType).toBe("video/mp4"); + expect(parsed.downloadPath).toContain("download=1"); + }); +}); diff --git a/packages/shared/src/validators/work-product.ts b/packages/shared/src/validators/work-product.ts index b068b9c9..ca8d3852 100644 --- a/packages/shared/src/validators/work-product.ts +++ b/packages/shared/src/validators/work-product.ts @@ -29,6 +29,18 @@ export const issueWorkProductReviewStateSchema = z.enum([ "changes_requested", ]); +export const attachmentArtifactWorkProductMetadataSchema = z.object({ + attachmentId: z.string().uuid(), + contentType: z.string().min(1), + byteSize: z.number().int().nonnegative(), + contentPath: z.string().min(1), + openPath: z.string().min(1), + downloadPath: z.string().min(1), + originalFilename: z.string().optional().nullable(), +}); + +export type AttachmentArtifactWorkProductMetadata = z.infer; + export const createIssueWorkProductSchema = z.object({ projectId: z.string().uuid().optional().nullable(), executionWorkspaceId: z.string().uuid().optional().nullable(), diff --git a/server/src/__tests__/attachment-types.test.ts b/server/src/__tests__/attachment-types.test.ts index 7dfc34d2..a1508cf0 100644 --- a/server/src/__tests__/attachment-types.test.ts +++ b/server/src/__tests__/attachment-types.test.ts @@ -112,7 +112,7 @@ describe("normalizeContentType", () => { describe("isInlineAttachmentContentType", () => { it("allows the configured inline-safe types", () => { - for (const contentType of ["image/png", "image/svg+xml", "application/pdf", "text/plain"]) { + for (const contentType of ["image/png", "image/svg+xml", "application/pdf", "text/plain", "video/mp4"]) { expect(isInlineAttachmentContentType(contentType)).toBe(true); } }); diff --git a/server/src/__tests__/issue-attachment-routes.test.ts b/server/src/__tests__/issue-attachment-routes.test.ts index 1dd2b988..04f93a68 100644 --- a/server/src/__tests__/issue-attachment-routes.test.ts +++ b/server/src/__tests__/issue-attachment-routes.test.ts @@ -113,7 +113,7 @@ type TestStorageService = StorageService & { }; }; -function createStorageService(): TestStorageService { +function createStorageService(body = Buffer.from("test")): TestStorageService { const calls: TestStorageService["__calls"] = {}; return { provider: "local_disk", @@ -130,15 +130,15 @@ function createStorageService(): TestStorageService { }; }, getObject: vi.fn(async () => ({ - stream: Readable.from(Buffer.from("test")), - contentLength: 4, + stream: Readable.from(body), + contentLength: body.length, })), headObject: vi.fn(), deleteObject: vi.fn(), }; } -async function createApp(storage: StorageService) { +async function createApp(storage: StorageService, options?: { companyIds?: string[]; source?: string }) { const [{ errorHandler }, { issueRoutes }] = await Promise.all([ vi.importActual("../middleware/index.js"), vi.importActual("../routes/issues.js"), @@ -148,8 +148,8 @@ async function createApp(storage: StorageService) { (req as any).actor = { type: "board", userId: "local-board", - companyIds: ["company-1"], - source: "local_implicit", + companyIds: options?.companyIds ?? ["company-1"], + source: options?.source ?? "local_implicit", isInstanceAdmin: false, }; next(); @@ -254,6 +254,52 @@ describe("issue attachment routes", () => { expect(res.body.contentType).toBe("application/zip"); }); + it("accepts default video uploads for issue attachments", async () => { + const storage = createStorageService(); + mockIssueService.getById.mockResolvedValue({ + id: "11111111-1111-4111-8111-111111111111", + companyId: "company-1", + identifier: "PAP-1", + }); + mockIssueService.createAttachment.mockResolvedValue(makeAttachment("video/mp4", "clip.mp4")); + + 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("mp4"), { filename: "clip.mp4", contentType: "video/mp4" }); + + expect(res.status).toBe(201); + expect(storage.__calls.putFile).toMatchObject({ + contentType: "video/mp4", + originalFilename: "clip.mp4", + }); + expect(res.body).toMatchObject({ + contentType: "video/mp4", + contentPath: "/api/attachments/attachment-1/content", + openPath: "/api/attachments/attachment-1/content", + downloadPath: "/api/attachments/attachment-1/content?download=1", + }); + }); + + it("rejects unsupported upload content types before storing the file", async () => { + const storage = createStorageService(); + mockIssueService.getById.mockResolvedValue({ + id: "11111111-1111-4111-8111-111111111111", + companyId: "company-1", + identifier: "PAP-1", + }); + + 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(); + }); + it("enforces the process-level issue attachment limit even when the company limit allows more", async () => { const storage = createStorageService(); mockIssueService.getById.mockResolvedValue({ @@ -326,4 +372,61 @@ describe("issue attachment routes", () => { 'inline; filename="preview.png"', ]).toContain(res.headers["content-disposition"]); }); + + it("serves video attachments inline with byte-range support", async () => { + const storage = createStorageService(Buffer.from("abcdef")); + mockIssueService.getAttachmentById.mockResolvedValue({ + ...makeAttachment("video/mp4", "clip.mp4"), + byteSize: 6, + }); + + const app = await createApp(storage); + const res = await request(app) + .get("/api/attachments/attachment-1/content") + .set("Range", "bytes=1-3"); + + expect(res.status).toBe(206); + expect(res.headers["content-type"]).toContain("video/mp4"); + expect(res.headers["accept-ranges"]).toBe("bytes"); + expect(res.headers["content-range"]).toBe("bytes 1-3/6"); + expect(res.headers["content-length"]).toBe("3"); + expect(res.headers["content-disposition"]).toBe('inline; filename="clip.mp4"'); + expect(Buffer.from(res.body).toString("utf8")).toBe("bcd"); + }); + + it("forces video downloads when the download path is requested", async () => { + const storage = createStorageService(); + mockIssueService.getAttachmentById.mockResolvedValue(makeAttachment("video/webm", "clip.webm")); + + const app = await createApp(storage); + const res = await request(app).get("/api/attachments/attachment-1/content?download=1"); + + expect(res.status).toBe(200); + expect(res.headers["content-disposition"]).toBe('attachment; filename="clip.webm"'); + }); + + it("rejects invalid byte ranges without streaming the object", async () => { + const storage = createStorageService(); + mockIssueService.getAttachmentById.mockResolvedValue(makeAttachment("video/mp4", "clip.mp4")); + + const app = await createApp(storage); + const res = await request(app) + .get("/api/attachments/attachment-1/content") + .set("Range", "bytes=99-100"); + + expect(res.status).toBe(416); + expect(res.headers["content-range"]).toBe("bytes */4"); + expect(storage.getObject).not.toHaveBeenCalled(); + }); + + it("rejects cross-company attachment content reads", async () => { + const storage = createStorageService(); + mockIssueService.getAttachmentById.mockResolvedValue(makeAttachment("video/mp4", "clip.mp4")); + + const app = await createApp(storage, { companyIds: ["company-2"], source: "session" }); + const res = await request(app).get("/api/attachments/attachment-1/content"); + + expect(res.status).toBe(403); + expect(storage.getObject).not.toHaveBeenCalled(); + }); }); diff --git a/server/src/attachment-types.ts b/server/src/attachment-types.ts index 23ad3f76..4784ef82 100644 --- a/server/src/attachment-types.ts +++ b/server/src/attachment-types.ts @@ -1,7 +1,7 @@ /** * Shared attachment content-type configuration. * - * By default a curated set of image/document/text types are allowed. Set the + * By default a curated set of image/document/text/media types are allowed. Set the * `PAPERCLIP_ALLOWED_ATTACHMENT_TYPES` environment variable to a * comma-separated list of MIME types or wildcard patterns to expand the * allowed set for routes that use this allowlist. @@ -26,11 +26,15 @@ export const DEFAULT_ALLOWED_TYPES: readonly string[] = [ "image/webp", "image/gif", "application/pdf", + "application/zip", "text/markdown", "text/plain", "application/json", "text/csv", "text/html", + "video/mp4", + "video/webm", + "video/quicktime", ]; export const DEFAULT_ATTACHMENT_CONTENT_TYPE = "application/octet-stream"; @@ -42,6 +46,9 @@ export const INLINE_ATTACHMENT_TYPES: readonly string[] = [ "text/markdown", "application/json", "text/csv", + "video/mp4", + "video/webm", + "video/quicktime", ]; /** diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index b2f2d872..6f1c6a23 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -1,4 +1,5 @@ import { randomUUID } from "node:crypto"; +import { Transform } from "node:stream"; import { Router, type Request, type Response } from "express"; import multer from "multer"; import { z } from "zod"; @@ -91,6 +92,7 @@ import { import { shouldWakeAssigneeOnCheckout } from "./issues-checkout-wakeup.js"; import { isInlineAttachmentContentType, + isAllowedContentType, normalizeIssueAttachmentMaxBytes, normalizeContentType, SVG_CONTENT_TYPE, @@ -1103,12 +1105,67 @@ export function issueRoutes( } function withContentPath(attachment: T) { + const contentPath = `/api/attachments/${attachment.id}/content`; return { ...attachment, - contentPath: `/api/attachments/${attachment.id}/content`, + contentPath, + openPath: contentPath, + downloadPath: `${contentPath}?download=1`, }; } + type ParsedAttachmentRange = + | { kind: "none" } + | { kind: "invalid" } + | { kind: "range"; start: number; end: number }; + + function parseAttachmentRangeHeader(raw: string | undefined, contentLength: number): ParsedAttachmentRange { + if (!raw) return { kind: "none" }; + if (!Number.isSafeInteger(contentLength) || contentLength <= 0) return { kind: "invalid" }; + + const prefix = "bytes="; + if (!raw.toLowerCase().startsWith(prefix)) return { kind: "invalid" }; + const spec = raw.slice(prefix.length).trim(); + if (!spec || spec.includes(",")) return { kind: "invalid" }; + + const [startRaw, endRaw] = spec.split("-", 2); + if (endRaw === undefined) return { kind: "invalid" }; + + if (startRaw === "") { + const suffixLength = Number.parseInt(endRaw, 10); + if (!Number.isSafeInteger(suffixLength) || suffixLength <= 0) return { kind: "invalid" }; + const start = Math.max(contentLength - suffixLength, 0); + return { kind: "range", start, end: contentLength - 1 }; + } + + const start = Number.parseInt(startRaw, 10); + if (!Number.isSafeInteger(start) || start < 0 || start >= contentLength) return { kind: "invalid" }; + const end = endRaw === "" ? contentLength - 1 : Number.parseInt(endRaw, 10); + if (!Number.isSafeInteger(end) || end < start) return { kind: "invalid" }; + return { kind: "range", start, end: Math.min(end, contentLength - 1) }; + } + + function createByteRangeStream(start: number, end: number) { + let offset = 0; + return new Transform({ + transform(chunk: Buffer | string, _encoding, callback) { + const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); + const chunkStart = offset; + const chunkEnd = offset + buffer.length - 1; + offset += buffer.length; + + if (chunkEnd < start || chunkStart > end) { + callback(); + return; + } + + const sliceStart = Math.max(start - chunkStart, 0); + const sliceEnd = Math.min(end - chunkStart + 1, buffer.length); + callback(null, buffer.subarray(sliceStart, sliceEnd)); + }, + }); + } + function parseBooleanQuery(value: unknown) { return value === true || value === "true" || value === "1"; } @@ -6081,6 +6138,10 @@ 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) { @@ -6139,22 +6200,49 @@ export function issueRoutes( } assertCompanyAccess(req, attachment.companyId); + const contentLength = attachment.byteSize; + const range = parseAttachmentRangeHeader( + typeof req.headers.range === "string" ? req.headers.range : undefined, + contentLength, + ); + res.setHeader("Accept-Ranges", "bytes"); + if (range.kind === "invalid") { + res.setHeader("Content-Range", `bytes */${contentLength}`); + res.status(416).end(); + return; + } + const object = await storage.getObject(attachment.companyId, attachment.objectKey); const responseContentType = normalizeContentType(attachment.contentType || object.contentType); res.setHeader("Content-Type", responseContentType); - res.setHeader("Content-Length", String(attachment.byteSize || object.contentLength || 0)); res.setHeader("Cache-Control", "private, max-age=60"); res.setHeader("X-Content-Type-Options", "nosniff"); if (responseContentType === SVG_CONTENT_TYPE) { res.setHeader("Content-Security-Policy", "sandbox; default-src 'none'; img-src 'self' data:; style-src 'unsafe-inline'"); } const filename = attachment.originalFilename ?? "attachment"; - const disposition = isInlineAttachmentContentType(responseContentType) ? "inline" : "attachment"; + const disposition = parseBooleanQuery(req.query.download) + ? "attachment" + : isInlineAttachmentContentType(responseContentType) ? "inline" : "attachment"; res.setHeader("Content-Disposition", `${disposition}; filename=\"${filename.replaceAll("\"", "")}\"`); object.stream.on("error", (err) => { next(err); }); + if (range.kind === "range") { + const rangeLength = range.end - range.start + 1; + res.status(206); + res.setHeader("Content-Length", String(rangeLength)); + res.setHeader("Content-Range", `bytes ${range.start}-${range.end}/${contentLength}`); + const rangeStream = createByteRangeStream(range.start, range.end); + rangeStream.on("error", (err) => { + next(err); + }); + object.stream.pipe(rangeStream).pipe(res); + return; + } + + res.setHeader("Content-Length", String(contentLength || object.contentLength || 0)); object.stream.pipe(res); });