Page MenuHomePhabricator

Investigate reminding people to update `npm audit`
Closed, ResolvedPublic

Description

We need a solution to keep us on top of required npm package updates.

Status Quo:

Speculation around approach:

  • During ticket polishing we discussed the fact that a calendar event could be enough? https://en.wikipedia.org/wiki/Patch_Tuesday
  • Automation of simply running npm audit could also probably be achieved with jenkins and github actions with email notifications

FWIW lerna-audit now (v1.3.1) does not show the problems anymore which we encountered earlier (e.g.) when using it to tend to monorepos.

Acceptance criteria β›Ίβœ¨ :

  • Know all of the places that we have package.json files that need to be audited (and their current state in temrs of audit being run)

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Lerna looks interesting but as far as I can see we'd need to commit to it for all packages in our "monorepo" (and it's really not clear to me how that would work with submodules like termbox).

Change 572828 had a related patch set uploaded (by Tarrow; owner: Tarrow):
[mediawiki/extensions/Wikibase@master] TR: Add as dev dependency of Wikibase

https://gerrit.wikimedia.org/r/572828

Change 573235 had a related patch set uploaded (by Tarrow; owner: Tarrow):
[integration/config@master] Add daily job to run npm audit on Wikibase

https://gerrit.wikimedia.org/r/573235

Assigning to myself to make the CI change happen!

Change 573235 merged by jenkins-bot:
[integration/config@master] Add daily job to run npm audit on Wikibase

https://gerrit.wikimedia.org/r/573235

Job merged and running.

See https://integration.wikimedia.org/ci/job/wikibase-daily-npm-audit-daily-node10-npmaudit-docker/7/console

Currently email notifications are turned off pending @Tarrow review of this and getting it green :)

Additional merged gerrit config patches:

The most recent run says these two need updating:

# Run  npm install --save-dev stylelint-config-wikimedia@0.10.1  to resolve 1 vulnerability
β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”
β”‚ Low           β”‚ Prototype Pollution                                          β”‚
β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
β”‚ Package       β”‚ yargs-parser                                                 β”‚
β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
β”‚ Dependency of β”‚ stylelint-config-wikimedia [dev]                             β”‚
β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
β”‚ Path          β”‚ stylelint-config-wikimedia > stylelint > meow > yargs-parser β”‚
β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
β”‚ More info     β”‚ https://nodesecurity.io/advisories/1500                      β”‚
β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜


# Run  npm update yargs --depth 2  to resolve 1 vulnerability
β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”
β”‚ Low           β”‚ Prototype Pollution                                          β”‚
β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
β”‚ Package       β”‚ yargs-parser                                                 β”‚
β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
β”‚ Dependency of β”‚ @wdio/cli [dev]                                              β”‚
β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
β”‚ Path          β”‚ @wdio/cli > yargs > yargs-parser                             β”‚
β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
β”‚ More info     β”‚ https://nodesecurity.io/advisories/1500                      β”‚
β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜

Change 602169 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] bridge: Update dependencies to pick up security fixes

https://gerrit.wikimedia.org/r/602169

Change 602175 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] TR: Update dependencies to pick up security fixes

https://gerrit.wikimedia.org/r/602175

@Addshore Maybe I'm missing something obvious but looking at https://gerrit.wikimedia.org/r/c/integration/config/+/573235/3/jjb/job-templates.yaml it does only checks the main wikibase package.json and not the important ones like bridge, TR, termbox (honestly, running npm audit on these submodules exploded majestically)

Is that intended?

Change 602177 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] Update wikimedia-stylint-config

https://gerrit.wikimedia.org/r/602177

I see there’s already a home-brewed solution to this problem but nevertheless I'd like to leave something here:
https://dependabot.com/ - this is a bot that regularly checks for updates in a repo's dependencies. If any are found it updates them and creates a pull request. It works for various languages.
This is how it looks (on a private project of mine) - https://github.com/tzhelyazkova/schema-form-prototype/pull/3
It should be possible to be configured to work with gerrit, here's the core lib - https://github.com/dependabot/dependabot-script

I see there’s already a home-brewed solution to this problem but nevertheless I'd like to leave something here:

Maybe I'm missing something obvious, but that supposed to work in Github only and the ones here are in gerrit (can we run dependabot in gerrit? I'd be really surprised).

Change 602169 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] bridge: Update dependencies to pick up security fixes

https://gerrit.wikimedia.org/r/602169

Maybe I'm missing something obvious, but that supposed to work in Github only and the ones here are in gerrit (can we run dependabot in gerrit? I'd be really surprised).

It is now native to GitHub, but I cannot think of a reason this bot cannot work with gerrit.
The last link in my message describes how it works in more detail, and it mentions how to set the bot up with GitLab.

Again, I just wanted to let y'all know a solution already exists, and it does more than what our thing does. So we might want to weigh in the pros and cons of both.
I did not spend a lot of time researching dependabot, so I don't know how to integrate it with gerrit.

For all I care this could be a Beer and Cake project :)

So, there is a bot for gerrit that does various upgrades for extensions etc.
And this now works for Wikibase https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/605191
It also reports vulnerable packages to https://libraryupgrader2.wmflabs.org/vulns/npm

Having this work on multiple package.json files in a single repo is T228527: Support nested package.json files

WMDE-leszek changed the task status from Open to Stalled.Dec 1 2020, 2:48 PM

Potentially will be solved magically by T228527

Addshore changed the task status from Stalled to Open.Mar 2 2021, 1:36 PM
Addshore updated the task description. (Show Details)

We might skip this in story time, as the phrasing of the description now sounds like we just need to make a decision as a team and then write a ticket for doing it (or make a cal event)

If the approach boils down to "Patch Tuesday", I'd assign this to management. We've had taken a first stab on introducing this as a "team activity" in the late 2020. Initial implementation failed due to the gallore of other "special" things happening in Deecember, but kickstarting it again would be easy and fairly non controversial.
If there is no technical/code work involved, I would not see it as a campsite task no more.

If there is no technical/code work involved, I would not see it as a campsite task no more.

I think if at estimation the technical solution is small it may be worth it (for more promptly noticing these things).
I think the manual solution would need to be facilitated with a list (such as the one we discussed in a retro today).
@WMDE-leszek I was going to propose asking the wider team to see if we as a whole would be happy with just an email reminder to do it.

In terms of reducing developer toil I think more time would likely be saved for implementing the technical solution rather than having someone manually run npm audit in 20+ places or so ever week or so

Oh, I missed the possible alternative approach, apologies (got too excited this is talking about the "team day" only as the comment mentioned a calendar event)

As I will not be able to join the Planning/Story time, my input to the discussion here: If the "implementing the technical solution" is preferred what is not yet defined is what to do in cases where trivial automated fix are not sufficient (basically the essential reason to actually have really people look into those deps)

In terms of the technical solution, this would only be to serve us a notification, not to do the actual fix.
So upon notification then someone would need to go look at the result of the audit and determine what libraries need updating etc.

what is not yet defined is what to do in cases where trivial automated fix are not sufficient

Thus this bit would be done on a case by case basis.

The jenkins job for the root of wikibase has actually been running for over a year now!
https://integration.wikimedia.org/ci/job/wikibase-daily-npm-audit-daily-node10-npmaudit-docker
I actually can't see that it has failed all year (probably because library upgrader also runs on the root of the repo updating the easy things that can be)

FWIW lerna-audit now (v1.3.1) does not show the problems anymore which we encountered earlier (e.g.) when using it to tend to monorepos.

noarave removed the point value for this task.

This was picked up on campsite for now with the AC of:

Acceptance criteria β›Ίβœ¨ :
Know all of the places that we have package.json files that need to be audited (and their current state in temrs of audit being run)

A dump of our repos to consider can be found at https://phabricator.wikimedia.org/P14784

Jakob_WMDE added a subscriber: Jakob_WMDE.

I went through the list provided in T244001#6904372 and for each of them checked whether they contained one or more package.json files and whether the dependencies in that file are automatically audited. For the Github ones I only checked whether Dependabot is enabled. (Not sure if there is a way to have it enabled but not do anything useful.) In one case (wikit) I know that it's enabled but not auditing nested packages and marked that in the spreadsheet. For the gerrit ones I checked https://gerrit.wikimedia.org/r/plugins/gitiles/labs/libraryupgrader/config/+/refs/heads/master/repositories.json which contained very few of the repositories. I'm not sure whether I'm missing something there. https://libraryupgrader2.wmcloud.org/vulns/npm?branch=master contains some repositories which aren't the the aforementioned repositories.json config.

Spreadsheet: https://docs.google.com/spreadsheets/d/1ZAo5o6aTyfSRuMoQWjwoXeWTk-m61Py8_k46VZw_5yM/edit

Change 691265 had a related patch set uploaded (by Addshore; author: Addshore):

[mediawiki/extensions/WikibaseManifest@master] Dependabot -> Gerrit, Github Action

https://gerrit.wikimedia.org/r/691265

I looked once again at the draft attempt at creating an action for this (was https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/626687)
It is now reimagined in the change above which has been tested.

My bot account (Addbot) is currently used.
We can trial this on the WikibaseManifest extension, see how it goes, and maybe roll this out more later.

I should also look at making this easily reusable and maintainable.
https://docs.github.com/en/actions/creating-actions/creating-a-composite-run-steps-action

Investigation is done, state is known, and a potential next step has been explored and will be followed up in another ticket (that I will backlink)

Change 602175 abandoned by Ladsgroup:

[mediawiki/extensions/Wikibase@master] TR: Update dependencies to pick up security fixes

Reason:

Already done.

https://gerrit.wikimedia.org/r/602175

Change 602177 abandoned by Ladsgroup:

[mediawiki/extensions/Wikibase@master] Update wikimedia-stylint-config

Reason:

Already done.

https://gerrit.wikimedia.org/r/602177