Skip to content

fix(mxc): PowerShell fails in the Windows node sandbox - #1147

Merged
shanselman merged 4 commits into
mainfrom
docs/gateway-node-exec-faq
Aug 11, 2026
Merged

fix(mxc): PowerShell fails in the Windows node sandbox#1147
shanselman merged 4 commits into
mainfrom
docs/gateway-node-exec-faq

Conversation

@shanselman

@shanselman shanselman commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Closes #1143

What Problem This Solves

Fixes an issue where users running common Windows commands such as PowerShell, whoami, and tasklist through the Windows node would receive startup failures or empty output because the MXC sandbox always blocked Win32k system calls.

Why This Change Was Made

Adds an explicit, secure-default Allow Windows UI APIs option to Node Sandbox settings. The option remains off by default, requires a confirmation dialog, keeps all presets UI-denied, and preserves filesystem, network, clipboard, timeout, and command-approval controls when enabled.

User Impact

Users can opt in to run PowerShell and other Win32k-dependent console tools inside MXC containment instead of disabling the sandbox entirely. When the option is off, gateway-shaped direct-argv PowerShell requests fail closed with actionable guidance.

Evidence

  • The setting persists, appears with localized warning copy in all supported locales, and is included in the live node settings snapshot.
  • Local MCP invocation returned OPENCLAW_MXC_UI_OK, exit code 0, through mxc-direct-appc.
  • Existing gateway invocation returned OPENCLAW_GATEWAY_MXC_UI_OK, exit code 0, after the normal Windows Allow Once prompt.
  • Formal isolated Gateway → Windows node → MXC validation passed all 17 tests, including PowerShell and denied-write containment.
  • Tray diagnostics showed contained=True, allowWindowsUi=True, uiAllowWindows=True, and containment=mxc.

Change Type

  • Bug fix
  • Feature
  • Refactor
  • Docs or instructions
  • Tests or validation
  • Security hardening
  • Chore or infrastructure

Scope

  • Tray or WinUI UX
  • Windows node capability
  • Local MCP or winnode
  • Gateway, connection, or pairing
  • Setup or onboarding
  • Permissions, privacy, or security
  • Tests, CI, or docs

Validation

  • .\build.ps1 - passed; all product projects built and 46 Markdown files validated.
  • dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore - 3,638 passed, 32 gated integration tests skipped, 0 failed.
  • dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore - 2,249 passed, 0 failed.
  • .\scripts\validate-mxc-e2e.ps1 - 17 passed, 0 failed. Required proofs reported: mirrored WSL-safe gateway port, PowerShell through real Gateway/Windows node/MXC, and denied writes to the tray data directory.
  • python .agents\skills\autoreview\scripts\autoreview --mode branch --base origin/main - clean on merged head, no accepted/actionable findings.
  • Rubber-duck review - stale proof-name gate and node snapshot risks checked; node snapshot omission was found by live MCP proof and fixed with a regression guard.

Real Behavior Proof

  • Environment tested: Windows 11, MXC 0.7 processcontainer/AppContainer, fresh isolated WSL gateway and existing OpenClaw gateway.
  • PR head or commit tested: 14073bf6.
  • Exact steps or command run: .\scripts\validate-mxc-e2e.ps1; local winnode system.run; raw gateway node.invoke with powershell.exe -NoProfile -Command "Write-Output OPENCLAW_GATEWAY_MXC_UI_OK".
  • Evidence after fix: formal E2E Test Run Successful. Total tests: 17, Passed: 17; existing gateway payload { stdout: "OPENCLAW_GATEWAY_MXC_UI_OK", stderr: "", exitCode: 0, success: true, durationMs: 638 }.
  • Observed result: PowerShell executed inside MXC after attended approval; diagnostics reported executor=mxc-direct-appc, contained=True, uiAllowWindows=True, and containment=mxc.
  • Screenshot or artifact links verified? N/A. Visible UI, cancellation rollback, confirmation flow, enabled warning, and accessible toggle state were verified with computer-use.
  • Not verified or blocked: None for the changed behavior.

Security Impact

  • New permissions or capabilities? Yes, explicit opt-in to Win32k system calls inside MXC.
  • Secrets or tokens handling changed? No.
  • New or changed network calls? No.
  • Command or tool execution surface changed? Yes.
  • Data access scope changed? No.
  • If any answer is Yes, explain the risk and mitigation: the Win32k boundary is relaxed only after an attended confirmation. The setting defaults off, all presets force it off, and a persistent warning remains visible while enabled. Filesystem, network, clipboard, timeout, and command approvals remain enforced.

Compatibility and Migration

  • Backward compatible? Yes.
  • Config or environment changes? Yes, one optional persisted boolean that defaults to false.
  • Migration needed? No.
  • If yes, list the exact upgrade steps: N/A.

Review Conversations

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

Add a secure-default, confirmed sandbox setting for Win32k-dependent tools such as PowerShell, wire it through node execution, and prove the local MCP and gateway paths.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 68c2f965-c837-4547-bb9b-75626d6725d4
Prove gateway-shaped PowerShell argv requests receive the structured fail-closed guidance when Windows UI access is disabled.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 68c2f965-c837-4547-bb9b-75626d6725d4
Retain the existing CI proof identifier while updating its behavior to validate PowerShell under the explicit Windows UI opt-in.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 68c2f965-c837-4547-bb9b-75626d6725d4
@clawsweeper

clawsweeper Bot commented Aug 11, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Aug 11, 2026
@clawsweeper

clawsweeper Bot commented Aug 11, 2026

Copy link
Copy Markdown

Codex review: found issues before merge. Reviewed August 10, 2026, 9:00 PM ET / August 11, 2026, 01:00 UTC.

ClawSweeper review

What this changes

This PR adds a default-off, confirmed Node Sandbox setting that permits Win32k UI system calls so MXC-contained PowerShell and similar Windows tools can run.

Merge readiness

⚠️ Needs maintainer review before merge - 5 items remain

Keep open. The PR addresses the linked MXC command failure with credible current-head proof, but it introduces a malformed-direct-argv validation regression and requires explicit maintainer acceptance of a persisted Win32k boundary relaxation.

Priority: P1
Reviewed head: 14073bf61ed356e8e7ffc0188633f4b05a392d4e
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Strong real MXC proof supports the core behavior, but a concrete input-validation regression and a security-boundary decision remain before merge.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (live_output): The PR body includes current-head after-fix Gateway and local MXC output, including a successful direct-argv PowerShell invocation and a 17-test formal MXC E2E run.
Patch quality 🦐 gold shrimp (3/6) Security review found an item that needs attention.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The PR body includes current-head after-fix Gateway and local MXC output, including a successful direct-argv PowerShell invocation and a 17-test formal MXC E2E run.
Evidence reviewed 6 items Current main behavior: Current main hardcodes the MXC policy to deny Windows UI APIs, matching the reported failure boundary.
PR policy wiring: The proposed policy maps the persisted setting to UI access while continuing to disable input injection.
Production MXC boundary: The MXC executor serializes the generated configuration and passes it to wxc-exec, so the policy change directly affects the external containment runtime.
Findings 1 actionable finding [P2] Preserve empty direct-argv validation
Security Needs attention Persisted Win32k relaxation broadens command capability: The new setting flows directly into the MXC UI policy, allowing desktop UI isolation for every sandboxed command while persisted. Input injection remains disabled, but this is still a material reduction of the existing syscall boundary.

How this fits together

Gateway and local MCP command requests enter the Windows node, which builds an MXC sandbox policy before starting a contained process and returning its result. The new setting controls whether that policy retains Win32k blocking or allows desktop UI APIs.

flowchart LR
A[Gateway or local MCP command] --> B[Windows node]
B --> C[Sandbox settings snapshot]
C --> D{Windows UI APIs enabled?}
D -->|No| E[PowerShell fails closed]
D -->|Yes| F[MXC contained execution]
F --> G[Command result and diagnostics]
Loading

Decision needed

Question Recommendation
Should OpenClaw accept a persisted, attended opt-in that relaxes MXC Win32k syscall blocking for all sandboxed commands in exchange for PowerShell and console-tool compatibility? Accept the default-off opt-in: Keep the confirmation-gated setting and its remaining sandbox controls, after the direct-argv validation regression is repaired.

Why: This is a deliberate containment-policy change, not a purely mechanical bug repair, so the acceptable security posture needs maintainer intent.

Before merge

  • Preserve empty direct-argv validation (P2) - An argv: [] request now takes the false branch of the count check while the local shell variable remains null, so this call throws NullReferenceException. Before this change it reached DirectArgvCommandLine.Build and returned the controlled “requires an executable” validation error; keep the direct-argv branch separate and add a regression test.
  • Resolve security concern: Persisted Win32k relaxation broadens command capability - The new setting flows directly into the MXC UI policy, allowing desktop UI isolation for every sandboxed command while persisted. Input injection remains disabled, but this is still a material reduction of the existing syscall boundary.
  • Resolve merge risk (P1) - When enabled, this persisted setting changes every sandboxed command from container UI isolation to desktop UI isolation, relaxing the Win32k boundary until the operator turns it off.
  • Complete next step (P2) - A maintainer must decide whether the persisted Win32k relaxation is acceptable; the contributor should also repair the identified direct-argv validation regression.

Findings

  • [P2] Preserve empty direct-argv validation — src/OpenClaw.Shared/Mxc/MxcConfigBuilder.cs:82-84
  • [medium] Persisted Win32k relaxation broadens command capability — src/OpenClaw.Shared/Mxc/MxcPolicyBuilder.cs:118
Agent review details

Security

Needs attention: The patch deliberately relaxes an MXC containment boundary when an operator opts in, so explicit maintainer acceptance is needed despite its default-off confirmation flow.

Review metrics

Metric Value Why it matters
Change surface 24 files affected, +456/-22 The patch spans policy, persisted settings, WinUI, five localizations, documentation, and runtime coverage.
Runtime versus tests runtime/UI +338, tests +108, docs +10 The implementation adds a cross-layer setting with substantial focused regression and E2E coverage.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #1143
Summary: This PR is the explicit candidate implementation for the Windows-node MXC UI-denial failure.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Repair and explicitly accept the boundary (recommended)
    Restore controlled empty-argv rejection, then merge only after a maintainer accepts the default-off Win32k relaxation.
  2. Retain mandatory Win32k blocking
    Pause this PR if the project does not want a persisted desktop-UI mode for contained commands.

Technical review

Best possible solution:

Preserve the default-off confirmed opt-in, retain filesystem, network, clipboard, timeout, and command-approval controls, and repair the malformed-argv guard before a maintainer accepts the containment tradeoff.

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

Yes. On an MXC-enabled Windows host, invoke direct-argv PowerShell through system.run while the default UI-deny policy is active; current main hardcodes that deny, and the PR provides a matching Gateway-to-node proof path.

Is this the best way to solve the issue?

Unclear. The default-off confirmed setting is a focused compatibility path with real runtime proof, but a maintainer must accept the broader UI boundary and the patch must preserve controlled handling of empty direct argv.

Full review comments:

  • [P2] Preserve empty direct-argv validation — src/OpenClaw.Shared/Mxc/MxcConfigBuilder.cs:82-84
    An argv: [] request now takes the false branch of the count check while the local shell variable remains null, so this call throws NullReferenceException. Before this change it reached DirectArgvCommandLine.Build and returned the controlled “requires an executable” validation error; keep the direct-argv branch separate and add a regression test.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 582a44713860.

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes current-head after-fix Gateway and local MXC output, including a successful direct-argv PowerShell invocation and a 17-test formal MXC E2E run.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

  • P1: The linked report shows a broken Windows-node command workflow for common PowerShell and console tools.
  • merge-risk: 🚨 security-boundary: Enabling the new setting intentionally permits Win32k calls for all contained system.run commands.
  • 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 (live_output): The PR body includes current-head after-fix Gateway and local MXC output, including a successful direct-argv PowerShell invocation and a 17-test formal MXC E2E run.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes current-head after-fix Gateway and local MXC output, including a successful direct-argv PowerShell invocation and a 17-test formal MXC E2E run.

Evidence

Security concerns:

  • [medium] Persisted Win32k relaxation broadens command capability — src/OpenClaw.Shared/Mxc/MxcPolicyBuilder.cs:118
    The new setting flows directly into the MXC UI policy, allowing desktop UI isolation for every sandboxed command while persisted. Input injection remains disabled, but this is still a material reduction of the existing syscall boundary.
    Confidence: 0.98

What I checked:

Likely related people:

  • Caleb Eden: Local blame attributes the current main UI-deny policy to the available d0fca93 history boundary, and later main work updated related MXC approval behavior. (role: earliest available policy-history contributor; confidence: medium; commits: d0fca93d5f61, c9608c8e54e6; files: src/OpenClaw.Shared/Mxc/MxcPolicyBuilder.cs, src/OpenClaw.Shared/Mxc/MxcConfigBuilder.cs)
  • shanselman: Prior merged history includes SandboxPage and MXC work, and the current branch carries the proposed end-to-end policy, UI, and proof changes. (role: recent sandbox-page and MXC contributor; confidence: medium; commits: cd02defd9942, 4166e0fd63f8; files: src/OpenClaw.Tray.WinUI/Pages/SandboxPage.xaml.cs, src/OpenClaw.Shared/Mxc/MxcConfigBuilder.cs)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Restore the controlled empty-direct-argv rejection and cover it with a regression test.
  • Obtain explicit maintainer acceptance of the persisted Win32k opt-in.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

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

History

Review history (1 earlier review cycle)
  • reviewed 2026-08-11T00:26:11.531Z sha 81d5e67 :: needs maintainer review before merge. :: none

Resolve MXC command and testing documentation conflicts while preserving the current main approval proofs and the PowerShell UI-access proof.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 68c2f965-c837-4547-bb9b-75626d6725d4
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Aug 11, 2026
@shanselman

Copy link
Copy Markdown
Collaborator Author

Maintainer decision: accept the narrow, default-off, attended Win32k UI-access opt-in. Filesystem, network, clipboard, timeout, and command approvals remain enforced. Formal isolated MXC validation passed 17/17, and all required PR checks are green.

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

Labels

merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P1 Urgent regression or broken agent/channel workflow affecting real users now. proof: sufficient Contributor real behavior proof is sufficient. 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.

Projects

None yet

1 participant