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

Alex-Alaniz · BradGroux · web-flow · commit b971ebaaab65 · 2026-05-05T22:39:41.000-05:00
* 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 &lt;3053586+BradGroux@users.noreply.github.com&gt;
diff --git a/CHANGELOG.md b/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.
diff --git a/src/infra/exec-approvals-store.test.ts b/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);
diff --git a/src/infra/exec-approvals.ts b/src/infra/exec-approvals.ts
