Skip to content
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions .github/workflows/release-please.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ on:
name: release-please

permissions:
contents: write
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about this, but per https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#permissions, "If you specify the access for any of these permissions, all of those that are not specified are set to none", which means contents will be none for steps that use GITHUB_TOKEN? Docs for trusted publishing (https://docs.npmjs.com/trusted-publishers#step-2-configure-your-cicd-workflow) have contents: read in the example, so perhaps it would be safer to change write -> read instead of just removing the line?

Copy link
Member Author

@lumirlumir lumirlumir Dec 19, 2025

Choose a reason for hiding this comment

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

I think it would still work well if we remove contents: write or contents: read for the following reasons:


You can use permissions to modify the default permissions granted to the GITHUB_TOKEN

According to https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#permissions, the permissions field applies to GITHUB_TOKEN, not to Personal Access Tokens (fine-grained or classic) such as WORKFLOW_PUSH_BOT_TOKEN. (This change now removes the use of GITHUB_TOKEN.)

As far as I know, the permissions for WORKFLOW_PUSH_BOT_TOKEN are set when the token is created, not by the explicit permissions field in GitHub Actions. For example, with the following token settings (the example below shows a classic token):

image

Docs for trusted publishing (https://docs.npmjs.com/trusted-publishers#step-2-configure-your-cicd-workflow) have contents: read in the example, so perhaps it would be safer to change write -> read instead of just removing the line?

As far as I know, content: write or contents: read aren't related to OIDC; only id-token is:

image

For example, when I publish my personal open-source packages, I only use id-token: write, and it works fine without contents: read.

Ref: https://github.com/lumirlumir/npm-clang-format-node/blob/5eeae2da116c405395eaf13391500c18e416f421/.github/workflows/publish.yml#L7-L8

image

Also, in https://github.com/eslint/css/pull/330/changes, we removed the contents: write field as shown below, yet the auto-created workflow was generated correctly and the CI ran successfully.

image

Given the reasons above, I think this would work well; however, if you’re still unsure, I’m happy to follow your guidance and update the workflow to use contents: read.

Copy link
Member

Choose a reason for hiding this comment

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

According to https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#permissions, the permissions field applies to GITHUB_TOKEN, not to Personal Access Tokens (fine-grained or classic) such as WORKFLOW_PUSH_BOT_TOKEN. (This change now removes the use of GITHUB_TOKEN.)

Yes, but only one step uses WORKFLOW_PUSH_BOT_TOKEN, while the others still use GITHUB_TOKEN?

Also, in https://github.com/eslint/css/pull/330/changes, we removed the contents: write field as shown below, yet the auto-created workflow was generated correctly and the CI ran successfully.

In that change, we entirely removed permissions, in which case I believe contents defaults to read. Also, the steps are different than here and maybe those steps don't require contents: read.

For example, when I publish my personal open-source packages, I only use id-token: write, and it works fine without contents: read.

Ref: https://github.com/lumirlumir/npm-clang-format-node/blob/5eeae2da116c405395eaf13391500c18e416f421/.github/workflows/publish.yml#L7-L8

Then maybe it isn't true that trusted publishing requires contents: read or that specifying any permissions automatically sets the others to none. However, I think it's safer to go by the docs and specify contents: read.

Copy link
Member Author

@lumirlumir lumirlumir Dec 23, 2025

Choose a reason for hiding this comment

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

However, I think it's safer to go by the docs and specify contents: read.

First of all, I followed your suggestion and updated it in 9dc5ae9.

Just a small note: contents: read would matters for private repositories (since the read permission is also restricted in private repositories) and can affect some resuable GitHub Actions workflows or OIDC. However, since the repositories where we publish packages are public, the behavior would be the same whether it’s specified as contents: read or not. That said, being explicit could also be a good option :)

pull-requests: write
id-token: write

jobs:
Expand All @@ -17,7 +15,7 @@ jobs:
- uses: googleapis/release-please-action@v4
id: release
with:
token: ${{secrets.GITHUB_TOKEN}}
token: ${{ secrets.WORKFLOW_PUSH_BOT_TOKEN }}

# Output which releases were created
- name: Output release info
Expand Down