-
Notifications
You must be signed in to change notification settings - Fork 287
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
Bot to pull changes from upstream #558
Conversation
I didn't expect to run into this many hurdles just setting up a simple github PR bot... @Bidon15 have you encountered any of this before? am I missing something simple? |
Hey @evan-forbes, I haven't encountered to such behaviours, yet. It's really weird though. |
I finally figured it out last night, it was just a permissions issue. The bot needs permission to edit workflows, along with reading and writing to the repo. Otherwise, if there are commits that change the |
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.
Is there any ways to make it so that modified workflows don't run until manually allowed?
idk, that's a good question. I'll look into it. I guess that's where we'd have to review them before we merge the PR. |
The issue is that if workflows run during the PR, then upstream can maliciously modify a workflow to e.g. exfiltrate CI keys. So either we make sure there are none of those (maybe not possible?), or stop CI from running on PRs the bot makes. |
That sounds terrible. Perhaps we should just have a bot open an issue to remind us, and then we can run a script and handle all conflicts by hand (something we'll have to do anyway)? If it's only once a week, I don't think that would be too much work. |
I'm not sure which way's the best to handle this. Any opinions @liamsi? |
after thinking about it a bit more, I think we should just auto open an issue to make sure we don't forget, and then manually merge. Most of the time, there will be at least one conflict, which will force us to pull the branch and fix the them by hand anyway. still curious to hear what others think. In the mean time, I'll switch this to a draft. |
If our point of concern is CI keys(secrets) being exposed by such malicious behaviour, then I propose we have a look 👁️ on Hashicorp's Vault TL;DR for Vault - dynamic secrets and we can audit and revoke if we see malicious behaviour or the key has been leaked in some way or another 🗝️ 🧐 They have even released a GH Action for Vault. 😮 wdyt? |
I'll close this for now, unless we decide otherwise. With each update, there was almost always a conflict that had to be handled manually, so there's no real point in having a bot imho |
* crypto/merkle: Add error handling (#558) * porting pr * Disallow nil root hashes * linting * making computeRootHash private * Update release notes. * Update .changelog/unreleased/breaking-changes/558-tm10011.md Co-authored-by: Thane Thomson <[email protected]> * Wrapping the private function into a public one with the old signature. * update comment * remove dependency on amino * Update .changelog/unreleased/breaking-changes/558-tm10011.md Co-authored-by: Thane Thomson <[email protected]> --------- Co-authored-by: Thane Thomson <[email protected]> (cherry picked from commit d067b74) # Conflicts: # crypto/merkle/proof.go # crypto/merkle/proof_test.go * fix conflicts --------- Co-authored-by: Lasaro <[email protected]>
Description
This PR introduced a workflow that pulls changes from upstream and the v0.34.x release branch twice a week.
This PR is blocked by having a github repository secret that is named PULL_UPSTREAM that has permissions for repo read and write, along with access to workflows.
we can see a working version on this fork
Closes: #528
Note: sometimes it fails when there are too many conflicts, but we have to handle conflicts by hand anyway.