Skip to content

fix(cli): harden Windows node pairing and approvals#88163

Closed
vincentkoc wants to merge 46 commits into
mainfrom
fix-windows-node-token-rotation
Closed

fix(cli): harden Windows node pairing and approvals#88163
vincentkoc wants to merge 46 commits into
mainfrom
fix-windows-node-token-rotation

Conversation

@vincentkoc

@vincentkoc vincentkoc commented May 29, 2026

Copy link
Copy Markdown
Member

Summary

  • preserve stable Windows node/device identity across reconnects and same-key metadata refresh approvals, while still rotating unsafe/stale/revoked/mismatched device tokens
  • harden node pairing approval authz, pending/paired node presentation, and gateway node-event routing so Windows nodes are less likely to fall back to manual/TUI repair paths
  • route devices/nodes/gateway CLI RPC helpers through direct local gateway-client auth for explicit loopback shared token/password calls, without letting stale inactive env/config tokens suppress device-token fallback
  • make host-native Windows exec approval get/set payloads work through the dedicated exec.approvals.node.* gateway methods, including native payloads that omit payloadJSON
  • block raw node.invoke systemExecApprovals.* / system.execApprovals.* policy mutation paths and keep exec policy mutation on audited dedicated methods
  • render host-native Windows node exec approval snapshots read-only in Control UI, including normalization for malformed node-supplied rules
  • expose host-native node approval snapshots in the public gateway protocol schema so generated/protocol consumers accept the Windows-native response shape
  • clarify the nodes invoke system.run CLI error so users are pointed to agent /exec host=node, not a nonexistent openclaw exec command
  • make build/package TypeScript helper scripts run through the repo tsx loader so Node builds without built-in TypeScript support can complete

Draft Snapshot

Current pushed draft head: be9eeea5ba88c6adb45c652482ab5019dc71229a.

This remains draft for maintainer review, not because of a known validation failure.

Validation

  • Rebased onto origin/main at e681569536ea89787031b8cb512af6491c460f49 and resolved current gateway/node-pairing/protocol/UI test conflicts.
  • Focused local regression passed after the rebase: 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.ts passed 8 shards / 236 tests. The first sparse UI shard failed because ui/config/control-ui-chunking.ts was absent from the sparse checkout; after adding ui/config to the sparse checkout, ui/src/ui/control-ui-vite-config.node.test.ts passed 1 shard / 4 tests.
  • Docs sanity passed: node scripts/docs-list.js; git diff --check passed.
  • Final autoreview passed at current head: .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main reported autoreview clean: no accepted/actionable findings reported. It ignored one out-of-scope Feishu finding outside this PR.
  • Changed gate was retried through Crabbox AWS: run_5dfd73c4262e, lease cbx_d33e2b4a9972, provider aws, command env 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 error src/channels/message/ingress-queue.test.ts(54,29): Property 'payload' does not exist..., introduced outside this PR by the current origin/main channel ingress queue work.
  • Blacksmith Testbox changed gate is currently blocked by local auth: blacksmith auth login required.
  • Native Windows and WSL2 Crabbox reruns are currently blocked by runner infrastructure. Native Windows raw Azure lease run_9ec55498b7e2 / cbx_4d4de9d86020 had 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 7180da4daf5939e2d94a4bf427ca9aa4ee2b932e

  • Signed prerelease used: openclaw-windows-node release v0.6.0-alpha.14, asset OpenClawCompanion-Setup-x64.exe, digest sha256:36405bd72e2c11bae0aa051b551b339982a66ffb59820d977ad9983dbf0b4d98.
  • Linux pond gateway checkout was refreshed to exact PR head 7180da4daf5939e2d94a4bf427ca9aa4ee2b932e, rebuilt successfully with CI=true PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN=false pnpm build, and restarted on the active pond gateway port.
  • Runtime reported OpenClaw 2026.5.31 (7180da4) and /health returned {"ok":true,"status":"live"}.
  • Pond config drift was remediated by setting gateway.port to the active gateway port; default CLI probes work without manual --url / --token overrides.
  • Pairing proof passed after exact-head restart: 0 pending, 2 paired, 2 connected Windows nodes: Windows Node (cbxcbx2a25b4920) and Windows Node (cbxcbx58d8b308d).
  • Exec approval proof passed on both connected Windows nodes: approvals get --node <id> --json returned enabled=true, defaultAction=deny, rules=22, and host-native policy data.
  • Exec boundary proof passed on both nodes: raw gateway call node.invoke for system.execApprovals.get is rejected with node.invoke does not allow system.execApprovals.*; use exec.approvals.node.*.
  • Command proof passed on both nodes: nodes invoke system.which resolved hostname, cmd, and powershell; direct gateway call node.invoke for system.run with hostname returned exit code 0 and each node hostname.
  • Feature smoke passed on both nodes: device.info, device.status, camera.list, and screen.snapshot returned successfully; screen snapshots produced 1024x768 PNG payloads of about 328 KB each.
  • Installed Windows app/signing proof passed on both nodes: running OpenClaw.Tray.WinUI.exe reports product version 0.6.0-alpha.14+Branch.master.Sha.eb06fba21c44b5d2220e83992b8da59bfcc1e67a..., file version 0.6.0.0, Authenticode Valid, signer OpenClaw Foundation, thumbprint FBBE0F15D46DBD9C99DBEBD1BC4C8EA0311BE4DA.
  • Installer signing proof passed on both nodes: OpenClawCompanion-Setup-x64.exe is Authenticode Valid with the same OpenClaw signer/thumbprint.

Release Signing State

  • openclaw-windows-node CI signs release executables/installers with Azure Trusted Signing endpoint https://eus.codesigning.azure.net/, signing account openclaw, certificate profile openclaw.
  • Azure CLI reports Trusted Signing account openclaw in resource group openclawwinnode, location eastus; certificate profile openclaw is Active / PublicTrust.
  • GitHub release v0.6.0-alpha.14 is published as a prerelease and contains x64 setup, arm64 setup, and x64 zip assets with GitHub SHA256 digests.

Continuation Probe - 2026-05-31

  • Fresh Azure run-command probe found the Linux pond gateway live at exact PR head 7180da4daf5939e2d94a4bf427ca9aa4ee2b932e, with /health returning {"ok":true,"status":"live"}.
  • The pond user's ~/.openclaw/openclaw.json had 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.
  • Re-ran default CLI probes without manual --url / --token: nodes list --json reported 2 paired and connected Windows nodes.
  • Re-ran native approval and feature smoke on both nodes: approvals are host-native, enabled, defaultAction=deny, 22 rules; system.which resolves hostname, cmd, and powershell; device.info, device.status, camera.list, and screen.snapshot all returned successfully.
  • Re-ran direct node execution boundary: raw gateway call node.invoke for system.execApprovals.get is blocked on both nodes with node.invoke does not allow system.execApprovals.*; use exec.approvals.node.*; direct system.run for hostname returned exit code 0 on both nodes.

Continuation Signing Probe - 2026-05-31

  • Fresh Azure PowerShell proof on both current Windows pond nodes shows one running OpenClaw.Tray.WinUI.exe process per node.
  • Installed app product version on both nodes is 0.6.0-alpha.14+Branch.master.Sha.eb06fba21c44b5d2220e83992b8da59bfcc1e67a...; file version is 0.6.0.0.
  • Installed app Authenticode is Valid on both nodes, signer OpenClaw Foundation, thumbprint FBBE0F15D46DBD9C99DBEBD1BC4C8EA0311BE4DA.
  • C:\Windows\Temp\OpenClawCompanion-Setup-x64.exe on both nodes has SHA256 36405bd72e2c11bae0aa051b551b339982a66ffb59820d977ad9983dbf0b4d98, matching the published GitHub release asset digest, and Authenticode Valid with the same signer/thumbprint.

Notes

  • During earlier pond validation, a previous root-launched gateway had left pairing files owned by root. Ownership was corrected and the gateway was relaunched as the pond user; startup then had no pairing-file permission warnings.
  • The stale local gateway on the old port was intentionally left untouched; the active pond proof uses the configured active port.

@openclaw-barnacle openclaw-barnacle Bot added size: M maintainer Maintainer-authored PR labels May 29, 2026
@clawsweeper

clawsweeper Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed June 21, 2026, 1:50 AM ET / 05:50 UTC.

Summary
The branch changes Windows node pairing/device identity, gateway and CLI loopback auth, exec-approval RPC/protocol/UI handling, docs, scripts, and regression tests.

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.

  • Protocol/API Surfaces: 1 node-pairing param added; exec snapshot and node-set schemas widened. These are public gateway protocol shapes, so generated consumers and upgrade compatibility need explicit review before merge.
  • Loopback Auth Entry Paths: 4 CLI/RPC helper paths changed. Gateway, devices, nodes, and runtime RPC helpers can switch identity mode for proven loopback shared auth.

Stored data model
Persistent data-model change detected: migration/backfill/repair: src/cli/program.nodes-basic.e2e.test.ts, migration/backfill/repair: src/infra/device-pairing.test.ts, serialized state: src/cli/devices-cli.test.ts, serialized state: src/gateway/server.node-invoke-approval-bypass.test.ts, serialized state: src/gateway/server/ws-connection/message-handler.ts, unknown-data-model-change: packages/gateway-protocol/src/schema/exec-approvals.ts, and 3 more. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

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

Rank-up moves:

  • [P2] Tighten native snapshot validation and add reject tests for empty or metadata-only snapshots.
  • Resolve current conflicts and refresh review on the exact merge result.
  • [P2] Record maintainer acceptance of the compatibility, auth, and security-boundary upgrade tradeoffs before merge.

Risk before merge

  • [P1] Native exec-approval snapshots can validate without any real policy field, so generated consumers may accept responses the Control UI cannot classify or render correctly.
  • [P1] The branch is draft and merge-conflicting, so maintainers need a fresh exact-merge-result review after rebase/conflict resolution.
  • [P1] Existing raw node.invoke tooling that mutates system.execApprovals.* will fail after merge and must move to exec.approvals.node.*.
  • [P1] Stable Windows nodes with stale, unbound, revoked, or mismatched device metadata may be forced through re-pairing or quarantine during upgrade.
  • [P1] Proven loopback shared token/password CLI calls switch from CLI/device identity to backend gateway-client identity with narrowed scopes, which can affect local automation that assumed CLI identity.

Maintainer options:

  1. Tighten And Refresh Before Merge (recommended)
    Require a real native snapshot policy field, add reject coverage, rebase the branch, and rerun review on the exact merge result.
  2. Accept The Upgrade Tradeoff
    Maintainers may intentionally accept node re-pairing/quarantine and raw node.invoke blocking if the replacement RPC and upgrade impact are documented for affected tooling.
  3. Split Narrowly First
    Maintainers can land the narrower exec-approval advertisement work through fix(gateway): advertise exec approval node commands #88296 while this broader hardening branch stays under review.

Next step before merge

  • [P2] Manual maintainer review is needed because this is a protected draft with merge conflicts, a concrete protocol validator defect, and compatibility/auth/security-boundary tradeoffs.

Security
Cleared: No lockfile, dependency source, CI, or supply-chain file change is present in the live PR file list; the intentional auth and exec-boundary changes remain merge risks, not concrete security defects.

Review findings

  • [P2] Require a real native snapshot shape — packages/gateway-protocol/src/schema/exec-approvals.ts:93-103
Review details

Best 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:

  • [P2] Require a real native snapshot shape — packages/gateway-protocol/src/schema/exec-approvals.ts:93-103
    NativeExecApprovalsSnapshotSchema has only optional fields, so {}, hash-only, or constraints-only payloads can pass validateExecApprovalsSnapshot. The Control UI only classifies native snapshots when rules, defaultAction, or enabled exists, so protocol clients can accept a response the UI cannot render correctly; require at least one real policy field and add reject tests.
    Confidence: 0.89

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4b2298e8cbcb.

Label changes

Label justifications:

  • P2: This is a normal-priority Windows node/gateway hardening PR with bounded but real compatibility and auth-boundary impact.
  • merge-risk: 🚨 compatibility: The PR can break raw node.invoke exec-approval mutation tooling and can force some stable Windows node pairings through re-approval or quarantine.
  • merge-risk: 🚨 auth-provider: The PR changes proven loopback token/password CLI calls from CLI/device identity to backend gateway-client identity with narrowed scopes.
  • merge-risk: 🚨 security-boundary: The PR intentionally tightens device binding, node approval RPC routing, and exec-approval mutation boundaries.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (logs): The PR body includes real Windows pond logs for signed connected Windows nodes, exec approval reads, raw node.invoke boundary rejection, and feature smoke, though exact-head native Windows/WSL proof remains a merge-risk decision.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes real Windows pond logs for signed connected Windows nodes, exec approval reads, raw node.invoke boundary rejection, and feature smoke, though exact-head native Windows/WSL proof remains a merge-risk decision.
Evidence reviewed

PR surface:

Source +1163, Tests +2756, Docs +8, Config 0, Other 0. Total +3927 across 68 files.

View PR surface stats
Area Files Added Removed Net
Source 28 1461 298 +1163
Tests 36 2778 22 +2756
Docs 2 9 1 +8
Config 1 3 3 0
Generated 0 0 0 0
Other 1 6 6 0
Total 68 4257 330 +3927

What I checked:

  • Repository policy read: Root and scoped AGENTS.md files for gateway, gateway server methods, UI, docs, scripts, tests, and TUI were read; the protected maintainer-label rule and compatibility/security-boundary guidance apply. (AGENTS.md:14, 4b2298e8cbcb)
  • Live PR state: GitHub reports the PR is open, draft, author-assigned, carries the maintainer label, and is merge-conflicting/dirty at head 568c7c7. (568c7c729aeb)
  • Current main does not replace the central change: Current main still omits NODE_EXEC_APPROVALS_COMMANDS from SYSTEM_COMMANDS and DESKTOP_HOST_COMMANDS, so the exec-approval command advertisement part of the branch is not already implemented on main. (src/gateway/node-command-policy.ts:55, 4b2298e8cbcb)
  • Protocol validator defect at PR head: NativeExecApprovalsSnapshotSchema makes every native snapshot field optional and unions it into ExecApprovalsSnapshotSchema, allowing empty or metadata-only snapshots. (packages/gateway-protocol/src/schema/exec-approvals.ts:93, 568c7c729aeb)
  • UI classifier mismatch: The PR-head Control UI only treats a snapshot as native when rules, defaultAction, or enabled exists, which diverges from the looser protocol validator. (ui/src/ui/controllers/exec-approvals.ts:108, 568c7c729aeb)
  • Test gap: The PR-head protocol tests accept a populated native snapshot and reject empty native set policies, but do not reject empty, hash-only, or constraints-only native snapshots. (packages/gateway-protocol/src/exec-approvals-validators.test.ts:54, 568c7c729aeb)

Likely related people:

  • vincentkoc: Current-main blame/log on node command policy, node pairing, protocol, and UI exec-approval paths points to Vincent Koc, and this maintainer draft is also authored by that account. (role: current fix owner and recent area contributor; confidence: high; commits: b43eedbb1818, 568c7c729aeb, 486d285c24f1; files: src/gateway/node-command-policy.ts, src/infra/node-pairing.ts, packages/gateway-protocol/src/schema/exec-approvals.ts)
  • steipete: Earlier exec-approval tooling, Control UI editor, and allowlist stabilization commits are by Peter Steinberger, which makes this a likely review/routing path for the protocol/UI invariant. (role: adjacent exec-approval feature history contributor; confidence: medium; commits: 3686bde783fd, 4de3c3a028ae, cad7ed1cb864; files: packages/gateway-protocol/src/schema/exec-approvals.ts, ui/src/ui/controllers/exec-approvals.ts, ui/src/ui/views/nodes-exec-approvals.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.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels May 29, 2026
@vincentkoc
vincentkoc force-pushed the fix-windows-node-token-rotation branch from 0597704 to 828773c Compare May 30, 2026 02:48
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels May 30, 2026
@openclaw-barnacle openclaw-barnacle Bot added the cli CLI command changes label May 30, 2026
@vincentkoc
vincentkoc force-pushed the fix-windows-node-token-rotation branch 2 times, most recently from 9515a69 to 791be73 Compare May 30, 2026 04:20
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 30, 2026
@vincentkoc
vincentkoc force-pushed the fix-windows-node-token-rotation branch from 791be73 to 432c9fe Compare May 30, 2026 04:41
@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels May 30, 2026
@vincentkoc vincentkoc changed the title fix(pairing): preserve stable device tokens on metadata refresh fix(cli): harden Windows node pairing and approvals May 30, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels May 30, 2026
@steipete

steipete commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: web-ui App: web-ui cli CLI command changes docs Improvements or additions to documentation gateway Gateway runtime maintainer Maintainer-authored PR merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. scripts Repository scripts size: XL status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants