fix(xcat-dep): repo gate — duplicate version is a hard error; signature strictly matches CLI key

Align the apt gate with the EL gate so both AGREE on success/failure: - Duplicate: parse_packages_index now DIES loudly on a package with two DISTINCT versions (stale .deb not cleaned), mirroring EL's rpm_version -- no more silent keep-highest. Removed the dpkg keep-highest oracle; added a happy/sad test. - Signature: sig_observed_key returns the signer fingerprint or undef (no presence-only fallback); the gate hard-fails (SIGKEY) if --gpg-key-id doesn't resolve to a fingerprint, so it always confirms the repo was signed by EXACTLY the CLI key. Signature is required only when --gpg-sign was used. - Align the MISSING message with EL (undef pin -> '*'). Document the gate + idiosyncrasies in BUILD.md. prove t/sbuild-all.t: 98/98. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
2026-09-12 12:36:23 +00:00 · 2026-08-12 17:11:42 -03:00
parent 6f7497cceb
commit 1ac98b6fb6
4 changed files with 71 additions and 39 deletions
@@ -356,6 +356,38 @@ prints the full manual; the shared flags (`--repo-root`, `--manifest`,
 `--skip-build/-install/-genesis/-xcat-dep`, `--build-number`, `--gpg-sign`, `--dry-run`, …) match
 `mockbuild-all.pl`.

+# Repository verification gate
+
+After the apt repo is assembled + signed, `sbuild-all.pl` runs a **manifest-driven gate** that fails
+the build if the published repo is incomplete or mis-signed. It uses `debs-manifest.conf` as the
+single source of truth and is layered so the decision logic is pure and unit-tested
+(`BuildUtils::verify_repo_packages` / `verify_repo_signature`; `parse_packages_index` /
+`resolve_present_names` for parsing/resolution), separate from the disk/gpg I/O.
+
+- **Runs automatically** at the end of `assemble_apt`, **per codename × arch**. Suppress with
+  `--no-verify-repo`; skipped under `--dry-run`. Verify an assembled tree out of band with
+  `--verify-repo=<apt_dir>` (manifest/dists/key from `--manifest`/`--dists`/`--gpg-key-id`/`--gpg-home`).
+- **Completeness:** for each cell, every package that codename×arch's manifest section requires (after
+  `required_pkgs` skip-filtering) must appear in the published `binary-<arch>/Packages` with a version
+  satisfying its pin. The arch-suffixed genesis (`xcat-genesis-base`) is resolved to **this cell's
+  arch** (`xcat-genesis-base-<arch>`) — never a different arch, so a missing native genesis is caught.
+- **Signature:** each `dists/<cn>/InRelease` (or detached `Release`+`Release.gpg`) must be a *good*
+  signature whose **primary-key fingerprint equals the fingerprint of `--gpg-key-id`** — the repo was
+  signed by exactly the CLI key. Expired/revoked keys and expired signatures are rejected; if the CLI
+  key does not resolve to a fingerprint the gate fails (`SIGKEY`), never passes. A signature is only
+  *required* when `--gpg-sign` was used (an intentionally-unsigned repo does not false-fail).
+
+**Semantic idiosyncrasies (intentional, and mirrored in the EL `mockbuild-all.pl` gate):**
+
+- **What "the repo" is:** the Ubuntu gate reads the **published `binary-<arch>/Packages` index** (what
+  apt serves, per codename×arch); the EL gate reads the **binary rpm files** in the per-target dir.
+  Both check the artifact that ships; they differ only in the Debian-vs-RHEL notion of "the repository".
+- **Duplicate = hard error:** a package appearing in the index with **two distinct versions** (a stale
+  `.deb` not cleaned from the pool) makes `parse_packages_index` **die loudly** rather than keep one —
+  identical behaviour to the EL `rpm_version` gate.
+- **Version pins** are the manifest's *upstream* version; the published Debian version's epoch/revision
+  is stripped (`deb_upstream_version`) before the pin compare.
+
 # References

 - [mock project repository](https://github.com/rpm-software-management/mock)
@@ -162,7 +162,7 @@ sub verify_repo_packages {
        my $pin = $expected->{$pkg};
        my $got = $present->{$pkg};
        if (!defined $got) {
-            push @problems, "MISSING $pkg (manifest requires $pin)";
+            push @problems, "MISSING $pkg (manifest requires " . (defined($pin) ? $pin : '*') . ")";
            next;
        }
        push @problems, "VERSION $pkg: repo has $got, manifest pins $pin"
@@ -197,30 +197,16 @@ sub verify_repo_signature {
    return @problems;
 }

-# _dpkg_available / _dpkg_ver_gt: a pure version-ORDERING oracle used only to break duplicate-stanza
-# ties in parse_packages_index. dpkg is asked to compare two version STRINGS (no repo/disk/manifest
-# I/O, deterministic); when the dpkg binary is absent the caller falls back to last-wins. Duplicate
-# package stanzas essentially never occur in an apt-ftparchive-generated index (one stanza per
-# package), so this path is defensive.
-my $DPKG_AVAILABLE;
-sub _dpkg_available {
-    return $DPKG_AVAILABLE if defined $DPKG_AVAILABLE;
-    $DPKG_AVAILABLE = (system('command -v dpkg >/dev/null 2>&1') == 0) ? 1 : 0;
-    return $DPKG_AVAILABLE;
-}
-sub _dpkg_ver_gt {
-    my ($a, $b) = @_;
-    return system('dpkg', '--compare-versions', $a, 'gt', $b) == 0 ? 1 : 0;
-}
-
 # parse_packages_index($text) -> \%{ package_name => version }
 # Parse a Debian 'Packages' index: RFC822 stanzas separated by blank line(s); each carries a
 # 'Package:' and a 'Version:'. Returns name => version (the FULL Debian version verbatim, epoch +
 # revision included -- the caller strips to the upstream part with deb_upstream_version). A stanza
 # lacking either field is skipped; malformed/empty input yields an empty hash.
-# DUP-NAME CHOICE: if a name appears in more than one stanza, keep the HIGHEST by dpkg version order
-# when dpkg is available, else LAST-WINS (the last stanza's version). Pure: no file/repo I/O (the only
-# subprocess is dpkg as a version-comparison oracle for the rare duplicate).
+# DUPLICATE = LOUD ERROR: apt-ftparchive emits one stanza per package, so a name appearing twice with
+# DISTINCT versions means a stale .deb was not cleaned from the pool before assemble -- a version pin
+# could then pass against the wrong .deb and both could ship. This dies (mirroring the EL rpm_version
+# behaviour), so EL and Ubuntu AGREE: a duplicate is always a hard failure, never a silent keep-one.
+# (An identical repeated version is harmless and kept.)
 sub parse_packages_index {
    my ($text) = @_;
    my %map;
@@ -230,15 +216,10 @@ sub parse_packages_index {
        my ($name) = $stanza =~ /^Package:[ \t]*(\S+)/m;
        my ($ver)  = $stanza =~ /^Version:[ \t]*(\S+)/m;
        next unless defined $name && defined $ver;
-        if (exists $map{$name}) {
-            if (_dpkg_available()) {
-                $map{$name} = $ver if _dpkg_ver_gt($ver, $map{$name});   # keep the highest
-            } else {
-                $map{$name} = $ver;                                      # last-wins fallback
-            }
-        } else {
-            $map{$name} = $ver;
-        }
+        die "FATAL: duplicate package '$name' in the Packages index with distinct versions "
+          . "'$map{$name}' and '$ver' (stale artifact not cleaned before assemble)\n"
+            if exists $map{$name} && $map{$name} ne $ver;
+        $map{$name} = $ver;
    }
    return \%map;
 }
@@ -575,7 +575,7 @@ sub resolve_expected_key {
 # fingerprint, that real fingerprint is returned so a mismatch surfaces as WRONGKEY. Otherwise the gate
 # degrades to presence-only (returns the expected key on success) rather than emit a spurious WRONGKEY.
 sub sig_observed_key {
-    my ($adir, $cn, $expected_key, $expected_is_fpr) = @_;
+    my ($adir, $cn) = @_;
    my $g = $gpg_home ? "GNUPGHOME=" . sh_quote($gpg_home) . " " : '';
    my $inrel  = "$adir/dists/$cn/InRelease";
    my $rel    = "$adir/dists/$cn/Release";
@@ -601,10 +601,12 @@ sub sig_observed_key {
            last;
        }
    }
-    # Only compare when we extracted a full fingerprint AND the expected side is one; otherwise degrade
-    # to presence-only (return the expected value so the pure decider sees a match). We deliberately do
-    # NOT fall back to a short GOODSIG keyid, which could never equal the 40-hex expected fpr.
-    return ($expected_is_fpr && $obs_fpr ne '') ? $obs_fpr : $expected_key;
+    # STRICT: return the signer's primary-key fingerprint, or undef if we could not extract one. No
+    # presence-only fallback -- the caller compares this against the resolved --gpg-key-id fingerprint,
+    # so the gate always confirms the repo was signed by EXACTLY the CLI key (undef -> UNSIGNED, a
+    # different fingerprint -> WRONGKEY). We never return a short GOODSIG keyid (it could not equal the
+    # 40-hex expected fpr) nor the expected value itself (which would rubber-stamp a pass).
+    return $obs_fpr ne '' ? $obs_fpr : undef;
 }

 # verify_assembled_repo($manifest_href, $apt_dir, $dists_aref): the ONE completeness+signature gate,
@@ -626,8 +628,12 @@ sub verify_assembled_repo {
    my $expected_is_fpr = ($expected_key =~ /^[0-9A-Fa-f]{16,}$/) ? 1 : 0;
    print "  apt-dir: $adir\n";
    print "  signature check: " . ($sig_enabled
-        ? "on (expected key $expected_key" . ($expected_is_fpr ? '' : ' [unresolved -> presence-only]') . ")"
+        ? "on (expected key $expected_key" . ($expected_is_fpr ? '' : ' [UNRESOLVED -> hard fail]') . ")"
        : "SKIPPED (no --gpg-home/--gpg-sign)") . "\n";
+    # STRICT: if signing is expected but --gpg-key-id does not resolve to a fingerprint (not in the
+    # keyring), we CANNOT confirm the signer -- that is a hard failure, never a presence-only pass.
+    push @all, "SIGKEY: cannot resolve --gpg-key-id '$gpg_key_id' to a fingerprint (in the $gpg_home keyring?)"
+        if $sig_enabled && !$expected_is_fpr;

    my (%exp_sig, %obs_sig);
    for my $cn (@$dists) {
@@ -649,13 +655,14 @@ sub verify_assembled_repo {
            my %pins = map { $_ => $req->{$_} } @names;
            push @all, map { "[$cn/$a] $_" } verify_repo_packages(\%pins, \%present);
        }
-        # signature IO: record the expected + observed signer for this codename (decided in bulk below).
-        if ($sig_enabled) {
+        # signature IO: record the expected + observed signer for this codename (compared in bulk below).
+        # Only when the expected key resolved to a fingerprint (else the SIGKEY hard-fail above stands).
+        if ($sig_enabled && $expected_is_fpr) {
            $exp_sig{$cn} = $expected_key;
-            $obs_sig{$cn} = sig_observed_key($adir, $cn, $expected_key, $expected_is_fpr);
+            $obs_sig{$cn} = sig_observed_key($adir, $cn);
        }
    }
-    push @all, verify_repo_signature(\%exp_sig, \%obs_sig) if $sig_enabled;
+    push @all, verify_repo_signature(\%exp_sig, \%obs_sig) if $sig_enabled && $expected_is_fpr;

    if (@all) {
        print "$_\n" for @all;
@@ -200,6 +200,18 @@ PKG
        'malformed input (no Package:/Version:) -> empty hash');
 }

+# DUPLICATE = loud error: a package appearing twice with DISTINCT versions (a stale .deb left in the
+# pool) must DIE -- so EL and Ubuntu AGREE that a duplicate is a hard failure, never a silent keep-one.
+{
+    my $dup  = "Package: ipmitool-xcat\nVersion: 1.8.18-4\n\nPackage: ipmitool-xcat\nVersion: 1.8.17-3\n";
+    my $ok   = eval { parse_packages_index($dup); 1 };
+    ok(!$ok, 'duplicate package with distinct versions dies (loud error)');
+    like($@, qr/duplicate package 'ipmitool-xcat'/, '... and names the offending package');
+    # an IDENTICAL repeated version is harmless (idempotent) and must NOT die.
+    my $m2 = eval { parse_packages_index("Package: a\nVersion: 1-1\n\nPackage: a\nVersion: 1-1\n") };
+    is(($m2 && $m2->{a}), '1-1', 'an identical repeated version is kept, not an error');
+}
+
 # ---- deb_upstream_version: strip epoch + debian revision ----------------------------------------
 is(deb_upstream_version('1.8.18-4'), '1.8.18', 'strip -revision');
 is(deb_upstream_version('2:0.3.3-snap202608101400.57'), '0.3.3', 'strip epoch and -revision');