From e74ee82a7668fdfe62d09a9663d06e035193a376 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 18:32:59 +0200 Subject: [PATCH 1/4] Renew the engine placed by hand to the proved build, and take a hand-over from the controller's terminal (hq issue 356) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The file on PATH was the engine a person copied at installation, and every delivered engine ran from the versions directory: the operator typed hand-over and a build from before it answered "unexpected argument". A proved engine now copies itself over that file when the bytes differ (never a link). And the hand-over of a directory used as found (issue 339) gets its channel: the controller's terminal asks this node's engine on mesh.node..ask.hand-over; the engine judges every value — the ask's size and shape, who asked, the path as it states it, a directory and no link, through no link — and records it or refuses and records nothing. Every text the engine addresses to the operator names the nox line (ADR 0272); the engine's own subcommands are install-time and rescue, and its usage says so. --- README.md | 33 ++++--- cmd/mesh-host/found.go | 126 ++++++++++++++++++------ cmd/mesh-host/found_test.go | 122 +++++++++++++++++++++-- cmd/mesh-host/main.go | 24 +++-- cmd/mesh-host/witnessing.go | 22 +++++ internal/apply/apply.go | 10 +- internal/link/handover.go | 79 +++++++++++++++ internal/link/handover_test.go | 89 +++++++++++++++++ internal/link/hearing_nats.go | 30 ++++++ internal/link/messages.go | 2 +- internal/link/queue.go | 7 +- internal/link/run.go | 5 + internal/upgrade/fallback.go | 137 ++++++++++++++++++++++++++ internal/upgrade/fallback_test.go | 154 ++++++++++++++++++++++++++++++ 14 files changed, 776 insertions(+), 64 deletions(-) create mode 100644 internal/link/handover.go create mode 100644 internal/link/handover_test.go create mode 100644 internal/upgrade/fallback.go create mode 100644 internal/upgrade/fallback_test.go diff --git a/README.md b/README.md index 96ab198..73aa3f1 100644 --- a/README.md +++ b/README.md @@ -4,11 +4,16 @@ Tier 0 of the Novox Mesh. The one thing ever installed by hand, and the only thi a machine. ``` -scp mesh-host root@machine:/usr/local/bin/ -mesh-host profile +scp nox-mesh-host root@machine:/usr/bin/ +nox-mesh-host profile ``` -That is the whole installation. One statically linked binary, nothing else present, no runtime +That is the whole installation. **It is a daemon**: the launcher runs it, and the operator's command is +`nox` (`mesh-cli`), on any node, from their own account (`novox/hq` ADR 0272). The subcommands below are +for installing a machine, joining it, and rescuing one no mesh can reach, run as root at the machine; a +line the mesh addresses to the operator names a `nox` line, never this binary. Once a delivered engine +proves itself, it renews the one placed by hand here to its own build, so the command on the machine is +the engine that runs it (`novox/hq` issue 356). One statically linked binary, nothing else present, no runtime to install first ([`novox/hq` ADR 0005](https://git.novox.be/novox/hq)). ## What it is for @@ -23,7 +28,7 @@ the host never queries the mesh database. It receives declarations and applies t ## Joining a mesh ``` -mesh-host enrol --token --name +nox-mesh-host enrol --token --name ``` **The node generates its own identity** — an Ed25519 keypair whose private half never leaves the @@ -52,20 +57,22 @@ and then stops, having saved nothing — so it can be run again unchanged. connects to nothing and listens on nothing — what it applies comes from a file. ``` -mesh-host profile what this machine can be asked to do -mesh-host inventory what this machine is, and what it holds -mesh-host apply FILE make this machine match a declaration from a file -mesh-host reconcile make this machine match what the mesh last told it — or, before - any mesh has, the bundle this host carries -mesh-host bundle show what this host carries -mesh-host owned what this host has applied and still owns +nox-mesh-host profile what this machine can be asked to do +nox-mesh-host inventory what this machine is, and what it holds +nox-mesh-host apply FILE make this machine match a declaration from a file +nox-mesh-host reconcile make this machine match what the mesh last told it — or, before + any mesh has, the bundle this host carries +nox-mesh-host bundle show what this host carries +nox-mesh-host owned what this host has applied and still owns +nox-mesh-host hand-over DIR hand a directory used as found to the mesh, on a machine no mesh + can reach; the operator's line is `nox node hand-over DIR` --json machine-readable --state where this node keeps what it knows --dry-run say what applying would change, and change nothing ``` ``` -$ mesh-host profile +$ nox-mesh-host profile linux/amd64 yes container-runtime 29.7.2 @@ -115,7 +122,7 @@ A host built for a machine carries its declaration **inside the binary**: make host BUNDLE=path/to/foundation.lock ``` -`mesh-host reconcile` then applies it, on a machine the mesh has told nothing yet. That is the +`nox-mesh-host reconcile` then applies it, on a machine the mesh has told nothing yet. That is the first node's path — no mesh present, nothing fetched, nothing else copied onto the machine. Once the mesh has spoken, `reconcile` holds the machine to what it last said and never to the bundle, which genesis consumed; a bundle or a file is refused when it says the other mode than the node diff --git a/cmd/mesh-host/found.go b/cmd/mesh-host/found.go index 62db797..48774d3 100644 --- a/cmd/mesh-host/found.go +++ b/cmd/mesh-host/found.go @@ -22,14 +22,20 @@ import ( // directory it never made to whichever account a declaration named. That is safe and, said nowhere, a module // whose data directory it cannot write would simply fail. So each such directory is said three ways: a WARNING // line in every apply's log; a resource of its module of kind `directory`, stated unhealthy on every look, -// which the controller raises as that module's condition naming the directory and its owner; and a way out at -// the machine's terminal, `mesh-host hand-over `, after which the next apply gives it the declared owner -// and mode like any directory of the mesh's. +// which the controller raises as that module's condition naming the directory and its owner; and a way out, +// which is the operator's line at the controller's terminal — `nox node hand-over ` on the +// control-node (novox/hq ADR 0272, issue 356) — after which the next apply gives it the declared owner and +// mode like any directory of the mesh's. The controller carries that line to this engine over the bus +// (link.AskHandOverSubject); the engine's own `hand-over` subcommand is the same act for a machine with no +// mesh to reach. // foundState is where the node's state is, for the looks to read the directories used as found from; empty // until the engine runs. var foundState atomic.Value +// foundNode is this node's name in the mesh, for the way out to name it; empty until the engine is enrolled. +var foundNode atomic.Value + // handedOverFrom has the apply read the directories handed over from beside the state at statePath. func handedOverFrom(statePath string) { apply.HandedOver = func() map[string]bool { @@ -58,9 +64,21 @@ func foundDirectories(known store.State) []store.Applied { return out } +// handOverLine is the operator's line that hands a directory to the mesh, with this node's name when it is +// known (novox/hq ADR 0272): typed on the control-node, from the operator's own account. The directory's +// path is the resource's target beside the reason, never in it: the controller says a reason on the +// operator's channel, which withholds a path (ADR 0234 §6), and keeps the target as evidence. +func handOverLine() string { + node, _ := foundNode.Load().(string) + if node == "" { + node = "" + } + return fmt.Sprintf("`nox node hand-over %s ` on the control-node", node) +} + // withFoundDirectories adds to a statement every directory used as found, as its module's resource of kind -// directory, unhealthy: the reason names its owner and mode as they are now, what was declared, and how to hand -// it over. The path is the target, which the controller keeps as evidence. +// directory, unhealthy: the reason names its owner and mode as they are now, what was declared, and the line +// that hands it over. The path is the target, which the controller keeps as evidence. func withFoundDirectories(h *link.Health, found []store.Applied) *link.Health { for _, r := range found { at := strings.LastIndex(r.ID, ".") @@ -80,9 +98,8 @@ func withFoundDirectories(h *link.Health, found []store.Applied) *link.Health { } h.Resources = append(h.Resources, link.ResourceHealth{Module: r.ID[:at], Resource: r.ID, Kind: link.KindDirectory, Target: r.Target, State: link.StateUnhealthy, - Reason: fmt.Sprintf("%s owned by %s, mode %s, as found; %s was declared and not given it — "+ - "`mesh-host hand-over` at the machine hands it to the mesh", link.ReasonUsedAsFound, owner, mode, - declared), + Reason: fmt.Sprintf("%s owned by %s, mode %s, as found; %s was declared and not given it — %s, with its "+ + "path, hands it to the mesh", link.ReasonUsedAsFound, owner, mode, declared, handOverLine()), Since: foundSince(r).UTC()}) } return h @@ -102,34 +119,81 @@ func foundNow() []store.Applied { return foundDirectories(known) } -// handOverCommand hands one directory used as found to the mesh: run by a person, as root, at the machine. -// It changes nothing on disk itself; the next apply gives the directory its declared owner and mode, and says so. -func handOverCommand(opts options) error { - path := filepath.Clean(opts.file) +// handOverFound hands one directory used as found to the mesh, on the operator's word carried by the +// controller (link.HandOverFunc) or typed at this machine. It changes nothing on disk itself; the next apply +// gives the directory its declared owner and mode, and says so. +// +// **Every value is judged, and a refusal records nothing.** The path is the directory's absolute path exactly +// as the engine states it — clean, so `..` and a doubled separator never name a directory; one this machine's +// state records as used as found; on disk a directory and not a link, reached through no link. The engine +// never follows a path it did not state: a hand-over is root giving a directory to an account at the next +// apply, and a path that resolves elsewhere would give that elsewhere. +func handOverFound(statePath, path, by string) (string, error) { if !filepath.IsAbs(path) { - return fmt.Errorf("hand-over takes the directory's absolute path, and %q is not one", opts.file) + return "", fmt.Errorf("hand-over takes the directory's absolute path, and %q is not one; nothing was handed over", path) } - known, err := store.Load(opts.state) + if filepath.Clean(path) != path { + return "", fmt.Errorf("%q is not the directory's path as the engine states it (no `..`, no doubled or trailing "+ + "separator); nothing was handed over", path) + } + known, err := store.Load(statePath) + if err != nil { + return "", err + } + var found *store.Applied + for _, r := range foundDirectories(known) { + if filepath.Clean(r.Target) == path { + r := r + found = &r + break + } + } + if found == nil { + return "", errors.New(path + " is not a directory this machine uses as found (the module's condition names " + + "the ones that are); nothing was handed over") + } + info, err := os.Lstat(path) + switch { + case err != nil: + return "", fmt.Errorf("%s cannot be read on this machine: %v; nothing was handed over", path, err) + case info.Mode()&os.ModeSymlink != 0: + return "", fmt.Errorf("%s is a link, and the mesh hands over no link (novox/hq ADR 0012); nothing was handed over", path) + case !info.IsDir(): + return "", fmt.Errorf("%s is not a directory on this machine; nothing was handed over", path) + } + resolved, err := filepath.EvalSymlinks(path) + if err != nil { + return "", fmt.Errorf("%s cannot be resolved on this machine: %v; nothing was handed over", path, err) + } + if resolved != path { + return "", fmt.Errorf("%s is reached through a link (it is %s), and the mesh hands over nothing through one "+ + "(novox/hq issue 339); nothing was handed over", path, resolved) + } + if err := store.AddHandedOver(statePath, store.HandOver{ID: found.ID, Path: path, At: time.Now().UTC(), By: by}); err != nil { + return "", err + } + return fmt.Sprintf("%s (%s) is handed to the mesh by %s: the next apply gives it %s, and it is the mesh's from "+ + "then on — removed when empty and no longer declared", path, found.ID, by, orUnsaid(found.Declared)), nil +} + +// handOverFrom is the engine's hand-over for the controller's ask: the state beside statePath, the path as asked. +func handOverFrom(statePath string) link.HandOverFunc { + return func(path, by string) (string, error) { return handOverFound(statePath, path, by) } +} + +// handOverCommand is `nox-mesh-host hand-over ` at this machine, as root: the same act, for a machine +// with no mesh to reach. The operator's line is `nox node hand-over ` on the control-node. +func handOverCommand(opts options) error { + by := "a person at the machine" + if u, err := user.Current(); err == nil { + by = u.Username + " at the machine" + } + said, err := handOverFound(opts.state, filepath.Clean(opts.file), by) if err != nil { return err } - for _, r := range foundDirectories(known) { - if filepath.Clean(r.Target) != path { - continue - } - by := "a person at the machine" - if u, err := user.Current(); err == nil { - by = u.Username - } - if err := store.AddHandedOver(opts.state, store.HandOver{ID: r.ID, Path: path, At: time.Now().UTC(), By: by}); err != nil { - return err - } - fmt.Fprintf(opts.out, "%s (%s) is handed to the mesh: the next apply gives it %s, and it is the mesh's from "+ - "then on — removed when empty and no longer declared\n", path, r.ID, orUnsaid(r.Declared)) - return nil - } - return errors.New(path + " is not a directory this machine uses as found (`mesh-host owned` lists what the " + - "engine applied); nothing was handed over") + fmt.Fprintln(opts.out, said) + return nil } // foundSince is when a directory was first found as it is: kept across applies, so the controller can tell one diff --git a/cmd/mesh-host/found_test.go b/cmd/mesh-host/found_test.go index b88dba4..1721bf6 100644 --- a/cmd/mesh-host/found_test.go +++ b/cmd/mesh-host/found_test.go @@ -14,13 +14,16 @@ import ( ) // Every directory used as found is its module's resource of kind directory, unhealthy, naming its owner, on -// every statement (novox/hq issue 339): the controller raises that as the module's condition. +// every statement (novox/hq issue 339): the controller raises that as the module's condition. The way out it +// names is the operator's line, with this node's name (novox/hq ADR 0272, issue 356). func TestAFoundDirectoryIsStatedUnhealthy(t *testing.T) { dir := t.TempDir() known := store.State{Resources: []store.Applied{ {ID: "notes.data", Type: "directory", Target: dir, AsFound: true, Declared: "root, mode 755", AppliedAt: time.Now()}, {ID: "notes.cache", Type: "directory", Target: dir}, }} + foundNode.Store("laptop") + t.Cleanup(func() { foundNode.Store("") }) h := withFoundDirectories(&link.Health{}, foundDirectories(known)) if len(h.Resources) != 1 { t.Fatalf("stated: %+v", h.Resources) @@ -28,16 +31,45 @@ func TestAFoundDirectoryIsStatedUnhealthy(t *testing.T) { r := h.Resources[0] if r.Module != "notes" || r.Kind != link.KindDirectory || r.State != link.StateUnhealthy || r.Target != dir || !strings.HasPrefix(r.Reason, link.ReasonUsedAsFound) || !strings.Contains(r.Reason, "owned by ") || - !strings.Contains(r.Reason, "root, mode 755") || !strings.Contains(r.Reason, "hand-over") { + !strings.Contains(r.Reason, "root, mode 755") { t.Fatalf("stated as: %+v", r) } + // The path is the target, kept as evidence, and never in the reason: the operator's channel withholds a path + // (novox/hq ADR 0234 §6), and the controller says the reason there. + if !strings.Contains(r.Reason, "`nox node hand-over laptop ` on the control-node") || + strings.Contains(r.Reason, "mesh-host") || strings.Contains(r.Reason, dir) { + t.Fatalf("the way out is not the operator's line, or carries the path: %q", r.Reason) + } + foundNode.Store("") + if r := withFoundDirectories(&link.Health{}, foundDirectories(known)).Resources[0]; !strings.Contains(r.Reason, + "`nox node hand-over `") { + t.Fatalf("before enrolment the line names a placeholder: %q", r.Reason) + } } -// `mesh-host hand-over` records a directory used as found, and refuses one that is not. +// realDir is a temporary directory with every link in its path resolved, so a judgement of links is about the +// links the test makes. +func realDir(t *testing.T) string { + t.Helper() + dir, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + return dir +} + +// A hand-over records a directory used as found, and refuses one that is not — through the engine's own +// subcommand and through the controller's ask alike, which are one judgement. func TestHandOverRecordsOnlyAFoundDirectory(t *testing.T) { - dir := t.TempDir() + dir := realDir(t) statePath := filepath.Join(dir, "state.json") found := filepath.Join(dir, "found") + if err := os.Mkdir(found, 0o700); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(filepath.Join(dir, "mine"), 0o755); err != nil { + t.Fatal(err) + } if err := store.Save(statePath, store.State{Resources: []store.Applied{ {ID: "notes.data", Type: "directory", Target: found, AsFound: true, Declared: "root, mode 755"}, {ID: "notes.mine", Type: "directory", Target: filepath.Join(dir, "mine")}, @@ -52,7 +84,7 @@ func TestHandOverRecordsOnlyAFoundDirectory(t *testing.T) { t.Fatal(err) } all, err := store.LoadHandedOver(statePath) - if err != nil || len(all) != 1 || all[0].Path != found { + if err != nil || len(all) != 1 || all[0].Path != found || !strings.HasSuffix(all[0].By, " at the machine") { t.Fatalf("recorded: %+v %v", all, err) } handedOverFrom(statePath) @@ -62,5 +94,83 @@ func TestHandOverRecordsOnlyAFoundDirectory(t *testing.T) { if !strings.Contains(out.String(), "root, mode 755") { t.Fatalf("said: %s", out.String()) } - _ = os.Remove(statePath) + + // The controller's ask is the same judgement, recording who asked in the controller's words. + _ = os.Remove(store.HandedOverPath(statePath)) + said, err := handOverFrom(statePath)(found, "jo through mesh-cli on anchor") + if err != nil || !strings.Contains(said, "by jo through mesh-cli on anchor") || !strings.Contains(said, "root, mode 755") { + t.Fatalf("the ask answered %q, %v", said, err) + } + all, _ = store.LoadHandedOver(statePath) + if len(all) != 1 || all[0].By != "jo through mesh-cli on anchor" || all[0].ID != "notes.data" { + t.Fatalf("recorded: %+v", all) + } +} + +// Every value of a hand-over is judged before anything is recorded, and each refusal records nothing: a relative +// path, a path that is not the engine's statement of the directory (`..`, a doubled or trailing separator), one +// not used as found, a link at the path, a link in the path, a file, a directory that is gone. +func TestAHandOverIsRefusedForEveryPathThatIsNotTheFoundDirectory(t *testing.T) { + dir := realDir(t) + statePath := filepath.Join(dir, "state.json") + found := filepath.Join(dir, "found") + if err := os.Mkdir(found, 0o700); err != nil { + t.Fatal(err) + } + // A link at the path of a second found directory, and a found directory below a linked directory. + linked := filepath.Join(dir, "linked") + if err := os.Symlink(found, linked); err != nil { + t.Fatal(err) + } + through := filepath.Join(dir, "via", "data") + if err := os.Symlink(dir, filepath.Join(dir, "via")); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(filepath.Join(dir, "data"), 0o700); err != nil { + t.Fatal(err) + } + file := filepath.Join(dir, "file") + if err := os.WriteFile(file, nil, 0o600); err != nil { + t.Fatal(err) + } + gone := filepath.Join(dir, "gone") + if err := store.Save(statePath, store.State{Resources: []store.Applied{ + {ID: "notes.data", Type: "directory", Target: found, AsFound: true}, + {ID: "notes.linked", Type: "directory", Target: linked, AsFound: true}, + {ID: "notes.through", Type: "directory", Target: through, AsFound: true}, + {ID: "notes.file", Type: "directory", Target: file, AsFound: true}, + {ID: "notes.gone", Type: "directory", Target: gone, AsFound: true}, + }}); err != nil { + t.Fatal(err) + } + refused := map[string]string{ + "a relative path": "found", + "a path with ..": dir + "/x/../found", + "a doubled separator": dir + "//found", + "a trailing separator": found + "/", + "a directory not used as found": filepath.Join(dir, "other"), + "a link at the path": linked, + "a link in the path": through, + "a file": file, + "a directory that is gone": gone, + } + for name, path := range refused { + said, err := handOverFound(statePath, path, "jo") + if err == nil || said != "" { + t.Fatalf("%s (%s) was handed over: %q", name, path, said) + } + if !strings.Contains(err.Error(), "nothing was handed over") { + t.Fatalf("%s's refusal does not say nothing was handed over: %v", name, err) + } + } + if all, err := store.LoadHandedOver(statePath); err != nil || len(all) != 0 { + t.Fatalf("a refusal recorded something: %+v %v", all, err) + } + // The found directory itself, stated exactly, is taken. + if _, err := handOverFound(statePath, found, "jo"); err != nil { + t.Fatal(err) + } + if all, _ := store.LoadHandedOver(statePath); len(all) != 1 || all[0].Path != found { + t.Fatalf("recorded: %+v", all) + } } diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index 03b8129..8b237c2 100644 --- a/cmd/mesh-host/main.go +++ b/cmd/mesh-host/main.go @@ -94,7 +94,12 @@ func versionAt(self, stamped string) string { return stamped } -const usage = `mesh-host — the node host +const usage = `nox-mesh-host — the node-engine + +A daemon: its launcher runs it, and it applies what the mesh declares for this machine. The +operator's command is nox (mesh-cli), on any node, from their own account (novox/hq ADR 0272); +the subcommands here are for installing a machine, joining it, and rescuing one no mesh can +reach, run as root at the machine. profile what this machine can be asked to do inventory what this machine is, and what it holds @@ -108,7 +113,9 @@ const usage = `mesh-host — the node host overlay take take over the tunnel found here (novox/hq ADR 0105): its key becomes this node's overlay key and the mesh is told, signed; --tunnel when several are up owned what this host has applied and still owns - hand-over hand a directory used as found to the mesh: the next apply gives it its declared owner and mode + hand-over hand a directory used as found to the mesh: the next apply gives it its declared + owner and mode. The operator's line is nox node hand-over on the + control-node; this is the same act for a machine with no mesh to reach version --json machine-readable output @@ -221,7 +228,7 @@ func parseArgs(args []string) (string, options, error) { // Anything left over was neither the command nor a flag. Refused rather than ignored: a // mistyped argument that changes nothing and reports success is worse than an error. if len(positionals) > 0 { - return "", opts, fmt.Errorf("unexpected argument %q — try `mesh-host help`", positionals[0]) + return "", opts, fmt.Errorf("unexpected argument %q — try `nox-mesh-host help`", positionals[0]) } return command, opts, nil } @@ -327,7 +334,7 @@ func run(ctx context.Context, command string, opts options) error { return nil default: - return fmt.Errorf("unknown command %q — try `mesh-host help`", command) + return fmt.Errorf("unknown command %q — try `nox-mesh-host help`", command) } } @@ -1077,6 +1084,8 @@ func runLink(ctx context.Context, opts options) error { // password, say — and restarting the host (design 28, task 5.2). Same file, same check. say := func(line string) { fmt.Println(line) } adoptDeliveredMembership(identity.Path(opts.state), &mine, say) + // So every statement about a directory used as found names the operator's line for this node. + foundNode.Store(mine.Node) fmt.Printf("node %s, linking to %s\n", mine.Node, mine.Membership.Broker) @@ -1181,8 +1190,11 @@ func runLink(ctx context.Context, opts options) error { }, Unsaid: unsaidBeside(opts.state), Numbers: numbersBeside(opts.state), - Say: say, - Timeout: opts.timeout, + // And the operator's hand-over of a directory used as found, carried by the controller from the + // terminal (novox/hq issue 356): judged and recorded here, applied by the next apply. + HandOver: handOverFrom(opts.state), + Say: say, + Timeout: opts.timeout, } worker := make(chan struct{}) go func() { diff --git a/cmd/mesh-host/witnessing.go b/cmd/mesh-host/witnessing.go index bef15ca..bf7bffb 100644 --- a/cmd/mesh-host/witnessing.go +++ b/cmd/mesh-host/witnessing.go @@ -70,10 +70,32 @@ func (p *proving) heard(r link.Report) { if len(retired) > 0 { p.say(fmt.Sprintf("host %s is proved; retired the host version(s) %s", p.version, strings.Join(retired, ", "))) } + renewFallback(p.version, p.say) renewLauncher(p.say) }) } +// renewFallback makes the engine placed by hand — the launcher's fallback, and the `nox-mesh-host` on this +// machine's PATH — a copy of this proved build when it is not one (novox/hq issue 356). On the same +// evidence known-good is written on, and said when it happens or cannot: a stale command that answers +// for a build that is not running would otherwise stay until somebody typed it. +func renewFallback(version string, say func(string)) { + self, err := os.Executable() + if err != nil { + say("the engine placed by hand could not be renewed: this build does not know its own path: " + err.Error()) + return + } + fallback := upgrade.FallbackPath() + renewed, err := upgrade.RenewFallback(self, fallback) + switch { + case err != nil: + say(fmt.Sprintf("the engine placed by hand at %s is not this build, and could not be renewed: %v", fallback, err)) + case renewed: + say(fmt.Sprintf("renewed the engine placed by hand at %s to this build, %s: the command on this machine is the "+ + "engine that runs it", fallback, version)) + } +} + // The launcher, as the service manager runs it. const ( launcherPath = upgrade.DefaultLibexec + "/launch" diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 3b106ec..02a98c0 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -781,7 +781,7 @@ func ApplyMindingWindows( report.Outcomes = append(report.Outcomes, outcome) if outcome.asFound { // Never quiet (novox/hq issue 339): said on every apply, and stated unhealthy on every look until a - // person hands it over (FoundDirectories, `mesh-host hand-over`). + // person hands it over (FoundDirectories, `nox node hand-over`, novox/hq issue 356). log(fmt.Sprintf(" WARNING %s (%s): %s", outcome.ID, outcome.Target, outcome.Detail)) } if handed { @@ -1047,7 +1047,7 @@ func applyDirectory(r *declaration.Directory, previous store.Applied, wasBefore } if existed && wasBefore { ours := previous.Target != "" && filepath.Clean(previous.Target) == filepath.Clean(r.Path) && !previous.AsFound - // Or handed to the mesh by a person at the machine (`mesh-host hand-over`): then it is given the + // Or handed to the mesh by the operator (`nox node hand-over`, or `hand-over` at the machine): then it is given the // declared owner and mode below, as any directory of the mesh's. if !ours && HandedOver()[handOverKey(r.ID, r.Path)] { ours = true @@ -1069,8 +1069,8 @@ func applyDirectory(r *declaration.Directory, previous store.Applied, wasBefore out.asFoundAt = previous.FoundAt } out.Detail = fmt.Sprintf("found here before the mesh and used as found: its owner %s and mode %o are "+ - "left, though %s were declared (novox/hq issue 339); `mesh-host hand-over %s` at the machine "+ - "hands it to the mesh", ownerText(before), before.Mode().Perm(), out.asFoundDeclared, r.Path) + "left, though %s were declared (novox/hq issue 339); `nox node hand-over %s` on the "+ + "control-node hands it to the mesh", ownerText(before), before.Mode().Perm(), out.asFoundDeclared, r.Path) return out, nil } } @@ -3417,7 +3417,7 @@ func removeFile(a store.Applied, now time.Time) (string, string, error) { "remove it by hand once you know what it is", aside), nil } -// HandedOver is every directory a person handed to the mesh at the machine (`mesh-host hand-over`), by +// HandedOver is every directory the operator handed to the mesh (`nox node hand-over`; `hand-over` at the machine), by // handOverKey: a found directory there is the mesh's, given its declared owner and mode at the next apply // (novox/hq issue 339). Set by the engine to read the file beside its state; a test sets its own. var HandedOver = func() map[string]bool { return nil } diff --git a/internal/link/handover.go b/internal/link/handover.go new file mode 100644 index 0000000..255f939 --- /dev/null +++ b/internal/link/handover.go @@ -0,0 +1,79 @@ +package link + +import ( + "encoding/json" + "fmt" + "strings" +) + +// A hand-over asked over the bus (novox/hq issue 356, issue 339). +// +// A directory used as found is handed to the mesh by its operator, at the controller's terminal: +// `nox node hand-over ` on the control-node (novox/hq ADR 0272). The controller asks this +// node's engine on AskHandOverSubject — a request on core NATS, its own machine's subject, which only the +// controller may publish — and the engine answers once, on the reply the request carried. The engine +// judges every value it is sent and records the hand-over, or refuses and records nothing; the next +// apply gives the directory its declared owner and mode. The controller holds the same two shapes in its +// own link code, with a test on each side holding the field names. + +// AskHandOverSubject is where the controller asks this node's engine to take a directory used as found as +// the mesh's. +func AskHandOverSubject(node string) string { return "mesh.node." + node + ".ask.hand-over" } + +// HandOverAsk is one hand-over asked: the directory's absolute path as the engine states it, and who asked, +// in the controller's words. +type HandOverAsk struct { + Path string `json:"path"` + By string `json:"by"` +} + +// HandOverAnswer is the engine's answer: what it recorded, or why it refused — never both, never neither. +type HandOverAnswer struct { + Said string `json:"said,omitempty"` + Refused string `json:"refused,omitempty"` +} + +// HandOverFunc judges and records one hand-over: the words to answer with, or the refusal. +type HandOverFunc func(path, by string) (string, error) + +// MaxHandOverAsk bounds a request: a path and a name are hundreds of bytes. +const MaxHandOverAsk = 8 << 10 + +// MaxHandOverBy bounds who asked. +const MaxHandOverBy = 200 + +// AnswerHandOver is the engine's judgement of one ask's bytes: every value is checked before anything is +// recorded, and a refusal records nothing. The handler is given only what passed. +func AnswerHandOver(handle HandOverFunc, body []byte) HandOverAnswer { + if handle == nil { + return HandOverAnswer{Refused: "this engine takes no hand-over"} + } + if len(body) > MaxHandOverAsk { + return HandOverAnswer{Refused: fmt.Sprintf("the ask is %d bytes, more than a path and a name; nothing was handed over", + len(body))} + } + var ask HandOverAsk + if err := json.Unmarshal(body, &ask); err != nil { + return HandOverAnswer{Refused: "the ask is not a hand-over: " + err.Error() + "; nothing was handed over"} + } + by := strings.TrimSpace(ask.By) + switch { + case by == "": + return HandOverAnswer{Refused: "the ask names nobody as asking; nothing was handed over"} + case len(by) > MaxHandOverBy || strings.ContainsAny(by, "\r\n\x00"): + return HandOverAnswer{Refused: "who asked is not one short line; nothing was handed over"} + case ask.Path == "": + return HandOverAnswer{Refused: "the ask names no directory; nothing was handed over"} + } + said, err := handle(ask.Path, by) + if err != nil { + return HandOverAnswer{Refused: err.Error()} + } + return HandOverAnswer{Said: said} +} + +// HandOvers is a link that hears the controller asking this node to hand a directory over. Optional, as +// Asked is. +type HandOvers interface { + HearHandOvers(HandOverFunc) +} diff --git a/internal/link/handover_test.go b/internal/link/handover_test.go new file mode 100644 index 0000000..ab272c7 --- /dev/null +++ b/internal/link/handover_test.go @@ -0,0 +1,89 @@ +package link + +import ( + "encoding/json" + "errors" + "sort" + "strings" + "testing" +) + +// The hand-over's ask and answer hold these field names; the controller's side holds the same list +// (mesh-controller internal/link, TestTheHandOverAskAndAnswerKeepTheirFieldNames). +func TestTheHandOverAskAndAnswerKeepTheirFieldNames(t *testing.T) { + body, _ := json.Marshal(HandOverAsk{Path: "/srv/notes", By: "jo through mesh-cli on anchor"}) + if got := keysOf(t, body); got != "by path" { + t.Fatalf("the ask's fields are %q", got) + } + body, _ = json.Marshal(HandOverAnswer{Said: "s", Refused: "r"}) + if got := keysOf(t, body); got != "refused said" { + t.Fatalf("the answer's fields are %q", got) + } + if AskHandOverSubject("laptop") != "mesh.node.laptop.ask.hand-over" { + t.Fatalf("the subject is %q", AskHandOverSubject("laptop")) + } +} + +func keysOf(t *testing.T, body []byte) string { + t.Helper() + var m map[string]any + if err := json.Unmarshal(body, &m); err != nil { + t.Fatal(err) + } + var keys []string + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + return strings.Join(keys, " ") +} + +// Every value of an ask is judged before the handler sees it, and a refusal hands nothing to the handler: +// an ask too large, not a hand-over, naming nobody, naming somebody over more than a line, or naming no +// directory. What passes reaches the handler with its path as given and who asked trimmed. +func TestAHandOverAskIsJudgedBeforeAnythingIsRecorded(t *testing.T) { + var given []string + handle := func(path, by string) (string, error) { + given = append(given, path+" by "+by) + if path == "/refused" { + return "", errors.New("not a directory this machine uses as found") + } + return path + " is handed to the mesh", nil + } + refusals := map[string][]byte{ + "too large": []byte(`{"path":"/` + strings.Repeat("a", MaxHandOverAsk) + `","by":"jo"}`), + "not a hand-over": []byte(`not json`), + "nobody": []byte(`{"path":"/srv/notes","by":" "}`), + "two lines": []byte(`{"path":"/srv/notes","by":"jo\nroot"}`), + "a name too long": []byte(`{"path":"/srv/notes","by":"` + strings.Repeat("x", MaxHandOverBy+1) + `"}`), + "no directory": []byte(`{"path":"","by":"jo"}`), + "a NUL in the name": []byte(`{"path":"/srv/notes","by":"jo\u0000"}`), + } + for name, body := range refusals { + a := AnswerHandOver(handle, body) + if a.Refused == "" || a.Said != "" { + t.Fatalf("%s was answered %+v; want a refusal", name, a) + } + if !strings.Contains(a.Refused, "nothing was handed over") { + t.Fatalf("%s's refusal does not say nothing was handed over: %q", name, a.Refused) + } + } + if len(given) != 0 { + t.Fatalf("the handler was given %v although every ask was refused", given) + } + if a := AnswerHandOver(nil, []byte(`{"path":"/srv/notes","by":"jo"}`)); a.Refused == "" { + t.Fatal("an engine with no handler took a hand-over") + } + + a := AnswerHandOver(handle, []byte(`{"path":"/refused","by":"jo"}`)) + if a.Refused != "not a directory this machine uses as found" || a.Said != "" { + t.Fatalf("the handler's refusal was answered as %+v", a) + } + a = AnswerHandOver(handle, []byte(`{"path":"/srv/notes","by":" jo through mesh-cli on anchor "}`)) + if a.Said != "/srv/notes is handed to the mesh" || a.Refused != "" { + t.Fatalf("answered %+v", a) + } + if strings.Join(given, "; ") != "/refused by jo; /srv/notes by jo through mesh-cli on anchor" { + t.Fatalf("the handler was given %v", given) + } +} diff --git a/internal/link/hearing_nats.go b/internal/link/hearing_nats.go index 712fae1..07708b0 100644 --- a/internal/link/hearing_nats.go +++ b/internal/link/hearing_nats.go @@ -2,6 +2,7 @@ package link import ( "context" + "encoding/json" "errors" "fmt" "strings" @@ -60,6 +61,7 @@ type natsLink struct { sub *nats.Subscription asking *nats.Subscription asked chan struct{} + handing *nats.Subscription node string arrived chan Declaration lost chan error @@ -192,6 +194,31 @@ func (l *natsLink) hearAsks() { } } +// HearHandOvers listens for the controller asking this node to hand a directory used as found to the mesh +// (novox/hq issue 356): a request on this machine's own subject, answered once on the reply it carried. +// The engine judges the ask (AnswerHandOver) and runs nothing it is sent. A bus whose user list is older +// than the grant refuses the subscription asynchronously, said by the client; the operator's line then +// says that nothing on this node answers. +func (l *natsLink) HearHandOvers(handle HandOverFunc) { + if handle == nil { + return + } + handing, err := l.conn.Subscribe(AskHandOverSubject(l.node), func(msg *nats.Msg) { + if msg.Reply == "" { + // Nobody to answer: nothing is handed over on a word nobody waits for. + return + } + body, err := json.Marshal(AnswerHandOver(handle, msg.Data)) + if err != nil { + return + } + _ = msg.Respond(body) + }) + if err == nil { + l.handing = handing + } +} + func (l *natsLink) Declarations() <-chan Declaration { return l.arrived } func (l *natsLink) AskedToReport() <-chan struct{} { return l.asked } func (l *natsLink) Lost() <-chan error { return l.lost } @@ -203,6 +230,9 @@ func (l *natsLink) Close() { if l.asking != nil { _ = l.asking.Unsubscribe() } + if l.handing != nil { + _ = l.handing.Unsubscribe() + } if l.conn != nil { l.conn.Close() } diff --git a/internal/link/messages.go b/internal/link/messages.go index 85a1ca2..7fe19e4 100644 --- a/internal/link/messages.go +++ b/internal/link/messages.go @@ -268,7 +268,7 @@ const KindAccount = "account" // KindDirectory is a directory a module declares that the engine uses as found (novox/hq issue 339): there before // the mesh, with another owner or mode than declared, and left so. Said unhealthy on every look, its reason -// starting ReasonUsedAsFound, until a person hands it to the mesh at the machine (`mesh-host hand-over`). +// starting ReasonUsedAsFound, until the operator hands it to the mesh (`nox node hand-over ` on the control-node, novox/hq issue 356). const KindDirectory = "directory" // ReasonUsedAsFound starts the reason of a directory used as found. diff --git a/internal/link/queue.go b/internal/link/queue.go index 5d938b8..af58f5e 100644 --- a/internal/link/queue.go +++ b/internal/link/queue.go @@ -51,8 +51,11 @@ type Queue struct { // Numbers keeps the report sequence and the refusals counted, across restarts and self-updates. // Nil counts in memory, which is only right where nothing outlives the process — a test. Numbers Numbers - Say Announce - Timeout time.Duration + // HandOver judges and records a directory used as found handed to the mesh by the operator, asked by + // the controller over the link (novox/hq issue 356). Nil answers every such ask with a refusal. + HandOver HandOverFunc + Say Announce + Timeout time.Duration once sync.Once wake chan struct{} diff --git a/internal/link/run.go b/internal/link/run.go index 0d615e4..039d9a9 100644 --- a/internal/link/run.go +++ b/internal/link/run.go @@ -239,6 +239,11 @@ func serve(ctx context.Context, link Link, m Membership, queue *Queue, say Annou if a, ok := link.(Asked); ok { asks = a.AskedToReport() } + // And the controller asking this node to hand a directory used as found to the mesh (novox/hq issue + // 356), answered on the link the question came over. + if h, ok := link.(HandOvers); ok { + h.HearHandOvers(queue.HandOver) + } for { // Asked to stop — by standing aside, say — nothing further is taken in. diff --git a/internal/upgrade/fallback.go b/internal/upgrade/fallback.go new file mode 100644 index 0000000..73a6e0a --- /dev/null +++ b/internal/upgrade/fallback.go @@ -0,0 +1,137 @@ +package upgrade + +import ( + "bytes" + "crypto/sha256" + "errors" + "fmt" + "io" + "os" + "path/filepath" +) + +// The engine placed by hand, kept current (novox/hq issue 356). +// +// The first engine on a machine is copied there by a person (novox/hq ADR 0141): the launcher runs it +// while nothing is delivered, and it is the one `nox-mesh-host` on any PATH. Every engine after it is +// delivered under VersionsDirName and run from there, so the file on PATH fell behind: on 2026-10-09 the +// operator typed its `hand-over` and was answered "unexpected argument" by a build months older than the +// one running the machine. A command that answers for a build that is not running is the lie this +// repository exists to refuse. +// +// So a proved engine (Proved) writes itself over the one placed by hand when the two differ: a plain +// copy, renamed into place — never a link, which the mesh creates none of (novox/hq ADR 0012), and never +// a write through one. The launcher's fallback is then the newest proved build, which is also the best +// one to fall back on. + +// DefaultFallback is where the engine placed by hand lives: the launcher's fallback, and the command. +const DefaultFallback = "/usr/bin/nox-mesh-host" + +// FallbackVar is the environment the launcher reads the fallback's path from; read here too, so the two +// never name different files. +const FallbackVar = "MESH_HOST_BIN" + +// FallbackPath is the engine placed by hand: FallbackVar, or DefaultFallback. +func FallbackPath() string { + if p := os.Getenv(FallbackVar); p != "" { + return p + } + return DefaultFallback +} + +// RenewFallback makes the file at fallback a copy of the executable at self, when it is not one already. +// +// Nothing is written when the two are the same file (an engine running from the fallback itself, while +// nothing is delivered) or hold the same bytes. A fallback that is a link is refused: the mesh writes +// through no link, and a link there would be somebody else's arrangement. Reports whether it was renewed. +func RenewFallback(self, fallback string) (bool, error) { + if self == "" || fallback == "" { + return false, errors.New("the running engine or the fallback has no path") + } + mine, err := os.Stat(self) + if err != nil { + return false, fmt.Errorf("cannot read the running engine at %s: %w", self, err) + } + if !mine.Mode().IsRegular() { + return false, fmt.Errorf("the running engine at %s is not a regular file", self) + } + placed, err := os.Lstat(fallback) + switch { + case errors.Is(err, os.ErrNotExist): + // Nothing placed by hand: the launcher would have nothing to fall back on. Placed now. + case err != nil: + return false, fmt.Errorf("cannot read the engine placed by hand at %s: %w", fallback, err) + case placed.Mode()&os.ModeSymlink != 0: + return false, fmt.Errorf("%s is a link, and the mesh writes through none (novox/hq ADR 0012): "+ + "it is left as it is", fallback) + case !placed.Mode().IsRegular(): + return false, fmt.Errorf("%s is not a regular file: it is left as it is", fallback) + case os.SameFile(mine, placed): + return false, nil + default: + same, err := sameBytes(self, fallback) + if err != nil { + return false, err + } + if same { + return false, nil + } + } + + src, err := os.Open(self) + if err != nil { + return false, err + } + defer src.Close() + tmp, err := os.CreateTemp(filepath.Dir(fallback), "."+filepath.Base(fallback)+"-*") + if err != nil { + return false, fmt.Errorf("cannot write beside %s: %w", fallback, err) + } + defer os.Remove(tmp.Name()) + if _, err := io.Copy(tmp, src); err != nil { + tmp.Close() + return false, err + } + if err := tmp.Sync(); err != nil { + tmp.Close() + return false, err + } + if err := tmp.Close(); err != nil { + return false, err + } + if err := os.Chmod(tmp.Name(), 0o755); err != nil { + return false, err + } + // Renamed, never written in place: the kernel refuses to truncate an executable that runs, and a + // rename leaves whatever runs from the old file untouched. + if err := os.Rename(tmp.Name(), fallback); err != nil { + return false, err + } + return true, nil +} + +// sameBytes says whether two files hold the same bytes, by digest. +func sameBytes(a, b string) (bool, error) { + da, err := digestOf(a) + if err != nil { + return false, err + } + db, err := digestOf(b) + if err != nil { + return false, err + } + return bytes.Equal(da, db), nil +} + +func digestOf(path string) ([]byte, error) { + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return nil, fmt.Errorf("reading %s: %w", path, err) + } + return h.Sum(nil), nil +} diff --git a/internal/upgrade/fallback_test.go b/internal/upgrade/fallback_test.go new file mode 100644 index 0000000..a96a01f --- /dev/null +++ b/internal/upgrade/fallback_test.go @@ -0,0 +1,154 @@ +package upgrade + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// The engine placed by hand becomes a copy of the proved one when the two differ (novox/hq issue 356): the +// command on PATH is the build that runs the machine. +func TestTheFallbackBecomesACopyOfTheProvedEngine(t *testing.T) { + dir := t.TempDir() + self := filepath.Join(dir, VersionsDirName, "v2", BinaryName) + if err := os.MkdirAll(filepath.Dir(self), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(self, []byte("the proved engine"), 0o755); err != nil { + t.Fatal(err) + } + fallback := filepath.Join(dir, "bin", BinaryName) + if err := os.MkdirAll(filepath.Dir(fallback), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(fallback, []byte("the stale engine placed by hand"), 0o644); err != nil { + t.Fatal(err) + } + + renewed, err := RenewFallback(self, fallback) + if err != nil || !renewed { + t.Fatalf("renewed %v, %v", renewed, err) + } + got, err := os.ReadFile(fallback) + if err != nil || string(got) != "the proved engine" { + t.Fatalf("the fallback holds %q, %v", got, err) + } + info, err := os.Stat(fallback) + if err != nil || info.Mode().Perm() != 0o755 { + t.Fatalf("the fallback's mode is %v, %v; want 0755 so it can be run", info.Mode(), err) + } + if _, err := os.Stat(self); err != nil { + t.Fatalf("the running engine was touched: %v", err) + } + + // Already a copy: left alone, so a proof every start does not rewrite it every start. + before, _ := os.Stat(fallback) + renewed, err = RenewFallback(self, fallback) + if err != nil || renewed { + t.Fatalf("a fallback that is already a copy was renewed again: %v, %v", renewed, err) + } + after, _ := os.Stat(fallback) + if !os.SameFile(before, after) { + t.Fatal("a fallback that is already a copy was replaced") + } + if left, _ := filepath.Glob(filepath.Join(filepath.Dir(fallback), "."+BinaryName+"-*")); len(left) > 0 { + t.Fatalf("temporary files left beside the fallback: %v", left) + } +} + +// An engine running from the fallback itself — nothing delivered yet — renews nothing: it would copy a file +// over itself. +func TestAnEngineRunningFromTheFallbackRenewsNothing(t *testing.T) { + dir := t.TempDir() + fallback := filepath.Join(dir, BinaryName) + if err := os.WriteFile(fallback, []byte("placed by hand and running"), 0o755); err != nil { + t.Fatal(err) + } + renewed, err := RenewFallback(fallback, fallback) + if err != nil || renewed { + t.Fatalf("renewed %v, %v", renewed, err) + } +} + +// Nothing placed by hand: the proved engine is placed, so the launcher has a fallback and the machine a command. +func TestAMissingFallbackIsPlaced(t *testing.T) { + dir := t.TempDir() + self := filepath.Join(dir, "v1", BinaryName) + if err := os.MkdirAll(filepath.Dir(self), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(self, []byte("proved"), 0o755); err != nil { + t.Fatal(err) + } + fallback := filepath.Join(dir, BinaryName) + renewed, err := RenewFallback(self, fallback) + if err != nil || !renewed { + t.Fatalf("renewed %v, %v", renewed, err) + } + if got, _ := os.ReadFile(fallback); string(got) != "proved" { + t.Fatalf("placed %q", got) + } +} + +// A link at the fallback's path is refused and left as it is: the mesh creates no link and writes through none +// (novox/hq ADR 0012). The link's target is untouched too. +func TestALinkAtTheFallbackIsRefusedAndLeft(t *testing.T) { + dir := t.TempDir() + self := filepath.Join(dir, "v1", BinaryName) + if err := os.MkdirAll(filepath.Dir(self), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(self, []byte("proved"), 0o755); err != nil { + t.Fatal(err) + } + target := filepath.Join(dir, "somebody-elses") + if err := os.WriteFile(target, []byte("theirs"), 0o755); err != nil { + t.Fatal(err) + } + fallback := filepath.Join(dir, BinaryName) + if err := os.Symlink(target, fallback); err != nil { + t.Fatal(err) + } + renewed, err := RenewFallback(self, fallback) + if err == nil || renewed || !strings.Contains(err.Error(), "link") { + t.Fatalf("a link was taken: %v, %v", renewed, err) + } + if got, _ := os.ReadFile(target); string(got) != "theirs" { + t.Fatalf("the link's target was written: %q", got) + } + if info, err := os.Lstat(fallback); err != nil || info.Mode()&os.ModeSymlink == 0 { + t.Fatalf("the link was replaced: %v %v", info, err) + } +} + +// A running engine that cannot be read is said, not copied as nothing. +func TestAnUnreadableRunningEngineIsSaid(t *testing.T) { + dir := t.TempDir() + fallback := filepath.Join(dir, BinaryName) + if err := os.WriteFile(fallback, []byte("placed"), 0o755); err != nil { + t.Fatal(err) + } + renewed, err := RenewFallback(filepath.Join(dir, "gone"), fallback) + if err == nil || renewed { + t.Fatalf("renewed %v, %v", renewed, err) + } + if got, _ := os.ReadFile(fallback); string(got) != "placed" { + t.Fatalf("the fallback was written from nothing: %q", got) + } + if _, err := RenewFallback("", fallback); err == nil { + t.Fatal("an empty path was taken") + } +} + +// The fallback's path is the launcher's: its environment variable, or the default. +func TestTheFallbackPathIsTheLaunchers(t *testing.T) { + t.Setenv(FallbackVar, "") + if FallbackPath() != DefaultFallback { + t.Fatalf("the default is %q", FallbackPath()) + } + t.Setenv(FallbackVar, "/elsewhere/nox-mesh-host") + if FallbackPath() != "/elsewhere/nox-mesh-host" { + t.Fatalf("the override is %q", FallbackPath()) + } +} -- 2.54.0 From fc6c5a92e605739a88e7969c7c11f328096911dd Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 19:45:00 +0200 Subject: [PATCH 2/4] Believe a hand-over only signed by the mesh, and give it through no link (hq issue 356, review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two holes the review proved. The ask's subject proved nothing — a responder can reach it through a reply — so the engine now verifies the mesh's signature over a fixed context with the key it verifies declarations with, before reading anything, and refuses an ask for another node, expired, good too long, without a nonce, or replayed. And the apply followed a link the hand-over had refused: a component swapped for a link after the record carried root's chmod and chown to its target. A handed-over directory must now still resolve to itself, and is changed only through one descriptor opened from / with no link followed (fchmod, fchown). Tests for the reply guard, the run loop's hook, the fallback renewal on proof and a non-regular fallback; 27 checks each removed in turn, each failing a test. --- cmd/mesh-host/found_test.go | 31 ++-- cmd/mesh-host/witnessing_test.go | 27 ++++ internal/apply/apply.go | 45 +++++- internal/apply/found_is_said_test.go | 98 +++++++++++++ internal/apply/homes_linux.go | 28 ++++ internal/apply/homes_other.go | 7 + internal/link/handover.go | 153 +++++++++++++++++--- internal/link/handover_test.go | 207 +++++++++++++++++++++++---- internal/link/hearing_nats.go | 26 ++-- internal/upgrade/fallback.go | 6 +- internal/upgrade/fallback_test.go | 32 +++++ 11 files changed, 575 insertions(+), 85 deletions(-) diff --git a/cmd/mesh-host/found_test.go b/cmd/mesh-host/found_test.go index 1721bf6..4a13d2c 100644 --- a/cmd/mesh-host/found_test.go +++ b/cmd/mesh-host/found_test.go @@ -143,24 +143,25 @@ func TestAHandOverIsRefusedForEveryPathThatIsNotTheFoundDirectory(t *testing.T) }}); err != nil { t.Fatal(err) } - refused := map[string]string{ - "a relative path": "found", - "a path with ..": dir + "/x/../found", - "a doubled separator": dir + "//found", - "a trailing separator": found + "/", - "a directory not used as found": filepath.Join(dir, "other"), - "a link at the path": linked, - "a link in the path": through, - "a file": file, - "a directory that is gone": gone, + // Each refused by its own check, which the reason names. + refused := map[string][2]string{ + "a relative path": {"found", "not one"}, + "a path with ..": {dir + "/x/../found", "as the engine states it"}, + "a doubled separator": {dir + "//found", "as the engine states it"}, + "a trailing separator": {found + "/", "as the engine states it"}, + "a directory not used as found": {filepath.Join(dir, "other"), "not a directory this machine uses as found"}, + "a link at the path": {linked, "is a link"}, + "a link in the path": {through, "reached through a link"}, + "a file": {file, "is not a directory on this machine"}, + "a directory that is gone": {gone, "cannot be read on this machine"}, } - for name, path := range refused { - said, err := handOverFound(statePath, path, "jo") + for name, c := range refused { + said, err := handOverFound(statePath, c[0], "jo") if err == nil || said != "" { - t.Fatalf("%s (%s) was handed over: %q", name, path, said) + t.Fatalf("%s (%s) was handed over: %q", name, c[0], said) } - if !strings.Contains(err.Error(), "nothing was handed over") { - t.Fatalf("%s's refusal does not say nothing was handed over: %v", name, err) + if !strings.Contains(err.Error(), "nothing was handed over") || !strings.Contains(err.Error(), c[1]) { + t.Fatalf("%s's refusal does not say %q and that nothing was handed over: %v", name, c[1], err) } } if all, err := store.LoadHandedOver(statePath); err != nil || len(all) != 0 { diff --git a/cmd/mesh-host/witnessing_test.go b/cmd/mesh-host/witnessing_test.go index 2fc5622..13ee112 100644 --- a/cmd/mesh-host/witnessing_test.go +++ b/cmd/mesh-host/witnessing_test.go @@ -5,6 +5,7 @@ import ( "os/exec" "path/filepath" "strconv" + "strings" "testing" "time" @@ -17,6 +18,12 @@ import ( func TestOnlyAnAccountUnderThisBuildProvesIt(t *testing.T) { state := filepath.Join(t.TempDir(), "state.json") t.Setenv("MESH_HOST_LIBEXEC", t.TempDir()) + // Never the machine's own engine placed by hand: proving renews it (issue 356). + fallback := filepath.Join(t.TempDir(), "nox-mesh-host") + if err := os.WriteFile(fallback, []byte("a stale engine placed by hand"), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv(upgrade.FallbackVar, fallback) var said []string p := &proving{statePath: state, version: "2.0", say: func(s string) { said = append(said, s) }} @@ -29,6 +36,9 @@ func TestOnlyAnAccountUnderThisBuildProvesIt(t *testing.T) { if !unproved(state, "2.0") { t.Fatalf("%+v proved this build", r) } + if got, _ := os.ReadFile(fallback); string(got) != "a stale engine placed by hand" { + t.Fatalf("%+v renewed the engine placed by hand", r) + } } p.heard(link.Report{Host: "2.0", Declared: "abc"}) if unproved(state, "2.0") { @@ -37,6 +47,23 @@ func TestOnlyAnAccountUnderThisBuildProvesIt(t *testing.T) { if got, _ := upgrade.ReadKnownGood(upgrade.KnownGoodPath(state)); got != "2.0" { t.Fatalf("known-good is %q", got) } + // **Proved, the engine placed by hand becomes this build** (issue 356): the command on the machine's PATH is + // the engine that runs it, and that is said. + self, err := os.Executable() + if err != nil { + t.Fatal(err) + } + want, _ := os.ReadFile(self) + if got, _ := os.ReadFile(fallback); len(want) == 0 || string(got) != string(want) { + t.Fatalf("the engine placed by hand was not renewed to this build (%d bytes, want %d)", len(got), len(want)) + } + renewed := false + for _, s := range said { + renewed = renewed || strings.Contains(s, "renewed the engine placed by hand at "+fallback) + } + if !renewed { + t.Fatalf("the renewal was not said: %q", said) + } } // Every verdict that stands is said, and a reconcile that has one says it unasked. diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 02a98c0..e6c9e1e 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -1047,10 +1047,10 @@ func applyDirectory(r *declaration.Directory, previous store.Applied, wasBefore } if existed && wasBefore { ours := previous.Target != "" && filepath.Clean(previous.Target) == filepath.Clean(r.Path) && !previous.AsFound - // Or handed to the mesh by the operator (`nox node hand-over`, or `hand-over` at the machine): then it is given the - // declared owner and mode below, as any directory of the mesh's. + // Or handed to the mesh by the operator (`nox node hand-over`, or `hand-over` at the machine): then it is + // given the declared owner and mode — through a descriptor that followed no link (giveHandedOver). if !ours && HandedOver()[handOverKey(r.ID, r.Path)] { - ours = true + return applyHandedOver(r, out, mode) } if !ours { owned, err := ownedBy(r.Path, r.Owner) @@ -1122,6 +1122,45 @@ func applyDirectory(r *declaration.Directory, previous store.Applied, wasBefore return out, nil } +// applyHandedOver gives a directory the operator handed to the mesh its declared mode and owner (novox/hq issue +// 356). **No link, checked again at the moment of use**: the hand-over refused a link when it was recorded, and a +// component of the path may have become one since — a parent swapped for a link to /etc would carry root's chmod +// and chown there. So the path must still resolve to itself, and the change is made through one descriptor opened +// from / with no link followed on any component (giveHandedOver); a link anywhere is refused and nothing changes. +func applyHandedOver(r *declaration.Directory, out Outcome, mode os.FileMode) (Outcome, error) { + clean := filepath.Clean(r.Path) + resolved, err := filepath.EvalSymlinks(clean) + if err != nil { + return out, err + } + if resolved != clean { + return out, &PlacementRefusedError{Path: r.Path, Why: fmt.Sprintf("it was handed to the mesh, and is now "+ + "reached through a link (it is %s): the node-engine follows no link to a directory it gives an owner", resolved)} + } + uid, gid, chown := 0, 0, r.Owner != "" + if chown { + if uid, gid, err = idsOf(r.Owner); err != nil { + return out, fmt.Errorf("%s should belong to %q: %w", r.Path, r.Owner, err) + } + } + before, beforeUID, beforeGID, err := giveHandedOver(clean, mode, uid, gid, chown) + if err != nil { + var link *LinkUnderHomeError + if errors.As(err, &link) { + return out, &PlacementRefusedError{Path: r.Path, Why: fmt.Sprintf("it was handed to the mesh, and %s is "+ + "now a link: the node-engine follows no link to a directory it gives an owner", link.Link)} + } + return out, err + } + out.Action = "unchanged" + if before.Perm() != mode.Perm() || (chown && (beforeUID != uid || beforeGID != gid)) { + out.Action = "updated" + out.Detail = fmt.Sprintf("handed to the mesh: owner %d:%d, mode %o, given what was declared", beforeUID, beforeGID, + before.Perm()) + } + return out, nil +} + // applyAccess confirms an operator-owned path is present, and owns nothing about it. // // **The mirror image of applyDirectory** (novox/hq ADR 0051). A directory the host makes, chmods, diff --git a/internal/apply/found_is_said_test.go b/internal/apply/found_is_said_test.go index 3c5a530..5510466 100644 --- a/internal/apply/found_is_said_test.go +++ b/internal/apply/found_is_said_test.go @@ -136,3 +136,101 @@ func TestAHandOverIsForTheResourceThatAskedForIt(t *testing.T) { t.Fatalf("a hand-over for notes.data gave other.data the directory: mode %o", info.Mode().Perm()) } } + +// **A handed-over directory is changed through no link, at the moment of use** (review of issue 356). The +// hand-over refused a link when it was recorded; between then and the apply its parent is swapped for a link to +// a victim tree. The apply refuses, and the victim's directory keeps its mode — and so does a directory whose own +// name became a link, and one whose grandparent did. +func TestAHandedOverDirectoryIsChangedThroughNoLink(t *testing.T) { + for _, swap := range []string{"parent", "grandparent", "itself"} { + t.Run(swap, func(t *testing.T) { + dir, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + found := filepath.Join(dir, "top", "parent", "data") + if err := os.MkdirAll(found, 0o700); err != nil { + t.Fatal(err) + } + decl := `{"id":"notes.data","type":"directory","path":"` + found + `","mode":"0755"}` + _, state, err := Apply(context.Background(), archHost(t), declare(t, decl), store.State{}, store.OriginDeclared, + noServices, nil, nil) + if err != nil { + t.Fatal(err) + } + if a, _ := state.Find("notes.data"); !a.AsFound { + t.Fatalf("not used as found: %+v", a) + } + before := HandedOver + HandedOver = func() map[string]bool { return map[string]bool{handOverKey("notes.data", found): true} } + t.Cleanup(func() { HandedOver = before }) + + // The victim: a tree of the same shape somewhere else, its data directory mode 0700. + victim := filepath.Join(dir, "victim", "top", "parent", "data") + if err := os.MkdirAll(victim, 0o700); err != nil { + t.Fatal(err) + } + var swapped, target string + switch swap { + case "parent": + swapped, target = filepath.Join(dir, "top", "parent"), filepath.Join(dir, "victim", "top", "parent") + case "grandparent": + swapped, target = filepath.Join(dir, "top"), filepath.Join(dir, "victim", "top") + case "itself": + swapped, target = found, victim + } + if err := os.Rename(swapped, swapped+".real"); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, swapped); err != nil { + t.Fatal(err) + } + _, _, err = Apply(context.Background(), archHost(t), declare(t, decl), state, store.OriginDeclared, + noServices, nil, nil) + if err == nil { + t.Fatal("a handed-over directory reached through a link was applied") + } + // Refused by the path's re-check before any descriptor is opened (a link at the name itself is refused + // earlier still, by the directory applier's own look at it). + want := "reached through a link" + if swap == "itself" { + want = "symbolic link" + } + if !strings.Contains(err.Error(), want) { + t.Fatalf("refused as %v; want %q", err, want) + } + if info, _ := os.Stat(victim); info.Mode().Perm() != 0o700 { + t.Fatalf("the victim's directory was given the declared mode through the link: %o", info.Mode().Perm()) + } + }) + } +} + +// The check at the moment of use, without the path's re-check: a component swapped for a link is refused by the +// descriptor walk itself, whatever was resolved a moment before. +func TestGiveHandedOverFollowsNoLink(t *testing.T) { + dir, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + victim := filepath.Join(dir, "victim", "data") + if err := os.MkdirAll(victim, 0o700); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(dir, "victim"), filepath.Join(dir, "parent")); err != nil { + t.Fatal(err) + } + if _, _, _, err := giveHandedOver(filepath.Join(dir, "parent", "data"), 0o755, 0, 0, false); err == nil { + t.Fatal("a directory reached through a link was changed") + } + if info, _ := os.Stat(victim); info.Mode().Perm() != 0o700 { + t.Fatalf("the victim was changed: %o", info.Mode().Perm()) + } + mode, _, _, err := giveHandedOver(victim, 0o750, 0, 0, false) + if err != nil || mode.Perm() != 0o700 { + t.Fatalf("the directory itself: before %o, %v", mode, err) + } + if info, _ := os.Stat(victim); info.Mode().Perm() != 0o750 { + t.Fatalf("the directory itself was not given its mode: %o", info.Mode().Perm()) + } +} diff --git a/internal/apply/homes_linux.go b/internal/apply/homes_linux.go index 58ad05a..79d2844 100644 --- a/internal/apply/homes_linux.go +++ b/internal/apply/homes_linux.go @@ -239,3 +239,31 @@ func writeUnder(home, path string, content []byte, mode os.FileMode) error { } return werr } + +// giveHandedOver gives a directory the operator handed to the mesh its declared mode and owner (novox/hq issue +// 356, issue 339) through one descriptor opened from / with no link followed on any component (openUnderAs with / +// as the home), judged a directory by fstat on that descriptor, and changed with fchmod and fchown on it. The +// hand-over refused a link when it was recorded; this refuses one at the moment of use, so a component swapped for +// a link between the record and the apply cannot carry root's chmod and chown to wherever it points. Returns the +// mode and owner the directory had before. +func giveHandedOver(path string, mode os.FileMode, uid, gid int, chown bool) (before os.FileMode, beforeUID, beforeGID int, err error) { + fd, err := openUnderAs("/", path, kindDir) + if err != nil { + return 0, 0, 0, err + } + defer unix.Close(fd) + var st unix.Stat_t + if err := unix.Fstat(fd, &st); err != nil { + return 0, 0, 0, &os.PathError{Op: "fstat", Path: path, Err: err} + } + before, beforeUID, beforeGID = os.FileMode(st.Mode&0o777), int(st.Uid), int(st.Gid) + if err := unix.Fchmod(fd, uint32(mode.Perm())); err != nil { + return 0, 0, 0, &os.PathError{Op: "chmod", Path: path, Err: err} + } + if chown && (beforeUID != uid || beforeGID != gid) { + if err := unix.Fchown(fd, uid, gid); err != nil { + return 0, 0, 0, &os.PathError{Op: "chown", Path: path, Err: err} + } + } + return before, beforeUID, beforeGID, nil +} diff --git a/internal/apply/homes_other.go b/internal/apply/homes_other.go index 64974f1..9874386 100644 --- a/internal/apply/homes_other.go +++ b/internal/apply/homes_other.go @@ -57,3 +57,10 @@ func writeUnder(home, path string, content []byte, mode os.FileMode) error { } return writeAtomicallyByPath(path, content, mode) } + +// giveHandedOver is refused where there is no openat with O_NOFOLLOW: a handed-over directory is never changed by +// path (novox/hq issue 356). The node-engine runs on Linux. +func giveHandedOver(path string, _ os.FileMode, _, _ int, _ bool) (os.FileMode, int, int, error) { + return 0, 0, 0, &PlacementRefusedError{Path: path, Why: "a handed-over directory is changed only through a " + + "descriptor that followed no link, which this system cannot open"} +} diff --git a/internal/link/handover.go b/internal/link/handover.go index 255f939..aa7cd22 100644 --- a/internal/link/handover.go +++ b/internal/link/handover.go @@ -1,32 +1,63 @@ package link import ( + "crypto/ed25519" + "encoding/hex" "encoding/json" "fmt" "strings" + "sync" + "time" ) // A hand-over asked over the bus (novox/hq issue 356, issue 339). // // A directory used as found is handed to the mesh by its operator, at the controller's terminal: // `nox node hand-over ` on the control-node (novox/hq ADR 0272). The controller asks this -// node's engine on AskHandOverSubject — a request on core NATS, its own machine's subject, which only the -// controller may publish — and the engine answers once, on the reply the request carried. The engine -// judges every value it is sent and records the hand-over, or refuses and records nothing; the next -// apply gives the directory its declared owner and mode. The controller holds the same two shapes in its -// own link code, with a test on each side holding the field names. +// node's engine on AskHandOverSubject, a request on core NATS, and the engine answers once, on the reply +// the request carried. +// +// **The subject proves nothing; the signature does** (review of issue 356). Only the controller is granted +// a publish on the subject, but the bus lets any principal allowed to answer reply to a message it received, +// on whatever reply subject that message named — a tool server asked on its own subject with this reply +// would deliver a body of its choosing here. So the controller signs the ask with the mesh's key, over +// HandOverContext and the ask's bytes, and the engine verifies it with the key it verifies declarations with +// before it reads anything out of it; then it holds the ask to this node, to its expiry, and to a nonce it +// takes once. Then it judges every value and records the hand-over, or refuses and records nothing; the next +// apply gives the directory its declared owner and mode. The controller holds the same shapes in its own link +// code, with a test on each side holding the field names. // AskHandOverSubject is where the controller asks this node's engine to take a directory used as found as // the mesh's. func AskHandOverSubject(node string) string { return "mesh.node." + node + ".ask.hand-over" } -// HandOverAsk is one hand-over asked: the directory's absolute path as the engine states it, and who asked, -// in the controller's words. +// HandOverAsk is one hand-over asked: the node it is for, the directory's absolute path as the engine states +// it, who asked in the controller's words, when it stops being good, and a nonce taken 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 mesh's signature. +type SignedHandOver struct { + Ask []byte `json:"ask"` + Signature []byte `json:"signature"` +} + +// HandOverContext is prefixed to an ask's bytes before signing: a hand-over's signature is never a +// declaration's, which the same key makes over a declaration's bytes alone. +const HandOverContext = "novox-mesh hand-over v1\n" + +// HandOverGood is how long the controller makes an ask good for; HandOverSkew is how far the machines' clocks +// may differ. An ask whose expiry is further ahead than both together is refused: it would be good too long. +const ( + HandOverGood = time.Minute + HandOverSkew = time.Minute +) + // HandOverAnswer is the engine's answer: what it recorded, or why it refused — never both, never neither. type HandOverAnswer struct { Said string `json:"said,omitempty"` @@ -36,34 +67,101 @@ type HandOverAnswer struct { // HandOverFunc judges and records one hand-over: the words to answer with, or the refusal. type HandOverFunc func(path, by string) (string, error) -// MaxHandOverAsk bounds a request: a path and a name are hundreds of bytes. +// MaxHandOverAsk bounds a request: a path, a name, a time, a nonce and a signature are hundreds of bytes. const MaxHandOverAsk = 8 << 10 // MaxHandOverBy bounds who asked. const MaxHandOverBy = 200 -// AnswerHandOver is the engine's judgement of one ask's bytes: every value is checked before anything is -// recorded, and a refusal records nothing. The handler is given only what passed. -func AnswerHandOver(handle HandOverFunc, body []byte) HandOverAnswer { +// HandOverCheck is what an ask is held to: the mesh's key, this node's name, the clock, and the nonces taken. +type HandOverCheck struct { + Signer ed25519.PublicKey + Node string + Now func() time.Time + Nonces *Nonces +} + +// Nonces are the nonces of asks taken, each kept until its ask expires: an ask is taken once. Kept in memory — +// an ask outlives no restart by more than its minute, and taking one again records the same hand-over again. +type Nonces struct { + mu sync.Mutex + seen map[string]time.Time +} + +// take records a nonce until its expiry, and says false when it was already taken. +func (n *Nonces) take(nonce string, expires, now time.Time) bool { + n.mu.Lock() + defer n.mu.Unlock() + if n.seen == nil { + n.seen = map[string]time.Time{} + } + for k, until := range n.seen { + if now.After(until) { + delete(n.seen, k) + } + } + if _, taken := n.seen[nonce]; taken { + return false + } + n.seen[nonce] = expires + return true +} + +// HandOverNonces are this process's: every link it opens takes from the same set, so a reconnect is no replay. +var HandOverNonces = &Nonces{} + +func refusedAsk(why string) HandOverAnswer { + return HandOverAnswer{Refused: why + "; nothing was handed over"} +} + +// AnswerHandOver is the engine's judgement of one ask's bytes: the signature first, then this node, the expiry +// and the nonce, then every value — and only then the handler, given only what passed. A refusal records nothing. +func AnswerHandOver(check HandOverCheck, handle HandOverFunc, body []byte) HandOverAnswer { if handle == nil { - return HandOverAnswer{Refused: "this engine takes no hand-over"} + return refusedAsk("this engine takes no hand-over") } if len(body) > MaxHandOverAsk { - return HandOverAnswer{Refused: fmt.Sprintf("the ask is %d bytes, more than a path and a name; nothing was handed over", - len(body))} + return refusedAsk(fmt.Sprintf("the ask is %d bytes, more than a hand-over", len(body))) + } + var signed SignedHandOver + if err := json.Unmarshal(body, &signed); err != nil { + return refusedAsk("the ask is not a signed hand-over: " + err.Error()) + } + if len(check.Signer) != ed25519.PublicKeySize || + !ed25519.Verify(check.Signer, append([]byte(HandOverContext), signed.Ask...), signed.Signature) { + return refusedAsk("the ask is not signed by the mesh") } var ask HandOverAsk - if err := json.Unmarshal(body, &ask); err != nil { - return HandOverAnswer{Refused: "the ask is not a hand-over: " + err.Error() + "; nothing was handed over"} + if err := json.Unmarshal(signed.Ask, &ask); err != nil { + return refusedAsk("the signed ask is not a hand-over: " + err.Error()) + } + now := time.Now() + if check.Now != nil { + now = check.Now() + } + nonces := check.Nonces + if nonces == nil { + nonces = HandOverNonces } by := strings.TrimSpace(ask.By) + nonce, nonceErr := hex.DecodeString(ask.Nonce) switch { + case check.Node == "" || ask.Node != check.Node: + return refusedAsk(fmt.Sprintf("the ask is for %q, and this is %q", ask.Node, check.Node)) + case ask.Expires.IsZero() || !now.Before(ask.Expires): + return refusedAsk("the ask has expired") + case ask.Expires.After(now.Add(HandOverGood + HandOverSkew)): + return refusedAsk("the ask is good for longer than an ask may be") + case nonceErr != nil || len(nonce) < 16: + return refusedAsk("the ask carries no nonce of its own") case by == "": - return HandOverAnswer{Refused: "the ask names nobody as asking; nothing was handed over"} + return refusedAsk("the ask names nobody as asking") case len(by) > MaxHandOverBy || strings.ContainsAny(by, "\r\n\x00"): - return HandOverAnswer{Refused: "who asked is not one short line; nothing was handed over"} + return refusedAsk("who asked is not one short line") case ask.Path == "": - return HandOverAnswer{Refused: "the ask names no directory; nothing was handed over"} + return refusedAsk("the ask names no directory") + case !nonces.take(ask.Nonce, ask.Expires, now): + return refusedAsk("the ask was taken already") } said, err := handle(ask.Path, by) if err != nil { @@ -72,6 +170,19 @@ func AnswerHandOver(handle HandOverFunc, body []byte) HandOverAnswer { return HandOverAnswer{Said: said} } +// HandOverReply is what a hand-over request is answered with, and whether to answer: a request with no reply +// subject is nobody's question, so nothing is judged, recorded or said. +func HandOverReply(check HandOverCheck, handle HandOverFunc, reply string, body []byte) ([]byte, bool) { + if reply == "" { + return nil, false + } + answer, err := json.Marshal(AnswerHandOver(check, handle, body)) + if err != nil { + return nil, false + } + return answer, true +} + // HandOvers is a link that hears the controller asking this node to hand a directory over. Optional, as // Asked is. type HandOvers interface { diff --git a/internal/link/handover_test.go b/internal/link/handover_test.go index ab272c7..eaa284f 100644 --- a/internal/link/handover_test.go +++ b/internal/link/handover_test.go @@ -1,20 +1,27 @@ package link import ( + "context" + "crypto/ed25519" "encoding/json" "errors" "sort" "strings" "testing" + "time" ) // The hand-over's ask and answer hold these field names; the controller's side holds the same list // (mesh-controller internal/link, TestTheHandOverAskAndAnswerKeepTheirFieldNames). func TestTheHandOverAskAndAnswerKeepTheirFieldNames(t *testing.T) { - body, _ := json.Marshal(HandOverAsk{Path: "/srv/notes", By: "jo through mesh-cli on anchor"}) - if got := keysOf(t, body); got != "by path" { + body, _ := json.Marshal(HandOverAsk{Node: "laptop", Path: "/srv/notes", By: "jo", Expires: time.Now(), Nonce: "n"}) + if got := keysOf(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 := keysOf(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 := keysOf(t, body); got != "refused said" { t.Fatalf("the answer's fields are %q", got) @@ -22,6 +29,9 @@ func TestTheHandOverAskAndAnswerKeepTheirFieldNames(t *testing.T) { if AskHandOverSubject("laptop") != "mesh.node.laptop.ask.hand-over" { t.Fatalf("the subject is %q", AskHandOverSubject("laptop")) } + if HandOverContext != "novox-mesh hand-over v1\n" { + t.Fatalf("the signing context is %q; the controller holds the same words", HandOverContext) + } } func keysOf(t *testing.T, body []byte) string { @@ -38,52 +48,193 @@ func keysOf(t *testing.T, body []byte) string { return strings.Join(keys, " ") } -// Every value of an ask is judged before the handler sees it, and a refusal hands nothing to the handler: -// an ask too large, not a hand-over, naming nobody, naming somebody over more than a line, or naming no -// directory. What passes reaches the handler with its path as given and who asked trimmed. -func TestAHandOverAskIsJudgedBeforeAnythingIsRecorded(t *testing.T) { +const aNonce = "00112233445566778899aabbccddeeff" + +// signedAsk is an ask as the controller makes it, signed with key over the context and its bytes. +func signedAsk(t *testing.T, key ed25519.PrivateKey, ask HandOverAsk) []byte { + t.Helper() + raw, err := json.Marshal(ask) + if err != nil { + t.Fatal(err) + } + body, err := json.Marshal(SignedHandOver{Ask: raw, Signature: ed25519.Sign(key, append([]byte(HandOverContext), raw...))}) + if err != nil { + t.Fatal(err) + } + return body +} + +// anAsk is a good ask at now. +func anAsk(now time.Time) HandOverAsk { + return HandOverAsk{Node: "n1", Path: "/srv/notes", By: " jo through mesh-cli on anchor ", Expires: now.Add(HandOverGood), + Nonce: aNonce} +} + +// **An ask is believed only signed by the mesh, for this node, unexpired, once** (review of issue 356): a +// responder can reach the subject through a reply, so the subject proves nothing. Each refusal below hands +// the handler nothing; the good ask reaches it with its path and who asked, and the same ask again is refused. +func TestAHandOverAskIsBelievedOnlySignedForThisNodeUnexpiredAndOnce(t *testing.T) { + m, key := aMember(t) + _, stranger, _ := ed25519.GenerateKey(nil) + now := time.Date(2026, 10, 9, 18, 0, 0, 0, time.UTC) var given []string handle := func(path, by string) (string, error) { given = append(given, path+" by "+by) if path == "/refused" { - return "", errors.New("not a directory this machine uses as found") + return "", errors.New("not a directory this machine uses as found; nothing was handed over") } return path + " is handed to the mesh", nil } + check := HandOverCheck{Signer: m.Signer, Node: m.Node, Now: func() time.Time { return now }, Nonces: &Nonces{}} + with := func(change func(*HandOverAsk)) HandOverAsk { + a := anAsk(now) + change(&a) + return a + } + good, _ := json.Marshal(anAsk(now)) + unsignedBody, _ := json.Marshal(SignedHandOver{Ask: good}) + // Signed as a declaration is, over the bytes alone: not a hand-over's signature. + asDeclaration, _ := json.Marshal(SignedHandOver{Ask: good, Signature: ed25519.Sign(key, good)}) refusals := map[string][]byte{ - "too large": []byte(`{"path":"/` + strings.Repeat("a", MaxHandOverAsk) + `","by":"jo"}`), - "not a hand-over": []byte(`not json`), - "nobody": []byte(`{"path":"/srv/notes","by":" "}`), - "two lines": []byte(`{"path":"/srv/notes","by":"jo\nroot"}`), - "a name too long": []byte(`{"path":"/srv/notes","by":"` + strings.Repeat("x", MaxHandOverBy+1) + `"}`), - "no directory": []byte(`{"path":"","by":"jo"}`), - "a NUL in the name": []byte(`{"path":"/srv/notes","by":"jo\u0000"}`), + "too large": signedAsk(t, key, with(func(a *HandOverAsk) { a.Path = "/" + strings.Repeat("a", MaxHandOverAsk) })), + "not a signed hand-over": []byte(`not json`), + "unsigned": unsignedBody, + "signed by another key": signedAsk(t, stranger, anAsk(now)), + "signed as a declaration": asDeclaration, + "the bare ask": good, + "for another node": signedAsk(t, key, with(func(a *HandOverAsk) { a.Node = "n2" })), + "expired": signedAsk(t, key, with(func(a *HandOverAsk) { a.Expires = now })), + "no expiry": signedAsk(t, key, with(func(a *HandOverAsk) { a.Expires = time.Time{} })), + "good for too long": signedAsk(t, key, with(func(a *HandOverAsk) { a.Expires = now.Add(HandOverGood + HandOverSkew + time.Second) })), + "no nonce": signedAsk(t, key, with(func(a *HandOverAsk) { a.Nonce = "" })), + "a short nonce": signedAsk(t, key, with(func(a *HandOverAsk) { a.Nonce = "0011" })), + "nobody": signedAsk(t, key, with(func(a *HandOverAsk) { a.By = " " })), + "two lines": signedAsk(t, key, with(func(a *HandOverAsk) { a.By = "jo\nroot" })), + "a name too long": signedAsk(t, key, with(func(a *HandOverAsk) { a.By = strings.Repeat("x", MaxHandOverBy+1) })), + "a NUL in the name": signedAsk(t, key, with(func(a *HandOverAsk) { a.By = "jo\x00" })), + "no directory": signedAsk(t, key, with(func(a *HandOverAsk) { a.Path = "" })), } for name, body := range refusals { - a := AnswerHandOver(handle, body) - if a.Refused == "" || a.Said != "" { - t.Fatalf("%s was answered %+v; want a refusal", name, a) - } - if !strings.Contains(a.Refused, "nothing was handed over") { - t.Fatalf("%s's refusal does not say nothing was handed over: %q", name, a.Refused) + a := AnswerHandOver(check, handle, body) + if a.Refused == "" || a.Said != "" || !strings.Contains(a.Refused, "nothing was handed over") { + t.Fatalf("%s was answered %+v; want a refusal saying nothing was handed over", name, a) } } if len(given) != 0 { t.Fatalf("the handler was given %v although every ask was refused", given) } - if a := AnswerHandOver(nil, []byte(`{"path":"/srv/notes","by":"jo"}`)); a.Refused == "" { + if a := AnswerHandOver(HandOverCheck{Node: "n1", Nonces: &Nonces{}}, handle, signedAsk(t, key, anAsk(time.Now()))); a.Refused == "" { + t.Fatal("an engine with no key believed an ask") + } + if a := AnswerHandOver(check, nil, signedAsk(t, key, anAsk(now))); a.Refused == "" { t.Fatal("an engine with no handler took a hand-over") } - a := AnswerHandOver(handle, []byte(`{"path":"/refused","by":"jo"}`)) - if a.Refused != "not a directory this machine uses as found" || a.Said != "" { - t.Fatalf("the handler's refusal was answered as %+v", a) - } - a = AnswerHandOver(handle, []byte(`{"path":"/srv/notes","by":" jo through mesh-cli on anchor "}`)) + body := signedAsk(t, key, anAsk(now)) + a := AnswerHandOver(check, handle, body) if a.Said != "/srv/notes is handed to the mesh" || a.Refused != "" { - t.Fatalf("answered %+v", a) + t.Fatalf("the good ask was answered %+v", a) } - if strings.Join(given, "; ") != "/refused by jo; /srv/notes by jo through mesh-cli on anchor" { + if again := AnswerHandOver(check, handle, body); !strings.Contains(again.Refused, "taken already") { + t.Fatalf("the same ask again was answered %+v", again) + } + refused := signedAsk(t, key, with(func(a *HandOverAsk) { a.Path = "/refused"; a.Nonce = strings.Repeat("ab", 16) })) + if a := AnswerHandOver(check, handle, refused); !strings.HasPrefix(a.Refused, "not a directory") { + t.Fatalf("the handler's refusal was answered %+v", a) + } + if strings.Join(given, "; ") != "/srv/notes by jo through mesh-cli on anchor; /refused by jo through mesh-cli on anchor" { t.Fatalf("the handler was given %v", given) } + // A nonce is forgotten once its ask has expired, so the set does not grow for ever. + now = now.Add(HandOverGood + time.Second) + check.Nonces.take("ff", now.Add(time.Second), now) + if len(check.Nonces.seen) != 1 { + t.Fatalf("expired nonces are kept: %v", check.Nonces.seen) + } +} + +// **A request with no reply is nobody's question**: nothing is judged, recorded or said. One with a reply is +// answered with the judgement. +func TestAHandOverIsAnsweredOnlyWhenItCarriesAReply(t *testing.T) { + m, key := aMember(t) + now := time.Now() + called := 0 + handle := func(string, string) (string, error) { called++; return "handed", nil } + check := HandOverCheck{Signer: m.Signer, Node: m.Node, Nonces: &Nonces{}} + if body, answer := HandOverReply(check, handle, "", signedAsk(t, key, anAsk(now))); answer || body != nil || called != 0 { + t.Fatalf("a request with no reply was answered %q %v, handled %d", body, answer, called) + } + body, answer := HandOverReply(check, handle, "_INBOX.x", signedAsk(t, key, anAsk(now))) + var said HandOverAnswer + if !answer || json.Unmarshal(body, &said) != nil || said.Said != "handed" || called != 1 { + t.Fatalf("answered %q %v, handled %d", body, answer, called) + } +} + +// handingLink is a quiet link that hears hand-overs, keeping the handler it was given. +type handingLink struct { + *quietLink + given chan HandOverFunc +} + +func (l handingLink) HearHandOvers(h HandOverFunc) { l.given <- h } + +// **The link is given the queue's hand-over** when it is served: what the controller asks reaches the engine's +// judgement, and nothing else. +func TestALinkThatHearsHandOversIsGivenTheQueues(t *testing.T) { + m, _ := aMember(t) + l := handingLink{quietLink: newQuietLink(), given: make(chan HandOverFunc, 1)} + q := aQueue(m, (&applies{}).apply, &keptInMemory{}) + q.HandOver = func(path, by string) (string, error) { return "the queue's: " + path + " by " + by, nil } + ctx, stop := context.WithCancel(context.Background()) + served := make(chan error, 1) + go func() { served <- serve(ctx, l, m, q, nil, time.Second) }() + select { + case h := <-l.given: + if said, err := h("/srv/notes", "jo"); err != nil || said != "the queue's: /srv/notes by jo" { + t.Fatalf("the link was given another handler: %q %v", said, err) + } + case <-time.After(5 * time.Second): + t.Fatal("the link was never given the queue's hand-over") + } + stop() + <-served +} + +// **Against a real server**: a signed request on the node's own subject is answered; one on another node's is +// not heard; a message with no reply is answered by nobody. +func TestNatsAHandOverIsAskedOfTheNode(t *testing.T) { + conn, js := aBus(t) + m, key := aMember(t) + l := &natsLink{conn: conn, js: js, node: m.Node, signer: m.Signer} + handled := make(chan string, 4) + l.HearHandOvers(func(path, by string) (string, error) { handled <- path; return "handed " + path, nil }) + defer l.Close() + if err := conn.Flush(); err != nil { + t.Fatal(err) + } + ask := anAsk(time.Now()) + ask.Nonce = strings.Repeat("cd", 16) + reply, err := conn.Request(AskHandOverSubject(m.Node), signedAsk(t, key, ask), 5*time.Second) + if err != nil { + t.Fatal(err) + } + var said HandOverAnswer + if err := json.Unmarshal(reply.Data, &said); err != nil || said.Said != "handed /srv/notes" { + t.Fatalf("answered %s, %v", reply.Data, err) + } + ask.Nonce = strings.Repeat("ef", 16) + if err := conn.Publish(AskHandOverSubject(m.Node), signedAsk(t, key, ask)); err != nil { + t.Fatal(err) + } + if _, err := conn.Request(AskHandOverSubject("another"), signedAsk(t, key, ask), 300*time.Millisecond); err == nil { + t.Fatal("another node's ask was answered here") + } + if err := conn.Flush(); err != nil { + t.Fatal(err) + } + time.Sleep(100 * time.Millisecond) + if len(handled) != 1 { + t.Fatalf("handled %d asks; want only the one with a reply", len(handled)) + } } diff --git a/internal/link/hearing_nats.go b/internal/link/hearing_nats.go index 07708b0..1466a33 100644 --- a/internal/link/hearing_nats.go +++ b/internal/link/hearing_nats.go @@ -2,7 +2,7 @@ package link import ( "context" - "encoding/json" + "crypto/ed25519" "errors" "fmt" "strings" @@ -63,6 +63,8 @@ type natsLink struct { asked chan struct{} handing *nats.Subscription node string + // signer is the mesh's key, which a hand-over ask is verified with as a declaration is (issue 356). + signer ed25519.PublicKey arrived chan Declaration lost chan error } @@ -112,7 +114,7 @@ func dialNats(ctx context.Context, m Membership, timeout time.Duration) (Link, e } l := &natsLink{ - conn: conn, js: js, node: m.Node, + conn: conn, js: js, node: m.Node, signer: m.Signer, arrived: make(chan Declaration, drainDepth), lost: make(chan error, 1), asked: make(chan struct{}, 1), @@ -195,24 +197,20 @@ func (l *natsLink) hearAsks() { } // HearHandOvers listens for the controller asking this node to hand a directory used as found to the mesh -// (novox/hq issue 356): a request on this machine's own subject, answered once on the reply it carried. -// The engine judges the ask (AnswerHandOver) and runs nothing it is sent. A bus whose user list is older -// than the grant refuses the subscription asynchronously, said by the client; the operator's line then -// says that nothing on this node answers. +// (novox/hq issue 356): a request on this machine's own subject, answered once on the reply it carried. The +// ask is verified with the mesh's key before anything is read out of it (AnswerHandOver), because a responder +// can reach this subject through a reply; the engine runs nothing it is sent. A bus whose user list is older +// than the grant refuses the subscription asynchronously, said by the client; the operator's line then says +// that nothing on this node answers. func (l *natsLink) HearHandOvers(handle HandOverFunc) { if handle == nil { return } + check := HandOverCheck{Signer: l.signer, Node: l.node} handing, err := l.conn.Subscribe(AskHandOverSubject(l.node), func(msg *nats.Msg) { - if msg.Reply == "" { - // Nobody to answer: nothing is handed over on a word nobody waits for. - return + if body, answer := HandOverReply(check, handle, msg.Reply, msg.Data); answer { + _ = msg.Respond(body) } - body, err := json.Marshal(AnswerHandOver(handle, msg.Data)) - if err != nil { - return - } - _ = msg.Respond(body) }) if err == nil { l.handing = handing diff --git a/internal/upgrade/fallback.go b/internal/upgrade/fallback.go index 73a6e0a..edd0bdd 100644 --- a/internal/upgrade/fallback.go +++ b/internal/upgrade/fallback.go @@ -41,8 +41,8 @@ func FallbackPath() string { // RenewFallback makes the file at fallback a copy of the executable at self, when it is not one already. // -// Nothing is written when the two are the same file (an engine running from the fallback itself, while -// nothing is delivered) or hold the same bytes. A fallback that is a link is refused: the mesh writes +// Nothing is written when the two hold the same bytes — which an engine running from the fallback itself, while +// nothing is delivered, always does. A fallback that is a link is refused: the mesh writes // through no link, and a link there would be somebody else's arrangement. Reports whether it was renewed. func RenewFallback(self, fallback string) (bool, error) { if self == "" || fallback == "" { @@ -66,8 +66,6 @@ func RenewFallback(self, fallback string) (bool, error) { "it is left as it is", fallback) case !placed.Mode().IsRegular(): return false, fmt.Errorf("%s is not a regular file: it is left as it is", fallback) - case os.SameFile(mine, placed): - return false, nil default: same, err := sameBytes(self, fallback) if err != nil { diff --git a/internal/upgrade/fallback_test.go b/internal/upgrade/fallback_test.go index a96a01f..a738b47 100644 --- a/internal/upgrade/fallback_test.go +++ b/internal/upgrade/fallback_test.go @@ -152,3 +152,35 @@ func TestTheFallbackPathIsTheLaunchers(t *testing.T) { t.Fatalf("the override is %q", FallbackPath()) } } + +// A fallback that is not a regular file — a directory where the engine should be — is refused and left: the mesh +// replaces a file, never something else at its name. +func TestAFallbackThatIsNotARegularFileIsRefused(t *testing.T) { + dir := t.TempDir() + self := filepath.Join(dir, "v1", BinaryName) + if err := os.MkdirAll(filepath.Dir(self), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(self, []byte("proved"), 0o755); err != nil { + t.Fatal(err) + } + fallback := filepath.Join(dir, BinaryName) + if err := os.Mkdir(fallback, 0o755); err != nil { + t.Fatal(err) + } + renewed, err := RenewFallback(self, fallback) + if err == nil || renewed || !strings.Contains(err.Error(), "not a regular file") { + t.Fatalf("a directory at the fallback was taken: %v, %v", renewed, err) + } + if info, err := os.Lstat(fallback); err != nil || !info.IsDir() { + t.Fatalf("the directory was replaced: %v %v", info, err) + } + // And a running engine that is not a regular file is refused before anything is written. + if _, err := RenewFallback(dir, filepath.Join(dir, "elsewhere")); err == nil || !strings.Contains(err.Error(), + "not a regular file") { + t.Fatalf("a directory was taken as the running engine: %v", err) + } + if _, err := os.Lstat(filepath.Join(dir, "elsewhere")); err == nil { + t.Fatal("something was placed from a directory") + } +} -- 2.54.0 From 9a807de65960e41cbb2db84b41fd3031ff1cb2ae Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 19:58:27 +0200 Subject: [PATCH 3/4] Give a handed-over directory through no link on every apply, not only the first (hq issue 356, review) After the first apply the record is no longer used as found, so the directory read as the mesh's own and later applies set its mode and owner by path: a parent swapped for a link after the hand-over took carried root's chmod and chown to its target. The hand-over is now checked before that shortcut, so a handed-over directory always goes through applyHandedOver. The test swaps the parent and the grandparent after an apply already gave the directory its mode; both cases fail with the old order. --- internal/apply/apply.go | 13 ++++++++----- internal/apply/found_is_said_test.go | 25 +++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/internal/apply/apply.go b/internal/apply/apply.go index e6c9e1e..e5ff726 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -1045,13 +1045,16 @@ func applyDirectory(r *declaration.Directory, previous store.Applied, wasBefore if existed && !before.IsDir() { return out, fmt.Errorf("%s exists and is not a directory", r.Path) } + // Handed to the mesh by the operator (`nox node hand-over`, or `hand-over` at the machine): given the declared + // owner and mode through a descriptor that followed no link (giveHandedOver) — **on every apply, not only the + // first** (review of issue 356). After the first, the record is no longer used as found and would read as the + // mesh's own, whose mode and owner are set by path; a parent swapped for a link after that would carry root's + // chmod and chown to its target. + if existed && HandedOver()[handOverKey(r.ID, r.Path)] { + return applyHandedOver(r, out, mode) + } if existed && wasBefore { ours := previous.Target != "" && filepath.Clean(previous.Target) == filepath.Clean(r.Path) && !previous.AsFound - // Or handed to the mesh by the operator (`nox node hand-over`, or `hand-over` at the machine): then it is - // given the declared owner and mode — through a descriptor that followed no link (giveHandedOver). - if !ours && HandedOver()[handOverKey(r.ID, r.Path)] { - return applyHandedOver(r, out, mode) - } if !ours { owned, err := ownedBy(r.Path, r.Owner) if err != nil { diff --git a/internal/apply/found_is_said_test.go b/internal/apply/found_is_said_test.go index 5510466..5155cce 100644 --- a/internal/apply/found_is_said_test.go +++ b/internal/apply/found_is_said_test.go @@ -142,8 +142,16 @@ func TestAHandOverIsForTheResourceThatAskedForIt(t *testing.T) { // a victim tree. The apply refuses, and the victim's directory keeps its mode — and so does a directory whose own // name became a link, and one whose grandparent did. func TestAHandedOverDirectoryIsChangedThroughNoLink(t *testing.T) { - for _, swap := range []string{"parent", "grandparent", "itself"} { - t.Run(swap, func(t *testing.T) { + for _, c := range []struct { + swap string + applyOnce bool + }{{"parent", false}, {"grandparent", false}, {"itself", false}, {"parent", true}, {"grandparent", true}} { + swap, applyOnce := c.swap, c.applyOnce + name := swap + if applyOnce { + name += " after an apply gave it its owner" + } + t.Run(name, func(t *testing.T) { dir, err := filepath.EvalSymlinks(t.TempDir()) if err != nil { t.Fatal(err) @@ -164,6 +172,19 @@ func TestAHandedOverDirectoryIsChangedThroughNoLink(t *testing.T) { before := HandedOver HandedOver = func() map[string]bool { return map[string]bool{handOverKey("notes.data", found): true} } t.Cleanup(func() { HandedOver = before }) + if applyOnce { + // The hand-over takes: the directory is given its mode, and the record is no longer as found. + if _, state, err = Apply(context.Background(), archHost(t), declare(t, decl), state, store.OriginDeclared, + noServices, nil, nil); err != nil { + t.Fatal(err) + } + if a, _ := state.Find("notes.data"); a.AsFound { + t.Fatalf("still as found after the hand-over took: %+v", a) + } + if info, _ := os.Stat(found); info.Mode().Perm() != 0o755 { + t.Fatalf("the hand-over did not take: %o", info.Mode().Perm()) + } + } // The victim: a tree of the same shape somewhere else, its data directory mode 0700. victim := filepath.Join(dir, "victim", "top", "parent", "data") -- 2.54.0 From 8fc1d72cd546817dba02a089608016cff6a5feb8 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 20:04:11 +0200 Subject: [PATCH 4/4] Keep the setuid search's time on the caller's clock SetuidCache compared the caller's now with a time read from the wall clock when the search finished. When the caller's clock ran behind the wall clock, a search was never due again: TestTheSetuidSearchRunsInTheBackgroundAndIsKeptForItsInterval, whose clock is a fixed date, has failed on main and every branch since 2026-10-09 18:00 UTC. A search's time is now the now it was started at; a result reads at most one search older than it is, which errs towards searching again. --- internal/accounts/ways.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/internal/accounts/ways.go b/internal/accounts/ways.go index 9aa525c..122b0cf 100644 --- a/internal/accounts/ways.go +++ b/internal/accounts/ways.go @@ -390,7 +390,7 @@ type SetuidCache struct { mu sync.Mutex wg sync.WaitGroup - at time.Time // when the last search that finished finished + at time.Time // when the last search that finished was started, on the caller's clock found []string running bool startedAt time.Time @@ -431,8 +431,14 @@ const ( ReasonIncomplete = rootsearch.ReasonIncomplete ) -// Look answers the last result and when its search finished, starting a search in the background when one is +// Look answers the last result and when its search was started, starting a search in the background when one is // due; it never waits for one. Nil c searches in place, for a caller that wants to. +// +// **One clock: the caller's.** A search's time is the `now` it was started at, never the wall clock read when it +// finished: due-ness compares a later `now` with it, and mixing the two clocks made a search never due again +// whenever the caller's clock ran behind the wall clock — the test with a fixed date failed on every branch from +// 2026-10-09 18:00 UTC (found while checking issue 356). Measured from the start, a result reads at most one +// search older than it is, which errs towards searching again. func (c *SetuidCache) Look(now time.Time, search func(ctx context.Context) ([]string, error)) ([]string, time.Time, error) { if c == nil { ctx, cancel := context.WithTimeout(context.Background(), SearchBound) @@ -446,6 +452,7 @@ func (c *SetuidCache) Look(now time.Time, search func(ctx context.Context) ([]st (c.failed == nil || now.Sub(c.failedAt) >= c.backoff) if due { c.running, c.startedAt = true, now + started := now c.wg.Add(1) go func() { defer c.wg.Done() @@ -455,13 +462,12 @@ func (c *SetuidCache) Look(now time.Time, search func(ctx context.Context) ([]st c.mu.Lock() defer c.mu.Unlock() c.running = false - done := time.Now() if err != nil { - c.failed, c.failedAt = err, done + c.failed, c.failedAt = err, started c.backoff = min(max(2*c.backoff, c.Every), MaxBackoff) return } - c.found, c.at, c.failed, c.backoff = found, done, nil, 0 + c.found, c.at, c.failed, c.backoff = found, started, nil, 0 }() } switch { -- 2.54.0