From 79ac62ffbcdb1c3f3bc895895e84f12a9d20d32e Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 20:17:46 +0100 Subject: [PATCH 1/2] Issue an account whose credential is not sealed, and forget an unassigned module's accounts (issue 1945, ADR 0543) Met a second time live on 2026-10-25 at 20:04 (note 35): mesh-build-go was assigned to ace again, and its account rows, minted at 17:19, had outlived the unassignment. The issue minted only when no row stood, so it minted and sealed nothing; the node had no credential and the bus no user, and the module crash-looped 20 times. The watch still said "the grants written". - A bus account now says when its credential was sealed to its node (migration 0108; every account standing before is taken as sealed). An issue under the assign act, the assignments' watch or a registration mints only an account that is not issued, a row with no seal counting as none, under the store's lock on the account across its look, mint and seal, and marks it sealed once sealed. A seal that fails leaves it unsealed for the next issue. - Unassigning a module forgets its bus accounts and their credentials on that node, so an assignment again issues them afresh. - Only sealed accounts count as minted for a composition and are named in a grant, so the assigner, which now wants a node only once the grant names its account (mesh-delivery, same branch), holds the node until it is. - The assignment's log line says which accounts were issued and sealed, or that they stood issued and sealed. The process-only issue drops the atomic insert of the review of 9ff47f6a for the lock, which also covers the seal. Tests: cases in the existing tables. Research 072: the grants piece (memberships and credentials heard from placement). --- cmd/mesh-controller/acts.go | 4 +- cmd/mesh-controller/assignment_accounts.go | 30 +++- .../assignment_accounts_test.go | 70 ++++++++++ cmd/mesh-controller/grants.go | 6 +- cmd/mesh-controller/modules.go | 132 ++++++++++++++---- cmd/mesh-controller/process_accounts.go | 9 +- internal/inventory/bususers.go | 113 ++++++++++----- .../inventory/bususers_integration_test.go | 76 +++++----- internal/inventory/catalogue.go | 4 + .../0108-a-bus-account-says-it-is-sealed.sql | 8 ++ internal/inventory/read_state.go | 6 +- 11 files changed, 347 insertions(+), 111 deletions(-) create mode 100644 internal/inventory/migrations/0108-a-bus-account-says-it-is-sealed.sql diff --git a/cmd/mesh-controller/acts.go b/cmd/mesh-controller/acts.go index 8de675b5..1eab1bbc 100644 --- a/cmd/mesh-controller/acts.go +++ b/cmd/mesh-controller/acts.go @@ -421,9 +421,9 @@ func issueOnAssign(ctx context.Context, open *stores, node, module string) strin // A module running the provisioner client alone is minted the client's account (issue 917). user = broker.Principal{Kind: broker.KindProvisioner, Node: node, Module: module}.Username() } - if _, minted, err := inv.BusUserHash(ctx, user); err != nil { + if _, sealed, err := inv.BusAccountIssued(ctx, user); err != nil { return "" - } else if minted { + } else if sealed { // a row whose credential is not sealed to the node is not issued (issue 1945) // Its own account stands and is kept; a process it gained is not issued yet (issue 1226). return processAccountsOnAssign(ctx, inv, m, node) } diff --git a/cmd/mesh-controller/assignment_accounts.go b/cmd/mesh-controller/assignment_accounts.go index 8c6ba982..1005718e 100644 --- a/cmd/mesh-controller/assignment_accounts.go +++ b/cmd/mesh-controller/assignment_accounts.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "slices" "strings" "sync" "time" @@ -195,22 +196,27 @@ func issueAssignmentAccounts(ctx context.Context, inv *inventory.Inventory, shel if !known || mayIssue(m) != nil { return false, "", nil } - if user, own := ownAccountOf(m, node); own { - _, minted, err := inv.BusUserHash(ctx, user) + if _, own := ownAccountOf(m, node); own { + minted, err := inv.BusMinted(ctx) // sealed accounts alone: a row with no sealed credential is not issued if err != nil { - return false, "", fmt.Errorf("whether %s is minted cannot be read: %w", user, err) + return false, "", fmt.Errorf("the issued accounts cannot be read: %w", err) } - if !minted { + if missingAccounts(m, node, minted) { busAddress, err := broker.BusAddress() if err != nil { return false, "", err } - // Its processes' accounts too: issueOnTheNewBus issues every one of them with the module's own. + // Its processes' and provisioner client's accounts too: issueOnTheNewBus issues each that is not issued. + ctx, got := withIssued(ctx) if err := issueOnTheNewBus(ctx, inv, m, node, busAddress); err != nil { return false, "", err } - return true, "its bus credential issued as " + user, nil + if issued := got.list(); len(issued) > 0 { + return true, "issued and sealed to " + node + ": " + strings.Join(issued, ", "), nil + } + return false, "", nil } + return false, "its bus accounts on " + node + " stood issued and sealed", nil } line, err := issueMissingProcessAccounts(ctx, inv, m, node) if err != nil { @@ -219,6 +225,18 @@ func issueAssignmentAccounts(ctx context.Context, inv *inventory.Inventory, shel return line != "", line, nil } +// missingAccounts says some account of m on node is not issued: its own, a process's, or its provisioner client's. +func missingAccounts(m catalogue.Manifest, node string, minted []string) bool { + if user, own := ownAccountOf(m, node); own && !slices.Contains(minted, user) { + return true + } + if runsTheProvisionerClient(m) && + !slices.Contains(minted, broker.Principal{Kind: broker.KindProvisioner, Node: node, Module: m.Module}.Username()) { + return true + } + return len(processAccountsMissing(m, node, minted)) > 0 +} + // liveAssignmentAccountSteps are the steps on the controller's own stores and the bus: the composer's passes, as the // done keys' take-ins use them, so a node asked for by both is composed once a pass. func liveAssignmentAccountSteps(open *stores) assignmentAccountSteps { diff --git a/cmd/mesh-controller/assignment_accounts_test.go b/cmd/mesh-controller/assignment_accounts_test.go index 277e8c8d..b7f1d67b 100644 --- a/cmd/mesh-controller/assignment_accounts_test.go +++ b/cmd/mesh-controller/assignment_accounts_test.go @@ -12,6 +12,7 @@ import ( "git.novox.be/novox/mesh-sdk/go/catalogue" "github.com/novox/mesh-controller/internal/broker" "github.com/novox/mesh-controller/internal/chain" + "github.com/novox/mesh-controller/internal/inventory" ) // An assignment that comes to stand issues its node's bus accounts and the module's grant (ADR 0543, issue 1945): on @@ -324,6 +325,48 @@ func TestTheAccountsAModuleSpeaksWithOnANode(t *testing.T) { } }) + // An issue mints and seals an account only when it is not issued: no row, or a row with no sealed credential (issue + // 1945, live on 2026-10-25 at 20:04: ace's mesh-build-go had a row from 17:19 and no credential), under the lock on + // the account; a seal that fails leaves it unsealed for the next issue. A rotation always mints. + for _, c := range []struct { + name string + only bool + minted, sealed bool + sealErr error + want string + }{ + {"no row: minted unsealed, sealed, marked", true, false, false, nil, "lock mint-unsealed seal mark release"}, + {"a row sealed: nothing", true, true, true, nil, "lock release"}, + {"a row with no sealed credential: minted and sealed again", true, true, false, nil, "lock mint-unsealed seal mark release"}, + {"a seal that fails: not marked", true, false, false, errors.New("the vault did not take it"), "lock mint-unsealed seal release"}, + {"a rotation mints whatever stands", false, true, true, nil, "mint seal"}, + } { + t.Run("issuing an account: "+c.name, func(t *testing.T) { + acts := &anAccountStore{minted: c.minted, sealed: c.sealed} + ctx, got := withIssued(t.Context()) + if c.only { + ctx = onlyIfAbsent(ctx) + } + err := issueAccount(ctx, acts, inventory.BusUser{Username: "ace.mesh-build-go", Kind: inventory.BusModule}, + func(password string) error { + acts.did("seal") + if password == "" { + t.Error("sealed no password") + } + return c.sealErr + }) + if (err != nil) != (c.sealErr != nil) { + t.Fatalf("error %v, want %v", err, c.sealErr) + } + if strings.Join(acts.acts, " ") != c.want { + t.Fatalf("acts %q, want %q", strings.Join(acts.acts, " "), c.want) + } + if issued := len(got.list()) == 1; issued != (c.sealErr == nil && strings.Contains(c.want, "seal")) { + t.Fatalf("said issued %v", got.list()) + } + }) + } + t.Run("a process may be issued only with its own broker secret", func(t *testing.T) { // mesh-mail's real shape: no `broker` of its own, its process declaring bus (review round 1 of issue 1181). if err := mayIssue(mailSenderWithProcess()); err != nil { @@ -342,3 +385,30 @@ func TestTheAccountsAModuleSpeaksWithOnANode(t *testing.T) { } }) } + +// anAccountStore is an accountIssuer that writes down what was asked of it. +type anAccountStore struct { + minted, sealed bool + acts []string +} + +func (a *anAccountStore) did(act string) { a.acts = append(a.acts, act) } +func (a *anAccountStore) LockBusAccount(context.Context, string) (func(), error) { + a.did("lock") + return func() { a.did("release") }, nil +} +func (a *anAccountStore) BusAccountIssued(context.Context, string) (bool, bool, error) { + return a.minted, a.sealed, nil +} +func (a *anAccountStore) MintBusPassword(context.Context, inventory.BusUser) (string, error) { + a.did("mint") + return "a-password", nil +} +func (a *anAccountStore) MintBusPasswordUnsealed(context.Context, inventory.BusUser) (string, error) { + a.did("mint-unsealed") + return "a-password", nil +} +func (a *anAccountStore) MarkBusAccountSealed(context.Context, string) error { + a.did("mark") + return nil +} diff --git a/cmd/mesh-controller/grants.go b/cmd/mesh-controller/grants.go index 32310c53..07759f39 100644 --- a/cmd/mesh-controller/grants.go +++ b/cmd/mesh-controller/grants.go @@ -248,7 +248,11 @@ func grantsIssued(ctx context.Context, inv *inventory.Inventory, js jetstream.Je } hashes := make(map[string]string, len(kept)) for name, u := range kept { - hashes[name] = u.PasswordHash + // A grant names an account whose credential is sealed to its node, never one only minted: the assigner wants a + // node only once the grant names its account (issue 1945, rule 6 per node). + if u.Sealed { + hashes[name] = u.PasswordHash + } } current, err := currentFingerprints(ctx, inv) if err != nil { diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index c690dd34..9e9f175c 100644 --- a/cmd/mesh-controller/modules.go +++ b/cmd/mesh-controller/modules.go @@ -9,8 +9,10 @@ import ( "net" "os" "path/filepath" + "slices" "sort" "strings" + "sync" "time" "git.novox.be/novox/mesh-sdk/go/catalogue" @@ -965,24 +967,91 @@ func processPrincipalFor(m catalogue.Manifest, processID string) (broker.Process return broker.ProcessPrincipal{}, false } -// mintOnlyIfAbsent marks a context whose issue mints only accounts no row names yet (onlyIfAbsent). +// mintOnlyIfAbsent marks a context whose issue mints only accounts that are not issued (onlyIfAbsent). type mintOnlyIfAbsent struct{} -// onlyIfAbsent is ctx for an issue that must never replace a minted account: the assign act's and the assignments' -// watch's, which may run at once for one account (novox/hq ADR 0543, the review of issue 1945). +// onlyIfAbsent is ctx for an issue that never replaces an issued account: the assign act's, the assignments' watch's +// and a registration's, which may run at once for one account (novox/hq ADR 0543, issue 1945). func onlyIfAbsent(ctx context.Context) context.Context { return context.WithValue(ctx, mintOnlyIfAbsent{}, true) } -// mintBusUser mints u's password. By default it replaces whatever stands (a rotation: `module issue`); under -// onlyIfAbsent it mints only when no row names u, and won false says another caller minted it first and seals it, -// so this one seals nothing. -func mintBusUser(ctx context.Context, inv *inventory.Inventory, u inventory.BusUser) (string, bool, error) { - if only, _ := ctx.Value(mintOnlyIfAbsent{}).(bool); only { - return inv.MintBusPasswordIfAbsent(ctx, u) +// issuedNow collects the accounts an issue minted and sealed, for the line that says it (withIssued). +type issuedNow struct { + mu sync.Mutex + users []string +} + +type issuedNowKey struct{} + +// withIssued is ctx collecting the accounts its issues mint and seal. +func withIssued(ctx context.Context) (context.Context, *issuedNow) { + got := &issuedNow{} + return context.WithValue(ctx, issuedNowKey{}, got), got +} + +func (g *issuedNow) list() []string { + if g == nil { + return nil } - password, err := inv.MintBusPassword(ctx, u) - return password, err == nil, err + g.mu.Lock() + defer g.mu.Unlock() + return slices.Clone(g.users) +} + +// accountIssuer is what issuing one account needs of the store. +type accountIssuer interface { + LockBusAccount(ctx context.Context, username string) (func(), error) + BusAccountIssued(ctx context.Context, username string) (minted, sealed bool, err error) + MintBusPassword(ctx context.Context, u inventory.BusUser) (string, error) + MintBusPasswordUnsealed(ctx context.Context, u inventory.BusUser) (string, error) + MarkBusAccountSealed(ctx context.Context, username string) error +} + +// issueAccount mints u's password and seals it to its node with seal. By default it replaces whatever stands (a +// rotation: `module issue`). Under onlyIfAbsent it issues only an account that is not issued, a row with no sealed +// credential counting as none (issue 1945, live on 2026-10-25 at 20:04: a row whose credential the node no longer held +// left ace's mesh-build-go with no credential), under the store's lock on the account, so two issuers never interleave +// their look, mint and seal: the hash the bus holds is always the password sealed. A seal that fails leaves the +// account unsealed, issued again by the next issue. +func issueAccount(ctx context.Context, accounts accountIssuer, u inventory.BusUser, seal func(password string) error) error { + only, _ := ctx.Value(mintOnlyIfAbsent{}).(bool) + if !only { + password, err := accounts.MintBusPassword(ctx, u) + if err != nil { + return err + } + if err := seal(password); err != nil { + return err + } + } else { + release, err := accounts.LockBusAccount(ctx, u.Username) + if err != nil { + return err + } + defer release() + if minted, sealed, err := accounts.BusAccountIssued(ctx, u.Username); err != nil { + return err + } else if minted && sealed { + return nil + } + password, err := accounts.MintBusPasswordUnsealed(ctx, u) + if err != nil { + return err + } + if err := seal(password); err != nil { + return err + } + if err := accounts.MarkBusAccountSealed(ctx, u.Username); err != nil { + return fmt.Errorf("%s is sealed and not marked so, and is issued again by the next issue: %w", u.Username, err) + } + } + if got, _ := ctx.Value(issuedNowKey{}).(*issuedNow); got != nil { + got.mu.Lock() + got.users = append(got.users, u.Username) + got.mu.Unlock() + } + return nil } // issueOnTheNewBus gives an assigned module its credential on the bus being built. @@ -1010,17 +1079,12 @@ func issueOnTheNewBus(ctx context.Context, inv *inventory.Inventory, m catalogue } if _, reads := m.OwnSecrets["broker"]; reads { user := broker.Principal{Kind: broker.KindModule, Node: node, Module: m.Module}.Username() - password, won, err := mintBusUser(ctx, inv, inventory.BusUser{ - Username: user, Kind: busKindOf(m.Module), Node: node, Module: m.Module, - }) - if err != nil { + if err := issueAccount(ctx, inv, inventory.BusUser{Username: user, Kind: busKindOf(m.Module), Node: node, + Module: m.Module}, func(password string) error { + return issueWith(ctx, inv, m, node, busAddress, known, reachable, user, password) + }); err != nil { return err } - if won { - if err := issueWith(ctx, inv, m, node, busAddress, known, reachable, user, password); err != nil { - return err - } - } } // Every process with a bus declaration gets an account of its own (novox/hq ADR 0436 §1, §2), minted and // delivered whether or not the module itself speaks on the bus — its own secret is broker-, @@ -1043,12 +1107,15 @@ func issueOnTheNewBus(ctx context.Context, inv *inventory.Inventory, m catalogue func issueProcess(ctx context.Context, inv *inventory.Inventory, m catalogue.Manifest, pr broker.ProcessPrincipal, node, busAddress string, known broker.Broker, reachable string) error { user := broker.Principal{Kind: broker.KindProcess, Node: node, Module: m.Module, Process: pr.Process}.Username() - password, won, err := mintBusUser(ctx, inv, inventory.BusUser{ - Username: user, Kind: inventory.BusProcess, Node: node, Module: m.Module, - }) - if err != nil || !won { - return err // lost: the caller that minted it seals it - } + return issueAccount(ctx, inv, inventory.BusUser{Username: user, Kind: inventory.BusProcess, Node: node, Module: m.Module}, + func(password string) error { + return sealProcess(ctx, inv, m, pr, node, busAddress, known, reachable, user, password) + }) +} + +// sealProcess seals a process's minted account to its node, hands it to the vault and makes its work-queue consumers. +func sealProcess(ctx context.Context, inv *inventory.Inventory, m catalogue.Manifest, pr broker.ProcessPrincipal, + node, busAddress string, known broker.Broker, reachable, user, password string) error { name := processSecretName(pr.Process) held, err := heldProcessAccount(known, reachable, node, m.Module, user, password) if err != nil { @@ -1113,12 +1180,15 @@ func runsTheProvisionerClient(m catalogue.Manifest) bool { func issueProvisionerClient(ctx context.Context, inv *inventory.Inventory, m catalogue.Manifest, node string, known broker.Broker, reachable string) error { user := broker.Principal{Kind: broker.KindProvisioner, Node: node, Module: m.Module}.Username() - password, won, err := mintBusUser(ctx, inv, inventory.BusUser{ - Username: user, Kind: inventory.BusProvisioner, Node: node, Module: m.Module, + return issueAccount(ctx, inv, inventory.BusUser{Username: user, Kind: inventory.BusProvisioner, Node: node, + Module: m.Module}, func(password string) error { + return sealProvisionerClient(ctx, inv, m, node, known, reachable, user, password) }) - if err != nil || !won { - return err // lost: the caller that minted it seals it - } +} + +// sealProvisionerClient seals the provisioner client's minted account to its machine and hands it to the vault. +func sealProvisionerClient(ctx context.Context, inv *inventory.Inventory, m catalogue.Manifest, node string, + known broker.Broker, reachable, user, password string) error { held, err := json.Marshal(struct { URL string `json:"url"` Fingerprint string `json:"fingerprint,omitempty"` diff --git a/cmd/mesh-controller/process_accounts.go b/cmd/mesh-controller/process_accounts.go index 2032697f..d7bede7a 100644 --- a/cmd/mesh-controller/process_accounts.go +++ b/cmd/mesh-controller/process_accounts.go @@ -94,15 +94,18 @@ func issueMissingProcessAccounts(ctx context.Context, inv *inventory.Inventory, if err != nil { busAddress = "" } - var ids []string + ctx, got := withIssued(ctx) for _, pr := range missing { if err := issueProcess(ctx, inv, m, pr, node, busAddress, known, reachable); err != nil { return "", err } - ids = append(ids, pr.Process) + } + issued := got.list() + if len(issued) == 0 { + return "", nil // another issuer sealed them meanwhile } return fmt.Sprintf("its process account(s) %s issued and sealed to %s, and arrive with the push", - strings.Join(ids, ", "), node), nil + strings.Join(issued, ", "), node), nil } // issueGainedProcessAccounts runs after a module's new definition is registered: on each machine assigned it, the diff --git a/internal/inventory/bususers.go b/internal/inventory/bususers.go index ee5c3be1..f879e4c6 100644 --- a/internal/inventory/bususers.go +++ b/internal/inventory/bususers.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "sync/atomic" + "time" "github.com/jackc/pgx/v5" "golang.org/x/crypto/bcrypt" @@ -52,6 +53,8 @@ type BusUser struct { // PasswordHash is what the composed file carries. The plaintext is returned once, by Mint, and // then exists only where it was sealed. PasswordHash string + // Sealed says its credential reached its node: an issue's mint is not sealed until then (ADR 0543 §2). + Sealed bool } // The kinds of bus user the mesh records. The same words the composer uses, so a row and a @@ -100,52 +103,98 @@ func (i *Inventory) RecordBusPassword(ctx context.Context, u BusUser, password s return i.writeBusUser(ctx, u, string(hash)) } -// writeBusUser is the row, whoever chose the password. +// writeBusUser is the row, whoever chose the password, taken as sealed. func (i *Inventory) writeBusUser(ctx context.Context, u BusUser, hash string) error { + return i.writeBusUserSealed(ctx, u, hash, true) +} + +// writeBusUserSealed is the row, sealed now or not yet. +func (i *Inventory) writeBusUserSealed(ctx context.Context, u BusUser, hash string, sealed bool) error { if _, err := i.store.Pool().Exec(ctx, - `insert into bus_user (username, kind, node, module, password_hash) - values ($1, $2, $3, $4, $5) + `insert into bus_user (username, kind, node, module, password_hash, sealed_at) + values ($1, $2, $3, $4, $5, case when $6 then now() end) on conflict (username) do update set kind = excluded.kind, node = excluded.node, module = excluded.module, - password_hash = excluded.password_hash, minted_at = now()`, - u.Username, u.Kind, u.Node, u.Module, hash); err != nil { + password_hash = excluded.password_hash, minted_at = now(), sealed_at = excluded.sealed_at`, + u.Username, u.Kind, u.Node, u.Module, hash, sealed); err != nil { return fmt.Errorf("cannot record the bus user %s: %w", u.Username, err) } return nil } -// MintBusPasswordIfAbsent mints u's password only when no row names u yet, in one statement: of two callers that both -// found the account missing, exactly one wins, and the other is told it lost (won false, no password) and must seal -// nothing, so the hash the bus holds is always the password the node is sealed (novox/hq ADR 0543, the review of -// issue 1945: the assign act and the assignments' watch once each minted, the later hash beside the earlier seal). -func (i *Inventory) MintBusPasswordIfAbsent(ctx context.Context, u BusUser) (string, bool, error) { - if u.Username == "" || u.Kind == "" { - return "", false, errors.New("a bus user needs a username and a kind") - } - raw := make([]byte, 32) - if _, err := rand.Read(raw); err != nil { - return "", false, fmt.Errorf("cannot generate a bus password: %w", err) - } - password := base64.RawURLEncoding.EncodeToString(raw) - hash, err := hashBusPassword(password) +// LockBusAccount holds the store's lock on one account until the answered release is called, so two issuers of it +// (the assign act, the assignments' watch, a registration's process accounts) never interleave their look, mint and +// seal: the hash the bus holds is always the password sealed to the node (novox/hq ADR 0543 §2, issue 1945). +func (i *Inventory) LockBusAccount(ctx context.Context, username string) (func(), error) { + conn, err := i.store.Pool().Acquire(ctx) if err != nil { - return "", false, fmt.Errorf("cannot hash a bus password: %w", err) + return nil, fmt.Errorf("the lock on %s cannot be taken: %w", username, err) } - tag, err := i.store.Pool().Exec(ctx, - `insert into bus_user (username, kind, node, module, password_hash) - values ($1, $2, $3, $4, $5) - on conflict (username) do nothing`, - u.Username, u.Kind, u.Node, u.Module, string(hash)) + if _, err := conn.Exec(ctx, `select pg_advisory_lock(hashtext($1))`, "bus_user/"+username); err != nil { + conn.Release() + return nil, fmt.Errorf("the lock on %s cannot be taken: %w", username, err) + } + return func() { + _, _ = conn.Exec(context.WithoutCancel(ctx), `select pg_advisory_unlock(hashtext($1))`, "bus_user/"+username) + conn.Release() + }, nil +} + +// ForgetBusAccountsOf forgets the bus accounts of module on node, and the credentials sealed to the node for them (its +// `broker`, each process's `broker-`, its provisioner client's): an unassigned module keeps no account, so an +// assignment again mints and seals afresh, never trusting a row whose credential the node no longer holds (issue 1945, +// live on 2026-10-25 at 20:04: ace's mesh-build-go rows from 17:19 survived its unassignment, nothing was minted or +// sealed again, and the module found no credential). +func (i *Inventory) ForgetBusAccountsOf(ctx context.Context, node, module string) error { + if node == "" || module == "" { + return errors.New("forgetting the bus accounts of no node or no module would forget others' accounts") + } + tx, err := i.store.Pool().Begin(ctx) if err != nil { - return "", false, fmt.Errorf("cannot record the bus user %s: %w", u.Username, err) + return err } - if tag.RowsAffected() == 0 { - return "", false, nil + defer func() { _ = tx.Rollback(context.WithoutCancel(ctx)) }() + if _, err := tx.Exec(ctx, `delete from bus_user where node = $1 and module = $2 and kind <> $3`, + node, module, BusController); err != nil { + return fmt.Errorf("the bus accounts of %s on %s are not forgotten: %w", module, node, err) } - return password, true, nil + if _, err := tx.Exec(ctx, + `delete from module_secret s using node n + where s.node = n.id and n.name = $1 and s.module = $2 + and (s.name = 'broker' or s.name like 'broker-%' or s.name = 'provisioner-broker')`, node, module); err != nil { + return fmt.Errorf("the bus credentials of %s on %s are not forgotten: %w", module, node, err) + } + return tx.Commit(ctx) +} + +// MintBusPasswordUnsealed mints u's password, replacing whatever stands, with no seal yet: the account is not issued +// until MarkBusAccountSealed says its credential reached the node (ADR 0543 §2). +func (i *Inventory) MintBusPasswordUnsealed(ctx context.Context, u BusUser) (string, error) { + return i.mintBusPassword(ctx, u, false) +} + +// MarkBusAccountSealed says the credential last minted for username is sealed to its node. +func (i *Inventory) MarkBusAccountSealed(ctx context.Context, username string) error { + _, err := i.store.Pool().Exec(ctx, `update bus_user set sealed_at = now() where username = $1`, username) + return err +} + +// BusAccountIssued says whether an account is minted, and whether its credential is sealed to its node. +func (i *Inventory) BusAccountIssued(ctx context.Context, username string) (minted, sealed bool, err error) { + var at *time.Time + err = i.store.Pool().QueryRow(ctx, `select sealed_at from bus_user where username = $1`, username).Scan(&at) + if errors.Is(err, pgx.ErrNoRows) { + return false, false, nil + } + return err == nil, err == nil && at != nil, err } func (i *Inventory) MintBusPassword(ctx context.Context, u BusUser) (string, error) { + return i.mintBusPassword(ctx, u, true) +} + +// mintBusPassword mints, marked sealed when the caller seals it itself in the same act (every mint but an issue's). +func (i *Inventory) mintBusPassword(ctx context.Context, u BusUser, sealed bool) (string, error) { if u.Username == "" || u.Kind == "" { return "", errors.New("a bus user needs a username and a kind") } @@ -163,7 +212,7 @@ func (i *Inventory) MintBusPassword(ctx context.Context, u BusUser) (string, err return "", fmt.Errorf("cannot hash a bus password: %w", err) } - if err := i.writeBusUser(ctx, u, string(hash)); err != nil { + if err := i.writeBusUserSealed(ctx, u, string(hash), sealed); err != nil { return "", err } return password, nil @@ -176,7 +225,7 @@ func (i *Inventory) MintBusPassword(ctx context.Context, u BusUser) (string, err // password, and the composer refuses it rather than writing a user anybody is. func (i *Inventory) BusUsers(ctx context.Context) (map[string]BusUser, error) { rows, err := i.store.Pool().Query(ctx, - `select username, kind, node, module, password_hash from bus_user order by username`) + `select username, kind, node, module, password_hash, sealed_at is not null from bus_user order by username`) if err != nil { return nil, err } @@ -184,7 +233,7 @@ func (i *Inventory) BusUsers(ctx context.Context) (map[string]BusUser, error) { out := map[string]BusUser{} for rows.Next() { var u BusUser - if err := rows.Scan(&u.Username, &u.Kind, &u.Node, &u.Module, &u.PasswordHash); err != nil { + if err := rows.Scan(&u.Username, &u.Kind, &u.Node, &u.Module, &u.PasswordHash, &u.Sealed); err != nil { return nil, err } out[u.Username] = u diff --git a/internal/inventory/bususers_integration_test.go b/internal/inventory/bususers_integration_test.go index e04a2786..229978a0 100644 --- a/internal/inventory/bususers_integration_test.go +++ b/internal/inventory/bususers_integration_test.go @@ -4,7 +4,9 @@ package inventory import ( "context" + "slices" "testing" + "time" "golang.org/x/crypto/bcrypt" ) @@ -63,48 +65,54 @@ func TestMintingAgainRotatesAndMintingIfAbsentHasOneWinner(t *testing.T) { t.Fatal("the previous password still verifies, so a rotation revoked nothing") } - // **Minting only if absent never rotates, and of several at once exactly one wins** (ADR 0543, the review of issue - // 1945): the assign act and the assignments' watch may both find an account missing; the winner's password is the - // one the store's hash verifies, so the one it seals is the one the bus accepts, and a loser seals nothing. - if _, won, err := inv.MintBusPasswordIfAbsent(ctx, aBusUser("shop")); err != nil || won { - t.Fatalf("a mint-if-absent over a minted account won %v (%v): it would replace a sealed password", won, err) + // **An issue's mint is not issued until its seal is marked, and forgotten with its assignment** (ADR 0543 §2, issue + // 1945, live on 2026-10-25 at 20:04): a row whose credential the node does not hold is not issued. + if minted, sealed, err := inv.BusAccountIssued(ctx, "one.shop"); err != nil || !minted || !sealed { + t.Fatalf("a mint that seals in its own act: minted %v sealed %v (%v)", minted, sealed, err) } - const racers = 8 - type minted struct { - password string - won bool - err error + if _, err := inv.MintBusPasswordUnsealed(ctx, aBusUser("racer")); err != nil { + t.Fatal(err) } - results := make(chan minted, racers) - for range racers { - go func() { - p, won, err := inv.MintBusPasswordIfAbsent(ctx, aBusUser("racer")) - results <- minted{p, won, err} - }() + if minted, sealed, err := inv.BusAccountIssued(ctx, "one.racer"); err != nil || !minted || sealed { + t.Fatalf("an issue's mint before its seal: minted %v sealed %v (%v)", minted, sealed, err) } - var winner string - wins := 0 - for range racers { - r := <-results - if r.err != nil { - t.Fatal(r.err) - } - if r.won { - wins++ - winner = r.password - } else if r.password != "" { - t.Fatal("a caller that lost was handed a password to seal") - } + if names, err := inv.BusMinted(ctx); err != nil || slices.Contains(names, "one.racer") { + t.Fatalf("an account not sealed counts as minted: %v (%v)", names, err) } - if wins != 1 { - t.Fatalf("%d of %d concurrent mints won, want exactly one", wins, racers) + if err := inv.MarkBusAccountSealed(ctx, "one.racer"); err != nil { + t.Fatal(err) } - stored, _, err := inv.BusUserHash(ctx, "one.racer") + if _, sealed, _ := inv.BusAccountIssued(ctx, "one.racer"); !sealed { + t.Fatal("an account marked sealed is not") + } + // The lock on an account holds a second issuer until the first lets go. + release, err := inv.LockBusAccount(ctx, "one.racer") if err != nil { t.Fatal(err) } - if err := bcrypt.CompareHashAndPassword([]byte(stored), []byte(winner)); err != nil { - t.Fatal("the stored hash is not the winner's password: the node would be sealed a password the bus refuses") + waiting := make(chan struct{}) + go func() { + again, err := inv.LockBusAccount(ctx, "one.racer") + if err == nil { + again() + } + close(waiting) + }() + select { + case <-waiting: + t.Fatal("a second issuer took the lock the first held") + case <-time.After(200 * time.Millisecond): + } + release() + <-waiting + if err := inv.ForgetBusAccountsOf(ctx, "one", "racer"); err != nil { + t.Fatal(err) + } + if minted, _, _ := inv.BusAccountIssued(ctx, "one.racer"); minted { + t.Fatal("an unassigned module's account was kept") + } + if minted, _, _ := inv.BusAccountIssued(ctx, "one.shop"); !minted { + t.Fatal("forgetting one module's accounts forgot another's") } } diff --git a/internal/inventory/catalogue.go b/internal/inventory/catalogue.go index cebcd655..c7677741 100644 --- a/internal/inventory/catalogue.go +++ b/internal/inventory/catalogue.go @@ -670,6 +670,10 @@ func (i *Inventory) Unassign(ctx context.Context, nodeName, module string) error if err := i.chainUnassign(ctx, c, node, module); err != nil { return err } + // Its bus accounts on the node go with it: an assignment again issues them afresh (issue 1945). + if err := i.ForgetBusAccountsOf(ctx, nodeName, module); err != nil { + return err + } // The seat it held falls back to derivation rather than pointing at nothing: what the store's foreign key did. if _, err := i.store.Pool().Exec(ctx, `delete from seat_holding where node = $1 and module = $2`, node.ID, module); err != nil { return err diff --git a/internal/inventory/migrations/0108-a-bus-account-says-it-is-sealed.sql b/internal/inventory/migrations/0108-a-bus-account-says-it-is-sealed.sql new file mode 100644 index 00000000..da7da155 --- /dev/null +++ b/internal/inventory/migrations/0108-a-bus-account-says-it-is-sealed.sql @@ -0,0 +1,8 @@ +-- A bus account says when the credential minted for it was sealed to its node (novox/hq ADR 0543 §2, issue 1945). The +-- assign act and the assignments' watch issue an account only when it is not issued: a row with no sealed credential +-- is not issued. On 2026-10-25 at 20:04 ace's mesh-build-go had a row minted at 17:19 whose credential the node no +-- longer held; minting nothing, the mesh sealed nothing, and the module crash-looped for its missing credential. Null +-- while an issue mints and seals; set once the credential is handed over. Every account standing before this migration +-- is taken as sealed: each works or worked, and its own issue sealed it. +alter table bus_user add column sealed_at timestamptz; +update bus_user set sealed_at = minted_at; diff --git a/internal/inventory/read_state.go b/internal/inventory/read_state.go index 7b7591be..f826f9cf 100644 --- a/internal/inventory/read_state.go +++ b/internal/inventory/read_state.go @@ -226,8 +226,10 @@ func (i *Inventory) BusMinted(ctx context.Context) ([]string, error) { return nil, err } out := make([]string, 0, len(kept)) - for name := range kept { - out = append(out, name) + for name, u := range kept { + if u.Sealed { // an account whose credential is not sealed to its node is not issued (ADR 0543 §2) + out = append(out, name) + } } sort.Strings(out) return out, nil -- 2.54.0 From fbf14b68dbabbe4980ea85c32de03b0dbeb97c37 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 20:27:30 +0100 Subject: [PATCH 2/2] Forget the accounts of modules no longer on their nodes at the start's look, and grant sealed accounts alone (issue 1945, ADR 0543, review) Migration 0108 marks every standing account sealed, ace's stale mesh-build-go rows from 17:19 among them, and the forgetting ran only at a later unassign: the next assignment to ace would have issued nothing again. - The start's look first forgets, from each node's own resolution (assigned and required), the accounts of every module that no longer resolves on its node, and says each; only from a reading that holds assignments, and a node whose resolution is unknown is left alone. - Unassign forgets a module's accounts only once it no longer resolves on the node (a module another requires keeps running), and never the node's own provisioner account; the forgetting moved from the inventory to the act. - Forgetting holds each account's lock across its delete, so an issue in flight never writes a row back after it. - A module whose accounts are not all sealed is granted its sealed accounts alone, and its grant is dropped when none is: an older grant naming ace no longer lets the assigner want it. - An issue's locked section has a deadline of two minutes, since it holds a connection of the store's pool across the vault and the bus. Tests: cases in the existing tables. Research 072: the grants piece (memberships and credentials heard from placement). --- cmd/mesh-controller/acts.go | 16 ++++ cmd/mesh-controller/assignment_accounts.go | 90 ++++++++++++++++++- .../assignment_accounts_test.go | 38 ++++++++ cmd/mesh-controller/grants.go | 33 +++++-- cmd/mesh-controller/grants_test.go | 21 ++++- cmd/mesh-controller/modules.go | 8 ++ internal/inventory/bususers.go | 54 ++++++++--- .../inventory/bususers_integration_test.go | 2 +- internal/inventory/catalogue.go | 4 - 9 files changed, 234 insertions(+), 32 deletions(-) diff --git a/cmd/mesh-controller/acts.go b/cmd/mesh-controller/acts.go index 1eab1bbc..354e83cc 100644 --- a/cmd/mesh-controller/acts.go +++ b/cmd/mesh-controller/acts.go @@ -292,6 +292,22 @@ func unassign(ctx context.Context, open *stores, node string, modules ...string) answer += "\n " + strings.TrimSpace(line) } } + // Its bus accounts on the node go with it once it no longer resolves there (a module another requires keeps running + // and keeps them), so an assignment again issues them afresh (issue 1945). The node's own provisioner account stays. + if after.err == nil { + for _, module := range modules { + if slices.Contains(after.modules, module) { + continue + } + forgot, err := open.inventory.ForgetBusAccountsOf(ctx, node, module, false) + switch { + case err != nil: + answer += fmt.Sprintf("\n its bus accounts on %s are not forgotten: %v", node, err) + case len(forgot) > 0: + answer += fmt.Sprintf("\n its bus accounts on %s forgotten: %s", node, strings.Join(forgot, ", ")) + } + } + } // What it leaves behind that is irreplaceable is kept and retired, never removed (novox/hq ADR 0233). for _, line := range keptOnUnassign(ctx, open.inventory, node, modules) { answer += "\n " + line diff --git a/cmd/mesh-controller/assignment_accounts.go b/cmd/mesh-controller/assignment_accounts.go index 1005718e..242853de 100644 --- a/cmd/mesh-controller/assignment_accounts.go +++ b/cmd/mesh-controller/assignment_accounts.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "maps" "slices" "strings" "sync" @@ -40,6 +41,9 @@ import ( type assignmentAccountSteps struct { // holds says this controller holds the lease: only its holder issues. holds func(ctx context.Context) bool + // forgetStale forgets every module's accounts on a node where the module no longer resolves, and answers a line for + // each (issue 1945: rows of an unassignment before this release, marked sealed by migration 0108). + forgetStale func(ctx context.Context) ([]string, error) // catalogue reads the catalogue once, for every assignment a look issues for. catalogue func(ctx context.Context) (map[string]catalogue.Manifest, error) // issue issues the module's accounts on the node that are not minted, from the catalogue read; issued says it @@ -86,6 +90,17 @@ func (s assignmentAccountSteps) settle(ctx context.Context, node, module string) // it issued (an assignment that came to stand while no controller heard it may have its accounts and still lack its // place in the bus's user list or its grant). It answers what it did. func (s assignmentAccountSteps) look(ctx context.Context, standing []chain.Assignment) (string, error) { + var forgot []string + if len(standing) > 0 && s.forgetStale != nil { + // Only from a reading of the assignments that holds some: an empty or failed one would forget every account. + if !s.holds(ctx) { + return "", errNotHolding + } + var err error + if forgot, err = s.forgetStale(ctx); err != nil { + return "", fmt.Errorf("the accounts of modules no longer on their nodes are not forgotten: %w", err) + } + } var looked []chain.Assignment for _, a := range standing { if a.Module != controllerModule { @@ -93,7 +108,7 @@ func (s assignmentAccountSteps) look(ctx context.Context, standing []chain.Assig } } if len(looked) == 0 { - return "", nil + return strings.Join(forgot, "\n"), nil } if !s.holds(ctx) { return "", errNotHolding @@ -114,7 +129,7 @@ func (s assignmentAccountSteps) look(ctx context.Context, standing []chain.Assig } } said, err := s.cover(ctx) - lines = append(lines, fmt.Sprintf("%d standing assignment(s) looked at; %s", len(looked), said)) + lines = append(append(forgot, lines...), fmt.Sprintf("%d standing assignment(s) looked at; %s", len(looked), said)) return strings.Join(lines, "\n"), err } @@ -247,7 +262,8 @@ func liveAssignmentAccountSteps(open *stores) assignmentAccountSteps { _, err := theLease.epoch(ctx) return err == nil }, - catalogue: func(ctx context.Context) (map[string]catalogue.Manifest, error) { return inv.Catalogue(ctx) }, + forgetStale: func(ctx context.Context) ([]string, error) { return forgetStaleAccounts(ctx, open) }, + catalogue: func(ctx context.Context) (map[string]catalogue.Manifest, error) { return inv.Catalogue(ctx) }, issue: func(ctx context.Context, shelf map[string]catalogue.Manifest, node, module string) (bool, string, error) { return issueAssignmentAccounts(ctx, inv, shelf, node, module) }, @@ -403,3 +419,71 @@ func watchAssignmentsOnce(ctx context.Context, watch func(context.Context) (<-ch } } } + +// staleAccounts is, by node, each module some account of which stands on a node where the module no longer resolves: +// the accounts of a module's own, its processes' and its provisioner client's. A node whose resolution is unknown is +// left alone. +func staleAccounts(users map[string]inventory.BusUser, resolved map[string]map[string]bool) map[string][]string { + seen := map[string]map[string]bool{} + for _, u := range users { + switch u.Kind { + case inventory.BusModule, inventory.BusNodeTools, inventory.BusProcess, inventory.BusProvisioner: + default: + continue + } + on, known := resolved[u.Node] + if u.Node == "" || u.Module == "" || !known || on[u.Module] { + continue + } + if seen[u.Node] == nil { + seen[u.Node] = map[string]bool{} + } + seen[u.Node][u.Module] = true + } + out := map[string][]string{} + for node, modules := range seen { + out[node] = slices.Sorted(maps.Keys(modules)) + } + return out +} + +// forgetStaleAccounts forgets the accounts of every module on a node where it no longer resolves (assigned or +// required), from each node's own resolution; a node that does not resolve is left alone. +func forgetStaleAccounts(ctx context.Context, open *stores) ([]string, error) { + inv := open.inventory + users, err := inv.BusUsers(ctx) + if err != nil { + return nil, err + } + ctx = withInputCache(ctx) // one reading of the mesh for every node resolved + resolved := map[string]map[string]bool{} + for _, u := range users { + if _, done := resolved[u.Node]; done || u.Node == "" { + continue + } + plan, _, err := resolveNode(ctx, open, u.Node) + if err != nil { + continue // unknown: nothing of this node is forgotten + } + on := map[string]bool{} + for _, m := range plan.Modules { + on[m.Module] = true + } + resolved[u.Node] = on + } + var lines []string + stale := staleAccounts(users, resolved) + for _, node := range slices.Sorted(maps.Keys(stale)) { + for _, module := range stale[node] { + forgot, err := inv.ForgetBusAccountsOf(ctx, node, module, true) + if err != nil { + return lines, err + } + if len(forgot) > 0 { + lines = append(lines, fmt.Sprintf("%s no longer runs on %s: its bus accounts forgotten: %s", module, node, + strings.Join(forgot, ", "))) + } + } + } + return lines, nil +} diff --git a/cmd/mesh-controller/assignment_accounts_test.go b/cmd/mesh-controller/assignment_accounts_test.go index b7f1d67b..0dd9cbc2 100644 --- a/cmd/mesh-controller/assignment_accounts_test.go +++ b/cmd/mesh-controller/assignment_accounts_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "reflect" "strings" "testing" "time" @@ -194,6 +195,7 @@ func TestSettlingAndTheStartsLook(t *testing.T) { wantComposed []string wantGrants bool wantErr string + stale []string // what forgetting the accounts of modules no longer on their nodes says }{ {name: "an account not minted", module: "mesh-build-node", issued: true, holder: "novox", wantIssues: 1, wantComposed: []string{"novox"}, wantGrants: true}, @@ -214,13 +216,21 @@ func TestSettlingAndTheStartsLook(t *testing.T) { look: []chain.Assignment{ace("mesh-build-node"), ace("mesh-build-go")}, holder: "novox", wantIssues: 2, wantComposed: []string{"novox"}, wantGrants: true}, {name: "the start's look at nothing does nothing", look: []chain.Assignment{ace(controllerModule)}, holder: "novox"}, + {name: "the start's look forgets the accounts of modules no longer on their nodes first", + look: []chain.Assignment{ace("mesh-build-node")}, holder: "novox", wantIssues: 1, wantComposed: []string{"novox"}, + wantGrants: true, stale: []string{"mesh-build-go no longer runs on ace: its bus accounts forgotten: ace.mesh-build-go"}}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { var composed []string issues, catalogues, grants := 0, 0, 0 + forgets := 0 steps := assignmentAccountSteps{ holds: func(context.Context) bool { return !c.notHolding }, + forgetStale: func(context.Context) ([]string, error) { + forgets++ + return c.stale, nil + }, catalogue: func(context.Context) (map[string]catalogue.Manifest, error) { catalogues++ return map[string]catalogue.Manifest{}, nil @@ -262,6 +272,14 @@ func TestSettlingAndTheStartsLook(t *testing.T) { if want := map[bool]int{true: 1}[c.wantGrants]; grants != want { t.Fatalf("grants written %d time(s), want %d", grants, want) } + if want := map[bool]int{true: 1}[len(c.look) > 0 && !c.notHolding]; forgets != want { + t.Fatalf("forgot stale accounts %d time(s), want %d (only from a reading that holds assignments)", forgets, want) + } + for _, line := range c.stale { + if !strings.Contains(said, line) { + t.Fatalf("said %q, want it to say %q", said, line) + } + } if c.wantGrants && c.wantErr == "" && !strings.Contains(said, "the grants written") { t.Fatalf("said %q, want it to name the grants", said) } @@ -367,6 +385,26 @@ func TestTheAccountsAModuleSpeaksWithOnANode(t *testing.T) { }) } + // The accounts of a module that no longer resolves on its node are stale, its provisioner client's among them; a + // module still resolved (assigned or required), an account of another kind, and a node whose resolution is unknown + // are left alone (issue 1945: ace's mesh-build-go rows from 17:19, marked sealed by migration 0108). + t.Run("stale accounts", func(t *testing.T) { + u := func(kind, node, module string) inventory.BusUser { + return inventory.BusUser{Username: node + "." + module + "." + kind, Kind: kind, Node: node, Module: module} + } + users := map[string]inventory.BusUser{} + for _, b := range []inventory.BusUser{u(inventory.BusModule, "ace", "mesh-build-go"), + u(inventory.BusProcess, "ace", "mesh-build-go"), u(inventory.BusProvisioner, "ace", "mesh-provisioner"), + u(inventory.BusModule, "ace", "mesh-build-node"), u(inventory.BusNode, "ace", ""), + u(inventory.BusModule, "g14", "mesh-build-go")} { + users[b.Username] = b + } + got := staleAccounts(users, map[string]map[string]bool{"ace": {"mesh-build-node": true}}) + if want := map[string][]string{"ace": {"mesh-build-go", "mesh-provisioner"}}; !reflect.DeepEqual(got, want) { + t.Fatalf("stale %v, want %v", got, want) + } + }) + t.Run("a process may be issued only with its own broker secret", func(t *testing.T) { // mesh-mail's real shape: no `broker` of its own, its process declaring bus (review round 1 of issue 1181). if err := mayIssue(mailSenderWithProcess()); err != nil { diff --git a/cmd/mesh-controller/grants.go b/cmd/mesh-controller/grants.go index 07759f39..4151aa0c 100644 --- a/cmd/mesh-controller/grants.go +++ b/cmd/mesh-controller/grants.go @@ -23,10 +23,13 @@ import ( // writes `.` then, naming the accounts it composed for the module, never removed. A module one // of whose accounts has no password yet (the list leaves it out) has no grant until a later send carries it. -// grantsFrom is the grant of every module whose accounts all have a password in hashes, at the fingerprint current names -// for it; a module whose current build names no fingerprint of the new form has none. principals are the bus's users -// as broker.Users composes them. -func grantsFrom(principals []broker.Principal, hashes map[string]string, current map[string]string) map[string]sdkdelivery.ControllerGrant { +// grantsFrom is the grant of every module at the fingerprint current names for it, naming its accounts that have a +// sealed password in hashes; a module whose current build names no fingerprint of the new form has none. A module one +// of whose accounts has none is granted its sealed accounts alone, and when it has none, its grant is dropped (drop): +// an older grant naming a node whose account is gone would let the assigner want it (issue 1945, rule 6 per node), and +// an empty grant names every node. principals are the bus's users as broker.Users composes them. +func grantsFrom(principals []broker.Principal, hashes map[string]string, current map[string]string) ( + map[string]sdkdelivery.ControllerGrant, []string) { accounts, missing := map[string][]string{}, map[string]bool{} for _, p := range principals { if p.Module == "" || (p.Kind != broker.KindModule && p.Kind != broker.KindProcess) { @@ -39,8 +42,9 @@ func grantsFrom(principals []broker.Principal, hashes map[string]string, current accounts[p.Module] = append(accounts[p.Module], p.Username()) } out := map[string]sdkdelivery.ControllerGrant{} + var drop []string for module, fp := range current { - if missing[module] || !strings.HasPrefix(fp, sdkdelivery.SourcePrefix2) { + if !strings.HasPrefix(fp, sdkdelivery.SourcePrefix2) { continue } key, err := sdkdelivery.ControllerGrantKey(module, fp) @@ -48,10 +52,15 @@ func grantsFrom(principals []broker.Principal, hashes map[string]string, current continue } names := accounts[module] + if missing[module] && len(names) == 0 { + drop = append(drop, key) + continue + } sort.Strings(names) out[key] = sdkdelivery.ControllerGrant{Accounts: names} } - return out + sort.Strings(drop) + return out, drop } // currentFingerprints is, by module, the fingerprint of the build each registered module is registered at: its @@ -258,7 +267,17 @@ func grantsIssued(ctx context.Context, inv *inventory.Inventory, js jetstream.Je if err != nil { return fmt.Errorf("the grants issued are not written: %w", err) } - wrote, err := keepGrants(ctx, kv, grantsFrom(principals, hashes, current)) + grants, drop := grantsFrom(principals, hashes, current) + wrote, err := keepGrants(ctx, kv, grants) + for _, key := range drop { + if _, gerr := kv.Get(ctx, key); gerr == nil { + if derr := kv.Delete(ctx, key); derr != nil { + err = errors.Join(err, fmt.Errorf("%s: %w", key, derr)) + } else { + fmt.Printf("%sthe grant %s dropped: none of its accounts is sealed to its node\n", indent, key) + } + } + } if wrote > 0 { fmt.Printf("%s%d grant(s) issued written for the assigner\n", indent, wrote) } diff --git a/cmd/mesh-controller/grants_test.go b/cmd/mesh-controller/grants_test.go index b14715ef..9bbe0395 100644 --- a/cmd/mesh-controller/grants_test.go +++ b/cmd/mesh-controller/grants_test.go @@ -23,12 +23,25 @@ func TestTheGrantsAListIssues(t *testing.T) { y := broker.Principal{Kind: broker.KindModule, Node: "ace", Module: "mesh-y"} node := broker.Principal{Kind: broker.KindNode, Node: "novox"} hashes := map[string]string{x.Username(): "h1", xp.Username(): "h2", node.Username(): "h3"} - got := grantsFrom([]broker.Principal{x, xp, y, node}, hashes, - map[string]string{"mesh-x": grantFP('a'), "mesh-y": grantFP('b'), "web": grantFP('c'), "old": "src1:abc"}) + // mesh-z is sealed on novox and not on ace (issue 1945: an older grant must not name ace): its grant names novox alone. + zn := broker.Principal{Kind: broker.KindModule, Node: "novox", Module: "mesh-z"} + za := broker.Principal{Kind: broker.KindModule, Node: "ace", Module: "mesh-z"} + hashes[zn.Username()] = "h4" + got, drop := grantsFrom([]broker.Principal{x, xp, y, node, zn, za}, hashes, + map[string]string{"mesh-x": grantFP('a'), "mesh-y": grantFP('b'), "web": grantFP('c'), "old": "src1:abc", + "mesh-z": grantFP('d')}) xKey, _ := sdkdelivery.ControllerGrantKey("mesh-x", grantFP('a')) + yKey, _ := sdkdelivery.ControllerGrantKey("mesh-y", grantFP('b')) + zKey, _ := sdkdelivery.ControllerGrantKey("mesh-z", grantFP('d')) webKey, _ := sdkdelivery.ControllerGrantKey("web", grantFP('c')) - if len(got) != 2 { - t.Fatalf("grants = %v, want mesh-x's and web's", got) + if len(got) != 3 { + t.Fatalf("grants = %v, want mesh-x's, mesh-z's and web's", got) + } + if g := got[zKey]; len(g.Accounts) != 1 || g.Accounts[0] != zn.Username() { + t.Errorf("mesh-z's grant = %+v, want its sealed account alone", g) + } + if len(drop) != 1 || drop[0] != yKey { + t.Errorf("dropped %v, want mesh-y's grant, none of whose accounts is sealed", drop) } if g := got[xKey]; len(g.Accounts) != 2 || g.Accounts[0] != x.Username() && g.Accounts[1] != x.Username() { t.Errorf("mesh-x's grant = %+v, want both its accounts", g) diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index 9e9f175c..1b2a5140 100644 --- a/cmd/mesh-controller/modules.go +++ b/cmd/mesh-controller/modules.go @@ -1008,6 +1008,10 @@ type accountIssuer interface { MarkBusAccountSealed(ctx context.Context, username string) error } +// issueLockedFor is the longest an issue holds an account's lock: a seal cut by it leaves the account unsealed, issued +// again by the next issue. +const issueLockedFor = 2 * time.Minute + // issueAccount mints u's password and seals it to its node with seal. By default it replaces whatever stands (a // rotation: `module issue`). Under onlyIfAbsent it issues only an account that is not issued, a row with no sealed // credential counting as none (issue 1945, live on 2026-10-25 at 20:04: a row whose credential the node no longer held @@ -1025,6 +1029,10 @@ func issueAccount(ctx context.Context, accounts accountIssuer, u inventory.BusUs return err } } else { + // A deadline on the locked section: it holds a connection of the store's pool across the vault and the bus. + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, issueLockedFor) + defer cancel() release, err := accounts.LockBusAccount(ctx, u.Username) if err != nil { return err diff --git a/internal/inventory/bususers.go b/internal/inventory/bususers.go index f879e4c6..08374b35 100644 --- a/internal/inventory/bususers.go +++ b/internal/inventory/bususers.go @@ -141,30 +141,58 @@ func (i *Inventory) LockBusAccount(ctx context.Context, username string) (func() } // ForgetBusAccountsOf forgets the bus accounts of module on node, and the credentials sealed to the node for them (its -// `broker`, each process's `broker-`, its provisioner client's): an unassigned module keeps no account, so an -// assignment again mints and seals afresh, never trusting a row whose credential the node no longer holds (issue 1945, -// live on 2026-10-25 at 20:04: ace's mesh-build-go rows from 17:19 survived its unassignment, nothing was minted or -// sealed again, and the module found no credential). -func (i *Inventory) ForgetBusAccountsOf(ctx context.Context, node, module string) error { +// `broker`, each process's `broker-`, and with nodeOwned its provisioner client's): a module that no longer resolves +// on a node keeps no account there, so an assignment again mints and seals afresh, never trusting a row whose credential +// the node no longer holds (issue 1945, live on 2026-10-25 at 20:04: ace's mesh-build-go rows from 17:19 outlived its +// unassignment, nothing was issued again, and the module found no credential). The provisioner client's account is the +// node's (`provisioner.`), forgotten only with nodeOwned. Each account's lock is held across its delete, so an issue +// in flight never writes a row back after it. It answers the accounts forgotten. +func (i *Inventory) ForgetBusAccountsOf(ctx context.Context, node, module string, nodeOwned bool) ([]string, error) { if node == "" || module == "" { - return errors.New("forgetting the bus accounts of no node or no module would forget others' accounts") + return nil, errors.New("forgetting the bus accounts of no node or no module would forget others' accounts") + } + rows, err := i.store.Pool().Query(ctx, + `select username from bus_user where node = $1 and module = $2 and kind <> $3 and ($4 or kind <> $5) + order by username`, node, module, BusController, nodeOwned, BusProvisioner) + if err != nil { + return nil, err + } + var names []string + for rows.Next() { + var n string + if err := rows.Scan(&n); err != nil { + rows.Close() + return nil, err + } + names = append(names, n) + } + rows.Close() + if err := rows.Err(); err != nil { + return nil, err + } + for _, n := range names { // in order, one lock at a time per issuer: no two hold each other's + release, err := i.LockBusAccount(ctx, n) + if err != nil { + return nil, err + } + defer release() } tx, err := i.store.Pool().Begin(ctx) if err != nil { - return err + return nil, err } defer func() { _ = tx.Rollback(context.WithoutCancel(ctx)) }() - if _, err := tx.Exec(ctx, `delete from bus_user where node = $1 and module = $2 and kind <> $3`, - node, module, BusController); err != nil { - return fmt.Errorf("the bus accounts of %s on %s are not forgotten: %w", module, node, err) + if _, err := tx.Exec(ctx, `delete from bus_user where username = any($1)`, names); err != nil { + return nil, fmt.Errorf("the bus accounts of %s on %s are not forgotten: %w", module, node, err) } if _, err := tx.Exec(ctx, `delete from module_secret s using node n where s.node = n.id and n.name = $1 and s.module = $2 - and (s.name = 'broker' or s.name like 'broker-%' or s.name = 'provisioner-broker')`, node, module); err != nil { - return fmt.Errorf("the bus credentials of %s on %s are not forgotten: %w", module, node, err) + and (s.name = 'broker' or s.name like 'broker-%' or ($3 and s.name = 'provisioner-broker'))`, + node, module, nodeOwned); err != nil { + return nil, fmt.Errorf("the bus credentials of %s on %s are not forgotten: %w", module, node, err) } - return tx.Commit(ctx) + return names, tx.Commit(ctx) } // MintBusPasswordUnsealed mints u's password, replacing whatever stands, with no seal yet: the account is not issued diff --git a/internal/inventory/bususers_integration_test.go b/internal/inventory/bususers_integration_test.go index 229978a0..8b185f80 100644 --- a/internal/inventory/bususers_integration_test.go +++ b/internal/inventory/bususers_integration_test.go @@ -105,7 +105,7 @@ func TestMintingAgainRotatesAndMintingIfAbsentHasOneWinner(t *testing.T) { } release() <-waiting - if err := inv.ForgetBusAccountsOf(ctx, "one", "racer"); err != nil { + if _, err := inv.ForgetBusAccountsOf(ctx, "one", "racer", true); err != nil { t.Fatal(err) } if minted, _, _ := inv.BusAccountIssued(ctx, "one.racer"); minted { diff --git a/internal/inventory/catalogue.go b/internal/inventory/catalogue.go index c7677741..cebcd655 100644 --- a/internal/inventory/catalogue.go +++ b/internal/inventory/catalogue.go @@ -670,10 +670,6 @@ func (i *Inventory) Unassign(ctx context.Context, nodeName, module string) error if err := i.chainUnassign(ctx, c, node, module); err != nil { return err } - // Its bus accounts on the node go with it: an assignment again issues them afresh (issue 1945). - if err := i.ForgetBusAccountsOf(ctx, nodeName, module); err != nil { - return err - } // The seat it held falls back to derivation rather than pointing at nothing: what the store's foreign key did. if _, err := i.store.Pool().Exec(ctx, `delete from seat_holding where node = $1 and module = $2`, node.ID, module); err != nil { return err -- 2.54.0