phase: 17_installer_device_filter

**Phase 17 — final verification pass: complete, all green.** Verified (criteria): - `grep -n "DevicePolicy\|DeviceAllow" deploy/install-agent.sh` → no match; `PrivateDevices=false` retained (line 472) ✓ - `install-agent.sh --help` names `--as-root`/`--caps` with the kernel-gate sentence; `--force` documented ✓ - Fresh fake-root install = pre-phase unit minus the filter lines: `test_a_default_install_unit_is_the_pre_phase_unit_minus_the_filter_lines` + `test_the_template_unit_has_no_device_filter_lines` pass ✓ - Re-run that differs → exit 1 with diff on stdout, `--force` applies, identical re-run silent: `test_a_re_run_that_differs_refuses_without_force_and_applies_with_it` (+ 3 sibling guard pins) pass ✓ - driveripper reproduction — verified **read-only**, no live change: `systemctl cat` shows `User=root`, `CapabilityBoundingSet=CAP_SYS_RAWIO CAP_SYS_ADMIN`, no filter, same `ExecStart`; `active`, `CapEff 0000000000220000`; `/api/disks/snapshot` → 16/16 drives with temperatures ✓ - `uv run pytest` → **1451 passed, 15 skipped**; `bash .agents/validate.sh` → **exit 0** (coverage **98.94%** ≥ 94, `ruff` clean, `pyright` 0 errors, `node --check` + hygiene clean); `uv run pytest -m live --live` → **11 passed**; dev server 9/9 sources `ok` (one initial live failure was a transient gufo timeout, clean on re-run) Docs (inconsistencies found + fixed, incl. pre-existing): - `AGENTS.md` fleet record said driveripper's unit runs `User=dashboards-agent` while the same bullet (and the live host) says `User=root` — rewritten to separate the installed user from the running one. - `AGENTS.md` gap 10 header still claimed "the installer template is what is left" → now records phase 17 as the fix. - `README.md` "with no capabilities" → "with an empty `CapabilityBoundingSet` (unless `--caps`…)", since this phase added the capability flag. - `README.md` battery-test "gated four times" now notes the two session gates in front, matching AGENTS invariant 1. - `.env.example` was missing three README-documented settings (`DASH_ENV_FILE`, `DASH_RELOAD`, `NUT_SIMULATE`) and `AGENT_CERT_BUNDLE` in the agent-variables note — added, annotated; all template values re-checked against `config.py` defaults. Deviations: no live `--force` run on driveripper (needs the Oct-18 sudo-staged installer re-staged — it lacks the flags — and would push agent 1.1.0 mid phase-15 rollout); command + expected diff + post-checks recorded in AGENTS/gap 10. `uv pip list --outdated` still advisory (fastapi 0.153→0.156), untouched as in tasks 02/04 — this phase moved no dependency. Next pending phase: `.agents/phases/todo/18_nav_hamburger/`.
2026-10-20 13:48:40 -04:00
parent 7e367a9014
commit b02c13f7d3
30 changed files with 1502 additions and 81 deletions
@@ -12,6 +12,7 @@ apart from the path the script was invoked through.

 from __future__ import annotations

+import difflib
 import json
 import os
 import re
@@ -372,11 +373,22 @@ def test_install_path_installs_into_its_own_paths_and_prints_the_token_once(tmp_
    # the bundle unpacks into (the agent falls back to a private mkdtemp without one).
    assert "--cert-bundle" not in unit, "no bundle installed means no TLS flag on a token install"
    assert "RuntimeDirectory=dashboards-agent" not in unit
-    # DevicePolicy=strict denies every node not listed, and systemd opens /dev/null for stdin
-    # before it execs python: without this line the unit dies at spawn (status 208/STDIN).
-    assert "DevicePolicy=strict" in unit
-    assert "DeviceAllow=/dev/null" in unit, "strict device policy needs /dev/null or the unit cannot start"
-    assert "DeviceAllow=/dev/nvme*" in unit, "SMART reads need the disk nodes back"
+    # No device filter, on purpose (phase 17): a strict device policy denies every node the
+    # allow-list fails to match, and systemd matches those globs by *name* against
+    # /proc/devices — where kernel 7.2 lists NVMe as `blkext`, so the old `/dev/nvme*` entry
+    # locked the whole device namespace on driveripper. Device access is DAC (the `disk` group
+    # the installer grants) plus SELinux, the standard SMART-monitoring arrangement.
+    assert "DevicePolicy" not in unit, (
+        "a strict device policy with a name-matched allow-list is the deny-all device filter "
+        "that locked driveripper's SMART reads — the unit must carry none"
+    )
+    assert "DeviceAllow" not in unit, "no device filter means no allow-list either"
+    # PrivateDevices=false survives as the explicit statement of the systemd default: a
+    # private /dev with no device list dies at spawn (status 208/STDIN), because systemd
+    # opens /dev/null for the service stdin before it execs python.
+    assert "PrivateDevices=false" in unit, (
+        "the explicit default is what keeps a private-/dev sandbox from dying at STDIN"
+    )
    assert token and len(token) >= 32, "a generated token is expected"
    assert token in result.stdout, "first install prints the token, and never again"
    assert f'HOST_TARGETS="hostname=http://{token}@10.0.0.5:9120"' in result.stdout
@@ -387,6 +399,473 @@ def test_install_path_installs_into_its_own_paths_and_prints_the_token_once(tmp_
    assert "chown root:dashboards-agent" in calls, "the token file must become group-readable"


+#: A line the unit heredoc may legitimately write: a comment, a `[Section]` header, or a
+#: systemd directive assignment. Anything else is the unquoted heredoc having *expanded*
+#: something — see `test_the_written_unit_is_only_comments_and_directives`.
+_UNIT_LINE_RE = re.compile(r"^(?:#|\[[A-Za-z]+\]|[A-Za-z]+=)")
+
+
+@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="running as root")
+def test_the_written_unit_is_only_comments_and_directives(tmp_path: Path) -> None:
+    """The unit heredoc is unquoted so `$TLS_UNIT_LINES` and `$TLS_EXEC_ARGS` can expand.
+
+    That is also what executes its **comments**: writing the phase-17 rationale with backticks
+    around `disk`, `blkext` and `/dev/nvme*` made the install run them (`disk: command not
+    found`, smartctl's version banner) and paste the output into the unit file, with `bash -n`
+    and every text assertion still happy. Quote the heredoc and phase 15's interpolated lines
+    stop working, so the guard is on the *result*: a unit line is a comment, a section header,
+    or a directive assignment — never the output of something the prose mentioned.
+    """
+    script, _call_log, env = _sandbox_installer(tmp_path)
+    result = subprocess.run(
+        [BASH, str(script), "--bind", "127.0.0.1", "--port", "9199"],
+        capture_output=True,
+        text=True,
+        env=env,
+        timeout=60,
+        check=False,
+    )
+    assert result.returncode == 0, f"install failed:\n{result.stdout}\n{result.stderr}"
+    unit = (tmp_path / "target" / "dashboards-agent.service").read_text()
+    offenders = [line for line in unit.splitlines() if line.strip() and not _UNIT_LINE_RE.match(line)]
+    assert not offenders, "the unit heredoc expanded its own text into the unit file:\n" + "\n".join(
+        offenders
+    )
+    assert result.stderr == "", "installing must not execute anything the unit's comments mention"
+
+
+# --- the template's unit: no device filter, and the whole unit pinned (phase 17 task 04) ------
+
+#: The ten lines the pre-phase template wrote between ``PrivateDevices=false`` and
+#: ``ProtectSystem=strict`` — the device filter phase 17 removes. Pinned as the *old* text, in
+#: order, because the fix is the absence of exactly these lines: re-adding
+#: ``DeviceAllow=/dev/nvme* rw`` with ``DevicePolicy=strict`` is the deny-all returning, and a
+#: test that only asserted "`DevicePolicy` is absent today" would pass while somebody added
+#: the allow-list back under a default `auto` policy that ignores it.
+DEVICE_FILTER_LINES = (
+    "DevicePolicy=strict",
+    "DeviceAllow=/dev/null rw",
+    "DeviceAllow=/dev/zero rw",
+    "DeviceAllow=/dev/full rw",
+    "DeviceAllow=/dev/random rw",
+    "DeviceAllow=/dev/urandom rw",
+    "DeviceAllow=/dev/nvme* rw",
+    "DeviceAllow=/dev/sd* rw",
+    "DeviceAllow=/dev/vd* rw",
+    "DeviceAllow=/dev/sg* rw",
+)
+
+#: The unit the template wrote **before** phase 17, rendered for a default token install
+#: (bind `10.0.0.5`, port `9120`, no bundle), with the sandbox root spelled `<ROOT>`.
+#:
+#: This is the whole unit and not a highlight reel: it is the reference the default install is
+#: compared against, so a template line added, dropped, reordered or reworded *outside* this
+#: phase's remit fails a test instead of landing in the next host's service file. The comment
+#: block is included verbatim (it is what the operator reads in `systemctl cat`) even though
+#: phase 17 task 01 rewrote that prose on purpose — which is why the comparison below reads
+#: directives, and the new rationale is pinned separately for the two facts it must carry.
+PRE_PHASE_DEFAULT_UNIT = """[Unit]
+Description=Homelab host agent (read-only system metrics)
+After=network-online.target
+Wants=network-online.target
+
+[Service]
+Type=simple
+User=dashboards-agent
+ExecStart=/usr/bin/env python3 <ROOT>/target/lib/host_agent.py --host 10.0.0.5 --port 9120 --token-file <ROOT>/target/token
+Restart=on-failure
+RestartSec=5
+Environment=PYTHONUNBUFFERED=1
+SyslogIdentifier=dashboards-agent
+
+# The agent reads /proc, /sys and statvfs() and writes nothing, so everything writable can
+# be taken away. smartctl is the one thing that needs a real /dev node, so instead of
+# PrivateDevices the unit closes all devices and reopens only the disk nodes, read/write
+# at the cgroup layer while the code (and smartctl -i -A -H) stays read-only by design.
+#
+# The pseudo nodes are not decoration: DevicePolicy=strict denies every node that is not
+# listed, and systemd needs /dev/null for the service's stdin before it ever execs python —
+# without it the unit dies at spawn with "Failed at step STDIN" (status 208), which is how
+# the first phase-10 install failed on a live host running systemd 257. /dev/zero, full,
+# random and urandom are the rest of the set a PrivateDevices= sandbox would have provided,
+# and none of them is a disk.
+NoNewPrivileges=true
+PrivateTmp=true
+PrivateDevices=false
+DevicePolicy=strict
+DeviceAllow=/dev/null rw
+DeviceAllow=/dev/zero rw
+DeviceAllow=/dev/full rw
+DeviceAllow=/dev/random rw
+DeviceAllow=/dev/urandom rw
+DeviceAllow=/dev/nvme* rw
+DeviceAllow=/dev/sd* rw
+DeviceAllow=/dev/vd* rw
+DeviceAllow=/dev/sg* rw
+ProtectSystem=strict
+ProtectHome=read-only
+ProtectKernelTunables=true
+ProtectKernelModules=true
+ProtectControlGroups=true
+RestrictSUIDSGID=true
+RestrictNamespaces=true
+RestrictRealtime=true
+LockPersonality=true
+MemoryDenyWriteExecute=true
+CapabilityBoundingSet=
+SystemCallFilter=@system-service
+
+[Install]
+WantedBy=multi-user.target
+"""
+
+
+def _unit_heredoc() -> str:
+    """The template's unit heredoc, exactly as the script holds it.
+
+    Scoped on purpose: the rest of the script is prose about flags, and a pin that read the
+    whole file for `DeviceAllow` would be satisfied by deleting a sentence rather than by the
+    absence of a *unit line*.
+    """
+    text = INSTALLER.read_text()
+    assert "<<UNIT_FILE" in text, "the unit is still written from a heredoc"
+    heredoc = text.split("<<UNIT_FILE\n", 1)[1].split("\nUNIT_FILE\n", 1)[0]
+    assert "[Service]" in heredoc and "CapabilityBoundingSet" in heredoc, (
+        "the heredoc region extracted from the script is not the unit"
+    )
+    return heredoc
+
+
+def _unit_directives(unit: str) -> list[str]:
+    """The lines systemd acts on — every line that is neither a comment nor blank.
+
+    Comments carry the phase's reasoning and *are* part of the file, but they are also the one
+    thing phase 17 task 01 rewrote deliberately (the old block justified the filter; the new
+    one explains its absence). Directives are what a host runs, so those are compared exactly.
+    """
+    return [line for line in unit.splitlines() if line.strip() and not line.startswith("#")]
+
+
+def test_the_template_unit_has_no_device_filter_lines() -> None:
+    """The fix is the absence, so the absence is what is pinned.
+
+    Three layers, because the regression has three shapes: a `DevicePolicy=strict` or
+    `DeviceAllow=` line back in the heredoc (the deny-all returning on kernel 7.2, where
+    systemd matches those globs by *name* against `/proc/devices` and lists NVMe as `blkext`,
+    so `/dev/nvme*` matched nothing and strict denied the whole namespace);
+    `PrivateDevices=false` removed as "redundant" because it is the systemd default (a private
+    `/dev` with no device list dies at spawn with `status=208/STDIN`); and a rationale comment
+    that stops naming the mechanism, which is how the next operator re-adds the filter while
+    believing they are hardening the unit.
+    """
+    text = INSTALLER.read_text()
+    # The phase's completion criterion is this exact grep, so it is pinned as one: neither
+    # directive may appear anywhere in the script, prose included.
+    assert "DevicePolicy" not in text, "no device policy line anywhere in the installer"
+    assert "DeviceAllow" not in text, "no device allow-list anywhere in the installer"
+
+    heredoc = _unit_heredoc()
+    offenders = [line for line in heredoc.splitlines() if re.match(r"^\s*(DevicePolicy|DeviceAllow)\s*=", line)]
+    assert offenders == [], "the unit heredoc grew a device filter line again:\n" + "\n".join(offenders)
+    assert "PrivateDevices=false" in heredoc, (
+        "the explicit default is the line that keeps the unit from dying at STDIN; the filter "
+        "going away does not make it redundant"
+    )
+    # The rewritten rationale has to be an explanation, not a shrug (task 01's own criterion).
+    comment = "\n".join(line for line in heredoc.splitlines() if line.startswith("#"))
+    assert "/proc/devices" in comment and "blkext" in comment, (
+        "the comment must name the mechanism, or the next reader cannot tell an omitted filter "
+        "from a forgotten one"
+    )
+    assert "208" in comment, "and the STDIN failure that PrivateDevices=false still prevents"
+
+
+@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="running as root")
+def test_a_default_install_unit_is_the_pre_phase_unit_minus_the_filter_lines(tmp_path: Path) -> None:
+    """No flags writes the unit this script has always written, minus exactly ten lines.
+
+    `AGENTS.md` invariant 3 in deployment clothes: a host that needs nothing new must not get a
+    new unit from an unrelated phase. The reference is the whole pre-phase unit recorded above,
+    so every directive is compared (not a keyword spot-check) and the only allowed difference is
+    the device filter's absence — asserted as the ordered list comparison *and* as each of the ten
+    lines being absent by name, so a filter line silently renamed rather than removed fails too.
+    """
+    script, _call_log, env = _sandbox_installer(tmp_path)
+    result = _install(script, env, "--bind", "10.0.0.5", "--port", "9120")
+    assert result.returncode == 0, f"install failed:\n{result.stdout}\n{result.stderr}"
+    installed = (tmp_path / "target" / "dashboards-agent.service").read_text()
+
+    reference = PRE_PHASE_DEFAULT_UNIT.replace("<ROOT>", str(tmp_path))
+    expected = [line for line in _unit_directives(reference) if line not in DEVICE_FILTER_LINES]
+    assert len(expected) == len(_unit_directives(reference)) - len(DEVICE_FILTER_LINES), (
+        "DEVICE_FILTER_LINES must match ten real lines of the reference unit — a filter line "
+        "this list no longer names is a filter that would pass this test"
+    )
+    got = _unit_directives(installed)
+    drift = "\n".join(
+        difflib.unified_diff(
+            expected, got, fromfile="pre-phase unit minus the filter", tofile="installed", lineterm=""
+        )
+    )
+    assert got == expected, f"the default unit drifted from the pre-phase unit by more than the filter:\n{drift}"
+
+    # And the ten lines are gone rather than reworded into something equivalent.
+    assert not [line for line in installed.splitlines() if line.startswith(DEVICE_FILTER_LINES)]
+
+
+# --- the kernel-gate unit: --as-root / --caps (phase 17 task 02) ---------------------------
+
+#: The exact sentence `--as-root` alone prints. It is not a refusal — a capability-less root is
+#: a legitimate if unusual choice — but it is the fact gap 10 paid for: the first root-user
+#: attempt on the kernel-7.2 host still read `n/a` on all 16 drives, because the bounding set
+#: bounds even root (`CapEff 0`). Pinned verbatim so a paraphrase that drops "not uid" is a
+#: test failure rather than an operator rediscovering it.
+AS_ROOT_NOTE = (
+    "note: --as-root with the default empty CapabilityBoundingSet leaves the process "
+    "capability-less (the kernel gates check capabilities, not uid); pair it with --caps "
+    "for a kernel-gated host"
+)
+
+#: `--caps` on the dedicated service user is not the kernel-gate fix (a non-root process cannot
+#: hold these capabilities across exec), so the combination is refused rather than written.
+CAPS_REFUSAL = "--caps needs --as-root: a capability grant on the service user is not the kernel-gate fix"
+
+#: The two capabilities the kernel names for the two gates (NVMe SMART log / SG_IO), and the
+#: `driveripper` recipe as `--help` must teach it (phase 17 completion criterion).
+KERNEL_CAPS = "CAP_SYS_RAWIO,CAP_SYS_ADMIN"
+KERNEL_GATE_RECIPE = f"--as-root --caps {KERNEL_CAPS}"
+
+
+def _install(script: Path, env: dict[str, str], *flags: str) -> subprocess.CompletedProcess[str]:
+    """One sandboxed installer run: fake root boundary, real script body."""
+    try:
+        return subprocess.run(
+            [BASH, str(script), *flags],
+            capture_output=True,
+            text=True,
+            env=env,
+            timeout=60,
+            check=False,
+        )
+    except subprocess.TimeoutExpired as exc:  # pragma: no cover - the recursion regression
+        pytest.fail(f"the installer did not return (flags {flags}, partial output: {exc.stdout!r})")
+
+
+def _directive(unit: str, key: str) -> list[str]:
+    """Every line of the unit assigning `key` — a duplicate assignment is a heredoc bug."""
+    return [line for line in unit.splitlines() if line.split("=", 1)[0] == key]
+
+
+@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="running as root")
+def test_the_default_unit_keeps_the_service_user_and_an_empty_bounding_set(tmp_path: Path) -> None:
+    """No flags = the unit this script wrote before the flags existed, byte-for-byte.
+
+    `CapabilityBoundingSet=` with nothing after the `=` is the capability-less default and what
+    every host whose drives answer through the `disk` group runs; it is also the line
+    `--caps` replaces, so its exact spelling is pinned here rather than as `in unit`.
+    """
+    script, _call_log, env = _sandbox_installer(tmp_path)
+    result = _install(script, env, "--bind", "127.0.0.1", "--port", "9199")
+    assert result.returncode == 0, f"install failed:\n{result.stdout}\n{result.stderr}"
+    unit = (tmp_path / "target" / "dashboards-agent.service").read_text()
+    assert _directive(unit, "User") == ["User=dashboards-agent"]
+    assert _directive(unit, "CapabilityBoundingSet") == ["CapabilityBoundingSet="]
+    assert AS_ROOT_NOTE not in result.stdout, "the note belongs to --as-root alone"
+
+
+@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="running as root")
+def test_as_root_with_caps_writes_the_kernel_gate_unit(tmp_path: Path) -> None:
+    """The two lines that reproduce the working `driveripper` unit, exactly.
+
+    Comma on the command line, space in the unit: that is systemd's own spelling for
+    `CapabilityBoundingSet`, and it is the whole reason the flag takes a list rather than the
+    line. Everything else in the unit — the token file, its `0640 root:dashboards-agent`, the
+    hardening block — is unchanged by these flags.
+    """
+    script, call_log, env = _sandbox_installer(tmp_path)
+    kernel_gate = ["--bind", "10.0.0.5", "--port", "9120", "--as-root", "--caps", KERNEL_CAPS]
+    result = _install(script, env, *kernel_gate)
+    assert result.returncode == 0, f"install failed:\n{result.stdout}\n{result.stderr}"
+    unit = (tmp_path / "target" / "dashboards-agent.service").read_text()
+    assert _directive(unit, "User") == ["User=root"]
+    assert _directive(unit, "CapabilityBoundingSet") == ["CapabilityBoundingSet=CAP_SYS_RAWIO CAP_SYS_ADMIN"]
+    assert AS_ROOT_NOTE not in result.stdout, "--caps answers the note: the grant is there"
+    # The token file's logic is deliberately untouched by --as-root: root reads it either way,
+    # and re-deriving the mode per service user would be a second rule for no new safety.
+    token = tmp_path / "target" / "token"
+    assert stat.S_IMODE(token.stat().st_mode) == 0o640
+    assert "chown root:dashboards-agent" in call_log.read_text()
+    assert "--host 10.0.0.5 --port 9120" in unit
+
+
+@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="running as root")
+def test_as_root_and_caps_write_the_kernel_gate_unit_lines(tmp_path: Path) -> None:
+    """The whole flag contract in one place, plus the part only a comparison can pin.
+
+    The three flag shapes (`--as-root --caps LIST`, `--caps` alone, `--as-root` alone) each have
+    their own detailed pin nearby; what none of them shows is **how much** the recipe moves. So:
+    two installs in two sandboxes at the same bind and port, one default and one kernel-gated, and
+    their directive lists must differ by exactly the two lines this phase promises and nothing
+    else — the hardening block, the token file's path, the `ExecStart` and the `[Install]` section
+    are not these flags' business, and an `--as-root` that also slipped a `ProtectSystem` change in
+    would be a privilege change nobody asked for. The two half-recipes are then run in their own
+    sandboxes (one host re-run with different flags is the guard's decision, not the parser's).
+    """
+    plain_script, _plain_log, plain_env = _sandbox_installer(tmp_path / "plain")
+    plain = _install(plain_script, plain_env, "--bind", "10.1.0.126", "--port", "9120")
+    assert plain.returncode == 0, f"default install failed:\n{plain.stdout}\n{plain.stderr}"
+
+    gate_script, gate_log, gate_env = _sandbox_installer(tmp_path / "gate")
+    recipe = _install(
+        gate_script, gate_env, "--bind", "10.1.0.126", "--port", "9120", "--as-root", "--caps", KERNEL_CAPS
+    )
+    assert recipe.returncode == 0, f"the driveripper recipe failed:\n{recipe.stdout}\n{recipe.stderr}"
+
+    def directives(where: str) -> list[str]:
+        root = tmp_path / where
+        unit = (root / "target" / "dashboards-agent.service").read_text()
+        return _unit_directives(unit.replace(str(root), "<ROOT>"))
+
+    changed = [
+        f"{a} -> {b}"
+        for a, b in zip(directives("plain"), directives("gate"), strict=True)
+        if a != b
+    ]
+    assert changed == [
+        "User=dashboards-agent -> User=root",
+        "CapabilityBoundingSet= -> CapabilityBoundingSet=CAP_SYS_RAWIO CAP_SYS_ADMIN",
+    ], "--as-root --caps may write these two lines and nothing else"
+    assert "chown root:dashboards-agent" in gate_log.read_text(), (
+        "--as-root leaves the token file's ownership alone: root reads it either way, and the "
+        "grant must not widen because of a flag that is about capabilities"
+    )
+
+    # The two half-recipes, in their own sandboxes (a differing re-run of one host is the
+    # guard's business, not the flag parser's).
+    caps_only_script, caps_only_log, caps_only_env = _sandbox_installer(tmp_path / "caps-only")
+    caps_only = _install(
+        caps_only_script, caps_only_env, "--bind", "127.0.0.1", "--port", "9199", "--caps", KERNEL_CAPS
+    )
+    assert caps_only.returncode == 1, "--caps without --as-root is refused"
+    assert CAPS_REFUSAL in caps_only.stderr
+    assert not caps_only_log.exists(), "and refused before a single privileged command"
+
+    root_only_script, _root_only_log, root_only_env = _sandbox_installer(tmp_path / "root-only")
+    root_only = _install(root_only_script, root_only_env, "--bind", "127.0.0.1", "--port", "9199", "--as-root")
+    assert root_only.returncode == 0, f"--as-root alone must proceed:\n{root_only.stderr}"
+    assert AS_ROOT_NOTE in root_only.stdout, "with the capability-less note"
+    root_unit = (tmp_path / "root-only" / "target" / "dashboards-agent.service").read_text()
+    assert _directive(root_unit, "User") == ["User=root"]
+    assert _directive(root_unit, "CapabilityBoundingSet") == ["CapabilityBoundingSet="]
+
+
+@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="running as root")
+def test_as_root_alone_installs_and_prints_the_capability_note(tmp_path: Path) -> None:
+    """A capability-less root proceeds — with the reminder, on stdout, and no refusal."""
+    script, _call_log, env = _sandbox_installer(tmp_path)
+    result = _install(script, env, "--bind", "127.0.0.1", "--port", "9199", "--as-root")
+    assert result.returncode == 0, f"--as-root alone must proceed:\n{result.stdout}\n{result.stderr}"
+    assert AS_ROOT_NOTE in result.stdout, "the note is the operator's only warning here"
+    unit = (tmp_path / "target" / "dashboards-agent.service").read_text()
+    assert _directive(unit, "User") == ["User=root"]
+    assert _directive(unit, "CapabilityBoundingSet") == ["CapabilityBoundingSet="], (
+        "--as-root alone writes no capability grant: the note is the whole effect"
+    )
+
+
+@pytest.mark.parametrize(
+    ("caps", "refusal"),
+    [
+        ("CAP_SYS_RAWIO", CAPS_REFUSAL),
+        ("CAP_SYS_RAWIO,CAP_SYS_ADMIN", CAPS_REFUSAL),
+        ("", "--caps needs at least one capability name"),
+        (" , ", "--caps needs at least one capability name"),
+    ],
+    ids=["one-cap", "two-caps", "empty", "punctuation-only"],
+)
+@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="running as root")
+def test_caps_without_as_root_is_refused_before_anything_is_written(
+    tmp_path: Path, caps: str, refusal: str
+) -> None:
+    """Exit 1, the reason on stderr, and no side effect at all.
+
+    The check sits *before* `need_root` (the `--cert-bundle` validator's reasoning): a wrong
+    flag combination gets its own sentence instead of a sudoers hint, and nothing after it —
+    user, group membership, token, unit — can have run.
+    """
+    script, call_log, env = _sandbox_installer(tmp_path)
+    result = _install(script, env, "--bind", "127.0.0.1", "--port", "9199", "--caps", caps)
+    assert result.returncode == 1, "the refusal keeps exit 1"
+    assert result.stdout == "", "a refusal is a failure: nothing on stdout"
+    assert refusal in result.stderr
+    assert GRANT_PREFIX not in result.stderr, "a flag mistake is not a sudoers problem"
+    target = tmp_path / "target"
+    assert not (target / "dashboards-agent.service").exists(), "refused means nothing written"
+    assert not (target / "token").exists()
+    assert not call_log.exists(), "no privileged command ran either"
+
+
+@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="running as root")
+def test_caps_without_an_argument_is_refused_rather_than_crashing(tmp_path: Path) -> None:
+    """`--caps` last on the command line: exit 1 naming the flag, not an `unbound variable`."""
+    script, _call_log, env = _sandbox_installer(tmp_path)
+    result = _install(script, env, "--bind", "127.0.0.1", "--port", "9199", "--as-root", "--caps")
+    assert result.returncode == 1
+    assert result.stdout == ""
+    assert "--caps needs a capability list" in result.stderr
+
+
+@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="running as root")
+def test_the_caps_list_is_normalised_to_systemd_spelling(tmp_path: Path) -> None:
+    """Empty entries and stray whitespace never reach the unit as a double space…
+
+    …and a `*` stays a literal: the list is split with pathname expansion switched off, so an
+    operator typo cannot substitute the working directory's file names into `User=`'s partner.
+    """
+    script, _call_log, env = _sandbox_installer(tmp_path)
+    messy = ["--bind", "127.0.0.1", "--port", "9199", "--as-root", "--caps", "CAP_SYS_RAWIO,, CAP_SYS_ADMIN "]
+    result = _install(script, env, *messy)
+    assert result.returncode == 0, f"install failed:\n{result.stdout}\n{result.stderr}"
+    unit = (tmp_path / "target" / "dashboards-agent.service").read_text()
+    assert _directive(unit, "CapabilityBoundingSet") == ["CapabilityBoundingSet=CAP_SYS_RAWIO CAP_SYS_ADMIN"]
+
+    script_glob, _log, env_glob = _sandbox_installer(tmp_path / "glob")
+    star = ["--bind", "127.0.0.1", "--port", "9199", "--as-root", "--caps", "*"]
+    result_glob = _install(script_glob, env_glob, *star)
+    assert result_glob.returncode == 0, f"install failed:\n{result_glob.stdout}\n{result_glob.stderr}"
+    unit_glob = (tmp_path / "glob" / "target" / "dashboards-agent.service").read_text()
+    assert _directive(unit_glob, "CapabilityBoundingSet") == ["CapabilityBoundingSet=*"], (
+        "the capability list is words, not a glob"
+    )
+
+
+def test_help_teaches_the_kernel_gate_recipe() -> None:
+    """Completion criterion: the `driveripper` flags are learnable from `--help` alone.
+
+    `--help` *is* the header comment, so the sentence that explains why a root service needs
+    its capabilities named is pinned here too — it is the one thing an executor on a fresh
+    clone cannot infer from a unit file.
+    """
+    result = _run([BASH, str(INSTALLER), "--help"])
+    assert result.returncode == 0
+    assert KERNEL_GATE_RECIPE in result.stdout, "the recipe line must be copy-pasteable"
+    assert "--as-root      run the service as root" in result.stdout
+    assert "--caps LIST    CapabilityBoundingSet line for the unit" in result.stdout
+    assert "capabilities, not uid" in result.stdout, "the kernel gate is the why of both flags"
+    assert "capability-less" in result.stdout, "--as-root without --caps has to say what it is"
+
+
+def test_the_status_probe_is_blind_to_who_the_service_runs_as() -> None:
+    """`--status` probes the agent over the wire; the unit's user is invisible to it, on purpose.
+
+    A status path that read `AS_ROOT`/`CAPS` would make a health probe depend on flags the
+    operator may not have passed on this invocation — the probe answers about the port.
+    """
+    status_body = INSTALLER.read_text().split("status() {", 1)[1].split("\n}", 1)[0]
+    assert "AS_ROOT" not in status_body
+    assert "CAPS" not in status_body
+
+
 def test_token_file_is_readable_by_the_service_user_and_nobody_else(tmp_path: Path) -> None:
    """`--token-file` is a path the *agent* opens, as its own user.

@@ -409,6 +888,297 @@ def test_token_file_is_readable_by_the_service_user_and_nobody_else(tmp_path: Pa
    assert not mode & 0o007, "the token file must never be world-readable"


+# --- the re-run guard: no silent clobber (phase 17 task 03) ----------------------------------
+
+def _refusal(unit: Path) -> str:
+    """The sentence a refused re-run prints, with the unit path substituted.
+
+    Pinned verbatim because it is the operator's only explanation of why a routine re-run
+    stopped, and because the diff follows it on **stdout** rather than stderr: the diff is the
+    payload of the answer (an operator pipes it into a pager or a file), and splitting a
+    sentence from its diff across two streams only interleaves them.
+    """
+    return f"refusing to overwrite {unit} (generated unit differs); run with --force to apply:"
+
+
+#: One line the operator can plausibly have edited on the host — `RestartSec` is a real knob,
+#: it is not a line any flag writes, and it sits far from every other assertion in these tests.
+OPERATOR_EDIT = ("RestartSec=5", "RestartSec=7  # hand-tuned on the host after a flapping start")
+
+
+def _hand_edit(unit_path: Path) -> str:
+    """Edit the installed unit the way an operator does, and return what it now holds."""
+    text = unit_path.read_text()
+    assert OPERATOR_EDIT[0] in text, "the template unit lost the line the operator edits"
+    edited = text.replace(*OPERATOR_EDIT)
+    unit_path.write_text(edited, encoding="utf-8")
+    return edited
+
+
+def _systemctl_calls(call_log: Path) -> list[str]:
+    """Only the `systemctl` lines of the stub log — the ones that touch the running service.
+
+    The privileged calls before the guard (`id`, `getent`, `usermod`, `chown`) do run again on
+    a refused re-run: they are the idempotent ones the guard's reasoning accepts. `systemctl`
+    is what a refused run must not do — no `daemon-reload`, no `enable --now` — so that is the
+    half compared here.
+    """
+    return [line for line in call_log.read_text().splitlines() if line.startswith("systemctl ")]
+
+
+@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="running as root")
+def test_a_rerun_that_would_change_the_installed_unit_refuses(tmp_path: Path) -> None:
+    """install → hand-edit → re-run: exit 1, the diff on stdout, nothing applied.
+
+    This is the incident the guard exists for: `driveripper`'s unit was fixed by hand, and the
+    installer rewrote it unconditionally. A refused run must leave the unit on disk as the
+    operator left it and must not reload or re-enable anything, or the refusal arrives after
+    the thing it is refusing.
+    """
+    script, call_log, env = _sandbox_installer(tmp_path)
+    unit_path = tmp_path / "target" / "dashboards-agent.service"
+    token_path = tmp_path / "target" / "token"
+
+    first = _install(script, env, "--bind", "10.0.0.5", "--port", "9120")
+    assert first.returncode == 0, f"first install failed:\n{first.stdout}\n{first.stderr}"
+    edited = _hand_edit(unit_path)
+    token_before = token_path.read_bytes()
+    reloads_before = _systemctl_calls(call_log)
+    assert reloads_before, "the first install did reload and enable the service"
+
+    result = _install(script, env, "--bind", "10.0.0.5", "--port", "9120")
+    assert result.returncode == 1, "a differing re-run refuses with exit 1"
+    assert result.stderr == "", "the refusal and its diff are one answer: stdout, not split"
+    assert _refusal(unit_path) in result.stdout
+    assert "--- " in result.stdout and "+++ " in result.stdout, "the diff is unified"
+    assert f"-{OPERATOR_EDIT[1]}" in result.stdout, "the diff shows the operator's line going away"
+    assert "+RestartSec=5" in result.stdout, "and the template's line coming back"
+    assert unit_path.read_text() == edited, "a refused run leaves the installed unit byte-identical"
+    # The reuse path re-reads the existing secret and rewrites the same bytes with the same
+    # mode; its *value* — what invariant 6 and the HOST_TARGETS entry depend on — never moves,
+    # which is the guarantee here (mtime is not: the write happens before the guard).
+    assert token_path.read_bytes() == token_before, "the reused token must not change"
+    assert stat.S_IMODE(token_path.stat().st_mode) == 0o640
+    assert _systemctl_calls(call_log) == reloads_before, (
+        "a refused run must not daemon-reload or enable: that would restart the service the "
+        "refusal exists to leave alone"
+    )
+
+
+@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="running as root")
+def test_force_prints_the_diff_then_writes_the_generated_unit(tmp_path: Path) -> None:
+    """`--force` is the opt-in, and the diff still arrives: an operator approves what they saw."""
+    script, call_log, env = _sandbox_installer(tmp_path)
+    unit_path = tmp_path / "target" / "dashboards-agent.service"
+    _install(script, env, "--bind", "10.0.0.5", "--port", "9120")
+    _hand_edit(unit_path)
+
+    result = _install(script, env, "--bind", "10.0.0.5", "--port", "9120", "--force")
+    assert result.returncode == 0, f"--force must proceed:\n{result.stdout}\n{result.stderr}"
+    assert _refusal(unit_path) not in result.stdout, "--force is not a refusal"
+    assert f"-{OPERATOR_EDIT[1]}" in result.stdout and "+RestartSec=5" in result.stdout, (
+        "the diff is printed even when the answer is yes"
+    )
+    unit = unit_path.read_text()
+    assert "RestartSec=5" in unit and OPERATOR_EDIT[1] not in unit, "the generated unit is written"
+    assert _systemctl_calls(call_log).count("systemctl enable --now dashboards-agent") == 2, (
+        "the overwrite is followed by the usual reload/enable"
+    )
+
+
+@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="running as root")
+def test_an_unchanged_rerun_is_silent_and_proceeds(tmp_path: Path) -> None:
+    """Idempotence is the normal path: same flags, same unit, no diff, no complaint, exit 0."""
+    script, call_log, env = _sandbox_installer(tmp_path)
+    unit_path = tmp_path / "target" / "dashboards-agent.service"
+    first = _install(script, env, "--bind", "10.0.0.5", "--port", "9120")
+    assert first.returncode == 0, f"first install failed:\n{first.stdout}\n{first.stderr}"
+    installed = unit_path.read_text()
+
+    result = _install(script, env, "--bind", "10.0.0.5", "--port", "9120")
+    assert result.returncode == 0, f"an identical re-run must proceed:\n{result.stdout}\n{result.stderr}"
+    assert _refusal(unit_path) not in result.stdout
+    assert "+++ " not in result.stdout, "nothing differs, so nothing is printed about differences"
+    assert "reusing the existing token" in result.stdout, "the token idiom is unchanged"
+    assert unit_path.read_text() == installed
+    assert _systemctl_calls(call_log).count("systemctl enable --now dashboards-agent") == 2
+
+
+@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="running as root")
+def test_a_re_run_that_differs_refuses_without_force_and_applies_with_it(tmp_path: Path) -> None:
+    """The refuse → force → silent sequence on **one** host, in that order.
+
+    Each leg has its own detailed pin beside it; what those cannot show is the transition — a
+    guard that refuses correctly the first time and then never lets an identical re-run through
+    silently is the same bug seen from the other side (an operator who has to type `--force`
+    every deploy stops reading the diff). So: install, hand-edit, re-run (refused, unit left
+    exactly as edited, no new `systemctl` call), re-run with `--force` (diff first, generated
+    unit on disk, service re-enabled), then re-run plain once more (silent, unit unmoved).
+    """
+    script, call_log, env = _sandbox_installer(tmp_path)
+    unit_path = tmp_path / "target" / "dashboards-agent.service"
+    flags = ["--bind", "10.1.0.126", "--port", "9120", "--as-root", "--caps", KERNEL_CAPS]
+
+    first = _install(script, env, *flags)
+    assert first.returncode == 0, f"first install failed:\n{first.stdout}\n{first.stderr}"
+    edited = _hand_edit(unit_path)
+    reloads = len(_systemctl_calls(call_log))
+
+    refused = _install(script, env, *flags)
+    assert refused.returncode == 1, "leg 1: a differing re-run refuses"
+    assert _refusal(unit_path) in refused.stdout and "+++ " in refused.stdout
+    assert unit_path.read_text() == edited, "leg 1: the hand edit survives"
+    assert len(_systemctl_calls(call_log)) == reloads, "leg 1: nothing was reloaded or re-enabled"
+
+    applied = _install(script, env, *flags, "--force")
+    assert applied.returncode == 0, f"leg 2: --force failed:\n{applied.stdout}\n{applied.stderr}"
+    assert "+++ " in applied.stdout, "leg 2: the diff is printed even on a yes"
+    assert unit_path.read_text() != edited, "leg 2: the generated unit replaced the hand edit"
+    assert "RestartSec=5" in unit_path.read_text()
+    generated = unit_path.read_text()
+    # three calls per applied install: daemon-reload, enable --now, and the is-active check
+    assert len(_systemctl_calls(call_log)) == reloads + 3, "leg 2: reload + enable, as a normal install"
+
+    settled = _install(script, env, *flags)
+    assert settled.returncode == 0, f"leg 3: a silent re-run failed:\n{settled.stdout}\n{settled.stderr}"
+    assert "refusing to overwrite" not in settled.stdout and "+++ " not in settled.stdout
+    assert "reusing the existing token" in settled.stdout
+    assert unit_path.read_text() == generated, "leg 3: the silent path leaves the applied unit in place"
+
+
+@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="running as root")
+def test_a_fresh_install_never_meets_the_guard(tmp_path: Path) -> None:
+    """No installed unit → no comparison, no output about one, and `--force` changes nothing.
+
+    The guard is about a *re*-run. A first install must stay exactly the run it was before this
+    task, with or without `--force` — hence the two sandboxes writing byte-identical units.
+    """
+    plain_script, _plain_log, plain_env = _sandbox_installer(tmp_path / "plain")
+    forced_script, _forced_log, forced_env = _sandbox_installer(tmp_path / "forced")
+
+    plain = _install(plain_script, plain_env, "--bind", "10.0.0.5", "--port", "9120")
+    forced = _install(forced_script, forced_env, "--bind", "10.0.0.5", "--port", "9120", "--force")
+    assert plain.returncode == 0, f"a first install failed:\n{plain.stdout}\n{plain.stderr}"
+    assert forced.returncode == 0, f"--force broke a first install:\n{forced.stdout}\n{forced.stderr}"
+    for stdout in (plain.stdout, forced.stdout):
+        assert "refusing to overwrite" not in stdout
+        assert "+++ " not in stdout
+
+    def rendered(where: str) -> str:
+        """The unit that sandbox wrote, with its own sandbox root name blanked out.
+
+        The two runs live in different directories, so the absolute paths in `ExecStart` and
+        `--token-file` differ while every *decision* in the unit is the same — and the decision
+        is what `--force` must not touch.
+        """
+        root = tmp_path / where
+        return (
+            (root / "target" / "dashboards-agent.service")
+            .read_text()
+            .replace(str(root), "<sandbox>")
+        )
+
+    assert rendered("plain") == rendered("forced"), "--force writes the unit the default install writes"
+
+
+@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="running as root")
+def test_the_kernel_gate_reproduction_refuses_once_and_then_holds(tmp_path: Path) -> None:
+    """The `driveripper` case end to end: a token-mode unit is *not* the kernel-gate unit.
+
+    Switching a host to `--as-root --caps` is a real change to a real unit, so it asks first,
+    applies with `--force`, and is silent afterwards — the flag line in `--help` is a
+    reproduction command only if running it twice lands the same unit and says nothing twice.
+    """
+    script, _call_log, env = _sandbox_installer(tmp_path)
+    unit_path = tmp_path / "target" / "dashboards-agent.service"
+    kernel_gate = ["--bind", "10.1.0.126", "--port", "9120", "--as-root", "--caps", KERNEL_CAPS]
+    _install(script, env, "--bind", "10.1.0.126", "--port", "9120")
+    token_mode = unit_path.read_text()
+
+    asked = _install(script, env, *kernel_gate)
+    assert asked.returncode == 1, "a flag change to an installed unit is a change, so it asks"
+    assert _refusal(unit_path) in asked.stdout
+    assert "-User=dashboards-agent" in asked.stdout and "+User=root" in asked.stdout
+    assert "+CapabilityBoundingSet=CAP_SYS_RAWIO CAP_SYS_ADMIN" in asked.stdout
+    assert unit_path.read_text() == token_mode, "the question must not also be the answer"
+
+    applied = _install(script, env, *kernel_gate, "--force")
+    assert applied.returncode == 0, f"--force failed:\n{applied.stdout}\n{applied.stderr}"
+    applied_unit = unit_path.read_text()
+    assert _directive(applied_unit, "User") == ["User=root"]
+    assert _directive(applied_unit, "CapabilityBoundingSet") == [
+        "CapabilityBoundingSet=CAP_SYS_RAWIO CAP_SYS_ADMIN"
+    ]
+
+    again = _install(script, env, *kernel_gate, "--force")
+    assert again.returncode == 0, f"the same command twice failed:\n{again.stdout}\n{again.stderr}"
+    assert "+++ " not in again.stdout, "once applied, the same flags are the zero-noise path"
+    assert unit_path.read_text() == applied_unit
+
+
+@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="running as root")
+def test_uninstall_is_still_deliberate_and_never_asks(tmp_path: Path) -> None:
+    """The guard belongs to `do_install`: `--uninstall` removes a hand-edited unit without one.
+
+    A guard on the teardown path would be a guard that keeps a unit nobody wants, and
+    `uninstall()` does not render a unit at all.
+    """
+    script, call_log, env = _sandbox_installer(tmp_path)
+    unit_path = tmp_path / "target" / "dashboards-agent.service"
+    _install(script, env, "--bind", "10.0.0.5", "--port", "9120")
+    _hand_edit(unit_path)
+
+    result = _install(script, env, "--uninstall")
+    assert result.returncode == 0, f"uninstall refused:\n{result.stdout}\n{result.stderr}"
+    assert "refusing to overwrite" not in result.stdout
+    assert not unit_path.exists(), "uninstall still removes the unit, edited or not"
+    assert "systemctl disable --now dashboards-agent" in _systemctl_calls(call_log)[-3:]
+
+
+def test_the_guard_sits_between_the_render_and_the_unit_write() -> None:
+    """Text pin for the ordering the behaviour depends on.
+
+    The comparison must read the *rendered* unit (every flag, bundle and capability decision
+    applied) and must come before the write, the `daemon-reload` and the `enable`; a `cat >
+    "$UNIT" <<UNIT_FILE` would put the clobber back while still passing every behavioural test
+    above on a host with no installed unit.
+    """
+    text = INSTALLER.read_text()
+    assert "do_install() {" in text, "the install body is still the function that writes the unit"
+    # Scoped to do_install: `uninstall()` also reloads the daemon, and an unscoped index would
+    # compare offsets across two different functions.
+    body = text.split("do_install() {", 1)[1]
+    render = body.index('cat > "$TMP_UNIT" <<UNIT_FILE')
+    guard = body.index('! cmp -s "$UNIT" "$TMP_UNIT"')
+    write = body.index('cat "$TMP_UNIT" > "$UNIT"')
+    reload = body.index("systemctl daemon-reload")
+    enable = body.index('systemctl enable --now "$SERVICE"')
+    assert render < guard < write < reload < enable
+    assert 'cat > "$UNIT" <<' not in text, "the template may only write $UNIT after comparing it"
+    assert 'if [ "$FORCE" -eq 1 ]; then' in text, "--force is the guard's own opt-in branch"
+    assert "rm -f \"$TMP_UNIT\"" in text, "the rendered temp file is cleaned up on every path"
+    assert 'mktemp "${TMPDIR:-/tmp}/dashboards-agent-unit.XXXXXX"' in text, (
+        "the diff's +++ side must name the generated unit, not an anonymous temp file"
+    )
+
+
+#: The `--force` help line's sentence (phase 17 completion criterion: the flags are learnable
+#: from `--help`, and the diff-first promise is the reason an operator trusts `--force`).
+FORCE_HELP_SENTENCE = "overwrite an existing unit even if the generated unit differs"
+
+
+def test_help_documents_force_and_keeps_the_flag_block_aligned() -> None:
+    stdout = _run([BASH, str(INSTALLER), "--help"]).stdout
+    lines = [line for line in stdout.splitlines() if FORCE_HELP_SENTENCE in line]
+    assert len(lines) == 1, f"exactly one --force flag line is expected, got {lines}"
+    assert lines[0].lstrip().startswith("--force")
+    assert "(prints the" in lines[0] and "diff first)" in stdout
+    # One column for every flag's description: the block is a table, and `--force` landing at
+    # its own column is how `--help` stays readable as a reference rather than a paragraph.
+    as_root = next(line for line in stdout.splitlines() if line.lstrip().startswith("--as-root"))
+    assert lines[0].index(FORCE_HELP_SENTENCE) == as_root.index("run the service as root")
+
+
 # --- the certificate bundle (phase 15 task 06) --------------------------------------------

 #: Where the installed bundle lives — one constant in the script, pinned here so a rename