Require exact attachment filenames

Author: none23

DESIGN.md

@@ -206,9 +206,10 @@ Permanent retention is deliberately not the default. It would eventually exhaust
 
 ### Public attachment URLs
 
-- Each URL contains an identifier with at least 128 bits of cryptographic randomness.
+- Each URL contains a UUID v4 identifier with 122 bits of cryptographic randomness.
 - The service exposes no listing, search, or sequential identifier endpoint.
-- The filename portion of the URL is cosmetic and is not used as the storage key.
+- The filename portion is not used as the storage key, but it must exactly match the stored
+  attachment filename.
 - Possession of the URL grants access to the file.
 
 Opaque URLs are public capability URLs, not access control equivalent to a private GitHub repository. They are suitable for nonsensitive screenshots and build artifacts. Secrets, customer data, private source archives, and other confidential files must not be uploaded.

README.md

@@ -245,6 +245,19 @@ npm run dev
 - Public URLs are capability URLs, not private-repository authorization.
 - The service does not scan downloads for malware.
 
+### Attachment enumeration
+
+The private R2 bucket is not exposed for direct public access, and the Worker exposes no attachment
+list or search route. Each public attachment URL combines a cryptographically random UUID with the
+exact filename recorded at upload time; the filename is URL-encoded in the request path. Attachment
+`GET` and `HEAD` requests with an incorrect filename receive the same `404` response as a missing
+attachment, while missing or malformed filename paths also return `404`.
+
+These controls make blind enumeration impractical, but they are not access control. Filenames are
+often predictable, and anyone who obtains a complete attachment URL can access it until it expires
+or is deleted. Upload only nonsensitive files, and avoid exposing attachment URLs in logs or other
+unintended locations.
+
 ## Design and license
 
 See [DESIGN.md](DESIGN.md) for the original proposal and design rationale.

openapi.yaml

@@ -138,7 +138,7 @@ components:
       name: filename
       in: path
       required: true
-      description: Cosmetic display filename; the opaque ID selects the object.
+      description: Exact attachment filename, URL-encoded in the request path.
       schema: { type: string }
   schemas:
     Attachment:

src/attachment.ts

@@ -2,19 +2,39 @@ import { readObjectPrefix } from "./config";
 import { errorResponse } from "./http";
 
 const ATTACHMENT_PATH =
-  /^\/a\/([0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12})(?:\/[^/]*)?$/i;
+  /^\/a\/([0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12})\/([^/]+)$/i;
 
 const INLINE_TYPES = new Set(["image/gif", "image/jpeg", "image/png", "image/webp"]);
 
+export interface AttachmentPath {
+  filename: string;
+  id: string;
+}
+
 export function attachmentKey(id: string, env: Pick<Env, "OBJECT_PREFIX">): string {
   return `${readObjectPrefix(env)}${id}`;
 }
 
-export function parseAttachmentId(pathname: string): string | null {
-  return ATTACHMENT_PATH.exec(pathname)?.[1]?.toLowerCase() ?? null;
+export function parseAttachmentPath(pathname: string): AttachmentPath | null {
+  const match = ATTACHMENT_PATH.exec(pathname);
+  const id = match?.[1];
+  const encodedFilename = match?.[2];
+  if (id === undefined || encodedFilename === undefined) {
+    return null;
+  }
+
+  try {
+    return { filename: decodeURIComponent(encodedFilename), id: id.toLowerCase() };
+  } catch {
+    return null;
+  }
 }
 
-export async function serveAttachment(request: Request, env: Env, id: string): Promise<Response> {
+export async function serveAttachment(
+  request: Request,
+  env: Env,
+  attachment: AttachmentPath,
+): Promise<Response> {
   if (request.method === "OPTIONS") {
     return new Response(null, {
       status: 204,
@@ -26,7 +46,7 @@ export async function serveAttachment(request: Request, env: Env, id: string): P
     });
   }
 
-  const key = attachmentKey(id, env);
+  const key = attachmentKey(attachment.id, env);
   const object =
     request.method === "HEAD"
       ? await env.ATTACHMENTS.head(key)
@@ -36,6 +56,13 @@ export async function serveAttachment(request: Request, env: Env, id: string): P
     return errorResponse(404, "attachment_not_found", "The attachment does not exist.");
   }
 
+  if (object.customMetadata?.filename !== attachment.filename) {
+    if (hasBody(object)) {
+      await object.body.cancel();
+    }
+    return errorResponse(404, "attachment_not_found", "The attachment does not exist.");
+  }
+
   if (isExpired(object.customMetadata?.expiresAt)) {
     if (hasBody(object)) {
       await object.body.cancel();

src/index.ts

@@ -1,4 +1,4 @@
-import { parseAttachmentId, serveAttachment } from "./attachment";
+import { parseAttachmentPath, serveAttachment } from "./attachment";
 import { errorResponse, jsonResponse } from "./http";
 import { handleDelete, handleQuota, handleUpload } from "./upload";
 
@@ -25,12 +25,12 @@ export default {
         return await handleDelete(request, env, url.pathname);
       }
 
-      const attachmentId = parseAttachmentId(url.pathname);
+      const attachment = parseAttachmentPath(url.pathname);
       if (
-        attachmentId !== null &&
+        attachment !== null &&
         (request.method === "GET" || request.method === "HEAD" || request.method === "OPTIONS")
       ) {
-        return await serveAttachment(request, env, attachmentId);
+        return await serveAttachment(request, env, attachment);
       }
 
       return errorResponse(404, "not_found", "The requested endpoint does not exist.");

test/attachment.test.ts

@@ -24,7 +24,7 @@ describe("attachment delivery", () => {
       inline: "1",
     });
 
-    const response = await fetchWorker(`/a/${IMAGE_ID}/anything.png`);
+    const response = await fetchWorker(`/a/${IMAGE_ID}/screen%20shot.png`);
 
     expect(response.status).toBe(200);
     expect(response.headers.get("content-type")).toBe("image/png");
@@ -36,6 +36,23 @@ describe("attachment delivery", () => {
     expect(new Uint8Array(await response.arrayBuffer())).toEqual(IMAGE_BYTES);
   });
 
+  it("requires the exact decoded attachment filename", async () => {
+    const filename = "résumé #1%.png";
+    await seedObject(IMAGE_ID, IMAGE_BYTES, { filename, inline: "1" });
+
+    const matching = await fetchWorker(`/a/${IMAGE_ID}/${encodeURIComponent(filename)}`);
+    expect(matching.status).toBe(200);
+
+    const mismatched = await fetchWorker(`/a/${IMAGE_ID}/resume.png`);
+    expect(mismatched.status).toBe(404);
+    await expect(mismatched.json()).resolves.toMatchObject({
+      error: { code: "attachment_not_found" },
+    });
+
+    expect((await fetchWorker(`/a/${IMAGE_ID}`)).status).toBe(404);
+    expect((await fetchWorker(`/a/${IMAGE_ID}/bad%encoding.png`)).status).toBe(404);
+  });
+
   it("forces active content to download", async () => {
     const id = "36f38491-c389-4f50-a7de-2f77707ea088";
     await seedObject(id, new TextEncoder().encode("<h1>test</h1>"), {
@@ -79,6 +96,9 @@ describe("attachment delivery", () => {
     const expired = await fetchWorker(`/a/${expiredId}/expired.png`, { method: "HEAD" });
     expect(expired.status).toBe(410);
 
+    const disguisedExpired = await fetchWorker(`/a/${expiredId}/other.png`, { method: "HEAD" });
+    expect(disguisedExpired.status).toBe(404);
+
     const missing = await fetchWorker("/a/7928e125-8d61-45ec-a2d3-2a5f639d342e/missing.png");
     expect(missing.status).toBe(404);
     await expect(missing.json()).resolves.toMatchObject({