fix(actions): reject jobs without runs-on (#39480)

Align job and `runs-on` validation with github.com, as implemented by
the parser in https://github.com/actions/runner. A job without `runs-on`
could be claimed by any runner, so a job meant for a container could run
on the host.

- Jobs without `runs-on` fail with `Required property is missing:
runs-on`, called workflows included
- Unknown job keys and callers (`uses:`) mixed with steps-only keys like
`runs-on` are rejected
- Empty, null and nested `runs-on` values are rejected
- A `runs-on` evaluating to such a value fails only that job
- Called workflows are validated at run creation, an invalid one fails
the run as an invalid workflow file
- Zero labels (`runs-on: []` or `{}`) never match a runner, including
jobs queued before upgrading

<img width="960" alt="image"
src="https://github.com/user-attachments/assets/e746ce5a-b711-4e8b-aab8-81336ff53d86"
/>

**Behavior Change:** workflows that omit `runs-on`, use unknown job keys
or mix `uses` with `runs-on` stop running until fixed.

---------

Co-authored-by: bircni <bircni@icloud.com>
Co-authored-by: silverwind <me@silverwind.io>

services/actions/approve.go

@@ -15,6 +15,7 @@ import (
 	user_model "gitea.dev/models/user"
 	"gitea.dev/modules/container"
 	"gitea.dev/modules/log"
+	"gitea.dev/modules/timeutil"
 	"gitea.dev/modules/util"
 
 	"xorm.io/builder"
@@ -99,6 +100,20 @@ func ApproveRuns(ctx context.Context, repo *repo_model.Repository, doer *user_mo
 				if !slots.available(job) {
 					continue
 				}
+				if invalid := invalidRunsOn(job); invalid != nil {
+					job.Status, job.Stopped = actions_model.StatusFailure, timeutil.TimeStampNow()
+					n, err := actions_model.UpdateRunJob(ctx, job, nil, "status", "stopped")
+					if err != nil {
+						return err
+					}
+					if n > 0 {
+						updatedJobs = append(updatedJobs, job)
+					}
+					if err := upsertJobErrorSummary(ctx, job, "runs-on", invalid); err != nil {
+						return err
+					}
+					continue
+				}
 				var jobsToCancel []*actions_model.ActionRunJob
 				job.Status, jobsToCancel, err = PrepareToStartJobWithConcurrency(ctx, job)
 				if err != nil {

services/actions/context_test.go

@@ -98,7 +98,7 @@ jobs:
 	assert.NotEmpty(t, persisted.RawConcurrency)
 }
 
-func TestPrepareRunAndInsert_JobIf(t *testing.T) {
+func TestPrepareRunAndInsert_JobIfAndRunsOn(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	defer test.MockVariableValue(&EmitJobsIfReadyByRun, func(int64) error { return nil })()
 
@@ -123,6 +123,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - run: echo
+  unset-runs-on:
+    runs-on: ${{ vars.UNSET }}
+    steps:
+      - run: echo
 `, false)
 
 	jobs := map[string]*actions_model.ActionRunJob{}
@@ -134,9 +138,12 @@ jobs:
 	assert.False(t, jobs["skip"].IsConcurrencyEvaluated)
 	assert.Equal(t, actions_model.StatusSkipped, jobs["skip-caller"].Status)
 	assert.Equal(t, actions_model.StatusSkipped, jobs["invalid"].Status)
-	summary, err := actions_model.GetActionRunJobSummary(t.Context(), run.RepoID, run.ID, run.LatestAttemptID, jobs["invalid"].ID, 0)
-	require.NoError(t, err)
-	assert.Contains(t, summary.Content, "Error when evaluating `if` for job `invalid`")
+	assert.Equal(t, actions_model.StatusFailure, jobs["unset-runs-on"].Status)
+	for id, key := range map[string]string{"invalid": "if", "unset-runs-on": "runs-on"} {
+		summary, err := actions_model.GetActionRunJobSummary(t.Context(), run.RepoID, run.ID, run.LatestAttemptID, jobs[id].ID, 0)
+		require.NoError(t, err)
+		assert.Contains(t, summary.Content, "Error when evaluating `"+key+"` for job `"+id+"`")
+	}
 }
 
 func TestComputeReusableCallerOutputs(t *testing.T) {

services/actions/helper.go

@@ -183,6 +183,18 @@ func upsertJobErrorSummary(ctx context.Context, job *actions_model.ActionRunJob,
 	return actions_model.UpsertActionRunJobSummary(ctx, job.RepoID, job.RunID, job.RunAttemptID, job.ID, 0, actions_model.JobSummaryContentTypeMarkdown, []byte(content))
 }
 
+// invalidRunsOn returns github.com's error for the job's evaluated runs-on.
+func invalidRunsOn(job *actions_model.ActionRunJob) error {
+	parsed, err := job.ParseJob()
+	if err != nil {
+		return err
+	}
+	if problem := parsed.RunsOnProblem(); problem != "" {
+		return errors.New(problem)
+	}
+	return nil
+}
+
 func findJobNeedsAndFillJobResults(ctx context.Context, job *actions_model.ActionRunJob) (map[string]*jobparser.JobResult, error) {
 	taskNeeds, jobsByID, err := FindTaskNeeds(ctx, job)
 	if err != nil {

services/actions/invalid_workflow.go

@@ -32,40 +32,46 @@ func handleInvalidWorkflows(ctx context.Context, input *notifyInput, ref git.Ref
 		if actionsConfig.IsWorkflowDisabled(entryName) {
 			continue
 		}
-		now := timeutil.TimeStampNow()
-		run := &actions_model.ActionRun{
-			Title: util.EllipsisDisplayString(commit.MessageTitle(), 255), RepoID: input.Repo.ID, Repo: input.Repo, OwnerID: input.Repo.OwnerID,
+		insertInvalidWorkflowRun(ctx, &actions_model.ActionRun{
+			Title: commit.MessageTitle(), RepoID: input.Repo.ID, Repo: input.Repo, OwnerID: input.Repo.OwnerID,
 			WorkflowID: entryName, TriggerUserID: input.Doer.ID, TriggerUser: input.Doer, Ref: ref.String(),
 			CommitSHA: commit.ID.String(), Event: input.Event, TriggerEvent: string(input.Event), EventPayload: string(payload),
-			WorkflowRepoID: input.Repo.ID, WorkflowCommitSHA: commit.ID.String(), Status: actions_model.StatusFailure, Started: now, Stopped: now,
-		}
-		if err := db.WithTx(ctx, func(ctx context.Context) error {
-			if run.Index, err = db.GetNextResourceIndex(ctx, "action_run_index", run.RepoID); err != nil {
-				return err
-			}
-			if err := db.Insert(ctx, run); err != nil {
-				return err
-			}
-			attempt := &actions_model.ActionRunAttempt{RepoID: run.RepoID, RunID: run.ID, Attempt: 1, TriggerUserID: run.TriggerUserID, Status: run.Status, Started: now, Stopped: now}
-			if err := db.Insert(ctx, attempt); err != nil {
-				return err
-			}
-			run.LatestAttemptID = attempt.ID
-			if err := actions_model.UpdateRun(ctx, run, "latest_attempt_id"); err != nil {
-				return err
-			}
-			content := fmt.Sprintf("**Invalid workflow file: %s**\n\n```\n%v\n```\n", entryName, parseErr)
-			return db.Insert(ctx, &actions_model.ActionRunJobSummary{
-				RepoID: run.RepoID, RunID: run.ID, RunAttemptID: attempt.ID, Content: content, ContentSize: int64(len(content)), ContentType: actions_model.JobSummaryContentTypeMarkdown,
-			})
-		}); err != nil {
-			log.Error("insert run for invalid workflow %q: %v", entryName, err)
-			continue
-		}
-		if err := createWorkflowCommitStatus(ctx, run.Repo, run.CommitSHA, entryName+" ("+run.TriggerEvent+")", run.WorkflowID,
-			commitstatus.CommitStatusFailure, run.Link(), "Invalid workflow file", false); err != nil {
-			log.Error("create commit status for invalid workflow %q: %v", entryName, err)
-		}
-		NotifyWorkflowRunStatusUpdate(ctx, run)
+			WorkflowRepoID: input.Repo.ID, WorkflowCommitSHA: commit.ID.String(),
+		}, parseErr)
 	}
 }
+
+// insertInvalidWorkflowRun records run as failed with parseErr as its summary.
+func insertInvalidWorkflowRun(ctx context.Context, run *actions_model.ActionRun, parseErr error) {
+	now := timeutil.TimeStampNow()
+	run.Title = util.EllipsisDisplayString(run.Title, 255)
+	run.Status, run.Started, run.Stopped = actions_model.StatusFailure, now, now
+	if err := db.WithTx(ctx, func(ctx context.Context) (err error) {
+		if run.Index, err = db.GetNextResourceIndex(ctx, "action_run_index", run.RepoID); err != nil {
+			return err
+		}
+		if err := db.Insert(ctx, run); err != nil {
+			return err
+		}
+		attempt := &actions_model.ActionRunAttempt{RepoID: run.RepoID, RunID: run.ID, Attempt: 1, TriggerUserID: run.TriggerUserID, Status: run.Status, Started: now, Stopped: now}
+		if err := db.Insert(ctx, attempt); err != nil {
+			return err
+		}
+		run.LatestAttemptID = attempt.ID
+		if err := actions_model.UpdateRun(ctx, run, "latest_attempt_id"); err != nil {
+			return err
+		}
+		content := fmt.Sprintf("**Invalid workflow file: %s**\n\n```\n%v\n```\n", run.WorkflowID, parseErr)
+		return db.Insert(ctx, &actions_model.ActionRunJobSummary{
+			RepoID: run.RepoID, RunID: run.ID, RunAttemptID: attempt.ID, Content: content, ContentSize: int64(len(content)), ContentType: actions_model.JobSummaryContentTypeMarkdown,
+		})
+	}); err != nil {
+		log.Error("insert run for invalid workflow %q: %v", run.WorkflowID, err)
+		return
+	}
+	if err := createWorkflowCommitStatus(ctx, run.Repo, run.CommitSHA, run.WorkflowID+" ("+run.TriggerEvent+")", run.WorkflowID,
+		commitstatus.CommitStatusFailure, run.Link(), "Invalid workflow file", false); err != nil {
+		log.Error("create commit status for invalid workflow %q: %v", run.WorkflowID, err)
+	}
+	NotifyWorkflowRunStatusUpdate(ctx, run)
+}

services/actions/job_emitter.go

@@ -590,6 +590,14 @@ func (r *jobStatusResolver) resolve(ctx context.Context) (map[int64]actions_mode
 			continue
 		}
 
+		if err := invalidRunsOn(actionRunJob); err != nil {
+			if err := upsertJobErrorSummary(ctx, actionRunJob, "runs-on", err); err != nil {
+				return nil, err
+			}
+			ret[id] = actions_model.StatusFailure
+			continue
+		}
+
 		// update concurrency and check whether the job can run now
 		if err := updateConcurrencyEvaluationForJobWithNeeds(ctx, actionRunJob, r.vars); errors.Is(err, util.ErrInvalidArgument) {
 			if err := upsertJobErrorSummary(ctx, actionRunJob, "concurrency", err); err != nil {

services/actions/job_emitter_test.go

@@ -173,6 +173,15 @@ jobs:
 			want: map[int64]actions_model.Status{2: actions_model.StatusFailure},
 			note: "Error when evaluating `concurrency` for job `job2`.",
 		},
+		{
+			name: "invalid evaluated `runs-on` fails the job with an annotation",
+			jobs: actions_model.ActionJobList{
+				{ID: 1, RepoID: 1, JobID: "job1", Status: actions_model.StatusSuccess},
+				{ID: 2, RepoID: 1, JobID: "job2", Status: actions_model.StatusBlocked, Needs: []string{"job1"}, WorkflowPayload: []byte("jobs: {job2: {runs-on: ''}}")},
+			},
+			want: map[int64]actions_model.Status{2: actions_model.StatusFailure},
+			note: "Error when evaluating `runs-on` for job `job2`.",
+		},
 		{
 			name: "max-parallel: a freed slot promotes the lowest blocked job id",
 			jobs: actions_model.ActionJobList{

services/actions/notifier_helper.go

@@ -394,6 +394,14 @@ func buildApproveAndInsertRun(
 		IsScopedRun:       isScopedRun,
 	}
 
+	if err := validateCalledWorkflows(ctx, run, dwf.Content); err != nil {
+		if isScopedRun {
+			return err
+		}
+		insertInvalidWorkflowRun(ctx, run, err)
+		return nil
+	}
+
 	approvalUsers, err := getApprovalUsers(ctx, input, isForkPullRequest)
 	if err != nil {
 		return err

services/actions/reusable_workflow.go

@@ -7,6 +7,8 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"maps"
+	"slices"
 	"strings"
 
 	"gitea.dev/actionslib/pkg/model"
@@ -97,6 +99,50 @@ func loadReusableWorkflowSource(ctx context.Context, run *actions_model.ActionRu
 	}
 }
 
+// validateCalledWorkflows validates all workflows content calls, recursively.
+func validateCalledWorkflows(ctx context.Context, run *actions_model.ActionRun, content []byte) error {
+	validated := make(container.Set[string])
+	var validate func(content []byte, source *actions_model.ActionRunJob, level int) error
+	validate = func(content []byte, source *actions_model.ActionRunJob, level int) error {
+		workflow, err := jobparser.ReadWorkflow(content)
+		if err != nil {
+			return err
+		}
+		for _, id := range slices.Sorted(maps.Keys(workflow.Jobs)) {
+			uses := workflow.Jobs[id].Uses
+			if uses == "" {
+				continue
+			}
+			if level > MaxReusableCallLevels {
+				return errCallLevelExceeded(uses)
+			}
+			if !validated.Add(fmt.Sprintf("%d@%s:%s", source.WorkflowSourceRepoID, source.WorkflowSourceCommitSHA, uses)) {
+				continue
+			}
+			ref, err := ResolveUses(ctx, uses)
+			if err != nil {
+				return fmt.Errorf("job %s: %w", id, err)
+			}
+			called, repoID, commitSHA, err := loadReusableWorkflowSource(ctx, run, source, ref)
+			if err != nil {
+				return fmt.Errorf("job %s: %w", id, err)
+			}
+			if _, err = jobparser.ValidateWorkflowStatic(called); err == nil {
+				err = validate(called, &actions_model.ActionRunJob{WorkflowSourceRepoID: repoID, WorkflowSourceCommitSHA: commitSHA}, level+1)
+			}
+			if err != nil {
+				return fmt.Errorf("job %s: Error from called workflow %s: %w", id, uses, err)
+			}
+		}
+		return nil
+	}
+	return validate(content, &actions_model.ActionRunJob{WorkflowSourceRepoID: run.WorkflowRepoID, WorkflowSourceCommitSHA: run.WorkflowCommitSHA}, 0)
+}
+
+func errCallLevelExceeded(uses string) error {
+	return fmt.Errorf("reusable workflow call exceeds the maximum nesting level of %d at %q", MaxReusableCallLevels, uses)
+}
+
 // resolveSameRepoWorkflowSourceCommit returns the commit to read a same-repo reusable workflow from.
 // pull_request_target runs must resolve local `uses:` at the PR base commit, not a stored head SHA.
 func resolveSameRepoWorkflowSourceCommit(run *actions_model.ActionRun, caller *actions_model.ActionRunJob) string {
@@ -149,7 +195,7 @@ func checkCallerChain(ctx context.Context, caller *actions_model.ActionRunJob) e
 		current = next
 		depth++
 		if depth > MaxReusableCallLevels {
-			return fmt.Errorf("reusable workflow call exceeds the maximum nesting level of %d at %q", MaxReusableCallLevels, caller.CallUses)
+			return errCallLevelExceeded(caller.CallUses)
 		}
 		if current.IsReusableCaller && current.CallUses != "" && !visited.Add(canonicalCallUses(current)) {
 			return fmt.Errorf("reusable workflow call cycle detected: %q", current.CallUses)
@@ -225,6 +271,9 @@ func expandReusableWorkflowCaller(ctx context.Context, run *actions_model.Action
 	if err := checkResolvedCallerCycle(ctx, caller, contentSourceRepoID, contentSourceCommitSHA, ref.Path); err != nil {
 		return err
 	}
+	if _, err := jobparser.ValidateWorkflowStatic(content); err != nil {
+		return fmt.Errorf("invalid called workflow: %w", err)
+	}
 
 	// 4. Parse the called workflow's spec (used by both secret validation and input evaluation).
 	wcSpec, err := jobparser.ParseWorkflowCallConfig(content)

services/actions/run.go

@@ -5,6 +5,7 @@ package actions
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	act_model "gitea.dev/actionslib/pkg/model"
@@ -12,6 +13,7 @@ import (
 	"gitea.dev/models/db"
 	"gitea.dev/modules/actions/jobparser"
 	"gitea.dev/modules/log"
+	"gitea.dev/modules/timeutil"
 	"gitea.dev/modules/util"
 
 	"go.yaml.in/yaml/v4"
@@ -185,6 +187,7 @@ func insertRunJob(ctx context.Context, run *actions_model.ActionRun, runAttempt
 	id, job := workflowJob.Job()
 	needs := job.Needs()
 	isMatrixDeferred := jobparser.HasDeferredMatrix(job)
+	runsOnProblem := job.RunsOnProblem() // SetJob's encoding drops the node's null tag
 	if err := workflowJob.SetJob(id, job.EraseNeeds()); err != nil {
 		return nil, nil, false, err
 	}
@@ -238,10 +241,15 @@ func insertRunJob(ctx context.Context, run *actions_model.ActionRun, runAttempt
 	}
 
 	// a skipped job must neither cancel its group peers nor take a slot
-	invalidIf, err := decideJobIf(ctx, run, runAttempt, runJob, vars)
+	invalidErr, err := decideJobIf(ctx, run, runAttempt, runJob, vars)
 	if err != nil {
 		return nil, nil, false, fmt.Errorf("evaluate job if: %w", err)
 	}
+	invalidKey := "if"
+	if runsOnProblem != "" && runJob.Status.IsWaiting() && slots.available(runJob) {
+		invalidKey, invalidErr = "runs-on", errors.New(runsOnProblem)
+		runJob.Status, runJob.Stopped = actions_model.StatusFailure, timeutil.TimeStampNow()
+	}
 
 	var cancelledConcurrencyJobs []*actions_model.ActionRunJob
 	// check job concurrency
@@ -275,8 +283,8 @@ func insertRunJob(ctx context.Context, run *actions_model.ActionRun, runAttempt
 	if err := db.Insert(ctx, runJob); err != nil {
 		return nil, nil, false, err
 	}
-	if invalidIf != nil {
-		if err := upsertJobErrorSummary(ctx, runJob, "if", invalidIf); err != nil {
+	if invalidErr != nil {
+		if err := upsertJobErrorSummary(ctx, runJob, invalidKey, invalidErr); err != nil {
 			return nil, nil, false, err
 		}
 	}
@@ -287,8 +295,8 @@ func insertRunJob(ctx context.Context, run *actions_model.ActionRun, runAttempt
 		}
 	}
 
-	// the emitter resolves an expanded caller's children and a skipped job's dependents
-	return runJob, cancelledConcurrencyJobs, runJob.IsExpanded || runJob.Status == actions_model.StatusSkipped, nil
+	// the emitter resolves an expanded caller's children and a skipped or failed job's dependents
+	return runJob, cancelledConcurrencyJobs, runJob.IsExpanded || runJob.Status.In(actions_model.StatusSkipped, actions_model.StatusFailure), nil
 }
 
 func expandInlineReusableCaller(ctx context.Context, run *actions_model.ActionRun, runAttempt *actions_model.ActionRunAttempt, caller *actions_model.ActionRunJob, vars map[string]string) error {

services/actions/schedule_tasks.go

@@ -17,6 +17,7 @@ import (
 	repo_model "gitea.dev/models/repo"
 	"gitea.dev/models/unit"
 	user_model "gitea.dev/models/user"
+	"gitea.dev/modules/actions/jobparser"
 	"gitea.dev/modules/json"
 	"gitea.dev/modules/log"
 	"gitea.dev/modules/timeutil"
@@ -144,6 +145,14 @@ func CreateScheduleTaskBySpec(ctx context.Context, spec *actions_model.ActionSch
 		WorkflowCommitSHA: cron.CommitSHA,
 	}
 
+	_, err := jobparser.ValidateWorkflowStatic(cron.Content)
+	if err == nil {
+		err = validateCalledWorkflows(ctx, run, cron.Content)
+	}
+	if err != nil {
+		return fmt.Errorf("invalid workflow: %w", err)
+	}
+
 	// FIXME cron.Content might be outdated if the workflow file has been changed.
 	// Load the latest sha from default branch
 	// Insert the action run and its associated jobs into the database

services/actions/schedule_tasks_test.go

@@ -103,7 +103,7 @@ func TestStartTasks(t *testing.T) {
 	}
 
 	due := timeutil.TimeStamp(time.Now().Add(-time.Minute).Unix())
-	validWorkflow := "jobs:\n  job:\n    runs-on: ubuntu-latest\n    steps:\n      - run: true\n"
+	validWorkflow := "on:\n  schedule:\n    - cron: '0 0 * * *'\njobs:\n  job:\n    runs-on: ubuntu-latest\n    steps:\n      - run: true\n"
 
 	// specs are processed by ascending id, so the broken one runs first and used to abort the whole pass
 	broken := insertSchedule(1, 2, "broken.yml", "@every 1m", "this: [is: not: a: workflow", due)

services/actions/workflow.go

@@ -140,7 +140,10 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re
 		return 0, err
 	}
 
-	if _, err := jobparser.ValidateWorkflowStatic(content); err != nil {
+	if _, err = jobparser.ValidateWorkflowStatic(content); err == nil {
+		err = validateCalledWorkflows(ctx, run, content)
+	}
+	if err != nil {
 		return 0, util.ErrorWrapTranslatable(util.NewInvalidArgumentErrorf("invalid workflow %q: %v", workflowID, err), "actions.runs.invalid_workflow_helper", err.Error())
 	}
 	workflow, err := jobparser.ReadWorkflow(content)