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/.github/workflows/test.yml b/.github/workflows/test.yml index d298bb5..e3b3674 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -16,18 +16,20 @@ 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 + 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..75c6182 --- /dev/null +++ b/.gitignore @@ -0,0 +1,3 @@ +__pycache__/ +*.pyc +.pytest_cache/ diff --git a/scripts/publish-pkg-repo.sh b/scripts/publish-pkg-repo.sh index e7dbb41..8ac29cb 100755 --- a/scripts/publish-pkg-repo.sh +++ b/scripts/publish-pkg-repo.sh @@ -679,13 +679,22 @@ 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). + # 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="github-actions[bot]" \ - -c user.email="github-actions[bot]@users.noreply.github.com" \ + -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 diff --git a/scripts/render-pkg-site.sh b/scripts/render-pkg-site.sh index 4aecc33..658a752 100755 --- a/scripts/render-pkg-site.sh +++ b/scripts/render-pkg-site.sh @@ -121,12 +121,22 @@ 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. + # 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="github-actions[bot]" \ - -c user.email="github-actions[bot]@users.noreply.github.com" \ + -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 diff --git a/tests/shell/publish_pkg_repo_spec.sh b/tests/shell/publish_pkg_repo_spec.sh index 3602b3c..c3e01e6 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 --- @@ -513,8 +514,48 @@ 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 '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 + 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-----' + # 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 + + 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' diff --git a/tests/shell/render_pkg_site_spec.sh b/tests/shell/render_pkg_site_spec.sh index e80e0ec..0c9c5a9 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 --- @@ -341,8 +342,43 @@ 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 + + # --- 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" + # 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 + 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-----' + # 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 + + 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 ecacb8f..bc32d5a 100644 --- a/tests/test_ingest_workflow.py +++ b/tests/test_ingest_workflow.py @@ -3,14 +3,48 @@ import hashlib import json import os +import re import subprocess import textwrap 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" +# 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 []) + + +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 _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 = _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 class IngestionWorkflowContractTests(unittest.TestCase): @@ -172,6 +206,62 @@ 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_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 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() + 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( + signing[0], + index, + f"{workflow.name}:{job} configures signing after it writes in {name!r}", + ) + + def test_the_signing_step_provisions_the_key_it_promises(self) -> None: + 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, + 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_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 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}") + def _tagged_intake_script() -> str: text = INGEST.read_text(encoding="utf-8")