{"thread":{"id":"58488","subject":"[PATCH] ci: restrict workflow's github token permissions","startedAt":"2022-09-22T07:27:38Z","lastAt":"2022-09-22T15:30:05Z","messageCount":2,"participants":["Alex via GitGitGadget","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"463428","messageId":"pull.1337.git.git.1663831649705.gitgitgadget@gmail.com","threadId":"58488","inReplyTo":null,"subject":"[PATCH] ci: restrict workflow's github token permissions","fromName":"Alex via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-09-22T07:27:29Z","receivedAt":"2022-09-22T07:27:38Z","isPatch":true,"sender":{"key":"ajb44.geo@yahoo.com","avatar":null},"body":"From: sashashura <aleksandrosansan@gmail.com>\n\nCurrently the workflow runs with the following permissions:\nGITHUB_TOKEN Permissions\n  Actions: write\n  Checks: write\n  Contents: write\n  Deployments: write\n  Discussions: write\n  Issues: write\n  Metadata: read\n  Packages: write\n  Pages: write\n  PullRequests: write\n  RepositoryProjects: write\n  SecurityEvents: write\n  Statuses: write\n\nSigned-off-by: sashashura <aleksandrosansan@gmail.com>\n---\n    ci: restrict workflow's github token permissions\n    \n    This PR adds explicit permissions section\n    [https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#permissions]\n    to workflows. This is a security best practice because by default\n    workflows run with extended set of permissions\n    [https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token]\n    (except from on: pull_request from external forks\n    [https://securitylab.github.com/research/github-actions-preventing-pwn-requests/]).\n    By specifying any permission explicitly all others are set to none. By\n    using the principle of least privilege the damage a compromised workflow\n    can do (because of an injection\n    [https://securitylab.github.com/research/github-actions-untrusted-input/]\n    or compromised third party tool or action) is restricted. It is\n    recommended to have most strict permissions on the top level\n    [https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions]\n    and grant write permissions on job level\n    [https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs]\n    case by case.\n    \n    check-whitespace.yml is triggered by pull_request only and receives\n    restricted token already. l10n.yml has permissions on job level already.\n    So I didn't make any changes to them. In both cases it is possible to\n    add explicit global lever permissions just for consistency if you\n    prefer. Let me know.\n    \n    Currently\n    [https://github.com/git/git/actions/runs/3100948073/jobs/5021781329] the\n    workflow runs with the following permissions: GITHUB_TOKEN Permissions\n    Actions: write Checks: write Contents: write Deployments: write\n    Discussions: write Issues: write Metadata: read Packages: write Pages:\n    write PullRequests: write RepositoryProjects: write SecurityEvents:\n    write Statuses: write\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1337%2Fsashashura%2Fpatch-2-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1337/sashashura/patch-2-v1\nPull-Request: https://github.com/git/git/pull/1337\n\n .github/workflows/main.yml | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 831f4df56c5..3ce9302c589 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -5,8 +5,14 @@ on: [push, pull_request]\n env:\n   DEVELOPER: 1\n \n+permissions:\n+  contents: read\n+\n jobs:\n   ci-config:\n+    permissions:\n+      contents: read\n+      actions: read # for github.actions.getWorkflowRun\n     name: config\n     runs-on: ubuntu-latest\n     outputs:\n\nbase-commit: dda7228a83e2e9ff584bf6adbf55910565b41e14\n-- \ngitgitgadget\n"},{"id":"463450","messageId":"ed506937-44fc-125f-7baa-a187b12414c9@dunelm.org.uk","threadId":"58488","inReplyTo":"pull.1337.git.git.1663831649705.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci: restrict workflow's github token permissions","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-09-22T15:29:38Z","receivedAt":"2022-09-22T15:30:05Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Alex\n\nOn 22/09/2022 08:27, Alex via GitGitGadget wrote:\n> From: sashashura <aleksandrosansan@gmail.com>\n> \n> Currently the workflow runs with the following permissions:\n> GITHUB_TOKEN Permissions\n>    Actions: write\n>    Checks: write\n>    Contents: write\n>    Deployments: write\n>    Discussions: write\n>    Issues: write\n>    Metadata: read\n>    Packages: write\n>    Pages: write\n>    PullRequests: write\n>    RepositoryProjects: write\n>    SecurityEvents: write\n>    Statuses: write\n\nThanks for working on this. On the face of it restricting the \npermissions sounds like a good idea but unfortunately the commit message \ndoes not explain the reasoning for the change being made or the \nimplications of the change. Some of the notes below the '---' line \nshould be rewritten into the commit message to explain the change. I \nwould also be helpful to briefly explain why we don't need any of these \npermissions. I'm not familiar with github's permissions model so it's \nhard to judge if this is a sensible change. It is not clear to me what \nall the permissions mean - does write permission control writing to my \nfork when I run a ci job or something else? Our ci scripts do cache some \nstate between jobs so we can skip running the tests if the tree is \nunchanged, it's not clear if that is affected by this change.\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: sashashura <aleksandrosansan@gmail.com>\n> ---\n>      ci: restrict workflow's github token permissions\n>      \n>      This PR adds explicit permissions section\n>      [https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#permissions]\n>      to workflows. This is a security best practice because by default\n>      workflows run with extended set of permissions\n>      [https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token]\n>      (except from on: pull_request from external forks\n>      [https://securitylab.github.com/research/github-actions-preventing-pwn-requests/]).\n>      By specifying any permission explicitly all others are set to none. By\n>      using the principle of least privilege the damage a compromised workflow\n>      can do (because of an injection\n>      [https://securitylab.github.com/research/github-actions-untrusted-input/]\n>      or compromised third party tool or action) is restricted. It is\n>      recommended to have most strict permissions on the top level\n>      [https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions]\n>      and grant write permissions on job level\n>      [https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs]\n>      case by case.\n>      \n>      check-whitespace.yml is triggered by pull_request only and receives\n>      restricted token already. l10n.yml has permissions on job level already.\n>      So I didn't make any changes to them. In both cases it is possible to\n>      add explicit global lever permissions just for consistency if you\n>      prefer. Let me know.\n>      \n>      Currently\n>      [https://github.com/git/git/actions/runs/3100948073/jobs/5021781329] the\n>      workflow runs with the following permissions: GITHUB_TOKEN Permissions\n>      Actions: write Checks: write Contents: write Deployments: write\n>      Discussions: write Issues: write Metadata: read Packages: write Pages:\n>      write PullRequests: write RepositoryProjects: write SecurityEvents:\n>      write Statuses: write\n> \n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1337%2Fsashashura%2Fpatch-2-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1337/sashashura/patch-2-v1\n> Pull-Request: https://github.com/git/git/pull/1337\n> \n>   .github/workflows/main.yml | 6 ++++++\n>   1 file changed, 6 insertions(+)\n> \n> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> index 831f4df56c5..3ce9302c589 100644\n> --- a/.github/workflows/main.yml\n> +++ b/.github/workflows/main.yml\n> @@ -5,8 +5,14 @@ on: [push, pull_request]\n>   env:\n>     DEVELOPER: 1\n>   \n> +permissions:\n> +  contents: read\n> +\n>   jobs:\n>     ci-config:\n> +    permissions:\n> +      contents: read\n> +      actions: read # for github.actions.getWorkflowRun\n>       name: config\n>       runs-on: ubuntu-latest\n>       outputs:\n> \n> base-commit: dda7228a83e2e9ff584bf6adbf55910565b41e14\n"}]}