timothyandClaude Fable 5.1
d62335bc8d
`_SECRET_REF` ran over the whole string while the comment above it claimed every
pattern was confined to `${{ }}` spans. Executed against the shipped module,
`secret_refs("# We deliberately pass no secrets. Then the pull is anonymous.")`
returned `['Then']`, and fed through the real collector that is one fault reading
"job `j` names stored secret(s) on the pull_request route: Then" — a fabricated
name, on a PR-route job, for its own comment. The same comment separately reddened
the text-versus-walk cross-check, because the line-level strip removes a `#` line
from the text half only.
The existing negative control passed for a reason that does not generalise: no `.`
follows the word in `"no secrets are used here"`. A sentence ENDING in "secrets."
is the likeliest thing to be written into a PR-route `run:` block on this branch's
own subject, so the trap was self-inflicted.
`secret_refs` now resolves names per `${{ }}` span, so every spelling is scoped the
way the residue counter already was. The added rows drive the real predecessor —
`_SECRET_REF` applied to the whole string — and assert it read a name where the
scoped reader reads none, so reverting the scoping reddens them.
The `INJECTED_SECRETS` comment stops calling the injected `GITEA_TOKEN` "bounded by
the workflow's own `permissions:`": on this route the head supplies that file and
can delete the block. Allow-listing it is a claim about the store it is not in, not
about a bound.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV
911 lines
52 KiB
Python
911 lines
52 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, and a head-authored run receives it whether or not any job names it.
|
|
# Its `permissions:` block narrows it for the COMMITTED file only — on this route the head supplies
|
|
# that file and can delete the block — so allow-listing it is NOT a claim that it is bounded. It is a
|
|
# statement about a MECHANISM (injected, unavoidable, out of the store this guard is about) 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"
|
|
|
|
# The name a WHOLE-CONTEXT reference is reported under. It cannot collide with a real secret: a
|
|
# secret name is `[A-Za-z_][A-Za-z0-9_]*`, so no stored name can contain `.` or `*`.
|
|
WHOLE_SECRETS_CONTEXT = "secrets.*"
|
|
|
|
# Every spelling of a `secrets` reference the expression grammar admits — NOT just `secrets.NAME`.
|
|
# `secrets['NAME']` is the same reference to the evaluator (GitHub's expression syntax, which Gitea's
|
|
# runner implements, defines `[]` as the index operator alongside `.`), and context names are matched
|
|
# case-insensitively, so `Secrets.NAME` is one too. Recognising only the dot spelling made this a
|
|
# detector an added job could step around by writing a different one — and the cross-check below
|
|
# cannot report that, because BOTH of its halves read through this same function: a spelling neither
|
|
# half knows is a SHARED blind spot they agree at zero on, not a disagreement
|
|
# (`proof-sharing-with-subject-proves-nothing`, ersatztv#819). Measured 2026-09-05 against the
|
|
# predecessor of this commit, an `env:` of `"${{ secrets['REGISTRY_PASSWORD'] }}"` on a synthetic
|
|
# `pull_request` job produced `stored_secret_faults(...) == []` AND `walk_versus_text_faults(...) == []`.
|
|
#
|
|
# The name is taken VERBATIM, not case-folded onto the store's uppercase spelling. Whether the
|
|
# evaluator resolves `secrets.gitea_token` to the stored `GITEA_TOKEN` was not probed from here, and
|
|
# the un-folded reading is the demanding one: an unrecognised spelling misses the one-member
|
|
# `INJECTED_SECRETS` allow-list and is reported, rather than being forgiven on an unmeasured claim.
|
|
_SECRET_REF = re.compile(
|
|
r"(?<![A-Za-z0-9_])secrets\s*(?:"
|
|
r"\.\s*([A-Za-z_][A-Za-z0-9_]*)" # secrets.NAME
|
|
r"|\[\s*'([^']*)'\s*\]" # secrets['NAME']
|
|
r"|\[\s*\"([^\"]*)\"\s*\]" # secrets["NAME"]
|
|
r")",
|
|
re.I,
|
|
)
|
|
|
|
# The residue: a `secrets` token inside a `${{ }}` expression that `_SECRET_REF` did NOT resolve to a
|
|
# literal name. Two shapes reach it, and both hand over more than one secret rather than fewer —
|
|
# `toJSON(secrets)` or a bare `secrets` passed as an action input, and an index whose subscript is
|
|
# COMPUTED (`secrets[format('{0}_TOKEN', x)]`), which no static reader resolves. Counting the residue
|
|
# rather than pattern-matching those two shapes is what makes the widening exhaustive over the
|
|
# grammar: any spelling of the context that does not yield a literal name is reported, including one
|
|
# nobody has written yet.
|
|
#
|
|
# EVERY pattern here reads only inside `${{ }}` spans, `_SECRET_REF` included, because outside one
|
|
# the word is ordinary English and `secrets.` is a sentence boundary. That scoping is enforced in
|
|
# `secret_refs` below rather than in the patterns, which cannot express "within the enclosing span".
|
|
# Applied to the whole string instead, `_SECRET_REF` reads the English `# We pass no secrets. Then
|
|
# the pull is anonymous.` as a reference to a secret named `Then` — a fabricated name, faulting a
|
|
# PR-route job for a comment, on this branch's own subject. A reference the runner actually resolves
|
|
# is always inside an expression, so the scoping costs no real spelling.
|
|
_EXPRESSION = re.compile(r"\$\{\{(.*?)\}\}", re.S)
|
|
_SECRETS_TOKEN = re.compile(r"(?<![A-Za-z0-9_])secrets(?![A-Za-z0-9_])", re.I)
|
|
|
|
|
|
def secret_refs(text: str) -> list[str]:
|
|
"""Every `secrets` reference in one string — occurrences, not names.
|
|
|
|
The single entry point for both the document walk and the text cross-check, so that widening one
|
|
spelling widens both. A list rather than a set for the reason `secret_name_counts` is a Counter:
|
|
only the lossless direction can be narrowed afterwards.
|
|
"""
|
|
found: list[str] = []
|
|
for expression in _EXPRESSION.findall(text):
|
|
resolved = [next(group for group in m.groups() if group is not None) for m in _SECRET_REF.finditer(expression)]
|
|
found.extend(resolved)
|
|
# The residue: `secrets` tokens in this span that resolved to no literal name.
|
|
found.extend([WHOLE_SECRETS_CONTEXT] * (len(_SECRETS_TOKEN.findall(expression)) - len(resolved)))
|
|
return found
|
|
|
|
|
|
# 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_name_counts(node: object) -> Counter:
|
|
"""Every secret reference reachable anywhere in a subtree — keys and values, at any depth — COUNTED.
|
|
|
|
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.
|
|
|
|
Occurrences, not a set of names, because the two consumers want different views of ONE walk and
|
|
only this direction is lossless: `secret_names` is derived from it below. A second traversal
|
|
with a different accumulator would be a copy of a mechanism, free to drift from the one the
|
|
assertion runs on — the shape this guard exists to catch, in the guard itself.
|
|
"""
|
|
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_refs(item))
|
|
return found
|
|
|
|
|
|
def secret_names(node: object) -> set[str]:
|
|
"""The names `secret_name_counts` reached. The fault collector wants names — it reports which
|
|
secret a job holds, once — and only the cross-check needs locations."""
|
|
return set(secret_name_counts(node))
|
|
|
|
|
|
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 walk_versus_text_faults(name: str, text: str) -> list[str]:
|
|
"""One file's half of the cross-check below, extracted so a negative control can DRIVE it.
|
|
|
|
Inlined in the loop it could be reverted to a name-set comparison — restoring the blind spot this
|
|
whole change is about — with every test in this file still green, because nothing but the loop
|
|
over the real workflows would ever call it and those agree either way
|
|
(`testing.guard-ships-with-mutation-proof`).
|
|
"""
|
|
walked = secret_name_counts(yaml.safe_load(text))
|
|
scanned = Counter(secret_refs("\n".join(line for line in text.splitlines() if not line.lstrip().startswith("#"))))
|
|
if walked == scanned:
|
|
return []
|
|
return [
|
|
f"{name}: the document walk reached {sorted(walked.items())} but the text scan found "
|
|
f"{sorted(scanned.items())}. The walk is what the stored-secret assertion runs over, so the "
|
|
f"difference is secret references this guard cannot see — widen `secret_names`' entry "
|
|
f"point, or take the reference out of the file."
|
|
]
|
|
|
|
|
|
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 against the predecessor of this commit — the guard as it
|
|
walked `jobs.<id>` only and compared per-file NAME SETS — 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.
|
|
|
|
WHAT THIS CROSS-CHECK STRUCTURALLY CANNOT REPORT, since it is the reason `secret_refs` has to be
|
|
widened rather than leaned on: both halves read through that one function, so a spelling IT does
|
|
not recognise is invisible to both and they agree at zero. The dot/index/whole-context spellings
|
|
are covered there; what remains uncovered is a name the expression COMPUTES
|
|
(`secrets[format('{0}_TOKEN', x)]`), which no static reader resolves. That spelling still faults,
|
|
but as `secrets.*` — the whole-context sentinel — because the index contains no literal, which is
|
|
the fail-closed direction and is asserted in
|
|
`test_the_collector_sees_every_SPELLING_of_a_secret_reference`.
|
|
"""
|
|
disagreements: list[str] = []
|
|
total = Counter()
|
|
for path in workflow_files():
|
|
text = path.read_text()
|
|
total += secret_name_counts(yaml.safe_load(text))
|
|
disagreements.extend(walk_versus_text_faults(path.name, text))
|
|
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_refs` 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)
|
|
|
|
# And the CROSS-CHECK ITSELF, on a text/walk pair whose NAME SETS AGREE. Reverting the
|
|
# comparison to name sets would restore the blind spot this change is about, so the difference
|
|
# is driven rather than left to the collectors' unit assertions above.
|
|
clean = 'jobs:\n j:\n env:\n A: "${{ secrets.REGISTRY_PASSWORD }}"\n'
|
|
assert walk_versus_text_faults("synthetic.yml", clean) == []
|
|
|
|
# The same name a SECOND time, at a location the YAML walk cannot reach — a trailing comment,
|
|
# which the line-level strip leaves in the text half.
|
|
duplicated = (
|
|
'jobs:\n j:\n env:\n A: "${{ secrets.REGISTRY_PASSWORD }}" # ${{ secrets.REGISTRY_PASSWORD }}\n'
|
|
)
|
|
faults = walk_versus_text_faults("synthetic.yml", duplicated)
|
|
assert len(faults) == 1, faults
|
|
assert "REGISTRY_PASSWORD" in faults[0]
|
|
|
|
# A name-set comparison — the mechanism this replaced — reports NOTHING on that same input.
|
|
assert secret_names(yaml.safe_load(duplicated)) == set(secret_refs(duplicated)) == {"REGISTRY_PASSWORD"}
|
|
|
|
# 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_sees_every_SPELLING_of_a_secret_reference() -> None:
|
|
"""The spellings that are the SAME reference to the evaluator and were invisible to the detector.
|
|
|
|
Driven against the real predecessor (`dot_only` below is `_SECRET_REF` exactly as this branch
|
|
shipped it before this commit), because that is the mutation this test exists to catch: revert
|
|
the widening and every case here goes back to `faults == []`. It cannot be caught by the
|
|
text-versus-walk cross-check — both halves read through `secret_refs`, so a spelling it does not
|
|
know is a shared blind spot they agree at zero on rather than a disagreement
|
|
(`proof-sharing-with-subject-proves-nothing`). Measured 2026-09-05, the predecessor reported
|
|
`stored_secret_faults(...) == []` and `walk_versus_text_faults(...) == []` on the index spelling.
|
|
|
|
Whether act_runner resolves each of these against this instance was NOT probed from here — that
|
|
needs a live run. The direction is what makes that acceptable: a spelling the runner happens not
|
|
to support costs a spurious demand on a job nobody has written; the omission cost a live
|
|
write-capable credential on the head-authored route.
|
|
"""
|
|
dot_only = re.compile(r"secrets\.([A-Za-z_][A-Za-z0-9_]*)")
|
|
|
|
for expression, reported in (
|
|
("${{ secrets['REGISTRY_PASSWORD'] }}", "REGISTRY_PASSWORD"),
|
|
('${{ secrets["REGISTRY_PASSWORD"] }}', "REGISTRY_PASSWORD"),
|
|
("${{ secrets [ 'REGISTRY_PASSWORD' ] }}", "REGISTRY_PASSWORD"),
|
|
("${{ Secrets.REGISTRY_PASSWORD }}", "REGISTRY_PASSWORD"),
|
|
# Names NO secret and hands over ALL of them; and an index the expression COMPUTES, which no
|
|
# static reader resolves. Both are reported under the whole-context sentinel.
|
|
("${{ toJSON(secrets) }}", WHOLE_SECRETS_CONTEXT),
|
|
("${{ secrets[format('{0}_PASSWORD', 'REGISTRY')] }}", WHOLE_SECRETS_CONTEXT),
|
|
):
|
|
assert dot_only.findall(expression) == [], (
|
|
f"{expression!r} is supposed to be a spelling the dot-only predecessor could not see — "
|
|
"if it can, this row proves nothing about the widening."
|
|
)
|
|
assert secret_refs(expression) == [reported], expression
|
|
doc = {True: {"pull_request": None}, "jobs": {"j": {"env": {"A": expression}}}}
|
|
faults = stored_secret_faults("synthetic.yml", doc)
|
|
assert len(faults) == 1, (expression, faults)
|
|
assert reported in faults[0], (expression, faults)
|
|
|
|
# The workflow scope is judged through the same function, so it widens with it.
|
|
scoped = {True: {"pull_request": None}, "env": {"A": "${{ secrets['REGISTRY_PASSWORD'] }}"}, "jobs": {}}
|
|
assert len(stored_secret_faults("synthetic.yml", scoped)) == 1
|
|
|
|
# The cross-check now COUNTS these spellings on both sides rather than agreeing at zero.
|
|
indexed = "jobs:\n j:\n env:\n A: \"${{ secrets['REGISTRY_PASSWORD'] }}\"\n"
|
|
assert secret_name_counts(yaml.safe_load(indexed)) == Counter({"REGISTRY_PASSWORD": 1})
|
|
assert walk_versus_text_faults("synthetic.yml", indexed) == []
|
|
|
|
# The over-match direction, which is the cost of matching a bare `secrets` at all: the word is
|
|
# only a context reference INSIDE an expression. Ordinary prose and a longer identifier are not.
|
|
assert secret_refs("echo 'no secrets are used here'") == []
|
|
assert secret_refs("${{ env.mysecrets.REGISTRY_PASSWORD }}") == []
|
|
prose = {True: {"pull_request": None}, "jobs": {"j": {"steps": [{"run": "echo 'no secrets here'"}]}}}
|
|
assert stored_secret_faults("synthetic.yml", prose) == []
|
|
|
|
# The prose row above passes for a reason that does not generalise — no `.` follows the word. An
|
|
# English sentence ENDING in "secrets." is the shape that reached the unscoped predecessor, and
|
|
# it is the likeliest sentence to be written into a PR-route `run:` block on this branch's own
|
|
# subject. Driven against that predecessor: `unscoped` is `_SECRET_REF` applied to the whole
|
|
# string, which is how `secret_refs` read before the span scoping.
|
|
for sentence in (
|
|
"# We deliberately pass no secrets. Then the pull is anonymous.",
|
|
"echo 'this job holds no secrets. Anonymous pull only'",
|
|
):
|
|
unscoped = [next(group for group in m.groups() if group is not None) for m in _SECRET_REF.finditer(sentence)]
|
|
assert unscoped, (
|
|
f"{sentence!r} is supposed to be a sentence the unscoped predecessor read as a secret "
|
|
"name — if it is not, this row proves nothing about the scoping."
|
|
)
|
|
assert secret_refs(sentence) == [], sentence
|
|
commented = {True: {"pull_request": None}, "jobs": {"j": {"steps": [{"run": f"{sentence}\ntrue"}]}}}
|
|
assert stored_secret_faults("synthetic.yml", commented) == [], sentence
|
|
# The same comment reddened the text-versus-walk cross-check separately, and for a different
|
|
# mechanism: the strip is LINE-level, so a fabricated name inside a `run:` scalar was removed
|
|
# from the text half only and the two halves disagreed on a name no workflow ever held.
|
|
as_text = f"jobs:\n j:\n steps:\n - run: |\n {sentence}\n true\n"
|
|
assert walk_versus_text_faults("synthetic.yml", as_text) == [], sentence
|
|
|
|
|
|
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 against the
|
|
predecessor of this commit: `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]
|