megalinter icon indicating copy to clipboard operation
megalinter copied to clipboard

Reduce generated permissions to `read-all` at top level for generated workflow file

Open andrewvaughan opened this issue 2 years ago • 5 comments

Describe the bug Checkov (rightfully) complains about CKV2_GHA_1 that the workflow file leaves permissions to the default write-all in .mega-linter.yml file. This line should be added to the top level:

permissions: read-all

The one build step permissions shouldn't have to change. It's a slightly pedantic issue, but it does occur on the default setup. It will also help prevent any mistakes if users add a second step beyond build.

andrewvaughan avatar Oct 20 '23 00:10 andrewvaughan

@andrewvaughan to post comments with results or push new commits, read-all won't be enough :)

But MegaLinter users are free to not use these features and set manually permissions to read-all :)

nvuillam avatar Oct 20 '23 16:10 nvuillam

Correct! But you should be able to put these permissions per-job. The github-actions linter is complaining because permissions: read-only is not set at the root. This requires each job to be explicit in setting its permissions.

So to be clear, this isn't replacing the existing permissions block, this is adding an additional permissions: read-only in the same level as the on clause to set read-only as a build default. The existing permission block will overwrite this.

As mentioned, it's pedantic, but it does enforce any extra build steps to require explicit permission building per-job, which is best practice. That is what Checkov errors about in the existing workflow.

Edit:

So it would look something like this:

name: MegaLinter

on:
  push:
    branches:
      - main
      - production
      - staging

  pull_request:

##
# Set any build permissions to `read-all` unless explicitly overridden. Important, as
# GitHub defaults to `write-all` which is a security risk.
#
permissions: read-all     # <---- Line to add, prevents `CKV2_GHA_1` linting error

## ... env, concurrency, etc ...


jobs:
  build:
    name: MegaLinter
    runs-on: ubuntu-latest

    ##
    # Override the permissions from the default `read-only` set above to what this job needs
    # to function. Each job should set their own permissions, accordingly.
    #
    permissions:
      contents: write
      issues: write
      pull-requests: write

      // ...

A way to look at this is "hey GitHub, by default, only give any jobs read-all permissions (instead of the default write-all) unless that job explicitly overrides the read-all setting.

You could imagine a developer adding a job parallel to build in the script, not realizing that they unexpectedly are running that new job with write-all capabilities. By setting a read-all default, it ensures that permissions are white-listed, by default, for each job.

Finally, adding this line adds zero change to the existing functionality for MegaLinter, which is nice. It's simply an additional layer of security. I've formatted my generated file a little more verbosely with explanatory comments and fully complying with default MegaLinter configurations, if the project wishes to steal any parts of it - feel free:

https://github.com/andrewvaughan/template-core/blob/main/.github/workflows/mega-linter.yml

andrewvaughan avatar Oct 21 '23 00:10 andrewvaughan

@andrewvaughan i fully agree with your clear explanations and will of course accept an incoming PR :)

nvuillam avatar Oct 21 '23 06:10 nvuillam

@andrewvaughan i fully agree with your clear explanations and will of course accept an incoming PR :)

I got you fam.

andrewvaughan avatar Oct 21 '23 14:10 andrewvaughan

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

github-actions[bot] avatar Nov 21 '23 00:11 github-actions[bot]