[ci] Fixed Github workflow for forks#217
Conversation
📝 WalkthroughWalkthroughThe stale PR auto-assignment job is restricted to the target repository. The CI build job uploads Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant BuildJob
participant SiteOutputArtifact as site-output artifact
participant DeployJob
participant GoogleCloudStorage
participant InvalidateCacheJob
BuildJob->>SiteOutputArtifact: upload output/
DeployJob->>SiteOutputArtifact: download into output/
DeployJob->>GoogleCloudStorage: gsutil rsync output/
InvalidateCacheJob->>GoogleCloudStorage: invalidate cache after deployment
Possibly related PRs
Suggested reviewers: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (7 passed)
✨ 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: 2
🤖 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/ci.yml:
- Around line 55-57: The deploy job and other jobs in the workflow lack explicit
least-privilege permissions declarations, allowing them to inherit overly broad
default GITHUB_TOKEN scope. Add an explicit permissions section at the job level
(or workflow level) for the deploy job and all other affected jobs (as noted at
lines 82-85) to specify only the minimal required permissions. For most
deployment scenarios, you may need to set permissions to read-only or none
depending on what the job actually needs to access. Reference the job names like
deploy and any other jobs mentioned in the comment to add the appropriate
permissions blocks.
- Line 50: Replace all floating version tags for GitHub Actions in the workflow
file with specific commit SHAs to ensure supply chain security. For each of the
four actions (actions/upload-artifact, actions/download-artifact,
google-github-actions/auth, and google-github-actions/setup-gcloud), change the
format from `@vX` tags to `@COMMIT_SHA` format where COMMIT_SHA is the full
immutable commit hash. This prevents potential compromises or unexpected changes
to third-party actions while maintaining the intended functionality.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 1265a0ce-777f-4233-b03f-654180e921ca
📒 Files selected for processing (2)
.github/workflows/bot-autoassign-stale-pr.yml.github/workflows/ci.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🧰 Additional context used
🪛 zizmor (1.25.2)
.github/workflows/ci.yml
[warning] 55-80: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 77-77: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 77-77: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 50-50: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 61-61: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 67-67: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 74-74: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[info] 55-55: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
[info] 82-82: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
🔇 Additional comments (1)
.github/workflows/bot-autoassign-stale-pr.yml (1)
15-15: LGTM!
nemesifier
changed the title
nemesifier
left a comment
nemesifier
left a comment
There was a problem hiding this comment.
@asmodehn can you make the same formatting changes (break line) done elsewhere please?
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
20-20: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUnpinned action references on new/changed lines.
actions/checkout@v7(line 20) andactions/setup-python@v6(line 25) use floating tags rather than immutable commit SHAs. This extends the same supply-chain concern already raised in past reviews for lines 50/61/67/74.Also applies to: 25-25
🤖 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/ci.yml at line 20, The workflow still uses floating action tags in the updated CI job, specifically the actions referenced by checkout and setup-python. Replace the unpinned references in the CI workflow with immutable commit SHAs, matching the existing pattern already used elsewhere in the file, and keep the changes centered around the checkout and setup-python steps.Source: Linters/SAST tools
🤖 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/ci.yml:
- Around line 20-22: The checkout step in the CI workflow is pulling
fork-controlled pull request code with default GitHub token credentials still
persisted, which can expose credentials later in the job. Update the
actions/checkout step used with github.event.pull_request.head.sha to disable
credential persistence by setting persist-credentials to false, and keep the
existing ref behavior intact. Also verify whether the actions/checkout and
actions/setup-python version bumps are intentionally omitted from the summary,
since this workflow change should be reflected consistently.
---
Duplicate comments:
In @.github/workflows/ci.yml:
- Line 20: The workflow still uses floating action tags in the updated CI job,
specifically the actions referenced by checkout and setup-python. Replace the
unpinned references in the CI workflow with immutable commit SHAs, matching the
existing pattern already used elsewhere in the file, and keep the changes
centered around the checkout and setup-python steps.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 6b82820b-f0bc-45f4-ab31-056bd594f5a5
📒 Files selected for processing (1)
.github/workflows/ci.yml
📜 Review details
🧰 Additional context used
🪛 zizmor (1.26.1)
.github/workflows/ci.yml
[warning] 20-22: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 20-20: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 25-25: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🔇 Additional comments (3)
.github/workflows/ci.yml (3)
46-54: 🔒 Security & PrivacyArtifact upload correctly captures the
make publishoutput.Uploading
output/assite-outputmatches the directory produced by the Makefile'spublishtarget, and correctly decouples build from deploy for the fork-gating fix. The unpinnedactions/upload-artifact@v4(line 50) reference was already flagged in a prior review.
55-81: 🔒 Security & PrivacyDeploy job gating and artifact download logic is correct; pin/permissions gaps remain outstanding.
The job-level
ifrestricting deploy topushevents onopenwisp/OpenWISP-Website, combined withneeds: buildand the artifact download intooutput/, correctly implements the fork-safety objective and preserves theSRC_URL/DST_URLdeployment. The unpinned actions (lines 61, 67, 74) and missing explicitpermissionsblock were already raised in a prior review and remain unaddressed here.
82-98: 🔒 Security & PrivacyCache invalidation dependency/gating change is correct; permissions gap remains outstanding.
Switching
needsfrombuildtodeployand using a job-levelifinstead of step-level push checks is the right approach and removes the redundant build-success condition. The missing explicitpermissionsblock for this job was already flagged in a prior review.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
20-20: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUnpinned action references on new/changed lines.
actions/checkout@v7(line 20) andactions/setup-python@v6(line 25) use floating tags rather than immutable commit SHAs. This extends the same supply-chain concern already raised in past reviews for lines 50/61/67/74.Also applies to: 25-25
🤖 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/ci.yml at line 20, The workflow still uses floating action tags in the updated CI job, specifically the actions referenced by checkout and setup-python. Replace the unpinned references in the CI workflow with immutable commit SHAs, matching the existing pattern already used elsewhere in the file, and keep the changes centered around the checkout and setup-python steps.Source: Linters/SAST tools
🤖 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/ci.yml:
- Around line 20-22: The checkout step in the CI workflow is pulling
fork-controlled pull request code with default GitHub token credentials still
persisted, which can expose credentials later in the job. Update the
actions/checkout step used with github.event.pull_request.head.sha to disable
credential persistence by setting persist-credentials to false, and keep the
existing ref behavior intact. Also verify whether the actions/checkout and
actions/setup-python version bumps are intentionally omitted from the summary,
since this workflow change should be reflected consistently.
---
Duplicate comments:
In @.github/workflows/ci.yml:
- Line 20: The workflow still uses floating action tags in the updated CI job,
specifically the actions referenced by checkout and setup-python. Replace the
unpinned references in the CI workflow with immutable commit SHAs, matching the
existing pattern already used elsewhere in the file, and keep the changes
centered around the checkout and setup-python steps.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 6b82820b-f0bc-45f4-ab31-056bd594f5a5
📒 Files selected for processing (1)
.github/workflows/ci.yml
📜 Review details
🔇 Additional comments (3)
.github/workflows/ci.yml (3)
46-54: 🔒 Security & PrivacyArtifact upload correctly captures the
make publishoutput.Uploading
output/assite-outputmatches the directory produced by the Makefile'spublishtarget, and correctly decouples build from deploy for the fork-gating fix. The unpinnedactions/upload-artifact@v4(line 50) reference was already flagged in a prior review.
55-81: 🔒 Security & PrivacyDeploy job gating and artifact download logic is correct; pin/permissions gaps remain outstanding.
The job-level
ifrestricting deploy topushevents onopenwisp/OpenWISP-Website, combined withneeds: buildand the artifact download intooutput/, correctly implements the fork-safety objective and preserves theSRC_URL/DST_URLdeployment. The unpinned actions (lines 61, 67, 74) and missing explicitpermissionsblock were already raised in a prior review and remain unaddressed here.
82-98: 🔒 Security & PrivacyCache invalidation dependency/gating change is correct; permissions gap remains outstanding.
Switching
needsfrombuildtodeployand using a job-levelifinstead of step-level push checks is the right approach and removes the redundant build-success condition. The missing explicitpermissionsblock for this job was already flagged in a prior review.
🛑 Comments failed to post (1)
.github/workflows/ci.yml (1)
20-22: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Checkout of fork PR head lacks
persist-credentials: false.Checking out
github.event.pull_request.head.sha(fork-controlled content) while persisting the defaultGITHUB_TOKENcredentials in.git/configis flagged by zizmor asartipacked. Since this step precedes an artifact upload later in the job, disable credential persistence to avoid the token being retrievable from the workspace.🔒 Proposed fix
- uses: actions/checkout@v7 with: ref: ${{ github.event.pull_request.head.sha }} + persist-credentials: falseAdditionally, this checkout/setup-python version bump (v7/v6) isn't mentioned in the AI-generated summary, which only describes the artifact/gating changes — please confirm this is intentional.
📝 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.- uses: actions/checkout@v7 with: ref: ${{ github.event.pull_request.head.sha }} persist-credentials: false🧰 Tools
🪛 zizmor (1.26.1)
[warning] 20-22: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 20-20: 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/ci.yml around lines 20 - 22, The checkout step in the CI workflow is pulling fork-controlled pull request code with default GitHub token credentials still persisted, which can expose credentials later in the job. Update the actions/checkout step used with github.event.pull_request.head.sha to disable credential persistence by setting persist-credentials to false, and keep the existing ref behavior intact. Also verify whether the actions/checkout and actions/setup-python version bumps are intentionally omitted from the summary, since this workflow change should be reflected consistently.Source: Linters/SAST tools
|
@coderabbitai resolve |
✅ Action performedComments resolved and changes approved. |
2 participants
Run the maintenance github actions workflows only on this repo (avoid running on forks).