57/P2: The node-engine writes a filled file on the provisioner client's request, and nothing else (issue 668) #111
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: novox/mesh-host#111
Reference in New Issue
Block a user
What changed
/run/nox-mesh-host/fill.sock(internal/filling), beside mesh-cli's. It reuses mesh-cli's kernel checks (SO_PEERCRED, same mount and user namespaces, own socket directory), now exported frominternal/meshcli/peer.go. The request is one JSON line{path, content}(content only); the answer is{path, action, digest, refused, failed}.provisioner_account(the provisioner client's account, by name), accepted by the strict parser, refused when root or not an account name. The contract is raised to 10 (link.ProvisionerContract), following P1's contract 9 and the units_test.go pattern. While the declaration names no account, every write is refused. The account is judged before the request is read, and again under the apply's lock./etc//a,..,./, a trailing slash and relative paths are refused), and the current declaration must name it infilled_files. Where the path leads (homes, agent homes, refused places, links under a home) is judged by the apply's own guards, because the write goes throughapplyOne.apply.WriteFilledapplies the declared file with the client's content in place of the declaration's. An owned file goes through applyFile, a seed through applyFile's create-once, and a managed file through applyInto or applyBlock. The write keeps the Keep backup, the declared owner and mode (a file that carried a secret stays 0600), and the apply's record. Then, in the apply's order, it applies what reads the file: restart-on, reload-on, a process's restart-on, and a running container's env-file or a file mounted into it. Each of those is recorded as an apply records it. The apply loop's record and its "moved" rule became the helpersrecordAppliedandmoved, with no change of behaviour.applyingin the process andstore.Lockbeside the state), reads the kept, verified declaration under them, and saves the state as an apply does.Why
ADR 0349 §4, design 57 P2. The provisioner's client needs the node-engine to write what it fills, without holding the right to write the machine's files itself.
How it is proven (tests that fail before and pass now)
TestAnUndeclaredPathIsRefusedandTestAWriteOfAPathNotFilledIsRefused.TestAnotherAccountIsRefused,TestTheSocketRefusesAnotherAccountBeforeReadingItsRequest(real socket, real SO_PEERCRED),TestEveryWriteIsRefusedWhileTheDeclarationNamesNoAccountandTestAnAccountThatIsRootHereIsRefused.TestAnUncleanPathIsRefused.TestAWriteUnderReloadOnReloadsItsProgramOnce. AlsoTestAWriteUnderRestartOnRestartsItsProgramandTestAWriteOfAFileAContainerReadsRecreatesIt.TestAFilledFileIsWrittenAsItsOwnershipSays(6 cases). AlsoTestAFilledFileThatCarriedASecretIsNotWorldReadable.TestAWriteAndAnApplyOfTheSameFileSerialise(real locks: the write waits while an apply holdsapplying, and while another process holdsstore.Lock) andTestAWriteWaitsForTheApply.TestTheDeclarationNamesTheProvisionersAccount,TestAProvisionersAccountThatIsNoAccountIsRefused,validate_testand the contract check inunits_test.Commands run
sh merge-check.shexits 0. It runs gofmt -l (clean), go vet, andgo test -race -count=1 ./...(all pass). It prints "NOT RUN AGAINST A BUS": this machine holds no nats-server, so the bus tests ininternal/link(those reading MESH_TEST_NATS) did not run. None of them is touched here.Deliberately left
filled_filesorprovisioner_accountbefore P6, so nothing changes on a node until then.provisioner_accountonly to a host saying contract 10.internal/fillinghere; moving them to mesh-sdk is P5's call.Reviewer's checklist
provisioner_accountis read before any controller writes it, and the contract is raised to 10.Delivery
novox/mesh-host@2767278228bc— delivered since 2026-10-14T01:15:52ZDelivery plan — builds mesh-host → ace, g14, novox, shanks; no bus step
Transitions
The commit's note under
refs/notes/mesh-plankeeps every transition:git log --notes=mesh-plan.Merge check at
499a6871Judged: head
499a687149bemerged into trunk6b54fed706f0; read beside it: mesh-catalogebf7f3773dc7, mesh-controller2d18f438c516, mesh-controller-main2d18f438c516, mesh-tools954e1a53eef9mesh/merge-gate: PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: mesh-host)mesh/repo-check: PASS — its merge-check.sh passedDelivery plan — builds mesh-host → ace, g14, novox, shanks; no bus step
Run by the build seat on ace as
build-1791938297026340805(builds --log build-1791938297026340805).Verdict: CHANGES at
499a687149Ran in my own worktree of
499a687:gofmt -l .(clean),go vet ./...(clean),sh merge-check.sh(exit 0; it said NOT RUN AGAINST A BUS because this machine has no nats-server, so the bus tests in internal/link were skipped). What holds: SO_PEERCRED and the namespace check run before any byte of the request is read. The account is judged again under the lock. The lock order isapplyingand thenstore.Lock, the same as applyAndKeepHeld, so I found no deadlock. A client that is slow before its request is read holds no lock. The content is not logged. Contract 10 is raised the same way as P1 raised 9. A final-component link is replaced by the rename and never followed.Must change:
in.held(the apply sets it at apply.go:720 from the library verdicts). A container that reads the filled file and mounts a library whose disk or mount is missing is recreated anyway. That bypasses the ADR 0328 §9 guard, so the container's writes land on the root filesystem. Compute the held roots, or refuse to recreate such a container, and add a test.busy. Thebusyrefusals also drain for 1 s with no bound on how many run at once, so a connection flood costs one goroutine and one fd each. mesh-cli's socket judges the account before it takes a slot and bounds its waiting refusals (refusalsWaiting). Do the same here: judge the uid against a cheap check before taking a slot, and bound the drains.Should (can follow):
4. filling.go:220 — mesh-cli's Serve chmods the directory to 0755 after its ownership check (meshcli.go:219). This Serve does not, so it relies on mesh-cli's Serve having run.
5. filling.go:297 — the decoder does not call DisallowUnknownFields. Use it so the wire stays strict for P5.
6. No test shows that a silent client, while it holds the socket open, neither blocks an apply nor holds a slot past
within. No test covers the two-resources refusal either.7. Parent directories on the path are still checked by path name, not walked by descriptor (issue 556). This is inherited from applyFile, but the write can now happen whenever the client asks. Say so in issue 556.
Merge check at
76dc93c4Judged: head
76dc93c421bfmerged into trunk6b54fed706f0; read beside it: mesh-catalogebf7f3773dc7, mesh-controller2d18f438c516, mesh-controller-main2d18f438c516, mesh-tools954e1a53eef9mesh/merge-gate: PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: mesh-host)mesh/repo-check: PASS — its merge-check.sh passedDelivery plan — builds mesh-host → ace, g14, novox, shanks; no bus step
Run by the build seat on shanks as
build-1791938823488081642(builds --log build-1791938823488081642).Merge check at
0c17c764Judged: head
0c17c76493f7merged into trunk6b54fed706f0; read beside it: mesh-catalogebf7f3773dc7, mesh-controller2d18f438c516, mesh-controller-main2d18f438c516, mesh-tools954e1a53eef9mesh/merge-gate: PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: mesh-host)mesh/repo-check: PASS — its merge-check.sh passedDelivery plan — builds mesh-host → ace, g14, novox, shanks; no bus step
Run by the build seat on ace as
build-1791938891609958811(builds --log build-1791938891609958811).Verdict: CHANGES at
0c17c76493Ran in my own worktree of
0c17c76:gofmt -l .(clean),go vet ./...(clean),sh merge-check.shwith a nats-server on PATH (exit 0; all 28 packages pass, the bus tests in internal/link ran against a throwaway nats-server).The earlier review (comment 11959) is fixed. I reverted each fix alone and saw its test fail:
in.held = heldLibraries(...), TestAWriteDoesNotRecreateAContainerHeldForItsLibrary fails. heldLibraries holds only on libraryWaitDisk, the same as the full apply, and is computed under the lock.within): each test fails with its fix reverted.Security pass. The peer is judged before any byte is read, and a refused caller is answered and closed without being read. No lock is held while the engine waits on a client, because the request is decoded before Write takes the lock and the reply is written after the lock is released. Memory is bounded: an 8 MiB LimitReader per request, at most 4 at once, and only the client's account gets that far. Accept back-off starts at 50 ms, doubles up to 5 s and resets on success, so it cannot spin, and ErrClosed and ctx end it.
Must change:
Should (can follow):
2. filling.go:91 and :299 — Accepts loads the signed declaration again for every connection and verifies it with ed25519. Under a flood, 16 of those run at once, and the client waits in the same queue. Caching the provisioner's user id, keyed on the declaration file's identity, would make the check before the slot cheap.
3. filling.go:299 — each accepted connection waits for
judgingin its own goroutine with no bound except RLIMIT_NOFILE. Past that, accepts fail and back off for up to 5 s, which also delays the client. Consider a bounded count of connections waiting to be judged, past which a caller is closed at once.4. filling_test.go:220 —
asksets no deadline, so a regression such as reverting fix 3 hangs the package for the 10-minute test timeout instead of failing. Give it a deadline.5. filling.go:208 — busyWaiting is a package-level channel, while slots and judging belong to each Serve call. Make it one Serve's too, for consistency and for tests that run in parallel.
Verdict: APPROVE at
2767278228Ran in my own worktree of
2767278:gofmt -l .(clean),go vet ./...(clean),sh merge-check.shwith a nats-server on PATH (exit 0; all 28 packages pass, and the bus tests in internal/link ran). I also rango test -race -count=10 ./internal/filling/, which passed, so the new timing tests are not flaky here.The must-fix from comment 12004 is fixed. I reverted each part alone and saw its test fail:
wg.Wait, so it runs after every connection goroutine has ended, and nothing counted later is lost.The small fixes hold.
asknow has a 5 s deadline: with the accept fix reverted, TestTheSocketGoesOnAfterAnAcceptFails fails in 5 s instead of hanging. The busy-refusal cap is a channel made in each Serve. Connections waiting to be judged are capped at 256, and a connection over the cap is closed unread and counted asoverflow: with the bound removed, TestAConnectionOverTheWaitingBoundIsClosedUnread fails.Nothing from comment 11959 regressed. The delta since
0c17c76touches only internal/filling, and the tests for the library hold, the flood, the silent client, the directory mode, unknown fields and the two-resources refusal all pass.Should (can follow):
wg. If Serve takes the ErrClosed branch (line 308) while ctx is still live, the deferredwg.Waitblocks forever and the error is never returned. Today that branch is unreachable, because only ctx closes the listener, but it would turn a loud failure into a silent hang. Stop the ticker on a channel that Serve closes when it returns.Acceptsasked for in comment 12004 (point 2) is still open. It can follow.Merge check at
27672782Judged: head
2767278228bcmerged into trunkc18d5a8ff38d; read beside it: mesh-catalogebf7f3773dc7, mesh-controllerdf9e5728a925, mesh-controller-maindf9e5728a925, mesh-tools954e1a53eef9mesh/merge-gate: PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: mesh-host)mesh/repo-check: PASS — its merge-check.sh passedDelivery plan — builds mesh-host → ace, g14, novox, shanks; no bus step
Run by the build seat on novox as
build-1791940034980212032(builds --log build-1791940034980212032).