Repository: nektos/act (language: go)

Issue:
Some formating yaml syntax errors can result in a complete Go crash
### Bug report info

```plain text
$ act --bug-report
act version:            0.2.42.r1.gf91b2aa5
GOOS:                   linux
GOARCH:                 amd64
NumCPU:                 16
Docker host:            DOCKER_HOST environment variable is unset/empty.
Sockets found:
	/var/run/docker.sock
Config files:
	/home/clement/.actrc:
		-P ubuntu-latest=catthehacker/ubuntu:act-latest
		-P ubuntu-22.04=catthehacker/ubuntu:act-22.04
		-P ubuntu-20.04=catthehacker/ubuntu:act-20.04
		-P ubuntu-18.04=catthehacker/ubuntu:act-18.04
	.actrc:
		-P ubuntu-latest=catthehacker/ubuntu:act-latest
		-P ubuntu-22.04=catthehacker/ubuntu:act-22.04
		-P ubuntu-20.04=catthehacker/ubuntu:act-20.04
		-P ubuntu-18.04=catthehacker/ubuntu:act-18.04
Build info:
	Go version:            go1.20
	Module path:           command-line-arguments
	Main version:
	Main path:
	Main checksum:
	Build settings:
		-buildmode:           pie
		-compiler:            gc
		-trimpath:            true
		CGO_ENABLED:          1
		GOARCH:               amd64
		GOOS:                 linux
		GOAMD64:              v1
Docker Engine:
	Engine version:        20.10.23
	Engine runtime:        runc
	Cgroup version:        2
	Cgroup driver:         systemd
	Storage driver:        overlay2
	Registry URI:          https://index.docker.io/v1/
	OS:                    Arch Linux
	OS type:               linux
	OS version:
	OS arch:               x86_64
	OS kernel:             6.1.7-arch1-1
	OS CPU:                16
	OS memory:             31802 MB
	Security options:
		name=seccomp,profile=default
		name=cgroupns
```


### Command used with act

```sh
act
```


### Describe issue

A slight error in the `yaml` of an action can cause a complete Go panic

### Link to GitHub repository

_No response_

### Workflow content

This is action content, not workflow. You just need to call whatever action containing this mistake.
As explained below, the same mistake is caught if present in a workflow file.

```yml
- name: Foo
- uses: Foo/Bar
```


### Relevant log output

```sh
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x5644f81519d3]

goroutine 62 [running]:
github.com/nektos/act/pkg/runner.runPreStep.func1({0x5644f85876d0, 0xc0003a5ec0})
        github.com/nektos/act/pkg/runner/action.go:503 +0x6b3
github.com/nektos/act/pkg/runner.runStepExecutor.func1({0x5644f85876d0, 0xc0003a5ec0})
        github.com/nektos/act/pkg/runner/step.go:109 +0x9c3
github.com/nektos/act/pkg/common.Executor.If.func1({0x5644f85876d0, 0xc0003a5ec0})
        github.com/nektos/act/pkg/common/executor.go:149 +0x48
github.com/nektos/act/pkg/common.Executor.If.func1({0x5644f85876d0, 0xc0003a5ec0})
        github.com/nektos/act/pkg/common/executor.go:149 +0x48
github.com/nektos/act/pkg/common.Executor.Then.func1({0x5644f85876d0, 0xc0003a5ec0})
        github.com/nektos/act/pkg/common/executor.go:141 +0x144
github.com/nektos/act/pkg/runner.useStepLogger.func1({0x5644f85876d0, 0xc00040f620})
        github.com/nektos/act/pkg/runner/job_executor.go:190 +0x39e
github.com/nektos/act/pkg/common.Executor.Then.func1({0x5644f85876d0, 0xc00040f620})
        github.com/nektos/act/pkg/common/executor.go:141 +0x144
github.com/nektos/act/pkg/common.Executor.Then.func1({0x5644f85876d0, 0xc00040f620})
        github.com/nektos/act/pkg/common/executor.go:129 +0x34
github.com/nektos/act/pkg/common.Executor.Then.func1({0x5644f85876d0, 0xc00040f620})
        github.com/nektos/act/pkg/common/executor.go:129 +0x34
github.com/nektos/act/pkg/common.Executor.Then.func1({0x5644f85876d0, 0xc00040f620})
        github.com/nektos/act/pkg/common/executor.go:129 +0x34
github.com/nektos/act/pkg/common.Executor.Finally.func1({0x5644f85876d0, 0xc00040f620})
        github.com/nektos/act/pkg/common/executor.go:175 +0x34
github.com/nektos/act/pkg/common.Executor.Finally.func1({0x5644f85876d0, 0xc00040f620})
        github.com/nektos/act/pkg/common/executor.go:175 +0x34
github.com/nektos/act/pkg/common.Executor.Finally.func1({0x5644f85876d0, 0xc00040f620})
        github.com/nektos/act/pkg/common/executor.go:175 +0x34
github.com/nektos/act/pkg/common.Executor.Then.func1({0x5644f85876d0, 0xc00040f620})
        github.com/nektos/act/pkg/common/executor.go:141 +0x144
github.com/nektos/act/pkg/runner.(*RunContext).Executor.func1({0x5644f85876d0, 0xc00040f620})
        github.com/nektos/act/pkg/runner/run_context.go:428 +0x58
github.com/nektos/act/pkg/runner.(*runnerImpl).NewPlanExecutor.func1.1({0x5644f85876d0, 0xc00040eab0})
        github.com/nektos/act/pkg/runner/runner.go:141 +0x193
github.com/nektos/act/pkg/common.NewParallelExecutor.func1.1(0x0?, 0x0?)
        github.com/nektos/act/pkg/common/executor.go:100 +0x5c
created by github.com/nektos/act/pkg/common.NewParallelExecutor.func1
        github.com/nektos/act/pkg/common/executor.go:98 +0x89
```


### Additional information

The error happened on a slight error in an action I was working on: a dash arrived on a `uses:` line while there was already a `- name:` on top of it.

The same mistake is caught for workflows so the panic only happens for actions.
In workflows we get something like this:
`Error: Invalid run/uses syntax for job:foo step:bar`


Discussion / hints:
Just run https://github.com/rhysd/actionlint right before calling `act` and you are good to go.

> Just run [rhysd/actionlint](https://github.com/rhysd/actionlint) right before calling `act` and you are good to go.

@KnisterPeter Yes linting is always a good idea. However, I don't think a complete crash of the program is something that should be considered as a normal behavior, even if this is due to a user's mistake.

Note that the same mistake is caught when done in the workflow file, as mentioned at the end of my message.
I believe the same kind of mistake should be handled the same way and caught the same way for actions as it's already done for workflows, wouldn't you agree?
Given that an invalid yaml file is syntactic garbage I'm not sure if I would agree. It might be good to not just panic, but the result would be the exact same as of now.

But feel free to provide a pull request with a fix for this. We are happy for every contribution.
@KnisterPeter actionlint doesn't support composite actions. We should fix crashs in act.

I add it to my backlog for act.

Fix patch:
diff --git a/pkg/runner/action.go b/pkg/runner/action.go
index 4d860a22a3b..94d9cfc1219 100644
--- a/pkg/runner/action.go
+++ b/pkg/runner/action.go
@@ -503,7 +503,10 @@ func runPreStep(step actionStep) common.Executor {
 				step.getCompositeRunContext(ctx)
 			}
 
-			return step.getCompositeSteps().pre(ctx)
+			if steps := step.getCompositeSteps(); steps != nil && steps.pre != nil {
+				return steps.pre(ctx)
+			}
+			return fmt.Errorf("missing steps in composite action")
 
 		default:
 			return nil
@@ -592,7 +595,10 @@ func runPostStep(step actionStep) common.Executor {
 				return err
 			}
 
-			return step.getCompositeSteps().post(ctx)
+			if steps := step.getCompositeSteps(); steps != nil && steps.post != nil {
+				return steps.post(ctx)
+			}
+			return fmt.Errorf("missing steps in composite action")
 
 		default:
 			return nil
diff --git a/pkg/runner/action_composite.go b/pkg/runner/action_composite.go
index 0b6fbe55272..b6ef58c831d 100644
--- a/pkg/runner/action_composite.go
+++ b/pkg/runner/action_composite.go
@@ -86,6 +86,10 @@ func execAsComposite(step actionStep) common.Executor {
 
 		steps := step.getCompositeSteps()
 
+		if steps == nil || steps.main == nil {
+			return fmt.Errorf("missing steps in composite action")
+		}
+
 		ctx = WithCompositeLogger(ctx, &compositeRC.Masks)
 
 		err := steps.main(ctx)


Test patch:
diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go
index fb51be976c2..b0ce4f209cb 100644
--- a/pkg/runner/runner_test.go
+++ b/pkg/runner/runner_test.go
@@ -299,6 +299,7 @@ func TestRunEvent(t *testing.T) {
 		{workdir, "do-not-leak-step-env-in-composite", "push", "", platforms, secrets},
 		{workdir, "set-env-step-env-override", "push", "", platforms, secrets},
 		{workdir, "set-env-new-env-file-per-step", "push", "", platforms, secrets},
+		{workdir, "no-panic-on-invalid-composite-action", "push", "jobs failed due to invalid action", platforms, secrets},
 	}
 
 	for _, table := range tables {
@@ -401,6 +402,7 @@ func TestRunEventHostEnvironment(t *testing.T) {
 			{workdir, "do-not-leak-step-env-in-composite", "push", "", platforms, secrets},
 			{workdir, "set-env-step-env-override", "push", "", platforms, secrets},
 			{workdir, "set-env-new-env-file-per-step", "push", "", platforms, secrets},
+			{workdir, "no-panic-on-invalid-composite-action", "push", "jobs failed due to invalid action", platforms, secrets},
 		}...)
 	}
 
diff --git a/pkg/runner/testdata/no-panic-on-invalid-composite-action/push.yml b/pkg/runner/testdata/no-panic-on-invalid-composite-action/push.yml
new file mode 100644
index 00000000000..6b9e4ae6468
--- /dev/null
+++ b/pkg/runner/testdata/no-panic-on-invalid-composite-action/push.yml
@@ -0,0 +1,29 @@
+on: push
+jobs:
+  local-invalid-step:
+    runs-on: ubuntu-latest
+    steps:
+    - run: |
+        runs:
+          using: composite
+          steps:
+          - name: Foo
+          - uses: Foo/Bar
+      shell: cp {0} action.yml
+    - uses: ./
+  local-missing-steps:
+    runs-on: ubuntu-latest
+    steps:
+    - run: |
+        runs:
+          using: composite
+      shell: cp {0} action.yml
+    - uses: ./
+  remote-invalid-step:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: nektos/act-test-actions/invalid-composite-action/invalid-step@main
+  remote-missing-steps:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: nektos/act-test-actions/invalid-composite-action/missing-steps@main
\ No newline at end of file