fix(gateway): handle exec approvals persistence errors by martins-oss · Pull Request #79861 · openclaw/openclaw

martins-oss · 2026-05-09T15:21:33Z

What

Wrap local exec.approvals.get and exec.approvals.set gateway handlers in structured unavailable-error handling.

Why

May gateway logs showed approval RPC traffic (exec.approval.list) but local approvals persistence paths could still throw through the request handler if state read/write failed, producing only generic gateway handler failures. This hardens the local path to match node approval handlers and returns an explicit UNAVAILABLE response instead.

How

Converts local get/set handlers to async handlers using existing respondUnavailableOnThrow.
Preserves existing validation/base-hash behavior.
Adds focused regression coverage for read/ensure and persist failures.

Real behavior proof

Behavior or issue addressed: Local gateway exec.approvals.get returns a structured RPC UNAVAILABLE response when approval state setup/read fails, instead of letting the filesystem exception bubble to the generic gateway request-handler failure path.

Real environment tested: OpenClaw checkout rex/approval-hardening-828 on Linux (iris-gateway), Node v22.22.0, running the patched gateway handler directly with node --import tsx. Filesystem failure reproduced with a temp OPENCLAW_HOME whose .openclaw state dir is a symlink to another temp dir, triggering the real exec-approvals path safety guard.

Exact steps or command run after this patch: Created temp OPENCLAW_HOME; symlinked $OPENCLAW_HOME/.openclaw to a separate temp directory; imported src/gateway/server-methods/exec-approvals.ts from this patched checkout; called execApprovalsHandlers['exec.approvals.get'] with a real handler request object and captured respond.

Evidence after fix: Console output from the patched checkout:

$ node --import tsx .tmp-proof-exec-approvals-get.mjs
{
  "ok": false,
  "code": "UNAVAILABLE",
  "message": "Error: Refusing to traverse symlink in exec approvals path must not traverse symlinked directory: /tmp/openclaw-approval-proof-zx6W3i/.openc"
}

Observed result after fix: The patched local handler caught the real filesystem safety exception and returned { ok: false, code: "UNAVAILABLE" } through respond, which is the intended structured RPC failure mode. May 2026 live gateway log review also confirmed approval RPC activity (exec.approval.list succeeded on May 9 at 07:18 and 07:41 ET).

What was not tested: No live gateway restart/deploy was performed, and no production approval state was mutated. exec.approvals.set live failure was not forced against the running gateway; it is covered by focused regression tests plus the same shared wrapper path.

Testing

node scripts/test-projects.mjs --maxWorkers=1 src/gateway/server-methods/exec-approvals.test.ts src/infra/exec-approvals-store.test.ts
pnpm format:check src/gateway/server-methods/exec-approvals.ts src/gateway/server-methods/exec-approvals.test.ts
pnpm lint:core
pnpm tsgo:core:test

clawsweeper · 2026-05-09T15:29:09Z

Codex review: needs maintainer review before merge. Reviewed June 23, 2026, 9:28 PM ET / 01:28 UTC.

Summary
The PR wraps local exec.approvals.get and exec.approvals.set handlers with structured unavailable-error handling and adds focused regression tests for read/setup and save failures.

PR surface: Source +4, Tests +82. Total +86 across 2 files.

Reproducibility: yes. from source inspection: current main invokes local approval persistence helpers directly, and those helpers can throw on real filesystem read/write safety failures. I did not execute a failing current-main repro because this cleanup review is read-only.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: src/gateway/server-methods/exec-approvals.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Refresh the branch against current main and rerun required checks, including Dependency Guard.

Risk before merge

[P1] Required checks are stale against current main: a member comment specifically says the branch needs updating before the new Dependency Guard check can pass.

Maintainer options:

Decide the mitigation before merge
Land the narrow local-handler UNAVAILABLE mapping and regression tests after maintainer review and fresh current-main required checks pass, without changing the exec approvals storage model or protocol shape.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No automated repair is indicated; maintainers need to refresh the branch/checks and review the ready focused gateway bug-fix patch.

Security
Cleared: The diff only changes gateway handler error mapping and tests; it adds no dependencies, workflows, permissions, package metadata, downloads, or secret-handling expansion.

Review details

Best possible solution:

Land the narrow local-handler UNAVAILABLE mapping and regression tests after maintainer review and fresh current-main required checks pass, without changing the exec approvals storage model or protocol shape.

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

Yes from source inspection: current main invokes local approval persistence helpers directly, and those helpers can throw on real filesystem read/write safety failures. I did not execute a failing current-main repro because this cleanup review is read-only.

Is this the best way to solve the issue?

Yes. Reusing the existing respondUnavailableOnThrow helper at the local handler boundary is the narrowest maintainable fix; a global dispatcher catch would over-map unrelated handler bugs, and storage-level catches would hide intentional safety failures.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 63874fa0d119.

Label changes

Label justifications:

P2: This is a normal-priority gateway error-handling bug fix with limited blast radius.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body supplies terminal proof from a patched Linux checkout invoking the real local handler path against a filesystem safety failure and observing a structured UNAVAILABLE response.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies terminal proof from a patched Linux checkout invoking the real local handler path against a filesystem safety failure and observing a structured UNAVAILABLE response.

Evidence reviewed

PR surface:

Source +4, Tests +82. Total +86 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	28	24	+4
Tests	1	82	0	+82
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	110	24	+86

What I checked:

Current-main local handler gap: On current main, local exec approval get/set handlers call ensureExecApprovals, readExecApprovalsSnapshot, and saveExecApprovals directly, so persistence failures can throw through the method body before a structured method response is sent. (src/gateway/server-methods/exec-approvals.ts:129, 63874fa0d119)
Sibling unavailable-wrapper pattern: The node-scoped exec approval helper validates first and then wraps node invocation in respondUnavailableOnThrow, which is the sibling pattern the PR applies to the local persistence path. (src/gateway/server-methods/exec-approvals.ts:115, 63874fa0d119)
Wrapper contract: respondUnavailableOnThrow catches thrown handler failures and responds with protocol ErrorCodes.UNAVAILABLE. (src/gateway/server-methods/nodes.helpers.ts:38, 63874fa0d119)
Async handler compatibility: Gateway handlers may return Promise<void>, lazy core handlers await family handlers, and handleGatewayRequest awaits the handler invocation through the gateway request scope. (src/gateway/server-methods/shared-types.ts:177, 63874fa0d119)
Persistence helpers can throw: Approval snapshot reads use fs.readFileSync outside a catch, and writes route through safety guards and atomic/fallback writes that throw on symlink, hard-link, and access-error cases. (src/infra/exec-approvals.ts:824, 63874fa0d119)
Existing store tests cover throw paths: Current tests already prove exec approval storage throws on blocked reads, symlink destinations, hard links, and symlinked path traversal, matching the failure class the PR maps to structured RPC errors. (src/infra/exec-approvals-store.test.ts:368, 63874fa0d119)

Likely related people:

steipete: GitHub history shows steipete introduced exec approvals tooling and later refactored the gateway validation/base-hash and node invoke error-handling paths this PR extends. (role: feature-history owner; confidence: high; commits: 3686bde783fd, b743e652c0a0, 73a97ee25539; files: src/gateway/server-methods/exec-approvals.ts, src/infra/exec-approvals.ts, src/gateway/server-methods/nodes.helpers.ts)
vincentkoc: Recent commits touched exec approval state-dir behavior and shared exec approvals node invoke handling near the gateway/server-methods and storage paths reviewed here. (role: recent area contributor; confidence: medium; commits: c6232347dcb2, adad27d7448e, 4d6593642e5a; files: src/gateway/server-methods/exec-approvals.ts, src/infra/exec-approvals.ts)
Alex-Alaniz: Recent merged persistence-hardening work covered exec approval write fallback failures adjacent to the storage errors this PR maps into structured gateway responses. (role: adjacent persistence-hardening contributor; confidence: medium; commits: b971ebaaab65; files: src/infra/exec-approvals.ts, src/infra/exec-approvals-store.test.ts)
jesse-merhi: Recent exec authorization planner work touched exec approval persistence, allow-always behavior, and focused approval coverage, making this person relevant for broader exec approval review context. (role: adjacent exec approvals contributor; confidence: medium; commits: c9707ab635b9; files: src/infra/exec-approvals.ts, src/gateway/server-methods/exec-approval.ts)

What the crustacean ranks mean

🦀 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.

How this review workflow works

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.

RomneyDa · 2026-05-29T06:31:04Z

Heads up: this PR needs to be updated against current main before the new required Dependency Guard check can pass.

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

fix(gateway): handle exec approvals persistence errors

9cdcb9c

vincentkoc force-pushed the rex/approval-hardening-828 branch from 1960b3a to 9cdcb9c Compare June 28, 2026 20:27

vincentkoc merged commit 904c871 into openclaw:main Jun 28, 2026
94 checks passed

github-actions Bot mentioned this pull request Jun 28, 2026

📡 Upstream Digest — 2026-06-28 20:53 UTC curtismercier/openclaw-mods#1169

Open

clawsweeper Bot mentioned this pull request Jun 28, 2026

[Bug]: OpenClaw CLI should handle node lacks exec approvals capability gracefully #97578

Closed

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 29, 2026

fix(gateway): handle exec approvals persistence errors (openclaw#79861)

001f27a

clawsweeper Bot mentioned this pull request Jun 29, 2026

fix(gateway): check node commands before exec-approvals invoke #97729

Closed

4 tasks

QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026

fix(gateway): handle exec approvals persistence errors (openclaw#79861)

d26e3d0

chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 1, 2026

fix(gateway): handle exec approvals persistence errors (openclaw#79861)

5a1274f

chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 3, 2026

fix(gateway): handle exec approvals persistence errors (openclaw#79861)

49a21a9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(gateway): handle exec approvals persistence errors#79861

fix(gateway): handle exec approvals persistence errors#79861
vincentkoc merged 1 commit into
openclaw:mainfrom
martins-oss:rex/approval-hardening-828

martins-oss commented May 9, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 9, 2026 •

edited

Loading

Uh oh!

RomneyDa commented May 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

martins-oss commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

How

Real behavior proof

Testing

Uh oh!

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

Uh oh!

Uh oh!

RomneyDa commented May 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

martins-oss commented May 9, 2026 •

edited

Loading

clawsweeper Bot commented May 9, 2026 •

edited

Loading