fix(697): scope CI's registry credential so head-resolved workflows cannot forge review-verdict/h10 #745

Merged

timothy merged 2 commits from fix/697-scope-ci-registry-credential into main 2026-08-05 21:56:53 +02:00

Conversation 1 Commits 2 Files Changed 8

+149 -27

timothy commented 2026-08-05 21:25:00 +02:00

Owner

Copy Link

Copy Source

Closes the credential half of #697. Three adversarial review rounds; final verdict MERGEABLE.

What was wrong

REGISTRY_USER/REGISTRY_PASSWORD were the admin account's basic auth, and docker-build.yml triggers on pull_request — head-resolved — so a PR's own code was handed instance-admin credentials. Basic auth carries no scope, so the secret that pushes an image also administers every repo on the instance, and can POST review-verdict/h10 with human provenance, which the gate's provenance check accepts as a real verdict.

The fix

REGISTRY_PASSWORD is now a PAT scoped write:package + read:repository (applied to the instance; not in this diff). Fixed at the credential rather than the triggers: patching triggers enumerates instances, and adding a new workflow file is itself a route, so that enumeration never completes.

Verified on Gitea 1.25.4, not inferred:

• registry push of a probe tag SUCCEEDED (deleted after; confirmed 404)
• GET /commits/{sha}/status 200 — what ci-detect-already-validated.sh does
• POST /statuses/{sha} 403 required=[write:repository], token scope=write:package,read:repository

What this does NOT close — read before citing it as a mitigation

1. RENOVATE_TOKEN — write:repository bot PAT in the same secret store, posts with a real creator, is inherited. Cannot be scoped down. → #742
2. The injected GITEA_TOKEN — write-capable in every job. permissions: YAML is a no-op before Gitea 1.26.0 and no app.ini lever exists at any version. → server-management#714
3. A collaborator's own token — branch protection binds the context name, not its issuer. Unfixable in-repo.
4. The v* tag push — publishes :prod from an arbitrary tagged commit; tag protections are empty.
5. Direct pushes to main are server-side permitted, so the gate is skippable with no forgery at all. → #743 (priority: high). A push whitelist closes more of this class than the 1.26 upgrade.

h10 is a process guard against mistakes and drift, not a security boundary against push access.

Also surfaced

Auditing the secret store rather than the workflow set found SERVERMGMT_DEPLOY_KEY, orphaned since 1b5efd7b9 removed its bump-prod-compose job, whose key on server-management is read_only: false — write access to the repo holding prod's GitOps stack definitions. Left in place by explicit decision, recorded so it is accepted rather than forgotten.

ci-image.yml's trigger filter was attempted here and reverted: editing that file re-points ci-image-pin's expected at the editing commit and fails a blocking job. Filed as #744, which also records that the justification used for it ("branch publishing is not load-bearing") was false.

Verification

432 script tests pass; decisions_validate.py --base origin/main --head HEAD and build_decisions_catalog.py --check both exit 0; ci-image-pin recomputed by hand and matching the pinned commit.

Docs

New record ci.actions-credential-scoping; corrections to ci.gate-trigger-base-resolved, release.verdict-status-check, ci.exemption-provenance, docs/ci-cd.md, and two comments in review-verdict.yml plus two test docstrings — all previously said #697 was open, and several overstated what the fix closes.

fixes #697

Closes the credential half of #697. **Three adversarial review rounds**; final verdict MERGEABLE. ## What was wrong `REGISTRY_USER`/`REGISTRY_PASSWORD` were the **admin account's** basic auth, and `docker-build.yml` triggers on `pull_request` — head-resolved — so a PR's own code was handed instance-admin credentials. Basic auth carries no scope, so the secret that pushes an image also administers every repo on the instance, and can POST `review-verdict/h10` with **human provenance**, which the gate's provenance check accepts as a real verdict. ## The fix `REGISTRY_PASSWORD` is now a PAT scoped `write:package` + `read:repository` (applied to the instance; not in this diff). Fixed at the credential rather than the triggers: patching triggers enumerates *instances*, and adding a new workflow file is itself a route, so that enumeration never completes. **Verified on Gitea 1.25.4, not inferred:** - registry push of a probe tag **SUCCEEDED** (deleted after; confirmed 404) - `GET /commits/{sha}/status` **200** — what `ci-detect-already-validated.sh` does - `POST /statuses/{sha}` **403** `required=[write:repository], token scope=write:package,read:repository` ## What this does NOT close — read before citing it as a mitigation 1. **`RENOVATE_TOKEN`** — `write:repository` bot PAT in the same secret store, posts with a real creator, **is inherited**. Cannot be scoped down. → **#742** 2. **The injected `GITEA_TOKEN`** — write-capable in every job. `permissions:` YAML is a **no-op** before Gitea 1.26.0 and no `app.ini` lever exists at any version. → **server-management#714** 3. **A collaborator's own token** — branch protection binds the context *name*, not its issuer. Unfixable in-repo. 4. **The `v*` tag push** — publishes `:prod` from an arbitrary tagged commit; tag protections are empty. 5. **Direct pushes to `main` are server-side permitted**, so the gate is skippable with **no forgery at all**. → **#743** (`priority: high`). A push whitelist closes more of this class than the 1.26 upgrade. `h10` is a process guard against mistakes and drift, **not** a security boundary against push access. ## Also surfaced Auditing the secret **store** rather than the workflow set found `SERVERMGMT_DEPLOY_KEY`, orphaned since `1b5efd7b9` removed its `bump-prod-compose` job, whose key on server-management is `read_only: false` — write access to the repo holding prod's GitOps stack definitions. **Left in place by explicit decision**, recorded so it is accepted rather than forgotten. `ci-image.yml`'s trigger filter was attempted here and **reverted**: editing that file re-points `ci-image-pin`'s `expected` at the editing commit and fails a **blocking** job. Filed as **#744**, which also records that the justification used for it ("branch publishing is not load-bearing") was false. ## Verification 432 script tests pass; `decisions_validate.py --base origin/main --head HEAD` and `build_decisions_catalog.py --check` both exit 0; `ci-image-pin` recomputed by hand and matching the pinned commit. ## Docs New record `ci.actions-credential-scoping`; corrections to `ci.gate-trigger-base-resolved`, `release.verdict-status-check`, `ci.exemption-provenance`, `docs/ci-cd.md`, and two comments in `review-verdict.yml` plus two test docstrings — all previously said #697 was open, and several overstated what the fix closes. fixes #697

timothy added 2 commits 2026-08-05 21:25:01 +02:00

fix(697): scope CI's registry credential so head-resolved workflows cannot forge review-verdict/h10 ... e298bb291e

`REGISTRY_USER`/`REGISTRY_PASSWORD` were the ADMIN account's basic auth, and
`docker-build.yml` triggers on `pull_request` — head-resolved — so a PR's own
code was handed instance-admin credentials. Basic auth carries no scope, so the
same secret that pushes an image administers every repo on the instance and can
POST `review-verdict/h10`, the required context that makes merge-consent derived
rather than assertable. Refs #697.

Fixed at the credential, not the triggers: patching triggers enumerates
instances of "a ref-resolved workflow obtains status-capable credentials", and
adding a new workflow file is itself a route. `REGISTRY_PASSWORD` is now a PAT
scoped `write:package` + `read:repository`.

Verified on Gitea 1.25.4, not inferred:
  - registry push of a probe tag SUCCEEDED (cleaned up, confirmed 404)
  - GET /commits/{sha}/status SUCCEEDED (what ci-detect-already-validated.sh does)
  - POST /statuses/{sha} REFUSED, HTTP 403:
    required=[write:repository], token scope=write:package,read:repository

Scope of what this closes, stated without overclaim. It closes the instance-wide
admin escalation and that credential's durable forgery route — durable because a
status POSTed with a USER credential carries a real `creator` and is inherited as
a human verdict, while an Actions job's carries `creator: null` and is re-derived.
It does NOT close the class. Three things survive it:

  - `RENOVATE_TOKEN` is a `write:repository` PAT of a real bot account in the
    SAME secret store, so it also posts with non-null `creator`. It cannot be
    scoped down (Renovate needs repo write), and secrets are a per-repo store
    that any PR-added workflow can reference. Closing this needs the provenance
    check tightened to an allow-list of approved reviewers.
  - Every job still receives a write-capable `GITEA_TOKEN`. `permissions:` YAML
    is a no-op before Gitea 1.26.0 and no `app.ini` lever exists at any version;
    only >=1.26 with the Actions default set to Restricted binds it.
    Tracked in server-management#714.
  - Branch protection binds the context NAME, not its issuer, so any write-scoped
    personal token forges the status with genuine human provenance. Unfixable
    in-repo. `h10` is a process guard, not a security boundary against push access.

Auditing the secret STORE rather than the workflow set also surfaced
`SERVERMGMT_DEPLOY_KEY`, still present though the `bump-prod-compose` job that
used it was removed in 1b5efd7b9 — an SSH deploy key to another repo, obtainable
by any PR-added workflow, with no remaining benefit.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

fix(697): correct the overclaims three adversarial review rounds found ... b91939e5c4

Round 1 BLOCKED (1 Blocker, 4 High, 3 Medium, 2 Low); round 2 BLOCKED on the fix
(1 Blocker, 2 High, 4 Medium, 2 Low); round 3 BLOCKED on one Medium. Every
finding re-verified against the live instance before acting.

ROUND 2 — the blocker was self-inflicted and the local gate could not see it.
Adding `branches: [main]` to ci-image.yml re-points `ci-image-pin`'s `expected`
at the editing commit, staling all five `container:` pins and failing that
BLOCKING job — for a change altering zero bytes of the toolchain image.
Reproduced: expected=ed9dd6254 vs pins=32747a0. Reverted here (the commit was
amended, so no commit on the branch touches that path) and filed as #744.

That edit had also FALSIFIED its own justification: branch publishing IS
load-bearing — docs/ci-cd.md documents the rebase-recovery flow as "let
ci-image.yml publish :<short sha>, then bump the pin", which is how you satisfy
ci-image-pin from inside a PR. Reverting also keeps three trigger descriptions
true (ci-cd.md:1043, the recovery flow, pr-checks.yml's escape-hatch comment).

Also fixed:
  - gate-trigger-base-resolved.md was the file round 1's fix did not touch, and
    still said "no workflow route retains human provenance" — false, since a
    PR-added workflow can reference RENOVATE_TOKEN. Its `rule:` also kept the
    race framing, and `rule:` is what the catalog and MemPalace mirror.
  - `mechanics:` claimed "independent review confirmed no CI consumption breaks".
    It confirmed no such thing. Round 3 then caught the REPLACEMENT sentence
    making the same class of error: only the `container:` pull is exercised by a
    PR, because `build` carries `if: github.event_name != 'pull_request'` and
    cache-to/cache-from live only there. Those and the base-image pull first run
    on the post-merge push to main — a wrong inference reddens main, not the PR.
  - A fourth surviving route was unnamed: docker-build.yml publishes :prod from a
    `v*` tag push and a tag may point at any commit (tag protections are empty).
    "three surviving routes" became "at least these" — a count reads as complete.
  - Unmarked inferences, a "three later sections" that undercounted four, a
    dangling "the two items below", and a #744 rationale that stated the pin toll
    without its documented remedy.

Local gate: 432 script tests pass; `decisions_validate.py --base origin/main
--head HEAD` and `build_decisions_catalog.py --check` both exit 0; ci-image-pin
recomputed by hand and matching the pinned commit. The record is 62 prose lines
against a 60-line ceiling that is a `::warning::` by design (#520) — the blocking
constraint is the 2-25% minority band, currently 10.8%.

Refs #697, #742, #743, #744.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

timothy commented 2026-08-05 21:26:06 +02:00

Author

Owner

Copy Link

Copy Source

Review-verdict: MERGEABLE @ b91939e

Independent cold adversarial review, 3 rounds (delegated agent, no implementation role). R1 BLOCKED 1B/4H/3M/2L, R2 BLOCKED 1B/2H/4M/2L, R3 BLOCKED 1M, R4 clean. Findings re-verified against the live instance by the author before acting, not accepted on assertion. Tree f59a5fb7 is byte-identical to reviewed sha 373ed4f09; the amend fixed only that commit's two Low message findings.

Review-verdict: MERGEABLE @ b91939e Independent cold adversarial review, 3 rounds (delegated agent, no implementation role). R1 BLOCKED 1B/4H/3M/2L, R2 BLOCKED 1B/2H/4M/2L, R3 BLOCKED 1M, R4 clean. Findings re-verified against the live instance by the author before acting, not accepted on assertion. Tree f59a5fb7 is byte-identical to reviewed sha 373ed4f09; the amend fixed only that commit's two Low message findings.

timothy merged commit 08e95f9ec1 into main 2026-08-05 21:56:52 +02:00

timothy referenced this issue from a commit 2026-08-05 21:56:53 +02:00

fix(697): scope CI's registry credential so head-resolved workflows cannot forge review-verdict/h10 (#745)

timothy deleted branch fix/697-scope-ci-registry-credential 2026-08-05 21:56:53 +02:00

timothy referenced this pull request 2026-08-05 21:57:07 +02:00

`ETV_STATUS_AUTH` can write commit statuses, so any head-resolved workflow can still forge `review-verdict/h10` #697

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

ad-hoc

One-off / ad-hoc work not tracked by a dedicated issue

api

REST API / HTTP endpoints

bug

Something isn't working

ci-cd

Build, test, deploy pipeline

content

Channel content / schedules / playlists

dependencies

Dependency updates (Renovate)

enhancement

New feature or improvement

frontend

ChicoryTV React SPA frontend

in-progress

Claimed by an active session — do not pick up

jellyfin

Jellyfin tuner / IPTV integration

parked

Excluded from automatic queue pickup; work only when explicitly selected

priority: high

priority: low

priority: medium

review

Adversarial review finding

security

Security / vulnerability fix

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate timothy (Timothy)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: timothy/ersatztv#745