From 99a753994e9eab91086e0160bc55605e5a223e51 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 6 Sep 2026 13:46:55 +0200 Subject: [PATCH] fix: fill a ptr-secret placeholder from the file-owner's credential, not the last consumer's The provider-seal-key gate: on a node with two modules requiring the same provision (baserow and letta both consuming postgres), sealedFor matched a need by provision NAME alone, so a file's ${secret:X} placeholder took whichever consumer's sealed credential came last in r.Needs -- the OTHER module's password. baserow was handed letta's password and could not authenticate. The secrets:-map delivery path already guards this (For == m.Module, novox/hq 04-ISSUES/022); the ${secret:...} placeholder path did not. Added the same guard. Also dedups the contributions file: when provider and consumer are co-located, grantsFor enumerates the same-node consumer, so a consumer was emitted twice into the provider's receives file (once full with its grant, once partial). The m.Contributes loop now skips a (provision, module) the grants loop already carried; non-grant contributions (routes) still emit. Regression test added: two consumers of one provision each get their own credential. Proven end-to-end on a two-node lab install (mesh-lab assigned-two-node-db): baserow and letta on one node, substrate on another, each authenticates with its own minted password. Claude-Session: https://claude.ai/code/session_01LrgweAeERJYBg88c5cKDzF --- internal/catalogue/declaration.go | 16 +++++++ internal/catalogue/secrets_into_files.go | 6 ++- internal/catalogue/secrets_into_files_test.go | 48 ++++++++++++++++++- 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index 2806531..2274829 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -496,6 +496,11 @@ func (r Resolution) contributions(settings SettingsBy, grants []Grant, } return sorted[i].From < sorted[j].From }) + // A consumer already carried by the grants loop, keyed (provision, module). When provider and + // consumer are co-located, `grantsFor` enumerates the same-node consumer too, so without this the + // module would be emitted a second time by the m.Contributes loop below — once full (with the + // grant's secret/as) and once partial — which is the duplicate seen in a co-located mesh.json. + granted := map[string]map[string]bool{} for _, g := range sorted { if g.From == "" { // As above: nothing on that machine asks for this any more, so the provider is not @@ -507,9 +512,20 @@ func (r Resolution) contributions(settings SettingsBy, grants []Grant, As: ConsumerIdentity(g.Consumer, IdentitySource(g.Slug, g.From)), Secret: grantPath(directories[g.Provision], g.Consumer, g.From), }) + if granted[g.Provision] == nil { + granted[g.Provision] = map[string]bool{} + } + granted[g.Provision][g.From] = true } for _, m := range modules { for _, to := range sortedKeys(m.Contributes) { + // The grants loop already emitted this module's contribution to this provision, with its + // minted secret — emitting the partial copy again would duplicate it. A contribution with + // no grant (a reverse-proxy route names a host, not a credential) is not in `granted`, so + // it still reaches the provider from here. + if granted[to][m.Module] { + continue + } // Settings reach a contribution the same way they reach a file. A route's hostname is // exactly the kind of thing that differs between one mesh and the next, and a module // that could not have it set would have to be edited to be reused. diff --git a/internal/catalogue/secrets_into_files.go b/internal/catalogue/secrets_into_files.go index b77580a..07ae2c0 100644 --- a/internal/catalogue/secrets_into_files.go +++ b/internal/catalogue/secrets_into_files.go @@ -71,7 +71,11 @@ func sealedFor(m Manifest, needs []Needed, with Rendering) (map[string]string, e "${secret:%s} could mean either — rename one of them", m.Module, to, to, to) } for i := range needs { - if needs[i].Name == to && needs[i].Sealed != "" { + // `For == m.Module`, not name alone: on a node with two modules requiring the same + // provision, both appear in `needs`, and matching by name would fill ${secret:X} with + // whichever came last — the other module's credential (novox/hq 04-ISSUES/022). The + // `secrets:`-map path already guards this way; the ${secret:…} placeholder path did not. + if needs[i].Name == to && needs[i].For == m.Module && needs[i].Sealed != "" { sealed[to] = needs[i].Sealed } } diff --git a/internal/catalogue/secrets_into_files_test.go b/internal/catalogue/secrets_into_files_test.go index a16ac0a..99352a9 100644 --- a/internal/catalogue/secrets_into_files_test.go +++ b/internal/catalogue/secrets_into_files_test.go @@ -67,7 +67,7 @@ func TestAGrantedCredentialCanBeShapedIntoAnEnvFile(t *testing.T) { "mode": "0600", }}, }}, - Needs: []Needed{{Name: "postgres-database", From: "anchor", Sealed: "sealed-db"}}, + Needs: []Needed{{Name: "postgres-database", For: "keycloak", From: "anchor", Sealed: "sealed-db"}}, } out, err := r.Declaration(Rendering{}) if err != nil { @@ -80,6 +80,52 @@ func TestAGrantedCredentialCanBeShapedIntoAnEnvFile(t *testing.T) { } } +// Two modules on one node requiring the same provision each get THEIR OWN credential in a +// ${secret:…} file — not whichever need happened to come last. This is the placeholder-path twin of +// the guard novox/hq 04-ISSUES/022 put on the secrets:-map path; without it, a node with baserow and +// letta both consuming postgres gave baserow letta's password and authentication failed. +func TestTwoConsumersOfOneProvisionEachGetTheirOwnInAPlaceholderFile(t *testing.T) { + r := Resolution{ + Node: "anchor", + Modules: []Manifest{ + { + Module: "keycloak", + Requires: []string{"postgres-database"}, + Secrets: map[string]string{"postgres-database": "/var/lib/keycloak/db.secret"}, + Resources: []map[string]any{{ + "id": "dbenv", "type": "file", "path": "/var/lib/keycloak/database.env", + "content": "PW=${secret:postgres-database}\n", "mode": "0600", + }}, + }, + { + Module: "grafana", + Requires: []string{"postgres-database"}, + Secrets: map[string]string{"postgres-database": "/var/lib/grafana/db.secret"}, + Resources: []map[string]any{{ + "id": "dbenv", "type": "file", "path": "/var/lib/grafana/database.env", + "content": "PW=${secret:postgres-database}\n", "mode": "0600", + }}, + }, + }, + Needs: []Needed{ + {Name: "postgres-database", For: "keycloak", From: "anchor", Sealed: "sealed-kc"}, + {Name: "postgres-database", For: "grafana", From: "anchor", Sealed: "sealed-gf"}, + }, + } + out, err := r.Declaration(Rendering{}) + if err != nil { + t.Fatal(err) + } + kc, _ := fileNamed(out, "keycloak.dbenv")["secrets"].(map[string]any) + if kc["postgres-database"] != "sealed-kc" { + t.Errorf("keycloak got the wrong credential (someone else's password): %v", kc) + } + gf, _ := fileNamed(out, "grafana.dbenv")["secrets"].(map[string]any) + if gf["postgres-database"] != "sealed-gf" { + t.Errorf("grafana got the wrong credential (someone else's password): %v", gf) + } +} + // A name nothing declared is a typo, and it is refused here rather than on the machine. func TestAFileNamingASecretTheModuleDoesNotHaveIsRefused(t *testing.T) { r := Resolution{Node: "anchor", Modules: []Manifest{{