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 issue with workflow on forks #99

Merged
merged 1 commit into from
Nov 15, 2023
Merged

Conversation

chrispine
Copy link
Contributor

This allows the add-to-project workflow to work on forks.

This allows the add-to-project workflow to work on forks.
@chrispine chrispine requested a review from nicksnyder November 15, 2023 17:24
Copy link

vercel bot commented Nov 15, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
connect ✅ Ready (Inspect) Visit Preview Nov 15, 2023 5:24pm

Copy link
Contributor

@emcfarlane emcfarlane left a comment

Choose a reason for hiding this comment

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

On reopened would a duplicate issue be created?

@@ -6,7 +6,7 @@ on:
- opened
- reopened
- transferred
pull_request:
pull_request_target:
Copy link
Member

Choose a reason for hiding this comment

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

I've heard a fair bit about secret exfiltration from repositories using this feature, but I don't know the details. Does allowing forks to access our secrets in this way potentially expose those secrets to code in the fork? Are the tokens here allowed to do anything sensitive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I discussed it with @rubensf who confirmed those issues don't apply in this case, and this is safe to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See also the TL;DR from here:

Combining pull_request_target workflow trigger with an explicit checkout of an untrusted PR is a dangerous practice that may lead to repository compromise.

We aren't doing any checkout here, and are using pull_request_target for its intended purpose: to process PRs safely. Also from that page:

pull_request_target runs in the context of the target repository of the PR, rather than in the merge commit. This means the standard checkout action uses the target repository to prevent accidental usage of the user supplied code.

@chrispine
Copy link
Contributor Author

On reopened would a duplicate issue be created?

This automation doesn't create issues, it just adds them to the project, and an issue cannot be added twice. See this test issue if you want to try it out yourself.

@nicksnyder nicksnyder merged commit 289a438 into main Nov 15, 2023
2 checks passed
@nicksnyder nicksnyder deleted the chrispine/add-to-project-forks branch November 15, 2023 23:40
chrispine added a commit to connectrpc/examples-go that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/connect-kotlin that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/connect-swift that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/connect-go that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/conformance that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/grpcreflect-go that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/vanguard-go that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/examples-es that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/otelconnect-go that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/validate-go that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/connect-es that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/connect-query-es that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/grpchealth-go that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/connect-playwright-es that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/envoy-demo that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/examples-go that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/connect-kotlin that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/connect-swift that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/connect-go that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/conformance that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/grpcreflect-go that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/vanguard-go that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/examples-es that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/otelconnect-go that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/validate-go that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/connect-es that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/connect-query-es that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/grpchealth-go that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/connect-playwright-es that referenced this pull request Nov 16, 2023
chrispine added a commit to connectrpc/envoy-demo that referenced this pull request Nov 16, 2023
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.

4 participants