Nwb: Clean command fails in path with spaces

[BurntCaramel]

⠋ Cleaning app/bin/sh: /Users/pgwsmith/Work/Royal: No such file or directory                                                   
✖ Cleaning app                                                                                                                 
Error running command: Command failed: /Users/pgwsmith/Work/Royal Icing/creme-brulee/web/flambe/node_modules/nwb/node_modules/rimraf/bin.js coverage list

Seems to be due to the space in the path. I checked out the source code at https://github.com/insin/nwb/blob/master/src/commands/clean-app.js and couldn’t see an easy fix.

Is it possibly line 10 here needs quotes for the args (although each arg would have to be quoted I think)? https://github.com/insin/nwb/blob/85af7f42a738b867584951f53083f3ca5b36c9d4/src/exec.js#L10

let command = `${require.resolve(`.bin/${bin}`)} ${args.join(' ')}`

->

let command = `${require.resolve(`.bin/${bin}`)} "${args.join(' ')}"`

Or perhaps in https://github.com/insin/nwb/blob/85af7f42a738b867584951f53083f3ca5b36c9d4/src/commands/clean-app.js#L6:

let dist = args._[1] || 'dist'

->

let dist = `"${args._[1]}"` || 'dist'

[BurntCaramel]

Actually testing this more, it seems nwb clean-app works but npm run clean with "clean": "nwb clean-app" does not.

[insin]

Thanks for the bug report - I assume if we pass the arguments as a separate array instead of hardcoding them into the command to be executed, Node should handle escaping them properly. Will give it a go soon.

[vpezeshkian]

I have faced this issue today and I did some debugging, it turns out "for me" that rimraf does not handle paths with spaces...

[isaacs]

Rimraf handles spaces just fine.

The bug here is that you're passing a string to exec, you're not calling spawn with an array of arguments.

Consider if you had a file called foo bar with a space. If you call rm foo bar on the shell, it's going to remove the files foo and bar.

In Node.js, if you call var childProcess = spawn('rm', ['foo bar']) then that's going to pass foo bar as a literal argument to the rm process.

But if you call exec('rm foo bar') then it's going to pass the string "rm foo bar" as an argument to sh like spawn('sh', ['-c', 'rm foo bar']) and it's going to fail.

You should really almost never be passing strings from userland or the filesystem directly into a command like this. It's almost always a security vulnerability. For example, I could publish a module with a file named some-file\ndo-something-evil and your program would happily execute the shell script:

rm some-file
do-something-evil

and then evil things would happen and I'd have control of your machine.

Since rimraf can be used as a module, and this script is in JavaScript, there's no need to execSync it. Just require() it and use the function.