From 8115f1ac42bf8291a2c9c05b2086b920df8b6999 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 18:59:07 +0200 Subject: [PATCH] Judge a definition's resolved paths where the definition is judged, by the node-engine's own rules, so a refusal never freezes a machine (review of #228, issue 496) --- internal/catalogue/dir_into.go | 5 +- internal/catalogue/engine_guard_test.go | 71 +++ internal/catalogue/manifest.go | 1 + internal/catalogue/placement.go | 134 +++-- internal/catalogue/resolved_path_test.go | 168 +++--- testdata/beside/CAPTURED | 8 + .../apply/placement_guard.go.captured | 549 ++++++++++++++++++ 7 files changed, 800 insertions(+), 136 deletions(-) create mode 100644 internal/catalogue/engine_guard_test.go create mode 100644 testdata/beside/mesh-host/internal/apply/placement_guard.go.captured diff --git a/internal/catalogue/dir_into.go b/internal/catalogue/dir_into.go index 3a9a919f..585358e3 100644 --- a/internal/catalogue/dir_into.go +++ b/internal/catalogue/dir_into.go @@ -257,10 +257,7 @@ func dirInto(resource map[string]any, dirs map[string]string, module string) err } resource["env-file"] = filled } - // **Placed, then judged** (novox/hq issue 496): the path a directory or file resolves to — the default - // layout, a place, a stated path, one beneath a ${dir:…} — is judged here, where every plan and the merge - // gate compose it, so a module whose directory lands in Docker's data is refused before it reaches a machine. - return judgeResolved(resource, module) + return nil } // unknownDirRefs is every ${dir:…} in the definition that names no directory the definition diff --git a/internal/catalogue/engine_guard_test.go b/internal/catalogue/engine_guard_test.go new file mode 100644 index 00000000..85b75970 --- /dev/null +++ b/internal/catalogue/engine_guard_test.go @@ -0,0 +1,71 @@ +package catalogue + +import ( + "go/ast" + "go/parser" + "go/token" + "path/filepath" + "slices" + "strconv" + "testing" + + "github.com/novox/mesh-controller/internal/beside" +) + +// The controller refuses a definition's resolved path by the node-engine's own rules, no more (novox/hq issue +// 496): the same lists, read here from mesh-host's internal/apply/placement_guard.go — in a merge check the clone +// beside it at the commit the mesh runs, elsewhere the copy captured in testdata/beside. When the engine's lists +// move, this fails until the controller's move with them, so the gate never refuses what the engine applies nor +// passes what it refuses by path alone. +func TestTheResolvedPathRulesAreTheNodeEnginesOwn(t *testing.T) { + file := filepath.Join(beside.Dir(t, "mesh-host"), "internal", "apply", "placement_guard.go") + parsed, err := parser.ParseFile(token.NewFileSet(), file, nil, 0) + if err != nil { + t.Fatalf("the node-engine's guard does not parse: %v", err) + } + lists := map[string][]string{} + consts := map[string]string{} + ast.Inspect(parsed, func(n ast.Node) bool { + spec, ok := n.(*ast.ValueSpec) + if !ok { + return true + } + for i, name := range spec.Names { + if i >= len(spec.Values) { + continue + } + switch v := spec.Values[i].(type) { + case *ast.CompositeLit: + for _, elt := range v.Elts { + if lit, ok := elt.(*ast.BasicLit); ok && lit.Kind == token.STRING { + s, _ := strconv.Unquote(lit.Value) + lists[name.Name] = append(lists[name.Name], s) + } + } + case *ast.BasicLit: + if v.Kind == token.STRING { + consts[name.Name], _ = strconv.Unquote(v.Value) + } + } + } + return true + }) + for name, ours := range map[string][]string{ + "protectedRoots": protectedRoots, "forbiddenBelow": forbiddenBelow, "engineTrees": engineTrees, + } { + theirs := lists[name] + if len(theirs) == 0 { + t.Errorf("the node-engine's guard names no %s any more; read it and say where its rule went", name) + continue + } + a, b := slices.Clone(ours), slices.Clone(theirs) + slices.Sort(a) + slices.Sort(b) + if !slices.Equal(a, b) { + t.Errorf("%s differs from the node-engine's:\n controller %v\n node-engine %v", name, a, b) + } + } + if consts["engineModule"] != engineModule { + t.Errorf("the node-engine's own module is %q there and %q here", consts["engineModule"], engineModule) + } +} diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index be0e5871..e344cb94 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -2138,6 +2138,7 @@ func ParseManifest(raw []byte) (Manifest, error) { problems = append(problems, m.undeclaredMounts()...) problems = append(problems, m.unknownDirRefs()...) problems = append(problems, m.unknownAccessRefs()...) + problems = append(problems, m.resolvedPathProblems()...) problems = append(problems, m.jailProblems()...) // What a module adds to the account's environment and to the login shell, and the holder's // placeholders for them (novox/hq ADR 0203, ADR 0204) — here, so the catalogue check refuses diff --git a/internal/catalogue/placement.go b/internal/catalogue/placement.go index 24d1000b..400c5b3f 100644 --- a/internal/catalogue/placement.go +++ b/internal/catalogue/placement.go @@ -92,79 +92,103 @@ func systemPath(path string) string { return "" } -// Where no directory or file the controller resolves for a module may be, from any route (novox/hq issue 496). +// Where no directory or file a module's definition resolves to may be (novox/hq issue 496). // -// mesh-catalog #205 gave docker's `state` directory `"place": "."`, which resolves to /docker: /var/lib/docker, -// every container's filesystem. systemPath judged only the `places` and `accesses` settings, so the default layout -// and a definition's own paths reached the merge gate unjudged; it composed every machine and passed, and only the -// node-engine refused the directory, at apply, failing the walk. So every path a directory or file resolves to is -// judged where the plan is composed, which is also where the gate composes. +// mesh-catalog #205 gave docker's `state` directory `"place": "."`, which resolves to /docker: with the +// default root, /var/lib/docker, every container's filesystem. systemPath judged only the `places` and `accesses` +// settings, so the default layout and a definition's own paths reached the merge gate unjudged; it composed every +// machine and passed, and only the node-engine refused the directory, at apply, failing the walk. // -// Not by systemPath's whole list: a module writes the machine's configuration in /etc and /usr by design (units, -// jails, a launcher), keeps run directories in /run, an account's keys in its .ssh, and the mesh places its own files -// for every module in /var/lib/mesh. Those are the node-engine's to judge with what only the machine knows (owners, -// links, where the runtimes really keep their data). Refused here is what no module ever writes, whoever owns it: -// the trees in neverWritten, the node-engine's own trees but by its own module, and a path that is one of the -// machine's roots or holds one. Each is refused by the node-engine too (its placement guard, issue 339), so nothing -// that applies today is refused here. +// **Judged where a definition is judged, never where a machine is composed.** resolvedPathProblems runs in +// ParseManifest, which every route a definition takes into the mesh passes: `module check`, registration of a +// build (the builder's and the registry verbs'), and the merge gate's reading of the changed repository. A +// refusal there stops one definition before it reaches any machine. Composition reads registered manifests +// without ParseManifest, and judges nothing of this: refusing there would fail the whole machine's declaration and +// freeze every module on it for one module's path, where the node-engine fails only that resource. +// +// **The node-engine's rules, no more** (mesh-host's internal/apply/placement_guard.go, with files judged as +// directories are after novox/hq issue 495, rule 6). A path is refused when it is one of protectedRoots or holds +// one, when it is at or below a tree in forbiddenBelow, or at or below one of engineTrees and its module is not +// the node-engine's. What the engine judges with what only the machine knows stays the engine's: where the +// runtimes really keep their data, links, the accounts' homes, a directory's owner below /etc. The lists are the +// engine's own words, and a test (engine_guard_test.go) holds them equal to mesh-host's beside this repository. +// systemPath stays the stricter rule for a setting: a setting is an operator's word about one machine, and the +// trees it lists (/etc, /usr, /run, the mesh's own) are where the mesh's own modules write by design. var ( - neverWritten = []string{"/boot", "/root", "/proc", "/sys", "/dev", "/bin", "/sbin", "/lib", "/lib32", "/lib64", - "/var/spool", "/var/lib/docker", "/var/lib/containers", "/var/lib/containerd", "/opt"} + 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 = []string{"/proc", "/sys", "/dev", "/boot", "/root", "/var/spool", "/opt", "/var/lib/docker", + "/var/lib/containers", "/var/lib/containerd"} engineTrees = []string{"/var/lib/mesh-host", "/usr/lib/nox-mesh-host"} - // engineModule is the node-engine's own module, the one that places in engineTrees. - engineModule = "mesh-host" - // usrRoots are the roots below /usr that hold every program and library: never a module's directory itself. - usrRoots = []string{"/usr/bin", "/usr/lib", "/usr/lib64", "/usr/local", "/usr/local/bin", "/usr/local/lib", - "/usr/local/sbin", "/usr/sbin", "/usr/share"} ) -// resolvedSystemPath says why a path resolved for one of module's directories or files is the machine's own, or "". -func resolvedSystemPath(path, module string) string { - if !strings.HasPrefix(path, "/") { +// engineModule is the node-engine's own module, the one that places in engineTrees. +const engineModule = "mesh-host" + +// enginePath says why the node-engine refuses a directory or file at path for module, or "". +func enginePath(path, module string) string { + path = filepath.Clean(path) + if !filepath.IsAbs(path) { return "" } - path = filepath.Clean(path) - in := func(tree string) bool { return path == tree || strings.HasPrefix(path, tree+"/") } - for _, tree := range neverWritten { - if in(tree) { - return path + " is in " + tree + ", which no module writes in" + atOrBelow := func(tree string) bool { return path == tree || strings.HasPrefix(path, tree+"/") } + for _, root := range protectedRoots { + if path == root || path == "/" || strings.HasPrefix(root, path+"/") { + return root + " is one of the machine's own directories, and owning it is owning everything in it" + } + } + for _, tree := range forbiddenBelow { + if atOrBelow(tree) { + return "nothing is placed in " + tree } } if module != engineModule { for _, tree := range engineTrees { - if in(tree) { - return path + " is in " + tree + ", the node-engine's own, which only its module places in" + if atOrBelow(tree) { + return tree + " is the node-engine's own, placed in by its own module alone" } } } - for _, roots := range [][]string{systemRoots, systemTrees, usrRoots} { - for _, root := range roots { - if path == root { - return path + " is one of the machine's roots, the parent of everything in it" - } - } - } - for _, tree := range append(append([]string(nil), systemTrees...), neverWritten...) { - if strings.HasPrefix(tree, path+"/") { - return path + " holds " + tree + ", the machine's own or the mesh's state" - } - } return "" } -// judgeResolved refuses a directory or file whose resolved path resolvedSystemPath refuses (novox/hq issue 496). -func judgeResolved(resource map[string]any, module string) error { - kind := fmt.Sprint(resource["type"]) - if kind != "directory" && kind != "file" { - return nil +// resolvedPathProblems is every directory and file of the definition whose path, resolved as a node with the +// default root resolves it, the node-engine would refuse (novox/hq issue 496). A path still holding a placeholder +// only a machine fills (a setting, an access the definition gives no default) is the engine's to judge. +func (m Manifest) resolvedPathProblems() []string { + dirs := dirsFor(m, Rendering{}) + accesses := map[string]string{} + for _, a := range m.Accesses { + if a.ID != "" && a.Path != "" { + accesses[a.ID] = a.Path + } } - path, _ := resource["path"].(string) - if why := resolvedSystemPath(path, module); why != "" { - return fmt.Errorf("%s's %s %q resolves to %s: %s. The node-engine refuses it at apply, so it is refused "+ - "where the plan is composed, and the merge gate with it (novox/hq issue 496)", - module, kind, fmt.Sprint(resource["id"]), filepath.Clean(path), why) + var problems []string + for _, r := range m.Resources { + kind := fmt.Sprint(r["type"]) + if kind != "directory" && kind != "file" { + continue + } + id := fmt.Sprint(r["id"]) + path, _ := r["path"].(string) + if kind == "directory" && path == "" { + path = dirs[id] + } + path, _ = dirFill(path, dirs, m.Module) + path, _ = accessFill(path, accesses, m.Module) + if path == "" || strings.Contains(path, "${") { + continue + } + if why := enginePath(path, m.Module); why != "" { + problems = append(problems, fmt.Sprintf("%s's %s %q resolves to %s, which the node-engine refuses: %s. "+ + "A module's directories and files are judged when its definition is, so the merge gate refuses it "+ + "before any machine does (novox/hq issue 496)", m.Module, kind, id, filepath.Clean(path), why)) + } } - return nil + return problems } // accessRef is how a module names one of its accesses: ${access:}. @@ -359,10 +383,6 @@ func accessInto(resource map[string]any, accesses map[string]string, module stri if resource["path"], err = fill(path); err != nil { return err } - // Judged again once the access is in it: dirInto judged it while it still named one (novox/hq issue 496). - if err := judgeResolved(resource, module); err != nil { - return err - } } if content, ok := resource["content"].(string); ok { if resource["content"], err = fill(content); err != nil { diff --git a/internal/catalogue/resolved_path_test.go b/internal/catalogue/resolved_path_test.go index 4872bd45..55b5519e 100644 --- a/internal/catalogue/resolved_path_test.go +++ b/internal/catalogue/resolved_path_test.go @@ -1,39 +1,22 @@ package catalogue -// Every path the controller resolves for a module's directories and files is judged where the plan is -// composed (novox/hq issue 496). mesh-catalog #205 gave docker's `state` directory `"place": "."`, which -// resolves to /docker — /var/lib/docker, every container's filesystem. The merge gate composed every -// machine and passed it; only the node-engine refused it, at apply, and failed the walk. The gate composes -// through Declaration, so a refusal here is a refusal at the gate, for every module. +// A definition's directories and files are judged at their resolved paths when the definition is (novox/hq issue +// 496). mesh-catalog #205 gave docker's `state` directory `"place": "."`, which resolves to /docker — +// /var/lib/docker, every container's filesystem. The merge gate composed every machine and passed it; only the +// node-engine refused it, at apply, and failed the walk. ParseManifest is what `module check`, registration and the +// merge gate's reading of a changed repository all run, so a refusal here is a refusal at each of them. import ( "strings" "testing" ) -func composedOn(m Manifest, with Rendering) ([]map[string]any, error) { - return Resolution{Node: "anchor", Modules: []Manifest{m}}.Declaration(with) -} - -func pathOf(t *testing.T, resources []map[string]any, id string) string { - t.Helper() - for _, r := range resources { - if r["id"] == id { - return r["path"].(string) - } - } - t.Fatalf("no resource %s in %v", id, resources) - return "" -} - -func TestADirectoryPlacedInDockersDataIsRefusedAtComposition(t *testing.T) { +func TestADirectoryPlacedInDockersDataIsRefusedWhereTheDefinitionIsJudged(t *testing.T) { // The #205 shape, as it was merged. - m := Manifest{Module: "docker", Version: "1", Resources: []map[string]any{ - {"id": "state", "type": "directory", "place": "."}, - }} - _, err := composedOn(m, Rendering{}) + _, err := ParseManifest([]byte(`{"module": "docker", "version": "1", + "resources": [{"id": "state", "type": "directory", "place": "."}]}`)) if err == nil { - t.Fatal("a directory resolving to /var/lib/docker composed; the node-engine refuses it at apply") + t.Fatal("a directory resolving to /var/lib/docker was accepted; the node-engine refuses it at apply") } for _, said := range []string{"docker", `"state"`, "/var/lib/docker", "issue 496"} { if !strings.Contains(err.Error(), said) { @@ -42,68 +25,103 @@ func TestADirectoryPlacedInDockersDataIsRefusedAtComposition(t *testing.T) { } } -func TestTheMeshsPlaceForDockerComposes(t *testing.T) { +func TestTheMeshsPlaceForDockerIsAccepted(t *testing.T) { // The fix #205 needed: the mesh's own directory for the module, /mesh/docker. - m := Manifest{Module: "docker", Version: "1", Resources: []map[string]any{ + m, err := ParseManifest([]byte(`{"module": "docker", "version": "1", "resources": [ {"id": "state", "type": "directory", "place": "mesh"}, - {"id": "marker", "type": "file", "path": "${dir:state}/applied", "content": "x"}, - }} - resources, err := composedOn(m, Rendering{}) + {"id": "marker", "type": "file", "path": "${dir:state}/applied", "content": "x"}]}`)) if err != nil { - t.Fatalf("place %q is the mesh's tree, which the mesh writes for every module: %v", "mesh", err) + t.Fatalf("place %q is in the mesh's tree, where the mesh writes for every module: %v", "mesh", err) } - if got := pathOf(t, resources, "docker.state"); got != "/var/lib/mesh/docker" { - t.Fatalf("got %q", got) + if got := dirsFor(m, Rendering{}); got["state"] != "/var/lib/mesh/docker" { + t.Fatalf("got %v", got) } } -func TestEveryResolvedPathIsJudged(t *testing.T) { - refused := map[string]Manifest{ - "a default directory under a data root inside containerd's data": {Module: "images", Version: "1", - Resources: []map[string]any{{"id": "cache", "type": "directory"}}}, - "a stated directory in docker's data": {Module: "sidecar", Version: "1", - Resources: []map[string]any{{"id": "volumes", "type": "directory", "path": "/var/lib/docker/volumes/x"}}}, - "a file beneath a placed directory that climbs out of it": {Module: "docker", Version: "1", - Resources: []map[string]any{{"id": "state", "type": "directory", "place": "mesh"}, - {"id": "f", "type": "file", "path": "${dir:state}/../../containers/x", "content": "x"}}}, - "a file in /boot": {Module: "grub", Version: "1", - Resources: []map[string]any{{"id": "cfg", "type": "file", "path": "/boot/grub/custom.cfg", "content": "x"}}}, - "a directory that is the data root itself": {Module: "lib", Version: "1", - Resources: []map[string]any{{"id": "all", "type": "directory", "place": "."}}}, - "a directory in the node-engine's own tree, by another module": {Module: "intruder", Version: "1", - Resources: []map[string]any{{"id": "d", "type": "directory", "path": "/var/lib/mesh-host/x"}}}, +// The node-engine's rules, for directories and — as the engine judges them since novox/hq issue 495 — files. +func TestADefinitionIsRefusedWhereTheNodeEngineRefusesItsPaths(t *testing.T) { + refused := map[string]string{ + "a stated directory in docker's data": `{"module": "sidecar", "version": "1", "resources": [ + {"id": "volumes", "type": "directory", "path": "/var/lib/docker/volumes/x"}]}`, + "a file in containerd's data": `{"module": "images", "version": "1", "resources": [ + {"id": "f", "type": "file", "path": "/var/lib/containerd/x", "content": "x"}]}`, + "a file beneath a placed directory that climbs out of it": `{"module": "docker", "version": "1", "resources": [ + {"id": "state", "type": "directory", "place": "mesh"}, + {"id": "f", "type": "file", "path": "${dir:state}/../../containers/x", "content": "x"}]}`, + "a file in /boot": `{"module": "grub", "version": "1", "resources": [ + {"id": "cfg", "type": "file", "path": "/boot/grub/custom.cfg", "content": "x"}]}`, + "a directory that is the mesh's whole tree": `{"module": "mesh", "version": "1", "resources": [ + {"id": "all", "type": "directory", "place": "."}]}`, + "a directory that holds /etc": `{"module": "x", "version": "1", "resources": [ + {"id": "d", "type": "directory", "path": "/"}]}`, + "a directory in the node-engine's own tree, by another module": `{"module": "intruder", "version": "1", + "resources": [{"id": "d", "type": "directory", "path": "/var/lib/mesh-host/x"}]}`, } - roots := map[string]Rendering{ - "a default directory under a data root inside containerd's data": {DataRoot: "/var/lib/containerd"}, - "a directory that is the data root itself": {DataRoot: "/var"}, - } - for name, m := range refused { - if _, err := composedOn(m, roots[name]); err == nil || !strings.Contains(err.Error(), "issue 496") { - t.Errorf("%s: composed or refused for another reason: %v", name, err) + for name, raw := range refused { + if _, err := ParseManifest([]byte(raw)); err == nil || !strings.Contains(err.Error(), "issue 496") { + t.Errorf("%s: accepted, or refused for another reason: %v", name, err) } } - // What the mesh's own modules place today, on every machine (read from each machine's live plan): the - // machine's configuration, the mesh's tree, the node-engine's own, run directories and an account's keys. - passes := map[string]Manifest{ - "a unit in /etc": {Module: "power", Version: "1", - Resources: []map[string]any{{"id": "d", "type": "directory", "path": "/etc/systemd/system/x.service.d"}, - {"id": "u", "type": "file", "path": "/etc/systemd/system/x.service.d/a.conf", "content": "x"}}}, - "a run directory": {Module: "fail2ban", Version: "1", - Resources: []map[string]any{{"id": "run-dir", "type": "directory", "path": "/var/run/fail2ban"}}}, - "a program in /usr/local": {Module: "claude-code", Version: "1", - Resources: []map[string]any{{"id": "start", "type": "file", "path": "/usr/local/bin/claude-agent", "content": "x"}}}, - "the node-engine's launcher, by the node-engine": {Module: "mesh-host", Version: "1", - Resources: []map[string]any{{"id": "launcher", "type": "file", "path": "/usr/lib/nox-mesh-host/launch", "content": "x"}}}, - "an account's .ssh": {Module: "ssh-client", Version: "1", - Resources: []map[string]any{{"id": "ssh-dir", "type": "directory", "path": "/home/someone/.ssh"}, - {"id": "config", "type": "file", "path": "/home/someone/.ssh/config", "content": "x"}}}, - "a module's own root": {Module: "mailu", Version: "1", - Resources: []map[string]any{{"id": "state", "type": "directory", "place": "."}}}, + // What the engine applies, the mesh's own modules' paths among them (read from every machine's live plan): + // the machine's configuration, run directories, programs below /usr/local, the node-engine's own trees by its + // own module, an account's keys, a module's own root, and — not on the engine's lists, so not refused here — + // below /lib and at /storage, /data, /services and /var/lock. + accepted := map[string]string{ + "a unit in /etc": `{"module": "power", "version": "1", "resources": [ + {"id": "d", "type": "directory", "path": "/etc/systemd/system/x.service.d"}, + {"id": "u", "type": "file", "path": "/etc/systemd/system/x.service.d/a.conf", "content": "x"}]}`, + "a run directory": `{"module": "fail2ban", "version": "1", "resources": [ + {"id": "run-dir", "type": "directory", "path": "/var/run/fail2ban"}]}`, + "a program in /usr/local/bin": `{"module": "claude-code", "version": "1", "resources": [ + {"id": "start", "type": "file", "path": "/usr/local/bin/claude-agent", "content": "x"}]}`, + "the node-engine's launcher and state, by the node-engine": `{"module": "mesh-host", "version": "1", "resources": [ + {"id": "launcher", "type": "file", "path": "/usr/lib/nox-mesh-host/launch", "content": "x"}, + {"id": "state", "type": "directory", "path": "/var/lib/mesh-host"}]}`, + "an account's .ssh": `{"module": "ssh-client", "version": "1", "resources": [ + {"id": "ssh-dir", "type": "directory", "path": "/home/someone/.ssh"}, + {"id": "config", "type": "file", "path": "/home/someone/.ssh/config", "content": "x"}]}`, + "a module's own root": `{"module": "mailu", "version": "1", "resources": [ + {"id": "state", "type": "directory", "place": "."}]}`, + "a file below /lib": `{"module": "udev", "version": "1", "resources": [ + {"id": "rule", "type": "file", "path": "/lib/udev/rules.d/99-x.rules", "content": "x"}]}`, + "directories at /storage, /data, /services and /var/lock": `{"module": "roots", "version": "1", "resources": [ + {"id": "a", "type": "directory", "path": "/storage"}, {"id": "b", "type": "directory", "path": "/data"}, + {"id": "c", "type": "directory", "path": "/services"}, {"id": "d", "type": "directory", "path": "/var/lock"}]}`, } - for name, m := range passes { - if _, err := composedOn(m, Rendering{}); err != nil { + for name, raw := range accepted { + if _, err := ParseManifest([]byte(raw)); err != nil { t.Errorf("%s: refused: %v", name, err) } } } + +func TestAPathThroughAnAccessIsJudgedWithTheAccessFilledIn(t *testing.T) { + // A file's path may name an access; the access's default path is the definition's, so it is judged with it. + _, err := ParseManifest([]byte(`{"module": "backup", "version": "1", + "accesses": [{"id": "images", "path": "/var/lib/docker/volumes"}], + "resources": [{"id": "marker", "type": "file", "path": "${access:images}/marker", "content": "x"}]}`)) + if err == nil || !strings.Contains(err.Error(), "/var/lib/docker/volumes/marker") || + !strings.Contains(err.Error(), "issue 496") { + t.Fatalf("a file reaching Docker's data through an access's default path was accepted: %v", err) + } + // An access the definition gives no path is placed by a setting, which Places and AccessPlaces judge, and on + // the machine by the node-engine: nothing here to resolve it against, so nothing is refused for it. + if _, err := ParseManifest([]byte(`{"module": "backup", "version": "1", + "accesses": [{"id": "images"}], + "resources": [{"id": "marker", "type": "file", "path": "${access:images}/marker", "content": "x"}]}`)); err != nil { + t.Fatalf("an access placed only by a setting cannot be judged at the definition: %v", err) + } +} + +func TestComposingAMachineIsNeverStoppedByOneModulesPath(t *testing.T) { + // A module registered from outside the catalogue never passes the gate. Refusing its path while a machine's + // declaration is composed would fail the whole declaration and freeze every module on that machine; the + // node-engine fails only the one resource. So composition leaves it to the engine. + m := Manifest{Module: "docker", Version: "1", Resources: []map[string]any{ + {"id": "state", "type": "directory", "place": "."}, + }} + if _, err := (Resolution{Node: "anchor", Modules: []Manifest{m}}).Declaration(Rendering{}); err != nil { + t.Fatalf("the machine's declaration failed for one module's path: %v", err) + } +} diff --git a/testdata/beside/CAPTURED b/testdata/beside/CAPTURED index 17d63986..78e7f73d 100644 --- a/testdata/beside/CAPTURED +++ b/testdata/beside/CAPTURED @@ -25,3 +25,11 @@ and write the commits here. The SDK is captured at the commit go.mod pins for gi rm -rf testdata/beside/mesh-sdk && mkdir -p testdata/beside/mesh-sdk git -C ../mesh-sdk archive conformance/events | tar -x -C testdata/beside/mesh-sdk + +And from mesh-host at the same commit as its line above, the node-engine's placement guard, which +internal/catalogue's engine_guard_test.go holds the controller's resolved-path rules to (novox/hq issue 496), +kept as .captured so no Go tool reads it as this repository's code: + + mkdir -p testdata/beside/mesh-host/internal/apply + git -C ../mesh-host show :internal/apply/placement_guard.go \ + > testdata/beside/mesh-host/internal/apply/placement_guard.go.captured diff --git a/testdata/beside/mesh-host/internal/apply/placement_guard.go.captured b/testdata/beside/mesh-host/internal/apply/placement_guard.go.captured new file mode 100644 index 00000000..49c1d83e --- /dev/null +++ b/testdata/beside/mesh-host/internal/apply/placement_guard.go.captured @@ -0,0 +1,549 @@ +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, /root, /var/spool, /opt or the container runtimes' data +// (/var/lib/docker, /var/lib/containers, /var/lib/containerd, and where the runtimes' configuration moves +// them: runtimeDataRoots), nor in the node-engine's +// own trees** (its state, its identity, its installed builds) but by its own module; nothing is mounted from +// those but /proc, /sys and /dev. A mount of a kernel file, a device or the clock is the plumbing +// systemPath names. +// 2a. **An account's .ssh is never an access or a mount**, and is a directory only below its account's home, +// as that account's (rule 4). +// 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" + "context" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "regexp" + "strconv" + "strings" + "sync" + "time" + + "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", "/var/spool", "/opt", "/var/lib/docker", + "/var/lib/containers", "/var/lib/containerd"} + // 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", "/var/spool", "/opt", "/var/lib/docker", "/var/lib/containers", + "/var/lib/containerd"} + 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"} +) + +// runtimeFiles are where the container runtimes say where they keep their data: dockerd's daemon.json, its +// unit and the unit's drop-ins (an ExecStart with --data-root, or the older -g/--graph, continued over lines, +// quoted, through Environment= or EnvironmentFile=, or a --config-file naming another daemon.json), and podman's +// storage.conf (graphroot, a basic or a literal string). The running runtimes are asked first. containerd keeps its own under /var/lib/containerd, which forbiddenBelow names; a +// containerd configured elsewhere, and podman's rootless stores under each account's home, are not read: the +// first is no runtime this mesh runs, and the second is below a home, which rule 4 already keeps for its +// account. A variable so a test names its own. +type runtimeFiles struct { + daemonJSON string + units []string + dropInDirs []string + storageConf string +} + +var runtimeConfigs = runtimeFiles{ + daemonJSON: "/etc/docker/daemon.json", + units: []string{"/etc/systemd/system/docker.service", "/usr/lib/systemd/system/docker.service", "/lib/systemd/system/docker.service"}, + dropInDirs: []string{"/etc/systemd/system/docker.service.d", "/run/systemd/system/docker.service.d", "/usr/lib/systemd/system/docker.service.d"}, + storageConf: "/etc/containers/storage.conf", +} + +var ( + dataRootFlag = regexp.MustCompile(`(?:--data-root|--graph|-g)(?:=|\s+)(\S+)`) + configFlag = regexp.MustCompile(`--config-file(?:=|\s+)(\S+)`) + graphRoot = regexp.MustCompile(`(?m)^\s*graphroot\s*=\s*(?:"([^"]+)"|'([^']+)')`) + envVar = regexp.MustCompile(`\$\{([A-Za-z_][A-Za-z0-9_]*)\}|\$([A-Za-z_][A-Za-z0-9_]*)`) +) + +// runtimeRoots is where the container runtimes keep their data, as the last apply read it (readRuntimeRoots); +// nil until an apply has read it, when the guard reads the files itself. +var ( + runtimeRootsMu sync.Mutex + runtimeRoots []string +) + +// AskRuntimes is how the engine asks the running container runtimes where they keep their data: set by the engine +// to run the commands, nil in a test, which then reads the files alone. Its own, never the apply's runner, whose +// commands a test reads back as what the apply did. +var AskRuntimes Runner + +// refreshRuntimeRoots reads where the runtimes keep their data once, at the start of an apply. +func refreshRuntimeRoots(ctx context.Context) { + roots := readRuntimeRoots(ctx, AskRuntimes) + runtimeRootsMu.Lock() + runtimeRoots = roots + runtimeRootsMu.Unlock() +} + +// runtimeDataRoots is every place the container runtimes keep their data, beyond the default trees forbiddenBelow +// names: every container's filesystem is there. +func runtimeDataRoots() []string { + runtimeRootsMu.Lock() + roots := runtimeRoots + runtimeRootsMu.Unlock() + if roots != nil { + return roots + } + return readRuntimeRoots(context.Background(), nil) +} + +// readRuntimeRoots asks the running runtimes where they keep their data, when run is given (`docker info`, +// `podman info`), and reads their configuration besides: an answer from a runtime that is running is what it +// really does, and the files say what it will do when it starts again. Both count. A runtime that is not running +// or not installed answers nothing, which is no error. +func readRuntimeRoots(ctx context.Context, run Runner) []string { + seen := map[string]bool{} + var out []string + add := func(p string) { + p = strings.Trim(strings.TrimSpace(p), `"'`) + if !filepath.IsAbs(p) { + return + } + p = filepath.Clean(p) + if !seen[p] { + seen[p] = true + out = append(out, p) + } + } + if run != nil { + // Each runtime has its own ten seconds: one that hangs costs the other nothing. + for _, q := range [][]string{{"docker", "info", "--format", "{{.DockerRootDir}}"}, + {"podman", "info", "--format", "{{.Store.GraphRoot}}"}} { + ask, cancel := context.WithTimeout(ctx, 10*time.Second) + if root, err := run(ask, q[0], q[1:]...); err == nil { + add(root) + } + cancel() + } + } + daemonJSON := func(path string) { + raw, err := os.ReadFile(path) + if err != nil { + return + } + var c struct { + DataRoot string `json:"data-root"` + Graph string `json:"graph"` + } + if json.Unmarshal(raw, &c) == nil { + add(c.DataRoot) + add(c.Graph) + } + } + daemonJSON(runtimeConfigs.daemonJSON) + units := append([]string(nil), runtimeConfigs.units...) + for _, dir := range runtimeConfigs.dropInDirs { + matches, _ := filepath.Glob(filepath.Join(dir, "*.conf")) + units = append(units, matches...) + } + // The unit and its drop-ins are one unit to systemd: a variable set in one is seen by an ExecStart in another. + env := map[string]string{} + var execs []string + for _, u := range units { + raw, err := os.ReadFile(u) + if err != nil { + continue + } + e, x := unitLines(string(raw)) + for k, v := range e { + env[k] = v + } + execs = append(execs, x...) + } + for _, line := range execs { + line = envVar.ReplaceAllStringFunc(line, func(ref string) string { + m := envVar.FindStringSubmatch(ref) + if v, ok := env[m[1]+m[2]]; ok { + return v + } + return ref + }) + for _, m := range dataRootFlag.FindAllStringSubmatch(line, -1) { + add(m[1]) + } + for _, m := range configFlag.FindAllStringSubmatch(line, -1) { + daemonJSON(strings.Trim(m[1], `"'`)) + } + } + if raw, err := os.ReadFile(runtimeConfigs.storageConf); err == nil { + for _, m := range graphRoot.FindAllStringSubmatch(string(raw), -1) { + add(m[1] + m[2]) + } + } + return out +} + +// unitLines reads a unit file as systemd does for what matters here: a line ending in a backslash continues on the +// next, Environment= sets variables (quoted or not, several to a line), EnvironmentFile= (a leading - says it may +// be missing) reads KEY=value lines, and every ExecStart line is returned whole. +func unitLines(text string) (map[string]string, []string) { + var joined []string + var cur strings.Builder + for _, line := range strings.Split(text, "\n") { + trimmed := strings.TrimRight(line, " \t") + if strings.HasSuffix(trimmed, "\\") { + cur.WriteString(strings.TrimSuffix(trimmed, "\\") + " ") + continue + } + cur.WriteString(line) + joined = append(joined, cur.String()) + cur.Reset() + } + if cur.Len() > 0 { + joined = append(joined, cur.String()) + } + env := map[string]string{} + setPairs := func(s string) { + for _, f := range splitQuoted(s) { + if k, v, ok := strings.Cut(f, "="); ok { + env[strings.TrimSpace(k)] = strings.Trim(strings.TrimSpace(v), `"'`) + } + } + } + var execs []string + for _, line := range joined { + l := strings.TrimSpace(line) + switch { + case strings.HasPrefix(l, "Environment="): + setPairs(strings.TrimPrefix(l, "Environment=")) + case strings.HasPrefix(l, "EnvironmentFile="): + path := strings.TrimPrefix(strings.TrimSpace(strings.TrimPrefix(l, "EnvironmentFile=")), "-") + if raw, err := os.ReadFile(path); err == nil { + for _, kv := range strings.Split(string(raw), "\n") { + kv = strings.TrimSpace(kv) + if kv == "" || strings.HasPrefix(kv, "#") { + continue + } + setPairs(kv) + } + } + case strings.HasPrefix(l, "ExecStart"): + execs = append(execs, l) + } + } + return env, execs +} + +// splitQuoted splits on blanks outside double or single quotes, keeping the quotes' contents whole. +func splitQuoted(s string) []string { + var out []string + var cur strings.Builder + var quote rune + for _, r := range s { + switch { + case quote != 0 && r == quote: + quote = 0 + case quote == 0 && (r == '"' || r == '\''): + quote = r + case quote == 0 && (r == ' ' || r == '\t'): + if cur.Len() > 0 { + out = append(out, cur.String()) + cur.Reset() + } + default: + cur.WriteRune(r) + } + } + if cur.Len() > 0 { + out = append(out, cur.String()) + } + return out +} + +// 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 := append([]string(nil), 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 = append([]string(nil), forbiddenBelowMount...) + } + // The container runtimes' data, wherever the machine keeps it: every container's filesystem is there. + trees = append(trees, runtimeDataRoots()...) + 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)} + } + } + ssh := false + for _, part := range strings.Split(path, "/") { + ssh = ssh || part == ".ssh" + } + if ssh && kind != placingDirectory { + return &PlacementRefusedError{Path: path, Why: "it is an account's .ssh, which holds its keys and who may " + + "log in as it, and is never an access or a mount"} + } + 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 ssh && deepest == "" { + return &PlacementRefusedError{Path: path, Why: "a .ssh directory is placed only below its account's home, " + + "as that account's"} + } + 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 +}