fix: harden doctor PATH checks and attribute cloud events by OS user

Doctor now verifies every installed package manager against the shim
directory, and system-install validation only requires a safe parent
directory. Cloud sync records username/uid on invocation context for
multi-user hosts sharing one endpoint.

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Sahilb315
2026-07-13 23:12:02 +05:30
co-authored by Cursor
parent 1c9b16f1fa
commit b1aa217011
10 changed files with 132 additions and 78 deletions
+1 -1
View File
@@ -967,7 +967,7 @@ jobs:
out=$(pmg setup doctor 2>&1 || true) out=$(pmg setup doctor 2>&1 || true)
echo "$out" echo "$out"
echo "$out" | grep -q 'No aliases (system install)' echo "$out" | grep -q 'No aliases (system install)'
echo "$out" | grep -Eq 'npm resolves to system shim|System shim directory is in PATH' echo "$out" | grep -Eq 'Package managers resolve to System shim directory|System shim directory is in PATH'
- name: Non-root user interception via system shims - name: Non-root user interception via system shims
run: | run: |
+55 -44
View File
@@ -304,67 +304,78 @@ func pathIsUnderDir(path, dir string) bool {
return strings.HasPrefix(cleanPath, prefix) return strings.HasPrefix(cleanPath, prefix)
} }
func checkShimInPathResult() doctor.CheckResult { func classifyPackageManagerResolutions(packageManagers []string, shimDir string, lookPath func(string) (string, error)) (underShim, shadowed []string) {
pathEntries := filepath.SplitList(os.Getenv("PATH")) for _, pm := range packageManagers {
systemDir := shim.SystemBinDir() resolved, err := lookPath(pm)
userDir, userDirErr := shim.UserBinDir() if err != nil {
resolved, lookErr := exec.LookPath("npm") continue
}
if lookErr == nil { if pathIsUnderDir(resolved, shimDir) {
if shim.SystemShimsInstalled() && pathIsUnderDir(resolved, systemDir) { underShim = append(underShim, pm)
return doctor.CheckResult{ continue
Status: doctor.StatusPass,
Message: "npm resolves to system shim",
ImpliesInterception: true,
}
}
if userDirErr == nil && pathIsUnderDir(resolved, userDir) {
return doctor.CheckResult{
Status: doctor.StatusPass,
Message: "npm resolves to PMG shim",
ImpliesInterception: true,
}
} }
shadowed = append(shadowed, pm)
} }
return underShim, shadowed
}
if shim.SystemShimsInstalled() && pathContainsDir(pathEntries, systemDir) { func checkShimDirResolution(shimDir, pathLabel string, pathEntries []string) doctor.CheckResult {
if lookErr == nil { underShim, shadowed := classifyPackageManagerResolutions(
alias.DefaultConfig().PackageManagers,
shimDir,
exec.LookPath,
)
if len(shadowed) > 0 {
if pathContainsDir(pathEntries, shimDir) || len(underShim) > 0 {
return doctor.CheckResult{ return doctor.CheckResult{
Status: doctor.StatusWarn, Status: doctor.StatusWarn,
Message: fmt.Sprintf("System shim directory is in PATH, but npm resolves to %s", resolved), Message: fmt.Sprintf("%s resolved outside %s", strings.Join(shadowed, ", "), pathLabel),
} }
} }
return doctor.CheckResult{
Status: doctor.StatusPass,
Message: "System shim directory is in PATH",
ImpliesInterception: true,
}
}
if userDirErr == nil && pathContainsDir(pathEntries, userDir) {
if lookErr == nil {
return doctor.CheckResult{
Status: doctor.StatusWarn,
Message: fmt.Sprintf("Shim directory is in PATH, but npm resolves to %s", resolved),
}
}
return doctor.CheckResult{
Status: doctor.StatusPass,
Message: "Shim directory is in PATH",
ImpliesInterception: true,
}
}
if shim.SystemShimsInstalled() {
return doctor.CheckResult{ return doctor.CheckResult{
Status: doctor.StatusFail, Status: doctor.StatusFail,
Message: "System shim directory not in PATH", Message: fmt.Sprintf("%s not in PATH", pathLabel),
}
}
if len(underShim) > 0 {
return doctor.CheckResult{
Status: doctor.StatusPass,
Message: fmt.Sprintf("Package managers resolve to %s", pathLabel),
ImpliesInterception: true,
}
}
if pathContainsDir(pathEntries, shimDir) {
return doctor.CheckResult{
Status: doctor.StatusPass,
Message: fmt.Sprintf("%s is in PATH", pathLabel),
ImpliesInterception: true,
} }
} }
return doctor.CheckResult{ return doctor.CheckResult{
Status: doctor.StatusFail, Status: doctor.StatusFail,
Message: "Shim directory not in PATH", Message: fmt.Sprintf("%s not in PATH", pathLabel),
} }
} }
func checkShimInPathResult() doctor.CheckResult {
pathEntries := filepath.SplitList(os.Getenv("PATH"))
if shim.SystemShimsInstalled() {
return checkShimDirResolution(shim.SystemBinDir(), "System shim directory", pathEntries)
}
userDir, err := shim.UserBinDir()
if err != nil {
return doctor.CheckResult{
Status: doctor.StatusWarn,
Message: fmt.Sprintf("Could not resolve shim directory: %v", err),
}
}
return checkShimDirResolution(userDir, "Shim directory", pathEntries)
}
func runProtectionChecks(coreResults []doctor.CheckResult) []doctor.CheckResult { func runProtectionChecks(coreResults []doctor.CheckResult) []doctor.CheckResult {
if !isInterceptionActive(coreResults) { if !isInterceptionActive(coreResults) {
var results []doctor.CheckResult var results []doctor.CheckResult
+26 -1
View File
@@ -1,6 +1,7 @@
package setup package setup
import ( import (
"os/exec"
"testing" "testing"
"github.com/safedep/pmg/internal/doctor" "github.com/safedep/pmg/internal/doctor"
@@ -47,10 +48,34 @@ func TestShimInPathImpliesInterception(t *testing.T) {
{ {
Name: checkShimInPath, Name: checkShimInPath,
Status: doctor.StatusPass, Status: doctor.StatusPass,
Message: "npm resolves to system shim", Message: "Package managers resolve to System shim directory",
ImpliesInterception: true, ImpliesInterception: true,
}, },
} }
assert.True(t, isInterceptionActive(results)) assert.True(t, isInterceptionActive(results))
} }
func TestClassifyPackageManagerResolutions(t *testing.T) {
shimDir := "/usr/local/lib/pmg/bin"
lookPath := func(name string) (string, error) {
switch name {
case "npm":
return shimDir + "/npm", nil
case "pip":
return "/usr/bin/pip", nil
case "uv":
return "", exec.ErrNotFound
default:
return "", exec.ErrNotFound
}
}
under, shadowed := classifyPackageManagerResolutions(
[]string{"npm", "pip", "uv"},
shimDir,
lookPath,
)
assert.Equal(t, []string{"npm"}, under)
assert.Equal(t, []string{"pip"}, shadowed)
}
+1 -1
View File
@@ -90,7 +90,7 @@ Optional lockdown (`global_lockdown: true`) is documented in [config.md](./confi
## Limitations ## Limitations
- **Virtualenv.** After `source .venv/bin/activate`, bare `pip` uses the venv binary and skips PMG shims. Call `pmg pip …` explicitly. - **Virtualenv.** After `source .venv/bin/activate`, bare `pip` uses the venv binary and skips PMG shims. Call `pmg pip …` explicitly.
- **Version managers.** Tools like nvm, pyenv, volta, and asdf often prepend their own bin directories from shell rc files that run after `/etc/profile.d`. That can put real `npm`/`pip` ahead of PMG shims even when the shim directory is on `PATH`. Prefer putting `/usr/local/lib/pmg/bin` first in a durable `ENV PATH` / login PATH, or call `pmg npm` / `pmg pip` explicitly. `pmg setup doctor` warns when `npm` resolves outside the shim directory. - **Version managers.** Tools like nvm, pyenv, volta, and asdf often prepend their own bin directories from shell rc files that run after `/etc/profile.d`. That can put real `npm`/`pip` ahead of PMG shims even when the shim directory is on `PATH`. Prefer putting `/usr/local/lib/pmg/bin` first in a durable `ENV PATH` / login PATH, or call `pmg npm` / `pmg pip` explicitly. `pmg setup doctor` warns for any supported package manager that resolves outside the shim directory.
- **No shell aliases.** System install only installs PATH shims. There is no `~/.pmg.rc` alias layer. - **No shell aliases.** System install only installs PATH shims. There is no `~/.pmg.rc` alias layer.
- **Config changes.** `pmg config set` and `pmg config edit` are unavailable while the system config is active. Edit `/etc/safedep/pmg/config.yml` as root, or redeploy the file. - **Config changes.** `pmg config set` and `pmg config edit` are unavailable while the system config is active. Edit `/etc/safedep/pmg/config.yml` as root, or redeploy the file.
- **Custom sandbox `policy_templates`.** Relative paths in the system config resolve under each user's config directory, not `/etc/safedep/pmg`. Prefer absolute paths. - **Custom sandbox `policy_templates`.** Relative paths in the system config resolve under each user's config directory, not `/etc/safedep/pmg`. Prefer absolute paths.
+1 -1
View File
@@ -4,7 +4,7 @@ go 1.25.1
require ( require (
buf.build/gen/go/safedep/api/grpc/go v1.6.2-20260528074646-b9e182189444.1 buf.build/gen/go/safedep/api/grpc/go v1.6.2-20260528074646-b9e182189444.1
buf.build/gen/go/safedep/api/protocolbuffers/go v1.36.11-20260620084912-77c7bb923ddb.1 buf.build/gen/go/safedep/api/protocolbuffers/go v1.36.11-20260713161921-716fa3011a21.1
github.com/Masterminds/semver v1.5.0 github.com/Masterminds/semver v1.5.0
github.com/elazarl/goproxy v1.8.1 github.com/elazarl/goproxy v1.8.1
github.com/fatih/color v1.18.0 github.com/fatih/color v1.18.0
+2
View File
@@ -6,6 +6,8 @@ buf.build/gen/go/safedep/api/grpc/go v1.6.2-20260528074646-b9e182189444.1 h1:zpj
buf.build/gen/go/safedep/api/grpc/go v1.6.2-20260528074646-b9e182189444.1/go.mod h1:8pVZh4owzo4YXcKvFvdWEYGr4k/1VHGR0h39XHsuHD4= buf.build/gen/go/safedep/api/grpc/go v1.6.2-20260528074646-b9e182189444.1/go.mod h1:8pVZh4owzo4YXcKvFvdWEYGr4k/1VHGR0h39XHsuHD4=
buf.build/gen/go/safedep/api/protocolbuffers/go v1.36.11-20260620084912-77c7bb923ddb.1 h1:AYEqYqmDeF99lbHGJYjyzACLhJhwF9cJVcNCdl3vwYQ= buf.build/gen/go/safedep/api/protocolbuffers/go v1.36.11-20260620084912-77c7bb923ddb.1 h1:AYEqYqmDeF99lbHGJYjyzACLhJhwF9cJVcNCdl3vwYQ=
buf.build/gen/go/safedep/api/protocolbuffers/go v1.36.11-20260620084912-77c7bb923ddb.1/go.mod h1:I8E+sZXJNqzWBtSlRGCoiEorLSRiix50h2R/66aBzME= buf.build/gen/go/safedep/api/protocolbuffers/go v1.36.11-20260620084912-77c7bb923ddb.1/go.mod h1:I8E+sZXJNqzWBtSlRGCoiEorLSRiix50h2R/66aBzME=
buf.build/gen/go/safedep/api/protocolbuffers/go v1.36.11-20260713161921-716fa3011a21.1 h1:k3kSCmCfcA8kJcI76f0Tj7JD9bx0U4EJuWvBrNO6JXY=
buf.build/gen/go/safedep/api/protocolbuffers/go v1.36.11-20260713161921-716fa3011a21.1/go.mod h1:I8E+sZXJNqzWBtSlRGCoiEorLSRiix50h2R/66aBzME=
cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
github.com/BurntSushi/toml v1.5.0 h1:W5quZX/G/csjUnuI8SUYlsHs9M38FC7znL0lIO+DvMg= github.com/BurntSushi/toml v1.5.0 h1:W5quZX/G/csjUnuI8SUYlsHs9M38FC7znL0lIO+DvMg=
+8 -1
View File
@@ -5,6 +5,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"os" "os"
"os/user"
"strings" "strings"
controltowerv1 "buf.build/gen/go/safedep/api/protocolbuffers/go/safedep/messages/controltower/v1" controltowerv1 "buf.build/gen/go/safedep/api/protocolbuffers/go/safedep/messages/controltower/v1"
@@ -17,7 +18,7 @@ import (
type cloudSink struct { type cloudSink struct {
*SyncClientBundle *SyncClientBundle
invocationID string invocationID string
ciResolver CloudSinkCIResolver ciResolver CloudSinkCIResolver
command string command string
workingDir string workingDir string
} }
@@ -94,6 +95,12 @@ func (s *cloudSink) buildInvocationContext() *controltowerv1.EndpointInvocationC
ctx.SetCommand(s.command) ctx.SetCommand(s.command)
ctx.SetWorkingDirectory(s.workingDir) ctx.SetWorkingDirectory(s.workingDir)
u, err := user.Current()
if err == nil {
ctx.SetUsername(u.Username)
ctx.SetUsernameUid(u.Uid)
}
if s.ciResolver != nil { if s.ciResolver != nil {
ci := &controltowerv1.EndpointCIContext{} ci := &controltowerv1.EndpointCIContext{}
ci.SetProvider(s.ciResolver.Provider()) ci.SetProvider(s.ciResolver.Provider())
+2
View File
@@ -154,4 +154,6 @@ func TestCloudSinkSetsInvocationContextOnSessionComplete(t *testing.T) {
require.NotNil(t, invCtx, "session complete event must have invocation context") require.NotNil(t, invCtx, "session complete event must have invocation context")
assert.Contains(t, invCtx.GetCommand(), "npm") assert.Contains(t, invCtx.GetCommand(), "npm")
assert.NotEmpty(t, invCtx.GetWorkingDirectory()) assert.NotEmpty(t, invCtx.GetWorkingDirectory())
assert.NotEmpty(t, invCtx.GetUsername())
assert.NotEmpty(t, invCtx.GetUsernameUid())
} }
+35 -28
View File
@@ -21,7 +21,7 @@ var (
systemBinDirOverride string systemBinDirOverride string
systemProfilePathOverride string systemProfilePathOverride string
// systemExecutableOwnershipCheck requires root ownership of the binary and // systemExecutableOwnershipCheck requires root ownership of the binary and
// its parent directories. Disabled in tests that cannot create root-owned files. // its parent directory. Disabled in tests that cannot create root-owned files.
systemExecutableOwnershipCheck = true systemExecutableOwnershipCheck = true
) )
@@ -80,9 +80,9 @@ func newSystemShimManager(validateExecutable bool) (*ShimManager, error) {
} }
// validateSystemExecutable rejects binaries unsafe for system-wide shims. // validateSystemExecutable rejects binaries unsafe for system-wide shims.
// System shims hard-code this path, so it must be world-executable, not // Shims hard-code this path, so the binary must be executable by all users,
// group/other-writable, and (when ownership checks are enabled) root-owned // not writable by group/others, and owned by root in a root-owned parent
// under a root-owned, non-group/other-writable directory chain. // directory that is not writable by group/others.
func validateSystemExecutable(path string) error { func validateSystemExecutable(path string) error {
info, err := os.Stat(path) info, err := os.Stat(path)
if err != nil { if err != nil {
@@ -90,18 +90,25 @@ func validateSystemExecutable(path string) error {
} }
perm := info.Mode().Perm() perm := info.Mode().Perm()
if perm&0o001 == 0 {
// Other users must be able to exec the hard-coded pmg path from system shims.
otherExecute := os.FileMode(0o001)
// Group/other write would let another account replace the binary.
groupOrOtherWrite := os.FileMode(0o022)
if perm&otherExecute == 0 {
return fmt.Errorf("pmg executable %s is not executable by all users", path) return fmt.Errorf("pmg executable %s is not executable by all users", path)
} }
if perm&0o022 != 0 { if perm&groupOrOtherWrite != 0 {
return fmt.Errorf("pmg executable %s is writable by group or others", path) return fmt.Errorf("pmg executable %s is writable by group or others", path)
} }
if systemExecutableOwnershipCheck { if systemExecutableOwnershipCheck {
// Root ownership of the binary and its parent blocks non-root replacement.
if err := requireRootOwnedPath(path, info); err != nil { if err := requireRootOwnedPath(path, info); err != nil {
return err return err
} }
if err := requireSafeAncestorDirs(filepath.Dir(path)); err != nil { if err := requireSafeParentDir(filepath.Dir(path)); err != nil {
return err return err
} }
} }
@@ -119,28 +126,28 @@ func requireRootOwnedPath(path string, info os.FileInfo) error {
return nil return nil
} }
func requireSafeAncestorDirs(dir string) error { func requireSafeParentDir(dir string) error {
for { info, err := os.Stat(dir)
info, err := os.Stat(dir) if err != nil {
if err != nil { return fmt.Errorf("failed to inspect directory %s: %w", dir, err)
return fmt.Errorf("failed to inspect directory %s: %w", dir, err)
}
if info.Mode().Perm()&0o022 != 0 {
return fmt.Errorf("directory %s on pmg executable path is writable by group or others", dir)
}
uid, ok := fileOwnerUID(info)
if !ok {
return fmt.Errorf("cannot determine owner of directory %s", dir)
}
if uid != 0 {
return fmt.Errorf("directory %s on pmg executable path must be owned by root", dir)
}
parent := filepath.Dir(dir)
if parent == dir {
return nil
}
dir = parent
} }
groupOrOtherWrite := os.FileMode(0o022)
if info.Mode().Perm()&groupOrOtherWrite != 0 {
return fmt.Errorf("directory %s containing pmg executable is writable by group or others", dir)
}
uid, ok := fileOwnerUID(info)
if !ok {
return fmt.Errorf("cannot determine owner of directory %s", dir)
}
if uid != 0 {
return fmt.Errorf("directory %s containing pmg executable must be owned by root", dir)
}
return nil
} }
// SystemShimsInstalled reports whether the system shim directory contains at // SystemShimsInstalled reports whether the system shim directory contains at
+1 -1
View File
@@ -38,7 +38,7 @@ func PrintSetupSystemInstallCmdInfo(shimBinDir, configDir, profilePath string) {
fmt.Printf(" %s\n", Colors.Dim(fmt.Sprintf("Shims: %s", shimBinDir))) fmt.Printf(" %s\n", Colors.Dim(fmt.Sprintf("Shims: %s", shimBinDir)))
fmt.Printf(" %s\n", Colors.Dim(fmt.Sprintf("Config: %s", configDir))) fmt.Printf(" %s\n", Colors.Dim(fmt.Sprintf("Config: %s", configDir)))
fmt.Printf(" %s\n", Colors.Dim(fmt.Sprintf("Profile: %s", profilePath))) fmt.Printf(" %s\n", Colors.Dim(fmt.Sprintf("Profile: %s", profilePath)))
fmt.Printf(" %s\n", Colors.Dim(fmt.Sprintf("Per-user config files are now ignored; edit %s/config.yml as root.", configDir))) fmt.Printf(" %s\n", Colors.Dim("Per-user config files are now ignored."))
fmt.Printf("\n%s For Docker builds (RUN does not source profile.d), add:\n", Colors.Dim("")) fmt.Printf("\n%s For Docker builds (RUN does not source profile.d), add:\n", Colors.Dim(""))
fmt.Printf(" %s\n", Colors.Bold(fmt.Sprintf(`ENV PATH="%s:$PATH"`, shimBinDir))) fmt.Printf(" %s\n", Colors.Bold(fmt.Sprintf(`ENV PATH="%s:$PATH"`, shimBinDir)))
fmt.Printf("%s Login shells pick up PATH from profile.d. After venv activate, use `pmg pip`.\n", Colors.Dim("")) fmt.Printf("%s Login shells pick up PATH from profile.d. After venv activate, use `pmg pip`.\n", Colors.Dim(""))