ci: security hardening for GitHub Actions workflows#993
Merged
Conversation
- Replace all @tag references with full 40-character SHA commits - Add version comment to each pinned action for easy reference - Covers all 31 action references across 4 workflow files - Actions affected: checkout, setup-python, cache, codecov, docker/*, github-script, peaceiris/actions-gh-pages, EnricoMi/publish-unit-test-result-action Signed-off-by: Tomer Figenblat <tomer@figenblat.com>
Signed-off-by: Tomer Figenblat <tomer@figenblat.com>
…anup Signed-off-by: Tomer Figenblat <tomer@figenblat.com>
Signed-off-by: Tomer Figenblat <tomer@figenblat.com>
Signed-off-by: Tomer Figenblat <tomer@figenblat.com>
Signed-off-by: Tomer Figenblat <tomer@figenblat.com>
…dependency Signed-off-by: Tomer Figenblat <tomer@figenblat.com>
Reviewer's GuideSecurity-hardens all GitHub Actions workflows by pinning actions to SHAs, adding least-privilege permissions and concurrency controls, tightening checkout/token and SSH key handling, and introducing hadolint-based Dockerfile linting that gates Docker builds and dependent jobs. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds workflow-level concurrency across GitHub Actions, pins third-party actions to specific commit SHAs, updates release SSH signing key handling, and tweaks Dockerfile + Hadolint config for leaner builds and adjusted lint rules. ChangesWorkflow Security and Docker Optimization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
concurrency.groupexpressions use${{workflow}}which is not a valid context; consider switching to${{ github.workflow }}(and similargithub.*contexts) so concurrency grouping works as expected. - In
pr.yml, thebuild-dockerande2e-smoke-testjobs still rely on default token permissions; consider adding explicit least-privilegepermissions:blocks (e.g.,contents: read) to align them with the hardening applied elsewhere. - The hadolint step and pinned action SHAs are repeated across multiple workflows; consider extracting a reusable workflow or composite action to centralize these definitions and make future updates less error-prone.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `concurrency.group` expressions use `${{workflow}}` which is not a valid context; consider switching to `${{ github.workflow }}` (and similar `github.*` contexts) so concurrency grouping works as expected.
- In `pr.yml`, the `build-docker` and `e2e-smoke-test` jobs still rely on default token permissions; consider adding explicit least-privilege `permissions:` blocks (e.g., `contents: read`) to align them with the hardening applied elsewhere.
- The hadolint step and pinned action SHAs are repeated across multiple workflows; consider extracting a reusable workflow or composite action to centralize these definitions and make future updates less error-prone.
## Individual Comments
### Comment 1
<location path=".github/workflows/release.yml" line_range="15-17" />
<code_context>
release:
types: [published]
+concurrency:
+ group: ${{workflow}}-${{github.ref}}
+ cancel-in-progress: true
</code_context>
<issue_to_address>
**issue (bug_risk):** Use the correct GitHub context for the concurrency group expression.
`workflow` without a context (as in `group: ${{workflow}}-${{github.ref}}`) is an invalid GitHub Actions expression and will cause the workflow to fail at runtime. Please change this to use the correct context, e.g. `group: ${{ github.workflow }}-${{ github.ref }}`, and update the same pattern in the other workflows for consistency.
</issue_to_address>
### Comment 2
<location path=".github/workflows/pages.yml" line_range="16-17" />
<code_context>
deploy-pages:
runs-on: ubuntu-latest
environment: github-pages
+ permissions:
+ pages: write
if: ${{ github.ref != 'refs/tags/early-access' }}
name: Build documentation site and deploy to GH-Pages
</code_context>
<issue_to_address>
**issue (bug_risk):** Job permissions here likely break checkout and gh-pages publishing.
With job-level permissions restricted to `pages: write`, the default `GITHUB_TOKEN` no longer has `contents` scope. `actions/checkout` requires `contents: read` and `peaceiris/actions-gh-pages` typically needs `contents: write` to push to `gh-pages`. Update this block to grant `contents: write` (and drop `pages: write` unless you’re explicitly calling the Pages deployment APIs), for example:
```yaml
deploy-pages:
permissions:
contents: write
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
auto-me-bot
Bot
added
status: review started
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pages.yml:
- Around line 8-10: The concurrency group uses an invalid context variable
`${{workflow}}`; update the value in the concurrency block (the "group" key) to
use the correct GitHub Actions context `${{ github.workflow }}` so the group
becomes `${{ github.workflow }}-${{ github.ref }}` ensuring the concurrency
grouping is valid.
- Around line 16-18: The workflow currently sets permissions: pages: write which
lacks the contents scope needed by actions/checkout and
peaceiris/actions-gh-pages when using GITHUB_TOKEN; update the permissions block
to include contents: write (in addition to pages: write) so third-party actions
can read and push repository contents; ensure the permissions stanza explicitly
contains both "contents: write" and "pages: write" so actions/checkout and
peaceiris/actions-gh-pages succeed.
In @.github/workflows/release.yml:
- Around line 15-17: The concurrency.group expression uses an invalid context
token `${{workflow}}`; replace it with the correct GitHub context `${{
github.workflow }}` so the group reads `${{ github.workflow }}-${{ github.ref
}}` (update the concurrency.group line and any similar occurrences) to use the
proper github.workflow context and avoid runtime evaluation errors.
- Around line 24-26: The workflow currently grants the unnecessary GitHub
Packages write scope via the permissions block (packages: write); remove the
packages: write entry and leave only contents: write so the GITHUB_TOKEN cannot
write packages/GHCR. In other words, update the permissions stanza (which
contains permissions: contents: write and packages: write) to drop the packages:
write line; keep contents: write and continue using docker/login-action with
DockerHub credentials and the separate RELEASE_PAT for creating releases.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 971d4dc1-c1f0-4756-aaf4-196a818966a3
📒 Files selected for processing (6)
.github/workflows/pages.yml.github/workflows/pr.yml.github/workflows/release.yml.github/workflows/stage.yml.hadolint.yamlDockerfile
Signed-off-by: Tomer Figenblat <tomer@figenblat.com>
Contributor
Test Results69 tests 69 ✅ 1s ⏱️ Results for commit e8b5266. ♻️ This comment has been updated with latest results. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #993 +/- ##
=======================================
Coverage 96.07% 96.07%
=======================================
Files 1 1
Lines 331 331
=======================================
Hits 318 318
Misses 13 13 🚀 New features to boost your workflow:
|
…ges write scope Signed-off-by: Tomer Figenblat <tomer@figenblat.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/pr.yml (1)
17-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
contents: readtobuild-appjob permissions.
build-appsets job-levelpermissionsbut omitscontents: read; this breaksactions/checkoutbecause checkout requires repository read access. When a job definespermissions, any scope not explicitly specified defaults tonone, preventing checkout from authenticating the clone operation.Suggested fix
build-app: runs-on: ubuntu-latest name: Build project permissions: + contents: read checks: write pull-requests: write🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pr.yml around lines 17 - 24, The job-level permissions block for the build-app job currently sets checks and pull-requests to write but omits contents, which prevents actions/checkout from working; update the permissions block referenced under the build-app job (the permissions: checks, pull-requests entries) to include contents: read so actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd can authenticate and clone the repo.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/pr.yml:
- Around line 17-24: The job-level permissions block for the build-app job
currently sets checks and pull-requests to write but omits contents, which
prevents actions/checkout from working; update the permissions block referenced
under the build-app job (the permissions: checks, pull-requests entries) to
include contents: read so
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd can authenticate and
clone the repo.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7a2bc529-e254-4da2-a845-5cd048d11050
📒 Files selected for processing (4)
.github/workflows/pages.yml.github/workflows/pr.yml.github/workflows/release.yml.github/workflows/stage.yml
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/stage.yml
- .github/workflows/pages.yml
- .github/workflows/release.yml
Signed-off-by: Tomer Figenblat <tomer@figenblat.com>
Owner
Author
|
@coderabbitai Added |
|
Tip For best results, initiate chat on the files or code changes.
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
auto-me-bot
Bot
added
status: merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
permissions:blocks following least-privilege principle across all 4 workflows$RUNNER_TEMP/and remove unnecessary cleanup stepconcurrency:blocks to cancel overlapping workflow runspersist-credentials: falseto checkout steps (remove saved token from disk)build-dockeras dependency fore2e-smoke-test(skip smoke test if hadolint/Docker build fails)content->contentsin pr.yml)Files Changed
.github/workflows/pr.yml.github/workflows/release.yml.github/workflows/stage.yml.github/workflows/pages.ymlDockerfile.hadolint.yaml(new)Changes
130 insertions, 23 deletions
Summary by CodeRabbit