Skip to content

chore(ci): add least-privilege permissions to build.yml and checks.yml - #8053

Open
yhabib wants to merge 1 commit into
mainfrom
chore/add-permissions-to-workflows
Open

chore(ci): add least-privilege permissions to build.yml and checks.yml#8053
yhabib wants to merge 1 commit into
mainfrom
chore/add-permissions-to-workflows

Conversation

@yhabib

@yhabib yhabib commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Motivation

build.yml and checks.yml run on every push and declare no permissions: key, so every job holds the repository default GITHUB_TOKEN, which is read and write.

Most jobs only read the checkout. Proof that a write token on every push is a real risk: nightly.yaml pushes a tag with the default token and no permissions: block, and git ls-remote --tags origin 'nightly-2026-09*' lists a tag for every day.

Changes

  • Added a workflow-level permissions: contents: read block to build.yml and checks.yml.
  • Raised test-rest and assets in build.yml to contents: write, because their Release steps call gh release create, gh release upload and gh release edit.
  • Raised small-tests in checks.yml to contents: read plus actions: read, because its CI wasm download step calls gh run list, gh run download and gh run view --log.
  • Added a one-line comment above each raised job block that names the step that needs the scope.

Tests

  • ./scripts/fmt-yaml --check exits 0.
  • yq confirms the workflow-level block is contents: read in both files, and that only test-rest, assets and small-tests carry a job-level block.
  • git diff -U0 against main shows only added permissions: lines and comments. No run: step changed.
  • Ran actionlint (not in this repo, downloaded 1.7.7 to check) against both files on main and on this branch. The finding sets are identical, so the new blocks add no schema warning.

The permissions block only takes effect on a real GitHub Actions run, so this PR itself is the remaining proof. Two things stay open until CI runs here:

  • Every job of build.yml and checks.yml must go green, including small-tests on both ubuntu-24.04 and macos-15-intel, and assets.
  • The contents: write release path needs a tag pushed to this branch head, so a draft release gets created and then deleted as a throwaway check.

Todos

  • Accessibility (a11y) – no impact, this only changes CI workflow files.
  • Changelog – not needed, a workflow permissions change is not user facing.
  • This is part 1 of 4 split out of item 1788181556. The other three parts (workflow permissions for the remaining files, and the repository setting) each land as their own pull request.
  • One acceptance point stays open at this stage: CI must go green on this PR, and a throwaway tag push must show the Release steps create a draft release. The next stage checks this.

Both workflows declared no permissions key. Each job held the repository
default GITHUB_TOKEN, which is read and write.

Set contents: read at workflow level in both files. Raise the two jobs that
publish a release to contents: write. Give small-tests actions: read for
download-ci-wasm.test.
@yhabib
yhabib requested a review from a team as a code owner September 4, 2026 23:26
@zeropath-ai

zeropath-ai Bot commented Sep 4, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 6fdf16f.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► .github/workflows/build.yml
    Add permissions blocks for contents and release-related steps
► .github/workflows/checks.yml
    Add permissions blocks for contents and actions, and document read permissions

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new workflow-level permissions drop actions scope to none, but both workflows still use Actions APIs (cache and artifacts), which will likely cause CI authorization failures unless actions permissions are granted appropriately.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR tightens GitHub Actions least-privilege usage by explicitly setting GITHUB_TOKEN permissions in the always-on build.yml and checks.yml workflows instead of relying on the repository default read/write token.

Changes:

  • Added workflow-level permissions: contents: read to both build.yml and checks.yml.
  • Added job-level contents: write to test-rest and assets in build.yml for release-related operations.
  • Added job-level actions: read to small-tests in checks.yml for gh run ... usage.
File summaries
File Description
.github/workflows/checks.yml Adds workflow/job permission blocks to reduce default token privileges.
.github/workflows/build.yml Adds workflow/job permission blocks to scope write access to release-producing jobs.
Review details

Suppressed comments (1)

.github/workflows/build.yml:477

  • This job defines a job-level permissions block, which overrides workflow-level permissions. The job uses both actions/download-artifact and actions/upload-artifact, so it needs actions: write in addition to contents: write.
    permissions:
      # The Release step creates a GitHub release. A later step pushes the
      # refs/notes/mainnet/wasm-sha note.
      contents: write
  • Files reviewed: 2/2 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/build.yml
Comment thread .github/workflows/build.yml
Comment thread .github/workflows/checks.yml
@yhabib

yhabib commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

About the suppressed comment on assets (needs actions: write for artifact steps): this PR's own run https://github.com/dfinity/nns-dapp/actions/runs/33929465747, job assets, downloads one artifact and uploads three, with only contents: write granted and no actions scope. The job is green. Same-run actions/upload-artifact and actions/download-artifact read a runner-issued token, not GITHUB_TOKEN, so the actions scope is not the gate here.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approved

The permission changes are minimal, scoped to the documented jobs/steps that require them, and align with least-privilege without altering any workflow behavior beyond token scopes.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants