megalinter
megalinter copied to clipboard
Reduce generated permissions to `read-all` at top level for generated workflow file
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 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 :)
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 i fully agree with your clear explanations and will of course accept an incoming PR :)
@andrewvaughan i fully agree with your clear explanations and will of course accept an incoming PR :)
I got you fam.
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.