chore: 1.0.2（enroll/install 的接缝修复）

四处"校验在错误的层"的缺陷，都属于同一类：每个组件单独看都对，接缝错了。

1. `msa-agent enroll --relay http://relay.example` / `relay.example` /
   `https://relay.example/v1` 都会写出配置文件，install 报成功，守护进程在**下一次启动**才
   拒绝，日志里只有一行 "msa-agent exiting"。规则原本散在三处（config 只查非空、transport
   查真实规则、enroll 的注册客户端另有一份），现在收进 `internal/relayurl` 一份，三处都调它。
   `config.File.Validate` 因此也补上了 https/host/无路径/无 query 的检查。

2. `enroll` 不再自己手写字段检查，改为构造真正的 `config.File` 并调用真正的 `Validate`——
   一份"什么是可用配置"的定义，结构上无法再漂移。`config.Marshal` 让 enroll 与 Save 用同一个
   编码器。

3. `install --config /srv/msa/config.json` 会给该文件改属主、交给服务用户，然后写一个
   ExecStart 指向 `/etc/msa-agent/config.json` 的单元——服务起来找不到配置就退出，而它需要的
   文件就在别处、属主正确、从未被读。`enroll --audit /srv/audit/audit.log` 同理：单元的
   `ReadWritePaths` 只覆盖 `/var/log/msa-agent`，在 `ProtectSystem=strict` 下 agent 的
   `MkdirAll` 报 EROFS，进程启动即退出，报错既不提那个 flag 也不提那条指令。现在单元是模板，
   `RenderedUnit` 按实际布局渲染，`--print-unit` 打印的就是会被写入的那一份；默认布局渲染结果
   与仓库里被 review 过的文件逐字节相同，所以 vendored 副本的漂移检查仍然有意义。

测试：新增 `internal/relayurl`，`config`/`enroll`/`install` 各补回归测试。每条都做了变异验证
（把修复回退，对应测试必须失败）。

Makefile

@@ -13,7 +13,7 @@ UNIT_DST   := internal/install/msa-agent.service
 # The version stamped into the binary, and the tag scripts/release.sh publishes under. It is the
 # agent's own version rather than the app's: the two are released together but a node can be running an
 # older agent than the phone, and the handshake's `agentBuild` field is what makes that visible.
-VERSION    ?= 1.0.0
+VERSION    ?= 1.0.2
 LDFLAGS    := -s -w -X main.version=$(VERSION)
 
 .PHONY: all vendor test vet fmt vectors build build-arm64 build-armv7 clean

README.md

@@ -39,6 +39,20 @@ relay's `agents.json` by hand. Without it, `enroll` prints the entry to add.
 Removing it again is `sudo msa-agent uninstall`; `--dry-run` shows the plan first, and `--keep-logs`
 keeps the audit trail.
 
+### After installing, pair
+
+The daemon refuses to start with no paired devices, on purpose: an agent that could authorise nothing
+must not sit there looking healthy. Pairing happens after install, so a freshly installed node's service
+will fail and stop — the unit's start limit makes systemd give up rather than restart every five seconds
+forever, and `systemctl status msa-agent` shows it as failed rather than flapping.
+
+```sh
+sudo /opt/msa-agent/msa-agent --pair     # opens the LAN window; then pair from the app
+```
+
+Then `systemctl start msa-agent` (or just wait — it is enabled) and check `journalctl -u msa-agent -n 20`
+for the policy digest and the device count.
+
 ## Subcommands
 
 | Command | What it does |
@@ -49,6 +63,7 @@ keeps the audit trail.
 | `uninstall` | Tells the relay first, then removes the service, unit, config, logs and user |
 | `version` | Prints the version stamped into the binary |
 | `--pair` | Opens the temporary LAN window a phone uses to pair; closes itself |
+| `--pair-bind` | The address that window binds to (an IP literal; the default is all interfaces) |
 | `--check` | Validates the config and policy, then exits |
 | `--once` | Runs a single poll cycle and exits |
 

cmd/msa-agent/main.go

@@ -485,10 +485,14 @@ func runInstallCommand(args []string) int {
 	if err := flags.Parse(args); err != nil {
 		return 2
 	}
+	paths := install.DefaultPaths()
+	paths.ConfigPath = *configPath
+	opts.Paths = paths
 	if *printUnit {
-		// The unit is embedded, so this is how an operator reads it before it is installed — which is the
-		// point of embedding rather than fetching: the file that will be installed is readable first.
-		if err := install.PrintUnit(os.Stdout); err != nil {
+		// The unit is embedded and rendered for this layout, so this is how an operator reads what will be
+		// installed — which is the point of embedding rather than fetching: the file that will be installed
+		// is readable first, including the parts that follow --config.
+		if err := install.PrintUnit(os.Stdout, paths); err != nil {
 			fmt.Fprintln(os.Stderr, "msa-agent install:", err)
 			return 1
 		}
@@ -500,9 +504,6 @@ func runInstallCommand(args []string) int {
 			return 1
 		}
 	}
-	paths := install.DefaultPaths()
-	paths.ConfigPath = *configPath
-	opts.Paths = paths
 	opts.Log = func(format string, args ...any) { fmt.Printf(format+"\n", args...) }
 	if err := install.Install(opts); err != nil {
 		fmt.Fprintln(os.Stderr, "msa-agent install:", err)

internal/config/config.go

@@ -15,6 +15,8 @@ import (
 	"os"
 	"regexp"
 	"syscall"
+
+	"fun.chick/msa-agent/internal/relayurl"
 )
 
 var nodePattern = regexp.MustCompile(`^nd_[A-Za-z0-9_-]{8,64}$`)
@@ -84,9 +86,16 @@ func (c File) Validate(pairMode bool) error {
 	if !nodePattern.MatchString(c.NodeID) {
 		return errors.New("nodeId must look like nd_xxxxxxxx")
 	}
+	// The URL is checked for shape here, not merely for being present. It used to be checked for
+	// presence alone, and the transport refused the real rule at dial time — so `enroll` would write a
+	// config, the daemon would refuse it at the next startup, and the operator would find out after
+	// the install had already reported success.
 	if c.RelayURL == "" {
 		return errors.New("relayUrl is required")
 	}
+	if err := relayurl.Validate(c.RelayURL); err != nil {
+		return err
+	}
 	if c.AgentToken == "" {
 		return errors.New("agentToken is required")
 	}
@@ -159,6 +168,13 @@ func loadFile(path string) (File, error) {
 	return cfg, nil
 }
 
+// Marshal renders a File the way Save writes it, so a caller that needs the bytes without writing them
+// — `enroll`, which validates before it writes — gets the same encoding rather than its own
+// json.MarshalIndent that could drift from this one.
+func Marshal(cfg File) ([]byte, error) {
+	return json.MarshalIndent(cfg, "", "  ")
+}
+
 // Save writes the config back with 0600, atomically, preserving the file's owner.
 //
 // Atomic (temp file + rename) because the pairing flow writes the device list at moments when the
@@ -176,7 +192,7 @@ func Save(path string, cfg File) error {
 	if err := cfg.Validate(true); err != nil {
 		return fmt.Errorf("config: refusing to save an invalid file: %w", err)
 	}
-	encoded, err := json.MarshalIndent(cfg, "", "  ")
+	encoded, err := Marshal(cfg)
 	if err != nil {
 		return err
 	}

internal/config/config_test.go

@@ -123,6 +123,33 @@ func TestSaveRefusesAnInvalidFile(t *testing.T) {
 	}
 }
 
+// The relay URL used to be checked only for being non-empty, so a shape the transport refuses was
+// accepted here and refused later — at dial time in the daemon, or at the next startup for a config
+// `enroll` had already written and reported as success. These are the values that reached real nodes.
+func TestValidateRefusesARelayURLTheDaemonCannotDial(t *testing.T) {
+	for _, raw := range []string{
+		"relay.example",
+		"http://relay.example",
+		"https://relay.example/v1",
+		"https://relay.example?x=1",
+		"https://",
+	} {
+		cfg := valid()
+		cfg.RelayURL = raw
+		if err := cfg.Validate(false); err == nil {
+			t.Errorf("accepted relayUrl %q", raw)
+		}
+		if err := cfg.Validate(true); err == nil {
+			t.Errorf("pair mode accepted relayUrl %q", raw)
+		}
+	}
+	cfg := valid()
+	cfg.RelayURL = "https://relay.example/"
+	if err := cfg.Validate(false); err != nil {
+		t.Fatalf("refused a trailing slash: %v", err)
+	}
+}
+
 func TestValidateNamesEveryMissingField(t *testing.T) {
 	cases := map[string]func(*File){
 		"nodeId":    func(c *File) { c.NodeID = "" },

internal/enroll/enroll.go

@@ -20,6 +20,7 @@ import (
 	"encoding/json"
 	"fmt"
 
+	"fun.chick/msa-agent/internal/config"
 	"fun.chick/msa-agent/internal/wire"
 )
 
@@ -69,17 +70,12 @@ type Registration struct {
 }
 
 // Generate mints the material and renders both artefacts.
+//
+// Every input rule is enforced by config.File.Validate below rather than restated here. The three
+// hand-written checks that used to stand here — relay URL non-empty, port in range, audit path present
+// — were a second, weaker copy of rules the daemon already had, and the relay URL one was the weakest
+// of the three: it accepted anything non-empty, including values the transport refuses at dial time.
 func Generate(opts Options) (Result, error) {
-	if opts.RelayURL == "" {
-		return Result{}, fmt.Errorf("enroll: relay URL is required")
-	}
-	if opts.LocalPort <= 0 || opts.LocalPort > 65535 {
-		return Result{}, fmt.Errorf("enroll: local port %d is out of range", opts.LocalPort)
-	}
-	if opts.AuditPath == "" {
-		return Result{}, fmt.Errorf("enroll: audit path is required")
-	}
-
 	// The signing key is Ed25519: it answers the relay's handshake challenge and signs responses.
 	signingPublic, signingKey, err := ed25519.GenerateKey(rand.Reader)
 	if err != nil {
@@ -109,34 +105,43 @@ func Generate(opts Options) (Result, error) {
 		pairingSecret = randomHex(16)
 	}
 
-	config := map[string]any{
-		"nodeId":     nodeID,
-		"relayUrl":   opts.RelayURL,
-		"agentToken": token,
-		"localPort":  opts.LocalPort,
-		"signKey":    base64.StdEncoding.EncodeToString(signingKey),
-		"kxKey":      base64.StdEncoding.EncodeToString(kxKey),
-		"auditPath":  opts.AuditPath,
-		// Only used to authorise the pairing window on an unprotected node. Deleting it after pairing
-		// is fine; nothing else reads it.
-		"provisionSecret": pairingSecret,
+	// The config is built as the type the daemon loads, and validated by the daemon's own rules before it
+	// is written.
+	//
+	// This used to be a map[string]any with a handful of hand-written checks, and it produced configs the
+	// daemon then refused: `--relay relay.example` (no scheme), `--relay https://relay.example/v1` (a
+	// path), `--node node-1` (a malformed id) were all written happily, and the failure appeared later as
+	// "msa-agent exiting" on a node the operator had just installed. Two places that must agree about
+	// what a valid config is, with nothing checking the agreement — the same shape of defect as the
+	// install command that skipped the service user.
+	//
+	// Building the real type and calling the real Validate makes the agreement structural: there is one
+	// definition of a usable config and this cannot drift from it.
+	file := config.File{
+		NodeID:     nodeID,
+		RelayURL:   opts.RelayURL,
+		AgentToken: token,
+		LocalPort:  opts.LocalPort,
+		SignKey:    base64.StdEncoding.EncodeToString(signingKey),
+		KxKey:      base64.StdEncoding.EncodeToString(kxKey),
+		AuditPath:  opts.AuditPath,
+		// Only used to authorise the pairing window on an unprotected node. Deleting it after pairing is
+		// fine; nothing else reads it.
+		ProvisionSecret: pairingSecret,
+		NodePassword:    opts.NodePassword,
+		RelayCAFile:     opts.RelayCAFile,
+		SelfEnrolled:    opts.SelfEnrolled,
 		// Empty on purpose: devices are added by pairing (`msa-agent --pair`), which is the only path
 		// that should ever be able to introduce a trusted phone.
-		"devices": []any{},
+		Devices: []config.Device{},
 	}
-	// Absent rather than empty when unset, so a config that has not been told about a password does not
-	// claim the node has none.
-	if opts.NodePassword != "" {
-		config["nodePassword"] = opts.NodePassword
-	}
-	if opts.RelayCAFile != "" {
-		config["relayCAFile"] = opts.RelayCAFile
-	}
-	if opts.SelfEnrolled {
-		config["selfEnrolled"] = true
+	// pairMode relaxes exactly the empty-device rule, which is the state enrollment produces: the node
+	// has keys and no phone yet.
+	if err := file.Validate(true); err != nil {
+		return Result{}, fmt.Errorf("enroll: refusing to write a config the agent would not load: %w", err)
 	}
 
-	encoded, err := json.MarshalIndent(config, "", "  ")
+	encoded, err := config.Marshal(file)
 	if err != nil {
 		return Result{}, fmt.Errorf("enroll: encoding the config: %w", err)
 	}

internal/enroll/enroll_test.go

@@ -129,6 +129,25 @@ func TestTwoEnrollmentsDiffer(t *testing.T) {
 	}
 }
 
+// Enrollment is where a bad relay URL is cheapest to catch and where it used to be missed entirely.
+// It built a map[string]any with a handful of hand-written checks, so `--relay relay.example` and
+// `--relay https://relay.example/v1` both produced a config file, the install reported success, and
+// the daemon refused the file at the next startup. The values below are the ones that were measured
+// being written.
+func TestARelayURLTheDaemonWouldRefuseIsNeverWritten(t *testing.T) {
+	for _, raw := range []string{
+		"relay.example",
+		"http://relay.example",
+		"https://relay.example/v1",
+		"https://relay.example?x=1",
+		"https://",
+	} {
+		if _, err := Generate(Options{RelayURL: raw, LocalPort: 3000, AuditPath: "/a"}); err == nil {
+			t.Errorf("wrote a config for relay %q", raw)
+		}
+	}
+}
+
 func TestMissingInputIsRefused(t *testing.T) {
 	cases := map[string]Options{
 		"no relay":  {LocalPort: 3000, AuditPath: "/a"},

internal/enroll/register.go

@@ -9,10 +9,11 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"net/url"
 	"os"
 	"strings"
 	"time"
+
+	"fun.chick/msa-agent/internal/relayurl"
 )
 
 // Register announces this node to the relay, using a one-time code an operator generated in the app.
@@ -43,14 +44,11 @@ type RegisterOptions struct {
 // The failure vocabulary is deliberately small and each case names the next action, because this runs
 // during an install and the operator is looking at one line of output.
 func Register(ctx context.Context, opts RegisterOptions) error {
-	parsed, err := url.Parse(opts.RelayURL)
-	if err != nil {
-		return fmt.Errorf("enroll: relay url: %w", err)
-	}
-	if parsed.Scheme != "https" {
-		// https is not negotiable here for the same reason the agent refuses it: a registration sent in
-		// the clear can be replayed by anyone who sees it, and the code is single-use.
-		return fmt.Errorf("enroll: relay url must be https, got %q", parsed.Scheme)
+	// The same rule the daemon applies, from the same package. This check is not redundant with the
+	// config validation in Generate: registration happens *before* the config is written, so a URL this
+	// rejected late would already have been sent to a host the operator did not mean to reach.
+	if err := relayurl.Validate(opts.RelayURL); err != nil {
+		return fmt.Errorf("enroll: %w", err)
 	}
 	if strings.TrimSpace(opts.Code) == "" {
 		return fmt.Errorf("enroll: the enrollment code is empty")

internal/install/msa-agent.service

@@ -11,6 +11,17 @@ Restart=always
 # Backing off on our own is not enough: a relay that is down for an hour should not be hammered by
 # every node in the fleet the moment it comes back.
 RestartSec=5
+# A freshly installed node has no paired devices, and the daemon refuses to run in that state on
+# purpose: an agent that could authorise nothing must not sit there looking healthy. But pairing happens
+# *after* install — the operator runs `--pair` on the node and then pairs the phone — so between those
+# two moments this unit would restart every five seconds, forever, filling the journal with the same
+# line. Measured on a real kernel: exit 1, "devices is empty, so no phone could ever be authorised".
+#
+# StartLimitIntervalSec/StartLimitBurst make systemd give up instead: ten attempts inside five minutes
+# and the unit enters `failed`, which is a state an operator can see and act on. The install command
+# prints the next step precisely so that state is expected rather than surprising.
+StartLimitIntervalSec=300
+StartLimitBurst=10
 TimeoutStopSec=20
 KillSignal=SIGTERM
 

internal/install/service.go

@@ -14,10 +14,20 @@ package install
 
 import (
 	_ "embed"
+	"encoding/json"
 	"fmt"
 	"io"
 	"os"
 	"path/filepath"
+	"strings"
+)
+
+// The two literals in the embedded unit that have to follow the install's own paths. They are named
+// here rather than parsed out of the file so a unit edit that removes one is a visible failure in
+// RenderedUnit's tests rather than a silent no-op substitution.
+const (
+	defaultConfigPath    = "/etc/msa-agent/config.json"
+	defaultWritablePaths = "/var/log/msa-agent"
 )
 
 // The vendored unit. It is copied from agent/systemd/ by the Makefile, and
@@ -31,6 +41,72 @@ var embeddedUnit []byte
 // UnitFile returns the unit this binary installs, for tests and for `install --print-unit`.
 func UnitFile() []byte { return embeddedUnit }
 
+// RenderedUnit is the unit as it will actually be written for a given layout.
+//
+// The embedded file is a template, and two of its literals have to follow the paths the install was
+// given. Both were measured drifting:
+//
+//   - `install --config /srv/msa/config.json` chowned that file, gave it to the service user, and then
+//     wrote a unit whose ExecStart named `/etc/msa-agent/config.json` — a path that on that node holds
+//     nothing, so the service fails with "no such file or directory" while the config it needs sits
+//     elsewhere, correctly owned and never read.
+//   - `enroll --audit /srv/audit/audit.log` puts that path in the config, and the unit's
+//     `ReadWritePaths=/var/log/msa-agent` does not cover it. Under `ProtectSystem=strict` the agent's
+//     `os.MkdirAll` for the parent directory fails with EROFS and the daemon exits at startup, with a
+//     permission error that names neither the flag nor the directive.
+//
+// The default layout renders byte-for-byte the embedded file, which is what keeps the vendored copy
+// and the drift test meaningful: the template is still the thing that was reviewed.
+func RenderedUnit(paths Paths, writableDirs []string) []byte {
+	unit := string(embeddedUnit)
+	// Only substituted when it differs, so the common install produces exactly the reviewed file and a
+	// diff of `install --print-unit` against the repository copy stays empty.
+	if paths.ConfigPath != defaultConfigPath {
+		unit = strings.Replace(unit, "--config "+defaultConfigPath, "--config "+paths.ConfigPath, 1)
+	}
+	dirs := uniqueDirs(append([]string{paths.LogDir}, writableDirs...))
+	if len(dirs) > 0 && strings.Join(dirs, " ") != defaultWritablePaths {
+		unit = strings.Replace(unit, "ReadWritePaths="+defaultWritablePaths, "ReadWritePaths="+strings.Join(dirs, " "), 1)
+	}
+	return []byte(unit)
+}
+
+// uniqueDirs keeps the first occurrence of each path, in order, so a log directory that is also an
+// audit directory appears once and the rendering is stable.
+func uniqueDirs(dirs []string) []string {
+	seen := map[string]bool{}
+	out := make([]string, 0, len(dirs))
+	for _, dir := range dirs {
+		dir = filepath.Clean(dir)
+		if dir == "" || dir == "." || dir == "/" || seen[dir] {
+			continue
+		}
+		seen[dir] = true
+		out = append(out, dir)
+	}
+	return out
+}
+
+// auditDirOf reports the directory the config's audit log needs to be writable, or "" when the config
+// cannot be read.
+//
+// An unreadable config is not an error here: `install --dry-run` is run on nodes where `enroll` has not
+// happened yet, and the real run refuses a missing config a few lines above. What it must not do is
+// invent a path — a wrong ReadWritePaths entry is a sandbox hole, and a missing one is the EROFS above.
+func auditDirOf(configPath string) string {
+	raw, err := os.ReadFile(configPath)
+	if err != nil {
+		return ""
+	}
+	var parsed struct {
+		AuditPath string `json:"auditPath"`
+	}
+	if err := json.Unmarshal(raw, &parsed); err != nil {
+		return ""
+	}
+	return filepath.Dir(parsed.AuditPath)
+}
+
 // InstallOptions configures an install.
 type InstallOptions struct {
 	Paths Paths
@@ -114,12 +190,16 @@ func Install(opts InstallOptions) error {
 		}
 	}
 
-	// 4. The unit, from this binary.
+	// 4. The unit, from this binary. The audit log's directory comes from the config that `enroll` wrote,
+	//    because the unit's ReadWritePaths is the only thing that makes it writable under
+	//    ProtectSystem=strict — and a config pointing outside it is a daemon that exits at startup with
+	//    EROFS, naming neither the flag nor the directive.
 	if stepWith(opts.DryRun, opts.Log, "writing %s", paths.UnitPath) {
 		if err := os.MkdirAll(filepath.Dir(paths.UnitPath), 0o755); err != nil {
 			return fmt.Errorf("install: creating the unit directory: %w", err)
 		}
-		if err := os.WriteFile(paths.UnitPath, embeddedUnit, 0o644); err != nil {
+		unit := RenderedUnit(paths, []string{auditDirOf(paths.ConfigPath)})
+		if err := os.WriteFile(paths.UnitPath, unit, 0o644); err != nil {
 			return fmt.Errorf("install: writing the unit: %w", err)
 		}
 	}
@@ -141,9 +221,13 @@ func Install(opts InstallOptions) error {
 		opts.Log("dry run: nothing was changed")
 		return nil
 	}
-	opts.Log("msa-agent installed and running")
-	opts.Log("check it with: journalctl -u %s -n 20", paths.ServiceName)
-	opts.Log("next: run `msa-agent --pair` once, then pair the phone from the app")
+	// The service will fail until a phone is paired, and that is expected rather than a fault: the daemon
+	// refuses an empty device list so it cannot sit there looking healthy while authorising nothing. The
+	// unit's start limit turns that into a `failed` state instead of a restart loop, and this line is
+	// what makes the state expected.
+	opts.Log("msa-agent installed and enabled")
+	opts.Log("next: run `msa-agent --pair` once and pair the phone; until then the service exits and stops")
+	opts.Log("check it with: systemctl status %s; journalctl -u %s -n 20", paths.ServiceName, paths.ServiceName)
 	return nil
 }
 
@@ -167,8 +251,13 @@ func chownTo(path, user string, opts InstallOptions) error {
 	return nil
 }
 
-// PrintUnit writes the embedded unit to w, so an operator can read what will be installed before it is.
-func PrintUnit(w io.Writer) error {
-	_, err := w.Write(embeddedUnit)
+// PrintUnit writes the unit as it would be installed for a layout to w, so an operator can read what
+// will be installed before it is.
+//
+// It renders rather than dumping the embedded bytes, because the two are not the same thing once the
+// install has been given a non-default config path or an audit log outside the log directory — and the
+// command's whole promise is that what it prints is what gets written.
+func PrintUnit(w io.Writer, paths Paths) error {
+	_, err := w.Write(RenderedUnit(paths, []string{auditDirOf(paths.ConfigPath)}))
 	return err
 }

internal/install/uninstall_test.go

@@ -426,3 +426,51 @@ func TestTheRemovalHintIsTheOneItWasGiven(t *testing.T) {
 		t.Fatalf("the uninstall still claims every node is in agents.json:\n%s", output)
 	}
 }
+
+// The install ends by starting a service that cannot run yet, and the unit has to say what to do about
+// that. A freshly installed node has no paired devices — pairing happens after install — and the daemon
+// refuses to run in that state on purpose, because an agent that could authorise nothing must not look
+// healthy. With `Restart=always` and `RestartSec=5` and no start limit, that is a restart every five
+// seconds until a phone pairs.
+//
+// Found by rehearsing the install on a real kernel: exit 1, "devices is empty, so no phone could ever be
+// authorised", and the journal filling with the same line.
+func TestTheUnitGivesUpInsteadOfRestartingForever(t *testing.T) {
+	unit := string(UnitFile())
+	if !strings.Contains(unit, "Restart=always") {
+		t.Skip("the unit no longer restarts, so there is nothing to bound")
+	}
+	if !strings.Contains(unit, "StartLimitIntervalSec=") || !strings.Contains(unit, "StartLimitBurst=") {
+		t.Fatal("the unit restarts without a start limit: an unpaired node would restart every RestartSec forever")
+	}
+	// And the install tells the operator what to do next, so the failed state is expected rather than
+	// surprising.
+	var lines []string
+	root := t.TempDir()
+	paths := Paths{
+		ServiceName: "msa-agent",
+		UnitPath:    filepath.Join(root, "etc/systemd/system/msa-agent.service"),
+		ConfigPath:  filepath.Join(root, "etc/msa-agent/config.json"),
+		LogDir:      filepath.Join(root, "var/log/msa-agent"),
+		BinaryPath:  filepath.Join(root, "opt/msa-agent/msa-agent"),
+		User:        "msa-agent",
+	}
+	for _, path := range []string{paths.BinaryPath, paths.ConfigPath} {
+		if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(path, []byte("x"), 0o600); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if err := Install(InstallOptions{
+		Paths:  paths,
+		Runner: &recorder{failOn: "getent"},
+		Log:    func(format string, args ...any) { lines = append(lines, fmt.Sprintf(format, args...)) },
+	}); err != nil {
+		t.Fatalf("Install: %v", err)
+	}
+	if !strings.Contains(strings.Join(lines, "\n"), "--pair") {
+		t.Fatalf("the install does not say to pair next:\n%s", strings.Join(lines, "\n"))
+	}
+}

internal/install/unitrender_test.go

@@ -0,0 +1,158 @@
+package install
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"fun.chick/msa-agent/internal/config"
+)
+
+// The embedded unit is a template, and the install's own paths have to reach it.
+//
+// Two literals were measured drifting, and both fail on the node rather than here:
+//
+//  1. `install --config /srv/msa/config.json` chowned that file and gave it to the service user, then
+//     wrote a unit whose ExecStart named the default path. The service starts, cannot find its config,
+//     and exits — with the file it needs sitting elsewhere, correctly owned and never read.
+//  2. `enroll --audit /srv/audit/audit.log` writes that path into the config, and the unit's
+//     ReadWritePaths covered only /var/log/msa-agent. Under ProtectSystem=strict the agent's MkdirAll
+//     for the parent directory fails with EROFS and the daemon exits at startup, naming neither the
+//     flag nor the directive.
+//
+// The rendering is deliberately identity-preserving for the default layout: the file a reviewer read
+// is still byte-for-byte what a default install writes, so the vendored-copy drift test keeps its
+// meaning.
+func TestRenderedUnitFollowsTheLayoutItWasGiven(t *testing.T) {
+	// (a) The default layout renders exactly the embedded file.
+	if got := string(RenderedUnit(DefaultPaths(), nil)); got != string(UnitFile()) {
+		t.Fatal("the default layout does not render the reviewed unit file")
+	}
+
+	// (b) A config path elsewhere reaches ExecStart.
+	custom := DefaultPaths()
+	custom.ConfigPath = "/srv/msa/config.json"
+	unit := string(RenderedUnit(custom, nil))
+	if !strings.Contains(unit, "ExecStart=/opt/msa-agent/msa-agent --config /srv/msa/config.json") {
+		t.Fatalf("ExecStart does not name the config the install was given:\n%s", execLines(unit))
+	}
+	if strings.Contains(unit, defaultConfigPath) {
+		t.Fatalf("the default config path is still in the unit:\n%s", execLines(unit))
+	}
+
+	// (c) An audit log outside the log directory becomes writable, and the default entry stays.
+	unit = string(RenderedUnit(DefaultPaths(), []string{"/srv/audit"}))
+	if !strings.Contains(unit, "ReadWritePaths=/var/log/msa-agent /srv/audit") {
+		t.Fatalf("the audit directory is not writable:\n%s", readWriteLines(unit))
+	}
+	// (d) A duplicate — an audit log that already lives in the log directory — is collapsed, so the
+	//     rendered file does not accumulate the same path on every re-install.
+	unit = string(RenderedUnit(DefaultPaths(), []string{"/var/log/msa-agent", "/var/log/msa-agent/"}))
+	if !strings.Contains(unit, "ReadWritePaths=/var/log/msa-agent\n") {
+		t.Fatalf("a duplicate writable path was rendered:\n%s", readWriteLines(unit))
+	}
+}
+
+// The install reads the audit path out of the config `enroll` wrote, so the two agree without anyone
+// passing the flag twice. An unreadable config yields nothing rather than a guess: a wrong
+// ReadWritePaths entry is a sandbox hole, and a missing one is the EROFS above.
+func TestTheAuditDirectoryComesFromTheConfig(t *testing.T) {
+	directory := t.TempDir()
+	path := filepath.Join(directory, "config.json")
+	cfg := config.File{
+		NodeID: "nd_7f3a9c2e", RelayURL: "https://relay.example", AgentToken: "tok",
+		LocalPort: 3000, SignKey: "AAAA", KxKey: "AAAA",
+		AuditPath: "/srv/audit/audit.log", Devices: []config.Device{},
+	}
+	if err := config.Save(path, cfg); err != nil {
+		t.Fatal(err)
+	}
+	if got := auditDirOf(path); got != "/srv/audit" {
+		t.Fatalf("auditDirOf = %q, want /srv/audit", got)
+	}
+	if got := auditDirOf(filepath.Join(directory, "absent.json")); got != "" {
+		t.Fatalf("auditDirOf invented %q for a config that is not there", got)
+	}
+	if err := os.WriteFile(path, []byte("not json"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if got := auditDirOf(path); got != "" {
+		t.Fatalf("auditDirOf invented %q for an unparseable config", got)
+	}
+}
+
+// And the whole install writes the rendered unit, not the embedded bytes. This is the assertion that
+// would fail if the two ever came apart again.
+func TestInstallWritesAUnitForItsOwnPaths(t *testing.T) {
+	root := t.TempDir()
+	paths := Paths{
+		ServiceName: "msa-agent",
+		UnitPath:    filepath.Join(root, "etc/systemd/system/msa-agent.service"),
+		ConfigPath:  filepath.Join(root, "srv/msa/config.json"),
+		LogDir:      filepath.Join(root, "var/log/msa-agent"),
+		BinaryPath:  filepath.Join(root, "opt/msa-agent/msa-agent"),
+		User:        "msa-agent",
+	}
+	for _, path := range []string{paths.BinaryPath, paths.ConfigPath} {
+		if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if err := os.WriteFile(paths.BinaryPath, []byte("binary"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	cfg := config.File{
+		NodeID: "nd_7f3a9c2e", RelayURL: "https://relay.example", AgentToken: "tok",
+		LocalPort: 3000, SignKey: "AAAA", KxKey: "AAAA",
+		AuditPath: filepath.Join(root, "srv/audit/audit.log"), Devices: []config.Device{},
+	}
+	if err := config.Save(paths.ConfigPath, cfg); err != nil {
+		t.Fatal(err)
+	}
+
+	runner := &recorder{failOn: "getent"}
+	if err := Install(InstallOptions{Paths: paths, Runner: runner, Log: func(string, ...any) {}}); err != nil {
+		t.Fatalf("Install: %v", err)
+	}
+	written, err := os.ReadFile(paths.UnitPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	unit := string(written)
+	if !strings.Contains(unit, "--config "+paths.ConfigPath) {
+		t.Fatalf("the installed unit does not name the config the install was given:\n%s", execLines(unit))
+	}
+	if !strings.Contains(unit, "ReadWritePaths="+paths.LogDir+" "+filepath.Join(root, "srv/audit")) {
+		t.Fatalf("the installed unit does not make the audit log writable:\n%s", readWriteLines(unit))
+	}
+}
+
+// PrintUnit is the operator's preview of the install, so it has to be the same rendering the install
+// writes. Dumping the embedded bytes would show a default-layout unit to someone installing with
+// --config, which is a preview of a different file.
+func TestPrintUnitShowsWhatTheInstallWouldWrite(t *testing.T) {
+	paths := DefaultPaths()
+	paths.ConfigPath = "/srv/msa/config.json"
+	var printed strings.Builder
+	if err := PrintUnit(&printed, paths); err != nil {
+		t.Fatal(err)
+	}
+	if printed.String() != string(RenderedUnit(paths, []string{auditDirOf(paths.ConfigPath)})) {
+		t.Fatal("--print-unit and the install would write different files")
+	}
+}
+
+func execLines(unit string) string { return linesStartingWith(unit, "ExecStart") }
+
+func readWriteLines(unit string) string { return linesStartingWith(unit, "ReadWritePaths") }
+
+func linesStartingWith(unit, prefix string) string {
+	var out []string
+	for _, line := range strings.Split(unit, "\n") {
+		if strings.HasPrefix(line, prefix) {
+			out = append(out, line)
+		}
+	}
+	return strings.Join(out, "\n")
+}

internal/relayurl/relayurl.go

@@ -0,0 +1,56 @@
+// Package relayurl holds the one definition of an acceptable relay URL.
+//
+// Three components need to agree on it — the config validator that decides whether the daemon starts
+// at all, the transport that dials the relay, and the enrollment client that registers a node before
+// either of them runs — and they did not. `msa-agent enroll --relay relay.example` (no scheme) and
+// `--relay https://relay.example/v1` (a path) both wrote a config happily; the daemon then refused it
+// at startup, on a node the operator had just finished installing, minutes later and in a different
+// process. The error arrived as "msa-agent exiting" in the journal.
+//
+// The rule is short enough to state once and nowhere else: https, a host, and nothing after it.
+package relayurl
+
+import (
+	"errors"
+	"fmt"
+	"net/url"
+)
+
+// Validate reports whether raw can be used as a relay base URL.
+//
+// The path rule is not cosmetic. The transport builds every request URL by concatenation
+// (baseURL + "/v1/hello"), so a base URL carrying a query would produce
+// `https://relay.example?x=1/v1/hello` — an empty path and a query of "/v1/hello". A base URL with a
+// path would silently address endpoints under it. Both fail somewhere far from the typo.
+func Validate(raw string) error {
+	if raw == "" {
+		// Named separately because the empty string is what a flag that was never given a value looks
+		// like, and "must be https, got """ is a worse thing to read at 2am than "is required".
+		return errors.New("relay url is required")
+	}
+	parsed, err := url.Parse(raw)
+	if err != nil {
+		return fmt.Errorf("relay url: %w", err)
+	}
+	if parsed.Scheme != "https" {
+		// Not negotiable. The relay is untrusted by design, so the channel's job is to stop a third
+		// party from substituting a different ciphertext; over an unauthenticated channel the
+		// end-to-end encryption above it is decorative.
+		return fmt.Errorf("relay url must be https, got %q", parsed.Scheme)
+	}
+	if parsed.Host == "" {
+		return errors.New("relay url has no host")
+	}
+	if parsed.Path != "" && parsed.Path != "/" {
+		return fmt.Errorf("relay url must not carry a path, got %q", parsed.Path)
+	}
+	if parsed.RawQuery != "" || parsed.Fragment != "" {
+		return errors.New("relay url must not carry a query or a fragment")
+	}
+	if parsed.User != nil {
+		// Credentials in the URL would be sent on every request and are not how this authenticates:
+		// the agent signs a challenge with its own key, and a bearer in the URL would end up in logs.
+		return errors.New("relay url must not carry credentials")
+	}
+	return nil
+}

internal/relayurl/relayurl_test.go

@@ -0,0 +1,37 @@
+package relayurl
+
+import "testing"
+
+// The list is the union of every malformed value that reached a real node: the two that were written
+// into a config and refused only at the next startup, and the ones the transport already refused.
+func TestValidateRefusesWhatTheDaemonCannotUse(t *testing.T) {
+	for _, raw := range []string{
+		"",                         // the flag was never given a value
+		"relay.example",            // no scheme: url.Parse reads this as a path
+		"http://relay.example",     // plaintext, so the ciphertext above it is decorative
+		"ws://relay.example",       // a scheme the transport cannot dial
+		"https://relay.example/v1", // a path the request builder would silently append to
+		"https://relay.example?x=1",
+		"https://relay.example#f",
+		"https://user:pw@relay.example",
+		"https://",  // no host
+		"not a url", // a space is not a valid URL byte
+	} {
+		if err := Validate(raw); err == nil {
+			t.Fatalf("accepted %q", raw)
+		}
+	}
+}
+
+func TestValidateAcceptsTheFormsAnOperatorActuallyTypes(t *testing.T) {
+	for _, raw := range []string{
+		"https://relay.example",
+		"https://relay.example/", // a trailing slash is trimmed by the transport, not an error
+		"https://relay.example:8443",
+		"https://203.0.113.9",
+	} {
+		if err := Validate(raw); err != nil {
+			t.Fatalf("refused %q: %v", raw, err)
+		}
+	}
+}

internal/transport/transport.go

@@ -21,10 +21,11 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"net/url"
 	"strconv"
 	"strings"
 	"time"
+
+	"fun.chick/msa-agent/internal/relayurl"
 )
 
 // Paths on the relay. Versioned, because an agent and a relay will not be upgraded on the same day.
@@ -135,18 +136,11 @@ type Config struct {
 // stop a third party from substituting a different ciphertext; an unauthenticated channel would make
 // the endpoint-to-endpoint encryption above it decorative.
 func New(cfg Config) (*Client, error) {
-	parsed, err := url.Parse(cfg.BaseURL)
-	if err != nil {
-		return nil, fmt.Errorf("transport: relay url: %w", err)
-	}
-	if parsed.Scheme != "https" {
-		return nil, fmt.Errorf("transport: relay url must be https, got %q", parsed.Scheme)
-	}
-	if parsed.Host == "" {
-		return nil, errors.New("transport: relay url has no host")
-	}
-	if parsed.Path != "" && parsed.Path != "/" {
-		return nil, fmt.Errorf("transport: relay url must not carry a path, got %q", parsed.Path)
+	// The rule lives in internal/relayurl and is applied here, in config.File.Validate and in the
+	// enrollment client. Three copies of it is how `enroll` came to write configs this constructor
+	// refused.
+	if err := relayurl.Validate(cfg.BaseURL); err != nil {
+		return nil, fmt.Errorf("transport: %w", err)
 	}
 	if len(cfg.Signer) != ed25519.PrivateKeySize {
 		return nil, errors.New("transport: agent signing key is the wrong size")

scripts/release.sh

@@ -0,0 +1,161 @@
+#!/usr/bin/env bash
+# Publishes this agent's cross-compiled binaries as a Gitea release.
+#
+# The release is what makes the install command real: `msa-agent enroll` runs on the node, and the node
+# has no Go toolchain, so the binary has to come from somewhere. Before this the only way to get one was
+# to build it on another machine and scp it, which is a step the install command cannot express.
+#
+# The token is read from the git credential helper rather than an environment variable, so it is not in
+# the process table and not in a shell history. Override with MSA_GITEA_TOKEN when running somewhere
+# that has no helper.
+#
+# Usage:
+#   ./scripts/release.sh                    # publish the version in the Makefile
+#   ./scripts/release.sh --tag v1.0.0       # publish an explicit tag
+#   ./scripts/release.sh --dry-run          # build, checksum, print, upload nothing
+#
+# This repository must stay publicly readable: the install command fetches the binary from a node that
+# has no credentials, and Gitea answers 404 for a private repository, so the failure on the node looks
+# like a wrong URL. The script checks that and refuses rather than publishing something unreachable.
+set -euo pipefail
+
+ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+cd "$ROOT"
+
+REMOTE="$(git config --get remote.origin.url)"
+HOST="$(printf '%s' "$REMOTE" | sed -E 's#^https?://([^/]+)/.*#\1#')"
+SLUG="$(printf '%s' "$REMOTE" | sed -E 's#^https?://[^/]+/##; s#\.git$##')"
+if [[ -z "$HOST" || -z "$SLUG" ]]; then
+  echo "release: cannot parse the origin remote ($REMOTE)" >&2
+  exit 1
+fi
+
+VERSION="$(sed -n 's/^VERSION[[:space:]]*?*=[[:space:]]*//p' Makefile | head -1)"
+TAG="v${VERSION}"
+DRY_RUN=0
+while [[ $# -gt 0 ]]; do
+  case "$1" in
+    --tag) TAG="$2"; shift 2 ;;
+    --dry-run) DRY_RUN=1; shift ;;
+    *) echo "release: unknown argument $1" >&2; exit 2 ;;
+  esac
+done
+
+step() { printf '\n\033[1m== %s\033[0m\n' "$1"; }
+
+step "building ${TAG}"
+# VERSION is passed through so the binary reports the tag it was published under, which is what an
+# operator sees in `msa-agent version` and in the agent's startup log line.
+make build-arm64 build-armv7 VERSION="${VERSION}"
+
+step "checksums"
+(
+  cd dist
+  shasum -a 256 msa-agent-linux-arm64 msa-agent-linux-armv7 > SHA256SUMS
+  cat SHA256SUMS
+)
+
+if [[ "$DRY_RUN" -eq 1 ]]; then
+  step "dry run: nothing was uploaded"
+  exit 0
+fi
+
+TOKEN="${MSA_GITEA_TOKEN:-}"
+if [[ -z "$TOKEN" ]]; then
+  # The credential helper is what `git push` already uses, so there is no second secret to keep.
+  TOKEN="$(printf 'protocol=https\nhost=%s\n\n' "$HOST" | git credential fill 2>/dev/null | sed -n 's/^password=//p')"
+fi
+if [[ -z "$TOKEN" ]]; then
+  echo "release: no API token. Set MSA_GITEA_TOKEN, or make sure git can authenticate to $HOST." >&2
+  exit 1
+fi
+
+API="https://${HOST}/api/v1/repos/${SLUG}"
+AUTH=(-H "Authorization: token ${TOKEN}")
+
+step "target repository"
+PRIVATE="$(curl -sf "${AUTH[@]}" "${API}" | python3 -c 'import json,sys; print(json.load(sys.stdin).get("private"))' 2>/dev/null || echo unknown)"
+echo "${SLUG} (private: ${PRIVATE})"
+if [[ "$PRIVATE" != "False" ]]; then
+  echo "release: ${SLUG} is not publicly readable. A node fetching the binary has no credentials and" >&2
+  echo "         would get 404, which looks like a wrong URL. Make it public before publishing." >&2
+  exit 1
+fi
+
+step "creating the release ${TAG}"
+# A release that already exists is reused rather than refused: re-running this after a fix should
+# replace the binaries, and failing here would mean the operator has to delete the release by hand.
+EXISTING="$(curl -sf "${AUTH[@]}" "${API}/releases/tags/${TAG}" 2>/dev/null || true)"
+if [[ -n "$EXISTING" ]]; then
+  RELEASE_ID="$(printf '%s' "$EXISTING" | python3 -c 'import json,sys; print(json.load(sys.stdin)["id"])')"
+  echo "reusing release ${TAG} (id ${RELEASE_ID})"
+else
+  BODY="$(python3 - "$TAG" "$VERSION" <<'PY'
+import json, sys
+tag, version = sys.argv[1], sys.argv[2]
+print(json.dumps({
+    "tag_name": tag,
+    "name": f"simadmin-relay agent {version}",
+    "body": (
+        "Outbound-only node agent for SimAdmin nodes, cross-compiled for the ARM Debian boxes SimAdmin "
+        "runs on. Static, no runtime dependencies, no third-party modules.\n\n"
+        "Install on a node:\n\n"
+        "```sh\n"
+        f"curl -fsSL https://HOST/SLUG/releases/download/{tag}/msa-agent-linux-arm64 \\\n"
+        "  -o /opt/msa-agent/msa-agent && chmod 755 /opt/msa-agent/msa-agent\n"
+        "/opt/msa-agent/msa-agent enroll --relay https://your-relay --code <code> \\\n"
+        "  --write /etc/msa-agent/config.json\n"
+        "/opt/msa-agent/msa-agent install\n"
+        "```\n\n"
+        "`armv7` is the same build for 32-bit ARM. `SHA256SUMS` covers both.\n\n"
+        "Removing it again is `msa-agent uninstall` on the node; `--dry-run` shows the plan first.\n"
+    ),
+    "draft": False,
+    "prerelease": False,
+}, ensure_ascii=False))
+PY
+)"
+  BODY="${BODY//HOST/$HOST}"
+  BODY="${BODY//SLUG/$SLUG}"
+  RELEASE_ID="$(curl -sf -X POST "${AUTH[@]}" -H 'Content-Type: application/json' \
+    "${API}/releases" -d "$BODY" | python3 -c 'import json,sys; print(json.load(sys.stdin)["id"])')"
+  echo "created release ${TAG} (id ${RELEASE_ID})"
+fi
+
+step "uploading binaries"
+for asset in msa-agent-linux-arm64 msa-agent-linux-armv7 SHA256SUMS; do
+  # An asset with the same name cannot be replaced, so an existing one is deleted first: otherwise a
+  # re-run after a rebuild would silently leave the previous binary in place.
+  EXISTING_ASSET="$(curl -sf "${AUTH[@]}" "${API}/releases/${RELEASE_ID}/assets" \
+    | python3 -c "
+import json, sys
+name = sys.argv[1]
+for asset in json.load(sys.stdin):
+    if asset['name'] == name:
+        print(asset['id'])
+        break
+" "$asset" || true)"
+  if [[ -n "$EXISTING_ASSET" ]]; then
+    # Gitea's delete path carries the release id; GitHub's does not. Using GitHub's shape here returns
+    # 404 and, under `set -e`, killed the script silently after the first asset — which is how a
+    # re-publish left the old binary in place while looking like it had replaced it.
+    curl -sf -X DELETE "${AUTH[@]}" "${API}/releases/${RELEASE_ID}/assets/${EXISTING_ASSET}" >/dev/null
+    echo "  replaced ${asset}"
+  fi
+  curl -sf -X POST "${AUTH[@]}" \
+    -F "attachment=@dist/${asset}" \
+    "${API}/releases/${RELEASE_ID}/assets?name=${asset}" >/dev/null
+  echo "  uploaded ${asset}"
+done
+
+step "published"
+printf 'release:  https://%s/%s/releases/tag/%s\n' "$HOST" "$SLUG" "$TAG"
+printf 'download: https://%s/%s/releases/download/%s/msa-agent-linux-arm64\n' "$HOST" "$SLUG" "$TAG"
+printf '\n'
+# The app renders the install command, so the URL it carries has to be this one. Printing it in the
+# exact form DefaultContainer holds is what makes the two comparable by eye.
+printf 'DefaultContainer.agentDownload must be:\n'
+printf '  https://%s/%s/releases/download/%s/msa-agent-linux-arm64\n' "$HOST" "$SLUG" "$TAG"
+printf '\nVerify it is fetchable with no credentials:\n'
+printf '  curl -fsSL -o /dev/null -w "%%{http_code}\\n" %s\n' \
+  "https://${HOST}/${SLUG}/releases/download/${TAG}/msa-agent-linux-arm64"

systemd/msa-agent.service

@@ -11,6 +11,17 @@ Restart=always
 # Backing off on our own is not enough: a relay that is down for an hour should not be hammered by
 # every node in the fleet the moment it comes back.
 RestartSec=5
+# A freshly installed node has no paired devices, and the daemon refuses to run in that state on
+# purpose: an agent that could authorise nothing must not sit there looking healthy. But pairing happens
+# *after* install — the operator runs `--pair` on the node and then pairs the phone — so between those
+# two moments this unit would restart every five seconds, forever, filling the journal with the same
+# line. Measured on a real kernel: exit 1, "devices is empty, so no phone could ever be authorised".
+#
+# StartLimitIntervalSec/StartLimitBurst make systemd give up instead: ten attempts inside five minutes
+# and the unit enters `failed`, which is a state an operator can see and act on. The install command
+# prints the next step precisely so that state is expected rather than surprising.
+StartLimitIntervalSec=300
+StartLimitBurst=10
 TimeoutStopSec=20
 KillSignal=SIGTERM