From 6093f95e98635ec1146e109399431e08ead32fc4 Mon Sep 17 00:00:00 2001 From: Karn Date: Mon, 10 Aug 2026 01:57:24 +0530 Subject: [PATCH] fix(session): default an unstated spawn cwd to the user's home MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A session spawned with no cwd handed the empty string to cmd.Dir, so the shell inherited the daemon's own working directory. Under launchd or systemd that directory is / — neither unit in internal/service sets one — so every plain new session opened at /; a hand-started daemon put them wherever it happened to be launched from, which is no better chosen. Registry.start now resolves an empty opts.Cwd to os.UserHomeDir(), the same call Revive already uses for a vanished directory, and feeds that one resolved value to both cmd.Dir and the recorded Info.Cwd — the old code seeded the record from os.Getwd() instead, a recorded-vs-actual split this closes. When home is unresolvable the empty string stands and the shell inherits, which is the old behaviour kept as the floor. Callers that state a cwd — group-heading spawns, restart with the exited session's cwd, revival from snapshot — are untouched; they all pass a non-empty value. Two registry tests pin each side: empty cwd lands the child in $HOME, a stated cwd is never contested by it. Co-Authored-By: Claude Opus 5 (1M context) --- internal/session/registry.go | 21 +++++++- internal/session/registry_test.go | 79 +++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/internal/session/registry.go b/internal/session/registry.go index 5ba7b07..d4a92c2 100644 --- a/internal/session/registry.go +++ b/internal/session/registry.go @@ -162,8 +162,22 @@ func (r *Registry) start(opts SpawnOpts, id string, preload []byte, restore Info argv = []string{shell, "-l"} } + // An empty Cwd is a caller with no preference, and it must not fall + // through to the daemon's own directory: a service-started daemon runs at + // / — neither unit in internal/service sets a working directory — and a + // hand-started one runs wherever it happened to be launched from, so + // every default session would open somewhere nobody chose. Home is what + // a terminal emulator or sshd would have picked, and it is where Revive + // already lands a session whose directory has vanished. When even home is + // unknowable the empty string stands and the shell inherits — the old + // behaviour, kept as the floor rather than the default. + cwd := opts.Cwd + if cwd == "" { + cwd, _ = os.UserHomeDir() + } + cmd := exec.Command(argv[0], argv[1:]...) - cmd.Dir = opts.Cwd + cmd.Dir = cwd cmd.Env = sessionEnv(os.Environ(), shell) cols, rows := opts.Cols, opts.Rows @@ -184,7 +198,10 @@ func (r *Registry) start(opts SpawnOpts, id string, preload []byte, restore Info size = DefaultRingSize } - cwd := opts.Cwd + // The recorded cwd is the value resolved above, so what Info reports is + // where the shell actually started rather than a second opinion. It is + // only still empty when home was unresolvable, and then the child + // inherited the daemon's directory — so that is what gets recorded. if cwd == "" { cwd, _ = os.Getwd() } diff --git a/internal/session/registry_test.go b/internal/session/registry_test.go index c6780e5..2b03f3e 100644 --- a/internal/session/registry_test.go +++ b/internal/session/registry_test.go @@ -3,6 +3,7 @@ package session import ( "strings" "testing" + "time" ) func TestLoginShellPrefersSHELL(t *testing.T) { @@ -82,6 +83,84 @@ func TestSessionEnvKeepsAnExistingSHELL(t *testing.T) { } } +// TestSpawnDefaultsCwdToHome pins where a session with no stated directory +// opens: the user's home, like any terminal emulator or sshd. The alternative +// — inheriting the daemon's own directory — is the launchd/systemd bug: a +// service-started daemon runs at /, so every plain new session opened there. +// os.UserHomeDir answers from $HOME on every platform flue targets, so +// setting it is setting the expectation. Both the recorded cwd and the +// child's real one are checked, because the fix is one resolution feeding +// both — a seed that disagreed with the process would be the old split back +// again. The poll and the symlink resolution are cwd_test.go's, for +// cwd_test.go's reasons. +func TestSpawnDefaultsCwdToHome(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + want := resolved(t, home) + + r := NewRegistry(nil) + s, err := r.Spawn(SpawnOpts{Cmd: []string{"sleep", "5"}, Cols: 80, Rows: 24}) + if err != nil { + t.Fatalf("Spawn: %v", err) + } + t.Cleanup(func() { _ = s.Close() }) + + s.mu.Lock() + seeded := s.info.Cwd + s.mu.Unlock() + if seeded != home { + t.Fatalf("recorded Cwd = %q, want the home %q", seeded, home) + } + + deadline := time.Now().Add(5 * time.Second) + for { + got, err := processCwd(s.pid) + if err == nil && resolved(t, got) == want { + return + } + if time.Now().After(deadline) { + t.Fatalf("processCwd(%d) = %q, %v; want %q", s.pid, got, err, want) + } + time.Sleep(5 * time.Millisecond) + } +} + +// TestSpawnKeepsExplicitCwd is the other half of the default: a caller that +// states a directory gets that directory, and home never competes with it. +// The stated one is deliberately not $HOME, so a spawn that reached for home +// anyway could not pass by coincidence. +func TestSpawnKeepsExplicitCwd(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + dir := t.TempDir() + want := resolved(t, dir) + + r := NewRegistry(nil) + s, err := r.Spawn(SpawnOpts{Cwd: dir, Cmd: []string{"sleep", "5"}, Cols: 80, Rows: 24}) + if err != nil { + t.Fatalf("Spawn: %v", err) + } + t.Cleanup(func() { _ = s.Close() }) + + s.mu.Lock() + seeded := s.info.Cwd + s.mu.Unlock() + if seeded != dir { + t.Fatalf("recorded Cwd = %q, want the stated %q", seeded, dir) + } + + deadline := time.Now().Add(5 * time.Second) + for { + got, err := processCwd(s.pid) + if err == nil && resolved(t, got) == want { + return + } + if time.Now().After(deadline) { + t.Fatalf("processCwd(%d) = %q, %v; want %q", s.pid, got, err, want) + } + time.Sleep(5 * time.Millisecond) + } +} + func contains(env []string, kv string) bool { for _, e := range env { if e == kv {