Merge pull request 'A finding's reference is ref, not token, and a held seat carries its own events (issue 1952)' (#150) from fix/l4-quick into main

go/buscompose/buscompose_test.go

@@ -251,6 +251,7 @@ func TestTheCopiedComposerComposesAsTheControllers(t *testing.T) {
 			if err := json.Unmarshal(want[i].Memberships, &wasM); err != nil {
 				t.Fatal(err)
 			}
+			heldEventsBeyond(gotM, wasM)
 			if !reflect.DeepEqual(gotM, wasM) {
 				t.Errorf("the memberships differ from the controller's:\n%s", raw)
 			}
@@ -825,6 +826,37 @@ func healthChecks(t *testing.T) {
 		URL: "nats://a:4222"}); w.Valid() != nil || w.OwnSecret() != "broker-nats-tools" {
 		t.Errorf("the tools account is held in %q (%v)", w.OwnSecret(), w.Valid())
 	}
+	// Issue 1952: a module that holds a seat says its own seat's events, also when the tool runner carries it, so the
+	// self-check's `checked`, the keeper's condition events and the healers' `healer-acted` reach their hearers. Its
+	// membership lists them, the runtime is granted them; a user of the seat is not, nor a machine's event on a node seat
+	// (granted per machine, to the holder's own account only).
+	selfCheck := Declared{Module: "mesh-self-check", Holds: []Seat{{Name: "mesh-self-check", Scope: "mesh", Emits: []string{"checked"}}}}
+	for _, c := range []struct {
+		name    string
+		d       Declared
+		subject string
+		may     bool
+	}{
+		{"the self-check says checked", selfCheck, "mesh.seat.mesh-self-check.event.checked", true},
+		{"the keeper says a condition raised", Declared{Module: "mesh-conditions", Holds: []Seat{{Name: "mesh-conditions", Scope: "mesh",
+			Emits: []string{"condition-raised", "condition-cleared"}}}}, "mesh.seat.mesh-conditions.event.condition-cleared", true},
+		{"a user does not say the seat's events", Declared{Module: "mesh-delivery-view", Uses: []Seat{{Name: "mesh-self-check",
+			Scope: "mesh", Emits: []string{"checked"}}}}, "mesh.seat.mesh-self-check.event.checked", false},
+		{"the holder says no other seat's", selfCheck, "mesh.seat.mesh-conditions.event.condition-raised", false},
+		{"a machine's event on a node seat is not composed here", Declared{Module: "pause", Holds: []Seat{{Name: "node-power",
+			Scope: "node", Emits: []string{"paused.*"}}}}, "mesh.seat.node-power.event.paused.b", false},
+	} {
+		m := MembershipFor("a", c.d, Placements{})
+		listed := m.SeatTraffic != nil && slices.ContainsFunc(m.SeatTraffic.Publish, func(p string) bool { return p == c.subject })
+		perms, err := PermissionsFor(Principal{Kind: KindNodeTools, Node: "a", Carries: []Declared{c.d}})
+		if err != nil {
+			t.Fatalf("%s: %v", c.name, err)
+		}
+		if listed != c.may || MayPublish(perms, c.subject) != c.may {
+			t.Errorf("%s: the membership lists %s %v and the runtime may publish it %v, want %v", c.name, c.subject, listed,
+				MayPublish(perms, c.subject), c.may)
+		}
+	}
 	seen := map[string]bool{}
 	for _, u := range users {
 		name := u.Username()
@@ -879,3 +911,52 @@ func healthChecks(t *testing.T) {
 		}
 	}
 }
+
+// heldEventsBeyond takes out of the copy's memberships the one row they list beyond the controller's on purpose (issue
+// 1952): a held seat's own events in its holder's seat traffic, so a module the tool runner carries may say them. The
+// controller's composer is frozen; any other difference is still a fault.
+func heldEventsBeyond(got, was any) {
+	gotAll, _ := got.(map[string]any)
+	wasAll, _ := was.(map[string]any)
+	for key, m := range gotAll {
+		gm, _ := m.(map[string]any)
+		traffic, _ := gm["seat-traffic"].(map[string]any)
+		if traffic == nil {
+			continue
+		}
+		listedBefore := map[string]bool{}
+		if wm, _ := wasAll[key].(map[string]any); wm != nil {
+			if wt, _ := wm["seat-traffic"].(map[string]any); wt != nil {
+				for _, p := range asStrings(wt["publish"]) {
+					listedBefore[p] = true
+				}
+			}
+		}
+		var kept []any
+		for _, p := range asStrings(traffic["publish"]) {
+			if parts := strings.Split(p, "."); !listedBefore[p] && len(parts) == 5 && parts[0] == "mesh" && parts[1] == "seat" && parts[3] == "event" {
+				continue
+			}
+			kept = append(kept, p)
+		}
+		if kept == nil {
+			delete(traffic, "publish")
+		} else {
+			traffic["publish"] = kept
+		}
+		if len(traffic) == 0 {
+			delete(gm, "seat-traffic")
+		}
+	}
+}
+
+func asStrings(v any) []string {
+	list, _ := v.([]any)
+	out := make([]string, 0, len(list))
+	for _, x := range list {
+		if s, ok := x.(string); ok {
+			out = append(out, s)
+		}
+	}
+	return out
+}

go/buscompose/seattraffic.go

@@ -58,6 +58,20 @@ func SeatTrafficOf(module string, holds, uses, watches []Seat) SeatTraffic {
 	var t SeatTraffic
 	for _, s := range holds {
 		if !s.isNewTraffic() {
+			// **A holder says its own seat's events** (novox/hq ADR 0564 clause 6, issue 1952): its own account is
+			// granted them by the held seats' rule (PermissionsFor, step 3), and a module the tool runner carries had
+			// no way to, so the self-check's `checked` was never said. Listed here, the membership names them for the
+			// runtime and the runtime is granted them. A machine's event on a node seat carries the machine, which
+			// only the holder's own account is composed with, so it is left to that rule.
+			if servedOnlyByTheController(s) {
+				continue
+			}
+			for _, e := range s.Emits {
+				if s.Scope == "node" && perMachineEvents[e] {
+					continue
+				}
+				t.Publish = append(t.Publish, seatSubject(s, "event", e))
+			}
 			continue
 		}
 		kind := ""

go/health/condition_test.go

@@ -117,7 +117,7 @@ func TestTheHealthLayoutsHoldTheKeepersRules(t *testing.T) {
 	if _, err := DecodeGoodRow([]byte(`{"v":2}`)); err == nil {
 		t.Fatal("a last good night of another version was read")
 	}
-	good := Finding{Key: "provider.keycloak.ace.mesh-mail.failing", Kind: "provider-failing", Token: "failing",
+	good := Finding{Key: "provider.keycloak.ace.mesh-mail.failing", Kind: "provider-failing", Ref: "failing",
 		Subject:  Subject{Scope: ScopeProvider, ID: "keycloak.ace.mesh-mail", Machine: "ace"},
 		Severity: Warning, Summary: "keycloak on ace fails mesh-mail", Source: "S8", Observer: WatchmanModule, At: at}
 	for _, c := range []struct {
@@ -152,6 +152,17 @@ func TestTheHealthLayoutsHoldTheKeepersRules(t *testing.T) {
 		if err != nil || back.Key != f.Key || back.V != FindingVersion || back.Evidence == nil {
 			t.Errorf("%s: read back as %+v, %v", c.name, back, err)
 		}
+		// The tool runner refuses a state value with a field named like a credential (mesh-tools bus/state.go
+		// credentialEndings), so a finding names none, or no observer could write it (issue 1952).
+		if field := credentialNamed(t, b); field != "" {
+			t.Errorf("%s: the finding carries the field %q, which the runtime refuses in state: %s", c.name, field, b)
+		}
+	}
+	// A finding written under the old name `token` is still read, its key's last part kept.
+	if old, err := DecodeFinding([]byte(`{"v":1,"key":"provider.keycloak.ace.mesh-mail.failing","kind":"provider-failing",` +
+		`"token":"failing","subject":{"scope":"provider","id":"keycloak.ace.mesh-mail"},"severity":"warning","summary":"s",` +
+		`"source":"S8","observer":"mesh-delivery-watchman"}`)); err != nil || old.Ref != "failing" {
+		t.Errorf("a finding under the old name read as %+v, %v", old, err)
 	}
 	if FindingsBucket(SelfCheckModule) != "mesh-self-check_findings" || Bucket(HealersModule, AttemptsState) != "mesh-healers_attempts" ||
 		Bucket(KeeperModule, TakeoverState) != "mesh-conditions_takeover" {
@@ -195,3 +206,38 @@ func TestTheHealthLayoutsHoldTheKeepersRules(t *testing.T) {
 		}
 	}
 }
+
+// credentialNamed is the first field of a JSON value whose name ends as mesh-tools' credentialEndings do, or "".
+func credentialNamed(t *testing.T, raw []byte) string {
+	t.Helper()
+	var v any
+	if err := json.Unmarshal(raw, &v); err != nil {
+		t.Fatal(err)
+	}
+	var walk func(any) string
+	walk = func(v any) string {
+		switch x := v.(type) {
+		case map[string]any:
+			for k, inner := range x {
+				norm := strings.NewReplacer("-", "", "_", "", " ", "").Replace(strings.ToLower(k))
+				for _, end := range []string{"password", "passwd", "secret", "token", "credential", "credentials",
+					"authorization", "apikey", "privatekey", "accesskey", "cookie"} {
+					if strings.HasSuffix(norm, end) {
+						return k
+					}
+				}
+				if found := walk(inner); found != "" {
+					return found
+				}
+			}
+		case []any:
+			for _, inner := range x {
+				if found := walk(inner); found != "" {
+					return found
+				}
+			}
+		}
+		return ""
+	}
+	return walk(v)
+}

go/health/findings.go

@@ -50,11 +50,12 @@ const FindingVersion = 1
 // severity, subject and source unchanged, its evidence kept newest first.
 type Finding struct {
 	V int `json:"v"`
-	// Key is Key(Subject.Scope, Subject.ID, Token or Kind): the condition's key.
+	// Key is Key(Subject.Scope, Subject.ID, Ref or Kind): the condition's key.
 	Key  string `json:"key"`
 	Kind string `json:"kind"`
-	// Token is the key's last part when it is not Kind (`silent` for a machine).
-	Token    string   `json:"token,omitempty"`
+	// Ref is the key's last part when it is not Kind (`silent` for a machine). It is not called `token`: the tool
+	// runner refuses any state field named like a credential, so a finding with one could not be written (issue 1952).
+	Ref      string   `json:"ref,omitempty"`
 	Subject  Subject  `json:"subject"`
 	Severity Severity `json:"severity"`
 	// Summary is one line in the mesh's words; Headline, Explanation, Resolved and Needs the plain words of ADR 0253
@@ -82,8 +83,8 @@ type Finding struct {
 func FindingKey(scope, id, token string) string { return Key(scope, id, token) }
 
 func (f Finding) token() string {
-	if f.Token != "" {
-		return f.Token
+	if f.Ref != "" {
+		return f.Ref
 	}
 	return f.Kind
 }
@@ -136,6 +137,14 @@ func DecodeFinding(data []byte) (Finding, error) {
 	if err := json.Unmarshal(data, &f); err != nil {
 		return Finding{}, fmt.Errorf("health: a finding is not readable: %w", err)
 	}
+	if f.Ref == "" {
+		// A finding written before the field was named `ref` (issue 1952) is read by its old name.
+		var old struct {
+			Token string `json:"token"`
+		}
+		_ = json.Unmarshal(data, &old)
+		f.Ref = old.Token
+	}
 	if f.V != FindingVersion {
 		return Finding{}, fmt.Errorf("health: a finding of version %d, and this reads %d", f.V, FindingVersion)
 	}