From 32efd78f24901a7fa0611faad7d86a2a40b9f84a Mon Sep 17 00:00:00 2001 From: Ratanlavate Date: Sun, 11 Oct 2026 09:35:12 +0000 Subject: [PATCH] feat: harden download endpoints and document usage (#5) - Add formatDownloadContentDisposition util to sanitize quote/linebreak characters in the Content-Disposition filename - Serve 0-byte binary files (binary_content_b64: "") as application/octet-stream with Content-Length: 0 instead of falling back to text - Document the /files/download query and path parameter styles in README - Add tests covering filenames with spaces, empty binary files, 404s and Content-Disposition sanitization --- README.md | 21 ++++ lib/utils/content-disposition.ts | 5 + routes/files/download.ts | 11 +-- routes/files/download/[[...file_path]].ts | 11 +-- tests/routes/download-hardening.test.ts | 111 ++++++++++++++++++++++ 5 files changed, 145 insertions(+), 14 deletions(-) create mode 100644 lib/utils/content-disposition.ts create mode 100644 tests/routes/download-hardening.test.ts diff --git a/README.md b/README.md index 993d309..8def392 100644 --- a/README.md +++ b/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 diff --git a/lib/utils/content-disposition.ts b/lib/utils/content-disposition.ts new file mode 100644 index 0000000..e0e45db --- /dev/null +++ b/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}"` +} diff --git a/routes/files/download.ts b/routes/files/download.ts index 4bb47d3..93087af 100644 --- a/routes/files/download.ts +++ b/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), }, }) }) diff --git a/routes/files/download/[[...file_path]].ts b/routes/files/download/[[...file_path]].ts index 3e8fa02..5c7c1b8 100644 --- a/routes/files/download/[[...file_path]].ts +++ b/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), }, }) }) diff --git a/tests/routes/download-hardening.test.ts b/tests/routes/download-hardening.test.ts new file mode 100644 index 0000000..afa1ce1 --- /dev/null +++ b/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=""', + ) +})