fix(agent): fix pacman update check under systemd sandbox

- store checkupdates' private DB in the agent data dir, since
  ProtectSystem=strict makes /tmp read-only
- sync the private DB every 12h and use `checkupdates -n` in between,
  forcing a sync when the DB is missing so -n never reports a false 0
- set WaitDelay so a timed-out command can't hang on child processes

agent/agent.go

@@ -156,7 +156,7 @@ func NewAgent(dataDir ...string) (agent *Agent, err error) {
 		slog.Debug("SMART", "err", err)
 	}
 
-	agent.packageUpdates = newPackageUpdatesManager()
+	agent.packageUpdates = newPackageUpdatesManager(agent.dataDir)
 
 	// initialize GPU manager
 	agent.gpuManager, err = NewGPUManager()

agent/package_updates.go

@@ -7,6 +7,7 @@ import (
 	"log/slog"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"runtime"
 	"slices"
 	"strings"
@@ -19,6 +20,9 @@ import (
 const (
 	defaultPackageUpdatesInterval = time.Hour
 	packageUpdatesTimeout         = 5 * time.Minute
+	// pacmanSyncInterval limits how often checkupdates downloads fresh sync
+	// databases. Checks in between reuse the last synced copy.
+	pacmanSyncInterval = 12 * time.Hour
 )
 
 // packageUpdatesCheck returns [total] or [total, security] pending package updates.
@@ -37,8 +41,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.
+// package database is not the host's. dataDir holds pacman's private sync databases.
-func newPackageUpdatesManager() *packageUpdatesManager {
+func newPackageUpdatesManager(dataDir string) *packageUpdatesManager {
 	if runtime.GOOS != "linux" || runningInContainer() {
 		return nil
 	}
@@ -54,7 +58,7 @@ func newPackageUpdatesManager() *packageUpdatesManager {
 			slog.Warn("Invalid PACKAGE_UPDATES_INTERVAL", "value", env)
 		}
 	}
-	name, check := detectPackageManager()
+	name, check := detectPackageManager(dataDir)
 	if check == nil {
 		return nil
 	}
@@ -97,7 +101,7 @@ func runningInContainer() bool {
 	return false
 }
 
-func detectPackageManager() (string, packageUpdatesCheck) {
+func detectPackageManager(dataDir string) (string, packageUpdatesCheck) {
 	switch {
 	case commandExists("apt-get"):
 		return "apt", checkApt
@@ -106,7 +110,7 @@ func detectPackageManager() (string, packageUpdatesCheck) {
 	case commandExists("zypper"):
 		return "zypper", checkZypper
 	case commandExists("checkupdates"):
-		return "pacman", checkPacman
+		return "pacman", newPacmanCheck(dataDir)
 	case commandExists("apk"):
 		return "apk", checkApk
 	}
@@ -121,8 +125,17 @@ func commandExists(name string) bool {
 // runPackageCommand runs a read-only package manager command and returns stdout.
 // okCodes lists non-zero exit codes that still mean success.
 func runPackageCommand(ctx context.Context, okCodes []int, name string, args ...string) (string, error) {
+	return runPackageCommandEnv(ctx, nil, okCodes, name, args...)
+}
+
+// runPackageCommandEnv is runPackageCommand with extra environment variables.
+func runPackageCommandEnv(ctx context.Context, env []string, okCodes []int, name string, args ...string) (string, error) {
 	cmd := exec.CommandContext(ctx, name, args...)
 	cmd.Env = append(os.Environ(), "LC_ALL=C")
+	cmd.Env = append(cmd.Env, env...)
+	// checkupdates is a shell script, so a timeout kills only the script and its
+	// 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
@@ -168,14 +181,44 @@ func checkZypper(ctx context.Context) ([]uint16, error) {
 	return []uint16{total, parseZypperTable(out)}, nil
 }
 
-// checkPacman uses checkupdates (pacman-contrib), which syncs a private copy of
+// newPacmanCheck uses checkupdates (pacman-contrib), which syncs a private copy of
-// the databases and never touches pacman's own. Exit code 2 means no updates.
+// the databases and never touches pacman's own. The copy lives in dataDir because
-func checkPacman(ctx context.Context) ([]uint16, error) {
+// the systemd unit's ProtectSystem=strict makes the default /tmp location read-only.
-	out, err := runPackageCommand(ctx, []int{2}, "checkupdates")
+// It syncs every pacmanSyncInterval and uses the existing copy (-n) in between.
-	if err != nil {
+// Local upgrades show up right away since checkupdates links the live local DB.
-		return nil, err
+// Exit code 2 means no updates.
+func newPacmanCheck(dataDir string) packageUpdatesCheck {
+	var env []string
+	var syncDir string
+	if dataDir != "" {
+		dbPath := filepath.Join(dataDir, "checkup-db")
+		env = []string{"CHECKUPDATES_DB=" + dbPath}
+		syncDir = filepath.Join(dbPath, "sync")
+	}
+	// checks never overlap (packageUpdatesManager.running), so no lock is needed
+	var lastSync time.Time
+	return func(ctx context.Context) ([]uint16, error) {
+		// -n with a missing database reports no updates rather than failing,
+		// so always sync first and whenever the private copy is missing
+		sync := lastSync.IsZero() || time.Since(lastSync) >= pacmanSyncInterval
+		if !sync && syncDir != "" {
+			if _, err := os.Stat(syncDir); err != nil {
+				sync = true
+			}
+		}
+		var args []string
+		if !sync {
+			args = append(args, "-n")
+		}
+		out, err := runPackageCommandEnv(ctx, env, []int{2}, "checkupdates", args...)
+		if err != nil {
+			return nil, err
+		}
+		if sync {
+			lastSync = time.Now()
+		}
+		return []uint16{parsePacmanCheckUpdates(out)}, nil
 	}
-	return []uint16{parsePacmanCheckUpdates(out)}, nil
 }
 
 func checkApk(ctx context.Context) ([]uint16, error) {

agent/package_updates_test.go

@@ -7,6 +7,8 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
+	"runtime"
+	"strings"
 	"testing"
 	"time"
 
@@ -25,7 +27,7 @@ func readPackageUpdatesTestData(t *testing.T, name string) string {
 
 func TestParseAptSimulate(t *testing.T) {
 	tests := []struct {
-		file             string
+		file            string
 		total, security uint16
 	}{
 		{"apt_debian12.txt", 44, 5},
@@ -149,3 +151,46 @@ func TestPackageUpdatesManagerCaching(t *testing.T) {
 	// failed check clears the counts
 	assert.Nil(t, pm.get(time.Now()))
 }
+
+func TestPacmanCheckSync(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("requires a shell script on PATH")
+	}
+	binDir := t.TempDir()
+	dataDir := t.TempDir()
+	logFile := filepath.Join(binDir, "calls.log")
+	// fake checkupdates logs its args and db path, and creates the sync dir when syncing
+	script := `#!/bin/sh
+echo "args=[$*] db=$CHECKUPDATES_DB" >> ` + logFile + `
+[ "$1" = "-n" ] || mkdir -p "$CHECKUPDATES_DB/sync"
+echo "linux 6.1-1 -> 6.2-1"
+`
+	require.NoError(t, os.WriteFile(filepath.Join(binDir, "checkupdates"), []byte(script), 0o755))
+	t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH"))
+
+	check := newPacmanCheck(dataDir)
+	dbPath := filepath.Join(dataDir, "checkup-db")
+	readCalls := func() []string {
+		data, err := os.ReadFile(logFile)
+		require.NoError(t, err)
+		return strings.Split(strings.TrimSpace(string(data)), "\n")
+	}
+
+	// first check syncs
+	counts, err := check(context.Background())
+	require.NoError(t, err)
+	assert.Equal(t, []uint16{1}, counts)
+	// later checks reuse the synced copy
+	_, err = check(context.Background())
+	require.NoError(t, err)
+	// a missing private copy forces a sync
+	require.NoError(t, os.RemoveAll(dbPath))
+	_, err = check(context.Background())
+	require.NoError(t, err)
+
+	assert.Equal(t, []string{
+		"args=[] db=" + dbPath,
+		"args=[-n] db=" + dbPath,
+		"args=[] db=" + dbPath,
+	}, readCalls())
+}