From 0f853e93fa11aee77f74cab9934d6943fc173338 Mon Sep 17 00:00:00 2001
From: jochen <jschoubben@gmail.com>
Date: Sun, 11 Oct 2026 19:17:28 +0200
Subject: [PATCH] Exempt only the user list's content from the grants step's
 guard, and test the guard in the send itself (review of #230)

The guard let any field of the list's resource move; its path or mode
changing is a change the step must not carry. The store test now calls
sendToEach, so removing the guard there fails it.
---
 cmd/mesh-controller/grants_step_store_test.go |  8 ++++++
 cmd/mesh-controller/grants_step_test.go       | 28 +++++++++++++++++++
 cmd/mesh-controller/release.go                | 22 ++++++++++-----
 3 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/cmd/mesh-controller/grants_step_store_test.go b/cmd/mesh-controller/grants_step_store_test.go
index ae01d6e7..7f675c07 100644
--- a/cmd/mesh-controller/grants_step_store_test.go
+++ b/cmd/mesh-controller/grants_step_store_test.go
@@ -3,11 +3,13 @@ package main
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/novox/mesh-controller/internal/catalogue"
+	"github.com/novox/mesh-controller/internal/identity"
 	"github.com/novox/mesh-controller/internal/inventory"
 	"github.com/novox/mesh-controller/internal/link"
 )
@@ -169,4 +171,10 @@ func TestTheGrantsStepSendsTheUserListAndNothingElse(t *testing.T) {
 	if !strings.Contains(only, "wallpaper newly assigned") || !strings.Contains(only, "+wallpaper") {
 		t.Fatalf("a new assignment on the bus's machine reads %q; want the step refused, naming it", only)
 	}
+	// And the send itself refuses it, unsent: judged where it is composed, before the bus is dialled.
+	identity.ForTest(t)
+	if _, err := sendToEach(kept, open, []string{"anchor"}); !errors.Is(err, errNotGrantsOnly) ||
+		!strings.Contains(err.Error(), "wallpaper") {
+		t.Fatalf("the grants step's send of a new assignment was not refused by the send: %v", err)
+	}
 }
diff --git a/cmd/mesh-controller/grants_step_test.go b/cmd/mesh-controller/grants_step_test.go
index 7592f8f1..2820bb51 100644
--- a/cmd/mesh-controller/grants_step_test.go
+++ b/cmd/mesh-controller/grants_step_test.go
@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"slices"
@@ -140,6 +141,33 @@ func TestAGrantsStepThatWouldCarryMoreIsNotSent(t *testing.T) {
 	}
 }
 
+// The step's exemption is the list's content alone: its path or mode moving is a change like any other.
+func TestOnlyTheUserListsContentIsExempt(t *testing.T) {
+	list := "nats.bus-users"
+	file := func(fields map[string]any) sentResource {
+		r := map[string]any{"id": list, "type": "file", "path": "/conf/accounts.conf", "mode": "0600", "content": "a"}
+		for k, v := range fields {
+			r[k] = v
+		}
+		body, _ := json.Marshal(map[string]any{"resources": []any{r}})
+		s, err := summarize(body)
+		if err != nil {
+			t.Fatal(err)
+		}
+		return s[0]
+	}
+	was := file(nil)
+	if other := otherThanTheList(diffSent([]sentResource{was}, []sentResource{file(map[string]any{"content": "b"})}), list); len(other) != 0 {
+		t.Errorf("the list's new content reads as more: %v", other)
+	}
+	for _, f := range []map[string]any{{"mode": "0644"}, {"path": "/elsewhere"}, {"content": "b", "owner": "nats"}} {
+		other := otherThanTheList(diffSent([]sentResource{was}, []sentResource{file(f)}), list)
+		if len(other) != 1 || !strings.HasPrefix(other[0], "~"+list) {
+			t.Errorf("the list's resource changed by %v reads %v; want it refused", f, other)
+		}
+	}
+}
+
 // The delivery plan says the step before the merge.
 func TestAChangePlanSaysTheGrantsStep(t *testing.T) {
 	const catalogue_ = "http://forge.internal:20000/novox/mesh-catalog.git"
diff --git a/cmd/mesh-controller/release.go b/cmd/mesh-controller/release.go
index 1b872036..8be20f46 100644
--- a/cmd/mesh-controller/release.go
+++ b/cmd/mesh-controller/release.go
@@ -1048,7 +1048,19 @@ func grantsOnlyIn(ctx context.Context, open *stores, holder string, plan catalog
 	if err := json.Unmarshal(prev, &before); err != nil {
 		return "", fmt.Errorf("what %s was last sent is kept in a form this version does not read: %w", holder, err)
 	}
-	d := diffSent(before, after)
+	other = append(other, otherThanTheList(diffSent(before, after), listID)...)
+	sort.Strings(other)
+	if len(other) == 0 {
+		return "", nil
+	}
+	return "its send would change more than the bus's user list: " + strings.Join(other, "; "), nil
+}
+
+// otherThanTheList is what a diff against the last send changes besides the user list's content: every
+// resource added, removed or changed, but the list's own resource when its content is all that changed —
+// its path, mode and every other field must be as last sent (novox/hq issue 490).
+func otherThanTheList(d sentDiff, listID string) []string {
+	var other []string
 	for _, id := range d.Added {
 		other = append(other, "+"+id)
 	}
@@ -1056,16 +1068,12 @@ func grantsOnlyIn(ctx context.Context, open *stores, holder string, plan catalog
 		other = append(other, "-"+id)
 	}
 	for _, c := range d.Changed {
-		if c.ID == listID {
+		if c.ID == listID && len(c.Fields) == 1 && c.Fields[0] == "content" {
 			continue
 		}
 		other = append(other, fmt.Sprintf("~%s (%s)", c.ID, strings.Join(c.Fields, ", ")))
 	}
-	sort.Strings(other)
-	if len(other) == 0 {
-		return "", nil
-	}
-	return "its send would change more than the bus's user list: " + strings.Join(other, "; "), nil
+	return other
 }
 
 // sendJudged is a gated send's sends: the grants step when the user list changed, then the machine judged.