From 1b502a37e0bd286b0b4ae5703a67f77208d39ffa Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 00:28:08 +0200 Subject: [PATCH 1/7] Let only the authority's root hold lines, refuse every line end, and keep places off the runtime's data and any .ssh The review of hq issue 339 found the PEM exception too wide (any module, any key, any label, anything base64), \v, \f, NEL and the Unicode separators still let a value end a line in some readers, and the spool, /opt, the container runtimes' data and an account's .ssh still placeable. Lines are now taken only in step-ca's root setting, as certificates encoding/pem decodes and x509 parses; every line end is refused; and those paths are the machine's own. The test certificate is a real one, made for the tests with its key thrown away. --- internal/catalogue/co_located_test.go | 11 +- internal/catalogue/placement.go | 18 +++- internal/catalogue/settings.go | 100 +++++++++---------- internal/catalogue/terminal_settings_test.go | 86 ++++++++++++---- 4 files changed, 137 insertions(+), 78 deletions(-) diff --git a/internal/catalogue/co_located_test.go b/internal/catalogue/co_located_test.go index bf098c16..3373936f 100644 --- a/internal/catalogue/co_located_test.go +++ b/internal/catalogue/co_located_test.go @@ -17,9 +17,16 @@ import ( // // 04-ISSUES/038 was the first of them (the port). These are the rest. -// A root certificate, in the shape a certificate authority serves one. +// A root certificate, as a certificate authority serves one: a real, self-signed one made for these tests (its key +// thrown away), because the one setting that may hold lines must parse as a certificate (novox/hq issue 339). const servedRoot = `-----BEGIN CERTIFICATE----- -MIIBeDCCAR2gAwIBAgIQfake000000000000000000000000 +MIIBPjCB8aADAgECAhRZG3p93hUUB5bz00uxhYo/nJnTQjAFBgMrZXAwFDESMBAG +A1UEAwwJdGVzdCByb290MCAXDTI2MTAwODIyMjE0NloYDzIxMjYwOTE0MjIyMTQ2 +WjAUMRIwEAYDVQQDDAl0ZXN0IHJvb3QwKjAFBgMrZXADIQA0pt/ld+W0MXwBhPfO +cuAt56kIW6Qcn+4vqWpuvHTiqaNTMFEwHQYDVR0OBBYEFIjgaLk4OVGIOeZQiqnN +p9vhciFjMB8GA1UdIwQYMBaAFIjgaLk4OVGIOeZQiqnNp9vhciFjMA8GA1UdEwEB +/wQFMAMBAf8wBQYDK2VwA0EAl9uWeSM2XAV8u0reyV3BLRxNVik+4FCRO1QKPs2k +IlB1rK9oAOYManH+VFuBMI/JJ31ajSti81q4E0CSw8r5DQ== -----END CERTIFICATE----- ` diff --git a/internal/catalogue/placement.go b/internal/catalogue/placement.go index 1f6a7e38..808151ea 100644 --- a/internal/catalogue/placement.go +++ b/internal/catalogue/placement.go @@ -54,18 +54,28 @@ var ownerShape = regexp.MustCompile(`^[0-9]+:[0-9]+$`) // before anything is kept or composed, and the node-engine refuses them again where it applies. // // systemTrees are refused at and below: the machine's system, the kernel's, the boot loader's, root's home, -// what lives only while the machine runs, and the node-engine's and the mesh's own state. systemRoots are +// what lives only while the machine runs, the spool (cron's tables are there), the container runtimes' data +// (every container's filesystem), /opt, and the node-engine's and the mesh's own state. Any home's .ssh is +// refused as well, wherever the home is. systemRoots are // refused at, and wherever a path holds one (an ancestor of /var/lib holds it): each is the parent of every // module's or every person's directories, and owning it is owning all of them. var ( systemTrees = []string{"/etc", "/usr", "/boot", "/root", "/run", "/var/run", "/var/lock", "/proc", "/sys", - "/dev", "/bin", "/sbin", "/lib", "/lib32", "/lib64", "/var/lib/mesh", "/var/lib/mesh-host"} - systemRoots = []string{"/", "/var", "/var/lib", "/var/cache", "/var/log", "/var/tmp", "/var/spool", "/home", - "/mnt", "/media", "/srv", "/opt", "/tmp", "/storage", "/data", "/services"} + "/dev", "/bin", "/sbin", "/lib", "/lib32", "/lib64", "/var/lib/mesh", "/var/lib/mesh-host", "/var/spool", + "/var/lib/docker", "/var/lib/containers", "/opt"} + systemRoots = []string{"/", "/var", "/var/lib", "/var/cache", "/var/log", "/var/tmp", "/home", + "/mnt", "/media", "/srv", "/tmp", "/storage", "/data", "/services"} ) // systemPath says why a clean absolute path is the machine's own and never a placement's or an access's, or "". func systemPath(path string) string { + // Any home's keys, wherever the home is: a .ssh directory is its account's, and the keys and the list of who + // may log in as it are in there. + for _, part := range strings.Split(path, "/") { + if part == ".ssh" { + return path + " is an account's .ssh, which holds its keys and who may log in as it" + } + } for _, tree := range systemTrees { if path == tree || strings.HasPrefix(path, tree+"/") { return path + " is in " + tree + ", the machine's own or the mesh's state" diff --git a/internal/catalogue/settings.go b/internal/catalogue/settings.go index f7042491..2fab2177 100644 --- a/internal/catalogue/settings.go +++ b/internal/catalogue/settings.go @@ -1,10 +1,14 @@ package catalogue import ( + "bytes" + "crypto/x509" "encoding/json" + "encoding/pem" "fmt" "regexp" "sort" + "strconv" "strings" ) @@ -209,18 +213,25 @@ func deepCopy(in map[string]any) map[string]any { return out } -// settingsHoldOneLine refuses a line break, a carriage return or a NUL in any string of any setting, for every +// settingsHoldOneLine refuses a line break, a carriage return, any other line end (lineEnds) or a NUL in any +// string of any setting, for every // module, at any depth, keys as well as values, in an object or a list (novox/hq issue 339). A value is // substituted into env and configuration files the node-engine writes as root — an app's env file, a logind // drop-in — and a line break there is a line of the caller's own: a directive, an assignment, a section. A NUL // ends a string early wherever C reads it. Judged where a layer is kept and again where it is composed, so a // layer that holds one, however it got into the store, is said with its key. What must hold lines is a file -// of the module's own, never a setting. +// of the module's own, never a setting — with one exception, the certificate authority's root as certificates +// (certificates), because that is how step-ca serves it. func settingsHoldOneLine(module string, layers []Layer) error { for _, layer := range layers { for _, key := range sortedKeysAny(layer.Values) { + if module == rootModule && key == rootSetting { + if text, ok := layer.Values[key].(string); ok && certificates(text) { + continue + } + } if at := lineBreakIn(layer.Values[key], key); at != "" { - return fmt.Errorf("%s: the setting %s in %q holds a line break, a carriage return or a NUL, which a "+ + return fmt.Errorf("%s: the setting %s in %q holds a line break, a carriage return, another line end or a NUL, which a "+ "file it is written into would read as a line of its own; a setting is one line (novox/hq "+ "issue 339)", module, at, layer.From) } @@ -229,66 +240,51 @@ func settingsHoldOneLine(module string, layers []Layer) error { return nil } -// pemBlocks says whether a value is PEM blocks and nothing else: the one value with lines a setting may hold, -// because a provider serves its certificate authority's root to its consumers as one (step-ca to the route -// proxy). Each block is a BEGIN line, base64 lines of exactly 64 characters but the last, and the END line of -// the same label; the last line is a whole number of base64 groups, padded at its end alone. So no line of it -// is a path, an option, a section or an assignment of a name a program reads — a line break with anything -// else around it is refused. -func pemBlocks(v string) bool { - lines := strings.Split(strings.TrimSuffix(v, "\n"), "\n") - if len(lines) < 3 { - return false - } - for i := 0; i < len(lines); { - label, ok := strings.CutPrefix(lines[i], "-----BEGIN ") - if !ok || !strings.HasSuffix(label, "-----") { - return false - } - label = strings.TrimSuffix(label, "-----") - if label == "" || strings.Trim(label, "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 ") != "" { - return false - } - i++ - body := 0 - for i < len(lines) && !strings.HasPrefix(lines[i], "-----END ") { - body++ - i++ - } - if body == 0 || i == len(lines) || lines[i] != "-----END "+label+"-----" { - return false - } - for j, line := range lines[i-body : i] { - if !base64Line(line, j == body-1) { - return false - } - } - i++ - } - return true -} +// lineEnds are every character a reader may take as the end of a line: \n and \r, vertical tab and form feed, +// NEL, and the Unicode line and paragraph separators; and NUL, which ends a string early wherever C reads it. +const lineEnds = "\n\r\v\f\x00\u0085\u2028\u2029" -// base64Line is one line of a PEM body: 64 characters of the base64 alphabet, or for the last line at most 64, -// a whole number of groups, with at most two '=' at its end. -func base64Line(line string, last bool) bool { - if line == "" || len(line) > 64 || (!last && len(line) != 64) || len(line)%4 != 0 { +// rootSetting is the one setting that may hold lines: the certificate authority's root, which step-ca serves to +// its consumers as the setting `root` and which they write into a bundle file (the co-located path, issue 038). +const ( + rootModule = "step-ca" + rootSetting = "root" +) + +// certificates says whether a value is one or more PEM CERTIFICATE blocks and nothing else: each decodes with +// encoding/pem, carries no headers, and parses with x509.ParseCertificate; nothing but a final line break +// surrounds them, and every line ends with \n alone. +func certificates(v string) bool { + if strings.ContainsAny(v, "\r\v\f\x00\u0085\u2028\u2029") { return false } - data := strings.TrimRight(line, "=") - if len(line)-len(data) > 2 || (!last && data != line) { - return false + rest := []byte(v) + found := 0 + for len(rest) > 0 { + if !bytes.HasPrefix(rest, []byte("-----BEGIN ")) { + return false + } + block, after := pem.Decode(rest) + if block == nil || block.Type != "CERTIFICATE" || len(block.Headers) > 0 { + return false + } + if _, err := x509.ParseCertificate(block.Bytes); err != nil { + return false + } + found++ + rest = after } - return strings.Trim(data, "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/") == "" + return found > 0 } // lineBreakIn is where the first string under v holding \n, \r or NUL is, or "". func lineBreakIn(v any, at string) string { - if strings.ContainsAny(at, "\n\r\x00") { - return strings.NewReplacer("\n", `\n`, "\r", `\r`, "\x00", `\0`).Replace(at) + if strings.ContainsAny(at, lineEnds) { + return strconv.Quote(at) } switch t := v.(type) { case string: - if strings.ContainsAny(t, "\n\r\x00") && !pemBlocks(t) { + if strings.ContainsAny(t, lineEnds) { return at } case map[string]any: diff --git a/internal/catalogue/terminal_settings_test.go b/internal/catalogue/terminal_settings_test.go index 27a2d4ea..bc3831fe 100644 --- a/internal/catalogue/terminal_settings_test.go +++ b/internal/catalogue/terminal_settings_test.go @@ -55,29 +55,75 @@ func TestASettingHoldsOneLine(t *testing.T) { } } -// PEM blocks alone may hold lines: a provider serves its authority's root as a setting. Anything around them, or -// a line in them that is not base64, is refused. -func TestAPEMBlockIsTheOneSettingWithLines(t *testing.T) { - m := Manifest{Module: "route-proxy"} - full := strings.Repeat("MIIB", 16) - pem := "-----BEGIN CERTIFICATE-----\n" + full + "\n" + full + "\nAbCd+/==\n-----END CERTIFICATE-----\n" - for _, ok := range []string{pem, pem + pem, strings.TrimSuffix(pem, "\n"), - "-----BEGIN CERTIFICATE-----\nMIIBeDCCAR2gAwIBAgIQfake000000000000000000000000\n-----END CERTIFICATE-----\n"} { - if err := JudgeSettings(m, []Layer{{From: "anchor", Values: map[string]any{"root": ok}}}, false); err != nil { - t.Errorf("a PEM block: %v", err) +// Lines are allowed in one setting only: step-ca's `root`, as certificates that encoding/pem decodes and +// x509.ParseCertificate parses (novox/hq issue 339). Any other module or key, another label, or a block that is +// not a certificate is refused like any other line break. +func TestOnlyTheAuthoritysRootMayHoldLines(t *testing.T) { + ca := Manifest{Module: "step-ca"} + judge := func(m Manifest, key, v string) error { + return JudgeSettings(m, []Layer{{From: "anchor", Values: map[string]any{key: v}}}, false) + } + for _, ok := range []string{servedRoot, servedRoot + servedRoot, strings.TrimSuffix(servedRoot, "\n")} { + if err := judge(ca, "root", ok); err != nil { + t.Errorf("the authority's root: %v", err) } } - for _, bad := range []string{ - pem + "PATH=/tmp\n", - "PATH=\n" + pem, - "-----BEGIN CERTIFICATE-----\n" + full + "\nPATH=\n-----END CERTIFICATE-----\n", - "-----BEGIN CERTIFICATE-----\nshort\n" + full + "\n-----END CERTIFICATE-----\n", - "-----BEGIN CERTIFICATE-----\n" + full + "\n-----END KEY-----\n", - "-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----\n", - "-----BEGIN CERTIFICATE-----\r\n" + full + "\r\n-----END CERTIFICATE-----\r\n", + body := strings.TrimSuffix(strings.TrimPrefix(servedRoot, "-----BEGIN CERTIFICATE-----\n"), "-----END CERTIFICATE-----\n") + for name, bad := range map[string]string{ + "a line after it": servedRoot + "PATH=/tmp\n", + "a line before it": "PATH=\n" + servedRoot, + "another label": "-----BEGIN PRIVATE KEY-----\n" + body + "-----END PRIVATE KEY-----\n", + "headers": "-----BEGIN CERTIFICATE-----\nProc-Type: 4,ENCRYPTED\n\n" + body + "-----END CERTIFICATE-----\n", + "base64 that is no cert": "-----BEGIN CERTIFICATE-----\nMIIBeDCCAR2gAwIBAgIQfake000000000000000000000000\n-----END CERTIFICATE-----\n", + "carriage returns": strings.ReplaceAll(servedRoot, "\n", "\r\n"), } { - if err := JudgeSettings(m, []Layer{{From: "anchor", Values: map[string]any{"root": bad}}}, false); err == nil { - t.Errorf("taken: %q", bad) + if err := judge(ca, "root", bad); err == nil { + t.Errorf("%s was taken", name) + } + } + if err := judge(ca, "other", servedRoot); err == nil { + t.Error("a certificate in another key of the authority was taken") + } + if err := judge(Manifest{Module: "mailu"}, "root", servedRoot); err == nil { + t.Error("a certificate in another module's setting was taken") + } + if err := JudgeSettings(ca, []Layer{{From: "anchor", Values: map[string]any{"root": []any{servedRoot}}}}, false); err == nil { + t.Error("a certificate below the top of the setting was taken") + } +} + +// Every character a reader takes as the end of a line is refused, not only \n and \r: vertical tab, form feed, +// NEL and the Unicode line and paragraph separators (novox/hq issue 339). +func TestEveryLineEndIsRefused(t *testing.T) { + m := Manifest{Module: "mailu"} + for _, v := range []string{"a\vb", "a\fb", "a\u0085b", "a\u2028b", "a\u2029b"} { + if err := JudgeSettings(m, []Layer{{From: "home", Values: map[string]any{"v": v}}}, false); err == nil || + !strings.Contains(err.Error(), "line break") { + t.Errorf("%q: %v", v, err) + } + } +} + +// The review's additions: the spool, the container runtimes' data, /opt, and any home's .ssh. +func TestTheRuntimesDataAndAnyHomesSSHAreTheMachinesOwn(t *testing.T) { + m := Manifest{Module: "notes", Resources: []map[string]any{{"id": "data", "type": "directory"}}, + Accesses: []Access{{ID: "media"}}} + for _, path := range []string{"/var/spool", "/var/spool/cron", "/var/lib/docker", "/var/lib/docker/volumes", + "/var/lib/containers/storage", "/opt", "/opt/app", "/home/alice/.ssh", "/home/alice/.ssh/keys", + "/srv/backup/.ssh", "/root/.ssh"} { + layers := []Layer{{From: "laptop", Values: map[string]any{PlacesSetting: map[string]any{"data": path}}}} + if _, err := Places(m, layers); err == nil || !strings.Contains(err.Error(), "issue 339") { + t.Errorf("a place at %s: %v", path, err) + } + layers = []Layer{{From: "laptop", Values: map[string]any{AccessesSetting: map[string]any{"media": path}}}} + if _, err := AccessPlaces(m, layers); err == nil || !strings.Contains(err.Error(), "issue 339") { + t.Errorf("an access at %s: %v", path, err) + } + } + for _, path := range []string{"/var/lib/dockerish", "/home/alice/ssh", "/home/alice/.sshd-notes", "/storage/media"} { + layers := []Layer{{From: "laptop", Values: map[string]any{PlacesSetting: map[string]any{"data": path}}}} + if _, err := Places(m, layers); err != nil { + t.Errorf("a place at %s: %v", path, err) } } } From 0f1e1b09fd98d3c89be82514832075b392550388 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 00:38:07 +0200 Subject: [PATCH 2/7] Change a test store's failure under its lock, which the keeper's goroutine reads TestAnUnreadableStoreClearsNothing wrote InMemory.Fail and its values unguarded while the keeper's teller appended to the same store, and the race detector failed mesh/repo-check on #170 (a test race on main, not the change). --- cmd/mesh-controller/conditions_test.go | 4 ++-- internal/conditions/memory.go | 8 ++++++++ internal/conditions/store_test.go | 6 ++++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/cmd/mesh-controller/conditions_test.go b/cmd/mesh-controller/conditions_test.go index 6e33426d..35bdefe2 100644 --- a/cmd/mesh-controller/conditions_test.go +++ b/cmd/mesh-controller/conditions_test.go @@ -86,7 +86,7 @@ func TestASilenceThroughTheVerbHoldsAndTheConditionStaysOpen(t *testing.T) { func TestUnreadableConditionsAreNotAWellMesh(t *testing.T) { open := aMesh(t) _, store := withConditionsInMemory(t) - store.Fail = errors.New("the bus is away") + store.SetFail(errors.New("the bus is away")) asked, err := theThreeQuestions(t.Context(), open) if err != nil { t.Fatal(err) @@ -98,7 +98,7 @@ func TestUnreadableConditionsAreNotAWellMesh(t *testing.T) { if !strings.HasPrefix(said, "the open conditions could NOT be read") || strings.Contains(said, "no open conditions") { t.Fatalf("%s", said) } - store.Fail = nil + store.SetFail(nil) asked, _ = theThreeQuestions(t.Context(), open) said = printed(t, func() error { return printStatus(asked) }) if asked.well() && !strings.Contains(said, "no open conditions;") { diff --git a/internal/conditions/memory.go b/internal/conditions/memory.go index 1e23ee25..61134461 100644 --- a/internal/conditions/memory.go +++ b/internal/conditions/memory.go @@ -61,6 +61,14 @@ func (m *InMemory) Update(_ context.Context, key string, value []byte, revision return nil } +// SetFail makes every call fail with err from now on, or none with nil: under the store's lock, so a test may +// change it while the keeper's own goroutine is telling. +func (m *InMemory) SetFail(err error) { + m.mu.Lock() + defer m.mu.Unlock() + m.Fail = err +} + func (m *InMemory) Delete(_ context.Context, key string, revision uint64) error { m.mu.Lock() defer m.mu.Unlock() diff --git a/internal/conditions/store_test.go b/internal/conditions/store_test.go index 2d571abe..47d1e494 100644 --- a/internal/conditions/store_test.go +++ b/internal/conditions/store_test.go @@ -199,15 +199,17 @@ func TestAnUnreadableStoreClearsNothing(t *testing.T) { if _, err := k.Observe(ctx, silent("ace")); err != nil { t.Fatal(err) } - store.Fail = errors.New("the bus is away") + store.SetFail(errors.New("the bus is away")) if err := k.Reconcile(ctx, "S1", nil); err == nil { t.Fatal("reconciled against a store it could not read") } if _, err := k.Open(ctx); err == nil { t.Fatal("an unreadable store answered as read") } - store.Fail = nil + store.SetFail(nil) + store.mu.Lock() store.values["machine.g14.silent"] = Entry{Value: []byte("{not a condition"), Revision: 99} + store.mu.Unlock() if _, err := k.Open(ctx); err == nil || !strings.Contains(err.Error(), "machine.g14.silent") { t.Fatalf("an unreadable condition was left out rather than said: %v", err) } From 0e0ba93f6c952ab74e03be0046ee053fba50329b Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 00:51:18 +0200 Subject: [PATCH 3/7] Take back the test store's lock fix: open #148 carries the fuller one Merged beside #148 the two would not compile (SetFail declared twice, m.Fail undefined). #148 lands on its own. --- cmd/mesh-controller/conditions_test.go | 4 ++-- internal/conditions/memory.go | 8 -------- internal/conditions/store_test.go | 6 ++---- 3 files changed, 4 insertions(+), 14 deletions(-) diff --git a/cmd/mesh-controller/conditions_test.go b/cmd/mesh-controller/conditions_test.go index 35bdefe2..6e33426d 100644 --- a/cmd/mesh-controller/conditions_test.go +++ b/cmd/mesh-controller/conditions_test.go @@ -86,7 +86,7 @@ func TestASilenceThroughTheVerbHoldsAndTheConditionStaysOpen(t *testing.T) { func TestUnreadableConditionsAreNotAWellMesh(t *testing.T) { open := aMesh(t) _, store := withConditionsInMemory(t) - store.SetFail(errors.New("the bus is away")) + store.Fail = errors.New("the bus is away") asked, err := theThreeQuestions(t.Context(), open) if err != nil { t.Fatal(err) @@ -98,7 +98,7 @@ func TestUnreadableConditionsAreNotAWellMesh(t *testing.T) { if !strings.HasPrefix(said, "the open conditions could NOT be read") || strings.Contains(said, "no open conditions") { t.Fatalf("%s", said) } - store.SetFail(nil) + store.Fail = nil asked, _ = theThreeQuestions(t.Context(), open) said = printed(t, func() error { return printStatus(asked) }) if asked.well() && !strings.Contains(said, "no open conditions;") { diff --git a/internal/conditions/memory.go b/internal/conditions/memory.go index 61134461..1e23ee25 100644 --- a/internal/conditions/memory.go +++ b/internal/conditions/memory.go @@ -61,14 +61,6 @@ func (m *InMemory) Update(_ context.Context, key string, value []byte, revision return nil } -// SetFail makes every call fail with err from now on, or none with nil: under the store's lock, so a test may -// change it while the keeper's own goroutine is telling. -func (m *InMemory) SetFail(err error) { - m.mu.Lock() - defer m.mu.Unlock() - m.Fail = err -} - func (m *InMemory) Delete(_ context.Context, key string, revision uint64) error { m.mu.Lock() defer m.mu.Unlock() diff --git a/internal/conditions/store_test.go b/internal/conditions/store_test.go index 47d1e494..2d571abe 100644 --- a/internal/conditions/store_test.go +++ b/internal/conditions/store_test.go @@ -199,17 +199,15 @@ func TestAnUnreadableStoreClearsNothing(t *testing.T) { if _, err := k.Observe(ctx, silent("ace")); err != nil { t.Fatal(err) } - store.SetFail(errors.New("the bus is away")) + store.Fail = errors.New("the bus is away") if err := k.Reconcile(ctx, "S1", nil); err == nil { t.Fatal("reconciled against a store it could not read") } if _, err := k.Open(ctx); err == nil { t.Fatal("an unreadable store answered as read") } - store.SetFail(nil) - store.mu.Lock() + store.Fail = nil store.values["machine.g14.silent"] = Entry{Value: []byte("{not a condition"), Revision: 99} - store.mu.Unlock() if _, err := k.Open(ctx); err == nil || !strings.Contains(err.Error(), "machine.g14.silent") { t.Fatalf("an unreadable condition was left out rather than said: %v", err) } From ec7b8bcd5871709323f7b7e13267cc375bf4ad55 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 00:57:10 +0200 Subject: [PATCH 4/7] Keep the mesh's trust anchors at the terminal, refuse containerd's tree, and read a found directory as a wait The review of #170: step-ca's root, roots and path and the identity provider's issuer are what every consumer trusts, and any caller of the settings verb could replace them; they are now terminal keys like places and accesses (hq issue 339). /var/lib/containerd joins the runtimes' data. And a directory the node-engine uses as found failed its module's gate and rolled its builds back; found before the send, it is now a wait for a person the gate passes with, as a relogin is (ADR 0254), and only one the send itself found holds the module. --- cmd/mesh-controller/found_wait_test.go | 104 ++++++++++++++++++ cmd/mesh-controller/module_health.go | 26 ++++- cmd/mesh-controller/modules.go | 20 ++-- cmd/mesh-controller/terminal_settings_test.go | 44 ++++++++ internal/catalogue/placement.go | 23 +++- internal/catalogue/terminal_settings_test.go | 17 ++- internal/link/protocol.go | 8 ++ 7 files changed, 228 insertions(+), 14 deletions(-) create mode 100644 cmd/mesh-controller/found_wait_test.go diff --git a/cmd/mesh-controller/found_wait_test.go b/cmd/mesh-controller/found_wait_test.go new file mode 100644 index 00000000..10d7b5dd --- /dev/null +++ b/cmd/mesh-controller/found_wait_test.go @@ -0,0 +1,104 @@ +package main + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// A directory the node-engine uses as found (novox/hq issue 339) waits for a person to hand it over at the +// machine. Found before this send, it is no fault of the build: the gate passes with the wait carried, so an +// urgent fix of that module still goes through. Found by this send, the send brought it, and the gate holds. +func foundDirectory(module string, since time.Time) inventory.ResourceHealth { + return inventory.ResourceHealth{Module: module, Resource: module + ".data", Kind: link.KindDirectory, + Target: "/srv/" + module, State: link.StateUnhealthy, Since: since, + Reason: link.ReasonUsedAsFound + " owned by 1000:1000, mode 700, as found; root, mode 755 was declared and " + + "not given it — `mesh-host hand-over` at the machine hands it to the mesh"} +} + +func TestADirectoryFoundBeforeTheSendIsAWaitForAPerson(t *testing.T) { + now := time.Now() + sent := now.Add(-time.Minute) + f := gateFacts{now: now, health: map[string]inventory.NodeHealth{"laptop": {Node: "laptop", HeardAt: now, + Resources: []inventory.ResourceHealth{foundDirectory("notes", sent.Add(-24*time.Hour))}}}} + h, why := moduleHealthWord("notes", "laptop", sent, f) + if h != healthPerson || !strings.Contains(why, "notes.data") || !strings.Contains(why, "hand-over") { + t.Fatalf("a directory found before the send reads %v %q; want a wait for a person", h, why) + } + // Found by this very send: the send brought it, and it is not passed. + f.health["laptop"] = inventory.NodeHealth{Node: "laptop", HeardAt: now, + Resources: []inventory.ResourceHealth{foundDirectory("notes", sent.Add(time.Second))}} + if h, why := moduleHealthWord("notes", "laptop", sent, f); h != healthNotYet { + t.Fatalf("a directory this send found reads %v %q; want not yet", h, why) + } + // A container down beside the old wait is a fault, as before. + f.health["laptop"] = inventory.NodeHealth{Node: "laptop", HeardAt: now, Resources: []inventory.ResourceHealth{ + foundDirectory("notes", sent.Add(-time.Hour)), + {Module: "notes", Resource: "notes.web", Kind: "container", Target: "notes", State: link.StateUnhealthy, Reason: "down"}}} + if h, why := moduleHealthWord("notes", "laptop", sent, f); h != healthNotYet { + t.Fatalf("a container down beside the wait reads %v %q; want not yet", h, why) + } +} + +// The whole walk: a module whose directory was used as found long before still gets its fix to every machine, +// its pass kept and the wait said; a directory this very send found holds it and puts it back. +func TestAFixGoesThroughPastADirectoryFoundBefore(t *testing.T) { + for _, c := range []struct { + name string + found time.Duration // when the directory was found, against now + passes bool + }{ + {"found a day before the send", -24 * time.Hour, true}, + {"found by this send", time.Hour, false}, + } { + t.Run(c.name, func(t *testing.T) { + b := aBacklog(t) + ctx := t.Context() + inv := b.open.inventory + releaseHeard = func(context.Context, *stores) (map[string]bool, error) { + return map[string]bool{"anchor": true, "laptop": true}, nil + } + backlogFacts := gatherGateFacts + gatherGateFacts = func(ctx context.Context, open *stores, component string) (gateFacts, error) { + f, err := backlogFacts(ctx, open, component) + f.health = map[string]inventory.NodeHealth{} + for _, n := range []string{"anchor", "laptop"} { + f.health[n] = inventory.NodeHealth{Node: n, HeardAt: time.Now(), Resources: []inventory.ResourceHealth{ + {Module: "app", Resource: "app.web", Kind: "container", Target: "app", State: link.StateHealthy}, + foundDirectory("app", time.Now().Add(c.found)), + {Module: "late", Resource: "late.web", Kind: "container", Target: "late", State: link.StateHealthy}}} + } + return f, err + } + wasSettle, wasEvery, wasBound := gateSettle, gateEvery, gateBound + t.Cleanup(func() { gateSettle, gateEvery, gateBound = wasSettle, wasEvery, wasBound }) + gateSettle, gateEvery, gateBound = 0, 0, 300*time.Millisecond + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + advancePlans(ctx, b.open) + if p := b.release(t); p.State != inventory.PlanRolling { + break + } + time.Sleep(20 * time.Millisecond) + } + p := b.release(t) + v, found, err := inv.GateOf(ctx, "build-app-c2") + if c.passes { + if p.State != inventory.PlanDone || err != nil || !found || v.Verdict != inventory.GatePassed { + t.Fatalf("the walk is %s (%s); app's verdict %+v: want the fix through", p.State, p.Note, v) + } + if !strings.Contains(v.Why+p.Note, "hand it over") { + t.Errorf("the wait is not carried: verdict %q, walk %q", v.Why, p.Note) + } + return + } + if p.State == inventory.PlanDone { + t.Fatalf("a directory this send found let the walk through: %s", p.Note) + } + }) + } +} diff --git a/cmd/mesh-controller/module_health.go b/cmd/mesh-controller/module_health.go index f34f999d..25717cab 100644 --- a/cmd/mesh-controller/module_health.go +++ b/cmd/mesh-controller/module_health.go @@ -499,6 +499,7 @@ func moduleHealthWord(module, machine string, since time.Time, f gateFacts) (hea if waits && !f.groupsAdded[module] { waits = false } + var found []string for _, r := range h.Resources { if r.Module != module { continue @@ -506,6 +507,14 @@ func moduleHealthWord(module, machine string, since time.Time, f gateFacts) (hea if waits && r.State == link.StateUnhealthy { continue } + // **A directory used as found before this send waits for a person** (novox/hq issue 339): the node-engine + // left its owner and mode, and only someone at the machine can hand it over. It is no fault of this + // build, so it does not hold the module's walk — an urgent fix still goes through — and the verdict + // carries the wait. Found by this very send, the send brought it, and it is judged as unhealthy. + if usedAsFound(r) && r.Since.Before(since) { + found = append(found, r.Resource) + continue + } switch r.State { case link.StateHealthy: case link.StateStarting: @@ -521,12 +530,25 @@ func moduleHealthWord(module, machine string, since time.Time, f gateFacts) (hea reasonAfter(r.Reason)) } } - if waits { - return healthPerson, wait + if waits || len(found) > 0 { + var said []string + if waits { + said = append(said, wait) + } + if len(found) > 0 { + said = append(said, fmt.Sprintf("on %s, %s uses %s as found and waits for a person to hand it over "+ + "(`mesh-host hand-over ` at the machine)", machine, module, strings.Join(found, ", "))) + } + return healthPerson, strings.Join(said, "; ") } return healthGood, "" } +// usedAsFound is a directory the node-engine states it uses as found (novox/hq issue 339). +func usedAsFound(r inventory.ResourceHealth) bool { + return r.Kind == link.KindDirectory && r.State == link.StateUnhealthy && strings.HasPrefix(r.Reason, link.ReasonUsedAsFound) +} + func reasonAfter(s string) string { if s == "" { return "" diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index 7bc6fc3b..352d6227 100644 --- a/cmd/mesh-controller/modules.go +++ b/cmd/mesh-controller/modules.go @@ -1062,12 +1062,12 @@ func declaresTools(m catalogue.Manifest) bool { return false } -// terminalSettings are the keys no verb may change (novox/hq issue 339). `places` says where the node-engine -// creates and, as root, owns a module's directories, with an owner the setting names; `accesses` says which of -// the machine's paths are mounted into a module's container. Set through a verb, either lets any caller of the -// mesh's console — an agent among them — have root hand it a directory, or mount one of the machine's into a -// container it reaches. They are the operator's, typed at the controller's terminal. -var terminalSettings = []string{catalogue.PlacesSetting, catalogue.AccessesSetting} +// The keys no verb may change are catalogue.TerminalKeys (novox/hq issue 339). `places` says where the +// node-engine creates and, as root, owns a module's directories, with an owner the setting names; `accesses` says +// which of the machine's paths are mounted into a module's container; a provider's trust anchors say what every +// consumer trusts. Set through a verb, any of them lets any caller of the mesh's verbs — an agent among them — +// have root hand it a directory, mount one of the machine's into a container it reaches, or have the mesh trust +// an authority of its own. They are the operator's, typed at the controller's terminal. // throughAVerb says whether this process runs a seat verb's command line: the serving controller names the // verb in the environment of every command it runs for one (runVerb), and a person at the terminal runs none. @@ -1085,16 +1085,16 @@ func refuseTerminalSettingsThroughAVerb(before, after map[string]any, module, wh if !through { return nil } - for _, key := range terminalSettings { + for _, key := range catalogue.TerminalKeys(module) { was, _ := json.Marshal(before[key]) now, _ := json.Marshal(after[key]) if string(was) == string(now) { continue } return fmt.Errorf("%s of %s on %s is set at the controller's terminal only, never through a verb (this "+ - "line came through %q): it says where root creates and owns a module's directories, or which of "+ - "the machine's paths are mounted into its container, and whoever may call a verb includes agents "+ - "(novox/hq issue 339). Nothing was changed", key, module, where, verb) + "line came through %q): it says where root creates and owns a module's directories, which of "+ + "the machine's paths are mounted into its container, or what the mesh's consumers trust, and whoever "+ + "may call a verb includes agents (novox/hq issue 339). Nothing was changed", key, module, where, verb) } return nil } diff --git a/cmd/mesh-controller/terminal_settings_test.go b/cmd/mesh-controller/terminal_settings_test.go index 48741817..79e6461f 100644 --- a/cmd/mesh-controller/terminal_settings_test.go +++ b/cmd/mesh-controller/terminal_settings_test.go @@ -150,3 +150,47 @@ func TestPlacesAndAccessesAreRefusedThroughEveryVerb(t *testing.T) { t.Fatalf("a refused call changed the layer: %s, was %s", got, kept) } } + +// The mesh's trust anchors are set at the terminal alone (novox/hq issue 339): through the settings verb, a caller +// could replace the internal authority's root every consumer trusts, or the issuer every login is checked against. +func TestATrustAnchorIsRefusedThroughAVerb(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + register(t, open, catalogue.Manifest{Module: "step-ca", Version: "1", + Provides: catalogue.FromAnywhere("acme-ca"), + Serves: map[string]map[string]any{"acme-ca": {"root": "", "path": "/acme/acme/directory"}}, + Resources: []map[string]any{{"id": "rc", "type": "file", "path": "/etc/step.conf", "mode": "0644", + "content": "x = ${setting:x}\n"}}}) + register(t, open, catalogue.Manifest{Module: "keycloak", Version: "1", + Provides: catalogue.FromAnywhere("oidc-client"), + Serves: map[string]map[string]any{"oidc-client": {"issuer": "${setting:issuer}"}}, + Resources: []map[string]any{{"id": "rc", "type": "file", "path": "/etc/kc.conf", "mode": "0644", + "content": "issuer = ${setting:issuer}\nx = ${setting:x}\n"}}}) + for _, m := range []string{"step-ca", "keycloak"} { + if _, err := assign(ctx, open, "anchor", m); err != nil { + t.Fatal(err) + } + } + refused := func(what string, err error) { + t.Helper() + if err == nil || !strings.Contains(err.Error(), "controller's terminal") { + t.Fatalf("%s: %v", what, err) + } + } + if err := atTheTerminal(t, "settings", "set", "keycloak", `{"issuer":"https://id.example/realms/mesh","x":0}`, + "--node", "anchor"); err != nil { + t.Fatalf("the issuer at the terminal: %v", err) + } + refused("the issuer through the verb", throughVerb(t, "settings", map[string]any{"module": "keycloak", + "node": "anchor", "values": `{"issuer":"https://evil.example/realms/mesh","x":0}`})) + refused("the authority's root path through the verb", throughVerb(t, "settings", map[string]any{"module": "step-ca", + "node": "anchor", "values": `{"path":"/evil","x":0}`})) + refused("the authority's root path, mesh-wide, through the verb", throughVerb(t, "settings", + map[string]any{"module": "step-ca", "values": `{"path":"/evil"}`})) + refused("clearing the issuer through the verb", throughVerb(t, "settings", map[string]any{"module": "keycloak", + "node": "anchor", "clear": "true"})) + if err := throughVerb(t, "settings", map[string]any{"module": "keycloak", "node": "anchor", + "values": `{"issuer":"https://id.example/realms/mesh","x":1}`}); err != nil { + t.Fatalf("another key through the verb, the issuer kept: %v", err) + } +} diff --git a/internal/catalogue/placement.go b/internal/catalogue/placement.go index 808151ea..24781dd0 100644 --- a/internal/catalogue/placement.go +++ b/internal/catalogue/placement.go @@ -62,7 +62,7 @@ var ownerShape = regexp.MustCompile(`^[0-9]+:[0-9]+$`) var ( systemTrees = []string{"/etc", "/usr", "/boot", "/root", "/run", "/var/run", "/var/lock", "/proc", "/sys", "/dev", "/bin", "/sbin", "/lib", "/lib32", "/lib64", "/var/lib/mesh", "/var/lib/mesh-host", "/var/spool", - "/var/lib/docker", "/var/lib/containers", "/opt"} + "/var/lib/docker", "/var/lib/containers", "/var/lib/containerd", "/opt"} systemRoots = []string{"/", "/var", "/var/lib", "/var/cache", "/var/log", "/var/tmp", "/home", "/mnt", "/media", "/srv", "/tmp", "/storage", "/data", "/services"} ) @@ -438,3 +438,24 @@ func namesOfAccessIDs(accesses map[string]string) []string { sort.Strings(names) return names } + +// trustAnchors are the settings a provider serves its consumers as what they trust, by module (novox/hq issue +// 339): set through a verb, any caller could point every consumer at an authority or an issuer of its own. +// +// - step-ca, the mesh's internal ACME authority: `root`, the root a consumer is handed to trust (the one +// setting that may hold lines, settingsHoldOneLine); `roots` and `path`, where a consumer fetches the roots +// and the ACME directory from, which a setting may override as it may any served fact. +// - keycloak, the identity provider: `issuer`, the issuer every OIDC consumer checks a login's token against. +// +// Named here, not in the manifests, because no manifest field says "this is trusted" yet; the catalogue was read +// for every served fact and every ${setting:…} on 2026-10-09, and these are the ones a consumer trusts. +var trustAnchors = map[string][]string{ + rootModule: {rootSetting, "roots", "path"}, + "keycloak": {"issuer"}, +} + +// TerminalKeys are the settings keys of a module that are set at the controller's terminal alone, never through +// a verb (novox/hq issue 339): places and accesses for every module, and a provider's trust anchors. +func TerminalKeys(module string) []string { + return append([]string{PlacesSetting, AccessesSetting}, trustAnchors[module]...) +} diff --git a/internal/catalogue/terminal_settings_test.go b/internal/catalogue/terminal_settings_test.go index bc3831fe..a8a722f2 100644 --- a/internal/catalogue/terminal_settings_test.go +++ b/internal/catalogue/terminal_settings_test.go @@ -109,7 +109,7 @@ func TestTheRuntimesDataAndAnyHomesSSHAreTheMachinesOwn(t *testing.T) { m := Manifest{Module: "notes", Resources: []map[string]any{{"id": "data", "type": "directory"}}, Accesses: []Access{{ID: "media"}}} for _, path := range []string{"/var/spool", "/var/spool/cron", "/var/lib/docker", "/var/lib/docker/volumes", - "/var/lib/containers/storage", "/opt", "/opt/app", "/home/alice/.ssh", "/home/alice/.ssh/keys", + "/var/lib/containers/storage", "/var/lib/containerd", "/var/lib/containerd/io.containerd.snapshotter.v1", "/opt", "/opt/app", "/home/alice/.ssh", "/home/alice/.ssh/keys", "/srv/backup/.ssh", "/root/.ssh"} { layers := []Layer{{From: "laptop", Values: map[string]any{PlacesSetting: map[string]any{"data": path}}}} if _, err := Places(m, layers); err == nil || !strings.Contains(err.Error(), "issue 339") { @@ -127,3 +127,18 @@ func TestTheRuntimesDataAndAnyHomesSSHAreTheMachinesOwn(t *testing.T) { } } } + +// A trust anchor the mesh hands its consumers is the terminal's too (novox/hq issue 339): the authority's root, +// where its roots and directory are, and the identity provider's issuer. +func TestTrustAnchorsAreTerminalKeys(t *testing.T) { + for module, keys := range map[string][]string{ + "step-ca": {"places", "accesses", "root", "roots", "path"}, + "keycloak": {"places", "accesses", "issuer"}, + "mailu": {"places", "accesses"}, + } { + got := TerminalKeys(module) + if strings.Join(got, ",") != strings.Join(keys, ",") { + t.Errorf("%s: %v; want %v", module, got, keys) + } + } +} diff --git a/internal/link/protocol.go b/internal/link/protocol.go index 3b28d86c..ad959149 100644 --- a/internal/link/protocol.go +++ b/internal/link/protocol.go @@ -446,6 +446,14 @@ const KindUnit = "unit" // starting ReasonRelogin when only a new login is missing. const KindAccount = "account" +// KindDirectory is a directory of a module that the node-engine uses as found (novox/hq issue 339): there before +// the mesh, with another owner or mode than declared, and left so until a person hands it over at the machine +// (`mesh-host hand-over`). Stated unhealthy with a reason that starts ReasonUsedAsFound. +const KindDirectory = "directory" + +// ReasonUsedAsFound starts the reason of a directory used as found. +const ReasonUsedAsFound = "used as found:" + // ReasonRelogin starts the reason of an account whose running session began before it was put in a group // (ADR 0252): the build did what it should, and a person has one step left (novox/hq ADR 0254). const ReasonRelogin = "relogin needed" From b9fc09c37539ffda8d0f9af3751c3093ed617c38 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 01:23:44 +0200 Subject: [PATCH 5/7] Derive the terminal's settings from what a module serves and which files it trusts; a found directory is its own condition The third review of #170 (hq issue 339): a setting overrides any key a provider serves, so any caller of the settings verb could move a database's port, a registry's port or an issuer to a listener of its own and collect what consumers present. TerminalKeys now derives from the manifest: places, accesses, every served key and every setting a served value asks for, and every setting a file marked `trusted` asks for. `trusted` is the catalogue's word, taken out before the declaration; `module check` warns of a file that asks for a setting without saying, and refuses it from 2026-10-30. The hand list is gone. A directory used as found is now its own condition kind, the operator's, never urgent, and the gate exempts it where it exempts a relogin. --- cmd/mesh-controller/check.go | 35 ++++++ cmd/mesh-controller/found_wait_test.go | 52 ++++++++ cmd/mesh-controller/gate.go | 10 +- cmd/mesh-controller/module_health.go | 67 ++++++++++- cmd/mesh-controller/modules.go | 18 ++- cmd/mesh-controller/plain_words.go | 8 ++ cmd/mesh-controller/terminal_settings_test.go | 50 +++++--- internal/catalogue/declaration.go | 1 + internal/catalogue/placement.go | 21 ---- internal/catalogue/settings.go | 1 + internal/catalogue/terminal_keys.go | 112 ++++++++++++++++++ internal/catalogue/terminal_settings_test.go | 51 ++++++-- 12 files changed, 368 insertions(+), 58 deletions(-) create mode 100644 internal/catalogue/terminal_keys.go diff --git a/cmd/mesh-controller/check.go b/cmd/mesh-controller/check.go index 31c0eec8..d53d3a82 100644 --- a/cmd/mesh-controller/check.go +++ b/cmd/mesh-controller/check.go @@ -65,6 +65,13 @@ func moduleCheckFor(paths []string, longestMachine int, out io.Writer) error { } // A definition names no installation (novox/hq ADR 0112, ADR 0155): judged here, in the // catalogue-wide test, and at registration, which refuses in the same words. + if wrong := catalogue.TrustProblems(m); len(wrong) > 0 { + for _, p := range wrong { + fmt.Fprintf(out, "%s: %s\n", path, p) + } + failed += len(wrong) + faulted[m.Module] = true + } if named := catalogue.InstallationProblems(m); len(named) > 0 { for _, p := range named { fmt.Fprintf(out, "%s: %s\n", path, p) @@ -130,6 +137,25 @@ func moduleCheckFor(paths []string, longestMachine int, out io.Writer) error { } } + // **Every file that asks for a setting says whether it is trusted** (novox/hq issue 339): warned until the + // date, refused from it, and counted for the catalogue's merge check like the resources without health. + unsaid := 0 + trustRequired := !checkNow().Before(catalogue.TrustRequiredFrom) + for _, name := range names { + missing := catalogue.UnsaidTrust(shelf[name]) + unsaid += len(missing) + if trustRequired { + for _, id := range missing { + fmt.Fprintf(out, "%s: the file %s asks for a setting and does not say whether it is trusted: say "+ + "%q true or false (novox/hq issue 339)\n", name, id, catalogue.TrustedField) + } + if len(missing) > 0 { + failed += len(missing) + faulted[name] = true + } + } + } + for _, name := range names { m := shelf[name] if faulted[name] { @@ -171,6 +197,10 @@ func moduleCheckFor(paths []string, longestMachine int, out io.Writer) error { if len(checks) > 0 { fmt.Fprintf(out, ", ready: %s", strings.Join(checks, "; ")) } + if missing := catalogue.UnsaidTrust(m); len(missing) > 0 { + fmt.Fprintf(out, "; WARNING: %s ask(s) for a setting and say(s) not whether it is trusted, refused from "+ + "%s (novox/hq issue 339)", strings.Join(missing, ", "), catalogue.TrustRequiredFrom.Format("2006-01-02")) + } if missing := catalogue.Undeclared(m); len(missing) > 0 { fmt.Fprintf(out, "; WARNING: %s stay(s) up and say(s) not how it is ready — judged by liveness alone, "+ "refused from %s (ADR 0240 rule 8)", strings.Join(missing, ", "), catalogue.HealthRequiredFrom.Format("2006-01-02")) @@ -179,6 +209,7 @@ func moduleCheckFor(paths []string, longestMachine int, out io.Writer) error { } // The count the catalogue keeps (ADR 0240 rule 8), in a line its merge check reads. fmt.Fprintf(out, "%s %d\n", UndeclaredHealthLine, undeclared) + fmt.Fprintf(out, "%s %d\n", UnsaidTrustLine, unsaid) if failed > 0 { return fmt.Errorf("%d problem(s) in %d manifest(s)", failed, len(paths)) } @@ -193,6 +224,10 @@ func moduleCheckFor(paths []string, longestMachine int, out io.Writer) error { // `health` in, over the manifests given: the catalogue's merge check compares it with the number it keeps. const UndeclaredHealthLine = "long-running resources without health:" +// UnsaidTrustLine starts the line `module check` says the count of files that ask for a setting and do not say +// whether it is trusted (novox/hq issue 339). +const UnsaidTrustLine = "files asking for a setting without saying whether it is trusted:" + // checkNow is the clock `module check` judges the date by; a test sets it. var checkNow = time.Now diff --git a/cmd/mesh-controller/found_wait_test.go b/cmd/mesh-controller/found_wait_test.go index 10d7b5dd..e383194b 100644 --- a/cmd/mesh-controller/found_wait_test.go +++ b/cmd/mesh-controller/found_wait_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/novox/mesh-controller/internal/conditions" "github.com/novox/mesh-controller/internal/inventory" "github.com/novox/mesh-controller/internal/link" ) @@ -65,6 +66,12 @@ func TestAFixGoesThroughPastADirectoryFoundBefore(t *testing.T) { backlogFacts := gatherGateFacts gatherGateFacts = func(ctx context.Context, open *stores, component string) (gateFacts, error) { f, err := backlogFacts(ctx, open, component) + // The used-as-found condition, raised after the send (its second statement): its own kind, never a + // fault the gate reads as the build's. + f.judged, f.openErr = true, nil + f.open = append(f.open, conditions.Condition{Key: usedAsFoundKey("app", "anchor"), Kind: kindUsedAsFound, + Subject: conditions.Subject{Scope: conditions.ScopeModule, ID: "app.anchor", Machine: "anchor"}, + Raised: time.Now()}) f.health = map[string]inventory.NodeHealth{} for _, n := range []string{"anchor", "laptop"} { f.health[n] = inventory.NodeHealth{Node: n, HeardAt: time.Now(), Resources: []inventory.ResourceHealth{ @@ -102,3 +109,48 @@ func TestAFixGoesThroughPastADirectoryFoundBefore(t *testing.T) { }) } } + +// The condition says the wait in its own kind: the operator's, never urgent, and cleared once handed over. +func TestADirectoryUsedAsFoundIsItsOwnCondition(t *testing.T) { + k, _ := withConditionsInMemory(t) + ctx := t.Context() + rs := map[string][]inventory.ResourceHealth{"notes": {foundDirectory("notes", time.Now().Add(-time.Hour))}} + for i := 0; i < 2; i++ { + if err := judgeModuleHealth(ctx, nil, k, "laptop", rs, map[string]int{"notes": i + 1}, time.Now()); err != nil { + t.Fatal(err) + } + } + open, _ := k.Open(ctx) + var got *conditions.Condition + for i, c := range open { + if c.Key == usedAsFoundKey("notes", "laptop") { + got = &open[i] + } + if c.Kind == kindModuleUnhealthy { + t.Fatalf("raised as a fault: %+v", c) + } + } + if got == nil || got.Resolver != conditions.ResolverOperator || got.Severity == conditions.Urgent || + !strings.Contains(got.Summary, "notes.data") { + t.Fatalf("the condition: %+v", got) + } + // Long open is still not urgent: only a person can hand it over, and nothing is broken by the wait. + if err := judgeModuleHealth(ctx, nil, k, "laptop", rs, map[string]int{"notes": 3}, time.Now().Add(48*time.Hour)); err != nil { + t.Fatal(err) + } + open, _ = k.Open(ctx) + for _, c := range open { + if c.Key == usedAsFoundKey("notes", "laptop") && c.Severity == conditions.Urgent { + t.Fatal("a directory used as found became urgent") + } + } + if err := judgeModuleHealth(ctx, nil, k, "laptop", map[string][]inventory.ResourceHealth{}, nil, time.Now()); err != nil { + t.Fatal(err) + } + open, _ = k.Open(ctx) + for _, c := range open { + if c.Key == usedAsFoundKey("notes", "laptop") { + t.Fatal("not cleared once handed over") + } + } +} diff --git a/cmd/mesh-controller/gate.go b/cmd/mesh-controller/gate.go index d9d71bcc..f47929cf 100644 --- a/cmd/mesh-controller/gate.go +++ b/cmd/mesh-controller/gate.go @@ -216,9 +216,10 @@ func judgeHealth(module, component string, m catalogue.Manifest, machine string, firstLine(f.openErr.Error()) } for _, c := range f.open { - // A wait for a person's new login is the module's reading, not a fault raised since the send: the - // gate reads it from the statement below (ADR 0254). - if c.Source == gateProbe || c.Raised.Before(since) || c.Kind == kindReloginNeeded { + // A wait for a person's new login, or for a directory used as found to be handed over, is the module's + // reading, not a fault raised since the send: the gate reads it from the statement below (ADR 0254, + // novox/hq issue 339). + if c.Source == gateProbe || c.Raised.Before(since) || c.Kind == kindReloginNeeded || c.Kind == kindUsedAsFound { continue } onIt := c.Subject.Machine == machine || slices.Contains(c.Subject.Also, machine) || @@ -329,7 +330,8 @@ func aboutTheMachine(machine string, moved []string, since time.Time, f gateFact for _, c := range f.open { aboutIt := c.Subject.Scope == conditions.ScopeMachine && (c.Subject.ID == machine || c.Subject.Machine == machine || slices.Contains(c.Subject.Also, machine)) - if !aboutIt || c.Source == gateProbe || c.Raised.Before(since) { + // A directory used as found waits for a person, whatever the send did (novox/hq issue 339). + if !aboutIt || c.Source == gateProbe || c.Raised.Before(since) || c.Kind == kindUsedAsFound { kept = append(kept, c) continue } diff --git a/cmd/mesh-controller/module_health.go b/cmd/mesh-controller/module_health.go index 25717cab..5a6759ce 100644 --- a/cmd/mesh-controller/module_health.go +++ b/cmd/mesh-controller/module_health.go @@ -135,7 +135,8 @@ func judgeModuleHealth(ctx context.Context, inv *inventory.Inventory, k *conditi } standing := map[string]conditions.Condition{} for _, c := range open { - if (c.Kind == kindModuleUnhealthy || c.Kind == kindReloginNeeded) && c.Subject.Machine == node { + if (c.Kind == kindModuleUnhealthy || c.Kind == kindReloginNeeded || c.Kind == kindUsedAsFound) && + c.Subject.Machine == node { standing[c.Key] = c } } @@ -158,6 +159,21 @@ func judgeModuleHealth(ctx context.Context, inv *inventory.Inventory, k *conditi heldOn := map[string]string{} providers := map[catalogue.Chosen]bool{} for _, m := range modules { + // **A directory used as found is said as that** (novox/hq issue 339): the operator's to hand over at the + // machine, never urgent — nothing is broken by the wait that a person was not told of — and its own kind, + // so the gate never reads it as a fault of the build that happened to be sent beside it. + if said, waits := foundWait(m, node, unhealthy[m]); waits { + o := usedAsFoundObservation(m, node, said, unhealthy[m]) + seen[o.Key()] = true + became[m] = kindUsedAsFound + if _, isOpen := standing[o.Key()]; streaks[m] < moduleUnhealthyAfter && !isOpen { + continue + } + if _, err := k.Observe(ctx, o); err != nil { + problems = append(problems, err.Error()) + } + continue + } // **A wait for a person's new login is said as that** (novox/hq ADR 0254): one plain sentence to the // operator, never urgent, cleared on the first statement that no longer says it. if said, waits := personWait(m, node, unhealthy[m]); waits { @@ -209,6 +225,9 @@ func judgeModuleHealth(ctx context.Context, inv *inventory.Inventory, k *conditi if c.Kind == kindReloginNeeded { why = fmt.Sprintf("%s says %s no longer waits for a new login", node, module) } + if c.Kind == kindUsedAsFound { + why = fmt.Sprintf("%s says no directory of %s is used as found any more", node, module) + } if on, held := heldOn[key]; held { why = fmt.Sprintf("what %s finds on %s waits on %s, which is unhealthy: held under its condition", module, node, on) } @@ -617,3 +636,49 @@ func addsAccountGroups(from catalogue.Manifest, hadFrom bool, to catalogue.Manif } return false } + +// kindUsedAsFound is a module's condition while the node-engine uses one of its directories as found (novox/hq +// issue 339): its own kind, the operator's, never urgent, and never read by the gate as a fault of a build. +const kindUsedAsFound = "directory-used-as-found" + +// usedAsFoundKey is a module's used-as-found condition on a machine. +func usedAsFoundKey(module, node string) string { + return conditions.Key(conditions.ScopeModule, module+"."+node, kindUsedAsFound) +} + +// foundWait is whether everything unhealthy of a module on a machine is a directory used as found, and that in +// one sentence. Anything else unhealthy beside it is judged as a fault, with the directory among its resources. +func foundWait(module, node string, rs []inventory.ResourceHealth) (string, bool) { + var ids, why []string + for _, r := range rs { + if r.Module != module || r.State != link.StateUnhealthy { + continue + } + if !usedAsFound(r) { + return "", false + } + ids = append(ids, r.Resource) + why = append(why, strings.TrimSpace(strings.TrimPrefix(r.Reason, link.ReasonUsedAsFound))) + } + if len(ids) == 0 { + return "", false + } + return fmt.Sprintf("%s on %s uses %s as found: %s", module, node, strings.Join(ids, ", "), + strings.Join(why, "; ")), true +} + +// usedAsFoundObservation is a module whose directory the node-engine uses as found, in words: the operator's, a +// warning however long it stays, its summary naming the directories and their owners; the paths are evidence. +func usedAsFoundObservation(module, node, said string, rs []inventory.ResourceHealth) conditions.Observation { + o := moduleUnhealthyObservation(module, node, rs) + o.Token, o.Kind, o.Resolver, o.Severity, o.Summary = kindUsedAsFound, kindUsedAsFound, conditions.ResolverOperator, + conditions.Warning, said + o.Headline = fmt.Sprintf("%s waits for a directory on %s", module, node) + o.Explanation = fmt.Sprintf("A directory of %s was already on %s, with another owner or mode than %s declares. "+ + "The mesh left it as it was rather than hand it to an account, so %s may not be able to use it.", + module, node, module, module) + o.Needs = fmt.Sprintf("on %s, run mesh-host hand-over with the directory's path as root.", node) + o.Resolved = fmt.Sprintf("%s's directory on %s is the mesh's", module, node) + o.Actions = nil + return o +} diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index 352d6227..442d1b47 100644 --- a/cmd/mesh-controller/modules.go +++ b/cmd/mesh-controller/modules.go @@ -445,7 +445,7 @@ func settingsCommand(ctx context.Context, args []string) error { if err != nil { return err } - if err := refuseTerminalSettingsThroughAVerb(before, values, positionals[0], where); err != nil { + if err := refuseTerminalSettingsThroughAVerb(ctx, inv, before, values, positionals[0], where); err != nil { return err } added, changed, removed := settingsChange(before, values) @@ -603,7 +603,7 @@ func settingsCommand(ctx context.Context, args []string) error { if err != nil { return err } - if err := refuseTerminalSettingsThroughAVerb(before, nil, positionals[0], where); err != nil { + if err := refuseTerminalSettingsThroughAVerb(ctx, inv, before, nil, positionals[0], where); err != nil { return err } if err := inv.ClearSettings(ctx, *node, positionals[0]); err != nil { @@ -998,6 +998,9 @@ func whereItComesFrom(repository, ref, commit, path string, self bool) (inventor // definition that got past the check — written elsewhere, or checked by nobody — is refused here // in the same words. A name meant on purpose is declared with its reason and passes. func namesNoInstallation(m catalogue.Manifest) error { + if problems := catalogue.TrustProblems(m); len(problems) > 0 { + return fmt.Errorf("%s", strings.Join(problems, "; ")) + } named := catalogue.InstallationProblems(m) if len(named) == 0 { return nil @@ -1080,12 +1083,19 @@ func throughAVerb() (string, bool) { // 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(before, after map[string]any, module, where string) error { +func refuseTerminalSettingsThroughAVerb(ctx context.Context, inv *inventory.Inventory, before, after map[string]any, + module, where string) error { verb, through := throughAVerb() if !through { return nil } - for _, key := range catalogue.TerminalKeys(module) { + // Judged against the module's definition as the catalogue holds it: what it serves and which of its files + // are trusted. A catalogue that cannot be read refuses rather than judging against nothing. + shelf, err := inv.Catalogue(ctx) + if err != nil { + return fmt.Errorf("which settings of %s are the terminal's cannot be read, so nothing was changed: %w", module, err) + } + for _, key := range catalogue.TerminalKeys(shelf[module]) { was, _ := json.Marshal(before[key]) now, _ := json.Marshal(after[key]) if string(was) == string(now) { diff --git a/cmd/mesh-controller/plain_words.go b/cmd/mesh-controller/plain_words.go index 81a568eb..faec2bbb 100644 --- a/cmd/mesh-controller/plain_words.go +++ b/cmd/mesh-controller/plain_words.go @@ -204,6 +204,14 @@ var plainWordings = map[string]func(conditions.Observation) words{ } return reloginWords(orModule(module), machineOr(o, "a machine"), false) }), + kindUsedAsFound: worded(func(o conditions.Observation) words { + module := "" + if o.Scope == conditions.ScopeModule && o.Machine != "" { + module = strings.TrimSuffix(o.ID, "."+o.Machine) + } + w := usedAsFoundObservation(orModule(module), machineOr(o, "a machine"), o.Summary, nil) + return words{Headline: w.Headline, Explanation: w.Explanation, Needs: w.Needs, Resolved: w.Resolved} + }), kindProviderFailing: worded(func(o conditions.Observation) words { thing, consumer := conditions.ThingWords(o), idPart(o, 2) if consumer == "" { diff --git a/cmd/mesh-controller/terminal_settings_test.go b/cmd/mesh-controller/terminal_settings_test.go index 79e6461f..849979a6 100644 --- a/cmd/mesh-controller/terminal_settings_test.go +++ b/cmd/mesh-controller/terminal_settings_test.go @@ -151,22 +151,33 @@ func TestPlacesAndAccessesAreRefusedThroughEveryVerb(t *testing.T) { } } -// The mesh's trust anchors are set at the terminal alone (novox/hq issue 339): through the settings verb, a caller -// could replace the internal authority's root every consumer trusts, or the issuer every login is checked against. -func TestATrustAnchorIsRefusedThroughAVerb(t *testing.T) { +// What a provider serves is set at the terminal alone (novox/hq issue 339): through the settings verb, a caller +// could move a database's port to a listener of its own and collect every consumer's credentials, or point every +// login at an issuer of its own. +func TestAServedKeyIsRefusedThroughAVerb(t *testing.T) { open := aMesh(t) ctx := t.Context() - register(t, open, catalogue.Manifest{Module: "step-ca", Version: "1", - Provides: catalogue.FromAnywhere("acme-ca"), - Serves: map[string]map[string]any{"acme-ca": {"root": "", "path": "/acme/acme/directory"}}, - Resources: []map[string]any{{"id": "rc", "type": "file", "path": "/etc/step.conf", "mode": "0644", - "content": "x = ${setting:x}\n"}}}) + register(t, open, catalogue.Manifest{Module: "store", Version: "1", + Provides: catalogue.FromAnywhere("database"), + Serves: map[string]map[string]any{"database": {"port": 5432.0}}, + Resources: []map[string]any{{"id": "rc", "type": "file", "path": "/etc/store.conf", "mode": "0644", + "trusted": false, "content": "x = ${setting:x}\n"}}}) register(t, open, catalogue.Manifest{Module: "keycloak", Version: "1", Provides: catalogue.FromAnywhere("oidc-client"), Serves: map[string]map[string]any{"oidc-client": {"issuer": "${setting:issuer}"}}, Resources: []map[string]any{{"id": "rc", "type": "file", "path": "/etc/kc.conf", "mode": "0644", - "content": "issuer = ${setting:issuer}\nx = ${setting:x}\n"}}}) - for _, m := range []string{"step-ca", "keycloak"} { + "trusted": false, "content": "x = ${setting:x}\n"}}}) + register(t, open, catalogue.Manifest{Module: "power", Version: "1", + Resources: []map[string]any{{"id": "logind", "type": "file", "path": "/etc/systemd/logind.conf.d/power.conf", + "mode": "0644", "trusted": true, "content": "HandleLidSwitch=${setting:lid}\nx=${setting:x}\n"}}}) + if err := atTheTerminal(t, "settings", "set", "keycloak", `{"issuer":"https://id.example/realms/mesh","x":0}`, + "--node", "anchor"); err != nil { + t.Fatalf("the issuer at the terminal: %v", err) + } + if err := atTheTerminal(t, "settings", "set", "power", `{"lid":"suspend","x":0}`, "--node", "anchor"); err != nil { + t.Fatal(err) + } + for _, m := range []string{"store", "keycloak", "power"} { if _, err := assign(ctx, open, "anchor", m); err != nil { t.Fatal(err) } @@ -177,20 +188,23 @@ func TestATrustAnchorIsRefusedThroughAVerb(t *testing.T) { t.Fatalf("%s: %v", what, err) } } - if err := atTheTerminal(t, "settings", "set", "keycloak", `{"issuer":"https://id.example/realms/mesh","x":0}`, - "--node", "anchor"); err != nil { - t.Fatalf("the issuer at the terminal: %v", err) - } + refused("a served port through the verb", throughVerb(t, "settings", map[string]any{"module": "store", + "node": "anchor", "values": `{"port":6543,"x":0}`})) + refused("a served port, mesh-wide, through the verb", throughVerb(t, "settings", + map[string]any{"module": "store", "values": `{"port":6543}`})) refused("the issuer through the verb", throughVerb(t, "settings", map[string]any{"module": "keycloak", "node": "anchor", "values": `{"issuer":"https://evil.example/realms/mesh","x":0}`})) - refused("the authority's root path through the verb", throughVerb(t, "settings", map[string]any{"module": "step-ca", - "node": "anchor", "values": `{"path":"/evil","x":0}`})) - refused("the authority's root path, mesh-wide, through the verb", throughVerb(t, "settings", - map[string]any{"module": "step-ca", "values": `{"path":"/evil"}`})) refused("clearing the issuer through the verb", throughVerb(t, "settings", map[string]any{"module": "keycloak", "node": "anchor", "clear": "true"})) if err := throughVerb(t, "settings", map[string]any{"module": "keycloak", "node": "anchor", "values": `{"issuer":"https://id.example/realms/mesh","x":1}`}); err != nil { t.Fatalf("another key through the verb, the issuer kept: %v", err) } + refused("a setting a trusted file asks for, through the verb", throughVerb(t, "settings", map[string]any{ + "module": "power", "node": "anchor", "values": `{"lid":"ignore","x":0}`})) + // And `trusted` is the catalogue's word: it never reaches the machine, whose engine parses strictly. + plan := printed(t, func() error { return atTheTerminal(t, "plan", "anchor", "--json") }) + if strings.Contains(plan, `"trusted"`) { + t.Fatal("the declaration carries the catalogue's `trusted`") + } } diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index 85a57b24..9e8b5cc7 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -913,6 +913,7 @@ func (r Resolution) compose(with Rendering, owner map[string]string, // such field, and the reason is for a reader of the manifest. delete(copied, SecretsInEnvironment) delete(copied, NamesOnPurpose) + delete(copied, TrustedField) // **An operator's value, from the assignment** (novox/hq ADR 0112, ADR 0155): what a // definition may not carry because it is true of one installation only. Filled from // the same layers a mergeable file takes, and refused when no layer set it. diff --git a/internal/catalogue/placement.go b/internal/catalogue/placement.go index 24781dd0..b552ba30 100644 --- a/internal/catalogue/placement.go +++ b/internal/catalogue/placement.go @@ -438,24 +438,3 @@ func namesOfAccessIDs(accesses map[string]string) []string { sort.Strings(names) return names } - -// trustAnchors are the settings a provider serves its consumers as what they trust, by module (novox/hq issue -// 339): set through a verb, any caller could point every consumer at an authority or an issuer of its own. -// -// - step-ca, the mesh's internal ACME authority: `root`, the root a consumer is handed to trust (the one -// setting that may hold lines, settingsHoldOneLine); `roots` and `path`, where a consumer fetches the roots -// and the ACME directory from, which a setting may override as it may any served fact. -// - keycloak, the identity provider: `issuer`, the issuer every OIDC consumer checks a login's token against. -// -// Named here, not in the manifests, because no manifest field says "this is trusted" yet; the catalogue was read -// for every served fact and every ${setting:…} on 2026-10-09, and these are the ones a consumer trusts. -var trustAnchors = map[string][]string{ - rootModule: {rootSetting, "roots", "path"}, - "keycloak": {"issuer"}, -} - -// TerminalKeys are the settings keys of a module that are set at the controller's terminal alone, never through -// a verb (novox/hq issue 339): places and accesses for every module, and a provider's trust anchors. -func TerminalKeys(module string) []string { - return append([]string{PlacesSetting, AccessesSetting}, trustAnchors[module]...) -} diff --git a/internal/catalogue/settings.go b/internal/catalogue/settings.go index 2fab2177..534c7424 100644 --- a/internal/catalogue/settings.go +++ b/internal/catalogue/settings.go @@ -90,6 +90,7 @@ func ApplySettings(resource map[string]any, layers []Layer) (map[string]any, err out["content"] = string(rendered) + "\n" delete(out, "merge") delete(out, "protected") + delete(out, TrustedField) return out, nil } diff --git a/internal/catalogue/terminal_keys.go b/internal/catalogue/terminal_keys.go new file mode 100644 index 00000000..cd294c0c --- /dev/null +++ b/internal/catalogue/terminal_keys.go @@ -0,0 +1,112 @@ +package catalogue + +import ( + "fmt" + "sort" + "time" +) + +// Which settings are the controller's terminal's alone (novox/hq issue 339). +// +// A setting is the operator's word on how a module is configured, and the `settings` verb writes it for any +// caller allowed to call verbs — agents among them. Most settings change only the module itself. Some change +// what root or another module trusts, and those are said at the terminal alone, never through a verb: +// +// 1. `places` and `accesses`: where the node-engine creates and, as root, owns a module's directories, and +// which of the machine's paths are mounted into its container. +// 2. **Every key a provider serves**, and every setting a served value asks for. A setting overrides a served +// key (Settle), and what is served is what every consumer of the provision connects to and believes: a +// database's port, an object store's scheme, a registry's port, an identity provider's issuer and token +// path. Through a verb, any caller could point every consumer at a listener of its own and collect the +// credentials they present. +// 3. **Every setting a file marked `trusted` asks for**: a file root or a consumer trusts — a logind drop-in, +// an env file that says which uid a container runs as, a script run as root. The manifest says so on the +// file (TrustedField), and `module check` names a file that asks for a setting without saying. +// +// 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 +// makes each a terminal key; false says, out loud, that none changes what root or a consumer trusts. Said in the +// catalogue, never on the machine: the composer takes it out before the node-engine, which parses strictly. +const TrustedField = "trusted" + +// TrustRequiredFrom is when `module check` refuses a file that asks for a setting and does not say whether it is +// trusted. Until then it is warned and counted: the catalogue's files get the field in their own change, which +// can only land once a controller that takes the field out of the declaration runs. +var TrustRequiredFrom = time.Date(2026, 10, 30, 0, 0, 0, 0, time.UTC) + +// TerminalKeys are the settings keys of a module that are set at the controller's terminal alone: places and +// accesses, every key its provisions serve and every setting a served value asks for, and every setting a file +// marked trusted asks for. Places and accesses first, then the rest sorted. +func TerminalKeys(m Manifest) []string { + keys := map[string]bool{} + for _, served := range m.Serves { + for key, value := range served { + keys[key] = true + if s, ok := value.(string); ok { + for _, asked := range settingsUsed(s) { + keys[asked] = true + } + } + } + } + for _, r := range m.Resources { + if trusted, _ := r[TrustedField].(bool); !trusted || fmt.Sprint(r["type"]) != "file" { + continue + } + if content, ok := r["content"].(string); ok { + for _, asked := range settingsUsed(content) { + keys[asked] = true + } + } + } + delete(keys, PlacesSetting) + delete(keys, AccessesSetting) + rest := make([]string, 0, len(keys)) + for k := range keys { + rest = append(rest, k) + } + sort.Strings(rest) + return append([]string{PlacesSetting, AccessesSetting}, rest...) +} + +// UnsaidTrust is every file of a module that asks for a setting and does not say whether it is trusted, by id. +func UnsaidTrust(m Manifest) []string { + var out []string + for _, r := range m.Resources { + if fmt.Sprint(r["type"]) != "file" { + continue + } + content, _ := r["content"].(string) + if len(settingsUsed(content)) == 0 { + continue + } + if _, said := r[TrustedField]; !said { + out = append(out, fmt.Sprint(r["id"])) + } + } + sort.Strings(out) + return out +} + +// TrustProblems are the ways a manifest states `trusted` wrongly: anything but true or false, or on anything but +// a file. Refused at registration and by `module check`. +func TrustProblems(m Manifest) []string { + var out []string + for _, r := range m.Resources { + v, said := r[TrustedField] + if !said { + continue + } + if fmt.Sprint(r["type"]) != "file" { + out = append(out, fmt.Sprintf("%s: %v is a %v and says %q; only a file says whether the settings it "+ + "asks for are trusted (novox/hq issue 339)", m.Module, r["id"], r["type"], TrustedField)) + continue + } + if _, ok := v.(bool); !ok { + out = append(out, fmt.Sprintf("%s: %v says %q as %v; it is true or false (novox/hq issue 339)", + m.Module, r["id"], TrustedField, v)) + } + } + return out +} diff --git a/internal/catalogue/terminal_settings_test.go b/internal/catalogue/terminal_settings_test.go index a8a722f2..79950ff7 100644 --- a/internal/catalogue/terminal_settings_test.go +++ b/internal/catalogue/terminal_settings_test.go @@ -128,17 +128,48 @@ func TestTheRuntimesDataAndAnyHomesSSHAreTheMachinesOwn(t *testing.T) { } } -// A trust anchor the mesh hands its consumers is the terminal's too (novox/hq issue 339): the authority's root, -// where its roots and directory are, and the identity provider's issuer. -func TestTrustAnchorsAreTerminalKeys(t *testing.T) { - for module, keys := range map[string][]string{ - "step-ca": {"places", "accesses", "root", "roots", "path"}, - "keycloak": {"places", "accesses", "issuer"}, - "mailu": {"places", "accesses"}, +// What a provider serves its consumers is the terminal's (novox/hq issue 339): any key under its `serves`, and any +// setting a served value asks for, is set at the terminal alone — a verb that could change a port could point every +// consumer at a listener of the caller's own. So is any setting a file marked `trusted` asks for. +func TestTerminalKeysAreDerived(t *testing.T) { + postgres := Manifest{Module: "postgres", Serves: map[string]map[string]any{"postgres-database": {"port": 5432.0}}} + keycloak := Manifest{Module: "keycloak", Serves: map[string]map[string]any{"oidc-client": { + "issuer": "${setting:issuer}", "token-path": "/protocol/openid-connect/token"}}} + power := Manifest{Module: "power", Resources: []map[string]any{ + {"id": "logind", "type": "file", "path": "/etc/systemd/logind.conf.d/power.conf", "trusted": true, + "content": "HandleLidSwitch=${setting:handle-lid-switch}\n"}, + {"id": "note", "type": "file", "path": "/var/lib/power/note", "trusted": false, "content": "${setting:greeting}\n"}}} + for _, c := range []struct { + m Manifest + want string + }{ + {postgres, "places,accesses,port"}, + {keycloak, "places,accesses,issuer,token-path"}, + {power, "places,accesses,handle-lid-switch"}, + {Manifest{Module: "plain"}, "places,accesses"}, } { - got := TerminalKeys(module) - if strings.Join(got, ",") != strings.Join(keys, ",") { - t.Errorf("%s: %v; want %v", module, got, keys) + if got := strings.Join(TerminalKeys(c.m), ","); got != c.want { + t.Errorf("%s: %s; want %s", c.m.Module, got, c.want) + } + } +} + +// A file that asks for a setting says whether what it asks is trusted (novox/hq issue 339): `trusted` is a +// boolean, on a file alone, and a file asking for a setting without it is named. +func TestAFileSaysWhetherItsSettingsAreTrusted(t *testing.T) { + m := Manifest{Module: "power", Resources: []map[string]any{ + {"id": "said", "type": "file", "path": "/etc/a", "trusted": true, "content": "${setting:a}"}, + {"id": "unsaid", "type": "file", "path": "/etc/b", "content": "${setting:b}"}, + {"id": "no-setting", "type": "file", "path": "/etc/c", "content": "plain"}}} + if got := strings.Join(UnsaidTrust(m), ","); got != "unsaid" { + t.Errorf("unsaid: %s; want unsaid", got) + } + for _, bad := range []map[string]any{ + {"id": "x", "type": "file", "path": "/etc/x", "trusted": "yes", "content": "${setting:a}"}, + {"id": "y", "type": "directory", "trusted": true}, + } { + if problems := TrustProblems(Manifest{Module: "power", Resources: []map[string]any{bad}}); len(problems) == 0 { + t.Errorf("%v was taken", bad) } } } From c3ae3f3e098f7c1586225e4cc7072eac50f68aa6 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 01:37:43 +0200 Subject: [PATCH 6/7] Refuse a file that asks for a setting without saying whether it is trusted from 2026-10-10 The operator's date (hq issue 339). A test holds the warning before it and the refusal from it. --- cmd/mesh-controller/health_check_test.go | 28 ++++++++++++++++++++++++ internal/catalogue/terminal_keys.go | 2 +- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/cmd/mesh-controller/health_check_test.go b/cmd/mesh-controller/health_check_test.go index 6f3f2351..4b835017 100644 --- a/cmd/mesh-controller/health_check_test.go +++ b/cmd/mesh-controller/health_check_test.go @@ -45,3 +45,31 @@ func TestModuleCheckCountsTheUndeclaredAndRefusesThemFromTheDate(t *testing.T) { t.Errorf("the refusal does not name the resource:\n%s", out.String()) } } + +// A file that asks for a setting says whether it is trusted (novox/hq issue 339): `module check` warns and counts +// it before the date, and refuses it from the date; a file that says so passes either way. +func TestModuleCheckCountsUnsaidTrustAndRefusesItFromTheDate(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "module.json") + os.WriteFile(path, []byte(`{"module":"power","resources":[ + {"id":"logind","type":"file","path":"/etc/systemd/logind.conf.d/power.conf","mode":"0644","trusted":true, + "content":"HandleLidSwitch=${setting:lid}\n"}, + {"id":"note","type":"file","path":"/var/lib/power/note","mode":"0644","content":"${setting:greeting}\n"}]}`), 0o600) + defer func() { checkNow = time.Now }() + + checkNow = func() time.Time { return catalogue.TrustRequiredFrom.Add(-time.Hour) } + var out bytes.Buffer + if err := moduleCheck([]string{path}, &out); err != nil { + t.Fatalf("refused before the date: %v\n%s", err, out.String()) + } + for _, want := range []string{"WARNING: note ask(s) for a setting", UnsaidTrustLine + " 1"} { + if !strings.Contains(out.String(), want) { + t.Errorf("the check does not say %q:\n%s", want, out.String()) + } + } + checkNow = func() time.Time { return catalogue.TrustRequiredFrom } + out.Reset() + if err := moduleCheck([]string{path}, &out); err == nil || !strings.Contains(out.String(), "power: the file note asks for a setting") { + t.Fatalf("an unsaid file passed after the date: %v\n%s", err, out.String()) + } +} diff --git a/internal/catalogue/terminal_keys.go b/internal/catalogue/terminal_keys.go index cd294c0c..a08dfa26 100644 --- a/internal/catalogue/terminal_keys.go +++ b/internal/catalogue/terminal_keys.go @@ -33,7 +33,7 @@ const TrustedField = "trusted" // TrustRequiredFrom is when `module check` refuses a file that asks for a setting and does not say whether it is // trusted. Until then it is warned and counted: the catalogue's files get the field in their own change, which // can only land once a controller that takes the field out of the declaration runs. -var TrustRequiredFrom = time.Date(2026, 10, 30, 0, 0, 0, 0, time.UTC) +var TrustRequiredFrom = time.Date(2026, 10, 10, 0, 0, 0, 0, time.UTC) // TerminalKeys are the settings keys of a module that are set at the controller's terminal alone: places and // accesses, every key its provisions serve and every setting a served value asks for, and every setting a file From 0f58602c740fa696e65ccba8aef0606dcd78cc7c Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 01:44:50 +0200 Subject: [PATCH 7/7] Count a file that says nothing as trusted, and drop the refusal date The fourth review (hq issue 339): the safe reading of a file that asks for a setting and does not say is that root or a consumer trusts it, so its settings are the terminal's; `"trusted": false` is the opt-out. With that, nothing unsafe is left to refuse: `module check` lists and counts the unmarked files and never refuses them. --- cmd/mesh-controller/check.go | 25 +++++---------- cmd/mesh-controller/health_check_test.go | 31 +++++++++---------- cmd/mesh-controller/terminal_settings_test.go | 5 ++- internal/catalogue/terminal_keys.go | 28 ++++++++--------- internal/catalogue/terminal_settings_test.go | 6 ++-- 5 files changed, 43 insertions(+), 52 deletions(-) diff --git a/cmd/mesh-controller/check.go b/cmd/mesh-controller/check.go index d53d3a82..cb4c6acd 100644 --- a/cmd/mesh-controller/check.go +++ b/cmd/mesh-controller/check.go @@ -137,23 +137,11 @@ func moduleCheckFor(paths []string, longestMachine int, out io.Writer) error { } } - // **Every file that asks for a setting says whether it is trusted** (novox/hq issue 339): warned until the - // date, refused from it, and counted for the catalogue's merge check like the resources without health. + // **A file that asks for a setting and does not say whether it is trusted counts as trusted** (novox/hq issue + // 339): listed and counted, never refused, so an author can opt out a file nothing trusts. unsaid := 0 - trustRequired := !checkNow().Before(catalogue.TrustRequiredFrom) for _, name := range names { - missing := catalogue.UnsaidTrust(shelf[name]) - unsaid += len(missing) - if trustRequired { - for _, id := range missing { - fmt.Fprintf(out, "%s: the file %s asks for a setting and does not say whether it is trusted: say "+ - "%q true or false (novox/hq issue 339)\n", name, id, catalogue.TrustedField) - } - if len(missing) > 0 { - failed += len(missing) - faulted[name] = true - } - } + unsaid += len(catalogue.UnsaidTrust(shelf[name])) } for _, name := range names { @@ -198,8 +186,9 @@ func moduleCheckFor(paths []string, longestMachine int, out io.Writer) error { fmt.Fprintf(out, ", ready: %s", strings.Join(checks, "; ")) } if missing := catalogue.UnsaidTrust(m); len(missing) > 0 { - fmt.Fprintf(out, "; WARNING: %s ask(s) for a setting and say(s) not whether it is trusted, refused from "+ - "%s (novox/hq issue 339)", strings.Join(missing, ", "), catalogue.TrustRequiredFrom.Format("2006-01-02")) + fmt.Fprintf(out, "; WARNING: %s ask(s) for a setting and do(es) not say whether it is trusted, so it counts as "+ + "trusted: set at the terminal alone; say %q false where nothing trusts it (novox/hq issue 339)", + strings.Join(missing, ", "), catalogue.TrustedField) } if missing := catalogue.Undeclared(m); len(missing) > 0 { fmt.Fprintf(out, "; WARNING: %s stay(s) up and say(s) not how it is ready — judged by liveness alone, "+ @@ -225,7 +214,7 @@ func moduleCheckFor(paths []string, longestMachine int, out io.Writer) error { const UndeclaredHealthLine = "long-running resources without health:" // UnsaidTrustLine starts the line `module check` says the count of files that ask for a setting and do not say -// whether it is trusted (novox/hq issue 339). +// whether it is trusted, and so count as trusted (novox/hq issue 339). const UnsaidTrustLine = "files asking for a setting without saying whether it is trusted:" // checkNow is the clock `module check` judges the date by; a test sets it. diff --git a/cmd/mesh-controller/health_check_test.go b/cmd/mesh-controller/health_check_test.go index 4b835017..5ac0e1cc 100644 --- a/cmd/mesh-controller/health_check_test.go +++ b/cmd/mesh-controller/health_check_test.go @@ -46,9 +46,9 @@ func TestModuleCheckCountsTheUndeclaredAndRefusesThemFromTheDate(t *testing.T) { } } -// A file that asks for a setting says whether it is trusted (novox/hq issue 339): `module check` warns and counts -// it before the date, and refuses it from the date; a file that says so passes either way. -func TestModuleCheckCountsUnsaidTrustAndRefusesItFromTheDate(t *testing.T) { +// A file that asks for a setting without saying whether it is trusted counts as trusted (novox/hq issue 339): +// `module check` lists and counts it, and never refuses it — there is nothing unsafe to refuse. +func TestModuleCheckListsUnmarkedFilesAndNeverRefusesThem(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "module.json") os.WriteFile(path, []byte(`{"module":"power","resources":[ @@ -56,20 +56,17 @@ func TestModuleCheckCountsUnsaidTrustAndRefusesItFromTheDate(t *testing.T) { "content":"HandleLidSwitch=${setting:lid}\n"}, {"id":"note","type":"file","path":"/var/lib/power/note","mode":"0644","content":"${setting:greeting}\n"}]}`), 0o600) defer func() { checkNow = time.Now }() - - checkNow = func() time.Time { return catalogue.TrustRequiredFrom.Add(-time.Hour) } - var out bytes.Buffer - if err := moduleCheck([]string{path}, &out); err != nil { - t.Fatalf("refused before the date: %v\n%s", err, out.String()) - } - for _, want := range []string{"WARNING: note ask(s) for a setting", UnsaidTrustLine + " 1"} { - if !strings.Contains(out.String(), want) { - t.Errorf("the check does not say %q:\n%s", want, out.String()) + for _, at := range []time.Time{time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC), time.Date(2036, 1, 1, 0, 0, 0, 0, time.UTC)} { + checkNow = func() time.Time { return at } + var out bytes.Buffer + if err := moduleCheck([]string{path}, &out); err != nil { + t.Fatalf("refused at %v: %v\n%s", at, err, out.String()) + } + for _, want := range []string{"note ask(s) for a setting and do(es) not say whether it is trusted, so it counts as trusted", + UnsaidTrustLine + " 1"} { + if !strings.Contains(out.String(), want) { + t.Errorf("the check does not say %q:\n%s", want, out.String()) + } } } - checkNow = func() time.Time { return catalogue.TrustRequiredFrom } - out.Reset() - if err := moduleCheck([]string{path}, &out); err == nil || !strings.Contains(out.String(), "power: the file note asks for a setting") { - t.Fatalf("an unsaid file passed after the date: %v\n%s", err, out.String()) - } } diff --git a/cmd/mesh-controller/terminal_settings_test.go b/cmd/mesh-controller/terminal_settings_test.go index 849979a6..9d9803e4 100644 --- a/cmd/mesh-controller/terminal_settings_test.go +++ b/cmd/mesh-controller/terminal_settings_test.go @@ -49,7 +49,10 @@ func TestPlacesAndAccessesAreRefusedThroughEveryVerb(t *testing.T) { register(t, open, catalogue.Manifest{Module: "notes", Version: "1", Accesses: []catalogue.Access{{ID: "media", Path: "/storage/media", Mode: "read"}}, Resources: []map[string]any{{"id": "data", "type": "directory", "mode": "0755"}, - {"id": "rc", "type": "file", "path": "/etc/notes.conf", "mode": "0644", "content": "x = ${setting:x}\n"}}}) + // Nothing trusts this file: said, so a verb may change what it asks for (an unmarked one counts as + // trusted, and only the terminal could). + {"id": "rc", "type": "file", "path": "/etc/notes.conf", "mode": "0644", "trusted": false, + "content": "x = ${setting:x}\n"}}}) if _, err := assign(ctx, open, "laptop", "notes"); err != nil { t.Fatal(err) } diff --git a/internal/catalogue/terminal_keys.go b/internal/catalogue/terminal_keys.go index a08dfa26..342b33ac 100644 --- a/internal/catalogue/terminal_keys.go +++ b/internal/catalogue/terminal_keys.go @@ -3,7 +3,6 @@ package catalogue import ( "fmt" "sort" - "time" ) // Which settings are the controller's terminal's alone (novox/hq issue 339). @@ -19,25 +18,22 @@ import ( // database's port, an object store's scheme, a registry's port, an identity provider's issuer and token // path. Through a verb, any caller could point every consumer at a listener of its own and collect the // credentials they present. -// 3. **Every setting a file marked `trusted` asks for**: a file root or a consumer trusts — a logind drop-in, -// an env file that says which uid a container runs as, a script run as root. The manifest says so on the -// file (TrustedField), and `module check` names a file that asks for a setting without saying. +// 3. **Every setting a file asks for, unless the file says `"trusted": false`.** A file root or a consumer trusts — +// a logind drop-in, an env file that says which uid a container runs as, a script run as root — must not +// change through a verb, and the safe reading of a file that says nothing is that it is one of them (fail +// closed). `"trusted": false` is the opt-out, for a file nothing trusts: a person's own notifier settings. +// `module check` lists the files that say nothing, so an author can opt one out where that is true. // // 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 -// makes each a terminal key; false says, out loud, that none changes what root or a consumer trusts. Said in the +// TrustedField is the key a file resource carries to say whether the settings it asks for are trusted: true, or +// absent, makes each a terminal key; false says, out loud, that none changes what root or a consumer trusts. Said in the // catalogue, never on the machine: the composer takes it out before the node-engine, which parses strictly. const TrustedField = "trusted" -// TrustRequiredFrom is when `module check` refuses a file that asks for a setting and does not say whether it is -// trusted. Until then it is warned and counted: the catalogue's files get the field in their own change, which -// can only land once a controller that takes the field out of the declaration runs. -var TrustRequiredFrom = time.Date(2026, 10, 10, 0, 0, 0, 0, time.UTC) - // TerminalKeys are the settings keys of a module that are set at the controller's terminal alone: places and // accesses, every key its provisions serve and every setting a served value asks for, and every setting a file -// marked trusted asks for. Places and accesses first, then the rest sorted. +// asks for unless it says `"trusted": false`. Places and accesses first, then the rest sorted. func TerminalKeys(m Manifest) []string { keys := map[string]bool{} for _, served := range m.Serves { @@ -51,7 +47,10 @@ func TerminalKeys(m Manifest) []string { } } for _, r := range m.Resources { - if trusted, _ := r[TrustedField].(bool); !trusted || fmt.Sprint(r["type"]) != "file" { + if fmt.Sprint(r["type"]) != "file" { + continue + } + if trusted, said := r[TrustedField].(bool); said && !trusted { continue } if content, ok := r["content"].(string); ok { @@ -70,7 +69,8 @@ func TerminalKeys(m Manifest) []string { return append([]string{PlacesSetting, AccessesSetting}, rest...) } -// UnsaidTrust is every file of a module that asks for a setting and does not say whether it is trusted, by id. +// UnsaidTrust is every file of a module that asks for a setting and does not say whether it is trusted, by id: +// each counts as trusted, and is listed so an author can opt out a file nothing trusts. func UnsaidTrust(m Manifest) []string { var out []string for _, r := range m.Resources { diff --git a/internal/catalogue/terminal_settings_test.go b/internal/catalogue/terminal_settings_test.go index 79950ff7..c9c3bac6 100644 --- a/internal/catalogue/terminal_settings_test.go +++ b/internal/catalogue/terminal_settings_test.go @@ -138,14 +138,16 @@ func TestTerminalKeysAreDerived(t *testing.T) { power := Manifest{Module: "power", Resources: []map[string]any{ {"id": "logind", "type": "file", "path": "/etc/systemd/logind.conf.d/power.conf", "trusted": true, "content": "HandleLidSwitch=${setting:handle-lid-switch}\n"}, - {"id": "note", "type": "file", "path": "/var/lib/power/note", "trusted": false, "content": "${setting:greeting}\n"}}} + {"id": "note", "type": "file", "path": "/var/lib/power/note", "trusted": false, "content": "${setting:greeting}\n"}, + // Unmarked counts as trusted (fail closed): only `"trusted": false` lets a verb change what a file asks for. + {"id": "unmarked", "type": "file", "path": "/etc/power/unmarked", "content": "${setting:unmarked}\n"}}} for _, c := range []struct { m Manifest want string }{ {postgres, "places,accesses,port"}, {keycloak, "places,accesses,issuer,token-path"}, - {power, "places,accesses,handle-lid-switch"}, + {power, "places,accesses,handle-lid-switch,unmarked"}, {Manifest{Module: "plain"}, "places,accesses"}, } { if got := strings.Join(TerminalKeys(c.m), ","); got != c.want {