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 workflow permission check logic #4084

Merged
merged 4 commits into from
Jul 19, 2024
Merged

Conversation

tonidero
Copy link
Contributor

Description

This improves the permission checking logic for running github actions. We also have it setup to only run actions by organization members, but this acts as an additional check.

@tonidero tonidero added the ci label Jul 19, 2024
@tonidero tonidero force-pushed the fix-trigger-all-test-workflow-logic branch from 28cc004 to 101e62a Compare July 19, 2024 07:29
@tonidero tonidero changed the base branch from main to andy/test_ci_triggers_2 July 19, 2024 07:29
@tonidero
Copy link
Contributor Author

@RCGitBot please test

1 similar comment
@tonidero
Copy link
Contributor Author

@RCGitBot please test

Base automatically changed from andy/test_ci_triggers_2 to main July 19, 2024 07:52
@tonidero tonidero marked this pull request as ready for review July 19, 2024 08:02
Copy link
Member

@JayShortway JayShortway left a comment

Choose a reason for hiding this comment

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

LGTM!

@tonidero
Copy link
Contributor Author

@RCGitBot please test

@tonidero
Copy link
Contributor Author

Will merge this for now, we can figure any other issues in followup PRs

@tonidero tonidero merged commit d9f783b into main Jul 19, 2024
4 checks passed
@tonidero tonidero deleted the fix-trigger-all-test-workflow-logic branch July 19, 2024 08:40
Copy link
Member

@aboedo aboedo left a comment

Choose a reason for hiding this comment

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

thanks for taking care of it!

id: verify
# ensure that only RevenueCat members can trigger this
run: |
RESPONSE=$(curl https://api.github.com/orgs/RevenueCat/members/${{ github.event.comment.user.login }})
if [[ "$RESPONSE" == *"Not Found"* ]]; then
RESPONSE=$(curl -s -o /dev/null --head -w "%{http_code}" -H "Authorization: Bearer $READ_ORG_GITHUB_TOKEN" https://api.github.com/orgs/RevenueCat/members/${{ github.event.comment.user.login }})
Copy link
Member

Choose a reason for hiding this comment

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

we should probably add a comment and maybe use \ to split this up into lines for clarity

@vegarotest
Copy link

@RCGitBot please test

tonidero added a commit that referenced this pull request Jul 22, 2024
### Description
Small cleanup from comments in #4084
@fire-at-will fire-at-will mentioned this pull request Jul 23, 2024
nyeu pushed a commit that referenced this pull request Oct 2, 2024
### Description
This improves the permission checking logic for running github actions.
We also have it setup to only run actions by organization members, but
this acts as an additional check.

---------

Co-authored-by: Andy Boedo <[email protected]>
nyeu pushed a commit that referenced this pull request Oct 2, 2024
### Description
Small cleanup from comments in #4084
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants