Skip to content

Conversation

@aloisklink
Copy link

Improve handling of shell metacharacters in folder names in the postinstall script by replacing execSync with execFileSync.

For example, spaces in folder names are now allowed:

Current behaviour

Installing msw with a msw.workerDirectory set in your package.json file fails if the folder structure has shell special characters (e.g. spaces) in it.

alois@my-pc:~/Documents/msw with a space (main)$ npm run postinstall

> [email protected] postinstall
> node -e "import('./config/scripts/postinstall.js').catch(() => void 0)"

node:internal/modules/cjs/loader:1215
  throw err;
  ^

Error: Cannot find module '/home/alois/Documents/msw'
    at Module._resolveFilename (node:internal/modules/cjs/loader:1212:15)
    at Module._load (node:internal/modules/cjs/loader:1043:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:164:12)
    at node:internal/main/run_main_module:28:49 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

Node.js v20.19.4
[MSW] Failed to automatically update the worker script.

Error: Command failed: node /home/alois/Documents/msw with a space/cli/index.js init
node:internal/modules/cjs/loader:1215
  throw err;
  ^

Error: Cannot find module '/home/alois/Documents/msw'
    at Module._resolveFilename (node:internal/modules/cjs/loader:1212:15)
    at Module._load (node:internal/modules/cjs/loader:1043:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:164:12)
    at node:internal/main/run_main_module:28:49 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

Node.js v20.19.4

Proposed

Installing msw with a msw.workerDirectory set in your package.json file works, even if your folder structure has spaces in it.

alois@my-pc:~/Documents/msw with a space (fix/improve-postinstall-script)$ npm run postinstall

> [email protected] postinstall
> node -e "import('./config/scripts/postinstall.js').catch(() => void 0)"

Future work

The config/scripts/patch-ts.js script is still vulnerable to this issue too, but since that only affects developers of this library, not users, I've ignored it for this PR:

const tscCompilation = await execAsync(
`tsc --noEmit --skipLibCheck ${typeDefsPaths.join(' ')}`,
{
cwd: BUILD_DIR,
},
)

`execSync` is dangerous, since if we're in a folder that has spaces in
it (e.g. 'My Documents/library/node_modules/msw'), the space will cause
this command to fail.

Using `execFileSync` instead fixes this issue.
The normal Node.JS function to create a new Node.JS process is
[`child_process.fork`][1]. Since it doesn't have a synchronous
API, we can't use it, but it does default to using the same Node.JS
version as the calling process by using `process.execPath`:

> By default, `child_process.fork()` will spawn new Node.js instances
> using the [`process.execPath`][2] of the parent process.

Considering that our postinstall script in `package.json` uses `node`,
this probably won't make a difference, but it's still good practice.

[1]: https://nodejs.org/docs/latest-v24.x/api/child_process.html#child_processforkmodulepath-args-options
[2]: https://nodejs.org/docs/latest-v24.x/api/process.html#processexecpath
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant