Fix: Resolve Checkov CKV2_GHA_1 error by setting root permissions for workflows (closes #3026)

[andrewvaughan]

Fixes #3026

Proposed Changes

1. Adds permissions: read-all to resolve Issue #3026

2. Resolves a logical error I found in some of the .mega-linter.yml files

Specifically, this:

VALIDATE_ALL_CODEBASE: >-
  ${{
    github.event_name == 'push' &&
    contains(fromJSON('["refs/heads/main", "refs/heads/master"]'), github.ref)
  }}

This is intended to match both refs/heads/main and refs/heads/master as a shorthand for an or-statement, but it will also match branches like refs/head/main-staging or refs/head/masterfoobar which isn't intended. I've reverted these to a proper, explicit or statement.

3. Improved documentation and resources within .mega-linter.yml files

I have improved commenting within the configuration files to provide new users better resources and information on what the files are for.

I've separated item 1 above from the rest into two commits, that way if the other changes are not of interest, we can just commit the permissions changes from issue #3026.

Readiness Checklist

Author/Contributor

• [x] Add entry to the CHANGELOG listing the change and linking to the corresponding issue (if appropriate)
• [x] If documentation is needed for this change, has that been included in this pull request

Reviewing Maintainer

• [ ] Label as breaking if this is a large fundamental change
• [ ] Label as either automation, bug, documentation, enhancement, infrastructure, or performance

[echoix]

Wow! I like that style! It's not rare to have questions from new users that would have needed these kind of hints, either on usage or configuration.

[rasa]

    contains(fromJSON('["refs/heads/main", "refs/heads/master"]'), github.ref)

... but it will also match branches like refs/head/main-staging or refs/head/masterfoobar which isn't intended.

With all due respect, not according to https://docs.github.com/en/actions/learn-github-actions/expressions#contains which says: Returns true if search contains item. If search is an array, this function returns true if the item is an element in the array. If search is a string, this function returns true if the item is a substring of search. This function is not case sensitive. Casts values to a string.

Edit: Though

(github.ref == "refs/heads/main" || github.ref == "refs/heads/master")

works just as well, and is perhaps more readable.

[andrewvaughan]

Wow! I like that style! It's not rare to have questions from new users that would have needed these kind of hints, either on usage or configuration.

Thanks! If you like my documentation design, you might like my https://github.com/andrewvaughan/template-core project. I've been revamping it the past week or so.

[andrewvaughan]

    contains(fromJSON('["refs/heads/main", "refs/heads/master"]'), github.ref)

... but it will also match branches like refs/head/main-staging or refs/head/masterfoobar which isn't intended.

With all due respect, not according to docs.github.com/en/actions/learn-github-actions/expressions#contains which says: Returns true if search contains item. If search is an array, this function returns true if the item is an element in the array. If search is a string, this function returns true if the item is a substring of search. This function is not case sensitive. Casts values to a string.

Edit: Though

(github.ref == "refs/heads/main" || github.ref == "refs/heads/master")

works just as well, and is perhaps more readable.

You're absolutely right and was a misunderstanding of how I understood contains( to work. I agree it's more readable and a better option then - but it'd be good for it to be consistent across the files (which it wasn't before). I will add a commit that updates it in a few.

[echoix]

I think there will be missing to sync up the other files and I'll take a new look

[andrewvaughan]

    contains(fromJSON('["refs/heads/main", "refs/heads/master"]'), github.ref)

... but it will also match branches like refs/head/main-staging or refs/head/masterfoobar which isn't intended.

With all due respect, not according to docs.github.com/en/actions/learn-github-actions/expressions#contains which says: Returns true if search contains item. If search is an array, this function returns true if the item is an element in the array. If search is a string, this function returns true if the item is a substring of search. This function is not case sensitive. Casts values to a string. Edit: Though

(github.ref == "refs/heads/main" || github.ref == "refs/heads/master")

works just as well, and is perhaps more readable.

You're absolutely right and was a misunderstanding of how I understood contains( to work. I agree it's more readable and a better option then - but it'd be good for it to be consistent across the files (which it wasn't before). I will add a commit that updates it in a few.

I've been thinking on this further...

I think keeping the more readable version:

(github.ref == "refs/heads/main" || github.ref == "refs/heads/master")

Has one major benefit - it's super easy to understand what's going on there. I do quite a bit with GitHub Actions programming and even I missed exactly what was going on with the contains directive. My vote would be to keep it with an explicit || writing so people can extend it easily without having to look into what fromJSON and contains do (or make the mis-assumption like I did that contains is a string-based function and not an array-based function).

Either way, it's your call, but if you could please make a proposed change (if any) for the files as to how you'd like it to look, I'll accept them. Thanks!

[andrewvaughan]

QQ for the maintainers - I haven't been super verbose with my commit message when accepting proposals. Do you want me to squash those commits when we've resolved all conversations, or is that something you'll manage during acceptance of the PR?

[echoix]

It'll be a squashed commit at the end... so it doesn't really matter. You always could pre-squash them if you'd like, you'd be one of the first to do it, but the committing style hasn't been held to a high standard here yet.

[echoix]

contains(fromJSON('["refs/heads/main", "refs/heads/master"]'), github.ref)

... but it will also match branches like refs/head/main-staging or refs/head/masterfoobar which isn't intended.

With all due respect, not according to docs.github.com/en/actions/learn-github-actions/expressions#contains which says: Returns true if search contains item. If search is an array, this function returns true if the item is an element in the array. If search is a string, this function returns true if the item is a substring of search. This function is not case sensitive. Casts values to a string.

Edit: Though

(github.ref == "refs/heads/main" || github.ref == "refs/heads/master")

works just as well, and is perhaps more readable.

You're absolutely right and was a misunderstanding of how I understood contains( to work. I agree it's more readable and a better option then - but it'd be good for it to be consistent across the files (which it wasn't before). I will add a commit that updates it in a few.

I've been thinking on this further...

I think keeping the more readable version:

(github.ref == "refs/heads/main" || github.ref == "refs/heads/master")

Has one major benefit - it's super easy to understand what's going on there. I do quite a bit with GitHub Actions programming and even I missed exactly what was going on with the contains directive. My vote would be to keep it with an explicit || writing so people can extend it easily without having to look into what fromJSON and contains do (or make the mis-assumption like I did that contains is a string-based function and not an array-based function).

Either way, it's your call, but if you could please make a proposed change (if any) for the files as to how you'd like it to look, I'll accept them. Thanks!

I agree with the second though, going with the explicit boolean expression that any programmer should be able to figure out without any docs.

[andrewvaughan]

It'll be a squashed commit at the end... so it doesn't really matter. You always could pre-squash them if you'd like, you'd be one of the first to do it, but the committing style hasn't been held to a high standard here yet.

I'll let y'all do it so you can determine what messages are helpful and what aren't. My SDLc actually asks people not to pre-squash for that reason.

[andrewvaughan]

contains(fromJSON('["refs/heads/main", "refs/heads/master"]'), github.ref)

... but it will also match branches like refs/head/main-staging or refs/head/masterfoobar which isn't intended.

With all due respect, not according to docs.github.com/en/actions/learn-github-actions/expressions#contains which says: Returns true if search contains item. If search is an array, this function returns true if the item is an element in the array. If search is a string, this function returns true if the item is a substring of search. This function is not case sensitive. Casts values to a string.

Edit: Though

(github.ref == "refs/heads/main" || github.ref == "refs/heads/master")

works just as well, and is perhaps more readable.

You're absolutely right and was a misunderstanding of how I understood contains( to work. I agree it's more readable and a better option then - but it'd be good for it to be consistent across the files (which it wasn't before). I will add a commit that updates it in a few.

I've been thinking on this further... I think keeping the more readable version:

(github.ref == "refs/heads/main" || github.ref == "refs/heads/master")

Has one major benefit - it's super easy to understand what's going on there. I do quite a bit with GitHub Actions programming and even I missed exactly what was going on with the contains directive. My vote would be to keep it with an explicit || writing so people can extend it easily without having to look into what fromJSON and contains do (or make the mis-assumption like I did that contains is a string-based function and not an array-based function). Either way, it's your call, but if you could please make a proposed change (if any) for the files as to how you'd like it to look, I'll accept them. Thanks!

I agree with the second though, going with the explicit boolean expression that any programmer should be able to figure out without any docs.

Cool, let's leave it as is, and that way they can add (or remove) any branches they need easily.

:Québec-Atlanta-High-Five:

[nvuillam]

@andrewvaughan you're just one cspell fix way from validation :)

In megalinter job artefacts, you can find an updated cspell.json that you can copy paste in the repo and check the diff to see if it's real typos or exceptions to add :)

[andrewvaughan]

@andrewvaughan you're just one cspell fix way from validation :)

In megalinter job artefacts, you can find an updated cspell.json that you can copy paste in the repo and check the diff to see if it's real typos or exceptions to add :)

BRB, going on vacation for 6 weeks.

Just playing, should have this wrapped up today for you!

[echoix]

If you finished applying the changes, I'd run the ./build.sh script, and afterwards again with ./build.sh --doc to make sure files aren't overwritten with the incorrect changes. Or you can try it yourself too

[echoix]

/build

Command run output Build command workflow started. Installing dependencies Running script ./build.sh Build command workflow completed without updating files.

[echoix]

/build

Command run output Build command workflow started. Installing dependencies Running script ./build.sh Build command workflow completed without updating files.

[andrewvaughan]

Hey all - sorry for dropping off for a few days, life actually did pop up. I'm working to resolve this today and tomorrow, though.

@echoix it wasn't entirely clear to me, do you still need me to run the build script to have this accepted? Thanks!

[echoix]

Nope, I tested it out and the changes won't be overwritten. I'd like to make a checkup that text applied in a suggestion was copied over to the other places

[andrewvaughan]

Just rebased the branch from main given the latest release.

[echoix]

contains(fromJSON('["refs/heads/main", "refs/heads/master"]'), github.ref)

... but it will also match branches like refs/head/main-staging or refs/head/masterfoobar which isn't intended.

With all due respect, not according to docs.github.com/en/actions/learn-github-actions/expressions#contains which says: Returns true if search contains item. If search is an array, this function returns true if the item is an element in the array. If search is a string, this function returns true if the item is a substring of search. This function is not case sensitive. Casts values to a string.

Edit: Though

(github.ref == "refs/heads/main" || github.ref == "refs/heads/master")

works just as well, and is perhaps more readable.

You're absolutely right and was a misunderstanding of how I understood contains( to work. I agree it's more readable and a better option then - but it'd be good for it to be consistent across the files (which it wasn't before). I will add a commit that updates it in a few.

I've been thinking on this further... I think keeping the more readable version:

(github.ref == "refs/heads/main" || github.ref == "refs/heads/master")

Has one major benefit - it's super easy to understand what's going on there. I do quite a bit with GitHub Actions programming and even I missed exactly what was going on with the contains directive. My vote would be to keep it with an explicit || writing so people can extend it easily without having to look into what fromJSON and contains do (or make the mis-assumption like I did that contains is a string-based function and not an array-based function). Either way, it's your call, but if you could please make a proposed change (if any) for the files as to how you'd like it to look, I'll accept them. Thanks!

I agree with the second though, going with the explicit boolean expression that any programmer should be able to figure out without any docs.

Cool, let's leave it as is, and that way they can add (or remove) any branches they need easily.

:Québec-Atlanta-High-Five:

I found out that we can use

github.ref == format('refs/heads/{0}', github.event.repository.default_branch)

instead of specifying what is the default branch in a or condition, What do you think?

[andrewvaughan]

contains(fromJSON('["refs/heads/main", "refs/heads/master"]'), github.ref)

... but it will also match branches like refs/head/main-staging or refs/head/masterfoobar which isn't intended.

With all due respect, not according to docs.github.com/en/actions/learn-github-actions/expressions#contains which says: Returns true if search contains item. If search is an array, this function returns true if the item is an element in the array. If search is a string, this function returns true if the item is a substring of search. This function is not case sensitive. Casts values to a string.

Edit: Though

(github.ref == "refs/heads/main" || github.ref == "refs/heads/master")

works just as well, and is perhaps more readable.

You're absolutely right and was a misunderstanding of how I understood contains( to work. I agree it's more readable and a better option then - but it'd be good for it to be consistent across the files (which it wasn't before). I will add a commit that updates it in a few.

I've been thinking on this further...

I think keeping the more readable version:

(github.ref == "refs/heads/main" || github.ref == "refs/heads/master")

Has one major benefit - it's super easy to understand what's going on there. I do quite a bit with GitHub Actions programming and even I missed exactly what was going on with the contains directive. My vote would be to keep it with an explicit || writing so people can extend it easily without having to look into what fromJSON and contains do (or make the mis-assumption like I did that contains is a string-based function and not an array-based function).

Either way, it's your call, but if you could please make a proposed change (if any) for the files as to how you'd like it to look, I'll accept them. Thanks!

I agree with the second though, going with the explicit boolean expression that any programmer should be able to figure out without any docs.

Cool, let's leave it as is, and that way they can add (or remove) any branches they need easily.

:Québec-Atlanta-High-Five:

I found out that we can use

github.ref == format('refs/heads/{0}', github.event.repository.default_branch)

instead of specifying what is the default branch in a or condition, What do you think?

I had something like this originally, but read that default branch is not available in the if conditional section.

[echoix]

I had something like this originally, but read that default branch is not available in the if conditional section.

Oh, let me continue testing it myself in my fork, since some older stackoverflow posts are outdated, since has made some updates over time, ie it wasn't available for scheduled events, but it is now.

[echoix]

/build

Command run output Build command workflow started. Installing dependencies Running script ./build.sh Build command workflow completed updating files.

[echoix]

After https://github.com/oxsecurity/megalinter/pull/3143, I think we're finally ready to tackle this, as the last release is now done, and the needed fixes to main (and the docs) will be finished.

[nvuillam]

@echoix you can merge this one once you are confident of its content :) (looks promising but you investigated it more than i did ^^ )

[echoix]

It will need a little manual work of fixing conflicts, and it seems a bad merge (or outdated starting point) reverted some changes of a previous PR.

As the potential breaking changes would be limited to new users generating a new workflow, I don't think it's really a problem, since there was nothing to break to begin with. If users would want to recreate their workflow by trying again and incorporating their changes back, it's ok, and that's how I expect a template generator to work.

[echoix]

And a title that better describes what that PR ended up being :)

[Jayllyz]

Any update on this fix ? Waiting this PR to be merged to fix this one : https://github.com/oxsecurity/megalinter/issues/3049