Skip to content

fix(actions): don't use pull_request_target#13666

Closed
allendema wants to merge 1 commit intolinuxmint:masterfrom
allendema:mod-actions-dont-run-on-target
Closed

fix(actions): don't use pull_request_target#13666
allendema wants to merge 1 commit intolinuxmint:masterfrom
allendema:mod-actions-dont-run-on-target

Conversation

@allendema
Copy link
Copy Markdown
Contributor

Why

pull_request_target is insecure.

What

Don't use pull_request_target.
Update and pin action.

Context

https://research.jfrog.com/post/part-1-pull-request-target-exploitation/#:~:text=pull%5Frequest%5Ftarget,-%2C%20where

See its usage here: https://github.com/linuxmint/cinnamon/actions?query=event%3Apull_request_target.

TODO

Remove all instances of https://github.com/search?q=org%3Alinuxmint+pull_request_target&type=code.
Update description in https://github.com/linuxmint/github-actions/blob/master/install-deps/action.yml#L6.

All actions in the org need to be updated, pinned.
Workflows in https://github.com/linuxmint should be completly rethinked.

Tests welcome.

mod(actions): update and pin action

Signed-off-by: Allen <64094914+allendema@users.noreply.github.com>
Copy link
Copy Markdown

@nilsreichardt nilsreichardt left a comment

Choose a reason for hiding this comment

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

FYI: This will break the workflow for every pull request coming from a fork (outside contributors) because GITHUB_TOKEN will be empty with pull_request.

@mtwebster
Copy link
Copy Markdown
Member

I don't think we're at risk here.

I use pull_request_target to allow our pattern-check script to post a comment on the pull request when it completes. Any other route to achieve this is a lot more painful, and for no gain:

I'm not sold on changing action versioning - will we lose notifications when these go out-of-date (like dependabot fly-bys, etc...)?

@mtwebster mtwebster closed this Mar 26, 2026
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.

3 participants