diff --git a/.scratch/proxyctl/issues/02-shell-target.md b/.scratch/proxyctl/issues/02-shell-target.md index 23923b2..f08a3fe 100644 --- a/.scratch/proxyctl/issues/02-shell-target.md +++ b/.scratch/proxyctl/issues/02-shell-target.md @@ -4,7 +4,7 @@ **Blocked by:** 01 — Create and activate persistent Proxy Profiles. -**Status:** done +**Status:** in-progress - [ ] A shared Target Interface exposes description, inspection, and desired-state operations without leaking platform paths or commands to the TUI. - [ ] The Shell Target is Available only for detected Bash or Zsh environments; unsupported Shells are explained outside the main Target list. diff --git a/cmd/proxyctl/main.go b/cmd/proxyctl/main.go index 2066530..8bb4e23 100644 --- a/cmd/proxyctl/main.go +++ b/cmd/proxyctl/main.go @@ -27,7 +27,7 @@ func run(args []string, input io.Reader, output io.Writer) error { if err != nil { return fmt.Errorf("locate proxyctl executable: %w", err) } - return runWithEnvironment(args, input, output, processEnvironment{}, shell.OSFileSystem{}, executable) + return runWithEnvironment(args, input, output, processEnvironment{}, nil, executable) } type processEnvironment struct{} diff --git a/internal/profile/store.go b/internal/profile/store.go index e283e26..a7035e8 100644 --- a/internal/profile/store.go +++ b/internal/profile/store.go @@ -1,8 +1,6 @@ package profile import ( - "crypto/rand" - "encoding/hex" "encoding/json" "errors" "fmt" @@ -15,6 +13,8 @@ import ( "strings" "golang.org/x/sys/unix" + + "proxyctl/internal/securefs" ) type FileStore struct { @@ -230,19 +230,11 @@ func (s *FileStore) Mutate(change func(State) (State, error)) (State, error) { } 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 + return securefs.OpenNoFollow(path, flags, mode) } 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 + return securefs.OpenAt(directory, name, flags, mode) } func (s *FileStore) load() (State, error) { @@ -465,17 +457,5 @@ func (s *FileStore) save(state State, directory *os.File) (bool, error) { } 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") + return securefs.CreateTemporaryAt(directory, ".profiles-") } diff --git a/internal/securefs/securefs.go b/internal/securefs/securefs.go new file mode 100644 index 0000000..ad882ac --- /dev/null +++ b/internal/securefs/securefs.go @@ -0,0 +1,237 @@ +// Package securefs provides descriptor-anchored file operations that never +// follow path components or final-file symbolic links. +package securefs + +import ( + "crypto/rand" + "encoding/hex" + "errors" + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + "strings" + + "golang.org/x/sys/unix" +) + +type Owner struct{ UID, GID int } + +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 CreateTemporaryAt(directory *os.File, prefix string) (*os.File, string, error) { + for range 100 { + bytes := make([]byte, 8) + if _, err := rand.Read(bytes); err != nil { + return nil, "", err + } + name := prefix + 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 file") +} + +type Store struct { + TrustedRoot string + Owner *Owner +} + +func (s Store) ReadFile(path string) ([]byte, error) { + directory, base, err := s.openParent(path, false) + if err != nil { + return nil, err + } + defer directory.Close() + file, err := OpenAt(directory, base, unix.O_RDONLY|unix.O_NONBLOCK, 0) + if err != nil { + return nil, err + } + defer file.Close() + if err := regular(file); err != nil { + return nil, err + } + return io.ReadAll(file) +} + +func (s Store) FileMode(path string) (os.FileMode, error) { + directory, base, err := s.openParent(path, false) + if err != nil { + return 0, err + } + defer directory.Close() + file, err := OpenAt(directory, base, unix.O_RDONLY|unix.O_NONBLOCK, 0) + if err != nil { + return 0, err + } + defer file.Close() + if err := regular(file); err != nil { + return 0, err + } + info, err := file.Stat() + if err != nil { + return 0, err + } + return info.Mode().Perm(), nil +} + +func (s Store) AtomicWrite(path string, data []byte, mode os.FileMode) error { + directory, base, err := s.openParent(path, true) + if err != nil { + return err + } + defer directory.Close() + if existing, openErr := OpenAt(directory, base, unix.O_RDONLY|unix.O_NONBLOCK, 0); openErr == nil { + err = regular(existing) + _ = existing.Close() + if err != nil { + return err + } + } else if !errors.Is(openErr, fs.ErrNotExist) { + return fmt.Errorf("inspect destination without following links: %w", openErr) + } + temporary, temporaryBase, err := CreateTemporaryAt(directory, ".proxyctl-") + if err != nil { + return err + } + committed := false + defer func() { + if !committed { + _ = unix.Unlinkat(int(directory.Fd()), temporaryBase, 0) + } + }() + if s.Owner != nil && (s.Owner.UID != os.Geteuid() || s.Owner.GID != os.Getegid()) { + if err := temporary.Chown(s.Owner.UID, s.Owner.GID); err != nil { + _ = temporary.Close() + return err + } + } + if err := temporary.Chmod(mode); err != nil { + _ = temporary.Close() + return err + } + if _, err := temporary.Write(data); err != nil { + _ = temporary.Close() + return err + } + if err := temporary.Sync(); err != nil { + _ = temporary.Close() + return err + } + if err := temporary.Close(); err != nil { + return err + } + if err := unix.Renameat(int(directory.Fd()), temporaryBase, int(directory.Fd()), base); err != nil { + return err + } + committed = true + return directory.Sync() +} + +func (s Store) Remove(path string) error { + directory, base, err := s.openParent(path, false) + if err != nil { + return err + } + defer directory.Close() + file, err := OpenAt(directory, base, unix.O_RDONLY|unix.O_NONBLOCK, 0) + if err != nil { + return err + } + if err := regular(file); err != nil { + _ = file.Close() + return err + } + _ = file.Close() + if err := unix.Unlinkat(int(directory.Fd()), base, 0); err != nil { + return err + } + return directory.Sync() +} + +func (s Store) openParent(path string, create bool) (*os.File, string, error) { + path = filepath.Clean(path) + root := filepath.Clean(s.TrustedRoot) + if !filepath.IsAbs(path) || !filepath.IsAbs(root) || !within(path, root) || path == root { + return nil, "", errors.New("secure file path escapes its trusted root") + } + parent, base := filepath.Dir(path), filepath.Base(path) + current, err := OpenNoFollow(string(filepath.Separator), unix.O_RDONLY|unix.O_DIRECTORY, 0) + if err != nil { + return nil, "", err + } + currentPath := string(filepath.Separator) + for _, component := range strings.Split(strings.TrimPrefix(parent, string(filepath.Separator)), string(filepath.Separator)) { + if component == "" { + continue + } + nextPath := filepath.Join(currentPath, component) + next, openErr := OpenAt(current, component, unix.O_RDONLY|unix.O_DIRECTORY, 0) + if errors.Is(openErr, fs.ErrNotExist) && create && nextPath != root && within(nextPath, root) { + created := false + if mkdirErr := unix.Mkdirat(int(current.Fd()), component, 0o700); mkdirErr != nil { + if !errors.Is(mkdirErr, fs.ErrExist) { + _ = current.Close() + return nil, "", mkdirErr + } + } else { + created = true + } + next, openErr = OpenAt(current, component, unix.O_RDONLY|unix.O_DIRECTORY, 0) + if openErr == nil && created { + if s.Owner != nil && (s.Owner.UID != os.Geteuid() || s.Owner.GID != os.Getegid()) { + if err := next.Chown(s.Owner.UID, s.Owner.GID); err != nil { + _ = next.Close() + _ = current.Close() + return nil, "", err + } + } + if err := next.Chmod(0o700); err != nil { + _ = next.Close() + _ = current.Close() + return nil, "", err + } + } + } + _ = current.Close() + if openErr != nil { + return nil, "", fmt.Errorf("open ancestor %q without following links: %w", nextPath, openErr) + } + current, currentPath = next, nextPath + } + return current, base, nil +} + +func regular(file *os.File) error { + info, err := file.Stat() + if err != nil { + return err + } + if !info.Mode().IsRegular() { + return errors.New("destination is not a regular file") + } + return nil +} + +func within(path, root string) bool { + relative, err := filepath.Rel(root, path) + return err == nil && relative != ".." && !strings.HasPrefix(relative, ".."+string(filepath.Separator)) +} diff --git a/internal/shell/shell.go b/internal/shell/shell.go index 787ebcc..5de23c8 100644 --- a/internal/shell/shell.go +++ b/internal/shell/shell.go @@ -3,7 +3,6 @@ package shell import ( "context" - "crypto/rand" "crypto/sha256" "encoding/hex" "encoding/json" @@ -16,6 +15,7 @@ import ( "strings" "proxyctl/internal/profile" + "proxyctl/internal/securefs" "proxyctl/internal/target" ) @@ -37,6 +37,7 @@ type Configuration struct { OwnFiles bool UID int GID int + TrustedRoots []string } // ResolveConfiguration keeps user Shell effects with the invoking user under sudo. @@ -69,11 +70,16 @@ func ResolveConfiguration(environment profile.IdentityEnvironment) (Configuratio case "zsh": startupFile = filepath.Join(owner.HomeDir, ".zshrc") } + stateRoot := owner.HomeDir + if !pathWithin(stateDirectory, owner.HomeDir) { + stateRoot = stateDirectory + } return Configuration{ Shell: shellPath, StartupFile: startupFile, EnvironmentFile: filepath.Join(configDirectory, "shell.env"), RecoveryFile: filepath.Join(stateDirectory, "proxyctl", "shell.json"), OwnFiles: true, UID: owner.UID, GID: owner.GID, + TrustedRoots: []string{owner.HomeDir, profileConfiguration.TrustedRoot, stateRoot}, }, nil } @@ -110,110 +116,34 @@ type FileSystem interface { Remove(string) error } -type OSFileSystem struct{} +type OSFileSystem struct{ TrustedRoots []string } -func (OSFileSystem) ReadFile(path string) ([]byte, error) { - info, err := os.Lstat(path) - if err != nil { - return nil, err - } - if !info.Mode().IsRegular() { - return nil, fmt.Errorf("%s is not a regular file", path) - } - return os.ReadFile(path) +func NewOSFileSystem(configuration Configuration) OSFileSystem { + return OSFileSystem{TrustedRoots: append([]string(nil), configuration.TrustedRoots...)} } -func (OSFileSystem) Remove(path string) error { return os.Remove(path) } -func (OSFileSystem) FileMode(path string) (os.FileMode, error) { - info, err := os.Lstat(path) - if err != nil { - return 0, err +func (f OSFileSystem) store(path string, owner *Ownership) securefs.Store { + root := "" + if len(f.TrustedRoots) == 0 { + root = string(filepath.Separator) } - if !info.Mode().IsRegular() { - return 0, fmt.Errorf("%s is not a regular file", path) + for _, candidate := range f.TrustedRoots { + if pathWithin(path, candidate) && (root == "" || len(filepath.Clean(candidate)) > len(filepath.Clean(root))) { + root = candidate + } } - return info.Mode().Perm(), nil + var secureOwner *securefs.Owner + if owner != nil { + secureOwner = &securefs.Owner{UID: owner.UID, GID: owner.GID} + } + return securefs.Store{TrustedRoot: root, Owner: secureOwner} } -func (OSFileSystem) AtomicWrite(path string, data []byte, mode os.FileMode, owner *Ownership) error { - if !filepath.IsAbs(path) { - return errors.New("Shell Target path must be absolute") - } - directory := filepath.Dir(path) - var missing []string - for current := directory; ; current = filepath.Dir(current) { - if _, err := os.Stat(current); err == nil { - break - } else if !errors.Is(err, fs.ErrNotExist) { - return err - } - missing = append(missing, current) - if filepath.Dir(current) == current { - break - } - } - if err := os.MkdirAll(directory, 0o700); err != nil { - return err - } - for i := len(missing) - 1; i >= 0; i-- { - if owner != nil && (owner.UID != os.Geteuid() || owner.GID != os.Getegid()) { - if err := os.Chown(missing[i], owner.UID, owner.GID); err != nil { - return err - } - } - if err := os.Chmod(missing[i], 0o700); err != nil { - return err - } - } - var temporary string - for range 100 { - bytes := make([]byte, 8) - if _, err := rand.Read(bytes); err != nil { - return err - } - temporary = filepath.Join(directory, ".proxyctl-"+hex.EncodeToString(bytes)) - file, err := os.OpenFile(temporary, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode) - if errors.Is(err, fs.ErrExist) { - continue - } - if err != nil { - return err - } - ok := false - defer func() { - if !ok { - _ = os.Remove(temporary) - } - }() - if owner != nil && (owner.UID != os.Geteuid() || owner.GID != os.Getegid()) { - if err = file.Chown(owner.UID, owner.GID); err != nil { - _ = file.Close() - return err - } - } - if _, err = file.Write(data); err == nil { - err = file.Sync() - } - if closeErr := file.Close(); err == nil { - err = closeErr - } - if err != nil { - return err - } - if err = os.Rename(temporary, path); err != nil { - return err - } - ok = true - dir, err := os.Open(directory) - if err != nil { - return err - } - err = dir.Sync() - closeErr := dir.Close() - if err != nil { - return err - } - return closeErr - } - return errors.New("allocate temporary Shell Target file") +func (f OSFileSystem) ReadFile(path string) ([]byte, error) { return f.store(path, nil).ReadFile(path) } +func (f OSFileSystem) Remove(path string) error { return f.store(path, nil).Remove(path) } +func (f OSFileSystem) FileMode(path string) (os.FileMode, error) { + return f.store(path, nil).FileMode(path) +} +func (f OSFileSystem) AtomicWrite(path string, data []byte, mode os.FileMode, owner *Ownership) error { + return f.store(path, owner).AtomicWrite(path, data, mode) } type recovery struct { @@ -230,7 +160,8 @@ type Adapter struct { func New(configuration Configuration, files FileSystem, environment Environment) *Adapter { if files == nil { - files = OSFileSystem{} + configured := NewOSFileSystem(configuration) + files = configured } if environment == nil { environment = MapEnvironment{} @@ -326,15 +257,18 @@ func (a *Adapter) enable(active profile.Profile) error { } generated := renderEnabled(normalized) metadata.Fingerprint = fingerprint(generated) - if err := a.writeOwned(a.configuration.EnvironmentFile, generated); err != nil { - return fmt.Errorf("write Shell environment: %w", err) - } data, err := json.MarshalIndent(metadata, "", " ") if err != nil { return err } + // The true original must be durable before the managed environment becomes + // visible. A retry then reuses this metadata instead of snapshotting its own + // partially applied output as External state. if err := a.writeOwned(a.configuration.RecoveryFile, append(data, '\n')); err != nil { - return fmt.Errorf("write Shell recovery metadata after environment changed: %w", err) + return fmt.Errorf("persist Shell recovery metadata before changing environment: %w", err) + } + if err := a.writeOwned(a.configuration.EnvironmentFile, generated); err != nil { + return fmt.Errorf("write Shell environment after recovery metadata persisted: %w", err) } return nil } @@ -413,30 +347,46 @@ func renderRestored(original map[string]string) []byte { func shellQuote(value string) string { return "'" + strings.ReplaceAll(value, "'", "'\\''") + "'" } func fingerprint(data []byte) string { sum := sha256.Sum256(data); return hex.EncodeToString(sum[:]) } func recognizesProxy(data []byte) bool { - text := string(data) + supported := make(map[string]bool, len(proxyVariables)) for _, name := range proxyVariables { - if strings.Contains(text, "export "+name+"=") || strings.Contains(text, name+"=") { - return true + supported[name] = true + } + for _, raw := range strings.Split(string(data), "\n") { + line := strings.TrimSpace(raw) + if line == "" || strings.HasPrefix(line, "#") { + continue } + if strings.HasPrefix(line, "export") { + if len(line) == len("export") || (line[len("export")] != ' ' && line[len("export")] != '\t') { + continue + } + line = strings.TrimSpace(line[len("export"):]) + } + name, _, found := strings.Cut(line, "=") + if !found || strings.TrimSpace(name) != name || !supported[name] { + continue + } + return true } return false } type redactedError struct { message string - cause error + class error } -func (e *redactedError) Error() string { return e.message } -func (e *redactedError) Unwrap() error { return e.cause } +func (e *redactedError) Error() string { return e.message } +func (e *redactedError) Is(target error) bool { return e.class != nil && errors.Is(e.class, target) } func redactError(err error, active *profile.Profile) error { if err == nil { return nil } message := profile.RedactText(err.Error()) + class := classifySafeError(err) if active == nil { - return &redactedError{message: message, cause: err} + return &redactedError{message: message, class: class} } for _, raw := range []string{active.SingleURL, active.HTTPURL, active.HTTPSURL, active.AllURL} { if raw == "" { @@ -449,7 +399,16 @@ func redactError(err error, active *profile.Profile) error { } } } - return &redactedError{message: message, cause: err} + return &redactedError{message: message, class: class} +} + +func classifySafeError(err error) error { + for _, safe := range []error{os.ErrPermission, fs.ErrNotExist, fs.ErrExist, context.Canceled, context.DeadlineExceeded} { + if errors.Is(err, safe) { + return safe + } + } + return nil } func (a *Adapter) Install(_ context.Context, executable string) error { @@ -465,6 +424,7 @@ func (a *Adapter) Install(_ context.Context, executable string) error { cleaned += "\n" } block := integrationStart + "\n" + + "[ -r " + shellQuote(a.configuration.EnvironmentFile) + " ] && . " + shellQuote(a.configuration.EnvironmentFile) + "\n" + "proxyctl() {\n" + " command " + shellQuote(executable) + " \"$@\"\n" + " proxyctl_status=$?\n" + diff --git a/internal/shell/shell_test.go b/internal/shell/shell_test.go index 0b6354d..68cf8ee 100644 --- a/internal/shell/shell_test.go +++ b/internal/shell/shell_test.go @@ -56,19 +56,73 @@ func TestShellAdapterRunsReusableTargetContract(t *testing.T) { return failed.SetDesired(context.Background(), true, activeProfile()) }, PartialFailure: func(t *testing.T) target.OperationResult { - failed := newAdapterWithFS(t, shellName, nil, &failingFS{FileSystem: shell.OSFileSystem{}, failState: true}) + failed := newAdapterWithFS(t, shellName, nil, &failingFS{FileSystem: shell.OSFileSystem{}, failEnvironment: true}) return failed.SetDesired(context.Background(), true, activeProfile()) }, RedactionFailure: func(t *testing.T) target.OperationResult { failed := newAdapterWithFS(t, shellName, nil, &failingFS{FileSystem: shell.OSFileSystem{}, failWrite: true}) return failed.SetDesired(context.Background(), true, activeProfile()) }, + RecoveryRetry: func(t *testing.T) (target.OperationResult, target.OperationResult, target.OperationResult) { + files := &failingFS{FileSystem: shell.OSFileSystem{}, failRecoveryOnce: true} + failed := newAdapterWithFS(t, shellName, map[string]string{"HTTP_PROXY": "http://prior.example:3128"}, files) + first := failed.SetDesired(context.Background(), true, activeProfile()) + retry := failed.SetDesired(context.Background(), true, activeProfile()) + disabled := failed.SetDesired(context.Background(), false, nil) + return first, retry, disabled + }, } }) }) } } +func TestRecoveryFailureRetryPreservesOriginalShellState(t *testing.T) { + files := &failingFS{FileSystem: shell.OSFileSystem{}, failRecoveryOnce: true} + a := newAdapterWithFS(t, "/bin/bash", map[string]string{"HTTP_PROXY": "http://prior.example:3128"}, files) + first := a.SetDesired(context.Background(), true, activeProfile()) + if first.Err == nil || first.Inspection.State != target.External { + t.Fatalf("first enable = %#v", first) + } + if _, err := os.Stat(a.EnvironmentPath()); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("managed environment was visible before recovery persisted: %v", err) + } + if retry := a.SetDesired(context.Background(), true, activeProfile()); retry.Err != nil || retry.Inspection.State != target.Enabled { + t.Fatalf("retry = %#v", retry) + } + if disabled := a.SetDesired(context.Background(), false, nil); disabled.Err != nil || disabled.Inspection.State != target.External { + t.Fatalf("disable = %#v", disabled) + } + if restored := readFile(t, a.EnvironmentPath()); !strings.Contains(restored, "http://prior.example:3128") || strings.Contains(restored, "proxy.example:8080") { + t.Fatalf("original was not restored: %s", restored) + } +} + +func TestRecognizesOnlyExactSupportedShellProxySyntax(t *testing.T) { + for _, tc := range []struct { + name, contents string + state target.State + }{ + {"comment", "# export HTTP_PROXY='http://comment.example:8080'\n", target.Disabled}, + {"prefixed variable", "export NOT_HTTP_PROXY='http://wrong.example:8080'\n", target.Disabled}, + {"prose", "echo export HTTP_PROXY=http://wrong.example:8080\n", target.Disabled}, + {"unset", "unset HTTP_PROXY http_proxy\n", target.Disabled}, + {"assignment", "HTTP_PROXY='http://external.example:8080'\n", target.External}, + {"export assignment", "export http_proxy='http://external.example:8080'\n", target.External}, + } { + t.Run(tc.name, func(t *testing.T) { + a := newAdapter(t, "/bin/bash", nil) + if err := os.MkdirAll(filepath.Dir(a.EnvironmentPath()), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(a.EnvironmentPath(), []byte(tc.contents), 0o600); err != nil { + t.Fatal(err) + } + assertState(t, a, activeProfile(), tc.state) + }) + } +} + func TestShellTargetContract(t *testing.T) { contract := func(t *testing.T, shellName string) { t.Helper() @@ -146,14 +200,14 @@ func TestShellTargetContract(t *testing.T) { }) t.Run("permission-and-partial-failure-reinspect", func(t *testing.T) { - fs := &failingFS{FileSystem: shell.OSFileSystem{}, failState: true} + fs := &failingFS{FileSystem: shell.OSFileSystem{}, failEnvironment: true} a := newAdapterWithFS(t, shellName, nil, fs) got := a.SetDesired(context.Background(), true, activeProfile()) if got.Err == nil || !errors.Is(got.Err, os.ErrPermission) { t.Fatalf("permission error = %#v", got) } - if got.Inspection.State != target.External { - t.Fatalf("partial write state = %s, want External", got.Inspection.State) + if got.Inspection.State != target.Drifted { + t.Fatalf("partial write state = %s, want Drifted", got.Inspection.State) } }) }) @@ -200,6 +254,80 @@ func TestIntegrationInstallUninstallPreservesStartupFileAndLoadsAfterCommand(t * } } +func TestDescriptorAnchoredWritesRejectSymlinkedAncestorVictims(t *testing.T) { + t.Run("configuration", func(t *testing.T) { + root, victim := t.TempDir(), t.TempDir() + if err := os.Symlink(victim, filepath.Join(root, "config")); err != nil { + t.Fatal(err) + } + a := shell.New(shell.Configuration{Shell: "/bin/bash", StartupFile: filepath.Join(root, "home", ".bashrc"), EnvironmentFile: filepath.Join(root, "config", "proxyctl", "shell.env"), RecoveryFile: filepath.Join(root, "state", "proxyctl", "shell.json"), TrustedRoots: []string{root}}, nil, shell.MapEnvironment{}) + if got := a.SetDesired(context.Background(), true, activeProfile()); got.Err == nil { + t.Fatal("enable followed symlinked configuration ancestor") + } + assertVictimEmpty(t, victim) + }) + t.Run("configuration final file", func(t *testing.T) { + root := t.TempDir() + victim := filepath.Join(t.TempDir(), "victim") + if err := os.WriteFile(victim, []byte("unchanged\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(root, "config", "proxyctl"), 0o700); err != nil { + t.Fatal(err) + } + environmentFile := filepath.Join(root, "config", "proxyctl", "shell.env") + if err := os.Symlink(victim, environmentFile); err != nil { + t.Fatal(err) + } + a := shell.New(shell.Configuration{Shell: "/bin/bash", StartupFile: filepath.Join(root, ".bashrc"), EnvironmentFile: environmentFile, RecoveryFile: filepath.Join(root, "state", "proxyctl", "shell.json"), TrustedRoots: []string{root}}, nil, shell.MapEnvironment{}) + if got := a.SetDesired(context.Background(), true, activeProfile()); got.Err == nil { + t.Fatal("enable replaced symlinked final configuration file") + } + if got := readFile(t, victim); got != "unchanged\n" { + t.Fatalf("final symlink victim changed to %q", got) + } + }) + t.Run("state", func(t *testing.T) { + root, victim := t.TempDir(), t.TempDir() + if err := os.Mkdir(filepath.Join(root, ".local"), 0o700); err != nil { + t.Fatal(err) + } + if err := os.Symlink(victim, filepath.Join(root, ".local", "state")); err != nil { + t.Fatal(err) + } + a := shell.New(shell.Configuration{Shell: "/bin/bash", StartupFile: filepath.Join(root, ".bashrc"), EnvironmentFile: filepath.Join(root, ".config", "proxyctl", "shell.env"), RecoveryFile: filepath.Join(root, ".local", "state", "proxyctl", "shell.json"), TrustedRoots: []string{root}}, nil, shell.MapEnvironment{}) + if got := a.SetDesired(context.Background(), true, activeProfile()); got.Err == nil { + t.Fatal("enable followed symlinked state ancestor") + } + assertVictimEmpty(t, victim) + if _, err := os.Stat(a.EnvironmentPath()); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("environment changed before secure recovery: %v", err) + } + }) + t.Run("integration", func(t *testing.T) { + root, victim := t.TempDir(), t.TempDir() + if err := os.Symlink(victim, filepath.Join(root, "home")); err != nil { + t.Fatal(err) + } + a := shell.New(shell.Configuration{Shell: "/bin/bash", StartupFile: filepath.Join(root, "home", ".bashrc"), EnvironmentFile: filepath.Join(root, "config", "shell.env"), RecoveryFile: filepath.Join(root, "state", "shell.json"), TrustedRoots: []string{root}}, nil, shell.MapEnvironment{}) + if err := a.Install(context.Background(), "/bin/proxyctl"); err == nil { + t.Fatal("install followed symlinked startup ancestor") + } + assertVictimEmpty(t, victim) + }) +} + +func assertVictimEmpty(t *testing.T, path string) { + t.Helper() + entries, err := os.ReadDir(path) + if err != nil { + t.Fatal(err) + } + if len(entries) != 0 { + t.Fatalf("symlink victim changed: %v", entries) + } +} + func TestIntegrationRefusesStartupSymlinkWithoutChangingVictim(t *testing.T) { a := newAdapter(t, "/bin/bash", nil) if err := os.MkdirAll(filepath.Dir(a.StartupPath()), 0o700); err != nil { @@ -237,6 +365,38 @@ func TestMalformedIntegrationMarkersNeverCauseStartupContentDeletion(t *testing. } } +func TestFreshIntegratedShellSourcesExistingEnvironmentAtStartup(t *testing.T) { + for _, shellName := range []string{"bash", "zsh"} { + t.Run(shellName, func(t *testing.T) { + path, err := exec.LookPath(shellName) + if err != nil { + t.Skip(shellName + " is not installed") + } + a := newAdapter(t, filepath.Join("/bin", shellName), nil) + if enabled := a.SetDesired(context.Background(), true, activeProfile()); enabled.Err != nil { + t.Fatal(enabled.Err) + } + if err := a.Install(context.Background(), "/bin/true"); err != nil { + t.Fatal(err) + } + var command *exec.Cmd + if shellName == "bash" { + command = exec.Command(path, "--noprofile", "--rcfile", a.StartupPath(), "-i", "-c", `printf %s "$HTTP_PROXY"`) + } else { + command = exec.Command(path, "-d", "-i", "-c", `printf %s "$HTTP_PROXY"`) + command.Env = append(os.Environ(), "ZDOTDIR="+filepath.Dir(a.StartupPath())) + } + output, err := command.Output() + if err != nil { + t.Fatalf("start fresh %s: %v", shellName, err) + } + if got := string(output); got != "http://alice:secret@proxy.example:8080" { + t.Fatalf("fresh %s HTTP_PROXY = %q", shellName, got) + } + }) + } +} + func TestInstalledBashWrapperReloadsParentEnvironmentAfterProxyctlExits(t *testing.T) { if _, err := exec.LookPath("bash"); err != nil { t.Skip("bash is not installed") @@ -322,7 +482,11 @@ func newAdapter(t *testing.T, shellName string, env map[string]string) *shell.Ad func newAdapterWithFS(t *testing.T, shellName string, env map[string]string, fs shell.FileSystem) *shell.Adapter { t.Helper() root := t.TempDir() - return shell.New(shell.Configuration{Shell: shellName, StartupFile: filepath.Join(root, "home", ".bashrc"), EnvironmentFile: filepath.Join(root, "config", "shell.env"), RecoveryFile: filepath.Join(root, "state", "shell.json")}, fs, shell.MapEnvironment(env)) + startup := ".bashrc" + if filepath.Base(shellName) == "zsh" { + startup = ".zshrc" + } + return shell.New(shell.Configuration{Shell: shellName, StartupFile: filepath.Join(root, "home", startup), EnvironmentFile: filepath.Join(root, "config", "shell.env"), RecoveryFile: filepath.Join(root, "state", "shell.json")}, fs, shell.MapEnvironment(env)) } func assertState(t *testing.T, a *shell.Adapter, p *profile.Profile, want target.State) { t.Helper() @@ -341,7 +505,7 @@ func readFile(t *testing.T, path string) string { type failingFS struct { shell.FileSystem - failWrite, failState, failRestore, permission bool + failWrite, failState, failEnvironment, failRestore, permission, failRecoveryOnce bool } func (f *failingFS) AtomicWrite(path string, data []byte, mode os.FileMode, owner *shell.Ownership) error { @@ -351,9 +515,16 @@ func (f *failingFS) AtomicWrite(path string, data []byte, mode os.FileMode, owne if f.failWrite { return errors.New("write http://alice:secret@proxy.example:8080 failed") } + if f.failEnvironment && strings.HasSuffix(path, "shell.env") { + return os.ErrPermission + } if f.failRestore && strings.HasSuffix(path, "shell.env") { return errors.New("restore http://alice:restore-secret@prior.example:8080 failed") } + if f.failRecoveryOnce && strings.HasSuffix(path, "shell.json") { + f.failRecoveryOnce = false + return os.ErrPermission + } if f.failState && strings.HasSuffix(path, "shell.json") { return os.ErrPermission } diff --git a/internal/target/targettest/contract.go b/internal/target/targettest/contract.go index 0ed783b..674b95b 100644 --- a/internal/target/targettest/contract.go +++ b/internal/target/targettest/contract.go @@ -4,6 +4,7 @@ package targettest import ( "context" "errors" + "fmt" "os" "strings" "testing" @@ -21,6 +22,7 @@ type Fixture struct { PermissionFailure func(*testing.T) target.OperationResult PartialFailure func(*testing.T) target.OperationResult RedactionFailure func(*testing.T) target.OperationResult + RecoveryRetry func(*testing.T) (target.OperationResult, target.OperationResult, target.OperationResult) Secret string } @@ -84,12 +86,24 @@ func Run(t *testing.T, factory Factory) { t.Fatalf("permission failure = %#v", permission) } partial := fixture.PartialFailure(t) - if partial.Err == nil || partial.Inspection.State != target.External { + if partial.Err == nil || partial.Inspection.State != target.Drifted { t.Fatalf("partial failure = %#v", partial) } + first, retry, disabled := fixture.RecoveryRetry(t) + if first.Err == nil || first.Inspection.State != target.External || retry.Err != nil || retry.Inspection.State != target.Enabled || disabled.Err != nil || disabled.Inspection.State != target.External { + t.Fatalf("recovery retry transaction = first %#v, retry %#v, disable %#v", first, retry, disabled) + } + redacted := fixture.RedactionFailure(t) - if redacted.Err == nil || fixture.Secret == "" || strings.Contains(redacted.Err.Error(), fixture.Secret) { - t.Fatalf("secret was not redacted: %#v", redacted) + if redacted.Err == nil || fixture.Secret == "" { + t.Fatalf("redaction failure = %#v", redacted) + } + for current := redacted.Err; current != nil; current = errors.Unwrap(current) { + for _, rendered := range []string{current.Error(), fmt.Sprintf("%v", current), fmt.Sprintf("%+v", current), fmt.Sprintf("%#v", current)} { + if strings.Contains(rendered, fixture.Secret) { + t.Fatalf("secret in error chain representation %q", rendered) + } + } } }) } diff --git a/internal/tui/model.go b/internal/tui/model.go index 21fd080..94c6cb7 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -283,6 +283,11 @@ func (m Model) mainView() string { fmt.Fprintf(&b, "\n%s\n%s\n", selected.Description.Summary, selected.Evidence) } } + for _, item := range m.snapshot.Targets { + if !item.Available { + fmt.Fprintf(&b, "\nUnavailable %s: %s\n", item.Description.Name, item.AvailabilityEvidence) + } + } b.WriteString("\nSpace: toggle ↑/k ↓/j: select p: manage Proxy Profiles q/Esc: quit\n") if m.status != "" { fmt.Fprintf(&b, "\n%s\n", m.status) diff --git a/internal/tui/target_test.go b/internal/tui/target_test.go index 83a187b..9ed336c 100644 --- a/internal/tui/target_test.go +++ b/internal/tui/target_test.go @@ -32,7 +32,7 @@ func TestTargetPanelShowsOnlyAvailableTargetsAndAppliesAsynchronously(t *testing module.result = app.Result{Snapshot: app.Snapshot{ActiveProfile: "office", Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell", Summary: "Exports for Bash/Zsh"}, Available: true, State: target.Enabled, Evidence: "matches"}}}, Notice: "Shell Target is now Enabled"} model := NewModel(module) view := model.View() - if !strings.Contains(view, "Shell") || strings.Contains(view, "APT") || !strings.Contains(view, "Exports for Bash/Zsh") { + if !strings.Contains(view, "Shell") || strings.Contains(view, "] APT") || !strings.Contains(view, "Unavailable APT") || !strings.Contains(view, "Exports for Bash/Zsh") { t.Fatalf("Target panel:\n%s", view) } @@ -49,6 +49,16 @@ func TestTargetPanelShowsOnlyAvailableTargetsAndAppliesAsynchronously(t *testing } } +func TestUnavailableShellIsOutsideMainListWithDetectionExplanation(t *testing.T) { + module := &targetModule{snapshot: app.Snapshot{Targets: []app.TargetSnapshot{ + {ID: "shell", Description: target.Description{Name: "Shell"}, Available: false, AvailabilityEvidence: "Shell Target requires Bash or Zsh; neither was detected"}, + }}} + view := NewModel(module).View() + if strings.Contains(view, "[Disabled ] Shell") || !strings.Contains(view, "Unavailable Shell") || !strings.Contains(view, "requires Bash or Zsh") { + t.Fatalf("unavailable Shell explanation:\n%s", view) + } +} + func TestTargetFailurePreservesTruthfulStateAndRendersRedactedError(t *testing.T) { initial := app.Snapshot{ActiveProfile: "office", Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Enabled}}} module := &targetModule{snapshot: initial, result: app.Result{Snapshot: initial, Err: errors.New("permission denied for alice:REDACTED@proxy")}}