diff --git a/.github/workflows/apply-rulesets-tests.yml b/.github/workflows/apply-rulesets-tests.yml new file mode 100644 index 00000000..5ac5e7eb --- /dev/null +++ b/.github/workflows/apply-rulesets-tests.yml @@ -0,0 +1,59 @@ +# Quality gates for the apply-rulesets script and its unit tests. +# +# Triggered on any PR that touches: +# - scripts/apply-rulesets.sh +# - test/scripts/apply-rulesets/** +# +# Gates (all must pass before merge): +# 1. shellcheck — static analysis for the applier script +# 2. bats — unit tests pinning that the detection-based builder does not +# inject the non-codified `Dev-Lead Agent / dev-lead` context +# (it must agree with standards/rulesets/code-quality.json) + +name: Apply Rulesets Tests + +on: + pull_request: + paths: + - 'scripts/apply-rulesets.sh' + - 'test/scripts/apply-rulesets/**' + - '.github/workflows/apply-rulesets-tests.yml' + push: + branches: + - main + paths: + - 'scripts/apply-rulesets.sh' + - 'test/scripts/apply-rulesets/**' + - '.github/workflows/apply-rulesets-tests.yml' + +permissions: + contents: read + +concurrency: + group: apply-rulesets-tests-${{ github.ref }} + cancel-in-progress: true + +jobs: + test: + name: Lint and bats + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Checkout repository + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + fetch-depth: 1 + + - name: Install bats, shellcheck, and jq + run: | + set -euo pipefail + sudo apt-get update -qq + sudo apt-get install -y --no-install-recommends bats shellcheck jq + + - name: shellcheck + run: | + set -euo pipefail + shellcheck --severity=warning -x scripts/apply-rulesets.sh + + - name: Run bats suite + run: bats --print-output-on-failure test/scripts/apply-rulesets/ diff --git a/scripts/apply-rulesets.sh b/scripts/apply-rulesets.sh index 790110f7..597b7f89 100755 --- a/scripts/apply-rulesets.sh +++ b/scripts/apply-rulesets.sh @@ -1,6 +1,25 @@ #!/usr/bin/env bash # apply-rulesets.sh — Apply standard repository rulesets to petry-projects repos # +# ───────────────────────────────────────────────────────────────────────────── +# LEGACY: DETECTION-BASED RULESET BUILDER +# +# This copy *builds* the `code-quality` ruleset in-code by probing each repo's +# workflow files (detect_required_checks). It predates the codified, +# file-driven applier in `.github-private/scripts/apply-rulesets.sh` (#889), +# which reads the checked-in desired-state JSON in `standards/rulesets/*.json` +# and PUTs it verbatim. Those JSON files — not this script — are the canonical +# source of truth for the `pr-quality` / `code-quality` rulesets (see +# standards/rulesets/README.md). +# +# When editing what this script produces, reconcile it against +# `standards/rulesets/code-quality.json`: the two tools must not disagree about +# the required-context set. In particular this builder deliberately does NOT add +# `Dev-Lead Agent / dev-lead` as a required context — the codified file omits it +# on purpose (#579: Dev-Lead Agent is per-PR review, not a merge gate; requiring +# it deadlocks any PR that touches workflow files). See #580. +# ───────────────────────────────────────────────────────────────────────────── +# # Companion script to compliance-audit.sh. Creates or updates the rulesets defined in: # standards/github-settings.md#repository-rulesets # @@ -125,9 +144,12 @@ detect_required_checks() { # the per-ecosystem jobs. checks+=("dependency-audit / Detect ecosystems") fi - if echo "$workflows" | grep -qx "dev-lead.yml"; then - checks+=("Dev-Lead Agent / dev-lead") - fi + # NOTE: dev-lead.yml is intentionally NOT mapped to a required check. The + # `Dev-Lead Agent / dev-lead` context is per-PR review, not a merge gate: the + # agent's GitHub App refuses to mint a token for any PR touching workflow + # files, so requiring it would deadlock every workflow-modifying PR. The + # codified standards/rulesets/code-quality.json deliberately omits it (#579), + # and this builder must agree (#580). # dependabot-automerge / dependabot-rebase are intentionally NOT # required: dependabot-automerge runs only on dependabot[bot] PRs and # dependabot-rebase runs only on push to main, neither of which are @@ -350,6 +372,11 @@ apply_rulesets() { fi } +# --------------------------------------------------------------------------- +# Entry point — guarded so the pure functions above (detect_required_checks, +# build_ruleset_json, …) can be sourced and unit-tested without running main. +# --------------------------------------------------------------------------- +main() { # --------------------------------------------------------------------------- # Parse arguments # --------------------------------------------------------------------------- @@ -404,3 +431,8 @@ if [ "$TARGET" = "--all" ]; then else apply_rulesets "$TARGET" fi +} + +if [ "${BASH_SOURCE[0]}" = "${0}" ]; then + main "$@" +fi diff --git a/standards/github-settings.md b/standards/github-settings.md index eb19efb8..ede90646 100644 --- a/standards/github-settings.md +++ b/standards/github-settings.md @@ -272,7 +272,7 @@ the `release-channel-tags` ruleset is managed in `.github-private` (see #### Required Check Categories The table below covers the **unconditional fleet-wide required check contexts** — the base set `detect_required_checks()` applies when each org-standard workflow file is present. -The script also conditionally adds **Dev-Lead Agent** (when `dev-lead.yml` exists) and **Secret scan** (when `ci.yml` contains the gitleaks action), but those are in staged rollout; see below. +The script also conditionally adds **Secret scan** (when `ci.yml` contains the gitleaks action), which is in staged rollout; see below. It does **not** add **Dev-Lead Agent** as a required context — that is per-PR review, not a merge gate ([#579](https://github.com/petry-projects/.github/issues/579), [#580](https://github.com/petry-projects/.github/issues/580)); see the table row below. | Check | Status | Check Name(s) | Notes | |-------|--------|---------------|-------| diff --git a/test/scripts/apply-rulesets/dev-lead-not-required.bats b/test/scripts/apply-rulesets/dev-lead-not-required.bats new file mode 100644 index 00000000..b6e87a94 --- /dev/null +++ b/test/scripts/apply-rulesets/dev-lead-not-required.bats @@ -0,0 +1,77 @@ +#!/usr/bin/env bats +# Regression tests pinning the #580 fix: the detection-based +# scripts/apply-rulesets.sh must NOT inject a `Dev-Lead Agent / dev-lead` +# required status check, so its generated `code-quality` set agrees with the +# codified standards/rulesets/code-quality.json (which deliberately omits it, +# per #579 — Dev-Lead Agent is per-PR review, not a merge gate). +# +# These source the script's pure functions directly (guarded main block), so +# they exercise detect_required_checks / build_ruleset_json in isolation. + +bats_require_minimum_version 1.5.0 + +load 'helpers/setup' + +setup() { + tt_make_tmpdir + tt_install_gh_stub + # detect_required_checks + build_ruleset_json are defined below the + # BASH_SOURCE guard, so sourcing does not run main. + source "$TT_SCRIPT" +} + +teardown() { + tt_cleanup_tmpdir +} + +@test "detect_required_checks omits Dev-Lead Agent even when dev-lead.yml is present" { + GH_STUB_WORKFLOWS="agent-shield.yml dependency-audit.yml dev-lead.yml" \ + GH_STUB_CODEQL_STATE="configured" \ + run detect_required_checks "some-repo" + + [ "$status" -eq 0 ] + [[ "$output" != *"Dev-Lead Agent / dev-lead"* ]] +} + +@test "detect_required_checks still emits the codified contexts" { + GH_STUB_WORKFLOWS="agent-shield.yml dependency-audit.yml dev-lead.yml" \ + GH_STUB_CODEQL_STATE="configured" \ + run detect_required_checks "some-repo" + + [ "$status" -eq 0 ] + [[ "$output" == *"CodeQL"* ]] + [[ "$output" == *"agent-shield / AgentShield"* ]] + [[ "$output" == *"dependency-audit / Detect ecosystems"* ]] +} + +@test "detect_required_checks agrees with the codified code-quality.json context set" { + # The generated set (for a repo carrying the codified workflows) must be a + # superset-free match of the checked-in source of truth: every context the + # detector emits for these workflows is present in code-quality.json, and it + # adds nothing the codified file omits. + GH_STUB_WORKFLOWS="agent-shield.yml dependency-audit.yml dev-lead.yml" \ + GH_STUB_CODEQL_STATE="configured" \ + run detect_required_checks "some-repo" + [ "$status" -eq 0 ] + + codified="$(jq -r '.rules[] | select(.type=="required_status_checks") | .parameters.required_status_checks[].context' \ + "${TT_REPO_ROOT}/standards/rulesets/code-quality.json")" + + while IFS= read -r ctx; do + [ -z "$ctx" ] && continue + grep -qxF "$ctx" <<< "$codified" || { + echo "detector emitted non-codified context: '$ctx'" >&2 + false + } + done <<< "$output" +} + +@test "build_ruleset_json injects no contexts beyond those passed" { + run build_ruleset_json "code-quality" "CodeQL" "agent-shield / AgentShield" + [ "$status" -eq 0 ] + + contexts="$(echo "$output" | jq -r '.rules[] | select(.type=="required_status_checks") | .parameters.required_status_checks[].context')" + [[ "$contexts" == *"CodeQL"* ]] + [[ "$contexts" == *"agent-shield / AgentShield"* ]] + [[ "$contexts" != *"Dev-Lead Agent / dev-lead"* ]] +} diff --git a/test/scripts/apply-rulesets/helpers/setup.bash b/test/scripts/apply-rulesets/helpers/setup.bash new file mode 100644 index 00000000..0be2919a --- /dev/null +++ b/test/scripts/apply-rulesets/helpers/setup.bash @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +# Common test helpers for the apply-rulesets bats suite. +set -euo pipefail + +TT_REPO_ROOT="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/../../../.." && pwd)" +readonly TT_REPO_ROOT +export TT_REPO_ROOT + +TT_SCRIPT="${TT_REPO_ROOT}/scripts/apply-rulesets.sh" +readonly TT_SCRIPT +export TT_SCRIPT + +TT_STUBS_DIR="${TT_REPO_ROOT}/test/scripts/apply-rulesets/stubs" +readonly TT_STUBS_DIR +export TT_STUBS_DIR + +tt_make_tmpdir() { + TT_TMP="$(mktemp -d)" + export TT_TMP +} + +tt_cleanup_tmpdir() { + if [[ -n "${TT_TMP:-}" && -d "${TT_TMP}" ]]; then + rm -rf "${TT_TMP}" + fi +} + +# Install the fake `gh` binary on PATH for the test. +tt_install_gh_stub() { + local tt_tmp="${TT_TMP:?TT_TMP must be set (call tt_make_tmpdir first)}" + local stubs_dir="${TT_STUBS_DIR:?TT_STUBS_DIR must be set}" + local stub_dir="${tt_tmp}/bin" + mkdir -p "$stub_dir" + cp "${stubs_dir}/gh" "$stub_dir/gh" + chmod +x "$stub_dir/gh" + PATH="${stub_dir}:${PATH}" + export PATH +} diff --git a/test/scripts/apply-rulesets/stubs/gh b/test/scripts/apply-rulesets/stubs/gh new file mode 100644 index 00000000..1183ea1d --- /dev/null +++ b/test/scripts/apply-rulesets/stubs/gh @@ -0,0 +1,50 @@ +#!/usr/bin/env bash +# Fake `gh` binary for the apply-rulesets bats suite. +# +# detect_required_checks() in scripts/apply-rulesets.sh probes a repo via a +# handful of `gh api` calls. This stub answers those endpoints from env vars so +# tests can shape a repo's workflow set without per-fixture stub variants. +# +# Recognized vars: +# GH_STUB_WORKFLOWS — whitespace-separated workflow filenames returned for +# the `.github/workflows` contents listing (default: +# empty → repo has no workflows). +# GH_STUB_CODEQL_STATE — value returned for the code-scanning default-setup +# `.state` probe (default: "configured"). +# +# Content fetches for individual workflow files return empty (the script guards +# every content call with `|| echo ""`), which is sufficient for the checks +# these tests exercise (they do not depend on sonarcloud.yml / ci.yml bodies). + +set -euo pipefail + +# Locate the api path: the first positional argument after the `api` subcommand. +path="" +prev="" +for arg in "$@"; do + if [ "$prev" = "api" ]; then + path="$arg" + break + fi + prev="$arg" +done + +case "$path" in + */contents/.github/workflows) + # The real call uses `--jq '.[].name'`; emit the already-projected names. + for wf in ${GH_STUB_WORKFLOWS:-}; do + printf '%s\n' "$wf" + done + ;; + */code-scanning/default-setup) + printf '%s' "${GH_STUB_CODEQL_STATE:-configured}" + ;; + */contents/.github/workflows/*) + # Individual workflow file content — not needed for these tests. + printf '' + ;; + *) + printf 'gh stub: unhandled api path: %s\n' "$path" >&2 + exit 1 + ;; +esac