Skip to content

Get rid of shelljs dependency #1044

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Get rid of shelljs dependency #1044

wants to merge 1 commit into from

Conversation

bodia-uz
Copy link
Contributor

@bodia-uz bodia-uz commented Apr 11, 2018

@bildja
Copy link

bildja commented Apr 11, 2018

A little bit of "why" behind this PR
https://snyk.io/vuln/npm:shelljs:20140723

@tmcw
Copy link
Member

tmcw commented Apr 11, 2018

I'm happy to remove a dependency, but in terms of security - does this really change anything? If we don't use the shell.exec method, and that method isn't used in the call path of the methods that we do use, then how is there any exploit risk?

@bildja
Copy link

bildja commented Apr 11, 2018

Well, I think, in terms of security this particular change does not change anything. Still, to make Snyk happy (now it raises High Severity issue), I would really appreciate shelljs to be removed. And as a bonus you have minus one dependency which is only used for things that can be done with no extra complexity

@tmcw
Copy link
Member

tmcw commented Apr 11, 2018

Rebased & merged in #1047

@tmcw tmcw closed this Apr 11, 2018
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.

3 participants