feat: add a policy job_timeout_minutes_is_required (#488)

* feat: add a policy `job_timeout_minutes_is_required`

- #485

* docs: add a policy

.github/workflows/test.yaml

@@ -17,6 +17,7 @@ jobs:
       renovate-config-validator: ${{steps.changes.outputs.renovate-config-validator}}
     runs-on: ubuntu-latest
     permissions: {}
+    timeout-minutes: 10
     steps:
       - uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
         id: changes
@@ -69,6 +70,7 @@ jobs:
     # This job is used for main branch's branch protection rule's status check.
     # If all dependent jobs succeed or are skipped this job succeeds.
     runs-on: ubuntu-latest
+    timeout-minutes: 10
     needs:
       - update-aqua-checksums
       - test
@@ -91,6 +93,7 @@ jobs:
   ghalint:
     runs-on: ubuntu-latest
     permissions: {}
+    timeout-minutes: 20
     steps:
       - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
       - run: go run ./cmd/ghalint run

.github/workflows/wc-enable-auto-merge.yaml

@@ -12,6 +12,7 @@ jobs:
     # Enable automerge to merge pull requests from Renovate automatically.
     runs-on: ubuntu-latest
     permissions: {}
+    timeout-minutes: 15
     steps:
       - uses: suzuki-shunsuke/enable-auto-merge-action@ec074392e76cd1062925255cd82a86ea1c44b6fd # v0.1.0
         with:

README.md

@@ -35,6 +35,7 @@ We've ported ghalint to [the lintnet module](https://github.com/lintnet-modules/
 1. [action_ref_should_be_full_length_commit_sha](docs/policies/008.md): action's ref should be full length commit SHA
 1. [github_app_should_limit_repositories](docs/policies/009.md): GitHub Actions issueing GitHub Access tokens from GitHub Apps should limit repositories
 1. [github_app_should_limit_permissions](docs/policies/010.md): GitHub Actions issueing GitHub Access tokens from GitHub Apps should limit permissions
+1. [job_timeout_minutes_is_required](docs/policies/012.md): All jobs should set [timeout-minutes](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idtimeout-minutes)
 
 ### 2. Action Policies
 

docs/policies/012.md

@@ -0,0 +1,64 @@
+# job_timeout_minutes_is_required
+
+All jobs should set [timeout-minutes](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idtimeout-minutes).
+
+## Examples
+
+:x:
+
+```yaml
+jobs:
+  foo: # The job doesn't have `timeout-minutes`
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo hello
+```
+
+:o:
+
+```yaml
+jobs:
+  foo:
+    runs-on: ubuntu-latest
+    timeout-minutes: 30
+    steps:
+      - run: echo hello
+```
+
+## Why?
+
+https://exercism.org/docs/building/github/gha-best-practices#h-set-timeouts-for-workflows
+
+> By default, GitHub Actions kills workflows after 6 hours if they have not finished by then. Many workflows don't need nearly as much time to finish, but sometimes unexpected errors occur or a job hangs until the workflow run is killed 6 hours after starting it. Therefore it's recommended to specify a shorter timeout.
+> 
+> The ideal timeout depends on the individual workflow but 30 minutes is typically more than enough for the workflows used in Exercism repos.
+> 
+> This has the following advantages:
+> 
+> PRs won't be pending CI for half the day, issues can be caught early or workflow runs can be restarted.
+> The number of overall parallel builds is limited, hanging jobs will not cause issues for other PRs if they are cancelled early.
+
+## Exceptions
+
+1. All steps set `timeout-minutes`
+
+```yaml
+jobs:
+  foo: # The job is missing `timeout-minutes`, but it's okay because all steps set timeout-minutes
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo hello
+        timeout-minutes: 5
+      - run: echo bar
+        timeout-minutes: 5
+```
+
+2. A job uses a reusable workflow
+
+When a reusable workflow is called with `uses`, `timeout-minutes` is not available.
+
+```yaml
+jobs:
+  foo:
+    uses: suzuki-shunsuke/renovate-config-validator-workflow/.github/workflows/validate.yaml@v0.2.3
+```

pkg/controller/run.go

@@ -27,6 +27,7 @@ func (c *Controller) Run(_ context.Context, logE *logrus.Entry, cfgFilePath stri
 	}
 	jobPolicies := []JobPolicy{
 		&policy.JobPermissionsPolicy{},
+		&policy.JobTimeoutMinutesIsRequiredPolicy{},
 		policy.NewJobSecretsPolicy(),
 		&policy.DenyInheritSecretsPolicy{},
 		&policy.DenyJobContainerLatestImagePolicy{},

pkg/policy/job_timeout_minutes_is_required.go

@@ -0,0 +1,35 @@
+package policy
+
+import (
+	"errors"
+
+	"github.com/sirupsen/logrus"
+	"github.com/suzuki-shunsuke/ghalint/pkg/config"
+	"github.com/suzuki-shunsuke/ghalint/pkg/workflow"
+)
+
+type JobTimeoutMinutesIsRequiredPolicy struct{}
+
+func (p *JobTimeoutMinutesIsRequiredPolicy) Name() string {
+	return "job_timeout_minutes_is_required"
+}
+
+func (p *JobTimeoutMinutesIsRequiredPolicy) ID() string {
+	return "012"
+}
+
+func (p *JobTimeoutMinutesIsRequiredPolicy) ApplyJob(_ *logrus.Entry, _ *config.Config, _ *JobContext, job *workflow.Job) error {
+	if job.TimeoutMinutes != 0 {
+		return nil
+	}
+	if job.Uses != "" {
+		// when a reusable workflow is called with "uses", "timeout-minutes" is not available.
+		return nil
+	}
+	for _, step := range job.Steps {
+		if step.TimeoutMinutes == 0 {
+			return errors.New("job's timeout-minutes is required")
+		}
+	}
+	return nil
+}

pkg/policy/job_timeout_minutes_is_required_test.go

@@ -0,0 +1,78 @@
+package policy_test
+
+import (
+	"testing"
+
+	"github.com/sirupsen/logrus"
+	"github.com/suzuki-shunsuke/ghalint/pkg/policy"
+	"github.com/suzuki-shunsuke/ghalint/pkg/workflow"
+)
+
+func TestJobTimeoutMinutesIsRequiredPolicy_ApplyJob(t *testing.T) { //nolint:funlen
+	t.Parallel()
+	data := []struct {
+		name  string
+		job   *workflow.Job
+		isErr bool
+	}{
+		{
+			name: "normal",
+			job: &workflow.Job{
+				TimeoutMinutes: 30,
+				Steps: []*workflow.Step{
+					{
+						Run: "echo hello",
+					},
+				},
+			},
+		},
+		{
+			name: "workflow using reusable workflow",
+			job: &workflow.Job{
+				Uses: "suzuki-shunsuke/renovate-config-validator-workflow/.github/workflows/validate.yaml@v0.2.3",
+			},
+		},
+		{
+			name: "job should have timeout-minutes",
+			job: &workflow.Job{
+				Steps: []*workflow.Step{
+					{
+						Run: "echo hello",
+					},
+				},
+			},
+			isErr: true,
+		},
+		{
+			name: "all steps have timeout-minutes",
+			job: &workflow.Job{
+				Steps: []*workflow.Step{
+					{
+						Run:            "echo hello",
+						TimeoutMinutes: 60,
+					},
+					{
+						Run:            "echo hello",
+						TimeoutMinutes: 60,
+					},
+				},
+			},
+		},
+	}
+	p := &policy.JobTimeoutMinutesIsRequiredPolicy{}
+	logE := logrus.NewEntry(logrus.New())
+	for _, d := range data {
+		t.Run(d.name, func(t *testing.T) {
+			t.Parallel()
+			if err := p.ApplyJob(logE, nil, nil, d.job); err != nil {
+				if !d.isErr {
+					t.Fatal(err)
+				}
+				return
+			}
+			if d.isErr {
+				t.Fatal("error must be returned")
+			}
+		})
+	}
+}

pkg/workflow/workflow.go

@@ -8,21 +8,23 @@ type Workflow struct {
 }
 
 type Job struct {
-	Permissions *Permissions
-	Env         map[string]string
-	Steps       []*Step
-	Secrets     *JobSecrets
-	Container   *Container
-	Uses        string
+	Permissions    *Permissions
+	Env            map[string]string
+	Steps          []*Step
+	Secrets        *JobSecrets
+	Container      *Container
+	Uses           string
+	TimeoutMinutes int `yaml:"timeout-minutes"`
 }
 
 type Step struct {
-	Uses  string
-	ID    string
-	Name  string
-	Run   string
-	Shell string
-	With  map[string]string
+	Uses           string
+	ID             string
+	Name           string
+	Run            string
+	Shell          string
+	With           map[string]string
+	TimeoutMinutes int `yaml:"timeout-minutes"`
 }
 
 type Action struct {