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

Ability to determine PR number on workflow_run events associated with fork repository PRs. #28

Merged
merged 2 commits into from
Jan 9, 2021

Conversation

bijujoseph
Copy link
Contributor

A solution for issue #27.

The external fork based pull_request gets a GITHUB_TOKEN that have read-only permission. The cobertura-action will not be able to add coverage comments of such PRs.

So, pull_request_number is not provided, it will check to see if the workflow is triggered due to a pull request associated with workflow_run event and appropriately determines the PR number.

…forks gets a read-only token. This causes the workflows involving `cobertura-action` fail, due to lack of permission.

> `Error: Resource not accessible by integration

Actions team recently introduced `pull_request_target`, but that pose additional security risks. To mitigate that, they recommend to split these workflows, a well-written recipe can be found in 5monkeys#2

Certainly, `cobertura-action` accepts `pull_request_number` parameter, but that will put the onus on the workflow author. So, inorder to keep things simple, this PR adds functionality to determine correctly the PR number on `workflow_run` events.  

References:
- 1 https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/
- 2 https://securitylab.github.com/research/github-actions-preventing-pwn-requests
…forks gets a read-only token. This causes the workflows involving `cobertura-action` fail, due to lack of permission.

> `Error: Resource not accessible by integration

Actions team recently introduced `pull_request_target`, but that pose additional security risks. To mitigate that, they recommend to split these workflows, a well-written recipe can be found in 5monkeys#2

Certainly, `cobertura-action` accepts `pull_request_number` parameter, but that will put the onus on the workflow author. So, inorder to keep things simple, this PR adds functionality to determine correctly the PR number on `workflow_run` events.  

References:
- 1 https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/
- 2 https://securitylab.github.com/research/github-actions-preventing-pwn-requests
@bijujoseph
Copy link
Contributor Author

bijujoseph commented Jan 4, 2021

Ironically this PR is failing due to exact same reason. Like the recipe in https://securitylab.github.com/research/github-actions-preventing-pwn-requests, we might have to split the workflow, to get these sort of PRs to pass.
OR you must use a Personal Access token that has write permission in https://github.com/5monkeys/cobertura-action/blob/master/.github/workflows/main.yml#L28

Copy link
Contributor

@hannseman hannseman left a comment

Choose a reason for hiding this comment

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

Thanks! I'll look into splitting the workflow.

@hannseman hannseman merged commit aaa1635 into 5monkeys:master Jan 9, 2021
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.

2 participants