fix: shellescape prettier command paths

Author: mitermayer

AGENTS.md

@@ -131,6 +131,10 @@ plugin versions:
   project-local Prettier outside this checkout.
 - `yarn lint` now uses pinned local Python requirements and a checkout-local
   virtualenv wrapper for `vim-vint`, and CI runs it as a blocking job.
+- Hardened the current POSIX shell-string command path by returning raw
+  executable paths from the resolver and shellescaping executable and
+  `--stdin-filepath` arguments at shell call sites. Windows and argv-list job
+  APIs remain open.
 
 ## Review Findings After b538e29
 
@@ -200,7 +204,9 @@ plugin versions:
 - [x] Fix resolver to search for Prettier from the buffer's file tree, not only
   `getcwd()`.
 - [x] Avoid mutating buffer filetype defaults when merging overrides.
-- [ ] Make command construction shell-safe for spaces, quotes, and Windows.
+- [x] Harden POSIX shell-string command construction for executable paths and
+  `--stdin-filepath` paths containing spaces and quotes.
+- [ ] Finish command construction hardening for Windows.
 - [ ] Prefer argv-list job/system APIs where Vim/Neovim support allows.
 - [ ] Expand config-file discovery for modern Prettier config names.
 

autoload/prettier.vim

@@ -27,7 +27,7 @@ function! prettier#PrettierCli(user_input) abort
   let l:execCmd = prettier#resolver#executable#getPath()
 
   if l:execCmd != -1
-    let l:out = system(l:execCmd. ' ' . a:user_input)
+    let l:out = system(shellescape(l:execCmd) . ' ' . a:user_input)
     echom l:out
   else
     call prettier#logging#error#log('EXECUTABLE_NOT_FOUND_ERROR')
@@ -65,7 +65,7 @@ function! prettier#Prettier(...) abort
     " TODO
     " => we should make sure we can resolve --range-start  and --range-end when required
     "    => when the above is required we should also update l:startSelection to '1' and l:endSelection to line('$')
-    let l:cmd = l:execCmd . prettier#resolver#config#resolve(
+    let l:cmd = shellescape(l:execCmd) . prettier#resolver#config#resolve(
           \ prettier#resolver#preset#resolve(l:config),
           \ l:partialFormatEnabled,
           \ l:startSelection, 

autoload/prettier/resolver/config.vim

@@ -147,7 +147,7 @@ endfunction
 " file.
 function! s:Flag_stdin_filepath(...) abort
   let l:current_file = simplify(expand('%:p'))
-  return '--stdin-filepath="' . l:current_file . '"'
+  return '--stdin-filepath=' . shellescape(l:current_file)
 endfunction
 
 " Returns '--loglevel error' or '--log-level error'.
@@ -304,7 +304,7 @@ function! s:Get_prettier_cli_version() abort
     return ''
   endif
 
-  let l:output = system(l:execCmd . ' --version')
+  let l:output = system(shellescape(l:execCmd) . ' --version')
   " The shell sends the string with whitespaces at both ends.
   let l:prettier_cli_version = s:Trim_internal_unprintable(trim(l:output))
   return l:prettier_cli_version

autoload/prettier/resolver/executable.vim

@@ -2,8 +2,8 @@ let s:ROOT_DIR = fnamemodify(resolve(expand('<sfile>:p')), ':h')
 let s:PLUGIN_ROOT_DIR = fnamemodify(s:ROOT_DIR, ':h:h:h')
 
 function! s:NormalizePath(path) abort
-  let l:path = substitute(a:path, '\\ ', ' ', 'g')
-  return substitute(resolve(fnamemodify(l:path, ':p')), '\\', '/', 'g')
+  let l:path = resolve(fnamemodify(a:path, ':p'))
+  return has('win32') || has('win64') ? substitute(l:path, '\\', '/', 'g') : l:path
 endfunction
 
 function! prettier#resolver#executable#isUnderPluginRoot(path) abort
@@ -39,24 +39,24 @@ function! prettier#resolver#executable#getPath() abort
     if isdirectory(l:bufferDir)
       let l:bufferLocalExec = s:ResolveExecutable(l:bufferDir)
       if executable(l:bufferLocalExec)
-        return fnameescape(l:bufferLocalExec)
+        return l:bufferLocalExec
       endif
     endif
   endif
 
   let l:localExec = s:ResolveExecutable(getcwd())
   if executable(l:localExec)
-    return fnameescape(l:localExec)
+    return l:localExec
   endif
 
   let l:globalExec = s:ResolveExecutable()
   if executable(l:globalExec)
-    return fnameescape(l:globalExec)
+    return l:globalExec
   endif
 
   let l:pluginExec = s:ResolveExecutable(s:ROOT_DIR)
   if executable(l:pluginExec)
-    return fnameescape(l:pluginExec)
+    return l:pluginExec
   endif
 
   return -1

package.json

@@ -10,7 +10,7 @@
   },
   "scripts": {
     "test": "node scripts/vim-version.js && LOG_LEVEL=error PRETTIER_FORMATTING_FIXTURE_LANE=all jest",
-    "test:smoke": "node scripts/vim-version.js && LOG_LEVEL=error PRETTIER_FORMATTING_FIXTURE_LANE=all jest tests/formatting.test.js --runInBand --testNamePattern=\"Prettier config\"",
+    "test:smoke": "node scripts/vim-version.js && LOG_LEVEL=error PRETTIER_FORMATTING_FIXTURE_LANE=all jest tests/formatting.test.js --runInBand --testNamePattern=\"Prettier config|Prettier command shell\"",
     "test:formatting:passing": "node scripts/vim-version.js && LOG_LEVEL=error PRETTIER_FORMATTING_FIXTURE_LANE=known-passing jest tests/formatting.test.js --runInBand",
     "test:formatting:quarantined": "node scripts/vim-version.js && LOG_LEVEL=error PRETTIER_FORMATTING_FIXTURE_LANE=quarantined jest tests/formatting.test.js --runInBand",
     "lint": "sh scripts/vint.sh --version && sh scripts/vint.sh ."

tests/formatting.test.js

@@ -48,6 +48,12 @@ const getBufferContents = async remote =>
 
 const vimString = value => `'${value.replace(/'/g, "''")}'`;
 
+const vimStringExpr = value =>
+  value
+    .split('"')
+    .map(vimString)
+    .join(' . nr2char(34) . ');
+
 const removeDirectorySync = dir => {
   if (!fs.existsSync(dir)) {
     return;
@@ -72,17 +78,41 @@ const cleanupTempProjects = () => {
   }
 };
 
-const writeFakePrettierExecutable = prettierPath => {
-  fs.writeFileSync(
-    prettierPath,
-    [
-      '#!/bin/sh',
-      'if [ "$1" = "--version" ]; then',
-      "  printf '3.0.3\\n'",
-      'fi',
-      'exit 0',
-    ].join('\n') + '\n'
-  );
+const writeFakePrettierExecutable = (prettierPath, options = {}) => {
+  const script = [
+    '#!/bin/sh',
+    'if [ "$1" = "--version" ]; then',
+    "  printf '3.0.3\\n'",
+    '  exit 0',
+    'fi',
+  ];
+
+  if (options.expectedStdinFilepath) {
+    script.push(
+      `expected_stdin_filepath=${shellQuote(options.expectedStdinFilepath)}`,
+      'found_stdin_filepath=0',
+      'for arg in "$@"; do',
+      '  if [ "$arg" = "--stdin-filepath=$expected_stdin_filepath" ]; then',
+      '    found_stdin_filepath=1',
+      '  fi',
+      'done',
+      'if [ "$found_stdin_filepath" != "1" ]; then',
+      "  printf '%s\\n' 'missing expected stdin filepath' >&2",
+      '  exit 2',
+      'fi'
+    );
+  }
+
+  if (Object.prototype.hasOwnProperty.call(options, 'formattedOutput')) {
+    script.push(
+      'cat >/dev/null',
+      `printf '%s\\n' ${shellQuote(options.formattedOutput)}`
+    );
+  }
+
+  script.push('exit 0');
+
+  fs.writeFileSync(prettierPath, script.join('\n') + '\n');
   fs.chmodSync(prettierPath, 0o755);
 };
 
@@ -105,8 +135,32 @@ const createProjectLocalPrettierFixture = extension => {
   return { root, prettierPath, sourcePath };
 };
 
+const createShellSafetyPrettierFixture = () => {
+  const root = fs.mkdtempSync(
+    path.join(fs.realpathSync(os.tmpdir()), 'vim-prettier shell "quote" ')
+  );
+  const binDir = path.join(root, 'node_modules', '.bin');
+  const sourceDir = path.join(root, 'src with spaces');
+  const prettierPath = path.join(binDir, 'prettier');
+  const sourcePath = path.join(sourceDir, 'index "quote".js');
+
+  tempProjectRoots.push(root);
+  fs.mkdirSync(binDir, { recursive: true });
+  fs.mkdirSync(sourceDir, { recursive: true });
+  writeFakePrettierExecutable(prettierPath, {
+    expectedStdinFilepath: sourcePath,
+    formattedOutput: 'const formatted = true;',
+  });
+  fs.writeFileSync(sourcePath, 'const formatted=false\n');
+
+  return { root, prettierPath, sourcePath };
+};
+
 const setVimCwd = dir =>
-  remote.execute(`execute 'cd' fnameescape(${vimString(dir)})`);
+  remote.execute(`execute 'cd' fnameescape(${vimStringExpr(dir)})`);
+
+const editFile = file =>
+  remote.execute(`execute 'edit' fnameescape(${vimStringExpr(file)})`);
 
 const resolveConfigFlags = config =>
   remote.eval(`prettier#resolver#config#resolve(${config}, 0, 1, 1)`);
@@ -430,6 +484,39 @@ if (FORMAT_FIXTURE_LANE === 'all') {
       expectNoPluginFlags(flags);
     });
   });
+
+  describe('Prettier command shell safety', () => {
+    test('shellescapes stdin filepath for paths containing spaces and quotes', async () => {
+      const project = createShellSafetyPrettierFixture();
+
+      await editFile(project.sourcePath);
+
+      const flags = await resolveConfigFlags('{}');
+      const expectedPath = await remote.eval(
+        "shellescape(simplify(expand('%:p')))"
+      );
+
+      expect(flags).toContain(`--stdin-filepath=${expectedPath}`);
+      expect(flags).not.toContain(`--stdin-filepath="${project.sourcePath}"`);
+    });
+
+    test(':Prettier runs a project-local executable from a shell-escaped path', async () => {
+      const project = createShellSafetyPrettierFixture();
+
+      await setVimCwd(__dirname);
+      await editFile(project.sourcePath);
+
+      const resolvedPath = await remote.eval(
+        'prettier#resolver#executable#getPath()'
+      );
+
+      expect(resolvedPath).toBe(project.prettierPath);
+
+      await remote.execute('Prettier');
+
+      expect(await getBufferContents(remote)).toBe('const formatted = true;');
+    });
+  });
 }
 
 // run formatting tests in all fixtures