Files
ersatztv/scripts/tests/test_workflow_persist_credentials.py
T
timothyandtimothy 45b17e58e1
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 6s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 23s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m53s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m23s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m15s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m27s
fix(744,835): ci-image.yml publishes from main only; guard persist-credentials with no exemption (#857)
Closes the push route into ci-image.yml (#744) and ships the persist-credentials guard that was waiting on it (#835).

ci-image.yml's push trigger had no branches: filter and was path-scoped to docker/ci/** AND to the workflow file itself. Gitea resolves a push workflow's definition from the pushed ref, so any branch push touching those paths ran that branch's own YAML on a docker-capable runner holding the credential that writes ersatztv:prod and the ersatztv-ci:<sha> five container: jobs execute.

Be precise about what the filter buys: it is loaded from the pushed ref like the rest of the file, so a branch that deletes it re-enables the route. This closes the DRIVE-BY case - publication as a side effect of an ordinary push - and is not a boundary against a writer who intends to run their own YAML. The wider class is #853.

The self-reference left both paths: and ci-image-pin's expected in the same change - a decided tradeoff with both prices stated, not a necessity. Branch publishing moves to workflow_dispatch, probed live: run 2340 on this branch published ersatztv-ci:43b1e45 and left :latest unchanged.

With both mechanical blockers gone, ci-image.yml's checkout takes persist-credentials: false (16 of 16) and scripts/tests/test_workflow_persist_credentials.py holds the convention with NO exemption list - git-index population, declared clause mutation re-run every suite, guard-inventory rows.

fixes #744
fixes #835

Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
2026-08-27 20:44:44 +00:00

302 lines
16 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 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 an earlier version of this function: 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."
)