diff --git a/.github/workflows/ci-lint.yml b/.github/workflows/ci-lint.yml index 60429ee..fd5798b 100644 --- a/.github/workflows/ci-lint.yml +++ b/.github/workflows/ci-lint.yml @@ -7,5 +7,9 @@ on: jobs: lint: + # Matches what metanorma/ci's ci-lint.yml declares for itself: yamllint, + # shellcheck and actionlint over a checkout, nothing more. + permissions: + contents: read uses: metanorma/ci/.github/workflows/ci-lint.yml@main diff --git a/.github/workflows/ci-repo-watcher.yml b/.github/workflows/ci-repo-watcher.yml index db462ef..97e85e2 100644 --- a/.github/workflows/ci-repo-watcher.yml +++ b/.github/workflows/ci-repo-watcher.yml @@ -8,6 +8,15 @@ on: jobs: watcher: + # `issues: write` is the load-bearing half. metanorma/ci's ci-repo-watcher.yml + # declares `contents: read` + `issues: write` for itself, and when it finds + # repos missing from cimas.yml it opens or updates an issue via + # metanorma/ci/comment-or-create. A caller granting less caps the called + # workflow, so trimming this to `contents: read` would fail that step on a + # daily cron nobody watches — the watcher would simply stop reporting. + permissions: + contents: read + issues: write uses: metanorma/ci/.github/workflows/ci-repo-watcher.yml@main secrets: token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/ci-spec.yml b/.github/workflows/ci-spec.yml index b87afc7..c13c59c 100644 --- a/.github/workflows/ci-spec.yml +++ b/.github/workflows/ci-spec.yml @@ -20,6 +20,9 @@ on: jobs: spec: + # Checkout and rspec; this job publishes nothing. + permissions: + contents: read runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 diff --git a/cimas-config/gh-actions/data/check-index.yml b/cimas-config/gh-actions/data/check-index.yml index 65dcf58..e919006 100644 --- a/cimas-config/gh-actions/data/check-index.yml +++ b/cimas-config/gh-actions/data/check-index.yml @@ -45,4 +45,25 @@ jobs: # branding lives in relaton/support's `data-index/configs.yml`, which # data-deploy.yml reads at build time — this run resolves it too, so a broken # configs.yml row fails the PR rather than the publish. + # + # Deliberately NARROWER than deploy.yml's grant, though both call the same + # reusable workflow. deploy.yml must grant the union of every scope + # data-deploy.yml's jobs request, including `pages: write` + `id-token: write` + # for its `deploy` job. Here that job cannot run at all: it is gated on + # `github.ref` equalling the repo's default branch, and a pull_request run's + # ref is `refs/pull/N/merge`. A job that is skipped is never dispatched, so + # its permission request is never evaluated — the union failure surfaces at + # the deploy step of a real build, not when the workflow graph is parsed. + # + # Which is why this is `contents: read` and must stay there: `pull_request` is + # the most reachable trigger in the repo, and the two scopes withheld are + # exactly the two that publish. Granting them here would hand every PR in + # every data repo the ability to deploy Pages, to buy nothing. + # + # If a synced repo's first PR ever fails with "requesting 'pages: write', but + # is only allowed 'pages: none'", that reading was wrong and the fix is to + # widen this to deploy.yml's three — not to touch the `if:` gate. + # Pinned by spec/workflow_permissions_spec.rb. + permissions: + contents: read uses: relaton/support/.github/workflows/data-deploy.yml@main diff --git a/cimas-config/gh-actions/data/check_data.yml b/cimas-config/gh-actions/data/check_data.yml index 6af1fd8..4ccc4a6 100644 --- a/cimas-config/gh-actions/data/check_data.yml +++ b/cimas-config/gh-actions/data/check_data.yml @@ -9,4 +9,8 @@ on: jobs: check-data: + # check-data.yml checks out the repo (with submodules) and runs + # `bundle exec ruby check_data.rb`. Nothing writes, so read is the union. + permissions: + contents: read uses: relaton/support/.github/workflows/check-data.yml@main \ No newline at end of file diff --git a/cimas-config/gh-actions/data/crawler.yml b/cimas-config/gh-actions/data/crawler.yml index c9397af..6c7d3ef 100644 --- a/cimas-config/gh-actions/data/crawler.yml +++ b/cimas-config/gh-actions/data/crawler.yml @@ -29,6 +29,18 @@ on: jobs: crawl: + # Required, not hardening. The shared crawler.yml's "Push data" step runs + # `git commit` + `git push` with the credentials actions/checkout persists, + # i.e. GITHUB_TOKEN — and a caller granting less caps what the called + # workflow gets. With only `contents: read` the crawl still runs to + # completion (166,658 documents on the largest flavor) and then fails on the + # push, having thrown the whole corpus away. + # + # The grant cannot live in crawler.yml itself: a called workflow's + # `permissions:` can only downgrade the caller's. Pinned by + # spec/data_deploy_caller_template_spec.rb. + permissions: + contents: write uses: relaton/support/.github/workflows/crawler.yml@main with: args: ${{ github.event.inputs.args }} \ No newline at end of file diff --git a/cimas-config/gh-actions/data/deploy.yml b/cimas-config/gh-actions/data/deploy.yml index 6f81efc..87383fa 100644 --- a/cimas-config/gh-actions/data/deploy.yml +++ b/cimas-config/gh-actions/data/deploy.yml @@ -84,4 +84,23 @@ jobs: # defaulted to `gem`. That default is now `git`, so those pins set the value # they would inherit anyway and a sync dropping them is the cleanup, not the # damage. Do not re-add one here. + # + # `permissions:` is the one thing this job does declare, and it is + # deliberately wider than `contents: read`. A called workflow cannot exceed + # what the caller granted, so a caller needs the UNION of every scope every + # job of the called workflow asks for — data-deploy.yml declares + # `contents: read` at workflow level and adds `pages: write` + + # `id-token: write` on its own `deploy` job. + # + # Narrowing this to `contents: read` — the reflexive "minimal permissions" + # edit — does not fail at parse time. It fails minutes into the build, at the + # deploy step, in all 30 repos at once, with: + # + # The workflow is requesting 'pages: write', but is only allowed 'pages: none'. + # + # Pinned by spec/data_deploy_caller_template_spec.rb. Do not narrow it. + permissions: + contents: read # the four checkouts in build_index_page + pages: write # actions/deploy-pages in the deploy job + id-token: write # OIDC verification for that deployment uses: relaton/support/.github/workflows/data-deploy.yml@main diff --git a/cimas-config/gh-actions/master/keep-alive.yml b/cimas-config/gh-actions/master/keep-alive.yml index 986fa76..fb5b417 100644 --- a/cimas-config/gh-actions/master/keep-alive.yml +++ b/cimas-config/gh-actions/master/keep-alive.yml @@ -1,8 +1,37 @@ name: keep-alive + +# This template had drifted away from what is actually deployed. Every data repo +# carried a hand-fixed copy — a `permissions:` block, a `workflow_dispatch:` +# trigger, and a `uses:` pointing at relaton/support — that the source template +# never picked up, so the next `cimas sync` would have reverted all three at once +# with nothing red in CI to say so. That is the failure mode of every hand-edit +# in a synced destination. The three differences are pinned individually by +# spec/workflow_permissions_spec.rb so they cannot drift apart again. on: schedule: - cron: "0 0 1 * *" # on the first day-of-month + # A keep-alive whose only trigger is a monthly cron is exactly the workflow + # GitHub disables for inactivity — and once disabled there is no cron left to + # re-enable it. This is the manual way back in. + workflow_dispatch: + +# Not an over-grant and not removable: re-enabling workflows through the Actions +# API (`gh api -X PUT /repos/.../actions/workflows//enable`) is this +# workflow's entire job. Dropping it reopens the CodeQL +# `actions/missing-workflow-permissions` alert that currently reads `fixed` in +# every data repo. Workflow-level rather than per-job, to match what is already +# deployed so a sync stays a functional no-op. +permissions: + actions: write jobs: keep-alive: - uses: metanorma/ci/.github/workflows/keep-alive.yml@main + # relaton/support's own copy, not metanorma/ci's. The fleet was deliberately + # moved onto it — every deployed keep-alive.yml points here — and only the + # template lagged. This is not a preference: metanorma/ci no longer has a + # keep-alive.yml at all, so the target this template used to name is gone and + # a sync would have pointed 40-odd repos at a 404. support's version also + # auto-discovers workflows by numeric id, which covers dynamic ones like + # CodeQL whose path is not under .github/workflows/ and so cannot be enabled + # by name. + uses: relaton/support/.github/workflows/keep-alive.yml@main diff --git a/cimas-config/gh-actions/master/release.yml b/cimas-config/gh-actions/master/release.yml index cc31cf4..55af4a5 100644 --- a/cimas-config/gh-actions/master/release.yml +++ b/cimas-config/gh-actions/master/release.yml @@ -14,6 +14,23 @@ on: jobs: release: + # The union of what relaton/support's release.yml declares for itself. A + # caller's grant is the ceiling for everything below it, so anything missing + # here fails at the publish step — after the version bump has already been + # committed and tagged, which is the worst possible place to find out. + # + # `packages: write` is here only because support's release.yml declares it. + # Nothing downstream asks for it: metanorma/ci's rubygems-release.yml grants + # its own jobs `contents: read` + `id-token: write` (preflight) and + # `contents: write` + `id-token: write` (release), and publishing goes to + # RubyGems, not the GitHub Packages registry. It is a real over-grant — but + # it cannot be dropped here alone, because removing it from the caller caps + # support's release.yml below what that file declares. Removing it means + # editing both, in one change, verified against a live release. + permissions: + id-token: write # OIDC trusted publishing to RubyGems + contents: write # the version bump commit and its tag + packages: write # see above — inherited over-grant, remove with release.yml uses: relaton/support/.github/workflows/release.yml@main with: next_version: ${{ github.event.inputs.next_version }} diff --git a/cimas-config/gh-actions/model/make.yml b/cimas-config/gh-actions/model/make.yml index 4726770..7efd758 100644 --- a/cimas-config/gh-actions/model/make.yml +++ b/cimas-config/gh-actions/model/make.yml @@ -8,4 +8,8 @@ on: jobs: make: + # make.yml checks out the repo and runs `bundle exec make clean all` across a + # three-OS matrix. It publishes nothing, so read is the union. + permissions: + contents: read uses: relaton/support/.github/workflows/make.yml@main \ No newline at end of file diff --git a/spec/data_deploy_caller_template_spec.rb b/spec/data_deploy_caller_template_spec.rb index c7a02c7..24914c9 100644 --- a/spec/data_deploy_caller_template_spec.rb +++ b/spec/data_deploy_caller_template_spec.rb @@ -247,4 +247,46 @@ expect(deploy_job).not_to have_key("with") end end + + describe "the token grant it hands the shared workflow" do + # The counterpart to the block above: everything there is about what this + # caller must NOT carry, and `permissions:` is the one thing it must. + # + # The rule that makes this non-obvious: a called workflow cannot exceed what + # the caller granted, so the caller needs the UNION of every scope every job + # of the called workflow asks for — not the narrowest set that describes what + # the caller itself does. data-deploy.yml declares `contents: read` at + # workflow level and adds `pages: write` + `id-token: write` on its `deploy` + # job, so all three belong here. + # + # Narrowing this to `contents: read` — the reflexive "minimal permissions" + # edit — does not fail at parse time. It fails minutes into the build, at the + # deploy step, in all 30 repos at once, with: + # + # The workflow is requesting 'pages: write', but is only allowed 'pages: none'. + # + # Which is exactly why it is asserted here rather than left to review. + deploy_job = deploy.fetch("jobs").fetch("deploy") + + it "grants the union of what data-deploy.yml's jobs request" do + expect(deploy_job.fetch("permissions")) + .to eq("contents" => "read", "pages" => "write", "id-token" => "write") + end + + it "keeps the two Pages scopes specifically" do + # Stated separately from the exact-match example above because dropping + # either one is the actual failure mode, and a future edit that legitimately + # adds a fourth scope should still not be able to remove these two. + expect(deploy_job.fetch("permissions")).to include("pages" => "write", "id-token" => "write") + end + + it "lets the crawler template push what it crawled" do + # `contents: write` is required, not hardening: the shared crawler.yml's + # "Push data" step runs `git commit` + `git push` with the credentials + # actions/checkout persists. Without it the crawl still runs to completion — + # 166,658 documents on the largest flavor — and then fails on the push. + expect(crawler.fetch("jobs").fetch("crawl").fetch("permissions")) + .to eq("contents" => "write") + end + end end diff --git a/spec/workflow_permissions_spec.rb b/spec/workflow_permissions_spec.rb new file mode 100644 index 0000000..c9e01b0 --- /dev/null +++ b/spec/workflow_permissions_spec.rb @@ -0,0 +1,189 @@ +# Encodes CodeQL's `actions/missing-workflow-permissions` rule as a test. +# +# A workflow that declares no `permissions:` runs with whatever the repo or org +# default happens to be — historically write-all. The alert was open fleet-wide: +# deploy.yml, crawler.yml and check_data.yml in at least seven relaton-data-* +# repos, plus this repo's own ci-lint/ci-spec/ci-repo-watcher. +# +# None of that could be fixed where it was flagged. Every flagged data-repo file +# is Cimas-generated, so a hand-edit there is reverted by the next `cimas sync`; +# and it cannot be fixed in the reusable workflows either, because a called +# workflow's `permissions:` can only *downgrade* what the caller granted. CodeQL +# knows this and deliberately skips `workflow_call`-only workflows. So the grant +# has to live on the caller, and the callers are the templates in this repo. +# +# The two sweeps below make the whole class a test failure rather than a list of +# instances someone fixed once. Both are silent when broken: nothing goes red in +# CI, the token is just wider (or narrower) than intended. +RSpec.describe "workflow token grants" do + repo_root = File.expand_path("..", __dir__) + + # CodeQL's condition, and GitHub's: a workflow-level block covers every job, or + # each job carries its own. Either satisfies the rule; requiring one shape + # would force gh-actions/master/rake.yml's existing workflow-level block to + # move for no benefit. + # + # An empty block is rejected as well as a missing one. `permissions: {}` passes + # CodeQL — it is the maximally restrictive grant — but here it would mean a + # caller handing the workflow it calls a token with every scope set to `none`, + # which is the same silent late failure a missing block risks, just inverted. + declares_grant = lambda do |doc| + granted = lambda do |block| + block.is_a?(String) ? !block.empty? : !(block || {}).empty? + end + + next granted.call(doc["permissions"]) if doc.key?("permissions") + + jobs = doc["jobs"] || {} + !jobs.empty? && jobs.values.all? do |job| + job.is_a?(Hash) && job.key?("permissions") && granted.call(job["permissions"]) + end + end + + describe "every Cimas-synced caller template" do + # Derived from cimas.yml rather than globbed, for the same reason + # spec/cimas_data_pages_spec.rb derives it: the templates that matter are the + # ones Cimas actually copies into other repos. A template nobody syncs is + # dead config; one that is synced is copied byte-for-byte into every mapped + # repo, so its grant is the grant the whole fleet runs under. + cimas = YAML.safe_load_file(File.join(repo_root, "cimas-config/cimas.yml")) + templates = cimas.fetch("repositories").values.compact + .flat_map { |r| (r["files"] || {}).values } + .uniq.select { |src| %w[.yml .yaml].include?(File.extname(src)) } + .sort + + it "is a non-empty list" do + # Guards the sweep itself: a cimas.yml restructure that broke the + # derivation would otherwise turn every example below into a vacuous pass. + expect(templates).not_to be_empty + end + + templates.each do |src| + it "#{src} declares a permissions block" do + doc = YAML.safe_load_file(File.join(repo_root, "cimas-config", src)) || {} + + expect(declares_grant.call(doc)).to be(true), + "#{src} is copied into every mapped repo as a " \ + "top-level workflow, so it runs with the repo's " \ + "default GITHUB_TOKEN scope unless it says " \ + "otherwise. Declare `permissions:` on the workflow " \ + "or on every job — and make it the UNION of what " \ + "the called workflow's jobs request, not the " \ + "narrowest thing that parses." + end + end + + # Presence is not enough. The sweep above would still pass if a "tighten the + # permissions" pass narrowed a grant to something the called workflow cannot + # run under — which is the *other* half of this failure mode, and the more + # expensive half, because it fails late and loudly in every synced repo at + # once rather than quietly widening a token. + # + # deploy.yml and crawler.yml are pinned in + # spec/data_deploy_caller_template_spec.rb, keep-alive.yml below. These are + # the rest, so every synced template has its exact grant written down + # somewhere. + expected = { + # rake.yml: pre-existing, and load-bearing — generic-rake.yml's + # `tests-passed` job fires a repository-dispatch, which needs contents. + "gh-actions/master/rake.yml" => { "contents" => "write" }, + # release.yml: narrowing this fails the publish step *after* the version + # bump has been committed and tagged. See the note in the template about + # `packages: write` being an inherited over-grant. + "gh-actions/master/release.yml" => { + "id-token" => "write", "contents" => "write", "packages" => "write" + }, + "gh-actions/data/check_data.yml" => { "contents" => "read" }, + "gh-actions/model/make.yml" => { "contents" => "read" }, + # check-index.yml: the interesting one. It calls the SAME reusable workflow + # as deploy.yml, yet grants strictly less — because the `pages: write` job + # it could otherwise reach is `if:`-gated off for a pull_request ref, and a + # skipped job never has its request evaluated. Pinned in both directions: + # widening it hands every PR in the fleet the ability to publish, narrowing + # it breaks the pre-merge check. See the template for the full argument. + "gh-actions/data/check-index.yml" => { "contents" => "read" }, + }.freeze + + expected.each do |src, grant| + it "#{src} grants exactly #{grant.inspect}" do + doc = YAML.safe_load_file(File.join(repo_root, "cimas-config", src)) + # Wherever it is declared: these templates are all single-job, so a + # workflow-level block and a job-level one are equivalent in effect. + declared = doc["permissions"] || doc.fetch("jobs").values.first["permissions"] + + expect(declared).to eq(grant) + end + end + end + + describe "every workflow this repo runs itself" do + # The reusables here are `workflow_call`-only and correctly leave the grant to + # their callers — CodeQL skips exactly that set, so this sweep does too. What + # is left is the hand-written ones that actually run in this repo. + # + # ci-repo-watcher.yml is deliberately NOT skipped: it carries `schedule` and + # `workflow_dispatch` alongside `workflow_call`, so it runs here on a daily + # cron and needs a grant of its own. + # Both extensions, matching the template sweep above and GitHub itself, which + # runs `.yaml` workflows exactly like `.yml` ones. Globbing only `.yml` would + # let a `codeql.yaml` reopen the very alert this file exists to close, with + # the suite still green and nothing to say the file was never looked at. + workflows = Dir[File.join(repo_root, ".github/workflows/*.{yml,yaml}")].sort + + it "is a non-empty list" do + expect(workflows).not_to be_empty + end + + workflows.each do |path| + name = File.basename(path) + + it "#{name} declares a permissions block, or is reusable-only" do + doc = YAML.safe_load_file(path) + # Psych reads the unquoted `on:` key as YAML 1.1 boolean true. + triggers = doc.fetch(true) + next if triggers.keys == ["workflow_call"] + + expect(declares_grant.call(doc)).to be(true), + "#{name} is triggered by #{triggers.keys.join(', ')} " \ + "in this repo, so it runs with the default " \ + "GITHUB_TOKEN scope unless it declares one." + end + end + end + + describe "cimas-config/gh-actions/master/keep-alive.yml" do + # This template silently drifted away from what is deployed. All three + # regressions below were live in the template while the data repos carried + # the fixed version by hand — so the next `cimas sync` would have reverted a + # CodeQL alert that reads `fixed` in six repos, with nothing red to say so. + # That is the failure mode a hand-edit in a synced destination always has, + # and the reason each difference is pinned separately here. + template = YAML.safe_load_file(File.join(repo_root, "cimas-config/gh-actions/master/keep-alive.yml")) + + it "grants actions: write" do + # Not an over-grant: re-enabling workflows through the Actions API + # (`gh api -X PUT /repos/.../actions/workflows//enable`) is the entire + # job. Workflow-level rather than per-job, to match what is deployed so a + # sync stays a functional no-op. + expect(template.fetch("permissions")).to eq("actions" => "write") + end + + it "can still be run by hand" do + # A keep-alive whose only trigger is a monthly cron is precisely the + # workflow GitHub disables for inactivity — and then there is no way to + # start it again from the UI. + expect(template.fetch(true)).to have_key("workflow_dispatch") + end + + it "calls this repo's own keep-alive workflow" do + # Every deployed copy points here rather than at metanorma/ci: the fleet + # was moved onto support's version deliberately, and only the template + # lagged. Asserted together with the file's existence, since a `uses:` + # naming a workflow that is not here fails at run time, in a monthly cron + # nobody watches. + expect(template.fetch("jobs").fetch("keep-alive").fetch("uses")) + .to eq("relaton/support/.github/workflows/keep-alive.yml@main") + expect(File).to exist(File.join(repo_root, ".github/workflows/keep-alive.yml")) + end + end +end