Dependency on insecure version of braces (Node security advisory 786)

[yhoiseth]

Is this a bug report?

Yes.

Did you try recovering your dependencies?

I don't think this step is necessary, due to the error being present in a brand new project.

Which terms did you search for in User Guide?

None.

Environment

Environment Info:

  System:
    OS: macOS 10.14.3
    CPU: x64 Intel(R) Core(TM) i5-4278U CPU @ 2.60GHz
  Binaries:
    Node: 10.15.0 - /usr/local/opt/node@10/bin/node
    Yarn: 1.13.0 - /usr/local/bin/yarn
    npm: 6.4.1 - /usr/local/opt/node@10/bin/npm
  Browsers:
    Chrome: 72.0.3626.109
    Firefox: 65.0
    Safari: 12.0.3
  npmPackages:
    react: ^16.8.2 => 16.8.2 
    react-dom: ^16.8.2 => 16.8.2 
    react-scripts: 2.1.5 => 2.1.5 
  npmGlobalPackages:
    create-react-app: Not Found

Steps to Reproduce

1. yarn create react-app my-app
2. cd my-app/
3. yarn audit

In addition, I've tried to add braces as a top-level dependency using yarn add braces. That didn't help.

Expected Behavior

Pass.

Actual Behavior

Fail:

➜  my-app git:(master) yarn audit
yarn audit v1.13.0
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Regular Expression Denial of Service                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ braces                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=2.3.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ react-scripts                                                │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ react-scripts > babel-jest > babel-plugin-istanbul >         │
│               │ test-exclude > micromatch > braces                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/786                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
…
63 vulnerabilities found - Packages audited: 36332
Severity: 63 Low
✨  Done in 3.12s.

Reproducible Demo

I don't think this is necessary, due to the required steps being very few.

[bugzpodder]

This is a development only package and it should not affect your production build in anyway. It should go away once a newer version of a downstream package that addresses this issue is available.

[sbcreates]

Getting the same vulnerabilities from npm audit regarding braces.

The vulnerability is in versions earlier than 2.3.1 and it seems react-scripts has a nested dependency that uses an earlier version of braces

app
├─┬ http-proxy-middleware@0.19.0
│ └─┬ micromatch@3.1.10
│   └── braces@2.3.2 
└─┬ react-scripts@2.1.5
  ├─┬ babel-jest@23.6.0
  │ └─┬ babel-plugin-istanbul@4.1.6
  │   └─┬ test-exclude@4.2.3
  │     └─┬ micromatch@2.3.11
  │       └── braces@1.8.5 
  ├─┬ fork-ts-checker-webpack-plugin-alt@0.4.14
  │ └─┬ chokidar@2.1.1
  │   └── braces@2.3.2  deduped
  └─┬ jest@23.6.0
    └─┬ jest-cli@23.6.0
      ├─┬ jest-config@23.6.0
      │ └─┬ micromatch@2.3.11
      │   └── braces@1.8.5 
      ├─┬ jest-haste-map@23.6.0
      │ └─┬ micromatch@2.3.11
      │   └── braces@1.8.5 
      ├─┬ jest-message-util@23.4.0
      │ └─┬ micromatch@2.3.11
      │   └── braces@1.8.5 
      ├─┬ jest-runtime@23.6.0
      │ └─┬ micromatch@2.3.11
      │   └── braces@1.8.5 
      └─┬ micromatch@2.3.11
        └── braces@1.8.5 

More information on the issue is here.

Do we wait for react-scripts to update the dependency in question?

[bugzpodder]

Unless jest releases a new 23.x version with updated dependencies to micromatch@3 (unlikely since this is a breaking change), or CRA updates to jest 24, I don't think there is an easy workaround for this issue.

[sorahn]

jestjs/jest#7917

As it stands right now, Jest is not planning on releasing a patch for v23.

[alexch]

See #6064 #6109 #5777 #5618 for earlier times something like this happened; I think it's useful to draw the connection between these recurring issues.

#6064 (comment) explains the reasoning behind this project's tradeoff between dependency pessimism (pinning, delayed upgrades, and enduser frustration) and dependency optimism (allow users to use npm audit fix to apply upgrades with ^ or ~, but remain exposed to the risk that a nested dependency might push a patch upgrade that contains a hidden bug) and give readers of this current issue a pointer to previous discussions and explanations.

I do see that in this case, Jest fixed the issue with a major version bump, not a patch, so even a SemVer "minors and patches OK" "babel-jest": "^23.6.0" wouldn't have helped this time; >=23.6.0 would but that's probably too optimistic (even for me :-) ).

This is a development only package and it should not affect your production build in anyway.

@bugzpodder That is a very confident prediction! Unfortunately, in fact, I can't push to production via my CI pipeline when my tests fail, and this problem causes react-scripts test to fail (since my React client directory is nested under a project root directory containing a NodeJS server, and the server's dependencies are unpinned, so the server got Jest 24.x smoothly via npm audit fix). We're not in active release mode right now, so I'm not personally affected by this particular glitch, but I'm confident there are others who are.

UPDATE: For my nested-node-project setup, I had to add SKIP_PREFLIGHT_CHECK=true to a .env file in the child project dir to allow Jest 23.6.0 in the client to peacefully coexist with Jest 24.1.0 in the server.

ALSO: I know open source development can be a thankless task, so BIG THANKS to everyone who's working on all these interrelated projects and the tangled dependency webs they weave.

[SimenB]

FWIW we originally bumped Micromatch in a minor of Jest 23, but it broke a ton of setups (notably on windows), so we reverted it: jestjs/jest#6661

[bugzpodder]

@alexch, your production build (i.e. running react-scripts build) will not be affected by this security vulnerability in any way. You'll get a notice about this low risk vulnerability if you run npm install, but otherwise an end user should not be affected by this issue. There is no need to rush upgrading to Jest 24.

[Tasemu]

This is causing issues with us being able to deploy our front-end through our CI also. Going to keep an eye out here for any movement. :)

[SimenB]

Nothing will happen here unless a patch is released for an old version of micromatch or a new major of CRA is released. Using a newer major of micromatch is a breaking change.

Seems like your CI should just check production dependencies, not dev deps. Doesn't seem like npm supports it, though 🤷‍♂️

https://npm.community/t/please-support-production-or-only-production-in-npm-audit/3005
npm/rfcs#18

[aboutlo]

WORKAROUND

npm audit --registry=https://registry.npmjs.org --parseable | wc -l
returns the number of vulnerabilities found

you can exclude low vulnerabilities with
npm audit --registry=https://registry.npmjs.org --parseable | grep -V https://nodesecurity.io/advisories/786 | wc -l

[naugtur]

Instead of using a workaround (which ignores the advisory in all modules you installed) you can use https://www.npmjs.com/package/npm-audit-resolver that wraps audit in fine-grained tools to manage your security decision

[jdalegonzalez]

@bugzpodder

This is a development only package and it should not affect your production build in anyway. It should go away once a newer version of a downstream package that addresses this issue is available.

I've seen messages like this a number of times for a variety of reported security vulnerabilities in various packages and it's not a particularly satisfying response for me but maybe I'm thinking about things the wrong way.

Is the expected flow:

• see reported security vulnerability in npm (or yarn).
• lookup that flow
• attempt to determine if the vulnerability will impact the product env or not
• if it's determined it won't, come up with some workaround that suppresses that message from the log
• if it's determined it will, post a bug

That flow seems really likely to allow something that wasn't expected to cause a production issue to end up causing one.

I had been operating under the expectation that the flow was:

• see reported security vulnerability
• immediately attempt to implement a fix - without worrying about whether or not it was expected to cause a prod issue.
• end of flow

Under that flow, we're less likely to let flaws creep into our projects but it means responses like "it only impacts dev" or "it's only a command-line thing" are insufficient.

Maybe the issue with with npm and yarn? Maybe they should have some sort of "dev only, prod and dev" flags or "mark as 'ok for me and don't tell me again'" like other vuln scanners have?

[alexch]

@bugzpodder If react-scripts build were my entire process for doing a production build, I would agree with you. But my process for development and deployment also includes running a suite of unit and acceptance tests; to make them work I need to disable preflight checks... which is fine temporarily, but that allows other dependency issues to creep in, and is also one more thing to remember to undo later.

@jdalegonzalez I think you're right that different teams/projects have different workflow expectations, and different levels of reliance on so-called "dev-only" packages, and that your latter flow ("apply fix ASAP") is much preferred over a flow that requires every project owner to deeply understand vulnerabilities and updates and workarounds and release schedules.

[bugzpodder]

@alexch you don't need to disable preflight checks if you install the same version of jest (23.6 instead of 24) in your node code. If you decide to use Jest 24, then disabling preflight checks is a perfectly valid course of action for your decision.

@jdalegonzalez this is unfortunately the difference between reality and an ideal world. Ideally you would run a command that would fix everything, but the reality is that APIs change between versions (especially majors), we have deeply nested dependencies, and most open source projects don't have the resources to back port each fix to older versions. All of your npm modules have various version dependencies (eg Jest 23 uses micromatch 2 but eslint might use micromatch 3 at the same time) so you can't always have the latest packages for every nested dependency. So yes, it needs to be on a case-by-case basis. If you are on a team then one person should be the POC for security related issues. If you aren't sure about a particular vulnerability assessment, issues are one of the perfectly valid ways of asking for help.