-
Notifications
You must be signed in to change notification settings - Fork 8
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
Feature request: Add pull_request_target event type #12
Comments
hey 👋 thanks a lot for the interest. Haven't worked on this action for a long time and I am also not really familiar with how the pr from a fork differs. So I probably need some help as well 😅 Have you tested this flow?
how could you obtain that number? |
Hi there! thanks to you for this action, I was thinking about making it myself until I saw yours. I have tested this flow from my fork: eruizalo/doric#16
It fails because you are checking the event type, which can only be a pull_request:
I thought at first this couldn't be so difficult, but now I am realising this might be more complex than I thought. pull_request event only has read permissions when working on forks, so if we try to write in a PR using this event it will fail (I crashed myself with this using another action). This is done this way to avoid security issues and could be avoided by two different approaches:
I think I blundered the other day when I said the GitHub context were different because I was thinking about one approach I did for commenting on a PR using Give me some days to find this out. If the fork context does not contain the PR number I would do the same as other actions are doing: request it as a param. There are some actions that could obtain this info or maybe the developer can do something to provide it (In order to comment a PR using |
Hello again @gkampitakis, it seems But... the GITHUB_SHA for both events are different so I had to force it in the checkout:
Not sure what would you do to check this case (maybe throw an error if current commit and base branch commit are the same. |
Hey 👋 had a quick look at this but not sure I have the time to work on this, unfortunately. Maybe it is doable but def needs some testing. I am happy to review any pr and help on the way |
Hi @gkampitakis I'm really sorry but I do not really know about typescript. I think it should work as I pushed in my branch, but it seems it does not work if I don't regenerate |
I believe
|
It would be nice to use this action in PRs coming from forks. In order to do this the easiest way is to use the
pull_request_target
event, which has write permissions. This approach is more difficult as you don't know the PR number by the GitHub context, but there are some mechanisms to fetch that.I'm just a newbie in typescript and my created GitHub actions are small and ad-hoc to my project, but if there is any way I could help I will be glad to do so.
The text was updated successfully, but these errors were encountered: