mirror of
https://github.com/safedep/pmg.git
synced 2026-08-03 07:24:09 +02:00
fix: resolve per-user paths from root's own home when running as root
Path resolution trusted HOME (and XDG_*), which sudo and su can preserve from the invoking user (GitHub runners, sudo -E, su without -). Any pmg run as root then created root-owned ~/.config/safedep inside that user's home, and event-log init fail-closed every later non-root pmg/npm/pip run for them. System install made sudo pmg the documented flow, turning this latent bug into the happy path. When euid is 0, configDir and cacheDir now resolve from root's passwd home instead of the environment, so root state lands under /root and user homes are never touched. PMG_CONFIG_DIR/PMG_CACHE_DIR still win, non-root resolution is unchanged, and Windows is unaffected (no euid). Event-log init stays fatal on failure; sudo-run package events are attributed via SUDO_USER and synced by the exit auto-sync as usual. E2E: GitHub runners preserve HOME under sudo, so assert that no sudo pmg run leaks state into the runner's home, and that the managed-config refusal fails for the documented reason rather than a permission brick.
This commit is contained in:
@@ -947,6 +947,21 @@ jobs:
|
|||||||
test -x "/usr/local/lib/pmg/bin/$shim" || { echo "Missing shim: $shim"; exit 1; }
|
test -x "/usr/local/lib/pmg/bin/$shim" || { echo "Missing shim: $shim"; exit 1; }
|
||||||
done
|
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
|
- name: PATH and profile.d activate shims
|
||||||
run: |
|
run: |
|
||||||
# Docker-style: non-login shells need PATH (or source profile.d)
|
# Docker-style: non-login shells need PATH (or source profile.d)
|
||||||
@@ -957,14 +972,18 @@ jobs:
|
|||||||
|
|
||||||
- name: Managed config refuses CLI mutation
|
- name: Managed config refuses CLI mutation
|
||||||
run: |
|
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"
|
echo "ERROR: config set should fail under system config"
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
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"
|
echo "ERROR: config set should fail under system config even as root"
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
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"
|
echo "SUCCESS: managed config is locked"
|
||||||
|
|
||||||
- name: Doctor reports system install state
|
- name: Doctor reports system install state
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
|
"os/user"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"runtime"
|
"runtime"
|
||||||
"time"
|
"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.
|
// configDir computes the path to the config directory.
|
||||||
func configDir() (string, error) {
|
func configDir() (string, error) {
|
||||||
dir := os.Getenv(pmgConfigDirEnvKey)
|
dir := os.Getenv(pmgConfigDirEnvKey)
|
||||||
@@ -617,6 +664,14 @@ func configDir() (string, error) {
|
|||||||
return dir, nil
|
return dir, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if configGeteuid() == 0 {
|
||||||
|
base, err := rootConfigDir()
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
return filepath.Join(base, pmgDefaultHomeRelativePath), nil
|
||||||
|
}
|
||||||
|
|
||||||
userConfigDir, err := os.UserConfigDir()
|
userConfigDir, err := os.UserConfigDir()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", fmt.Errorf("failed to retrieve user config directory: %w", err)
|
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
|
return filepath.Join(baseDir, pmgDefaultHomeRelativePath), nil
|
||||||
case "darwin", "linux":
|
case "darwin", "linux":
|
||||||
|
if configGeteuid() == 0 {
|
||||||
|
base, err := rootCacheDir()
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
return filepath.Join(base, pmgDefaultHomeRelativePath), nil
|
||||||
|
}
|
||||||
|
|
||||||
userCacheDir, err := os.UserCacheDir()
|
userCacheDir, err := os.UserCacheDir()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", fmt.Errorf("failed to retrieve user cache directory: %w", err)
|
return "", fmt.Errorf("failed to retrieve user cache directory: %w", err)
|
||||||
|
|||||||
@@ -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")
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user