Repositories which have protected branches with push restrictions have no ability to grant push rights to Integrations


#21

That’s great, I had previously said that I wanted integration users to be treated like a real user, but in retrospect I agree that if you’ve granted the integration write access it should just bypass the protected branch check.


#22

@kytrinyx have you been able to get your colleague to look into this?


#23

@rarkins no, we’ve all been slammed with GitHub Universe. It’s still on our list of things we’re trying to clear off our plate.


#24

Hi!

I think this is along the lines of what I’m trying to do with a bot being able to approve a PR in the case where PR reviews are required - it has write access to the repo, but the checkmark is still grey and does not allow the PR to be merged even if approved. I submitted a support request about this, but went poking around here too.

It’s possible there’s the same common issue underlying both of our cases! If so, I hope that it will be included in your fix :slight_smile:

Thanks!
Brian


#25

@bmhatfield let me add a test for your use case as well, to make sure.


#26

@bmhatfield It looks like this is a different issue.

I’m going to open up an investigation into apps+protected branches more broadly than these two issues to ensure that we’re not missing more of these.

I can’t promise a timeline or ETA on a fix, but will report back when we have findings.


#27

Thank you very much! In the meantime it appears that the only workaround I can find for my use-case is to register a regular Github Account and associate it with my org, using it as a service account, is that correct?


#28

@bmhatfield Yes, unfortunately. I can’t think of another workaround.


#30

@rarkins I wanted to let you know that this has ended up generating a very healthy debate internally. I can’t promise a timeline for a fix, but we’re working on it.


#31

:wink: I had the same problem when first uploaded photo…


#32

@kytrinyx can you let us know if there’s any progress on this, or perhaps what the debate is about? If it’s about best “workflow” for protecting branches + authorising apps, then some of my users probably have opinions on the topic if you need. Although right now the opinion is mostly “please can you get app merging working whatever way you need to”!


#33

@rarkins Given this part of the product is complicated, we’re working internally to ensure we are able to provide the right long-term solution. Once we have an update, we’ll be sure to report back to you here.


#34

I’ll chime in and say I really wouldn’t want our app (Dependabot) to have push access to protected branches without the user explicitly giving it to us (probably from within the standard protected branches UI).

We often get asked about the security considerations of installing an app which has write access to code, and one really useful piece of advice we give users is to protect any branches they automatically deploy from. It materially decreases the amount of damage an app’s credentials could do if they fell into the wrong hands.

I can understand the debate above, and agree there should be some way to grant apps access to protected branches, but I’d be really concerned if they got it by default. Protected branches are useful to stop malicious actors, not just fat-fingered humans.


#35

@kytrinyx I’m starting to think we’ll see the 1 year anniversary of this post before we see a fix :frowning:

In case discussions ever got off the rails, I’d like to summarise again.

In a repository’s branch protection settings, the restriction now reads like this:

Restrict who can push to this branch
Specify people or teams allowed to push to this branch. Required status checks will still prevent these people from merging if the checks fail.

If someone enables this box, no GitHub App can then push to master or merge a PR, regardless of App permissions.

Assuming the delay to resolving this is because you’ve decided to not just grant implicit push approval to apps that have request read/write content access, surely the solution is then to allow Apps to be added at this UI?

So here’s how I think it would work:

(1) You update the text to say “Specify people, teams or apps allowed to push to this branch…” and allow apps to be included at that point

(2) The API would be updated to reflect this too, e.g. enabling this setting without specifying any non-admins would result of:

"restrictions": {
    "url": "https://api.github.com/repos/myorg/myrepo/branches/master/protection/restrictions",
    "users_url": "https://api.github.com/repos/renovateapp/myorg/myrepo/branches/master/protection/restrictions/users",
    "teams_url": "https://api.github.com/repos/myorg/myrepo/branches/master/protection/restrictions/teams",
    "apps": [],
    "users": [],
    "teams": []
},

voilà?

I know you’ve got some backend work to do too, but… compared to all the other stuff achieved so far it doesn’t seem too complicated. Is there any disagreement within GitHub on whether my above description is “the way” to solve it? Is it just a matter of scheduling resources/prioritisation or is there still internal disagreement on how it should look to the user?


#36

To be honest, I had a working “fix” for this shortly after I verified the issue with a failing integration test, but this sparked internal discussion about what the right solution needs to be, and we don’t have an answer to that yet.


#37

@kytrinyx can you try pushing again with what I described above? Assuming you don’t want to grant automatic access to apps, I don’t see how there’s any alternative than what I described.


#38

@rarkins Yes, I’ve done so.


#39

Hi there, as a big user of renovate I just want to chime in and say that this is a much required feature for us. We are currently disabling protected branch only to get renovate to be able to push directly to master.

I guess this is a complex problem and only wanted to raise a voice from our own usage that we would be happy if this issue had a good fix ultimately. Thanks a lot GitHub :slight_smile:


#40

I can only speculate but here is how I think the internal discussion is going:

GitHub apps where introduced to replace system accounts. They solve the problem that accounts don’t have enough fine-grained access control and also make it easier to insert them into a markeplace, … While building this replacement, GitHub engineers didn’t think that it would be useful to @mention apps (not directly related to this thread) or be able to treat them like general accounts overall (so they can be added to the list of allowed merge users).

So now they are stuck because apps can have any name, which might clash with user accounts. And retrofitting apps to behave more accounts touches all of the inner bowels of github.


#41

How is the discussion at GitHub advancing?

My team is blocked on an effort converting an internal tool to a GitHub App. The intent of the project is to enforce SSO login to our GitHub account so that there aren’t any users with elevated access controls. The goal would be that all team members (from Software Engineer to the CTO) are required follow our engineering team’s Pull Request process. The team will be able to move forward with our CI/CD project but our last CI milestone will not be finished due to being blocked by this limitation of GitHub.