diff --git a/.github/workflows/pmg-e2e.yml b/.github/workflows/pmg-e2e.yml index 62bd5c4..381866b 100644 --- a/.github/workflows/pmg-e2e.yml +++ b/.github/workflows/pmg-e2e.yml @@ -947,6 +947,21 @@ jobs: test -x "/usr/local/lib/pmg/bin/$shim" || { echo "Missing shim: $shim"; exit 1; } done + - name: Root runs keep per-user state out of the invoking user's home + run: | + # GitHub runner sudo preserves HOME. Every sudo pmg run above used to + # create root-owned ~/.config/safedep for the runner user, which + # fail-closes all their later pmg/npm runs. Must run before any + # non-root pmg invocation legitimately creates that directory. + sudo sh -c 'echo "sudo sees HOME=$HOME"' + if [ -e "$HOME/.config/safedep" ]; then + echo "ERROR: root-created state leaked into $HOME/.config/safedep" + ls -laR "$HOME/.config/safedep" + exit 1 + fi + sudo test -d /root/.config/safedep/pmg/logs + echo "SUCCESS: root state stayed under /root" + - name: PATH and profile.d activate shims run: | # Docker-style: non-login shells need PATH (or source profile.d) @@ -957,14 +972,18 @@ jobs: - name: Managed config refuses CLI mutation run: | - if pmg config set dependency_cooldown.days 7; then + # Assert the refusal reason: a permission-denied brick (poisoned home) + # would also make config set fail and mask a regression. + if out=$(pmg config set dependency_cooldown.days 7 2>&1); then echo "ERROR: config set should fail under system config" exit 1 fi - if sudo pmg config set dependency_cooldown.days 7; then + echo "$out" | grep -qi 'globally managed' || { echo "ERROR: failed for the wrong reason:"; echo "$out"; exit 1; } + if out=$(sudo pmg config set dependency_cooldown.days 7 2>&1); then echo "ERROR: config set should fail under system config even as root" exit 1 fi + echo "$out" | grep -qi 'globally managed' || { echo "ERROR: root run failed for the wrong reason:"; echo "$out"; exit 1; } echo "SUCCESS: managed config is locked" - name: Doctor reports system install state diff --git a/config/config.go b/config/config.go index 2ecc7d6..5a45fbe 100644 --- a/config/config.go +++ b/config/config.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "os" + "os/user" "path/filepath" "runtime" "time" @@ -610,6 +611,52 @@ func loadConfig() { } } +// configGeteuid is overridable in tests to exercise root path resolution +// without running as root. +var configGeteuid = os.Geteuid + +// rootHomeDir returns root's home from the passwd database. Path resolution +// for root must not consult HOME or XDG_*: sudo and su can preserve the +// invoking user's environment (GitHub runners, sudo -E, su without -), which +// would make root create root-owned state inside that user's home and +// fail-close every later non-root pmg run for them. +func rootHomeDir() (string, error) { + u, err := user.LookupId("0") + if err != nil { + return "", fmt.Errorf("failed to resolve root home directory: %w", err) + } + if u.HomeDir == "" { + return "", fmt.Errorf("root user has no home directory") + } + return u.HomeDir, nil +} + +// rootConfigDir mirrors os.UserConfigDir platform conventions for root's +// passwd home. +func rootConfigDir() (string, error) { + home, err := rootHomeDir() + if err != nil { + return "", err + } + if runtime.GOOS == "darwin" { + return filepath.Join(home, "Library", "Application Support"), nil + } + return filepath.Join(home, ".config"), nil +} + +// rootCacheDir mirrors os.UserCacheDir platform conventions for root's +// passwd home. +func rootCacheDir() (string, error) { + home, err := rootHomeDir() + if err != nil { + return "", err + } + if runtime.GOOS == "darwin" { + return filepath.Join(home, "Library", "Caches"), nil + } + return filepath.Join(home, ".cache"), nil +} + // configDir computes the path to the config directory. func configDir() (string, error) { dir := os.Getenv(pmgConfigDirEnvKey) @@ -617,6 +664,14 @@ func configDir() (string, error) { return dir, nil } + if configGeteuid() == 0 { + base, err := rootConfigDir() + if err != nil { + return "", err + } + return filepath.Join(base, pmgDefaultHomeRelativePath), nil + } + userConfigDir, err := os.UserConfigDir() if err != nil { return "", fmt.Errorf("failed to retrieve user config directory: %w", err) @@ -737,6 +792,14 @@ func cacheDir() (string, error) { } return filepath.Join(baseDir, pmgDefaultHomeRelativePath), nil case "darwin", "linux": + if configGeteuid() == 0 { + base, err := rootCacheDir() + if err != nil { + return "", err + } + return filepath.Join(base, pmgDefaultHomeRelativePath), nil + } + userCacheDir, err := os.UserCacheDir() if err != nil { return "", fmt.Errorf("failed to retrieve user cache directory: %w", err) diff --git a/config/rootdir_test.go b/config/rootdir_test.go new file mode 100644 index 0000000..0df6b93 --- /dev/null +++ b/config/rootdir_test.go @@ -0,0 +1,80 @@ +package config + +import ( + "os/user" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func withEuid(t *testing.T, euid int) { + t.Helper() + orig := configGeteuid + configGeteuid = func() int { return euid } + t.Cleanup(func() { configGeteuid = orig }) +} + +func poisonUserEnv(t *testing.T) { + t.Helper() + t.Setenv("PMG_CONFIG_DIR", "") + t.Setenv("PMG_CACHE_DIR", "") + t.Setenv("HOME", "/home/victim") + t.Setenv("XDG_CONFIG_HOME", "/home/victim/.config") + t.Setenv("XDG_CACHE_HOME", "/home/victim/.cache") +} + +func TestConfigDirAsRootIgnoresPreservedHome(t *testing.T) { + poisonUserEnv(t) + withEuid(t, 0) + + dir, err := configDir() + require.NoError(t, err) + + rootUser, err := user.LookupId("0") + require.NoError(t, err) + assert.True(t, strings.HasPrefix(dir, rootUser.HomeDir), "expected %s under root home %s", dir, rootUser.HomeDir) + assert.NotContains(t, dir, "/home/victim") +} + +func TestConfigDirAsNonRootUsesEnvHome(t *testing.T) { + poisonUserEnv(t) + withEuid(t, 1000) + + dir, err := configDir() + require.NoError(t, err) + assert.Contains(t, dir, "/home/victim") +} + +func TestConfigDirEnvOverrideWinsForRoot(t *testing.T) { + poisonUserEnv(t) + t.Setenv("PMG_CONFIG_DIR", "/custom/pmg") + withEuid(t, 0) + + dir, err := configDir() + require.NoError(t, err) + assert.Equal(t, "/custom/pmg", dir) +} + +func TestCacheDirAsRootIgnoresPreservedHome(t *testing.T) { + poisonUserEnv(t) + withEuid(t, 0) + + dir, err := cacheDir() + require.NoError(t, err) + + rootUser, err := user.LookupId("0") + require.NoError(t, err) + assert.True(t, strings.HasPrefix(dir, rootUser.HomeDir), "expected %s under root home %s", dir, rootUser.HomeDir) + assert.NotContains(t, dir, "/home/victim") +} + +func TestCacheDirAsNonRootUsesEnvHome(t *testing.T) { + poisonUserEnv(t) + withEuid(t, 1000) + + dir, err := cacheDir() + require.NoError(t, err) + assert.Contains(t, dir, "/home/victim") +}