Conversation
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
edsiper
requested review from
celalettin1286,
niedbalski and
patrick-stephens
as code owners
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds explicit GitHub Actions permissions across many workflow files (mostly Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/call-test-images.yaml:
- Around line 39-40: The workflow only grants contents: read but uses
docker/login-action to access GHCR, which requires packages: read; update the
permissions block in this reusable workflow (the permissions: contents: read
declaration) to include packages: read (e.g., add packages: read alongside
contents: read) and also ensure the caller workflow (staging-test.yaml, the
caller of this reusable workflow) has packages: read in its permissions so the
caller's token can authenticate for GHCR pulls; verify docker/login-action usage
remains unchanged after adding packages: read.
In @.github/workflows/cron-trivy.yaml:
- Around line 27-29: The workflow grants SARIF upload privileges via
permissions: security-events: write but calls the mutable reference
aquasecurity/trivy-action@master in two places; replace both occurrences of
aquasecurity/trivy-action@master with the corresponding full commit SHAs (pinned
commit SHAs) so the job running with security-events: write cannot be changed by
branch updates, and ensure both invocations are updated to the intended fixed
SHAs.
In @.github/workflows/pr-integration-test.yaml:
- Around line 41-42: The workflow grants elevated permission "pull-requests:
write" in the label jobs running on the pull_request_target context but
references the mutable action tag actions-ecosystem/action-add-labels@v1; update
both occurrences of that action reference used by the label jobs to pinned
commit SHAs (replace the `@v1` tag with the full commit SHA for
actions-ecosystem/action-add-labels) so the action is immutable while keeping
the permission "pull-requests: write" and the rest of the job configuration
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1764bcf5-bc01-4f65-808a-0b87ec8e5bb6
📒 Files selected for processing (19)
.github/workflows/build-branch-containers.yaml.github/workflows/build-master-packages.yaml.github/workflows/call-test-images.yaml.github/workflows/call-test-packages.yaml.github/workflows/commit-lint.yaml.github/workflows/cron-stale.yaml.github/workflows/cron-trivy.yaml.github/workflows/master-integration-test.yaml.github/workflows/pr-commit-message.yaml.github/workflows/pr-compile-check.yaml.github/workflows/pr-fuzz.yaml.github/workflows/pr-integration-test.yaml.github/workflows/pr-labels.yaml.github/workflows/pr-lint.yaml.github/workflows/pr-perf-test.yaml.github/workflows/pr-windows-build.yaml.github/workflows/skipped-unit-tests.yaml.github/workflows/staging-build.yaml.github/workflows/staging-test.yaml
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 553338b8c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| permissions: | ||
| contents: read |
There was a problem hiding this comment.
call-test-images.yaml now limits GITHUB_TOKEN to contents: read, but this reusable workflow performs authenticated GHCR logins (docker/login-action in the image verification jobs) and is invoked from staging-test.yaml with secrets.GITHUB_TOKEN. In runs where the staging image/package is not fully public (a common/default GHCR setup), removing packages: read causes registry auth/pulls to fail, so the staging image test workflow will fail before tests execute.
Useful? React with 👍 / 👎.
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/pr-labels.yaml (1)
7-8: Consider addingcontents: readfor consistency with other hardened workflows.The permission block only declares
pull-requests: write. While this is functionally sufficient for the labeling action, other similar workflows in this PR (e.g., context snippet 1 showspr-labels.yamlwith bothcontents: readand per-jobpull-requests: write) include explicitcontents: readat the top level. Adding it here would maintain consistency and explicitly document the minimal read access.♻️ Suggested change for consistency
permissions: + contents: read pull-requests: write🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-labels.yaml around lines 7 - 8, Add the missing top-level permissions entry by updating the permissions block in pr-labels.yaml to include contents: read alongside the existing pull-requests: write; locate the permissions section (the "permissions:" block containing "pull-requests: write") and add a "contents: read" entry to match other hardened workflows and explicitly grant minimal read access..github/workflows/cron-trivy.yaml (1)
27-29: Job-level permissions duplicate top-level declaration.The
trivy-latestjob declares the same permissions already set at the workflow level (lines 15-18). While this is not incorrect and follows defense-in-depth principles, it's redundant since job-level permissions cannot exceed workflow-level permissions. Consider removing the job-level block if you prefer DRY, or keep it for explicit documentation—either approach is acceptable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/cron-trivy.yaml around lines 27 - 29, The job-level permissions block under the trivy-latest job is redundant because the same permissions (contents: read, security-events: write) are already declared at the top-level; remove the permissions: block from the trivy-latest job (i.e., delete the job-scoped permissions entry) so the job inherits the workflow-level permissions, or if you prefer to keep it for clarity leave it — but to follow DRY, remove the job-level permissions under the trivy-latest job.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/cron-trivy.yaml:
- Around line 27-29: The job-level permissions block under the trivy-latest job
is redundant because the same permissions (contents: read, security-events:
write) are already declared at the top-level; remove the permissions: block from
the trivy-latest job (i.e., delete the job-scoped permissions entry) so the job
inherits the workflow-level permissions, or if you prefer to keep it for clarity
leave it — but to follow DRY, remove the job-level permissions under the
trivy-latest job.
In @.github/workflows/pr-labels.yaml:
- Around line 7-8: Add the missing top-level permissions entry by updating the
permissions block in pr-labels.yaml to include contents: read alongside the
existing pull-requests: write; locate the permissions section (the
"permissions:" block containing "pull-requests: write") and add a "contents:
read" entry to match other hardened workflows and explicitly grant minimal read
access.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e008063-050a-4485-b174-955f83f8dc0c
📒 Files selected for processing (6)
.github/workflows/call-test-images.yaml.github/workflows/cron-trivy.yaml.github/workflows/pr-integration-test.yaml.github/workflows/pr-labels.yaml.github/workflows/pr-perf-test.yaml.github/workflows/staging-test.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/staging-test.yaml
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
2 participants
Changes Made
requests: write.
lint.yaml, pr-windows-build.yaml, skipped-unit-tests.yaml.
staging-build.yaml, staging-test.yaml, build-branch-containers.yaml, build-master-packages.yaml, cron-stale.yaml, cron-trivy.yaml.
Residual Risks / Manual Review Needed
code there.
external reusable workflow fluent/fluent-bit-ci/...@main in pr-perf-test.yaml.
Short summary: the exploitable fork PR pattern is removed by stopping fork-controlled code from running inside privileged pull_request_target jobs, and every workflow now declares
explicit token permissions. The main side effect is intentional: labeled fork PRs will no longer run the privileged integration/performance workflows; maintainers will need a same-repo
branch or another trusted trigger for those cases.
GitHub Actions Security Audit
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit