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

Project Management Automation: Avoid milestone task for forks #20049

Merged
merged 1 commit into from
Feb 5, 2020

Conversation

aduth
Copy link
Member

@aduth aduth commented Feb 5, 2020

Previously: #20021
Related: #17324

The milestone Action currently generate errors for merged pull requests originating from a fork repository:

https://github.com/WordPress/gutenberg/commit/084f38a21662a27d00bb3d32f350e9141f9caf38/checks?check_suite_id=439313372#step:4:12

The milestone task was originally omitted from the conditional runs implemented in #20021 based on the incorrect assumption the merge event would be considered with higher permissions, even if the pull request being merged originated from a fork. This is clearly not the case, and it seems all pull_request events will be subject to these restrictions.

https://help.github.com/en/actions/automating-your-workflow-with-github-actions/authenticating-with-the-github_token#permissions-for-the-github_token

Understandably, this diminishes the value of this automation, as we can't rely that the milestone will have been applied.

Separately, there might be alternatives to explore here, where the event is handled in response to the commit as it ends up being pushed to master. In other words, some combination of:

@aduth aduth added [Type] Bug An existing feature does not function as intended [Package] Project management automation /packages/project-management-automation labels Feb 5, 2020
@aduth aduth requested a review from epiqueras February 5, 2020 13:41
@youknowriad
Copy link
Contributor

Is it possible to comment on the PR and ping the merger otherwise? or are comments limited in the same way?

@aduth
Copy link
Member Author

aduth commented Feb 5, 2020

Is it possible to comment on the PR and ping the merger otherwise? or are comments limited in the same way?

If it happens in response to the pull_request event (the merging of the pull request), my understanding is that all of its permissions are read-only:

https://help.github.com/en/actions/automating-your-workflow-with-github-actions/authenticating-with-the-github_token#permissions-for-the-github_token

There's a note there about custom tokens as secrets. Based on the reasons these permissions are read-only in the first place (limiting what can be done from forks), I'm not entirely sure about how secure this would be.

Copy link
Contributor

@epiqueras epiqueras left a comment

Choose a reason for hiding this comment

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

GitHub is aware of this limitation and working on an alternative. Let's do this for now:

Separately, there might be alternatives to explore here, where the event is handled in response to the commit as it ends up being pushed to master. In other words, some combination of:

@aduth Do you want to tackle it, or should I?

@epiqueras epiqueras merged commit 525dd41 into master Feb 5, 2020
@epiqueras epiqueras deleted the fix/pm-automation-merge-permissions branch February 5, 2020 14:29
@github-actions github-actions bot added this to the Gutenberg 7.5 milestone Feb 5, 2020
@aduth
Copy link
Member Author

aduth commented Feb 5, 2020

@aduth Do you want to tackle it, or should I?

I'll likely not be able to prioritize this 'til after next week's 5.4 beta. Feel free to take it up if you're interested, otherwise I can plan to revisit it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Project management automation /packages/project-management-automation [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants