Skip to content

[security] Scope CI and release permissions to jobs that need them - #8637

Merged
ScottTodd merged 2 commits into
mainfrom
users/scotttodd/permissions-ci-release
Sep 30, 2026
Merged

ScottTodd merged 2 commits into
mainfrom
users/scotttodd/permissions-ci-release

Conversation

@ScottTodd

Copy link
Copy Markdown
Member

Motivation

Sets minimal permissions across CI/CD workflow files to resolve https://docs.zizmor.sh/audits/#excessive-permissions findings.

Technical Details

  • Move id-token: write from workflow level to narrower job level where needed
  • Remove id-token: write from jobs that don't need it (e.g. no AWS credential setup for uploading files to S3)
  • Add contents: read to jobs that specify permissions (these override the workflow default, checkout works for public repositories but private repositories reusing these workflows need the permission)

Test Plan

Submission Checklist

Validation: pre-commit checks passed.

Generated with Codex
Keep read-only workflow defaults and grant OIDC access to the MSI build job, matching native Linux packaging. Retain contents read access for checkout.

Validation: pre-commit checks passed.

Generated with Codex
@therock-pr-bot

therock-pr-bot Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass PR does not contain code files — Unit Test auto-passed
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@ScottTodd
ScottTodd marked this pull request as ready for review September 30, 2026 21:27
@ScottTodd
ScottTodd requested a review from geomin12 September 30, 2026 21:27

@nunnikri nunnikri 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.

LGTM

@geomin12 geomin12 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.

lgtm if ci passes

@ScottTodd

Copy link
Copy Markdown
Member Author

Relevant CI jobs have passed and I believe that the workflow code in rockrel (notably https://github.com/ROCm/rockrel/blob/main/.github/workflows/multi_arch_release.yml) is still compatible with these changes. Remaining CI jobs are queued on GPU test machines.

@ScottTodd
ScottTodd merged commit 92058ae into main Sep 30, 2026
267 of 273 checks passed
@ScottTodd
ScottTodd deleted the users/scotttodd/permissions-ci-release branch September 30, 2026 22:45
lsudarsh-amd added a commit that referenced this pull request Oct 6, 2026
…tage

- Suppress unpinned-images on the stage job's image, same as #8684.
- Give the stage job its own id-token: write. #8637 moved it off the
  workflow level, so the job would lose AWS access once main is merged.
- Fix the inspector docstring: unit tests pin the rc filename, not the
  build step.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants