Resolve the CI-script exceptions outside the deploy pipeline

.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}"

.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 <expected-ref> <error-message>
+# 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

.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

.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

.github/workflows/sync-cloud-run-secrets.yml

@@ -24,13 +24,10 @@ jobs:
       contents: read
       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
       - 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:

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.

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

rules/baselines/ci-multiline-run.txt

@@ -1,11 +1,8 @@
 # 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
 .github/workflows/push.yml :: deploy-cloud-run-staging :: Install release test dependencies
 .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

rules/baselines/ci-shellcheck.txt

@@ -1,2 +1 @@
 # Tolerated existing violations. This list may only shrink.
-.github/scripts/deploy_observability_collector.sh

rules/baselines/ci-untested-scripts.txt

@@ -1,2 +1 @@
 # Tolerated existing violations. This list may only shrink.
-.github/scripts/cleanup_cloud_run_images.sh

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"

scripts/rules/ci_scripts.py

@@ -25,6 +25,13 @@
 
 REPO_ROOT = Path(__file__).resolve().parents[2]
 SEPARATOR = " :: "
+# Rule ids end with these names (``<repo>.ci.<name>``). 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,30 +142,33 @@ 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,
             "move the commands into a tested script under "
             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,
             "fix what `shellcheck <script>` reports",
         ),
     )
     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==<version>)",

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(

tests/unit/test_cloud_run_deploy_scripts.py

@@ -2252,8 +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
-    assert "github.ref != 'refs/heads/master'" in workflow
-    assert "google-github-actions/auth@v2" 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 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