mirror of
https://github.com/safedep/pmg.git
synced 2026-08-03 07:24:09 +02:00
fix: triage the unwritable config dir remedy by cause
The chown hint is only correct when another account created files inside the current user's own home. When a leaked HOME or XDG_CONFIG_HOME points at another user's home (e.g. sudo -u on GitHub runners), following it would chown that user's directory and brick their pmg instead. Classify the failure against the passwd home, which the leaked environment cannot influence, and prescribe: - dir inside own home: restore ownership with chown - dir outside own home: fix the leaked environment, never chown - explicit PMG_CONFIG_DIR: make it writable Used by both the fatal event-log error and the doctor check, and the docs troubleshooting now carries the same two-case triage.
This commit is contained in:
+4
-3
@@ -259,8 +259,9 @@ func runCoreChecks(cfg *config.RuntimeConfig) []doctor.CheckResult {
|
|||||||
|
|
||||||
// checkEventLogDirResult is the testable core of the event-log dir check.
|
// checkEventLogDirResult is the testable core of the event-log dir check.
|
||||||
// Event logging is mandatory (init failure is fatal), so an unwritable dir
|
// 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
|
// fail-closes every pmg command for this user. The remedy is triaged: chown
|
||||||
// sudo run having created the per-user directory as root, hence the chown fix.
|
// when another account created files in this user's home, an environment fix
|
||||||
|
// when a leaked HOME/XDG_CONFIG_HOME points at another user's home.
|
||||||
func checkEventLogDirResult(skipEventLogging bool, logDir, configDir string) doctor.CheckResult {
|
func checkEventLogDirResult(skipEventLogging bool, logDir, configDir string) doctor.CheckResult {
|
||||||
if skipEventLogging {
|
if skipEventLogging {
|
||||||
return doctor.CheckResult{
|
return doctor.CheckResult{
|
||||||
@@ -287,7 +288,7 @@ func checkEventLogDirResult(skipEventLogging bool, logDir, configDir string) doc
|
|||||||
return doctor.CheckResult{
|
return doctor.CheckResult{
|
||||||
Status: doctor.StatusFail,
|
Status: doctor.StatusFail,
|
||||||
Message: "Event log directory not writable",
|
Message: "Event log directory not writable",
|
||||||
Fix: fmt.Sprintf("sudo chown -R $(id -un) %s", configDir),
|
Fix: config.UnwritableConfigDirRemedy(configDir),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if err := probe.Close(); err != nil {
|
if err := probe.Close(); err != nil {
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ import (
|
|||||||
"path/filepath"
|
"path/filepath"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"github.com/safedep/pmg/config"
|
||||||
"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"
|
"github.com/stretchr/testify/require"
|
||||||
@@ -133,7 +134,7 @@ func TestCheckEventLogDirResult(t *testing.T) {
|
|||||||
assert.Equal(t, doctor.StatusPass, result.Status)
|
assert.Equal(t, doctor.StatusPass, result.Status)
|
||||||
})
|
})
|
||||||
|
|
||||||
t.Run("unwritable directory fails with chown fix", func(t *testing.T) {
|
t.Run("unwritable directory fails with triaged remedy", func(t *testing.T) {
|
||||||
if os.Geteuid() == 0 {
|
if os.Geteuid() == 0 {
|
||||||
t.Skip("running as root: directory permissions are not enforced")
|
t.Skip("running as root: directory permissions are not enforced")
|
||||||
}
|
}
|
||||||
@@ -146,7 +147,6 @@ func TestCheckEventLogDirResult(t *testing.T) {
|
|||||||
result := checkEventLogDirResult(false, dir, configDir)
|
result := checkEventLogDirResult(false, dir, configDir)
|
||||||
assert.Equal(t, doctor.StatusFail, result.Status)
|
assert.Equal(t, doctor.StatusFail, result.Status)
|
||||||
assert.Equal(t, "Event log directory not writable", result.Message)
|
assert.Equal(t, "Event log directory not writable", result.Message)
|
||||||
assert.Contains(t, result.Fix, "sudo chown -R")
|
assert.Equal(t, config.UnwritableConfigDirRemedy(configDir), result.Fix)
|
||||||
assert.Contains(t, result.Fix, configDir)
|
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ import (
|
|||||||
"os/user"
|
"os/user"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"runtime"
|
"runtime"
|
||||||
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
_ "embed"
|
_ "embed"
|
||||||
@@ -657,6 +658,42 @@ func rootCacheDir() (string, error) {
|
|||||||
return filepath.Join(home, ".cache"), nil
|
return filepath.Join(home, ".cache"), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// realUserHomeDir returns the current user's home from the passwd database,
|
||||||
|
// ignoring HOME and XDG_* env vars that may be leaked from another account.
|
||||||
|
// Overridable in tests.
|
||||||
|
var realUserHomeDir = func() (string, error) {
|
||||||
|
u, err := user.Current()
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
return u.HomeDir, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// UnwritableConfigDirRemedy returns actionable help for a per-user config or
|
||||||
|
// event-log directory the current user cannot write. The wrong remedy is
|
||||||
|
// harmful: chown-ing a directory that belongs to another account steals it and
|
||||||
|
// bricks that account instead, so chown is only suggested when the directory
|
||||||
|
// is inside the current user's real home.
|
||||||
|
func UnwritableConfigDirRemedy(dir string) string {
|
||||||
|
if os.Getenv(pmgConfigDirEnvKey) != "" {
|
||||||
|
return fmt.Sprintf("PMG_CONFIG_DIR points at %s; make it writable by your user", dir)
|
||||||
|
}
|
||||||
|
|
||||||
|
home, err := realUserHomeDir()
|
||||||
|
if err == nil && home != "" && !pathWithinDir(dir, home) {
|
||||||
|
return fmt.Sprintf(
|
||||||
|
"pmg resolved its config directory to %s, outside your home (%s): HOME or XDG_CONFIG_HOME leaked from another account (e.g. sudo -u). Fix the environment, e.g. export XDG_CONFIG_HOME=\"$HOME/.config\"; do not chown another user's directory",
|
||||||
|
dir, home)
|
||||||
|
}
|
||||||
|
|
||||||
|
return fmt.Sprintf("If a root or sudo run created it, restore ownership: sudo chown -R $(id -un) %s", dir)
|
||||||
|
}
|
||||||
|
|
||||||
|
func pathWithinDir(path, dir string) bool {
|
||||||
|
cleanPath, cleanDir := filepath.Clean(path), filepath.Clean(dir)
|
||||||
|
return cleanPath == cleanDir || strings.HasPrefix(cleanPath, cleanDir+string(os.PathSeparator))
|
||||||
|
}
|
||||||
|
|
||||||
// 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)
|
||||||
|
|||||||
@@ -0,0 +1,51 @@
|
|||||||
|
package config
|
||||||
|
|
||||||
|
import (
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
)
|
||||||
|
|
||||||
|
func withRealUserHome(t *testing.T, home string) {
|
||||||
|
t.Helper()
|
||||||
|
orig := realUserHomeDir
|
||||||
|
realUserHomeDir = func() (string, error) { return home, nil }
|
||||||
|
t.Cleanup(func() { realUserHomeDir = orig })
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestUnwritableConfigDirRemedy(t *testing.T) {
|
||||||
|
t.Run("dir inside real home suggests chown", func(t *testing.T) {
|
||||||
|
t.Setenv("PMG_CONFIG_DIR", "")
|
||||||
|
withRealUserHome(t, "/home/alice")
|
||||||
|
|
||||||
|
remedy := UnwritableConfigDirRemedy("/home/alice/.config/safedep/pmg")
|
||||||
|
assert.Contains(t, remedy, "sudo chown -R")
|
||||||
|
assert.Contains(t, remedy, "/home/alice/.config/safedep/pmg")
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("dir outside real home blames leaked env, never suggests chown", func(t *testing.T) {
|
||||||
|
t.Setenv("PMG_CONFIG_DIR", "")
|
||||||
|
withRealUserHome(t, "/home/pmgtest")
|
||||||
|
|
||||||
|
remedy := UnwritableConfigDirRemedy("/home/runner/.config/safedep/pmg")
|
||||||
|
assert.Contains(t, remedy, "XDG_CONFIG_HOME")
|
||||||
|
assert.NotContains(t, remedy, "sudo chown")
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("explicit PMG_CONFIG_DIR gets its own remedy", func(t *testing.T) {
|
||||||
|
t.Setenv("PMG_CONFIG_DIR", "/srv/pmg")
|
||||||
|
withRealUserHome(t, "/home/alice")
|
||||||
|
|
||||||
|
remedy := UnwritableConfigDirRemedy("/srv/pmg")
|
||||||
|
assert.Contains(t, remedy, "PMG_CONFIG_DIR")
|
||||||
|
assert.NotContains(t, remedy, "sudo chown")
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("sibling dir with home prefix is outside home", func(t *testing.T) {
|
||||||
|
t.Setenv("PMG_CONFIG_DIR", "")
|
||||||
|
withRealUserHome(t, "/home/alice")
|
||||||
|
|
||||||
|
remedy := UnwritableConfigDirRemedy("/home/alice-evil/.config/safedep/pmg")
|
||||||
|
assert.NotContains(t, remedy, "sudo chown")
|
||||||
|
})
|
||||||
|
}
|
||||||
@@ -117,13 +117,12 @@ The invoking user must be able to write their config directory. PMG records an e
|
|||||||
|
|
||||||
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:
|
If every `pmg` command fails with `permission denied` on the event log, check where the reported path points:
|
||||||
|
|
||||||
```bash
|
- **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`
|
||||||
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.
|
||||||
```
|
|
||||||
|
|
||||||
`pmg setup doctor` detects an unwritable event log directory and prints the same fix.
|
The error message and `pmg setup doctor` print the fix matching your case.
|
||||||
|
|
||||||
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).
|
||||||
|
|
||||||
|
|||||||
@@ -229,7 +229,7 @@ func eventlogInitError(err error) error {
|
|||||||
return usefulerror.NewUsefulError().
|
return usefulerror.NewUsefulError().
|
||||||
WithCode(errcodes.PermissionDenied).
|
WithCode(errcodes.PermissionDenied).
|
||||||
WithHumanError("event logging is required but its directory is not writable").
|
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())).
|
WithHelp(config.UnwritableConfigDirRemedy(config.Get().ConfigDir())).
|
||||||
Wrap(err)
|
Wrap(err)
|
||||||
}
|
}
|
||||||
return usefulerror.NewUsefulError().
|
return usefulerror.NewUsefulError().
|
||||||
|
|||||||
Reference in New Issue
Block a user