Share one approval branch per source commit across environments

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"

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(
+            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"
+            "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()

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-<env>-<commit>` 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-<commit>` 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.