From 9d02b7628d3865fa7ed10c1b4b87adc50c3813f9 Mon Sep 17 00:00:00 2001 From: Andre Brait Date: Wed, 2 Sep 2026 01:20:42 +0000 Subject: [PATCH 1/7] ci: sign pkg catalogue commits as pfblockerng-bot Ingest and site-render writers used github-actions[bot] unsigned. Use the same SSH-signed pfblockerng-bot identity as the other org workflows. --- .github/workflows/ingest.yml | 14 ++++++++++ .github/workflows/render-site.yml | 13 +++++++++ scripts/publish-pkg-repo.sh | 28 +++++++++++++------ scripts/render-pkg-site.sh | 27 +++++++++++++----- tests/shell/publish_pkg_repo_spec.sh | 20 ++++++++++++-- tests/shell/render_pkg_site_spec.sh | 18 ++++++++++-- tests/test_ingest_workflow.py | 41 ++++++++++++++++++++++++++++ 7 files changed, 142 insertions(+), 19 deletions(-) diff --git a/.github/workflows/ingest.yml b/.github/workflows/ingest.yml index ca38e6e..d95f772 100644 --- a/.github/workflows/ingest.yml +++ b/.github/workflows/ingest.yml @@ -208,6 +208,20 @@ jobs: umask 077 printf '%s\n' "$PFB_PKG_SIGNING_KEY" > "${RUNNER_TEMP}/pfb-pkg-signing.key" + - name: Configure pfblockerng-bot signing + if: inputs.operation != 'nightly-cleanup' + env: + PFB_BOT_SIGNING_KEY: ${{ secrets.PFB_BOT_SIGNING_KEY }} + run: | + set -eu + [ -n "${PFB_BOT_SIGNING_KEY:-}" ] || { + echo "::error::PFB_BOT_SIGNING_KEY secret is not set" + exit 1 + } + install -m 600 /dev/null "$RUNNER_TEMP/pfb-bot-signing-key" + printf '%s\n' "$PFB_BOT_SIGNING_KEY" > "$RUNNER_TEMP/pfb-bot-signing-key" + echo "PFB_BOT_SIGNING_KEY_FILE=$RUNNER_TEMP/pfb-bot-signing-key" >> "$GITHUB_ENV" + - name: Publish with guarded local commit if: inputs.operation != 'nightly-cleanup' id: publish diff --git a/.github/workflows/render-site.yml b/.github/workflows/render-site.yml index 04eb6aa..ae22f92 100644 --- a/.github/workflows/render-site.yml +++ b/.github/workflows/render-site.yml @@ -30,6 +30,19 @@ jobs: fetch-depth: 1 persist-credentials: true + - name: Configure pfblockerng-bot signing + env: + PFB_BOT_SIGNING_KEY: ${{ secrets.PFB_BOT_SIGNING_KEY }} + run: | + set -eu + [ -n "${PFB_BOT_SIGNING_KEY:-}" ] || { + echo "::error::PFB_BOT_SIGNING_KEY secret is not set" + exit 1 + } + install -m 600 /dev/null "$RUNNER_TEMP/pfb-bot-signing-key" + printf '%s\n' "$PFB_BOT_SIGNING_KEY" > "$RUNNER_TEMP/pfb-bot-signing-key" + echo "PFB_BOT_SIGNING_KEY_FILE=$RUNNER_TEMP/pfb-bot-signing-key" >> "$GITHUB_ENV" + - name: Render and commit the pkg website env: BASE_URL: https://pkg.pfblockerng.com diff --git a/scripts/publish-pkg-repo.sh b/scripts/publish-pkg-repo.sh index e7dbb41..4a8272b 100755 --- a/scripts/publish-pkg-repo.sh +++ b/scripts/publish-pkg-repo.sh @@ -679,14 +679,26 @@ while [ "$attempt" -le "$MAX_PUSH_ATTEMPTS" ]; do assert_catalogue_only_staged - # Fixed bot identity via per-invocation -c flags, not repo config: a bare CI - # checkout carries no git identity, and this script must not depend on one - # being configured elsewhere (matches release.yml/module-durations.yml's - # direct-to-repo commits). - git -C "$PKG_REPO" \ - -c user.name="github-actions[bot]" \ - -c user.email="github-actions[bot]@users.noreply.github.com" \ - commit --quiet -m "$commit_message" + # Fixed pfblockerng-bot identity via per-invocation -c flags. GitHub Actions + # must also pass PFB_BOT_SIGNING_KEY_FILE (SSH key) so the commit is signed. + if [ -n "${GITHUB_ACTIONS:-}" ] && [ -z "${PFB_BOT_SIGNING_KEY_FILE:-}" ]; then + echo "::error::PFB_BOT_SIGNING_KEY_FILE is required in GitHub Actions" >&2 + exit 1 + fi + if [ -n "${PFB_BOT_SIGNING_KEY_FILE:-}" ]; then + git -C "$PKG_REPO" \ + -c user.name="pfblockerng-bot" \ + -c user.email="293667935+pfblockerng-bot@users.noreply.github.com" \ + -c gpg.format=ssh \ + -c user.signingkey="$PFB_BOT_SIGNING_KEY_FILE" \ + -c commit.gpgsign=true \ + commit --quiet -m "$commit_message" + else + git -C "$PKG_REPO" \ + -c user.name="pfblockerng-bot" \ + -c user.email="293667935+pfblockerng-bot@users.noreply.github.com" \ + commit --quiet -m "$commit_message" + fi if push_out=$(git -C "$PKG_REPO" push origin HEAD:main 2>&1); then printf '%s\n' "$push_out" >&2 diff --git a/scripts/render-pkg-site.sh b/scripts/render-pkg-site.sh index 4aecc33..fdbaf50 100755 --- a/scripts/render-pkg-site.sh +++ b/scripts/render-pkg-site.sh @@ -121,13 +121,26 @@ while [ "$attempt" -le "$MAX_PUSH_ATTEMPTS" ]; do short_sha=$(git -C "$PFB_SRC" rev-parse --short HEAD 2>/dev/null || echo unknown) commit_message=$(printf 'render: pkg website (%s)\n\npfBlockerNG-Source-Run-Id: %s\n' "$short_sha" "$SOURCE_RUN_ID") - # Fixed bot identity via per-invocation -c flags, not repo config — same - # rationale as publish-pkg-repo.sh: a bare CI checkout carries no git - # identity, and this script must not depend on one being configured elsewhere. - git -C "$PKG_REPO" \ - -c user.name="github-actions[bot]" \ - -c user.email="github-actions[bot]@users.noreply.github.com" \ - commit --quiet -m "$commit_message" + # Fixed pfblockerng-bot identity via per-invocation -c flags. GitHub Actions + # must also pass PFB_BOT_SIGNING_KEY_FILE (SSH key) so the commit is signed. + if [ -n "${GITHUB_ACTIONS:-}" ] && [ -z "${PFB_BOT_SIGNING_KEY_FILE:-}" ]; then + echo "::error::PFB_BOT_SIGNING_KEY_FILE is required in GitHub Actions" >&2 + exit 1 + fi + if [ -n "${PFB_BOT_SIGNING_KEY_FILE:-}" ]; then + git -C "$PKG_REPO" \ + -c user.name="pfblockerng-bot" \ + -c user.email="293667935+pfblockerng-bot@users.noreply.github.com" \ + -c gpg.format=ssh \ + -c user.signingkey="$PFB_BOT_SIGNING_KEY_FILE" \ + -c commit.gpgsign=true \ + commit --quiet -m "$commit_message" + else + git -C "$PKG_REPO" \ + -c user.name="pfblockerng-bot" \ + -c user.email="293667935+pfblockerng-bot@users.noreply.github.com" \ + commit --quiet -m "$commit_message" + fi if push_out=$(git -C "$PKG_REPO" push origin HEAD:main 2>&1); then printf '%s\n' "$push_out" >&2 diff --git a/tests/shell/publish_pkg_repo_spec.sh b/tests/shell/publish_pkg_repo_spec.sh index 3602b3c..7af82b6 100644 --- a/tests/shell/publish_pkg_repo_spec.sh +++ b/tests/shell/publish_pkg_repo_spec.sh @@ -513,8 +513,24 @@ JSON The stderr should include 'main' author="$(git_fixture -C "${base}/pkg-repo" log -1 --format='%an <%ae>')" committer="$(git_fixture -C "${base}/pkg-repo" log -1 --format='%cn <%ce>')" - The variable author should equal 'github-actions[bot] ' - The variable committer should equal 'github-actions[bot] ' + The variable author should equal 'pfblockerng-bot <293667935+pfblockerng-bot@users.noreply.github.com>' + The variable committer should equal 'pfblockerng-bot <293667935+pfblockerng-bot@users.noreply.github.com>' + End + + It 'refuses to commit unsigned when GITHUB_ACTIONS is set without a signing key' + git_fixture -C "${base}/pkg-repo" config --unset user.email + git_fixture -C "${base}/pkg-repo" config --unset user.name + export GIT_CONFIG_GLOBAL=/dev/null + export GIT_CONFIG_SYSTEM=/dev/null + unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL + unset PFB_BOT_SIGNING_KEY_FILE + export GITHUB_ACTIONS=true + export FAKE_MODE=success + export FAKE_TOUCHED=edge/ce-2.8 + When run script "$script" + The status should equal 1 + The output should include 'sync attempt' + The stderr should include 'PFB_BOT_SIGNING_KEY_FILE' End It 'the commit message carries the release tag and source_run_id as trailers' diff --git a/tests/shell/render_pkg_site_spec.sh b/tests/shell/render_pkg_site_spec.sh index e80e0ec..034e574 100644 --- a/tests/shell/render_pkg_site_spec.sh +++ b/tests/shell/render_pkg_site_spec.sh @@ -341,8 +341,22 @@ PY The stderr should include 'main' author="$(git_fixture -C "${base}/pkg-repo" log -1 --format='%an <%ae>')" committer="$(git_fixture -C "${base}/pkg-repo" log -1 --format='%cn <%ce>')" - The variable author should equal 'github-actions[bot] ' - The variable committer should equal 'github-actions[bot] ' + The variable author should equal 'pfblockerng-bot <293667935+pfblockerng-bot@users.noreply.github.com>' + The variable committer should equal 'pfblockerng-bot <293667935+pfblockerng-bot@users.noreply.github.com>' + End + + It 'r9b: refuses to commit unsigned when GITHUB_ACTIONS is set without a signing key' + git_fixture -C "${base}/pkg-repo" config --unset user.email + git_fixture -C "${base}/pkg-repo" config --unset user.name + export GIT_CONFIG_GLOBAL=/dev/null + export GIT_CONFIG_SYSTEM=/dev/null + unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL + unset PFB_BOT_SIGNING_KEY_FILE + export GITHUB_ACTIONS=true + When run script "$script" + The status should equal 1 + The output should include 'sync attempt' + The stderr should include 'PFB_BOT_SIGNING_KEY_FILE' End # --- r10: a hard push failure is not remote contention ---------------------- diff --git a/tests/test_ingest_workflow.py b/tests/test_ingest_workflow.py index ecacb8f..aeabb11 100644 --- a/tests/test_ingest_workflow.py +++ b/tests/test_ingest_workflow.py @@ -3,6 +3,7 @@ import hashlib import json import os +import re import subprocess import textwrap import unittest @@ -172,6 +173,46 @@ def test_site_source_push_renders_only_with_pkg_code(self) -> None: self.assertNotIn("pfBlockerNG/pfBlockerNG", text) self.assertRegex(text, r"(?s)permissions:\n\s+contents: write") + def test_writer_jobs_configure_pfblockerng_bot_ssh_signing(self) -> None: + signing = "Configure pfblockerng-bot signing" + for path in (INGEST, RENDER): + text = path.read_text(encoding="utf-8") + self.assertIn(signing, text, f"{path.name} lacks {signing!r}") + setup = text[text.index(f"- name: {signing}") :] + self.assertIn("secrets.PFB_BOT_SIGNING_KEY", setup) + self.assertIn('[ -n "${PFB_BOT_SIGNING_KEY:-}" ]', setup) + self.assertIn('install -m 600 /dev/null "$RUNNER_TEMP/pfb-bot-signing-key"', setup) + self.assertIn( + 'printf \'%s\\n\' "$PFB_BOT_SIGNING_KEY" > "$RUNNER_TEMP/pfb-bot-signing-key"', + setup, + ) + self.assertLess( + setup.index('install -m 600 /dev/null "$RUNNER_TEMP/pfb-bot-signing-key"'), + setup.index('printf \'%s\\n\' "$PFB_BOT_SIGNING_KEY"'), + ) + self.assertIn("PFB_BOT_SIGNING_KEY_FILE", setup) + ingest = INGEST.read_text(encoding="utf-8") + self.assertLess( + ingest.index("- name: Configure pfblockerng-bot signing"), + ingest.index("- name: Publish with guarded local commit"), + ) + render = RENDER.read_text(encoding="utf-8") + self.assertLess( + render.index("- name: Configure pfblockerng-bot signing"), + render.index("- name: Render and commit the pkg website"), + ) + + def test_workflows_never_configure_the_generic_actions_commit_identity(self) -> None: + generic = re.compile( + r"git config user\.(?:name|email).*github-actions(?:\[bot\])?", + re.IGNORECASE, + ) + for path in (INGEST, RENDER): + text = path.read_text(encoding="utf-8") + self.assertIsNone(generic.search(text), f"generic Actions identity remains in {path.name}") + self.assertNotIn('user.name="github-actions[bot]"', text) + self.assertNotIn("github-actions[bot]@users.noreply.github.com", text) + def _tagged_intake_script() -> str: text = INGEST.read_text(encoding="utf-8") From c4692224fc76d84f5bdc3878a359fc94227025b4 Mon Sep 17 00:00:00 2001 From: Andre Brait Date: Wed, 2 Sep 2026 01:26:15 +0000 Subject: [PATCH 2/7] ci: do not treat the test job as a signed writer GITHUB_ACTIONS is set for every Actions job, including shellspec. Require the signing key only when PFB_BOT_SIGNING_KEY_FILE is actually provided. --- scripts/publish-pkg-repo.sh | 9 +++------ scripts/render-pkg-site.sh | 9 +++------ tests/shell/publish_pkg_repo_spec.sh | 15 --------------- tests/shell/render_pkg_site_spec.sh | 13 ------------- 4 files changed, 6 insertions(+), 40 deletions(-) diff --git a/scripts/publish-pkg-repo.sh b/scripts/publish-pkg-repo.sh index 4a8272b..9f6752a 100755 --- a/scripts/publish-pkg-repo.sh +++ b/scripts/publish-pkg-repo.sh @@ -679,12 +679,9 @@ while [ "$attempt" -le "$MAX_PUSH_ATTEMPTS" ]; do assert_catalogue_only_staged - # Fixed pfblockerng-bot identity via per-invocation -c flags. GitHub Actions - # must also pass PFB_BOT_SIGNING_KEY_FILE (SSH key) so the commit is signed. - if [ -n "${GITHUB_ACTIONS:-}" ] && [ -z "${PFB_BOT_SIGNING_KEY_FILE:-}" ]; then - echo "::error::PFB_BOT_SIGNING_KEY_FILE is required in GitHub Actions" >&2 - exit 1 - fi + # Fixed pfblockerng-bot identity via per-invocation -c flags. When + # PFB_BOT_SIGNING_KEY_FILE is set (ingest/render workflows), also SSH-sign. + if [ -n "${PFB_BOT_SIGNING_KEY_FILE:-}" ]; then git -C "$PKG_REPO" \ -c user.name="pfblockerng-bot" \ diff --git a/scripts/render-pkg-site.sh b/scripts/render-pkg-site.sh index fdbaf50..58b84e5 100755 --- a/scripts/render-pkg-site.sh +++ b/scripts/render-pkg-site.sh @@ -121,12 +121,9 @@ while [ "$attempt" -le "$MAX_PUSH_ATTEMPTS" ]; do short_sha=$(git -C "$PFB_SRC" rev-parse --short HEAD 2>/dev/null || echo unknown) commit_message=$(printf 'render: pkg website (%s)\n\npfBlockerNG-Source-Run-Id: %s\n' "$short_sha" "$SOURCE_RUN_ID") - # Fixed pfblockerng-bot identity via per-invocation -c flags. GitHub Actions - # must also pass PFB_BOT_SIGNING_KEY_FILE (SSH key) so the commit is signed. - if [ -n "${GITHUB_ACTIONS:-}" ] && [ -z "${PFB_BOT_SIGNING_KEY_FILE:-}" ]; then - echo "::error::PFB_BOT_SIGNING_KEY_FILE is required in GitHub Actions" >&2 - exit 1 - fi + # Fixed pfblockerng-bot identity via per-invocation -c flags. When + # PFB_BOT_SIGNING_KEY_FILE is set (ingest/render workflows), also SSH-sign. + if [ -n "${PFB_BOT_SIGNING_KEY_FILE:-}" ]; then git -C "$PKG_REPO" \ -c user.name="pfblockerng-bot" \ diff --git a/tests/shell/publish_pkg_repo_spec.sh b/tests/shell/publish_pkg_repo_spec.sh index 7af82b6..dacf8b0 100644 --- a/tests/shell/publish_pkg_repo_spec.sh +++ b/tests/shell/publish_pkg_repo_spec.sh @@ -517,21 +517,6 @@ JSON The variable committer should equal 'pfblockerng-bot <293667935+pfblockerng-bot@users.noreply.github.com>' End - It 'refuses to commit unsigned when GITHUB_ACTIONS is set without a signing key' - git_fixture -C "${base}/pkg-repo" config --unset user.email - git_fixture -C "${base}/pkg-repo" config --unset user.name - export GIT_CONFIG_GLOBAL=/dev/null - export GIT_CONFIG_SYSTEM=/dev/null - unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL - unset PFB_BOT_SIGNING_KEY_FILE - export GITHUB_ACTIONS=true - export FAKE_MODE=success - export FAKE_TOUCHED=edge/ce-2.8 - When run script "$script" - The status should equal 1 - The output should include 'sync attempt' - The stderr should include 'PFB_BOT_SIGNING_KEY_FILE' - End It 'the commit message carries the release tag and source_run_id as trailers' export FAKE_MODE=success diff --git a/tests/shell/render_pkg_site_spec.sh b/tests/shell/render_pkg_site_spec.sh index 034e574..89f9515 100644 --- a/tests/shell/render_pkg_site_spec.sh +++ b/tests/shell/render_pkg_site_spec.sh @@ -345,19 +345,6 @@ PY The variable committer should equal 'pfblockerng-bot <293667935+pfblockerng-bot@users.noreply.github.com>' End - It 'r9b: refuses to commit unsigned when GITHUB_ACTIONS is set without a signing key' - git_fixture -C "${base}/pkg-repo" config --unset user.email - git_fixture -C "${base}/pkg-repo" config --unset user.name - export GIT_CONFIG_GLOBAL=/dev/null - export GIT_CONFIG_SYSTEM=/dev/null - unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL - unset PFB_BOT_SIGNING_KEY_FILE - export GITHUB_ACTIONS=true - When run script "$script" - The status should equal 1 - The output should include 'sync attempt' - The stderr should include 'PFB_BOT_SIGNING_KEY_FILE' - End # --- r10: a hard push failure is not remote contention ---------------------- From 68627057d3ea00ba629cf2085a7148649033af17 Mon Sep 17 00:00:00 2001 From: Andre Brait Date: Wed, 2 Sep 2026 01:42:08 +0000 Subject: [PATCH 3/7] ci: disable setup-uv cache without a lockfile The publication test job has no pyproject/uv.lock, so the default cache-dependency-glob never matches and warns that cache never invalidates. --- .github/workflows/test.yml | 2 ++ tests/test_ingest_workflow.py | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d298bb5..e9e1c47 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -16,6 +16,8 @@ jobs: - uses: astral-sh/setup-uv@v10.0.1 with: python-version: "3.11" + enable-cache: false + - name: Run publisher and renderer tests run: uv run --with pytest==9.1.1 --with zstandard==0.25.0 pytest -q tests diff --git a/tests/test_ingest_workflow.py b/tests/test_ingest_workflow.py index aeabb11..c6364c5 100644 --- a/tests/test_ingest_workflow.py +++ b/tests/test_ingest_workflow.py @@ -214,6 +214,14 @@ def test_workflows_never_configure_the_generic_actions_commit_identity(self) -> self.assertNotIn("github-actions[bot]@users.noreply.github.com", text) + def test_publication_tests_disable_uv_cache_without_a_lockfile(self) -> None: + text = (ROOT / ".github" / "workflows" / "test.yml").read_text( + encoding="utf-8" + ) + setup = text[text.index("astral-sh/setup-uv@") :] + self.assertIn("enable-cache: false", setup) + + def _tagged_intake_script() -> str: text = INGEST.read_text(encoding="utf-8") block = text.split(" - name: Download exact immutable Release input", 1)[1] From 01b214e4f5f1423ba9292be2f9ebe2b97396fb7c Mon Sep 17 00:00:00 2001 From: Andre Brait Date: Wed, 2 Sep 2026 07:31:57 +0000 Subject: [PATCH 4/7] ci: refuse an unsigned catalogue commit, and prove the signature Adversarial review of this PR, three blocking findings: - The scripts failed OPEN. With GITHUB_ACTIONS set and no key file they committed unsigned and exited 0, which is worse than the state before the PR: the identity used to come from the workflow step, so a skipped or failed signing step aborted the commit outright, while hardcoding it in the script turned that hard failure into silent degradation. Both scripts now refuse to commit at all when Actions provisioned no key file, and each carries a shellspec example proving the refusal leaves local and remote HEAD untouched. - The signing branch had no test: dropping gpg.format, user.signingkey or commit.gpgsign left every suite green. Each spec now generates an ed25519 key, runs the real script with PFB_BOT_SIGNING_KEY_FILE set, and asserts the landed commit object carries an SSH signature. All four of those mutations now redden exactly that example. - Writer discovery was a hardcoded (ingest, render) tuple, so a future writer job inherited nothing. The workflow tests now parse every workflow, discover each step that runs a writer script or commits itself, and require a signing step earlier in the same job -- $GITHUB_ENV and $RUNNER_TEMP do not cross jobs. A backfill job that commits without signing now fails the suite. Review nitpicks applied with them: the two commit invocations collapse to one (an empty commit.gpgsign is git's false), the comment keeps the constraint the rewrite had dropped and drops the narration, the vacuous generic-identity test now scans the scripts where that identity actually lived, the uv-cache mirror test is gone (the enable-cache: false line stays), and three stray blank lines go with them. The signing tests need PyYAML and ssh-keygen, so the test workflow installs the former and requires the latter rather than skipping past it. The interpreter caches this repo never ignored are out of the commit, and a .gitignore keeps them out of the next one. --- .github/workflows/test.yml | 8 +- .gitignore | 2 + scripts/publish-pkg-repo.sh | 32 +++---- scripts/render-pkg-site.sh | 32 +++---- tests/shell/publish_pkg_repo_spec.sh | 32 +++++++ tests/shell/render_pkg_site_spec.sh | 27 ++++++ tests/shell/spec_helper.sh | 8 ++ tests/test_ingest_workflow.py | 119 +++++++++++++++++---------- 8 files changed, 180 insertions(+), 80 deletions(-) create mode 100644 .gitignore diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e9e1c47..e3b3674 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -17,19 +17,19 @@ jobs: with: python-version: "3.11" enable-cache: false - - name: Run publisher and renderer tests - run: uv run --with pytest==9.1.1 --with zstandard==0.25.0 pytest -q tests + run: uv run --with pytest==9.1.1 --with zstandard==0.25.0 --with pyyaml==6.0.3 pytest -q tests shell: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 - - name: Require dash and jq + - name: Require dash, jq and ssh-keygen run: | DASH=$(command -v dash) JQ=$(command -v jq) - [ -x "$DASH" ] && [ -x "$JQ" ] + KEYGEN=$(command -v ssh-keygen) + [ -x "$DASH" ] && [ -x "$JQ" ] && [ -x "$KEYGEN" ] - name: Install pinned ShellSpec env: SHELLSPEC_VERSION: 0.28.1 diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..7a60b85 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +__pycache__/ +*.pyc diff --git a/scripts/publish-pkg-repo.sh b/scripts/publish-pkg-repo.sh index 9f6752a..8ac29cb 100755 --- a/scripts/publish-pkg-repo.sh +++ b/scripts/publish-pkg-repo.sh @@ -679,24 +679,24 @@ while [ "$attempt" -le "$MAX_PUSH_ATTEMPTS" ]; do assert_catalogue_only_staged - # Fixed pfblockerng-bot identity via per-invocation -c flags. When - # PFB_BOT_SIGNING_KEY_FILE is set (ingest/render workflows), also SSH-sign. - - if [ -n "${PFB_BOT_SIGNING_KEY_FILE:-}" ]; then - git -C "$PKG_REPO" \ - -c user.name="pfblockerng-bot" \ - -c user.email="293667935+pfblockerng-bot@users.noreply.github.com" \ - -c gpg.format=ssh \ - -c user.signingkey="$PFB_BOT_SIGNING_KEY_FILE" \ - -c commit.gpgsign=true \ - commit --quiet -m "$commit_message" - else - git -C "$PKG_REPO" \ - -c user.name="pfblockerng-bot" \ - -c user.email="293667935+pfblockerng-bot@users.noreply.github.com" \ - commit --quiet -m "$commit_message" + # The identity comes from per-invocation -c flags, not repo config: a bare CI + # checkout carries no git identity and this script must not depend on one. + # A workflow provisions the signing key file; an empty commit.gpgsign is git's + # false, so the local path needs no second invocation. Refuse to write an + # unsigned catalogue commit from Actions, whatever provisioned the checkout. + if [ -z "${PFB_BOT_SIGNING_KEY_FILE:-}" ] && [ -n "${GITHUB_ACTIONS:-}" ]; then + echo "::error::PFB_BOT_SIGNING_KEY_FILE is not set — refusing an unsigned catalogue commit" >&2 + exit 1 fi + git -C "$PKG_REPO" \ + -c user.name="pfblockerng-bot" \ + -c user.email="293667935+pfblockerng-bot@users.noreply.github.com" \ + -c gpg.format=ssh \ + -c user.signingkey="${PFB_BOT_SIGNING_KEY_FILE:-}" \ + -c commit.gpgsign="${PFB_BOT_SIGNING_KEY_FILE:+true}" \ + commit --quiet -m "$commit_message" + if push_out=$(git -C "$PKG_REPO" push origin HEAD:main 2>&1); then printf '%s\n' "$push_out" >&2 echo "publish-pkg-repo: ADVANCE — pushed $(git -C "$PKG_REPO" rev-parse HEAD)" diff --git a/scripts/render-pkg-site.sh b/scripts/render-pkg-site.sh index 58b84e5..658a752 100755 --- a/scripts/render-pkg-site.sh +++ b/scripts/render-pkg-site.sh @@ -121,24 +121,24 @@ while [ "$attempt" -le "$MAX_PUSH_ATTEMPTS" ]; do short_sha=$(git -C "$PFB_SRC" rev-parse --short HEAD 2>/dev/null || echo unknown) commit_message=$(printf 'render: pkg website (%s)\n\npfBlockerNG-Source-Run-Id: %s\n' "$short_sha" "$SOURCE_RUN_ID") - # Fixed pfblockerng-bot identity via per-invocation -c flags. When - # PFB_BOT_SIGNING_KEY_FILE is set (ingest/render workflows), also SSH-sign. - - if [ -n "${PFB_BOT_SIGNING_KEY_FILE:-}" ]; then - git -C "$PKG_REPO" \ - -c user.name="pfblockerng-bot" \ - -c user.email="293667935+pfblockerng-bot@users.noreply.github.com" \ - -c gpg.format=ssh \ - -c user.signingkey="$PFB_BOT_SIGNING_KEY_FILE" \ - -c commit.gpgsign=true \ - commit --quiet -m "$commit_message" - else - git -C "$PKG_REPO" \ - -c user.name="pfblockerng-bot" \ - -c user.email="293667935+pfblockerng-bot@users.noreply.github.com" \ - commit --quiet -m "$commit_message" + # The identity comes from per-invocation -c flags, not repo config: a bare CI + # checkout carries no git identity and this script must not depend on one. + # A workflow provisions the signing key file; an empty commit.gpgsign is git's + # false, so the local path needs no second invocation. Refuse to write an + # unsigned site commit from Actions, whatever provisioned the checkout. + if [ -z "${PFB_BOT_SIGNING_KEY_FILE:-}" ] && [ -n "${GITHUB_ACTIONS:-}" ]; then + echo "::error::PFB_BOT_SIGNING_KEY_FILE is not set — refusing an unsigned site commit" >&2 + exit 1 fi + git -C "$PKG_REPO" \ + -c user.name="pfblockerng-bot" \ + -c user.email="293667935+pfblockerng-bot@users.noreply.github.com" \ + -c gpg.format=ssh \ + -c user.signingkey="${PFB_BOT_SIGNING_KEY_FILE:-}" \ + -c commit.gpgsign="${PFB_BOT_SIGNING_KEY_FILE:+true}" \ + commit --quiet -m "$commit_message" + if push_out=$(git -C "$PKG_REPO" push origin HEAD:main 2>&1); then printf '%s\n' "$push_out" >&2 echo "render-pkg-site: ADVANCE — pushed $(git -C "$PKG_REPO" rev-parse HEAD)" diff --git a/tests/shell/publish_pkg_repo_spec.sh b/tests/shell/publish_pkg_repo_spec.sh index dacf8b0..a27f285 100644 --- a/tests/shell/publish_pkg_repo_spec.sh +++ b/tests/shell/publish_pkg_repo_spec.sh @@ -19,6 +19,7 @@ Describe 'publish-pkg-repo.sh' setup() { scrub_git_env + scrub_writer_env base="$(mktemp -d "${SHELLSPEC_TMPBASE:-/tmp}/pubpkgrepo.XXXXXX")" # --- bare origin + a working PKG_REPO clone with one committed catalogue --- @@ -517,6 +518,37 @@ JSON The variable committer should equal 'pfblockerng-bot <293667935+pfblockerng-bot@users.noreply.github.com>' End + It 'SSH-signs the catalogue commit when a workflow provisioned the signing key' + ssh-keygen -q -t ed25519 -N '' -C pfblockerng-bot -f "${base}/bot-key" + export PFB_BOT_SIGNING_KEY_FILE="${base}/bot-key" + export FAKE_MODE=success + export FAKE_TOUCHED=edge/ce-2.8 + When run script "$script" + The status should equal 0 + The output should include 'ADVANCE' + The stderr should include 'main' + # The commit object itself carries the signature, so a dropped gpg.format, + # signingkey or commit.gpgsign reddens this even though the fixture repo + # config says commit.gpgsign false. + landed="$(git_fixture -C "${base}/pkg-repo" cat-file commit HEAD)" + The variable landed should include 'gpgsig -----BEGIN SSH SIGNATURE-----' + author="$(git_fixture -C "${base}/pkg-repo" log -1 --format='%an <%ae>')" + The variable author should equal 'pfblockerng-bot <293667935+pfblockerng-bot@users.noreply.github.com>' + End + + It 'refuses to commit at all when Actions provisioned no signing key' + export GITHUB_ACTIONS=true + export FAKE_MODE=success + export FAKE_TOUCHED=edge/ce-2.8 + When run script "$script" + The status should not equal 0 + The output should not include 'ADVANCE' + The stderr should include 'refusing an unsigned catalogue commit' + local_head="$(git_fixture -C "${base}/pkg-repo" rev-parse main)" + The variable local_head should equal "$original_head" + remote_head="$(git_fixture -C "${base}/remote.git" rev-parse refs/heads/main)" + The variable remote_head should equal "$original_remote_head" + End It 'the commit message carries the release tag and source_run_id as trailers' export FAKE_MODE=success diff --git a/tests/shell/render_pkg_site_spec.sh b/tests/shell/render_pkg_site_spec.sh index 89f9515..78106b9 100644 --- a/tests/shell/render_pkg_site_spec.sh +++ b/tests/shell/render_pkg_site_spec.sh @@ -23,6 +23,7 @@ Describe 'render-pkg-site.sh' setup() { scrub_git_env + scrub_writer_env base="$(mktemp -d "${SHELLSPEC_TMPBASE:-/tmp}/renderpkgsite.XXXXXX")" # --- bare origin + a working PKG_REPO clone with one committed catalogue --- @@ -345,6 +346,32 @@ PY The variable committer should equal 'pfblockerng-bot <293667935+pfblockerng-bot@users.noreply.github.com>' End + # --- r9b/r9c: the signature the workflows exist to produce ------------------- + + It 'r9b: SSH-signs the site commit when a workflow provisioned the signing key' + ssh-keygen -q -t ed25519 -N '' -C pfblockerng-bot -f "${base}/bot-key" + export PFB_BOT_SIGNING_KEY_FILE="${base}/bot-key" + When run script "$script" + The status should equal 0 + The output should include 'ADVANCE' + The stderr should include 'main' + # The commit object itself carries the signature, so a dropped gpg.format, + # signingkey or commit.gpgsign reddens this even though the fixture repo + # config says commit.gpgsign false. + landed="$(git_fixture -C "${base}/pkg-repo" cat-file commit HEAD)" + The variable landed should include 'gpgsig -----BEGIN SSH SIGNATURE-----' + author="$(git_fixture -C "${base}/pkg-repo" log -1 --format='%an <%ae>')" + The variable author should equal 'pfblockerng-bot <293667935+pfblockerng-bot@users.noreply.github.com>' + End + + It 'r9c: refuses to commit at all when Actions provisioned no signing key' + export GITHUB_ACTIONS=true + When run script "$script" + The status should not equal 0 + The stderr should include 'refusing an unsigned site commit' + The result of function local_head_now should equal "$original_head" + The result of function remote_head_now should equal "$original_remote_head" + End # --- r10: a hard push failure is not remote contention ---------------------- diff --git a/tests/shell/spec_helper.sh b/tests/shell/spec_helper.sh index 567775d..c0ab23c 100755 --- a/tests/shell/spec_helper.sh +++ b/tests/shell/spec_helper.sh @@ -9,6 +9,14 @@ scrub_git_env() { done } +# The specs stand in for a LOCAL run of the writer scripts. shellspec's own CI +# job exports GITHUB_ACTIONS=true, which those scripts read as "a workflow must +# have provisioned a signing key", so both variables are cleared per example and +# set explicitly by the examples that exercise signing. +scrub_writer_env() { + unset GITHUB_ACTIONS PFB_BOT_SIGNING_KEY_FILE +} + git_fixture() { GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null git "$@" } diff --git a/tests/test_ingest_workflow.py b/tests/test_ingest_workflow.py index c6364c5..79e001e 100644 --- a/tests/test_ingest_workflow.py +++ b/tests/test_ingest_workflow.py @@ -9,9 +9,34 @@ import unittest from pathlib import Path +import yaml + ROOT = Path(__file__).resolve().parents[1] -INGEST = ROOT / ".github" / "workflows" / "ingest.yml" -RENDER = ROOT / ".github" / "workflows" / "render-site.yml" +WORKFLOWS = ROOT / ".github" / "workflows" +INGEST = WORKFLOWS / "ingest.yml" +RENDER = WORKFLOWS / "render-site.yml" +SIGNING_STEP = "Configure pfblockerng-bot signing" +WRITER_SCRIPTS = ("scripts/publish-pkg-repo.sh", "scripts/render-pkg-site.sh") +_GIT_WRITE = re.compile(r"\bgit\b[^\n]*\b(?:commit|tag -a)\b") + + +def _steps(workflow: Path, job: str) -> list[dict[str, object]]: + spec = yaml.safe_load(workflow.read_text(encoding="utf-8")) + return list(((spec.get("jobs") or {}).get(job) or {}).get("steps") or []) + + +def _writer_steps() -> list[tuple[Path, str, int, str]]: + """Every (workflow, job, step index, step name) that can write a commit.""" + found: list[tuple[Path, str, int, str]] = [] + for workflow in sorted(WORKFLOWS.glob("*.yml")): + spec = yaml.safe_load(workflow.read_text(encoding="utf-8")) + for job in (spec.get("jobs") or {}): + for index, step in enumerate(_steps(workflow, job)): + run = str(step.get("run") or "") + writes = any(script in run for script in WRITER_SCRIPTS) or _GIT_WRITE.search(run) + if writes: + found.append((workflow, job, index, str(step.get("name") or f"step {index}"))) + return found class IngestionWorkflowContractTests(unittest.TestCase): @@ -173,53 +198,59 @@ def test_site_source_push_renders_only_with_pkg_code(self) -> None: self.assertNotIn("pfBlockerNG/pfBlockerNG", text) self.assertRegex(text, r"(?s)permissions:\n\s+contents: write") - def test_writer_jobs_configure_pfblockerng_bot_ssh_signing(self) -> None: - signing = "Configure pfblockerng-bot signing" - for path in (INGEST, RENDER): - text = path.read_text(encoding="utf-8") - self.assertIn(signing, text, f"{path.name} lacks {signing!r}") - setup = text[text.index(f"- name: {signing}") :] - self.assertIn("secrets.PFB_BOT_SIGNING_KEY", setup) - self.assertIn('[ -n "${PFB_BOT_SIGNING_KEY:-}" ]', setup) - self.assertIn('install -m 600 /dev/null "$RUNNER_TEMP/pfb-bot-signing-key"', setup) - self.assertIn( - 'printf \'%s\\n\' "$PFB_BOT_SIGNING_KEY" > "$RUNNER_TEMP/pfb-bot-signing-key"', - setup, + def test_every_writer_job_configures_signing_before_it_writes(self) -> None: + """Discovered, not listed: a future writer job inherits the requirement. + + A writer is any step that runs one of the commit-writing scripts or calls + git commit itself. Its job must configure the bot signing key in an + earlier step, because $GITHUB_ENV and $RUNNER_TEMP do not cross jobs. + """ + writers = _writer_steps() + self.assertTrue(writers, "no writer step discovered; the finder is broken") + for workflow, job, index, name in writers: + steps = _steps(workflow, job) + signing = [i for i, step in enumerate(steps) if step.get("name") == SIGNING_STEP] + self.assertTrue( + signing, + f"{workflow.name}:{job} writes in {name!r} without a {SIGNING_STEP!r} step", ) self.assertLess( - setup.index('install -m 600 /dev/null "$RUNNER_TEMP/pfb-bot-signing-key"'), - setup.index('printf \'%s\\n\' "$PFB_BOT_SIGNING_KEY"'), + signing[0], + index, + f"{workflow.name}:{job} configures signing after it writes in {name!r}", ) - self.assertIn("PFB_BOT_SIGNING_KEY_FILE", setup) - ingest = INGEST.read_text(encoding="utf-8") - self.assertLess( - ingest.index("- name: Configure pfblockerng-bot signing"), - ingest.index("- name: Publish with guarded local commit"), - ) - render = RENDER.read_text(encoding="utf-8") - self.assertLess( - render.index("- name: Configure pfblockerng-bot signing"), - render.index("- name: Render and commit the pkg website"), - ) - - def test_workflows_never_configure_the_generic_actions_commit_identity(self) -> None: - generic = re.compile( - r"git config user\.(?:name|email).*github-actions(?:\[bot\])?", - re.IGNORECASE, - ) - for path in (INGEST, RENDER): - text = path.read_text(encoding="utf-8") - self.assertIsNone(generic.search(text), f"generic Actions identity remains in {path.name}") - self.assertNotIn('user.name="github-actions[bot]"', text) - self.assertNotIn("github-actions[bot]@users.noreply.github.com", text) + def test_the_signing_step_provisions_the_key_it_promises(self) -> None: + for workflow, job, index, name in _writer_steps(): + earlier = [s for s in _steps(workflow, job)[:index] if s.get("name") == SIGNING_STEP] + self.assertTrue( + earlier, + f"{workflow.name}:{job} writes in {name!r} with no {SIGNING_STEP!r} step before it", + ) + step = earlier[0] + self.assertEqual( + (step.get("env") or {}).get("PFB_BOT_SIGNING_KEY"), + "${{ secrets.PFB_BOT_SIGNING_KEY }}", + f"{workflow.name}:{job} does not consume the org signing-key secret", + ) + script = step.get("run") or "" + self.assertIn('[ -n "${PFB_BOT_SIGNING_KEY:-}" ]', script) + # The three lines in order and adjacent: an empty mode-600 file, then + # the key into it, then the path (never the key) into the step env. + self.assertIn( + 'install -m 600 /dev/null "$RUNNER_TEMP/pfb-bot-signing-key"\n' + 'printf \'%s\\n\' "$PFB_BOT_SIGNING_KEY" > "$RUNNER_TEMP/pfb-bot-signing-key"\n' + 'echo "PFB_BOT_SIGNING_KEY_FILE=$RUNNER_TEMP/pfb-bot-signing-key" >> "$GITHUB_ENV"\n', + script, + f"{workflow.name}:{job} does not provision the key file exactly as the writers expect", + ) - def test_publication_tests_disable_uv_cache_without_a_lockfile(self) -> None: - text = (ROOT / ".github" / "workflows" / "test.yml").read_text( - encoding="utf-8" - ) - setup = text[text.index("astral-sh/setup-uv@") :] - self.assertIn("enable-cache: false", setup) + def test_nothing_commits_as_the_generic_actions_identity(self) -> None: + # The identity lives in the scripts, which is where the retired one lived + # too, so the ban covers them and not just the workflows that call them. + for path in sorted(WORKFLOWS.glob("*.yml")) + [ROOT / script for script in WRITER_SCRIPTS]: + text = path.read_text(encoding="utf-8") + self.assertNotIn("github-actions[bot]", text, f"generic Actions identity in {path.name}") def _tagged_intake_script() -> str: From b8b34d91d2db2d2251a382deb997a4b14e7cfe46 Mon Sep 17 00:00:00 2001 From: Andre Brait Date: Wed, 2 Sep 2026 07:46:40 +0000 Subject: [PATCH 5/7] tests: widen writer discovery and pin the real CI quadrant Re-review nitpicks. The discovery regex was line-bound and knew only commit and `tag -a`, so a writer step that continued its arguments onto the next line or reached history through merge, cherry-pick, revert or am inherited nothing; both evasions now fail the suite. The signing examples ran with GITHUB_ACTIONS unset, so the quadrant every real workflow run occupies -- Actions set AND a provisioned key -- was only covered transitively; both now export it. --- tests/shell/publish_pkg_repo_spec.sh | 2 ++ tests/shell/render_pkg_site_spec.sh | 2 ++ tests/test_ingest_workflow.py | 6 ++++-- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/shell/publish_pkg_repo_spec.sh b/tests/shell/publish_pkg_repo_spec.sh index a27f285..b8da939 100644 --- a/tests/shell/publish_pkg_repo_spec.sh +++ b/tests/shell/publish_pkg_repo_spec.sh @@ -520,6 +520,8 @@ JSON It 'SSH-signs the catalogue commit when a workflow provisioned the signing key' ssh-keygen -q -t ed25519 -N '' -C pfblockerng-bot -f "${base}/bot-key" + # The real CI quadrant: Actions set AND a provisioned key. + export GITHUB_ACTIONS=true export PFB_BOT_SIGNING_KEY_FILE="${base}/bot-key" export FAKE_MODE=success export FAKE_TOUCHED=edge/ce-2.8 diff --git a/tests/shell/render_pkg_site_spec.sh b/tests/shell/render_pkg_site_spec.sh index 78106b9..71375b5 100644 --- a/tests/shell/render_pkg_site_spec.sh +++ b/tests/shell/render_pkg_site_spec.sh @@ -350,6 +350,8 @@ PY It 'r9b: SSH-signs the site commit when a workflow provisioned the signing key' ssh-keygen -q -t ed25519 -N '' -C pfblockerng-bot -f "${base}/bot-key" + # The real CI quadrant: Actions set AND a provisioned key. + export GITHUB_ACTIONS=true export PFB_BOT_SIGNING_KEY_FILE="${base}/bot-key" When run script "$script" The status should equal 0 diff --git a/tests/test_ingest_workflow.py b/tests/test_ingest_workflow.py index 79e001e..b1fd1b6 100644 --- a/tests/test_ingest_workflow.py +++ b/tests/test_ingest_workflow.py @@ -17,7 +17,9 @@ RENDER = WORKFLOWS / "render-site.yml" SIGNING_STEP = "Configure pfblockerng-bot signing" WRITER_SCRIPTS = ("scripts/publish-pkg-repo.sh", "scripts/render-pkg-site.sh") -_GIT_WRITE = re.compile(r"\bgit\b[^\n]*\b(?:commit|tag -a)\b") +# Every git verb that writes an object history can carry: a step reaching any of +# them is a writer, whichever line its arguments continue onto. +_GIT_WRITE = re.compile(r"\bgit\b[^\n]*\b(?:commit|tag|merge|cherry-pick|revert|am)\b") def _steps(workflow: Path, job: str) -> list[dict[str, object]]: @@ -32,7 +34,7 @@ def _writer_steps() -> list[tuple[Path, str, int, str]]: spec = yaml.safe_load(workflow.read_text(encoding="utf-8")) for job in (spec.get("jobs") or {}): for index, step in enumerate(_steps(workflow, job)): - run = str(step.get("run") or "") + run = str(step.get("run") or "").replace("\\\n", " ") writes = any(script in run for script in WRITER_SCRIPTS) or _GIT_WRITE.search(run) if writes: found.append((workflow, job, index, str(step.get("name") or f"step {index}"))) From db32f123a29d22169ccfd1b29af23d97697755e2 Mon Sep 17 00:00:00 2001 From: Andre Brait Date: Wed, 2 Sep 2026 07:59:16 +0000 Subject: [PATCH 6/7] tests: discover writers by shape, not by a listed pair Round-2 contract nitpicks. Discovery matched two named scripts and *.yml only, so a third writer script, a workflow saved as .yaml, or a step that cd's into scripts/ before running one inherited nothing. A step is now a writer when it runs any scripts/*.sh at all, both workflow spellings are read, the identity ban scans every script, and the provisioning test guards against an empty discovery instead of passing vacuously. .pytest_cache/ joins the .gitignore alongside the caches that already did. --- .gitignore | 1 + tests/test_ingest_workflow.py | 28 ++++++++++++++++++---------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/.gitignore b/.gitignore index 7a60b85..75c6182 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ __pycache__/ *.pyc +.pytest_cache/ diff --git a/tests/test_ingest_workflow.py b/tests/test_ingest_workflow.py index b1fd1b6..bc32d5a 100644 --- a/tests/test_ingest_workflow.py +++ b/tests/test_ingest_workflow.py @@ -16,12 +16,18 @@ INGEST = WORKFLOWS / "ingest.yml" RENDER = WORKFLOWS / "render-site.yml" SIGNING_STEP = "Configure pfblockerng-bot signing" -WRITER_SCRIPTS = ("scripts/publish-pkg-repo.sh", "scripts/render-pkg-site.sh") -# Every git verb that writes an object history can carry: a step reaching any of -# them is a writer, whichever line its arguments continue onto. +# A step is a writer when it runs any repo shell script (the two writers today, +# and whatever joins them) or reaches a git verb that writes history itself, +# whichever line its arguments continue onto. +_RUNS_REPO_SCRIPT = re.compile(r"\bscripts/[\w.-]+\.sh\b") _GIT_WRITE = re.compile(r"\bgit\b[^\n]*\b(?:commit|tag|merge|cherry-pick|revert|am)\b") +def _workflows() -> list[Path]: + # GitHub honours both spellings, so a writer saved as .yaml is still a writer. + return sorted(WORKFLOWS.glob("*.yml")) + sorted(WORKFLOWS.glob("*.yaml")) + + def _steps(workflow: Path, job: str) -> list[dict[str, object]]: spec = yaml.safe_load(workflow.read_text(encoding="utf-8")) return list(((spec.get("jobs") or {}).get(job) or {}).get("steps") or []) @@ -30,12 +36,12 @@ def _steps(workflow: Path, job: str) -> list[dict[str, object]]: def _writer_steps() -> list[tuple[Path, str, int, str]]: """Every (workflow, job, step index, step name) that can write a commit.""" found: list[tuple[Path, str, int, str]] = [] - for workflow in sorted(WORKFLOWS.glob("*.yml")): + for workflow in _workflows(): spec = yaml.safe_load(workflow.read_text(encoding="utf-8")) for job in (spec.get("jobs") or {}): for index, step in enumerate(_steps(workflow, job)): run = str(step.get("run") or "").replace("\\\n", " ") - writes = any(script in run for script in WRITER_SCRIPTS) or _GIT_WRITE.search(run) + writes = _RUNS_REPO_SCRIPT.search(run) or _GIT_WRITE.search(run) if writes: found.append((workflow, job, index, str(step.get("name") or f"step {index}"))) return found @@ -203,8 +209,8 @@ def test_site_source_push_renders_only_with_pkg_code(self) -> None: def test_every_writer_job_configures_signing_before_it_writes(self) -> None: """Discovered, not listed: a future writer job inherits the requirement. - A writer is any step that runs one of the commit-writing scripts or calls - git commit itself. Its job must configure the bot signing key in an + A writer is any step that runs a repo shell script or calls a git verb + that writes history. Its job must configure the bot signing key in an earlier step, because $GITHUB_ENV and $RUNNER_TEMP do not cross jobs. """ writers = _writer_steps() @@ -223,7 +229,9 @@ def test_every_writer_job_configures_signing_before_it_writes(self) -> None: ) def test_the_signing_step_provisions_the_key_it_promises(self) -> None: - for workflow, job, index, name in _writer_steps(): + writers = _writer_steps() + self.assertTrue(writers, "no writer step discovered; the finder is broken") + for workflow, job, index, name in writers: earlier = [s for s in _steps(workflow, job)[:index] if s.get("name") == SIGNING_STEP] self.assertTrue( earlier, @@ -249,8 +257,8 @@ def test_the_signing_step_provisions_the_key_it_promises(self) -> None: def test_nothing_commits_as_the_generic_actions_identity(self) -> None: # The identity lives in the scripts, which is where the retired one lived - # too, so the ban covers them and not just the workflows that call them. - for path in sorted(WORKFLOWS.glob("*.yml")) + [ROOT / script for script in WRITER_SCRIPTS]: + # too, so the ban covers every script and every workflow, not a listed few. + for path in _workflows() + sorted((ROOT / "scripts").glob("*.sh")): text = path.read_text(encoding="utf-8") self.assertNotIn("github-actions[bot]", text, f"generic Actions identity in {path.name}") From 574afbf5952338ef1cd33965c5898d589392564e Mon Sep 17 00:00:00 2001 From: Andre Brait Date: Wed, 2 Sep 2026 08:06:16 +0000 Subject: [PATCH 7/7] tests: verify the signature against the provisioned key CodeRabbit nitpick. The signing examples proved a signature existed, not that the provisioned key made it: a script signing with any key it liked passed. Each example now builds an allowed-signers file from the public half of the key it provisioned, under the bot's e-mail, and requires git to report the landed commit as a good signature by that signer. Signing with another key fails it. --- tests/shell/publish_pkg_repo_spec.sh | 6 ++++++ tests/shell/render_pkg_site_spec.sh | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/tests/shell/publish_pkg_repo_spec.sh b/tests/shell/publish_pkg_repo_spec.sh index b8da939..c3e01e6 100644 --- a/tests/shell/publish_pkg_repo_spec.sh +++ b/tests/shell/publish_pkg_repo_spec.sh @@ -534,6 +534,12 @@ JSON # config says commit.gpgsign false. landed="$(git_fixture -C "${base}/pkg-repo" cat-file commit HEAD)" The variable landed should include 'gpgsig -----BEGIN SSH SIGNATURE-----' + # And the signature is by THAT key: verified against an allowed-signers file + # naming the bot's e-mail with the public half of the provisioned key. + printf '293667935+pfblockerng-bot@users.noreply.github.com %s\n' \ + "$(cut -d' ' -f1,2 "${base}/bot-key.pub")" > "${base}/allowed-signers" + verified="$(git_fixture -C "${base}/pkg-repo" -c gpg.ssh.allowedSignersFile="${base}/allowed-signers" log -1 --format=%G?)" + The variable verified should equal 'G' author="$(git_fixture -C "${base}/pkg-repo" log -1 --format='%an <%ae>')" The variable author should equal 'pfblockerng-bot <293667935+pfblockerng-bot@users.noreply.github.com>' End diff --git a/tests/shell/render_pkg_site_spec.sh b/tests/shell/render_pkg_site_spec.sh index 71375b5..0c9c5a9 100644 --- a/tests/shell/render_pkg_site_spec.sh +++ b/tests/shell/render_pkg_site_spec.sh @@ -362,6 +362,12 @@ PY # config says commit.gpgsign false. landed="$(git_fixture -C "${base}/pkg-repo" cat-file commit HEAD)" The variable landed should include 'gpgsig -----BEGIN SSH SIGNATURE-----' + # And the signature is by THAT key: verified against an allowed-signers file + # naming the bot's e-mail with the public half of the provisioned key. + printf '293667935+pfblockerng-bot@users.noreply.github.com %s\n' \ + "$(cut -d' ' -f1,2 "${base}/bot-key.pub")" > "${base}/allowed-signers" + verified="$(git_fixture -C "${base}/pkg-repo" -c gpg.ssh.allowedSignersFile="${base}/allowed-signers" log -1 --format=%G?)" + The variable verified should equal 'G' author="$(git_fixture -C "${base}/pkg-repo" log -1 --format='%an <%ae>')" The variable author should equal 'pfblockerng-bot <293667935+pfblockerng-bot@users.noreply.github.com>' End