[backport] Fix dart-sass failure on Windows usernames containing ampersand#14535
[backport] Fix dart-sass failure on Windows usernames containing ampersand#14535cderv wants to merge 2 commits into
Conversation
…gression (#14534) The v1.9 introduction of safeWindowsExec broke dart-sass for users whose Windows profile path contains `&` (e.g., C:\Users\Tom & Jerry\). The ampersand is a cmd.exe command separator, so the temp .bat wrapper that safeWindowsExec writes is split mid-path and fails. The existing #14367 fallback to direct sass.bat execution does not help: Windows still spawns cmd.exe internally to interpret any .bat file, and the ampersand splits again. Backport of the dart.exe + sass.snapshot direct-invocation rewrite from PR #14273 (commit 4a7b6ce). Calling dart.exe through Deno.Command bypasses cmd.exe entirely and removes the whole class of .bat issues on Windows. The bundled dart-sass version is identical between v1.9 and main (DARTSASS=1.87.0), so the src/dart.exe + src/sass.snapshot layout is the same on disk. This also subsumes the #14367 enterprise group-policy fallback (no .bat = no policy block) and reinforces the #14267 accented-path fix already backported via #14274. Out of scope: other safeWindowsExec callers (texlive, zip, shell) still go through cmd.exe and remain vulnerable to ampersand in paths. Those have separate fixes upstream and are not covered by this backport.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Cover the exact path pattern that broke v1.9 (e.g. C:\Users\Tom & Jerry\) to prevent re-regression. Mirrors the existing spaced/accented tests by junctioning the real dart-sass install dir into an ampersand-containing parent directory and asserting compilation succeeds. The junction itself is created via cmd /c mklink — Deno.Command passes each arg as a separate CreateProcess argument, so the ampersand in the target path is not interpreted by cmd.exe as a command separator.
|
@cscheid I am thinking of backporting this fix that I initially did not backport because it was bigger change that I would expect for a backport. However it impacts RStudio users (#14534) for which quarto was working in v1.8 and no more in v1.8. It was reported in our repo from a report on RStudio IDE by @gtritchie. It is scoped to Windows user only, and should not create problem because it is improvement but... 🤷♂️ you never know. What do you think ? This is so next RStudio IDE could maybe bundle a patch v1.9 before we do release a stable v1.10. @gtritchie if we do a patch release with such fix, is there greater probability to have a v1.9 patch bundled into next RStudio IDE ? Anyhow, this would be available for user to download stable themself. @cscheid I just prefered to have your go on this before merging. (Reminder: the fix is already in v1.10.3 following #14273) |
|
I think a backport is ideal here, assuming everything goes smoothly. But I also think that using |
| * dart-sass.ts | ||
| * | ||
| * Copyright (C) 2020-2022 Posit Software, PBC | ||
| * Copyright (C) 2020-2025 Posit Software, PBC |
There was a problem hiding this comment.
If we're going to fix this, it's 2026 :D
There was a problem hiding this comment.
Took me a long time to convince Claude Code that it was 2026. I don't think it believed me.
|
The upcoming release of RStudio (2026.05) is already locked and loaded, we can't take changes for anything unless incredibly serious. Handling ampersands in usernames would not meet the bar, IMO. Having a patched 1.9 would be nice since we could tell impacted users to install that separately, without having to downgrade to 1.8. We are doing monthly releases of RStudio now, and I could incorporate this into 2026.06 if available in the next few weeks. |
4 participants
Important
Backport from #14273
When the Windows user profile path contains
&(e.g.,C:\Users\Tom & Jerry\),quarto renderof any document with sass fails with'C:\Users\Tom' is not recognized as an internal or external command. The ampersand is acmd.execommand separator, so the temp.batwrapper thatsafeWindowsExecwrites is split mid-path and the second half is interpreted as a separate command.Root Cause
v1.9 introduced
safeWindowsExecto handle spaced paths (#13997), which writes a temp.batand runs it throughcmd.exe /c. Any&in the program path letscmd.exesplit the line. The existing #14367 fallback to directsass.batexecution does not help — Windows still spawnscmd.exeinternally to interpret any.bat, and the ampersand splits again there.Fix
Backport of the
dart.exe+sass.snapshotdirect-invocation rewrite from #14273. Callingdart.exeviaDeno.Commandbypassescmd.exeentirely, removing the whole class of.bat-related issues on Windows (ampersand splitting, OEM/UTF-8 code page mismatch, enterprise group policy blocks).The bundled dart-sass version is identical between v1.9 and main (
DARTSASS=1.87.0), so thesrc/dart.exe+src/sass.snapshotlayout assumed by the fix is the same on disk in v1.9.This subsumes the #14367 fallback approach (no
.bat= no policy block) and reinforces the #14267 accented-path fix already backported via #14274.Out of scope: other
safeWindowsExeccallers (texlive.ts,zip.ts,shell.ts) still go throughcmd.exeand remain vulnerable to&in paths. Those have separate upstream fixes and are not covered by this backport.Test Plan
&QUARTO_DART_SASSenv override pointing to a customsass.exeunit/dart-sass.test.tsandunit/windows-exec.test.tspass on Windows CIFixes #14534