Onboarding a drive keeps a library on the internal disk (#828)

The onboarding wizard deleted /media/wrolpi/config before mounting a
drive, and the shadowed data check ignored config/.  A library kept on
the internal disk keeps wrolpi.db and its configs there, so onboarding
a drive later deleted them without warning.  Only a config/ holding
nothing but the ssl/ CA is now treated as transient.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

controller/lib/disks.py

@@ -271,15 +271,25 @@ def is_mount_busy(mount_point: str) -> bool:
         return False
 
 
+def is_transient_config(config_dir: Path) -> bool:
+    """True when `config_dir` holds only the `ssl/` CA that `generate_certificates.sh` writes before the primary
+    drive is mounted.  Anything else (wrolpi.db, configs) belongs to a library kept on that disk."""
+    try:
+        return all(e.name == "ssl" for e in config_dir.iterdir())
+    except FileNotFoundError:
+        return True
+    except OSError:
+        return False
+
+
 def check_shadowed_data(target: str) -> Optional[dict]:
     """Look for user data at `target` that would be hidden by a mount over it.
 
-    Returns a dict with `size_bytes` and `entries` when non-config content is
-    present, or None when the target is empty (or contains only `config/`).
+    Returns a dict with `size_bytes` and `entries` when user data is present,
+    or None when the target is empty (or contains only a transient `config/`).
 
-    `config/` is whitelisted because it is created transiently by
-    `generate_certificates.sh` before the primary drive is mounted; that is
-    expected, not user data.
+    A `config/` holding only `ssl/` is ignored because `generate_certificates.sh`
+    creates it before the primary drive is mounted; see `is_transient_config`.
 
     Returns None for non-existent targets.
     """
@@ -288,7 +298,8 @@ def check_shadowed_data(target: str) -> Optional[dict]:
         return None
 
     try:
-        entries = [e.name for e in target_path.iterdir() if e.name != "config"]
+        entries = [e.name for e in target_path.iterdir()
+                   if not (e.name == "config" and is_transient_config(e))]
     except OSError:
         return None
     if not entries:

controller/lib/onboarding.py

@@ -21,7 +21,13 @@ from controller.lib.config import (
     save_config,
     update_config,
 )
-from controller.lib.disks import check_shadowed_data, get_block_devices, mount_drive, unmount_drive
+from controller.lib.disks import (
+    check_shadowed_data,
+    get_block_devices,
+    is_transient_config,
+    mount_drive,
+    unmount_drive,
+)
 from controller.lib.fstab import add_fstab_entry
 from controller.lib.scripts import start_script
 
@@ -226,9 +232,12 @@ def commit_onboarding(
 
         # Step 1b: Clean up any transient config directory created on the root
         # filesystem (e.g. by generate_certificates.sh running before the drive
-        # was mounted). This prevents stale root CAs from lingering.
+        # was mounted). This prevents stale root CAs from lingering.  A config
+        # directory with anything else in it is a library kept on the internal
+        # disk; the shadowed data check below protects it.
         transient_config = Path(media_dir) / "config"
-        if transient_config.exists() and not Path(media_dir).is_mount():
+        if transient_config.exists() and not Path(media_dir).is_mount() \
+                and is_transient_config(transient_config):
             import shutil
             shutil.rmtree(transient_config)
             logger.info("Removed transient config directory at %s", transient_config)

controller/test/test_disks.py

@@ -437,13 +437,24 @@ class TestCheckShadowedData:
         assert check_shadowed_data(str(target)) is None
 
     def test_returns_none_when_only_config_present(self, tmp_path):
-        """`config/` is whitelisted — it's expected pre-mount state."""
+        """A `config/` holding only the CA from generate_certificates.sh is expected pre-mount state."""
         target = tmp_path / "wrolpi"
         target.mkdir()
-        (target / "config").mkdir()
-        (target / "config" / "ssl.crt").write_text("cert")
+        (target / "config" / "ssl").mkdir(parents=True)
+        (target / "config" / "ssl" / "ca.crt").write_text("cert")
         assert check_shadowed_data(str(target)) is None
 
+    def test_detects_config_with_user_data(self, tmp_path):
+        """A library kept on the internal disk has its database and configs in `config/`."""
+        target = tmp_path / "wrolpi"
+        target.mkdir()
+        (target / "config" / "ssl").mkdir(parents=True)
+        (target / "config" / "ssl" / "ca.crt").write_text("cert")
+        (target / "config" / "wrolpi.db").write_bytes(b"x" * 10)
+        result = check_shadowed_data(str(target))
+        assert result["entries"] == ["config"]
+        assert result["size_bytes"] == 14
+
     def test_detects_directory_entry(self, tmp_path):
         target = tmp_path / "wrolpi"
         target.mkdir()

controller/test/test_onboarding.py

@@ -259,17 +259,20 @@ class TestCommitOnboarding:
         """Full onboarding with config file on drive."""
         media_dir = tmp_path / "wrolpi"
         media_dir.mkdir()
-        config_dir = media_dir / "config"
-        config_dir.mkdir()
-
         config_data = {"drives": {"mounts": [
             {"device": "/dev/sdb1", "mount_point": "/media/backup", "fstype": "ext4"},
         ]}}
-        (config_dir / "controller.yaml").write_text(yaml.dump(config_data))
+
+        def mount_side_effect(device, mount_point, **kwargs):
+            if device == "/dev/sda1":
+                # The drive's config appears once it is mounted.
+                (media_dir / "config").mkdir()
+                (media_dir / "config" / "controller.yaml").write_text(yaml.dump(config_data))
+            return {"success": True}
 
         with mock.patch("controller.lib.onboarding.get_media_directory", return_value=media_dir), \
              mock.patch("controller.lib.onboarding._cleanup_temp_mount"), \
-             mock.patch("controller.lib.onboarding.mount_drive", return_value={"success": True}), \
+             mock.patch("controller.lib.onboarding.mount_drive", side_effect=mount_side_effect), \
              mock.patch("controller.lib.onboarding.add_fstab_entry", return_value={"success": True}), \
              mock.patch("controller.lib.onboarding.reload_config_from_drive", return_value=True), \
              mock.patch("controller.lib.onboarding.save_config"), \
@@ -277,7 +280,7 @@ class TestCommitOnboarding:
             result = commit_onboarding("/dev/sda1", "ext4")
 
         assert result["success"] is True
-        assert str(media_dir) in result["mounts"]
+        assert result["mounts"] == [str(media_dir), "/media/backup"]
         assert result["repair_started"] is True
 
     def test_fresh_drive_proceeds(self, mock_docker_mode, tmp_path):
@@ -402,13 +405,9 @@ class TestCommitOnboarding:
         """Secondary mount failure should not abort onboarding."""
         media_dir = tmp_path / "wrolpi"
         media_dir.mkdir()
-        config_dir = media_dir / "config"
-        config_dir.mkdir()
-
         config_data = {"drives": {"mounts": [
             {"device": "/dev/sdb1", "mount_point": "/media/backup", "fstype": "ext4"},
         ]}}
-        (config_dir / "controller.yaml").write_text(yaml.dump(config_data))
 
         call_count = 0
 
@@ -416,7 +415,10 @@ class TestCommitOnboarding:
             nonlocal call_count
             call_count += 1
             if call_count == 1:
-                return {"success": True}  # Primary mount succeeds
+                # Primary mount succeeds, and the drive's config appears.
+                (media_dir / "config").mkdir()
+                (media_dir / "config" / "controller.yaml").write_text(yaml.dump(config_data))
+                return {"success": True}
             return {"success": False, "error": "Not found"}  # Secondary fails
 
         with mock.patch("controller.lib.onboarding.get_media_directory", return_value=media_dir), \
@@ -429,6 +431,7 @@ class TestCommitOnboarding:
             result = commit_onboarding("/dev/sda1", "ext4")
 
         assert result["success"] is True
+        assert call_count == 2
         # Only primary mount should be in the list
         assert len(result["mounts"]) == 1
 
@@ -476,8 +479,8 @@ class TestCommitOnboardingShadowedData:
         """A target containing only `config/` is the expected pre-mount state."""
         media_dir = tmp_path / "wrolpi"
         media_dir.mkdir()
-        (media_dir / "config").mkdir()
-        (media_dir / "config" / "ssl.crt").write_text("cert")
+        (media_dir / "config" / "ssl").mkdir(parents=True)
+        (media_dir / "config" / "ssl" / "ca.crt").write_text("cert")
 
         with contextlib.ExitStack() as stack:
             for p in self._patched_mount_calls(media_dir):
@@ -486,6 +489,36 @@ class TestCommitOnboardingShadowedData:
 
         assert result["success"] is True
 
+    def test_internal_library_config_blocks_and_is_kept(self, mock_docker_mode, tmp_path):
+        """A library kept on the internal disk is never deleted by onboarding a drive."""
+        media_dir = tmp_path / "wrolpi"
+        (media_dir / "config" / "ssl").mkdir(parents=True)
+        (media_dir / "config" / "wrolpi.db").write_bytes(b"x" * 10)
+
+        with contextlib.ExitStack() as stack:
+            for p in self._patched_mount_calls(media_dir):
+                stack.enter_context(p)
+            result = commit_onboarding("/dev/sda1", "ext4")
+
+        assert result["success"] is False
+        assert result["needs_force"] == "shadowed"
+        assert result["shadowed_data"]["entries"] == ["config"]
+        assert (media_dir / "config" / "wrolpi.db").is_file()
+
+    def test_internal_library_config_kept_when_forced(self, mock_docker_mode, tmp_path):
+        """Forcing the mount hides the internal library, but does not delete it."""
+        media_dir = tmp_path / "wrolpi"
+        (media_dir / "config" / "ssl").mkdir(parents=True)
+        (media_dir / "config" / "wrolpi.db").write_bytes(b"x" * 10)
+
+        with contextlib.ExitStack() as stack:
+            for p in self._patched_mount_calls(media_dir):
+                stack.enter_context(p)
+            result = commit_onboarding("/dev/sda1", "ext4", force_shadowed=True)
+
+        assert result["success"] is True
+        assert (media_dir / "config" / "wrolpi.db").is_file()
+
     def test_shadowed_data_blocks_without_force(self, mock_docker_mode, tmp_path):
         """A target with non-config data should soft-block."""
         media_dir = tmp_path / "wrolpi"