diff --git a/cmd/proxyctl/main.go b/cmd/proxyctl/main.go index 7e53743..3e17e3c 100644 --- a/cmd/proxyctl/main.go +++ b/cmd/proxyctl/main.go @@ -23,11 +23,11 @@ func run(args []string, input io.Reader, output io.Writer) error { if len(args) != 0 { return fmt.Errorf("this build supports no command-line arguments") } - path, err := profile.ConfigPath() + configuration, err := profile.ResolveConfigurationForCurrentProcess() if err != nil { return err } - application, err := app.New(profile.NewFileStore(path)) + application, err := app.New(profile.NewOwnedFileStore(configuration.Path, configuration.Owner)) if err != nil { return err } diff --git a/go.mod b/go.mod index ac8dd42..da6282c 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,10 @@ module proxyctl go 1.24.0 -require github.com/charmbracelet/bubbletea v1.3.10 +require ( + github.com/charmbracelet/bubbletea v1.3.10 + golang.org/x/sys v0.36.0 +) require ( github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect @@ -21,6 +24,5 @@ require ( github.com/muesli/termenv v0.16.0 // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect - golang.org/x/sys v0.36.0 // indirect golang.org/x/text v0.3.8 // indirect ) diff --git a/internal/app/app.go b/internal/app/app.go index 520213f..be73baf 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -77,6 +77,19 @@ func (a *Application) Dispatch(intent Intent) Result { }) if err == nil { a.state = next + } else { + var committed *profile.CommitError + if errors.As(err, &committed) { + // Rename committed next before directory durability reporting failed. + // Prefer that state even if the required verification reload also fails. + a.state = next + reloaded, loadErr := a.store.Load() + if loadErr != nil { + err = errors.Join(err, fmt.Errorf("reload committed Profile configuration: %w", loadErr)) + } else { + a.state = reloaded + } + } } return Result{Snapshot: a.Snapshot(), Notice: notice, Err: err} } diff --git a/internal/app/storage_test.go b/internal/app/storage_test.go index bc0b512..6857d45 100644 --- a/internal/app/storage_test.go +++ b/internal/app/storage_test.go @@ -1,6 +1,7 @@ package app_test import ( + "errors" "io" "os" "path/filepath" @@ -71,6 +72,46 @@ func TestProfileConfigurationUsesIsolatedXDGPathOwnerOnlyAndAtomicReplacement(t } } +type committedErrorStore struct { + state profile.State + loadCalls int +} + +func (s *committedErrorStore) Load() (profile.State, error) { + s.loadCalls++ + return s.state, nil +} + +func (s *committedErrorStore) Mutate(change func(profile.State) (profile.State, error)) (profile.State, error) { + next, err := change(s.state) + if err != nil { + return s.state, err + } + s.state = next + return next, &profile.CommitError{Err: errors.New("injected directory sync failure")} +} + +func TestApplicationReloadsStateAfterCommittedDurabilityError(t *testing.T) { + store := &committedErrorStore{} + application, err := app.New(store) + if err != nil { + t.Fatal(err) + } + result := application.Dispatch(app.Intent{Kind: app.CreateProfile, Profile: profile.Profile{ + Name: "committed", Mode: profile.SingleMode, SingleURL: "http://proxy.example:8080", + }}) + var committed *profile.CommitError + if !errors.As(result.Err, &committed) { + t.Fatalf("mutation error = %v, want committed durability error", result.Err) + } + if store.loadCalls != 2 { + t.Fatalf("Load calls = %d, want startup plus post-commit reload", store.loadCalls) + } + if len(result.Snapshot.Profiles) != 1 || result.Snapshot.Profiles[0].Name != "committed" { + t.Fatalf("Application snapshot after committed error = %#v", result.Snapshot) + } +} + func TestConcurrentApplicationsMutateLatestStateWithoutLostUpdates(t *testing.T) { path := filepath.Join(t.TempDir(), "proxyctl", "profiles.json") first, err := app.New(profile.NewFileStore(path)) diff --git a/internal/profile/store.go b/internal/profile/store.go index 68b304f..0db3f3d 100644 --- a/internal/profile/store.go +++ b/internal/profile/store.go @@ -1,6 +1,8 @@ package profile import ( + "crypto/rand" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -9,21 +11,55 @@ import ( "os" "os/user" "path/filepath" + "strconv" "strings" - "syscall" + + "golang.org/x/sys/unix" ) type FileStore struct { - path string + path string + owner *UserIdentity + operations fileOperations +} + +type fileOperations interface { + SetOwner(*os.File, int, int) error + SyncDirectory(*os.File) error +} + +type systemFileOperations struct{} + +func (systemFileOperations) SetOwner(file *os.File, uid, gid int) error { + return file.Chown(uid, gid) +} + +func (systemFileOperations) SyncDirectory(directory *os.File) error { + return directory.Sync() } func NewFileStore(path string) *FileStore { - return &FileStore{path: path} + return &FileStore{path: path, operations: systemFileOperations{}} +} + +func NewOwnedFileStore(path string, owner UserIdentity) *FileStore { + return &FileStore{path: path, owner: &owner, operations: systemFileOperations{}} +} + +func newFileStore(path string, owner *UserIdentity, operations fileOperations) *FileStore { + return &FileStore{path: path, owner: owner, operations: operations} } type UserIdentity struct { Username string HomeDir string + UID int + GID int +} + +type Configuration struct { + Path string + Owner UserIdentity } type IdentityEnvironment interface { @@ -34,12 +70,24 @@ type IdentityEnvironment interface { type systemIdentityEnvironment struct{} +func identityFromUser(value *user.User) (UserIdentity, error) { + uid, err := strconv.Atoi(value.Uid) + if err != nil { + return UserIdentity{}, fmt.Errorf("parse uid %q: %w", value.Uid, err) + } + gid, err := strconv.Atoi(value.Gid) + if err != nil { + return UserIdentity{}, fmt.Errorf("parse gid %q: %w", value.Gid, err) + } + return UserIdentity{Username: value.Username, HomeDir: value.HomeDir, UID: uid, GID: gid}, nil +} + func (systemIdentityEnvironment) CurrentUser() (UserIdentity, error) { value, err := user.Current() if err != nil { return UserIdentity{}, err } - return UserIdentity{Username: value.Username, HomeDir: value.HomeDir}, nil + return identityFromUser(value) } func (systemIdentityEnvironment) LookupUser(name string) (UserIdentity, error) { @@ -47,7 +95,7 @@ func (systemIdentityEnvironment) LookupUser(name string) (UserIdentity, error) { if err != nil { return UserIdentity{}, err } - return UserIdentity{Username: value.Username, HomeDir: value.HomeDir}, nil + return identityFromUser(value) } func (systemIdentityEnvironment) LookupEnv(name string) (string, bool) { @@ -55,24 +103,32 @@ func (systemIdentityEnvironment) LookupEnv(name string) (string, bool) { } func ConfigPath() (string, error) { - return ConfigPathWith(systemIdentityEnvironment{}) + configuration, err := ResolveConfigurationForCurrentProcess() + return configuration.Path, err } -func ConfigPathWith(environment IdentityEnvironment) (string, error) { +func ResolveConfigurationForCurrentProcess() (Configuration, error) { + return ResolveConfiguration(systemIdentityEnvironment{}) +} + +func ResolveConfiguration(environment IdentityEnvironment) (Configuration, error) { owner, err := environment.CurrentUser() if err != nil { - return "", fmt.Errorf("identify configuration owner: %w", err) + return Configuration{}, fmt.Errorf("identify configuration owner: %w", err) } invokingUser := false if name, ok := environment.LookupEnv("SUDO_USER"); ok && name != "" && name != owner.Username { owner, err = environment.LookupUser(name) if err != nil { - return "", fmt.Errorf("identify invoking user %q: %w", name, err) + return Configuration{}, fmt.Errorf("identify invoking user %q: %w", name, err) } invokingUser = true } if owner.HomeDir == "" { - return "", errors.New("configuration owner has no home directory") + return Configuration{}, errors.New("configuration owner has no home directory") + } + if owner.UID < 0 || owner.GID < 0 { + return Configuration{}, errors.New("configuration owner has an invalid uid or gid") } configDir := "" @@ -84,7 +140,15 @@ func ConfigPathWith(environment IdentityEnvironment) (string, error) { if configDir == "" { configDir = filepath.Join(owner.HomeDir, ".config") } - return filepath.Join(configDir, "proxyctl", "profiles.json"), nil + return Configuration{ + Path: filepath.Join(configDir, "proxyctl", "profiles.json"), + Owner: owner, + }, nil +} + +func ConfigPathWith(environment IdentityEnvironment) (string, error) { + configuration, err := ResolveConfiguration(environment) + return configuration.Path, err } func pathWithin(path, parent string) bool { @@ -92,28 +156,37 @@ func pathWithin(path, parent string) bool { return err == nil && relative != ".." && !strings.HasPrefix(relative, ".."+string(filepath.Separator)) } +type CommitError struct { + Err error +} + +func (e *CommitError) Error() string { + return fmt.Sprintf("Profile configuration was committed but durability could not be confirmed: %v", e.Err) +} + +func (e *CommitError) Unwrap() error { return e.Err } + func (s *FileStore) Load() (State, error) { return s.load() } func (s *FileStore) Mutate(change func(State) (State, error)) (State, error) { - if err := s.secureDirectory(); err != nil { + directory, err := s.secureDirectory() + if err != nil { return State{}, err } - lock, err := os.OpenFile(s.path+".lock", os.O_CREATE|os.O_RDWR, 0o600) + defer directory.Close() + lock, err := s.openLockAt(directory) if err != nil { - return State{}, fmt.Errorf("open Profile configuration lock: %w", err) + return State{}, err } defer lock.Close() - if err := lock.Chmod(0o600); err != nil { - return State{}, fmt.Errorf("secure Profile configuration lock: %w", err) - } - if err := syscall.Flock(int(lock.Fd()), syscall.LOCK_EX); err != nil { + if err := unix.Flock(int(lock.Fd()), unix.LOCK_EX); err != nil { return State{}, fmt.Errorf("lock Profile configuration: %w", err) } - defer syscall.Flock(int(lock.Fd()), syscall.LOCK_UN) //nolint:errcheck -- closing also releases the lock + defer func() { _ = unix.Flock(int(lock.Fd()), unix.LOCK_UN) }() - current, err := s.load() + current, err := s.loadFrom(directory) if err != nil { return State{}, err } @@ -121,32 +194,55 @@ func (s *FileStore) Mutate(change func(State) (State, error)) (State, error) { if err != nil { return current, err } - if err := s.save(next); err != nil { + committed, err := s.save(next, directory) + if committed { + return next, err + } + if err != nil { return current, err } return next, nil } +func openNoFollow(path string, flags int, mode uint32) (*os.File, error) { + fd, err := unix.Open(path, flags|unix.O_CLOEXEC|unix.O_NOFOLLOW, mode) + if err != nil { + return nil, err + } + return os.NewFile(uintptr(fd), path), nil +} + +func openAt(directory *os.File, name string, flags int, mode uint32) (*os.File, error) { + fd, err := unix.Openat(int(directory.Fd()), name, flags|unix.O_CLOEXEC|unix.O_NOFOLLOW, mode) + if err != nil { + return nil, err + } + return os.NewFile(uintptr(fd), filepath.Join(directory.Name(), name)), nil +} + func (s *FileStore) load() (State, error) { - file, err := os.Open(s.path) + directory, err := openNoFollow(filepath.Dir(s.path), unix.O_RDONLY|unix.O_DIRECTORY, 0) if errors.Is(err, fs.ErrNotExist) { return State{}, nil } if err != nil { - return State{}, fmt.Errorf("read Profile configuration: %w", err) + return State{}, fmt.Errorf("open Profile configuration directory without following links: %w", err) + } + defer directory.Close() + return s.loadFrom(directory) +} + +func (s *FileStore) loadFrom(directory *os.File) (State, error) { + file, err := openAt(directory, filepath.Base(s.path), unix.O_RDONLY|unix.O_NONBLOCK, 0) + if errors.Is(err, fs.ErrNotExist) { + return State{}, nil + } + if err != nil { + return State{}, fmt.Errorf("open Profile configuration without following links: %w", err) } defer file.Close() - info, err := file.Stat() - if err != nil { - return State{}, fmt.Errorf("inspect Profile configuration: %w", err) - } - if !info.Mode().IsRegular() { - return State{}, errors.New("Profile configuration is not a regular file") - } - if info.Mode().Perm() != 0o600 { - if err := file.Chmod(0o600); err != nil { - return State{}, fmt.Errorf("secure Profile configuration: %w", err) - } + if err := s.secureRegularFile(file, "Profile configuration"); err != nil { + return State{}, err } data, err := io.ReadAll(file) if err != nil { @@ -159,51 +255,114 @@ func (s *FileStore) load() (State, error) { return state, nil } -func (s *FileStore) secureDirectory() error { - directory := filepath.Dir(s.path) - if err := os.MkdirAll(directory, 0o700); err != nil { - return fmt.Errorf("create Profile configuration directory: %w", err) +func (s *FileStore) secureRegularFile(file *os.File, description string) error { + info, err := file.Stat() + if err != nil { + return fmt.Errorf("inspect %s: %w", description, err) } - if err := os.Chmod(directory, 0o700); err != nil { - return fmt.Errorf("secure Profile configuration directory: %w", err) + if !info.Mode().IsRegular() { + return fmt.Errorf("%s is not a regular file", description) + } + if s.owner != nil { + if err := s.operations.SetOwner(file, s.owner.UID, s.owner.GID); err != nil { + return fmt.Errorf("set %s owner: %w", description, err) + } + } + if info.Mode().Perm() != 0o600 { + if err := file.Chmod(0o600); err != nil { + return fmt.Errorf("secure %s: %w", description, err) + } } return nil } -func (s *FileStore) save(state State) error { - if err := s.secureDirectory(); err != nil { - return err +func (s *FileStore) secureDirectory() (*os.File, error) { + directoryPath := filepath.Dir(s.path) + if err := os.MkdirAll(directoryPath, 0o700); err != nil { + return nil, fmt.Errorf("create Profile configuration directory: %w", err) } + directory, err := openNoFollow(directoryPath, unix.O_RDONLY|unix.O_DIRECTORY, 0) + if err != nil { + return nil, fmt.Errorf("open Profile configuration directory without following links: %w", err) + } + if s.owner != nil { + if err := s.operations.SetOwner(directory, s.owner.UID, s.owner.GID); err != nil { + directory.Close() + return nil, fmt.Errorf("set Profile configuration directory owner: %w", err) + } + } + if err := directory.Chmod(0o700); err != nil { + directory.Close() + return nil, fmt.Errorf("secure Profile configuration directory: %w", err) + } + return directory, nil +} + +func (s *FileStore) openLockAt(directory *os.File) (*os.File, error) { + lock, err := openAt(directory, filepath.Base(s.path)+".lock", unix.O_CREAT|unix.O_RDWR, 0o600) + if err != nil { + return nil, fmt.Errorf("open Profile configuration lock without following links: %w", err) + } + if err := s.secureRegularFile(lock, "Profile configuration lock"); err != nil { + lock.Close() + return nil, err + } + return lock, nil +} + +func (s *FileStore) save(state State, directory *os.File) (bool, error) { data, err := json.MarshalIndent(state, "", " ") if err != nil { - return fmt.Errorf("encode Profile configuration: %w", err) + return false, fmt.Errorf("encode Profile configuration: %w", err) } data = append(data, '\n') - temp, err := os.CreateTemp(filepath.Dir(s.path), ".profiles-*") + temp, tempBase, err := createTemporaryAt(directory) if err != nil { - return fmt.Errorf("create temporary Profile configuration: %w", err) + return false, fmt.Errorf("create temporary Profile configuration: %w", err) + } + defer unix.Unlinkat(int(directory.Fd()), tempBase, 0) //nolint:errcheck -- rename normally removes the temporary name + if s.owner != nil { + if err := s.operations.SetOwner(temp, s.owner.UID, s.owner.GID); err != nil { + temp.Close() + return false, fmt.Errorf("set temporary Profile configuration owner: %w", err) + } } - tempName := temp.Name() - defer os.Remove(tempName) if err := temp.Chmod(0o600); err != nil { temp.Close() - return fmt.Errorf("secure temporary Profile configuration: %w", err) + return false, fmt.Errorf("secure temporary Profile configuration: %w", err) } if _, err := temp.Write(data); err != nil { temp.Close() - return fmt.Errorf("write temporary Profile configuration: %w", err) + return false, fmt.Errorf("write temporary Profile configuration: %w", err) } if err := temp.Sync(); err != nil { temp.Close() - return fmt.Errorf("sync temporary Profile configuration: %w", err) + return false, fmt.Errorf("sync temporary Profile configuration: %w", err) } if err := temp.Close(); err != nil { - return fmt.Errorf("close temporary Profile configuration: %w", err) + return false, fmt.Errorf("close temporary Profile configuration: %w", err) } - // Rename is the commit point. There are deliberately no fallible operations - // afterwards, so callers never retain stale memory after a visible commit. - if err := os.Rename(tempName, s.path); err != nil { - return fmt.Errorf("replace Profile configuration: %w", err) + if err := unix.Renameat(int(directory.Fd()), tempBase, int(directory.Fd()), filepath.Base(s.path)); err != nil { + return false, fmt.Errorf("replace Profile configuration: %w", err) } - return nil + if err := s.operations.SyncDirectory(directory); err != nil { + return true, &CommitError{Err: fmt.Errorf("sync Profile configuration directory: %w", err)} + } + return true, nil +} + +func createTemporaryAt(directory *os.File) (*os.File, string, error) { + for range 100 { + bytes := make([]byte, 8) + if _, err := rand.Read(bytes); err != nil { + return nil, "", err + } + name := ".profiles-" + hex.EncodeToString(bytes) + file, err := openAt(directory, name, unix.O_CREAT|unix.O_EXCL|unix.O_RDWR, 0o600) + if errors.Is(err, fs.ErrExist) { + continue + } + return file, name, err + } + return nil, "", errors.New("could not allocate a unique temporary Profile configuration name") } diff --git a/internal/profile/store_test.go b/internal/profile/store_test.go index 9274b08..81fefad 100644 --- a/internal/profile/store_test.go +++ b/internal/profile/store_test.go @@ -2,14 +2,37 @@ package profile import ( "errors" + "fmt" "os" "os/exec" "path/filepath" + "strings" "syscall" "testing" "time" ) +type ownerCall struct { + path string + uid, gid int +} + +type fakeFileOperations struct { + calls []ownerCall + failContains string + syncErr error +} + +func (f *fakeFileOperations) SetOwner(file *os.File, uid, gid int) error { + f.calls = append(f.calls, ownerCall{path: file.Name(), uid: uid, gid: gid}) + if f.failContains != "" && strings.Contains(filepath.Base(file.Name()), f.failContains) { + return errors.New("injected ownership failure") + } + return nil +} + +func (f *fakeFileOperations) SyncDirectory(*os.File) error { return f.syncErr } + type fakeIdentityEnvironment struct { current UserIdentity users map[string]UserIdentity @@ -86,29 +109,182 @@ func TestFileStoreLockSerializesAcrossProcesses(t *testing.T) { } } +func TestOwnedStoreAssignsInvokingUserToDirectoryLockAndReplacement(t *testing.T) { + path := filepath.Join(t.TempDir(), "proxyctl", "profiles.json") + owner := UserIdentity{Username: "alice", UID: 1001, GID: 1002} + operations := &fakeFileOperations{} + store := newFileStore(path, &owner, operations) + if _, err := store.Mutate(func(state State) (State, error) { + state.Profiles = append(state.Profiles, Profile{Name: "office"}) + return state, nil + }); err != nil { + t.Fatal(err) + } + + wantKinds := map[string]bool{"directory": false, "lock": false, "temporary replacement": false} + for _, call := range operations.calls { + if call.uid != owner.UID || call.gid != owner.GID { + t.Fatalf("ownership call for %q used %d:%d, want %d:%d", call.path, call.uid, call.gid, owner.UID, owner.GID) + } + switch { + case call.path == filepath.Dir(path): + wantKinds["directory"] = true + case call.path == path+".lock": + wantKinds["lock"] = true + case strings.HasPrefix(filepath.Base(call.path), ".profiles-"): + wantKinds["temporary replacement"] = true + } + } + for kind, seen := range wantKinds { + if !seen { + t.Errorf("no ownership assignment for %s; calls=%#v", kind, operations.calls) + } + } +} + +func TestOwnedStoreReportsOwnershipFailuresBeforeCommit(t *testing.T) { + for _, failure := range []string{"proxyctl", "profiles.json.lock", ".profiles-"} { + t.Run(failure, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "proxyctl", "profiles.json") + owner := UserIdentity{Username: "alice", UID: 1001, GID: 1002} + operations := &fakeFileOperations{failContains: failure} + store := newFileStore(path, &owner, operations) + _, err := store.Mutate(func(state State) (State, error) { + state.Profiles = append(state.Profiles, Profile{Name: "must-not-commit"}) + return state, nil + }) + if err == nil || !strings.Contains(err.Error(), "owner") { + t.Fatalf("ownership failure = %v", err) + } + if _, statErr := os.Stat(path); !errors.Is(statErr, os.ErrNotExist) { + t.Fatalf("configuration was committed after ownership failure: %v", statErr) + } + }) + } +} + +func TestOwnedLoadRejectsConfigurationOwnershipFailure(t *testing.T) { + path := filepath.Join(t.TempDir(), "proxyctl", "profiles.json") + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(`{"profiles":[]}`), 0o600); err != nil { + t.Fatal(err) + } + owner := UserIdentity{Username: "alice", UID: 1001, GID: 1002} + operations := &fakeFileOperations{failContains: "profiles.json"} + _, err := newFileStore(path, &owner, operations).Load() + if err == nil || !strings.Contains(err.Error(), "set Profile configuration owner") { + t.Fatalf("configuration ownership failure = %v", err) + } +} + +func TestLoadRejectsProfileConfigurationSymlinkWithoutChangingTarget(t *testing.T) { + directory := t.TempDir() + target := filepath.Join(directory, "target.json") + if err := os.WriteFile(target, []byte(`{"profiles":[]}`), 0o644); err != nil { + t.Fatal(err) + } + path := filepath.Join(directory, "profiles.json") + if err := os.Symlink(target, path); err != nil { + t.Fatal(err) + } + _, err := NewFileStore(path).Load() + if err == nil || !strings.Contains(err.Error(), "without following links") { + t.Fatalf("symlink load error = %v", err) + } + info, statErr := os.Stat(target) + if statErr != nil { + t.Fatal(statErr) + } + if info.Mode().Perm() != 0o644 { + t.Fatalf("symlink target mode changed to %#o", info.Mode().Perm()) + } +} + +func TestMutationRejectsLockSymlinkWithoutChangingTarget(t *testing.T) { + directory := filepath.Join(t.TempDir(), "proxyctl") + if err := os.MkdirAll(directory, 0o700); err != nil { + t.Fatal(err) + } + target := filepath.Join(t.TempDir(), "lock-target") + if err := os.WriteFile(target, []byte("unchanged"), 0o644); err != nil { + t.Fatal(err) + } + path := filepath.Join(directory, "profiles.json") + if err := os.Symlink(target, path+".lock"); err != nil { + t.Fatal(err) + } + _, err := NewFileStore(path).Mutate(func(state State) (State, error) { return state, nil }) + if err == nil || !strings.Contains(err.Error(), "without following links") { + t.Fatalf("lock symlink error = %v", err) + } + contents, readErr := os.ReadFile(target) + if readErr != nil { + t.Fatal(readErr) + } + if string(contents) != "unchanged" { + t.Fatalf("lock symlink target was modified: %q", contents) + } + info, statErr := os.Stat(target) + if statErr != nil { + t.Fatal(statErr) + } + if info.Mode().Perm() != 0o644 { + t.Fatalf("lock symlink target mode changed to %#o", info.Mode().Perm()) + } +} + +func TestDirectorySyncFailureReturnsCommittedStateAndRecognizableError(t *testing.T) { + path := filepath.Join(t.TempDir(), "proxyctl", "profiles.json") + operations := &fakeFileOperations{syncErr: errors.New("injected sync failure")} + store := newFileStore(path, nil, operations) + next, err := store.Mutate(func(state State) (State, error) { + state.Profiles = append(state.Profiles, Profile{Name: "committed"}) + return state, nil + }) + var committed *CommitError + if !errors.As(err, &committed) { + t.Fatalf("sync failure = %v, want CommitError", err) + } + if len(next.Profiles) != 1 || next.Profiles[0].Name != "committed" { + t.Fatalf("returned committed state = %#v", next) + } + loaded, loadErr := NewFileStore(path).Load() + if loadErr != nil { + t.Fatal(loadErr) + } + if fmt.Sprint(loaded) != fmt.Sprint(next) { + t.Fatalf("disk state = %#v, committed state = %#v", loaded, next) + } +} + func TestConfigPathUsesInvokingUserIdentityUnderSudo(t *testing.T) { environment := fakeIdentityEnvironment{ current: UserIdentity{Username: "root", HomeDir: "/root"}, - users: map[string]UserIdentity{"alice": {Username: "alice", HomeDir: "/home/alice"}}, + users: map[string]UserIdentity{"alice": {Username: "alice", HomeDir: "/home/alice", UID: 1001, GID: 1002}}, env: map[string]string{ "SUDO_USER": "alice", "XDG_CONFIG_HOME": "/home/alice/.xdg/config", }, } - path, err := ConfigPathWith(environment) + configuration, err := ResolveConfiguration(environment) if err != nil { t.Fatal(err) } want := filepath.Join("/home/alice/.xdg/config", "proxyctl", "profiles.json") - if path != want { - t.Fatalf("configuration path = %q, want invoking user's XDG path %q", path, want) + if configuration.Path != want { + t.Fatalf("configuration path = %q, want invoking user's XDG path %q", configuration.Path, want) + } + if configuration.Owner.Username != "alice" || configuration.Owner.UID != 1001 || configuration.Owner.GID != 1002 { + t.Fatalf("configuration owner = %#v, want verified invoking-user identity", configuration.Owner) } } func TestConfigPathDoesNotReuseRootXDGForInvokingUser(t *testing.T) { environment := fakeIdentityEnvironment{ current: UserIdentity{Username: "root", HomeDir: "/root"}, - users: map[string]UserIdentity{"alice": {Username: "alice", HomeDir: "/home/alice"}}, + users: map[string]UserIdentity{"alice": {Username: "alice", HomeDir: "/home/alice", UID: 1001, GID: 1002}}, env: map[string]string{ "SUDO_USER": "alice", "XDG_CONFIG_HOME": "/root/.config",