Serialize enrollment install with enrolling.lock

Concurrent enroll-url launches could both pass the enrollment
marker check and run overlapping elevated installs, racing on
LoadOrExchangeAPIKey and overwriting agent.key.

Add an exclusive flock on {configDir}/enrolling.lock for the
full install path and re-check IsEnrolled under that lock so
only one install exchanges a token and configures the device.

Signed-off-by: Ludovic Vielle <ludovic@probo.com>

pkg/deviceagent/state_cleanup.go

@@ -23,88 +23,132 @@ package deviceagent
 import (
 	"errors"
 	"fmt"
+	"io/fs"
 	"os"
 	"path/filepath"
 )
 
 const (
-	// SigstoreCacheDirName is the TUF metadata cache under the agent
-	// state directory.
 	SigstoreCacheDirName = "sigstore-cache"
-
-	// configFileTmpName is the atomic-write leftover from SaveConfig.
-	configFileTmpName = ConfigFileName + ".tmp"
+	configFileTmpName    = ConfigFileName + ".tmp"
 )
 
-// RemoveLocalState deletes known agent local state for an install: allowlisted
-// files under the state dir and EnrollmentRunDir, plus the sigstore-cache
-// subdirectory. It never recursively deletes either directory itself.
-// Missing paths are ignored. Filesystem roots and empty paths are rejected.
-//
-// Every path is attempted even if an earlier removal fails.
-//
-// enrolling.lock is intentionally retained: flock binds to the inode,
-// so unlinking the path while LoadOrExchangeAPIKey holds the lock would
-// let a concurrent caller create a new inode and bypass serialization.
+var stateCleanupNames = []string{
+	ConfigFileName,
+	configFileTmpName,
+	KeyFileName,
+	KeyFileName + ".tmp",
+	KeyFileName + ".old",
+	pendingPosturesFileName,
+	pendingPosturesFileName + ".tmp",
+}
+
+// RemoveLocalState removes allowlisted state/run files and sigstore-cache.
+// Missing paths are ignored; filesystem roots are refused. Does not remove
+// enrolling.lock (flock is inode-based).
 func RemoveLocalState(dir string) error {
 	if dir == "" {
 		return errors.New("state directory is empty")
 	}
 
+	stateDir, err := resolveCleanupDir(dir)
+	if err != nil {
+		return err
+	}
+
+	runDir, err := resolveCleanupDir(EnrollmentRunDir(dir))
+	if err != nil {
+		return err
+	}
+
+	return errors.Join(
+		removeAllUnder(stateDir, SigstoreCacheDirName),
+		cleanDir(stateDir, stateCleanupNames),
+		cleanDir(runDir, []string{EnrollmentMarkerName}),
+	)
+}
+
+// resolveCleanupDir resolves dir and rejects filesystem roots.
+// Missing paths return ("", nil).
+func resolveCleanupDir(dir string) (string, error) {
 	abs, err := filepath.Abs(dir)
 	if err != nil {
-		return fmt.Errorf("cannot resolve state directory: %w", err)
+		return "", fmt.Errorf("cannot resolve path: %w", err)
 	}
 
 	cleaned := filepath.Clean(abs)
 	if isFilesystemRoot(cleaned) {
-		return fmt.Errorf("refusing to remove filesystem root %q", cleaned)
+		return "", fmt.Errorf("refusing to remove filesystem root %q", cleaned)
 	}
 
-	// Use the original dir (not Abs/Clean) so custom relative --dir values
-	// resolve the same sibling run tree as MarkEnrolled / IsEnrolled.
-	runDir := EnrollmentRunDir(dir)
-	runDirAbs, err := filepath.Abs(runDir)
+	real, err := filepath.EvalSymlinks(cleaned)
 	if err != nil {
-		return fmt.Errorf("cannot resolve enrollment run dir: %w", err)
+		if errors.Is(err, fs.ErrNotExist) {
+			return "", nil
+		}
+
+		return "", fmt.Errorf("cannot resolve path: %w", err)
 	}
 
-	runDir = filepath.Clean(runDirAbs)
-	if isFilesystemRoot(runDir) {
-		return fmt.Errorf("refusing to remove enrollment run dir %q", runDir)
+	if isFilesystemRoot(real) {
+		return "", fmt.Errorf("refusing to remove filesystem root %q", real)
 	}
 
-	knownPaths := []string{
-		filepath.Join(cleaned, ConfigFileName),
-		filepath.Join(cleaned, configFileTmpName),
-		filepath.Join(cleaned, KeyFileName),
-		filepath.Join(cleaned, KeyFileName+".tmp"),
-		filepath.Join(cleaned, KeyFileName+".old"),
-		filepath.Join(cleaned, pendingPosturesFileName),
-		filepath.Join(cleaned, pendingPosturesFileName+".tmp"),
-		filepath.Join(runDir, EnrollmentMarkerName),
+	return real, nil
+}
+
+func cleanDir(dir string, names []string) error {
+	if dir == "" {
+		return nil
 	}
 
+	root, err := os.OpenRoot(dir)
+	if err != nil {
+		if errors.Is(err, fs.ErrNotExist) {
+			return nil
+		}
+
+		return fmt.Errorf("cannot open %s: %w", dir, err)
+	}
+
+	defer func() { _ = root.Close() }()
+
 	var errs error
-	for _, path := range knownPaths {
-		if err := os.Remove(path); err != nil && !errors.Is(err, os.ErrNotExist) {
-			errs = errors.Join(errs, fmt.Errorf("cannot remove %s: %w", path, err))
+
+	for _, name := range names {
+		if err := root.Remove(name); err != nil && !errors.Is(err, fs.ErrNotExist) {
+			errs = errors.Join(errs, fmt.Errorf("cannot remove %s: %w", name, err))
 		}
 	}
 
-	cacheDir := filepath.Join(cleaned, SigstoreCacheDirName)
-	if err := os.RemoveAll(cacheDir); err != nil {
-		errs = errors.Join(errs, fmt.Errorf("cannot remove %s: %w", SigstoreCacheDirName, err))
-	}
-
-	// Best-effort: remove the dirs only when empty. Leave them alone when
-	// foreign files remain or removal is otherwise refused.
-	_ = os.Remove(cleaned)
-	_ = os.Remove(runDir)
+	_ = os.Remove(dir)
 
 	return errs
 }
 
+func removeAllUnder(dir, name string) error {
+	if dir == "" {
+		return nil
+	}
+
+	root, err := os.OpenRoot(dir)
+	if err != nil {
+		if errors.Is(err, fs.ErrNotExist) {
+			return nil
+		}
+
+		return fmt.Errorf("cannot open %s: %w", dir, err)
+	}
+
+	defer func() { _ = root.Close() }()
+
+	if err := root.RemoveAll(name); err != nil {
+		return fmt.Errorf("cannot remove %s: %w", name, err)
+	}
+
+	return nil
+}
+
 func isFilesystemRoot(path string) bool {
 	cleaned := filepath.Clean(path)
 	return cleaned != "" && cleaned == filepath.Dir(cleaned)