Fixed 1Password op:// secrets being clobbered by .env loading

cmd/root.go

@@ -127,13 +127,7 @@ func newRootCommand() *cobra.Command {
 				return fmt.Errorf("failed to bind flags: %w", err)
 			}
 
-			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
+			// Update log level if verbose flag is set — must happen before everything else
 			if verbose := v.GetBool(settings.Flags.Verbose.Name); verbose {
 				ui.SetVerbose(true)
 				newLogger := log.Level(zerolog.DebugLevel)
@@ -144,6 +138,14 @@ func newRootCommand() *cobra.Command {
 				runtimeContext.ClientFactory = client.NewFactory(&newLogger, v)
 			}
 
+			log = runtimeContext.Logger
+
+			settings.ResolveAndLoadBothEnvFiles(
+				log, v,
+				settings.Flags.CliEnvFile.Name, constants.DefaultEnvFileName,
+				settings.Flags.CliPublicEnvFile.Name, constants.DefaultPublicEnvFileName,
+			)
+
 			// Start the global spinner for commands that do initialization work
 			spinner := ui.GlobalSpinner()
 			showSpinner := shouldShowSpinner(cmd)

internal/settings/settings.go

@@ -115,8 +115,11 @@ func New(logger *zerolog.Logger, v *viper.Viper, cmd *cobra.Command, registryCha
 	}, nil
 }
 
-// loadEnvFile loads the file at envPath into the process environment via
-// godotenv.Overload and returns the path + parsed vars on success.
+// loadEnvFile loads the file at envPath into the process environment and
+// returns the path + parsed vars on success.
+// Plain values override any pre-existing shell env var (Overload semantics).
+// Values beginning with "op://" are intentionally skipped so that secrets
+// resolved by `op run` are not clobbered by the unresolved reference.
 // If envPath is empty or loading fails, appropriate messages are logged
 // and ("", nil) is returned.
 func loadEnvFile(logger *zerolog.Logger, envPath string) (string, map[string]string) {
@@ -127,15 +130,30 @@ func loadEnvFile(logger *zerolog.Logger, envPath string) (string, map[string]str
 		return "", nil
 	}
 
-	if err := godotenv.Overload(envPath); err != nil {
+	vars, err := godotenv.Read(envPath)
+	if err != nil {
 		logger.Error().Str("path", envPath).Err(err).Msg(
 			"Not able to load configuration from environment file. " +
 				"CLI tool will read and verify individual environment variables (they MUST be exported). " +
 				"If the file is present, please check that it follows the correct format: https://dotenvx.com/docs/env-file")
 		return "", nil
 	}
 
-	vars, _ := godotenv.Read(envPath)
+	for k, v := range vars {
+		if strings.HasPrefix(v, "op://") {
+			// Leave the value already in the environment (resolved by `op run`) untouched.
+			logger.Debug().Str("key", k).Msg(
+				fmt.Sprintf("Skipping op:// reference in env file; use `op run --env-file %s -- cre ...` to resolve 1Password secrets", envPath))
+			continue
+		}
+		err = os.Setenv(k, v)
+		if err != nil {
+			logger.Error().Str("key", k).Str("value", v).Err(err).Msg(
+				"Failed to set environment variable; CLI tool will read and verify individual environment variables (they MUST be exported). " +
+					"If the file is present, please check that it follows the correct format: https://dotenvx.com/docs/env-file")
+		}
+	}
+
 	return envPath, vars
 }
 

internal/settings/settings_load_test.go

@@ -236,6 +236,32 @@ func TestLoadEnvOverridesExistingEnv(t *testing.T) {
 	assert.Equal(t, "staging", v.GetString("CRE_TARGET"))
 }
 
+func TestLoadEnvSkipsOpURIButPreservesResolvedEnvVar(t *testing.T) {
+	// Simulate `op run` having already resolved the secret into the process env.
+	os.Setenv("MY_SECRET", "resolved-value")
+	t.Cleanup(func() { os.Unsetenv("MY_SECRET"); os.Unsetenv("PLAIN_VAR") })
+
+	tempDir := t.TempDir()
+	envFilePath := filepath.Join(tempDir, ".env")
+	require.NoError(t, godotenv.Write(map[string]string{
+		"MY_SECRET": "op://vault/item/field",
+		"PLAIN_VAR": "plain-value",
+	}, envFilePath))
+
+	logger := testutil.NewTestLogger()
+	v := viper.New()
+	settings.LoadEnv(logger, v, envFilePath)
+
+	// op:// reference must not clobber the value injected by op run.
+	assert.Equal(t, "resolved-value", os.Getenv("MY_SECRET"),
+		"op:// reference in .env should not overwrite the value resolved by op run")
+	// Plain values still override the shell env (Overload semantics preserved).
+	assert.Equal(t, "plain-value", os.Getenv("PLAIN_VAR"))
+	// loadedEnvVars still reflects the raw file content.
+	require.NotNil(t, settings.LoadedEnvVars())
+	assert.Equal(t, "op://vault/item/field", settings.LoadedEnvVars()["MY_SECRET"])
+}
+
 func TestLoadEnvStateResetsBetweenCalls(t *testing.T) {
 	tempDir := t.TempDir()
 	envFilePath := filepath.Join(tempDir, ".env")