chore: update fullsend shim workflow - #960
Conversation
|
🤖 Finished Review · ✅ Success · Started 8:55 PM UTC · Completed 9:00 PM UTC |
ReviewFindingsHigh
Medium
Low
Next steps:
Previous runReviewFindingsHigh
Next steps:
Previous run (2)ReviewFindingsHigh
Medium
Low
Next steps:
Previous run (3)ReviewFindingsHigh
Medium
Low
Next steps:
Previous run (4)ReviewFindingsHigh
Low
Next steps:
Previous run (5)Review — PR #960Verdict: request-changes SummaryThis PR changes the fullsend shim workflow ( Findings🔴 HIGH — Supply chain: reusable workflow unpinned from SHA to mutable branch referenceFile: The - uses: konflux-ci/.fullsend/.github/workflows/dispatch.yml@ec21706cccc58d01588ecd842464a5afcc375ba1 # main
+ uses: konflux-ci/.fullsend/.github/workflows/dispatch.yml@mainWhy this matters: This workflow declares elevated permissions — GitHub's security hardening guide explicitly recommends pinning to full-length commit SHAs for supply chain integrity. Mitigating factors:
These mitigating factors reduce practical risk but do not eliminate it. A compromise of the Remediation: If the intent is to update to a newer version of uses: konflux-ci/.fullsend/.github/workflows/dispatch.yml@<current-sha> # main1 finding | 1 high Labels: PR modifies GitHub Actions workflow with security implications Previous run (6)Review — PR #960Verdict: request-changes SummaryThis PR replaces a SHA-pinned reusable workflow reference with a branch reference ( Findings🔴 [high] Supply chain: SHA pin removed from reusable workflow referenceFile: The change replaces a commit-SHA-pinned reference: uses: konflux-ci/.fullsend/.github/workflows/dispatch.yml@ec21706cccc58d01588ecd842464a5afcc375ba1 # mainwith an unpinned branch reference: uses: konflux-ci/.fullsend/.github/workflows/dispatch.yml@mainWhy this matters:
Mitigating factors acknowledged:
Remediation: Keep the SHA-pinned reference. If uses: konflux-ci/.fullsend/.github/workflows/dispatch.yml@<current-sha> # mainThis maintains the project's consistent SHA-pinning convention while still tracking the latest version. Dimensions Reviewed
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #960 +/- ##
==========================================
+ Coverage 76.96% 77.14% +0.17%
==========================================
Files 26 26
Lines 2818 2818
==========================================
+ Hits 2169 2174 +5
+ Misses 450 446 -4
+ Partials 199 198 -1
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
32a6fff to
bc5258e
Compare
|
🤖 Finished Review · ✅ Success · Started 8:53 AM UTC · Completed 8:57 AM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
github_actions
bc5258e to
7d543a9
Compare
PR Summary by QodoUpdate fullsend shim workflow to track upstream template
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
|
🤖 Review · ❌ Terminated · Started 12:47 PM UTC · Ended 12:59 PM UTC |
Code Review by Qodo
1. Unpinned workflow reference
|
![qodo-app-for-konflux-ci[bot]](https://avatars.githubusercontent.com/u/155583288?s=60&v=4){kind=small}
| github.event_name != 'issue_comment' | ||
| || github.event.comment.user.type != 'Bot' | ||
| uses: konflux-ci/.fullsend/.github/workflows/dispatch.yml@ec21706cccc58d01588ecd842464a5afcc375ba1 # main | ||
| uses: konflux-ci/.fullsend/.github/workflows/dispatch.yml@main |
There was a problem hiding this comment.
1. Unpinned workflow reference 🐞 Bug ⛨ Security
The fullsend shim now calls konflux-ci/.fullsend using the mutable ref @main, so upstream changes can alter what runs in this repo without a PR here. This reduces auditability/reproducibility and increases supply-chain blast radius (this workflow requests actions: write and id-token: write).
Agent Prompt
### Issue description
`.github/workflows/fullsend.yaml` references a reusable workflow via `@main`, which is mutable and can change behavior without review in this repo.
### Issue Context
This workflow runs with elevated permissions (including `id-token: write`), so keeping the called workflow immutable improves auditability and limits supply-chain risk.
### Fix Focus Areas
- .github/workflows/fullsend.yaml[44-44]
### Suggested fix
- Replace `@main` with an immutable ref (commit SHA, or a protected/immutable tag if your org uses those).
- If you need to stay aligned with the template, update the pinned SHA as part of a reviewed/automated bump process (e.g., periodic PRs).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Review · ✅ Success · Started 12:47 PM UTC · Completed 12:59 PM UTC |
7d543a9 to
ad2cddf
Compare
|
🤖 Finished Review · ✅ Success · Started 9:28 PM UTC · Completed 9:39 PM UTC Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
ad2cddf to
c534e31
Compare
|
🤖 Finished Review · ✅ Success · Started 7:49 PM UTC · Completed 8:03 PM UTC Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Code review by qodo was updated up to the latest commit b3cfa76 |
|
🤖 Finished Review · ✅ Success · Started 6:40 AM UTC · Completed 6:51 AM UTC Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
b3cfa76 to
47b8abc
Compare
|
🤖 Finished Review · ✅ Success · Started 5:16 PM UTC · Completed 5:33 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.21 |
|
Risk Assessment: moderate (2/5) DetailsA bot-authored single-file CI workflow sync with small diff, clean churn history, and no security-sensitive file changes yields a moderate composite score of 2, despite the CI_WORKFLOW_CHANGED and protected-path signals being elevated by the low-risk bot-author and minimal git churn. |
| github.event_name != 'issue_comment' | ||
| || github.event.comment.user.type != 'Bot' | ||
| uses: konflux-ci/.fullsend/.github/workflows/dispatch.yml@9ee3c2553bf4b71786b0e1444cc9a0706d09980b # main | ||
| uses: konflux-ci/.fullsend/.github/workflows/dispatch.yml@main |
There was a problem hiding this comment.
[high] permission-expansion / supply-chain
The reusable workflow reference was changed from a pinned SHA (@9ee3c2553bf4b71786b0e1444cc9a0706d09980b) to a mutable branch ref (@main). The dispatch job inherits the top-level workflow permissions which include actions: write and id-token: write (OIDC token generation). With @main, any future push to the main branch of konflux-ci/.fullsend changes what code runs under these elevated permissions. A compromised contributor, bot, or supply-chain attack on that repository would automatically propagate to every enrolled repo. Additionally, every other uses: reference across the repository's workflows consistently pins to a full 40-character SHA with a version comment, making this a break from the established convention.
Suggested fix: Pin the uses: reference to the specific commit SHA of the current main HEAD of konflux-ci/.fullsend, e.g. uses: konflux-ci/.fullsend/.github/workflows/dispatch.yml@ # main. Use Dependabot or Renovate with a grouping rule to keep the pin updated with reviewed PRs.
| types: [created] | ||
| pull_request_target: | ||
| types: [opened, synchronize, ready_for_review, closed] | ||
| types: [closed] |
There was a problem hiding this comment.
[medium] behavioral regression
Three pull_request_target event types (opened, synchronize, ready_for_review) were removed, leaving only closed. The dispatch job previously forwarded these PR lifecycle events to dispatch.yml. If dispatch.yml has stage branches that trigger on these event actions, they are now unreachable from this repository. This is a silent behavioral change -- no error is raised; the stages simply stop firing.
Suggested fix: Confirm that dispatch.yml@main no longer has stage branches for opened, synchronize, or ready_for_review actions.
| COMMENT_BODY: ${{ github.event.comment.body }} | ||
| COMMENT_AUTHOR_ASSOC: ${{ github.event.comment.author_association }} | ||
| run: | | ||
| set -euo pipefail |
There was a problem hiding this comment.
[low] Error-handling idioms
The new review-denied job's run: block uses set -euo pipefail, but the sibling stop-fix job runs bare gh calls without any shell-option preamble, creating an inconsistent idiom.
47b8abc to
ffbb48f
Compare
Update the shim workflow to match the current template in the .fullsend config repo.
ffbb48f to
0a62836
Compare
1 participant
This PR updates the fullsend shim workflow to match the current template in the
.fullsendconfig repo.The shim content has drifted from the template — this brings it back in sync.