Disks: allow persisting the primary mount from the UI; clean NTFS before mounting.

The Disks page treated /media/wrolpi as fully protected, so a
manually-mounted primary drive could not be persisted from the UI — and
the Persist toggle would have been wrong anyway: it wrote fstab.yaml,
which the wrolpi-mounts reconciler refuses to apply for the primary
mount (RESERVED_MOUNT_POINTS).  Route Persist for the primary mount to
the host's /etc/fstab (as onboarding does), report its entry in the
fstab list endpoint, and enable the toggle in both UIs while keeping
Unmount blocked.  Mounts outside /media (e.g. /boot/efi, which showed
an Unmount button) are now entirely untouchable System mounts.

Also:
- Run `ntfsfix -d` before mounting ntfs/ntfs3 volumes so the Windows
  dirty flag (Fast Startup, unclean unplug) cannot block mounting;
  non-fatal and a no-op on clean volumes.
- Add uid/gid fstab options for ntfs/ntfs3, matching mount_drive().
- Log reconcile diagnostics (desired/already-mounted/skipped with
  reasons) so an idle wrolpi-mounts run explains itself in the journal.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Roland 2026-07-05 14:50:56 -06:00
parent 85bd5619fa
commit c25902b92a
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
11 changed files with 251 additions and 18 deletions

View file

@ -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() {
<Table.Body>
{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() {
)}
</Table.Cell>
<Table.Cell>
{isMounted && !isProtected ? (
{isMounted && !isSystemMount ? (
<Toggle
checked={persistent}
onChange={(checked) => handleTogglePersist(disk, checked)}
@ -1031,7 +1035,7 @@ function DiskSection() {
)}
</Table.Cell>
<Table.Cell>
{isMounted && isProtected ? (
{isMounted && (isSystemMount || isPrimary) ? (
<span style={{color: '#888'}}>System</span>
) : isMounted ? (
<Button

View file

@ -23,7 +23,10 @@ from typing import Optional
from fastapi import APIRouter, HTTPException
from pydantic import BaseModel, Field
from controller.lib.config import is_docker_mode
from controller.lib.config import get_media_directory, is_docker_mode
from controller.lib.fstab import add_fstab_entry as add_etc_fstab_entry
from controller.lib.fstab import parse_fstab as parse_etc_fstab
from controller.lib.fstab import remove_fstab_entry as remove_etc_fstab_entry
from controller.lib.disks import (
check_shadowed_data,
get_block_devices,
@ -297,7 +300,7 @@ async def list_fstab():
"""Return the WROLPi-managed mount table (fstab.yaml)."""
_check_native_mode()
data = load_fstab()
return [
entries = [
{
"type": "mount",
"device": e.device,
@ -309,6 +312,24 @@ async def list_fstab():
}
for e in data.mounts
]
# The primary mount persists via the host's /etc/fstab (the reconciler
# refuses to manage it) — include it so UIs can show its Persist state.
media_dir = str(get_media_directory())
try:
for e in parse_etc_fstab():
if e.get("type") == "mount" and e.get("mount_point") == media_dir:
entries.append({
"type": "mount",
"device": e["device"],
"mount_point": e["mount_point"],
"fstype": e["fstype"],
"options": e["options"],
"dump": e.get("dump", "0"),
"pass": e.get("pass", "2"),
})
except OSError:
pass
return entries
@router.post("/fstab")
@ -330,6 +351,16 @@ async def add_fstab(request: FstabRequest):
except ValueError as e:
raise HTTPException(status_code=400, detail=str(e))
if request.mount_point == str(get_media_directory()):
# The primary mount persists via the host's /etc/fstab; a fstab.yaml
# entry would never be applied (RESERVED_MOUNT_POINTS).
result = add_etc_fstab_entry(request.device, request.mount_point, request.fstype)
if not result.get("success"):
raise HTTPException(status_code=500,
detail=result.get("error") or "Failed to update /etc/fstab")
return {"success": True, "device": result.get("device"),
"mount_point": request.mount_point}
device_spec = _to_uuid_spec(request.device)
data = load_fstab()
data.add_or_replace(FstabEntry(
@ -362,6 +393,14 @@ async def delete_fstab(mount_point: str):
except ValueError as e:
raise HTTPException(status_code=400, detail=str(e))
if mount_point == str(get_media_directory()):
result = remove_etc_fstab_entry(mount_point)
if not result.get("success"):
error = result.get("error") or f"No entry found for {mount_point}"
status = 404 if "No entry found" in error else 500
raise HTTPException(status_code=status, detail=error)
return {"success": True, "mount_point": mount_point}
data = load_fstab()
if not data.remove_by_mount_point(mount_point):
raise HTTPException(status_code=404,

View file

@ -178,6 +178,22 @@ def mount_drive(
uid, gid = get_wrolpi_uid_gid()
options = f"{options},uid={uid},gid={gid}"
# NTFS volumes are frequently left "dirty" (Windows Fast Startup, or an
# unclean unplug), which makes the driver refuse to mount them. ntfsfix -d
# clears the dirty flag and replays the journal; it is a no-op on clean
# volumes. Failure is non-fatal: the mount below reports the real error.
if fstype in ("ntfs", "ntfs3"):
try:
fix = subprocess.run(
["ntfsfix", "-d", device],
capture_output=True, text=True, timeout=60,
)
if fix.returncode != 0:
logger.warning("ntfsfix %s failed (mount will still be attempted): %s",
device, (fix.stderr or fix.stdout).strip())
except (subprocess.TimeoutExpired, OSError) as e:
logger.warning("Could not run ntfsfix on %s: %s", device, e)
# Build mount command
cmd = ["mount"]
if fstype:

View file

@ -164,9 +164,10 @@ def add_fstab_entry(
# This ensures we don't get duplicates when remounting the same device to a different location
entries = _filter_entry_with_comment(entries, mount_point=mount_point, device=device_spec)
# For exfat/vfat, add uid/gid options to set ownership to wrolpi user
# These filesystems don't support POSIX permissions natively
if fstype in ("exfat", "vfat"):
# For filesystems without native POSIX permissions, add uid/gid options to
# set ownership to the wrolpi user. Covers the FAT family and both NTFS
# drivers (ntfs-3g FUSE and the in-kernel ntfs3), matching mount_drive().
if fstype in ("exfat", "vfat", "ntfs", "ntfs3"):
uid, gid = get_wrolpi_uid_gid()
options = f"{options},uid={uid},gid={gid}"

View file

@ -16,7 +16,7 @@ import sys
from controller.lib.disks import get_wrolpi_uid_gid
from controller.lib.fstab_migration import migrate_etc_fstab
from controller.lib.mount_executor import SubprocessMountExecutor
from controller.lib.reconciler import Reconciler
from controller.lib.reconciler import DEFAULT_FSTAB_PATH, Reconciler
logger = logging.getLogger(__name__)
@ -48,11 +48,19 @@ def main(argv=None) -> int:
)
result = reconciler.apply()
if result.desired_count == 0:
logger.info("%s lists no mounts; nothing to reconcile.", DEFAULT_FSTAB_PATH)
logger.info(
"Reconcile complete: %d mounted, %d unmounted, %d mount failures, %d unmount failures",
"Reconcile complete: %d desired, %d already mounted, %d skipped, "
"%d mounted, %d unmounted, %d mount failures, %d unmount failures",
result.desired_count, len(result.already_mounted), len(result.skipped),
len(result.mounted), len(result.unmounted),
len(result.mount_failures), len(result.unmount_failures),
)
for mp in result.already_mounted:
logger.info("Already mounted: %s", mp)
for mp, reason in result.skipped:
logger.info("Skipped %s: %s", mp, reason)
for mp, err in result.mount_failures:
logger.error("Failed to mount %s: %s", mp, err)
for mp, err in result.unmount_failures:

View file

@ -54,6 +54,11 @@ class ReconcileResult:
mount_failures: list[tuple[str, str]] = field(default_factory=list)
unmount_failures: list[tuple[str, str]] = field(default_factory=list)
skipped: list[tuple[str, str]] = field(default_factory=list)
# Diagnostics: how many entries fstab.yaml listed, and which of them were
# satisfied before this run. Together with `skipped` these explain a
# "nothing to do" reconcile in the journal.
desired_count: int = 0
already_mounted: list[str] = field(default_factory=list)
@property
def all_succeeded(self) -> bool:
@ -87,6 +92,7 @@ class Reconciler:
desired = load_fstab(self._fstab_path)
live = self._executor.current_mount_points()
managed = self._read_managed()
result.desired_count = len(desired.mounts)
# 1. Mount each entry in fstab.yaml that isn't already live.
for entry in desired.mounts:
@ -98,6 +104,7 @@ class Reconciler:
# Already mounted — claim ownership so a later removal can
# safely unmount it.
managed.add(entry.mount_point)
result.already_mounted.append(entry.mount_point)
continue
options = self._inject_uid_gid(entry)
self._ensure_dir(entry.mount_point)

View file

@ -1663,8 +1663,11 @@
<td>
`;
const protectedMounts = ['/', '/boot', '/boot/firmware', '/media/wrolpi'];
const isProtected = protectedMounts.includes(disk.mountpoint);
// The primary mount cannot be unmounted, but its persistence
// (/etc/fstab entry) can be toggled. Mounts outside /media
// (/, /boot/efi, ...) are system mounts: entirely untouchable.
const isPrimary = disk.mountpoint === '/media/wrolpi';
const isSystemMount = mounted && !isPrimary && !disk.mountpoint.startsWith('/media/');
// Check if this disk has a persistent entry (by mount point or UUID)
let isPersistent = false;
@ -1679,7 +1682,7 @@
}
// Persist column - show for mounted drives OR unmounted persistent drives
if ((mounted || isPersistent) && !isProtected) {
if ((mounted || isPersistent) && !isSystemMount) {
const mountPoint = mounted ? disk.mountpoint : persistentEntry.mount_point;
html += `
<label class="toggle">
@ -1697,10 +1700,10 @@
html += `</td><td>`;
// Actions column
if (mounted && !isProtected) {
html += `<button class="btn btn-danger" onclick="unmountDisk('${disk.mountpoint}')">Unmount</button>`;
} else if (mounted && isProtected) {
if (mounted && (isSystemMount || isPrimary)) {
html += `<span style="color: #888;">System</span>`;
} else if (mounted) {
html += `<button class="btn btn-danger" onclick="unmountDisk('${disk.mountpoint}')">Unmount</button>`;
} else {
const defaultMount = disk.label ? `/media/${disk.label}` : `/media/${disk.name}`;
html += `<button class="btn btn-primary" onclick="mountDisk('${disk.path}', '${defaultMount}', '${disk.fstype || ''}')">Mount</button>`;

View file

@ -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

View file

@ -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:

View file

@ -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"

View file

@ -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"}))