Change the order of step output writers (#851)

* Change the order of step output writers

* Rename log filter tests to secret filtering tests

* Test secret filtering in failing step logs

* Fix yaml lint issues

_tests/integration/log_filter_test.go → _tests/integration/secret_filtering_test.go

@@ -20,9 +20,9 @@ bitrise_testmfsWwlaF+Y0w0xVfAcABHdYjWHx2UHP02EC1ZGUAqF9z6XaCV8l9
 oMHHu9lvWKuxpVNPcGY/kR3G897Qn+6vE3yuVwbD4reu0IHAWZzBgt7e3we5
 -----END RSA PRIVATE KEY-----`
 
-func Test_LogFilter(t *testing.T) {
-	configPth := "log_filter_test_bitrise.yml"
-	secretsPth := "log_filter_test_secrets.yml"
+func Test_SecretFiltering(t *testing.T) {
+	configPth := "secret_filtering_test_bitrise.yml"
+	secretsPth := "secret_filtering_test_secrets.yml"
 
 	t.Log("trivial test")
 	{
@@ -96,7 +96,7 @@ starts in a new line`)
 
 	t.Log("disable filtering test")
 	{
-		secretsPth = "log_filter_disabled_test_secrets.yml"
+		secretsPth = "secret_filtering_disabled_test_secrets.yml"
 
 		os.Unsetenv("BITRISE_SECRET_FILTERING")
 
@@ -106,3 +106,18 @@ starts in a new line`)
 		require.Contains(t, out, sshKeyLogChunk)
 	}
 }
+
+func Test_Secret_Filtering_FailingStep(t *testing.T) {
+	configPth := "secret_filtering_test_bitrise.yml"
+	secretsPth := "secret_filtering_test_secrets.yml"
+	workflowID := "failing_step_test"
+	secretEnvVarValue := "secret value"
+	regularEnvVarValue := "regular value"
+
+	cmd := command.New(binPath(), "run", workflowID, "--config", configPth, "--inventory", secretsPth)
+	out, err := cmd.RunAndReturnTrimmedCombinedOutput()
+	require.Error(t, err, out)
+	require.Equal(t, "exit status 1", err.Error(), out)
+	require.NotContains(t, out, secretEnvVarValue)
+	require.Contains(t, out, regularEnvVarValue)
+}

_tests/integration/log_filter_test_bitrise.yml → _tests/integration/secret_filtering_test_bitrise.yml

@@ -1,6 +1,10 @@
 format_version: 1.3.0
 default_step_lib_source: https://github.com/bitrise-io/bitrise-steplib.git
 
+app:
+  envs:
+  - REGULAR_ENV_VAR: regular value
+
 workflows:
   primary:
     steps:
@@ -47,3 +51,27 @@ workflows:
             echo 'SECRET_WITH_NEWLINES_IN_THE_MIDDLE: empty line after this\n\nand before this'
             echo 'SECRET_ENDING_WITH_NEWLINE: ending with newline\n'
             echo "starts in a new line"
+
+  failing_step_test:
+    steps:
+    - script:
+        title: Successful Step
+        inputs:
+        - content: |-
+            #!/usr/bin/env bash
+            set -e
+
+            echo -e "A secret env var value (${SECRET_ENV_VAR}) is redacted."
+            echo "While a regular env var value ($REGULAR_ENV_VAR) is visible."
+    - script:
+        title: Failing Step
+        inputs:
+        - content: |-
+            #!/usr/bin/env bash
+            set -e
+
+            RED='\033[31;1m'
+            NC='\033[0m'
+            echo -e "${RED}A secret env var value (${SECRET_ENV_VAR}) is redacted.${NC}"
+            echo "While a regular env var value ($REGULAR_ENV_VAR) is visible."
+            exit 2

_tests/integration/log_filter_test_secrets.yml → _tests/integration/secret_filtering_test_secrets.yml

@@ -16,3 +16,4 @@ envs:
     -----END RSA PRIVATE KEY-----
 - SECRET_WITH_NEWLINES_IN_THE_MIDDLE: "empty line after this\n\nand before this"
 - SECRET_ENDING_WITH_NEWLINE: "ending with newline\n"
+- SECRET_ENV_VAR: secret value

cli/run_util.go

@@ -421,7 +421,6 @@ func (r WorkflowRunner) executeStep(
 		noOutputTimeout,
 		secrets,
 		nil,
-		&logWriter,
 		&logWriter)
 }
 

plugins/run.go

@@ -163,7 +163,6 @@ func runPlugin(plugin Plugin, args []string, envs PluginConfig, input []byte) er
 		-1,
 		nil,
 		input,
-		&logWriter,
 		&logWriter)
 
 	if err != nil {

tools/tools.go

@@ -277,29 +277,24 @@ func EnvmanRun(envStorePth,
 	noOutputTimeout time.Duration,
 	secrets []string,
 	stdInPayload []byte,
-	stdout io.Writer,
-	stderr io.Writer,
+	out io.Writer,
 ) (int, error) {
 	envs, err := envman.ReadAndEvaluateEnvs(envStorePth, &envmanEnv.DefaultEnvironmentSource{})
 	if err != nil {
 		return 1, err
 	}
 
-	var inReader io.Reader
 	var outWriter io.Writer
-	var errWriter io.Writer
-	errorFinder := errorfinder.NewErrorFinder()
-	var fw *filterwriter.Writer
-
-	if !configs.IsSecretFiltering {
-		outWriter = errorFinder.WrapWriter(stdout)
-		errWriter = errorFinder.WrapWriter(stderr)
-	} else {
-		fw = filterwriter.New(secrets, stdout)
-		outWriter = errorFinder.WrapWriter(fw)
-		errWriter = outWriter
+	errorFinderWriter := errorfinder.NewErrorFinder()
+	outWriter = errorFinderWriter.WrapWriter(out)
+
+	var secretRedactorWriter *filterwriter.Writer
+	if configs.IsSecretFiltering {
+		secretRedactorWriter = filterwriter.New(secrets, outWriter)
+		outWriter = secretRedactorWriter
 	}
 
+	var inReader io.Reader
 	inReader = os.Stdin
 	if stdInPayload != nil {
 		inReader = bytes.NewReader(stdInPayload)
@@ -314,20 +309,20 @@ func EnvmanRun(envStorePth,
 	cmd := timeoutcmd.New(workDirPth, name, args...)
 	cmd.SetTimeout(timeout)
 	cmd.SetHangTimeout(noOutputTimeout)
-	cmd.SetStandardIO(inReader, outWriter, errWriter)
+	cmd.SetStandardIO(inReader, outWriter, outWriter)
 	cmd.SetEnv(append(envs, "PWD="+workDirPth))
 
 	err = cmd.Start()
 
 	// flush the writer anyway if the process is finished
 	if configs.IsSecretFiltering {
-		_, ferr := fw.Flush()
+		_, ferr := secretRedactorWriter.Flush()
 		if ferr != nil {
-			return 1, errorFinder.WrapError(ferr)
+			return 1, errorFinderWriter.WrapError(ferr)
 		}
 	}
 
-	return timeoutcmd.ExitStatus(err), errorFinder.WrapError(err)
+	return timeoutcmd.ExitStatus(err), errorFinderWriter.WrapError(err)
 }
 
 // ------------------