Skip to content

ci: add explicit least-privilege permissions to read-only workflows#657

Open
arpitjain099 wants to merge 3 commits into
microsoft:mainfrom
arpitjain099:security/workflow-permissions
Open

ci: add explicit least-privilege permissions to read-only workflows#657
arpitjain099 wants to merge 3 commits into
microsoft:mainfrom
arpitjain099:security/workflow-permissions

Conversation

@arpitjain099
Copy link
Copy Markdown

What

Adds a workflow-level permissions: contents: read block to seven workflows that currently declare no permissions at all:

  • build.yaml
  • code-formatting-check.yaml
  • docs.yaml
  • local-development-makefile.yaml
  • test.yaml
  • typos.yaml
  • version-checks.yaml

Why

All seven are pure CI workflows — build, format check, docs build, makefile compile, tests, typos spell check, and version consistency check. None push commits, create releases, comment on PRs, or call any GitHub API endpoint that needs write scopes. The GITHUB_TOKEN for these workflows only needs read access for the checkout step, so declaring contents: read at the workflow level documents the minimum scope and removes reliance on the repository default token permissions.

This matches the existing project pattern: workflows that need elevated scopes (cargo-audit.yaml, codeql.yaml, github-dependency-review.yaml, lint.yaml) already declare them at the job level. This PR fills the gap for the read-only workflows that were leaving scope implicit.

Verification

  • YAML validity: python3 -c "import yaml; yaml.safe_load(open(...))" passes on each of the seven files.
  • No change to jobs, steps, or matrix; only the workflow-level permissions: block is added.

Reference

Seven workflows currently declare no permissions block at all, which leaves
GITHUB_TOKEN scope dependent on the repository default. Adding an explicit
workflow-level contents: read documents the minimum scope needed and matches
GitHub's defense-in-depth guidance.

Workflows touched (all are pure CI - build, lint, docs, tests, spell check,
version consistency check). None push commits, create releases, or call
write APIs:
  - build.yaml
  - code-formatting-check.yaml
  - docs.yaml
  - local-development-makefile.yaml
  - test.yaml
  - typos.yaml
  - version-checks.yaml

Existing workflows that need elevated scopes (cargo-audit, codeql,
github-dependency-review, lint) already declare per-job permissions; this PR
does not modify those.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
Copilot AI review requested due to automatic review settings May 13, 2026 06:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens GitHub Actions security by explicitly setting least-privilege GITHUB_TOKEN permissions (contents: read) for CI workflows that previously relied on the repository default token permissions.

Changes:

  • Add workflow-level permissions: contents: read to 7 CI workflows to make read-only intent explicit.
  • Align these workflows with the repo’s existing pattern of explicitly declaring permissions (especially for workflows that need elevated scopes).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
.github/workflows/build.yaml Adds explicit workflow-level contents: read token permissions.
.github/workflows/code-formatting-check.yaml Adds explicit workflow-level contents: read token permissions.
.github/workflows/docs.yaml Adds explicit workflow-level contents: read token permissions.
.github/workflows/local-development-makefile.yaml Adds explicit workflow-level contents: read token permissions.
.github/workflows/test.yaml Adds explicit workflow-level contents: read token permissions.
.github/workflows/typos.yaml Adds explicit workflow-level contents: read token permissions.
.github/workflows/version-checks.yaml Adds explicit workflow-level contents: read token permissions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +12 to +14
permissions:
contents: read

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.45%. Comparing base (ebc705a) to head (bcb5fff).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #657      +/-   ##
==========================================
+ Coverage   78.69%   79.45%   +0.75%     
==========================================
  Files          25       26       +1     
  Lines        5234     5500     +266     
  Branches     5234     5500     +266     
==========================================
+ Hits         4119     4370     +251     
- Misses        995     1001       +6     
- Partials      120      129       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@arpitjain099 arpitjain099 force-pushed the security/workflow-permissions branch from 45fcee0 to bcb5fff Compare May 13, 2026 17:13
@arpitjain099
Copy link
Copy Markdown
Author

Hi @svasista-ms, gentle ping on this. PR has been open for 4 days without review. I noticed you've been on the recent-merger side of recent merges in this repo. When you have a moment, would you mind giving it a quick look? No urgency. Happy to address any feedback.

@svasista-ms
Copy link
Copy Markdown
Contributor

Thanks for the PR @arpitjain099. We verified that the default GITHUB_TOKEN permissions are already set to read-only at the org level, so there's no security gap. However, it doesn't hurt to make this explicit at the workflow level, so we're happy to take this in.

A couple of small asks before we merge:

  1. Could you update the PR title to follow Conventional Commits? Something like:
    ci: add explicit least-privilege permissions to read-only workflows
  2. Could you please address Copilot's review note?

I have approved the workflows. Once these issues are addressed and workflows pass, this is GTG. Thanks again!

arpitjain099 and others added 2 commits May 26, 2026 16:24
Per Copilot review on microsoft#657: `taiki-e/cache-cargo-install-action` writes to the GitHub Actions cache. With the workflow's default permissions pinned to `contents: read`, that step would fail without an `actions: write` scope on the `taplo-fmt` job. Adds a per-job permissions block so the cache save/restore continues to work after the workflow-level restriction.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
Copilot AI review requested due to automatic review settings May 26, 2026 07:33
@arpitjain099 arpitjain099 changed the title Add explicit least-privilege permissions to read-only CI workflows ci: add explicit least-privilege permissions to read-only workflows May 26, 2026
@arpitjain099
Copy link
Copy Markdown
Author

Thanks @svasista-ms. Both addressed:

  1. Title updated to conventional commits format.
  2. For Copilot's note on code-formatting-check.yaml - added a per-job permissions block on taplo-fmt granting contents: read + actions: write so the taiki-e/cache-cargo-install-action step keeps working under the workflow-level read-only default. Other jobs in the workflow only need contents: read, so the broader scope stays scoped to the one job that uses the Actions cache.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

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.

4 participants