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

ci: remove pull request target usages #676

Merged
merged 1 commit into from
Jul 18, 2024
Merged

ci: remove pull request target usages #676

merged 1 commit into from
Jul 18, 2024

Conversation

enocom
Copy link
Member

@enocom enocom commented Jul 18, 2024

The pull request target is inherently fragile and prone to security vulnerabilities. The only reason we used it was to put our testing project name, our testing service account email (not key), and WIF provider pool ID into secrets. In fact, all three of those values aren't necessarily secrets and work just as well in environment variables.

We will still need to vet a PR before clicking "approve and run," but that's a much smaller attack surface area than pull request target.

@enocom enocom requested a review from a team as a code owner July 18, 2024 20:17
@enocom enocom force-pushed the no-pr-target branch 3 times, most recently from 4ff00ce to 64758ba Compare July 18, 2024 20:35
Copy link
Contributor

@jackwotherspoon jackwotherspoon left a comment

Choose a reason for hiding this comment

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

LGTM

@jackwotherspoon jackwotherspoon changed the title chore: remove pull request target usages ci: remove pull request target usages Jul 18, 2024
The pull request target is inherently fragile and prone to security
vulnerabilities. The only reason we used it was to put our testing
project name, our testing service account email (not key), and WIF
provider pool ID into secrets. In fact, all three of those values aren't
necessarily secrets and work just as well in environment variables.

We will still need to vet a PR before clicking "approve and run," but
that's a much smaller attack surface area than pull request target.
@enocom enocom merged commit 0ae249a into main Jul 18, 2024
18 checks passed
@enocom enocom deleted the no-pr-target branch July 18, 2024 21:41
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