From d91c201968592422d6417cb22807717bcbf62731 Mon Sep 17 00:00:00 2001
From: Shunsuke Suzuki <suzuki-shunsuke@users.noreply.github.com>
Date: Sat, 9 Dec 2023 15:42:59 +0900
Subject: [PATCH] feat: add a policy action_shell_is_required (#281)

---
 README.md                                   |  1 +
 docs/policies/011.md                        | 24 ++++++++++
 pkg/controller/act/run.go                   |  1 +
 pkg/policy/action_shell_is_required.go      | 26 +++++++++++
 pkg/policy/action_shell_is_required_test.go | 50 +++++++++++++++++++++
 pkg/workflow/workflow.go                    | 10 +++--
 test-action.yaml                            |  3 ++
 7 files changed, 111 insertions(+), 4 deletions(-)
 create mode 100644 docs/policies/011.md
 create mode 100644 pkg/policy/action_shell_is_required.go
 create mode 100644 pkg/policy/action_shell_is_required_test.go

diff --git a/README.md b/README.md
index 8e90bac..2123153 100644
--- a/README.md
+++ b/README.md
@@ -33,6 +33,7 @@ ghalint is a command line tool to check GitHub Actions Workflows anc action.yaml
 1. [action_ref_should_be_full_length_commit_sha](docs/policies/008.md): action's ref should be full length commit SHA
 1. [github_app_should_limit_repositories](docs/policies/009.md): GitHub Actions issueing GitHub Access tokens from GitHub Apps should limit repositories
 1. [github_app_should_limit_permissions](docs/policies/010.md): GitHub Actions issueing GitHub Access tokens from GitHub Apps should limit permissions
+1. [action_shell_is_required](docs/policies/011.md): `shell` is required if `run` is set
 
 ## How to install
 
diff --git a/docs/policies/011.md b/docs/policies/011.md
new file mode 100644
index 0000000..41959c0
--- /dev/null
+++ b/docs/policies/011.md
@@ -0,0 +1,24 @@
+# action_shell_is_required
+
+`shell` is required if `run` is set
+
+## Examples
+
+:x:
+
+```yaml
+- run: echo hello
+```
+
+⭕
+
+```yaml
+- run: echo hello
+  shell: bash
+```
+
+## Why?
+
+> Required if run is set.
+
+https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#runsstepsshell
diff --git a/pkg/controller/act/run.go b/pkg/controller/act/run.go
index 53d6d61..5efbde2 100644
--- a/pkg/controller/act/run.go
+++ b/pkg/controller/act/run.go
@@ -27,6 +27,7 @@ func (c *Controller) Run(_ context.Context, logE *logrus.Entry, cfgFilePath stri
 	stepPolicies := []controller.StepPolicy{
 		&policy.GitHubAppShouldLimitRepositoriesPolicy{},
 		&policy.GitHubAppShouldLimitPermissionsPolicy{},
+		&policy.ActionShellIsRequiredPolicy{},
 		policy.NewActionRefShouldBeSHA1Policy(),
 	}
 	failed := false
diff --git a/pkg/policy/action_shell_is_required.go b/pkg/policy/action_shell_is_required.go
new file mode 100644
index 0000000..f65433f
--- /dev/null
+++ b/pkg/policy/action_shell_is_required.go
@@ -0,0 +1,26 @@
+package policy
+
+import (
+	"errors"
+
+	"github.com/sirupsen/logrus"
+	"github.com/suzuki-shunsuke/ghalint/pkg/config"
+	"github.com/suzuki-shunsuke/ghalint/pkg/workflow"
+)
+
+type ActionShellIsRequiredPolicy struct{}
+
+func (p *ActionShellIsRequiredPolicy) Name() string {
+	return "action_shell_is_required"
+}
+
+func (p *ActionShellIsRequiredPolicy) ID() string {
+	return "011"
+}
+
+func (p *ActionShellIsRequiredPolicy) ApplyStep(_ *logrus.Entry, _ *config.Config, _ *StepContext, step *workflow.Step) error {
+	if step.Run != "" && step.Shell == "" {
+		return errors.New("shell is required if run is set")
+	}
+	return nil
+}
diff --git a/pkg/policy/action_shell_is_required_test.go b/pkg/policy/action_shell_is_required_test.go
new file mode 100644
index 0000000..422b5e7
--- /dev/null
+++ b/pkg/policy/action_shell_is_required_test.go
@@ -0,0 +1,50 @@
+package policy_test
+
+import (
+	"testing"
+
+	"github.com/sirupsen/logrus"
+	"github.com/suzuki-shunsuke/ghalint/pkg/policy"
+	"github.com/suzuki-shunsuke/ghalint/pkg/workflow"
+)
+
+func TestActionShellIsRequiredPolicy_ApplyStep(t *testing.T) {
+	t.Parallel()
+	data := []struct {
+		name  string
+		step  *workflow.Step
+		isErr bool
+	}{
+		{
+			name: "pass",
+			step: &workflow.Step{
+				Run:   "echo hello",
+				Shell: "bash",
+			},
+		},
+		{
+			name:  "step error",
+			isErr: true,
+			step: &workflow.Step{
+				Run: "echo hello",
+			},
+		},
+	}
+	p := &policy.ActionShellIsRequiredPolicy{}
+	logE := logrus.NewEntry(logrus.New())
+	for _, d := range data {
+		d := d
+		t.Run(d.name, func(t *testing.T) {
+			t.Parallel()
+			if err := p.ApplyStep(logE, nil, nil, d.step); err != nil {
+				if d.isErr {
+					return
+				}
+				t.Fatal(err)
+			}
+			if d.isErr {
+				t.Fatal("error must be returned")
+			}
+		})
+	}
+}
diff --git a/pkg/workflow/workflow.go b/pkg/workflow/workflow.go
index 96f9dfe..fd3b173 100644
--- a/pkg/workflow/workflow.go
+++ b/pkg/workflow/workflow.go
@@ -17,10 +17,12 @@ type Job struct {
 }
 
 type Step struct {
-	Uses string
-	ID   string
-	Name string
-	With map[string]string
+	Uses  string
+	ID    string
+	Name  string
+	Run   string
+	Shell string
+	With  map[string]string
 }
 
 type Action struct {
diff --git a/test-action.yaml b/test-action.yaml
index af31a71..3bac2bd 100644
--- a/test-action.yaml
+++ b/test-action.yaml
@@ -50,3 +50,6 @@ runs:
       with:
         app-id: ${{vars.APP_ID}}
         private-key: ${{secrets.PRIVATE_KEY}}
+
+    - run: echo hello
+      # action_shell_is_required