Merge pull request 'only-office: keep its working directories under a root-only box (issue 1843)' (#448) from fix/1843-private-box into main

modules/only-office/README.md

@@ -7,10 +7,24 @@ The `only-office` module of the mesh catalogue: runs the container image `onlyof
 Each directory or archive this module declares says how it treats a file it did not write (ADR 0422, ADR 0416).
 
 - `${dir:state}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2).
+- `${dir:box}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2).
 - `${dir:logs}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2).
 - `${dir:data}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2).
+- `${dir:server-lib}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2).
 - `${dir:lib}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2).
 - `${dir:db}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2).
 - `${dir:rabbitmq}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2).
 - `${dir:redis}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2).
 - `${dir:fonts}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2).
+
+## Where its directories live
+
+Everything the container mounts sits in `${dir:box}` (`/var/lib/only-office/box`), root-owned and 0700 (issue 1843).
+The image opens what is below each mount to every account on every start (`chmod -R 755`), so the barrier is a
+parent the container never sees. The module root stays root-owned 0711: the converter, which runs on the host
+as `only-office-converter`, passes through it to its two secrets and nothing else.
+
+`${dir:lib}` (`/var/lib/only-office/lib`) is the copy the container used before, no longer mounted, kept by the
+mesh because it is not empty and closed to 0700. It keeps its id so mesh-host sets that mode: a new id at a path
+the image owns would be used as found (issue 339). Its contents, and those of the old `logs`, `data`, `db`,
+`rabbitmq`, `redis` and `fonts` beside it, are removed on the operator's word, and then `lib` leaves the manifest.

modules/only-office/manifest_test.go

@@ -96,32 +96,201 @@ func TestSecureLinkSecretIsPinned(t *testing.T) {
 }
 
 // 1837/AC3 (unit: manifest check)
-// Given the catalogue, when only-office's manifest is read, then the lib directory (mounted at
+// Given the catalogue, when only-office's manifest is read, then the directory mounted at
-// /var/lib/onlyoffice, where the document service writes the converted files nginx serves) lets
+// /var/lib/onlyoffice (server-lib, where the document service writes the converted files nginx
-// other users pass through it (mode 0711): the image makes it the document service's own, and nginx
+// serves) lets other users pass through it (mode 0711): the image makes it the document service's
-// runs as another user, so a mode of 0700 makes nginx answer 403 "Permission denied" on Editor.bin.
+// own, and nginx runs as another user, so a mode of 0700 makes nginx answer 403 "Permission denied"
+// on Editor.bin. Host accounts are kept out by the box above it (1843/AC1).
 func TestLibDirectoryLetsNginxThrough(t *testing.T) {
+	m := readLayout(t)
+	src := m.volumeSource(t, "/var/lib/onlyoffice")
+	if src != "${dir:server-lib}" {
+		t.Fatalf("/var/lib/onlyoffice is mounted from %q, want ${dir:server-lib}", src)
+	}
+	d := m.dir(t, "server-lib")
+	if d.Mode != "0711" {
+		t.Errorf("server-lib directory mode = %q, want 0711 so nginx can reach the document service's cache", d.Mode)
+	}
+}
+
+// 1843/AC1 (unit: manifest check)
+// Given the catalogue, when only-office's manifest is read, then it declares a directory box with no
+// place, no path and no owner (root, /var/lib/only-office/box) and mode 0700, and every host source of
+// the server container's volumes is a directory whose path is ${dir:box}/<name>, so no account but
+// root can pass to anything the container writes: the image opens what is below each mount to every
+// account on every start.
+func TestContainerDirectoriesSitInARootOnlyBox(t *testing.T) {
+	m := readLayout(t)
+	box := m.dir(t, "box")
+	if box.Place != "" || box.Path != "" || box.Owner != "" {
+		t.Errorf("box = place %q, path %q, owner %q, want none of them (root, at <root>/only-office/box)", box.Place, box.Path, box.Owner)
+	}
+	if box.Mode != "0700" {
+		t.Errorf("box mode = %q, want 0700", box.Mode)
+	}
+	c := m.container(t, "server")
+	if len(c.Volumes) == 0 {
+		t.Fatal("the server container mounts nothing")
+	}
+	for _, v := range c.Volumes {
+		src, target := splitVolume(v)
+		id, ok := strings.CutPrefix(src, "${dir:")
+		if !ok || !strings.HasSuffix(id, "}") {
+			t.Errorf("volume %q has a host source that is not a directory of the module", v)
+			continue
+		}
+		d := m.dir(t, strings.TrimSuffix(id, "}"))
+		if !strings.HasPrefix(d.Path, "${dir:box}/") || strings.Count(d.Path, "/") != 1 {
+			t.Errorf("%s (mounted at %s) has path %q, want ${dir:box}/<name>", d.ID, target, d.Path)
+		}
+		if d.Owner != "" {
+			t.Errorf("%s is owned by %q, want root", d.ID, d.Owner)
+		}
+	}
+}
+
+// 1843/AC2 (unit: manifest check)
+// Given the catalogue, when only-office's manifest is read, then the module root (state) is still
+// root-owned, mode 0711, place ".", and the converter's secrets and environment still name
+// ${dir:state}/jwt.secret and ${dir:state}/broker-converter.secret: the converter passes the root and
+// nothing below it, and no account but root owns a directory on any path.
+func TestConverterPassesOnlyTheRoot(t *testing.T) {
+	m := readLayout(t)
+	state := m.dir(t, "state")
+	if state.Place != "." || state.Owner != "" || state.Mode != "0711" {
+		t.Errorf("state = place %q, owner %q, mode %q, want place ., root, 0711", state.Place, state.Owner, state.Mode)
+	}
+	for _, r := range m.Resources {
+		if r.Type == "directory" && r.Owner != "" {
+			t.Errorf("directory %s is owned by %q, want root", r.ID, r.Owner)
+		}
+	}
+	for name, want := range map[string]string{"jwt": "${dir:state}/jwt.secret", "broker-converter": "${dir:state}/broker-converter.secret"} {
+		if got := m.OwnSecrets[name].Path; got != want {
+			t.Errorf("own-secrets.%s.path = %q, want %q", name, got, want)
+		}
+	}
+	var conv *layoutResource
+	for i := range m.Resources {
+		if m.Resources[i].ID == "converter" {
+			conv = &m.Resources[i]
+		}
+	}
+	if conv == nil {
+		t.Fatal("manifest declares no converter")
+	}
+	if conv.Env["ONLY_OFFICE_JWT"] != "${dir:state}/jwt.secret" || conv.Env["ONLY_OFFICE_BROKER"] != "${dir:state}/broker-converter.secret" {
+		t.Errorf("converter env = %v, want its secrets in ${dir:state}", conv.Env)
+	}
+}
+
+// 1843/AC3 (unit: manifest check)
+// Given the catalogue, when only-office's manifest is read, then the directory lib is declared with no
+// path (its old place, /var/lib/only-office/lib) and mode 0700, so the next apply closes the copy the
+// image opened, which mesh-host keeps because it is not empty; no container volume names ${dir:lib};
+// and every data entry points at a directory that is mounted now.
+func TestOldLibIsClosedAndUnmounted(t *testing.T) {
+	m := readLayout(t)
+	lib := m.dir(t, "lib")
+	if lib.Path != "" || lib.Place != "" {
+		t.Errorf("lib = path %q, place %q, want neither: it stays where the old copy is", lib.Path, lib.Place)
+	}
+	if lib.Mode != "0700" {
+		t.Errorf("lib mode = %q, want 0700", lib.Mode)
+	}
+	mounted := map[string]bool{}
+	for _, v := range m.container(t, "server").Volumes {
+		src, _ := splitVolume(v)
+		mounted[src] = true
+	}
+	if mounted["${dir:lib}"] {
+		t.Error("the server container still mounts ${dir:lib}")
+	}
+	for _, d := range m.Data.Own {
+		if !mounted[d.Path] {
+			t.Errorf("data entry %s points at %q, which the server container does not mount", d.ID, d.Path)
+		}
+	}
+}
+
+type layoutResource struct {
+	ID      string            `json:"id"`
+	Type    string            `json:"type"`
+	Mode    string            `json:"mode"`
+	Place   string            `json:"place"`
+	Path    string            `json:"path"`
+	Owner   string            `json:"owner"`
+	Volumes []string          `json:"volumes"`
+	Env     map[string]string `json:"env"`
+}
+
+type layout struct {
+	Resources  []layoutResource `json:"resources"`
+	OwnSecrets map[string]struct {
+		Path string `json:"path"`
+	} `json:"own-secrets"`
+	Data struct {
+		Own []struct {
+			ID   string `json:"id"`
+			Path string `json:"path"`
+		} `json:"own"`
+	} `json:"data"`
+}
+
+func readLayout(t *testing.T) layout {
+	t.Helper()
 	raw, err := os.ReadFile("module.json")
 	if err != nil {
 		t.Fatalf("reading module.json: %v", err)
 	}
-	var m struct {
+	if _, err := catalogue.ParseManifest(raw); err != nil {
-		Resources []struct {
+		t.Fatalf("parsing module.json: %v", err)
-			ID   string `json:"id"`
-			Type string `json:"type"`
-			Mode string `json:"mode"`
-		} `json:"resources"`
 	}
+	var m layout
 	if err := json.Unmarshal(raw, &m); err != nil {
 		t.Fatalf("parsing module.json: %v", err)
 	}
+	return m
+}
+
+func (m layout) dir(t *testing.T, id string) layoutResource {
+	t.Helper()
 	for _, r := range m.Resources {
-		if r.ID == "lib" && r.Type == "directory" {
+		if r.ID == id && r.Type == "directory" {
-			if r.Mode != "0711" {
+			return r
-				t.Errorf("lib directory mode = %q, want 0711 so nginx can reach the document service's cache", r.Mode)
-			}
-			return
 		}
 	}
-	t.Fatal("manifest declares no lib directory")
+	t.Fatalf("manifest declares no directory %s", id)
+	return layoutResource{}
+}
+
+func (m layout) container(t *testing.T, id string) layoutResource {
+	t.Helper()
+	for _, r := range m.Resources {
+		if r.ID == id && r.Type == "container" {
+			return r
+		}
+	}
+	t.Fatalf("manifest declares no container %s", id)
+	return layoutResource{}
+}
+
+func (m layout) volumeSource(t *testing.T, target string) string {
+	t.Helper()
+	for _, v := range m.container(t, "server").Volumes {
+		if src, to := splitVolume(v); to == target {
+			return src
+		}
+	}
+	t.Fatalf("the server container mounts nothing at %s", target)
+	return ""
+}
+
+// splitVolume is a volume's host source and container target; the source's own ${dir:…} holds a colon.
+func splitVolume(v string) (src, target string) {
+	i := strings.LastIndex(v, ":")
+	if i < 0 {
+		return v, ""
+	}
+	return v[:i], v[i+1:]
 }

modules/only-office/module.json

@@ -104,7 +104,7 @@
       },
       {
         "id": "lib",
-        "path": "${dir:lib}",
+        "path": "${dir:server-lib}",
         "class": "rebuildable",
         "why": "the document server's working files"
       },
@@ -161,45 +161,64 @@
       "mode": "0600",
       "template": "files/server.env"
     },
+    {
+      "id": "box",
+      "type": "directory",
+      "mode": "0700",
+      "unknown": "program"
+    },
     {
       "id": "logs",
       "type": "directory",
+      "path": "${dir:box}/logs",
       "mode": "0700",
       "unknown": "program"
     },
     {
       "id": "data",
       "type": "directory",
+      "path": "${dir:box}/data",
       "mode": "0700",
       "unknown": "program"
     },
+    {
+      "id": "server-lib",
+      "type": "directory",
+      "path": "${dir:box}/lib",
+      "mode": "0711",
+      "unknown": "program"
+    },
     {
       "id": "lib",
       "type": "directory",
-      "mode": "0711",
+      "mode": "0700",
       "unknown": "program"
     },
     {
       "id": "db",
       "type": "directory",
+      "path": "${dir:box}/db",
       "mode": "0700",
       "unknown": "program"
     },
     {
       "id": "rabbitmq",
       "type": "directory",
+      "path": "${dir:box}/rabbitmq",
       "mode": "0700",
       "unknown": "program"
     },
     {
       "id": "redis",
       "type": "directory",
+      "path": "${dir:box}/redis",
       "mode": "0700",
       "unknown": "program"
     },
     {
       "id": "fonts",
       "type": "directory",
+      "path": "${dir:box}/fonts",
       "mode": "0700",
       "unknown": "program"
     },
@@ -223,7 +242,7 @@
       "volumes": [
         "${dir:logs}:/var/log/onlyoffice",
         "${dir:data}:/var/www/onlyoffice/Data",
-        "${dir:lib}:/var/lib/onlyoffice",
+        "${dir:server-lib}:/var/lib/onlyoffice",
         "${dir:db}:/var/lib/postgresql",
         "${dir:rabbitmq}:/var/lib/rabbitmq",
         "${dir:redis}:/var/lib/redis",