fix: explicit set workflow permission and move secrets to necessary#3484
fix: explicit set workflow permission and move secrets to necessary#3484
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR updates GitHub Actions workflows to harden security permissions and modernize workflow configurations. Changes include adding top-level Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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: 1
🧹 Nitpick comments (2)
.github/workflows/pypi.yml (1)
47-49: PreferGITHUB_REF_NAMEover parsingGITHUB_REF.
GITHUB_REFis the fully qualified ref, whileGITHUB_REF_NAMEis already the short branch/tag name GitHub exposes for exactly this use case. Using the built-in short ref avoids shell parsing here and makes theGITHUB_OUTPUTmigration cleaner. (docs.github.com)Suggested change
- run: echo "TAG_NAME=$(echo $GITHUB_REF | cut -d / -f 3)" >> "$GITHUB_OUTPUT" + run: echo "TAG_NAME=$GITHUB_REF_NAME" >> "$GITHUB_OUTPUT"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pypi.yml around lines 47 - 49, Replace the shell parsing of GITHUB_REF in the "Extract tag name" step (id: tag) with the built-in short ref variable GITHUB_REF_NAME: set TAG_NAME to the value of GITHUB_REF_NAME and write it to GITHUB_OUTPUT (echo "TAG_NAME=$GITHUB_REF_NAME" >> "$GITHUB_OUTPUT") so the step uses the shorter, safer ref name instead of cutting GITHUB_REF..github/workflows/tests.yml (1)
31-33: Consider a job-levelpull-requests: readoverride forgate-skip-e2e.This workflow now sets only
contents: readat the top level, which makes unspecified scopesnone. The lateractions/github-scriptstep callsgithub.rest.pulls.listCommits(), sogate-skip-e2eis the one job here that likely needspull-requests: readin addition tocontents: read. Keeping that override job-local preserves the least-privilege intent of this PR. (docs.github.com)Suggested change
jobs: gate-skip-e2e: + permissions: + contents: read + pull-requests: read needs: [pre-commit] runs-on: ubuntu-latest🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/tests.yml around lines 31 - 33, Top-level workflow permissions only grant contents: read, leaving other scopes none; add a job-level override for the gate-skip-e2e job to include pull-requests: read so the actions/github-script step that calls github.rest.pulls.listCommits() can succeed. Locate the gate-skip-e2e job in the workflow and add a permissions block that sets pull-requests: read (keeping contents: read as needed) so the job retains least-privilege access.
🤖 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/tests.yml:
- Around line 422-423: Remove the unnecessary secret exposure by deleting the
env block that sets CODECOV_TOKEN for the cicd.cleanup entrypoint: locate the
workflow step that calls the cicd.cleanup action/entrypoint and remove the "env:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}" lines so the cleanup step no longer
receives the Codecov token; ensure no other references to CODECOV_TOKEN remain
in that cleanup step.
---
Nitpick comments:
In @.github/workflows/pypi.yml:
- Around line 47-49: Replace the shell parsing of GITHUB_REF in the "Extract tag
name" step (id: tag) with the built-in short ref variable GITHUB_REF_NAME: set
TAG_NAME to the value of GITHUB_REF_NAME and write it to GITHUB_OUTPUT (echo
"TAG_NAME=$GITHUB_REF_NAME" >> "$GITHUB_OUTPUT") so the step uses the shorter,
safer ref name instead of cutting GITHUB_REF.
In @.github/workflows/tests.yml:
- Around line 31-33: Top-level workflow permissions only grant contents: read,
leaving other scopes none; add a job-level override for the gate-skip-e2e job to
include pull-requests: read so the actions/github-script step that calls
github.rest.pulls.listCommits() can succeed. Locate the gate-skip-e2e job in the
workflow and add a permissions block that sets pull-requests: read (keeping
contents: read as needed) so the job retains least-privilege access.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: edc7a76c-8d47-48e1-b35a-1d6f7b30839a
📒 Files selected for processing (8)
.github/workflows/base.yml.github/workflows/lint.yml.github/workflows/main.yml.github/workflows/multi-gpu-e2e.yml.github/workflows/nightlies.yml.github/workflows/pypi.yml.github/workflows/tests-nightly.yml.github/workflows/tests.yml
.github/workflows/tests.yml
Outdated
| env: | ||
| CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd 'cleanup\.py$' .
rg -n -C3 'CODECOV_TOKEN|codecov' cicdRepository: axolotl-ai-cloud/axolotl
Length of output: 1901
🏁 Script executed:
cat -n ./cicd/cleanup.py | head -50Repository: axolotl-ai-cloud/axolotl
Length of output: 558
🏁 Script executed:
cat -n ./cicd/cleanup.shRepository: axolotl-ai-cloud/axolotl
Length of output: 406
Remove CODECOV_TOKEN from the cleanup step environment.
The cicd.cleanup entrypoint only performs cache cleanup and does not upload coverage or use the Codecov token. This step should not have access to the secret.
Remove the env: block (lines 422-423) to avoid unnecessary secret exposure:
Change
- name: Run cleanup
run: modal run cicd.cleanup
- env:
- CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| env: | |
| CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} | |
| - name: Run cleanup | |
| run: modal run cicd.cleanup |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/tests.yml around lines 422 - 423, Remove the unnecessary
secret exposure by deleting the env block that sets CODECOV_TOKEN for the
cicd.cleanup entrypoint: locate the workflow step that calls the cicd.cleanup
action/entrypoint and remove the "env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN
}}" lines so the cleanup step no longer receives the Codecov token; ensure no
other references to CODECOV_TOKEN remain in that cleanup step.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
3 participants
steps only
Followup to #3480 , this PR:
Description
Motivation and Context
How has this been tested?
AI Usage Disclaimer
Claude for review for potential vulnerabilities
Screenshots (if appropriate)
Types of changes
Social Handles (Optional)
Summary by CodeRabbit