[BUG]: Ignore comment placement is not intuitive

@woodruffw

Pre-submission checks

I am not filing a feature request. These should be filed via the feature request form instead.
I have looked through the open issues for a duplicate report.

Expected behavior

Ignore results within composite actions using inline comments.

Actual behavior

Results are not ignored when inline comments are used within composite actions.

Reproduction steps

On a composite action that raises error[template-injection] for example, adding # zizmor: ignore[template-injection] has no effect.

Logs

Additional context

Kudos for the great work being done @woodruffw.

zizmor is being used to tighten up our actions here: ansys/actions#725

Note

I am opening this as a bug and not as a feature request because the documentation implies that inline comments should be supported within composite actions. Although on this page, it is stated that a zizmor.yml config cannot be used. I stand to be corrected though, maybe I missed something. Thanks!

Similar experience when running zizmor as part of a reusable workflow, inline ignore settings and zizmor.yml in caller repository are not honored.

Hi both, thanks for the report. Could you please provide a reproducer case for me? Inline ignore comments should definitely work in all workflow and action definitions, although you're right that configuration-level ignores are currently workflow-only.

For context, this works as expected for me, i.e. produces no findings except one ignored one:

name: test-workflow-ignores

on:
  push:
  workflow_call:

permissions: {}

jobs:
  inject-me:
    runs-on: ubuntu-latest

    steps:
      - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # zizmor: ignore[template-injection]
        with:
          script: |
            return "${{ github.event.issue.title }}"

it also works if you put the ignore comment on an interior element, e.g.:

    steps:
      - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea
        with:
          script: | # zizmor: ignore[template-injection]
            return "${{ github.event.issue.title }}"

However, what doesn't work is if you put the comment within the YAML string itself, e.g.:

    steps:
      - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea
        with:
          # does not work!
          script: |
            return "${{ github.event.issue.title }}" # zizmor: ignore[template-injection]

The reason for this is essentially syntactical: the first two examples are YAML comments, while the third is a bash comment that happens to share the same syntax as a YAML comment. zizmor handles (most) ignores on the syntactical level of YAML, since we don't know whether arbitrary strings have internal syntaxes that are compatible with YAML.

TL;DR: Your inline ignore comment needs to be somewhere in the YAML syntax tree itself, not within a string literal, since zizmor can't analyze the latter. This could probably be documented better within the online docs, and I'd welcome thoughts on how to do that 🙂

@woodruffw

Hi @woodruffw ,

I have repo A where I define a reusable workflow with zizmor, and repo B (caller repo) from which I call that workflow. There are some private actions that I use within my workflows in repo B, which I would prefer to ignore scanning against some of the online audit rules such as impostor-commit for example, as the GH_TOKEN for my caller workflow does not have the permissions (by design) to make http requests to my private actions repo at https://api.github.com/repos/org/private-actions-repo. Adding an inline ignore comment within the workflow file in repo B, where I call these private actions is not getting honored.

steps:
 - uses: private-actions-repo/my-action@v1 # zizmor: ignore[impostor-commit]

Similarly, defining a zizmor.yml config file in repo B, does not get picked up. If I define one however, in repo A, where my zizmor reusable workflow is defined, that configuration file is picked up.

Thanks for the additional context!

There are some private actions that I use within my workflows in repo B, which I would prefer to ignore scanning against some of the online audit rules such as impostor-commit for example, as the GH_TOKEN for my caller workflow does not have the permissions (by design) to make http requests to my private actions repo at api.github.com/repos/org/private-actions-repo. Adding an inline ignore comment within the workflow file in repo B, where I call these private actions is not getting honored.

There are two different things here:

Disabling a particular audit so that it doesn't produce any results at all;
Ignoring the results of an audit after it has run.

It sounds like you're trying to do (1) since you have permission limitations, but you're trying to use the ignore machinery, which only does (2). More specifically, the ignore machinery only does (2) because it works by filtering results after the audit runs; it can't stop the audit itself from running.

To fix this, you can run zizmor with --no-online-audits, as documented here: https://woodruffw.github.io/zizmor/usage/#operating-modes -- that will perform online collection if you need it, but will disable every audit that needs GitHub API access. Or, if you don't need online access at all, you can run zizmor with --offline to completely disable all network behavior.

Similarly, defining a zizmor.yml config file in repo B, does not get picked up. If I define one however, in repo A, where my zizmor reusable workflow is defined, that configuration file is picked up.

This is semi-intentional behavior: zizmor's configuration was originally intended to be global, i.e. per-run instead of per-repository. In practice, this means that remote configs are never loaded, and that you can't "extend" a run's configuration by loading indirect configurations.

I have plans to fix this more generally by introducing a "v2" config format that can be loaded multiple times, once per repository being audited. That's going to require significant architectural changes, however, so I haven't begun on it yet.

I've opened #614 to fix the documentation side of this -- please take a look and let me know if that further clarifies how ignore comments should be placed.

@woodruffw

@woodruffw Thanks for the detailed explanations.

I followed your guide of adding the comments to the yaml elements, and here are my findings:

Considering the following example:

name: "Automerge dependabot and pre-commit.ci PRs"
description: "Action merging dependabot and pre-commit.ci PRs."

inputs:
  approver:
    description: "Approver GitHub ID."
    required: true
    type: string
  approver-token:
    description: "Approver GitHub token."
    required: true
    type: string

runs:
  using: "composite"
  steps:

    - name: Verify allowed PRs (dependabot and pre-commit.ci)
      id: verify-allowed-prs
      shell: bash
      run: |
        # Check if the author is either dependabot or pre-commit.ci.
        if [[ ${{ github.event.pull_request.user.login }} == 'dependabot[bot]' ]] || [[ ${{ github.event.pull_request.user.login }} == 'pre-commit-ci[bot]' ]]; then
        .
        .
        .

Comment placed beside the first key: value pair in the step sequence works:

name: "Automerge dependabot and pre-commit.ci PRs"
description: "Action merging dependabot and pre-commit.ci PRs."

inputs:
  approver:
    description: "Approver GitHub ID."
    required: true
    type: string
  approver-token:
    description: "Approver GitHub token."
    required: true
    type: string

runs:
  using: "composite"
  steps:

    - name: Verify allowed PRs (dependabot and pre-commit.ci)  # zizmor: ignore[template-injection] this works fine
      id: verify-allowed-prs
      shell: bash
      run: |
        # Check if the author is either dependabot or pre-commit.ci.
        if [[ ${{ github.event.pull_request.user.login }} == 'dependabot[bot]' ]] || [[ ${{ github.event.pull_request.user.login }} == 'pre-commit-ci[bot]' ]]; then
        .
        .
        .

Comment in front of any other key: value pair in the step sequence doesn't work:

name: "Automerge dependabot and pre-commit.ci PRs"
description: "Action merging dependabot and pre-commit.ci PRs."

inputs:
  approver:
    description: "Approver GitHub ID."
    required: true
    type: string
  approver-token:
    description: "Approver GitHub token."
    required: true
    type: string

runs:
  using: "composite"
  steps:

    - name: Verify allowed PRs (dependabot and pre-commit.ci)
      id: verify-allowed-prs   # zizmor: ignore[template-injection] this will not work
      shell: bash
      run: |
        # Check if the author is either dependabot or pre-commit.ci.
        if [[ ${{ github.event.pull_request.user.login }} == 'dependabot[bot]' ]] || [[ ${{ github.event.pull_request.user.login }} == 'pre-commit-ci[bot]' ]]; then
        .
        .
        .

This may be by design, but maybe it is worth clarifying in the documentation too. Something like to ignore findings within a single composite action step, inline comments should be placed in front of the first **key: value** pair should do.

This may be by design, but maybe it is worth clarifying in the documentation too.

Hmm, that specific case looks like a bug -- I'll be able to look into that more in a bit.

Ah yeah, I see what's happening here -- we create a span for the script location and a span for the name if present, and the latter ends up excluding the id from the set of spans in the finding:

In other words, the findings look like this:

error[template-injection]: code injection via template expansion
  --> tests/integration/test-data/issue-612-repro/action.yml:15:7
   |
15 |       - name: Verify allowed PRs (dependabot and pre-commit.ci)
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this step
16 |         id: verify-allowed-prs # zizmor: ignore[template-injection] this works fine
17 |         shell: bash
18 | /       run: |
19 | |         # Check if the author is either dependabot or pre-commit.ci.
...  |
23 | |           exit 1
24 | |         fi
   | |___________^ github.event.pull_request.user.login may expand into attacker-controllable code
   |
   = note: audit confidence → High

error[template-injection]: code injection via template expansion
  --> tests/integration/test-data/issue-612-repro/action.yml:15:7
   |
15 |       - name: Verify allowed PRs (dependabot and pre-commit.ci)
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this step
16 |         id: verify-allowed-prs # zizmor: ignore[template-injection] this works fine
17 |         shell: bash
18 | /       run: |
19 | |         # Check if the author is either dependabot or pre-commit.ci.
...  |
23 | |           exit 1
24 | |         fi
   | |___________^ github.event.pull_request.user.login may expand into attacker-controllable code
   |
   = note: audit confidence → High

2 findings: 0 unknown, 0 informational, 0 low, 0 medium, 2 high

(Note that id: is included in the summary, but as the lines indicate is not covered by a span.)

This is pretty confusing and IMO is indeed a bug (in terms of being an unintentional consequence of me being too clever with spans).

Thanks for the reproducer, I should be able to fix this soon!

tests: add repro case for #612

#616 will fix this, once I fill it in.

tests: add repro case for #612 (#616)

Navigation Menu

Repository navigation

[BUG]: Ignore comment placement is not intuitive #612

Pre-submission checks

Expected behavior

Actual behavior

Reproduction steps

Logs

Additional context

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[BUG]: Ignore comment placement is not intuitive #612

Description

Pre-submission checks

Expected behavior

Actual behavior

Reproduction steps

Logs

Additional context

Activity

temenuzhka-thede commented on Mar 17, 2025

woodruffw commented on Mar 17, 2025

woodruffw commented on Mar 17, 2025

temenuzhka-thede commented on Mar 18, 2025

woodruffw commented on Mar 18, 2025

woodruffw commented on Mar 18, 2025

moe-ad commented on Mar 18, 2025

woodruffw commented on Mar 18, 2025

woodruffw commented on Mar 18, 2025

woodruffw commented on Mar 18, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions