Sign the hand-over ask, and fail the line on the engine's refusal (hq issue 356, review)

The subject proved nothing: the bus lets any principal allowed to answer reply to a message it received on the reply subject that message named, so a tool server — the operator's account, every agent — could deliver a hand-over to an engine. The controller now signs the ask with the mesh's key over a fixed context (node, path, who asked, a minute's expiry, a fresh nonce), as declarations are signed, and the engine verifies it. The writers table gains the row for mesh.node.*.ask.hand-over; the subject's comment no longer claims who the engine hears. The line's known-node check and the refusal branch are tested; every check was removed in turn and a test failed.

cmd/mesh-controller/handover_test.go

@@ -1,6 +1,8 @@
 package main
 
 import (
+	"bytes"
+	"errors"
 	"strings"
 	"testing"
 	"time"
@@ -88,3 +90,45 @@ func TestTheUsedAsFoundConditionNamesTheOperatorsLine(t *testing.T) {
 		t.Fatalf("the module's health reads %v %q; want the operator's line", h, why)
 	}
 }
+
+// **Nothing is asked of a node the mesh does not know, and the engine's refusal is the command's failure**
+// (review of issue 356): a refused hand-over never exits as a success.
+func TestAHandOverAsksOnlyAKnownNodeAndFailsOnARefusal(t *testing.T) {
+	t.Setenv(link.CallerVar, "jo through mesh-cli on anchor")
+	asked := 0
+	ask := func(answer link.HandOverAnswer) func(node, path, by string) (link.HandOverAnswer, error) {
+		return func(node, path, by string) (link.HandOverAnswer, error) {
+			asked++
+			if node != "laptop" || path != "/srv/notes" || by != "jo through mesh-cli on anchor" {
+				t.Fatalf("asked %q %q %q", node, path, by)
+			}
+			return answer, nil
+		}
+	}
+	unknown := func(string) error { return errors.New("no node called laptop") }
+	known := func(string) error { return nil }
+	var out bytes.Buffer
+	err := handOverAsked([]string{"laptop", "/srv/notes"}, unknown, ask(link.HandOverAnswer{Said: "x"}), &out)
+	if err == nil || asked != 0 || !strings.Contains(err.Error(), "nothing was asked") {
+		t.Fatalf("an unknown node: %v, asked %d", err, asked)
+	}
+	err = handOverAsked([]string{"laptop", "/srv/../etc"}, known, ask(link.HandOverAnswer{Said: "x"}), &out)
+	if err == nil || asked != 0 {
+		t.Fatalf("a refused line was asked: %v, asked %d", err, asked)
+	}
+	err = handOverAsked([]string{"laptop", "/srv/notes"}, known,
+		ask(link.HandOverAnswer{Refused: "/srv/notes is not used as found; nothing was handed over"}), &out)
+	if err == nil || !strings.Contains(err.Error(), "laptop refused: /srv/notes is not used as found") || out.Len() != 0 {
+		t.Fatalf("a refusal: %v, printed %q", err, out.String())
+	}
+	err = handOverAsked([]string{"laptop", "/srv/notes"}, known, ask(link.HandOverAnswer{Said: "handed over"}), &out)
+	if err != nil || !strings.HasPrefix(out.String(), "handed over\n") || !strings.Contains(out.String(), "`nox push laptop`") {
+		t.Fatalf("a record: %v, printed %q", err, out.String())
+	}
+	failing := func(string, string, string) (link.HandOverAnswer, error) {
+		return link.HandOverAnswer{}, errors.New("no engine")
+	}
+	if err := handOverAsked([]string{"laptop", "/srv/notes"}, known, failing, &out); err == nil {
+		t.Fatal("an ask that failed was a success")
+	}
+}

cmd/mesh-controller/nodes.go

@@ -118,7 +118,7 @@ func nodeCommand(ctx context.Context, args []string) error {
 		// A directory the node-engine uses as found, handed to the mesh (novox/hq issue 356, issue 339). Here, at
 		// the controller's terminal, and nowhere else: at the next apply root gives the directory to the account
 		// the module declares, and whoever may call a verb includes agents.
-		return nodeHandOver(ctx, inv, args[1:])
+		return nodeHandOver(ctx, open, args[1:])
 
 	default:
 		return fmt.Errorf("node has no %q; it has add, list, show, public-domain, account, agent-account and hand-over", args[0])
@@ -160,33 +160,53 @@ func handOverBy() string {
 
 // nodeHandOver asks the node's engine to take a directory it uses as found as the mesh's, and says what came of
 // it. The engine records the hand-over or refuses; nothing is recorded here, because the directory is the
-// machine's and the engine is the one that reads it.
-func nodeHandOver(ctx context.Context, inv *inventory.Inventory, args []string) error {
+// machine's and the engine is the one that reads it. The ask is signed with the mesh's key (issue 356's review).
+func nodeHandOver(ctx context.Context, open *stores, args []string) error {
+	known := func(node string) error {
+		_, err := open.inventory.NodeByName(ctx, node)
+		return err
+	}
+	ask := func(node, path, by string) (link.HandOverAnswer, error) {
+		ident, err := open.Identity(ctx)
+		if err != nil {
+			return link.HandOverAnswer{}, fmt.Errorf("the mesh's signing key cannot be read, so nothing was asked of %s: %w",
+				node, err)
+		}
+		address, err := broker.BusAddress()
+		if err != nil {
+			return link.HandOverAnswer{}, err
+		}
+		js, err := broker.Dial(address)
+		if err != nil {
+			return link.HandOverAnswer{}, fmt.Errorf("cannot reach the bus, so nothing was asked of %s: %w", node, err)
+		}
+		defer js.Close()
+		return link.AskHandOver(ctx, js.Conn(), ident, node, path, by, link.HandOverWithin)
+	}
+	return handOverAsked(args, known, ask, os.Stdout)
+}
+
+// handOverAsked is the hand-over's line with its two acts given: whether the mesh knows the node, and the ask.
+// Nothing is asked of a line or a node that is refused, and the engine's refusal is this command's failure —
+// never a success with the refusal printed.
+func handOverAsked(args []string, known func(node string) error,
+	ask func(node, path, by string) (link.HandOverAnswer, error), out io.Writer) error {
 	node, path, err := handOverLine(args)
 	if err != nil {
 		return err
 	}
-	if _, err := inv.NodeByName(ctx, node); err != nil {
-		return err
+	if err := known(node); err != nil {
+		return fmt.Errorf("nothing was asked: %w", err)
 	}
-	address, err := broker.BusAddress()
-	if err != nil {
-		return err
-	}
-	js, err := broker.Dial(address)
-	if err != nil {
-		return fmt.Errorf("cannot reach the bus, so nothing was asked of %s: %w", node, err)
-	}
-	defer js.Close()
-	answer, err := link.AskHandOver(ctx, js.Conn(), node, path, handOverBy(), link.HandOverWithin)
+	answer, err := ask(node, path, handOverBy())
 	if err != nil {
 		return err
 	}
 	if answer.Refused != "" {
 		return fmt.Errorf("%s refused: %s", node, answer.Refused)
 	}
-	fmt.Println(answer.Said)
-	fmt.Printf("  the module's condition clears once %s applies; `nox push %s` applies it now\n", node, node)
+	fmt.Fprintln(out, answer.Said)
+	fmt.Fprintf(out, "  the module's condition clears once %s applies; `nox push %s` applies it now\n", node, node)
 	return nil
 }
 

internal/broker/nats.go

@@ -291,7 +291,10 @@ func AskReportSubject(node string) string { return "mesh.node." + node + ".ask.r
 
 // AskHandOverSubject is where the controller's terminal asks one machine's node-engine to hand a directory it
 // uses as found to the mesh (novox/hq issue 356, issue 339): a request on core NATS, answered once on the reply
-// it carries. Only the controller publishes under `mesh.node.`, so the engine hears nobody else.
+// it carries. Only the controller is granted a publish here (the writers table holds it), but **that is not who
+// the engine hears**: the bus lets any principal allowed to answer reply to a message it received, on the reply
+// subject that message named, so a message can arrive here from any responder. The ask is therefore signed with
+// the mesh's key (link.SignedHandOver), and the engine verifies it before reading anything out of it.
 func AskHandOverSubject(node string) string { return "mesh.node." + node + ".ask.hand-over" }
 
 // inbox is a principal's own reply space. No user is ever granted a bare `_INBOX.>` (design 25
@@ -872,8 +875,10 @@ func PermissionsFor(p Principal) (Permissions, error) {
 		Subscribe:   sub,
 		// A module answers what it was asked — a tool call reaches it on its own namespace, so the
 		// authority is bounded by having been asked — and so does the controller. A node is asked one
-		// thing, a hand-over on its own subject (novox/hq issue 356), and answers that. A person is never
-		// asked anything, and is granted nothing here.
+		// thing, a hand-over on its own subject (novox/hq issue 356), and answers that: the node is its
+		// machine's engine, root there already, and it is delivered only its own subjects. What it answers is
+		// never trusted for being an answer — the controller reads the engine's words and records nothing. A
+		// person is never asked anything, and is granted nothing here.
 		AllowResponses: p.Kind == KindModule || p.Kind == KindController || p.Kind == KindNodeTools || p.Kind == KindNode,
 	}, nil
 }

internal/broker/writers.go

@@ -84,6 +84,13 @@ func kvOf(bucket string) []string { return []string{"$KV." + bucket + ".>"} }
 var WritersTable = []WriterRow{
 	{State: "a machine's declaration", Writer: "controller (lease holder)", KeptIn: "the bus, last per subject",
 		Others: "read", Subjects: []string{"mesh.node.*.declare"}, Writes: isController},
+	// The operator's hand-over of a directory used as found, asked of the machine's engine at the controller's
+	// terminal (novox/hq issue 356). One publisher; and because a responder can still reach the subject through a
+	// reply, the ask is signed with the mesh's key and the engine verifies it — the row bounds who is granted the
+	// publish, the signature who is believed.
+	{State: "a hand-over asked of a machine", Writer: "controller, at its terminal", KeptIn: "the machine, beside its state",
+		Others:   "the engine verifies the mesh's signature and records it, or refuses",
+		Subjects: []string{"mesh.node.*.ask.hand-over"}, Writes: isController},
 	{State: "a machine's applied state and its report", Writer: "the node-engine's apply queue",
 		KeptIn: "the machine; the report on the bus", Others: "the reconcile and a delivery enqueue, never apply",
 		// And its health statement between reports (novox/hq ADR 0240): the same writer stating the same

internal/broker/writers_test.go

@@ -15,6 +15,7 @@ import (
 // to the table; one dropped from either fails.
 var designRows = []string{
 	"a machine's declaration",
+	"a hand-over asked of a machine",
 	"a machine's applied state and its report",
 	"the controller lease",
 	"plans and their tiers",
@@ -150,3 +151,22 @@ func TestSubjectsOverlap(t *testing.T) {
 		}
 	}
 }
+
+// **A hand-over asked of a machine has one publisher, the controller** (novox/hq issue 356): a grant that lets any
+// other principal publish it — a node, the node tools, a module — is refused at composition, naming the state.
+// (Who the engine believes is the signature's; this bounds who is granted the publish.)
+func TestAHandOverAskHasOnePublisher(t *testing.T) {
+	for _, p := range []Principal{
+		{Kind: KindNode, Node: "laptop"},
+		{Kind: KindNodeTools, Node: "laptop", Module: RuntimeModule},
+		{Kind: KindModule, Node: "laptop", Module: "notes"},
+	} {
+		err := CheckWriters(p, []string{"mesh.node.laptop.ask.hand-over"})
+		if err == nil || !strings.Contains(err.Error(), "a hand-over asked of a machine") {
+			t.Errorf("%s may publish a hand-over: %v", p.Username(), err)
+		}
+	}
+	if err := CheckWriters(Principal{Kind: KindController}, []string{"mesh.node.>"}); err != nil {
+		t.Fatalf("the controller may not ask a hand-over: %v", err)
+	}
+}

internal/link/handover.go

@@ -2,6 +2,8 @@ package link
 
 import (
 	"context"
+	"crypto/rand"
+	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -21,13 +23,35 @@ import (
 // in its own link code (mesh-host internal/link HandOverAsk, HandOverAnswer); a test on each side holds
 // the field names.
 
-// HandOverAsk is what the controller asks: the directory's absolute path as the engine states it, and who
-// asked, in the controller's words.
+// HandOverAsk is what the controller asks: the node it is for, the directory's absolute path as the engine states
+// it, who asked in the controller's words, when the ask stops being good, and a nonce the engine takes once.
 type HandOverAsk struct {
-	Path string `json:"path"`
-	By   string `json:"by"`
+	Node    string    `json:"node"`
+	Path    string    `json:"path"`
+	By      string    `json:"by"`
+	Expires time.Time `json:"expires"`
+	Nonce   string    `json:"nonce"`
 }
 
+// SignedHandOver is the ask as it travels: its bytes exactly as signed, and the signature.
+//
+// **Signed, because the subject proves nothing** (review of issue 356). Only the controller may publish
+// `mesh.node.<node>.ask.hand-over`, but the bus lets any principal allowed to answer reply to a message it
+// received, on whatever reply subject that message named — so a tool server asked on its own subject with that
+// reply could hand the engine an ask the controller never made. The engine verifies this signature, with the
+// key it verifies declarations with, before it reads anything out of the ask.
+type SignedHandOver struct {
+	Ask       []byte `json:"ask"`
+	Signature []byte `json:"signature"`
+}
+
+// HandOverContext is prefixed to an ask's bytes before signing, so a hand-over's signature is never a
+// declaration's: the same key signs both, and a declaration is signed over its bytes alone.
+const HandOverContext = "novox-mesh hand-over v1\n"
+
+// HandOverGood is how long a signed ask is good for: the engine refuses one past it, and one further ahead.
+const HandOverGood = time.Minute
+
 // HandOverAnswer is the engine's answer: what it recorded, or why it refused.
 type HandOverAnswer struct {
 	Said    string `json:"said,omitempty"`
@@ -41,11 +65,12 @@ const HandOverWithin = 30 * time.Second
 // AskHandOver asks one machine's node-engine to hand a directory used as found to the mesh, and reads its
 // answer. An error is the ask not reaching an engine, or an answer that is not one; a refusal is the engine's
 // and comes back in the answer.
-func AskHandOver(ctx context.Context, conn *nats.Conn, node, path, by string, timeout time.Duration) (HandOverAnswer, error) {
+func AskHandOver(ctx context.Context, conn *nats.Conn, signer Signer, node, path, by string,
+	timeout time.Duration) (HandOverAnswer, error) {
 	if conn == nil {
 		return HandOverAnswer{}, errors.New("this controller is not on the bus")
 	}
-	body, err := json.Marshal(HandOverAsk{Path: path, By: by})
+	body, err := SignHandOver(ctx, signer, HandOverAsk{Node: node, Path: path, By: by})
 	if err != nil {
 		return HandOverAnswer{}, err
 	}
@@ -90,3 +115,26 @@ func AskHandOver(ctx context.Context, conn *nats.Conn, node, path, by string, ti
 	}
 	return answer, nil
 }
+
+// SignHandOver fills the ask's expiry and nonce and signs it with the mesh's key, over HandOverContext and the
+// ask's bytes exactly as they travel.
+func SignHandOver(ctx context.Context, signer Signer, ask HandOverAsk) ([]byte, error) {
+	if signer == nil {
+		return nil, errors.New("no signing key, so no hand-over can be asked")
+	}
+	nonce := make([]byte, 16)
+	if _, err := rand.Read(nonce); err != nil {
+		return nil, err
+	}
+	ask.Nonce = hex.EncodeToString(nonce)
+	ask.Expires = time.Now().UTC().Add(HandOverGood)
+	raw, err := json.Marshal(ask)
+	if err != nil {
+		return nil, err
+	}
+	signature, err := signer.Sign(ctx, append([]byte(HandOverContext), raw...))
+	if err != nil {
+		return nil, fmt.Errorf("cannot sign the hand-over: %w", err)
+	}
+	return json.Marshal(SignedHandOver{Ask: raw, Signature: signature})
+}

internal/link/handover_test.go

@@ -2,6 +2,7 @@ package link
 
 import (
 	"context"
+	"crypto/ed25519"
 	"encoding/json"
 	"strings"
 	"testing"
@@ -16,10 +17,15 @@ import (
 // The hand-over's ask and answer hold these field names; the engine's side holds the same list (mesh-host
 // internal/link, TestTheHandOverAskAndAnswerKeepTheirFieldNames).
 func TestTheHandOverAskAndAnswerKeepTheirFieldNames(t *testing.T) {
-	body, _ := json.Marshal(HandOverAsk{Path: "/srv/notes", By: "jo through mesh-cli on anchor"})
-	if got := keysIn(t, body); got != "by path" {
+	body, _ := json.Marshal(HandOverAsk{Node: "laptop", Path: "/srv/notes", By: "jo through mesh-cli on anchor",
+		Expires: time.Now(), Nonce: "n"})
+	if got := keysIn(t, body); got != "by expires node nonce path" {
 		t.Fatalf("the ask's fields are %q", got)
 	}
+	body, _ = json.Marshal(SignedHandOver{Ask: []byte("{}"), Signature: []byte("s")})
+	if got := keysIn(t, body); got != "ask signature" {
+		t.Fatalf("the signed ask's fields are %q", got)
+	}
 	body, _ = json.Marshal(HandOverAnswer{Said: "s", Refused: "r"})
 	if got := keysIn(t, body); got != "refused said" {
 		t.Fatalf("the answer's fields are %q", got)
@@ -27,6 +33,67 @@ func TestTheHandOverAskAndAnswerKeepTheirFieldNames(t *testing.T) {
 	if broker.AskHandOverSubject("laptop") != "mesh.node.laptop.ask.hand-over" {
 		t.Fatalf("the subject is %q", broker.AskHandOverSubject("laptop"))
 	}
+	if HandOverContext != "novox-mesh hand-over v1\n" {
+		t.Fatalf("the signing context is %q; the engine holds the same words", HandOverContext)
+	}
+}
+
+// keySigner signs with one key, as the controller's identity does.
+type keySigner struct{ key ed25519.PrivateKey }
+
+func (k keySigner) Sign(_ context.Context, message []byte) ([]byte, error) {
+	return ed25519.Sign(k.key, message), nil
+}
+
+func testSigner(t *testing.T) (keySigner, ed25519.PublicKey) {
+	t.Helper()
+	public, private, err := ed25519.GenerateKey(nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return keySigner{private}, public
+}
+
+// **The ask is signed over the context and its bytes, with a fresh nonce and a short expiry** (review of issue
+// 356): the signature verifies over HandOverContext and the ask's bytes, and not over the bytes alone — so it is
+// never a declaration's — and two asks never share a nonce.
+func TestAHandOverIsSignedWithAContextANonceAndAnExpiry(t *testing.T) {
+	signer, public := testSigner(t)
+	body, err := SignHandOver(context.Background(), signer, HandOverAsk{Node: "laptop", Path: "/srv/notes", By: "jo"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	var signed SignedHandOver
+	if err := json.Unmarshal(body, &signed); err != nil {
+		t.Fatal(err)
+	}
+	if !ed25519.Verify(public, append([]byte(HandOverContext), signed.Ask...), signed.Signature) {
+		t.Fatal("the signature does not verify over the context and the ask")
+	}
+	if ed25519.Verify(public, signed.Ask, signed.Signature) {
+		t.Fatal("the signature verifies over the ask's bytes alone, as a declaration's would")
+	}
+	var ask HandOverAsk
+	if err := json.Unmarshal(signed.Ask, &ask); err != nil {
+		t.Fatal(err)
+	}
+	if ask.Node != "laptop" || ask.Path != "/srv/notes" || ask.By != "jo" || len(ask.Nonce) != 32 {
+		t.Fatalf("signed %+v", ask)
+	}
+	if left := time.Until(ask.Expires); left <= 0 || left > HandOverGood {
+		t.Fatalf("the ask is good for %s", left)
+	}
+	again, _ := SignHandOver(context.Background(), signer, HandOverAsk{Node: "laptop", Path: "/srv/notes", By: "jo"})
+	var other SignedHandOver
+	_ = json.Unmarshal(again, &other)
+	var second HandOverAsk
+	_ = json.Unmarshal(other.Ask, &second)
+	if second.Nonce == ask.Nonce {
+		t.Fatal("two asks share a nonce")
+	}
+	if _, err := SignHandOver(context.Background(), nil, HandOverAsk{}); err == nil {
+		t.Fatal("an ask was made with no key")
+	}
 }
 
 // The ask reaches the machine's engine on its own subject and its answer comes back whole: what it recorded, or
@@ -40,9 +107,16 @@ func TestAHandOverIsAskedOfTheMachineAndItsAnswerComesBack(t *testing.T) {
 	}
 	t.Cleanup(conn.Close)
 
+	signer, public := testSigner(t)
 	var heard HandOverAsk
 	engine, err := conn.Subscribe(broker.AskHandOverSubject("laptop"), func(msg *nats.Msg) {
-		_ = json.Unmarshal(msg.Data, &heard)
+		var signed SignedHandOver
+		_ = json.Unmarshal(msg.Data, &signed)
+		if !ed25519.Verify(public, append([]byte(HandOverContext), signed.Ask...), signed.Signature) {
+			_ = msg.Respond([]byte(`{"refused":"not the mesh's signature"}`))
+			return
+		}
+		_ = json.Unmarshal(signed.Ask, &heard)
 		switch heard.Path {
 		case "/srv/notes":
 			body, _ := json.Marshal(HandOverAnswer{Said: "/srv/notes (notes.data) is handed to the mesh by " + heard.By})
@@ -60,26 +134,26 @@ func TestAHandOverIsAskedOfTheMachineAndItsAnswerComesBack(t *testing.T) {
 	t.Cleanup(func() { _ = engine.Unsubscribe() })
 
 	ctx := context.Background()
-	a, err := AskHandOver(ctx, conn, "laptop", "/srv/notes", "jo through mesh-cli on anchor", 5*time.Second)
+	a, err := AskHandOver(ctx, conn, signer, "laptop", "/srv/notes", "jo through mesh-cli on anchor", 5*time.Second)
 	if err != nil || a.Refused != "" || !strings.Contains(a.Said, "handed to the mesh by jo through mesh-cli on anchor") {
 		t.Fatalf("answered %+v, %v", a, err)
 	}
-	if heard.Path != "/srv/notes" || heard.By != "jo through mesh-cli on anchor" {
+	if heard.Node != "laptop" || heard.Path != "/srv/notes" || heard.By != "jo through mesh-cli on anchor" {
 		t.Fatalf("the engine heard %+v", heard)
 	}
-	a, err = AskHandOver(ctx, conn, "laptop", "/srv/other", "jo", 5*time.Second)
+	a, err = AskHandOver(ctx, conn, signer, "laptop", "/srv/other", "jo", 5*time.Second)
 	if err != nil || a.Said != "" || !strings.Contains(a.Refused, "nothing was handed over") {
 		t.Fatalf("a refusal came back as %+v, %v", a, err)
 	}
-	if _, err := AskHandOver(ctx, conn, "laptop", "/srv/empty", "jo", 5*time.Second); err == nil ||
+	if _, err := AskHandOver(ctx, conn, signer, "laptop", "/srv/empty", "jo", 5*time.Second); err == nil ||
 		!strings.Contains(err.Error(), "neither") {
 		t.Fatalf("an empty answer was taken: %v", err)
 	}
-	if _, err := AskHandOver(ctx, conn, "anchor", "/srv/notes", "jo", 5*time.Second); err == nil ||
+	if _, err := AskHandOver(ctx, conn, signer, "anchor", "/srv/notes", "jo", 5*time.Second); err == nil ||
 		!strings.Contains(err.Error(), "node-engine") {
 		t.Fatalf("a machine with no engine listening: %v", err)
 	}
-	if _, err := AskHandOver(ctx, nil, "anchor", "/srv/notes", "jo", time.Second); err == nil {
+	if _, err := AskHandOver(ctx, nil, signer, "anchor", "/srv/notes", "jo", time.Second); err == nil {
 		t.Fatal("asked with no bus")
 	}
 }