Add ShellCheck GitHub Actions workflow#745
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sebrandon1 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 |
|
Warning Review limit reached
More reviews will be available in 48 minutes and 4 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughA ShellCheck linting pipeline is added: a new ChangesShellCheck Linting Infrastructure
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 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)
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 |
f5df3f0 to
4e64b0c
Compare
4e64b0c to
bfc0c78
Compare
|
It looks like there is no way for users to run the shellcheck locally, before pushing their changes. Also, shellcheck should be a verified dependency under the check-deps target if it's gonna be available to run locally. |
|
Ah yes good call I will update it to match the same flow as the kustomization workflow. |
bfc0c78 to
49f17fd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/shellcheck.yml:
- Around line 20-21: The actions/checkout action is not pinned to a specific
commit SHA, creating a supply-chain risk. Replace the version tag reference
(currently `@v4`) in the uses field with a pinned commit SHA. Additionally,
explicitly disable credential persistence by adding a with section to the
checkout action with persist-credentials: false. This change should be applied
to all instances of the actions/checkout action across the workflow files
(shellcheck.yml, kustomize-validation.yml, and doc-updater.yml).
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 83072a75-c21c-4a99-b1f7-322a35576cd9
📒 Files selected for processing (5)
.github/workflows/shellcheck.ymlMakefilehack/test-shellcheck.shtelco-ran/configuration/extra-manifests-builder/01-container-mount-ns-and-kubelet-conf/test.shtelco-ran/configuration/extra-manifests-builder/08-set-rcu-normal/test.sh
| - name: Checkout repository | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify workflow action pinning and checkout credential persistence settings.
rg -n --type=yaml 'uses:\s*actions/checkout@' .github/workflows
rg -n --type=yaml 'persist-credentials:\s*false' .github/workflowsRepository: openshift-kni/telco-reference
Length of output: 388
Pin actions/checkout to a commit SHA and disable credential persistence.
actions/checkout@v4 is not pinned to a commit SHA, creating a supply-chain risk. Additionally, persist-credentials: false should be explicitly set unless authenticated git writes are required. This applies to all checkout actions in the workflows (also found in kustomize-validation.yml and doc-updater.yml).
Suggested hardening
- name: Checkout repository
- uses: actions/checkout@v4
+ uses: actions/checkout@<pinned_commit_sha>
+ with:
+ persist-credentials: false🧰 Tools
🪛 zizmor (1.26.1)
[warning] 20-21: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 21-21: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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/shellcheck.yml around lines 20 - 21, The actions/checkout
action is not pinned to a specific commit SHA, creating a supply-chain risk.
Replace the version tag reference (currently `@v4`) in the uses field with a
pinned commit SHA. Additionally, explicitly disable credential persistence by
adding a with section to the checkout action with persist-credentials: false.
This change should be applied to all instances of the actions/checkout action
across the workflow files (shellcheck.yml, kustomize-validation.yml, and
doc-updater.yml).
Source: Linters/SAST tools
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
- Add hack/test-shellcheck.sh wrapper script following the same pattern as hack/test-kustomize.sh for local developer use - Add test-shellcheck Makefile target that invokes the wrapper script - Add shellcheck to check-deps dependency verification - GHA workflow delegates to make test-shellcheck instead of inline commands - Fix SC2145 shellcheck errors in extra-manifests-builder test scripts
49f17fd to
3bed570
Compare
Summary
make test-shellchecktarget andhack/test-shellcheck.shwrapper script following the same pattern asmake test-kustomize/hack/test-kustomize.shmake test-shellcheckwhen PRs touch shell scripts or the Makefileshellcheckto themake check-depsdependency verification$@in echo strings changed to$*) in extra-manifests-builder test scriptsseverity=errorto catch real bugs without blocking on style warningsThe repo has 32 shell scripts across hack/, extra-manifests-builder/, compare utilities, and configuration scripts. Users can now validate scripts locally with
make test-shellcheckbefore pushing.