Conversation
Publishing two branches of one app and arch in a single execution had both pushes write into <records-dir>/<app-id>-<arch>, 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.
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.
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
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.
Problem
Pushing two branches of the same app and arch in one execution loses one of them from the published index.
CellDirbuilds the record path from the app ID and arch only:so both pushes write into
<records-dir>/<app-id>-<arch>and the secondrecord.jsonoverwrites the first.build-sitemerges from those cells, so only the last branch pushed reaches the static index.Observed while publishing a
masterand a2.54branch of two apps in one run — five pushes, three cells:The failure is quiet and quite confusing from the outside: both branches are exported, signed, and pushed to the registry under correct, distinct tags (
…-2.54-x86_64and…-master-x86_64), and every step reports success — but only-masterentries appear inindex/staticand only-master.flatpakreffiles get generated, so the other branch cannot be installed even though its images are present.Fix
Put the branch in the cell path, matching the OCI tag in
pkg/oci/oci.go, which already distinguishes branches as<app-id>-<branch>-<arch>.Two existing properties keep the change contained:
IterRecordslocates cells by walking forrecord.json/labels.jsonand reads the branch from the JSON, so it never parsed the directory name.mergeRecordkeys index images byorg.flatpak.ref+ architecture, and the ref already carries the branch, so nothing downstream re-collapses the two.A record with no branch keeps the old path, so callers that never set one are unaffected and previously written cells still load.
Since the branch becomes a path segment,
Validatenow checks it againstbranchRegexpwhen set — previously a branch like../escapedwas unvalidated while app ID and arch were.Testing
make fmt,make vet(also with-tags=integration) andmake testare clean.New
TestWriteRecordSeparatesBranchescovers two branches of one app and arch. With theCellDirchange reverted it fails, so it pins the regression:Added a
Validatecase for a path-traversal branch.Updated the record paths asserted in
tests/push_autodetect_test.goto the new layout, derived from each scenario's mock branch. Scenario 3a's singleApp1-x86_64assertion previously covered both x86_64 branches, so itsbetacell is now asserted explicitly rather than losing that coverage.ARCHITECTURE.mddocumented the old layout and now describes both it and the fallback.Second problem: a dotted branch makes the image unsignable
Once both branches reach the index, installing the one whose branch contains a dot fails:
The tag is built as
<app-id>-<branch>-<arch>with dots stripped from the app ID, and the comment there says why — signature tag strip mismatches — but the branch is interpolated raw:flatpak strips the tag off a signature's
docker-referencewith^(.*?)(?::TAG)?(?:@DIGEST)?$, where itsTAGpattern is[0-9A-Za-z_][0-9A-Za-z_-]{0,127}— no dot, though OCI tags allow one. A dotted tag therefore can't match the optional tag group, the lazy first group swallows the whole reference, and the identity check compares the fullregistry/repo:tagagainstregistry/repo:org_example_App-master-x86_64ghcr.io/org/repo✓org_example_App-2.54-x86_64ghcr.io/org/repo:org_example_App-2.54-x86_64✗So the existing dot handling was right, just not applied widely enough.
CleanTagalready exists for this and covers the app ID too, so the tag is now built through it, givingorg_example_App-2_54-x86_64. Tags without dots — every current one — are byte-identical to before.TestTagHasNoDotscovers the three shapes, including a dotted app ID with a dotted branch.Fixing it here rather than in flatpak keeps published repositories installable with released flatpak versions.