Leave out a module whose stored manifest has an unknown field, and raise it (hq ADR 0262 review)

A key dropped silently ran a module without what its manifest says, and a key inside a block still
failed the whole catalogue. Judge a key by what it is about, and narrow the listing to one machine.

internal/catalogue/data.go

@@ -159,7 +159,7 @@ func (b *DataBackup) UnmarshalJSON(raw []byte) error {
 	dec := json.NewDecoder(bytes.NewReader(raw))
 	dec.DisallowUnknownFields()
 	if err := dec.Decode(&full); err != nil {
-		return fmt.Errorf("a data item's backup is \"copy\", \"none\" or {dump, into}: %w", err)
+		return fmt.Errorf("a data item's backup is \"copy\", \"none\" or {dump, into}: %w", typedUnknown(err))
 	}
 	*b = DataBackup{Dump: full.Dump, Into: full.Into}
 	return nil

internal/catalogue/declaration.go

@@ -318,6 +318,10 @@ func (e *NotMadeError) Error() string {
 func (r Resolution) LeftOut(settings SettingsBy, adopted bool) map[string]string {
 	out := map[string]string{}
 	for _, m := range r.Modules {
+		if why := UnknownFieldReason(m); why != "" {
+			out[m.Module] = why
+			continue
+		}
 		if err := JudgeSettings(m, settings[m.Module], adopted); err != nil {
 			out[m.Module] = err.Error()
 		}

internal/catalogue/manifest.go

@@ -197,7 +197,7 @@ func (i *OfferIdentity) UnmarshalJSON(raw []byte) error {
 	dec := json.NewDecoder(bytes.NewReader(raw))
 	dec.DisallowUnknownFields()
 	if err := dec.Decode(&full); err != nil {
-		return fmt.Errorf("an offer's identity is false or {max, in}: %w", err)
+		return fmt.Errorf("an offer's identity is false or {max, in}: %w", typedUnknown(err))
 	}
 	*i = OfferIdentity{Max: full.Max, In: full.In}
 	return nil
@@ -316,7 +316,7 @@ func (o *Offer) UnmarshalJSON(raw []byte) error {
 	dec.DisallowUnknownFields()
 	if err := dec.Decode(&full); err != nil {
 		return fmt.Errorf("a provided name is either a string or {name, scope, credential, reach, identity, "+
-			"keeps-consumer-data}: %w", err)
+			"keeps-consumer-data}: %w", typedUnknown(err))
 	}
 	o.Name, o.Scope, o.Credential, o.Reach, o.Identity = full.Name, full.Scope, full.Credential, full.Reach, full.Identity
 	o.KeepsConsumerData = full.Keeps
@@ -465,9 +465,10 @@ type Manifest struct {
 	// stays refused by name until a layer sets it. The mesh's layer, then the node's, override it.
 	Settings map[string]SettingDeclaration `json:"settings,omitempty"`
 
-	// unknown is the first key this manifest has that this controller does not know, when it was read
-	// leniently (novox/hq ADR 0262): a stored manifest written for a newer controller. ParseManifest
-	// refuses it; reading the stored catalogue keeps the rest of the manifest.
+	// unknown is the first key this manifest has that this controller does not know, at any depth, when
+	// it was read from the store (novox/hq ADR 0262): a manifest a newer controller registered.
+	// ParseManifest refuses it; the store's catalogue still loads, and the module is left out of every
+	// machine's declaration by name until the controller is updated (LeftOut).
 	unknown string
 
 	// Data is every kind of data this module keeps — its own, by directory, and what it keeps for
@@ -1279,13 +1280,19 @@ func (m *Manifest) UnmarshalJSON(raw []byte) error {
 	var fields manifestFields
 	unknown := ""
 	if err := decoder.Decode(&fields); err != nil {
-		if !strings.HasPrefix(err.Error(), "json: unknown field ") {
+		if asUnknownField(err) == nil {
 			return err
 		}
+		// Read without it where the key is at the top; where it is inside a block, the block's own
+		// decoder refuses it again, and the manifest keeps its name and version alone. Either way the
+		// module is left out of every declaration by name (LeftOut), so nothing runs on a part-read
+		// manifest.
 		unknown = err.Error()
 		fields = manifestFields{}
-		if err := json.Unmarshal(rest, &fields); err != nil {
-			return err
+		if json.Unmarshal(rest, &fields) != nil {
+			fields = manifestFields{}
+			_ = json.Unmarshal(keys["module"], &fields.Module)
+			_ = json.Unmarshal(keys["version"], &fields.Version)
 		}
 	}
 	*m = Manifest(fields)
@@ -2479,7 +2486,7 @@ func (o *OwnSecrets) UnmarshalJSON(raw []byte) error {
 		dec := json.NewDecoder(bytes.NewReader(body))
 		dec.DisallowUnknownFields()
 		if err := dec.Decode(&long); err != nil {
-			return fmt.Errorf("own-secrets.%s: a path, or {\"path\", \"taken\", \"issued-by\"}: %w", name, err)
+			return fmt.Errorf("own-secrets.%s: a path, or {\"path\", \"taken\", \"issued-by\"}: %w", name, typedUnknown(err))
 		}
 		out[name] = OwnSecret{Path: long.Path, Taken: long.Taken, IssuedBy: long.IssuedBy}
 	}

internal/catalogue/setting_defaults.go

@@ -46,32 +46,54 @@ var meshWords = map[string]bool{
 	PlacesSetting: true, AccessesSetting: true, NetworksSetting: true,
 }
 
-// operatorsOwn are words that, as a whole word of a key's name, say its value is the operator's and
-// never a preference: a default for one would be the literal ADR 0112 removed from definitions. Whole
-// words, split at dashes, underscores and dots, so `max-tokens`, `show-hostname` and `mailbox-size` are
-// preferences and `api-token`, `host` and `mail-domain` are not.
+// operatorsOwn are the words that, as what a key's name is about, say its value is the operator's and
+// never a preference: a default for one would be the literal ADR 0112 removed from definitions.
 var operatorsOwn = map[string]bool{
-	"domain": true, "host": true, "fqdn": true, "zone": true, "realm": true, "tenant": true,
-	"issuer": true, "url": true, "uri": true, "webhook": true, "origin": true, "dsn": true, "ip": true,
+	"domain": true, "host": true, "hostname": true, "servername": true, "fqdn": true, "zone": true,
+	"realm": true, "tenant": true, "site": true, "timezone": true,
+	"issuer": true, "url": true, "uri": true, "webhook": true, "origin": true, "dsn": true,
+	"ip": true, "ipv4": true, "ipv6": true,
 	"email": true, "mail": true, "phone": true, "address": true,
 	"identity": true, "login": true, "user": true, "username": true, "account": true, "owner": true,
 	"uid": true, "gid": true, "puid": true, "pgid": true,
 	"password": true, "pass": true, "passwd": true, "passphrase": true, "secret": true, "token": true,
-	"key": true, "credential": true,
+	"key": true, "apikey": true, "bearer": true, "cert": true, "credential": true,
 }
 
-// operatorsWord is the word of a key's name that says its value is the operator's, or "".
+// operatorsCompounds are names of two words that are the operator's though neither word alone says so
+// at the end of a key: a client's identifier, and a name the world knows a site or server by.
+var operatorsCompounds = map[string]bool{
+	"client-id": true, "site-name": true, "server-name": true, "public-name": true, "smtp-relay": true,
+}
+
+// aboutAnAmount are first words that make a key about how many or whether, never about whom:
+// `max-tokens` is a number, `show-hostname` a switch.
+var aboutAnAmount = map[string]bool{
+	"max": true, "min": true, "num": true, "count": true, "show": true, "hide": true, "enable": true,
+	"disable": true, "use": true, "allow": true,
+}
+
+// operatorsWord is what in a key's name says its value is the operator's, or "". A key is about its
+// last word — `url-timeout` is a timeout, `user-agent` an agent, `mail-domain` a domain — or its last two
+// as one of operatorsCompounds. A plural is read as its singular.
 func operatorsWord(key string) string {
 	words := strings.FieldsFunc(key, func(r rune) bool { return r == '-' || r == '_' || r == '.' })
+	if len(words) == 0 || (len(words) > 1 && aboutAnAmount[words[0]]) {
+		return ""
+	}
 	for i, w := range words {
-		if operatorsOwn[w] {
-			return w
+		if !operatorsOwn[w] && strings.HasSuffix(w, "s") && operatorsOwn[strings.TrimSuffix(w, "s")] {
+			words[i] = strings.TrimSuffix(w, "s")
 		}
-		// An identifier a client is known by: `client-id`, `oauth-client-id`.
-		if w == "client" && i+1 < len(words) && words[i+1] == "id" {
-			return "client-id"
+	}
+	if n := len(words); n > 1 {
+		if pair := words[n-2] + "-" + words[n-1]; operatorsCompounds[pair] {
+			return pair
 		}
 	}
+	if last := words[len(words)-1]; operatorsOwn[last] {
+		return last
+	}
 	return ""
 }
 

internal/catalogue/setting_defaults_test.go

@@ -218,18 +218,24 @@ func TestANodeCalledDefaultIsANodesLayer(t *testing.T) {
 	}
 }
 
-// The operator's own value is told by a whole word of the key's name, not by a part of one.
-func TestAKeyIsTheOperatorsByAWholeWord(t *testing.T) {
+// The operator's own value is told by what a key's name is about: its last word, or its last two as
+// a known compound; a plural as its singular; and never a number or a switch.
+func TestAKeyIsTheOperatorsByWhatItIsAbout(t *testing.T) {
 	for _, key := range []string{"max-tokens", "show-hostname", "ghost-opacity", "users-per-page", "mailbox-size",
-		"font-size", "width", "keyboard-delay", "ipv6-preferred", "client-width"} {
+		"font-size", "width", "keyboard-delay", "ipv6-preferred", "client-width", "user-agent", "url-timeout",
+		"site-title", "cert-renewal-days", "name", "font-name"} {
 		if w := operatorsWord(key); w != "" {
 			t.Errorf("%s read as the operator's (%s)", key, w)
 		}
 	}
-	for key, word := range map[string]string{"mail-domain": "mail", "site-domain": "domain", "api-key": "key", "admin-password": "password",
+	for key, word := range map[string]string{"mail-domain": "domain", "api-key": "key", "admin-password": "password",
 		"oauth-client-id": "client-id", "db-dsn": "dsn", "public-ip": "ip", "puid": "puid", "dns-zone": "zone",
 		"webhook": "webhook", "notify_phone": "phone", "cors.origin": "origin", "smtp-pass": "pass",
-		"backup-passphrase": "passphrase", "data-owner": "owner", "fqdn": "fqdn", "tenant": "tenant", "host": "host"} {
+		"backup-passphrase": "passphrase", "data-owner": "owner", "fqdn": "fqdn", "tenant": "tenant", "host": "host",
+		"allowed-hosts": "host", "admin-emails": "email", "tokens": "token", "hostname": "hostname",
+		"apikey": "apikey", "servername": "servername", "tls-cert": "cert", "site": "site", "timezone": "timezone",
+		"bearer": "bearer", "bind-ipv4": "ipv4", "listen-ipv6": "ipv6", "site-name": "site-name",
+		"server-name": "server-name", "public-name": "public-name", "smtp-relay": "smtp-relay"} {
 		if w := operatorsWord(key); w != word {
 			t.Errorf("%s: read %q, want %q", key, w, word)
 		}
@@ -279,22 +285,50 @@ func TestADefaultFillsAContributionAndAServedFactButNotALiteral(t *testing.T) {
 	}
 }
 
-// A stored manifest with a key this controller does not know is read without it, and said; the module
-// check still refuses it.
-func TestAStoredManifestWithAnUnknownKeyIsReadAndRegistrationRefusesIt(t *testing.T) {
-	raw := []byte(`{"module": "later", "version": "1", "tools": ["later_x"], "a-field-from-later": {"x": 1}}`)
-	var m Manifest
-	if err := json.Unmarshal(raw, &m); err != nil {
-		t.Fatalf("a stored manifest with an unknown key was not read: %v", err)
-	}
-	if m.Module != "later" || len(m.Tools) != 1 || !strings.Contains(m.UnknownField(), `"a-field-from-later"`) {
-		t.Fatalf("read as %+v, unknown %q", m, m.UnknownField())
-	}
-	if _, err := ParseManifest(raw); err == nil || !strings.Contains(err.Error(), `unknown field "a-field-from-later"`) {
-		t.Fatalf("registration took it: %v", err)
+// A stored manifest with a key this controller does not know, at the top or inside any block, is read
+// and its module is left out of every declaration by name; the rest of the catalogue is read; the module
+// check refuses it. One case per block whose decoder wraps the decoder's words in its own.
+func TestAStoredManifestWithAnUnknownKeyIsLeftOutAndRegistrationRefusesIt(t *testing.T) {
+	for where, raw := range map[string]string{
+		"the top":             `{"module": "later", "version": "2", "a-field-from-later": {"x": 1}}`,
+		"a state":             `{"module": "later", "version": "2", "state": [{"name": "s", "a-field-from-later": 1}]}`,
+		"a provided name":     `{"module": "later", "version": "2", "provides": [{"name": "p", "a-field-from-later": 1}]}`,
+		"an offer's identity": `{"module": "later", "version": "2", "provides": [{"name": "p", "identity": {"in": "x", "a-field-from-later": 1}}]}`,
+		"a backup":            `{"module": "later", "version": "2", "data": {"own": [{"id": "d", "path": "${dir:d}", "class": "valuable", "backup": {"dump": "x", "into": "y", "a-field-from-later": 1}}]}}`,
+		"an own secret":       `{"module": "later", "version": "2", "own-secrets": {"s": {"path": "/x", "a-field-from-later": 1}}}`,
+		"a seat's verb":       `{"module": "later", "version": "2", "seats": [{"name": "later-seat", "serves": [{"name": "v", "a-field-from-later": 1}]}]}`,
+	} {
+		var m Manifest
+		if err := json.Unmarshal([]byte(raw), &m); err != nil {
+			t.Errorf("%s: the stored manifest was not read: %v", where, err)
+			continue
+		}
+		if m.Module != "later" || m.Version != "2" || !strings.Contains(m.UnknownField(), `"a-field-from-later"`) {
+			t.Errorf("%s: read as %q %q, unknown %q", where, m.Module, m.Version, m.UnknownField())
+			continue
+		}
+		if where != "the top" && !strings.Contains(m.UnknownField(), ": json: unknown field") {
+			t.Errorf("%s: the block's decoder did not say it: %q", where, m.UnknownField())
+		}
+		left := Resolution{Node: "laptop", Modules: []Manifest{m, notifier()}}.LeftOut(nil, false)
+		if why := left["later"]; !strings.Contains(why, "uses a field this controller does not know") ||
+			!strings.Contains(why, "a-field-from-later") {
+			t.Errorf("%s: not left out by name: %v", where, left)
+		}
+		if _, notifierLeft := left["notifier"]; notifierLeft {
+			t.Errorf("%s: another module was left out with it: %v", where, left)
+		}
+		if _, err := ParseManifest([]byte(raw)); err == nil || !strings.Contains(err.Error(), "a-field-from-later") {
+			t.Errorf("%s: the module check took it: %v", where, err)
+		}
 	}
 	var known Manifest
-	if err := json.Unmarshal([]byte(`{"module": "now"}`), &known); err != nil || known.UnknownField() != "" {
+	if err := json.Unmarshal([]byte(`{"module": "now", "state": [{"name": "s"}]}`), &known); err != nil || known.UnknownField() != "" {
 		t.Fatalf("a known manifest: %v %q", err, known.UnknownField())
 	}
+	// A malformed manifest is still refused: only an unknown key is read past.
+	var bad Manifest
+	if err := json.Unmarshal([]byte(`{"module": "bad", "state": [{"name": 3}]}`), &bad); err == nil {
+		t.Fatal("a malformed stored manifest was read")
+	}
 }

internal/catalogue/state.go

@@ -49,7 +49,7 @@ func (s *StateDeclaration) UnmarshalJSON(raw []byte) error {
 	dec := json.NewDecoder(bytes.NewReader(trimmed))
 	dec.DisallowUnknownFields()
 	if err := dec.Decode(&full); err != nil {
-		return fmt.Errorf("a state is either a name or {name, history, ttl-seconds, per-machine}: %w", err)
+		return fmt.Errorf("a state is either a name or {name, history, ttl-seconds, per-machine}: %w", typedUnknown(err))
 	}
 	*s = StateDeclaration(full)
 	return nil

internal/catalogue/unknown_field.go

@@ -0,0 +1,58 @@
+package catalogue
+
+import (
+	"errors"
+	"regexp"
+)
+
+// A key this controller does not know, in a manifest (novox/hq ADR 0262).
+//
+// Registration refuses it, as it always has. A manifest the store already holds was registered by a newer
+// controller, and is read by this one after a rollback: refusing it there failed the whole catalogue, and
+// with it every plan and every send. Dropping the key silently would be worse — a module running without
+// something its manifest says. So the manifest is read, the module is left out of every machine's
+// declaration by name, and the controller raises a condition until it is updated.
+
+// UnknownFieldError is a key a manifest has that this controller does not know, wherever it is: at the
+// top of the manifest or inside a block (a state, an offer, a data item's backup, an own secret, a verb).
+// Typed, because the blocks' decoders wrap the decoder's words in their own.
+type UnknownFieldError struct {
+	Field string
+	err   error
+}
+
+func (e *UnknownFieldError) Error() string { return e.err.Error() }
+func (e *UnknownFieldError) Unwrap() error { return e.err }
+
+// unknownFieldText is how the JSON decoder words a key its target does not have.
+var unknownFieldText = regexp.MustCompile(`^json: unknown field "([^"]*)"$`)
+
+// typedUnknown is err as an UnknownFieldError when it is the decoder refusing an unknown key, else err.
+func typedUnknown(err error) error {
+	if err == nil {
+		return nil
+	}
+	if m := unknownFieldText.FindStringSubmatch(err.Error()); m != nil {
+		return &UnknownFieldError{Field: m[1], err: err}
+	}
+	return err
+}
+
+// asUnknownField is the unknown key err is about, at any depth, or nil.
+func asUnknownField(err error) *UnknownFieldError {
+	var u *UnknownFieldError
+	if errors.As(typedUnknown(err), &u) {
+		return u
+	}
+	return nil
+}
+
+// UnknownFieldReason is why a module whose stored manifest has a key this controller does not know is
+// left out of a machine's declaration, or "" when it has none.
+func UnknownFieldReason(m Manifest) string {
+	if m.unknown == "" {
+		return ""
+	}
+	return m.Module + " uses a field this controller does not know (" + m.unknown + "); it is left out " +
+		"until the controller is updated (novox/hq ADR 0262)"
+}

internal/catalogue/verbs.go

@@ -56,7 +56,7 @@ func (v *Verb) UnmarshalJSON(raw []byte) error {
 	decoder := json.NewDecoder(bytes.NewReader(trimmed))
 	decoder.DisallowUnknownFields()
 	if err := decoder.Decode(&p); err != nil {
-		return fmt.Errorf("a served verb is a name or {name, description, input, output}: %w", err)
+		return fmt.Errorf("a served verb is a name or {name, description, input, output}: %w", typedUnknown(err))
 	}
 	if p.Name == "" {
 		return fmt.Errorf("a served verb has no name: %s", trimmed)