Skip to content

github: Fix outdated aspects of Comment-on-PR.yml and Notify-Convention-Change.yml#291

Merged
mshafer-NI merged 5 commits intoni:mainfrom
bkeryan:users/bkeryan/update-actions
Mar 17, 2026
Merged

github: Fix outdated aspects of Comment-on-PR.yml and Notify-Convention-Change.yml#291
mshafer-NI merged 5 commits intoni:mainfrom
bkeryan:users/bkeryan/update-actions

Conversation

@bkeryan
Copy link
Collaborator

@bkeryan bkeryan commented Mar 17, 2026

What does this Pull Request accomplish?

Remove with: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} when calling thollander/actions-comment-pull-request. This parameter was renamed from GITHUB_TOKEN to github-token in v3 and has been generating warnings ever since we upgraded, but the action has still been working without it.

Remove uses: actions/checkout. I'm not aware of any reason why commenting on PRs requires checking out a copy of the repo. Also, one of these workflows was using actions/checkout@main, which Renovate did not pin to a commit SHA because main is a branch, not a tag.

Add permissions: pull-requests: write so that we can lock down the repo's default permissions for GitHub Actions workflows without breaking this workflow.

Add a name for Comment-on-PR.yml so it doesn't show up as .github/workflows/Comment-on-PR.yml in the Actions tab.

Why should this Pull Request be merged?

Comment-on-PR.yml and Notify-Convention-Change.yml use on: pull_request_target. When someone creates a PR from a fork, this causes the workflows to run using a GitHub token with write access to this repo, not the fork. As a result, we should ensure that these workflows do not perform unnecessary actions that expose them to the contents of the fork repo.

What testing has been done?

I committed these changes to the default branch of my personal fork of this repo and created a PR targeting the fork. Comment-on-PR.yml successfully commented on the PR without with: GITHUB_TOKEN: or uses: actions/checkout.

I also tested it with "workflow permissions" set to "Read repository contents and packages permissions".

@github-actions
Copy link
Contributor

Thank you for contributing! 👋

@mshafer-NI mshafer-NI merged commit 8e5cbe9 into ni:main Mar 17, 2026
1 check passed
@bkeryan
Copy link
Collaborator Author

bkeryan commented Mar 17, 2026

FYI @mshafer-NI After you merged this, I tested it with a PR in my personal fork targeting the base repo and it still works.

#292

@bkeryan bkeryan deleted the users/bkeryan/update-actions branch March 17, 2026 16:08
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.

2 participants