From 537ad544d3a96a9bfeda4a86cc043857a6e3f26d Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 17:47:44 +0200 Subject: [PATCH] =?UTF-8?q?A=20container=20may=20not=20mount=20a=20path=20?= =?UTF-8?q?the=20module=20never=20declared=20=E2=80=94=20and=20three=20thi?= =?UTF-8?q?ngs=20declare=20one?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Brought back from 53eb000, withdrawn because it refused the builder's mount of the container runtime's socket. A path is declared as the module's own (a directory or file resource, or where a secret, grant or contribution lands), as the operator's (an accesses entry, ADR 0051), or as the machine's (a facility a declared capability grants: container-runtime grants its socket). Every catalogue manifest passes, and a test says so (novox/hq 04-ISSUES/026, ADR 0091). --- internal/catalogue/manifest.go | 98 +++++++++++++++++++++++++++++++ internal/catalogue/mounts_test.go | 86 +++++++++++++++++++++++++++ 2 files changed, 184 insertions(+) create mode 100644 internal/catalogue/mounts_test.go diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index d2096cb..4c82a6f 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -980,6 +980,25 @@ func ParseManifest(raw []byte) (Manifest, error) { } } + // **A container may not mount a path the module never declared** (novox/hq 04-ISSUES/026, + // ADR 0091). A bind mount whose source does not exist is created by the container runtime, as + // root, with whatever mode it picks — so `owner` and `mode` never reach the directory holding + // the module's data, and the rule that keeps data when a module goes away (ADR 0030) does not + // cover it, because the mesh has never heard of it. + // + // Three things declare a path, and they are the three kinds of thing a path can be: + // - the module's own: a directory or file resource, or where a secret, a grant or a + // contribution lands — created and owned by the mesh for this module; + // - the operator's: an `accesses` entry (ADR 0051) — pre-existing, shared, granted for use; + // - the machine's: a facility a declared capability grants, such as the container runtime's + // socket. It exists, the machine owns it, and declaring it as the module's own directory + // would be a lie the host would act on — which is why the first version of this check was + // withdrawn: it refused the builder. + // + // Checked here rather than on the machine because the machine cannot tell the difference: by + // the time it sees the mount it is being asked to create the directory, which it can do. + problems = append(problems, m.undeclaredMounts()...) + for i, r := range m.Resources { id, _ := r["id"].(string) if id == "" { @@ -1075,3 +1094,82 @@ func (m Manifest) MachineSide(port int) (at int, mayAssign bool) { } return port, false } + +// facilitiesOf is what each capability lets a container mount: paths the machine owns and a module +// is granted the use of by declaring the capability, never by declaring them as its own. +var facilitiesOf = map[string][]string{ + "container-runtime": {"/var/run/docker.sock"}, +} + +// undeclaredMounts is every bind-mount source no declaration covers — see the check above. +func (m Manifest) undeclaredMounts() []string { + declared := map[string]bool{} + claim := func(p string) { + if strings.HasPrefix(p, "/") { + declared[strings.TrimRight(p, "/")] = true + } + } + for _, r := range m.Resources { + switch fmt.Sprint(r["type"]) { + case "directory", "file": + claim(fmt.Sprint(r["path"])) + } + } + for _, where := range m.OwnSecrets { + claim(where) + } + for _, where := range m.Secrets { + claim(where) + } + for _, where := range m.Receives { + claim(where) + } + for _, where := range m.Grants { + claim(where) + } + for _, a := range m.Accesses { + claim(a.Path) + } + for _, c := range m.Capabilities { + for _, p := range facilitiesOf[c] { + claim(p) + } + } + // Under a declared directory is declared: a module that says where its data lives has said so + // for what it puts inside. + covers := func(path string) bool { + for at := path; strings.HasPrefix(at, "/"); { + if declared[at] { + return true + } + cut := strings.LastIndex(at, "/") + if cut <= 0 { + return false + } + at = at[:cut] + } + return false + } + var problems []string + for _, r := range m.Resources { + if fmt.Sprint(r["type"]) != "container" { + continue + } + mounts, _ := r["volumes"].([]any) + for _, v := range mounts { + from, _, _ := strings.Cut(fmt.Sprint(v), ":") + if !strings.HasPrefix(from, "/") || covers(strings.TrimRight(from, "/")) { + continue + } + problems = append(problems, fmt.Sprintf( + "%s mounts %q into %v, and nothing in %s declares it. A bind mount the module did "+ + "not declare is created by the container runtime as root, so the module's own "+ + "owner and mode do not reach the directory that holds its data, and the rule "+ + "that keeps data when a module goes away (novox/hq ADR 0030) does not cover it. "+ + "Declare it: a directory resource if it is the module's, an `accesses` entry if "+ + "it is the operator's, or the capability that grants it if it is the machine's", + m.Module, from, r["id"], m.Module)) + } + } + return problems +} diff --git a/internal/catalogue/mounts_test.go b/internal/catalogue/mounts_test.go new file mode 100644 index 0000000..0e323c5 --- /dev/null +++ b/internal/catalogue/mounts_test.go @@ -0,0 +1,86 @@ +package catalogue + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// A bind mount the module never declared is refused where it is written (novox/hq 04-ISSUES/026, +// ADR 0091). The container runtime creates a missing bind source itself, as root, with a mode it +// picks, so the module's own owner and mode never reach the directory holding its data, and the +// rule that keeps data when a module goes away (ADR 0030) does not cover it. + +const aContainerMounting = `{"id":"server","type":"container","name":"store","image":"x@sha256:` + + `0000000000000000000000000000000000000000000000000000000000000000","volumes":["%s:/inside"]}` + +func manifestMounting(from string, beside string) []byte { + res := aContainerMounting + if beside != "" { + res = beside + "," + res + } + return []byte(`{"module":"store","resources":[` + strings.Replace(res, "%s", from, 1) + `]}`) +} + +func TestAMountNothingDeclaresIsRefused(t *testing.T) { + _, err := ParseManifest(manifestMounting("/services/store/data", "")) + if err == nil { + t.Fatal("a container mounting a path no resource declares was accepted; the runtime " + + "would create it as root and the module's owner and mode would never apply") + } + if !strings.Contains(err.Error(), "/services/store/data") { + t.Fatalf("refused without naming the path, which leaves the author guessing: %v", err) + } +} + +// Declaring it is enough — including declaring the directory above it. +func TestAMountUnderADeclaredDirectoryIsAccepted(t *testing.T) { + _, err := ParseManifest(manifestMounting("/services/store/data", + `{"id":"state","type":"directory","path":"/services/store","mode":"0700"}`)) + if err != nil { + t.Fatalf("a module that said where its data lives was refused anyway: %v", err) + } +} + +// The operator's path, granted for use (ADR 0051), is declared by `accesses`, not by a directory the +// module would then own. +func TestAMountOfAnAccessedPathIsAccepted(t *testing.T) { + _, err := ParseManifest([]byte(`{"module":"store","accesses":[{"path":"/services/media/movies","mode":"read"}],` + + `"resources":[` + strings.Replace(aContainerMounting, "%s", "/services/media/movies", 1) + `]}`)) + if err != nil { + t.Fatalf("a mount of a path the module declares it accesses was refused: %v", err) + } +} + +// The machine's facility — the container runtime's socket — is granted by the capability that names +// it, and declaring it as the module's own directory would be a lie the host would act on. This is +// the case the first version of this check refused, and was withdrawn for: the builder. +func TestTheRuntimeSocketIsGrantedByTheCapabilityAndNotOtherwise(t *testing.T) { + granted := `{"module":"builder","capabilities":["container-runtime"],"resources":[` + + strings.Replace(aContainerMounting, "%s", "/var/run/docker.sock", 1) + `]}` + if _, err := ParseManifest([]byte(granted)); err != nil { + t.Fatalf("a module with the container-runtime capability may mount its socket: %v", err) + } + if _, err := ParseManifest(manifestMounting("/var/run/docker.sock", "")); err == nil { + t.Fatal("a module mounted the container runtime's socket without declaring the capability, and was accepted") + } +} + +// **Every manifest in the catalogue beside this checkout passes**, so the rule is not one the +// catalogue is already breaking. Skipped, aloud, where the catalogue is not there. +func TestEveryCatalogueManifestDeclaresWhatItMounts(t *testing.T) { + files, _ := filepath.Glob("../../../mesh-catalog/modules/*/module.json") + if len(files) == 0 { + t.Skip("the catalogue is not beside this checkout") + } + for _, file := range files { + raw, err := os.ReadFile(file) + if err != nil { + t.Fatal(err) + } + if _, err := ParseManifest(raw); err != nil { + t.Errorf("%s: %v", filepath.Base(filepath.Dir(file)), err) + } + } +}