diff --git a/apps/vscode/CHANGELOG.md b/apps/vscode/CHANGELOG.md index 8a4643db2..102790284 100644 --- a/apps/vscode/CHANGELOG.md +++ b/apps/vscode/CHANGELOG.md @@ -2,6 +2,8 @@ ## 1.139.0 (Unreleased) +- Fixed "Quarto: Preview" and render in the terminal when the document path contains shell special characters, such as parentheses, `;`, `@`, `$`, or quotes (). + - The extension now requires VS Code 1.101 or later. Positron is unaffected (). - In the visual editor, citations inserted into a `.bib` bibliography for software, graphics, songs, films, legislation, legal cases, interviews, personal communications, hearings, maps, figures and classics are now written as `@misc` entries instead of `@article` (). - The extension now reads YAML the way Quarto CLI does: merge keys (`<<`) in front matter and `_quarto.yml` are no longer resolved, and dates are read as strings (). diff --git a/apps/vscode/src/core/python.ts b/apps/vscode/src/core/python.ts index f54ac9129..03222341f 100644 --- a/apps/vscode/src/core/python.ts +++ b/apps/vscode/src/core/python.ts @@ -11,8 +11,6 @@ import child_process from "node:child_process"; import { Uri, extensions } from "vscode"; import { activeWorkspaceFolder } from "./workspace"; -import { shQuote } from "core"; - export async function activePythonInterpreter(uri?: Uri) { const workspaceFolder = activeWorkspaceFolder(uri); @@ -85,7 +83,10 @@ export function pythonIsCondaEnv(python: string) { "import sys, os; print(os.path.exists(os.path.join(sys.prefix, 'conda-meta')))", ]; const output = ( - child_process.execFileSync(shQuote(python), args, { + // Note: execFileSync does not spawn a shell, so the executable path must + // not be shell-quoted here (shQuote is only for command strings typed + // into a terminal). + child_process.execFileSync(python, args, { encoding: "utf-8", }) as unknown as string ).trim(); diff --git a/apps/vscode/src/core/terminal.ts b/apps/vscode/src/core/terminal.ts index a1f768dab..9d9119645 100644 --- a/apps/vscode/src/core/terminal.ts +++ b/apps/vscode/src/core/terminal.ts @@ -8,8 +8,8 @@ import * as os from "node:os"; import fs from "node:fs"; -import { Uri, workspace, extensions, window } from "vscode"; -import { pathWithForwardSlashes, shQuote, sleep, winShEscape } from "core"; +import { Uri, workspace, extensions, window, env } from "vscode"; +import { pathWithForwardSlashes, shQuote, shQuoteUnknownShell, sleep, TerminalShell } from "core"; import { Terminal } from "vscode"; import { QuartoContext, fileCrossrefIndexStorage } from "quarto-core"; import { TerminalOptions } from "vscode"; @@ -94,21 +94,64 @@ export function terminalOptions(name: string, target: Uri, env: TerminalEnv) { } export function terminalCommand(command: string, context: QuartoContext, target?: Uri) { + const shell = targetShell(context); const quarto = "quarto"; // binPath prepended to PATH so we don't need the full form - const cmd: string[] = [ - context.useCmd ? winShEscape(quarto) : shQuote(quarto), - command - ]; + // Note: the executable name must NOT be shell-quoted. PowerShell does not + // run a line that starts with a quoted string as a command (it needs `&`), + // so quoting it broke every Windows preview. `quarto` is a fixed literal + // with no shell-special characters, so it is safe bare in all shells. + const cmd: string[] = [quarto, command]; if (target) { - cmd.push(shQuote( - context.useCmd - ? target.fsPath - : pathWithForwardSlashes(target.fsPath) - )); + const targetPath = + context.useCmd ? target.fsPath : pathWithForwardSlashes(target.fsPath); + cmd.push( + shell === undefined ? shQuoteUnknownShell(targetPath) : shQuote(targetPath, shell) + ); } return cmd; } +// Map a shell executable path to the quoting rules it needs. Exported for +// unit testing. +export function terminalShellForPath( + shellPath: string | undefined +): TerminalShell | undefined { + if (!shellPath) { + return undefined; + } + const base = shellPath + .split(/[\\/]/) + .pop()! + .toLowerCase() + .replace(/\.(exe|cmd|bat)$/, ""); + if (base === "pwsh" || base === "powershell") { + return "powershell"; + } + if (base === "cmd") { + return "cmd"; + } + if (base === "bash" || base === "zsh" || base === "sh") { + return "posix"; + } + return undefined; +} + +// The shell that will interpret a generated terminal command: `useCmd` +// means the command targets cmd.exe (old Quarto on Windows shipped a .cmd +// shim); otherwise the quoting follows the shell the integrated terminal +// will actually start, read from vscode.env.shell. The OS platform alone +// cannot tell us — `terminalOptions()` does not set shellPath, so the +// terminal starts the user's default profile, which on Windows can be +// PowerShell, cmd.exe, Git Bash, or WSL, and on macOS/Linux pwsh or fish. +// Returns undefined when the shell is not recognized; the caller then falls +// back to the pre-shell-aware quoting. +function targetShell(context: QuartoContext): TerminalShell | undefined { + if (context.useCmd) { + return "cmd"; + } + return terminalShellForPath(env.shell); +} + export async function sendTerminalCommand( terminal: Terminal, terminalEnv: TerminalEnv, diff --git a/apps/vscode/src/test/index.ts b/apps/vscode/src/test/index.ts index 4368326d2..5f664aed1 100644 --- a/apps/vscode/src/test/index.ts +++ b/apps/vscode/src/test/index.ts @@ -19,7 +19,7 @@ export async function run(): Promise { timeout: 60000, }); - const testFiles = glob.sync(path.join(__dirname, 'symbols.test.js')); + const testFiles = glob.sync(path.join(__dirname, '{symbols,terminal,python}.test.js')); for (const file of testFiles) { mocha.addFile(file); diff --git a/apps/vscode/src/test/python.test.ts b/apps/vscode/src/test/python.test.ts new file mode 100644 index 000000000..aa343350b --- /dev/null +++ b/apps/vscode/src/test/python.test.ts @@ -0,0 +1,33 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from "assert"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; + +import { pythonIsCondaEnv } from "../core/python"; + +suite("pythonIsCondaEnv", function () { + test("accepts a raw interpreter path with shell special characters", function () { + if (process.platform === "win32") { + // A shebang script does not run on Windows. + this.skip(); + } + // execFileSync does not spawn a shell, so the interpreter path must be + // passed raw: with the old shell-quoted path the file does not exist and + // the function returns false (quarto-dev/quarto#1197). A shebang script + // named `python` stands in for a real interpreter without needing conda. + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "quarto-test-dir (x) $y-")); + const script = path.join(dir, "python"); + try { + fs.writeFileSync(script, "#!/bin/sh\necho True\n"); + fs.chmodSync(script, 0o755); + assert.strictEqual(pythonIsCondaEnv(script), true); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/apps/vscode/src/test/terminal.test.ts b/apps/vscode/src/test/terminal.test.ts new file mode 100644 index 000000000..353cd7409 --- /dev/null +++ b/apps/vscode/src/test/terminal.test.ts @@ -0,0 +1,64 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from "assert"; +import * as vscode from "vscode"; +import { QuartoContext } from "quarto-core"; + +import { terminalCommand, terminalShellForPath } from "../core/terminal"; + +suite("terminalCommand", function () { + test("does not quote the quarto executable name", function () { + // PowerShell does not run a line that starts with a quoted string as a + // command (it needs `&`), so the executable must stay a bare word — + // quoting it broke every Windows preview (quarto-dev/quarto#1197). + const context = { useCmd: false } as unknown as QuartoContext; + const cmd = terminalCommand("preview", context); + assert.deepStrictEqual(cmd, ["quarto", "preview"]); + assert.ok(!/^['"]/.test(cmd[0]), "executable name must not be quoted"); + }); + + test("passes the document path as one literal argument", function () { + // The #1194 fix itself: a path with parentheses must survive as one + // literal argument. Quoting shape is shell-dependent (and the test + // host's default shell varies), so assert singularity and literal + // survival; exact serializer output is covered by the shQuote tests. + const context = { useCmd: false } as unknown as QuartoContext; + const target = vscode.Uri.file("C:/Users/me/Article(md)/01_introduction.md"); + const cmd = terminalCommand("preview", context, target); + assert.strictEqual(cmd[0], "quarto"); + assert.strictEqual(cmd[1], "preview"); + assert.strictEqual(cmd.length, 3, "document path must be a single argument"); + assert.ok( + cmd[2].includes("Article(md)"), + `document path must survive literally, got: ${cmd[2]}` + ); + }); +}); + +suite("terminalShellForPath", function () { + test("maps the terminal's actual shell to quoting rules", function () { + // terminalCommand() quotes for the shell the terminal will actually + // start (vscode.env.shell), not the OS platform (quarto-dev/quarto#1197). + const cases: Array<[string | undefined, string | undefined]> = [ + ["C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe", "powershell"], + ["/usr/local/bin/pwsh", "powershell"], + ["C:\\Windows\\System32\\cmd.exe", "cmd"], + ["/bin/bash", "posix"], + ["/bin/zsh", "posix"], + ["/bin/sh", "posix"], + ["/usr/bin/fish", undefined], + ["", undefined], + [undefined, undefined], + ]; + for (const [shellPath, expected] of cases) { + assert.strictEqual( + terminalShellForPath(shellPath), + expected, + `shell path: ${shellPath}` + ); + } + }); +}); diff --git a/packages/core/src/strings.ts b/packages/core/src/strings.ts index 58d91cd61..157671156 100644 --- a/packages/core/src/strings.ts +++ b/packages/core/src/strings.ts @@ -8,14 +8,51 @@ export function escapeRegExpCharacters(value: string): string { return value.replace(/[\\{}*+?|^$.[\]()]/g, "\\$&"); } -export function shQuote(value: string): string { - if (/\s/g.test(value)) { - return `"${value}"`; - } else { - return value; +// The shell that will interpret a generated terminal command. +export type TerminalShell = "posix" | "powershell" | "cmd"; + +// Serialize one command-line argument so the target shell passes it to the +// child process as a single literal argument: no word splitting, globbing, +// or expansion. Always quotes, because each shell has its own literal-quoting +// rules and a generic double-quote wrapper is not enough (POSIX shells and +// PowerShell both still expand $HOME inside double quotes; cmd.exe still +// expands %NAME% inside them). +export function shQuote(value: string, shell: TerminalShell): string { + switch (shell) { + case "posix": + // Single quotes are fully literal; an embedded single quote is + // written as '\'' (close, escaped quote, reopen). + return `'${value.replace(/'/g, `'\\''`)}'`; + case "powershell": + // Single-quoted strings are fully literal; an embedded single quote + // is doubled. + return `'${value.replace(/'/g, "''")}'`; + case "cmd": { + // Double quotes group words for CommandLineToArgvW, but cmd.exe still + // expands %NAME% inside them (and !NAME! with delayed expansion). + // Note that cmd expands %NAME% before it processes carets, so ^% + // does not itself stop the expansion: the output is safe because the + // " and ^ characters break the variable name — cmd sees %"PATH"^%, + // no such variable is defined, and cmd keeps the text. Emit each % + // and ! outside the quotes, caret-escaped; a literal " is "" + // per CommandLineToArgvW. A part ending in a backslash needs its + // trailing backslashes doubled, otherwise CommandLineToArgvW reads + // \" as a literal quote instead of the closing quote. + return value + .split(/([%!])/) + .map((part) => + part === "%" || part === "!" + ? `^${part}` + : `"${part.replace(/"/g, `""`).replace(/(\\+)$/, "$1$1")}"` + ) + .join(""); + } } } -export function winShEscape(value: string): string { - return value.replace(" ", "^ "); +// Quoting for when the terminal's shell cannot be determined: the +// pre-shell-aware behavior (quote only on whitespace). Never worse than +// main for paths that already worked there. +export function shQuoteUnknownShell(value: string): string { + return /\s/.test(value) ? `"${value}"` : value; } diff --git a/packages/core/test/strings.test.ts b/packages/core/test/strings.test.ts new file mode 100644 index 000000000..911449cb8 --- /dev/null +++ b/packages/core/test/strings.test.ts @@ -0,0 +1,198 @@ +import { execFileSync } from "node:child_process"; +import assert from "node:assert/strict"; +import test from "node:test"; + +import { shQuote, shQuoteUnknownShell } from "../src/strings"; + +test("shQuote serializes one literal argument for POSIX sh", () => { + // Single quotes are fully literal in POSIX shells. + const cases: Array<[string, string]> = [ + ["quarto", "'quarto'"], + // Parentheses without whitespace (quarto-dev/quarto#1194): the original + // report, where PowerShell parsed `(md)` as a subexpression. + ["c:/Users/me/Article(md)/01_introduction.md", "'c:/Users/me/Article(md)/01_introduction.md'"], + ["a b", "'a b'"], + ["a$b", "'a$b'"], + ["$HOME", "'$HOME'"], + ["$(echo PWNED)", "'$(echo PWNED)'"], + ["a`b", "'a`b'"], + ['a"b', `'a"b'`], + ["a\\b", "'a\\b'"], + ["a\nb", "'a\nb'"], + ["a;b", "'a;b'"], + ["a@b", "'a@b'"], + ["a&b", "'a&b'"], + ["a|b", "'a|b'"], + ["a*b", "'a*b'"], + ["a?b", "'a?b'"], + ["a~b", "'a~b'"], + ["", "''"], + // An embedded single quote closes, escapes, and reopens the quoting. + ["a'b", "'a'\\''b'"], + ]; + for (const [value, expected] of cases) { + assert.equal(shQuote(value, "posix"), expected, JSON.stringify(value)); + } +}); + +test("shQuote serializes one literal argument for PowerShell", () => { + // Single-quoted strings are fully literal in PowerShell ($, backtick, and + // double quotes included); an embedded single quote is doubled. + const cases: Array<[string, string]> = [ + ["quarto", "'quarto'"], + [ + "C:/Users/me/Article(md)/01_introduction.md", + "'C:/Users/me/Article(md)/01_introduction.md'", + ], + ["a b", "'a b'"], + ["a$b", "'a$b'"], + ["$HOME", "'$HOME'"], + ["a@b", "'a@b'"], + ["a`b", "'a`b'"], + ['a"b', `'a"b'`], + ["a\nb", "'a\nb'"], + ["a'b", "'a''b'"], + ]; + for (const [value, expected] of cases) { + assert.equal(shQuote(value, "powershell"), expected, JSON.stringify(value)); + } +}); + +test("shQuote serializes one literal argument for cmd.exe", () => { + // Double quotes group words, but cmd.exe still expands %NAME% inside them + // (and !NAME! with delayed expansion). cmd expands %NAME% before it + // processes carets, so the output is safe because the " and ^ break the + // variable name — cmd sees %"PATH"^%, no such variable is defined, and + // cmd keeps the text. A literal " is "" per CommandLineToArgvW, and + // trailing backslashes of each part are doubled so \" is not read as a + // literal quote. + const cases: Array<[string, string]> = [ + ["quarto", '"quarto"'], + ["C:\\dir(name)\\file.qmd", '"C:\\dir(name)\\file.qmd"'], + ["a b", '"a b"'], + ["a&b", '"a&b"'], + ["a%b", '"a"^%"b"'], + ["100%", '"100"^%""'], + ["%PATH%", '""^%"PATH"^%""'], + ["a!b", '"a"^!"b"'], + ['a"b', '"a""b"'], + // A trailing backslash must be doubled before the closing quote, or + // CommandLineToArgvW reads \" as a literal quote (quarto-dev/quarto#1197). + ["C:\\", '"C:\\\\"'], + ]; + for (const [value, expected] of cases) { + assert.equal(shQuote(value, "cmd"), expected, JSON.stringify(value)); + } +}); + +test("shQuoteUnknownShell keeps the pre-shell-aware behavior", () => { + // Fallback for an unrecognized terminal shell: quote only on whitespace. + const cases: Array<[string, string]> = [ + ["quarto", "quarto"], + ["C:/Users/me/Article(md)/01_introduction.md", "C:/Users/me/Article(md)/01_introduction.md"], + ["a b", '"a b"'], + ]; + for (const [value, expected] of cases) { + assert.equal(shQuoteUnknownShell(value), expected, JSON.stringify(value)); + } +}); + +test( + "shQuote posix output round-trips through a real shell", + { skip: process.platform === "win32" }, + () => { + // Behavior-based check: the serialized argument must survive a real + // POSIX shell as one literal argument. The $(...)/backtick cases double + // as canaries: if the serializer ever emitted them unquoted or merely + // double-quoted, the shell would execute them and the output would be + // "PWNED" instead of the literal text. + const values = [ + "quarto", + "c:/Users/me/Article(md)/01_introduction.md", + "a b", + "a$b", + "$HOME", + "$(echo PWNED)", + "a`b", + "`echo PWNED`", + "a'b", + 'a"b', + "a\\b", + "a\nb", + "a;b", + "a@b", + "a&b", + "a|b", + "a*b", + "a?b", + "a~b", + "a#b", + "", + ]; + for (const value of values) { + const quoted = shQuote(value, "posix"); + const roundTripped = execFileSync("bash", ["-c", `printf %s ${quoted}`], { + encoding: "utf-8", + }); + assert.equal(roundTripped, value, JSON.stringify(value)); + } + } +); + +test( + "shQuote powershell output round-trips through pwsh to a native command", + { skip: !hasPwsh() && "pwsh is not installed" }, + () => { + // Behavior-based check: pass each serialized value from pwsh to a native + // executable (node), the same way the generated terminal command passes + // the target path to quarto. The $(...) case is a canary: if the value + // were not literal, pwsh would run it and print "PWNED". + const node = shQuote(process.execPath, "powershell"); + const script = shQuote( + "process.stdout.write(process.argv[1])", + "powershell" + ); + const values = [ + "C:/Users/me/Article(md)/01_introduction.md", + "a b", + "a$b", + "$HOME", + "$(echo PWNED)", + "a`b", + "a'b", + 'a"b', + "a\\b", + "a\nb", + "a;b", + "a@b", + "a&b", + "a|b", + "a#b", + "%PATH%", + "", + ]; + for (const value of values) { + const quoted = shQuote(value, "powershell"); + const roundTripped = execFileSync( + "pwsh", + [ + "-NoProfile", + "-NonInteractive", + "-Command", + `& ${node} -e ${script} ${quoted}`, + ], + { encoding: "utf-8" } + ); + assert.equal(roundTripped, value, JSON.stringify(value)); + } + } +); + +function hasPwsh(): boolean { + try { + execFileSync("pwsh", ["-NoProfile", "-Command", "exit 0"]); + return true; + } catch { + return false; + } +}