feat: replace legacy exec approvals with V2

[bkudiess]

Summary

• Replace the legacy Windows exec-approval policy and prompt path with the V2 coordinator, store, and hardened dialog introduced by feat: add exec approval prompt dialog for system.run on the Windows node #1045.
• Make low-level system.run and system.run.prepare canonical argv-only, matching the upstream gateway contract.
• Execute canonical argv through Local and MXC runners, including cmd-aware serialization for cmd.exe /d /s /c.
• Port macOS parity hardening for secret redaction, display bounds, bypass detection, one-shot decisions, policy currency, audit-suppression approval, path display, and attended-desktop detection.
• Align fresh effective policy with macOS: empty allowlist, prompt on miss when attended, deny fallback when unattended. Malformed, untrusted, and migration-failed paths remain hard deny.
• Make the Windows V1 cutover explicit: legacy command-text rules are left untouched but are not evaluated or converted; prior allows require attended V2 reapproval and unattended calls deny.
• Move Permissions policy editing onto the App-owned V2 store with single-read hashing, CAS, explicit UI deltas, alternate-state path support, and remote grant restrictions.
• Remove the legacy V1 policy, prompt service, settings flag, tests, and runtime fallback.

Architecture ownership

• Old owner: SystemCapability, ExecApprovalPolicy, and ExecApprovalPromptService split approval text matching, prompting, and shell execution across parallel paths.
• New owner: ExecApprovalsCoordinator owns authorization; ExecApprovalsStore owns policy persistence/CAS; ExecApprovalV2UiPromptHandler and ExecApprovalDialog own presentation; runners receive only the approved execution payload.
• Preserved invariant: the resolved canonical argv approved by V2 is the argv executed by the selected runner. Missing resolution, changed policy, unsupported transport, unsafe persistence, or presentation failure denies execution.

Behavior changes

• Low-level callers must send command as string[] argv. The normal gateway exec host=node path already wraps Windows command strings as ["cmd.exe","/d","/s","/c",command].
• Raw MCP, direct node.invoke, plugin, and winnode callers migrate from {"command":"echo hello","shell":"cmd"} to {"command":["cmd.exe","/d","/s","/c","echo hello"],"rawCommand":"echo hello"}. String requests return command-array-required.
• A fresh valid policy defaults to security=allowlist, ask=on-miss, askFallback=deny, and an empty allowlist. Only truly missing trustworthy state gets this default; invalid or unreadable active/legacy paths fail closed to security=deny.
• Non-empty custom env is rejected until environment values can be identity-bound and displayed safely.
• Shells and script/code interpreters can be approved once, but cannot receive reusable executable-only grants.
• Remote system.execApprovals.set may preserve/remove existing grants and preserve/tighten effective policy, but cannot add/change grants or weaken security, ask, askFallback, or autoAllowSkills.
• The active V2 store defaults to %APPDATA%\OpenClawTray\exec-approvals.json; OPENCLAW_STATE_DIR overrides it.

Upgrade contract

Windows V1 exec-policy.json command-text globs are intentionally retired without migration.

• V1 files remain byte-for-byte untouched during normal runtime.
• No V1 allow or deny rule is imported into V2.
• A fresh valid V2 policy uses an empty allowlist with prompt-on-miss and deny fallback.
• Prior V1 allows require attended V2 reapproval; unattended calls deny.
• Prior V1 denies are not imported and may be explicitly superseded by an attended V2 approval.
• Existing valid V2 state wins; invalid V2 state remains hard deny and is never replaced from V1.

Mechanical conversion and V1 runtime fallback were rejected because V1 authorized shell command text while V2 binds resolved executable identity and canonical argv. Conversion could widen narrow text rules into broad command-host grants, while dual evaluators would preserve parallel authorization paths.

Validation

• .\build.ps1
• dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore
  • 3,249 passed, 32 skipped, 0 failed
• dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore
  • 1,954 passed, 0 failed
• dotnet test .\tests\OpenClaw.WinNode.Cli.Tests\OpenClaw.WinNode.Cli.Tests.csproj --no-restore
  • 126 passed, 0 failed
• dotnet test .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj --no-restore
  • 697 passed, 0 failed
• dotnet test .\tests\OpenClaw.Tray.IntegrationTests\OpenClaw.Tray.IntegrationTests.csproj --no-restore
  • 19 passed, 0 failed
• .\scripts\validate-mxc-e2e.ps1
  • 2 passed, 0 failed, 0 skipped
• PowerShell parser validation passed for both modified uninstall scripts.

Real behavior proof

Gateway to Windows node to MXC

Canonical argv:

["cmd.exe","/d","/s","/c","echo OPENCLAW_GATEWAY_SYSTEM_RUN_MXC_SOURCE_READY && echo OPENCLAW_GATEWAY_SYSTEM_RUN_MXC_DENIED_PAYLOAD > \"C:\\Users\\...\\AppData\\Roaming\\OpenClawE2E\\...\\mxc-denied-write-....txt\""]

Observed:

SOURCE_READY: present
stderr: Access is denied.
exitCode: 1
fileExists: False
executor: mxc-direct-appc
contained: True
containment: mxc

The companion execution proof returned the expected marker with exit code 0 through the same contained path.

Local MCP

winnode --list-tools advertised:

system.run: command (string[] argv, required)

Canonical invocation returned:

{
  "stdout": "OPENCLAW_LOCAL_MCP_CANONICAL_ARGV_OK",
  "stderr": "",
  "exitCode": 0,
  "timedOut": false,
  "success": true
}

Runtime logs confirmed path=v2, decision=Allow, contained=True, and containment=mxc.

Upgrade characterization

Automated upgrade tests cover:

• V1 allow rule with no V2: V1 unchanged, unattended deny, attended prompt/Allow once.
• V1 deny rule with no V2: V1 unchanged, unattended deny, attended decision explicitly owns any override.
• Malformed V1: never parsed, remains unchanged, fresh V2 behavior applies.
• Existing valid V2 plus V1: V2 wins unchanged.
• Invalid V2 plus V1: hard deny; neither file is overwritten.

Adversarial review

Claude Opus 4.8 and GPT-5.6 Sol independently reviewed the complete diff through three rounds with cross-model challenge and re-review after fixes.

• Final Opus confidence: 95/100
• Final Sol confidence: 96/100
• Unresolved HIGH/MEDIUM findings: none
• Remaining actionable findings: none

[clawsweeper]

Codex review: found issues before merge. Reviewed July 28, 2026, 2:03 AM ET / 06:03 UTC.

ClawSweeper review

What this changes

This PR replaces Windows command-text approval rules with a V2 executable-identity approval system, makes low-level system.run argv-only, and sends approved argv to local or MXC-contained execution runners.

Merge readiness

⚠️ Needs maintainer review before merge - 7 items remain

Keep this PR open for maintainer review. The V2 authorization design and real MXC/MCP proof are substantial, but the branch intentionally breaks existing raw string-form system.run callers and retires persisted V1 policy semantics without maintainer-confirmed release handling; the collaborator-authored status also requires explicit human judgment rather than automated cleanup.

Priority: P1
Reviewed head: 25660b078d6c3fce84411f4a77f482e15f396cd2
Owner decision: Required. See Decision needed.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The implementation has unusually strong runtime proof and broad validation claims, but a P1 compatibility contract break and an unresolved upgrade-policy decision prevent a higher readiness rating.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (live_output): The PR body contains after-fix real gateway-to-Windows-node MXC execution and contained-denied-write output, plus a local MCP canonical-argv invocation with observed command output; redact any private paths or tokens in future proof updates.
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 contains after-fix real gateway-to-Windows-node MXC execution and contained-denied-write output, plus a local MCP canonical-argv invocation with observed command output; redact any private paths or tokens in future proof updates.
Evidence reviewed	6 items	Current PR contract change: The final PR head rejects a string-valued command with command-array-required, while the updated MCP and CLI documentation directs raw MCP, node.invoke, plugin, and winnode callers to send an explicit argv wrapper. Upgrade behavior is intentionally breaking: The PR body and upgrade characterization state that legacy exec-policy.json rules remain untouched but are neither evaluated nor migrated. Existing attended users can therefore move from a persisted V1 deny/allow decision to a fresh V2 prompt decision. Prior reviewer concern remains a release decision: The July 27 review explicitly required compatibility or documented, gated migration guidance for string-form calls and V1 policy retirement. The author described both as deliberate on July 28, but the supplied discussion contains no explicit maintainer approval of the compatibility break.
Findings	1 actionable finding	[P1] Preserve the supported string-form system.run contract
Security	Needs attention	V1 deny intent is no longer enforced after cutover: The branch deliberately stops evaluating or migrating persisted V1 deny rules. In attended operation, a command previously denied by configuration can become a new V2 prompt decision, so the authorization-boundary change needs an explicit maintainer-owned upgrade policy.

How this fits together

Gateway, MCP, CLI, and plugin callers send system.run requests to the Windows node. The node validates canonical argv, applies its locally stored approval policy, optionally presents a native approval dialog, then runs only the approved payload through a local or MXC-contained runner.

flowchart LR
  A[Gateway, MCP, CLI, plugins] --> B[Windows node request]
  B --> C[Canonical argv validation]
  C --> D[V2 approval policy]
  D --> E[Native approval dialog]
  D --> F[Approved execution payload]
  F --> G[Local or MXC runner]
  G --> H[Command result]

Loading

Decision needed

Question	Recommendation
Should the Windows node preserve compatibility for existing low-level string-form system.run callers and persisted V1 approval intent, or should this PR ship as an explicitly approved breaking V2 cutover?	Preserve boundary compatibility: Translate the existing supported string-plus-shell request form at the node boundary into canonical argv where safe, and show an explicit V1 reapproval migration notice while keeping V2 as the sole evaluator.

Why: The security model makes automatic V1 rule conversion unsafe, but rejecting a documented existing request shape and discarding prior policy semantics changes real user upgrades. The supplied discussion records the author’s rationale but not an approving maintainer decision.

Before merge

• Preserve the supported string-form system.run contract (P1) - The added early rejection makes every existing raw caller that sends command as a string fail with command-array-required. The gateway wrapper does not cover raw MCP, direct node.invoke, plugin, or winnode callers. Preserve a narrow boundary translation or obtain explicit maintainer approval for this versioned API break before merging.
• Resolve security concern: V1 deny intent is no longer enforced after cutover - The branch deliberately stops evaluating or migrating persisted V1 deny rules. In attended operation, a command previously denied by configuration can become a new V2 prompt decision, so the authorization-boundary change needs an explicit maintainer-owned upgrade policy.
• Resolve merge risk (P1) - Existing raw MCP, direct node.invoke, plugin, and winnode integrations that send a string command fail at runtime after upgrade instead of receiving a compatible translation.
• Resolve merge risk (P1) - Retiring V1 policy evaluation changes persisted authorization intent: unattended calls fail closed, but an attended V1 deny can become a new V2 approval decision.
• Resolve merge risk (P1) - The branch changes 80 files and the remaining CI matrix is still running, so final merge confidence should be refreshed against the exact head after the product/release decision.
• Complete next step (P2) - A maintainer must choose the public upgrade contract before any safe mechanical follow-up can be routed; this is not an autonomous repair candidate.

Findings

• [P1] Preserve the supported string-form system.run contract — src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2InputValidator.cs:17-22
• [medium] V1 deny intent is no longer enforced after cutover

Agent review details

Security

Needs attention: The V2 path appears designed to fail closed and the reported remote-policy weakening fix is focused, but retiring persisted V1 policy enforcement changes a security boundary and needs explicit upgrade acceptance.

Review metrics

Metric	Value	Why it matters
Authorization surface	80 files changed, 5,756 added, 6,902 removed	The PR replaces the core approval implementation, UI wiring, MCP contract, runners, uninstall handling, documentation, and test coverage in one change.
Runtime proof paths	2 real execution paths described	The PR includes gateway-to-MXC containment evidence and local MCP canonical-argv evidence, which materially improves confidence in the new execution path.

Merge-risk options

Maintainer options:

1. Restore a narrow compatibility boundary (recommended)
   Keep V2 as the only evaluator but translate supported legacy string requests to explicit argv and surface a clear V1-policy reapproval migration path before merge.
2. Accept an explicit breaking upgrade
   Approve the argv-only and V1-retirement contract only with maintainers owning the release communication and migration guidance for direct callers.
3. Pause pending product direction
   Do not merge the default-path cutover until the repository decides whether preserving existing caller and policy behavior is required.

Technical review

Best possible solution:

Keep the V2 executable-identity coordinator and MXC argv path, but either preserve the supported raw string-call boundary with a narrow canonicalization shim and visible V1 reapproval notice, or obtain explicit maintainer approval for a versioned breaking change with migration guidance and release communication.

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

Not applicable: this PR proposes a new default authorization architecture rather than fixing a reported current-main failure. The PR does provide high-signal live evidence for the changed MXC and MCP execution paths.

Is this the best way to solve the issue?

No, not yet. The V2 coordinator is a coherent security direction, but the default cutover is not the best merge-ready solution until maintainers decide whether existing low-level callers and stored V1 policy intent require a compatibility boundary or an explicitly accepted breaking-release path.

Full review comments:

• [P1] Preserve the supported string-form system.run contract — src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2InputValidator.cs:17-22
  The added early rejection makes every existing raw caller that sends command as a string fail with command-array-required. The gateway wrapper does not cover raw MCP, direct node.invoke, plugin, or winnode callers. Preserve a narrow boundary translation or obtain explicit maintainer approval for this versioned API break before merging.
  Confidence: 0.96

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

• P1: The proposed default cutover can break active external command-execution workflows immediately after upgrade.
• merge-risk: 🚨 compatibility: Rejecting string-form command requests and retiring V1 policy evaluation changes supported callers and persisted upgrade behavior.
• merge-risk: 🚨 security-boundary: Persisted V1 authorization constraints are removed from evaluation, so maintainers must explicitly own how prior deny and allow intent maps to V2.
• 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 contains after-fix real gateway-to-Windows-node MXC execution and contained-denied-write output, plus a local MCP canonical-argv invocation with observed command output; redact any private paths or tokens in future proof updates.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body contains after-fix real gateway-to-Windows-node MXC execution and contained-denied-write output, plus a local MCP canonical-argv invocation with observed command output; redact any private paths or tokens in future proof updates.

Evidence

Security concerns:

• [medium] V1 deny intent is no longer enforced after cutover
  The branch deliberately stops evaluating or migrating persisted V1 deny rules. In attended operation, a command previously denied by configuration can become a new V2 prompt decision, so the authorization-boundary change needs an explicit maintainer-owned upgrade policy.
  Confidence: 0.9

What I checked:

• Current PR contract change: The final PR head rejects a string-valued command with command-array-required, while the updated MCP and CLI documentation directs raw MCP, node.invoke, plugin, and winnode callers to send an explicit argv wrapper. (src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2InputValidator.cs:17, 25660b078d6c)
• Upgrade behavior is intentionally breaking: The PR body and upgrade characterization state that legacy exec-policy.json rules remain untouched but are neither evaluated nor migrated. Existing attended users can therefore move from a persisted V1 deny/allow decision to a fresh V2 prompt decision. (src/OpenClaw.Shared/ExecApprovals/ExecApprovalsStore.cs:69, 25660b078d6c)
• Prior reviewer concern remains a release decision: The July 27 review explicitly required compatibility or documented, gated migration guidance for string-form calls and V1 policy retirement. The author described both as deliberate on July 28, but the supplied discussion contains no explicit maintainer approval of the compatibility break.
• Concrete remote-policy weakening fix appears addressed: The final commit is titled fix: prevent remote exec policy weakening; the author reports effective-policy comparisons and focused monotonicity tests across security, ask, askFallback, and autoAllowSkills. (src/OpenClaw.Shared/Capabilities/SystemCapability.cs:1, 25660b078d6c)
• After-fix runtime proof is provided: The PR body reports a real gateway-to-Windows-node MXC execution proof and a contained denied-write proof, plus a local MCP canonical-argv invocation with observed output. This is direct behavior evidence rather than only unit-test output. (tests/OpenClaw.E2ETests/Setup/MxcSetupAndConnectTests.cs:1, 25660b078d6c)
• Repository policy applies: The repository AGENTS.md requires MXC E2E validation for Windows node command-execution and exec-approval changes, and requires explicit UI/MCP proof practices. The PR reports the required build, shared/tray/CLI tests, and validate-mxc-e2e.ps1 run. (AGENTS.md:1, fe43b157b561)

Likely related people:

• AlexAlves87: Merged PR feat: add exec approval prompt dialog for system.run on the Windows node #1045 introduced the Windows V2 approval dialog and prompt-handling foundation that this PR extends into the default authorization path. (role: introduced the merged V2 dialog foundation; confidence: high; commits: 76f4d52c3b5e; files: src/OpenClaw.Tray.WinUI/Dialogs/ExecApprovalDialog.cs, src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2UiPromptHandler.cs)
• shanselman: The July 27 review identified the outstanding public-contract, upgrade-policy, and remote-policy concerns, including the requirement for explicit release handling of intentional breaks. (role: recent security and release-contract reviewer; confidence: high; files: src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2InputValidator.cs, src/OpenClaw.Shared/ExecApprovals/ExecApprovalsStore.cs)

Rank-up moves

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

• Obtain an explicit maintainer decision on the argv-only and V1-retirement upgrade contract.
• If compatibility is required, add the narrow translation or migration behavior with focused upgrade tests.
• Refresh the final review after the remaining CI matrix completes on the exact head.

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 (3 earlier review cycles)

• reviewed 2026-07-24T18:47:39.372Z sha 44c7a96 :: found issues before merge. :: [P1] Preserve configured approvals across upgrades
• reviewed 2026-07-27T06:59:41.984Z sha f7e2ae0 :: found issues before merge. :: [P1] Preserve existing approval constraints across upgrades | [P1] Keep compatibility for string-form system.run callers
• reviewed 2026-07-27T07:25:22.321Z sha c444067 :: found issues before merge. :: [P1] Preserve the supported string-form system.run contract | [P1] Preserve or explicitly migrate existing approval constraints

[bkudiess]

@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

• State: Review in progress
• Detail: The exact-review queue leased this run; Codex is reviewing the item.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/30244414687
• Updated: 2026-07-27T06:57:54.266Z

[bkudiess]

@clawsweeper re-review

[shanselman]

I am holding this below the repository's 90% merge-confidence bar after independent Opus and Codex reviews of the current head c4440674. Both reviewers confirmed the core V2 authorization path is fail-closed, but three unresolved release/security-contract issues remain:

1. String-form system.run compatibility is removed. Existing external callers using a string command (or safe custom env) now receive a hard error. Please preserve a boundary compatibility shim or document and gate the versioned contract break with migration guidance.
2. Persisted V1 allow/deny policy is ignored on upgrade. Existing unattended allows stop working, and prior explicit denies lose their configured intent. Since V1 text rules cannot safely become V2 resolved identities, please add a visible reapproval/migration notice or obtain explicit product sign-off for the breaking upgrade behavior.
3. Remote system.execApprovals.set can weaken prompt semantics for existing grants. ValidateExecApprovalsDelta prevents new allowlist patterns but allows ask: always to become on-miss/off with the same patterns, converting confirm-every-time entries into unattended execution. Please enforce immutable-or-more-restrictive ask, askFallback, and security changes and add focused tests.

The first two may be deliberate product decisions, but they need explicit release handling. The third is a concrete authorization-policy gap that should be fixed before merge.

[bkudiess]

@shanselman Thanks for the review. The concrete remote-policy weakening gap is fixed on the updated head:

• system.execApprovals.set now compares effective defaults, wildcard, and every agent scope.
• Remote updates may preserve or tighten security, ask, askFallback, and autoAllowSkills, but cannot weaken them.
• Existing locally-established Full values can round-trip while another axis tightens; lower policies cannot introduce Full.
• Focused monotonicity tests cover all four weakening axes, tighter updates, lower-to-Full rejection, and Full-preserving tightening.

Explicit product/release decision for the other two items:

1. Low-level system.run is intentionally V2 argv-only. The normal gateway exec host=node flow already emits canonical Windows argv. Raw MCP, direct node.invoke, plugin, and winnode callers must migrate from string+shell to explicit argv. Runtime now returns command-array-required, and README, architecture docs, and winnode skill include exact before/after migration examples. Non-empty custom env remains intentionally unsupported until it can be identity-bound and displayed safely.
2. Windows V1 policy is intentionally retired without migration. V1 command-text rules remain byte-for-byte untouched but are not evaluated or converted. Fresh valid V2 uses empty allowlist + prompt-on-miss + unattended deny. Upgrade characterization covers V1 allow, deny, malformed, valid-V2 precedence, and invalid-V2 hard deny. This break is accepted to avoid unsafe semantic conversion and parallel authorization engines, matching the macOS cutover approach.

Current validation: build green; Shared 3,249 passed / 32 skipped; Tray 1,954; CLI 126; SetupEngine 697; Tray Integration 19; MXC E2E 2/2 with true contained Access denied proof. Opus and Sol found no remaining actionable issues at 95% / 96% confidence.

Please reconsider the merge-confidence gate on the updated head.

[bkudiess]

@clawsweeper re-review

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.