From b9c24a7b42cb2db5fe9fe32e8176b303c1cf2239 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 30 Oct 2026 18:36:38 +0100 Subject: [PATCH] Read the vault's structured move-in refusal and take a value it holds another of as superseded, so the move is not asked again for ever (issue 774) mesh-vault (mesh-catalog f3b13de2) answers a move it refuses with `refused: {reason, held}` and `why`, and no read-back. The engine read only the read-back, so every such row failed "answered with no read-back" and the controller asked it again on every pass. Quick fix: treat any answer without a read-back as done. Thorough fix (chosen): read `refused`. For holds-another-value the vault's value is the live one: the move answers done, superseded, at the held generation with the held record in its words, so the controller records the row and stops asking; the journal says it once per secret. Any other reason fails in the vault's own words. --- cmd/mesh-host/movesecret.go | 55 +++++++++++++++++++++++++++++++- cmd/mesh-host/movesecret_test.go | 41 ++++++++++++++++++++---- internal/link/movesecret.go | 12 ++++++- internal/link/movesecret_test.go | 5 +-- 4 files changed, 102 insertions(+), 11 deletions(-) diff --git a/cmd/mesh-host/movesecret.go b/cmd/mesh-host/movesecret.go index da6c23c..0748460 100644 --- a/cmd/mesh-host/movesecret.go +++ b/cmd/mesh-host/movesecret.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "os" + "sync" "time" "golang.org/x/crypto/nacl/box" @@ -40,6 +41,11 @@ func moveSecretFrom(statePath string, queue *link.Session) link.MoveSecretFunc { if err != nil { fmt.Fprintf(os.Stderr, "mesh-host: a value was not moved into the vault: %v\n", err) } + if moved.Superseded != "" { + if _, said := supersededSaid.LoadOrStore(moved.Secret, true); !said { + fmt.Fprintf(os.Stderr, "mesh-host: %s\n", moved.Superseded) + } + } return moved, err } } @@ -104,8 +110,30 @@ func moveIntoTheVault(ask vaultAsk, key identity.SealingKey, m link.MoveSecret) Generation int `json:"generation"` Unchanged bool `json:"unchanged"` ReadBack string `json:"read_back"` + // Refused is the vault's refusal as fields (mesh-vault's MoveRefused, mesh-catalog f3b13de2): nothing was taken. + Refused *struct { + Reason string `json:"reason"` + Held *heldValue `json:"held"` + } `json:"refused"` + Why string `json:"why"` } - if err := json.Unmarshal(raw, &kept); err != nil || kept.Name == "" || kept.ReadBack == "" { + if err := json.Unmarshal(raw, &kept); err != nil { + return link.MovedSecret{}, fmt.Errorf("the vault's answer for %s does not read: %v", opened.Secret, err) + } + if r := kept.Refused; r != nil { + // The vault holds another value for the name, and the move never replaces one: the vault's value is the live + // one, and this machine's older value is superseded, never asked again (the sweep of 2026-10-30: it failed + // "answered with no read-back" on every ask). + if r.Reason == vaultHoldsAnother && r.Held != nil && r.Held.Name != "" { + return link.MovedSecret{Secret: r.Held.Name, Generation: r.Held.Generation, Superseded: r.Held.words()}, nil + } + why := kept.Why + if why == "" { + why = r.Reason + } + return link.MovedSecret{}, fmt.Errorf("the vault refused %s: %s", opened.Secret, why) + } + if kept.Name == "" || kept.ReadBack == "" { return link.MovedSecret{}, fmt.Errorf("the vault answered %s with no read-back", opened.Secret) } blob, err := base64.StdEncoding.DecodeString(kept.ReadBack) @@ -124,6 +152,31 @@ func moveIntoTheVault(ask vaultAsk, key identity.SealingKey, m link.MoveSecret) return link.MovedSecret{Secret: kept.Name, Generation: kept.Generation, Unchanged: kept.Unchanged}, nil } +// vaultHoldsAnother is the vault's reason when it holds another value for the name (mesh-vault RefusedHeldAnother). +const vaultHoldsAnother = "holds-another-value" + +// heldValue is the record of the value the vault holds instead, never the value (mesh-vault HeldAnother). +type heldValue struct { + Name string `json:"name"` + Generation int `json:"generation"` + ChangedBy string `json:"changed_by"` + ChangedAt time.Time `json:"changed_at"` + Origin string `json:"origin"` +} + +// words say the held value's record, for the journal and the controller. +func (h heldValue) words() string { + w := fmt.Sprintf("the vault holds another value for %s, generation %d, changed by %s at %s", h.Name, h.Generation, + h.ChangedBy, h.ChangedAt.UTC().Format(time.RFC3339)) + if h.Origin != "" { + w += ", origin " + h.Origin + } + return w + ": that value is the live one, and this machine's older value is superseded and not moved" +} + +// supersededSaid holds the names a superseded move was said of, so the journal says each once. +var supersededSaid sync.Map + func wipe(b []byte) { for i := range b { b[i] = 0 diff --git a/cmd/mesh-host/movesecret_test.go b/cmd/mesh-host/movesecret_test.go index e362c77..2da4d47 100644 --- a/cmd/mesh-host/movesecret_test.go +++ b/cmd/mesh-host/movesecret_test.go @@ -17,13 +17,15 @@ import ( // standInVault answers move-in's two steps as the vault does: a key made for the call, the value opened with it, // and the value kept read back sealed to the key the second step sent. lie makes its read-back another value. type standInVault struct { - t *testing.T - pub *[32]byte - priv *[32]byte - opened map[string]any - kept string - lie string - refuse string + t *testing.T + pub *[32]byte + priv *[32]byte + opened map[string]any + kept string + lie string + refuse string + // answer replaces the second step's answer: the vault's refusal as fields (mesh-catalog f3b13de2). + answer map[string]any carried []string } @@ -51,6 +53,9 @@ func (v *standInVault) ask(verb string, args any) (json.RawMessage, error) { v.t.Fatal("what the engine sealed does not open with the call's key") } v.kept = string(value) + if v.answer != nil { + return json.Marshal(v.answer) + } back := v.kept if v.lie != "" { back = v.lie @@ -119,6 +124,28 @@ func TestTheEngineSaysAMoveThatDidNotHold(t *testing.T) { !strings.Contains(err.Error(), "never replaces") { t.Fatalf("the vault's refusal: %v", err) } + // The vault's refusal as fields (mesh-catalog f3b13de2, no read-back): for another value held, the vault's is the + // live one and the move is done, superseded, at the held generation; any other reason fails in the vault's words. + held := map[string]any{"name": "own/anchor/postgres/superuser-password", "generation": 3, + "changed_by": "mesh-vault@novox", "changed_at": "2026-10-30T15:50:00Z", "origin": "made"} + for _, row := range []struct { + refused map[string]any + gen int + fails string + }{ + {map[string]any{"reason": "holds-another-value", "held": held}, 3, ""}, + {map[string]any{"reason": "locked"}, 0, "the vault refused"}, + } { + v := &standInVault{t: t, answer: map[string]any{"refused": row.refused, "why": "in words"}} + moved, err := moveIntoTheVault(v.ask, key, aMoveOf(t, key)) + switch { + case row.fails != "" && (err == nil || !strings.Contains(err.Error(), row.fails)): + t.Errorf("refused %v: %v", row.refused["reason"], err) + case row.fails == "" && (err != nil || moved.Generation != row.gen || moved.Secret != held["name"] || + !strings.Contains(moved.Superseded, "changed by mesh-vault@novox")): + t.Errorf("refused %v: moved %+v, %v", row.refused["reason"], moved, err) + } + } // A pair credential names both its ends. pair := aMoveOf(t, key) pair.Module, pair.Name = "", "" diff --git a/internal/link/movesecret.go b/internal/link/movesecret.go index 95463bb..4342796 100644 --- a/internal/link/movesecret.go +++ b/internal/link/movesecret.go @@ -54,6 +54,9 @@ type MoveSecretAnswer struct { Secret string `json:"secret,omitempty"` Generation int `json:"generation,omitempty"` Unchanged bool `json:"unchanged,omitempty"` + // Superseded is set when the vault holds another value for the name, which is the live one: the row is done and + // is not asked again; Generation is the held value's, and Said its record in words. + Superseded bool `json:"superseded,omitempty"` } // MoveSecret is one move asked and believed, as the handler is given it: the row, this machine, and who asked. @@ -70,6 +73,9 @@ type MovedSecret struct { Secret string Generation int Unchanged bool + // Superseded says, in words, that the vault holds another value for the name, which is the live one: nothing was + // moved, and the move is done (the vault's record of the held value, its generation in Generation). + Superseded string } // MoveSecretFunc moves one value into the vault: what it kept, or why not. @@ -133,7 +139,11 @@ func AnswerMoveSecret(check HandOverCheck, handle MoveSecretFunc, body []byte) M if moved.Unchanged { said = fmt.Sprintf("the vault held %s already, the same value (generation %d)", moved.Secret, moved.Generation) } - return MoveSecretAnswer{Said: said, Secret: moved.Secret, Generation: moved.Generation, Unchanged: moved.Unchanged} + if moved.Superseded != "" { + said = moved.Superseded + } + return MoveSecretAnswer{Said: said, Secret: moved.Secret, Generation: moved.Generation, Unchanged: moved.Unchanged, + Superseded: moved.Superseded != ""} } // MoveSecretReply is what a move request is answered with, and whether to answer, as HandOverReply. diff --git a/internal/link/movesecret_test.go b/internal/link/movesecret_test.go index 41ae611..fc01756 100644 --- a/internal/link/movesecret_test.go +++ b/internal/link/movesecret_test.go @@ -28,8 +28,9 @@ func TestTheMovesAskKeepsItsFieldNamesAndContext(t *testing.T) { if MoveSecretContext != "novox-mesh move-secret v1\n" { t.Fatalf("the signing context is %q", MoveSecretContext) } - answer, _ := json.Marshal(MoveSecretAnswer{Said: "s", Refused: "r", Secret: "own/a/b/c", Generation: 1, Unchanged: true}) - if got := keysOf(t, answer); got != "generation refused said secret unchanged" { + answer, _ := json.Marshal(MoveSecretAnswer{Said: "s", Refused: "r", Secret: "own/a/b/c", Generation: 1, Unchanged: true, + Superseded: true}) + if got := keysOf(t, answer); got != "generation refused said secret superseded unchanged" { t.Fatalf("the answer's fields are %q", got) } }