action-semantic-pull-request Why is `GITHUB

I'm wondering why a GITHUB_TOKEN is necessary for this action to work correctly.

Both pull_request and pull_request_target events deliver a payload that contains the current pull request title (src). The full payload is available via the github context under the event property (src).

Nov 22 '22 16:11 ajschmidt8

I see the code and comment below, which indicates that The pull request info on the context isn't up to date.

https://github.com/amannn/action-semantic-pull-request/blob/0b14f54ac155d88e12522156e52cb6e397745cfd/src/index.js#L38-L46

This leads me to believe that the authenticated client.rest.pulls.get call was implemented to account for potential race conditions when multiple events are fired in quick succession. Is that correct? If so, it seems like a more appropriate solution could be to use GitHub Actions' native concurrency settings below to ensure that only the latest event is ever used.

https://docs.github.com/en/actions/using-jobs/using-concurrency

Nov 22 '22 16:11 ajschmidt8

I traced that comment back to the PR below. It doesn't seem that there's any additional context in the PR description.

https://github.com/amannn/action-semantic-pull-request/pull/1

Nov 22 '22 16:11 ajschmidt8

Hey @ajschmidt8 and thank you for your comment!

The reason why I added the necessity for the token is described in the comment from the code snippet you've linked to above. If you know how to avoid this while still being able to support the same feature set, I'd be more than happy to review a PR!

Nov 23 '22 08:11 amannn

If so, it seems like a more appropriate solution could be to use GitHub Actions' native concurrency settings below to ensure that only the latest event is ever used.

@ajschmidt8 Can you say more about why this is a race condition?

Let's discuss what the concurrency fix is.

Apr 25 '23 17:04 lucasgonze

@ajschmidt8 Can you say more about why this is a race condition?

Let's discuss what the concurrency fix is.

If a PR title is updated multiple times in quick succession, then it could trigger the workflow to run multiple times in parallel.

This could cause problems depending on the details of the title changes on each trigger.

To prevent this workflow from running multiple times in parallel, you could set up concurrency limits like this:

name: "Lint PR"

on:
  pull_request_target:
    types:
      - opened
      - edited
      - synchronize

concurrency:
  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: true

jobs:
  main:
    name: Validate PR title
    runs-on: ubuntu-latest
    steps:
      - uses: amannn/action-semantic-pull-request@v5

This would ensure that any pending workflow runs are canceled before a new one begins.

I don't think that's related to the GITHUB_TOKEN issue here though.

Upon further inspection of the source code, it's the following code block that requires users to use the pull_request_target event (which has write permissions): https://github.com/amannn/action-semantic-pull-request/blob/ff373f4e8056b732dfd0eadd42ae54c004e5523b/src/index.js#L149-L162

This command writes a custom status back to the PR depending on the results of the function.

The default GITHUB_TOKEN provided to pull requests doesn't have write permissions, only read permissions (see docs here).

Composite actions aren't typically supposed to write additional statuses to PRs. They're simply supposed to run some code and exit with a non-zero code if there are problems.

Writing additional statuses is what requires a GITHUB_TOKEN with write permissions and adds weird complexity to this action.

Apr 25 '23 19:04 ajschmidt8

If a PR title is updated multiple times in quick succession, then it could trigger the workflow to run multiple times in parallel.

This could cause problems depending on the details of the title changes on each trigger.

I think that's true. Also, the workaround of calling the Github API wouldn't fix it systematically, just reduce the frequency by adding a little latency. Also, I think you're right that this isn't why the pull_request_target trigger is necessary.

I'm not sure that pull_request_target changes the privilege level of the token, though.

The architecture documented in the securitylab.github.com blog might be the fix:

Together with the pull_request_target, a new trigger workflow_run was introduced to enable scenarios that require building the untrusted code and also need write permissions to update the PR with e.g. code coverage results or other test results. To do this in a secure manner, the untrusted code must be handled via the pull_request trigger so that it is isolated in an unprivileged environment. The workflow processing the PR should then store any results like code coverage or failed/passed tests in artifacts and exit. The following workflow then starts on workflow_run where it is granted write permission to the target repository and access to repository secrets, so that it can download the artifacts and make any necessary modifications to the repository or interact with third party services that require repository secrets (e.g. API tokens).

Apr 25 '23 20:04 lucasgonze

Agreed, the API hits to read pull request data shouldn't be necessary. Those details are included in the payload provided by GitHub.

pull_request_target does change the permission though.

Check the details here: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target

Specifically the red warning (emphasis mine):

Warning: For workflows that are triggered by the pull_request_target event, the GITHUB_TOKEN is granted read/write repository permission unless the permissions key is specified and the workflow can access secrets, even when it is triggered from a fork...

Apr 25 '23 20:04 ajschmidt8

I have the secrets.GITHUB_TOKEN defined, but still getting error.