diff --git a/app/src/components/admin/ControllerPage.js b/app/src/components/admin/ControllerPage.js index 83117264..1991fef2 100644 --- a/app/src/components/admin/ControllerPage.js +++ b/app/src/components/admin/ControllerPage.js @@ -647,8 +647,10 @@ function SmartDetailsModal({drive, open, onClose}) { } -// Protected mount points that should not be unmounted -const PROTECTED_MOUNTS = ['/', '/boot', '/boot/firmware', '/media/wrolpi']; +// The primary mount cannot be unmounted from the UI, but its persistence +// (an /etc/fstab entry) can be toggled. Any mount outside /media belongs to +// the system (/, /boot/efi, /boot/firmware, …) and is entirely untouchable. +const PRIMARY_MOUNT = '/media/wrolpi'; // Helper to format disk size const formatSize = (size) => { @@ -1003,7 +1005,9 @@ function DiskSection() { {disks.map((disk) => { const isMounted = disk.mountpoint && disk.mountpoint !== ''; - const isProtected = PROTECTED_MOUNTS.includes(disk.mountpoint); + const isPrimary = disk.mountpoint === PRIMARY_MOUNT; + // Mounts outside /media (/, /boot/efi, …) are system mounts. + const isSystemMount = isMounted && !isPrimary && !disk.mountpoint.startsWith('/media/'); const persistent = isMounted && isPersistent(disk.mountpoint); return ( @@ -1020,7 +1024,7 @@ function DiskSection() { )} - {isMounted && !isProtected ? ( + {isMounted && !isSystemMount ? ( handleTogglePersist(disk, checked)} @@ -1031,7 +1035,7 @@ function DiskSection() { )} - {isMounted && isProtected ? ( + {isMounted && (isSystemMount || isPrimary) ? ( System ) : isMounted ? ( `; - } else if (mounted && isProtected) { + if (mounted && (isSystemMount || isPrimary)) { html += `System`; + } else if (mounted) { + html += ``; } else { const defaultMount = disk.label ? `/media/${disk.label}` : `/media/${disk.name}`; html += ``; diff --git a/controller/test/test_disks.py b/controller/test/test_disks.py index 76e271a7..a8c19b8e 100644 --- a/controller/test/test_disks.py +++ b/controller/test/test_disks.py @@ -268,6 +268,45 @@ class TestMountDrive: assert result["success"] is False assert "mount failed" in result["error"] + def test_ntfs_mount_runs_ntfsfix_first(self): + """NTFS mounts run ntfsfix -d before mount to clear the Windows dirty flag.""" + with mock.patch("controller.lib.disks.is_docker_mode", return_value=False), \ + mock.patch("controller.lib.disks.get_wrolpi_uid_gid", return_value=(1001, 1001)), \ + mock.patch("pathlib.Path.mkdir"), \ + mock.patch("subprocess.run") as mock_run: + mock_run.return_value = mock.Mock(returncode=0, stdout="", stderr="") + result = mount_drive("/dev/sdb2", "/media/test", fstype="ntfs") + assert result["success"] is True + commands = [c[0][0] for c in mock_run.call_args_list] + assert commands[0][:2] == ["ntfsfix", "-d"] + assert "/dev/sdb2" in commands[0] + assert commands[1][0] == "mount" + + def test_ntfsfix_failure_does_not_block_mount(self): + """A failed ntfsfix is logged but the mount is still attempted.""" + with mock.patch("controller.lib.disks.is_docker_mode", return_value=False), \ + mock.patch("controller.lib.disks.get_wrolpi_uid_gid", return_value=(1001, 1001)), \ + mock.patch("pathlib.Path.mkdir"), \ + mock.patch("subprocess.run") as mock_run: + mock_run.side_effect = [ + mock.Mock(returncode=1, stdout="", stderr="ntfsfix failed"), # ntfsfix + mock.Mock(returncode=0, stdout="", stderr=""), # mount + ] + result = mount_drive("/dev/sdb2", "/media/test", fstype="ntfs") + assert result["success"] is True + + def test_ext4_mount_does_not_run_ntfsfix(self): + """Non-NTFS mounts must not invoke ntfsfix.""" + with mock.patch("controller.lib.disks.is_docker_mode", return_value=False), \ + mock.patch("pathlib.Path.mkdir"), \ + mock.patch("subprocess.run") as mock_run: + mock_run.return_value = mock.Mock(returncode=0, stdout="", stderr="") + result = mount_drive("/dev/sda1", "/media/test", fstype="ext4") + assert result["success"] is True + commands = [c[0][0] for c in mock_run.call_args_list] + assert len(commands) == 1 + assert commands[0][0] == "mount" + @pytest.mark.parametrize("fstype,expect_uid_gid", [ ("exfat", True), ("vfat", True), # FAT32 diff --git a/controller/test/test_disks_api.py b/controller/test/test_disks_api.py index ebb53bdf..bc9a6adc 100644 --- a/controller/test/test_disks_api.py +++ b/controller/test/test_disks_api.py @@ -345,6 +345,89 @@ class TestFstabMutateEndpoints: assert response.status_code == 404 +# --- Primary mount persistence (/etc/fstab, not fstab.yaml) ---------------- + +class TestPrimaryMountPersist: + """The primary mount (/media/wrolpi) persists via the host's /etc/fstab, + not fstab.yaml: the wrolpi-mounts reconciler refuses to manage it + (RESERVED_MOUNT_POINTS), so a fstab.yaml entry would never be applied.""" + + @pytest.fixture(autouse=True) + def primary_media_dir(self, monkeypatch): + """conftest isolates MEDIA_DIRECTORY to a tmp dir; these tests need the + real primary mount point so the endpoints route to /etc/fstab.""" + monkeypatch.setenv("MEDIA_DIRECTORY", "/media/wrolpi") + + def test_post_primary_routes_to_etc_fstab(self, test_client, disks_env): + with mock.patch( + "controller.api.disks.add_etc_fstab_entry", + return_value={"success": True, "device": "UUID=abc-123", + "mount_point": "/media/wrolpi"}, + ) as add_etc: + response = test_client.post( + "/api/disks/fstab", + json={"device": "/dev/sdb2", + "mount_point": "/media/wrolpi", + "fstype": "ntfs"}, + ) + assert response.status_code == 200 + assert response.json()["device"] == "UUID=abc-123" + add_etc.assert_called_once() + # fstab.yaml must NOT contain the primary mount. + assert fstab_yaml.load(disks_env["fstab_path"]).mount_points() == set() + + def test_post_primary_reports_etc_fstab_failure(self, test_client, disks_env): + with mock.patch( + "controller.api.disks.add_etc_fstab_entry", + return_value={"success": False, "error": "WROL Mode is enabled"}, + ): + response = test_client.post( + "/api/disks/fstab", + json={"device": "/dev/sdb2", + "mount_point": "/media/wrolpi", + "fstype": "ntfs"}, + ) + assert response.status_code == 500 + assert "WROL Mode" in response.json()["detail"] + + def test_delete_primary_routes_to_etc_fstab(self, test_client, disks_env): + with mock.patch( + "controller.api.disks.remove_etc_fstab_entry", + return_value={"success": True, "mount_point": "/media/wrolpi"}, + ) as remove_etc: + response = test_client.delete("/api/disks/fstab/media/wrolpi") + assert response.status_code == 200 + remove_etc.assert_called_once_with("/media/wrolpi") + + def test_delete_primary_missing_returns_404(self, test_client, disks_env): + with mock.patch( + "controller.api.disks.remove_etc_fstab_entry", + return_value={"success": False, + "error": "No entry found for /media/wrolpi"}, + ): + response = test_client.delete("/api/disks/fstab/media/wrolpi") + assert response.status_code == 404 + + def test_list_includes_primary_from_etc_fstab(self, test_client, disks_env): + etc_entries = [ + {"type": "comment", "line": "# /etc/fstab\n"}, + {"type": "mount", "device": "UUID=root", "mount_point": "/", + "fstype": "ext4", "options": "defaults", "dump": "0", "pass": "1"}, + {"type": "mount", "device": "UUID=abc-123", + "mount_point": "/media/wrolpi", "fstype": "ntfs", + "options": "defaults,nofail", "dump": "0", "pass": "2"}, + ] + with mock.patch("controller.api.disks.parse_etc_fstab", + return_value=etc_entries): + response = test_client.get("/api/disks/fstab") + assert response.status_code == 200 + body = response.json() + mount_points = [e["mount_point"] for e in body] + # The primary appears; unrelated system mounts (e.g. /) do not. + assert mount_points == ["/media/wrolpi"] + assert body[0]["device"] == "UUID=abc-123" + + # --- SMART ---------------------------------------------------------------- class TestSmartEndpoints: diff --git a/controller/test/test_fstab.py b/controller/test/test_fstab.py index 88049a66..ab532f16 100644 --- a/controller/test/test_fstab.py +++ b/controller/test/test_fstab.py @@ -218,6 +218,30 @@ UUID=5678 /media/64GB exfat defaults,nofail,x-systemd.device-timeout=10s 0 2 assert "uid=" not in written_str assert "gid=" not in written_str + def test_ntfs_fstab_entry_includes_uid_gid(self): + """Should add uid/gid options when adding ntfs fstab entry (no POSIX permissions).""" + fstab_content = "# empty\n" + written_content = [] + + def mock_write(content): + written_content.append(content) + + mock_file = mock.mock_open(read_data=fstab_content) + mock_file().write = mock_write + + with mock.patch("controller.lib.fstab.require_normal_mode"): + with mock.patch("controller.lib.fstab.get_uuid", return_value="1234-5678"): + with mock.patch("controller.lib.fstab.backup_fstab"): + with mock.patch("controller.lib.fstab.get_wrolpi_uid_gid", return_value=(1001, 1001)): + with mock.patch("builtins.open", mock_file): + with mock.patch("subprocess.run"): + result = add_fstab_entry("/dev/sdb2", "/media/wrolpi", "ntfs") + assert result["success"] is True + written_str = "".join(written_content) + # Should have uid/gid options + assert "uid=1001" in written_str + assert "gid=1001" in written_str + def test_vfat_fstab_entry_includes_uid_gid(self): """Should add uid/gid options when adding vfat (FAT32) fstab entry.""" fstab_content = "# empty\n" diff --git a/controller/test/test_reconciler.py b/controller/test/test_reconciler.py index 65d41af2..909b9ba5 100644 --- a/controller/test/test_reconciler.py +++ b/controller/test/test_reconciler.py @@ -84,6 +84,15 @@ class TestMounts: result = rec.apply() assert result.mounted == [] assert fake.mount_calls == [] + # Diagnostics explain the "nothing to do" run in the journal. + assert result.desired_count == 1 + assert result.already_mounted == ["/media/wrolpi/usb"] + + def test_empty_fstab_reports_zero_desired(self, tmp_path): + rec, fake = make_reconciler(tmp_path, fstab=FstabFile()) + result = rec.apply() + assert result.desired_count == 0 + assert result.already_mounted == [] def test_mount_failure_is_recorded(self, tmp_path): fake = FakeMountExecutor(fail_for_mount=frozenset({"/media/wrolpi/usb"}))