From 8b55de734c3921348be43e587e15c1a64e604b87 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Tue, 1 Sep 2026 10:29:08 +0200 Subject: [PATCH] R-414: the fallback scratch must also be DELETABLE - caught by live validation The system-data fallback resolved a scratch fine and removeProofScratch then refused to delete it: its accepted-roots list is built from REGISTERED drives, and a driveless box has none. Observed on demo-felhom: 'refusing to remove ... it is not inside a proof root', with the copy still on disk. Every nightly proof would have left one behind, growing forever, on exactly the boxes the fallback exists for. My defect, introduced with the fallback in the same session. The unit tests missed it because every one of them registers a drive; the new pair deliberately does not, and the second asserts the guard still REFUSES a path outside every proof root, so the fix is not a widening into uselessness. --- controller/internal/backup/offbox_proof.go | 15 ++++++- .../internal/backup/r414_reachability_test.go | 44 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/controller/internal/backup/offbox_proof.go b/controller/internal/backup/offbox_proof.go index 170a627..70ee995 100644 --- a/controller/internal/backup/offbox_proof.go +++ b/controller/internal/backup/offbox_proof.go @@ -338,7 +338,20 @@ func (m *Manager) restoreUnitReadOnly(ctx context.Context, stack, id, unitPath, // out-of-sandbox path means a helper above is wrong and a best-effort skip would hide that. func (m *Manager) removeProofScratch(stack, scratch string) { clean := filepath.Clean(scratch) - for _, drive := range m.offsiteRestoreDriveRoots() { + // R-414: the accepted roots must include the SYSTEM DATA PATH, because that is now where a + // unit-only scratch lands on a box with no registered drive. + // + // CAUGHT BY LIVE VALIDATION ON demo-felhom, 2026-09-01, and it was a leak I introduced with the + // fallback itself: `offsiteRestoreDriveRoots` is built from registered drives and schedulable + // paths, so on a driveless box it is EMPTY — the scratch resolved fine and then this refused to + // delete it ("refusing to remove … it is not inside a proof root"). Every nightly proof would have + // left a copy behind, growing forever, on exactly the boxes the fallback exists for. The unit + // tests did not see it because they register a drive. + roots := append(m.offsiteRestoreDriveRoots(), strings.TrimSpace(m.cfg.Paths.SystemDataPath)) + for _, drive := range roots { + if strings.TrimSpace(drive) == "" { + continue + } root := filepath.Clean(m.offsiteProofRootFor(drive)) + string(filepath.Separator) if strings.HasPrefix(clean+string(filepath.Separator), root) { if err := os.RemoveAll(clean); err != nil { diff --git a/controller/internal/backup/r414_reachability_test.go b/controller/internal/backup/r414_reachability_test.go index 69ca775..a1a4bcf 100644 --- a/controller/internal/backup/r414_reachability_test.go +++ b/controller/internal/backup/r414_reachability_test.go @@ -3,6 +3,7 @@ package backup import ( "context" "encoding/json" + "os" "path/filepath" "strings" "testing" @@ -197,3 +198,46 @@ func TestR414_NoCustomerAlarm(t *testing.T) { t.Fatalf("a driveless box must raise no event from the backup layer; got %d", pushed) } } + +// TestR414_ProofScratchIsDeletedOnADrivelessBox — the leak live validation caught. +// +// The fallback resolved a scratch on the system data path, and `removeProofScratch` then refused to +// delete it because its accepted-roots list is built from REGISTERED drives, which a driveless box has +// none of. Observed on demo-felhom 2026-09-01: *"refusing to remove … it is not inside a proof root"*, +// with the copy still on disk. Every nightly proof would have left one behind. +// +// The unit tests did not catch it because they all register a drive. This one deliberately does not. +func TestR414_ProofScratchIsDeletedOnADrivelessBox(t *testing.T) { + m, _, _ := drivelessHarness(t, "opengist") + + scratch, _, err := m.offboxProofScratchDir("opengist") + if err != nil { + t.Fatalf("the unit-only scratch must resolve: %v", err) + } + if err := os.MkdirAll(filepath.Join(scratch, "marker"), 0o755); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(scratch); err != nil { + t.Fatalf("fixture: the scratch must exist before the removal is attempted: %v", err) + } + + m.removeProofScratch("opengist", scratch) + + if _, err := os.Stat(scratch); !os.IsNotExist(err) { + t.Fatalf("the proof copy must be removed on a driveless box too; %s still exists (stat err=%v)", scratch, err) + } +} + +// TestR414_RemovalStillRefusesOutsideAProofRoot — the guard must not be widened into uselessness. +func TestR414_RemovalStillRefusesOutsideAProofRoot(t *testing.T) { + m, _, _ := drivelessHarness(t, "opengist") + outside := t.TempDir() + keep := filepath.Join(outside, "not-a-proof-root") + if err := os.MkdirAll(keep, 0o755); err != nil { + t.Fatal(err) + } + m.removeProofScratch("opengist", keep) + if _, err := os.Stat(keep); err != nil { + t.Fatalf("a path outside every proof root must be REFUSED, not deleted: %v", err) + } +}