fix(lifecycle): properly quote args (#9018)

* fix(lifecycle): properly quote args

close #8980
close #7641

.changeset/famous-rocks-glow.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/lifecycle": patch
+pnpm: patch
+---
+
+Quote args for scripts with shell-quote to support new lines (on POSIX only) [#8980](https://github.com/pnpm/pnpm/issues/8980).

exec/lifecycle/package.json

@@ -45,6 +45,7 @@
     "@pnpm/types": "workspace:*",
     "is-windows": "catalog:",
     "path-exists": "catalog:",
+    "shell-quote": "catalog:",
     "run-groups": "catalog:"
   },
   "devDependencies": {
@@ -55,6 +56,7 @@
     "@pnpm/test-ipc-server": "workspace:*",
     "@types/is-windows": "catalog:",
     "@types/rimraf": "catalog:",
+    "@types/shell-quote": "catalog:",
     "@zkochan/rimraf": "catalog:",
     "load-json-file": "catalog:"
   },

exec/lifecycle/src/runLifecycleHook.ts

@@ -6,6 +6,7 @@ import { type DependencyManifest, type ProjectManifest, type PrepareExecutionEnv
 import { PnpmError } from '@pnpm/error'
 import { existsSync } from 'fs'
 import isWindows from 'is-windows'
+import { quote as shellQuote } from 'shell-quote'
 
 function noop () {} // eslint-disable-line:no-empty
 
@@ -87,8 +88,11 @@ Please unset the script-shell option, or configure it to a .exe instead.
     break
   }
   if (opts.args?.length && m.scripts?.[stage]) {
-    const escapedArgs = opts.args.map((arg) => JSON.stringify(arg))
-    m.scripts[stage] = `${m.scripts[stage]} ${escapedArgs.join(' ')}`
+    // It is impossible to quote a command line argument that contains newline for Windows cmd.
+    const escapedArgs = isWindows()
+      ? opts.args.map((arg) => JSON.stringify(arg)).join(' ')
+      : shellQuote(opts.args)
+    m.scripts[stage] = `${m.scripts[stage]} ${escapedArgs}`
   }
   // This script is used to prevent the usage of npm or Yarn.
   // It does nothing, when pnpm is used, so we may skip its execution.

exec/lifecycle/test/fixtures/escape-newline/echo.sh

@@ -0,0 +1,6 @@
+#!/usr/bin/env node
+
+const fs = require('fs');
+const path = require('path');
+
+fs.writeFileSync(path.join(__dirname, 'output.json'), JSON.stringify(process.argv.slice(2), null, 2))

exec/lifecycle/test/fixtures/escape-newline/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "issue-8980",
+  "version": "1.0.0",
+  "scripts": {
+    "echo": "node echo.sh"
+  }
+}

exec/lifecycle/test/index.ts

@@ -46,6 +46,23 @@ test('runLifecycleHook() escapes the args passed to the script', async () => {
   expect((await import(path.join(pkgRoot, 'output.json'))).default).toStrictEqual(['Revert "feature (#1)"'])
 })
 
+test('runLifecycleHook() passes newline correctly', async () => {
+  const pkgRoot = f.find('escape-newline')
+  const pkg = await import(path.join(pkgRoot, 'package.json'))
+  await runLifecycleHook('echo', pkg, {
+    depPath: 'escape-newline@1.0.0',
+    pkgRoot,
+    rawConfig: {},
+    rootModulesDir,
+    unsafePerm: true,
+    args: ['a\nb'],
+  })
+
+  expect((await import(path.join(pkgRoot, 'output.json'))).default).toStrictEqual([
+    process.platform === 'win32' ? 'a\\nb' : 'a\nb',
+  ])
+})
+
 test('runLifecycleHook() sets frozen-lockfile to false', async () => {
   const pkgRoot = f.find('inspect-frozen-lockfile')
   await using server = await createTestIpcServer(path.join(pkgRoot, 'test.sock'))

pnpm-lock.yaml

@@ -2229,6 +2229,9 @@ importers:
       run-groups:
         specifier: 'catalog:'
         version: 3.0.1
+      shell-quote:
+        specifier: 'catalog:'
+        version: 1.8.2
     devDependencies:
       '@pnpm/lifecycle':
         specifier: workspace:*
@@ -2251,6 +2254,9 @@ importers:
       '@types/rimraf':
         specifier: 'catalog:'
         version: 3.0.2
+      '@types/shell-quote':
+        specifier: 'catalog:'
+        version: 1.7.5
       '@zkochan/rimraf':
         specifier: 'catalog:'
         version: 3.0.2
@@ -13625,9 +13631,6 @@ packages:
     resolution: {integrity: sha512-7++dFhtcx3353uBaq8DDR4NuxBetBzC7ZQOhmTQInHEd6bSrXdiEyzCvG07Z44UYdLShWUyXt5M/yhz8ekcb1A==}
     engines: {node: '>=8'}
 
-  shell-quote@1.8.1:
-    resolution: {integrity: sha512-6j1W9l1iAs/4xYBI1SYOVZyFcCis9b4KCLQ8fgAGG07QvzaRLVVRQvAy85yNmmZSjYjg4MWh4gNvlPujU/5LpA==}
-
   shell-quote@1.8.2:
     resolution: {integrity: sha512-AzqKpGKjrj7EM6rKVQEPpB288oCfnrEIuyoT9cyF4nmGa7V8Zk6f7RRqYisX8X9m+Q7bd632aZW4ky7EhbQztA==}
     engines: {node: '>= 0.4'}
@@ -17658,7 +17661,7 @@ snapshots:
       date-fns: 2.30.0
       lodash: 4.17.21
       rxjs: 7.8.1
-      shell-quote: 1.8.1
+      shell-quote: 1.8.2
       spawn-command: 0.0.2
       supports-color: 8.1.1
       tree-kill: 1.2.2
@@ -21349,8 +21352,6 @@ snapshots:
 
   shebang-regex@3.0.0: {}
 
-  shell-quote@1.8.1: {}
-
   shell-quote@1.8.2: {}
 
   shelljs@0.8.5:

pnpm/test/run.ts

@@ -96,7 +96,9 @@ test('recursive test: pass the args to the command that is specified in the buil
 
   const result = execPnpmSync(['-r', 'test', 'arg', '--flag=true'])
 
-  expect((result.stdout as Buffer).toString('utf8')).toMatch(/ts-node test "arg" "--flag=true"/)
+  expect((result.stdout as Buffer).toString('utf8')).toMatch(
+    process.platform === 'win32' ? /ts-node test "arg" "--flag=true"/ : /ts-node test arg --flag\\=true/
+  )
 })
 
 test('start: run "node server.js" by default', async () => {