secret accept is refused for a name the module does not declare, and a pair delivery for a requirement or local it has not got (novox/hq issue 078)

internal/inventory/forget_test.go

@@ -32,7 +32,7 @@ func TestRegisteringAModuleAgainKeepsWhatTheMeshHoldsForIt(t *testing.T) {
 		t.Fatal(err)
 	}
 	m := catalogue.Manifest{Module: "step-ca", Version: "1",
-		Provides: catalogue.Offers("acme-ca")}
+		Provides: catalogue.Offers("acme-ca"), OwnSecrets: map[string]string{"password": "/run/password"}}
 	if err := inv.RegisterModule(ctx, m, Source{}); err != nil {
 		t.Fatal(err)
 	}
@@ -101,7 +101,7 @@ func TestForgettingAModuleRefusesRatherThanDiscardingWhatTheMeshHolds(t *testing
 	if err := inv.RecordSealingKey(ctx, node.ID, key); err != nil {
 		t.Fatal(err)
 	}
-	if err := inv.RegisterModule(ctx, manifest("step-ca", nil, nil), Source{}); err != nil {
+	if err := inv.RegisterModule(ctx, withOwnSecret(manifest("step-ca", nil, nil), "password"), Source{}); err != nil {
 		t.Fatal(err)
 	}
 	if err := inv.SetSettings(ctx, "anchor", "step-ca", map[string]any{"port": 9000}); err != nil {
@@ -156,7 +156,7 @@ func TestDiscardingAModuleSaysWhatWentWithIt(t *testing.T) {
 	if err := inv.RecordSealingKey(ctx, node.ID, key); err != nil {
 		t.Fatal(err)
 	}
-	if err := inv.RegisterModule(ctx, manifest("step-ca", nil, nil), Source{}); err != nil {
+	if err := inv.RegisterModule(ctx, withOwnSecret(manifest("step-ca", nil, nil), "password"), Source{}); err != nil {
 		t.Fatal(err)
 	}
 	if err := inv.SetSettings(ctx, "", "step-ca", map[string]any{"issuer": "the mesh"}); err != nil {
@@ -206,7 +206,7 @@ func TestAModuleStillAssignedRefusesBeforeAnythingAboutWhatItHolds(t *testing.T)
 	if _, err := inv.AddNode(ctx, "anchor"); err != nil {
 		t.Fatal(err)
 	}
-	if err := inv.RegisterModule(ctx, manifest("step-ca", nil, nil), Source{}); err != nil {
+	if err := inv.RegisterModule(ctx, withOwnSecret(manifest("step-ca", nil, nil), "password"), Source{}); err != nil {
 		t.Fatal(err)
 	}
 	if err := inv.SetSettings(ctx, "anchor", "step-ca", map[string]any{"a": 1}); err != nil {
@@ -224,3 +224,10 @@ func TestAModuleStillAssignedRefusesBeforeAnythingAboutWhatItHolds(t *testing.T)
 		}
 	}
 }
+
+// withOwnSecret gives a fixture manifest an own secret, so a delivery to it is one the module
+// declares (novox/hq 04-ISSUES/078).
+func withOwnSecret(m catalogue.Manifest, name string) catalogue.Manifest {
+	m.OwnSecrets = map[string]string{name: "/run/" + name}
+	return m
+}

internal/inventory/operator_test.go

@@ -13,7 +13,8 @@ import (
 // opens exactly the value the node was given.
 func TestAnOwnSecretIsSealedToTheOperatorToo(t *testing.T) {
 	inv, ctx := twoNodesWithKeys(t)
-	if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "postgres", Version: "1"}, Source{}); err != nil {
+	if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "postgres", Version: "1",
+		OwnSecrets: map[string]string{"superuser": "/run/superuser", "replication": "/run/replication"}}, Source{}); err != nil {
 		t.Fatal(err)
 	}
 

internal/inventory/secrets.go

@@ -4,9 +4,13 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"slices"
+	"sort"
+	"strings"
 
 	"github.com/jackc/pgx/v5"
 
+	"github.com/novox/mesh-controller/internal/catalogue"
 	"github.com/novox/mesh-controller/internal/secrets"
 )
 
@@ -137,6 +141,27 @@ func (i *Inventory) SecretFor(ctx context.Context, name, consumer, consumerModul
 // what differs is that both ends of the pair are sealed to, and that the record says `accepted`
 // so a later read never replaces it with a minted one. The plaintext is discarded here.
 func (i *Inventory) AcceptSecretForPair(ctx context.Context, name, consumer, consumerModule, provider, local, value string) error {
+	// Refused for a requirement the module does not have, or a local it does not keep under it
+	// (novox/hq 04-ISSUES/078): the credential would sit in the pair unread.
+	m, err := i.declared(ctx, consumerModule)
+	if err != nil {
+		return err
+	}
+	if !slices.Contains(m.Requires, name) {
+		return fmt.Errorf("%s does not require %q; it requires: %s", consumerModule, name, orNone(m.Requires))
+	}
+	if locals := m.SecretsMany[name]; len(locals) > 0 {
+		if local == "" {
+			return fmt.Errorf("%s keeps several secrets for %q; name one with --local: %s",
+				consumerModule, name, orNone(sortedNames(locals)))
+		}
+		if _, kept := locals[local]; !kept {
+			return fmt.Errorf("%s does not keep %q for %q; it keeps: %s",
+				consumerModule, local, name, orNone(sortedNames(locals)))
+		}
+	} else if local != "" {
+		return fmt.Errorf("%s keeps one secret for %q, not several; drop --local", consumerModule, name)
+	}
 	consumerKey, err := i.SealingKeyOf(ctx, consumer)
 	if err != nil {
 		return err
@@ -362,6 +387,16 @@ func (i *Inventory) SecretForModule(ctx context.Context, node, module, name stri
 // Sealed on the way in and the plaintext discarded, exactly as a generated one is — so the only
 // difference between the two is where the value came from.
 func (i *Inventory) AcceptSecretForModule(ctx context.Context, node, module, name, value string) error {
+	// Refused for a name the module does not declare. A value stored under a name nothing reads
+	// is a delivery that changed nothing and reported success — the shape of failure the mesh
+	// is built to refuse (novox/hq 04-ISSUES/078).
+	m, err := i.declared(ctx, module)
+	if err != nil {
+		return err
+	}
+	if _, own := m.OwnSecrets[name]; !own {
+		return fmt.Errorf("%s does not declare %q as an own secret; %s", module, name, declaresOwn(m))
+	}
 	key, err := i.SealingKeyOf(ctx, node)
 	if err != nil {
 		return err
@@ -448,3 +483,40 @@ func localFlag(local string) string {
 	}
 	return " --local " + local
 }
+
+// declared is the manifest the mesh holds for a module — what a delivered value is checked
+// against, so a delivery for a name the module does not have is refused rather than stored.
+func (i *Inventory) declared(ctx context.Context, module string) (catalogue.Manifest, error) {
+	known, err := i.Catalogue(ctx)
+	if err != nil {
+		return catalogue.Manifest{}, err
+	}
+	m, ok := known[module]
+	if !ok {
+		return catalogue.Manifest{}, fmt.Errorf("%s is not a module the mesh knows; `module add` it first", module)
+	}
+	return m, nil
+}
+
+func declaresOwn(m catalogue.Manifest) string {
+	if len(m.OwnSecrets) == 0 {
+		return "it declares no own secrets"
+	}
+	return "it declares: " + strings.Join(sortedNames(m.OwnSecrets), ", ")
+}
+
+func sortedNames(of map[string]string) []string {
+	names := make([]string, 0, len(of))
+	for name := range of {
+		names = append(names, name)
+	}
+	sort.Strings(names)
+	return names
+}
+
+func orNone(names []string) string {
+	if len(names) == 0 {
+		return "none"
+	}
+	return strings.Join(names, ", ")
+}

internal/inventory/secrets_test.go

@@ -52,8 +52,10 @@ func twoNodesWithKeys(t *testing.T) (*Inventory, context.Context) {
 	// before it can (novox/hq 04-ISSUES/022). Two of them, because "two consumers on one node"
 	// is the case that key exists for.
 	for _, m := range []string{"gitea", "keycloak"} {
-		if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: m, Version: "1"},
+		// Each requires what a test delivers to it: a pair credential is refused for a
-			Source{}); err != nil {
+		// requirement the module does not have (novox/hq 04-ISSUES/078).
+		if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: m, Version: "1",
+			Requires: []string{"secret", "postgres-database"}}, Source{}); err != nil {
 			t.Fatal(err)
 		}
 	}
@@ -404,8 +406,8 @@ func TestACredentialGoesWhenEitherMachineDoes(t *testing.T) {
 // secret was delivered — which it was.
 func TestASecretTheMeshWasGivenIsNotReinventedWhenTheMachineRejoins(t *testing.T) {
 	inv, ctx := twoNodesWithKeys(t)
-	if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "builder", Version: "1"},
+	if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "builder", Version: "1",
-		Source{}); err != nil {
+		OwnSecrets: map[string]string{"broker": "/run/broker"}}, Source{}); err != nil {
 		t.Fatal(err)
 	}
 	const url = "amqps://builder:the-password-the-broker-was-told@broker/"
@@ -436,8 +438,8 @@ func TestASecretTheMeshWasGivenIsNotReinventedWhenTheMachineRejoins(t *testing.T
 // that would make the refusal above useless if it were wrong.
 func TestASecretTheMeshWasGivenSurvivesAnOrdinaryPush(t *testing.T) {
 	inv, ctx := twoNodesWithKeys(t)
-	if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "builder", Version: "1"},
+	if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "builder", Version: "1",
-		Source{}); err != nil {
+		OwnSecrets: map[string]string{"broker": "/run/broker"}}, Source{}); err != nil {
 		t.Fatal(err)
 	}
 	if err := inv.AcceptSecretForModule(ctx, "consumer", "builder", "broker",
@@ -715,3 +717,58 @@ func TestTheOperatorRecoversEachLocalNameApart(t *testing.T) {
 		t.Fatalf("the export does not name the local names: %v", kept)
 	}
 }
+
+// **A delivered secret is accepted only under a name the module declares** (novox/hq
+// 04-ISSUES/078). A value stored under a name nothing reads is a delivery that changed nothing
+// and reported success; refused, naming what the module does declare.
+func TestADeliveredSecretIsRefusedUnderANameTheModuleDoesNotDeclare(t *testing.T) {
+	inv, ctx := twoNodesWithKeys(t)
+	if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "step-ca", Version: "1",
+		OwnSecrets: map[string]string{"password": "/run/password"}}, Source{}); err != nil {
+		t.Fatal(err)
+	}
+	err := inv.AcceptSecretForModule(ctx, "consumer", "step-ca", "root-key", "not-a-key")
+	if err == nil {
+		t.Fatal("a secret delivered under a name the module does not declare was accepted")
+	}
+	for _, want := range []string{"root-key", "password"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("the refusal does not say %q: %v", want, err)
+		}
+	}
+	if err := inv.AcceptSecretForModule(ctx, "consumer", "step-ca", "password", "hunter2"); err != nil {
+		t.Fatalf("a secret delivered under a declared name was refused: %v", err)
+	}
+	// A module the mesh does not know is said, not stored.
+	err = inv.AcceptSecretForModule(ctx, "consumer", "nobody", "password", "hunter2")
+	if err == nil || !strings.Contains(err.Error(), "module add") {
+		t.Errorf("a delivery to an unknown module was not refused with the remedy: %v", err)
+	}
+}
+
+func TestADeliveredPairCredentialIsRefusedForARequirementTheModuleDoesNotHave(t *testing.T) {
+	inv, ctx := twoNodesWithKeys(t)
+	// gitea requires secret and postgres-database (the fixture); not an object store.
+	err := inv.AcceptSecretForPair(ctx, "object-store", "consumer", "gitea", "provider", "", "hunter2")
+	if err == nil || !strings.Contains(err.Error(), "postgres-database") {
+		t.Fatalf("a pair credential for a requirement the module does not have was not refused naming what it requires: %v", err)
+	}
+	// A module keeping several secrets for one requirement (ADR 0094) takes a delivery only
+	// under one of its locals.
+	m := catalogue.Manifest{Module: "mailu", Version: "1", Requires: []string{"secret"},
+		SecretsMany: map[string]map[string]string{"secret": {"admin": "/run/admin", "api-token": "/run/api-token"}}}
+	if err := inv.RegisterModule(ctx, m, Source{}); err != nil {
+		t.Fatal(err)
+	}
+	err = inv.AcceptSecretForPair(ctx, "secret", "consumer", "mailu", "provider", "", "hunter2")
+	if err == nil || !strings.Contains(err.Error(), "--local") {
+		t.Errorf("a delivery to a module keeping several secrets, with no local named, was not refused: %v", err)
+	}
+	err = inv.AcceptSecretForPair(ctx, "secret", "consumer", "mailu", "provider", "secret-key", "hunter2")
+	if err == nil || !strings.Contains(err.Error(), "api-token") {
+		t.Errorf("a delivery under a local the module does not keep was not refused naming the ones it keeps: %v", err)
+	}
+	if err := inv.AcceptSecretForPair(ctx, "secret", "consumer", "mailu", "provider", "admin", "hunter2"); err != nil {
+		t.Errorf("a delivery under a kept local was refused: %v", err)
+	}
+}