mesh-grants: refuse a take-over over an uncompared deny; ask words only for added rights

Review round 1 of issue 1945's fix found two ways the guard could let a loss
through or hide one from the operator.

A publish deny was never compared: grants.User carried no deny, so a widened
allow that brings a new deny (a lost publish right) passed as an added right.
The user now carries its deny; a deny the shadow adds is a loss, and while
nats_users answers no deny for the loaded list, a user whose grants differ
and whose shadow grant has a deny refuses, named as a deny the loaded list
cannot be compared on. None of today's 41 differing users carries a deny
(only the four node-tools users do, and they are the same in both lists).

The ask said "takes none away" whenever rights were added, whatever else
differed. Its words now name the added rights only when nothing else refuses
(Refusals: unread list, Removing, a missing hash), and otherwise say what the
yes would be refused for, naming the users; Decide uses the same Refusals.

issue 1945

modules/mesh-grants/internal/grants/part.go

@@ -299,7 +299,7 @@ func (p *Part) Compose() Result {
 		}
 	}
 	if !p.deciding.Load() {
-		if err := p.RaiseRequested(res.UserList.Compare); err != nil {
+		if err := p.RaiseRequested(res.UserList); err != nil {
 			res.Errors = append(res.Errors, err.Error())
 		}
 		if err := p.Reconcile(); err != nil {

modules/mesh-grants/internal/grants/takeover.go

@@ -5,7 +5,6 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
-	"sort"
 	"strings"
 	"time"
 
@@ -179,7 +178,7 @@ func (p *Part) request(act, caller, why string) (any, error) {
 
 // RaiseRequested is the process's: it asks the operator the request a tool left, once, naming the rights the pass's
 // comparison c adds. Where the part has no asker (the tools) it does nothing.
-func (p *Part) RaiseRequested(c Comparison) error {
+func (p *Part) RaiseRequested(st UserListState) error {
 	if p.Asker == nil {
 		return nil
 	}
@@ -197,12 +196,12 @@ func (p *Part) RaiseRequested(c Comparison) error {
 	if s, err := p.readSwitch(); err != nil || s.Open != r.ID {
 		return nil // an ask no longer open is not asked
 	}
-	ask, err := p.AskOf(r, c)
+	ask, err := p.AskOf(r, st)
 	if err != nil {
 		return err
 	}
 	if r.Act == ActTakeOver {
-		r.Adds = c.AddedDigest()
+		r.Adds = st.Compare.AddedDigest()
 	}
 	if err := p.Asker.Ask(ask); err != nil {
 		// Closed as refused, and the half-made record the asker kept before publishing is taken away, so nothing stays open.
@@ -302,8 +301,10 @@ func (p *Part) Following() string {
 }
 
 // AskOf is the ask of a request, in the words the operator reads: what changes, why, what it risks; for a take-over every
-// right the grants part's list adds by the comparison c, by user (ADR 0562 §3). Its yes binds the act.
-func (p *Part) AskOf(r Request, c Comparison) (asks.Ask, error) {
+// right the grants part's list adds by the user list st compares now, by user, or what a yes would be refused for
+// (ADR 0562 §3, TakeOverWords). Its yes binds the act.
+func (p *Part) AskOf(r Request, st UserListState) (asks.Ask, error) {
+	c := st.Compare
 	verb, headline, does, stays, expl := "grants_take_over", "Switch the bus's user list to the grants part?", "The switch is recorded; it takes effect when the bus node loads the grants part's list.",
 		"The controller's list stays in force.", "The grants part has composed the user list the bus holds today. Your yes records the switch; it takes effect when the bus node loads the part's list.\n"+
 			"If the lists differ other than by an added right, or a user has no hash in the vault, the switch is refused and you are told which."
@@ -313,9 +314,14 @@ func (p *Part) AskOf(r Request, c Comparison) (asks.Ask, error) {
 	}
 	details := "Asked by " + r.By + ": " + r.Why
 	act := asks.Act{"verb": verb, "module": "mesh-grants", "ask": r.ID}
-	if summary, points, list := c.AddedWords(); r.Act == ActTakeOver && summary != "" {
-		expl = summary + " Your yes records the switch; it takes effect when the bus node loads the part's list.\n" + strings.Join(points, "\n")
-		details = "Added, by user:\n" + list + "\nA user added or dropped, a right taken away or a missing hash refuses the switch, naming them.\n" + details
+	if summary, points, list := TakeOverWords(c, st.Credentials); r.Act == ActTakeOver && summary != "" {
+		if len(Refusals(c, st.Credentials)) > 0 {
+			expl = summary + "\n" + strings.Join(points, "\n")
+			details = "Refused for:\n" + list + "\n" + details
+		} else {
+			expl = summary + " Your yes records the switch; it takes effect when the bus node loads the part's list.\n" + strings.Join(points, "\n")
+			details = "Added, by user:\n" + list + "\nA user added or dropped, a right taken away or a missing hash refuses the switch, naming them.\n" + details
+		}
 		act["adds"] = c.AddedDigest()
 	}
 	bind, err := asks.ActDigest(act)
@@ -418,31 +424,11 @@ func (p *Part) decide(ask string) []string {
 	p.deciding.Store(true)
 	res := p.Compose()
 	p.deciding.Store(false)
-	var reasons []string
-	if c := res.UserList.Compare; !c.Equal() {
-		if c.Unread != "" {
-			reasons = append(reasons, "the loaded list could not be read: "+c.Unread)
-		}
-		if d := c.Removing(); len(d) > 0 {
-			reasons = append(reasons, fmt.Sprintf("%d users differ between the shadow list and the loaded list other than by an added right: %s", len(d), strings.Join(d, ", ")))
-		}
-	}
+	reasons := Refusals(res.UserList.Compare, res.UserList.Credentials)
 	if ask != "" && p.askedAdds(ask) != res.UserList.Compare.AddedDigest() {
 		reasons = append(reasons, "the rights the grants part's list adds changed since you were asked, so the yes does not cover them: ask the take-over again")
 	}
 	reasons = append(reasons, p.followReasons(res, ReadInputs(p.Read, &p.cache))...)
-	cr, gone := res.UserList.Credentials, res.UserList.Compare.Gone
-	var unhashed []string
-	for _, u := range append(append([]string(nil), cr.LeftOut...), cr.LosesLogin...) {
-		if !contains(gone, u) {
-			unhashed = append(unhashed, u)
-		}
-	}
-	if len(unhashed) > 0 {
-		sort.Strings(unhashed)
-		reasons = append(reasons, fmt.Sprintf("the vault holds no hash for %d users (ADR 0547 clause 9: handed or adopted before take-over): %s",
-			len(unhashed), strings.Join(unhashed, ", ")))
-	}
 	if len(reasons) == 0 {
 		if s, err := p.readSwitch(); err == nil && len(s.Refusing) > 0 {
 			p.clearRefused(&s)

modules/mesh-grants/internal/grants/userlist.go

@@ -21,6 +21,9 @@ type User struct {
 	Publish        []string `json:"publish"`
 	Subscribe      []string `json:"subscribe"`
 	AllowResponses bool     `json:"allow_responses"`
+	// PublishDeny is what the user may not publish though an allow reaches it (buscompose's ControllerOnly). The bus
+	// module's nats_users answers none today, so a loaded user's is nil, read as unknown; empty and not nil once it says.
+	PublishDeny []string `json:"publish_deny,omitempty"`
 	// Hash is the bcrypt hash the vault holds for the user (WithHashes): in memory alone, never in a digest, a compare
 	// or the part's state, and empty for a loaded user.
 	Hash string `json:"-"`
@@ -126,7 +129,7 @@ func ComposeUsers(r buscompose.Records) ([]User, error) {
 			return nil, fmt.Errorf("%s: %w", p.Username(), err)
 		}
 		out = append(out, User{User: p.Username(), Publish: set(perms.Publish), Subscribe: set(perms.Subscribe),
-			AllowResponses: perms.AllowResponses})
+			AllowResponses: perms.AllowResponses, PublishDeny: set(perms.PublishDeny)})
 	}
 	sort.Slice(out, func(i, j int) bool { return out[i].User < out[j].User })
 	return out, nil
@@ -224,6 +227,16 @@ type UserDiff struct {
 	SubscribeAdded   []string `json:"subscribe_added,omitempty"`
 	SubscribeDropped []string `json:"subscribe_dropped,omitempty"`
 	Responses        string   `json:"responses,omitempty"`
+	// DenyAdded is what the shadow list denies the user to publish and the loaded list does not: a right lost. DenyUnread
+	// is the shadow's deny of a user whose grants differ, where the loaded list's deny cannot be read (nats_users answers
+	// none): it cannot be compared, so it is no added right either (ADR 0562 §2).
+	DenyAdded  []string `json:"deny_added,omitempty"`
+	DenyUnread []string `json:"deny_unread,omitempty"`
+}
+
+// same says the user's grants agree in both lists.
+func (d UserDiff) same() bool {
+	return len(d.PublishAdded)+len(d.PublishDropped)+len(d.SubscribeAdded)+len(d.SubscribeDropped)+len(d.DenyAdded) == 0 && d.Responses == ""
 }
 
 // Comparison is the shadow list against the loaded list.
@@ -292,16 +305,23 @@ func (c Comparison) Removing() []string {
 		if d.responsesLost() {
 			lost = append(lost, "its allowance of responses")
 		}
-		if len(lost) > 0 {
+		if n := len(d.DenyAdded); n > 0 {
+			lost = append(lost, fmt.Sprintf("%d publish to a deny added", n))
+		}
+		switch {
+		case len(lost) > 0:
 			out = append(out, d.User+" (would lose "+strings.Join(lost, " and ")+")")
+		case len(d.DenyUnread) > 0:
+			out = append(out, d.User+" (a deny the loaded list cannot be compared on)")
 		}
 	}
 	sort.Strings(out)
 	return out
 }
 
-// AddedRights are the users whose grants the shadow list widens and nothing else, with what it adds: the differences of a
-// comparison whose Removing is empty (ADR 0562 §1).
+// AddedRights are the rights the shadow list adds, by user: every user's added publish and subscribe rights and an
+// allowance of responses gained, whatever else the comparison holds. They are the whole difference only where Removing is
+// empty, and only then is a take-over allowed for them (ADR 0562 §1, §2).
 func (c Comparison) AddedRights() []UserDiff {
 	var out []UserDiff
 	for _, d := range c.Differ {
@@ -395,7 +415,7 @@ const AddedWordsBudget = 2200
 // a name that would not pass written with spaces) and AddedWordsBudget, past which the last lines are counted, not named.
 func (c Comparison) AddedWords() (summary string, points []string, list string) {
 	adds := c.AddedRights()
-	if len(adds) == 0 {
+	if len(adds) == 0 || c.Unread != "" || len(c.Removing()) > 0 {
 		return "", nil, ""
 	}
 	total, byKind, usersOf := 0, map[string]int{}, map[string]int{}
@@ -467,6 +487,64 @@ func count(n int, thing string) string {
 	return fmt.Sprintf("%d %ss", n, thing)
 }
 
+// Refusals are why a take-over is refused for the lists themselves, each naming its users: the loaded list unread, any user
+// the lists keep apart other than by an added right (Removing), and any user with no hash in the vault but those both
+// lists agree are gone. Empty when the lists differ at most by added rights and every login is kept.
+func Refusals(c Comparison, cr Credentials) []string {
+	var reasons []string
+	if c.Unread != "" {
+		reasons = append(reasons, "the loaded list could not be read: "+c.Unread)
+	}
+	if d := c.Removing(); len(d) > 0 {
+		reasons = append(reasons, fmt.Sprintf("%d users differ between the shadow list and the loaded list other than by an added right: %s", len(d), strings.Join(d, ", ")))
+	}
+	var unhashed []string
+	for _, u := range append(append([]string(nil), cr.LeftOut...), cr.LosesLogin...) {
+		if !slices.Contains(c.Gone, u) {
+			unhashed = append(unhashed, u)
+		}
+	}
+	if len(unhashed) > 0 {
+		sort.Strings(unhashed)
+		reasons = append(reasons, fmt.Sprintf("the vault holds no hash for %d users (ADR 0547 clause 9: handed or adopted before take-over): %s",
+			len(unhashed), strings.Join(unhashed, ", ")))
+	}
+	return reasons
+}
+
+// TakeOverWords are the take-over ask's words for the lists as compared now: the rights the grants part's list adds
+// (AddedWords) when nothing else keeps them apart, or else what a yes would be refused for, the users named in its list
+// (ADR 0562 §2, §3). Empty when the lists agree. Held to the content rule: the loaded list's read error is not said.
+func TakeOverWords(c Comparison, cr Credentials) (summary string, points []string, list string) {
+	refusals := Refusals(c, cr)
+	if len(refusals) == 0 {
+		return c.AddedWords()
+	}
+	summary = "Today a yes would be refused: the lists differ other than by an added right, the loaded list is unread, or a user has no hash."
+	var lines []string
+	if c.Unread != "" {
+		lines = append(lines, "The loaded list could not be read.")
+		refusals = refusals[1:]
+	}
+	used := 0
+	for _, r := range refusals {
+		if used+len([]rune(r)) > AddedWordsBudget-80 {
+			r = string([]rune(r)[:max(0, AddedWordsBudget-80-used)]) + "… grants_take_over_state says the rest."
+		}
+		lines = append(lines, r)
+		used += len([]rune(r)) + 1
+		if used >= AddedWordsBudget-80 {
+			break
+		}
+	}
+	for i := range lines {
+		if _, ok := outward.Check(lines[i]); !ok {
+			lines[i] = strings.ReplaceAll(lines[i], ".", " ")
+		}
+	}
+	return summary, []string{"Your yes now is refused and said, naming them; ask again once they agree."}, strings.Join(lines, "\n")
+}
+
 // plainName is a user's name as the operator channel's content rule lets it pass: as it is, or with spaces for its dots
 // where it reads as a host name.
 func plainName(user string) string {
@@ -497,10 +575,17 @@ func Compare(shadow, loaded []User, placed func(user string) bool) Comparison {
 		if s.AllowResponses != l.AllowResponses {
 			d.Responses = fmt.Sprintf("shadow %v, loaded %v", s.AllowResponses, l.AllowResponses)
 		}
-		if len(d.PublishAdded)+len(d.PublishDropped)+len(d.SubscribeAdded)+len(d.SubscribeDropped) == 0 && d.Responses == "" {
+		if l.PublishDeny != nil {
+			d.DenyAdded = minus(s.PublishDeny, l.PublishDeny)
+		}
+		if d.same() {
+			// The same allows compose the same deny: one composer derives it from them.
 			c.Same++
 			continue
 		}
+		if l.PublishDeny == nil {
+			d.DenyUnread = s.PublishDeny
+		}
 		c.Differ = append(c.Differ, d)
 	}
 	for _, l := range loaded {
@@ -567,7 +652,7 @@ func (c Comparison) Excusing(f func(user, subject string, added, subscribe bool)
 		}
 		d.PublishAdded, d.PublishDropped = keep(d.PublishAdded, true, false), keep(d.PublishDropped, false, false)
 		d.SubscribeAdded, d.SubscribeDropped = keep(d.SubscribeAdded, true, true), keep(d.SubscribeDropped, false, true)
-		if len(d.PublishAdded)+len(d.PublishDropped)+len(d.SubscribeAdded)+len(d.SubscribeDropped) == 0 && d.Responses == "" {
+		if d.same() {
 			c.Same++
 			continue
 		}