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

Run compressed-size on pull_request_target #10707

Closed
wants to merge 1 commit into from
Closed

Conversation

kurkle
Copy link
Member

@kurkle kurkle commented Sep 26, 2022

This should enable it to comment, because the version already in master will be executed.

https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target

@LeeLenaleee
Copy link
Collaborator

If I understand it correct this will make it that if I make branch X and open a PR against master it will run the size limit against branch X in the PR and not against the current changes of X in master already?

@kurkle
Copy link
Member Author

kurkle commented Sep 26, 2022

I think it should use the PR code, just the action is taken from master. (so you can not use the action with elevated rights to hijack the repo using a PR)

Copy link
Collaborator

@LeeLenaleee LeeLenaleee left a comment

Choose a reason for hiding this comment

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

Yeah I also read that in the docs, thats why I thought it would run against only the PR.

Downside is that it might work in the PR but than fail in the repo. In this case @stockiNail PR would pass the size limit as I understand it correctly but then fail in master since my PR was also in there.

But security is more important so if this issue starts to arrise we will need to give it a look then or throw a PR after it that increases the size limit 👍🏻

@LeeLenaleee LeeLenaleee added this to the Version 4.0 milestone Sep 26, 2022
@kurkle
Copy link
Member Author

kurkle commented Sep 26, 2022

Dejavu #8440

Nothing noted why it was reverted, but I'm guessing its just what @LeeLenaleee said, checking the wrong code for size.
There has been a update from checkout v2 to checkout v3, that could have an effect to this. Will have a look.

@LeeLenaleee
Copy link
Collaborator

What was the reason then for the revert?

#8454

@kurkle
Copy link
Member Author

kurkle commented Sep 26, 2022

Closing as this would not work as expected or if tweaked to checkout the PR code, it would be unsafe.

@kurkle kurkle closed this Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants