Workflow permissions#6839
Conversation
Co-authored-by: hanabi1224 <harlowmoo@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
WalkthroughThis PR adds explicit GitHub Actions permissions blocks (enforcing least-privilege access) to 26+ workflow files, setting Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted filessee 6 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
.github/workflows/unit-tests.yml (1)
73-82: Consider versioning cache keys only when snapshot generation logic changes.The static cache keys
proof-params-keysandactor-bundle(lines 77, 82) are currently reused across all runs without restore-keys fallback. While this could theoretically cause stale data issues if upstream snapshots rotate, the team has explicitly decided to defer cache key versioning until it becomes necessary. Per the caching strategy established in PR#5978, a version string can be added to the cache key (e.g.,${{ env.TEST_DATA_CACHE_VERSION }}) if and when the snapshot generation logic changes. No immediate action needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/unit-tests.yml around lines 73 - 82, The workflow uses static cache keys for the actions with ids cache-proof-params and cache-actor-bundle (keys "proof-params-keys" and "actor-bundle"); update the key values to include a version token (for example by appending `${{ env.TEST_DATA_CACHE_VERSION }}`) whenever snapshot generation logic changes so caches are invalidated only when needed—modify the key fields for the cache-proof-params and cache-actor-bundle steps to include the version env var when you bump snapshot logic..github/workflows/forest.yml (1)
2-4: Scopeissues: writetoextra_testsinstead of the whole workflow.Only
extra_testscreates issues, but Lines 2-4 give issue-mutation rights to every build and integration job in this workflow. That is broader than needed for the hardening pass. If you move the write scope down, keepcontents: readonextra_teststoo, since job-level permissions override the root set and that job consumes a repo-hosted issue template.Suggested permission narrowing
permissions: contents: read - issues: write extra_tests: + permissions: + contents: read + issues: write if: ${{ github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' || (github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'Release')) }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/forest.yml around lines 2 - 4, The workflow currently grants issues: write at the top-level which is too broad; remove the top-level issues: write and instead add issues: write to the job permissions for the extra_tests job (keep contents: read at root and also include contents: read in the extra_tests job permissions) so only extra_tests has issue-mutation rights; update the permissions block in the job named extra_tests accordingly..github/workflows/docker-latest-tag.yml (1)
4-6:contents: readlooks unused in this workflow.This job only authenticates to GHCR and retags existing images; it never checks out the repository or reads repository files. Dropping
contents: readwould make this workflow fully least-privilege.Suggested cleanup
permissions: - contents: read packages: write🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docker-latest-tag.yml around lines 4 - 6, The workflow permissions include an unused "contents: read" scope; remove that line so the permissions block only grants the minimal "packages: write" scope. Update the permissions map (the "permissions" YAML and the keys "contents" and "packages") to drop "contents: read" and keep "packages: write", and verify no steps reference repository contents or require checkout before committing the change..github/workflows/docker.yml (1)
2-4: Scopepackages: writetobuild-and-push-docker-imageonly.Lines 2-4 currently give registry write access to
build-ubuntu-amd64andbuild-ubuntu-arm64too, even though only the publish job logs into GHCR and pushes images. That weakens the least-privilege hardening this PR is aiming for. If you move it down to the job, keepcontents: readthere as well, since job-level permissions replace the workflow-level set.Suggested permission narrowing
permissions: contents: read - packages: write jobs: build-and-push-docker-image: + permissions: + contents: read + packages: write🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docker.yml around lines 2 - 4, The workflow-level permissions currently grant packages: write broadly; remove packages: write from the top-level permissions (leave contents: read there) and add a job-level permissions block under the build-and-push-docker-image job that sets packages: write and contents: read so only that job can push to GHCR; reference the top-level permissions keys (permissions: contents/packages) and the job name build-and-push-docker-image when making the change, leaving other jobs like build-ubuntu-amd64 and build-ubuntu-arm64 unaffected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/cargo-publish-dry-run.yml:
- Around line 39-50: Update the failing-run guards so issues are only created
for main branch failures: change the two steps named "Set WORKFLOW_URL" and the
"JasonEtco/create-an-issue@v2" step to use the combined condition github.ref ==
'refs/heads/main' && failure() instead of just failure(), ensuring both the
WORKFLOW_URL export and the issue-creation action only run for failed runs on
refs/heads/main.
In @.github/workflows/release_dispatch.yml:
- Around line 3-4: The workflow currently sets a workflow-level "permissions:
contents: write" which is overly broad; change to least-privilege by removing or
changing the global "contents: write" and instead set job-level permissions:
grant "contents: write" only on the build job (the job that uploads release
artifacts) and ensure the publish job uses "contents: read" (or the minimal
needed permission) while keeping the build and publish job names ("build",
"publish") intact so you update the correct job blocks.
---
Nitpick comments:
In @.github/workflows/docker-latest-tag.yml:
- Around line 4-6: The workflow permissions include an unused "contents: read"
scope; remove that line so the permissions block only grants the minimal
"packages: write" scope. Update the permissions map (the "permissions" YAML and
the keys "contents" and "packages") to drop "contents: read" and keep "packages:
write", and verify no steps reference repository contents or require checkout
before committing the change.
In @.github/workflows/docker.yml:
- Around line 2-4: The workflow-level permissions currently grant packages:
write broadly; remove packages: write from the top-level permissions (leave
contents: read there) and add a job-level permissions block under the
build-and-push-docker-image job that sets packages: write and contents: read so
only that job can push to GHCR; reference the top-level permissions keys
(permissions: contents/packages) and the job name build-and-push-docker-image
when making the change, leaving other jobs like build-ubuntu-amd64 and
build-ubuntu-arm64 unaffected.
In @.github/workflows/forest.yml:
- Around line 2-4: The workflow currently grants issues: write at the top-level
which is too broad; remove the top-level issues: write and instead add issues:
write to the job permissions for the extra_tests job (keep contents: read at
root and also include contents: read in the extra_tests job permissions) so only
extra_tests has issue-mutation rights; update the permissions block in the job
named extra_tests accordingly.
In @.github/workflows/unit-tests.yml:
- Around line 73-82: The workflow uses static cache keys for the actions with
ids cache-proof-params and cache-actor-bundle (keys "proof-params-keys" and
"actor-bundle"); update the key values to include a version token (for example
by appending `${{ env.TEST_DATA_CACHE_VERSION }}`) whenever snapshot generation
logic changes so caches are invalidated only when needed—modify the key fields
for the cache-proof-params and cache-actor-bundle steps to include the version
env var when you bump snapshot logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 53d39938-e9b8-4cdc-b9a1-d7f4ab17156b
📒 Files selected for processing (33)
.github/CARGO_PUBLISH_DRY_RUN_ISSUE_TEMPLATE.md.github/workflows/butterflynet.yml.github/workflows/cargo-advisories.yml.github/workflows/cargo-publish-dry-run.yml.github/workflows/checkpoints.yml.github/workflows/coverage.yml.github/workflows/curio-devnet-publish.yml.github/workflows/docker-dev.yml.github/workflows/docker-latest-tag.yml.github/workflows/docker-lint.yml.github/workflows/docker.yml.github/workflows/dockerfile-check.yml.github/workflows/docs-auto-update.yml.github/workflows/docs-check.yml.github/workflows/docs-required-override.yml.github/workflows/docs-rpc-auto-update.yml.github/workflows/forest.yml.github/workflows/link-check.yml.github/workflows/lists-lint.yml.github/workflows/lotus-api-bump.yml.github/workflows/lotus-devnet-publish.yml.github/workflows/python-lint.yml.github/workflows/release.yml.github/workflows/release_dispatch.yml.github/workflows/rpc-parity-report.yml.github/workflows/rpc-parity.yml.github/workflows/rubocop.yml.github/workflows/rust-lint.yml.github/workflows/shellcheck.yml.github/workflows/snapshot-parity.yml.github/workflows/this-month-in-forest-reminder.yml.github/workflows/unit-tests.yml.github/workflows/yaml-lint.yml
24b2c20 to
111d22d
Compare
Summary of changes
On top of #6838
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes #5873
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit