Repository navigation
ci: Resolve most zizmor findings as of zizmor@1.31.1
#7212
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
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
71e643a
ci: Disable credential persistence
hashcatHitman e1ab351
ci: Expand `github.event.inputs.commit_hash` in `env`
hashcatHitman eed5eec
ci: Expand `github.event.inputs.rustfmt_configs` in `env`
hashcatHitman a149e10
ci: Expand `github.event.inputs.clone_url` in `env`
hashcatHitman 66bf953
ci: Expand `github.event.inputs.branch_name` in `env`
hashcatHitman 6251ca8
ci: Expand `github.event.inputs.language_edition` in `env`
hashcatHitman f6fc894
ci: Expand `github.event.inputs.style_edition` in `env`
hashcatHitman a2d4800
ci: Expand `github.event.release.tag_name` in `env`
hashcatHitman ef138d3
ci: Prefer shell expansion over template expansion
hashcatHitman bce8aa0
ci: Expand `matrix.target` in `env`
hashcatHitman add9f81
ci: Expand `matrix.os` in `env`
hashcatHitman 0f85075
ci: Expand `matrix.build` in `env`
hashcatHitman da2aab9
ci: Expand `matrix.integration` in `env`
hashcatHitman 144bbf6
ci: Expand `matrix.package` in `env`
hashcatHitman 43dc4a9
ci: Expand `toJson(needs)` in `env`
hashcatHitman 7d06a5c
ci: Disable all permissions by default
hashcatHitman f87c96e
ci: Grant minimum required permissions
hashcatHitman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
Is there a way we can require zizmor checks to pass as opposed to monitor for now? I know we're still in the gradually-cover-all-the-repo phase.
(Or wait, is it just some rules are not enforced org-wide yet?)
Uh oh!
There was an error while loading. Please reload this page.
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.
I'm not sure, I've never dealt with a workflow that mysteriously imposes itself from another repo like this before. If you have access to branch rulesets, my best guess would be to add a ruleset that requires status checks to pass, but I'm not sure if Crabwatch will actually show up in that list. (I don't have such access over
surveys, which would have been my place to check, so I'm guessing you probably don't have such access?)Alternatively, you could just re-run
zizmoryourself in this repository and require it the same way you would any other workflow. It'd be a bit redundant, but it is an option.This seems to imply that you think once we get out of that phase we're going to start requiring
crabwatchto pass. While that was the original plan, I believe we've decided against doing that for the foreseeable future due to several lints being capable of failing due to factors outside of our control (originally, this referred toarchived-uses, but I believe I made a more complete list in this issue: rust-lang/crabwatch#92). This change of plans was announced here: #t-infra/announcements > GitHub Actions linter @ 💬.We maintain a policy here: https://github.com/rust-lang/crabwatch/blob/main/zizmor-policy.yml
Not all rules are enabled by default. We're enabling more rules over time, but before we do that, we have to fix them so that they pass. The policy allows defining overrides for individual repos, so the workflow looks like one of two things, generally:
${repo}from the repos that lint fires forrust-lang/${repo}rust-lang/${repo}${repo}from the repos that any lint fires forrust-lang/${repo}as possiblerust-lang/${repo}I started by doing something like the first approach, where I'd specifically target ~3 lints at a time, but I've found it isn't much more work to just tackle almost all of them at once. Fewer PRs to bother each repository with, and saying "I'm here to fix all/almost all of the lints" makes it less surprising for me to enable lints that were already solved for a given repository.
I've been using the script found here: https://github.com/hashcatHitman/.zizmor_meta/blob/514f583f97e1fda98861582cfdd4bebfe7f639f8/zizmor_blast.bash
To do bulk scans which I manually cache to pick through later. I'm not sure if we actually use
crabwatchon any repos that are archived, not public, or forks, but I've started by prioritizing the public, non-archived, non-forks. In general, thecrabwatchcustom property is what decides if we runcrabwatchon a repo. You'd probably get more accurate results by running the actualcrabwatchwrapper, but I'm stubborn and don't feel like downloadingyqjust for this, yet.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.
Ah okay, thanks for the context.
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.
Hm yeah alright, I didn't consider the external factor possibility (i.e. an action can get archived and now if you hard-require zizmor nothing can be merged). That makes sense 👍