From c8624a26078c313cd88e4ad0c7854053490f80e1 Mon Sep 17 00:00:00 2001 From: Wynne Pirini Date: Fri, 16 Oct 2026 16:44:02 +1300 Subject: [PATCH] fix(ns8): normalise the registry path case on the fetch, not just the token d5cc8ee fixed this defect's token half and left the fetch half, which is why main has been red on every push since. The commit message was accurate about what it did ("lowercase the repo in assert-multiarc's token scope"), the gap was in reading it as the whole fix. The two endpoints disagree about case, and that is what hid it: token() lowercased the scope, so it asked for permission correctly get() built the manifest URL from the raw repo, so it asked the wrong URL ghcr's token endpoint wants a lowercase scope and answers HTTP 400 otherwise. ghcr's MANIFEST endpoint is stricter still: an uppercase path is simply a repository that does not exist, so it answers HTTP 404 for the very image whose lowercase path resolves. Lowercasing inside token() therefore made the release look fixed while the fetch kept failing. Measured against the real registry, same image, two spellings: ghcr.io/calnode/ns8-calnode:edge PASS ghcr.io/Calnode/ns8-calnode:edge HTTP 404 ghcr.io/calnode/ns8-calnode:latest PASS ghcr.io/Calnode/ns8-calnode:latest HTTP 404 ghcr.io/calnode/ns8-calnode:0.11.1 PASS ghcr.io/Calnode/ns8-calnode:0.11.1 HTTP 404 ns8-module.yml passes `ghcr.io/${{ github.repository_owner }}/ns8-calnode`, and github.repository_owner is `Calnode` verbatim. So every push to main and dev pushed :edge and then asserted a URL that cannot exist. Confirming that is the live failure and not something I introduced: failure 5b4acef push Merge pull request #151 failure d5cc8ee push lowercase the repo in token scope failure 3995484 push v0.11.1 failure 3a4ea7c push Merge pull request #150 failure 212f779 push Merge pull request #149 success 0f721a4 push Merge pull request #148 (assertion did not exist yet) #149 introduced the registry assertion and every push to main has been red since. Fixed with one normaliser applied where a registry path becomes a URL, so no caller can be missed and neither endpoint can drift again. The fix deliberately lives in the script rather than in the workflow's env: the caller passing `Calnode` is not the bug, and patching the env would leave every other caller and the app-repo path exposed to the same slip. python3 assert-multiarc.py ghcr.io/Calnode/ns8-calnode edge latest 0.11.1 PASS all 3 tag(s) are multi-arch with coherent labels Also corrects the docstring, which overstated the case against an app-repo override. An override used as a SUBSTITUTE for the label is a footgun and is why there is no such option today. An override used as an ASSERTION is the opposite: it would be strictly stronger, and --expect-app-repo would catch a module labelled with a foreign app repository, which --expect-app-tag cannot since a tag is not a repository. That is a real future need for a fork publishing under its own namespace. The docstring now says so and records that the distinction to preserve is which of the two it is, not whether to have one, instead of forbidding the flag outright. Full proof set after the change: uppercase repo, real tags exit 0 (was the red-main case) lowercase repo, real tags exit 0 (no regression) dead --app-repo flag exit 2, still rejected --pair --expect-app-tag 0.11.1 exit 0 genuinely missing app image FAILED in 2.2s, HTTP 404 mismatched pair exit 1, mismatch reported module-only contacts no app repo confirmed, still true gate logic from the shipped YAML 5/5 correct both workflows parse OK --- packaging/ns8/assert-multiarc.py | 58 +++++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 12 deletions(-) diff --git a/packaging/ns8/assert-multiarc.py b/packaging/ns8/assert-multiarc.py index 533e7f8..2f27650 100644 --- a/packaging/ns8/assert-multiarc.py +++ b/packaging/ns8/assert-multiarc.py @@ -25,13 +25,29 @@ --expect-app-tag T assert every label names exactly this app tag, so the module release and the app release are the same version -There is deliberately NO option to override the app repository. The app image a -module runs is whatever its org.nethserver.images label names, so the label is -the only authoritative source and it is also what a fork needs: a module built -under someone else's namespace names their app image, and deriving from the -label handles that with no configuration. An override here would be worse than -useless, because it would let a pair check be asserted against an app repository -the module does not actually reference, hiding a mislabelled module. +There is deliberately no option to override the app repository, and the reason +is narrower than "an override is always wrong". + +The app image a module runs is whatever its org.nethserver.images label names, +so the label is the only authoritative source. Deriving from it also needs no +configuration for a fork: a module built under another namespace names their app +image, and reading the label handles that on its own. + +So an override used as a SUBSTITUTE for the label is a footgun. It would assert +the pair against an app repository the module does not actually reference, which +hides a mislabelled module instead of catching it. That is the same hole +--expect-app-tag exists to close, one level up. + +An override used as an ASSERTION instead would be strictly stronger, not weaker, +and is a real future need. Today --expect-app-tag catches a wrong app TAG, but a +module labelled `ghcr.io/someoneelse/calnode:0.11.1` still passes as long as that +image exists and is multi-arch. A `--expect-app-repo` would fail unless the label +names exactly the repository AND tag this release publishes, which is what a fork +publishing under its own namespace wants and cannot express from the label alone. + +We do not need it until someone maintains such a fork. Until then, adding it as a +substitute would be the bug; adding it as an assertion would be the fix. The +distinction to preserve is which of the two it is, not whether to have it. Exit status is 0 only if every tag passes every applicable check. @@ -72,12 +88,28 @@ class Fail(Exception): """A check failed in a way worth reporting verbatim.""" +def normalize_repo(repo): + """Registry repository paths are case-sensitive and must be lowercase. + + ghcr's TOKEN endpoint wants a lowercase scope and answers HTTP 400 + otherwise. Its MANIFEST endpoint is stricter: an uppercase path is simply a + repository that does not exist, so it answers HTTP 404 while the lowercase + path of the very same image resolves. + + That combination is what made this invisible. The owner arrives from GitHub + as `github.repository_owner` verbatim, which is `Calnode` for this repo, so + ns8-module.yml passes `ghcr.io/Calnode/ns8-calnode`. Lowercasing inside + token() alone fixed the scope request and left the fetch 404ing, so the + release looked fixed while every push to main and dev stayed red. + + Normalise at the single point where a registry path becomes a URL, so every + caller is covered and neither endpoint can be missed again. + """ + return repo.lower() + + def token(repo): - # ghcr's token endpoint requires the repository path LOWERCASE and answers - # HTTP 400 otherwise. The owner arrives from GitHub as `github.repository_owner` - # verbatim, which is `Calnode` for this repo, so normalise here rather than - # trusting every caller to remember: a case slip here fails a correct release. - repo = repo.lower() + repo = normalize_repo(repo) if repo not in _tokens: url = f"{REGISTRY}/token?scope=repository:{repo}:pull&service=ghcr.io" with urllib.request.urlopen(url, timeout=30) as f: @@ -87,6 +119,7 @@ def token(repo): def get(repo, reference): """GET a manifest, returning (body, mediaType).""" + repo = normalize_repo(repo) req = urllib.request.Request( f"{REGISTRY}/v2/{repo}/manifests/{reference}", headers={"Authorization": f"Bearer {token(repo)}", "Accept": ACCEPT}, @@ -96,6 +129,7 @@ def get(repo, reference): def get_blob(repo, digest): + repo = normalize_repo(repo) req = urllib.request.Request( f"{REGISTRY}/v2/{repo}/blobs/{digest}", headers={"Authorization": f"Bearer {token(repo)}"},