From 149e14237dabad8b37a5105bd97bdd0d318a2b53 Mon Sep 17 00:00:00 2001 From: chodeus <190988615+chodeus@users.noreply.github.com> Date: Sat, 22 Aug 2026 18:17:26 +0800 Subject: [PATCH 1/3] fix(version): check for updates against the published image, not commit count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The update check compared the build number baked into the running image against the number of commits on main. Those are not the same thing: a commit that touches no image path — docs, workflows, and more of them since #581 stopped needless rebuilds — advances the commit count while publishing nothing. Every instance was then told an update was ready that it could never pull. Measured before the fix: :full carried BUILD_NUMBER=1550 while main stood at 1551 commits, and _check_remote_version returned update_available=True. It now asks the registry what actually exists — an anonymous GHCR read of the rolling tag this container tracks (:full when CHUB_IMAGE_FLAVOR=full, else :latest), pulling BUILD_NUMBER back out of the image config. Any failure returns None and claims no update, so an unreachable registry cannot raise a badge. --- backend/util/version.py | 96 +++++++++++++++++++++++++++++++++------- tests/test_version.py | 97 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+), 16 deletions(-) diff --git a/backend/util/version.py b/backend/util/version.py index f8bb8053..102e329d 100755 --- a/backend/util/version.py +++ b/backend/util/version.py @@ -44,6 +44,84 @@ def get_version() -> str: return base_version +# ----- what is actually published ------------------------------------------- +GHCR_IMAGE = "chodeus/chub" +_GHCR_ACCEPT = ",".join( + ( + "application/vnd.oci.image.index.v1+json", + "application/vnd.docker.distribution.manifest.list.v2+json", + "application/vnd.oci.image.manifest.v1+json", + ) +) + + +def _image_tag() -> str: + """Rolling tag this container tracks: :full when the extensions are baked in.""" + return "full" if os.getenv("CHUB_IMAGE_FLAVOR") == "full" else "latest" + + +def _published_build(logger, tag: str | None = None) -> int | None: + """BUILD_NUMBER baked into the newest published image, or None if unknown. + + Asks the registry what exists rather than counting commits on main: a commit + that changes no image path (docs, workflows) publishes nothing, and comparing + against the commit count told every user an update was ready that they could + never pull. None on any failure — an unknown remote must never raise a badge. + """ + tag = tag or _image_tag() + try: + tok = requests.get( + f"https://ghcr.io/token?scope=repository:{GHCR_IMAGE}:pull&service=ghcr.io", + timeout=5, + ) + if not tok.ok: + logger.debug(f"GHCR token failed: {tok.status_code}") + return None + head = { + "Authorization": f"Bearer {tok.json()['token']}", + "Accept": _GHCR_ACCEPT, + } + man = requests.get( + f"https://ghcr.io/v2/{GHCR_IMAGE}/manifests/{tag}", headers=head, timeout=5 + ) + if not man.ok: + logger.debug(f"GHCR manifest {tag} failed: {man.status_code}") + return None + doc = man.json() + if "manifests" in doc: # multi-arch index; BUILD_NUMBER is per-build, not per-arch + children = [ + m + for m in doc["manifests"] + if m.get("platform", {}).get("architecture") not in (None, "unknown") + ] + if not children: + return None + man = requests.get( + f"https://ghcr.io/v2/{GHCR_IMAGE}/manifests/{children[0]['digest']}", + headers=head, + timeout=5, + ) + if not man.ok: + logger.debug(f"GHCR child manifest failed: {man.status_code}") + return None + doc = man.json() + blob = requests.get( + f"https://ghcr.io/v2/{GHCR_IMAGE}/blobs/{doc['config']['digest']}", + headers=head, + timeout=5, + ) + if not blob.ok: + logger.debug(f"GHCR config blob failed: {blob.status_code}") + return None + for entry in blob.json().get("config", {}).get("Env", []): + if entry.startswith("BUILD_NUMBER="): + return int(entry.split("=", 1)[1]) + return None + except Exception as exc: + logger.debug(f"Exception reading the published image: {exc}") + return None + + def _check_remote_version(local_version, branch, logger): raw_url = f"https://raw.githubusercontent.com/chodeus/chub/{branch}/.release-please-manifest.json" @@ -59,22 +137,8 @@ def _check_remote_version(local_version, branch, logger): logger.debug(f"Exception fetching manifest: {e}") return None, None, False - api_url = ( - f"https://api.github.com/repos/chodeus/chub/commits?sha={branch}&per_page=1" - ) - try: - resp = requests.get(api_url, timeout=5) - if not resp.ok: - logger.debug(f"Could not fetch commit count: {resp.status_code}") - return remote_version_str, None, False - link = resp.headers.get("Link") - if not link: - build_count = 1 - else: - match = re.search(r"&page=(\d+)>; rel=\"last\"", link) - build_count = int(match.group(1)) if match else 1 - except Exception as e: - logger.debug(f"Exception fetching build count: {e}") + build_count = _published_build(logger) + if build_count is None: return remote_version_str, None, False remote_full = f"{remote_version_str}.{branch}{build_count}" diff --git a/tests/test_version.py b/tests/test_version.py index f606123b..ca8cec26 100644 --- a/tests/test_version.py +++ b/tests/test_version.py @@ -1,8 +1,13 @@ """Tests for backend/util/version.py — manifest-driven versioning.""" +import json import subprocess from unittest.mock import MagicMock, patch +import pytest + +import backend.util.version as version_mod + from backend.util.version import _read_base_version, check_for_update, get_version @@ -69,3 +74,95 @@ def test_check_for_update_handles_network_failure(): assert result["update_available"] is False assert result["checked"] is False assert result["branch"] == "develop" + + +# ----- update check compares against what is PUBLISHED, not commits on main ---- + + +class _Resp: + def __init__(self, payload=None, ok=True, status=200, text=None): + self._payload, self.ok, self.status_code = payload, ok, status + self.text = text if text is not None else json.dumps(payload or {}) + + def json(self): + return self._payload + + +def _ghcr(build_number="1550", **over): + """URL -> response for a healthy GHCR read; `over` replaces one leg.""" + routes = { + "token": _Resp({"token": "t"}), + "manifests/full": _Resp({"manifests": [ + {"digest": "sha256:child", "platform": {"architecture": "amd64"}}]}), + "manifests/latest": _Resp({"manifests": [ + {"digest": "sha256:child", "platform": {"architecture": "amd64"}}]}), + "manifests/sha256:child": _Resp({"config": {"digest": "sha256:cfg"}}), + "blobs/": _Resp({"config": {"Env": ["PATH=/usr/bin", f"BUILD_NUMBER={build_number}"]}}), + } + routes.update(over) + + def get(url, **kwargs): + for key, resp in routes.items(): + if key in url: + return resp + raise AssertionError(f"unexpected URL {url}") + + return get + + +def test_published_build_reads_the_image_env(monkeypatch): + monkeypatch.setattr(version_mod.requests, "get", _ghcr("1556")) + assert version_mod._published_build(MagicMock()) == 1556 + + +@pytest.mark.parametrize("leg", ["token", "manifests/latest", "blobs/"]) +def test_published_build_is_none_when_the_registry_fails(leg, monkeypatch): + """Unknown must never raise an update badge.""" + monkeypatch.setattr( + version_mod.requests, "get", _ghcr(**{leg: _Resp(ok=False, status=503)}) + ) + assert version_mod._published_build(MagicMock()) is None + + +def test_image_tag_follows_the_flavour(monkeypatch): + monkeypatch.setenv("CHUB_IMAGE_FLAVOR", "full") + assert version_mod._image_tag() == "full" + monkeypatch.setenv("CHUB_IMAGE_FLAVOR", "lean") + assert version_mod._image_tag() == "latest" + monkeypatch.delenv("CHUB_IMAGE_FLAVOR", raising=False) + assert version_mod._image_tag() == "latest" + + +def test_commits_that_publish_no_image_do_not_raise_an_update(monkeypatch): + """The bug this replaced: a docs/CI-only commit bumps the commit count on main + but publishes nothing, so every user was told an update was ready that did not + exist. The comparison must read the registry and never the commits API.""" + seen = [] + + def get(url, **kwargs): + seen.append(url) + if "raw.githubusercontent.com" in url: + return _Resp(text=json.dumps({".": "2.48.0"})) + return _ghcr("1556")(url, **kwargs) + + monkeypatch.setattr(version_mod.requests, "get", get) + remote, build, update = version_mod._check_remote_version( + "2.48.0.main1556", "main", MagicMock() + ) + + assert build == 1556 + assert update is False, "running the newest published image is not an update" + assert not any("api.github.com" in u for u in seen), "must not count commits" + + +def test_a_newer_published_image_is_an_update(monkeypatch): + def get(url, **kwargs): + if "raw.githubusercontent.com" in url: + return _Resp(text=json.dumps({".": "2.48.0"})) + return _ghcr("1560")(url, **kwargs) + + monkeypatch.setattr(version_mod.requests, "get", get) + _remote, build, update = version_mod._check_remote_version( + "2.48.0.main1556", "main", MagicMock() + ) + assert (build, update) == (1560, True) From f6a78869b86a9cebf41d43c3fd50cc57d0398f24 Mon Sep 17 00:00:00 2001 From: chodeus <190988615+chodeus@users.noreply.github.com> Date: Sat, 22 Aug 2026 18:43:00 +0800 Subject: [PATCH 2/3] fix(tests): parse the host instead of substring-matching it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL on the new tests, four alerts in one round: 335/336/337 (incomplete URL substring sanitization) — the mock router keyed off `"raw.githubusercontent.com" in url`. Harmless in a fixture, but it is the bypassable shape the query exists to catch (a path or query can carry the host string), and leaving it teaches the pattern. Now compares urlparse().netloc. 338 (module imported with both `import` and `import from`) — the new `import backend.util.version as version_mod` sat beside the existing `from` import purely to reach `requests` for monkeypatching. String targets do that without the module object, so the second import form is gone. Also trims _published_build's docstring to the comment cap, per CodeRabbit and the path instructions — the rationale for reading the registry lives in the PR body, not the module. --- backend/util/version.py | 7 ++---- tests/test_version.py | 47 +++++++++++++++++++++++++---------------- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/backend/util/version.py b/backend/util/version.py index 102e329d..c2def4f2 100755 --- a/backend/util/version.py +++ b/backend/util/version.py @@ -61,12 +61,9 @@ def _image_tag() -> str: def _published_build(logger, tag: str | None = None) -> int | None: - """BUILD_NUMBER baked into the newest published image, or None if unknown. + """BUILD_NUMBER of the newest published image, or None if unknown. - Asks the registry what exists rather than counting commits on main: a commit - that changes no image path (docs, workflows) publishes nothing, and comparing - against the commit count told every user an update was ready that they could - never pull. None on any failure — an unknown remote must never raise a badge. + Every failure returns None, so an unreachable registry never raises a badge. """ tag = tag or _image_tag() try: diff --git a/tests/test_version.py b/tests/test_version.py index ca8cec26..9b72a164 100644 --- a/tests/test_version.py +++ b/tests/test_version.py @@ -3,12 +3,18 @@ import json import subprocess from unittest.mock import MagicMock, patch +from urllib.parse import urlparse import pytest -import backend.util.version as version_mod - -from backend.util.version import _read_base_version, check_for_update, get_version +from backend.util.version import ( + _check_remote_version, + _image_tag, + _published_build, + _read_base_version, + check_for_update, + get_version, +) def test_read_base_version_returns_string(): @@ -88,6 +94,11 @@ def json(self): return self._payload +def _host(url): + """Parsed host — a substring test would match an attacker-controlled path.""" + return urlparse(url).netloc + + def _ghcr(build_number="1550", **over): """URL -> response for a healthy GHCR read; `over` replaces one leg.""" routes = { @@ -111,26 +122,26 @@ def get(url, **kwargs): def test_published_build_reads_the_image_env(monkeypatch): - monkeypatch.setattr(version_mod.requests, "get", _ghcr("1556")) - assert version_mod._published_build(MagicMock()) == 1556 + monkeypatch.setattr("backend.util.version.requests.get", _ghcr("1556")) + assert _published_build(MagicMock()) == 1556 @pytest.mark.parametrize("leg", ["token", "manifests/latest", "blobs/"]) def test_published_build_is_none_when_the_registry_fails(leg, monkeypatch): """Unknown must never raise an update badge.""" monkeypatch.setattr( - version_mod.requests, "get", _ghcr(**{leg: _Resp(ok=False, status=503)}) + "backend.util.version.requests.get", _ghcr(**{leg: _Resp(ok=False, status=503)}) ) - assert version_mod._published_build(MagicMock()) is None + assert _published_build(MagicMock()) is None def test_image_tag_follows_the_flavour(monkeypatch): monkeypatch.setenv("CHUB_IMAGE_FLAVOR", "full") - assert version_mod._image_tag() == "full" + assert _image_tag() == "full" monkeypatch.setenv("CHUB_IMAGE_FLAVOR", "lean") - assert version_mod._image_tag() == "latest" + assert _image_tag() == "latest" monkeypatch.delenv("CHUB_IMAGE_FLAVOR", raising=False) - assert version_mod._image_tag() == "latest" + assert _image_tag() == "latest" def test_commits_that_publish_no_image_do_not_raise_an_update(monkeypatch): @@ -140,29 +151,29 @@ def test_commits_that_publish_no_image_do_not_raise_an_update(monkeypatch): seen = [] def get(url, **kwargs): - seen.append(url) - if "raw.githubusercontent.com" in url: + seen.append(_host(url)) + if _host(url) == "raw.githubusercontent.com": return _Resp(text=json.dumps({".": "2.48.0"})) return _ghcr("1556")(url, **kwargs) - monkeypatch.setattr(version_mod.requests, "get", get) - remote, build, update = version_mod._check_remote_version( + monkeypatch.setattr("backend.util.version.requests.get", get) + _remote, build, update = _check_remote_version( "2.48.0.main1556", "main", MagicMock() ) assert build == 1556 assert update is False, "running the newest published image is not an update" - assert not any("api.github.com" in u for u in seen), "must not count commits" + assert "api.github.com" not in seen, "must not count commits" def test_a_newer_published_image_is_an_update(monkeypatch): def get(url, **kwargs): - if "raw.githubusercontent.com" in url: + if _host(url) == "raw.githubusercontent.com": return _Resp(text=json.dumps({".": "2.48.0"})) return _ghcr("1560")(url, **kwargs) - monkeypatch.setattr(version_mod.requests, "get", get) - _remote, build, update = version_mod._check_remote_version( + monkeypatch.setattr("backend.util.version.requests.get", get) + _remote, build, update = _check_remote_version( "2.48.0.main1556", "main", MagicMock() ) assert (build, update) == (1560, True) From 038c7ce4e26eb00ef31f2f2f29deaff888a91b52 Mon Sep 17 00:00:00 2001 From: chodeus <190988615+chodeus@users.noreply.github.com> Date: Sat, 22 Aug 2026 19:39:03 +0800 Subject: [PATCH 3/3] test(version): assert which tag the update check actually asks for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit, and it was right: _ghcr() routed both manifests/full and manifests/latest, so the test passed whichever tag _published_build requested. Verified by mutation — hardcoding tag="latest", which makes every :full instance compare itself against the lean image, passed all 13 tests. Now parameterized over full / lean / unset, asserting the requested manifest path rather than only the number that comes back. The same mutant now fails. --- tests/test_version.py | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/tests/test_version.py b/tests/test_version.py index 9b72a164..4bc3782f 100644 --- a/tests/test_version.py +++ b/tests/test_version.py @@ -8,6 +8,7 @@ import pytest from backend.util.version import ( + GHCR_IMAGE, _check_remote_version, _image_tag, _published_build, @@ -121,9 +122,28 @@ def get(url, **kwargs): return get -def test_published_build_reads_the_image_env(monkeypatch): - monkeypatch.setattr("backend.util.version.requests.get", _ghcr("1556")) +@pytest.mark.parametrize( + "flavour,tag", [("full", "full"), ("lean", "latest"), (None, "latest")] +) +def test_published_build_reads_the_env_of_the_tag_its_flavour_tracks( + monkeypatch, flavour, tag +): + """A :full instance compared against :latest would read the wrong build, so + the requested tag is asserted — not just the number that comes back.""" + if flavour is None: + monkeypatch.delenv("CHUB_IMAGE_FLAVOR", raising=False) + else: + monkeypatch.setenv("CHUB_IMAGE_FLAVOR", flavour) + seen, routed = [], _ghcr("1556") + + def get(url, **kwargs): + seen.append(urlparse(url).path) + return routed(url, **kwargs) + + monkeypatch.setattr("backend.util.version.requests.get", get) + assert _published_build(MagicMock()) == 1556 + assert f"/v2/{GHCR_IMAGE}/manifests/{tag}" in seen @pytest.mark.parametrize("leg", ["token", "manifests/latest", "blobs/"])