From 8b49b76c9a901d9be557d237a8891fdd0ebcc53a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kriszti=C3=A1n=20G=C3=B6drei?= Date: Thu, 2 Feb 2023 16:30:39 +0100 Subject: [PATCH] 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 --- ...ecret_filtering_disabled_test_secrets.yml} | 0 ...ilter_test.go => secret_filtering_test.go} | 23 +++++++++++--- ....yml => secret_filtering_test_bitrise.yml} | 28 +++++++++++++++++ ....yml => secret_filtering_test_secrets.yml} | 1 + cli/run_util.go | 1 - plugins/run.go | 1 - tools/tools.go | 31 ++++++++----------- 7 files changed, 61 insertions(+), 24 deletions(-) rename _tests/integration/{log_filter_disabled_test_secrets.yml => secret_filtering_disabled_test_secrets.yml} (100%) rename _tests/integration/{log_filter_test.go => secret_filtering_test.go} (79%) rename _tests/integration/{log_filter_test_bitrise.yml => secret_filtering_test_bitrise.yml} (63%) rename _tests/integration/{log_filter_test_secrets.yml => secret_filtering_test_secrets.yml} (96%) diff --git a/_tests/integration/log_filter_disabled_test_secrets.yml b/_tests/integration/secret_filtering_disabled_test_secrets.yml similarity index 100% rename from _tests/integration/log_filter_disabled_test_secrets.yml rename to _tests/integration/secret_filtering_disabled_test_secrets.yml diff --git a/_tests/integration/log_filter_test.go b/_tests/integration/secret_filtering_test.go similarity index 79% rename from _tests/integration/log_filter_test.go rename to _tests/integration/secret_filtering_test.go index 65111360d..613c469b9 100644 --- a/_tests/integration/log_filter_test.go +++ b/_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) +} diff --git a/_tests/integration/log_filter_test_bitrise.yml b/_tests/integration/secret_filtering_test_bitrise.yml similarity index 63% rename from _tests/integration/log_filter_test_bitrise.yml rename to _tests/integration/secret_filtering_test_bitrise.yml index ab6d96857..bffbe2d28 100644 --- a/_tests/integration/log_filter_test_bitrise.yml +++ b/_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 diff --git a/_tests/integration/log_filter_test_secrets.yml b/_tests/integration/secret_filtering_test_secrets.yml similarity index 96% rename from _tests/integration/log_filter_test_secrets.yml rename to _tests/integration/secret_filtering_test_secrets.yml index 3f93b1ea3..642ac6086 100644 --- a/_tests/integration/log_filter_test_secrets.yml +++ b/_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 diff --git a/cli/run_util.go b/cli/run_util.go index 15a1b0f2a..c12dc720a 100644 --- a/cli/run_util.go +++ b/cli/run_util.go @@ -421,7 +421,6 @@ func (r WorkflowRunner) executeStep( noOutputTimeout, secrets, nil, - &logWriter, &logWriter) } diff --git a/plugins/run.go b/plugins/run.go index f5e342f15..4d35132ae 100644 --- a/plugins/run.go +++ b/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 { diff --git a/tools/tools.go b/tools/tools.go index cc0823dbe..fb49a3fc8 100644 --- a/tools/tools.go +++ b/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) } // ------------------