fix(ci): builder-image pushes authenticate, and :latest stops being a tag anyone can move #60

Merged
enricobuehler merged 1 commits from worktree-security-h6-registry-auth into main 2026-08-05 20:11:22 +00:00
Owner

Second half of H-6 from the security review 2026-08-05. Pairs with unom/infra#15.

Merge this reasonably soon. The infra half is already live, so until this lands docker.yml fails on a release tag (its Tag for release PUT now gets a 405) and on any push touching ci/. Ordinary pushes are fine — a docker.yml run on main went green after the infra change, because it hit the cache and skipped the push. Relevant to the prepared-but-untagged 0.25.0.

What the infra half did

The LAN registry accepted anonymous pushes from anyone, and every secret-bearing job here runs inside an image pulled from it. It's now split in two over one store: :5010 serves GET/HEAD only and 405s everything else, :5011 demands basic auth on every request including the /v2/ ping. A registry keys its store by repository name rather than by the host:port the client used, so an image pushed to :5011 is the identical image every consumer pulls from :5010.

What changes here

Builds tag the write port, a docker login precedes the push, and the release-tag manifest PUTs authenticate. CI_REGISTRY_PASSWORD is a repo Actions secret (already set).

Consumers are untouched. All 38 workflow files still pull anonymously from :5010, and ci/rust-ci-arm64cross.Dockerfile's FROM 192.168.1.58:5010/... still resolves — that's a read.

⚠ The digest pinning was replaced, not skipped

The review's fix line said "pin consumers by @sha256: digest". That was right while the registry was open. It isn't any more, and the header of docker.yml explains this at length:

Once pushes are authenticated, the set of people who can overwrite a tag is exactly the set who can push to main and edit a pinned digest in this very file. A pin defends against nobody it did not already trust, while costing a two-commit dance on every ci/ change (~3× a month) during which consumers silently run a builder image predating the change they are testing.

What does close the residual gap — a tag quietly overwritten out of band — is making :latest a checked function of the tree. The new .gitea/scripts/reconcile-latest.sh asserts on every run that :latest and :ck-$KEY are the same digest, re-points it when they aren't, and warns loudly. No churn, and it catches an overwrite on the next push to main.

It repairs rather than fails, because the same divergence has a legitimate cause: reverting ci/ leaves :latest on the newer build forever, since the older key is a cache hit and nothing re-points it. That's a pre-existing bug this fixes on the side, and it must not red-line main.

Revisit inline pins if the push credential ever leaves the maintainer trust set.

Verified

Against the live registry from a runner host with the real docker client:

unauthenticated push denied — no basic auth credentials
push to :5010 refused — 405 Method Not Allowed
authenticated push to :5011 accepted
same image pulled anonymously from :5010 works

reconcile-latest.sh exercised over all three of its cases: diverged → repaired (verified the digest actually moved), already equal → clean no-op, missing key → ::error:: and exit 1.

All seven builder images are consistent with their content keys today, so the reconcile step is a silent no-op on its first real run — no spurious warning on merge.

Small extra

A docker logout with if: always() closes both jobs. The job container is ephemeral today (the ubuntu-24.04 label is a docker:// image) so the credential would die with it anyway — this just stops that being a load-bearing assumption about a runner label someone may later point at a host runner.

Second half of **H-6** from the security review 2026-08-05. Pairs with unom/infra#15. ⚠ **Merge this reasonably soon.** The infra half is already live, so until this lands `docker.yml` fails on a release tag (its `Tag for release` PUT now gets a 405) and on any push touching `ci/`. Ordinary pushes are fine — a `docker.yml` run on main went green after the infra change, because it hit the cache and skipped the push. Relevant to the prepared-but-untagged 0.25.0. ## What the infra half did The LAN registry accepted anonymous pushes from anyone, and every secret-bearing job here runs inside an image pulled from it. It's now split in two over one store: `:5010` serves `GET`/`HEAD` only and 405s everything else, `:5011` demands basic auth on every request including the `/v2/` ping. A registry keys its store by repository name rather than by the `host:port` the client used, so an image pushed to `:5011` is the identical image every consumer pulls from `:5010`. ## What changes here Builds tag the write port, a `docker login` precedes the push, and the release-tag manifest PUTs authenticate. `CI_REGISTRY_PASSWORD` is a repo Actions secret (already set). **Consumers are untouched.** All 38 workflow files still pull anonymously from `:5010`, and `ci/rust-ci-arm64cross.Dockerfile`'s `FROM 192.168.1.58:5010/...` still resolves — that's a read. ## ⚠ The digest pinning was replaced, not skipped The review's fix line said *"pin consumers by `@sha256:` digest"*. That was right while the registry was open. It isn't any more, and the header of `docker.yml` explains this at length: > Once pushes are authenticated, the set of people who can overwrite a tag is exactly the set who can push to main and edit a pinned digest in this very file. A pin defends against nobody it did not already trust, while costing a two-commit dance on every `ci/` change (~3× a month) during which consumers silently run a builder image predating the change they are testing. What *does* close the residual gap — a tag quietly overwritten out of band — is making `:latest` a **checked function of the tree**. The new `.gitea/scripts/reconcile-latest.sh` asserts on every run that `:latest` and `:ck-$KEY` are the same digest, re-points it when they aren't, and warns loudly. No churn, and it catches an overwrite on the next push to main. It **repairs rather than fails**, because the same divergence has a legitimate cause: reverting `ci/` leaves `:latest` on the newer build forever, since the older key is a cache hit and nothing re-points it. That's a pre-existing bug this fixes on the side, and it must not red-line main. Revisit inline pins if the push credential ever leaves the maintainer trust set. ## Verified Against the live registry from a runner host with the real docker client: | | | |---|---| | unauthenticated push | denied — `no basic auth credentials` | | push to `:5010` | refused — `405 Method Not Allowed` | | authenticated push to `:5011` | accepted | | same image pulled anonymously from `:5010` | works | `reconcile-latest.sh` exercised over all three of its cases: diverged → repaired (verified the digest actually moved), already equal → clean no-op, missing key → `::error::` and exit 1. All seven builder images are consistent with their content keys today, so the reconcile step is a **silent no-op on its first real run** — no spurious warning on merge. ## Small extra A `docker logout` with `if: always()` closes both jobs. The job container is ephemeral today (the `ubuntu-24.04` label is a `docker://` image) so the credential would die with it anyway — this just stops that being a load-bearing assumption about a runner label someone may later point at a host runner.
enricobuehler added 1 commit 2026-08-05 19:44:34 +00:00
fix(ci): builder-image pushes authenticate, and :latest stops being a tag anyone can move
ci / docs-site (pull_request) Successful in 1m20s
ci / web (pull_request) Successful in 1m25s
ci / rust-arm64 (pull_request) Successful in 1m40s
ci / rust (pull_request) Successful in 6m8s
19f637ea6e
Second half of security-review-2026-08-05 H-6. The infra half (unom/infra,
runners/ci-core/) split the LAN registry in two: :5010 serves GET/HEAD only and
refuses everything else with 405, :5011 demands basic auth on every request
including the /v2/ ping. Both fronts sit on one store, and a registry keys by
repository name rather than by the host:port the client used, so an image
pushed to :5011 is the identical image every consumer pulls from :5010.

So: builds tag the write port, a docker login precedes the push, and the
release-tag manifest PUTs authenticate. Consumers are untouched — every
`container:` in every other workflow still pulls anonymously from :5010, and
ci/rust-ci-arm64cross.Dockerfile's `FROM 192.168.1.58:5010/...` still resolves.

Not doing the digest pinning the review asked for, deliberately, and the header
says why at length. Once pushes are authenticated, the people who can overwrite
a tag are exactly the people who can push to main and edit a pinned digest in
this file — a pin defends against nobody it did not already trust, and costs a
two-commit dance on every ci/ change (~3x a month) during which consumers run a
builder image predating the change they are testing.

What does close the residual gap is making :latest a checked function of the
tree. reconcile-latest.sh asserts on every run that :latest and :ck-$KEY are the
same digest, re-points it when they are not, and warns loudly. An out-of-band
overwrite is caught on the next push to main with no churn, and it fixes a
pre-existing bug on the side: reverting ci/ used to leave :latest on the newer
build forever, because the older key is a cache hit and nothing re-pointed it.
Repair rather than fail, because a legitimate revert must not red-line main.

Verified against the live registry from a runner host with the real docker
client: unauthenticated push denied, push to :5010 refused 405, authenticated
push to :5011 accepted, that same image pulled back anonymously from :5010.
reconcile-latest.sh exercised over all three cases (diverged -> repaired,
already equal -> no-op, missing key -> exit 1). All seven builder images are
consistent with their content keys today, so the new step is a silent no-op on
its first real run.
enricobuehler merged commit cbd0e9664d into main 2026-08-05 20:11:22 +00:00
enricobuehler deleted branch worktree-security-h6-registry-auth 2026-08-05 20:11:30 +00:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: unom/punktfunk#60