3 ms·

> Requiring the PR to be closed before the code runs would solve this. That's not going to work! You want to make sure all the tests that run as part of the CI

> Requiring the PR to be closed before the code runs would solve this. That's not going to work! You want to make sure all the tests that run as part of the CI pass before you merge. What you can do is to make a blanket ban on auto-running the CI pipeline if the CI config was changed till the maintainer clicks Run Actions.

serial_dev 6y ago
That would also not work for multiple reasons. This simple rule change would defeat the purpose of running CI CD on external contributions: I want to see if the tests run and everything is up to my quality standards. I don't want to manually trigger the pipeline, that adds around 5 minute to every PR I receive... However, as an attacker, I can still execute anything I want. Sure, maybe it's not as convenient as replacing the yml file, but I could embed a script in the tests that will just mine as long as possible. The point is that you didn't solve anything, you just ruined CI CD
- smarx007 6y ago
  > that adds around 5 minute to every PR I receive... I am sure not every PR you get touches files under '.github/workflows'. Are you sure you were replying to me and not @_fat_santa? It's his approach that ruins CI/CD as far as tests are concerned. > Sure, maybe it's not as convenient as replacing the yml file, but I could embed a script in the tests that will just mine as long as possible. Yes, but that slows things down a lot. Now you need to write a fake unit test that spawns a process and that will require to clone a project, get a project to build, writing different code for different programming languages and unit test frameworks... Also, you would not be able to defeat my runtime timeouts (just added to https://github.com/eclipse/lyo/blob/master/.github/workflows/maven.yml#L12 https://github.com/eclipse/lyo/blob/master/.github/workflows... after reading this post) if GH tarpits CI execution with manual approvals if you touch that config in your PR.

serial_dev 6y ago

That would also not work for multiple reasons. This simple rule change would defeat the purpose of running CI CD on external contributions: I want to see if the tests run and everything is up to my quality standards. I don't want to manually trigger the pipeline, that adds around 5 minute to every PR I receive... However, as an attacker, I can still execute anything I want. Sure, maybe it's not as convenient as replacing the yml file, but I could embed a script in the tests that will just mine as long as possible. The point is that you didn't solve anything, you just ruined CI CD

smarx007 6y ago
> that adds around 5 minute to every PR I receive... I am sure not every PR you get touches files under '.github/workflows'. Are you sure you were replying to me and not @_fat_santa? It's his approach that ruins CI/CD as far as tests are concerned. > Sure, maybe it's not as convenient as replacing the yml file, but I could embed a script in the tests that will just mine as long as possible. Yes, but that slows things down a lot. Now you need to write a fake unit test that spawns a process and that will require to clone a project, get a project to build, writing different code for different programming languages and unit test frameworks... Also, you would not be able to defeat my runtime timeouts (just added to https://github.com/eclipse/lyo/blob/master/.github/workflows/maven.yml#L12 https://github.com/eclipse/lyo/blob/master/.github/workflows... after reading this post) if GH tarpits CI execution with manual approvals if you touch that config in your PR.