improve(setup): keep WSL operations inside the app-owned directory

[calebeden]

Additional instructions

MUST: Keep Allow edits from maintainers enabled for this PR so maintainers
can help update the branch when needed.

What Problem This Solves

Resolves a problem where developers running the standalone SetupEngine with a custom WSL distro name could cause cleanup or installation paths to resolve outside %LOCALAPPDATA%\OpenClawTray\wsl. Because cleanup recursively deletes the resolved path, a traversal value such as ..\.. could target an unintended directory.

No known user incident prompted this change. The packaged application supplies its fixed distro name after config/environment loading, so the exposed path is the repository/developer SetupEngine flow.

Why This Change Was Made

Setup now derives every WSL distro install path through one policy that validates the distro name, canonicalizes the path, requires it to be an immediate child of the app-owned wsl root, and rejects reparse points at the trusted local-data root or WSL root. Recursive cleanup revalidates ownership before every retry, and rollback unregisters the name-based distro before attempting guarded filesystem cleanup.

The remaining same-user check-to-use filesystem race is accepted as a non-goal: additional checks can narrow it, but fully atomic deletion or wsl --install would require handle-based operating-system support that these APIs do not expose.

User Impact

Developers invoking SetupEngine directly now receive a terminal validation error before distro-targeting WSL commands or recursive cleanup run when a distro name or install path is unsafe. Generic WSL availability preflight may run first. Shipped/default distro names continue to work unchanged; packaged application users should see no behavioral difference.

Evidence

The SetupEngine tests exercise real Windows junctions and verify that traversal values are rejected before distro-targeting commands run, paths outside the expected immediate child are never deleted, reparse-point ancestors are rejected, and a parent swapped to a junction during retry backoff is detected before the next recursive-delete attempt. The redirected sentinel file remains intact.

A current-head live Dev Box run additionally verified a valid WSL install and uninstall, followed by rejection of ..\.. with the outside sentinel preserved and no distro-targeting or recursive-delete operation logged.

Three independent dual-model review passes found no blocking correctness issue. The only recurring observation was the documented same-user check-to-use limitation.

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; Shared, CLI, WinNode CLI, SetupEngine, and WinUI built successfully
• dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore — 2,863 passed, 31 skipped, 0 failed
• dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore — 1,717 passed, 0 skipped, 0 failed
• dotnet test ./tests/OpenClaw.SetupEngine.Tests/OpenClaw.SetupEngine.Tests.csproj --no-restore — 443 passed, 0 skipped, 0 failed

Real Behavior Proof

• Environment tested: disposable Windows x64 Dev Box; .NET SDK 10.0.302
• PR head tested: 63e5a9b9cb3939b2e462c708d702c22e66ca8018
• Exact flow: built a temporary executable harness referencing OpenClaw.SetupEngine, ran valid headless setup and destructive uninstall against isolated paths under C:\OpenClawProof, then ran setup with distro name ..\.. and an outside sentinel
• Observed result: valid setup completed and registered OpenClawProof at C:\OpenClawProof\Local\wsl\OpenClawProof; uninstall removed both the registration and install path; the unsafe name exited 1 at cleanup-distro, preserved PRESERVE-ME, and logged zero distro-targeting commands and zero recursive distro deletions

Live Dev Box proof

1. Current branch, full head SHA, and successful runner build

2. Valid install: setup complete, registered distro, and canonical install path

3. Valid uninstall: distro registration and install path absent

4. Unsafe name: terminal rejection, preserved outside sentinel, and zero destructive operations

• Screenshot or artifact links verified? Yes — all four GitHub user-attachment links resolve
• Not verified or blocked: none for the changed live setup, cleanup, and unsafe-name behavior

Security Impact

• New permissions or capabilities? No
• 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: Existing wsl --install --location and recursive cleanup inputs are now constrained to a validated app-owned path. This reduces the execution/data risk; it does not add a new command or expand access. A same-user process can still theoretically race a filesystem check, which is documented as an accepted limitation.

Compatibility and Migration

• Backward compatible? No — intentionally stricter for custom distro names
• Config or environment changes? No
• Migration needed? No for shipped/default configurations
• If yes, list the exact upgrade steps: Custom developer configurations must use a 1–64 character distro name containing ASCII letters, digits, periods, underscores, or hyphens, beginning and ending with a letter or digit.

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.

[clawsweeper]

Codex review: found issues before merge. Reviewed July 17, 2026, 8:53 PM ET / July 18, 2026, 00:53 UTC.

Summary
The PR adds a centralized WSL distro install-path policy, routes setup cleanup and rollback deletion through it, documents the new ownership boundary, and adds traversal/reparse-point regression tests.

Reproducibility: yes. for the reported unsafe-path behavior: the new tests and current-head Dev Box proof exercise traversal rejection before distro-targeting and recursive-delete operations. The compatibility regression is source-reproducible because the new policy rejects any configured name outside its ASCII allowlist before cleanup begins.

Review metrics: 2 noteworthy metrics.

• Patch surface: 4 files affected; 455 added, 22 removed. Most of the change is a new deletion-boundary policy plus focused setup regression coverage, so the compatibility contract deserves explicit review.
• Behavior proof: 4 current-head terminal captures. The supplied Dev Box proof covers valid install, valid uninstall, and unsafe-name rejection on the proposed head.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster ✨ media proof bonus
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:

• Resolve the P1 compatibility finding by preserving safe legacy names or obtaining an explicit maintainer decision with upgrade coverage.

Risk before merge

• [P1] Existing developer or repository workflows using a safe custom WSL distro name that falls outside the new ASCII 1–64-character grammar can fail before cleanup, setup, or guarded deletion proceeds, without a migration or explicit compatibility decision.
• [P1] The policy improves containment but still documents an unavoidable same-user filesystem check-to-use race; maintainers should accept that residual boundary explicitly rather than treating the validation as atomic deletion protection.

Maintainer options:

1. Retain containment without a broad name break (recommended)
   Revise the policy to preserve safe existing custom names while retaining canonical-root, immediate-child, reparse-point, and retry-time ownership validation.
2. Accept and document the restriction
   Approve the new name contract only with clear upgrade behavior and tests showing how existing unsupported custom names are recovered without unsafe cleanup.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Preserve safe legacy custom distro names while rejecting only paths that cannot be proven to be an immediate non-reparse child of the app-owned WSL root; add regression coverage for a previously safe non-ASCII or otherwise legacy-compatible name.

Next step before merge

• [P2] A maintainer must choose the supported custom-distro-name compatibility contract before any narrow repair can be safely applied.

Maintainer decision needed

• Question: Should standalone SetupEngine continue accepting existing safe custom WSL distro names, or should the project intentionally restrict all such names to the new ASCII 1–64-character grammar?
• Rationale: The path-safety objective is clear, but the PR changes an existing developer-facing configuration contract and can strand legacy cleanup or setup flows; source inspection and tests cannot choose that compatibility policy safely.
• Likely owner: calebeden — They authored the present policy and can clarify the intended SetupEngine compatibility boundary, although a project owner must make the final product decision.
• Options:
  • Preserve safe custom names (recommended): Keep canonical containment and reparse-point protections while allowing existing names that resolve to a safe immediate child of the app-owned WSL root.
  • Approve the narrower contract: Accept the ASCII-only restriction as an intentional breaking change and add explicit upgrade, cleanup, and compatibility coverage for prior custom names.

Security
Cleared: No new dependency, permission, secret, network, or supply-chain exposure was found; the patch narrows existing WSL install and recursive-cleanup path handling, subject to the documented non-atomic same-user filesystem race.

Review findings

• [P1] Preserve safe existing custom distro names — src/OpenClaw.SetupEngine/DistroInstallPathPolicy.cs:19

Review details

Best possible solution:

Preserve already-safe custom distro names through canonical immediate-child containment and reparse-point checks, rejecting only names or paths that resolve outside the app-owned root; adopt the narrower name grammar only if maintainers explicitly choose that compatibility break and document an upgrade path.

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

Yes for the reported unsafe-path behavior: the new tests and current-head Dev Box proof exercise traversal rejection before distro-targeting and recursive-delete operations. The compatibility regression is source-reproducible because the new policy rejects any configured name outside its ASCII allowlist before cleanup begins.

Is this the best way to solve the issue?

No, not yet: centralized canonical containment and reparse validation are the right safety mechanism, but an unconditional broad name allowlist is not the narrowest solution until maintainers approve breaking existing safe custom-name workflows.

Full review comments:

• [P1] Preserve safe existing custom distro names — src/OpenClaw.SetupEngine/DistroInstallPathPolicy.cs:19
  The new allowlist rejects every configured distro name outside its new ASCII 1–64-character grammar before cleanup/setup runs. The PR acknowledges this as a compatibility break, but it provides neither a migration nor maintainer-approved contract change; retain path containment checks while preserving safe legacy names, or obtain an explicit decision and upgrade path before merge.
  Confidence: 0.95

Overall correctness: patch is incorrect
Overall confidence: 0.95

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

• add proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides current-head Dev Box terminal screenshots covering a successful isolated install, uninstall, and unsafe-name rejection with the outside sentinel preserved; this is appropriate direct proof for native setup and filesystem behavior.
• add proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The PR body provides current-head Dev Box terminal screenshots covering a successful isolated install, uninstall, and unsafe-name rejection with the outside sentinel preserved; this is appropriate direct proof for native setup and filesystem behavior.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (screenshot): The PR body provides current-head Dev Box terminal screenshots covering a successful isolated install, uninstall, and unsafe-name rejection with the outside sentinel preserved; this is appropriate direct proof for native setup and filesystem behavior.
• remove status: 📣 needs proof: Current PR status label is status: ⏳ waiting on author.
• remove rating: 🦪 silver shellfish: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

• P2: This is meaningful setup and filesystem-boundary hardening, but the stated exposure is the standalone developer SetupEngine path and no user incident is reported.
• merge-risk: 🚨 compatibility: The policy intentionally rejects previously possible custom distro-name configurations without a migration or approved compatibility contract.
• merge-risk: 🚨 security-boundary: The diff changes the validation boundary protecting recursive cleanup and WSL install locations, where an incorrect policy could affect destructive-path safety.
• 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 (screenshot): The PR body provides current-head Dev Box terminal screenshots covering a successful isolated install, uninstall, and unsafe-name rejection with the outside sentinel preserved; this is appropriate direct proof for native setup and filesystem behavior.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides current-head Dev Box terminal screenshots covering a successful isolated install, uninstall, and unsafe-name rejection with the outside sentinel preserved; this is appropriate direct proof for native setup and filesystem behavior.
• proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The PR body provides current-head Dev Box terminal screenshots covering a successful isolated install, uninstall, and unsafe-name rejection with the outside sentinel preserved; this is appropriate direct proof for native setup and filesystem behavior.

Evidence reviewed

What I checked:

• Restrictive policy: The new policy rejects every custom distro name outside a 1–64-character ASCII allowlist before cleanup or setup operations can use the existing configuration, despite the PR explicitly describing this as intentionally backward incompatible. (src/OpenClaw.SetupEngine/DistroInstallPathPolicy.cs:19, 63e5a9b9cb39)
• Destructive-path coverage: Cleanup now obtains its WSL directory through the policy before listing, unregistering, or deleting the configured distro path, so a previously accepted custom name can become unable to complete its existing cleanup workflow. (src/OpenClaw.SetupEngine/SetupSteps.cs:243, 63e5a9b9cb39)
• Regression coverage: The branch adds focused policy and cleanup tests for missing names, traversal, path separators, canonical immediate-child containment, reparse points, and retry-time path substitution. (tests/OpenClaw.SetupEngine.Tests/SetupStepsTests.cs:52, 63e5a9b9cb39)
• Prior-review continuity: The only prior blocking finding was to preserve safe existing custom distro names; the PR head remains 63e5a9b and the new policy still enforces the same breaking name restriction. (src/OpenClaw.SetupEngine/DistroInstallPathPolicy.cs:19, 63e5a9b9cb39)
• Current-head behavior proof: The PR body supplies four linked Dev Box terminal captures for the exact head: build identification, valid install, valid uninstall, and traversal rejection with the external sentinel preserved and no destructive distro operations logged. (63e5a9b9cb39)
• Current-main status: The PR is open, unmerged, and cleanly mergeable against its base; the provided current-main SHA is 4518dd8, so the branch hardening is not an implemented-on-main close candidate. (4518dd87ed2b)

Likely related people:

• calebeden: Authored the two commits introducing the proposed path-policy boundary and the associated setup regression tests; no independent current-main area owner could be established from the available review context. (role: recent setup-path contributor; confidence: low; commits: e2c7889befba, 63e5a9b9cb39; files: src/OpenClaw.SetupEngine/DistroInstallPathPolicy.cs, src/OpenClaw.SetupEngine/SetupSteps.cs, tests/OpenClaw.SetupEngine.Tests/SetupStepsTests.cs)

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.

Review history (4 earlier review cycles)

• reviewed 2026-07-17T03:08:45.297Z sha 63e5a9b :: needs real behavior proof before merge. :: none
• reviewed 2026-07-17T03:49:10.119Z sha 63e5a9b :: needs real behavior proof before merge. :: [P1] Preserve safe existing custom distro names
• reviewed 2026-07-17T04:29:54.013Z sha 63e5a9b :: needs real behavior proof before merge. :: [P1] Preserve safe existing custom distro names
• reviewed 2026-07-18T00:12:57.437Z sha 63e5a9b :: needs real behavior proof before merge. :: [P1] Preserve safe existing custom distro names

[calebeden]

Marking as draft until I can further test this tomorrow

[calebeden]

@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: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The durable review comment was published and its verdict was queued for target-serialized routing.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/29624084791
• Updated: 2026-07-18T00:57:34.644Z

[shanselman]

GitHub Copilot maintainer-assistant review — automated feedback, not written personally by Scott.

Reviewed head: 63e5a9b9cb3939b2e462c708d702c22e66ca8018

The containment policy is well designed: canonical immediate-child checks, tight distro-name allowlisting, reparse-point ancestor rejection, retry-time revalidation, and real junction-based TOCTOU tests are strong. There is no existing repository path-policy utility that should replace this owner.

The remaining merge blocker is compatibility for existing custom distro names. Previously accepted names containing spaces or non-ASCII characters can now fail before replacement, cleanup, rollback, or uninstall filesystem work. That can strand an existing managed installation even though the new-install policy is safer.

Please choose and prove a migration contract:

1. Preferably allow legacy existing distro records to be safely located/cleaned using canonical containment checks while enforcing the stricter name allowlist only for newly created distros; or
2. Provide a guided migration/recovery path with actionable errors and tests for legacy names such as OpenClaw Gateway and a non-ASCII example.

Add integration-style tests for replacement and uninstall/rollback of a legacy custom-name distro, proving nothing outside the app-owned root is touched. Also add boundary coverage for exactly 64 vs 65 characters and a rollback test asserting invalid/traversal names fail closed without deleting external paths.

The first-use TOCTOU limitation is documented and acceptable for this PR, but the legacy-name upgrade/uninstall path needs a concrete supported behavior before merge.