Repository navigation
ci: Resolve most zizmor findings as of zizmor@1.31.1 - #775
Conversation
|
After investigating, the CI failures appear unrelated to the changes made in this PR. |
|
Suggestion: merge #774, then give me a chance to rebase this PR so I can fix any issues it reintroduces. |
hashcatHitman
changed the title
zizmor findings as of zizmor@1.31.1zizmor findings as of zizmor@1.31.1
|
Merged. |
|
Ah, thanks! I'll get this working. |
See: https://github.com/actions/checkout/releases/tag/v4.4.0 This is the same version we're already implicitly using. Signed-off-by: hashcatHitman <155700084+hashcatHitman@users.noreply.github.com>
See: https://github.com/release-plz/action/releases/tag/v0.5.142 This is the same version we're already implicitly using. Signed-off-by: hashcatHitman <155700084+hashcatHitman@users.noreply.github.com>
Signed-off-by: hashcatHitman <155700084+hashcatHitman@users.noreply.github.com>
Signed-off-by: hashcatHitman <155700084+hashcatHitman@users.noreply.github.com>
Signed-off-by: hashcatHitman <155700084+hashcatHitman@users.noreply.github.com>
See: https://docs.github.com/en/actions/reference/workflows-and-actions/expressions#case Signed-off-by: hashcatHitman <155700084+hashcatHitman@users.noreply.github.com>
|
This should be ready to merge now, as long as there aren't any concerns on the review side. |
Kobzol
left a comment
Kobzol
left a comment
There was a problem hiding this comment.
I find the move from inline expressions (that are not user controlled) to environment variables kinda stupid, tbh.. But if Zizmor wants it..
|
The justification for it is precisely "template injection", and the pedantic persona prohibits all, which tbh to me feels a bit like saying "run shellcode only if someone can get it into an env var, and move your vulns into setting your env vars". hello??? as I wish to annihilate this repo's (relevance) by moving it, I would rather not change such things that would require me to reunderstand the CI code significantly in the interim while this repo still lives, if that is alright. |
|
everything else is fine. |
Sure, I'll drop 2daac5c, f98e955, and aad2764. I can always revisit it another time, if need be. |
Signed-off-by: hashcatHitman <155700084+hashcatHitman@users.noreply.github.com>
|
Should be good to go now. |
workingjubilee
left a comment
workingjubilee
left a comment
There was a problem hiding this comment.
Thanks! This should be good.
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
| uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 |
There was a problem hiding this comment.
It kinda sucks that this is what we gotta do and pinning a version down to the patch ain't good enough, but I absolutely prefer the result.
When I first found out about the reality that every version was just a mutable tag, I had to wonder if that code was written by 
Would that we could retain such innocence in this world!
There was a problem hiding this comment.
If it makes you feel better, lockfiles are a thing for GitHub Actions now. We don't want to use them just yet, but they might be an alternative in the (near?) future. See: #t-infra > github actions workflow lockfiles
There was a problem hiding this comment.
That would def be better, but the hash plus comment is good enough for me, merely noisy. if inline comments for version instead of only tailing comments were allowed it would address my only druther here also (I want the version number "first", and an above-comment isn't quite right either) but I almost kinda hope yaml doesn't support that...?
There was a problem hiding this comment.
I almost kinda hope yaml doesn't support that...?
It doesn't. I hope. In any case, zizmor doesn't, so :).
As I mentioned, per #778 we intend to make this repo much more irrelevant, so I'd rather no convenient automation to make the code more comfortable where it is currently sitting. |
Sounds good to me, yeah. I just offer it to everyone so they aren't left with harder-to-update dependencies and no help about it. Thanks! |
|
Appreciated! |
In pursuit of getting more
zizmorlints enforced acrossrust-langrepos throughcrabwatch, this PR fixes mostzizmorfindings as ofzizmor@1.31.1, using theauditorpersona. The lints that actually fired per-persona level are:regularpersona findings:artipackedexcessive-permissions(See also: Solve theexcessive-permissionslint crabwatch#97)unpinned-usesunsound-ternarypedanticpersona findings:regularundocumented-permissionsconcurrency-limits(Due to concerns about special cases. See also: Determine special cases forconcurrency-limitscrabwatch#107)template-injection(By request)auditorpersona findings:pedanticThe hash pin format is supported by tools like Dependabot and Renovate, which can update the pinned hashes and their associated comments. If you'd like help setting either of those up, let me know which you prefer, and if you'd like it in this PR or a new one. By default, I'd assume to set it up to only update GitHub Actions.