From a8aa0c231baafc81c9bbb2d7667657627341ac14 Mon Sep 17 00:00:00 2001
From: unkin-agent <unkin-agent@unkin.net>
Date: Wed, 12 Aug 2026 23:14:51 +1000
Subject: [PATCH] rpm: make local repodata deterministic (fixes #117) (#118)

Local RPM repos regenerated `repomd.xml` on every request and advertised a `primary.xml.gz` sha256 that drifted every second, because `time.Now().Unix()` was embedded inside the gzipped `primary.xml` (and in `repomd` `<revision>`/`<timestamp>`). The advertised hash therefore never matched the content-addressed `<sha256>-primary.xml.gz` bytes a second later or on the other replica, so `dnf` failed with a checksum mismatch. Part of #117.

How:
- Derives `<time file=>` in `primary.xml` from the persisted `rpm_metadata.created_at` instead of the wall clock; unset timestamps collapse to a fixed `0`.
- Derives `repomd` `<revision>`/`<timestamp>` from the newest package upload time, so `repomd.xml` is byte-identical across replicas and requests.
- Adds `file_path` as a total-order tiebreak to the metadata `ORDER BY`.
- Pins the gzip header (`OS: 255`) so compressed bytes depend only on the payload.
- Adds regression tests: generators are byte-identical across two runs, and the sha256 in `repomd.xml` equals the sha256 of the bytes each `serve*` handler returns.

Reviewed-on: https://git.unkin.net/unkin/artifactapi/pulls/118
Co-authored-by: unkin-agent <unkin-agent@unkin.net>
Co-committed-by: unkin-agent <unkin-agent@unkin.net>
---
 internal/database/rpm_metadata.go             |   9 +-
 internal/provider/provider.go                 |   4 +
 internal/provider/rpm/rpm.go                  |  35 ++++-
 .../provider/rpm/rpm_deterministic_test.go    | 148 ++++++++++++++++++
 4 files changed, 190 insertions(+), 6 deletions(-)
 create mode 100644 internal/provider/rpm/rpm_deterministic_test.go

diff --git a/internal/database/rpm_metadata.go b/internal/database/rpm_metadata.go
index a5cd842..2466b06 100644
--- a/internal/database/rpm_metadata.go
+++ b/internal/database/rpm_metadata.go
@@ -3,6 +3,7 @@ package database
 import (
 	"context"
 	"encoding/json"
+	"time"
 
 	"git.unkin.net/unkin/artifactapi/internal/provider"
 )
@@ -65,6 +66,7 @@ type RPMMetadataRow struct {
 	Obsoletes     json.RawMessage
 	Files         json.RawMessage
 	Changelogs    json.RawMessage
+	CreatedAt     time.Time
 }
 
 func (db *DB) ListRPMMetadataEntries(ctx context.Context, repoName string) ([]provider.RPMMetadata, error) {
@@ -94,6 +96,7 @@ func (db *DB) ListRPMMetadataEntries(ctx context.Context, repoName string) ([]pr
 			SourceRPM:     r.SourceRPM,
 			URL:           r.URL,
 			Packager:      r.Packager,
+			CreatedAt:     r.CreatedAt,
 		}
 		json.Unmarshal(r.Requires, &meta.Requires)
 		json.Unmarshal(r.Provides, &meta.Provides)
@@ -112,10 +115,11 @@ func (db *DB) ListRPMMetadata(ctx context.Context, repoName string) ([]RPMMetada
 			name, epoch, version, release, arch,
 			summary, description, rpm_size, installed_size,
 			license, vendor, build_group, build_host, source_rpm, url, packager,
-			requires, provides, conflicts, obsoletes, files, changelogs
+			requires, provides, conflicts, obsoletes, files, changelogs,
+			created_at
 		FROM rpm_metadata
 		WHERE repo_name = $1
-		ORDER BY name, epoch, version, release, arch
+		ORDER BY name, epoch, version, release, arch, file_path
 	`, repoName)
 	if err != nil {
 		return nil, err
@@ -131,6 +135,7 @@ func (db *DB) ListRPMMetadata(ctx context.Context, repoName string) ([]RPMMetada
 			&r.Summary, &r.Description, &r.RPMSize, &r.InstalledSize,
 			&r.License, &r.Vendor, &r.Group, &r.BuildHost, &r.SourceRPM, &r.URL, &r.Packager,
 			&r.Requires, &r.Provides, &r.Conflicts, &r.Obsoletes, &r.Files, &r.Changelogs,
+			&r.CreatedAt,
 		); err != nil {
 			return nil, err
 		}
diff --git a/internal/provider/provider.go b/internal/provider/provider.go
index f8bab30..6ccb1c9 100644
--- a/internal/provider/provider.go
+++ b/internal/provider/provider.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"time"
 
 	"git.unkin.net/unkin/artifactapi/pkg/models"
 )
@@ -185,6 +186,9 @@ type RPMMetadata struct {
 	Obsoletes     []RPMDep
 	Files         []RPMFile
 	Changelogs    []RPMChangelog
+	// CreatedAt is the persisted upload timestamp; used as a stable, replica-independent
+	// value for the repodata <time>/<revision> fields so generated indexes are deterministic.
+	CreatedAt time.Time
 }
 
 type RPMDep struct {
diff --git a/internal/provider/rpm/rpm.go b/internal/provider/rpm/rpm.go
index ce0bc54..a0f279b 100644
--- a/internal/provider/rpm/rpm.go
+++ b/internal/provider/rpm/rpm.go
@@ -275,7 +275,7 @@ func (p *Provider) serveRepomd(w http.ResponseWriter, r *http.Request, reader pr
 	filelistsHash := sha256Hex(filelists)
 	otherHash := sha256Hex(other)
 
-	repomd := generateRepomd(primaryHash, len(primary), filelistsHash, len(filelists), otherHash, len(other))
+	repomd := generateRepomd(repomdRevision(metas), primaryHash, len(primary), filelistsHash, len(filelists), otherHash, len(other))
 
 	w.Header().Set("Content-Type", "application/xml")
 	w.WriteHeader(http.StatusOK)
@@ -315,8 +315,32 @@ func (p *Provider) serveOther(w http.ResponseWriter, r *http.Request, reader pro
 	w.Write(generateOtherXMLGZ(metas))
 }
 
-func generateRepomd(primaryHash string, primarySize int, filelistsHash string, filelistsSize int, otherHash string, otherSize int) []byte {
-	ts := fmt.Sprintf("%d", time.Now().Unix())
+// stableUnix maps a persisted timestamp to a fixed integer for repodata's
+// informational <time>/<timestamp> fields. Zero times (unset) collapse to 0 so
+// output stays byte-identical across replicas and requests. dnf does not
+// validate these values.
+func stableUnix(t time.Time) int64 {
+	if t.IsZero() {
+		return 0
+	}
+	return t.Unix()
+}
+
+// repomdRevision derives repomd.xml's <revision>/<timestamp> from persisted
+// state: the newest package upload time in the repo. It changes only when the
+// repo's package set does, and is identical on every replica reading the same
+// rows, so repomd.xml is byte-stable.
+func repomdRevision(metas []provider.RPMMetadata) string {
+	var max int64
+	for _, m := range metas {
+		if u := stableUnix(m.CreatedAt); u > max {
+			max = u
+		}
+	}
+	return fmt.Sprintf("%d", max)
+}
+
+func generateRepomd(ts string, primaryHash string, primarySize int, filelistsHash string, filelistsSize int, otherHash string, otherSize int) []byte {
 	var b bytes.Buffer
 	b.WriteString(xml.Header)
 	b.WriteString(`<repomd xmlns="http://linux.duke.edu/metadata/repo" xmlns:rpm="http://linux.duke.edu/metadata/rpm">` + "\n")
@@ -359,7 +383,7 @@ func generatePrimaryXMLGZ(metas []provider.RPMMetadata) []byte {
 		if m.URL != "" {
 			fmt.Fprintf(&xmlBuf, "  <url>%s</url>\n", xmlEscape(m.URL))
 		}
-		fmt.Fprintf(&xmlBuf, "  <time file=\"%d\" build=\"0\"/>\n", time.Now().Unix())
+		fmt.Fprintf(&xmlBuf, "  <time file=\"%d\" build=\"0\"/>\n", stableUnix(m.CreatedAt))
 		fmt.Fprintf(&xmlBuf, "  <size package=\"%d\" installed=\"%d\" archive=\"0\"/>\n", m.RPMSize, m.InstalledSize)
 		fmt.Fprintf(&xmlBuf, "  <location href=\"%s\"/>\n", xmlEscape(m.FilePath))
 		fmt.Fprintf(&xmlBuf, "  <format>\n")
@@ -484,6 +508,9 @@ func xmlEscape(s string) string {
 func gzipBytes(data []byte) []byte {
 	var buf bytes.Buffer
 	gz := gzip.NewWriter(&buf)
+	// Pin every header field so the compressed bytes (and their sha256) depend
+	// only on the payload, never on wall-clock time or the Go version's gzip defaults.
+	gz.Header = gzip.Header{OS: 255}
 	gz.Write(data)
 	gz.Close()
 	return buf.Bytes()
diff --git a/internal/provider/rpm/rpm_deterministic_test.go b/internal/provider/rpm/rpm_deterministic_test.go
new file mode 100644
index 0000000..38e6b9b
--- /dev/null
+++ b/internal/provider/rpm/rpm_deterministic_test.go
@@ -0,0 +1,148 @@
+package rpm
+
+import (
+	"bytes"
+	"compress/gzip"
+	"encoding/xml"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	"git.unkin.net/unkin/artifactapi/internal/provider"
+)
+
+func gunzip(t *testing.T, data []byte) string {
+	t.Helper()
+	zr, err := gzip.NewReader(bytes.NewReader(data))
+	if err != nil {
+		t.Fatalf("gzip reader: %v", err)
+	}
+	out, err := io.ReadAll(zr)
+	if err != nil {
+		t.Fatalf("gunzip: %v", err)
+	}
+	return string(out)
+}
+
+// sampleMetas returns a fixed two-package repo state whose upload timestamps are
+// pinned, so any nondeterminism must come from the generators themselves.
+func sampleMetas() []provider.RPMMetadata {
+	base := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC)
+	return []provider.RPMMetadata{
+		{
+			Name: "alpha", Version: "1.0", Release: "1", Arch: "x86_64",
+			Summary: "a", Description: "d", ContentHash: "sha256:aaa",
+			FilePath: "Packages/alpha-1.0-1.x86_64.rpm", RPMSize: 10, InstalledSize: 20,
+			Provides:  []provider.RPMDep{{Name: "alpha"}},
+			Requires:  []provider.RPMDep{{Name: "libc", Flags: "GE", Version: "2.0"}},
+			CreatedAt: base,
+		},
+		{
+			Name: "beta", Version: "2.0", Release: "3", Arch: "noarch",
+			Summary: "b", Description: "d2", ContentHash: "sha256:bbb",
+			FilePath: "Packages/beta-2.0-3.noarch.rpm", RPMSize: 30, InstalledSize: 40,
+			CreatedAt: base.Add(time.Hour),
+		},
+	}
+}
+
+// TestRepodataGeneratorsDeterministic is the direct regression guard for #117:
+// generating each metadata document twice from identical state must yield
+// byte-identical output (hence an identical sha256). The old code embedded
+// time.Now() inside primary.xml.gz, so its bytes/hash drifted every second.
+func TestRepodataGeneratorsDeterministic(t *testing.T) {
+	metas := sampleMetas()
+	gens := map[string]func([]provider.RPMMetadata) []byte{
+		"primary":   generatePrimaryXMLGZ,
+		"filelists": generateFilelistsXMLGZ,
+		"other":     generateOtherXMLGZ,
+	}
+	for name, gen := range gens {
+		a := gen(metas)
+		b := gen(metas)
+		if sha256Hex(a) != sha256Hex(b) {
+			t.Errorf("%s: sha256 differs between two generations (nondeterministic): %s != %s",
+				name, sha256Hex(a), sha256Hex(b))
+		}
+	}
+
+	// repomd.xml itself must also be byte-stable across regenerations.
+	r1 := generateRepomd(repomdRevision(metas), sha256Hex(generatePrimaryXMLGZ(metas)), 1, "f", 2, "o", 3)
+	r2 := generateRepomd(repomdRevision(metas), sha256Hex(generatePrimaryXMLGZ(metas)), 1, "f", 2, "o", 3)
+	if string(r1) != string(r2) {
+		t.Error("repomd.xml differs between two generations")
+	}
+}
+
+// TestPrimaryTimeUsesPersistedCreatedAt proves the <time> element is a pure
+// function of the persisted upload timestamp, not the wall clock.
+func TestPrimaryTimeUsesPersistedCreatedAt(t *testing.T) {
+	metas := sampleMetas()
+	out := gunzip(t, generatePrimaryXMLGZ(metas))
+	if want := `<time file="1767323045" build="0"/>`; !strings.Contains(out, want) {
+		t.Errorf("primary.xml missing persisted <time> %q; got:\n%s", want, out)
+	}
+	// A zero (unset) CreatedAt collapses to a fixed 0, never a live clock value.
+	metas[0].CreatedAt = time.Time{}
+	out = gunzip(t, generatePrimaryXMLGZ(metas))
+	if !strings.Contains(out, `<time file="0" build="0"/>`) {
+		t.Errorf("zero CreatedAt should emit file=\"0\"; got:\n%s", out)
+	}
+}
+
+type repomdDoc struct {
+	Revision string `xml:"revision"`
+	Data     []struct {
+		Type     string `xml:"type,attr"`
+		Checksum struct {
+			Value string `xml:",chardata"`
+		} `xml:"checksum"`
+		Location struct {
+			Href string `xml:"href,attr"`
+		} `xml:"location"`
+	} `xml:"data"`
+}
+
+// TestRepomdHashMatchesServedBytes asserts the exact invariant #117 violated:
+// the sha256 advertised in repomd.xml equals the sha256 of the bytes the
+// content-addressed serve* handler returns for the same repo state.
+func TestRepomdHashMatchesServedBytes(t *testing.T) {
+	p := &Provider{}
+	reader := fakeRPMReader{metas: sampleMetas()}
+
+	serve := func(path string) *httptest.ResponseRecorder {
+		w := httptest.NewRecorder()
+		r := httptest.NewRequest(http.MethodGet, "/"+path, nil)
+		if !p.ServeLocalIndex(w, r, reader, "repo", path) {
+			t.Fatalf("ServeLocalIndex false for %q", path)
+		}
+		if w.Code != http.StatusOK {
+			t.Fatalf("%s: code %d", path, w.Code)
+		}
+		return w
+	}
+
+	var doc repomdDoc
+	if err := xml.Unmarshal(serve("repodata/repomd.xml").Body.Bytes(), &doc); err != nil {
+		t.Fatalf("parse repomd: %v", err)
+	}
+	if len(doc.Data) != 3 {
+		t.Fatalf("expected 3 <data> entries, got %d", len(doc.Data))
+	}
+
+	for _, d := range doc.Data {
+		// The advertised location is content-addressed: repodata/<sha256>-<type>.xml.gz.
+		body := serve("repodata/" + d.Location.Href[len("repodata/"):]).Body.Bytes()
+		got := sha256Hex(body)
+		if got != d.Checksum.Value {
+			t.Errorf("%s: repomd advertises %s but served bytes hash to %s (dnf would reject)",
+				d.Type, d.Checksum.Value, got)
+		}
+		if d.Location.Href != "repodata/"+d.Checksum.Value+"-"+d.Type+".xml.gz" {
+			t.Errorf("%s: location %q not addressed by its checksum %s", d.Type, d.Location.Href, d.Checksum.Value)
+		}
+	}
+}