|
| 1 | +# SPDX-License-Identifier: AGPL-3.0-or-later |
| 2 | +# Copyright (C) 2026 MessageFoundry Organization and contributors |
| 3 | +"""Pin the premises that make `failure-signal.yml`'s `workflow_run` trigger safe. |
| 4 | +
|
| 5 | +`.github/zizmor.yml` suppresses `dangerous-triggers` for this file. That suppression is only honest |
| 6 | +while the properties it rests on hold, and a comment cannot enforce them. These tests are what make |
| 7 | +the suppression a claim rather than a hope, in the same shape as `test_nightly_notice.py`. |
| 8 | +
|
| 9 | +WHAT ZIZMOR IS OBJECTING TO, stated so a future reader does not have to guess. `workflow_run` runs |
| 10 | +from the DEFAULT BRANCH with a privileged token. The attack it names -- the "pwn request" class -- is |
| 11 | +a workflow that then checks out and EXECUTES the triggering pull request's code, which escalates any |
| 12 | +fork pull request into a write token. Every precondition for that is absent here, and each one below |
| 13 | +is asserted rather than described. |
| 14 | +
|
| 15 | +ONE DIFFERENCE FROM nightly-notice.yml, AND IT IS DELIBERATE. That workflow reacts only to |
| 16 | +`schedule`, so no pull request can reach it at all. This one MUST react to pull-request runs, because |
| 17 | +labelling the pull request is the entire point. So the fork path is open, and the tests below cover |
| 18 | +what that costs instead of pretending it is closed: no code from the head is fetched or run, the |
| 19 | +token cannot modify code, and the one attacker-influenceable field is gated on an event a fork cannot |
| 20 | +produce. |
| 21 | +""" |
| 22 | + |
| 23 | +from __future__ import annotations |
| 24 | + |
| 25 | +import re |
| 26 | +from pathlib import Path |
| 27 | + |
| 28 | +import pytest |
| 29 | + |
| 30 | +yaml = pytest.importorskip("yaml") |
| 31 | + |
| 32 | +WORKFLOWS = Path(__file__).resolve().parents[1] / ".github" / "workflows" |
| 33 | +FILE = WORKFLOWS / "failure-signal.yml" |
| 34 | + |
| 35 | + |
| 36 | +def _doc() -> dict: |
| 37 | + return yaml.safe_load(FILE.read_text(encoding="utf-8")) |
| 38 | + |
| 39 | + |
| 40 | +def _on(doc: dict) -> dict: |
| 41 | + # PyYAML parses a bare `on:` key as the BOOLEAN True, not the string "on". Reading doc["on"] |
| 42 | + # returns None and every assertion below would pass vacuously. |
| 43 | + return doc[True] |
| 44 | + |
| 45 | + |
| 46 | +def _steps() -> list[dict]: |
| 47 | + steps = _doc()["jobs"]["signal"]["steps"] |
| 48 | + # Positive control for every assertion built on this list. An empty or renamed job would make |
| 49 | + # "no offenders were found" and "nothing was looked at" render identically. |
| 50 | + assert steps, "the signal job has no steps to inspect" |
| 51 | + return steps |
| 52 | + |
| 53 | + |
| 54 | +def _run_blocks() -> list[str]: |
| 55 | + blocks = [s["run"] for s in _steps() if "run" in s] |
| 56 | + assert blocks, "the signal job has no `run:` step to inspect" |
| 57 | + return blocks |
| 58 | + |
| 59 | + |
| 60 | +def _step(step_id: str) -> dict: |
| 61 | + """One named step, looked up by `id:` rather than by position. |
| 62 | +
|
| 63 | + Positional indexing would still fail if the steps were reordered, but it would fail somewhere |
| 64 | + unrelated to the reorder. Naming the step makes the message say what actually moved. |
| 65 | + """ |
| 66 | + matches = [s for s in _steps() if s.get("id") == step_id] |
| 67 | + assert len(matches) == 1, f"expected exactly one step with id {step_id!r}, found {len(matches)}" |
| 68 | + return matches[0] |
| 69 | + |
| 70 | + |
| 71 | +def _run_block(step_id: str) -> str: |
| 72 | + step = _step(step_id) |
| 73 | + assert "run" in step, f"step {step_id!r} has no `run:` body to inspect" |
| 74 | + return str(step["run"]) |
| 75 | + |
| 76 | + |
| 77 | +def test_it_pulls_in_no_third_party_actions() -> None: |
| 78 | + """Nothing from the triggering ref is fetched, let alone executed. |
| 79 | +
|
| 80 | + This is the strongest of the properties: with no `uses:` at all there is no checkout, no |
| 81 | + third-party bundle, and therefore no path from a fork's branch to code running under the |
| 82 | + default branch's token. |
| 83 | + """ |
| 84 | + assert [s for s in _steps() if "uses" in s] == [], ( |
| 85 | + "failure-signal.yml gained a `uses:`. The zizmor suppression for dangerous-triggers " |
| 86 | + "rests on this workflow running no third-party code and checking nothing out. Either " |
| 87 | + "remove it, or re-justify the suppression in .github/zizmor.yml." |
| 88 | + ) |
| 89 | + |
| 90 | + |
| 91 | +def test_it_is_least_privilege_and_cannot_modify_code() -> None: |
| 92 | + """The token can label a pull request or comment on an issue. It cannot push, tag or write code.""" |
| 93 | + doc = _doc() |
| 94 | + assert doc["permissions"] == {"contents": "read"}, ( |
| 95 | + f"top-level permissions are {doc.get('permissions')!r}. Keep the file default read-only so a " |
| 96 | + "job added here cannot inherit write scope by accident." |
| 97 | + ) |
| 98 | + job_perms = doc["jobs"]["signal"].get("permissions") |
| 99 | + assert job_perms == {"pull-requests": "write", "issues": "write"}, ( |
| 100 | + f"the signal job's permissions are {job_perms!r}. It needs exactly these two writes. Anything " |
| 101 | + "that can modify code -- `contents: write`, `packages: write`, `id-token: write` -- turns the " |
| 102 | + "open fork path into the escalation the zizmor suppression says is closed." |
| 103 | + ) |
| 104 | + |
| 105 | + |
| 106 | +def test_every_event_value_reaches_a_script_through_env() -> None: |
| 107 | + """A branch name is chosen by whoever opened the branch. |
| 108 | +
|
| 109 | + Interpolating `${{ github.event.* }}` into a `run:` body would splice attacker-controlled text |
| 110 | + into a shell script. Every such value must arrive as an environment variable instead. |
| 111 | + """ |
| 112 | + offenders = [b for b in _run_blocks() if "${{" in b] |
| 113 | + assert offenders == [], ( |
| 114 | + "A run block interpolates a GitHub expression directly. Hoist it to the step's `env:` " |
| 115 | + "and reference it as a shell variable." |
| 116 | + ) |
| 117 | + # The positive half of the same claim. Absence of `${{` also holds for a workflow that reads no |
| 118 | + # event value at all, so on its own it cannot tell "hoisted to env" from "gone". Assert the |
| 119 | + # hoist itself: the resolve step is where the attacker-influenceable fields arrive. |
| 120 | + hoisted = [v for v in _step("resolve").get("env", {}).values() if "github.event" in str(v)] |
| 121 | + assert hoisted, ( |
| 122 | + "the resolve step declares no `github.event` value in its `env:`. Either the values moved " |
| 123 | + "into the script body -- which the check above would then have to catch -- or this test is " |
| 124 | + "now watching the wrong step." |
| 125 | + ) |
| 126 | + |
| 127 | + |
| 128 | +def test_the_merge_queue_parse_is_gated_on_an_event_a_fork_cannot_produce() -> None: |
| 129 | + """`head_branch` is the one attacker-influenceable field this workflow reads. |
| 130 | +
|
| 131 | + It is parsed only to recover the pull-request number from a merge-queue ref, and only when the |
| 132 | + triggering run was a `merge_group`. A fork pull request cannot produce that event, so a branch |
| 133 | + named to look like a queue ref never reaches the parse. |
| 134 | + """ |
| 135 | + resolve = _run_block("resolve") |
| 136 | + assert "HEAD_BRANCH" in resolve, ( |
| 137 | + "the resolve step no longer reads HEAD_BRANCH. If the merge-queue parse moved, move this " |
| 138 | + "assertion with it; the suppression in .github/zizmor.yml names this test by name." |
| 139 | + ) |
| 140 | + guard = re.search(r'if \[ -z "\$pr" \] && \[ "\$RUN_EVENT" = "merge_group" \]', resolve) |
| 141 | + assert guard is not None, ( |
| 142 | + "The merge-queue branch parse is no longer gated on RUN_EVENT = merge_group. Ungated, a " |
| 143 | + "crafted branch name could steer the label onto an unrelated pull request." |
| 144 | + ) |
| 145 | + |
| 146 | + |
| 147 | +def test_every_watched_workflow_exists() -> None: |
| 148 | + """A watched name that no workflow answers to is dead config that reads as coverage. |
| 149 | +
|
| 150 | + `workflow_run` matches on a workflow's `name:`, not its filename, so renaming one silently |
| 151 | + retires the watch -- no error, no run, permanent silence. That is the failure this signal exists |
| 152 | + to end, one level up. |
| 153 | +
|
| 154 | + It asserts EXISTENCE only. An earlier name for this test also claimed each watched workflow |
| 155 | + produces a required context, and that is false: `.github/required-contexts.txt` lists CodeQL |
| 156 | + under "DELIBERATELY NOT REQUIRED", because its SARIF upload needs a scope fork-PR tokens lack. |
| 157 | + Watching a non-required workflow is intentional -- a red CodeQL run is still worth attributing. |
| 158 | + """ |
| 159 | + watched = set(_on(_doc())["workflow_run"]["workflows"]) |
| 160 | + assert watched, "the watch list is empty, so every assertion below would pass against nothing" |
| 161 | + present = set() |
| 162 | + for path in WORKFLOWS.glob("*.yml"): |
| 163 | + doc = yaml.safe_load(path.read_text(encoding="utf-8")) or {} |
| 164 | + name = doc.get("name") |
| 165 | + if isinstance(name, str): |
| 166 | + present.add(name) |
| 167 | + # Positive control: the scan must actually be reading workflows, or `missing` below is just the |
| 168 | + # watch list back again and the failure message would blame the wrong file. |
| 169 | + assert len(present) > 5, f"the workflow scan found only {len(present)} named files" |
| 170 | + missing = watched - present |
| 171 | + assert missing == set(), ( |
| 172 | + f"failure-signal.yml watches names no workflow answers to: {missing}. " |
| 173 | + f"Names present: {sorted(present)}" |
| 174 | + ) |
| 175 | + |
| 176 | + |
| 177 | +def test_it_only_acts_on_a_real_failure() -> None: |
| 178 | + """A cancelled run is not a red. |
| 179 | +
|
| 180 | + Branch protection gates on the latest head, so a cancelled predecessor says nothing about the |
| 181 | + current one. Labelling on `cancelled` would train readers to ignore the label. |
| 182 | + """ |
| 183 | + condition = _doc()["jobs"]["signal"]["if"] |
| 184 | + assert "conclusion == 'failure'" in condition |
| 185 | + assert "cancelled" not in condition |
0 commit comments