ci: replace secrets inherit with explicit token mapping for chatops - #5071
ci: replace secrets inherit with explicit token mapping for chatops#5071mayankdev-oss wants to merge 2 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
/kind cleanup |
There was a problem hiding this comment.
Do we not also need to update the workflows in the plumbing repo to accept this?
The identifier must match the name of a secret defined by
on.workflow_call.secrets.<secret_id>in the called workflow.
This isn't required when using inherit as all secrets the calling workflow has access to are made available to the called workflow.
If the secrets are inherited by using
secrets: inheritin the calling workflow, you can reference them even if they are not explicitly defined in theonkey.
eb638cb to
5a2f716
Compare
5a2f716 to
0600c5c
Compare
0600c5c to
4b4073b
Compare
|
Great catch, Alan! Thanks for pointing that out. I totally missed that the reusable workflows would need on.workflow_call.secrets explicitly defined once we drop inherit. I've just pushed a commit to fix the indentation nit here. Should I go ahead and open a quick PR over in the tektoncd/plumbing repo to update _chatops_retest.yml and _slash.yml? Happy to get that done so we can unblock this one! |
|
@AlanGreene following up on this. Should I go ahead and open a PR in the tektoncd/plumbing repo to explicitly map on.workflow_call.secrets for _chatops_retest.yml and _slash.yml to unblock this? |
|
Yes the plumbing change is required to unblock this |
|
@AlanGreene I have submitted the required plumbing repository changes across two clean PRs to explicitly declare the secrets schema:
Please let me know if these look good or if you need any adjustments on either side! |
AlanGreene
left a comment
There was a problem hiding this comment.
@mayankdev-oss Thanks again for your work on this so far. The changes in the plumbing repo were merged a few weeks ago so this PR now just needs to be updated to use the new versions of the relevant workflows.
I've also left a few comments on unrelated changes to newlines / comments that should be reverted.
Please squash your commits when done.
| @@ -1,14 +1,12 @@ | |||
| name: Rerun Failed Actions | |||
|
|
|||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
| on: | ||
| repository_dispatch: | ||
| types: [retest-command] | ||
|
|
| @@ -1,17 +1,15 @@ | |||
| name: Slash Command Routing | |||
|
|
|||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
| on: | ||
| issue_comment: | ||
| types: [created] | ||
|
|
| issues: write # for peter-evans/slash-command-dispatch to create issue reaction | ||
| pull-requests: write # for peter-evans/slash-command-dispatch to create PR reaction |
Changes
Fixes #5068
This PR addresses the security vulnerability warnings flagged by
zizmorregarding unrestricted secret inheritance. Instead of usingsecrets: inheritwhich passes all repository secrets to called workflows unconditionally, this explicitly maps only the required tokens to improve the security posture of our CI/CD pipelines.Specifically, it:
secrets: inheritin.github/workflows/chatops-retest.ymlwith an explicit pass-through mapping forCHATOPS_TOKEN.secrets: inheritin.github/workflows/slash.ymlwith an explicit pass-through mapping forCHATOPS_TOKEN.Submitter Checklist
As the author of this PR, please check off the items in this checklist:
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes