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

yhoiseth · 2019-02-16T15:35:24Z

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

yarn create react-app my-app
cd my-app/
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.

The text was updated successfully, but these errors were encountered:

bugzpodder · 2019-02-16T19:44:01Z

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 · 2019-02-16T22:45:54Z

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
├─┬ [email protected]
│ └─┬ [email protected]
│   └── [email protected] 
└─┬ [email protected]
  ├─┬ [email protected]
  │ └─┬ [email protected]
  │   └─┬ [email protected]
  │     └─┬ [email protected]
  │       └── [email protected] 
  ├─┬ [email protected]
  │ └─┬ [email protected]
  │   └── [email protected]  deduped
  └─┬ [email protected]
    └─┬ [email protected]
      ├─┬ [email protected]
      │ └─┬ [email protected]
      │   └── [email protected] 
      ├─┬ [email protected]
      │ └─┬ [email protected]
      │   └── [email protected] 
      ├─┬ [email protected]
      │ └─┬ [email protected]
      │   └── [email protected] 
      ├─┬ [email protected]
      │ └─┬ [email protected]
      │   └── [email protected] 
      └─┬ [email protected]
        └── [email protected]

More information on the issue is here.

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

bugzpodder · 2019-02-17T03:04:35Z

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 · 2019-02-17T06:42:09Z

jestjs/jest#7917

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

alexch · 2019-02-17T21:32:04Z

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 · 2019-02-17T22:37:37Z

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 · 2019-02-18T07:46:34Z

@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 · 2019-02-18T10:15:08Z

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 · 2019-02-18T10:24:09Z

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 · 2019-02-18T11:00:22Z

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 · 2019-02-18T14:18:30Z

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 · 2019-02-18T14:57:55Z

@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 · 2019-02-18T15:39:33Z

@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 · 2019-02-18T18:06:11Z

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

ztolley · 2019-02-18T21:14:58Z

Handlebars just joined the party with a High priority vulnerability too

jdalegonzalez · 2019-02-18T23:32:26Z

Yep. That's why we pulled it out. We can live without test coverage reporting

…

On Mon, Feb 18, 2019, 4:15 PM Zac Tolley ***@***.*** wrote: Handlebars just joined the party with a High priority vulnerability too — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6443 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABZ6hKAredtyt7L_aQCyipooTEOkJDXSks5vOxf3gaJpZM4a_CiD> .

dlipeles · 2019-02-19T00:16:00Z

Any ideas for suppressing this on yarn audit? Our CI is currently blocked even though jest is a dev dependency.

Looks like yarn doesn't allow for auditing production-only dependencies.

emilymclean2 · 2019-02-19T03:05:14Z

I don't really understand why, but it seems like this is related to my perfect-scrollbar being on the left of my page instead of the right - I'm getting identical vulnerabilities after running npm audit, but everything else is working fine.

Edit: running Material Dashboard Pro

bugzpodder · 2019-02-19T03:17:45Z

@dlipeles why would this block a CI? Is it because of the preflight check or do you have a build step that hinges on yarn audit succeeding? In any case, if you are yarn, you could use yarn resolutions to pin micromatch to v3 but it might break something, YMMV.

jitenderchand1 · 2019-02-19T09:02:31Z

NPM Audit is part of CI pipeline and due to this issue it is failing every time I pushed the code.

Why this issue started to occurred, I have not updated any package?

bugzpodder · 2019-02-19T09:55:26Z

Is it possible to change your CI pipeline to omit npm audit step? From my experience it takes a non-trivial amount of time to test/deploy/propagate a security fix. In most cases, when a security advisory is posted, either it doesn't affect your production build (such as this case), or that it is a real issue. In the latter case, either a fix might not be immediately available/feasible, or there could be workarounds that might mitigate risk while keeping the package as-is, and blocking all builds until the package is updated might not be the best course of action.

EDIT: This comment isn't meant as a call to ignore security warnings. npm audit has its place and it should be done as part of security audit for your application. It should be monitored and dealt on a case by case basis. IMO it doesn't belong in a CI pipeline for the reasons mentioned above.

jdalegonzalez · 2019-02-19T13:32:48Z

Is it possible to change your CI pipeline to omit npm audit step? From my experience it takes a non-trivial amount of time to test/deploy/propagate a security fix.

@bugzpodder With all due respect, this feels like bad advice to me. While I recognize that right this minute we're in a bit of a pickle as it relates to NPM audit, the idea that we all should ignore security warnings because most of the time they don't impact our app feels a little careless. Almost all high-profile breaches I've ever seen reported start with someone thinking it wasn't a high-priority to patch some previously reported vulnerability. A very low percentage of hacks are exploiting zero-day/unreported vulnerabilities. Within the last six months, there were packages on npm that had the ability to steal cryptocurrency - no impact to the "app" itself, it just made the app a conduit for theft.

If you feel like "for right now" removing the audit from the CI pipeline is necessary while solutions are explored - I would at least sympathize and maybe agree. If you're advocating (like some are) that the standard npm audit be wrapped in some script that filters out those warnings deemed as acceptable risk (again until npm fixes their audit system to support this natively), I would agree too.

Making the blanket suggestion that people stop depending on npm audit as a part of CI altogether because "in most cases" security advisories don't impact "most" production apps feels like a bridge too far.

(of course, that's just my opinion. I could be wrong)

sorahn · 2019-02-19T13:39:44Z

@jitenderchand1

Why this issue started to occurred, I have not updated any package?

The vulnerability was discovered in an existing package.

jdalegonzalez · 2019-02-19T14:07:58Z

A vulnerability was discovered and reported in a package that one of your packages ultimately depends on. You didn’t need to update anything. The vulnerability report is what is new. From: Daryl Roberts <[email protected]> Reply-To: facebook/create-react-app <[email protected]> Date: Tuesday, February 19, 2019 at 8:40 AM To: facebook/create-react-app <[email protected]> Cc: jdalegonzalez <[email protected]>, Mention <[email protected]> Subject: Re: [facebook/create-react-app] Dependency on insecure version of braces (Node security advisory 786) (#6443) @jitenderchand1 Why this issue started to occurred, I have not updated any package? The vulnerability was discovered in an existing package. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

iansu · 2019-02-19T16:50:05Z

Let's focus on the issue here. This is not the place to debate the merits of npm audit or how your CI should be set up.

Since Jest isn't planning to backport this fix to Jest 23 the only thing we can do is release a major version of Create React App with Jest 24. We are currently working on that and you can follow the progress on upgrading to Jest 24 here: #6278

SimenB · 2019-02-19T17:12:12Z

Potentially, you can get a patch into braces/micromatch (although it's v2, with v3 being current major and v4 right around the corner).

Note that upgrading from micromatch 2 to 3 is a breaking change in and of itself, so there's really no way Jest can backport a fix to v23

there is still some unfixed issue in react-script. See: - facebook/create-react-app#6443 - facebook/create-react-app#6278

Waseemrajashaik · 2019-02-25T09:43:04Z

any updates ??

yhoiseth · 2019-02-25T10:59:23Z

@Waseemrajashaik Based on @iansu's comment above, I think #6278 is relevant to your question.

godmar · 2019-02-28T23:04:15Z

Let's focus on the issue here. This is not the place to debate the merits of npm audit or how your CI should be set up.

Agreed. I shared my view on the merits of npm audit in the npm forum. I hope I represented the technical aspects correctly.

ghost · 2019-03-13T01:26:08Z

even thought installed latest CRA it still gives. Any updates?

ianschmitz · 2019-03-13T01:30:21Z

We haven't yet upgraded to Jest 24 which includes the fix but it is in progress. This will be part of the 3.0 release.

There is still an unresolvable dependency issue. See facebook/create-react-app#6443

methodbox · 2019-03-30T20:00:15Z

FWIW, I went through my node_modules folder and searched out each instance of micromatch. I opened the respective folders and the package.json in each and manually changed the braces dependency version to 2.3.2. Jest was not the only culprit; after updating their package.json files I also had to update this in about 8 other packages edit nested inside Jest; so I guess you could say it's all Jest. They were all on 2.3.1, except one that was 1.8.5.

I did npm install to re-scan and install per the new package.jsons.

This resolved the warning (as the fix is >2.3.1) and CRA works fine.

Here's my npm tree now:

edit - I'm parsing this down. My npm tree was frickin huge. Hopefully this makes it easier for others to do the same, instead of digging folder by folder.

+-- [email protected]
| ...
| +-- [email protected]
| | ...
| |   +-- [email protected]
| |   +-- [email protected]
| |   | +-- [email protected]
| |   | `-- [email protected]
| |   |   ...
| |   |   +-- [email protected]
| |   |   | ...
| |   |   | +-- [email protected] 
| |   |   | ...
| |   +-- [email protected]
| |   | ...
| |   | +-- [email protected] 
...
| | +-- [email protected]
| | | ...
| | | +-- [email protected]
| | | | ...
| | | | +-- [email protected]
| | | | | ...
| | +-- [email protected]
| | | ...
| | | +-- [email protected]
| | | | +-- [email protected]
| | | | `-- [email protected]
| | | |   +-- [email protected]
| | | |   | ...
| | +-- [email protected]
| | | ...
| | | +-- [email protected]
| | | |...
| +-- [email protected]
| | ...
| | +-- [email protected]
| | | +...
| | | | ...
| | | +-- [email protected]
...
| | | +-- [email protected]
| | | | +-- [email protected] deduped
| | | | +-- [email protected]
| | | | | ...
| | | | | +-- [email protected]
| | | | | |...
| | +-- [email protected]
| | |...
| | | +-- [email protected] deduped
| | | `-- [email protected]
| | |   ...
| | |   +-- [email protected]
| | |   ...
`-- [email protected]

marcinwasilewski · 2019-03-31T19:35:40Z

Hmm ok so what would be the fastest way to continue working on my old project right now if npm audit fails, barring completely ignoring the issue by installing with --no-audit?

ianschmitz · 2019-03-31T19:45:44Z

For those that need this resolved immediately you can try out our alpha build by following the instructions over at #6475. Closing this for now as it should be resolved and will be part of the 3.0 release.

ianschmitz added the issue: needs investigation label Feb 16, 2019

bugzpodder mentioned this issue Feb 20, 2019

react-scripts has vulnerability in dependencies #6469

Closed

natasha-audrey mentioned this issue Feb 20, 2019

Upgrade to Jest 24 when CRA does LectureGoggles/lecture-goggles-frontend#27

Closed

rdubigny added a commit to betagouv/signup-front that referenced this issue Feb 22, 2019

fix dependency vulnerability

ea9ab6e

there is still some unfixed issue in react-script. See: - facebook/create-react-app#6443 - facebook/create-react-app#6278

rdubigny added a commit to betagouv/signup-front that referenced this issue Mar 20, 2019

npm audit fix

4ea4f50

There is still an unresolvable dependency issue. See facebook/create-react-app#6443

ianschmitz removed the issue: needs investigation label Mar 31, 2019

ianschmitz added this to the 3.0 milestone Mar 31, 2019

ianschmitz closed this as completed Mar 31, 2019

lock bot locked and limited conversation to collaborators Apr 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

yhoiseth commented Feb 16, 2019 •

edited

Loading

bugzpodder commented Feb 16, 2019

sbcreates commented Feb 16, 2019 •

edited

Loading

bugzpodder commented Feb 17, 2019

sorahn commented Feb 17, 2019 •

edited

Loading

alexch commented Feb 17, 2019 •

edited

Loading

SimenB commented Feb 17, 2019

bugzpodder commented Feb 18, 2019

Tasemu commented Feb 18, 2019

SimenB commented Feb 18, 2019

aboutlo commented Feb 18, 2019

naugtur commented Feb 18, 2019

jdalegonzalez commented Feb 18, 2019

alexch commented Feb 18, 2019

bugzpodder commented Feb 18, 2019 •

edited

Loading

ztolley commented Feb 18, 2019

jdalegonzalez commented Feb 18, 2019 via email

dlipeles commented Feb 19, 2019 •

edited

Loading

emilymclean2 commented Feb 19, 2019 •

edited

Loading

bugzpodder commented Feb 19, 2019

jitenderchand1 commented Feb 19, 2019 •

edited

Loading

bugzpodder commented Feb 19, 2019 •

edited

Loading

jdalegonzalez commented Feb 19, 2019

sorahn commented Feb 19, 2019

jdalegonzalez commented Feb 19, 2019 via email

iansu commented Feb 19, 2019

SimenB commented Feb 19, 2019

Waseemrajashaik commented Feb 25, 2019

yhoiseth commented Feb 25, 2019

godmar commented Feb 28, 2019

ghost commented Mar 13, 2019

ianschmitz commented Mar 13, 2019

methodbox commented Mar 30, 2019 •

edited

Loading

marcinwasilewski commented Mar 31, 2019

ianschmitz commented Mar 31, 2019

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

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

Comments

yhoiseth commented Feb 16, 2019 • edited Loading

Is this a bug report?

Did you try recovering your dependencies?

Which terms did you search for in User Guide?

Environment

Steps to Reproduce

Expected Behavior

Actual Behavior

Reproducible Demo

bugzpodder commented Feb 16, 2019

sbcreates commented Feb 16, 2019 • edited Loading

bugzpodder commented Feb 17, 2019

sorahn commented Feb 17, 2019 • edited Loading

alexch commented Feb 17, 2019 • edited Loading

SimenB commented Feb 17, 2019

bugzpodder commented Feb 18, 2019

Tasemu commented Feb 18, 2019

SimenB commented Feb 18, 2019

aboutlo commented Feb 18, 2019

naugtur commented Feb 18, 2019

jdalegonzalez commented Feb 18, 2019

alexch commented Feb 18, 2019

bugzpodder commented Feb 18, 2019 • edited Loading

ztolley commented Feb 18, 2019

jdalegonzalez commented Feb 18, 2019 via email

dlipeles commented Feb 19, 2019 • edited Loading

emilymclean2 commented Feb 19, 2019 • edited Loading

bugzpodder commented Feb 19, 2019

jitenderchand1 commented Feb 19, 2019 • edited Loading

bugzpodder commented Feb 19, 2019 • edited Loading

jdalegonzalez commented Feb 19, 2019

sorahn commented Feb 19, 2019

jdalegonzalez commented Feb 19, 2019 via email

iansu commented Feb 19, 2019

SimenB commented Feb 19, 2019

Waseemrajashaik commented Feb 25, 2019

yhoiseth commented Feb 25, 2019

godmar commented Feb 28, 2019

ghost commented Mar 13, 2019

ianschmitz commented Mar 13, 2019

methodbox commented Mar 30, 2019 • edited Loading

marcinwasilewski commented Mar 31, 2019

ianschmitz commented Mar 31, 2019

yhoiseth commented Feb 16, 2019 •

edited

Loading

sbcreates commented Feb 16, 2019 •

edited

Loading

sorahn commented Feb 17, 2019 •

edited

Loading

alexch commented Feb 17, 2019 •

edited

Loading

bugzpodder commented Feb 18, 2019 •

edited

Loading

dlipeles commented Feb 19, 2019 •

edited

Loading

emilymclean2 commented Feb 19, 2019 •

edited

Loading

jitenderchand1 commented Feb 19, 2019 •

edited

Loading

bugzpodder commented Feb 19, 2019 •

edited

Loading

methodbox commented Mar 30, 2019 •

edited

Loading