pip_audit: dedupe vulnerability reports by alias

[woodruffw]

Only implemented for PyPI so far. OSV probably needs similar changes.

Closes #231.

[di]

I was kind of imagining that this would either happen here or here in a way that isn't specific to the service, in a way that would take a list of VulnerabilityResults and condense ones with a shared alias into a MultiVulnerabilityResult (or some other name) that contains multiple VulnerabilityResults.

Additionally, this would make it easier to provide an option to display all the correlated vulnerability IDs when outputting these, e.g.

$ pip-audit --show-dupes
Found 2 known vulnerabilities in 1 package
Name  Version ID                              Fix Versions
----  ------- ------------------------------- ------------
Flask 0.5     PYSEC-2019-179, FOOBAR-2019-666 1.0
Flask 0.5     PYSEC-2018-66                   0.12.3

[woodruffw]

Yeah, that’s a great point — now that we’re tracking the aliases on the results themselves we can move this out of the individual service level. Sent from mobile. Please excuse my brevity.

…

On Feb 4, 2022, at 5:38 PM, Dustin Ingram ***@***.***> wrote:  I was kind of imagining that this would either happen here or here in a way that isn't specific to the service, in a way that would take a list of VulnerabilityResults and condense ones with a shared alias into a MultiVulnerabilityResult (or some other name) that contains multiple VulnerabilityResults. Additionally, this would make it easier to provide an option to display all the correlated vulnerability IDs when outputting these, e.g. $ pip-audit --show-dupes Found 2 known vulnerabilities in 1 package Name Version ID Fix Versions ---- ------- ------------------------------- ------------ Flask 0.5 PYSEC-2019-179, FOOBAR-2019-666 1.0 Flask 0.5 PYSEC-2018-66 0.12.3 — Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you authored the thread.

[tetsuo-cpp]

I'd also like the deduplication logic to be moved out of the vuln service if possible. But other than that, it looks good.

[woodruffw]

Should be good for another review. We now consider each vulnerability's id when evaluating aliases, per #231 (comment)

One open question: should we collect all aliases for a vulnerability and update the "canonical" vulnerability with them? For example, the following result list:

        [
            VulnerabilityResult(
                id="PYSEC-0",
                description="fake",
                fix_versions=[Version("1.1.0")],
                aliases={"CVE-XXXX-YYYYY"},
            ),
            VulnerabilityResult(
                id="FAKE-1",
                description="fake",
                fix_versions=[Version("1.1.0")],
                aliases={"CVE-XXXX-YYYYY"},
            ),
            VulnerabilityResult(
                id="CVE-XXXX-YYYYY",
                description="fake",
                fix_versions=[Version("1.1.0")],
                aliases={"FAKE-1"},
            ),
        ]

results in one vulnerability with an id of PYSEC-0 and an aliases set of just {"CVE-XXXX-YYYYY"}. Should it include FAKE-1 as well?

[di]

Yes, I think so. Let's assume the alias-id relationship to be bi-directional even if we don't see both directions.

[woodruffw]

Sounds good, I'll adjust for that in a moment.

[di]

Thanks, this looks a lot cleaner than the first pass. I think this LGTM once the comment regarding aliases/ids is resolved.

[woodruffw]

Added alias merging and a corresponding test; let me know what you think.