node typescript security and caching - npm or yarn?

[henrjk]

This is related to #147 and how the node typescript toolset builds:

ods-pipeline/build/package/scripts/build-typescript.sh

Lines 81 to 82 in b0c1b2c

	echo "Installing dependencies ..."
	npm ci

I expected that npm ci would take package-lock.json as its source of truth, never install any bits not specified by hashes in the lock file and perhaps also that npm ci would fail if package-lock.json is inconsistent with package.json. Unfortunately exactly if these files are inconsistent npm ci does install stuff compatible with package.json even if these bits are not blessed by the hashes in the lock file. As package-lock.json is generated/updated by npm install, this situation can easily happen if one forgets to check in both package.json and package-lock.json. Details are discussed in [BUG] npm ci succeeds when package-lock.json doesn't match package.json · Issue #2701 · npm/cli.

Nonetheless I investigated how to introduce caching with npm ci using npm version 8.1.2.
npm always uses a cache. By default this is located at ~/.npm, but the location can be changed via npm config as well as with a command line flag --cache <dir> or via an environment variable.

Following is a summary of my observations.
npm ci and npm i (npm install) were equally fast if used

• without the cache and without node_modules as well as
• with the cache and without node_modules

Performance of using the npm cache appears to be mostly independent of the cache size.

On my local system for a case with 363 packages it took around 15s to install from scratch (with an empty cache and no node_modules) and ~ 7s with a cache, so the time was reduced to about half.
In the ods-pipeline (which also has no cache or existing node_modules) on OpenShift it took around 15s as well.

npm ci is slower when node_modules are present as it will first delete it. The perceived performance was similar to rm -rf node_modules. The issue is discussed in Do not remove node_modules on npm ci. This can take considerable time if there are many packages installed. In the same issue a workaround is detailed which skips invoking npm ci if package-lock.json has not changed compared to the prior build. This would be viable assuming the problem described initially was fixed.

npm ci slowed down to take about twice as long when package-lock.json was written by an earlier npm version. Instead of failing the command tries to do its best which again can cause undesirable bits to get into the build.

npm i is very fast (less than 1s) if node_modules was up to date even with an empty cache. As npm ci is wiping node_modules this is where the widest performance gap happens.

I believe there is no way to change where node_modules resides, but have not investigated this deeply.

Given the mentioned issues it appears we should give serious consideration to using yarn (https://yarnpkg.com) instead of npm. I have not yet researched yarn at all and have no experience with it. Thanks for reading and let me know your thoughts on whether yarn is viable for replacing npm.

[netzartist]

Interesting findings! A fast but (due to human factors weak) way around could be to enforce checking in both package files?

Yes, Yarn might be worth to take a look at with these topics in mind, didn't experiment with version 2 yet. Also, the Pnpm package manager doesn't let a package access and mess around with dependencies other than their own, and is even faster than Yarn in most scenarios. Also worth investigating deeper.

[michaelsauter]

A fast but (due to human factors weak) way around could be to enforce checking in both package files?

We could let the ods-build-typescript task fail when a package-lock.json is not present?

[henrjk]

One could have something based on npm/cli#2701 (comment) in a pre-commit hook.

We encountered this while migrating where Jenkins would still use an older node version and thus we needed to keep an older package-lock.json version. During that time it was useful that the builds did not fail. We could consider making an inconsistency a warning and a missing package-lock.json and error.

FYI @henninggross