openclaw - ✅(Solved) Fix [Bug]: Node allowlist writeback can restore revoked exec approvals [1 pull requests, 1 comments, 2 participants]

Official PRs (…)
ON THIS PAGE

Recommended Tools

×6

Utilities matched from this issue’s tags and category — try them while you read without losing context.

GitHub issue graph ai analysis

Paste a GitHub issue URL. We fetch that issue, discover linked issues from bodies/comments/timeline, collect linked pull requests, and produce a structured English report.

The report is written in English Markdown for sharing and archival.

Helpful · Quick feedback

Loading…
GitHub stats
openclaw/openclaw#78225Fetched 2026-05-07 03:39:25
View on GitHub
Comments
1
Participants
2
Timeline
2
Reactions
2
Author
Timeline (top)
commented ×1cross-referenced ×1

Root Cause

This crosses the documented operator-scope boundary for identity-bearing Gateway clients rather than the out-of-scope shared-secret operator bucket. node.invoke is a write-scoped relay (src/gateway/method-scopes.ts:129-151), while exec.approvals.node.set stays admin-only because exec.approvals.* is a reserved core admin namespace (src/shared/gateway-method-policy.ts:1-20; docs/gateway/protocol.md:236-250). OpenClaw also documents that explicit device-token scopes remain authoritative (docs/gateway/protocol.md:626-628) and that node pairing does not replace the node's own system.run exec approval policy (docs/gateway/operator-scopes.md:91-100), so a write-scoped caller restoring admin-revoked node exec policy is not covered by the out-of-scope semantic-model caveat in openclaw/SECURITY.md:171.

Impact

Fix Action

Fixed

PR fix notes

PR #78226: fix: Node allowlist writeback can restore revoked exec approvals

Description (problem / solution / changelog)

Summary

  • Fixes a race where execution-side allowlist metadata writeback could save a stale exec-approvals.json snapshot after an admin revoked or changed policy.
  • Adds a hash/CAS guard plus a file-lock-backed write path for exec approval updates that can race with admin writes.
  • Propagates the resolved approvals hash through node-host and gateway execution paths so usage metadata and allow-always persistence only write when the evaluated snapshot is still current.
  • Adds regression coverage for stale writeback and the remaining interleaving where the file changes between reload and save.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

  • Closes #78225
  • This PR fixes a bug or regression

Root Cause

Execution-time approval helpers updated allowlist usage metadata by mutating and saving an approvals object derived from an earlier policy-evaluation snapshot. Unlike admin approval writes, those metadata updates did not verify the file hash before saving. A write-scoped execution path could therefore overwrite a newer admin policy change with stale allowlist data.

Regression Test Plan

  • Added/updated src/infra/exec-approvals-store.test.ts coverage for stale allowlist metadata writeback.
  • Covered the race where an admin revocation lands after the execution helper reloads but before it saves.
  • Verified the touched execution and admin write paths still typecheck and lint cleanly.

User-visible / Behavior Changes

  • Execution-side allowlist metadata writes are now best-effort when the approvals file changed after policy evaluation. If an admin update wins the race, the metadata write is skipped rather than restoring stale policy.
  • Admin approval writes continue to use base-hash validation and now go through the shared locked save helper.

Security Impact

  • New permissions/capabilities? (Yes/No): No
  • Secrets/tokens handling changed? (Yes/No): No
  • New/changed network calls? (Yes/No): No
  • Command/tool execution surface changed? (Yes/No): Yes
  • Data access scope changed? (Yes/No): Yes
  • If any Yes, explain risk + mitigation: The changed execution path now refuses stale exec-approval writeback instead of broadening command execution. Regression tests cover stale snapshot writeback, and validation passed on the touched files.

Repro + Verification

Environment

  • Runtime/container: local OpenClaw worktree
  • Model/provider: github-copilot/gpt-5.4 generated the broader fix; manual recovery added the missing hash helper and reran validation.
  • Relevant config: staged high-security submission workflow with release/drift skips recorded for recovery.

Steps

  1. Reproduce the linked issue by evaluating a command against an allowlisted snapshot, revoking that approval via admin policy update, then allowing execution-side metadata writeback to run.
  2. Apply this branch.
  3. Run the validation commands below.

Expected

  • Execution-side metadata writeback does not restore a revoked or changed approval when the underlying approvals file changed after policy evaluation.
  • Validation passes for the touched behavior.

Actual

  • Validation status: passed.

Evidence

Validation commands run on the amended branch:

  • pnpm exec oxlint src/infra/exec-approvals.ts src/infra/exec-approvals-store.test.ts src/node-host/invoke-system-run.ts src/agents/bash-tools.exec-host-gateway.ts src/cli/exec-approvals-cli.ts src/cli/exec-policy-cli.ts src/gateway/server-methods/exec-approvals.ts src/node-host/invoke.ts
  • pnpm build
  • pnpm check
  • pnpm test src/infra/exec-approvals-store.test.ts

Results:

  • Changed-file lint: passed, 0 warnings/errors.
  • Build: passed.
  • Check/type/lint policy suite: passed.
  • Targeted regression test: 19 passed in src/infra/exec-approvals-store.test.ts.

CVSS from linked issue bundle:

  • CVSS v3.1: 7.1 High
  • CVSS v4.0: 7.1 High

Changed files:

  • src/agents/bash-tools.exec-host-gateway.ts
  • src/cli/exec-approvals-cli.ts
  • src/cli/exec-policy-cli.ts
  • src/gateway/server-methods/exec-approvals.ts
  • src/infra/exec-approvals-store.test.ts
  • src/infra/exec-approvals.ts
  • src/node-host/invoke-system-run.ts
  • src/node-host/invoke.ts

Human Verification

  • Verified scenarios: stale allowlist metadata writeback no longer restores policy when the approvals file hash changed; the remaining reload-before-save race is covered by regression tests.
  • Edge cases checked: empty/no-op metadata update, current hash mismatch, and changed-file lint/type/build behavior.
  • What you did not verify: A full repo-wide pnpm test completed previously with baseline/no-output infrastructure noise in the submission workflow; after the final amended commit, verification was limited to targeted regression tests plus build/check/lint.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

  • Backward compatible? (Yes/No): Yes
  • Config/env changes? (Yes/No): No
  • Migration needed? (Yes/No): No
  • If yes, exact upgrade steps: N/A

Risks and Mitigations

  • Risk: Approval metadata such as lastUsedAt may be skipped when an admin concurrently changes policy.
    • Mitigation: This is intentional fail-closed behavior for policy freshness. A later execution can record metadata once it evaluates the latest approvals snapshot.
  • Risk: Locking approval writes changes local write sequencing.
    • Mitigation: The lock is scoped to exec approvals writes and validation passed through build, check, lint, and targeted regression tests.

Changed files

  • src/agents/bash-tools.exec-host-gateway.ts (modified, +12/-5)
  • src/cli/exec-approvals-cli.ts (modified, +4/-4)
  • src/cli/exec-policy-cli.ts (modified, +2/-2)
  • src/gateway/server-methods/exec-approvals.ts (modified, +3/-3)
  • src/infra/exec-approvals-store.test.ts (modified, +118/-8)
  • src/infra/exec-approvals.ts (modified, +265/-66)
  • src/node-host/invoke-system-run.ts (modified, +10/-3)
  • src/node-host/invoke.ts (modified, +2/-2)

Code Example

const approvals = resolveExecApprovals(parsed.agentId, {
  security: configuredSecurity,
  ask: configuredAsk,
});
const bins = autoAllowSkills ? await opts.skillBins.current() : [];

recordAllowlistMatchesUse({
  approvals: phase.approvals.file,
  agentId: phase.agentId,
  matches: phase.allowlistMatches,
  command: phase.commandText,
});

approvals.agents = agents;
saveExecApprovals(approvals);
RAW_BUFFERClick to expand / collapse

Severity Assessment

CVSS Assessment

Metricv3.1v4.0
Score7.1 / 10.07.1 / 10.0
SeverityHighHigh
VectorCVSS:3.1/AV:N/AC:H/PR:L/UI:R/S:C/C:L/I:H/A:LCVSS:4.0/AV:N/AC:H/AT:N/PR:L/UI:P/VC:L/VI:H/VA:L/SC:L/SI:H/SA:L
CalculatorCVSS v3.1 CalculatorCVSS v4.0 Calculator

Threat Model Alignment

Classification: security-specific

This crosses the documented operator-scope boundary for identity-bearing Gateway clients rather than the out-of-scope shared-secret operator bucket. node.invoke is a write-scoped relay (src/gateway/method-scopes.ts:129-151), while exec.approvals.node.set stays admin-only because exec.approvals.* is a reserved core admin namespace (src/shared/gateway-method-policy.ts:1-20; docs/gateway/protocol.md:236-250). OpenClaw also documents that explicit device-token scopes remain authoritative (docs/gateway/protocol.md:626-628) and that node pairing does not replace the node's own system.run exec approval policy (docs/gateway/operator-scopes.md:91-100), so a write-scoped caller restoring admin-revoked node exec policy is not covered by the out-of-scope semantic-model caveat in openclaw/SECURITY.md:171.

Impact

A write-scoped caller can win a race against an admin policy change and write an older exec-approvals.json snapshot back to the node host. That restores revoked allowlist entries or older defaults, so later node.invoke system.run requests run under stale node policy until an admin reapplies the change.

Affected Component

File: src/node-host/invoke-system-run.ts:379-392, src/node-host/invoke-system-run.ts:620-647, src/infra/exec-approvals.ts:870-894

const approvals = resolveExecApprovals(parsed.agentId, {
  security: configuredSecurity,
  ask: configuredAsk,
});
const bins = autoAllowSkills ? await opts.skillBins.current() : [];

recordAllowlistMatchesUse({
  approvals: phase.approvals.file,
  agentId: phase.agentId,
  matches: phase.allowlistMatches,
  command: phase.commandText,
});

approvals.agents = agents;
saveExecApprovals(approvals);

Technical Reproduction

  1. Run a headless or fallback node host with a manual allowlist entry in the node exec approvals file and autoAllowSkills: true for the affected agent.
  2. Ensure the node-side SkillBinsCache is cold or expired so evaluateSystemRunPolicyPhase() blocks in await opts.skillBins.current() after resolveExecApprovals() has already loaded the approvals snapshot (src/node-host/invoke-system-run.ts:379-392; src/node-host/runner.ts:116-145).
  3. Connect one identity-bearing operator session with operator.write and a second with operator.admin, then issue a write-scoped node.invoke system.run request for the already-allowlisted command.
  4. While that request is paused, use the admin session to call exec.approvals.node.set and remove the matching allowlist entry. The admin update is freshness-checked with baseHash before system.execApprovals.set saves the new file (src/gateway/server-methods/exec-approvals.ts:160-191; src/node-host/invoke.ts:389-408).
  5. Let the original system.run continue. recordAllowlistMatchesUse() updates lastUsedAt on the stale in-memory file object and saves that older snapshot back to disk (src/node-host/invoke-system-run.ts:637-647; src/infra/exec-approvals.ts:870-894), restoring the revoked entry for subsequent write-scoped node.invoke requests.

Demonstrated Impact

The admin mutation path and the execution-side metadata path use different consistency rules. exec.approvals.node.set forwards to system.execApprovals.set, which reloads the current file, requires a matching baseHash, and only then saves (src/gateway/server-methods/exec-approvals.ts:160-191; src/node-host/invoke.ts:389-408). The execution path does not re-read or compare hashes: it keeps the older ExecApprovalsFile returned by resolveExecApprovals(), then recordAllowlistUse() mutates that stale object and calls saveExecApprovals(approvals) directly (src/infra/exec-approvals.ts:870-894).

The privilege split is real in the shipped code and docs. Gateway method authorization admits node.invoke with operator.write (src/gateway/server-methods.ts:51-77; src/gateway/method-scopes.ts:129-151), but exec.approvals.node.set stays admin-only under the reserved exec.approvals.* namespace (src/shared/gateway-method-policy.ts:1-20). OpenClaw's docs also state that explicit device-token scopes remain authoritative and that node exec policy is enforced by the node's own approvals layer (docs/gateway/protocol.md:626-628; docs/gateway/operator-scopes.md:91-100).

This is conditional on a real async gap, not speculative. The stale window exists whenever the node pauses after loading approvals and before metadata writeback, such as a cold SkillBinsCache refresh on headless nodes. Existing controls do not close that gap because the gateway only strips forged approval override fields from node.invoke; it does not make the later node-side metadata write transactional (src/gateway/node-invoke-system-run-approval.ts:78-83).

Environment

Verified against OpenClaw v2026.4.19-beta.2 at commit 744c48496360029bc063ab7a83559047a7dddb05, published at 2026-04-19T05:55:49Z. The reproducer needs a paired node that exposes system.run, an existing allowlisted command for the target agent, autoAllowSkills: true, and two identity-bearing operator sessions with operator.write and operator.admin scopes.

Remediation Advice

Make execution-side approval metadata updates compare against the latest on-disk approvals hash before saving. Reload-and-merge lastUsedAt updates under a lock, or drop metadata writeback entirely when the underlying exec approvals file changed after policy evaluation began.

<!-- submission-marker:CA-oyi-node-exec-approvals-race-restores-revoked-policy-2-dedup -->

Vote matrix · Quick signals

Works
Did the solution work? Tap to confirm.
Easy Fix
Was it a quick fix?
Time Saver
Did it save you time?
Blocking
Was it severely blocking?
Common Issue
Are others likely hitting this too?
Flaky / Intermittent
Is it intermittent?
Verified / Reproducible
Can you reproduce it reliably?
Loading…

Still need to ship something?

×6

Another batch ranked right after the header list — different links, same matching logic.

Back to top recommendations

TRENDING