From 5f201a0109b53b29e5ee5f7f07e94736098fa1ef Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 4 May 2026 13:53:38 +0200 Subject: [PATCH 01/14] Add kubectl-style plugin dispatch for entire- binaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the user invokes `entire ` and `` isn't a built-in subcommand, look for an `entire-` binary on PATH and exec it with the remaining args. Stdio passes through, exit code propagates, and context cancellation sends SIGINT (with a 5s grace before SIGKILL) so plugins get a chance to clean up. Built-ins always win on name conflict, and binaries matching the `entire-agent-*` prefix are explicitly skipped — they belong to the external agent protocol, not the passthrough plugin path. Plugin invocations get `ENTIRE_REPO_ROOT` and `ENTIRE_CLI_VERSION` in their environment, mirroring what external agents already see. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 9965c83a5806 --- cmd/entire/cli/plugin.go | 135 ++++++++++++++++++++++++++++++ cmd/entire/cli/plugin_test.go | 150 ++++++++++++++++++++++++++++++++++ cmd/entire/main.go | 9 ++ 3 files changed, 294 insertions(+) create mode 100644 cmd/entire/cli/plugin.go create mode 100644 cmd/entire/cli/plugin_test.go diff --git a/cmd/entire/cli/plugin.go b/cmd/entire/cli/plugin.go new file mode 100644 index 0000000000..cdcca7a505 --- /dev/null +++ b/cmd/entire/cli/plugin.go @@ -0,0 +1,135 @@ +package cli + +import ( + "context" + "errors" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "strings" + "time" + + "github.com/entireio/cli/cmd/entire/cli/paths" + "github.com/entireio/cli/cmd/entire/cli/versioninfo" + "github.com/spf13/cobra" +) + +// Plugin dispatch — kubectl-style. When the user invokes `entire ` and +// isn't a built-in subcommand, look for an `entire-` binary on +// PATH and exec it with the remaining args. Stdio and exit codes pass +// through. Binaries matching the agent protocol prefix are ignored here — +// they're handled by the external agent registry. +const ( + pluginBinaryPrefix = "entire-" + agentPluginBinaryPrefix = "entire-agent-" +) + +// MaybeDispatchPlugin returns (true, exitCode) when an external plugin +// handled the invocation, and (false, 0) otherwise (in which case the caller +// should fall through to normal Cobra execution). +func MaybeDispatchPlugin(ctx context.Context, rootCmd *cobra.Command, args []string) (handled bool, exitCode int) { + binPath, pluginArgs, ok := resolvePlugin(rootCmd, args) + if !ok { + return false, 0 + } + return true, runPlugin(ctx, binPath, pluginArgs, os.Stdin, os.Stdout, os.Stderr) +} + +// resolvePlugin decides whether args should be routed to an external plugin. +// It is split out from MaybeDispatchPlugin so tests can exercise resolution +// without spawning a subprocess. +func resolvePlugin(rootCmd *cobra.Command, args []string) (binPath string, pluginArgs []string, ok bool) { + if len(args) == 0 { + return "", nil, false + } + name := args[0] + if !isPluginCandidate(name) { + return "", nil, false + } + // Built-in commands always win. + if cmd, _, err := rootCmd.Find(args); err == nil && cmd != rootCmd { + return "", nil, false + } + binPath, err := exec.LookPath(pluginBinaryPrefix + name) + if err != nil { + return "", nil, false + } + if isAgentProtocolBinary(binPath) { + return "", nil, false + } + return binPath, args[1:], true +} + +// isPluginCandidate filters out args that obviously aren't plugin invocations: +// flags, empty strings, and names that would map onto agent protocol binaries +// or contain path separators. +func isPluginCandidate(name string) bool { + if name == "" { + return false + } + if strings.HasPrefix(name, "-") { + return false + } + if strings.HasPrefix(name, "agent-") { + return false + } + if strings.ContainsAny(name, `/\`) { + return false + } + return true +} + +// isAgentProtocolBinary returns true when the binary name is reserved for the +// external agent protocol. We check both the literal name and the +// extension-stripped form so a user with `entire-agent-foo.exe` on Windows +// still gets filtered out. +func isAgentProtocolBinary(binPath string) bool { + base := filepath.Base(binPath) + if strings.HasPrefix(base, agentPluginBinaryPrefix) { + return true + } + if strings.HasPrefix(stripPluginExeExt(base), agentPluginBinaryPrefix) { + return true + } + return false +} + +// stripPluginExeExt mirrors agent/external/discovery.stripExeExt for plugin +// name normalization on Windows. +func stripPluginExeExt(name string) string { + switch strings.ToLower(filepath.Ext(name)) { + case ".exe", ".bat", ".cmd": + return strings.TrimSuffix(name, filepath.Ext(name)) + } + return name +} + +// runPlugin executes the resolved plugin binary, propagating its exit code. +// On context cancellation the child is sent SIGINT (with a short grace +// window before the runtime falls back to SIGKILL), so plugins get a chance +// to clean up. Terminal signals (Ctrl+C) reach the child directly through +// the foreground process group as well. +func runPlugin(ctx context.Context, binPath string, args []string, stdin io.Reader, stdout, stderr io.Writer) int { + cmd := exec.CommandContext(ctx, binPath, args...) + cmd.Cancel = func() error { return cmd.Process.Signal(os.Interrupt) } + cmd.WaitDelay = 5 * time.Second + cmd.Stdin = stdin + cmd.Stdout = stdout + cmd.Stderr = stderr + cmd.Env = append(os.Environ(), "ENTIRE_CLI_VERSION="+versioninfo.Version) + if repoRoot, err := paths.WorktreeRoot(ctx); err == nil { + cmd.Env = append(cmd.Env, "ENTIRE_REPO_ROOT="+repoRoot) + } + + if err := cmd.Run(); err != nil { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + return exitErr.ExitCode() + } + fmt.Fprintf(stderr, "entire: failed to run plugin %s: %v\n", filepath.Base(binPath), err) + return 1 + } + return 0 +} diff --git a/cmd/entire/cli/plugin_test.go b/cmd/entire/cli/plugin_test.go new file mode 100644 index 0000000000..2d042befcd --- /dev/null +++ b/cmd/entire/cli/plugin_test.go @@ -0,0 +1,150 @@ +package cli + +import ( + "context" + "fmt" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/spf13/cobra" +) + +// writePluginBinary creates a shell script that records its argv to argFile and +// exits with the requested code. Returns the binary's absolute path. +func writePluginBinary(t *testing.T, dir, name, argFile string, exitCode int) string { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("plugin shell-script harness only runs on Unix") + } + path := filepath.Join(dir, name) + script := fmt.Sprintf("#!/bin/sh\nprintf '%%s\\n' \"$@\" > %q\nexit %d\n", argFile, exitCode) + if err := os.WriteFile(path, []byte(script), 0o755); err != nil { + t.Fatalf("write plugin %s: %v", path, err) + } + return path +} + +// withPathDir prepends dir to PATH for the duration of the test. +func withPathDir(t *testing.T, dir string) { + t.Helper() + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) +} + +func newTestRoot() *cobra.Command { + root := &cobra.Command{Use: "entire"} + root.AddCommand(&cobra.Command{Use: "session", Run: func(*cobra.Command, []string) {}}) + root.AddCommand(&cobra.Command{Use: "agent", Run: func(*cobra.Command, []string) {}}) + return root +} + +func TestResolvePlugin_FoundOnPath(t *testing.T) { //nolint:paralleltest // mutates PATH via t.Setenv + dir := t.TempDir() + binPath := writePluginBinary(t, dir, "entire-pgr", filepath.Join(dir, "args.txt"), 0) + withPathDir(t, dir) + + got, args, ok := resolvePlugin(newTestRoot(), []string{"pgr", "--flag", "value"}) + if !ok { + t.Fatal("expected plugin to resolve") + } + if got != binPath { + t.Errorf("binPath: got %q, want %q", got, binPath) + } + if want := []string{"--flag", "value"}; !equalStrings(args, want) { + t.Errorf("plugin args: got %v, want %v", args, want) + } +} + +func TestResolvePlugin_BuiltinWins(t *testing.T) { //nolint:paralleltest // mutates PATH via t.Setenv + dir := t.TempDir() + writePluginBinary(t, dir, "entire-session", filepath.Join(dir, "args.txt"), 0) + withPathDir(t, dir) + + if _, _, ok := resolvePlugin(newTestRoot(), []string{"session", "list"}); ok { + t.Fatal("built-in 'session' must take precedence over entire-session plugin") + } +} + +func TestResolvePlugin_NotFound(t *testing.T) { + t.Parallel() + if _, _, ok := resolvePlugin(newTestRoot(), []string{"nope-no-such-plugin"}); ok { + t.Fatal("missing plugin must not resolve") + } +} + +func TestResolvePlugin_RejectsAgentPrefix(t *testing.T) { //nolint:paralleltest // mutates PATH via t.Setenv + dir := t.TempDir() + writePluginBinary(t, dir, "entire-agent-foo", filepath.Join(dir, "args.txt"), 0) + withPathDir(t, dir) + + // Reserved name pattern — refuse before even hitting PATH. + if _, _, ok := resolvePlugin(newTestRoot(), []string{"agent-foo"}); ok { + t.Fatal("agent-foo must not resolve as a passthrough plugin") + } +} + +func TestIsAgentProtocolBinary(t *testing.T) { + t.Parallel() + cases := []struct { + path string + want bool + }{ + {"/usr/local/bin/entire-agent-foo", true}, + {"/usr/local/bin/entire-agent-foo.exe", true}, + {"/usr/local/bin/entire-pgr", false}, + {"entire-pgr", false}, + {"entire-agent-bar.bat", true}, + } + for _, tc := range cases { + if got := isAgentProtocolBinary(tc.path); got != tc.want { + t.Errorf("isAgentProtocolBinary(%q) = %v, want %v", tc.path, got, tc.want) + } + } +} + +func TestResolvePlugin_FlagAsFirstArg(t *testing.T) { + t.Parallel() + if _, _, ok := resolvePlugin(newTestRoot(), []string{"--help"}); ok { + t.Fatal("flags must not trigger plugin dispatch") + } +} + +func TestResolvePlugin_PathTraversal(t *testing.T) { + t.Parallel() + for _, name := range []string{"../evil", `..\evil`, "foo/bar"} { + if _, _, ok := resolvePlugin(newTestRoot(), []string{name}); ok { + t.Errorf("name %q must not resolve", name) + } + } +} + +func TestRunPlugin_ExitCodePropagation(t *testing.T) { //nolint:paralleltest // mutates PATH via t.Setenv + dir := t.TempDir() + binPath := writePluginBinary(t, dir, "entire-exit42", filepath.Join(dir, "args.txt"), 42) + + code := runPlugin(context.Background(), binPath, []string{"a", "b"}, nil, os.Stderr, os.Stderr) + if code != 42 { + t.Errorf("exit code: got %d, want 42", code) + } + contents, err := os.ReadFile(filepath.Join(dir, "args.txt")) + if err != nil { + t.Fatalf("read argfile: %v", err) + } + if got := strings.TrimSpace(string(contents)); got != "a\nb" { + t.Errorf("argv: got %q, want %q", got, "a\nb") + } +} + +func equalStrings(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/cmd/entire/main.go b/cmd/entire/main.go index b829d02122..f078d70391 100644 --- a/cmd/entire/main.go +++ b/cmd/entire/main.go @@ -32,6 +32,15 @@ func main() { // Create and execute root command rootCmd := cli.NewRootCmd() + + // Kubectl-style plugin dispatch: `entire ` → exec `entire-` + // on PATH when isn't a built-in. Built-ins always win; agent + // protocol binaries (entire-agent-*) are skipped here. + if handled, code := cli.MaybeDispatchPlugin(ctx, rootCmd, os.Args[1:]); handled { + cancel() + os.Exit(code) + } + err := rootCmd.ExecuteContext(ctx) if err != nil { var silent *cli.SilentError From d46db53c796175a32c2f0b2f3894e05e22ad2ab7 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 4 May 2026 13:54:21 +0200 Subject: [PATCH 02/14] Track plugin invocations for an allowlisted set only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirror gh's extension policy: third-party plugin names can carry sensitive identifiers, so we never record them. Plugins listed in the hardcoded officialPlugins allowlist (currently empty — populate as Entire ships its own plugins) emit a `cli_plugin_executed` event with the plugin name; everything else runs silently. Telemetry follows the existing opt-in semantics — settings.Telemetry must be explicitly true and ENTIRE_TELEMETRY_OPTOUT must be unset. The event payload deliberately omits flags and args; only command path, plugin name, cli_version, os, arch, and isEntireEnabled are recorded. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 34fb779fd053 --- cmd/entire/cli/plugin.go | 23 ++++++++++- cmd/entire/cli/plugin_official.go | 28 +++++++++++++ cmd/entire/cli/plugin_official_test.go | 42 ++++++++++++++++++++ cmd/entire/cli/telemetry/detached.go | 48 +++++++++++++++++++++++ cmd/entire/cli/telemetry/detached_test.go | 37 +++++++++++++++++ 5 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 cmd/entire/cli/plugin_official.go create mode 100644 cmd/entire/cli/plugin_official_test.go diff --git a/cmd/entire/cli/plugin.go b/cmd/entire/cli/plugin.go index cdcca7a505..cdf36e40e2 100644 --- a/cmd/entire/cli/plugin.go +++ b/cmd/entire/cli/plugin.go @@ -12,6 +12,7 @@ import ( "time" "github.com/entireio/cli/cmd/entire/cli/paths" + "github.com/entireio/cli/cmd/entire/cli/telemetry" "github.com/entireio/cli/cmd/entire/cli/versioninfo" "github.com/spf13/cobra" ) @@ -34,7 +35,27 @@ func MaybeDispatchPlugin(ctx context.Context, rootCmd *cobra.Command, args []str if !ok { return false, 0 } - return true, runPlugin(ctx, binPath, pluginArgs, os.Stdin, os.Stdout, os.Stderr) + pluginName := args[0] + exitCode = runPlugin(ctx, binPath, pluginArgs, os.Stdin, os.Stdout, os.Stderr) + maybeTrackPluginInvocation(ctx, pluginName) + return true, exitCode +} + +// maybeTrackPluginInvocation fires telemetry only for plugins on the +// official allowlist and only when telemetry is opted in via settings. +// Third-party plugin names are never sent. +func maybeTrackPluginInvocation(ctx context.Context, pluginName string) { + if !IsOfficialPlugin(pluginName) { + return + } + s, err := LoadEntireSettings(ctx) + if err != nil { + return + } + if s.Telemetry == nil || !*s.Telemetry { + return + } + telemetry.TrackPluginDetached(pluginName, s.Enabled, versioninfo.Version) } // resolvePlugin decides whether args should be routed to an external plugin. diff --git a/cmd/entire/cli/plugin_official.go b/cmd/entire/cli/plugin_official.go new file mode 100644 index 0000000000..5ce576726a --- /dev/null +++ b/cmd/entire/cli/plugin_official.go @@ -0,0 +1,28 @@ +package cli + +// Official plugin allowlist. Names listed here may contribute their plugin +// name to telemetry; all other (third-party / user-installed) plugins are +// invoked silently with no event recorded. The reasoning mirrors gh's +// extension policy — third-party plugin names can carry sensitive identifiers +// (project names, vendor names), so we only attribute usage for plugins we +// ship ourselves. +// +// To add a plugin: append its name to officialPlugins. Match must be exact +// and case-sensitive. The corresponding binary is `entire-`. +// +//nolint:gochecknoglobals // small immutable allowlist +var officialPlugins = []string{ + // Add Entire-shipped plugin names here as they're released. + // e.g. "pgr" +} + +// IsOfficialPlugin reports whether name appears in the hardcoded allowlist. +// Used to decide whether plugin invocation telemetry should record the name. +func IsOfficialPlugin(name string) bool { + for _, p := range officialPlugins { + if p == name { + return true + } + } + return false +} diff --git a/cmd/entire/cli/plugin_official_test.go b/cmd/entire/cli/plugin_official_test.go new file mode 100644 index 0000000000..f115e82e71 --- /dev/null +++ b/cmd/entire/cli/plugin_official_test.go @@ -0,0 +1,42 @@ +package cli + +import "testing" + +func TestIsOfficialPlugin(t *testing.T) { + t.Parallel() + // Snapshot and restore the package-level allowlist so this test stays + // independent of which plugins ship by default. + saved := officialPlugins + t.Cleanup(func() { officialPlugins = saved }) + + officialPlugins = []string{"pgr", "stack"} + + cases := []struct { + name string + want bool + }{ + {"pgr", true}, + {"stack", true}, + {"PGR", false}, // case-sensitive + {"pgr2", false}, // exact match only + {"", false}, + {"unknown", false}, + } + for _, tc := range cases { + if got := IsOfficialPlugin(tc.name); got != tc.want { + t.Errorf("IsOfficialPlugin(%q) = %v, want %v", tc.name, got, tc.want) + } + } +} + +func TestOfficialPlugins_DefaultAllowlist(t *testing.T) { + t.Parallel() + // Sanity check: no third-party plugin name should be officially + // recognized by default. If you are adding an Entire-shipped plugin, + // update this test alongside the registry. + for _, n := range officialPlugins { + if n == "" { + t.Errorf("officialPlugins contains an empty name") + } + } +} diff --git a/cmd/entire/cli/telemetry/detached.go b/cmd/entire/cli/telemetry/detached.go index 303a1fef1f..71d20d20f7 100644 --- a/cmd/entire/cli/telemetry/detached.go +++ b/cmd/entire/cli/telemetry/detached.go @@ -109,6 +109,54 @@ func TrackCommandDetached(cmd *cobra.Command, agent string, isEntireEnabled bool } } +// BuildPluginEventPayload constructs the event payload for a plugin invocation. +// The plugin name is embedded as a property; we never record plugin args or +// flags. Returns nil if the payload cannot be built. +func BuildPluginEventPayload(pluginName string, isEntireEnabled bool, version string) *EventPayload { + if pluginName == "" { + return nil + } + + machineID, err := machineid.ProtectedID("entire-cli") + if err != nil { + return nil + } + + properties := map[string]any{ + "command": "entire " + pluginName, + "plugin": pluginName, + "isEntireEnabled": isEntireEnabled, + "cli_version": version, + "os": runtime.GOOS, + "arch": runtime.GOARCH, + } + + return &EventPayload{ + Event: "cli_plugin_executed", + DistinctID: machineID, + Properties: properties, + Timestamp: time.Now(), + } +} + +// TrackPluginDetached records a plugin invocation event in a detached +// subprocess. Call sites are responsible for restricting invocation to +// allowlisted plugin names so third-party plugin identifiers don't leak. +func TrackPluginDetached(pluginName string, isEntireEnabled bool, version string) { + if os.Getenv("ENTIRE_TELEMETRY_OPTOUT") != "" { + return + } + + payload := BuildPluginEventPayload(pluginName, isEntireEnabled, version) + if payload == nil { + return + } + + if payloadJSON, err := json.Marshal(payload); err == nil { + spawnDetachedAnalytics(string(payloadJSON)) + } +} + // SendEvent processes an event payload in the detached subprocess. // This is called by the hidden __send_analytics command. func SendEvent(payloadJSON string) { diff --git a/cmd/entire/cli/telemetry/detached_test.go b/cmd/entire/cli/telemetry/detached_test.go index 780646edb6..217d061343 100644 --- a/cmd/entire/cli/telemetry/detached_test.go +++ b/cmd/entire/cli/telemetry/detached_test.go @@ -8,6 +8,43 @@ import ( "github.com/spf13/cobra" ) +func TestBuildPluginEventPayload(t *testing.T) { + t.Parallel() + payload := BuildPluginEventPayload("pgr", true, "1.2.3") + if payload == nil { + t.Fatal("BuildPluginEventPayload returned nil") + } + if payload.Event != "cli_plugin_executed" { + t.Errorf("Event = %q, want %q", payload.Event, "cli_plugin_executed") + } + if got := payload.Properties["plugin"]; got != "pgr" { + t.Errorf("plugin property = %v, want %q", got, "pgr") + } + if got := payload.Properties["command"]; got != "entire pgr" { + t.Errorf("command property = %v, want %q", got, "entire pgr") + } + if got := payload.Properties["cli_version"]; got != "1.2.3" { + t.Errorf("cli_version property = %v, want %q", got, "1.2.3") + } + if got := payload.Properties["isEntireEnabled"]; got != true { + t.Errorf("isEntireEnabled property = %v, want true", got) + } + // Plugin args/flags must never appear in the payload. + if _, ok := payload.Properties["flags"]; ok { + t.Error("plugin payload must not include 'flags'") + } + if _, ok := payload.Properties["args"]; ok { + t.Error("plugin payload must not include 'args'") + } +} + +func TestBuildPluginEventPayload_EmptyName(t *testing.T) { + t.Parallel() + if got := BuildPluginEventPayload("", true, "1.0.0"); got != nil { + t.Errorf("expected nil for empty plugin name, got %+v", got) + } +} + func TestEventPayloadSerialization(t *testing.T) { payload := EventPayload{ Event: "cli_command_executed", From 8e30f4e917b17b951865540b8edddb457c7e5e0c Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 4 May 2026 14:40:58 +0200 Subject: [PATCH 03/14] Add integration coverage for the early-dispatch path in main.go The unit tests in cmd/entire/cli/plugin_test.go exercise resolvePlugin and runPlugin in isolation but never invoke the real `entire` binary, so the wiring in cmd/entire/main.go (pre-Cobra dispatch, exit-code propagation, stdio passthrough, signal handling) was only covered by manual smoke checks. These integration tests build the actual binary and shell out: - happy-path argv/stdout/stderr/exit-code - exit code propagation (non-zero) - built-in commands win over plugins of the same name - unknown command falls through to Cobra's normal error path - flag-shaped args after the plugin name aren't eaten by Cobra - stdin passthrough - ENTIRE_CLI_VERSION reaches the plugin's environment - entire-agent-* binaries are skipped (reserved for the agent protocol) A separate Unix-only file covers the signal edge: SIGINT to the parent must reach the plugin via the dispatcher's context-cancel handler so the plugin can clean up rather than being SIGKILL'd. The plugin writes a readiness marker before signalling to avoid racing the trap install against the kill. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: abcdeb6500a8 --- .../plugin_dispatch_signal_unix_test.go | 95 ++++++ .../integration_test/plugin_dispatch_test.go | 281 ++++++++++++++++++ 2 files changed, 376 insertions(+) create mode 100644 cmd/entire/cli/integration_test/plugin_dispatch_signal_unix_test.go create mode 100644 cmd/entire/cli/integration_test/plugin_dispatch_test.go diff --git a/cmd/entire/cli/integration_test/plugin_dispatch_signal_unix_test.go b/cmd/entire/cli/integration_test/plugin_dispatch_signal_unix_test.go new file mode 100644 index 0000000000..e4be132e07 --- /dev/null +++ b/cmd/entire/cli/integration_test/plugin_dispatch_signal_unix_test.go @@ -0,0 +1,95 @@ +//go:build integration && !windows + +package integration + +import ( + "bytes" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +// TestPluginDispatch_SigintReachesPlugin asserts that when the parent CLI +// receives SIGINT, the running plugin gets a chance to handle it (rather +// than being SIGKILL'd). The contract: runPlugin uses CommandContext with a +// custom Cancel that sends SIGINT, plus terminal SIGINT reaches the child +// directly via the shared process group. Either path getting the signal to +// the child is acceptable; both being missing is the regression we guard. +func TestPluginDispatch_SigintReachesPlugin(t *testing.T) { + t.Parallel() + dir := t.TempDir() + signalFile := filepath.Join(dir, "got-sigint.txt") + + // The plugin installs a SIGINT trap, writes a "ready" marker after the + // trap is in place, then loops. The test waits for "ready" before + // signalling so the trap is guaranteed to be installed (avoids racing + // SIGINT against shell startup). On SIGINT the plugin writes the + // "trapped" marker and exits 130. Without a working signal path the + // parent's WaitDelay (5s) would expire and the child would be SIGKILL'd + // with no marker. + readyFile := filepath.Join(dir, "ready.txt") + body := fmt.Sprintf( + "#!/bin/sh\ntrap 'echo trapped > %q; exit 130' INT\n"+ + "echo ready > %q\n"+ + "i=0\nwhile [ $i -lt 100 ]; do sleep 0.1; i=$((i+1)); done\nexit 0\n", + signalFile, readyFile, + ) + if err := os.WriteFile(filepath.Join(dir, "entire-trapint"), []byte(body), 0o755); err != nil { //nolint:gosec // test fixture + t.Fatalf("write plugin: %v", err) + } + + cmd := exec.Command(getTestBinary(), "trapint") + cmd.Env = pathWith(dir) + var pStderr bytes.Buffer + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &pStderr + + if err := cmd.Start(); err != nil { + t.Fatalf("start: %v", err) + } + + // Wait for the plugin to install its trap before signalling. + if !waitForFile(readyFile, 3*time.Second) { + _ = cmd.Process.Kill() + _ = cmd.Wait() + t.Fatalf("plugin never reached ready state\nparent stderr:\n%s", pStderr.String()) + } + + // Send SIGINT only to the parent (entire) PID. The child must learn + // about it through the parent's context-cancel handler invoking + // exec.Cmd.Cancel, which sends SIGINT to the child. + if err := cmd.Process.Signal(os.Interrupt); err != nil { + t.Fatalf("signal parent: %v", err) + } + + if !waitForFile(signalFile, 5*time.Second) { + _ = cmd.Wait() + t.Fatalf("plugin never observed SIGINT — marker missing\nparent stderr:\n%s", pStderr.String()) + } + _ = cmd.Wait() + + contents, err := os.ReadFile(signalFile) + if err != nil { + t.Fatalf("read marker: %v", err) + } + if got := strings.TrimSpace(string(contents)); got != "trapped" { + t.Errorf("marker = %q, want %q", got, "trapped") + } +} + +// waitForFile polls until path exists or the deadline elapses. Returns true +// if the file appeared, false on timeout. +func waitForFile(path string, timeout time.Duration) bool { + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + if _, err := os.Stat(path); err == nil { + return true + } + time.Sleep(20 * time.Millisecond) + } + return false +} diff --git a/cmd/entire/cli/integration_test/plugin_dispatch_test.go b/cmd/entire/cli/integration_test/plugin_dispatch_test.go new file mode 100644 index 0000000000..b506fe4748 --- /dev/null +++ b/cmd/entire/cli/integration_test/plugin_dispatch_test.go @@ -0,0 +1,281 @@ +//go:build integration + +package integration + +import ( + "bytes" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// These tests exercise the early-dispatch path in cmd/entire/main.go: the +// real `entire` binary is invoked with `entire ` arguments, and we +// verify a kubectl-style `entire-` plugin on PATH is exec'd before +// Cobra handles the args. Coverage here complements the unit tests in +// cmd/entire/cli/plugin_test.go, which test resolvePlugin/runPlugin in +// isolation but cannot validate the main() wiring or exit-code propagation +// of the actual binary. + +// writePluginScript creates a shell script plugin at dir/ that +// records its argv to argFile (one line per arg) and exits with exitCode. +// On non-Unix platforms the test calling this is skipped. +func writePluginScript(t *testing.T, dir, binaryName, argFile string, exitCode int) string { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("plugin shell-script harness only runs on Unix") + } + path := filepath.Join(dir, binaryName) + body := fmt.Sprintf( + "#!/bin/sh\nprintf '%%s\\n' \"$@\" > %q\n"+ + "echo \"plugin stdout\"\n"+ + "echo \"plugin stderr\" 1>&2\n"+ + "exit %d\n", + argFile, exitCode, + ) + if err := os.WriteFile(path, []byte(body), 0o755); err != nil { //nolint:gosec // test fixture + t.Fatalf("write plugin %s: %v", path, err) + } + return path +} + +// pathWith prepends dir to the current PATH and returns a slice suitable for +// passing as cmd.Env (as opposed to mutating the test process environment, +// which would break parallel tests). +func pathWith(dir string) []string { + env := os.Environ() + for i, kv := range env { + if strings.HasPrefix(kv, "PATH=") { + env[i] = "PATH=" + dir + string(os.PathListSeparator) + strings.TrimPrefix(kv, "PATH=") + return env + } + } + return append(env, "PATH="+dir) +} + +func TestPluginDispatch_HappyPath(t *testing.T) { + t.Parallel() + dir := t.TempDir() + argFile := filepath.Join(dir, "argv.txt") + writePluginScript(t, dir, "entire-pgr", argFile, 0) + + cmd := exec.Command(getTestBinary(), "pgr", "hello", "--flag", "value") + cmd.Env = pathWith(dir) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + if err := cmd.Run(); err != nil { + t.Fatalf("entire pgr failed: %v\nstderr: %s", err, stderr.String()) + } + if got := strings.TrimSpace(stdout.String()); got != "plugin stdout" { + t.Errorf("stdout = %q, want %q", got, "plugin stdout") + } + if got := strings.TrimSpace(stderr.String()); got != "plugin stderr" { + t.Errorf("stderr = %q, want %q", got, "plugin stderr") + } + argsBytes, err := os.ReadFile(argFile) + if err != nil { + t.Fatalf("read argv file: %v", err) + } + if got := strings.TrimSpace(string(argsBytes)); got != "hello\n--flag\nvalue" { + t.Errorf("plugin argv = %q, want %q", got, "hello\n--flag\nvalue") + } +} + +func TestPluginDispatch_ExitCodePropagation(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writePluginScript(t, dir, "entire-failing", filepath.Join(dir, "argv.txt"), 42) + + cmd := exec.Command(getTestBinary(), "failing") + cmd.Env = pathWith(dir) + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &bytes.Buffer{} + + err := cmd.Run() + if err == nil { + t.Fatal("expected non-zero exit") + } + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("expected *exec.ExitError, got %T: %v", err, err) + } + if got := exitErr.ExitCode(); got != 42 { + t.Errorf("exit code = %d, want 42", got) + } +} + +func TestPluginDispatch_BuiltinWins(t *testing.T) { + t.Parallel() + dir := t.TempDir() + // A plugin shadowing a real built-in must NOT be invoked. If the plugin + // runs, it exits 99 with sentinel output we can detect. + body := "#!/bin/sh\necho 'plugin-shadowed-builtin'\nexit 99\n" + if runtime.GOOS == "windows" { + t.Skip("plugin shell-script harness only runs on Unix") + } + if err := os.WriteFile(filepath.Join(dir, "entire-version"), []byte(body), 0o755); err != nil { //nolint:gosec // test fixture + t.Fatalf("write plugin: %v", err) + } + + cmd := exec.Command(getTestBinary(), "version") + cmd.Env = pathWith(dir) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + if err := cmd.Run(); err != nil { + t.Fatalf("entire version failed: %v\nstderr: %s", err, stderr.String()) + } + if strings.Contains(stdout.String(), "plugin-shadowed-builtin") { + t.Errorf("built-in version was shadowed by entire-version plugin\nstdout: %s", stdout.String()) + } + if !strings.Contains(stdout.String(), "Entire CLI") { + t.Errorf("expected built-in version output, got: %s", stdout.String()) + } +} + +func TestPluginDispatch_PluginNotFound(t *testing.T) { + t.Parallel() + // PATH deliberately points at an empty dir so no plugin can resolve. + dir := t.TempDir() + + cmd := exec.Command(getTestBinary(), "definitely-not-a-real-plugin-or-builtin") + cmd.Env = pathWith(dir) + var stderr bytes.Buffer + cmd.Stderr = &stderr + + err := cmd.Run() + if err == nil { + t.Fatal("expected failure for unknown command") + } + // Cobra's normal error path should fire — the dispatcher must not have + // swallowed the invocation. + if !strings.Contains(stderr.String(), "unknown command") && + !strings.Contains(stderr.String(), "Invalid usage") { + t.Errorf("expected Cobra unknown-command error, got stderr: %s", stderr.String()) + } +} + +func TestPluginDispatch_FlagAfterPluginNameNotEatenByCobra(t *testing.T) { + t.Parallel() + // Cobra normally interprets --help itself. The dispatcher runs before + // Cobra parses, so once we're routing to a plugin everything (including + // flag-shaped args) must reach the child verbatim. + dir := t.TempDir() + argFile := filepath.Join(dir, "argv.txt") + writePluginScript(t, dir, "entire-passthrough", argFile, 0) + + cmd := exec.Command(getTestBinary(), "passthrough", "--help", "--version", "subcmd") + cmd.Env = pathWith(dir) + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &bytes.Buffer{} + if err := cmd.Run(); err != nil { + t.Fatalf("entire passthrough failed: %v", err) + } + + argsBytes, err := os.ReadFile(argFile) + if err != nil { + t.Fatalf("read argv file: %v", err) + } + want := "--help\n--version\nsubcmd" + if got := strings.TrimSpace(string(argsBytes)); got != want { + t.Errorf("plugin argv = %q, want %q (Cobra ate flags)", got, want) + } +} + +func TestPluginDispatch_StdinPassthrough(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { + t.Skip("plugin shell-script harness only runs on Unix") + } + dir := t.TempDir() + outFile := filepath.Join(dir, "stdin.txt") + body := fmt.Sprintf("#!/bin/sh\ncat > %q\nexit 0\n", outFile) + if err := os.WriteFile(filepath.Join(dir, "entire-stdincat"), []byte(body), 0o755); err != nil { //nolint:gosec // test fixture + t.Fatalf("write plugin: %v", err) + } + + cmd := exec.Command(getTestBinary(), "stdincat") + cmd.Env = pathWith(dir) + cmd.Stdin = strings.NewReader("hello from parent stdin\n") + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &bytes.Buffer{} + if err := cmd.Run(); err != nil { + t.Fatalf("entire stdincat failed: %v", err) + } + + got, err := os.ReadFile(outFile) + if err != nil { + t.Fatalf("read stdin file: %v", err) + } + if want := "hello from parent stdin\n"; string(got) != want { + t.Errorf("plugin stdin = %q, want %q", string(got), want) + } +} + +func TestPluginDispatch_EnvVarsForwarded(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { + t.Skip("plugin shell-script harness only runs on Unix") + } + dir := t.TempDir() + envFile := filepath.Join(dir, "env.txt") + body := fmt.Sprintf( + "#!/bin/sh\necho \"ENTIRE_CLI_VERSION=$ENTIRE_CLI_VERSION\" > %q\nexit 0\n", + envFile, + ) + if err := os.WriteFile(filepath.Join(dir, "entire-envcheck"), []byte(body), 0o755); err != nil { //nolint:gosec // test fixture + t.Fatalf("write plugin: %v", err) + } + + cmd := exec.Command(getTestBinary(), "envcheck") + cmd.Env = pathWith(dir) + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &bytes.Buffer{} + if err := cmd.Run(); err != nil { + t.Fatalf("entire envcheck failed: %v", err) + } + + got, err := os.ReadFile(envFile) + if err != nil { + t.Fatalf("read env file: %v", err) + } + out := strings.TrimSpace(string(got)) + if !strings.HasPrefix(out, "ENTIRE_CLI_VERSION=") { + t.Fatalf("env line missing prefix: %q", out) + } + // Just confirm the variable is set to *something* non-empty — value + // depends on build-time linker flags and will be "dev" in tests. + if strings.TrimPrefix(out, "ENTIRE_CLI_VERSION=") == "" { + t.Errorf("ENTIRE_CLI_VERSION was empty") + } +} + +func TestPluginDispatch_AgentProtocolBinarySkipped(t *testing.T) { + t.Parallel() + // `entire agent-foo` must not be routed to entire-agent-foo (which is a + // protocol binary, not a passthrough plugin). Cobra should see "agent-foo" + // as an unknown command — the literal `agent` group exists, but + // `agent-foo` is not a subcommand of agent and not a passthrough name. + dir := t.TempDir() + writePluginScript(t, dir, "entire-agent-foo", filepath.Join(dir, "argv.txt"), 0) + + cmd := exec.Command(getTestBinary(), "agent-foo") + cmd.Env = pathWith(dir) + var stderr bytes.Buffer + cmd.Stderr = &stderr + + if err := cmd.Run(); err == nil { + t.Fatal("expected failure — entire-agent-* must not be dispatched as a plugin") + } + if _, err := os.Stat(filepath.Join(dir, "argv.txt")); err == nil { + t.Error("entire-agent-foo was invoked but must have been skipped") + } +} From 63a7d21e23768052a5e02bde2168f9b838dc5eb9 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 4 May 2026 15:56:17 +0200 Subject: [PATCH 04/14] Simplify plugin dispatch: dedupe, trim narrative comments, tighten tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code reuse: - Export agent/external.StripExeExt and reuse it from plugin.go instead of the duplicated stripPluginExeExt helper. - Replace the manual loop in IsOfficialPlugin with slices.Contains. Quality: - Collapse the redundant double-prefix check in isAgentProtocolBinary — StripExeExt only removes suffixes, so the un-stripped check is subsumed by the stripped check. - Drop the stdin/stdout/stderr params from runPlugin; production always routes to os.Std*, and the test only asserts argv + exit code. - Align runPlugin's error message with the codebase convention ("Failed to run plugin X" — no "entire:" prefix). - Strip narrative/restating doc comments across plugin.go, plugin_official.go, detached.go, main.go, and the tests; keep only non-obvious WHY. - Replace the always-passing TestOfficialPlugins_DefaultAllowlist with the misleading "//immutable" comment removed from the global. Test improvements: - TestPluginDispatch_BuiltinWins now uses writePluginScript like its siblings instead of inlining its own shell. - TestPluginDispatch_AgentProtocolBinarySkipped now also asserts the invocation falls through to Cobra's unknown-command path, not just that the binary wasn't run. - The signal test's plugin-loop bound is now expressed via a named constant tied to the parent's WaitDelay, documenting why it must outlast the SIGKILL fallback window. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 65bf76b862d4 --- cmd/entire/cli/agent/external/discovery.go | 6 +- .../cli/agent/external/external_test.go | 4 +- .../plugin_dispatch_signal_unix_test.go | 31 +++------ .../integration_test/plugin_dispatch_test.go | 56 +++++++--------- cmd/entire/cli/plugin.go | 66 +++++++------------ cmd/entire/cli/plugin_official.go | 27 +++----- cmd/entire/cli/plugin_official_test.go | 17 +---- cmd/entire/cli/plugin_test.go | 11 ++-- cmd/entire/cli/telemetry/detached.go | 10 ++- cmd/entire/main.go | 3 - 10 files changed, 81 insertions(+), 150 deletions(-) diff --git a/cmd/entire/cli/agent/external/discovery.go b/cmd/entire/cli/agent/external/discovery.go index 2f5f944b47..177a9e27a3 100644 --- a/cmd/entire/cli/agent/external/discovery.go +++ b/cmd/entire/cli/agent/external/discovery.go @@ -84,7 +84,7 @@ func discoverAndRegister(ctx context.Context) { // Strip Windows executable extensions (.exe, .bat) before deriving agent name. // On Unix, binaries have no extension, so this is a no-op. - cleanName := stripExeExt(name) + cleanName := StripExeExt(name) agentName := types.AgentName(strings.TrimPrefix(cleanName, binaryPrefix)) if registered[agentName] { logging.Debug(ctx, "skipping external agent (name conflict with built-in)", @@ -131,10 +131,10 @@ func discoverAndRegister(ctx context.Context) { } } -// stripExeExt removes Windows executable extensions (.exe, .bat, .cmd) from a +// 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. -func stripExeExt(name string) string { +func StripExeExt(name string) string { switch strings.ToLower(filepath.Ext(name)) { case ".exe", ".bat", ".cmd": return strings.TrimSuffix(name, filepath.Ext(name)) diff --git a/cmd/entire/cli/agent/external/external_test.go b/cmd/entire/cli/agent/external/external_test.go index 643a616fcd..9c39b110ca 100644 --- a/cmd/entire/cli/agent/external/external_test.go +++ b/cmd/entire/cli/agent/external/external_test.go @@ -876,8 +876,8 @@ func TestStripExeExt(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - if got := stripExeExt(tt.in); got != tt.want { - t.Errorf("stripExeExt(%q) = %q, want %q", tt.in, got, tt.want) + if got := StripExeExt(tt.in); got != tt.want { + t.Errorf("StripExeExt(%q) = %q, want %q", tt.in, got, tt.want) } }) } diff --git a/cmd/entire/cli/integration_test/plugin_dispatch_signal_unix_test.go b/cmd/entire/cli/integration_test/plugin_dispatch_signal_unix_test.go index e4be132e07..4e9f8fe285 100644 --- a/cmd/entire/cli/integration_test/plugin_dispatch_signal_unix_test.go +++ b/cmd/entire/cli/integration_test/plugin_dispatch_signal_unix_test.go @@ -13,30 +13,25 @@ import ( "time" ) -// TestPluginDispatch_SigintReachesPlugin asserts that when the parent CLI -// receives SIGINT, the running plugin gets a chance to handle it (rather -// than being SIGKILL'd). The contract: runPlugin uses CommandContext with a -// custom Cancel that sends SIGINT, plus terminal SIGINT reaches the child -// directly via the shared process group. Either path getting the signal to -// the child is acceptable; both being missing is the regression we guard. +// SIGINT to the parent must reach the plugin so it can clean up — not +// just be SIGKILL'd by the runtime. Guards both signal paths: terminal +// (via process group) and parent's context-cancel handler. func TestPluginDispatch_SigintReachesPlugin(t *testing.T) { t.Parallel() dir := t.TempDir() signalFile := filepath.Join(dir, "got-sigint.txt") - // The plugin installs a SIGINT trap, writes a "ready" marker after the - // trap is in place, then loops. The test waits for "ready" before - // signalling so the trap is guaranteed to be installed (avoids racing - // SIGINT against shell startup). On SIGINT the plugin writes the - // "trapped" marker and exits 130. Without a working signal path the - // parent's WaitDelay (5s) would expire and the child would be SIGKILL'd - // with no marker. + // The plugin loops longer than the parent's WaitDelay+grace so that if + // the signal path were broken, the parent would SIGKILL the child and + // the marker would never be written. Ready-marker handshake avoids + // racing SIGINT against shell startup before the trap is installed. readyFile := filepath.Join(dir, "ready.txt") + const pluginLoopSeconds = 10 // > parent WaitDelay (5s) + grace body := fmt.Sprintf( "#!/bin/sh\ntrap 'echo trapped > %q; exit 130' INT\n"+ "echo ready > %q\n"+ - "i=0\nwhile [ $i -lt 100 ]; do sleep 0.1; i=$((i+1)); done\nexit 0\n", - signalFile, readyFile, + "i=0\nwhile [ $i -lt %d ]; do sleep 0.1; i=$((i+1)); done\nexit 0\n", + signalFile, readyFile, pluginLoopSeconds*10, ) if err := os.WriteFile(filepath.Join(dir, "entire-trapint"), []byte(body), 0o755); err != nil { //nolint:gosec // test fixture t.Fatalf("write plugin: %v", err) @@ -52,16 +47,12 @@ func TestPluginDispatch_SigintReachesPlugin(t *testing.T) { t.Fatalf("start: %v", err) } - // Wait for the plugin to install its trap before signalling. if !waitForFile(readyFile, 3*time.Second) { _ = cmd.Process.Kill() _ = cmd.Wait() t.Fatalf("plugin never reached ready state\nparent stderr:\n%s", pStderr.String()) } - // Send SIGINT only to the parent (entire) PID. The child must learn - // about it through the parent's context-cancel handler invoking - // exec.Cmd.Cancel, which sends SIGINT to the child. if err := cmd.Process.Signal(os.Interrupt); err != nil { t.Fatalf("signal parent: %v", err) } @@ -81,8 +72,6 @@ func TestPluginDispatch_SigintReachesPlugin(t *testing.T) { } } -// waitForFile polls until path exists or the deadline elapses. Returns true -// if the file appeared, false on timeout. func waitForFile(path string, timeout time.Duration) bool { deadline := time.Now().Add(timeout) for time.Now().Before(deadline) { diff --git a/cmd/entire/cli/integration_test/plugin_dispatch_test.go b/cmd/entire/cli/integration_test/plugin_dispatch_test.go index b506fe4748..b1e01f4c02 100644 --- a/cmd/entire/cli/integration_test/plugin_dispatch_test.go +++ b/cmd/entire/cli/integration_test/plugin_dispatch_test.go @@ -14,17 +14,13 @@ import ( "testing" ) -// These tests exercise the early-dispatch path in cmd/entire/main.go: the -// real `entire` binary is invoked with `entire ` arguments, and we -// verify a kubectl-style `entire-` plugin on PATH is exec'd before -// Cobra handles the args. Coverage here complements the unit tests in -// cmd/entire/cli/plugin_test.go, which test resolvePlugin/runPlugin in -// isolation but cannot validate the main() wiring or exit-code propagation -// of the actual binary. +// Integration tests for the early-dispatch path in cmd/entire/main.go. +// They build and exec the real binary so the wiring (pre-Cobra dispatch, +// exit-code propagation, stdio passthrough, signal handling) is exercised +// end-to-end — unit tests in cmd/entire/cli/plugin_test.go can't. -// writePluginScript creates a shell script plugin at dir/ that -// records its argv to argFile (one line per arg) and exits with exitCode. -// On non-Unix platforms the test calling this is skipped. +// writePluginScript writes a shell script that records argv and exits +// with exitCode. Skips the calling test on Windows. func writePluginScript(t *testing.T, dir, binaryName, argFile string, exitCode int) string { t.Helper() if runtime.GOOS == "windows" { @@ -44,9 +40,8 @@ func writePluginScript(t *testing.T, dir, binaryName, argFile string, exitCode i return path } -// pathWith prepends dir to the current PATH and returns a slice suitable for -// passing as cmd.Env (as opposed to mutating the test process environment, -// which would break parallel tests). +// pathWith returns os.Environ with dir prepended to PATH. Returning a +// fresh env slice (rather than t.Setenv) keeps tests parallel-safe. func pathWith(dir string) []string { env := os.Environ() for i, kv := range env { @@ -114,15 +109,9 @@ func TestPluginDispatch_ExitCodePropagation(t *testing.T) { func TestPluginDispatch_BuiltinWins(t *testing.T) { t.Parallel() dir := t.TempDir() - // A plugin shadowing a real built-in must NOT be invoked. If the plugin - // runs, it exits 99 with sentinel output we can detect. - body := "#!/bin/sh\necho 'plugin-shadowed-builtin'\nexit 99\n" - if runtime.GOOS == "windows" { - t.Skip("plugin shell-script harness only runs on Unix") - } - if err := os.WriteFile(filepath.Join(dir, "entire-version"), []byte(body), 0o755); err != nil { //nolint:gosec // test fixture - t.Fatalf("write plugin: %v", err) - } + // If the shadowing plugin ran, the parent's exit code would be 99 + // (writePluginScript bakes that in via the requested code). + writePluginScript(t, dir, "entire-version", filepath.Join(dir, "argv.txt"), 99) cmd := exec.Command(getTestBinary(), "version") cmd.Env = pathWith(dir) @@ -133,8 +122,8 @@ func TestPluginDispatch_BuiltinWins(t *testing.T) { if err := cmd.Run(); err != nil { t.Fatalf("entire version failed: %v\nstderr: %s", err, stderr.String()) } - if strings.Contains(stdout.String(), "plugin-shadowed-builtin") { - t.Errorf("built-in version was shadowed by entire-version plugin\nstdout: %s", stdout.String()) + if _, err := os.Stat(filepath.Join(dir, "argv.txt")); err == nil { + t.Errorf("entire-version plugin was invoked but built-in must take precedence\nstdout: %s", stdout.String()) } if !strings.Contains(stdout.String(), "Entire CLI") { t.Errorf("expected built-in version output, got: %s", stdout.String()) @@ -165,9 +154,8 @@ func TestPluginDispatch_PluginNotFound(t *testing.T) { func TestPluginDispatch_FlagAfterPluginNameNotEatenByCobra(t *testing.T) { t.Parallel() - // Cobra normally interprets --help itself. The dispatcher runs before - // Cobra parses, so once we're routing to a plugin everything (including - // flag-shaped args) must reach the child verbatim. + // Once we're routing to a plugin, flag-shaped args must reach the + // child verbatim — Cobra's --help/--version handlers must not see them. dir := t.TempDir() argFile := filepath.Join(dir, "argv.txt") writePluginScript(t, dir, "entire-passthrough", argFile, 0) @@ -251,8 +239,7 @@ func TestPluginDispatch_EnvVarsForwarded(t *testing.T) { if !strings.HasPrefix(out, "ENTIRE_CLI_VERSION=") { t.Fatalf("env line missing prefix: %q", out) } - // Just confirm the variable is set to *something* non-empty — value - // depends on build-time linker flags and will be "dev" in tests. + // Value depends on build-time linker flags; just check it's non-empty. if strings.TrimPrefix(out, "ENTIRE_CLI_VERSION=") == "" { t.Errorf("ENTIRE_CLI_VERSION was empty") } @@ -260,10 +247,8 @@ func TestPluginDispatch_EnvVarsForwarded(t *testing.T) { func TestPluginDispatch_AgentProtocolBinarySkipped(t *testing.T) { t.Parallel() - // `entire agent-foo` must not be routed to entire-agent-foo (which is a - // protocol binary, not a passthrough plugin). Cobra should see "agent-foo" - // as an unknown command — the literal `agent` group exists, but - // `agent-foo` is not a subcommand of agent and not a passthrough name. + // `entire-agent-*` is reserved for the protocol — never dispatched as + // a passthrough plugin even when present on PATH. dir := t.TempDir() writePluginScript(t, dir, "entire-agent-foo", filepath.Join(dir, "argv.txt"), 0) @@ -278,4 +263,9 @@ func TestPluginDispatch_AgentProtocolBinarySkipped(t *testing.T) { if _, err := os.Stat(filepath.Join(dir, "argv.txt")); err == nil { t.Error("entire-agent-foo was invoked but must have been skipped") } + // Should fall through to Cobra's unknown-command path, not be eaten silently. + if !strings.Contains(stderr.String(), "unknown command") && + !strings.Contains(stderr.String(), "Invalid usage") { + t.Errorf("expected Cobra unknown-command error, got stderr: %s", stderr.String()) + } } diff --git a/cmd/entire/cli/plugin.go b/cmd/entire/cli/plugin.go index cdf36e40e2..161e55a3a9 100644 --- a/cmd/entire/cli/plugin.go +++ b/cmd/entire/cli/plugin.go @@ -4,13 +4,13 @@ import ( "context" "errors" "fmt" - "io" "os" "os/exec" "path/filepath" "strings" "time" + "github.com/entireio/cli/cmd/entire/cli/agent/external" "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/telemetry" "github.com/entireio/cli/cmd/entire/cli/versioninfo" @@ -28,22 +28,23 @@ const ( ) // MaybeDispatchPlugin returns (true, exitCode) when an external plugin -// handled the invocation, and (false, 0) otherwise (in which case the caller -// should fall through to normal Cobra execution). +// handled the invocation. On launch failure (e.g. missing executable bit) +// returns (true, 1) after printing to stderr. On no-match returns (false, 0) +// so the caller can fall through to Cobra. func MaybeDispatchPlugin(ctx context.Context, rootCmd *cobra.Command, args []string) (handled bool, exitCode int) { binPath, pluginArgs, ok := resolvePlugin(rootCmd, args) if !ok { return false, 0 } pluginName := args[0] - exitCode = runPlugin(ctx, binPath, pluginArgs, os.Stdin, os.Stdout, os.Stderr) + exitCode = runPlugin(ctx, binPath, pluginArgs) maybeTrackPluginInvocation(ctx, pluginName) return true, exitCode } // maybeTrackPluginInvocation fires telemetry only for plugins on the -// official allowlist and only when telemetry is opted in via settings. -// Third-party plugin names are never sent. +// official allowlist. Third-party plugin names are never sent — see +// IsOfficialPlugin for the rationale. func maybeTrackPluginInvocation(ctx context.Context, pluginName string) { if !IsOfficialPlugin(pluginName) { return @@ -58,9 +59,6 @@ func maybeTrackPluginInvocation(ctx context.Context, pluginName string) { telemetry.TrackPluginDetached(pluginName, s.Enabled, versioninfo.Version) } -// resolvePlugin decides whether args should be routed to an external plugin. -// It is split out from MaybeDispatchPlugin so tests can exercise resolution -// without spawning a subprocess. func resolvePlugin(rootCmd *cobra.Command, args []string) (binPath string, pluginArgs []string, ok bool) { if len(args) == 0 { return "", nil, false @@ -83,9 +81,6 @@ func resolvePlugin(rootCmd *cobra.Command, args []string) (binPath string, plugi return binPath, args[1:], true } -// isPluginCandidate filters out args that obviously aren't plugin invocations: -// flags, empty strings, and names that would map onto agent protocol binaries -// or contain path separators. func isPluginCandidate(name string) bool { if name == "" { return false @@ -93,6 +88,7 @@ func isPluginCandidate(name string) bool { if strings.HasPrefix(name, "-") { return false } + // `agent-*` is reserved for the external agent protocol. if strings.HasPrefix(name, "agent-") { return false } @@ -102,43 +98,25 @@ func isPluginCandidate(name string) bool { return true } -// isAgentProtocolBinary returns true when the binary name is reserved for the -// external agent protocol. We check both the literal name and the -// extension-stripped form so a user with `entire-agent-foo.exe` on Windows -// still gets filtered out. +// isAgentProtocolBinary returns true when the binary name is reserved for +// the external agent protocol. Strip Windows extensions first so +// `entire-agent-foo.exe` is also recognized. func isAgentProtocolBinary(binPath string) bool { - base := filepath.Base(binPath) - if strings.HasPrefix(base, agentPluginBinaryPrefix) { - return true - } - if strings.HasPrefix(stripPluginExeExt(base), agentPluginBinaryPrefix) { - return true - } - return false -} - -// stripPluginExeExt mirrors agent/external/discovery.stripExeExt for plugin -// name normalization on Windows. -func stripPluginExeExt(name string) string { - switch strings.ToLower(filepath.Ext(name)) { - case ".exe", ".bat", ".cmd": - return strings.TrimSuffix(name, filepath.Ext(name)) - } - return name + base := external.StripExeExt(filepath.Base(binPath)) + return strings.HasPrefix(base, agentPluginBinaryPrefix) } // runPlugin executes the resolved plugin binary, propagating its exit code. -// On context cancellation the child is sent SIGINT (with a short grace -// window before the runtime falls back to SIGKILL), so plugins get a chance -// to clean up. Terminal signals (Ctrl+C) reach the child directly through -// the foreground process group as well. -func runPlugin(ctx context.Context, binPath string, args []string, stdin io.Reader, stdout, stderr io.Writer) int { +// 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 { cmd := exec.CommandContext(ctx, binPath, args...) cmd.Cancel = func() error { return cmd.Process.Signal(os.Interrupt) } cmd.WaitDelay = 5 * time.Second - cmd.Stdin = stdin - cmd.Stdout = stdout - cmd.Stderr = stderr + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr cmd.Env = append(os.Environ(), "ENTIRE_CLI_VERSION="+versioninfo.Version) if repoRoot, err := paths.WorktreeRoot(ctx); err == nil { cmd.Env = append(cmd.Env, "ENTIRE_REPO_ROOT="+repoRoot) @@ -149,7 +127,9 @@ func runPlugin(ctx context.Context, binPath string, args []string, stdin io.Read if errors.As(err, &exitErr) { return exitErr.ExitCode() } - fmt.Fprintf(stderr, "entire: failed to run plugin %s: %v\n", filepath.Base(binPath), err) + // Prefix with the plugin name so users can tell parent vs child + // errors apart in mixed stderr. + fmt.Fprintf(os.Stderr, "Failed to run plugin %s: %v\n", filepath.Base(binPath), err) return 1 } return 0 diff --git a/cmd/entire/cli/plugin_official.go b/cmd/entire/cli/plugin_official.go index 5ce576726a..e9fbd6bc6d 100644 --- a/cmd/entire/cli/plugin_official.go +++ b/cmd/entire/cli/plugin_official.go @@ -1,28 +1,19 @@ package cli -// Official plugin allowlist. Names listed here may contribute their plugin -// name to telemetry; all other (third-party / user-installed) plugins are -// invoked silently with no event recorded. The reasoning mirrors gh's -// extension policy — third-party plugin names can carry sensitive identifiers -// (project names, vendor names), so we only attribute usage for plugins we -// ship ourselves. -// -// To add a plugin: append its name to officialPlugins. Match must be exact -// and case-sensitive. The corresponding binary is `entire-`. +import "slices" + +// Telemetry is recorded only for plugin names listed here. Third-party +// plugin names can carry sensitive identifiers (project, vendor), so +// everything outside this allowlist is invoked silently — see gh's +// extension-telemetry posture for the reasoning. Match is case-sensitive +// and exact; the binary on disk is `entire-`. // -//nolint:gochecknoglobals // small immutable allowlist +//nolint:gochecknoglobals // package-level allowlist; mutated by tests via snapshot/restore. var officialPlugins = []string{ // Add Entire-shipped plugin names here as they're released. // e.g. "pgr" } -// IsOfficialPlugin reports whether name appears in the hardcoded allowlist. -// Used to decide whether plugin invocation telemetry should record the name. func IsOfficialPlugin(name string) bool { - for _, p := range officialPlugins { - if p == name { - return true - } - } - return false + return slices.Contains(officialPlugins, name) } diff --git a/cmd/entire/cli/plugin_official_test.go b/cmd/entire/cli/plugin_official_test.go index f115e82e71..ae679cc176 100644 --- a/cmd/entire/cli/plugin_official_test.go +++ b/cmd/entire/cli/plugin_official_test.go @@ -3,9 +3,8 @@ package cli import "testing" func TestIsOfficialPlugin(t *testing.T) { - t.Parallel() - // Snapshot and restore the package-level allowlist so this test stays - // independent of which plugins ship by default. + // Snapshot the allowlist so the test is independent of shipped plugins. + // Cannot t.Parallel — mutates package-level state. saved := officialPlugins t.Cleanup(func() { officialPlugins = saved }) @@ -28,15 +27,3 @@ func TestIsOfficialPlugin(t *testing.T) { } } } - -func TestOfficialPlugins_DefaultAllowlist(t *testing.T) { - t.Parallel() - // Sanity check: no third-party plugin name should be officially - // recognized by default. If you are adding an Entire-shipped plugin, - // update this test alongside the registry. - for _, n := range officialPlugins { - if n == "" { - t.Errorf("officialPlugins contains an empty name") - } - } -} diff --git a/cmd/entire/cli/plugin_test.go b/cmd/entire/cli/plugin_test.go index 2d042befcd..bab179f5ac 100644 --- a/cmd/entire/cli/plugin_test.go +++ b/cmd/entire/cli/plugin_test.go @@ -12,8 +12,8 @@ import ( "github.com/spf13/cobra" ) -// writePluginBinary creates a shell script that records its argv to argFile and -// exits with the requested code. Returns the binary's absolute path. +// writePluginBinary writes a shell script that records argv to argFile. +// Skips the calling test on Windows. func writePluginBinary(t *testing.T, dir, name, argFile string, exitCode int) string { t.Helper() if runtime.GOOS == "windows" { @@ -27,7 +27,6 @@ func writePluginBinary(t *testing.T, dir, name, argFile string, exitCode int) st return path } -// withPathDir prepends dir to PATH for the duration of the test. func withPathDir(t *testing.T, dir string) { t.Helper() t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) @@ -79,7 +78,6 @@ func TestResolvePlugin_RejectsAgentPrefix(t *testing.T) { //nolint:paralleltest writePluginBinary(t, dir, "entire-agent-foo", filepath.Join(dir, "args.txt"), 0) withPathDir(t, dir) - // Reserved name pattern — refuse before even hitting PATH. if _, _, ok := resolvePlugin(newTestRoot(), []string{"agent-foo"}); ok { t.Fatal("agent-foo must not resolve as a passthrough plugin") } @@ -120,11 +118,12 @@ func TestResolvePlugin_PathTraversal(t *testing.T) { } } -func TestRunPlugin_ExitCodePropagation(t *testing.T) { //nolint:paralleltest // mutates PATH via t.Setenv +func TestRunPlugin_ExitCodePropagation(t *testing.T) { + t.Parallel() dir := t.TempDir() binPath := writePluginBinary(t, dir, "entire-exit42", filepath.Join(dir, "args.txt"), 42) - code := runPlugin(context.Background(), binPath, []string{"a", "b"}, nil, os.Stderr, os.Stderr) + code := runPlugin(context.Background(), binPath, []string{"a", "b"}) if code != 42 { t.Errorf("exit code: got %d, want 42", code) } diff --git a/cmd/entire/cli/telemetry/detached.go b/cmd/entire/cli/telemetry/detached.go index 71d20d20f7..7a98cdf8a7 100644 --- a/cmd/entire/cli/telemetry/detached.go +++ b/cmd/entire/cli/telemetry/detached.go @@ -109,9 +109,8 @@ func TrackCommandDetached(cmd *cobra.Command, agent string, isEntireEnabled bool } } -// BuildPluginEventPayload constructs the event payload for a plugin invocation. -// The plugin name is embedded as a property; we never record plugin args or -// flags. Returns nil if the payload cannot be built. +// BuildPluginEventPayload deliberately omits plugin args/flags — only the +// allowlisted plugin name is recorded. Returns nil on failure. func BuildPluginEventPayload(pluginName string, isEntireEnabled bool, version string) *EventPayload { if pluginName == "" { return nil @@ -139,9 +138,8 @@ func BuildPluginEventPayload(pluginName string, isEntireEnabled bool, version st } } -// TrackPluginDetached records a plugin invocation event in a detached -// subprocess. Call sites are responsible for restricting invocation to -// allowlisted plugin names so third-party plugin identifiers don't leak. +// TrackPluginDetached records a plugin invocation. Call sites must gate +// on the plugin allowlist — this function does no name filtering itself. func TrackPluginDetached(pluginName string, isEntireEnabled bool, version string) { if os.Getenv("ENTIRE_TELEMETRY_OPTOUT") != "" { return diff --git a/cmd/entire/main.go b/cmd/entire/main.go index f078d70391..7daf0af5a1 100644 --- a/cmd/entire/main.go +++ b/cmd/entire/main.go @@ -33,9 +33,6 @@ func main() { // Create and execute root command rootCmd := cli.NewRootCmd() - // Kubectl-style plugin dispatch: `entire ` → exec `entire-` - // on PATH when isn't a built-in. Built-ins always win; agent - // protocol binaries (entire-agent-*) are skipped here. if handled, code := cli.MaybeDispatchPlugin(ctx, rootCmd, os.Args[1:]); handled { cancel() os.Exit(code) From 61deaab28dc37f8ef23ac4a4b64e2be0c929f1d8 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 4 May 2026 16:06:51 +0200 Subject: [PATCH 05/14] Surface non-executable plugins as launch errors instead of falling through MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit exec.LookPath conflates "not on PATH" with "found but not executable", so an entire- file with mode 0o644 was silently falling through to Cobra's generic unknown-command path — confusing for users who installed a plugin but forgot to chmod +x. resolvePlugin now scans PATH on LookPath miss; if it finds a non-directory file with the right name, returns it so runPlugin reports "Failed to run plugin " with a non-zero exit. This matches what the MaybeDispatchPlugin doc comment already claimed. Coverage: - TestResolvePlugin_NonExecutableSurfacesAsLaunchError (unit) - TestPluginDispatch_NonExecutableReportsLaunchError (integration) Also: the new test added a third "windows" string literal in the cli package, tripping goconst — replaced with the existing windowsGOOS constant. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: b17ee321e0b8 --- .../integration_test/plugin_dispatch_test.go | 28 ++++++++++++++++++ cmd/entire/cli/plugin.go | 29 ++++++++++++++++++- cmd/entire/cli/plugin_test.go | 24 ++++++++++++++- 3 files changed, 79 insertions(+), 2 deletions(-) diff --git a/cmd/entire/cli/integration_test/plugin_dispatch_test.go b/cmd/entire/cli/integration_test/plugin_dispatch_test.go index b1e01f4c02..16863cbdbb 100644 --- a/cmd/entire/cli/integration_test/plugin_dispatch_test.go +++ b/cmd/entire/cli/integration_test/plugin_dispatch_test.go @@ -245,6 +245,34 @@ func TestPluginDispatch_EnvVarsForwarded(t *testing.T) { } } +func TestPluginDispatch_NonExecutableReportsLaunchError(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { + t.Skip("executable bit semantics tested on Unix only") + } + dir := t.TempDir() + // Mode 0o644 — file exists on PATH but cannot be exec'd. The dispatcher + // must report a launch failure rather than silently falling through to + // Cobra's generic unknown-command path. + if err := os.WriteFile(filepath.Join(dir, "entire-noexec"), []byte("#!/bin/sh\nexit 0\n"), 0o644); err != nil { //nolint:gosec // test fixture + t.Fatalf("write plugin: %v", err) + } + + cmd := exec.Command(getTestBinary(), "noexec") + cmd.Env = pathWith(dir) + var stderr bytes.Buffer + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &stderr + + err := cmd.Run() + if err == nil { + t.Fatal("expected non-zero exit for non-executable plugin") + } + if !strings.Contains(stderr.String(), "Failed to run plugin entire-noexec") { + t.Errorf("expected launch-failure message in stderr, got: %s", stderr.String()) + } +} + func TestPluginDispatch_AgentProtocolBinarySkipped(t *testing.T) { t.Parallel() // `entire-agent-*` is reserved for the protocol — never dispatched as diff --git a/cmd/entire/cli/plugin.go b/cmd/entire/cli/plugin.go index 161e55a3a9..fc84ae5ffc 100644 --- a/cmd/entire/cli/plugin.go +++ b/cmd/entire/cli/plugin.go @@ -71,8 +71,16 @@ func resolvePlugin(rootCmd *cobra.Command, args []string) (binPath string, plugi if cmd, _, err := rootCmd.Find(args); err == nil && cmd != rootCmd { return "", nil, false } - binPath, err := exec.LookPath(pluginBinaryPrefix + name) + binName := pluginBinaryPrefix + name + binPath, err := exec.LookPath(binName) if err != nil { + // LookPath conflates "not on PATH" with "found but not executable". + // Distinguish: if a file with this name exists on PATH but isn't + // executable, surface that as a launch error rather than falling + // through to Cobra's generic unknown-command path. + if p, found := findInaccessiblePlugin(binName); found { + return p, args[1:], true + } return "", nil, false } if isAgentProtocolBinary(binPath) { @@ -81,6 +89,25 @@ func resolvePlugin(rootCmd *cobra.Command, args []string) (binPath string, plugi return binPath, args[1:], true } +// findInaccessiblePlugin scans PATH for a non-directory file with the +// given name. Only meaningful after exec.LookPath has already failed — +// indicates the file exists but lacks the executable bit (or the +// equivalent platform-specific accessibility). +func findInaccessiblePlugin(filename string) (string, bool) { + for _, dir := range filepath.SplitList(os.Getenv("PATH")) { + if dir == "" { + continue + } + candidate := filepath.Join(dir, filename) + info, err := os.Stat(candidate) //nolint:gosec // PATH entries are user-trusted; scanning them is the point. + if err != nil || info.IsDir() { + continue + } + return candidate, true + } + return "", false +} + func isPluginCandidate(name string) bool { if name == "" { return false diff --git a/cmd/entire/cli/plugin_test.go b/cmd/entire/cli/plugin_test.go index bab179f5ac..d80aa8da41 100644 --- a/cmd/entire/cli/plugin_test.go +++ b/cmd/entire/cli/plugin_test.go @@ -16,7 +16,7 @@ import ( // Skips the calling test on Windows. func writePluginBinary(t *testing.T, dir, name, argFile string, exitCode int) string { t.Helper() - if runtime.GOOS == "windows" { + if runtime.GOOS == windowsGOOS { t.Skip("plugin shell-script harness only runs on Unix") } path := filepath.Join(dir, name) @@ -109,6 +109,28 @@ func TestResolvePlugin_FlagAsFirstArg(t *testing.T) { } } +func TestResolvePlugin_NonExecutableSurfacesAsLaunchError(t *testing.T) { //nolint:paralleltest // mutates PATH via t.Setenv + if runtime.GOOS == windowsGOOS { + t.Skip("executable bit semantics tested on Unix only") + } + dir := t.TempDir() + // Same script body as writePluginBinary but mode 0o644 (not executable). + path := filepath.Join(dir, "entire-bad") + script := "#!/bin/sh\nexit 0\n" + if err := os.WriteFile(path, []byte(script), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + withPathDir(t, dir) + + got, _, ok := resolvePlugin(newTestRoot(), []string{"bad"}) + if !ok { + t.Fatal("non-executable plugin must surface as a launch failure, not a fall-through") + } + if got != path { + t.Errorf("binPath: got %q, want %q", got, path) + } +} + func TestResolvePlugin_PathTraversal(t *testing.T) { t.Parallel() for _, name := range []string{"../evil", `..\evil`, "foo/bar"} { From 989255fb5ba74f5befb6a8fc2d001382a98676d0 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 4 May 2026 16:07:20 +0200 Subject: [PATCH 06/14] Run version-check post-hook for plugin invocations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plugin commands bypass rootCmd.ExecuteContext, so PersistentPostRun in root.go never runs for them. That dropped versioncheck.CheckAndNotify entirely for plugins — a user-visible regression compared with every built-in top-level command, which surface "a new release of Entire is available" notices through that hook. MaybeDispatchPlugin now invokes CheckAndNotify directly after the plugin returns. Future shared post-run behavior added at the root will need similar wiring here unless we factor it out. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: a0e53aaf1ef1 --- cmd/entire/cli/plugin.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/entire/cli/plugin.go b/cmd/entire/cli/plugin.go index fc84ae5ffc..c658d58eb3 100644 --- a/cmd/entire/cli/plugin.go +++ b/cmd/entire/cli/plugin.go @@ -13,6 +13,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/agent/external" "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/telemetry" + "github.com/entireio/cli/cmd/entire/cli/versioncheck" "github.com/entireio/cli/cmd/entire/cli/versioninfo" "github.com/spf13/cobra" ) @@ -39,6 +40,7 @@ func MaybeDispatchPlugin(ctx context.Context, rootCmd *cobra.Command, args []str pluginName := args[0] exitCode = runPlugin(ctx, binPath, pluginArgs) maybeTrackPluginInvocation(ctx, pluginName) + versioncheck.CheckAndNotify(ctx, os.Stdout, versioninfo.Version) return true, exitCode } From fd4fe2da24369a30bb8a87696cb2f954ef825e62 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 4 May 2026 16:08:35 +0200 Subject: [PATCH 07/14] Gate plugin post-hooks on a successful (exit-0) run Cobra's PersistentPostRun runs only after a successful RunE, so built-in command telemetry and the version-check notice both skip failed/crashing invocations. Plugin telemetry was firing unconditionally after runPlugin, which would inflate usage events with launch failures and make the plugin event stream incomparable with the existing command stream. Wrap both maybeTrackPluginInvocation and versioncheck.CheckAndNotify in an exit-0 guard so plugin post-hooks track Cobra's semantics. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 710ea750b55b --- cmd/entire/cli/plugin.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cmd/entire/cli/plugin.go b/cmd/entire/cli/plugin.go index c658d58eb3..f6087129c5 100644 --- a/cmd/entire/cli/plugin.go +++ b/cmd/entire/cli/plugin.go @@ -32,6 +32,9 @@ const ( // handled the invocation. On launch failure (e.g. missing executable bit) // returns (true, 1) after printing to stderr. On no-match returns (false, 0) // so the caller can fall through to Cobra. +// +// Telemetry and the version-check notice mirror Cobra's PersistentPostRun +// behavior for built-ins: both fire only on a successful (exit-0) run. func MaybeDispatchPlugin(ctx context.Context, rootCmd *cobra.Command, args []string) (handled bool, exitCode int) { binPath, pluginArgs, ok := resolvePlugin(rootCmd, args) if !ok { @@ -39,8 +42,10 @@ func MaybeDispatchPlugin(ctx context.Context, rootCmd *cobra.Command, args []str } pluginName := args[0] exitCode = runPlugin(ctx, binPath, pluginArgs) - maybeTrackPluginInvocation(ctx, pluginName) - versioncheck.CheckAndNotify(ctx, os.Stdout, versioninfo.Version) + if exitCode == 0 { + maybeTrackPluginInvocation(ctx, pluginName) + versioncheck.CheckAndNotify(ctx, os.Stdout, versioninfo.Version) + } return true, exitCode } From 68a429a4e53a36866479b44f76419f48aa8194d2 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 4 May 2026 16:46:38 +0200 Subject: [PATCH 08/14] Document plugin dispatch architecture Adds docs/architecture/plugin-dispatch.md covering the kubectl-style passthrough model: discovery rules, plugin author contract (env vars, exit codes, signal handling, Windows specifics), telemetry posture (allowlist + reasoning), and a side-by-side comparison with the agent protocol so the two PATH-naming conventions (entire- vs entire-agent-) are unambiguous. Also points the project CLAUDE.md at the new doc so it's reachable when navigating the codebase. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 4efc7b48f24c --- CLAUDE.md | 2 +- docs/architecture/plugin-dispatch.md | 99 ++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 docs/architecture/plugin-dispatch.md diff --git a/CLAUDE.md b/CLAUDE.md index f397e08a41..909bc233b1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -10,7 +10,7 @@ This repo contains the CLI for Entire. ### Commands (`cmd/`) -- `entire/`: Main CLI entry point +- `entire/`: Main CLI entry point. Also home to kubectl-style plugin dispatch (`entire ` → `entire-` on PATH) — see [Plugin Dispatch](docs/architecture/plugin-dispatch.md). - `entire/cli`: CLI utilities and helpers (Cobra commands, helpers, group roots) - `entire/cli/commands`: actual command implementations - `entire/cli/agent`: agent implementations (Claude Code, Gemini CLI, OpenCode, Cursor, Factory AI Droid, Copilot CLI) - see [Agent Integration Checklist](docs/architecture/agent-integration-checklist.md) and [Agent Implementation Guide](docs/architecture/agent-guide.md) diff --git a/docs/architecture/plugin-dispatch.md b/docs/architecture/plugin-dispatch.md new file mode 100644 index 0000000000..006d09420e --- /dev/null +++ b/docs/architecture/plugin-dispatch.md @@ -0,0 +1,99 @@ +# Plugin Dispatch + +## Overview + +The Entire CLI supports kubectl-style plugins — standalone binaries on `$PATH` that extend the CLI without modifying the main repository. When the user invokes `entire ` and `` isn't a built-in subcommand, the CLI looks for an `entire-` binary on `$PATH` and execs it with the remaining arguments. Stdio passes through, exit codes propagate, and the parent CLI does no further processing of the plugin's output. + +This is **not** the same mechanism as the [external agent protocol](external-agent-protocol.md). Plugins have no protocol, no JSON contract, no lifecycle hooks. Use the agent protocol when you need checkpoint integration; use plugins for everything else. + +## Discovery + +The CLI does not scan `$PATH` at startup. Resolution is lazy: when `os.Args[1]` doesn't match a built-in subcommand, the CLI calls `exec.LookPath("entire-" + os.Args[1])`. If a binary is found and executable, dispatch happens before Cobra parses arguments. + +Resolution rules, in order: + +1. **Built-ins win.** If the first argument matches a Cobra subcommand (or one of its aliases), the plugin is never considered. +2. **Reserved names are skipped.** Names beginning with `agent-` are reserved for the [agent protocol](external-agent-protocol.md). The dispatcher refuses to invoke them as plugins. +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 dispatcher reports `Failed to run plugin entire-` with exit code 1, rather than falling through to Cobra's "unknown command" path. + +## Environment + +Each plugin invocation receives: + +| Variable | Description | +|---|---| +| `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. | + +Plus the parent's full environment. The working directory is **not** changed — plugins run in the user's current directory, the same as any other shell command. + +## Plugin Author Contract + +Plugins are arbitrary executables. No SDK, no protocol, no manifest. The contract: + +- **Stdio is the parent's terminal.** Stdin, stdout, and stderr are connected directly. Plugins can prompt interactively, stream output, and behave like any other CLI tool. +- **Exit codes propagate verbatim.** The parent `entire` exits with the plugin's exit code. +- **Signals reach the plugin.** Terminal signals (Ctrl+C) reach the plugin directly via the foreground process group. If the parent's context is cancelled (e.g. via `signal.Notify` plumbing), the plugin receives `SIGINT` with a 5-second grace before the runtime falls back to `SIGKILL`. Plugins that need clean shutdown should trap `SIGINT`. +- **Arguments after the plugin name pass through verbatim.** `entire pgr --help foo` invokes `entire-pgr` with argv `["--help", "foo"]`. Cobra's flag parsing does not run. +- **Windows.** On Windows, `exec.LookPath` resolves `.exe`, `.bat`, and `.cmd` extensions automatically. The "found but not executable" path is Unix-only — Windows treats extension match as the only correctness signal. + +## What Plugins Do Not Get + +- **No checkpoint integration.** Plugin file modifications are not tracked in checkpoints. Plugins do not appear in `entire activity`. If a plugin needs to participate in the session/checkpoint lifecycle, it must use the [agent protocol](external-agent-protocol.md) instead. +- **No transcript recording.** Plugin stdio is not captured. +- **No hook installation.** Plugins cannot register git hooks or agent hooks via the dispatcher. They are free to install their own, but `entire` does not coordinate. +- **No automatic update checks for the plugin itself.** The CLI runs `versioncheck.CheckAndNotify` for the parent CLI's version, not the plugin's. Plugin authors should handle their own update notifications. + +## Telemetry + +Plugin invocations are tracked only for plugins on a hardcoded allowlist (`officialPlugins` in `cmd/entire/cli/plugin_official.go`). Third-party plugin names are **never** sent — even with telemetry opted in. The reasoning matches gh's extension-telemetry posture: arbitrary plugin names can carry sensitive identifiers (project names, vendor names), and the safest default is silence. + +When an allowlisted plugin runs successfully, the CLI emits a `cli_plugin_executed` event with: + +- `plugin` — the plugin name +- `command` — `entire ` +- `cli_version`, `os`, `arch`, `isEntireEnabled` + +Plugin args and flags are deliberately **not** recorded. + +Telemetry fires only when: + +1. The plugin name is in `officialPlugins`. +2. `entire` settings have `Telemetry: true`. +3. `ENTIRE_TELEMETRY_OPTOUT` is unset. +4. The plugin exited with status 0. Failed/crashing invocations are not tracked, matching Cobra's `PersistentPostRun` semantics for built-in commands. + +## Adding an Entire-Shipped Plugin to the Allowlist + +When publishing an Entire-owned plugin (e.g. `entire-pgr`): + +1. Append the plugin name to `officialPlugins` in `cmd/entire/cli/plugin_official.go`. +2. Match must be exact and case-sensitive — the binary on disk is `entire-`. +3. Update or add tests if the plugin has unusual telemetry shape. + +Once allowlisted, `cli_plugin_executed` events for that plugin will flow through the existing PostHog pipeline. + +## Comparison with the Agent Protocol + +| | Plugin Dispatch | [Agent Protocol](external-agent-protocol.md) | +|---|---|---| +| **Binary name pattern** | `entire-` | `entire-agent-` | +| **Discovery** | Lazy on invocation | Eager scan during init (or lazy for hooks) | +| **Communication** | Process exec; stdio passthrough | Subcommand protocol; JSON over stdin/stdout | +| **Versioning** | None | `ENTIRE_PROTOCOL_VERSION` envelope | +| **Lifecycle integration** | None | Full (sessions, checkpoints, hooks, transcripts) | +| **Telemetry** | Allowlist only | Standard agent telemetry | +| **Working directory** | User's cwd | Repository root | +| **Use when** | You want to add a CLI verb | You want an AI agent to participate in checkpointed sessions | + +## Implementation + +The dispatcher lives in `cmd/entire/cli/plugin.go`. The entry point is `MaybeDispatchPlugin(ctx, rootCmd, args)`, called from `cmd/entire/main.go` before `rootCmd.ExecuteContext`. Returns `(handled bool, exitCode int)` — when `handled` is true, the caller exits with `exitCode`; otherwise it falls through to normal Cobra execution. + +Key files: + +- `cmd/entire/cli/plugin.go` — dispatcher, `resolvePlugin`, `runPlugin` +- `cmd/entire/cli/plugin_official.go` — `officialPlugins` allowlist, `IsOfficialPlugin` +- `cmd/entire/cli/telemetry/detached.go` — `BuildPluginEventPayload`, `TrackPluginDetached` +- `cmd/entire/cli/integration_test/plugin_dispatch_test.go` — end-to-end coverage of the early-dispatch path From 114196ce13549b3ce4bed02115ecc48400c84a4b Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 4 May 2026 17:09:59 +0200 Subject: [PATCH 09/14] Correct agent-discovery row in plugin/agent comparison table The previous wording ("Eager scan during init (or lazy for hooks)") overstates how external agent discovery actually works. The implementation is conditional and command-specific: - Normal discovery is gated by the external_agents setting (agent/external/discovery.go). - Several commands trigger it lazily at runtime: attach, resume, rewind, hooks_cmd. - Setup flows bypass the gate entirely via DiscoverAndRegisterAlways. Reword to "Lazy at command entry, gated by external_agents setting (setup flows bypass the gate via DiscoverAndRegisterAlways)" so plugin authors comparing the two systems get an accurate mental model. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 7e4787f2290b --- docs/architecture/plugin-dispatch.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/plugin-dispatch.md b/docs/architecture/plugin-dispatch.md index 006d09420e..b5b9dd5edb 100644 --- a/docs/architecture/plugin-dispatch.md +++ b/docs/architecture/plugin-dispatch.md @@ -79,7 +79,7 @@ Once allowlisted, `cli_plugin_executed` events for that plugin will flow through | | Plugin Dispatch | [Agent Protocol](external-agent-protocol.md) | |---|---|---| | **Binary name pattern** | `entire-` | `entire-agent-` | -| **Discovery** | Lazy on invocation | Eager scan during init (or lazy for hooks) | +| **Discovery** | Lazy, on first non-built-in arg | Lazy at command entry, gated by `external_agents` setting (setup flows bypass the gate via `DiscoverAndRegisterAlways`) | | **Communication** | Process exec; stdio passthrough | Subcommand protocol; JSON over stdin/stdout | | **Versioning** | None | `ENTIRE_PROTOCOL_VERSION` envelope | | **Lifecycle integration** | None | Full (sessions, checkpoints, hooks, transcripts) | From f92469853c2fb126d75ac3a38169f5ea63f73a7b Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 4 May 2026 17:10:12 +0200 Subject: [PATCH 10/14] Assert ENTIRE_REPO_ROOT propagation in plugin env-vars test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The plugin-dispatch architecture doc promotes ENTIRE_REPO_ROOT to part of the plugin contract alongside ENTIRE_CLI_VERSION, but coverage only asserted the latter. A regression in repo-root propagation would contradict the documented behavior without failing tests. Spawn the parent CLI from inside a freshly-init'd git repo (via testutil.InitRepo), have the plugin echo both env vars to a file, and assert ENTIRE_REPO_ROOT matches the resolved repo path exactly. Symlinks are evaluated up front to match what paths.WorktreeRoot returns on macOS (/var → /private/var), avoiding flakes. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: a031a2305924 --- .../integration_test/plugin_dispatch_test.go | 53 +++++++++++++++---- 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/cmd/entire/cli/integration_test/plugin_dispatch_test.go b/cmd/entire/cli/integration_test/plugin_dispatch_test.go index 16863cbdbb..8251493c1a 100644 --- a/cmd/entire/cli/integration_test/plugin_dispatch_test.go +++ b/cmd/entire/cli/integration_test/plugin_dispatch_test.go @@ -12,6 +12,8 @@ import ( "runtime" "strings" "testing" + + "github.com/entireio/cli/cmd/entire/cli/testutil" ) // Integration tests for the early-dispatch path in cmd/entire/main.go. @@ -213,18 +215,32 @@ func TestPluginDispatch_EnvVarsForwarded(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("plugin shell-script harness only runs on Unix") } - dir := t.TempDir() - envFile := filepath.Join(dir, "env.txt") + // Spawn the parent CLI from inside a real git repo so it can resolve + // the repo root and forward ENTIRE_REPO_ROOT. testutil.InitRepo + // configures user.name/email and disables GPG signing. + repoDir := t.TempDir() + resolvedRepo, err := filepath.EvalSymlinks(repoDir) + if err != nil { + t.Fatalf("eval symlinks: %v", err) + } + testutil.InitRepo(t, resolvedRepo) + + pluginDir := t.TempDir() + envFile := filepath.Join(pluginDir, "env.txt") body := fmt.Sprintf( - "#!/bin/sh\necho \"ENTIRE_CLI_VERSION=$ENTIRE_CLI_VERSION\" > %q\nexit 0\n", + "#!/bin/sh\n{\n"+ + " echo \"ENTIRE_CLI_VERSION=$ENTIRE_CLI_VERSION\"\n"+ + " echo \"ENTIRE_REPO_ROOT=$ENTIRE_REPO_ROOT\"\n"+ + "} > %q\nexit 0\n", envFile, ) - if err := os.WriteFile(filepath.Join(dir, "entire-envcheck"), []byte(body), 0o755); err != nil { //nolint:gosec // test fixture + if err := os.WriteFile(filepath.Join(pluginDir, "entire-envcheck"), []byte(body), 0o755); err != nil { //nolint:gosec // test fixture t.Fatalf("write plugin: %v", err) } cmd := exec.Command(getTestBinary(), "envcheck") - cmd.Env = pathWith(dir) + cmd.Env = pathWith(pluginDir) + cmd.Dir = resolvedRepo cmd.Stdout = &bytes.Buffer{} cmd.Stderr = &bytes.Buffer{} if err := cmd.Run(); err != nil { @@ -235,14 +251,31 @@ func TestPluginDispatch_EnvVarsForwarded(t *testing.T) { if err != nil { t.Fatalf("read env file: %v", err) } - out := strings.TrimSpace(string(got)) - if !strings.HasPrefix(out, "ENTIRE_CLI_VERSION=") { - t.Fatalf("env line missing prefix: %q", out) - } + envVars := parseEnvLines(t, string(got)) + // Value depends on build-time linker flags; just check it's non-empty. - if strings.TrimPrefix(out, "ENTIRE_CLI_VERSION=") == "" { + if v := envVars["ENTIRE_CLI_VERSION"]; v == "" { t.Errorf("ENTIRE_CLI_VERSION was empty") } + if got, want := envVars["ENTIRE_REPO_ROOT"], resolvedRepo; got != want { + t.Errorf("ENTIRE_REPO_ROOT = %q, want %q", got, want) + } +} + +// parseEnvLines splits "KEY=value" lines into a map. Missing keys map +// to empty strings. +func parseEnvLines(t *testing.T, contents string) map[string]string { + t.Helper() + m := map[string]string{} + for _, line := range strings.Split(strings.TrimRight(contents, "\n"), "\n") { + k, v, ok := strings.Cut(line, "=") + if !ok { + t.Errorf("malformed env line: %q", line) + continue + } + m[k] = v + } + return m } func TestPluginDispatch_NonExecutableReportsLaunchError(t *testing.T) { From 5d22d450e6715a7c9c08be87521981f9a6b69743 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 4 May 2026 17:42:36 +0200 Subject: [PATCH 11/14] =?UTF-8?q?Rename=20"plugin=20dispatch"=20=E2=86=92?= =?UTF-8?q?=20"external=20commands"=20to=20avoid=20collision?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The user-facing "dispatch" terminology collides with the existing top-level `entire dispatch` command, which causes confusion when discussing the kubectl-style PATH-resolution feature. Sweep: - Doc: docs/architecture/plugin-dispatch.md → external-commands.md, title "External Commands", prose updated throughout. Comparison table header now reads "External Commands" vs "Agent Protocol". - API: MaybeDispatchPlugin → MaybeRunPlugin (in plugin.go and main.go). - Tests: TestPluginDispatch_* → TestExternalCommand_* across both integration test files; files renamed to external_command_test.go and external_command_signal_unix_test.go. - CLAUDE.md cross-link updated. Internal nouns (officialPlugins, runPlugin, resolvePlugin, IsOfficialPlugin, plugin.go) are kept — kubectl uses "plugin" internally too, and the conflict was only with the verb. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 63a22badde7c --- CLAUDE.md | 2 +- ...o => external_command_signal_unix_test.go} | 2 +- ...patch_test.go => external_command_test.go} | 26 ++--- cmd/entire/cli/plugin.go | 21 ++-- cmd/entire/cli/plugin_test.go | 2 +- cmd/entire/main.go | 2 +- docs/architecture/external-commands.md | 99 +++++++++++++++++++ docs/architecture/plugin-dispatch.md | 99 ------------------- 8 files changed, 127 insertions(+), 126 deletions(-) rename cmd/entire/cli/integration_test/{plugin_dispatch_signal_unix_test.go => external_command_signal_unix_test.go} (97%) rename cmd/entire/cli/integration_test/{plugin_dispatch_test.go => external_command_test.go} (92%) create mode 100644 docs/architecture/external-commands.md delete mode 100644 docs/architecture/plugin-dispatch.md diff --git a/CLAUDE.md b/CLAUDE.md index 909bc233b1..b972baeef7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -10,7 +10,7 @@ This repo contains the CLI for Entire. ### Commands (`cmd/`) -- `entire/`: Main CLI entry point. Also home to kubectl-style plugin dispatch (`entire ` → `entire-` on PATH) — see [Plugin Dispatch](docs/architecture/plugin-dispatch.md). +- `entire/`: Main CLI entry point. Also home to kubectl-style external-command resolution (`entire ` → `entire-` on PATH) — see [External Commands](docs/architecture/external-commands.md). - `entire/cli`: CLI utilities and helpers (Cobra commands, helpers, group roots) - `entire/cli/commands`: actual command implementations - `entire/cli/agent`: agent implementations (Claude Code, Gemini CLI, OpenCode, Cursor, Factory AI Droid, Copilot CLI) - see [Agent Integration Checklist](docs/architecture/agent-integration-checklist.md) and [Agent Implementation Guide](docs/architecture/agent-guide.md) diff --git a/cmd/entire/cli/integration_test/plugin_dispatch_signal_unix_test.go b/cmd/entire/cli/integration_test/external_command_signal_unix_test.go similarity index 97% rename from cmd/entire/cli/integration_test/plugin_dispatch_signal_unix_test.go rename to cmd/entire/cli/integration_test/external_command_signal_unix_test.go index 4e9f8fe285..71b7305444 100644 --- a/cmd/entire/cli/integration_test/plugin_dispatch_signal_unix_test.go +++ b/cmd/entire/cli/integration_test/external_command_signal_unix_test.go @@ -16,7 +16,7 @@ import ( // SIGINT to the parent must reach the plugin so it can clean up — not // just be SIGKILL'd by the runtime. Guards both signal paths: terminal // (via process group) and parent's context-cancel handler. -func TestPluginDispatch_SigintReachesPlugin(t *testing.T) { +func TestExternalCommand_SigintReachesPlugin(t *testing.T) { t.Parallel() dir := t.TempDir() signalFile := filepath.Join(dir, "got-sigint.txt") diff --git a/cmd/entire/cli/integration_test/plugin_dispatch_test.go b/cmd/entire/cli/integration_test/external_command_test.go similarity index 92% rename from cmd/entire/cli/integration_test/plugin_dispatch_test.go rename to cmd/entire/cli/integration_test/external_command_test.go index 8251493c1a..0de2e81648 100644 --- a/cmd/entire/cli/integration_test/plugin_dispatch_test.go +++ b/cmd/entire/cli/integration_test/external_command_test.go @@ -16,10 +16,10 @@ import ( "github.com/entireio/cli/cmd/entire/cli/testutil" ) -// Integration tests for the early-dispatch path in cmd/entire/main.go. -// They build and exec the real binary so the wiring (pre-Cobra dispatch, -// exit-code propagation, stdio passthrough, signal handling) is exercised -// end-to-end — unit tests in cmd/entire/cli/plugin_test.go can't. +// Integration tests for external-command resolution in cmd/entire/main.go. +// They build and exec the real binary so the pre-Cobra routing (exit-code +// propagation, stdio passthrough, signal handling) is exercised end-to-end +// — unit tests in cmd/entire/cli/plugin_test.go can't. // writePluginScript writes a shell script that records argv and exits // with exitCode. Skips the calling test on Windows. @@ -55,7 +55,7 @@ func pathWith(dir string) []string { return append(env, "PATH="+dir) } -func TestPluginDispatch_HappyPath(t *testing.T) { +func TestExternalCommand_HappyPath(t *testing.T) { t.Parallel() dir := t.TempDir() argFile := filepath.Join(dir, "argv.txt") @@ -85,7 +85,7 @@ func TestPluginDispatch_HappyPath(t *testing.T) { } } -func TestPluginDispatch_ExitCodePropagation(t *testing.T) { +func TestExternalCommand_ExitCodePropagation(t *testing.T) { t.Parallel() dir := t.TempDir() writePluginScript(t, dir, "entire-failing", filepath.Join(dir, "argv.txt"), 42) @@ -108,7 +108,7 @@ func TestPluginDispatch_ExitCodePropagation(t *testing.T) { } } -func TestPluginDispatch_BuiltinWins(t *testing.T) { +func TestExternalCommand_BuiltinWins(t *testing.T) { t.Parallel() dir := t.TempDir() // If the shadowing plugin ran, the parent's exit code would be 99 @@ -132,7 +132,7 @@ func TestPluginDispatch_BuiltinWins(t *testing.T) { } } -func TestPluginDispatch_PluginNotFound(t *testing.T) { +func TestExternalCommand_PluginNotFound(t *testing.T) { t.Parallel() // PATH deliberately points at an empty dir so no plugin can resolve. dir := t.TempDir() @@ -154,7 +154,7 @@ func TestPluginDispatch_PluginNotFound(t *testing.T) { } } -func TestPluginDispatch_FlagAfterPluginNameNotEatenByCobra(t *testing.T) { +func TestExternalCommand_FlagAfterPluginNameNotEatenByCobra(t *testing.T) { t.Parallel() // Once we're routing to a plugin, flag-shaped args must reach the // child verbatim — Cobra's --help/--version handlers must not see them. @@ -180,7 +180,7 @@ func TestPluginDispatch_FlagAfterPluginNameNotEatenByCobra(t *testing.T) { } } -func TestPluginDispatch_StdinPassthrough(t *testing.T) { +func TestExternalCommand_StdinPassthrough(t *testing.T) { t.Parallel() if runtime.GOOS == "windows" { t.Skip("plugin shell-script harness only runs on Unix") @@ -210,7 +210,7 @@ func TestPluginDispatch_StdinPassthrough(t *testing.T) { } } -func TestPluginDispatch_EnvVarsForwarded(t *testing.T) { +func TestExternalCommand_EnvVarsForwarded(t *testing.T) { t.Parallel() if runtime.GOOS == "windows" { t.Skip("plugin shell-script harness only runs on Unix") @@ -278,7 +278,7 @@ func parseEnvLines(t *testing.T, contents string) map[string]string { return m } -func TestPluginDispatch_NonExecutableReportsLaunchError(t *testing.T) { +func TestExternalCommand_NonExecutableReportsLaunchError(t *testing.T) { t.Parallel() if runtime.GOOS == "windows" { t.Skip("executable bit semantics tested on Unix only") @@ -306,7 +306,7 @@ func TestPluginDispatch_NonExecutableReportsLaunchError(t *testing.T) { } } -func TestPluginDispatch_AgentProtocolBinarySkipped(t *testing.T) { +func TestExternalCommand_AgentProtocolBinarySkipped(t *testing.T) { t.Parallel() // `entire-agent-*` is reserved for the protocol — never dispatched as // a passthrough plugin even when present on PATH. diff --git a/cmd/entire/cli/plugin.go b/cmd/entire/cli/plugin.go index f6087129c5..4a3c605bd6 100644 --- a/cmd/entire/cli/plugin.go +++ b/cmd/entire/cli/plugin.go @@ -18,24 +18,25 @@ import ( "github.com/spf13/cobra" ) -// Plugin dispatch — kubectl-style. When the user invokes `entire ` and -// isn't a built-in subcommand, look for an `entire-` binary on -// PATH and exec it with the remaining args. Stdio and exit codes pass -// through. Binaries matching the agent protocol prefix are ignored here — -// they're handled by the external agent registry. +// External-command resolution, kubectl-style. When the user invokes +// `entire ` and isn't a built-in subcommand, look for an +// `entire-` binary on PATH and exec it with the remaining args. +// Stdio and exit codes pass through. Binaries matching the agent +// protocol prefix are reserved for the external agent registry and +// skipped here. const ( pluginBinaryPrefix = "entire-" agentPluginBinaryPrefix = "entire-agent-" ) -// MaybeDispatchPlugin returns (true, exitCode) when an external plugin -// handled the invocation. On launch failure (e.g. missing executable bit) -// returns (true, 1) after printing to stderr. On no-match returns (false, 0) -// so the caller can fall through to Cobra. +// MaybeRunPlugin returns (true, exitCode) when an external command was +// resolved and run. On launch failure (e.g. missing executable bit) +// returns (true, 1) after printing to stderr. On no-match returns +// (false, 0) so the caller can fall through to Cobra. // // Telemetry and the version-check notice mirror Cobra's PersistentPostRun // behavior for built-ins: both fire only on a successful (exit-0) run. -func MaybeDispatchPlugin(ctx context.Context, rootCmd *cobra.Command, args []string) (handled bool, exitCode int) { +func MaybeRunPlugin(ctx context.Context, rootCmd *cobra.Command, args []string) (handled bool, exitCode int) { binPath, pluginArgs, ok := resolvePlugin(rootCmd, args) if !ok { return false, 0 diff --git a/cmd/entire/cli/plugin_test.go b/cmd/entire/cli/plugin_test.go index d80aa8da41..defa3bca70 100644 --- a/cmd/entire/cli/plugin_test.go +++ b/cmd/entire/cli/plugin_test.go @@ -105,7 +105,7 @@ func TestIsAgentProtocolBinary(t *testing.T) { func TestResolvePlugin_FlagAsFirstArg(t *testing.T) { t.Parallel() if _, _, ok := resolvePlugin(newTestRoot(), []string{"--help"}); ok { - t.Fatal("flags must not trigger plugin dispatch") + t.Fatal("flags must not trigger external-command lookup") } } diff --git a/cmd/entire/main.go b/cmd/entire/main.go index 7daf0af5a1..b7e5d3c4f3 100644 --- a/cmd/entire/main.go +++ b/cmd/entire/main.go @@ -33,7 +33,7 @@ func main() { // Create and execute root command rootCmd := cli.NewRootCmd() - if handled, code := cli.MaybeDispatchPlugin(ctx, rootCmd, os.Args[1:]); handled { + if handled, code := cli.MaybeRunPlugin(ctx, rootCmd, os.Args[1:]); handled { cancel() os.Exit(code) } diff --git a/docs/architecture/external-commands.md b/docs/architecture/external-commands.md new file mode 100644 index 0000000000..dcbcd8b25c --- /dev/null +++ b/docs/architecture/external-commands.md @@ -0,0 +1,99 @@ +# External Commands + +## Overview + +The Entire CLI supports kubectl-style external commands — standalone binaries on `$PATH` that extend the CLI without modifying the main repository. When the user invokes `entire ` and `` isn't a built-in subcommand, the CLI looks for an `entire-` binary on `$PATH` and execs it with the remaining arguments. Stdio passes through, exit codes propagate, and the parent CLI does no further processing of the child's output. + +This is **not** the same mechanism as the [external agent protocol](external-agent-protocol.md). External commands have no protocol, no JSON contract, no lifecycle hooks. Use the agent protocol when you need checkpoint integration; use external commands for everything else. + +## Resolution + +The CLI does not scan `$PATH` at startup. Resolution is lazy: when `os.Args[1]` doesn't match a built-in subcommand, the CLI calls `exec.LookPath("entire-" + os.Args[1])`. If a binary is found and executable, it runs before Cobra parses arguments. + +Rules, in order: + +1. **Built-ins win.** If the first argument matches a Cobra subcommand (or one of its aliases), the external command is never considered. +2. **Reserved names are skipped.** Names beginning with `agent-` are reserved for the [agent protocol](external-agent-protocol.md). The resolver refuses to invoke them as external commands. +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. + +## Environment + +Each external-command invocation receives: + +| Variable | Description | +|---|---| +| `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. | + +Plus the parent's full environment. The working directory is **not** changed — external commands run in the user's current directory, the same as any other shell command. + +## Author Contract + +External commands are arbitrary executables. No SDK, no protocol, no manifest. The contract: + +- **Stdio is the parent's terminal.** Stdin, stdout, and stderr are connected directly. The command can prompt interactively, stream output, and behave like any other CLI tool. +- **Exit codes propagate verbatim.** The parent `entire` exits with the child's exit code. +- **Signals reach the child.** Terminal signals (Ctrl+C) reach the child directly via the foreground process group. If the parent's context is cancelled (e.g. via `signal.Notify` plumbing), the child receives `SIGINT` with a 5-second grace before the runtime falls back to `SIGKILL`. Commands that need clean shutdown should trap `SIGINT`. +- **Arguments after the command name pass through verbatim.** `entire pgr --help foo` invokes `entire-pgr` with argv `["--help", "foo"]`. Cobra's flag parsing does not run. +- **Windows.** On Windows, `exec.LookPath` resolves `.exe`, `.bat`, and `.cmd` extensions automatically. The "found but not executable" path is Unix-only — Windows treats extension match as the only correctness signal. + +## What External Commands Do Not Get + +- **No checkpoint integration.** File modifications are not tracked in checkpoints. External commands do not appear in `entire activity`. If a tool needs to participate in the session/checkpoint lifecycle, it must use the [agent protocol](external-agent-protocol.md) instead. +- **No transcript recording.** External-command stdio is not captured. +- **No hook installation.** External commands cannot register git hooks or agent hooks via the resolver. They are free to install their own, but `entire` does not coordinate. +- **No automatic update checks for the command itself.** The CLI runs `versioncheck.CheckAndNotify` for the parent CLI's version, not the child's. Authors should handle their own update notifications. + +## Telemetry + +External-command invocations are tracked only for names on a hardcoded allowlist (`officialPlugins` in `cmd/entire/cli/plugin_official.go`). Third-party command names are **never** sent — even with telemetry opted in. The reasoning matches gh's extension-telemetry posture: arbitrary command names can carry sensitive identifiers (project names, vendor names), and the safest default is silence. + +When an allowlisted command runs successfully, the CLI emits a `cli_plugin_executed` event with: + +- `plugin` — the command name +- `command` — `entire ` +- `cli_version`, `os`, `arch`, `isEntireEnabled` + +Args and flags are deliberately **not** recorded. + +Telemetry fires only when: + +1. The command name is in `officialPlugins`. +2. `entire` settings have `Telemetry: true`. +3. `ENTIRE_TELEMETRY_OPTOUT` is unset. +4. The command exited with status 0. Failed/crashing invocations are not tracked, matching Cobra's `PersistentPostRun` semantics for built-in commands. + +## Adding an Entire-Shipped Command to the Allowlist + +When publishing an Entire-owned external command (e.g. `entire-pgr`): + +1. Append the command name to `officialPlugins` in `cmd/entire/cli/plugin_official.go`. +2. Match must be exact and case-sensitive — the binary on disk is `entire-`. +3. Update or add tests if the command has unusual telemetry shape. + +Once allowlisted, `cli_plugin_executed` events for that command will flow through the existing PostHog pipeline. + +## Comparison with the Agent Protocol + +| | External Commands | [Agent Protocol](external-agent-protocol.md) | +|---|---|---| +| **Binary name pattern** | `entire-` | `entire-agent-` | +| **Discovery** | Lazy, on first non-built-in arg | Lazy at command entry, gated by `external_agents` setting (setup flows bypass the gate via `DiscoverAndRegisterAlways`) | +| **Communication** | Process exec; stdio passthrough | Subcommand protocol; JSON over stdin/stdout | +| **Versioning** | None | `ENTIRE_PROTOCOL_VERSION` envelope | +| **Lifecycle integration** | None | Full (sessions, checkpoints, hooks, transcripts) | +| **Telemetry** | Allowlist only | Standard agent telemetry | +| **Working directory** | User's cwd | Repository root | +| **Use when** | You want to add a CLI verb | You want an AI agent to participate in checkpointed sessions | + +## Implementation + +The resolver lives in `cmd/entire/cli/plugin.go`. The entry point is `MaybeRunPlugin(ctx, rootCmd, args)`, called from `cmd/entire/main.go` before `rootCmd.ExecuteContext`. Returns `(handled bool, exitCode int)` — when `handled` is true, the caller exits with `exitCode`; otherwise it falls through to normal Cobra execution. + +Key files: + +- `cmd/entire/cli/plugin.go` — entry point, `resolvePlugin`, `runPlugin` +- `cmd/entire/cli/plugin_official.go` — `officialPlugins` allowlist, `IsOfficialPlugin` +- `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 diff --git a/docs/architecture/plugin-dispatch.md b/docs/architecture/plugin-dispatch.md deleted file mode 100644 index b5b9dd5edb..0000000000 --- a/docs/architecture/plugin-dispatch.md +++ /dev/null @@ -1,99 +0,0 @@ -# Plugin Dispatch - -## Overview - -The Entire CLI supports kubectl-style plugins — standalone binaries on `$PATH` that extend the CLI without modifying the main repository. When the user invokes `entire ` and `` isn't a built-in subcommand, the CLI looks for an `entire-` binary on `$PATH` and execs it with the remaining arguments. Stdio passes through, exit codes propagate, and the parent CLI does no further processing of the plugin's output. - -This is **not** the same mechanism as the [external agent protocol](external-agent-protocol.md). Plugins have no protocol, no JSON contract, no lifecycle hooks. Use the agent protocol when you need checkpoint integration; use plugins for everything else. - -## Discovery - -The CLI does not scan `$PATH` at startup. Resolution is lazy: when `os.Args[1]` doesn't match a built-in subcommand, the CLI calls `exec.LookPath("entire-" + os.Args[1])`. If a binary is found and executable, dispatch happens before Cobra parses arguments. - -Resolution rules, in order: - -1. **Built-ins win.** If the first argument matches a Cobra subcommand (or one of its aliases), the plugin is never considered. -2. **Reserved names are skipped.** Names beginning with `agent-` are reserved for the [agent protocol](external-agent-protocol.md). The dispatcher refuses to invoke them as plugins. -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 dispatcher reports `Failed to run plugin entire-` with exit code 1, rather than falling through to Cobra's "unknown command" path. - -## Environment - -Each plugin invocation receives: - -| Variable | Description | -|---|---| -| `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. | - -Plus the parent's full environment. The working directory is **not** changed — plugins run in the user's current directory, the same as any other shell command. - -## Plugin Author Contract - -Plugins are arbitrary executables. No SDK, no protocol, no manifest. The contract: - -- **Stdio is the parent's terminal.** Stdin, stdout, and stderr are connected directly. Plugins can prompt interactively, stream output, and behave like any other CLI tool. -- **Exit codes propagate verbatim.** The parent `entire` exits with the plugin's exit code. -- **Signals reach the plugin.** Terminal signals (Ctrl+C) reach the plugin directly via the foreground process group. If the parent's context is cancelled (e.g. via `signal.Notify` plumbing), the plugin receives `SIGINT` with a 5-second grace before the runtime falls back to `SIGKILL`. Plugins that need clean shutdown should trap `SIGINT`. -- **Arguments after the plugin name pass through verbatim.** `entire pgr --help foo` invokes `entire-pgr` with argv `["--help", "foo"]`. Cobra's flag parsing does not run. -- **Windows.** On Windows, `exec.LookPath` resolves `.exe`, `.bat`, and `.cmd` extensions automatically. The "found but not executable" path is Unix-only — Windows treats extension match as the only correctness signal. - -## What Plugins Do Not Get - -- **No checkpoint integration.** Plugin file modifications are not tracked in checkpoints. Plugins do not appear in `entire activity`. If a plugin needs to participate in the session/checkpoint lifecycle, it must use the [agent protocol](external-agent-protocol.md) instead. -- **No transcript recording.** Plugin stdio is not captured. -- **No hook installation.** Plugins cannot register git hooks or agent hooks via the dispatcher. They are free to install their own, but `entire` does not coordinate. -- **No automatic update checks for the plugin itself.** The CLI runs `versioncheck.CheckAndNotify` for the parent CLI's version, not the plugin's. Plugin authors should handle their own update notifications. - -## Telemetry - -Plugin invocations are tracked only for plugins on a hardcoded allowlist (`officialPlugins` in `cmd/entire/cli/plugin_official.go`). Third-party plugin names are **never** sent — even with telemetry opted in. The reasoning matches gh's extension-telemetry posture: arbitrary plugin names can carry sensitive identifiers (project names, vendor names), and the safest default is silence. - -When an allowlisted plugin runs successfully, the CLI emits a `cli_plugin_executed` event with: - -- `plugin` — the plugin name -- `command` — `entire ` -- `cli_version`, `os`, `arch`, `isEntireEnabled` - -Plugin args and flags are deliberately **not** recorded. - -Telemetry fires only when: - -1. The plugin name is in `officialPlugins`. -2. `entire` settings have `Telemetry: true`. -3. `ENTIRE_TELEMETRY_OPTOUT` is unset. -4. The plugin exited with status 0. Failed/crashing invocations are not tracked, matching Cobra's `PersistentPostRun` semantics for built-in commands. - -## Adding an Entire-Shipped Plugin to the Allowlist - -When publishing an Entire-owned plugin (e.g. `entire-pgr`): - -1. Append the plugin name to `officialPlugins` in `cmd/entire/cli/plugin_official.go`. -2. Match must be exact and case-sensitive — the binary on disk is `entire-`. -3. Update or add tests if the plugin has unusual telemetry shape. - -Once allowlisted, `cli_plugin_executed` events for that plugin will flow through the existing PostHog pipeline. - -## Comparison with the Agent Protocol - -| | Plugin Dispatch | [Agent Protocol](external-agent-protocol.md) | -|---|---|---| -| **Binary name pattern** | `entire-` | `entire-agent-` | -| **Discovery** | Lazy, on first non-built-in arg | Lazy at command entry, gated by `external_agents` setting (setup flows bypass the gate via `DiscoverAndRegisterAlways`) | -| **Communication** | Process exec; stdio passthrough | Subcommand protocol; JSON over stdin/stdout | -| **Versioning** | None | `ENTIRE_PROTOCOL_VERSION` envelope | -| **Lifecycle integration** | None | Full (sessions, checkpoints, hooks, transcripts) | -| **Telemetry** | Allowlist only | Standard agent telemetry | -| **Working directory** | User's cwd | Repository root | -| **Use when** | You want to add a CLI verb | You want an AI agent to participate in checkpointed sessions | - -## Implementation - -The dispatcher lives in `cmd/entire/cli/plugin.go`. The entry point is `MaybeDispatchPlugin(ctx, rootCmd, args)`, called from `cmd/entire/main.go` before `rootCmd.ExecuteContext`. Returns `(handled bool, exitCode int)` — when `handled` is true, the caller exits with `exitCode`; otherwise it falls through to normal Cobra execution. - -Key files: - -- `cmd/entire/cli/plugin.go` — dispatcher, `resolvePlugin`, `runPlugin` -- `cmd/entire/cli/plugin_official.go` — `officialPlugins` allowlist, `IsOfficialPlugin` -- `cmd/entire/cli/telemetry/detached.go` — `BuildPluginEventPayload`, `TrackPluginDetached` -- `cmd/entire/cli/integration_test/plugin_dispatch_test.go` — end-to-end coverage of the early-dispatch path From 01fdc8503523162adb6ec74b180008db30d77686 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 4 May 2026 18:20:18 +0200 Subject: [PATCH 12/14] Use execx.NonInteractive for integration test subprocesses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The integration tests that spawn the real entire binary were using raw exec.Command, but the established pattern in this package (testenv.go's RunCLIWithError) is execx.NonInteractive, which puts the child in a new session with no controlling terminal. That ensures interactive.CanPromptInteractively() returns false in the child without relying on env-var plumbing — important if a future change makes any plugin-dispatch path attempt interactive prompts. Switches all 10 invocations across external_command_test.go and external_command_signal_unix_test.go. The signal test continues to pass: Setsid puts the child in a new session/process group, but direct cmd.Process.Signal(os.Interrupt) to the parent's PID is unaffected, and the parent's context-cancel handler still forwards SIGINT to the plugin via exec.Cmd.Cancel. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 0171e6b97736 --- .../external_command_signal_unix_test.go | 6 ++++-- .../integration_test/external_command_test.go | 20 ++++++++++--------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/cmd/entire/cli/integration_test/external_command_signal_unix_test.go b/cmd/entire/cli/integration_test/external_command_signal_unix_test.go index 71b7305444..ace120b9ff 100644 --- a/cmd/entire/cli/integration_test/external_command_signal_unix_test.go +++ b/cmd/entire/cli/integration_test/external_command_signal_unix_test.go @@ -4,13 +4,15 @@ package integration import ( "bytes" + "context" "fmt" "os" - "os/exec" "path/filepath" "strings" "testing" "time" + + "github.com/entireio/cli/cmd/entire/cli/execx" ) // SIGINT to the parent must reach the plugin so it can clean up — not @@ -37,7 +39,7 @@ func TestExternalCommand_SigintReachesPlugin(t *testing.T) { t.Fatalf("write plugin: %v", err) } - cmd := exec.Command(getTestBinary(), "trapint") + cmd := execx.NonInteractive(context.Background(), getTestBinary(), "trapint") cmd.Env = pathWith(dir) var pStderr bytes.Buffer cmd.Stdout = &bytes.Buffer{} diff --git a/cmd/entire/cli/integration_test/external_command_test.go b/cmd/entire/cli/integration_test/external_command_test.go index 0de2e81648..2cc9e94e07 100644 --- a/cmd/entire/cli/integration_test/external_command_test.go +++ b/cmd/entire/cli/integration_test/external_command_test.go @@ -4,6 +4,7 @@ package integration import ( "bytes" + "context" "errors" "fmt" "os" @@ -13,6 +14,7 @@ import ( "strings" "testing" + "github.com/entireio/cli/cmd/entire/cli/execx" "github.com/entireio/cli/cmd/entire/cli/testutil" ) @@ -61,7 +63,7 @@ func TestExternalCommand_HappyPath(t *testing.T) { argFile := filepath.Join(dir, "argv.txt") writePluginScript(t, dir, "entire-pgr", argFile, 0) - cmd := exec.Command(getTestBinary(), "pgr", "hello", "--flag", "value") + cmd := execx.NonInteractive(context.Background(), getTestBinary(), "pgr", "hello", "--flag", "value") cmd.Env = pathWith(dir) var stdout, stderr bytes.Buffer cmd.Stdout = &stdout @@ -90,7 +92,7 @@ func TestExternalCommand_ExitCodePropagation(t *testing.T) { dir := t.TempDir() writePluginScript(t, dir, "entire-failing", filepath.Join(dir, "argv.txt"), 42) - cmd := exec.Command(getTestBinary(), "failing") + cmd := execx.NonInteractive(context.Background(), getTestBinary(), "failing") cmd.Env = pathWith(dir) cmd.Stdout = &bytes.Buffer{} cmd.Stderr = &bytes.Buffer{} @@ -115,7 +117,7 @@ func TestExternalCommand_BuiltinWins(t *testing.T) { // (writePluginScript bakes that in via the requested code). writePluginScript(t, dir, "entire-version", filepath.Join(dir, "argv.txt"), 99) - cmd := exec.Command(getTestBinary(), "version") + cmd := execx.NonInteractive(context.Background(), getTestBinary(), "version") cmd.Env = pathWith(dir) var stdout, stderr bytes.Buffer cmd.Stdout = &stdout @@ -137,7 +139,7 @@ func TestExternalCommand_PluginNotFound(t *testing.T) { // PATH deliberately points at an empty dir so no plugin can resolve. dir := t.TempDir() - cmd := exec.Command(getTestBinary(), "definitely-not-a-real-plugin-or-builtin") + cmd := execx.NonInteractive(context.Background(), getTestBinary(), "definitely-not-a-real-plugin-or-builtin") cmd.Env = pathWith(dir) var stderr bytes.Buffer cmd.Stderr = &stderr @@ -162,7 +164,7 @@ func TestExternalCommand_FlagAfterPluginNameNotEatenByCobra(t *testing.T) { argFile := filepath.Join(dir, "argv.txt") writePluginScript(t, dir, "entire-passthrough", argFile, 0) - cmd := exec.Command(getTestBinary(), "passthrough", "--help", "--version", "subcmd") + cmd := execx.NonInteractive(context.Background(), getTestBinary(), "passthrough", "--help", "--version", "subcmd") cmd.Env = pathWith(dir) cmd.Stdout = &bytes.Buffer{} cmd.Stderr = &bytes.Buffer{} @@ -192,7 +194,7 @@ func TestExternalCommand_StdinPassthrough(t *testing.T) { t.Fatalf("write plugin: %v", err) } - cmd := exec.Command(getTestBinary(), "stdincat") + cmd := execx.NonInteractive(context.Background(), getTestBinary(), "stdincat") cmd.Env = pathWith(dir) cmd.Stdin = strings.NewReader("hello from parent stdin\n") cmd.Stdout = &bytes.Buffer{} @@ -238,7 +240,7 @@ func TestExternalCommand_EnvVarsForwarded(t *testing.T) { t.Fatalf("write plugin: %v", err) } - cmd := exec.Command(getTestBinary(), "envcheck") + cmd := execx.NonInteractive(context.Background(), getTestBinary(), "envcheck") cmd.Env = pathWith(pluginDir) cmd.Dir = resolvedRepo cmd.Stdout = &bytes.Buffer{} @@ -291,7 +293,7 @@ func TestExternalCommand_NonExecutableReportsLaunchError(t *testing.T) { t.Fatalf("write plugin: %v", err) } - cmd := exec.Command(getTestBinary(), "noexec") + cmd := execx.NonInteractive(context.Background(), getTestBinary(), "noexec") cmd.Env = pathWith(dir) var stderr bytes.Buffer cmd.Stdout = &bytes.Buffer{} @@ -313,7 +315,7 @@ func TestExternalCommand_AgentProtocolBinarySkipped(t *testing.T) { dir := t.TempDir() writePluginScript(t, dir, "entire-agent-foo", filepath.Join(dir, "argv.txt"), 0) - cmd := exec.Command(getTestBinary(), "agent-foo") + cmd := execx.NonInteractive(context.Background(), getTestBinary(), "agent-foo") cmd.Env = pathWith(dir) var stderr bytes.Buffer cmd.Stderr = &stderr From 97c9111e8f02b27e1400d3f1d1e8dfd31bb5d5f0 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Wed, 6 May 2026 15:41:49 +0200 Subject: [PATCH 13/14] Prime Cobra's lazy help/completion before plugin Find MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cobra adds the `help` and `completion` commands to the tree inside Execute, not in the constructor. resolvePlugin runs before Execute, so Find(["help"]) was returning an "unknown command" error and the "built-ins always win" guard never fired — letting an entire-help (or entire-completion) binary on PATH shadow the built-in. Call InitDefaultHelpCmd / InitDefaultCompletionCmd before Find. Both are idempotent and Execute calls them again later. Adds regression tests that fail on the unpatched code. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 2bff6dccfe6f --- cmd/entire/cli/plugin.go | 7 +++++++ cmd/entire/cli/plugin_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/cmd/entire/cli/plugin.go b/cmd/entire/cli/plugin.go index 4a3c605bd6..221f66cbd3 100644 --- a/cmd/entire/cli/plugin.go +++ b/cmd/entire/cli/plugin.go @@ -75,6 +75,13 @@ func resolvePlugin(rootCmd *cobra.Command, args []string) (binPath string, plugi if !isPluginCandidate(name) { return "", nil, false } + // Cobra adds `help` and `completion` to the command tree inside Execute, + // not in the constructor / SetHelpCommand. Without priming them, Find + // reports "unknown command" for those names and an entire-help (or + // entire-completion) binary on PATH would shadow the built-in. Both + // initializers are idempotent and Execute calls them again later. + rootCmd.InitDefaultHelpCmd() + rootCmd.InitDefaultCompletionCmd(args...) // Built-in commands always win. if cmd, _, err := rootCmd.Find(args); err == nil && cmd != rootCmd { return "", nil, false diff --git a/cmd/entire/cli/plugin_test.go b/cmd/entire/cli/plugin_test.go index defa3bca70..237e62ffdb 100644 --- a/cmd/entire/cli/plugin_test.go +++ b/cmd/entire/cli/plugin_test.go @@ -73,6 +73,39 @@ func TestResolvePlugin_NotFound(t *testing.T) { } } +// Cobra registers `help` and `completion` lazily, inside Execute. The plugin +// resolver runs before Execute, so it must prime those commands before +// consulting Find — otherwise an entire-help / entire-completion binary on +// PATH would shadow the built-in, violating "built-ins always win." +func TestResolvePlugin_BuiltinHelpWins(t *testing.T) { //nolint:paralleltest // mutates PATH via t.Setenv + dir := t.TempDir() + writePluginBinary(t, dir, "entire-help", filepath.Join(dir, "args.txt"), 0) + withPathDir(t, dir) + + // Use a Cobra-style root that mirrors NewRootCmd: SetHelpCommand only + // stashes the help command on the struct — it is not in the tree until + // InitDefaultHelpCmd runs. + root := newTestRoot() + root.SetHelpCommand(&cobra.Command{Use: "help"}) + + if _, _, ok := resolvePlugin(root, []string{"help"}); ok { + t.Fatal("built-in 'help' must take precedence over entire-help plugin") + } + if _, _, ok := resolvePlugin(root, []string{"help", "session"}); ok { + t.Fatal("'help session' must route to built-in help, not entire-help plugin") + } +} + +func TestResolvePlugin_BuiltinCompletionWins(t *testing.T) { //nolint:paralleltest // mutates PATH via t.Setenv + dir := t.TempDir() + writePluginBinary(t, dir, "entire-completion", filepath.Join(dir, "args.txt"), 0) + withPathDir(t, dir) + + if _, _, ok := resolvePlugin(newTestRoot(), []string{"completion", "bash"}); ok { + t.Fatal("built-in 'completion' must take precedence over entire-completion plugin") + } +} + func TestResolvePlugin_RejectsAgentPrefix(t *testing.T) { //nolint:paralleltest // mutates PATH via t.Setenv dir := t.TempDir() writePluginBinary(t, dir, "entire-agent-foo", filepath.Join(dir, "args.txt"), 0) From ec0a9cb53d0574ca117229c5bcac4984dff1a52d Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Wed, 6 May 2026 18:01:16 +0200 Subject: [PATCH 14/14] Filter the plugin process environment through an allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit External commands previously inherited the parent's full environment. That meant a binary on PATH saw AWS_ACCESS_KEY_ID, GITHUB_TOKEN, OPENAI_API_KEY, and similar credentials whether or not it had any business with them. kubectl and gh take the same approach, but neither treats env scrubbing as a security boundary. Defense in depth here is cheap: forward only OS-plumbing, locale, terminal, CI-detection, proxy, SSH-agent, and Windows essentials by default, plus the ENTIRE_*, LC_*, and XDG_* namespaces. Everything else is dropped. Users can opt names back in via ENTIRE_PLUGIN_ENV (comma-separated list of exact names or PREFIX_* wildcards) — for example ENTIRE_PLUGIN_ENV=AWS_*,GH_TOKEN. Caller-injected extras (ENTIRE_CLI_VERSION, ENTIRE_REPO_ROOT) are appended last so they override any matching parent value, per cmd/exec's last-wins contract. Coverage: table-driven unit test in plugin_env_test.go for the allowlist, prefix, override-exact, override-wildcard, whitespace, malformed-entry, and last-wins cases; integration tests assert end-to-end through the real entire binary that GITHUB_TOKEN / AWS_ACCESS_KEY_ID are dropped and that ENTIRE_PLUGIN_ENV opens names back up. Documents the behavior and the escape hatch in docs/architecture/external-commands.md. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: dcce922e6495 --- .../integration_test/external_command_test.go | 89 ++++++++++ cmd/entire/cli/plugin.go | 5 +- cmd/entire/cli/plugin_env.go | 157 +++++++++++++++++ cmd/entire/cli/plugin_env_test.go | 161 ++++++++++++++++++ docs/architecture/external-commands.md | 44 ++++- 5 files changed, 453 insertions(+), 3 deletions(-) create mode 100644 cmd/entire/cli/plugin_env.go create mode 100644 cmd/entire/cli/plugin_env_test.go diff --git a/cmd/entire/cli/integration_test/external_command_test.go b/cmd/entire/cli/integration_test/external_command_test.go index 2cc9e94e07..2d75037fbe 100644 --- a/cmd/entire/cli/integration_test/external_command_test.go +++ b/cmd/entire/cli/integration_test/external_command_test.go @@ -264,6 +264,95 @@ func TestExternalCommand_EnvVarsForwarded(t *testing.T) { } } +// writeEnvDumpPlugin creates an entire-envfilter plugin in its own dir +// that dumps the full child environment to env.txt. Each caller gets a +// fresh dir so parallel subtests don't trample each other's output. +func writeEnvDumpPlugin(t *testing.T) (pluginDir, envFile string) { + t.Helper() + pluginDir = t.TempDir() + envFile = filepath.Join(pluginDir, "env.txt") + body := fmt.Sprintf("#!/bin/sh\nenv > %q\nexit 0\n", envFile) + if err := os.WriteFile(filepath.Join(pluginDir, "entire-envfilter"), []byte(body), 0o755); err != nil { //nolint:gosec // test fixture + t.Fatalf("write plugin: %v", err) + } + return pluginDir, envFile +} + +// TestExternalCommand_EnvFiltered_CredentialsDropped asserts that +// credential-shaped variables in the parent environment do NOT reach +// the plugin, while allowlisted OS-plumbing variables do. +func TestExternalCommand_EnvFiltered_CredentialsDropped(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { + t.Skip("plugin shell-script harness only runs on Unix") + } + pluginDir, envFile := writeEnvDumpPlugin(t) + + cmd := execx.NonInteractive(context.Background(), getTestBinary(), "envfilter") + cmd.Env = append(pathWith(pluginDir), + "GITHUB_TOKEN=must-not-leak", + "AWS_ACCESS_KEY_ID=must-not-leak", + "NO_COLOR=1", + ) + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &bytes.Buffer{} + if err := cmd.Run(); err != nil { + t.Fatalf("entire envfilter failed: %v", err) + } + got, err := os.ReadFile(envFile) + if err != nil { + t.Fatalf("read env file: %v", err) + } + envVars := parseEnvLines(t, string(got)) + if _, ok := envVars["GITHUB_TOKEN"]; ok { + t.Error("GITHUB_TOKEN must be filtered out of plugin env") + } + if _, ok := envVars["AWS_ACCESS_KEY_ID"]; ok { + t.Error("AWS_ACCESS_KEY_ID must be filtered out of plugin env") + } + if got := envVars["NO_COLOR"]; got != "1" { + t.Errorf("NO_COLOR = %q, want %q", got, "1") + } +} + +// TestExternalCommand_EnvFiltered_OverrideWildcard asserts that +// ENTIRE_PLUGIN_ENV opens names back up via wildcard, but does not +// disable filtering for everything else. +func TestExternalCommand_EnvFiltered_OverrideWildcard(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { + t.Skip("plugin shell-script harness only runs on Unix") + } + pluginDir, envFile := writeEnvDumpPlugin(t) + + cmd := execx.NonInteractive(context.Background(), getTestBinary(), "envfilter") + cmd.Env = append(pathWith(pluginDir), + "ENTIRE_PLUGIN_ENV=AWS_*", + "AWS_PROFILE=dev", + "AWS_REGION=us-east-1", + "GITHUB_TOKEN=still-must-not-leak", + ) + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &bytes.Buffer{} + if err := cmd.Run(); err != nil { + t.Fatalf("entire envfilter failed: %v", err) + } + got, err := os.ReadFile(envFile) + if err != nil { + t.Fatalf("read env file: %v", err) + } + envVars := parseEnvLines(t, string(got)) + if got := envVars["AWS_PROFILE"]; got != "dev" { + t.Errorf("AWS_PROFILE = %q, want %q (override should admit it)", got, "dev") + } + if got := envVars["AWS_REGION"]; got != "us-east-1" { + t.Errorf("AWS_REGION = %q, want %q (override should admit it)", got, "us-east-1") + } + if _, ok := envVars["GITHUB_TOKEN"]; ok { + t.Error("GITHUB_TOKEN must remain filtered even with override") + } +} + // parseEnvLines splits "KEY=value" lines into a map. Missing keys map // to empty strings. func parseEnvLines(t *testing.T, contents string) map[string]string { diff --git a/cmd/entire/cli/plugin.go b/cmd/entire/cli/plugin.go index 221f66cbd3..6897d5b761 100644 --- a/cmd/entire/cli/plugin.go +++ b/cmd/entire/cli/plugin.go @@ -159,10 +159,11 @@ func runPlugin(ctx context.Context, binPath string, args []string) int { cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - cmd.Env = append(os.Environ(), "ENTIRE_CLI_VERSION="+versioninfo.Version) + extras := []string{"ENTIRE_CLI_VERSION=" + versioninfo.Version} if repoRoot, err := paths.WorktreeRoot(ctx); err == nil { - cmd.Env = append(cmd.Env, "ENTIRE_REPO_ROOT="+repoRoot) + extras = append(extras, "ENTIRE_REPO_ROOT="+repoRoot) } + cmd.Env = pluginEnv(os.Environ(), extras...) if err := cmd.Run(); err != nil { var exitErr *exec.ExitError diff --git a/cmd/entire/cli/plugin_env.go b/cmd/entire/cli/plugin_env.go new file mode 100644 index 0000000000..6fd89dcc32 --- /dev/null +++ b/cmd/entire/cli/plugin_env.go @@ -0,0 +1,157 @@ +package cli + +import ( + "runtime" + "strings" +) + +// Plugin process environment is filtered, not inherited wholesale, so that +// credentials and other tool-specific config don't leak into third-party +// binaries on PATH. The threat model is "defense in depth" — a malicious +// plugin can still read files under $HOME — but a small allowlist makes +// accidental exposure (AWS_*, GITHUB_TOKEN, OPENAI_API_KEY) require an +// explicit user opt-in. + +// pluginEnvAllowed is the exact-match allowlist of OS-plumbing and +// TUI-detection variables that virtually every program needs. Add entries +// here only when the whole namespace is unambiguously non-credential. +var pluginEnvAllowed = map[string]struct{}{ + // POSIX basics + "PATH": {}, "HOME": {}, "USER": {}, "LOGNAME": {}, + "SHELL": {}, "PWD": {}, "TMPDIR": {}, "TZ": {}, + + // Locale (LC_* covered by prefix below) + "LANG": {}, "LANGUAGE": {}, + + // Terminal / color rendering + "TERM": {}, "TERM_PROGRAM": {}, "TERM_PROGRAM_VERSION": {}, + "COLORTERM": {}, "NO_COLOR": {}, "FORCE_COLOR": {}, + "CLICOLOR": {}, "CLICOLOR_FORCE": {}, + + // CI detection — broad set because most tools can't detect CI any + // other way and many writeups assume these are visible. + "CI": {}, "GITHUB_ACTIONS": {}, "GITLAB_CI": {}, + "BUILDKITE": {}, "CIRCLECI": {}, "JENKINS_URL": {}, + "TEAMCITY_VERSION": {}, "TRAVIS": {}, + + // Proxies. curl and git read both upper- and lowercase forms; we keep + // both because the user's shell may set either. + "HTTP_PROXY": {}, "HTTPS_PROXY": {}, "NO_PROXY": {}, "ALL_PROXY": {}, + "http_proxy": {}, "https_proxy": {}, "no_proxy": {}, "all_proxy": {}, + + // SSH agent — plugins that fetch via git/ssh keep working. + "SSH_AUTH_SOCK": {}, "SSH_CONNECTION": {}, + + // Windows essentials. Names are upper-case here; lookup is + // case-insensitive on Windows. + "SYSTEMROOT": {}, "WINDIR": {}, "COMSPEC": {}, "PATHEXT": {}, + "APPDATA": {}, "LOCALAPPDATA": {}, "PROGRAMDATA": {}, + "PROGRAMFILES": {}, "PROGRAMFILES(X86)": {}, + "USERPROFILE": {}, "USERNAME": {}, "HOMEDRIVE": {}, "HOMEPATH": {}, + + // Documented in CLAUDE.md as the toggle for accessibility mode. + "ACCESSIBLE": {}, +} + +// pluginEnvPrefixes are namespaces we either own (ENTIRE_*) or that are +// long-standing passthrough conventions (LC_*, XDG_*). +var pluginEnvPrefixes = []string{ + "ENTIRE_", + "LC_", + "XDG_", +} + +// pluginEnvOverrideVar lets a user opt names back into the plugin env +// without a CLI release. Comma-separated list of exact names or +// `PREFIX_*` wildcards. Example: +// +// ENTIRE_PLUGIN_ENV="AWS_*,GH_TOKEN,EDITOR" +const pluginEnvOverrideVar = "ENTIRE_PLUGIN_ENV" + +// pluginEnv builds the child environment from the parent. Only allowlisted +// names plus user-declared overrides are forwarded. Caller-provided extras +// are appended verbatim; per cmd/exec docs, when Env contains duplicate +// keys the last one wins, so extras override any matching parent value. +func pluginEnv(parent []string, extra ...string) []string { + exact, prefixes := parsePluginEnvOverride(lookupEnv(parent, pluginEnvOverrideVar)) + out := make([]string, 0, len(parent)+len(extra)) + for _, kv := range parent { + eq := strings.IndexByte(kv, '=') + if eq <= 0 { + continue + } + if isPluginEnvAllowed(kv[:eq], exact, prefixes) { + out = append(out, kv) + } + } + out = append(out, extra...) + return out +} + +func isPluginEnvAllowed(name string, userExact map[string]struct{}, userPrefixes []string) bool { + if _, ok := pluginEnvAllowed[name]; ok { + return true + } + if runtime.GOOS == windowsGOOS { + // Env var names are case-insensitive on Windows. The allowlist + // stores the canonical upper-case form, so normalize here. + if _, ok := pluginEnvAllowed[strings.ToUpper(name)]; ok { + return true + } + } + for _, p := range pluginEnvPrefixes { + if hasPrefixOSAware(name, p) { + return true + } + } + if _, ok := userExact[name]; ok { + return true + } + for _, p := range userPrefixes { + if hasPrefixOSAware(name, p) { + return true + } + } + return false +} + +// parsePluginEnvOverride splits the comma-separated override into exact +// names and prefix patterns (`FOO_*`). Whitespace around items is trimmed. +func parsePluginEnvOverride(s string) (exact map[string]struct{}, prefixes []string) { + if s == "" { + return nil, nil + } + exact = map[string]struct{}{} + for _, raw := range strings.Split(s, ",") { + item := strings.TrimSpace(raw) + if item == "" { + continue + } + if strings.HasSuffix(item, "*") { + prefixes = append(prefixes, strings.TrimSuffix(item, "*")) + continue + } + exact[item] = struct{}{} + } + return exact, prefixes +} + +// lookupEnv returns the value of name from a KEY=VALUE slice, or empty if +// absent. Used to read ENTIRE_PLUGIN_ENV out of the parent slice without +// touching process state, so tests stay parallel-safe. +func lookupEnv(env []string, name string) string { + prefix := name + "=" + for _, kv := range env { + if strings.HasPrefix(kv, prefix) { + return kv[len(prefix):] + } + } + return "" +} + +func hasPrefixOSAware(name, prefix string) bool { + if runtime.GOOS == windowsGOOS { + return strings.HasPrefix(strings.ToUpper(name), strings.ToUpper(prefix)) + } + return strings.HasPrefix(name, prefix) +} diff --git a/cmd/entire/cli/plugin_env_test.go b/cmd/entire/cli/plugin_env_test.go new file mode 100644 index 0000000000..ca0c7fc3b5 --- /dev/null +++ b/cmd/entire/cli/plugin_env_test.go @@ -0,0 +1,161 @@ +package cli + +import ( + "slices" + "testing" +) + +func TestPluginEnv(t *testing.T) { + t.Parallel() + cases := []struct { + name string + parent []string + extra []string + wantHave []string // names that must be in the result + wantMiss []string // names that must NOT be in the result + }{ + { + name: "OS basics pass through", + parent: []string{"PATH=/bin", "HOME=/h", "TERM=xterm", "LANG=en_US.UTF-8"}, + wantHave: []string{"PATH", "HOME", "TERM", "LANG"}, + }, + { + name: "credentials are dropped", + parent: []string{"AWS_ACCESS_KEY_ID=x", "GITHUB_TOKEN=y", "DATABASE_URL=z", "OPENAI_API_KEY=k", "PATH=/bin"}, + wantHave: []string{"PATH"}, + wantMiss: []string{"AWS_ACCESS_KEY_ID", "GITHUB_TOKEN", "DATABASE_URL", "OPENAI_API_KEY"}, + }, + { + name: "tool-specific config dropped", + parent: []string{"EDITOR=vim", "VISUAL=vim", "PAGER=less", "GIT_ASKPASS=/x", "PATH=/bin"}, + wantHave: []string{"PATH"}, + wantMiss: []string{"EDITOR", "VISUAL", "PAGER", "GIT_ASKPASS"}, + }, + { + name: "ENTIRE namespace passes", + parent: []string{"ENTIRE_FOO=1", "ENTIRE_AUTH_TOKEN=secret", "PATH=/bin"}, + wantHave: []string{"ENTIRE_FOO", "ENTIRE_AUTH_TOKEN", "PATH"}, + }, + { + name: "LC_ prefix passes", + parent: []string{"LC_ALL=C", "LC_TIME=en_US", "LC_NUMERIC=en_US"}, + wantHave: []string{"LC_ALL", "LC_TIME", "LC_NUMERIC"}, + }, + { + name: "XDG_ prefix passes", + parent: []string{"XDG_CONFIG_HOME=/h/.cfg", "XDG_DATA_HOME=/h/.data"}, + wantHave: []string{"XDG_CONFIG_HOME", "XDG_DATA_HOME"}, + }, + { + name: "CI detection passes", + parent: []string{"CI=true", "GITHUB_ACTIONS=true", "BUILDKITE=true"}, + wantHave: []string{"CI", "GITHUB_ACTIONS", "BUILDKITE"}, + }, + { + name: "proxies pass in both cases", + parent: []string{"HTTP_PROXY=p", "https_proxy=p", "NO_PROXY=localhost"}, + wantHave: []string{"HTTP_PROXY", "https_proxy", "NO_PROXY"}, + }, + { + name: "extras are always added", + parent: []string{"PATH=/bin"}, + extra: []string{"ENTIRE_CLI_VERSION=1.0", "ENTIRE_REPO_ROOT=/r"}, + wantHave: []string{"ENTIRE_CLI_VERSION", "ENTIRE_REPO_ROOT", "PATH"}, + }, + { + name: "override admits an exact name", + parent: []string{"ENTIRE_PLUGIN_ENV=AWS_PROFILE", "AWS_PROFILE=dev", "AWS_REGION=us-east-1", "PATH=/bin"}, + wantHave: []string{"AWS_PROFILE", "PATH"}, + wantMiss: []string{"AWS_REGION"}, + }, + { + name: "override admits a wildcard prefix", + parent: []string{"ENTIRE_PLUGIN_ENV=AWS_*", "AWS_PROFILE=dev", "AWS_REGION=us-east-1", "GITHUB_TOKEN=x"}, + wantHave: []string{"AWS_PROFILE", "AWS_REGION"}, + wantMiss: []string{"GITHUB_TOKEN"}, + }, + { + name: "override accepts mixed list with whitespace", + parent: []string{"ENTIRE_PLUGIN_ENV= AWS_* , GH_TOKEN ", "AWS_PROFILE=dev", "GH_TOKEN=t", "GITHUB_TOKEN=x"}, + wantHave: []string{"AWS_PROFILE", "GH_TOKEN"}, + wantMiss: []string{"GITHUB_TOKEN"}, + }, + { + name: "malformed entries are skipped", + parent: []string{"PATH=/bin", "", "=novalue", "NOEQUAL"}, + wantHave: []string{"PATH"}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := pluginEnv(tc.parent, tc.extra...) + gotNames := envNames(got) + for _, want := range tc.wantHave { + if !slices.Contains(gotNames, want) { + t.Errorf("missing %q from result %v", want, gotNames) + } + } + for _, missing := range tc.wantMiss { + if slices.Contains(gotNames, missing) { + t.Errorf("expected %q to be filtered out, got %v", missing, gotNames) + } + } + }) + } +} + +// TestPluginEnv_ExtrasOverrideParent documents the cmd/exec contract: when +// the env slice contains duplicate keys the last value wins. We rely on +// this so caller-injected ENTIRE_CLI_VERSION / ENTIRE_REPO_ROOT always +// reflect the parent CLI's state, not a stale shell value. +func TestPluginEnv_ExtrasOverrideParent(t *testing.T) { + t.Parallel() + got := pluginEnv( + []string{"ENTIRE_CLI_VERSION=stale", "PATH=/bin"}, + "ENTIRE_CLI_VERSION=fresh", + ) + // Last occurrence in the slice should be the override. + var last string + for _, kv := range got { + if k, v, ok := splitKV(kv); ok && k == "ENTIRE_CLI_VERSION" { + last = v + } + } + if last != "fresh" { + t.Errorf("ENTIRE_CLI_VERSION (last) = %q, want %q (full env: %v)", last, "fresh", got) + } +} + +// TestPluginEnv_OverrideVarItselfPasses confirms the override declaration +// is forwarded to the child (matches the ENTIRE_ prefix). Useful so +// plugins can introspect what was opened up. +func TestPluginEnv_OverrideVarItselfPasses(t *testing.T) { + t.Parallel() + got := pluginEnv([]string{"ENTIRE_PLUGIN_ENV=AWS_*", "PATH=/bin"}) + if !slices.Contains(envNames(got), "ENTIRE_PLUGIN_ENV") { + t.Errorf("ENTIRE_PLUGIN_ENV should pass through to plugins; got %v", envNames(got)) + } +} + +func envNames(env []string) []string { + out := make([]string, 0, len(env)) + for _, kv := range env { + if k, _, ok := splitKV(kv); ok { + out = append(out, k) + } + } + return out +} + +func splitKV(kv string) (key, value string, ok bool) { + for i := range len(kv) { + if kv[i] == '=' { + if i == 0 { + return "", "", false + } + return kv[:i], kv[i+1:], true + } + } + return "", "", false +} diff --git a/docs/architecture/external-commands.md b/docs/architecture/external-commands.md index dcbcd8b25c..5d99096dcb 100644 --- a/docs/architecture/external-commands.md +++ b/docs/architecture/external-commands.md @@ -26,7 +26,48 @@ 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. | -Plus the parent's full environment. The working directory is **not** changed — external commands run in the user's current directory, the same as any other shell command. +The working directory is **not** changed — external commands run in the user's current directory, the same as any other shell command. + +### Environment filtering + +Unlike `kubectl` and `gh`, which forward the parent's full environment to every plugin, Entire **filters** the parent environment through a small allowlist before invoking an external command. The motivation is defense in depth: a plugin you installed shouldn't see `AWS_ACCESS_KEY_ID`, `GITHUB_TOKEN`, or `OPENAI_API_KEY` unless it has a reason to. (A malicious plugin can still read files under `$HOME` — the boundary is "what's accidentally exposed", not "what an attacker can reach".) + +Variables forwarded by default fall into a few categories: + +- **POSIX basics** — `PATH`, `HOME`, `USER`, `LOGNAME`, `SHELL`, `PWD`, `TMPDIR`, `TZ` +- **Locale** — `LANG`, `LANGUAGE`, and the entire `LC_*` family +- **Terminal / color** — `TERM`, `TERM_PROGRAM`, `COLORTERM`, `NO_COLOR`, `FORCE_COLOR`, `CLICOLOR`, `CLICOLOR_FORCE` +- **CI detection** — `CI`, `GITHUB_ACTIONS`, `GITLAB_CI`, `BUILDKITE`, `CIRCLECI`, `JENKINS_URL`, `TEAMCITY_VERSION`, `TRAVIS` +- **Proxies** — `HTTP_PROXY`, `HTTPS_PROXY`, `NO_PROXY`, `ALL_PROXY` (and lowercase variants) +- **SSH agent** — `SSH_AUTH_SOCK`, `SSH_CONNECTION` +- **Windows essentials** — `SYSTEMROOT`, `WINDIR`, `APPDATA`, `LOCALAPPDATA`, `PROGRAMDATA`, `PROGRAMFILES`, `PROGRAMFILES(X86)`, `USERPROFILE`, `USERNAME`, `HOMEDRIVE`, `HOMEPATH`, `COMSPEC`, `PATHEXT` +- **Namespace prefixes** — anything starting with `ENTIRE_`, `LC_`, or `XDG_` + +The full list lives in `pluginEnvAllowed` and `pluginEnvPrefixes` in `cmd/entire/cli/plugin_env.go`. + +### Opting names back in: `ENTIRE_PLUGIN_ENV` + +If a plugin needs an environment variable that isn't on the allowlist (for example `AWS_PROFILE` for an `entire-deploy` command), the user can opt names back in via `ENTIRE_PLUGIN_ENV`. It's a comma-separated list of either exact names or `PREFIX_*` wildcards: + +```sh +# Forward AWS_* and EDITOR +ENTIRE_PLUGIN_ENV='AWS_*,EDITOR' entire deploy + +# Forward a single token +ENTIRE_PLUGIN_ENV='GH_TOKEN' entire pgr +``` + +`ENTIRE_PLUGIN_ENV` itself is forwarded to plugins (it matches the `ENTIRE_` prefix), so plugins can introspect what was opened up. + +### Why filter? + +This is a **defense-in-depth** boundary, not a security perimeter. Plugins on `$PATH` are trusted to run as the user — they can read `~/.aws/credentials` directly if they want. The filter exists to: + +1. Avoid accidental token leakage to plugins that don't need credentials. +2. Make the contract between the CLI and a plugin explicit (plugins document the env they require). +3. Catch typos and stale env (a forgotten `OPENAI_API_KEY=...` from yesterday's experiment). + +Plugin authors who need a variable should either rely on the allowlist or document the `ENTIRE_PLUGIN_ENV` value users should set. ## Author Contract @@ -94,6 +135,7 @@ The resolver lives in `cmd/entire/cli/plugin.go`. The entry point is `MaybeRunPl 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/telemetry/detached.go` — `BuildPluginEventPayload`, `TrackPluginDetached` - `cmd/entire/cli/integration_test/external_command_test.go` — end-to-end coverage of the resolution path