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

Check if package-lock.json was changed and fail the build (Mobile only) #6915

Closed
wants to merge 6 commits into from

Conversation

mandrigin
Copy link
Contributor

@mandrigin mandrigin commented Nov 28, 2018

Due to vulnerabilities, like the one found in event-stream, we can't trust npm even when locking the version numbers.
Hence, we are trying to mitigate similar issues by checking if package-lock.json was updated after npm install and failing the build if it was.
The lock file contains a checksum of the minified package, so if someone updates it but keeps the same version, this script should detect it.

  • Write the script
  • Update the package-lock.json content
  • Plug in the new script instead of npm install
  • Ensure the deps versions in the package.json are locked
  • (Temporarily) Disable Desktop and add a follow-up PR. ([WIP] Lock npm packages for Desktop #6918)

fixes #6906

status: ready

@status-github-bot
Copy link

status-github-bot bot commented Nov 28, 2018

Pull Request Checklist

  • Have you updated the documentation, if impacted (e.g. docs.status.im)?

@status-github-bot status-github-bot bot added this to REVIEW in Pipeline for QA Nov 28, 2018
ci/common.groovy Outdated
@@ -53,14 +53,20 @@ def installJSDeps(platform) {
def attempt = 1
def maxAttempts = 10
def installed = false
def errroCode = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

typo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@pedropombeiro
Copy link
Contributor

Output for make prepare-desktop: output.log

@@ -8791,7 +8791,9 @@
}
},
"react-native-http-bridge": {
"version": "git+https://github.com/status-im/react-native-http-bridge.git#214301a806743d0ce04f4559c3d55b3e8ff95f5d"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@flexsurfer @yenda so we have switched from our own fork of the react-native-http-bridge back to the npm version, right?

@mandrigin mandrigin changed the title [WIP] Check if package-lock.json was changed and fail the build Check if package-lock.json was changed and fail the build Nov 28, 2018
@mandrigin mandrigin changed the title Check if package-lock.json was changed and fail the build Check if package-lock.json was changed and fail the build (Mobile only) Nov 28, 2018
@mandrigin
Copy link
Contributor Author

Continuing my adventures. npm is a truly amazing piece of... technology: npm/npm#18712

@pedropombeiro
Copy link
Contributor

I know we tried moving to yarn before and abandoned that for some reason, but haven't come across a write-up of what happened. It does seem to be something we'd want to do sooner rather than later, especially before Reproducible Builds.

@cammellos
Copy link
Member

@pombeirp description is in the issue #3838

@mandrigin
Copy link
Contributor Author

Okay, I will try to switch us to Yarn, let's close this PR for now.

@mandrigin mandrigin closed this Nov 28, 2018
Pipeline for QA automation moved this from REVIEW to DONE Nov 28, 2018
@rasom rasom deleted the security/lock-node-deps branch January 11, 2019 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Fail build if package-lock.json changes
4 participants