R-414: the fallback scratch must also be DELETABLE - caught by live validation
gates / gates (push) Successful in 12s

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.
This commit is contained in:
2026-09-01 10:29:08 +02:00
parent fcef8e069c
commit 8b55de734c
2 changed files with 58 additions and 1 deletions
+14 -1
View File
@@ -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 {
@@ -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)
}
}