[JiuYue0820]

What does this PR do?

On WSL, openExternalUrl opened http(s)/mailto links with cmd.exe /c start "" <url>. WSL binfmt interop only quotes argv elements that contain spaces. A URL never does, so the URL is concatenated unquoted into the cmd command line. & in a query string is then a command separator: a chat/Markdown link such as https://example.com/?a=1&calc&z=2 opens the first segment and runs calc on the Windows host.

The WSL branch now launches rundll32.exe url.dll,FileProtocolHandler <url>, which takes the URL as a single argument and does not re-parse it as a cmd line. Failure still falls back to shell.openExternal.

The argv helper lives in electron/wsl-open-url.ts so it can be unit-tested without booting Electron.

Related Issue

No existing issue or PR covers this cmd.exe /c start URL path. Filed as a PR directly.

Fixes #

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• apps/desktop/electron/wsl-open-url.ts: wslWindowsOpenUrlArgs() returns rundll32 FileProtocolHandler
• apps/desktop/electron/main.ts: WSL openExternalUrl uses that helper
• apps/desktop/electron/wsl-open-url.test.ts: asserts the helper never returns cmd.exe / start

How to Test

1. From apps/desktop: npx vitest run --project electron electron/wsl-open-url.test.ts (this worktree has no node_modules; the helper was checked by reading the source contract instead)
2. Manual on WSL Desktop: click https://example.com/?a=1&calc&z=2 — Windows must not launch calc; the URL should open in the host browser as a single address
3. A normal https://example.com/path link still opens

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• Focused helper test is in-tree. I could not run vitest here (desktop node_modules not present in the worktree). Source contract checked: main.ts no longer contains cmd.exe /c start for this path.
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: Windows 11 (source-level). I do not have a WSL Desktop session in this environment for a live click test.

Documentation & Housekeeping

• N/A — no config keys, no architecture docs, no tool schemas

For New Skills

N/A

Screenshots / Logs

N/A

---

AI assistance disclosure: found during a code review of apps/desktop/electron/main.ts; the fix, tests, and this description were prepared with AI assistance (Grok 4.6 via PokeAPI) and reviewed by the human submitter (FirmaSpring). Live WSL click-through was not re-run in this environment; the argv change matches the rundll32 FileProtocolHandler approach described in the review notes.

[andrexibiza]

Requesting changes. Removing cmd.exe /c start is the correct boundary fix, and rundll32.exe url.dll,FileProtocolHandler is a plausible WSL host launcher. The patch is not merge-ready yet:

1. The regression does not exercise the production WSL launch path.

The tests only assert that wslWindowsOpenUrlArgs() returns two constants. They never run or intercept the branch in openExternalUrl() that actually calls spawn(). main.ts could regress to cmd.exe /c start while these tests continued to pass unchanged.

isCmdStartUnsafeForUrl() is also dead production code used solely by its test; it does not protect the boundary.

Please extract the actual WSL launch operation behind injected spawn, fallback, and logging dependencies. The regression should call that production seam and assert that:

• the executable is not a command shell;
• the normalized URL is supplied as exactly one argv element;
• an https: URL containing &, |, ^, <, >, and % remains unchanged;
• the same guarantee holds for an allowed mailto: URL;
• the old cmd.exe /c start route cannot be selected.

Then remove isCmdStartUnsafeForUrl().

2. The fallback currently handles spawn failure, not launcher failure.

ChildProcess#error fires when the process cannot be spawned. If rundll32.exe starts but exits nonzero because dispatch fails, the child emits exit/close; this function has already returned true, and shell.openExternal() is never called.

Add a once-guarded fallback covering both 'error' and nonzero termination. Test:

• spawn error → fallback once;
• nonzero exit → fallback once;
• error followed by exit → still fallback once;
• successful exit → no fallback.

3. There is no executed verification yet.

The PR says Vitest was not run and there was no live WSL click-through. The workflow is currently action_required with no test jobs executed. Since this changes a host-command security boundary, source-level inspection is not enough.

Please rebase onto current main, resolve the existing conflict, run the Electron Vitest project and typecheck, and provide one real WSL proof using a metacharacter-bearing URL.

The launcher choice itself is not my blocker. Removing cmd reparsing closes the reported injection class. The blockers are proof through the real execution path, complete failure handling, and the current unmergeable repository state.

[JiuYue0820]

Addressed the review on #88558.

1. Production launch path
   openExternalUrl() now calls openWslExternalUrl() from electron/wsl-open-url.ts. Spawn, fallback, and logging are injected so the tests drive the same seam main.ts uses. isCmdStartUnsafeForUrl() is gone.

The regression asserts:

• the executable is not a command shell
• the normalized URL is exactly one argv element
• an https: URL containing &, |, ^, <, >, and % is unchanged
• the same holds for an allowed mailto: URL
• the old cmd.exe /c start route cannot be selected

2. Failure handling
   Fallback is once-guarded and covers both 'error' and nonzero 'exit'/'close'. Tests cover:

• spawn error -> fallback once
• nonzero exit -> fallback once
• error followed by exit -> still fallback once
• successful exit -> no fallback

3. Rebase and verification
   Rebased onto current main (e02d1e4) and resolved the main.ts import conflict (detectBundleSkew kept; openWslExternalUrl import placed with the other wsl-* siblings).

Local results on this Windows 11 host:

• npx tsc -p tsconfig.electron.json --noEmit — clean
• npx vitest run --project electron electron/wsl-open-url.test.ts — 7/7 passed
• Electron project run: 1387 passed; the 28 failures are existing Windows/SSH/path tests (ssh-connection, windows-hermes-path, stage-native-deps), not this launch path

WSL proof (Ubuntu-22.04):

wsl.exe -d Ubuntu-22.04 -- rundll32.exe url.dll,FileProtocolHandler "https://example.com/?a=1&calc&z=2"

Exit 0. No new calc.exe / CalculatorApp.exe process. The URL stayed one argv element; &calc& did not become a host command.

Happy to add a live Desktop click-through if you want that on top of the host-launcher proof.

[andrexibiza]

Re-review receipt — requested WSL boundary fixes are present at a87d06de

Verified the current head against the three review blockers.

• openExternalUrl() now delegates to the production openWslExternalUrl() seam with injected spawn, fallback, and logging dependencies; the old test-only isCmdStartUnsafeForUrl() helper is gone.
• The URL is passed as one argv element to a non-shell launcher, with regressions covering metacharacter-bearing HTTPS and mailto: values and excluding the old cmd.exe /c start route.
• Fallback is once-guarded across spawn error and nonzero exit/close, including the error-then-exit double-signal case; successful exit does not fall back.
• The author rebased onto e02d1e4, ran Electron typecheck and the focused 7-test suite, and supplied a real Ubuntu-22.04 WSL host-launcher proof showing &calc& was not interpreted as a host command.

The code-side review requirements are satisfied at this head. The remaining gate is repository execution: CI run 32138797910 and Docker run 32138797279 are both action_required, so GitHub has not run any checks against this exact candidate. I am not marking it merge-ready until those workflows execute successfully.