R-204 item 4 (box half): a rebuilt box DECLARES that it needs a credential (v0.199.0)

An absent off-site object has four meanings — never configured, mid-restart, a
transient config read failure, and rebuilt-and-stranded — and the hub cannot tell
them apart. The box can, from two local facts it holds with certainty, so it says
so instead of leaving the hub to deduce it from a silence (operator ruling).

The ACK's identity_blob_present is now recorded on EVERY ACK, before the gates
that used to discard it: on a box with no off-site target the auto-confirm returns
immediately, which is exactly a rebuilt box, so the one fact distinguishing it from
a box that never had off-site backups was thrown away every cycle.

The declaration needs BOTH halves — a fresh data area AND a hub-held recovery
package. Freshness alone is a box that never had off-site backups; dropping that
condition makes the whole fleet ask for credentials, which is what the Scenario B
test exists to catch.

The object carries enabled:false and zero sizes, which is what makes it inert to
the hub's existing fill and staleness checkers and to a pre-upgrade hub. A
configured box's JSON is byte-identical to v0.198.0's.

controller/internal/backup/offbox_declare_test.go

@@ -0,0 +1,156 @@
+package backup
+
+import (
+	"encoding/json"
+	"io"
+	"log"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"gitea.dooplex.hu/admin/felhom-controller/internal/config"
+	"gitea.dooplex.hu/admin/felhom-controller/internal/settings"
+)
+
+// R-204 item 4 / R-193 — a REBUILT box declares that it needs an off-site credential, instead of
+// reporting an absence the hub cannot interpret.
+//
+// THE POINT OF THESE TESTS is the conjunction. An absent off-site object has FOUR meanings (never
+// configured / mid-restart / a transient read failure / rebuilt-and-stranded). The declaration has
+// one, and it is only sound because BOTH halves are required: a fresh data area AND a hub-held
+// recovery package. Scenario B is the one that matters most — drop the escrow half and every
+// un-configured box in the fleet starts asking for a credential.
+
+// bareManager builds a Manager with NO off-site target and NO repository password — the shape of a
+// freshly rebuilt box before anything is configured.
+func bareManager(t *testing.T) (*Manager, *settings.Settings) {
+	t.Helper()
+	lg := log.New(io.Discard, "", 0)
+	dataDir := t.TempDir()
+	sett, err := settings.Load(filepath.Join(dataDir, "settings.json"), lg)
+	if err != nil {
+		t.Fatal(err)
+	}
+	cfg := &config.Config{}
+	cfg.Paths.DataDir = dataDir
+	cfg.Paths.SystemDataPath = filepath.Join(dataDir, "sys")
+	return NewManager(cfg, sett, lg), sett
+}
+
+// SCENARIO A — a rebuilt box (fresh data area + a hub-held escrow) DECLARES the state.
+//
+// RED-PROOF: remove the `GetHubEscrowIdentityPresent()` condition from needsOffsiteCredential —
+// Scenario A still passes (it has an escrow), and SCENARIO B FAILS, which is the point: the plausible
+// wrong fix is to declare on freshness alone, and that would make every un-configured box in the
+// fleet ask for a credential.
+func TestOffsiteDeclare_RebuiltBoxDeclaresNeedsCredential(t *testing.T) {
+	m, sett := bareManager(t)
+	if err := sett.SetHubEscrowIdentityPresent(true); err != nil {
+		t.Fatal(err)
+	}
+
+	st := m.OffboxReportStatus()
+	if st == nil {
+		t.Fatal("a rebuilt box reported NO off-site object — the hub cannot distinguish it from a box that never had off-site backups (this is the defect)")
+	}
+	if st.State != OffsiteStateNeedsCredential {
+		t.Fatalf("declared state = %q, want %q", st.State, OffsiteStateNeedsCredential)
+	}
+	// Enabled MUST be false and the sizes zero — that is what makes the declaration inert to the
+	// hub's existing fill and staleness checkers (and to a pre-upgrade hub).
+	if st.Enabled {
+		t.Error("a declaration must not claim the tier is enabled — the hub's staleness check keys on it")
+	}
+	if st.QuotaGB != 0 || st.RepoSizeBytes != 0 || st.SnapshotCount != 0 {
+		t.Errorf("a declaration must carry zero sizes (fill band keys on them): %+v", st)
+	}
+	// And it must be on the off-site object, not a new top-level field.
+	b, err := json.Marshal(st)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(string(b), `"state":"needs_credential"`) {
+		t.Fatalf("declared state absent from the marshalled off-site object: %s", b)
+	}
+	if !strings.Contains(string(b), `"enabled":false`) {
+		t.Fatalf("marshalled object must carry enabled:false: %s", b)
+	}
+}
+
+// SCENARIO B — a box that never had off-site backups says NOTHING. This is the guard on the
+// conjunction; without it the feature churns credentials fleet-wide.
+func TestOffsiteDeclare_NeverHadOffsiteSaysNothing(t *testing.T) {
+	m, _ := bareManager(t) // fresh data area, but NO hub-held escrow
+
+	if st := m.OffboxReportStatus(); st != nil {
+		t.Fatalf("a box that never had off-site backups DECLARED a need: %+v — every un-configured box in the fleet would now ask for a credential", st)
+	}
+}
+
+// The other half of the conjunction: a box that still holds its repository password is NOT stranded,
+// even though the hub holds an escrow for it. That is simply a healthy box between configurations.
+func TestOffsiteDeclare_BoxThatStillHoldsItsRepoPasswordDoesNotDeclare(t *testing.T) {
+	m, sett := bareManager(t)
+	if err := sett.SetHubEscrowIdentityPresent(true); err != nil {
+		t.Fatal(err)
+	}
+	// Place a repository password exactly where the manager looks for it.
+	if err := os.MkdirAll(m.offboxDir(), 0o700); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(m.offboxPwPath(), []byte("a-repository-password"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+
+	if st := m.OffboxReportStatus(); st != nil {
+		t.Fatalf("a box holding its repository password declared a need: %+v", st)
+	}
+}
+
+// A DISABLED target is the customer's own choice, not a rebuild — it must not declare either.
+func TestOffsiteDeclare_DisabledTargetIsNotStranded(t *testing.T) {
+	m, sett := bareManager(t)
+	if err := sett.SetHubEscrowIdentityPresent(true); err != nil {
+		t.Fatal(err)
+	}
+	if err := sett.SetOffboxTarget(&settings.OffboxTarget{
+		Enabled: false, Host: "nas.local", Port: 22, User: "felhom", RepoPath: "/srv/repo",
+	}); err != nil {
+		t.Fatal(err)
+	}
+
+	if st := m.OffboxReportStatus(); st != nil {
+		t.Fatalf("a deliberately DISABLED target declared a need: %+v", st)
+	}
+}
+
+// A CONFIGURED box's report object must be byte-identical to v0.198.0's — no `state` key at all.
+// This is what lets a pre-upgrade hub and every existing checker read the fleet unchanged.
+func TestOffsiteDeclare_ConfiguredBoxJSONIsUnchanged(t *testing.T) {
+	m, sett := bareManager(t)
+	if err := sett.SetOffboxTarget(&settings.OffboxTarget{
+		Enabled: true, Host: "nas.local", Port: 22, User: "felhom", RepoPath: "/srv/repo",
+		Schedule: "daily", EscrowState: "escrowed", LastStatus: "ok",
+	}); err != nil {
+		t.Fatal(err)
+	}
+
+	st := m.OffboxReportStatus()
+	if st == nil {
+		t.Fatal("a configured box must still report an off-site object")
+	}
+	if st.State != "" {
+		t.Errorf("a configured box must declare NO state, got %q", st.State)
+	}
+	b, err := json.Marshal(st)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strings.Contains(string(b), `"state"`) {
+		t.Fatalf("a healthy report's JSON gained a `state` key — it must stay byte-compatible: %s", b)
+	}
+	if !strings.Contains(string(b), `"enabled":true`) {
+		t.Fatalf("a configured box must report enabled:true: %s", b)
+	}
+}