fix: reject workflows with jobs lacking runs-on (#1271)

Fail the workflow before any of its jobs run when a regular job omits `runs-on` or leaves it empty, as GitHub does, instead of running the job in the fallback image. Gitea rejects such workflows itself since https://github.com/go-gitea/gitea/pull/39480. This covers released Gitea versions, which hand a job without labels to any runner, and `exec`. Reusable workflow jobs and the stepless placeholders the runner builds for a job's `needs` are exempt.

Closes https://gitea.com/gitea/runner/issues/836

---------

Co-authored-by: silverwind <me@silverwind.io>
Reviewed-on: https://gitea.com/gitea/runner/pulls/1271
Reviewed-by: silverwind <2021+silverwind@noreply.gitea.com>

README.md

@@ -205,7 +205,7 @@ a workflow with `runs-on: ubuntu-latest` is executed in the `runner-images:ubunt
 
 Names may themselves contain a colon, for example `pool:e57e18d4-10d4-406f-93bf-60f127221bdd`. Only `host`, `docker` and `kubernetes` are treated as schemas.
 
-If a job's `runs-on` matches none of the runner's labels, or sets no `runs-on` at all, it still runs in `runner.default_image`: as a container when the runner has `docker` labels, as a pod when it has `kubernetes` labels, else as a container where docker is available and on the host where it is not. Images maintained for this purpose are listed at [gitea/runner-images](https://gitea.com/gitea/runner-images).
+Jobs run on `runner.default_image` when no runner label matches their `runs-on`: as a pod on runners with `kubernetes` labels, otherwise in a container, or on the host when docker is unavailable. Suitable images are listed at [gitea/runner-images](https://gitea.com/gitea/runner-images).
 
 Labels are chosen at registration time (`--labels`, or the interactive prompt) and can be changed afterwards by editing `runner.labels` in the config file, or in the Gitea UI under the runner's settings.
 

act/runner/run_context.go

@@ -1236,10 +1236,6 @@ func (rc *RunContext) Executor() (common.Executor, error) {
 }
 
 func (rc *RunContext) runsOnImage(ctx context.Context) (string, error) {
-	if rc.Run.Job().RunsOn() == nil && rc.containerSpec.Image == "" {
-		common.Logger(ctx).Errorf("'runs-on' key not defined in %s", rc.String())
-	}
-
 	runsOn := rc.Run.Job().RunsOn()
 	for i, v := range runsOn {
 		var err error
@@ -1708,9 +1704,7 @@ func parentDir(p string) string {
 // still names a release when the label is a rolling one such as ubuntu-latest.
 func (rc *RunContext) imageOS(ctx context.Context) string {
 	if rc.Run.Job().RunsOn() == nil {
-		// A composite action runs on a synthetic job, and resolving its image would only
-		// log that runs-on is missing.
-		return ""
+		return "" // a composite action runs on a synthetic job without runs-on
 	}
 	if imageOS := imageOSFromImage(rc.platformImage); imageOS != "" {
 		return imageOS

act/runner/runner.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"os"
 	"runtime"
+	"slices"
 	"sync"
 	"time"
 
@@ -173,6 +174,15 @@ func maxParallelFor(strategy *model.Strategy, combinations int) int {
 
 // NewPlanExecutor ...
 func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor {
+	for _, stage := range plan.Stages {
+		for _, run := range stage.Runs {
+			job := run.Job()
+			if runsOn := job.RunsOn(); job.Uses == "" && len(job.Steps) > 0 && (len(runsOn) == 0 || slices.Contains(runsOn, "")) { // Gitea's needs placeholders carry no steps
+				return common.NewErrorExecutor(fmt.Errorf("job %q requires a non-empty 'runs-on'", run.JobID))
+			}
+		}
+	}
+
 	maxJobNameLen := 0
 
 	stagePipeline := make([]common.Executor, 0)

act/runner/runner_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/joho/godotenv"
 	log "github.com/sirupsen/logrus"
 	assert "github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 var (
@@ -151,6 +152,21 @@ func TestGraphWithSomeMissing(t *testing.T) {
 	log.SetOutput(out)
 }
 
+func TestNewPlanExecutorRequiresRunsOnBeforeRunningAnyJob(t *testing.T) {
+	for _, runsOn := range []string{"", "if: false", "runs-on: [ubuntu-latest, '']"} {
+		workflow, err := model.ReadWorkflow(strings.NewReader("jobs:\n  first:\n    runs-on: ubuntu-latest\n    steps: [run: echo]\n  test:\n    needs: first\n    " + runsOn + "\n    steps: [run: echo]\n"))
+		require.NoError(t, err)
+		plan, err := model.CombineWorkflowPlanner(workflow).PlanJob("test")
+		require.NoError(t, err)
+		require.ErrorContains(t, (&runnerImpl{config: &Config{}}).NewPlanExecutor(plan)(t.Context()), `job "test" requires a non-empty 'runs-on'`, runsOn)
+		assert.Empty(t, workflow.Jobs["first"].Result, runsOn)
+	}
+
+	workflow := &model.Workflow{Jobs: map[string]*model.Job{"build": {Result: "success"}}}
+	require.NoError(t, (&runnerImpl{config: &Config{}}).NewPlanExecutor(&model.Plan{Stages: []*model.Stage{{Runs: []*model.Run{{Workflow: workflow, JobID: "build"}}}}})(t.Context()))
+	assert.Equal(t, "success", workflow.Jobs["build"].Result)
+}
+
 func TestGraphEvent(t *testing.T) {
 	planner, err := model.NewWorkflowPlanner("testdata/basic", true)
 	assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act