diff --git a/CHANGELOG.md b/CHANGELOG.md index c03e08c..ea21356 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,26 @@ +## Unreleased (2026-10-07) — the agent can no longer hand the guest any image; felhom-op's pct lines are exact (R-861 (a) A1, (b) B2; `09` §3 decision 165) + +**Delivery order: agent binary FIRST, then the config bundle.** The new sudoers drops the agent's in-guest `tee` +grant; an older binary still calls `tee`, so a bundle that lands before the binary would stop managed controller +updates (and the old binary's capability probe would read `controllerswap-write` degraded). No bundle path is added +(`felhom-priv-apply` and `/etc/sudoers.d/felhom-op` are already bundle files), so no step bundle. + +- `configs/felhom-priv-apply`: new verb `controller-image ` — reads the ref on stdin (≤ 256 bytes, ASCII, one + optional trailing newline), requires `^gitea\.dooplex\.hu/admin/felhom-controller:[0-9]+\.[0-9]+\.[0-9]+$` (the agent's + own `controllerImageRe`), then runs `pct exec -- tee /etc/felhom-controller-image` AS ROOT; refusal rule `I1` + (rc 3), a bad vmid `A1` (rc 2); listed in `--self-check`. +- `configs/felhom-agent.sudoers` `FELHOM_CONTROLLERSWAP`: `pct ^exec [0-9]+ -- tee /etc/felhom-controller-image$` + REMOVED; `/usr/local/sbin/felhom-priv-apply ^controller-image [0-9]+$` added. +- `internal/localapi`: `GuestExecutor.GuestExecStdin` replaced by `WriteControllerImage`; `GuestBinder.WriteControllerImage` + pipes `ref\n` to the verb through the fenced runner; the swap's `writeImage` calls it. Capability `controllerswap-write` + now probes the verb. +- `configs/felhom-op.sudoers` (B2, hygiene): `pct start|stop|unlock [0-9]*` → `pct ^start [0-9]+$` etc. (the glob's `*` + matched spaces: `pct stop 9201 --skiplock 1` passed). +- Tests: `ControllerImage` (5, `configs/test_felhom_priv_apply.py`), `TestSudoersRefusesTheR861Injections` (+3 lines), + `TestSudoersAllowsTheControllerImageVerb`, `TestFelhomOpSudoersPctIsExact`, `TestR861_WriteControllerImageUsesTheRootVerb`, + `TestControllerSwap_WriteViaRootVerb_NoShell`. Red-proofs: `felhom.eu/documentation/audits/day-2026-10-07/C/`. +- `README.md`: the controller-swap paragraph described the removed `tee` path — corrected. + ## v0.150.0 — the Docker step proves the engine reports a memory kill; after a restart the agent remembers the last backup per tier; three more SMART counters on the wire (R-528, R-894, R-330; `09` §3 decisions 157, 161) (2026-10-07) Released by `scripts/release-agent.sh`: binary sha256 `a23d1c9085bc7fd4fc48fe0327f6504aa83e6331510fb4a3d23e042dddb26f9c` diff --git a/README.md b/README.md index 8555cd7..274a8ed 100644 --- a/README.md +++ b/README.md @@ -44,13 +44,14 @@ unnoticed until a user hit them. `internal/capability` makes that loud: (`HostCapabilityChecker`) alerts the operator on a Critical capability going degraded. Serve-degraded — the probe never blocks startup. (Next self-health slice: the controller↔agent channel check.) -**Controller-swap under non-root (v0.45.0).** The agent-owned controller image swap -(`internal/localapi/controllerswap.go`) no longer shells out: `writeImage` pipes the image ref on -**stdin** into an in-guest `tee /etc/felhom-controller-image` (via `GuestExecStdin` → -`Runner.RunStdin`, the same fenced `sudo -n` runner) — no `bash -c`, no interpolation. Its 5 narrow -grants live in the `FELHOM_CONTROLLERSWAP` sudoers alias (all read-only or fixed-target; the `tee` -target is the FIXED image path, content stdin-fed) and in the capability manifest (Critical), so a -dropped grant is a build failure + a live degraded signal. No general `pct exec` is granted. +**Controller-swap under non-root (v0.45.0; the write since R-861 (a) A1).** The agent-owned controller image swap +(`internal/localapi/controllerswap.go`) no longer shells out. The write goes on **stdin** to the ROOT verb +`felhom-priv-apply controller-image ` (`GuestBinder.WriteControllerImage` → `Runner.RunStdin`, the same fenced +`sudo -n` runner), which re-checks the ref against our registry + repository + an x.y.z tag and writes +`/etc/felhom-controller-image` inside the guest itself; the agent has no in-guest `tee` grant any more (before, a +compromised agent could feed any image — sudo cannot see stdin). Its grants live in the `FELHOM_CONTROLLERSWAP` sudoers +alias (read-only or fixed-target) and in the capability manifest (Critical), so a dropped grant is a build failure + a +live degraded signal. No general `pct exec` is granted. ## The `storage` package — observe + watchdog (slice 5) diff --git a/REUSE.md b/REUSE.md index e05131b..5134fba 100644 --- a/REUSE.md +++ b/REUSE.md @@ -15,7 +15,7 @@ | `SudoHostOps.run` | internal/storage/hostops.go | `run(ctx, name, args...) error` | allowlisted exec with stderr-wrapped error | Every arg pre-validated via validate.go before this is called | | `Prober.Probe` | internal/capability/probe.go | `Probe(ctx) []Status` | live sudo-policy capability check (`sudo -n -l --`) | Needs a DIRECT runner (never the sudo-prefixing one — double-sudo); never executes probed cmds. v0.86.0: config-gated caps (`Capability.GatedBy` + `Prober.GateActive`) report `inactive`/"disabled by configuration" ONLY when healthy — broken plumbing stays degraded; the pbsdr-* gate answers from `pbsdr.Manager.DRConfigured` (marker-backed across restarts) | | ~~`stageTemp`~~ (REMOVED v0.146.0, R-861) | — | — | — | Nothing the agent writes is `install`ed where root reads it any more: use `felhom-priv-apply` (below) or ship a fixed file in the bundle | -| `felhom-priv-apply` (v0.146.0, R-861) | configs/felhom-priv-apply | `felhom-priv-apply unit \| dnsmasq \| wg \| sshd-config \| sshd-key` | ANY agent-rendered file a root program reads (systemd unit, dnsmasq drop-in, wg-quick conf, OOB sshd) — fixed source + destination, CONTENT checked against the agent's own renderers | A new renderer needs a verb + a contract test (`internal/privapplytest.Check`) feeding its REAL output; never a new `install` sudoers line | +| `felhom-priv-apply` (v0.146.0, R-861) | configs/felhom-priv-apply | `felhom-priv-apply unit \| dnsmasq \| wg \| sshd-config \| sshd-key \| controller-image ` (the last reads the ref on stdin, R-861 (a) A1) | ANY agent-rendered file a root program reads (systemd unit, dnsmasq drop-in, wg-quick conf, OOB sshd) — fixed source + destination, CONTENT checked against the agent's own renderers | A new renderer needs a verb + a contract test (`internal/privapplytest.Check`) feeding its REAL output; never a new `install` sudoers line | | `privapplytest.Check` | internal/privapplytest/check.go | `Check(t, verb, name, content) string` | the Go↔root-checker contract: a renderer's real output must read `OK` | Skips without python3; one call per rendered shape + one refused control | | `BUNDLE_FILES` + `Bundle` (mode `bundle`, `--install-bundle`; mode `agent_update` v0.146.0, R-861) | configs/felhom-os-apply | the ONE table of root-owned paths + the installer of them | ANY new root-owned file the installer writes (sudoers line, wrapper, unit) — add it to the table, never a new installer fetch (R-840) | The builder (`scripts/build-config-bundle.py`) and the installer read the same table; `test_every_root_file_the_installer_writes_is_in_the_bundle` fails on a path the bundle lacks. Trust files (`/etc/felhom/os-trust.json`, `operator-signers`) are NEVER bundle paths (R17) | | `osupdate.ConfigUpdateExecutor` | internal/osupdate/bundle.go | signed op `agent_config_update` {agent_version, bundle_sha256} | delivering the bundle to an installed box | a courier only: the root wrapper re-verifies signature, host, nonce and sha itself | @@ -168,7 +168,7 @@ | `backup.SpecBuilder` / `backup.TierPicker` / `(*BackupRunner).PickSettledRestoreCandidateOn` | internal/backup/schedule.go, runner.go | `func(ctx,archive) RestoreTestSpec`; `func(ctx,target,notAfter) (archive,landed,error)` | The per-run restore-test spec + per-tier **settled** candidate lookup (R-85, widened by R-86) | The spec is built **PER RUN**, never frozen at construction — the pre-R-85 immediately-invoked value made the offsite tier unschedulable AND went stale on any config change. `SourceTier` comes from **the archive**, never the configured target (the v0.100.0 rule). A tier with no archive returns `("", zero, nil)` — **`""` is NOT an error**, or every fresh box looks broken for its first week. **R-86: `notAfter` is the settle cutoff** (zero = no cutoff, which is what keeps `PickRestoreCandidateOn` a one-line call into it), and the picker now skips entries failing `archivePlausiblyComplete` — under per-archive due-ness an incomplete phantom would be picked forever, fail forever, never earn proof, and make the tier due at EVERY evaluation. | | `localapi.BackupTier` + `normalizeBackupTiers` / `config.BackupConfig.BackupTiers` | internal/localapi/backup_tiers.go, internal/config/config.go | `normalizeBackupTiers(tiers, legacy, cadence) []BackupTier`; `BackupTiers() ([]BackupTier, []string)` | THE R-82 multi-tier resolution — one runner per tier, primary first | **The untargeted local-API contract is FROZEN**: no `?target=` ⇒ primary tier ⇒ pre-R-82 response BYTES (Target is `omitempty` and stays empty). Never default a missing cadence — reject it and log the warning at ERROR. Never share one retention knob between tiers. Jobs are keyed by (vmid,target). | | `localapi.StaleLockController` | internal/localapi/stalelock.go | `*staleLockController` (Client + Runner + pool) | `fakeStaleLock` (Server-level) stalelock_test.go; `fakeStaleLockAPI` (controller-level, tests the A1 pool intersect) stalelock_pool_test.go | -| `localapi.GuestExecutor` | internal/localapi/controllerswap.go | `*GuestBinder` (pct exec) | `fakeGuestExec` internal/localapi/controllerswap_test.go | +| `localapi.GuestExecutor` | internal/localapi/controllerswap.go | `*GuestBinder` (pct exec; the image write via `felhom-priv-apply controller-image`) | `fakeGuestExec` internal/localapi/controllerswap_test.go | | `guestnet.Runner` / `guestnet.GuestSource` (R-54, v0.92.0) | internal/guestnet/{probe,watchdog}.go | `*proxmox.ExecRunner`; the POOL-VERIFIED `localapi.StaleLockController.Guests` (ListLXC ∩ felhom pool, audit A1) | `scriptedRunner` + `fakeGuests` internal/guestnet/watchdog_test.go. **Never wire a bare `ListLXC` here** — under a broad token that would run dhclient inside a co-tenant's container. Every assertion is an exec COUNT, and the load-bearing ones are the negatives: a static guest, an unprobeable guest, a boot-race guest and an unproven guest list must record **zero** heal calls | | `guestnet.Watchdog.SetDampers` / `now` (clock seam) | internal/guestnet/watchdog.go | config `guest_net.*`; `now` defaults to `time.Now` | tests advance a manual clock (the storage-watchdog pattern) and assert the heal ceilings EXACTLY — ≥10 min apart, ≤3/hour, and ≤30 over a scripted 10 hours of permanent failure. A damper with no test is a comment | | `hub.GuestNetReporter` (R-54) | internal/hub/collect.go | `*guestnet.Watchdog` (`GuestNetStatus`) | internal/hub/collect_guestnet_test.go asserts the stanza through the PRODUCTION `Collect` path AND that the `guest_net` key is ABSENT from the wire when no reporter is wired — an always-present empty stanza would make "not wired" and "found nothing" the same signal, which is the shape v0.91.0 hid behind | diff --git a/configs/felhom-agent.sudoers b/configs/felhom-agent.sudoers index 5643b51..b7ba9d7 100644 --- a/configs/felhom-agent.sudoers +++ b/configs/felhom-agent.sudoers @@ -111,15 +111,17 @@ Cmnd_Alias FELHOM_INTERMEDIARY = \ # docker inspect -f * — container running/health/image (read-only; `*` spans the -f template # + container across spaces, spike-confirmed) # systemctl restart — re-run the golden's bootstrap (the only state change) -# tee — WRITE the ref; content is fed on STDIN (no shell, no interpolation), -# the agent strict-validates the ref (controllerImageRe) before the write. +# felhom-priv-apply controller-image — WRITE the ref (R-861 (a) A1, `09` §3 decision 165): the ref goes on +# STDIN to the ROOT wrapper, which requires our registry + repository + an x.y.z tag +# and writes the guest file itself. The agent's own `tee` grant is GONE: before, a +# compromised agent could hand the guest's bootstrap ANY image (sudo cannot see stdin). # Validated GO: felhom.eu/documentation/audits/SPIKE-controllerswap-narrow-grants-2026-06-29.md. Cmnd_Alias FELHOM_CONTROLLERSWAP = \ /usr/sbin/pct ^exec [0-9]+ -- cat /etc/felhom-controller-image$, \ /usr/sbin/pct ^exec [0-9]+ -- docker image inspect gitea\.dooplex\.hu/admin/felhom-controller\:[0-9]+\.[0-9]+\.[0-9]+$, \ /usr/sbin/pct ^exec [0-9]+ -- docker inspect -f .+ (felhom-controller|cloudflared)$, \ /usr/sbin/pct ^exec [0-9]+ -- systemctl restart felhom-controller-bootstrap\.service$, \ - /usr/sbin/pct ^exec [0-9]+ -- tee /etc/felhom-controller-image$ + /usr/local/sbin/felhom-priv-apply ^controller-image [0-9]+$ # Stale-lock recovery (F2-b, v0.49.0). A host reboot DURING a vzdump backup leaves the guest with a # `snapshot-delete`/`backup` lock + `onboot:1` then can't start it → the customer box stays DOWN. The diff --git a/configs/felhom-op.sudoers b/configs/felhom-op.sudoers index 5118ba2..c2127b4 100644 --- a/configs/felhom-op.sudoers +++ b/configs/felhom-op.sudoers @@ -16,8 +16,10 @@ Cmnd_Alias FELHOM_OP_REPAIR = \ /usr/bin/systemctl reset-failed felhom-sshd, \ /usr/bin/systemctl restart felhom-sshd, \ /usr/sbin/pct list, \ - /usr/sbin/pct start [0-9]*, \ - /usr/sbin/pct stop [0-9]*, \ - /usr/sbin/pct unlock [0-9]* + /usr/sbin/pct ^start [0-9]+$, \ + /usr/sbin/pct ^stop [0-9]+$, \ + /usr/sbin/pct ^unlock [0-9]+$ +# R-861 (b) B2 (`09` §3 decision 165, hygiene): one numeric vmid per pct verb, anchored — the old glob `[0-9]*` also +# matched spaces, so `pct stop 9201 --skiplock 1` passed. Pinned by TestFelhomOpSudoersPctIsExact. felhom-op ALL=(root) NOPASSWD: FELHOM_OP_REPAIR diff --git a/configs/felhom-priv-apply b/configs/felhom-priv-apply index 7183b08..15c5d57 100755 --- a/configs/felhom-priv-apply +++ b/configs/felhom-priv-apply @@ -17,6 +17,8 @@ Verbs (each one sudoers line, exact-match pattern): wg /var/lib/felhom-agent/wg/wg-felhom.conf -> /etc/wireguard/wg-felhom.conf (0600) sshd-config /var/lib/felhom-agent/felhom-sshd/sshd_config -> /etc/felhom-sshd/sshd_config sshd-key /var/lib/felhom-agent/felhom-sshd/authorized_keys.felhom-op -> /etc/felhom-sshd/authorized_keys/felhom-op + controller-image the ref on STDIN -> /etc/felhom-controller-image INSIDE guest (R-861 (a) A1): only + our registry + our repository + an x.y.z tag; the agent no longer has a `tee` grant --self-check prints "felhom-priv-apply ok verbs=..." (the bundle's self-check) Exit codes: 0 installed (or already identical), 2 usage, 3 refused (content or source), 4 install failed. @@ -39,7 +41,14 @@ WG_SRC, WG_DEST = STATE + "/wg/wg-felhom.conf", "/etc/wireguard/wg-felhom.conf" SSHD_SRC, SSHD_DEST = STATE + "/felhom-sshd/sshd_config", "/etc/felhom-sshd/sshd_config" KEY_SRC, KEY_DEST = STATE + "/felhom-sshd/authorized_keys.felhom-op", "/etc/felhom-sshd/authorized_keys/felhom-op" MAX_BYTES = 64 * 1024 -VERBS = ("unit", "dnsmasq", "wg", "sshd-config", "sshd-key") +VERBS = ("unit", "dnsmasq", "wg", "sshd-config", "sshd-key", "controller-image") +# R-861 (a) A1 (`09` §3 decision 165): the SAME pattern as the agent's controllerImageRe (internal/localapi/ +# controllerswap.go) — a compromised agent cannot hand the guest's bootstrap any other image. Pinned by +# configs/test_felhom_priv_apply.py ControllerImage. +CONTROLLER_IMAGE_RE = re.compile(r"^gitea\.dooplex\.hu/admin/felhom-controller:[0-9]+\.[0-9]+\.[0-9]+$") +CONTROLLER_IMAGE_FILE = "/etc/felhom-controller-image" +CONTROLLER_IMAGE_MAX = 256 +VMID_RE = re.compile(r"^[0-9]{1,9}$") UNIT_NAME_RE = re.compile(r"^mnt-[A-Za-z0-9_.\\-]+\.(mount|automount)$") DNSMASQ_TMP_RE = re.compile(r"^/tmp/felhom-resolver-[0-9]+\.conf$") @@ -123,6 +132,16 @@ class Host: pass raise + def read_stdin(self, limit): + return sys.stdin.buffer.read(limit + 1) + + def write_guest_image(self, vmid, data): + """As root: `pct exec -- tee ` with the checked ref on stdin (no shell).""" + r = subprocess.run(["/usr/sbin/pct", "exec", str(vmid), "--", "tee", CONTROLLER_IMAGE_FILE], + input=data, stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, timeout=60) + if r.returncode != 0: + raise OSError(f"pct exec {vmid} tee exited {r.returncode}: {r.stderr.decode(errors='replace').strip()[:200]}") + def log(self, line): print(line, file=sys.stderr) try: @@ -377,6 +396,34 @@ def plan(argv): raise Refused("A1", f"wrong arguments for {v}") +def controller_image(rest, host): + """R-861 (a) A1: read the ref on stdin, check it, write it INSIDE the guest as root.""" + try: + if len(rest) != 1 or not VMID_RE.match(rest[0]): + raise Refused("A1", "usage: felhom-priv-apply controller-image (the ref on stdin)") + raw = host.read_stdin(CONTROLLER_IMAGE_MAX) + if len(raw) > CONTROLLER_IMAGE_MAX: + raise Refused("I1", f"the image ref is longer than {CONTROLLER_IMAGE_MAX} bytes") + try: + text = raw.decode("ascii") + except UnicodeDecodeError: + raise Refused("I1", "the image ref is not ASCII") + ref = text[:-1] if text.endswith("\n") else text + if not CONTROLLER_IMAGE_RE.match(ref) or "\n" in ref: + raise Refused("I1", "the image ref is not gitea.dooplex.hu/admin/felhom-controller:") + except Refused as e: + host.log(f"felhom-priv-apply: REFUSED [{e.rule}] controller-image {' '.join(rest)[:40]}: {e.reason}") + return 2 if e.rule == "A1" else 3 + vmid = int(rest[0]) + try: + host.write_guest_image(vmid, (ref + "\n").encode()) + except (OSError, subprocess.SubprocessError) as e: + host.log(f"felhom-priv-apply: FAILED controller-image {vmid}: {e}") + return 4 + host.log(f"felhom-priv-apply: WROTE controller-image {vmid} {ref}") + return 0 + + def main(argv, host=None): host = host or Host() if argv == ["--self-check"]: @@ -396,6 +443,8 @@ def main(argv, host=None): return 3 print("OK") return 0 + if argv and argv[0] == "controller-image": + return controller_image(argv[1:], host) try: verb, src, dest, mode, checker = plan(argv) data = host.read_source(src) diff --git a/configs/test_felhom_priv_apply.py b/configs/test_felhom_priv_apply.py index b00b9e6..c9bd97d 100644 --- a/configs/test_felhom_priv_apply.py +++ b/configs/test_felhom_priv_apply.py @@ -56,6 +56,18 @@ class FakeHost: def log(self, line): self.logs.append(line) + # controller-image (R-861 (a) A1): the ref arrives on stdin and is written INSIDE the guest by root. + stdin = b"" + guest_writes = None + + def read_stdin(self, limit): + return self.stdin[:limit + 1] + + def write_guest_image(self, vmid, data): + if self.guest_writes is None: + self.guest_writes = [] + self.guest_writes.append((vmid, data)) + LOCAL_UNIT = """# Managed by felhom-agent — do not edit by hand. [Unit] @@ -316,5 +328,53 @@ class Refuses(unittest.TestCase): self.refused(h, ["wg", "/etc/shadow"], "A1") +class ControllerImage(unittest.TestCase): + """R-861 (a) A1 (`09` §3 decision 165): the agent can no longer `tee` any image ref into the guest. The root verb + reads the ref on stdin, requires our registry + our repository + an x.y.z tag, and writes the guest file itself. + RED-PROOF: on the pre-A1 wrapper `controller-image` is not a verb (A1 usage, rc 2) — the accepted case fails.""" + + def go(self, ref, *argv): + h = FakeHost() + h.stdin = ref.encode() if isinstance(ref, str) else ref + return h, run(h, *(argv or ("controller-image", "9201"))) + + def test_our_controller_ref_is_written_in_the_guest(self): + h, rc = self.go("gitea.dooplex.hu/admin/felhom-controller:0.301.0\n") + self.assertEqual(rc, 0, h.logs) + self.assertEqual(h.guest_writes, [(9201, b"gitea.dooplex.hu/admin/felhom-controller:0.301.0\n")]) + + def test_a_foreign_image_is_refused(self): + for ref in ("docker.io/library/alpine:latest\n", "alpine\n", + "gitea.dooplex.hu/admin/felhom-controller:latest\n", + "gitea.dooplex.hu/admin/other:0.1.0\n", + "evil.example/admin/felhom-controller:0.301.0\n", + "gitea.dooplex.hu/admin/felhom-controller:0.301.0\nalpine\n", + "gitea.dooplex.hu/admin/felhom-controller:0.301.0 x\n", + "", "\n"): + h, rc = self.go(ref) + self.assertEqual(rc, 3, f"{ref!r} was accepted") + self.assertFalse(h.guest_writes, f"{ref!r} wrote the guest file") + self.assertTrue(any("[I1]" in l for l in h.logs), h.logs) + + def test_oversize_stdin_is_refused(self): + h, rc = self.go("gitea.dooplex.hu/admin/felhom-controller:0.301.0" + " " * 300) + self.assertEqual(rc, 3) + self.assertFalse(h.guest_writes) + + def test_vmid_must_be_numeric(self): + for argv in (("controller-image", "9201;id"), ("controller-image", "-1"), ("controller-image",), + ("controller-image", "9201", "9202")): + h, rc = self.go("gitea.dooplex.hu/admin/felhom-controller:0.301.0\n", *argv) + self.assertIn(rc, (2, 3), argv) + self.assertFalse(h.guest_writes, argv) + + def test_self_check_names_the_verb(self): + import io, contextlib + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + pa.main(["--self-check"]) + self.assertIn("controller-image", buf.getvalue()) + + if __name__ == "__main__": unittest.main(verbosity=2) diff --git a/internal/capability/manifest.go b/internal/capability/manifest.go index bd88044..b247d4a 100644 --- a/internal/capability/manifest.go +++ b/internal/capability/manifest.go @@ -145,7 +145,8 @@ var manifest = []Capability{ {"controllerswap-image-inspect", "controller-swap / managed auto-update", "/usr/sbin/pct", []string{"exec", "9201", "--", "docker", "image", "inspect", "gitea.dooplex.hu/admin/felhom-controller:0.0.0"}, true, ""}, {"controllerswap-inspect", "controller-swap / managed auto-update", "/usr/sbin/pct", []string{"exec", "9201", "--", "docker", "inspect", "-f", "{{.State.Running}}", "felhom-controller"}, true, ""}, {"controllerswap-restart", "controller-swap / managed auto-update", "/usr/sbin/pct", []string{"exec", "9201", "--", "systemctl", "restart", "felhom-controller-bootstrap.service"}, true, ""}, - {"controllerswap-write", "controller-swap / managed auto-update", "/usr/sbin/pct", []string{"exec", "9201", "--", "tee", "/etc/felhom-controller-image"}, true, ""}, + // R-861 (a) A1 (decision 165): the write goes through the ROOT verb that checks the ref; the agent has no `tee` grant. + {"controllerswap-write", "controller-swap / managed auto-update", "/usr/local/sbin/felhom-priv-apply", []string{"controller-image", "9201"}, true, ""}, // ---- Stale-lock recovery (FELHOM_STALELOCK, v0.49.0; Critical: a guest stuck behind a stale // reboot-during-backup lock can't start → the customer box stays DOWN until this clears it) ---- diff --git a/internal/capability/manifest_test.go b/internal/capability/manifest_test.go index 966d716..c0edd3a 100644 --- a/internal/capability/manifest_test.go +++ b/internal/capability/manifest_test.go @@ -200,7 +200,8 @@ func TestRedProof_DroppedGrantFailsCheck(t *testing.T) { } // TestRedProof_DroppedControllerSwapTeeFailsCheck is the companion red-proof for the v0.45.0 -// FELHOM_CONTROLLERSWAP grants: with the `tee /etc/felhom-controller-image` line removed, the +// FELHOM_CONTROLLERSWAP grants: with the write grant removed (since R-861 (a) A1 the `felhom-priv-apply controller-image` +// line; before it, an agent `tee /etc/felhom-controller-image`), the // controllerswap-write capability MUST be reported uncovered. Proves the build gate watches the new // swap write grant (so dropping it can't ship a non-root agent that silently can't auto-update). func TestRedProof_DroppedControllerSwapTeeFailsCheck(t *testing.T) { @@ -210,7 +211,7 @@ func TestRedProof_DroppedControllerSwapTeeFailsCheck(t *testing.T) { } var kept []string for _, ln := range strings.Split(string(data), "\n") { - if strings.Contains(ln, "tee /etc/felhom-controller-image") { + if strings.Contains(ln, "felhom-priv-apply ^controller-image") { // R-861 (a) A1: the write's grant continue } kept = append(kept, ln) @@ -229,7 +230,7 @@ func TestRedProof_DroppedControllerSwapTeeFailsCheck(t *testing.T) { } cmdline := write.Binary + " " + strings.Join(write.ReprArgs, " ") if matchesAny(cmdline, entries) { - t.Errorf("red-proof FAILED: controllerswap-write still matches after dropping the tee grant") + t.Errorf("red-proof FAILED: controllerswap-write still matches after dropping its grant") } if full := parseSudoersEntries(t, string(data)); !matchesAny(cmdline, full) { t.Errorf("controllerswap-write should be covered by the real sudoers") diff --git a/internal/capability/r861_injection_test.go b/internal/capability/r861_injection_test.go index 4b651b5..c0b8022 100644 --- a/internal/capability/r861_injection_test.go +++ b/internal/capability/r861_injection_test.go @@ -49,6 +49,10 @@ var r861Injections = []string{ "/usr/local/sbin/felhom-priv-apply unit ../../etc/x.mount", "/usr/local/sbin/felhom-priv-apply dnsmasq /etc/shadow felhom-x.conf", "/usr/local/sbin/felhom-priv-apply wg /etc/shadow", + // R-861 (a) A1 (decision 165): the agent wrote ANY image ref into the guest by `tee` — now only the root verb may + "/usr/sbin/pct exec 9201 -- tee /etc/felhom-controller-image", + "/usr/local/sbin/felhom-priv-apply controller-image 9201 9202", + "/usr/local/sbin/felhom-priv-apply controller-image 9201;id", } func TestSudoersRefusesTheR861Injections(t *testing.T) { @@ -93,3 +97,42 @@ func TestSudoersFstrimRuleIsExact(t *testing.T) { } } } + +// R-861 (a) A1: the managed controller update still has its route — the root verb, one numeric vmid. +func TestSudoersAllowsTheControllerImageVerb(t *testing.T) { + data, err := os.ReadFile(sudoersPath) + if err != nil { + t.Fatal(err) + } + if !matchesAny("/usr/local/sbin/felhom-priv-apply controller-image 9201", parseSudoersEntries(t, string(data))) { + t.Fatal("the sudoers does not allow `felhom-priv-apply controller-image 9201` — a managed controller update cannot write its image") + } +} + +// R-861 (b) B2 (decision 165, hygiene): felhom-op's `pct start|stop|unlock` grants are ONE numeric vmid each. The old +// glob `[0-9]*` eats spaces, so `pct stop 9201 --skiplock 1` and two vmids matched. +// RED-PROOF: on the pre-B2 felhom-op.sudoers (`/usr/sbin/pct stop [0-9]*`) the decoys match. +func TestFelhomOpSudoersPctIsExact(t *testing.T) { + data, err := os.ReadFile("../../configs/felhom-op.sudoers") + if err != nil { + t.Fatal(err) + } + entries := parseSudoersEntries(t, string(data)) + for _, ok := range []string{"/usr/sbin/pct start 9201", "/usr/sbin/pct stop 9201", "/usr/sbin/pct unlock 9201", "/usr/sbin/pct list"} { + if !matchesAny(ok, entries) { + t.Errorf("felhom-op lost a repair verb: %s", ok) + } + } + for _, bad := range []string{ + "/usr/sbin/pct stop 9201 --skiplock 1", + "/usr/sbin/pct start 9201 9202", + "/usr/sbin/pct unlock 9201 --whatever", + "/usr/sbin/pct start 92a1", + "/usr/sbin/pct stop ", + "/usr/sbin/pct destroy 9201", + } { + if matchesAny(bad, entries) { + t.Errorf("felhom-op's sudoers allows %q", bad) + } + } +} diff --git a/internal/localapi/controllersupervisor_test.go b/internal/localapi/controllersupervisor_test.go index 9b857e7..48d3fc5 100644 --- a/internal/localapi/controllersupervisor_test.go +++ b/internal/localapi/controllersupervisor_test.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "errors" - "io" "log/slog" "os" "path/filepath" @@ -54,8 +53,8 @@ func (f *supExec) GuestExec(_ context.Context, vmid int, args ...string) (string } return "", errors.New("supExec: unexpected args") } -func (f *supExec) GuestExecStdin(context.Context, int, io.Reader, ...string) (string, error) { - return "", errors.New("supExec: no stdin exec expected") +func (f *supExec) WriteControllerImage(context.Context, int, string) error { + return errors.New("supExec: no image write expected") } func (f *supExec) count(vmid int) int { f.mu.Lock() diff --git a/internal/localapi/controllerswap.go b/internal/localapi/controllerswap.go index 7797011..c150f6a 100644 --- a/internal/localapi/controllerswap.go +++ b/internal/localapi/controllerswap.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "io" "log/slog" "net/http" "os" @@ -41,9 +40,10 @@ func ValidControllerImage(ref string) bool { return controllerImageRe.MatchStrin // faked in tests. The single seam the swap composes over (no hand-rolled pct). type GuestExecutor interface { GuestExec(ctx context.Context, vmid int, args ...string) (string, error) - // GuestExecStdin is GuestExec with the command's stdin fed from stdin — the swap write pipes the - // image ref into an in-guest `tee` (no shell vector). - GuestExecStdin(ctx context.Context, vmid int, stdin io.Reader, args ...string) (string, error) + // WriteControllerImage writes the image ref into the guest's /etc/felhom-controller-image through the ROOT + // verb `felhom-priv-apply controller-image ` (R-861 (a) A1, `09` §3 decision 165), which re-checks the + // ref against our registry + repository + x.y.z. The agent no longer holds a `tee` grant into the guest. + WriteControllerImage(ctx context.Context, vmid int, image string) error } // ControllerSwapState is the durable record of a swap (crash-safety + status). Written before the swap @@ -141,14 +141,11 @@ func (c *ControllerSwapper) imagePresent(ctx context.Context, vmid int, image st } func (c *ControllerSwapper) writeImage(ctx context.Context, vmid int, image string) error { - // Non-root path: pipe the image ref into an in-guest `tee` over stdin — no shell, no - // interpolation, no `bash -c` (the only swap vector that would have needed an arbitrary-exec - // grant). The trailing "\n" makes the on-disk bytes byte-identical to the golden's - // `printf '%s\n'`; the bootstrap reads `IMAGE=$(cat …)` so the newline is stripped on read - // (spike SPIKE-controllerswap-narrow-grants-2026-06-29). image is strict-validated - // (controllerImageRe) upstream in Swap; defence-in-depth, the stdin path can't smuggle anyway. - _, err := c.exec.GuestExecStdin(ctx, vmid, strings.NewReader(image+"\n"), "tee", controllerImageFile) - return err + // R-861 (a) A1: the ROOT verb writes the file (it re-checks the ref — a compromised agent cannot hand the guest's + // bootstrap another image). The bytes are `image\n`, byte-identical to the golden's `printf '%s\n'`; the bootstrap + // reads `IMAGE=$(cat …)` so the newline is stripped on read. image is also strict-validated (controllerImageRe) + // upstream in Swap. + return c.exec.WriteControllerImage(ctx, vmid, image) } func (c *ControllerSwapper) restartBootstrap(ctx context.Context, vmid int) error { diff --git a/internal/localapi/controllerswap_test.go b/internal/localapi/controllerswap_test.go index 284833f..30ddeb1 100644 --- a/internal/localapi/controllerswap_test.go +++ b/internal/localapi/controllerswap_test.go @@ -23,7 +23,7 @@ type fakeGuestExec struct { present map[string]bool // images pulled into the guest good map[string]bool // images that report healthy when running containerImg string // image the running container currently has - teeStdin []string // raw bytes piped into each `tee` write (the swap's write vector) + teeStdin []string // image refs handed to WriteControllerImage (the root verb, R-861 (a) A1) failRestart bool noHealthBlock bool // if set, .State.Health is absent ("none") restartCount int // F1: .RestartCount reported by docker inspect (a crash-looper has >0) @@ -75,20 +75,15 @@ func (f *fakeGuestExec) GuestExec(_ context.Context, _ int, args ...string) (str return "", fmt.Errorf("fake: unexpected exec %v", args) } -// GuestExecStdin models the swap's write vector: `tee /etc/felhom-controller-image` with the image -// piped on stdin. It records the raw stdin bytes and sets the modeled file content (newline-stripped, -// as the bootstrap's `IMAGE=$(cat …)` read would see it). -func (f *fakeGuestExec) GuestExecStdin(_ context.Context, _ int, stdin io.Reader, args ...string) (string, error) { +// WriteControllerImage models the swap's write: the root verb `felhom-priv-apply controller-image ` (R-861 +// (a) A1). It records the ref and sets the modeled file content, as the bootstrap's `IMAGE=$(cat …)` would read it. +func (f *fakeGuestExec) WriteControllerImage(_ context.Context, _ int, image string) error { f.mu.Lock() defer f.mu.Unlock() - f.calls = append(f.calls, args) - b, _ := io.ReadAll(stdin) - if len(args) >= 2 && args[0] == "tee" && args[1] == controllerImageFile { - f.teeStdin = append(f.teeStdin, string(b)) - f.imageFile = strings.TrimSpace(string(b)) - return string(b), nil // tee echoes stdin to stdout - } - return "", fmt.Errorf("fake: unexpected exec-stdin args=%v stdin=%q", args, string(b)) + f.calls = append(f.calls, []string{"felhom-priv-apply", "controller-image", image}) + f.teeStdin = append(f.teeStdin, image+"\n") + f.imageFile = image + return nil } // wrote reports whether the image was written via the stdin `tee` vector with the exact `image\n` @@ -162,7 +157,7 @@ func TestControllerSwap_Happy(t *testing.T) { // The write vector must be the stdin `tee` with byte-identical `image\n` and NO shell — the // controllerswap.go writeImage rewrite. This would FAIL on the pre-change `bash -c "printf … >"` impl. -func TestControllerSwap_WriteViaStdinTee_NoShell(t *testing.T) { +func TestControllerSwap_WriteViaRootVerb_NoShell(t *testing.T) { fe := &fakeGuestExec{ imageFile: prevImg, present: map[string]bool{newImg: true}, @@ -173,22 +168,15 @@ func TestControllerSwap_WriteViaStdinTee_NoShell(t *testing.T) { t.Fatalf("state = %q, want done", st.State) } if !fe.wrote(newImg) { - t.Errorf("expected a tee write of %q+\\n; teeStdin=%q", newImg, fe.teeStdin) + t.Errorf("expected the root verb to write %q; writes=%q", newImg, fe.teeStdin) } - sawTee := false for _, c := range fe.calls { - if len(c) >= 2 && c[0] == "tee" { - sawTee = true - if c[1] != controllerImageFile { - t.Errorf("tee target = %q, want fixed %q", c[1], controllerImageFile) - } + if len(c) >= 1 && c[0] == "tee" { + t.Errorf("the swap still uses an in-guest tee (R-861 (a) A1 removed that grant): %v", c) } } - if !sawTee { - t.Error("no tee call recorded — writeImage did not use the stdin tee vector") - } if fe.usedShell() { - t.Errorf("swap used a shell vector (bash/-c/printf) — must be stdin tee only; calls=%v", fe.calls) + t.Errorf("swap used a shell vector (bash/-c/printf); calls=%v", fe.calls) } } @@ -300,9 +288,7 @@ func (s *inspectScript) GuestExec(_ context.Context, _ int, args ...string) (str } return "", nil } -func (s *inspectScript) GuestExecStdin(_ context.Context, _ int, _ io.Reader, _ ...string) (string, error) { - return "", nil -} +func (s *inspectScript) WriteControllerImage(context.Context, int, string) error { return nil } func fastSwapper(exec GuestExecutor) *ControllerSwapper { s := NewControllerSwapper(exec, "", discardLogger()) diff --git a/internal/localapi/guestbind.go b/internal/localapi/guestbind.go index 52315b1..6f1c94c 100644 --- a/internal/localapi/guestbind.go +++ b/internal/localapi/guestbind.go @@ -3,7 +3,6 @@ package localapi import ( "context" "fmt" - "io" "log/slog" "strconv" "strings" @@ -126,14 +125,16 @@ func (b *GuestBinder) GuestExec(ctx context.Context, vmid int, args ...string) ( return string(out), nil } -// GuestExecStdin is GuestExec with the in-guest command's stdin fed from stdin. The controller-swap -// write uses it to pipe the image ref into an in-guest `tee` (no shell vector, no interpolation), -// through the same fenced runner so the `sudo -n` prefix stays in one place. -func (b *GuestBinder) GuestExecStdin(ctx context.Context, vmid int, stdin io.Reader, args ...string) (string, error) { - pctArgs := append([]string{"exec", strconv.Itoa(vmid), "--"}, args...) - out, stderr, err := b.runner.RunStdin(ctx, stdin, "pct", pctArgs...) +// privApplyBin is the root content checker (R-861); its `controller-image` verb writes the guest's image file. +const privApplyBin = "/usr/local/sbin/felhom-priv-apply" + +// WriteControllerImage pipes `image\n` to `felhom-priv-apply controller-image ` through the same fenced runner +// (the `sudo -n` prefix stays in one place). The verb checks the ref as root and writes the guest file itself +// (R-861 (a) A1, `09` §3 decision 165). Pinned by TestR861_WriteControllerImageUsesTheRootVerb. +func (b *GuestBinder) WriteControllerImage(ctx context.Context, vmid int, image string) error { + _, stderr, err := b.runner.RunStdin(ctx, strings.NewReader(image+"\n"), privApplyBin, "controller-image", strconv.Itoa(vmid)) if err != nil { - return string(out), fmt.Errorf("pct exec %d %v: %w: %s", vmid, args, err, strings.TrimSpace(string(stderr))) + return fmt.Errorf("felhom-priv-apply controller-image %d: %w: %s", vmid, err, strings.TrimSpace(string(stderr))) } - return string(out), nil + return nil } diff --git a/internal/localapi/r861_controller_image_test.go b/internal/localapi/r861_controller_image_test.go new file mode 100644 index 0000000..39ae732 --- /dev/null +++ b/internal/localapi/r861_controller_image_test.go @@ -0,0 +1,48 @@ +package localapi + +import ( + "context" + "io" + "testing" +) + +// stdinRecorder is a proxmox.Runner that records each call and the stdin it was handed. +type stdinRecorder struct { + name string + args []string + stdin string +} + +func (r *stdinRecorder) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) { + r.name, r.args = name, args + return nil, nil, nil +} + +func (r *stdinRecorder) RunStdin(_ context.Context, stdin io.Reader, name string, args ...string) ([]byte, []byte, error) { + b, _ := io.ReadAll(stdin) + r.name, r.args, r.stdin = name, args, string(b) + return nil, nil, nil +} + +// R-861 (a) A1 (`09` §3 decision 165): the managed controller update writes the guest's image file through the ROOT +// verb, never through an in-guest `tee` the agent could feed any image. +// +// COMPANION RED-PROOF (observed): restore the pre-A1 body (`b.runner.RunStdin(ctx, …, "pct", "exec", vmid, "--", +// "tee", controllerImageFile)`) → this fails with "the image write ran pct …, want felhom-priv-apply". Restored. +func TestR861_WriteControllerImageUsesTheRootVerb(t *testing.T) { + rec := &stdinRecorder{} + b := NewGuestBinder(rec, discardLogger()) + const img = "gitea.dooplex.hu/admin/felhom-controller:0.302.0" + if err := b.WriteControllerImage(context.Background(), 9201, img); err != nil { + t.Fatal(err) + } + if rec.name != privApplyBin { + t.Fatalf("the image write ran %s %v, want felhom-priv-apply", rec.name, rec.args) + } + if len(rec.args) != 2 || rec.args[0] != "controller-image" || rec.args[1] != "9201" { + t.Fatalf("argv = %v, want [controller-image 9201] (the sudoers line `^controller-image [0-9]+$`)", rec.args) + } + if rec.stdin != img+"\n" { + t.Fatalf("stdin = %q, want the ref plus one newline", rec.stdin) + } +}