fix(server): atomic writes keep the file's permission bits

packages/shared/src/atomicWrite.test.ts

@@ -1,5 +1,10 @@
+// @effect-diagnostics nodeBuiltinImport:off - Verify observable Node filesystem contents and permissions.
+import * as NodeFS from "node:fs";
+import * as NodePath from "node:path";
+
 import * as NodeServices from "@effect/platform-node/NodeServices";
 import { assert, it } from "@effect/vitest";
+import * as HostProcess from "./HostProcess.ts";
 import * as Effect from "effect/Effect";
 import * as Exit from "effect/Exit";
 import * as FileSystem from "effect/FileSystem";
@@ -9,6 +14,12 @@ import * as PlatformError from "effect/PlatformError";
 
 import { writeFileStringAtomically } from "./atomicWrite.ts";
 
+const windowsHost = HostProcess.Platform.defaultValue() === "win32";
+const makeTestDirectory = Effect.gen(function* () {
+  const fs = yield* FileSystem.FileSystem;
+  return yield* fs.makeTempDirectoryScoped({ prefix: "t3-atomic-write-test-" });
+});
+
 it.layer(NodeServices.layer)("writeFileStringAtomically", (it) => {
   it.effect("keeps a symlinked file linked and rewrites its destination", () =>
     Effect.gen(function* () {
@@ -20,12 +31,18 @@ it.layer(NodeServices.layer)("writeFileStringAtomically", (it) => {
       yield* fs.makeDirectory(path.dirname(destination), { recursive: true });
       yield* fs.makeDirectory(path.dirname(link), { recursive: true });
       yield* fs.writeFileString(destination, "before");
+      if (!windowsHost) {
+        yield* fs.chmod(destination, 0o600);
+      }
       yield* fs.symlink(destination, link);
 
       yield* writeFileStringAtomically({ filePath: link, contents: "after" });
 
       assert.strictEqual(yield* fs.readLink(link), destination);
       assert.strictEqual(yield* fs.readFileString(destination), "after");
+      if (!windowsHost) {
+        assert.strictEqual((yield* fs.stat(destination)).mode & 0o777, 0o600);
+      }
     }),
   );
 
@@ -43,6 +60,9 @@ it.layer(NodeServices.layer)("writeFileStringAtomically", (it) => {
 
       assert.strictEqual(yield* fs.readLink(link), destination);
       assert.strictEqual(yield* fs.readFileString(destination), "fresh");
+      if (!windowsHost) {
+        assert.strictEqual((yield* fs.stat(destination)).mode & 0o777, 0o600);
+      }
     }),
   );
 
@@ -80,12 +100,18 @@ it.layer(NodeServices.layer)("writeFileStringAtomically", (it) => {
       yield* fs.makeDirectory(home, { recursive: true });
       yield* fs.symlink(linkedState, path.join(home, "state"));
       yield* fs.writeFileString(destination, "before");
+      if (!windowsHost) {
+        yield* fs.chmod(destination, 0o664);
+      }
       yield* fs.symlink("../config/settings.json", link);
 
       yield* writeFileStringAtomically({ filePath: link, contents: "after" });
 
       assert.strictEqual(yield* fs.readLink(link), "../config/settings.json");
       assert.strictEqual(yield* fs.readFileString(destination), "after");
+      if (!windowsHost) {
+        assert.strictEqual((yield* fs.stat(destination)).mode & 0o777, 0o664);
+      }
     }),
   );
 
@@ -99,6 +125,9 @@ it.layer(NodeServices.layer)("writeFileStringAtomically", (it) => {
       yield* writeFileStringAtomically({ filePath, contents: "fresh" });
 
       assert.strictEqual(yield* fs.readFileString(filePath), "fresh");
+      if (!windowsHost) {
+        assert.strictEqual((yield* fs.stat(filePath)).mode & 0o777, 0o600);
+      }
     }),
   );
 });
@@ -138,6 +167,94 @@ it.effect("surfaces an unreadable link instead of writing over it", () =>
   ),
 );
 
+it.layer(NodeServices.layer)("writeFileStringAtomically", (it) => {
+  it.effect.each([
+    { label: "0600", mode: 0o600 },
+    { label: "0644", mode: 0o644 },
+    { label: "0664", mode: 0o664 },
+  ])("preserves an existing $label file's mode", ({ mode }) =>
+    Effect.gen(function* () {
+      const dir = yield* makeTestDirectory;
+      const filePath = NodePath.join(dir, "settings.json");
+      NodeFS.writeFileSync(filePath, "old contents");
+      NodeFS.chmodSync(filePath, mode);
+
+      yield* writeFileStringAtomically({ filePath, contents: "new contents" });
+
+      if (!windowsHost) {
+        assert.equal(NodeFS.statSync(filePath).mode & 0o777, mode);
+      }
+      assert.equal(NodeFS.readFileSync(filePath, "utf8"), "new contents");
+    }),
+  );
+
+  it.effect.each(["settings.json", "nested/parent/settings.json"])(
+    "creates missing %s with owner-only permissions",
+    (filename) =>
+      Effect.gen(function* () {
+        const dir = yield* makeTestDirectory;
+        const filePath = NodePath.join(dir, filename);
+
+        yield* writeFileStringAtomically({ filePath, contents: "new contents" });
+
+        assert.equal(NodeFS.readFileSync(filePath, "utf8"), "new contents");
+        if (!windowsHost) {
+          assert.equal(NodeFS.statSync(filePath).mode & 0o777, 0o600);
+        }
+      }),
+  );
+
+  it.effect("preserves the exact UTF-8 bytes, including a trailing newline", () =>
+    Effect.gen(function* () {
+      const dir = yield* makeTestDirectory;
+      const filePath = NodePath.join(dir, "settings.json");
+      const contents = "Zażółć gęślą jaźń, 日本語, 🎵\n";
+
+      yield* writeFileStringAtomically({ filePath, contents });
+
+      assert.deepEqual(NodeFS.readFileSync(filePath), Buffer.from(contents, "utf8"));
+    }),
+  );
+
+  it.effect("removes the temporary directory after a successful write", () =>
+    Effect.gen(function* () {
+      const dir = yield* makeTestDirectory;
+      const filePath = NodePath.join(dir, "settings.json");
+
+      yield* writeFileStringAtomically({ filePath, contents: "new contents" });
+
+      assert.deepEqual(NodeFS.readdirSync(dir), ["settings.json"]);
+    }),
+  );
+
+  it.effect.skipIf(windowsHost || process.getuid?.() === 0)(
+    "leaves the original file intact and no temporary directory when the parent is read-only",
+    () =>
+      Effect.gen(function* () {
+        const dir = yield* makeTestDirectory;
+        const filePath = NodePath.join(dir, "settings.json");
+        NodeFS.writeFileSync(filePath, "old contents");
+        NodeFS.chmodSync(filePath, 0o600);
+        const parentMode = NodeFS.statSync(dir).mode & 0o777;
+        NodeFS.chmodSync(dir, 0o555);
+
+        try {
+          const error = yield* writeFileStringAtomically({
+            filePath,
+            contents: "new contents",
+          }).pipe(Effect.flip);
+
+          assert.instanceOf(error, PlatformError.PlatformError);
+          assert.equal(NodeFS.readFileSync(filePath, "utf8"), "old contents");
+          assert.equal(NodeFS.statSync(filePath).mode & 0o777, 0o600);
+          assert.deepEqual(NodeFS.readdirSync(dir), ["settings.json"]);
+        } finally {
+          NodeFS.chmodSync(dir, parentMode);
+        }
+      }),
+  );
+});
+
 it.effect("succeeds when the write lands but its temp directory cannot be removed", () =>
   Effect.gen(function* () {
     const renamed: Array<string> = [];
@@ -156,6 +273,7 @@ it.effect("succeeds when the write lands but its temp directory cannot be remove
       makeDirectory: () => Effect.void,
       makeTempDirectory: () => Effect.succeed("/home/settings.json.abc123"),
       writeFileString: () => Effect.void,
+      chmod: () => Effect.void,
       rename: (_from, to) => Effect.sync(() => void renamed.push(to)),
       remove: (path) =>
         Effect.sync(() => void removed.push(path)).pipe(

packages/shared/src/atomicWrite.ts

@@ -19,6 +19,13 @@ export const writeFileStringAtomically = (input: {
       const targetPath = yield* resolveSymlinkTarget(input.filePath);
       const targetDirectory = path.dirname(targetPath);
 
+      // The rename replaces the inode, so carry the target's permission bits over
+      // (these files can hold secrets); new files start owner-only.
+      const mode = yield* fs.stat(targetPath).pipe(
+        Effect.map((info) => info.mode & 0o777),
+        Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(0o600)),
+      );
+
       yield* fs.makeDirectory(targetDirectory, { recursive: true });
       // The temp directory is cleanup, not part of the write: failing to remove
       // it (a virus scanner holding it on Windows) must not fail a write that
@@ -33,6 +40,8 @@ export const writeFileStringAtomically = (input: {
       const tempPath = path.join(tempDirectory, "contents.tmp");
 
       yield* fs.writeFileString(tempPath, input.contents);
+      // chmod, not a write `mode`, because the umask would mask it; the temp directory is 0700.
+      yield* fs.chmod(tempPath, mode);
       yield* fs.rename(tempPath, targetPath);
     }),
   );