Add a badge for known vulnerabilities

[aviadatsnyk]

After suggesting this in the wrong place (https://github.com/expressjs/response-time/pull/11), following @dougwilson 's suggestion to take this up here.

Disclaimer: I am a snyk employee. Disclaimer: I am a user of express.

Seeing all these badges on the top of the readme, I find I'd also like to know that the latest version is free of known vulnerabilities, which is what this badge is all about. (see https://snyk.io/docs/badges)

Evaluating the badge: 1. Does the badge inform the user of something important? I believe the answer is yes. Knowing if a package we use is safe - has no known vulnerabilities - is imo very important. It is also something maintainers often see as their responsibility and care greatly about.

2. Is the badge only updated with a direct response to a contributor action (i.e. pushing to the repo)? No. A vulnerability is often found long after a package was added as a dependency. This is orthogonal to contributor action.

3. If the badge changes on it's own, does the underlying service provide a minimum of 24 hours notice prior to altering the badge state such that the contributors can take action? If the vulnerability is found in one of the express packages, snyk takes great care to responsibly disclose it, with more than 24 hours to fix (see disclosure policy. If it's found in one of the dependencies - no prior notice is given before the badge changes, since we believe once the vulnerability is public - users should be informed. There are many ways to fix the vulnerabilities, either by upgrading to a non vulnerable version, or by applying a snyk patch.

[dougwilson]

So if there is no way the contributors can take action (or know they need to take action), I think that would disqualify ever adding the badge to the modules. The issue here is that we add the badge, and now every single day someone will need to go open the README of every single one of our modules just to see if the badge is telling potential users to stop using this module. That seems like a large burden on module authors to add this badge.

If it's found in one of the dependencies - no prior notice is given before the badge changes, since we believe once the vulnerability is public - users should be informed.

When this happens, does the state of the badge change from not vulnerable to vulnerable? If it does, then that would disqualify the badge on this point as well.

I'm also see there is nodesecurity.io which seems to have a different set of vuluns in their database. It is not clear to me that if adding a vulun badge how to even choose one service over the other, as it doesn't seem maintainable to add several vulnerability badges.

[dougwilson]

Additionally, I also question the true value the badge has to users. Typically the users will head to the module's repo for the first install, but once they install it, it seems unlikely they are going to come back to look at the badge over and over to determine if the module has any vulnerabilities (and I assume the badge would only represent the current version of this module, which the user may or may not have installed, so it can give them a false sense of security). The nodesecurity.io project provides the nsp command line tool that can be run against an entire project's dependency graph, including the specific versions the users are using, which seems like much more useful of a tool than a badge in a README, at least to me.

I see that Snyk has a command line tool as well, but it requires me to provide my personal information to Snyk just to use it... ?

[dougwilson]

I have been digging into the badges more today since I haven't heard anything and it seems there is a bug in their functionality: the badge shows the stats for the version most recently published to npm, even if that is not the version that is marked at "latest" in npm (i.e. the one users will get when they "npm install module").

For example, the badge for the express module is actually showing the status for the 5.0 alpha version, not for the latest 4.x version (which is what "npm install express" gives).

[guypod]

@dougwilson good thoughts, please allow me to comment on some (note I'm also from Snyk):

Regarding Badges First, regarding the badge, you can use the badge URL on any Snyk test report. For instance, you can test the latest version by visiting https://snyk.io/test/npm/express/latest, and can get a badge for it with the URL https://snyk.io/test/npm/express/latest/badge.svg. Similarly, you can choose to test a GitHub repo (or even a commit hash) using this test URL: https://snyk.io/test/github/expressjs/express, and create a badge for it using https://snyk.io/test/github/expressjs/express.svg.

All that said, we agree we should use the latest and not the most recent version in our default, and that change is being rolled out.

Regarding getting alerts Independent of posting a badge, you can - and should IMO - monitor this repository using Snyk, which will send you alerts on newly disclosed vulnerabilities that affect its dependencies. You can monitor your repos super easily through Snyk's Web UI, or add Snyk's CLI to your CI to take a snapshot the dependencies you want to monitor (docs).

The badge is meant to show the world you've considered the risk of vulnerable dependencies and are taking care of it, and that they should care too. Security is invisible all too often, I believe the badge is a good way to raise security awareness throughout.

Regarding disqualifying the badge if it turns red when you have issues - IMO this is the way it should work. The badge doesn't make your application vulnerable, and the "bad guys" are the ones making an effort to find out it became vulnerable first, while users are often unaware. By placing a badge on the repo, you're making a stronger commitment to your users to keep the repo free of vulnerable dependencies. If you lagged in doing so, you should want to make your users aware of it, not hide it.

Regarding which tool to use My (obviously biased) opinion is that Snyk is the tool to use, and I can guarantee Snyk's DB includes all the issues you'll find with nsp.

That said, as both Snyk and nsp are free for open source, for a project as important and widely used as express, I truly think you should use both tools. A vulnerable dependency in express is a truly big deal, and I believe it's well worth the effort to ensure you track for such vulnerabilities as tightly as you can.

I'm also happy to give Express free access to early notifications (which our Enterprise customers get), for this very reason.

[dougwilson]

Thanks for the information, @guypod !

All that said, we agree we should use the latest and not the most recent version in our default, and that change is being rolled out.

That is awesome to hear! Let me know when it's available :)

The badge is meant to show the world you've considered the risk of vulnerable dependencies and are taking care of it, and that they should care too. Security is invisible all too often, I believe the badge is a good way to raise security awareness throughout.

In my opinion, a badge will end up indicating the opposite of that. When there are no issue, cool, it is a badge that says everything is good, but then the maintainer goes to sleep, wakes up in the morning only to find out that the badge has been telling all visitors "hey, this module is vulnerable, don't use me". The badge does not give any kind of signal to the users that the maintainer doesn't know yet, perhaps is working to update the dependency (which is not always a trivial thing to do), etc.

But I think there is a middle ground here: a long time ago, we setup "badgeboards" for all our organizations (https://expressjs.github.io/badgeboard/ for expressjs). I think adding the Snyk badge on this page would make sense; a maintainer can easily gauge the state of all the organization's modules in one place. A blocker, though, is that the badge for Snyk needs to be on shields.io project, or at least allow the square style form of the badge currently.

[dougwilson]

Another point to consider is that we also use https://david-dm.org/ already, which provides vulnerability notices on the badges, so I'm not sure why two would be necessary here.

[dougwilson]

@guypod thanks for linking to https://snyk.io/docs/security#disclosure-policy . I'm a bit concerned using your service, because I don't remember anything coming through for the advisory https://snyk.io/vuln/npm:express:20150803 which contains some inaccurate information, like the versions affected. Can you provide any insight into how that advisory got into the database? With nodesecurity.io I have always worked closely with them on reports and there are no reports on our modules I didn't already know about, but that doesn't seem to be the case with the Snyk database, with the above report as an example that seems to have materialized without working with anyone from the project the report is against.

[guypod]

@dougwilson re badgeboards, sounds great. You know best how to communicate with your users, though I would point out that at the time the badge shows the vulnerability count, express indeed has a vulnerable dependency. Not having the badge doesn't change this fact, it just hides it. still, the badge definitely emphasises this reality, for good and bad.

Re David-dm badge, our experience has been that it often misses issues we do pick up. That said, I don't want to get into competition bashing here - I'd encourage you to make the call you see fit.

Re the vulnerability in question, we weren't the ones who found it, but rather extracted it from past release notes. I asked the right person on the team to dig in and review it again - can you email any inaccuracies you see to support@snyk.io? We absolutely follow a responsible disclosure process for issues we research, as was shown in recent issues we reported on qs, ms and various others. Do call us out if we fail to do that - it shouldn't happen!

[guypod]

One more thing: the fixed "latest" version is already deployed!

[dougwilson]

One more thing: the fixed "latest" version is already deployed!

Sweet, I thought it looked like that, but wasn't sure :)

[dougwilson]

though I would point out that at the time the badge shows the vulnerability count, express indeed has a vulnerable dependency. Not having the badge doesn't change this fact, it just hides it. still, the badge definitely emphasises this reality, for good and bad.

Sure, but that answer seems to gloss over all the stuff I said...

Re David-dm badge, our experience has been that it often misses issues we do pick up. That said, I don't want to get into competition bashing here - I'd encourage you to make the call you see fit.

I don't see why David can't also pull from Snyk, just like it pulls from Nodesecurity.io. Having Davis include Snyk issues would immediately improve the amount of people you're helping here, as many projects have Davis badges and improving the Davis badge would help the community better than trying to get every project add yet another badge to their README (and many projects, like ours, only use shields.io badges for uniformity, which Snyk doesn't (yet?) offer that as an option; both Travis and AppVeyor are both onboard the shields.io service).

Re the vulnerability in question, we weren't the ones who found it, but rather extracted it from past release notes. I asked the right person on the team to dig in and review it again - can you email any inaccuracies you see to support@snyk.io? We absolutely follow a responsible disclosure process for issues we research, as was shown in recent issues we reported on qs, ms and various others. Do call us out if we fail to do that - it shouldn't happen!

I'm not sure what release notes that is, because we post our all security notes right here on our website: http://expressjs.com/en/advanced/security-updates.html and that is not there, since it is not a security issue (your app won't even function if you encounter that bug). Just remove the inaccurate advisory.

[dougwilson]

(The issue closure was a "fat finger")

[guypod]

@dougwilson give me a day or so to look into what being a shields.io badge means, I assumed we already are a "standard" badge so will dig in.

I also asked the right people to investigate the issue you pointed out. I'm not convinced it's not an issue and want my security team to either confirm or come back here and explain why it is.

So stay tuned :)

[dougwilson]

I also asked the right people to investigate the issue you pointed out. I'm not convinced it's not an issue and want my security team to either confirm or come back here and explain why it is.

And I'm not necessarily arguing against that, but it seems like suddenly adding vulnerabilities to this database without even contacting the project (even after the fact), really does not install trust in your services, especially when security is a delicate subject to many.

... come back here and explain why it is

Please, do not do this if you are saying you care about security. I would hope you care about following project's stated security disclosure procedure (ours is https://github.com/expressjs/express/blob/master/Security.md). Just because it is for a previous version, does not mean users are not using it and does not mean you should not be going through the normal disclosure processes, especially since at the time that was added, we advertised any version 4.11.1 or higher as secure, so you effectively released a vulnerability notice on users and our project with no notice.

I hope you can demonstrate better practices, and perhaps through better practices, you can help instill the trust in your service that is missing right now from privacy issues like requiring sign ups just to use the CLI to check for vulnerabilities to not even following your own disclosure process.

[guypod]

@dougwilson a lot to unpack in your statements here...

We will dig deeper into the issue and correspond privately to confirm it's indeed an issue and why, or remove it from our DB if it's not.

I want to stress we treat discovered vulnerabilities very carefully. This was a case of what we call an "unsurfaced vulnerability", which means an issue that was already disclosed and fixed, and we are simply curating it. I can see how in certain cases, notably express, even those cases require the private disclosure process, and will adapt our practices.

Regarding requiring authentication for our tests, I've explained the reasons for it in our blog: https://snyk.io/blog/requiring-authentication/

It's trivial to setup an anonymous account, and the authentication allowed us to control the abusive usage we were getting (including from potentially bad actors), and to communicate with our users. As you well know, providing a service that lets open source projects freely and easily identify vulnerable dependencies, as well as staffing a security team to curate and research such issues, is neither cheap nor easy. While not infalliable, our privacy practices are of a very high standard, and will continue to be so.

[dougwilson]

which means an issue that was already disclosed

Can you explain how you came to that conclusion? We disclose our vulnerabilities publically at http://expressjs.com/en/advanced/security-updates.html and that is not a listed vulnerability. What is your process for this type of determination? How can I trust adding the badge to the repos when my past experience from Snyk has been so bad?

[guypod]

The vast majority of open source projects we track do not track their vulnerabilities in a single location. Most don't even include detailed release notes that make it easy to spot such vulnerabilities. We try to make it easier for consumers of these open source packages to find out about vulnerabilities, and so we look for vulnerabilities discussed or logged and add them to the DB so you can find out if you're using a package that has such a stated vulnerability.

We treat such vulnerabilities differently than brand new ones, as they've already been acknowledged, and users already have a fix they can apply. This also includes bugs that have been fixed, but haven't been recognised as a vulnerability. I agree that in those cases we should refine our process and include better notification to the owners. This is especially true for projects like express, which document their security issues diligently.

Regarding the badge, while I would be happy to have you add it, to raise awareness to this concern further, if you're uncomfortable you could also just use Snyk for a while, get a feel for our accuracy and cadence, and add the badge if/when you're sufficiently comfortable. Does that sound reasonable?

[dougwilson]

I'm certainly willing to work together to get a solution, and from my experience with Snyk I tried to highlight above, I really won't want to (we have also rejected adding the badge to express quite a while back last summer).

I really do not want to end up with a million badges at the top of the README, either. Ideally Snyk would just be added as a source for Davis and thus we can add just a single badge that provides the complete state of the security and up-to-date-ness of the project instead of multiple badges.