Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,9 @@ This creates TypeScript + WASM output in `wasm/`.
### Informative Tests

| Test specification | 2.0 | 2.1 (experimental) |
| --- |-------------------|--------------------|
| 6.3.1 | | |
| 6.3.8 | ✅ | ✅ |
|--------|-------------------|--------------------|
| 6.3.1 | | |
| 6.3.8 | ✅ | ✅ |
| 6.3.16 | ⭕ | ✅ |


2 changes: 1 addition & 1 deletion csaf-rs/src/csaf2_1/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ impl Validatable for CommonSecurityAdvisoryFramework {
"6.3.13" => None, // Some(ValidatorForTest6_3_13.validate(self)),
"6.3.14" => None, // Some(ValidatorForTest6_3_14.validate(self)),
"6.3.15" => None, // Some(ValidatorForTest6_3_15.validate(self)),
"6.3.16" => None, // Some(ValidatorForTest6_3_16.validate(self)),
"6.3.16" => Some(ValidatorForTest6_3_16.validate(self)),
"6.3.17" => None, // Some(ValidatorForTest6_3_17.validate(self)),
"6.3.18" => Some(ValidatorForTest6_3_18.validate(self)),
"6.3.19" => None, // Some(ValidatorForTest6_3_19.validate(self)),
Expand Down
1 change: 1 addition & 0 deletions csaf-rs/src/validations/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,5 +129,6 @@ pub mod test_6_3_09;
pub mod test_6_3_10;
pub mod test_6_3_11;
pub mod test_6_3_12;
pub mod test_6_3_16;
pub mod test_6_3_18;
pub mod test_6_3_20;
208 changes: 208 additions & 0 deletions csaf-rs/src/validations/test_6_3_16.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
use crate::csaf::types::language::CsafLanguage;
use crate::csaf_traits::{
AcknowledgmentTrait, AggregateSeverityTrait, CsafTrait, CsafVersion, DistributionTrait, DocumentTrait,
InvolvementTrait, NoteTrait, ProductGroupTrait, ProductTreeTrait, PublisherTrait, ReferenceTrait, RemediationTrait,
RestartRequiredTrait, RevisionTrait, ThreatTrait, TrackingTrait, VulnerabilityTrait,
};
use crate::validation::{TestFinding, TestFindingData};
use crate::validations::utils::text_check::{TextCheckKind, check_text};

fn create_grammar_finding_info(text: &str, instance_path: &str) -> TestFinding {
TestFinding::Information(TestFindingData {
message: format!("Grammar mistake: '{text}'"),
instance_path: instance_path.to_string(),
})
}

/// 6.3.16 Grammar Check
///
/// If the document language is given it MUST be tested that a grammar check for the given
/// language does not find any mistakes. The test is skipped if the document language is not
/// set. It fails if the given language is not supported (only English is currently supported).
pub fn test_6_3_16_grammar_check(doc: &impl CsafTrait) -> Result<(), Vec<TestFinding>> {
let document = doc.get_document();

// Skip this test if language is not set
let lang = match document.get_lang() {
None => return Ok(()), // #409 skipped
Some(lang) => lang,
};

// Check if the language is supported
// TODO: currently, only english is supported, this will be delegated to the text_check module
// matching in the future
let lang = match &lang {
CsafLanguage::Valid(valid_lang) if valid_lang.is_english() => valid_lang,
_ => {
return Err(vec![TestFinding::Information(TestFindingData {
message: format!("Grammar check does not support language '{lang}'"),
instance_path: "/document/lang".to_string(),
})]);
},
};

let mut errors: Option<Vec<TestFinding>> = None;

// Runs the grammar-check for a single piece of text and appends any resulting findings
let mut check = |text: &str, instance_path: String| {
for finding in check_text(TextCheckKind::Grammar, text, lang) {
errors
.get_or_insert_default()
.push(create_grammar_finding_info(&finding.word, &instance_path));
}
};

// Check all text fields listed in the spec
if let Some(acknowledgments) = document.get_acknowledgments() {
for (a_i, ack) in acknowledgments.iter().enumerate() {
if let Some(summary) = ack.get_summary() {
check(summary, format!("/document/acknowledgments/{a_i}/summary"));
}
}
}

if let Some(aggregate_severity) = document.get_aggregate_severity() {
check(
aggregate_severity.get_text(),
"/document/aggregate_severity/text".to_string(),
);
}

let distribution_text = match document.get_csaf_version() {
CsafVersion::X20 => document.get_distribution_20().and_then(|d| d.get_text()),
CsafVersion::X21 => document.get_distribution_21().ok().and_then(|d| d.get_text()),
};
if let Some(text) = distribution_text {
check(text, "/document/distribution/text".to_string());
}

if let Some(notes) = document.get_notes() {
for (n_i, note) in notes.iter().enumerate() {
if let Some(audience) = note.get_audience() {
check(audience, format!("/document/notes/{n_i}/audience"));
}
check(note.get_text(), format!("/document/notes/{n_i}/text"));
if let Some(title) = note.get_title() {
check(title, format!("/document/notes/{n_i}/title"));
}
}
}

let publisher = document.get_publisher();
if let Some(issuing_authority) = publisher.get_issuing_authority() {
check(issuing_authority, "/document/publisher/issuing_authority".to_string());
}

if let Some(references) = document.get_references() {
for (r_i, reference) in references.iter().enumerate() {
check(reference.get_summary(), format!("/document/references/{r_i}/summary"));
}
}

check(document.get_title(), "/document/title".to_string());

let tracking = document.get_tracking();
for (r_i, revision) in tracking.get_revision_history().iter().enumerate() {
check(
revision.get_summary(),
format!("/document/tracking/revision_history/{r_i}/summary"),
);
}

if let Some(product_tree) = doc.get_product_tree() {
for (pg_i, product_group) in product_tree.get_product_groups().iter().enumerate() {
if let Some(summary) = product_group.get_summary() {
check(summary, format!("/product_tree/product_groups/{pg_i}/summary"));
}
}
}

for (v_i, vuln) in doc.get_vulnerabilities().iter().enumerate() {
let vuln_prefix = format!("/vulnerabilities/{v_i}");

if let Some(acknowledgments) = vuln.get_acknowledgments() {
for (a_i, ack) in acknowledgments.iter().enumerate() {
if let Some(summary) = ack.get_summary() {
check(summary, format!("{vuln_prefix}/acknowledgments/{a_i}/summary"));
}
}
}

for (i_i, involvement) in vuln.get_involvements().iter().flat_map(|v| v.iter()).enumerate() {
if let Some(summary) = involvement.get_summary() {
check(summary, format!("{vuln_prefix}/involvements/{i_i}/summary"));
}
}

if let Some(notes) = vuln.get_notes() {
for (n_i, note) in notes.iter().enumerate() {
if let Some(audience) = note.get_audience() {
check(audience, format!("{vuln_prefix}/notes/{n_i}/audience"));
}
check(note.get_text(), format!("{vuln_prefix}/notes/{n_i}/text"));
if let Some(title) = note.get_title() {
check(title, format!("{vuln_prefix}/notes/{n_i}/title"));
}
}
}

if let Some(references) = vuln.get_references() {
for (r_i, reference) in references.iter().enumerate() {
check(
reference.get_summary(),
format!("{vuln_prefix}/references/{r_i}/summary"),
);
}
}

for (r_i, remediation) in vuln.get_remediations().iter().enumerate() {
check(
remediation.get_details(),
format!("{vuln_prefix}/remediations/{r_i}/details"),
);
for (e_i, entitlement) in remediation.get_entitlements().into_iter().enumerate() {
check(
entitlement,
format!("{vuln_prefix}/remediations/{r_i}/entitlements/{e_i}"),
);
}
if let Some(restart_required) = remediation.get_restart_required()
&& let Some(details) = restart_required.get_details()
{
check(
details,
format!("{vuln_prefix}/remediations/{r_i}/restart_required/details"),
);
}
}

for (t_i, threat) in vuln.get_threats().iter().enumerate() {
check(threat.get_details(), format!("{vuln_prefix}/threats/{t_i}/details"));
}

if let Some(title) = vuln.get_title() {
check(title, format!("{vuln_prefix}/title"));
}
}

errors.map_or(Ok(()), Err)
}

crate::test_validation::impl_validator!(csaf2_1, ValidatorForTest6_3_16, test_6_3_16_grammar_check);

#[cfg(test)]
mod tests {
use super::*;
use crate::csaf2_1::testcases::ExpectedResults_6_3_16 as ExpectedResults;
use crate::csaf2_1::testcases::TESTS_2_1;

#[test]
fn test_test_6_3_16() {
let case_01 = Err(vec![create_grammar_finding_info("must", "/document/notes/0/text")]);

TESTS_2_1.test_6_3_16.expect(ExpectedResults {
case_01,
case_11: Ok(()),
});
}
}
15 changes: 14 additions & 1 deletion csaf-rs/src/validations/utils/text_check/harper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,20 @@ impl TextCheckKind {
fn matches(self, lint_kind: harper_core::linting::LintKind) -> bool {
match self {
TextCheckKind::Spell => lint_kind.is_spelling() || lint_kind.is_typo(),
TextCheckKind::Grammar => lint_kind.is_grammar(),
// Harper does not exclusively classify grammar issues under `LintKind::Grammar`.
// Missing words ("must followed" -> "must be followed") are reported as
// `Miscellaneous`. We ignore spelling and typos, and lints that are purely stilistic.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ironic 👀 Should have asked harper to spellcheck the PR

TextCheckKind::Grammar => {
!(lint_kind.is_spelling()
|| lint_kind.is_typo()
|| lint_kind.is_word_choice()
|| lint_kind.is_style()
|| lint_kind.is_regionalism()
|| lint_kind.is_readability()
|| lint_kind.is_nonstandard()
|| lint_kind.is_enhancement()
|| lint_kind.is_agreement())
Comment on lines +69 to +71

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will re-check the docs and fix

},
}
}
}
Expand Down
39 changes: 37 additions & 2 deletions csaf-rs/src/validations/utils/text_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ use crate::csaf::types::language::ValidCsafLanguage;
pub enum TextCheckKind {
/// Spell checking only.
Spell,
/// Grammar checking only (TODO not yet implemented)
#[allow(dead_code)]
/// Grammar checking only.
Grammar,
}

Expand Down Expand Up @@ -133,4 +132,40 @@ mod tests {
"expected no findings for non-English text, got: {findings:?}"
);
}

#[test]
fn detects_grammar_mistake() {
let text = "The security hardening guide must followed for ensure secure operations of a products.";
let findings = check_text(TextCheckKind::Grammar, text, &ValidCsafLanguage::new_for_tests("en-US"));
assert_eq!(findings.len(), 1, "expected exactly one finding, got: {findings:?}");
let finding = findings.first().unwrap();
assert_eq!(
&text[finding.start..finding.end],
"must",
"expected a grammar finding for the missing 'be'"
);
}

#[test]
fn does_not_flag_correct_grammar() {
let findings = check_text(
TextCheckKind::Grammar,
"The security hardening guide must be followed to ensure secure operations of the products.",
&ValidCsafLanguage::new_for_tests("en-US"),
);
assert!(findings.is_empty(), "expected no grammar findings, got: {findings:?}");
}

#[test]
fn grammar_check_ignores_pure_spelling_issues() {
let findings = check_text(
TextCheckKind::Grammar,
"Secruity researchers",
&ValidCsafLanguage::new_for_tests("en-US"),
);
assert!(
findings.is_empty(),
"grammar check should not flag pure spelling issues, got: {findings:?}"
);
}
}
Loading