Skip to content
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

fix: add missing ignoreMissingVulnerabilityReport logic and add tests #6

Merged
merged 1 commit into from
Nov 27, 2024
Merged

fix: add missing ignoreMissingVulnerabilityReport logic and add tests #6

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
108 changes: 82 additions & 26 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ fn verify_container_image(
"image" => image,
"error" => ?e,
);

return;
}
image_validation_errors.insert(image.to_string(), e);
Expand All @@ -216,7 +217,7 @@ fn verify_container_image(

/// obtains the manifest of the image. This can be done either by extracting the digest from the image reference or by fetching the manifest from the registry.
fn get_image_digest(image: &str) -> Result<String, ImageValidationError> {
// The image reference might already container its digest,
// The image reference might already contain its digest,
// in that case we can return it directly, saving a network call
let image_ref: oci_spec::distribution::Reference =
image
Expand All @@ -230,6 +231,7 @@ fn get_image_digest(image: &str) -> Result<String, ImageValidationError> {

let digest = get_manifest_digest(image)
.map_err(|e| ImageValidationError::ManifestFetchError(e.to_string()))?;

Ok(digest.digest)
}

Expand All @@ -240,21 +242,35 @@ fn verify_image_vulnerabilities(
) -> Result<(), ImageValidationError> {
let namespace = settings.vulnerability_report_namespace.as_str();

let vulnerability_report: VulnerabilityReport = get_resource(&GetResourceRequest {
let vulnerability_report: VulnerabilityReport = match get_resource(&GetResourceRequest {
api_version: "storage.sbombastic.rancher.io/v1alpha1".to_string(),
kind: "VulnerabilityReport".to_string(),
name: digest.to_string(),
namespace: Some(namespace.to_string()),
disable_cache: false,
})
.map_err(|e| {
warn!(LOG_DRAIN, "error fetching vulnerability report";
"error" => ?e,
"digest" => digest,
"namespace" => namespace,
);
ImageValidationError::VulnerabilityReportNotFound()
})?;
}) {
Ok(report) => report,
Err(e) => {
if settings.ignore_missing_vulnerability_report {
info!(LOG_DRAIN,
"ignoring error while attempting to fetch the vulnerability report because ignoreMissingVulnerabilityReport is enabled";
"error" => ?e,
"digest" => digest,
"namespace" => namespace,
);

return Ok(());
}

warn!(LOG_DRAIN, "error fetching vulnerability report";
"error" => ?e,
"digest" => digest,
"namespace" => namespace,
);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@flavio not sure about this, any error is mapped as VulnerabilityReportNotFound.
Unfortunately get_resource is using anyhow instead of typed errors, so maybe we could change the error name here.

Copy link
Member

Choose a reason for hiding this comment

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

Let's stick with that in the meantime. We should define a more granular API for host capabilities, something that returns back error codes

return Err(ImageValidationError::VulnerabilityReportNotFound());
}
};

let trivy_report: sarif::trivy::Report = vulnerability_report.try_into()?;
let report = sarif::Report::new(trivy_report, &settings.allow_always)?;
Expand Down Expand Up @@ -425,7 +441,7 @@ mod tests {

#[rstest]
#[case::always_denied_vulnerability_not_found(
include_bytes!("../test_data/vulnerabilityreport_ubuntu_latest.yaml"),
Some("vulnerabilityreport_ubuntu_latest.yaml".to_owned()),
Settings {
max_severity: None,
allow_always: HashSet::new(),
Expand All @@ -436,7 +452,7 @@ mod tests {
true
)]
#[case::always_denied_vulnerability_found(
include_bytes!("../test_data/vulnerabilityreport_ubuntu_latest.yaml"),
Some("vulnerabilityreport_ubuntu_latest.yaml".to_owned()),
Settings {
max_severity: None,
allow_always: HashSet::new(),
Expand All @@ -447,7 +463,7 @@ mod tests {
false
)]
#[case::no_critical_vulnerability_found(
include_bytes!("../test_data/vulnerabilityreport_ubuntu_latest.yaml"),
Some("vulnerabilityreport_ubuntu_latest.yaml".to_owned()),
Settings {
max_severity: Some(settings::MaxSeverity {
critical: Some(SeverityCount::Total(0)),
Expand All @@ -463,7 +479,7 @@ mod tests {
true
)]
#[case::too_many_low_vulnerabilities(
include_bytes!("../test_data/vulnerabilityreport_ubuntu_latest.yaml"),
Some("vulnerabilityreport_ubuntu_latest.yaml".to_owned()),
Settings {
max_severity: Some(settings::MaxSeverity {
critical: None,
Expand All @@ -479,7 +495,7 @@ mod tests {
false
)]
#[case::too_many_medium_vulnerabilities(
include_bytes!("../test_data/vulnerabilityreport_ubuntu_latest.yaml"),
Some("../test_data/vulnerabilityreport_ubuntu_latest.yaml".to_owned()),
Settings {
max_severity: Some(settings::MaxSeverity {
critical: None,
Expand All @@ -495,28 +511,61 @@ mod tests {
false
)]
#[case::always_allowed_cve_found(
include_bytes!("../test_data/vulnerabilityreport_ubuntu_latest.yaml"),
Some("vulnerabilityreport_ubuntu_latest.yaml".to_owned()),
Settings {
max_severity: Some(settings::MaxSeverity {
critical: None,
high: None,
medium: Some(SeverityCount::Total(0)),
low: None,
}),
allow_always: vec!["CVE-2024-2236".to_string()].into_iter().collect(), // this is the only MEDIUM vulnerability in the report
deny_always: HashSet::new(),
ignore_missing_vulnerability_report: false,
vulnerability_report_namespace: "sbombastic".to_string(),
},
true
)]
#[case::vulnerability_report_not_found(
None,
Settings {
max_severity: Some(settings::MaxSeverity {
critical: None,
high: None,
medium: Some(SeverityCount::Total(0)),
low: None,
}),
allow_always: vec!["CVE-2024-2236".to_string()].into_iter().collect(), // this is the
// only MEDIUM vulnerability in the report
allow_always: HashSet::new(),
deny_always: HashSet::new(),
ignore_missing_vulnerability_report: false,
vulnerability_report_namespace: "sbombastic".to_string(),
},
false
)]
#[case::vulnerability_report_not_found_ignore_missing(
None,
Settings {
max_severity: Some(settings::MaxSeverity {
critical: None,
high: None,
medium: Some(SeverityCount::Total(0)),
low: None,
}),
allow_always: HashSet::new(),
deny_always: HashSet::new(),
ignore_missing_vulnerability_report: true,
vulnerability_report_namespace: "sbombastic".to_string(),
},
true
)]
#[serial]
fn validate_pod(
#[case] vulnerability_report_raw: &[u8],
#[case] vulnerability_report_fixture: Option<String>,
#[case] settings: Settings,
#[case] accepted: bool,
) {
use std::path::PathBuf;

let image = "nginx:1.27.1";
let image_digest = "sha256:1234";
let resource = pod(image);
Expand Down Expand Up @@ -547,11 +596,6 @@ mod tests {
}
});

let sbomtastic_vul_report: VulnerabilityReport =
serde_yaml::from_slice(vulnerability_report_raw)
.expect("cannot deserialize VulnerabilityReport fixture");
println!("{:?}", sbomtastic_vul_report);

let ctx_get_resource = mock_kubernetes_sdk::get_resource_context();
ctx_get_resource
.expect::<sbombastic::storage::v1alpha1::VulnerabilityReport>()
Expand All @@ -569,7 +613,19 @@ mod tests {
));
}

Ok(sbomtastic_vul_report.clone())
if let Some(fixture) = vulnerability_report_fixture.clone() {
let fixture_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.join("test_data")
.join(fixture);
let fixture_file =
std::fs::File::open(fixture_path).expect("cannot open fixture file");
let vulnerability_report: VulnerabilityReport =
serde_yaml::from_reader(fixture_file).expect("cannot parse fixture file");

return Ok(vulnerability_report);
};

Err(anyhow!("VulnerabilityReport not found"))
});

let response = validate(serde_json::to_vec(&request).unwrap().as_slice()).unwrap();
Expand Down
Loading