v0.80.0: PBS DR tier slice 2 — the apply-bridge (pbs_dr consumer, felhom-pbs-apply set-only wrapper, verify-pin-before-consume, adoption-first, loud consumed-failed, escrow seed)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6
2026-07-10 21:50:36 +02:00
parent a6e8bcb475
commit e5e8f3920a
15 changed files with 1309 additions and 2 deletions
@@ -0,0 +1,402 @@
+package pbsdr
+
+// The apply-bridge laws, each pinned by a non-hollow test (fake exec recorder — no docker/pct/
+// real /dev, the REUSE §4 doctrine): set-only re-apply, secret-on-stdin-never-argv,
+// verify-pin-BEFORE-consume, non-destructive adoption (no consume, tenancy entry-owned),
+// consumed-but-failed loud + recover-only-via-fresh-secret, marker idempotency, old-hub compat.
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+	"log/slog"
+	"os"
+	"path/filepath"
+	"regexp"
+	"strings"
+	"sync"
+	"testing"
+
+	"gitea.dooplex.hu/admin/felhom-agent/internal/hub"
+	"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
+)
+
+// recordedCall is one exec through the runner seam — argv AND the stdin bytes.
+type recordedCall struct {
+	Name  string
+	Args  []string
+	Stdin string
+}
+
+type fakeRunner struct {
+	mu    sync.Mutex
+	calls []recordedCall
+	// failVerb → error for calls whose first arg matches (e.g. "create").
+	failVerb string
+	failErr  error
+}
+
+func (f *fakeRunner) Run(ctx context.Context, name string, args ...string) ([]byte, []byte, error) {
+	return f.RunStdin(ctx, nil, name, args...)
+}
+
+func (f *fakeRunner) RunStdin(_ context.Context, stdin io.Reader, name string, args ...string) ([]byte, []byte, error) {
+	var in []byte
+	if stdin != nil {
+		in, _ = io.ReadAll(stdin)
+	}
+	f.mu.Lock()
+	f.calls = append(f.calls, recordedCall{Name: name, Args: args, Stdin: string(in)})
+	f.mu.Unlock()
+	if f.failVerb != "" && len(args) > 0 && args[0] == f.failVerb {
+		return nil, []byte("boom-stderr"), f.failErr
+	}
+	return nil, nil, nil
+}
+
+func (f *fakeRunner) recorded() []recordedCall {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	return append([]recordedCall(nil), f.calls...)
+}
+
+type fakeStorage struct {
+	entry  *proxmox.StorageEntryConfig
+	found  bool
+	active []bool // consumed per StorageActive call; last value repeats
+	calls  int
+}
+
+func (f *fakeStorage) StorageEntry(context.Context, string) (*proxmox.StorageEntryConfig, bool, error) {
+	return f.entry, f.found, nil
+}
+
+func (f *fakeStorage) StorageActive(context.Context, string) (bool, error) {
+	i := f.calls
+	f.calls++
+	if i >= len(f.active) {
+		if len(f.active) == 0 {
+			return false, nil
+		}
+		return f.active[len(f.active)-1], nil
+	}
+	return f.active[i], nil
+}
+
+type fakeConsumer struct {
+	secret string
+	err    error
+	calls  int
+}
+
+func (f *fakeConsumer) ConsumePBSToken(context.Context) (string, error) {
+	f.calls++
+	if f.err != nil {
+		return "", f.err
+	}
+	return f.secret, nil
+}
+
+const testFP = "c6:07:28:3f:5b:7b:5a:41:90:28:d7:ca:4f:37:14:70:56:39:2e:2f:0b:71:e8:06:ca:60:4a:d5:56:5f:3c:fd"
+
+func testBlock() *hub.WirePBSDR {
+	return &hub.WirePBSDR{
+		Enabled: true, StorageID: "felhom-pbs", PBSTunnelIP: "10.77.0.1",
+		Datastore: "felhom-offsite", Namespace: "peti", TokenID: "felhom@pbs!peti",
+		Fingerprint: testFP,
+	}
+}
+
+// newTestManager: fakes everywhere; the fingerprint probe defaults to PASS (override per test);
+// a real temp agent.json so the escrow seed is asserted end-to-end.
+func newTestManager(t *testing.T, r *fakeRunner, st *fakeStorage, c *fakeConsumer) (*Manager, string) {
+	t.Helper()
+	dir := t.TempDir()
+	cfgPath := filepath.Join(dir, "agent.json")
+	if err := os.WriteFile(cfgPath, []byte(`{"log_level":"info","escrow":{"posture":"zero_knowledge"},"custom_unknown":{"keep":1}}`), 0o600); err != nil {
+		t.Fatalf("seed config: %v", err)
+	}
+	m := NewManager(r, st, c, dir, "/etc/pve/priv/storage", cfgPath,
+		slog.New(slog.NewTextHandler(io.Discard, nil)))
+	m.probeFP = func(context.Context, string, string) error { return nil }
+	return m, cfgPath
+}
+
+func TestFreshPath_SecretOnStdinNeverArgv(t *testing.T) {
+	r := &fakeRunner{}
+	st := &fakeStorage{found: false, active: []bool{true}} // post-apply probe active
+	c := &fakeConsumer{secret: "SUPER-SECRET"}
+	m, cfgPath := newTestManager(t, r, st, c)
+
+	m.Apply(context.Background(), true, testBlock())
+
+	calls := r.recorded()
+	if len(calls) != 2 || calls[0].Args[0] != "create" || calls[1].Args[0] != "grant" {
+		t.Fatalf("calls = %+v, want [create, grant]", calls)
+	}
+	// THE STDIN LAW: the secret appears in NO argv, ONLY on the create call's stdin.
+	// (Red-proof: pass it as an argument → this fails with the secret visible in Args.)
+	for _, call := range calls {
+		for _, a := range call.Args {
+			if strings.Contains(a, "SUPER-SECRET") {
+				t.Fatalf("secret leaked into argv: %v", call.Args)
+			}
+		}
+	}
+	if calls[0].Stdin != "SUPER-SECRET\n" {
+		t.Errorf("create stdin = %q, want the secret + newline", calls[0].Stdin)
+	}
+	if calls[1].Stdin != "" {
+		t.Errorf("grant received stdin %q", calls[1].Stdin)
+	}
+	// Non-secret coords ride argv, descriptor-exact.
+	want := []string{"create", "felhom-pbs", "10.77.0.1", "felhom-offsite", "peti", "felhom@pbs!peti", testFP, "/etc/pve/priv/storage"}
+	if fmt.Sprint(calls[0].Args) != fmt.Sprint(want) {
+		t.Errorf("create argv = %v, want %v", calls[0].Args, want)
+	}
+	if c.calls != 1 {
+		t.Errorf("consume calls = %d, want 1", c.calls)
+	}
+	// Converged: marker applied + escrow seeded + unknown config keys preserved.
+	if s := m.Status(); s == nil || s.State != "applied" {
+		t.Fatalf("status = %+v, want applied", s)
+	}
+	raw, _ := os.ReadFile(cfgPath)
+	var doc map[string]json.RawMessage
+	json.Unmarshal(raw, &doc)
+	var esc map[string]string
+	json.Unmarshal(doc["escrow"], &esc)
+	if esc["pbs_storage_id"] != "felhom-pbs" {
+		t.Errorf("escrow not seeded: %s", doc["escrow"])
+	}
+	if esc["posture"] != "zero_knowledge" {
+		t.Errorf("existing escrow fields clobbered: %s", doc["escrow"])
+	}
+	if _, ok := doc["custom_unknown"]; !ok {
+		t.Error("unknown config key dropped by the seed write")
+	}
+	if strings.Contains(string(raw), "SUPER-SECRET") {
+		t.Error("secret leaked into agent.json")
+	}
+}
+
+func TestVerifyPinBeforeConsume(t *testing.T) {
+	r := &fakeRunner{}
+	st := &fakeStorage{found: false}
+	c := &fakeConsumer{secret: "S"}
+	m, _ := newTestManager(t, r, st, c)
+	m.probeFP = func(context.Context, string, string) error { return errors.New("pin mismatch") }
+
+	m.Apply(context.Background(), true, testBlock())
+
+	// THE ORDERING LAW: a failing pre-consume verify means NOTHING consumed, NOTHING executed.
+	// (Red-proof: reorder consume before the probe → calls=1 → FAIL.)
+	if c.calls != 0 {
+		t.Fatalf("consume calls = %d, want 0 — the secret was touched before the fingerprint verify", c.calls)
+	}
+	if len(r.recorded()) != 0 {
+		t.Fatalf("runner calls = %+v, want none", r.recorded())
+	}
+	if s := m.Status(); s == nil || s.State != "verify_failed" {
+		t.Fatalf("status = %+v, want verify_failed", s)
+	}
+}
+
+func TestAdoption_HealthyEntryNoConsumeTenancyEntryOwned(t *testing.T) {
+	r := &fakeRunner{}
+	st := &fakeStorage{
+		found: true,
+		entry: &proxmox.StorageEntryConfig{Storage: "felhom-offsite", Type: "pbs",
+			Server: "10.77.0.1", Datastore: "felhom-offsite", Namespace: "demo-felhom-01",
+			Username: "felhom@pbs!demo-felhom-01", Fingerprint: testFP},
+		active: []bool{true},
+	}
+	c := &fakeConsumer{secret: "STAGED-BUT-MUST-STAY"}
+	m, _ := newTestManager(t, r, st, c)
+
+	block := testBlock()
+	block.StorageID = "felhom-offsite"
+	block.Namespace = "demo" // the hub-provisioned tenancy differs — the entry must WIN
+	m.Apply(context.Background(), true, block)
+
+	if c.calls != 0 {
+		t.Fatalf("adoption consumed the staged secret (%d calls) — the no-consume law", c.calls)
+	}
+	calls := r.recorded()
+	if len(calls) != 1 || calls[0].Args[0] != "grant" {
+		t.Fatalf("adoption calls = %+v, want exactly [grant]", calls)
+	}
+	s := m.Status()
+	if s == nil || s.State != "adopted" {
+		t.Fatalf("status = %+v, want adopted", s)
+	}
+	if s.Namespace != "demo-felhom-01" {
+		t.Errorf("reported namespace = %q, want the ENTRY's (demo-felhom-01) — tenancy is entry-owned", s.Namespace)
+	}
+	if !strings.Contains(s.Message, "entry wins") {
+		t.Errorf("adoption note missing from message: %q", s.Message)
+	}
+}
+
+// TestSetOnlyLaw pins the data-loss-class guard twice over: (1) the re-apply path over an
+// existing entry uses `reconcile` (pvesm set) — never create, never any deletion verb; (2) the
+// wrapper script itself contains no deletion path (grep gate over the shipped file).
+// Red-proof: introduce a remove+re-add path → the recorded verbs change → FAIL.
+func TestSetOnlyLaw(t *testing.T) {
+	r := &fakeRunner{}
+	st := &fakeStorage{
+		found: true,
+		entry: &proxmox.StorageEntryConfig{Storage: "felhom-pbs", Type: "pbs", Namespace: "peti", Fingerprint: testFP},
+		// unhealthy → recovery path (verify → consume → reconcile) → post-apply healthy
+		active: []bool{false, true},
+	}
+	c := &fakeConsumer{secret: "FRESH"}
+	m, _ := newTestManager(t, r, st, c)
+
+	m.Apply(context.Background(), true, testBlock())
+
+	calls := r.recorded()
+	if len(calls) != 2 || calls[0].Args[0] != "reconcile" || calls[1].Args[0] != "grant" {
+		t.Fatalf("re-apply calls = %+v, want [reconcile, grant] (set-only)", calls)
+	}
+	deletionish := regexp.MustCompile(`remove|delete|destroy`)
+	for _, call := range calls {
+		for _, a := range call.Args {
+			if deletionish.MatchString(a) {
+				t.Fatalf("deletion-class verb reached the wrapper: %v — K destruction path", call.Args)
+			}
+		}
+	}
+	if calls[0].Stdin != "FRESH\n" {
+		t.Errorf("reconcile stdin = %q, want the fresh secret (re-issue recovery)", calls[0].Stdin)
+	}
+
+	// (2) the wrapper file: no deletion path, grep-assertable (the spike's set-only law).
+	wrapper, err := os.ReadFile(filepath.Join("..", "..", "configs", "felhom-pbs-apply"))
+	if err != nil {
+		t.Fatalf("read wrapper: %v", err)
+	}
+	if regexp.MustCompile(`pvesm (remove|delete)`).Match(wrapper) {
+		t.Fatal("configs/felhom-pbs-apply contains a pvesm deletion verb — the set-only law is dead")
+	}
+	if regexp.MustCompile(`rm\s+.*\.enc`).Match(wrapper) {
+		t.Fatal("configs/felhom-pbs-apply deletes a .enc file — K destruction path")
+	}
+}
+
+func TestConsumedButFailed_LoudAndRecoversOnlyViaFreshSecret(t *testing.T) {
+	r := &fakeRunner{failVerb: "create", failErr: errors.New("pvesm add exploded")}
+	st := &fakeStorage{found: false}
+	c := &fakeConsumer{secret: "BURNED"}
+	m, _ := newTestManager(t, r, st, c)
+	block := testBlock()
+
+	// 1. consume + create fails → LOUD persistent state.
+	m.Apply(context.Background(), true, block)
+	s := m.Status()
+	if s == nil || s.State != "consumed_failed" || !s.ConsumedFailed {
+		t.Fatalf("status = %+v, want consumed_failed", s)
+	}
+	if _, err := os.Stat(m.consumedFailedPath()); err != nil {
+		t.Fatal("consumed-failed state file not written")
+	}
+
+	// 2. next ticks WITHOUT a fresh secret: no silent recovery, state stays loud.
+	c.err = hub.ErrNoPBSSecret
+	m.Apply(context.Background(), true, block)
+	if s := m.Status(); s == nil || s.State != "consumed_failed" {
+		t.Fatalf("status after no-secret retry = %+v, want consumed_failed (never quiet waiting)", s)
+	}
+
+	// 3. operator re-issue staged a FRESH secret → the bridge recovers on the next tick.
+	c.err = nil
+	c.secret = "FRESH-AFTER-REISSUE"
+	r.failVerb = ""
+	st.active = []bool{true}
+	m.Apply(context.Background(), true, block)
+	if s := m.Status(); s == nil || s.State != "applied" {
+		t.Fatalf("status after re-issue = %+v, want applied", s)
+	}
+	if _, err := os.Stat(m.consumedFailedPath()); !os.IsNotExist(err) {
+		t.Error("consumed-failed state not cleared after recovery")
+	}
+}
+
+func TestMarkerIdempotency(t *testing.T) {
+	r := &fakeRunner{}
+	st := &fakeStorage{found: false, active: []bool{true}}
+	c := &fakeConsumer{secret: "S"}
+	m, _ := newTestManager(t, r, st, c)
+	block := testBlock()
+
+	m.Apply(context.Background(), true, block)
+	n := len(r.recorded())
+	m.Apply(context.Background(), true, block) // same descriptor hash → pure no-op
+	if len(r.recorded()) != n || c.calls != 1 {
+		t.Fatalf("re-apply over an unchanged descriptor ran ops (calls %d→%d, consume %d)", n, len(r.recorded()), c.calls)
+	}
+	if s := m.Status(); s == nil || s.State != "applied" || s.AppliedAt == "" {
+		t.Fatalf("status = %+v, want applied with applied_at", s)
+	}
+}
+
+func TestOldHubAndDisabledCompat(t *testing.T) {
+	r := &fakeRunner{}
+	c := &fakeConsumer{secret: "S"}
+	m, _ := newTestManager(t, r, &fakeStorage{}, c)
+
+	m.Apply(context.Background(), false, nil) // no desired data yet
+	m.Apply(context.Background(), true, nil)  // pre-slice-1 hub: no pbs_dr key
+	if len(r.recorded()) != 0 || c.calls != 0 || m.Status() != nil {
+		t.Fatalf("nil-block Apply had effects (runner %d, consume %d, status %+v)", len(r.recorded()), c.calls, m.Status())
+	}
+
+	m.Apply(context.Background(), true, &hub.WirePBSDR{Enabled: false, StorageID: "felhom-pbs"})
+	if len(r.recorded()) != 0 || c.calls != 0 {
+		t.Fatal("disabled descriptor ran ops (teardown is not this slice)")
+	}
+	if s := m.Status(); s == nil || s.State != "disabled" {
+		t.Fatalf("status = %+v, want disabled", s)
+	}
+}
+
+// TestWireFieldNames pins the cross-repo descriptor contract (hub/internal/web/pbsdr.go
+// pbsDRDescriptor): the exact JSON the hub writes must land in WirePBSDR field-for-field.
+func TestWireFieldNames(t *testing.T) {
+	hubJSON := `{"desired_state":{"guests":[],"pbs_dr":{"enabled":true,"storage_id":"felhom-pbs",` +
+		`"pbs_tunnel_ip":"10.77.0.1","datastore":"felhom-offsite","namespace":"peti",` +
+		`"token_id":"felhom@pbs!peti","fingerprint":"aa:bb"}},"generation":7}`
+	var resp hub.DesiredStateResponse
+	if err := json.Unmarshal([]byte(hubJSON), &resp); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	b := resp.DesiredState.PBSDR
+	if b == nil || !b.Enabled || b.StorageID != "felhom-pbs" || b.PBSTunnelIP != "10.77.0.1" ||
+		b.Datastore != "felhom-offsite" || b.Namespace != "peti" ||
+		b.TokenID != "felhom@pbs!peti" || b.Fingerprint != "aa:bb" {
+		t.Fatalf("wire mapping wrong: %+v", b)
+	}
+}
+
+func TestEscrowSeed_NeverClobbersDifferentValue(t *testing.T) {
+	r := &fakeRunner{}
+	st := &fakeStorage{found: false, active: []bool{true}}
+	c := &fakeConsumer{secret: "S"}
+	m, cfgPath := newTestManager(t, r, st, c)
+	os.WriteFile(cfgPath, []byte(`{"escrow":{"pbs_storage_id":"operator-set-id"}}`), 0o600)
+
+	m.Apply(context.Background(), true, testBlock())
+
+	raw, _ := os.ReadFile(cfgPath)
+	var doc struct {
+		Escrow struct {
+			PBSStorageID string `json:"pbs_storage_id"`
+		} `json:"escrow"`
+	}
+	json.Unmarshal(raw, &doc)
+	if doc.Escrow.PBSStorageID != "operator-set-id" {
+		t.Fatalf("an operator-set escrow.pbs_storage_id was clobbered: %s", raw)
+	}
+}