facebook / docusaurus

Easy to maintain open source documentation websites.
https://docusaurus.io
MIT License
55.73k stars 8.35k forks source link

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

Closed mgwidmann closed 3 years ago

mgwidmann commented 3 years ago

🐛 Bug Report

Prerequisites

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 https://github.com/npm/rfcs/pull/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 https://github.com/npm/rfcs/pull/18 )

Your environment

Reproducible demo

See above reproduce steps to build a local environment.

lex111 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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: https://github.com/facebook/docusaurus/issues/3552#issuecomment-763623781

lex111 commented 3 years ago

@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).