-
-
Notifications
You must be signed in to change notification settings - Fork 51
security: remove package-lock.json #141
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
security: remove package-lock.json #141
Conversation
…on files, ignore package-lock.json in .npmignore to avoid publishing a package with package-lock.json, gitignore package-lock.json We could be prone to a supply-chain-attack when we not carefully review changes in the package-lock.json. urls to packages could be changed to malicious variants. To avoid this, we disable the generation package-lock.json. We should not accept any PRs with package-lock.json.
In #140 sinon gets updated while patching the code. While updating a package is not an issue, reviewing the package-lock.json is a pain in the ass and also a reviewer could be negligent and just merge without looking twice. So to avoid that we get some attack vector by a overlooked change in the package-lock.json we should absolutely not use package-lock.json |
I am with you. The biggest issue for me is that it causes package-lock changes to appear in random commits, which can lead to merge conflicts. It also can introduces security vulnerabilities. https://snyk.io/blog/why-npm-lockfiles-can-be-a-security-blindspot-for-injecting-malicious-modules/ So +1 to remove it @jorenvandeweyer what do you think? |
But isn't security the very reason for lockfile, so there is no auto-install of new malicious packages that were hijacked and published? ALso does the lockfile show the dependencies integrity (sha512 sum) and I think we should rather think about a lockfile lint and accept lockfile updates only in combination with dependency version updates. |
dependabot also uses package-lock.json so we should rather keep it and make sure nothing bad slips in. |
in mongoose, we dont use package-lock.json and dependabot works as expected :) |
@Uzlopak thank you for this hint. So, from my understanding we can basically "pin" versions in "dependencies": {
"bson": "^4.6.2",
"kareem": "2.3.5",
"mongodb": "4.5.0",
"mpath": "0.9.0",
"mquery": "4.0.3",
"ms": "2.1.3",
"sift": "16.0.0"
}, All packages, besides I just try to understand this, before simply copying this technique. From that point I would propose for this PR to pin all the versions of |
Strange. Will pin down the bson version in mongoose. |
Lol nice, inadvertently found a small "bug" in mongoose. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I will pin the versions in development
and release-4.2.0
remove package-lock.json set .npmrc to not create package-lock.json files, ignore package-lock.json in .npmignore to avoid publishing a package with package-lock.json, gitignore package-lock.json
We could be prone to a supply-chain-attack when we not carefully review changes in the package-lock.json. urls to packages could be changed to malicious variants. To avoid this, we disable the generation package-lock.json. We should not accept any PRs with package-lock.json.