Recoverable means sealed to the current operator key; recovery names the provider

From review: the export counted any operator-sealed row as recoverable, so a
secret sealed to a replaced key was reported as openable with the current one;
replacing the key counted orphans in one table of two; and a pair credential
held from two providers was recovered as whichever row came first. The export
now lists what the current key opens, what an earlier key opens, and what has
no copy; `secret recover` takes --provider and refuses ambiguity; files that
must not exist are created exclusively; one constructor builds the export for
the operator's file and the vault's disk alike.

internal/inventory/operator.go

@@ -4,10 +4,12 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
 
 	"github.com/jackc/pgx/v5"
 
 	"github.com/novox/mesh-controller/internal/catalogue"
+	"github.com/novox/mesh-controller/internal/secrets"
 )
 
 // The operator's sealing key: the one holder of secrets that is not a node.
@@ -18,6 +20,29 @@ import (
 // What is recorded here is the public half, which is all the mesh needs to seal to it; what it
 // yields is one more blob per secret that the mesh cannot open.
 
+// operatorColumns is the pair of nullable columns a secret row carries for its operator copy:
+// both null when the mesh has no operator key, so a row says plainly that no such copy exists.
+func operatorColumns(operator, blob string) (sealed, key *string) {
+	if operator == "" || blob == "" {
+		return nil, nil
+	}
+	return &blob, &operator
+}
+
+// operatorSeal seals a value somebody supplied to the operator key, when the mesh has one.
+func (i *Inventory) operatorSeal(ctx context.Context, value string) (sealed, key *string, err error) {
+	operator, err := i.OperatorKey(ctx)
+	if err != nil || operator == "" {
+		return nil, nil, err
+	}
+	blob, err := secrets.Seal(operator, []byte(value))
+	if err != nil {
+		return nil, nil, err
+	}
+	sealed, key = operatorColumns(operator, blob)
+	return sealed, key, nil
+}
+
 // OperatorKey is the public key secrets are also sealed to, or empty when the mesh has none.
 func (i *Inventory) OperatorKey(ctx context.Context) (string, error) {
 	var key string
@@ -44,9 +69,12 @@ func (i *Inventory) SetOperatorKey(ctx context.Context, public string) (orphaned
 		return 0, err
 	}
 	defer tx.Rollback(ctx)
+	// Both tables: a module's own secrets and the pair credentials. A count over one of them said
+	// "nothing orphaned" about a mesh whose every vault-provided secret had just been.
 	if err := tx.QueryRow(ctx,
-		`select count(*) from module_secret
-		  where operator_key is not null and operator_key <> $1`, public).Scan(&orphaned); err != nil {
+		`select (select count(*) from module_secret where operator_key is not null and operator_key <> $1)
+		      + (select count(*) from secret        where operator_key is not null and operator_key <> $1)`,
+		public).Scan(&orphaned); err != nil {
 		return 0, err
 	}
 	if _, err := tx.Exec(ctx, `delete from operator_key where public <> $1`, public); err != nil {
@@ -62,12 +90,38 @@ func (i *Inventory) SetOperatorKey(ctx context.Context, public string) (orphaned
 // Kept is the catalogue's: one secret as the operator can recover it.
 type Kept = catalogue.Kept
 
-// KeptForOperator is every secret the operator can recover, and which cannot.
+// OperatorExport is the export as the operator and the vault both keep it: every secret sealed to
+// the mesh's current operator key, every one sealed to an earlier key (recoverable with that key,
+// if the person still has it), and every one with no operator copy at all. Nil when the mesh has
+// no operator key. One constructor, so the file `secret export` writes and the file the mesh puts
+// on the vault's disk cannot drift apart.
+func (i *Inventory) OperatorExport(ctx context.Context) (*catalogue.KeptExport, error) {
+	operator, err := i.OperatorKey(ctx)
+	if err != nil || operator == "" {
+		return nil, err
+	}
+	kept, earlier, unrecoverable, err := i.KeptForOperator(ctx)
+	if err != nil {
+		return nil, err
+	}
+	return &catalogue.KeptExport{
+		Export: 1, OperatorKey: operator, Fingerprint: secrets.Fingerprint(operator),
+		Kept: kept, EarlierKey: earlier, Unrecoverable: unrecoverable,
+	}, nil
+}
+
+// KeptForOperator is every secret by what can open it: the mesh's current operator key, an
+// earlier operator key, or nothing.
 //
-// The second list is the honest half: a secret minted before the mesh had an operator key has no
-// operator-sealed copy and cannot get one — the plaintext was discarded. Naming those is what lets
-// an export say what it does not cover, rather than being taken for complete.
-func (i *Inventory) KeptForOperator(ctx context.Context) (kept []Kept, unrecoverable []Kept, err error) {
+// The last two are the honest half. A secret minted before the mesh had an operator key has no
+// operator-sealed copy and cannot get one — the plaintext was discarded; one sealed to a key the
+// mesh has since replaced is not opened by the current key, however the export is labelled. Naming
+// both is what lets an export say what it does not cover, rather than being taken for complete.
+func (i *Inventory) KeptForOperator(ctx context.Context) (kept, earlier, unrecoverable []Kept, err error) {
+	current, err := i.OperatorKey(ctx)
+	if err != nil {
+		return nil, nil, nil, err
+	}
 	rows, err := i.store.Pool().Query(ctx,
 		`select 'own', n.name, s.module, s.name, '', s.origin, coalesce(s.operator_sealed, ''),
 		        coalesce(s.operator_key, ''), s.made_at
@@ -78,27 +132,34 @@ func (i *Inventory) KeptForOperator(ctx context.Context) (kept []Kept, unrecover
 		   from secret s join node c on c.id = s.consumer join node p on p.id = s.provider
 		  order by 1, 2, 3, 4`)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
 	defer rows.Close()
 	for rows.Next() {
 		var k Kept
 		if err := rows.Scan(&k.Kind, &k.Node, &k.Module, &k.Name, &k.Provider, &k.Origin, &k.Sealed, &k.Key, &k.MadeAt); err != nil {
-			return nil, nil, err
+			return nil, nil, nil, err
 		}
-		if k.Sealed == "" {
+		switch {
+		case k.Sealed == "":
 			unrecoverable = append(unrecoverable, k)
-			continue
+		case k.Key != current:
+			earlier = append(earlier, k)
+		default:
+			kept = append(kept, k)
 		}
-		kept = append(kept, k)
 	}
-	return kept, unrecoverable, rows.Err()
+	return kept, earlier, unrecoverable, rows.Err()
 }
 
 // KeptSecret is one secret's operator-sealed copy, for recovery.
-func (i *Inventory) KeptSecret(ctx context.Context, node, module, name string) (Kept, error) {
-	// An own secret first, then a pair credential by the provision's name. A module whose own
-	// secret and requirement share a name is refused at resolution, so the two cannot both answer.
+//
+// An own secret first, then a pair credential by the provision's name — a module whose own secret
+// and requirement share a name is refused at resolution, so the two cannot both answer. A pair
+// credential is keyed by provider as well, and a consumer whose provision moved leaves the old
+// provider's row behind: two rows is refused with both providers named, never answered with
+// whichever came first, unless `provider` says which.
+func (i *Inventory) KeptSecret(ctx context.Context, node, module, name, provider string) (Kept, error) {
 	var k Kept
 	err := i.store.Pool().QueryRow(ctx,
 		`select 'own', n.name, s.module, s.name, '', s.origin, coalesce(s.operator_sealed, ''),
@@ -107,12 +168,40 @@ func (i *Inventory) KeptSecret(ctx context.Context, node, module, name string) (
 		  where n.name = $1 and s.module = $2 and s.name = $3`, node, module, name).
 		Scan(&k.Kind, &k.Node, &k.Module, &k.Name, &k.Provider, &k.Origin, &k.Sealed, &k.Key, &k.MadeAt)
 	if errors.Is(err, pgx.ErrNoRows) {
-		err = i.store.Pool().QueryRow(ctx,
+		rows, qerr := i.store.Pool().Query(ctx,
 			`select 'pair', c.name, s.consumer_module, s.name, p.name, 'made', coalesce(s.operator_sealed, ''),
 			        coalesce(s.operator_key, ''), s.created_at
 			   from secret s join node c on c.id = s.consumer join node p on p.id = s.provider
-			  where c.name = $1 and s.consumer_module = $2 and s.name = $3`, node, module, name).
-			Scan(&k.Kind, &k.Node, &k.Module, &k.Name, &k.Provider, &k.Origin, &k.Sealed, &k.Key, &k.MadeAt)
+			  where c.name = $1 and s.consumer_module = $2 and s.name = $3 and ($4 = '' or p.name = $4)
+			  order by p.name`, node, module, name, provider)
+		if qerr != nil {
+			return Kept{}, qerr
+		}
+		defer rows.Close()
+		var found []Kept
+		for rows.Next() {
+			var row Kept
+			if err := rows.Scan(&row.Kind, &row.Node, &row.Module, &row.Name, &row.Provider, &row.Origin, &row.Sealed, &row.Key, &row.MadeAt); err != nil {
+				return Kept{}, err
+			}
+			found = append(found, row)
+		}
+		if err := rows.Err(); err != nil {
+			return Kept{}, err
+		}
+		switch len(found) {
+		case 0:
+			err = pgx.ErrNoRows
+		case 1:
+			k, err = found[0], nil
+		default:
+			providers := make([]string, 0, len(found))
+			for _, f := range found {
+				providers = append(providers, f.Provider)
+			}
+			return Kept{}, fmt.Errorf("%s on %s holds a %q credential from more than one provider (%s); say which with --provider",
+				module, node, name, strings.Join(providers, ", "))
+		}
 	}
 	if errors.Is(err, pgx.ErrNoRows) {
 		return Kept{}, fmt.Errorf("%s on %s holds nothing called %q — neither a secret of its own nor a credential for a provision", module, node, name)

internal/inventory/operator_test.go

@@ -2,6 +2,7 @@ package inventory
 
 import (
 	"context"
+	"strings"
 	"testing"
 
 	"github.com/novox/mesh-controller/internal/catalogue"
@@ -20,10 +21,10 @@ func TestAnOwnSecretIsSealedToTheOperatorToo(t *testing.T) {
 	if _, err := inv.SecretForModule(ctx, "consumer", "postgres", "superuser"); err != nil {
 		t.Fatal(err)
 	}
-	if _, err := inv.KeptSecret(ctx, "consumer", "postgres", "superuser"); err == nil {
+	if _, err := inv.KeptSecret(ctx, "consumer", "postgres", "superuser", ""); err == nil {
 		t.Fatal("a secret made before the operator key was reported recoverable")
 	}
-	kept, unrecoverable, err := inv.KeptForOperator(ctx)
+	kept, _, unrecoverable, err := inv.KeptForOperator(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -46,14 +47,14 @@ func TestAnOwnSecretIsSealedToTheOperatorToo(t *testing.T) {
 	if err := inv.AcceptSecretForModule(ctx, "provider", "postgres", "replication", "given-by-a-person"); err != nil {
 		t.Fatal(err)
 	}
-	kept, unrecoverable, err = inv.KeptForOperator(ctx)
+	kept, _, unrecoverable, err = inv.KeptForOperator(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}
 	if len(kept) != 2 || len(unrecoverable) != 1 {
 		t.Fatalf("after a key: %d kept, %d unrecoverable", len(kept), len(unrecoverable))
 	}
-	got, err := inv.KeptSecret(ctx, "provider", "postgres", "replication")
+	got, err := inv.KeptSecret(ctx, "provider", "postgres", "replication", "")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -67,7 +68,7 @@ func TestAnOwnSecretIsSealedToTheOperatorToo(t *testing.T) {
 	if got.Origin != "accepted" || got.Key != pub {
 		t.Fatalf("kept as %+v", got)
 	}
-	minted, err := inv.KeptSecret(ctx, "provider", "postgres", "superuser")
+	minted, err := inv.KeptSecret(ctx, "provider", "postgres", "superuser", "")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -75,26 +76,59 @@ func TestAnOwnSecretIsSealedToTheOperatorToo(t *testing.T) {
 		t.Fatalf("the minted secret did not open to a 40-character value: %v", err)
 	}
 
-	// The old secret, remade for a rejoined node, becomes recoverable — it was issued again.
+	// The old secret is kept, not resealed: asking again is a read, the plaintext is gone, and it
+	// stays honestly unrecoverable.
 	if _, err := inv.SecretForModule(ctx, "consumer", "postgres", "superuser"); err != nil {
 		t.Fatal(err)
 	}
-	if _, err := inv.KeptSecret(ctx, "consumer", "postgres", "superuser"); err == nil {
+	if _, err := inv.KeptSecret(ctx, "consumer", "postgres", "superuser", ""); err == nil {
 		t.Fatal("asking again did not remake, yet it became recoverable")
 	}
+	// Until the node rejoins with a new sealing key: then the secret is remade, and the remake is
+	// sealed to the operator — the one scenario the vault exists for.
+	rejoined, err := inv.NodeByName(ctx, "consumer")
+	if err != nil {
+		t.Fatal(err)
+	}
+	newKey, _ := aSealingKey(t)
+	if err := inv.RecordSealingKey(ctx, rejoined.ID, newKey); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := inv.SecretForModule(ctx, "consumer", "postgres", "superuser"); err != nil {
+		t.Fatal(err)
+	}
+	remade, err := inv.KeptSecret(ctx, "consumer", "postgres", "superuser", "")
+	if err != nil {
+		t.Fatalf("the remade secret is not recoverable: %v", err)
+	}
+	if _, err := secrets.Open(priv, remade.Sealed); err != nil {
+		t.Fatal(err)
+	}
 
-	// Replacing the key says how many secrets stay sealed to the old one.
+	// Replacing the key says how many secrets stay sealed to the old one — and those move out of
+	// the recoverable list, whatever the export is labelled with.
 	pub2, _, _ := secrets.Keypair()
 	orphaned, err := inv.SetOperatorKey(ctx, pub2)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if orphaned != 2 {
-		t.Fatalf("replacing the key orphaned %d, and two were sealed to it", orphaned)
+	if orphaned != 3 {
+		t.Fatalf("replacing the key orphaned %d, and three were sealed to it", orphaned)
 	}
 	if now, _ := inv.OperatorKey(ctx); now != pub2 {
 		t.Fatal("the new key is not the mesh's key")
 	}
+	kept, earlier, _, err := inv.KeptForOperator(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(kept) != 0 || len(earlier) != 3 {
+		t.Fatalf("after replacing the key: %d recoverable with it, %d sealed to the earlier key", len(kept), len(earlier))
+	}
+	doc, err := inv.OperatorExport(ctx)
+	if err != nil || doc == nil || len(doc.EarlierKey) != 3 || len(doc.Kept) != 0 {
+		t.Fatalf("the export does not say what the current key cannot open: %+v %v", doc, err)
+	}
 }
 
 // A pair credential — what the vault provides a module — is sealed to the operator too, and the
@@ -134,13 +168,42 @@ func TestAPairCredentialIsSealedToTheOperatorToo(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	kept, err := inv.KeptSecret(ctx, "consumer", "gitea", "secret")
+	kept, err := inv.KeptSecret(ctx, "consumer", "gitea", "secret", "")
 	if err != nil {
 		t.Fatal(err)
 	}
 	if kept.Kind != "pair" || kept.Provider != "provider" {
 		t.Fatalf("kept as %+v", kept)
 	}
+	all, _, _, err := inv.KeptForOperator(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var pairs int
+	for _, k := range all {
+		if k.Kind == "pair" {
+			pairs++
+		}
+	}
+	if pairs != 1 {
+		t.Fatalf("%d pair credential(s) recoverable, expected 1", pairs)
+	}
+
+	// A second provider of the same provision: two rows, refused rather than the first one taken,
+	// unless the provider is named. And replacing the key counts pair credentials as orphaned.
+	if _, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "consumer"); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := inv.KeptSecret(ctx, "consumer", "gitea", "secret", ""); err == nil || !strings.Contains(err.Error(), "--provider") {
+		t.Fatalf("two providers were not refused: %v", err)
+	}
+	if byName, err := inv.KeptSecret(ctx, "consumer", "gitea", "secret", "provider"); err != nil || byName.Provider != "provider" {
+		t.Fatalf("naming the provider did not select it: %+v %v", byName, err)
+	}
+	pub2, _, _ := secrets.Keypair()
+	if orphaned, err := inv.SetOperatorKey(ctx, pub2); err != nil || orphaned != 2 {
+		t.Fatalf("replacing the key orphaned %d pair credential(s), and two were sealed to it (%v)", orphaned, err)
+	}
 	fromOperator, err := secrets.Open(priv, kept.Sealed)
 	if err != nil {
 		t.Fatal(err)
@@ -153,17 +216,4 @@ func TestAPairCredentialIsSealedToTheOperatorToo(t *testing.T) {
 	if string(fromOperator) != string(fromNode) {
 		t.Fatal("the operator's copy of the pair credential differs from the consumer's")
 	}
-	all, _, err := inv.KeptForOperator(ctx)
-	if err != nil {
-		t.Fatal(err)
-	}
-	var pairs int
-	for _, k := range all {
-		if k.Kind == "pair" {
-			pairs++
-		}
-	}
-	if pairs != 1 {
-		t.Fatalf("%d pair credential(s) in the export, expected 1", pairs)
-	}
 }

internal/inventory/secrets.go

@@ -80,18 +80,11 @@ func (i *Inventory) SecretFor(ctx context.Context, name, consumer, consumerModul
 	if err != nil {
 		return Secret{}, err
 	}
-	var also []string
-	if operator != "" {
-		also = append(also, operator)
-	}
-	made, more, err := secrets.MakeAlso(consumerKey, providerKey, also...)
+	made, blob, err := secrets.MakeWithOperator(consumerKey, providerKey, operator)
 	if err != nil {
 		return Secret{}, err
 	}
-	var forOperator, operatorKey *string
-	if operator != "" {
-		forOperator, operatorKey = &more[0], &operator
-	}
+	forOperator, operatorKey := operatorColumns(operator, blob)
 	_, err = i.store.Pool().Exec(ctx,
 		`insert into secret (name, consumer, consumer_module, provider, for_consumer, for_provider,
 		                     consumer_key, provider_key, operator_sealed, operator_key)
@@ -246,21 +239,14 @@ func (i *Inventory) SecretForModule(ctx context.Context, node, module, name stri
 	if err != nil {
 		return "", err
 	}
-	var also []string
-	if operator != "" {
-		also = append(also, operator)
-	}
-	made, more, err := secrets.MakeAlso(key, key, also...)
-	if err != nil {
-		return "", err
-	}
 	// Sealed once to the machine — Make seals to two ends because a provision has two; here both
 	// are the same machine, and only one copy is kept — and once more to the operator when the
 	// mesh has one (novox/hq ADR 0085, amended), which is the copy a person can recover from.
-	var forOperator, operatorKey *string
-	if operator != "" {
-		forOperator, operatorKey = &more[0], &operator
+	made, blob, err := secrets.MakeWithOperator(key, key, operator)
+	if err != nil {
+		return "", err
 	}
+	forOperator, operatorKey := operatorColumns(operator, blob)
 	if _, err := i.store.Pool().Exec(ctx,
 		`insert into module_secret (node, module, name, sealed, node_key, origin, operator_sealed, operator_key)
 		 values ($1, $2, $3, $4, $5, 'made', $6, $7)
@@ -304,18 +290,10 @@ func (i *Inventory) AcceptSecretForModule(ctx context.Context, node, module, nam
 	}
 	// And to the operator, when the mesh has one: a value a person supplied is the one a person
 	// most needs to get back, since the mesh cannot make another (novox/hq ADR 0085, amended).
-	operator, err := i.OperatorKey(ctx)
+	forOperator, operatorKey, err := i.operatorSeal(ctx, value)
 	if err != nil {
 		return err
 	}
-	var forOperator, operatorKey *string
-	if operator != "" {
-		blob, err := secrets.Seal(operator, []byte(value))
-		if err != nil {
-			return err
-		}
-		forOperator, operatorKey = &blob, &operator
-	}
 	_, err = i.store.Pool().Exec(ctx,
 		`insert into module_secret (node, module, name, sealed, node_key, origin, operator_sealed, operator_key)
 		 values ($1, $2, $3, $4, $5, 'accepted', $6, $7)