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

Is pull_request_target required for forks (doc may be outdated) #219

Open
davidhsingyuchen opened this issue Nov 22, 2022 · 2 comments
Open

Comments

@davidhsingyuchen
Copy link

Describe the bug

From the pull_request section in Event triggers:

If this configuration is used and a pull request from a fork is opened, you'll encounter an error as the GitHub token environment parameter is not available.

However, according to Github documentation, the main difference between pull_request_target and pull_request is that the former also gives write permissions, which does not seem to be required by action-semantic-pull-request.

Furthermore, a run triggered from a fork did finish successfully even though it's pull_request instead of pull_reuqest_target in the corresponding workflow file.

As a result, the documentation may be outdated.

To reproduce

  1. Use this action and configure pull_request to be the triggering events.
  2. Create a PR from a fork. The check should not work according to the documentation, but it works.

Expected behavior

Updated documentation.

@amannn
Copy link
Owner

amannn commented Nov 23, 2022

Hey, and thank you for raising this question! There's a chance that we could get rid of the token, maybe that would allow us to recommend a single event trigger in the future: #218. It's likely possible that the docs became outdated at some point, unfortunately I don't have the time currently to look into this in detail.

ningziwen added a commit to runfinch/common-tests that referenced this issue Nov 23, 2022
*Description of changes:*
PR fixes the TODOs by repos being private

Reference:
amannn/action-semantic-pull-request#219

*Testing done:*
N/A



- [ X ] I've reviewed the guidance in CONTRIBUTING.md


#### License Acceptance

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

Signed-off-by: Ziwen Ning <ningziwe@amazon.com>
ningziwen added a commit to runfinch/finch-core that referenced this issue Nov 23, 2022
*Description of changes:*
PR fixes the TODOs by repos being private

Reference:
amannn/action-semantic-pull-request#219


*Testing done:*
N/A

- [ X ] I've reviewed the guidance in CONTRIBUTING.md


#### License Acceptance

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

Signed-off-by: Ziwen Ning <ningziwe@amazon.com>
@lucasgonze
Copy link

I'd like to vote for this issue. The use of pull_request_target triggers Dangerous-Workflow in OpenSSF Scorecard because it permits a malicious repo or action to take advantage of the write permission. https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

BlairCurrey added a commit to BlairCurrey/trpc-koa-adapter that referenced this issue Mar 17, 2024
- should work with forks and avoids potential secutity issue?
amannn/action-semantic-pull-request#219
BlairCurrey added a commit to BlairCurrey/trpc-koa-adapter that referenced this issue Mar 20, 2024
* ci: add lint pr workflow

* chore: make first mention of tRPC and Koa links

* chore: manual trigger for test

* Revert "chore: manual trigger for test"

This reverts commit bb5efac.

* chore: attempt to trigger on pr

* Revert "chore: attempt to trigger on pr"

This reverts commit a8a192d.

* chore: switch trigger

- should work with forks and avoids potential secutity issue?
amannn/action-semantic-pull-request#219

* ci: run tests against node version 16, 18, 20

* ci: fix node 16 tests

* chore: rm commit msg lint

linting in PR title. individual commits technically dont
matter because they are squash/merged.

* chore: rm typo item for unused pkg manager file

* chore: make eslint stricter, apply to test

* ci: add lint, format to gh workflow

* chore: rm husky hooks

* fix: gh action lint, install order

* chore: update package lock
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

No branches or pull requests

3 participants