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

move npm audit to CI from Dockerfile #228

Closed
wants to merge 2 commits into from

Conversation

minrk
Copy link
Member

@minrk minrk commented Mar 12, 2020

If this new test stage fails, running npm audit fix and committing the result will fix it.

Moves the fixes to the repo's package-lock.json, rather than deferring to the docker build. This means that all installs of CHP will get fixes, not just docker, and ensures that package-lock.json accurately represents what is installed in the image.

It also ensures that we are notified when a fix is needed.

The base image is also pinned only to the major node version to ensure that security patches are received on every build, rather than forcing us to keep bumping the base image.

This also reverts the removal of npm in #226, which didn't have any effect on the vulnerability of the image, but there may be some unfortunate policy reasons why it needs to be done, despite having no effect.

pin to major version instead of patch version, ensures we get security fixes and the like on each build

avoid calling `npm audit fix` in the image, which results in package-lock.json not being representative of what's in the image

instead, `npm audit fix` should be called on the main repo and committed
so CI fails if there are vulnerabilities
@minrk minrk mentioned this pull request Mar 12, 2020
@consideRatio
Copy link
Member

consideRatio commented Mar 12, 2020

avoid calling npm audit fix in the image, which results in package-lock.json not being representative of what's in the image

If we want the docker image to represent our in-repo package-lock.json, we should ensure we install from package-lock.json, which I think we currently don't when we write npm install --production that will update package-lock.json.

I think then perhaps what we need is npm ci, see: https://docs.npmjs.com/cli/ci.html#description

Also, since we run npm install instead of npm ci in travis, we will actually audit the newly created package-lock.json instead of the commited package-lock.json file!

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.

2 participants