fix(885): PR-route registry credential exposure closed; anonymous reads replace the stored secret #919
@@ -120,13 +120,29 @@ env:
|
||||
# is NOT granted, and that holds at any owner default — it is not conditional on Restricted being on.
|
||||
# Only `review-verdict.yml` needs write; it declares that at the job and says why there. Full
|
||||
# rationale and the per-workflow credential audit: docs/ci-cd.md -> "Workflow token scope".
|
||||
# Every credentialed thing this file does uses the scoped REGISTRY_* PAT, never the injected token:
|
||||
# its registry pushes, its five `container:` image pulls, its three commit-status GET steps
|
||||
# (`ETV_STATUS_AUTH` in jobs `test`, `migrations` and `functional-e2e`, each a read-only GET via
|
||||
# scripts/ci-detect-already-validated.sh) and its registry tag READ (`ETV_REGISTRY_AUTH` in job
|
||||
# `toolchain-preflight`, via scripts/ci-toolchain-image-resolves.sh). The injected token therefore
|
||||
# serves only its eight `actions/checkout` steps. Note this file needs no `packages:` unit for that
|
||||
# same reason: the `container:` blocks carry explicit `credentials:`.
|
||||
# NO JOB ON THE `pull_request` ROUTE NAMES A STORED SECRET (ersatztv#885,
|
||||
# `ci.pr-route-carries-no-stored-credential`). Gitea resolves a `pull_request` run from the PR HEAD,
|
||||
# so this file is attacker-authored on that route and every `secrets.*` it names is materialised
|
||||
# into the run. The scoped REGISTRY_* PAT is therefore held by `build` alone, which is gated
|
||||
# `if: github.event_name != 'pull_request'`. The jobs that used to hold it now work without it:
|
||||
# the five `container:` pulls and `toolchain-preflight`'s registry tag READ go through the registry's
|
||||
# anonymous bearer-token flow, and the three commit-status GETs
|
||||
# (scripts/ci-detect-already-validated.sh in `test`, `migrations` and `functional-e2e`) read the
|
||||
# combined-status API unauthenticated. Those are TWO dependencies, on two different objects, with
|
||||
# opposite failure directions — do not collapse them into one "keep it public or CI breaks loudly".
|
||||
# The `ersatztv-ci` package is linked to no repository (measured 2026-09-05: every version of it
|
||||
# reports `"repository": null`), so THIS repo's visibility is not what gates the anonymous pull
|
||||
# token. (1) The five pulls and the preflight need that PACKAGE to stay anonymously pullable, and
|
||||
# losing it IS loud: every `container:` job dies at image pull, before it runs a step, both
|
||||
# required contexts among them, and `toolchain-preflight` names the cause in its own 401/403
|
||||
# message. (2) The three commit-status GETs need `timothy/ersatztv` itself to stay publicly
|
||||
# readable, and losing that is SILENT: `curl -sf` fails, ci-detect-already-validated.sh falls
|
||||
# through to `skip=false`, and the jobs stay GREEN — only the ersatztv#420 cross-run skip quietly
|
||||
# stops firing, which costs a redundant re-validation and never a skip that was not earned. The
|
||||
# invariant is held by
|
||||
# scripts/tests/test_workflow_persist_credentials.py::test_no_PULL_REQUEST_route_job_names_a_STORED_secret.
|
||||
# The injected token serves only this file's eight `actions/checkout` steps. Note it needs no
|
||||
# `packages:` unit: the container pulls are anonymous, not token-authenticated.
|
||||
# (Sites above are named by JOB, not by line number: this file is ~1150 lines, so any edit above a
|
||||
# citation silently invalidates it — a line-number citation here has gone stale within two lines
|
||||
# of being written.)
|
||||
@@ -150,8 +166,6 @@ jobs:
|
||||
with:
|
||||
persist-credentials: false
|
||||
- name: Resolve the pinned toolchain tag in the registry
|
||||
env:
|
||||
ETV_REGISTRY_AUTH: ${{ secrets.REGISTRY_USER }}:${{ secrets.REGISTRY_PASSWORD }}
|
||||
run: |
|
||||
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark resolve
|
||||
scripts/ci-toolchain-image-resolves.sh
|
||||
@@ -165,9 +179,6 @@ jobs:
|
||||
runs-on: ubuntu-latest
|
||||
container:
|
||||
image: 192.168.1.95:3000/timothy/ersatztv-ci:32747a0
|
||||
credentials:
|
||||
username: ${{ secrets.REGISTRY_USER }}
|
||||
password: ${{ secrets.REGISTRY_PASSWORD }}
|
||||
env:
|
||||
CI_EXECUTION_CLASS: toolchain
|
||||
CI_JOB_ROLE: guard
|
||||
@@ -195,8 +206,6 @@ jobs:
|
||||
scripts/ci-detect-docs-only.sh
|
||||
- name: Detect already-validated tree (#420)
|
||||
id: revalidate
|
||||
env:
|
||||
ETV_STATUS_AUTH: ${{ secrets.REGISTRY_USER }}:${{ secrets.REGISTRY_PASSWORD }}
|
||||
run: |
|
||||
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark revalidate
|
||||
scripts/ci-detect-already-validated.sh
|
||||
@@ -389,9 +398,6 @@ jobs:
|
||||
runs-on: ubuntu-latest
|
||||
container:
|
||||
image: 192.168.1.95:3000/timothy/ersatztv-ci:32747a0
|
||||
credentials:
|
||||
username: ${{ secrets.REGISTRY_USER }}
|
||||
password: ${{ secrets.REGISTRY_PASSWORD }}
|
||||
# Independent gate (not a 'needs' of build yet) so the new MySql-service dependency
|
||||
# can't block image builds until it's proven reliable on the runner. Promote to a
|
||||
# required check / build dependency once green. (ersatztv#13)
|
||||
@@ -468,8 +474,6 @@ jobs:
|
||||
scripts/ci-detect-docs-only.sh
|
||||
- name: Detect already-validated tree (#420)
|
||||
id: revalidate
|
||||
env:
|
||||
ETV_STATUS_AUTH: ${{ secrets.REGISTRY_USER }}:${{ secrets.REGISTRY_PASSWORD }}
|
||||
run: |
|
||||
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark revalidate
|
||||
scripts/ci-detect-already-validated.sh
|
||||
@@ -611,9 +615,6 @@ jobs:
|
||||
if: github.event_name == 'pull_request' || github.ref == 'refs/heads/main'
|
||||
container:
|
||||
image: 192.168.1.95:3000/timothy/ersatztv-ci:32747a0
|
||||
credentials:
|
||||
username: ${{ secrets.REGISTRY_USER }}
|
||||
password: ${{ secrets.REGISTRY_PASSWORD }}
|
||||
env:
|
||||
CI_EXECUTION_CLASS: toolchain
|
||||
CI_JOB_ROLE: guard
|
||||
@@ -632,8 +633,6 @@ jobs:
|
||||
run: scripts/ci-detect-docs-only.sh
|
||||
- name: Detect already-validated tree (#420)
|
||||
id: revalidate
|
||||
env:
|
||||
ETV_STATUS_AUTH: ${{ secrets.REGISTRY_USER }}:${{ secrets.REGISTRY_PASSWORD }}
|
||||
run: scripts/ci-detect-already-validated.sh
|
||||
|
||||
- name: Cache NuGet packages
|
||||
@@ -1044,9 +1043,6 @@ jobs:
|
||||
runs-on: ubuntu-latest
|
||||
container:
|
||||
image: 192.168.1.95:3000/timothy/ersatztv-ci:32747a0
|
||||
credentials:
|
||||
username: ${{ secrets.REGISTRY_USER }}
|
||||
password: ${{ secrets.REGISTRY_PASSWORD }}
|
||||
if: github.event_name == 'pull_request'
|
||||
env:
|
||||
CI_EXECUTION_CLASS: toolchain
|
||||
@@ -1149,9 +1145,6 @@ jobs:
|
||||
runs-on: ubuntu-latest
|
||||
container:
|
||||
image: 192.168.1.95:3000/timothy/ersatztv-ci:32747a0
|
||||
credentials:
|
||||
username: ${{ secrets.REGISTRY_USER }}
|
||||
password: ${{ secrets.REGISTRY_PASSWORD }}
|
||||
if: github.event_name == 'pull_request'
|
||||
env:
|
||||
CI_EXECUTION_CLASS: toolchain
|
||||
|
||||
@@ -1446,10 +1446,20 @@ jobs:
|
||||
# credential, and reachable from any workflow because secrets are a per-repo store.
|
||||
#
|
||||
# What this STILL does not claim: the test asks "was this POSTed by an account we accept
|
||||
# verdicts from", NOT "by someone who actually reviewed it". `ETV_STATUS_AUTH`'s registry
|
||||
# credential can no longer POST a status at all (#697, fixed by scoping it off
|
||||
# `write:repository` — see `ci.actions-credential-scoping`), and `RENOVATE_TOKEN` is now
|
||||
# re-derived rather than inherited; but an ALLOW-LISTED collaborator's own token can still
|
||||
# verdicts from", NOT "by someone who actually reviewed it". The registry credential
|
||||
# `docker-build.yml` used to pass as `ETV_STATUS_AUTH` can no longer POST a status at all
|
||||
# (#697, fixed by scoping it off `write:repository` — see `ci.actions-credential-scoping`),
|
||||
# and since #885 that route materialises no STORED credential to refuse: nothing on the
|
||||
# `pull_request` route names a stored secret and the status read is anonymous
|
||||
# (`ci.pr-route-carries-no-stored-credential`). The INJECTED `GITEA_TOKEN` is still handed
|
||||
# to every job there and STAYS IN THIS RESIDUAL SET: `docker-build.yml`'s workflow-level
|
||||
# `permissions:` (`code: read`) NARROWS it for the COMMITTED file only, and on the
|
||||
# `pull_request` route the head supplies that file and can delete the block — with the
|
||||
# owner-level Actions default at `permissive` (`ci.actions-credential-scoping`) that alone
|
||||
# yields a write-capable token. Removing the stored secrets does not touch it.
|
||||
# `RENOVATE_TOKEN` is now re-derived rather than inherited.
|
||||
#
|
||||
# But an ALLOW-LISTED collaborator's own token can still
|
||||
# POST a success with an attacker-chosen `Review-verdict:` description, which this guard then
|
||||
# preserves. That is provenance, not authentication — branch protection binds the context
|
||||
# NAME, not its issuer, and Gitea offers no way to restrict who may post a context.
|
||||
|
||||
+84
-28
@@ -101,6 +101,19 @@ commit in the range (see the `decisions.md` header) — routine lifecycle metada
|
||||
|
||||
1. Confirm `main` CI is green; run the full local gate plus `dotnet list package --vulnerable
|
||||
--include-transitive`; then push a `vYY.N.P` tag on that exact `main` commit.
|
||||
|
||||
> **`v*` tags are protected — push as `timothy`.** Since ersatztv#885 the repo carries one
|
||||
> tag-protection rule (`name_pattern: v*`, `whitelist_usernames: ["timothy"]`,
|
||||
> `release.tag-protection-v-star`), because a `v*` push builds and publishes `:prod`. A push from
|
||||
> any other account is expected to be refused — configured, not exercised, since only the
|
||||
> `timothy` credential is available here. **The positive half is UNVERIFIED too**: the rule was
|
||||
> applied 2026-09-04 and only its negative half was measured (a non-`v*` tag still pushes), because
|
||||
> pushing a `v*` tag outside a real cut would publish `:prod`. This cut is its first real
|
||||
> exercise. If the tag push is refused, unblock with
|
||||
> `DELETE /api/v1/repos/timothy/ersatztv/tag_protections/1`, push, then re-`POST` the rule
|
||||
> (prefer a `PATCH` adding the account if the cut has moved to a different operator, so the
|
||||
> protection is never left off) — and record the outcome on `release.tag-protection-v-star`,
|
||||
> which is waiting for it.
|
||||
2. Wait for tag CI to build `:prod` + the immutable `:<version>` + `:<sha>` images. Run
|
||||
`scripts/security-scan.sh` on jazz against **the immutable `:<version>` image**, not a
|
||||
moving tag, and triage every ZAP/semgrep finding.
|
||||
@@ -170,9 +183,15 @@ grants `releases: write`, which nothing here uses. The two read units are not sp
|
||||
`/issues/{n}/timeline` (the ersatztv#706 retarget fence) and, through
|
||||
`scripts/pr-changed-files.sh`, `/pulls/{n}` and `/pulls/{n}/files` (the ersatztv#698 exemption path).
|
||||
|
||||
No workflow needs a `packages:` unit. All five `container:` blocks in `docker-build.yml` carry
|
||||
explicit `credentials:` using the scoped `REGISTRY_*` PAT, so no job pulls the toolchain image with
|
||||
the injected token.
|
||||
No workflow needs a `packages:` unit. Since ersatztv#885 the five `container:` blocks in
|
||||
`docker-build.yml` carry no `credentials:` at all: they run on the `pull_request` route, so they may
|
||||
hold no stored secret, and the runner pulls the toolchain image ANONYMOUSLY — this registry issues a
|
||||
pull token for a public package with no credential (measured at the HTTP level 2026-09-04, and at the
|
||||
daemon level 2026-09-05 — a `docker pull` of the pinned tag with an empty docker config succeeds on
|
||||
the runner host, which is the same daemon that creates the job containers; see
|
||||
`ci.pr-route-carries-no-stored-credential`, whose `mechanics:` names both measurements and the two
|
||||
things neither exercises). So no job pulls that image with the injected
|
||||
token, and none pulls it with the `REGISTRY_*` PAT either.
|
||||
|
||||
### The binding was measured, not assumed (2026-08-27, Gitea 1.27.1)
|
||||
|
||||
@@ -736,7 +755,11 @@ the image build.
|
||||
for `192.168.1.95:3000` — **BuildKit does not inherit the host daemon's
|
||||
`insecure-registries`**, so without this, cache/base-image/push over the HTTP
|
||||
registry fails (`http: server gave HTTP response to HTTPS client`).
|
||||
3. `docker/login-action` with repo secrets `REGISTRY_USER` / `REGISTRY_PASSWORD`.
|
||||
3. `docker/login-action` with repo secrets `REGISTRY_USER` / `REGISTRY_PASSWORD`. **`build` is the
|
||||
ONLY job in this workflow that names them** (ersatztv#885,
|
||||
`ci.pr-route-carries-no-stored-credential`): it is gated `if: github.event_name != 'pull_request'`,
|
||||
and every job that IS on the PR route now reads the registry and the status API anonymously,
|
||||
because a `pull_request` run executes head-supplied YAML.
|
||||
**`REGISTRY_PASSWORD` is a scoped PAT (`write:package` + `read:repository`), not an account
|
||||
password** — deliberately, so head-resolved PR code cannot use it to forge a commit status
|
||||
(`ci.actions-credential-scoping`, ersatztv#697). If a job ever fails with `token does not have at
|
||||
@@ -901,8 +924,11 @@ the docs-only detect), and every heavy step gains an added `&& steps.revalidate.
|
||||
squash, rebase, fast-forward, or a direct push have no `HEAD^2`, so they run);
|
||||
- `git rev-parse HEAD^{tree}` equals `HEAD^2^{tree}` — main did not advance since the PR's last run,
|
||||
a byte-identical tree;
|
||||
- `HEAD^2` has a **green Gitea combined commit status**, queried via the API with
|
||||
`ETV_STATUS_AUTH`. Trusting the aggregate `.state` is sound: a `skipped` context does **not** drag
|
||||
- `HEAD^2` has a **green Gitea combined commit status**, queried via the API **anonymously**
|
||||
(ersatztv#885: the three jobs that call `scripts/ci-detect-already-validated.sh` also run on the
|
||||
`pull_request` route, so they hold no stored secret; `timothy/ersatztv` is public and answers the
|
||||
combined-status GET without a credential — measured 2026-09-04). Trusting the aggregate `.state`
|
||||
is sound: a `skipped` context does **not** drag
|
||||
the combined state below `success` (verified live against this instance — a real merge commit with
|
||||
four `skipped` PR-only contexts still reported `.state == success`), and the two required jobs
|
||||
never report `skipped` (they always run and report a real `success`/`failure`), so
|
||||
@@ -1432,8 +1458,14 @@ re-opens the hole this section exists to close.
|
||||
|
||||
Practical consequences: **every** change to `main` goes through a PR, including a one-line docs fix;
|
||||
and the client-side Husky guards (H6/H11/H13) remain useful friction but were never the control —
|
||||
they are fail-open and `--no-verify` bypasses them. Tag pushes are unaffected (separate mechanism;
|
||||
`tag_protections` is empty), so the release cut in "Cutting a release" still works unchanged.
|
||||
they are fail-open and `--no-verify` bypasses them. Tag pushes go through a separate mechanism and
|
||||
are meant to stay unaffected for the release operator: since ersatztv#885 `tag_protections` carries
|
||||
one rule, `v*` whitelisted to `timothy` (`release.tag-protection-v-star`), which is intended to leave
|
||||
the release cut in "Cutting a release" working unchanged while refusing a `v*` tag from the `renovate`
|
||||
bot — a push that would otherwise have published a `:prod` image. Both halves of that are EXPECTED,
|
||||
UNVERIFIED: only the `timothy` credential exists here, so neither a real release cut nor a refused bot
|
||||
push has been exercised. `release.tag-protection-v-star` carries the same caveat and waits on the
|
||||
next cut.
|
||||
|
||||
**A fourth arm: guard-scope freshness (ersatztv#787).** `scripts/tests/test_ci_dropped_step_guard.py`
|
||||
decides which CI jobs must carry per-step execution markers by DERIVING that scope from
|
||||
@@ -1919,8 +1951,10 @@ as establishing that the gate cannot be forged (see the residual below, and ersa
|
||||
`docs/decisions/records/ci/gate-trigger-base-resolved.md`. **This closes the rewrite route through
|
||||
this workflow, not the class:** `docker-build.yml` is also head-resolved and must stay on
|
||||
`pull_request` because it builds the PR's code, so it got the read-only status identity instead —
|
||||
its `ETV_STATUS_AUTH` is now a PAT scoped `write:package` + `read:repository`, which the status
|
||||
endpoint refuses (`ci.actions-credential-scoping`, ersatztv#697). The inventory was never that one
|
||||
the `ETV_STATUS_AUTH` it passed was a PAT scoped `write:package` + `read:repository`, which the
|
||||
status endpoint refuses (`ci.actions-credential-scoping`, ersatztv#697). Since ersatztv#885 it
|
||||
passes no credential there at all: the status read is anonymous, so the PR route materialises
|
||||
nothing to refuse. The inventory was never that one
|
||||
workflow, though: Gitea injects a write-capable `GITEA_TOKEN` into every job and branch protection
|
||||
binds the *context*, not its issuer. Gitea >=1.26 with the Actions default set to **Restricted**
|
||||
(server-management#714) binds the injected token, but does not close the class either — not against
|
||||
@@ -2273,16 +2307,27 @@ documented API paths mention "environment", and the config file the running serv
|
||||
restricting dispatch would close nothing: the `pull_request:` row above is cheaper than any
|
||||
dispatch, because it runs **attacker-authored YAML**, which can name any secret in the repo store —
|
||||
not only the ones the committed workflows reference. That is what makes it cover `renovate.yml`'s
|
||||
`RENOVATE_TOKEN` without dispatching `renovate.yml` at all. Concretely today, six jobs in
|
||||
`docker-build.yml` hold `REGISTRY_PASSWORD` and run on the PR route (`toolchain-preflight`, `test`,
|
||||
`migrations`, `functional-e2e`, `api-docs`, `format`), two of them required contexts — carry that as
|
||||
*every job on the PR route that names `secrets.REGISTRY_PASSWORD`*, not as the list, and not as
|
||||
"every `container:` job" either: `toolchain-preflight` is container-free and would fall out of that
|
||||
predicate. "Push a branch, open a PR" therefore reaches the credential with **no act outside the
|
||||
ordinary contribution flow**, where a dispatch costs one. (`dependency-scan.yml` references no
|
||||
secrets at all.) Full reasoning and the probe: `ci.workflow-dispatch-ref-unrestricted`. The live
|
||||
residuals are that PR route and the `v*` tag push, tracked in #885 — along with `tag_protections`,
|
||||
which is still empty.
|
||||
`RENOVATE_TOKEN` without dispatching `renovate.yml` at all. (`dependency-scan.yml` references no
|
||||
secrets at all.) Full reasoning and the probe: `ci.workflow-dispatch-ref-unrestricted`.
|
||||
|
||||
Six jobs in `docker-build.yml` used to hold `REGISTRY_PASSWORD` on that route, two of them required
|
||||
contexts. Since ersatztv#885 **no job on the `pull_request` route names any stored secret**
|
||||
(`ci.pr-route-carries-no-stored-credential`): the toolchain image is pulled through the registry's
|
||||
anonymous bearer-token flow and the commit-status API is read unauthenticated, leaving the PAT to
|
||||
`build`, which the route cannot reach. The invariant is held by
|
||||
`scripts/tests/test_workflow_persist_credentials.py::test_no_PULL_REQUEST_route_job_names_a_STORED_secret`,
|
||||
which derives the population from the git index by the predicate *every job of a
|
||||
`pull_request`-triggered workflow that names a `secrets.*`* — never a list, and never "every
|
||||
`container:` job" either, which names five of the six because `toolchain-preflight` is
|
||||
container-free. The `v*` tag-push row is closed against the bot by `release.tag-protection-v-star`.
|
||||
|
||||
What that does **not** close, and must not be read as closing: `REGISTRY_PASSWORD` is still in the
|
||||
repo's Actions store, and head-supplied YAML can still name it, `RENOVATE_TOKEN` or
|
||||
`SERVERMGMT_DEPLOY_KEY` in a job of its own. "Push a branch, open a PR" still reaches every secret in
|
||||
the store with **no act outside the ordinary contribution flow**. Bounding the store needs
|
||||
per-environment secret scoping, which Gitea 1.27.1 does not have. What is removed is the routine
|
||||
materialisation of a write-capable credential into six PR-run environments — blast radius, not the
|
||||
route.
|
||||
|
||||
**A `v*` tag push does not fire `ci-image.yml`.** There is no `tags:` key, and a `branches:` filter
|
||||
is compared against a branch ref — so a release cut no longer republishes the toolchain image as a
|
||||
@@ -2367,7 +2412,7 @@ sha tag once 15 newer versions of the package exist. `ci-image.yml` publishes a
|
||||
and on every push to `main` touching `docker/ci/**` (plus every branch dispatch), while the pin only
|
||||
moves when a human bumps it — so a pin ages toward eviction on its own. That is what happened between 2026-08-11 and
|
||||
2026-08-13 (ersatztv#772): the tag vanished, and every `container:` job — **both required contexts
|
||||
included** — died after 1–2s with
|
||||
included** — died at image pull, before running a step, with
|
||||
|
||||
```
|
||||
Error response from daemon: failed to resolve reference ".../ersatztv-ci:<pin>": not found
|
||||
@@ -2413,16 +2458,27 @@ because they send you to different places:
|
||||
| HTTP 200 with a manifest body | green | resolves |
|
||||
| HTTP 404 | **red** | `IS GONE` — rebuild the tag (recovery above) |
|
||||
| HTTP 200, body is not a manifest | **red** | something is answering for the registry (proxy, login page) |
|
||||
| 401 / 403 | **red** | the credentials were rejected — fix the secrets |
|
||||
| 401 / 403 that survives a bearer the run actually obtained | **red** | the registry `refused an ANONYMOUS read ... even after a Bearer token was obtained` — about the PACKAGE: check that the `ersatztv-ci` package is still PUBLIC. It is linked to no repository (measured 2026-09-05: every version reports `"repository": null`), so this is the package's own visibility and NOT `timothy/ersatztv`'s |
|
||||
| 401 whose token leg ANSWERED and yielded no bearer — no `Www-Authenticate` at all, a challenge naming no realm, or a token endpoint answering with no token | **red**, asked once | `could NOT OBTAIN an anonymous pull token ... after 1 token-leg attempt(s)` — about the registry's TOKEN ENDPOINT, not the package's visibility. The endpoint said something, so asking again cannot change it |
|
||||
| 401 whose token endpoint could not be REACHED, or answered 5xx | **red** after `ETV_CI_ATTEMPTS` token-leg tries | the same message, naming the attempts it actually made. An endpoint that said nothing is the same transport blip a flaky manifest read gets to survive — the two legs of one read must not have opposite flake tolerances when a red here denies a merge |
|
||||
| 403 on the FIRST read, before any token leg — `probe` enters it on a `401` only | **red** | `refused an ANONYMOUS read ... and NO TOKEN WAS EVER REQUESTED` — it reports what the RUN did (never followed this answer as a challenge), not what the registry sent. About ACCESS to the registry: visibility, or something answering for it |
|
||||
| anything else (5xx, unreachable, no `curl`) | **red** after `ETV_CI_ATTEMPTS` tries | `could NOT VERIFY` — check the registry's health, NOT the pin |
|
||||
| `ETV_REGISTRY_AUTH` unset, malformed, or either half empty | **red**, before any query | an absent secret interpolates to `":"`, which is not a credential |
|
||||
|
||||
The last two rows are the ones worth defending, because warning on them and exiting 0 is the natural
|
||||
way to write this check and it is wrong: a missing `curl`, a moved registry and a DNS change all land
|
||||
there, and a green-with-a-warning job is indistinguishable from a healthy pin forever after. The
|
||||
**The refusal rows say only what actually ran**, and that is the point of wording them apart
|
||||
rather than a nicety: a message naming a step the run skipped is evidence for a diagnosis nobody
|
||||
performed. So the arm branches on `token` first — a bearer was obtained — then on whether the token
|
||||
leg was attempted at all, and only then reports the never-asked case; and the token-leg message
|
||||
carries its own attempt count rather than leaving the reader to infer one from the retry policy.
|
||||
|
||||
Those refusal rows and the `could NOT VERIFY` row are the ones worth defending, because warning
|
||||
on them and exiting 0 is the natural way to write this check and it is wrong: a missing `curl`, a
|
||||
moved registry and a DNS change all land in the last of them, and a green-with-a-warning job is
|
||||
indistinguishable from a healthy pin forever after. The
|
||||
unknown arm retries first (`ETV_CI_ATTEMPTS`, default 3, `ETV_CI_RETRY_SECONDS` apart) so an ordinary
|
||||
registry blip does not redden a PR — that pause is what makes failing on unknown affordable, and
|
||||
shortening it silently trades this guard for flake.
|
||||
shortening it silently trades this guard for flake. **An unreachable token endpoint is in that
|
||||
class, not in the refusal class**, and is retried on the same budget: the split is by what the
|
||||
endpoint SAID, never by which leg of the read it happened on.
|
||||
|
||||
**It is not a `needs:` of anything, but it is not consequence-free either.** The merge-consent hook
|
||||
reads the PR's **combined** status and denies on a non-`success` combined state (a `skipped` context
|
||||
@@ -2464,7 +2520,7 @@ check and takes no arguments. Run it from the CURRENT checkout, not the pin work
|
||||
`$repo` is kept above:
|
||||
|
||||
```bash
|
||||
ETV_REGISTRY_AUTH=user:pass scripts/ci-toolchain-image-resolves.sh
|
||||
scripts/ci-toolchain-image-resolves.sh
|
||||
```
|
||||
|
||||
**What this rebuild does and does not restore.** It restores a *working* toolchain at that tag, built
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -5,9 +5,9 @@ status: active
|
||||
since: '2026-08-05'
|
||||
supersedes: none
|
||||
superseded-by: none
|
||||
rule: 'Any credential reachable from an Actions job is scoped to what that job needs. The container-registry secret `REGISTRY_PASSWORD` is a personal access token scoped `write:package` + `read:repository` — never an account PASSWORD. This matters because Gitea has NO `status` token scope: `POST /repos/{o}/{r}/statuses/{sha}` is gated by `reqRepoWriter(unit.TypeCode)`, so ANY credential that can write the repository can forge `review-verdict/h10`, the required context that is supposed to make merge-consent derived rather than assertable. Package-write IS a separate scope, so the registry credential can be made status-incapable at no cost: `scripts/ci-detect-already-validated.sh` only GETs. `permissions:` on a workflow/job DOES bind on this instance — MEASURED 2026-08-27 on 1.27.1 by matched scratch-base probe PRs differing only in one unit, the block carried at JOB level on `set-verdict-status` (`code: write` posted the probe status; `code: read` 403ed the POST, curl exit 22, no status written), so all six workflows here now declare it (#748). Two properties that make it usable: the declaration is EXHAUSTIVE, not additive — a unit omitted is not granted — and it binds while the owner-level default is `permissive`, which is what makes the five `code: read` declarations effective TODAY rather than only after a flip. NOT established: what a declared `code: write` does under a RESTRICTED default. GitHub semantics let `permissions:` only narrow, never widen past the default, and if Gitea copies that, Restricted would cap the gate job at read and `review-verdict/h10` would stop being writable — the exact catastrophic case. The probe ran under `permissive` and CANNOT rule that out. Flip the owner default only behind the scratch-base probe re-run under Restricted (server-management#714, still open for this reason). The earlier form of this rule said the opposite ("do NOT add a `permissions:` key on the assumption that it binds — below Gitea 1.26.0 it is silently a NO-OP"); that was correct at 1.25.4 and is retained here so a reader meeting the old advice recognises it as superseded. There is still no API surface for the owner-level default (`/api/v1/settings/actions` 404s at 1.27.1). The instance default HAS since been probed and is NOT unknown: it was set to Restricted, verified, and reverted to `permissive` on 2026-08-05 (#748), which is where it stands. Probe before relying on it; do not read the upgrade alone as the constraint now working. Scoping is necessary and not sufficient: it bounds what a job may DO, never whether attacker YAML runs at all, so a self-referencing trigger needs its own filter. That landed for `ci-image.yml` in #744 (`ci.toolchain-image-publish-is-a-dispatch`) — deliberately NOT bundled here, because until it also removed the file from `ci-image-pin`''s `expected`, editing it re-pointed that job at the editing commit and reddened a blocking check. A second, separate consequence of the same boundary: `actions/checkout` persists whatever the job token can do into `.git/config` unless `persist-credentials: false` is set — read-only everywhere since #748 declared `permissions:` on all six workflows — and all 16 of this repo''s checkouts now set it — 15 in #746 and `ci-image.yml`''s in #744, once `ci.toolchain-image-publish-is-a-dispatch` removed the two mechanical reasons it was excluded; the convention is held with no exemption list by `scripts/tests/test_workflow_persist_credentials.py` (#835). Ordering is part of the rule: unmask the dependent fetches FIRST, because until then a credential regression presents as an empty changed-file set and a silently skipped check rather than a red job. This record closes ONE route. It does not close the class, and the later sections say exactly what survives — read them before citing this record as a mitigation. The `workflow_dispatch` half of what survives is now settled rather than open: #853 probed Gitea 1.27.1 and ACCEPTED it (`ci.workflow-dispatch-ref-unrestricted`) — there is no ref restriction and no protected-environment concept to gate a secret behind, and restricting dispatch would close nothing anyway, because `docker-build.yml`''s head-resolved `pull_request:` runs attacker-authored YAML, which reaches EVERY secret in the store and not merely the ones the committed workflows name. The PR route and the `v*` tag push, not dispatch, are the live residuals (#885).'
|
||||
rule: 'Any credential reachable from an Actions job is scoped to what that job needs. The container-registry secret `REGISTRY_PASSWORD` is a personal access token scoped `write:package` + `read:repository` — never an account PASSWORD. This matters because Gitea has NO `status` token scope: `POST /repos/{o}/{r}/statuses/{sha}` is gated by `reqRepoWriter(unit.TypeCode)`, so ANY credential that can write the repository can forge `review-verdict/h10`, the required context that is supposed to make merge-consent derived rather than assertable. Package-write IS a separate scope, so the registry credential can be made status-incapable at no cost: `scripts/ci-detect-already-validated.sh` only GETs. `permissions:` on a workflow/job DOES bind on this instance — MEASURED 2026-08-27 on 1.27.1 by matched scratch-base probe PRs differing only in one unit, the block carried at JOB level on `set-verdict-status` (`code: write` posted the probe status; `code: read` 403ed the POST, curl exit 22, no status written), so all six workflows here now declare it (#748). Two properties that make it usable: the declaration is EXHAUSTIVE, not additive — a unit omitted is not granted — and it binds while the owner-level default is `permissive`, which is what makes the five `code: read` declarations effective TODAY rather than only after a flip. NOT established: what a declared `code: write` does under a RESTRICTED default. GitHub semantics let `permissions:` only narrow, never widen past the default, and if Gitea copies that, Restricted would cap the gate job at read and `review-verdict/h10` would stop being writable — the exact catastrophic case. The probe ran under `permissive` and CANNOT rule that out. Flip the owner default only behind the scratch-base probe re-run under Restricted (server-management#714, still open for this reason). The earlier form of this rule said the opposite ("do NOT add a `permissions:` key on the assumption that it binds — below Gitea 1.26.0 it is silently a NO-OP"); that was correct at 1.25.4 and is retained here so a reader meeting the old advice recognises it as superseded. There is still no API surface for the owner-level default (`/api/v1/settings/actions` 404s at 1.27.1). The instance default HAS since been probed and is NOT unknown: it was set to Restricted, verified, and reverted to `permissive` on 2026-08-05 (#748), which is where it stands. Probe before relying on it; do not read the upgrade alone as the constraint now working. Scoping is necessary and not sufficient: it bounds what a job may DO, never whether attacker YAML runs at all, so a self-referencing trigger needs its own filter. That landed for `ci-image.yml` in #744 (`ci.toolchain-image-publish-is-a-dispatch`) — deliberately NOT bundled here, because until it also removed the file from `ci-image-pin`''s `expected`, editing it re-pointed that job at the editing commit and reddened a blocking check. A second, separate consequence of the same boundary: `actions/checkout` persists whatever the job token can do into `.git/config` unless `persist-credentials: false` is set — read-only everywhere since #748 declared `permissions:` on all six workflows — and all 16 of this repo''s checkouts now set it — 15 in #746 and `ci-image.yml`''s in #744, once `ci.toolchain-image-publish-is-a-dispatch` removed the two mechanical reasons it was excluded; the convention is held with no exemption list by `scripts/tests/test_workflow_persist_credentials.py` (#835). Ordering is part of the rule: unmask the dependent fetches FIRST, because until then a credential regression presents as an empty changed-file set and a silently skipped check rather than a red job. This record closes ONE route. It does not close the class, and the later sections say exactly what survives — read them before citing this record as a mitigation. The `workflow_dispatch` half of what survives is now settled rather than open: #853 probed Gitea 1.27.1 and ACCEPTED it (`ci.workflow-dispatch-ref-unrestricted`) — there is no ref restriction and no protected-environment concept to gate a secret behind, and restricting dispatch would close nothing anyway, because `docker-build.yml`''s head-resolved `pull_request:` runs attacker-authored YAML, which reaches EVERY secret in the store and not merely the ones the committed workflows name. The PR route and the `v*` tag push, not dispatch, were the live residuals; both were addressed in #885 — the tag push is whitelisted to the release operator (`release.tag-protection-v-star`) and NOTHING on the `pull_request` route names a stored secret any more — no job and no workflow scope (`ci.pr-route-carries-no-stored-credential`), so the `container:` pull, `ETV_REGISTRY_AUTH` and `ETV_STATUS_AUTH` are all gone from that route and `REGISTRY_PASSWORD` reaches only `build`. That bounds the blast radius; it does not close the route, because head-supplied YAML can still name any secret in the store.'
|
||||
signals: 'admin password in CI secrets, registry credential scope, ETV_STATUS_AUTH can write statuses, forge review-verdict/h10, head-resolved workflow holds credentials, persist-credentials on actions/checkout, credential left in .git/config, masked git fetch yields empty changed set, Gitea token scopes, no status scope, write:package vs write:repository, permissions key no-op, GITEA_TOKEN default read/write, Restricted default token permissions, orphan secret, deploy key in secret store, toolchain image overwrite, prod floating tag write · paths: `.gitea/workflows/docker-build.yml`, `.gitea/workflows/pr-checks.yml`, `.gitea/workflows/dependency-scan.yml`, `.gitea/workflows/ci-image.yml`, `.gitea/workflows/renovate.yml`, `scripts/ci-detect-already-validated.sh` · issues: #697, #672, #698, #742, #743, #746, #748, #835, #420, server-management#714'
|
||||
mechanics: 'PAT `ci-registry-scoped-697`, scopes `write:package,read:repository`, stored as repo Actions secret `REGISTRY_PASSWORD`; `REGISTRY_USER` remains `timothy`. Verified 2026-08-05 on Gitea 1.25.4: registry push of a probe tag SUCCEEDED; `GET /commits/{sha}/status` 200; `POST /statuses/{sha}` REFUSED HTTP 403 `token does not have at least one of required scope(s), required=[write:repository], token scope=write:package,read:repository`. Probe artifacts deleted, confirmed 404. RE-PROBED 2026-09-02 on Gitea 1.27.1 (#869) in a throwaway repo with a freshly minted PAT of the same two scopes: same 403 with the same message, plus a read control (same token GET -> 200) and a write control (admin POST -> 201); repo and token deleted, deletion confirmed. NOT COVERED BY EITHER PROBE (2026-08-05 or the #869 re-probe) — note this means not probed, NOT unexercised, and the `container:` pull is in fact exercised by the shipped credential on every PR: the `container:` pull, the buildcache write and the base-image pull. Those rest on Gitea''s scope model (write implies read per category, re-read at tag `v1.27.1` 2026-09-02) — INFERRED. Note WHICH run proves which: only the `container:` pull is exercised by a PR. `cache-to`/`cache-from` and the base-image pull are confined to the `build` job, which carries `if: github.event_name != ''pull_request''`, so they are first exercised on the post-merge push to `main` — AFTER the merge gate has passed. A wrong inference there reddens main, not the PR.'
|
||||
mechanics: 'PAT `ci-registry-scoped-697`, scopes `write:package,read:repository`, stored as repo Actions secret `REGISTRY_PASSWORD`; `REGISTRY_USER` remains `timothy`. Verified 2026-08-05 on Gitea 1.25.4: registry push of a probe tag SUCCEEDED; `GET /commits/{sha}/status` 200; `POST /statuses/{sha}` REFUSED HTTP 403 `token does not have at least one of required scope(s), required=[write:repository], token scope=write:package,read:repository`. Probe artifacts deleted, confirmed 404. RE-PROBED 2026-09-02 on Gitea 1.27.1 (#869) in a throwaway repo with a freshly minted PAT of the same two scopes: same 403 with the same message, plus a read control (same token GET -> 200) and a write control (admin POST -> 201); repo and token deleted, deletion confirmed. NOT COVERED BY EITHER PROBE (2026-08-05 or the #869 re-probe): the `container:` pull, the buildcache write and the base-image pull. Those rest on Gitea''s scope model (write implies read per category, re-read at tag `v1.27.1` 2026-09-02) — INFERRED. Note WHICH run exercises which, and that this CHANGED on 2026-09-04 (#885). The buildcache write and the base-image pull are confined to the `build` job, which carries `if: github.event_name != ''pull_request''`. The `container:` pull is not in `build` at all — `build` declares no `container:` — and it is the one a PR used to exercise with this credential; the five `container:` blocks now declare no `credentials:` and pull ANONYMOUSLY, so there is no credential left there to infer about (`ci.pr-route-carries-no-stored-credential`). No PR run therefore exercises this PAT at all, and the two credentialed uses are first exercised on the post-merge push to `main`, AFTER the merge gate has passed, so a wrong inference reddens main, not the PR.'
|
||||
---
|
||||
|
||||
**What was wrong.** `REGISTRY_USER`/`REGISTRY_PASSWORD` were the **admin account's** basic auth, and
|
||||
@@ -218,9 +218,13 @@ inherited, which is the residual the allow-list explicitly does not touch. **Whi
|
||||
was settled in #853** (`ci.workflow-dispatch-ref-unrestricted`): not the four unrestricted
|
||||
`workflow_dispatch` triggers but `docker-build.yml`'s head-resolved `pull_request:`, which runs
|
||||
attacker-authored YAML and therefore reaches EVERY secret in the store, not only the ones the
|
||||
committed workflows name — six of its jobs already hold `REGISTRY_PASSWORD` on that route, two of them
|
||||
required contexts. Residuals in #885. `docker-build.yml` publishes `:prod` from a `v*` tag push and a tag
|
||||
may point at ANY commit — a prod image with no PR, review or status (tag protections are empty).
|
||||
committed workflows name. Six of its jobs held `REGISTRY_PASSWORD` on that route, two of them required
|
||||
contexts; #885 removed all six (`ci.pr-route-carries-no-stored-credential`), which narrows the blast
|
||||
radius and leaves the ROUTE exactly as cheap as before — the store, not the committed references, is
|
||||
what head YAML reaches. `docker-build.yml` publishes `:prod` from a `v*` tag push and a tag may point
|
||||
at ANY commit — a prod image with no PR, review or status. Since #885 that push is whitelisted to
|
||||
`timothy` (`release.tag-protection-v-star`), which closes it against the bot and not against the
|
||||
admin.
|
||||
**And none of it was necessary: direct pushes to `main` were server-side permitted, so the gate was
|
||||
bypassable with no forgery at all (#743).** That route is now closed — `main` carries
|
||||
`enable_push: false` (`release.main-direct-push-disabled`), which removes `main` as a destination for
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -6,8 +6,8 @@ since: '2026-08-30'
|
||||
supersedes: none
|
||||
superseded-by: none
|
||||
stale-after: '2027-02-28'
|
||||
rule: 'Gitea 1.27.1 offers NO mechanism to restrict `workflow_dispatch` by ref, and has no protected-environment concept at all — PROBED across the REST API, the loaded config and the CLI, not assumed (the WEB UI was not swept; the body says why that is acceptable here and where it would matter). The dispatch body schema `CreateActionWorkflowDispatch` makes `ref` a required free-form string with no allow-list or pattern field; zero of the 308 documented API paths contain "environment", and Actions secrets exist only at org/repo/user scope with no per-ref or per-environment gate; `/api/v1/settings/actions` 404s; the config file the running server actually loads (`/etc/gitea/app.ini`, named by its own `--config`) sets only `ENABLED` and `DEFAULT_ACTIONS_URL` under `[actions]`; and the `gitea` CLI exposes exactly ONE Actions subcommand, `gitea actions generate-runner-token`, which registers a runner and restricts nothing. Treat the VERSION, not the `stale-after` date, as the real trigger to re-probe: an upgrade past 1.27.1 invalidates every capability claim here the day it lands, months before the date fires. The four unrestricted dispatches (`ci-image.yml`, `docker-build.yml`, `dependency-scan.yml`, `renovate.yml`) are therefore ACCEPTED — but the operative reason is NOT "repository write access is the boundary", which is the argument to avoid because it is unfalsifiable and it hides the real route. The operative reason is that **dispatch is not the cheapest route to ANY of it**: `docker-build.yml` triggers on `pull_request:`, which Gitea resolves from the PR HEAD, so that route executes ATTACKER-AUTHORED YAML — and such YAML can name any secret in the repo store, not merely the ones the committed workflows happen to reference (`ci.gate-trigger-base-resolved`, verbatim: "any PR-added workflow can reference `RENOVATE_TOKEN`, a `write:repository` bot PAT in the same store"). Label that step honestly: it is INFERRED from the repo-scoped secret model plus that record, NOT measured here, because the measurement would print a live credential into a run log. That generalizing step is what makes the argument cover all four rather than just the registry pair: `renovate.yml`''s `RENOVATE_TOKEN`/`GH_COM_TOKEN` are reachable from a PR without dispatching `renovate.yml` at all, and `dependency-scan.yml` references no `secrets.` whatever — which corrects #853''s own table row for it. On the registry credential as it stands, SIX jobs in `docker-build.yml` hold `REGISTRY_PASSWORD` and run on the PR route (`toolchain-preflight`, `test`, `migrations`, `functional-e2e`, `api-docs`, `format`), two of them — `test` and `migrations` — branch-protection required contexts per `.gitea/required-status-contexts.json`; carry that as the INVARIANT "every job on the PR route that NAMES `secrets.REGISTRY_PASSWORD`", never as the six-name list, because a remediation scoped to a stale list misses whatever lands next. Resist the tempting "every `container:` job" — `toolchain-preflight` is deliberately container-free and takes the credential through `ETV_REGISTRY_AUTH`, so that predicate names five of the six and reproduces on day one the exact staleness it was written to prevent. "Deliberate act" throughout carries `ci.toolchain-image-publish-is-a-dispatch`''s sense — an act OUTSIDE the ordinary contribution flow, not a raw step count: opening a PR costs zero such acts and a dispatch costs one. Restricting dispatch would therefore close the more visible route and change nothing. The residuals worth tracking are the PR route AND the `v*` tag push — a single act, explicitly outside `release.main-direct-push-disabled` — both in #885, not dispatch.'
|
||||
signals: 'workflow_dispatch ref restriction, restrict dispatch by branch, protected environment, environment-gated secret, Gitea 1.27.1 actions capabilities, settings/actions 404, attacker-supplied workflow YAML, head-resolved pull_request trigger, PR run gets repo secrets, REGISTRY_PASSWORD reachable from a PR, tag_protections empty, renovate bot write scope · paths: `.gitea/workflows/ci-image.yml`, `.gitea/workflows/docker-build.yml`, `.gitea/workflows/dependency-scan.yml`, `.gitea/workflows/renovate.yml`, `docs/ci-cd.md` · issues: #853, #744, #885, #697, #742, #748, #845'
|
||||
rule: 'Gitea 1.27.1 offers NO mechanism to restrict `workflow_dispatch` by ref, and has no protected-environment concept at all — PROBED across the REST API, the loaded config and the CLI, not assumed (the WEB UI was not swept; the body says why that is acceptable here and where it would matter). The dispatch body schema `CreateActionWorkflowDispatch` makes `ref` a required free-form string with no allow-list or pattern field; zero of the 308 documented API paths contain "environment", and Actions secrets exist only at org/repo/user scope with no per-ref or per-environment gate; `/api/v1/settings/actions` 404s; the config file the running server actually loads (`/etc/gitea/app.ini`, named by its own `--config`) sets only `ENABLED` and `DEFAULT_ACTIONS_URL` under `[actions]`; and the `gitea` CLI exposes exactly ONE Actions subcommand, `gitea actions generate-runner-token`, which registers a runner and restricts nothing. Treat the VERSION, not the `stale-after` date, as the real trigger to re-probe: an upgrade past 1.27.1 invalidates every capability claim here the day it lands, months before the date fires. The four unrestricted dispatches (`ci-image.yml`, `docker-build.yml`, `dependency-scan.yml`, `renovate.yml`) are therefore ACCEPTED — but the operative reason is NOT "repository write access is the boundary", which is the argument to avoid because it is unfalsifiable and it hides the real route. The operative reason is that **dispatch is not the cheapest route to ANY of it**: `docker-build.yml` triggers on `pull_request:`, which Gitea resolves from the PR HEAD, so that route executes ATTACKER-AUTHORED YAML — and such YAML can name any secret in the repo store, not merely the ones the committed workflows happen to reference (`ci.gate-trigger-base-resolved`, verbatim: "any PR-added workflow can reference `RENOVATE_TOKEN`, a `write:repository` bot PAT in the same store"). Label that step honestly: it is INFERRED from the repo-scoped secret model plus that record, NOT measured here, because the measurement would print a live credential into a run log. That generalizing step is what makes the argument cover all four rather than just the registry pair: `renovate.yml`''s `RENOVATE_TOKEN`/`GH_COM_TOKEN` are reachable from a PR without dispatching `renovate.yml` at all, and `dependency-scan.yml` references no `secrets.` whatever — which corrects #853''s own table row for it. On the registry credential, SIX jobs in `docker-build.yml` held `REGISTRY_PASSWORD` on the PR route (`toolchain-preflight`, `test`, `migrations`, `functional-e2e`, `api-docs`, `format`), two of them — `test` and `migrations` — branch-protection required contexts per `.gitea/required-status-contexts.json`. That is CLOSED as of 2026-09-04 (#885, `ci.pr-route-carries-no-stored-credential`): nothing on the `pull_request` route names any stored secret — no job and no workflow scope — the toolchain image is pulled anonymously and the commit-status API read unauthenticated, and the PAT survives in `build`, which the route cannot reach. The invariant it is held by is "no part of a `pull_request`-triggered workflow may name a `secrets.*`" — every job the trigger reaches AND the workflow scope outside `jobs:`, since a root `env:`/`defaults:` is materialised into every job and no job-level `if:` can take it off the route — DERIVED from the git index by `scripts/tests/test_workflow_persist_credentials.py::test_no_PULL_REQUEST_route_job_names_a_STORED_secret`, never the six-name list, and never "every `container:` job" — `toolchain-preflight` was deliberately container-free and took the credential through `ETV_REGISTRY_AUTH`, so that predicate names five of the six and reproduces on day one the exact staleness it was written to prevent. NOTE WHAT THAT DID NOT CHANGE, which is this record''s whole point: head-supplied YAML can still name every secret in the store, so the PR route is still cheaper than any dispatch. "Deliberate act" throughout carries `ci.toolchain-image-publish-is-a-dispatch`''s sense — an act OUTSIDE the ordinary contribution flow, not a raw step count: opening a PR costs zero such acts and a dispatch costs one. Restricting dispatch would therefore close the more visible route and change nothing. The residuals were the PR route AND the `v*` tag push — a single act, explicitly outside `release.main-direct-push-disabled` — and both were addressed in #885 rather than by restricting dispatch: the tag push is now whitelisted to the release operator (`release.tag-protection-v-star`) and the PR route carries no stored credential. Neither makes dispatch worth restricting, because the remaining exposure — head YAML naming any secret in the store — is reached by opening a PR and is unaffected by either.'
|
||||
signals: 'workflow_dispatch ref restriction, restrict dispatch by branch, protected environment, environment-gated secret, Gitea 1.27.1 actions capabilities, settings/actions 404, attacker-supplied workflow YAML, head-resolved pull_request trigger, PR run gets repo secrets, REGISTRY_PASSWORD reachable from a PR, tag_protections, v* tag whitelist, renovate bot write scope · paths: `.gitea/workflows/ci-image.yml`, `.gitea/workflows/docker-build.yml`, `.gitea/workflows/dependency-scan.yml`, `.gitea/workflows/renovate.yml`, `docs/ci-cd.md` · issues: #853, #744, #885, #697, #742, #748, #845'
|
||||
mechanics: '`docs/ci-cd.md` -> "CI toolchain image" -> "Publishing from a branch is a dispatch, not a push"'
|
||||
sources: 'Gitea 1.27.1 probe, 2026-08-30 — `swagger.v1.json` (308 paths, 0 matching "environment"); `CreateActionWorkflowDispatch` schema; live `tag_protections` = `[]`; live `branch_protections` = one `main` rule; `/api/v1/admin/users` = exactly `timothy` (admin) + `renovate` (non-admin), with `permission: write` from `/repos/timothy/ersatztv/collaborators/renovate/permission`; host `app.ini` `[actions]`; differential dispatch-authorization probe with anonymous and authenticated-non-writer controls'
|
||||
---
|
||||
@@ -47,16 +47,21 @@ sources: 'Gitea 1.27.1 probe, 2026-08-30 — `swagger.v1.json` (308 paths, 0 mat
|
||||
into a run log. **That is the step that makes this
|
||||
cover all four dispatches**, `renovate.yml` included, even though `renovate.yml` has no
|
||||
`pull_request:` trigger of its own; without it the argument would reach only the registry pair.
|
||||
Concretely on the registry credential today, SIX jobs hold `REGISTRY_PASSWORD` and run on the PR
|
||||
route — `toolchain-preflight`, `test`, `migrations`, `functional-e2e`, `api-docs`, `format` — of
|
||||
which `test` and `migrations` are both branch-protection required contexts
|
||||
(`.gitea/required-status-contexts.json`). **Carry the invariant, not the list:** *every job on the PR
|
||||
route that names `secrets.REGISTRY_PASSWORD`*, because a remediation scoped to today's six names
|
||||
silently misses the seventh. Not "every `container:` job" — `toolchain-preflight` is deliberately
|
||||
container-free (`runs-on: small`, credential via `ETV_REGISTRY_AUTH`), so that predicate names five of
|
||||
six and is stale before it ships.
|
||||
So "push a branch, open a PR" reaches the credential with **no act outside the ordinary contribution
|
||||
flow**, where a dispatch costs one — that is this repo's sense of "deliberate"
|
||||
On the registry credential, SIX jobs held `REGISTRY_PASSWORD` on the PR route —
|
||||
`toolchain-preflight`, `test`, `migrations`, `functional-e2e`, `api-docs`, `format` — of which
|
||||
`test` and `migrations` are both branch-protection required contexts
|
||||
(`.gitea/required-status-contexts.json`). That was closed in #885
|
||||
(`ci.pr-route-carries-no-stored-credential`), by the invariant and not by the list: *no part of a
|
||||
`pull_request`-triggered workflow may name a `secrets.*`* — neither a job the trigger reaches nor
|
||||
the workflow scope outside `jobs:`, whose `env:`/`defaults:` are materialised into every job —
|
||||
derived from the git index, because a remediation scoped to those six names silently misses the
|
||||
seventh. Not "every `container:` job" either — `toolchain-preflight` is deliberately
|
||||
container-free (`runs-on: small`) and took the credential through `ETV_REGISTRY_AUTH`, so that
|
||||
predicate names five of six and is stale before it ships.
|
||||
**The route itself is untouched by that fix**, which is why this record still stands: head-supplied
|
||||
YAML can name any secret in the store, so "push a branch, open a PR" still reaches all of them with
|
||||
**no act outside the ordinary contribution flow**, where a dispatch costs one — that is this
|
||||
repo's sense of "deliberate"
|
||||
(`ci.toolchain-image-publish-is-a-dispatch`), not a raw step count, which would read 2 vs 2. Any
|
||||
control that restricted dispatch and left this open would close the more visible route and report
|
||||
progress that did not happen. Per-`if:` ref gates on the publish steps do not change this: they live
|
||||
@@ -76,11 +81,10 @@ sources: 'Gitea 1.27.1 probe, 2026-08-30 — `swagger.v1.json` (308 paths, 0 mat
|
||||
with `"not found"` while `renovate` on this repo reaches ref resolution with `"ref ... doesn't
|
||||
exist"` — the same response a known writer gets — so `renovate` passes the dispatch authorization
|
||||
gate.
|
||||
- **One cheap control exists and was deliberately NOT taken here: `tag_protections` is empty.** Gitea
|
||||
1.27.1 does support tag protection (`name_pattern` plus a username/team whitelist), and a `v*` rule
|
||||
whitelisting the release operator would close `docker-build.yml`'s tag-push row against the bot at
|
||||
no operational cost, since only the operator cuts releases. It is not applied in this change because
|
||||
it is live branch-protection-class configuration whose failure mode is a broken release cut, which
|
||||
deserves its own change and its own verification that a legitimate tag push still succeeds — not a
|
||||
rider on a decision record. It carries no weight against the admin actor either way. Tracked with
|
||||
the `pull_request:` residual in #885.
|
||||
- **One cheap control was identified here and applied in #885: the `v*` tag rule.** Gitea 1.27.1
|
||||
supports tag protection (`name_pattern` plus a username/team whitelist), and `tag_protections` was
|
||||
empty when this record was written (2026-08-30), leaving `docker-build.yml`'s tag-push row open
|
||||
against the bot. It was deliberately not applied as a rider on a decision record — its failure mode
|
||||
is a broken release cut, so it earned its own change and its own verification. It now exists as
|
||||
`v*` whitelisted to `timothy` (`release.tag-protection-v-star`), and it carries no weight against
|
||||
the admin actor, which was true when it was proposed and is true now.
|
||||
|
||||
@@ -5,9 +5,9 @@ status: active
|
||||
since: '2026-08-05'
|
||||
supersedes: none
|
||||
superseded-by: none
|
||||
rule: 'Branch protection on `main` carries `enable_push: false` AND `block_admin_merge_override: true`. Both halves are required and neither is sufficient. `enable_push: false` removes the direct-push path, leaving the PR merge path — the only path on which Gitea evaluates `status_check_contexts`, and therefore the only path on which `review-verdict/h10` is consulted at all. `block_admin_merge_override: true` then closes the force-merge bypass on that remaining path: with it false (the default), `CanBypassBranchProtection` returns true for a repo admin, so `POST /pulls/{n}/merge` with `force_merge: true` merges a PR whose `h10` is missing or red — one API call, no forgery, no PATCH. Do NOT "soften" the push half to a push WHITELIST: measured here, a whitelist naming `timothy` still admits the push, and `timothy` is the identity every agent session, PAT and injected `GITEA_TOKEN` already acts as, so the whitelist form closes nothing while reading in review as a control. Same reasoning is why the admin-override half is needed: an admin-shaped control that exempts the only admin exempts everybody. What remains open: a credential that can PATCH branch protection off can still undo either half — an accepted residual, not a closed route. Tag pushes are unaffected (`tag_protections` governs those separately), so the release cut still works.'
|
||||
rule: 'Branch protection on `main` carries `enable_push: false` AND `block_admin_merge_override: true`. Both halves are required and neither is sufficient. `enable_push: false` removes the direct-push path, leaving the PR merge path — the only path on which Gitea evaluates `status_check_contexts`, and therefore the only path on which `review-verdict/h10` is consulted at all. `block_admin_merge_override: true` then closes the force-merge bypass on that remaining path: with it false (the default), `CanBypassBranchProtection` returns true for a repo admin, so `POST /pulls/{n}/merge` with `force_merge: true` merges a PR whose `h10` is missing or red — one API call, no forgery, no PATCH. Do NOT "soften" the push half to a push WHITELIST: measured here, a whitelist naming `timothy` still admits the push, and `timothy` is the identity every agent session, PAT and injected `GITEA_TOKEN` already acts as, so the whitelist form closes nothing while reading in review as a control. Same reasoning is why the admin-override half is needed: an admin-shaped control that exempts the only admin exempts everybody. What remains open: a credential that can PATCH branch protection off can still undo either half — an accepted residual, not a closed route. Tag pushes are governed by `tag_protections`, an entirely separate mechanism that does NOT inherit from this rule; since #885 it carries one entry, `v*` whitelisted to `timothy` (`release.tag-protection-v-star`), which is intended to leave the release cut working for the operator while refusing a `v*` push by the `renovate` bot that would publish a `:prod` image. Both halves are EXPECTED, UNVERIFIED: only the `timothy` credential exists here, so neither a real release cut nor a refused bot push has been exercised, and that record carries the same caveat.'
|
||||
signals: 'direct push to main, push whitelist, enable_push false, branch protection bypass, review-verdict/h10 bypassable without forging, merge consent derived not asserted, pre-receive hook declined, Not allowed to push to protected branch, protected branch, tag_protections, release tag push, GITEA_TOKEN repo write, RENOVATE_TOKEN, site admin bypass, PR-only flow · paths: `docs/ci-cd.md` · issues: #743, #697, #698, #622, #672, #706, #742, server-management#714'
|
||||
mechanics: 'Gitea 1.27.1. `PATCH /api/v1/repos/timothy/ersatztv/branch_protections/main` with `{"enable_push": false, "block_admin_merge_override": true}`; whitelist fields left off (`enable_push_whitelist: false`, empty arrays), `enable_force_push: false`, `enable_merge_whitelist: false`, `required_approvals: 0`. MEASURED 2026-08-05 against a throwaway `probe-743-*` rule rather than against `main`: with `enable_push: false` a push by `timothy` (site admin) was REFUSED — `pre-receive hook declined`, `Not allowed to push to protected branch`; after PATCHing the same rule to `enable_push: true` + `enable_push_whitelist: true` + `push_whitelist_usernames: ["timothy"]` the identical push SUCCEEDED. Separately probed on a second throwaway rule: a contents-API write (`PUT /repos/{o}/{r}/contents/{path}` with `branch` set to the protected branch) was REFUSED HTTP 403 `user cannot commit to repo [user: timothy]` — so the web-editor/API file-write surface does not bypass it either. Then on `main` itself: `git push origin HEAD:main` REFUSED, and a tag-only push SUCCEEDED from the same worktree. `GET .../tag_protections` returns `[]`; repo is `fork: false`, `mirror: false`. All probe artifacts (two rules, two branches, one tag) deleted and confirmed gone; `origin/main` head unchanged at `08e95f9ec` throughout. MEASURED 2026-08-28 on 1.27.1 (#747), superseding the earlier source-attested-only note on this field. Four SEPARATE throwaway scratch bases, each with its own protection rule, head branch and PR (they differ by construction, not by mutating one rule in place — arm A merged, so its PR could not be re-used); `main` was never merge-probed and its rule never PATCHed, `updated_at` unchanged at 2026-08-05. Each base required one probe-named context that was never posted, except in the control step below. `scratch/747-base` (#863), field at its `false` default, ORDINARY merge: REFUSED HTTP 405 `Not all required status checks successful`; posting only that context as `success` and repeating the ordinary merge on the same PR returned HTTP 200 — the one attribution control in the set, and the only variable between those two calls is the context. `scratch/747a-base` (#864), field `false`, admin `force_merge`: MERGED HTTP 200. `scratch/747b-base` (#865), rule identical but for the field set `true`, same call: REFUSED HTTP 405 — the A/B pair is the evidence that the field governs the force path; B''s 405 has no control of its own. `scratch/747c-base` (#866), rule as A but with `enable_bypass_allowlist: true` and an EMPTY `bypass_allowlist_usernames`: MERGED HTTP 200, so in THAT configuration the allowlist does not substitute for `block_admin_merge_override`; a non-empty list was not tested, and this cannot distinguish `does not govern admins` from `empty list treated as unconfigured`. Probing never required merging an unreviewed PR into `main`: a scratch BASE branch carries its own rule, so the whole experiment is disposable. All #747 probe artifacts (four scratch bases — `scratch/747-base` for the ordinary-merge pair plus `scratch/747{a,b,c}-base` for the three force-merge arms — each with its own protection rule and head branch, so four rules and eight branches, and PRs #863-#866) deleted and confirmed gone; `origin/main` head unchanged at `b16ec15d6` throughout, and `main`''s own rule was never PATCHed.'
|
||||
mechanics: 'Gitea 1.27.1. `PATCH /api/v1/repos/timothy/ersatztv/branch_protections/main` with `{"enable_push": false, "block_admin_merge_override": true}`; whitelist fields left off (`enable_push_whitelist: false`, empty arrays), `enable_force_push: false`, `enable_merge_whitelist: false`, `required_approvals: 0`. MEASURED 2026-08-05 against a throwaway `probe-743-*` rule rather than against `main`: with `enable_push: false` a push by `timothy` (site admin) was REFUSED — `pre-receive hook declined`, `Not allowed to push to protected branch`; after PATCHing the same rule to `enable_push: true` + `enable_push_whitelist: true` + `push_whitelist_usernames: ["timothy"]` the identical push SUCCEEDED. Separately probed on a second throwaway rule: a contents-API write (`PUT /repos/{o}/{r}/contents/{path}` with `branch` set to the protected branch) was REFUSED HTTP 403 `user cannot commit to repo [user: timothy]` — so the web-editor/API file-write surface does not bypass it either. Then on `main` itself: `git push origin HEAD:main` REFUSED, and a tag-only push SUCCEEDED from the same worktree. `GET .../tag_protections` returned `[]` at that probe; since #885 it carries one `v*` rule whitelisted to `timothy` (`release.tag-protection-v-star`). Repo is `fork: false`, `mirror: false`. All probe artifacts (two rules, two branches, one tag) deleted and confirmed gone; `origin/main` head unchanged at `08e95f9ec` throughout. MEASURED 2026-08-28 on 1.27.1 (#747), superseding the earlier source-attested-only note on this field. Four SEPARATE throwaway scratch bases, each with its own protection rule, head branch and PR (they differ by construction, not by mutating one rule in place — arm A merged, so its PR could not be re-used); `main` was never merge-probed and its rule never PATCHed, `updated_at` unchanged at 2026-08-05. Each base required one probe-named context that was never posted, except in the control step below. `scratch/747-base` (#863), field at its `false` default, ORDINARY merge: REFUSED HTTP 405 `Not all required status checks successful`; posting only that context as `success` and repeating the ordinary merge on the same PR returned HTTP 200 — the one attribution control in the set, and the only variable between those two calls is the context. `scratch/747a-base` (#864), field `false`, admin `force_merge`: MERGED HTTP 200. `scratch/747b-base` (#865), rule identical but for the field set `true`, same call: REFUSED HTTP 405 — the A/B pair is the evidence that the field governs the force path; B''s 405 has no control of its own. `scratch/747c-base` (#866), rule as A but with `enable_bypass_allowlist: true` and an EMPTY `bypass_allowlist_usernames`: MERGED HTTP 200, so in THAT configuration the allowlist does not substitute for `block_admin_merge_override`; a non-empty list was not tested, and this cannot distinguish `does not govern admins` from `empty list treated as unconfigured`. Probing never required merging an unreviewed PR into `main`: a scratch BASE branch carries its own rule, so the whole experiment is disposable. All #747 probe artifacts (four scratch bases — `scratch/747-base` for the ordinary-merge pair plus `scratch/747{a,b,c}-base` for the three force-merge arms — each with its own protection rule and head branch, so four rules and eight branches, and PRs #863-#866) deleted and confirmed gone; `origin/main` head unchanged at `b16ec15d6` throughout, and `main`''s own rule was never PATCHed.'
|
||||
---
|
||||
|
||||
**Why a whitelist was the wrong shape.** #743 proposed "a push whitelist on `main` (or disable direct
|
||||
@@ -109,11 +109,13 @@ mirror (not applicable: `fork: false`, `mirror: false`). Merge remains the one i
|
||||
|
||||
**Why the release cut does not deadlock.** #743 flagged that the tag path had to keep working, and
|
||||
#719 documents H11 blocking a tag-only push on every release cut. Branch protection is scoped to
|
||||
`refs/heads/main`; tags are governed by an entirely separate mechanism, and `tag_protections` on this
|
||||
repo is empty, so tag pushes are unrestricted by anything except ordinary write permission. Demonstrated
|
||||
rather than assumed: from one worktree, the branch push to `main` was refused and a tag push succeeded.
|
||||
Do not conflate the two mechanisms — disabling branch push says nothing about tags, and a future
|
||||
tag-protection rule would not inherit from this one.
|
||||
`refs/heads/main`; tags are governed by an entirely separate mechanism. `tag_protections` was empty
|
||||
when this record was written (measured 2026-08-24), so tag pushes were unrestricted by anything except
|
||||
ordinary write permission. Demonstrated rather than assumed: from one worktree, the branch push to
|
||||
`main` was refused and a tag push succeeded. Do not conflate the two mechanisms — disabling branch
|
||||
push says nothing about tags, and the tag-protection rule #885 later added
|
||||
(`release.tag-protection-v-star`, `v*` whitelisted to `timothy`) does not inherit from this one in
|
||||
either direction.
|
||||
|
||||
**The `docker-build.yml` `persist-credentials` question (#743's fourth box), since resolved in #746.**
|
||||
Head-resolved jobs kept a write-capable credential in `.git/config` because their `actions/checkout`
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
---
|
||||
key: release.tag-protection-v-star
|
||||
title: '2026-09-04 — `v*` tag pushes are whitelisted to the release operator, closing the tag-push route against the bot (#885)'
|
||||
status: active
|
||||
since: '2026-09-04'
|
||||
supersedes: none
|
||||
superseded-by: none
|
||||
rule: 'The repository carries one tag-protection rule: `name_pattern: v*`, `whitelist_usernames: ["timothy"]`. This matters because `docker-build.yml` triggers on `push.tags: [v*]` and a `v*` tag push BUILDS AND PUBLISHES the `:prod` image — the tag the `jazz-media` Komodo stack follows. `release.main-direct-push-disabled` closed every route to `main`, and left this one open: a tag push is a separate mechanism that branch protection does not govern, so it was the cheapest remaining single act that reaches production. It buys NOTHING against the admin actor (`timothy` is on the whitelist, and no self-administered control bounds an owner) and everything it buys is against `renovate`, the repo''s other write-capable account (`permission: write`, `is_admin: false` from `GET /repos/timothy/ersatztv/collaborators/renovate/permission`, measured in #853 2026-08-30 and re-read 2026-09-04): the H10 verdict gate and the Renovate exemption rule bound what that bot can get MERGED, and nothing bounded what it could TAG. The rule''s failure mode is a BROKEN RELEASE CUT, so it is verified in both directions before it is trusted: the negative half is done and recorded below; the POSITIVE half — a real `v*` release-cut tag push by `timothy` still succeeding — is DEFERRED to the operator''s next real cut, because pushing a `v*` tag from any working session publishes `:prod` and a release is the operator''s act, not a verification step. Treat that as OPEN until a cut lands. There is no committed mirror of this rule: it lives in Gitea, and this record''s `mechanics:` carries the exact call and the read-back, the same shape `release.main-direct-push-disabled` uses.'
|
||||
signals: 'tag_protections empty, v* tag push publishes prod, release cut blocked, renovate can push a tag, tag protection whitelist, release operator whitelist, prod image published by tag, deferred positive verification · paths: `.gitea/workflows/docker-build.yml` · issues: #885, #853, #743, #698, #742'
|
||||
mechanics: 'Applied 2026-09-04 with `POST /api/v1/repos/timothy/ersatztv/tag_protections` body `{"name_pattern": "v*", "whitelist_usernames": ["timothy"]}`, which returned `{"id": 1, "name_pattern": "v*", "whitelist_usernames": ["timothy"], "whitelist_teams": [], "created_at": "2026-09-04T23:18:50+02:00"}`. Read back the same day with `GET .../tag_protections` -> the one rule, fields as echoed. It is reversible with `DELETE .../tag_protections/1`. NOTE THE ROUTE NAME: `/repos/{owner}/{repo}/tags/protection` is NOT it — measured 2026-09-04, that path resolves `protection` as a TAG NAME and answers 404 `{"message": "tag doesn''t exist: protection"}`. Before this the endpoint returned `[]` (measured #743 2026-08-24 and again 2026-09-04). NEGATIVE VERIFICATION available from this session and performed: a NON-`v*` tag (`probe-885-tagpush`) pushed by `timothy` succeeded and was deleted, confirmed gone by `git ls-remote --tags` — so tag pushes still work at all and the rule is not mis-scoped to `*`. NOT VERIFIED, and it cannot be from a working session: that a real `v*` release-cut push still succeeds, and that a push by `renovate` is refused (only the `timothy` credential is available here). The first is the operator''s next cut; the second would require the bot''s credential.'
|
||||
---
|
||||
|
||||
**What this closes.** `release.main-direct-push-disabled` made every change to `main` go through a PR
|
||||
with a sha-bound review verdict, and said in the same breath that "tag pushes are unaffected
|
||||
(separate mechanism)". That sentence was accurate and was also the gap: `docker-build.yml` publishes
|
||||
`:prod` on a `v*` tag, so one tag push reaches the image prod's stack follows, with no PR, no review
|
||||
verdict and no merge gate anywhere in the path. `tag_protections` was empty, so nothing stood in it.
|
||||
|
||||
**What it does not close, said plainly.** Against `timothy` it is decoration: the whitelist contains
|
||||
that account, and an owner can delete the rule with one API call anyway. No self-administered control
|
||||
is a boundary against the actor who administers it. The rule is worth having because the actor set
|
||||
has a *second* member — `renovate` holds repository write, opens PRs on its own, and is bounded on
|
||||
the merge path by `review-verdict/h10` and the exemption rule (#698, #742, #845) while being
|
||||
completely unbounded on the tag path. This makes the two paths agree.
|
||||
|
||||
**Why it was split out of #853 rather than bundled.** Its failure mode is not a leaked credential, it
|
||||
is a release cut that will not push — a failure that surfaces at the worst moment, when someone is
|
||||
trying to ship. That earns its own change and its own verification, which is why the positive half is
|
||||
recorded above as deferred rather than quietly assumed. A rule whose only failure mode is untested is
|
||||
a rule that gets deleted in a hurry by whoever hits it first.
|
||||
|
||||
**How to unblock a release cut that this rule breaks.** `DELETE /api/v1/repos/timothy/ersatztv/tag_protections/1`,
|
||||
push the tag, re-`POST` the rule. Prefer adding the pushing account to `whitelist_usernames` with a
|
||||
`PATCH` if the cut is moving to a different operator, so the protection is never left off.
|
||||
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
@@ -133,7 +133,7 @@ classifications differ; otherwise the strictest applies and the Note names the e
|
||||
| `scripts/ci-peak-anon.sh` | `N/A` | Reads no live remote state — samples the runner's local cgroup `memory.stat`/`memory.peak`. |
|
||||
| `scripts/ci-prove-ban-detects.sh` | `N/A` | Reads no live remote state — mutates a local workflow copy and runs pytest against the local checkout. |
|
||||
| `scripts/ci-step-ran.sh` | `N/A` | Reads no live remote state — reads runner-supplied env vars and local marker files it wrote itself. |
|
||||
| `scripts/ci-toolchain-image-resolves.sh` — registry manifest read for the pinned toolchain tag | `UNSAFE-KNOWN` | Reads a MUTABLE identifier (a registry tag) with nothing re-checking it before the `container:` jobs pull, so a tag deleted between the preflight and the pull is reported as present. Graded `UNSAFE-KNOWN` rather than `N/A` deliberately: nothing proceeds on the strength of the read — it can only turn its own job red, which is not nothing (the merge-consent hook denies on the COMBINED status, ersatztv#598) but is not authorization either — while a stale PASS is read by a human as "the image is fine", which is an assertion about remote state this file exists to grade. The residual is bounded by what it degrades to: a stale pass leaves exactly the pre-#772 behaviour (five jobs failing at pull), never anything that proceeds on the strength of the read. The opposite error is closed by the EXIT CODE rather than by wording: an unusable credential, an unverifiable answer (after retries) and an HTTP 200 whose body is not a manifest all FAIL the job. THE TRAP, since warning on those and exiting 0 is the natural way to write this check: a missing `curl`, a moved registry or a DNS change all land there, and a green-with-a-warning job is indistinguishable from a healthy pin forever after — "the check could not run" presenting as "the pin is fine", which is precisely what this row would then be asserting falsely. |
|
||||
| `scripts/ci-toolchain-image-resolves.sh` — registry manifest read for the pinned toolchain tag | `UNSAFE-KNOWN` | Reads a MUTABLE identifier (a registry tag) with nothing re-checking it before the `container:` jobs pull, so a tag deleted between the preflight and the pull is reported as present. Graded `UNSAFE-KNOWN` rather than `N/A` deliberately: nothing proceeds on the strength of the read — it can only turn its own job red, which is not nothing (the merge-consent hook denies on the COMBINED status, ersatztv#598) but is not authorization either — while a stale PASS is read by a human as "the image is fine", which is an assertion about remote state this file exists to grade. The residual is bounded by what it degrades to: a stale pass leaves exactly the pre-#772 behaviour (five jobs failing at pull), never anything that proceeds on the strength of the read. The opposite error is closed by the EXIT CODE rather than by wording: an unverifiable answer (after retries), an HTTP 200 whose body is not a manifest, and all three shapes of refused anonymous read — a `401`/`403` surviving a bearer the run actually obtained, a `401` whose token leg produced no bearer, and a first-read `403` that never entered the token leg at all — every one of them FAILS the job. The script names no credential (ersatztv#885, `ci.pr-route-carries-no-stored-credential`): it runs on the `pull_request` route, where the YAML is head-supplied, so it reads this public package through the registry's anonymous Bearer-token flow instead. That changes nothing about the grade — the tag it reads is mutable either way. THE TRAP, since warning on those and exiting 0 is the natural way to write this check: a missing `curl`, a moved registry or a DNS change all land there, and a green-with-a-warning job is indistinguishable from a healthy pin forever after — "the check could not run" presenting as "the pin is fine", which is precisely what this row would then be asserting falsely. |
|
||||
| `scripts/set-provider.sh` | `N/A` | Reads no live remote state — sets local `dotnet user-secrets` values. |
|
||||
| `scripts/__init__.py` | `N/A` | Empty package marker — executes nothing. |
|
||||
| `scripts/scripted-schedules/entrypoint.py` — `ScriptedScheduleApi.get_context(build_id)`, then `define_content` / `reset_playout` / `build_playout` against the same live server | `UNSAFE-KNOWN` | A genuine read-then-act over live ErsatzTV state. The context is fetched, handed to user-supplied script functions that mutate the playout, and re-fetched after a reset with nothing pinning either read — a concurrent build or edit between them is invisible. Accepted because it runs inside a single scripted-schedule build the server itself serialises per playout, and because the API exposes no version or ETag on the context to compare against; the honest bound is that the blast radius is one playout's content, reversible by rebuilding. |
|
||||
|
||||
@@ -63,16 +63,21 @@ if [ "$merge_tree" != "$pr_tree" ]; then
|
||||
fi
|
||||
|
||||
# Merge tree matches the PR head tree exactly. Confirm that PR head was actually validated green
|
||||
# before trusting it -- query the Gitea combined commit status API. Auth is required (private
|
||||
# instance); a missing/failing/non-success response always falls through to skip=false.
|
||||
if [ -z "${ETV_STATUS_AUTH:-}" ]; then
|
||||
echo "ETV_STATUS_AUTH not set; cannot verify PR head status; running full validation (safe default)"
|
||||
emit false
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# before trusting it -- query the Gitea combined commit status API.
|
||||
#
|
||||
# THE READ IS ANONYMOUS (ersatztv#885, `ci.pr-route-carries-no-stored-credential`). The three jobs
|
||||
# that call this run on the `pull_request` route as well as on the main push, and Gitea resolves a
|
||||
# `pull_request` run from the PR HEAD -- so those jobs may hold no stored secret, and the
|
||||
# `ETV_STATUS_AUTH` they used to carry is gone. `timothy/ersatztv` is public and answers this GET
|
||||
# unauthenticated (measured 2026-09-04: HTTP 200 carrying the combined state). No credential
|
||||
# override is kept in its place: the URL below names ONE instance, that instance is public, so an
|
||||
# override would be a code path with no caller -- and an unusable one (":", the shape an absent
|
||||
# secret interpolates to) would draw a 401 and turn a working read into a permanent skip=false.
|
||||
#
|
||||
# Every failure direction here is safe: a missing, failing or non-success response falls through to
|
||||
# skip=false, which re-runs validation. Nothing about this step can cause a skip that was not earned.
|
||||
status_url="http://192.168.1.95:3000/api/v1/repos/timothy/ersatztv/commits/${pr_head}/status"
|
||||
status_json="$(curl -sf -u "$ETV_STATUS_AUTH" "$status_url" || true)"
|
||||
status_json="$(curl -sf "$status_url" || true)"
|
||||
if [ -z "$status_json" ]; then
|
||||
echo "status API request for PR head ${pr_head} failed; running full validation (safe default)"
|
||||
emit false
|
||||
|
||||
@@ -3,7 +3,8 @@
|
||||
#
|
||||
# WHY THIS EXISTS. `docker-build.yml` pins its five `container:` jobs to an immutable
|
||||
# `ersatztv-ci:<sha>`. Between 2026-08-11 and 2026-08-13 that tag was deleted from the Gitea
|
||||
# registry and every one of those jobs — including BOTH required contexts — died after 1-2s with
|
||||
# registry and every one of those jobs — including BOTH required contexts — died at image pull,
|
||||
# before running a step, with
|
||||
#
|
||||
# Error response from daemon: failed to resolve reference "…/ersatztv-ci:<the pinned sha>": not found
|
||||
#
|
||||
@@ -18,9 +19,10 @@
|
||||
# failure into one actionable message, which is all a consumer of someone else's registry can do.
|
||||
#
|
||||
# WHY IT DOES NOT GATE THE CONTAINER JOBS with `needs:`. Serialising five jobs behind a checkout +
|
||||
# one curl would tax every green run to speed up the rare red one, and the container jobs already
|
||||
# fail fast (1-2s) when the pull fails. This runs in PARALLEL: the diagnosis is present the moment
|
||||
# anyone looks, and the happy path pays nothing.
|
||||
# one curl would tax every green run to speed up the rare red one, and a container job that cannot
|
||||
# pull its image fails at the pull, before it runs a step, so it wastes no work waiting to be told.
|
||||
# This runs in PARALLEL: the diagnosis is present the moment anyone looks, and the happy path pays
|
||||
# nothing.
|
||||
#
|
||||
# UNKNOWN IS NOT A PASS. Warning and exiting 0 on every answer that is not 200 or
|
||||
# 404 makes "curl is missing from this runner", "the
|
||||
@@ -29,13 +31,30 @@
|
||||
# RETRIED (they are usually transient) and then FAIL. The message stays distinct from the deleted
|
||||
# case: "could not verify" and "IS GONE" send an operator to different places.
|
||||
#
|
||||
# Env (all optional except the credential; the defaults are the live values):
|
||||
# IT READS THE REGISTRY ANONYMOUSLY, AND THAT IS LOAD-BEARING RATHER THAN INCIDENTAL. This job runs
|
||||
# on the `pull_request` route, where the workflow YAML is supplied by the PR head, so it may hold no
|
||||
# stored secret (ersatztv#885, `ci.pr-route-carries-no-stored-credential`). A naive unauthenticated
|
||||
# GET of a manifest is a 401 for every tag, present or deleted — which is why this used to demand a
|
||||
# credential — but that 401 is a Bearer CHALLENGE, and this registry issues an anonymous pull token
|
||||
# for a public package against it. Measured 2026-09-04: the token endpoint answers 200 with no
|
||||
# credential, that token reads the pinned manifest and its config blob (200), and a tag that does
|
||||
# not exist answers 404 rather than 401 — so the deleted-tag diagnosis this whole script exists for
|
||||
# survives the change. What does NOT survive it is the `ersatztv-ci` PACKAGE ceasing to be
|
||||
# anonymously pullable: the token leg then refuses, and this fails loudly with a message that names
|
||||
# that cause rather than reporting could-not-tell. That is the package and not this repo — the
|
||||
# package is linked to no repository (measured 2026-09-05: every version of it reports
|
||||
# `"repository": null`), so `timothy/ersatztv` going private would not take the pull token away.
|
||||
# What that WOULD take away is the unauthenticated commit-status GET in
|
||||
# scripts/ci-detect-already-validated.sh, which is a different dependency with the opposite failure
|
||||
# direction: it falls through to `skip=false` and stays green. Do not cite this script's loudness
|
||||
# for that one.
|
||||
#
|
||||
# Env (all optional; the defaults are the live values):
|
||||
# ETV_CI_REGISTRY registry host:port (default 192.168.1.95:3000)
|
||||
# ETV_CI_IMAGE_REPO package path inside the registry (default timothy/ersatztv-ci)
|
||||
# ETV_CI_WORKFLOW workflow file to read the pin from (default .gitea/workflows/docker-build.yml)
|
||||
# ETV_CI_ATTEMPTS tries per pin before an unknown becomes a failure (default 3)
|
||||
# ETV_CI_RETRY_SECONDS pause between those tries (default 5)
|
||||
# ETV_REGISTRY_AUTH user:pass — REQUIRED; the registry rejects anonymous reads with 401
|
||||
set -euo pipefail
|
||||
|
||||
registry="${ETV_CI_REGISTRY:-192.168.1.95:3000}"
|
||||
@@ -52,60 +71,157 @@ fail() { printf '::error::ci-toolchain-image-resolves: %s\n' "$*" >&2; exit 1; }
|
||||
pins=$(grep -oE 'ersatztv-ci:[0-9a-f]+' "$workflow" | cut -d: -f2 | sort -u || true)
|
||||
[ -n "$pins" ] || fail "no ersatztv-ci pin found in $workflow — if the grep pattern stopped matching, fix it here and in pr-checks.yml::ci-image-pin together"
|
||||
|
||||
# No credentials is NOT a pass. An unauthenticated read of this registry is a 401 for every tag,
|
||||
# present or deleted, so a run without them would report "cannot tell" for a live pin and for a
|
||||
# deleted one alike — the shape where a guard reports green having checked nothing.
|
||||
#
|
||||
# The EMPTY-halves check is the one that matters in CI and is easy to miss: an absent secret does
|
||||
# not arrive here as an unset variable. `ETV_REGISTRY_AUTH: ${{ secrets.REGISTRY_USER }}:${{ ... }}`
|
||||
# interpolates a missing secret to the empty string, so the job passes the non-empty string ":".
|
||||
# Testing only the unset case would leave the production shape uncovered.
|
||||
auth="${ETV_REGISTRY_AUTH:-}"
|
||||
[ -n "$auth" ] || fail "ETV_REGISTRY_AUTH (user:pass) is unset, so the registry cannot be queried — this check refuses to report a pass it did not establish"
|
||||
case "$auth" in
|
||||
*:*) ;;
|
||||
*) fail "ETV_REGISTRY_AUTH must be user:pass, got a value with no ':' — the registry cannot be queried and this check refuses to report a pass it did not establish" ;;
|
||||
esac
|
||||
[ -n "${auth%%:*}" ] && [ -n "${auth#*:}" ] \
|
||||
|| fail "ETV_REGISTRY_AUTH has an empty half (user or password) — this is what an ABSENT REGISTRY_USER/REGISTRY_PASSWORD secret interpolates to, not a credential. Fix the secrets rather than reading an unauthenticated 401 as could-not-tell."
|
||||
|
||||
accept='application/vnd.oci.image.manifest.v1+json,application/vnd.docker.distribution.manifest.v2+json,application/vnd.oci.image.index.v1+json,application/vnd.docker.distribution.manifest.list.v2+json'
|
||||
attempts="${ETV_CI_ATTEMPTS:-3}"
|
||||
retry_seconds="${ETV_CI_RETRY_SECONDS:-5}"
|
||||
rc=0
|
||||
|
||||
# One GET, returning "<code> <is-a-manifest>". The body is fetched rather than a HEAD sent, because
|
||||
# The anonymous pull token, acquired once and reused for every pin. Empty until the registry
|
||||
# challenges. `token_leg_done` separates "not tried yet" from "tried", which is what lets the
|
||||
# refusal arms below report what the run actually DID rather than what it might have done.
|
||||
#
|
||||
# `token_leg_retryable` separates the two ways the leg can come back empty, and they are not the
|
||||
# same event. A token endpoint that ANSWERED and named no token — or a challenge naming no realm, or
|
||||
# no challenge at all — has told us something, and asking it again cannot change the answer: that is
|
||||
# the registry genuinely refusing anonymous reads, and it is asked once per RUN. A token endpoint
|
||||
# that could not be REACHED, or that answered 5xx, told us nothing; it is the same transport blip the
|
||||
# manifest read absorbs, so it is retried on the same `ETV_CI_ATTEMPTS` budget. Without that split
|
||||
# the two legs of one read had opposite flake tolerances — measured 2026-09-05 on the predecessor of
|
||||
# this commit with `ETV_CI_ATTEMPTS=3`, an unreachable token endpoint failed after 1 token call and
|
||||
# 1 manifest call while a flaky manifest read got 3 retries — and this job's red denies a merge (the
|
||||
# consent hook reads the COMBINED status, ersatztv#598), so a one-second token-endpoint outage
|
||||
# blocked a merge until someone re-ran it.
|
||||
token=""
|
||||
token_leg_done=0
|
||||
token_leg_retryable=0
|
||||
token_leg_attempts=0
|
||||
headers=$(mktemp)
|
||||
trap 'rm -f "$headers"' EXIT
|
||||
|
||||
# One directive out of a `Www-Authenticate: Bearer realm="…",service="…"` challenge. The realm is
|
||||
# read from the challenge rather than assumed, so a registry that moves its token endpoint is
|
||||
# followed instead of guessed at; `service` is optional (this registry issues a token without it,
|
||||
# measured 2026-09-04) and is passed through when the challenge names one.
|
||||
#
|
||||
# The directive NAME is matched in any case, through a character class generated from the key, and
|
||||
# the header name likewise (below): RFC 7235 §2.1 makes auth-param names case-insensitive and RFC
|
||||
# 9110 §5.1 makes field names case-insensitive, so a registry answering `WWW-Authenticate: Bearer
|
||||
# Realm="…"` is spelling this legally. Matching one spelling would send that challenge down the
|
||||
# "named no realm" arm and `fail` the job with the wrong diagnosis — loud, but pointing an operator
|
||||
# at a token endpoint that is fine. The VALUE keeps its case: a realm URL is case-sensitive.
|
||||
challenge_field() {
|
||||
local key="$2" pattern="" index char
|
||||
for ((index = 0; index < ${#key}; index++)); do
|
||||
char=${key:index:1}
|
||||
pattern="${pattern}[${char}$(printf '%s' "$char" | tr '[:lower:]' '[:upper:]')]"
|
||||
done
|
||||
printf '%s' "$1" | sed -n "s/.*[ ,]$pattern=\"\([^\"]*\)\".*/\1/p" | head -1
|
||||
}
|
||||
|
||||
# The token leg. Returns non-zero on every shape that leaves us without a bearer — no challenge, no
|
||||
# realm in it, a token endpoint that will not answer, or an answer carrying no token. Each of those
|
||||
# is "could not establish anonymous access", which the caller turns into a REFUSAL rather than a
|
||||
# could-not-tell: an empty token would otherwise fall through to a second 401 and read as an
|
||||
# ordinary auth failure with no cause named. It also SORTS them, into `token_leg_retryable`: only
|
||||
# the shapes where the endpoint said nothing at all are worth asking again.
|
||||
acquire_token() {
|
||||
local challenge realm service url resp body http
|
||||
token_leg_done=1
|
||||
token_leg_retryable=0
|
||||
token_leg_attempts=$((token_leg_attempts + 1))
|
||||
challenge=$(tr -d '\r' < "$headers" |
|
||||
awk -F: 'tolower($1) == "www-authenticate" { sub(/^[^:]*:[[:space:]]*/, ""); print; exit }')
|
||||
[ -n "$challenge" ] || return 1
|
||||
realm=$(challenge_field "$challenge" realm)
|
||||
[ -n "$realm" ] || return 1
|
||||
service=$(challenge_field "$challenge" service)
|
||||
url="$realm?scope=repository:$image_repo:pull"
|
||||
# Spelled as a full `if` rather than `[ … ] && …`: as a bare statement the latter returns the
|
||||
# test's exit status, which is 1 whenever `service` is absent — a legal challenge shape.
|
||||
if [ -n "$service" ]; then url="$url&service=$service"; fi
|
||||
# The status code is read for the same reason the manifest read reads it: it is the only thing
|
||||
# that separates "this endpoint REFUSED" from "this endpoint was not there", and only the second
|
||||
# is worth a second ask. A `curl` that fails outright (DNS, connect, reset) leaves no response at
|
||||
# all and lands in the same class.
|
||||
resp=$(curl -s -w '\n%{http_code}' "$url") || resp=""
|
||||
if [ -z "$resp" ]; then
|
||||
token_leg_retryable=1
|
||||
return 1
|
||||
fi
|
||||
http=${resp##*$'\n'}
|
||||
body=${resp%$'\n'*}
|
||||
case "$http" in
|
||||
000|5??) token_leg_retryable=1; return 1 ;;
|
||||
esac
|
||||
token=$(printf '%s' "$body" | sed -n 's/.*"token"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' | head -1)
|
||||
[ -n "$token" ] || return 1
|
||||
return 0
|
||||
}
|
||||
|
||||
# One GET, recording the HTTP code and whether the body is a manifest. The body is fetched rather
|
||||
# than a HEAD sent, because
|
||||
# HTTP 200 alone does not mean "the manifest is there": a proxy, a captive login page or an error
|
||||
# document all answer 200 with a body that is not a manifest, and a check that reads only the status
|
||||
# line reports those as "resolves". A manifest always carries `schemaVersion`, so the body is matched
|
||||
# for it — with a shell `case`, so nothing depends on jq being installed and no pipeline can invert
|
||||
# the result on a large body.
|
||||
probe() {
|
||||
local url="$1" resp code body
|
||||
# `-u` puts the credential in argv, visible to `ps` for the length of the call — and this job has
|
||||
# no `container:`, so that is the shared host. Kept because it is the shape every other curl caller
|
||||
# in scripts/ already uses (`ci-detect-already-validated.sh`, `pr-changed-files.sh`,
|
||||
# `select-queue.sh`, `issue-qualification-audit.sh`): fixing one site would leave the class intact
|
||||
# and the codebase inconsistent. The class is tracked in ersatztv#821.
|
||||
resp=$(curl -s -w '\n%{http_code}' -u "$auth" -H "Accept: $accept" "$url") || resp=""
|
||||
[ -n "$resp" ] || { printf '000 no\n'; return 0; }
|
||||
#
|
||||
# It ASSIGNS `code`/`is_manifest` rather than printing them, and so does `probe` — because the token
|
||||
# and the "already tried the token leg" flag must survive from one pin to the next, and a
|
||||
# `$(probe …)` command substitution runs in a SUBSHELL whose assignments are discarded. Reading the
|
||||
# answer through a subshell would silently re-run the whole two-leg exchange for every pin.
|
||||
request() {
|
||||
local url="$1" resp body
|
||||
if [ -n "$token" ]; then
|
||||
resp=$(curl -s -w '\n%{http_code}' -D "$headers" -H "Authorization: Bearer $token" -H "Accept: $accept" "$url") || resp=""
|
||||
else
|
||||
resp=$(curl -s -w '\n%{http_code}' -D "$headers" -H "Accept: $accept" "$url") || resp=""
|
||||
fi
|
||||
if [ -z "$resp" ]; then
|
||||
code="000"
|
||||
is_manifest="no"
|
||||
return 0
|
||||
fi
|
||||
code=${resp##*$'\n'}
|
||||
body=${resp%$'\n'*}
|
||||
case "$body" in
|
||||
*'"schemaVersion"'*) printf '%s yes\n' "$code" ;;
|
||||
*) printf '%s no\n' "$code" ;;
|
||||
*'"schemaVersion"'*) is_manifest="yes" ;;
|
||||
*) is_manifest="no" ;;
|
||||
esac
|
||||
}
|
||||
|
||||
# The two legs of an anonymous registry read: the plain GET, and — only if it is challenged and the
|
||||
# leg is not already settled — the token exchange followed by ONE retry carrying the bearer. A 401
|
||||
# that survives the retry is left as a 401 and refused by the caller; it is never rounded off to
|
||||
# could-not-tell, because the cause (this package is no longer readable without a credential) sends
|
||||
# an operator somewhere quite different from a flaky registry.
|
||||
#
|
||||
# "Settled" is `token_leg_done` AND NOT `token_leg_retryable`: a leg abandoned because its endpoint
|
||||
# could not be reached settled nothing, so the next attempt asks again.
|
||||
probe() {
|
||||
request "$1"
|
||||
if [ "$code" = "401" ] && { [ "$token_leg_done" -eq 0 ] || [ "$token_leg_retryable" -eq 1 ]; }; then
|
||||
if acquire_token; then
|
||||
request "$1"
|
||||
fi
|
||||
fi
|
||||
}
|
||||
|
||||
code=""
|
||||
is_manifest="no"
|
||||
for pin in $pins; do
|
||||
url="http://$registry/v2/$image_repo/manifests/$pin"
|
||||
attempt=1
|
||||
while : ; do
|
||||
read -r code is_manifest <<EOF
|
||||
$(probe "$url")
|
||||
EOF
|
||||
probe "$url"
|
||||
case "$code" in
|
||||
200|404|401|403) break ;;
|
||||
200|404|403) break ;;
|
||||
401)
|
||||
# A 401 is an answer — unless the only reason it still stands is a token leg whose endpoint
|
||||
# could not be reached, in which case it is an unknown wearing the first read's status code
|
||||
# and belongs in the retry class below with every other unknown.
|
||||
if [ "$token_leg_retryable" -eq 0 ]; then break; fi
|
||||
;;
|
||||
esac
|
||||
# Only the unknown answers are retried: 200/404 are answers, and an auth failure will not cure
|
||||
# itself. A transient registry is the common case for the rest, and absorbing it here is what
|
||||
@@ -134,7 +250,41 @@ EOF
|
||||
401|403)
|
||||
# `fail` rather than `rc=1`: unlike a 404, this says nothing about the pin, and it will say
|
||||
# the same thing about every remaining one. Abandoning the loop keeps the log to one cause.
|
||||
fail "the registry rejected these credentials (HTTP $code) for $registry/$image_repo:$pin, so the pin could not be checked. Fix REGISTRY_USER/REGISTRY_PASSWORD rather than reading this as a pass."
|
||||
#
|
||||
# THREE ways to get here, and they send an operator to three different places, so they are
|
||||
# worded apart — the same reason `404` and `could NOT VERIFY` are. Each message states only
|
||||
# what actually ran, because a message naming a step that did not happen is evidence for a
|
||||
# diagnosis nobody performed:
|
||||
#
|
||||
# * a refusal that survived a GOOD token is an answer about this PACKAGE, and since
|
||||
# ersatztv#885 it is not a preflight-only problem: every `container:` job pulls the same
|
||||
# image with no credential, so they fail at image pull too, including both required
|
||||
# contexts;
|
||||
# * a challenge that yielded no usable token is an infrastructure answer about the TOKEN
|
||||
# ENDPOINT;
|
||||
# * a FIRST-READ 403 never reached the token leg at all — `probe` enters it on a 401 only —
|
||||
# so nothing was ever asked for. This is an answer about ACCESS to the registry. Its
|
||||
# message says NO TOKEN WAS EVER REQUESTED rather than "the registry issued no challenge":
|
||||
# a 403 MAY carry a `Www-Authenticate` and this script would still not follow it, so only
|
||||
# the first is something the run observed. Note the boundary too: a 401 carrying no
|
||||
# challenge DOES enter the token leg and abandon it, so it lands in the arm above, not
|
||||
# this one. The branch order below is `token` first precisely so this case cannot borrow
|
||||
# either of the other two mechanisms.
|
||||
#
|
||||
# A token leg that ANSWERED is not retried, deliberately: `token_leg_done` is set before the
|
||||
# attempt and `token_leg_retryable` stays 0, so a registry genuinely refusing anonymous reads
|
||||
# is asked once per RUN rather than once per pin or once per attempt. A token endpoint that
|
||||
# could not be REACHED settled nothing and IS retried, on the same `ETV_CI_ATTEMPTS` budget the
|
||||
# manifest read uses — the two legs of one read must not have opposite flake tolerances when a
|
||||
# red here denies a merge. The message below reports how many token-leg attempts actually ran,
|
||||
# so the two cases are distinguishable in the log rather than only in this comment.
|
||||
if [ -n "$token" ]; then
|
||||
fail "the registry refused an ANONYMOUS read (HTTP $code) of $registry/$image_repo:$pin even after a Bearer token was obtained, so the pin could not be checked. Every container: job pulls this image without a credential too, so they will fail at image pull. Check that the ersatztv-ci package is still PUBLIC (it is linked to no repository, so this is the package's own visibility, not the repo's) — do not read this as a pass."
|
||||
fi
|
||||
if [ "$token_leg_done" -eq 1 ]; then
|
||||
fail "could NOT OBTAIN an anonymous pull token for $registry/$image_repo:$pin after $token_leg_attempts token-leg attempt(s) — the registry answered HTTP $code and the token leg produced none: either there was no Www-Authenticate challenge, or it named no realm, or the token endpoint did not answer with a token. The pin was NOT checked. Look at the registry's token endpoint, not at the pin."
|
||||
fi
|
||||
fail "the registry refused an ANONYMOUS read (HTTP $code) of $registry/$image_repo:$pin and NO TOKEN WAS EVER REQUESTED: the token leg is entered on a 401 only, so this answer was never followed as a Bearer challenge and the pin could not be checked. An outright refusal is an answer about ACCESS to the registry, not about the pin. Every container: job pulls this image without a credential too, so they will fail at image pull. Check that the ersatztv-ci package is still PUBLIC (it is linked to no repository, so this is the package's own visibility, not the repo's), and that nothing (a proxy, an ACL) is answering for the registry — do not read this as a pass."
|
||||
;;
|
||||
*)
|
||||
# NOT gone, and NOT a pass either. Deliberately worded apart from the 404 message: this sends
|
||||
|
||||
@@ -679,4 +679,30 @@ CLAIMS: tuple[Claim, ...] = (
|
||||
"The mutation drops `record_wing_faults()` from the call and keeps `yaml_faults`, which is "
|
||||
"the deletion the sentence describes rather than a caricature of it.",
|
||||
),
|
||||
Claim(
|
||||
site="docs/guard-inventory.md",
|
||||
quote=(
|
||||
"deleting `build`'s `if: github.event_name != 'pull_request'` from the shipped "
|
||||
"`docker-build.yml` reddens `test_no_PULL_REQUEST_route_job_names_a_STORED_secret`"
|
||||
),
|
||||
target=".gitea/workflows/docker-build.yml",
|
||||
clause=" if: github.event_name != 'pull_request'",
|
||||
replacement=" # if: clause removed by the mutation harness",
|
||||
proof="test_workflow_persist_credentials.py::test_no_PULL_REQUEST_route_job_names_a_STORED_secret",
|
||||
outcome=RED,
|
||||
expect="job `build` names stored secret(s) on the pull_request route",
|
||||
why="`MUTATIONS` is keyed one entry per GUARD FILE, and "
|
||||
"`scripts/tests/test_workflow_persist_credentials.py` holds TWO invariants since #885 — the "
|
||||
"`actions/checkout` one its declared entry stands for, and the PR-route stored-secret one. "
|
||||
"The second would otherwise have no executed mutation at all, so the inventory row states it "
|
||||
"in prose and the prose is bound here, which is the population #881 widened this file to "
|
||||
"cover. The clause is the ONE thing keeping `build` — the last job naming `REGISTRY_PASSWORD` "
|
||||
"— off the head-authored route, so the mutation is the defect the invariant exists for and "
|
||||
"not a caricature: with it deleted the collector must name that job and that secret. The "
|
||||
"mutation is driven into the SHIPPED workflow rather than a fixture for the same reason the "
|
||||
"module's own two mutation tests are: a synthetic document would only prove the collector "
|
||||
"works on YAML this repo does not run. The declared diagnostic is the fault's own wording "
|
||||
"rather than the assertion's, because a red anywhere else in that test — a parse error, an "
|
||||
"empty population — prints a different message and is evidence about nothing.",
|
||||
),
|
||||
)
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
"""Tests for `scripts/ci-toolchain-image-resolves.sh` (ersatztv#772).
|
||||
"""Tests for `scripts/ci-toolchain-image-resolves.sh` (ersatztv#772, ersatztv#885).
|
||||
|
||||
The script answers one question — does the tag `docker-build.yml` pins still exist? — and the whole
|
||||
value is in *which answers it refuses to round off*. A registry read has three outcomes, not two:
|
||||
@@ -10,6 +10,13 @@ becomes decoration, so each is driven here through the real entry point with a s
|
||||
answer fails the job too, disarming the `404` arm still exits non-zero, so the EXIT CODE separates
|
||||
nothing. What the disarm destroys is the DIAGNOSTIC — the outage is reported as "could not verify",
|
||||
which sends an operator to the registry's health instead of to the rebuild that fixes it.
|
||||
|
||||
THE READ IS ANONYMOUS since ersatztv#885: this job runs on the `pull_request` route, where the YAML
|
||||
is head-supplied, so it may hold no stored secret. The stub therefore reproduces the registry's
|
||||
TWO-LEG shape rather than a credential check — an unauthenticated read is answered `401` carrying a
|
||||
Bearer CHALLENGE, and only a request bearing the token that challenge leads to is served. That is
|
||||
what keeps the file from certifying a script that has quietly stopped doing the token exchange:
|
||||
without it every request would be answered `401` and every assertion below would go red.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -24,38 +31,106 @@ import pytest
|
||||
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
SCRIPT = REPO_ROOT / "scripts" / "ci-toolchain-image-resolves.sh"
|
||||
|
||||
# Stands in for `curl -s -w '\n%{http_code}' -u <auth> -H Accept <url>`: prints a scripted body,
|
||||
# a newline and the HTTP code, and logs the call. It VALIDATES `-u` rather than ignoring it — a stub
|
||||
# that answers 200 whether or not the script authenticates would stay green if the real `-u` were
|
||||
# deleted, which is the fidelity gap that lets a test double certify a script the live registry
|
||||
# would reject on every request.
|
||||
# Stands in for the two curl shapes the script issues: the registry GET
|
||||
# (`-s -w '\n%{http_code}' -D <headers> [-H Authorization] -H Accept <url>`) and the token GET
|
||||
# (`-s <realm>?scope=…`). It prints a scripted body, a newline and the HTTP code, dumps response
|
||||
# headers where `-D` asks for them, and logs every call with the Authorization header it carried.
|
||||
#
|
||||
# It ENFORCES the protocol rather than ignoring it: an unauthenticated registry read is 401 + a
|
||||
# challenge, exactly as the live registry answers (measured 2026-09-04), so a script that stopped
|
||||
# performing the token leg would fail every test in this file rather than sailing through on a stub
|
||||
# that answers 200 regardless.
|
||||
CURL_SHIM = r"""#!/usr/bin/env python3
|
||||
import os, pathlib, sys
|
||||
|
||||
state = pathlib.Path(os.environ["STUB_DIR"])
|
||||
args = sys.argv[1:]
|
||||
url = [a for a in args if a.startswith("http")][-1]
|
||||
tag = url.rsplit("/", 1)[-1]
|
||||
auth = args[args.index("-u") + 1] if "-u" in args else ""
|
||||
with (state / "calls").open("a") as fh:
|
||||
fh.write(f"{url} auth={auth}\n")
|
||||
|
||||
# The live registry answers 401 to an anonymous read of ANY tag, present or deleted.
|
||||
user, _, password = auth.partition(":")
|
||||
if not user or not password:
|
||||
|
||||
def flag(name):
|
||||
return args[args.index(name) + 1] if name in args and args.index(name) + 1 < len(args) else None
|
||||
|
||||
|
||||
def header(name):
|
||||
prefix = name.lower() + ":"
|
||||
for i, a in enumerate(args):
|
||||
if a == "-H" and i + 1 < len(args) and args[i + 1].lower().startswith(prefix):
|
||||
return args[i + 1].split(":", 1)[1].strip()
|
||||
return ""
|
||||
|
||||
|
||||
authorization = header("Authorization")
|
||||
dump = flag("-D")
|
||||
codes = dict(pair.split("=", 1) for pair in (state / "codes").read_text().split() if pair)
|
||||
|
||||
with (state / "calls").open("a") as fh:
|
||||
fh.write("%s auth=%s\n" % (url, authorization))
|
||||
|
||||
# --- the token endpoint --------------------------------------------------------------------
|
||||
if "/token" in url:
|
||||
behaviour = codes.get("TOKEN", "issue")
|
||||
if behaviour == "blip":
|
||||
# Unreachable on the FIRST ask and healthy afterwards -- a token-endpoint outage shorter
|
||||
# than the retry budget, which is the shape a retry exists to absorb.
|
||||
marker = state / "token-blip"
|
||||
behaviour = "issue" if marker.exists() else "unreachable"
|
||||
marker.write_text("1")
|
||||
if behaviour == "unreachable":
|
||||
sys.exit(7)
|
||||
if behaviour == "server-error":
|
||||
# It ANSWERS, but with nothing a client can use -- the other half of "told us nothing".
|
||||
print("<html>502 Bad Gateway</html>\n502", end="")
|
||||
sys.exit(0)
|
||||
if behaviour == "empty":
|
||||
print('{"expires_in": 300}\n200', end="")
|
||||
else:
|
||||
print('{"token": "anon-token"}\n200', end="")
|
||||
sys.exit(0)
|
||||
|
||||
# --- the registry --------------------------------------------------------------------------
|
||||
tag = url.rsplit("/", 1)[-1]
|
||||
code = codes.get(tag, codes.get("*", "200"))
|
||||
|
||||
if code == "TRANSPORT":
|
||||
sys.exit(7)
|
||||
|
||||
if authorization != "Bearer anon-token" and codes.get("CHALLENGE") == "none":
|
||||
# A registry that REFUSES an unauthenticated read outright instead of challenging: no
|
||||
# Www-Authenticate header at all, so a client has nowhere to ask for a token. `REFUSAL` picks
|
||||
# the code, because a 403 and a challenge-less 401 take DIFFERENT paths through the script --
|
||||
# `probe` enters the token leg on 401 only.
|
||||
refusal = codes.get("REFUSAL", "403")
|
||||
if dump:
|
||||
pathlib.Path(dump).write_text("HTTP/1.1 %s Refused\r\n" % refusal)
|
||||
print("{}\n%s" % refusal, end="")
|
||||
sys.exit(0)
|
||||
|
||||
if authorization != "Bearer anon-token":
|
||||
# 401 for a live tag and a deleted one alike -- the challenge is the ONLY thing that tells a
|
||||
# client where a token can be had.
|
||||
if dump:
|
||||
realm = codes.get("REALM", "http://registry.test/v2/token")
|
||||
# A legal challenge in a case no part of the script spells: field names and auth-param
|
||||
# names are both case-insensitive, so this is the same header, not a malformed one.
|
||||
line = 'Www-Authenticate: Bearer realm="%s",service="container_registry",scope="*"\r\n'
|
||||
if codes.get("CHALLENGE") == "upper":
|
||||
line = 'WWW-AUTHENTICATE: Bearer REALM="%s",SERVICE="container_registry",SCOPE="*"\r\n'
|
||||
pathlib.Path(dump).write_text("HTTP/1.1 401 Unauthorized\r\n" + line % realm)
|
||||
print("{}\n401", end="")
|
||||
sys.exit(0)
|
||||
|
||||
if dump:
|
||||
pathlib.Path(dump).write_text("HTTP/1.1 %s\r\n" % code)
|
||||
|
||||
if code == "401-AFTER-TOKEN":
|
||||
print("{}\n401", end="")
|
||||
sys.exit(0)
|
||||
|
||||
codes = dict(pair.split("=", 1) for pair in (state / "codes").read_text().split() if pair)
|
||||
code = codes.get(tag, codes.get("*", "200"))
|
||||
if code == "TRANSPORT":
|
||||
# Only the EXIT STATUS is observable: the script's `|| resp=""` discards whatever curl printed,
|
||||
# so what this reproduces is the non-zero exit, not the `\n000` real curl also emits.
|
||||
sys.exit(7)
|
||||
body = '{"schemaVersion": 2, "mediaType": "application/vnd.oci.image.manifest.v1+json"}'
|
||||
if code == "200-NOT-A-MANIFEST":
|
||||
code, body = "200", "<html><title>Sign in</title></html>"
|
||||
print(f"{body}\n{code}", end="")
|
||||
print("%s\n%s" % (body, code), end="")
|
||||
"""
|
||||
|
||||
WORKFLOW_TEMPLATE = """jobs:
|
||||
@@ -84,7 +159,7 @@ def preflight(tmp_path):
|
||||
env["PATH"] = f"{bindir}{os.pathsep}{env['PATH']}"
|
||||
env["STUB_DIR"] = str(state)
|
||||
env["ETV_CI_WORKFLOW"] = str(workflow)
|
||||
env["ETV_REGISTRY_AUTH"] = "stub-user:stub-pass"
|
||||
env.pop("ETV_REGISTRY_AUTH", None)
|
||||
# The retry PAUSE is what makes failing on an unknown affordable in CI and unaffordable in a
|
||||
# test suite; the retry COUNT is behaviour, so it is kept and only the wait is removed.
|
||||
env["ETV_CI_ATTEMPTS"] = "2"
|
||||
@@ -107,6 +182,22 @@ def preflight(tmp_path):
|
||||
log = state / "calls"
|
||||
return log.read_text().splitlines() if log.exists() else []
|
||||
|
||||
def token_calls(self):
|
||||
return [c for c in self.calls() if "/token" in c]
|
||||
|
||||
def manifest_calls(self):
|
||||
return [c for c in self.calls() if "/manifests/" in c]
|
||||
|
||||
def authenticated_manifest_calls(self):
|
||||
"""The manifest reads that actually carried the bearer.
|
||||
|
||||
The ATTEMPT count is this, not `manifest_calls`: the run opens with exactly one
|
||||
unauthenticated read, which is the challenge that starts the token leg, and every read
|
||||
after the token is acquired carries it. Counting the raw calls would report one more
|
||||
attempt than the retry loop made.
|
||||
"""
|
||||
return [c for c in self.manifest_calls() if c.endswith("auth=Bearer anon-token")]
|
||||
|
||||
def run(self, script: Path | None = None):
|
||||
return subprocess.run(
|
||||
["bash", str(script or SCRIPT)],
|
||||
@@ -124,7 +215,7 @@ def test_a_pin_that_resolves_passes(preflight):
|
||||
result = preflight.run()
|
||||
assert result.returncode == 0, result.stderr
|
||||
assert "32747a0 resolves" in result.stdout
|
||||
assert preflight.calls(), "the registry was never queried, so nothing was established"
|
||||
assert preflight.authenticated_manifest_calls(), "the registry was never read with a token"
|
||||
|
||||
|
||||
def test_MUTATION_a_deleted_tag_is_reported_as_a_failure(preflight):
|
||||
@@ -170,14 +261,14 @@ def test_an_unknown_is_RETRIED_before_it_fails(preflight):
|
||||
preflight.env["ETV_CI_ATTEMPTS"] = "3"
|
||||
preflight.set_codes({"32747a0": "503"})
|
||||
assert preflight.run().returncode != 0
|
||||
assert len(preflight.calls()) == 3, f"expected 3 attempts, got {preflight.calls()}"
|
||||
assert len(preflight.authenticated_manifest_calls()) == 3, f"expected 3 attempts, got {preflight.calls()}"
|
||||
|
||||
|
||||
def test_an_ANSWER_is_not_retried(preflight):
|
||||
"""404 and 200 are answers; retrying them would only slow the job down."""
|
||||
preflight.set_codes({"32747a0": "404"})
|
||||
assert preflight.run().returncode != 0
|
||||
assert len(preflight.calls()) == 1, f"a 404 must not be retried, got {preflight.calls()}"
|
||||
assert len(preflight.authenticated_manifest_calls()) == 1, f"a 404 must not be retried, got {preflight.calls()}"
|
||||
|
||||
|
||||
def test_HTTP_200_with_a_body_that_is_not_a_manifest_is_not_a_pass(preflight):
|
||||
@@ -189,59 +280,212 @@ def test_HTTP_200_with_a_body_that_is_not_a_manifest_is_not_a_pass(preflight):
|
||||
assert "IS GONE" not in result.stderr
|
||||
|
||||
|
||||
@pytest.mark.parametrize("code", ["401", "403"])
|
||||
def test_rejected_credentials_refuse_rather_than_pass(preflight, code):
|
||||
@pytest.mark.parametrize("code", ["401-AFTER-TOKEN", "403"])
|
||||
def test_a_401_or_403_AFTER_the_token_leg_REFUSES_rather_than_passing(preflight, code):
|
||||
"""The failure mode that would otherwise make this job green forever.
|
||||
|
||||
An anonymous read of this registry is 401 for a live tag and a deleted one alike, so treating
|
||||
an auth failure as "could not tell, carry on" would turn a broken secret into a permanent,
|
||||
silent pass.
|
||||
Reached only once the token leg has run, so it means one specific thing: this registry will not
|
||||
serve an anonymous pull of this package — normally because the repo or the package has been made
|
||||
PRIVATE. Treating that as "could not tell, carry on" would turn it into a permanent silent pass,
|
||||
and it is not a preflight-only problem: since ersatztv#885 every `container:` job pulls the same
|
||||
image with no credential, so they fail at image pull too.
|
||||
"""
|
||||
preflight.set_codes({"32747a0": code})
|
||||
result = preflight.run()
|
||||
assert result.returncode != 0
|
||||
assert "rejected these credentials" in result.stderr
|
||||
assert "refused an ANONYMOUS read" in result.stderr
|
||||
assert "even after a Bearer token was obtained" in result.stderr, (
|
||||
"this arm is the one where a token really was obtained, so it is the only one allowed to "
|
||||
f"say so. stderr={result.stderr}"
|
||||
)
|
||||
assert "PUBLIC" in result.stderr, "the message must name the cause an operator can act on"
|
||||
assert preflight.authenticated_manifest_calls(), (
|
||||
"a message claiming the read survived a Bearer token must be reached with a read that "
|
||||
"CARRIED one — `token_calls` only shows the token was asked for"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("value", "shape"),
|
||||
[
|
||||
(None, "unset"),
|
||||
(":", "both secrets absent — WHAT THE WORKFLOW ACTUALLY PASSES"),
|
||||
("user:", "password secret absent"),
|
||||
(":pass", "user secret absent"),
|
||||
("no-colon", "malformed"),
|
||||
],
|
||||
)
|
||||
def test_unusable_credentials_refuse_BEFORE_querying_anything(preflight, value, shape):
|
||||
"""The empty-halves cases are the ones that happen, and testing only `unset` misses them.
|
||||
@pytest.mark.parametrize("refusal", ["403", "401"])
|
||||
def test_a_refusal_with_NO_CHALLENGE_never_claims_a_token_was_obtained(preflight, refusal):
|
||||
"""The message must not name a mechanism the run did not perform.
|
||||
|
||||
`ETV_REGISTRY_AUTH: ${{ secrets.REGISTRY_USER }}:${{ secrets.REGISTRY_PASSWORD }}` interpolates
|
||||
a missing secret to the empty string, so a job with no secrets configured passes the non-empty
|
||||
string ":" — which is a perfectly good non-empty value and a useless credential. The registry
|
||||
answers 401 to it for a live tag and a deleted one alike.
|
||||
`probe` enters the token leg on a `401` only, so a registry answering `403` on the first read —
|
||||
or a `401` carrying no `Www-Authenticate` — leaves the script with no token having asked for
|
||||
nothing. Measured 2026-09-05 on the predecessor of this commit, the `403` shape reported "even
|
||||
after a Bearer token was obtained", which sends an operator to package visibility on evidence
|
||||
that does not exist (`dont-narrate-mechanisms-you-didnt-measure`). The two codes are BOTH driven
|
||||
because they take different paths: the challenge-less `401` still enters and abandons the token
|
||||
leg, the `403` never enters it.
|
||||
|
||||
The shim answering 401 + a challenge to every unauthenticated read is why the pre-existing
|
||||
`403` case could not reach this — it could only ever be observed AFTER the token leg — so the
|
||||
shim grew a challenge-less behaviour rather than the assertion being written against the old one.
|
||||
"""
|
||||
if value is None:
|
||||
del preflight.env["ETV_REGISTRY_AUTH"]
|
||||
else:
|
||||
preflight.env["ETV_REGISTRY_AUTH"] = value
|
||||
preflight.set_codes({"*": "200", "CHALLENGE": "none", "REFUSAL": refusal})
|
||||
result = preflight.run()
|
||||
assert result.returncode != 0, f"{shape}: reported success on a credential it cannot use"
|
||||
assert "ETV_REGISTRY_AUTH" in result.stderr
|
||||
assert preflight.calls() == [], "it must not query the registry it cannot authenticate to"
|
||||
assert result.returncode != 0, "an unreadable registry is not a pass"
|
||||
assert "even after a Bearer token was obtained" not in result.stderr, (
|
||||
f"no token was obtained on this path. stderr={result.stderr}"
|
||||
)
|
||||
assert preflight.authenticated_manifest_calls() == [], "no read can have carried a token here"
|
||||
if refusal == "403":
|
||||
assert preflight.token_calls() == [], "a 403 first read must not even ask for a token"
|
||||
assert "NO TOKEN WAS EVER REQUESTED" in result.stderr, result.stderr
|
||||
assert "PUBLIC" in result.stderr, "the message must still name a cause an operator can act on"
|
||||
else:
|
||||
# A challenge-less 401 DOES enter the token leg (and abandons it for want of a realm), so it
|
||||
# is the token-endpoint diagnosis rather than the never-asked one.
|
||||
assert preflight.token_calls() == [], "there was no realm to request a token from"
|
||||
assert "could NOT OBTAIN an anonymous pull token" in result.stderr, result.stderr
|
||||
assert "no Www-Authenticate challenge" in result.stderr, (
|
||||
f"the token-leg message must admit the challenge was missing. stderr={result.stderr}"
|
||||
)
|
||||
|
||||
|
||||
def test_the_credential_actually_REACHES_the_registry(preflight):
|
||||
"""Anti-vacuity for every test above: the stub 401s an unauthenticated read, as the live
|
||||
registry does, so a script that stopped passing `-u` would redden the whole file rather than
|
||||
sailing through on a stub that answers 200 regardless."""
|
||||
def test_the_TOKEN_LEG_actually_runs_and_the_bearer_REACHES_the_registry(preflight):
|
||||
"""Anti-vacuity for every test above, and the shape of the whole anonymous read in one place.
|
||||
|
||||
The stub answers 401 to an unauthenticated read exactly as the live registry does, so a script
|
||||
that stopped exchanging the challenge for a token would redden this entire file rather than
|
||||
passing on a stub that serves anyone. The call sequence is pinned rather than counted: challenge,
|
||||
token, re-read WITH the bearer.
|
||||
"""
|
||||
preflight.set_codes({"*": "200"})
|
||||
assert preflight.run().returncode == 0
|
||||
assert preflight.calls() == [
|
||||
"http://192.168.1.95:3000/v2/timothy/ersatztv-ci/manifests/32747a0 auth=stub-user:stub-pass"
|
||||
"http://192.168.1.95:3000/v2/timothy/ersatztv-ci/manifests/32747a0 auth=",
|
||||
"http://registry.test/v2/token?scope=repository:timothy/ersatztv-ci:pull&service=container_registry auth=",
|
||||
"http://192.168.1.95:3000/v2/timothy/ersatztv-ci/manifests/32747a0 auth=Bearer anon-token",
|
||||
]
|
||||
|
||||
|
||||
def test_the_REALM_is_read_from_the_CHALLENGE_rather_than_assumed(preflight):
|
||||
"""A hardcoded token endpoint would work here and break the day the registry moves it.
|
||||
|
||||
The realm is whatever the `Www-Authenticate` header names, so this points the challenge somewhere
|
||||
else entirely and requires the token request to follow it.
|
||||
"""
|
||||
preflight.set_codes({"*": "200", "REALM": "http://elsewhere.test/auth/v2/token"})
|
||||
assert preflight.run().returncode == 0
|
||||
assert preflight.token_calls() == [
|
||||
"http://elsewhere.test/auth/v2/token?scope=repository:timothy/ersatztv-ci:pull&service=container_registry auth="
|
||||
]
|
||||
|
||||
|
||||
def test_the_CHALLENGE_is_read_in_ANY_case(preflight):
|
||||
"""`WWW-AUTHENTICATE: Bearer REALM="…"` is the same challenge, and must take the same path.
|
||||
|
||||
Field names (RFC 9110 §5.1) and auth-param names (RFC 7235 §2.1) are both case-insensitive, so
|
||||
a registry may answer in this spelling without being broken. The predecessor read the header
|
||||
name in a fixed case for all but four letters and the directive name in lowercase only, which
|
||||
sent a legal challenge down the "named no realm" arm: a `fail` naming a cause that is not the
|
||||
real one, pointing an operator at a healthy token endpoint. Driven end to end rather than on the
|
||||
parser, so the whole anonymous read has to survive the spelling.
|
||||
"""
|
||||
preflight.set_codes({"*": "200", "CHALLENGE": "upper"})
|
||||
result = preflight.run()
|
||||
assert result.returncode == 0, result.stderr
|
||||
assert preflight.token_calls() == [
|
||||
"http://registry.test/v2/token?scope=repository:timothy/ersatztv-ci:pull&service=container_registry auth="
|
||||
], preflight.calls()
|
||||
assert preflight.authenticated_manifest_calls(), preflight.calls()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("behaviour", ["empty", "unreachable"])
|
||||
def test_a_token_endpoint_that_yields_NO_TOKEN_refuses(preflight, behaviour):
|
||||
"""No bearer is not "carry on unauthenticated" — it is could-not-establish, and it must refuse.
|
||||
|
||||
Both shapes leave the script without a token: an answer carrying no `token` field, and an
|
||||
endpoint that will not answer at all. Falling through to a second unauthenticated read would
|
||||
surface as an ordinary 401 with no cause named, which is the diagnosis this arm exists to give.
|
||||
"""
|
||||
preflight.set_codes({"*": "200", "TOKEN": behaviour})
|
||||
result = preflight.run()
|
||||
assert result.returncode != 0, "a run that never obtained a token must not report success"
|
||||
assert "could NOT OBTAIN an anonymous pull token" in result.stderr, (
|
||||
"a failed token leg must be worded apart from a refusal that survived a GOOD token: one "
|
||||
"sends an operator to the registry's token endpoint, the other to the package's visibility. "
|
||||
f"stderr={result.stderr}"
|
||||
)
|
||||
assert "still PUBLIC" not in result.stderr, "nothing here establishes the package went private"
|
||||
assert preflight.authenticated_manifest_calls() == [], "no read can have carried a token"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("behaviour", ["unreachable", "server-error"])
|
||||
def test_a_token_endpoint_that_SAID_NOTHING_is_retried_like_any_other_unknown(preflight, behaviour):
|
||||
"""The two legs of one read must not have opposite flake tolerances.
|
||||
|
||||
A token endpoint that cannot be reached (or answers 5xx) has told the run nothing — it is the
|
||||
same transport blip a flaky manifest read gets three tries to survive. Measured 2026-09-05 on
|
||||
the predecessor of this commit, it got ZERO: `token_leg_done` was set once per RUN, the failed
|
||||
leg left the first read's `401` standing, and `401` was in the retry loop's break list, so the
|
||||
job reddened after one token call. A red here denies a merge (the consent hook reads the
|
||||
COMBINED status, ersatztv#598), so that was a one-second outage blocking a merge until someone
|
||||
re-ran the job.
|
||||
"""
|
||||
preflight.env["ETV_CI_ATTEMPTS"] = "3"
|
||||
preflight.set_codes({"*": "200", "TOKEN": behaviour})
|
||||
result = preflight.run()
|
||||
assert result.returncode != 0, "a run that never obtained a token must not report success"
|
||||
assert len(preflight.token_calls()) == 3, (
|
||||
f"expected the token leg to be retried on the attempt budget, got {preflight.calls()}"
|
||||
)
|
||||
assert "could NOT OBTAIN an anonymous pull token" in result.stderr, result.stderr
|
||||
assert "after 3 token-leg attempt(s)" in result.stderr, (
|
||||
f"the message must report what the run actually did, not what it might have. stderr={result.stderr}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("codes", [{"TOKEN": "empty"}, {"CHALLENGE": "none", "REFUSAL": "401"}])
|
||||
def test_a_token_endpoint_that_ANSWERED_is_asked_ONCE(preflight, codes):
|
||||
"""The other direction, and the property the retry above must not cost.
|
||||
|
||||
An endpoint that answered and named no token — or a `401` carrying no challenge to follow — has
|
||||
said something, and asking again cannot change it. That is a registry genuinely refusing
|
||||
anonymous reads, and it is asked once per RUN.
|
||||
"""
|
||||
preflight.env["ETV_CI_ATTEMPTS"] = "3"
|
||||
preflight.set_codes({"*": "200", **codes})
|
||||
result = preflight.run()
|
||||
assert result.returncode != 0
|
||||
assert len(preflight.token_calls()) <= 1, f"a settled token leg must not be re-asked, got {preflight.calls()}"
|
||||
assert len(preflight.manifest_calls()) == 1, (
|
||||
f"nor may the manifest read be retried behind it, got {preflight.calls()}"
|
||||
)
|
||||
assert "could NOT OBTAIN an anonymous pull token" in result.stderr, result.stderr
|
||||
|
||||
|
||||
def test_a_token_endpoint_BLIP_shorter_than_the_budget_RECOVERS(preflight):
|
||||
"""Retrying is only worth anything if the run can still succeed.
|
||||
|
||||
The endpoint is unreachable on the first ask and healthy on the second, so the whole point is
|
||||
the exit code: the pin resolves, with a bearer that reached the registry.
|
||||
"""
|
||||
preflight.env["ETV_CI_ATTEMPTS"] = "3"
|
||||
preflight.set_codes({"*": "200", "TOKEN": "blip"})
|
||||
result = preflight.run()
|
||||
assert result.returncode == 0, f"stdout={result.stdout}\nstderr={result.stderr}"
|
||||
assert "32747a0 resolves" in result.stdout
|
||||
assert len(preflight.token_calls()) == 2, f"expected one failed ask and one good one, got {preflight.calls()}"
|
||||
assert preflight.authenticated_manifest_calls(), "the recovered token never reached the registry"
|
||||
|
||||
|
||||
def test_the_TOKEN_LEG_is_performed_ONCE_across_every_pin(preflight):
|
||||
"""The token is state that must survive the pin loop.
|
||||
|
||||
`probe` assigns it rather than printing it precisely because a `$(…)` command substitution runs
|
||||
in a subshell whose assignments are discarded — a version that read the answer through one would
|
||||
re-run the challenge and the exchange for every pin, and pass every other test in this file.
|
||||
"""
|
||||
preflight.set_workflow_text(
|
||||
WORKFLOW_TEMPLATE.format(pin="32747a0") + " image: 192.168.1.95:3000/timothy/ersatztv-ci:15d2439\n"
|
||||
)
|
||||
preflight.set_codes({"*": "200"})
|
||||
assert preflight.run().returncode == 0
|
||||
assert len(preflight.token_calls()) == 1, f"the token leg ran {len(preflight.token_calls())} times"
|
||||
assert len(preflight.authenticated_manifest_calls()) == 2
|
||||
|
||||
|
||||
def test_a_workflow_with_no_pin_at_all_is_a_failure(preflight):
|
||||
"""If the grep stops matching, the honest report is 'I found nothing', not 'all clear'."""
|
||||
preflight.set_workflow_text("jobs:\n test:\n runs-on: ubuntu-latest\n")
|
||||
@@ -259,7 +503,7 @@ def test_every_distinct_pin_is_checked_and_one_gone_fails_the_job(preflight):
|
||||
result = preflight.run()
|
||||
assert result.returncode != 0
|
||||
assert "15d2439" in result.stderr
|
||||
assert len(preflight.calls()) == 2, f"both pins must be queried, got {preflight.calls()}"
|
||||
assert len(preflight.authenticated_manifest_calls()) == 2, f"both pins must be queried, got {preflight.calls()}"
|
||||
|
||||
|
||||
def test_the_grep_line_cannot_match_ITSELF(preflight):
|
||||
@@ -296,7 +540,9 @@ def test_the_PRODUCTION_retry_defaults_are_the_ones_that_run(preflight):
|
||||
elapsed = time.monotonic() - started
|
||||
|
||||
assert result.returncode != 0
|
||||
assert len(preflight.calls()) == 3, f"the default attempt count is not 3 — got {len(preflight.calls())} call(s)"
|
||||
assert len(preflight.authenticated_manifest_calls()) == 3, (
|
||||
f"the default attempt count is not 3 — got {len(preflight.authenticated_manifest_calls())} call(s)"
|
||||
)
|
||||
assert elapsed >= 8, (
|
||||
f"two pauses at the default 5s should clear the 8s floor; took {elapsed:.1f}s, so the pause "
|
||||
"has been shortened out from under the 'a blip does not redden a PR' argument"
|
||||
|
||||
@@ -851,11 +851,13 @@ def test_no_OTHER_workflow_writes_the_review_verdict_status():
|
||||
The trigger test above hardens ONE file, and by construction cannot see the more general
|
||||
problem: the gate is forgeable by *any* head-resolved workflow holding credentials that can POST
|
||||
a commit status, not only by the gate's own definition. `docker-build.yml` is exactly that — it
|
||||
triggers on `pull_request` (correctly; it builds the PR's code) and carries `ETV_STATUS_AUTH`.
|
||||
A probe once confirmed those credentials could WRITE statuses, not merely read them
|
||||
(ersatztv#697); a follow-up probe confirmed `REGISTRY_PASSWORD` is now scoped off
|
||||
`write:repository` and the same POST returns 403 (see `ci.actions-credential-scoping`). That
|
||||
closes the registry-credential route, not the general problem — `GITEA_TOKEN`, `RENOVATE_TOKEN`,
|
||||
triggers on `pull_request` (correctly; it builds the PR's code) and used to carry
|
||||
`ETV_STATUS_AUTH`. A probe once confirmed those credentials could WRITE statuses, not merely read
|
||||
them (ersatztv#697); a follow-up probe confirmed `REGISTRY_PASSWORD` is now scoped off
|
||||
`write:repository` and the same POST returns 403 (see `ci.actions-credential-scoping`); and since
|
||||
ersatztv#885 no job on that route names a stored secret at all — the status read is anonymous
|
||||
(`ci.pr-route-carries-no-stored-credential`). That closes the registry-credential route, not the
|
||||
general problem — `GITEA_TOKEN`, `RENOVATE_TOKEN`,
|
||||
and a collaborator's own token can all still POST a status, and none of that is closed by this
|
||||
test.
|
||||
|
||||
|
||||
@@ -1,4 +1,9 @@
|
||||
"""Every `actions/checkout` drops the persisted credential (ersatztv#835).
|
||||
"""TWO credential invariants over one derived workflow population.
|
||||
|
||||
Every `actions/checkout` drops the persisted credential (ersatztv#835) — below — and no job
|
||||
the `pull_request` trigger reaches, nor the workflow scope outside `jobs:`, names a stored
|
||||
secret (ersatztv#885). The second is documented at its own banner further down, where the
|
||||
population it adds to this one is derived; what follows here is the first.
|
||||
|
||||
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
|
||||
@@ -27,6 +32,7 @@ habit. `test_no_workflow_hides_in_a_SUBDIRECTORY` covers nesting below `.gitea/w
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from collections import Counter
|
||||
from pathlib import Path
|
||||
|
||||
import yaml
|
||||
@@ -299,3 +305,882 @@ def test_no_workflow_lives_under_dot_GITHUB() -> None:
|
||||
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.
|
||||
#
|
||||
# THE SCOPING COSTS EXACTLY ONE REAL SPELLING, AND IT IS NOT NOTHING: an `if:` value is an expression
|
||||
# WHETHER OR NOT it is wrapped, so `if: secrets.REGISTRY_PASSWORD != ''` names a stored secret in a
|
||||
# document that contains no `${{` at all. That an unwrapped condition is evaluated rather than read
|
||||
# as text is demonstrated by this repo's own `docker-build.yml` and not inferred — `build` carries
|
||||
# `if: github.event_name != 'pull_request'` bare, and `PR_EXCLUDING_IFS` below pins that exact
|
||||
# unwrapped string. `if:` is the only key whose value the grammar lets omit the delimiters, so the
|
||||
# scoping is repaired AT THAT KEY rather than abandoned: `condition_refs` reads an `if:` value as one
|
||||
# span, and `secret_name_counts` routes the value there instead of through `secret_refs`. Everywhere
|
||||
# else the scoping stands, and the English sentence above still costs nothing.
|
||||
_EXPRESSION = re.compile(r"\$\{\{(.*?)\}\}", re.S)
|
||||
_SECRETS_TOKEN = re.compile(r"(?<![A-Za-z0-9_])secrets(?![A-Za-z0-9_])", re.I)
|
||||
|
||||
# `${{` and `}}` as tokens, for reading an `if:` value that mixes the wrapped and unwrapped forms.
|
||||
# They are replaced by a SPACE and not deleted: `${{ secrets.A }}${{ secrets.B }}` collapsed by
|
||||
# deletion reads as the one identifier `secrets.Asecrets` and LOSES a reference, the fail-open
|
||||
# direction.
|
||||
_EXPRESSION_DELIMITER = re.compile(r"\$\{\{|\}\}")
|
||||
|
||||
# The one key whose value the expression grammar evaluates with the delimiters omitted. Matched
|
||||
# case-INSENSITIVELY where it is read, the direction `_SECRET_REF` and `SECRETS_KEY` both take.
|
||||
IF_KEY = "if"
|
||||
|
||||
|
||||
def _refs_in_expression(expression: str) -> list[str]:
|
||||
"""Every `secrets` reference inside ONE expression span — the resolved names, then the residue.
|
||||
|
||||
Shared by both entry points, so that widening a spelling widens the wrapped and the unwrapped
|
||||
reading together: a second copy of this resolution would be free to drift from the one the
|
||||
assertion runs on, which is the shape this guard exists to catch.
|
||||
"""
|
||||
resolved = [next(group for group in m.groups() if group is not None) for m in _SECRET_REF.finditer(expression)]
|
||||
# The residue: `secrets` tokens in this span that resolved to no literal name.
|
||||
residue = len(_SECRETS_TOKEN.findall(expression)) - len(resolved)
|
||||
return resolved + [WHOLE_SECRETS_CONTEXT] * residue
|
||||
|
||||
|
||||
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):
|
||||
found.extend(_refs_in_expression(expression))
|
||||
return found
|
||||
|
||||
|
||||
def condition_refs(condition: str) -> list[str]:
|
||||
"""Every `secrets` reference in an `if:` value, which is an expression with or without `${{ }}`.
|
||||
|
||||
The WHOLE value is read as one span, with the delimiters neutralised rather than honoured, so
|
||||
that a condition mixing the two forms — `${{ true }} && secrets.X != ''` — is covered by the same
|
||||
read as the bare one, and a fully wrapped condition still counts each reference exactly once.
|
||||
This is strictly more demanding than `secret_refs` on the same string and never less: an `if:` is
|
||||
never prose, so the over-match the span scoping exists to avoid cannot arise here.
|
||||
|
||||
WHAT AN `if:` REFERENCE COSTS DIFFERS FROM EVERY OTHER SITE THIS GUARD READS, and it is faulted
|
||||
anyway. A secret in an `env:` or a `run:` is materialised into the job environment; a secret in a
|
||||
condition is resolved by the evaluator and the job sees only the boolean. The predicate here is
|
||||
"names a stored secret", never "exports one" — on the head-authored route a condition comparing a
|
||||
secret to a literal the contributor chooses is an oracle over its value, and a predicate that
|
||||
asked about EXPOSURE would have to model what each site does with the reference, which is the
|
||||
structure-blindness this collector deliberately does not give up (it is what saw
|
||||
`toolchain-preflight`'s step `env:` when a `container:`-shaped predicate did not).
|
||||
"""
|
||||
return _refs_in_expression(_EXPRESSION_DELIMITER.sub(" ", condition))
|
||||
|
||||
|
||||
# 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.
|
||||
|
||||
One key is read differently, and it is a KEY rather than a place in the document: the value of an
|
||||
`if:` is an expression with or without `${{ }}`, so it goes through `condition_refs`. Read as an
|
||||
ordinary string it would be scoped to its `${{ }}` spans and a bare `if: secrets.X != ''` would
|
||||
be invisible, on the route where the head writes the file. The value is routed there INSTEAD of
|
||||
onto the stack, so a wrapped condition is counted once rather than twice.
|
||||
"""
|
||||
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)
|
||||
if isinstance(key, str) and key.lower() == IF_KEY and isinstance(value, str):
|
||||
found.update(condition_refs(value))
|
||||
else:
|
||||
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))
|
||||
|
||||
|
||||
# The second way a document hands stored secrets to a run: a `secrets:` KEY, which names them in the
|
||||
# YAML grammar rather than inside an expression. `jobs.<id>.secrets:` on a `uses:` (reusable-workflow)
|
||||
# job passes the caller's store to the called workflow, and its value takes one of two shapes — a
|
||||
# MAPPING of name -> value, whose values are `${{ }}` expressions the collector above already reads,
|
||||
# or the bare scalar `inherit`, which hands over the WHOLE store while naming nothing at all.
|
||||
#
|
||||
# `inherit` is the `toJSON(secrets)` shape one level up, and it defeats the collector for the same
|
||||
# reason the span scoping is correct: `secret_refs` reads only inside `${{ }}`, and `inherit` is not
|
||||
# an expression. Measured 2026-09-05 against the predecessor of this commit, a `pull_request` job of
|
||||
# `{'uses': './.gitea/workflows/reusable.yml', 'secrets': 'inherit'}` was put in the population by
|
||||
# `pull_request_jobs`, walked, and reported CLEAN (`stored_secret_faults(...) == []`), while the
|
||||
# mapping spelling produced one correctly-named fault — so the miss was in the VALUE SHAPE, not the
|
||||
# key. That is precisely a new job silently joining the population unprotected. Both halves of that
|
||||
# measurement are RE-DERIVED every run rather than left as prose — `secret_names(job) -
|
||||
# INJECTED_SECRETS` is the predecessor collector verbatim, and
|
||||
# `test_a_SECRETS_HANDOVER_naming_nothing_is_reported_under_the_WHOLE_CONTEXT_sentinel` asserts it
|
||||
# empty on the same fixture it asserts the fault on.
|
||||
#
|
||||
# The test is on the value shape and not on the word `inherit`, for the reason the residue counter is
|
||||
# not a match on `toJSON`: any value that is not a mapping of names hands over something this guard
|
||||
# cannot enumerate, including a spelling act_runner grows later. Whether this instance's runner
|
||||
# resolves `workflow_call` + `secrets: inherit` at all was NOT probed from here; the direction makes
|
||||
# that acceptable, as it does for the expression spellings — an unsupported shape costs a spurious
|
||||
# demand on a job nobody has written, and the omission cost the whole store.
|
||||
#
|
||||
# This clause is DOCUMENT-ONLY, stated here rather than left to be discovered: `secrets:` is a plain
|
||||
# YAML key, so the text-versus-walk cross-check has nothing to match on and does not cover it. It
|
||||
# also does not REDDEN that cross-check, because the clause feeds the fault collector and not
|
||||
# `secret_name_counts`, which both halves read through.
|
||||
#
|
||||
# The key is matched case-INSENSITIVELY, the same direction `_SECRET_REF` takes: the grammar spells it
|
||||
# lowercase, and reading `Secrets:` as one too can only ever be too demanding.
|
||||
SECRETS_KEY = "secrets"
|
||||
|
||||
|
||||
def opaque_secret_handovers(node: object) -> int:
|
||||
"""`secrets:` keys in a subtree whose value is not a mapping of names — occurrences, not sites.
|
||||
|
||||
Structure-blind for the reason `secret_name_counts` is: walking for the key anywhere rather than
|
||||
at `jobs.<id>.secrets` avoids pinning the one location today's grammar documents
|
||||
(`testing.guard-derives-population-from-source`).
|
||||
"""
|
||||
found = 0
|
||||
stack: list[object] = [node]
|
||||
while stack:
|
||||
item = stack.pop()
|
||||
if isinstance(item, dict):
|
||||
for key, value in item.items():
|
||||
if isinstance(key, str) and key.lower() == SECRETS_KEY and not isinstance(value, dict):
|
||||
found += 1
|
||||
stack.append(value)
|
||||
elif isinstance(item, list):
|
||||
stack.extend(item)
|
||||
return found
|
||||
|
||||
|
||||
def held_secret_names(node: object) -> set[str]:
|
||||
"""Every STORED secret a subtree hands to the run: the names it references, plus the whole-context
|
||||
sentinel when it hands over a set this guard cannot enumerate.
|
||||
|
||||
The one place `INJECTED_SECRETS` is subtracted, so the two fault sites below cannot drift apart on
|
||||
which references are forgiven.
|
||||
"""
|
||||
held = secret_names(node) - INJECTED_SECRETS
|
||||
if opaque_secret_handovers(node):
|
||||
held.add(WHOLE_SECRETS_CONTEXT)
|
||||
return held
|
||||
|
||||
|
||||
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(held_secret_names(outside_jobs(doc)))
|
||||
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(held_secret_names(job))
|
||||
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.
|
||||
|
||||
A THIRD asymmetry, which is not the strip's: an `if:` value is read by `condition_refs`, so an
|
||||
UNWRAPPED `if: secrets.X != ''` is counted by the walk and not by the text half, which has no key
|
||||
to read it by and is scoped to `${{ }}` spans. The walk's count is then the larger one, which
|
||||
reddens — the safe direction, and on a document that is already faulting the stored-secret
|
||||
assertion for the same reference. Measured 2026-09-05, no tracked workflow names a secret in an
|
||||
`if:` at all, so nothing in the tree reaches it.
|
||||
|
||||
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`.
|
||||
|
||||
It equally cannot report a handover written in the YAML grammar rather than the expression
|
||||
grammar — `jobs.<id>.secrets: inherit` on a `uses:` job — and for a stronger reason than a shared
|
||||
blind spot: there is no expression for the text half to match at all. That shape is judged by
|
||||
`opaque_secret_handovers`, which reads the DOCUMENT only, and is asserted in
|
||||
`test_a_SECRETS_HANDOVER_naming_nothing_is_reported_under_the_WHOLE_CONTEXT_sentinel`.
|
||||
"""
|
||||
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
|
||||
|
||||
# AND THE SPELLING THAT SCOPING COSTS: an `if:` value is an expression whether or not it is
|
||||
# wrapped, so a condition naming a stored secret contains no `${{` and the span scoping cannot
|
||||
# see it. `docker-build.yml`'s own `build` job carries an unwrapped `if:`, so this is the shape
|
||||
# the repo already writes and not a hypothetical. Driven against the REAL predecessor on BOTH
|
||||
# rows: `secret_refs` is how `secret_name_counts` read an `if:` value before `condition_refs`,
|
||||
# and it is asserted empty on each condition here before the fault is demanded.
|
||||
for name, condition, at_step in (
|
||||
("REGISTRY_PASSWORD", "secrets.REGISTRY_PASSWORD != ''", False),
|
||||
("REGISTRY_PASSWORD", "secrets.REGISTRY_PASSWORD != ''", True),
|
||||
("RENOVATE_TOKEN", "secrets.RENOVATE_TOKEN != ''", False),
|
||||
("RENOVATE_TOKEN", "secrets.RENOVATE_TOKEN != ''", True),
|
||||
):
|
||||
assert secret_refs(condition) == [], (
|
||||
f"{condition!r} is supposed to be invisible to the span-scoped reader — if it is not, "
|
||||
"this row proves nothing about `condition_refs`."
|
||||
)
|
||||
assert condition_refs(condition) == [name], condition
|
||||
|
||||
job = (
|
||||
{"steps": [{"if": condition, "run": "true"}]} if at_step else {"if": condition, "steps": [{"run": "true"}]}
|
||||
)
|
||||
doc = {True: {"pull_request": None}, "jobs": {"j": job}}
|
||||
faults = stored_secret_faults("synthetic.yml", doc)
|
||||
assert len(faults) == 1, (name, faults)
|
||||
assert name in faults[0], (name, faults)
|
||||
|
||||
# A WRAPPED condition counts once, not twice — the delimiters are neutralised rather than read as
|
||||
# a second span, so the cross-check still agrees with the text half on the shape workflows write.
|
||||
wrapped = "jobs:\n j:\n if: ${{ secrets.REGISTRY_PASSWORD != '' }}\n steps:\n - run: true\n"
|
||||
assert secret_name_counts(yaml.safe_load(wrapped)) == Counter({"REGISTRY_PASSWORD": 1})
|
||||
assert walk_versus_text_faults("synthetic.yml", wrapped) == []
|
||||
|
||||
# A condition MIXING the two forms is read whole, so the unwrapped half is not lost behind the
|
||||
# wrapped one.
|
||||
assert condition_refs("${{ true }} && secrets.RENOVATE_TOKEN != ''") == ["RENOVATE_TOKEN"]
|
||||
# And two wrapped references in one condition stay two: the delimiters become a SPACE, so the
|
||||
# names cannot collapse into one identifier.
|
||||
assert condition_refs("${{ secrets.REGISTRY_USER }}${{ secrets.REGISTRY_PASSWORD }}") == [
|
||||
"REGISTRY_USER",
|
||||
"REGISTRY_PASSWORD",
|
||||
]
|
||||
|
||||
# The clause is on the `if:` KEY, so the English sentences above are untouched by it: they are
|
||||
# `run:` scalars, where the word is prose and the scoping still costs nothing.
|
||||
still_clean = {
|
||||
True: {"pull_request": None},
|
||||
"jobs": {"j": {"if": "github.event_name == 'push'", "steps": [{"run": "# We pass no secrets. Then true"}]}},
|
||||
}
|
||||
assert stored_secret_faults("synthetic.yml", still_clean) == []
|
||||
|
||||
# The pinned exclusion is itself an unwrapped condition and must stay clean — it names no secret,
|
||||
# and reading conditions must not start faulting every gated job.
|
||||
assert condition_refs("github.event_name != 'pull_request'") == []
|
||||
|
||||
|
||||
def test_a_SECRETS_HANDOVER_naming_nothing_is_reported_under_the_WHOLE_CONTEXT_sentinel() -> None:
|
||||
"""`secrets: inherit` hands the WHOLE store to a `uses:` job while naming no secret at all.
|
||||
|
||||
The `toJSON(secrets)` shape one level up — in the YAML grammar rather than the expression
|
||||
grammar — and so the one shape the span scoping above cannot see, since `inherit` is a plain
|
||||
scalar and not an expression. Driven against the REAL predecessor rather than a hand-written
|
||||
mutant: `secret_names(job) - INJECTED_SECRETS` is the collector exactly as it read before this
|
||||
clause, and it is asserted empty on every row here while the job IS in the population — walked,
|
||||
and reported clean. That is a job joining the population unprotected without reddening anything.
|
||||
|
||||
Whether act_runner on this instance resolves `workflow_call` + `secrets: inherit` was NOT probed
|
||||
from here; it affects reachability today, not the guard's silence, and the direction is the same
|
||||
one the spelling rows take — an unsupported shape costs a spurious demand on a job nobody has
|
||||
written.
|
||||
"""
|
||||
for value in ("inherit", "INHERIT", None, ["REGISTRY_PASSWORD"]):
|
||||
job = {"uses": "./.gitea/workflows/reusable.yml", "secrets": value}
|
||||
doc = {True: {"pull_request": None}, "jobs": {"reused": job}}
|
||||
|
||||
assert [job_id for job_id, _ in pull_request_jobs(doc)] == ["reused"], value
|
||||
assert secret_names(job) - INJECTED_SECRETS == set(), (
|
||||
f"a `secrets:` value of {value!r} is supposed to name nothing the reference collector "
|
||||
"can see — if it does, this row proves nothing about the handover clause."
|
||||
)
|
||||
|
||||
faults = stored_secret_faults("synthetic.yml", doc)
|
||||
assert len(faults) == 1, (value, faults)
|
||||
assert WHOLE_SECRETS_CONTEXT in faults[0], (value, faults)
|
||||
|
||||
# The KEY is read case-insensitively, the direction `_SECRET_REF` takes — asserted rather than
|
||||
# only claimed in the comment beside it.
|
||||
cased = {True: {"pull_request": None}, "jobs": {"reused": {"uses": "./x.yml", "Secrets": "inherit"}}}
|
||||
assert len(stored_secret_faults("synthetic.yml", cased)) == 1
|
||||
|
||||
# The MAPPING spelling is not what this clause reports: every value in it is an expression the
|
||||
# reference collector already reads, so it faults by NAME and not under the sentinel.
|
||||
named = {
|
||||
True: {"pull_request": None},
|
||||
"jobs": {"reused": {"uses": "./x.yml", "secrets": {"TOK": "${{ secrets.RENOVATE_TOKEN }}"}}},
|
||||
}
|
||||
faults = stored_secret_faults("synthetic.yml", named)
|
||||
assert len(faults) == 1, faults
|
||||
assert "RENOVATE_TOKEN" in faults[0] and WHOLE_SECRETS_CONTEXT not in faults[0], faults
|
||||
|
||||
# A `workflow_call` DECLARATION is a mapping of names, not a handover — the negative control that
|
||||
# keeps the clause from faulting every reusable workflow that declares its own inputs.
|
||||
declaring = {
|
||||
True: {"pull_request": None, "workflow_call": {"secrets": {"TOK": {"required": True}}}},
|
||||
"jobs": {"j": {"steps": [{"run": "true"}]}},
|
||||
}
|
||||
assert stored_secret_faults("synthetic.yml", declaring) == []
|
||||
|
||||
# The workflow scope is judged through the same helper, so the clause widens with it.
|
||||
scoped = {True: {"pull_request": None}, "secrets": "inherit", "jobs": {}}
|
||||
scope_faults = stored_secret_faults("synthetic.yml", scoped)
|
||||
assert len(scope_faults) == 1 and "WORKFLOW SCOPE" in scope_faults[0], scope_faults
|
||||
|
||||
# And it is judged on the route only, and only for a job the trigger reaches.
|
||||
off_route = {True: {"push": None}, "jobs": {"reused": {"uses": "./x.yml", "secrets": "inherit"}}}
|
||||
assert stored_secret_faults("synthetic.yml", off_route) == []
|
||||
gated = {
|
||||
True: {"pull_request": None},
|
||||
"jobs": {
|
||||
"reused": {"if": "github.event_name != 'pull_request'", "uses": "./x.yml", "secrets": "inherit"},
|
||||
},
|
||||
}
|
||||
assert stored_secret_faults("synthetic.yml", gated) == []
|
||||
|
||||
# The clause is DOCUMENT-ONLY: `secrets:` is a plain YAML key, so the text half of the
|
||||
# cross-check has nothing to match and must stay SILENT rather than report a disagreement over a
|
||||
# reference neither half can name.
|
||||
as_text = "jobs:\n reused:\n uses: ./x.yml\n secrets: inherit\n"
|
||||
assert secret_name_counts(yaml.safe_load(as_text)) == Counter()
|
||||
assert walk_versus_text_faults("synthetic.yml", as_text) == []
|
||||
|
||||
|
||||
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]
|
||||
|
||||
Reference in New Issue
Block a user