From ac3a67ebaba59e93b982d3c7fd7d20589775f84c Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 00:00:04 +0200 Subject: [PATCH] Issue 339: any caller of the settings verb could have the node-engine hand a system directory to an account Located in the controller (places and accesses taken through every verb, no line-break check) and the node-engine (a found directory chowned as root, through a link too, and any path mounted). ADR 0266's boundary depends on the fix. --- .../00-report.md | 96 +++++++++++++ .../01-diagnosis.md | 127 ++++++++++++++++++ 2 files changed, 223 insertions(+) create mode 100644 04-ISSUES/339-a-verb-can-place-a-directory-at-the-machines-own/00-report.md create mode 100644 04-ISSUES/339-a-verb-can-place-a-directory-at-the-machines-own/01-diagnosis.md diff --git a/04-ISSUES/339-a-verb-can-place-a-directory-at-the-machines-own/00-report.md b/04-ISSUES/339-a-verb-can-place-a-directory-at-the-machines-own/00-report.md new file mode 100644 index 00000000..f3ddb4c5 --- /dev/null +++ b/04-ISSUES/339-a-verb-can-place-a-directory-at-the-machines-own/00-report.md @@ -0,0 +1,96 @@ +--- +status: located +opened: 2026-10-08 +located-in: [mesh-controller (internal/catalogue/placement.go, internal/catalogue/settings.go, cmd/mesh-controller/modules.go, cmd/mesh-controller/seatverbs.go), mesh-host (internal/apply/apply.go, the directory, access and container appliers)] +fixed-by: +amended-design: +--- + +# 339. Any caller of the settings verb can have the node-engine hand a system directory to an account + +**Severity: critical.** It is a path from the mesh MCP server to root on any node. + +## Symptom + +A review on 2026-10-08 read the code and found this. Nothing was run against a node. + +A module's `places` setting moves one of its declared directories to any absolute path, with any numeric +`uid:gid` as its owner ([issue 153](../153-an-adopted-machines-data-cannot-be-placed-where-it-is/00-report.md), +[ADR 0112](../../02-DECISIONS/0112-a-module-definition-names-no-node-mesh-or-path.md)). The controller's +`settings` verb writes that setting for any caller allowed to call it +([ADR 0154](../../02-DECISIONS/0154-the-meshs-own-verbs-are-the-controller-seats-tools.md)). The node-engine +then applies the directory as root. It creates it if missing, and if it exists it chmods it and chowns it +to the owner the setting named. It follows a link at that path as well. + +So one settings call and one push hand the machine to an account of the caller's choice. For example, +`settings {module: claude-code, node: , values: {"places": {"state": {"path": "/etc", +"owner": ":"}}}}` followed by a push gives that uid `/etc`. + +`accesses` is the same hole for mounts. It names the path on the machine behind one of a module's accesses, and the +module's container mounts it. An access at `/` mounts the machine's root into a container, and a container +that runs as root can then write anywhere on the machine. + +A third, smaller path goes with it. A setting's value is substituted into env and configuration files that +the node-engine writes as root, such as a mail server's env file or a logind drop-in. A value holding a line +break adds a line of the caller's own to such a file. + +## Why it is an issue + +These settings were made for the operator. They say where an adopted machine keeps a module's data. But +the mesh has one way to write any setting, the `settings` verb, and whoever may call a verb includes every +agent session on a machine that runs the mesh MCP server. The generic `command` verb reaches the same +`settings set` line. Nothing on either side of the bus asked who set a placement, or where it pointed: + +- the controller accepted any absolute path and any numeric owner; +- the node-engine chowned and chmodded whatever directory it was handed, whether the mesh had made it or + not, through a link too. + +**ADR 0266's boundary depends on this fix.** ADR 0266 (still on its own pull request, so not linked here) +gives agents on the control node an account of their own that cannot become root without a person. Through +this hole that account becomes root anyway: one call places a module's directory at `/etc`, owned by the +agent account. Until this is fixed, the node-engine's judgement that "no account an agent runs as can become +root" measures the account and misses the path. + +## Impact + +- **Any caller of `mesh-controller.settings`, or of `mesh-controller.command`, could gain root on any node** + at that node's next push. That includes every agent session. +- **Whether anyone did is not known.** Step 3 of the diagnosis checks every module's layer on every node. + All 15 layers that set `places` or `accesses` hold legitimate data placements, and none points at a + system tree. +- Every module whose files take a setting could have a line injected into a file written as root. + +## The acceptance test (the operator's view) + +No caller of the mesh MCP server can make a node hand any system directory, or any directory the mesh did +not create, to some account, or mount a host path into a container, by changing a module's settings and +pushing. + +## How it is checked + +- **In the controller's tests, through the real verb dispatch.** The test composes each call with the verb + table (`argvFor`), runs it through this binary's own command dispatch, and sets the environment + `runVerb` gives every command a verb runs. Setting, moving or dropping `places` or `accesses` is + refused through the `settings` verb, for one machine and for the whole mesh, and so is clearing a layer + that holds one. A `settings set` or `settings clear` line through `command` is refused. The layer is + unchanged after every refusal. At the terminal the same placement is taken, but the machine's own trees + (`/`, `/etc`, `/usr`, `/var/lib`, `/home`, the node-engine's state and the rest) are refused from + anywhere. A line break, carriage return or NUL in any setting is refused, keys included. PEM blocks + alone may hold lines, because a provider serves its authority's root as a setting. Each of these tests + fails on the commit before the fix. +- **In the node-engine's tests, through `Apply`.** These are refused, each as a failed resource with its + reason: + - a directory, access or mount source that is one of the machine's roots or holds one; + - anything placed in the kernel's, the boot loader's, root's or the node-engine's own trees; + - a non-root directory below `/etc`, `/usr` or `/run`, and an access there; + - root's or another account's directory below a home; + - a container mounting anything the engine refused; + - a directory whose last component is a link. + + Each case is also judged with every link in the path resolved. A directory the mesh did not make keeps + its owner and mode, and is never removed. The guard was also run over the declarations all four nodes + would be sent today, and it refused nothing. Each behavioural test fails on the commit before the fix. +- **Live, after both fixes are taken:** the calls in the diagnosis's last section are each refused, and + `mesh-controller.plan` for every node still composes. + +The diagnosis is in [01-diagnosis.md](01-diagnosis.md). diff --git a/04-ISSUES/339-a-verb-can-place-a-directory-at-the-machines-own/01-diagnosis.md b/04-ISSUES/339-a-verb-can-place-a-directory-at-the-machines-own/01-diagnosis.md new file mode 100644 index 00000000..66c3ae05 --- /dev/null +++ b/04-ISSUES/339-a-verb-can-place-a-directory-at-the-machines-own/01-diagnosis.md @@ -0,0 +1,127 @@ +# 339 — Diagnosis + +## 2026-10-08 + +**The records first.** `records_search` for the `places` setting found +[issue 153](../153-an-adopted-machines-data-cannot-be-placed-where-it-is/00-report.md), which made the +setting, and [ADR 0112](../../02-DECISIONS/0112-a-module-definition-names-no-node-mesh-or-path.md), which +puts a machine's paths on the assignment. Neither says who may set a placement or where it may point. +[ADR 0182](../../02-DECISIONS/0182-inside-a-home-the-mesh-owns-what-it-places-and-holds-the-rest-as-found.md) +holds what the mesh finds inside a home "as found". Nothing applied that rule to a directory found anywhere +else. + +### The controller + +- `Places` (`internal/catalogue/placement.go`) read a placement as any absolute path, with an owner of any + numeric `uid:gid`. `AccessPlaces` read an access the same way, with any absolute path. +- `JudgeSettings` and `UnusedSettings` (`internal/catalogue/settings.go`) took either key for any module that + declares a directory or an access. They judged the shape and nothing else. +- `settingsCommand` (`cmd/mesh-controller/modules.go`) is the one place a layer is written (`SetSettings`, + `ClearSettings`). It cannot tell a line typed at the terminal from one a verb composed. +- Every verb runs its command line in a fresh process of the same binary (`runVerb`, + `cmd/mesh-controller/seatverbs.go`). That covers both the `settings` verb and the generic `command` verb, + which runs any line as given, `settings set` included. `runVerb` already names the verb in the process's + environment (`MESH_VERB`, issue 327), so the command can know it came through a verb. +- No setting refused a line break. The node-engine writes settings into files of other formats, so a + value with one adds a line to the file. + +### The node-engine + +- `applyDirectory` (`internal/apply/apply.go`) ran `os.Stat` (which follows a link), then `MkdirAll` when the + directory was missing, then `os.Chmod` and `os.Chown` on whatever was there. It did this whether or not + the mesh had made the directory, and wherever it was. +- `applyAccess` checked only that the path exists and is a directory. +- A container's mounts were passed to the runtime as declared, so a directory the engine might refuse was + still mounted. + +Nothing on the node knows whether a directory's path came from the module's definition or from a +`places` setting: the controller resolves both into one concrete path (ADR 0112). So the engine's guard +has to hold for every directory and every mount, and must not break the ones definitions legitimately +declare. Definitions declare directories below `/etc` (drop-in directories, all root's), below `/var/lib` +(owned by a container's uid), and below the operator's or the agent account's home (owned by that +account). They mount the runtime's socket, and, in the engine's own tests, kernel files and the clock. + +### The fix + +**In the controller** (pull request in `mesh-controller`): + +1. A layer change that adds, changes or removes `places` or `accesses` is refused in any process a verb + runs. Clearing a layer that holds either is refused the same way. A change to other keys that leaves + both as they were is still taken. The check is in `settingsCommand`, keyed on the environment `runVerb` + sets, so it holds for every route a verb has to that command. +2. The generic `command` verb refuses any `settings set` or `settings clear` line before it runs. A layer is + written through the `settings` verb alone. +3. `Places` and `AccessPlaces` refuse the machine's own trees from any route, the terminal included: + - at or below `/etc`, `/usr`, `/boot`, `/root`, `/run`, `/proc`, `/sys`, `/dev`, the binary trees, and the + mesh's and the node-engine's state; + - exactly at a parent of every module's or person's directories (`/`, `/var`, `/var/lib`, `/home`, `/srv`, + `/mnt` and the like). + + Paths are cleaned first, so `/srv/../etc` is `/etc`. +4. `JudgeSettings` refuses a line break, carriage return or NUL in any string of any setting, keys included, + at any depth. It runs where a layer is kept and again where it is composed. PEM blocks alone may hold + lines: every line between BEGIN and END is base64, 64 characters but the last, and the last is padded + only at its end. A provider serves its authority's root to its consumers this way. + +**In the node-engine** (pull request in `mesh-host`), the engine refuses these whatever the declaration says, +each as a failed resource with its reason in the node's report: + +1. A directory, an access or a mount source that is one of the machine's roots, or holds one. +2. A directory or an access in `/proc`, `/sys`, `/dev`, `/boot`, `/root`, or the node-engine's own trees + (except for the engine's own module). A mount from `/boot`, `/root` or the engine's trees. Mounts of + kernel files and devices stay allowed, because they are plumbing a container may need. +3. A directory below `/etc`, `/usr` or `/run` that is not root's. An access anywhere there. +4. Below a home, a directory that is not that account's. +5. A container that mounts anything the engine refused. + +Every path is also judged with its links resolved. A directory whose last component is a link is refused, +because a chmod or chown through it would change where it points. A directory that was already there, and +that the mesh did not record making at that path, is **used as found**: its owner and mode are not +changed, the outcome says so, and it is never removed. That is ADR 0182 applied beyond the home. The record +of every directory on a running machine already names its path, so nothing applied today changes. + +### The two pull requests are independent + +The controller's tests do not compose with the node-engine, and the engine's guard needs nothing from the +controller. Either can merge first. Each closes the hole on its own side. + +### Step 3: what the live layers hold + +On 2026-10-08 every module's layer was read through `mesh-controller.settings` (read only): the whole-mesh +layer of all 136 modules, and the layer of every machine each is assigned to, 381 in all. Only the +`places` and `accesses` keys were copied. Fifteen layers set one of them: + +- **The home server:** 13 media and automation modules. They place their configuration below the default + data root or below a mount of the data disks, owned by the container's uid. Their accesses name media and + download libraries on the data disks. +- **The home server's backup module:** places its repository on the data disks. +- **The anchor's backup module:** places its repository directly under the homes' root, at a directory + that is not `/home` itself. + +None is a system tree. Each passes the controller's new refusal and the engine's new guard. The guard was +also run over the declarations `mesh-controller.plan` composes for all four nodes today: 356 directories, +96 containers and every access. It refused none. The home rule there was read against the operator's +machine's user database. The other nodes' homes, read from their declarations, are each owned by the +account the directory names. + +So no layer was set to exploit this, as far as the layers show. The layers' history (`settings --history`) +was not read, so a placement that was set and then removed would not show here. + +### The acceptance test, live, after both are taken + +Each of these calls must be refused, and the layer read afterwards must be unchanged: + +- `mesh-controller.settings {module: claude-code, node: , values: {"places": {"state": {"path": "/etc", "owner": ":"}}}}` + is refused as a terminal-only setting; +- the same with a harmless path, `/srv/x`, is refused for the same reason; +- `mesh-controller.settings {module: n8n, node: , values: {"accesses": {"media": "/"}, …}}` is + refused; +- `mesh-controller.settings {module: , node: , clear: "true"}` is + refused; +- `mesh-controller.command {command: "settings set claude-code '{\"places\":{\"state\":\"/etc\"}}' --node "}` + is refused before it runs; +- `mesh-controller.settings {module: , node: , values: {"x": "a\nb"}}` is refused for its line + break. + +Then `mesh-controller.plan` for every node composes as before, and no condition is raised for a refused +directory on any node.