Peer Dependency has Security Vulnerability

[ferencbeutel4711]

Hello there,

since today, a new issue in minimist < 1.2.3 is found:
https://www.npmjs.com/advisories/1179

This lib is using optimist 0.6.1, which requires minimist in 0.0.10, which is obviously affected.
Will there be a new version with a fix for this? Our Security-Scanner is running wild because of this.

Thanks & Best Regards

[timbru31]

Possible duplicate of #1658 (at least optimist is deprecated and probably won't bump minimist)

[jfoclpf]

same here, please update asap, Thanks a lot

 === npm audit security report ===                        
                                                                                
┌──────────────────────────────────────────────────────────────────────────────┐
│                                Manual Review                                 │
│            Some vulnerabilities require your attention to resolve            │
│                                                                              │
│         Visit https://go.npm.me/audit-guide for additional guidance          │
└──────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ minimist                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.2.1 <1.0.0 || >=1.2.3                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ express-handlebars                                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ express-handlebars > handlebars > optimist > minimist        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1179                            │
└───────────────┴──────────────────────────────────────────────────────────────┘


$npm list handlebars express-handlebars optimist minimist
[email protected] /home/joao/delp.pt
├─┬ [email protected] 
│ └─┬ [email protected] 
│   └─┬ [email protected] 
│     └── [email protected]

[karlhorky]

If you use Yarn, here's a workaround for now until this is fixed (maybe with #1662):

Add the following resolution to your package.json and run yarn.

  "resolutions": {
    "**/optimist/minimist": "0.2.1"
  }

This will force all versions of optimist to use [email protected], regardless of which package is depending on optimist.

[yudai-nkt]

Edit This approach may break your app depending on the dependency situation. Please read #1661 (comment) and be cautious!

If you use npm, there is an npm package npm-force-resolutions that emulates yarn's dependency resolution. You can find the usage at yudai-nkt/sesame-client@81d0664#diff-b9cfc7f2cdf78a7f4b91a753d10865a2, and then run npm install.

Also make sure that your test suites will pass because dependency resolution doesn't honor semver compatibility.

[karlhorky]

@yudai-nkt unfortunately, because npm-force-resolutions does not have all features of Yarn Resolutions, this can cause problems if you specify a version of minimist that is incompatible with the one that the dependency is requiring.

This is true in the case that you posted above. Review your lockfile entry for optimist:

1. You specify [email protected], which depends on minimist@~0.0.1 (or at least something that fulfills the same API / contract)
2. You override this with [email protected], which means your bumping a major (breaking changes)
3. This means that any code in optimist that uses minimist in a way that works for 0.0.8 but does not work for 1.0.0 would break, and your application could break
4. This also applies to any other dependencies that you have updated past a major version bump

As far I know, this shortcoming is also true for npm itself too (without manual editing of the lockfile) - be careful with what you are forcing npm to update to! There is no way to easily achieve what Yarn is capable of.

---

In case you absolutely cannot use Yarn, what you may be able to do, which would be more safe would be:

1. Add a resolution for [email protected]
2. Run npm-force-resolutions. This will cause any packages depending on minimist@>=1 to possibly break.
3. Copy the resulting object from the package-lock.json file
4. Remove the resolution again
5. Manually edit your package-lock.json to force this dependency just on the compatible versions

But... huge shortcoming - this edit could be destroyed by future npm commands.

Without proper resolutions support, it looks like npm is not up to the task here.

[yudai-nkt]

@karlhorky I'm aware of the potential breakage, and I went this direction for the following reasons:

• optimist has been deprecated for 6 years, so I suppose what works with minimist@1 as of now will be likely to work in a future update (or it will be fixed or won't be my package's dependency anymore)
• eslint will drop dependency on minimist in the next major release
• minimist is not a production dependency, so it won't affect package users

That being said, I obviously didn't explain enough and it can cause future readers' misunderstanding. I appreciate your thorough supplement!

[createthis]

The long term solution is to use yargs instead of optimist, right? Do we have an ETA on that?

[jfoclpf]

@yudai-nkt would it perhaps work if in npm-force-resolutions we just allow forcing minor updates, which are by default backwards compatible? The vulnerability is solved in minimist 0.2.1

[jfoclpf]

@karlhorky what do you think, forcing minimist to the lower version that solves the vulnerability, the 0.2.1, which is just minor and hence backwards compatible?

[karlhorky]

@jfoclpf unfortunately, there are other dependencies on minimist above v1 (examples below)

• https://github.com/yudai-nkt/sesame-client/blob/81d0664c16aa5a371f1c91dccd8e4f7b9eab5601/package-lock.json#L3688
• https://github.com/yudai-nkt/sesame-client/blob/81d0664c16aa5a371f1c91dccd8e4f7b9eab5601/package-lock.json#L302

So if you force 0.2.1, then these dependents may break, if they are using features that are only in versions above 1...

[jfoclpf]

@karlhorky ok, that would depend on each package we have.

On what handlebars is concerned, I just see these 3 dependencies

"dependencies": {
  "neo-async": "^2.6.0",
  "source-map": "^0.6.1",
  "yargs": "^3.32.0"
},

Is there on this dependency tree any case wherein minimist > 1 is needed?

[hfiguiere]

@karlhorky ok, that would depend on each package we have.

On what handlebars is concerned, I just see these 3 dependencies

"dependencies": {
  "neo-async": "^2.6.0",
  "source-map": "^0.6.1",
  "yargs": "^3.32.0"
},

Is there on this dependency tree any case wherein minimist > 1 is needed?

People use 4.7.3. You are looking at master

[yudai-nkt]

ok, that would depend on each package we have.

@jfoclpf if you want to check how your specific application/library depends on minimist or whatever, use npm ls and you will see something like this:

$ npm ls minimist                                                                                                                               typescriptify
[email protected] /Users/ynkt/src/github.com/yudai-nkt/deckgl-etude
├─┬ @babel/[email protected]
│ └─┬ [email protected]
│   └── [email protected]
├─┬ @danmarshall/[email protected]
│ └─┬ [email protected]
│   └── [email protected]
├─┬ [email protected]
│ ├─┬ [email protected]
│ │ └─┬ [email protected]
│ │   └── [email protected]
│ └─┬ [email protected]
│   └── [email protected]
├─┬ [email protected]
│ └─┬ [email protected]
│   ├─┬ @mapbox/[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]

[AlAyoub]

Is it fair to assume that the master branch is the most current working branch? If so, is there a version bump coming soon?

[nknapp]

4.x is tbe current working branch... I have had another look. The change itself does not seem to be a big deal, but I really need to be sure that it does not break anything in the CLI. I am not in a situation where I can quickly revert anything.

So either there need to be some tests in #1662 for the CLI. Have a look at the tasks folder in the tests-bin.js file. There is one test already but the file certainly needs to be refactored a bit in order to cleanly contain multiple tests.

[fabb]

I started adding some tests in #1666, but since I'm not using handlebars, I'd be happy to be guided on which additional tests to add.

[ErisDS]

Closed by #1666 and released in 4.7.4.