fix(exec-approvals): guard Windows rename fallback by Alex-Alaniz · Pull Request #77907 · openclaw/openclaw

Alex-Alaniz · 2026-05-05T15:19:13Z

Summary

add a guarded EPERM/EEXIST fallback for exec-approvals.json rename-overwrite failures
preserve the existing symlink and hard-link safety boundaries before using the copy fallback
snapshot and restore the previous approvals file if fallback copy fails after opening/truncating the destination
cover successful fallback cleanup, hard-linked destination refusal, and destructive copy-failure restoration in store tests

Root cause

writeExecApprovalsRaw writes a temp file and then atomically renames it over exec-approvals.json. On Windows, rename-overwrite can fail with EPERM when another process has a transient handle on the target file, which blocks exec approval persistence before the exec call can continue.

Real behavior proof

Behavior or issue addressed: when exec-approvals.json rename-overwrite is rejected with a Windows-style EPERM, the approval store falls back to a guarded copy, preserves file mode 600, and removes its temp file; hard-linked approval files are still rejected, and a failed fallback copy restores the previous approvals bytes.
Real environment tested: local OpenClaw checkout on macOS with Node running the patched saveExecApprovals path against isolated temporary OPENCLAW_HOME directories under /var/folders/.../T.
Exact steps or command run after this patch: ran node --import tsx from the patched checkout, created a real approvals file, forced fs.renameSync to throw EPERM only for that approvals target, called saveExecApprovals, repeated with a hard-linked approvals target to verify the guard rejects fallback, then forced fs.copyFileSync to truncate and throw to verify restore.
Evidence after fix: copied live terminal output from the fallback/hard-link command:

{
  "fallbackHome": "/var/folders/cf/mtzn4vhx6kz_83fmvt5qs60m0000gn/T/openclaw-77785-fallback-tIUOeJ",
  "fallbackPath": "/var/folders/cf/mtzn4vhx6kz_83fmvt5qs60m0000gn/T/openclaw-77785-fallback-tIUOeJ/.openclaw/exec-approvals.json",
  "forcedRenameErrors": 1,
  "fallbackContainsSecurity": true,
  "fallbackMode": "600",
  "fallbackTempFiles": [],
  "hardlinkHome": "/var/folders/cf/mtzn4vhx6kz_83fmvt5qs60m0000gn/T/openclaw-77785-hardlink-fA79BX",
  "hardlinkErrorIncludesGuard": true,
  "hardlinkSentinelPreserved": true,
  "hardlinkTempFiles": []
}

Copied live terminal output from the fallback copy-failure restore command:

{
  "home": "/var/folders/cf/mtzn4vhx6kz_83fmvt5qs60m0000gn/T/openclaw-77785-restore-proof-VQVbZA",
  "forcedRenameErrors": 1,
  "forcedCopyErrors": 1,
  "errorIncludesCopyFailure": true,
  "previousApprovalsRestored": true,
  "finalMode": "600",
  "tempFiles": []
}

Observed result after fix: the forced EPERM path persisted the updated approvals file, kept secure permissions, and left no temp files; the hard-linked approvals target threw the hard-link guard and left the sentinel link content unchanged; the simulated destructive copy failure restored the previous approvals content and mode.
What was not tested: I did not reproduce a native Windows locked-handle gateway process on this macOS machine; the live proof forced the same EPERM and copy-failure boundaries in the actual store writer.

Validation

pnpm test src/infra/exec-approvals-store.test.ts
pnpm test src/agents/bash-tools.exec-host-gateway.test.ts
pnpm exec oxfmt --check --threads=1 src/infra/exec-approvals.ts src/infra/exec-approvals-store.test.ts CHANGELOG.md
pnpm check:changed
git diff --check

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: deaca9a7ac

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-05T15:22:53Z

+      throw err;
+    }
+    assertSafeExecApprovalsOverwriteFallback(filePath);
+    fs.copyFileSync(tempPath, filePath);


Avoid deleting approvals file on fallback copy errors

Using fs.copyFileSync(tempPath, filePath) as the overwrite fallback can destroy the existing exec-approvals.json if the copy fails after opening the destination (for example ENOSPC/I/O errors), because Node may remove the destination on copy failure. Before this change, a failed renameSync left the old approvals file intact; now the same failure path can lose the persisted approvals entirely, which can reset policy state and block or weaken expected approval behavior on the next load.

Useful? React with 👍 / 👎.

clawsweeper · 2026-05-05T15:23:08Z

Codex review: needs maintainer review before merge.

Summary
The branch replaces exec approvals persistence with a guarded temp-file writer that falls back to fd-based copy on Windows-style rename EPERM/EEXIST, adds regression tests, and updates the changelog.

Reproducibility: yes. source-level: current main persists exec approvals through a temp rename path, and the linked Windows report gives concrete EPERM failures across shipped versions. I did not run a native Windows locked-handle gateway repro in this read-only pass.

Real behavior proof
Sufficient (terminal): The PR body includes copied terminal output from a real patched checkout exercising the forced EPERM fallback, guard failures, restoration path, modes, and cleanup.

Next step before merge
No repair lane: the latest head has no discrete fixable defect after the previous review findings were addressed; remaining action is normal maintainer merge review.

Security
Cleared: No concrete security or supply-chain regression found in the latest diff; the security-sensitive approvals writer now retains the relevant permission, symlink, hard-link, and restoration safeguards.

Review details

Best possible solution:

Land this writer-local fallback after normal maintainer approval so the paired Windows regression can close through the PR merge.

Do we have a high-confidence way to reproduce the issue?

Yes, source-level: current main persists exec approvals through a temp rename path, and the linked Windows report gives concrete EPERM failures across shipped versions. I did not run a native Windows locked-handle gateway repro in this read-only pass.

Is this the best way to solve the issue?

Yes. The latest head keeps the fix in the exec approvals writer, adds guarded fallback and restoration behavior, and preserves directory/file permission and symlink/hard-link safety boundaries.

What I checked:

Current main write path: Current main still routes saveExecApprovals through writeExecApprovalsRaw and privateFileStoreSync(dir).writeText, whose @openclaw/fs-safe@0.1.1 sync writer uses a temp file followed by renameSync without the guarded copy fallback this PR adds. (src/infra/exec-approvals.ts:500, ae9f779e5ff4)
Guarded fallback implementation: The PR head handles EPERM/EEXIST from renameSync by validating the destination, opening a checked fd, copying bytes through that fd, restoring the prior file on copy failure, and removing the temp file. (src/infra/exec-approvals.ts:455, 91fe6a62c97c)
Permission safeguards restored: The latest head chmods the approvals directory to 0700, chmods the temp file before fallback reads, and normalizes the final approvals file to 0600 after rename or fallback. (src/infra/exec-approvals.ts:268, 91fe6a62c97c)
Regression coverage: New tests cover final file mode, directory mode, EPERM fallback success, temp readability under restrictive mode, destructive copy-failure restoration, symlink-swap refusal, hard-link refusal, and temp cleanup. (src/infra/exec-approvals-store.test.ts:184, 91fe6a62c97c)
Prior review findings addressed: Earlier review comments flagged copy-failure data loss, symlink swap, missing final chmod, missing directory chmod, and unreadable temp files; the latest code and tests now cover those exact boundaries. (src/infra/exec-approvals.ts:456, 91fe6a62c97c)
Real behavior proof: The PR body includes copied terminal JSON from a patched checkout showing forced EPERM fallback persistence, mode 600, temp cleanup, hard-link refusal, and destructive copy-failure restoration. (91fe6a62c97c)

Likely related people:

steipete: Recent current-main history extracted fs-safe/private-store usage and includes repeated exec approvals maintenance on the affected writer and safety contracts. (role: recent maintainer and adjacent owner; confidence: high; commits: 538605ff44d2, 3f7e6eebc2f5, fff6333773f2; files: src/infra/exec-approvals.ts, src/infra/private-file-store.ts, src/agents/bash-tools.exec-host-shared.ts)
vincentkoc: GitHub history shows major exec approvals fixes around remote approval defaults, durable approval behavior, and trusted approvals home symlinks. (role: feature-history owner; confidence: medium; commits: 2d53ffdec1da, 364d49889e67, ecb4ea983011; files: src/infra/exec-approvals.ts, src/infra/exec-approvals-store.test.ts)
Takhoffman: The local exec-policy CLI work hardened exec approvals writes, rollback, and approvals path checks on the same storage surface. (role: adjacent owner; confidence: medium; commits: 4bf94aa0d669; files: src/infra/exec-approvals.ts, src/infra/exec-approvals-store.test.ts)

Remaining risk / open question:

Native Windows locked-handle gateway behavior was not re-run in this read-only review; the after-fix proof uses forced EPERM on the real store writer plus exact-head CI.

Codex review notes: model gpt-5.5, reasoning high; reviewed against ae9f779e5ff4.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 90681bd82d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-05T15:37:45Z

+    assertSafeExecApprovalsOverwriteFallback(filePath);
+    const snapshot = readExecApprovalsFallbackSnapshot(filePath);
+    try {
+      fs.copyFileSync(tempPath, filePath);


Prevent symlink swap between fallback check and copy

The new Windows fallback still has a TOCTOU gap: after assertSafeExecApprovalsOverwriteFallback(filePath) verifies the destination is not a symlink, another process can replace exec-approvals.json with a symlink before fs.copyFileSync(tempPath, filePath) runs. In that race, copyFileSync follows the symlink and overwrites the link target, so an EPERM/EEXIST rename failure can now clobber arbitrary user-writable files outside the approvals path, which defeats the symlink safety boundary this code is trying to enforce.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 153092fab4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-05T15:54:04Z

+    renameExecApprovalsWithFallback(tempPath, filePath);
  } finally {
    if (tempWritten && fs.existsSync(tempPath)) {
      fs.rmSync(tempPath, { force: true });
    }


Preserve 0600 mode after successful temp-file rename

writeExecApprovalsRaw no longer reapplies permissions after a successful rename, so the final exec-approvals.json mode now depends on the process umask. Because writeFileSync(..., { mode: 0o600 }) is still masked by umask, restrictive service umasks can leave the file unreadable (for example 000), which then makes later reads fall back to defaults and drops persisted approvals; the previous chmodSync step avoided this regression.

Useful? React with 👍 / 👎.

BradGroux · 2026-05-06T02:19:25Z

Maintainer prep update for #77907.

What changed:

rebased the Windows EPERM fallback onto current main after the exec approvals writer moved through privateFileStoreSync
kept the guarded copy fallback for Windows rename-overwrite failures
restored the final 0600 chmod on the normal rename path and added regression coverage for restrictive-created temp file modes
kept symlink, hard-link, swapped-destination, copy-failure restore, and temp cleanup coverage

Prepared head: 6dc0c0f61a589312079683176ecb4aa51f050054

Local verification:

pnpm test src/infra/exec-approvals-store.test.ts
pnpm test src/agents/bash-tools.exec-host-gateway.test.ts
pnpm exec oxfmt --check --threads=1 src/infra/exec-approvals.ts src/infra/exec-approvals-store.test.ts CHANGELOG.md
pnpm check:changed
git diff --check

CI note: earlier runs were blocked by unrelated current-main archive/fs-safe race-test failures in checks-node-core-runtime-infra-state; #77907 was rebased after the upstream test-fix commits landed and a fresh run is now in progress.

BradGroux · 2026-05-06T02:36:36Z

Maintainer refresh: rebased again onto current main and pushed prepared head 4530dd1c436042658358b8bceb050e0ef01e01fb. Focused verification after the rebase: pnpm test src/infra/exec-approvals-store.test.ts (22 passed) and pnpm exec oxfmt --check --threads=1 src/infra/exec-approvals.ts src/infra/exec-approvals-store.test.ts CHANGELOG.md. Fresh CI is running; the previous failure was the unrelated core shared cron shard stalling/no-output timeout.

BradGroux · 2026-05-06T03:20:11Z

Maintainer refresh: picked up the extra approvals directory-mode commit, rebased onto current main, resolved the changelog move, and pushed prepared head 28834ad0d1e6600b00956b6f70960d9c8dcd1c90. Focused verification after the rebase: pnpm test src/infra/exec-approvals-store.test.ts (23 passed) and pnpm exec oxfmt --check --threads=1 src/infra/exec-approvals.ts src/infra/exec-approvals-store.test.ts CHANGELOG.md. Fresh CI is running.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7f3d0eebe5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-06T03:21:25Z

+}
+
+function copyExecApprovalsFallback(tempPath: string, filePath: string): void {
+  const contents = fs.readFileSync(tempPath);


Make the fallback temp file readable before copying

When the process has a restrictive umask (for example 0777) and renameSync takes the new EPERM/EEXIST fallback, the temp file created with { mode: 0o600 } is still umask-masked, so reopening it here by path can throw EACCES before the guarded copy runs. The final chmodSync(filePath, 0o600) only happens after renameExecApprovalsWithFallback returns, so this fallback still fails to persist approvals in those service environments; chmod the temp file before reading it or copy from an already-open descriptor.

Useful? React with 👍 / 👎.

* 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>

openclaw-barnacle Bot added the size: S label May 5, 2026

chatgpt-codex-connector Bot reviewed May 5, 2026

View reviewed changes

openclaw-barnacle Bot added size: M and removed size: S labels May 5, 2026

chatgpt-codex-connector Bot reviewed May 5, 2026

View reviewed changes

BradGroux mentioned this pull request May 6, 2026

WORKING: All Microsoft Issues and PRs (refresh) #74163

Draft

BradGroux force-pushed the codex/issue-77785-exec-approvals-rename-fallback branch from 153092f to b7c0816 Compare May 6, 2026 01:41

openclaw-barnacle Bot added the proof: supplied External PR includes structured after-fix real behavior proof. label May 6, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 6, 2026

BradGroux force-pushed the codex/issue-77785-exec-approvals-rename-fallback branch from b7c0816 to f97f4e8 Compare May 6, 2026 01:54

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 6, 2026

BradGroux force-pushed the codex/issue-77785-exec-approvals-rename-fallback branch from f97f4e8 to 9bc4a4a Compare May 6, 2026 02:00

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 6, 2026

BradGroux force-pushed the codex/issue-77785-exec-approvals-rename-fallback branch from 9bc4a4a to 461bb87 Compare May 6, 2026 02:14

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 6, 2026

BradGroux force-pushed the codex/issue-77785-exec-approvals-rename-fallback branch from 461bb87 to 6dc0c0f Compare May 6, 2026 02:19

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 6, 2026

BradGroux force-pushed the codex/issue-77785-exec-approvals-rename-fallback branch from 6dc0c0f to 4530dd1 Compare May 6, 2026 02:34

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 6, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 6, 2026

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 6, 2026

BradGroux force-pushed the codex/issue-77785-exec-approvals-rename-fallback branch from 7f3d0ee to 28834ad Compare May 6, 2026 03:19

chatgpt-codex-connector Bot reviewed May 6, 2026

View reviewed changes

BradGroux and others added 2 commits May 5, 2026 23:23

fix exec approvals Windows rename fallback

11e83a6

fix(exec-approvals): restore approvals directory mode

460ad5c

fix(exec-approvals): normalize fallback temp mode

91fe6a6

Alex-Alaniz force-pushed the codex/issue-77785-exec-approvals-rename-fallback branch from 28834ad to 91fe6a6 Compare May 6, 2026 03:24

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 6, 2026

BradGroux merged commit b971eba into openclaw:main May 6, 2026
97 checks passed

BradGroux mentioned this pull request May 6, 2026

fix(msteams): surface network errors blocking bot JWT validation and outbound replies (#77674) #78081

Merged

Uh oh!

Conversation

Alex-Alaniz commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Real behavior proof

Validation

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 5, 2026

Choose a reason for hiding this comment

Uh oh!

clawsweeper Bot commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 5, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 5, 2026

Choose a reason for hiding this comment

Uh oh!

BradGroux commented May 6, 2026

Uh oh!

BradGroux commented May 6, 2026

Uh oh!

BradGroux commented May 6, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Alex-Alaniz commented May 5, 2026 •

edited

Loading

clawsweeper Bot commented May 5, 2026 •

edited

Loading