Merge pull request 'Make no value for an own secret issued outside the mesh that nobody gave (issue 401)' (#262) from fix/401-an-outside-secret-not-given-is-not-made into main

cmd/mesh-controller/outside_not_given_test.go

@@ -0,0 +1,120 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/novox/mesh-controller/internal/catalogue"
+)
+
+// An own secret issued outside the mesh that nobody gave is never made (novox/hq issue 401, ADR 0228). Seen on
+// 2026-10-10: the send made a random value for the mounts module's smb-credentials and placed it, and the module read
+// it as a credential given and wrong. The send now makes none, places none, and places a value only once a person
+// gave one — and a value the mesh made for it before is not placed either.
+
+// anAssistant is a module with a secret the mesh makes and one only an outside party issues.
+func anAssistant() catalogue.Manifest {
+	return catalogue.Manifest{Module: "assistant", Version: "1",
+		OwnSecrets: catalogue.OwnSecrets{
+			"password":   {Path: "/var/lib/mesh/assistant/password", Taken: catalogue.TakenAtStart},
+			"vendor-key": {Path: "/var/lib/mesh/assistant/vendor-key", Taken: catalogue.TakenAtStart, IssuedBy: catalogue.IssuedOutside},
+		},
+		Resources: []map[string]any{
+			{"id": "state", "type": "directory", "path": "/var/lib/mesh/assistant", "mode": "0700"},
+		}}
+}
+
+// sentTo composes what a push sends to a machine, on the send path.
+func sentTo(t *testing.T, open *stores, node string) []map[string]any {
+	t.Helper()
+	plan, settings, err := planFor(t.Context(), open, node)
+	if err != nil {
+		t.Fatal(err)
+	}
+	declared, err := declarationFor(t.Context(), open, node, plan, settings)
+	if err != nil {
+		t.Fatalf("the send to %s did not compose: %v", node, err)
+	}
+	return declared.Resources
+}
+
+func placedAt(resources []map[string]any, path string) map[string]any {
+	for _, r := range resources {
+		if r["path"] == path && r["sealed"] != nil {
+			return r
+		}
+	}
+	return nil
+}
+
+func TestTheSendMakesNoValueForAnOutsideSecretNobodyGave(t *testing.T) {
+	open := aMesh(t)
+	ctx := t.Context()
+	register(t, open, anAssistant())
+	if _, err := assign(ctx, open, "laptop", "assistant"); err != nil {
+		t.Fatal(err)
+	}
+
+	sent := sentTo(t, open, "laptop")
+	if placedAt(sent, "/var/lib/mesh/assistant/password") == nil {
+		t.Fatal("the secret the mesh makes was not placed")
+	}
+	if got := placedAt(sent, "/var/lib/mesh/assistant/vendor-key"); got != nil {
+		t.Fatalf("a secret nobody gave was placed: %v", got)
+	}
+	if _, held, err := open.inventory.ModuleSecretIfIssued(ctx, "laptop", "assistant", "vendor-key"); err != nil || held {
+		t.Fatalf("the send made a value for a secret issued outside the mesh (held %v, %v)", held, err)
+	}
+
+	// A value the mesh made before this rule is not one a person gave: it is not placed.
+	if _, err := open.inventory.SecretForModule(ctx, "laptop", "assistant", "vendor-key"); err != nil {
+		t.Fatal(err)
+	}
+	if got := placedAt(sentTo(t, open, "laptop"), "/var/lib/mesh/assistant/vendor-key"); got != nil {
+		t.Fatalf("a value the mesh made for a secret issued outside was placed: %v", got)
+	}
+
+	// Given, it is placed.
+	if err := open.inventory.AcceptSecretForModule(ctx, "laptop", "assistant", "vendor-key", "the vendor's key"); err != nil {
+		t.Fatal(err)
+	}
+	given, ok, err := open.inventory.GivenOwnSecret(ctx, "laptop", "assistant", "vendor-key")
+	if err != nil || !ok || !given.Current {
+		t.Fatalf("the value given is not read as given: %+v %v %v", given, ok, err)
+	}
+	placed := placedAt(sentTo(t, open, "laptop"), "/var/lib/mesh/assistant/vendor-key")
+	if placed == nil || placed["sealed"] != given.Sealed {
+		t.Fatalf("the value given is placed as %v", placed)
+	}
+}
+
+// The read paths answer as the send does: a question about the machine composes without the secret nobody gave, and
+// asking writes nothing.
+func TestAQuestionComposesWithoutAnOutsideSecretNobodyGave(t *testing.T) {
+	open := aMesh(t)
+	ctx := t.Context()
+	register(t, open, anAssistant())
+	if _, err := assign(ctx, open, "laptop", "assistant"); err != nil {
+		t.Fatal(err)
+	}
+	pushedBy(t, open, "laptop") // the mesh's own secret is made by the send
+	plan, settings, err := planFor(ctx, open, "laptop")
+	if err != nil {
+		t.Fatal(err)
+	}
+	gens, err := generators(ctx, open)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, choosing := range []Choosing{Reading, Foreseeing} {
+		got, err := declarationWith(ctx, open, "laptop", plan, settings, gens, choosing)
+		if err != nil {
+			t.Fatalf("%v: %v", choosing, err)
+		}
+		if placedAt(got.Resources, "/var/lib/mesh/assistant/vendor-key") != nil || len(got.foreseen) != 0 {
+			t.Fatalf("%v: a secret nobody gave is composed (foreseen %v)", choosing, got.foreseen)
+		}
+	}
+	if _, held, err := open.inventory.ModuleSecretIfIssued(ctx, "laptop", "assistant", "vendor-key"); err != nil || held {
+		t.Fatalf("a question made a value (held %v, %v)", held, err)
+	}
+}

cmd/mesh-controller/plan.go

@@ -590,6 +590,13 @@ func busCredentialIssued(ctx context.Context, inv *inventory.Inventory, node, mo
 	return nil
 }
 
+// givenUnderAnOldKey is the refusal for a value a person gave — a family's member, or an own secret issued outside
+// the mesh — sealed to a key the machine no longer holds: the mesh cannot make another, so it is given again.
+func givenUnderAnOldKey(module, node, name string) error {
+	return fmt.Errorf("%s on %s holds %q, which was given to the mesh rather than made by it, and %s has "+
+		"since generated a new sealing key. The mesh cannot make another; give it again", module, node, name, node)
+}
+
 // renderingFor is everything a node's declaration is composed with, and the node's record.
 func renderingFor(ctx context.Context, open *stores, node string,
 	plan catalogue.Resolution, settings catalogue.SettingsBy,
@@ -708,10 +715,7 @@ func renderingFor(ctx context.Context, open *stores, node string,
 				}
 				if !g.Current {
 					if choosing == Allocating {
-						return catalogue.Rendering{}, inventory.Node{}, fmt.Errorf(
+						return catalogue.Rendering{}, inventory.Node{}, givenUnderAnOldKey(m.Module, node, g.Name)
-							"%s on %s holds %q, which was given to the mesh rather than made by it, and %s has "+
-								"since generated a new sealing key. The mesh cannot make another; give it again",
-							m.Module, node, g.Name, node)
 					}
 					continue
 				}
@@ -721,7 +725,31 @@ func renderingFor(ctx context.Context, open *stores, node string,
 				needed[m.Module][g.Name] = g.Sealed
 			}
 		}
-		for name := range m.OwnSecrets.Plain() {
+		for name, own := range m.OwnSecrets.Plain() {
+			// **An own secret issued outside the mesh is never made** (novox/hq issue 401, ADR 0228; ADR 0283 for
+			// a family): only a value a person gave is placed. One not given is nothing — no file, and an empty
+			// value where a file of the module names it — so the module can tell it was not given, where a value
+			// the mesh made looked given and wrong. A value the mesh made for it before is not one either.
+			if own.IssuedBy == catalogue.IssuedOutside {
+				g, given, err := inv.GivenOwnSecret(ctx, node, m.Module, name)
+				if err != nil {
+					return catalogue.Rendering{}, inventory.Node{}, err
+				}
+				if !given {
+					continue
+				}
+				if !g.Current {
+					if choosing == Allocating {
+						return catalogue.Rendering{}, inventory.Node{}, givenUnderAnOldKey(m.Module, node, name)
+					}
+					continue
+				}
+				if needed[m.Module] == nil {
+					needed[m.Module] = map[string]string{}
+				}
+				needed[m.Module][name] = g.Sealed
+				continue
+			}
 			// Minted on the send path and only read on every other. Making one is an insert, and
 			// a question that writes is a question that can block against the machine it is about.
 			var sealed string

internal/catalogue/declaration.go

@@ -645,6 +645,11 @@ func (r Resolution) compose(with Rendering, owner map[string]string,
 				sealed = ForeseenSealed
 				*foreseen = append(*foreseen, m.Module+"/"+name)
 			}
+			if sealed == "" && m.OwnSecrets[name].IssuedBy == IssuedOutside {
+				// **Not given, and the mesh never makes it** (novox/hq issue 401, ADR 0228): no file, as for a
+				// family's member (ADR 0283), so the module reads its absence as not given.
+				continue
+			}
 			if sealed == "" {
 				// Declared and not made. Refused rather than skipped: a module whose own
 				// credential is silently absent starts, fails to authenticate, and the reason is
@@ -1002,7 +1007,7 @@ func (r Resolution) compose(with Rendering, owner map[string]string,
 			// content is where a setting lands, so a placeholder may only exist once the setting
 			// has been put in — filling secrets first would look at content that is not yet what
 			// the machine receives.
-			if err := intoFile(copied, sealed, m.Module); err != nil {
+			if err := intoFile(copied, sealed, outsideNotGiven(m, with.Needed[m.Module]), m.Module); err != nil {
 				return nil, err
 			}
 			// **Substituted here, not on the machine.** A bound value is not secret — the mesh
@@ -2431,6 +2436,9 @@ func secretsReadBy(resource map[string]any, m Manifest, given map[string]string)
 	}
 	paths := map[string]string{}
 	for name, own := range m.OwnSecrets.Plain() {
+		if own.IssuedBy == IssuedOutside && given[name] == "" {
+			continue // not given, so not placed: nothing to restart on (novox/hq issue 401)
+		}
 		paths[name] = own.Path
 	}
 	for name, sealed := range given {

internal/catalogue/outside_not_given_test.go

@@ -0,0 +1,106 @@
+package catalogue
+
+import (
+	"strings"
+	"testing"
+)
+
+// An own secret issued outside the mesh that nobody gave (novox/hq issue 401, ADR 0228): the mesh makes no value for it,
+// so the composition places no file, a file of the module that names it says empty there, and nothing restarts on it.
+// Given, it is placed and filled like any own secret.
+
+func outsideManifest(t *testing.T) Manifest {
+	t.Helper()
+	m, err := ParseManifest([]byte(`{"module":"assistant","version":"1",
+	  "own-secrets":{
+	    "password":{"path":"/var/lib/assistant/password.secret","taken":"at-start"},
+	    "vendor-key":{"path":"/var/lib/assistant/vendor-key.secret","taken":"at-start","issued-by":"outside"}},
+	  "resources":[
+	    {"id":"env","type":"file","path":"/var/lib/assistant/env","mode":"0600",
+	     "content":"PASSWORD=${secret:password}\nVENDOR_KEY=${secret:vendor-key}\n"},
+	    {"id":"server","type":"container","name":"assistant","image":"assistant:1",
+	     "env-file":["/var/lib/assistant/env"],
+	     "volumes":["/var/lib/assistant/vendor-key.secret:/run/vendor-key:ro"],
+	     "secrets-in-environment":"reads its settings from the environment only"}]}`))
+	if err != nil {
+		t.Fatal(err)
+	}
+	return m
+}
+
+func resourceAt(out []map[string]any, path string) map[string]any {
+	for _, r := range out {
+		if r["path"] == path {
+			return r
+		}
+	}
+	return nil
+}
+
+func resourceNamed(out []map[string]any, name string) map[string]any {
+	for _, r := range out {
+		if r["name"] == name {
+			return r
+		}
+	}
+	return nil
+}
+
+func TestAnOutsideSecretNotGivenIsNoFileAndAFileNamingItSaysEmpty(t *testing.T) {
+	r := Resolution{Node: "server", Modules: []Manifest{outsideManifest(t)}}
+	out, err := r.Declaration(Rendering{Needed: map[string]map[string]string{"assistant": {"password": "sealed-password"}}})
+	if err != nil {
+		t.Fatalf("a module whose outside secret nobody gave did not compose: %v", err)
+	}
+	if got := resourceAt(out, "/var/lib/assistant/vendor-key.secret"); got != nil {
+		t.Fatalf("a secret nobody gave was placed: %v", got)
+	}
+	env := resourceAt(out, "/var/lib/assistant/env")
+	if env == nil {
+		t.Fatal("the module's own file is not composed")
+	}
+	if content := env["content"].(string); content != "PASSWORD=${secret:password}\nVENDOR_KEY=\n" {
+		t.Fatalf("the file names the secret nobody gave as %q", content)
+	}
+	secrets, _ := env["secrets"].(map[string]any)
+	if len(secrets) != 1 || secrets["password"] != "sealed-password" {
+		t.Fatalf("the file is filled from %v", secrets)
+	}
+	server := resourceNamed(out, "assistant")
+	if on := strings.Join(stringsIn(server["restart-on"]), " "); strings.Contains(on, "vendor-key") {
+		t.Fatalf("the container restarts on a secret nobody gave: %q", on)
+	}
+}
+
+func TestAnOutsideSecretGivenIsPlacedAndFilled(t *testing.T) {
+	r := Resolution{Node: "server", Modules: []Manifest{outsideManifest(t)}}
+	out, err := r.Declaration(Rendering{Needed: map[string]map[string]string{"assistant": {
+		"password": "sealed-password", "vendor-key": "sealed-vendor-key"}}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	placed := resourceAt(out, "/var/lib/assistant/vendor-key.secret")
+	if placed == nil || placed["sealed"] != "sealed-vendor-key" {
+		t.Fatalf("the secret given is placed as %v", placed)
+	}
+	env := resourceAt(out, "/var/lib/assistant/env")
+	if content := env["content"].(string); content != "PASSWORD=${secret:password}\nVENDOR_KEY=${secret:vendor-key}\n" {
+		t.Fatalf("the file was changed: %q", content)
+	}
+	if secrets, _ := env["secrets"].(map[string]any); secrets["vendor-key"] != "sealed-vendor-key" {
+		t.Fatalf("the file is filled from %v", secrets)
+	}
+	server := resourceNamed(out, "assistant")
+	if on := strings.Join(stringsIn(server["restart-on"]), " "); !strings.Contains(on, "assistant."+NeedID("vendor-key")) {
+		t.Fatalf("the container does not restart on the secret given: %q", on)
+	}
+}
+
+// A secret the mesh makes is still refused when it was not made: only an outside one is left out.
+func TestAnOwnSecretTheMeshMakesIsStillRefusedWhenNotMade(t *testing.T) {
+	r := Resolution{Node: "server", Modules: []Manifest{outsideManifest(t)}}
+	_, err := r.Declaration(Rendering{})
+	if nm, ok := err.(*NotMadeError); !ok || nm.Name != "password" {
+		t.Fatalf("a secret the mesh makes, not made: %v", err)
+	}
+}

internal/catalogue/secrets_into_files.go

@@ -156,7 +156,11 @@ func secretFilesOf(resources []map[string]any) map[string]bool {
 // A name the module never declared is refused here rather than on the machine. The host would
 // refuse it too — but it would do so having already been handed a declaration, which reads as the
 // mesh sending something broken, and the name it could not find is a manifest's typo.
-func intoFile(resource map[string]any, sealed map[string]string, module string) error {
+//
+// **An own secret issued outside the mesh that nobody gave is filled with nothing** (novox/hq issue 401): the mesh
+// never makes one (ADR 0228), and a file is where the module reads it, so the place says empty — the module sees it
+// was not given, where a value the mesh made looked given and wrong. notGiven names those secrets.
+func intoFile(resource map[string]any, sealed map[string]string, notGiven map[string]bool, module string) error {
 	if fmt.Sprint(resource["type"]) != "file" {
 		return nil
 	}
@@ -171,6 +175,10 @@ func intoFile(resource map[string]any, sealed map[string]string, module string)
 	into := map[string]any{}
 	for _, name := range used {
 		value := sealed[name]
+		if value == "" && notGiven[name] {
+			content = strings.ReplaceAll(content, "${secret:"+name+"}", "")
+			continue
+		}
 		if value == "" {
 			return fmt.Errorf(
 				"%s has a file that says ${secret:%s}, and %s has no secret of its own by that "+
@@ -179,10 +187,25 @@ func intoFile(resource map[string]any, sealed map[string]string, module string)
 		}
 		into[name] = value
 	}
+	resource["content"] = content
+	if len(into) > 0 {
 		resource["secrets"] = into
+	}
 	return nil
 }
 
+// outsideNotGiven is each own secret of a module issued outside the mesh that this composition holds no value for
+// (novox/hq issue 401): what intoFile fills with nothing, and what nothing restarts on.
+func outsideNotGiven(m Manifest, given map[string]string) map[string]bool {
+	out := map[string]bool{}
+	for name, own := range m.OwnSecrets.Plain() {
+		if own.IssuedBy == IssuedOutside && given[name] == "" {
+			out[name] = true
+		}
+	}
+	return out
+}
+
 // namesOr says what a module could have written, because the answer to "that name is wrong" is
 // almost always one of two or three right ones.
 func namesOr(sealed map[string]string) string {

internal/inventory/secrets.go

@@ -325,6 +325,8 @@ func (i *Inventory) ModuleSecretIfIssued(
 	return sealed, true, nil
 }
 
+// SecretForModule makes what is not held, so it is never asked for an own secret issued outside the mesh: the
+// composition reads only a value a person gave for one (GivenOwnSecret; novox/hq issue 401, ADR 0228).
 func (i *Inventory) SecretForModule(ctx context.Context, node, module, name string) (string, error) {
 	key, err := i.SealingKeyOf(ctx, node)
 	if err != nil {
@@ -655,6 +657,35 @@ func (i *Inventory) GivenMembers(ctx context.Context, node, module, family strin
 	return out, rows.Err()
 }
 
+// GivenOwnSecret is an own secret issued outside the mesh as a person gave it on a machine (novox/hq issue 401, ADR
+// 0228): its value sealed to the machine and whether that is the key the machine holds now. Not given is false: a
+// value the mesh made for it is not one a person gave, and is never read as one; a machine with no sealing key
+// holds none either.
+func (i *Inventory) GivenOwnSecret(ctx context.Context, node, module, name string) (GivenMember, bool, error) {
+	key, err := i.SealingKeyOf(ctx, node)
+	if err != nil || key == "" {
+		return GivenMember{}, false, err
+	}
+	record, err := i.NodeByName(ctx, node)
+	if err != nil {
+		return GivenMember{}, false, err
+	}
+	g := GivenMember{Name: name}
+	var against string
+	err = i.store.Pool().QueryRow(ctx,
+		`select sealed, node_key from module_secret
+		  where node = $1 and module = $2 and name = $3 and origin = 'accepted'`,
+		record.ID, module, name).Scan(&g.Sealed, &against)
+	if errors.Is(err, pgx.ErrNoRows) {
+		return GivenMember{}, false, nil
+	}
+	if err != nil {
+		return GivenMember{}, false, err
+	}
+	g.Current = against == key
+	return g, true, nil
+}
+
 // GivenOwnSecrets is every own secret of a module a person gave on a machine, by name, with when (novox/hq ADR
 // 0283): what the controller checks a module's wait for a secret against. A value the mesh made is not one.
 func (i *Inventory) GivenOwnSecrets(ctx context.Context, node, module string) (map[string]time.Time, error) {

testdata/store-tests

@@ -27,3 +27,5 @@
 ./cmd/mesh-controller TestAnUnassignThatLeavesItsNodeResolvedSaysNothingOfIt
 ./cmd/mesh-controller TestAnAssignThatLeavesItsNodeUnresolvedSaysOnlyThePlansRefusal
 ./cmd/mesh-controller TestADirectoryOwnersKeyIsRefusedThroughAVerb
+./cmd/mesh-controller TestTheSendMakesNoValueForAnOutsideSecretNobodyGave
+./cmd/mesh-controller TestAQuestionComposesWithoutAnOutsideSecretNobodyGave