From e3bdefbfa8c14210796b1f1636f2f18dd9a8b6ad Mon Sep 17 00:00:00 2001
From: Emmanuel Berkowicz <berkowiczemmanuel@gmail.com>
Date: Sun, 10 Aug 2025 12:20:29 +1000
Subject: [PATCH] Feature: Require or recommend the timeout-minutes property
 for all jobs #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?
---
 crates/zizmor/src/audit/mod.rs             |  1 +
 crates/zizmor/src/audit/timeout_minutes.rs | 62 ++++++++++++++++++++++
 crates/zizmor/src/registry.rs              |  5 +-
 test-workflow.yml                          |  8 +++
 4 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 crates/zizmor/src/audit/timeout_minutes.rs
 create mode 100644 test-workflow.yml

diff --git a/crates/zizmor/src/audit/mod.rs b/crates/zizmor/src/audit/mod.rs
index f6d84d26f..a3f196597 100644
--- a/crates/zizmor/src/audit/mod.rs
+++ b/crates/zizmor/src/audit/mod.rs
@@ -34,6 +34,7 @@ pub(crate) mod secrets_inherit;
 pub(crate) mod self_hosted_runner;
 pub(crate) mod stale_action_refs;
 pub(crate) mod template_injection;
+pub(crate) mod timeout_minutes;
 pub(crate) mod unpinned_images;
 pub(crate) mod unpinned_uses;
 pub(crate) mod unredacted_secrets;
diff --git a/crates/zizmor/src/audit/timeout_minutes.rs b/crates/zizmor/src/audit/timeout_minutes.rs
new file mode 100644
index 000000000..0bcd056a1
--- /dev/null
+++ b/crates/zizmor/src/audit/timeout_minutes.rs
@@ -0,0 +1,62 @@
+use anyhow::Result;
+
+use crate::{
+    finding::{
+        Confidence, Finding, Persona, Severity,
+        location::{Locatable as _, SymbolicLocation},
+    },
+    models::workflow::JobExt as _,
+    state::AuditState,
+};
+
+use super::{Audit, AuditLoadError, audit_meta};
+
+pub(crate) struct TimeoutMinutes;
+
+impl TimeoutMinutes {
+    fn build_finding<'doc>(
+        &self,
+        location: SymbolicLocation<'doc>,
+        annotation: &str,
+        job: &super::NormalJob<'doc>,
+    ) -> Result<Finding<'doc>> {
+        let mut annotated_location = location;
+        annotated_location = annotated_location.annotated(annotation);
+        Self::finding()
+            .severity(Severity::Medium)
+            .confidence(Confidence::High)
+            .add_location(annotated_location)
+            .persona(Persona::Pedantic)
+            .build(job.parent())
+    }
+}
+
+audit_meta!(
+    TimeoutMinutes,
+    "timeout-minutes",
+    "missing timeout-minutes on jobs"
+);
+
+impl Audit for TimeoutMinutes {
+    fn new(_state: &AuditState<'_>) -> Result<Self, AuditLoadError> {
+        Ok(Self)
+    }
+
+    fn audit_normal_job<'doc>(
+        &self,
+        job: &super::NormalJob<'doc>,
+    ) -> anyhow::Result<Vec<Finding<'doc>>> {
+        let mut findings = vec![];
+
+        // Check if timeout-minutes is missing
+        if job.timeout_minutes.is_none() {
+            findings.push(self.build_finding(
+                job.location().primary(),
+                "job is missing timeout-minutes",
+                job,
+            )?);
+        }
+
+        Ok(findings)
+    }
+}
diff --git a/crates/zizmor/src/registry.rs b/crates/zizmor/src/registry.rs
index 3a74f4a0a..7c5464120 100644
--- a/crates/zizmor/src/registry.rs
+++ b/crates/zizmor/src/registry.rs
@@ -2,13 +2,13 @@
 //! audits.
 
 use std::{
-    collections::{BTreeMap, btree_map},
+    collections::{btree_map, BTreeMap},
     fmt::Display,
     process::ExitCode,
     str::FromStr,
 };
 
-use anyhow::{Context, anyhow};
+use anyhow::{anyhow, Context};
 use camino::{Utf8Path, Utf8PathBuf};
 use indexmap::IndexMap;
 use serde::Serialize;
@@ -352,6 +352,7 @@ impl AuditRegistry {
         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);
         register_audit!(audit::insecure_commands::InsecureCommands);
         register_audit!(audit::github_env::GitHubEnv);
         register_audit!(audit::cache_poisoning::CachePoisoning);
diff --git a/test-workflow.yml b/test-workflow.yml
new file mode 100644
index 000000000..d4f0286ab
--- /dev/null
+++ b/test-workflow.yml
@@ -0,0 +1,8 @@
+name: Test Workflow
+on: push
+
+jobs:
+  test-job:
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo "Hello World"
\ No newline at end of file