diff --git a/apps/server/src/workspace/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/WorkspaceFileSystem.test.ts index 34ddeaa872ad4..605d56f392546 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.test.ts @@ -25,7 +25,12 @@ import { vi } from "vite-plus/test"; vi.mock("node:fs/promises", async (importOriginal) => { const actual = await importOriginal(); - return { ...actual, realpath: vi.fn(actual.realpath), open: vi.fn(actual.open) }; + return { + ...actual, + realpath: vi.fn(actual.realpath), + open: vi.fn(actual.open), + writeFile: vi.fn(actual.writeFile), + }; }); const injectShortReads = Effect.fn("injectShortReads")(function* (target: string) { @@ -572,6 +577,78 @@ it.layer(layerTest, { excludeTestServices: true })("WorkspaceFileSystemLive", (i }); describe("writeFile", () => { + it.effect.each([ + { existing: false, failure: "write" }, + { existing: true, failure: "write" }, + { existing: false, failure: "rename" }, + { existing: true, failure: "rename" }, + { existing: true, failure: "cleanup" }, + { existing: true, failure: "directory" }, + ])( + "handles $failure failure without losing destination contents (existing: $existing)", + ({ existing, failure }) => + Effect.gen(function* () { + const service = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* makeTempDir; + const target = path.join(yield* fs.realPath(cwd), "report.md"); + const original = "original contents\n"; + if (existing) yield* writeTextFile(cwd, "report.md", original); + const native = yield* Effect.promise(() => + vi.importActual("node:fs/promises"), + ); + const cause = Object.assign(new Error("Save failed"), { + code: failure === "write" ? "EFBIG" : "EACCES", + }); + using _write = vi + .spyOn(NodeFSP, "writeFile") + .mockImplementation(async (file, data, options) => { + if (failure !== "write") return native.writeFile(file, data, options); + await native.writeFile(file, String(data).slice(0, 4), options); + throw cause; + }); + using _rename = vi.spyOn(NodeFSP, "rename").mockImplementation(async (...args) => { + if (failure === "rename") throw cause; + return native.rename(...args); + }); + using _cleanup = vi.spyOn(NodeFSP, "rm").mockImplementation(async (...args) => { + if (failure === "cleanup") throw cause; + return native.rm(...args); + }); + using _directory = vi.spyOn(NodeFSP, "mkdtemp").mockImplementation(async (...args) => { + if (failure === "directory") throw cause; + return native.mkdtemp(...args); + }); + + if (failure === "cleanup") { + expect( + yield* service.writeFile({ + cwd, + relativePath: "report.md", + contents: "replacement\n", + }), + ).toEqual({ relativePath: "report.md" }); + expect(yield* fs.readFileString(target)).toBe("replacement\n"); + return; + } + + const error = yield* service + .writeFile({ cwd, relativePath: "report.md", contents: "replacement contents\n" }) + .pipe(Effect.flip); + + expect(error).toMatchObject({ + _tag: "WorkspaceFileSystemOperationError", + operation: "write-file", + operationPath: target, + cause, + }); + if (existing) expect(yield* fs.readFileString(target)).toBe(original); + else expect(yield* fs.exists(target)).toBe(false); + expect(yield* fs.readDirectory(cwd)).toEqual(existing ? ["report.md"] : []); + }), + ); + it.effect("rejects writes outside a known workspace", () => Effect.gen(function* () { const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; @@ -638,6 +715,8 @@ it.layer(layerTest, { excludeTestServices: true })("WorkspaceFileSystemLive", (i const path = yield* Path.Path; const cwd = yield* makeTempDir; yield* writeTextFile(cwd, "src/main.ts", "export {};\n"); + const target = path.join(cwd, "src/main.ts"); + if (HostProcess.Platform.defaultValue() !== "win32") yield* fileSystem.chmod(target, 0o750); yield* workspaceFileSystem.writeFile({ cwd, @@ -648,6 +727,9 @@ it.layer(layerTest, { excludeTestServices: true })("WorkspaceFileSystemLive", (i expect(yield* fileSystem.readFileString(path.join(cwd, "src/main.ts"))).toBe( "export const edited = true;\n", ); + if (HostProcess.Platform.defaultValue() !== "win32") { + expect((yield* fileSystem.stat(target)).mode & 0o777).toBe(0o750); + } yield* writeTextFile(cwd, "report.txt", "ordinary file"); for (const relativePath of whitespaceFilePaths) { diff --git a/apps/server/src/workspace/WorkspaceFileSystem.ts b/apps/server/src/workspace/WorkspaceFileSystem.ts index 7f3fb67b15000..ac4801abbc613 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -602,16 +602,43 @@ export const make = Effect.gen(function* () { }), ), ); + const platform = yield* HostProcess.Platform; + let cleanupFailure: unknown; yield* Effect.tryPromise({ - // A symlink swapped in after the check is not followed. Windows lacks O_NOFOLLOW. - try: () => - NodeFSP.writeFile(realTargetPath, contents, { - flag: - NodeFS.constants.O_WRONLY | - NodeFS.constants.O_CREAT | - NodeFS.constants.O_TRUNC | - (NodeFS.constants.O_NOFOLLOW ?? 0), - }), + try: async () => { + // Check write access without truncating; a swapped-in symlink is not followed on POSIX. + const previous = await NodeFSP.open( + realTargetPath, + NodeFS.constants.O_WRONLY | (NodeFS.constants.O_NOFOLLOW ?? 0), + ).catch((error: unknown) => { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return undefined; + throw error; + }); + let mode: number | undefined; + if (previous) { + try { + mode = (await previous.stat()).mode & 0o777; + } finally { + await previous.close(); + } + } + const temporaryDirectory = await NodeFSP.mkdtemp( + path.join(path.dirname(realTargetPath), ".t3-save-"), + ); + try { + const temporaryPath = path.join(temporaryDirectory, "contents"); + await NodeFSP.writeFile(temporaryPath, contents, { flag: "wx", mode }); + if (mode !== undefined && platform !== "win32") await NodeFSP.chmod(temporaryPath, mode); + await NodeFSP.rename(temporaryPath, realTargetPath); + } finally { + // Cleanup must not report failure after the replacement has landed. + await NodeFSP.rm(temporaryDirectory, { recursive: true, force: true }).catch( + (cause: unknown) => { + cleanupFailure = cause; + }, + ); + } + }, catch: (cause) => new WorkspaceFileSystemOperationError({ workspaceRoot: input.cwd, @@ -621,7 +648,16 @@ export const make = Effect.gen(function* () { operation: "write-file", cause, }), - }); + }).pipe( + Effect.onExit(() => + cleanupFailure === undefined + ? Effect.void + : Effect.logWarning("Workspace save temporary cleanup failed", { + path: realTargetPath, + cause: cleanupFailure, + }), + ), + ); // Editor autosave overwrites existing files, which leaves the indexed paths unchanged, and // fff's watcher applies the new contents on its own. A forced rescan re-walks the whole // workspace, and the save would wait for it.