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

PR deployment fails often due to mergability check #1327

Closed
nguyentvan7 opened this issue Nov 9, 2023 · 1 comment · Fixed by #1378
Closed

PR deployment fails often due to mergability check #1327

nguyentvan7 opened this issue Nov 9, 2023 · 1 comment · Fixed by #1378
Labels
bug Something isn't working

Comments

@nguyentvan7
Copy link
Collaborator

We are using pull_request_target action trigger so forks can have secret permission. Compared to the default pull_request action trigger, pull_request_target does not wait for the mergability check to complete before triggering the action.

This means the merge commit is commonly not created by the time we start the action, and so the action will actually run on an old commit, compared to the head of the PR.

From how I understand it, adding a sleep/delay to the action will not fix it, as once the action starts, all the local variables, such as the merge commit reference, are locked in and unchangeable.

actions/checkout#518 (comment)

@nguyentvan7 nguyentvan7 added the bug Something isn't working label Nov 9, 2023
@nguyentvan7
Copy link
Collaborator Author

We could resolve this by fetching the merge commit via rest API during the action

However this is unsafe, as the commit the action was approved on could be different than what is fetched, if a malicious actor did something between the safe commit and us fetching the merge commit.

There is probably some way to validate the merge commit we fetched matches the head of the PR at the time of running the action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant