[core] Upgrade dependencies from yarn audit #16625

merceyz · 2019-07-17T22:16:24Z

I have followed (at least) the PR section of the contributing guide.

An attempt to make yarn audit happier after https://snyk.io/blog/snyk-research-team-discovers-severe-prototype-pollution-security-vulnerabilities-affecting-all-versions-of-lodash/

Initial
1160 vulnerabilities found - Packages audited: 82118
Severity: 1 Low | 3 Moderate | 1156 High

Upgraded lodash
656 vulnerabilities found - Packages audited: 82118
Severity: 1 Low | 3 Moderate | 652 High

Update lodash.mergewith
655 vulnerabilities found - Packages audited: 82118
Severity: 1 Low | 3 Moderate | 651 High

Reinstall next to update it's dependencies
652 vulnerabilities found - Packages audited: 82147
Severity: 1 Low | 651 High

Reinstall rollup-plugin-size-snapshot to update it's dependencies
577 vulnerabilities found - Packages audited: 77643
Severity: 1 Low | 576 High

Update mixin-deep
385 vulnerabilities found - Packages audited: 77643
Severity: 1 Low | 384 High

Update set-value
193 vulnerabilities found - Packages audited: 77643
Severity: 1 Low | 192 High

Update union-value
1 vulnerabilities found - Packages audited: 78219
Severity: 1 Low

mui-pr-bot · 2019-07-17T22:25:26Z

Details of bundle changes.

Comparing: 3b6c822...68512f2

bundle	parsed diff	gzip diff	prev parsed	current parsed	prev gzip	current gzip
@material-ui/core	0.00%	0.00%	327,255	327,255	90,349	90,349
@material-ui/core/Paper	0.00%	0.00%	68,477	68,477	20,410	20,410
@material-ui/core/Paper.esm	0.00%	0.00%	61,761	61,761	19,177	19,177
@material-ui/core/Popper	0.00%	0.00%	28,896	28,896	10,394	10,394
@material-ui/core/Textarea	0.00%	0.00%	5,534	5,534	2,369	2,369
@material-ui/core/TrapFocus	0.00%	0.00%	3,753	3,753	1,577	1,577
@material-ui/core/styles/createMuiTheme	0.00%	0.00%	16,156	16,156	5,816	5,816
@material-ui/core/useMediaQuery	0.00%	0.00%	3,098	3,098	1,310	1,310
@material-ui/lab	0.00%	0.00%	141,699	141,699	43,813	43,813
@material-ui/styles	0.00%	0.00%	51,886	51,886	15,380	15,380
@material-ui/system	0.00%	0.00%	15,576	15,576	4,445	4,445
Button	0.00%	0.00%	79,711	79,711	24,358	24,358
Modal	0.00%	0.00%	14,548	14,548	5,102	5,102
Portal	0.00%	0.00%	3,471	3,471	1,568	1,568
Rating	0.00%	0.00%	70,267	70,267	22,068	22,068
Slider	0.00%	0.00%	75,096	75,096	23,311	23,311
colorManipulator	0.00%	0.00%	3,904	3,904	1,543	1,543
docs.landing	0.00%	0.00%	54,338	54,338	13,762	13,762
docs.main	+0.02% 🔺	+0.03% 🔺	647,615	647,768	204,253	204,322
packages/material-ui/build/umd/material-ui.production.min.js	0.00%	0.00%	299,670	299,670	86,116	86,116

Generated by 🚫 dangerJS against 68512f2

merceyz · 2019-07-17T22:50:30Z

The last one can't be updated because of ~

yarn audit v1.17.3
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Regular Expression Denial of Service                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ debug                                                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >= 2.6.9 < 3.0.0 || >= 3.1.0                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ vrtest                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ vrtest > image-diff > gm > debug                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://www.npmjs.com/advisories/534                         │
└───────────────┴──────────────────────────────────────────────────────────────┘
1 vulnerabilities found - Packages audited: 78219
Severity: 1 Low

eps1lon · 2019-07-18T06:32:24Z

90% of the time security issues in dev dependenciens aren't actual issues. Would be nice if we could automate that whole process to keep track of when deps get outdated and what changed so that we can decide if we want to upgrade.

merceyz · 2019-07-18T10:46:47Z

90% of the time security issues in dev dependenciens aren't actual issues

In this case they don't seem to affect us in any way

Would be nice if we could automate that whole process to keep track of when deps get outdated and what changed so that we can decide if we want to upgrade.

There is a bot owned/made by GitHub for this https://github.com/marketplace/dependabot-preview

eps1lon · 2019-07-18T10:52:58Z

There is a bot owned/made by GitHub for this github.com/marketplace/dependabot-preview

I'm testing it on my fork currently but it doesn't seem to pick up anything. It's perfect since we can configure it to only run weekly.

I doubt it can pick up that we mix app with library code in the repository though.

oliviertassinari · 2019-07-18T20:42:19Z

I have personally disabled the GitHub automatic security pull requests on most of my small projects. It's noise. I would be more worried if I was hosting the source of final end-product.

In the case of Material-UI, we start to be high-profile, we have to be cautious. However, I agree with @eps1lon. If we had to choose between the two, I think that we should invest our time in keeping the dependencies up to date rather than spending time on dev dependencies security issues. I used to upgrade the dependencies every week. I was enjoying doing it. Time has passed and realized that I was doing it mostly to satisfy my curiosity. A new ritual or a new bot could do it.

oliviertassinari · 2019-07-19T08:38:09Z

There is a yarn conflict.

eps1lon · 2019-07-19T09:24:16Z

I've enabled dependabot for https://github.com/eps1lon/material-ui/ and will see how this works out. We have to be careful since we mix app and library code. We need to prevent upgrades of library dependencies that only change the lockfile but not the package.json. Some other issues as well.

Will test it for a week and then see if there are outstanding issues. It already helps a lot to get automatic updates with links to the changelog. That should already reduce workload by a lot.

oliviertassinari · 2019-07-19T09:40:00Z

@eps1lon Great. Does the dependabot has an option to batch changes? The n°1 thing I think that we should avoid, is to have a constant flow, every few days, of dependency update on the repository. It would only keep us busy. While we can put our time at better use.
It's probably fine to upgrade the dependencies every 3 months. But I think that we can aim for a shorter cycle, an update of the dependencies every few weeks [1-4] could do it. I don't know what would be the best characteristic time for us. We will need to experiment.

eps1lon · 2019-07-19T09:42:01Z

Great. Does the dependabot has an option to batch changes?

Yes. live, daily, weekly, monthly. security only and then all sorts of commonds for specific dependencies: eps1lon#15

oliviertassinari · 2019-07-19T09:51:14Z

Yes. live, daily, weekly, monthly

Perfect, so for instance, we could expect a single pull request with all the updates every week 👌 .

merceyz · 2019-07-19T10:10:40Z

There is a yarn conflict.

@oliviertassinari I'll get to it 👍

we could expect a single pull request with all the updates every week

@oliviertassinari There is one PR per dependency

We need to prevent upgrades of library dependencies that only change the lockfile but not the package.json

@eps1lon https://www.npmjs.com/package/syncyarnlock
It currently doesn't respect constraints so when I tested it, it downgraded some of the packages, I have a fix for it locally.

oliviertassinari · 2019-07-19T10:29:40Z

There is one PR per dependency

@merceyz The advantage is that we can more easily isolate which dependency upgrade break what. This was my pain point n°1 in the past when batching the changes manually. From time to time, it was time-consuming to identify which upgrade break what (the last occurrence I remember was related to TypeScript and my lack of knowledge of it, I didn't know if the CI fail was an issue with the typescript engine, the linter or one of the third-party dependencies). But maybe this pain point is marginal to the gain of batching? I don't know. I think that we need to make sure all these PRs, over the course of, let's a 1 month, won't disturb us too much. For instance, https://github.com/react-bootstrap/react-bootstrap/commits/master, it's 37 pull-requests a week they manually accept. That's doesn't look OK.

merceyz · 2019-07-19T11:03:05Z

it's 37 pull-requests a week they manually accept. That's doesn't look OK.

Now is the time to adopt conventional commits so you don't have to look at the commits 😄
https://www.conventionalcommits.org
https://github.com/conventional-changelog/standard-version

There is a yarn conflict.

Rebased

oliviertassinari · 2019-07-19T17:18:33Z

Now is the time to adopt conventional commits so you don't have to look at the commits

Interesting, how do you see this commit convention solve pain points we face here? (I don't see the link with dependabot and I'm interested in learning more of the other dimensions it would help us with).

merceyz · 2019-07-19T17:32:24Z

how do you see this commit convention solve pain points we face here?

You said that the commit history doesn't look good with 37 commits from dependabot, the joke was that if you use conventional commits you don't have to look at the commit history to get a changelog.
Commits from dependabot would be filtered out.

I'm interested in learning more of the other dimensions it would help us with

Would mostly just automate the changelog process, and (IMO) make it easier to see what happened in a commit as people would be forced to be specific.
Just looking at the commit history, commits like [docs] Batch changes doesn't really say what happened, so at a glance, I've got no clue what changed

oliviertassinari · 2019-07-19T17:48:55Z

Oh, my concern what at 70% with the fact that the React Bootstrap team has reviewed and manually approved, on average, 5 pull requests a day. This sounds like something that keeps them busy, not making them more productive. Could they save this context shift? 30% with the fact that people looking at the change history will see 50% of noise in the commits (we also merge, on average 5 PRs a day).

I use the follow hacky script to generate the changelog:

Array.from(document.querySelectorAll('.commit'))
  .map(node => {
    return `- ${node.querySelector('.commit-message a').getAttribute('title').split('\n')[0]} ${node.querySelector('.avatar img').getAttribute('alt')}`;
  })
  .sort()
  .join('\n');

I'm sure it could be improved. I have count that I spend on average 48h a year just to do the release. Spending a days to further automate it would make sense from a business perspective.

merceyz changed the title ~~[core] Update dependencies from audit~~ [core] Update dependencies from audit [ci skip] Jul 17, 2019

merceyz changed the title ~~[core] Update dependencies from audit [ci skip]~~ [core] Update dependencies from yarn audit Jul 17, 2019

merceyz marked this pull request as ready for review July 17, 2019 22:47

merceyz force-pushed the audit branch from 5260d55 to 6fcef16 Compare July 18, 2019 13:28

oliviertassinari approved these changes Jul 18, 2019

View reviewed changes

oliviertassinari added the core label Jul 18, 2019

oliviertassinari added the PR: good for merge label Jul 18, 2019

merceyz changed the title ~~[core] Update dependencies from yarn audit~~ [core] Upgrade dependencies from yarn audit Jul 18, 2019

merceyz added 7 commits July 19, 2019 12:15

Upgraded lodash

e4cb44c

lodash.mergewith

7e06e20

mixin-deep

721e098

set-value

f9258f4

union-value

eff5a1c

rollup-plugin-size-snapshot

6d1388b

nextjs

68512f2

merceyz force-pushed the audit branch from 6fcef16 to 68512f2 Compare July 19, 2019 10:21

oliviertassinari merged commit b977023 into mui:master Jul 19, 2019

merceyz deleted the audit branch July 19, 2019 17:32

snyk-bot mentioned this pull request Sep 4, 2019

[Snyk] Upgrade @material-ui/styles from 4.3.0 to 4.3.3 iliutastoica/React-Discord-Clone#5

Open

zannager added the core Infrastructure work going on behind the scenes label Feb 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] Upgrade dependencies from yarn audit #16625

[core] Upgrade dependencies from yarn audit #16625

merceyz commented Jul 17, 2019 •

edited

Loading

mui-pr-bot commented Jul 17, 2019 •

edited

Loading

merceyz commented Jul 17, 2019 •

edited

Loading

eps1lon commented Jul 18, 2019

merceyz commented Jul 18, 2019

eps1lon commented Jul 18, 2019

oliviertassinari commented Jul 18, 2019 •

edited

Loading

oliviertassinari commented Jul 19, 2019

eps1lon commented Jul 19, 2019

oliviertassinari commented Jul 19, 2019 •

edited

Loading

eps1lon commented Jul 19, 2019

oliviertassinari commented Jul 19, 2019 •

edited

Loading

merceyz commented Jul 19, 2019

oliviertassinari commented Jul 19, 2019 •

edited

Loading

merceyz commented Jul 19, 2019

oliviertassinari commented Jul 19, 2019 •

edited

Loading

merceyz commented Jul 19, 2019 •

edited

Loading

oliviertassinari commented Jul 19, 2019 •

edited

Loading

[core] Upgrade dependencies from yarn audit #16625

[core] Upgrade dependencies from yarn audit #16625

Conversation

merceyz commented Jul 17, 2019 • edited Loading

mui-pr-bot commented Jul 17, 2019 • edited Loading

merceyz commented Jul 17, 2019 • edited Loading

eps1lon commented Jul 18, 2019

merceyz commented Jul 18, 2019

eps1lon commented Jul 18, 2019

oliviertassinari commented Jul 18, 2019 • edited Loading

oliviertassinari commented Jul 19, 2019

eps1lon commented Jul 19, 2019

oliviertassinari commented Jul 19, 2019 • edited Loading

eps1lon commented Jul 19, 2019

oliviertassinari commented Jul 19, 2019 • edited Loading

merceyz commented Jul 19, 2019

oliviertassinari commented Jul 19, 2019 • edited Loading

merceyz commented Jul 19, 2019

oliviertassinari commented Jul 19, 2019 • edited Loading

merceyz commented Jul 19, 2019 • edited Loading

oliviertassinari commented Jul 19, 2019 • edited Loading

merceyz commented Jul 17, 2019 •

edited

Loading

mui-pr-bot commented Jul 17, 2019 •

edited

Loading

merceyz commented Jul 17, 2019 •

edited

Loading

oliviertassinari commented Jul 18, 2019 •

edited

Loading

oliviertassinari commented Jul 19, 2019 •

edited

Loading

oliviertassinari commented Jul 19, 2019 •

edited

Loading

oliviertassinari commented Jul 19, 2019 •

edited

Loading

oliviertassinari commented Jul 19, 2019 •

edited

Loading

merceyz commented Jul 19, 2019 •

edited

Loading

oliviertassinari commented Jul 19, 2019 •

edited

Loading