diff --git a/internal/health/installs.go b/internal/health/installs.go index c671374..ea2a3de 100644 --- a/internal/health/installs.go +++ b/internal/health/installs.go @@ -80,13 +80,46 @@ func HomebrewInstall(self string) (path, version string) { // logosVersion is what a logos at path answers to --version. Bounded, because // the file may be any program with that name. +// +// It runs in a home of its own with no vault named. Every logos does its +// startup work before it reads its arguments, and a 0.4.x logos's startup +// adopts ~/brain — it writes the vault pointer and renames .brain — so asking +// an old Homebrew install its version changed the user's vault. A version +// answer needs none of the user's state. func logosVersion(path string) (string, bool) { + home, err := os.MkdirTemp("", "logos-probe-") + if err != nil { + return "", false + } + defer os.RemoveAll(home) ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - out, err := exec.CommandContext(ctx, path, "--version").Output() + cmd := exec.CommandContext(ctx, path, "--version") + cmd.Env = probeEnv(home) + out, err := cmd.Output() fields := strings.Fields(string(out)) if err != nil || len(fields) < 2 || fields[0] != "logos" { return "", false } return strings.TrimPrefix(fields[1], "v"), true } + +// probeEnv is this process's environment with home, config and data dirs +// pointed at home and every LOGOS_ and BRAIN_ variable removed: those name a +// vault, a runtime or a host to wire, and the probe must reach none of them. +func probeEnv(home string) []string { + homeVars := map[string]bool{"HOME": true, "USERPROFILE": true, "APPDATA": true, "LOCALAPPDATA": true, + "XDG_CONFIG_HOME": true, "XDG_DATA_HOME": true, "XDG_STATE_HOME": true, "XDG_CACHE_HOME": true} + var env []string + for _, kv := range os.Environ() { + k, _, _ := strings.Cut(kv, "=") + if homeVars[strings.ToUpper(k)] || strings.HasPrefix(k, "LOGOS_") || strings.HasPrefix(k, "BRAIN_") { + continue + } + env = append(env, kv) + } + for k := range homeVars { + env = append(env, k+"="+home) + } + return env +} diff --git a/internal/health/installs_test.go b/internal/health/installs_test.go index 01126c4..b39d10f 100644 --- a/internal/health/installs_test.go +++ b/internal/health/installs_test.go @@ -84,3 +84,38 @@ func TestOneHomebrewInstallFirstOnPathReportsNothing(t *testing.T) { t.Errorf("a single install was reported as two: %+v", c) } } + +// Doctor asks another install its version by running it, and every logos does +// its startup work before it reads its arguments. A 0.4.x logos's startup +// adopts ~/brain, writing the vault pointer and renaming .brain, so a 0.5.0 +// doctor next to an old Homebrew logos changed the user's vault by looking. +func TestAskingAnotherInstallItsVersionCannotTouchTheUsersHomeOrVault(t *testing.T) { + prefix := t.TempDir() + cellar := filepath.Join(prefix, "Cellar", "logos-mcp", "0.4.9", "bin", "logos") + brewAt(t, prefix, "0.4.9") + // The old install's startup, as far as this test cares: it writes into + // whatever home and vault its environment names, then answers. + script := "#!/bin/sh\n" + + "mkdir -p \"$HOME/brain\" && touch \"$HOME/brain/adopted\"\n" + + "[ -n \"$LOGOS_VAULT\" ] && touch \"$LOGOS_VAULT/touched\"\n" + + "[ -n \"$BRAIN_VAULT\" ] && touch \"$BRAIN_VAULT/touched\"\n" + + "echo \"logos v0.4.9 darwin/arm64\"\n" + if err := os.WriteFile(cellar, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + home, vault := t.TempDir(), t.TempDir() + t.Setenv("HOME", home) + t.Setenv("LOGOS_VAULT", vault) + t.Setenv("BRAIN_VAULT", vault) + self := fakeLogos(t, filepath.Join(t.TempDir(), ".local", "bin", "logos"), "0.5.0") + + c := CheckOtherInstall(self, "v0.5.0") + if !strings.Contains(c.Detail, "0.4.9") { + t.Fatalf("the Homebrew install's version was not read: %+v", c) + } + for _, p := range []string{filepath.Join(home, "brain"), filepath.Join(vault, "touched")} { + if _, err := os.Stat(p); err == nil { + t.Errorf("running the other install to read its version wrote %s", p) + } + } +}