build: add GitHub token permissions for workflows

[varunsh-coder]

This PR adds minimum token permissions for the GITHUB_TOKEN using https://github.com/step-security/secure-workflows.

GitHub recommends defining minimum GITHUB_TOKEN permissions for securing GitHub Actions workflows

• https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/
• https://docs.github.com/en/actions/security-guides/automatic-token-authentication#modifying-the-permissions-for-the-github_token
• The Open Source Security Foundation (OpenSSF) Scorecards treats not setting token permissions as a high-risk issue

This project is part of the top 100 critical projects as per OpenSSF (https://github.com/ossf/wg-securing-critical-projects), so fixing the token permissions to improve security.

Signed-off-by: Varun Sharma varunsh@stepsecurity.io

[nodejs-github-bot]

Review requested:

• @nodejs/actions

[F3n67u]

@varunsh-coder Thanks for your contribution!

Please amend both the commit message and pr title to "build: add GitHub token permissions for workflows".

"ci" is not a valid subsystem so ci is red now. The first word after subsystem(s) in title should also be lowercase.

Refs: https://github.com/nodejs/node/blob/main/doc/contributing/pull-requests.md#commit-message-guidelines for more details.

[aduh95]

Please amend both the commit message and pr title to "build: add GitHub token permissions for workflows".

"ci" is not a valid subsystem so ci is red now. The first word after subsystem(s) in title should also be lowercase.

Refs: https://github.com/nodejs/node/blob/main/doc/contributing/pull-requests.md#commit-message-guidelines for more details.

To be clear that's a non-blocking concern, amending the commit message can be done by whoever lands this PR – although having well-formatted commit messages is preferable because we can use the automated commit-queue to land the PR.

[aduh95]

nit: we may not need contents permission at all for this workflow since we're using GH_USER_TOKEN to clone the repo – but I suppose it's easier to always have it at the top of each workflow file, it can't hurt on an open-source repo anyway.

[varunsh-coder]

Yes, the contents: read is not strictly needed here. Setting that at the workflow level helps if a new job is added to the workflow. In that case, the new job will have contents: read, which is a secure-by-default setting.

[Trott]

This workflow might open a pull request. Is this setting correct?

[varunsh-coder]

The setting is correct because the credential used to open the PR is not the GITHUB_TOKEN. It is a different secret which has been explicitly setup called GH_USER_TOKEN.

node/.github/workflows/authors.yml

Line 26 in 19cc1e8

GITHUB_TOKEN: ${{ secrets.GH_USER_TOKEN }}

[Trott]

I have a question about a setting that appears to me like it might be too restrictive. But LGTM anyway. Even if a setting somewhere is too restrictive, we can always address it subsequently.

[bnb]

just want to make sure I understand how this was generated correctly, @varunsh-coder: the tool your employer makes just does sniffing on the permissions needed and then adds them to YAML?

I'm mildly averse to taking on a dependency of a tool from a security company I've never heard of, so just want to make sure we're not doing that.

[Trott]

just want to make sure I understand how this was generated correctly, @varunsh-coder: the tool your employer makes just does sniffing on the permissions needed and then adds them to YAML?

I'm mildly averse to taking on a dependency of a tool from a security company I've never heard of, so just want to make sure we're not doing that.

If I understand correctly, this is entirely adding stuff to existing YAML and not adding a tool or anything. There's no "taking on a dependency" here. All this does is add YAML (that could have just as well been hand-coded) to prevent the GitHub token from being used for more privileges than the workflow needs. This limits the potential damage if there is a bug in the workflow that allows an attacker to use the token in ways we did not intend.

[targos]

Instead of specifying the default in all workflows, shouldn't we change it in the repository settings?

[varunsh-coder]

just want to make sure I understand how this was generated correctly, @varunsh-coder: the tool your employer makes just does sniffing on the permissions needed and then adds them to YAML?
I'm mildly averse to taking on a dependency of a tool from a security company I've never heard of, so just want to make sure we're not doing that.

If I understand correctly, this is entirely adding stuff to existing YAML and not adding a tool or anything. There's no "taking on a dependency" here. All this does is add YAML (that could have just as well been hand-coded) to prevent the GitHub token from being used for more privileges than the workflow needs. This limits the potential damage if there is a bug in the workflow that allows an attacker to use the token in ways we did not intend.

This is correct.

In addition, the tool that does this transformation to add permissions key to the YAML workflow files is open source (https://github.com/step-security/secure-workflows).

[varunsh-coder]

Instead of specifying the default in all workflows, shouldn't we change it in the repository settings?

Hi @targos, you can set it at the repository level as well. At the same time, it is a best practice to set the permissions in the workflow files explicitly.

• The permissions are encapsulated with the workflow file, so easier to review and version (same reason applies for other as-code files)
• If someone forks the repo, the repository settings are not copied over, so the workflow in the forked repo is not secure. Whereas if the workflow file has explicit permissions, the settings are copied over.

So, setting Read repository contents permission in the repo setting is a good idea as future workflows in the repo will be secure-by-default. In addition, I would also recommend setting permissions explicitly in the workflow files.

[nodejs-github-bot]

Landed in d6988ec

[bnb]

In addition, the tool that does this transformation to add permissions key to the YAML workflow files is open source (https://github.com/step-security/secure-workflows).

This (from @varunsh-coder) is what I was talking about @Trott. My understanding is that he ran this tool on our actions and then PR'ed the output but the output itself is a GitHub feature, not a feature of the tool. Obviously already merged, but I just want to document that I'm +1 to using the GitHub feature and -1 to having Node.js start running this third-party tool ourselves as an action in the repo (outside is fine if people want to generate and PR, like @varunsh-coder did with this PR) when we inevitably need to do this work again in the future.