fix cipher passphrase lookup in the environment

The value of the passphrase was overwritten by the one from the config file. Move the check and fallback to the environment in run() after options from cli and conf are merged.
orgrim · Dec 21, 2021 · 50bb19b · 50bb19b
1 parent 1d3e515
commit 50bb19b
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 210 deletions.
diff --git a/config.go b/config.go
@@ -391,26 +391,6 @@ func parseCli(args []string) (options, []string, error) {
 		return opts, changed, fmt.Errorf("options --encrypt and --decrypt are mutually exclusive")
 	}
 
-	// Ensure a non-empty passphrase is set when asking for encryption
-	if (opts.Encrypt || opts.Decrypt) && len(opts.CipherPassphrase) == 0 {
-		oncli := false
-		for _, v := range changed {
-			if v == "cipher-pass" {
-				oncli = true
-				break
-			}
-		}
-
-		// Fallback on the environment
-		if !oncli {
-			opts.CipherPassphrase, _ = os.LookupEnv("PGBK_PASSPHRASE")
-		}
-
-		if len(opts.CipherPassphrase) == 0 {
-			return opts, changed, fmt.Errorf("cannot use an empty passphrase for encryption")
-		}
-	}
-
 	// Validate upload option
 	stores := []string{"none", "s3", "sftp", "gcs", "azure"}
 	if err := validateEnum(opts.Upload, stores); err != nil {
@@ -540,10 +520,6 @@ func loadConfigurationFile(path string) (options, error) {
 	}
 	opts.Format = []rune(format)[0]
 
-	if opts.Encrypt && len(opts.CipherPassphrase) == 0 {
-		return opts, fmt.Errorf("cannot use an empty passphrase for encryption")
-	}
-
 	// Validate upload option
 	stores := []string{"none", "s3", "sftp", "gcs", "azure"}
 	if err := validateEnum(opts.Upload, stores); err != nil {

diff --git a/config_test.go b/config_test.go
@@ -34,7 +34,6 @@ import (
 	"io/ioutil"
 	"os"
 	"runtime"
-	"strings"
 	"testing"
 	"time"
 )
@@ -209,8 +208,6 @@ func TestParseCli(t *testing.T) {
 		timeFormat = "2006-01-02_15-04-05"
 	}
 
-	os.Unsetenv("PGBK_PASSPHRASE")
-
 	var (
 		defaults = defaultOptions()
 		tests    = []struct {
@@ -291,76 +288,6 @@ func TestParseCli(t *testing.T) {
 				"",
 				"some/path",
 			},
-			{
-				[]string{"--encrypt"},
-				options{
-					Directory:     "/var/backups/postgresql",
-					Format:        'c',
-					DirJobs:       1,
-					CompressLevel: -1,
-					Jobs:          1,
-					PauseTimeout:  3600,
-					PurgeInterval: -30 * 24 * time.Hour,
-					PurgeKeep:     0,
-					SumAlgo:       "none",
-					CfgFile:       "/etc/pg_back/pg_back.conf",
-					TimeFormat:    timeFormat,
-					Encrypt:       true,
-					Upload:        "none",
-					AzureEndpoint: "blob.core.windows.net",
-				},
-				false,
-				false,
-				"cannot use an empty passphrase for encryption",
-				"",
-			},
-			{
-				[]string{"--encrypt", "--cipher-pass", ""},
-				options{
-					Directory:     "/var/backups/postgresql",
-					Format:        'c',
-					DirJobs:       1,
-					CompressLevel: -1,
-					Jobs:          1,
-					PauseTimeout:  3600,
-					PurgeInterval: -30 * 24 * time.Hour,
-					PurgeKeep:     0,
-					SumAlgo:       "none",
-					CfgFile:       "/etc/pg_back/pg_back.conf",
-					TimeFormat:    timeFormat,
-					Encrypt:       true,
-					Upload:        "none",
-					AzureEndpoint: "blob.core.windows.net",
-				},
-				false,
-				false,
-				"cannot use an empty passphrase for encryption",
-				"",
-			},
-			{
-				[]string{"--encrypt", "--cipher-pass", "testpass"},
-				options{
-					Directory:        "/var/backups/postgresql",
-					Format:           'c',
-					DirJobs:          1,
-					CompressLevel:    -1,
-					Jobs:             1,
-					PauseTimeout:     3600,
-					PurgeInterval:    -30 * 24 * time.Hour,
-					PurgeKeep:        0,
-					SumAlgo:          "none",
-					CfgFile:          "/etc/pg_back/pg_back.conf",
-					TimeFormat:       timeFormat,
-					Encrypt:          true,
-					CipherPassphrase: "testpass",
-					Upload:           "none",
-					AzureEndpoint:    "blob.core.windows.net",
-				},
-				false,
-				false,
-				"",
-				"",
-			},
 			{
 				[]string{"--upload", "wrong"},
 				options{
@@ -437,119 +364,6 @@ func TestParseCli(t *testing.T) {
 	}
 }
 
-func TestParseCliEnv(t *testing.T) {
-	timeFormat := time.RFC3339
-	if runtime.GOOS == "windows" {
-		timeFormat = "2006-01-02_15-04-05"
-	}
-
-	os.Unsetenv("PGBK_PASSPHRASE")
-
-	var (
-		tests = []struct {
-			args []string
-			want options
-			err  string
-			env  string
-		}{
-			{
-				[]string{"--encrypt"},
-				options{
-					Directory:     "/var/backups/postgresql",
-					Format:        'c',
-					DirJobs:       1,
-					CompressLevel: -1,
-					Jobs:          1,
-					PauseTimeout:  3600,
-					PurgeInterval: -30 * 24 * time.Hour,
-					PurgeKeep:     0,
-					SumAlgo:       "none",
-					CfgFile:       "/etc/pg_back/pg_back.conf",
-					TimeFormat:    timeFormat,
-					Encrypt:       true,
-					Upload:        "none",
-					AzureEndpoint: "blob.core.windows.net",
-				},
-				"cannot use an empty passphrase for encryption",
-				"PGBK_PASSPHRASE=",
-			},
-			{
-				[]string{"--encrypt"},
-				options{
-					Directory:        "/var/backups/postgresql",
-					Format:           'c',
-					DirJobs:          1,
-					CompressLevel:    -1,
-					Jobs:             1,
-					PauseTimeout:     3600,
-					PurgeInterval:    -30 * 24 * time.Hour,
-					PurgeKeep:        0,
-					SumAlgo:          "none",
-					CfgFile:          "/etc/pg_back/pg_back.conf",
-					TimeFormat:       timeFormat,
-					Encrypt:          true,
-					CipherPassphrase: "testpass",
-					Upload:           "none",
-					AzureEndpoint:    "blob.core.windows.net",
-				},
-				"",
-				"PGBK_PASSPHRASE=testpass",
-			},
-			{
-				[]string{"--encrypt", "--cipher-pass", "testpass"},
-				options{
-					Directory:        "/var/backups/postgresql",
-					Format:           'c',
-					DirJobs:          1,
-					CompressLevel:    -1,
-					Jobs:             1,
-					PauseTimeout:     3600,
-					PurgeInterval:    -30 * 24 * time.Hour,
-					PurgeKeep:        0,
-					SumAlgo:          "none",
-					CfgFile:          "/etc/pg_back/pg_back.conf",
-					TimeFormat:       timeFormat,
-					Encrypt:          true,
-					CipherPassphrase: "testpass",
-					Upload:           "none",
-					AzureEndpoint:    "blob.core.windows.net",
-				},
-				"",
-				"PGBK_PASSPHRASE=testenv",
-			},
-		}
-	)
-	for i, st := range tests {
-		t.Run(fmt.Sprintf("%v", i), func(t *testing.T) {
-			var (
-				opts options
-				err  error
-			)
-
-			// Prepare the environment
-			if st.env != "" {
-				v := strings.Split(st.env, "=")
-				os.Setenv(v[0], v[1])
-			}
-			// reset pflag default flagset between each sub test
-			pflag.CommandLine = pflag.NewFlagSet(os.Args[0], pflag.ContinueOnError)
-
-			// when testing for help or version the usage is output to stderr, discard it with a pipe
-			opts, _, err = parseCli(st.args)
-
-			if err != nil {
-				if st.err != err.Error() {
-					t.Errorf("got error %v, expected %v", st.err, err)
-				}
-			} else {
-				if diff := cmp.Diff(st.want, opts, cmpopts.EquateEmpty()); diff != "" {
-					t.Errorf("ParseCli() mismatch (-want +got):\n%s", diff)
-				}
-			}
-		})
-	}
-}
-
 func TestLoadConfigurationFile(t *testing.T) {
 	timeFormat := time.RFC3339
 	if runtime.GOOS == "windows" {

diff --git a/main.go b/main.go
@@ -158,6 +158,16 @@ func run() (retVal error) {
 	// the command line
 	opts := mergeCliAndConfigOptions(cliOpts, configOpts, cliOptList)
 
+	// Ensure a non-empty passphrase is set when asking for encryption
+	if (opts.Encrypt || opts.Decrypt) && len(opts.CipherPassphrase) == 0 {
+		// Fallback on the environment
+		opts.CipherPassphrase = os.Getenv("PGBK_PASSPHRASE")
+
+		if len(opts.CipherPassphrase) == 0 {
+			return fmt.Errorf("cannot use an empty passphrase for encryption")
+		}
+	}
+
 	// When asked to decrypt the backups, do it here and exit, we have all
 	// required input (passphrase and backup directory)
 	if opts.Decrypt {