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

Revert pull_request action back #337

Open
miroslavpojer opened this issue Jul 21, 2023 · 3 comments
Open

Revert pull_request action back #337

miroslavpojer opened this issue Jul 21, 2023 · 3 comments

Comments

@miroslavpojer
Copy link
Contributor

This issue is related to:

Status in time of writing this issue is that we are using pull_request_target instead of usage pull_request in yml files:

  • build-scala2.12-spark3.2.yml
  • build-scala2.13-spark3.2.yml

This change was needed to be able to merge Approved changes from repository fork.
We were not able to find any better solution in time of change.

Now, when merge is done we are in state where another changes can come inside repository from forks.
It open potential security risk which was solved by simple change configuration for now.

  • Configuration change: PR from fork will start GH actions after team member confirmation. This can avoid start of potential dangerous GH action with writing rights.

Fast fix:

  • Return back pull_request action to both yml files mentioned above.

Slow fix:

  • Invite DevOps colleagues into problem solution and do analysis what changes needs to be done.
@cerveada
Copy link
Collaborator

Another idea that could help with this is to split tests and test coverage in separated actions.

  • Tests don't need the write permission at all and can happily run using pull_request
  • Test coverage require the permission, but even if it fails, we can still asses the amount of new untested code easily. Adding to that - external PRs are usually small, so it's even less of a problem.

@kevinwallimann
Copy link
Collaborator

pull_request_target doesn't work like we think. It "runs in the context of the base of the pull request", so it doesn't actually run with the changes from the pull request. It may be useful for tasks like auto-labeling issues, but not to run the build of the PR. It is not a drop-in replacement for pull_request

@cerveada
Copy link
Collaborator

In #339 we reverted to using pull_request and separated the workflows for tests and test coverage.

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

No branches or pull requests

3 participants