site serve and view: pick a free port, and report a held port as taken #251

Closed
allod-agent wants to merge 9 commits from agent/site-serve-free-port into master
Member

allod site serve now serves a site that has no port in the registry, on a free port it picks, and allod site view forwards whatever port the VM reports. A start is reported as working only when the preview itself holds the port, so another program on that port is no longer printed as the site's address.

  • What changes for a running machine: a site with no preview_port is served instead of refused. A port something else holds is refused with exit 1 before anything starts. A preview whose app listens on all addresses instead of 127.0.0.1 is no longer reported as served.
  • What stays identical: a site with a registry port, or given --port, on a free port is started with the same systemd-run command as before. --stop needs no port. Exit codes keep their meaning.
  • What proves it: the race the issue describes was run for real on a dev VM, before and after, with another process taking each picked port. Tests cover each validation line of the issue.
  • Not proven here: view forwarding from a real hypervisor to a real VM. It is tested up to the ssh arguments it builds. That run is yours.
  • If it is wrong: revert this PR. The only state is the environment of a running unit, which goes when the unit stops.

Closes allod/tools#249

The issue's default was not enough

The issue proposes a check before the start and one retry, and leaves "accept a connection only when the listening socket belongs to a process of the unit" as a follow-up "unless the check before the start proves not enough". It was measured, and it is not enough. With the default alone, and another process taking the picked port before the app bound it:

took 35579 at 00:34:44.041
--- exit=0
--- stdout:
http://127.0.0.1:35579
--- the unit's own log:
[ERROR live_server::http_layer::listener] Address 127.0.0.1:35579 is already in use

The unit is still up while nix run evaluates, so a connection answered by the other process was taken for the preview. This PR therefore carries both the check before the start and the socket-owner rule. The same race on this branch, at 4d3e582:

took 37943 at 01:03:54.281
took 38687 at 01:03:54.586
--- exit=1
--- stdout:
--- stderr:
allod: no preview port for example/site: picked 37943 for this run; ...
allod: no preview port for example/site: picked 38687 for this run; ...
allod: allod-preview-example-site is no longer running; its last log lines are above
--- units:

Exit 1, no address, no unit left, and two starts: one retry.

What changed

  • Port order in serve: --port, the registry's preview_port, the port of the site's unit if it is running, then a port picked by binding 127.0.0.1 port 0 and releasing it.
  • Where a picked port is kept: in the unit, as the ALLOD_PREVIEW_PORT it was started with, read back with systemctl --user show -p Environment. There is no state file; the port lasts exactly as long as the unit.
  • The check before a start: one connection to the port. An answer is exit 1, naming the port, and nothing is started. A port just picked is not checked again.
  • One retry: a picked port that the app loses is replaced once. A port that was given is never retried.
  • Success means the unit holds the socket (preview_socket_linux.go): the listening socket for 127.0.0.1:<port> in /proc/net/tcp, or for the IPv4-mapped loopback in /proc/net/tcp6, must be open in a process of the unit's control group.
  • view with no port of its own passes none, reads the address the remote start printed, and forwards that port.
  • Messages: a picked port is announced on standard error, with the preview_port that would make it permanent. A start's standard output is the address line alone.
  • docs/allod-site-preview.md and both help texts say the same.

Three behaviours beyond the issue's text:

  • A preview app that ignores ALLOD_PREVIEW_INTERFACE and listens on 0.0.0.0 or :: ends at exit 3, still starting, where it used to be reported as served. The contract already required 127.0.0.1.
  • A running unit with no readable port in its environment is refused, pointing at --stop or --port, instead of getting a second picked port.
  • When the socket's owner cannot be read at all, serve exits 1 and stops the unit if this run started it.

Risk

R3. The change touches how a systemd unit is started and judged, and makes the hypervisor act on a value a VM reports.

  • The hypervisor trusts one number from the VM. view keeps at most 128 bytes of what the VM prints, accepts exactly one line of exactly http://127.0.0.1:<digits> with the port from 1024 to 65535, and hands ssh -L the port as a formatted integer. A VM can therefore choose which loopback port at or above 1024 the hypervisor listens on, and nothing else. It could not before.
  • A false "not served" is the costly failure. If the socket check misses a listener that does serve correctly, a working preview ends at exit 3 after 60 seconds. Two kinds of listener are matched and were measured against real sockets. A preview whose server runs outside the unit's control group would be missed; none of the three worked examples in the doc does that.
  • Linux only. Elsewhere serve exits 1 saying the owner cannot be read. serve needs a user systemd manager before it gets that far.
  • Not closed: the VM's standard error, and with serve --vm its standard output, reach the hypervisor's terminal verbatim, control sequences included. That was so before this change and is left for an issue of its own.
  • This branch and allod/tools#250 both edit site_view.go and site_serve.go, on different lines. Whichever merges second needs master merged into it first.

Validation

Run by me on a dev VM. The tests, go vet, gofmt and allod check ran at 05ac91d, the head of this branch. The runs of the binary ran at 4d3e582, which differs from the head in one test file only, against a throwaway site whose preview app is live-server:

  • go test ./..., go test -tags site ./..., go test -tags secret ./..., go test -tags site,secret ./..., each with -count=1: all seven packages ok in each of the four runs.
  • go vet ./... and gofmt -l .: no output.
  • allod check: All 11 steps passed (checks: 9 built). It runs the Go tests inside a nix build sandbox, where TestPreviewSocketOwnerReadsTheControlGroup skips: the sandbox names a control group whose directory it does not mount. That test runs and passes in the dev shell on the VM, so inside the repository check the socket-owner rule is not witnessed against a real control group.
  • No registry port, no --port: exit 0, http://127.0.0.1:38811, the page answers 200, and the listener on that port is the unit's live-server.
  • A second serve while running: the same address.
  • --stop: exit 0, with no port given.
  • A port held by socat, from the registry and from --port: port 18649 is already in use: ... was not started, exit 1, no unit.
  • Every picked port taken between the pick and the bind: quoted above.
  • Afterwards no allod-preview-* unit, no socat, no live-server.

Run by the implementing worker at 1b2badc, which differs from 4d3e582 in comments and one reflowed help line, and not repeated by me:

  • Only the first picked port taken: exit 0 on the second port, held by the unit's own server.
  • --stop, then serve: a new port is picked.
  • A registry port on a free port: served there.
  • An AF_INET6 socket bound to ::ffff:127.0.0.1 answers a connection to 127.0.0.1 and appears only in /proc/net/tcp6.
  • Each new test was shown failing, either on the code before its fix or under a temporary sabotage that was reverted.

Not run:

  • view from a second machine: the ssh to the VM and the ssh -L forward.
  • Anything on a non-Linux machine.
  • The socket check on a machine without /proc/net/tcp6; that branch is written and not exercised.

Review

One reviewer from another model family (gpt-5.6-sol) read the diff cold at 1e496ca, followed by one fix round.

  • Acted on: view buffered the VM's output without limit; it now keeps 128 bytes.
  • Acted on: a listener on the IPv4-mapped loopback was never recognised; /proc/net/tcp6 is read too.
  • Acted on: a quoted Environment value could be misread as the port; a line with a quote character is refused.
  • Acted on: an unreadable socket owner left a unit running that this run had started.
  • Acted on: two tests that witnessed less than their names said, and one doc sentence that was false for --stop.
  • Declined as outside this issue: the VM's output reaching the hypervisor's terminal verbatim, noted under Risk.
  • The reviewer found no way for the VM's output to reach an ssh argument or a shell other than as the parsed integer, and no case of another process's socket reported as the unit's.

Comments were cut down at the owner's request before this PR was opened, as on allod/archetypes#120 and allod/tools#250.

`allod site serve` now serves a site that has no port in the registry, on a free port it picks, and `allod site view` forwards whatever port the VM reports. A start is reported as working only when the preview itself holds the port, so another program on that port is no longer printed as the site's address. - **What changes for a running machine:** a site with no `preview_port` is served instead of refused. A port something else holds is refused with exit 1 before anything starts. A preview whose app listens on all addresses instead of 127.0.0.1 is no longer reported as served. - **What stays identical:** a site with a registry port, or given `--port`, on a free port is started with the same `systemd-run` command as before. `--stop` needs no port. Exit codes keep their meaning. - **What proves it:** the race the issue describes was run for real on a dev VM, before and after, with another process taking each picked port. Tests cover each validation line of the issue. - **Not proven here:** `view` forwarding from a real hypervisor to a real VM. It is tested up to the `ssh` arguments it builds. That run is yours. - **If it is wrong:** revert this PR. The only state is the environment of a running unit, which goes when the unit stops. Closes allod/tools#249 ## The issue's default was not enough The issue proposes a check before the start and one retry, and leaves "accept a connection only when the listening socket belongs to a process of the unit" as a follow-up "unless the check before the start proves not enough". It was measured, and it is not enough. With the default alone, and another process taking the picked port before the app bound it: ``` took 35579 at 00:34:44.041 --- exit=0 --- stdout: http://127.0.0.1:35579 --- the unit's own log: [ERROR live_server::http_layer::listener] Address 127.0.0.1:35579 is already in use ``` The unit is still up while `nix run` evaluates, so a connection answered by the other process was taken for the preview. This PR therefore carries both the check before the start and the socket-owner rule. The same race on this branch, at `4d3e582`: ``` took 37943 at 01:03:54.281 took 38687 at 01:03:54.586 --- exit=1 --- stdout: --- stderr: allod: no preview port for example/site: picked 37943 for this run; ... allod: no preview port for example/site: picked 38687 for this run; ... allod: allod-preview-example-site is no longer running; its last log lines are above --- units: ``` Exit 1, no address, no unit left, and two starts: one retry. ## What changed - **Port order in `serve`:** `--port`, the registry's `preview_port`, the port of the site's unit if it is running, then a port picked by binding 127.0.0.1 port 0 and releasing it. - **Where a picked port is kept:** in the unit, as the `ALLOD_PREVIEW_PORT` it was started with, read back with `systemctl --user show -p Environment`. There is no state file; the port lasts exactly as long as the unit. - **The check before a start:** one connection to the port. An answer is exit 1, naming the port, and nothing is started. A port just picked is not checked again. - **One retry:** a picked port that the app loses is replaced once. A port that was given is never retried. - **Success means the unit holds the socket** (`preview_socket_linux.go`): the listening socket for `127.0.0.1:<port>` in `/proc/net/tcp`, or for the IPv4-mapped loopback in `/proc/net/tcp6`, must be open in a process of the unit's control group. - **`view`** with no port of its own passes none, reads the address the remote start printed, and forwards that port. - **Messages:** a picked port is announced on standard error, with the `preview_port` that would make it permanent. A start's standard output is the address line alone. - `docs/allod-site-preview.md` and both help texts say the same. Three behaviours beyond the issue's text: - A preview app that ignores `ALLOD_PREVIEW_INTERFACE` and listens on `0.0.0.0` or `::` ends at exit 3, still starting, where it used to be reported as served. The contract already required 127.0.0.1. - A running unit with no readable port in its environment is refused, pointing at `--stop` or `--port`, instead of getting a second picked port. - When the socket's owner cannot be read at all, `serve` exits 1 and stops the unit if this run started it. ## Risk R3. The change touches how a systemd unit is started and judged, and makes the hypervisor act on a value a VM reports. - **The hypervisor trusts one number from the VM.** `view` keeps at most 128 bytes of what the VM prints, accepts exactly one line of exactly `http://127.0.0.1:<digits>` with the port from 1024 to 65535, and hands `ssh -L` the port as a formatted integer. A VM can therefore choose which loopback port at or above 1024 the hypervisor listens on, and nothing else. It could not before. - **A false "not served" is the costly failure.** If the socket check misses a listener that does serve correctly, a working preview ends at exit 3 after 60 seconds. Two kinds of listener are matched and were measured against real sockets. A preview whose server runs outside the unit's control group would be missed; none of the three worked examples in the doc does that. - **Linux only.** Elsewhere `serve` exits 1 saying the owner cannot be read. `serve` needs a user systemd manager before it gets that far. - **Not closed:** the VM's standard error, and with `serve --vm` its standard output, reach the hypervisor's terminal verbatim, control sequences included. That was so before this change and is left for an issue of its own. - This branch and allod/tools#250 both edit `site_view.go` and `site_serve.go`, on different lines. Whichever merges second needs master merged into it first. ## Validation Run by me on a dev VM. The tests, `go vet`, `gofmt` and `allod check` ran at `05ac91d`, the head of this branch. The runs of the binary ran at `4d3e582`, which differs from the head in one test file only, against a throwaway site whose preview app is `live-server`: - `go test ./...`, `go test -tags site ./...`, `go test -tags secret ./...`, `go test -tags site,secret ./...`, each with `-count=1`: all seven packages `ok` in each of the four runs. - `go vet ./...` and `gofmt -l .`: no output. - `allod check`: `All 11 steps passed (checks: 9 built).` It runs the Go tests inside a nix build sandbox, where `TestPreviewSocketOwnerReadsTheControlGroup` skips: the sandbox names a control group whose directory it does not mount. That test runs and passes in the dev shell on the VM, so inside the repository check the socket-owner rule is not witnessed against a real control group. - No registry port, no `--port`: exit 0, `http://127.0.0.1:38811`, the page answers 200, and the listener on that port is the unit's `live-server`. - A second `serve` while running: the same address. - `--stop`: exit 0, with no port given. - A port held by `socat`, from the registry and from `--port`: `port 18649 is already in use: ... was not started`, exit 1, no unit. - Every picked port taken between the pick and the bind: quoted above. - Afterwards no `allod-preview-*` unit, no `socat`, no `live-server`. Run by the implementing worker at `1b2badc`, which differs from `4d3e582` in comments and one reflowed help line, and not repeated by me: - Only the first picked port taken: exit 0 on the second port, held by the unit's own server. - `--stop`, then `serve`: a new port is picked. - A registry port on a free port: served there. - An AF_INET6 socket bound to `::ffff:127.0.0.1` answers a connection to 127.0.0.1 and appears only in `/proc/net/tcp6`. - Each new test was shown failing, either on the code before its fix or under a temporary sabotage that was reverted. Not run: - `view` from a second machine: the `ssh` to the VM and the `ssh -L` forward. - Anything on a non-Linux machine. - The socket check on a machine without `/proc/net/tcp6`; that branch is written and not exercised. ## Review One reviewer from another model family (`gpt-5.6-sol`) read the diff cold at `1e496ca`, followed by one fix round. - Acted on: `view` buffered the VM's output without limit; it now keeps 128 bytes. - Acted on: a listener on the IPv4-mapped loopback was never recognised; `/proc/net/tcp6` is read too. - Acted on: a quoted `Environment` value could be misread as the port; a line with a quote character is refused. - Acted on: an unreadable socket owner left a unit running that this run had started. - Acted on: two tests that witnessed less than their names said, and one doc sentence that was false for `--stop`. - Declined as outside this issue: the VM's output reaching the hypervisor's terminal verbatim, noted under Risk. - The reviewer found no way for the VM's output to reach an `ssh` argument or a shell other than as the parsed integer, and no case of another process's socket reported as the unit's. Comments were cut down at the owner's request before this PR was opened, as on allod/archetypes#120 and allod/tools#250.
Port resolution in 'serve' becomes --port, the registry's preview_port, the
port the running unit was started on, then one the kernel picks. A picked
port is kept with the unit and nowhere else: systemd-run puts it in the
unit's environment, which lasts exactly as long as the unit.

Before starting a unit that is not running, 'serve' connects to the port
once; a port that answers belongs to something else, so it exits 1 and
starts nothing. A picked port skips that check, having just been free, and
is retried once with a new port when the unit stops before it serves.

Refs allod/tools#249
With no --port and no preview_port of its own, 'view' passes no --port to
the remote start, captures its standard output and forwards the port in the
address it printed. That output is untrusted input on the hypervisor, so
exactly one line reading http://127.0.0.1:<digits> is accepted, with the
digits in the range previewPortValue takes; anything else exits 1 saying
the serving machine reported no usable address.

Refs allod/tools#249
The contract bullet no longer says a held port is reported as serving. Both
sections gain the port order, where a picked port is kept, and the check
before a start; the exit-code paragraph names a port already in use.

Refs allod/tools#249
Measured on this machine with a real unit: with the check before the start
alone, a port taken between the pick and the app's bind was still reported
as serving. The unit is up while nix evaluates, so the squatter answered
first, 'serve' printed its address and exited 0, and the app in the unit
then failed with "Address 127.0.0.1:<port> is already in use".

The wait now also asks the kernel who holds the listening socket:
/proc/net/tcp gives its inode, the unit's ControlGroup names the processes
under /sys/fs/cgroup, and /proc/<pid>/fd says which inodes each holds. A
connection answered while no process of the unit holds that socket is not
success, so the existing stopped-unit and timeout paths report instead.
Linux only, split as termios_linux.go and termios_other.go are.

Refs allod/tools#249
The VM is untrusted on the hypervisor and can print without end, so the
remote start's standard output now goes through previewCapture: the first
128 bytes and a flag that more followed. An overflow is the same refusal as
any other unusable answer, and memory no longer grows with what the remote
prints.

Refs allod/tools#249
Measured here: an AF_INET6 socket with IPV6_V6ONLY off, bound to
::ffff:127.0.0.1, answers a connection to 127.0.0.1 and shows up only in
/proc/net/tcp6, as 0000000000000000FFFF00000100007F. A preview serving from
such a socket read as one that had not bound its port, and ended at exit 3.
That address is now matched in /proc/net/tcp6 as well, LISTEN only, and
nothing else from that file.

Refs allod/tools#249
Two smaller faults in the serve path. systemd quotes an Environment value
holding whitespace, so splitting the line on whitespace could read a port
out of the middle of another variable's value; a line carrying a quote is
not one sitePreviewStart wrote, and now yields no port. And when the socket
owner cannot be read, a unit this run started is stopped before the
refusal, since its environment would otherwise hand the next run a port
this run could not confirm; a unit that was already running is left alone.

Also: a test of the real port picker rather than the replaced one, an exact
command assertion in the control-group stub, and the standard-output
sentence qualified to a start, which '--stop' is not.

Refs allod/tools#249
Each comment that stays names a contract or a trap in one to three lines.
The ones that said what a function or a test does are gone. The serve help
loses one over-long line; no code changes.

Refs allod/tools#249
A nix build sandbox reports a control group in /proc/self/cgroup whose
directory under /sys/fs/cgroup it does not mount, so the test read its own
socket as not owned and failed the repository check. It now skips there
and still runs on a machine, where the group is real.

Refs allod/tools#249
vnprc force-pushed agent/site-serve-free-port from 05ac91d910 to eb35256652 2026-09-29 02:12:34 +01:00 Compare
Author
Member

Closing unmerged, at the owner's decision. Picking a port is dropped: a site is served on the port the registry gives it, or on --port, and on nothing else. The reasoning is in the closing comment on allod/tools#249.

The part worth keeping, refusing to start on a port something else holds, continues as allod/tools#252 on a new branch cut from master, so master does not carry nine commits that add the pick followed by more that remove it.

The measurements and the review recorded in this pull request's description stay valid for what they describe. The socket-owner rule built here is named in allod/tools#252 as the follow-up if a port is ever seen taken between the check and the app's bind.

Closing unmerged, at the owner's decision. Picking a port is dropped: a site is served on the port the registry gives it, or on `--port`, and on nothing else. The reasoning is in the closing comment on allod/tools#249. The part worth keeping, refusing to start on a port something else holds, continues as allod/tools#252 on a new branch cut from master, so master does not carry nine commits that add the pick followed by more that remove it. The measurements and the review recorded in this pull request's description stay valid for what they describe. The socket-owner rule built here is named in allod/tools#252 as the follow-up if a port is ever seen taken between the check and the app's bind.
allod-agent closed this pull request 2026-09-29 02:25:59 +01:00

Pull request closed

Sign in to join this conversation.
No description provided.