Harden download attachment filenames

lib/utils/content-disposition.ts

@@ -0,0 +1,6 @@
+export function getAttachmentContentDisposition(filePath: string): string {
+  const rawFileName = filePath.split(/[\\/]/).filter(Boolean).pop() || "file"
+  const safeFileName = rawFileName.replace(/["\r\n\\]/g, "_").trim() || "file"
+
+  return `attachment; filename="${safeFileName}"`
+}

lib/utils/resolve-file-proxy.ts

@@ -2,6 +2,7 @@ import type { FileProxy } from "../db/schema"
 import { readFile } from "node:fs/promises"
 import { join } from "node:path"
 import { normalizePath } from "./normalize-path"
+import { getAttachmentContentDisposition } from "./content-disposition"
 
 export async function resolveFileProxy(
   proxy: FileProxy,
@@ -36,7 +37,7 @@ async function resolveDiskProxy(
     return new Response(content, {
       headers: {
         "Content-Type": contentType,
-        "Content-Disposition": `attachment; filename="${fileName}"`,
+        "Content-Disposition": getAttachmentContentDisposition(fileName),
         "Content-Length": content.byteLength.toString(),
       },
     })
@@ -79,7 +80,10 @@ async function resolveHttpProxy(
       headers.set("Content-Length", contentLength)
     }
     const fileName = relativePath.split("/").pop() || "file"
-    headers.set("Content-Disposition", `attachment; filename="${fileName}"`)
+    headers.set(
+      "Content-Disposition",
+      getAttachmentContentDisposition(fileName),
+    )
 
     return new Response(response.body, {
       status: response.status,

routes/files/download.ts

@@ -5,6 +5,7 @@ import {
   uint8ArrayToArrayBuffer,
 } from "lib/utils/decode-base64"
 import { resolveFileProxy } from "lib/utils/resolve-file-proxy"
+import { getAttachmentContentDisposition } from "lib/utils/content-disposition"
 
 export default withRouteSpec({
   methods: ["GET"],
@@ -34,9 +35,7 @@ export default withRouteSpec({
     return new Response(responseBody, {
       headers: {
         "Content-Type": "application/octet-stream",
-        "Content-Disposition": `attachment; filename="${file.file_path
-          .split("/")
-          .pop()}"`,
+        "Content-Disposition": getAttachmentContentDisposition(file.file_path),
         "Content-Length": binaryBody.byteLength.toString(),
       },
     })
@@ -45,9 +44,7 @@ export default withRouteSpec({
   return new Response(file.text_content!, {
     headers: {
       "Content-Type": "text/plain",
-      "Content-Disposition": `attachment; filename="${file.file_path
-        .split("/")
-        .pop()}"`,
+      "Content-Disposition": getAttachmentContentDisposition(file.file_path),
     },
   })
 })

routes/files/download/[[...file_path]].ts

@@ -5,6 +5,7 @@ import {
   uint8ArrayToArrayBuffer,
 } from "lib/utils/decode-base64"
 import { resolveFileProxy } from "lib/utils/resolve-file-proxy"
+import { getAttachmentContentDisposition } from "lib/utils/content-disposition"
 
 export default withRouteSpec({
   methods: ["GET"],
@@ -35,9 +36,7 @@ export default withRouteSpec({
     return new Response(responseBody, {
       headers: {
         "Content-Type": "application/octet-stream",
-        "Content-Disposition": `attachment; filename="${file.file_path
-          .split("/")
-          .pop()}"`,
+        "Content-Disposition": getAttachmentContentDisposition(file.file_path),
         "Content-Length": binaryBody.byteLength.toString(),
       },
     })
@@ -46,9 +45,7 @@ export default withRouteSpec({
   return new Response(file.text_content!, {
     headers: {
       "Content-Type": "text/plain",
-      "Content-Disposition": `attachment; filename="${file.file_path
-        .split("/")
-        .pop()}"`,
+      "Content-Disposition": getAttachmentContentDisposition(file.file_path),
     },
   })
 })

tests/routes/file-proxy02.test.ts

@@ -91,6 +91,38 @@ test("disk proxy with query param download", async () => {
   }
 })
 
+test("disk proxy download sanitizes unsafe attachment filename", async () => {
+  const { axios } = await getTestServer()
+
+  const { mkdtemp, writeFile, rm } = await import("node:fs/promises")
+  const { tmpdir } = await import("node:os")
+  const { join } = await import("node:path")
+
+  const tempDir = await mkdtemp(join(tmpdir(), "file-proxy-header-test-"))
+
+  try {
+    await writeFile(join(tempDir, 'bad"name.txt'), "Sanitized proxy header")
+
+    await axios.post("/file_proxies/create", {
+      proxy_type: "disk",
+      disk_path: tempDir,
+      matching_pattern: "header-test/*",
+    })
+
+    const downloadRes = await axios.get(
+      '/files/download/header-test/bad"name.txt',
+    )
+
+    expect(downloadRes.status).toBe(200)
+    expect(downloadRes.data).toBe("Sanitized proxy header")
+    expect(downloadRes.headers.get("content-disposition")).toBe(
+      'attachment; filename="bad_name.txt"',
+    )
+  } finally {
+    await rm(tempDir, { recursive: true, force: true })
+  }
+})
+
 test("disk proxy binary file", async () => {
   const { axios } = await getTestServer()
 

tests/routes/files.test.ts

@@ -112,6 +112,26 @@ test("file download operations2", async () => {
   })
 })
 
+test("download content disposition sanitizes unsafe filenames", async () => {
+  const { axios } = await getTestServer()
+
+  const createRes = await axios.post("/files/upsert", {
+    file_path: '/quoted/bad"name\r\nx-injected: 1.txt',
+    text_content: "safe header",
+  })
+
+  const downloadRes = await axios.get("/files/download", {
+    params: { file_id: createRes.data.file.file_id },
+  })
+
+  expect(downloadRes.status).toBe(200)
+  expect(downloadRes.data).toBe("safe header")
+  expect(downloadRes.headers.get("content-disposition")).toBe(
+    'attachment; filename="bad_name__x-injected: 1.txt"',
+  )
+  expect(downloadRes.headers.get("x-injected")).toBeNull()
+})
+
 test("file delete operations", async () => {
   const { axios } = await getTestServer()