Allow arbitrary issue attachments

Co-Authored-By: Paperclip <noreply@paperclip.ing>

server/src/__tests__/attachment-types.test.ts

@@ -1,8 +1,11 @@
 import { describe, it, expect } from "vitest";
 import {
-  parseAllowedTypes,
-  matchesContentType,
   DEFAULT_ALLOWED_TYPES,
+  INLINE_ATTACHMENT_TYPES,
+  isInlineAttachmentContentType,
+  matchesContentType,
+  normalizeContentType,
+  parseAllowedTypes,
 } from "../attachment-types.js";
 
 describe("parseAllowedTypes", () => {
@@ -95,3 +98,28 @@ describe("matchesContentType", () => {
     expect(matchesContentType("application/zip", patterns)).toBe(true);
   });
 });
+
+describe("normalizeContentType", () => {
+  it("lowercases and trims explicit types", () => {
+    expect(normalizeContentType(" Application/Zip ")).toBe("application/zip");
+  });
+
+  it("falls back to octet-stream when the type is missing", () => {
+    expect(normalizeContentType(undefined)).toBe("application/octet-stream");
+    expect(normalizeContentType("")).toBe("application/octet-stream");
+  });
+});
+
+describe("isInlineAttachmentContentType", () => {
+  it("allows the configured inline-safe types", () => {
+    for (const contentType of ["image/png", "image/svg+xml", "application/pdf", "text/plain"]) {
+      expect(isInlineAttachmentContentType(contentType)).toBe(true);
+    }
+  });
+
+  it("rejects potentially unsafe or binary download types", () => {
+    expect(INLINE_ATTACHMENT_TYPES).not.toContain("text/html");
+    expect(isInlineAttachmentContentType("text/html")).toBe(false);
+    expect(isInlineAttachmentContentType("application/zip")).toBe(false);
+  });
+});

server/src/__tests__/issue-attachment-routes.test.ts

@@ -0,0 +1,175 @@
+import { Readable } from "node:stream";
+import express from "express";
+import request from "supertest";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { errorHandler } from "../middleware/index.js";
+import { issueRoutes } from "../routes/issues.js";
+import type { StorageService } from "../storage/types.js";
+
+const mockIssueService = vi.hoisted(() => ({
+  getById: vi.fn(),
+  getByIdentifier: vi.fn(),
+  createAttachment: vi.fn(),
+  getAttachmentById: vi.fn(),
+}));
+
+const mockLogActivity = vi.hoisted(() => vi.fn(async () => undefined));
+
+vi.mock("../services/index.js", () => ({
+  accessService: () => ({
+    canUser: vi.fn(),
+    hasPermission: vi.fn(),
+  }),
+  agentService: () => ({
+    getById: vi.fn(),
+  }),
+  documentService: () => ({}),
+  executionWorkspaceService: () => ({}),
+  feedbackService: () => ({
+    listIssueVotesForUser: vi.fn(async () => []),
+    saveIssueVote: vi.fn(async () => ({ vote: null, consentEnabledNow: false, sharingEnabled: false })),
+  }),
+  goalService: () => ({}),
+  heartbeatService: () => ({
+    wakeup: vi.fn(async () => undefined),
+    reportRunActivity: vi.fn(async () => undefined),
+    getRun: vi.fn(async () => null),
+    getActiveRunForAgent: vi.fn(async () => null),
+    cancelRun: vi.fn(async () => null),
+  }),
+  instanceSettingsService: () => ({
+    get: vi.fn(async () => ({
+      id: "instance-settings-1",
+      general: {
+        censorUsernameInLogs: false,
+        feedbackDataSharingPreference: "prompt",
+      },
+    })),
+    listCompanyIds: vi.fn(async () => ["company-1"]),
+  }),
+  issueApprovalService: () => ({}),
+  issueService: () => mockIssueService,
+  logActivity: mockLogActivity,
+  projectService: () => ({}),
+  routineService: () => ({
+    syncRunStatusForIssue: vi.fn(async () => undefined),
+  }),
+  workProductService: () => ({}),
+}));
+
+function createStorageService(): StorageService {
+  return {
+    provider: "local_disk",
+    putFile: vi.fn(async (input) => ({
+      provider: "local_disk",
+      objectKey: `${input.namespace}/${input.originalFilename ?? "upload"}`,
+      contentType: input.contentType,
+      byteSize: input.body.length,
+      sha256: "sha256-sample",
+      originalFilename: input.originalFilename,
+    })),
+    getObject: vi.fn(async () => ({
+      stream: Readable.from(Buffer.from("test")),
+      contentLength: 4,
+    })),
+    headObject: vi.fn(),
+    deleteObject: vi.fn(),
+  };
+}
+
+function createApp(storage: StorageService) {
+  const app = express();
+  app.use((req, _res, next) => {
+    (req as any).actor = {
+      type: "board",
+      userId: "local-board",
+      companyIds: ["company-1"],
+      source: "local_implicit",
+      isInstanceAdmin: false,
+    };
+    next();
+  });
+  app.use("/api", issueRoutes({} as any, storage));
+  app.use(errorHandler);
+  return app;
+}
+
+function makeAttachment(contentType: string, originalFilename: string) {
+  const now = new Date("2026-01-01T00:00:00.000Z");
+  return {
+    id: "attachment-1",
+    companyId: "company-1",
+    issueId: "11111111-1111-4111-8111-111111111111",
+    issueCommentId: null,
+    assetId: "asset-1",
+    provider: "local_disk",
+    objectKey: `issues/issue-1/${originalFilename}`,
+    contentType,
+    byteSize: 4,
+    sha256: "sha256-sample",
+    originalFilename,
+    createdByAgentId: null,
+    createdByUserId: "local-board",
+    createdAt: now,
+    updatedAt: now,
+  };
+}
+
+describe("issue attachment routes", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("accepts zip 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("application/zip", "bundle.zip"));
+
+    const res = await request(createApp(storage))
+      .post("/api/companies/company-1/issues/11111111-1111-4111-8111-111111111111/attachments")
+      .attach("file", Buffer.from("zip"), { filename: "bundle.zip", contentType: "application/zip" });
+
+    expect(res.status).toBe(201);
+    const putFileCall = vi.mocked(storage.putFile).mock.calls[0]?.[0];
+    expect(putFileCall).toMatchObject({
+      companyId: "company-1",
+      namespace: "issues/11111111-1111-4111-8111-111111111111",
+      originalFilename: "bundle.zip",
+      contentType: "application/zip",
+    });
+    expect(Buffer.isBuffer(putFileCall?.body)).toBe(true);
+    expect(mockIssueService.createAttachment).toHaveBeenCalledWith(
+      expect.objectContaining({
+        issueId: "11111111-1111-4111-8111-111111111111",
+        contentType: "application/zip",
+        originalFilename: "bundle.zip",
+      }),
+    );
+    expect(res.body.contentType).toBe("application/zip");
+  });
+
+  it("serves html attachments as downloads with nosniff", async () => {
+    const storage = createStorageService();
+    mockIssueService.getAttachmentById.mockResolvedValue(makeAttachment("text/html", "report.html"));
+
+    const res = await request(createApp(storage)).get("/api/attachments/attachment-1/content");
+
+    expect(res.status).toBe(200);
+    expect(res.headers["content-disposition"]).toBe('attachment; filename="report.html"');
+    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"));
+
+    const res = await request(createApp(storage)).get("/api/attachments/attachment-1/content");
+
+    expect(res.status).toBe(200);
+    expect(res.headers["content-disposition"]).toBe('inline; filename="preview.png"');
+  });
+});

server/src/attachment-types.ts

@@ -1,10 +1,10 @@
 /**
  * Shared attachment content-type configuration.
  *
- * By default only image types are allowed.  Set the
+ * By default a curated set of image/document/text 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.
+ * allowed set for routes that use this allowlist.
  *
  * Examples:
  *   PAPERCLIP_ALLOWED_ATTACHMENT_TYPES=image/*,application/pdf
@@ -29,6 +29,17 @@ export const DEFAULT_ALLOWED_TYPES: readonly string[] = [
   "text/html",
 ];
 
+export const DEFAULT_ATTACHMENT_CONTENT_TYPE = "application/octet-stream";
+export const SVG_CONTENT_TYPE = "image/svg+xml";
+export const INLINE_ATTACHMENT_TYPES: readonly string[] = [
+  "image/*",
+  "application/pdf",
+  "text/plain",
+  "text/markdown",
+  "application/json",
+  "text/csv",
+];
+
 /**
  * Parse a comma-separated list of MIME type patterns into a normalised array.
  * Returns the default image-only list when the input is empty or undefined.
@@ -59,6 +70,15 @@ export function matchesContentType(contentType: string, allowedPatterns: string[
   });
 }
 
+export function normalizeContentType(contentType: string | null | undefined): string {
+  const normalized = (contentType ?? "").trim().toLowerCase();
+  return normalized || DEFAULT_ATTACHMENT_CONTENT_TYPE;
+}
+
+export function isInlineAttachmentContentType(contentType: string): boolean {
+  return matchesContentType(contentType, [...INLINE_ATTACHMENT_TYPES]);
+}
+
 // ---------- Module-level singletons read once at startup ----------
 
 const allowedPatterns: string[] = parseAllowedTypes(

server/src/routes/issues.ts

@@ -47,7 +47,12 @@ import { logger } from "../middleware/logger.js";
 import { forbidden, HttpError, unauthorized } from "../errors.js";
 import { assertCompanyAccess, getActorInfo } from "./authz.js";
 import { shouldWakeAssigneeOnCheckout } from "./issues-checkout-wakeup.js";
-import { isAllowedContentType, MAX_ATTACHMENT_BYTES } from "../attachment-types.js";
+import {
+  isInlineAttachmentContentType,
+  MAX_ATTACHMENT_BYTES,
+  normalizeContentType,
+  SVG_CONTENT_TYPE,
+} from "../attachment-types.js";
 import { queueIssueAssignmentWakeup } from "../services/issue-assignment-wakeup.js";
 
 const MAX_ISSUE_COMMENT_LIMIT = 500;
@@ -2108,11 +2113,7 @@ export function issueRoutes(
       res.status(400).json({ error: "Missing file field 'file'" });
       return;
     }
-    const contentType = (file.mimetype || "").toLowerCase();
-    if (!isAllowedContentType(contentType)) {
-      res.status(422).json({ error: `Unsupported attachment type: ${contentType || "unknown"}` });
-      return;
-    }
+    const contentType = normalizeContentType(file.mimetype);
     if (file.buffer.length <= 0) {
       res.status(422).json({ error: "Attachment is empty" });
       return;
@@ -2176,11 +2177,17 @@ export function issueRoutes(
     assertCompanyAccess(req, attachment.companyId);
 
     const object = await storage.getObject(attachment.companyId, attachment.objectKey);
-    res.setHeader("Content-Type", attachment.contentType || object.contentType || "application/octet-stream");
+    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";
-    res.setHeader("Content-Disposition", `inline; filename=\"${filename.replaceAll("\"", "")}\"`);
+    const disposition = isInlineAttachmentContentType(responseContentType) ? "inline" : "attachment";
+    res.setHeader("Content-Disposition", `${disposition}; filename=\"${filename.replaceAll("\"", "")}\"`);
 
     object.stream.on("error", (err) => {
       next(err);

ui/src/pages/IssueDetail.tsx

@@ -1121,7 +1121,6 @@ export function IssueDetail() {
       <input
         ref={fileInputRef}
         type="file"
-        accept="image/*,application/pdf,text/plain,text/markdown,application/json,text/csv,text/html,.md,.markdown"
         className="hidden"
         onChange={handleFilePicked}
         multiple