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

[wip] yarn 1.16, safety, audit in CI 🧶🧷🚨 #6336

Closed
wants to merge 1 commit into from

Conversation

bollwyvl
Copy link
Contributor

References

Code changes

User-facing changes

  • None yet.

Backwards-incompatible changes

  • None.

Alternatives

While upgrading yarn seems like a fine idea, and audit could allow us to help users understand potential ramifications of installing extensions, we could alternatively go with...

audit-ci

IBM/audit-ci is much more configurable than the built-in yarn audit. For example, for Reasons, let's say we can't upgrade marked right now, which generates a couple hundred moderate warnings. audit-ci would let us ignore the specific, numbered advisories related to regex explosion by adding a line to its config file. This would give us an "out" when a vulnerability shows up, as the chances of a well-sorted JSON file leading to merge conflicts from multiple PRs is far less impactful than a bunch of package.json and yarn.lock changes.

snyk.io, pyup.io

Instead of rolling our own, go with bot-based services. Both offer free accounts for open source projects. As I mentioned on #6331, yarn audit depends on the largesse of npm, but is a POST request of basically our whole yarn.lock, which seems to 503 fairly frequently (at least in the last 24 ours). By having the bots call us, we'll likely get a better workflow, and burn less CI coal.

🛎️ @jasongrout

@jupyterlab-dev-mode
Copy link

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

@bollwyvl
Copy link
Contributor Author

So it found one high in staging:


++ jlpm audit --level high
~/work/1/s/jupyterlab/staging ~/work/1/s
yarn audit v1.16.0
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ high          │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ handlebars                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=4.0.14 <4.1.0 || >=4.1.2                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ handlebars                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ handlebars                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://www.npmjs.com/advisories/755                         │
└───────────────┴──────────────────────────────────────────────────────────────┘
231 vulnerabilities found - Packages audited: 618847
Severity: 230 Moderate | 1 High

Hooray data! However, it didn't make it to the dev yarn.lock.

Perhaps the right place to put these in CI in an xunit so it can be aggregated with other stuff. I set about doing a bit of this (lazily, templating with jinja, which you Should Never Do, Right? because stdlib xml is awful) and got to thinking: it might in fact make sense to make a jupyter lab audit right out of the gate.

In the initial case of using it for CI, it should be able to emit --xunit (or --json... heck, all of our commands should have a --json, but that's another story). It would need a --dev-mode (not strictly accurate, but...) to pick up the root yarn.lock.

(Paranoid) end users would be able to make use of it, and heck, lab build could even accept an --audit flag to reuse the machinery before even attempting the build... though who knows what damage would already be done at that point. Anyhooo...

Building it "right" would also mean it would work with custom --app-dirs. I would be pretty happy to have this information for my crazy confections.

Thoughts?

@bollwyvl
Copy link
Contributor Author

@blink1073 blink1073 added this to the 1.1 milestone May 24, 2019
@blink1073 blink1073 modified the milestones: 1.1, 1.2 Aug 27, 2019
@bollwyvl bollwyvl mentioned this pull request Sep 19, 2019
25 tasks
@jasongrout jasongrout modified the milestones: 1.2, 2.0 Oct 10, 2019
@jasongrout
Copy link
Contributor

@bollwyvl - do you want to take this up for jlab 2.0? We'd like to minimally upgrade yarn, but if some more of this work can get in, that would be great too.

@jasongrout jasongrout modified the milestones: 2.0, 3.0 Dec 2, 2019
@vidartf
Copy link
Member

vidartf commented Dec 2, 2019

Some xrefs: #3759 #333

@blink1073
Copy link
Member

Closing as stale

@blink1073 blink1073 closed this Aug 26, 2020
@github-actions github-actions bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Feb 23, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. status:Work in Progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to yarn 1.16, use audit (and safety)?
4 participants