Second-drive copy: two races closed — the start-up/retry removal holds the destination's lock (a copy this process starts is never removed), and a run records itself in one step with the restore check, every leg asking again; red-proved, race-clean; unreleased

CHANGELOG.md

@@ -30,7 +30,13 @@
   background retry for strays and a failed start-up check; tests switch it off (`tier2RetryOff` — a timer outliving a test
   would reach the host's real Docker). N9: the R-1187 (c) refusal no longer advises removing an unhealthy app (its data
   may be newer than the bundle) — „kérd az üzemeltető segítségét". Tests `TestStrayCopy_AStrayClears…`, `…KeepsThis
-  ProcessesOwnCopy`, `…SameDestinationWaits`, `TestR1180_ADirectRunTier2…`, each red-proved; `-race` clean. Tests `TestStrayCopy_*` (backup, web,
+  ProcessesOwnCopy`, `…SameDestinationWaits`, `TestR1180_ADirectRunTier2…`, each red-proved; `-race` clean.
+- **Two races closed (the commit's own security review).** The start-up check lists the labelled containers, then removes
+  them — a copy of the same destination could start between: each removal now holds that destination's lock (TryLock;
+  held → this process copies there → skipped). `RunTier2` checked for a restore, then recorded the copy later — a restore
+  beginning between saw nothing: the check and the record are now one step under the restore record's lock
+  (`beginTier2Run`), and every leg asks again before it copies. Tests `TestStrayCopy_TheStartUpCheckSkipsADestination
+  BeingCopied`, `TestR1180_ARestoreBeginningMidRunStopsTheNextLeg`, `TestR1180_ARunInFlightBlocks…`, each red-proved. Tests `TestStrayCopy_*` (backup, web,
   wiring; Docker-free seams), each red-proved; 9202 proof in felhom.eu `audits/night-2026-10-23/P1-straycopy/`.
 
 ## Unreleased — the second-drive copy no longer squeezes the apps' memory (R-1214), 2026-10-22 night

controller/internal/backup/r1180_mirror_during_restore_test.go

@@ -80,3 +80,46 @@ func TestR1180_ADirectRunTier2NeverRunsDuringARestore(t *testing.T) {
 	}
 	_ = target
 }
+
+// Commit security review 2026-10-23 night: a restore may begin while a run is already past its first check. The run
+// records itself in one step under the restore record's lock, and every leg asks again. RED-PROOF: with the per-leg
+// check removed, ARestoreBeginningMidRun fails (the photo leg is mirrored over the restore) — seen failing 2026-10-23.
+func TestR1180_ARestoreBeginningMidRunStopsTheNextLeg(t *testing.T) {
+	m, src, _, prov := newTier2V2(t, "app")
+	prov.has["app"] = true
+	prov.binds["app"] = []ClassifiedBind{mHDD("appdata/app/photos")}
+	m.stackProvider = r1180Provider{prov}
+	var dsts []string
+	m.tier2Mirror = func(s, d string) error {
+		dsts = append(dsts, d)
+		if len(dsts) == 1 {
+			m.BeginRestoreOp("tier2-whole-restore", "app") // the household starts a restore during the unit leg
+		}
+		return rsyncMirror(s, d)
+	}
+	mustWrite(t, filepath.Join(src, "appdata", "app", "photos", "a.jpg"), "a")
+	if err := m.RunTier2("app"); err != nil {
+		t.Fatal(err)
+	}
+	if len(dsts) != 1 {
+		t.Fatalf("after a restore began, no further leg may be mirrored: %v", dsts)
+	}
+	if _, blocked := m.Tier2CopyBlocksRestore(); blocked {
+		t.Fatal("the run must have ended its record")
+	}
+}
+
+func TestR1180_ARunInFlightBlocksARestoreAndARestoreBlocksARun(t *testing.T) {
+	m := &Manager{}
+	if !m.beginTier2Run("app") {
+		t.Fatal("no restore: the run must begin")
+	}
+	if _, blocked := m.Tier2CopyBlocksRestore(); !blocked {
+		t.Fatal("a run in flight must refuse a restore")
+	}
+	tier2CopyEnd("app")
+	m.opRunning, m.opStack = true, "app"
+	if m.beginTier2Run("app") {
+		t.Fatal("a restore of the app must refuse the run")
+	}
+}

controller/internal/backup/tier2.go

@@ -342,10 +342,13 @@ func (m *Manager) tier2ReconcileRoots(destBase string, roots, legRels []string)
 func (m *Manager) RunTier2(stackName string) error {
 	// R-1180 for EVERY caller (review 2026-10-23 night, N5): the household's „Tier 2 on" save and the debug run call
 	// this directly, outside RunAllTier2's skip — a mirror (rsync --delete) must never start during a restore of the app.
-	if m.restoreRunningFor(stackName) {
+	// The check and the copy's record are one step under m.mu (commit security review 2026-10-23 night): a restore that
+	// begins after this point sees the copy (Tier2CopyBlocksRestore), and every leg below asks again before it copies.
+	if !m.beginTier2Run(stackName) {
 		m.logger.Printf("[WARN] [backup] Tier 2 for %s not started — a restore of it is running; it is mirrored at the next run (R-1180)", stackName)
 		return nil
 	}
+	defer tier2CopyEnd(stackName)
 	// Customer turned Tier 2 off for this app (config panel) — skip without touching status.
 	if m.settings != nil {
 		if cd := m.settings.GetCrossDriveConfig(stackName); cd != nil && cd.UserDisabled {
@@ -443,6 +446,9 @@ func (m *Manager) RunTier2(stackName string) error {
 		m.logger.Printf("[WARN] [backup] Tier 2 %s: unit leg SKIPPED — the recovery unit on the source drive lists no database dumps and no volume tars, while the existing copy at %s does. The copy was PRESERVED rather than replaced with an empty one (R-403). The other legs continue.",
 			stackName, destUnit)
 		warns = append(warns, m.note(tier2UnitPreservedKey))
+	} else if m.restoreRunningFor(stackName) {
+		m.logger.Printf("[WARN] [backup] Tier 2 %s stopped before the unit leg — a restore of it began (R-1180)", stackName)
+		return nil
 	} else if err := mirror(unitDir, destUnit); err != nil {
 		m.recordTier2Failure(stackName, target, err)
 		if m.tier2Notify != nil {
@@ -460,6 +466,10 @@ func (m *Manager) RunTier2(stackName string) error {
 			warns = append(warns, m.note("note.tier2.emptied_kept", lg.DestRel))
 			continue
 		}
+		if m.restoreRunningFor(stackName) {
+			m.logger.Printf("[WARN] [backup] Tier 2 %s stopped before leg %s — a restore of it began (R-1180)", stackName, lg.DestRel)
+			return nil
+		}
 		if err := mirror(lg.Src, dst); err != nil {
 			m.recordTier2Failure(stackName, target, err)
 			if m.tier2Notify != nil {
@@ -930,11 +940,27 @@ func dirSizeBytes(dir string) int64 {
 	return size
 }
 
+// beginTier2Run records a second-drive run of stack as in flight — unless a restore of it runs — in ONE step under m.mu,
+// the lock the restore record lives under. The caller ends it with tier2CopyEnd(stack).
+func (m *Manager) beginTier2Run(stack string) bool {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	if m.restoreRunningForLocked(stack) {
+		return false
+	}
+	tier2CopyBegin(stack)
+	return true
+}
+
 // restoreRunningFor reports whether a restore of stack is in flight (the restore record the pages follow, R-550).
 // The shares restores are recorded under their display name.
 func (m *Manager) restoreRunningFor(stack string) bool {
 	m.mu.Lock()
 	defer m.mu.Unlock()
+	return m.restoreRunningForLocked(stack)
+}
+
+func (m *Manager) restoreRunningForLocked(stack string) bool {
 	if !m.opRunning {
 		return false
 	}

controller/internal/backup/tier2_copy_lifecycle.go

@@ -232,6 +232,11 @@ func tier2RetryOnce(ctx context.Context) bool {
 
 // tier2RemoveCopyQuiet is the retry's removal: a failure keeps the stray without starting another retry.
 func tier2RemoveCopyQuiet(ctx context.Context, name string) {
+	lk := tier2CopyLock(name)
+	if !lk.TryLock() {
+		return // a copy of that destination runs now: it removes the stray itself before it starts
+	}
+	defer lk.Unlock()
 	rctx, cancel := context.WithTimeout(ctx, 60*time.Second)
 	defer cancel()
 	out, err := tier2Docker(rctx, "rm", "-f", name)
@@ -278,15 +283,23 @@ func clearLeftoverTier2CopiesOnce(ctx context.Context) error {
 		}
 	}
 	tier2Copies.Unlock()
+	// The check above and the removal below are not atomic: a copy of the same destination may start in between. Each
+	// removal holds that destination's lock (TryLock: a copy of it runs in this process → skipped), so it can never hit
+	// a copy this process starts (commit security review 2026-10-23 night).
 	if len(names) == 0 {
 		tier2Logf("[INFO] [tier2] start-up: no leftover second-drive copy container")
 		return nil
 	}
 	failed := 0
 	for _, n := range names {
+		lk := tier2CopyLock(n)
+		if !lk.TryLock() {
+			continue // this process copies into that destination right now — its own container, not a leftover
+		}
 		rctx, rcancel := context.WithTimeout(ctx, 60*time.Second)
 		rout, rerr := tier2Docker(rctx, "rm", "-f", n)
 		rcancel()
+		lk.Unlock()
 		if rerr != nil {
 			failed++
 			tier2Logf("[ERROR] [tier2] start-up: could not remove the leftover copy container %s: %v %s", n, rerr, strings.TrimSpace(string(rout)))

controller/internal/backup/tier2_copy_lifecycle_test.go

@@ -305,3 +305,24 @@ func TestStrayCopy_ASecondCopyOfTheSameDestinationWaits(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
+// Commit security review 2026-10-23 night: the start-up check's list and its removals are not atomic — a copy of the
+// same destination may start between them. A destination this process copies into (its lock held) is skipped.
+// RED-PROOF: with the TryLock skip removed, this fails — seen failing 2026-10-23 night.
+func TestStrayCopy_TheStartUpCheckSkipsADestinationBeingCopied(t *testing.T) {
+	_, life := strayCopySeams(t, t.TempDir()+"/", nil, func(args []string) ([]byte, error) {
+		if args[0] == "ps" {
+			return []byte("felhom-tier2-copy-busy-00000003\n"), nil
+		}
+		return nil, nil
+	})
+	lk := tier2CopyLock("felhom-tier2-copy-busy-00000003")
+	lk.Lock() // a copy of that destination is starting in this process (not yet marked running)
+	defer lk.Unlock()
+	if err := clearLeftoverTier2CopiesOnce(context.Background()); err != nil {
+		t.Fatal(err)
+	}
+	if strings.Contains(life.all(), "rm -f felhom-tier2-copy-busy-00000003") {
+		t.Fatalf("the check removed a container of a destination this process is copying into:\n%s", life.all())
+	}
+}