From 5e168b86c8c011193d486dca5cfbbfd43645eeb8 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 12 Aug 2026 07:36:29 +0200 Subject: [PATCH] refactor: ask the mode module instead of retyping it Four sites still compared the verification mode to a literal after the module owning that policy existed. One of them mattered: an unrecognised mode fell through to MD5 verification while compute_severity was scoring it as existence, so a typo in a platform YAML produced a report whose checks and severities described different platforms. The mode is normalized once per run and the consumers ask for what they need. A test reads the sources and fails on a literal comparison, so the next consumer cannot quietly grow a fifth copy. --- scripts/generate_pack.py | 20 ++++++++++++----- scripts/generate_site.py | 3 ++- scripts/verify.py | 17 ++++++++++----- tests/test_native_mode.py | 46 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 12 deletions(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 55f1238a..bace9ef0 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -56,7 +56,11 @@ from common import ( import region as region_mod import slot as slot_mod from deterministic_zip import _FIXED_DATE_TIME, rebuild_zip_deterministic -from nativemode import hash_mismatch_excludes_file +from nativemode import ( + digest_algorithm, + hash_mismatch_excludes_file, + reads_file_contents, +) from validation import ( _build_validation_index, check_file_validation, @@ -1745,7 +1749,9 @@ def generate_pack( file_reasons[dedup_key] = "not found" continue - if status == "hash_mismatch" and verification_mode != "existence": + if status == "hash_mismatch" and hash_mismatch_excludes_file( + verification_mode + ): zf_name = file_entry.get("zipped_file") if zf_name and local_path: inner_md5_raw = file_entry.get("md5", "") @@ -3779,7 +3785,8 @@ def generate_manifest( # hash the local dump contradicts is not a reason to withhold # the file. Hash platforms would reject it, so they omit it. if status in ("not_found", "external") or ( - status == "hash_mismatch" and verification_mode != "existence" + status == "hash_mismatch" + and hash_mismatch_excludes_file(verification_mode) ): record_omission(full_dest, file_entry, sys_id, status, None) continue @@ -4529,19 +4536,20 @@ def verify_pack_against_platform( # error when the repo holds a file matching a declaration: without # one, the pack ships its best effort and the gap is a data issue # reported by verify.py, not a generation bug. - if verification_mode in ("md5", "sha1"): + digest = digest_algorithm(verification_mode) + if reads_file_contents(verification_mode) and digest: for member, decl_entries in decl_by_member.items(): checkable = [ fe for fe in decl_entries - if str(fe.get(verification_mode) or "").strip() + if str(fe.get(digest) or "").strip() ] if not checkable: continue member_errors = [] satisfied = False for fe in checkable: - err = _check_member_hash(zf, member, fe, verification_mode) + err = _check_member_hash(zf, member, fe, digest) if err is None: satisfied = True break diff --git a/scripts/generate_site.py b/scripts/generate_site.py index 25810183..e992349d 100644 --- a/scripts/generate_site.py +++ b/scripts/generate_site.py @@ -42,6 +42,7 @@ from common import ( write_if_changed, yaml_load, ) +from nativemode import reads_file_contents yaml = require_yaml() from generate_readme import compute_coverage, manifest_totals @@ -1603,7 +1604,7 @@ def generate_platform_page( pct_val = cov["present"] / cov["total"] * 100 if cov["total"] else 0 mode_badge = ( - "rb-badge-success" if mode in ("md5", "sha1") else "rb-badge-info" + "rb-badge-success" if reads_file_contents(mode) else "rb-badge-info" ) lines = [ diff --git a/scripts/verify.py b/scripts/verify.py index b3e8317d..dda6ca5e 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -53,7 +53,12 @@ from common import ( ) yaml = require_yaml() -from nativemode import hash_mismatch_excludes_file, reads_file_contents +from nativemode import ( + digest_algorithm, + hash_mismatch_excludes_file, + normalize as normalize_mode, + reads_file_contents, +) from validation import ( _build_validation_index, _parse_validation, @@ -764,7 +769,9 @@ def verify_platform( A region priority list narrows the report to the files a pack built with the same list would carry, using the same selection function as the builder. """ - mode = config.get("verification_mode", "existence") + # Normalized once: a typo in a platform YAML must not verify with one + # mode and be scored with another. + mode = normalize_mode(config.get("verification_mode")) platform = config.get("platform", "unknown") has_zipped = any( @@ -842,14 +849,14 @@ def verify_platform( zip_contents, data_dir_registry=data_dir_registry, ) - if mode == "existence": + if not reads_file_contents(mode): result = verify_entry_existence( file_entry, local_path, validation_index, db, ) - elif mode == "sha1": + elif digest_algorithm(mode) == "sha1": result = verify_entry_sha1(file_entry, local_path) else: result = verify_entry_md5(file_entry, local_path, resolve_status) @@ -1158,7 +1165,7 @@ def print_platform_result( problems = total - ok_count # Summary line - if mode == "existence": + if not reads_file_contents(mode): if problems: missing = c.get(Severity.WARNING, 0) + c.get(Severity.CRITICAL, 0) optional_missing = c.get(Severity.INFO, 0) diff --git a/tests/test_native_mode.py b/tests/test_native_mode.py index 1e1c2e1b..3e144f37 100644 --- a/tests/test_native_mode.py +++ b/tests/test_native_mode.py @@ -116,6 +116,52 @@ class BothConsumersAgree(unittest.TestCase): ) +class ThePolicyIsSpeltInOnePlace(unittest.TestCase): + """Consumers ask the module; they do not re-spell the rule. + + Four sites still compared the mode to a literal after this module existed, + and one of them dispatched an unknown mode to MD5 verification while + compute_severity was scoring it as existence. + """ + + LITERALS = ('!= "existence"', '== "existence"', 'in ("md5", "sha1")') + + def test_no_consumer_compares_the_mode_to_a_literal(self): + scripts = Path(__file__).resolve().parent.parent / "scripts" + offenders = [] + for path in sorted(scripts.glob("*.py")): + if path.name == "nativemode.py": + continue + for number, line in enumerate(path.read_text().splitlines(), 1): + if any(literal in line for literal in self.LITERALS): + offenders.append(f"{path.name}:{number}: {line.strip()}") + self.assertEqual( + offenders, [], + "route these through nativemode: " + "; ".join(offenders), + ) + + def test_an_unknown_mode_verifies_the_way_it_is_scored(self): + """A typo in a platform YAML must not verify one way and score another.""" + import verify + + self.assertFalse(nativemode.reads_file_contents("sha256")) + self.assertEqual( + verify.compute_severity(verify.Status.MISSING, True, "sha256"), + verify.Severity.WARNING, + ) + config = { + "platform": "Typo", + "verification_mode": "shaa1", + "systems": {}, + "cores": [], + } + result = verify.verify_platform( + config, {"files": {}, "indexes": {}}, emu_profiles={}, + supplemental_names=set(), + ) + self.assertEqual(result["verification_mode"], nativemode.DEFAULT_MODE) + + class GapAnalysisAgreesWithTheBuilder(unittest.TestCase): """"Available" must mean the pack will carry it.