Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: allow filename wildcard in config file #435

Open
2 tasks done
Chocobo1 opened this issue Jan 13, 2025 · 7 comments
Open
2 tasks done

Feature: allow filename wildcard in config file #435

Chocobo1 opened this issue Jan 13, 2025 · 7 comments
Labels
enhancement New feature or request

Comments

@Chocobo1
Copy link

Pre-submission checks

  • I am not reporting a bug (crash, false positive/negative, etc). These must be filed via the bug report template.
  • I have looked through the open issues for a duplicate request.

What's the problem this feature will solve?

I would like to disable/ignore one or more rules completely but the closest mechanism seems to be config files.

For example I have this zizmor.yml:

rules:
  unpinned-uses:
    ignore:
      - "*"

This is invalid config file at the time of writing. And this is the associated error: rules.unpinned-uses.ignore: invalid workflow filename: * at line 4 column 7.

I would like to suppress the rule unpinned-uses completely, i.e. I wish the * could be allowed and it would match all files. I'm aware that I could manually list all filenames here but it is cumbersome and it would break easily in the long run.

Describe the solution you'd like

Allow wildcard (or regex) matching in every rules.<id>.ignore element. For example the following would be allowed: *.yaml, *.

Additional context

I'm using zizmor as an auxiliary/external checker. I would prefer it not get into the workflow files itself, that means suppressing rules via inline comment is not acceptable.

@Chocobo1 Chocobo1 added the enhancement New feature or request label Jan 13, 2025
@woodruffw
Copy link
Owner

Hi @Chocobo1, thanks for filing a feature request.

I'm currently hesitant to support blanket ignores of a rule for all files: I intentionally left that out of the original configuration feature set, because I think it makes it too easy to suppress future findings unintentionally.

There are two alternatives:

  1. As you're already noted, you can list your workflows manually. I understand this is cumbersome, but it's intended to be: listing each is intended to be a behavior shaping constraint that forces people to review new changes.
  2. If (1) is too cumbersome for your use case, you can use zizmor's JSON or SARIF output and manually filter the results by their audit ID. This could easily be done as an intermediate step between a zizmor run and the JSON/SARIF's use elsewhere. The docs have some starting tips for writing a jq filter here: https://woodruffw.github.io/zizmor/usage/#filtering-results.

Assuming (1) is still unacceptable, could you give (2) a try and see if it works for your case?

@Chocobo1
Copy link
Author

Assuming (1) is still unacceptable, could you give (2) a try and see if it works for your case?

FYI, we intend to use zizmor as a step in a CI workflow and the output (check results) is intended to be read a human directly. JSON or SARIF isn't good for this purpose.

I intentionally left that out of the original configuration feature set, because I think it makes it too easy to suppress future findings unintentionally.

Just to be clear, this feature request wouldn't suppress future findings, it only disable specified rules.

One might thought that all rules are there for the user's safety. While the intention is good, IMO it risk the program becoming a police which unnecessary infulence things.
Take unpinned-uses as an example, it currently says: uses: actions/checkout@v4 help: action is not pinned to a hash ref, but our project is fine with using a tag instead of a hash. Even if I could manually write out all workflow filenames now, it still require unnecessary maintenance later when we introduced new workflow files. I mean, what if there are a few rules we want to ignore? Having a long list of filenames in config could look silly and unreadable. And in the long run, (at least myself) will consider zizmor has high maintenance cost and will severely affect its adoption/deployment rate. (i.e. only serious big projects would be considered getting the deployment)

@woodruffw
Copy link
Owner

woodruffw commented Jan 14, 2025

FYI, we intend to use zizmor as a step in a CI workflow and the output (check results) is intended to be read a human directly. JSON or SARIF isn't good for this purpose.

I'm not sure I follow: if you use SARIF, you can then upload the filtered SARIF to GitHub for human presentation -- there's an example of that in the docs: https://woodruffw.github.io/zizmor/usage/#use-in-github-actions

If you use JSON, you can always adapt the filtered JSON to a human format like Markdown (e.g. in a step summary via GITHUB_STEP_SUMMARY). I believe that's what a handful of users are doing.

Just to be clear, this feature request wouldn't suppress future findings, it only disable specified rules.

Right, but for all files, including new ones. That's the concerning part: if a user has foo.yml and bar.yml that they've reviewed for safety, they might forget to review baz.yml because it's been implicitly ignored already.

While the intention is good, IMO it risk the program becoming a police which unnecessary infulence things.

This is a tool I develop in my free time for people to use if they want to. I don't like being compared to a police state, especially since you don't have to use my tool!

With that being said, you shouldn't be seeing that finding at all unless you're running with --pedantic or --persona=pedantic. Are you using one of those options? Those are generally too sensitive to use in CI/CD, which is why they aren't the default.

@Chocobo1
Copy link
Author

I'm not sure I follow: if you use SARIF, you can then upload the filtered SARIF to GitHub for human presentation -- there's an example of that in the docs: https://woodruffw.github.io/zizmor/usage/#use-in-github-actions

Thank you! I wasn't aware of this integration path. I suppose this is a viable path for me.

This is a tool I develop in my free time for people to use if they want to. I don't like being compared to a police state, especially since you don't have to use my tool!

Apologies, didn't mean to offend anyone. Also, I appreciate this project. Github has blogged good security practices but there were no tool verify it until now.

That's the concerning part: if a user has foo.yml and bar.yml that they've reviewed for safety, they might forget to review baz.yml because it's been implicitly ignored already.

This is the part I find overprotective for the user. Which is why the police state concept pop into my head. You don't seem to trust the user enough and expect them to be careless. For the user perspective, it could be insulting.
I believe one does not easily use wildcard for ignoring, it is always a conscious decision. And even if they misuse wildcard, what could you do? Put in more barricades to stop them? They always find a way to overcome it if they are determined (like filtering with jq as you mentioned). And what is the result? The config becomes scattered and fragile, now the user have to maintain ignore list for jq and zizmor config file. It is not a good outcome.

Or maybe you could consider adding a command line switch to allow wildcards? Just throwing out random ideas...

With that being said, you shouldn't be seeing that finding at all unless you're running with --pedantic or --persona=pedantic. Are you using one of those options? Those are generally too sensitive to use in CI/CD, which is why they aren't the default.

Thanks for the concern. I'm not currently but I intend to. I would like to see all warnings and learn the implications. After that I can decide whether to suppress the rule or not. (the suppression would be global, not just individual workflow files)

@woodruffw
Copy link
Owner

Thank you! I wasn't aware of this integration path. I suppose this is a viable path for me.

Yes, this is what I'd recommend -- a significant number of users are already integrating via SARIF, and the format is stable/standardized so you should be able to write a filter that won't need to be changed over time.

They always find a way to overcome it if they are determined (like filtering with jq as you mentioned). And what is the result? The config becomes scattered and fragile, now the user have to maintain ignore list for jq and zizmor config file. It is not a good outcome.

Right, the goal is not to prevent someone who's determined to pave their own path. It's to avoid exposing footguns. There's no perfect way to do this, but the theory is that the more manual step of having to write a jq filter should act as a minor speedbump that makes people revisit their assumptions.

I think this is especially important now that a lot of people are using generative AI to mash out massive amounts of unreviewed code.

(I apologize if you find this insulting -- it's not meant to demean users, but to establish a boundary between "zizmor will let you do something by default" and "zizmor won't stop you, but you're on your own if you want to do this thing.")

Or maybe you could consider adding a command line switch to allow wildcards? Just throwing out random ideas...

This is a possibility -- I've been thinking about adding an --expert-mode/ZIZMOR_EXPERT_MODE=1 option toggle that would enable more things, but I don't have a good sense of which things should be under that (and I'm wary of the maintenance burden of a new user path).

This is worth discussing in a follow up Discussion thread, however, if this is something you're more broadly interested in 🙂

Thanks for the concern. I'm not currently but I intend to. I would like to see all warnings and learn the implications. After that I can decide whether to suppress the rule or not. (the suppression would be global, not just individual workflow files)

That's understandable, but FWIW I'd recommend reading the docs on the different personas if you haven't already: https://woodruffw.github.io/zizmor/usage/#using-personas

In particular, the pedantic and auditor personas are much more sensitive that the default one is, which means that you'll get a lot more false positives that need human review. This is an intentional design decision, but it also makes those personas (typically) unsuitable for use in CI/CD. That being said, filtering is of course still an option.

@userdocs
Copy link
Contributor

There no real need to jq > into markdown when $GITHUB_SUMMARY can take the standard output. You can just do zizmor . >> $GITHUB_STEP_SUMMARY but you won't see that output in the workflow.

This was the pr I created that's related to this thread. So this workflow outputs to the standout output, the github summary in a markdown codebox as well as capturing the exit code. With no processing needed and is readable enough.

      - name: Check GitHub Action workflows
        shell: bash {0}
        run: |
          pip install zizmor

          zizmor="$(zizmor . --gh-token "${{ github.token }}")"
          exit_code="$?"

          printf '%s\n' "$zizmor"

          printf '%b\n' "\`\`\`" >> $GITHUB_STEP_SUMMARY
          printf '%s\n' "$zizmor" >> $GITHUB_STEP_SUMMARY
          printf '%b' "\`\`\`" >> $GITHUB_STEP_SUMMARY

          exit "$exit_code"

What would make sense in this situation is that if there was a blanket ignore, you could show these suppressed warnings but not as errors, as they would be with personas. You just want to be aware of them.

Like a persona but they don't cause exit codes of non 0 of you have a blanket ignore, just puts them in special suppression category that by default is always shown?

I think that's really what it comes down to, not hiding this info from future tests by a blanket ignore whilst not causing an exit code of greater than 0.

@userdocs
Copy link
Contributor

Maybe combined with a unique exit code just for these globally supressed errors.

https://woodruffw.github.io/zizmor/usage/#exit-codes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants