From 8729b52ddad376643cd78ee6929f32e20345f6d9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 20:57:51 +0000 Subject: [PATCH 1/4] Start fix for #1645 Assisted-by: Claude Code:claude-opus-5-5 From 3632712e60c77f4e12458b1a01817244e5953a6c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 20:57:51 +0000 Subject: [PATCH 2/4] CI: pin floating Node, Python and Go versions The e2e, yarn-classic, test, coverage and hosted-e2e jobs installed Node '24.x', Python '3.12.x' and Go '1.24'. Any upstream release then changed what CI ran with no commit here, which is how uv 0.13.2 turned main red and stalled the merge queue for 2.7 h on 10-20. Pin them to exact releases: Node 24.21.0 (already used by the vlt legs), Go 1.24.13, and Python 3.12.10, the newest 3.12 that actions/python-versions builds for Windows and macOS as well as Linux. Later 3.12 patches are source-only, so '3.12.x' already resolved to 3.12.10 on those runners. Refs #1645 Assisted-by: Claude Code:claude-opus-5-5 --- .github/workflows/ci.yml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 764e0bbbf..6bbb309c1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -400,7 +400,7 @@ jobs: # SHA pin resolved from `gh api repos/actions/setup-go/git/refs/tags/v6.4.0`. uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: - go-version: '1.24' + go-version: '1.24.13' # exact: bump deliberately (#1645) cache: false - name: Restore vexctl (macOS) @@ -617,7 +617,7 @@ jobs: # This job is the Linux leg of `test` (see there): same Go pin. uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: - go-version: '1.24' + go-version: '1.24.13' # exact: bump deliberately (#1645) cache: false - name: Install vexctl @@ -1419,13 +1419,13 @@ jobs: if: matrix.suite == 'e2e_redirect_pnpm_build' uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: - node-version: '24.x' + node-version: '24.21.0' # exact: bump deliberately (#1645) - name: Setup Python if: matrix.suite == 'e2e_pypi' || matrix.uv != '' || matrix.poetry != '' || matrix.pdm != '' || matrix.hatch != '' uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0 with: - python-version: '3.12.x' + python-version: '3.12.10' # exact: last 3.12 with Windows+macOS builds; bump deliberately (#1645) - name: Setup uv (uv legs) if: matrix.uv != '' @@ -2238,7 +2238,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: - node-version: '24.x' + node-version: '24.21.0' # exact: bump deliberately (#1645) - name: Manifest-less VEX over every real yarn classic flow # The script exports SOCKET_PATCH_YARN_E2E_REQUIRED=1 and # SOCKET_PATCH_YARN_CLASSIC_E2E_VERSION itself. @@ -2632,9 +2632,9 @@ jobs: # `node:sqlite` for its store index, which does not exist before # Node 22 (and is only stable in 24) — on 20 every `pnpm install` # dies with ERR_UNKNOWN_BUILTIN_MODULE. This suite drives real, - # current package managers against production, so it tracks what - # users actually run rather than the pin the offline suites need. - node-version: '24.x' + # current package managers against production, so it runs the Node + # line users run rather than the 20.x pin the offline suites need. + node-version: '24.21.0' # exact: bump deliberately (#1645) - name: Setup npm-family package managers if: steps.gate.outputs.run == 'true' @@ -2679,7 +2679,7 @@ jobs: if: steps.gate.outputs.run == 'true' uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0 with: - python-version: '3.12.x' + python-version: '3.12.10' # exact: last 3.12 with Windows+macOS builds; bump deliberately (#1645) - name: Install uv if: steps.gate.outputs.run == 'true' @@ -2699,7 +2699,7 @@ jobs: if: steps.gate.outputs.run == 'true' uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: - go-version: '1.24' + go-version: '1.24.13' # exact: bump deliberately (#1645) cache: false - name: Run hosted-mode production e2e From cd80d9090a84c3237b2388a73d75cf220a1da4b5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 20:57:51 +0000 Subject: [PATCH 3/4] CI: lint ci.yml for floating toolchain versions pin-check only made `uses:` references immutable. Nothing stopped a setup step from taking the latest tool, so a new upstream release could break the merge queue until someone landed a pin. Add scripts/check-tool-pins.py and run it from pin-check on ci.yml, the only workflow that gates PRs and the merge queue. It fails when setup-uv has no `version:`, or when a uv, node, python or go version is not an exact X.Y.Z, including literal fallbacks and the matrix values an expression reads. The pin-check job moves to the Depot runner mapping with the DISABLE_DEPOT_RUNNERS switch. Fixes #1645 Assisted-by: Claude Code:claude-opus-5-5 --- .github/workflows/pin-check.yml | 12 ++- scripts/check-tool-pins.py | 121 +++++++++++++++++++++++ scripts/tests/test_check_tool_pins.py | 135 ++++++++++++++++++++++++++ 3 files changed, 267 insertions(+), 1 deletion(-) create mode 100755 scripts/check-tool-pins.py create mode 100644 scripts/tests/test_check_tool_pins.py diff --git a/.github/workflows/pin-check.yml b/.github/workflows/pin-check.yml index 2245236f8..b96e132ef 100644 --- a/.github/workflows/pin-check.yml +++ b/.github/workflows/pin-check.yml @@ -10,18 +10,20 @@ on: paths: - '.github/workflows/**' - '.github/actions/**' + - 'scripts/check-tool-pins.py' push: branches: - main paths: - '.github/workflows/**' - '.github/actions/**' + - 'scripts/check-tool-pins.py' permissions: {} jobs: check: - runs-on: ubuntu-latest + runs-on: ${{ vars.DISABLE_DEPOT_RUNNERS == 'true' && 'ubuntu-latest' || 'depot-ubuntu-24.04-4' }} permissions: contents: read steps: @@ -50,3 +52,11 @@ jobs: exit 1 fi echo "All action references are SHA-pinned." + + - name: Verify ci.yml toolchain versions are exact + # A floating `node-version: '24.x'`, `python-version: '3.12.x'`, + # `go-version: '1.24'` or an unversioned setup-uv lets an upstream + # release turn main red with no commit here; uv 0.13.2 stalled the + # merge queue for 2.7 h that way (#1645). Only ci.yml gates PRs and + # the merge queue, so only it is checked. + run: python3 scripts/check-tool-pins.py .github/workflows/ci.yml diff --git a/scripts/check-tool-pins.py b/scripts/check-tool-pins.py new file mode 100755 index 000000000..221a9b53f --- /dev/null +++ b/scripts/check-tool-pins.py @@ -0,0 +1,121 @@ +#!/usr/bin/env python3 +"""Fail when a workflow installs an oracle toolchain at a floating version. + +`pin-check.yml` already makes every `uses:` reference an immutable SHA, but +the *tool* a setup action installs can still float: `node-version: '24.x'`, +`python-version: '3.12.x'`, `go-version: '1.24'`, or `astral-sh/setup-uv` +with no `version:` at all. Any upstream release then changes what CI runs +while main stays the same. On 2026-10-20 uv 0.13.2 did exactly that, failed +a Pipenv leg on every merge-queue entry and stalled the queue for 2.7 h +(#1645). + +Rules, per step: + * `astral-sh/setup-uv` must set `with.version`. + * `with.version` of setup-uv, and `with.node-version`, `python-version` + and `go-version` of any step, must be an exact `X.Y.Z` release. + * A `${{ ... }}` expression is allowed when every quoted literal inside it + is exact, and every `matrix.` it reads is exact in each matrix + entry of that job that sets the key. A matrix built with `fromJSON` + can't be resolved statically and is left to review. + +Usage: check-tool-pins.py WORKFLOW.yml [...] (exit 1 on any violation) +""" + +import re +import sys + +import yaml + +EXACT = re.compile(r"^\d+\.\d+\.\d+$") +VERSION_KEYS = ("node-version", "python-version", "go-version") +UV_ACTION = "astral-sh/setup-uv" +LITERAL = re.compile(r"'([^']*)'") +MATRIX_REF = re.compile(r"\bmatrix\.([A-Za-z0-9_-]+)") + + +def matrix_values(job, key): + """Every value `matrix.` takes in `job`, or None if unresolvable.""" + matrix = (job.get("strategy") or {}).get("matrix") + if matrix is None: + return [] + if isinstance(matrix, str): + return None + values = [] + axis = matrix.get(key) + if isinstance(axis, str): + return None + if isinstance(axis, list): + values.extend(axis) + include = matrix.get("include") + if isinstance(include, str): + return None + for entry in include or []: + if isinstance(entry, dict) and key in entry: + values.append(entry[key]) + return values + + +def check_value(job, value): + """Return a list of reasons `value` is not an exact pin (empty if ok).""" + text = str(value).strip() + if "${{" not in text: + return [] if EXACT.match(text) else [f"{text!r} is not an exact X.Y.Z"] + reasons = [] + for literal in LITERAL.findall(text): + if not EXACT.match(literal): + reasons.append(f"fallback {literal!r} is not an exact X.Y.Z") + for key in MATRIX_REF.findall(text): + values = matrix_values(job, key) + if values is None: + continue + for v in values: + if not EXACT.match(str(v)): + reasons.append(f"matrix.{key} value {v!r} is not an exact X.Y.Z") + return reasons + + +def check_workflow(path): + with open(path, encoding="utf-8") as f: + doc = yaml.safe_load(f) or {} + violations = [] + for job_name, job in (doc.get("jobs") or {}).items(): + if not isinstance(job, dict): + continue + for i, step in enumerate(job.get("steps") or []): + if not isinstance(step, dict): + continue + where = f"{path}: job {job_name!r}, step {step.get('name') or i!r}" + with_ = step.get("with") or {} + uses = str(step.get("uses") or "") + if uses.split("@")[0] == UV_ACTION: + if "version" not in with_: + violations.append(f"{where}: {UV_ACTION} without `version:`") + else: + for r in check_value(job, with_["version"]): + violations.append(f"{where}: version {r}") + for key in VERSION_KEYS: + if key in with_: + for r in check_value(job, with_[key]): + violations.append(f"{where}: {key} {r}") + return violations + + +def main(argv): + if not argv: + print(__doc__.strip().splitlines()[-1], file=sys.stderr) + return 2 + violations = [v for path in argv for v in check_workflow(path)] + for v in violations: + print(f"::error::{v}") + if violations: + print( + f"{len(violations)} floating toolchain version(s). Pin each to an " + "exact release and bump it deliberately in its own PR." + ) + return 1 + print("All toolchain versions are pinned to exact releases.") + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/scripts/tests/test_check_tool_pins.py b/scripts/tests/test_check_tool_pins.py new file mode 100644 index 000000000..c3dee6fc2 --- /dev/null +++ b/scripts/tests/test_check_tool_pins.py @@ -0,0 +1,135 @@ +"""Tests for scripts/check-tool-pins.py, the floating-toolchain lint (#1645).""" + +import importlib.util +from pathlib import Path +import tempfile +import unittest + +import yaml + +ROOT = Path(__file__).parents[2] + + +def load_script(): + spec = importlib.util.spec_from_file_location( + "check_tool_pins", ROOT / "scripts" / "check-tool-pins.py" + ) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +pins = load_script() + + +SHA = "0" * 40 + + +def violations(steps, matrix=None): + job = {"runs-on": "ubuntu-latest", "steps": steps} + if matrix is not None: + job["strategy"] = {"matrix": matrix} + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "ci.yml" + path.write_text(yaml.safe_dump({"jobs": {"e2e": job}})) + return pins.check_workflow(str(path)) + + +def setup(action, **inputs): + step = {"uses": f"{action}@{SHA}"} + if inputs: + step["with"] = {k.replace("_", "-"): v for k, v in inputs.items()} + return step + + +class FloatingVersions(unittest.TestCase): + def test_floating_runtime_versions_are_rejected(self): + for key, value in [ + ("node-version", "24.x"), + ("python-version", "3.12.x"), + ("python-version", "3.12"), + ("python-version", 3.12), + ("go-version", "1.24"), + ("node-version", "24"), + ("node-version", 24), + ("node-version", "latest"), + ("go-version", ">=1.24.0"), + ]: + with self.subTest(key=key, value=value): + found = violations([{"uses": f"actions/setup-x@{SHA}", "with": {key: value}}]) + self.assertEqual(len(found), 1, found) + self.assertIn(key, found[0]) + + def test_exact_versions_pass(self): + self.assertEqual(violations([ + setup("actions/setup-node", node_version="24.21.0"), + setup("actions/setup-python", python_version="3.12.10"), + setup("actions/setup-go", go_version="1.24.13"), + ]), []) + + def test_other_inputs_are_not_checked(self): + self.assertEqual(violations([ + setup("ruby/setup-ruby", bundler="2.6"), + {"run": "echo node-version 24.x"}, + ]), []) + + +class SetupUv(unittest.TestCase): + def test_setup_uv_without_version_is_rejected(self): + # The 10-20 incident: no `version:`, so every run took the latest uv. + found = violations([setup("astral-sh/setup-uv", enable_cache=False)]) + self.assertEqual(len(found), 1, found) + self.assertIn("without `version:`", found[0]) + + def test_setup_uv_with_no_inputs_is_rejected(self): + found = violations([setup("astral-sh/setup-uv")]) + self.assertEqual(len(found), 1, found) + + def test_setup_uv_floating_version_is_rejected(self): + found = violations([setup("astral-sh/setup-uv", version="0.13.x")]) + self.assertEqual(len(found), 1, found) + + def test_setup_uv_exact_version_passes(self): + self.assertEqual(violations([setup("astral-sh/setup-uv", version="0.13.1")]), []) + + +class Expressions(unittest.TestCase): + STEP = setup("actions/setup-node", node_version="${{ matrix.node || '24.21.0' }}") + + def test_exact_matrix_values_and_fallback_pass(self): + matrix = {"include": [{"node": "22.22.1"}, {"os": "x"}]} + self.assertEqual(violations([self.STEP], matrix), []) + + def test_floating_matrix_include_value_is_rejected(self): + found = violations([self.STEP], {"include": [{"node": "22.x"}]}) + self.assertEqual(len(found), 1, found) + self.assertIn("matrix.node", found[0]) + + def test_floating_matrix_axis_value_is_rejected(self): + found = violations([self.STEP], {"node": ["24.21.0", "22"]}) + self.assertEqual(len(found), 1, found) + self.assertIn("'22'", found[0]) + + def test_floating_fallback_literal_is_rejected(self): + found = violations([setup("actions/setup-node", node_version="${{ matrix.node || '24.x' }}")]) + self.assertEqual(len(found), 1, found) + self.assertIn("fallback", found[0]) + + def test_from_json_matrix_is_left_to_review(self): + self.assertEqual(violations([self.STEP], "${{ fromJSON(vars.M) }}"), []) + + +class Repository(unittest.TestCase): + def test_ci_yml_has_no_floating_toolchains(self): + workflow = ROOT / ".github" / "workflows" / "ci.yml" + self.assertEqual(pins.check_workflow(str(workflow)), []) + + def test_main_exit_codes(self): + self.assertEqual(pins.main([]), 2) + self.assertEqual( + pins.main([str(ROOT / ".github" / "workflows" / "ci.yml")]), 0 + ) + + +if __name__ == "__main__": + unittest.main() From 4c5f40349be691aa40b40a998f3cdc6b3c9793bb Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 21:02:46 +0000 Subject: [PATCH 4/4] CI: gate pins in lint-ecosystems, stdlib only Review follow-up. The pin lint imported PyYAML, which the runner's python3 may not have; on such a runner the new test module would break the required lint-ecosystems job. The lint now scans the workflow as text with the standard library, like the other ci.yml tests do. The pin-check step is dropped. lint-ecosystems already runs every scripts/tests module on each PR, so the repository test gates ci.yml with no new step or job, and pin-check is back to its main version. The lint also now rejects expressions that read vars/env/inputs, skips comparison operands such as `!= ''`, and matches setup-uv regardless of case. Assisted-by: Claude Code:claude-opus-5-5 --- .github/workflows/pin-check.yml | 12 +- scripts/check-tool-pins.py | 245 ++++++++++++++++++++------ scripts/tests/test_check_tool_pins.py | 211 +++++++++++++++------- 3 files changed, 340 insertions(+), 128 deletions(-) diff --git a/.github/workflows/pin-check.yml b/.github/workflows/pin-check.yml index b96e132ef..2245236f8 100644 --- a/.github/workflows/pin-check.yml +++ b/.github/workflows/pin-check.yml @@ -10,20 +10,18 @@ on: paths: - '.github/workflows/**' - '.github/actions/**' - - 'scripts/check-tool-pins.py' push: branches: - main paths: - '.github/workflows/**' - '.github/actions/**' - - 'scripts/check-tool-pins.py' permissions: {} jobs: check: - runs-on: ${{ vars.DISABLE_DEPOT_RUNNERS == 'true' && 'ubuntu-latest' || 'depot-ubuntu-24.04-4' }} + runs-on: ubuntu-latest permissions: contents: read steps: @@ -52,11 +50,3 @@ jobs: exit 1 fi echo "All action references are SHA-pinned." - - - name: Verify ci.yml toolchain versions are exact - # A floating `node-version: '24.x'`, `python-version: '3.12.x'`, - # `go-version: '1.24'` or an unversioned setup-uv lets an upstream - # release turn main red with no commit here; uv 0.13.2 stalled the - # merge queue for 2.7 h that way (#1645). Only ci.yml gates PRs and - # the merge queue, so only it is checked. - run: python3 scripts/check-tool-pins.py .github/workflows/ci.yml diff --git a/scripts/check-tool-pins.py b/scripts/check-tool-pins.py index 221a9b53f..9992a51e5 100755 --- a/scripts/check-tool-pins.py +++ b/scripts/check-tool-pins.py @@ -1,8 +1,8 @@ #!/usr/bin/env python3 """Fail when a workflow installs an oracle toolchain at a floating version. -`pin-check.yml` already makes every `uses:` reference an immutable SHA, but -the *tool* a setup action installs can still float: `node-version: '24.x'`, +`pin-check.yml` makes every `uses:` reference an immutable SHA, but the +*tool* a setup action installs can still float: `node-version: '24.x'`, `python-version: '3.12.x'`, `go-version: '1.24'`, or `astral-sh/setup-uv` with no `version:` at all. Any upstream release then changes what CI runs while main stays the same. On 2026-10-20 uv 0.13.2 did exactly that, failed @@ -13,10 +13,20 @@ * `astral-sh/setup-uv` must set `with.version`. * `with.version` of setup-uv, and `with.node-version`, `python-version` and `go-version` of any step, must be an exact `X.Y.Z` release. - * A `${{ ... }}` expression is allowed when every quoted literal inside it - is exact, and every `matrix.` it reads is exact in each matrix - entry of that job that sets the key. A matrix built with `fromJSON` - can't be resolved statically and is left to review. + * A `${{ ... }}` expression may read only `matrix.*`. Every quoted value + it can yield (comparison operands aside) must be exact, and so must + every value the job's matrix gives a key it reads. A matrix built with + `fromJSON` can't be resolved statically and is left to review. + +The workflow is scanned as text with the standard library only, the same +way scripts/tests reads ci.yml, so the lint runs on any python3. It relies +on the block style ci.yml uses: steps as `- ` items, `with:` inputs one +per line, matrix rows as `{key: value, ...}` or `key: [a, b]`. A step +shared through a YAML anchor is checked once, against the matrix of the +job that defines it. + +Repo gate: scripts/tests/test_check_tool_pins.py runs this on ci.yml in +the lean `lint-ecosystems` job. Usage: check-tool-pins.py WORKFLOW.yml [...] (exit 1 on any violation) """ @@ -24,82 +34,201 @@ import re import sys -import yaml - EXACT = re.compile(r"^\d+\.\d+\.\d+$") VERSION_KEYS = ("node-version", "python-version", "go-version") UV_ACTION = "astral-sh/setup-uv" LITERAL = re.compile(r"'([^']*)'") -MATRIX_REF = re.compile(r"\bmatrix\.([A-Za-z0-9_-]+)") +CONTEXT_REF = re.compile(r"\b([A-Za-z_][A-Za-z0-9_]*)\.([A-Za-z0-9_-]+)") +COMPARED = re.compile(r"(==|!=)\s*$") +COMPARES = re.compile(r"^\s*(==|!=)") +JOB = re.compile(r"^ ([A-Za-z0-9_-]+):\s*(#.*)?$") +KEY_VALUE = re.compile(r"^(\s*)(-\s+)?([A-Za-z0-9_-]+):\s*(.*)$") + + +def indent(line): + return len(line) - len(line.lstrip(" ")) + + +def strip_comment(value): + """Drop a trailing ` # ...` comment that sits outside quotes.""" + quote = None + for i, ch in enumerate(value): + if quote: + if ch == quote: + quote = None + elif ch in "'\"": + quote = ch + elif ch == "#" and (i == 0 or value[i - 1].isspace()): + return value[:i].rstrip() + return value.strip() -def matrix_values(job, key): - """Every value `matrix.` takes in `job`, or None if unresolvable.""" - matrix = (job.get("strategy") or {}).get("matrix") - if matrix is None: +def unquote(value): + value = value.strip() + if len(value) >= 2 and value[0] == value[-1] and value[0] in "'\"": + return value[1:-1] + return value + + +def is_content(line): + stripped = line.strip() + return stripped != "" and not stripped.startswith("#") + + +def job_spans(lines): + """(name, start, end) for every job under the top-level `jobs:` key.""" + try: + top = next(i for i, l in enumerate(lines) if re.match(r"^jobs:\s*(#.*)?$", l)) + except StopIteration: return [] - if isinstance(matrix, str): + starts = [] + for i in range(top + 1, len(lines)): + line = lines[i] + if is_content(line) and indent(line) == 0: + break + m = JOB.match(line) + if m: + starts.append((m.group(1), i)) + end_of_jobs = next( + (i for i in range(top + 1, len(lines)) if is_content(lines[i]) and indent(lines[i]) == 0), + len(lines), + ) + return [ + (name, start, starts[n + 1][1] if n + 1 < len(starts) else end_of_jobs) + for n, (name, start) in enumerate(starts) + ] + + +def matrix_values(lines, start, end, key): + """Every value the job's matrix gives `key`, or None if unresolvable.""" + matrix_at = None + for i in range(start, end): + m = KEY_VALUE.match(lines[i]) + if m and m.group(3) == "matrix" and not m.group(2): + matrix_at = i + break + if matrix_at is None: + return [] + m = KEY_VALUE.match(lines[matrix_at]) + if "${{" in m.group(4): return None + base = indent(lines[matrix_at]) + block_end = matrix_at + 1 + while block_end < end and (not is_content(lines[block_end]) or indent(lines[block_end]) > base): + block_end += 1 values = [] - axis = matrix.get(key) - if isinstance(axis, str): - return None - if isinstance(axis, list): - values.extend(axis) - include = matrix.get("include") - if isinstance(include, str): - return None - for entry in include or []: - if isinstance(entry, dict) and key in entry: - values.append(entry[key]) + flow = re.compile(r"[{,]\s*" + re.escape(key) + r"""\s*:\s*('[^']*'|"[^"]*"|[^,}]+)""") + for i in range(matrix_at + 1, block_end): + line = strip_comment(lines[i]) + for v in flow.findall(line): + values.append(unquote(v)) + kv = KEY_VALUE.match(line) + if kv and kv.group(3) == key and not kv.group(2): + rest = kv.group(4).strip() + if rest.startswith("${{"): + return None + if rest.startswith("["): + values.extend(unquote(v) for v in rest.strip("[]").split(",") if v.strip()) + elif rest: + values.append(unquote(rest)) + else: + j = i + 1 + while j < block_end and (not is_content(lines[j]) or indent(lines[j]) > indent(lines[i])): + item = re.match(r"^\s*-\s+(.*)$", strip_comment(lines[j])) + if item: + values.append(unquote(item.group(1))) + j += 1 return values -def check_value(job, value): - """Return a list of reasons `value` is not an exact pin (empty if ok).""" - text = str(value).strip() +def yielded_literals(expr): + """Quoted literals an expression can evaluate to (not compared against).""" + out = [] + for m in LITERAL.finditer(expr): + before, after = expr[: m.start()], expr[m.end() :] + if COMPARED.search(before) or COMPARES.match(after): + continue + out.append(m.group(1)) + return out + + +def check_value(lines, span, value): + """Return reasons `value` is not an exact pin (empty if it is).""" + text = unquote(strip_comment(value)) if "${{" not in text: return [] if EXACT.match(text) else [f"{text!r} is not an exact X.Y.Z"] reasons = [] - for literal in LITERAL.findall(text): - if not EXACT.match(literal): - reasons.append(f"fallback {literal!r} is not an exact X.Y.Z") - for key in MATRIX_REF.findall(text): - values = matrix_values(job, key) - if values is None: + body = LITERAL.sub("''", text) + for ctx, key in CONTEXT_REF.findall(body): + if ctx != "matrix": + reasons.append(f"reads `{ctx}.{key}`, which can't be checked; use a literal or matrix value") continue - for v in values: - if not EXACT.match(str(v)): + values = matrix_values(lines, span[1], span[2], key) if span else None + for v in values or []: + if not EXACT.match(v): reasons.append(f"matrix.{key} value {v!r} is not an exact X.Y.Z") + for literal in yielded_literals(text): + if not EXACT.match(literal): + reasons.append(f"value {literal!r} in the expression is not an exact X.Y.Z") return reasons -def check_workflow(path): - with open(path, encoding="utf-8") as f: - doc = yaml.safe_load(f) or {} - violations = [] - for job_name, job in (doc.get("jobs") or {}).items(): - if not isinstance(job, dict): +def steps(lines): + """Yield (line_no, uses, {input: (line_no, value)}) for every `- ` item.""" + for i, line in enumerate(lines): + m = re.match(r"^(\s*)-\s+(.*)$", line) + if not m or m.group(2).startswith("{") or m.group(2).startswith("#"): continue - for i, step in enumerate(job.get("steps") or []): - if not isinstance(step, dict): - continue - where = f"{path}: job {job_name!r}, step {step.get('name') or i!r}" - with_ = step.get("with") or {} - uses = str(step.get("uses") or "") - if uses.split("@")[0] == UV_ACTION: - if "version" not in with_: - violations.append(f"{where}: {UV_ACTION} without `version:`") - else: - for r in check_value(job, with_["version"]): - violations.append(f"{where}: version {r}") - for key in VERSION_KEYS: - if key in with_: - for r in check_value(job, with_[key]): - violations.append(f"{where}: {key} {r}") + dash = len(m.group(1)) + item = dash + 2 + uses, inputs, with_at = None, {}, None + j = i + while j < len(lines): + cur = lines[j] + if j > i and is_content(cur) and indent(cur) <= dash: + break + text = cur if j > i else " " * item + m.group(2) + kv = KEY_VALUE.match(text) + if kv and not kv.group(2): + level = len(kv.group(1)) + if level == item: + with_at = j if kv.group(3) == "with" else None + if kv.group(3) == "uses": + uses = unquote(strip_comment(kv.group(4))) + elif with_at is not None and level == item + 2: + inputs[kv.group(3)] = (j, kv.group(4)) + j += 1 + if uses is not None or inputs: + yield i, uses, inputs + + +def check_text(path, text): + lines = text.splitlines() + spans = job_spans(lines) + violations = [] + for line_no, uses, inputs in steps(lines): + span = next((s for s in spans if s[1] <= line_no < s[2]), None) + where = f"{path}:{line_no + 1}" + (f": job {span[0]!r}" if span else "") + if uses and uses.split("@")[0].lower() == UV_ACTION: + if "version" not in inputs: + violations.append(f"{where}: {UV_ACTION} without `version:`") + else: + at, value = inputs["version"] + for r in check_value(lines, span, value): + violations.append(f"{path}:{at + 1}: setup-uv version {r}") + for key in VERSION_KEYS: + if key in inputs: + at, value = inputs[key] + for r in check_value(lines, span, value): + violations.append(f"{path}:{at + 1}: {key} {r}") return violations +def check_workflow(path): + with open(path, encoding="utf-8") as f: + return check_text(path, f.read()) + + def main(argv): if not argv: print(__doc__.strip().splitlines()[-1], file=sys.stderr) diff --git a/scripts/tests/test_check_tool_pins.py b/scripts/tests/test_check_tool_pins.py index c3dee6fc2..bc4d1c06a 100644 --- a/scripts/tests/test_check_tool_pins.py +++ b/scripts/tests/test_check_tool_pins.py @@ -1,13 +1,16 @@ -"""Tests for scripts/check-tool-pins.py, the floating-toolchain lint (#1645).""" +"""Tests for scripts/check-tool-pins.py, the floating-toolchain lint (#1645). + +`Repository` is the gate: lint-ecosystems runs this module on every PR, so a +floating node/python/go/uv version in ci.yml fails the lean CI run. +""" import importlib.util from pathlib import Path -import tempfile +import textwrap import unittest -import yaml - ROOT = Path(__file__).parents[2] +SHA = "0" * 40 def load_script(): @@ -22,113 +25,203 @@ def load_script(): pins = load_script() -SHA = "0" * 40 +def workflow(steps, matrix=""): + """A one-job workflow; `steps` and `matrix` are dedented YAML blocks.""" + out = "on: push\njobs:\n e2e:\n runs-on: ubuntu-latest\n" + if matrix: + out += " strategy:\n" + textwrap.indent(textwrap.dedent(matrix).strip("\n"), " ") + "\n" + out += " steps:\n" + textwrap.indent(textwrap.dedent(steps).strip("\n"), " ") + "\n" + return out -def violations(steps, matrix=None): - job = {"runs-on": "ubuntu-latest", "steps": steps} - if matrix is not None: - job["strategy"] = {"matrix": matrix} - with tempfile.TemporaryDirectory() as tmp: - path = Path(tmp) / "ci.yml" - path.write_text(yaml.safe_dump({"jobs": {"e2e": job}})) - return pins.check_workflow(str(path)) +def violations(steps, matrix=""): + return pins.check_text("ci.yml", workflow(steps, matrix)) -def setup(action, **inputs): - step = {"uses": f"{action}@{SHA}"} - if inputs: - step["with"] = {k.replace("_", "-"): v for k, v in inputs.items()} - return step +def setup(action, key, value): + return f""" + - name: Setup + uses: {action}@{SHA} # vX + with: + {key}: {value} + """ class FloatingVersions(unittest.TestCase): def test_floating_runtime_versions_are_rejected(self): for key, value in [ - ("node-version", "24.x"), - ("python-version", "3.12.x"), + ("node-version", "'24.x'"), + ("python-version", "'3.12.x'"), + ("python-version", "'3.12'"), ("python-version", "3.12"), - ("python-version", 3.12), - ("go-version", "1.24"), + ("go-version", "'1.24'"), ("node-version", "24"), - ("node-version", 24), ("node-version", "latest"), - ("go-version", ">=1.24.0"), + ("go-version", "'>=1.24.0'"), + ("node-version", "'24.x' # exact? no"), ]: with self.subTest(key=key, value=value): - found = violations([{"uses": f"actions/setup-x@{SHA}", "with": {key: value}}]) + found = violations(setup("actions/setup-x", key, value)) self.assertEqual(len(found), 1, found) self.assertIn(key, found[0]) + self.assertIn("ci.yml:9:", found[0]) def test_exact_versions_pass(self): - self.assertEqual(violations([ - setup("actions/setup-node", node_version="24.21.0"), - setup("actions/setup-python", python_version="3.12.10"), - setup("actions/setup-go", go_version="1.24.13"), - ]), []) + self.assertEqual(violations( + setup("actions/setup-node", "node-version", "'24.21.0' # bump deliberately") + + setup("actions/setup-python", "python-version", "3.12.10") + + setup("actions/setup-go", "go-version", '"1.24.13"') + ), []) + + def test_step_starting_with_uses_is_checked(self): + found = violations(f""" + - uses: actions/setup-node@{SHA} + with: + node-version: '24.x' + """) + self.assertEqual(len(found), 1, found) - def test_other_inputs_are_not_checked(self): - self.assertEqual(violations([ - setup("ruby/setup-ruby", bundler="2.6"), - {"run": "echo node-version 24.x"}, - ]), []) + def test_other_inputs_and_run_scripts_are_not_checked(self): + self.assertEqual(violations(f""" + - uses: ruby/setup-ruby@{SHA} + with: + bundler: '2.6' + - name: Script + run: | + echo node-version: 24.x + - with: + version: latest + - name: Env + env: + go-version: '1.24' + run: true + """), []) class SetupUv(unittest.TestCase): def test_setup_uv_without_version_is_rejected(self): # The 10-20 incident: no `version:`, so every run took the latest uv. - found = violations([setup("astral-sh/setup-uv", enable_cache=False)]) + found = violations(f""" + - name: Setup uv + uses: astral-sh/setup-uv@{SHA} # v6.8.0 + with: + enable-cache: false + - name: Next + with: + version: '0.13.1' + """) self.assertEqual(len(found), 1, found) self.assertIn("without `version:`", found[0]) def test_setup_uv_with_no_inputs_is_rejected(self): - found = violations([setup("astral-sh/setup-uv")]) + found = violations(f"- uses: astral-sh/setup-uv@{SHA}\n") + self.assertEqual(len(found), 1, found) + + def test_owner_and_repo_match_case_insensitively(self): + found = violations(f"- uses: Astral-sh/Setup-UV@{SHA}\n") self.assertEqual(len(found), 1, found) def test_setup_uv_floating_version_is_rejected(self): - found = violations([setup("astral-sh/setup-uv", version="0.13.x")]) + found = violations(setup("astral-sh/setup-uv", "version", "'0.13.x'")) self.assertEqual(len(found), 1, found) - def test_setup_uv_exact_version_passes(self): - self.assertEqual(violations([setup("astral-sh/setup-uv", version="0.13.1")]), []) + def test_setup_uv_exact_version_with_comments_passes(self): + self.assertEqual(violations(f""" + - name: Setup uv + uses: astral-sh/setup-uv@{SHA} + with: + # Pin uv: bump deliberately. + version: "0.13.1" + """), []) class Expressions(unittest.TestCase): - STEP = setup("actions/setup-node", node_version="${{ matrix.node || '24.21.0' }}") - - def test_exact_matrix_values_and_fallback_pass(self): - matrix = {"include": [{"node": "22.22.1"}, {"os": "x"}]} - self.assertEqual(violations([self.STEP], matrix), []) - - def test_floating_matrix_include_value_is_rejected(self): - found = violations([self.STEP], {"include": [{"node": "22.x"}]}) + STEP = setup("actions/setup-node", "node-version", "${{ matrix.node || '24.21.0' }}") + + def test_exact_include_values_and_fallback_pass(self): + matrix = """ + matrix: + include: + - {os: ubuntu-latest, node: '22.22.1'} + - {os: x} + """ + self.assertEqual(violations(self.STEP, matrix), []) + + def test_floating_include_value_is_rejected(self): + matrix = """ + matrix: + include: + - {os: ubuntu-latest, node: '22.x', bun: '1'} + """ + found = violations(self.STEP, matrix) self.assertEqual(len(found), 1, found) - self.assertIn("matrix.node", found[0]) + self.assertIn("matrix.node value '22.x'", found[0]) - def test_floating_matrix_axis_value_is_rejected(self): - found = violations([self.STEP], {"node": ["24.21.0", "22"]}) + def test_floating_inline_axis_value_is_rejected(self): + found = violations(self.STEP, "matrix:\n node: ['24.21.0', '22']\n") self.assertEqual(len(found), 1, found) self.assertIn("'22'", found[0]) + def test_floating_block_axis_value_is_rejected(self): + found = violations(self.STEP, "matrix:\n node:\n - 24.21.0\n - 22.x\n") + self.assertEqual(len(found), 1, found) + self.assertIn("'22.x'", found[0]) + def test_floating_fallback_literal_is_rejected(self): - found = violations([setup("actions/setup-node", node_version="${{ matrix.node || '24.x' }}")]) + found = violations(setup("actions/setup-node", "node-version", "${{ matrix.node || '24.x' }}")) self.assertEqual(len(found), 1, found) - self.assertIn("fallback", found[0]) + self.assertIn("'24.x'", found[0]) + + def test_comparison_operands_are_not_versions(self): + step = setup( + "actions/setup-node", + "node-version", + "${{ matrix.node != '' && matrix.node || runner_os == 'Windows' && '22.22.1' || '24.21.0' }}", + ) + found = violations(step) + # `runner_os` isn't a context reference (no dot); only the yielded + # literals are checked, and both are exact. + self.assertEqual(found, []) + + def test_non_matrix_context_is_rejected(self): + for expr in ("${{ vars.NODE_VERSION }}", "${{ env.NODE }}", "${{ inputs.node || '24.21.0' }}"): + with self.subTest(expr=expr): + found = violations(setup("actions/setup-node", "node-version", expr)) + self.assertEqual(len(found), 1, found) + self.assertIn("can't be checked", found[0]) def test_from_json_matrix_is_left_to_review(self): - self.assertEqual(violations([self.STEP], "${{ fromJSON(vars.M) }}"), []) + found = violations(self.STEP, "matrix: ${{ fromJSON(vars.M) }}\n") + self.assertEqual(found, []) + + def test_matrix_of_another_job_is_not_used(self): + text = workflow(self.STEP) + " other:\n strategy:\n matrix:\n node: ['22.x']\n" + self.assertEqual(pins.check_text("ci.yml", text), []) class Repository(unittest.TestCase): + CI = ROOT / ".github" / "workflows" / "ci.yml" + def test_ci_yml_has_no_floating_toolchains(self): - workflow = ROOT / ".github" / "workflows" / "ci.yml" - self.assertEqual(pins.check_workflow(str(workflow)), []) + self.assertEqual(pins.check_workflow(str(self.CI)), []) + + def test_ci_yml_setup_steps_are_seen(self): + # Guard against the text scan silently finding nothing: count the + # inputs it checks in ci.yml. + lines = self.CI.read_text().splitlines() + seen = [ + key + for _, _, inputs in pins.steps(lines) + for key in inputs + if key in pins.VERSION_KEYS + ] + self.assertGreaterEqual(seen.count("node-version"), 6, seen) + self.assertGreaterEqual(seen.count("python-version"), 2, seen) + self.assertGreaterEqual(seen.count("go-version"), 3, seen) def test_main_exit_codes(self): self.assertEqual(pins.main([]), 2) - self.assertEqual( - pins.main([str(ROOT / ".github" / "workflows" / "ci.yml")]), 0 - ) + self.assertEqual(pins.main([str(self.CI)]), 0) if __name__ == "__main__":