ci: pin XDG_CONFIG_HOME for the cross-user e2e step; terse doctor fix

GitHub runners export XDG_CONFIG_HOME=/home/runner/.config and it leaks
through sudo -u, so the pmgtest pmg resolved the runner user's config
dir and fail-closed on its runner-owned log file (run 29289868727 shows
the triaged error catching exactly this). Set it inside the login shell
so it wins regardless of how the leak is delivered.

The remedy now returns a full-help and doctor-table pair from a single
triage, and drops the do-not-chown tail from the leak message.
This commit is contained in:
Sahilb315
2026-07-14 04:12:27 +05:30
parent cd9b45b3bc
commit e0580b1f79
5 changed files with 39 additions and 22 deletions
+5
View File
@@ -1000,6 +1000,11 @@ jobs:
# Pass runner PATH so setup-node's npm remains visible after FilterPMGFromPath. # Pass runner PATH so setup-node's npm remains visible after FilterPMGFromPath.
sudo -u pmgtest env "PATH=/usr/local/lib/pmg/bin:$PATH" HOME=/home/pmgtest bash -lc ' sudo -u pmgtest env "PATH=/usr/local/lib/pmg/bin:$PATH" HOME=/home/pmgtest bash -lc '
set -euo pipefail set -euo pipefail
# GH runners export XDG_CONFIG_HOME=/home/runner/.config and it
# leaks through sudo -u, so pmg would resolve the runner user
# config dir and fail on its runner-owned log file.
export XDG_CONFIG_HOME="$HOME/.config"
echo "HOME=$HOME XDG_CONFIG_HOME=$XDG_CONFIG_HOME"
which npm | grep -q /usr/local/lib/pmg/bin/npm which npm | grep -q /usr/local/lib/pmg/bin/npm
mkdir -p "$HOME/sys-e2e" && cd "$HOME/sys-e2e" mkdir -p "$HOME/sys-e2e" && cd "$HOME/sys-e2e"
npm init -y npm init -y
+2 -1
View File
@@ -285,10 +285,11 @@ func checkEventLogDirResult(skipEventLogging bool, logDir, configDir string) doc
probe, err := os.CreateTemp(logDir, ".pmg-doctor-*") probe, err := os.CreateTemp(logDir, ".pmg-doctor-*")
if err != nil { if err != nil {
_, fix := config.UnwritableConfigDirRemedy(configDir)
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: config.UnwritableConfigDirRemedy(configDir), Fix: fix,
} }
} }
if err := probe.Close(); err != nil { if err := probe.Close(); err != nil {
+2 -1
View File
@@ -147,6 +147,7 @@ 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.Equal(t, config.UnwritableConfigDirRemedy(configDir), result.Fix) _, expectedFix := config.UnwritableConfigDirRemedy(configDir)
assert.Equal(t, expectedFix, result.Fix)
}) })
} }
+14 -9
View File
@@ -670,23 +670,28 @@ var realUserHomeDir = func() (string, error) {
} }
// UnwritableConfigDirRemedy returns actionable help for a per-user config or // UnwritableConfigDirRemedy returns actionable help for a per-user config or
// event-log directory the current user cannot write. The wrong remedy is // event-log directory the current user cannot write: help is the full
// harmful: chown-ing a directory that belongs to another account steals it and // explanation for fatal CLI errors, fix the terse variant for the doctor
// bricks that account instead, so chown is only suggested when the directory // table. Prescribing the wrong remedy is harmful: chown-ing a directory that
// is inside the current user's real home. // belongs to another account steals it and bricks that account instead, so
func UnwritableConfigDirRemedy(dir string) string { // its is only suggested when the directory is inside the current user's
// real (passwd) home, which a leaked environment cannot influence.
func UnwritableConfigDirRemedy(dir string) (help, fix string) {
if os.Getenv(pmgConfigDirEnvKey) != "" { if os.Getenv(pmgConfigDirEnvKey) != "" {
return fmt.Sprintf("PMG_CONFIG_DIR points at %s; make it writable by your user", dir) return fmt.Sprintf("PMG_CONFIG_DIR points at %s; make it writable by your user", dir),
"Make PMG_CONFIG_DIR writable"
} }
home, err := realUserHomeDir() home, err := realUserHomeDir()
if err == nil && home != "" && !pathWithinDir(dir, home) { if err == nil && home != "" && !pathWithinDir(dir, home) {
return fmt.Sprintf( 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", "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\"",
dir, home) dir, home),
`Fix leaked env: export XDG_CONFIG_HOME="$HOME/.config"`
} }
return fmt.Sprintf("If a root or sudo run created it, restore ownership: sudo chown -R $(id -un) %s", dir) chown := fmt.Sprintf("sudo chown -R $(id -un) %s", dir)
return fmt.Sprintf("If a root or sudo run created it, restore ownership: %s", chown), chown
} }
func pathWithinDir(path, dir string) bool { func pathWithinDir(path, dir string) bool {
+16 -11
View File
@@ -18,34 +18,39 @@ func TestUnwritableConfigDirRemedy(t *testing.T) {
t.Setenv("PMG_CONFIG_DIR", "") t.Setenv("PMG_CONFIG_DIR", "")
withRealUserHome(t, "/home/alice") withRealUserHome(t, "/home/alice")
remedy := UnwritableConfigDirRemedy("/home/alice/.config/safedep/pmg") help, fix := UnwritableConfigDirRemedy("/home/alice/.config/safedep/pmg")
assert.Contains(t, remedy, "sudo chown -R") assert.Contains(t, help, "sudo chown -R")
assert.Contains(t, remedy, "/home/alice/.config/safedep/pmg") assert.Contains(t, help, "/home/alice/.config/safedep/pmg")
assert.Contains(t, fix, "sudo chown -R")
}) })
t.Run("dir outside real home blames leaked env, never suggests chown", func(t *testing.T) { t.Run("dir outside real home blames leaked env, never suggests chown", func(t *testing.T) {
t.Setenv("PMG_CONFIG_DIR", "") t.Setenv("PMG_CONFIG_DIR", "")
withRealUserHome(t, "/home/pmgtest") withRealUserHome(t, "/home/pmgtest")
remedy := UnwritableConfigDirRemedy("/home/runner/.config/safedep/pmg") help, fix := UnwritableConfigDirRemedy("/home/runner/.config/safedep/pmg")
assert.Contains(t, remedy, "XDG_CONFIG_HOME") assert.Contains(t, help, "XDG_CONFIG_HOME")
assert.NotContains(t, remedy, "sudo chown") assert.NotContains(t, help, "chown")
assert.Contains(t, fix, "XDG_CONFIG_HOME")
assert.NotContains(t, fix, "chown")
}) })
t.Run("explicit PMG_CONFIG_DIR gets its own remedy", func(t *testing.T) { t.Run("explicit PMG_CONFIG_DIR gets its own remedy", func(t *testing.T) {
t.Setenv("PMG_CONFIG_DIR", "/srv/pmg") t.Setenv("PMG_CONFIG_DIR", "/srv/pmg")
withRealUserHome(t, "/home/alice") withRealUserHome(t, "/home/alice")
remedy := UnwritableConfigDirRemedy("/srv/pmg") help, fix := UnwritableConfigDirRemedy("/srv/pmg")
assert.Contains(t, remedy, "PMG_CONFIG_DIR") assert.Contains(t, help, "PMG_CONFIG_DIR")
assert.NotContains(t, remedy, "sudo chown") assert.NotContains(t, help, "chown")
assert.Contains(t, fix, "PMG_CONFIG_DIR")
assert.NotContains(t, fix, "chown")
}) })
t.Run("sibling dir with home prefix is outside home", func(t *testing.T) { t.Run("sibling dir with home prefix is outside home", func(t *testing.T) {
t.Setenv("PMG_CONFIG_DIR", "") t.Setenv("PMG_CONFIG_DIR", "")
withRealUserHome(t, "/home/alice") withRealUserHome(t, "/home/alice")
remedy := UnwritableConfigDirRemedy("/home/alice-evil/.config/safedep/pmg") help, _ := UnwritableConfigDirRemedy("/home/alice-evil/.config/safedep/pmg")
assert.NotContains(t, remedy, "sudo chown") assert.NotContains(t, help, "chown")
}) })
} }