Verify integrity of dependencies in production with yarn

[kumar303]

After #9965 we will be using yarn to install dependencies in production.

Let's make sure all packages get installed without any surprises.

• A production install should never upgrade a package or its dependencies, even if a sub-dependency is declared with a loose version range. Please see this detailed example of what we should prevent.
• Verify the integrity of all packages. If one gets tampered with, the install step should fail. (Does yarn already do this by default? Maybe we need yarn check)

This issue is about preventing malware on NPM from creeping into a production build (see the eslint malware scenario as an example). It would still be possible for a developer to unknowingly introduce malware when upgrading a dependency -- that problem is out of scope.

[willdurand]

Where should we run the check command?

[kumar303]

Probably right before we run yarn install https://github.com/mozilla/addons-frontend/blob/master/Dockerfile#L23

but I don't think we should spend time on this issue yet. I have a feeling we need to solve #10400 first which may not be so trivial.

[willdurand]

@kumar303 we could probably install the dependencies with yarn install --pure-lockfile in the Dockerfile instead? https://yarnpkg.com/lang/en/docs/cli/install/#toc-yarn-install-pure-lockfile

[kumar303]

Two npm packages we rely on were recently compromised. They were for development only but they could have been production deps all the same.

• eslint-config-eslint 5.0.2 infected eslint/eslint#10600
• Virus in eslint-scope? eslint/eslint-scope#39

Bumping up priority

[kumar303]

Well, huh, I'm not sure exactly how yarn lock files work. It seems that it only has a loose requirement on eslint-scope, one of the compromised packages:

https://github.com/mozilla/addons-frontend/blob/29ac1f7f801d75da180bbc1bed21c3646bb1e685/yarn.lock#L565

I don't know if that would have helped us pin the version of eslint-scope.

The goal for this issue is to make sure a production install never upgrades a package beyond what we have committed to master.

[willdurand]

The goal for this issue is to make sure a production install never upgrades a package beyond what we have committed to master.

I suppose that's the aim of the --pure-lockfile option.

[kumar303]

We should consider yarn install --offline as a possible solution here.

[willdurand]

We should consider yarn install --offline as a possible solution here.

If I understand correctly, that would mean committing all the production node modules in this repo? Isn't it a bit too much (in terms of solution vs problem to be solved)?

[willdurand]

@kumar303 so it results in 60MB of tar packages to add to our repo if we go for the offline mode. Also, it will mean everyone has to use that and updating packages is a bit less straightforward.

[kumar303]

Hmm, that does sound annoying.

[kumar303]

The docs for --pure-lockfile and --frozen-lockfile are terrible so I'm not sure what they do.

[willdurand]

The docs for --pure-lockfile and --frozen-lockfile are terrible so I'm not sure what they do.

lol, I thought I left the exact same comment before. Yeah, I was looking at the docs and issues and code to see what the differences are, and which one we should be using, and also why it is not the default behavior. 🤦‍♂️

[willdurand]

So apparently, both do what we want except that frozen fails if there is an update, which, I think, means when the lock file is out of date. Source: yarnpkg/yarn#5847

[willdurand]

I think --frozen-lockfile will reject all the Greenkeeper PRs.

[willdurand]

So here are the PRs for the two options:

• --pure-lockfile: use yarn.lock when building the apps addons-frontend#5897
• --offline: [WIP] Setup yarn offline addons-frontend#5896

I just noticed that the Dockerfile does not use the yarn.lock file when fixing mozilla/addons-frontend#5897.

[willdurand]

The --offline does not work for obscure reasons, so I tend to prefer the --pure-lockfile approach.

[kumar303]

Yeah, I was looking at the docs ... also why it is not the default behavior.

Making --pure-lockfile the default was proposed in yarnpkg/yarn#570 but that issued got closed in favor of yarnpkg/yarn#1576 . The patch for that still seems to allow package.json to become the source of truth if it has differences.

[kumar303]

Sorry if the requirements of this issue were vague. It was also confusing because it made some assumptions about yarn.lock that were untrue. I updated the description to better illustrate what I originally intended.

[willdurand]

Quoted from: https://yarnpkg.com/blog/2016/11/24/offline-mirror/

Each of these dependencies have a resolved field with a remote URL. If you delete your node_modules folder and run yarn install again, Yarn will download the same resolved dependencies specified in this lockfile. It will even guarantee that no one modified the files since your first installs by verifying checksum for each of them.

So.. I suppose we are quite safe now that we use the lock file?

An offline mirror seems useful to install packages without an Internet connection, but it seems Yarn does what we want:

• it downloads the packages from the resolved URL in the lock file (so it uses the same packages, not new packages)
• it checks downloaded packages with checksums

[willdurand]

ping @kumar303

[willdurand]

I created a demo project and two dependencies to test it: https://github.com/willdurand/pkg-app.

Lockfile is used by yarn on install and there is no way to update a sub-dependency in a malicious manner. It only uses what's in the yarn.lock file.

[willdurand]

I am closing this issue now that it has been verified.

If you want to verify it:

1. clone the demo project:

git clone https://github.com/willdurand/pkg-app

2. install the deps:

yarn install

3. run the script:

node index.js

If you remove the yarn.lock file and re-install the dependencies (yarn install), you'll see an update of the sub-dependency (pkg-001). You can see the update by running node index.js again.

If you discard your changes (git checkout -- .), and re-install the deps, it will be back to the state at (3).