From dc93f02c41b8ee4b04f04799c8b5504bec9f40ab Mon Sep 17 00:00:00 2001 From: Viktor Petersson Date: Thu, 16 Jul 2026 14:03:16 +0000 Subject: [PATCH] fix(wizard): pin emitted action ref to the latest GitHub release SHA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The emitted workflow's `uses:` ref was built from the running build's __version__. Dev/Docker builds report a PEP 440 local version like 26.7.0+ad3dc1d, so the tag->SHA lookup 404'd and the fallback emitted the invalid ref sbomify/sbomify-action@v26.7.0+ad3dc1d — flagged by SonarCloud ("Use full commit SHA hash for this dependency"). Resolve the pin from GitHub's latest published release instead of assuming the built version exists as a tag, and only default to the build's version (with the +local segment stripped) when GitHub is unreachable. Co-Authored-By: Claude Fable 5 --- sbomify_action/cli/wizard/ci_emitter.py | 68 ++++++++++++++++++++++--- tests/conftest.py | 12 +++-- tests/test_wizard_emitter.py | 68 ++++++++++++++++++++++++- 3 files changed, 134 insertions(+), 14 deletions(-) diff --git a/sbomify_action/cli/wizard/ci_emitter.py b/sbomify_action/cli/wizard/ci_emitter.py index 7259fd6e..edc6a283 100644 --- a/sbomify_action/cli/wizard/ci_emitter.py +++ b/sbomify_action/cli/wizard/ci_emitter.py @@ -37,10 +37,13 @@ # The sbomify-action reference in emitted workflows is resolved at wizard run # time (see ``resolve_action_ref``) instead of hard-coded, so there's no -# per-release SHA to keep in sync. We ask GitHub for the commit behind *this* -# build's version tag and pin to @ — the security guarantee per -# SECURITY.md, since a tag rewrite then can't swap the action's code under the -# user. Offline, we fall back to a tag-pinned (non-SHA) @ ref. +# per-release SHA to keep in sync. We ask GitHub for the latest release — +# never assuming the running build's version is a published tag (dev/Docker +# builds carry local versions like ``26.7.0+ad3dc1d`` that don't exist as +# tags) — and pin to @, the security guarantee per SECURITY.md, since a +# tag rewrite then can't swap the action's code under the user. When GitHub +# is unreachable we default to this build's version (local part stripped) as +# a tag-pinned (non-SHA) @ ref. ACTION_REPO = "sbomify/sbomify-action" # Resolving the pin is best-effort and must never block the wizard for long; @@ -49,6 +52,11 @@ _SHA_RE = re.compile(r"[0-9a-f]{40}") +# Sanity check on a GitHub-reported release tag before it lands verbatim in +# an emitted YAML file: plain tag characters only (no whitespace, quotes, or +# comment markers). +_TAG_RE = re.compile(r"[A-Za-z0-9._\-]+") + # Third-party actions used by the emitted workflow. Pinned-by-SHA per # SECURITY.md; bump when consuming a known-good newer release. PINNED_CHECKOUT_SHA = "de0fac2e4500dabe0009e67214ff5f5447ce83dd" @@ -93,13 +101,47 @@ def _action_version() -> str: Prefers the running package's reported version; in a metadata-less dev checkout (``__version__ == "unknown"``) it reads ``project.version`` from ``pyproject.toml`` so there's no hard-coded version to drift. + + Dev/Docker builds report PEP 440 local versions like ``26.7.0+ad3dc1d``; + the ``+…`` local segment is build metadata, not part of any release tag, + so it's stripped — the result must name a ref that can exist on GitHub. """ version = (_PACKAGE_VERSION or "").strip() if not version or version == "unknown": version = (_version_from_pyproject() or "").strip() + version = version.split("+", 1)[0] return f"v{version}" if version else "unknown" +def _resolve_latest_release_tag() -> str | None: + """Tag name of the latest published release per GitHub, or ``None``. + + The wizard can't assume the running build's version exists as a tag + (dev and Docker builds carry local-version suffixes), so the emitted + pin is anchored to the latest actual release. Best-effort/never-raise, + like ``_resolve_tag_sha``. + """ + url = f"https://api.github.com/repos/{ACTION_REPO}/releases/latest" + try: + response = requests.get( + url, + timeout=_PIN_TIMEOUT, + headers={"Accept": "application/vnd.github+json"}, + ) + except requests.RequestException: + return None + if response.status_code != 200: + return None + try: + payload = response.json() + except ValueError: + return None + if not isinstance(payload, dict): + return None + tag = payload.get("tag_name", "") + return tag if isinstance(tag, str) and _TAG_RE.fullmatch(tag) else None + + def _resolve_tag_sha(version: str) -> str | None: """Commit SHA behind ``version`` per GitHub, or ``None`` on any failure. @@ -149,7 +191,14 @@ def _build_action_ref(version: str, sha: str | None) -> str: @functools.lru_cache(maxsize=1) def _resolve_action_ref_cached() -> str: - version = _action_version() + version = _resolve_latest_release_tag() + if not version: + version = _action_version() + logger.info( + "Could not resolve the latest %s release from GitHub; defaulting to this build's version %s.", + ACTION_REPO, + version, + ) sha = _resolve_tag_sha(version) if not sha: logger.info( @@ -165,10 +214,13 @@ def resolve_action_ref() -> str: Single-flight and cached for the process, so the review preview and the apply write share exactly one lookup and always pin the same ref even when - their worker threads call concurrently. Online: - ``sbomify/sbomify-action@ # ``. Offline (unreachable, + their worker threads call concurrently. Online: the latest published + release resolved to its commit, + ``sbomify/sbomify-action@ # `` — never the running + build's own version, which may not exist as a tag. Offline (unreachable, rate-limited, or tag missing): a tag-pinned - ``sbomify/sbomify-action@`` — still pinned, just no SHA. + ``sbomify/sbomify-action@`` defaulting to the version we built — + still pinned, just no SHA. """ with _RESOLVE_LOCK: return _resolve_action_ref_cached() diff --git a/tests/conftest.py b/tests/conftest.py index 6c943b0f..c9b223a7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,17 +7,19 @@ def offline_action_pin(monkeypatch): """Keep emitted sbomify-action pin resolution offline in all tests. - ``apply_plan`` and the review preview resolve the action's commit SHA - from GitHub at run time; without this every such test would make a real - network call. Stub only the network boundary (``_resolve_tag_sha``) so the - real ``resolve_action_ref`` still runs and falls back to the tag-pinned + ``apply_plan`` and the review preview resolve the latest release and its + commit SHA from GitHub at run time; without this every such test would + make real network calls. Stub only the network boundaries + (``_resolve_latest_release_tag`` and ``_resolve_tag_sha``) so the real + ``resolve_action_ref`` still runs and falls back to the tag-pinned (offline) ref. The lru_cache is cleared around each test so the stubbed result never bleeds across tests. Tests exercising the online path - re-stub ``_resolve_tag_sha`` themselves after clearing the cache. + re-stub these themselves after clearing the cache. """ from sbomify_action.cli.wizard import ci_emitter ci_emitter.resolve_action_ref.cache_clear() + monkeypatch.setattr(ci_emitter, "_resolve_latest_release_tag", lambda: None) monkeypatch.setattr(ci_emitter, "_resolve_tag_sha", lambda version: None) yield ci_emitter.resolve_action_ref.cache_clear() diff --git a/tests/test_wizard_emitter.py b/tests/test_wizard_emitter.py index 9674c76f..7f6195a5 100644 --- a/tests/test_wizard_emitter.py +++ b/tests/test_wizard_emitter.py @@ -14,6 +14,7 @@ HEADER_SENTINEL, _action_version, _build_action_ref, + _resolve_latest_release_tag, _resolve_tag_sha, emit_workflow, ) @@ -398,7 +399,71 @@ def test_action_version_prefixes_v(monkeypatch) -> None: assert _action_version() == "v26.2.0" +def test_action_version_strips_local_version_segment(monkeypatch) -> None: + # Dev/Docker builds report PEP 440 local versions like 26.7.0+ad3dc1d. + # The +… build metadata is not part of any release tag and must never + # reach an emitted `uses:` ref. + monkeypatch.setattr(ci_emitter, "_PACKAGE_VERSION", "26.7.0+ad3dc1d") + assert _action_version() == "v26.7.0" + + +def test_resolve_latest_release_tag_online(mocker) -> None: + response = MagicMock(status_code=200) + response.json.return_value = {"tag_name": "v27.1.0"} + mocker.patch.object(ci_emitter.requests, "get", return_value=response) + assert _resolve_latest_release_tag() == "v27.1.0" + + +def test_resolve_latest_release_tag_offline_returns_none(mocker) -> None: + mocker.patch.object(ci_emitter.requests, "get", side_effect=requests.RequestException("offline")) + assert _resolve_latest_release_tag() is None + + +def test_resolve_latest_release_tag_non_200_returns_none(mocker) -> None: + response = MagicMock(status_code=404) + mocker.patch.object(ci_emitter.requests, "get", return_value=response) + assert _resolve_latest_release_tag() is None + + +def test_resolve_latest_release_tag_invalid_json_returns_none(mocker) -> None: + response = MagicMock(status_code=200) + response.json.side_effect = ValueError("not json") + mocker.patch.object(ci_emitter.requests, "get", return_value=response) + assert _resolve_latest_release_tag() is None + + +def test_resolve_latest_release_tag_non_dict_json_returns_none(mocker) -> None: + response = MagicMock(status_code=200) + response.json.return_value = ["not", "a", "dict"] + mocker.patch.object(ci_emitter.requests, "get", return_value=response) + assert _resolve_latest_release_tag() is None + + +def test_resolve_latest_release_tag_rejects_unsafe_tag(mocker) -> None: + # A tag lands verbatim in emitted YAML — reject anything outside plain + # tag characters (whitespace, quotes, comment markers). + response = MagicMock(status_code=200) + response.json.return_value = {"tag_name": "v1.0 # evil"} + mocker.patch.object(ci_emitter.requests, "get", return_value=response) + assert _resolve_latest_release_tag() is None + + +def test_resolve_action_ref_pins_latest_release_sha(monkeypatch) -> None: + # Online: the emitted pin is the latest published release resolved to its + # commit SHA — not the running build's version, which may not exist as a + # tag (e.g. a dev build's 26.7.0+ad3dc1d). + sha = "f" * 40 + ci_emitter.resolve_action_ref.cache_clear() + monkeypatch.setattr(ci_emitter, "_PACKAGE_VERSION", "26.7.0+ad3dc1d") + monkeypatch.setattr(ci_emitter, "_resolve_latest_release_tag", lambda: "v27.1.0") + monkeypatch.setattr(ci_emitter, "_resolve_tag_sha", lambda version: sha if version == "v27.1.0" else None) + assert ci_emitter.resolve_action_ref() == f"{ACTION_REPO}@{sha} # v27.1.0" + ci_emitter.resolve_action_ref.cache_clear() + + def test_resolve_action_ref_online_includes_sha(monkeypatch) -> None: + # Release lookup fails (autouse fixture stubs it to None) but the tag + # lookup succeeds: pin the build-version tag's SHA. sha = "d" * 40 ci_emitter.resolve_action_ref.cache_clear() monkeypatch.setattr(ci_emitter, "_resolve_tag_sha", lambda version: sha) @@ -407,7 +472,8 @@ def test_resolve_action_ref_online_includes_sha(monkeypatch) -> None: def test_resolve_action_ref_offline_is_tag_pinned() -> None: - # The autouse offline_action_pin fixture forces _resolve_tag_sha -> None. + # The autouse offline_action_pin fixture forces both GitHub lookups -> + # None: default to the version we built, tag-pinned. ci_emitter.resolve_action_ref.cache_clear() ref = ci_emitter.resolve_action_ref() assert ref == f"{ACTION_REPO}@{_action_version()}"