Skip to content

It seems that to remove branch protection we also need to specify branch#699

Closed
potiuk wants to merge 1 commit intomainfrom
actually-actually-remove-branch-protection
Closed

It seems that to remove branch protection we also need to specify branch#699
potiuk wants to merge 1 commit intomainfrom
actually-actually-remove-branch-protection

Conversation

@potiuk
Copy link
Copy Markdown
Member

@potiuk potiuk commented Apr 12, 2026

No description provided.

@potiuk potiuk requested review from dave2wave and raboof April 12, 2026 16:59
@raboof
Copy link
Copy Markdown
Member

raboof commented Apr 12, 2026

Why do you believe this? The workflows are able to create commits like 32617e6 .

@potiuk
Copy link
Copy Markdown
Member Author

potiuk commented Apr 12, 2026

Why do you believe this? The workflows are able to create commits like 32617e6 .

Hmm... Because I see this:

Screenshot 2026-04-12 at 19 05 28

@raboof
Copy link
Copy Markdown
Member

raboof commented Apr 12, 2026

Huh indeed, that is surprising.

So somehow now the workflow pushing to the main branch is allowed, but at least 1 review is still required before merging PRs? If that's the case that's unexpected, but seems OK for now (though we'd still want to also restrict pushing to the main branch eventually).

@potiuk
Copy link
Copy Markdown
Member Author

potiuk commented Apr 12, 2026

I think it's more complex than that. Looking at it.

@potiuk
Copy link
Copy Markdown
Member Author

potiuk commented Apr 12, 2026

Something is very strange here.

@dave2wave
Copy link
Copy Markdown
Member

I think we need someone with Admin on this repository to manually remove the branch protection and/or manually do the configuration we wanted that asfyaml is not able to do.

Copy link
Copy Markdown
Member

@dave2wave dave2wave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to revert the change which causes an error with asfyaml processing.

@potiuk
Copy link
Copy Markdown
Member Author

potiuk commented Apr 12, 2026

Ok. I see whole picture now:

[jarekpotiuk:~/code/infrastructure-actions] actually-actually-remove-branch-protection 1 ± gh secret list
! Multiple remotes detected. Due to the sensitive nature of secrets, requiring disambiguation.
? Select a repo apache/infrastructure-actions
NAME                      UPDATED
ALLOWLIST_WORKFLOW_TOKEN  about 1 year ago

We have the ALLOWLIST_WORKFLOW_TOKEN - apparently added year ago by Jacob. This one is used by two workflows:

Those PAT tokens can push directly to main - and bypass our branch protection. Those are personal access tokens - likely with admin level that can override branch protection (apparently this repo has admin-override set). This one updates dummy.yml with newly added actions (and also generates approved_actions.yml).

The 1st workflow is used when someone manually adds actions.yml, The second is a scheduled cleanup.

For the regular -merged PRs (dependabot) there is another workflow: https://github.com/apache/infrastructure-actions/actions/workflows/update_actions.yml -> this one takes dummy.yml changes and produces actions.yml and approved_actions.ymls with whatever comes from dummy.yml upgrade. This one does not use the token, it uses the default write token that "push" event gets - and this one would fail if there is a change from dependabot merged.

The last one succeeded because it had nothing to commit https://github.com/apache/infrastructure-actions/actions/runs/24311711727/job/70982380792#step:5:12

@potiuk
Copy link
Copy Markdown
Member Author

potiuk commented Apr 12, 2026

So... while a bit of surprise we already have a a token that bypasses the branch protection, there is an easy fix to that:

  1. restore branch protection
  2. use the admin token in the "update-actions.yml" workflow

I have to step out for a moment - but I will fix it in 20 mins or so if you are ok wiht it @raboof @dave2wave

@dave2wave
Copy link
Copy Markdown
Member

Whose PAT is ALLOWLIST_WORKFLOW_TOKEN? asfgit?

@potiuk
Copy link
Copy Markdown
Member Author

potiuk commented Apr 12, 2026

Whose PAT is ALLOWLIST_WORKFLOW_TOKEN? asfgit?

No idea and hard to say - the commits are not signed, the user/email on GH commit can be about anything you set. Can't see it - the way how gh secrets works - they are "write-only" - as repo maintainers we can upload a new one and see that one is created, but we cannot see the token that is updated.

We can do some inspection of that token to see who is attached to it - but it's definitely an admin token - see

remote: Bypassed rule violations for refs/heads/main:

[main 32617e6] Update approved_patterns.yml and .github/workflows/dummy.yml based on actions.yml
 1 file changed, 1 insertion(+), 1 deletion(-)
remote: Bypassed rule violations for refs/heads/main:        
remote: 
remote: - Changes must be made through a pull request.        
remote: 
remote: 
remote: GitHub found 1 vulnerability on apache/infrastructure-actions's default branch (1 low). To find out more, visit:        
remote:      https://github.com/apache/infrastructure-actions/security/dependabot/18        
remote: 
To https://github.com/apache/infrastructure-actions

@dave2wave
Copy link
Copy Markdown
Member

Then after we fix the workflow to use ALLOWLIST_WORKFLOW_TOKEN and reset branch protection we should later recycle the PAT so we know.

@potiuk
Copy link
Copy Markdown
Member Author

potiuk commented Apr 12, 2026

#700 - should display all details (non-sensitive) of it once merged.

@dave2wave
Copy link
Copy Markdown
Member

Subsumed by #700

@dave2wave dave2wave closed this Apr 12, 2026
@dave2wave dave2wave deleted the actually-actually-remove-branch-protection branch April 12, 2026 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants