Why is GITHUB_TOKEN necessary?

[ajschmidt8]

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).

[ajschmidt8]

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

action-semantic-pull-request/src/index.js

Lines 38 to 46 in 0b14f54

	// The pull request info on the context isn't up to date. When
	// the user updates the title and re-runs the workflow, it would
	// be outdated. Therefore fetch the pull request via the REST API
	// to ensure we use the current title.
	const {data: pullRequest} = await client.rest.pulls.get({
	owner,
	repo,
	pull_number: contextPullRequest.number
	});

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

[ajschmidt8]

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

• fix: Always use latest title #1

[amannn]

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!

[lucasgonze]

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.

[ajschmidt8]

@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):

action-semantic-pull-request/src/index.js

Lines 149 to 162 in ff373f4

	await client.request('POST /repos/:owner/:repo/statuses/:sha', {
	owner,
	repo,
	sha: pullRequest.head.sha,
	state: newStatus,
	target_url: 'https://github.com/amannn/action-semantic-pull-request',
	description: isWip
	? 'This PR is marked with "[WIP]".'
	: validationError
	? 'PR title validation failed'
	: 'Ready for review & merge.',
	context: 'action-semantic-pull-request'
	});
	}

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.

[lucasgonze]

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).

[ajschmidt8]

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...

[actuarysailor]

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

Same issue if I updated to the most recent version too.

[actuarysailor]

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

Same issue if I updated to the most recent version too.

Note the error seems to be because I am using this as a shared workflow; e.g. workflow_call. If I run it directly within my repo, it works fine