From 55d8b43f303c26fefbc599069aedce8f626e48b5 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 02:54:41 +0200 Subject: [PATCH] Refuse any change through a verb to a module with a trusted mergeable file A mergeable file takes any key, not only those its content names, so an empty runtime configuration a provider reads took a url of the caller's through the settings verb (hq issue 340 review). --- cmd/mesh-controller/modules.go | 18 ++++++ cmd/mesh-controller/terminal_settings_test.go | 62 +++++++++++++++++++ internal/catalogue/terminal_keys.go | 26 ++++++++ 3 files changed, 106 insertions(+) diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index 72dbd622..9f4d306d 100644 --- a/cmd/mesh-controller/modules.go +++ b/cmd/mesh-controller/modules.go @@ -1081,6 +1081,16 @@ func throughAVerb() (string, bool) { return verb, verb != "" } +// sameLayer says whether two layers hold the same values, an absent layer and an empty one alike. +func sameLayer(a, b map[string]any) bool { + if len(a) == 0 && len(b) == 0 { + return true + } + ra, _ := json.Marshal(a) + rb, _ := json.Marshal(b) + return string(ra) == string(rb) +} + // refuseTerminalSettingsThroughAVerb refuses a layer change through a verb that would add, change or remove // places or accesses; a change that leaves both as they were is not refused. func refuseTerminalSettingsThroughAVerb(ctx context.Context, inv *inventory.Inventory, before, after map[string]any, @@ -1095,6 +1105,14 @@ func refuseTerminalSettingsThroughAVerb(ctx context.Context, inv *inventory.Inve if err != nil { return fmt.Errorf("which settings of %s are the terminal's cannot be read, so nothing was changed: %w", module, err) } + // A trusted mergeable file takes any key, so its module's whole layer is the terminal's (novox/hq issue 340). + if files := catalogue.TrustedMergeable(shelf[module]); len(files) > 0 && !sameLayer(before, after) { + return fmt.Errorf("the settings of %s on %s are set at the controller's terminal only, never through a verb (this "+ + "line came through %q): %s merges whatever key a layer sets into a file root or a consumer trusts, so "+ + "any key could point the module at a listener of the caller's, and whoever may call a verb includes "+ + "agents (novox/hq issue 340; a file nothing trusts says \"trusted\": false). Nothing was changed", + module, where, verb, strings.Join(files, ", ")) + } for _, key := range catalogue.TerminalKeys(shelf[module]) { was, _ := json.Marshal(before[key]) now, _ := json.Marshal(after[key]) diff --git a/cmd/mesh-controller/terminal_settings_test.go b/cmd/mesh-controller/terminal_settings_test.go index db17994b..6eb7e6a9 100644 --- a/cmd/mesh-controller/terminal_settings_test.go +++ b/cmd/mesh-controller/terminal_settings_test.go @@ -282,3 +282,65 @@ func TestTheAgentsManagedSettingsAreRefusedThroughAVerb(t *testing.T) { t.Fatalf("reading through the verb: %v", err) } } + +// A mergeable file takes any key, not only those its content names (novox/hq issue 340): an empty runtime +// configuration that a provider reads would take a `url` of the caller's, and the provider would send its admin +// login there. So a module with a mergeable file not marked `"trusted": false` has its whole layer set at the +// terminal: through a verb, any change is refused, whatever the key. +func TestAnyKeyOfAModuleWithATrustedMergeableFileIsRefusedThroughAVerb(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + register(t, open, catalogue.Manifest{Module: "keycloak", Version: "1", + Resources: []map[string]any{{"id": "runtime-config", "type": "file", "path": "/var/lib/kc/runtime.json", + "mode": "0600", "merge": "json", "content": "{}"}}}) + register(t, open, catalogue.Manifest{Module: "notifier", Version: "1", + Resources: []map[string]any{{"id": "look", "type": "file", "path": "/var/lib/notifier/look.json", + "mode": "0644", "merge": "json", "trusted": false, "content": "{}"}}}) + for _, m := range []string{"keycloak", "notifier"} { + if _, err := assign(ctx, open, "anchor", m); err != nil { + t.Fatal(err) + } + } + layer := func(module string) string { + t.Helper() + values, _, err := open.inventory.Layer(ctx, "anchor", module) + if err != nil { + t.Fatal(err) + } + raw, _ := json.Marshal(values) + return string(raw) + } + refused := func(what string, err error) { + t.Helper() + if err == nil || !strings.Contains(err.Error(), "controller's terminal") || !strings.Contains(err.Error(), "issue 340") { + t.Fatalf("%s: %v", what, err) + } + } + refused("a new url through the verb", throughVerb(t, "settings", map[string]any{"module": "keycloak", "node": "anchor", + "values": `{"url":"http://listener.example:8080"}`})) + refused("a new url for the mesh through the verb", throughVerb(t, "settings", map[string]any{"module": "keycloak", + "values": `{"url":"http://listener.example:8080"}`})) + if err := throughVerb(t, "command", map[string]any{"command": `settings set keycloak '{"url":"http://x"}' --node anchor`}); err == nil { + t.Fatal("the command verb set a key of a trusted mergeable file") + } + if got := layer("keycloak"); got != "null" && got != "{}" { + t.Fatalf("a refused call kept a layer: %s", got) + } + if err := atTheTerminal(t, "settings", "set", "keycloak", `{"url":"https://id.example"}`, "--node", "anchor"); err != nil { + t.Fatalf("at the terminal: %v", err) + } + kept := layer("keycloak") + refused("changed", throughVerb(t, "settings", map[string]any{"module": "keycloak", "node": "anchor", + "values": `{"url":"http://listener.example"}`})) + refused("cleared", throughVerb(t, "settings", map[string]any{"module": "keycloak", "node": "anchor", "clear": "true"})) + if got := layer("keycloak"); got != kept { + t.Fatalf("a refused call changed the layer: %s, was %s", got, kept) + } + if err := throughVerb(t, "settings", map[string]any{"module": "keycloak", "node": "anchor"}); err != nil { + t.Fatalf("reading through the verb: %v", err) + } + // A mergeable file that says out loud nothing trusts it stays the verb's. + if err := throughVerb(t, "settings", map[string]any{"module": "notifier", "node": "anchor", "values": `{"font":13}`}); err != nil { + t.Fatalf("a file marked untrusted through the verb: %v", err) + } +} diff --git a/internal/catalogue/terminal_keys.go b/internal/catalogue/terminal_keys.go index e2d975b7..cd5bb8e1 100644 --- a/internal/catalogue/terminal_keys.go +++ b/internal/catalogue/terminal_keys.go @@ -30,6 +30,10 @@ import ( // managed settings and tool servers every Claude Code session on a node obeys in one, and through a verb an // agent could have given every person's session a hook of its own. // +// 4. **A module's whole layer, when it has a mergeable file not marked `"trusted": false`** (novox/hq issue 340, +// TrustedMergeable). A mergeable file takes any key a layer sets, not only those its content names, so no list +// of keys covers it: an empty runtime configuration a provider reads would take a `url` of the caller's. +// // Derived from the manifest, never listed by hand, so a provider or a trusted file added tomorrow is covered. // TrustedField is the key a file resource carries to say whether the settings it asks for are trusted: true, or @@ -92,6 +96,28 @@ func UnsaidTrust(m Manifest) []string { return out } +// TrustedMergeable is every mergeable file of a module not marked `"trusted": false`, by id (novox/hq issue 340). +// A mergeable file takes any key a layer sets, not only those its content names: an empty runtime configuration a +// provider reads takes a `url` of a caller's as surely as a declared one. So a module holding one has its whole +// layer set at the controller's terminal; a verb may read it and change nothing. +func TrustedMergeable(m Manifest) []string { + var out []string + for _, r := range m.Resources { + if fmt.Sprint(r["type"]) != "file" { + continue + } + if how, _ := r["merge"].(string); how == "" { + continue + } + if trusted, said := r[TrustedField].(bool); said && !trusted { + continue + } + out = append(out, fmt.Sprint(r["id"])) + } + sort.Strings(out) + return out +} + // asksFor is every setting a file resource asks for: each `${setting:…}` in its content and, for a mergeable file, // every key at the top of the content it merges into (novox/hq issue 340). Those are the keys the file declares it // takes: a layer's value for one of them lands in it as surely as a placeholder would be filled. A key the content