From 2e117a4d2b6a195243c698686fec7f6a4a10409c Mon Sep 17 00:00:00 2001
From: sebastien-perpignane
 <63523775+sebastien-perpignane@users.noreply.github.com>
Date: Thu, 19 Sep 2024 10:28:45 +0200
Subject: [PATCH] bug/issue #2448 - manage special bash options when no shell
 is defined (#2449)

* bash without "-o pipefail" option when "bash" is not explicitely
defined in the workflow
* bonus: fix inverted expected and actual in TestGetGitHubContext assertions
---
 pkg/model/workflow.go          | 12 ++++++++++--
 pkg/model/workflow_test.go     | 11 +++++++----
 pkg/runner/run_context_test.go | 18 +++++++++---------
 pkg/runner/step_run.go         | 14 +++++++++-----
 4 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/pkg/model/workflow.go b/pkg/model/workflow.go
index e66bf4ce949..57e8f57dc61 100644
--- a/pkg/model/workflow.go
+++ b/pkg/model/workflow.go
@@ -572,6 +572,8 @@ type Step struct {
 	Uses               string            `yaml:"uses"`
 	Run                string            `yaml:"run"`
 	WorkingDirectory   string            `yaml:"working-directory"`
+	// WorkflowShell is the shell really configured in the job, directly at step level or higher in defaults.run.shell
+	WorkflowShell      string            `yaml:"-"`
 	Shell              string            `yaml:"shell"`
 	Env                yaml.Node         `yaml:"env"`
 	With               map[string]string `yaml:"with"`
@@ -614,8 +616,14 @@ func (s *Step) ShellCommand() string {
 
 	//Reference: https://github.com/actions/runner/blob/8109c962f09d9acc473d92c595ff43afceddb347/src/Runner.Worker/Handlers/ScriptHandlerHelpers.cs#L9-L17
 	switch s.Shell {
-	case "", "bash":
-		shellCommand = "bash --noprofile --norc -e -o pipefail {0}"
+	case "":
+		shellCommand = "bash -e {0}"
+	case "bash":
+		if s.WorkflowShell == "" {
+			shellCommand = "bash -e {0}"
+		} else {
+			shellCommand = "bash --noprofile --norc -e -o pipefail {0}"
+		}
 	case "pwsh":
 		shellCommand = "pwsh -command . '{0}'"
 	case "python":
diff --git a/pkg/model/workflow_test.go b/pkg/model/workflow_test.go
index 03f29fd6967..92de8ca33fe 100644
--- a/pkg/model/workflow_test.go
+++ b/pkg/model/workflow_test.go
@@ -397,15 +397,18 @@ func TestReadWorkflow_Strategy(t *testing.T) {
 func TestStep_ShellCommand(t *testing.T) {
 	tests := []struct {
 		shell string
+		workflowShell string
 		want  string
 	}{
-		{"pwsh -v '. {0}'", "pwsh -v '. {0}'"},
-		{"pwsh", "pwsh -command . '{0}'"},
-		{"powershell", "powershell -command . '{0}'"},
+		{"pwsh -v '. {0}'", "", "pwsh -v '. {0}'"},
+		{"pwsh", "", "pwsh -command . '{0}'"},
+		{"powershell", "", "powershell -command . '{0}'"},
+		{"bash", "", "bash -e {0}"},
+		{"bash", "bash", "bash --noprofile --norc -e -o pipefail {0}"},
 	}
 	for _, tt := range tests {
 		t.Run(tt.shell, func(t *testing.T) {
-			got := (&Step{Shell: tt.shell}).ShellCommand()
+			got := (&Step{Shell: tt.shell, WorkflowShell: tt.workflowShell}).ShellCommand()
 			assert.Equal(t, got, tt.want)
 		})
 	}
diff --git a/pkg/runner/run_context_test.go b/pkg/runner/run_context_test.go
index 692b4eefa75..0656aad881c 100644
--- a/pkg/runner/run_context_test.go
+++ b/pkg/runner/run_context_test.go
@@ -387,15 +387,15 @@ func TestGetGitHubContext(t *testing.T) {
 		owner = o
 	}
 
-	assert.Equal(t, ghc.RunID, "1")
-	assert.Equal(t, ghc.RunNumber, "1")
-	assert.Equal(t, ghc.RetentionDays, "0")
-	assert.Equal(t, ghc.Actor, actor)
-	assert.Equal(t, ghc.Repository, repo)
-	assert.Equal(t, ghc.RepositoryOwner, owner)
-	assert.Equal(t, ghc.RunnerPerflog, "/dev/null")
-	assert.Equal(t, ghc.Token, rc.Config.Secrets["GITHUB_TOKEN"])
-	assert.Equal(t, ghc.Job, "job1")
+	assert.Equal(t, "1", ghc.RunID)
+	assert.Equal(t, "1", ghc.RunNumber)
+	assert.Equal(t, "0", ghc.RetentionDays)
+	assert.Equal(t, actor, ghc.Actor)
+	assert.Equal(t, repo, ghc.Repository)
+	assert.Equal(t, owner, ghc.RepositoryOwner)
+	assert.Equal(t, "/dev/null", ghc.RunnerPerflog)
+	assert.Equal(t, rc.Config.Secrets["GITHUB_TOKEN"], ghc.Token)
+	assert.Equal(t, "job1", ghc.Job)
 }
 
 func TestGetGithubContextRef(t *testing.T) {
diff --git a/pkg/runner/step_run.go b/pkg/runner/step_run.go
index 054ed702321..a905a20e1f6 100644
--- a/pkg/runner/step_run.go
+++ b/pkg/runner/step_run.go
@@ -166,16 +166,18 @@ func (sr *stepRun) setupShell(ctx context.Context) {
 	step := sr.Step
 
 	if step.Shell == "" {
-		step.Shell = rc.Run.Job().Defaults.Run.Shell
+		step.WorkflowShell = rc.Run.Job().Defaults.Run.Shell
+	} else {
+		step.WorkflowShell = step.Shell
 	}
 
-	step.Shell = rc.NewExpressionEvaluator(ctx).Interpolate(ctx, step.Shell)
+	step.WorkflowShell = rc.NewExpressionEvaluator(ctx).Interpolate(ctx, step.WorkflowShell)
 
-	if step.Shell == "" {
-		step.Shell = rc.Run.Workflow.Defaults.Run.Shell
+	if step.WorkflowShell == "" {
+		step.WorkflowShell = rc.Run.Workflow.Defaults.Run.Shell
 	}
 
-	if step.Shell == "" {
+	if step.WorkflowShell == "" {
 		if _, ok := rc.JobContainer.(*container.HostEnvironment); ok {
 			shellWithFallback := []string{"bash", "sh"}
 			// Don't use bash on windows by default, if not using a docker container
@@ -196,6 +198,8 @@ func (sr *stepRun) setupShell(ctx context.Context) {
 			// Currently only linux containers are supported, use sh by default like actions/runner
 			step.Shell = "sh"
 		}
+	} else {
+		step.Shell = step.WorkflowShell
 	}
 }