timothyandClaude Fable 5.1
b71bf4d7b2
The shipped guard walked `jobs.<id>` only and leaned on a text-versus-walk
cross-check to catch anything the walk could not reach. That cross-check compared
per-file NAME SETS, and the two halves cancelled on the one file the invariant is
about: measured 2026-09-05 at 59003d5a3, hoisting
env:
ETV_REGISTRY_AUTH: ${{ secrets.REGISTRY_USER }}:${{ secrets.REGISTRY_PASSWORD }}
into `.gitea/workflows/docker-build.yml`'s root `env:` — which materialises into
EVERY job on the head-authored PR route — left `pytest
scripts/tests/test_workflow_persist_credentials.py -q` at `14 passed`, rc=0. The
same hoist in `pr-checks.yml` reddened, because no job there already names those
secrets. The guard could only ever see a name NO job used; a second copy of a
reference `build` legitimately keeps naming changed no set. That is
`dont-keep-a-copy-of-a-set` / `proof-sharing-with-subject-proves-nothing`: the
proof shared its accumulator with its subject and cancelled.
Two changes, because the cross-check was being asked to do the assertion's job:
* the workflow scope (everything outside `jobs:`) is now judged in its own right
by the same structure-blind collector — it is a second entry site on equal
footing with the job subtree, not an edge case, since no job-level `if:` can
take a root `env:`/`defaults:` off the route;
* the cross-check walks the whole document and compares occurrence COUNTS. A
duplicate at an unreachable location now reddens: probed 2026-09-05, a trailing
`# ${{ secrets.REGISTRY_PASSWORD }}` on a root `env:` line reports `walk
[('REGISTRY_PASSWORD', 1)] vs text [('REGISTRY_PASSWORD', 2)]` where the set
version agreed. Under counting the comment strip becomes load-bearing rather
than the no-op the old docstring admitted it was.
Driven by a mutation on the SHIPPED `docker-build.yml`, the way the `build`-loses-
its-`if:` mutation already is, plus a direct assertion on the two collectors that
a duplicated reference changes the count and not the names. Witnessed red with the
hoist in the tree (3 failed) and green without it (17 passed).
The decision record's own claims were false in the same way and are corrected:
`rule:` said "NO job ... may name a stored secret" (a root `env:` is not a job) and
the prose said "a text-versus-walk cross-check reports any reference the walk
cannot reach".
Refs #885
Decisions-Edit: yes
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV
744 lines
40 KiB
Python
744 lines
40 KiB
Python
"""Every `actions/checkout` drops the persisted credential (ersatztv#835).
|
|
|
|
WHAT THIS IS PROTECTING. `actions/checkout` writes an `Authorization` header into `.git/config`
|
|
unless `persist-credentials: false` is set, and this instance's Actions default token permission is
|
|
`permissive` — so that header is write-capable, and every later step in the job inherits it, not
|
|
just the checkout. ersatztv#746 set the flag on every checkout but one; nothing held the convention
|
|
afterwards. A job added tomorrow gets a checkout without the flag, and **nothing goes
|
|
red** — the silent-by-construction shape `docs/defect-shapes-773.md` §4 exists to catch.
|
|
|
|
WHY THERE IS NO EXEMPTION LIST. `ci-image.yml`'s checkout was the one left unset, for a mechanical
|
|
reason in another issue (#744), and this guard was deliberately held back rather than shipped with a
|
|
one-entry exemption for it: an exemption outlives its reason silently — once #744 set the flag there,
|
|
the entry would still have passed and the guard would have been permanently blind to the very file it
|
|
was written for. #744 landed with this, so the assertion is universal over the derived population.
|
|
|
|
POPULATION. The git index, via `tracked_files.tracked_paths` — not a filesystem walk and not a
|
|
hand-written list (`testing.guard-derives-population-from-source`, ersatztv#806). Both `*.yml` and
|
|
`*.yaml` are matched: Gitea accepts either spelling, so a `.yaml` workflow would otherwise be
|
|
structurally invisible to a check that reads as covering all of them.
|
|
|
|
`.github/workflows` is OUT of the population and is asserted empty rather than assumed so: Gitea
|
|
reads it only when `.gitea/workflows` is absent, which is a precedence rule this repo has not probed,
|
|
and `.github/` already exists here (ISSUE_TEMPLATE), so it is a plausible place to add a workflow by
|
|
habit. `test_no_workflow_hides_in_a_SUBDIRECTORY` covers nesting below `.gitea/workflows`.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import re
|
|
from collections import Counter
|
|
from pathlib import Path
|
|
|
|
import yaml
|
|
|
|
from scripts.tests.tracked_files import _git_ls_files, tracked_paths
|
|
|
|
WORKFLOW_DIR = ".gitea/workflows"
|
|
WORKFLOWS = (WORKFLOW_DIR, ("*.yml", "*.yaml"))
|
|
|
|
# Compared LOWERCASE throughout: Gitea resolves an action ref through a forge that is
|
|
# case-insensitive on owner/repo, so `uses: Actions/Checkout@v4` runs the real checkout. A
|
|
# case-sensitive comparison missed it in BOTH halves identically, so the count cross-check agreed
|
|
# and the guard reported clean over a step that persists the credential.
|
|
CHECKOUT = "actions/checkout"
|
|
|
|
# Matches the `uses:` line the YAML walk is supposed to reach, in every spelling below. Used ONLY as
|
|
# an independent second opinion on how many there are — never as the population itself.
|
|
_USES_CHECKOUT = re.compile(
|
|
r"^\s*-?\s*uses:\s*['\"]?(?:[^'\"\s]*/)?actions/checkout(?:\.git)?(@|\s|['\"]|$)",
|
|
re.M | re.I,
|
|
)
|
|
|
|
|
|
def _canonical_action(uses: str) -> str:
|
|
"""The last two path segments of a `uses:` value, without its `@version`.
|
|
|
|
Gitea accepts a full action URL as well as `owner/repo`, and the host in one is NOT required to
|
|
contain a dot — `http://your-git-server/actions/checkout@v4` is documented and valid. So this
|
|
takes the last two segments rather than trying to recognise a hostname: every spelling of the
|
|
same action canonicalises to `actions/checkout`, with no host heuristic to get wrong.
|
|
|
|
It over-matches in one direction on purpose: a genuinely different action whose path happens to
|
|
END in `actions/checkout` would be treated as a checkout and required to carry the flag. That
|
|
costs a spurious requirement on an action nobody has; the opposite error costs a live credential.
|
|
Spellings normalised here, each of which escaped a simpler normalisation: an absolute
|
|
URL with or without a scheme, a host with no dot, a doubled slash, `@`-userinfo before the host,
|
|
a `.git` suffix, and any letter case.
|
|
"""
|
|
# rsplit, not split: a URL may carry userinfo (`https://user@host/actions/checkout@v4`), and
|
|
# splitting on the FIRST `@` would strip the host instead of the version.
|
|
ref = uses.strip().strip("'\"").rsplit("@", 1)[0].strip().lower()
|
|
ref = re.sub(r"^https?://", "", ref)
|
|
# Empty segments absorb a doubled slash; `.git` is how Gitea itself writes the clone URL.
|
|
segments = [seg for seg in ref.split("/") if seg]
|
|
if segments:
|
|
segments[-1] = re.sub(r"\.git$", "", segments[-1])
|
|
if ref.startswith("./") or ref.startswith("../") or len(segments) < 2:
|
|
return "/".join(segments) if segments else ref
|
|
return "/".join(segments[-2:])
|
|
|
|
|
|
def workflow_files() -> list[Path]:
|
|
return tracked_paths(*WORKFLOWS)
|
|
|
|
|
|
def _checkout_steps(doc: object) -> list[tuple[str, int, dict]]:
|
|
"""Every `actions/checkout` step in a parsed workflow, as (job id, index within job, step).
|
|
|
|
Reads `jobs.<id>.steps` only, which is the whole shape this repo uses. Different escapes are
|
|
reported by different tests, and they are not interchangeable:
|
|
`test_the_walk_finds_every_actions_checkout_the_TEXT_does` covers a shape INSIDE a workflow file
|
|
that the walk cannot reach, because both halves read the same files. A **composite action** is
|
|
invisible to both halves — its `action.yml` is not in the workflow population at all — so that
|
|
case is reported by `test_no_LOCAL_COMPOSITE_ACTION_exists_AT_ALL` instead. A **reusable
|
|
workflow** (`jobs.<id>.uses:`) is a third shape, invisible to both halves in the same way, and is
|
|
reported by `test_no_job_DELEGATES_to_a_reusable_workflow` — it is not covered by the text
|
|
cross-check, which would see zero on both sides and agree.
|
|
"""
|
|
found: list[tuple[str, int, dict]] = []
|
|
if not isinstance(doc, dict):
|
|
return found
|
|
jobs = doc.get("jobs")
|
|
if not isinstance(jobs, dict):
|
|
return found
|
|
for job_id, job in jobs.items():
|
|
if not isinstance(job, dict):
|
|
continue
|
|
steps = job.get("steps")
|
|
if not isinstance(steps, list):
|
|
continue
|
|
for index, step in enumerate(steps):
|
|
if not isinstance(step, dict):
|
|
continue
|
|
uses = step.get("uses")
|
|
if isinstance(uses, str) and _canonical_action(uses) == CHECKOUT:
|
|
found.append((str(job_id), index, step))
|
|
return found
|
|
|
|
|
|
def checkout_faults(rel: str, doc: object) -> list[str]:
|
|
"""Human-readable faults for one workflow — accumulated, not failed fast.
|
|
|
|
A missing key and an explicit `true` are the same defect and are reported the same way: what
|
|
matters is whether the credential is left behind, not how the step spelled it.
|
|
"""
|
|
faults: list[str] = []
|
|
for job_id, index, step in _checkout_steps(doc):
|
|
with_block = step.get("with")
|
|
value = with_block.get("persist-credentials") if isinstance(with_block, dict) else None
|
|
if value is False:
|
|
continue
|
|
how = "does not set it at all" if value is None else f"sets it to {value!r}"
|
|
faults.append(
|
|
f"{rel}: job `{job_id}` step #{index} uses {CHECKOUT} and {how} — so the action "
|
|
f"persists a write-capable Authorization header into .git/config, and every later "
|
|
f"step in that job inherits it. Add `persist-credentials: false` under `with:` "
|
|
f"(ersatztv#746, guarded by ersatztv#835). There is deliberately no exemption list: "
|
|
f"if a checkout genuinely needs the credential, say so in the step and change this "
|
|
f"guard in the same PR."
|
|
)
|
|
return faults
|
|
|
|
|
|
def test_every_actions_checkout_DROPS_the_persisted_credential() -> None:
|
|
faults: list[str] = []
|
|
for path in workflow_files():
|
|
rel = path.relative_to(Path(__file__).resolve().parents[2]).as_posix()
|
|
faults.extend(checkout_faults(rel, yaml.safe_load(path.read_text())))
|
|
assert not faults, "\n".join(faults)
|
|
|
|
|
|
def test_the_walk_finds_every_actions_checkout_the_TEXT_does() -> None:
|
|
"""The YAML walk and a plain text scan must agree on the count, per file.
|
|
|
|
Without this, a step list the walk cannot reach — a composite action, a reusable workflow, a
|
|
shape act_runner grows later — makes checkouts structurally invisible while the guard above
|
|
still reports a clean run. Absence of output is not evidence; make the system report it.
|
|
"""
|
|
disagreements: list[str] = []
|
|
for path in workflow_files():
|
|
text = path.read_text()
|
|
walked = len(_checkout_steps(yaml.safe_load(text)))
|
|
scanned = len(_USES_CHECKOUT.findall(text))
|
|
if walked != scanned:
|
|
disagreements.append(
|
|
f"{path.name}: the jobs.<id>.steps walk found {walked} {CHECKOUT} step(s) but the "
|
|
f"text scan found {scanned}. The walk is what the flag assertion runs over, so the "
|
|
f"difference is checkouts this guard cannot see — widen `_checkout_steps`."
|
|
)
|
|
assert not disagreements, "\n".join(disagreements)
|
|
|
|
|
|
def test_the_population_and_the_checkout_set_are_NOT_empty() -> None:
|
|
"""Anti-vacuity, in both directions.
|
|
|
|
An empty population, or a population of workflows in which the walk finds no checkout at all,
|
|
makes the assertion above pass while measuring nothing — which is the failure this guard exists
|
|
to prevent, arriving through the guard itself.
|
|
"""
|
|
files = workflow_files()
|
|
assert files, (
|
|
"`git ls-files` reported no .gitea/workflows/*.yml — the derivation is broken, not the repo, "
|
|
"and every assertion in this file would pass vacuously."
|
|
)
|
|
total = sum(len(_checkout_steps(yaml.safe_load(p.read_text()))) for p in files)
|
|
assert total > 0, (
|
|
f"Parsed {len(files)} workflow(s) and found no {CHECKOUT} step in any of them. Either the "
|
|
"repo really has none (then delete this guard and say why) or `_checkout_steps` stopped "
|
|
"matching, in which case the flag assertion is measuring an empty set."
|
|
)
|
|
|
|
|
|
def test_the_fault_collector_reports_a_checkout_that_OMITS_the_flag() -> None:
|
|
"""Negative control on the collector itself, over both defect spellings.
|
|
|
|
The mutation proof (`mutation_manifest.py`) drives the real tree; this drives the collector
|
|
directly so a collector that silently stopped reporting is caught without waiting for the
|
|
harness, and so `true` is covered as well as an absent key.
|
|
"""
|
|
omitted = {"jobs": {"j": {"steps": [{"uses": "actions/checkout@v4"}]}}}
|
|
explicit_true = {"jobs": {"j": {"steps": [{"uses": "actions/checkout@v4", "with": {"persist-credentials": True}}]}}}
|
|
compliant = {"jobs": {"j": {"steps": [{"uses": "actions/checkout@v4", "with": {"persist-credentials": False}}]}}}
|
|
assert len(checkout_faults("synthetic.yml", omitted)) == 1
|
|
assert "does not set it at all" in checkout_faults("synthetic.yml", omitted)[0]
|
|
assert len(checkout_faults("synthetic.yml", explicit_true)) == 1
|
|
assert "sets it to True" in checkout_faults("synthetic.yml", explicit_true)[0]
|
|
assert checkout_faults("synthetic.yml", compliant) == []
|
|
|
|
# Gitea accepts a full action URL, so the same step spelled as a URL must not escape either —
|
|
# pinned rather than probed once, because a canonicalisation that quietly stopped working would
|
|
# leave the guard green over a checkout it no longer recognises as one.
|
|
for spelling in (
|
|
"https://github.com/actions/checkout@v4",
|
|
# A Gitea action host is not required to contain a dot — this is the documented
|
|
# self-hosted spelling, and a dot-requiring canonicaliser let it through silently.
|
|
"http://your-git-server/actions/checkout@v4",
|
|
"github.com/actions/checkout@v4",
|
|
# The forge is case-insensitive on owner/repo, so this runs the real checkout.
|
|
"Actions/Checkout@v4",
|
|
# Gitea writes the clone URL with the suffix; both halves must accept it.
|
|
"https://github.com/actions/checkout.git@v4",
|
|
# Userinfo before the host — splitting on the FIRST `@` used to strip the host.
|
|
"https://user@github.com/actions/checkout@v4",
|
|
"https://github.com//actions/checkout@v4",
|
|
):
|
|
as_url = {"jobs": {"j": {"steps": [{"uses": spelling}]}}}
|
|
assert len(checkout_faults("synthetic.yml", as_url)) == 1, spelling
|
|
assert _USES_CHECKOUT.search(f" - uses: {spelling}"), spelling
|
|
assert not _USES_CHECKOUT.search(" - uses: actions/setup-node@v4")
|
|
|
|
|
|
def test_no_workflow_hides_in_a_SUBDIRECTORY() -> None:
|
|
"""The population is direct children of `.gitea/workflows`; prove nothing sits below it.
|
|
|
|
`tracked_children` is direct-children-only by design (see `tracked_files`), which is right for a
|
|
flat directory and blind the moment one stops being flat. This does not widen the population —
|
|
it makes the assumption REPORT itself, so a nested workflow reddens the guard instead of being
|
|
silently uncovered, and it covers the two sibling guards that derive the same directory the same
|
|
way (`test_ci_image_pin_population.py`, `test_pr_changed_files.py`).
|
|
"""
|
|
nested = sorted(
|
|
path for path in _git_ls_files() if path.startswith(f"{WORKFLOW_DIR}/") and "/" in path[len(WORKFLOW_DIR) + 1 :]
|
|
)
|
|
assert not nested, (
|
|
f"tracked workflow file(s) below {WORKFLOW_DIR}/: {nested}. The population here is direct "
|
|
"children only, so these are NOT checked for `persist-credentials: false` — widen the "
|
|
"derivation (and the sibling guards that share it) before adding them."
|
|
)
|
|
|
|
|
|
def test_no_LOCAL_COMPOSITE_ACTION_exists_AT_ALL() -> None:
|
|
"""A checkout inside a local composite action runs in the job but is outside the workflow set.
|
|
|
|
The repo has no `action.yml`/`action.yaml` today, so rather than write a parser for a shape that
|
|
does not exist, assert the absence: the first one added reddens this and its author widens the
|
|
guard, instead of the guard reading as complete while a whole class walked around it.
|
|
"""
|
|
actions = sorted(path for path in _git_ls_files() if path.rpartition("/")[2] in {"action.yml", "action.yaml"})
|
|
assert not actions, (
|
|
f"local composite action definition(s) found: {actions}. A composite action's steps run in "
|
|
"the job, so an `actions/checkout` in one is subject to the same rule and is NOT covered by "
|
|
"the workflow-file population above — extend `_checkout_steps` to walk them."
|
|
)
|
|
|
|
|
|
def test_no_job_DELEGATES_to_a_reusable_workflow() -> None:
|
|
"""A `jobs.<id>.uses:` callee's steps run in the job but are outside the walk.
|
|
|
|
Both halves would see zero and AGREE, so the count cross-check reports nothing — the same blind
|
|
spot the composite-action test exists for. Gitea Actions has no `workflow_call` today, so assert
|
|
the absence rather than parse a shape that cannot occur: the first one added reddens here and its
|
|
author widens the guard.
|
|
"""
|
|
delegating = []
|
|
for path in workflow_files():
|
|
doc = yaml.safe_load(path.read_text())
|
|
jobs = doc.get("jobs") if isinstance(doc, dict) else None
|
|
if not isinstance(jobs, dict):
|
|
continue
|
|
for job_id, job in jobs.items():
|
|
if isinstance(job, dict) and isinstance(job.get("uses"), str):
|
|
delegating.append(f"{path.name}: job `{job_id}` -> {job['uses']}")
|
|
assert not delegating, (
|
|
f"job(s) delegating to a reusable workflow: {delegating}. The callee's `actions/checkout` "
|
|
"steps run in this job and are subject to the same rule, but are NOT in the population "
|
|
"above — extend the walk before adding one."
|
|
)
|
|
|
|
|
|
def test_no_workflow_lives_under_dot_GITHUB() -> None:
|
|
"""`.github/workflows` is out of the population, so prove it is empty rather than assume it.
|
|
|
|
Gitea reads `.github/workflows` only when `.gitea/workflows` is absent — a precedence rule this
|
|
repo has never probed — and `.github/` already exists here, so it is a plausible place to add a
|
|
workflow out of habit. Cheap to assert; silently uncovered otherwise.
|
|
"""
|
|
stray = sorted(p for p in _git_ls_files() if p.startswith(".github/workflows/"))
|
|
assert not stray, (
|
|
f"tracked workflow file(s) under .github/workflows: {stray}. This guard's population is "
|
|
f"{WORKFLOW_DIR} only — decide whether Gitea runs these and widen the population or delete "
|
|
"them, but do not leave them unchecked."
|
|
)
|
|
|
|
|
|
# =================================================================================================
|
|
# NO JOB ON THE `pull_request` ROUTE NAMES A STORED SECRET (ersatztv#885)
|
|
#
|
|
# The second credential invariant over the same derived population, and the same shape of defect one
|
|
# step out: `persist-credentials` is about a credential a step LEAVES BEHIND, this is about a
|
|
# credential the workflow ASKS FOR. Gitea resolves a `pull_request` run from the PR HEAD, so on that
|
|
# route the YAML is authored by the contributor, and every `secrets.*` it names is materialised into
|
|
# the run environment. Six jobs in `docker-build.yml` held `REGISTRY_PASSWORD` that way, two of them
|
|
# branch-protection required contexts.
|
|
#
|
|
# WHY THE PREDICATE IS "names a stored secret", NOT "is a `container:` job". The issue's own first
|
|
# statement of the invariant was the latter, and it was wrong: `toolchain-preflight` is deliberately
|
|
# container-free and took the credential through a step `env:` instead, so that predicate named five
|
|
# of six and would have gone stale the day it shipped. A population derived by the WRONG predicate is
|
|
# not better than a hand-written list — it is a list with a false claim of completeness attached.
|
|
#
|
|
# WHY `pull_request_target` IS NOT IN THIS POPULATION. That trigger is BASE-resolved
|
|
# (`ci.gate-trigger-base-resolved`): the YAML that runs is `main`'s, not the head's, which is exactly
|
|
# why `review-verdict.yml` uses it to hold a write-capable token. The exposure here is head-authored
|
|
# YAML, so the trigger that is not head-authored is out — and a workflow that adds `pull_request`
|
|
# alongside it enters the population on that key alone.
|
|
#
|
|
# WHAT THIS DOES NOT CLOSE, so no reader mistakes it for a boundary: `REGISTRY_PASSWORD` is still in
|
|
# the repo's Actions store for `build`, and head-supplied YAML can still NAME it, or `RENOVATE_TOKEN`,
|
|
# or `SERVERMGMT_DEPLOY_KEY`. What is removed is the ROUTINE materialisation of a write-capable
|
|
# credential into six PR-run environments. Bounding the store itself needs per-environment secret
|
|
# scoping, which Gitea 1.27.1 does not have (probed in ersatztv#853).
|
|
# =================================================================================================
|
|
|
|
# The repo's Actions secret store, read 2026-09-04: GH_COM_TOKEN, REGISTRY_PASSWORD, REGISTRY_USER,
|
|
# RENOVATE_TOKEN, SERVERMGMT_DEPLOY_KEY. `GITEA_TOKEN` is deliberately NOT one of them — it is the
|
|
# per-run token Gitea injects, bounded by the workflow's own `permissions:` block, and a head-authored
|
|
# run receives it whether or not any job names it. Allow-listing it is therefore a statement about a
|
|
# MECHANISM (injected, scoped, unavoidable) and not an exemption for a site, which is why it is a
|
|
# closed one-member set rather than a list that can grow: a second entry would be an exemption, and
|
|
# an exemption outlives its reason silently.
|
|
INJECTED_SECRETS = frozenset({"GITEA_TOKEN"})
|
|
|
|
PULL_REQUEST = "pull_request"
|
|
|
|
_SECRET_REF = re.compile(r"secrets\.([A-Za-z_][A-Za-z0-9_]*)")
|
|
|
|
# The ONLY job-level `if:` in this repo that takes a job OFF the `pull_request` route. This is a PIN,
|
|
# not an expression parser, and the direction is the point: an `if:` that is not in this set leaves
|
|
# the job IN the population, so an unrecognised guard reddens rather than exempting. Parsing
|
|
# `github.event_name` expressions was tried elsewhere and withdrawn after repeated defects from that
|
|
# one mechanism (`test_image_build_delegates_the_spa_suite`); a pin has no such failure mode, because
|
|
# the only way to get it wrong is to be too demanding.
|
|
PR_EXCLUDING_IFS = frozenset({"github.event_name != 'pull_request'"})
|
|
|
|
|
|
def _unwrap_expression(value: str) -> str:
|
|
"""`${{ x }}` -> `x`; anything else unchanged, whitespace-trimmed."""
|
|
stripped = value.strip()
|
|
if stripped.startswith("${{") and stripped.endswith("}}"):
|
|
stripped = stripped[3:-2]
|
|
return stripped.strip()
|
|
|
|
|
|
def workflow_triggers(doc: object) -> set[str]:
|
|
"""The trigger names under `on:`, in all three spellings it can take.
|
|
|
|
`on` is read back from `yaml.safe_load` as the BOOLEAN `True`, not the string `"on"` — YAML 1.1
|
|
resolves a bare `on` to a boolean, and PyYAML implements 1.1. A `doc.get("on")` here returns
|
|
None for every workflow in this repo, which would empty the population and make every assertion
|
|
below pass having measured nothing. Both keys are read so the function survives a loader that
|
|
resolves it either way.
|
|
"""
|
|
if not isinstance(doc, dict):
|
|
return set()
|
|
on = doc.get(True)
|
|
if on is None:
|
|
on = doc.get("on")
|
|
if isinstance(on, str):
|
|
return {on}
|
|
if isinstance(on, list):
|
|
return {str(item) for item in on}
|
|
if isinstance(on, dict):
|
|
return {str(key) for key in on}
|
|
return set()
|
|
|
|
|
|
def runs_on_pull_request(doc: object) -> bool:
|
|
return PULL_REQUEST in workflow_triggers(doc)
|
|
|
|
|
|
def pull_request_jobs(doc: object) -> list[tuple[str, dict]]:
|
|
"""Every job of a `pull_request`-triggered workflow that the trigger can actually reach."""
|
|
jobs = doc.get("jobs") if isinstance(doc, dict) else None
|
|
if not isinstance(jobs, dict):
|
|
return []
|
|
reachable: list[tuple[str, dict]] = []
|
|
for job_id, job in jobs.items():
|
|
if not isinstance(job, dict):
|
|
continue
|
|
condition = job.get("if")
|
|
if isinstance(condition, str) and _unwrap_expression(condition) in PR_EXCLUDING_IFS:
|
|
continue
|
|
reachable.append((str(job_id), job))
|
|
return reachable
|
|
|
|
|
|
def secret_names(node: object) -> set[str]:
|
|
"""Every `secrets.NAME` reachable anywhere in a subtree — keys and values, at any depth.
|
|
|
|
Deliberately structure-blind: the credential entered through `container.credentials`, a step
|
|
`env:`, and a job `env:` in this repo already, and naming those three places would be the
|
|
hand-written-population mistake in a different coat.
|
|
"""
|
|
found: set[str] = set()
|
|
stack: list[object] = [node]
|
|
while stack:
|
|
item = stack.pop()
|
|
if isinstance(item, dict):
|
|
for key, value in item.items():
|
|
stack.append(key)
|
|
stack.append(value)
|
|
elif isinstance(item, list):
|
|
stack.extend(item)
|
|
elif isinstance(item, str):
|
|
found.update(_SECRET_REF.findall(item))
|
|
return found
|
|
|
|
|
|
def secret_name_counts(node: object) -> Counter:
|
|
"""`secret_names`, counting OCCURRENCES rather than collapsing them to a set of names.
|
|
|
|
Same traversal, different accumulator, so the cross-check below measures the same walk the
|
|
assertion runs on. Kept beside `secret_names` rather than replacing it: the fault collector
|
|
genuinely wants names (it reports which secret a job holds, once), and only the cross-check
|
|
needs locations.
|
|
"""
|
|
found: Counter = Counter()
|
|
stack: list[object] = [node]
|
|
while stack:
|
|
item = stack.pop()
|
|
if isinstance(item, dict):
|
|
for key, value in item.items():
|
|
stack.append(key)
|
|
stack.append(value)
|
|
elif isinstance(item, list):
|
|
stack.extend(item)
|
|
elif isinstance(item, str):
|
|
found.update(_SECRET_REF.findall(item))
|
|
return found
|
|
|
|
|
|
def outside_jobs(doc: object) -> dict:
|
|
"""Everything in a workflow document EXCEPT `jobs:` — the workflow scope.
|
|
|
|
A root `env:` is materialised into every job, and `defaults:` likewise; neither is reachable by
|
|
a walk that starts at `jobs.<id>`, and no job-level `if:` can take a workflow-scope reference off
|
|
the route. So the scope is a SECOND site the credential can enter through, on equal footing with
|
|
the job subtree, and it is walked by the same structure-blind collector rather than by naming
|
|
`env:` and `defaults:` — naming them would reproduce the hand-written-population mistake the
|
|
job-level predicate already avoids (`testing.guard-derives-population-from-source`).
|
|
"""
|
|
if not isinstance(doc, dict):
|
|
return {}
|
|
return {key: value for key, value in doc.items() if key != "jobs"}
|
|
|
|
|
|
def stored_secret_faults(rel: str, doc: object) -> list[str]:
|
|
"""Human-readable faults for one workflow — accumulated, not failed fast."""
|
|
faults: list[str] = []
|
|
if not runs_on_pull_request(doc):
|
|
return faults
|
|
workflow_scope = sorted(secret_names(outside_jobs(doc)) - INJECTED_SECRETS)
|
|
if workflow_scope:
|
|
faults.append(
|
|
f"{rel}: the WORKFLOW SCOPE (outside `jobs:`) names stored secret(s) on the "
|
|
f"pull_request route: {', '.join(workflow_scope)}. A root `env:` or `defaults:` is "
|
|
f"materialised into EVERY job, so no job-level "
|
|
f"`if: github.event_name != 'pull_request'` can take it off the head-authored route. "
|
|
f"Move the reference into a job that is gated off the route the way `build` is, or "
|
|
f"take the credential out entirely (the toolchain image pulls anonymously and the "
|
|
f"commit-status API answers unauthenticated). There is deliberately no exemption "
|
|
f"list: `ci.pr-route-carries-no-stored-credential`."
|
|
)
|
|
for job_id, job in pull_request_jobs(doc):
|
|
named = sorted(secret_names(job) - INJECTED_SECRETS)
|
|
if named:
|
|
faults.append(
|
|
f"{rel}: job `{job_id}` names stored secret(s) on the pull_request route: "
|
|
f"{', '.join(named)}. Gitea resolves a `pull_request` run from the PR HEAD, so this "
|
|
f"YAML is contributor-authored and every secret it names is handed to that run. "
|
|
f"Take the credential out of the job (the toolchain image pulls anonymously and the "
|
|
f"commit-status API answers unauthenticated), or gate the job off the route with "
|
|
f"`if: github.event_name != 'pull_request'` the way `build` is. There is "
|
|
f"deliberately no exemption list: `ci.pr-route-carries-no-stored-credential`."
|
|
)
|
|
return faults
|
|
|
|
|
|
def test_no_PULL_REQUEST_route_job_names_a_STORED_secret() -> None:
|
|
faults: list[str] = []
|
|
for path in workflow_files():
|
|
rel = path.relative_to(Path(__file__).resolve().parents[2]).as_posix()
|
|
faults.extend(stored_secret_faults(rel, yaml.safe_load(path.read_text())))
|
|
assert not faults, "\n".join(faults)
|
|
|
|
|
|
def test_the_DOCUMENT_walk_finds_every_secret_reference_the_TEXT_does() -> None:
|
|
"""The document walk and a plain text scan must agree per file, COUNTED not merely named.
|
|
|
|
The two halves of the assertion above — the workflow scope and each reachable job — are both
|
|
reached from the document root, so the cross-check is over the whole document rather than over
|
|
`jobs.<id>`. A reference the walk cannot reach at all (a shape act_runner grows later, a key the
|
|
loader drops) is then a disagreement, and the guard says so instead of reporting clean.
|
|
|
|
IT COUNTS OCCURRENCES, IT DOES NOT COMPARE NAME SETS, and that is the load-bearing part rather
|
|
than a refinement. Measured 2026-09-05 on the shipped tree at 59003d5a3: hoisting
|
|
`ETV_REGISTRY_AUTH: ${{ secrets.REGISTRY_USER }}:${{ secrets.REGISTRY_PASSWORD }}` into
|
|
`docker-build.yml`'s root `env:` left a set-comparison cross-check GREEN (`14 passed`), because
|
|
`build` legitimately keeps naming both names — the two halves cancelled and the check could only
|
|
ever see a name NO job already used. That is the `dont-keep-a-copy-of-a-set` /
|
|
`proof-sharing-with-subject-proves-nothing` shape: a set says a name appears SOMEWHERE, which is
|
|
exactly the fact a second copy of the same reference does not change. A count changes.
|
|
|
|
COMMENT-ONLY LINES ARE STRIPPED first, so that a comment DISCUSSING a secret name does not read
|
|
as a reference the walk missed — `review-verdict.yml` names `secrets.GITEA_TOKEN` in prose, and
|
|
under counting that strip IS load-bearing (a set comparison forgave it; a count does not).
|
|
|
|
Two blind spots the strip has, stated rather than left to be discovered:
|
|
* it is LINE-level, so a TRAILING comment (`foo: bar # secrets.X`) survives into the text
|
|
half. The text count is then larger than the walk's, which REDDENS — the safe direction;
|
|
* conversely a `#`-prefixed line inside a `run: |` block is a shell comment to a reader and
|
|
part of the YAML scalar to the walk, so the strip removes it from the text half only and
|
|
the walk's count becomes the larger one. That reddens too, for a benign cause; reword the
|
|
comment rather than widening the strip, which would start hiding real references.
|
|
|
|
A YAML anchor/alias would also redden benignly (the walk visits the aliased node once per
|
|
reference, the text carries `*alias`); measured 2026-09-05 no tracked workflow uses one.
|
|
"""
|
|
disagreements: list[str] = []
|
|
total = Counter()
|
|
for path in workflow_files():
|
|
text = path.read_text()
|
|
walked = secret_name_counts(yaml.safe_load(text))
|
|
total += walked
|
|
scanned = Counter(
|
|
_SECRET_REF.findall("\n".join(line for line in text.splitlines() if not line.lstrip().startswith("#")))
|
|
)
|
|
if walked != scanned:
|
|
disagreements.append(
|
|
f"{path.name}: the document walk reached {sorted(walked.items())} but the text scan "
|
|
f"found {sorted(scanned.items())}. The walk is what the stored-secret assertion runs "
|
|
f"over, so the difference is secret references this guard cannot see — widen "
|
|
f"`secret_names`' entry point, or take the reference out of the file."
|
|
)
|
|
assert not disagreements, "\n".join(disagreements)
|
|
assert sum(total.values()) > 0, (
|
|
"no workflow reached by this check names a single `secrets.*` reference. Two empty Counters "
|
|
"compare equal, so the loop above would agree having measured nothing — either every "
|
|
"reference really is gone (say so here and in `ci.pr-route-carries-no-stored-credential`) "
|
|
"or `workflow_files()`/`_SECRET_REF` has stopped matching."
|
|
)
|
|
|
|
|
|
def test_the_cross_check_COUNTS_locations_rather_than_collecting_NAMES() -> None:
|
|
"""The property that distinguishes this check from the one it replaced, asserted directly.
|
|
|
|
A duplicated reference — the same secret named at a second location — changes no NAME and
|
|
changes the COUNT. That difference is the entire reason a workflow-level hoist of a name `build`
|
|
already uses was invisible before, so it is pinned on the collectors themselves rather than left
|
|
to be inferred from the mutation tests.
|
|
"""
|
|
once = {"env": {"A": "${{ secrets.REGISTRY_PASSWORD }}"}}
|
|
twice = {"env": {"A": "${{ secrets.REGISTRY_PASSWORD }}", "B": "${{ secrets.REGISTRY_PASSWORD }}"}}
|
|
|
|
assert secret_names(once) == secret_names(twice) == {"REGISTRY_PASSWORD"}
|
|
assert secret_name_counts(once) == Counter({"REGISTRY_PASSWORD": 1})
|
|
assert secret_name_counts(twice) == Counter({"REGISTRY_PASSWORD": 2})
|
|
assert secret_name_counts(once) != secret_name_counts(twice)
|
|
|
|
# Two references in ONE scalar count twice, which is the shape a hoisted
|
|
# `user:${{ secrets.X }}:${{ secrets.Y }}` line takes.
|
|
pair = {"env": {"A": "${{ secrets.REGISTRY_USER }}:${{ secrets.REGISTRY_PASSWORD }}"}}
|
|
assert secret_name_counts(pair) == Counter({"REGISTRY_USER": 1, "REGISTRY_PASSWORD": 1})
|
|
|
|
|
|
def test_the_PULL_REQUEST_populations_are_NOT_empty() -> None:
|
|
"""Anti-vacuity in three directions, because each one alone can empty the assertion silently."""
|
|
docs = [(p, yaml.safe_load(p.read_text())) for p in workflow_files()]
|
|
pr_workflows = [(p, d) for p, d in docs if runs_on_pull_request(d)]
|
|
assert pr_workflows, (
|
|
"no tracked workflow was read as triggering on `pull_request`. Either the repo really has "
|
|
"none, or `workflow_triggers` has stopped resolving the `on:` key — note YAML 1.1 gives it "
|
|
"back as the boolean True. Every assertion above would pass having measured nothing."
|
|
)
|
|
reachable = sum(len(pull_request_jobs(d)) for _, d in pr_workflows)
|
|
assert reachable > 0, (
|
|
f"{len(pr_workflows)} workflow(s) trigger on `pull_request` and not one reachable job was "
|
|
"found — `pull_request_jobs` is excluding everything, so the guard measures an empty set."
|
|
)
|
|
named = set()
|
|
for _, doc in pr_workflows:
|
|
jobs = doc.get("jobs") if isinstance(doc, dict) else None
|
|
if isinstance(jobs, dict):
|
|
named |= secret_names(jobs)
|
|
assert "REGISTRY_PASSWORD" in named, (
|
|
"`REGISTRY_PASSWORD` is no longer named anywhere in the pull_request-triggered workflows. "
|
|
"It is supposed to survive in `build`, which is gated off the route — if the secret was "
|
|
"renamed, this guard has quietly become a check on a string nothing uses, so update the "
|
|
"name here in the same commit."
|
|
)
|
|
|
|
|
|
def test_a_job_whose_IF_is_UNRECOGNISED_stays_on_the_pull_request_route() -> None:
|
|
"""The fail-closed direction of the `if:` pin, driven rather than assumed.
|
|
|
|
`PR_EXCLUDING_IFS` is one string. Every other condition — including ones that a human can see
|
|
exclude the route — must leave the job in the population, so that widening the pin is a
|
|
deliberate edit and never an accident of expression parsing.
|
|
"""
|
|
for condition in (
|
|
"github.event_name == 'push'",
|
|
"${{ github.event_name != 'pull_request' && true }}",
|
|
"github.ref == 'refs/heads/main'",
|
|
"always()",
|
|
):
|
|
job = {"if": condition, "env": {"X": "${{ secrets.REGISTRY_PASSWORD }}"}}
|
|
doc = {True: ["pull_request"], "jobs": {"j": job}}
|
|
assert [job_id for job_id, _ in pull_request_jobs(doc)] == ["j"], condition
|
|
assert len(stored_secret_faults("synthetic.yml", doc)) == 1, condition
|
|
|
|
gated = {"if": "${{ github.event_name != 'pull_request' }}", "env": {"X": "${{ secrets.REGISTRY_PASSWORD }}"}}
|
|
excluded = {True: ["pull_request"], "jobs": {"j": gated}}
|
|
assert pull_request_jobs(excluded) == []
|
|
assert stored_secret_faults("synthetic.yml", excluded) == []
|
|
|
|
|
|
def test_the_stored_secret_collector_reports_a_pull_request_job_naming_ONE() -> None:
|
|
"""Negative control on the collector, over every shape the credential actually entered through."""
|
|
for job in (
|
|
{"container": {"image": "x", "credentials": {"username": "${{ secrets.REGISTRY_USER }}"}}},
|
|
{"env": {"A": "${{ secrets.REGISTRY_PASSWORD }}"}},
|
|
{"steps": [{"run": "x", "env": {"A": "${{ secrets.RENOVATE_TOKEN }}"}}]},
|
|
{"steps": [{"run": "echo ${{ secrets.SERVERMGMT_DEPLOY_KEY }}"}]},
|
|
):
|
|
doc = {True: {"pull_request": None}, "jobs": {"j": job}}
|
|
faults = stored_secret_faults("synthetic.yml", doc)
|
|
assert len(faults) == 1, job
|
|
assert "names stored secret(s) on the pull_request route" in faults[0]
|
|
|
|
# The injected token is not a stored secret and must not be reported.
|
|
injected = {True: {"pull_request": None}, "jobs": {"j": {"env": {"A": "${{ secrets.GITEA_TOKEN }}"}}}}
|
|
assert stored_secret_faults("synthetic.yml", injected) == []
|
|
|
|
# A workflow that never triggers on `pull_request` is out of the population entirely, and
|
|
# `pull_request_target` does NOT put it back in: that trigger is base-resolved, so the YAML that
|
|
# runs is `main`'s rather than the head's.
|
|
for trigger in ({"push": None}, {"pull_request_target": None}, {"workflow_dispatch": None}):
|
|
off_route = {True: trigger, "jobs": {"j": {"env": {"A": "${{ secrets.REGISTRY_PASSWORD }}"}}}}
|
|
assert stored_secret_faults("synthetic.yml", off_route) == [], trigger
|
|
|
|
|
|
def test_the_collector_reports_a_WORKFLOW_SCOPE_reference_no_job_if_can_reach() -> None:
|
|
"""The workflow scope is judged even when every job is gated OFF the route.
|
|
|
|
This is the case a job-only walk gets exactly backwards: the document looks maximally safe (its
|
|
one job carries the pinned exclusion) while the root `env:` is materialised into that job's
|
|
environment on the head-authored route anyway. The fault must therefore not depend on any job
|
|
being in the population.
|
|
"""
|
|
for scope in (
|
|
{"env": {"A": "${{ secrets.REGISTRY_PASSWORD }}"}},
|
|
{"defaults": {"run": {"shell": "bash -c 'echo ${{ secrets.RENOVATE_TOKEN }}'"}}},
|
|
):
|
|
doc = {
|
|
True: {"pull_request": None},
|
|
**scope,
|
|
"jobs": {"j": {"if": "github.event_name != 'pull_request'", "steps": [{"run": "true"}]}},
|
|
}
|
|
faults = stored_secret_faults("synthetic.yml", doc)
|
|
assert len(faults) == 1, (scope, faults)
|
|
assert "WORKFLOW SCOPE" in faults[0], faults[0]
|
|
assert pull_request_jobs(doc) == [], "the job is supposed to be OFF the route in this fixture"
|
|
|
|
# The injected token is not a stored secret at workflow scope either.
|
|
injected = {True: {"pull_request": None}, "env": {"A": "${{ secrets.GITEA_TOKEN }}"}, "jobs": {}}
|
|
assert stored_secret_faults("synthetic.yml", injected) == []
|
|
|
|
# And the scope is only judged on the route: a push-only workflow may hold one.
|
|
off_route = {True: {"push": None}, "env": {"A": "${{ secrets.REGISTRY_PASSWORD }}"}, "jobs": {}}
|
|
assert stored_secret_faults("synthetic.yml", off_route) == []
|
|
|
|
|
|
def test_MUTATION_a_WORKFLOW_LEVEL_env_in_the_SHIPPED_workflow_is_DETECTED() -> None:
|
|
"""Drives the SHIPPED `docker-build.yml`, hoisting the credential to workflow scope.
|
|
|
|
This is the mutation that a name-set cross-check could not see (measured 2026-09-05 at
|
|
59003d5a3: `14 passed`, rc=0) — `build` already names both halves, so a second copy at root
|
|
changed no NAME. It is exercised on the real file rather than a synthetic document for the same
|
|
reason `test_MUTATION_the_BUILD_job_losing_its_route_EXCLUSION_is_DETECTED` is: `docker-build.yml`
|
|
is the file the invariant is about, and a fixture would only prove the collector works on a
|
|
document this repo does not ship.
|
|
"""
|
|
root = Path(__file__).resolve().parents[2]
|
|
doc = yaml.safe_load((root / ".gitea/workflows/docker-build.yml").read_text())
|
|
assert stored_secret_faults("docker-build.yml", doc) == [], "the unmutated tree must be clean"
|
|
assert runs_on_pull_request(doc), "`docker-build.yml` no longer triggers on pull_request"
|
|
assert "REGISTRY_PASSWORD" in secret_names(doc["jobs"]["build"]), (
|
|
"`build` no longer names `REGISTRY_PASSWORD`, so this mutation no longer reproduces the "
|
|
"cancelling name set it was written for — re-point it before trusting it."
|
|
)
|
|
assert secret_names(outside_jobs(doc)) == set(), "the shipped workflow scope must name no secret"
|
|
|
|
doc["env"]["ETV_REGISTRY_AUTH"] = "${{ secrets.REGISTRY_USER }}:${{ secrets.REGISTRY_PASSWORD }}"
|
|
faults = stored_secret_faults("docker-build.yml", doc)
|
|
assert len(faults) == 1, f"expected exactly one fault, got {faults}"
|
|
assert "WORKFLOW SCOPE" in faults[0], faults[0]
|
|
assert "REGISTRY_PASSWORD" in faults[0] and "REGISTRY_USER" in faults[0], faults[0]
|
|
|
|
|
|
def test_MUTATION_the_BUILD_job_losing_its_route_EXCLUSION_is_DETECTED() -> None:
|
|
"""Drives the SHIPPED workflow, one clause changed — not a synthetic document.
|
|
|
|
`build` is the one job that still holds `REGISTRY_PASSWORD`, and the only thing keeping it off
|
|
the head-authored route is its `if: github.event_name != 'pull_request'`. Deleting that clause is
|
|
the whole defect this guard exists for, so the guard is required to name it, by job id and by
|
|
secret. A synthetic fixture cannot show that: it would prove the collector works on a document
|
|
this repo does not ship.
|
|
"""
|
|
root = Path(__file__).resolve().parents[2]
|
|
doc = yaml.safe_load((root / ".gitea/workflows/docker-build.yml").read_text())
|
|
assert stored_secret_faults("docker-build.yml", doc) == [], "the unmutated tree must be clean"
|
|
|
|
build = doc["jobs"]["build"]
|
|
assert _unwrap_expression(build["if"]) in PR_EXCLUDING_IFS, (
|
|
"`build`'s `if:` is no longer the pinned pull_request exclusion, so this mutation no longer "
|
|
f"changes anything — it reads {build.get('if')!r}. Re-point the mutation before trusting it."
|
|
)
|
|
assert "REGISTRY_PASSWORD" in secret_names(build), "`build` no longer names the secret this mutation is about"
|
|
|
|
del build["if"]
|
|
faults = stored_secret_faults("docker-build.yml", doc)
|
|
assert len(faults) == 1, f"expected exactly one fault, got {faults}"
|
|
assert "job `build`" in faults[0] and "REGISTRY_PASSWORD" in faults[0], faults[0]
|