Skip to content

Run integration tests after workflow run #174

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

Merged
merged 3 commits into from
Mar 20, 2025

Conversation

pezholio
Copy link
Contributor

It turns out that pull_request_target runs regardless of permissions, as code that is run is considered to be trusted, which we don’t necessarily want.

Changing this to workflow_run ensures that the action is ONLY run after the rest of the CI runs (which will need to be accepted for external contributions). Like pull_request_target, workflow_run triggers have access to secrets, but won’t get triggered until the other actions are approved and have run.

@coveralls
Copy link

coveralls commented Mar 20, 2025

Pull Request Test Coverage Report for Build 13975687207

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 13974532889: 0.0%
Covered Lines: 424
Relevant Lines: 424

💛 - Coveralls

It turns out that `pull_request_target` runs regardless of permissions,
as code that is run is considered to be trusted, which we don’t
necessarily want.

Changing this to `workflow_run` ensures that the action is ONLY run
after the rest of the CI runs (which will need to be accepted for
external contributions). Like `pull_request_target`, `workflow_run`
triggers have access to secrets, but won’t get triggered until the
other actions are approved and have run.
@pezholio pezholio force-pushed the run-integration-tests-after-workflow-run branch from 9457f6d to 95ffe87 Compare March 20, 2025 17:02
Running Zizmor, we had some suggestions made to tighten security
Copy link
Contributor

@dragon-dxw dragon-dxw left a comment

Choose a reason for hiding this comment

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

@pezholio pezholio force-pushed the run-integration-tests-after-workflow-run branch 2 times, most recently from 426add2 to 06c03ed Compare March 20, 2025 17:29
@pezholio
Copy link
Contributor Author

Annoyingly, we need to access secrets to run the integration tests, but this approach is more secure than the one we added last time

Copy link
Contributor

@dragon-dxw dragon-dxw left a comment

Choose a reason for hiding this comment

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

Given that the CI actions should block external code from running without permission, this is probably better than what stood before, even if it replaces a horrible workflow with a different horrible workflow. Don't like it, will take a closer look tomorrow.

@pezholio pezholio force-pushed the run-integration-tests-after-workflow-run branch from 06c03ed to 2563dfa Compare March 20, 2025 17:32
@pezholio pezholio merged commit 7d2b99c into main Mar 20, 2025
13 of 16 checks passed
@pezholio pezholio deleted the run-integration-tests-after-workflow-run branch March 20, 2025 17:35
@pezholio pezholio mentioned this pull request Mar 21, 2025
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