diff --git a/agent/package_updates.go b/agent/package_updates.go index a7e29b4eb..4e81c37e4 100644 --- a/agent/package_updates.go +++ b/agent/package_updates.go @@ -4,6 +4,7 @@ import ( "bufio" "context" "errors" + "fmt" "log/slog" "os" "os/exec" @@ -54,7 +55,8 @@ type packageUpdatesManager struct { // newPackageUpdatesManager returns nil if disabled or no supported package manager // is found. Agents running in a container are skipped because the container's -// package database is not the host's. dataDir holds pacman's private sync databases. +// package database is not the host's. dataDir holds the package managers' per-user +// caches and state, which they cannot write to their usual locations as the agent user. func newPackageUpdatesManager(dataDir string) *packageUpdatesManager { if runtime.GOOS != "linux" || runningInContainer() { return nil @@ -150,9 +152,9 @@ func detectPackageManager(dataDir string) (string, packageUpdatesCheck) { case commandExists("apt-get") && exec.Command("dpkg-query", "-W", "apt").Run() == nil: return "apt", checkApt case commandExists("dnf"): - return "dnf", checkDnf + return "dnf", newDnfCheck(dataDir) case commandExists("zypper"): - return "zypper", checkZypper + return "zypper", newZypperCheck(dataDir) case commandExists("checkupdates"): return "pacman", newPacmanCheck(dataDir) case commandExists("apk"): @@ -183,8 +185,14 @@ func runPackageCommandEnv(ctx context.Context, env []string, okCodes []int, name // children can keep stdout open. WaitDelay stops Output from waiting on them. cmd.WaitDelay = 10 * time.Second out, err := cmd.Output() - if exitErr, ok := errors.AsType[*exec.ExitError](err); ok && slices.Contains(okCodes, exitErr.ExitCode()) { - return string(out), nil + if exitErr, ok := errors.AsType[*exec.ExitError](err); ok { + if slices.Contains(okCodes, exitErr.ExitCode()) { + return string(out), nil + } + // include stderr so a failure says why, not just "exit status 1" + if stderr := strings.TrimSpace(string(exitErr.Stderr)); stderr != "" { + err = fmt.Errorf("%s: %w: %s", name, err, stderr) + } } return string(out), err } @@ -214,10 +222,26 @@ func checkApt(ctx context.Context) (packageUpdatesResult, error) { }, nil } +// newDnfCheck points dnf5's per-user state (log) and cache dirs into dataDir. +// They default to $HOME/.local/state and $HOME/.cache, and the agent user's home +// is usually /nonexistent. With -C, dnf5 still reads the system metadata cache. +func newDnfCheck(dataDir string) packageUpdatesCheck { + var env []string + if dataDir != "" { + env = []string{ + "XDG_STATE_HOME=" + filepath.Join(dataDir, "state"), + "XDG_CACHE_HOME=" + filepath.Join(dataDir, "cache"), + } + } + return func(ctx context.Context) (packageUpdatesResult, error) { + return checkDnf(ctx, env) + } +} + // checkDnf uses the system metadata cache only (-C), so it never downloads metadata. // check-update lists only available versions, so installed versions come from rpm. -func checkDnf(ctx context.Context) (packageUpdatesResult, error) { - out, err := runPackageCommand(ctx, []int{100}, "dnf", "-q", "-C", "check-update") +func checkDnf(ctx context.Context, env []string) (packageUpdatesResult, error) { + out, err := runPackageCommandEnv(ctx, env, []int{100}, "dnf", "-q", "-C", "check-update") if err != nil { return packageUpdatesResult{}, err } @@ -237,7 +261,7 @@ func checkDnf(ctx context.Context) (packageUpdatesResult, error) { } } - out, err = runPackageCommand(ctx, []int{100}, "dnf", "-q", "-C", "check-update", "--security") + out, err = runPackageCommandEnv(ctx, env, []int{100}, "dnf", "-q", "-C", "check-update", "--security") if err == nil { // --security lists the lowest version that fixes an advisory, which may be // older than the version check-update offers, so match on name.arch only @@ -261,20 +285,28 @@ func checkDnf(ctx context.Context) (packageUpdatesResult, error) { return result, nil } -// checkZypper lists package updates. Security updates come from patches, which +// newZypperCheck lists package updates. Security updates come from patches, which // zypper does not map to packages here, so only the security count is known. -func checkZypper(ctx context.Context) (packageUpdatesResult, error) { - out, err := runPackageCommand(ctx, nil, "zypper", "--no-refresh", "-q", "list-updates") - if err != nil { - return packageUpdatesResult{}, err +// zypper rebuilds the solv cache from the raw metadata when it is missing or stale, +// which the agent user cannot do in /var/cache/zypp, so it keeps its own in dataDir. +func newZypperCheck(dataDir string) packageUpdatesCheck { + args := []string{"--no-refresh", "-q"} + if dataDir != "" { + args = append(args, "--solv-cache-dir", filepath.Join(dataDir, "zypp-solv")) } - packages := parseZypperListUpdates(out) - result := packageUpdatesResult{packages: packages, counts: []uint16{uint16(len(packages))}} - out, err = runPackageCommand(ctx, nil, "zypper", "--no-refresh", "-q", "list-patches", "--category", "security") - if err == nil { - result.counts = append(result.counts, parseZypperTable(out)) + return func(ctx context.Context) (packageUpdatesResult, error) { + out, err := runPackageCommand(ctx, nil, "zypper", slices.Concat(args, []string{"list-updates"})...) + if err != nil { + return packageUpdatesResult{}, err + } + packages := parseZypperListUpdates(out) + result := packageUpdatesResult{packages: packages, counts: []uint16{uint16(len(packages))}} + out, err = runPackageCommand(ctx, nil, "zypper", slices.Concat(args, []string{"list-patches", "--category", "security"})...) + if err == nil { + result.counts = append(result.counts, parseZypperTable(out)) + } + return result, nil } - return result, nil } // newPacmanCheck uses checkupdates (pacman-contrib), which syncs a private copy of diff --git a/agent/package_updates_test.go b/agent/package_updates_test.go index 7b0131f7e..7bc1e368c 100644 --- a/agent/package_updates_test.go +++ b/agent/package_updates_test.go @@ -278,7 +278,7 @@ exit 100`, "rpm": `cat "` + testDataPath(t, tt.installed) + `" exit 1`, }) - result, err := checkDnf(context.Background()) + result, err := newDnfCheck(t.TempDir())(context.Background()) require.NoError(t, err) assert.Equal(t, []uint16{uint16(tt.total), uint16(tt.securityCount)}, result.counts) assert.True(t, result.securityKnown) @@ -297,12 +297,36 @@ echo "bash.x86_64 5.1.8-9.el9 baseos" exit 100`, "rpm": `echo "bash.x86_64 5.1.8-6.el9_1"`, }) - result, err := checkDnf(context.Background()) + result, err := newDnfCheck(t.TempDir())(context.Background()) require.NoError(t, err) assert.Equal(t, []uint16{1}, result.counts) assert.False(t, result.securityKnown) assert.Equal(t, []system.PackageUpdate{{Name: "bash", Current: "5.1.8-6.el9_1", Available: "5.1.8-9.el9"}}, result.packages) }) + + t.Run("error includes stderr", func(t *testing.T) { + fakeCommands(t, map[string]string{ + "dnf": `echo "Error: Cache-only enabled but no cache for 'baseos'" >&2 +exit 1`, + }) + _, err := newDnfCheck(t.TempDir())(context.Background()) + require.Error(t, err) + assert.Equal(t, "dnf: exit status 1: Error: Cache-only enabled but no cache for 'baseos'", err.Error()) + }) + + t.Run("state and cache in data dir", func(t *testing.T) { + dataDir := t.TempDir() + envFile := filepath.Join(t.TempDir(), "env") + fakeCommands(t, map[string]string{ + "dnf": `echo "$XDG_STATE_HOME $XDG_CACHE_HOME" >> "` + envFile + `"`, + }) + _, err := newDnfCheck(dataDir)(context.Background()) + require.NoError(t, err) + env, err := os.ReadFile(envFile) + require.NoError(t, err) + want := filepath.Join(dataDir, "state") + " " + filepath.Join(dataDir, "cache") + "\n" + assert.Equal(t, want+want, string(env)) + }) } func TestDetectPackageManager(t *testing.T) { @@ -357,15 +381,25 @@ func TestParseZypperListUpdates(t *testing.T) { } func TestCheckZypper(t *testing.T) { + dataDir := t.TempDir() + argsFile := filepath.Join(t.TempDir(), "args") fakeCommands(t, map[string]string{ - "zypper": `case "$*" in *list-patches*) cat "` + testDataPath(t, "zypper_leap155_list_patches_security.txt") + `" ;; *) cat "` + testDataPath(t, "zypper_leap155_list_updates.txt") + `" ;; esac`, + "zypper": `echo "$*" >> "` + argsFile + `" +case "$*" in *list-patches*) cat "` + testDataPath(t, "zypper_leap155_list_patches_security.txt") + `" ;; *) cat "` + testDataPath(t, "zypper_leap155_list_updates.txt") + `" ;; esac`, }) - result, err := checkZypper(context.Background()) + result, err := newZypperCheck(dataDir)(context.Background()) require.NoError(t, err) // security patches don't map to packages, so only the count is known assert.Equal(t, []uint16{22, 4}, result.counts) assert.False(t, result.securityKnown) assert.Len(t, result.packages, 22) + + // both commands use the agent's own solv cache + args, err := os.ReadFile(argsFile) + require.NoError(t, err) + solvDir := filepath.Join(dataDir, "zypp-solv") + assert.Equal(t, "--no-refresh -q --solv-cache-dir "+solvDir+" list-updates\n"+ + "--no-refresh -q --solv-cache-dir "+solvDir+" list-patches --category security\n", string(args)) } func TestParsePacmanCheckUpdates(t *testing.T) { diff --git a/supplemental/debian/beszel-agent.service b/supplemental/debian/beszel-agent.service index d86388dff..da0f4c55e 100644 --- a/supplemental/debian/beszel-agent.service +++ b/supplemental/debian/beszel-agent.service @@ -20,6 +20,7 @@ ProtectHome=read-only ProtectHostname=yes ProtectKernelLogs=yes ProtectSystem=strict +PrivateTmp=yes RemoveIPC=yes RestrictSUIDSGID=true diff --git a/supplemental/scripts/install-agent.sh b/supplemental/scripts/install-agent.sh index 09e6b59d1..d74402b71 100755 --- a/supplemental/scripts/install-agent.sh +++ b/supplemental/scripts/install-agent.sh @@ -1369,6 +1369,7 @@ ProtectHome=read-only ProtectHostname=yes ProtectKernelLogs=yes ProtectSystem=strict +PrivateTmp=yes RemoveIPC=yes RestrictSUIDSGID=true @@ -1396,6 +1397,13 @@ EOF sed -i '/^User=beszel$/a SupplementaryGroups=systemd-journal' /etc/systemd/system/beszel-agent.service fi + # ProtectSystem=strict makes /var/tmp read-only, but dnf4 and zypper run as a non-root + # user keep temporary files in /var/tmp, so package update checks fail without this. + if grep -q '^ProtectSystem=strict$' /etc/systemd/system/beszel-agent.service && ! grep -q '^PrivateTmp=' /etc/systemd/system/beszel-agent.service; then + echo "Giving the agent service a private writable /tmp and /var/tmp" + sed -i '/^ProtectSystem=strict$/a PrivateTmp=yes' /etc/systemd/system/beszel-agent.service + fi + # Load and start the service printf "\nLoading and starting the agent service...\n" systemctl daemon-reload