Description

exec-approvals.json is now resolved relative to OPENCLAW_STATE_DIR when set. If that environment variable points to a new/empty directory, OpenClaw will ignore an existing hardened approvals file in the default ~/.openclaw/exec-approvals.json and will silently create a new approvals file in the override directory.

This weakens local exec protections because the newly-created file has no explicit defaults policy, so effective defaults fall back to permissive built-ins (security: "full", ask: "off").

Impact:

• A host-local configuration like ask: "always" (meant to force prompts) can be unintentionally dropped if the process starts with OPENCLAW_STATE_DIR set.
• ensureExecApprovals() always writes the (possibly empty) normalized config to disk, making the downgrade persistent.

Vulnerable flow:

• Input: process.env.OPENCLAW_STATE_DIR
• Decision: resolveExecApprovalsPath() uses the override directory
• Sink: ensureExecApprovals() creates/saves a new approvals file when none exists at the override location
• Effect: prior stricter approvals stored at the legacy location are not migrated or even warned about

Vulnerable code:

const override = env.OPENCLAW_STATE_DIR?.trim();
if (override) {
  const resolved = resolveHomeRelativePath(override, { env });
  return { path: resolved, displayPath: resolved };
}
...
export function ensureExecApprovals(): ExecApprovalsFile {
  const loaded = loadExecApprovals();
  ...
  saveExecApprovals(updated);
  return updated;
}

Recommendation

Prevent accidental policy downgrades when switching state roots.

Options (pick one):

1. Automatic migration with warning: if OPENCLAW_STATE_DIR is set and $OPENCLAW_STATE_DIR/exec-approvals.json does not exist but the legacy ~/.openclaw/exec-approvals.json exists, copy/merge the legacy file into the new location (or at least merge defaults/agents), and emit a clear warning.

2. Fail closed / require explicit opt-in: if override is set and no approvals file exists at the override path while a legacy file exists, refuse to start unless a --migrate-exec-approvals flag (or similar) is provided.

Example migration logic:

const legacyPath = path.join(expandHomePrefix("~/.openclaw"), "exec-approvals.json");
const newPath = resolveExecApprovalsPath();

if (process.env.OPENCLAW_STATE_DIR && !fs.existsSync(newPath) && fs.existsSync(legacyPath)) {
  ​// copy or merge
  fs.mkdirSync(path.dirname(newPath), { recursive: true });
  fs.copyFileSync(legacyPath, newPath, fs.constants.COPYFILE_EXCL);
  console.warn(`Migrated exec approvals from ${legacyPath} to ${newPath}`);
}

Also consider writing explicit defaults into a newly-created approvals file (rather than relying on permissive runtime fallbacks), so the user can see and audit the effective policy.

Description

OPENCLAW_STATE_DIR can override the location of the exec-approvals UNIX socket. If it is set to a shared/writable directory (e.g. /tmp), another local user can pre-create/bind a fake socket at that path and spoof approval decisions.

Why this is a security issue:

• The socket path is derived directly from OPENCLAW_STATE_DIR with no validation of directory ownership/permissions or existing socket type.
• The client sends the token inside the request payload, so a spoofed server learns the token immediately and can reply with any decision (the token does not authenticate the server).

Vulnerable code (path override + socket usage):

const override = env.OPENCLAW_STATE_DIR?.trim();
...
return path.join(resolveExecApprovalsStateDir().path, EXEC_APPROVALS_SOCKET);

const payload = JSON.stringify({ type: "request", token, ... });
return await requestJsonlSocket({ socketPath, requestLine: payload, ... });

Impact:

• A local attacker can force allow-once / allow-always responses, effectively bypassing exec approvals prompts/decisions for the operator when the socket is placed in an attacker-controllable location.

Recommendation

Harden the socket location and add peer validation:

1. Reject unsafe socket directories (especially when overridden):

• Ensure the parent directory is owned by the current user and is not group/other-writable.
• Prefer creating/using a dedicated state subdirectory with mode 0700.

2. Validate the socket file before connecting:

• lstat() the socket path and require stat.isSocket().
• Optionally validate ownership (stat.uid) and that it is not group/other-writable.

3. If you need authentication, use OS-backed peer credentials (e.g., SO_PEERCRED on Linux) or a proper challenge/response where the client does not reveal the only secret first.

Example (pre-connect check):

import os from "node:os";

function assertSafeSocketPath(socketPath: string) {
  const st = fs.lstatSync(socketPath);
  if (!st.isSocket()) throw new Error(`Not a socket: ${socketPath}`);
  if ((st.mode & 0o022) !== 0) throw new Error(`Socket is writable by group/others: ${socketPath}`);
  if (typeof st.uid === "number" && st.uid !== os.userInfo().uid) {
    throw new Error(`Socket not owned by current user: ${socketPath}`);
  }
}

assertSafeSocketPath(socketPath);

And when creating the state directory, set restrictive perms:

fs.mkdirSync(stateDir, { recursive: true, mode: 0o700 });

Best possible solution:

Land a state-dir-aware exec approvals fix only after it preserves or explicitly migrates existing approval policy, removes release-owned changelog churn, and refreshes against current main.

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

Yes for source proof, not live execution: current main hardcodes exec approvals to ~/.openclaw while the canonical state-dir resolver honors OPENCLAW_STATE_DIR. The PR's upgrade downgrade is also source-reproducible through its override branch plus the ensureExecApprovals() write path.

Is this the best way to solve the issue?

No. Routing the path through the state directory is the right direction for fresh setups, but this implementation needs an upgrade-safe migration, warning, or fail-closed behavior before it is the best fix.

Full review comments:

• [P1] Preserve existing approval policy when moving state roots — src/infra/exec-approvals.ts:190-195
  This switches the default approvals file/socket to OPENCLAW_STATE_DIR without checking whether an existing ~/.openclaw/exec-approvals.json contains stricter defaults. On upgrade, a user who set ask: "always" or security: "allowlist" in the old file and then starts with a state-dir override gets a fresh file at the override path, so effective policy falls back to permissive built-ins. Migrate/copy the legacy policy or fail/warn before creating the new file.
  Confidence: 0.9
• [P3] Drop the release-owned changelog edit — CHANGELOG.md:16
  Normal PRs should not edit CHANGELOG.md; the release flow owns that file, and this stale insertion is also likely to conflict with current release notes. Keep the release-note context in the PR body or squash message instead.
  Confidence: 0.87

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

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

Label changes:

• add P1: The PR targets a real exec approval state-root bug but currently risks weakening a security-sensitive host exec approval workflow on upgrade.
• add merge-risk: 🚨 compatibility: Changing the default approvals file root for OPENCLAW_STATE_DIR users can strand existing persisted approvals unless a migration or explicit upgrade path is added.
• add merge-risk: 🚨 security-boundary: Exec approvals gate host command execution, and silently losing stricter approval defaults can bypass the user's intended command-approval boundary.
• add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🌊 off-meta tidepool and patch quality is 🧂 unranked krab.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The contributor proof gate is not applied because this is a ProjectClownfish bot repair; the linked source PR reports minimal and Docker verification, but the patch still has merge-blocking security/upgrade issues.
• remove rating: 🌊 off-meta tidepool: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.

Label justifications:

• P1: The PR targets a real exec approval state-root bug but currently risks weakening a security-sensitive host exec approval workflow on upgrade.
• merge-risk: 🚨 compatibility: Changing the default approvals file root for OPENCLAW_STATE_DIR users can strand existing persisted approvals unless a migration or explicit upgrade path is added.
• merge-risk: 🚨 security-boundary: Exec approvals gate host command execution, and silently losing stricter approval defaults can bypass the user's intended command-approval boundary.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🌊 off-meta tidepool and patch quality is 🧂 unranked krab.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The contributor proof gate is not applied because this is a ProjectClownfish bot repair; the linked source PR reports minimal and Docker verification, but the patch still has merge-blocking security/upgrade issues.

PR surface:

Source +26, Tests +72, Docs +9. Total +107 across 6 files.

Security concerns:

• [high] Existing strict approvals can be bypassed on upgrade — src/infra/exec-approvals.ts:190
  When the override target has no approvals file, the PR ignores a stricter legacy ~/.openclaw/exec-approvals.json and persists a fresh file whose omitted defaults resolve to permissive built-ins. That can change approval behavior for host command execution without an operator-visible migration.
  Confidence: 0.88

What I checked:

• Current main still hardcodes exec approvals storage: resolveExecApprovalsPath() and resolveExecApprovalsSocketPath() still expand ~/.openclaw/exec-approvals.* on current main, so the central bug is not already implemented there. (src/infra/exec-approvals.ts:295, b9933b2ec119)
• State-dir contract exists elsewhere: The canonical state-dir resolver honors OPENCLAW_STATE_DIR, which supports the PR's intended product direction for relocated mutable state. (src/config/paths.ts:61, b9933b2ec119)
• PR override branch lacks upgrade handling: The PR sends any non-empty OPENCLAW_STATE_DIR directly to a new approvals path without checking whether a stricter legacy approvals file already exists at the old default path. (src/infra/exec-approvals.ts:190, 3acd66879e0a)
• Missing-file path persists defaults: On the PR branch, ensureExecApprovals() still saves a normalized file after loading, so an empty override directory can get a fresh approvals file instead of preserving an existing stricter legacy policy. (src/infra/exec-approvals.ts:545, 3acd66879e0a)
• Release-owned changelog edit: The PR adds a normal unreleased changelog entry even though repository policy keeps CHANGELOG.md release-owned for ordinary PRs. (CHANGELOG.md:16, 3acd66879e0a)
• Security discussion already flagged the same downgrade: The PR discussion includes an Aisle security finding that OPENCLAW_STATE_DIR pointing at a fresh directory can silently bypass a stricter existing ~/.openclaw/exec-approvals.json policy.

Likely related people:

• steipete: Git history shows Peter Steinberger introduced and repeatedly hardened the exec approvals flow, including initial tooling, socket requester, security binding, and default host exec policy work. (role: feature owner and heavy area contributor; confidence: high; commits: efdb33c97587, 3686bde783fd, 78a7ff2d50fb; files: src/infra/exec-approvals.ts, src/infra/exec-approvals-effective.ts, docs/tools/exec-approvals.md)
• vincentkoc: Recent history and blame on the current hardcoded resolver lines point to Vincent Koc through release/restoration and adjacent exec approval fixes, making him a useful routing candidate for the current branch state. (role: recent area contributor; confidence: medium; commits: 086df266ccaf, d9a3ecd109ee, 2d53ffdec1da; files: src/infra/exec-approvals.ts, CHANGELOG.md)
• gumadeiras: Gustavo Madeira Santana authored nearby effective-policy reporting work, which is one of the PR's touched surfaces. (role: adjacent effective-policy contributor; confidence: medium; commits: ba735d015809, f69570f82020; files: src/infra/exec-approvals-effective.ts, src/infra/exec-approvals-policy.test.ts)
• Takhoffman: Tak Hoffman added the local exec-policy CLI that reads and writes the same approvals policy surface this PR changes. (role: adjacent CLI policy contributor; confidence: low; commits: 4bf94aa0d669; files: src/infra/exec-approvals.ts, docs/tools/exec-approvals.md)

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.