Skip to content

fix(gateway): return structured capability error when node lacks exec approvals#98823

Closed
yplongapp wants to merge 4 commits into
openclaw:mainfrom
yplongapp:fix/node-exec-approvals-capability-check
Closed

fix(gateway): return structured capability error when node lacks exec approvals#98823
yplongapp wants to merge 4 commits into
openclaw:mainfrom
yplongapp:fix/node-exec-approvals-capability-check

Conversation

@yplongapp

@yplongapp yplongapp commented Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown

What Problem This Solves

openclaw approvals get --node <node-id> --json currently fails with a raw JavaScript TypeError when the selected node does not advertise the exec approvals capability.

Observed output:

Cannot read properties of undefined (reading 'socket')

Why This Change Was Made

Two-part fix:

  1. Preserve exec-approval commands through reconciliation: Added NODE_EXEC_APPROVALS_COMMANDS (system.execApprovals.get, system.execApprovals.set) to SYSTEM_COMMANDS in node-command-policy.ts. Previously these commands were not in the allowlist, so gateway command reconciliation filtered them out of node.commands even when the node host advertised them.

  2. Gate on the effective approved surface: Check node.commands (the reconciled, approved command list) before invoking. If the node is connected but does not have the requested command in its effective surface, return a structured UNAVAILABLE error telling the operator which capability is missing.

Changes

  • src/gateway/node-command-policy.ts: Add NODE_EXEC_APPROVALS_COMMANDS to SYSTEM_COMMANDS (3 files, +11/-11 total)

Real behavior proof

Behavior addressed: Return structured UNAVAILABLE error instead of raw TypeError when node lacks exec approvals capability.
Environment tested: Node 22.x on Linux
Steps run after the patch: Called production error formatting function via node --import tsx
Evidence after fix:

The structured error produced by the fix code path:

{
  "code": "UNAVAILABLE",
  "message": "Node node-win-01 does not support exec approvals management. The node must advertise system.execApprovals.get, or the operator must edit the node host approvals file directly.",
  "details": {
    "nodeId": "node-win-01",
    "missingCommand": "system.execApprovals.get",
    "nodeCommands": ["system.run", "system.which", "system.run.prepare"]
  }
}

Test results (6/6 passed):

 ✓ returns a structured unavailable error when local approvals get cannot read state
 ✓ returns a structured unavailable error when local approvals set cannot persist
 ✓ rejects node exec approvals get when the node lacks the capability
 ✓ accepts node exec approvals get when the node advertises the capability
 ✓ accepts node exec approvals set when the node advertises the capability
 ✓ rejects node exec approvals set when the node lacks the capability

Observed result: All 6 tests pass. Code produces structured error with code, message, and details showing the missing command and the node's available commands.
Not tested: Live Docker-plus-Windows-node reproduction with a real node that lacks exec approvals commands.

Fixes #97578

@yplongapp
fix(gateway): return structured capability error when node lacks exec…
3a5a581 
… approvals

When openclaw approvals get --node <node-id> is run against a node that
does not advertise system.execApprovals.get or system.execApprovals.set,
the CLI previously exposed a raw "Cannot read properties of undefined
(reading 'socket')" TypeError that hid the real problem.

Check the node's declared commands before invoking. If the node is
connected but does not support the requested exec approvals command,
return a structured UNAVAILABLE error that tells the operator exactly
which capability is missing and how to resolve it.

Fixes openclaw#97578
@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: XS labels Jul 2, 2026
@clawsweeper

clawsweeper Bot commented Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: stale review; fresh review needed.

Summary
The latest durable ClawSweeper review was for head 3a5a581a942813a522bbc4c2b5daccea232970d6, but the PR head is now dddeee336b3a6be8dba1352fa7c28a97d678f2f2. Its old verdict and PR readiness labels are no longer current.

Next step
Run or wait for a fresh ClawSweeper review on the current PR head.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jul 2, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 2, 2026
Closed
4 tasks
@yplongapp
fix(gateway): use declaredCommands for node capability preflight; add…
c3abdc0 
… tests

ClawSweeper review identified two P1 issues:

1. Use node.declaredCommands instead of node.commands for the capability
   check, because gateway command reconciliation can filter out exec
   approvals commands from the effective surface even when the node
   advertised them.

2. Remove the early disconnected-node return so missing/not-connected
   nodes fall through to nodeRegistry.invoke which returns proper
   NOT_CONNECTED error with details.nodeError.

Add test coverage for all four combinations:
- node.get rejects when capability is missing
- node.get accepts when capability is present
- node.set rejects when capability is missing
- node.set accepts when capability is present
@yplongapp

yplongapp commented Jul 2, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added the merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. label Jul 2, 2026
@yplongapp
fix(gateway): preserve exec-approval commands in SYSTEM_COMMANDS; gat…
ab269a8 
…e on effective node surface

ClawSweeper identified that using node.declaredCommands could allow
forwarding exec-approval RPCs even when the command was not approved
for the session.

Fix:
1. Add NODE_EXEC_APPROVALS_COMMANDS to SYSTEM_COMMANDS in
   node-command-policy.ts so they survive gateway command reconciliation
   for node hosts that advertise them.
2. Check node.commands (the effective approved surface) instead of
   node.declaredCommands for the capability preflight.
@yplongapp

yplongapp commented Jul 2, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@yplongapp

yplongapp commented Jul 2, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@yplongapp
fix(infra): require operator.admin pairing scope for exec-approval co…
dddeee3 
…mmands

Adding NODE_EXEC_APPROVALS_COMMANDS to SYSTEM_COMMANDS made them
available through the default desktop command surface, but the pairing
authorization (resolveNodePairApprovalScopes) did not escalate them to
operator.admin scope. A node advertising exec-approval commands could
be paired without admin approval.

Check NODE_EXEC_APPROVALS_COMMANDS alongside NODE_SYSTEM_RUN_COMMANDS
in resolveNodePairApprovalScopes so both command groups require admin
scope for pairing approval.
@yplongapp

yplongapp commented Jul 2, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jul 2, 2026
@clawsweeper clawsweeper Bot removed merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 3, 2026
@steipete

steipete commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Superseded by the stronger landed fix #100505 / fccb888.

The landed version preserves capable desktop-node commands through pairing/reconnect and gates the dedicated relay on the effective approved surface plus the current Gateway allowlist. This covers the structured-error case without broadening raw node invocation.

Thank you @yplongapp for the implementation and evidence.

@steipete steipete closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

gateway Gateway runtime P2 Normal backlog priority with limited blast radius. size: M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

2 participants

@yplongapp @steipete