From bfe0df1d5443ccbb86b07e9d293270121d87eff3 Mon Sep 17 00:00:00 2001 From: Patrick Griffis Date: Mon, 7 Sep 2026 12:11:20 -0500 Subject: [PATCH 1/2] fix(record): include branch in the cell directory path Publishing two branches of one app and arch in a single execution had both pushes write into /-, so the second record overwrote the first. build-site then merged only the surviving branch: the other branch's images were pushed to the registry and tagged correctly, but never appeared in the static index or as a .flatpakref, leaving them impossible to install. The OCI tag already distinguishes branches, so let the cell path do the same. Records written without a branch keep the old path, and IterRecords reads the branch from record.json rather than the directory name, so existing cells still load. --- ARCHITECTURE.md | 2 +- pkg/record/record.go | 22 +++++++--- pkg/record/record_test.go | 79 +++++++++++++++++++++++++++++++++-- tests/push_autodetect_test.go | 28 +++++++------ 4 files changed, 109 insertions(+), 22 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index ae74e4f..367b6ab 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -161,7 +161,7 @@ The `StreamWithPrefix` function reads output from subprocess pipes line-by-line * Bundles OSTree repositories into OCI images using `flatpak build-bundle`. * Interacts with container registries using the `go-containerregistry` library. * Generates an image manifest index and signs the manifest payload. Supports bypassing GPG signing when `no_sign` is set to `true`. When `no_sign` is `false`, it enforces that a GPG key must be provided, failing the push unless `allow_unsigned` is explicitly enabled. -* Writes a uniform execution record (`record.json`) under `/-/` tracking the OCI digest, registry, and label metadata. +* Writes a uniform execution record (`record.json`) under `/--/` tracking the OCI digest, registry, and label metadata. Records written without a branch fall back to `/-/`. ### `pkg/site` * Assembles the static repository landing page. diff --git a/pkg/record/record.go b/pkg/record/record.go index 0ff6572..567e26b 100644 --- a/pkg/record/record.go +++ b/pkg/record/record.go @@ -10,11 +10,12 @@ import ( ) var ( - appIDRegexp = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]{0,254}$`) - archRegexp = regexp.MustCompile(`^[A-Za-z0-9_.-]+$`) + appIDRegexp = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]{0,254}$`) + archRegexp = regexp.MustCompile(`^[A-Za-z0-9_.-]+$`) + branchRegexp = regexp.MustCompile(`^[A-Za-z0-9_.-]+$`) ) -// Record is an immutable snapshot of one published (app, arch) cell. +// Record is an immutable snapshot of one published (app, arch, branch) cell. type Record struct { AppID string `json:"app-id"` Arch string `json:"arch"` @@ -47,15 +48,26 @@ func (r Record) Validate() error { return fmt.Errorf("Record arch %q is invalid (must match %s)", r.Arch, archRegexp.String()) } + // Records written before the branch became part of the cell path carry no + // branch, so only validate it when set. + if r.Branch != "" && !branchRegexp.MatchString(r.Branch) { + return fmt.Errorf("Record branch %q is invalid (must match %s)", r.Branch, branchRegexp.String()) + } + return nil } -// CellDir resolves the cell directory path under root. +// CellDir resolves the cell directory path under root. The branch is part of +// the path so that publishing several branches of one app and arch in a single +// execution does not have each push overwrite the last one's record. func (r Record) CellDir(root string) (string, error) { if err := r.Validate(); err != nil { return "", err } - return filepath.Join(root, fmt.Sprintf("%s-%s", r.AppID, r.Arch)), nil + if r.Branch == "" { + return filepath.Join(root, fmt.Sprintf("%s-%s", r.AppID, r.Arch)), nil + } + return filepath.Join(root, fmt.Sprintf("%s-%s-%s", r.AppID, r.Branch, r.Arch)), nil } // WriteRecord writes the record and labels into a cell directory under root. diff --git a/pkg/record/record_test.go b/pkg/record/record_test.go index aeffe9c..90cc4d9 100644 --- a/pkg/record/record_test.go +++ b/pkg/record/record_test.go @@ -58,6 +58,24 @@ func TestRecordValidation(t *testing.T) { }, wantErr: true, }, + { + name: "valid branch", + rec: Record{ + AppID: "org.example.App", + Arch: "x86_64", + Branch: "2.54", + }, + wantErr: false, + }, + { + name: "path traversal in branch", + rec: Record{ + AppID: "org.example.App", + Arch: "x86_64", + Branch: "../escaped", + }, + wantErr: true, + }, } for _, tt := range tests { @@ -138,11 +156,11 @@ func TestWriteAndIterRecords(t *testing.T) { } // Iterate should return sorted by cell directory name: - // AetherPak directory name for rec1: org.example.AppA-x86_64 - // AetherPak directory name for rec2: org.example.AppB-aarch64 + // AetherPak directory name for rec1: org.example.AppA-stable-x86_64 + // AetherPak directory name for rec2: org.example.AppB-beta-aarch64 // Sorted: - // 1st: org.example.AppA-x86_64 (rec1) - // 2nd: org.example.AppB-aarch64 (rec2) + // 1st: org.example.AppA-stable-x86_64 (rec1) + // 2nd: org.example.AppB-beta-aarch64 (rec2) if records[0].Record.AppID != "org.example.AppA" { t.Errorf("expected first sorted record to be org.example.AppA, got %s", records[0].Record.AppID) } @@ -240,3 +258,56 @@ func TestIterRecordsRecursive(t *testing.T) { t.Errorf("expected path to be %s, got %s", cellBDir, records[1].Path) } } + +func TestWriteRecordSeparatesBranches(t *testing.T) { + tempDir := t.TempDir() + + rec := Record{ + AppID: "org.example.App", + Arch: "x86_64", + Name: "my-org/my-app", + Registry: "ghcr.io", + } + + stable := rec + stable.Branch = "master" + stable.Digest = "sha256:1111111111111111111111111111111111111111111111111111111111111111" + stable.Ref = "app/org.example.App/x86_64/master" + + release := rec + release.Branch = "2.54" + release.Digest = "sha256:2222222222222222222222222222222222222222222222222222222222222222" + release.Ref = "app/org.example.App/x86_64/2.54" + + stableCell, err := WriteRecord(tempDir, stable, map[string]string{"org.flatpak.ref": stable.Ref}) + if err != nil { + t.Fatalf("failed to write master record: %v", err) + } + releaseCell, err := WriteRecord(tempDir, release, map[string]string{"org.flatpak.ref": release.Ref}) + if err != nil { + t.Fatalf("failed to write 2.54 record: %v", err) + } + + if stableCell == releaseCell { + t.Fatalf("expected distinct cells for two branches of one app and arch, got %q for both", stableCell) + } + + records, err := IterRecords(tempDir) + if err != nil { + t.Fatalf("failed to iter records: %v", err) + } + if len(records) != 2 { + t.Fatalf("expected 2 records, got %d", len(records)) + } + + digests := map[string]string{} + for _, rwl := range records { + digests[rwl.Record.Branch] = rwl.Record.Digest + } + if digests["master"] != stable.Digest { + t.Errorf("master record digest = %q, want %q", digests["master"], stable.Digest) + } + if digests["2.54"] != release.Digest { + t.Errorf("2.54 record digest = %q, want %q", digests["2.54"], release.Digest) + } +} diff --git a/tests/push_autodetect_test.go b/tests/push_autodetect_test.go index 43ea003..4b50d2b 100644 --- a/tests/push_autodetect_test.go +++ b/tests/push_autodetect_test.go @@ -197,8 +197,8 @@ func TestPushOCIRefAutoDetection(t *testing.T) { } // Verify that records exist for both resolved targets - rec1 := filepath.Join(recordsDir, "org.example.App1-x86_64", "record.json") - rec2 := filepath.Join(recordsDir, "org.example.App2-x86_64", "record.json") + rec1 := filepath.Join(recordsDir, "org.example.App1-stable-x86_64", "record.json") + rec2 := filepath.Join(recordsDir, "org.example.App2-beta-x86_64", "record.json") if _, err := os.Stat(rec1); os.IsNotExist(err) { t.Errorf("Expected record for App1 to exist: %s", rec1) @@ -288,8 +288,8 @@ apps: } // Verify only RepoApp was pushed - recRepoApp := filepath.Join(recordsDir, "org.example.RepoApp-x86_64", "record.json") - recConfigApp := filepath.Join(recordsDir, "org.example.ConfigApp-x86_64", "record.json") + recRepoApp := filepath.Join(recordsDir, "org.example.RepoApp-stable-x86_64", "record.json") + recConfigApp := filepath.Join(recordsDir, "org.example.ConfigApp-stable-x86_64", "record.json") if _, err := os.Stat(recRepoApp); os.IsNotExist(err) { t.Errorf("Expected RepoApp to be pushed and record to exist: %s", recRepoApp) @@ -387,8 +387,9 @@ apps: } // Verify only aarch64 was pushed - recAarch64 := filepath.Join(recordsDir, "org.example.App1-aarch64", "record.json") - recX86_64 := filepath.Join(recordsDir, "org.example.App1-x86_64", "record.json") + recAarch64 := filepath.Join(recordsDir, "org.example.App1-stable-aarch64", "record.json") + recX86_64 := filepath.Join(recordsDir, "org.example.App1-stable-x86_64", "record.json") + recX86_64Beta := filepath.Join(recordsDir, "org.example.App1-beta-x86_64", "record.json") if _, err := os.Stat(recAarch64); os.IsNotExist(err) { t.Errorf("Expected aarch64 record to exist: %s", recAarch64) @@ -396,6 +397,9 @@ apps: if _, err := os.Stat(recX86_64); err == nil { t.Errorf("Expected x86_64 record NOT to exist: %s", recX86_64) } + if _, err := os.Stat(recX86_64Beta); err == nil { + t.Errorf("Expected x86_64 beta record NOT to exist: %s", recX86_64Beta) + } } // Test Scenario 3b: Filter by branch --branch beta @@ -418,8 +422,8 @@ apps: } // Verify only beta was pushed (which is on x86_64) - recBeta := filepath.Join(recordsDir, "org.example.App1-x86_64", "record.json") - recAarch64 := filepath.Join(recordsDir, "org.example.App1-aarch64", "record.json") + recBeta := filepath.Join(recordsDir, "org.example.App1-beta-x86_64", "record.json") + recAarch64 := filepath.Join(recordsDir, "org.example.App1-stable-aarch64", "record.json") if _, err := os.Stat(recBeta); os.IsNotExist(err) { t.Errorf("Expected x86_64 (beta) record to exist: %s", recBeta) @@ -450,8 +454,8 @@ apps: } // Verify only aarch64 stable was pushed - recAarch64 := filepath.Join(recordsDir, "org.example.App1-aarch64", "record.json") - recX86_64 := filepath.Join(recordsDir, "org.example.App1-x86_64", "record.json") + recAarch64 := filepath.Join(recordsDir, "org.example.App1-stable-aarch64", "record.json") + recX86_64 := filepath.Join(recordsDir, "org.example.App1-stable-x86_64", "record.json") if _, err := os.Stat(recAarch64); os.IsNotExist(err) { t.Errorf("Expected aarch64 record to exist: %s", recAarch64) @@ -495,7 +499,7 @@ apps: } // Verify that a record exists under the actual built branch 'stable' rather than 'beta' - rec := filepath.Join(recordsDir, "org.example.AppFallback-x86_64", "record.json") + rec := filepath.Join(recordsDir, "org.example.AppFallback-stable-x86_64", "record.json") if _, err := os.Stat(rec); os.IsNotExist(err) { t.Fatalf("Expected record for AppFallback to exist: %s", rec) } @@ -540,7 +544,7 @@ apps: } // Verify record exists and has runtime ref - rec := filepath.Join(recordsDir, "org.freedesktop.Sdk.Extension.mock-x86_64", "record.json") + rec := filepath.Join(recordsDir, "org.freedesktop.Sdk.Extension.mock-stable-x86_64", "record.json") if _, err := os.Stat(rec); os.IsNotExist(err) { t.Fatalf("Expected record for runtime to exist: %s", rec) } From dea531b3047d4dd69462c56e3351eaad905d51c1 Mon Sep 17 00:00:00 2001 From: Patrick Griffis Date: Mon, 7 Sep 2026 13:27:43 -0500 Subject: [PATCH 2/2] fix(oci): keep dots out of the whole image tag The app ID was stripped of dots to avoid signature tag strip mismatches, but the branch was interpolated raw, so a branch like 2.54 produced a tag flatpak cannot parse: its docker-reference tag pattern omits '.', so stripping the tag off the signature identity leaves it intact and the image is rejected as unsigned. --- pkg/oci/oci.go | 8 ++++---- pkg/oci/oci_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/pkg/oci/oci.go b/pkg/oci/oci.go index c4a1e0b..3a0b237 100644 --- a/pkg/oci/oci.go +++ b/pkg/oci/oci.go @@ -65,10 +65,10 @@ func Push(opts PushOptions) (PushResult, error) { } logger.Info("Pushing application to OCI: %s/%s (arch: %s, branch: %s)", opts.Registry, opts.OCIRepository, opts.Arch, opts.Branch) - // Format OCI tag as --, converting '.' to '_' in app-id to prevent - // signature tag strip mismatch problems (see architectural invariants). - safeAppID := strings.ReplaceAll(opts.AppID, ".", "_") - tag := fmt.Sprintf("%s-%s-%s", safeAppID, opts.Branch, opts.Arch) + // Format OCI tag as --, converting '.' to '_' throughout + // to prevent signature tag strip mismatch problems (see architectural + // invariants). A branch like 2.54 needs this as much as the app ID does. + tag := CleanTag(fmt.Sprintf("%s-%s-%s", opts.AppID, opts.Branch, opts.Arch)) logger.Debug("Target OCI image tag resolved to: %s", tag) refType := opts.RefType diff --git a/pkg/oci/oci_test.go b/pkg/oci/oci_test.go index c36b6f9..b683b06 100644 --- a/pkg/oci/oci_test.go +++ b/pkg/oci/oci_test.go @@ -203,6 +203,31 @@ func TestCleanTag(t *testing.T) { } } +// A dot anywhere in the tag makes flatpak's docker-reference parser fail to +// strip it, and the signature identity check then rejects the image. +func TestTagHasNoDots(t *testing.T) { + tests := []struct { + appID string + branch string + arch string + expected string + }{ + {"org.example.App", "master", "x86_64", "org_example_App-master-x86_64"}, + {"org.example.App", "2.54", "x86_64", "org_example_App-2_54-x86_64"}, + {"org.example.App.Extension", "2.54", "aarch64", "org_example_App_Extension-2_54-aarch64"}, + } + + for _, tt := range tests { + actual := CleanTag(fmt.Sprintf("%s-%s-%s", tt.appID, tt.branch, tt.arch)) + if actual != tt.expected { + t.Errorf("tag for %s//%s (%s) = %q; expected %q", tt.appID, tt.branch, tt.arch, actual, tt.expected) + } + if strings.Contains(actual, ".") { + t.Errorf("tag %q still contains a dot", actual) + } + } +} + func TestPushSigned(t *testing.T) { // Generate GPG Key entity, err := openpgp.NewEntity("AetherPak Test", "Test key", "test@aetherpak.local", nil)