From 35c5c2bb9b9d87b2729a27462ee928bb478599d5 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 23:31:06 +0200 Subject: [PATCH] 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 | 15 ++++-- internal/inventory/operator_test.go | 3 +- internal/inventory/secrets.go | 72 +++++++++++++++++++++++++++++ internal/inventory/secrets_test.go | 69 ++++++++++++++++++++++++--- 4 files changed, 148 insertions(+), 11 deletions(-) diff --git a/internal/inventory/forget_test.go b/internal/inventory/forget_test.go index a8a8ca3..5c675a0 100644 --- a/internal/inventory/forget_test.go +++ b/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 +} diff --git a/internal/inventory/operator_test.go b/internal/inventory/operator_test.go index 9cfe052..dd328ae 100644 --- a/internal/inventory/operator_test.go +++ b/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) } diff --git a/internal/inventory/secrets.go b/internal/inventory/secrets.go index 1364e13..235e929 100644 --- a/internal/inventory/secrets.go +++ b/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, ", ") +} diff --git a/internal/inventory/secrets_test.go b/internal/inventory/secrets_test.go index 671f381..ac4942c 100644 --- a/internal/inventory/secrets_test.go +++ b/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"}, - Source{}); err != nil { + // Each requires what a test delivers to it: a pair credential is refused for a + // 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"}, - Source{}); err != nil { + if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "builder", Version: "1", + 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"}, - Source{}); err != nil { + if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "builder", Version: "1", + 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) + } +}