From 6546116e28d784af58e7d7956a9382dc35b73cac Mon Sep 17 00:00:00 2001 From: Sahil Bansal Date: Tue, 12 May 2026 22:27:05 +0530 Subject: [PATCH] feat: migrate PMG to use PATH shims for package manager wrapper (#246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add FilterPMGFromPath utility for PATH shim recursion prevention * feat: add FilterPMGFromEnv to filter PATH from env slices * feat: filter ~/.pmg/bin from PATH in proxy subprocess env * feat: add PathExport method to Shell interface for shim PATH integration * feat: add ShimManager for PATH shim install/remove lifecycle * feat: wire ShimManager into setup commands with --use-aliases fallback * refactor: add DefaultShimConfig helper to reduce setup boilerplate * fix: resolve real binary path to prevent shim double-invocation exec.CommandContext resolves the binary using the current process PATH, which still contains ~/.pmg/bin. This caused pmg to launch the shim instead of the real package manager, resulting in a second pmg instance with its own proxy — producing duplicate error messages and wasted work. ResolveRealBinary searches a filtered PATH (without ~/.pmg/bin) to find the real package manager binary before execution. * fix: resolve real binary in runner.Execute and expand path resolution tests Ensure guard mode and proxy skip paths also resolve through ResolveRealBinary to prevent infinite shim recursion. Add table-driven tests covering error cases, multi-binary PATH, and PATH restoration. * fix: handle error return values from os.Setenv and file Close calls Address errcheck lint failures: check os.Setenv returns in ResolveRealBinary, and check f.Close/tempFile.Close in ShimManager. * feat: auto-migrate shell aliases to PATH shims on setup install When running `pmg setup install`, detect existing shell aliases and automatically remove them before installing shims. Existing users get a seamless migration with no extra flags or commands needed. * fix: update E2E test to verify shim installation instead of alias RC file Replace the .pmg.rc file check with assertions that ~/.pmg/bin/ exists and contains executable shim scripts for npm and pip. * feat: add FilterPMGFromPath utility for PATH shim recursion prevention * feat: add FilterPMGFromEnv to filter PATH from env slices * feat: filter ~/.pmg/bin from PATH in proxy subprocess env * feat: add PathExport method to Shell interface for shim PATH integration * feat: add ShimManager for PATH shim install/remove lifecycle * feat: wire ShimManager into setup commands with --use-aliases fallback * refactor: add DefaultShimConfig helper to reduce setup boilerplate * fix: resolve real binary path to prevent shim double-invocation exec.CommandContext resolves the binary using the current process PATH, which still contains ~/.pmg/bin. This caused pmg to launch the shim instead of the real package manager, resulting in a second pmg instance with its own proxy — producing duplicate error messages and wasted work. ResolveRealBinary searches a filtered PATH (without ~/.pmg/bin) to find the real package manager binary before execution. * fix: resolve real binary in runner.Execute and expand path resolution tests Ensure guard mode and proxy skip paths also resolve through ResolveRealBinary to prevent infinite shim recursion. Add table-driven tests covering error cases, multi-binary PATH, and PATH restoration. * fix: handle error return values from os.Setenv and file Close calls Address errcheck lint failures: check os.Setenv returns in ResolveRealBinary, and check f.Close/tempFile.Close in ShimManager. * feat: auto-migrate shell aliases to PATH shims on setup install When running `pmg setup install`, detect existing shell aliases and automatically remove them before installing shims. Existing users get a seamless migration with no extra flags or commands needed. * fix: update E2E test to verify shim installation instead of alias RC file Replace the .pmg.rc file check with assertions that ~/.pmg/bin/ exists and contains executable shim scripts for npm and pip. * feat: install both aliases and shims for full coverage Aliases win in interactive shells (including venvs), shims catch non-interactive contexts (IDEs, CI, subprocesses). Remove --use-aliases flag and migration logic since both are always installed together. Update E2E to verify all shim scripts and alias RC file. * feat: address review feedback for shim implementation - Install both aliases and shims together for full coverage - Move homeDir resolution into NewDefaultShimManager (internal concern) - Add mutex to ResolveRealBinary to guard against concurrent PATH mutation - Use filepath.SplitList for platform-correct PATH splitting - Add ResolveRealBinary to runner.Execute and proxy flow to prevent shim recursion in all execution paths - Remove print side-effects from ShimManager.Remove - Update E2E to verify all shim scripts and alias RC file - Expand ResolveRealBinary tests with table-driven cases * fix: restore errcheck handling and add concurrency test for ResolveRealBinary - Restore proper defer with log.Warnf for PATH restoration in ResolveRealBinary - Restore errcheck handling for f.Close() and tempFile.Close() in ShimManager - Add explanatory comment for ResolveRealBinary call in proxy_flow - Add TestResolveRealBinaryConcurrent to verify mutex guards concurrent access * feat: skip shell integration on Windows with informative warning On Windows, pmg setup install now writes only the config file and prints a warning that shell aliases and PATH shims require WSL. * fix: PMG use pre-resolved binary path (#253) --------- Co-authored-by: Abhisek Datta --- .github/workflows/pmg-e2e.yml | 6 +- cmd/setup/setup.go | 89 ++++++++---- internal/alias/alias.go | 2 - internal/alias/bash.go | 4 + internal/alias/fish.go | 6 + internal/alias/shells.go | 6 + internal/alias/shells_test.go | 46 ++++++ internal/alias/zsh.go | 4 + internal/flows/proxy_flow.go | 13 +- internal/runner/execute.go | 8 +- internal/shim/path.go | 75 ++++++++++ internal/shim/path_test.go | 256 ++++++++++++++++++++++++++++++++++ internal/shim/shim.go | 245 ++++++++++++++++++++++++++++++++ internal/shim/shim_test.go | 200 ++++++++++++++++++++++++++ internal/ui/info.go | 12 +- 15 files changed, 936 insertions(+), 36 deletions(-) create mode 100644 internal/shim/path.go create mode 100644 internal/shim/path_test.go create mode 100644 internal/shim/shim.go create mode 100644 internal/shim/shim_test.go diff --git a/.github/workflows/pmg-e2e.yml b/.github/workflows/pmg-e2e.yml index 8cb46d3..22a7f2d 100644 --- a/.github/workflows/pmg-e2e.yml +++ b/.github/workflows/pmg-e2e.yml @@ -78,9 +78,13 @@ jobs: run: | test -f $HOME/.config/safedep/pmg/config.yml - - name: Test pmg.rc File is Created + - name: Test PMG Aliases and Shims are Installed run: | test -f $HOME/.pmg.rc + test -d $HOME/.pmg/bin + for shim in npm pip pip3 pnpm bun uv yarn poetry npx pnpx; do + test -x $HOME/.pmg/bin/$shim || { echo "Missing shim: $shim"; exit 1; } + done - name: Test NPM - Single Package & Manifest run: | diff --git a/cmd/setup/setup.go b/cmd/setup/setup.go index 545062e..073ed90 100644 --- a/cmd/setup/setup.go +++ b/cmd/setup/setup.go @@ -3,9 +3,11 @@ package setup import ( "fmt" "os" + "runtime" "github.com/safedep/pmg/config" "github.com/safedep/pmg/internal/alias" + "github.com/safedep/pmg/internal/shim" "github.com/safedep/pmg/internal/ui" "github.com/safedep/pmg/internal/version" "github.com/spf13/cobra" @@ -18,8 +20,8 @@ var ( func NewSetupCommand() *cobra.Command { setupCmd := &cobra.Command{ Use: "setup", - Short: "Manage PMG shell aliases and integration", - Long: "Setup and manage PMG config, shell aliases that allow you to use package manager commands with security guardrails.", + Short: "Manage PMG shell integration (aliases and shims)", + Long: "Setup and manage PMG config, shell aliases and PATH shims that allow you to use package manager commands with security guardrails.", RunE: func(cmd *cobra.Command, args []string) error { return cmd.Help() }, @@ -36,42 +38,60 @@ func NewSetupCommand() *cobra.Command { func NewInstallCommand() *cobra.Command { return &cobra.Command{ Use: "install", - Short: "Setup PMG config and aliases for package managers (npm, pnpm, pip, and more)", + 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)) - - cfg := alias.DefaultConfig() - rcFileManager, err := alias.NewDefaultRcFileManager(cfg.RcFileName) - if err != nil { - return fmt.Errorf("failed to create alias manager: %w", err) - } - - aliasManager := alias.New(cfg, rcFileManager) - err = aliasManager.Install() - if err != nil { - return fmt.Errorf("failed to install aliases: %w", err) - } - - if err := config.WriteTemplateConfig(); err != nil { - return fmt.Errorf("failed to write template config: %w", err) - } - - ui.PrintSetupInstallCmdInfo(aliasManager.GetRcPath(), config.Get().ConfigDir()) - return nil + return install() }, } } +func install() error { + if err := config.WriteTemplateConfig(); err != nil { + return fmt.Errorf("failed to write template config: %w", err) + } + + if runtime.GOOS == "windows" { + fmt.Printf("%s %s\n", ui.Colors.Green("✓"), "PMG config written successfully") + fmt.Printf(" %s\n", ui.Colors.Dim(fmt.Sprintf("Config: %s", config.Get().ConfigDir()))) + fmt.Printf("\n%s Shell aliases and PATH shims are not supported on Windows. Use WSL for full shell integration.\n", + ui.Colors.Yellow("⚠")) + return nil + } + + cfg := alias.DefaultConfig() + rcFileManager, err := alias.NewDefaultRcFileManager(cfg.RcFileName) + if err != nil { + return fmt.Errorf("failed to create alias manager: %w", err) + } + + aliasManager := alias.New(cfg, rcFileManager) + if err := aliasManager.Install(); err != nil { + return fmt.Errorf("failed to install aliases: %w", err) + } + + shimMgr, err := shim.NewDefaultShimManager() + if err != nil { + return fmt.Errorf("failed to create shim manager: %w", err) + } + + if err := shimMgr.Install(); err != nil { + return fmt.Errorf("failed to install shims: %w", err) + } + + ui.PrintSetupInstallCmdInfo(aliasManager.GetRcPath(), shimMgr.GetBinDir(), config.Get().ConfigDir()) + return nil +} + func NewRemoveCommand() *cobra.Command { cmd := &cobra.Command{ Use: "remove", - Short: "Removes pmg aliases from the user's shell config file.", + 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)) - // We remove the config file only if explicitly asked to do so. if setupRemoveConfigFile { config := config.Get() if err := os.Remove(config.ConfigFilePath()); err != nil && !os.IsNotExist(err) { @@ -79,6 +99,11 @@ func NewRemoveCommand() *cobra.Command { } } + if runtime.GOOS == "windows" { + fmt.Printf("%s %s\n", ui.Colors.Green("✓"), "PMG config removed. No aliases or shims to clean up on Windows.") + return nil + } + cfg := alias.DefaultConfig() rcFileManager, err := alias.NewDefaultRcFileManager(cfg.RcFileName) if err != nil { @@ -86,7 +111,21 @@ func NewRemoveCommand() *cobra.Command { } aliasManager := alias.New(cfg, rcFileManager) - return aliasManager.Remove() + if err := aliasManager.Remove(); err != nil { + return fmt.Errorf("failed to remove aliases: %w", err) + } + + shimMgr, err := shim.NewDefaultShimManager() + if err != nil { + return fmt.Errorf("failed to create shim manager: %w", err) + } + + if err := shimMgr.Remove(); err != nil { + return fmt.Errorf("failed to remove shims: %w", err) + } + + fmt.Printf("%s %s\n", ui.Colors.Green("✓"), "PMG aliases and shims removed. Restart your terminal for changes to take effect") + return nil }, } diff --git a/internal/alias/alias.go b/internal/alias/alias.go index 1346b5c..4aa3795 100644 --- a/internal/alias/alias.go +++ b/internal/alias/alias.go @@ -9,7 +9,6 @@ import ( "strings" "github.com/safedep/dry/log" - "github.com/safedep/pmg/internal/ui" ) // AliasManager manages shell aliases for package managers. @@ -135,7 +134,6 @@ func (a *AliasManager) Remove() error { return fmt.Errorf("failed to clean shell configs: %w", err) } - fmt.Printf("%s %s\n", ui.Colors.Green("✓"), "PMG config removed. Existing aliases need a shell restart") return nil } diff --git a/internal/alias/bash.go b/internal/alias/bash.go index 964fb68..9ed24c3 100644 --- a/internal/alias/bash.go +++ b/internal/alias/bash.go @@ -12,6 +12,10 @@ func (b bashShell) Source(rcPath string) string { return defaultShellSource(rcPath) } +func (b bashShell) PathExport(binDir string) string { + return defaultPathExport(binDir) +} + func (b bashShell) Name() string { return "bash" } diff --git a/internal/alias/fish.go b/internal/alias/fish.go index 913d513..ad4ae13 100644 --- a/internal/alias/fish.go +++ b/internal/alias/fish.go @@ -1,5 +1,7 @@ package alias +import "fmt" + type fishShell struct{} var _ Shell = &fishShell{} @@ -12,6 +14,10 @@ func (f fishShell) Source(rcPath string) string { return defaultShellSource(rcPath) } +func (f fishShell) PathExport(binDir string) string { + return fmt.Sprintf("%s\nfish_add_path --prepend \"%s\" # PMG shims\n", commentForRemovingShellShims, binDir) +} + func (f fishShell) Name() string { return "fish" } diff --git a/internal/alias/shells.go b/internal/alias/shells.go index 77e42d7..7d07785 100644 --- a/internal/alias/shells.go +++ b/internal/alias/shells.go @@ -8,11 +8,17 @@ import ( type Shell interface { Source(rcPath string) string + PathExport(binDir string) string Name() string Path() string } var commentForRemovingShellSource = "# remove aliases by running `pmg setup remove` or deleting the line" +var commentForRemovingShellShims = "# remove PMG shims by running `pmg setup remove` or deleting the line" + +func defaultPathExport(binDir string) string { + return fmt.Sprintf("%s\nexport PATH=\"%s:$PATH\" # PMG shims\n", commentForRemovingShellShims, binDir) +} func defaultShellSource(rcPath string) string { return fmt.Sprintf("%s \n[ -f '%s' ] && source '%s' # PMG source aliases\n", commentForRemovingShellSource, rcPath, rcPath) diff --git a/internal/alias/shells_test.go b/internal/alias/shells_test.go index 534ac4d..5d33452 100644 --- a/internal/alias/shells_test.go +++ b/internal/alias/shells_test.go @@ -7,6 +7,52 @@ import ( "github.com/stretchr/testify/assert" ) +func TestShellPathExport(t *testing.T) { + tests := []struct { + name string + shell Shell + binDir string + contains []string + }{ + { + name: "bash path export", + shell: &bashShell{}, + binDir: "/home/user/.pmg/bin", + contains: []string{ + `export PATH="/home/user/.pmg/bin:$PATH"`, + "PMG shims", + }, + }, + { + name: "zsh path export", + shell: &zshShell{}, + binDir: "/home/user/.pmg/bin", + contains: []string{ + `export PATH="/home/user/.pmg/bin:$PATH"`, + "PMG shims", + }, + }, + { + name: "fish path export", + shell: &fishShell{}, + binDir: "/home/user/.pmg/bin", + contains: []string{ + `fish_add_path --prepend "/home/user/.pmg/bin"`, + "PMG shims", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := tc.shell.PathExport(tc.binDir) + for _, s := range tc.contains { + assert.Contains(t, result, s) + } + }) + } +} + func TestDetectShell(t *testing.T) { cases := []struct { name string diff --git a/internal/alias/zsh.go b/internal/alias/zsh.go index 7d291d8..f95a516 100644 --- a/internal/alias/zsh.go +++ b/internal/alias/zsh.go @@ -12,6 +12,10 @@ func (z zshShell) Source(rcPath string) string { return defaultShellSource(rcPath) } +func (z zshShell) PathExport(binDir string) string { + return defaultPathExport(binDir) +} + func (z zshShell) Name() string { return "zsh" } diff --git a/internal/flows/proxy_flow.go b/internal/flows/proxy_flow.go index 8c89b1c..a962897 100644 --- a/internal/flows/proxy_flow.go +++ b/internal/flows/proxy_flow.go @@ -17,6 +17,7 @@ import ( "github.com/safedep/pmg/internal/audit" "github.com/safedep/pmg/internal/pty" "github.com/safedep/pmg/internal/runner" + "github.com/safedep/pmg/internal/shim" "github.com/safedep/pmg/internal/ui" "github.com/safedep/pmg/packagemanager" "github.com/safedep/pmg/proxy" @@ -209,6 +210,16 @@ func (f *proxyFlow) Run(ctx context.Context, args []string, parsedCmd *packagema proxyEnv := f.setupEnvForProxy(proxyAddr, caCertPath) + // Resolve the real package manager binary by searching PATH with ~/.pmg/bin + // stripped out. Without this, exec.CommandContext resolves to the shim script + // (because ~/.pmg/bin is still in the current process's PATH), causing + // infinite recursion: shim → pmg → shim → pmg → ... + realBinary, err := shim.ResolveRealBinary(parsedCmd.Command.Exe) + if err != nil { + return fmt.Errorf("failed to resolve real %s binary: %w", parsedCmd.Command.Exe, err) + } + parsedCmd.Command.Exe = realBinary + var executionError error if pty.IsInteractiveTerminal() { // Execute the package manager command with proxy environment variables @@ -330,7 +341,7 @@ func (f *proxyFlow) setupEnvForProxy(proxyAddr, caCertPath string) []string { noProxyList := "localhost,127.0.0.1,[::1]" - env := os.Environ() + env := shim.FilterPMGFromEnv(os.Environ()) env = append(env, "NODE_USE_ENV_PROXY=1", fmt.Sprintf("HTTP_PROXY=%s", proxyURL), diff --git a/internal/runner/execute.go b/internal/runner/execute.go index 9e5fa15..31e493f 100644 --- a/internal/runner/execute.go +++ b/internal/runner/execute.go @@ -7,6 +7,7 @@ import ( "os/exec" "github.com/safedep/dry/log" + "github.com/safedep/pmg/internal/shim" "github.com/safedep/pmg/packagemanager" "github.com/safedep/pmg/sandbox/executor" ) @@ -23,7 +24,12 @@ func Execute(ctx context.Context, pc *packagemanager.ParsedCommand, pmName strin return nil } - cmd := exec.CommandContext(ctx, pc.Command.Exe, pc.Command.Args...) + realBinary, err := shim.ResolveRealBinary(pc.Command.Exe) + if err != nil { + return fmt.Errorf("failed to resolve real %s binary: %w", pc.Command.Exe, err) + } + + cmd := exec.CommandContext(ctx, realBinary, pc.Command.Args...) cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr diff --git a/internal/shim/path.go b/internal/shim/path.go new file mode 100644 index 0000000..0bbda76 --- /dev/null +++ b/internal/shim/path.go @@ -0,0 +1,75 @@ +package shim + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + + "github.com/safedep/dry/log" +) + +const pmgBinSuffix = "/.pmg/bin" + +var resolverMu sync.Mutex + +func FilterPMGFromPath(pathEnv string) string { + if pathEnv == "" { + return "" + } + + entries := filepath.SplitList(pathEnv) + filtered := make([]string, 0, len(entries)) + + for _, entry := range entries { + if !strings.HasSuffix(entry, pmgBinSuffix) { + filtered = append(filtered, entry) + } + } + + return strings.Join(filtered, string(os.PathListSeparator)) +} + +// ResolveRealBinary finds the real binary path for a command by searching +// PATH with ~/.pmg/bin stripped out. This prevents exec.CommandContext from +// resolving to the shim script, which would cause infinite recursion. +func ResolveRealBinary(name string) (string, error) { + resolverMu.Lock() + defer resolverMu.Unlock() + + originalPath := os.Getenv("PATH") + filteredPath := FilterPMGFromPath(originalPath) + + if err := os.Setenv("PATH", filteredPath); err != nil { + return "", fmt.Errorf("failed to set filtered PATH: %w", err) + } + defer func() { + if err := os.Setenv("PATH", originalPath); err != nil { + log.Warnf("failed to restore PATH: %v", err) + } + }() + + resolved, err := exec.LookPath(name) + if err != nil { + return "", fmt.Errorf("could not find %s in PATH (excluding pmg shims): %w", name, err) + } + + return resolved, nil +} + +func FilterPMGFromEnv(env []string) []string { + result := make([]string, 0, len(env)) + + for _, entry := range env { + if pathValue, ok := strings.CutPrefix(entry, "PATH="); ok { + filtered := FilterPMGFromPath(pathValue) + result = append(result, "PATH="+filtered) + } else { + result = append(result, entry) + } + } + + return result +} diff --git a/internal/shim/path_test.go b/internal/shim/path_test.go new file mode 100644 index 0000000..7657f40 --- /dev/null +++ b/internal/shim/path_test.go @@ -0,0 +1,256 @@ +package shim + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFilterPMGFromPath(t *testing.T) { + tests := []struct { + name string + path string + expected string + }{ + { + name: "removes pmg bin from middle", + path: "/usr/local/bin:/home/user/.pmg/bin:/usr/bin", + expected: "/usr/local/bin:/usr/bin", + }, + { + name: "removes pmg bin from start", + path: "/home/user/.pmg/bin:/usr/local/bin:/usr/bin", + expected: "/usr/local/bin:/usr/bin", + }, + { + name: "removes pmg bin from end", + path: "/usr/local/bin:/usr/bin:/home/user/.pmg/bin", + expected: "/usr/local/bin:/usr/bin", + }, + { + name: "no pmg bin present", + path: "/usr/local/bin:/usr/bin", + expected: "/usr/local/bin:/usr/bin", + }, + { + name: "empty path", + path: "", + expected: "", + }, + { + name: "only pmg bin", + path: "/home/user/.pmg/bin", + expected: "", + }, + { + name: "does not remove partial matches", + path: "/usr/local/bin:/home/user/.pmg/binaries:/usr/bin", + expected: "/usr/local/bin:/home/user/.pmg/binaries:/usr/bin", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := FilterPMGFromPath(tc.path) + assert.Equal(t, tc.expected, result) + }) + } +} + +func TestResolveRealBinary(t *testing.T) { + tests := []struct { + name string + setupDirs func(t *testing.T, tmpDir string) (pmgBin, realBin string) + binary string + wantPath func(realBinDir string) string + wantErr bool + }{ + { + name: "skips shim and finds real binary", + setupDirs: func(t *testing.T, tmpDir string) (string, string) { + pmgBin := filepath.Join(tmpDir, ".pmg", "bin") + realBin := filepath.Join(tmpDir, "real-bin") + require.NoError(t, os.MkdirAll(pmgBin, 0o755)) + require.NoError(t, os.MkdirAll(realBin, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(pmgBin, "npm"), []byte("#!/bin/sh\necho shim"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(realBin, "npm"), []byte("#!/bin/sh\necho real"), 0o755)) + return pmgBin, realBin + }, + binary: "npm", + wantPath: func(realBin string) string { return filepath.Join(realBin, "npm") }, + }, + { + name: "returns error when binary not found outside shim dir", + setupDirs: func(t *testing.T, tmpDir string) (string, string) { + pmgBin := filepath.Join(tmpDir, ".pmg", "bin") + realBin := filepath.Join(tmpDir, "real-bin") + require.NoError(t, os.MkdirAll(pmgBin, 0o755)) + require.NoError(t, os.MkdirAll(realBin, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(pmgBin, "npm"), []byte("#!/bin/sh\necho shim"), 0o755)) + return pmgBin, realBin + }, + binary: "npm", + wantErr: true, + }, + { + name: "works when no shim dir exists in PATH", + setupDirs: func(t *testing.T, tmpDir string) (string, string) { + realBin := filepath.Join(tmpDir, "real-bin") + require.NoError(t, os.MkdirAll(realBin, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(realBin, "npm"), []byte("#!/bin/sh\necho real"), 0o755)) + return "", realBin + }, + binary: "npm", + wantPath: func(realBin string) string { return filepath.Join(realBin, "npm") }, + }, + { + name: "resolves correct binary when multiple exist", + setupDirs: func(t *testing.T, tmpDir string) (string, string) { + pmgBin := filepath.Join(tmpDir, ".pmg", "bin") + firstBin := filepath.Join(tmpDir, "first-bin") + secondBin := filepath.Join(tmpDir, "second-bin") + require.NoError(t, os.MkdirAll(pmgBin, 0o755)) + require.NoError(t, os.MkdirAll(firstBin, 0o755)) + require.NoError(t, os.MkdirAll(secondBin, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(pmgBin, "npm"), []byte("#!/bin/sh\necho shim"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(firstBin, "npm"), []byte("#!/bin/sh\necho first"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(secondBin, "npm"), []byte("#!/bin/sh\necho second"), 0o755)) + return pmgBin, firstBin + ":" + secondBin + }, + binary: "npm", + wantPath: func(realBin string) string { return filepath.Join(filepath.SplitList(realBin)[0], "npm") }, + }, + { + name: "restores original PATH after resolution", + setupDirs: func(t *testing.T, tmpDir string) (string, string) { + pmgBin := filepath.Join(tmpDir, ".pmg", "bin") + realBin := filepath.Join(tmpDir, "real-bin") + require.NoError(t, os.MkdirAll(pmgBin, 0o755)) + require.NoError(t, os.MkdirAll(realBin, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(pmgBin, "npm"), []byte("#!/bin/sh\necho shim"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(realBin, "npm"), []byte("#!/bin/sh\necho real"), 0o755)) + return pmgBin, realBin + }, + binary: "npm", + wantPath: func(realBin string) string { return filepath.Join(realBin, "npm") }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tmpDir := t.TempDir() + pmgBin, realBin := tc.setupDirs(t, tmpDir) + + var pathParts []string + if pmgBin != "" { + pathParts = append(pathParts, pmgBin) + } + pathParts = append(pathParts, filepath.SplitList(realBin)...) + + t.Setenv("PATH", strings.Join(pathParts, ":")) + originalPath := os.Getenv("PATH") + + resolved, err := ResolveRealBinary(tc.binary) + + if tc.wantErr { + assert.Error(t, err) + return + } + + require.NoError(t, err) + assert.Equal(t, tc.wantPath(realBin), resolved) + + assert.Equal(t, originalPath, os.Getenv("PATH"), "PATH should be restored after ResolveRealBinary") + }) + } +} + +func TestResolveRealBinaryConcurrent(t *testing.T) { + tmpDir := t.TempDir() + + pmgBin := filepath.Join(tmpDir, ".pmg", "bin") + realBin := filepath.Join(tmpDir, "real-bin") + require.NoError(t, os.MkdirAll(pmgBin, 0o755)) + require.NoError(t, os.MkdirAll(realBin, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(pmgBin, "npm"), []byte("#!/bin/sh\necho shim"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(realBin, "npm"), []byte("#!/bin/sh\necho real"), 0o755)) + + t.Setenv("PATH", pmgBin+":"+realBin) + + const goroutines = 10 + errs := make(chan error, goroutines) + paths := make(chan string, goroutines) + + for range goroutines { + go func() { + resolved, err := ResolveRealBinary("npm") + if err != nil { + errs <- err + paths <- "" + return + } + errs <- nil + paths <- resolved + }() + } + + expectedPath := filepath.Join(realBin, "npm") + for i := range goroutines { + assert.NoError(t, <-errs, "goroutine %d should not error", i) + resolved := <-paths + if resolved != "" { + assert.Equal(t, expectedPath, resolved, "goroutine %d should resolve to real binary", i) + } + } + + assert.Equal(t, pmgBin+":"+realBin, os.Getenv("PATH"), "PATH should be restored after concurrent calls") +} + +func TestFilterPMGFromEnv(t *testing.T) { + tests := []struct { + name string + env []string + expected []string + }{ + { + name: "filters PATH entry", + env: []string{ + "HOME=/home/user", + "PATH=/home/user/.pmg/bin:/usr/local/bin:/usr/bin", + "SHELL=/bin/zsh", + }, + expected: []string{ + "HOME=/home/user", + "PATH=/usr/local/bin:/usr/bin", + "SHELL=/bin/zsh", + }, + }, + { + name: "no PATH entry", + env: []string{ + "HOME=/home/user", + "SHELL=/bin/zsh", + }, + expected: []string{ + "HOME=/home/user", + "SHELL=/bin/zsh", + }, + }, + { + name: "empty env", + env: []string{}, + expected: []string{}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := FilterPMGFromEnv(tc.env) + assert.Equal(t, tc.expected, result) + }) + } +} diff --git a/internal/shim/shim.go b/internal/shim/shim.go new file mode 100644 index 0000000..cc2b5ad --- /dev/null +++ b/internal/shim/shim.go @@ -0,0 +1,245 @@ +package shim + +import ( + "bufio" + "bytes" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/safedep/dry/log" + "github.com/safedep/pmg/internal/alias" +) + +const shimMarker = "PMG shims" + +type ShimConfig struct { + BinDir string + HomeDir string + PMGBin string + PackageManagers []string + Shells []alias.Shell +} + +type ShimManager struct { + config ShimConfig +} + +func NewShimManager(config ShimConfig) *ShimManager { + return &ShimManager{config: config} +} + +func NewDefaultShimManager() (*ShimManager, error) { + homeDir, err := os.UserHomeDir() + if err != nil { + return nil, fmt.Errorf("failed to get home directory: %w", err) + } + + aliasCfg := alias.DefaultConfig() + pmgBin, err := currentExecutable() + if err != nil { + return nil, err + } + + return &ShimManager{config: ShimConfig{ + BinDir: filepath.Join(homeDir, ".pmg", "bin"), + HomeDir: homeDir, + PMGBin: pmgBin, + PackageManagers: aliasCfg.PackageManagers, + Shells: aliasCfg.Shells, + }}, nil +} + +func (m *ShimManager) Install() error { + if m.config.PMGBin == "" { + pmgBin, err := currentExecutable() + if err != nil { + return err + } + m.config.PMGBin = pmgBin + } + + if err := os.MkdirAll(m.config.BinDir, 0o755); err != nil { + return fmt.Errorf("failed to create shim directory %s: %w", m.config.BinDir, err) + } + + for _, pm := range m.config.PackageManagers { + if err := m.writeShimScript(pm); err != nil { + return fmt.Errorf("failed to write shim for %s: %w", pm, err) + } + } + + if err := m.addPathToShells(); err != nil { + return fmt.Errorf("failed to update shell configs: %w", err) + } + + return nil +} + +func (m *ShimManager) Remove() error { + if err := os.RemoveAll(m.config.BinDir); err != nil && !os.IsNotExist(err) { + log.Warnf("Warning: failed to remove shim directory: %v", err) + } + + if err := m.removePathFromShells(); err != nil { + return fmt.Errorf("failed to clean shell configs: %w", err) + } + + return nil +} + +func (m *ShimManager) IsInstalled() (bool, error) { + for _, shell := range m.config.Shells { + configPath := filepath.Join(m.config.HomeDir, shell.Path()) + + data, err := os.ReadFile(configPath) + if err != nil { + if os.IsNotExist(err) { + continue + } + log.Warnf("Warning: could not read %s (%s)", shell.Name(), err) + continue + } + + if strings.Contains(string(data), shimMarker) { + return true, nil + } + } + + return false, nil +} + +func (m *ShimManager) GetBinDir() string { + return m.config.BinDir +} + +func (m *ShimManager) writeShimScript(pm string) error { + shimPath := filepath.Join(m.config.BinDir, pm) + pmgBin := shellQuote(m.config.PMGBin) + + content := fmt.Sprintf(`#!/bin/sh +# PMG shim - do not edit, managed by pmg setup +PMG_BIN=%s +if [ ! -x "$PMG_BIN" ]; then + echo "[pmg] error: PMG binary not found or not executable: $PMG_BIN" >&2 + echo "[pmg] error: run 'pmg setup install' again or remove shims with 'pmg setup remove'" >&2 + exit 127 +fi +exec "$PMG_BIN" %s "$@" +`, pmgBin, pm) + + return os.WriteFile(shimPath, []byte(content), 0o755) +} + +func currentExecutable() (string, error) { + exe, err := os.Executable() + if err != nil { + return "", fmt.Errorf("failed to resolve pmg executable: %w", err) + } + + resolved, err := filepath.EvalSymlinks(exe) + if err != nil { + return filepath.Abs(exe) + } + + return resolved, nil +} + +func shellQuote(value string) string { + return "'" + strings.ReplaceAll(value, "'", "'\\''") + "'" +} + +func (m *ShimManager) addPathToShells() error { + for _, shell := range m.config.Shells { + configPath := filepath.Join(m.config.HomeDir, shell.Path()) + + data, err := os.ReadFile(configPath) + if err != nil { + if os.IsNotExist(err) { + continue + } + log.Warnf("Warning: skipping %s (%s)", shell.Name(), err) + continue + } + + if strings.Contains(string(data), shimMarker) { + continue + } + + f, err := os.OpenFile(configPath, os.O_APPEND|os.O_WRONLY, 0o644) + if err != nil { + log.Warnf("Warning: skipping %s (%s)", shell.Name(), err) + continue + } + + _, err = fmt.Fprintf(f, "\n%s", shell.PathExport(m.config.BinDir)) + if closeErr := f.Close(); closeErr != nil { + log.Warnf("Warning: failed to close %s: %s", shell.Name(), closeErr) + } + if err != nil { + log.Warnf("Warning: failed to write PATH export to %s: %s", shell.Name(), err) + } + } + + return nil +} + +func (m *ShimManager) removePathFromShells() error { + for _, shell := range m.config.Shells { + configPath := filepath.Join(m.config.HomeDir, shell.Path()) + + data, err := os.ReadFile(configPath) + if err != nil { + if os.IsNotExist(err) { + continue + } + log.Warnf("Warning: skipping %s (%s)", shell.Name(), err) + continue + } + + info, err := os.Stat(configPath) + if err != nil { + log.Warnf("Warning: skipping %s (%s)", shell.Name(), err) + continue + } + + tempFile, err := os.CreateTemp(filepath.Dir(configPath), ".tmp-"+filepath.Base(configPath)) + if err != nil { + log.Warnf("Warning: failed to create temporary file for %s: %s", configPath, err) + continue + } + tempPath := tempFile.Name() + + scanner := bufio.NewScanner(bytes.NewReader(data)) + writer := bufio.NewWriter(tempFile) + + for scanner.Scan() { + line := scanner.Text() + if strings.Contains(line, shimMarker) { + continue + } + if _, err := writer.WriteString(line + "\n"); err != nil { + log.Warnf("Warning: failed to write to temporary file: %s", err) + } + } + + if err := writer.Flush(); err != nil { + log.Warnf("Warning: failed to flush temporary file: %s", err) + } + if err := tempFile.Close(); err != nil { + log.Warnf("Warning: failed to close temporary file: %s", err) + } + + if err := os.Chmod(tempPath, info.Mode()); err != nil { + log.Warnf("Warning: failed to set permissions on temporary file: %s", err) + } + + if err := os.Rename(tempPath, configPath); err != nil { + _ = os.Remove(tempPath) + log.Warnf("Warning: failed to update %s: %s", configPath, err) + } + } + + return nil +} diff --git a/internal/shim/shim_test.go b/internal/shim/shim_test.go new file mode 100644 index 0000000..84d82a3 --- /dev/null +++ b/internal/shim/shim_test.go @@ -0,0 +1,200 @@ +package shim + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/safedep/pmg/internal/alias" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestShimManagerInstall(t *testing.T) { + homeDir := t.TempDir() + binDir := filepath.Join(homeDir, ".pmg", "bin") + + bashrc := filepath.Join(homeDir, ".bashrc") + zshrc := filepath.Join(homeDir, ".zshrc") + fishConfig := filepath.Join(homeDir, ".config", "fish") + require.NoError(t, os.MkdirAll(fishConfig, 0o755)) + require.NoError(t, os.WriteFile(bashrc, []byte("# existing bashrc\n"), 0o644)) + require.NoError(t, os.WriteFile(zshrc, []byte("# existing zshrc\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(fishConfig, "config.fish"), []byte("# existing fish config\n"), 0o644)) + + pms := []string{"npm", "pip"} + pmgBin := filepath.Join(homeDir, "bin", "pmg") + shells := []alias.Shell{ + &stubShell{name: "bash", path: ".bashrc", useFish: false}, + &stubShell{name: "fish", path: ".config/fish/config.fish", useFish: true}, + } + + mgr := NewShimManager(ShimConfig{ + BinDir: binDir, + HomeDir: homeDir, + PMGBin: pmgBin, + PackageManagers: pms, + Shells: shells, + }) + + require.NoError(t, mgr.Install()) + + for _, pm := range pms { + shimPath := filepath.Join(binDir, pm) + info, err := os.Stat(shimPath) + require.NoError(t, err, "shim %s should exist", pm) + assert.NotZero(t, info.Mode()&0o111, "shim %s should be executable", pm) + + content, err := os.ReadFile(shimPath) + require.NoError(t, err) + assert.Contains(t, string(content), "#!/bin/sh") + assert.Contains(t, string(content), "PMG_BIN='"+pmgBin+"'") + assert.Contains(t, string(content), `exec "$PMG_BIN" `+pm+` "$@"`) + assert.NotContains(t, string(content), "command -v pmg") + assert.NotContains(t, string(content), "exec pmg") + assert.NotContains(t, string(content), "falling back to native") + } + + bashContent, err := os.ReadFile(bashrc) + require.NoError(t, err) + assert.Contains(t, string(bashContent), ".pmg/bin") + + fishContent, err := os.ReadFile(filepath.Join(fishConfig, "config.fish")) + require.NoError(t, err) + assert.Contains(t, string(fishContent), ".pmg/bin") +} + +func TestShimManagerInstallIdempotent(t *testing.T) { + homeDir := t.TempDir() + binDir := filepath.Join(homeDir, ".pmg", "bin") + + bashrc := filepath.Join(homeDir, ".bashrc") + require.NoError(t, os.WriteFile(bashrc, []byte("# existing bashrc\n"), 0o644)) + + mgr := NewShimManager(ShimConfig{ + BinDir: binDir, + HomeDir: homeDir, + PackageManagers: []string{"npm"}, + Shells: []alias.Shell{&stubShell{name: "bash", path: ".bashrc", useFish: false}}, + }) + + require.NoError(t, mgr.Install()) + require.NoError(t, mgr.Install()) + + content, err := os.ReadFile(bashrc) + require.NoError(t, err) + + count := 0 + for _, line := range strings.Split(string(content), "\n") { + if strings.Contains(line, ".pmg/bin") { + count++ + } + } + assert.Equal(t, 1, count, "PATH export should appear exactly once") +} + +func TestShimManagerRemove(t *testing.T) { + homeDir := t.TempDir() + binDir := filepath.Join(homeDir, ".pmg", "bin") + + bashrc := filepath.Join(homeDir, ".bashrc") + require.NoError(t, os.WriteFile(bashrc, []byte("# existing bashrc\n"), 0o644)) + + mgr := NewShimManager(ShimConfig{ + BinDir: binDir, + HomeDir: homeDir, + PackageManagers: []string{"npm"}, + Shells: []alias.Shell{&stubShell{name: "bash", path: ".bashrc", useFish: false}}, + }) + + require.NoError(t, mgr.Install()) + require.NoError(t, mgr.Remove()) + + _, err := os.Stat(binDir) + assert.True(t, os.IsNotExist(err), "bin dir should be removed") + + content, err := os.ReadFile(bashrc) + require.NoError(t, err) + assert.NotContains(t, string(content), ".pmg/bin") +} + +func TestShimManagerIsInstalled(t *testing.T) { + homeDir := t.TempDir() + binDir := filepath.Join(homeDir, ".pmg", "bin") + + bashrc := filepath.Join(homeDir, ".bashrc") + require.NoError(t, os.WriteFile(bashrc, []byte("# existing bashrc\n"), 0o644)) + + mgr := NewShimManager(ShimConfig{ + BinDir: binDir, + HomeDir: homeDir, + PackageManagers: []string{"npm"}, + Shells: []alias.Shell{&stubShell{name: "bash", path: ".bashrc", useFish: false}}, + }) + + installed, err := mgr.IsInstalled() + require.NoError(t, err) + assert.False(t, installed) + + require.NoError(t, mgr.Install()) + + installed, err = mgr.IsInstalled() + require.NoError(t, err) + assert.True(t, installed) +} + +func TestNewDefaultShimManager(t *testing.T) { + mgr, err := NewDefaultShimManager() + require.NoError(t, err) + + assert.NotEmpty(t, mgr.GetBinDir()) + assert.Contains(t, mgr.GetBinDir(), ".pmg/bin") + assert.NotEmpty(t, mgr.config.PMGBin) + assert.True(t, filepath.IsAbs(mgr.config.PMGBin)) + assert.NotEmpty(t, mgr.config.PackageManagers) + assert.Contains(t, mgr.config.PackageManagers, "npm") + assert.Contains(t, mgr.config.PackageManagers, "pip") + assert.NotEmpty(t, mgr.config.Shells) +} + +func TestShimManagerInstallEscapesPMGBin(t *testing.T) { + homeDir := t.TempDir() + binDir := filepath.Join(homeDir, ".pmg", "bin") + pmgBin := filepath.Join(homeDir, "PMG's bin", "pmg") + + mgr := NewShimManager(ShimConfig{ + BinDir: binDir, + HomeDir: homeDir, + PMGBin: pmgBin, + PackageManagers: []string{"npm"}, + }) + + require.NoError(t, mgr.Install()) + + content, err := os.ReadFile(filepath.Join(binDir, "npm")) + require.NoError(t, err) + assert.Contains(t, string(content), `PMG_BIN='`+homeDir+`/PMG'\''s bin/pmg'`) + assert.NotContains(t, string(content), "command -v pmg") +} + +type stubShell struct { + name string + path string + useFish bool +} + +func (s *stubShell) Source(rcPath string) string { + return "" +} + +func (s *stubShell) PathExport(binDir string) string { + if s.useFish { + return fmt.Sprintf("fish_add_path --prepend \"%s\" # PMG shims\n", binDir) + } + return fmt.Sprintf("export PATH=\"%s:$PATH\" # PMG shims\n", binDir) +} + +func (s *stubShell) Name() string { return s.name } +func (s *stubShell) Path() string { return s.path } diff --git a/internal/ui/info.go b/internal/ui/info.go index 6399ebb..d4b2ee7 100644 --- a/internal/ui/info.go +++ b/internal/ui/info.go @@ -25,10 +25,10 @@ func PrintInfoSection(title string, entries map[string]string) { } } -// PrintSetupInstallCmdInfo prints a success message with the alias & config path, and a restart reminder. -func PrintSetupInstallCmdInfo(rcPath, configPath string) { - fmt.Printf("%s %s\n", Colors.Green("✓"), "PMG aliases installed successfully") - fmt.Printf(" %s\n", Colors.Dim(fmt.Sprintf("Installed to: %s", rcPath))) - fmt.Printf(" %s\n", Colors.Dim(fmt.Sprintf("Config at: %s", configPath))) - fmt.Printf(" %s\n", Colors.Dim("Restart your terminal or source your shell to use the new aliases")) +func PrintSetupInstallCmdInfo(aliasPath, shimBinDir, configPath string) { + fmt.Printf("%s %s\n", Colors.Green("✓"), "PMG installed successfully") + fmt.Printf(" %s\n", Colors.Dim(fmt.Sprintf("Aliases: %s", aliasPath))) + fmt.Printf(" %s\n", Colors.Dim(fmt.Sprintf("Shims: %s", shimBinDir))) + fmt.Printf(" %s\n", Colors.Dim(fmt.Sprintf("Config: %s", configPath))) + fmt.Printf(" %s\n", Colors.Dim("Restart your terminal for changes to take effect")) }