Skip to content

Add support for systemd socket activation#584

Merged
wesm merged 7 commits intoroborev-dev:mainfrom
atheriel:socket-activation
Mar 29, 2026
Merged

Add support for systemd socket activation#584
wesm merged 7 commits intoroborev-dev:mainfrom
atheriel:socket-activation

Conversation

@atheriel
Copy link
Copy Markdown
Contributor

@atheriel atheriel commented Mar 27, 2026

This commit facilitates running roborev as a socket-activated systemd user service. When LISTEN_FDS is set (handled by go-systemd internals), the daemon uses the systemd-provided socket instead of creating its own, enabling on-demand startup and lifecycle management driven by systemd.

We enforce that socket activation is used only with loopback TCP listeners and unix sockets with safe permissions.

We also avoid cleaning up the socket file in this mode, since we don't own it.

Closes #569.

Sample systemd user service and socket files, which we could put into the documentation:

[Unit]
Description=roborev daemon
Requires=roborev.socket

[Service]
Type=simple
ExecStart=%h/.local/bin/roborev daemon run
Restart=on-failure

[Install]
WantedBy=default.target
[Unit]
Description=roborev daemon socket

[Socket]
ListenStream=%t/roborev/daemon.sock
SocketMode=0600

[Install]
WantedBy=sockets.target

These both go into ~/.config/systemd/user. To run roborev as a socket-activated user service, you'd then:

$ systemctl --user daemon-reload
$ systemctl --user enable --now roborev.socket
# Use roborev, e.g.:
$ roborev --server "unix://$XDG_RUNTIME_DIR/roborev/daemon.sock" status

@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Mar 27, 2026

roborev: Combined Review (29b25d1)

Summary verdict: changes are directionally sound, but they introduce one high-severity trust-boundary regression and two medium-severity correctness/security gaps in the socket-activation path.

High

  • internal/daemon/server.go:136
    The socket-activation path accepts any listener handed over by systemd without re-validating that it preserves the daemon's local-only trust boundary. If the .socket unit binds 0.0.0.0, [::], or another non-loopback interface, the daemon's unauthenticated HTTP API becomes remotely reachable. The activated listener should be rejected unless it is loopback-only or matches the expected configured local endpoint.

Medium

  • internal/daemon/server.go:136
    The runtime metadata stores listener.Addr().String() for activated TCP listeners. With common systemd setups this can be a wildcard listen address such as 0.0.0.0:7373 or [::]:7373, which is not a usable client dial target. Since the CLI relies on this metadata to reconnect, the daemon can start successfully but become unreachable to local clients. Normalize the stored address to a concrete local dial address, or reuse the configured address when compatible.

  • internal/daemon/server.go:180
    Activated Unix sockets bypass the existing hardening that validates the parent directory and enforces owner-only socket permissions. A permissive .socket unit or directory configuration can therefore expose the daemon to other local users, breaking the current single-user authorization assumption. The startup path should validate equivalent ownership/mode guarantees for activated Unix sockets, or fail closed when they are too broad.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@atheriel atheriel force-pushed the socket-activation branch from 29b25d1 to 633fa45 Compare March 27, 2026 20:08
@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Mar 27, 2026

roborev: Combined Review (633fa45)

Verdict: No Medium, High, or Critical findings; the change looks clean based on all review outputs.

All three reviews reported no issues, and the two detailed reviews specifically called out the systemd socket activation handling as preserving the existing local-only trust model and daemon lifecycle behavior.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

atheriel and others added 6 commits March 29, 2026 10:29
This commit facilitates running roborev as a socket-activated systemd
user service. When LISTEN_FDS is set (handled by go-systemd internals),
the daemon uses the systemd-provided socket instead of creating its own,
enabling on-demand startup and lifecycle management driven by systemd.

We enforce that socket activation is used only with loopback TCP
listeners and unix sockets with safe permissions.

We also avoid cleaning up the socket file in this mode, since we don't
own it.

Closes roborev-dev#569.

Signed-off-by: Aaron Jacobs <atheriel@gmail.com>
CleanupZombieDaemons previously called KillDaemon for unresponsive
zombies, which unconditionally unlinked Unix sockets. When the zombie's
socket matched the systemd-managed target, this broke future connects.
Now we kill the process and clean up the runtime file while preserving
the socket.

Also add daemon.SdNotify(READY=1) so Type=notify systemd units work
correctly, and fix go-systemd dependency classification (direct, not
indirect).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When killing a zombie whose socket matches the systemd target, verify
the process is actually gone before removing runtime metadata. If the
kill fails, leave the runtime file so the next startup attempt can
retry.

Also guard against nil listener entries from activation.Listeners(),
which returns nil for unsupported socket types (e.g. UDP FDs).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
killProcess returns true when the PID has been reused by a non-roborev
process, since the original daemon is gone. The previous isProcessAlive
check would see the reused process as alive and leave stale runtime
metadata behind, blocking startup on the systemd-managed socket.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Exercises the CleanupZombieDaemons target-socket path where
killProcess returns true because the PID was reused by a non-roborev
process. Verifies the runtime file is removed while the socket is
preserved.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Abstract sockets (@name) don't exist on the filesystem, so they can't
be permission-checked or recorded in runtime metadata. Reject them
early with an actionable error pointing at ListenStream= rather than
letting them fall through to a confusing "path must be absolute" error
from ParseEndpoint.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@wesm wesm force-pushed the socket-activation branch from 633fa45 to ba346f3 Compare March 29, 2026 21:14
@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Mar 29, 2026

roborev: Combined Review (ba346f3)

Verdict: Changes look good overall, with 1 medium-severity correctness issue to address before merge.

Medium

  • Stale socketActivated state can break reused server startup
    • Location: internal/daemon/server.go:134-141, internal/daemon/server.go:177
    • Issue: s.socketActivated is only set to true and never reset when Start runs without an activated listener. If the same Server instance is reused after a socket-activated run, a later non-systemd start can incorrectly skip the normal listener bind path, leaving listener nil and breaking startup/cleanup behavior.
    • Suggested fix: Set s.socketActivated = (listener != nil) at the start of Start, or explicitly reset it to false in the non-systemd branch before bind logic.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@wesm
Copy link
Copy Markdown
Collaborator

wesm commented Mar 29, 2026

Addressed some minor things from running roborev reviews, will merge this soon

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Mar 29, 2026

roborev: Combined Review (84f6120)

Verdict: No Medium, High, or Critical findings.

Reviewed outputs did not identify any issues at Medium severity or above. One review reported only Low-severity notes, which are omitted per your instructions.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@wesm wesm merged commit ea575f3 into roborev-dev:main Mar 29, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for systemd socket activation

2 participants