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

README.md

@@ -109,6 +109,21 @@ Response: {
 }
 ```
 
+#### Download File
+
+Download files directly with `Content-Disposition: attachment` headers. Supports both query parameter and path parameter URL styles:
+
+```http
+# Query parameter style (by file_path or file_id)
+GET /files/download?file_path=path/to/file.txt
+GET /files/download?file_id=1
+
+# Path parameter style
+GET /files/download/path/to/file.txt
+```
+
+Responses return the raw file content (`text/plain` for text files, or `application/octet-stream` for binary files) with proper attachment filename headers.
+
 #### List Files
 
 ```http

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,26 +29,22 @@ 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(),
       },
     })
   }
 
-  return new Response(file.text_content!, {
+  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,26 +30,22 @@ 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(),
       },
     })
   }
 
-  return new Response(file.text_content!, {
+  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,80 @@
+import { test, expect } from "bun:test"
+import { getTestServer } from "tests/fixtures/get-test-server"
+import { Buffer } from "node:buffer"
+
+test("download file with spaces in filename", async () => {
+  const { axios } = await getTestServer()
+
+  await axios.post("/files/upsert", {
+    file_path: "/sub folder/my test document.txt",
+    text_content: "Document content with spaces",
+  })
+
+  // Query parameter style
+  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")
+  const queryDisposition = queryRes.headers.get("content-disposition")
+  expect(queryDisposition).toBe('attachment; filename="my test document.txt"')
+
+  // Path parameter style
+  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")
+  const pathDisposition = pathRes.headers.get("content-disposition")
+  expect(pathDisposition).toBe('attachment; filename="my test document.txt"')
+})
+
+test("download empty binary file", 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 resByPath = await axios.get("/files/download", {
+    params: { file_path: "/empty.bin" },
+    responseType: "arrayBuffer",
+  })
+  expect(resByPath.status).toBe(200)
+  expect(resByPath.headers.get("content-type")).toBe("application/octet-stream")
+  expect(resByPath.headers.get("content-length")).toBe("0")
+  expect(Buffer.from(resByPath.data).byteLength).toBe(0)
+
+  const resById = await axios.get("/files/download", {
+    params: { file_id: fileId },
+    responseType: "arrayBuffer",
+  })
+  expect(resById.status).toBe(200)
+  expect(resById.headers.get("content-type")).toBe("application/octet-stream")
+  expect(resById.headers.get("content-length")).toBe("0")
+})
+
+test("download non-existent file returns 404", 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",
+    },
+  )
+})