Repository navigation
feat(release): release on merge to the trunk, not on a tag - #11
Conversation
Implements the approved publishing policy (.github-config#17) in the shared workflows: - release-check runs on the trunk only, reads the version from the merged manifest, and reports release=false when that version already has a GitHub release, so a push that keeps the version publishes nothing. The new manifest.py release command refuses prerelease versions, a Helm chart that disagrees with the manifest, and git dependencies, so stable pins released versions. - publish creates the tag and the GitHub release last, on the released commit, with a token from the release GitHub App, the only actor the release-tags-create ruleset lets create a v* tag. - crates, docker and publish run in the release environment, which a bbot-leads member approves. crates.io uses trusted publishing and skips a version already published. docker drops the dev tag; dev images go to GHCR. - The tag subcommand and release.sh, which cut tags by hand, are gone.
pjhartlieb
left a comment
pjhartlieb
left a comment
There was a problem hiding this comment.
Review: release on merge to the trunk
@singlerider, I reviewed this at 4894c4ad against main (0f515e02, which includes #8), checked against the step 4 spec on .github-config#16 (A1–A4 and the names table) and the publishing policy in .github-config#17. In a clone of the PR head, the 16 tests pass and actionlint is clean. I also ran manifest.py release against some hand-made manifests, checked the two new action pins against their tags, and read the seven caller PRs at their current heads, but only for how they call these workflows.
There's a lot here that's right, and some of it is stronger than the spec:
environment: releaseis hard-coded indocker.yml,crates.ymland bothpublish.ymljobs, where the spec sketched it as an input. That's the better choice. A caller can't passenvironment: ''or some environment with no gate. Any job that reads a publish credential has to clearrelease's deployment-branch policy and reviewers, whatever workflow called it, and whatever branch it called from.- The repo token drops to
contents: readin both publish jobs. Everygithub.ref_nameis gone. The tag and the release are created together, onGITHUB_SHA, by the App token, and only after agh release viewcheck, so a re-run is a no-op. SBOMs and image SBOMs upload with--clobberunder the same token. - The App token is minted with no
owner/repositories, so it's scoped to the current repository, withpermission-contents: writeonly. crates.ymlis done the way A4 asks: OIDC throughcrates-io-auth-actionpinned to the v1.0.5 commit,id-token: writeon the job, an existence check against the crates.io API, and noCARGO_REGISTRY_TOKENfallback left.- Shared code is still checked out at
job.workflow_sha. Inputs reachrun:only throughenv:, never through${{ }}interpolation. Docker publisheslatest/stable/X.Y/Xunconditionally, and has nodevtag. manifest.py releaserefuses prereleases, chart mismatches and the common git-dependency forms, and has tests for each. Deletingrelease.shfully undoes #8, which changed nothing else. Org code search finds no other callers of the removed pieces.
The blocking issue is how the App ID and key get into the workflow. The rest are smaller.
High
H1. With the App ID and key passed in from the caller, the key ends up at repo or org level, or never arrives.
publish.yml:5-8 takes release-app-id as an input, and :45-47 takes release-app-key as a workflow_call secret. Every caller PR passes them from the calling job: release-app-id: ${{ vars.RELEASE_APP_ID }} and release-app-key: ${{ secrets.RELEASE_APP_KEY }} (for example bbot#3355 publish.yml:63,68). That calling job has no environment, and it can't have one. So:
vars.RELEASE_APP_IDthere reads repo and org variables only. Thereleaseenvironment's variable isn't visible to it.- For secrets, GitHub's documented route is that the reusable workflow's job sets
environment:, and the secret is passed under the same name.release-app-keycan't be that name, because secret names can't contain hyphens. Whether a secret renamed on the way in is resolved again inside the environment isn't documented, and I couldn't test it: noreleaseenvironment exists yet. If it isn't, the key resolves to empty, andcreate-github-app-tokenfails in the last job, after Docker and PyPI have already published. The only reliable way to make the current wiring work is to storeRELEASE_APP_KEYas a repo or org secret. The policy rules that out: every branch,devincluded, can read a repo secret. - Independently of that, the names differ from the ones the spec fixed and that the cut-over plan and Terraform are built on:
BLS_RELEASE_APP_ID(an environment variable set by Terraform in .github-config#1/#2) andBLS_RELEASE_PRIVATE_KEY(an environment secret, set by an owner at cut-over).
This isn't an equivalent design. Keeping it would mean changing the Terraform variable and the owners' cut-over steps, and would still leave the key-location problem.
Requested:
- Drop the
release-app-idinput and therelease-app-keysecret. - In both jobs that mint a token, read
app-id: ${{ vars.BLS_RELEASE_APP_ID }}andprivate-key: ${{ secrets.BLS_RELEASE_PRIVATE_KEY }}insideenvironment: release. DeclareBLS_RELEASE_PRIVATE_KEYunderworkflow_call.secretswithrequired: false, so a caller can't be forced to supply it. - Callers pass nothing for the App: no input, no
secrets:entry, and notsecrets: inherit, which would hand the shared workflow every repo-level publish token too. Then update the seven caller PRs to match. Terraform and the cut-over tasks stay as specified. - Prove it once on a test repo before any cut-over: a
releaseenvironment holding the variable and secret, a caller that passes nothing, and a run that mints a token. GitHub documents that an environment secret is used in a reusable workflow's environment job, but not explicitly for a secret the caller didn't pass. - Check that both values are non-empty, or mint the token, in the first
releasejob of the run, so a missing key fails before anything is published.
Medium
M1. release no longer enforces MAJOR.MINOR.PATCH.
cmd_release (scripts/manifest.py, the parsed.is_prerelease or parsed.is_devrelease or str(parsed) != declared check) rejects pre- and dev releases, but accepts 1.2, 1.2.3.4, 1.2.3.post1, 1!1.2.3 and 1.2.3+local. I ran each one: all exit 0 and emit tag=v1!1.2.3, tag=v1.2.3+local and so on. The old TAG regex refused all of these. Its error message still says MAJOR.MINOR.PATCH, and PyPI rejects local versions after Docker has already pushed.
Requested: require re.fullmatch(r"\d+\.\d+\.\d+", declared) (with parsed.release having three parts, or both), and add these five to test_release_refuses_a_prerelease_version or a sibling test.
M2. "Already released" is keyed on the GitHub release, so a version can be published from two commits.
release-check.yml:67-77 sets release=false only when gh release view succeeds. Any other outcome counts as new: no release, a deleted release, or an API error. That leaves two gaps:
- A run that pushed Docker and PyPI and then failed before the release job is supposed to be re-run on the same commit. If another PR merges to
stablefirst with the version unchanged, the new run sees no release, re-pushesvXDocker tags built from different code, skips PyPI (skip-existing), and tags the new commit. PyPI'sXand Docker'sXnow come from different commits. - If someone deletes a release (the tag survives under the immutable ruleset), the next merge does the same thing.
gh release create --targetis then ignored, because the tag already exists.
On the spec's "fail: bump the version" versus this PR's no-op: I'd keep the no-op. Merges to stable that aren't releases are expected, such as the revert/re-merge steps in the hotfix flow (D7), and a red publish run on each of them would train people to ignore red. But the no-op should rest on the immutable record, and a half-finished release shouldn't be silently published over.
Requested: decide "already released" from the tag (gh api repos/$GITHUB_REPOSITORY/git/ref/tags/$TAG), not from the release, and treat an API error as a failure rather than as "new". When the tag is absent but the version already exists on a registry (PyPI JSON API, crates.io, or the vX Docker tag), fail with "vX is partly published from another run; re-run that run or bump the version". A re-run of failed jobs reuses check's outputs and doesn't re-run check, so this doesn't block the legitimate re-run.
M3. The release branch is the default branch, not stable.
release-check.yml:40-45 compares github.ref to github.event.repository.default_branch. A1 and policy rule 2 say stable. For six of the seven repos that's the same today, but webcap's default branch is master, and webcap has no stable branch (webcap#132 triggers on master). The release environment that Terraform will create allows stable only, so webcap's release job would be refused at the environment. The check also moves if a repo admin changes the default branch, which is a settings change nobody reviews. The environment still holds in that case, but this check is meant to be the early, readable failure.
Requested: compare against refs/heads/stable, and raise webcap's master → stable rename as a cut-over item on .github-config#16. If you'd rather support a per-repo trunk, that's a policy change, and it goes in .github-config#17 with the Terraform deployment policy changed to match.
M4. The description says dev images go to GHCR, but nothing in this PR pushes to GHCR.
docker.yml only logs in to Docker Hub (:66-69). Nothing requests packages: write, and there's no ghcr.io reference in the PR or in any of the seven caller PRs' workflows. Dropping the dev tag is correct. The GHCR half of the dev channel (policy D8, step 7) isn't done anywhere yet.
Requested: correct the description. Then either add a GHCR path, a separate job or workflow that runs on dev with the repo token and packages: write and no release environment (no Docker Hub credentials involved), or say explicitly that bbot will build it itself, as spec B has it.
Low
- L1. Git dependency forms that slip through.
git_dependenciestests"git" in sourcefortool.uv.sources. A list of sources (uv's multi-source form with markers) is a list, so the check never sees thegitkey. Cargo's[target.'cfg(...)'.dependencies]and git deps declared in[workspace.dependencies]and pulled in with{ workspace = true }are not read either. I ran all three and each exits 0. Non-git direct URLs (pkg @ https://…/archive/dev.zip) also pass. Worth covering, since the check is meant to be thestableguard the policy promises. - L2. The outputs changed. The spec asked to keep
prereleaseandsemver. This drops them and addstagandrelease. With no prereleases left, that's reasonable, and every caller PR already uses the new outputs. Please state it in the description as an interface change, so nobody bumps a pin without reading it. - L3. Approvals per release.
docker.ymlbuild and description,crates.yml, the callers'pypijob, andpublish.ymlrelease and image-sbom each setenvironment: release, and they run one after another. As I understand GitHub's behavior, each job that reaches a protected environment waits for its own review, so one release could take three or four approvals frombbot-leads. I haven't been able to verify this without an environment. Please check on the first cut-over. If it's true, merging jobs (description into build, image-sbom into release) would cut the count. - L4.
crates.ymlpicks the first package.cargo metadata --no-deps | jq '.packages[0].name'is fine for a single crate. In a workspace it checks whichever member comes first. Either read the name fromworking-directory's ownCargo.toml, or document that this is single-crate only.
Noted, outside this PR
bbot#3355 docs/dev/releasing.md still describes tags pushed by people, release.sh and -rc.N from dev. That review covers it; I'm noting it here because this PR deletes the tool those docs point to.
Coverage against the spec
| Item | Status |
|---|---|
| A1 release-check | Done differently: trunk = default branch, not stable (M3); version from the manifest; already-released is a no-op, not a failure (M2: keep the no-op, but key it on the tag); outputs changed (L2) |
| A2 publish.yml | Done, except the App inputs and names (H1) |
| A3 docker.yml | Done, with the environment hard-coded; the GHCR claim isn't implemented (M4) |
| A4 crates.yml | Done |
Verdict
Request changes: H1 before any repo cuts over, because it decides where the App key lives. M1 to M3 in this PR as well, each a few lines, and M4 either fixed or written up in the description. The lows are optional. Once H1 and the Mediums are in, I'll approve, and the seven caller PRs can then re-pin to that commit.
Next: we're making these changes ourselves in a follow-up PR, which will be linked here when it opens. This review is the record of what that PR addresses.
Read the release App ID and key from the release environment (vars.BLS_RELEASE_APP_ID, secrets.BLS_RELEASE_PRIVATE_KEY) instead of caller inputs, and mint a token in release-check before anything publishes so a missing key fails first. - release-check: release from stable only; decide "already released" from the v<version> tag; fail on an API error or a version already on PyPI, crates.io or Docker Hub with no tag - manifest.py: require exact MAJOR.MINOR.PATCH; catch uv multi-source and url sources, PEP 508 direct URLs, Cargo target and workspace git deps; emit pypi_name and crate_name - crates.yml: pick the package whose manifest is in working-directory - docker.yml: declare Docker secrets so callers pass none; push the Hub description from the build job - docker-dev.yml: push dev images to GHCR with the repository token, outside the release environment - publish.yml: build image SBOMs outside the environment, attach them in the single release job
Callers can now pass CODECOV_TOKEN, BBOT_IO_API_KEY and the CLA App credentials by name instead of secrets: inherit, which handed every repository secret, publish tokens included, to test and CLA runs.
Runs manifest.py release before merge, so a promotion carrying a prerelease version, a chart mismatch or a git dependency fails review instead of the release run.
pjhartlieb
left a comment
pjhartlieb
left a comment
There was a problem hiding this comment.
Re-reviewed at 07e14bcb. Everything from the first review is fixed: H1, M1–M4, L1, L3, L4.
Verify: bash verify-cla11.sh 07e14bcbba5e737e1b29bd9e3f9128fbf2483be9 ./actionlint gives 23 PASS, 0 FAIL (pytest 29 passed, actionlint 1.7.12 clean). As a negative control, the same script at 4894c4ad gives 17 FAIL, which are the first review's findings.
verify-cla11.sh
#!/usr/bin/env bash
# Reproducible check of CLA#11 at a given head against our 2026-10-21 review (H1, M1-M4, L1-L4).
# Clean clone at the SHA, tests via uv, manifest probes, static checks of the workflow files, actionlint
# (pinned, checksum-verified). Changes nothing outside a temp dir and nothing on GitHub.
# Negative control: run it at the reviewed head 4894c4ad; the M1/L1/H1/M3 checks must FAIL there.
# Usage: bash verify-cla11.sh <sha> [actionlint-binary]
set -uo pipefail
SHA=$1; AL=${2:-}
W=$(mktemp -d "${TMPDIR:-/tmp}/cla11.XXXXXX"); trap 'rm -rf "$W"' EXIT
git clone -q https://github.com/blacklanternsecurity/CLA "$W/CLA" && git -C "$W/CLA" fetch -q origin "$SHA" && git -C "$W/CLA" -c advice.detachedHead=false checkout -q "$SHA" || { echo "clone failed"; exit 2; }
cd "$W/CLA"; P=0; F=0
ok() { echo "PASS $1"; P=$((P+1)); }; no() { echo "FAIL $1"; F=$((F+1)); }
echo "head $(git rev-parse HEAD)"
if uv run -q --group dev pytest -q tests >"$W/pytest.log" 2>&1; then ok "pytest ($(tail -1 "$W/pytest.log"))"; else no "pytest ($(tail -1 "$W/pytest.log"))"; fi
probe() { # dir expect(refuse|accept) label
local out; out=$(uv run -q --no-project --with packaging --with pyyaml python scripts/manifest.py --root "$1" release 2>&1); local rc=$?
if { [ "$2" = refuse ] && [ $rc -ne 0 ]; } || { [ "$2" = accept ] && [ $rc -eq 0 ]; }; then ok "$3"; else no "$3 (rc=$rc: ${out##*$'\n'})"; fi; }
mk() { local d="$W/p/$1"; mkdir -p "$d"; git -C "$d" init -q; printf '%s\n' "$2" > "$d/pyproject.toml"; [ -z "${3:-}" ] || printf '%s\n' "$3" > "$d/Cargo.toml"; echo "$d"; }
for v in 1.2 1.2.3.4 1.2.3.post1 '1!1.2.3' 1.2.3+local 01.2.3 1.2.3rc1; do
probe "$(mk "v$v" "[project]
name = \"x\"
version = \"$v\"")" refuse "M1 refuses version $v"; done
probe "$(mk ok '[project]
name = "x"
version = "1.2.3"')" accept "M1 accepts 1.2.3 (control)"
probe "$(mk l1a '[project]
name = "x"
version = "1.2.3"
dependencies = ["y"]
[tool.uv.sources]
y = [{ git = "https://example.invalid/y", marker = "sys_platform == '"'linux'"'" }]')" refuse "L1 uv.sources list form with git"
probe "$(mk l1b '[project]
name = "x"
version = "1.2.3"
dependencies = ["y @ https://example.invalid/y/archive/dev.zip"]')" refuse "L1 direct URL dependency"
probe "$(mk l1c '[project]
name = "x"
version = "1.2.3"' '[package]
name = "x"
version = "1.2.3"
[target.'"'cfg(unix)'"'.dependencies]
y = { git = "https://example.invalid/y" }')" refuse "L1 Cargo target.*.dependencies git"
probe "$(mk l1d '[project]
name = "x"
version = "1.2.3"' '[package]
name = "x"
version = "1.2.3"
[workspace.dependencies]
y = { git = "https://example.invalid/y" }')" refuse "L1 Cargo workspace.dependencies git"
WF=.github/workflows
grep -q 'release-app-id\|release-app-key' $WF/publish.yml && no "H1 caller-passed App input/secret removed" || ok "H1 caller-passed App input/secret removed"
grep -q 'vars.BLS_RELEASE_APP_ID' $WF/publish.yml && grep -q 'secrets.BLS_RELEASE_PRIVATE_KEY' $WF/publish.yml && ok "H1 publish.yml reads BLS_RELEASE_* from the environment" || no "H1 publish.yml reads BLS_RELEASE_* from the environment"
grep -q 'create-github-app-token' $WF/release-check.yml && ok "H1 token minted in the first release job (fail fast)" || no "H1 token minted in the first release job (fail fast)"
grep -q "github.ref != 'refs/heads/stable'" $WF/release-check.yml && ok "M3 release branch is refs/heads/stable" || no "M3 release branch is refs/heads/stable"
grep -q 'git/ref/tags/' $WF/release-check.yml && ok "M2 already-released keyed on the tag" || no "M2 already-released keyed on the tag"
grep -q 'partly published' $WF/release-check.yml && ok "M2 partial publish fails" || no "M2 partial publish fails"
[ -f $WF/docker-dev.yml ] && grep -q 'ghcr.io' $WF/docker-dev.yml && ok "M4 GHCR dev path exists" || no "M4 GHCR dev path exists"
grep -q 'manifest_path' $WF/crates.yml && ok "L4 crates.yml picks the working-directory crate" || no "L4 crates.yml picks the working-directory crate"
n=$(grep -l 'environment: release' $WF/*.yml | wc -l | tr -d ' '); echo "INFO files with environment: release: $n"
grep -rq 'secrets: inherit' $WF && no "no secrets: inherit in shared workflows" || ok "no secrets: inherit in shared workflows"
if [ -n "$AL" ]; then if "$AL" -no-color >"$W/al.log" 2>&1; then ok "actionlint clean"; else no "actionlint ($(wc -l <"$W/al.log" | tr -d ' ') lines)"; sed 's/^/ /' "$W/al.log" | head -20; fi; fi
echo "SUMMARY $P PASS, $F FAIL"
[ $F -eq 0 ]Before merge:
- Conflicts with
maininpublish.ymlandrelease-check.yml(#10 bumped the pins). After resolving them, the seven callers need to re-pin to the merge commit onmain, not07e14bcb. - Release-time provenance check (policy, cloudcheck exception): every commit since the last
v*tag has to come from a merged PR, except paths in adirect-push-pathsinput. cloudcheck needs this before it cuts over, and for the other repos it costs nothing. Our version is in #12,31d7219(scripts/provenance.pyplus arelease-check.ymlstep); take whatever is useful from it.
Suggested: an environment guard in check. It fails if release lets admins bypass it, or isn't limited to stable. GitHub auto-creates a missing environment with no protection, and the guard catches that before anything publishes. It's in #12, 6fc2cf5, and needs actions: read on the calling job.
I'll close #12 once these are in.
# Conflicts: # .github/workflows/publish.yml # .github/workflows/release-check.yml
A job that names a missing environment makes GitHub create it with no protection. Before any job enters release, the check job now reads the environment and fails unless it exists, admins can't bypass it, deployments are limited to stable only, and, on public repositories, it has required reviewers with prevent self-review. Anything unreadable fails the run. The check job needs actions: read; callers must grant it. From pjhartlieb's 6fc2cf5 on #12.
Before a release, check each commit since the last v* tag against the pull requests it belongs to (GitHub's commit-to-PR association), not its author, which anyone can set. A commit that didn't arrive through a merged pull request may touch only the paths in the new direct-push-paths input (cloudcheck: its daily data files). With no paths given, the default, any such commit fails the release. A first release (no earlier v* tag) is skipped with a notice. This is the release-time backstop the publishing policy requires for the cloudcheck-update-bot exception. The check job needs pull-requests: read; callers must grant it. From pjhartlieb's 31d7219 on #12. Refs blacklanternsecurity/.github-config#16, blacklanternsecurity/bbot-enterprise#406
|
@pjhartlieb addressed at 812db99:
Validation: Callers re-pin to the merge commit on |
pjhartlieb
left a comment
pjhartlieb
left a comment
There was a problem hiding this comment.
Re-reviewed at 812db99a. The conflicts are resolved, the provenance check and the environment guard are in, and nothing from earlier regressed.
Verify: verify-cla11.sh 812db99a… ./actionlint gives 23 PASS, 0 FAIL (37 tests). verify-cla11-r3b.sh 812db99a… gives 7 PASS, 1 FAIL (F1). The control at 07e14bcb gives 8 FAIL.
F1 (Medium). This is my code from #12. provenance.py accepts a commit if any merged PR is associated with it, whatever that PR's base. For example, gh api repos/blacklanternsecurity/cloudcheck/commits/eb8b813ede280e4cae51ae6bbd19652196a50522/pulls returns a commit on stable whose only merged PR is into dev. So if a bypass actor pushes dev commits straight to stable, they pass without a stable PR. That actor could be the cloudcheck bot, or an admin.
Fix: walk git rev-list --first-parent <tag>..HEAD. Accept a commit only if it's associated with a merged PR whose base.ref is stable, or if it touches only direct-push-paths. Commits off the first-parent line came in through that PR. On cloudcheck, v11.2.0..stable (11 commits) is accepted under this rule: PR merges into stable, plus data-only bot commits.
Low. previous_tag returns None on any git describe error, which skips the check as if this were the first release. It should skip only when there are no v* tags at all.
After merge: callers re-pin to the merge commit on main, and give the check job contents, actions and pull-requests: read. asndb#39 already does.
I'll close #12 once F1 is settled.
verify-cla11-r3b.sh
#!/usr/bin/env bash
# Reproducible check of CLA#11's release-time provenance check (R3b) and environment guard, plus the
# base-branch gap in provenance.py. Clean clone at the SHA; GitHub reads are GET only.
# Negative control: at 07e14bcb (before R3b) the R3b and guard checks must FAIL.
# Expected at 812db99a: R3b/guard PASS; "F1" FAILs (the finding), its evidence line shows a commit on
# cloudcheck stable whose only merged PR has base=dev, which provenance.py accepts.
# Usage: bash verify-cla11-r3b.sh <sha>
set -uo pipefail
SHA=$1
W=$(mktemp -d "${TMPDIR:-/tmp}/cla11r3b.XXXXXX"); trap 'rm -rf "$W"' EXIT
git clone -q https://github.com/blacklanternsecurity/CLA "$W/CLA" && git -C "$W/CLA" fetch -q origin "$SHA" && git -C "$W/CLA" -c advice.detachedHead=false checkout -q "$SHA" || { echo "clone failed"; exit 2; }
cd "$W/CLA"; P=0; F=0
ok() { echo "PASS $1"; P=$((P+1)); }; no() { echo "FAIL $1"; F=$((F+1)); }
echo "head $(git rev-parse HEAD)"
RC=.github/workflows/release-check.yml
[ -f scripts/provenance.py ] && ok "R3b scripts/provenance.py present" || no "R3b scripts/provenance.py present"
grep -q 'direct-push-paths:' $RC && ok "R3b direct-push-paths input" || no "R3b direct-push-paths input"
grep -q 'provenance.py' $RC && ok "R3b release-check runs provenance.py" || no "R3b release-check runs provenance.py"
grep -q 'fetch-depth: 0' $RC && ok "R3b history fetched (fetch-depth 0)" || no "R3b history fetched (fetch-depth 0)"
if [ -f tests/test_provenance.py ] && uv run -q --group dev pytest -q tests/test_provenance.py >"$W/p.log" 2>&1; then ok "R3b tests ($(tail -1 "$W/p.log"))"; else no "R3b tests"; fi
grep -q 'environments/release' $RC && grep -q 'can_admins_bypass' $RC && grep -q '"branch:stable"' $RC && ok "guard: env readable, no admin bypass, stable only" || no "guard: env readable, no admin bypass, stable only"
awk '/^ check:/{f=1;next} f&&/^ [a-z-]+:$/{f=0} f' $RC | grep -q 'actions: read' && ok "guard: check job declares actions: read" || no "guard: check job declares actions: read"
# F1: does the rule tie a merged PR to the release branch (base ref, or a first-parent walk)?
if [ -f scripts/provenance.py ] && grep -qE "first-parent|base\"?\]?\.?\[?\"?ref|\[\"base\"\]" scripts/provenance.py; then ok "F1 provenance ties the PR to the release branch"; else no "F1 provenance ties the PR to the release branch (accepts a PR merged into any branch)"; fi
# Evidence for F1 (GET): a commit on cloudcheck stable whose associated merged PR has base dev.
gh api --method GET repos/blacklanternsecurity/cloudcheck/commits/eb8b813ede280e4cae51ae6bbd19652196a50522/pulls \
--jq '"INFO F1 evidence: cloudcheck eb8b813e (on stable) -> " + ([.[]|"#\(.number) base=\(.base.ref) merged=\(.merged_at!=null)"]|join(" "))'
echo "SUMMARY $P PASS, $F FAIL"
[ $F -eq 0 ]…ents A commit associated with any merged pull request passed, whatever that pull request's base. A dev commit pushed straight to stable by a bypass actor therefore passed without a stable pull request (pjhartlieb, F1). Accept a commit only when a merged pull request into --base (default stable) carries it, and walk only the first-parent line, since commits off it arrived through the merge that brought them. previous_tag skipped the check on any git describe error, as if this were a first release. It now skips only when no release tag exists, and an unreachable tag fails the job.
pjhartlieb
left a comment
pjhartlieb
left a comment
There was a problem hiding this comment.
Re-reviewed at 9c75e85c. F1 and the Low are fixed.
Verify: verify-cla11.sh 9c75e85c… ./actionlint gives 23 PASS, 0 FAIL (41 tests, actionlint clean). The new verify-cla11-r4.sh 9c75e85c… gives 10 PASS, 0 FAIL. It runs main() on test histories with a stubbed gh, and on cloudcheck stable. On cloudcheck, all 12 first-parent commits since v11.2.0 pass; with --base dev, it fails. The control at 812db99a fails F1 and the Low.
Nit, not blocking. test_only_the_first_parent_line_is_walked runs git rev-list directly, not provenance.py. If --first-parent is dropped from main(), all 41 tests still pass. Moving the rev-list call into a function and testing that would pin it.
Before merge: update the branch. main now has #13 and requires python / passed, actionlint, and branches up to date. The merge is clean and keeps #13's change in cla-reusable.yml.
verify-cla11-r4.sh
#!/usr/bin/env bash
# Reproducible check of CLA#11's F1 fix (provenance.py: PR must be merged into the release branch; first-parent
# walk) and the review-3 Low (describe error must not skip). Behavioural: runs provenance.py's main() against
# synthetic histories with a stub `gh`, and against real cloudcheck history with the real `gh` (GET only).
# Negative controls: the same synthetic F1 case run with OLD's provenance.py must be ACCEPTED (proves the case
# reproduces F1); real cloudcheck with --base dev must FAIL (proves the base check can reject).
# Usage: bash verify-cla11-r4.sh <full-head-sha> [old-full-sha] [actionlint-binary]
set -uo pipefail
SHA=$1; OLD=${2:-812db99a39f3244b7d6c50453f0299cfc07d0830}; AL=${3:-}
W=$(mktemp -d "${TMPDIR:-/tmp}/cla11r4.XXXXXX"); trap 'rm -rf "$W"' EXIT
git clone -q https://github.com/blacklanternsecurity/CLA "$W/CLA" && git -C "$W/CLA" fetch -q origin "$SHA" "$OLD" \
&& git -C "$W/CLA" -c advice.detachedHead=false checkout -q "$SHA" || { echo "clone failed"; exit 2; }
git -C "$W/CLA" show "$OLD:scripts/provenance.py" > "$W/provenance-old.py" || { echo "old provenance.py missing"; exit 2; }
NEW="$W/CLA/scripts/provenance.py"; P=0; F=0
ok() { echo "PASS $1"; P=$((P+1)); }; no() { echo "FAIL $1"; F=$((F+1)); }
echo "head $(git -C "$W/CLA" rev-parse HEAD) old $OLD"
cd "$W/CLA"
if uv run -q --group dev pytest -q tests >"$W/pytest.log" 2>&1; then ok "pytest ($(tail -1 "$W/pytest.log"))"; else no "pytest ($(tail -1 "$W/pytest.log"))"; fi
cd "$W"
# Stub gh: prints $STUB_JSON for any call (provenance.py only calls commits/{sha}/pulls).
mkdir -p "$W/stub"; printf '#!/bin/sh\nprintf "%%s" "$STUB_JSON"\n' > "$W/stub/gh"; chmod +x "$W/stub/gh"
G() { git -C "$1" -c user.name=t -c user.email=t@t "${@:2}"; }
repo() { local d="$W/r/$1"; mkdir -p "$d"; git -C "$d" init -q -b stable; echo 1 > "$d/a"; G "$d" add -A; G "$d" commit -qm initial; echo "$d"; }
run() { # script repo stub_json [args...] -> exit code
local s=$1 d=$2 j=$3; shift 3
STUB_JSON=$j PATH="$W/stub:$PATH" python3 "$s" --root "$d" --repo o/r "$@" >"$W/last.log" 2>&1; echo $?; }
DEV='[{"merged_at":"t","base":{"ref":"dev"}}]'; STB='[{"merged_at":"t","base":{"ref":"stable"}}]'
# F1 case: a direct commit to stable whose only associated merged PR is into dev.
d=$(repo f1); G "$d" tag v1.0.0; mkdir -p "$d/src"; echo x > "$d/src/x.py"; G "$d" add -A; G "$d" commit -qm "dev work pushed to stable"
[ "$(run "$NEW" "$d" "$DEV")" != 0 ] && ok "F1 new: commit whose PR base=dev is rejected" || no "F1 new: commit whose PR base=dev is rejected ($(tail -1 "$W/last.log"))"
[ "$(run "$NEW" "$d" "$STB")" = 0 ] && ok "F1 new: same commit with PR base=stable is accepted (control)" || no "F1 new: base=stable accepted ($(tail -1 "$W/last.log"))"
[ "$(run "$W/provenance-old.py" "$d" "$DEV")" = 0 ] && ok "F1 control: OLD accepts the base=dev commit (case reproduces F1)" || no "F1 control: OLD accepts base=dev ($(tail -1 "$W/last.log"))"
# Bypass merge: dev merged into stable locally and pushed (no PR) must fail; data-only direct push must pass.
d=$(repo merge); G "$d" tag v1.0.0; G "$d" checkout -qb dev; mkdir -p "$d/src"; echo y > "$d/src/y.py"; G "$d" add -A; G "$d" commit -qm dev
G "$d" checkout -q stable; G "$d" merge -q --no-ff -m "merge dev" dev
[ "$(run "$NEW" "$d" '[]')" != 0 ] && ok "bypass merge commit without a PR is rejected" || no "bypass merge rejected ($(tail -1 "$W/last.log"))"
d=$(repo data); G "$d" tag v1.0.0; echo '{}' > "$d/cloud_providers_v3.json"; G "$d" add -A; G "$d" commit -qm data
[ "$(run "$NEW" "$d" '[]' --allow cloud_providers_v3.json README.md)" = 0 ] && ok "data-only direct push is accepted" || no "data-only accepted ($(tail -1 "$W/last.log"))"
# Low: tags exist but none reachable from HEAD must not be skipped as a first release.
d=$(repo low); G "$d" checkout -qb side; echo s > "$d/s"; G "$d" add -A; G "$d" commit -qm side; G "$d" tag v1.0.0; G "$d" checkout -q stable
rc=$(run "$NEW" "$d" "$STB"); if [ "$rc" != 0 ] && ! grep -q 'first release' "$W/last.log"; then ok "Low: unreachable tag fails instead of skipping"; else no "Low: unreachable tag (rc=$rc: $(tail -1 "$W/last.log"))"; fi
# Real history: cloudcheck stable since its last v* tag, real gh (GET).
git clone -q --filter=blob:none https://github.com/blacklanternsecurity/cloudcheck "$W/cc" 2>/dev/null && git -C "$W/cc" checkout -q stable
if python3 "$NEW" --root "$W/cc" --repo blacklanternsecurity/cloudcheck --allow cloud_providers_v3.json README.md >"$W/cc.log" 2>&1; then
ok "cloudcheck stable passes ($(grep -m1 '^checking' "$W/cc.log"))"; else no "cloudcheck stable ($(tail -2 "$W/cc.log" | tr '\n' ' '))"; fi
if python3 "$NEW" --root "$W/cc" --repo blacklanternsecurity/cloudcheck --base dev --allow cloud_providers_v3.json README.md >"$W/ccd.log" 2>&1; then
no "cloudcheck control: --base dev must fail"; else ok "cloudcheck control: --base dev fails ($(grep -c '::error::' "$W/ccd.log") rejected)"; fi
# Mutation probes: does the test suite pin each half of the fix?
mut() { # label sed-expr
rm -rf "$W/m"; cp -R "$W/CLA" "$W/m"; sed -i '' "$2" "$W/m/scripts/provenance.py"
if cmp -s "$NEW" "$W/m/scripts/provenance.py"; then echo "INFO mutation '$1' did not apply"; return; fi
if (cd "$W/m" && uv run -q --group dev pytest -q tests >/dev/null 2>&1); then echo "INFO mutation '$1' SURVIVES the test suite"; else echo "INFO mutation '$1' killed by the test suite"; fi; }
mut "drop --first-parent in main()" 's/"rev-list", "--first-parent", /"rev-list", /'
mut "ignore PR base in merged_into" 's/ and pr.get("base", {}).get("ref") == base//'
if [ -n "$AL" ]; then if (cd "$W/CLA" && "$AL" -no-color >"$W/al.log" 2>&1); then ok "actionlint clean"; else no "actionlint"; sed 's/^/ /' "$W/al.log" | head -20; fi; fi
echo "SUMMARY $P PASS, $F FAIL"
[ $F -eq 0 ]
en0f
left a comment
en0f
left a comment
There was a problem hiding this comment.
Approved. As was stated, just for record:
The branch is behind main by #13 (cla-reusable permissions fix) and #14 (CODEOWNERS for bbot-leads). Main requires branches to be up to date, so it needs an “Update branch.” The merge is clean. Updating will also bring in CODEOWNERS. If code-owner review is required, a bbot-leads approval will be needed after the update.
pjhartlieb
left a comment
pjhartlieb
left a comment
There was a problem hiding this comment.
Approving at ef3d473c. The update onto main is a clean merge: its tree is identical to 9c75e85c merged with aac1b8a8, so review 4's verification still applies.
Implements step 4 of the publishing policy hand-off (.github-config#16, policy in .github-config#17) in the shared workflows.
Change
release-check.yml: runs fromstableonly. Version from the merged manifest (manifest.py release), which accepts exact MAJOR.MINOR.PATCH only and refuses a Helm chart that disagrees and git or URL dependencies.release=falsewhen thev<version>tag exists. Fails when the tag lookup errors, or when the version is already on PyPI, crates.io or Docker Hub (imageinput) with no tag. Acredentialsjob in thereleaseenvironment mints the App token before anything publishes, so a missing key fails first.publish.yml:releaseenvironment. App ID and key are read from that environment (vars.BLS_RELEASE_APP_ID,secrets.BLS_RELEASE_PRIVATE_KEY). Callers pass nothing and neversecrets: inherit. Image SBOMs are built outside the environment and attached in the one release job, which creates the tag and release last, onGITHUB_SHA.crates.yml:releaseenvironment, trusted publishing (rust-lang/crates-io-auth-action), checks the crate whose manifest is inworking-directory, skips a version already on crates.io.docker.yml:releaseenvironment.DOCKER_USERNAME/DOCKER_TOKENare declared and read from the environment, so callers pass none. The Hub description is pushed from the build job. Nodevtag.docker-dev.yml(new): ondev, pushesghcr.io/<image>:devand:dev-<sha>with the repository token andpackages: write, outside thereleaseenvironment.stable-guard.yml(new): callers run it on pull requests intostable.release readyrunsmanifest.py release, so a version, chart or dependency that can't release fails review instead of the release run.cla-reusable.ymlandpython-tests.ymldeclare the secrets they read (APP_ID,APP_PRIVATE_KEY,CODECOV_TOKEN,BBOT_IO_API_KEY), so callers pass them by name instead ofsecrets: inherit.manifest.py tag/callerandscripts/release.sh, which cut tags by hand.Interface changes
release-check.ymloutputs:version,tag,major,minor,release.prereleaseandsemverare gone. Newimageinput.publish.yml:release-app-idinput andrelease-app-keysecret removed.secrets: inherit.Validation
Tag probe checked live:
gh api -i repos/blacklanternsecurity/bbot/git/ref/tags/v3.0.2.177rcis200, a missing tag is404. Docker Hubv2/repositories/<image>/tags/<version>returns200/404the same way. The environment-scoped secrets and the GHCR push have not run yet: noreleaseenvironment exists. Verify on the first cut-over.Callers move in bbot#3355, baddns#949, asndb#39, radixtarget#96, blastdns#139, blasthttp#120 and webcap#132, all pinned to 07e14bc.
Refs blacklanternsecurity/.github-config#16