diff --git a/modules/mounts/README.md b/modules/mounts/README.md index a9d56837..063f61bf 100644 --- a/modules/mounts/README.md +++ b/modules/mounts/README.md @@ -83,8 +83,33 @@ password: the options, fstab lines and mountinfo the tools read are redacted. for a node's share, 445 for an SMB source, the device for a disk — and says each one's state. - The bundle (the operator account) reads what the sharing machine exports from `nfs-server`'s state `exports`, keyed by that machine, and leaves it for `apply`; a failed read keeps the last answer. It puts - what `watch` found in the module's state `shares`, keyed by this machine. No module reads another's - files (ADR 0260). + what `watch` found in the module's state `shares`, keyed by this machine, with `wants`: each share this + machine asks of which server and whether read-write or read-only (`[{"server", "share", "access"}]`). + The server exports a share only to the machines that want it and that it grants, and decides the access + itself. Every five minutes the bundle also asks the controller's `data` verb where the mesh keeps data on + this machine, and leaves the paths for `apply`; a failed ask keeps the last paths. No module reads + another's files (ADR 0260). + +## What it refuses + +Each refusal is said as the share's state `failed`, with why, and nothing is written for it: + +- a mount point at, under or above a path where the mesh keeps data on this machine (the controller's + `data` verb: Plex's library and configuration, a backup repository, an agent's home). Until that list + has been read once, nothing new is set up; a pair the module already has stays; +- a mount point the system owns (`/`, `/etc`, `/usr`, `/boot`, `/proc`, `/sys`, `/dev`, `/run` and what + is under them, and `/var`, `/home`, `/root`, `/tmp` themselves); +- a mount point that something else is mounted at, or an automount that is not the module's; +- a folder that holds files: a mount would hide them. Only a folder not in the kernel's mount table is + opened, so a server that is away cannot hold the look; +- a share that this machine serves itself. + +And what it never does: + +- remove a pair while an entry of `shares` or `sources` cannot be read: nobody can tell which mount point + the mistyped entry meant, so every pair stays until the settings read again; +- stop or remove a pair whose mount point has a filesystem mounted that is not the pair's own source; the + verb `unmount` refuses the same. The states: `armed` (the automount waits), `mounted`, `absent` (an occasional source that is not there — a state, not a fault), `unreachable` (should be there and is not), `failed` (tried and refused, or not diff --git a/modules/mounts/cmd/mounts/adopt.go b/modules/mounts/cmd/mounts/adopt.go index 302f63ea..b0ec8e37 100644 --- a/modules/mounts/cmd/mounts/adopt.go +++ b/modules/mounts/cmd/mounts/adopt.go @@ -23,11 +23,16 @@ import ( // AdoptedPrefix opens a line the module took over. const AdoptedPrefix = "# adopted by the mesh (module mounts) on " -var passwordValue = regexp.MustCompile(`(?i)\b(password2?|pass|passwd)=[^,\s]*`) +// A password's value runs to the next single comma or blank: mount.cifs writes a comma inside it as two. +var passwordValue = regexp.MustCompile(`(?i)\b(password2?|pass|passwd)=(?:,,+[^,\s]*|[^,\s])*`) + +// userPassword is user=name%password, where mount.cifs takes the password after the percent sign. +var userPassword = regexp.MustCompile(`(?i)\b(user(?:name)?=[^,\s%]*)%(?:,,+[^,\s]*|[^,\s])*`) // RedactLine replaces every credential's value in a raw fstab line. func RedactLine(line string) string { - return passwordValue.ReplaceAllString(line, "${1}="+Redacted) + line = passwordValue.ReplaceAllString(line, "${1}="+Redacted) + return userPassword.ReplaceAllString(line, "${1}%"+Redacted) } // Adoption is what taking a line over does, or would do. diff --git a/modules/mounts/cmd/mounts/apply.go b/modules/mounts/cmd/mounts/apply.go index 5bfbfd1a..f284fc41 100644 --- a/modules/mounts/cmd/mounts/apply.go +++ b/modules/mounts/cmd/mounts/apply.go @@ -210,6 +210,18 @@ func (a *Applier) Look(ctx context.Context) Status { st.Server = cfg.ServerNode() st.Problems = append(st.Problems, cfg.Problems...) + // Where the mesh keeps data on this machine (finding 4d). Without it nothing new is set up: a pair the + // module already has stays, and one it does not have waits until the list is read. + data, derr := readJSON[DataPaths](a.M, DataFile) + haveData := derr == nil && data.Paths != nil + if !haveData { + why := "it has not been left by the module's tools yet" + if derr == nil && data.Error != "" { + why = data.Error + } + st.Problems = append(st.Problems, "where the mesh keeps data on this machine is not known ("+why+"); nothing new is set up until it is") + } + exports, eerr := readJSON[ExportsAnswer](a.M, ExportsFile) byName := map[string]ServerShare{} for _, e := range exports.Shares { @@ -224,6 +236,9 @@ func (a *Applier) Look(ctx context.Context) Status { case cfg.At == "": notSet[w.Name] = ShareStatus{Name: w.Name, Kind: "nfs", Mountpoint: w.Mountpoint, State: StateFailed, Why: "no machine shares nfs-share for this one: the binding gives no address"} + case cfg.Node != "" && st.Server == cfg.Node: + notSet[w.Name] = ShareStatus{Name: w.Name, Kind: "nfs", Mountpoint: w.Mountpoint, State: StateFailed, + Why: "this machine shares it itself (" + cfg.Node + "): a share is never mounted onto the machine that serves it"} case !ok && eerr != nil: notSet[w.Name] = ShareStatus{Name: w.Name, Kind: "nfs", Mountpoint: w.Mountpoint, State: StateUnreachable, Why: "what the sharing machine " + st.Server + " exports has not been read from the bus yet"} @@ -307,6 +322,24 @@ func (a *Applier) Look(ctx context.Context) Status { changed := false var written []Pair for _, p := range pairs { + ours := own[p.MountUnit] && own[p.AutoUnit] + refuse := func(why string) { + notSet[p.Name] = ShareStatus{Name: p.Name, Kind: p.Kind, Occasional: p.Occasional, Mountpoint: p.Mountpoint, What: p.What, + State: StateFailed, Why: why} + } + if haveData { + if err := nearData(p.Mountpoint, data.Paths); err != nil { + refuse(err.Error()) + continue + } + } else if !ours { + refuse("where the mesh keeps data on this machine is not known yet (the controller's data verb), so a new mount point is not set up") + continue + } + if err := a.M.mountpointFree(p, own, mounted); err != nil { + refuse(err.Error()) + continue + } if err := a.foreign(p, own); err != nil { notSet[p.Name] = ShareStatus{Name: p.Name, Kind: p.Kind, Occasional: p.Occasional, Mountpoint: p.Mountpoint, What: p.What, State: StateFailed, Why: err.Error()} @@ -339,13 +372,39 @@ func (a *Applier) Look(ctx context.Context) Status { written = append(written, p) } - // The module's own pairs whose mount point nothing names any more: stopped, disabled and removed. + // The module's own pairs whose mount point nothing names any more: stopped, disabled and removed — + // unless an entry of the settings could not be read, when nobody can tell which mount point it named + // (finding 8), or what is mounted there is not the pair's own source (finding 4c). var gone []string - for unit := range own { - base := strings.TrimSuffix(strings.TrimSuffix(unit, ".automount"), ".mount") - if !named[base] && a.Act { + if len(cfg.Unreadable) > 0 { + if a.Act { + st.Problems = append(st.Problems, "nothing is removed while an entry of shares or sources cannot be read: "+strings.Join(cfg.Unreadable, "; ")) + } + } else { + kept := map[string]bool{} + for unit := range own { + base := strings.TrimSuffix(strings.TrimSuffix(unit, ".automount"), ".mount") + if named[base] || !a.Act || kept[base] { + continue + } + text, _ := a.M.read(filepath.Join(UnitDir, base+".mount")) + what, where := unitField(text, "What"), unitField(text, "Where") + if where != "" { + if err := a.M.notOwnSource(kindOf(what, unitField(text, "Type")), what, where, mounted); err != nil { + kept[base] = true + st.Problems = append(st.Problems, fmt.Sprintf("the pair %s is no longer named and is kept, and nothing is stopped: %v", base, err)) + continue + } + } gone = append(gone, unit) } + var still []string + for _, unit := range gone { + if !kept[strings.TrimSuffix(strings.TrimSuffix(unit, ".automount"), ".mount")] { + still = append(still, unit) + } + } + gone = still } sort.Strings(gone) for _, unit := range gone { diff --git a/modules/mounts/cmd/mounts/bus.go b/modules/mounts/cmd/mounts/bus.go index 7d61df91..ce3d0057 100644 --- a/modules/mounts/cmd/mounts/bus.go +++ b/modules/mounts/cmd/mounts/bus.go @@ -62,38 +62,95 @@ func AskExports(cfg Config, previous ExportsAnswer, get Getter, now time.Time) E return out } +// SharesValue is what the module keeps on the bus for one machine: what its process last found, and what +// the machine asks of which server, so that the server exports a share to the machines that ask for it +// and to nobody else (the review of 2026-10-08, finding 5). The server decides the access it gives. +type SharesValue struct { + Status + Wants []Want `json:"wants"` +} + // Share is one pass of the bundle's work over a machine. func (m *Machine) Share(node string, get Getter, put Putter, last *string, now time.Time) []string { var said []string + wants := []Want{} if text, err := m.read(ConfigFile); err == nil { cfg := ParseConfig(text) + wants = cfg.Wants() if len(cfg.Shares) > 0 { previous, _ := readJSON[ExportsAnswer](m, ExportsFile) answer := AskExports(cfg, previous, get, now) if answer.Error != "" { said = append(said, "what the sharing machine exports was not read: "+answer.Error) } - raw, _ := json.MarshalIndent(answer, "", " ") - full := filepath.Join(m.Root, ExportsFile) - tmp := full + ".new" - if err := os.WriteFile(tmp, append(raw, '\n'), 0o644); err != nil { - said = append(said, "the answer was not left for the process: "+err.Error()) - } else if err := os.Rename(tmp, full); err != nil { + if err := m.leave(ExportsFile, answer); err != nil { said = append(said, "the answer was not left for the process: "+err.Error()) } } } if node != "" && put != nil { - if text, err := m.read(StatusFile); err == nil && text != *last { - var st Status - if json.Unmarshal([]byte(text), &st) == nil { - if err := put(node, st); err != nil { - said = append(said, "the state was not put: "+err.Error()) - } else { - *last = text - } + value := SharesValue{Wants: wants} + if text, err := m.read(StatusFile); err == nil { + json.Unmarshal([]byte(text), &value.Status) + } + if value.Status.Shares == nil { + value.Status.Shares = []ShareStatus{} + } + raw, _ := json.Marshal(value) + if string(raw) != *last { + if err := put(node, value); err != nil { + said = append(said, "the state was not put: "+err.Error()) + } else { + *last = string(raw) } } } return said } + +// leave writes a file of the bundle's for the process, through a temporary beside it. +func (m *Machine) leave(p string, v any) error { + raw, _ := json.MarshalIndent(v, "", " ") + full := filepath.Join(m.Root, p) + tmp := full + ".new" + if err := os.WriteFile(tmp, append(raw, '\n'), 0o644); err != nil { + return err + } + return os.Rename(tmp, full) +} + +// Asker calls a tool on the mesh as this module. +type Asker func(key string, body any) (json.RawMessage, error) + +// DataVerb is the controller's verb that says where the mesh keeps data on a machine. +const DataVerb = "seat:mesh-controller.data" + +// LeaveData asks the controller where the mesh keeps data on this machine and leaves the paths for the +// process (finding 4d). A failed ask keeps the last paths, with why; with none, the process sets up +// nothing new. +func (m *Machine) LeaveData(node string, ask Asker, now time.Time) []string { + if node == "" || ask == nil { + return nil + } + previous, _ := readJSON[DataPaths](m, DataFile) + out := DataPaths{Machine: node, Asked: now.Format(time.RFC3339)} + raw, err := ask(DataVerb, map[string]any{"machine": node}) + var paths []string + if err == nil { + paths, err = DataFrom(raw) + } + var said []string + if err != nil { + out.Error = err.Error() + if previous.Machine == node { + out.Paths, out.Answered = previous.Paths, previous.Answered + } + said = append(said, "where the mesh keeps data here was not read: "+err.Error()) + } else { + out.Paths, out.Answered = paths, out.Asked + } + if err := m.leave(DataFile, out); err != nil { + said = append(said, "the data list was not left for the process: "+err.Error()) + } + return said +} diff --git a/modules/mounts/cmd/mounts/config.go b/modules/mounts/cmd/mounts/config.go index e2ee219e..b0a9deb9 100644 --- a/modules/mounts/cmd/mounts/config.go +++ b/modules/mounts/cmd/mounts/config.go @@ -46,8 +46,14 @@ type Config struct { Secret string `json:"secret,omitempty"` State string `json:"state,omitempty"` Account string `json:"account,omitempty"` + // Node is this machine's name: a share this machine serves itself is never mounted onto it. + Node string `json:"node,omitempty"` // Problems are what in the file could not be read; each is said, and its share is left out. Problems []string `json:"problems,omitempty"` + // Unreadable are the entries of shares or sources that could not be read. While there is one, no + // pair is removed: an entry the operator mistyped names a mount point nobody can tell, so the + // module cannot tell which of its pairs it still names (review of 2026-10-08, finding 8). + Unreadable []string `json:"unreadable,omitempty"` } // Wanted is one NFS share this machine mounts: the share's name on the sharing machine, and where. @@ -219,9 +225,12 @@ func ParseConfig(text string) Config { var p []string c.Shares, p = ParseShares(kv["shares"]) c.Problems = append(c.Problems, p...) + c.Unreadable = append(c.Unreadable, p...) c.Sources, p = ParseSources(kv["sources"]) c.Problems = append(c.Problems, p...) + c.Unreadable = append(c.Unreadable, p...) c.Server, c.At, c.Secret, c.State, c.Account = kv["server"], kv["at"], kv["secret"], kv["state"], kv["account"] + c.Node = kv["node"] if c.At != "" && !hostRe.MatchString(c.At) { c.Problems = append(c.Problems, fmt.Sprintf("the sharing machine's address %q is not a name or an address", c.At)) c.At = "" diff --git a/modules/mounts/cmd/mounts/data.go b/modules/mounts/cmd/mounts/data.go new file mode 100644 index 00000000..4550ed81 --- /dev/null +++ b/modules/mounts/cmd/mounts/data.go @@ -0,0 +1,214 @@ +package main + +// What the module may not mount over (the review of 2026-10-08, findings 4a–4d): the mesh's own data on +// this machine, a mount point that something else uses, a folder that holds files, and a share this +// machine serves itself. And what the bundle leaves for the process so that it can tell: the paths the +// mesh keeps data in on this machine, from the controller's `data` verb over the bus. + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "strings" +) + +// DataFile is what the bundle left from mesh-controller.data for this machine. +const DataFile = BusDir + "/data.json" + +// DataPaths is where the mesh keeps data on this machine, as its controller says. +type DataPaths struct { + Machine string `json:"machine"` + Asked string `json:"asked"` + Paths []string `json:"paths"` + // Error is why the last ask got no answer; the paths are then the last answer's, or none. + Error string `json:"error,omitempty"` + Answered string `json:"answered,omitempty"` +} + +// Want is one share this machine asks of one server, put on the bus so that the server exports the share +// to this machine and to nobody who does not ask (finding 5). The server decides the access it gives; this +// is only what is asked. +type Want struct { + Server string `json:"server"` + Share string `json:"share"` + Access string `json:"access"` +} + +// Wants are what a config asks of its server, by share name. +func (c Config) Wants() []Want { + out := []Want{} + server := c.ServerNode() + if server == "" { + return out + } + for _, w := range c.Shares { + access := "rw" + if w.ReadOnly { + access = "ro" + } + out = append(out, Want{Server: server, Share: w.Name, Access: access}) + } + sortWants(out) + return out +} + +func sortWants(w []Want) { + for i := 1; i < len(w); i++ { + for j := i; j > 0 && w[j].Share < w[j-1].Share; j-- { + w[j], w[j-1] = w[j-1], w[j] + } + } +} + +// DataFrom reads the paths out of mesh-controller.data's answer: `answer.data`, `data`, or the same as +// text in `output`. An answer with no data list is an error, never an empty list: an empty list would +// let every mount point through. +func DataFrom(raw json.RawMessage) ([]string, error) { + type item struct { + Path string `json:"path"` + } + var whole struct { + Answer *struct { + Data *[]item `json:"data"` + } `json:"answer"` + Data *[]item `json:"data"` + Output string `json:"output"` + } + if err := json.Unmarshal(raw, &whole); err != nil { + return nil, fmt.Errorf("the controller's data answer is not JSON: %v", err) + } + var items *[]item + switch { + case whole.Answer != nil && whole.Answer.Data != nil: + items = whole.Answer.Data + case whole.Data != nil: + items = whole.Data + case whole.Output != "": + var inner struct { + Data *[]item `json:"data"` + } + if err := json.Unmarshal([]byte(whole.Output), &inner); err == nil && inner.Data != nil { + items = inner.Data + } + } + if items == nil { + return nil, errors.New("the controller's data answer names no data list") + } + paths := []string{} + for _, it := range *items { + if strings.HasPrefix(it.Path, "/") { + paths = append(paths, filepath.Clean(it.Path)) + } + } + return paths, nil +} + +// nearData refuses a mount point at, under or above a path the mesh keeps data in here: at or under, the +// mount would hide the data; above, it would hide it too, and everything beside it. +func nearData(point string, paths []string) error { + for _, d := range paths { + switch { + case point == d: + return fmt.Errorf("the mount point %s is at %s, where the mesh keeps data on this machine; it is never mounted over", point, d) + case strings.HasPrefix(point, d+"/"): + return fmt.Errorf("the mount point %s is under %s, where the mesh keeps data on this machine; it is never mounted over", point, d) + case strings.HasPrefix(d, point+"/"): + return fmt.Errorf("the mount point %s is above %s, where the mesh keeps data on this machine; mounting there would hide it", point, d) + } + } + return nil +} + +// sourceIs says whether what is mounted at a point is a pair's own source. For a network share the source +// is compared as the kernel writes it; for a disk, the device its by-uuid link resolves to. +func (m *Machine) sourceIs(kind, what string, mt Mounted) bool { + switch kind { + case "nfs": + return strings.HasPrefix(mt.Type, "nfs") && strings.TrimSuffix(mt.Source, "/") == strings.TrimSuffix(what, "/") + case "smb": + return (mt.Type == "cifs" || mt.Type == "smb3") && strings.EqualFold(strings.TrimSuffix(mt.Source, "/"), strings.TrimSuffix(what, "/")) + case "disk": + dev, err := filepath.EvalSymlinks(filepath.Join(m.Root, what)) + if err != nil { + return false + } + src, err := filepath.EvalSymlinks(filepath.Join(m.Root, mt.Source)) + return err == nil && src == dev + } + return false +} + +// kindOf is a unit's kind from what it mounts. +func kindOf(what, fstype string) string { + switch { + case strings.HasPrefix(fstype, "nfs"): + return "nfs" + case fstype == "cifs" || strings.HasPrefix(what, "//"): + return "smb" + case strings.HasPrefix(what, "/dev/"): + return "disk" + } + return "" +} + +// notOwnSource says why a mounted filesystem at a point is not the source a unit of the module mounts +// there, or nothing when it is, or nothing is mounted there but an automount. +func (m *Machine) notOwnSource(kind, what, point string, mounted map[string]Mounted) error { + mt, there := mounted[point] + if !there || mt.Type == "autofs" { + return nil + } + if kind != "" && m.sourceIs(kind, what, mt) { + return nil + } + return fmt.Errorf("%s is mounted at %s (%s), which is not what the module mounts there (%s)", mt.Source, point, mt.Type, what) +} + +// mountpointFree refuses a mount point another mount uses, or a folder that holds files. A point in +// mountinfo is never opened, so a network server that is away cannot hold the look. +func (m *Machine) mountpointFree(p Pair, own map[string]bool, mounted map[string]Mounted) error { + ours := own[p.MountUnit] && own[p.AutoUnit] + if mt, there := mounted[p.Mountpoint]; there { + if mt.Type == "autofs" { + if ours { + return nil + } + return fmt.Errorf("%s is already an automount that is not the module's (%s)", p.Mountpoint, mt.Source) + } + if ours && m.sourceIs(p.Kind, p.What, mt) { + return nil + } + return fmt.Errorf("%s is already mounted: %s (%s) is there, so this share is not mounted over it", p.Mountpoint, mt.Source, mt.Type) + } + dir, err := os.Open(filepath.Join(m.Root, p.Mountpoint)) + if err != nil { + return nil // missing: it is made + } + defer dir.Close() + info, err := dir.Stat() + if err != nil { + return fmt.Errorf("%s cannot be looked at: %v", p.Mountpoint, err) + } + if !info.IsDir() { + return fmt.Errorf("%s is a file, not a folder", p.Mountpoint) + } + if names, err := dir.Readdirnames(1); err == nil && len(names) > 0 { + return fmt.Errorf("%s is not empty (it holds %s); a mount would hide what is in it, so it is not mounted there", p.Mountpoint, names[0]) + } else if err != nil && err != io.EOF { + return fmt.Errorf("%s cannot be read: %v", p.Mountpoint, err) + } + return nil +} + +// unitField reads one Key= of a unit file. +func unitField(text, key string) string { + for _, line := range strings.Split(text, "\n") { + if v, ok := strings.CutPrefix(strings.TrimSpace(line), key+"="); ok { + return strings.TrimSpace(v) + } + } + return "" +} diff --git a/modules/mounts/cmd/mounts/main.go b/modules/mounts/cmd/mounts/main.go index 33acfb1d..ea5b32c0 100644 --- a/modules/mounts/cmd/mounts/main.go +++ b/modules/mounts/cmd/mounts/main.go @@ -62,9 +62,14 @@ func shareOnTheBus(m *Machine) { _, err := stdio.State(SharesState).Put(key, value) return err } + ask := func(key string, body any) (json.RawMessage, error) { return stdio.Ask(key, body) } last, lastSaid := "", "" - for { + for n := 0; ; n++ { said := m.Share(node, get, put, &last, time.Now()) + // Where the mesh keeps data here changes only with an assignment: asked at start and every five minutes. + if n%10 == 0 { + said = append(said, m.LeaveData(node, ask, time.Now())...) + } if s := fmt.Sprint(said); len(said) > 0 && s != lastSaid { fmt.Fprintln(os.Stderr, "mounts:", s) lastSaid = s diff --git a/modules/mounts/cmd/mounts/manifest_test.go b/modules/mounts/cmd/mounts/manifest_test.go index ab1e3491..9676f26f 100644 --- a/modules/mounts/cmd/mounts/manifest_test.go +++ b/modules/mounts/cmd/mounts/manifest_test.go @@ -13,6 +13,7 @@ type manifest struct { Tools []string `json:"tools"` Requires []string `json:"requires"` Reads []string `json:"reads"` + Invokes []string `json:"invokes"` Claims []map[string]any `json:"claims"` State []map[string]any `json:"state"` Settings map[string]map[string]any `json:"settings"` @@ -68,8 +69,15 @@ func TestTheManifestNamesWhatTheBundleServesAndWhatItNeeds(t *testing.T) { if strings.Join(m.Requires, ",") != "nfs-share" { t.Errorf("requires %v", m.Requires) } - if strings.Join(m.Reads, ",") != ExportsState || strings.Contains(raw, `"invokes"`) { - t.Errorf("the module reads nfs-server's exports from the bus, and calls nothing: reads %v", m.Reads) + if strings.Join(m.Reads, ",") != ExportsState { + t.Errorf("the module reads nfs-server's exports from the bus: reads %v", m.Reads) + } + // It calls one verb: the controller's, for where the mesh keeps data on this machine (review finding 4d). + if strings.Join(m.Invokes, ",") != DataVerb { + t.Errorf("invokes %v, want %s", m.Invokes, DataVerb) + } + if !strings.Contains(raw, `node=${machine:name}`) { + t.Error("the config gives the process this machine's name, so it never mounts a share onto the machine that serves it") } if len(m.State) != 1 || m.State[0]["name"] != SharesState || m.State[0]["per-machine"] != true { t.Errorf("state %v", m.State) diff --git a/modules/mounts/cmd/mounts/mounts.go b/modules/mounts/cmd/mounts/mounts.go index c8606ebb..f96992aa 100644 --- a/modules/mounts/cmd/mounts/mounts.go +++ b/modules/mounts/cmd/mounts/mounts.go @@ -29,13 +29,29 @@ func RedactOptions(opts string) string { return opts } parts := strings.Split(opts, ",") - for i, p := range parts { - k, _, has := strings.Cut(p, "=") - if has && secretKeys[strings.ToLower(strings.TrimSpace(k))] { - parts[i] = k + "=" + Redacted + var out []string + for i := 0; i < len(parts); i++ { + p := parts[i] + k, v, has := strings.Cut(p, "=") + key := strings.ToLower(strings.TrimSpace(k)) + switch { + case has && secretKeys[key]: + // mount.cifs writes a comma inside a password as two, so the value runs on over every ",,": an + // empty part, and the part after an empty one, are the password's. Read on the side of hiding: + // a run of three commas could be either, and is taken as the password's. + for i+1 < len(parts) && (parts[i+1] == "" || parts[i] == "") { + i++ + } + out = append(out, k+"="+Redacted) + case has && (key == "user" || key == "username") && strings.Contains(v, "%"): + // user=name%password: mount.cifs takes the password after the percent sign. + name, _, _ := strings.Cut(v, "%") + out = append(out, k+"="+name+"%"+Redacted) + default: + out = append(out, p) } } - return strings.Join(parts, ",") + return strings.Join(out, ",") } // IsNetwork says whether a mount's files live on another machine, by its type or, for a type that diff --git a/modules/mounts/cmd/mounts/review_test.go b/modules/mounts/cmd/mounts/review_test.go new file mode 100644 index 00000000..c0893f5c --- /dev/null +++ b/modules/mounts/cmd/mounts/review_test.go @@ -0,0 +1,308 @@ +package main + +// The review of 2026-10-08 (mesh-catalog #136): each finding reproduced by a test before it was fixed. + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// dataAnswer is mesh-controller.data's answer for this machine, as the bundle leaves it for the process. +func (f *fakeSystem) dataPaths(paths ...string) { + f.t.Helper() + if paths == nil { + paths = []string{} + } + raw, _ := json.Marshal(DataPaths{Machine: "workstation", Asked: time.Now().Format(time.RFC3339), Paths: paths}) + f.write(DataFile, string(raw)) +} + +func shareOf(st Status, name string) ShareStatus { + for _, s := range st.Shares { + if s.Name == name { + return s + } + } + return ShareStatus{} +} + +const libraryOnly = "shares=library=/srv/library\nserver=homeserver\nat=192.0.2.10\nnode=workstation\n" + +// Finding 8: a setting entry that cannot be read removed the module's pair for a mount point it had named +// before, because only what parsed counted as named. +func TestAnUnreadableEntryRemovesNoMount(t *testing.T) { + f := newSystem(t) + f.dataPaths() + f.write(ExportsFile, exportsAnswer(t, library)) + f.write(filepath.Join(UnitDir, "srv-library.mount"), Marker+"\n[Mount]\nWhat=192.0.2.10:/tank/media\nWhere=/srv/library\n") + f.write(filepath.Join(UnitDir, "srv-library.automount"), Marker+"\n[Automount]\nWhere=/srv/library\n") + // The operator meant library=/srv/library and wrote it without its slash. + f.write(ConfigFile, "shares=library=srv/library\nserver=homeserver\nat=192.0.2.10\nnode=workstation\n") + st := f.applier(time.Now()).Look(context.Background()) + if _, err := os.Stat(filepath.Join(f.root, UnitDir, "srv-library.mount")); err != nil { + t.Fatal("an entry that cannot be read removes no pair") + } + if f.called("systemctl stop srv-library.mount") || f.called("systemctl disable --now srv-library.automount") { + t.Errorf("nothing is stopped: %q", f.calls) + } + said := strings.Join(st.Problems, " ") + if !strings.Contains(said, "srv/library") || !strings.Contains(said, "nothing is removed") { + t.Errorf("the bad entry and what is kept are said: %q", st.Problems) + } +} + +// Finding 4a: a mount point already mounted by something else, or a folder with files in it, was taken. +func TestAMountPointInUseOrNotEmptyIsRefused(t *testing.T) { + f := newSystem(t) + f.dataPaths() + f.write(ConfigFile, "shares=library=/srv/library downloads=/srv/downloads\nserver=homeserver\nat=192.0.2.10\nnode=workstation\n") + f.write(ExportsFile, exportsAnswer(t, library, downloads)) + // Something else is mounted at the library's mount point; the downloads' folder holds a file. + f.write("/proc/self/mountinfo", "36 1 8:2 / /srv/library rw,relatime shared:1 - ext4 /dev/sdb1 rw\n") + f.write("/srv/downloads/notes.txt", "the operator's") + st := f.applier(time.Now()).Look(context.Background()) + for _, name := range []string{"library", "downloads"} { + s := shareOf(st, name) + if s.State != StateFailed { + t.Errorf("%s: %+v", name, s) + } + if f.called("systemctl enable --now srv-" + name + ".automount") { + t.Errorf("%s is not armed", name) + } + if _, err := os.Stat(filepath.Join(f.root, UnitDir, "srv-"+name+".mount")); !os.IsNotExist(err) { + t.Errorf("%s: no unit is written", name) + } + } + if s := shareOf(st, "library"); !strings.Contains(s.Why, "/dev/sdb1") { + t.Errorf("the library says what is mounted there: %q", s.Why) + } + if s := shareOf(st, "downloads"); !strings.Contains(s.Why, "not empty") { + t.Errorf("the downloads say the folder is not empty: %q", s.Why) + } + if f.read("/srv/downloads/notes.txt") != "the operator's" { + t.Error("the operator's file is untouched") + } +} + +// Its own share, mounted by its own unit, is not "in use by something else". +func TestTheModulesOwnMountIsNotRefused(t *testing.T) { + f := newSystem(t) + f.dataPaths() + f.write(ConfigFile, libraryOnly) + f.write(ExportsFile, exportsAnswer(t, library)) + f.write("/proc/self/mountinfo", "40 1 0:50 / /srv/library rw - autofs systemd-1 rw\n"+ + "41 40 0:51 / /srv/library rw - nfs4 192.0.2.10:/tank/media rw\n") + f.write(filepath.Join(UnitDir, "srv-library.mount"), Marker+"\n") + f.write(filepath.Join(UnitDir, "srv-library.automount"), Marker+"\n") + f.reachable["192.0.2.10:2049"] = true + st := f.applier(time.Now()).Look(context.Background()) + if s := shareOf(st, "library"); s.State != StateMounted { + t.Errorf("library %+v", s) + } +} + +// Finding 4b: a share the machine serves itself was mounted onto itself. +func TestAShareThisMachineServesIsRefused(t *testing.T) { + f := newSystem(t) + f.dataPaths() + f.write(ConfigFile, "shares=library=/srv/library\nserver=homeserver\nat=192.0.2.10\nnode=homeserver\n") + f.write(ExportsFile, exportsAnswer(t, library)) + st := f.applier(time.Now()).Look(context.Background()) + s := shareOf(st, "library") + if s.State != StateFailed || !strings.Contains(s.Why, "this machine shares it") { + t.Errorf("library %+v", s) + } + if _, err := os.Stat(filepath.Join(f.root, UnitDir, "srv-library.mount")); !os.IsNotExist(err) { + t.Error("no unit is written") + } +} + +// Finding 4c: removing a pair stopped its .mount even when what was mounted there was not its source. +func TestAMountThatIsNotItsOwnSourceIsNeverStopped(t *testing.T) { + f := newSystem(t) + f.dataPaths() + f.write(ConfigFile, "shares=none\nnode=workstation\n") + f.write(filepath.Join(UnitDir, "srv-old.mount"), Marker+"\n[Mount]\nWhat=192.0.2.10:/tank/old\nWhere=/srv/old\nType=nfs4\n") + f.write(filepath.Join(UnitDir, "srv-old.automount"), Marker+"\n[Automount]\nWhere=/srv/old\n") + f.write("/proc/self/mountinfo", "36 1 8:2 / /srv/old rw - ext4 /dev/sdb1 rw\n") + st := f.applier(time.Now()).Look(context.Background()) + if f.called("systemctl stop srv-old.mount") || f.called("systemctl disable --now srv-old.automount") { + t.Errorf("a mount that is not its own source is not stopped: %q", f.calls) + } + if _, err := os.Stat(filepath.Join(f.root, UnitDir, "srv-old.mount")); err != nil { + t.Error("and its pair is kept") + } + if !strings.Contains(strings.Join(st.Problems, " "), "/dev/sdb1") { + t.Errorf("said: %q", st.Problems) + } + // The verb unmount refuses the same. + f.write(StatusFile, `{"shares":[{"name":"old","kind":"nfs","mountpoint":"/srv/old","what":"192.0.2.10:/tank/old","state":"mounted","mount_unit":"srv-old.mount","automount_unit":"srv-old.automount"}]}`) + f.calls = nil + _, err := callVerb(t, f.machine(), "unmount", map[string]any{"name": "old"}) + if err == nil || !strings.Contains(err.Error(), "/dev/sdb1") || f.called("systemctl stop") { + t.Errorf("unmount refuses: %v %q", err, f.calls) + } +} + +// Finding 4d: a mount point at, under or above data the mesh keeps on this machine was taken. +func TestAMountPointNearTheMeshsDataIsRefused(t *testing.T) { + cases := map[string]string{ + "/mnt/plex": "above", + "/mnt/plex/data": "at", + "/mnt/plex/data/x": "under", + } + for point, how := range cases { + f := newSystem(t) + f.dataPaths("/mnt/plex/data", "/mnt/plex/config") + f.write(ConfigFile, "shares=library="+point+"\nserver=homeserver\nat=192.0.2.10\nnode=workstation\n") + f.write(ExportsFile, exportsAnswer(t, library)) + st := f.applier(time.Now()).Look(context.Background()) + s := shareOf(st, "library") + if s.State != StateFailed || !strings.Contains(s.Why, "/mnt/plex/data") || !strings.Contains(s.Why, how) { + t.Errorf("%s: %+v", point, s) + } + if f.called("systemctl enable") { + t.Errorf("%s is not armed", point) + } + } +} + +// When what the mesh keeps here cannot be read, nothing new is set up; what exists stays. +func TestWithoutTheMeshsDataNothingNewIsSetUp(t *testing.T) { + f := newSystem(t) + os.Remove(filepath.Join(f.root, DataFile)) + f.write(ConfigFile, libraryOnly) + f.write(ExportsFile, exportsAnswer(t, library)) + st := f.applier(time.Now()).Look(context.Background()) + if _, err := os.Stat(filepath.Join(f.root, UnitDir, "srv-library.mount")); !os.IsNotExist(err) { + t.Error("nothing new is written without the data list") + } + if s := shareOf(st, "library"); s.State != StateFailed || !strings.Contains(s.Why, "data") { + t.Errorf("library %+v", s) + } + // A pair that is already the module's stays and stays armed. + f.write(filepath.Join(UnitDir, "srv-library.mount"), Marker+"\n") + f.write(filepath.Join(UnitDir, "srv-library.automount"), Marker+"\n") + f.applier(time.Now()).Look(context.Background()) + if _, err := os.Stat(filepath.Join(f.root, UnitDir, "srv-library.mount")); err != nil { + t.Error("an existing pair stays") + } +} + +// The bundle asks the controller for this machine's data and leaves the paths for the process. +func TestTheBundleLeavesTheMeshsDataForTheProcess(t *testing.T) { + f := newSystem(t) + f.write(ConfigFile, libraryOnly) + answer := json.RawMessage(`{"answer":{"data":[{"machine":"workstation","module":"plex","item":"data","path":"/mnt/plex/data"}]},"ok":true}`) + paths, err := DataFrom(answer) + if err != nil || len(paths) != 1 || paths[0] != "/mnt/plex/data" { + t.Fatalf("paths %q %v", paths, err) + } + // The controller's verbs also answer as text in output. + paths, err = DataFrom(json.RawMessage(`{"ok":true,"output":"{\"data\":[{\"path\":\"/srv/x\"}]}"}`)) + if err != nil || len(paths) != 1 || paths[0] != "/srv/x" { + t.Fatalf("paths from output %q %v", paths, err) + } + if _, err := DataFrom(json.RawMessage(`{"ok":false}`)); err == nil { + t.Error("an answer with no data list is an error, not an empty list") + } +} + +// Finding 5: the server exports a share only to the nodes that want it, so the bundle says what this node +// wants of which server, also before the process has looked. +func TestTheBundleSaysWhatThisNodeWants(t *testing.T) { + f := newSystem(t) + f.write(ConfigFile, "shares=library=/srv/library downloads=/srv/downloads:ro\nserver=homeserver\nat=192.0.2.10\nnode=workstation\n") + var put []any + last := "" + ask := func(string) (json.RawMessage, error) { return json.RawMessage(`{"shares":[]}`), nil } + f.machine().Share("workstation", ask, func(_ string, v any) error { put = append(put, v); return nil }, &last, time.Now()) + if len(put) != 1 { + t.Fatalf("the wants are put before the process has looked: %d", len(put)) + } + raw, _ := json.Marshal(put[0]) + var got struct { + Wants []Want `json:"wants"` + } + json.Unmarshal(raw, &got) + want := []Want{{Server: "homeserver", Share: "downloads", Access: "ro"}, {Server: "homeserver", Share: "library", Access: "rw"}} + if len(got.Wants) != 2 || got.Wants[0] != want[0] || got.Wants[1] != want[1] { + t.Errorf("wants %s", raw) + } +} + +// Finding 9: the seat passes confirm as the string "true". +func TestConfirmIsTrueAsABooleanOrAString(t *testing.T) { + for _, v := range []any{true, "true"} { + if !flag(map[string]any{"confirm": v}, "confirm") { + t.Errorf("%#v is a confirmation", v) + } + } + for _, v := range []any{false, "false", "yes", "1", nil} { + if flag(map[string]any{"confirm": v}, "confirm") { + t.Errorf("%#v is not a confirmation", v) + } + } +} + +// Low: a password hidden in user=name%pass, and one holding a comma written as two (",,"). +func TestEveryWayAPasswordIsWrittenIsRedacted(t *testing.T) { + for _, opts := range []string{ + "user=bob%" + testPassword + ",vers=3.0", + "username=bob%" + testPassword, + "password=" + testPassword + ",,tail,vers=3.0", + "password=,,," + testPassword, + "pass=" + testPassword, + } { + if got := RedactOptions(opts); strings.Contains(got, testPassword) || strings.Contains(got, "tail") { + t.Errorf("RedactOptions(%q) = %q", opts, got) + } + line := "//192.0.2.76/Share /mnt/games cifs " + opts + " 0 0" + if got := RedactLine(line); strings.Contains(got, testPassword) || strings.Contains(got, "tail") { + t.Errorf("RedactLine(%q) = %q", line, got) + } + } + if got := RedactOptions("user=bob,vers=3.0"); got != "user=bob,vers=3.0" { + t.Errorf("a user name alone is kept: %q", got) + } +} + +func callVerb(t *testing.T, m *Machine, name string, args map[string]any) (any, error) { + t.Helper() + for _, tool := range Verbs(m) { + if tool.Name == Seat+"."+name { + return tool.Run(args) + } + } + t.Fatalf("no verb %s", name) + return nil, nil +} + +// The bundle asks the controller, leaves the paths, and keeps the last ones when an ask fails. +func TestTheBundleKeepsTheLastDataWhenAnAskFails(t *testing.T) { + f := newSystem(t) + m := f.machine() + var asked []string + ok := func(key string, body any) (json.RawMessage, error) { + raw, _ := json.Marshal(body) + asked = append(asked, key+" "+string(raw)) + return json.RawMessage(`{"answer":{"data":[{"path":"/mnt/plex/data"}]}}`), nil + } + if said := m.LeaveData("workstation", ok, time.Now()); len(said) != 0 { + t.Fatalf("said %q", said) + } + if len(asked) != 1 || asked[0] != `seat:mesh-controller.data {"machine":"workstation"}` { + t.Errorf("asked %q", asked) + } + failing := func(string, any) (json.RawMessage, error) { return nil, os.ErrDeadlineExceeded } + said := m.LeaveData("workstation", failing, time.Now()) + d, err := readJSON[DataPaths](m, DataFile) + if err != nil || len(d.Paths) != 1 || d.Paths[0] != "/mnt/plex/data" || d.Error == "" || len(said) == 0 { + t.Errorf("a failed ask keeps the last paths and says why: %+v %v %q", d, err, said) + } +} diff --git a/modules/mounts/cmd/mounts/verbs.go b/modules/mounts/cmd/mounts/verbs.go index dc4eae4c..aadf29bd 100644 --- a/modules/mounts/cmd/mounts/verbs.go +++ b/modules/mounts/cmd/mounts/verbs.go @@ -118,6 +118,11 @@ func Verbs(m *Machine) []stdio.Tool { if err != nil { return nil, err } + // Never stop a mount whose filesystem is not its own source (review of 2026-10-08, finding 4c). + info, _ := m.read("/proc/self/mountinfo") + if err := m.notOwnSource(s.Kind, s.What, s.Mountpoint, ParseMountinfo(info)); err != nil { + return nil, fmt.Errorf("%s is not released: %v", s.Name, err) + } if _, err := m.Escalate(ctx, "systemctl", "stop", s.MountUnit); err != nil { return nil, err } diff --git a/modules/mounts/cmd/mounts/write_test.go b/modules/mounts/cmd/mounts/write_test.go index 2e9b9973..4b126c2d 100644 --- a/modules/mounts/cmd/mounts/write_test.go +++ b/modules/mounts/cmd/mounts/write_test.go @@ -33,6 +33,8 @@ func newSystem(t *testing.T) *fakeSystem { } } f.write("/proc/self/mountinfo", "") + // The mesh keeps no data on this machine, as the controller said through the bundle. + f.write(DataFile, `{"machine":"workstation","paths":[]}`) return f } diff --git a/modules/mounts/module.json b/modules/mounts/module.json index a17e8e02..c11f0283 100644 --- a/modules/mounts/module.json +++ b/modules/mounts/module.json @@ -7,6 +7,9 @@ "reads": [ "nfs-server.exports" ], + "invokes": [ + "seat:mesh-controller.data" + ], "capabilities": [ "package-manager", "service-manager" @@ -82,7 +85,7 @@ "type": "file", "path": "/etc/mesh-mounts/mounts.conf", "mode": "0644", - "content": "# Written by the mesh (module mounts, novox/hq ADR 0263) from this machine's assignment: the settings\n# shares and sources, and the binding of nfs-share. Replaced on every push; change the settings, not this.\n# The module's process reads it as root every 30 seconds and writes one .mount and .automount per entry.\nshares=${setting:shares}\nsources=${setting:sources}\nserver=${bound:nfs-share:from}\nat=${bound:nfs-share:at}\naccount=${machine:account}\nsecret=${dir:state}/smb-credentials.secret\nstate=${dir:state}\n" + "content": "# Written by the mesh (module mounts, novox/hq ADR 0263) from this machine's assignment: the settings\n# shares and sources, and the binding of nfs-share. Replaced on every push; change the settings, not this.\n# The module's process reads it as root every 30 seconds and writes one .mount and .automount per entry.\nshares=${setting:shares}\nsources=${setting:sources}\nserver=${bound:nfs-share:from}\nat=${bound:nfs-share:at}\naccount=${machine:account}\nnode=${machine:name}\nsecret=${dir:state}/smb-credentials.secret\nstate=${dir:state}\n" }, { "id": "bus-dir-parent",