ci: move permissions to job level #3118
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughMoved workflow-level Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-12-21T19:03:59.068ZApplied to files:
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
arkid15r
left a comment
arkid15r
left a comment
There was a problem hiding this comment.
Make sure to update all workflows and do it right :)
Could you add the same changes (see my recent update) for all github workflows -- in other files too -- to keep it consistent?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/run-ci-cd.yaml (1)
183-183: Docker build timeouts may be too tight for reliable builds.The 10-minute timeout for
build-staging-imagesandbuild-production-imagesmay be insufficient. Docker multi-stage builds with GHA caching, layer cachingcache-to: type=registry, and imagepush: trueoperations can exceed this duration under network latency or large image sizes. Consider increasing build job timeouts to 15–20 minutes or benchmarking actual build durations. Test job timeouts (10 min) are reasonable.Also applies to: 217-217, 249-249, 383-383, 716-716
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/run-ci-cd.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-21T19:03:59.068Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1139
File: .github/workflows/setup-backend-environment/action.yaml:16-27
Timestamp: 2025-12-21T19:03:59.068Z
Learning: Composite actions (runs: using: composite) execute as steps within the calling job's context and can access the job context, including job.services.* properties (e.g., job.services.<service_id>.id, job.services.<service_id>.ports). Service containers must be defined at the job level, but a composite action's steps can reference them via the job context.
Applied to files:
.github/workflows/run-ci-cd.yaml
🔇 Additional comments (3)
.github/workflows/run-ci-cd.yaml (3)
28-29: LGTM! Permissions correctly implement least privilege.The addition of
permissions: contents: readat the job level for all jobs that check out the repository correctly implements the principle of least privilege as stated in the PR objectives and addresses the SonarCloud security concern from issue #3104.Also applies to: 63-64, 98-99, 115-116, 136-137, 156-157, 190-191, 224-225, 279-280, 389-390, 432-433, 548-549, 575-576, 616-617, 722-723, 769-770, 896-897
251-264: Correctly omits permissions for set-release-version job.The
set-release-versionjob correctly does not include apermissionsblock because it doesn't check out the repository—it only computes a version string from the release tag or commit SHA and sets job outputs.
595-601: Appropriate step-level timeout for Lighthouse CI.The 15-minute step timeout for Lighthouse CI is reasonable for performance audits and prevents hanging test executions.
|
@arkid15r sure !!!, |
You're good -- don't worry about the deadline. We use it to keep things in shape when people just abandon their work. |
|
2 participants
Proposed change
Resolves #3104
This PR refines GitHub Actions permissions by moving
contents: readfrom theworkflow level to the specific jobs that require it.
This follows the principle of least privilege and aligns with recent review
feedback to scope permissions more precisely. There are no functional or
behavioral changes to the CI/CD pipeline.
Checklist
make check-testlocally and all tests passed