Search for setuid programs in the background, and judge each polkit rule alone

A search over a large disk ran inside the look, holding the judge's lock and
stalling the health statement; it now runs apart, serving its last result,
backing off after a failure, and saying not judged until it has one. And a
rule naming a user, or a comment, no longer hides another rule's
unconditional yes (hq ADR 0266).

internal/accounts/accounts.go

@@ -38,6 +38,7 @@ package accounts
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"sort"
 	"strings"
@@ -241,12 +242,22 @@ func (j *Judge) Look(ctx context.Context) (Statement, bool) {
 func (j *Judge) judge(ctx context.Context, a Account) (string, string) {
 	if a.Root {
 		ways, err := j.reader.Escalation(ctx, a.Name, a.Secrets)
-		if err != nil {
+		var notJudged *SetuidNotJudged
+		switch {
+		case len(ways) > 0:
+			// A way found is a way found, whatever else could not be judged; said beside it.
+			reason := ReasonRoot + ": " + strings.Join(ways, "; ")
+			if errors.As(err, &notJudged) {
+				reason += "; " + firstLine(err.Error())
+			}
+			return Unhealthy, reason
+		case errors.As(err, &notJudged):
+			// The setuid search has no result yet: not judged, said as such — the same words on every look
+			// until it has one, never a search that stalls the look (novox/hq ADR 0266).
+			return Unknown, firstLine(err.Error())
+		case err != nil:
 			return Unknown, "whether the account can become root could not be read: " + firstLine(err.Error())
 		}
-		if len(ways) > 0 {
-			return Unhealthy, ReasonRoot + ": " + strings.Join(ways, "; ")
-		}
 		if len(a.Groups) == 0 {
 			// Declared for root alone: nothing of a session to read.
 			return Healthy, ""

internal/accounts/exec.go

@@ -110,10 +110,12 @@ func (e Exec) Escalation(ctx context.Context, account string, secrets []string)
 		}
 	}
 	more, err := e.moreWays(ctx, account, uid, names, gids, secrets)
-	if err != nil {
+	var notJudged *SetuidNotJudged
+	if err != nil && !errors.As(err, &notJudged) {
 		return nil, err
 	}
-	return append(ways, more...), nil
+	// Every way found, and — when the setuid search has no result yet — that it is not judged.
+	return append(ways, more...), err
 }
 
 // Readable is whether an account of uid, in the groups gids, reads a file of m by its permission bits, as

internal/accounts/ways.go

@@ -30,8 +30,9 @@ var Judged = []string{
 	"any doas rule permitting it or one of its groups (/etc/doas.conf, /etc/opendoas.conf)",
 	"any polkit rule naming it or one of its groups (/etc/polkit-1/rules.d, /usr/share/polkit-1/rules.d, " +
 		"/etc/polkit-1/localauthority), which is how pkexec and systemd's own actions are granted",
-	"a polkit rule that grants every account: a .rules file that answers polkit.Result.YES and names no user " +
-		"and no group, or a .pkla whose Identity is unix-user:* or unix-group:* with a Result of yes",
+	"a polkit rule that grants every account: a rule of a .rules file that answers polkit.Result.YES and names " +
+		"no user and no group, or a .pkla section whose Identity is unix-user:* or unix-group:* with a Result of " +
+		"yes — each rule and section judged on its own, comments taken out",
 	"write access to a container runtime's socket (docker, podman, containerd), by owner, group, other or " +
 		"POSIX ACL",
 	"a container runtime's API listening on TCP port 2375 or 2376 (/proc/net/tcp, /proc/net/tcp6), which any " +
@@ -39,7 +40,8 @@ var Judged = []string{
 	"read access to a secret the mesh placed for another account, by owner, group, other or POSIX ACL",
 	"a program that no installed package owns and is setuid with owner root, or setgid with group root, " +
 		"whoever owns it, on every local filesystem mounted without nosuid (container and image layers, " +
-		"network and pseudo filesystems excepted)",
+		"network and pseudo filesystems excepted) — searched in the background at most hourly, its last result " +
+		"served with when it was found; until a search has finished, the verdict says not judged",
 	"hard links or symbolic links to other accounts' files left unprotected by the kernel " +
 		"(fs.protected_hardlinks or fs.protected_symlinks is 0)",
 }
@@ -203,11 +205,22 @@ func PolkitNames(text, account string, groups []string) bool {
 }
 
 // PolkitGrantsEveryone says whether a polkit rule or authority file grants every account, naming none: a .rules
-// file whose function answers polkit.Result.YES with no condition on a user or a group, or a .pkla whose
-// Identity is every user or every group with a Result of yes.
+// file one of whose rules answers polkit.Result.YES with no condition on a user or a group, or a .pkla one of
+// whose sections has an Identity of every user or every group with a Result of yes.
+//
+// **Each rule is judged on its own, comments taken out first** (the third review of 2026-10-08): a rule naming a
+// user elsewhere in the file, or a user named only in a comment, must not hide another rule's unconditional
+// yes. A rule's text is read, not run: a condition the text does not show is listed under NotJudged.
 func PolkitGrantsEveryone(path, text string) bool {
 	if strings.HasSuffix(path, ".pkla") {
-		for _, section := range strings.Split(text, "[") {
+		var kept []string
+		for _, line := range strings.Split(text, "\n") {
+			if t := strings.TrimSpace(line); strings.HasPrefix(t, "#") || strings.HasPrefix(t, ";") {
+				continue
+			}
+			kept = append(kept, line)
+		}
+		for _, section := range strings.Split(strings.Join(kept, "\n"), "[") {
 			var every, yes bool
 			for _, line := range strings.Split(section, "\n") {
 				k, v, ok := strings.Cut(strings.TrimSpace(line), "=")
@@ -218,7 +231,7 @@ func PolkitGrantsEveryone(path, text string) bool {
 				switch strings.TrimSpace(k) {
 				case "Identity":
 					for _, id := range strings.Split(v, ";") {
-						if id == "unix-user:*" || id == "unix-group:*" {
+						if id = strings.TrimSpace(id); id == "unix-user:*" || id == "unix-group:*" {
 							every = true
 						}
 					}
@@ -234,17 +247,71 @@ func PolkitGrantsEveryone(path, text string) bool {
 		}
 		return false
 	}
-	if !strings.Contains(text, "polkit.Result.YES") {
-		return false
-	}
-	// A condition on a user or a group names whom it grants; one on an active local session grants only an
-	// account with a seat, which an agent started through sudo has not (listed under NotJudged).
-	for _, condition := range []string{".user", "isInGroup", "unix-user:", "unix-group:", ".active", ".local"} {
-		if strings.Contains(text, condition) {
-			return false
+	for _, rule := range polkitRules(stripJSComments(text)) {
+		if !strings.Contains(rule, "polkit.Result.YES") {
+			continue
+		}
+		// A condition on a user or a group names whom it grants; one on an active local session grants only an
+		// account with a seat, which an agent started through sudo has not (listed under NotJudged).
+		conditioned := false
+		for _, condition := range []string{".user", "isInGroup", "unix-user:", "unix-group:", ".active", ".local"} {
+			if strings.Contains(rule, condition) {
+				conditioned = true
+				break
+			}
+		}
+		if !conditioned {
+			return true
 		}
 	}
-	return true
+	return false
+}
+
+// polkitRules is each rule of a .rules file: the text from one polkit.addRule to the next. Text before the first
+// is no rule.
+func polkitRules(text string) []string {
+	parts := strings.Split(text, "addRule(")
+	if len(parts) < 2 {
+		return nil
+	}
+	return parts[1:]
+}
+
+// stripJSComments takes /* … */ and // … comments out of a rule file, leaving what is in strings alone.
+func stripJSComments(text string) string {
+	var b strings.Builder
+	var quote byte
+	for i := 0; i < len(text); i++ {
+		c := text[i]
+		switch {
+		case quote != 0:
+			b.WriteByte(c)
+			if c == '\\' && i+1 < len(text) {
+				i++
+				b.WriteByte(text[i])
+			} else if c == quote {
+				quote = 0
+			}
+		case c == '"' || c == '\'' || c == '`':
+			quote = c
+			b.WriteByte(c)
+		case c == '/' && i+1 < len(text) && text[i+1] == '*':
+			end := strings.Index(text[i+2:], "*/")
+			if end < 0 {
+				return b.String()
+			}
+			i += end + 3
+		case c == '/' && i+1 < len(text) && text[i+1] == '/':
+			end := strings.IndexByte(text[i:], '\n')
+			if end < 0 {
+				return b.String()
+			}
+			i += end - 1
+		default:
+			b.WriteByte(c)
+		}
+	}
+	return b.String()
 }
 
 // RuntimeAPIPorts are the ports a container runtime's API listens on by convention: 2375 plain, 2376 TLS.
@@ -312,29 +379,101 @@ func contains(xs []string, s string) bool {
 	return false
 }
 
-// SetuidCache keeps the search for setuid programs, which walks the root filesystem, for Every: the judge
-// looks every minute, and a setuid program appears only by root's act.
+// SetuidCache keeps the search for setuid programs, which walks every local filesystem, and runs it in the
+// background (novox/hq ADR 0266, the third review): a disk too large to search in time must never stall a
+// look, the health statement or the apply report. A look reads the last result the search left, and starts a
+// new search when the last one is older than Every; a search that failed or did not finish is tried again
+// after a back-off that doubles, from Every up to MaxBackoff.
 type SetuidCache struct {
 	Every time.Duration
-	mu    sync.Mutex
-	at    time.Time
-	found []string
-	err   error
+
+	mu        sync.Mutex
+	wg        sync.WaitGroup
+	at        time.Time // when the last search that finished finished
+	found     []string
+	running   bool
+	startedAt time.Time
+	failedAt  time.Time
+	failed    error
+	backoff   time.Duration
 }
 
-func (c *SetuidCache) get(now time.Time, search func() ([]string, error)) ([]string, error) {
+// SearchBound is how long one search may run in the background; MaxBackoff the longest wait after failures.
+const (
+	SearchBound = 15 * time.Minute
+	MaxBackoff  = 6 * time.Hour
+)
+
+// SetuidNotJudged is the search's answer when it has no result to give: never run to its end yet, or failed
+// every time so far. The judge says the way is not judged, never that there is none.
+type SetuidNotJudged struct {
+	// Pending is true while the first search runs; false after one failed or did not finish.
+	Pending bool
+	Since   time.Time
+	Err     error
+}
+
+func (e *SetuidNotJudged) Error() string {
+	if e.Pending {
+		return fmt.Sprintf("%s: the search for setuid programs, started at %s, has not finished yet", ReasonPending,
+			e.Since.Local().Format("15:04"))
+	}
+	return fmt.Sprintf("%s: the search for setuid programs did not finish (last tried at %s: %v); it is tried "+
+		"again later", ReasonIncomplete, e.Since.Local().Format("15:04"), e.Err)
+}
+
+// The reasons of a root verdict the setuid search could not answer.
+const (
+	ReasonPending    = "not judged yet (search running)"
+	ReasonIncomplete = "not judged (search incomplete)"
+)
+
+// Look answers the last result and when its search finished, starting a search in the background when one is
+// due; it never waits for one. Nil c searches in place, for a caller that wants to.
+func (c *SetuidCache) Look(now time.Time, search func(ctx context.Context) ([]string, error)) ([]string, time.Time, error) {
 	if c == nil {
-		return search()
+		ctx, cancel := context.WithTimeout(context.Background(), SearchBound)
+		defer cancel()
+		found, err := search(ctx)
+		return found, now, err
 	}
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	if c.at.IsZero() || now.Sub(c.at) >= c.Every || c.err != nil {
-		c.found, c.err = search()
-		c.at = now
+	due := !c.running && (c.at.IsZero() || now.Sub(c.at) >= c.Every) &&
+		(c.failed == nil || now.Sub(c.failedAt) >= c.backoff)
+	if due {
+		c.running, c.startedAt = true, now
+		c.wg.Add(1)
+		go func() {
+			defer c.wg.Done()
+			ctx, cancel := context.WithTimeout(context.Background(), SearchBound)
+			found, err := search(ctx)
+			cancel()
+			c.mu.Lock()
+			defer c.mu.Unlock()
+			c.running = false
+			done := time.Now()
+			if err != nil {
+				c.failed, c.failedAt = err, done
+				c.backoff = min(max(2*c.backoff, c.Every), MaxBackoff)
+				return
+			}
+			c.found, c.at, c.failed, c.backoff = found, done, nil, 0
+		}()
+	}
+	switch {
+	case !c.at.IsZero():
+		return c.found, c.at, nil
+	case c.failed != nil:
+		return nil, time.Time{}, &SetuidNotJudged{Since: c.failedAt, Err: c.failed}
+	default:
+		return nil, time.Time{}, &SetuidNotJudged{Pending: true, Since: c.startedAt}
 	}
-	return c.found, c.err
 }
 
+// Wait is for a test: until the search running, if any, has finished.
+func (c *SetuidCache) Wait() { c.wg.Wait() }
+
 // setuidSearch is every setuid- or setgid-root regular file on the root filesystem that no installed package
 // owns, found with find and asked of the package manager. Bounded: a search that does not finish is an
 // unanswered question, never "none".
@@ -516,12 +655,18 @@ func (e Exec) moreWays(ctx context.Context, account string, uid int, groups []st
 	if len(mounts) == 0 {
 		return nil, errors.New("no filesystem a setuid program could run from was found in /proc/mounts")
 	}
-	unowned, err := e.Cache.get(now(), func() ([]string, error) { return e.setuidSearch(ctx, mounts) })
-	if err != nil {
-		return nil, err
+	// In the background, its last result served (SetuidCache): never a stall. No result yet is said beside
+	// the other ways, which are judged all the same.
+	unowned, searchedAt, searchErr := e.Cache.Look(now(), func(sctx context.Context) ([]string, error) {
+		return e.setuidSearch(sctx, mounts)
+	})
+	var notJudged *SetuidNotJudged
+	if searchErr != nil && !errors.As(searchErr, &notJudged) {
+		return nil, searchErr
 	}
 	for _, p := range unowned {
-		ways = append(ways, "a setuid-root program no package owns: "+p)
+		ways = append(ways, "a setuid-root program no package owns: "+p+" (searched at "+
+			searchedAt.Local().Format("15:04")+")")
 	}
 
 	// The kernel's protection of links: off, any account may link another's file where root then acts on it.
@@ -554,6 +699,10 @@ func (e Exec) moreWays(ctx context.Context, account string, uid int, groups []st
 			}
 		}
 	}
+	if notJudged != nil {
+		// The other ways are judged; this one is said as not judged, never as none.
+		return ways, notJudged
+	}
 	return ways, nil
 }
 

internal/accounts/ways_test.go

@@ -131,17 +131,82 @@ func TestAFurtherQuestionUnansweredIsUnknown(t *testing.T) {
 	}
 }
 
-func TestTheSetuidSearchIsKeptForItsInterval(t *testing.T) {
+func TestTheSetuidSearchRunsInTheBackgroundAndIsKeptForItsInterval(t *testing.T) {
 	c := &SetuidCache{Every: time.Hour}
 	searched := 0
-	search := func() ([]string, error) { searched++; return nil, nil }
+	release := make(chan struct{})
+	search := func(context.Context) ([]string, error) { <-release; searched++; return []string{"/opt/x"}, nil }
 	at := time.Date(2026, 10, 8, 19, 0, 0, 0, time.UTC)
-	c.get(at, search)
-	c.get(at.Add(30*time.Minute), search)
-	c.get(at.Add(61*time.Minute), search)
-	if searched != 2 {
-		t.Fatalf("searched %d times in 61 minutes, want 2", searched)
+	_, _, err := c.Look(at, search)
+	var nj *SetuidNotJudged
+	if !errors.As(err, &nj) || !nj.Pending || !strings.HasPrefix(err.Error(), ReasonPending) {
+		t.Fatalf("a look while the first search runs answers at once, not judged yet: %v", err)
 	}
+	if _, _, err := c.Look(at.Add(time.Minute), search); err == nil {
+		t.Fatal("still running: still not judged")
+	}
+	close(release)
+	c.Wait()
+	found, when, err := c.Look(at.Add(30*time.Minute), search)
+	if err != nil || len(found) != 1 || when.IsZero() {
+		t.Fatalf("the result once there: %v %v %v", found, when, err)
+	}
+	c.Look(at.Add(24*time.Hour), search)
+	c.Wait()
+	if searched != 2 {
+		t.Fatalf("searched %d times, want 2: once at first, once when the result was older than its interval", searched)
+	}
+}
+
+func TestASearchThatFailsIsNotJudgedAndBacksOff(t *testing.T) {
+	c := &SetuidCache{Every: time.Hour}
+	tries := 0
+	search := func(context.Context) ([]string, error) { tries++; return nil, errors.New("did not finish") }
+	at := time.Now()
+	c.Look(at, search)
+	c.Wait()
+	_, _, err := c.Look(at.Add(time.Minute), search)
+	var nj *SetuidNotJudged
+	if !errors.As(err, &nj) || nj.Pending || !strings.HasPrefix(err.Error(), ReasonIncomplete) {
+		t.Fatalf("a failed search is not judged (search incomplete): %v", err)
+	}
+	c.Wait()
+	if tries != 1 {
+		t.Fatalf("tried again within its back-off: %d", tries)
+	}
+	c.Look(at.Add(2*time.Hour), search)
+	c.Wait()
+	if tries != 2 {
+		t.Fatalf("tried again after its back-off: %d", tries)
+	}
+}
+
+func TestANotJudgedSearchStillSaysAWayFound(t *testing.T) {
+	j := New(fakeReader{ways: []string{"in the group docker, which grants root"}, err: &SetuidNotJudged{Pending: true, Since: time.Now()}})
+	j.Set([]Account{{Module: "m", ID: "m.a", Name: "agent", Root: true}})
+	st, _ := j.Look(context.Background())
+	if v := st.Accounts[0]; v.State != Unhealthy || !strings.Contains(v.Reason, "docker") || !strings.Contains(v.Reason, ReasonPending) {
+		t.Fatalf("a way found is unhealthy, with the search's state beside it: %+v", v)
+	}
+	j = New(fakeReader{err: &SetuidNotJudged{Since: time.Now(), Err: errors.New("timeout")}})
+	j.Set([]Account{{Module: "m", ID: "m.a", Name: "agent", Root: true}})
+	st, _ = j.Look(context.Background())
+	if v := st.Accounts[0]; v.State != Unknown || !strings.HasPrefix(v.Reason, ReasonIncomplete) {
+		t.Fatalf("nothing found and the search incomplete: unknown, said so: %+v", v)
+	}
+}
+
+type fakeReader struct {
+	ways []string
+	err  error
+}
+
+func (f fakeReader) InDatabase(context.Context, string) ([]string, error) { return nil, nil }
+func (f fakeReader) Session(context.Context, string, []string) (Session, error) {
+	return Session{}, nil
+}
+func (f fakeReader) Escalation(context.Context, string, []string) ([]string, error) {
+	return f.ways, f.err
 }
 
 func TestParseACL(t *testing.T) {
@@ -225,3 +290,31 @@ func TestSuidMountsLeaveOutNosuidPseudoAndContainerLayers(t *testing.T) {
 		t.Fatalf("got %q", got)
 	}
 }
+
+// Each polkit rule is judged on its own, comments taken out (the third review of 2026-10-08).
+func TestAnUnconditionalPolkitYesIsNotMaskedByAnotherRuleOrAComment(t *testing.T) {
+	cases := []struct {
+		name, text string
+		every      bool
+	}{
+		{"another rule names a user", `polkit.addRule(function(a, s) { if (s.user == "operator") return polkit.Result.YES; });
+polkit.addRule(function(a, s) { return polkit.Result.YES; });`, true},
+		{"a user named only in a comment", `// subject.user == "x"
+polkit.addRule(function(a, s) { /* isInGroup("wheel") */ return polkit.Result.YES; });`, true},
+		{"the yes only in a comment", `polkit.addRule(function(a, s) { // return polkit.Result.YES;
+ return polkit.Result.NO; });`, false},
+		{"a condition in the same rule", `polkit.addRule(function(a, s) { if (s.isInGroup("wheel")) { return polkit.Result.YES; } });`, false},
+		{"a comment marker inside a string", `polkit.addRule(function(a, s) { var u = "http://x"; return polkit.Result.YES; });`, true},
+	}
+	for _, c := range cases {
+		if got := PolkitGrantsEveryone("/etc/polkit-1/rules.d/x.rules", c.text); got != c.every {
+			t.Errorf("%s: grants every account %v, want %v", c.name, got, c.every)
+		}
+	}
+	if PolkitGrantsEveryone("/x.pkla", "# [a]\n# Identity=unix-user:*\n# ResultAny=yes\n") {
+		t.Error("a .pkla section only in comments grants nobody")
+	}
+	if !PolkitGrantsEveryone("/x.pkla", "[a]\nIdentity=unix-group:wheel\nResultAny=yes\n[b]\nIdentity=unix-user:*\nResultActive=yes\n") {
+		t.Error("each .pkla section on its own")
+	}
+}