fix(plugin): refuse /tmp fallback and verify path ownership before exec (D-1396)

internal/plugin/cli_binary.go

@@ -37,7 +37,7 @@ import (
 // PluginBinDir returns the directory the command proxy searches for plugin
 // binaries. Exported so the installer and the router cannot drift apart on
 // where that is.
-func PluginBinDir() string { return pluginBinDir() }
+func PluginBinDir() (string, error) { return pluginBinDir() }
 
 // PublishedBinaryPath returns where a plugin's command binary is published,
 // including the platform's executable suffix.
@@ -46,7 +46,11 @@ func PluginBinDir() string { return pluginBinDir() }
 // the test compute the name itself is how the Windows job went red: the
 // installer appended .exe and the test did not.
 func PublishedBinaryPath(binName string) string {
-	dst := filepath.Join(pluginBinDir(), binName)
+	binDir, err := pluginBinDir()
+	if err != nil {
+		return ""
+	}
+	dst := filepath.Join(binDir, binName)
 	if runtime.GOOS == "windows" {
 		dst += ".exe"
 	}
@@ -133,7 +137,10 @@ func linkCLIBinary(destDir, name string, m *PluginManifest) error {
 		return err
 	}
 
-	binDir := pluginBinDir()
+	binDir, err := pluginBinDir()
+	if err != nil {
+		return err
+	}
 	if err := os.MkdirAll(binDir, 0o755); err != nil {
 		return fmt.Errorf("creating plugin bin dir: %w", err)
 	}
@@ -168,7 +175,11 @@ func linkCLIBinary(destDir, name string, m *PluginManifest) error {
 // unlinkCLIBinary removes a plugin's published binary. A missing file is not an
 // error — the plugin may never have shipped one.
 func unlinkCLIBinary(name string, m *PluginManifest) error {
-	return unlinkCLIBinaryIn(filepath.Dir(pluginBinDir()), name, m)
+	binDir, err := pluginBinDir()
+	if err != nil {
+		return err
+	}
+	return unlinkCLIBinaryIn(filepath.Dir(binDir), name, m)
 }
 
 // unlinkCLIBinaryIn is unlinkCLIBinary with the plugin directory whose installed

internal/plugin/cli_binary_test.go

@@ -123,7 +123,10 @@ func TestUnlinkCLIBinary_RemovesPublishedCommand(t *testing.T) {
 	t.Setenv("HOME", home)
 	t.Setenv("USERPROFILE", home)
 
-	binDir := PluginBinDir()
+	binDir, err := PluginBinDir()
+	if err != nil {
+		t.Fatal(err)
+	}
 	if err := os.MkdirAll(binDir, 0o755); err != nil {
 		t.Fatalf("MkdirAll: %v", err)
 	}
@@ -168,7 +171,10 @@ func TestIsCommandInstalled_TracksThePublishedBinary(t *testing.T) {
 		t.Fatal("reported installed before anything was published")
 	}
 
-	binDir := PluginBinDir()
+	binDir, err := PluginBinDir()
+	if err != nil {
+		t.Fatal(err)
+	}
 	if err := os.MkdirAll(binDir, 0o755); err != nil {
 		t.Fatalf("MkdirAll: %v", err)
 	}

internal/plugin/cli_collision_test.go

@@ -87,7 +87,11 @@ func TestCheckCLIBinaries_ReinstallAndOrphan(t *testing.T) {
 		t.Fatalf("reinstall refused: %v", err)
 	}
 
-	if err := os.MkdirAll(PluginBinDir(), 0o755); err != nil {
+	binDir, err := PluginBinDir()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := os.MkdirAll(binDir, 0o755); err != nil {
 		t.Fatal(err)
 	}
 	if err := os.WriteFile(PublishedBinaryPath("nself-orphan"), []byte("old"), 0o755); err != nil {

internal/plugin/name_validate.go

@@ -78,7 +78,11 @@ func safePublishedBinaryPath(bin string) (string, error) {
 		return "", err
 	}
 	dst := PublishedBinaryPath(bin)
-	rel, err := filepath.Rel(pluginBinDir(), dst)
+	binDir, err := pluginBinDir()
+	if err != nil {
+		return "", err
+	}
+	rel, err := filepath.Rel(binDir, dst)
 	if err != nil || rel == "." || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || filepath.IsAbs(rel) {
 		return "", errs.Newf("E401", "plugin binary %q resolves outside the plugin bin directory", bin)
 	}

internal/plugin/name_validate_round2_test.go

@@ -41,7 +41,10 @@ func binFixture(t *testing.T) (binDir, victim string) {
 	home := t.TempDir()
 	t.Setenv("HOME", home)
 	t.Setenv("USERPROFILE", home)
-	binDir = pluginBinDir()
+	binDir, err := pluginBinDir()
+	if err != nil {
+		t.Fatal(err)
+	}
 	if err := os.MkdirAll(binDir, 0o755); err != nil {
 		t.Fatal(err)
 	}
@@ -91,7 +94,8 @@ func TestLinkCLIBinaryRefusesTraversal(t *testing.T) {
 func TestSafePublishedBinaryPathContainment(t *testing.T) {
 	binFixture(t)
 	p, err := safePublishedBinaryPath("nself-foo")
-	if err != nil || filepath.Dir(p) != pluginBinDir() {
+	binDir, pbdErr := pluginBinDir()
+	if err != nil || pbdErr != nil || filepath.Dir(p) != binDir {
 		t.Fatalf("nself-foo -> %q, %v", p, err)
 	}
 	if _, err := safePublishedBinaryPath("../x"); err == nil {

internal/plugin/router.go

@@ -6,11 +6,12 @@ import (
 	"os/exec"
 	"path/filepath"
 	"runtime"
-	"strings"
 
 	"github.com/nself-org/cli/internal/compat"
 	"github.com/nself-org/cli/internal/errs"
 	"github.com/nself-org/cli/internal/plugin/catalog"
+	"github.com/nself-org/cli/internal/pluginname"
+	"github.com/nself-org/cli/internal/pluginperm"
 )
 
 // ExitCodeError is returned when a plugin process exits with a non-zero code.
@@ -31,14 +32,15 @@ func (e *ExitCodeError) ExitCode() int { return e.Code }
 // inherited stdout/stderr and has already reported the failure itself.
 func (e *ExitCodeError) Silent() bool { return true }
 
-// pluginBinDir returns the directory where plugin binaries are installed.
-// This is ~/.nself/plugins/bin/ (or /tmp/.nself/plugins/bin/ as fallback).
-func pluginBinDir() string {
+// pluginBinDir returns ~/.nself/plugins/bin/. With HOME unset it refuses rather
+// than fall back to a shared directory such as /tmp, where any local user could
+// plant a binary that cron or systemd would run with the caller's secrets (D-1396).
+func pluginBinDir() (string, error) {
 	home, err := os.UserHomeDir()
 	if err != nil {
-		return filepath.Join("/tmp", ".nself", "plugins", "bin")
+		return "", errs.Newf("E406", "HOME is not set; refusing to resolve plugin binaries in a shared directory")
 	}
-	return filepath.Join(home, ".nself", "plugins", "bin")
+	return filepath.Join(home, ".nself", "plugins", "bin"), nil
 }
 
 // ProxyCommand checks if a plugin binary exists and executes it.
@@ -68,7 +70,10 @@ func ProxyCommandWithHint(cmdName string, args []string, installHint string) err
 	pluginBinary := fmt.Sprintf("nself-%s", cmdName)
 
 	// S-002: Only look in the plugin bin directory, never the full PATH.
-	binDir := pluginBinDir()
+	binDir, err := pluginBinDir()
+	if err != nil {
+		return err
+	}
 	candidate := filepath.Join(binDir, pluginBinary)
 	if runtime.GOOS == "windows" {
 		candidate += ".exe"
@@ -118,8 +123,11 @@ func ProxyCommandWithHint(cmdName string, args []string, installHint string) err
 // The mount validates the binary at discovery time (mount.Discover, E406);
 // a binary that vanishes between discovery and exec fails with E406 here.
 func ProxyBinary(binName, slug string, args []string) error {
-	binDir := pluginBinDir()
-	if !safeBinaryName(binName) {
+	binDir, err := pluginBinDir()
+	if err != nil {
+		return err
+	}
+	if !pluginname.IsCommandBinary(binName) {
 		return errs.Newf("E406", "plugin %q binary %q has an invalid name", slug, binName)
 	}
 	fileName := binName
@@ -133,40 +141,12 @@ func ProxyBinary(binName, slug string, args []string) error {
 // The plugins directory owner can replace the file between check and exec;
 // this accepted window does not cross a different trust boundary.
 func ProxyBinaryAt(candidate, rootDir, slug string, args []string) error {
-	resolvedRoot, err := filepath.EvalSymlinks(rootDir)
-	if err != nil {
-		return errs.Newf("E406", "plugin %q binary directory is unavailable", slug)
-	}
-	resolved, err := filepath.EvalSymlinks(candidate)
-	if err != nil {
-		return errs.Newf("E406", "plugin %q binary is unavailable", slug)
-	}
-	rel, err := filepath.Rel(resolvedRoot, resolved)
-	if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
-		return errs.Newf("E406", "plugin %q binary resolves outside the plugins directory", slug)
-	}
-	info, err := os.Stat(resolved)
-	if err != nil || !info.Mode().IsRegular() || (runtime.GOOS != "windows" && info.Mode().Perm()&0111 == 0) {
-		return errs.Newf("E406", "plugin %q binary is not executable", slug)
+	if _, err := pluginperm.Resolve(candidate, rootDir, slug); err != nil {
+		return errs.Newf("E406", "%v", err)
 	}
 	return execPluginBinary(candidate, slug, args)
 }
 
-func safeBinaryName(name string) bool {
-	if !strings.HasPrefix(name, "nself-") || len(name) <= len("nself-") {
-		return false
-	}
-	if name[len("nself-")] < 'a' || name[len("nself-")] > 'z' {
-		return false
-	}
-	for _, r := range name[len("nself-"):] {
-		if (r < 'a' || r > 'z') && (r < '0' || r > '9') && r != '-' {
-			return false
-		}
-	}
-	return true
-}
-
 // execPluginBinary is the shared run of a plugin binary: inherited stdio, the
 // project settings the plugin's manifest declares, and the plugin's exit
 // status surfaced as ExitCodeError (silent: the child already reported).
@@ -213,7 +193,11 @@ func pluginEnvForCommand(cmdName string) []string {
 	}
 	// Installed plugins sit alongside the bin directory the proxy reads, so
 	// deriving the path from pluginBinDir keeps the two from drifting apart.
-	installed := filepath.Join(filepath.Dir(pluginBinDir()), cmdName)
+	binDir, err := pluginBinDir()
+	if err != nil {
+		return nil
+	}
+	installed := filepath.Join(filepath.Dir(binDir), cmdName)
 	m := readPluginManifest(installed)
 	if m == nil {
 		return nil

internal/plugin/router_test.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 
@@ -82,3 +83,47 @@ func TestRouterKeepsExistingHint(t *testing.T) {
 		t.Fatalf("no cache should keep the generic hint: %v", err)
 	}
 }
+
+// TestPluginBinDirRefusesWithoutHOME proves D-1396: when HOME is unset the
+// router must not fall back to a shared directory like /tmp/.nself/plugins/bin,
+// because any local user could plant a binary there that cron or systemd would
+// execute with the caller's secrets.
+func TestPluginBinDirRefusesWithoutHOME(t *testing.T) {
+	t.Setenv("HOME", "")
+	t.Setenv("USERPROFILE", "")
+	_, err := pluginBinDir()
+	if err == nil {
+		t.Fatal("pluginBinDir succeeded without HOME; expected refusal")
+	}
+	if !strings.Contains(err.Error(), "HOME is not set") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
+// TestProxyBinaryAtRefusesUnsafePath wires the ownership check through
+// ProxyBinaryAt so that a binary in an unsafe directory is rejected even
+// when the binary itself is well-formed. Removing the call to
+// pluginperm.Verify in ProxyBinaryAt would make this test pass.
+func TestProxyBinaryAtRefusesUnsafePath(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("ownership check is a no-op on Windows")
+	}
+	root := t.TempDir()
+	binDir := filepath.Join(root, "bin")
+	if err := os.MkdirAll(binDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	bin := filepath.Join(binDir, "nself-testplug")
+	if err := os.WriteFile(bin, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+
+	// Make the bin directory world-writable -> ProxyBinaryAt must refuse.
+	if err := os.Chmod(binDir, 0o757); err != nil {
+		t.Fatal(err)
+	}
+	err := ProxyBinaryAt(bin, root, "testplug", nil)
+	if err == nil || !strings.Contains(err.Error(), "world-writable") {
+		t.Fatalf("ProxyBinaryAt did not refuse world-writable dir: %v", err)
+	}
+}

internal/pluginname/name.go

@@ -103,6 +103,12 @@ func SlugReason(name string) string {
 // element). It is wider than a plugin slug only for '.', '_' and capitals.
 var BinaryRe = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]*$`)
 
+// IsCommandBinary reports whether name is "nself-" plus a plugin slug, the shape
+// of every command binary the router will exec.
+func IsCommandBinary(name string) bool {
+	return strings.HasPrefix(name, "nself-") && SlugRe.MatchString(name[len("nself-"):])
+}
+
 // BinaryReason returns why bin is not a single plain path element that is safe
 // to join to the plugin bin directory, or "". Same checks as a plugin name (no
 // separator, no dot reference, no control character, Base unchanged) with the

internal/pluginname/name_test.go

@@ -41,3 +41,16 @@ func TestBinaryReason(t *testing.T) {
 		}
 	}
 }
+
+func TestIsCommandBinary(t *testing.T) {
+	for _, ok := range []string{"nself-a", "nself-foo-2"} {
+		if !IsCommandBinary(ok) {
+			t.Errorf("IsCommandBinary(%q) = false, want true", ok)
+		}
+	}
+	for _, bad := range []string{"", "nself-", "nself-2x", "nself-Foo", "nself-a_b", "nself-a.b", "nself-a\n", "foo-bar", "nself-../x"} {
+		if IsCommandBinary(bad) {
+			t.Errorf("IsCommandBinary(%q) = true, want false", bad)
+		}
+	}
+}

internal/pluginperm/perm_unix.go

@@ -0,0 +1,49 @@
+//go:build !windows
+
+package pluginperm
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"syscall"
+)
+
+// Verify checks that every component of path up to and including root is
+// safe for plugin execution: owned by the current uid and not writable by group
+// or world, whatever the group (a shared group such as macOS staff would let
+// other members replace plugin binaries). A missing or unstat-able component
+// fails closed.
+func Verify(path, root string) error {
+	uid := uint32(syscall.Getuid())
+	cur := path
+	for {
+		info, err := os.Lstat(cur)
+		if err != nil {
+			return fmt.Errorf("plugin binary path component %q is not accessible", cur)
+		}
+		stat, ok := info.Sys().(*syscall.Stat_t)
+		if !ok {
+			return fmt.Errorf("plugin binary path component %q has no stat info", cur)
+		}
+		if stat.Uid != uid {
+			return fmt.Errorf("plugin binary path component %q is owned by uid %d, not %d", cur, stat.Uid, uid)
+		}
+		mode := info.Mode().Perm()
+		if mode&0o002 != 0 {
+			return fmt.Errorf("plugin binary path component %q is world-writable", cur)
+		}
+		if mode&0o020 != 0 {
+			return fmt.Errorf("plugin binary path component %q is group-writable (run chmod g-w %q to fix)", cur, cur)
+		}
+		if cur == root || cur == "/" || cur == "." {
+			break
+		}
+		parent := filepath.Dir(cur)
+		if parent == cur {
+			break
+		}
+		cur = parent
+	}
+	return nil
+}