Error when using workflow: ”The nested job 'genmatrix' is requesting 'pull-requests: read'“

martinheise commented 2 months ago

Module version(s) affected

^1.9

Description

I have used the workflow as described with default setup pointing to tag “v1”, both in a new repository and in an older one (both containing Silverstripe modules):

For both I get an error message now when running the CI workflow related to permission (details see below).

As the older repository used to work this way without code changes in the meantime, I tried several configurations using different tags – it seems the issue appears with branch 1.9, while 1.8. still runs fine.

How to reproduce

Setup a basic CI workflow as described on https://github.com/silverstripe/gha-ci/ in some repository:

name: CI

on:
  push:
  pull_request:
  workflow_dispatch:

jobs:
  ci:
    name: CI
    uses: silverstripe/gha-ci/.github/workflows/ci.yml@v1

and trigger the CI action manually or by pushing a branch.

The action aborts with error message:

The workflow is not valid. .github/workflows/ci.yml (Line: 9, Col: 3): Error calling workflow 'silverstripe/gha-ci/.github/workflows/ci.yml@v1'. The nested job 'genmatrix' is requesting 'pull-requests: read', but is only allowed 'pull-requests: none'. .github/workflows/ci.yml (Line: 9, Col: 3): Error calling workflow 'silverstripe/gha-ci/.github/workflows/ci.yml@v1'. The nested job 'patchrelease' is requesting 'contents: write', but is only allowed 'contents: read'.

Changing the referenced tag/branch for the included job to e.g.:

silverstripe/gha-ci/.github/workflows/ci.yml@1.8

Using branch 1.8 here still works and the job runs successfully, new branches 1.9, 1.10, 1.11, 1.12 fail with the same message.

Possible Solution

I’m not sure if this about wrong permission inside gha-ci module, or if some different setup in the repository using it is required because of some change – in the latter case just the documentation would need enhancement.

Additional Context

Thanks for checking!

Validations

[X] Check that there isn't already an issue that reports the same bug
[X] Double check that your reproduction steps work in a fresh installation of silverstripe/installer (with any code examples you've provided)

Notes

See https://github.com/silverstripe/gha-ci/pull/138#issuecomment-2254720726 - we're opting to split the patch tagging as its own workflow, so we can have contents:read instead of contents:write in the ci.yml workflow.

PRs

After merging, reassign to Guy so they can do these steps

[x] Set default branch for gha-tag-release to 2
[x] Manually tag 2.0.0 of gha-tag-release
[x] Manually tag v2 of gha-tag-release
[x] Run module standardiser against the next-patch branch

Next set of PRs

[!IMPORTANT] DO NOT merge these until the PRs above are merged, and the steps above are complete.

After this set of PRs is merged, reassign to Guy to:

[x] Tag new minor releases for the various affected gha repos
[x] Redeploy elvis so it picks up on the changes
[x] Archive gha-gauge-release

GuySartorelli commented 2 months ago

Looks like the relatively recent security hardening has resulted in the default GitHub token that your repository uses for GitHub Actions not having enough permission to run the workflow. I don't think this was an intended side effect.

You'll need to make the following changes for the CI to run:

 name: CI

 on:
   push:
   pull_request:
   workflow_dispatch:
+
+ permissions: {}

 jobs:
   ci:
     name: CI
     uses: silverstripe/gha-ci/.github/workflows/ci.yml@v1
+    permissions:
+      pull-requests: read
+      contents: write

The contents: write is a bit unfortunate - that is required by silverstripe/gha-tag-release which will be skipped in your repos, so that permission won't ever actually be used. Nevertheless GitHub requires that it's provided.

To ensure you protect yourself from malicious actors, I recommend you set the "Fork pull request workflows from outside collaborators" setting in https://github.com/<org>/<repo>/settings/actions to one of

Require approval for first-time contributors
Require approval for all outside collaborators

I'll work on getting the README here updated, but for now hopefully that gives you a way forward.

martinheise commented 2 months ago

Thanks, @GuySartorelli, these changes work fine to fix the issue.

GuySartorelli commented 2 months ago

Sweet. FYI we're working on an improvement that will allow you to just have contents: read instead of giving it write permissions.

emteknetnz commented 2 months ago

^ I think you won't even need to specify contents: read after the improvements? The default github token in repos provides read level access.

[x] Read repository contents and packages permissions
Workflows have read permissions in the repository for the contents and packages scopes only.

emteknetnz commented 2 months ago

@GuySartorelli A bunch of the unmerged PRs created by module standardiser are targetting the next-minor, they should be targeting the next-patch branch? The top load of PRs that were merged targetted next-patch

GuySartorelli commented 2 months ago

@emteknetnz oh! Thank you for noticing that, yeah they should all be for next-patch. I'll sort that out.

emteknetnz commented 2 months ago

@GuySartorelli noticed your new PR commit messages have:

Co-authored-by: DDEV User <nobody@example.com>

Those shouldn't be there

GuySartorelli commented 2 months ago

It's a comment in a commit message, it really affects nothing. But I'll remove it to avoid ping pong. Edit: I guess it would have technically resulted in tagging patches that didn't need tagging, for what that's worth.

GuySartorelli commented 1 month ago

@martinheise You should be able to demote contents:write to contents:read now.

GuySartorelli commented 1 month ago

All PRs merged and follow-up actions performed.

silverstripe / gha-ci