-
Notifications
You must be signed in to change notification settings - Fork 46
chore: update fullsend shim workflow #960
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,7 +41,7 @@ jobs: | |
| if: >- | ||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [high] security/supply-chain Reusable workflow reference unpinned from commit SHA to mutable branch reference (@main). This workflow runs with id-token:write and actions:write permissions under pull_request_target, giving it access to repository secrets and OIDC token minting. Any commit pushed to the main branch of konflux-ci/.fullsend will automatically execute with these elevated permissions. SHA pinning is a concrete supply chain integrity control recommended by GitHub security hardening guidance. Suggested fix: Pin to the current commit SHA on main rather than using the branch reference. Example: uses: konflux-ci/.fullsend/.github/workflows/dispatch.yml@ # main There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. Unpinned workflow reference 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [high] supply chain / reproducibility The change replaces a pinned commit SHA (ec21706cccc58d01588ecd842464a5afcc375ba1) with a mutable branch reference (@main) for the reusable workflow konflux-ci/.fullsend/.github/workflows/dispatch.yml. All 18 external action/workflow references across 7 workflow files in this repository are SHA-pinned. This breaks that universal convention and introduces supply-chain risk: any push to .fullsend main will immediately change the code executed in this repo's CI. See also: [permission-expansion] finding at this location. Suggested fix: Pin to the current commit SHA of konflux-ci/.fullsend's main branch: uses: konflux-ci/.fullsend@701e62a # main There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [high] permission-expansion SHA-pinned reusable workflow reference replaced with mutable branch reference (@main). The workflow runs on pull_request_target with top-level permissions including actions:write and id-token:write (OIDC). A compromise or malicious push to the .fullsend repo's main branch would immediately gain these elevated permissions in this repository without any review gate. See also: [supply chain / reproducibility] finding at this location. Suggested fix: Keep the workflow reference SHA-pinned. Pin to the new commit SHA with a trailing comment indicating the branch (e.g., @701e62a9c6f104ed68f8d4085d9c3b8bad3a82e4 # main). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] missing-authorization Non-trivial workflow change with no linked issue. The change unpins the fullsend dispatch workflow from a fixed SHA to track main branch, which alters the security and deployment model. Suggested fix: Create a linked issue documenting why unpinning is authorized. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [high] supply-chain The reusable workflow reference is changed from a pinned commit SHA (ec21706cccc58d01588ecd842464a5afcc375ba1) to the mutable branch reference @main. This workflow runs on privileged event triggers including pull_request_target and issue_comment, and has top-level permissions for actions: write and id-token: write. A mutable branch reference means anyone with write access to the .fullsend repository can change what code executes in this repository CI context without any review or visibility in this repository. The OIDC id-token: write permission is particularly sensitive — the called workflow can mint OIDC tokens scoped to this repository. Suggested fix: Keep the SHA-pinned reference. If updating to a newer version of dispatch.yml, pin to the new SHA and retain the # main comment for traceability. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] dependency-pinning-convention All 30+ workflow references across 9 workflow files in this repository use pinned SHA commits with a trailing comment indicating the branch/version. This change is the only reference that would use a bare branch name, violating the established repository convention. Suggested fix: Revert to the pinned SHA pattern. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [high] supply-chain integrity regression Replaces a SHA-pinned reusable workflow reference (@ec21706cccc58d01588ecd842464a5afcc375ba1) with a mutable branch reference (@main). Every other uses: directive across all workflows in this repository is SHA-pinned for supply-chain integrity, and the most recent commit (b71d1e0, PR #1010) explicitly established this policy. A mutable @main reference means any future commit to konflux-ci/.fullsend will execute automatically in this repository context without review. Suggested fix: Pin to the new commit SHA with a trailing comment (e.g., @ # main), matching the convention used throughout the repository. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [high] permission-expansion The unpinned workflow reference inherits actions: write and id-token: write permissions via pull_request_target. A compromised or malicious upstream commit to konflux-ci/.fullsend would gain OIDC token access across all enrolled repositories without any review gate in this repo. Suggested fix: Keep the reference SHA-pinned. Update the SHA when the upstream workflow changes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [high] contradicts-project-direction The most recent commit to this file (b71d1e0, PR #1010) was titled Pin kindest/node image by digest for supply-chain integrity. This PR reverses that security posture for the fullsend workflow reference without explaining why the trade-off is different for this dependency. Suggested fix: Document why this dependency should be unpinned, or pin it to match the project supply-chain integrity policy. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] scope-classification-mismatch PR titled chore: update fullsend shim workflow but the change removes supply-chain integrity protections (SHA pinning) — this is a security-relevant change, not a maintenance chore. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] consistency Uses @main while all other workflow references in the repository use SHA-pinned format with trailing version/branch comments. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [high] security/supply-chain Replacing the SHA-pinned reusable workflow reference (@ec21706cccc58d01588ecd842464a5afcc375ba1) with a floating branch reference (@main) removes supply-chain integrity for this external workflow call. The dispatch.yml workflow from konflux-ci/.fullsend will now resolve to whatever commit happens to be at main at invocation time. Because this workflow runs on pull_request_target with id-token: write and actions: write permissions, any future push to main in the .fullsend repo instantly changes what privileged code executes in this repository without any review gate in this repo. Every other uses: directive across all workflows in this repository is SHA-pinned, making this an inconsistency with the established security convention. Suggested fix: Keep the SHA-pinned reference. If the intent is to update to a newer commit of dispatch.yml, pin to the new SHA with the branch name as a trailing comment (e.g., uses: konflux-ci/.fullsend/.github/workflows/dispatch.yml@ # main), consistent with every other workflow reference in this repository. |
||
| with: | ||
| event_action: ${{ github.event.action }} | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[high] supply-chain-security
SHA-pinned reusable workflow reference replaced with unpinned branch reference (@main). This breaks the project-wide convention of SHA-pinning all 22 action/workflow references across 6 workflow files. The workflow has elevated permissions (id-token: write, actions: write), meaning any future commit to main of konflux-ci/.fullsend is automatically trusted without review in the consuming repo.
Suggested fix: Keep the SHA-pinned reference. If dispatch.yml has been updated, pin to its current HEAD commit SHA: dispatch.yml@ # main