From 50f206d2980831374ed15d6969afb18996d9704a Mon Sep 17 00:00:00 2001 From: kazuki nakai Date: Sun, 26 Jul 2026 18:31:58 +0900 Subject: [PATCH] feat: allow intentional protected-workflow changes via PR label A workflow declaring repo-quality-gate/db-tests as of the base revision is permanently unimprovable through the normal PR lane: detect_native_gates.sh hard-fails any PR that touches it, with no escape hatch. This bit a real case in agiletec/agiletec: db-tests.yml skips its pgTAP job on every main push because its own change-detection diffs github.event.before under fetch-depth: 1, which never sees the merge's files. Fixing that requires editing the protected workflow, which the guard forbids. Add a deliberate, auditable escape hatch: the PR label allow-protected-workflow-change lets a protected-workflow diff through and prints a notice naming each changed path, instead of failing. Absent the label, behavior is unchanged. The label is threaded in from the pull_request event context (github.event.pull_request.labels.*.name) via the PR_LABELS env var, never from repository file content, so a head cannot self-authorize by editing tracked files. merge_group events carry no labels, so PR_LABELS is empty there and strict behavior still applies. --- .github/scripts/detect_native_gates.sh | 32 ++++++-- .github/workflows/quality-gate.yml | 4 + tests/test_native_required_gates.py | 106 ++++++++++++++++++++++++- 3 files changed, 136 insertions(+), 6 deletions(-) diff --git a/.github/scripts/detect_native_gates.sh b/.github/scripts/detect_native_gates.sh index c4b4255..43d6f3e 100755 --- a/.github/scripts/detect_native_gates.sh +++ b/.github/scripts/detect_native_gates.sh @@ -31,6 +31,18 @@ revision_has_supabase() { git -C "$repository" cat-file -e "$1:supabase/config.toml" 2>/dev/null } +# PR_LABELS carries labels from the pull_request event context (never from +# repository file content), so a head cannot self-authorize by editing +# tracked files. merge_group events have no labels, so this is empty there +# and the strict behavior below applies unchanged. +has_allow_label=0 +for label in ${PR_LABELS:-}; do + if [ "$label" = "allow-protected-workflow-change" ]; then + has_allow_label=1 + break + fi +done + for revision in "$base" "$head"; do if [ -z "$revision" ] || ! revision_exists "$revision"; then printf 'Cannot resolve required-gate revision: %s\n' "${revision:-}" >&2 @@ -40,19 +52,29 @@ done # A gate already required by the protected base revision is itself protected. # Allowing its workflow file to change in the same head would let that head -# manufacture a passing check with weaker behavior. +# manufacture a passing check with weaker behavior. The +# allow-protected-workflow-change PR label is a deliberate, auditable escape +# hatch for intentionally strengthening a protected workflow. while IFS= read -r -d '' path; do if ! git -C "$repository" diff --quiet "$base" "$head" -- "$path"; then - printf 'Base-required workflow was modified or removed: %s\n' "$path" >&2 - exit 1 + if [ "$has_allow_label" -eq 1 ]; then + printf 'Protected workflow changed under allow-protected-workflow-change: %s\n' "$path" + else + printf 'Base-required workflow was modified or removed: %s\n' "$path" >&2 + exit 1 + fi fi done < <(workflows_with_job "$base" repo-quality-gate) if revision_has_supabase "$base"; then while IFS= read -r -d '' path; do if ! git -C "$repository" diff --quiet "$base" "$head" -- "$path"; then - printf 'Base-required workflow was modified or removed: %s\n' "$path" >&2 - exit 1 + if [ "$has_allow_label" -eq 1 ]; then + printf 'Protected workflow changed under allow-protected-workflow-change: %s\n' "$path" + else + printf 'Base-required workflow was modified or removed: %s\n' "$path" >&2 + exit 1 + fi fi done < <(workflows_with_job "$base" db-tests) fi diff --git a/.github/workflows/quality-gate.yml b/.github/workflows/quality-gate.yml index e7e105b..163fa53 100644 --- a/.github/workflows/quality-gate.yml +++ b/.github/workflows/quality-gate.yml @@ -102,6 +102,10 @@ jobs: env: BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }} HEAD_SHA: ${{ github.sha }} + # From the pull_request event context only, never repository file + # content, so a head cannot self-authorize by editing tracked files. + # merge_group events carry no labels, so this is empty there. + PR_LABELS: ${{ join(github.event.pull_request.labels.*.name, ' ') }} run: bash .quality-gate-source/.github/scripts/detect_native_gates.sh . "$BASE_SHA" "$HEAD_SHA" node-ci: diff --git a/tests/test_native_required_gates.py b/tests/test_native_required_gates.py index ee1af2a..914c951 100644 --- a/tests/test_native_required_gates.py +++ b/tests/test_native_required_gates.py @@ -53,9 +53,17 @@ def commit(self, message: str) -> str: capture_output=True, ).stdout.strip() - def detect_result(self, base: str, head: str) -> subprocess.CompletedProcess[str]: + def detect_result( + self, base: str, head: str, pr_labels: str | None = None + ) -> subprocess.CompletedProcess[str]: + environment = os.environ.copy() + if pr_labels is not None: + environment["PR_LABELS"] = pr_labels + else: + environment.pop("PR_LABELS", None) return subprocess.run( ["bash", str(DETECT), str(self.repository), base, head], + env=environment, text=True, capture_output=True, check=False, @@ -97,6 +105,102 @@ def test_modifying_base_gate_workflow_is_rejected(self) -> None: self.assertNotEqual(result.returncode, 0) self.assertIn(".github/workflows/quality.yml", result.stderr) + def test_modifying_base_gate_workflow_without_label_still_rejected(self) -> None: + self.write( + ".github/workflows/quality.yml", + "jobs:\n repo-quality-gate:\n runs-on: ubuntu-latest\n", + ) + base = self.commit("base native aggregate") + self.write( + ".github/workflows/quality.yml", + "jobs:\n repo-quality-gate:\n runs-on: self-hosted\n", + ) + head = self.commit("attempt to modify protected workflow") + + result = self.detect_result(base, head, pr_labels="some-other-label") + self.assertNotEqual(result.returncode, 0) + self.assertIn("Base-required workflow was modified or removed", result.stderr) + self.assertIn(".github/workflows/quality.yml", result.stderr) + + def test_modifying_base_gate_workflow_with_allow_label_succeeds(self) -> None: + self.write( + ".github/workflows/quality.yml", + "jobs:\n repo-quality-gate:\n runs-on: ubuntu-latest\n", + ) + base = self.commit("base native aggregate") + self.write( + ".github/workflows/quality.yml", + "jobs:\n repo-quality-gate:\n runs-on: self-hosted\n", + ) + head = self.commit("intentionally strengthen protected workflow") + + result = self.detect_result( + base, head, pr_labels="allow-protected-workflow-change" + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn( + "Protected workflow changed under allow-protected-workflow-change: " + ".github/workflows/quality.yml", + result.stdout, + ) + + def test_unrelated_workflow_change_unaffected_by_label(self) -> None: + self.write( + ".github/workflows/quality.yml", + "jobs:\n repo-quality-gate:\n runs-on: ubuntu-latest\n", + ) + self.write( + ".github/workflows/other.yml", + "jobs:\n other-job:\n runs-on: ubuntu-latest\n", + ) + base = self.commit("base with protected and unrelated workflows") + self.write( + ".github/workflows/other.yml", + "jobs:\n other-job:\n runs-on: self-hosted\n", + ) + head = self.commit("modify unrelated workflow only") + + for pr_labels in (None, "allow-protected-workflow-change"): + with self.subTest(pr_labels=pr_labels): + result = self.detect_result(base, head, pr_labels=pr_labels) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertNotIn("other.yml", result.stderr) + + def test_allow_label_has_no_effect_without_protected_workflow_change(self) -> None: + self.write( + ".github/workflows/quality.yml", + "jobs:\n repo-quality-gate:\n runs-on: ubuntu-latest\n", + ) + base = self.commit("base native aggregate") + self.write("README.md", "unrelated change\n") + head = self.commit("touch unrelated file") + + result = self.detect_result( + base, head, pr_labels="allow-protected-workflow-change" + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertNotIn("Protected workflow changed", result.stdout) + + def test_merge_group_style_empty_labels_keeps_strict_behavior(self) -> None: + self.write( + ".github/workflows/quality.yml", + "jobs:\n repo-quality-gate:\n runs-on: ubuntu-latest\n", + ) + base = self.commit("base native aggregate") + self.write( + ".github/workflows/quality.yml", + "jobs:\n repo-quality-gate:\n runs-on: self-hosted\n", + ) + head = self.commit("attempt to modify protected workflow in merge_group") + + # merge_group events carry no labels, so PR_LABELS is unset/empty and + # the strict rejection must apply exactly as without the label. + for pr_labels in (None, ""): + with self.subTest(pr_labels=pr_labels): + result = self.detect_result(base, head, pr_labels=pr_labels) + self.assertNotEqual(result.returncode, 0) + self.assertIn(".github/workflows/quality.yml", result.stderr) + def test_adding_gate_in_head_activates_requirement(self) -> None: self.write("README.md", "initial\n") base = self.commit("base")