Add zizmor pre-commit and fix reported issues

[BarbUk]

• Add zizmor precommit
• Fix ci workflow

Description

https://github.com/zizmorcore/zizmor is a static analysis tool for github action.

This PR add a precommit hook to check the CI workflow if modified.

Motivation and Context

Github workflow can be a security issue when not configured properly.
Zizmor will check for issues and report recommantion on how to fix those.

How Has This Been Tested?

❯ zizmor .github/
🌈 zizmor v1.23.1
 INFO audit: zizmor: 🌈 completed .github/workflows/ci.yml
No findings to report. Good job! (9 suppressed)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• If my change requires a change to the documentation, I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• If I have added a new file, I also added it to clean_files.txt and formatted it using lint_clean_files.sh.
• I have added tests to cover my changes, and all the new and existing tests pass.

[seefood]

I played with the idea, but I think it's an overkill to add this as a pre-commit dependency... this is maybe more of a post-commit action in the CI? what do you think?

[BarbUk]

but I think it's an overkill to add this as a pre-commit dependency

It will run only if the github workflow are modified in the commit AND if the user has installed the precommit hook.

Running lint_clean_files.sh will never check the github workflow, as it works only on files / dir defined in clean_files.txt.

this is maybe more of a post-commit action in the CI? what do you think?

At the moment it's neither.

It will only run if we install the pre-commit hook with prek install or pre-commit install as defined in https://bash-it.readthedocs.io/en/latest/development/#using-the-pre-commit-hook.

I can modify the CI to run it if the workflow are modified in the PR.

My goal is to enforce a minimum of security with github action on this project.
Recent article from astral show some of the good practice available: https://astral.sh/blog/open-source-security-at-astral

[seefood]

maybe we should use zizmorcore/zizmor-action@71321a2 instead in the workflow? I don't want to force the users to instal zizmor just for us. alternatively: quietly skip the prek test if zizmor is not installed?

[BarbUk]

maybe we should use zizmorcore/zizmor-action@71321a2 instead in the workflow?

Yes, I can work on that.

I don't want to force the users to instal zizmor just for us.

This is not the goal.

The pre-commit hooks are only installed manually if someone want to test bash-it automatically when doing a new commit on the repo, as outlined in https://bash-it.readthedocs.io/en/latest/development/#using-the-pre-commit-hook.

The zizmor hook does't need zizmor installed, pre-commit will will download it in a specific venv to test the commit.

[github-advanced-security]

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

• The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
• Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
• You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

[seefood]

Ideally I want people to install prek (or pre-commit) if they want to submit a PR. we already assume they install shfmt and shellcheck, but I don't want them to have to install zizmor. the action is good enough, as the workflows are part of the CI, not really part of the bash-it project.

[BarbUk]

Note that #2383 should be merge before for the CI to pass.