Skip to contentSkip to conversation

ci: restrict workflow's github token permissions

PATCH In review Alex via GitGitGadget 2 participants 2 messages Sep 22, 2022

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ci: restrict workflow's github token permissions

From: sashashura <[email protected]>

Currently the workflow runs with the following permissions:
GITHUB_TOKEN Permissions
Actions: write
Checks: write
Contents: write
Deployments: write
Discussions: write
Issues: write
Metadata: read
Packages: write
Pages: write
PullRequests: write
RepositoryProjects: write
SecurityEvents: write
Statuses: write

---
ci: restrict workflow's github token permissions

This PR adds explicit permissions section
[https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#permissions]
to workflows. This is a security best practice because by default workflows run with extended set of permissions
[https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token]
(except from on: pull_request from external forks
[https://securitylab.github.com/research/github-actions-preventing-pwn-requests/]).
By specifying any permission explicitly all others are set to none. By using the principle of least privilege the damage a compromised workflow can do (because of an injection
[https://securitylab.github.com/research/github-actions-untrusted-input/]
or compromised third party tool or action) is restricted. It is recommended to have most strict permissions on the top level
[https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions]
and grant write permissions on job level
[https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs]
case by case.

check-whitespace.yml is triggered by pull_request only and receives restricted token already. l10n.yml has permissions on job level already. So I didn't make any changes to them. In both cases it is possible to add explicit global lever permissions just for consistency if you prefer. Let me know.

Currently
[https://github.com/git/git/actions/runs/3100948073/jobs/5021781329] the workflow runs with the following permissions: GITHUB_TOKEN Permissions
Actions: write Checks: write Contents: write Deployments: write
Discussions: write Issues: write Metadata: read Packages: write Pages:
write PullRequests: write RepositoryProjects: write SecurityEvents:
write Statuses: write

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1337%2Fsashashura%2Fpatch-2-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1337/sashashura/patch-2-v1
Pull-Request: https://github.com/git/git/pull/1337

 .github/workflows/main.yml | 6 ++++++
 1 file changed, 6 insertions(+)
.github/workflows/main.yml +6 -0
index 831f4df56c5..3ce9302c589 100644
@@ -5,8 +5,14 @@ on: [push, pull_request]
 env:
   DEVELOPER: 1
 
+permissions:
+  contents: read
+
 jobs:
   ci-config:
+    permissions:
+      contents: read
+      actions: read # for github.actions.getWorkflowRun
     name: config
     runs-on: ubuntu-latest
     outputs:

base-commit: dda7228a83e2e9ff584bf6adbf55910565b41e14

Signature · 1 line
gitgitgadget
#1 lore raw

Hi Alex

Quoted 17 lines · Alex via GitGitGadget

From: sashashura <[email protected]>

Currently the workflow runs with the following permissions:
GITHUB_TOKEN Permissions
Actions: write
Checks: write
Contents: write
Deployments: write
Discussions: write
Issues: write
Metadata: read
Packages: write
Pages: write
PullRequests: write
RepositoryProjects: write
SecurityEvents: write
Statuses: write

Thanks for working on this. On the face of it restricting the permissions sounds like a good idea but unfortunately the commit message does not explain the reasoning for the change being made or the implications of the change. Some of the notes below the '---' line should be rewritten into the commit message to explain the change. I would also be helpful to briefly explain why we don't need any of these permissions. I'm not familiar with github's permissions model so it's hard to judge if this is a sensible change. It is not clear to me what all the permissions mean - does write permission control writing to my fork when I run a ci job or something else? Our ci scripts do cache some state between jobs so we can skip running the tests if the tree is unchanged, it's not clear if that is affected by this change.

Best Wishes

Phillip

Quoted 64 lines

---
ci: restrict workflow's github token permissions

This PR adds explicit permissions section
[https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#permissions]
to workflows. This is a security best practice because by default workflows run with extended set of permissions
[https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token]
(except from on: pull_request from external forks
[https://securitylab.github.com/research/github-actions-preventing-pwn-requests/]).
By specifying any permission explicitly all others are set to none. By using the principle of least privilege the damage a compromised workflow can do (because of an injection
[https://securitylab.github.com/research/github-actions-untrusted-input/]
or compromised third party tool or action) is restricted. It is recommended to have most strict permissions on the top level
[https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions]
and grant write permissions on job level
[https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs]
case by case.

check-whitespace.yml is triggered by pull_request only and receives restricted token already. l10n.yml has permissions on job level already. So I didn't make any changes to them. In both cases it is possible to add explicit global lever permissions just for consistency if you prefer. Let me know.

Currently
[https://github.com/git/git/actions/runs/3100948073/jobs/5021781329] the workflow runs with the following permissions: GITHUB_TOKEN Permissions
Actions: write Checks: write Contents: write Deployments: write
Discussions: write Issues: write Metadata: read Packages: write Pages:
write PullRequests: write RepositoryProjects: write SecurityEvents:
write Statuses: write

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1337%2Fsashashura%2Fpatch-2-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1337/sashashura/patch-2-v1
Pull-Request: https://github.com/git/git/pull/1337

  .github/workflows/main.yml | 6 ++++++
  1 file changed, 6 insertions(+)
.github/workflows/main.yml +6 -0
index 831f4df56c5..3ce9302c589 100644
@@ -5,8 +5,14 @@ on: [push, pull_request]
  env:
    DEVELOPER: 1
 
+permissions:
+  contents: read
+
  jobs:
    ci-config:
+    permissions:
+      contents: read
+      actions: read # for github.actions.getWorkflowRun
      name: config
      runs-on: ubuntu-latest
      outputs:

base-commit: dda7228a83e2e9ff584bf6adbf55910565b41e14