fix: harden and simplify Linux system install

Tighten shim detection, profile repair, and install ordering while
trimming over-specific doctor/info hints from the system-install path.

Co-authored-by: Cursor <cursoragent@cursor.com>

cmd/setup/doctor.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"slices"
 
 	"github.com/safedep/pmg/config"
 	"github.com/safedep/pmg/internal/alias"
@@ -120,12 +119,6 @@ func runCoreChecks(cfg *config.RuntimeConfig) []doctor.CheckResult {
 			Name:     checkShellAliases,
 			Category: "Shell Integration",
 			Run: func() doctor.CheckResult {
-				if shim.SystemShimsInstalled() {
-					return doctor.CheckResult{
-						Status:  doctor.StatusPass,
-						Message: "System shims installed (aliases optional)",
-					}
-				}
 				aliasCfg := alias.DefaultConfig()
 				rcFileManager, err := alias.NewDefaultRcFileManager(aliasCfg.RcFileName)
 				if err != nil {
@@ -142,15 +135,21 @@ func runCoreChecks(cfg *config.RuntimeConfig) []doctor.CheckResult {
 						Message: fmt.Sprintf("Could not determine alias status: %v", err),
 					}
 				}
-				if !installed {
+				if installed {
 					return doctor.CheckResult{
-						Status:  doctor.StatusFail,
-						Message: "Aliases not installed",
+						Status:  doctor.StatusPass,
+						Message: "Shell aliases installed",
+					}
+				}
+				if shim.SystemShimsInstalled() {
+					return doctor.CheckResult{
+						Status:  doctor.StatusWarn,
+						Message: "Aliases not installed (optional with system shims)",
 					}
 				}
 				return doctor.CheckResult{
-					Status:  doctor.StatusPass,
-					Message: "Shell aliases installed",
+					Status:  doctor.StatusFail,
+					Message: "Aliases not installed",
 				}
 			},
 		},
@@ -190,33 +189,29 @@ func runCoreChecks(cfg *config.RuntimeConfig) []doctor.CheckResult {
 			Category: "Shell Integration",
 			Run: func() doctor.CheckResult {
 				pathEntries := filepath.SplitList(os.Getenv("PATH"))
-				if shim.SystemShimsInstalled() {
-					systemDir := shim.SystemBinDir()
-					if slices.Contains(pathEntries, systemDir) {
-						return doctor.CheckResult{
-							Status:  doctor.StatusPass,
-							Message: "System shim directory is in PATH",
-						}
-					}
+				systemDir := shim.SystemBinDir()
+				if shim.SystemShimsInstalled() && pathContainsDir(pathEntries, systemDir) {
 					return doctor.CheckResult{
-						Status:  doctor.StatusFail,
-						Message: fmt.Sprintf("System shim directory not in PATH (add ENV PATH=\"%s:$PATH\" for Docker RUN)", systemDir),
+						Status:  doctor.StatusPass,
+						Message: "System shim directory is in PATH",
 					}
 				}
-				sm, err := shim.NewDefaultShimManager()
-				if err != nil {
-					return doctor.CheckResult{
-						Status:  doctor.StatusWarn,
-						Message: fmt.Sprintf("Could not check shims: %v", err),
-					}
+				userDir := ""
+				if home, err := os.UserHomeDir(); err == nil {
+					userDir = filepath.Join(home, ".pmg", "bin")
 				}
-				shimDir := sm.GetBinDir()
-				if slices.Contains(pathEntries, shimDir) {
+				if pathContainsDir(pathEntries, userDir) {
 					return doctor.CheckResult{
 						Status:  doctor.StatusPass,
 						Message: "Shim directory is in PATH",
 					}
 				}
+				if shim.SystemShimsInstalled() {
+					return doctor.CheckResult{
+						Status:  doctor.StatusFail,
+						Message: "System shim directory not in PATH",
+					}
+				}
 				return doctor.CheckResult{
 					Status:  doctor.StatusFail,
 					Message: "Shim directory not in PATH",
@@ -307,6 +302,19 @@ func runCoreChecks(cfg *config.RuntimeConfig) []doctor.CheckResult {
 	return doctor.RunChecks(checks)
 }
 
+func pathContainsDir(pathEntries []string, dir string) bool {
+	if dir == "" {
+		return false
+	}
+	cleanDir := filepath.Clean(dir)
+	for _, entry := range pathEntries {
+		if filepath.Clean(entry) == cleanDir {
+			return true
+		}
+	}
+	return false
+}
+
 func runProtectionChecks(coreResults []doctor.CheckResult) []doctor.CheckResult {
 	if !isInterceptionActive(coreResults) {
 		var results []doctor.CheckResult
@@ -366,15 +374,15 @@ var checkDisplayNames = map[string]string{
 var checkFixes = map[string]string{
 	checkConfigFile:         "pmg setup install",
 	checkEventLogDir:        "pmg setup install",
-	checkShellAliases:       "pmg setup install [--system]",
-	checkShimDirectory:      "pmg setup install [--system]",
-	checkShimInPath:         "Restart shell, source profile, or set ENV PATH for Docker",
+	checkShellAliases:       "pmg setup install",
+	checkShimDirectory:      "pmg setup install",
+	checkShimInPath:         "Restart shell or source profile",
 	checkProxyMode:          "Set proxy.enabled: true in config",
 	checkSandbox:            "Set sandbox.enabled: true in config",
 	checkDependencyCooldown: "Set dependency_cooldown.enabled: true in config",
 	checkEventLogging:       "Set skip_event_logging: false in config",
-	checkProtectionNpm:      "pmg setup install [--system]",
-	checkProtectionPip:      "pmg setup install [--system]",
+	checkProtectionNpm:      "pmg setup install",
+	checkProtectionPip:      "pmg setup install",
 	checkCA:                 "pmg setup cert install",
 }
 

cmd/setup/doctor_system_test.go

@@ -0,0 +1,23 @@
+package setup
+
+import (
+	"testing"
+
+	"github.com/safedep/pmg/internal/doctor"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestPathContainsDir(t *testing.T) {
+	assert.True(t, pathContainsDir([]string{"/usr/local/lib/pmg/bin/"}, "/usr/local/lib/pmg/bin"))
+	assert.False(t, pathContainsDir([]string{"/usr/local/bin"}, "/usr/local/lib/pmg/bin"))
+	assert.False(t, pathContainsDir([]string{"/usr/bin"}, ""))
+}
+
+func TestSystemShimsWithoutPathDoNotActivateInterception(t *testing.T) {
+	results := []doctor.CheckResult{
+		{Name: checkShellAliases, Status: doctor.StatusWarn},
+		{Name: checkShimInPath, Status: doctor.StatusFail},
+	}
+
+	assert.False(t, isInterceptionActive(results))
+}

cmd/setup/info.go

@@ -80,10 +80,10 @@ func executeSetupInfo() error {
 	shellEntries["Detected Shell"] = shell
 	shellEntries["Alias Installed"] = strconv.FormatBool(isInstalled)
 	shellEntries["User Shims"] = strconv.FormatBool(shim.UserShimsInstalled())
-	shellEntries["System Shims"] = strconv.FormatBool(shim.SystemShimsInstalled())
-	shellEntries["System Profile"] = strconv.FormatBool(shim.SystemProfileInstalled())
 	if shim.SystemShimsInstalled() {
-		shellEntries["System Shim Dir"] = shim.SystemBinDir()
+		shellEntries["System Shims"] = shim.SystemBinDir()
+	} else {
+		shellEntries["System Shims"] = "not installed"
 	}
 	ui.PrintInfoSection("Shell Integration", shellEntries)
 

cmd/setup/setup.go

@@ -16,12 +16,6 @@ import (
 	"github.com/spf13/cobra"
 )
 
-var (
-	setupRemoveConfigFile bool
-	setupInstallSystem    bool
-	setupRemoveSystem     bool
-)
-
 var setupGeteuid = os.Geteuid
 
 func NewSetupCommand() *cobra.Command {
@@ -45,21 +39,22 @@ func NewSetupCommand() *cobra.Command {
 }
 
 func NewInstallCommand() *cobra.Command {
+	var system bool
 	cmd := &cobra.Command{
 		Use:          "install",
 		Short:        "Setup PMG config, aliases, and shims for package managers (npm, pnpm, pip, and more)",
 		SilenceUsage: true,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			fmt.Print(ui.GeneratePMGBanner(version.Version, version.Commit))
-			return install()
+			return install(system)
 		},
 	}
-	cmd.Flags().BoolVar(&setupInstallSystem, "system", false, "Install system-wide for all users (Linux, requires root)")
+	cmd.Flags().BoolVar(&system, "system", false, "Install system-wide for all users (Linux, requires root)")
 	return cmd
 }
 
-func install() error {
-	if setupInstallSystem {
+func install(system bool) error {
+	if system {
 		return installSystem()
 	}
 
@@ -114,45 +109,50 @@ func installSystem() error {
 		return err
 	}
 
-	if err := config.WriteSystemTemplateConfig(); err != nil {
-		return fmt.Errorf("failed to write system config: %w", err)
-	}
-
 	shimMgr, err := shim.NewSystemShimManager()
 	if err != nil {
 		return fmt.Errorf("failed to create system shim manager: %w", err)
 	}
 
+	// Shims/profile first so a failed config write does not leave a managed
+	// config active without interception.
 	if err := shimMgr.Install(); err != nil {
 		return fmt.Errorf("failed to install system shims: %w", err)
 	}
+	if err := config.WriteSystemTemplateConfig(); err != nil {
+		return fmt.Errorf("failed to write system config: %w", err)
+	}
 
 	ui.PrintSetupSystemInstallCmdInfo(shimMgr.GetBinDir(), config.SystemConfigDir(), shim.SystemProfilePath())
 	return nil
 }
 
 func NewRemoveCommand() *cobra.Command {
+	var (
+		removeConfig bool
+		system       bool
+	)
 	cmd := &cobra.Command{
 		Use:          "remove",
 		Short:        "Removes pmg aliases and shims from the user's shell config.",
 		SilenceUsage: true,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			fmt.Print(ui.GeneratePMGBanner(version.Version, version.Commit))
-			return remove()
+			return remove(system, removeConfig)
 		},
 	}
 
-	cmd.Flags().BoolVar(&setupRemoveConfigFile, "config-file", false, "Remove the config file")
-	cmd.Flags().BoolVar(&setupRemoveSystem, "system", false, "Remove system-wide install (Linux, requires root)")
+	cmd.Flags().BoolVar(&removeConfig, "config-file", false, "Remove the config file")
+	cmd.Flags().BoolVar(&system, "system", false, "Remove system-wide install (Linux, requires root)")
 	return cmd
 }
 
-func remove() error {
-	if setupRemoveSystem {
-		return removeSystem()
+func remove(system, removeConfig bool) error {
+	if system {
+		return removeSystem(removeConfig)
 	}
 
-	if setupRemoveConfigFile {
+	if removeConfig {
 		// Only ever remove the per-user file; the globally managed
 		// config is not ours to delete from a per-user uninstall.
 		if err := config.RemoveUserConfigFile(); err != nil {
@@ -189,17 +189,11 @@ func remove() error {
 	return nil
 }
 
-func removeSystem() error {
+func removeSystem(removeConfig bool) error {
 	if err := errIfSystemInstallAllowed(); err != nil {
 		return err
 	}
 
-	if setupRemoveConfigFile {
-		if err := config.RemoveSystemConfigFile(); err != nil {
-			return err
-		}
-	}
-
 	shimMgr, err := shim.NewSystemShimManager()
 	if err != nil {
 		return fmt.Errorf("failed to create system shim manager: %w", err)
@@ -208,6 +202,11 @@ func removeSystem() error {
 	if err := shimMgr.Remove(); err != nil {
 		return fmt.Errorf("failed to remove system shims: %w", err)
 	}
+	if removeConfig {
+		if err := config.RemoveSystemConfigFile(); err != nil {
+			return err
+		}
+	}
 
 	fmt.Printf("%s %s\n", ui.Colors.Green("✓"), "PMG system install removed")
 	return nil

cmd/setup/setup_test.go

@@ -37,30 +37,19 @@ func TestErrIfSystemInstallAllowed(t *testing.T) {
 	}
 }
 
-func TestInstallSystemRequiresLinuxAndRoot(t *testing.T) {
+func TestInstallSystemRequiresRoot(t *testing.T) {
 	orig := setupGeteuid
-	t.Cleanup(func() {
-		setupGeteuid = orig
-		setupInstallSystem = false
-	})
+	t.Cleanup(func() { setupGeteuid = orig })
 
-	setupInstallSystem = true
-	setupGeteuid = func() int { return 0 }
-
-	err := install()
-	if runtime.GOOS == "linux" {
-		if err != nil {
-			usefulErr, ok := usefulerror.AsUsefulError(err)
-			if ok {
-				assert.NotEqual(t, errcodes.UnsupportedPlatform, usefulErr.Code())
-				assert.NotEqual(t, errcodes.PermissionDenied, usefulErr.Code())
-			}
-		}
-		return
-	}
+	setupGeteuid = func() int { return 1000 }
 
+	err := install(true)
 	require.Error(t, err)
 	usefulErr, ok := usefulerror.AsUsefulError(err)
 	require.True(t, ok)
-	assert.Equal(t, errcodes.UnsupportedPlatform, usefulErr.Code())
+	if runtime.GOOS == "linux" {
+		assert.Equal(t, errcodes.PermissionDenied, usefulErr.Code())
+	} else {
+		assert.Equal(t, errcodes.UnsupportedPlatform, usefulErr.Code())
+	}
 }