unpinned-tools: false positive on steps with if: false (allowlist-registration pattern)

[potiuk]

Summary

The unpinned-tools audit flags steps that are syntactically if: false — i.e., guaranteed never to execute. In repos that use a composite action as a declarative allowlist registry (where every - uses: entry is if: false purely so dependabot tracks the SHA for inclusion in an approved-actions list), this produces a false positive: the audit's premise ("when this step runs, it'll install an unpinned tool") doesn't hold for steps that don't run.

Reproducer

Minimal composite action:

runs:
  using: "composite"
  steps:
    - uses: 1Password/load-secrets-action@<sha>  # v4.0.0
      if: false

zizmor v1.25.2 raises warning[unpinned-tools] here, even though the step has no runtime effect.

Real-world example: https://github.com/apache/infrastructure-actions/blob/main/.github/actions/for-dependabot-triggered-reviews/action.yml — 105 if: false entries serving as the approved-actions allowlist; one currently triggers unpinned-tools.

Suggested handling

Skip unpinned-tools (and arguably similar runtime-behavior audits — bot-conditions, cache-poisoning, etc.) on steps where if is a statically-falsey literal: if: false, if: ${{ false }}. Detecting more sophisticated never-runs conditions (e.g. if: ${{ github.actor == 'never-this-user' }}) is harder and out of scope; literal-false detection alone covers the common allowlist-registry pattern.

This wouldn't weaken coverage of any step that could actually run.

Workaround in the meantime

Inline # zizmor: ignore[unpinned-tools] per offending step — works, but each new entry that triggers the audit needs its own ignore, which scales poorly across allowlists of 100+ entries.

Happy to discuss / send a PR if there's interest.

[woodruffw]

Thanks @potiuk!

This is an interesting one -- I hadn't considered adding if:-awareness to the various audits, but I agree these findings are effectively mooted by being in a step that can't actually fire.

I think the right thing to do here is to handle this at the audit trait layer -- arguably the Steps iterator should completely skip any step that is statically inferred to not execute. I'm not 100% sure about that though, since perhaps there's an argument for surfacing these in a pedantic/auditor finding. Curious what you think 🙂

[potiuk]

Yeah. I think that's the right call. Also some people might simply even want to disable the steps and that sounds like a good idea to skip those while they are disabled.

[woodruffw]

(P.S. how did you file this without one of the templates? I thought I had limited that. Not a problem since this is very detailed - thanks! - but I'm curious if there's another issue template bypass I'm not aware of 🙂)

[potiuk]

When you create it with `gh pr create' , it does not use any template.

[woodruffw]

@potiuk Sorry, forgot to say explicitly: if you're open to sending a PR for this, please do!

(I suspect filtering the Steps is the right way to go, but I haven't thought through it fully.)

[potiuk]

Opened :)