feat(cmd): Test --verbose and --verbose-sensitive-output

Signed-off-by: Oliver Gondža <[email protected]>

Author: olivergondza

cmd/generate.go

@@ -20,8 +20,8 @@ import (
 func NewGenerateCommand() *cobra.Command {
 	const StdIn = "-"
 	var configPath, secretName string
-	var verboseOutput bool
-	var verboseUnsafe bool
+	var verboseSafe bool
+	var verboseSensitive bool
 	var disableCache bool
 
 	var command = &cobra.Command{
@@ -64,9 +64,12 @@ func NewGenerateCommand() *cobra.Command {
 			}
 
 			v := viper.New()
-			viper.Set("verboseOutput", verboseOutput)
-			viper.Set("verboseUnsafe", verboseUnsafe)
+			viper.Set("verbose", verboseSafe || verboseSensitive)
+			viper.Set("verboseRedact", verboseSafe && !verboseSensitive)
 			viper.Set("disableCache", disableCache)
+			if verboseSensitive {
+				utils.VerboseToStdErr("Running with --verbose-sensitive-output. Sensitive information will be printed to standard error!")
+			}
 			cmdConfig, err := config.New(v, &config.Options{
 				SecretName: secretName,
 				ConfigPath: configPath,
@@ -119,8 +122,8 @@ func NewGenerateCommand() *cobra.Command {
 
 	command.Flags().StringVarP(&configPath, "config-path", "c", "", "path to a file containing Vault configuration (YAML, JSON, envfile) to use")
 	command.Flags().StringVarP(&secretName, "secret-name", "s", "", "name of a Kubernetes Secret in the argocd namespace containing Vault configuration data in the argocd namespace of your ArgoCD host (Only available when used in ArgoCD). The namespace can be overridden by using the format <namespace>:<name>")
-	command.Flags().BoolVar(&verboseOutput, "verboseOutput", false, "enable verboseOutput mode for detailed info to help with debugging. Omits sensitive data (credentials), logged to stderr")
-	command.Flags().BoolVar(&verboseUnsafe, "verboseOutput-sensitive-output", false, "enable verboseOutput mode for detailed info to help with debugging. Includes sensitive data (credentials), logged to stderr")
+	command.Flags().BoolVar(&verboseSafe, "verbose", false, "enable verbose mode for detailed info to help with debugging. Omits sensitive data (credentials), logged to stderr")
+	command.Flags().BoolVar(&verboseSensitive, "verbose-sensitive-output", false, "enable verbose mode for detailed info to help with debugging. Includes sensitive data (credentials), logged to stderr")
 	command.Flags().BoolVar(&disableCache, "disable-token-cache", false, "disable the automatic token cache feature that store tokens locally")
 	return command
 }

cmd/generate_test.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"bytes"
 	"fmt"
+	"github.com/stretchr/testify/assert"
 	"io"
 	"os"
 	"strings"
@@ -308,3 +309,60 @@ func TestMain(t *testing.T) {
 	os.Unsetenv("VAULT_SKIP_VERIFY")
 	os.Unsetenv("AVP_PATH_VALIDATION")
 }
+
+func TestVerboseness(t *testing.T) {
+	cluster, roleid, secretid = helpers.CreateTestAppRoleVault(t)
+	os.Setenv("AVP_TYPE", "vault")
+	os.Setenv("VAULT_ADDR", cluster.Cores[0].Client.Address())
+	os.Setenv("AVP_AUTH_TYPE", "approle")
+	os.Setenv("AVP_SECRET_ID", "broken_but_secret")
+	os.Setenv("AVP_ROLE_ID", "broken_but_secret")
+	os.Setenv("VAULT_SKIP_VERIFY", "true")
+
+	t.Run("Quiet", func(t *testing.T) {
+		cmd := NewGenerateCommand()
+		cmd.SetArgs([]string{"../fixtures/input/nonempty/secret_path.yaml"})
+		cmd.SetOut(bytes.NewBufferString(""))
+		cmd.SetErr(bytes.NewBufferString(""))
+		logOut := helpers.CaptureOutput(func() {
+			cmd.Execute()
+		})
+
+		assert.Equal(t, "", logOut)
+	})
+
+	t.Run("Safe verbose", func(t *testing.T) {
+		cmd := NewGenerateCommand()
+		cmd.SetArgs([]string{"../fixtures/input/nonempty/secret_path.yaml", "--verbose"})
+		cmd.SetOut(bytes.NewBufferString(""))
+		cmd.SetErr(bytes.NewBufferString(""))
+		logOut := helpers.CaptureOutput(func() {
+			cmd.Execute()
+		})
+
+		assert.Contains(t, logOut, "Hashicorp Vault authenticating with role ID ***REDACTED(17 characters)*** and secret ID ***REDACTED(17 characters)*** at path auth/approle")
+		assert.NotContains(t, logOut, "broken_but_secret")
+	})
+
+	t.Run("Sensitive verbose", func(t *testing.T) {
+		cmd := NewGenerateCommand()
+		cmd.SetArgs([]string{"../fixtures/input/nonempty/secret_path.yaml", "--verbose-sensitive-output"})
+		cmd.SetOut(bytes.NewBufferString(""))
+		cmd.SetErr(bytes.NewBufferString(""))
+		logOut := helpers.CaptureOutput(func() {
+			cmd.Execute()
+		})
+
+		assert.Contains(t, logOut, "Running with --verbose-sensitive-output. Sensitive information will be printed to standard error!")
+		assert.Contains(t, logOut, "Hashicorp Vault authenticating with role ID broken_but_secret and secret ID broken_but_secret at path auth/approle")
+		assert.NotContains(t, logOut, "***REDACTED")
+	})
+
+	os.Unsetenv("AVP_TYPE")
+	os.Unsetenv("VAULT_ADDR")
+	os.Unsetenv("AVP_AUTH_TYPE")
+	os.Unsetenv("AVP_SECRET_ID")
+	os.Unsetenv("AVP_ROLE_ID")
+	os.Unsetenv("VAULT_SKIP_VERIFY")
+	os.Unsetenv("AVP_PATH_VALIDATION")
+}

pkg/config/config_test.go

@@ -1,14 +1,13 @@
 package config_test
 
 import (
-	"bytes"
 	"fmt"
-	"log"
 	"os"
 	"strings"
 	"testing"
 
 	"github.com/argoproj-labs/argocd-vault-plugin/pkg/config"
+	"github.com/argoproj-labs/argocd-vault-plugin/pkg/helpers"
 	"github.com/spf13/viper"
 )
 
@@ -267,19 +266,6 @@ func TestNewConfigNoAuthType(t *testing.T) {
 	os.Unsetenv("AVP_TYPE")
 }
 
-// Helper function that captures log output from a function call into a string
-// Adapted from https://stackoverflow.com/a/26806093/170154
-func captureOutput(f func()) string {
-	var buf bytes.Buffer
-	flags := log.Flags()
-	log.SetOutput(&buf)
-	log.SetFlags(0) // don't include any date or time in the logging messages
-	f()
-	log.SetOutput(os.Stderr)
-	log.SetFlags(flags)
-	return buf.String()
-}
-
 func TestNewConfigAwsRegionWarning(t *testing.T) {
 	testCases := []struct {
 		environment  map[string]interface{}
@@ -314,7 +300,7 @@ func TestNewConfigAwsRegionWarning(t *testing.T) {
 		viper.Set("verboseOutput", true)
 
 		v := viper.New()
-		output := captureOutput(func() {
+		output := helpers.CaptureOutput(func() {
 			config, err := config.New(v, &config.Options{})
 			if err != nil {
 				t.Error(err)

pkg/helpers/test_helpers.go

@@ -1,8 +1,11 @@
 package helpers
 
 import (
+	"bytes"
 	"fmt"
+	"log"
 	"net"
+	"os"
 	"strconv"
 	"testing"
 
@@ -543,3 +546,16 @@ func (v *MockVault) GetIndividualSecret(path, secret, version string, annotation
 	num, _ := strconv.ParseInt(version, 10, 0)
 	return v.Data[num-1][secret], nil
 }
+
+// Helper function that captures log output from a function call into a string
+// Adapted from https://stackoverflow.com/a/26806093/170154
+func CaptureOutput(f func()) string {
+	var buf bytes.Buffer
+	flags := log.Flags()
+	log.SetOutput(&buf)
+	log.SetFlags(0) // don't include any date or time in the logging messages
+	f()
+	log.SetOutput(os.Stderr)
+	log.SetFlags(flags)
+	return buf.String()
+}

pkg/utils/util.go

@@ -142,15 +142,17 @@ func DefaultHttpClient() *http.Client {
 	return httpClient
 }
 
+// VerboseToStdErr formatand prints message to stderr, if either `--verbose` or `--verbose-sensitive-output` were passed.
+// It is a responsibility of the user to call SanitizeUnsafe on all arguments that can contain sensitive data.
 func VerboseToStdErr(format string, message ...interface{}) {
-	if viper.GetBool("verboseOutput") {
+	if viper.GetBool("verbose") {
 		log.Printf(fmt.Sprintf("%s\n", format), message...)
 	}
 }
 
-// SanitizeUnsafe replaces the message data with redacted literal unless `--verbose-sensitive-output` was passed
+// SanitizeUnsafe replaces the message data with redacted literal unless `--verbose-sensitive-output` was passed.
 func SanitizeUnsafe(message interface{}) interface{} {
-	if viper.GetBool("verboseOutput") && !viper.GetBool("verboseUnsafe") {
+	if viper.GetBool("verboseRedact") {
 		messageLen := len(fmt.Sprintf("%s", message))
 		return fmt.Sprintf("***REDACTED(%v characters)***", messageLen)
 	} else {