diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8d6f692fda..9bcfa2d7b4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -162,6 +162,16 @@ jobs: - name: Every workflow file must parse run: python3 -m pytest tests/test_workflow_yaml_valid.py -q + # WorkflowIntegrityGuard. Written for ADR-005 ("a CI step that cannot + # fail is worse than no CI step") and, until #5744, run by NO job at all + # -- the exact trap this repo's own convention warns about, on the guard + # whose whole subject is guards that do nothing. It also now rejects a + # `${{ }}` expansion of a workflow input inside a `run:` body, the + # template-injection shape #5673 fixed in auto-deploy-cloud.yml and + # #5744 fixed again here. + - name: Workflow integrity (dead steps, unreachable triggers, input injection) + run: python3 -m pytest tests/test_ci_workflow_invocations_are_real.py -q + # Named explicitly because this repo's CI runs FILE LISTS, not # `pytest tests/`. /api/runtime-summary claimed to mirror the daemon's # runtimeSummary slice but re-implemented it as a capped raw-event scan: diff --git a/.github/workflows/verify-published-wheel.yml b/.github/workflows/verify-published-wheel.yml index b591b4dd07..0d6495f04d 100644 --- a/.github/workflows/verify-published-wheel.yml +++ b/.github/workflows/verify-published-wheel.yml @@ -73,16 +73,43 @@ jobs: - name: Resolve version id: ver shell: python + # The dispatch input is bound here rather than expanded inside the + # script. A `${{ }}` expansion is pasted into the program text before + # the interpreter runs, so the value becomes part of the program; an + # env var is data the program only ever reads. This step is the taint + # root for the whole job -- the verify step below reads + # `steps.ver.outputs.version`, which is this value. + # + # The rule is recorded, not just applied: "A workflow input is data, + # never program text" in the Release Verification and Merge Gating + # blueprint. #5673 fixed this same construct in auto-deploy-cloud.yml + # and it recurred here, because nothing wrote the rule down and nothing + # failed when it came back. WorkflowIntegrityGuard now covers the + # pattern, so a third occurrence fails a check instead of waiting for a + # reviewer to recognise the shape. + env: + INPUT_VERSION: ${{ github.event.inputs.version }} run: | - import json, os, urllib.request - want = "${{ github.event.inputs.version }}".strip() + import json, os, re, urllib.request + want = os.environ.get("INPUT_VERSION", "").strip() if not want: with urllib.request.urlopen( "https://pypi.org/pypi/clawmetry/json", timeout=30) as r: want = json.load(r)["info"]["version"] + # Belt to the env-binding's braces: what goes into GITHUB_OUTPUT is a + # bare version and nothing else, so a later step cannot inherit a + # newline (which would forge a second output) or shell metacharacters. + if not re.fullmatch(r"[0-9A-Za-z.+!-]{1,64}", want): + raise SystemExit(f"not a version string, refusing to continue: {want!r}") with open(os.environ["GITHUB_OUTPUT"], "a") as fh: fh.write(f"version={want}\n") print(f"verifying clawmetry=={want}") + # `shell: bash` is explicit because this runs on the Windows leg too, + # where the default shell is pwsh and `$VERSION` would silently read as + # an unset PowerShell variable rather than the environment variable. - name: Verify the published wheel - run: python scripts/verify_published_wheel.py --version "${{ steps.ver.outputs.version }}" + shell: bash + env: + VERSION: ${{ steps.ver.outputs.version }} + run: python scripts/verify_published_wheel.py --version "$VERSION" diff --git a/tests/test_ci_workflow_invocations_are_real.py b/tests/test_ci_workflow_invocations_are_real.py index 79bfdca33f..e588d1a18e 100644 --- a/tests/test_ci_workflow_invocations_are_real.py +++ b/tests/test_ci_workflow_invocations_are_real.py @@ -172,3 +172,94 @@ def test_pypi_wait_polls_the_index_pip_reads(): assert "pip download" in src or "pip index versions" in src, ( "the PyPI wait must prove installability through pip's own index" ) + + +# ------------------------------------------- template injection into run: bodies + +# Contexts an actor who is not a repository writer can influence, or that a +# writer can set to arbitrary text through a form field. A `${{ }}` naming one +# of these is pasted into the step body BEFORE the interpreter starts, so the +# value arrives as program SOURCE rather than as a value. +_UNTRUSTED_CONTEXTS = ( + "github.event.inputs.", + "inputs.", + "github.head_ref", + "github.event.issue.", + "github.event.pull_request.", + "github.event.comment.", + "github.event.review.", + "github.event.discussion.", + "github.event.workflow_run.head_branch", +) +_EXPR_RE = re.compile(r"\$\{\{\s*(?P[^}]+?)\s*\}\}") + + +def _run_steps(): + """Every `run:` step body in every workflow, with where it came from. + + Scope is derived from the workflow files, so a new workflow is covered + without editing this test. + """ + yaml = pytest.importorskip("yaml") + for wf in WORKFLOWS: + try: + doc = yaml.safe_load(_read(wf)) + except Exception as exc: # a malformed workflow is its own test's problem + pytest.fail("{}: {}".format(os.path.basename(wf), exc)) + if not isinstance(doc, dict): + continue + for job_name, job in (doc.get("jobs") or {}).items(): + if not isinstance(job, dict): + continue + for step in job.get("steps") or []: + if isinstance(step, dict) and isinstance(step.get("run"), str): + yield wf, job_name, step + + +def test_no_workflow_expands_an_input_into_a_step_body(): + """#5744, and #5673 before it: a workflow input is data, never program text. + + `verify-published-wheel.yml` expanded its `workflow_dispatch` `version` + input straight into a `shell: python` program, which is the template + injection class `zizmor` flags and Scorecard reports as DangerousWorkflow. + #5673 had already closed the identical shape in `auto-deploy-cloud.yml`; + nothing recorded the rule, so it came back. This is the check that stops a + third occurrence, rather than a third reviewer having to recognise it. + + The fix is always the same: bind the value with `env:` and read it with + `os.environ` / `$VAR`, where it is unambiguously a value. + """ + checked = 0 + for wf, job_name, step in _run_steps(): + checked += 1 + for expr in _EXPR_RE.findall(step["run"]): + bad = [c for c in _UNTRUSTED_CONTEXTS if c in expr] + if not bad: + continue + raise AssertionError( + "{}: job {!r}, step {!r} expands ${{{{ {} }}}} directly into " + "its `run:` body. The expansion happens before the " + "interpreter starts, so that value arrives as program source, " + "not as a value. Bind it with `env:` and read it from the " + "environment instead. See 'A workflow input is data, never " + "program text' in the Release Verification and Merge Gating " + "blueprint.".format( + os.path.basename(wf), job_name, + step.get("name") or "(unnamed)", expr, + ) + ) + assert checked > 0, "no run: steps discovered to check" + + +def test_the_guard_would_have_caught_the_defect_it_was_written_for(): + """Proving the check goes RED on the unfixed shape. + + A guard nobody has seen fail is indistinguishable from one that cannot + (ADR-005 on that blueprint), so the pattern this test rejects is asserted + against the exact construct #5744 removed. + """ + unfixed = 'want = "${{ github.event.inputs.version }}".strip()' + assert any(c in m for m in _EXPR_RE.findall(unfixed) + for c in _UNTRUSTED_CONTEXTS), unfixed + fixed = 'want = os.environ.get("INPUT_VERSION", "").strip()' + assert not _EXPR_RE.findall(fixed)