From 63a99d43237f6d2c3aa6d90c585a32047d9fb5b8 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 14:30:00 +0200 Subject: [PATCH] Name the vault's placed directories among a node's secret places, fail loudly on one that cannot be resolved, and keep the reads account out of every group (review 54, issue 418) The real mesh-vault manifest declares its directories without a path, so the list left its state and ledger out, and a place still holding a placeholder vanished from it without a word. The provider's directories are now resolved where the node places them, an unresolved place fails the composition naming the module, and a node-reads holder whose account is put in a group is refused. --- internal/catalogue/reads.go | 80 +++++++++++++++++++++++--------- internal/catalogue/reads_test.go | 41 +++++++++++++--- internal/catalogue/runtime.go | 2 +- 3 files changed, 93 insertions(+), 30 deletions(-) diff --git a/internal/catalogue/reads.go b/internal/catalogue/reads.go index 0f27849c..93686a2a 100644 --- a/internal/catalogue/reads.go +++ b/internal/catalogue/reads.go @@ -91,54 +91,86 @@ func readsVerbs() []Verb { // account of its own, so the node's file modes bound what it sees. Carried by the machine's runtime it would // read as the operator's account, which sees the operator's keys and the agent's credentials. func ReadsHolderProblems(m Manifest) []string { - if !m.ClaimsSeat(ReadsSeat) || m.RunsAs != "" { + if !m.ClaimsSeat(ReadsSeat) { return nil } - return []string{fmt.Sprintf("%s claims %s and must run as an account of its own (runs-as): its reads are "+ - "bounded by that account's file modes, and the machine's runtime runs as the operator's account "+ - "(novox/hq ADR 0310 §3)", m.Module, ReadsSeat)} + if m.RunsAs == "" { + return []string{fmt.Sprintf("%s claims %s and must run as an account of its own (runs-as): its reads are "+ + "bounded by that account's file modes, and the machine's runtime runs as the operator's account "+ + "(novox/hq ADR 0310 §3)", m.Module, ReadsSeat)} + } + // In no group beside its own: a group is a way to a secret (docker's is root's), and the account's file modes + // are what bound its reads. + for _, r := range m.Resources { + if fmt.Sprint(r["type"]) != "user" || fmt.Sprint(r["name"]) != m.RunsAs { + continue + } + if groups, said := r["groups"]; said { + if list, ok := groups.([]any); !ok || len(list) > 0 { + return []string{fmt.Sprintf("%s claims %s, and its account %s is put in groups %v: the account that "+ + "reads is in no group beside its own (novox/hq ADR 0310 §3)", m.Module, ReadsSeat, m.RunsAs, groups)} + } + } + } + return nil } // secretPlaces is every place on this node a secret is kept, as the placed manifests say (novox/hq ADR 0310 // §5): the directory of every module's own secret, every credential file a module is given and every grant // directory a provider is handed its consumers' credentials in, the place a module keeps the operator-sealed -// secrets, and every directory of the module that provides `secret` — the vault. Sorted, each once, and a -// place under another dropped, so the list says each refusal once. -func (r Resolution) secretPlaces() []string { +// secrets, and every directory of the module that provides `secret` — the vault — wherever the node places it, +// a directory declared without a path included. Sorted, each once, and a place under another dropped, so the +// list says each refusal once. +// +// **Failure is loud.** A place still holding a placeholder, or not absolute, fails the composition with the +// module named: a list that silently shrank would be a refusal the decision promises and nothing does. +func (r Resolution) secretPlaces(with Rendering) ([]string, error) { seen := map[string]bool{} - add := func(path string) { + var problems []string + add := func(module, what, path string) { path = strings.TrimSpace(path) + if path == "" { + return + } if !strings.HasPrefix(path, "/") || strings.Contains(path, "${") { + problems = append(problems, fmt.Sprintf("%s's %s is %q, which is not a place on %s", module, what, path, r.Node)) return } path = filepath.Clean(path) if path == "/" { + problems = append(problems, fmt.Sprintf("%s's %s is the root directory", module, what)) return } seen[path] = true } for _, m := range r.Modules { - for _, own := range m.OwnSecrets { - add(filepath.Dir(own.Path)) + for name, own := range m.OwnSecrets { + add(m.Module, "own secret "+name, own.Path) + if strings.HasPrefix(own.Path, "/") { + add(m.Module, "own secret "+name+"'s directory", filepath.Dir(own.Path)) + } } for _, to := range m.SecretRequirements() { for _, f := range m.SecretFiles(to) { - add(f.Path) + add(m.Module, "credential file for "+to, f.Path) } } - for _, where := range m.Grants { - add(where) + for to, where := range m.Grants { + add(m.Module, "grant directory for "+to, where) } - add(m.Keeps) + add(m.Module, "place for the operator-sealed secrets (keeps)", m.Keeps) if providesSecret(m) { - for _, res := range m.Resources { - if fmt.Sprint(res["type"]) == "directory" { - path, _ := res["path"].(string) - add(path) - } + dirs := dirsFor(m, with) + for _, id := range sortedKeys(dirs) { + add(m.Module, "directory "+id, dirs[id]) } } } + if len(problems) > 0 { + sort.Strings(problems) + return nil, fmt.Errorf("the holder of %s on %s cannot be told where a secret is kept, so it would read "+ + "one (novox/hq ADR 0310 §5): %s", ReadsSeat, r.Node, strings.Join(problems, "; ")) + } all := make([]string, 0, len(seen)) for p := range seen { all = append(all, p) @@ -151,7 +183,7 @@ func (r Resolution) secretPlaces() []string { } out = append(out, p) } - return out + return out, nil } func providesSecret(m Manifest) bool { @@ -164,11 +196,15 @@ func providesSecret(m Manifest) bool { } // readsWords is what the holder of node-reads is given beside its own words: the node's secret places. -func (r Resolution) readsWords(m Manifest, words map[string]string) (map[string]string, error) { +func (r Resolution) readsWords(m Manifest, words map[string]string, with Rendering) (map[string]string, error) { if !m.ClaimsSeat(ReadsSeat) { return words, nil } - body, err := json.Marshal(r.secretPlaces()) + places, err := r.secretPlaces(with) + if err != nil { + return nil, err + } + body, err := json.Marshal(places) if err != nil { return nil, err } diff --git a/internal/catalogue/reads_test.go b/internal/catalogue/reads_test.go index ce94cd0f..90e17daf 100644 --- a/internal/catalogue/reads_test.go +++ b/internal/catalogue/reads_test.go @@ -73,12 +73,11 @@ func TestTheHolderOfNodeReadsIsToldWhereItsNodeKeepsASecret(t *testing.T) { desk := aToolsModule(t, "desk", "tools/index.js") desk.RunsAs, desk.SecretsOwner = "desk", "desk" desk.OwnSecrets = OwnSecrets{"broker": {Path: "/var/lib/desk/broker"}} - vault := Manifest{Module: "vault", Version: "1", Provides: []Offer{{Name: "secret"}}, - Keeps: "/var/lib/vault/root", Grants: map[string]string{"secret": "/var/lib/vault/grants"}, - Resources: []map[string]any{ - {"id": "ledger", "type": "directory", "path": "/var/lib/vault/ledger"}, - {"id": "grants", "type": "directory", "path": "/var/lib/vault/grants"}, - }} + // The vault as the catalogue declares it: its directories have no path of their own, and the node places them. + vault, err := placedManifest(catalogueManifest(t, "mesh-vault"), with) + if err != nil { + t.Fatal(err) + } app := Manifest{Module: "app", Version: "1", Secrets: map[string]string{"postgres-database": "/etc/app/db.secret"}, OwnSecrets: OwnSecrets{"token": {Path: "/home/ops/.config/app/token"}}} r := Resolution{Node: "anchor", Account: "ops", Modules: []Manifest{reads, desk, vault, app, goRuntime}} @@ -102,8 +101,36 @@ func TestTheHolderOfNodeReadsIsToldWhereItsNodeKeepsASecret(t *testing.T) { t.Fatalf("the holder is not told its node's secret places: %v (%v)", err, given) } want := []string{"/etc/app/db.secret", "/home/ops/.config/app", "/var/lib/desk", "/var/lib/mesh-reads", - "/var/lib/mesh/" + RuntimeModule, "/var/lib/vault/grants", "/var/lib/vault/ledger", "/var/lib/vault/root"} + "/var/lib/mesh-vault", "/var/lib/mesh/" + RuntimeModule} if strings.Join(places, " ") != strings.Join(want, " ") { t.Errorf("secret places:\n got %v\nwant %v", places, want) } } + +// The review of #257: a secret place the controller cannot resolve fails the composition, naming the module, +// rather than leaving the list one place shorter. +func TestASecretPlaceThatCannotBeResolvedFailsTheComposition(t *testing.T) { + r := Resolution{Node: "anchor", Modules: []Manifest{ + {Module: "app", Version: "1", Keeps: "${dir:keep}/sealed"}, + }} + if _, err := r.secretPlaces(Rendering{}); err == nil || !strings.Contains(err.Error(), "app") { + t.Errorf("an unresolved place was dropped: %v", err) + } + r.Modules[0].Keeps = "relative/place" + if _, err := r.secretPlaces(Rendering{}); err == nil { + t.Error("a relative place was dropped") + } +} + +// ADR 0310 §3: the account that reads is in no group beside its own; one put in a group is refused. +func TestTheReadsAccountIsInNoGroup(t *testing.T) { + m := Manifest{Module: "reads", RunsAs: "mesh-reads", Claims: []Claim{{Name: ReadsSeat}}, + Resources: []map[string]any{{"id": "account", "type": "user", "name": "mesh-reads", "groups": []any{"docker"}}}} + if p := ReadsHolderProblems(m); len(p) != 1 || !strings.Contains(p[0], "docker") { + t.Errorf("an account in docker was not refused: %v", p) + } + m.Resources[0]["groups"] = []any{} + if p := ReadsHolderProblems(m); len(p) != 0 { + t.Errorf("an account in no group was refused: %v", p) + } +} diff --git a/internal/catalogue/runtime.go b/internal/catalogue/runtime.go index 2f83c219..874f7108 100644 --- a/internal/catalogue/runtime.go +++ b/internal/catalogue/runtime.go @@ -304,7 +304,7 @@ func (r Resolution) ownRuntimes(with Rendering) ([]map[string]any, error) { return nil, err } // The holder of node-reads is told where this node keeps its secrets (novox/hq ADR 0310 §5). - if words, err = r.readsWords(m, words); err != nil { + if words, err = r.readsWords(m, words, with); err != nil { return nil, err } if len(words) > 0 {