From 29b4c0270987c8ba718ffdfd9cd70def2573ee37 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 17:24:05 +0200 Subject: [PATCH] only-office: keep its working directories under a root-only box (issue 1843) The image opens everything below each mount to every account on every start, so with the module root and lib at 0711 any account on the machine could read the document cache. The mounts now sit under a root-owned 0700 box the container never sees; the root stays 0711 for the converter. The old lib keeps its id so mesh-host closes it to 0700 (a new id there would be used as found, issue 339). --- modules/only-office/README.md | 14 ++ modules/only-office/manifest_test.go | 201 ++++++++++++++++++++++++--- modules/only-office/module.json | 25 +++- 3 files changed, 221 insertions(+), 19 deletions(-) diff --git a/modules/only-office/README.md b/modules/only-office/README.md index 3129fc4b..8b1a5291 100644 --- a/modules/only-office/README.md +++ b/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. diff --git a/modules/only-office/manifest_test.go b/modules/only-office/manifest_test.go index 88cbddfe..2e44316d 100644 --- a/modules/only-office/manifest_test.go +++ b/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 -// /var/lib/onlyoffice, where the document service writes the converted files nginx serves) lets -// other users pass through it (mode 0711): the image makes it the document service's own, and nginx -// runs as another user, so a mode of 0700 makes nginx answer 403 "Permission denied" on Editor.bin. +// Given the catalogue, when only-office's manifest is read, then the directory mounted at +// /var/lib/onlyoffice (server-lib, where the document service writes the converted files nginx +// serves) lets other users pass through it (mode 0711): the image makes it the document service's +// 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}/, 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 /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}/", 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 { - Resources []struct { - ID string `json:"id"` - Type string `json:"type"` - Mode string `json:"mode"` - } `json:"resources"` + if _, err := catalogue.ParseManifest(raw); err != nil { + t.Fatalf("parsing module.json: %v", err) } + 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.Mode != "0711" { - t.Errorf("lib directory mode = %q, want 0711 so nginx can reach the document service's cache", r.Mode) - } - return + if r.ID == id && r.Type == "directory" { + return r } } - 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:] } diff --git a/modules/only-office/module.json b/modules/only-office/module.json index e31fefc0..b4c1f5a1 100644 --- a/modules/only-office/module.json +++ b/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",