diff --git a/cmd/common/compile_test.go b/cmd/common/compile_test.go index de8952a4..6fa560ac 100644 --- a/cmd/common/compile_test.go +++ b/cmd/common/compile_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/joho/godotenv" + "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -193,16 +194,17 @@ func TestWarnGOTOOLCHAIN(t *testing.T) { } logger := testutil.NewTestLogger() + v := viper.New() if tc.envFileContent != nil { dir := t.TempDir() envPath := filepath.Join(dir, ".env.public") require.NoError(t, godotenv.Write(tc.envFileContent, envPath)) - settings.LoadPublicEnv(logger, envPath) + settings.LoadPublicEnv(logger, v, envPath) for k := range tc.envFileContent { t.Cleanup(func() { os.Unsetenv(k) }) } } else { - settings.LoadPublicEnv(logger, "") + settings.LoadPublicEnv(logger, v, "") } output := captureStderr(t, func() { diff --git a/cmd/root.go b/cmd/root.go index fe2cf42a..62dadcac 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -125,8 +125,11 @@ func newRootCommand() *cobra.Command { return fmt.Errorf("failed to bind flags: %w", err) } - settings.ResolveAndLoadEnv(log, v, settings.Flags.CliEnvFile.Name, constants.DefaultEnvFileName) - settings.ResolveAndLoadPublicEnv(log, v, settings.Flags.CliPublicEnvFile.Name, constants.DefaultPublicEnvFileName) + settings.ResolveAndLoadBothEnvFiles( + log, v, + settings.Flags.CliEnvFile.Name, constants.DefaultEnvFileName, + settings.Flags.CliPublicEnvFile.Name, constants.DefaultPublicEnvFileName, + ) // Update log level if verbose flag is set — must happen before spinner starts if verbose := v.GetBool(settings.Flags.Verbose.Name); verbose { diff --git a/internal/settings/settings.go b/internal/settings/settings.go index ccd2e9a9..580b6940 100644 --- a/internal/settings/settings.go +++ b/internal/settings/settings.go @@ -141,73 +141,111 @@ func loadEnvFile(logger *zerolog.Logger, envPath string) (string, map[string]str // resolveEnvPath checks the Viper flag; if empty, auto-discovers the file by // walking up the directory tree from the current working directory. -func resolveEnvPath(v *viper.Viper, flagName, defaultFileName string) string { +// Returns the resolved path and whether it was explicitly set via the CLI flag. +func resolveEnvPath(v *viper.Viper, flagName, defaultFileName string) (string, bool) { p := v.GetString(flagName) - if p == "" { - if found, err := FindEnvFile(".", defaultFileName); err == nil { - p = found - } + if p != "" { + return p, true + } + if found, err := FindEnvFile(".", defaultFileName); err == nil { + return found, false } - return p + return "", false } // LoadEnv loads environment variables from envPath into the process -// environment, binds sensitive variables into Viper, and logs outcomes. -// If envPath is empty no file is loaded and a debug message is emitted. +// environment, then binds all loaded variables plus the sensitive defaults +// into Viper. AutomaticEnv is always activated so every OS env var is +// reachable via Viper regardless of whether a file was loaded. // Errors are logged but do not halt execution — the CLI continues so // that commands which don't need the env file can still run. func LoadEnv(logger *zerolog.Logger, v *viper.Viper, envPath string) { loadedEnvFilePath = "" loadedEnvVars = nil loadedEnvFilePath, loadedEnvVars = loadEnvFile(logger, envPath) - - if loadedEnvFilePath != "" { - if err := bindEnv(v); err != nil { - logger.Error().Err(err).Msg( - "Not able to bind environment variables that represent sensitive data. " + - "They are required for the CLI tool to function properly, without them some commands may not work. " + - "Please export them manually or set via .env file (check example.env for more information).") - } - } + bindAllVars(v, loadedEnvVars, EthPrivateKeyEnvVar, CreTargetEnvVar) } -// LoadPublicEnv loads variables from envPath into the process environment. -// Unlike LoadEnv it does not bind sensitive variables into Viper — it is -// intended for non-sensitive, shared build configuration (e.g. GOTOOLCHAIN). -func LoadPublicEnv(logger *zerolog.Logger, envPath string) { +// LoadPublicEnv loads variables from envPath into the process environment +// and binds all loaded variables into Viper. It is intended for non-sensitive, +// shared build configuration (e.g. GOTOOLCHAIN). +func LoadPublicEnv(logger *zerolog.Logger, v *viper.Viper, envPath string) { loadedPublicEnvFilePath = "" loadedPublicEnvVars = nil loadedPublicEnvFilePath, loadedPublicEnvVars = loadEnvFile(logger, envPath) + bindAllVars(v, loadedPublicEnvVars) } // ResolveAndLoadEnv resolves the .env file path from the given CLI flag // (auto-detecting defaultFileName in parent dirs if the flag is empty), -// then loads it and binds sensitive variables into Viper. +// logs a debug message when the flag was not explicitly set, then loads +// the file and binds all variables into Viper. func ResolveAndLoadEnv(logger *zerolog.Logger, v *viper.Viper, flagName, defaultFileName string) { - LoadEnv(logger, v, resolveEnvPath(v, flagName, defaultFileName)) + path, explicit := resolveEnvPath(v, flagName, defaultFileName) + if !explicit && path != "" { + logger.Debug(). + Str("default", defaultFileName). + Str("path", path). + Msg("--env not specified; using auto-discovered file") + } + LoadEnv(logger, v, path) } // ResolveAndLoadPublicEnv resolves the public env file path from the given // CLI flag (auto-detecting defaultFileName in parent dirs if the flag is -// empty), then loads it into the process environment. +// empty), logs a debug message when the flag was not explicitly set, then +// loads the file and binds all variables into Viper. func ResolveAndLoadPublicEnv(logger *zerolog.Logger, v *viper.Viper, flagName, defaultFileName string) { - LoadPublicEnv(logger, resolveEnvPath(v, flagName, defaultFileName)) -} - -func bindEnv(v *viper.Viper) error { - envVars := []string{ - EthPrivateKeyEnvVar, - CreTargetEnvVar, + path, explicit := resolveEnvPath(v, flagName, defaultFileName) + if !explicit && path != "" { + logger.Debug(). + Str("default", defaultFileName). + Str("path", path). + Msg("--public-env not specified; using auto-discovered file") } + LoadPublicEnv(logger, v, path) +} - for _, variable := range envVars { - if err := v.BindEnv(variable); err != nil { - return fmt.Errorf("failed to bind environment variable: %s", variable) +// ResolveAndLoadBothEnvFiles resolves, loads, and binds variables from both +// the .env and .env.public files, applying the following rules: +// +// 1. If a flag is not explicitly set, a debug message is emitted; if the +// default file is found it is loaded automatically. +// 2. Variables are prioritized: public-env > env file > other OS vars. +// A warning is emitted for any key present in both files. +// 3. All loaded variables from both files are bound into Viper. +func ResolveAndLoadBothEnvFiles( + logger *zerolog.Logger, + v *viper.Viper, + envFlagName, envDefaultFile string, + publicEnvFlagName, publicEnvDefaultFile string, +) { + // Load .env first (lower priority); public env loaded second overrides it. + ResolveAndLoadEnv(logger, v, envFlagName, envDefaultFile) + ResolveAndLoadPublicEnv(logger, v, publicEnvFlagName, publicEnvDefaultFile) + + // Rule 2: warn for keys present in both files. + for key := range loadedPublicEnvVars { + if _, inEnv := loadedEnvVars[key]; inEnv { + logger.Warn(). + Str("key", key). + Str("env", envDefaultFile). + Str("public-env", publicEnvDefaultFile). + Msgf("%s is defined in both env files; %s takes precedence", key, publicEnvDefaultFile) } } +} +// bindAllVars activates AutomaticEnv on v, explicitly binds every key in +// vars, and also binds any additional named keys supplied via extra. +func bindAllVars(v *viper.Viper, vars map[string]string, extra ...string) { v.AutomaticEnv() - return nil + for key := range vars { + _ = v.BindEnv(key) + } + for _, key := range extra { + _ = v.BindEnv(key) + } } // FindEnvFile walks up from startDir looking for a file named fileName. diff --git a/internal/settings/settings_load_test.go b/internal/settings/settings_load_test.go index f5250dba..0915c17f 100644 --- a/internal/settings/settings_load_test.go +++ b/internal/settings/settings_load_test.go @@ -257,3 +257,195 @@ func TestLoadEnvStateResetsBetweenCalls(t *testing.T) { assert.Empty(t, settings.LoadedEnvFilePath(), "state should be reset on subsequent call") assert.Nil(t, settings.LoadedEnvVars(), "state should be reset on subsequent call") } + +func TestResolveAndLoadBothEnvFiles(t *testing.T) { + callBoth := func(logger *zerolog.Logger, v *viper.Viper) { + settings.ResolveAndLoadBothEnvFiles( + logger, v, + settings.Flags.CliEnvFile.Name, constants.DefaultEnvFileName, + settings.Flags.CliPublicEnvFile.Name, constants.DefaultPublicEnvFileName, + ) + } + + writeFile := func(t *testing.T, path string, vars map[string]string) { + t.Helper() + require.NoError(t, godotenv.Write(vars, path)) + } + + t.Run("flag unspecified file auto discovered debug log emitted", func(t *testing.T) { + tempDir := t.TempDir() + writeFile(t, filepath.Join(tempDir, constants.DefaultEnvFileName), map[string]string{"ENV_AD": "env-val"}) + writeFile(t, filepath.Join(tempDir, constants.DefaultPublicEnvFileName), map[string]string{"PUB_AD": "pub-val"}) + t.Cleanup(func() { os.Unsetenv("ENV_AD"); os.Unsetenv("PUB_AD") }) + + restoreWD, err := testutil.ChangeWorkingDirectory(tempDir) + require.NoError(t, err) + defer restoreWD() + + logger, buf := newBufferLogger() + v := viper.New() + callBoth(logger, v) + + logOutput := buf.String() + assert.Contains(t, logOutput, "--env not specified") + assert.Contains(t, logOutput, "--public-env not specified") + assert.Contains(t, logOutput, "auto-discovered") + + assert.Equal(t, "env-val", os.Getenv("ENV_AD")) + assert.Equal(t, "pub-val", os.Getenv("PUB_AD")) + assert.Equal(t, "env-val", v.GetString("ENV_AD")) + assert.Equal(t, "pub-val", v.GetString("PUB_AD")) + }) + + t.Run("flag unspecified file not found debug log emitted", func(t *testing.T) { + tempDir := t.TempDir() + restoreWD, err := testutil.ChangeWorkingDirectory(tempDir) + require.NoError(t, err) + defer restoreWD() + + logger, buf := newBufferLogger() + v := viper.New() + callBoth(logger, v) + + logOutput := buf.String() + assert.Contains(t, logOutput, "No environment file specified") + assert.Contains(t, logOutput, "MUST be exported") + assert.Empty(t, settings.LoadedEnvFilePath()) + assert.Empty(t, settings.LoadedPublicEnvFilePath()) + }) + + t.Run("explicit flags no unspecified debug log", func(t *testing.T) { + tempDir := t.TempDir() + envPath := filepath.Join(tempDir, "my.env") + pubPath := filepath.Join(tempDir, "my.env.public") + writeFile(t, envPath, map[string]string{"E_EXPL": "1"}) + writeFile(t, pubPath, map[string]string{"P_EXPL": "2"}) + t.Cleanup(func() { os.Unsetenv("E_EXPL"); os.Unsetenv("P_EXPL") }) + + logger, buf := newBufferLogger() + v := viper.New() + v.Set(settings.Flags.CliEnvFile.Name, envPath) + v.Set(settings.Flags.CliPublicEnvFile.Name, pubPath) + callBoth(logger, v) + + logOutput := buf.String() + assert.NotContains(t, logOutput, "not specified") + assert.NotContains(t, logOutput, "auto-discovered") + + assert.Equal(t, "1", os.Getenv("E_EXPL")) + assert.Equal(t, "2", os.Getenv("P_EXPL")) + assert.Equal(t, "1", v.GetString("E_EXPL")) + assert.Equal(t, "2", v.GetString("P_EXPL")) + }) + + t.Run("invalid file path error logged", func(t *testing.T) { + logger, buf := newBufferLogger() + v := viper.New() + v.Set(settings.Flags.CliEnvFile.Name, "/nonexistent/.env") + callBoth(logger, v) + + logOutput := buf.String() + assert.Contains(t, logOutput, "Not able to load configuration from environment file") + assert.Contains(t, logOutput, "dotenvx.com/docs/env-file") + assert.Empty(t, settings.LoadedEnvFilePath()) + assert.Nil(t, settings.LoadedEnvVars()) + }) + + t.Run("public env overrides env file for same key and warns", func(t *testing.T) { + tempDir := t.TempDir() + writeFile(t, filepath.Join(tempDir, constants.DefaultEnvFileName), map[string]string{"PRIO_VAR": "from-env"}) + writeFile(t, filepath.Join(tempDir, constants.DefaultPublicEnvFileName), map[string]string{"PRIO_VAR": "from-public"}) + t.Cleanup(func() { os.Unsetenv("PRIO_VAR") }) + + restoreWD, err := testutil.ChangeWorkingDirectory(tempDir) + require.NoError(t, err) + defer restoreWD() + + logger, buf := newBufferLogger() + v := viper.New() + callBoth(logger, v) + + assert.Equal(t, "from-public", os.Getenv("PRIO_VAR")) + assert.Equal(t, "from-public", v.GetString("PRIO_VAR")) + + logOutput := buf.String() + assert.Contains(t, logOutput, "PRIO_VAR") + assert.Contains(t, logOutput, "defined in both") + assert.Contains(t, logOutput, constants.DefaultPublicEnvFileName) + }) + + t.Run("env file overrides pre existing os vars", func(t *testing.T) { + t.Setenv("OS_VAR", "from-shell") + + tempDir := t.TempDir() + writeFile(t, filepath.Join(tempDir, constants.DefaultEnvFileName), map[string]string{"OS_VAR": "from-env-file"}) + t.Cleanup(func() { os.Unsetenv("OS_VAR") }) + + restoreWD, err := testutil.ChangeWorkingDirectory(tempDir) + require.NoError(t, err) + defer restoreWD() + + logger, buf := newBufferLogger() + v := viper.New() + callBoth(logger, v) + + assert.Equal(t, "from-env-file", os.Getenv("OS_VAR")) + assert.Equal(t, "from-env-file", v.GetString("OS_VAR")) + assert.NotContains(t, buf.String(), "level\":\"error\"") + }) + + t.Run("no warning when keys are distinct", func(t *testing.T) { + tempDir := t.TempDir() + writeFile(t, filepath.Join(tempDir, constants.DefaultEnvFileName), map[string]string{"ONLY_ENV": "e"}) + writeFile(t, filepath.Join(tempDir, constants.DefaultPublicEnvFileName), map[string]string{"ONLY_PUB": "p"}) + t.Cleanup(func() { os.Unsetenv("ONLY_ENV"); os.Unsetenv("ONLY_PUB") }) + + restoreWD, err := testutil.ChangeWorkingDirectory(tempDir) + require.NoError(t, err) + defer restoreWD() + + logger, buf := newBufferLogger() + v := viper.New() + callBoth(logger, v) + + assert.NotContains(t, buf.String(), "defined in both") + assert.Equal(t, "e", os.Getenv("ONLY_ENV")) + assert.Equal(t, "p", os.Getenv("ONLY_PUB")) + }) + + t.Run("all vars from both files accessible via viper", func(t *testing.T) { + tempDir := t.TempDir() + writeFile(t, filepath.Join(tempDir, constants.DefaultEnvFileName), map[string]string{ + "CUSTOM_ENV_VAR": "env-value", + settings.EthPrivateKeyEnvVar: "abc123", + settings.CreTargetEnvVar: "staging", + }) + writeFile(t, filepath.Join(tempDir, constants.DefaultPublicEnvFileName), map[string]string{ + "CUSTOM_PUB_VAR": "pub-value", + "GOTOOLCHAIN": "go1.25.3", + }) + t.Cleanup(func() { + for _, k := range []string{ + "CUSTOM_ENV_VAR", "CUSTOM_PUB_VAR", "GOTOOLCHAIN", + settings.EthPrivateKeyEnvVar, settings.CreTargetEnvVar, + } { + os.Unsetenv(k) + } + }) + + restoreWD, err := testutil.ChangeWorkingDirectory(tempDir) + require.NoError(t, err) + defer restoreWD() + + logger, buf := newBufferLogger() + v := viper.New() + callBoth(logger, v) + + assert.Equal(t, "env-value", v.GetString("CUSTOM_ENV_VAR")) + assert.Equal(t, "abc123", v.GetString(settings.EthPrivateKeyEnvVar)) + assert.Equal(t, "staging", v.GetString(settings.CreTargetEnvVar)) + assert.Equal(t, "pub-value", v.GetString("CUSTOM_PUB_VAR")) + assert.Equal(t, "go1.25.3", v.GetString("GOTOOLCHAIN")) + assert.NotContains(t, buf.String(), "\"level\":\"error\"") + }) +}