-
Notifications
You must be signed in to change notification settings - Fork 419
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
Description seems dangerous #755
Comments
Hello @simon-friedberger |
@simon-friedberger Just found this issue. According to the linked securitylab.github.com page:
That means (as far as I understand) as long as you don't check out and use the PR source code within the same workflow it is secure to use As pull_request_target switches the context to your repository there is no insecure code executed per default |
I mostly agree. However, the difference between checking out the base source and the PR source can be very non-obvious
and I expect many people will intentionally check it out. After all, they are trying to label the pull request based on something that is inside the pull request. In other words, I agree with you on the technical points I just think the "as long as you don't" part will not be sufficiently clear to people. |
To make a proposal:
|
Lines 231 to 257 in 506e1a0
|
From the docs, emphasis mine:
Afaict this contradicts, e.g.: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/ .
Using
pull_request_target
is not safe at all and setting additional permissions is necessary.The text was updated successfully, but these errors were encountered: