-
-
Notifications
You must be signed in to change notification settings - Fork 116
Feature: Require or recommend the timeout-minutes property for all jobs #1023 #1067
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: main
Are you sure you want to change the base?
Conversation
zizmorcore#1023 feat: add basic timeout-minutes audit for GitHub Actions jobs Implements a new audit rule that detects missing timeout-minutes property on GitHub Actions jobs to prevent runaway jobs from consuming runner minutes. What's implemented: - Basic audit structure following existing patterns (unpinned-images) - Detection of missing timeout-minutes on normal jobs - Proper finding generation with medium severity - Integration with pedantic persona - Registration in audit registry What's still needed (for future iterations): - Handle reusable workflows (jobs with 'uses' don't support timeout-minutes directly) - Check step-level timeout-minutes and transitive coverage by job timeouts - Add comprehensive test coverage - Consider different severity levels?
Kusari Analysis ResultsAnalysis for commit: f0c3711, performed at: 2025-08-10T02:25:03Z • • Recommendation✅ PROCEED with this Pull Request Summary✅ No Flagged Issues Detected All values appear to be within acceptable risk parameters. No pinned version dependency changes, code issues or exposed secrets detected! Found this helpful? Give it a 👍 or 👎 reaction! |
|
Thanks for tackling this @EmmanuelBerkowicz! Please give me a ping when this is ready for an initial review (that can be now, but I didn't want to assume). |
|
@woodruffw , I've given it my best shot, but like I said in the PR, it's not the entire issue scope just yet. If you could give it a once over and let me know if my approach is in the right direction that would be a huge help! If I am reading the scope correctly, there is still a fair bit to go and it will be a lot easier for me to do if I know that I'm heading in the right direction :) |
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 @EmmanuelBerkowicz!
Your approach here looks good to me; I think once this includes checks for step-level timeouts and some tests then it'll be good to go.
(Let me know if you have any questions about how to run the tests, including updating the snapshots. Most of it is documented here: https://docs.zizmor.sh/development/#testing)
| if job.timeout_minutes.is_none() { | ||
| findings.push(self.build_finding( | ||
| job.location().primary(), | ||
| "job is missing timeout-minutes", |
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.
| "job is missing timeout-minutes", | |
| "missing timeout-minutes", |
| let mut findings = vec![]; | ||
|
|
||
| // Check if timeout-minutes is missing | ||
| if job.timeout_minutes.is_none() { |
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.
This approach looks right to me! Let's do the same for each step in the job as well 🙂
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.
This should probably be moved under crates/zizmor/tests/integration/test-data and named timeout-minutes.yml or similar, once you're ready 🙂
| register_audit!(audit::self_hosted_runner::SelfHostedRunner); | ||
| register_audit!(audit::known_vulnerable_actions::KnownVulnerableActions); | ||
| register_audit!(audit::unpinned_uses::UnpinnedUses); | ||
| register_audit!(audit::timeout_minutes::TimeoutMinutes); |
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.
Move this to the end, please -- right now these are declared in order of introduction, not alphabetically.
feat: add basic timeout-minutes audit for GitHub Actions jobs
Summary: I have made a start on the open issue, requesting feedback on progress as this is my first rust project.
Implements a new audit rule that detects missing timeout-minutes property on GitHub Actions jobs to prevent runaway jobs from consuming runner minutes.
What's implemented:
What's still needed (for future iterations):
Addresses #1023