Uh oh!
There was an error while loading. Please reload this page.
fix(ci): declare permissions on every caller template and own workflow - #73
Merged
Conversation
CodeQL's actions/missing-workflow-permissions was open fleet-wide: deploy.yml, crawler.yml and check_data.yml across at least seven relaton-data-* repos, plus this repo's own ci-lint, ci-spec and ci-repo-watcher. It could not be fixed where it was flagged. Every flagged data-repo file is Cimas-generated, so a hand-edit there is reverted by the next 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 skips workflow_call-only workflows. The grant belongs on the caller. Each grant is the UNION of every scope every job of the called workflow requests, not the narrowest set describing what the caller itself does. That matters most for deploy.yml: a naive `contents: read` there does not fail at parse time, it fails minutes into the build at the deploy step, in 29 repos at once, with "requesting 'pages: write', but is only allowed 'pages: none'". Also resyncs the keep-alive template, which had silently drifted away from what every data repo carries by hand. It was missing `permissions: actions: write` (so a sync would have reopened an alert that currently reads `fixed`), missing `workflow_dispatch:`, and still pointing at metanorma/ci — which no longer has a keep-alive.yml at all, so the target was already a 404. `packages: write` on the release caller is a known inherited over-grant: nothing downstream requests it, but it cannot be dropped from the caller alone without capping support's own release.yml below what that file declares. Documented in place for a coordinated follow-up. spec/workflow_permissions_spec.rb encodes the CodeQL rule itself, so the whole class stays closed rather than these instances being fixed once.
check-index.yml landed in main (support#68) after this branch was cut, with no `permissions:` block — so the new sweep in spec/workflow_permissions_spec.rb caught it on the PR merge commit. That is the guard working, not a defect: a caller template can no longer be added to cimas.yml without a grant. Deliberately `contents: read`, narrower than deploy.yml's block, though both call the same reusable workflow. deploy.yml needs 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: it is gated on `github.ref` being the repo's default branch, and a pull_request run's ref is `refs/pull/N/merge`. A skipped job is never dispatched, so its request is never evaluated — the union failure surfaces at the deploy step of a real build, not at graph-parse time. Granting the two publishing scopes on `pull_request`, the most reachable trigger in the repo, would buy nothing and widen the token in all 30 data repos. The template records what to change if that reading turns out to be wrong. Also realigns two "29 repos" comments with main's renumbering to 30.
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the fleet-wide CodeQL
actions/missing-workflow-permissionsalert (medium). Found via the security-bot review on relaton-data-jcgm#1.Why it has to be fixed here
The alert is open on
deploy.yml,crawler.ymlandcheck_data.ymlin at least sevenrelaton-data-*repos (jcgm, iana, iso, ieee, iec, ids, iho), plus this repo's ownci-lint,ci-specandci-repo-watcher.It cannot be fixed where it is flagged:
cimas sync.permissions:can only downgrade what the caller granted. CodeQL knows this and deliberately skipsworkflow_call-only workflows, which is whyrelaton/support's owncrawler.ymlandcheck-data.ymlare not flagged despite declaring nothing.So the grant goes on the caller templates, which live here.
The trap: each grant is a union, not a minimum
A caller must grant the union of every scope every job of the called workflow requests — not the narrowest set describing what the caller itself does.
This matters most for
deploy.yml.data-deploy.ymldeclarescontents: readat workflow level and addspages: write+id-token: writeon itsdeployjob. A reflexive "minimal permissions" edit tocontents: readdoes not fail at parse time — it fails minutes into the build, at the deploy step, in all 29 data repos at once:Every grant below was verified by reading the called workflow, including the upstream
metanorma/ciones.data/deploy.ymlcontents: read,pages: write,id-token: writedata-deploy.ymldeclarescontents: read(workflow) +pages/id-token: write(deploy job)data/crawler.ymlcontents: writecrawler.yml's "Push data" step runsgit commit+git pushwith checkout's persistedGITHUB_TOKENdata/check_data.ymlcontents: readbundle exec ruby check_data.rbmaster/release.ymlid-token/contents/packages: writesupport/release.ymldeclares (see caveat)model/make.ymlcontents: readbundle exec make clean allmaster/rake.ymlcontents: writeci-lint.ymlcontents: readmetanorma/cici-lint.ymldeclarescontents: readci-spec.ymlcontents: readci-repo-watcher.ymlcontents: read,issues: writemetanorma/cici-repo-watcher.ymldeclares both and opens/updates an issue viacomment-or-createci-repo-watcherwas the one worth reading rather than guessing: too narrow means a silent daily-cron failure nobody watches.Also: the keep-alive template had drifted
master/keep-alive.ymlwas three regressions behind what every data repo carries by hand — and none of them were visible in CI:permissions: actions: write. A sync would have reopened a CodeQL alert that currently readsfixedin six repos.workflow_dispatch:, leaving a monthly cron as the only trigger — precisely the workflow GitHub disables for inactivity, with no way back in.uses:still pointing atmetanorma/ci, which no longer has akeep-alive.ymlat all. The target was already a 404.It now diffs functionally clean against the deployed
relaton-data-ianacopy, so that sync is a no-op.Known over-grant, deliberately left
packages: writeon the release caller. Nothing downstream requests it —metanorma/ci'srubygems-release.ymlgrants its jobs onlycontents+id-token, and publishing goes to RubyGems, not GitHub Packages. It is here only becausesupport/release.ymldeclares it, and removing it from the caller alone would cap that file below its own declaration. Removing it means editing both files in one change, verified against a live release. Documented in place; flagged for follow-up rather than smuggled into this PR.Tests
spec/workflow_permissions_spec.rb(new) encodes the CodeQL rule itself so the class stays closed rather than these instances being fixed once:cimas.yml, not globbed);workflow_call-only set CodeQL skips;spec/data_deploy_caller_template_spec.rbgains the deploy/crawler union assertions, sitting next to the existing block that asserts what that job must not carry.333 examples, 0 failures.
Verification still outstanding
The only thing that truly proves the union was right needs credentials:
cimas sync -g dataand confirm the diff in one data repo is only the addedpermissions:blocks.Deployviaworkflow_dispatchand confirm thedeployjob publishes rather than failing thepages: nonecheck.Related, not touched
feat/data-check-index-calleradds a newcheck-index.ymltemplate with nopermissions:block — the new sweep will fail on it when those branches meet.data/check_data.yml's push branches are[ master, main ], but all three repos it syncs into (bipm, iho, iso) are onv2, so its push trigger never fires. Pre-existing, out of scope.