From 3cce2d91006a4c160a0e9d2ef5ccf7bb4a467d4d Mon Sep 17 00:00:00 2001
From: Ivan Styazhkin <nesusvet@meta.com>
Date: Thu, 2 Nov 2023 07:55:56 -0700
Subject: [PATCH] Bash executor should not tolerate non-zero exit codes by
 default

Summary:
From github issue #210
https://github.com/facebookincubator/TTPForge/issues/210

> All inline TTPs will have set -e by default. If a user wants to not run a step with that, they can add set +e to their TTP.

Differential Revision: D50892103
---
 pkg/blocks/basicstep.go | 11 +++++++++--
 pkg/blocks/step_test.go | 28 ++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/pkg/blocks/basicstep.go b/pkg/blocks/basicstep.go
index 5c590dbb..808c4acf 100755
--- a/pkg/blocks/basicstep.go
+++ b/pkg/blocks/basicstep.go
@@ -81,7 +81,7 @@ func (b *BasicStep) Validate(execCtx TTPExecutionContext) error {
 	// Set Executor to "bash" if it is not provided
 	if b.Executor == "" && b.Inline != "" {
 		logging.L().Debug("defaulting to bash since executor was not provided")
-		b.Executor = "bash"
+		b.Executor = ExecutorBash
 	}
 
 	// Return if Executor is ExecutorBinary
@@ -153,8 +153,15 @@ func (b *BasicStep) executeBashStdin(ptx context.Context, execCtx TTPExecutionCo
 	return result, nil
 }
 
+func (b *BasicStep) buildCommand(ctx context.Context, executor string) *exec.Cmd {
+	if executor == ExecutorBash {
+		return exec.CommandContext(ctx, executor, "-o", "errexit")
+	}
+	return exec.CommandContext(ctx, executor)
+}
+
 func (b *BasicStep) prepareCommand(ctx context.Context, execCtx TTPExecutionContext, envAsList []string, inline string) *exec.Cmd {
-	cmd := exec.CommandContext(ctx, b.Executor)
+	cmd := b.buildCommand(ctx, b.Executor)
 	cmd.Env = envAsList
 	cmd.Dir = execCtx.WorkDir
 	cmd.Stdin = strings.NewReader(inline)
diff --git a/pkg/blocks/step_test.go b/pkg/blocks/step_test.go
index d0dba37f..b2fa8926 100644
--- a/pkg/blocks/step_test.go
+++ b/pkg/blocks/step_test.go
@@ -70,6 +70,34 @@ inline: this will error`,
 			content:            `inline: echo should_error_before_execution`,
 			wantUnmarshalError: true,
 		},
+		{
+			name: "Basic bash executor doesn't tolerate non-zero exit codes in inline scripts",
+			content: `name: inline_step
+description: this is a test
+inline: |
+  false
+  echo executing
+cleanup:
+  inline: echo cleanup
+`,
+			wantExecuteError:      true,
+			expectedExecuteStdout: "",
+			expectedCleanupStdout: "cleanup\n",
+		},
+		{
+			name: "Basic bash supports setting error processing option to ignore errors",
+			content: `name: inline_step
+inline: |
+  set +e
+  false
+  echo executing
+cleanup:
+  inline: echo cleanup
+`,
+			wantExecuteError:      false,
+			expectedExecuteStdout: "executing\n",
+			expectedCleanupStdout: "cleanup\n",
+		},
 	}
 
 	for _, tc := range testCases {