fix(quality): compliance false-passes, gate mappings, metric counting, score math#304
Open
matrosov wants to merge 4 commits into
Open
fix(quality): compliance false-passes, gate mappings, metric counting, score math#304matrosov wants to merge 4 commits into
matrosov wants to merge 4 commits into
Conversation
… classical crypto, undocumented CBOM) First cluster from the quality/compliance audit — the flagship problem: a tool advertising CNSA 2.0 / NIST PQC checking green-lit SBOMs it should reject, so a user could certify quantum-safety the tool never actually verified. The audit's threat model is a false regulatory claim; every fix here flips a false verdict in the correct direction, and the A/B confirms no over-correction (legitimate PQC crypto still passes). Vacuous pass on empty inventory: - check_cnsa2 / check_nist_pqc iterated components, `continue`d past every non-Cryptographic one, and emitted nothing — so a plain application SBOM (no CBOM) reported COMPLIANT for CNSA 2.0 and NIST PQC. They now emit an Error (SBOM-CNSA2-000 / SBOM-PQC-000) when zero cryptographic assets were evaluated: a PQC-readiness verdict must not be "compliant" having checked nothing. Classical quantum-vulnerable crypto passed PQC: - PQC-001 fired only when nistQuantumSecurityLevel == Some(0), and the broken-family list had no classical asymmetric algorithm — so an RSA / ECDSA / ECDH / DH / DSA CBOM with an unset quantum level (the common real-world case) passed NIST PQC. A new AlgorithmProperties::is_classical_quantum_vulnerable() flags these on the family alone (broken by Shor's algorithm regardless of key size); PQC-001 now fires on it, and the quantum_vulnerable_count metric counts it. Grade-A for undocumented crypto: - CryptographyMetrics incremented total_crypto_components BEFORE the crypto_properties guard, so a Cryptographic component with no cryptoProperties made has_data() true and the six CBOM sub-scores returned 100 on empty denominators (grade-A crypto for nothing documented). It now counts only documented assets, so the crypto score is N/A rather than 100. CNSA gate escapes + ECC false-fail: - The hash gate only matched family literally "SHA-2"/"SHA2" with param "256", so SHA-224 and family-form "SHA-256" escaped; it now catches both encodings. AES now requires an explicit 256 (via param or classical security level) instead of passing on an absent parameter. - Conversely, the key-size check false-FAILED every ECC key (flat <2048), but an ECC key size is the curve bit-length (P-256 → 256 ≈ 128-bit security). Strong ECC curve sizes (255/256/384/448/521) are now adequate while RSA/finite-field still needs ≥2048; 512 is deliberately excluded so a factorable 512-bit RSA/DSA key is not passed as "strong ECC". Verified: false-pass/false-fail A/B vs main (all four false-passes flip to fail; ML-KEM-1024/ML-DSA-87/SHA-384/AES-256 clean CBOM stays compliant; ECC P-256/P-384 no longer false-failed; RSA-1024 still flagged) and an over-correction audit (no PQC family or hybrid trips the classical list; it caught the 512-bit-RSA hole in the ECC allowlist, fixed here). 7 new tests; two compliance TUI snapshots correctly flip a crypto-less fixture from pass to fail. Full suite green (1649 tests), clippy -D warnings and fmt clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ity exit code
Second cluster from the quality/compliance audit — gate mis-mappings where a
required element was declared mandatory but never actually failed the verdict,
plus a score/verdict exit-code inconsistency. Threat model is the same false
regulatory claim; each fix flips a false verdict in the correct direction, and
the A/B confirms a fully-complete SBOM is not newly false-failed.
Required elements now gate:
- NTIA "Timestamp" (one of the seven required minimum data fields) was a literal
no-op: the check was skipped on the assumption "we always set a timestamp."
But a missing/invalid source timestamp is stored as the UNIX_EPOCH sentinel,
so it now gates on has_known_timestamp() with an Error (SBOM-NTIA-TIMESTAMP)
for NtiaMinimum/Comprehensive.
- EO 14028 §4(e) Supplier Name — a required NTIA element — was a non-gating
Warning behind a >30% missing threshold while version and unique-id were
Errors. It is now a gating Error on any missing supplier, consistent with the
other required elements.
- BSI TR-03183-2 §5.3 mandatory component name was a comment-only no-op that
claimed the name was "already enforced globally" — but the dedicated BSI
checker never runs the generic component check, so a nameless component
passed. It now enforces the name (SBOM-BSI-TR-03183-2-5-3).
False-fail fixed:
- CRA Art. 24 OSS-steward vulnerability-handling gate only inspected component
external refs and the sidecar, ignoring the document-level
vulnerability_disclosure_url / security_contact that its own doc comment says
satisfy it. Those document fields now count (a strict OR-widening).
Score/verdict exit code:
- `quality` printed the compliance verdict ("NON-COMPLIANT (N errors)") but its
only exit gate was --min-score, so a grade-A but non-compliant SBOM exited 0.
Added an opt-in --fail-on-noncompliant flag that exits COMPLIANCE_ERRORS when
the verdict is non-compliant; the default stays score-only so existing scripts
are unaffected. Exit-code help text updated.
N/A stays compliant (deliberate): a standard that does not apply to an SBOM
(e.g. EU AI Act on a non-AI SBOM) is not a violation, so is_compliant is left
true — unlike the crypto case, where no crypto inventory under a *crypto*
standard means readiness cannot be asserted. The CRA "Default product class
under-gates" behavior is the intended CRA-P3.2 severity-tiering-by-class, not a
bug, and is left as-is.
Also aligned two registry default_severity entries (EO14028-SUPPLIER,
BSI-5-3) to the Error severity actually emitted, and expanded the Art.24
remediation message to mention the document-level satisfiers.
Verified: verdict A/B vs the crypto-cluster commit (all four gates flip in the
correct direction with the expected rule_ids; a fully-complete SBOM stays
compliant under NTIA/EO/BSI; the flag gates only when set, confirmed through the
built binary) and an over-correction audit (the widenings cannot introduce a
false-fail; compliant golden fixtures still pass zero errors). 4 new tests; full
suite green (1653 tests), clippy -D warnings and fmt clean; no golden/snapshot
churn.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-entry Third cluster from the quality audit — metric-counting bugs that inflated scores by conflating per-entry counts with per-component denominators, plus a fake SPDX validator. All fixes are in how metrics COUNT; the A/B confirms a well-formed SBOM scores byte-identically before and after. NOASSERTION counted as licensed: - CompletenessMetrics and LicenseMetrics counted any non-empty declared list as "has license", but the CycloneDX parser emits declared=["NOASSERTION"] for empty license objects — so an SBOM with no real license data reported 100% licensed. A component now needs at least one non-NOASSERTION entry (SPDX NONE still counts as documented: asserting no license exists is information; making no assertion is not). A/B: 4-of-5 NOASSERTION SBOM drops from 100% to 20% licensed. SPDX validity was a substring check: - is_valid_spdx_license accepted any string containing " OR "/" AND "/ " WITH " — "GARBAGE OR NONSENSE" validated. Deleted; metrics now use the is_valid_spdx the model already computes in LicenseExpression::new via the spdx crate (LAX parse). Net direction is MORE accepting of real-world forms (GPL-2.0+, Zlib, MIT/Apache-2.0, case variants now valid; the old 25-id list rejected them) while garbage operands finally fail. - validate_spdx's NOASSERTION/NONE guard now matches whole tokens, so LicenseRef-NONEXCLUSIVE is no longer substring-rejected; and from_spdx_id validates instead of hardcoding is_valid_spdx=true now that scoring trusts the bit. Per-entry counts against per-component denominators: - valid_spdx_expressions counted per declared ENTRY but divided by the per-component with_declared, so one multi-licensed component pushed spdx_ratio to 3.0 and the 30-pt bonus through the 100 clamp (license score 100 at 50% coverage). All license counters are now per-component with valid+non_standard == with_declared, making the ratio <= 1.0 by construction. A/B: that SBOM now scores 60, not 100. - IdentifierMetrics counted CPEs per entry and summed per-type counts, so one component with 3 CPEs masked two components with no identifier at all (coverage 100%). New components_with_valid_id field is the coverage numerator; per-type counts are per-component. A/B: the masking SBOM drops to 33%. Lifecycle double-count: - Deprecated/archived components were counted twice — once for the StalenessLevel and once for the boolean flag, which the enrichment sets together (the normal case, not an edge). Each state now counts a component once; the audit's state-matrix check confirms NEW <= OLD in every combination. Verified: score A/B vs the gate-mapping commit (all five scenarios move by the exact expected amounts; clean SBOM byte-identical; 7-fixture sweep shows only one change, a 59.3->60.3 gain) and an adversarial audit (100+ license forms LAX-vs-old — no legitimate form flips invalid; all construction paths go through LicenseExpression::new; serde round-trip surfaces checked; the concluded-NOASSERTION guard is load-bearing via the SPDX3 path). 5 new tests; full suite green (1657), clippy -D warnings and fmt clean; zero snapshot churn. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…, freshness, coverage clamp) Fourth and final cluster from the quality audit — score-math defects where the arithmetic itself produced perverse or unstable results. Disclosing a vulnerability lowered the score: - A bare VulnerabilityRef (no CVSS/CWE/remediation) scored the VulnDocs category 0, while an SBOM with NO vulnerability data got N/A and its weight redistributed — so disclosing was worse than stripping the data, punishing exactly the transparency an SBOM exists to provide. documentation_score now grants a 40-point disclosure baseline plus up to 60 for quality (CVSS 24 / CWE 18 / remediation 18 — the old 40:30:30 proportions preserved; fully documented still lands exactly on 100). A/B: the perverse gap halves (6.74 → 3.27 pts on the probe SBOM); the quality gradient survives (bare 40 < partial < full 100). The residual gap is deliberate: full parity would require rewarding disclosure above the SBOM's own average. Freshness made scores non-deterministic: - is_fresh (created < 90 days, live wall clock) carried 12 points of the provenance checklist, so identical SBOM bytes scored differently across midnight, and a future-dated document always counted fresh. Freshness is now display-only metadata: the checklist item is removed (denominator 105 → 93 for CycloneDX), making the quality score a pure function of the document — the same determinism contract as the parser content-hash work. Future-dated documents no longer read as fresh. NOTE: fresh documents lose a little provenance ((S+12)/105 → S/93), stale and timestamp-less ones gain — that redistribution is the point. The TUI score explanation no longer cites freshness, and the two quality snapshots move by exactly the renormalization (9→10 / 10%→11%). Cyclic graphs absorbed their own penalty: - Dependency coverage uses an N/(N-1) denominator, so a fully-cyclic graph reached ~125% raw coverage and swallowed the cycle penalty before the final clamp (ring graph scored a perfect 100). Coverage now clamps to 100 before penalties: the same ring scores 95 with its cycle visible; acyclic graphs are bit-identical. Wrong-field recommendation + hardcoded flag: - The "Add VCS URLs" recommendation derived missing-VCS from HASH coverage: it stayed silent for SBOMs with no VCS refs (all hashed) and fired for fully-VCS'd SBOMs missing hashes. AuditabilityMetrics is now plumbed into generate_recommendations and the count uses components_with_vcs. - CompletenessMetrics.has_timestamp was hardcoded true; it now reports has_known_timestamp() (score-neutral; TUI checklist and reports only). Verified: score A/B vs the metric-counting commit (all four fixes move by the measured amounts; a well-formed SBOM's full report JSON differs in exactly two fields — the provenance renormalization) and an adversarial audit (baseline-40 coefficients binary-exact at 100; no consumer special-cases score 0; the one-fake-CVE gaming vector is bounded and smaller than the pre-existing fully-documented-CVE one; both scorer paths checked — score_ai_readiness builds its own recommendations). 4 new tests, one prior test corrected (its "fresh" fixture was year-3999, silently relying on the future-dating bug). Full suite green (1660), clippy -D warnings and fmt clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes all 4 clusters from the quality/compliance audit (32 confirmed findings, 12 high). The threat model throughout: a false regulatory claim — the tool telling a user they comply when they don't.
1. Crypto/CBOM compliance false-passes (
17ae0b0)validate pqc/validate cnsa2no longer pass vacuously on SBOMs with no crypto inventory (new gating ErrorsSBOM-PQC-000/SBOM-CNSA2-000)is_classical_quantum_vulnerable()— an RSA-2048 CBOM previously passed PQC validationCryptographiccomponents no longer score grade A on empty denominators2. Compliance gate mappings (
3f996e5)--fail-on-noncompliantflag:qualitycan no longer print NON-COMPLIANT while exiting 0 (default unchanged — score-only gate)3. Metric counting (
bbacf17)spdx_ratio ≤ 1.0by construction; one multi-CPE component can't mask identifier-less components (was false 100% coverage)ORpassed) is deleted — metrics use the model's realspdx-crate validation; net more accepting of legitimate ids (Zlib, GPL-2.0+, LicenseRef-*)4. Score math (
80818d9)Verification
Every cluster was ground-truth A/B'd against its parent commit and adversarially audited before commit (verdict-flip tables, over-correction checks: clean ML-KEM/ML-DSA CBOMs still pass, complete SBOMs not false-failed, no legitimate license form flips invalid, full-report JSON for a well-formed SBOM differs only in the intended provenance renormalization). 20 new regression tests. Full suite (1660), clippy
-D warnings, fmt clean at every commit.🤖 Generated with Claude Code
https://claude.ai/code/session_01CQvKahGY6VMYWSGMPBh77L