Keep the walk's paths as bytes, forget a walk root superseded, and leave other mounts unread (hq issue 361 review)

The review found a cursor of a name that is not UTF-8 came back changed
from JSON, so a resumed walk skipped what it stood before; an ask to
search again was lost to a restart, which carried the older walk on; a
fresh search judged a filesystem it never walked; a mount point root
cannot read failed or hung the walk; a found file gone before the package
manager was asked failed every resume; and skipped or kept outcomes
started a search on every reconcile, back to back with a long walk.

internal/accounts/setuid_cache.go

@@ -58,6 +58,7 @@ type SetuidCache struct {
 	running    bool
 	startedAt  time.Time // when the search running, or the last one, was started (a resumed one: its start)
 	attemptAt  time.Time // when the running or last search was started or resumed
+	endedAt    time.Time // when the last search ended, on the same clock
 	again      bool      // asked to search again since the running or last one started
 	failedAt   time.Time
 	failed     error
@@ -71,8 +72,8 @@ type setuidResult struct {
 	Started  time.Time `json:"started"`
 	Finished time.Time `json:"finished"`
 	// Mounts are the filesystems it walked; Found the setuid-root programs no package owns.
-	Mounts []string `json:"mounts"`
-	Found  []string `json:"found"`
+	Mounts Paths `json:"mounts"`
+	Found  Paths `json:"found"`
 }
 
 const (
@@ -145,6 +146,10 @@ func (c *SetuidCache) SearchAgain() {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 	c.again = true
+	// The walk under way, or kept, started before root acted: never carried on, even by an engine that
+	// restarts before the next search starts (the review of 2026-10-10). It keeps no progress from now on.
+	c.unfinished = nil
+	c.forgetProgress()
 }
 
 // SetuidSearch is one search: it carries p on to its end — walking what p has not walked, calling checkpoint
@@ -176,10 +181,17 @@ func (c *SetuidCache) Look(now time.Time, mounts []string, search SetuidSearch)
 	if c.last != nil {
 		age = now.Sub(c.last.Started)
 	}
-	fresh := c.last != nil && age > -clockSkew && age < c.freshFor()
-	asked := c.again || (c.last != nil && !covers(c.last.Mounts, mounts))
+	// Fresh only while it walked every filesystem a search would walk now: one mounted since is not judged
+	// by a search that never saw it.
+	fresh := c.last != nil && age > -clockSkew && age < c.freshFor() && covers(c.last.Mounts, mounts)
+	// Asked for (an apply, a new filesystem): at most once an AgainAfter, counted from when the last search
+	// ended, so a walk of hours is not followed by another at once, apply after apply.
+	gap := now.Sub(c.attemptAt)
+	if !c.endedAt.IsZero() {
+		gap = now.Sub(c.endedAt)
+	}
 	due := !c.running && (c.failed == nil || now.Sub(c.failedAt) >= c.backoff) &&
-		(!fresh || age >= c.every() || (asked && (c.attemptAt.IsZero() || now.Sub(c.attemptAt) >= AgainAfter)))
+		(!fresh || age >= c.every() || (c.again && (c.attemptAt.IsZero() || gap >= AgainAfter)))
 	if due {
 		c.start(now, mounts, search)
 	}
@@ -250,6 +262,12 @@ func (c *SetuidCache) start(now time.Time, mounts []string, search SetuidSearch)
 		if err == nil {
 			unkept := false
 			found, err = search(context.Background(), p, func() {
+				c.mu.Lock()
+				superseded := c.again
+				c.mu.Unlock()
+				if superseded {
+					return // root acted since this walk started: its progress is not to be carried on
+				}
 				if kerr := c.keepProgress(p); kerr != nil && !unkept {
 					unkept = true
 					c.say("the search for setuid programs could not keep how far it got, and starts over if " +
@@ -261,10 +279,14 @@ func (c *SetuidCache) start(now time.Time, mounts []string, search SetuidSearch)
 		c.mu.Lock()
 		defer c.mu.Unlock()
 		c.running = false
+		c.endedAt = now.Add(took)
 		if err != nil {
-			c.failed, c.failedAt = err, now
+			// The back-off counts from when it failed.
+			c.failed, c.failedAt = err, c.endedAt
 			c.backoff = min(max(2*c.backoff, FailedBackoff), MaxBackoff)
-			c.unfinished = p
+			if !c.again { // superseded by an ask to search again: never carried on
+				c.unfinished = p
+			}
 			c.say(fmt.Sprintf("the search for setuid programs started %s did not complete (%s this time): %v",
 				p.Started.Local().Format("2006-01-02 15:04"), took.Round(time.Second), err))
 			return

internal/accounts/setuid_cache_test.go

@@ -139,7 +139,7 @@ func TestAnIncompleteSearchIsNeverFree(t *testing.T) {
 	if starts(c) != 1 {
 		t.Fatalf("tried again within its back-off: %d", starts(c))
 	}
-	c.Look(at.Add(FailedBackoff), root, g.search)
+	c.Look(at.Add(FailedBackoff+time.Second), root, g.search)
 	if starts(c) != 2 {
 		t.Fatalf("not tried again after its back-off: %d", starts(c))
 	}
@@ -168,7 +168,10 @@ func TestAnApplyThatMovedRootSearchesAgain(t *testing.T) {
 	c.Look(at, root, g.search)
 	g.finish(c)
 	c.SearchAgain()
-	if _, _, err := c.Look(at.Add(AgainAfter), root, g.search); err != nil {
+	if _, _, err := c.Look(at.Add(time.Minute), root, g.search); err != nil || starts(c) != 1 {
+		t.Fatalf("asked again within AgainAfter of the last search's end: %v, %d searches", err, starts(c))
+	}
+	if _, _, err := c.Look(at.Add(AgainAfter+time.Second), root, g.search); err != nil {
 		t.Fatalf("the last complete search judges while the new one runs: %v", err)
 	}
 	if starts(c) != 2 {
@@ -180,20 +183,71 @@ func TestAnApplyThatMovedRootSearchesAgain(t *testing.T) {
 	if starts(c) != 2 {
 		t.Fatalf("a second search started beside the running one: %d", starts(c))
 	}
-	g.finish(c)
-	c.Look(at.Add(2*AgainAfter+time.Minute), root, g.search)
+	g.finish(c) // ends, on the cache's clock, at about AgainAfter+1s
+	c.Look(at.Add(2*AgainAfter), root, g.search)
+	if starts(c) != 2 {
+		t.Fatal("a search started again less than AgainAfter after the last one ended")
+	}
+	c.Look(at.Add(2*AgainAfter+2*time.Second), root, g.search)
 	if starts(c) != 3 {
 		t.Fatalf("the ask made while a search ran was dropped: %d", starts(c))
 	}
 	g.finish(c)
-	// A filesystem the last search did not walk asks for one too.
-	c.Look(at.Add(4*AgainAfter), []string{"/", "/srv"}, g.search)
-	if starts(c) != 4 {
+	var nilCache *SetuidCache
+	nilCache.SearchAgain()
+}
+
+// A filesystem the last complete search did not walk is not judged by it: not judged yet, and a search starts.
+func TestAFilesystemTheSearchDidNotWalkIsNotJudgedByIt(t *testing.T) {
+	c := &SetuidCache{}
+	g := newGate()
+	c.Look(at, root, g.search)
+	g.finish(c)
+	_, _, err := c.Look(at.Add(time.Hour), []string{"/", "/srv"}, g.search)
+	notJudged(t, err, true)
+	if starts(c) != 2 {
 		t.Fatalf("a new filesystem was not searched: %d", starts(c))
 	}
 	g.finish(c)
-	var nilCache *SetuidCache
-	nilCache.SearchAgain()
+	if _, _, err := c.Look(at.Add(time.Hour), []string{"/", "/srv"}, g.search); err != nil {
+		t.Fatalf("once walked it judges: %v", err)
+	}
+}
+
+// An ask to search again forgets the walk under way, so an engine restarted before the next search does not
+// carry on a walk that started before root acted.
+func TestAnAskToSearchAgainIsNotLostToARestart(t *testing.T) {
+	dir := t.TempDir()
+	paths := func() *SetuidCache {
+		return &SetuidCache{Path: filepath.Join(dir, SetuidSearchFile), ProgressPath: filepath.Join(dir, SetuidProgressFile)}
+	}
+	seen := make(chan *SetuidProgress, 1)
+	first := paths()
+	first.Look(at, root, interrupted(seen))
+	<-seen
+	first.SearchAgain() // an apply moved root's things; then the engine restarts
+	restarted := paths()
+	var with SetuidProgress
+	restarted.Look(at.Add(time.Hour), root, func(ctx context.Context, p *SetuidProgress, _ func()) ([]string, error) {
+		with = *p
+		return nil, nil
+	})
+	restarted.Wait()
+	if !with.Started.Equal(at.Add(time.Hour)) || len(with.Found) != 0 {
+		t.Fatalf("the walk from before the apply was carried on: %+v", with)
+	}
+}
+
+// A cursor whose name is not UTF-8 is kept as its bytes: JSON strings would change it, and the resumed walk
+// would skip what it stood before.
+func TestACursorOfAnyBytesIsKeptExactly(t *testing.T) {
+	p := &SetuidProgress{Started: at, Mounts: Paths{"/"}, Cursor: Cursors{"/": "/home/agent/\x80/x"},
+		Found: Paths{"/opt/\xff"}}
+	var back SetuidProgress
+	fromJSON(t, mustJSON(t, p), &back)
+	if back.Cursor["/"] != "/home/agent/\x80/x" || back.Found[0] != "/opt/\xff" {
+		t.Fatalf("kept as %q, %q", back.Cursor["/"], back.Found[0])
+	}
 }
 
 func TestTheCompleteSearchIsKeptAndReadAfterARestart(t *testing.T) {

internal/accounts/setuid_walk.go

@@ -2,6 +2,7 @@ package accounts
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io/fs"
@@ -34,12 +35,66 @@ type SetuidProgress struct {
 	ID      string    `json:"id"`
 	Started time.Time `json:"started"`
 	// Mounts are the filesystems it walks, root first; Done those walked to their end.
-	Mounts []string `json:"mounts"`
-	Done   []string `json:"done"`
+	Mounts Paths `json:"mounts"`
+	Done   Paths `json:"done"`
 	// Cursor is, for a filesystem being walked, the last path visited.
-	Cursor map[string]string `json:"cursor"`
+	Cursor Cursors `json:"cursor"`
 	// Found is every setuid- or setgid-root regular file found so far, before the package manager is asked.
-	Found []string `json:"found"`
+	Found Paths `json:"found"`
+}
+
+// Paths are paths kept as their bytes. **Never as JSON strings**: encoding/json replaces bytes that are not
+// UTF-8 with U+FFFD, and a cursor so changed sorts after the names it stood before — a resumed walk would skip
+// them unread (the review of 2026-10-10). A name of any bytes is any account's to make.
+type Paths []string
+
+func (ps Paths) MarshalJSON() ([]byte, error) {
+	raw := make([][]byte, len(ps))
+	for i, p := range ps {
+		raw[i] = []byte(p)
+	}
+	return json.Marshal(raw)
+}
+
+func (ps *Paths) UnmarshalJSON(b []byte) error {
+	var raw [][]byte
+	if err := json.Unmarshal(b, &raw); err != nil {
+		return err
+	}
+	*ps = make(Paths, len(raw))
+	for i, r := range raw {
+		(*ps)[i] = string(r)
+	}
+	return nil
+}
+
+// Cursors are each filesystem's cursor, kept as bytes as Paths are.
+type Cursors map[string]string
+
+type keptCursor struct {
+	Mount []byte `json:"mount"`
+	Path  []byte `json:"path"`
+}
+
+func (cs Cursors) MarshalJSON() ([]byte, error) {
+	raw := []keptCursor{}
+	for m, p := range cs {
+		raw = append(raw, keptCursor{Mount: []byte(m), Path: []byte(p)})
+	}
+	slices.SortFunc(raw, func(a, b keptCursor) int { return strings.Compare(string(a.Mount), string(b.Mount)) })
+	return json.Marshal(raw)
+}
+
+func (cs *Cursors) UnmarshalJSON(b []byte) error {
+	var raw []keptCursor
+	if err := json.Unmarshal(b, &raw); err != nil {
+		return err
+	}
+	*cs = Cursors{}
+	for _, r := range raw {
+		(*cs)[string(r.Mount)] = string(r.Path)
+	}
+	return nil
 }
 
 // CheckpointEvery is how often a walk's progress is kept.
@@ -49,10 +104,13 @@ const CheckpointEvery = time.Minute
 var prunedPaths = []string{"/proc", "/sys", "/var/lib/docker", "/var/lib/containers"}
 
 // walker walks for setuid programs. rootUID and rootGID are the owner and group a match has: root's (0), but
-// for a test.
+// for a test. mountPoints are every filesystem mounted on the machine (MountPoints): a directory that is one is
+// another filesystem, left without being read — a FUSE mount root cannot read, or a network mount whose
+// server is away, would fail or hang the walk (the review of 2026-10-10).
 type walker struct {
 	rootUID, rootGID uint32
 	every            time.Duration
+	mountPoints      map[string]bool
 }
 
 // walk carries p on to its end, calling checkpoint every w.every and after each filesystem. It ends early only
@@ -60,7 +118,7 @@ type walker struct {
 // A file or directory that vanished while it walked is not an error: what still exists was walked.
 func (w walker) walk(ctx context.Context, p *SetuidProgress, checkpoint func()) error {
 	if p.Cursor == nil {
-		p.Cursor = map[string]string{}
+		p.Cursor = Cursors{}
 	}
 	last := time.Now()
 	for _, m := range p.Mounts {
@@ -75,8 +133,23 @@ func (w walker) walk(ctx context.Context, p *SetuidProgress, checkpoint func())
 		checkpoint()
 		last = time.Now()
 	}
-	slices.Sort(p.Found)
-	p.Found = slices.Compact(p.Found)
+	// Looked at again: a file found that is gone, or no longer setuid-root, is no answer the package manager
+	// can give, and would fail every search that resumes this walk (the review of 2026-10-10).
+	var still Paths
+	for _, f := range p.Found {
+		info, err := os.Lstat(f)
+		if errors.Is(err, fs.ErrNotExist) {
+			continue
+		}
+		if err != nil {
+			return fmt.Errorf("the walk for setuid programs could not read %s again: %w", f, err)
+		}
+		if w.setuidRoot(info) {
+			still = append(still, f)
+		}
+	}
+	slices.Sort(still)
+	p.Found = slices.Compact(still)
 	return nil
 }
 
@@ -113,7 +186,7 @@ func (w walker) walkMount(ctx context.Context, p *SetuidProgress, mount string,
 		}
 		switch {
 		case d.IsDir():
-			if path != mount && slices.Contains(prunedPaths, path) {
+			if path != mount && (slices.Contains(prunedPaths, path) || w.mountPoints[path]) {
 				return filepath.SkipDir
 			}
 			info, err := os.Lstat(path)

internal/accounts/setuid_walk_test.go

@@ -182,3 +182,32 @@ func TestTheSearchRunsOnALoweredThreadOfItsOwn(t *testing.T) {
 		t.Fatalf("the thread could not be lowered: %v", err)
 	}
 }
+
+// Another filesystem's mount point is left without being read, and a file found that is gone, or no longer
+// setuid-root, when the walk ends is not asked about.
+func TestTheWalkLeavesMountPointsAndDropsWhatWentMeanwhile(t *testing.T) {
+	top := tree(t, "fuse/secret!", "bin/x!", "bin/y!")
+	w := mine()
+	w.mountPoints = map[string]bool{filepath.Join(top, "fuse"): true}
+	w.every = 0
+	p := &SetuidProgress{Mounts: []string{top}}
+	removed := false
+	if err := w.walk(t.Context(), p, func() {
+		if !removed && strings.HasSuffix(p.Cursor[top], "/bin/y") {
+			removed = true
+			os.Remove(filepath.Join(top, "bin/x"))
+		}
+	}); err != nil {
+		t.Fatal(err)
+	}
+	if got := rel(top, p.Found); strings.Join(got, ",") != "bin/y" {
+		t.Fatalf("found %v", got)
+	}
+}
+
+func TestAMountPointIsReadAsTheKernelEscapesIt(t *testing.T) {
+	mp := MountPoints("/dev/a /mnt/a\\040b ext4 rw 0 0\n/dev/b /mnt/back\\134slash ext4 rw 0 0\n")
+	if !mp["/mnt/a b"] || !mp[`/mnt/back\slash`] {
+		t.Fatalf("read %v", mp)
+	}
+}

internal/accounts/ways.go

@@ -355,7 +355,7 @@ func SuidMounts(text string) []string {
 		if len(f) < 4 || pseudoFS[f[2]] || strings.HasPrefix(f[2], "fuse") {
 			continue
 		}
-		at := strings.ReplaceAll(strings.ReplaceAll(f[1], `\040`, " "), `\011`, "\t")
+		at := unescapeMount(f[1])
 		if strings.HasPrefix(at, "/var/lib/docker") || strings.HasPrefix(at, "/var/lib/containers") ||
 			strings.HasPrefix(at, "/proc") || strings.HasPrefix(at, "/sys") {
 			continue
@@ -370,6 +370,45 @@ func SuidMounts(text string) []string {
 	return out
 }
 
+// MountPoints is every mount point /proc/mounts lists, of any type: the walk leaves each it meets inside
+// another filesystem without reading it.
+func MountPoints(text string) map[string]bool {
+	out := map[string]bool{}
+	for _, line := range strings.Split(text, "\n") {
+		if f := strings.Fields(line); len(f) >= 2 {
+			out[unescapeMount(f[1])] = true
+		}
+	}
+	return out
+}
+
+// unescapeMount is a mount point as /proc/mounts escapes it — a space, a tab, a newline and a backslash as
+// three octal digits after a backslash — made a path again.
+func unescapeMount(s string) string {
+	var b strings.Builder
+	for i := 0; i < len(s); i++ {
+		if s[i] == '\\' && i+3 < len(s) && isOctal(s[i+1:i+4]) {
+			b.WriteByte((s[i+1]-'0')<<6 | (s[i+2]-'0')<<3 | (s[i+3] - '0'))
+			i += 3
+			continue
+		}
+		b.WriteByte(s[i])
+	}
+	return b.String()
+}
+
+func isOctal(s string) bool {
+	if len(s) != 3 {
+		return false
+	}
+	for i := 0; i < 3; i++ {
+		if s[i] < '0' || s[i] > '7' {
+			return false
+		}
+	}
+	return true
+}
+
 func contains(xs []string, s string) bool {
 	for _, x := range xs {
 		if x == s {
@@ -384,10 +423,10 @@ func contains(xs []string, s string) bool {
 // search's runner (Exec.SearchRun). **No bound of its own** (novox/hq issue 361): it runs until it has walked
 // everything. A walk that could not complete — a directory it could not read — is an unanswered question, never
 // "none"; a file that vanished while it walked is not.
-func (e Exec) setuidSearch(ctx context.Context, p *SetuidProgress, checkpoint func()) ([]string, error) {
+func (e Exec) setuidSearch(ctx context.Context, p *SetuidProgress, checkpoint func(), mountPoints map[string]bool) ([]string, error) {
 	walk := e.Walk
 	if walk == nil {
-		walk = walker{every: CheckpointEvery}.walk
+		walk = walker{every: CheckpointEvery, mountPoints: mountPoints}.walk
 	}
 	if err := walk(ctx, p, checkpoint); err != nil {
 		return nil, fmt.Errorf("the walk for setuid programs did not complete: %w", err)
@@ -592,7 +631,11 @@ func (e Exec) moreWays(ctx context.Context, account string, uid int, groups []st
 	}
 	// In the background, the last complete result served while it is fresh (SetuidCache): never a stall. No
 	// fresh complete result is said beside the other ways, which are judged all the same.
-	unowned, searchedAt, searchErr := e.Cache.Look(now(), mounts, e.setuidSearch)
+	mountPoints := MountPoints(string(mountsText))
+	unowned, searchedAt, searchErr := e.Cache.Look(now(), mounts,
+		func(ctx context.Context, p *SetuidProgress, checkpoint func()) ([]string, error) {
+			return e.setuidSearch(ctx, p, checkpoint, mountPoints)
+		})
 	var notJudged *SetuidNotJudged
 	if searchErr != nil && !errors.As(searchErr, &notJudged) {
 		return nil, searchErr

internal/accounts/ways_test.go

@@ -275,7 +275,7 @@ func TestThePackageManagerIsAskedOnceForEveryPathFound(t *testing.T) {
 	m.setuid = "/usr/bin/sudo\n/usr/bin/passwd\n/usr/local/bin/x\n/opt/y\n"
 	m.packaged = map[string]bool{"/usr/bin/sudo": true, "/usr/bin/passwd": true}
 	search := func(e Exec) ([]string, error) {
-		return e.setuidSearch(t.Context(), &SetuidProgress{Mounts: []string{"/"}}, func() {})
+		return e.setuidSearch(t.Context(), &SetuidProgress{Mounts: []string{"/"}}, func() {}, nil)
 	}
 	found, err := search(Exec{Run: m.run, Walk: m.walk})
 	if err != nil {

internal/apply/root_moved.go

@@ -16,11 +16,14 @@ var rootPaths = []string{"/etc/passwd", "/etc/shadow", "/etc/group", "/etc/gshad
 // MovesRoot says an apply changed something through which root's reach could have changed — an account, a
 // package, an action run as root, or a file in rootPaths (novox/hq issue 361). The engine then asks for a new
 // search for setuid programs: root acted, and a complete search is sound only until root acts. Outcomes that
-// moved nothing (unchanged, held, waiting, held still) do not count.
+// moved nothing (unchanged, held, waiting, held still, skipped, kept, forgotten) do not count.
 func MovesRoot(outcomes []Outcome) bool {
 	for _, o := range outcomes {
+		// Only what moved the machine: skipped, kept and forgotten touch nothing, and counting them made one
+		// broken module start a search on every reconcile (the review of 2026-10-10).
 		switch o.Action {
-		case "unchanged", "held", "waiting", heldStill:
+		case "created", "updated", "corrected", "removed":
+		default:
 			continue
 		}
 		switch declaration.Type(o.Type) {

internal/apply/root_moved_test.go

@@ -21,6 +21,9 @@ func TestAnApplyMovesRootThroughAnAccountAPackageAnActionOrSudosRules(t *testing
 		{Outcome{Type: "file", Target: "/etc/nginx/nginx.conf", Action: "updated"}, false},
 		{Outcome{Type: "container", Target: "plex", Action: "updated"}, false},
 		{Outcome{Type: "package", Target: "x", Action: heldStill}, false},
+		{Outcome{Type: "action", Target: "setup", Action: "skipped"}, false},
+		{Outcome{Type: "user", Target: "nox", Action: "forgotten"}, false},
+		{Outcome{Type: "package", Target: "x", Action: "kept"}, false},
 	} {
 		if got := MovesRoot([]Outcome{c.o}); got != c.want {
 			t.Errorf("%+v: moves root %v, want %v", c.o, got, c.want)