Composer Audit: ability to ignore vulnerabilities

[Seldaek]

Discussed in #11294

^{Originally posted by ivancli February 4, 2023}
Hi guys, we use composer audit in a CI pipeline of our project to check vulnerabilities. However once a vulnerability is discovered, the pipeline fails and blocks the subsequent processes, e.g. deployment.

Just wondering if composer has a built-in functionality to ignore a list of vulnerabilities by CVE IDs.

If not, will this be considered to be implemented in the near future?

You can find similar functionality in Aqua Trivy which ignores vulnerabilities specified in the .trivyignore file.

---

I agree it probably makes sense to have an option to dismiss some vulns by CVE id or something.

[mxr576]

Maybe instead of an opt-out feature - which sounds pretty bad if we consider that we are talking about known security vulnerabilities - the community would need something like this. I'd use audit and audit-changes in different scenarios. audit as-is today better fits for daily checks than running on CR/MR/PRs.

Feedbacks are welcomed, I've just built this quickly.

https://packagist.org/packages/mxr576/composer-audit-changes

[Seldaek]

Yeah I mean generally speaking I don't think it should block your pipeline if you have vulnerabilities. It should be a reported failure but not a hard fail blocking deployment, that's way too disturbing to the daily business and might cause you bigger troubles.

But that's up to people and how they integrate things.. So if you do make it fail hard I can see that you may want to be able to ignore some specific vulnerabilities if you have had a look and determined you are not affected.

[Seldaek]

See #11556 if anyone wants to review

[mxr576]

Hm, #11556 has been merged sooner as I expected. I am sorry that I did not have time earlier to share my 2 cents. The PR also did not get feedback from the community.

Nonetheless, please allow me to share why I would not have added #11556 as solution to Composer. (2.6.0 is not out yet, maybe there is a chance for a rollback?)

I strongly believe that security advisories is not something that anybody would like to ignore. However, definitely there are real life scenarios where the combination of:

• a newly published CVE and
• the currently available tooling (composer audit) and
• the way how developers and DEVOPS ppl integrated it to their CI pipelines and automated QA process
  cause troubles and pain.

I think we need to understand the root of the problem and react to that.

My understand is that composer audit is perfect as it is and it does not require this kind of further adjustments. Simply, it MUST NOT be used on CR/MR/PR reviews or executed before (production) deployments in a way that they can block merges or deployments. (@Seldaek, I believe you sad the same thing in #11298#issuecomment-1560001843.
My opinion and proposal that on CR/MR/PR reviews, or before (prod) deployments, only the modified dependencies should be audited because what we actually would like to prevent is adding/upgrading/downgrading to insecure package versions. When a CR/MR/PR introduces a dependency with known security issues, it MUST BE still blocked by the CI.
By auditing only changed dependencies in these scenarios we can also avoid unpleasant surprises caused by a recently published CVE for an existing dependency right before a CR/MR/PR merge or a deployment.

The solution introduced in #11556 is problematic because:

• the failure is still going to happen, somebody need to add a given CVE to the ignore list. The list is context independent, a given CVE gets ignored in multiple stages of the SDLC. Question: How somebody specify that in a given context (e.g. QA environment or nightly scan) the ignore list should be ignored?
• after a CVE was added to the ignore list there is a new problem that needs to be handled, getting rid of the technical debt (a security related one). Who is going to remember that the CVE has to be removed from the ignore list and the package needs to be updated?

The composer audit-changes command that I have promoted in my previous comment already proves that it is possible to implement the above described tool. The only remaining part is educating developers/DEVOPS ppl to the right tool (Composer command) for the right job.

[cafferata]

For what it's worth, I share @mxr576 opinion. I've seen the GitHub pull request come by and thought I'd type out my concerns after my vacation (instead of from a mobile phone like now). 🤞

[Seldaek]

I am of a different opinion on two points:

• if I get a new vulnerability warning I'll usually look at the CVE and either upgrade or decide that we are not affected and then the upgrade isn't urgent and adding to the ignore list would make sense to me as it's been looked at.
• when a new vulnerability occurs for existing dependencies I want to know about it, I don't want alerts just for new deps with vulnerabilities. That said, I would also not make the whole build fail based on that. I'd have a separate non-essential build for auditing. That matches more what we get with packagist.com security alerts where i want to be alerted but not block deployment or anything.

So I think different people have different preferences in terms of workflow, and offering the option to ignore is sensible. Maybe not for you tho but that's ok you don't have to use everything.

[NicoHaase]

I strongly believe that security advisories is not something that anybody would like to ignore.

It depends. We use composer audit in our pipelines and let them fail if any CVE occurs. But for some projects, we would like to ignore some vulnerabilities, for example if we know that it currently cannot be triggered in the application

If you don’t agree with the current implementation, you're safe not to use it in any way. Unless you change your configuration, the behavior does not change

[mxr576]

Surely there are opinions and opinionated workflows. I can be also convinced that #11556 is the right direction if it follows industry standards and there are precedents for this kind of features in other package managers (auditing tools) - I dd not do my homework.

Maybe my expectations are also misaligned... What I would expected from Composer (as the de-facto package manager for PHP) is strong guidance on best practices, incl. security ones. With this ignore list feature, the intentions were definitely good, but you can be also sure that it is going to be abused several times. This is what I am worried about the most. Not about those ppl who know what they are doing and aware of consequences, but about those who do not.

By the way, the described workflow by @Seldaek is not different from what I would also do...

My gut says whoever needed a feature like this, could already achieved by running composer audit --format=json and filtered the result with jq. In this scenario the complexity, the risk and the technical debt remained on the "consumer" end - and whoever were capable of setting up workflow like this, were surely one of those ppl who know what they are doing...

So maybe a simple documentation about filtering Composer audit results would have been enough for a start and see the reactions. Because "no is temporary, yes is forever." , so if #11556 gets released in 2.6.0 there is no way to get rid of it. However, if an alternative approach gets suggested and it fails, #11556 can still be added and everybody will be happy.

[janmashat]

I have to agree with @mxr576 that this seems like a bad idea - especially from a compliance perspective.

Rather than an "audit.ignored config setting to ignore security advisories" perhaps we could agree on this compromise: "audit.pass config setting to exit(0) for certain security advisories" (but still list them in the console output).

That way, if somebody is reviewing the "audit log" then they won't have to wonder whether some entries are missing.

[mxr576]

Really good idea! 👍

And see how easy was changing the current implementation to match the expected behavior... #11605

🤞