Skip to content

Commit

Permalink
Accomodate API breaking changes around issues. (#1345)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mathew-horner
mathew-horner authored Jan 24, 2024
1 parent f526420 commit bc2a7a6
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 100 deletions.
60 changes: 30 additions & 30 deletions cli/src/filter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::str::FromStr;
use phylum_types::types::job::JobStatusResponse;
use phylum_types::types::package::PackageStatusExtended;

use crate::types::{Package, RiskDomain, RiskLevel, RiskType};
use crate::types::{Package, RiskDomain, RiskLevel};

/// Remove issues based on a filter.
pub trait FilterIssues {
Expand All @@ -14,7 +14,7 @@ pub trait FilterIssues {

impl FilterIssues for Package {
fn filter(&mut self, filter: &Filter) {
self.issues.retain(|issue| !should_filter_issue(filter, issue.impact, issue.risk_type));
self.issues.retain(|issue| !should_filter_issue(filter, issue.severity, issue.domain));
}
}

Expand All @@ -39,15 +39,15 @@ impl FilterIssues for PackageStatusExtended {
}

/// Check if a package should be filtered out.
fn should_filter_issue(filter: &Filter, level: RiskLevel, risk_type: impl Into<RiskType>) -> bool {
fn should_filter_issue(filter: &Filter, level: RiskLevel, risk_domain: RiskDomain) -> bool {
if let Some(filter_level) = filter.level {
if level < filter_level {
return true;
}
}

if let Some(domains) = &filter.domains {
if !domains.contains(&risk_type.into()) {
if !domains.contains(&risk_domain) {
return true;
}
}
Expand All @@ -57,7 +57,7 @@ fn should_filter_issue(filter: &Filter, level: RiskLevel, risk_type: impl Into<R

pub struct Filter {
pub level: Option<RiskLevel>,
pub domains: Option<Vec<RiskType>>,
pub domains: Option<Vec<RiskDomain>>,
}

impl FromStr for Filter {
Expand Down Expand Up @@ -88,28 +88,28 @@ impl FromStr for Filter {
let domains = tokens
.iter()
.filter_map(|t| match *t {
"aut" => Some(RiskType::AuthorsRisk),
"AUT" => Some(RiskType::AuthorsRisk),
"auth" => Some(RiskType::AuthorsRisk),
"author" => Some(RiskType::AuthorsRisk),
"eng" => Some(RiskType::EngineeringRisk),
"ENG" => Some(RiskType::EngineeringRisk),
"engineering" => Some(RiskType::EngineeringRisk),
"code" => Some(RiskType::MaliciousRisk),
"malicious_code" => Some(RiskType::MaliciousRisk),
"malicious" => Some(RiskType::MaliciousRisk),
"mal" => Some(RiskType::MaliciousRisk),
"MAL" => Some(RiskType::MaliciousRisk),
"vuln" => Some(RiskType::Vulnerabilities),
"vulnerability" => Some(RiskType::Vulnerabilities),
"VLN" => Some(RiskType::Vulnerabilities),
"vln" => Some(RiskType::Vulnerabilities),
"license" => Some(RiskType::LicenseRisk),
"lic" => Some(RiskType::LicenseRisk),
"LIC" => Some(RiskType::LicenseRisk),
"aut" => Some(RiskDomain::AuthorRisk),
"AUT" => Some(RiskDomain::AuthorRisk),
"auth" => Some(RiskDomain::AuthorRisk),
"author" => Some(RiskDomain::AuthorRisk),
"eng" => Some(RiskDomain::EngineeringRisk),
"ENG" => Some(RiskDomain::EngineeringRisk),
"engineering" => Some(RiskDomain::EngineeringRisk),
"code" => Some(RiskDomain::Malicious),
"malicious_code" => Some(RiskDomain::Malicious),
"malicious" => Some(RiskDomain::Malicious),
"mal" => Some(RiskDomain::Malicious),
"MAL" => Some(RiskDomain::Malicious),
"vuln" => Some(RiskDomain::Vulnerabilities),
"vulnerability" => Some(RiskDomain::Vulnerabilities),
"VLN" => Some(RiskDomain::Vulnerabilities),
"vln" => Some(RiskDomain::Vulnerabilities),
"license" => Some(RiskDomain::LicenseRisk),
"lic" => Some(RiskDomain::LicenseRisk),
"LIC" => Some(RiskDomain::LicenseRisk),
_ => None,
})
.collect::<HashSet<RiskType>>();
.collect::<HashSet<RiskDomain>>();

let domains = if domains.is_empty() { None } else { Some(Vec::from_iter(domains)) };

Expand Down Expand Up @@ -150,8 +150,8 @@ mod tests {
let domains = filter.domains.expect("No risk domains parsed from filter string");

assert_eq!(domains.len(), 2);
assert!(domains.contains(&RiskType::AuthorsRisk));
assert!(domains.contains(&RiskType::EngineeringRisk));
assert!(domains.contains(&RiskDomain::AuthorRisk));
assert!(domains.contains(&RiskDomain::EngineeringRisk));

let filter_string = "crit,author,AUT,med,ENG,foo,engineering,VLN";

Expand All @@ -160,9 +160,9 @@ mod tests {
let domains = filter.domains.expect("No risk domains parsed from filter string");

assert_eq!(domains.len(), 3);
assert!(domains.contains(&RiskType::AuthorsRisk));
assert!(domains.contains(&RiskType::EngineeringRisk));
assert!(domains.contains(&RiskType::Vulnerabilities));
assert!(domains.contains(&RiskDomain::AuthorRisk));
assert!(domains.contains(&RiskDomain::EngineeringRisk));
assert!(domains.contains(&RiskDomain::Vulnerabilities));
}

#[test]
Expand Down
22 changes: 11 additions & 11 deletions cli/src/format.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ use vulnreach_types::Vulnerability;
use crate::commands::status::PhylumStatus;
use crate::print::{self, table_format};
use crate::types::{
HistoryJob, IssuesListItem, Package, PolicyEvaluationResponse, PolicyEvaluationResponseRaw,
RiskLevel, RiskType, UserToken,
HistoryJob, Issue, Package, PolicyEvaluationResponse, PolicyEvaluationResponseRaw, RiskLevel,
UserToken,
};

/// Format type for CLI output.
Expand Down Expand Up @@ -133,10 +133,10 @@ impl Format for PolicyEvaluationResponseRaw {
let _ = writeln!(writer, "[{}] {}@{}", package.registry, package.name, package.version);

for rejection in package.rejections.iter().filter(|rejection| !rejection.suppressed) {
let domain = rejection.source.domain.map_or_else(
|| " ".into(),
|domain| format!("[{}]", RiskType::from(domain)),
);
let domain = rejection
.source
.domain
.map_or_else(|| " ".into(), |domain| format!("[{}]", domain));
let message = format!("{domain} {}", rejection.title);

let colored = match rejection.source.severity {
Expand Down Expand Up @@ -285,7 +285,7 @@ impl Format for Package {
let issues = self.issues.to_owned();

for issue in &issues {
let rows: Vec<Row> = issueslistitem_to_row(issue);
let rows: Vec<Row> = issue_to_row(issue);
for mut row in rows {
row.remove_cell(2);
issues_table.add_row(row);
Expand Down Expand Up @@ -466,11 +466,11 @@ fn leftpad(text: &str, width: usize) -> String {
format!("{}{}", text, str::repeat(" ", delta))
}

fn issueslistitem_to_row(issue: &IssuesListItem) -> Vec<Row> {
fn issue_to_row(issue: &Issue) -> Vec<Row> {
let row_1 = Row::new(vec![
Cell::new_align(&issue.impact.to_string(), Alignment::LEFT)
.with_style(Attr::ForegroundColor(risk_level_to_color(&issue.impact))),
Cell::new_align(&format!("{} [{}]", &issue.title, issue.risk_type), Alignment::LEFT)
Cell::new_align(&issue.severity.to_string(), Alignment::LEFT)
.with_style(Attr::ForegroundColor(risk_level_to_color(&issue.severity))),
Cell::new_align(&format!("{} [{}]", &issue.title, issue.domain), Alignment::LEFT)
.with_style(Attr::Bold),
]);

Expand Down
77 changes: 18 additions & 59 deletions cli/src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,7 @@ pub struct Package {
pub download_count: u32,
pub risk_scores: RiskScores,
pub total_risk_score_dynamics: Option<Vec<ScoreDynamicsPoint>>,
pub issues_details: Vec<Issue>,
pub issues: Vec<IssuesListItem>,
pub issues: Vec<Issue>,
pub authors: Vec<Author>,
pub developer_responsiveness: Option<DeveloperResponsiveness>,
pub issue_impacts: IssueImpacts,
Expand Down Expand Up @@ -210,7 +209,7 @@ pub struct ScoreDynamicsPoint {
}

/// An issue that Phylum has found with a package.
#[derive(Serialize, Deserialize)]
#[derive(Clone, Serialize, Deserialize)]
pub struct Issue {
pub tag: Option<String>,
pub id: Option<String>,
Expand All @@ -224,13 +223,13 @@ pub struct Issue {
}

/// Extra information about an issue that depends on the type of issue.
#[derive(Serialize, Deserialize)]
#[derive(Clone, Serialize, Deserialize)]
#[serde(rename_all = "snake_case", tag = "type")]
pub enum IssueDetails {
Vulnerability(VulnDetails),
}

#[derive(Serialize, Deserialize)]
#[derive(Clone, Serialize, Deserialize)]
pub struct VulnDetails {
/// The CVE ids that this vuln is linked to.
pub cves: Vec<String>,
Expand All @@ -240,59 +239,6 @@ pub struct VulnDetails {
pub cvss_vector: String,
}

#[derive(Serialize, Deserialize, Clone)]
#[serde(rename_all = "camelCase")]
pub struct IssuesListItem {
pub risk_type: RiskType,
pub score: f32,
pub impact: RiskLevel,
pub description: String,
pub title: String,
pub tag: Option<String>,
pub id: Option<String>,
pub ignored: IgnoredReason,
}

/// The category of risk that an issue poses.
#[derive(Serialize, Deserialize, PartialEq, Eq, Hash, Copy, Clone)]
#[serde(rename_all = "camelCase")]
pub enum RiskType {
TotalRisk,
Vulnerabilities,
#[serde(alias = "maliciousRisk")]
#[serde(rename = "maliciousCodeRisk")]
MaliciousRisk,
AuthorsRisk,
EngineeringRisk,
LicenseRisk,
}

impl fmt::Display for RiskType {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let risk_domain = match self {
RiskType::MaliciousRisk => "MAL",
RiskType::Vulnerabilities => "VLN",
RiskType::EngineeringRisk => "ENG",
RiskType::AuthorsRisk => "AUT",
RiskType::LicenseRisk => "LIC",
RiskType::TotalRisk => "ALL",
};
write!(f, "{risk_domain}")
}
}

impl From<RiskDomain> for RiskType {
fn from(risk_domain: RiskDomain) -> Self {
match risk_domain {
RiskDomain::Malicious => RiskType::MaliciousRisk,
RiskDomain::Vulnerabilities => RiskType::Vulnerabilities,
RiskDomain::EngineeringRisk => RiskType::EngineeringRisk,
RiskDomain::AuthorRisk => RiskType::AuthorsRisk,
RiskDomain::LicenseRisk => RiskType::LicenseRisk,
}
}
}

/// The user-specified reason for an issue to be ignored.
#[derive(Serialize, Deserialize, Clone)]
#[serde(rename_all = "camelCase")]
Expand Down Expand Up @@ -345,7 +291,7 @@ pub struct PackageReleaseData {
pub last_release_date: String,
}

#[derive(Serialize, Deserialize, PartialEq, Eq, Copy, Clone, Debug)]
#[derive(Serialize, Deserialize, PartialEq, Eq, Copy, Clone, Debug, Hash)]
pub enum RiskDomain {
/// One or more authors is a possible bad actor or other problems.
#[serde(rename = "author")]
Expand All @@ -365,6 +311,19 @@ pub enum RiskDomain {
LicenseRisk,
}

impl fmt::Display for RiskDomain {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let risk_domain = match self {
RiskDomain::Malicious => "MAL",
RiskDomain::Vulnerabilities => "VLN",
RiskDomain::EngineeringRisk => "ENG",
RiskDomain::AuthorRisk => "AUT",
RiskDomain::LicenseRisk => "LIC",
};
write!(f, "{risk_domain}")
}
}

impl From<PTRiskDomain> for RiskDomain {
fn from(foreign: PTRiskDomain) -> Self {
match foreign {
Expand Down

0 comments on commit bc2a7a6

Please sign in to comment.