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

[BUG]: credential persistence through GitHub Actions artifacts #290

Open
2 tasks done
dcampbell24 opened this issue Dec 13, 2024 · 4 comments
Open
2 tasks done

[BUG]: credential persistence through GitHub Actions artifacts #290

dcampbell24 opened this issue Dec 13, 2024 · 4 comments
Labels
bug Something isn't working triage Issue is being triaged

Comments

@dcampbell24
Copy link

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

It should tell you that you need

        uses: actions/checkout@v4
        with:
          persist-credentials: false

Actual behavior

Instead of

   |
15 |         - name: "Checkout repo"
   |  _________-
16 | |         uses: actions/checkout@v4
   | |_________________________________- does not set persist-credentials: false
   |

Reproduction steps

  1. Missing persist-credentials: false
  2. Run zizmor

Logs

No response

Additional context

No response

@dcampbell24 dcampbell24 added bug Something isn't working triage Issue is being triaged labels Dec 13, 2024
@woodruffw
Copy link
Owner

Hi @dcampbell24, thanks for the report. To make sure I understand: is the problem here that zizmor isn't including the with: clause in the finding's annotation?

The reason we don't currently do that is a rendering limitation: emitting a multi-line annotation won't be very pretty or readable at the moment.

An alternative here would be for us to emit does not set persist-credentials: false (inside with:) or similar, but I suspect that's also somewhat confusing. I'm curious if you have thoughts for presenting this.

@dcampbell24
Copy link
Author

dcampbell24 commented Dec 13, 2024

Yeah, I tried it at first without the with:, then realized I needed it if it wasn't already present. Ideally, it should handle both the case where there is already a with: clause and where there isn't one yet.

If you already have a with: clause it gives:

warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> /home/...
    |
113 |       - name: checkout
    |  _______-
114 | |       uses: actions/checkout@v4
115 | |       with:
116 | |         ref: ${{ github.ref_name }}
    | |___________________________________- does not set persist-credentials: false
    |
    = note: audit confidence → Low

instead of:

warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> /home/...
    |
113 |       - name: checkout
114 |         uses: actions/checkout@v4
115 |         with:
116 |           ref: ${{ github.ref_name }}
    |  ________-
    | |___________________________________- does not set persist-credentials: false
    |
    = note: audit confidence → Low

Can you emit two annotations? One that says to add with: if it is missing and a second that says to add persist-credentials: false. Like above with the proper indenting if the with: is missing and:

    |
113 |       - name: checkout
114 |         uses: actions/checkout@v4
    |  ______-
    | |___________________________________- does not set with:

@dcampbell24
Copy link
Author

Change the order so the with: comes first if it is needed.

@woodruffw
Copy link
Owner

Hmm yeah, that might work -- in general multiple annotations on the same span don't flow super well, but I'll give that a try later today!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage Issue is being triaged
Projects
None yet
Development

No branches or pull requests

2 participants