fix os.Env precedence resolving variables in .env file

Signed-off-by: Nicolas De Loof <[email protected]>

Author: ndeloof

cli/options.go

@@ -255,9 +255,8 @@ func WithDotEnv(o *ProjectOptions) error {
 		return err
 	}
 	for k, v := range envMap {
-		o.Environment[k] = v
-		if osVal, ok := os.LookupEnv(k); ok {
-			o.Environment[k] = osVal
+		if _, set := o.Environment[k]; !set {
+			o.Environment[k] = v
 		}
 	}
 	return nil
@@ -304,15 +303,12 @@ func GetEnvFromFile(currentEnv map[string]string, workingDir string, filenames [
 		}
 
 		env, err := dotenv.ParseWithLookup(bytes.NewReader(b), func(k string) (string, bool) {
-			v, ok := envMap[k]
+			v, ok := currentEnv[k]
 			if ok {
 				return v, true
 			}
-			v, ok = currentEnv[k]
-			if !ok {
-				return "", false
-			}
-			return v, true
+			v, ok = envMap[k]
+			return v, ok
 		})
 		if err != nil {
 			return envMap, errors.Wrapf(err, "failed to read %s", dotEnvFile)

cli/options_test.go

@@ -322,3 +322,46 @@ func TestGetEnvFromFile(t *testing.T) {
 	_, err = GetEnvFromFile(nil, wd, []string{f})
 	assert.Check(t, strings.HasSuffix(err.Error(), ".env is a directory"))
 }
+
+func TestEnvVariablePrecedence(t *testing.T) {
+	testcases := []struct {
+		name     string
+		dotEnv   string
+		osEnv    []string
+		expected map[string]string
+	}{
+		{
+			"no value set in environment",
+			"FOO=foo\nBAR=${FOO}",
+			nil,
+			map[string]string{
+				"FOO": "foo",
+				"BAR": "foo",
+			},
+		},
+		{
+			"conflict with value set in environment",
+			"FOO=foo\nBAR=${FOO}",
+			[]string{"FOO=zot"},
+			map[string]string{
+				"FOO": "zot",
+				"BAR": "zot",
+			},
+		},
+	}
+
+	for _, test := range testcases {
+		t.Run(test.name, func(t *testing.T) {
+			wd := t.TempDir()
+			err := os.WriteFile(filepath.Join(wd, ".env"), []byte(test.dotEnv), 0o700)
+			assert.NilError(t, err)
+			options, err := NewProjectOptions(nil,
+				// First load os.Env variable, higher in precedence rule
+				WithEnv(test.osEnv),
+				// Then load dotEnv file
+				WithWorkingDirectory(wd), WithDotEnv)
+			assert.NilError(t, err)
+			assert.DeepEqual(t, test.expected, options.Environment)
+		})
+	}
+}

dotenv/godotenv.go

@@ -162,10 +162,11 @@ func readFile(filename string, lookupFn LookupFn) (map[string]string, error) {
 
 func expandVariables(value string, envMap map[string]string, lookupFn LookupFn) (string, error) {
 	retVal, err := template.Substitute(value, func(k string) (string, bool) {
-		if v, ok := envMap[k]; ok {
-			return v, ok
+		if v, ok := lookupFn(k); ok {
+			return v, true
 		}
-		return lookupFn(k)
+		v, ok := envMap[k]
+		return v, ok
 	})
 	if err != nil {
 		return value, err

dotenv/godotenv_test.go

@@ -587,9 +587,9 @@ func TestSubstitutionsWithEnvFilePrecedence(t *testing.T) {
 	envFileName := "fixtures/substitutions.env"
 	expectedValues := map[string]string{
 		"OPTION_A": "1",
-		"OPTION_B": "1",
-		"OPTION_C": "1",
-		"OPTION_D": "1_1",
+		"OPTION_B": "5",
+		"OPTION_C": "5",
+		"OPTION_D": "5_5",
 		"OPTION_E": "",
 	}