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

Use pull_request in validation workflow #56

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

joyeecheung
Copy link
Member

See #55 - using pull_request_target would allow the workflow to run without authorization. While there are some code in this workflow to defend against naive attacks e.g. adding scripts to the actions, there could be other attack vectors e.g. via specially crafted branch names or file names that evade GitHub's escape rules. It would be too hard to wrap our head around this, so the easiest way to defend against it would be to restrict full validation that require access to secrets to PRs opened from branches in this repo.

For PRs opened from forks, I think we should go back to what I proposed in #10 - that is, only run local validations that require no access to secrets, and use pull_request for it.

cc @aduh95 @nodejs/tsc

See #55 - using pull_request_target would allow the workflow to run without authorization. While there are some code in this workflow to defend against naive attacks e.g. adding scripts to the actions, there could be other attack vectors e.g. via specially crafted branch names or file names that evade GitHub's escape rules. It would be too hard to wrap our head around this, so the easiest way to defend against it would be to restrict full validation that require access to secrets to PRs opened from branches in this repo.

For PRs opened from forks, I think we should go back to what I proposed in #10 - that is, only run local validations that require no access to secrets, and use `pull_request` for it.

cc @aduh95 @nodejs/tsc
@joyeecheung joyeecheung requested a review from a team as a code owner January 9, 2025 18:10
@github-actions github-actions bot added the automation Changes to the automation label Jan 9, 2025
@joyeecheung joyeecheung merged commit f8e9222 into main Jan 9, 2025
1 check passed
@joyeecheung joyeecheung deleted the update-target-validation branch January 9, 2025 18:13
@joyeecheung joyeecheung mentioned this pull request Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation Changes to the automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants