git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH] ci: restrict workflow's github token permissions
@ 2022-09-22  7:27 Alex via GitGitGadget
  2022-09-22 15:29 ` Phillip Wood
  0 siblings, 1 reply; 2+ messages in thread
From: Alex via GitGitGadget @ 2022-09-22  7:27 UTC (permalink / raw)
  To: git; +Cc: Alex, sashashura

From: sashashura <aleksandrosansan@gmail.com>

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

Signed-off-by: sashashura <aleksandrosansan@gmail.com>
---
    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(+)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 831f4df56c5..3ce9302c589 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -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
-- 
@@ -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
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] ci: restrict workflow's github token permissions
  2022-09-22  7:27 [PATCH] ci: restrict workflow's github token permissions Alex via GitGitGadget
@ 2022-09-22 15:29 ` Phillip Wood
  0 siblings, 0 replies; 2+ messages in thread
From: Phillip Wood @ 2022-09-22 15:29 UTC (permalink / raw)
  To: Alex via GitGitGadget, git; +Cc: Alex

Hi Alex

On 22/09/2022 08:27, Alex via GitGitGadget wrote:
> From: sashashura <aleksandrosansan@gmail.com>
> 
> 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

> Signed-off-by: sashashura <aleksandrosansan@gmail.com>
> ---
>      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(+)
> 
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index 831f4df56c5..3ce9302c589 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-09-22 15:31 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-22  7:27 [PATCH] ci: restrict workflow's github token permissions Alex via GitGitGadget
2022-09-22 15:29 ` Phillip Wood

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).