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

Docusaurus does not allow for a passing npm audit in CI/CD pipelines #5501

Closed
4 tasks done
mgwidmann opened this issue Sep 7, 2021 · 5 comments
Closed
4 tasks done
Labels
closed: please-fix-this-cve This issue is asking for fixing a CVE in a build-only dep which doesn't pose any real threat.

Comments

@mgwidmann
Copy link

🐛 Bug Report

Prerequisites

  • I'm using the latest version of Docusaurus.
  • I have tried the npm run clear or yarn clear command.
  • I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • [-] I have tried creating a repro with https://new.docusaurus.io
  • I have read the console error message carefully (if applicable)

Description

It is expected that npx @docusaurus/init@latest init my-website classic will not install dependencies with known CVE issues. However, npm audit returns vulernabilities. Even using npm audit --fix also does not allow for overriding them. Below are two examples of vulnerabilities. This prevents a project with a CI/CD pipeline (using npm audit) from using docusaurus since it will fail the build indefinitely (see the RFC on fixing this npm/rfcs#18).

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Regular Expression Denial of Service in trim                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ trim                                                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.0.3                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @docusaurus/preset-classic                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @docusaurus/preset-classic > @docusaurus/theme-classic >     │
│               │ @mdx-js/mdx > remark-parse > trim                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1700                            │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │ Regular Expression Denial of Service                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ browserslist                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=4.16.5                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @docusaurus/core                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @docusaurus/core > react-dev-utils > browserslist            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1747                            │
└───────────────┴──────────────────────────────────────────────────────────────┘

Have you read the Contributing Guidelines on issues?

Yes. This is not a security vulnerability that is not already publicly known, this is just reporting the fact that docusaurus does not allow upgrading to remove packages with known CVEs and is therefore a bug in docusaurus's dependency tree.

Steps to reproduce

  1. mkdir docusaurus-playground
  2. cd docusaurus-playground
  3. npx @docusaurus/init@latest init my-website classic
  4. npm i --package-lock-only
  5. npm audit --prod

Expected behavior

NPM should find no vulnerabilities or npm audit --fix should fix them.

Actual behavior

found 94 vulnerabilities (68 moderate, 26 high) in 1337 scanned packages
  94 vulnerabilities require manual review. See the full report for details.

Running npm audit --fix does not allow for overriding any vulnerabilities. If a CI/CD pipeline is built with npm audit this blocks the pipeline without any way to unblock it (see discussion about this issue in the RFC npm/rfcs#18 )

Your environment

  • Public source code: N/A
  • Public site URL: N/A
  • Docusaurus version used: 2.0.0-beta.6
  • Environment name and version (e.g. Chrome 78.0.3904.108, Node.js 10.17.0): v14.17.5
  • Operating system and version (e.g. Ubuntu 20.04.2 LTS): Mac OSX

Reproducible demo

See above reproduce steps to build a local environment.

@mgwidmann mgwidmann added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Sep 7, 2021
@lex111
Copy link
Contributor

lex111 commented Sep 8, 2021

First of all, I recommend that you read this issue to learn more about the problem when using npm audit. Docusaurus as well as CRA is a build tool, so everything described in that issue applies to it. In short, at this point there is nothing to worry about if npm audit has found some vulnerabilities.

If it is important for you that npm audit succeed, you can either move @docusaurus/preset-classic dependency in devDependencies field or you can try to use resolutions tools like npm-force-resolutions.

@mgwidmann
Copy link
Author

Moving it to devDependencies worked to clear npm audit. Does this mean you have no intention of upgrading the packages that are flagged by npm? It seems a healthy regular cadence of updating dependencies would fix this problem. Take trim for example. Its no longer used in the latest versions of @mdx-js/mdx > remark-parse, so if @docusaurus/theme-classic updated its version of @mdx-js/mdx this vulnerability would go away.

I understand if you want to make the argument that since its a build tool its not vulnerable to these issues but perhaps by keeping dependencies up to date you can meet people half way. Then its an easier argument to make since there would be no action Docusaurus can take to remedy the problem.

@lex111
Copy link
Contributor

lex111 commented Sep 8, 2021

Yes, of course we will update dependencies, we do it periodically now. However, MDX v2 is not ready for production use yet, so we can't update it. However, we will upgrade webpack-dev-server to v4 (chokidar) soon. When new version of CRA (react-dev-utils) is available, we will also update this dependency. So, there's no reason to be worried about this issue.

@HonkingGoose
Copy link
Contributor

HonkingGoose commented Sep 9, 2021

Moving it to devDependencies worked to clear npm audit. Does this mean you have no intention of upgrading the packages that are flagged by npm? It seems a healthy regular cadence of updating dependencies would fix this problem. Take trim for example. Its no longer used in the latest versions of @mdx-js/mdx > remark-parse, so if @docusaurus/theme-classic updated its version of @mdx-js/mdx this vulnerability would go away.

To help Docusaurus with updating, the creator of Renovate bot and I created a configuration that should fit the needs of the Docusaurus project. This is basically "ready to go", but Renovate itself needs to be installed into the organization account, and needs to be allowed to run on the facebook/docusaurus repository.

See this issue for the full discussion/details:

If anybody from the Docusaurus team wants to try out Renovate bot for themselves follow the instructions I posted here: #3552 (comment)

@lex111
Copy link
Contributor

lex111 commented Sep 9, 2021

@HonkingGoose thanks, we will consider enabling Renovate bot this month. I'm closing this issue for now in favor of #3552, so the vulnerabilities found relate to packages we can't update yet (apart from webpack-dev-server).

@lex111 lex111 closed this as completed Sep 9, 2021
@Josh-Cena Josh-Cena added closed: wontfix A fix will bring significant overhead, or is out of scope (for feature requests) and removed status: needs triage This issue has not been triaged by maintainers labels Feb 21, 2022
@Josh-Cena Josh-Cena added closed: please-fix-this-cve This issue is asking for fixing a CVE in a build-only dep which doesn't pose any real threat. and removed bug An error in the Docusaurus core causing instability or issues with its execution closed: wontfix A fix will bring significant overhead, or is out of scope (for feature requests) labels Apr 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed: please-fix-this-cve This issue is asking for fixing a CVE in a build-only dep which doesn't pose any real threat.
Projects
None yet
Development

No branches or pull requests

4 participants