fix(cli): harden Windows node pairing and approvals#88163
Conversation
|
Codex review: found issues before merge. Reviewed June 21, 2026, 1:50 AM ET / 05:50 UTC. Summary PR surface: Source +1163, Tests +2756, Docs +8, Config 0, Other 0. Total +3927 across 68 files. Reproducibility: yes. source-reproducible. Current main lacks the exec-approval command allowlist additions and stable node device binding, while the PR body includes real Windows pond logs for the intended behavior; I did not run a live Windows node in this read-only review. Review metrics: 2 noteworthy metrics.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Tighten the native snapshot protocol shape, resolve the current conflicts, and land only after maintainers accept or document the Windows upgrade and auth-boundary behavior. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible. Current main lacks the exec-approval command allowlist additions and stable node device binding, while the PR body includes real Windows pond logs for the intended behavior; I did not run a live Windows node in this read-only review. Is this the best way to solve the issue? No, not merge-ready yet. The owner boundaries are plausible, but the native snapshot schema must be tightened and the compatibility/auth/security-boundary tradeoffs need maintainer acceptance after conflict resolution. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4b2298e8cbcb. Label changesLabel justifications:
Evidence reviewedPR surface: Source +1163, Tests +2756, Docs +8, Config 0, Other 0. Total +3927 across 68 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
0597704 to
828773c
Compare
9515a69 to
791be73
Compare
791be73 to
432c9fe
Compare
|
Closing as fully superseded after a selective maintainer replay.
The original fork branch is conflicting and does not allow maintainer edits, so it could not be refreshed in place. Contributor credit is preserved on the replacement implementation. Thank you, @vincentkoc, for finding and developing the Windows gaps. For future contributor PRs, enabling Allow edits by maintainers lets us repair the original branch directly. |
Summary
exec.approvals.node.*gateway methods, including native payloads that omitpayloadJSONnode.invoke systemExecApprovals.*/system.execApprovals.*policy mutation paths and keep exec policy mutation on audited dedicated methodsnodes invoke system.runCLI error so users are pointed to agent/exec host=node, not a nonexistentopenclaw execcommandtsxloader so Node builds without built-in TypeScript support can completeDraft Snapshot
Current pushed draft head:
be9eeea5ba88c6adb45c652482ab5019dc71229a.This remains draft for maintainer review, not because of a known validation failure.
Validation
origin/mainate681569536ea89787031b8cb512af6491c460f49and resolved current gateway/node-pairing/protocol/UI test conflicts.node scripts/run-vitest.mjs run src/tui/tui-command-handlers.test.ts src/gateway/call.test.ts src/gateway/server.node-invoke-approval-bypass.test.ts src/gateway/server.node-pairing-authz.test.ts src/infra/node-pairing.test.ts ui/src/ui/control-ui-vite-config.node.test.ts packages/gateway-protocol/src/exec-approvals-validators.test.ts src/cli/direct-loopback-gateway-auth.test.ts src/cli/gateway-rpc.runtime.test.ts src/cli/nodes-cli/register.invoke.approval-transport-timeout.test.tspassed 8 shards / 236 tests. The first sparse UI shard failed becauseui/config/control-ui-chunking.tswas absent from the sparse checkout; after addingui/configto the sparse checkout,ui/src/ui/control-ui-vite-config.node.test.tspassed 1 shard / 4 tests.node scripts/docs-list.js;git diff --checkpassed..agents/skills/autoreview/scripts/autoreview --mode branch --base origin/mainreportedautoreview clean: no accepted/actionable findings reported. It ignored one out-of-scope Feishu finding outside this PR.run_5dfd73c4262e, leasecbx_d33e2b4a9972, provideraws, commandenv OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 CI=1 PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN=false corepack pnpm check:changed. It is blocked by the current-main type errorsrc/channels/message/ingress-queue.test.ts(54,29): Property 'payload' does not exist..., introduced outside this PR by the currentorigin/mainchannel ingress queue work.blacksmith auth loginrequired.run_9ec55498b7e2/cbx_4d4de9d86020had no Node/Corepack before hydration. A hydrated retry could not start because the Azure coordinator reports monthly budget exceeded ($25000.49/$25000.00); an Azure Linux retry also hit low-priority-core quota. No after-rebase native Windows or WSL2 result is claimed here.Previous Windows Node Pond Proof - head
7180da4daf5939e2d94a4bf427ca9aa4ee2b932eopenclaw-windows-nodereleasev0.6.0-alpha.14, assetOpenClawCompanion-Setup-x64.exe, digestsha256:36405bd72e2c11bae0aa051b551b339982a66ffb59820d977ad9983dbf0b4d98.7180da4daf5939e2d94a4bf427ca9aa4ee2b932e, rebuilt successfully withCI=true PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN=false pnpm build, and restarted on the active pond gateway port.OpenClaw 2026.5.31 (7180da4)and/healthreturned{"ok":true,"status":"live"}.gateway.portto the active gateway port; default CLI probes work without manual--url/--tokenoverrides.Windows Node (cbxcbx2a25b4920)andWindows Node (cbxcbx58d8b308d).approvals get --node <id> --jsonreturnedenabled=true,defaultAction=deny,rules=22, and host-native policy data.gateway call node.invokeforsystem.execApprovals.getis rejected withnode.invoke does not allow system.execApprovals.*; use exec.approvals.node.*.nodes invoke system.whichresolvedhostname,cmd, andpowershell; directgateway call node.invokeforsystem.runwithhostnamereturned exit code 0 and each node hostname.device.info,device.status,camera.list, andscreen.snapshotreturned successfully; screen snapshots produced 1024x768 PNG payloads of about 328 KB each.OpenClaw.Tray.WinUI.exereports product version0.6.0-alpha.14+Branch.master.Sha.eb06fba21c44b5d2220e83992b8da59bfcc1e67a..., file version0.6.0.0, AuthenticodeValid, signerOpenClaw Foundation, thumbprintFBBE0F15D46DBD9C99DBEBD1BC4C8EA0311BE4DA.OpenClawCompanion-Setup-x64.exeis AuthenticodeValidwith the same OpenClaw signer/thumbprint.Release Signing State
openclaw-windows-nodeCI signs release executables/installers with Azure Trusted Signing endpointhttps://eus.codesigning.azure.net/, signing accountopenclaw, certificate profileopenclaw.openclawin resource groupopenclawwinnode, locationeastus; certificate profileopenclawisActive/PublicTrust.v0.6.0-alpha.14is published as a prerelease and contains x64 setup, arm64 setup, and x64 zip assets with GitHub SHA256 digests.Continuation Probe - 2026-05-31
7180da4daf5939e2d94a4bf427ca9aa4ee2b932e, with/healthreturning{"ok":true,"status":"live"}.~/.openclaw/openclaw.jsonhad drifted missing again. Restored the minimal local gateway config from the existing runtime token file without printing the token:gateway.mode=local,gateway.bind=lan,gateway.port=18790,gateway.auth.mode=token.--url/--token:nodes list --jsonreported 2 paired and connected Windows nodes.defaultAction=deny, 22 rules;system.whichresolveshostname,cmd, andpowershell;device.info,device.status,camera.list, andscreen.snapshotall returned successfully.gateway call node.invokeforsystem.execApprovals.getis blocked on both nodes withnode.invoke does not allow system.execApprovals.*; use exec.approvals.node.*; directsystem.runforhostnamereturned exit code 0 on both nodes.Continuation Signing Probe - 2026-05-31
OpenClaw.Tray.WinUI.exeprocess per node.0.6.0-alpha.14+Branch.master.Sha.eb06fba21c44b5d2220e83992b8da59bfcc1e67a...; file version is0.6.0.0.Validon both nodes, signerOpenClaw Foundation, thumbprintFBBE0F15D46DBD9C99DBEBD1BC4C8EA0311BE4DA.C:\Windows\Temp\OpenClawCompanion-Setup-x64.exeon both nodes has SHA25636405bd72e2c11bae0aa051b551b339982a66ffb59820d977ad9983dbf0b4d98, matching the published GitHub release asset digest, and AuthenticodeValidwith the same signer/thumbprint.Notes