Option to be happy with no inputs collected

[funnelfiasco]

In general, I think a non-zero exit on "no inputs collected" is good. However, there may be use cases where that's not desirable. For example, $dayjob has a tool that examines pull requests and runs zizmor if there are files changed in .github/workflows. But sometimes those files aren't actual workflow files, they're something else. We've changed it to explicitly look for *y{,a}ml but even then, I think it's possible that a yaml file exists that isn't parsed as a GitHub worfklow.

So! I thought it might be good to have an option for a happy (0) exit in that case. --no-inputs-is-fine-actually (I'm workshopping the name still) would be false by default, but give users the option to not care in cases where they might not actually care. The downside is that it would mean zizmor doesn't catch files that can't be parsed because they're invalid (I think), but for some users, that might be preferable to zizmor giving an unclean exit.

I'm happy to contribute this if it seems useful, but I wanted to bring up the idea before I started doing the work. :-)

[woodruffw]

Thanks for the laying out the idea! I appreciate the detail and motivating case.

I think my main concern with a "no error on no inputs" type flag is that it can be a footgun for unsuspecting users. Two scenarios off the top of my head:

1. A user interacts with zizmor indirectly through some other system (a GitHub Action, or whatever) that uses this flag. They pass .gituhb/ (sic) by accident and see no errors and think that everything is fine, when in reality they've made a typo and no audit of their inputs was ever performed. I think they'd be right to complain about being mislead in such a case.
2. A user sceripts zizmor with a glob input, e.g, zizmor .github/workflows/*.yml. They're accustomed to this working because they typically run zizmor on a Unix-y host through a Unix-y shell. However, one day they run their script on a Windows host, or they wrap it in another script that executes it through exec*(2) instead of a subshell. This would then fail normally (since the glob won't expand and zizmor will error on input collection), but with this flag the user would believe that the audit completed successfully.

More philosophically, I think I'm wary of flags that turn commands into no-ops: they're essentially a way to push control flow into a program, when often the clearer thing is to write control flow around a program.

With that said, the scenario you're describing is a tough one -- I hadn't really considered that people would only want to run zizmor on specific changes, rather than blanket triggering it any time any repo change occurs 😅. And it's especially hard/annoying when you have to not just sniff the file extension but the file shape (workflow or not), as you say.

Thinking laterally: running zizmor is usually pretty fast, especially if you aren't running the online audits (particularly impostor-commit). Is it possible you could instead run zizmor unconditionally, and deduplicate the results that don't change between runs? I believe that's what GitHub's Advanced Security does with SARIF inputs under the hood.

[funnelfiasco]

I think my main concern with a "no error on no inputs" type flag is that it can be a footgun for unsuspecting users.

Yeah, it's definitely a tradeoff of one bad case for another. Of course, my preferred bad case is clearly the right choice. 😉

Run comparison is a reasonable suggestion, but it's not workable right now, since we don't store the scan results. It also would require having a baseline to compare the first Inspector-enabled PR to (which is a thing we're working on, but I'm not sure how we'll approach it technically).

I don't think this rises to the level of a "we'll maintain a fork with this feature added because we really want it" at this point. We've worked around the problem to some degree by being more selective about what can trigger running zizmor in a scan. There will still be corner cases, but the can is kicked down the road for the time being.

I'm happy to leave things as-is (as-are?) for now and see if other folks have input. I also wonder if it would be possible to scope the "it's fine, actually" to the case where 1+ files were read and they just don't seem like GitHub workflows (i.e. they're not obviously invalid workflows but maybe someone has a Ansible file in there for some reason). If we could reasonably do that, would you be more amenable?

[woodruffw]

I'm happy to leave things as-is (as-are?) for now and see if other folks have input. I also wonder if it would be possible to scope the "it's fine, actually" to the case where 1+ files were read and they just don't seem like GitHub workflows (i.e. they're not obviously invalid workflows but maybe someone has a Ansible file in there for some reason). If we could reasonably do that, would you be more amenable?

Yeah, I think I'm inclined to leave things as are, at least for the immediate term 😅 -- I like your idea about limiting this to the case where 1+ files were actually read, but I worry that the distinction there is hard to communicate to users (and still leaves open footguns, e.g. zizmor ./foo/action/ .github/* would still surface the Windows glowing problem, since we'd successfully audit the action under ./foo/action and then go on to silently ignore the unexpanded glob).

[woodruffw]

#1515 should address this!