Security vulnerability in dependency tree

[notquitedilbert]

npm audit reports finding 2 vulnerabilities

Although the security warning relates to Lodash, it actuall the Vorpal package causing the problem - it hasn't been updated for months, and appears to be dead

Expected Behavior

There should be no vulnerabilities.

Current Behavior

npm reports

found 2 vulnerabilities (1 low, 1 moderate) in 115 scanned packages
2 vulnerabilities require manual review. See the full report for details.

Affected packages

• cli
• core
• prompt
• config-angular

Possible Solution

Steps to Reproduce (for bugs)

1. Create a new project (or use an existing one)
2. npm install --save-dev @commitlint/prompt
3. npm audit

output ->

                       === npm audit security report ===

┌──────────────────────────────────────────────────────────────────────────────┐
│                                Manual Review                                 │
│            Some vulnerabilities require your attention to resolve            │
│                                                                              │
│         Visit https://go.npm.me/audit-guide for additional guidance          │
└──────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ lodash                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=4.17.11                                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @commitlint/prompt [dev]                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @commitlint/prompt > vorpal > inquirer > lodash              │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/782                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ lodash                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=4.17.5                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @commitlint/prompt [dev]                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @commitlint/prompt > vorpal > inquirer > lodash              │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/577                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
found 2 vulnerabilities (1 low, 1 moderate) in 115 scanned packages
  2 vulnerabilities require manual review. See the full report for details.

Context

Your Environment

Executable	Version
commitlint/prompt --version	7.5.0
git --version	2.17.2
node --version	11.2.0

[byCedric]

@marionebl @escapedcat I think we need to work on #402 to fix this one, Vorpal hasn't been updated in over 2 years. @notquitedilbert thanks for reporting!

[escapedcat]

There's an effort to update vorpal but the critical deps are also not updated so far.
@byCedric @marionebl you know an alternative to vorpal which could be used?

[marionebl]

Let's do this:

1. Address prompt: reimplement with ink #86 and rewrite @commitlint/prompt-cli to use ink instead of vorpal.
2. Address @commitlint/prompt does not use commitizen inquirer #402 and make @commitlint/prompt a "true" commitizen adapter, changing the UX to commitizen's defaults

What do you think?

[hugomn]

Any updates on this? Is @commitlint/prompt-cli being migrated to ink?

[escapedcat]

@hugomn I think our latest thoughts were to discontinue prompt-cli and replace it with a (native?) commitizen adapter (did I explain this correct? @marionebl @byCedric).

We actually wanted to open an issue here and ask for feedback of others. What do you think?

[hugomn]

@escapedcat I'm not sure if I got exactly what "replace with a native commitizen adapter" means. But anyway, I think it's a good idea to open a new issue to discuss this migration. These 2 security vulnerabilities are hanging here for a while.

[byCedric]

Right now the @commitlint/prompt is implemented with a custom "inquirer" (the library that asks the questions). It's partly using Commitizen, and part custom. Exactly this custom part is the "troublemaker" here. I think it was built like this because @commitlint/prompt-cli was created before Commitizen was done, but I might be wrong here.

Now we want to rewrite the @commitlint/prompt in such a way that it fully uses Commitizen with your current commitlint config, no custom part, no audit warnings, and no extra config. Unfortunately, this is set for "phase 3" of the TypeScript port. This port is important for us to make it less maintenance intensive, stable and way cleaner to work with. Once we have that in place, I'll make the extra effort to replace that part and fix this issue.

Please be aware that the @commitlint/prompt-cli will be deprecated in the future to solely use the @commitlint/prompt adapter with Commitizen. We will open a ticket about this soon, further explaining why this will go.

TL;DR; We are working on a full TypeScript port of Commitlint, once that's done this part will be replaced and fixed! 😄

@escapedcat @marionebl This is the proper longer explanation right?

[superliuwr]

Hi guys, I understand porting to TS has a higher priority but this issue stops us from using commitlint in our products. As mentioned at the vorpal issue, is it possible to switch to use the fork before we have a proper rewrite?

[asciidisco]

As this has became a bit stale, I would like to ask if you were to welcome a PR that follows up on @superliuwr comment above?

[escapedcat]

Hey @asciidisco , sure, happy if you want to give it a try.

[robaxelsen]

Any update on this? @byCedric do you know if there is an ETA on when #659 will be completed?

[cmalard]

The message became a bit scarier 🙈🍻

[escapedcat]

@cmalard not completely sure, but as long as you don't deploy the code to an attackable server it's not really critical, is it?

Apart from that we're thinking about deprecating prompt/prompt-cli because of its problematic status/maintenance and replacing it with this:
#2547

@AdeAttwood what do you think?

[cmalard]

@cmalard not completely sure, but as long as you don't deploy the code to an attackable server it's not really critical, is it?

From a human point of view, I agree 😉
When my CI tolds me there is a high severity on one of the newly added dependencies and then refuses to build, it becomes more problematic 🙈 🍻

[AdeAttwood]

From my playing with #2547 works nice. I haven't used the prompt but there seems to be quite a few problems with it. I think we get #2547 into master, then we can start to work on the docs and iron out any issues before releasing it to the latest channel.

[fbritoferreira]

Do you guys know when this can be fixed?

[escapedcat]

Hey @itsfilipoficial ,
armano started a PR to switch vorpal. We recently updated the branch: #2697
Happy if you want to take a look at the PR/branch and see if this works for you.

[fbritoferreira]

Hey @itsfilipoficial , armano started a PR to switch vorpal. We recently updated the branch: #2697 Happy if you want to take a look at the PR/branch and see if this works for you.

That pr branch seems to work for me when I tested locally

[escapedcat]

@itsfilipoficial 🎉 ! Thanks for the quick feedback!

My idea would be to merge this and a create next-release. Then you could test that again.
Would try to create this during the weekend.

[escapedcat]

@itsfilipoficial done, please double check if you have time ;)

[escapedcat]

Reopen till released

[escapedcat]

Should be released, please check