Skip to content

fix: validation of PRs from forks - #31

Merged
joyeecheung merged 3 commits into
nodejs:mainfrom
aduh95:validate-prs-from-forks
Dec 21, 2024
Merged

fix: validation of PRs from forks#31
joyeecheung merged 3 commits into
nodejs:mainfrom
aduh95:validate-prs-from-forks

Conversation

@aduh95

@aduh95 aduh95 commented Dec 13, 2024

Copy link
Copy Markdown
Contributor

By switching from pull_request event to pull_request_target, the workflow would have access to repo secrets. This should allow proper validation for PRs opened from a fork.

This does come with more risks of leaking something, so I added a few protection mechanisms, and for extra piece of mind, I would recommend changing the setting below to the stricter "Require approval for all external contributors".

image

Comment thread .github/workflows/validate.yml
Comment thread .github/workflows/validate.yml Outdated
@aduh95 aduh95 linked an issue Dec 18, 2024 that may be closed by this pull request

@bmuenzenmeyer bmuenzenmeyer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM - coupled with the config change

@joyeecheung joyeecheung left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I feel that we can just skip running the action when there are any files outside of records/new, but this also LGTM

@joyeecheung

Copy link
Copy Markdown
Member

Changed the permission

Screen Shot 2024-12-21 at 22 45 23

@joyeecheung
joyeecheung merged commit b094a27 into nodejs:main Dec 21, 2024
@aduh95
aduh95 deleted the validate-prs-from-forks branch December 21, 2024 23:05
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.

Split validation workflow

3 participants