Skip to content

fix: deduplicate vulnerabilities in recommend endpoint #2160

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.

Sign up for GitHub

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

Merged
Strum355 merged 1 commit into from
Dec 4, 2025
Merged

fix: deduplicate vulnerabilities in recommend endpoint #2160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions etc/test-data/osv/RUSTSEC-2021-0079-DUPLICATE.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"id": "DUPLICATE",
"modified": "2021-10-19T22:14:35Z",
"published": "2021-07-07T12:00:00Z",
"aliases": [
"CVE-2021-32714"
],
"affected": [
{
"package": {
"ecosystem": "crates.io",
"name": "hyper",
"purl": "pkg:cargo/hyper"
},
"ranges": [
{
"type": "SEMVER",
"events": [
{
"introduced": "0.0.0-0"
},
{
"fixed": "0.14.10"
}
]
}
]
}
]
}
105 changes: 105 additions & 0 deletions modules/fundamental/src/purl/endpoints/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,3 +379,108 @@ async fn get_recommendations_no_version(ctx: &TrustifyContext) -> Result<(), any

Ok(())
}

#[test_context(TrustifyContext)]
#[test(actix_web::test)]
async fn get_recommendations_dedup(ctx: &TrustifyContext) -> Result<(), anyhow::Error> {
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Add a complementary test to ensure different vulnerabilities are not merged by the deduplication logic.

You already cover multiple advisories for the same vulnerability ID. Please also add a test where two advisories reference different vulnerability IDs for the same PURL and assert that both are returned. This will help catch any future over-deduplication that might merge distinct vulnerabilities.

Suggested implementation:

#[test_context(TrustifyContext)]
#[test(actix_web::test)]
async fn get_recommendations_dedup(ctx: &TrustifyContext) -> Result<(), anyhow::Error> {
    ctx.ingestor
        .graph()
        .ingest_qualified_package(
            &Purl::from_str("pkg:cargo/hyper@0.14.1-redhat-00001")?,
            &ctx.db,
        )
        .await?;

    // Two advisories that refer to the same vulnerability ID for the same package.
    ctx.ingest_documents([
        "osv/RUSTSEC-2021-0079.json",
        "osv/RUSTSEC-2021-0079-DUPLICATE.json",
    ])
    .await?;

    let app = caller(ctx).await?;
    let body: Value = app
        .call_and_read_body_json(
            TestRequest::post()
                .uri("/api/v1/purl/recommendations")
                .set_json(&json!({
                    "packages": [{
                        "purl": "pkg:cargo/hyper@0.14.1-redhat-00001"
                    }]
                })),
        )
        .await;

    // Ensure that deduplication merges multiple advisories for the same vulnerability.
    let packages = body["packages"].as_array().expect("packages should be an array");
    assert_eq!(packages.len(), 1, "expected a single package in the response");

    let vulns = packages[0]["vulnerabilities"]
        .as_array()
        .expect("vulnerabilities should be an array");
    assert_eq!(
        vulns.len(),
        1,
        "expected duplicate advisories for the same vulnerability to be deduplicated"
    );

    Ok(())
}

#[test_context(TrustifyContext)]
#[test(actix_web::test)]
async fn get_recommendations_no_overdedup(ctx: &TrustifyContext) -> Result<(), anyhow::Error> {
    // Same package as in the deduplication test.
    ctx.ingestor
        .graph()
        .ingest_qualified_package(
            &Purl::from_str("pkg:cargo/hyper@0.14.1-redhat-00001")?,
            &ctx.db,
        )
        .await?;

    // Two advisories that reference the same PURL but represent *different* vulnerability IDs.
    ctx.ingest_documents([
        "osv/RUSTSEC-2021-0079.json",
        "osv/RUSTSEC-2021-0080.json",
    ])
    .await?;

    let app = caller(ctx).await?;
    let body: Value = app
        .call_and_read_body_json(
            TestRequest::post()
                .uri("/api/v1/purl/recommendations")
                .set_json(&json!({
                    "packages": [{
                        "purl": "pkg:cargo/hyper@0.14.1-redhat-00001"
                    }]
                })),
        )
        .await;

    // Ensure that the two distinct vulnerabilities are both returned and not merged.
    let packages = body["packages"].as_array().expect("packages should be an array");
    assert_eq!(packages.len(), 1, "expected a single package in the response");

    let vulns = packages[0]["vulnerabilities"]
        .as_array()
        .expect("vulnerabilities should be an array");
    assert_eq!(
        vulns.len(),
        2,
        "expected two distinct vulnerabilities for the same PURL"
    );

    let vuln_ids: std::collections::HashSet<_> = vulns
        .iter()
        .filter_map(|v| v["id"].as_str().map(|s| s.to_string()))
        .collect();
    assert_eq!(
        vuln_ids.len(),
        2,
        "expected two distinct vulnerability IDs"
    );

    Ok(())
}
  • Ensure serde_json::json is imported at the top of the file (e.g. use serde_json::{json, Value};) if it is not already.
  • Adjust the endpoint path ("/api/v1/purl/recommendations"), request shape, and response field paths ("packages", "vulnerabilities", "id") to match the actual API if they differ in your codebase.
  • Replace "osv/RUSTSEC-2021-0080.json" with an advisory fixture that:
    1. Affects the same PURL (pkg:cargo/hyper@0.14.1-redhat-00001), and
    2. Has a different vulnerability identifier than RUSTSEC-2021-0079,
      so that the new test truly verifies that different vulnerabilities are not merged by the deduplication logic.

ctx.ingestor
.graph()
.ingest_qualified_package(
&Purl::from_str("pkg:cargo/hyper@0.14.1-redhat-00001")?,
&ctx.db,
)
.await?;

ctx.ingest_documents([
"osv/RUSTSEC-2021-0079.json",
"osv/RUSTSEC-2021-0079-DUPLICATE.json",
])
.await?;

let app = caller(ctx).await?;
let recommendations: Value = app
.call_and_read_body_json(
TestRequest::post()
.uri("/api/v2/purl/recommend")
.set_json(json!({"purls": ["pkg:cargo/hyper@0.14.1"]}))
.to_request(),
)
.await;

log::info!("{recommendations:#?}");

let entry =
&recommendations["recommendations"].as_object().unwrap()["pkg:cargo/hyper@0.14.1"][0];
assert_eq!(entry["vulnerabilities"].as_array().unwrap().len(), 1);
assert_eq!(
entry["vulnerabilities"].as_array().unwrap()[0]["id"]
.as_str()
.unwrap(),
"CVE-2021-32714"
);

Ok(())
}

#[test_context(TrustifyContext)]
#[test(actix_web::test)]
async fn get_recommendations_other_status(ctx: &TrustifyContext) -> Result<(), anyhow::Error> {
use sea_orm::{ActiveModelTrait, ColumnTrait, EntityTrait, QueryFilter, Set};
use trustify_entity::{purl_status, status};

ctx.ingestor
.graph()
.ingest_qualified_package(
&Purl::from_str("pkg:cargo/hyper@0.14.1-redhat-00001")?,
&ctx.db,
)
.await?;

ctx.ingest_documents(["osv/RUSTSEC-2021-0079.json"]).await?;

let custom_status_id = Uuid::new_v4();
let custom_status = status::ActiveModel {
id: Set(custom_status_id),
slug: Set("custom_status".to_string()),
name: Set("Custom Status".to_string()),
description: Set(Some("A custom status for testing".to_string())),
};
status::Entity::insert(custom_status).exec(&ctx.db).await?;

let purl_statuses = purl_status::Entity::find()
.filter(purl_status::Column::VulnerabilityId.eq("CVE-2021-32714"))
.all(&ctx.db)
.await?;

assert!(!purl_statuses.is_empty());

for ps in purl_statuses {
let mut active: purl_status::ActiveModel = ps.into();
active.status_id = Set(custom_status_id);
active.update(&ctx.db).await?;
}

let app = caller(ctx).await?;
let recommendations: Value = app
.call_and_read_body_json(
TestRequest::post()
.uri("/api/v2/purl/recommend")
.set_json(json!({"purls": ["pkg:cargo/hyper@0.14.1"]}))
.to_request(),
)
.await;

log::info!("{recommendations:#?}");

let entry =
&recommendations["recommendations"].as_object().unwrap()["pkg:cargo/hyper@0.14.1"][0];
let vulns = entry["vulnerabilities"].as_array().unwrap();
let vuln = vulns
.iter()
.find(|v| v["id"].as_str().unwrap() == "CVE-2021-32714")
.unwrap();

assert_eq!(vuln["status"], "custom_status");

Ok(())
}
1 change: 1 addition & 0 deletions modules/fundamental/src/purl/model/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ pub enum VexStatus {
NotAffected,
UnderInvestigation,
Recommended,
#[serde(untagged)]
Copy link
Copy Markdown
Contributor

@dejanb dejanb Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in general. I would also note that serde untagged is both not documented nor tested behaviour. Do we have an example of "Other" status, that we can test with?

Copy link
Copy Markdown
Member Author

@Strum355 Strum355 Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a test but it seems I forgot to git add the change. QE had examples of it during testing but I wasnt able to reproduce it locally so I had to do a bit of a workaround in the unit test by manually adding it to postgres

#[test_context(TrustifyContext)]
#[test(actix_web::test)]
async fn get_recommendations_other_status(ctx: &TrustifyContext) -> Result<(), anyhow::Error> {
    use sea_orm::{ActiveModelTrait, ColumnTrait, EntityTrait, QueryFilter, Set};
    use trustify_entity::{purl_status, status};

    ctx.ingestor
        .graph()
        .ingest_qualified_package(
            &Purl::from_str("pkg:cargo/hyper@0.14.1-redhat-00001")?,
            &ctx.db,
        )
        .await?;

    ctx.ingest_documents(["osv/RUSTSEC-2021-0079.json"]).await?;

    let custom_status_id = Uuid::new_v4();
    let custom_status = status::ActiveModel {
        id: Set(custom_status_id),
        slug: Set("custom_status".to_string()),
        name: Set("Custom Status".to_string()),
        description: Set(Some("A custom status for testing".to_string())),
    };
    status::Entity::insert(custom_status).exec(&ctx.db).await?;

    let purl_statuses = purl_status::Entity::find()
        .filter(purl_status::Column::VulnerabilityId.eq("CVE-2021-32714"))
        .all(&ctx.db)
        .await?;

    assert!(!purl_statuses.is_empty());

    for ps in purl_statuses {
        let mut active: purl_status::ActiveModel = ps.into();
        active.status_id = Set(custom_status_id);
        active.update(&ctx.db).await?;
    }

    let app = caller(ctx).await?;
    let recommendations: Value = app
        .call_and_read_body_json(
            TestRequest::post()
                .uri("/api/v2/purl/recommend")
                .set_json(json!({"purls": ["pkg:cargo/hyper@0.14.1"]}))
                .to_request(),
        )
        .await;

    log::info!("{recommendations:#?}");

    let entry =
        &recommendations["recommendations"].as_object().unwrap()["pkg:cargo/hyper@0.14.1"][0];
    let vulns = entry["vulnerabilities"].as_array().unwrap();
    let vuln = vulns
        .iter()
        .find(|v| v["id"].as_str().unwrap() == "CVE-2021-32714")
        .unwrap();

    assert_eq!(vuln["status"], "custom_status");

    Ok(())
}

without the variant being untagged, this test yields:

2025-12-03T11:30:22.239191Z  INFO trustify_module_fundamental::purl::endpoints::test: Object {
    "recommendations": Object {
        "pkg:cargo/hyper@0.14.1": Array [
            Object {
                "package": String("pkg:cargo/hyper@0.14.1-redhat-00001"),
                "vulnerabilities": Array [
                    Object {
                        "id": String("CVE-2021-32714"),
                        "status": Object {
                            "Other": String("custom_status"),
                        },
                    },
                ],
            },
        ],
    },
}

thread 'purl::endpoints::test::get_recommendations_other_status' (599692) panicked at modules/fundamental/src/purl/endpoints/test.rs:483:5:
assertion `left == right` failed
  left: Object {"Other": String("custom_status")}
 right: "custom_status"

Other(String),
}

Expand Down
13 changes: 7 additions & 6 deletions modules/fundamental/src/purl/service/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::{
summary::{base_purl::BasePurlSummary, purl::PurlSummary, r#type::TypeSummary},
},
};
use itertools::Itertools;
use regex::Regex;
use sea_orm::{
ColumnTrait, ConnectionTrait, DbBackend, EntityTrait, FromQueryResult, QueryFilter, QueryOrder,
Expand Down Expand Up @@ -509,12 +510,12 @@ impl PurlService {
vulnerabilities: purl_details
.advisories
.iter()
.flat_map(|advisory| {
advisory.status.iter().map(|status| VulnerabilityStatus {
id: status.vulnerability.identifier.clone(),
status: Some(status.into()),
justification: None,
})
.flat_map(|advisory| &advisory.status)
.unique_by(|status| &status.vulnerability.identifier)
.map(|status| VulnerabilityStatus {
id: status.vulnerability.identifier.clone(),
status: Some(status.into()),
justification: None,
})
.collect(),
});
Expand Down