fix: gate SUDO_USER trust and root path diversion; add doctor binary check

Address review findings on the system-install PR:

- cloud_sink: honor SUDO_USER for audit attribution only when euid==0.
  Without the gate any user could set SUDO_USER and spoof cloud-audit
  attribution to another account. Matches the guard in cmd/setup/cert.go.

- config: divert per-user paths to root's passwd home only on an actual
  sudo elevation (euid==0 && SUDO_USER set), not for every root euid.
  The blanket root diversion ignored HOME/XDG_CONFIG_HOME and silently
  stopped reading genuine root users' config (golden Docker images),
  regressing two tests that only fail when the suite runs as root.
  Genuine root honors the environment as before; su without - leaves no
  marker and stays a documented, loud-failing residual.

- doctor: add a system-only check re-validating that the binary the
  installed shims exec is still root-owned and non-writable, catching
  permission/ownership drift after install.

- shim: fold the duplicated shim-scan loop into firstShimContent.

cmd/setup/doctor.go

@@ -33,6 +33,7 @@ const (
 	checkProtectionNpm      = "protection-npm"
 	checkProtectionPip      = "protection-pip"
 	checkCA                 = "ca-cert"
+	checkSystemBinary       = "system-binary"
 
 	aliasesInstalledMessage = "Shell aliases installed"
 )
@@ -254,9 +255,42 @@ func runCoreChecks(cfg *config.RuntimeConfig) []doctor.CheckResult {
 			},
 		},
 	}
+
+	// System-only: the binary every user's shim execs must stay root-owned and
+	// non-writable. Validation runs at install; re-check it here to catch later
+	// permission/ownership drift (redeploy, chmod, image rebuild).
+	if shim.SystemShimsInstalled() {
+		checks = append(checks, doctor.Check{
+			Name:     checkSystemBinary,
+			Category: "Security",
+			Run:      checkSystemBinaryResult,
+		})
+	}
+
 	return doctor.RunChecks(checks)
 }
 
+func checkSystemBinaryResult() doctor.CheckResult {
+	path, ok := shim.SystemShimBinary()
+	if !ok {
+		return doctor.CheckResult{
+			Status:  doctor.StatusWarn,
+			Message: "Could not determine system shim binary",
+		}
+	}
+	if err := shim.ValidateSystemBinary(path); err != nil {
+		return doctor.CheckResult{
+			Status:  doctor.StatusFail,
+			Message: fmt.Sprintf("System binary unsafe: %v", err),
+			Fix:     "Reinstall with pmg setup install --system, or restore root ownership/permissions",
+		}
+	}
+	return doctor.CheckResult{
+		Status:  doctor.StatusPass,
+		Message: fmt.Sprintf("System binary is root-owned and safe (%s)", path),
+	}
+}
+
 // checkEventLogDirResult is the testable core of the event-log dir check.
 // Event logging is mandatory (init failure is fatal), so an unwritable dir
 // fail-closes every pmg command for this user. The remedy is triaged: chown
@@ -371,6 +405,9 @@ func shimDirs() []string {
 	return dirs
 }
 
+// checkShimDirResolution classifies interception against all shim dirs via
+// shimDirs(); shimDir/pathLabel only select the PATH-membership fallback and
+// the display label, not which directories count as intercepting.
 func checkShimDirResolution(shimDir, pathLabel string, pathEntries []string) doctor.CheckResult {
 	underShim, shadowed := classifyPackageManagerResolutions(
 		alias.DefaultConfig().PackageManagers,
@@ -481,6 +518,7 @@ var checkDisplayNames = map[string]string{
 	checkProtectionNpm:      "npm protection",
 	checkProtectionPip:      "pip protection",
 	checkCA:                 "MITM CA",
+	checkSystemBinary:       "System binary",
 }
 
 var checkFixes = map[string]string{

cmd/setup/doctor_test.go

@@ -107,6 +107,17 @@ func TestClassifyPackageManagerResolutionsAcceptsEitherShimDir(t *testing.T) {
 	assert.Equal(t, []string{"yarn"}, shadowed)
 }
 
+func TestCheckSystemBinaryResult(t *testing.T) {
+	// No system shims installed -> could not determine binary (Warn).
+	result := checkSystemBinaryResult()
+	// On a dev machine with no /usr/local/lib/pmg/bin shims, SystemShimBinary
+	// returns !ok, so we get a Warn rather than a spurious Fail.
+	assert.Contains(t, []doctor.CheckStatus{doctor.StatusWarn, doctor.StatusPass, doctor.StatusFail}, result.Status)
+	if result.Status == doctor.StatusWarn {
+		assert.Equal(t, "Could not determine system shim binary", result.Message)
+	}
+}
+
 func TestCheckEventLogDirResult(t *testing.T) {
 	configDir := "/home/dev/.config/safedep/pmg"
 

config/config.go

@@ -705,6 +705,19 @@ func pathWithinDir(path, dir string) bool {
 	return cleanPath == cleanDir || strings.HasPrefix(cleanPath, cleanDir+string(os.PathSeparator))
 }
 
+// isSudoElevation reports whether pmg is running as root via sudo, i.e. a
+// non-root user elevated and sudo may have preserved that user's HOME/XDG_*.
+// Only then do per-user paths divert to root's own home, so root does not
+// create state inside the invoking user's home. Running genuinely as root
+// (no sudo) keeps honoring HOME/XDG_*, which is legitimate and intended (e.g.
+// golden Docker images that set HOME/XDG_CONFIG_HOME on purpose). This mirrors
+// the SUDO_USER guard used elsewhere (cmd/setup/cert.go). su without sudo does
+// not set SUDO_USER and is not covered; the unwritable-dir remedy still guides
+// the user if such a run poisons a directory.
+func isSudoElevation() bool {
+	return configGeteuid() == 0 && os.Getenv("SUDO_USER") != ""
+}
+
 // configDir computes the path to the config directory.
 func configDir() (string, error) {
 	dir := os.Getenv(pmgConfigDirEnvKey)
@@ -712,7 +725,7 @@ func configDir() (string, error) {
 		return dir, nil
 	}
 
-	if configGeteuid() == 0 {
+	if isSudoElevation() {
 		if base, err := rootConfigDirResolver(); err == nil {
 			return filepath.Join(base, pmgDefaultHomeRelativePath), nil
 		} else {
@@ -844,7 +857,7 @@ func cacheDir() (string, error) {
 		}
 		return filepath.Join(baseDir, pmgDefaultHomeRelativePath), nil
 	case "darwin", "linux":
-		if configGeteuid() == 0 {
+		if isSudoElevation() {
 			if base, err := rootCacheDirResolver(); err == nil {
 				return filepath.Join(base, pmgDefaultHomeRelativePath), nil
 			} else {

config/rootdir_test.go

@@ -25,9 +25,10 @@ func poisonUserEnv(t *testing.T) {
 	t.Setenv("XDG_CACHE_HOME", "/home/victim/.cache")
 }
 
-func TestConfigDirAsRootIgnoresPreservedHome(t *testing.T) {
+func TestConfigDirUnderSudoIgnoresPreservedHome(t *testing.T) {
 	poisonUserEnv(t)
 	withEuid(t, 0)
+	t.Setenv("SUDO_USER", "victim")
 
 	dir, err := configDir()
 	require.NoError(t, err)
@@ -38,6 +39,18 @@ func TestConfigDirAsRootIgnoresPreservedHome(t *testing.T) {
 	assert.NotContains(t, dir, "/home/victim")
 }
 
+func TestConfigDirGenuineRootHonorsEnv(t *testing.T) {
+	// Root without sudo (SUDO_USER unset) is the intended user, e.g. a golden
+	// Docker image that deliberately sets XDG_CONFIG_HOME. It must not divert.
+	poisonUserEnv(t)
+	withEuid(t, 0)
+	t.Setenv("SUDO_USER", "")
+
+	dir, err := configDir()
+	require.NoError(t, err)
+	assert.Contains(t, dir, "/home/victim")
+}
+
 func TestConfigDirAsNonRootUsesEnvHome(t *testing.T) {
 	poisonUserEnv(t)
 	withEuid(t, 1000)
@@ -47,19 +60,21 @@ func TestConfigDirAsNonRootUsesEnvHome(t *testing.T) {
 	assert.Contains(t, dir, "/home/victim")
 }
 
-func TestConfigDirEnvOverrideWinsForRoot(t *testing.T) {
+func TestConfigDirEnvOverrideWinsUnderSudo(t *testing.T) {
 	poisonUserEnv(t)
 	t.Setenv("PMG_CONFIG_DIR", "/custom/pmg")
 	withEuid(t, 0)
+	t.Setenv("SUDO_USER", "victim")
 
 	dir, err := configDir()
 	require.NoError(t, err)
 	assert.Equal(t, "/custom/pmg", dir)
 }
 
-func TestCacheDirAsRootIgnoresPreservedHome(t *testing.T) {
+func TestCacheDirUnderSudoIgnoresPreservedHome(t *testing.T) {
 	poisonUserEnv(t)
 	withEuid(t, 0)
+	t.Setenv("SUDO_USER", "victim")
 
 	dir, err := cacheDir()
 	require.NoError(t, err)
@@ -70,6 +85,16 @@ func TestCacheDirAsRootIgnoresPreservedHome(t *testing.T) {
 	assert.NotContains(t, dir, "/home/victim")
 }
 
+func TestCacheDirGenuineRootHonorsEnv(t *testing.T) {
+	poisonUserEnv(t)
+	withEuid(t, 0)
+	t.Setenv("SUDO_USER", "")
+
+	dir, err := cacheDir()
+	require.NoError(t, err)
+	assert.Contains(t, dir, "/home/victim")
+}
+
 func TestCacheDirAsNonRootUsesEnvHome(t *testing.T) {
 	poisonUserEnv(t)
 	withEuid(t, 1000)
@@ -82,6 +107,7 @@ func TestCacheDirAsNonRootUsesEnvHome(t *testing.T) {
 func TestRootDirsFallBackToEnvWhenPasswdUnavailable(t *testing.T) {
 	poisonUserEnv(t)
 	withEuid(t, 0)
+	t.Setenv("SUDO_USER", "victim")
 
 	origConfig, origCache := rootConfigDirResolver, rootCacheDirResolver
 	rootConfigDirResolver = func() (string, error) { return "", assert.AnError }

docs/system-install.md

@@ -115,7 +115,7 @@ Shared policy lives under `/etc/safedep/pmg`. Runtime data stays per user:
 
 You can relocate these with `PMG_CONFIG_DIR` and `PMG_CACHE_DIR`.
 
-When pmg runs as root (including via sudo), its per-user data goes under `/root`, regardless of any `HOME` preserved by sudo. Root never writes into another user's home.
+When pmg runs under `sudo` (a non-root user elevated to root), its per-user data resolves under root's own home (`/root`), not the invoking user's, even if sudo preserved their `HOME`. Running directly as root honors `HOME`/`XDG_CONFIG_HOME` as usual, so golden images that set those on purpose keep working. This detection relies on sudo's `SUDO_USER` marker: `su` without `-` leaks the caller's environment but leaves no marker, so a root shell obtained that way can still write into the caller's home. Prefer `su -` or `sudo`.
 
 The invoking user must be able to write their config directory. PMG records an event log there on each run and fails the command if it cannot (unless event logging is disabled in config).
 
@@ -123,7 +123,7 @@ In Docker images, avoid creating `/home/<user>/.config/safedep` as root during t
 
 If every `pmg` command fails with `permission denied` on the event log, check where the reported path points:
 
-- **Inside your own home**: a root run created it as root (preserved `HOME`: `sudo -E`, `su` without `-`, images that set `ENV HOME` before dropping root). Restore ownership: `sudo chown -R $(id -un) ~/.config/safedep`
+- **Inside your own home**: a root run created it as root (`su` without `-`, images that set `ENV HOME` before dropping root, or an older pmg under `sudo`). Restore ownership: `sudo chown -R $(id -un) ~/.config/safedep`
 - **Inside another user's home**: your environment leaked that user's `HOME` or `XDG_CONFIG_HOME` (e.g. `sudo -u <user>` on GitHub-hosted runners). Fix the environment (`export XDG_CONFIG_HOME="$HOME/.config"`). Do not chown another user's directory; that bricks their pmg instead.
 
 The error message and `pmg setup doctor` print the fix matching your case.

internal/audit/cloud_sink.go

@@ -118,12 +118,17 @@ func (s *cloudSink) buildInvocationContext() *controltowerv1.EndpointInvocationC
 	return ctx
 }
 
-// invokingUser resolves the human behind the command, preferring SUDO_USER so a
-// `sudo npm ...` is attributed to the operator rather than root.
+var auditGeteuid = os.Geteuid
+
+// invokingUser resolves the human behind the command. SUDO_USER is honored
+// only when the process is actually elevated (euid 0); otherwise any user
+// could set SUDO_USER to spoof cloud-audit attribution to another account.
 func invokingUser() *user.User {
-	if name := os.Getenv("SUDO_USER"); name != "" {
-		if u, err := user.Lookup(name); err == nil {
-			return u
+	if auditGeteuid() == 0 {
+		if name := os.Getenv("SUDO_USER"); name != "" {
+			if u, err := user.Lookup(name); err == nil {
+				return u
+			}
 		}
 	}
 	u, err := user.Current()

internal/audit/cloud_sink_test.go

@@ -2,6 +2,7 @@ package audit
 
 import (
 	"context"
+	"os/user"
 	"testing"
 	"time"
 
@@ -157,3 +158,25 @@ func TestCloudSinkSetsInvocationContextOnSessionComplete(t *testing.T) {
 	assert.NotEmpty(t, invCtx.GetUsername())
 	assert.NotEmpty(t, invCtx.GetUsernameUid())
 }
+
+func TestInvokingUserIgnoresSudoUserWhenNotElevated(t *testing.T) {
+	current, err := user.Current()
+	require.NoError(t, err)
+
+	orig := auditGeteuid
+	t.Cleanup(func() { auditGeteuid = orig })
+
+	// Non-root process: SUDO_USER must be ignored, else attribution is spoofable.
+	auditGeteuid = func() int { return 1000 }
+	t.Setenv("SUDO_USER", "root")
+	got := invokingUser()
+	require.NotNil(t, got)
+	assert.Equal(t, current.Username, got.Username, "SUDO_USER must not override attribution when not elevated")
+
+	// Elevated (euid 0): SUDO_USER is trusted and used.
+	auditGeteuid = func() int { return 0 }
+	t.Setenv("SUDO_USER", current.Username)
+	got = invokingUser()
+	require.NotNil(t, got)
+	assert.Equal(t, current.Username, got.Username)
+}

internal/shim/system.go

@@ -168,10 +168,55 @@ func SystemShimsInstalled() bool {
 	return shimsPresent(SystemBinDir())
 }
 
+// SystemShimBinary returns the pmg binary path that installed system shims
+// execute (hard-coded as PMG_BIN in every shim). ok is false when no system
+// shim with a resolvable PMG_BIN is present. This is the binary every user's
+// shim runs, so it is the one whose integrity matters after install. All shims
+// are written from the same template in one pass, so reading one suffices.
+func SystemShimBinary() (string, bool) {
+	content, ok := firstShimContent(SystemBinDir())
+	if !ok {
+		return "", false
+	}
+	return parseShimPMGBin(content)
+}
+
+// parseShimPMGBin extracts the PMG_BIN value from a shim script, reversing the
+// shellQuote used by writeShimScript.
+func parseShimPMGBin(content string) (string, bool) {
+	for line := range strings.SplitSeq(content, "\n") {
+		if rest, ok := strings.CutPrefix(line, "PMG_BIN="); ok {
+			return shellUnquote(rest), true
+		}
+	}
+	return "", false
+}
+
+// shellUnquote reverses shellQuote for the single-quoted form it emits.
+func shellUnquote(s string) string {
+	s = strings.TrimSpace(s)
+	s = strings.TrimPrefix(s, "'")
+	s = strings.TrimSuffix(s, "'")
+	return strings.ReplaceAll(s, `'\''`, `'`)
+}
+
+// ValidateSystemBinary re-runs the system-install safety checks against path.
+// Used by `pmg setup doctor` to detect ownership/permission drift of the
+// installed binary after setup (validation otherwise runs only at install).
+func ValidateSystemBinary(path string) error {
+	return validateSystemExecutable(path)
+}
+
 func shimsPresent(dir string) bool {
+	_, ok := firstShimContent(dir)
+	return ok
+}
+
+// firstShimContent returns the content of the first managed shim script in dir.
+func firstShimContent(dir string) (string, bool) {
 	entries, err := os.ReadDir(dir)
 	if err != nil {
-		return false
+		return "", false
 	}
 	for _, e := range entries {
 		if e.IsDir() {
@@ -179,10 +224,10 @@ func shimsPresent(dir string) bool {
 		}
 		content, err := os.ReadFile(filepath.Join(dir, e.Name()))
 		if err == nil && strings.Contains(string(content), shimScriptMarker) {
-			return true
+			return string(content), true
 		}
 	}
-	return false
+	return "", false
 }
 
 // SystemProfileInstalled reports whether the system profile snippet exists and

internal/shim/system_test.go

@@ -165,6 +165,41 @@ func TestValidateSystemExecutableRejectsNonRootOwner(t *testing.T) {
 	assert.Contains(t, err.Error(), "must be owned by root")
 }
 
+func TestSystemShimBinaryResolvesInstalledPath(t *testing.T) {
+	root := t.TempDir()
+	useSystemPaths(t, root)
+
+	exe := filepath.Join(root, "pmg")
+	require.NoError(t, os.WriteFile(exe, []byte("#!/bin/sh\n"), 0o755))
+	resolveExecutable = func() (string, error) { return exe, nil }
+
+	mgr, err := NewSystemShimManager()
+	require.NoError(t, err)
+	require.NoError(t, mgr.Install())
+
+	got, ok := SystemShimBinary()
+	require.True(t, ok)
+	assert.Equal(t, exe, got)
+}
+
+func TestSystemShimBinaryFalseWhenNoShims(t *testing.T) {
+	root := t.TempDir()
+	useSystemPaths(t, root)
+	require.NoError(t, os.MkdirAll(SystemBinDir(), 0o755))
+
+	_, ok := SystemShimBinary()
+	assert.False(t, ok)
+}
+
+func TestParseShimPMGBinRoundTripsShellQuote(t *testing.T) {
+	for _, path := range []string{"/usr/local/bin/pmg", "/opt/pmg dir/pmg", "/weird/o'brien/pmg"} {
+		content := "#!/bin/sh\n" + shimScriptMarker + "\nPMG_BIN=" + shellQuote(path) + "\n"
+		got, ok := parseShimPMGBin(content)
+		require.True(t, ok, path)
+		assert.Equal(t, path, got)
+	}
+}
+
 func TestNewSystemShimManagerForRemoveSkipsValidation(t *testing.T) {
 	root := t.TempDir()
 	useSystemPaths(t, root)