From 8f85864bea2d3f9763a102021344a4f6b11b59ce Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 00:07:57 +0400 Subject: [PATCH 1/4] Fix the CI-script exceptions outside the deploy pipeline - alembic-v1-check.yml: the folded pytest command becomes one line. - pr.yml: split apt-get update and the Redis install into two steps. - sync-cloud-run-secrets.yml: the master-only gate becomes require_git_ref.sh, tested, and checked before authentication. - cleanup_cloud_run_images.sh: add tests with a fake gcloud. - deploy_observability_collector.sh: group the GITHUB_OUTPUT writes (shellcheck SC2129); behaviour unchanged. Co-Authored-By: Claude Opus 5.5 --- .../scripts/deploy_observability_collector.sh | 8 +- .github/scripts/require_git_ref.sh | 16 +++ .github/workflows/alembic-v1-check.yml | 5 +- .github/workflows/pr.yml | 4 +- .github/workflows/sync-cloud-run-secrets.yml | 5 +- tests/unit/test_cloud_run_deploy_scripts.py | 4 +- tests/unit/test_workflow_guard_scripts.py | 125 ++++++++++++++++++ 7 files changed, 154 insertions(+), 13 deletions(-) create mode 100644 .github/scripts/require_git_ref.sh create mode 100644 tests/unit/test_workflow_guard_scripts.py diff --git a/.github/scripts/deploy_observability_collector.sh b/.github/scripts/deploy_observability_collector.sh index e2ec1f45d..49e8ea05c 100755 --- a/.github/scripts/deploy_observability_collector.sh +++ b/.github/scripts/deploy_observability_collector.sh @@ -140,9 +140,11 @@ if [[ -n "${public_members}" || "${invoker_iam_disabled}" == "true" ]]; then fi if [[ -n "${GITHUB_OUTPUT:-}" ]]; then - echo "revision=${revision}" >> "${GITHUB_OUTPUT}" - echo "service_url=${service_url}" >> "${GITHUB_OUTPUT}" - echo "image_uri=${image_uri}" >> "${GITHUB_OUTPUT}" + { + echo "revision=${revision}" + echo "service_url=${service_url}" + echo "image_uri=${image_uri}" + } >> "${GITHUB_OUTPUT}" fi printf 'Deployed collector revision %s at %s\n' "${revision}" "${service_url}" diff --git a/.github/scripts/require_git_ref.sh b/.github/scripts/require_git_ref.sh new file mode 100644 index 000000000..0de0bb311 --- /dev/null +++ b/.github/scripts/require_git_ref.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash + +# Fail unless the workflow runs from the expected git ref. +# +# Usage: require_git_ref.sh +# Reads GITHUB_REF, which GitHub Actions sets for every job. + +set -euo pipefail + +expected_ref="${1:?expected git ref is required}" +message="${2:?error message is required}" + +if [[ "${GITHUB_REF:-}" != "${expected_ref}" ]]; then + echo "::error::${message}" + exit 1 +fi diff --git a/.github/workflows/alembic-v1-check.yml b/.github/workflows/alembic-v1-check.yml index 6e9094b16..43a2fef7c 100644 --- a/.github/workflows/alembic-v1-check.yml +++ b/.github/workflows/alembic-v1-check.yml @@ -33,10 +33,7 @@ jobs: - name: Install dependencies run: make install - name: Test Alembic configuration and MySQL lifecycle - run: >- - python -m pytest tests/unit/data/test_alembic_baseline.py - tests/unit/data/test_v1_database_migration.py - tests/integration/test_alembic_mysql_lifecycle.py -q + run: python -m pytest tests/unit/data/test_alembic_baseline.py tests/unit/data/test_v1_database_migration.py tests/integration/test_alembic_mysql_lifecycle.py -q env: ALEMBIC_DATABASE_URL: mysql+pymysql://root:policyengine_test@127.0.0.1:3306/policyengine_alembic_test - name: Require database at all v1 heads diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 117b1ee5e..7c2fb0cbf 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -228,8 +228,10 @@ jobs: uses: actions/setup-python@v5 with: python-version: "3.12" + - name: Update package lists + run: sudo apt-get update - name: Install system dependencies - run: sudo apt-get update && sudo apt-get install -y redis-server + run: sudo apt-get install -y redis-server - name: Start Redis run: sudo systemctl start redis-server - name: Auth diff --git a/.github/workflows/sync-cloud-run-secrets.yml b/.github/workflows/sync-cloud-run-secrets.yml index c6b46f80a..4acd2400d 100644 --- a/.github/workflows/sync-cloud-run-secrets.yml +++ b/.github/workflows/sync-cloud-run-secrets.yml @@ -25,10 +25,7 @@ jobs: id-token: write steps: - name: Require master branch - if: github.ref != 'refs/heads/master' - run: | - echo "::error::Cloud Run secret sync must run from master." - exit 1 + run: bash .github/scripts/require_git_ref.sh refs/heads/master "Cloud Run secret sync must run from master." - name: Checkout repo uses: actions/checkout@v4 - name: GCP authentication diff --git a/tests/unit/test_cloud_run_deploy_scripts.py b/tests/unit/test_cloud_run_deploy_scripts.py index a22289af8..fec2010ab 100644 --- a/tests/unit/test_cloud_run_deploy_scripts.py +++ b/tests/unit/test_cloud_run_deploy_scripts.py @@ -2252,7 +2252,9 @@ def test_sync_cloud_run_secrets_workflow_is_manual_and_environment_gated(): assert "- production" in workflow assert "environment: ${{ inputs.deployment_environment }}" in workflow assert "id-token: write" in workflow - assert "github.ref != 'refs/heads/master'" in workflow + master_gate = "bash .github/scripts/require_git_ref.sh refs/heads/master" + assert master_gate in workflow + assert workflow.index(master_gate) < workflow.index("google-github-actions/auth@v2") assert "google-github-actions/auth@v2" in workflow assert ( 'workload_identity_provider: "${{ secrets.GCP_WORKLOAD_IDENTITY_PROVIDER }}"' diff --git a/tests/unit/test_workflow_guard_scripts.py b/tests/unit/test_workflow_guard_scripts.py new file mode 100644 index 000000000..9283cc15b --- /dev/null +++ b/tests/unit/test_workflow_guard_scripts.py @@ -0,0 +1,125 @@ +"""Tests for small workflow scripts: require_git_ref.sh and cleanup_cloud_run_images.sh.""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +REPO = Path(__file__).resolve().parents[2] +REQUIRE_GIT_REF = ".github/scripts/require_git_ref.sh" +CLEANUP_IMAGES = ".github/scripts/cleanup_cloud_run_images.sh" + + +def _run( + script: str, env: dict[str, str], *args: str +) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["bash", script, *args], + cwd=REPO, + env={"HOME": os.environ.get("HOME", ""), "PATH": os.environ["PATH"], **env}, + text=True, + capture_output=True, + check=False, + ) + + +def test_require_git_ref_passes_on_the_expected_ref() -> None: + result = _run( + REQUIRE_GIT_REF, + {"GITHUB_REF": "refs/heads/master"}, + "refs/heads/master", + "must run from master.", + ) + + assert result.returncode == 0, result.stderr + assert result.stdout == "" + + +def test_require_git_ref_fails_with_an_annotation_on_another_ref() -> None: + result = _run( + REQUIRE_GIT_REF, + {"GITHUB_REF": "refs/heads/feature"}, + "refs/heads/master", + "Cloud Run secret sync must run from master.", + ) + + assert result.returncode == 1 + assert result.stdout == "::error::Cloud Run secret sync must run from master.\n" + + +def _fake_gcloud(tmp_path: Path, rows: list[str], *, delete_exit: int = 0) -> Path: + """A gcloud that lists ``rows`` and records every other call.""" + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + listing = tmp_path / "listing.txt" + listing.write_text("".join(f"{row}\n" for row in rows)) + calls = tmp_path / "calls.log" + gcloud = bin_dir / "gcloud" + gcloud.write_text( + "#!/usr/bin/env bash\n" + 'if [[ "$1 $2 $3 $4" == "artifacts docker images list" ]]; then\n' + f' cat "{listing}"\n' + " exit 0\n" + "fi\n" + f'echo "$*" >> "{calls}"\n' + f"exit {delete_exit}\n" + ) + gcloud.chmod(0o755) + return bin_dir + + +IMAGES = [ + "2026-01-01T00:00:00Z|repo/api|sha256:oldest", + "2026-03-01T00:00:00Z|repo/api|sha256:newest", + "2026-02-01T00:00:00Z|repo/api|sha256:middle", +] + + +def _cleanup( + tmp_path: Path, rows: list[str], **env: str +) -> tuple[subprocess.CompletedProcess[str], list[str]]: + bin_dir = _fake_gcloud(tmp_path, rows, delete_exit=int(env.pop("DELETE_EXIT", "0"))) + result = _run( + CLEANUP_IMAGES, + {"PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", **env}, + ) + calls_file = tmp_path / "calls.log" + calls = calls_file.read_text().splitlines() if calls_file.exists() else [] + return result, calls + + +def test_cleanup_deletes_all_but_the_newest_images(tmp_path: Path) -> None: + result, calls = _cleanup(tmp_path, IMAGES, KEEP="1") + + assert result.returncode == 0, result.stderr + assert calls == [ + "artifacts docker images delete repo/api@sha256:middle --delete-tags --quiet", + "artifacts docker images delete repo/api@sha256:oldest --delete-tags --quiet", + ] + assert "Deleted 2/2 image(s)." in result.stdout + + +def test_cleanup_dry_run_deletes_nothing(tmp_path: Path) -> None: + result, calls = _cleanup(tmp_path, IMAGES, KEEP="1", DRY_RUN="1") + + assert result.returncode == 0, result.stderr + assert calls == [] + assert "[dry-run] would delete repo/api@sha256:middle" in result.stdout + assert "Deleted" not in result.stdout + + +def test_cleanup_keeps_everything_within_the_limit(tmp_path: Path) -> None: + result, calls = _cleanup(tmp_path, IMAGES) + + assert result.returncode == 0, result.stderr + assert calls == [] + assert "Deleting: 0" in result.stdout + + +def test_cleanup_counts_only_successful_deletions(tmp_path: Path) -> None: + result, calls = _cleanup(tmp_path, IMAGES, KEEP="1", DELETE_EXIT="1") + + assert result.returncode == 0, result.stderr + assert len(calls) == 2 + assert "Deleted 0/2 image(s)." in result.stdout From ccb237c407e743a4b23e96226c42fbd40ce4ec15 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 00:07:57 +0400 Subject: [PATCH 2/4] Shrink the CI-script baselines and enforce two CI rules The multi-line baseline drops from 10 to 7 (the deploy pipeline remains). The untested-script and shellcheck baselines are empty, so api.ci.workflow-scripts-tested and api.ci.shell-scripts-pass-shellcheck move from ratcheted to enforced. Co-Authored-By: Claude Opus 5.5 --- changelog.d/ci-scripts-exceptions.changed.md | 1 + docs/generated/rules.md | 14 ++++++-------- rules/baselines/ci-multiline-run.txt | 3 --- rules/baselines/ci-shellcheck.txt | 1 - rules/baselines/ci-untested-scripts.txt | 1 - rules/registry.toml | 12 +++--------- 6 files changed, 10 insertions(+), 22 deletions(-) create mode 100644 changelog.d/ci-scripts-exceptions.changed.md diff --git a/changelog.d/ci-scripts-exceptions.changed.md b/changelog.d/ci-scripts-exceptions.changed.md new file mode 100644 index 000000000..78ea70b32 --- /dev/null +++ b/changelog.d/ci-scripts-exceptions.changed.md @@ -0,0 +1 @@ +Turned the remaining multi-line steps outside the deploy pipeline into single-line steps or a tested script, added tests for the Cloud Run image cleanup script, and fixed a shellcheck finding in the observability collector deploy script. diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 458f36e46..8663517ab 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -8,7 +8,7 @@ Every rule that governs `policyengine-api` is listed here with how it is enforce | Tier | enforced | ratcheted | advisory | doc-only | Total | | --- | ---: | ---: | ---: | ---: | ---: | -| mechanical | 37 | 3 | 2 | 12 | 54 | +| mechanical | 39 | 1 | 2 | 12 | 54 | | metric | 0 | 0 | 0 | 0 | 0 | | judgment | 0 | 0 | 0 | 0 | 0 | @@ -64,9 +64,7 @@ Every rule that governs `policyengine-api` is listed here with how it is enforce ### Ratcheted (existing violations still tolerated) -- `api.ci.multiline-run-is-script`: 10 remaining in `rules/baselines/ci-multiline-run.txt` -- `api.ci.shell-scripts-pass-shellcheck`: 1 remaining in `rules/baselines/ci-shellcheck.txt` -- `api.ci.workflow-scripts-tested`: 1 remaining in `rules/baselines/ci-untested-scripts.txt` +- `api.ci.multiline-run-is-script`: 7 remaining in `rules/baselines/ci-multiline-run.txt` ### Exceptions and review dates @@ -328,16 +326,16 @@ flowchart LR | Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | | --- | --- | --- | --- | --- | --- | --- | --- | | `api.ci.multiline-run-is-script` | A GitHub Actions run: block holds one line of code and no chained commands (&&, \|\|, ;); anything more lives in a tested script under .github/scripts/, called from a single-line step. | mechanical | ratcheted | yes | quality guard `ci-scripts` | `pr.yml:quality-guards` | yes | -| `api.ci.shell-scripts-pass-shellcheck` | Every shell script under .github/scripts/ passes shellcheck. | mechanical | ratcheted | yes | quality guard `ci-scripts` | `pr.yml:quality-guards` | yes | -| `api.ci.workflow-scripts-tested` | Every shell script a workflow calls is run by at least one test. | mechanical | ratcheted | yes | quality guard `ci-scripts` | `pr.yml:quality-guards` | yes | +| `api.ci.shell-scripts-pass-shellcheck` | Every shell script under .github/scripts/ passes shellcheck. | mechanical | enforced | yes | quality guard `ci-scripts` | `pr.yml:quality-guards` | yes | +| `api.ci.workflow-scripts-tested` | Every shell script a workflow calls is run by at least one test. | mechanical | enforced | yes | quality guard `ci-scripts` | `pr.yml:quality-guards` | yes | ```mermaid flowchart LR r_api_ci_multiline_run_is_script("api.ci.multiline-run-is-script (ratcheted)") e_quality_guard_ci_scripts["quality guard ci-scripts"] j_CI_pr_yml_quality_guards["CI pr.yml:quality-guards"] - r_api_ci_shell_scripts_pass_shellcheck("api.ci.shell-scripts-pass-shellcheck (ratcheted)") - r_api_ci_workflow_scripts_tested("api.ci.workflow-scripts-tested (ratcheted)") + r_api_ci_shell_scripts_pass_shellcheck("api.ci.shell-scripts-pass-shellcheck (enforced)") + r_api_ci_workflow_scripts_tested("api.ci.workflow-scripts-tested (enforced)") r_api_ci_multiline_run_is_script --> e_quality_guard_ci_scripts e_quality_guard_ci_scripts --> j_CI_pr_yml_quality_guards r_api_ci_shell_scripts_pass_shellcheck --> e_quality_guard_ci_scripts diff --git a/rules/baselines/ci-multiline-run.txt b/rules/baselines/ci-multiline-run.txt index 6013bfefa..ec7719679 100644 --- a/rules/baselines/ci-multiline-run.txt +++ b/rules/baselines/ci-multiline-run.txt @@ -1,6 +1,4 @@ # Tolerated existing violations. This list may only shrink. -.github/workflows/alembic-v1-check.yml :: mysql-lifecycle :: Test Alembic configuration and MySQL lifecycle -.github/workflows/pr.yml :: test :: Install system dependencies .github/workflows/push.yml :: deploy-cloud-run-candidate :: Compute Cloud Run candidate metadata .github/workflows/push.yml :: deploy-cloud-run-candidate :: Restore previous Cloud Run production revision .github/workflows/push.yml :: deploy-cloud-run-staging :: Compute Cloud Run staging metadata @@ -8,4 +6,3 @@ .github/workflows/push.yml :: deploy-cloud-run-staging :: Run release tests .github/workflows/push.yml :: promote-cloud-run-staging :: Restore previous Cloud Run staging revision .github/workflows/push.yml :: versioning :: Build changelog -.github/workflows/sync-cloud-run-secrets.yml :: sync-cloud-run-secrets :: Require master branch diff --git a/rules/baselines/ci-shellcheck.txt b/rules/baselines/ci-shellcheck.txt index de7e00a3d..924c0983a 100644 --- a/rules/baselines/ci-shellcheck.txt +++ b/rules/baselines/ci-shellcheck.txt @@ -1,2 +1 @@ # Tolerated existing violations. This list may only shrink. -.github/scripts/deploy_observability_collector.sh diff --git a/rules/baselines/ci-untested-scripts.txt b/rules/baselines/ci-untested-scripts.txt index b2651e812..924c0983a 100644 --- a/rules/baselines/ci-untested-scripts.txt +++ b/rules/baselines/ci-untested-scripts.txt @@ -1,2 +1 @@ # Tolerated existing violations. This list may only shrink. -.github/scripts/cleanup_cloud_run_images.sh diff --git a/rules/registry.toml b/rules/registry.toml index 1a1994188..e53acc471 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -771,7 +771,7 @@ job = "quality-guards" invocation = "run_quality_guards.py" [rule.baseline] file = "rules/baselines/ci-multiline-run.txt" -count = 10 +count = 7 [rule.proof] bad_fixture = "scripts/rules/tests/fixtures/ci_scripts/multiline_bad" good_fixture = "scripts/rules/tests/fixtures/ci_scripts/multiline_good" @@ -787,7 +787,7 @@ statement = "Every shell script a workflow calls is run by at least one test." rationale = "docs/engineering/skills/ci-workflows.md#testing-a-workflow-script" scope = [".github/scripts/*.sh"] tier = "mechanical" -status = "ratcheted" +status = "enforced" autofix = false [rule.enforcer] kind = "quality-guard" @@ -796,9 +796,6 @@ name = "ci-scripts" workflow = ".github/workflows/pr.yml" job = "quality-guards" invocation = "run_quality_guards.py" -[rule.baseline] -file = "rules/baselines/ci-untested-scripts.txt" -count = 1 [rule.proof] bad_fixture = "scripts/rules/tests/fixtures/ci_scripts/untested_bad" good_fixture = "scripts/rules/tests/fixtures/ci_scripts/untested_good" @@ -814,7 +811,7 @@ statement = "Every shell script under .github/scripts/ passes shellcheck." rationale = "docs/engineering/skills/ci-workflows.md#writing-a-workflow-script" scope = [".github/scripts/*.sh"] tier = "mechanical" -status = "ratcheted" +status = "enforced" autofix = false [rule.enforcer] kind = "quality-guard" @@ -823,9 +820,6 @@ name = "ci-scripts" workflow = ".github/workflows/pr.yml" job = "quality-guards" invocation = "run_quality_guards.py" -[rule.baseline] -file = "rules/baselines/ci-shellcheck.txt" -count = 1 [rule.proof] bad_fixture = "scripts/rules/tests/fixtures/ci_scripts/shellcheck/bad_script.sh" good_fixture = "scripts/rules/tests/fixtures/ci_scripts/shellcheck/good_script.sh" From 35f87704aebd2bc5a989c6d56c6b33b68c254ae0 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 00:11:27 +0400 Subject: [PATCH 3/4] Attribute ci-scripts findings to enforced rules Once a CI-script rule is enforced it has no baseline, so its findings were reported as [registry]. Fall back to the rule whose id ends in the check's name (.ci.multiline-run-is-script, ...). Co-Authored-By: Claude Opus 5.5 --- scripts/rules/ci_scripts.py | 25 +++++++++++++++++++++---- scripts/rules/tests/test_ci_scripts.py | 16 ++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/scripts/rules/ci_scripts.py b/scripts/rules/ci_scripts.py index 02b5f3f47..5a0a6a066 100644 --- a/scripts/rules/ci_scripts.py +++ b/scripts/rules/ci_scripts.py @@ -25,6 +25,13 @@ REPO_ROOT = Path(__file__).resolve().parents[2] SEPARATOR = " :: " +# Rule ids end with these names (``.ci.``). Used to attribute a +# finding to its rule once the rule is enforced and has no baseline left. +RULE_NAMES = { + "multiline": "multiline-run-is-script", + "untested": "workflow-scripts-tested", + "shellcheck": "shell-scripts-pass-shellcheck", +} @dataclass @@ -118,10 +125,15 @@ def _baseline(root: Path, path: str) -> set[str]: } -def _rule_for(registry: Registry, baseline: str) -> str | None: +def _rule_for(registry: Registry, baseline: str, check: str) -> str | None: for rule in registry.rules: if rule.baseline is not None and rule.baseline.file == baseline: return rule.id + for rule in registry.rules: + if rule.enforcer.get("name") == "ci-scripts" and rule.id.endswith( + f".ci.{RULE_NAMES[check]}" + ): + return rule.id return None @@ -130,6 +142,7 @@ def compare(registry: Registry, findings: Findings, root: Path) -> list[Violatio assert config is not None checks = ( ( + "multiline", "multi-line run: block", findings.multiline, config.multiline_baseline, @@ -137,12 +150,14 @@ def compare(registry: Registry, findings: Findings, root: Path) -> list[Violatio f"{config.scripts_dir} and call it from a single-line run:", ), ( + "untested", "untested workflow script", findings.untested, config.untested_baseline, "add a test that runs the script (see docs/engineering/skills/ci-workflows.md)", ), ( + "shellcheck", "shellcheck finding in", findings.shellcheck, config.shellcheck_baseline, @@ -150,10 +165,10 @@ def compare(registry: Registry, findings: Findings, root: Path) -> list[Violatio ), ) violations = [] - for label, found, baseline_path, fix in checks: + for check, label, found, baseline_path, fix in checks: if found is None: continue - rule_id = _rule_for(registry, baseline_path) + rule_id = _rule_for(registry, baseline_path, check) baseline = _baseline(root, baseline_path) for entry in sorted(found - baseline): violations.append(Violation(rule_id, "ci-scripts", f"{label} {entry}", fix)) @@ -199,7 +214,9 @@ def run_check(root: Path = REPO_ROOT) -> tuple[list[Violation], list[str]]: if findings.shellcheck is None and os.environ.get("GITHUB_ACTIONS"): violations.append( Violation( - _rule_for(registry, registry.ci_scripts.shellcheck_baseline), + _rule_for( + registry, registry.ci_scripts.shellcheck_baseline, "shellcheck" + ), "ci-scripts", "shellcheck is not installed in this CI job", "install it in the job (pip install shellcheck-py==)", diff --git a/scripts/rules/tests/test_ci_scripts.py b/scripts/rules/tests/test_ci_scripts.py index 13aef4fa3..c05a4bc3f 100644 --- a/scripts/rules/tests/test_ci_scripts.py +++ b/scripts/rules/tests/test_ci_scripts.py @@ -188,6 +188,22 @@ def test_new_finding_fails_with_its_rule_id(self) -> None: messages[0], ) + def test_enforced_rule_without_baseline_keeps_its_id(self) -> None: + findings = Findings(multiline=set(), untested={"x.sh"}, shellcheck=set()) + registry, _ = load_text( + REGISTRY.replace("ex.ci.multiline", "ex.ci.workflow-scripts-tested") + .replace('status = "ratcheted"', 'status = "enforced"') + .replace( + '[rule.baseline]\nfile = "rules/baselines/ci-multiline-run.txt"\ncount = 1\n', + "", + ) + ) + assert registry is not None + messages = [str(v) for v in compare(registry, findings, self.root)] + self.assertTrue( + messages[-1].startswith("[ex.ci.workflow-scripts-tested]"), messages + ) + def test_fixed_finding_must_leave_the_baseline(self) -> None: messages = self.messages(set()) self.assertIn( From 197b28e4ac68d8468ac934647903f53ef36bd389 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 15 Oct 2026 00:53:05 +0400 Subject: [PATCH 4/4] Run the master-branch check after checking out the repository 8f85864b replaced the inline, if-guarded "Require master branch" step in sync-cloud-run-secrets.yml with a call to .github/scripts/require_git_ref.sh, but left the step ahead of actions/checkout. The script is not in the workspace until the checkout runs, so every run of the Cloud Run secret sync, including runs from master, would have failed with "No such file or directory" before syncing anything. The gate now runs directly after the checkout and still before GCP authentication, so a run from any other branch stops before it can authenticate. The workflow test only asserted that the gate comes before authentication, which the broken ordering satisfied. It now also asserts that the checkout comes before the gate; against the old ordering it fails with "assert 765 < 625". Co-Authored-By: Claude Opus 5.5 --- .github/workflows/sync-cloud-run-secrets.yml | 4 ++-- tests/unit/test_cloud_run_deploy_scripts.py | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/.github/workflows/sync-cloud-run-secrets.yml b/.github/workflows/sync-cloud-run-secrets.yml index 4acd2400d..9ac877456 100644 --- a/.github/workflows/sync-cloud-run-secrets.yml +++ b/.github/workflows/sync-cloud-run-secrets.yml @@ -24,10 +24,10 @@ jobs: contents: read id-token: write steps: - - name: Require master branch - run: bash .github/scripts/require_git_ref.sh refs/heads/master "Cloud Run secret sync must run from master." - name: Checkout repo uses: actions/checkout@v4 + - name: Require master branch + run: bash .github/scripts/require_git_ref.sh refs/heads/master "Cloud Run secret sync must run from master." - name: GCP authentication uses: "google-github-actions/auth@v2" with: diff --git a/tests/unit/test_cloud_run_deploy_scripts.py b/tests/unit/test_cloud_run_deploy_scripts.py index fec2010ab..2153ec036 100644 --- a/tests/unit/test_cloud_run_deploy_scripts.py +++ b/tests/unit/test_cloud_run_deploy_scripts.py @@ -2252,10 +2252,16 @@ def test_sync_cloud_run_secrets_workflow_is_manual_and_environment_gated(): assert "- production" in workflow assert "environment: ${{ inputs.deployment_environment }}" in workflow assert "id-token: write" in workflow + checkout_step = "uses: actions/checkout@v4" master_gate = "bash .github/scripts/require_git_ref.sh refs/heads/master" + gcp_authentication_step = "google-github-actions/auth@v2" + assert checkout_step in workflow assert master_gate in workflow - assert workflow.index(master_gate) < workflow.index("google-github-actions/auth@v2") - assert "google-github-actions/auth@v2" in workflow + assert gcp_authentication_step in workflow + # The gate runs a script from the repository, so it needs the checkout + # first, and it must pass before anything authenticates to GCP. + assert workflow.index(checkout_step) < workflow.index(master_gate) + assert workflow.index(master_gate) < workflow.index(gcp_authentication_step) assert ( 'workload_identity_provider: "${{ secrets.GCP_WORKLOAD_IDENTITY_PROVIDER }}"' in workflow