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.
This commit is contained in:
jochen
@@ -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: <the control node>, values: {"places": {"state": {"path": "/etc",
|
||||
"owner": "<uid>:<gid>"}}}}` 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).
|
||||
@@ -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: <a node>, values: {"places": {"state": {"path": "/etc", "owner": "<uid>:<gid>"}}}}`
|
||||
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: <its node>, values: {"accesses": {"media": "/"}, …}}` is
|
||||
refused;
|
||||
- `mesh-controller.settings {module: <a module with a placement>, node: <its node>, clear: "true"}` is
|
||||
refused;
|
||||
- `mesh-controller.command {command: "settings set claude-code '{\"places\":{\"state\":\"/etc\"}}' --node <a node>"}`
|
||||
is refused before it runs;
|
||||
- `mesh-controller.settings {module: <any>, node: <a 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.
|
||||
Reference in New Issue
Block a user