fix: harden file download responses

lib/utils/create-file-download-response.ts

@@ -0,0 +1,52 @@
+import type { File } from "lib/db/schema"
+import {
+  decodeBase64ToUint8Array,
+  uint8ArrayToArrayBuffer,
+} from "lib/utils/decode-base64"
+
+const getDownloadFilename = (filePath: string) => {
+  return filePath.split("/").filter(Boolean).pop() || "download"
+}
+
+const getContentDisposition = (filePath: string) => {
+  const filename = getDownloadFilename(filePath)
+  const fallback = filename.replace(/["\\]/g, "_")
+
+  if (fallback === filename && /^[\x20-\x7e]+$/.test(filename)) {
+    return `attachment; filename="${filename}"`
+  }
+
+  return `attachment; filename="${fallback}"; filename*=UTF-8''${encodeURIComponent(
+    filename,
+  )}`
+}
+
+export const createFileDownloadResponse = (file: File) => {
+  const baseHeaders = {
+    "Content-Disposition": getContentDisposition(file.file_path),
+  }
+
+  if (file.binary_content_b64) {
+    const binaryBody = decodeBase64ToUint8Array(file.binary_content_b64)
+    const responseBody = uint8ArrayToArrayBuffer(binaryBody)
+
+    return new Response(responseBody, {
+      headers: {
+        ...baseHeaders,
+        "Content-Type": "application/octet-stream",
+        "Content-Length": binaryBody.byteLength.toString(),
+      },
+    })
+  }
+
+  const textBody = file.text_content ?? ""
+  const textLength = new TextEncoder().encode(textBody).byteLength
+
+  return new Response(textBody, {
+    headers: {
+      ...baseHeaders,
+      "Content-Type": "text/plain",
+      "Content-Length": textLength.toString(),
+    },
+  })
+}

routes/files/download.ts

@@ -1,10 +1,7 @@
 import { withRouteSpec } from "lib/middleware/with-winter-spec"
 import { z } from "zod"
-import {
-  decodeBase64ToUint8Array,
-  uint8ArrayToArrayBuffer,
-} from "lib/utils/decode-base64"
 import { resolveFileProxy } from "lib/utils/resolve-file-proxy"
+import { createFileDownloadResponse } from "lib/utils/create-file-download-response"
 
 export default withRouteSpec({
   methods: ["GET"],
@@ -27,27 +24,5 @@ export default withRouteSpec({
     return new Response("File not found", { status: 404 })
   }
 
-  const isText = file.text_content !== undefined
-  if (!isText && file.binary_content_b64) {
-    const binaryBody = decodeBase64ToUint8Array(file.binary_content_b64)
-    const responseBody = uint8ArrayToArrayBuffer(binaryBody)
-    return new Response(responseBody, {
-      headers: {
-        "Content-Type": "application/octet-stream",
-        "Content-Disposition": `attachment; filename="${file.file_path
-          .split("/")
-          .pop()}"`,
-        "Content-Length": binaryBody.byteLength.toString(),
-      },
-    })
-  }
-
-  return new Response(file.text_content!, {
-    headers: {
-      "Content-Type": "text/plain",
-      "Content-Disposition": `attachment; filename="${file.file_path
-        .split("/")
-        .pop()}"`,
-    },
-  })
+  return createFileDownloadResponse(file)
 })

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

@@ -1,10 +1,7 @@
 import { withRouteSpec } from "lib/middleware/with-winter-spec"
 import { z } from "zod"
-import {
-  decodeBase64ToUint8Array,
-  uint8ArrayToArrayBuffer,
-} from "lib/utils/decode-base64"
 import { resolveFileProxy } from "lib/utils/resolve-file-proxy"
+import { createFileDownloadResponse } from "lib/utils/create-file-download-response"
 
 export default withRouteSpec({
   methods: ["GET"],
@@ -28,27 +25,5 @@ export default withRouteSpec({
     return new Response("File not found", { status: 404 })
   }
 
-  const isText = file.text_content !== undefined
-  if (!isText && file.binary_content_b64) {
-    const binaryBody = decodeBase64ToUint8Array(file.binary_content_b64)
-    const responseBody = uint8ArrayToArrayBuffer(binaryBody)
-    return new Response(responseBody, {
-      headers: {
-        "Content-Type": "application/octet-stream",
-        "Content-Disposition": `attachment; filename="${file.file_path
-          .split("/")
-          .pop()}"`,
-        "Content-Length": binaryBody.byteLength.toString(),
-      },
-    })
-  }
-
-  return new Response(file.text_content!, {
-    headers: {
-      "Content-Type": "text/plain",
-      "Content-Disposition": `attachment; filename="${file.file_path
-        .split("/")
-        .pop()}"`,
-    },
-  })
+  return createFileDownloadResponse(file)
 })

tests/routes/files.test.ts

@@ -60,12 +60,25 @@ test("binary file operations", async () => {
   expect(downloadRes.headers.get("content-length")).toBe(
     buffer.length.toString(),
   )
+
+  await axios.post("/files/upsert", {
+    file_path: "/nested/bin.dat",
+    binary_content_b64: base64,
+  })
+  const pathDownloadRes = await axios.get("/files/download/nested/bin.dat", {
+    responseType: "arrayBuffer",
+  })
+  expect(pathDownloadRes.status).toBe(200)
+  expect(Buffer.from(pathDownloadRes.data)).toEqual(buffer)
+  expect(pathDownloadRes.headers.get("content-disposition")).toBe(
+    'attachment; filename="bin.dat"',
+  )
 })
 
 test("file download operations", async () => {
   const { axios } = await getTestServer()
 
-  await axios.post("/files/upsert", {
+  const createRes = await axios.post("/files/upsert", {
     file_path: "/download-test.txt",
     text_content: "Test download content",
   })
@@ -79,6 +92,29 @@ test("file download operations", async () => {
   expect(successRes.headers.get("content-disposition")).toBe(
     'attachment; filename="download-test.txt"',
   )
+  expect(successRes.headers.get("content-length")).toBe(
+    "Test download content".length.toString(),
+  )
+
+  const byIdRes = await axios.get("/files/download", {
+    params: { file_id: createRes.data.file.file_id },
+  })
+  expect(byIdRes.status).toBe(200)
+  expect(byIdRes.data).toBe("Test download content")
+  expect(byIdRes.headers.get("content-disposition")).toBe(
+    'attachment; filename="download-test.txt"',
+  )
+
+  await axios.post("/files/upsert", {
+    file_path: '/quote"name.txt',
+    text_content: "quoted filename",
+  })
+  const quotedNameRes = await axios.get("/files/download", {
+    params: { file_path: '/quote"name.txt' },
+  })
+  expect(quotedNameRes.headers.get("content-disposition")).toBe(
+    `attachment; filename="quote_name.txt"; filename*=UTF-8''quote%22name.txt`,
+  )
 
   expect(
     axios.get("/files/download", {