From cdbff1777e26011fdc358cf1144415057782ccd0 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 22:23:04 +0200 Subject: [PATCH 1/3] Take a directory in the mesh's own tree that the engine's account owns as the mesh's, not as found (issue 1873) postgres's data directory moved to /var/lib/mesh/postgres. On ace and novox that directory was there, root:root 0700, before the directory resource was applied, so the engine used it as found (issue 339's rule) and left it to root: the postgres provider, running as the operator's account, cannot reach its own secrets, consumers and grants there. /var/lib/mesh is the engine's tree (root, 0755): only the engine's account makes a directory in it. A directory below it owned by that account is now the mesh's and is given its declared owner and mode. Anywhere else, or owned by another account, issue 339's rule stands. --- internal/apply/apply.go | 15 +++++++ internal/apply/mesh_dirs.go | 5 +++ internal/apply/mesh_tree_found_test.go | 62 ++++++++++++++++++++++++++ 3 files changed, 82 insertions(+) create mode 100644 internal/apply/mesh_tree_found_test.go diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 5162f19..5c553eb 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -1482,6 +1482,9 @@ 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 + // In the mesh's own tree, a directory the engine's own account owns was made by the mesh, on the way to + // something below it or by an earlier apply that did not record it: it is the mesh's (novox/hq issue 1873). + ours = ours || madeInTheMeshsTree(r.Path, before) if !ours { owned, err := ownedBy(r.Path, r.Owner) if err != nil { @@ -1557,6 +1560,18 @@ func applyDirectory(r *declaration.Directory, previous store.Applied, wasBefore return out, nil } +// madeInTheMeshsTree is whether a directory lies in the mesh's own tree (meshDataRoot, below it, never the root +// itself) and belongs to the engine's own account: only that account makes a directory there, so it is the mesh's +// and not found (novox/hq issue 1873). Anywhere else, or owned by any other account, issue 339's rule stands. +func madeInTheMeshsTree(path string, found os.FileInfo) bool { + root := filepath.Clean(meshDataRoot) + if !strings.HasPrefix(filepath.Clean(path), root+string(filepath.Separator)) { + return false + } + uid, _, ok := ownerOf(found) + return ok && uid == os.Geteuid() +} + // 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 diff --git a/internal/apply/mesh_dirs.go b/internal/apply/mesh_dirs.go index d21094e..78c2c8d 100644 --- a/internal/apply/mesh_dirs.go +++ b/internal/apply/mesh_dirs.go @@ -124,3 +124,8 @@ func JudgeDirectories(d *declaration.Declaration) string { placed+refused+guarded+homed, placed, refused, guarded, homed) return strings.Join(append([]string{head}, lines...), "\n") } + +// meshDataRoot is the mesh's own tree, where a module placed "mesh" keeps its directory (/mesh/): +// the engine's, 0755, so only the engine's account makes a directory in it (novox/hq issue 1873). A variable so a +// test can place it. +var meshDataRoot = "/var/lib/mesh" diff --git a/internal/apply/mesh_tree_found_test.go b/internal/apply/mesh_tree_found_test.go new file mode 100644 index 0000000..f37a893 --- /dev/null +++ b/internal/apply/mesh_tree_found_test.go @@ -0,0 +1,62 @@ +package apply + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-host/internal/store" +) + +// novox/hq issue 1873: a module's directory in the mesh's own tree (/mesh/, place "mesh") that is +// there before its declaration, owned by the engine's own account, was made by the mesh on the way to something below +// it: it is the mesh's, given its declared mode and owner, never used as found. Only the engine's account makes a +// directory in a tree that is the engine's (0755, root); on ace and novox /var/lib/mesh/postgres was there as +// root:root 0700 before postgres's data directory was applied, and the engine left it to root, so the postgres +// provider, running as the operator's account, could not reach its own files. +func TestADirectoryInTheMeshsOwnTreeMadeByTheEngineIsTheMeshs(t *testing.T) { + root := t.TempDir() + before := meshDataRoot + meshDataRoot = filepath.Join(root, "mesh") + t.Cleanup(func() { meshDataRoot = before }) + made := filepath.Join(meshDataRoot, "postgres") + if err := os.MkdirAll(made, 0o700); err != nil { + t.Fatal(err) + } + if err := os.Chmod(made, 0o700); err != nil { + t.Fatal(err) + } + decl := `{"id":"postgres.data","type":"directory","path":"` + made + `","mode":"0750"}` + var said []string + _, state, err := Apply(context.Background(), archHost(t), declare(t, decl), store.State{}, store.OriginDeclared, + noServices, func(line string) { said = append(said, line) }, nil) + if err != nil { + t.Fatal(err) + } + if strings.Contains(strings.Join(said, "\n"), "WARNING postgres.data") { + t.Errorf("a directory the engine's account made in the mesh's tree was used as found: %q", said) + } + if a, _ := state.Find("postgres.data"); a.AsFound { + t.Error("it is recorded as found") + } + if info, _ := os.Stat(made); info.Mode().Perm() != 0o750 { + t.Errorf("it was not given its declared mode: %04o", info.Mode().Perm()) + } + + // Outside the mesh's tree the rule of issue 339 stands: found, used as found. + other := filepath.Join(root, "var-lib", "notes") + if err := os.MkdirAll(other, 0o700); err != nil { + t.Fatal(err) + } + said = nil + decl = `{"id":"notes.data","type":"directory","path":"` + other + `","mode":"0750"}` + if _, _, err := Apply(context.Background(), archHost(t), declare(t, decl), store.State{}, store.OriginDeclared, + noServices, func(line string) { said = append(said, line) }, nil); err != nil { + t.Fatal(err) + } + if !strings.Contains(strings.Join(said, "\n"), "WARNING notes.data") { + t.Errorf("a directory found outside the mesh's tree was not used as found: %q", said) + } +} -- 2.54.0 From 03b9c47af4c46871ce6991d9338772e5bc468674 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 22:27:45 +0200 Subject: [PATCH 2/3] Judge the mesh's tree by where a path leads, not only by its text (issue 1873, review) A root-only link inside /var/lib/mesh pointing out of it would have made a directory outside the tree, owned by the engine's account, the mesh's to give away: the fault of issue 1873 again. The path and the tree's root are now both resolved, links followed, and the directory is the tree's only when the resolved path lies below the resolved root as well. --- internal/apply/apply.go | 16 +++++++++-- internal/apply/mesh_tree_found_test.go | 38 ++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 5c553eb..a6dac27 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -1564,14 +1564,26 @@ func applyDirectory(r *declaration.Directory, previous store.Applied, wasBefore // itself) and belongs to the engine's own account: only that account makes a directory there, so it is the mesh's // and not found (novox/hq issue 1873). Anywhere else, or owned by any other account, issue 339's rule stands. func madeInTheMeshsTree(path string, found os.FileInfo) bool { - root := filepath.Clean(meshDataRoot) - if !strings.HasPrefix(filepath.Clean(path), root+string(filepath.Separator)) { + // By its text and by where it leads: a link in the tree pointing out of it leads outside, and what is there is + // not the tree's (review of issue 1873). + if !inTheMeshsTree(path) { return false } uid, _, ok := ownerOf(found) return ok && uid == os.Geteuid() } +// inTheMeshsTree is whether a path lies strictly below the mesh's own tree, both as written and with every link in +// it followed (resolved), against the tree's root resolved the same way (novox/hq issue 1873). +func inTheMeshsTree(path string) bool { + root := filepath.Clean(meshDataRoot) + sep := string(filepath.Separator) + if !strings.HasPrefix(filepath.Clean(path), root+sep) { + return false + } + return strings.HasPrefix(resolved(filepath.Clean(path)), resolved(root)+sep) +} + // 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 diff --git a/internal/apply/mesh_tree_found_test.go b/internal/apply/mesh_tree_found_test.go index f37a893..a3603a0 100644 --- a/internal/apply/mesh_tree_found_test.go +++ b/internal/apply/mesh_tree_found_test.go @@ -1,3 +1,5 @@ +//go:build linux + package apply import ( @@ -7,6 +9,7 @@ import ( "strings" "testing" + "git.novox.be/novox/mesh-sdk/go/declaration" "github.com/novox/mesh-host/internal/store" ) @@ -60,3 +63,38 @@ func TestADirectoryInTheMeshsOwnTreeMadeByTheEngineIsTheMeshs(t *testing.T) { t.Errorf("a directory found outside the mesh's tree was not used as found: %q", said) } } + +// Review of issue 1873's fix: the tree is judged by where the path leads, not by its text. A root-only link inside the +// mesh's tree pointing out of it (to /var/lib/systemd, say) would otherwise make a directory outside the tree, owned by +// the engine's account, the mesh's to chown: issue 1873's fault again. It is used as found. +func TestALinkInTheMeshsTreeLeadingOutOfItIsNotTheTree(t *testing.T) { + rootAlone(t, "") + root := t.TempDir() + before := meshDataRoot + meshDataRoot = filepath.Join(root, "mesh") + t.Cleanup(func() { meshDataRoot = before }) + outside := filepath.Join(root, "var-lib", "systemd") + if err := os.MkdirAll(filepath.Join(outside, "data"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Chmod(filepath.Join(outside, "data"), 0o700); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(meshDataRoot, 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, filepath.Join(meshDataRoot, "postgres")); err != nil { + t.Fatal(err) + } + r := &declaration.Directory{ID: "postgres.data", Path: filepath.Join(meshDataRoot, "postgres", "data"), Mode: "0750"} + out, err := applyDirectory(r, store.Applied{}, true) + if err != nil { + t.Fatal(err) + } + if !out.asFound { + t.Errorf("a directory reached through a link out of the mesh's tree was taken as the mesh's: %+v", out) + } + if got := permOf(t, filepath.Join(outside, "data")); got != 0o700 { + t.Errorf("the directory outside the tree was given mode %o (was 700)", got) + } +} -- 2.54.0 From 0d12fb11a48be930b5b7b7660425db109f66b9a9 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 22:29:39 +0200 Subject: [PATCH 3/3] Make no container before a file it mounts is there, and clear the empty directory a runtime left at a file's path (issue 1873) On ace the postgres container was replaced at 22:11:51 mounting superuser.secret, a file the provisioner's client fills only after the apply. The runtime made the missing source a directory, and every fill after it failed with "is a directory". A container whose mount source is a file the declaration declares, not yet on the machine, now waits and is neither created nor replaced. The write of that file (WriteFilled) applies what reads it, as before, so the container is made once the file is there. What the runtime already left is cured by the mesh: a declared file whose path is an empty directory in the mesh's own tree (by its path and by where it leads), owned by the engine's account, has that directory removed before the file is written, and the outcome says so. A directory with anything in it, or outside the tree, is never removed. --- internal/apply/apply.go | 27 +++++++ internal/apply/filled.go | 1 + internal/apply/mount_waits.go | 58 ++++++++++++++ internal/apply/mount_waits_test.go | 121 +++++++++++++++++++++++++++++ 4 files changed, 207 insertions(+) create mode 100644 internal/apply/mount_waits.go create mode 100644 internal/apply/mount_waits_test.go diff --git a/internal/apply/apply.go b/internal/apply/apply.go index a6dac27..a22e345 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -584,6 +584,7 @@ func ApplyMindingWindows( in.refused = refusedPlaces(d.Resources) in.agentHomes = rootNeverHomes(d) in.dirsBefore = map[string]bool{} + in.files = declaredFiles(d.Resources) for _, resource := range d.Resources { in.declares[resource.Identity()] = declaredDigest(resource) if dir, ok := resource.(*declaration.Directory); ok { @@ -1333,6 +1334,16 @@ func applyOne(ctx context.Context, sys system.System, r declaration.Resource, ru "created once it is (novox/hq ADR 0328 §9)" return out, nil } + // Not created or replaced before a file it mounts is there: the runtime would make the missing source a + // directory, and the file could never be written there (novox/hq issue 1873). The write of the file applies + // what reads it (WriteFilled, readsChange), this container among them. + if src := mountsAFileNotThere(res, in.files); src != "" { + out := begin(r) + out.Action = "waiting" + out.Detail = "not created: it mounts " + src + ", a file the declaration declares that is not there " + + "yet; created once it is (novox/hq issue 1873)" + return out, nil + } } // Nothing below a home is touched through a link an account put there (novox/hq ADR 0266). // And nothing is unpacked or written into below an agent's home (agent_home_guard.go). @@ -1633,6 +1644,19 @@ func applyHandedOver(r *declaration.Directory, out Outcome, mode os.FileMode) (O // keepFound, when not nil, is where the original of a file this host has no record of is kept // before it is written over (novox/hq ADR 0100): once, never overwritten, and named in the outcome. func applyFile(r *declaration.File, previous store.Applied, unseal Unseal, keepFound Keep) (Outcome, error) { + cleared, err := clearRuntimesEmptyDirectory(r.Path) + if err != nil { + return begin(r), err + } + out, err := applyFileAt(r, previous, unseal, keepFound) + if cleared != "" { + out.Detail = joinDetail(cleared, out.Detail) + } + return out, err +} + +// applyFileAt writes a declared file: applyFile's work once its path is cleared of what a runtime left there. +func applyFileAt(r *declaration.File, previous store.Applied, unseal Unseal, keepFound Keep) (Outcome, error) { if r.Into == declaration.IntoBlock { return applyBlock(r, previous) } @@ -2465,6 +2489,9 @@ type inputs struct { // held is the root of every library whose disk or mount is not there, with its name: no container that mounts // it is created (novox/hq ADR 0328 §9). held map[string]string + // files is the path of every file the declaration declares: a container mounting one that is not there yet waits + // for it (novox/hq issue 1873). + files map[string]bool // handed is every group a record that went this apply handed to a resource still asking for it. handed Handed } diff --git a/internal/apply/filled.go b/internal/apply/filled.go index e010323..5d4d2ca 100644 --- a/internal/apply/filled.go +++ b/internal/apply/filled.go @@ -76,6 +76,7 @@ func WriteFilled( in.refused = refusedPlaces(d.Resources) in.agentHomes = rootNeverHomes(d) in.dirsBefore = map[string]bool{} + in.files = declaredFiles(d.Resources) // A container that reads the file and mounts a library whose disk or mount is not there is held, as a full apply // holds it: not created over the missing disk (novox/hq ADR 0328 §9). in.held = heldLibraries(ctx, d.Libraries) diff --git a/internal/apply/mount_waits.go b/internal/apply/mount_waits.go new file mode 100644 index 0000000..e6d8661 --- /dev/null +++ b/internal/apply/mount_waits.go @@ -0,0 +1,58 @@ +package apply + +import ( + "errors" + "os" + "path/filepath" + + "git.novox.be/novox/mesh-sdk/go/declaration" +) + +// declaredFiles is the path of every file a declaration declares. +func declaredFiles(resources []declaration.Resource) map[string]bool { + out := map[string]bool{} + for _, r := range resources { + if f, ok := r.(*declaration.File); ok { + out[filepath.Clean(f.Path)] = true + } + } + return out +} + +// mountsAFileNotThere is the first mount source of a container that is a file the declaration declares and is not +// on the machine yet, or "" (novox/hq issue 1873): the runtime makes a missing source a directory. +func mountsAFileNotThere(c *declaration.Container, files map[string]bool) string { + for _, v := range c.Volumes { + src := mountSource(v) + if !filepath.IsAbs(src) || !files[filepath.Clean(src)] { + continue + } + if _, err := os.Lstat(src); errors.Is(err, os.ErrNotExist) { + return src + } + } + return "" +} + +// clearRuntimesEmptyDirectory removes what a container runtime leaves at a file's path when it mounted the file before +// it was there (novox/hq issue 1873): an empty directory, in the mesh's own tree by its path and by where its path +// leads, owned by the engine's own account. It says what it removed, or "" when nothing was. Anything else at the path +// is left for the write to meet as before: a directory with anything in it, or one outside the tree, is never removed. +func clearRuntimesEmptyDirectory(path string) (string, error) { + info, err := os.Lstat(path) + if err != nil || !info.IsDir() || !inTheMeshsTree(path) { + return "", nil + } + if uid, _, ok := ownerOf(info); !ok || uid != os.Geteuid() { + return "", nil + } + entries, err := os.ReadDir(path) + if err != nil || len(entries) > 0 { + return "", nil + } + if err := os.Remove(path); err != nil { + return "", err + } + return "removed the empty directory a container runtime left at " + path + ", where this file belongs " + + "(novox/hq issue 1873)", nil +} diff --git a/internal/apply/mount_waits_test.go b/internal/apply/mount_waits_test.go new file mode 100644 index 0000000..5e2c33d --- /dev/null +++ b/internal/apply/mount_waits_test.go @@ -0,0 +1,121 @@ +//go:build linux + +package apply + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-host/internal/store" +) + +// novox/hq issue 1873, its second fault: on ace the postgres container was replaced at 22:11:51 mounting +// superuser.secret, a file the provisioner's client fills only after the apply; the runtime made the missing source +// a directory, and every fill after it failed with "is a directory". A container whose mount source is a file the +// declaration declares, not yet there, is not created or replaced: it waits, and the write of that file (WriteFilled) +// applies what reads it, the container among them (readsChange), once the file is there. +func TestAContainerWaitsForAFileItMountsAndIsMadeWhenTheFillLands(t *testing.T) { + secret := filepath.Join(t.TempDir(), "postgres", "superuser.secret") + d := filledDecl(t, secret, `"content":"","mode":"0600","owner-at-runtime":"owned"`, + `{"id":"postgres.server","type":"container","name":"postgres","image":"`+pinned+`","volumes":["`+ + secret+`:/run/secrets/superuser:ro"]}`) + m := &machine{containers: map[string]*fakeContainer{}} + report, state, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared, m.run, nil, nil) + if err != nil { + t.Fatal(err) + } + if o := outcomeOf(report, "postgres.server"); o.Action != "waiting" || !strings.Contains(o.Detail, secret) { + t.Errorf("the container mounting a file not yet filled is %+v, not waiting for it", o) + } + if m.did("docker run") { + t.Fatalf("the container was made before the file it mounts was there: %v", m.asked) + } + if _, err := os.Lstat(secret); err == nil { + t.Fatal("something was made at the file's path before the fill") + } + + report, _, err = WriteFilled(context.Background(), archHost(t), d, state, secret, []byte("s3cret"), m.run, nil, + nil, nil) + if err != nil { + t.Fatal(err) + } + if !m.did("docker run") { + t.Fatalf("the container was not made when the file it mounts was filled: %v", m.asked) + } + if info, err := os.Lstat(secret); err != nil || !info.Mode().IsRegular() { + t.Fatalf("the filled file is not a file: %v %v", info, err) + } +} + +// novox/hq issue 1873: what the runtime left on ace is cured by the mesh. A declared file whose path is an empty +// directory in the mesh's own tree, the engine's account's, is the runtime's missing mount source: the engine +// removes that empty directory and writes the file, and says so. A directory with anything in it, or one outside the +// tree, is never removed: the write fails as before. +func TestAnEmptyDirectoryTheRuntimeLeftAtAFilesPathIsReplacedByTheFile(t *testing.T) { + rootAlone(t, "") + root := t.TempDir() + before := meshDataRoot + meshDataRoot = filepath.Join(root, "mesh") + t.Cleanup(func() { meshDataRoot = before }) + + secret := filepath.Join(meshDataRoot, "postgres", "superuser.secret") + if err := os.MkdirAll(secret, 0o755); err != nil { + t.Fatal(err) + } + d := filledDecl(t, secret, `"content":"","mode":"0600","owner-at-runtime":"owned"`, "") + m := &machine{containers: map[string]*fakeContainer{}} + report, _, err := WriteFilled(context.Background(), archHost(t), d, store.State{}, secret, []byte("s3cret"), m.run, + nil, nil, nil) + if err != nil { + t.Fatalf("the file was not written over the empty directory the runtime left: %v", err) + } + if info, err := os.Lstat(secret); err != nil || !info.Mode().IsRegular() { + t.Fatalf("%s is not a file: %v %v", secret, info, err) + } + if o := outcomeOf(report, "m.conf"); !strings.Contains(o.Detail, "empty directory") { + t.Errorf("the write did not say it removed the empty directory: %+v", o) + } + + // Not empty: kept, and the write fails. + full := filepath.Join(meshDataRoot, "postgres", "reader.secret") + if err := os.MkdirAll(filepath.Join(full, "something"), 0o755); err != nil { + t.Fatal(err) + } + d = filledDecl(t, full, `"content":"","mode":"0600","owner-at-runtime":"owned"`, "") + if _, _, err := WriteFilled(context.Background(), archHost(t), d, store.State{}, full, []byte("x"), m.run, nil, + nil, nil); err == nil { + t.Error("a directory with something in it was replaced by a file") + } + if _, err := os.Stat(filepath.Join(full, "something")); err != nil { + t.Errorf("what was in the directory is gone: %v", err) + } + + // Outside the tree: kept, and the write fails. + outside := filepath.Join(root, "var-lib", "postgres", "superuser.secret") + if err := os.MkdirAll(outside, 0o755); err != nil { + t.Fatal(err) + } + d = filledDecl(t, outside, `"content":"","mode":"0600","owner-at-runtime":"owned"`, "") + if _, _, err := WriteFilled(context.Background(), archHost(t), d, store.State{}, outside, []byte("x"), m.run, nil, + nil, nil); err == nil { + t.Error("an empty directory outside the mesh's tree was replaced by a file") + } + + // Reached through a link out of the tree: kept, and the write fails. + elsewhere := filepath.Join(root, "elsewhere") + if err := os.MkdirAll(filepath.Join(elsewhere, "x.secret"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(elsewhere, filepath.Join(meshDataRoot, "linked")); err != nil { + t.Fatal(err) + } + through := filepath.Join(meshDataRoot, "linked", "x.secret") + d = filledDecl(t, through, `"content":"","mode":"0600","owner-at-runtime":"owned"`, "") + _, _, _ = WriteFilled(context.Background(), archHost(t), d, store.State{}, through, []byte("x"), m.run, nil, nil, nil) + if info, err := os.Lstat(filepath.Join(elsewhere, "x.secret")); err != nil || !info.IsDir() { + t.Errorf("a directory outside the tree, reached through a link in it, was replaced: %v %v", info, err) + } +} -- 2.54.0