mirror of
https://github.com/safedep/pmg.git
synced 2026-08-03 07:24:09 +02:00
fix: actionable remedy for root-created per-user config dir
A pmg run as root with a preserved HOME (GitHub runners, sudo -E, su without -) creates the invoking user's ~/.config/safedep as root-owned, and event-log init then fail-closes every later non-root command. Make that state self-solvable: - event-log init permission errors exit with a usefulerror naming the likely cause and the chown fix instead of a bare fatal - pmg setup doctor probes event-log dir writability and reports the same fix via a new per-result Fix override - document the mechanism and remedy in system-install.md, along with the binary ownership requirements for --system - consolidate this branch's doctor tests into doctor_test.go
This commit is contained in:
+51
-23
@@ -7,6 +7,7 @@ import (
|
|||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
|
"github.com/safedep/dry/log"
|
||||||
"github.com/safedep/pmg/config"
|
"github.com/safedep/pmg/config"
|
||||||
"github.com/safedep/pmg/internal/alias"
|
"github.com/safedep/pmg/internal/alias"
|
||||||
"github.com/safedep/pmg/internal/doctor"
|
"github.com/safedep/pmg/internal/doctor"
|
||||||
@@ -94,29 +95,7 @@ func runCoreChecks(cfg *config.RuntimeConfig) []doctor.CheckResult {
|
|||||||
Name: checkEventLogDir,
|
Name: checkEventLogDir,
|
||||||
Category: "Configuration",
|
Category: "Configuration",
|
||||||
Run: func() doctor.CheckResult {
|
Run: func() doctor.CheckResult {
|
||||||
if cfg.Config.SkipEventLogging {
|
return checkEventLogDirResult(cfg.Config.SkipEventLogging, cfg.EventLogDir(), cfg.ConfigDir())
|
||||||
return doctor.CheckResult{
|
|
||||||
Status: doctor.StatusWarn,
|
|
||||||
Message: "Event logging is disabled",
|
|
||||||
}
|
|
||||||
}
|
|
||||||
info, err := os.Stat(cfg.EventLogDir())
|
|
||||||
if err != nil {
|
|
||||||
return doctor.CheckResult{
|
|
||||||
Status: doctor.StatusFail,
|
|
||||||
Message: "Event log directory not found",
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if !info.IsDir() {
|
|
||||||
return doctor.CheckResult{
|
|
||||||
Status: doctor.StatusFail,
|
|
||||||
Message: "Event log path is not a directory",
|
|
||||||
}
|
|
||||||
}
|
|
||||||
return doctor.CheckResult{
|
|
||||||
Status: doctor.StatusPass,
|
|
||||||
Message: "Event log directory found",
|
|
||||||
}
|
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
@@ -278,6 +257,52 @@ func runCoreChecks(cfg *config.RuntimeConfig) []doctor.CheckResult {
|
|||||||
return doctor.RunChecks(checks)
|
return doctor.RunChecks(checks)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// 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 common cause is a root or
|
||||||
|
// sudo run having created the per-user directory as root, hence the chown fix.
|
||||||
|
func checkEventLogDirResult(skipEventLogging bool, logDir, configDir string) doctor.CheckResult {
|
||||||
|
if skipEventLogging {
|
||||||
|
return doctor.CheckResult{
|
||||||
|
Status: doctor.StatusWarn,
|
||||||
|
Message: "Event logging is disabled",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
info, err := os.Stat(logDir)
|
||||||
|
if err != nil {
|
||||||
|
return doctor.CheckResult{
|
||||||
|
Status: doctor.StatusFail,
|
||||||
|
Message: "Event log directory not found",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !info.IsDir() {
|
||||||
|
return doctor.CheckResult{
|
||||||
|
Status: doctor.StatusFail,
|
||||||
|
Message: "Event log path is not a directory",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
probe, err := os.CreateTemp(logDir, ".pmg-doctor-*")
|
||||||
|
if err != nil {
|
||||||
|
return doctor.CheckResult{
|
||||||
|
Status: doctor.StatusFail,
|
||||||
|
Message: "Event log directory not writable",
|
||||||
|
Fix: fmt.Sprintf("sudo chown -R $(id -un) %s", configDir),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if err := probe.Close(); err != nil {
|
||||||
|
log.Warnf("failed to close doctor probe file: %v", err)
|
||||||
|
}
|
||||||
|
if err := os.Remove(probe.Name()); err != nil {
|
||||||
|
log.Warnf("failed to remove doctor probe file: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return doctor.CheckResult{
|
||||||
|
Status: doctor.StatusPass,
|
||||||
|
Message: "Event log directory found",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func pathContainsDir(pathEntries []string, dir string) bool {
|
func pathContainsDir(pathEntries []string, dir string) bool {
|
||||||
if dir == "" {
|
if dir == "" {
|
||||||
return false
|
return false
|
||||||
@@ -520,6 +545,9 @@ func printResults(results []doctor.CheckResult) {
|
|||||||
fix := ui.Colors.Dim("—")
|
fix := ui.Colors.Dim("—")
|
||||||
if r.Status != doctor.StatusPass {
|
if r.Status != doctor.StatusPass {
|
||||||
fix = fixHint(r.Name)
|
fix = fixHint(r.Name)
|
||||||
|
if r.Fix != "" {
|
||||||
|
fix = r.Fix
|
||||||
|
}
|
||||||
}
|
}
|
||||||
rows = append(rows, []string{
|
rows = append(rows, []string{
|
||||||
statusBadge(r.Status),
|
statusBadge(r.Status),
|
||||||
|
|||||||
@@ -1,11 +1,14 @@
|
|||||||
package setup
|
package setup
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"os"
|
||||||
"os/exec"
|
"os/exec"
|
||||||
|
"path/filepath"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/safedep/pmg/internal/doctor"
|
"github.com/safedep/pmg/internal/doctor"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestPathContainsDir(t *testing.T) {
|
func TestPathContainsDir(t *testing.T) {
|
||||||
@@ -102,3 +105,48 @@ func TestClassifyPackageManagerResolutionsAcceptsEitherShimDir(t *testing.T) {
|
|||||||
assert.ElementsMatch(t, []string{"npm", "pip"}, under)
|
assert.ElementsMatch(t, []string{"npm", "pip"}, under)
|
||||||
assert.Equal(t, []string{"yarn"}, shadowed)
|
assert.Equal(t, []string{"yarn"}, shadowed)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestCheckEventLogDirResult(t *testing.T) {
|
||||||
|
configDir := "/home/dev/.config/safedep/pmg"
|
||||||
|
|
||||||
|
t.Run("skipped when event logging disabled", func(t *testing.T) {
|
||||||
|
result := checkEventLogDirResult(true, t.TempDir(), configDir)
|
||||||
|
assert.Equal(t, doctor.StatusWarn, result.Status)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("missing directory fails", func(t *testing.T) {
|
||||||
|
result := checkEventLogDirResult(false, filepath.Join(t.TempDir(), "absent"), configDir)
|
||||||
|
assert.Equal(t, doctor.StatusFail, result.Status)
|
||||||
|
assert.Equal(t, "Event log directory not found", result.Message)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("file instead of directory fails", func(t *testing.T) {
|
||||||
|
path := filepath.Join(t.TempDir(), "logs")
|
||||||
|
require.NoError(t, os.WriteFile(path, []byte("x"), 0o644))
|
||||||
|
|
||||||
|
result := checkEventLogDirResult(false, path, configDir)
|
||||||
|
assert.Equal(t, doctor.StatusFail, result.Status)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("writable directory passes", func(t *testing.T) {
|
||||||
|
result := checkEventLogDirResult(false, t.TempDir(), configDir)
|
||||||
|
assert.Equal(t, doctor.StatusPass, result.Status)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("unwritable directory fails with chown fix", func(t *testing.T) {
|
||||||
|
if os.Geteuid() == 0 {
|
||||||
|
t.Skip("running as root: directory permissions are not enforced")
|
||||||
|
}
|
||||||
|
dir := t.TempDir()
|
||||||
|
require.NoError(t, os.Chmod(dir, 0o555))
|
||||||
|
t.Cleanup(func() {
|
||||||
|
require.NoError(t, os.Chmod(dir, 0o755))
|
||||||
|
})
|
||||||
|
|
||||||
|
result := checkEventLogDirResult(false, dir, configDir)
|
||||||
|
assert.Equal(t, doctor.StatusFail, result.Status)
|
||||||
|
assert.Equal(t, "Event log directory not writable", result.Message)
|
||||||
|
assert.Contains(t, result.Fix, "sudo chown -R")
|
||||||
|
assert.Contains(t, result.Fix, configDir)
|
||||||
|
})
|
||||||
|
}
|
||||||
+14
-5
@@ -6,16 +6,17 @@ Use system install when one machine or image should protect every user account:
|
|||||||
sudo pmg setup install --system
|
sudo pmg setup install --system
|
||||||
```
|
```
|
||||||
|
|
||||||
Requires Linux and root. To uninstall:
|
Requires Linux and root. Because every user's shims execute the PMG binary by its absolute path, `--system` validates it first: the binary must be **root-owned**, world-executable, and not writable by group or others, and it must sit in a **root-owned directory** that is not world-writable. Install PMG as root into a standard path such as `/usr/local/bin`; a user-local build (e.g. `~/go/bin/pmg`) is rejected.
|
||||||
|
|
||||||
|
Per-user `pmg setup install` remains available and does not conflict with a system install.
|
||||||
|
|
||||||
|
To uninstall:
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
sudo pmg setup remove --system
|
sudo pmg setup remove --system
|
||||||
sudo pmg setup remove --system --config-file # also remove the system config file
|
sudo pmg setup remove --system --config-file # also remove the system config file
|
||||||
```
|
```
|
||||||
|
|
||||||
Per-user `pmg setup install` remains available and does not conflict with a system install.
|
|
||||||
The PMG executable used during setup must be executable by every user. Install PMG under a system path such as `/usr/local/bin`.
|
|
||||||
|
|
||||||
## Files created
|
## Files created
|
||||||
|
|
||||||
|
|
||||||
@@ -112,10 +113,18 @@ Shared policy lives under `/etc/safedep/pmg`. Runtime data stays per user:
|
|||||||
|
|
||||||
You can relocate these with `PMG_CONFIG_DIR` and `PMG_CACHE_DIR`.
|
You can relocate these with `PMG_CONFIG_DIR` and `PMG_CACHE_DIR`.
|
||||||
|
|
||||||
The invoking user must be able to write their config directory.
|
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).
|
||||||
|
|
||||||
In Docker images, avoid creating `/home/<user>/.config/safedep` as root during the build. Either fix ownership for the runtime user, or set `PMG_CONFIG_DIR` to a writable location.
|
In Docker images, avoid creating `/home/<user>/.config/safedep` as root during the build. Either fix ownership for the runtime user, or set `PMG_CONFIG_DIR` to a writable location.
|
||||||
|
|
||||||
|
If every `pmg` command fails with `permission denied` on the event log, a root run created the per-user directory as root. This happens where `sudo` preserves `HOME` (GitHub-hosted runners, `sudo -E`, `su` without `-`) and in images that set `ENV HOME` before dropping root. Restore ownership:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
sudo chown -R $(id -un) ~/.config/safedep
|
||||||
|
```
|
||||||
|
|
||||||
|
`pmg setup doctor` detects an unwritable event log directory and prints the same fix.
|
||||||
|
|
||||||
For cloud sync, enable cloud in the system config and provide credentials (`SAFEDEP_API_KEY` and `SAFEDEP_TENANT_ID`, or a keychain login on developer machines).
|
For cloud sync, enable cloud in the system config and provide credentials (`SAFEDEP_API_KEY` and `SAFEDEP_TENANT_ID`, or a keychain login on developer machines).
|
||||||
|
|
||||||
## Certificates
|
## Certificates
|
||||||
|
|||||||
@@ -14,6 +14,8 @@ type CheckResult struct {
|
|||||||
Status CheckStatus
|
Status CheckStatus
|
||||||
Message string
|
Message string
|
||||||
ImpliesInterception bool
|
ImpliesInterception bool
|
||||||
|
// Fix overrides the check's static fix hint for this specific result.
|
||||||
|
Fix string
|
||||||
}
|
}
|
||||||
|
|
||||||
type Check struct {
|
type Check struct {
|
||||||
|
|||||||
@@ -1,12 +1,15 @@
|
|||||||
package main
|
package main
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"io/fs"
|
||||||
"os"
|
"os"
|
||||||
"runtime"
|
"runtime"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"github.com/safedep/dry/log"
|
"github.com/safedep/dry/log"
|
||||||
|
"github.com/safedep/dry/usefulerror"
|
||||||
"github.com/safedep/pmg/cmd/cloud"
|
"github.com/safedep/pmg/cmd/cloud"
|
||||||
configCmd "github.com/safedep/pmg/cmd/config"
|
configCmd "github.com/safedep/pmg/cmd/config"
|
||||||
"github.com/safedep/pmg/cmd/executors"
|
"github.com/safedep/pmg/cmd/executors"
|
||||||
@@ -19,6 +22,7 @@ import (
|
|||||||
"github.com/safedep/pmg/cmd/setup"
|
"github.com/safedep/pmg/cmd/setup"
|
||||||
"github.com/safedep/pmg/cmd/version"
|
"github.com/safedep/pmg/cmd/version"
|
||||||
"github.com/safedep/pmg/config"
|
"github.com/safedep/pmg/config"
|
||||||
|
"github.com/safedep/pmg/errcodes"
|
||||||
"github.com/safedep/pmg/internal/analytics"
|
"github.com/safedep/pmg/internal/analytics"
|
||||||
"github.com/safedep/pmg/internal/audit"
|
"github.com/safedep/pmg/internal/audit"
|
||||||
"github.com/safedep/pmg/internal/eventlog"
|
"github.com/safedep/pmg/internal/eventlog"
|
||||||
@@ -103,7 +107,7 @@ func main() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if eventlogErr != nil {
|
if eventlogErr != nil {
|
||||||
ui.Fatalf("failed to initialize event logging: %v", eventlogErr)
|
ui.ErrorExit(eventlogInitError(eventlogErr))
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := audit.Initialize(config.Get()); err != nil {
|
if err := audit.Initialize(config.Get()); err != nil {
|
||||||
@@ -215,6 +219,25 @@ func main() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// eventlogInitError classifies event-log init failures. Event logging is
|
||||||
|
// mandatory, so init failure stays fatal; the permission case gets an
|
||||||
|
// actionable remedy because the common cause is a root or sudo run having
|
||||||
|
// created the per-user directory as root (some environments preserve HOME
|
||||||
|
// under sudo).
|
||||||
|
func eventlogInitError(err error) error {
|
||||||
|
if errors.Is(err, fs.ErrPermission) {
|
||||||
|
return usefulerror.NewUsefulError().
|
||||||
|
WithCode(errcodes.PermissionDenied).
|
||||||
|
WithHumanError("event logging is required but its directory is not writable").
|
||||||
|
WithHelp(fmt.Sprintf("If a root or sudo run created it, restore ownership: sudo chown -R $(id -un) %s", config.Get().ConfigDir())).
|
||||||
|
Wrap(err)
|
||||||
|
}
|
||||||
|
return usefulerror.NewUsefulError().
|
||||||
|
WithCode(errcodes.Lifecycle).
|
||||||
|
WithHumanError("failed to initialize event logging").
|
||||||
|
Wrap(err)
|
||||||
|
}
|
||||||
|
|
||||||
func logDebugContext() {
|
func logDebugContext() {
|
||||||
cfg := config.Get()
|
cfg := config.Get()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user