Storybook: ShellJS Security Warning

[danielduan]

There's a SEVERE security warning for one of the dependencies we're using.
https://www.bithound.io/github/storybooks/storybook/master/dependencies/npm/shelljs#security-advisories

Is it something we're concerned about? Should we move off of it? Help patch it?

[Hypnosphi]

@ndelangen You had some considerations AFAIK

[ndelangen]

Yeah, personally I feel this security warning is a bit nonsense:
The package is specifically for being able to run shell commands cross platform.
So for this package to have "command injection" is a feature, not a bug.

But having said that, shelljs could make their package more secure by default and opt in to shell-mode. More information here:
https://github.com/shelljs/shelljs/issues/143

I'm pretty sure we're not using shelljs.exec and interpolate data from outside our domain into the command, anyway. Thus there's no possibility for command injection in our case.

I am annoyed by the warning, I do not want people to think Storybook is insecure because of this, because this is first and foremost a devDependency, and second (as i already mentioned) the way we use it doesn't allow command injection to happen.

ShellJS is just as insecure as child_process, which is a core NodeJS package..
I think the SEVERE security warning is a "storm in een glas water" as we dutch like to say.

[danielduan]

gonna close this then.

[Hypnosphi]

because this is first and foremost a devDependency

not anymore
https://github.com/storybookjs/storybook/blob/next/lib/core/package.json#L82

[ndelangen]

Hmm..

Still no vulnerability:
We only use the .mkdir & .cp commands in user executed code:
https://github.com/storybookjs/storybook/blob/9835ab9d7982b3125a96582577b5b96dd2fee0d5/lib/core/src/server/build-static.js#L170

We do not use .exec in this context.