From 158719eaebf06cb47757ee17514fc7ccdd34a71a Mon Sep 17 00:00:00 2001 From: Sal <59910950+ss-o@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:03:39 +0100 Subject: [PATCH] feat(governance): audit reusable-workflow pins not on main --- .../ci/workflow-contract.instructions.md | 1 + .github/workflows/community-health-audit.yml | 11 +- automation/governance/repo-settings-audit.rb | 139 ++++++++++++++++-- .../governance/test-repo-settings-audit.rb | 111 ++++++++++++++ knowledge/domains/ci/workflow-contract.md | 1 + 5 files changed, 246 insertions(+), 17 deletions(-) diff --git a/.github/instructions/ci/workflow-contract.instructions.md b/.github/instructions/ci/workflow-contract.instructions.md index 42d084cf0..e2b9d84eb 100644 --- a/.github/instructions/ci/workflow-contract.instructions.md +++ b/.github/instructions/ci/workflow-contract.instructions.md @@ -44,6 +44,7 @@ permissions: - **Immutable Commit SHAs**: Every remote action and reusable workflow `uses:` reference, including organization references, MUST be pinned to a full 40-character commit SHA. - **Version Comments**: Append the associated release version after the SHA for auditability (e.g., `# v4.3.1`). For organization reusable workflows from `z-shell/.github/.github/workflows/` without an adopted workflow release, retain the established interim `# main` comment. The comment identifies the source branch; the full SHA selects the code. +- **On the source branch**: Pin an organization reusable workflow only to a commit on the default branch of `z-shell/.github`, never to a pull request's branch commit. A squash merge leaves that commit on no branch, and callers pinned to it end every run in a startup failure with no jobs ([#774](https://github.com/z-shell/.github/issues/774)). Before pinning, `gh api repos/z-shell/.github/compare/main...SHA --jq .status` must print `behind` or `identical`. The daily Community Health Audit reports any other pin (`repo-settings-audit.rb --workflow-pins`). - **Prohibited**: Never use mutable tags (e.g., `@v4`, `@main`, `@latest`). ```yaml diff --git a/.github/workflows/community-health-audit.yml b/.github/workflows/community-health-audit.yml index 904f7a5cb..57d7bb8af 100644 --- a/.github/workflows/community-health-audit.yml +++ b/.github/workflows/community-health-audit.yml @@ -10,9 +10,10 @@ on: - "knowledge/domains/governance/data/template-exceptions.yml" - ".github/workflows/community-health-audit.yml" -# Report-only audit of shadow community health files (#720). It reads public -# repository trees with the workflow token and fails only on community health -# drift or a failed read of those trees: --skip-settings keeps rulesets, branch +# Report-only audit of shadow community health files (#720) and of +# reusable-workflow pins to this repository that are not on its default branch +# (#774). It reads public repository trees with the workflow token and fails +# only on that drift or a failed read: --skip-settings keeps rulesets, branch # protection and settings out of the run. It is not a required status check. permissions: contents: read @@ -38,7 +39,7 @@ jobs: run: | set -o pipefail ruby automation/governance/repo-settings-audit.rb --all-repos \ - --community-health --skip-settings --fail-on-drift | tee -a "$GITHUB_STEP_SUMMARY" + --community-health --workflow-pins --skip-settings --fail-on-drift | tee -a "$GITHUB_STEP_SUMMARY" # --all-repos leaves out forks, so each public fork is named here. - name: "🔎 Audit public forks" @@ -51,4 +52,4 @@ jobs: --repo z-shell/z-a-linkbin \ --repo z-shell/zsh-web-artwork \ --repo z-shell/zunit \ - --community-health --skip-settings --fail-on-drift | tee -a "$GITHUB_STEP_SUMMARY" + --community-health --workflow-pins --skip-settings --fail-on-drift | tee -a "$GITHUB_STEP_SUMMARY" diff --git a/automation/governance/repo-settings-audit.rb b/automation/governance/repo-settings-audit.rb index 994308a97..b1e6baadc 100755 --- a/automation/governance/repo-settings-audit.rb +++ b/automation/governance/repo-settings-audit.rb @@ -16,6 +16,9 @@ # --skip-settings narrows a --community-health run to the community health # files: it reads no settings, rulesets or branch protection, so a token that # cannot read them still produces a complete community health report. +# --workflow-pins reports reusable-workflow pins to the organization repository +# that are not on its default branch (z-shell/.github#774); --skip-settings +# narrows such a run the same way. # # Read-only by design: this script has no --apply/--confirm-apply mode. The # settings changes recorded in #478 were hand-judged, per-repository ruleset @@ -434,6 +437,78 @@ def status_for(path, blob, entry, approved, org_defaults) end end + # Reusable-workflow pins to the organization repository (z-shell/.github#774). + # A caller pinned to a commit that is not on the source repository's default + # branch, such as the pre-squash commit of a merged pull request, can stop + # resolving and end every run in a startup failure with no jobs. Each + # `uses: ORG/.github/.github/workflows/NAME@SHA` in a top-level workflow file + # is checked with the compare API: `identical` or `behind` means the SHA is on + # the default branch; anything else is drift. + class WorkflowPins + PIN_PATTERN = %r{uses:\s*["']?(?[\w.-]+/[\w.-]+)/(?\.github/workflows/[\w.-]+\.ya?ml)@(?[0-9a-f]{40})\b} + WORKFLOW_FILE = %r{\A\.github/workflows/[^/]+\.ya?ml\z} + ON_MAIN = %w[identical behind].freeze + DRIFT_STATUSES = %w[off_main missing].freeze + + def self.extract(text, org) + source = "#{org}/.github" + text.to_enum(:scan, PIN_PATTERN).map { Regexp.last_match }.select { |match| match[:repo] == source }.map do |match| + { "workflow" => "#{match[:repo]}/#{match[:path]}", "sha" => match[:sha] } + end + end + + def self.context(client, org) + branch = client.json("/repos/#{org}/.github").fetch("default_branch", "main") + new(client: client, org: org, branch: branch) + end + + # Identical workflow files recur across repositories, so blob contents and + # compare verdicts are cached for the whole run. + def initialize(client:, org:, branch:) + @client = client + @org = org + @branch = branch + @blobs = {} + @statuses = {} + end + + def evaluate(repo, default_branch) + tree = @client.json("/repos/#{repo}/git/trees/#{default_branch}?recursive=1") + raise GitHubError, "#{repo} tree response is truncated" if tree["truncated"] + + files = Array(tree["tree"]).select { |entry| entry["type"] == "blob" && WORKFLOW_FILE.match?(entry["path"]) } + rows = files.sort_by { |entry| entry.fetch("path") }.flat_map do |entry| + self.class.extract(blob_text(repo, entry.fetch("sha")), @org).map do |pin| + { "path" => entry.fetch("path"), "workflow" => pin.fetch("workflow"), "sha" => pin.fetch("sha"), + "status" => status_for(pin.fetch("sha")) } + end + end + { "pins" => rows, "drift" => rows.count { |row| DRIFT_STATUSES.include?(row.fetch("status")) } } + end + + private + + def blob_text(repo, sha) + @blobs[sha] ||= begin + blob = @client.json("/repos/#{repo}/git/blobs/#{sha}") + raise GitHubError, "#{repo} blob #{sha} is not base64" unless blob["encoding"] == "base64" + + blob.fetch("content").unpack1("m").force_encoding(Encoding::UTF_8) + end + end + + def status_for(sha) + @statuses[sha] ||= begin + compare = @client.json("/repos/#{@org}/.github/compare/#{@branch}...#{sha}") + ON_MAIN.include?(compare["status"]) ? "on_main" : "off_main" + rescue GitHubError => error + raise unless [404, 422].include?(error.status) + + "missing" + end + end + end + # Fetches one repository's live rulesets and classic protection, extracts # its settings, and evaluates them against its Baseline row. class RepoAuditor @@ -442,12 +517,13 @@ class RepoAuditor # community_health section to the result. skip_settings reads only the # repository and its community health files: no rulesets, branch # protection or workflows, and no settings verdicts. - def self.audit(client:, repo:, class_resolver:, community_health: nil, skip_settings: false) + def self.audit(client:, repo:, class_resolver:, community_health: nil, skip_settings: false, workflow_pins: nil) repository = client.json("/repos/#{repo}") default_branch = repository.fetch("default_branch", "main") if skip_settings result = settings_skipped_result(repo, class_resolver, default_branch) add_community_health(result, client, repo, default_branch, community_health) if community_health + add_workflow_pins(result, repo, default_branch, workflow_pins) if workflow_pins return result end @@ -478,6 +554,7 @@ def self.audit(client:, repo:, class_resolver:, community_health: nil, skip_sett "errors" => [] } add_community_health(result, client, repo, default_branch, community_health) if community_health + add_workflow_pins(result, repo, default_branch, workflow_pins) if workflow_pins result end @@ -515,6 +592,14 @@ def self.add_community_health(result, client, repo, default_branch, community_he result["errors"] << { "status" => error.status, "message" => error.message } end + # A failed read is recorded as an error on this repository, like a failed + # community health read. + def self.add_workflow_pins(result, repo, default_branch, workflow_pins) + result["workflow_pins"] = workflow_pins.evaluate(repo, default_branch) + rescue GitHubError => error + result["errors"] << { "status" => error.status, "message" => error.message } + end + def self.branch_ruleset_details(client, repo) summaries = client.json("/repos/#{repo}/rulesets") raise GitHubError, "rulesets response must be an array" unless summaries.is_a?(Array) @@ -536,7 +621,7 @@ def self.fetch_workflow_count(client, repo) client.json("/repos/#{repo}/actions/workflows").fetch("total_count", 0) end - private_class_method :settings_skipped_result, :add_community_health, :branch_ruleset_details, :fetch_classic_protection, :fetch_workflow_count + private_class_method :settings_skipped_result, :add_community_health, :add_workflow_pins, :branch_ruleset_details, :fetch_classic_protection, :fetch_workflow_count end # Enumerates target repositories -- an explicit list, or every active, @@ -545,8 +630,9 @@ def self.fetch_workflow_count(client, repo) # A single repository's failure becomes an error record, not a crash: one # broken `gh api` call must not blank out the rest of the org's results. class Inventory - def initialize(client:, org:, class_resolver:, repos: nil, community_health: nil, skip_settings: false) + def initialize(client:, org:, class_resolver:, repos: nil, community_health: nil, skip_settings: false, workflow_pins: nil) @client = client + @workflow_pins = workflow_pins @org = org @class_resolver = class_resolver @repos = repos @@ -557,7 +643,7 @@ def initialize(client:, org:, class_resolver:, repos: nil, community_health: nil def run target_repos.map do |repo| RepoAuditor.audit(client: @client, repo: repo, class_resolver: @class_resolver, community_health: @community_health, - skip_settings: @skip_settings) + skip_settings: @skip_settings, workflow_pins: @workflow_pins) rescue GitHubError => error error_record(repo, error) end @@ -621,6 +707,9 @@ def json(results, org:) if results.any? { |result| result.key?("community_health") } payload["repos_with_community_health_drift"] = results.count { |result| community_health_drift(result).positive? } end + if results.any? { |result| result.key?("workflow_pins") } + payload["repos_with_workflow_pin_drift"] = results.count { |result| workflow_pin_drift(result).positive? } + end payload["results"] = results JSON.pretty_generate(payload) + "\n" end @@ -629,6 +718,10 @@ def self.community_health_drift(result) result.dig("community_health", "drift").to_i end + def self.workflow_pin_drift(result) + result.dig("workflow_pins", "drift").to_i + end + def markdown(results, include_clean:) lines = ["# Repository Settings Audit", "", "Baseline: decisions/0013-repository-settings-baseline.md", ""] if results.any? { |result| result["settings_skipped"] } @@ -648,9 +741,13 @@ def community_health_drift(result) self.class.community_health_drift(result) end + def workflow_pin_drift(result) + self.class.workflow_pin_drift(result) + end + def clean?(result) result.fetch("errors").empty? && result.fetch("summary").fetch("fail").zero? && result.fetch("summary").fetch("warn").zero? && - community_health_drift(result).zero? + community_health_drift(result).zero? && workflow_pin_drift(result).zero? end def repo_section(result) @@ -695,6 +792,16 @@ def repo_section(result) lines << "" end + if workflow_pin_drift(result).positive? + lines << "**Reusable-workflow pins not on the organization default branch (z-shell/.github#774):**" + result.fetch("workflow_pins").fetch("pins").each do |row| + next unless WorkflowPins::DRIFT_STATUSES.include?(row.fetch("status")) + + lines << "- `#{row.fetch("path")}`: #{row.fetch("workflow")}@#{row.fetch("sha")[0, 7]} #{row.fetch("status")}" + end + lines << "" + end + notes = [] notes << "default branch is `#{result.fetch("default_branch")}`, not `main` (audit-only)" unless result.fetch("default_branch_is_main") notes << "both a ruleset and classic branch protection are active" if result.fetch("flags")["dual_protection_systems"] @@ -712,7 +819,7 @@ def repo_section(result) class CLI def self.run(argv, client: GitHubClient.new, stdout: $stdout, stderr: $stderr) options = { org: "z-shell", repos: [], all_repos: false, json: false, include_clean: false, - community_health: false, fail_on_drift: false, skip_settings: false, + community_health: false, workflow_pins: false, fail_on_drift: false, skip_settings: false, classes_file: File.expand_path("../../knowledge/domains/governance/data/repository-classes.yml", __dir__), exceptions_file: File.expand_path("../../knowledge/domains/governance/data/template-exceptions.yml", __dir__) } parser = build_parser(options) @@ -722,10 +829,11 @@ def self.run(argv, client: GitHubClient.new, stdout: $stdout, stderr: $stderr) class_resolver = ClassResolver.load(options[:classes_file]) community_health = community_health_context(client, options) if options[:community_health] + workflow_pins = WorkflowPins.context(client, options[:org]) if options[:workflow_pins] inventory = Inventory.new( client: client, org: options[:org], class_resolver: class_resolver, repos: options[:all_repos] ? nil : options[:repos], community_health: community_health, - skip_settings: options[:skip_settings] + skip_settings: options[:skip_settings], workflow_pins: workflow_pins ) results = inventory.run @@ -735,13 +843,14 @@ def self.run(argv, client: GitHubClient.new, stdout: $stdout, stderr: $stderr) return 1 if results.any? { |result| !result.fetch("errors").empty? } return 1 if options[:fail_on_drift] && results.any? { |result| Renderer.community_health_drift(result).positive? } + return 1 if options[:fail_on_drift] && results.any? { |result| Renderer.workflow_pin_drift(result).positive? } 0 rescue OptionParser::ParseError, ArgumentError => error stderr.puts error.message 2 rescue GitHubError => error - stderr.puts "organization community health defaults: #{error.message}" + stderr.puts "organization defaults: #{error.message}" 1 end @@ -765,8 +874,13 @@ def self.build_parser(options) option.on("--exceptions-file PATH", "Approved local files (default: knowledge/domains/governance/data/template-exceptions.yml)") do |value| options[:exceptions_file] = value end - option.on("--fail-on-drift", "Exit 1 when --community-health finds drift (default: report only)") { options[:fail_on_drift] = true } - option.on("--skip-settings", "With --community-health, read no settings, rulesets or branch protection") do + option.on("--workflow-pins", "Also report reusable-workflow pins to ORG/.github that are not on its default branch (#774)") do + options[:workflow_pins] = true + end + option.on("--fail-on-drift", "Exit 1 when --community-health or --workflow-pins finds drift (default: report only)") do + options[:fail_on_drift] = true + end + option.on("--skip-settings", "With --community-health or --workflow-pins, read no settings, rulesets or branch protection") do options[:skip_settings] = true end end @@ -777,8 +891,9 @@ def self.validate!(options) raise OptionParser::InvalidOption, "use either --all-repos or one or more --repo values, not both" end raise OptionParser::MissingArgument, "pass at least one --repo OWNER/REPO or --all-repos" if !options[:all_repos] && options[:repos].empty? - raise OptionParser::InvalidOption, "--fail-on-drift needs --community-health" if options[:fail_on_drift] && !options[:community_health] - raise OptionParser::InvalidOption, "--skip-settings needs --community-health" if options[:skip_settings] && !options[:community_health] + drift_check = options[:community_health] || options[:workflow_pins] + raise OptionParser::InvalidOption, "--fail-on-drift needs --community-health or --workflow-pins" if options[:fail_on_drift] && !drift_check + raise OptionParser::InvalidOption, "--skip-settings needs --community-health or --workflow-pins" if options[:skip_settings] && !drift_check options[:repos].each do |repo| raise OptionParser::InvalidArgument, "--repo must be OWNER/REPO: #{repo}" unless repo.match?(%r{\A[^/]+/[^/]+\z}) diff --git a/automation/governance/test-repo-settings-audit.rb b/automation/governance/test-repo-settings-audit.rb index 4a1264261..4f00e4339 100755 --- a/automation/governance/test-repo-settings-audit.rb +++ b/automation/governance/test-repo-settings-audit.rb @@ -841,6 +841,117 @@ def test_auditor_records_a_tree_failure_as_an_error_and_keeps_settings assert_equal(1, result.fetch("errors").length) refute(result.fetch("settings").empty?, "expected the settings verdicts to remain") end + + # --- WorkflowPins ------------------------------------------------------ + + ON_MAIN_PIN = "6fac3d47a61b27db54e50a0519c1e79b2935eda4" + OFF_MAIN_PIN = "64a427d8ac33c9ef1dc89af35920eb0d3de9fa9c" + + def caller_workflow(sha, quote: "") + <<~YAML + jobs: + policy: + uses: #{quote}z-shell/.github/.github/workflows/commit-lint.yml@#{sha}#{quote} # main + build: + steps: + - uses: actions/checkout@#{"b" * 40} # v6 + - uses: ./.github/actions/local + other: + uses: someone/else/.github/workflows/x.yml@#{"c" * 40} + YAML + end + + # Workflow files of z-shell/p as path => text; compare answers map a pinned + # SHA to the compare API status, or to a GitHubErrorResponse. + def workflow_pin_routes(files, compare) + routes = { + "/repos/z-shell/.github" => { "default_branch" => "main" }, + "/repos/z-shell/p" => { "default_branch" => "main" }, + "/repos/z-shell/p/git/trees/main?recursive=1" => { + "truncated" => false, + "tree" => files.each_with_index.map { |(path, _text), index| { "path" => path, "type" => "blob", "sha" => index.to_s * 40 } } + + [{ "path" => "README.md", "type" => "blob", "sha" => "e" * 40 }] + } + } + files.each_with_index do |(_path, text), index| + routes["/repos/z-shell/p/git/blobs/#{index.to_s * 40}"] = { "encoding" => "base64", "content" => [text].pack("m") } + end + compare.each do |sha, answer| + routes["/repos/z-shell/.github/compare/main...#{sha}"] = answer.is_a?(String) ? { "status" => answer } : answer + end + routes + end + + def test_workflow_pins_extract_only_organization_reusable_workflow_pins + pins = RepoSettingsAudit::WorkflowPins.extract(caller_workflow(ON_MAIN_PIN, quote: '"'), "z-shell") + + assert_equal([{ "workflow" => "z-shell/.github/.github/workflows/commit-lint.yml", "sha" => ON_MAIN_PIN }], pins) + end + + def test_workflow_pins_report_a_pin_that_is_not_on_the_source_main_branch + routes = workflow_pin_routes( + { ".github/workflows/commit-lint.yml" => caller_workflow(OFF_MAIN_PIN), ".github/workflows/ci.yaml" => caller_workflow(ON_MAIN_PIN) }, + { OFF_MAIN_PIN => "diverged", ON_MAIN_PIN => "behind" } + ) + checker = RepoSettingsAudit::WorkflowPins.context(FixtureClient.new(routes), "z-shell") + + result = checker.evaluate("z-shell/p", "main") + + assert_equal(1, result.fetch("drift")) + assert_equal( + [[".github/workflows/ci.yaml", ON_MAIN_PIN, "on_main"], [".github/workflows/commit-lint.yml", OFF_MAIN_PIN, "off_main"]], + result.fetch("pins").map { |row| [row.fetch("path"), row.fetch("sha"), row.fetch("status")] } + ) + end + + def test_workflow_pins_report_a_pin_the_source_repository_does_not_have + routes = workflow_pin_routes({ ".github/workflows/commit-lint.yml" => caller_workflow(OFF_MAIN_PIN) }, + { OFF_MAIN_PIN => GitHubErrorResponse.new(status: 404) }) + + result = RepoSettingsAudit::WorkflowPins.context(FixtureClient.new(routes), "z-shell").evaluate("z-shell/p", "main") + + assert_equal("missing", result.fetch("pins").first.fetch("status")) + assert_equal(1, result.fetch("drift")) + end + + def test_workflow_pins_accept_an_identical_pin_and_ignore_nested_workflow_paths + routes = workflow_pin_routes( + { ".github/workflows/commit-lint.yml" => caller_workflow(ON_MAIN_PIN), ".github/workflows/fixtures/old.yml" => caller_workflow(OFF_MAIN_PIN) }, + { ON_MAIN_PIN => "identical" } + ) + + result = RepoSettingsAudit::WorkflowPins.context(FixtureClient.new(routes), "z-shell").evaluate("z-shell/p", "main") + + assert_equal(0, result.fetch("drift")) + assert_equal([".github/workflows/commit-lint.yml"], result.fetch("pins").map { |row| row.fetch("path") }) + end + + def workflow_pin_args(*extra) + ["--repo", "z-shell/p", "--classes-file", CLASSES_FILE, "--workflow-pins", "--skip-settings", *extra] + end + + def test_cli_workflow_pins_fail_on_drift_exits_1_on_an_off_main_pin_and_0_when_on_main + off = workflow_pin_routes({ ".github/workflows/commit-lint.yml" => caller_workflow(OFF_MAIN_PIN) }, { OFF_MAIN_PIN => "diverged" }) + on = workflow_pin_routes({ ".github/workflows/commit-lint.yml" => caller_workflow(ON_MAIN_PIN) }, { ON_MAIN_PIN => "behind" }) + report = StringIO.new + drift = RepoSettingsAudit::CLI.run(workflow_pin_args("--fail-on-drift"), client: FixtureClient.new(off), stdout: report, stderr: StringIO.new) + json = StringIO.new + clean = RepoSettingsAudit::CLI.run(workflow_pin_args("--fail-on-drift", "--json"), client: FixtureClient.new(on), stdout: json, stderr: StringIO.new) + + assert_equal(1, drift) + assert_equal(0, clean) + assert(report.string.include?("`.github/workflows/commit-lint.yml`: z-shell/.github/.github/workflows/commit-lint.yml@64a427d off_main"), + "expected the off-main pin in the Markdown report, got: #{report.string}") + assert_equal(0, JSON.parse(json.string).fetch("repos_with_workflow_pin_drift")) + end + + def test_cli_workflow_pins_alone_satisfy_skip_settings_and_fail_on_drift + status = RepoSettingsAudit::CLI.run( + workflow_pin_args("--fail-on-drift"), + client: FixtureClient.new(workflow_pin_routes({}, {})), stdout: StringIO.new, stderr: StringIO.new + ) + assert_equal(0, status) + end end GitHubErrorResponse = Struct.new(:status, :message) do diff --git a/knowledge/domains/ci/workflow-contract.md b/knowledge/domains/ci/workflow-contract.md index 08fe83878..ede86b2d6 100644 --- a/knowledge/domains/ci/workflow-contract.md +++ b/knowledge/domains/ci/workflow-contract.md @@ -40,6 +40,7 @@ permissions: - **Immutable Commit SHAs**: Every remote action and reusable workflow `uses:` reference, including organization references, MUST be pinned to a full 40-character commit SHA. - **Version Comments**: Append the associated release version after the SHA for auditability (e.g., `# v4.3.1`). For organization reusable workflows from `z-shell/.github/.github/workflows/` without an adopted workflow release, retain the established interim `# main` comment. The comment identifies the source branch; the full SHA selects the code. +- **On the source branch**: Pin an organization reusable workflow only to a commit on the default branch of `z-shell/.github`, never to a pull request's branch commit. A squash merge leaves that commit on no branch, and callers pinned to it end every run in a startup failure with no jobs ([#774](https://github.com/z-shell/.github/issues/774)). Before pinning, `gh api repos/z-shell/.github/compare/main...SHA --jq .status` must print `behind` or `identical`. The daily Community Health Audit reports any other pin (`repo-settings-audit.rb --workflow-pins`). - **Prohibited**: Never use mutable tags (e.g., `@v4`, `@main`, `@latest`). ```yaml