fix(exec-approvals): guard Windows rename fallback (#77907)

* fix exec approvals Windows rename fallback

* fix(exec-approvals): restore approvals directory mode

* fix(exec-approvals): normalize fallback temp mode

---------

Co-authored-by: Brad Groux <3053586+BradGroux@users.noreply.github.com>

CHANGELOG.md

@@ -104,6 +104,7 @@ Docs: https://docs.openclaw.ai
 - Control UI/Sessions: make the compaction count a compact `N Checkpoint(s)` disclosure and show expanded session-level details with modern checkpoint history cards across responsive table layouts. Thanks @BunsDev.
 - Control UI/performance: keep chat and channel tabs responsive while history payloads and channel probes are slow, label partial channel status, and record slow chat/config render timings in the event log. Thanks @BunsDev.
 - Control UI/sessions: fire the documented `/new` command and lifecycle hooks only for explicit Control UI session creation, restoring session-memory and custom hook capture without changing SDK parent-session creates. Fixes #76957. Thanks @BunsDev.
+- Exec approvals: fall back to a guarded copy when Windows rejects rename-overwrite for `exec-approvals.json`, while preserving symlink, hard-link, and owner-only permission safeguards. Fixes #77785. (#77907) Thanks @Alex-Alaniz and @MilleniumGenAI.
 - Slack: preserve Socket Mode SDK error context and structured Slack API fields in reconnect logs, so startup failures no longer collapse to a bare `unknown error`.
 - iOS pairing: allow setup-code and manual `ws://` connects for private LAN and `.local` gateways while keeping Tailscale/public routes on `wss://`, and prefer explicit gateway passwords over stale bootstrap tokens in mixed-auth reconnects. Fixes #47887; carries forward #65185. Thanks @draix and @BunsDev.
 - Plugins/diagnostics: make source-only TypeScript package warnings actionable by explaining that missing compiled runtime output is a publisher packaging issue and pointing users to update/reinstall or disable/uninstall the plugin. Fixes #77835. Thanks @googlerest.

src/infra/exec-approvals-store.test.ts

@@ -79,6 +79,14 @@ function readApprovalsFile(homeDir: string): ExecApprovalsFile {
   return JSON.parse(fs.readFileSync(approvalsFilePath(homeDir), "utf8")) as ExecApprovalsFile;
 }
 
+function listExecApprovalTempFiles(homeDir: string): string[] {
+  const dir = path.dirname(approvalsFilePath(homeDir));
+  if (!fs.existsSync(dir)) {
+    return [];
+  }
+  return fs.readdirSync(dir).filter((name) => name.endsWith(".tmp"));
+}
+
 describe("exec approvals store helpers", () => {
   it("expands home-prefixed default file and socket paths", () => {
     const dir = createHomeDir();
@@ -173,6 +181,189 @@ describe("exec approvals store helpers", () => {
     expect(fs.statSync(approvalsPath).ino).not.toBe(fs.statSync(linkedPath).ino);
   });
 
+  it("normalizes successful rename writes to owner-only permissions", () => {
+    const dir = createHomeDir();
+    const actualWriteFileSync = fs.writeFileSync.bind(fs);
+    vi.spyOn(fs, "writeFileSync").mockImplementation((file, data, options) => {
+      const result = actualWriteFileSync(file, data, options as never);
+      const filePath = String(file);
+      if (
+        typeof file !== "number" &&
+        filePath.includes(".exec-approvals.") &&
+        filePath.endsWith(".tmp")
+      ) {
+        fs.chmodSync(file, 0o000);
+      }
+      return result;
+    });
+
+    saveExecApprovals({ version: 1, defaults: { security: "full" }, agents: {} });
+
+    expect(fs.readFileSync(approvalsFilePath(dir), "utf8")).toContain('"security": "full"');
+    expect(fs.statSync(approvalsFilePath(dir)).mode & 0o777).toBe(0o600);
+  });
+
+  it("normalizes the approvals directory to owner-only permissions", () => {
+    const dir = createHomeDir();
+    const approvalsDir = path.dirname(approvalsFilePath(dir));
+    fs.mkdirSync(approvalsDir, { recursive: true });
+    fs.chmodSync(approvalsDir, 0o777);
+
+    saveExecApprovals({ version: 1, defaults: { security: "full" }, agents: {} });
+
+    expect(fs.readFileSync(approvalsFilePath(dir), "utf8")).toContain('"security": "full"');
+    expect(fs.statSync(approvalsDir).mode & 0o777).toBe(0o700);
+  });
+
+  it("falls back to copying when rename cannot overwrite the approvals file", () => {
+    const dir = createHomeDir();
+    const approvalsPath = approvalsFilePath(dir);
+    fs.mkdirSync(path.dirname(approvalsPath), { recursive: true });
+    fs.writeFileSync(approvalsPath, '{"version":1,"agents":{}}\n', "utf8");
+    const actualRenameSync = fs.renameSync.bind(fs);
+    const rename = vi.spyOn(fs, "renameSync").mockImplementation((from, to) => {
+      if (String(to) === approvalsPath) {
+        const error = Object.assign(new Error("locked target"), { code: "EPERM" });
+        throw error;
+      }
+      return actualRenameSync(from, to);
+    });
+
+    saveExecApprovals({ version: 1, defaults: { security: "full" }, agents: {} });
+
+    expect(rename).toHaveBeenCalled();
+    expect(fs.readFileSync(approvalsPath, "utf8")).toContain('"security": "full"');
+    expect(fs.statSync(approvalsPath).mode & 0o777).toBe(0o600);
+    expect(listExecApprovalTempFiles(dir)).toEqual([]);
+  });
+
+  it("normalizes fallback temp files before copying", () => {
+    const dir = createHomeDir();
+    const approvalsPath = approvalsFilePath(dir);
+    fs.mkdirSync(path.dirname(approvalsPath), { recursive: true });
+    fs.writeFileSync(approvalsPath, '{"version":1,"agents":{}}\n', "utf8");
+    const actualWriteFileSync = fs.writeFileSync.bind(fs);
+    vi.spyOn(fs, "writeFileSync").mockImplementation((file, data, options) => {
+      const result = actualWriteFileSync(file, data, options as never);
+      const filePath = String(file);
+      if (
+        typeof file !== "number" &&
+        filePath.includes(".exec-approvals.") &&
+        filePath.endsWith(".tmp")
+      ) {
+        fs.chmodSync(file, 0o000);
+      }
+      return result;
+    });
+    const actualRenameSync = fs.renameSync.bind(fs);
+    vi.spyOn(fs, "renameSync").mockImplementation((from, to) => {
+      if (String(to) === approvalsPath) {
+        const error = Object.assign(new Error("locked target"), { code: "EPERM" });
+        throw error;
+      }
+      return actualRenameSync(from, to);
+    });
+
+    saveExecApprovals({ version: 1, defaults: { security: "full" }, agents: {} });
+
+    expect(fs.readFileSync(approvalsPath, "utf8")).toContain('"security": "full"');
+    expect(fs.statSync(approvalsPath).mode & 0o777).toBe(0o600);
+    expect(listExecApprovalTempFiles(dir)).toEqual([]);
+  });
+
+  it("restores the previous approvals file when fallback copy fails", () => {
+    const dir = createHomeDir();
+    const approvalsPath = approvalsFilePath(dir);
+    const previousRaw = '{"version":1,"defaults":{"security":"deny"},"agents":{}}\n';
+    fs.mkdirSync(path.dirname(approvalsPath), { recursive: true });
+    fs.writeFileSync(approvalsPath, previousRaw, { encoding: "utf8", mode: 0o600 });
+    const actualRenameSync = fs.renameSync.bind(fs);
+    vi.spyOn(fs, "renameSync").mockImplementation((from, to) => {
+      if (String(to) === approvalsPath) {
+        const error = Object.assign(new Error("locked target"), { code: "EPERM" });
+        throw error;
+      }
+      return actualRenameSync(from, to);
+    });
+    const actualFtruncateSync = fs.ftruncateSync.bind(fs);
+    let forcedFallbackFailure = false;
+    vi.spyOn(fs, "ftruncateSync").mockImplementation((fd, len) => {
+      if (!forcedFallbackFailure && len === 0) {
+        forcedFallbackFailure = true;
+        actualFtruncateSync(fd, len);
+        const error = Object.assign(new Error("copy failed after opening destination"), {
+          code: "ENOSPC",
+        });
+        throw error;
+      }
+      return actualFtruncateSync(fd, len);
+    });
+
+    expect(() =>
+      saveExecApprovals({ version: 1, defaults: { security: "full" }, agents: {} }),
+    ).toThrow(/copy failed after opening destination/);
+    expect(fs.readFileSync(approvalsPath, "utf8")).toBe(previousRaw);
+    expect(fs.statSync(approvalsPath).mode & 0o777).toBe(0o600);
+    expect(listExecApprovalTempFiles(dir)).toEqual([]);
+  });
+
+  it("does not follow a symlink swapped in before fallback copy", () => {
+    const dir = createHomeDir();
+    const approvalsPath = approvalsFilePath(dir);
+    const targetPath = path.join(dir, "elsewhere.json");
+    fs.mkdirSync(path.dirname(approvalsPath), { recursive: true });
+    fs.writeFileSync(approvalsPath, '{"version":1,"agents":{}}\n', "utf8");
+    fs.writeFileSync(targetPath, '{"sentinel":true}\n', "utf8");
+    const actualRenameSync = fs.renameSync.bind(fs);
+    vi.spyOn(fs, "renameSync").mockImplementation((from, to) => {
+      if (String(to) === approvalsPath) {
+        const error = Object.assign(new Error("locked target"), { code: "EPERM" });
+        throw error;
+      }
+      return actualRenameSync(from, to);
+    });
+    const actualStatSync = fs.statSync.bind(fs);
+    let swappedDestination = false;
+    vi.spyOn(fs, "statSync").mockImplementation((file, options) => {
+      const result = actualStatSync(file, options as never);
+      if (!swappedDestination && String(file) === approvalsPath) {
+        swappedDestination = true;
+        fs.rmSync(approvalsPath);
+        fs.symlinkSync(targetPath, approvalsPath);
+      }
+      return result;
+    });
+
+    expect(() =>
+      saveExecApprovals({ version: 1, defaults: { security: "full" }, agents: {} }),
+    ).toThrow(/symlink|ELOOP/);
+    expect(fs.readFileSync(targetPath, "utf8")).toBe('{"sentinel":true}\n');
+    expect(listExecApprovalTempFiles(dir)).toEqual([]);
+  });
+
+  it("does not use the copy fallback for hard-linked approvals files", () => {
+    const dir = createHomeDir();
+    const approvalsPath = approvalsFilePath(dir);
+    const linkedPath = path.join(dir, "linked.json");
+    fs.mkdirSync(path.dirname(approvalsPath), { recursive: true });
+    fs.writeFileSync(linkedPath, '{"sentinel":true}\n', "utf8");
+    fs.linkSync(linkedPath, approvalsPath);
+    const actualRenameSync = fs.renameSync.bind(fs);
+    vi.spyOn(fs, "renameSync").mockImplementation((from, to) => {
+      if (String(to) === approvalsPath) {
+        const error = Object.assign(new Error("locked target"), { code: "EPERM" });
+        throw error;
+      }
+      return actualRenameSync(from, to);
+    });
+
+    expect(() =>
+      saveExecApprovals({ version: 1, defaults: { security: "full" }, agents: {} }),
+    ).toThrow(/hard-linked exec approvals file/);
+    expect(fs.readFileSync(linkedPath, "utf8")).toBe('{"sentinel":true}\n');
+    expect(listExecApprovalTempFiles(dir)).toEqual([]);
+  });
+
   it("refuses to write approvals through a symlink destination", () => {
     const dir = createHomeDir();
     const approvalsPath = approvalsFilePath(dir);

src/infra/exec-approvals.ts

@@ -15,7 +15,6 @@ import type { ExecAllowlistEntry } from "./exec-approvals.types.js";
 import { assertNoSymlinkParentsSync } from "./fs-safe-advanced.js";
 import { expandHomePrefix, resolveRequiredHomeDir } from "./home-dir.js";
 import { requestJsonlSocket } from "./jsonl-socket.js";
-import { privateFileStoreSync } from "./private-file-store.js";
 export * from "./exec-approvals-analysis.js";
 export * from "./exec-approvals-allowlist.js";
 export type { ExecAllowlistEntry } from "./exec-approvals.types.js";
@@ -266,6 +265,13 @@ function ensureDir(filePath: string) {
   if (!dirStat.isDirectory() || dirStat.isSymbolicLink()) {
     throw new Error(`Refusing to use unsafe exec approvals directory: ${dir}`);
   }
+  try {
+    fs.chmodSync(dir, 0o700);
+  } catch (err) {
+    if (process.platform !== "win32") {
+      throw err;
+    }
+  }
   return dir;
 }
 
@@ -291,6 +297,201 @@ function assertSafeExecApprovalsDestination(filePath: string): void {
   }
 }
 
+function assertSafeExecApprovalsOverwriteFallback(filePath: string): void {
+  assertSafeExecApprovalsDestination(filePath);
+  try {
+    const stat = fs.statSync(filePath);
+    if (stat.nlink > 1) {
+      throw new Error(`Refusing copy fallback for hard-linked exec approvals file: ${filePath}`);
+    }
+  } catch (err) {
+    if ((err as NodeJS.ErrnoException).code !== "ENOENT") {
+      throw err;
+    }
+  }
+}
+
+type ExecApprovalsFallbackDestination = {
+  existed: boolean;
+  fd: number;
+  snapshot: Buffer | null;
+};
+
+function sameFilesystemEntry(left: fs.Stats, right: fs.Stats): boolean {
+  return left.dev === right.dev && left.ino === right.ino;
+}
+
+function readExecApprovalsFallbackSnapshotFromFd(fd: number): Buffer {
+  const chunks: Buffer[] = [];
+  const buffer = Buffer.alloc(64 * 1024);
+  let position = 0;
+  while (true) {
+    const bytesRead = fs.readSync(fd, buffer, 0, buffer.length, position);
+    if (bytesRead === 0) {
+      break;
+    }
+    chunks.push(Buffer.from(buffer.subarray(0, bytesRead)));
+    position += bytesRead;
+  }
+  return Buffer.concat(chunks);
+}
+
+function validateExecApprovalsFallbackFd(filePath: string, fd: number): fs.Stats {
+  const linkStat = fs.lstatSync(filePath);
+  if (linkStat.isSymbolicLink()) {
+    throw new Error(`Refusing to write exec approvals via symlink: ${filePath}`);
+  }
+  const pathStat = fs.statSync(filePath);
+  const fdStat = fs.fstatSync(fd);
+  if (!fdStat.isFile()) {
+    throw new Error(`Refusing copy fallback for non-file exec approvals path: ${filePath}`);
+  }
+  if (fdStat.nlink > 1) {
+    throw new Error(`Refusing copy fallback for hard-linked exec approvals file: ${filePath}`);
+  }
+  if (!sameFilesystemEntry(pathStat, fdStat)) {
+    throw new Error(`Refusing copy fallback after exec approvals path changed: ${filePath}`);
+  }
+  return fdStat;
+}
+
+function openExistingExecApprovalsFallbackDestination(
+  filePath: string,
+): ExecApprovalsFallbackDestination {
+  const noFollowFlag = fs.constants.O_NOFOLLOW ?? 0;
+  const fd = fs.openSync(filePath, fs.constants.O_RDWR | noFollowFlag, 0o600);
+  try {
+    validateExecApprovalsFallbackFd(filePath, fd);
+    return {
+      existed: true,
+      fd,
+      snapshot: readExecApprovalsFallbackSnapshotFromFd(fd),
+    };
+  } catch (err) {
+    try {
+      fs.closeSync(fd);
+    } catch {
+      // best-effort after validation failure
+    }
+    throw err;
+  }
+}
+
+function createExecApprovalsFallbackDestination(
+  filePath: string,
+): ExecApprovalsFallbackDestination {
+  const noFollowFlag = fs.constants.O_NOFOLLOW ?? 0;
+  try {
+    const fd = fs.openSync(
+      filePath,
+      fs.constants.O_RDWR | fs.constants.O_CREAT | fs.constants.O_EXCL | noFollowFlag,
+      0o600,
+    );
+    try {
+      validateExecApprovalsFallbackFd(filePath, fd);
+      return { existed: false, fd, snapshot: null };
+    } catch (err) {
+      try {
+        fs.closeSync(fd);
+      } catch {
+        // best-effort after validation failure
+      }
+      throw err;
+    }
+  } catch (err) {
+    if ((err as NodeJS.ErrnoException).code === "EEXIST") {
+      return openExistingExecApprovalsFallbackDestination(filePath);
+    }
+    throw err;
+  }
+}
+
+function openExecApprovalsFallbackDestination(filePath: string): ExecApprovalsFallbackDestination {
+  try {
+    return openExistingExecApprovalsFallbackDestination(filePath);
+  } catch (err) {
+    if ((err as NodeJS.ErrnoException).code === "ENOENT") {
+      return createExecApprovalsFallbackDestination(filePath);
+    }
+    throw err;
+  }
+}
+
+function writeExecApprovalsFallbackBuffer(fd: number, contents: Buffer): void {
+  fs.ftruncateSync(fd, 0);
+  let written = 0;
+  while (written < contents.length) {
+    written += fs.writeSync(fd, contents, written, contents.length - written, written);
+  }
+  fs.ftruncateSync(fd, contents.length);
+  try {
+    fs.fchmodSync(fd, 0o600);
+  } catch {
+    // best-effort on platforms without chmod
+  }
+}
+
+function restoreExecApprovalsFallbackDestination(
+  filePath: string,
+  destination: ExecApprovalsFallbackDestination,
+): void {
+  if (!destination.existed) {
+    try {
+      const pathStat = fs.statSync(filePath);
+      const fdStat = fs.fstatSync(destination.fd);
+      if (sameFilesystemEntry(pathStat, fdStat)) {
+        fs.rmSync(filePath, { force: true });
+      }
+    } catch (err) {
+      if ((err as NodeJS.ErrnoException).code !== "ENOENT") {
+        throw err;
+      }
+    }
+    return;
+  }
+  writeExecApprovalsFallbackBuffer(destination.fd, destination.snapshot ?? Buffer.alloc(0));
+}
+
+function copyExecApprovalsFallback(tempPath: string, filePath: string): void {
+  const contents = fs.readFileSync(tempPath);
+  const destination = openExecApprovalsFallbackDestination(filePath);
+  try {
+    writeExecApprovalsFallbackBuffer(destination.fd, contents);
+    validateExecApprovalsFallbackFd(filePath, destination.fd);
+  } catch (copyErr) {
+    try {
+      restoreExecApprovalsFallbackDestination(filePath, destination);
+    } catch (restoreErr) {
+      throw new Error(
+        `Failed to restore exec approvals after copy fallback failure for ${filePath}: ${String(
+          copyErr,
+        )}`,
+        { cause: restoreErr },
+      );
+    }
+    throw copyErr;
+  } finally {
+    fs.closeSync(destination.fd);
+  }
+}
+
+function renameExecApprovalsWithFallback(tempPath: string, filePath: string): void {
+  try {
+    fs.renameSync(tempPath, filePath);
+    return;
+  } catch (err) {
+    const code = (err as NodeJS.ErrnoException).code;
+    // Windows can reject rename-overwrite when another process has a transient
+    // handle on the target approvals file.
+    if (code !== "EPERM" && code !== "EEXIST") {
+      throw err;
+    }
+    assertSafeExecApprovalsOverwriteFallback(filePath);
+    copyExecApprovalsFallback(tempPath, filePath);
+    fs.rmSync(tempPath, { force: true });
+  }
+}
+
 // Coerce legacy/corrupted allowlists into `ExecAllowlistEntry[]` before we spread
 // entries to add ids (spreading strings creates {"0":"l","1":"s",...}).
 function coerceAllowlistEntries(allowlist: unknown): ExecAllowlistEntry[] | undefined {
@@ -500,7 +701,27 @@ export function saveExecApprovals(file: ExecApprovalsFile) {
 function writeExecApprovalsRaw(filePath: string, raw: string) {
   const dir = ensureDir(filePath);
   assertSafeExecApprovalsDestination(filePath);
-  privateFileStoreSync(dir).writeText(path.basename(filePath), raw);
+  const tempPath = path.join(dir, `.exec-approvals.${process.pid}.${crypto.randomUUID()}.tmp`);
+  let tempWritten = false;
+  try {
+    fs.writeFileSync(tempPath, raw, { mode: 0o600, flag: "wx" });
+    try {
+      fs.chmodSync(tempPath, 0o600);
+    } catch {
+      // best-effort on platforms without chmod
+    }
+    tempWritten = true;
+    renameExecApprovalsWithFallback(tempPath, filePath);
+  } finally {
+    if (tempWritten && fs.existsSync(tempPath)) {
+      fs.rmSync(tempPath, { force: true });
+    }
+  }
+  try {
+    fs.chmodSync(filePath, 0o600);
+  } catch {
+    // best-effort on platforms without chmod
+  }
 }
 
 export function restoreExecApprovalsSnapshot(snapshot: ExecApprovalsSnapshot): void {