Add zizmor for CI (security) linting

[githubnemo]

To add a bit of a safety net to our CI jobs it might make sense to add a CI security linting tool such as zizmor.

The linting run should be green at the moment since I fixed all reported issues:

• setting persist-credentials: false in all checkout runs
• changing template substitutions to environment variable substitutions

I added an ignore rule for dangerous-triggers to ignore the upload_pr_to_documentation workflow as our actions are configured to only run such steps on approval which should already have seen at least maintainer eyes and the zizmor run.

[HuggingFaceDocBuilderDev]

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

[BenjaminBossan]

Thanks for improving the security of the PEFT workflows.

changing template substitutions to environment variable substitutions

Could you explain why this is more secure?

persist-credentials: false

Just to clarify, this is not a security issue on the repo as we're currently not using ssh, this is just a precaution, as that right?

Edit: For context, we wanted to harden the repo after the ultralytics hack.

[BenjaminBossan]

How about only executing this workflow if anything was changed inside of .github using the path argument? Or could we be missing something that way?

[githubnemo]

Good idea, added accordingly.

[BenjaminBossan]

It's not beautiful but I don't know if there is anything else that we can do here.

[githubnemo]

The zizmor documentatin recommends switching to the workflow_call trigger which has the security benefit of not passing environment variables from one workflow to the other - they're more isolated.

FWIW it seems to be the better scheme for this workflow anyway but I'm not sure if there are unforeseen consequences.

[githubnemo]

changing template substitutions to environment variable substitutions

Could you explain why this is more secure?

Sure. With env: FOO=${{ foo }} we're separating code and data. Environment variables are just a place to store strings and no interpretation of the contents is happening. You'd have to force an interpretation using eval $FOO or something similar.

Contrast this to the behavior before: run: echo "something ${{ foo }}" - here we're modifying the code directly with whatever the content of ${{ foo }} is - there is no conversion to a shell string, there is no separation of code and data and therefore there is no way for the shell to know what is right or wrong.

This is, of course, assuming that github action is not implementing the env statement as

export FOO=${{ foo }}

but rather as something like environ['FOO'] = str(vars('foo')) :)

persist-credentials: false

Just to clarify, this is not a security issue on the repo as we're currently not using ssh, this is just a precaution, as that right?

Yes, this is just to make the linter happy and avoid accidents should we ever (for whatever reason) use manual triggers using SSH.

[githubnemo]

@McPatate might be interested in this too, perhaps?

[BenjaminBossan]

LGTM, thanks.