diff --git a/commands/daemon.install.go b/commands/daemon.install.go index 7d9a742..c84395c 100644 --- a/commands/daemon.install.go +++ b/commands/daemon.install.go @@ -67,7 +67,7 @@ func commandDaemonInstall(c *cli.Context) error { // recovery branch above can restore it on the next `daemon install` — // no GitHub re-download when the user later clears the system binary. curlDaemonPath := model.GetCurlInstallerDaemonPath() - if daemonBinPath != curlDaemonPath { + if shouldPreserveCurlDaemon(daemonBinPath, curlDaemonPath) { if info, statErr := os.Stat(curlDaemonPath); statErr == nil && !info.IsDir() { preservedPath := curlDaemonPath + ".bak" _ = os.Remove(preservedPath) @@ -105,3 +105,13 @@ func commandDaemonInstall(c *cli.Context) error { color.Green.Println("💡 Your commands will now be automatically synced to shelltime.xyz faster") return nil } + +// shouldPreserveCurlDaemon reports whether the curl-installer daemon at +// curlDaemonPath should be renamed to .bak before installing the service. It is +// true only when the resolved daemon is a genuinely different on-disk file — so +// we never move away the binary the service is about to point at, even when the +// resolved path is just a symlink/alias of the curl binary (the cause of the +// "shelltime-daemon.bak but no shelltime-daemon" stuck loop on Linux). +func shouldPreserveCurlDaemon(daemonBinPath, curlDaemonPath string) bool { + return daemonBinPath != curlDaemonPath && !model.SameDaemonFile(daemonBinPath, curlDaemonPath) +} diff --git a/commands/daemon.install_test.go b/commands/daemon.install_test.go new file mode 100644 index 0000000..a0212ac --- /dev/null +++ b/commands/daemon.install_test.go @@ -0,0 +1,53 @@ +package commands + +import ( + "os" + "path/filepath" + "testing" +) + +// TestShouldPreserveCurlDaemon pins the decision that gates renaming the +// curl-installer daemon to .bak. The critical case is the symlink alias: when +// resolution returns a path that is merely an alias of the curl binary, we must +// NOT move it aside — doing so was the cause of the Linux stuck loop where +// ~/.shelltime/bin ended up with shelltime-daemon.bak but no shelltime-daemon. +func TestShouldPreserveCurlDaemon(t *testing.T) { + dir := t.TempDir() + + curlPath := filepath.Join(dir, "shelltime-daemon") + if err := os.WriteFile(curlPath, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatal(err) + } + + // A genuinely different binary (e.g. a real Homebrew/system install). + distinct := filepath.Join(dir, "system-shelltime-daemon") + if err := os.WriteFile(distinct, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatal(err) + } + + // A symlink that points back at the curl binary — the same on-disk file. + aliasDir := t.TempDir() + alias := filepath.Join(aliasDir, "shelltime-daemon") + if err := os.Symlink(curlPath, alias); err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + daemonBinPath string + curlDaemonPath string + want bool + }{ + {"identical path strings", curlPath, curlPath, false}, + {"genuinely distinct files", distinct, curlPath, true}, + {"resolved path is a symlink alias of curl", alias, curlPath, false}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := shouldPreserveCurlDaemon(tc.daemonBinPath, tc.curlDaemonPath); got != tc.want { + t.Errorf("shouldPreserveCurlDaemon(%q, %q) = %v, want %v", + tc.daemonBinPath, tc.curlDaemonPath, got, tc.want) + } + }) + } +} diff --git a/model/path.go b/model/path.go index 639bb84..8463bb4 100644 --- a/model/path.go +++ b/model/path.go @@ -143,18 +143,23 @@ func ResolveDaemonBinaryPath() (string, error) { curlPath := GetCurlInstallerDaemonPath() // 1. Check PATH (covers Homebrew and other package managers). Ignore the - // result if it happens to resolve to the curl-installer path — we want - // step 3 to be the only branch that returns that location. + // result if it is the same on-disk file as the curl-installer binary + // (symlink/alias) — step 3 is the only branch that may return that file. if path, err := exec.LookPath(binaryName); err == nil { - if resolved, _ := filepath.Abs(path); resolved != curlPath { - return path, nil + if resolved, absErr := filepath.Abs(path); absErr == nil && !sameFile(resolved, curlPath) { + return resolved, nil } } // 2. Explicit Homebrew/Linuxbrew fallback paths, in case PATH was stripped. + // Same-file guard: a stray symlink in e.g. /usr/local/bin that points back + // at the curl binary must not be treated as a distinct system install. for _, dir := range daemonHomebrewSearchPaths { p := filepath.Join(dir, binaryName) if info, err := os.Stat(p); err == nil && !info.IsDir() { + if sameFile(p, curlPath) { + continue + } return p, nil } } @@ -166,3 +171,27 @@ func ResolveDaemonBinaryPath() (string, error) { return "", fmt.Errorf("%s not found on PATH, in standard Homebrew locations, or at %s", binaryName, curlPath) } + +// sameFile reports whether a and b refer to the same on-disk file. It follows +// symlinks (os.Stat) and compares device+inode via os.SameFile, so two +// different path strings that resolve to the same file — a symlinked $HOME, a +// realpath'd PATH entry, or a symlink in /usr/local/bin pointing back at the +// curl binary — are recognized as identical. It is conservative: any stat error +// (missing file, broken symlink, permission) yields false, and files on +// different filesystems correctly compare unequal (no inode collisions). +func sameFile(a, b string) bool { + ai, err := os.Stat(a) + if err != nil { + return false + } + bi, err := os.Stat(b) + if err != nil { + return false + } + return os.SameFile(ai, bi) +} + +// SameDaemonFile reports whether two daemon paths refer to the same on-disk +// file (symlink/hardlink/alias aware). Used by the installer to avoid moving the +// binary the service points at to .bak. +func SameDaemonFile(a, b string) bool { return sameFile(a, b) } diff --git a/model/path_test.go b/model/path_test.go index 93d0e27..1bb93c8 100644 --- a/model/path_test.go +++ b/model/path_test.go @@ -394,6 +394,53 @@ func TestResolveDaemonBinaryPath(t *testing.T) { } }) + t.Run("skips PATH result that is a symlink back to the curl path", func(t *testing.T) { + home := withIsolatedDaemonResolution(t) + + curl := writeFakeDaemon(t, filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin")) + + // A dir on PATH whose shelltime-daemon is only a symlink to the curl + // binary. LookPath finds it first, but it is the SAME on-disk file, so + // the resolver must fall through and return the real curl path — + // otherwise daemon.install would move the only binary to .bak. + linkDir := t.TempDir() + if err := os.Symlink(curl, filepath.Join(linkDir, "shelltime-daemon")); err != nil { + t.Fatalf("failed to create symlink: %v", err) + } + t.Setenv("PATH", linkDir) + + got, err := ResolveDaemonBinaryPath() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != curl { + t.Errorf("expected curl-installer path %s, got %s", curl, got) + } + }) + + t.Run("skips homebrew-search result that is a symlink back to the curl path", func(t *testing.T) { + home := withIsolatedDaemonResolution(t) + + curl := writeFakeDaemon(t, filepath.Join(home, COMMAND_BASE_STORAGE_FOLDER, "bin")) + + // Simulate a stray /usr/local/bin/shelltime-daemon symlink (reached via + // the explicit search list when PATH is stripped) pointing back at the + // curl binary. It must not be treated as a distinct system install. + fakeUsrLocal := t.TempDir() + if err := os.Symlink(curl, filepath.Join(fakeUsrLocal, "shelltime-daemon")); err != nil { + t.Fatalf("failed to create symlink: %v", err) + } + daemonHomebrewSearchPaths = []string{fakeUsrLocal} + + got, err := ResolveDaemonBinaryPath() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != curl { + t.Errorf("expected curl-installer path %s, got %s", curl, got) + } + }) + t.Run("returns error when no daemon is found anywhere", func(t *testing.T) { withIsolatedDaemonResolution(t) @@ -403,3 +450,45 @@ func TestResolveDaemonBinaryPath(t *testing.T) { } }) } + +func Test_sameFile(t *testing.T) { + dir := t.TempDir() + + fileA := filepath.Join(dir, "a") + if err := os.WriteFile(fileA, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + // Distinct file with identical contents — must NOT be reported as same. + fileB := filepath.Join(dir, "b") + if err := os.WriteFile(fileB, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + linkA := filepath.Join(dir, "link-a") + if err := os.Symlink(fileA, linkA); err != nil { + t.Fatal(err) + } + broken := filepath.Join(dir, "broken") + if err := os.Symlink(filepath.Join(dir, "does-not-exist"), broken); err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + a, b string + want bool + }{ + {"same path twice", fileA, fileA, true}, + {"symlink and its target", linkA, fileA, true}, + {"distinct files with identical contents", fileA, fileB, false}, + {"missing left operand", filepath.Join(dir, "nope"), fileA, false}, + {"missing right operand", fileA, filepath.Join(dir, "nope"), false}, + {"broken symlink", broken, fileA, false}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := sameFile(tc.a, tc.b); got != tc.want { + t.Errorf("sameFile(%q, %q) = %v, want %v", tc.a, tc.b, got, tc.want) + } + }) + } +}