diff --git a/go/buscompose/buscompose_test.go b/go/buscompose/buscompose_test.go index 9412fac..42c4312 100644 --- a/go/buscompose/buscompose_test.go +++ b/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 +} diff --git a/go/buscompose/seattraffic.go b/go/buscompose/seattraffic.go index e89b208..715c4b3 100644 --- a/go/buscompose/seattraffic.go +++ b/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 := "" diff --git a/go/health/condition_test.go b/go/health/condition_test.go index f0085b7..066f6e3 100644 --- a/go/health/condition_test.go +++ b/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) +} diff --git a/go/health/findings.go b/go/health/findings.go index 393a52f..00a5acd 100644 --- a/go/health/findings.go +++ b/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) }