feat: implement validation test 6.2.32 use of Same Product Identifica… by georgbramm · Pull Request #698 · csaf-rs/csaf

georgbramm · 2026-06-23T12:20:15Z

To fulfill the CSAF 2.0/2.1 specification for Test 6.2.32 (Use of Same Product Identification Helper for Different Products), I implemented a complete validation rule in src/validations/test_6_2_32.rs that ensures identifying product metadata is pairwise disjoint across all distinct products.

What Was Done

Comprehensive Path Traversal: Integrated the framework's native visit_all_products_generic visitor engine. This automatically sweeps all valid paths defined in the spec, including recursive nested branches[], flat full_product_names[], and product_paths[] structures without requiring manual pointer-chasing.
Pairwise Disjoint Set Validation: Implemented tracking maps (HashMap<String, String>) for every distinct helper property category: purls (Package URLs), skus (Stock Keeping Units), serial_numbers, model_numbers, hashes (Cryptographic file hashes)
Object Deep-Equality with Order Independence: Built a robust serialization strategy for complex array properties whose items are JSON objects (such as hashes). The implementation extracts the filename and maps nested hashes into a deterministic format (Algorithm:HashValue), then explicitly sorts the array signatures before structural comparison. This guarantees that deep-equality checks pass successfully even if keys or items appear out of order.
Error Reporting Alignment: Configured the rule to emit a detailed ValidationError whenever an identical identifier is found bound to multiple distinct product_id values, pinpointing the precise helper property path layout.
Test Suite Stabilization: Aligned the unit tests with the automated test fixtures (case_01, case_02, case_03), verifying that both intentional failures (like the nested branch serial/model number collisions in Examples 1 & 2) and valid layouts pass cleanly against the standard framework mock runner.

…tion Helper for Different Products

peinjoh

Two more general comments: I think (and please correct me if I'm overlooking something) that some of the to_string() allocs are not necessary here. Is there a reason we are for example re-allocating the serial number as sn_str before using it as a hashmap key, and can this also be done without the to_string() call?

Also, the test coverage only seems to concern serial_numbers and model_numbers right now. I would argue this should be extended to als PIH's, if you have some thoughs here from your work on this task, feel free to open a ticket :)

peinjoh · 2026-06-23T15:22:03Z

+            // Check PURLs
+            if let Some(purls) = helper.get_purls() {
+                for purl in purls {
+                    let purl_str = format!("{purl:?}");


This will use the debug output of CsafPurl enum returned by get_purls. This can be a) Valid or Invalid, depending on if the string could be parsed as a purl. The Valid struct provides a normalized_purl field, which might be helpful for the purpose of this test.

I think it would be interesting to discuss here if this test should consider purl normalization and how invalid purls (that break 6.1.13) should be handled, or if a simple comparison of the original strings is sufficient.

@georgbramm
In the ValidPurl case, this would look like

Valid(ValidPurl { original_purl: "pkg:generic/Example-Company-Product-A@1.0?type=product", normalized_purl: "pkg:generic/Example-Company-Product-A@1.0?type=product", base_without_qualifiers: "pkg:generic/Example-Company-Product-A@1.0" })

which is not fitting for a hashmap key. You could use the normalized_purl in the valid case, and original_purl in the invalid case, or original_purl in both cases. Either way, we shouldn't use the Debug output for hash keys.

georgbramm · 2026-06-24T12:55:04Z

thanks again for your valuable feedback, i will try to incorporate the requested changes by next week, as i am currently on a business trip

…or; inlined test cases; Wiring test case & updated README.md

georgbramm · 2026-06-29T11:32:48Z

Hopefully fixes Issue #318

…nimal lifetime allocations where possible

georgbramm · 2026-06-29T14:47:01Z

Case 03 Messaging & Cardinality::
If three distinct products share the exact same identifier, all three are violating the pairwise disjoint rule. Instead of tracking the "first" product ID and naming it in the error message, I grouped by the metadata value, counted the occurrences, and if a value appears more than once, emited a clean error for every single product containing it without making assumptions.

tziemek · 2026-07-01T09:04:17Z

+                "/product_tree/branches/0/branches/0/branches/1/product",
+            ),
+        ];
+        case_01_errors.sort_by_key(|e| e.instance_path.clone());


Why is sorting necessary here?

Since we collect product occurrences using a HashMap, the internal iteration order of the errors is non-deterministic across test runs. Because the .expect() test harness macro validates strict index-by-index positional array equality, sorting the expected vectors in the test blocks by instance_path guarantees deterministic passes without testing bugs.

Sorry this is not correct. For the expect macro the cases itself must be in the correct order, but the expected errors within each don't have to be, see

csaf/csaf-rs/src/test_result_comparison.rs

Line 37 in b723aec

// Check that all expected errors exist in actual errors (ignoring order)

georgbramm · 2026-07-03T12:37:45Z

i pushed a fix that hopefully resolves your requested changes

tziemek · 2026-07-06T13:22:33Z

+                // Instead of one combined string, track each inner hash independently:
+                for fh in hash_obj.get_file_hashes() {
+                    let specific_hash_key =
+                        format!("file:{filename};alg:{:?};value:{}", fh.get_algorithm(), fh.get_hash());


We shouldn't use the :? debug variant here too. Either use specific data or implement Display.

Additionally, for algorithm and hash value, we should use lowercase here. The schema will prohibit uppercase letters in these parts, but this test shouldn't rely on the schema test to run.

tziemek · 2026-07-06T13:33:19Z

+    errors.sort_by(|a, b| a.instance_path.cmp(&b.instance_path).then(a.message.cmp(&b.message)));
+    errors.dedup_by(|a, b| a.instance_path == b.instance_path && a.message == b.message);


ValidationError is a struct with PartialEq so comparing the fields on its own should not be necessary. Maybe a simple dedup or other way to ensure uniqueness is simpler.
Also the sort_by should not be necessary, as the test comparison ignores ordering.

… Cleaned up the error deduplication by using simple .dedup() backed by ValidationError's native PartialEq

georgbramm · 2026-07-06T14:35:33Z

Excellent callouts.

Switched out the {:?} debug string generation and replaced it with clean Display matching, forcing everything to .to_lowercase() to insulate the hashing tests from casing deviations regardless of schema enforcement rules.
Refactored the process_violations closure out into an independent, pure private helper function to make it fully testable.
Cleaned up the error deduplication by using simple .dedup() backed by ValidationError's native PartialEq implementation, and completely removed the redundant sorting step since the test suite runner handles random order matching natively.

tziemek · 2026-07-06T15:18:18Z

Please also add some supplementary test cases for every edge case discussed here.

georgbramm · 2026-07-07T09:12:51Z

I added three test cases for CSAF validation rule 6.2.32. The added tests verify that product identification helper metadata remains pairwise disjoint across all distinct entries within the product tree structure. These edge cases were implemented in a dedicated, isolated test function (test_test_6_2_32_edge_cases) to cleanly isolate manually structured validation payloads from the automatic schema formatting expectations of the pre-generated test matrix runner.

The test suite evaluates three specific conditions:

Case 01: Duplicate Cryptographic Hash Detection (Failure)
Verifies that the validation engine flags an error when an identical cryptographic hash string (sha256 value) is declared across two separate product definitions (CSAFPID-HASH-FAIL-1 and CSAFPID-HASH-FAIL-2).

Case 02: Duplicate Package URL (PURL) Detection (Failure)
Verifies that the validation engine flags an error when an identical Package URL (pkg:npm/csaf-validator@0.5.1) is declared across two separate product definitions (CSAFPID-PURL-FAIL-1 and CSAFPID-PURL-FAIL-2).

Case 03: Pairwise Disjoint Identifiers (Success)
Verifies that the validation engine returns an Ok status when separate products declare entirely unique, non-overlapping hashes and PURLs, ensuring that valid configurations pass the rule without raising false positives.

tziemek · 2026-07-07T09:35:41Z

I added three test cases for CSAF validation rule 6.2.32. The added tests verify that product identification helper metadata remains pairwise disjoint across all distinct entries within the product tree structure. These edge cases were implemented in a dedicated, isolated test function (test_test_6_2_32_edge_cases) to cleanly isolate manually structured validation payloads from the automatic schema formatting expectations of the pre-generated test matrix runner.

The test suite evaluates three specific conditions:

Case 01: Duplicate Cryptographic Hash Detection (Failure) Verifies that the validation engine flags an error when an identical cryptographic hash string (sha256 value) is declared across two separate product definitions (CSAFPID-HASH-FAIL-1 and CSAFPID-HASH-FAIL-2).

Case 02: Duplicate Package URL (PURL) Detection (Failure) Verifies that the validation engine flags an error when an identical Package URL (pkg:npm/csaf-validator@0.5.1) is declared across two separate product definitions (CSAFPID-PURL-FAIL-1 and CSAFPID-PURL-FAIL-2).

Case 03: Pairwise Disjoint Identifiers (Success) Verifies that the validation engine returns an Ok status when separate products declare entirely unique, non-overlapping hashes and PURLs, ensuring that valid configurations pass the rule without raising false positives.

Perhaps my request was not clear enough: we try to create csaf test files to improve the overall test coverage of the oasis csaf repository. These are called supplementary test cases and are located within the type-generator in this repo. I would have expected some testfiles to cover all relevant and/or edge cases.

Additionally, because I have only seen that now, your code doesn't cover all product_identification_helpers. Both code and testfiles should cover all cases. For the supplementary test files, you also can combine multiple errors into one file to keep it more simple.

…er collisions

georgbramm · 2026-07-08T09:13:10Z

I have updated the test suite to address the comments and ensure full coverage of rule 6.2.32.

Key changes:

Comprehensive Test Asset: Added csaf-rs_csaf-csaf_2_1-6-2-32-s01.json to include intentional collisions for all ProductIdentificationHelper categories: hashes, purls, serial_numbers, model_numbers, and skus.

Refactored Test Code: Updated the test expectation vector to match the new comprehensive collision set, ensuring deterministic validation.

georgbramm · 2026-07-08T09:36:16Z

please wait with the review... im gonna fix the json & tests according to 6.2.31 comments

…, refine test comments

georgbramm · 2026-07-08T10:23:39Z

now its ready for a review.. hopefully =)

tziemek · 2026-07-09T11:39:54Z

now its ready for a review.. hopefully =)

Please address or answer the open discussions in this PR.

feat: implement validation test 6.2.32 use of Same Product Identifica…

87211b2

…tion Helper for Different Products

georgbramm requested a review from a team as a code owner June 23, 2026 12:20

peinjoh reviewed Jun 23, 2026

View reviewed changes

tziemek requested changes Jun 23, 2026

View reviewed changes

Comment thread csaf-rs/src/validations/test_6_2_32.rs Outdated

Comment thread csaf-rs/src/validations/test_6_2_32.rs Outdated

Bramm added 2 commits June 29, 2026 12:43

added instance_path to helper_error; Switch to version-specific visit…

9d6f480

…or; inlined test cases; Wiring test case & updated README.md

cargo fmt run

d85a780

fixes Issue csaf-rs#318; Collect all occurrences using zero-copy / mi…

a733888

…nimal lifetime allocations where possible

tziemek requested changes Jul 1, 2026

View reviewed changes

Fix hash overlap validation

dbe02c8

tziemek reviewed Jul 6, 2026

View reviewed changes

Comment thread csaf-rs/src/validations/test_6_2_32.rs Outdated

tziemek reviewed Jul 6, 2026

View reviewed changes

fixed pr comments:No {:?}; Refactored the process_violations closure;…

419e82f

… Cleaned up the error deduplication by using simple .dedup() backed by ValidationError's native PartialEq

added 3 edge case tests in test_test_6_2_32_edge_cases

d59c12d

Bramm added 2 commits July 8, 2026 11:10

Fix 6.2.32: Add edge cases to test for comprehensive product identifi…

cef97a8

…er collisions

Fix 6.2.32: updated case comment to match json and test code

0b56256

Fix 6.2.32: Synchronize publisher info and tracking ID, formated JSON…

dd23848

…, refine test comments

		errors.sort_by(\|a, b\| a.instance_path.cmp(&b.instance_path).then(a.message.cmp(&b.message)));
		errors.dedup_by(\|a, b\| a.instance_path == b.instance_path && a.message == b.message);

Uh oh!

Conversation

georgbramm commented Jun 23, 2026

Uh oh!

peinjoh left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

georgbramm commented Jun 24, 2026

Uh oh!

georgbramm commented Jun 29, 2026

Uh oh!

georgbramm commented Jun 29, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

georgbramm commented Jul 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

georgbramm commented Jul 6, 2026

Uh oh!

tziemek commented Jul 6, 2026

Uh oh!

georgbramm commented Jul 7, 2026

Uh oh!

tziemek commented Jul 7, 2026

Uh oh!

georgbramm commented Jul 8, 2026

Uh oh!

georgbramm commented Jul 8, 2026

Uh oh!

georgbramm commented Jul 8, 2026

Uh oh!

tziemek commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

peinjoh left a comment •

edited

Loading