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

fix: incorrect workflow checkout ref #62165

Merged
merged 1 commit into from
Nov 14, 2022

Conversation

casswedson
Copy link
Contributor

Summary

None

Purpose of change

we have a little linter/reviewer workflow that's supposed to test the incoming code and spit out a review if there are easy to fix errors say whitespace, indentation etc

it can't do that at the moment cause pull_request_target will make the checkout action checkout cleverraven's master; master is always linted so we aren't doing anything

force it to check the incoming code, a little trick (see references) - a hack
very much a dirty hack this get us the pr's head ref, something we can run actual tests on and still spit a review because we are using both data from the pull_request event and permissions from the pull_request_target event

the ugly downside of this is that running on both events, we'll have duplicated workflows, I don't know at this moment if I can somehow remove that, I can however skip the workflow if it isn't running on pull_request_target

if this works it would 100% be worth the ugly bit, automatic reviews are a dream I don't think anyone on github has

references:
https://semgrep.dev/r?q=yaml.github-actions.security.pull-request-target-code-checkout.pull-request-target-code-checkout

Describe the solution

# run the workflow on 2 events one has the permissions to make reviews
# the other has data this needs
on:
  pull_request_target:
  pull_request:

jobs:
# skip the workflow on one of the events, the one that doesn't have write
# permissions
  lint:
    if: ${{ github.event_name == 'pull_request_target' }}

Describe alternatives you've considered

Testing

casswedson#91 see here the workflows skipping on pull_request and the other failing because it finds a style error, after that it sends a review with corrections

Additional context

I was like: why isn't the linter working? it was checking out master all along, I didn't realize in #62060 that pr_target forces running workflows on the master files

duh. you didn't know that


one quick look at the workflow after this will determine if this dream is possible or not, I'll just remove it if it doesn't work

we have a litte linter/reviewer workflow that's supposed to test the
incoming code and spit out a review if there are easy to fix errors
say whitespace, indentation etc

it can't do that at the moment cause pull_request_target will make the
checkout action checkout master; master is always linted so we aren't
doing anything

force it to check the incoming code, a little trick

references:
https://semgrep.dev/r?q=yaml.github-actions.security.pull-request-target-code-checkout.pull-request-target-code-checkout
@github-actions github-actions bot added Code: Tooling Tooling that is not part of the main game but is part of the repo. json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Nov 11, 2022
@dseguin dseguin merged commit b47895a into CleverRaven:master Nov 14, 2022
@casswedson casswedson deleted the ref-workaround branch November 14, 2022 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions Code: Tooling Tooling that is not part of the main game but is part of the repo. json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants