feat: --validate and --strict (#2717)

* feat: `--validate` and `--strict` * fix missing changes * add test for strict validate --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
2025-06-12 00:02:16 +02:00
parent d9c6afc648
commit 4ba1c2bde6
9 changed files with 79 additions and 35 deletions
--- a/pkg/artifacts/server_test.go
+++ b/pkg/artifacts/server_test.go
@@ -298,7 +298,7 @@ func runTestJobFile(ctx context.Context, t *testing.T, tjfi TestJobFileInfo) {
 		runner, err := runner.New(runnerConfig)
 		assert.Nil(t, err, tjfi.workflowPath)

-		planner, err := model.NewWorkflowPlanner(fullWorkflowPath, true)
+		planner, err := model.NewWorkflowPlanner(fullWorkflowPath, true, false)
 		assert.Nil(t, err, fullWorkflowPath)

 		plan, err := planner.PlanEvent(tjfi.eventName)
--- a/pkg/model/planner.go
+++ b/pkg/model/planner.go
@@ -56,7 +56,7 @@ type WorkflowFiles struct {
 }

 // NewWorkflowPlanner will load a specific workflow, all workflows from a directory or all workflows from a directory and its subdirectories
-func NewWorkflowPlanner(path string, noWorkflowRecurse bool) (WorkflowPlanner, error) {
+func NewWorkflowPlanner(path string, noWorkflowRecurse, strict bool) (WorkflowPlanner, error) {
 	path, err := filepath.Abs(path)
 	if err != nil {
 		return nil, err
@@ -124,7 +124,7 @@ func NewWorkflowPlanner(path string, noWorkflowRecurse bool) (WorkflowPlanner, e
 			}

 			log.Debugf("Reading workflow '%s'", f.Name())
-			workflow, err := ReadWorkflow(f)
+			workflow, err := ReadWorkflow(f, strict)
 			if err != nil {
 				_ = f.Close()
 				if err == io.EOF {
@@ -161,7 +161,7 @@ func NewSingleWorkflowPlanner(name string, f io.Reader) (WorkflowPlanner, error)
 	wp := new(workflowPlanner)

 	log.Debugf("Reading workflow %s", name)
-	workflow, err := ReadWorkflow(f)
+	workflow, err := ReadWorkflow(f, false)
 	if err != nil {
 		if err == io.EOF {
 			return nil, fmt.Errorf("unable to read workflow '%s': file is empty: %w", name, err)
--- a/pkg/model/planner_test.go
+++ b/pkg/model/planner_test.go
@@ -31,7 +31,7 @@ func TestPlanner(t *testing.T) {
 	assert.NoError(t, err, workdir)
 	for _, table := range tables {
 		fullWorkflowPath := filepath.Join(workdir, table.workflowPath)
-		_, err = NewWorkflowPlanner(fullWorkflowPath, table.noWorkflowRecurse)
+		_, err = NewWorkflowPlanner(fullWorkflowPath, table.noWorkflowRecurse, false)
 		if table.errorMessage == "" {
 			assert.NoError(t, err, "WorkflowPlanner should exit without any error")
 		} else {
--- a/pkg/model/workflow.go
+++ b/pkg/model/workflow.go
@@ -80,6 +80,20 @@ func (w *Workflow) UnmarshalYAML(node *yaml.Node) error {
 	return node.Decode((*WorkflowDefault)(w))
 }

+type WorkflowStrict Workflow
+
+func (w *WorkflowStrict) UnmarshalYAML(node *yaml.Node) error {
+	// Validate the schema before deserializing it into our model
+	if err := (&schema.Node{
+		Definition: "workflow-root-strict",
+		Schema:     schema.GetWorkflowSchema(),
+	}).UnmarshalYAML(node); err != nil {
+		return errors.Join(err, fmt.Errorf("Actions YAML Strict Schema Validation Error detected:\nFor more information, see: https://nektosact.com/usage/schema.html"))
+	}
+	type WorkflowDefault Workflow
+	return node.Decode((*WorkflowDefault)(w))
+}
+
 type WorkflowDispatchInput struct {
 	Description string   `yaml:"description"`
 	Required    bool     `yaml:"required"`
@@ -711,7 +725,12 @@ func (s *Step) Type() StepType {
 }

 // ReadWorkflow returns a list of jobs for a given workflow file reader
-func ReadWorkflow(in io.Reader) (*Workflow, error) {
+func ReadWorkflow(in io.Reader, strict bool) (*Workflow, error) {
+	if strict {
+		w := new(WorkflowStrict)
+		err := yaml.NewDecoder(in).Decode(w)
+		return (*Workflow)(w), err
+	}
 	w := new(Workflow)
 	err := yaml.NewDecoder(in).Decode(w)
 	return w, err
--- a/pkg/model/workflow_test.go
+++ b/pkg/model/workflow_test.go
@@ -19,7 +19,7 @@ jobs:
    - uses: ./actions/docker-url
 `

-	workflow, err := ReadWorkflow(strings.NewReader(yaml))
+	workflow, err := ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")

 	assert.Len(t, workflow.On(), 1)
@@ -38,7 +38,7 @@ jobs:
    - uses: ./actions/docker-url
 `

-	workflow, err := ReadWorkflow(strings.NewReader(yaml))
+	workflow, err := ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")

 	assert.Len(t, workflow.On(), 2)
@@ -64,7 +64,7 @@ jobs:
    - uses: ./actions/docker-url
 `

-	workflow, err := ReadWorkflow(strings.NewReader(yaml))
+	workflow, err := ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")
 	assert.Len(t, workflow.On(), 2)
 	assert.Contains(t, workflow.On(), "push")
@@ -83,7 +83,7 @@ jobs:
    steps:
    - uses: ./actions/docker-url`

-	workflow, err := ReadWorkflow(strings.NewReader(yaml))
+	workflow, err := ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")
 	assert.Equal(t, workflow.Jobs["test"].RunsOn(), []string{"ubuntu-latest"})
 }
@@ -101,7 +101,7 @@ jobs:
    steps:
    - uses: ./actions/docker-url`

-	workflow, err := ReadWorkflow(strings.NewReader(yaml))
+	workflow, err := ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")
 	assert.Equal(t, workflow.Jobs["test"].RunsOn(), []string{"ubuntu-latest", "linux"})
 }
@@ -126,7 +126,7 @@ jobs:
    - uses: ./actions/docker-url
 `

-	workflow, err := ReadWorkflow(strings.NewReader(yaml))
+	workflow, err := ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")
 	assert.Len(t, workflow.Jobs, 2)
 	assert.Contains(t, workflow.Jobs["test"].Container().Image, "nginx:latest")
@@ -156,7 +156,7 @@ jobs:
    - uses: ./actions/docker-url
 `

-	workflow, err := ReadWorkflow(strings.NewReader(yaml))
+	workflow, err := ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")
 	assert.Len(t, workflow.Jobs, 1)

@@ -194,7 +194,7 @@ jobs:
    uses: ./some/path/to/workflow.yaml
 `

-	workflow, err := ReadWorkflow(strings.NewReader(yaml))
+	workflow, err := ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")
 	assert.Len(t, workflow.Jobs, 6)

@@ -238,7 +238,7 @@ jobs:
    uses: some/path/to/workflow.yaml
 `

-	workflow, err := ReadWorkflow(strings.NewReader(yaml))
+	workflow, err := ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")
 	assert.Len(t, workflow.Jobs, 4)

@@ -280,7 +280,7 @@ jobs:
        uses: ./local-action
 `

-	_, err := ReadWorkflow(strings.NewReader(yaml))
+	_, err := ReadWorkflow(strings.NewReader(yaml), false)
 	assert.Error(t, err, "read workflow should fail")
 }

@@ -312,7 +312,7 @@ jobs:
          echo "${{ needs.test1.outputs.some-b-key }}"
 `

-	workflow, err := ReadWorkflow(strings.NewReader(yaml))
+	workflow, err := ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")
 	assert.Len(t, workflow.Jobs, 2)

@@ -327,7 +327,7 @@ jobs:
 }

 func TestReadWorkflow_Strategy(t *testing.T) {
-	w, err := NewWorkflowPlanner("testdata/strategy/push.yml", true)
+	w, err := NewWorkflowPlanner("testdata/strategy/push.yml", true, false)
 	assert.NoError(t, err)

 	p, err := w.PlanJob("strategy-only-max-parallel")
@@ -418,7 +418,7 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) {
 	yaml := `
    name: local-action-docker-url
    `
-	workflow, err := ReadWorkflow(strings.NewReader(yaml))
+	workflow, err := ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")
 	workflowDispatch := workflow.WorkflowDispatchConfig()
 	assert.Nil(t, workflowDispatch)
@@ -427,7 +427,7 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) {
    name: local-action-docker-url
    on: push
    `
-	workflow, err = ReadWorkflow(strings.NewReader(yaml))
+	workflow, err = ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")
 	workflowDispatch = workflow.WorkflowDispatchConfig()
 	assert.Nil(t, workflowDispatch)
@@ -436,7 +436,7 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) {
    name: local-action-docker-url
    on: workflow_dispatch
    `
-	workflow, err = ReadWorkflow(strings.NewReader(yaml))
+	workflow, err = ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")
 	workflowDispatch = workflow.WorkflowDispatchConfig()
 	assert.NotNil(t, workflowDispatch)
@@ -446,7 +446,7 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) {
    name: local-action-docker-url
    on: [push, pull_request]
    `
-	workflow, err = ReadWorkflow(strings.NewReader(yaml))
+	workflow, err = ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")
 	workflowDispatch = workflow.WorkflowDispatchConfig()
 	assert.Nil(t, workflowDispatch)
@@ -455,7 +455,7 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) {
    name: local-action-docker-url
    on: [push, workflow_dispatch]
    `
-	workflow, err = ReadWorkflow(strings.NewReader(yaml))
+	workflow, err = ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")
 	workflowDispatch = workflow.WorkflowDispatchConfig()
 	assert.NotNil(t, workflowDispatch)
@@ -467,7 +467,7 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) {
        - push
        - workflow_dispatch
    `
-	workflow, err = ReadWorkflow(strings.NewReader(yaml))
+	workflow, err = ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")
 	workflowDispatch = workflow.WorkflowDispatchConfig()
 	assert.NotNil(t, workflowDispatch)
@@ -479,7 +479,7 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) {
        push:
        pull_request:
    `
-	workflow, err = ReadWorkflow(strings.NewReader(yaml))
+	workflow, err = ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")
 	workflowDispatch = workflow.WorkflowDispatchConfig()
 	assert.Nil(t, workflowDispatch)
@@ -501,7 +501,7 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) {
                    - warning
                    - debug
    `
-	workflow, err = ReadWorkflow(strings.NewReader(yaml))
+	workflow, err = ReadWorkflow(strings.NewReader(yaml), false)
 	assert.NoError(t, err, "read workflow should succeed")
 	workflowDispatch = workflow.WorkflowDispatchConfig()
 	assert.NotNil(t, workflowDispatch)
@@ -517,3 +517,19 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) {
 		Type:     "choice",
 	}, workflowDispatch.Inputs["logLevel"])
 }
+
+func TestReadWorkflow_InvalidStringEvent(t *testing.T) {
+	yaml := `
+name: local-action-docker-url
+on: push2
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: ./actions/docker-url
+`
+
+	_, err := ReadWorkflow(strings.NewReader(yaml), true)
+	assert.Error(t, err, "read workflow should succeed")
+}
--- a/pkg/runner/reusable_workflow.go
+++ b/pkg/runner/reusable_workflow.go
@@ -115,7 +115,7 @@ func cloneIfRequired(rc *RunContext, remoteReusableWorkflow remoteReusableWorkfl

 func newReusableWorkflowExecutor(rc *RunContext, directory string, workflow string) common.Executor {
 	return func(ctx context.Context) error {
-		planner, err := model.NewWorkflowPlanner(path.Join(directory, workflow), true)
+		planner, err := model.NewWorkflowPlanner(path.Join(directory, workflow), true, false)
 		if err != nil {
 			return err
 		}
--- a/pkg/runner/runner_test.go
+++ b/pkg/runner/runner_test.go
@@ -55,7 +55,7 @@ func init() {
 }

 func TestNoWorkflowsFoundByPlanner(t *testing.T) {
-	planner, err := model.NewWorkflowPlanner("res", true)
+	planner, err := model.NewWorkflowPlanner("res", true, false)
 	assert.NoError(t, err)

 	out := log.StandardLogger().Out
@@ -75,7 +75,7 @@ func TestNoWorkflowsFoundByPlanner(t *testing.T) {
 }

 func TestGraphMissingEvent(t *testing.T) {
-	planner, err := model.NewWorkflowPlanner("testdata/issue-1595/no-event.yml", true)
+	planner, err := model.NewWorkflowPlanner("testdata/issue-1595/no-event.yml", true, false)
 	assert.NoError(t, err)

 	out := log.StandardLogger().Out
@@ -93,7 +93,7 @@ func TestGraphMissingEvent(t *testing.T) {
 }

 func TestGraphMissingFirst(t *testing.T) {
-	planner, err := model.NewWorkflowPlanner("testdata/issue-1595/no-first.yml", true)
+	planner, err := model.NewWorkflowPlanner("testdata/issue-1595/no-first.yml", true, false)
 	assert.NoError(t, err)

 	plan, err := planner.PlanEvent("push")
@@ -103,7 +103,7 @@ func TestGraphMissingFirst(t *testing.T) {
 }

 func TestGraphWithMissing(t *testing.T) {
-	planner, err := model.NewWorkflowPlanner("testdata/issue-1595/missing.yml", true)
+	planner, err := model.NewWorkflowPlanner("testdata/issue-1595/missing.yml", true, false)
 	assert.NoError(t, err)

 	out := log.StandardLogger().Out
@@ -122,7 +122,7 @@ func TestGraphWithMissing(t *testing.T) {
 func TestGraphWithSomeMissing(t *testing.T) {
 	log.SetLevel(log.DebugLevel)

-	planner, err := model.NewWorkflowPlanner("testdata/issue-1595/", true)
+	planner, err := model.NewWorkflowPlanner("testdata/issue-1595/", true, false)
 	assert.NoError(t, err)

 	out := log.StandardLogger().Out
@@ -140,7 +140,7 @@ func TestGraphWithSomeMissing(t *testing.T) {
 }

 func TestGraphEvent(t *testing.T) {
-	planner, err := model.NewWorkflowPlanner("testdata/basic", true)
+	planner, err := model.NewWorkflowPlanner("testdata/basic", true, false)
 	assert.NoError(t, err)

 	plan, err := planner.PlanEvent("push")
@@ -198,7 +198,7 @@ func (j *TestJobFileInfo) runTest(ctx context.Context, t *testing.T, cfg *Config
 	runner, err := New(runnerConfig)
 	assert.Nil(t, err, j.workflowPath)

-	planner, err := model.NewWorkflowPlanner(fullWorkflowPath, true)
+	planner, err := model.NewWorkflowPlanner(fullWorkflowPath, true, false)
 	if j.errorMessage != "" && err != nil {
 		assert.Error(t, err, j.errorMessage)
 	} else if assert.Nil(t, err, fullWorkflowPath) {