fix: RESTIC_PASSWORD env shold not take precedent over repo passwords

internal/config/migrations/003relativescheduling.go

@@ -5,7 +5,7 @@ import (
 	"go.uber.org/zap"
 )
 
-var migration003RelativeScheduling = func(config *v1.Config) {
+var migration003RelativeScheduling = func(config *v1.Config) error {
 	zap.L().Info("applying config migration 003: relative scheduling")
 	// loop over plans and examine prune policy's
 	for _, repo := range config.Repos {
@@ -22,4 +22,5 @@ var migration003RelativeScheduling = func(config *v1.Config) {
 			schedule.Clock = v1.Schedule_CLOCK_LAST_RUN_TIME
 		}
 	}
+	return nil
 }

internal/config/migrations/004repoguid.go

@@ -7,7 +7,7 @@ import (
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
 )
 
-var migration004RepoGuid = func(config *v1.Config) {
+var migration004RepoGuid = func(config *v1.Config) error {
 	for _, repo := range config.Repos {
 		if repo.Guid != "" {
 			continue
@@ -16,4 +16,5 @@ var migration004RepoGuid = func(config *v1.Config) {
 		h.Write([]byte(repo.Id))
 		repo.Guid = hex.EncodeToString(h.Sum(nil))
 	}
+	return nil
 }

internal/config/migrations/005checkrepopasswords.go

@@ -0,0 +1,127 @@
+package migrations
+
+import (
+	"fmt"
+	"os"
+	"strings"
+
+	v1 "github.com/garethgeorge/backrest/gen/go/v1"
+	"go.uber.org/zap"
+)
+
+var conflictingEnvVars = []string{
+	"RESTIC_PASSWORD",
+	"RESTIC_PASSWORD_FILE",
+	"RESTIC_PASSWORD_COMMAND",
+}
+
+// migration005CheckRepoPasswords detects repos whose restic password may have
+// been set incorrectly due to a bug fixed in this release
+// (https://github.com/garethgeorge/backrest/issues/1139).
+//
+// The bug: RESTIC_PASSWORD, RESTIC_PASSWORD_FILE, and RESTIC_PASSWORD_COMMAND
+// inherited from the process environment previously took precedence over the
+// password configured in Backrest's UI, because WithEnviron() was appended
+// after the config password. Repos initialized or used under those conditions
+// were encrypted with the env var's password, not the one the user entered.
+//
+// On first run after upgrading, if any repo has a config password AND one of
+// the above env vars is set, this migration returns an error and refuses to
+// start, printing instructions for the user.
+//
+// If ISSUE_1139_FIX_PASSWORDS=1 is set in the environment, the migration
+// instead applies the following automatic fixes to each affected repo and
+// writes the corrected config to disk before startup continues:
+//
+//   - RESTIC_PASSWORD: the env var's value is written into repo.Password,
+//     replacing the (likely wrong) value the user had entered in the UI.
+//
+//   - RESTIC_PASSWORD_FILE or RESTIC_PASSWORD_COMMAND: repo.Password is
+//     cleared and the env var (name=value) is appended to repo.Env, so
+//     restic continues to resolve the password via the same mechanism it
+//     was using before. The env var can then be removed from the Backrest
+//     process environment, as the password source is now explicit in the
+//     repo config.
+//
+// After this migration runs successfully (whether or not a fix was needed)
+// the config version is bumped and it will not run again on future starts.
+var migration005CheckRepoPasswords = func(config *v1.Config) error {
+	// Find repos that have a password configured in the UI.
+	var affectedRepos []*v1.Repo
+	for _, repo := range config.Repos {
+		if repo.GetPassword() != "" {
+			affectedRepos = append(affectedRepos, repo)
+		}
+	}
+	if len(affectedRepos) == 0 {
+		return nil
+	}
+
+	// Check which conflicting env vars are set.
+	var setEnvVars []string
+	for _, envVar := range conflictingEnvVars {
+		if os.Getenv(envVar) != "" {
+			setEnvVars = append(setEnvVars, envVar)
+		}
+	}
+	if len(setEnvVars) == 0 {
+		return nil
+	}
+
+	// There is a potential conflict. Check if the user has acknowledged it.
+	if os.Getenv("ISSUE_1139_FIX_PASSWORDS") != "" {
+		for _, repo := range affectedRepos {
+			for _, envVar := range setEnvVars {
+				val := os.Getenv(envVar)
+				switch envVar {
+				case "RESTIC_PASSWORD":
+					zap.S().Warnf("repo %q: overwriting config password with value of RESTIC_PASSWORD env var (issue 1139 fix)", repo.Id)
+					repo.Password = val
+				case "RESTIC_PASSWORD_FILE", "RESTIC_PASSWORD_COMMAND":
+					zap.S().Warnf("repo %q: clearing config password and adding %s=%s to repo env (issue 1139 fix)", repo.Id, envVar, val)
+					repo.Password = ""
+					repo.Env = append(repo.Env, envVar+"="+val)
+				}
+			}
+		}
+		return nil
+	}
+
+	// Block startup with a detailed error message.
+	var b strings.Builder
+	b.WriteString("IMPORTANT: Backrest detected a potential password conflict affecting your restic repos (see https://github.com/garethgeorge/backrest/issues/1139).\n")
+	b.WriteString("\n")
+	b.WriteString("What happened:\n")
+	b.WriteString("  Backrest was using the wrong password for restic repos. The environment variables listed\n")
+	b.WriteString("  below were inherited by Backrest's process and previously took precedence over the password\n")
+	b.WriteString("  you configured in the UI. Your repos may have been encrypted with a different password than\n")
+	b.WriteString("  what your Backrest config says.\n")
+	b.WriteString("\n")
+	b.WriteString("Conflicting environment variables found:\n")
+	for _, envVar := range setEnvVars {
+		fmt.Fprintf(&b, "  - %s\n", envVar)
+	}
+	b.WriteString("\n")
+	b.WriteString("Potentially affected repos:\n")
+	for _, repo := range affectedRepos {
+		fmt.Fprintf(&b, "  - %s\n", repo.Id)
+	}
+	b.WriteString("\n")
+	b.WriteString("How to fix:\n")
+	b.WriteString("  Rerun Backrest with ISSUE_1139_FIX_PASSWORDS=1 set in the environment.\n")
+	b.WriteString("  Backrest will automatically update each affected repo's config as follows,\n")
+	b.WriteString("  then write the corrected config to disk and start normally:\n")
+	b.WriteString("\n")
+	b.WriteString("    RESTIC_PASSWORD:         the env var's value is written into the repo's\n")
+	b.WriteString("                             password field, replacing the value entered in the UI.\n")
+	b.WriteString("\n")
+	b.WriteString("    RESTIC_PASSWORD_FILE /   the repo's password field is cleared and the env var\n")
+	b.WriteString("    RESTIC_PASSWORD_COMMAND: (name=value) is appended to the repo's env list, so\n")
+	b.WriteString("                             restic continues to resolve the password the same way.\n")
+	b.WriteString("\n")
+	b.WriteString("  Once the fix has been applied you can remove the conflicting environment variable\n")
+	b.WriteString("  from your Backrest process environment — the password source will be stored\n")
+	b.WriteString("  explicitly in the repo config from that point on.\n")
+
+	return fmt.Errorf("%s", b.String())
+}

internal/config/migrations/005checkrepopasswords_test.go

@@ -0,0 +1,199 @@
+package migrations
+
+import (
+	"strings"
+	"testing"
+
+	v1 "github.com/garethgeorge/backrest/gen/go/v1"
+)
+
+func TestMigration005CheckRepoPasswords(t *testing.T) {
+	// Cannot use t.Parallel() because subtests use t.Setenv.
+
+	tests := []struct {
+		name        string
+		config      func() *v1.Config   // factory so each run gets a fresh config
+		envVars     map[string]string    // env vars to set for the test
+		ackEnvVar   bool                 // whether to set ISSUE_1139_FIX_PASSWORDS=1
+		wantErr     bool
+		errContains string              // substring that must appear in the error
+		checkConfig func(*testing.T, *v1.Config) // optional post-migration assertions
+	}{
+		{
+			name: "no repos with passwords - no conflict",
+			config: func() *v1.Config {
+				return &v1.Config{Repos: []*v1.Repo{{Id: "r", Uri: "/tmp/r"}}}
+			},
+			envVars: map[string]string{"RESTIC_PASSWORD": "somevalue"},
+			wantErr: false,
+		},
+		{
+			name: "repos with passwords but no conflicting env vars - no conflict",
+			config: func() *v1.Config {
+				return &v1.Config{Repos: []*v1.Repo{{Id: "r", Uri: "/tmp/r", Password: "secret"}}}
+			},
+			envVars: map[string]string{},
+			wantErr: false,
+		},
+		{
+			name: "RESTIC_PASSWORD conflict blocks startup without ack",
+			config: func() *v1.Config {
+				return &v1.Config{Repos: []*v1.Repo{{Id: "my-repo", Uri: "/tmp/r", Password: "wrong"}}}
+			},
+			envVars:     map[string]string{"RESTIC_PASSWORD": "envpassword"},
+			wantErr:     true,
+			errContains: "RESTIC_PASSWORD",
+		},
+		{
+			name: "RESTIC_PASSWORD_FILE conflict blocks startup without ack",
+			config: func() *v1.Config {
+				return &v1.Config{Repos: []*v1.Repo{{Id: "r", Uri: "/tmp/r", Password: "wrong"}}}
+			},
+			envVars:     map[string]string{"RESTIC_PASSWORD_FILE": "/run/secrets/restic"},
+			wantErr:     true,
+			errContains: "RESTIC_PASSWORD_FILE",
+		},
+		{
+			name: "RESTIC_PASSWORD_COMMAND conflict blocks startup without ack",
+			config: func() *v1.Config {
+				return &v1.Config{Repos: []*v1.Repo{{Id: "r", Uri: "/tmp/r", Password: "wrong"}}}
+			},
+			envVars:     map[string]string{"RESTIC_PASSWORD_COMMAND": "cat /secrets/pw"},
+			wantErr:     true,
+			errContains: "RESTIC_PASSWORD_COMMAND",
+		},
+		{
+			name: "error message names the affected repo",
+			config: func() *v1.Config {
+				return &v1.Config{Repos: []*v1.Repo{{Id: "my-repo", Uri: "/tmp/r", Password: "wrong"}}}
+			},
+			envVars:     map[string]string{"RESTIC_PASSWORD": "envpassword"},
+			wantErr:     true,
+			errContains: "my-repo",
+		},
+		{
+			name: "error message references ISSUE_1139_FIX_PASSWORDS",
+			config: func() *v1.Config {
+				return &v1.Config{Repos: []*v1.Repo{{Id: "r", Uri: "/tmp/r", Password: "wrong"}}}
+			},
+			envVars:     map[string]string{"RESTIC_PASSWORD": "envpassword"},
+			wantErr:     true,
+			errContains: "ISSUE_1139_FIX_PASSWORDS",
+		},
+		{
+			name: "RESTIC_PASSWORD: ack inlines value into repo.Password",
+			config: func() *v1.Config {
+				return &v1.Config{Repos: []*v1.Repo{{Id: "r", Uri: "/tmp/r", Password: "wrong"}}}
+			},
+			envVars:   map[string]string{"RESTIC_PASSWORD": "real-password"},
+			ackEnvVar: true,
+			wantErr:   false,
+			checkConfig: func(t *testing.T, cfg *v1.Config) {
+				t.Helper()
+				if got := cfg.Repos[0].Password; got != "real-password" {
+					t.Errorf("expected repo.Password = %q, got %q", "real-password", got)
+				}
+			},
+		},
+		{
+			name: "RESTIC_PASSWORD_FILE: ack moves to repo.Env and clears Password",
+			config: func() *v1.Config {
+				return &v1.Config{Repos: []*v1.Repo{{Id: "r", Uri: "/tmp/r", Password: "wrong"}}}
+			},
+			envVars:   map[string]string{"RESTIC_PASSWORD_FILE": "/run/secrets/pw"},
+			ackEnvVar: true,
+			wantErr:   false,
+			checkConfig: func(t *testing.T, cfg *v1.Config) {
+				t.Helper()
+				repo := cfg.Repos[0]
+				if repo.Password != "" {
+					t.Errorf("expected repo.Password to be cleared, got %q", repo.Password)
+				}
+				wantEnv := "RESTIC_PASSWORD_FILE=/run/secrets/pw"
+				for _, e := range repo.Env {
+					if e == wantEnv {
+						return
+					}
+				}
+				t.Errorf("expected repo.Env to contain %q, got %v", wantEnv, repo.Env)
+			},
+		},
+		{
+			name: "RESTIC_PASSWORD_COMMAND: ack moves to repo.Env and clears Password",
+			config: func() *v1.Config {
+				return &v1.Config{Repos: []*v1.Repo{{Id: "r", Uri: "/tmp/r", Password: "wrong"}}}
+			},
+			envVars:   map[string]string{"RESTIC_PASSWORD_COMMAND": "cat /secrets/pw"},
+			ackEnvVar: true,
+			wantErr:   false,
+			checkConfig: func(t *testing.T, cfg *v1.Config) {
+				t.Helper()
+				repo := cfg.Repos[0]
+				if repo.Password != "" {
+					t.Errorf("expected repo.Password to be cleared, got %q", repo.Password)
+				}
+				wantEnv := "RESTIC_PASSWORD_COMMAND=cat /secrets/pw"
+				for _, e := range repo.Env {
+					if e == wantEnv {
+						return
+					}
+				}
+				t.Errorf("expected repo.Env to contain %q, got %v", wantEnv, repo.Env)
+			},
+		},
+		{
+			name: "only repos with passwords are considered affected",
+			config: func() *v1.Config {
+				return &v1.Config{Repos: []*v1.Repo{
+					{Id: "no-pass", Uri: "/tmp/r1"},
+					{Id: "with-pass", Uri: "/tmp/r2", Password: "wrong"},
+				}}
+			},
+			envVars:     map[string]string{"RESTIC_PASSWORD": "envpassword"},
+			wantErr:     true,
+			errContains: "with-pass",
+		},
+		{
+			name: "no repos at all - no conflict",
+			config: func() *v1.Config {
+				return &v1.Config{}
+			},
+			envVars: map[string]string{"RESTIC_PASSWORD": "envpassword"},
+			wantErr: false,
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			for k, v := range tc.envVars {
+				t.Setenv(k, v)
+			}
+			// Ensure conflicting env vars not in tc.envVars are unset.
+			for _, k := range conflictingEnvVars {
+				if _, ok := tc.envVars[k]; !ok {
+					t.Setenv(k, "")
+				}
+			}
+			if tc.ackEnvVar {
+				t.Setenv("ISSUE_1139_FIX_PASSWORDS", "1")
+			} else {
+				t.Setenv("ISSUE_1139_FIX_PASSWORDS", "")
+			}
+
+			cfg := tc.config()
+			err := migration005CheckRepoPasswords(cfg)
+			if (err != nil) != tc.wantErr {
+				t.Errorf("migration005CheckRepoPasswords() error = %v, wantErr %v", err, tc.wantErr)
+			}
+			if tc.errContains != "" && err != nil {
+				if !strings.Contains(err.Error(), tc.errContains) {
+					t.Errorf("expected error to contain %q, got: %s", tc.errContains, err.Error())
+				}
+			}
+			if tc.checkConfig != nil {
+				tc.checkConfig(t, cfg)
+			}
+		})
+	}
+}

internal/config/migrations/migrations.go

@@ -8,11 +8,12 @@ import (
 	"google.golang.org/protobuf/proto"
 )
 
-var migrations = []*func(*v1.Config){
+var migrations = []*func(*v1.Config) error{
 	&noop, // migration001PrunePolicy is deprecated
 	&noop, // migration002Schedules is deprecated
 	&migration003RelativeScheduling,
 	&migration004RepoGuid,
+	&migration005CheckRepoPasswords,
 }
 
 var CurrentVersion = int32(len(migrations))
@@ -39,13 +40,16 @@ func ApplyMigrations(config *v1.Config) error {
 		if m == &noop {
 			return fmt.Errorf("config version %d is too old to migrate, please try first upgrading to backrest 1.4.0 which is the last version that may be compatible with your config", config.Version)
 		}
-		(*m)(config)
+		if err := (*m)(config); err != nil {
+			return err
+		}
 	}
 	config.Version = CurrentVersion
 
 	return nil
 }
 
-var noop = func(config *v1.Config) {
+var noop = func(config *v1.Config) error {
 	// do nothing
+	return nil
 }

internal/orchestrator/repo/repo_test.go

@@ -371,6 +371,48 @@ func initRepoHelper(t *testing.T, config *v1.Config, repo *v1.Repo) *RepoOrchest
 	return orchestrator
 }
 
+// TestConfigPasswordPrecedence verifies that a password set in the Backrest
+// repo config takes precedence over RESTIC_PASSWORD (and related env vars) set
+// in the process environment. This is a regression test for
+// https://github.com/garethgeorge/backrest/issues/1139.
+func TestConfigPasswordPrecedence(t *testing.T) {
+	// Cannot use t.Parallel() because t.Setenv is used.
+
+	repoDir := t.TempDir()
+	configPassword := "config-password"
+	wrongEnvPassword := "env-password-should-be-ignored"
+
+	repo := &v1.Repo{
+		Id:       "test",
+		Uri:      repoDir,
+		Password: configPassword,
+		Flags:    []string{"--no-cache"},
+	}
+
+	// Initialize the repo using the config password.
+	orchestrator := initRepoHelper(t, configForTest, repo)
+
+	// Now set RESTIC_PASSWORD in the environment to a *different* value.
+	// NewRepoOrchestrator should still use the config password, not this one.
+	t.Setenv("RESTIC_PASSWORD", wrongEnvPassword)
+	t.Setenv("RESTIC_PASSWORD_FILE", "")
+	t.Setenv("RESTIC_PASSWORD_COMMAND", "")
+
+	// Re-create the orchestrator so it picks up the current environment.
+	orchestrator2, err := NewRepoOrchestrator(configForTest, repo, helpers.ResticBinary(t))
+	if err != nil {
+		t.Fatalf("failed to create repo orchestrator: %v", err)
+	}
+
+	// Listing snapshots exercises the repo unlock path and will fail with
+	// "wrong password" if the env var is incorrectly taking precedence.
+	if _, err := orchestrator2.Snapshots(context.Background()); err != nil {
+		t.Fatalf("Snapshots() failed — config password did not take precedence over RESTIC_PASSWORD env var: %v", err)
+	}
+
+	_ = orchestrator // suppress unused warning
+}
+
 func TestRestoreAmbiguity(t *testing.T) {
 	t.Parallel()
 	repoDir := t.TempDir()