Add sanitization for SVG uploads and enhance security headers for asset responses

- Introduced SVG sanitization using `dompurify` to prevent malicious content. - Updated tests to validate SVG sanitization with various scenarios. - Enhanced response headers for assets, adding CSP and nosniff for SVGs. - Adjusted UI to better clarify supported file types for logo uploads. - Updated dependencies to include `jsdom` and `dompurify`.
2026-03-06 17:18:43 -05:00
parent 1448b55ca4
commit a4702e48f9
6 changed files with 569 additions and 32 deletions
--- a/server/package.json
+++ b/server/package.json
@@ -34,17 +34,19 @@
    "@paperclipai/adapter-claude-local": "workspace:*",
    "@paperclipai/adapter-codex-local": "workspace:*",
    "@paperclipai/adapter-cursor-local": "workspace:*",
-    "@paperclipai/adapter-opencode-local": "workspace:*",
    "@paperclipai/adapter-openclaw": "workspace:*",
+    "@paperclipai/adapter-opencode-local": "workspace:*",
    "@paperclipai/adapter-utils": "workspace:*",
    "@paperclipai/db": "workspace:*",
    "@paperclipai/shared": "workspace:*",
    "better-auth": "1.4.18",
    "detect-port": "^2.1.0",
+    "dompurify": "^3.3.2",
    "dotenv": "^17.0.1",
    "drizzle-orm": "^0.38.4",
    "embedded-postgres": "^18.1.0-beta.16",
    "express": "^5.1.0",
+    "jsdom": "^28.1.0",
    "multer": "^2.0.2",
    "open": "^11.0.0",
    "pino": "^9.6.0",
@@ -56,6 +58,7 @@
  "devDependencies": {
    "@types/express": "^5.0.0",
    "@types/express-serve-static-core": "^5.0.0",
+    "@types/jsdom": "^28.0.0",
    "@types/multer": "^2.0.0",
    "@types/node": "^24.6.0",
    "@types/supertest": "^6.0.2",
--- a/server/src/tests/assets.test.ts
+++ b/server/src/tests/assets.test.ts
@@ -25,10 +25,10 @@ function createAsset() {
    companyId: "company-1",
    provider: "local",
    objectKey: "assets/abc",
-    contentType: "image/svg+xml",
+    contentType: "image/png",
    byteSize: 40,
    sha256: "sha256-sample",
-    originalFilename: "logo.svg",
+    originalFilename: "logo.png",
    createdByAgentId: null,
    createdByUserId: "user-1",
    createdAt: now,
@@ -36,7 +36,7 @@ function createAsset() {
  };
 }

-function createStorageService(contentType = "image/svg+xml"): StorageService {
+function createStorageService(contentType = "image/png"): StorageService {
  const putFile: StorageService["putFile"] = vi.fn(async (input: {
    companyId: string;
    namespace: string;
@@ -84,29 +84,63 @@ describe("POST /api/companies/:companyId/assets/images", () => {
    logActivityMock.mockReset();
  });

-  it("accepts SVG image uploads and returns an asset path", async () => {
-    const svg = createStorageService("image/svg+xml");
-    const app = createApp(svg);
+  it("accepts PNG image uploads and returns an asset path", async () => {
+    const png = createStorageService("image/png");
+    const app = createApp(png);

    createAssetMock.mockResolvedValue(createAsset());

    const res = await request(app)
      .post("/api/companies/company-1/assets/images")
      .field("namespace", "companies")
-      .attach("file", Buffer.from("<svg xmlns='http://www.w3.org/2000/svg'></svg>"), "logo.svg");
+      .attach("file", Buffer.from("png"), "logo.png");

    expect(res.status).toBe(201);
    expect(res.body.contentPath).toBe("/api/assets/asset-1/content");
    expect(createAssetMock).toHaveBeenCalledTimes(1);
-    expect(svg.putFile).toHaveBeenCalledWith({
+    expect(png.putFile).toHaveBeenCalledWith({
      companyId: "company-1",
      namespace: "assets/companies",
-      originalFilename: "logo.svg",
-      contentType: "image/svg+xml",
+      originalFilename: "logo.png",
+      contentType: "image/png",
      body: expect.any(Buffer),
    });
  });

+  it("sanitizes SVG image uploads before storing them", async () => {
+    const svg = createStorageService("image/svg+xml");
+    const app = createApp(svg);
+
+    createAssetMock.mockResolvedValue({
+      ...createAsset(),
+      contentType: "image/svg+xml",
+      originalFilename: "logo.svg",
+    });
+
+    const res = await request(app)
+      .post("/api/companies/company-1/assets/images")
+      .field("namespace", "companies")
+      .attach(
+        "file",
+        Buffer.from(
+          "<svg xmlns='http://www.w3.org/2000/svg' onload='alert(1)'><script>alert(1)</script><a href='https://evil.example/'><circle cx='12' cy='12' r='10'/></a></svg>",
+        ),
+        "logo.svg",
+      );
+
+    expect(res.status).toBe(201);
+    expect(svg.putFile).toHaveBeenCalledTimes(1);
+    const stored = (svg.putFile as ReturnType<typeof vi.fn>).mock.calls[0]?.[0];
+    expect(stored.contentType).toBe("image/svg+xml");
+    expect(stored.originalFilename).toBe("logo.svg");
+    const body = stored.body.toString("utf8");
+    expect(body).toContain("<svg");
+    expect(body).toContain("<circle");
+    expect(body).not.toContain("<script");
+    expect(body).not.toContain("onload=");
+    expect(body).not.toContain("https://evil.example/");
+  });
+
  it("rejects files larger than 100 KB", async () => {
    const app = createApp(createStorageService());
    createAssetMock.mockResolvedValue(createAsset());
@@ -154,4 +188,18 @@ describe("POST /api/companies/:companyId/assets/images", () => {
    expect(res.body.error).toBe("Unsupported image type: text/plain");
    expect(createAssetMock).not.toHaveBeenCalled();
  });
+
+  it("rejects SVG image uploads that cannot be sanitized", async () => {
+    const app = createApp(createStorageService("image/svg+xml"));
+    createAssetMock.mockResolvedValue(createAsset());
+
+    const res = await request(app)
+      .post("/api/companies/company-1/assets/images")
+      .field("namespace", "companies")
+      .attach("file", Buffer.from("not actually svg"), "logo.svg");
+
+    expect(res.status).toBe(422);
+    expect(res.body.error).toBe("SVG could not be sanitized");
+    expect(createAssetMock).not.toHaveBeenCalled();
+  });
 });
--- a/server/src/routes/assets.ts
+++ b/server/src/routes/assets.ts
@@ -1,5 +1,7 @@
 import { Router, type Request, type Response } from "express";
 import multer from "multer";
+import createDOMPurify from "dompurify";
+import { JSDOM } from "jsdom";
 import type { Db } from "@paperclipai/db";
 import { createAssetImageMetadataSchema } from "@paperclipai/shared";
 import type { StorageService } from "../storage/types.js";
@@ -8,15 +10,80 @@ import { assertCompanyAccess, getActorInfo } from "./authz.js";

 const MAX_ASSET_IMAGE_BYTES = Number(process.env.PAPERCLIP_ATTACHMENT_MAX_BYTES) || 10 * 1024 * 1024;
 const MAX_COMPANY_LOGO_BYTES = 100 * 1024;
+const SVG_CONTENT_TYPE = "image/svg+xml";
 const ALLOWED_IMAGE_CONTENT_TYPES = new Set([
  "image/png",
  "image/jpeg",
  "image/jpg",
  "image/webp",
  "image/gif",
-  "image/svg+xml",
+  SVG_CONTENT_TYPE,
 ]);

+function sanitizeSvgBuffer(input: Buffer): Buffer | null {
+  const raw = input.toString("utf8").trim();
+  if (!raw) return null;
+
+  const baseDom = new JSDOM("");
+  const domPurify = createDOMPurify(
+    baseDom.window as unknown as Parameters<typeof createDOMPurify>[0],
+  );
+  domPurify.addHook("uponSanitizeAttribute", (_node, data) => {
+    const attrName = data.attrName.toLowerCase();
+    const attrValue = (data.attrValue ?? "").trim();
+
+    if (attrName.startsWith("on")) {
+      data.keepAttr = false;
+      return;
+    }
+
+    if ((attrName === "href" || attrName === "xlink:href") && attrValue && !attrValue.startsWith("#")) {
+      data.keepAttr = false;
+    }
+  });
+
+  let parsedDom: JSDOM | null = null;
+  try {
+    const sanitized = domPurify.sanitize(raw, {
+      USE_PROFILES: { svg: true, svgFilters: true, html: false },
+      FORBID_TAGS: ["script", "foreignObject"],
+      FORBID_CONTENTS: ["script", "foreignObject"],
+      RETURN_TRUSTED_TYPE: false,
+    });
+
+    parsedDom = new JSDOM(sanitized, { contentType: SVG_CONTENT_TYPE });
+    const document = parsedDom.window.document;
+    const root = document.documentElement;
+    if (!root || root.tagName.toLowerCase() !== "svg") return null;
+
+    for (const el of Array.from(root.querySelectorAll("script, foreignObject"))) {
+      el.remove();
+    }
+    for (const el of Array.from(root.querySelectorAll("*"))) {
+      for (const attr of Array.from(el.attributes)) {
+        const attrName = attr.name.toLowerCase();
+        const attrValue = attr.value.trim();
+        if (attrName.startsWith("on")) {
+          el.removeAttribute(attr.name);
+          continue;
+        }
+        if ((attrName === "href" || attrName === "xlink:href") && attrValue && !attrValue.startsWith("#")) {
+          el.removeAttribute(attr.name);
+        }
+      }
+    }
+
+    const output = root.outerHTML.trim();
+    if (!output || !/^<svg[\s>]/i.test(output)) return null;
+    return Buffer.from(output, "utf8");
+  } catch {
+    return null;
+  } finally {
+    parsedDom?.window.close();
+    baseDom.window.close();
+  }
+}
+
 export function assetRoutes(db: Db, storage: StorageService) {
  const router = Router();
  const svc = assetService(db);
@@ -58,12 +125,21 @@ export function assetRoutes(db: Db, storage: StorageService) {
      return;
    }

-    const contentType = (file.mimetype || "").toLowerCase();
+    let contentType = (file.mimetype || "").toLowerCase();
    if (!ALLOWED_IMAGE_CONTENT_TYPES.has(contentType)) {
      res.status(422).json({ error: `Unsupported image type: ${contentType || "unknown"}` });
      return;
    }
-    if (file.buffer.length <= 0) {
+    let fileBody = file.buffer;
+    if (contentType === SVG_CONTENT_TYPE) {
+      const sanitized = sanitizeSvgBuffer(file.buffer);
+      if (!sanitized || sanitized.length <= 0) {
+        res.status(422).json({ error: "SVG could not be sanitized" });
+        return;
+      }
+      fileBody = sanitized;
+    }
+    if (fileBody.length <= 0) {
      res.status(422).json({ error: "Image is empty" });
      return;
    }
@@ -76,7 +152,7 @@ export function assetRoutes(db: Db, storage: StorageService) {

    const namespaceSuffix = parsedMeta.data.namespace ?? "general";
    const isCompanyLogoNamespace = namespaceSuffix === "companies" || namespaceSuffix.startsWith("companies/");
-    if (isCompanyLogoNamespace && file.buffer.length > MAX_COMPANY_LOGO_BYTES) {
+    if (isCompanyLogoNamespace && fileBody.length > MAX_COMPANY_LOGO_BYTES) {
      res.status(422).json({ error: `Image exceeds ${MAX_COMPANY_LOGO_BYTES} bytes` });
      return;
    }
@@ -87,7 +163,7 @@ export function assetRoutes(db: Db, storage: StorageService) {
      namespace: `assets/${namespaceSuffix}`,
      originalFilename: file.originalname || null,
      contentType,
-      body: file.buffer,
+      body: fileBody,
    });

    const asset = await svc.create(companyId, {
@@ -144,9 +220,14 @@ export function assetRoutes(db: Db, storage: StorageService) {
    assertCompanyAccess(req, asset.companyId);

    const object = await storage.getObject(asset.companyId, asset.objectKey);
-    res.setHeader("Content-Type", asset.contentType || object.contentType || "application/octet-stream");
+    const responseContentType = asset.contentType || object.contentType || "application/octet-stream";
+    res.setHeader("Content-Type", responseContentType);
    res.setHeader("Content-Length", String(asset.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 = asset.originalFilename ?? "asset";
    res.setHeader("Content-Disposition", `inline; filename=\"${filename.replaceAll("\"", "")}\"`);