From d8840f871b0e9fa304b784f02aac299536fbbbaf Mon Sep 17 00:00:00 2001 From: Anthony Ronning <101225832+AnthonyRonning@users.noreply.github.com> Date: Mon, 14 Sep 2026 18:03:12 +0000 Subject: [PATCH 1/2] Share one approval branch per source commit across environments Each OpenSecret EIF release run pushed its own opensecret/pcr-approval-- branch, so when one commit changed both measurement sets the two single-environment pull requests each failed the other environment's approval comparison (#918 and #919), and the approvals had to be combined by hand (#920). The publisher now targets opensecret/pcr-approval-: if the branch already exists it must descend from the source commit and change only approval files, the run refuses to repeat an environment that is already on it, and its commit is rebased onto the branch tip. A push rejected by the other environment's concurrent run is refetched, replayed and retried up to five times. Fixtures cover the sequential second environment, the repeated environment refusal, and a lost push race driven through a git shim; the runbook describes the shared branch. Co-Authored-By: Claude Fable 5.1 --- scripts/ci/publish_opensecret_approval.sh | 63 ++++++++++++++++---- scripts/ci/test_opensecret_workflows.py | 70 ++++++++++++++++++++--- services/opensecret/docs/nitro-deploy.md | 10 +++- 3 files changed, 122 insertions(+), 21 deletions(-) diff --git a/scripts/ci/publish_opensecret_approval.sh b/scripts/ci/publish_opensecret_approval.sh index f5784513b..fa0d0719b 100755 --- a/scripts/ci/publish_opensecret_approval.sh +++ b/scripts/ci/publish_opensecret_approval.sh @@ -1,6 +1,9 @@ #!/usr/bin/env bash -# Commit the approval files changed by the signing step to a review branch. -# Never writes master, creates a tag, or touches GitHub Releases. +# Commit the approval files changed by the signing step to the review branch +# for this source commit. Both environments' runs share that branch, so one +# pull request carries every approval for the commit; a later run lands its +# commit on top of the earlier one. Never writes master, creates a tag, or +# touches GitHub Releases. set -euo pipefail if [[ $# -ne 2 ]]; then @@ -43,29 +46,64 @@ while read -r _ path; do esac done <<< "$changed" -branch="opensecret/pcr-approval-${mode}-${source_sha:0:12}" +branch="opensecret/pcr-approval-${source_sha:0:12}" # Same mechanism actions/checkout uses; the token never appears in a URL. authorization=$(printf 'x-access-token:%s' "$GITHUB_TOKEN" | base64 | tr -d '\n') git_auth() { git -c "http.https://github.com/.extraheader=AUTHORIZATION: basic $authorization" "$@" } -if git_auth ls-remote --exit-code --heads origin "refs/heads/$branch" >/dev/null 2>&1; then - echo "Branch $branch already exists; merge or delete it before rerunning." >&2 - exit 1 -fi +git_bot() { + git -c user.name='github-actions[bot]' \ + -c user.email='41898282+github-actions[bot]@users.noreply.github.com' "$@" +} pcr0=$(jq -r .measurements.PCR0 "$handoff") eif_sha256=$(jq -r .eif_sha256 "$handoff") git add -- "$snapshot" "$history" -git -c user.name='github-actions[bot]' \ - -c user.email='41898282+github-actions[bot]@users.noreply.github.com' \ - commit --quiet \ +git_bot commit --quiet \ -m "Approve ${mode} OpenSecret EIF measurements from ${source_sha:0:12}" \ -m "Source commit: ${source_sha}" \ -m "PCR0: ${pcr0}" \ -m "EIF SHA-256: ${eif_sha256}" \ -m "Signed by the OpenSecret EIF release workflow run ${GITHUB_RUN_ID:-local}." -git_auth push --quiet origin "HEAD:refs/heads/$branch" + +# Land this commit on top of whatever the branch already holds for this source +# commit. A push loses the race only to the other environment's run, which +# touches different files, so refetch and replay once and try again. +pushed=0 +for attempt in 1 2 3 4 5; do + if git_auth fetch --quiet origin "refs/heads/$branch:refs/remotes/origin/$branch" 2>/dev/null; then + tip=$(git rev-parse "refs/remotes/origin/$branch") + git merge-base --is-ancestor "$source_sha" "$tip" || { + echo "Branch $branch does not descend from $source_sha; inspect it before rerunning." >&2 + exit 1 + } + while read -r path; do + [[ -n "$path" ]] || continue + case "$path" in + "$snapshot" | "$history") + echo "Branch $branch already carries the $mode approval; merge or delete it before rerunning." >&2 + exit 1 ;; + services/opensecret/pcrDev.json | services/opensecret/pcrDevHistory.json | \ + services/opensecret/pcrProd.json | services/opensecret/pcrProdHistory.json) ;; + *) echo "Branch $branch changes more than approval files: $path" >&2; exit 1 ;; + esac + done <<< "$(git diff --name-only "$source_sha" "$tip")" + if [[ "$(git rev-parse HEAD~1)" != "$tip" ]]; then + git_bot rebase --quiet "$tip" >/dev/null + fi + fi + if git_auth push --quiet origin "HEAD:refs/heads/$branch" 2>/dev/null; then + pushed=1 + break + fi + echo "Push of $branch was rejected (attempt $attempt); refetching." >&2 + sleep 2 +done +[[ "$pushed" == 1 ]] || { + echo "Could not push $branch after five attempts; the approval commit is at $(git rev-parse HEAD) in this checkout." >&2 + exit 1 +} compare="/${GITHUB_REPOSITORY}/compare/master...${branch}?expand=1" if [[ -n "${GITHUB_STEP_SUMMARY:-}" ]]; then @@ -74,6 +112,9 @@ if [[ -n "${GITHUB_STEP_SUMMARY:-}" ]]; then echo echo "Open the pull request: $compare" echo + echo "The branch is shared by every environment approved from this source commit;" + echo "an existing pull request for it simply gains this approval." + echo echo "After it merges, hand deployment the merge commit as \`OPENSECRET_SOURCE_REF\`," echo "the artifact directory added to the Nix store as \`OPENSECRET_EIF_DIR\`, and" echo "\`OPENSECRET_EIF_SHA256=${eif_sha256}\`. Mirror the four PCR files to the legacy" diff --git a/scripts/ci/test_opensecret_workflows.py b/scripts/ci/test_opensecret_workflows.py index 5480a6833..db18075c3 100644 --- a/scripts/ci/test_opensecret_workflows.py +++ b/scripts/ci/test_opensecret_workflows.py @@ -577,12 +577,16 @@ def write_handoff(self, **overrides): "measurements": VALID_MEASUREMENTS, "eif_sha256": "f" * 64, **overrides} (self.artifact / "handoff.json").write_text(json.dumps(handoff)) - def change_approvals(self): - for name in ("pcrDev.json", "pcrDevHistory.json"): + def change_approvals(self, mode="dev"): + prefix = "pcrDev" if mode == "dev" else "pcrProd" + for name in (f"{prefix}.json", f"{prefix}History.json"): (self.repo / "services/opensecret" / name).write_text(f"{name} approved\n") - def run_publish(self, mode="dev"): - env = {"PATH": os.environ["PATH"], "HOME": str(self.root), "GITHUB_TOKEN": "fixture-token", + def branch(self): + return f"opensecret/pcr-approval-{self.source_sha[:12]}" + + def run_publish(self, mode="dev", path=None): + env = {"PATH": path or os.environ["PATH"], "HOME": str(self.root), "GITHUB_TOKEN": "fixture-token", "GITHUB_REPOSITORY": "fixture/repo", "GITHUB_RUN_ID": "7", "GITHUB_STEP_SUMMARY": str(self.summary)} return subprocess.run( [shutil.which("bash"), "--noprofile", "--norc", @@ -597,7 +601,7 @@ def test_pushes_only_the_approval_files_to_a_review_branch(self): self.change_approvals() result = self.run_publish() self.assertEqual(result.returncode, 0, result.stderr) - branch = f"opensecret/pcr-approval-dev-{self.source_sha[:12]}" + branch = self.branch() self.assertEqual(sorted(self.origin_branches().split()), sorted(["master", branch])) self.assertEqual(self.git("-C", str(self.origin), "rev-parse", "master"), self.source_sha) changed = self.git("-C", str(self.origin), "diff", "--name-only", "master", branch) @@ -610,7 +614,7 @@ def test_pushes_only_the_approval_files_to_a_review_branch(self): self.assertNotIn("fixture-token", (self.repo / ".git/config").read_text()) self.assertNotIn("fixture-token", result.stdout + result.stderr) - def test_refuses_unexpected_changes_and_existing_branches(self): + def test_refuses_unexpected_changes_and_a_repeated_environment(self): self.change_approvals() (self.repo / "services/opensecret/other.rs").write_text("fn main() { changed }\n") result = self.run_publish() @@ -622,7 +626,59 @@ def test_refuses_unexpected_changes_and_existing_branches(self): self.change_approvals() result = self.run_publish() self.assertNotEqual(result.returncode, 0) - self.assertIn("already exists", result.stderr) + self.assertIn("already carries the dev approval", result.stderr) + self.assertEqual(len(self.git("-C", str(self.origin), "rev-list", f"master..{self.branch()}").split()), 1) + + def test_second_environment_lands_on_the_same_branch(self): + self.change_approvals("dev") + self.assertEqual(self.run_publish("dev").returncode, 0) + self.git("-C", str(self.repo), "reset", "-q", "--hard", self.source_sha) + self.change_approvals("prod") + self.write_handoff(environment="prod") + result = self.run_publish("prod") + self.assertEqual(result.returncode, 0, result.stderr) + branch = self.branch() + self.assertEqual(sorted(self.origin_branches().split()), sorted(["master", branch])) + self.assertEqual(self.git("-C", str(self.origin), "rev-parse", "master"), self.source_sha) + self.assertEqual(len(self.git("-C", str(self.origin), "rev-list", f"master..{branch}").split()), 2) + changed = self.git("-C", str(self.origin), "diff", "--name-only", "master", branch) + self.assertEqual(changed.split(), ["services/opensecret/pcrDev.json", "services/opensecret/pcrDevHistory.json", + "services/opensecret/pcrProd.json", "services/opensecret/pcrProdHistory.json"]) + subjects = self.git("-C", str(self.origin), "log", "--format=%s", f"master..{branch}") + self.assertEqual(subjects.splitlines(), [f"Approve prod OpenSecret EIF measurements from {self.source_sha[:12]}", + f"Approve dev OpenSecret EIF measurements from {self.source_sha[:12]}"]) + + def test_lost_push_race_is_replayed_on_top_of_the_other_environment(self): + # A git shim lets the prod run push to the branch after the dev run has + # fetched and just before it pushes, so the first push is rejected. + rival = self.root / "rival" + self.git("clone", "-q", str(self.origin), str(rival)) + for name in ("pcrProd.json", "pcrProdHistory.json"): + (rival / "services/opensecret" / name).write_text(f"{name} approved\n") + self.git("-C", str(rival), "commit", "-q", "-am", f"Approve prod OpenSecret EIF measurements from {self.source_sha[:12]}") + shim_dir = self.root / "shim" + shim_dir.mkdir() + marker = self.root / "raced" + real_git = shutil.which("git") + (shim_dir / "git").write_text( + "#!/usr/bin/env bash\n" + "for arg in \"$@\"; do\n" + f" if [ \"$arg\" = push ] && [ ! -e '{marker}' ]; then touch '{marker}'; " + f"'{real_git}' -C '{rival}' push -q origin HEAD:refs/heads/{self.branch()}; fi\n" + "done\n" + f"exec '{real_git}' \"$@\"\n") + (shim_dir / "git").chmod(0o755) + self.change_approvals("dev") + result = self.run_publish("dev", path=f"{shim_dir}:{os.environ['PATH']}") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("rejected (attempt 1)", result.stderr) + branch = self.branch() + self.assertEqual(len(self.git("-C", str(self.origin), "rev-list", f"master..{branch}").split()), 2) + changed = self.git("-C", str(self.origin), "diff", "--name-only", "master", branch) + self.assertEqual(changed.split(), ["services/opensecret/pcrDev.json", "services/opensecret/pcrDevHistory.json", + "services/opensecret/pcrProd.json", "services/opensecret/pcrProdHistory.json"]) + subjects = self.git("-C", str(self.origin), "log", "--format=%s", f"master..{branch}") + self.assertEqual(subjects.splitlines()[0], f"Approve dev OpenSecret EIF measurements from {self.source_sha[:12]}") def test_nothing_to_publish_and_handoff_mismatches(self): result = self.run_publish() diff --git a/services/opensecret/docs/nitro-deploy.md b/services/opensecret/docs/nitro-deploy.md index 8174e522b..e4882c557 100644 --- a/services/opensecret/docs/nitro-deploy.md +++ b/services/opensecret/docs/nitro-deploy.md @@ -39,9 +39,13 @@ compare without signing. pinned public key before the history is rewritten atomically. Setup is in [`secretspec/README.md`](../secretspec/README.md). 3. The job commits the two changed approval files to - `opensecret/pcr-approval--` and prints the compare link. Open - the pull request yourself; the approval checks then rebuild and compare on - that PR, and the root `pcr-compatibility` check verifies every signature. + `opensecret/pcr-approval-` and prints the compare link. That branch + is shared by both environments' runs for the same source commit: when both + measurement sets change, the second run lands its commit on top of the + first, so one pull request carries both approvals and each environment's + approval check compares against its updated file. Open the pull request + yourself; the approval checks then rebuild and compare on that PR, and the + root `pcr-compatibility` check verifies every signature. After merging, mirror the four files to the legacy repository with the manual [compatibility procedure](pcr-compatibility.md) and verify both public locations. From 16860fd0707aff127466461663fa35e19390d61e Mon Sep 17 00:00:00 2001 From: Anthony Ronning <101225832+AnthonyRonning@users.noreply.github.com> Date: Mon, 14 Sep 2026 18:07:02 +0000 Subject: [PATCH 2/2] Give the race fixture's git shim the sandbox's bash The Nix check sandbox has no /usr/bin/env, so the shim must name the bash the test itself found. Co-Authored-By: Claude Fable 5.1 --- scripts/ci/test_opensecret_workflows.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/ci/test_opensecret_workflows.py b/scripts/ci/test_opensecret_workflows.py index db18075c3..060f162d9 100644 --- a/scripts/ci/test_opensecret_workflows.py +++ b/scripts/ci/test_opensecret_workflows.py @@ -661,7 +661,7 @@ def test_lost_push_race_is_replayed_on_top_of_the_other_environment(self): marker = self.root / "raced" real_git = shutil.which("git") (shim_dir / "git").write_text( - "#!/usr/bin/env bash\n" + f"#!{shutil.which('bash')}\n" "for arg in \"$@\"; do\n" f" if [ \"$arg\" = push ] && [ ! -e '{marker}' ]; then touch '{marker}'; " f"'{real_git}' -C '{rival}' push -q origin HEAD:refs/heads/{self.branch()}; fi\n"