feat: harden download endpoints and document usage (#5)

README.md

@@ -124,6 +124,27 @@ Response: {
 }
 ```
 
+#### Download File
+
+Download a file by its `file_id` or `file_path` using query parameters, or by
+appending the file path directly to the URL. Responses are sent as an
+attachment with a `Content-Disposition` header, and binary files are served as
+`application/octet-stream`.
+
+```http
+# Query parameter style
+GET /files/download?file_id=1
+# or
+GET /files/download?file_path=path/to/file.txt
+
+# Path parameter style (catch-all)
+GET /files/download/path/to/file.txt
+```
+
+If no matching file is found, the server responds with `404 File not found`.
+When the requested path matches a configured file proxy, the download is
+resolved through that proxy instead.
+
 ### Event Operations
 
 #### Create Event

lib/utils/content-disposition.ts

@@ -0,0 +1,5 @@
+export const formatDownloadContentDisposition = (filePath: string): string => {
+  const rawFilename = filePath.split("/").pop() ?? "download"
+  const sanitizedAscii = rawFilename.replace(/[\r\n"]/g, "_")
+  return `attachment; filename="${sanitizedAscii}"`
+}

routes/files/download.ts

@@ -5,6 +5,7 @@ import {
   uint8ArrayToArrayBuffer,
 } from "lib/utils/decode-base64"
 import { resolveFileProxy } from "lib/utils/resolve-file-proxy"
+import { formatDownloadContentDisposition } from "lib/utils/content-disposition"
 
 export default withRouteSpec({
   methods: ["GET"],
@@ -28,15 +29,13 @@ export default withRouteSpec({
   }
 
   const isText = file.text_content !== undefined
-  if (!isText && file.binary_content_b64) {
+  if (!isText && file.binary_content_b64 !== undefined) {
     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-Disposition": formatDownloadContentDisposition(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": formatDownloadContentDisposition(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 { formatDownloadContentDisposition } from "lib/utils/content-disposition"
 
 export default withRouteSpec({
   methods: ["GET"],
@@ -29,15 +30,13 @@ export default withRouteSpec({
   }
 
   const isText = file.text_content !== undefined
-  if (!isText && file.binary_content_b64) {
+  if (!isText && file.binary_content_b64 !== undefined) {
     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-Disposition": formatDownloadContentDisposition(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": formatDownloadContentDisposition(file.file_path),
     },
   })
 })

tests/routes/download-hardening.test.ts

@@ -0,0 +1,111 @@
+import { test, expect } from "bun:test"
+import { Buffer } from "node:buffer"
+import { getTestServer } from "tests/fixtures/get-test-server"
+import { formatDownloadContentDisposition } from "lib/utils/content-disposition"
+
+test("download file with spaces in filename (query + path)", async () => {
+  const { axios } = await getTestServer()
+
+  await axios.post("/files/upsert", {
+    file_path: "/sub folder/my test document.txt",
+    text_content: "Document content with spaces",
+  })
+
+  const queryRes = await axios.get("/files/download", {
+    params: { file_path: "/sub folder/my test document.txt" },
+  })
+  expect(queryRes.status).toBe(200)
+  expect(queryRes.data).toBe("Document content with spaces")
+  expect(queryRes.headers.get("content-type")).toBe("text/plain")
+  expect(queryRes.headers.get("content-disposition")).toBe(
+    'attachment; filename="my test document.txt"',
+  )
+
+  const pathRes = await axios.get(
+    "/files/download/sub folder/my test document.txt",
+  )
+  expect(pathRes.status).toBe(200)
+  expect(pathRes.data).toBe("Document content with spaces")
+  expect(pathRes.headers.get("content-type")).toBe("text/plain")
+  expect(pathRes.headers.get("content-disposition")).toBe(
+    'attachment; filename="my test document.txt"',
+  )
+})
+
+test("download empty (0-byte) binary file as octet-stream", async () => {
+  const { axios } = await getTestServer()
+
+  const upsertRes = await axios.post("/files/upsert", {
+    file_path: "/empty.bin",
+    binary_content_b64: "",
+  })
+  expect(upsertRes.status).toBe(200)
+  const fileId = upsertRes.data.file.file_id
+
+  const byPath = await axios.get("/files/download", {
+    params: { file_path: "/empty.bin" },
+    responseType: "arrayBuffer",
+  })
+  expect(byPath.status).toBe(200)
+  expect(byPath.headers.get("content-type")).toBe("application/octet-stream")
+  expect(byPath.headers.get("content-length")).toBe("0")
+  expect(Buffer.from(byPath.data).byteLength).toBe(0)
+
+  const byId = await axios.get("/files/download", {
+    params: { file_id: fileId },
+    responseType: "arrayBuffer",
+  })
+  expect(byId.status).toBe(200)
+  expect(byId.headers.get("content-type")).toBe("application/octet-stream")
+  expect(byId.headers.get("content-length")).toBe("0")
+})
+
+test("download non-existent file returns 404 (query + path)", async () => {
+  const { axios } = await getTestServer()
+
+  expect(
+    axios.get("/files/download", {
+      params: { file_path: "/nonexistent.txt" },
+    }),
+  ).rejects.toMatchObject({
+    status: 404,
+    data: "File not found",
+  })
+
+  expect(axios.get("/files/download/does/not/exist.txt")).rejects.toMatchObject(
+    {
+      status: 404,
+      data: "File not found",
+    },
+  )
+})
+
+test("download filename with quotes is sanitized in Content-Disposition", async () => {
+  const { axios } = await getTestServer()
+
+  await axios.post("/files/upsert", {
+    file_path: '/evil";x="y.txt',
+    text_content: "quoted filename",
+  })
+
+  const res = await axios.get("/files/download", {
+    params: { file_path: '/evil";x="y.txt' },
+  })
+  expect(res.status).toBe(200)
+  expect(res.data).toBe("quoted filename")
+  expect(res.headers.get("content-disposition")).toBe(
+    'attachment; filename="evil_;x=_y.txt"',
+  )
+})
+
+test("formatDownloadContentDisposition sanitizes control characters", () => {
+  expect(formatDownloadContentDisposition("/a/b/file.txt")).toBe(
+    'attachment; filename="file.txt"',
+  )
+  expect(formatDownloadContentDisposition('/a/"weird"\nname.txt')).toBe(
+    'attachment; filename="_weird__name.txt"',
+  )
+  expect(formatDownloadContentDisposition("/a/b/")).toBe(
+    'attachment; filename=""',
+  )
+})