diff --git a/deploy/fleetd.service b/deploy/fleetd.service index 6d4e4eb..38e42ef 100644 --- a/deploy/fleetd.service +++ b/deploy/fleetd.service @@ -1,72 +1,72 @@ # CB-504 — systemd unit for fleetd (Linux). # -# The macOS launchd agent (deploy/dev.ltms.fleetd.plist) is the supervision target for the -# current single-host deployment. This unit exists for the per-host gateways CB-308 introduces, -# which will run on Linux. +# fleetd #360: the previous version of this file started clean and broke the daemon in three ways +# that nothing logs (see the DO NOT block and the ExecStart/PrivateTmp comments below for what and +# why). The unit below, plus its companion deploy/herdr.service, is the version that has actually +# run on fleet01 without those failures. Do not "improve" it back toward the old shape without +# re-reading why each line is the way it is. # # Install (user service — fleetd drives the user's herdr, not a system daemon): # mkdir -p ~/.config/systemd/user -# cp deploy/fleetd.service ~/.config/systemd/user/ -# # edit ExecStart / WorkingDirectory / Environment below, then: +# cp deploy/fleetd.service deploy/herdr.service ~/.config/systemd/user/ +# # edit WorkingDirectory / ExecStart below for your host's paths and java location # systemctl --user daemon-reload -# systemctl --user enable --now fleetd +# systemctl --user enable --now herdr fleetd # journalctl --user -u fleetd -f +# +# Secrets (AI_GATEWAY_TOKEN, WORKER_GITEA_TOKEN, LAVINMQ_URI, COORD_AMQP_URI, ...) are not set +# here and need no systemd drop-in: ExecStart runs a login shell, so they come from wherever your +# login shell already sources them (this host: ~/.fleet/secrets.sh via ~/.zprofile). If a token is +# missing there, fleetd still starts — the daemon reports every secret a configured profile +# references, by name, never by value: +# journalctl --user -u fleetd | grep 'startup secret' +# A resolved one logs "startup secret NAME: set (profile 'x' tokenEnv)"; a missing one logs +# "startup secret NAME: MISSING" at WARN and the daemon starts anyway — the first visible symptom +# is a member that cannot open a pull request, hours later and in a different component. [Unit] -Description=fleetd — claude-bridge message server +Description=fleetd — fleet message server Documentation=https://git.ltms.dev/fleet/fleetd/wiki -# Ordering only: herdr is a user process and its socket may appear after us. This is advisory — -# fleetd retries the herdr socket rather than exiting, which is what actually makes a late -# socket survivable. Do NOT add Requires=: a herdr restart must not take fleetd down with it. +# Ordering only. fleetd retries the herdr socket rather than exiting, which is what actually makes +# a late socket survivable. Do NOT add Requires=: a herdr restart must not take fleetd down too. After=herdr.service Wants=herdr.service [Service] Type=simple -WorkingDirectory=%h/src/claude-bridge/fleetd -ExecStart=/usr/lib/jvm/temurin-25-jdk/bin/java -jar target/fleetd.jar fleetd.yaml +WorkingDirectory=%h/LTMS/fleetd/fleetd -Environment=HERDR_SOCKET_PATH=%h/.config/herdr/herdr.sock -# PATH matters more than it looks (CB-511): fleetd propagates its own PATH to every worker it -# spawns, so this line decides whether the fleet can run a build at all. systemd does not source a -# login shell, so without it the daemon — and every worker — gets a bare default with no JDK/Maven. -Environment=PATH=/usr/lib/jvm/temurin-25-jdk/bin:/usr/share/maven/bin:/usr/local/bin:/usr/bin:/bin -# Secrets are NOT set here — this file is committed. Put ALL three tokens in a private drop-in -# that systemd reads with restrictive permissions. In `systemctl --user edit fleetd`, add: -# [Service] -# Environment=FLEETD_API_TOKEN=... -# Environment=WORKER_GITEA_TOKEN=... -# Environment=AI_GATEWAY_TOKEN=... -# FLEETD_API_TOKEN protects fleetd's API. WORKER_GITEA_TOKEN lets members open pull requests; if -# it is missing, fleetd still starts, but a member fails when it later tries to open a pull request. -# AI_GATEWAY_TOKEN authenticates gateway profiles; if it is missing, fleetd still starts, but a -# gateway profile later returns HTTP 401. Or, put the same three variables in a 0600 file and add: -# EnvironmentFile=%h/.config/fleetd/env -# After starting, check which of them actually resolved. The daemon reports every secret a -# configured profile references, by name, never by value: -# journalctl --user -u fleetd | grep 'startup secret' -# A resolved one logs "startup secret NAME: set (profile 'x' tokenEnv)". A missing one logs -# "startup secret NAME: MISSING" at WARN — and the daemon starts anyway, which is the whole -# problem: without this grep the first sign is a member that cannot open a pull request, hours -# later and in a different component. -# Note what the report can and cannot tell you. It lists only names some profile actually -# references (tokenEnv, gitTokenEnv, and the broker uriEnv). A secret nothing references is never -# reported, because nothing needs it. +# A LOGIN shell, not java directly. Every secret this daemon needs (AI_GATEWAY_TOKEN, +# WORKER_GITEA_TOKEN, LAVINMQ_URI, COORD_AMQP_URI) lives in ~/.fleet/secrets.sh, which only +# ~/.zprofile sources. systemd runs no login shell. Started any other way the daemon boots fine +# and looks healthy, and the failure appears hours later as a member that cannot open a pull +# request. exec keeps it one process, so systemd tracks the right PID. +# This also avoids a SECOND copy of the secrets in a systemd drop-in: one source of truth. +ExecStart=/bin/zsh -lc "exec java -jar target/fleetd.jar fleetd.yaml" + +# PrivateTmp MUST stay false -- see herdr.service. fleetd creates the member ZDOTDIR scrub dir and +# the opencode config dir under java.io.tmpdir, and the member pane (a herdr child, a different +# unit) has to read them. A private /tmp turns the credential scrub into a silent no-op. +PrivateTmp=false Restart=on-failure RestartSec=10s -# A bad config (e.g. a non-loopback bind without token auth) makes fleetd fail fast by design. -# Give up rather than restart-loop on a permanent error. +# A bad config makes fleetd fail fast by design. Give up rather than restart-loop forever. StartLimitBurst=5 StartLimitIntervalSec=120 -# The daemon reads the repo, writes worktrees, and talks to a Unix socket — it needs no more. +# DO NOT add ProtectSystem=, ProtectHome=, ProtectKernelTunables= or ProtectControlGroups=. +# Measured on fleet01 2026-09-05: each of those gives the unit its own mount namespace, and +# fleetd resolves a caller role by running lsof to find the loopback peer PID +# (mcp/LsofPeerPidLookup). Inside such a namespace lsof returns nothing, every caller falls back +# to ANONYMOUS, and the primary is refused every orchestration call with +# "unauthenticated: anonymous may not SPAWN". +# The daemon still starts, healthz still returns ok and the secrets still resolve - the only +# symptom is that the fleet cannot be driven at all. Verified by bisecting the directives: +# no sandbox 3 lsof lines | ProtectSystem=strict 0 | ProtectHome=read-only 0 +# ProtectKernelTunables 0 | ProtectControlGroups 0 | RestrictSUIDSGID 3 | NoNewPrivileges 3 +# The two below add no mount namespace and are safe. NoNewPrivileges=true -PrivateTmp=true -ProtectSystem=strict -ProtectHome=read-write -ProtectKernelTunables=true -ProtectControlGroups=true RestrictSUIDSGID=true StandardOutput=journal diff --git a/deploy/herdr-inner.sh b/deploy/herdr-inner.sh new file mode 100755 index 0000000..5f3c416 --- /dev/null +++ b/deploy/herdr-inner.sh @@ -0,0 +1,19 @@ +#!/bin/zsh +# fleetd #360 — template for the script deploy/herdr.service's ExecStart wraps in a pty. +# +# `script -qfec /dev/null` needs a real command to run, and that command has to be a LOGIN +# shell script: herdr itself needs the same secrets fleetd.service's login shell picks up (this +# host: ~/.fleet/secrets.sh via ~/.zprofile), because members it spawns inherit its environment. +# systemd's own Environment= lines in herdr.service are not enough for that -- they set TERM and a +# bare PATH so the pty starts at all, nothing more. +# +# Copy this file to the path deploy/herdr.service's ExecStart names +# (%h/LTMS/fleetd/fleetd-run/herdr-inner.sh by default) and `chmod +x` it. Not committed under +# that path itself because the session name below is host-specific. + +# A 0x0 pty makes every pane spawn fail with "ghostty error -2" (see herdr-multi-instance-facts / +# fleet01-headless-herdr-standup) -- give it a real size before herdr ever touches it. +stty rows 50 cols 200 + +# -l: login shell, so herdr and everything it spawns gets the real secrets and PATH. +exec zsh -lc 'exec herdr --session ' diff --git a/deploy/herdr.service b/deploy/herdr.service new file mode 100644 index 0000000..ecb999c --- /dev/null +++ b/deploy/herdr.service @@ -0,0 +1,40 @@ +# fleetd #360 — systemd unit for herdr (Linux), the terminal multiplexer fleetd drives. +# +# This is fleetd.service's companion: fleetd.service's After=/Wants=herdr.service assumes this +# unit exists. Before this ticket it did not, so on a fresh host fleetd started against a herdr +# that systemd never supervised at all. +# +# Install: see deploy/fleetd.service's header comment (both units install the same way). +# +# ExecStart below runs deploy/herdr-inner.sh (copy the template of that name from this directory +# to the path in ExecStart, or point ExecStart at wherever you keep it, and make it executable). +# It is a separate file rather than an inline command because it must itself be a login shell (see +# its own header for why) and systemd's ExecStart does not run one. + +[Unit] +Description=herdr terminal multiplexer (fleet session) +Documentation=https://git.ltms.dev/fleet/fleetd/wiki + +[Service] +Type=simple +# script(1) gives herdr a real pty. Without it the client reports a 0x0 window and every pane +# spawn fails with "ghostty error -2" -- which surfaces as a fleetd spawn failure, not a herdr one. +ExecStart=/usr/bin/script -qfec %h/LTMS/fleetd/fleetd-run/herdr-inner.sh /dev/null +StandardInput=null +Environment=TERM=xterm-256color +Environment=PATH=%h/.local/bin:/usr/local/bin:/usr/bin:/bin + +# PrivateTmp MUST stay false. fleetd writes the member ZDOTDIR scrub dir and the opencode config +# dir under its own java.io.tmpdir, and the member pane -- a child of THIS process -- has to read +# them. A private /tmp here silently breaks the credential scrub instead of failing loudly. +PrivateTmp=false + +Restart=on-failure +RestartSec=5s + +StandardOutput=journal +StandardError=journal +SyslogIdentifier=herdr + +[Install] +WantedBy=default.target diff --git a/fleetd/src/test/java/dev/ltms/fleet/deploy/SystemdUnitSafetyTest.java b/fleetd/src/test/java/dev/ltms/fleet/deploy/SystemdUnitSafetyTest.java new file mode 100644 index 0000000..f27f3cf --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/deploy/SystemdUnitSafetyTest.java @@ -0,0 +1,232 @@ +package dev.ltms.fleet.deploy; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.regex.Pattern; +import java.util.stream.Stream; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * fleetd #360: {@code deploy/fleetd.service} started clean on fleet01 and broke the daemon in + * three ways that nothing logs at startup. + * + * + * + *

Review round 2 on fleetd #360 found the same shape one file further down the chain: + * {@code herdr.service}'s {@code ExecStart} only names {@code deploy/herdr-inner.sh}, so that + * script is the ONLY place two more of these properties live, and nothing was reading it. + * + *

+ * + *

None of these five failures makes the unit (or the script) fail to start, or makes + * {@code /healthz} report unhealthy, so nothing short of reading the files catches a regression. + * This test is that read. + * + *

It checks source text, not behaviour -- it cannot start systemd or fork an mount + * namespace in a build sandbox. It parses the same two files an operator would install and fails + * if a forbidden directive is active, exactly the way {@code McpContractDocTest} guards + * {@code docs/MCP-Contract.md} against naming a tool that does not exist. + */ +class SystemdUnitSafetyTest { + + /** Tests run with the module directory (fleetd/) as cwd; deploy/ is the repo-root sibling. */ + private static final Path FLEETD_SERVICE = Path.of("../deploy/fleetd.service"); + private static final Path HERDR_SERVICE = Path.of("../deploy/herdr.service"); + private static final Path HERDR_INNER_SCRIPT = Path.of("../deploy/herdr-inner.sh"); + + private static final List NAMESPACING_DIRECTIVES = List.of( + "ProtectSystem", "ProtectHome", "ProtectKernelTunables", "ProtectControlGroups"); + + /** + * A shell invoked with a login flag, e.g. {@code zsh -lc '...'} or {@code /bin/sh -l}. The + * property under test is the {@code -l}, not the specific shell or the rest of its flags, so + * this matches a token ending in "sh" followed by a flag cluster containing "l" -- tighter + * than a bare {@code contains("-lc")}, which a "-lc" anywhere in the line would also satisfy. + */ + private static final Pattern LOGIN_SHELL_INVOCATION = + Pattern.compile("(?:^|\\s)\\S*sh\\s+-[A-Za-z]*l[A-Za-z]*\\b"); + + private static final Pattern POSITIVE_ROWS = Pattern.compile("\\brows\\s+([1-9]\\d*)\\b"); + private static final Pattern POSITIVE_COLS = Pattern.compile("\\bcols\\s+([1-9]\\d*)\\b"); + + static Stream bothUnits() { + return Stream.of(FLEETD_SERVICE, HERDR_SERVICE); + } + + private static boolean invokesALoginShell(List activeLines) { + return activeLines.stream().anyMatch(l -> LOGIN_SHELL_INVOCATION.matcher(l).find()); + } + + /** An active {@code stty} line naming both a positive row count and a positive column count. */ + private static boolean setsANonZeroPtySize(List activeLines) { + return activeLines.stream() + .filter(l -> l.startsWith("stty")) + .anyMatch(l -> POSITIVE_ROWS.matcher(l).find() && POSITIVE_COLS.matcher(l).find()); + } + + /** Lines that are actually in force: comments and blank lines don't count. */ + private static List activeLines(Path unit) throws Exception { + List active = new ArrayList<>(); + for (String line : Files.readAllLines(unit)) { + String stripped = line.strip(); + if (!stripped.isEmpty() && !stripped.startsWith("#")) { + active.add(stripped); + } + } + return active; + } + + @ParameterizedTest + @MethodSource("bothUnits") + @DisplayName("[SOURCE TEXT] no active mount-namespacing directive -- it blinds lsof and turns every caller ANONYMOUS") + void doesNotActivateAMountNamespace(Path unit) throws Exception { + List active = activeLines(unit); + + for (String directive : NAMESPACING_DIRECTIVES) { + Pattern activeDirective = Pattern.compile("^" + Pattern.quote(directive) + "\\s*="); + List hits = active.stream().filter(l -> activeDirective.matcher(l).find()).toList(); + assertTrue(hits.isEmpty(), + unit + " sets " + directive + " (" + hits + "). That directive gives the unit its " + + "own mount namespace; inside it, fleetd's lsof-based caller lookup " + + "(mcp/LsofPeerPidLookup) returns nothing, so every MCP caller falls back to " + + "ANONYMOUS and the primary is refused every orchestration call with " + + "\"unauthenticated: anonymous may not SPAWN\" -- while the daemon still " + + "starts and /healthz still reports ok. Measured on fleet01 2026-09-05 " + + "(fleetd #360). A commented-out mention in the file's own DO-NOT-add block " + + "is fine; an active directive is not."); + } + } + + @ParameterizedTest + @MethodSource("bothUnits") + @DisplayName("[SOURCE TEXT] PrivateTmp is not true -- it silently no-ops the credential scrub") + void privateTmpIsNotTrue(Path unit) throws Exception { + List active = activeLines(unit); + Pattern privateTmpTrue = Pattern.compile("^PrivateTmp\\s*=\\s*true\\b"); + + boolean hasPrivateTmpTrue = active.stream().anyMatch(l -> privateTmpTrue.matcher(l).find()); + assertFalse(hasPrivateTmpTrue, + unit + " sets PrivateTmp=true. fleetd writes the member ZDOTDIR credential-scrub " + + "directory and the opencode config directory under java.io.tmpdir, and the " + + "member pane -- a child of a DIFFERENT unit -- has to read them back. A " + + "private /tmp on either unit turns the credential scrub into a silent no-op: " + + "no error, no log line, the scrub just never happens (fleetd #360)."); + } + + @Test + @DisplayName("[SOURCE TEXT] fleetd.service's ExecStart goes through a login shell -- otherwise every secret is empty") + void execStartUsesALoginShell() throws Exception { + List active = activeLines(FLEETD_SERVICE); + List execStartLines = active.stream().filter(l -> l.startsWith("ExecStart=")).toList(); + + assertTrue(execStartLines.size() == 1, + FLEETD_SERVICE + " must have exactly one active ExecStart= line; found " + + execStartLines.size() + ": " + execStartLines); + + String execStart = execStartLines.get(0); + assertTrue(LOGIN_SHELL_INVOCATION.matcher(execStart).find(), + FLEETD_SERVICE + "'s ExecStart (" + execStart + ") does not run a login shell (a shell " + + "invoked with a \"-l\" flag, e.g. \"zsh -lc\"). Every secret fleetd needs " + + "(AI_GATEWAY_TOKEN, WORKER_GITEA_TOKEN, LAVINMQ_URI, COORD_AMQP_URI) lives in a " + + "file only the login shell sources; systemd runs no login shell on its own. " + + "Running java directly boots fine with every credential empty, and the failure " + + "surfaces hours later as a member that cannot open a pull request (fleetd #360)."); + } + + @Test + @DisplayName("[SOURCE TEXT] herdr-inner.sh execs a login shell -- otherwise herdr and every member it spawns start with empty credentials") + void herdrInnerScriptUsesALoginShell() throws Exception { + List active = activeLines(HERDR_INNER_SCRIPT); + + assertTrue(invokesALoginShell(active), + HERDR_INNER_SCRIPT + " does not exec a login shell (a shell invoked with a \"-l\" flag, " + + "e.g. \"zsh -lc\"). herdr.service's ExecStart only names this script, so this " + + "is the ONLY place herdr's login-shell property lives. Without it, herdr -- and " + + "every member pane it spawns as a child of herdr -- starts with none of the " + + "secrets that only a login shell sources (this host: ~/.fleet/secrets.sh via " + + "~/.zprofile), and the failure surfaces hours later as a member with no " + + "credentials at all, not just fleetd (fleetd #360)."); + } + + @Test + @DisplayName("[SOURCE TEXT] herdr-inner.sh sets a non-zero pty size before starting herdr -- otherwise every pane spawn fails with \"ghostty error -2\"") + void herdrInnerScriptSetsANonZeroPtySize() throws Exception { + List active = activeLines(HERDR_INNER_SCRIPT); + + assertTrue(setsANonZeroPtySize(active), + HERDR_INNER_SCRIPT + " does not run \"stty rows N cols M\" with both N and M positive " + + "before starting herdr. Without a real pty size, herdr reports a 0x0 window and " + + "every pane spawn fails with \"ghostty error -2\" -- which surfaces as a fleetd " + + "spawn failure, not a herdr one, and gives no hint that the actual cause is this " + + "script (fleetd #360)."); + } + + /** + * The denominator guard, same shape as {@code McpContractDocTest}'s: a check that scans for a + * forbidden pattern passes trivially if it is handed nothing to scan. Pin that all three files + * exist, are non-trivial, and that the DO-NOT block's commented mentions are still there -- so + * the "comment survives, directive doesn't" distinction above is actually being exercised. + */ + @Test + @DisplayName("[SOURCE TEXT] the namespacing check is not vacuous -- all three files exist and the DO-NOT block still mentions every forbidden directive in a comment") + void theCheckActuallyHasSomethingToCheck() throws Exception { + assertTrue(Files.size(FLEETD_SERVICE) > 200, + FLEETD_SERVICE + " is missing or unexpectedly small -- the checks above would pass " + + "vacuously against an empty or absent file"); + assertTrue(Files.size(HERDR_SERVICE) > 200, + HERDR_SERVICE + " is missing or unexpectedly small -- the checks above would pass " + + "vacuously against an empty or absent file"); + assertTrue(Files.size(HERDR_INNER_SCRIPT) > 50, + HERDR_INNER_SCRIPT + " is missing or unexpectedly small -- the login-shell and " + + "pty-size checks above would either pass vacuously or fail with an opaque " + + "\"file not found\" instead of naming the actual consequence (empty " + + "credentials / \"ghostty error -2\") against an empty or absent file"); + + String fleetdService = Files.readString(FLEETD_SERVICE); + for (String directive : NAMESPACING_DIRECTIVES) { + assertTrue(fleetdService.contains(directive), + FLEETD_SERVICE + " no longer mentions " + directive + " anywhere, not even in the " + + "DO-NOT-add comment block that explains why it must stay out. That comment " + + "is the whole point of fleetd #360 -- it is what stops the next edit from " + + "re-adding the directive without knowing why."); + } + } +}