Skip to content
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

Create adr-labeler workflow #300

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

jonchurch
Copy link
Member

Related to #296 (comment)

Will label any PR that touches the adr dir w/ ADR label

@bjohansebas
Copy link
Member

I like it, although I'm sure it will give an error like in expressjs/expressjs.com#1553

@jonchurch
Copy link
Member Author

Right, default token perms are read on prs

I think it needs this update


permissions:
  pull-requests: write
  issues: write

@bjohansebas
Copy link
Member

I still think it wouldn't work, we tried that in expressjs/expressjs.com#1557 and it didn't work. The only way we believe might work, although we haven't tested it yet, is by creating a token with the permissions as attempted in expressjs/expressjs.com#1642.

@jonchurch
Copy link
Member Author

Ah, it's the permission model for forks mucking that up in the linked issue.

See the docs for GH_TOKEN perms, there's a note about this at the bottom.

I'll push an update to allow this to run based on PRs from forks.

@jonchurch
Copy link
Member Author

jonchurch commented Nov 6, 2024

After looking into this, idk that there's a safe way to do this tbh. We can use pull_request_target to give forks the right permissions, but then they could possibly leak the token w/ those elevated perms. Same w/ the PAT approach, could trigger a workflow run, alter the workflow file in the PR, leak the token.

The original PR works as long as folks aren't opening PRs based on forks, but creating branches here in the repo.

A github bot or github app might be the safest way to do this, can edit the PRs w/o having elevated permissions in a workflow file that can be altered by PRs coming from forks.

TIL: the permission model of GHA is pretty wild

@wesleytodd
Copy link
Member

Can we change the name to RFC? I thought we had discussed that, but maybe with this new automation it is the time to do it?

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