fix(studio): claim upload filenames exclusively (#3786)

* fix(studio): claim upload filenames exclusively

* fix(studio): write uploads through exclusive descriptors

packages/studio-server/src/routes/files.pathSafety.test.ts

@@ -1,4 +1,4 @@
-import { afterEach, describe, expect, it, type TestContext } from "vitest";
+import { afterEach, describe, expect, it, vi, type TestContext } from "vitest";
 import { Hono } from "hono";
 import {
   existsSync,
@@ -17,6 +17,7 @@ import type { StudioApiAdapter } from "../types";
 
 const tempDirs: string[] = [];
 afterEach(() => {
+  vi.restoreAllMocks();
   for (const dir of tempDirs.splice(0)) rmSync(dir, { recursive: true, force: true });
 });
 
@@ -201,3 +202,61 @@ describe("file route containment", () => {
     );
   });
 });
+
+describe("upload collision races", () => {
+  function raceDuringRead(filename: string, collide: () => void) {
+    const file = new File(["new upload"], filename);
+    const read = file.arrayBuffer.bind(file);
+    vi.spyOn(file, "arrayBuffer").mockImplementation(async () => {
+      collide();
+      return read();
+    });
+    const form = new FormData();
+    form.append("files", file);
+    vi.spyOn(Request.prototype, "formData").mockResolvedValue(form);
+  }
+
+  it("preserves an upload that appears while the body is read", async () => {
+    const { app, project } = fixture();
+    raceDuringRead("upload.txt", () => writeFileSync(join(project, "upload.txt"), "other upload"));
+    const response = await upload(app);
+    expect(response.status).toBe(201);
+    expect(await response.json()).toMatchObject({ files: ["upload (2).txt"] });
+    expect(readFileSync(join(project, "upload.txt"), "utf8")).toBe("other upload");
+    expect(readFileSync(join(project, "upload (2).txt"), "utf8")).toBe("new upload");
+  });
+
+  it.each([".gitignore", "archive.tar.txt"])(
+    "retries suffix races without changing extension rules: %s",
+    async (name) => {
+      const { app, project } = fixture();
+      const dot = name.indexOf(".", name.startsWith(".") ? 1 : 0);
+      const base = dot > 0 ? name.slice(0, dot) : name;
+      const ext = dot > 0 ? name.slice(dot) : "";
+      writeFileSync(join(project, name), "original");
+      raceDuringRead(name, () => {
+        writeFileSync(join(project, `${base} (2)${ext}`), "second");
+        writeFileSync(join(project, `${base} (3)${ext}`), "third");
+      });
+      const response = await upload(app, "", name);
+      expect(response.status).toBe(201);
+      expect(await response.json()).toMatchObject({ files: [`${base} (4)${ext}`] });
+      expect(readFileSync(join(project, `${base} (2)${ext}`), "utf8")).toBe("second");
+      expect(readFileSync(join(project, `${base} (3)${ext}`), "utf8")).toBe("third");
+    },
+  );
+
+  it("does not write through a symlink planted during the upload read", async (context) => {
+    const { app, project, outside } = fixture();
+    // Establish symlink support before entering the mocked asynchronous read.
+    const probe = join(project, "probe-link");
+    linkOrSkip(context, join(outside, "secret.txt"), probe, "file");
+    rmSync(probe);
+    raceDuringRead("upload.txt", () =>
+      symlinkSync(join(outside, "secret.txt"), join(project, "upload.txt")),
+    );
+    const response = await upload(app);
+    expect(await response.json()).toMatchObject({ files: [] });
+    expect(readFileSync(join(outside, "secret.txt"), "utf8")).toBe("outside secret");
+  });
+});

packages/studio-server/src/routes/files.ts

@@ -2204,13 +2204,14 @@ async function processUploadedFiles(
     // Don't overwrite — append (2), (3), etc.
     let finalPath = destPath;
     let finalName = name;
+    // Handle dotfiles correctly: .gitignore → ext="", base=".gitignore"
+    const dotIdx = name.indexOf(".", name.startsWith(".") ? 1 : 0);
+    const ext = dotIdx > 0 ? name.slice(dotIdx) : "";
+    const base = dotIdx > 0 ? name.slice(0, dotIdx) : name;
+    const MAX_COPY_INDEX = 10000;
+    let n = 1;
     if (existsSync(finalPath)) {
-      // Handle dotfiles correctly: .gitignore → ext="", base=".gitignore"
-      const dotIdx = name.indexOf(".", name.startsWith(".") ? 1 : 0);
-      const ext = dotIdx > 0 ? name.slice(dotIdx) : "";
-      const base = dotIdx > 0 ? name.slice(0, dotIdx) : name;
-      let n = 2;
-      const MAX_COPY_INDEX = 10000;
+      n = 2;
       while (n < MAX_COPY_INDEX && existsSync(resolve(targetDir, `${base} (${n})${ext}`))) n++;
       if (n >= MAX_COPY_INDEX) {
         skipped.push(name);
@@ -2231,7 +2232,33 @@ async function processUploadedFiles(
       continue;
     }
 
-    writeFileSync(finalPath, buffer);
+    // Reading the upload yields: another request can claim the selected name.
+    // Only exclusive creation authorizes a write; retry collisions without
+    // following a newly planted link or overwriting another upload.
+    let written = false;
+    while (n < MAX_COPY_INDEX && isSafePath(projectDir, finalPath)) {
+      try {
+        const fd = openSync(finalPath, "wx");
+        try {
+          writeFileSync(fd, buffer);
+        } finally {
+          closeSync(fd);
+        }
+        written = true;
+        break;
+      } catch (error) {
+        if (!(error instanceof Error) || !("code" in error) || error.code !== "EEXIST") {
+          throw error;
+        }
+        n++;
+        finalName = `${base} (${n})${ext}`;
+        finalPath = resolve(targetDir, finalName);
+      }
+    }
+    if (!written) {
+      if (n >= MAX_COPY_INDEX) skipped.push(name);
+      continue;
+    }
     const relativePath = subDir ? join(subDir, finalName) : finalName;
     uploaded.push(relativePath);
     if (isAudioFile(finalName)) {