diff --git a/models/actions/runner.go b/models/actions/runner.go index 02db85015b9..4a9a1ac0b50 100644 --- a/models/actions/runner.go +++ b/models/actions/runner.go @@ -200,7 +200,7 @@ func (r *ActionRunner) GenerateAndFillToken() { // CanMatchLabels checks whether the runner's labels can match a job's "runs-on" // See https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#jobsjob_idruns-on func (r *ActionRunner) CanMatchLabels(jobRunsOn []string) bool { - return !slices.ContainsFunc(jobRunsOn, func(label string) bool { return !util.SliceContainsString(r.AgentLabels, label, true) }) + return len(jobRunsOn) > 0 && !slices.ContainsFunc(jobRunsOn, func(label string) bool { return !util.SliceContainsString(r.AgentLabels, label, true) }) } func init() { diff --git a/models/actions/runner_test.go b/models/actions/runner_test.go index 77937db173f..3b5d0ea05ea 100644 --- a/models/actions/runner_test.go +++ b/models/actions/runner_test.go @@ -86,4 +86,5 @@ func TestCanMatchLabelsCaseInsensitive(t *testing.T) { runner := &ActionRunner{AgentLabels: []string{"self-hosted", "Linux", "X64"}} assert.True(t, runner.CanMatchLabels([]string{"SELF-HOSTED", "linux"})) assert.False(t, runner.CanMatchLabels([]string{"linux", "arm64"})) + assert.False(t, runner.CanMatchLabels(nil)) } diff --git a/modules/actions/jobparser/jobparser.go b/modules/actions/jobparser/jobparser.go index 912b1278548..4230903143d 100644 --- a/modules/actions/jobparser/jobparser.go +++ b/modules/actions/jobparser/jobparser.go @@ -225,7 +225,6 @@ func replaceScalars(node *yaml.Node, replace func(string) string) { // buildMatrixCombos builds one Job per matrix combination from src, baking the combination into the // strategy and interpolating the name, runs-on and continue-on-error with it. func buildMatrixCombos(jobID string, src *Job, matrixes []map[string]any, gitCtx *model.GithubContext, results map[string]*JobResult, vars map[string]string, inputs map[string]any) ([]*Job, error) { - srcRunsOn := model.RunsOnFromNode(src.RawRunsOn) order, names := make([]int, len(matrixes)), make([]string, len(matrixes)) for index, matrix := range matrixes { order[index], names[index] = index, matrixName(matrix) @@ -259,14 +258,15 @@ func buildMatrixCombos(jobID string, src *Job, matrixes []map[string]any, gitCtx if err := evaluator.EvaluateYamlNode(&rawRunsOn); err != nil { return nil, fmt.Errorf("interpolate runs-on for job %q: %w", jobID, err) } - runsOn := model.RunsOnFromNode(rawRunsOn) - if len(runsOn) == 0 && len(srcRunsOn) > 0 { // match no runner rather than every runner - runsOn = []string{""} + if rawRunsOn.Kind != 0 && runsOnProblem(&rawRunsOn) != "" { + combo.RawRunsOn = rawRunsOn + } else { + runsOn := model.RunsOnFromNode(rawRunsOn) + for i := range runsOn { + runsOn[i] = escapeExpressions(runsOn[i]) + } + combo.RawRunsOn = model.RunsOnNode(runsOn, "") } - for i := range runsOn { - runsOn[i] = escapeExpressions(runsOn[i]) - } - combo.RawRunsOn = model.RunsOnNode(runsOn, "") } if err := evaluator.EvaluateYamlNode(&combo.RawContinueOnError); err != nil { return nil, fmt.Errorf("evaluate continue-on-error for job %q: %w", jobID, err) diff --git a/modules/actions/jobparser/jobparser_test.go b/modules/actions/jobparser/jobparser_test.go index 3da5d2f45a0..4606e9eb294 100644 --- a/modules/actions/jobparser/jobparser_test.go +++ b/modules/actions/jobparser/jobparser_test.go @@ -290,17 +290,26 @@ func TestParseInterpolatesRunName(t *testing.T) { assert.Empty(t, result[0].RunName) } -func TestParseRunsOnFromJSONArray(t *testing.T) { +func TestParseRunsOnFromJSONKeepsWhatGitHubRejectsForTheJobToFail(t *testing.T) { content := []byte("on: push\njobs:\n build:\n runs-on: ${{ fromJSON(vars.RUNNER) }}\n steps: [{run: echo}]\n") _, err := Parse(content) require.NoError(t, err) - for runner, want := range map[string][]string{`["self-hosted", "linux"]`: {"self-hosted", "linux"}, "[]": {""}} { + for runner, want := range map[string][]string{`["self-hosted", "linux"]`: {"self-hosted", "linux"}, "[]": {}, "{}": {}} { result, err := Parse(content, WithGitContext(&model.GithubContext{}), WithVars(map[string]string{"RUNNER": runner})) require.NoError(t, err) require.Len(t, result, 1) _, job := result[0].Job() assert.Equal(t, want, job.RunsOn(), runner) } + for runner, problem := range map[string]string{`["a"]`: "", `""`: "Unexpected value ''", `[["a"]]`: "A sequence was not expected"} { + result, err := Parse(content, WithGitContext(&model.GithubContext{}), WithVars(map[string]string{"RUNNER": runner})) + require.NoError(t, err) + payload, err := result[0].Marshal() + require.NoError(t, err) + _, job, err := ParseRawSingleWorkflow(payload) + require.NoError(t, err) + assert.Equal(t, problem, job.RunsOnProblem(), runner) + } } func TestJobFieldsWithoutMatrix(t *testing.T) { @@ -458,6 +467,37 @@ func TestReadWorkflowJobConditionContexts(t *testing.T) { } } +func TestValidateWorkflowStaticJobKindAndRunsOnLikeGitHub(t *testing.T) { + for job, want := range map[string]string{ + "{runs-on: x, steps: [{run: echo}]}": "", + "{uses: o/r/.gitea/workflows/c.yml@main}": "", + "{runs-on: []}": "", + "{runs-on: {}}": "", + "{runs-on: {group: org/g, labels: [a, 1]}}": "", + "{runs-on: {group: '${{ vars.G }}'}}": "", + "{steps: [{run: echo}]}": "Required property is missing: runs-on", + "{with: {}}": "Required property is missing: uses", + "{runs-on: x, uses: o/r/.gitea/workflows/c.yml@main}": "Unexpected value 'uses'", + "{Runs-On: x}": "Unexpected value 'Runs-On'", + "{runs-on: ~}": "runs-on: Unexpected value ''", + "{runs-on: ['']}": "runs-on: Unexpected value ''", + "{runs-on: [[a]]}": "runs-on: A sequence was not expected", + "{runs-on: {labels: {a: b}}}": "runs-on: A mapping was not expected", + "{runs-on: {foo: x}}": "runs-on: Unexpected value 'foo'", + "{runs-on: {group: org/}}": "runs-on: Invalid runs-on group name 'org/'.", + "{runs-on: {group: a/b/c}}": "runs-on: Invalid runs-on group name 'a/b/c'. Please use 'organization/' or 'enterprise/' prefix to target a single runner group.", + "{if: true}": "There's not enough info to determine what you meant. Add one of these properties: " + + "cancel-timeout-minutes, container, continue-on-error, defaults, env, environment, outputs, runs-on, secrets, services, snapshot, steps, timeout-minutes, uses, with", + } { + _, err := ValidateWorkflowStatic([]byte("on: push\njobs:\n build: " + job + "\n")) + if want == "" { + assert.NoError(t, err, job) + } else { + assert.EqualError(t, err, "job build: "+want, job) + } + } +} + func TestRejectsUnevaluatedMatrixFilters(t *testing.T) { for _, filter := range []string{"include", "exclude"} { t.Run(filter, func(t *testing.T) { diff --git a/modules/actions/jobparser/model.go b/modules/actions/jobparser/model.go index a5b7b1d6897..9fbb8599ee7 100644 --- a/modules/actions/jobparser/model.go +++ b/modules/actions/jobparser/model.go @@ -174,6 +174,14 @@ func (j *Job) EraseNeeds() *Job { return j } +// RunsOnProblem returns github.com's error for the job's runs-on, "" if valid. +func (j *Job) RunsOnProblem() string { + if j.RawRunsOn.Kind == 0 { + return "" + } + return runsOnProblem(&j.RawRunsOn) +} + // RunsOn returns the labels Gitea matches runners against, unescaped like DisplayName. func (j *Job) RunsOn() []string { runsOn := model.RunsOnFromNode(j.RawRunsOn) diff --git a/modules/actions/jobparser/validate.go b/modules/actions/jobparser/validate.go index c054d30c40b..7e695992d7d 100644 --- a/modules/actions/jobparser/validate.go +++ b/modules/actions/jobparser/validate.go @@ -7,10 +7,13 @@ import ( "errors" "fmt" "slices" + "strings" "gitea.dev/actionslib/pkg/expreval" "gitea.dev/actionslib/pkg/exprparser" "gitea.dev/actionslib/pkg/model" + + "go.yaml.in/yaml/v4" ) // jobConditionContexts are what github.com gives `jobs..if`, which it decides before the matrix, plus the `gitea` alias. @@ -21,7 +24,7 @@ func ValidateWorkflowStatic(content []byte) ([]*Event, error) { if err != nil { return nil, err } - // Keep unknown and case-distinct keys accepted for existing Gitea workflows. + // Keep unknown and case-distinct keys outside of jobs accepted for existing Gitea workflows. workflow, err := readWorkflowDoc(doc) if err != nil { return nil, err @@ -33,6 +36,9 @@ func ValidateWorkflowStatic(content []byte) ([]*Event, error) { if err := validateWorkflowStructure(workflow); err != nil { return nil, err } + if err := validateJobKinds(doc); err != nil { + return nil, err + } var header struct { RunName string `yaml:"run-name"` } @@ -76,7 +82,6 @@ func validateWorkflowStructure(workflow *model.Workflow) error { if job == nil { return fmt.Errorf("job %q has no configuration", id) } - // a job without runs-on is accepted and runs on any runner, github.com rejects it for _, dependency := range job.Needs() { if _, ok := workflow.Jobs[dependency]; !ok { return fmt.Errorf("job %q needs unknown job %q", id, dependency) @@ -110,3 +115,116 @@ func validateWorkflowStructure(workflow *model.Workflow) error { } return nil } + +// job keys of github.com's workflow schema, by the kind of job allowing them +var ( + stepsJobKeys = []string{"cancel-timeout-minutes", "container", "continue-on-error", "defaults", "env", "environment", "outputs", "runs-on", "services", "snapshot", "steps", "timeout-minutes"} + callerJobKeys = []string{"secrets", "uses", "with"} + sharedJobKeys = []string{"concurrency", "if", "name", "needs", "permissions", "strategy"} +) + +// validateJobKinds applies github.com's job kinds, decided by the first kind-specific key. +func validateJobKinds(doc *yaml.Node) error { + jobs := mappingValue(doc.Content[0], "jobs") + for i := 0; i+1 < len(jobs.Content); i += 2 { + id, job := jobs.Content[i].Value, jobs.Content[i+1] + var required string + for j := 0; j+1 < len(job.Content); j += 2 { + key := job.Content[j].Value + isStepsKey, isCallerKey := slices.Contains(stepsJobKeys, key), slices.Contains(callerJobKeys, key) + switch { + case required == "runs-on" && isCallerKey, required == "uses" && isStepsKey, !isStepsKey && !isCallerKey && !slices.Contains(sharedJobKeys, key): + return fmt.Errorf("job %s: Unexpected value '%s'", id, key) + case required == "" && isStepsKey: + required = "runs-on" + case required == "" && isCallerKey: + required = "uses" + } + } + if required == "" { + keys := slices.Concat(stepsJobKeys, callerJobKeys) + slices.Sort(keys) + return fmt.Errorf("job %s: There's not enough info to determine what you meant. Add one of these properties: %s", id, strings.Join(keys, ", ")) + } + value := mappingValue(job, required) + if value == nil { + return fmt.Errorf("job %s: Required property is missing: %s", id, required) + } + if required != "runs-on" { + continue + } + if problem := runsOnProblem(value); problem != "" { + return fmt.Errorf("job %s: runs-on: %s", id, problem) + } + } + return nil +} + +// runsOnProblem returns github.com's schema error for a runs-on, "" if valid. +func runsOnProblem(node *yaml.Node) string { + if node.Kind != yaml.MappingNode { + return runsOnLabelsProblem(node) + } + for i := 0; i+1 < len(node.Content); i += 2 { + var problem string + switch key := node.Content[i].Value; key { + case "labels": + problem = runsOnLabelsProblem(node.Content[i+1]) + case "group": + problem = runsOnGroupProblem(node.Content[i+1]) + default: + problem = fmt.Sprintf("Unexpected value '%s'", key) + } + if problem != "" { + return problem + } + } + return "" +} + +func runsOnLabelsProblem(node *yaml.Node) string { + if node.Kind != yaml.SequenceNode { + return nonEmptyStringProblem(node) + } + for _, label := range node.Content { + if problem := nonEmptyStringProblem(label); problem != "" { + return problem + } + } + return "" +} + +func runsOnGroupProblem(node *yaml.Node) string { + if problem := nonEmptyStringProblem(node); problem != "" || hasExpression(node.Value) { + return problem + } + switch prefix, name, found := strings.Cut(node.Value, "/"); { + case found && name == "": + return fmt.Sprintf("Invalid runs-on group name '%s'.", node.Value) + case found && (strings.Contains(name, "/") || !slices.Contains([]string{"org", "organization", "ent", "enterprise"}, prefix)): + return fmt.Sprintf("Invalid runs-on group name '%s'. Please use 'organization/' or 'enterprise/' prefix to target a single runner group.", node.Value) + } + return "" +} + +// nonEmptyStringProblem mirrors github.com's non-empty-string, which also accepts non-string scalars. +func nonEmptyStringProblem(node *yaml.Node) string { + switch { + case node.Kind == yaml.SequenceNode: + return "A sequence was not expected" + case node.Kind == yaml.MappingNode: + return "A mapping was not expected" + case node.Value == "" || node.ShortTag() == "!!null": + return "Unexpected value ''" + } + return "" +} + +func mappingValue(node *yaml.Node, key string) *yaml.Node { + for i := 0; i+1 < len(node.Content); i += 2 { + if node.Content[i].Value == key { + return node.Content[i+1] + } + } + return nil +} diff --git a/modules/actions/workflows_test.go b/modules/actions/workflows_test.go index 27a9d6e35bd..486e734900a 100644 --- a/modules/actions/workflows_test.go +++ b/modules/actions/workflows_test.go @@ -30,18 +30,23 @@ jobs: func TestReadWorkflowEventsStaticErrors(t *testing.T) { for content, static := range map[string]bool{ - "on: push\njobs: {}": true, - "on: push\njobs: {test: {needs: absent}}": true, - "on: push\njobs: {one: {needs: two}, two: {needs: one}}": true, - "on: push\njobs: {test: {strategy: {matrix: {os: []}}}}": true, - "on: push\nrun-name: ${{ secrets.TOKEN }}\njobs: {test: {}}": true, - "on: push\nrun-name: ${{ fromJSON(inputs.x) }}\njobs: {test: {steps: [{run: echo}]}}": false, + "on: push\njobs: {}": true, + "on: push\njobs: {test: {runs-on: x, needs: absent}}": true, + "on: push\njobs: {one: {runs-on: x, needs: two}, two: {runs-on: x, needs: one}}": true, + "on: push\njobs: {test: {runs-on: x, strategy: {matrix: {os: []}}}}": true, + "on: push\nrun-name: ${{ secrets.TOKEN }}\njobs: {test: {runs-on: x}}": true, + "on: push\njobs: {test: {steps: [{run: echo}]}}": true, + "on: push\nrun-name: ${{ fromJSON(inputs.x) }}\njobs: {test: {runs-on: x, steps: [{run: echo}]}}": false, } { _, gotStatic, err := readWorkflowEvents([]byte(content)) require.Error(t, err, content) assert.Equal(t, static, gotStatic, content) } - for _, content := range []string{"on: push\njobs: {test: {steps: [{run: echo}]}}", "on: push\nrun-name: ${{ github.ref }}\njobs: {test: {}}"} { + for _, content := range []string{ + "on: push\njobs: {test: {runs-on: x, steps: [{run: echo}]}}", + "on: push\nrun-name: ${{ github.ref }}\njobs: {test: {runs-on: x}}", + "on: push\njobs: {call: {uses: ./.gitea/workflows/called.yml}}", + } { _, _, err := readWorkflowEvents([]byte(content)) assert.NoError(t, err, content) } diff --git a/routers/web/repo/actions/actions.go b/routers/web/repo/actions/actions.go index 4cb4c687ab5..d75ee062e13 100644 --- a/routers/web/repo/actions/actions.go +++ b/routers/web/repo/actions/actions.go @@ -569,7 +569,7 @@ func (data *actionRunListData) processActionRuns(ctx *context.Context) bool { break } } - if job.Status.IsWaiting() { + if job.Status.IsWaiting() && !job.IsReusableCaller { hasOnlineRunner := false for _, runner := range runners { if !runner.IsDisabled && runner.CanMatchLabels(job.RunsOn) { diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 9aff66aba56..03bbdf6a9d8 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -773,7 +773,7 @@ func describePendingJobDetail(ctx *context_module.Context, current *actions_mode if pending := pendingNeeds(current, jobs); len(pending) > 0 { return ctx.Locale.TrString("actions.runs.waiting_for_dependent_jobs", strings.Join(pending, ", ")) } - case current.Status.IsWaiting(): + case current.Status.IsWaiting() && !current.IsReusableCaller: // a caller waits on its called jobs, never on a runner // A waiting job has no runner to pick it up yet. A busy runner is still // "online", so distinguish three cases: no runner online at all, online // runners but none match the labels, and a matching runner that is busy. diff --git a/services/actions/approve.go b/services/actions/approve.go index 327072aa062..6744935a790 100644 --- a/services/actions/approve.go +++ b/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 { diff --git a/services/actions/context_test.go b/services/actions/context_test.go index b16f1247b1f..fa7b456084b 100644 --- a/services/actions/context_test.go +++ b/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) { diff --git a/services/actions/helper.go b/services/actions/helper.go index 195822291a1..75602adb45d 100644 --- a/services/actions/helper.go +++ b/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 { diff --git a/services/actions/invalid_workflow.go b/services/actions/invalid_workflow.go index fc211fbdcbc..56edb0a9776 100644 --- a/services/actions/invalid_workflow.go +++ b/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) +} diff --git a/services/actions/job_emitter.go b/services/actions/job_emitter.go index 0b974b10de8..e152c15c968 100644 --- a/services/actions/job_emitter.go +++ b/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 { diff --git a/services/actions/job_emitter_test.go b/services/actions/job_emitter_test.go index 6c96abab105..4dc7bd040a5 100644 --- a/services/actions/job_emitter_test.go +++ b/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{ diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 6b8ac63814e..e6223c0a6a5 100644 --- a/services/actions/notifier_helper.go +++ b/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 diff --git a/services/actions/reusable_workflow.go b/services/actions/reusable_workflow.go index 1ac89705a56..b606edb3542 100644 --- a/services/actions/reusable_workflow.go +++ b/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) diff --git a/services/actions/run.go b/services/actions/run.go index aa33a7f65ce..4b4011fccec 100644 --- a/services/actions/run.go +++ b/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 { diff --git a/services/actions/schedule_tasks.go b/services/actions/schedule_tasks.go index 0656767c985..52c5da43ede 100644 --- a/services/actions/schedule_tasks.go +++ b/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 diff --git a/services/actions/schedule_tasks_test.go b/services/actions/schedule_tasks_test.go index 1bfb60ff680..9e16aacdc39 100644 --- a/services/actions/schedule_tasks_test.go +++ b/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) diff --git a/services/actions/workflow.go b/services/actions/workflow.go index b02a6dd4381..f29d4e5e64c 100644 --- a/services/actions/workflow.go +++ b/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) diff --git a/tests/integration/actions_invalid_workflow_test.go b/tests/integration/actions_invalid_workflow_test.go index 91898a49306..ae91f4edefe 100644 --- a/tests/integration/actions_invalid_workflow_test.go +++ b/tests/integration/actions_invalid_workflow_test.go @@ -33,7 +33,7 @@ func TestActionsInvalidWorkflowPush(t *testing.T) { content string wantErrors []string }{ - {"expression", "on: push\nrun-name: '${{ github.ref'\njobs: {check: {if: unknown.x}}\n", []string{"Unrecognized named-value: 'unknown'", "unclosed expression"}}, + {"expression", "on: push\nrun-name: '${{ github.ref'\njobs: {check: {runs-on: ubuntu-latest, if: unknown.x}}\n", []string{"Unrecognized named-value: 'unknown'", "unclosed expression"}}, {"trigger", "on:\njobs: {check: {runs-on: ubuntu-latest, steps: [{run: echo hello}]}}\n", []string{"invalid event"}}, } { t.Run(testCase.name, func(t *testing.T) { diff --git a/tests/integration/actions_reusable_workflow_test.go b/tests/integration/actions_reusable_workflow_test.go index 3f4ccf0880f..db13d3b3c3f 100644 --- a/tests/integration/actions_reusable_workflow_test.go +++ b/tests/integration/actions_reusable_workflow_test.go @@ -405,8 +405,8 @@ jobs: from: 'consumer' `) - // Phase 1: no grant. The cross-repo read check fails, and NO ActionRun row gets persisted. - assert.Equal(t, 0, unittest.GetCount(t, &actions_model.ActionRun{RepoID: consumerRepo.ID})) + // Phase 1: no grant. + assertInvalidWorkflowRun(t, consumerRepo.ID, "cross-caller.yaml", "reusable workflow repository user2/reusable-lib-private does not exist or is not readable") runner.fetchNoTask(t) // Phase 2: user2 (libRepo owner) adds user4 (consumer owner) as a Collaborative Owner of libRepo. @@ -418,7 +418,7 @@ jobs: // Phase 3: trigger the workflow again createRepoWorkflowFile(t, user4, user4Token, consumerRepo, "marker.txt", "trigger after grant") - run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: consumerRepo.ID}) + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: consumerRepo.ID, Index: 2}) crossJob := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{RunID: run.ID, JobID: "cross_job"}) assert.True(t, crossJob.IsReusableCaller) assert.True(t, crossJob.IsExpanded) @@ -484,8 +484,7 @@ jobs: uses: user2/reusable-lib-public-denied/.gitea/workflows/reusable_lib.yaml@main `) - // Denied: the cross-repo read check fails for the public caller, so NO ActionRun is persisted and no task is dispatched. - assert.Equal(t, 0, unittest.GetCount(t, &actions_model.ActionRun{RepoID: consumerRepo.ID})) + assertInvalidWorkflowRun(t, consumerRepo.ID, "cross-caller.yaml", "reusable workflow repository user2/reusable-lib-public-denied does not exist or is not readable") runner.fetchNoTask(t) }) @@ -563,35 +562,32 @@ jobs: unittest.AssertNotExistsBean(t, &actions_model.ActionRunJob{RunID: run.ID, JobID: "util_consumer_job"}) }) - t.Run("Missing callee file", func(t *testing.T) { - // A caller workflow references a callee path that does not exist in the repo. - - apiRepo := createActionsTestRepo(t, user2Token, "caller-missing-callee", false) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: apiRepo.ID}) - - createRepoWorkflowFile(t, user2, user2Token, repo, ".gitea/workflows/caller.yaml", - `name: Caller -on: push -jobs: - plain_job: - runs-on: ubuntu-latest - steps: - - run: echo 'job' - call_missing: - uses: ./.gitea/workflows/does-not-exist.yml -`) - - assert.Equal(t, 0, unittest.GetCount(t, &actions_model.ActionRun{RepoID: repo.ID})) + t.Run("Missing or invalid callee fails the run as an invalid workflow file", func(t *testing.T) { + for name, testCase := range map[string]struct{ callee, want string }{ + "missing": {"", "job call: read user2/caller-missing-callee@"}, + "no-runs-on": {"on: workflow_call\njobs:\n inner:\n steps:\n - run: echo\n", "job call: Error from called workflow ./.gitea/workflows/callee.yml: job inner: Required property is missing: runs-on"}, + } { + apiRepo := createActionsTestRepo(t, user2Token, "caller-"+name+"-callee", false) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: apiRepo.ID}) + if testCase.callee != "" { + createRepoWorkflowFile(t, user2, user2Token, repo, ".gitea/workflows/callee.yml", testCase.callee) + } + createRepoWorkflowFile(t, user2, user2Token, repo, ".gitea/workflows/caller.yaml", + "on: push\njobs:\n plain_job:\n runs-on: ubuntu-latest\n steps:\n - run: echo\n call:\n needs: plain_job\n uses: ./.gitea/workflows/callee.yml\n") + assertInvalidWorkflowRun(t, repo.ID, "caller.yaml", testCase.want) + } }) - t.Run("Nested caller with missing callee fails with the error as summary instead of blocking", func(t *testing.T) { - // When the expansion hits a terminal error (e.g. missing callee), the emitter must fail the caller and let the run finish as failed, not retry the expansion forever. - apiRepo := createActionsTestRepo(t, user2Token, "nested-caller-missing-callee", false) + t.Run("Nested caller failing to expand fails with the error as summary instead of blocking", func(t *testing.T) { + // When the expansion hits a terminal error, the emitter must fail the caller and let the run finish as failed, not retry the expansion forever. + apiRepo := createActionsTestRepo(t, user2Token, "nested-caller-bad-callee", false) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: apiRepo.ID}) runner := newMockRunner() runner.registerAsRepoRunner(t, repo.OwnerName, repo.Name, "mock-runner", []string{"ubuntu-latest"}, false) + createRepoWorkflowFile(t, user2, user2Token, repo, ".gitea/workflows/lib.yml", + "on:\n workflow_call:\n secrets:\n token:\n required: true\njobs:\n inner:\n runs-on: ubuntu-latest\n steps:\n - run: echo\n") createRepoWorkflowFile(t, user2, user2Token, repo, ".gitea/workflows/caller.yaml", `name: Caller on: push @@ -602,7 +598,7 @@ jobs: - run: echo 'job' bad_caller: needs: plain_job - uses: ./.gitea/workflows/does-not-exist.yml + uses: ./.gitea/workflows/lib.yml `) plainTask := runner.fetchTask(t) @@ -614,7 +610,7 @@ jobs: runner.execTask(t, plainTask, &mockTaskOutcome{result: runnerv1.Result_RESULT_SUCCESS}) - // The emitter now tries to expand bad_caller, hits the missing callee, and fails the caller. + // The emitter now tries to expand bad_caller, misses the required secret, and fails the caller. badCaller := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: badCallerPre.ID}) assert.Equal(t, actions_model.StatusFailure, badCaller.Status) // No children were inserted (the terminal error precedes the child inserts). @@ -626,7 +622,7 @@ jobs: runner.fetchNoTask(t) // no task scheduled for the failed caller; the run is not stuck summary, err := actions_model.GetActionRunJobSummary(t.Context(), repo.ID, run.ID, badCaller.RunAttemptID, badCaller.ID, 0) require.NoError(t, err) - assert.Contains(t, summary.Content, "does-not-exist.yml") + assert.Contains(t, summary.Content, "secret token is required, but not provided while calling") }) t.Run("Fork PR with secrets: inherit does not leak base repo secrets", func(t *testing.T) { @@ -989,6 +985,16 @@ jobs: }) } +func assertInvalidWorkflowRun(t *testing.T, repoID int64, workflowID, want string) { + t.Helper() + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: repoID, WorkflowID: workflowID}) + assert.Equal(t, actions_model.StatusFailure, run.Status) + assert.Zero(t, unittest.GetCount(t, &actions_model.ActionRunJob{RunID: run.ID})) + summary, err := actions_model.GetActionRunJobSummary(t.Context(), repoID, run.ID, run.LatestAttemptID, 0, 0) + require.NoError(t, err) + assert.Contains(t, summary.Content, want) +} + // token must belong to u (the commit identity) and have write access to repo. Reuse the caller's // existing token rather than logging in per call, which would re-run bcrypt password verification each time. func createRepoWorkflowFile(t *testing.T, u *user_model.User, token string, repo *repo_model.Repository, treePath, content string) {