ci(e2e): refuse a new fork image before the build starts

The fork e2e lane pushes a fork's image archives only under names the
base tree already exports, which keeps a fork from minting repositories
under the CI registry. An image the PR itself adds is therefore refused,
but only at the push, after the whole build has run, and nothing the
contributor does on the PR can clear it.

Compare the image names of the PR tree and its base in the plan job of
fork runs, with the same make -n derivation the push allowlist uses,
and fail there naming the new images and what has to happen first. The
push allowlist is unchanged and remains the boundary: this check runs
from the fork's own tree, so it only saves an honest contributor the
wasted build.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
This commit is contained in:
Aleksei Sviridkin committed 2026-10-06 16:28:51 +03:00
1 parent 9734a16692
commit 80eb2e5037
4 files changed
+132 -1

No files matched your search

+10
View File
@@ -131,6 +131,16 @@ jobs:
- uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4
with:
fetch-depth: 0
# e2e-fork.yaml pushes a fork's images only under names the base tree
# already exports, so an image the PR adds is refused there after the
# whole build. Refuse it here instead, before anything builds. The push
# check stays the boundary; a fork can edit this step away.
- name: Refuse images the base branch does not build (fork PR)
if: ${{ github.event.pull_request.head.repo.fork }}
run: |
bt="$(mktemp -d)/basetree"
git worktree add --quiet --detach "$bt" HEAD^1
hack/fork-new-images.sh "$bt" .
- name: Compute changed files and build matrix
id: p
env:
+1 -1
View File
@@ -169,7 +169,7 @@ Consequences to keep in mind when touching either workflow:
- **Fork TIA reads trusted inputs, but the fork still writes the selector.** `e2e-fork.yaml`'s `e2e` job runs the same `select-e2e.sh` TIA as in-tree (§7), and its inputs are hardened: the changed-file list and the `full-e2e` label come from `resolve`'s base-repo API lookups (`pulls.listFiles` / the PR's labels), the same trusted source as the docs-only decision, passed in as outputs (`changed_b64`, `full_e2e`) rather than re-derived from a `git diff` of the fork-controlled merge ref. Keep that part as it is: re-deriving the selection from the checked-out tree would add a second bypass for no gain.
- **What the fork lane is, and is not.** The `e2e` job runs from the fork's tree, so `select-e2e.sh`, the suites under `hack/e2e-chainsaw/`, `hack/e2e-install-cozystack.bats` and `packages/core/testing`'s recipes are all fork-authored. A fork that wants its own suite to report green can edit any of them, and hardening the TIA inputs does not change that. Read the lane accordingly: it is a correctness gate that stops an honest contributor shipping a regression, not a boundary against a hostile one. What contains a hostile fork is that no fork run executes without maintainer approval (below), that the job holds no push credential, and that a diff editing the test harness is visible in review.
- **A fork PR that adds a new image cannot go green until that image exists on the base branch.** `publish` derives the set of archive names it will accept from the base parent (`HEAD^1`), so that a fork cannot mint arbitrary repositories under the CI registry by naming an artifact. An image the PR itself introduces is therefore not in that set, and the job fails with `image '<name>' is not in the base-tree allowlist`. This is fail-closed by design and there is no label override — land the image on the base branch first, or run the suite from a same-repo branch. If a fork e2e run fails on that line, the contribution is not broken; the allowlist is doing its job.
- **A fork PR that adds a new image cannot go green until that image exists on the base branch.** `publish` derives the set of archive names it will accept from the base parent (`HEAD^1`), so that a fork cannot mint arbitrary repositories under the CI registry by naming an artifact. An image the PR itself introduces is therefore not in that set, and the job fails with `image '<name>' is not in the base-tree allowlist`. This is fail-closed by design and there is no label override — land the image on the base branch first, or run the suite from a same-repo branch. If a fork e2e run fails on that line, the contribution is not broken; the allowlist is doing its job. So that a contributor learns this before the build rather than after it, `plan` in `pull-requests.yaml` runs `hack/fork-new-images.sh` on fork PRs and fails with the names of the new images. That check runs from the fork's tree and is a courtesy, not the boundary; `hack/fork-new-images.bats` holds its derivation to the one `publish` uses.
- **`labeled` events run in their own concurrency group, and a cancelled run reports nothing.** Because the gate is a status rather than a job, a run that dies without posting leaves the head SHA at "Expected" — which blocks the merge rather than passing it. Several separate pieces implement it, and weakening any one of them reopens the gap. Each lane's terminal reporter runs under `!cancelled()` rather than `always()`, so a run that was cancelled publishes nothing at all and the `pending` it opened stands until the run that superseded it reports. Each lane's concurrency key then keeps a run that is going to publish nothing out of the group of the run that is: `pull-requests.yaml`'s appends `-label` for a `labeled` event that `plan` discards, so labelling an open PR cannot cancel a live `opened`/`synchronize` run (the replacement run would never post the status the cancelled run was going to) — the condition is the complement of `plan`'s guard, so a `full-e2e` label, which does publish, stays in the main group and supersedes the head run instead of racing it, `e2e-fork.yaml`'s concurrency key appends the triggering run's id when that run concluded `cancelled` or `skipped` so a privileged run that is going to post nothing cannot cancel the one that is going to post, and `e2e-fork.yaml`'s `resolve` returns early on a `cancelled`/`skipped` triggering run instead of posting `failure` for it. If you touch any of them, keep the invariant: exactly one run per head SHA is responsible for the status, and a run that is not that one must post nothing at all.
+83
View File
@@ -0,0 +1,83 @@
#!/usr/bin/env bats
# hack/fork-new-images.sh refuses a fork PR that adds an image the base tree
# does not export, before the build. Its derivation has to stay the one
# e2e-fork.yaml uses for the push allowlist, or the early check and the push
# disagree about what counts as new.
ROOT="$(pwd)"
SCRIPT="$ROOT/hack/fork-new-images.sh"
FORK_WF="$ROOT/.github/workflows/e2e-fork.yaml"
PR_WF="$ROOT/.github/workflows/pull-requests.yaml"
# make_tree DIR NAME... — a git index holding one package per NAME whose image
# target exports NAME.oci.tar, the way the real recipes do under OCI_EXPORT_DIR.
make_tree() {
dir="$1"
shift
mkdir -p "$dir"
git -C "$dir" init --quiet
for name in "$@"; do
mkdir -p "$dir/packages/system/$name"
printf 'image:\n\tdocker buildx build --output type=oci,dest=$(OCI_EXPORT_DIR)/%s.oci.tar .\n' "$name" \
> "$dir/packages/system/$name/Makefile"
done
git -C "$dir" add --all
}
@test "an image only the head tree exports is refused and named" {
tmp="$(mktemp -d)"
make_tree "$tmp/base" alpha beta
make_tree "$tmp/head" alpha beta gamma delta
rc=0
"$SCRIPT" "$tmp/base" "$tmp/head" > "$tmp/out" 2>&1 || rc=$?
[ "$rc" -eq 1 ]
grep -q '^::error title=New image in a fork PR::' "$tmp/out"
grep -q 'does not build: delta gamma\.' "$tmp/out"
rm -rf "$tmp"
}
@test "the same image set, or a removed image, passes" {
tmp="$(mktemp -d)"
make_tree "$tmp/base" alpha beta
make_tree "$tmp/same" alpha beta
make_tree "$tmp/fewer" alpha
"$SCRIPT" "$tmp/base" "$tmp/same"
"$SCRIPT" "$tmp/base" "$tmp/fewer"
rm -rf "$tmp"
}
@test "a base tree that yields no names fails as a derivation error, not a new image" {
tmp="$(mktemp -d)"
make_tree "$tmp/base"
make_tree "$tmp/head" alpha
rc=0
"$SCRIPT" "$tmp/base" "$tmp/head" > "$tmp/out" 2>&1 || rc=$?
[ "$rc" -eq 1 ]
grep -q 'produced no names for the base tree' "$tmp/out"
if grep -q 'New image' "$tmp/out"; then echo "FAIL: blamed the PR for an empty base set"; false; fi
rm -rf "$tmp"
}
@test "the repository tree derives image names and passes against itself" {
"$SCRIPT" "$ROOT" "$ROOT"
}
@test "the derivation matches the e2e-fork.yaml push allowlist step" {
allow="$(yq '.jobs.publish.steps[] | select(.name == "Derive trusted image allowlist (base tree)").run' "$FORK_WF")"
[ -n "$allow" ]
for frag in \
"ls-files 'packages/*/*/Makefile'" \
"grep -qE '^image:'" \
"OCI_EXPORT_DIR=/oci REGISTRY=r COZYSTACK_VERSION=0 2>/dev/null || true" \
"grep -oE '(dest=/oci/|oci-archive:/oci/)[a-z0-9._-]+\.oci\.tar'" \
"sed -E 's#.*/##; s#\.oci\.tar\$##' | sort -u"; do
grep -qF -- "$frag" "$SCRIPT" || { echo "fragment missing from the script: $frag"; false; }
printf '%s\n' "$allow" | grep -qF -- "$frag" || { echo "fragment missing from e2e-fork.yaml: $frag"; false; }
done
}
@test "plan runs the check for fork PRs only" {
step="$(yq '.jobs.plan.steps[] | select(.run // "" | test("hack/fork-new-images\.sh"))' "$PR_WF")"
[ -n "$step" ]
[ "$(printf '%s\n' "$step" | yq '.if')" = '${{ github.event.pull_request.head.repo.fork }}' ]
}
+38
View File
@@ -0,0 +1,38 @@
#!/usr/bin/env bash
# Usage: hack/fork-new-images.sh BASE_TREE HEAD_TREE
#
# Fails, naming them, when HEAD_TREE exports image archives that BASE_TREE does
# not. e2e-fork.yaml pushes a fork PR's archives only under names the base tree
# exports, so such a PR would otherwise build everything and then be refused at
# the push, with nothing the contributor can do about it.
#
# image_names mirrors the "Derive trusted image allowlist" step of
# e2e-fork.yaml. That step cannot call this script, because it runs from the
# default branch over an untrusted checkout; hack/fork-new-images.bats keeps the
# two derivations in step.
set -eu
[ "$#" -eq 2 ] || { echo "usage: $0 BASE_TREE HEAD_TREE" >&2; exit 2; }
image_names() {
git -C "$1" ls-files 'packages/*/*/Makefile' | while IFS= read -r mk; do
grep -qE '^image:' "$1/$mk" || continue
make -n -C "$1/$(dirname "$mk")" image \
OCI_EXPORT_DIR=/oci REGISTRY=r COZYSTACK_VERSION=0 2>/dev/null || true
done \
| grep -oE '(dest=/oci/|oci-archive:/oci/)[a-z0-9._-]+\.oci\.tar' \
| sed -E 's#.*/##; s#\.oci\.tar$##' | sort -u
}
base="$(image_names "$1")"
head="$(image_names "$2")"
# An empty base set would report every head image as new and blame the PR for
# a derivation that broke on its own.
[ -n "$base" ] || { echo "::error::image name derivation produced no names for the base tree" >&2; exit 1; }
new="$(comm -13 <(printf '%s\n' "$base") <(printf '%s\n' "$head"))"
[ -z "$new" ] && exit 0
names="$(printf '%s' "$new" | tr '\n' ' ')"
echo "::error title=New image in a fork PR::This PR adds images the base branch does not build: ${names% }. The fork e2e lane pushes only images whose names the base branch already exports, so it would refuse these after the whole build. A maintainer has to land the new image on the base branch first, or the change has to run from a branch in the upstream repository. See docs/agents/e2e-testing.md, section 10."
exit 1