controllerswap: stdin tee write + narrow FELHOM_CONTROLLERSWAP grants (non-root, v0.45.0)

writeImage drops bash -c/printf for GuestExecStdin(img+\n -> tee /etc/felhom-controller-image);
new Runner.RunStdin/GuestExecStdin route stdin through the fenced sudo -n runner. 5 narrow,
auditable sudoers grants (no general pct exec, no bash -c) + capability manifest entries (Critical)
so the self-probe watches them and the build-test asserts coverage (companion red-proof). No
controller change; swap orchestration/rollback/state unchanged. Spike GO.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EPZ4GJ8L5Jqf8UiPwbn1kt
This commit is contained in:
2026-06-29 19:42:30 +02:00
parent 61c89a7efa
commit 8a4ccab3e6
15 changed files with 230 additions and 23 deletions
+8
View File
@@ -89,4 +89,12 @@ var manifest = []Capability{
{"dnsmasq-rm", "dnsmasq drop-in remove (decommission)", "/usr/bin/rm", []string{"-f", "/etc/dnsmasq.d/felhom-x.conf"}, false},
{"dnsmasq-guest-ip", "guest LAN IP discovery", "/usr/sbin/pct", []string{"exec", "9201", "--", "ip", "-4", "-o", "addr", "show", "dev", "eth0"}, false},
{"dnsmasq-guest-domain", "guest domain discovery", "/usr/sbin/pct", []string{"exec", "9201", "--", "docker", "exec", "felhom-controller", "cat", "/opt/docker/felhom-controller/controller.yaml"}, false},
// ---- Controller-swap / managed auto-update (FELHOM_CONTROLLERSWAP, v0.45.0; Critical: a
// silently-broken fleet auto-update is operator-alert-worthy) ----
{"controllerswap-read", "controller-swap / managed auto-update", "/usr/sbin/pct", []string{"exec", "9201", "--", "cat", "/etc/felhom-controller-image"}, true},
{"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},
}
+37
View File
@@ -179,3 +179,40 @@ func TestRedProof_DroppedGrantFailsCheck(t *testing.T) {
t.Errorf("guest-init-pid should be covered by the real sudoers")
}
}
// 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
// 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) {
data, err := os.ReadFile(sudoersPath)
if err != nil {
t.Fatalf("read sudoers: %v", err)
}
var kept []string
for _, ln := range strings.Split(string(data), "\n") {
if strings.Contains(ln, "tee /etc/felhom-controller-image") {
continue
}
kept = append(kept, ln)
}
mutated := strings.Join(kept, "\n")
entries := parseSudoersEntries(t, mutated)
var write Capability
for _, c := range Manifest() {
if c.Name == "controllerswap-write" {
write = c
}
}
if write.Name == "" {
t.Fatal("manifest missing controllerswap-write")
}
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")
}
if full := parseSudoersEntries(t, string(data)); !matchesAny(cmdline, full) {
t.Errorf("controllerswap-write should be covered by the real sudoers")
}
}
+11 -4
View File
@@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
"io"
"log/slog"
"net/http"
"os"
@@ -39,6 +40,9 @@ 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)
}
// ControllerSwapState is the durable record of a swap (crash-safety + status). Written before the swap
@@ -133,10 +137,13 @@ func (c *ControllerSwapper) imagePresent(ctx context.Context, vmid int, image st
}
func (c *ControllerSwapper) writeImage(ctx context.Context, vmid int, image string) error {
// image is strict-validated (controllerImageRe) before we ever get here, so this single-quoted
// interpolation cannot smuggle shell metacharacters.
cmd := fmt.Sprintf("printf '%%s\\n' '%s' > %s", image, controllerImageFile)
_, err := c.exec.GuestExec(ctx, vmid, "bash", "-c", cmd)
// 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
}
+68 -12
View File
@@ -6,7 +6,6 @@ import (
"io"
"log/slog"
"net/http/httptest"
"regexp"
"strings"
"sync"
"testing"
@@ -24,12 +23,11 @@ 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)
failRestart bool
noHealthBlock bool // if set, .State.Health is absent ("none")
}
var writeImgRe = regexp.MustCompile(`'([^']+)' >`)
func (f *fakeGuestExec) GuestExec(_ context.Context, _ int, args ...string) (string, error) {
f.mu.Lock()
defer f.mu.Unlock()
@@ -42,13 +40,6 @@ func (f *fakeGuestExec) GuestExec(_ context.Context, _ int, args ...string) (str
return args[3], nil
}
return "", fmt.Errorf("no such image: %s", args[3])
case len(args) >= 3 && args[0] == "bash" && args[1] == "-c":
m := writeImgRe.FindStringSubmatch(args[2])
if m == nil {
return "", fmt.Errorf("fake: unparseable write cmd %q", args[2])
}
f.imageFile = m[1]
return "", nil
case len(args) >= 3 && args[0] == "systemctl" && args[1] == "restart":
if f.failRestart {
return "", fmt.Errorf("fake: systemctl restart failed")
@@ -70,17 +61,50 @@ 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) {
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))
}
// wrote reports whether the image was written via the stdin `tee` vector with the exact `image\n`
// bytes (byte-identical to the golden's printf '%s\n').
func (f *fakeGuestExec) wrote(image string) bool {
f.mu.Lock()
defer f.mu.Unlock()
for _, c := range f.calls {
if len(c) >= 3 && c[0] == "bash" && strings.Contains(c[2], "'"+image+"'") {
for _, w := range f.teeStdin {
if w == image+"\n" {
return true
}
}
return false
}
// usedShell reports whether ANY exec used a shell vector (bash/-c/printf/sh) — the thing the swap
// rewrite removes. A regression to `bash -c "printf … >"` would make this true.
func (f *fakeGuestExec) usedShell() bool {
f.mu.Lock()
defer f.mu.Unlock()
for _, c := range f.calls {
for _, a := range c {
if a == "bash" || a == "sh" || a == "-c" || strings.HasPrefix(a, "printf") {
return true
}
}
}
return false
}
func newTestSwapper(t *testing.T, fe *fakeGuestExec) *ControllerSwapper {
t.Helper()
s := NewControllerSwapper(fe, t.TempDir(), nil)
@@ -122,6 +146,38 @@ 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) {
fe := &fakeGuestExec{
imageFile: prevImg,
present: map[string]bool{newImg: true},
good: map[string]bool{newImg: true, prevImg: true},
}
s := newTestSwapper(t, fe)
if st := s.Swap(context.Background(), 9201, newImg); st.State != "done" {
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)
}
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 !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)
}
}
// The load-bearing failure path: an unhealthy target must roll back to the previous image.
func TestControllerSwap_RollbackOnUnhealthy(t *testing.T) {
fe := &fakeGuestExec{
+13
View File
@@ -3,6 +3,7 @@ package localapi
import (
"context"
"fmt"
"io"
"log/slog"
"strconv"
"strings"
@@ -124,3 +125,15 @@ 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...)
if err != nil {
return string(out), fmt.Errorf("pct exec %d %v: %w: %s", vmid, args, err, strings.TrimSpace(string(stderr)))
}
return string(out), nil
}
+8 -1
View File
@@ -20,6 +20,10 @@ type recRunner struct {
fail string // if a command's name == fail, return an error
}
func (r *recRunner) RunStdin(ctx context.Context, _ io.Reader, name string, args ...string) ([]byte, []byte, error) {
return r.Run(ctx, name, args...)
}
func (r *recRunner) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) {
r.mu.Lock()
r.cmds = append(r.cmds, append([]string{name}, args...))
@@ -70,7 +74,10 @@ type mintMinter struct {
vmids []int
}
func (m *mintMinter) Mint(vmid int) (string, error) { m.vmids = append(m.vmids, vmid); return m.token, nil }
func (m *mintMinter) Mint(vmid int) (string, error) {
m.vmids = append(m.vmids, vmid)
return m.token, nil
}
func testLogger() *slog.Logger { return slog.New(slog.NewTextHandler(io.Discard, nil)) }
+4
View File
@@ -42,6 +42,10 @@ type mockRunner struct {
err error
}
func (m *mockRunner) RunStdin(ctx context.Context, _ io.Reader, name string, args ...string) ([]byte, []byte, error) {
return m.Run(ctx, name, args...)
}
func (m *mockRunner) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) {
m.calls++
m.lastCmd = name
+12
View File
@@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
"io"
"os/exec"
"strconv"
)
@@ -26,6 +27,10 @@ import (
// production implementation; tests inject a mock to assert which commands ran.
type Runner interface {
Run(ctx context.Context, name string, args ...string) (stdout, stderr []byte, err error)
// RunStdin is Run with the command's stdin fed from r (nil = no stdin). Used by the
// controller-swap write (image piped into an in-guest `tee`), so the swap needs no shell
// vector. Keeps the sudo-prefix/mode handling in the same place as Run.
RunStdin(ctx context.Context, stdin io.Reader, name string, args ...string) (stdout, stderr []byte, err error)
}
// RunnerMode selects how privileged commands are executed.
@@ -48,6 +53,12 @@ type ExecRunner struct {
// Run implements Runner.
func (r *ExecRunner) Run(ctx context.Context, name string, args ...string) ([]byte, []byte, error) {
return r.RunStdin(ctx, nil, name, args...)
}
// RunStdin is Run with the process stdin fed from stdin (nil = no stdin). The sudo-prefix/mode
// handling is identical to Run — kept here so both paths share one place.
func (r *ExecRunner) RunStdin(ctx context.Context, stdin io.Reader, name string, args ...string) ([]byte, []byte, error) {
var cmd *exec.Cmd
if r.Mode == RunnerSudo {
sudo := r.SudoPath
@@ -59,6 +70,7 @@ func (r *ExecRunner) Run(ctx context.Context, name string, args ...string) ([]by
cmd = exec.CommandContext(ctx, name, args...)
}
var stdout, stderr capBuf
cmd.Stdin = stdin
cmd.Stdout = &stdout
cmd.Stderr = &stderr
err := cmd.Run()
+5
View File
@@ -3,6 +3,7 @@ package storage
import (
"context"
"errors"
"io"
"strings"
"testing"
)
@@ -14,6 +15,10 @@ type scriptedRunner struct {
errs map[string]error
}
func (r *scriptedRunner) RunStdin(ctx context.Context, _ io.Reader, name string, args ...string) ([]byte, []byte, error) {
return r.Run(ctx, name, args...)
}
func (r *scriptedRunner) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) {
r.calls = append(r.calls, append([]string{name}, args...))
base := name
+5
View File
@@ -2,6 +2,7 @@ package storage
import (
"context"
"io"
"os"
"path/filepath"
"strings"
@@ -18,6 +19,10 @@ type scriptRunner struct {
err error
}
func (s *scriptRunner) RunStdin(ctx context.Context, _ io.Reader, name string, args ...string) ([]byte, []byte, error) {
return s.Run(ctx, name, args...)
}
func (s *scriptRunner) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) {
s.calls = append(s.calls, append([]string{name}, args...))
return s.out[name], nil, s.err
+4 -4
View File
@@ -38,10 +38,10 @@ func TestSystemDisks_FromBootMounts(t *testing.T) {
func TestRoleForStorage_DemoMapping(t *testing.T) {
sys, ok := SystemDisks(demoHost())
cases := []struct {
name string
typ string
device string
want DeviceRole
name string
typ string
device string
want DeviceRole
}{
{"builtin local (root fs)", hub.StorageTypeLocal, "", RoleSystem},
{"local-lvm (lvmthin)", hub.StorageTypeLVMThin, "", RoleSystem},
+5
View File
@@ -2,6 +2,7 @@ package storage
import (
"context"
"io"
"strings"
"testing"
@@ -16,6 +17,10 @@ type recordingRunner struct {
err error
}
func (r *recordingRunner) RunStdin(ctx context.Context, _ io.Reader, name string, args ...string) ([]byte, []byte, error) {
return r.Run(ctx, name, args...)
}
func (r *recordingRunner) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) {
r.calls = append(r.calls, append([]string{name}, args...))
return nil, nil, r.err