-
Notifications
You must be signed in to change notification settings - Fork 735
Add Zizmor Scanning on push/pull request #3141
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
base: devel
Are you sure you want to change the base?
Conversation
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for submitting this one @x1101
Just looking at the commit history. May I suggest squashing some of those commits down? I know they'll probably all get squashed on merge but it might be easier on reviewers to have a cleaner set of commits.
Bump to test action Update zizmor.yml adding `.github` as an input, per [this update](https://github.com/zizmorcore/zizmor/blob/1c3de9cd6f0dc5c56b726b4e57e7c20bf233776a/.github/workflows/zizmor.yml#L25) Update zizmor.yml Update zizmor.yml Update zizmor.yml renamed Update .github/workflows/scan.yml Applying suggestions based on feedback Co-authored-by: Felix Fontein <felix@fontein.de>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor suggestions to remove unnecessary comments but it looks good @x1101 thank you!
.github/workflows/scan.yml
Outdated
| # contents: read # only needed for private repos | ||
| # actions: read # only needed for private repos |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # contents: read # only needed for private repos | |
| # actions: read # only needed for private repos |
|
Bah, it looks like that one fixup got lost in the cherry-pick and rebase shuffle. I'll get that back in momentarily. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you @x1101
@oraNod the linked list has 40 errors and warnings. They should probably be addressed before merging. They don't appear in the PR until it's merged. @x1101 you should be able to invoke |
| jobs: | ||
| zizmor: | ||
| name: Run zizmor 🌈 | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| security-events: write | ||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 | ||
| with: | ||
| persist-credentials: false | ||
|
|
||
| - name: Run zizmor 🌈 | ||
| uses: zizmorcore/zizmor-action@e673c3917a1aef3c65c972347ed84ccd013ecda4 # v0.2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@x1101 could you just use the reusable workflow instead of duplicating the job steps? As in this example: #2681 (comment)
| - .github/workflows/* | ||
| pull_request: | ||
| paths: | ||
| - .github/workflows/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make this future-proof if the project ever starts using in-tree actions. Also, I'd still run it on PR merge every time, for faster feedback:
| - .github/workflows/* | |
| pull_request: | |
| paths: | |
| - .github/workflows/* | |
| pull_request: | |
| paths: | |
| - .github/actions/*/action.yml | |
| - .github/workflows/*.yml |
|
@x1101 could you also extract bits of Nox integration work from the patch linked @ #2681 (comment)? Do you know how to cherry-pick commits? Doing this will allow you (and other people) to also run zizmor locally. Meaning you'd be able to address those violations mentioned above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use a more specific name than just scan. It's too generic and can refer to just about anything.
Per discussion in #2681, adding zizmor scanning.
There are several things we could tweak on this.
I've left this as all branches on push and on pull request at this point, but would be happy to adjust if that doesn't work for some reason.
Resolves #2681.