diff --git a/news/changelog-1.9.md b/news/changelog-1.9.md index 240630d15f0..8fc84d8a18a 100644 --- a/news/changelog-1.9.md +++ b/news/changelog-1.9.md @@ -3,7 +3,7 @@ ## In this release - ([#14304](https://github.com/quarto-dev/quarto-cli/issues/14304)): Fix `quarto install tinytex` silently ignoring extraction failures. When archive extraction fails (e.g., `.tar.xz` on a system without `xz-utils`), the installer now reports a clear error instead of proceeding and failing with a confusing `NotFound` message. -- ([#14367](https://github.com/quarto-dev/quarto-cli/issues/14367)): Fix Dart Sass invocation failing on enterprise Windows systems where Group Policy blocks `.bat` execution from `%TEMP%`. When the `safeWindowsExec` temp wrapper is blocked, Quarto now falls back to calling `sass.bat` directly. +- ([#14367](https://github.com/quarto-dev/quarto-cli/issues/14367), [#14534](https://github.com/quarto-dev/quarto-cli/issues/14534)): Fix Dart Sass invocation failing on Windows when the user profile path contains `&` (e.g., `C:\Users\Tom & Jerry\`) and on enterprise systems where Group Policy blocks `.bat` execution from `%TEMP%`. Quarto now invokes the bundled `dart.exe` with `sass.snapshot` directly, bypassing `sass.bat` and `cmd.exe` entirely. - ([#14489](https://github.com/quarto-dev/quarto-cli/issues/14489)): Restore `--output-dir` support for `quarto preview` of single files when no `_quarto.yml` is present (e.g. R-package workspaces). Regression introduced in v1.9.18. ## In previous releases diff --git a/src/core/dart-sass.ts b/src/core/dart-sass.ts index 1d925889a06..1d08e2d68b9 100644 --- a/src/core/dart-sass.ts +++ b/src/core/dart-sass.ts @@ -1,20 +1,18 @@ /* * dart-sass.ts * - * Copyright (C) 2020-2022 Posit Software, PBC + * Copyright (C) 2020-2026 Posit Software, PBC */ import { join } from "../deno_ral/path.ts"; import { architectureToolsPath } from "./resources.ts"; import { execProcess } from "./process.ts"; -import { ProcessResult } from "./process-types.ts"; import { TempContext } from "./temp.ts"; import { lines } from "./text.ts"; import { debug, info } from "../deno_ral/log.ts"; import { existsSync } from "../deno_ral/fs.ts"; import { warnOnce } from "./log.ts"; import { isWindows } from "../deno_ral/platform.ts"; -import { requireQuoting, safeWindowsExec } from "./windows.ts"; export function dartSassInstallDir() { return architectureToolsPath("dart-sass"); @@ -60,17 +58,39 @@ export async function dartCompile( */ export interface DartCommandOptions { /** - * Override the sass executable path. - * Primarily used for testing with spaced paths. + * Override the dart-sass install directory. + * Used for testing with non-standard paths (spaces, accented characters). */ - sassPath?: string; + installDir?: string; } -export async function dartCommand( - args: string[], - options?: DartCommandOptions, -) { - const resolvePath = () => { +/** + * Resolve the dart-sass command and its base arguments. + * + * On Windows, calls dart.exe + sass.snapshot directly instead of going + * through sass.bat. The bundled sass.bat is a thin wrapper generated by + * dart_cli_pkg that just runs: + * "%SCRIPTPATH%\src\dart.exe" "%SCRIPTPATH%\src\sass.snapshot" %arguments% + * + * Template source: + * https://github.com/google/dart_cli_pkg/blob/main/lib/src/templates/standalone/executable.bat.mustache + * Upstream issue to ship standalone .exe instead of .bat + dart.exe: + * https://github.com/google/dart_cli_pkg/issues/67 + * + * Bypassing sass.bat avoids multiple .bat file issues on Windows: + * - Deno quoting bugs with spaced paths (#13997) + * - cmd.exe OEM code page misreading UTF-8 accented paths (#14267) + * - Enterprise group policy blocking .bat execution (#6651) + */ +function resolveSassCommand(options?: DartCommandOptions): { + cmd: string; + baseArgs: string[]; +} { + const installDir = options?.installDir; + if (installDir == null) { + // Only check env var override when no explicit installDir is provided. + // If QUARTO_DART_SASS doesn't exist on disk, fall through to use the + // bundled dart-sass at the default architectureToolsPath. const dartOverrideCmd = Deno.env.get("QUARTO_DART_SASS"); if (dartOverrideCmd) { if (!existsSync(dartOverrideCmd)) { @@ -78,77 +98,51 @@ export async function dartCommand( `Specified QUARTO_DART_SASS does not exist, using built in dart sass.`, ); } else { - return dartOverrideCmd; + return { cmd: dartOverrideCmd, baseArgs: [] }; } } + } - const command = isWindows ? "sass.bat" : "sass"; - return architectureToolsPath(join("dart-sass", command)); - }; - const sass = options?.sassPath ?? resolvePath(); + const sassDir = installDir ?? architectureToolsPath("dart-sass"); - // Process result helper (shared by Windows and non-Windows paths) - const processResult = (result: ProcessResult): string | undefined => { - if (result.success) { - if (result.stderr) { - info(result.stderr); - } - return result.stdout; - } else { - debug(`[DART path] : ${sass}`); - debug(`[DART args] : ${args.join(" ")}`); - debug(`[DART stdout] : ${result.stdout}`); - debug(`[DART stderr] : ${result.stderr}`); - - const errLines = lines(result.stderr || ""); - // truncate the last 2 lines (they include a pointer to the temp file containing - // all of the concatenated sass, which is more or less incomprehensible for users. - const errMsg = errLines.slice(0, errLines.length - 2).join("\n"); - throw new Error("Theme file compilation failed:\n\n" + errMsg); - } - }; - - // On Windows, use safeWindowsExec to handle paths with spaces - // (e.g., when Quarto is installed in C:\Program Files\) - // See https://github.com/quarto-dev/quarto-cli/issues/13997 if (isWindows) { - const quoted = requireQuoting([sass, ...args]); - const result = await safeWindowsExec( - quoted.args[0], - quoted.args.slice(1), - (cmd: string[]) => { - return execProcess({ - cmd: cmd[0], - args: cmd.slice(1), - stdout: "piped", - stderr: "piped", - }); - }, - ); - if (result.success) { - return processResult(result); - } - - // safeWindowsExec failed — fall back to direct execution (v1.8 behavior). - // Enterprise environments may block .bat execution from %TEMP% via - // Group Policy / AppLocker, causing the temp wrapper to fail. - // See https://github.com/quarto-dev/quarto-cli/issues/14367 - debug("[DART] safeWindowsExec failed, falling back to direct execution"); - const directResult = await execProcess({ - cmd: sass, - args, - stdout: "piped", - stderr: "piped", - }); - return processResult(directResult); + return { + cmd: join(sassDir, "src", "dart.exe"), + baseArgs: [join(sassDir, "src", "sass.snapshot")], + }; } - // Non-Windows: direct execution + return { cmd: join(sassDir, "sass"), baseArgs: [] }; +} + +export async function dartCommand( + args: string[], + options?: DartCommandOptions, +) { + const { cmd, baseArgs } = resolveSassCommand(options); + const result = await execProcess({ - cmd: sass, - args, + cmd, + args: [...baseArgs, ...args], stdout: "piped", stderr: "piped", }); - return processResult(result); + + if (result.success) { + if (result.stderr) { + info(result.stderr); + } + return result.stdout; + } else { + debug(`[DART cmd] : ${cmd}`); + debug(`[DART args] : ${[...baseArgs, ...args].join(" ")}`); + debug(`[DART stdout] : ${result.stdout}`); + debug(`[DART stderr] : ${result.stderr}`); + + const errLines = lines(result.stderr || ""); + // truncate the last 2 lines (they include a pointer to the temp file containing + // all of the concatenated sass, which is more or less incomprehensible for users. + const errMsg = errLines.slice(0, errLines.length - 2).join("\n"); + throw new Error("Theme file compilation failed:\n\n" + errMsg); + } } diff --git a/tests/unit/dart-sass.test.ts b/tests/unit/dart-sass.test.ts index 0e37241353c..6d5d3de86b9 100644 --- a/tests/unit/dart-sass.test.ts +++ b/tests/unit/dart-sass.test.ts @@ -2,7 +2,10 @@ * dart-sass.test.ts * * Tests for dart-sass functionality. - * Validates fix for https://github.com/quarto-dev/quarto-cli/issues/13997 + * Validates fixes for: + * https://github.com/quarto-dev/quarto-cli/issues/13997 (spaced paths) + * https://github.com/quarto-dev/quarto-cli/issues/14267 (accented paths) + * https://github.com/quarto-dev/quarto-cli/issues/6651 (enterprise .bat blocking) * * Copyright (C) 2020-2025 Posit Software, PBC */ @@ -13,46 +16,53 @@ import { isWindows } from "../../src/deno_ral/platform.ts"; import { join } from "../../src/deno_ral/path.ts"; import { dartCommand, dartSassInstallDir } from "../../src/core/dart-sass.ts"; +/** + * Helper: create a junction to the real dart-sass install dir at `targetDir`. + * Returns cleanup function to remove the junction. + */ +async function createDartSassJunction(targetDir: string) { + const sassInstallDir = dartSassInstallDir(); + const result = await new Deno.Command("cmd", { + args: ["/c", "mklink", "/J", targetDir, sassInstallDir], + }).output(); + + if (!result.success) { + const stderr = new TextDecoder().decode(result.stderr); + throw new Error(`Failed to create junction: ${stderr}`); + } + + return async () => { + await new Deno.Command("cmd", { + args: ["/c", "rmdir", targetDir], + }).output(); + }; +} + // Test that dartCommand handles spaced paths on Windows (issue #13997) -// The bug only triggers when BOTH the executable path AND arguments contain spaces. +// dart.exe is called directly, bypassing sass.bat and its quoting issues. unitTest( "dartCommand - handles spaced paths on Windows (issue #13997)", async () => { - // Create directories with spaces for both sass and file arguments const tempBase = Deno.makeTempDirSync({ prefix: "quarto_test_" }); const spacedSassDir = join(tempBase, "Program Files", "dart-sass"); const spacedProjectDir = join(tempBase, "My Project"); - const sassInstallDir = dartSassInstallDir(); + + let removeJunction: (() => Promise) | undefined; try { - // Create directories Deno.mkdirSync(join(tempBase, "Program Files"), { recursive: true }); Deno.mkdirSync(spacedProjectDir, { recursive: true }); - // Create junction (Windows directory symlink) to actual dart-sass - const junctionResult = await new Deno.Command("cmd", { - args: ["/c", "mklink", "/J", spacedSassDir, sassInstallDir], - }).output(); + removeJunction = await createDartSassJunction(spacedSassDir); - if (!junctionResult.success) { - const stderr = new TextDecoder().decode(junctionResult.stderr); - throw new Error(`Failed to create junction: ${stderr}`); - } - - // Create test SCSS file in spaced path (args with spaces) const inputScss = join(spacedProjectDir, "test style.scss"); const outputCss = join(spacedProjectDir, "test style.css"); Deno.writeTextFileSync(inputScss, "body { color: red; }"); - const spacedSassPath = join(spacedSassDir, "sass.bat"); - - // This is the exact bug scenario: spaced exe path + spaced args - // Without the fix, this fails with "C:\...\Program" not recognized const result = await dartCommand([inputScss, outputCss], { - sassPath: spacedSassPath, + installDir: spacedSassDir, }); - // Verify compilation succeeded (no stdout expected for file-to-file compilation) assert( result === undefined || result === "", "Sass compile should succeed (no stdout for file-to-file compilation)", @@ -62,14 +72,103 @@ unitTest( "Output CSS file should be created", ); } finally { - // Cleanup: remove junction first (rmdir for junctions), then temp directory try { - await new Deno.Command("cmd", { - args: ["/c", "rmdir", spacedSassDir], - }).output(); + if (removeJunction) await removeJunction(); + await Deno.remove(tempBase, { recursive: true }); + } catch (e) { + console.debug("Test cleanup failed:", e); + } + } + }, + { ignore: !isWindows }, +); + +// Test that dartCommand handles ampersand in paths (issue #14534) +// Windows user profile paths like C:\Users\Tom & Jerry\ broke dart-sass +// because the temp .bat wrapper written by safeWindowsExec was split on `&` +// by cmd.exe. Direct dart.exe invocation bypasses cmd.exe entirely. +unitTest( + "dartCommand - handles ampersand in paths (issue #14534)", + async () => { + const tempBase = Deno.makeTempDirSync({ prefix: "quarto_test_" }); + const ampSassDir = join(tempBase, "Tom & Jerry", "dart-sass"); + const ampProjectDir = join(tempBase, "Tom & Jerry", "project"); + + let removeJunction: (() => Promise) | undefined; + + try { + Deno.mkdirSync(join(tempBase, "Tom & Jerry"), { recursive: true }); + Deno.mkdirSync(ampProjectDir, { recursive: true }); + + removeJunction = await createDartSassJunction(ampSassDir); + + const inputScss = join(ampProjectDir, "style.scss"); + const outputCss = join(ampProjectDir, "style.css"); + Deno.writeTextFileSync(inputScss, "body { color: green; }"); + + const result = await dartCommand([inputScss, outputCss], { + installDir: ampSassDir, + }); + + assert( + result === undefined || result === "", + "Sass compile should succeed with ampersand in path", + ); + assert( + Deno.statSync(outputCss).isFile, + "Output CSS file should be created at ampersand path", + ); + } finally { + try { + if (removeJunction) await removeJunction(); + await Deno.remove(tempBase, { recursive: true }); + } catch (e) { + console.debug("Test cleanup failed:", e); + } + } + }, + { ignore: !isWindows }, +); + +// Test that dartCommand handles accented characters in paths (issue #14267) +// Accented chars in user paths (e.g., C:\Users\Sébastien\) broke when +// dart-sass was invoked through a .bat wrapper with UTF-8/OEM mismatch. +unitTest( + "dartCommand - handles accented characters in paths (issue #14267)", + async () => { + const tempBase = Deno.makeTempDirSync({ prefix: "quarto_test_" }); + const accentedSassDir = join(tempBase, "Sébastien", "dart-sass"); + const accentedProjectDir = join(tempBase, "Sébastien", "project"); + + let removeJunction: (() => Promise) | undefined; + + try { + Deno.mkdirSync(join(tempBase, "Sébastien"), { recursive: true }); + Deno.mkdirSync(accentedProjectDir, { recursive: true }); + + removeJunction = await createDartSassJunction(accentedSassDir); + + const inputScss = join(accentedProjectDir, "style.scss"); + const outputCss = join(accentedProjectDir, "style.css"); + Deno.writeTextFileSync(inputScss, "body { color: blue; }"); + + const result = await dartCommand([inputScss, outputCss], { + installDir: accentedSassDir, + }); + + assert( + result === undefined || result === "", + "Sass compile should succeed with accented path", + ); + assert( + Deno.statSync(outputCss).isFile, + "Output CSS file should be created at accented path", + ); + } finally { + try { + if (removeJunction) await removeJunction(); await Deno.remove(tempBase, { recursive: true }); } catch (e) { - // Best effort cleanup - log for debugging if it fails console.debug("Test cleanup failed:", e); } }