From 3a002f6e7b68448833cc6984f70bfde8daeed050 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 8 Oct 2026 23:58:14 +0200 Subject: [PATCH 1/2] Refuse a placement or mount at the machine's own trees, and use a found directory as found A module's places setting could point a directory anywhere, with an owner it named, and the engine chowned and chmodded whatever it was pointed at as root, through a link too; its accesses setting could mount any path into a container (hq issue 339). The engine now refuses, as a failed resource with its reason, a directory, access or mount source that is or holds one of the machine's roots, anything placed in the kernel's, the boot loader's, root's or the engine's own trees, a non-root directory below /etc, /usr or /run, an access there, another account's directory below a home, and a container mounting anything it refused, judged with every link resolved. A directory whose last component is a link is refused, and one found here that the mesh did not make keeps its owner and mode and is never removed. --- internal/apply/apply.go | 75 ++++++- internal/apply/placement_guard.go | 300 +++++++++++++++++++++++++ internal/apply/placement_guard_test.go | 195 ++++++++++++++++ internal/apply/placement_homes_test.go | 67 ++++++ internal/store/store.go | 5 + 5 files changed, 639 insertions(+), 3 deletions(-) create mode 100644 internal/apply/placement_guard.go create mode 100644 internal/apply/placement_guard_test.go create mode 100644 internal/apply/placement_homes_test.go diff --git a/internal/apply/apply.go b/internal/apply/apply.go index be49645..b63bc7b 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -75,6 +75,8 @@ type Outcome struct { groups []string // unpacked is, for an archive, what it put on the machine (novox/hq issue 162). unpacked *store.Unpacked + // asFound is, for a directory, that it is used as it was found (novox/hq issue 339). + asFound bool // reads is, for a container, the digest of each file it was created reading, by path — so // the next apply can say which one changed (novox/hq 04-ISSUES/103). reads map[string]string @@ -460,8 +462,17 @@ func ApplyMindingWindows( // does not answer during a maintenance window waits for the window instead of failing. in := inputs{declares: map[string]string{}, known: &known, windows: windows, away: newStoreAway(windows, log), groups: wanted} + // Where nothing is placed or mounted from, judged before anything is applied (novox/hq issue 339), and + // which declared directories were already here: one this apply makes is the mesh's, one found may not be. + in.refused = refusedPlaces(d.Resources) + in.dirsBefore = map[string]bool{} for _, resource := range d.Resources { in.declares[resource.Identity()] = declaredDigest(resource) + if dir, ok := resource.(*declaration.Directory); ok { + if _, err := os.Lstat(dir.Path); err == nil { + in.dirsBefore[filepath.Clean(dir.Path)] = true + } + } } // Everything is attempted, and every failure is reported. @@ -725,6 +736,7 @@ func ApplyMindingWindows( Linger: outcome.linger, Groups: outcome.groups, Unpacked: outcome.unpacked, + AsFound: outcome.asFound, Holds: holds(resource), }) if outcome.found != nil { @@ -918,9 +930,21 @@ type Unseal func(sealed string) ([]byte, error) func applyOne(ctx context.Context, sys system.System, r declaration.Resource, run Runner, changed map[string]bool, in inputs, previous store.Applied, unseal Unseal, keepFound Keep) (Outcome, error) { + // Nothing is placed in or mounted from the machine's own trees, whoever asked (novox/hq issue 339): a + // refusal is this resource failing, with its reason, in the node's report. + switch res := r.(type) { + case *declaration.Directory, *declaration.Access: + if err := in.refused[filepath.Clean(r.Target())]; err != nil { + return begin(r), err + } + case *declaration.Container: + if err := refuseMounts(res, in.refused); err != nil { + return begin(r), err + } + } switch res := r.(type) { case *declaration.Directory: - return applyDirectory(res) + return applyDirectory(res, previous, in.dirsBefore[filepath.Clean(res.Path)]) case *declaration.File: return applyFile(res, previous, unseal, keepFound) case *declaration.Service: @@ -965,21 +989,56 @@ func modeOf(spec string, fallback os.FileMode) (os.FileMode, error) { return os.FileMode(parsed), nil } -func applyDirectory(r *declaration.Directory) (Outcome, error) { +// applyDirectory makes a directory what was declared. +// +// **The path is not followed** (novox/hq issue 339): a directory whose last component is a link is refused, +// because a chmod and a chown through it would change wherever the link points — /etc, as root. +// +// **A directory found here, that the mesh did not make, is used as found** (novox/hq issue 339): its owner +// and mode are left, and the outcome says what was declared and what was found. Changing them would be root +// handing a directory it never made — wherever a declaration pointed it — to whatever account was named. A +// directory is the mesh's when its record says the mesh applied it at this path before (a record from before +// this rule counts, which is every directory on a running machine), when this apply made it, or when it +// already has the declared owner and mode, so a person who sets them by hand at the machine hands it over. +func applyDirectory(r *declaration.Directory, previous store.Applied, wasBefore bool) (Outcome, error) { out := begin(r) mode, err := modeOf(r.Mode, 0o755) if err != nil { return out, err } - before, err := os.Stat(r.Path) + before, err := os.Lstat(r.Path) existed := err == nil if err != nil && !errors.Is(err, os.ErrNotExist) { return out, err } + if existed && before.Mode()&os.ModeSymlink != 0 { + return out, &PlacementRefusedError{Path: r.Path, Why: "it is a symbolic link, and the node-engine " + + "follows no link to a directory it owns"} + } if existed && !before.IsDir() { return out, fmt.Errorf("%s exists and is not a directory", r.Path) } + if existed && wasBefore { + ours := previous.Target != "" && filepath.Clean(previous.Target) == filepath.Clean(r.Path) && !previous.AsFound + if !ours { + owned, err := ownedBy(r.Path, r.Owner) + if err != nil { + return out, err + } + if !owned || before.Mode().Perm() != mode.Perm() { + out.Action, out.asFound = "unchanged", true + owner := r.Owner + if owner == "" { + owner = "root" + } + out.Detail = fmt.Sprintf("found here before the mesh and used as found: its owner and mode %o are "+ + "left, though %s and %o were declared (novox/hq issue 339); set them by hand to hand it to the mesh", + before.Mode().Perm(), owner, mode.Perm()) + return out, nil + } + } + } if !existed { if err := makeDirs(r.Path, mode, r.Owner); err != nil { @@ -1623,6 +1682,10 @@ func remove(ctx context.Context, sys system.System, a store.Applied, run Runner, // This is the host's own line, applied to the one shape where getting it wrong is not // recoverable: it removes what it made and leaves what it merely configured. An empty // directory is what it made. A full one is not. + if a.AsFound { + // Found here before the mesh, and never the mesh's (novox/hq issue 339). + return "forgotten", "found here before the mesh and used as found: left as it is", nil + } entries, err := os.ReadDir(a.Target) if errors.Is(err, os.ErrNotExist) { return "forgotten", "no longer there", nil @@ -1850,6 +1913,12 @@ type inputs struct { // groups is every group the declaration asks an account to be in, and by which resources // (novox/hq ADR 0252): a group one user resource stops asking for stays while another asks. groups Wanted + // refused is every directory and access the engine will not place, by path, with why (novox/hq issue 339): + // each fails, and a container mounting one fails with it. + refused map[string]error + // dirsBefore is every declared directory that was on the machine when this apply began (novox/hq issue + // 339): one the apply itself made — a file's parent — is the mesh's, one that was there may not be. + dirsBefore map[string]bool } // fileDigest is what a file the container reads holds, by digest. diff --git a/internal/apply/placement_guard.go b/internal/apply/placement_guard.go new file mode 100644 index 0000000..e253e26 --- /dev/null +++ b/internal/apply/placement_guard.go @@ -0,0 +1,300 @@ +package apply + +// Where the node-engine places nothing and mounts nothing, whoever asks (novox/hq issue 339). +// +// A directory names its path, and the controller resolves part of that path from what was set for the +// module: its `places` setting moves a directory anywhere, with an owner it names, and its `accesses` setting +// says which of the machine's paths are mounted into its container. The engine runs as root, so a path it +// accepts blindly is a path anyone who could change those settings hands to any account: a directory at /etc +// owned by a caller's account gives it /etc, and an access at / mounts the machine's root into a container. +// The controller refuses both where a setting is made; the engine refuses them again where it applies, +// because a guard in one place is a guard one change away from gone. Whatever the declaration says: +// +// 1. **No directory, access or mount source is one of the machine's own roots, or holds one**: /, /etc, +// /usr, /var, /var/lib, /home, /run and the rest of protectedRoots. Modules place directories BELOW /etc +// or /var/lib, never the root itself; owning one is owning everything in it. +// 2. **Nothing is placed in /proc, /sys, /dev, /boot or /root, nor in the node-engine's own trees** (its +// state, its identity, its installed builds) but by its own module; nothing is mounted from /boot, /root or +// the engine's trees. A mount of a kernel file, a device or the clock is the plumbing systemPath names. +// 3. **A directory below /etc, /usr or /run is root's.** The machine's configuration and programs are read +// as root's word; a directory there owned by another account is that account writing root's word. An +// access is never there at all: the operator's data is not the machine's configuration. +// 4. **Below a person's or an agent's home, a directory is that account's.** Root's or another account's +// directory there is one the account does not control in a tree whose every parent it does. A home +// itself may be a module's directory (a backup repository kept as an account's home is one), and as +// every directory the mesh did not make, it is used as found: never chowned or chmodded (applyDirectory). +// 5. **A mount source that is a refused directory or a refused access is refused with it**: the container +// would otherwise bind the very path the engine would not place, and the runtime creates a missing one +// as root. +// +// Each is a failed resource with its reason in the node's report; nothing is touched. Paths are judged as +// declared and again with every link in them resolved, so a link at /srv/x pointing at /etc places nothing. + +import ( + "bufio" + "errors" + "fmt" + "os" + "path/filepath" + "strconv" + "strings" + + "github.com/novox/mesh-host/internal/declaration" +) + +// protectedRoots are paths no directory, access or mount source may be, nor hold. +var protectedRoots = []string{"/", "/bin", "/boot", "/dev", "/etc", "/home", "/lib", "/lib32", "/lib64", + "/media", "/mnt", "/opt", "/proc", "/root", "/run", "/sbin", "/srv", "/sys", "/tmp", "/usr", "/usr/bin", + "/usr/lib", "/usr/lib64", "/usr/local", "/usr/local/bin", "/usr/local/lib", "/usr/local/sbin", "/usr/sbin", + "/usr/share", "/var", "/var/cache", "/var/lib", "/var/lib/mesh", "/var/log", "/var/run", "/var/tmp", + "/var/spool"} + +// forbiddenBelow are trees nothing is placed in or mounted from: the kernel's, the boot loader's and root's +// home. engineTrees are the node-engine's own, which only its own module places in. +var ( + forbiddenBelow = []string{"/proc", "/sys", "/dev", "/boot", "/root"} + // forbiddenBelowMount are the trees no container mounts from: a mount of the kernel's files and devices is + // the plumbing a container may need (systemPath); root's home and the boot loader's are no plumbing. + forbiddenBelowMount = []string{"/boot", "/root"} + engineTrees = []string{"/var/lib/mesh-host", "/usr/lib/nox-mesh-host"} + // rootsOnly are trees a directory below is root's, and an access is never in. + rootsOnly = []string{"/etc", "/usr", "/run", "/var/run"} +) + +// engineModule is the module whose resources may place in the engine's own trees. +const engineModule = "mesh-host" + +// passwdFile is the user database homes are read from. A variable so a test names its own. +var passwdFile = "/etc/passwd" + +// PlacementRefusedError is a resource the engine will not place, or mount, where it says. +type PlacementRefusedError struct { + Path, Why string +} + +func (e *PlacementRefusedError) Error() string { + return fmt.Sprintf("%s is not placed: %s (novox/hq issue 339); nothing was touched", e.Path, e.Why) +} + +// homeAccount is a person's or an agent's account and its home. +type homeAccount struct { + Name string + UID int +} + +// accountsOfHomes is each person's or agent's home and the account it belongs to, from the user database: an +// account with a uid of 1000 or more, or a home under /home. A variable so a test names its own. +var accountsOfHomes = func() map[string]homeAccount { + f, err := os.Open(passwdFile) + if err != nil { + return nil + } + defer f.Close() + out := map[string]homeAccount{} + sc := bufio.NewScanner(f) + for sc.Scan() { + fields := strings.Split(sc.Text(), ":") + if len(fields) < 6 { + continue + } + uid, err := strconv.Atoi(fields[2]) + if err != nil { + continue + } + home := filepath.Clean(fields[5]) + if home == "/" || home == "." || home == "" || home == "/nonexistent" { + continue + } + if (uid >= 1000 && uid != 65534) || strings.HasPrefix(home, "/home/") { + out[home] = homeAccount{Name: fields[0], UID: uid} + } + } + return out +} + +// below says whether path is strictly below dir. +func below(path, dir string) bool { + if dir == "/" { + return path != "/" + } + return strings.HasPrefix(path, dir+"/") +} + +// atOrBelow says whether path is dir or below it. +func atOrBelow(path, dir string) bool { return path == dir || below(path, dir) } + +// resolved is a path with every link in it followed, as far as the path exists, and the rest as declared. +func resolved(path string) string { + rest := "" + for p := path; ; p = filepath.Dir(p) { + if real, err := filepath.EvalSymlinks(p); err == nil { + return filepath.Clean(filepath.Join(real, rest)) + } + if filepath.Dir(p) == p { + return path + } + rest = filepath.Join(filepath.Base(p), rest) + } +} + +// ownedByAccount says whether a declared owner is that account: by name, or by its uid ("1001", "1001:1001"). +func ownedByAccount(owner string, a homeAccount) bool { + if owner == a.Name { + return true + } + user, _, _ := strings.Cut(owner, ":") + if uid, err := strconv.Atoi(user); err == nil { + return uid == a.UID + } + return false +} + +// rootOwner says whether a declared owner is root: none, "root", or uid 0. +func rootOwner(owner string) bool { + if owner == "" || owner == "root" { + return true + } + user, _, _ := strings.Cut(owner, ":") + return user == "0" +} + +// what a guarded path is, for the words of a refusal. +type placing int + +const ( + placingDirectory placing = iota + placingAccess + placingMount +) + +// refusePath says why a path is not placed or mounted; nil when it may be. module is the resource's module, +// owner a directory's declared owner. +func refusePath(path string, kind placing, module, owner string) error { + clean := filepath.Clean(path) + if !filepath.IsAbs(clean) { + return nil // the declaration refuses a relative path already; a named volume is not a path + } + for _, p := range []string{clean, resolved(clean)} { + if err := refuseOne(p, kind, module, owner); err != nil { + if p != clean { + err.Why = fmt.Sprintf("through a link, it is %s, and %s", p, err.Why) + err.Path = clean + } + return err + } + } + return nil +} + +func refuseOne(path string, kind placing, module, owner string) *PlacementRefusedError { + for _, root := range protectedRoots { + if path == root || below(root, path) { + return &PlacementRefusedError{Path: path, Why: root + " is one of the machine's own directories, " + + "and owning or mounting it would be owning or mounting everything in it"} + } + } + trees := forbiddenBelow + if kind == placingMount { + // A mount is the machine's plumbing as often as a module's data — the clock, a kernel file, /dev/null + // (systemPath) — and what a setting can mount at all is a directory or an access, refused above it. + trees = forbiddenBelowMount + } + for _, tree := range trees { + if atOrBelow(path, tree) { + return &PlacementRefusedError{Path: path, Why: "nothing is placed in or mounted from " + tree} + } + } + if module != engineModule { + for _, tree := range engineTrees { + if atOrBelow(path, tree) { + return &PlacementRefusedError{Path: path, Why: tree + " is the node-engine's own, placed in by " + + "its own module alone"} + } + } + } + for _, tree := range rootsOnly { + if !below(path, tree) { + continue + } + switch { + case kind == placingAccess: + return &PlacementRefusedError{Path: path, Why: "an access is the operator's data, and " + tree + + " is the machine's own"} + case kind == placingDirectory && !rootOwner(owner): + return &PlacementRefusedError{Path: path, Why: fmt.Sprintf("a directory below %s is root's, and this "+ + "one is declared %s's", tree, owner)} + } + } + if kind != placingDirectory { + return nil + } + homes := accountsOfHomes() + var deepest string + for home := range homes { + if below(path, home) && len(home) > len(deepest) { + deepest = home + } + } + if deepest != "" { + if a := homes[deepest]; !ownedByAccount(owner, a) { + if owner == "" { + owner = "root" + } + return &PlacementRefusedError{Path: path, Why: fmt.Sprintf("it is below %s's home and declared %s's; "+ + "below a home only that account's directories are placed", a.Name, owner)} + } + } + return nil +} + +// moduleOfID is the module a resource id names, or "". +func moduleOfID(id string) string { + module, _ := moduleOf(id) + return module +} + +// refusedPlaces judges every directory and access of a declaration before anything is applied, and answers +// each refusal by path: what is refused is refused again as a container's mount source. +func refusedPlaces(resources []declaration.Resource) map[string]error { + out := map[string]error{} + for _, r := range resources { + var err error + switch res := r.(type) { + case *declaration.Directory: + err = refusePath(res.Path, placingDirectory, moduleOfID(res.ID), res.Owner) + case *declaration.Access: + err = refusePath(res.Path, placingAccess, moduleOfID(res.ID), "") + default: + continue + } + if err != nil { + out[filepath.Clean(r.Target())] = err + } + } + return out +} + +// refuseMounts says why a container's mounts are not made; nil when they may be. +func refuseMounts(c *declaration.Container, refused map[string]error) error { + for _, v := range c.Volumes { + src := mountSource(v) + if !strings.HasPrefix(src, "/") { + continue // a named volume, which the runtime keeps in its own tree + } + src = filepath.Clean(src) + for path, why := range refused { + if atOrBelow(src, path) { + var refusal *PlacementRefusedError + if errors.As(why, &refusal) { + return &PlacementRefusedError{Path: src, Why: "it is mounted from " + path + + ", which is refused: " + refusal.Why} + } + return why + } + } + if err := refusePath(src, placingMount, moduleOfID(c.ID), ""); err != nil { + return err + } + } + return nil +} diff --git a/internal/apply/placement_guard_test.go b/internal/apply/placement_guard_test.go new file mode 100644 index 0000000..68d53bb --- /dev/null +++ b/internal/apply/placement_guard_test.go @@ -0,0 +1,195 @@ +package apply + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-host/internal/store" +) + +// Where the node-engine places nothing and mounts nothing, whoever asks (novox/hq issue 339). A module's +// `places` setting moved a directory anywhere, with an owner it named, and the engine — root — made it so: a +// directory at /etc owned by a caller's account handed it /etc. Each test here is a declaration the controller +// might send; the engine refuses it whatever the controller decided. + +func applyOnce(t *testing.T, resources string, run Runner, state store.State) (Report, store.State, error) { + t.Helper() + if run == nil { + run = noServices + } + return Apply(context.Background(), archHost(t), declare(t, resources), state, store.OriginDeclared, run, nil, nil) +} + +func refusedFor339(t *testing.T, what string, err error) { + t.Helper() + if err == nil || !strings.Contains(err.Error(), "issue 339") || !strings.Contains(err.Error(), "nothing was touched") { + t.Fatalf("%s was not refused as the machine's own: %v", what, err) + } +} + +func TestNoDirectoryIsPlacedAtTheMachinesOwn(t *testing.T) { + for _, path := range []string{"/", "/etc", "/etc/", "/usr", "/var", "/var/lib", "/home", "/run", "/boot", "/root", + "/var/lib/mesh-host", "/var/lib/mesh-host/identity", "/proc/1/task", "/sys/kernel", "/dev/shm", "/root/.ssh", + "/srv/../etc"} { + _, _, err := applyOnce(t, `{"id":"notes.data","type":"directory","path":"`+path+`","mode":"0755"}`, nil, store.State{}) + refusedFor339(t, path, err) + } + // Below /etc, /usr or /run, a directory is root's: the machine's configuration is read as root's word. + _, _, err := applyOnce(t, `{"id":"notes.data","type":"directory","path":"/etc/sudoers.d","owner":"1001:1001"}`, + nil, store.State{}) + refusedFor339(t, "/etc/sudoers.d owned by another account", err) + _, _, err = applyOnce(t, `{"id":"notes.data","type":"directory","path":"/etc/systemd/system/getty@.service.d",`+ + `"owner":"1001:1001"}`, nil, store.State{}) + refusedFor339(t, "a drop-in directory owned by another account", err) +} + +func TestNoAccessIsAtTheMachinesOwn(t *testing.T) { + for _, path := range []string{"/", "/etc", "/etc/ssl", "/usr", "/var/lib", "/home", "/root", "/proc", + "/var/lib/mesh-host", "/run/docker"} { + _, _, err := applyOnce(t, `{"id":"notes.media","type":"access","path":"`+path+`"}`, nil, store.State{}) + refusedFor339(t, "an access at "+path, err) + } +} + +// A container is never handed the machine's own as a mount, nor a directory or access the engine refused. +func TestNoContainerMountsTheMachinesOwn(t *testing.T) { + ran := false + run := func(ctx context.Context, name string, args ...string) (string, error) { + switch { + case args[0] == "info": + return "27.0\n", nil + case args[0] == "container": + return "", os.ErrNotExist + case args[0] == "run": + ran = true + return "deadbeef\n", nil + } + return "", nil + } + for _, mount := range []string{"/:/host", "/etc:/host-etc", "/var/lib:/data", "/home:/homes:ro", + "/var/lib/mesh-host:/state", "/proc:/p", "/dev:/d", "/root/.ssh:/keys", "/boot/efi:/efi"} { + ran = false + _, _, err := Apply(context.Background(), archHost(t), parseTrusted(t, `{"declaration":1,"resources":[ + {"id":"notes.server","type":"container","name":"notes","image":"`+pinned+`","volumes":["`+mount+`"]} + ]}`), store.State{}, store.OriginCarried, run, nil, nil) + refusedFor339(t, "a container mounting "+mount, err) + if ran { + t.Fatalf("a container mounting %s was run", mount) + } + } +} + +// The path is not followed: a link where a directory is declared, or a link on the way to it, is judged where +// it leads. +func TestALinkIsNotFollowedToTheMachinesOwn(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "target") + if err := os.Mkdir(target, 0o700); err != nil { + t.Fatal(err) + } + link := filepath.Join(dir, "link") + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + _, _, err := applyOnce(t, `{"id":"notes.data","type":"directory","path":"`+link+`","mode":"0755"}`, nil, store.State{}) + refusedFor339(t, "a directory that is a link", err) + if info, _ := os.Stat(target); info.Mode().Perm() != 0o700 { + t.Fatalf("the link's target was changed to %o", info.Mode().Perm()) + } + // A link on the way: /via → /usr, so /via/share is /usr/share. + via := filepath.Join(dir, "via") + if err := os.Symlink("/usr", via); err != nil { + t.Fatal(err) + } + _, _, err = applyOnce(t, `{"id":"notes.data","type":"directory","path":"`+via+`/share"}`, nil, store.State{}) + refusedFor339(t, "a directory reached through a link to /usr", err) + _, _, err = applyOnce(t, `{"id":"notes.media","type":"access","path":"`+via+`"}`, nil, store.State{}) + refusedFor339(t, "an access reached through a link to /usr", err) +} + +// A directory found here, that the mesh did not make, keeps its owner and mode; one the mesh made keeps the +// declared mode, as before. +func TestAFoundDirectoryIsUsedAsFound(t *testing.T) { + dir := t.TempDir() + found := filepath.Join(dir, "found") + if err := os.Mkdir(found, 0o700); err != nil { + t.Fatal(err) + } + report, state, err := applyOnce(t, `{"id":"notes.data","type":"directory","path":"`+found+`","mode":"0755"}`, + nil, store.State{}) + if err != nil { + t.Fatal(err) + } + if info, _ := os.Stat(found); info.Mode().Perm() != 0o700 { + t.Fatalf("a directory the mesh did not make was chmodded to %o", info.Mode().Perm()) + } + if len(report.Outcomes) != 1 || !strings.Contains(report.Outcomes[0].Detail, "used as found") { + t.Fatalf("the report does not say it was used as found: %+v", report.Outcomes) + } + if a, _ := state.Find("notes.data"); !a.AsFound { + t.Fatal("the record does not say the directory was found") + } + // Still found at the next apply, and never removed: undeclared, it is forgotten and left. + if _, state, err = applyOnce(t, `{"id":"notes.data","type":"directory","path":"`+found+`","mode":"0755"}`, + nil, state); err != nil { + t.Fatal(err) + } + if info, _ := os.Stat(found); info.Mode().Perm() != 0o700 { + t.Fatalf("a found directory was chmodded at the next apply to %o", info.Mode().Perm()) + } + if _, _, err = applyOnce(t, `{"id":"notes.other","type":"directory","path":"`+filepath.Join(dir, "other")+`"}`, + nil, state); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(found); err != nil { + t.Fatalf("a found directory was removed: %v", err) + } + + // The mesh's own: made by it, its mode is maintained. + made := filepath.Join(dir, "made") + _, state, err = applyOnce(t, `{"id":"notes.made","type":"directory","path":"`+made+`","mode":"0755"}`, nil, store.State{}) + if err != nil { + t.Fatal(err) + } + if err := os.Chmod(made, 0o700); err != nil { + t.Fatal(err) + } + if _, state, err = applyOnce(t, `{"id":"notes.made","type":"directory","path":"`+made+`","mode":"0755"}`, + nil, state); err != nil { + t.Fatal(err) + } + if info, _ := os.Stat(made); info.Mode().Perm() != 0o755 { + t.Fatalf("the mesh's own directory was not given its mode back: %o", info.Mode().Perm()) + } + // Moved — a placement changed — onto a directory that was already there: that one is found, not the mesh's. + if _, _, err = applyOnce(t, `{"id":"notes.made","type":"directory","path":"`+found+`","mode":"0755"}`, + nil, state); err != nil { + t.Fatal(err) + } + if info, _ := os.Stat(found); info.Mode().Perm() != 0o700 { + t.Fatalf("a placement moved onto a found directory chmodded it to %o", info.Mode().Perm()) + } +} + +// The module's own directories keep being placed: below /var/lib, below /etc as root's, a container's mount of +// the machine's docker socket. +func TestAModulesOwnPlacesAreStillPlaced(t *testing.T) { + for _, path := range []string{"/var/lib/notes/data", "/etc/systemd/logind.conf.d", "/var/run/fail2ban", + "/srv/notes", "/mnt/plex/config", "/storage/media", "/var/lib/mesh/registry"} { + if err := refusePath(path, placingDirectory, "notes", ""); err != nil { + t.Errorf("%s: %v", path, err) + } + } + for _, path := range []string{"/var/lib/minio-store", "/var/run/docker.sock", "/services/ollama", "/storage/media", + "/etc/localtime", "/proc/cpuinfo", "/dev/null", "/dev/dri"} { + if err := refusePath(path, placingMount, "notes", ""); err != nil { + t.Errorf("a mount of %s: %v", path, err) + } + } + if err := refusePath("/var/lib/notes/data", placingDirectory, "notes", "1001:2000"); err != nil { + t.Errorf("an owner below /var/lib: %v", err) + } +} diff --git a/internal/apply/placement_homes_test.go b/internal/apply/placement_homes_test.go new file mode 100644 index 0000000..39c53e2 --- /dev/null +++ b/internal/apply/placement_homes_test.go @@ -0,0 +1,67 @@ +package apply + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strconv" + "testing" + + "github.com/novox/mesh-host/internal/store" +) + +// Below a person's or an agent's home, a directory is that account's (novox/hq issue 339): root's, or another +// account's, there is one the account does not control in a tree whose every parent it does — and a container +// mounting one the engine refused is refused with it. +func TestBelowAHomeOnlyThatAccountsDirectoryIsPlaced(t *testing.T) { + dir := t.TempDir() + home := filepath.Join(dir, "home", "alice") + if err := os.MkdirAll(home, 0o755); err != nil { + t.Fatal(err) + } + me := os.Getuid() + passwd := filepath.Join(dir, "passwd") + if err := os.WriteFile(passwd, fmt.Appendf(nil, "root:x:0:0::/root:/bin/sh\nalice:x:%d:%d::%s:/bin/sh\n", + me, os.Getgid(), home), 0o644); err != nil { + t.Fatal(err) + } + before := passwdFile + passwdFile = passwd + t.Cleanup(func() { passwdFile = before }) + + _, _, err := applyOnce(t, `{"id":"notes.data","type":"directory","path":"`+home+`/.config/autostart"}`, nil, store.State{}) + refusedFor339(t, "root's directory below alice's home", err) + _, _, err = applyOnce(t, `{"id":"notes.data","type":"directory","path":"`+home+`/.config/autostart",`+ + `"owner":"`+strconv.Itoa(me+1)+`"}`, nil, store.State{}) + refusedFor339(t, "another account's directory below alice's home", err) + if _, err := os.Stat(filepath.Join(home, ".config")); err == nil { + t.Fatal("a refused directory's parent was made") + } + // Alice's own is placed. + if _, _, err := applyOnce(t, `{"id":"notes.data","type":"directory","path":"`+home+`/.config/notes",`+ + `"owner":"`+strconv.Itoa(me)+":"+strconv.Itoa(os.Getgid())+`"}`, nil, store.State{}); err != nil { + t.Fatalf("alice's own directory: %v", err) + } + // A container mounting a refused directory is refused with it, and not run. + ran := false + run := func(ctx context.Context, name string, args ...string) (string, error) { + switch args[0] { + case "info": + return "27.0\n", nil + case "container": + return "", os.ErrNotExist + case "run": + ran = true + } + return "", nil + } + _, _, err = Apply(context.Background(), archHost(t), parseTrusted(t, `{"declaration":1,"resources":[ + {"id":"notes.data","type":"directory","path":"`+home+`/.ssh"}, + {"id":"notes.server","type":"container","name":"notes","image":"`+pinned+`","volumes":["`+home+`/.ssh:/keys"]} + ]}`), store.State{}, store.OriginCarried, run, nil, nil) + refusedFor339(t, "a container mounting a refused directory", err) + if ran { + t.Fatal("a container mounting a refused directory was run") + } +} diff --git a/internal/store/store.go b/internal/store/store.go index 01db4d6..a7ca2a6 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -131,6 +131,11 @@ type Applied struct { // anything else in the directory, and leaves it in place. Unpacked *Unpacked `json:"unpacked,omitempty"` + // AsFound is, for a directory, that it was there before the mesh first applied it and was not the + // declared owner and mode (novox/hq issue 339): the mesh uses it as found, never changing its owner or + // mode, and never removes it. Absent on a record from before: such a directory is the mesh's. + AsFound bool `json:"as_found,omitempty"` + // Reads is, for a container, the digest of each file it was created reading — its env-files // and the files mounted into it — by path (novox/hq 04-ISSUES/103). // From f39dbdbaf0730a03a0c97f0ea533ee0170f45c47 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 00:10:28 +0200 Subject: [PATCH 2/2] Give the home test's account a uid of its own, so it holds when the suite runs as root --- internal/apply/placement_homes_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/apply/placement_homes_test.go b/internal/apply/placement_homes_test.go index 39c53e2..6d924f9 100644 --- a/internal/apply/placement_homes_test.go +++ b/internal/apply/placement_homes_test.go @@ -20,10 +20,11 @@ func TestBelowAHomeOnlyThatAccountsDirectoryIsPlaced(t *testing.T) { if err := os.MkdirAll(home, 0o755); err != nil { t.Fatal(err) } - me := os.Getuid() + // A uid of its own, never the test's: the build seat runs tests as root, and root's home is no person's. + const alice = 4242 passwd := filepath.Join(dir, "passwd") if err := os.WriteFile(passwd, fmt.Appendf(nil, "root:x:0:0::/root:/bin/sh\nalice:x:%d:%d::%s:/bin/sh\n", - me, os.Getgid(), home), 0o644); err != nil { + alice, alice, home), 0o644); err != nil { t.Fatal(err) } before := passwdFile @@ -33,14 +34,13 @@ func TestBelowAHomeOnlyThatAccountsDirectoryIsPlaced(t *testing.T) { _, _, err := applyOnce(t, `{"id":"notes.data","type":"directory","path":"`+home+`/.config/autostart"}`, nil, store.State{}) refusedFor339(t, "root's directory below alice's home", err) _, _, err = applyOnce(t, `{"id":"notes.data","type":"directory","path":"`+home+`/.config/autostart",`+ - `"owner":"`+strconv.Itoa(me+1)+`"}`, nil, store.State{}) + `"owner":"`+strconv.Itoa(alice+1)+`"}`, nil, store.State{}) refusedFor339(t, "another account's directory below alice's home", err) if _, err := os.Stat(filepath.Join(home, ".config")); err == nil { t.Fatal("a refused directory's parent was made") } - // Alice's own is placed. - if _, _, err := applyOnce(t, `{"id":"notes.data","type":"directory","path":"`+home+`/.config/notes",`+ - `"owner":"`+strconv.Itoa(me)+":"+strconv.Itoa(os.Getgid())+`"}`, nil, store.State{}); err != nil { + // Alice's own is placed (judged, not applied: a chown to her needs root). + if err := refusePath(home+"/.config/notes", placingDirectory, "notes", strconv.Itoa(alice)+":"+strconv.Itoa(alice)); err != nil { t.Fatalf("alice's own directory: %v", err) } // A container mounting a refused directory is refused with it, and not run.