diff --git a/cmd/entire/cli/agent/external/discovery.go b/cmd/entire/cli/agent/external/discovery.go index 177a9e27a..2fb5fcb1f 100644 --- a/cmd/entire/cli/agent/external/discovery.go +++ b/cmd/entire/cli/agent/external/discovery.go @@ -131,12 +131,17 @@ func discoverAndRegister(ctx context.Context) { } } -// StripExeExt removes Windows executable extensions (.exe, .bat, .cmd) from a -// file name so that the agent name derived from the binary matches on all platforms. -// On Unix this is effectively a no-op because binaries have no extension. +// StripExeExt removes Windows executable extensions (.exe, .bat, .cmd, .com) +// from a file name so that the derived name matches on all platforms. On Unix +// this is effectively a no-op because binaries have no extension. +// +// .com is included because Windows PATHEXT defaults to ".COM;.EXE;.BAT;.CMD;…", +// so exec.LookPath can resolve a `.com` next to a `.exe`. Without stripping +// it, a managed-plugin or agent-binary installer would treat foo.exe and +// foo.com as distinct names while PATHEXT silently picks one. func StripExeExt(name string) string { switch strings.ToLower(filepath.Ext(name)) { - case ".exe", ".bat", ".cmd": + case ".exe", ".bat", ".cmd", ".com": return strings.TrimSuffix(name, filepath.Ext(name)) } return name diff --git a/cmd/entire/cli/agent/external/external_test.go b/cmd/entire/cli/agent/external/external_test.go index 9c39b110c..c66f807eb 100644 --- a/cmd/entire/cli/agent/external/external_test.go +++ b/cmd/entire/cli/agent/external/external_test.go @@ -862,9 +862,11 @@ func TestStripExeExt(t *testing.T) { {name: "exe lowercase", in: "entire-agent-test.exe", want: "entire-agent-test"}, {name: "bat lowercase", in: "entire-agent-test.bat", want: "entire-agent-test"}, {name: "cmd lowercase", in: "entire-agent-test.cmd", want: "entire-agent-test"}, + {name: "com lowercase", in: "entire-agent-test.com", want: "entire-agent-test"}, {name: "exe uppercase", in: "entire-agent-test.EXE", want: "entire-agent-test"}, {name: "bat mixed case", in: "entire-agent-test.Bat", want: "entire-agent-test"}, {name: "cmd mixed case", in: "entire-agent-test.CmD", want: "entire-agent-test"}, + {name: "com uppercase", in: "entire-agent-test.COM", want: "entire-agent-test"}, {name: "no extension", in: "entire-agent-test", want: "entire-agent-test"}, {name: "unrelated extension", in: "entire-agent-test.sh", want: "entire-agent-test.sh"}, {name: "dot only", in: "entire-agent-test.", want: "entire-agent-test."}, diff --git a/cmd/entire/cli/explain.go b/cmd/entire/cli/explain.go index 27b79eac3..9794b9afa 100644 --- a/cmd/entire/cli/explain.go +++ b/cmd/entire/cli/explain.go @@ -2553,6 +2553,21 @@ func upsertEnv(env []string, key, value string) []string { return result } +// removeEnvKey returns env with every entry for key dropped. Useful when a +// caller wants to guarantee a child process inherits no value for key, even +// if the parent's environment has one set. +func removeEnvKey(env []string, key string) []string { + prefix := key + "=" + result := make([]string, 0, len(env)) + for _, e := range env { + if strings.HasPrefix(e, prefix) { + continue + } + result = append(result, e) + } + return result +} + // outputWithPager outputs content through a pager if stdout is a terminal and content is long. func outputWithPager(w io.Writer, content string) { // Check if we're writing to stdout and it's a terminal diff --git a/cmd/entire/cli/integration_test/external_command_test.go b/cmd/entire/cli/integration_test/external_command_test.go index 2d75037fb..3149b958f 100644 --- a/cmd/entire/cli/integration_test/external_command_test.go +++ b/cmd/entire/cli/integration_test/external_command_test.go @@ -233,6 +233,7 @@ func TestExternalCommand_EnvVarsForwarded(t *testing.T) { "#!/bin/sh\n{\n"+ " echo \"ENTIRE_CLI_VERSION=$ENTIRE_CLI_VERSION\"\n"+ " echo \"ENTIRE_REPO_ROOT=$ENTIRE_REPO_ROOT\"\n"+ + " echo \"ENTIRE_PLUGIN_DATA_DIR=$ENTIRE_PLUGIN_DATA_DIR\"\n"+ "} > %q\nexit 0\n", envFile, ) @@ -240,8 +241,10 @@ func TestExternalCommand_EnvVarsForwarded(t *testing.T) { t.Fatalf("write plugin: %v", err) } + // Pin the plugin parent dir so we can assert the per-plugin data path. + pluginRoot := t.TempDir() cmd := execx.NonInteractive(context.Background(), getTestBinary(), "envcheck") - cmd.Env = pathWith(pluginDir) + cmd.Env = append(pathWith(pluginDir), "ENTIRE_PLUGIN_DIR="+pluginRoot) cmd.Dir = resolvedRepo cmd.Stdout = &bytes.Buffer{} cmd.Stderr = &bytes.Buffer{} @@ -262,6 +265,10 @@ func TestExternalCommand_EnvVarsForwarded(t *testing.T) { if got, want := envVars["ENTIRE_REPO_ROOT"], resolvedRepo; got != want { t.Errorf("ENTIRE_REPO_ROOT = %q, want %q", got, want) } + wantData := filepath.Join(pluginRoot, "data", "envcheck") + if got := envVars["ENTIRE_PLUGIN_DATA_DIR"]; got != wantData { + t.Errorf("ENTIRE_PLUGIN_DATA_DIR = %q, want %q", got, wantData) + } } // writeEnvDumpPlugin creates an entire-envfilter plugin in its own dir diff --git a/cmd/entire/cli/plugin.go b/cmd/entire/cli/plugin.go index 6897d5b76..7eb6550e9 100644 --- a/cmd/entire/cli/plugin.go +++ b/cmd/entire/cli/plugin.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "log/slog" "os" "os/exec" "path/filepath" @@ -11,6 +12,7 @@ import ( "time" "github.com/entireio/cli/cmd/entire/cli/agent/external" + "github.com/entireio/cli/cmd/entire/cli/logging" "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/telemetry" "github.com/entireio/cli/cmd/entire/cli/versioncheck" @@ -42,7 +44,7 @@ func MaybeRunPlugin(ctx context.Context, rootCmd *cobra.Command, args []string) return false, 0 } pluginName := args[0] - exitCode = runPlugin(ctx, binPath, pluginArgs) + exitCode = runPlugin(ctx, pluginName, binPath, pluginArgs) if exitCode == 0 { maybeTrackPluginInvocation(ctx, pluginName) versioncheck.CheckAndNotify(ctx, os.Stdout, versioninfo.Version) @@ -123,21 +125,12 @@ func findInaccessiblePlugin(filename string) (string, bool) { return "", false } +// isPluginCandidate reports whether name is a syntactically valid plugin +// name the dispatcher should attempt to resolve. It is a thin bool wrapper +// over validatePluginName so the dispatcher's gate and the managed store's +// install-time check can never drift. func isPluginCandidate(name string) bool { - if name == "" { - return false - } - if strings.HasPrefix(name, "-") { - return false - } - // `agent-*` is reserved for the external agent protocol. - if strings.HasPrefix(name, "agent-") { - return false - } - if strings.ContainsAny(name, `/\`) { - return false - } - return true + return validatePluginName(name) == nil } // isAgentProtocolBinary returns true when the binary name is reserved for @@ -152,7 +145,7 @@ func isAgentProtocolBinary(binPath string) bool { // On context cancellation the child gets SIGINT (with a 5s grace before the // runtime falls back to SIGKILL) so plugins can clean up. Terminal signals // reach the child directly via the shared process group. -func runPlugin(ctx context.Context, binPath string, args []string) int { +func runPlugin(ctx context.Context, pluginName, binPath string, args []string) int { cmd := exec.CommandContext(ctx, binPath, args...) cmd.Cancel = func() error { return cmd.Process.Signal(os.Interrupt) } cmd.WaitDelay = 5 * time.Second @@ -163,7 +156,34 @@ func runPlugin(ctx context.Context, binPath string, args []string) int { if repoRoot, err := paths.WorktreeRoot(ctx); err == nil { extras = append(extras, "ENTIRE_REPO_ROOT="+repoRoot) } - cmd.Env = pluginEnv(os.Environ(), extras...) + // Per-plugin durable storage. Passed regardless of where the binary lives + // so plugins installed via raw PATH and via `entire plugin install` get + // the same contract. The dir is not pre-created — that's the plugin's + // responsibility on first use. + // + // PluginDataDir can only fail in degenerate environments (no resolvable + // home dir, or a relative ENTIRE_PLUGIN_DIR override). The plugin name + // itself already passed isPluginCandidate in resolvePlugin, so the name + // validator branch can't fire here. Proceed without the var rather than + // refuse to launch: a misconfigured environment is the user's problem to + // surface, not a reason to break plugins that don't read the var. The + // failure is logged at debug rather than printed to stderr — printing + // would noise every plugin invocation in a degenerate env. + parentEnv := os.Environ() + if dataDir, err := PluginDataDir(pluginName); err == nil { + extras = append(extras, pluginEnvPluginData+"="+dataDir) + } else { + // Strip any inherited value so the plugin doesn't silently see a + // value we never sanctioned. Without this strip, a user with + // ENTIRE_PLUGIN_DATA_DIR pre-set in their shell would have that + // value pass through (ENTIRE_* is in the pluginEnv allowlist + // prefix), even though resolution here failed. + parentEnv = removeEnvKey(parentEnv, pluginEnvPluginData) + logging.Debug(ctx, "ENTIRE_PLUGIN_DATA_DIR unset for plugin", + slog.String("plugin", pluginName), + slog.String("error", err.Error())) + } + cmd.Env = pluginEnv(parentEnv, extras...) if err := cmd.Run(); err != nil { var exitErr *exec.ExitError diff --git a/cmd/entire/cli/plugin_group.go b/cmd/entire/cli/plugin_group.go new file mode 100644 index 000000000..64332e012 --- /dev/null +++ b/cmd/entire/cli/plugin_group.go @@ -0,0 +1,160 @@ +package cli + +import ( + "fmt" + "io" + + "github.com/spf13/cobra" +) + +// newPluginGroupCmd builds `entire plugin` and its subcommands. The kubectl +// dispatcher in plugin.go is the runtime mechanism — these commands manage a +// per-user managed directory that the dispatcher discovers because main.go +// prepends it to PATH at startup. +// +// Currently only local symlink installs are supported. GitHub-release +// asset and git-clone install paths are deferred until there's demand. +func newPluginGroupCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "plugin", + Short: "Manage Entire plugins (install, list, remove)", + Long: `Manage Entire plugins. + +Plugins are external executables named 'entire-'. The CLI discovers +plugins on $PATH and from a per-user managed directory which is +auto-prepended to PATH at startup. The managed directory is, in order of +precedence: + + $ENTIRE_PLUGIN_DIR/bin (override) + $XDG_DATA_HOME/entire/plugins/bin (Linux/macOS, when set) + ~/.local/share/entire/plugins/bin (Linux/macOS default) + %LOCALAPPDATA%\entire\plugins\bin (Windows, when set) + ~\AppData\Local\entire\plugins\bin (Windows fallback when LOCALAPPDATA is unset) + +Commands: + install Install a plugin by linking or copying an existing executable + list List plugins installed in the managed directory + remove Remove a plugin from the managed directory + +Examples: + entire plugin install ./dist/entire-pgr + entire plugin list + entire plugin remove pgr`, + } + + cmd.AddCommand(newPluginInstallCmd()) + cmd.AddCommand(newPluginListCmd()) + cmd.AddCommand(newPluginRemoveCmd()) + return cmd +} + +func newPluginInstallCmd() *cobra.Command { + var force bool + cmd := &cobra.Command{ + Use: "install ", + Short: "Link or copy a plugin executable into the managed directory", + Long: `Link or copy a plugin executable into the managed directory. + +The source must be a file whose basename starts with 'entire-' (the +dispatcher only resolves names of that shape). On Unix the file must be +executable. + +The CLI prefers a symlink so rebuilds of the source are reflected +immediately, and falls back to a hardlink, then a copy, if symlinks aren't +available (notably Windows without Developer Mode). + +After install, 'entire ' invokes the plugin via the kubectl-style +dispatcher — the managed directory is auto-prepended to $PATH. + +Examples: + entire plugin install ./dist/entire-pgr + entire plugin install /usr/local/bin/entire-pgr --force`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + p, err := InstallPluginFromPath(InstallPluginOptions{ + SourcePath: args[0], + Force: force, + }) + if err != nil { + return fmt.Errorf("install plugin: %w", err) + } + fmt.Fprintf(cmd.OutOrStdout(), "Installed plugin %q → %s\n", p.Name, p.Path) + warnIfShadowsBuiltin(cmd, p.Name) + return nil + }, + } + cmd.Flags().BoolVar(&force, "force", false, "Replace an existing entry with the same name") + return cmd +} + +// warnIfShadowsBuiltin prints a one-line note to stderr when the just-installed +// plugin name matches a built-in command. The dispatcher's resolvePlugin gates +// dispatch on rootCmd.Find, so the built-in always wins at runtime — without +// this hint, a user who installed a shadowed plugin would silently get the +// built-in and have no idea their install was inert. We mirror the dispatcher's +// help/completion priming so names like "help" surface the warning too. +func warnIfShadowsBuiltin(cmd *cobra.Command, name string) { + root := cmd.Root() + if root == nil { + return + } + root.InitDefaultHelpCmd() + root.InitDefaultCompletionCmd(name) + if c, _, err := root.Find([]string{name}); err == nil && c != root { + fmt.Fprintf(cmd.ErrOrStderr(), "Note: %q shadows the built-in command; the built-in will take precedence at runtime.\n", name) + } +} + +func newPluginListCmd() *cobra.Command { + return &cobra.Command{ + Use: "list", + Short: "List plugins installed in the managed directory", + RunE: func(cmd *cobra.Command, _ []string) error { + return runPluginList(cmd.OutOrStdout()) + }, + } +} + +func runPluginList(w io.Writer) error { + plugins, err := ListInstalledPlugins() + if err != nil { + return fmt.Errorf("list plugins: %w", err) + } + dir, err := PluginBinDir() + if err != nil { + return fmt.Errorf("plugin bin dir: %w", err) + } + if len(plugins) == 0 { + fmt.Fprintf(w, "No plugins installed in %s.\n", dir) + fmt.Fprintln(w, "Install one with 'entire plugin install ', or drop an entire- binary anywhere on $PATH.") + return nil + } + fmt.Fprintf(w, "Managed plugin directory: %s\n\n", dir) + for _, p := range plugins { + if p.Symlink { + fmt.Fprintf(w, " %-20s → %s\n", p.Name, p.LinkTarget) + } else { + fmt.Fprintf(w, " %-20s %s\n", p.Name, p.Path) + } + } + return nil +} + +func newPluginRemoveCmd() *cobra.Command { + return &cobra.Command{ + Use: "remove ", + Short: "Remove a plugin from the managed directory", + Long: `Remove a plugin from the managed directory. + +Only entries in the managed directory are affected. Plugins installed by +dropping a binary elsewhere on $PATH are unmanaged — remove those by hand.`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if err := RemoveInstalledPlugin(args[0]); err != nil { + return fmt.Errorf("remove plugin: %w", err) + } + fmt.Fprintf(cmd.OutOrStdout(), "Removed plugin %q\n", args[0]) + return nil + }, + } +} diff --git a/cmd/entire/cli/plugin_group_test.go b/cmd/entire/cli/plugin_group_test.go new file mode 100644 index 000000000..c0c7173a3 --- /dev/null +++ b/cmd/entire/cli/plugin_group_test.go @@ -0,0 +1,54 @@ +package cli + +import ( + "bytes" + "strings" + "testing" + + "github.com/spf13/cobra" +) + +// TestWarnIfShadowsBuiltin verifies the install-time hint fires for names that +// match a built-in command (the runtime resolver always picks the built-in +// over a managed plugin) and stays silent for plugin-only names. +func TestWarnIfShadowsBuiltin(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + install string + wantWarn bool + wantSubst string + }{ + {name: "shadows built-in", install: "status", wantWarn: true, wantSubst: "shadows the built-in"}, + {name: "plugin-only name", install: "pgr", wantWarn: false}, + {name: "shadows help (cobra-injected)", install: "help", wantWarn: true, wantSubst: "shadows the built-in"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + root := &cobra.Command{Use: "entire"} + root.AddCommand(&cobra.Command{Use: "status"}) + plugin := &cobra.Command{Use: "plugin"} + install := &cobra.Command{Use: "install"} + plugin.AddCommand(install) + root.AddCommand(plugin) + + var stderr bytes.Buffer + install.SetErr(&stderr) + install.SetOut(&bytes.Buffer{}) + + warnIfShadowsBuiltin(install, tc.install) + + got := stderr.String() + if tc.wantWarn { + if !strings.Contains(got, tc.wantSubst) { + t.Errorf("expected warning containing %q, got %q", tc.wantSubst, got) + } + } else if got != "" { + t.Errorf("expected no warning, got %q", got) + } + }) + } +} diff --git a/cmd/entire/cli/plugin_store.go b/cmd/entire/cli/plugin_store.go new file mode 100644 index 000000000..fb900370b --- /dev/null +++ b/cmd/entire/cli/plugin_store.go @@ -0,0 +1,552 @@ +package cli + +import ( + "context" + "crypto/rand" + "encoding/hex" + "errors" + "fmt" + "io" + "log/slog" + "os" + "path/filepath" + "runtime" + "sort" + "strings" + + "github.com/entireio/cli/cmd/entire/cli/agent/external" + "github.com/entireio/cli/cmd/entire/cli/logging" +) + +// Managed plugin storage. The kubectl-style dispatcher in plugin.go resolves +// `entire-` binaries from $PATH, period. To let `entire plugin install` +// be additive rather than a parallel mechanism, this file provides: +// +// 1. PluginBinDir() — a per-user managed dir that main.go prepends to PATH +// before the dispatcher runs. Anything dropped here (or symlinked here) +// becomes invocable as `entire ` without the user fiddling with PATH. +// +// 2. PluginDataDir(name) — a per-plugin durable storage dir, passed to plugins +// as ENTIRE_PLUGIN_DATA_DIR. Independent of where the binary itself lives +// so plugins installed via PATH and via the managed dir get the same +// contract. +// +// Honors ENTIRE_PLUGIN_DIR as a parent-dir override; falls back to +// XDG_DATA_HOME, then a platform default. + +const ( + pluginEnvPluginDir = "ENTIRE_PLUGIN_DIR" + pluginManagedBinSubdir = "bin" + pluginManagedDataSubdir = "data" + pluginEnvPluginData = "ENTIRE_PLUGIN_DATA_DIR" + // Path segments for the managed plugin tree. Kept as separate + // segments (rather than "entire/plugins") so filepath.Join produces + // platform-native separators on Windows. + pluginManagedTopDir = "entire" + pluginManagedSubDir = "plugins" +) + +// pluginParentDir returns the per-user directory that holds the managed +// plugin storage. Resolution, in order: +// +// 1. ENTIRE_PLUGIN_DIR (cross-platform override). +// 2. On Windows: LOCALAPPDATA if set, else ~\AppData\Local\entire\plugins. +// 3. On Unix: XDG_DATA_HOME if set, else ~/.local/share/entire/plugins. +// +// XDG_DATA_HOME is deliberately ignored on Windows even when set (e.g. in +// MSYS/Cygwin) — Windows users expect Windows conventions, and routing +// through XDG would produce a surprising location. +// +// os.UserHomeDir is called only when no env-var branch resolves, so a +// degenerate environment with $LOCALAPPDATA or $XDG_DATA_HOME but no home +// still returns a usable path. +func pluginParentDir() (string, error) { + // ENTIRE_PLUGIN_DIR must be absolute. A relative value would resolve + // against the user's CWD at startup — typically inside their repo — + // which is the wrong place for managed plugin storage. Reject loudly + // rather than silently falling through to the platform default, since + // a misconfigured override is almost certainly a user error worth + // surfacing. + if v := os.Getenv(pluginEnvPluginDir); v != "" { + if !filepath.IsAbs(v) { + return "", fmt.Errorf("%s must be an absolute path, got %q", pluginEnvPluginDir, v) + } + return v, nil + } + if runtime.GOOS == windowsGOOS { + if appData := os.Getenv("LOCALAPPDATA"); appData != "" { + return filepath.Join(appData, pluginManagedTopDir, pluginManagedSubDir), nil + } + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("resolve home dir: %w", err) + } + return filepath.Join(home, "AppData", "Local", pluginManagedTopDir, pluginManagedSubDir), nil + } + if v := os.Getenv("XDG_DATA_HOME"); v != "" { + return filepath.Join(v, pluginManagedTopDir, pluginManagedSubDir), nil + } + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("resolve home dir: %w", err) + } + return filepath.Join(home, ".local", "share", pluginManagedTopDir, pluginManagedSubDir), nil +} + +// PluginBinDir returns the managed install directory. Binaries (or symlinks) +// placed here are auto-discovered by the kubectl-style dispatcher because +// main.go prepends this dir to PATH before MaybeRunPlugin runs. +func PluginBinDir() (string, error) { + parent, err := pluginParentDir() + if err != nil { + return "", err + } + return filepath.Join(parent, pluginManagedBinSubdir), nil +} + +// PluginDataDir returns the per-plugin data directory for the given bare name +// (e.g. "pgr" for `entire-pgr`). The returned path is not created — that's +// the plugin's responsibility on first use. +// +// Returns an error for names the dispatcher would never invoke (empty, +// flag-shaped, agent-protocol-reserved, "."/".." path-traversal, slashes). +// This guarantees ENTIRE_PLUGIN_DATA_DIR always points inside the managed +// data subtree. +func PluginDataDir(name string) (string, error) { + if err := validatePluginName(name); err != nil { + return "", err + } + parent, err := pluginParentDir() + if err != nil { + return "", err + } + return filepath.Join(parent, pluginManagedDataSubdir, name), nil +} + +// validatePluginName mirrors the dispatcher's isPluginCandidate rules and +// returns a descriptive error for invalid names. Used by every entry point +// that takes a plugin name from outside the dispatcher (data dir resolution, +// managed-dir install). +func validatePluginName(name string) error { + if name == "" { + return errors.New("plugin name is empty") + } + if strings.HasPrefix(name, "-") { + return fmt.Errorf("plugin name %q must not start with '-'", name) + } + if strings.HasPrefix(name, "agent-") { + return fmt.Errorf("plugin name %q is reserved for the external agent protocol", name) + } + if strings.ContainsAny(name, `/\`) { + return fmt.Errorf("plugin name %q must not contain path separators", name) + } + if name == "." || name == ".." { + return fmt.Errorf("plugin name %q is not a valid identifier", name) + } + return nil +} + +// EnsurePluginBinDir creates the managed install dir if it doesn't exist. +func EnsurePluginBinDir() (string, error) { + dir, err := PluginBinDir() + if err != nil { + return "", err + } + if err := os.MkdirAll(dir, 0o750); err != nil { + return "", fmt.Errorf("create plugin bin dir: %w", err) + } + return dir, nil +} + +// PrependPluginBinDirToPATH prepends the managed bin dir to the process's +// PATH so the kubectl dispatcher discovers managed-installed plugins. +// Idempotent against an already-prepended dir. +// +// Returns a restore closure the caller invokes to revert PATH to its +// previous value. Restoring matters when no plugin runs: built-in commands +// and the subprocesses they spawn (git, hooks, less, …) should see the +// user's original PATH, not one with the managed plugin dir prepended. +// When a plugin *is* dispatched, callers can simply skip the restore — the +// process exits anyway, and the plugin child intentionally inherits the +// prepended PATH so it can spawn sibling managed plugins. +// +// Errors and no-op cases (already-prepended, lookup failure) return a +// no-op restore so callers always have a safe func to call. Failures are +// emitted at debug level — the surface symptom ("my managed plugin +// doesn't run") is otherwise silent and hard to diagnose; a debug log +// surfaces the cause for users who flip log_level=DEBUG. +func PrependPluginBinDirToPATH(ctx context.Context) func() { + dir, err := PluginBinDir() + if err != nil || dir == "" { + if err != nil { + logging.Debug(ctx, "skip prepend managed plugin bin dir to PATH: resolve failed", slog.String("error", err.Error())) + } + return func() {} + } + prev := os.Getenv("PATH") + sep := string(os.PathListSeparator) + if prev == "" { + if err := os.Setenv("PATH", dir); err != nil { + logging.Debug(ctx, "skip prepend managed plugin bin dir to PATH: setenv failed", slog.String("error", err.Error())) + return func() {} + } + return func() { _ = os.Setenv("PATH", "") } + } + // Idempotent: if the first entry already matches, leave PATH alone. + // Windows PATH lookups are case-insensitive (`C:\Foo` and `c:\foo` + // resolve identically), so a case-different first entry is the same + // dir and we should not double-prepend. + first := prev + if i := strings.Index(prev, sep); i >= 0 { + first = prev[:i] + } + if pathEntriesEqual(first, dir) { + return func() {} + } + if err := os.Setenv("PATH", dir+sep+prev); err != nil { + logging.Debug(ctx, "skip prepend managed plugin bin dir to PATH: setenv failed", slog.String("error", err.Error())) + return func() {} + } + return func() { _ = os.Setenv("PATH", prev) } +} + +// pathEntriesEqual reports whether two PATH entries refer to the same dir. +// filepath.Clean normalizes trailing separators and (on Windows) slash +// orientation; case folding handles Windows' case-insensitive lookup. +func pathEntriesEqual(a, b string) bool { + a = filepath.Clean(a) + b = filepath.Clean(b) + if runtime.GOOS == windowsGOOS { + return strings.EqualFold(a, b) + } + return a == b +} + +// InstalledPlugin describes a single entry in the managed bin dir. +type InstalledPlugin struct { + // Name is the bare plugin name (without the `entire-` prefix and any + // platform-specific extension). + Name string + // Path is the absolute path inside the managed bin dir. + Path string + // Symlink is true when Path is a symlink to a source location elsewhere + // (the typical local-dev install). LinkTarget is populated in that case. + Symlink bool + LinkTarget string +} + +// ListInstalledPlugins enumerates entries in the managed bin dir whose name +// starts with `entire-`. Sorted by bare name. A missing dir returns no error +// and an empty slice. +func ListInstalledPlugins() ([]*InstalledPlugin, error) { + dir, err := PluginBinDir() + if err != nil { + return nil, err + } + entries, err := os.ReadDir(dir) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return nil, nil + } + return nil, fmt.Errorf("read plugin bin dir: %w", err) + } + + var out []*InstalledPlugin + for _, e := range entries { + full := e.Name() + if !strings.HasPrefix(full, pluginBinaryPrefix) { + continue + } + bare := bareNameFromBinaryName(full) + if bare == "" { + continue + } + path := filepath.Join(dir, full) + info, err := os.Lstat(path) + if err != nil { + continue + } + ip := &InstalledPlugin{Name: bare, Path: path} + if info.Mode()&os.ModeSymlink != 0 { + ip.Symlink = true + if target, err := os.Readlink(path); err == nil { + ip.LinkTarget = target + } + } + out = append(out, ip) + } + sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) + return out, nil +} + +// FindInstalledPlugin returns the entry for the given bare name, or nil if +// it isn't installed in the managed dir. +func FindInstalledPlugin(name string) (*InstalledPlugin, error) { + all, err := ListInstalledPlugins() + if err != nil { + return nil, err + } + for _, p := range all { + if p.Name == name { + return p, nil + } + } + return nil, nil //nolint:nilnil // not-installed signal +} + +// InstallPluginOptions configures InstallPluginFromPath. +type InstallPluginOptions struct { + // SourcePath is the absolute (or working-dir-relative) path to the plugin + // executable. Its basename — minus any platform extension — must match + // `entire-` so the dispatcher can resolve it. + SourcePath string + // Force replaces an already-installed plugin with the same name. + Force bool +} + +// InstallPluginFromPath symlinks SourcePath into the managed bin dir. The +// caller is responsible for built-in conflict checks (resolvePlugin already +// gates dispatch on rootCmd.Find — installing a name that shadows a built-in +// is allowed but the built-in still wins at runtime). +// +// Refuses names the dispatcher will never invoke (agent-protocol prefix, +// flag-shaped, "."/"..", slashes), and refuses self-install when the source +// is the same file as the would-be managed entry. The replace step is +// atomic: a new symlink is created at .tmp and renamed onto , +// so a failed --force never leaves the previous install missing. +func InstallPluginFromPath(opts InstallPluginOptions) (*InstalledPlugin, error) { + src, err := filepath.Abs(opts.SourcePath) + if err != nil { + return nil, fmt.Errorf("resolve source path: %w", err) + } + srcInfo, err := os.Stat(src) + if err != nil { + return nil, fmt.Errorf("stat source: %w", err) + } + if srcInfo.IsDir() { + return nil, fmt.Errorf("source must be a file, got directory: %s", src) + } + base := filepath.Base(src) + bare := bareNameFromBinaryName(base) + if bare == "" { + return nil, fmt.Errorf("source basename %q must start with %q and have a runnable name", base, pluginBinaryPrefix) + } + if err := validatePluginName(bare); err != nil { + return nil, fmt.Errorf("derived plugin name from %q is not dispatchable: %w", base, err) + } + if runtime.GOOS != windowsGOOS && srcInfo.Mode()&0o111 == 0 { + return nil, fmt.Errorf("source %s is not executable (chmod +x)", src) + } + + binDir, err := EnsurePluginBinDir() + if err != nil { + return nil, err + } + dest := filepath.Join(binDir, base) + + // Reject self-install: the source path already equals the managed + // destination path. Without this guard, `--force` would atomically + // rename a tmp symlink onto its own target file and the underlying + // binary would be unlinked. + // + // We deliberately do NOT use os.SameFile here. The legitimate + // repeat-install case has dest as a symlink we created on a prior + // install; SameFile would resolve dest through the symlink to src + // and falsely trip. Path equality is the precise risky case. + if filepath.Clean(src) == filepath.Clean(dest) { + return nil, fmt.Errorf("source %s is already the managed entry; nothing to install", src) + } + + // Conflict check on the bare name (not the exact filename). On Windows, + // entire-foo.exe / .bat / .cmd all map to bare name "foo"; checking only + // the destination filename would let a second install of a different + // extension silently coexist with the first, with PATHEXT ordering then + // deciding which one runs. List all variants and require --force when + // any exist. + conflicts, err := installedVariantsByBareName(bare) + if err != nil { + return nil, err + } + if len(conflicts) > 0 && !opts.Force { + return nil, fmt.Errorf("plugin %q already installed at %s; use --force to replace", bare, conflicts[0].Path) + } + // With --force, remove every variant other than the one we're about to + // atomically overwrite. The same-extension case is handled by the + // rename below, which atomically replaces dest. + for _, c := range conflicts { + if filepath.Clean(c.Path) == filepath.Clean(dest) { + continue + } + if err := os.Remove(c.Path); err != nil && !errors.Is(err, os.ErrNotExist) { + return nil, fmt.Errorf("remove existing variant %s: %w", c.Path, err) + } + } + + // Atomic replace via tmp + rename. Rename is atomic on POSIX and + // replaces an existing target on Windows (Go's os.Rename uses + // MoveFileEx with MOVEFILE_REPLACE_EXISTING). If linking or rename + // fails, the previously installed plugin (if any) is unaffected. + // + // The tmp path uses a random suffix and a `.install-` prefix that does + // NOT match `entire-`. This protects against two distinct hazards: + // 1. A user can have a legitimate plugin named "foo.tmp" (file + // "entire-foo.tmp"), which a naive `dest + ".tmp"` would clobber. + // 2. ListInstalledPlugins filters by `entire-` prefix, so a tmp that + // starts with `.install-` will not appear in `entire plugin list` + // while the install is in progress. + tmpDest, err := makeInstallTmpPath(binDir) + if err != nil { + return nil, err + } + if err := materializeManagedEntry(src, tmpDest, srcInfo); err != nil { + return nil, fmt.Errorf("install plugin: %w", err) + } + if err := os.Rename(tmpDest, dest); err != nil { + _ = os.Remove(tmpDest) // best-effort cleanup; previous install is intact + return nil, fmt.Errorf("install plugin: %w", err) + } + return FindInstalledPlugin(bare) +} + +// installedVariantsByBareName returns every managed entry whose bare name +// matches name. On Unix this is at most one entry; on Windows multiple +// extensions can map to the same bare name. +func installedVariantsByBareName(name string) ([]*InstalledPlugin, error) { + all, err := ListInstalledPlugins() + if err != nil { + return nil, err + } + var out []*InstalledPlugin + for _, p := range all { + if p.Name == name { + out = append(out, p) + } + } + return out, nil +} + +// makeInstallTmpPath returns a unique scratch path inside binDir for the +// in-progress install. The `.install-` prefix is deliberately distinct from +// pluginBinaryPrefix so ListInstalledPlugins ignores it; a 16-char hex +// suffix from crypto/rand makes collisions vanishingly unlikely and keeps +// concurrent installs safe from each other. +func makeInstallTmpPath(binDir string) (string, error) { + var b [8]byte + if _, err := rand.Read(b[:]); err != nil { + return "", fmt.Errorf("generate install tmp suffix: %w", err) + } + return filepath.Join(binDir, ".install-"+hex.EncodeToString(b[:])), nil +} + +// materializeManagedEntry creates dest as a reference to src, falling back +// through symlink → hardlink → copy in that order. +// +// Symlink-first preserves the dev-loop property that rebuilding the source +// is immediately reflected in the managed entry. The fallbacks exist for +// Windows: os.Symlink there requires Developer Mode or admin, and silently +// breaks `entire plugin install` for typical users without either. Mirrors +// the pattern in setup_test.go's copyExecutable. +// +// On a successful copy the file mode of the source is preserved so the +// executable bit survives. +func materializeManagedEntry(src, dest string, srcInfo os.FileInfo) error { + if err := os.Symlink(src, dest); err == nil { + return nil + } + if err := os.Link(src, dest); err == nil { + return nil + } + return copyFileStreaming(src, dest, srcInfo) +} + +// copyFileStreaming copies src to dest in fixed-size buffers, preserving the +// source's executable mode. Plugin binaries can be tens of megabytes; using +// io.Copy avoids the heap spike of reading the whole file into memory. +func copyFileStreaming(src, dest string, srcInfo os.FileInfo) error { + mode := srcInfo.Mode().Perm() + if mode == 0 { + mode = 0o755 + } + in, err := os.Open(src) //nolint:gosec // src is the user-provided plugin executable; reading it is the point + if err != nil { + return fmt.Errorf("open source for copy fallback: %w", err) + } + defer in.Close() + + // G304: dest is always inside the managed bin dir. The basename comes + // from a validated plugin name (validatePluginName ran upstream), and + // the parent dir comes from EnsurePluginBinDir. + out, err := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, mode) //nolint:gosec // dest is constrained to the managed bin dir + if err != nil { + return fmt.Errorf("open destination for copy fallback: %w", err) + } + if _, err := io.Copy(out, in); err != nil { + _ = out.Close() + _ = os.Remove(dest) + return fmt.Errorf("copy fallback: %w", err) + } + if err := out.Close(); err != nil { + _ = os.Remove(dest) + return fmt.Errorf("close destination after copy fallback: %w", err) + } + return nil +} + +// RemoveInstalledPlugin removes every managed-dir entry whose bare name +// matches name. Symlinks are unlinked without touching the source file. +// +// Iterating all variants matters on Windows, where entire-foo.exe, +// entire-foo.bat, and entire-foo.cmd all map to bare name "foo" and could +// otherwise leave a runnable variant behind after `entire plugin remove foo`. +// On Unix the loop typically runs once. +func RemoveInstalledPlugin(name string) error { + variants, err := installedVariantsByBareName(name) + if err != nil { + return err + } + if len(variants) == 0 { + return fmt.Errorf("plugin %q is not installed in the managed directory", name) + } + for _, p := range variants { + if err := os.Remove(p.Path); err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("remove plugin entry %s: %w", p.Path, err) + } + } + return nil +} + +// bareNameFromBinaryName turns a plugin executable's basename into the bare +// name the dispatcher uses (e.g. "entire-pgr" → "pgr"). Returns "" if the +// input doesn't match the expected shape. +// +// Extension stripping is platform-conditional: +// +// - On Windows, .exe/.bat/.cmd are the natural extensions for executables +// and exec.LookPath resolves them via PATHEXT, so we strip them so the +// bare name a user types ("pgr") matches both the managed-list display +// and the dispatcher's lookup. +// +// - On Unix, exec.LookPath matches the exact filename. If we stripped here, +// "entire-pgr.exe" would be listed as "pgr" and the user would type +// "entire pgr", but the dispatcher's exec.LookPath("entire-pgr") would +// not find "entire-pgr.exe". Leaving the dot in place keeps the listed +// name aligned with the only invocation that actually resolves +// ("entire pgr.exe"), avoiding silent shadowing surprises. +func bareNameFromBinaryName(base string) string { + if !strings.HasPrefix(base, pluginBinaryPrefix) { + return "" + } + cleaned := base + if runtime.GOOS == windowsGOOS { + // Reuse the canonical Windows-executable-extension list from + // agent/external rather than maintaining a parallel copy. plugin.go + // already depends on this package for isAgentProtocolBinary, so + // there's no new layering cost. + cleaned = external.StripExeExt(base) + } + bare := strings.TrimPrefix(cleaned, pluginBinaryPrefix) + if bare == "" { + return "" + } + return bare +} diff --git a/cmd/entire/cli/plugin_store_test.go b/cmd/entire/cli/plugin_store_test.go new file mode 100644 index 000000000..48347ae1d --- /dev/null +++ b/cmd/entire/cli/plugin_store_test.go @@ -0,0 +1,533 @@ +package cli + +import ( + "context" + "errors" + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// testPluginName is the bare plugin name used across managed-store tests. +const testPluginName = "pgr" + +// withPluginDir points $ENTIRE_PLUGIN_DIR at a fresh temp dir so the managed +// helpers operate in isolation. Mutates process state, so the calling test +// must not be t.Parallel. +func withPluginDir(t *testing.T) string { + t.Helper() + dir := t.TempDir() + t.Setenv(pluginEnvPluginDir, dir) + return dir +} + +func TestPluginParentDir_HonorsOverride(t *testing.T) { //nolint:paralleltest // mutates env + dir := withPluginDir(t) + got, err := pluginParentDir() + if err != nil { + t.Fatalf("pluginParentDir: %v", err) + } + if got != dir { + t.Errorf("pluginParentDir = %q, want %q", got, dir) + } +} + +func TestPluginParentDir_RejectsRelativeOverride(t *testing.T) { //nolint:paralleltest // mutates env + // A relative ENTIRE_PLUGIN_DIR would resolve against startup CWD — + // typically inside the user's repo. Reject rather than silently + // fall through to the platform default. + t.Setenv(pluginEnvPluginDir, "plugins-relative") + if _, err := pluginParentDir(); err == nil { + t.Errorf("pluginParentDir with relative override = nil error; want error") + } + t.Setenv(pluginEnvPluginDir, ".") + if _, err := pluginParentDir(); err == nil { + t.Errorf("pluginParentDir with '.' override = nil error; want error") + } +} + +func TestPluginParentDir_WindowsIgnoresXDG(t *testing.T) { //nolint:paralleltest // mutates env + if runtime.GOOS != windowsGOOS { + t.Skip("Windows-only behavior") + } + // ENTIRE_PLUGIN_DIR not set; XDG_DATA_HOME set. Result must NOT be + // rooted at the XDG path — Windows users expect Windows conventions. + xdg := t.TempDir() + t.Setenv(pluginEnvPluginDir, "") + t.Setenv("XDG_DATA_HOME", xdg) + got, err := pluginParentDir() + if err != nil { + t.Fatalf("pluginParentDir: %v", err) + } + xdgRoot := filepath.Clean(filepath.Join(xdg, pluginManagedTopDir, pluginManagedSubDir)) + if strings.HasPrefix(filepath.Clean(got), xdgRoot) { + t.Errorf("pluginParentDir = %q is rooted at XDG path %q; XDG_DATA_HOME must be ignored on Windows", got, xdgRoot) + } +} + +func TestPluginParentDir_UnixHonorsXDG(t *testing.T) { //nolint:paralleltest // mutates env + if runtime.GOOS == windowsGOOS { + t.Skip("Unix-only behavior") + } + t.Setenv(pluginEnvPluginDir, "") + xdg := t.TempDir() + t.Setenv("XDG_DATA_HOME", xdg) + got, err := pluginParentDir() + if err != nil { + t.Fatalf("pluginParentDir: %v", err) + } + want := filepath.Join(xdg, "entire", "plugins") + if got != want { + t.Errorf("pluginParentDir = %q, want %q", got, want) + } +} + +func TestPluginBinDir_AndDataDir(t *testing.T) { //nolint:paralleltest // mutates env + root := withPluginDir(t) + bin, err := PluginBinDir() + if err != nil { + t.Fatalf("PluginBinDir: %v", err) + } + if want := filepath.Join(root, "bin"); bin != want { + t.Errorf("PluginBinDir = %q, want %q", bin, want) + } + data, err := PluginDataDir("pgr") + if err != nil { + t.Fatalf("PluginDataDir: %v", err) + } + if want := filepath.Join(root, "data", "pgr"); data != want { + t.Errorf("PluginDataDir(pgr) = %q, want %q", data, want) + } +} + +func TestPrependPluginBinDirToPATH(t *testing.T) { //nolint:paralleltest // mutates env + root := withPluginDir(t) + original := "/usr/bin:/bin" + t.Setenv("PATH", original) + restore := PrependPluginBinDirToPATH(context.Background()) + bin := filepath.Join(root, "bin") + got := os.Getenv("PATH") + if !strings.HasPrefix(got, bin+string(os.PathListSeparator)) { + t.Errorf("PATH does not start with managed bin dir: %q", got) + } + // Idempotent: a second call returns a no-op restore and does not + // double-prepend. + noop := PrependPluginBinDirToPATH(context.Background()) + if strings.Count(os.Getenv("PATH"), bin) != 1 { + t.Errorf("PATH contains managed bin dir %d times after second prepend; want 1: %q", strings.Count(os.Getenv("PATH"), bin), os.Getenv("PATH")) + } + noop() // safe no-op + + // Restoring must return PATH to exactly what it was before the + // prepend, so built-in execution doesn't inherit the managed dir. + restore() + if got := os.Getenv("PATH"); got != original { + t.Errorf("PATH after restore = %q; want %q", got, original) + } +} + +func TestInstallPluginFromPath_SymlinksAndLists(t *testing.T) { //nolint:paralleltest // mutates env + if runtime.GOOS == windowsGOOS { + t.Skip("symlink path is Unix-only here") + } + withPluginDir(t) + src := filepath.Join(t.TempDir(), "entire-pgr") + if err := os.WriteFile(src, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatalf("write src: %v", err) + } + + p, err := InstallPluginFromPath(InstallPluginOptions{SourcePath: src}) + if err != nil { + t.Fatalf("InstallPluginFromPath: %v", err) + } + if p.Name != testPluginName { + t.Errorf("Name = %q, want %s", p.Name, testPluginName) + } + if !p.Symlink { + t.Errorf("expected Symlink=true; got %+v", p) + } + + plugins, err := ListInstalledPlugins() + if err != nil { + t.Fatalf("ListInstalledPlugins: %v", err) + } + if len(plugins) != 1 || plugins[0].Name != testPluginName { + t.Errorf("ListInstalledPlugins = %+v; want one entry named %s", plugins, testPluginName) + } + + // Re-install without --force fails. + if _, err := InstallPluginFromPath(InstallPluginOptions{SourcePath: src}); err == nil { + t.Errorf("expected error on re-install without --force") + } + // With --force succeeds. + if _, err := InstallPluginFromPath(InstallPluginOptions{SourcePath: src, Force: true}); err != nil { + t.Errorf("InstallPluginFromPath --force: %v", err) + } + + // Remove unlinks the managed entry without disturbing the source. + if err := RemoveInstalledPlugin(testPluginName); err != nil { + t.Fatalf("RemoveInstalledPlugin: %v", err) + } + if _, err := os.Stat(src); err != nil { + t.Errorf("source %s was disturbed by RemoveInstalledPlugin: %v", src, err) + } + if err := RemoveInstalledPlugin(testPluginName); err == nil { + t.Errorf("expected error removing already-removed plugin") + } +} + +func TestInstallPluginFromPath_RejectsBadBasename(t *testing.T) { //nolint:paralleltest // mutates env + if runtime.GOOS == windowsGOOS { + t.Skip("Unix permissions checks") + } + withPluginDir(t) + src := filepath.Join(t.TempDir(), "not-prefixed") + if err := os.WriteFile(src, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatalf("write src: %v", err) + } + if _, err := InstallPluginFromPath(InstallPluginOptions{SourcePath: src}); err == nil { + t.Errorf("expected error for non-prefixed basename") + } +} + +func TestInstallPluginFromPath_RejectsNonExecutable(t *testing.T) { //nolint:paralleltest // mutates env + if runtime.GOOS == windowsGOOS { + t.Skip("Unix permissions checks") + } + withPluginDir(t) + src := filepath.Join(t.TempDir(), "entire-noexec") + if err := os.WriteFile(src, []byte("#!/bin/sh\nexit 0\n"), 0o644); err != nil { + t.Fatalf("write src: %v", err) + } + if _, err := InstallPluginFromPath(InstallPluginOptions{SourcePath: src}); err == nil { + t.Errorf("expected error for non-executable source") + } +} + +func TestBareNameFromBinaryName(t *testing.T) { + t.Parallel() + // Cases that hold on every platform. + common := map[string]string{ + "entire-pgr": "pgr", + "entire-": "", + "foo": "", + "": "", + } + for in, want := range common { + if got := bareNameFromBinaryName(in); got != want { + t.Errorf("bareNameFromBinaryName(%q) = %q; want %q", in, got, want) + } + } + // Platform-conditional: extensions are stripped only on Windows so a + // managed entry actually resolves at runtime via exec.LookPath. + if runtime.GOOS == windowsGOOS { + for in, want := range map[string]string{ + "entire-pgr.exe": "pgr", + "entire-foo.bat": "foo", + "entire-foo.cmd": "foo", + } { + if got := bareNameFromBinaryName(in); got != want { + t.Errorf("[windows] bareNameFromBinaryName(%q) = %q; want %q", in, got, want) + } + } + } else { + // On Unix, a .exe basename is *not* a valid bare name — installing + // it would yield a managed entry that LookPath would never match. + // We accept that bareNameFromBinaryName may return a non-empty + // string here (the dispatcher uses exact-match LookPath); the + // guarantee we test is that "entire-pgr.exe" doesn't collapse to + // "pgr" on Unix. + if got := bareNameFromBinaryName("entire-pgr.exe"); got == "pgr" { + t.Errorf("[unix] bareNameFromBinaryName(entire-pgr.exe) collapsed to %q; should not strip .exe on Unix", got) + } + } +} + +func TestValidatePluginName(t *testing.T) { + t.Parallel() + good := []string{"pgr", "foo-bar", "x", "v1"} + for _, n := range good { + if err := validatePluginName(n); err != nil { + t.Errorf("validatePluginName(%q) = %v; want nil", n, err) + } + } + bad := []string{"", ".", "..", "-foo", "agent-foo", "foo/bar", `foo\bar`} + for _, n := range bad { + if err := validatePluginName(n); err == nil { + t.Errorf("validatePluginName(%q) = nil; want error", n) + } + } +} + +func TestPluginDataDir_RejectsPathTraversal(t *testing.T) { //nolint:paralleltest // mutates env + withPluginDir(t) + for _, name := range []string{"", ".", "..", "agent-foo", "foo/bar"} { + if _, err := PluginDataDir(name); err == nil { + t.Errorf("PluginDataDir(%q) = nil error; want error", name) + } + } +} + +func TestInstallPluginFromPath_RejectsAgentReservedName(t *testing.T) { //nolint:paralleltest // mutates env + if runtime.GOOS == windowsGOOS { + t.Skip("Unix-only test") + } + withPluginDir(t) + src := filepath.Join(t.TempDir(), "entire-agent-foo") + if err := os.WriteFile(src, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatalf("write src: %v", err) + } + if _, err := InstallPluginFromPath(InstallPluginOptions{SourcePath: src}); err == nil { + t.Errorf("expected error: agent-* basename is reserved") + } +} + +func TestInstallPluginFromPath_RejectsSelfInstall(t *testing.T) { //nolint:paralleltest // mutates env + if runtime.GOOS == windowsGOOS { + t.Skip("Unix-only test") + } + root := withPluginDir(t) + binDir := filepath.Join(root, "bin") + if err := os.MkdirAll(binDir, 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + // Drop the source directly into the managed dir, then attempt to + // install it from that same path. Without the self-install guard, + // --force would Remove() this file before symlinking to a missing + // target, deleting the working install. + src := filepath.Join(binDir, "entire-foo") + if err := os.WriteFile(src, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatalf("write src: %v", err) + } + _, err := InstallPluginFromPath(InstallPluginOptions{SourcePath: src, Force: true}) + if err == nil { + t.Fatalf("expected self-install rejection; got nil") + } + if _, statErr := os.Stat(src); statErr != nil { + t.Errorf("self-install attempt deleted the source: %v", statErr) + } +} + +// TestMaterializeManagedEntry_HappyPath is a smoke test that the helper +// completes successfully on a normal write-capable destination. On Unix it +// exits through the symlink branch; the hardlink and copy fallbacks exist +// for Windows-without-Developer-Mode and aren't portably triggerable from an +// in-process test (forcing os.Symlink to fail without mocks would require a +// non-portable filesystem setup). The other tests in this file exercise +// InstallPluginFromPath end-to-end, which calls into here. +func TestMaterializeManagedEntry_HappyPath(t *testing.T) { + t.Parallel() + if runtime.GOOS == windowsGOOS { + t.Skip("test exercises Unix file modes") + } + srcDir := t.TempDir() + src := filepath.Join(srcDir, "src-bin") + body := []byte("#!/bin/sh\nexit 0\n") + if err := os.WriteFile(src, body, 0o755); err != nil { + t.Fatalf("write src: %v", err) + } + srcInfo, err := os.Stat(src) + if err != nil { + t.Fatalf("stat src: %v", err) + } + + dest := filepath.Join(t.TempDir(), "out") + if err := materializeManagedEntry(src, dest, srcInfo); err != nil { + t.Fatalf("materializeManagedEntry: %v", err) + } + got, err := os.ReadFile(dest) + if err != nil { + t.Fatalf("read result: %v", err) + } + if string(got) != string(body) { + t.Errorf("dest content mismatch: got %q want %q", got, body) + } +} + +func TestRemoveInstalledPlugin_RemovesAllVariants(t *testing.T) { //nolint:paralleltest // mutates env + // Simulate a corrupted state with two variants for the same bare name + // (the situation `entire plugin install` now prevents but legacy state + // or hand-edits could produce). RemoveInstalledPlugin must clean up + // every match, not just the first one FindInstalledPlugin returns. + if runtime.GOOS == windowsGOOS { + t.Skip("file-naming below is Unix-style") + } + root := withPluginDir(t) + binDir := filepath.Join(root, "bin") + if err := os.MkdirAll(binDir, 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + // On Unix, bareNameFromBinaryName preserves dots, so to produce two + // entries with identical bare names we need names that the helper + // folds together. We can't natively get that on Unix, so this test + // asserts the loop behavior by placing one entry and verifying the + // removal path iterates correctly even when only a single match + // exists. The Windows-specific multi-variant path is covered by the + // implementation reading installedVariantsByBareName. + body := []byte("#!/bin/sh\nexit 0\n") + if err := os.WriteFile(filepath.Join(binDir, "entire-foo"), body, 0o755); err != nil { + t.Fatalf("write entry: %v", err) + } + if err := RemoveInstalledPlugin("foo"); err != nil { + t.Fatalf("RemoveInstalledPlugin: %v", err) + } + if _, err := os.Stat(filepath.Join(binDir, "entire-foo")); !errors.Is(err, os.ErrNotExist) { + t.Errorf("entire-foo still present after remove: %v", err) + } +} + +func TestInstallPluginFromPath_TmpDoesNotClobberDottedPlugin(t *testing.T) { //nolint:paralleltest // mutates env + if runtime.GOOS == windowsGOOS { + t.Skip("symlink path is Unix-only here") + } + root := withPluginDir(t) + binDir := filepath.Join(root, "bin") + if err := os.MkdirAll(binDir, 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + // Pre-populate a plugin literally named "foo.tmp" — entirely valid: + // the dispatcher's name validator allows dots. The naive `dest+".tmp"` + // scheme would have clobbered this on the install below. + dotted := filepath.Join(binDir, "entire-foo.tmp") + dottedBody := []byte("#!/bin/sh\necho dotted\n") + if err := os.WriteFile(dotted, dottedBody, 0o755); err != nil { + t.Fatalf("write dotted: %v", err) + } + + // Now install entire-foo. Its temp path must not collide with + // entire-foo.tmp. + src := filepath.Join(t.TempDir(), "entire-foo") + if err := os.WriteFile(src, []byte("#!/bin/sh\necho foo\n"), 0o755); err != nil { + t.Fatalf("write src: %v", err) + } + if _, err := InstallPluginFromPath(InstallPluginOptions{SourcePath: src}); err != nil { + t.Fatalf("InstallPluginFromPath: %v", err) + } + + // The dotted plugin must still exist with its original content. + got, err := os.ReadFile(dotted) + if err != nil { + t.Fatalf("dotted plugin disappeared: %v", err) + } + if string(got) != string(dottedBody) { + t.Errorf("dotted plugin clobbered: got %q want %q", got, dottedBody) + } +} + +func TestInstallPluginFromPath_RequiresForceForSameBareName(t *testing.T) { //nolint:paralleltest // mutates env + // A second install of a different source file that resolves to the + // same bare name as a prior install must require --force. The + // cross-extension flavor of this conflict (entire-foo.exe vs + // entire-foo.bat sharing bare name "foo") is Windows-only and + // exercised by installedVariantsByBareName at the implementation + // level — the same-bare-name guard tested here is the user-visible + // surface on every platform. + if runtime.GOOS == windowsGOOS { + t.Skip("file-naming below is Unix-style; Windows tests would need a different harness") + } + root := withPluginDir(t) + binDir := filepath.Join(root, "bin") + if err := os.MkdirAll(binDir, 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + + // Install entire-foo first. + srcA := filepath.Join(t.TempDir(), "entire-foo") + if err := os.WriteFile(srcA, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatalf("write src A: %v", err) + } + if _, err := InstallPluginFromPath(InstallPluginOptions{SourcePath: srcA}); err != nil { + t.Fatalf("first install: %v", err) + } + + // A second install of the exact same source path is a self-install + // (path-equal) — that's tested elsewhere. Here we test that a + // different-source same-bare-name install requires --force. + srcB := filepath.Join(t.TempDir(), "entire-foo") + if err := os.WriteFile(srcB, []byte("#!/bin/sh\nexit 1\n"), 0o755); err != nil { + t.Fatalf("write src B: %v", err) + } + if _, err := InstallPluginFromPath(InstallPluginOptions{SourcePath: srcB}); err == nil { + t.Errorf("expected error: bare name %q already installed", "foo") + } + if _, err := InstallPluginFromPath(InstallPluginOptions{SourcePath: srcB, Force: true}); err != nil { + t.Errorf("force install: %v", err) + } +} + +func TestMakeInstallTmpPath_Unique(t *testing.T) { + t.Parallel() + dir := t.TempDir() + a, err := makeInstallTmpPath(dir) + if err != nil { + t.Fatalf("makeInstallTmpPath: %v", err) + } + b, err := makeInstallTmpPath(dir) + if err != nil { + t.Fatalf("makeInstallTmpPath: %v", err) + } + if a == b { + t.Errorf("two calls returned the same path: %q", a) + } + // Tmp prefix must not match the listing filter (which keys off + // "entire-"); the dot-prefix achieves that. + if !strings.HasPrefix(filepath.Base(a), ".install-") { + t.Errorf("tmp path %q does not start with .install-", a) + } +} + +func TestRemoveEnvKey(t *testing.T) { + t.Parallel() + in := []string{"FOO=1", "BAR=2", "FOO=3", "BAZ=4"} + got := removeEnvKey(in, "FOO") + want := []string{"BAR=2", "BAZ=4"} + if !equalStringSlices(got, want) { + t.Errorf("removeEnvKey = %v, want %v", got, want) + } +} + +func equalStringSlices(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +func TestInstallPluginFromPath_AtomicForceReplace(t *testing.T) { //nolint:paralleltest // mutates env + if runtime.GOOS == windowsGOOS { + t.Skip("symlink/atomic-rename behavior is Unix-focused here") + } + withPluginDir(t) + srcDir := t.TempDir() + src := filepath.Join(srcDir, "entire-foo") + if err := os.WriteFile(src, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatalf("write src: %v", err) + } + if _, err := InstallPluginFromPath(InstallPluginOptions{SourcePath: src}); err != nil { + t.Fatalf("first install: %v", err) + } + binDir, err := PluginBinDir() + if err != nil { + t.Fatalf("PluginBinDir: %v", err) + } + dest := filepath.Join(binDir, "entire-foo") + if _, err := os.Lstat(dest); err != nil { + t.Fatalf("first install missing: %v", err) + } + + // Force-install from the same source. The replace should succeed and + // the previous symlink remains valid throughout. + if _, err := InstallPluginFromPath(InstallPluginOptions{SourcePath: src, Force: true}); err != nil { + t.Errorf("force replace: %v", err) + } + if _, err := os.Lstat(dest); err != nil { + t.Errorf("dest missing after force replace: %v", err) + } +} diff --git a/cmd/entire/cli/plugin_test.go b/cmd/entire/cli/plugin_test.go index 237e62ffd..2074d805d 100644 --- a/cmd/entire/cli/plugin_test.go +++ b/cmd/entire/cli/plugin_test.go @@ -178,7 +178,7 @@ func TestRunPlugin_ExitCodePropagation(t *testing.T) { dir := t.TempDir() binPath := writePluginBinary(t, dir, "entire-exit42", filepath.Join(dir, "args.txt"), 42) - code := runPlugin(context.Background(), binPath, []string{"a", "b"}) + code := runPlugin(context.Background(), "exit42", binPath, []string{"a", "b"}) if code != 42 { t.Errorf("exit code: got %d, want 42", code) } diff --git a/cmd/entire/cli/root.go b/cmd/entire/cli/root.go index 3ee457649..60d7743f2 100644 --- a/cmd/entire/cli/root.go +++ b/cmd/entire/cli/root.go @@ -87,6 +87,7 @@ func NewRootCmd() *cobra.Command { cmd.AddCommand(newAuthCmd()) // 'auth' cmd.AddCommand(newDoctorCmd()) // 'doctor' (group: trace/logs/bundle) cmd.AddCommand(newLabsCmd()) // 'labs' (experimental workflow discovery) + cmd.AddCommand(newPluginGroupCmd()) // 'plugin' (managed install/list/remove) // Top-level lifecycle and standalone commands. cmd.AddCommand(cliReview.NewCommand(buildReviewDeps(newReviewAttachCmd()))) // hidden during maturation; runs configured review skills diff --git a/cmd/entire/main.go b/cmd/entire/main.go index b7e5d3c4f..334797f6d 100644 --- a/cmd/entire/main.go +++ b/cmd/entire/main.go @@ -33,10 +33,21 @@ func main() { // Create and execute root command rootCmd := cli.NewRootCmd() + // Make managed-installed plugins discoverable by the kubectl-style + // dispatcher: prepend the managed bin dir to PATH before resolution. + // Idempotent and silent on failure (managed installs simply won't be + // found this run; PATH-installed plugins still work). The closure + // restores PATH so built-in commands and their subprocesses don't + // inherit the prepended dir. When a plugin runs, we skip the restore + // — the os.Exit ends the process, and the plugin child intentionally + // inherits the prepended PATH so it can spawn sibling managed plugins. + restorePATH := cli.PrependPluginBinDirToPATH(ctx) + if handled, code := cli.MaybeRunPlugin(ctx, rootCmd, os.Args[1:]); handled { cancel() os.Exit(code) } + restorePATH() err := rootCmd.ExecuteContext(ctx) if err != nil { diff --git a/docs/architecture/external-commands.md b/docs/architecture/external-commands.md index 5d99096dc..0e4cc5903 100644 --- a/docs/architecture/external-commands.md +++ b/docs/architecture/external-commands.md @@ -17,6 +17,19 @@ Rules, in order: 3. **Path-traversal candidates are rejected.** Names containing `/` or `\` never resolve. 4. **Found-but-not-executable surfaces as a launch error.** If `entire-` exists on `$PATH` but lacks the executable bit, the resolver reports `Failed to run plugin entire-` with exit code 1, rather than falling through to Cobra's "unknown command" path. +### Managed install directory + +Users can drop binaries anywhere on `$PATH`, but a per-user managed directory is also automatically discovered: + +- **Default:** `$XDG_DATA_HOME/entire/plugins/bin` (Linux/macOS) or `%LOCALAPPDATA%\entire\plugins\bin` (Windows). +- **Override:** `$ENTIRE_PLUGIN_DIR/bin`. + +The CLI prepends this directory to `$PATH` at startup via `cli.PrependPluginBinDirToPATH()` so the existing `exec.LookPath` resolution finds managed installs without any special-casing. This is purely additive — the kubectl-style `$PATH` model is unchanged. + +`entire plugin install/list/remove` manage the contents of this directory. Authors who prefer the raw "drop a binary on `$PATH`" model don't need to use it. + +> **Compatibility note:** the `entire plugin` command group is itself a built-in. Per the "built-ins win" rule above, it shadows any external command named `entire-plugin` that may have existed on `$PATH` previously. The collision is intentional — managing plugins is a built-in concern — but worth flagging for anyone who shipped an `entire-plugin` external command before this layer landed. + ## Environment Each external-command invocation receives: @@ -25,6 +38,7 @@ Each external-command invocation receives: |---|---| | `ENTIRE_CLI_VERSION` | The CLI's version string (e.g. `0.42.0`, `dev`) | | `ENTIRE_REPO_ROOT` | Absolute path to the git repository root, when the CLI is invoked inside one. Omitted otherwise. | +| `ENTIRE_PLUGIN_DATA_DIR` | Per-plugin durable storage directory (`/data/`). Not pre-created — the plugin should `mkdir -p` on first write. Set regardless of whether the plugin is on raw `$PATH` or in the managed dir, so plugins get the same contract either way. Omitted only in degenerate environments where the per-user data root cannot be resolved (e.g. no home dir, no `LOCALAPPDATA`/`XDG_DATA_HOME`/`ENTIRE_PLUGIN_DIR`); the parent CLI prints a warning to stderr in that case. | The working directory is **not** changed — external commands run in the user's current directory, the same as any other shell command. @@ -137,5 +151,7 @@ Key files: - `cmd/entire/cli/plugin.go` — entry point, `resolvePlugin`, `runPlugin` - `cmd/entire/cli/plugin_env.go` — `pluginEnv`, the allowlist, and `ENTIRE_PLUGIN_ENV` parsing - `cmd/entire/cli/plugin_official.go` — `officialPlugins` allowlist, `IsOfficialPlugin` +- `cmd/entire/cli/plugin_store.go` — managed install directory, `PluginBinDir`, `PluginDataDir`, `InstallPluginFromPath`, `ListInstalledPlugins`, `RemoveInstalledPlugin`, `PrependPluginBinDirToPATH` +- `cmd/entire/cli/plugin_group.go` — `entire plugin install/list/remove` Cobra commands - `cmd/entire/cli/telemetry/detached.go` — `BuildPluginEventPayload`, `TrackPluginDetached` - `cmd/entire/cli/integration_test/external_command_test.go` — end-to-end coverage of the resolution path