feat: implement validation test 6.2.31 for hardware and software path…#696
feat: implement validation test 6.2.31 for hardware and software path…#696georgbramm wants to merge 11 commits into
Conversation
peinjoh
left a comment
There was a problem hiding this comment.
Hey there, thank you for your contribution! Here are some prelimary comments, I'll take a thourogh look later today and get back to you 👍
|
Also: Please wire up the tests to the application by commenting in the validations.rs line 👍 |
| // 1. Gather all string references from the product paths cleanly | ||
| let mut valid_path_references: Vec<&str> = Vec::new(); | ||
| let paths = product_tree.get_product_paths(); | ||
|
|
There was a problem hiding this comment.
Two points here: As we only need to check for presence in the vec later, can this be deduplicated (with the included perf improvement) by using a HashSet?
I thought we have a utility function for this already. If not, extracting this as a utility function might be a good idea.
There was a problem hiding this comment.
We have get_relationships_product_references already, but here the product_full_name are also included, which is, i think, not intended (see my comment).
|
|
||
| #[test] | ||
| fn test_test_6_2_31() { | ||
| let case_01_failing_example_1 = Err(vec![generate_hardware_software_mix_error("CSAFPID-908070601")]); |
There was a problem hiding this comment.
I would argue that the test coverage can be improved here. There is currently no coverage of the model_numbers codepath, as the existing tests only concern serial_numbers. Similarly, there is no test products for serial_numbers and model_numbers being present at the same time (which is kind of trivial), and no test for what happens when there is no product_tree. We usually create a seperate issue for test coverage improvement, so this is not something you need to implement, but feel free to collect your thoughs on this on link them here 👍
There was a problem hiding this comment.
Also: Test coverage could be extended to cover files that should produce multiple 6.2.31 warnings, i.e. with multiple offending products.
tziemek
left a comment
There was a problem hiding this comment.
Please also update the README and validation.rs file(-s).
| for path in paths { | ||
| let beg_ref = path.get_beginning_product_reference(); | ||
| valid_path_references.push(beg_ref); | ||
|
|
There was a problem hiding this comment.
A full_product_name in a product_path with the required product_identification_helpers would pass as valid here, despite being not referenced in another product_path. Please clarify if this is as intended, otherwise remove these from the valid paths so they are reported.
Also, please add a supplementary test to cover this.
|
thanks for your valuable feedback, i will try to incorporate the requested changes by next week, as i am currently on a business trip |
…cluded test in csaf2_1/validation.rs
|
i pushed a fix that hopefully resolves your requested changes |
… by being defined inline as a full product node
…rences with get_relationships_product_references.
|
Please add some supplementary test cases for everything that was unclear/discussed in this PR. |
|
Last commit introduced a dedicated suite of supplementary test fixtures to resolve all edge-case validation behavior requested during the code review for Rule 6.2.31 (Hardware/Software Mix). These test scenarios are explicitly organized within a dedicated, manual test function (test_test_6_2_31_edge_cases). Isolating these structural, complex data matrix setups avoids interference with the rigid schema formatting assumptions used by the pre-generated test runner framework. Rule 6.2.31 Supplementary Test Cases Self-Validation Prevention (Case 02): Validates that standalone product entries containing conflicting hardware/software identifiers are caught, ensuring a product cannot implicitly clear its own requirement checks without valid external reference paths. Model Numbers Code Path (Case 03): Adds targeted execution coverage specifically evaluating the model_numbers validation branch, ensuring full parity with existing serial_numbers tests. Simultaneous Identifier Arrays (Case 04): Assures that the simultaneous presence of both serial_numbers and model_numbers inside a single configuration block is parsed and evaluated safely. Multiple Offending Products (Case 05): Verifies full iteration and error accumulation mechanics by demonstrating that the validation engine captures all violation instances across multiple failing products instead of short-circuiting at the first failure. |
Case 01: looks fine, it's the obvious case Missing: case for a product declared within the product_path which has a serial or model number. As mentioned in the other PR. Please add supplementary test files which can then be passed to the oasis tc. |
|
I have updated the pull request to resolve the CSAF 6.2.31 validation issues, including the necessary JSON schema corrections and the requested supplementary test coverage. The updated test suite now includes five specific edge cases to ensure the validator correctly identifies products lacking a valid product path: Case 1: Tests a standard nested product that fails due to a lack of branch category distinction. |
| { | ||
| "category": "product_version", | ||
| "name": "1.0", | ||
| "product": { | ||
| "product_id": "CSAFPID-908070601", | ||
| "name": "Product 1", | ||
| "product_identification_helper": { "serial_numbers": ["SN-1"] } | ||
| } | ||
| }, |
There was a problem hiding this comment.
serial_numbers is already covered by the official test case 01.
| { | ||
| "category": "product_version", | ||
| "name": "4.0", | ||
| "product": { | ||
| "product_id": "CSAFPID-SELF-VAL", | ||
| "name": "Product 4", | ||
| "product_identification_helper": { "serial_numbers": ["SN-4"], "model_numbers": ["MOD-4"] } | ||
| } | ||
| }, | ||
| { | ||
| "category": "product_version", | ||
| "name": "5.0", | ||
| "product": { | ||
| "product_id": "CSAFPID-FIFTH-CASE", | ||
| "name": "Product 5", | ||
| "product_identification_helper": { "serial_numbers": ["SN-5"], "model_numbers": ["MOD-5"] } | ||
| } | ||
| } |
There was a problem hiding this comment.
What is the difference between Product 3, 4 and 5?
| } | ||
| // S01: Edge cases | ||
| let mut s01 = vec![ | ||
| // 1. Nested Product: Fails because it lacks a clear branch category distinction |
There was a problem hiding this comment.
What is meant by that? That there is product_name branch category missing? This doesn't concern this test.
Also it would be true for the other four cases listed here.
| let mut s01 = vec![ | ||
| // 1. Nested Product: Fails because it lacks a clear branch category distinction | ||
| generate_hardware_software_mix_error("CSAFPID-908070601", "/product_tree/branches/0/branches/0/product"), | ||
| // 2. Model Only: Fails validation as branch lacks hardware/software context |
There was a problem hiding this comment.
The branch is not lacking the hardware/software context, a product path is missing.
| generate_hardware_software_mix_error("CSAFPID-908070601", "/product_tree/branches/0/branches/0/product"), | ||
| // 2. Model Only: Fails validation as branch lacks hardware/software context | ||
| generate_hardware_software_mix_error("CSAFPID-MODEL-ONLY", "/product_tree/branches/0/branches/1/product"), | ||
| // 3. Simultaneous: Fails as concurrent IDs create ambiguity in versioned branches |
There was a problem hiding this comment.
This description seems wrong. There is model and serialnumber present, which by itself is fine, but the product path is missing. There is no ambiguity.
| generate_hardware_software_mix_error("CSAFPID-MODEL-ONLY", "/product_tree/branches/0/branches/1/product"), | ||
| // 3. Simultaneous: Fails as concurrent IDs create ambiguity in versioned branches | ||
| generate_hardware_software_mix_error("CSAFPID-SIMULTANEOUS", "/product_tree/branches/0/branches/2/product"), | ||
| // 4. Isolated Self-Val: Fails as branch nesting fails to qualify as a valid leaf |
There was a problem hiding this comment.
This makes no sense, as it's the same data as case 03 and thus adds no additional value.
| generate_hardware_software_mix_error("CSAFPID-SIMULTANEOUS", "/product_tree/branches/0/branches/2/product"), | ||
| // 4. Isolated Self-Val: Fails as branch nesting fails to qualify as a valid leaf | ||
| generate_hardware_software_mix_error("CSAFPID-SELF-VAL", "/product_tree/branches/0/branches/3/product"), | ||
| // 5. Fifth Case: Fails due to unanchored product identification in hierarchy |
There was a problem hiding this comment.
This makes no sense, as it's the same data as case 03 and thus adds no additional value.
…ant test cases, and refine error messaging for missing product paths
|
Fixed: Synchronize publisher info and tracking ID, remove redundant test cases, and refine error messaging for missing product paths |
Implemented the mandatory CSAF Test 6.2.31 (Hardware and Software) validation logic to detect invalid mixtures of hardware and software products within the product tree.
Changes
Verification
Ran cargo test to verify compliance. All underlying framework validation test cases (case_01 through case_13) pass successfully.