Skip to content
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

Remove configstore - fixing npm vulnerability #62

Merged
merged 1 commit into from
Aug 20, 2020

Conversation

herecydev
Copy link
Contributor

@herecydev herecydev commented Jul 30, 2020

Resolves https://www.npmjs.com/advisories/1213 and by extension gatsbyjs/gatsby#26128

I couldn't see any use of this package @davewasmer. Let me know if I'm incorrect.

@herecydev
Copy link
Contributor Author

@davewasmer do you have any comments with this PR? It would be really good to remove the vulnerability from Gatsby (which is dependant on this change)

@hoobdeebla
Copy link

cc @zetlen

@JackHowa
Copy link

JackHowa commented Aug 13, 2020

should I keep #64 open upgrading configstore or is removing it the priority? sorry to bother @zetlen @Js-Brecht

@herecydev
Copy link
Contributor Author

The reason I opened this PR is because I'm pretty sure it's not used. So removing it would be preferable. If it turns out it is used then we can go with your PR.

@Js-Brecht
Copy link
Contributor

I believe configstore was a part of the initial development effort, but its intended usage never made it to the remote
967e4c1

Looks like that import was stripped in January 2018:
e0ad2d6#diff-f41e9d04a45c83f3b6f6e630f10117feL12

I can't find its implementation in devcert-san, either. So according to the history, it only ever appeared as an import in index.ts... its intended purpose may be forever lost. I'm all for getting rid of it. Might as well get rid of @types/configstore, too.

@JackHowa
Copy link

JackHowa commented Aug 14, 2020

thanks @Js-Brecht. I will close mine #64 then to reduce confusion @zetlen

@zetlen
Copy link
Collaborator

zetlen commented Aug 20, 2020

Thanks for your patience, folks. I'm reviewing this now; I think it's likely that configstore is no longer used, but I'll check.

Copy link
Collaborator

@zetlen zetlen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you

@zetlen zetlen merged commit 7418e3c into davewasmer:master Aug 20, 2020
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.

6 participants