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

update minimist to handle security vulnerability #114

Closed
wants to merge 1 commit into from

Conversation

elvece
Copy link

@elvece elvece commented Mar 18, 2020

Resolves issue 113

@elvece
Copy link
Author

elvece commented Mar 21, 2020

@dominictarr approved and ready to merge

@Naktibalda
Copy link

There is something weird going on with rc dependencies.
This change is not necessary because according to semantic versioning ^1.2.0 allows installation of any version greater or equal to 1.2.0 and lower than 2.0.

In my project I have [email protected] using [email protected] in one place and [email protected] in another:

$ npm ls minimist
@[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]
          └─┬ [email protected]
            └── [email protected]

A common pattern is that [email protected] is used when[email protected] is required by node-pre-gyp, but registry-auth-token allows [email protected] to use [email protected].

Running npm update minimist --depth=4 updates minimist in dependencies of rc, but then npm starts complaining about.

$ npm update minimist --depth=4
+ [email protected]
updated 1 package in 0.3s
+ [email protected]
updated 1 package in 0.515s
$ npm ls minimist
@[email protected] /
├─┬ [email protected]
│ └─┬ [email protected]
│   ├─┬ [email protected]
│   │ └── [email protected]
│   └─┬ [email protected]
│     └── UNMET DEPENDENCY [email protected]
├─┬ [email protected]
│ └─┬ [email protected]
│   ├─┬ [email protected]
│   │ └── [email protected]
│   └─┬ [email protected]
│     └── UNMET DEPENDENCY [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]

npm ERR! missing: [email protected], required by [email protected]
npm ERR! missing: [email protected], required by [email protected]

@dominictarr
Copy link
Owner

Ah yeah, npm could be more deterministic. I switched to yarn.
as rc will correctly resolve the latest, secure, version of minimist, I don't think it needs to change.
It must either be a) some other modules want old versions or b) npm is confused. or both!

@dominictarr dominictarr closed this Apr 3, 2020
@jayaddison
Copy link

For anyone else reading this: it looks like some of the difficulties identifying the source of the minimist audit reports are due to use of NPM bundledDependencies.

For example [email protected] bundles node-pre-gyp (and that bundle includes rc and therefore minimist).

[email protected] has this same configuration.

I think that means that the dependency that does the bundling will have to be updated in order for upstream projects to see a fix, since the downloaded copy of an existing, unfixed package containing bundles will continue to provide the dependencies regardless of semver rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants