Controller can unmount foreign mounts; protect the primary drive.

The /api/disks/unmount endpoint only removed entries from fstab.yaml and
asked the reconciler to unmount, but the reconciler deliberately never
unmounts paths it has not claimed.  Foreign mounts (udisks2 desktop
automounts, manual mounts) therefore could not be unmounted from the UI,
failing with a confusing "check journalctl" error.

- Fall back to a direct umount(8) when a reconcile leaves the requested
  mount point live.
- Reject unmounting the primary WROLPi drive (/media/wrolpi) with a
  clear 400, normalized to catch path traversal.
- Require the normalized mount point to be strictly under /media/
  before any direct umount, since the fallback runs as root.
- Show "System" instead of an Unmount button for /media/wrolpi in both
  the React UI and the Controller's simplified UI.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Roland 2026-06-02 13:37:51 -06:00 • committed by lrnselfreliance
parent ed4546d1ad
commit 5313b0dccc
4 changed files with 93 additions and 12 deletions

View file

@ -633,7 +633,7 @@ function SmartDetailsModal({drive, open, onClose}) {
// Protected mount points that should not be unmounted
const PROTECTED_MOUNTS = ['/', '/boot', '/boot/firmware'];
const PROTECTED_MOUNTS = ['/', '/boot', '/boot/firmware', '/media/wrolpi'];
// Helper to format disk size
const formatSize = (size) => {

View file

@ -2,14 +2,21 @@
Disk management API endpoints for WROLPi Controller.
The Controller defines what should be mounted (writes fstab.yaml on the
WROLPi drive) but never calls mount(8) or umount(8) itself. After every
write to fstab.yaml the Controller triggers wrolpi-mounts.service via
WROLPi drive) but never calls mount(8) itself. After every write to
fstab.yaml the Controller triggers wrolpi-mounts.service via
``systemctl restart``; the service reconciles live state to match. The
Controller verifies the outcome by reading /proc/mounts through the same
MountExecutor abstraction the service uses.
One exception: the reconciler deliberately never unmounts paths it has
not claimed, so foreign mounts (udisks2 desktop automounts, manual
mounts) under /media/ would otherwise be impossible to remove from the
UI. When a reconcile leaves the requested mount point live, /unmount
falls back to a direct umount(8) for non-reserved /media/ paths.
"""
import logging
import os
import subprocess
from typing import Optional
@ -33,6 +40,7 @@ from controller.lib.mount_executor import (
MountExecutor,
SubprocessMountExecutor,
)
from controller.lib.reconciler import RESERVED_MOUNT_POINTS
from controller.lib.smart import (
get_all_smart_status,
is_smart_available,
@ -222,7 +230,12 @@ async def disk_mount(request: MountRequest):
@router.post("/unmount")
async def disk_unmount(request: UnmountRequest):
"""Remove a mount from fstab.yaml and reconcile."""
"""Remove a mount from fstab.yaml and reconcile.
Falls back to a direct umount when the mount survives the reconcile —
a foreign mount (udisks2 automount, manual mount) the reconciler never
claimed, or a managed mount the reconciler could not release.
"""
_check_native_mode()
try:
@ -235,6 +248,22 @@ async def disk_unmount(request: UnmountRequest):
except ValueError as e:
raise HTTPException(status_code=400, detail=str(e))
# The direct-umount fallback below runs as root, so be strict about
# what we will pass to umount(8): a normalized path strictly under
# /media/ that is not the primary drive.
normalized = os.path.normpath(request.mount_point)
if normalized in RESERVED_MOUNT_POINTS:
raise HTTPException(
status_code=400,
detail=f"Cannot unmount {request.mount_point}: it is the primary "
f"WROLPi drive.",
)
if not normalized.startswith("/media/"):
raise HTTPException(
status_code=400,
detail=f"Refusing to unmount {normalized}: not under /media/.",
)
data = load_fstab()
removed = data.remove_by_mount_point(request.mount_point)
save_fstab(data)
@ -244,12 +273,15 @@ async def disk_unmount(request: UnmountRequest):
raise HTTPException(status_code=500,
detail=f"Failed to reconcile mounts: {err}")
if request.mount_point in _executor.current_mount_points():
raise HTTPException(
status_code=500,
detail=f"Reconciler ran but {request.mount_point} is still mounted. "
f"Check journalctl -u wrolpi-mounts.service.",
)
if normalized in _executor.current_mount_points():
# The reconciler did not release it — either a foreign mount it
# never claimed, or the umount failed. Try directly.
result = _executor.unmount(normalized)
if not result.ok:
raise HTTPException(
status_code=500,
detail=f"{normalized} is still mounted: {result.error}",
)
return {
"success": True,

View file

@ -1604,7 +1604,7 @@
<td>
`;
const protectedMounts = ['/', '/boot', '/boot/firmware'];
const protectedMounts = ['/', '/boot', '/boot/firmware', '/media/wrolpi'];
const isProtected = protectedMounts.includes(disk.mountpoint);
// Check if this disk has a persistent entry (by mount point or UUID)

View file

@ -219,7 +219,8 @@ class TestUnmountEndpoint:
assert fstab_yaml.load(disks_env["fstab_path"]).mounts == []
def test_unmount_returns_500_when_still_mounted(self, test_client, disks_env):
# Reconciler refuses the unmount — surface the failure.
# Reconciler refuses the unmount (e.g. busy) — the direct fallback
# fails the same way; surface the failure.
disks_env["fake"]._live.add("/media/wrolpi/usb")
disks_env["fake"].fail_for_unmount = frozenset({"/media/wrolpi/usb"})
(disks_env["fstab_path"].parent / "managed").write_text(
@ -232,6 +233,54 @@ class TestUnmountEndpoint:
assert response.status_code == 500
assert "still mounted" in response.json()["detail"]
def test_unmount_foreign_mount_falls_back_to_direct_umount(
self, test_client, disks_env,
):
# A udisks2 desktop automount: live, but never in fstab.yaml and
# never claimed by the reconciler. The reconciler will not touch
# it, so the endpoint must unmount it directly.
foreign = "/media/wrolpi/31a8ecf1-c00b-4691-b26f-b0966073659f"
disks_env["fake"]._live.add(foreign)
response = test_client.post(
"/api/disks/unmount",
json={"mount_point": foreign},
)
assert response.status_code == 200
body = response.json()
assert body["success"] is True
assert body["removed_from_fstab"] is False
assert disks_env["fake"].current_mount_points() == set()
@pytest.mark.parametrize("mount_point", [
"/media", # not strictly under /media/
"/media_evil", # prefix-collides with /media
"/media/wrolpi/../wrolpi", # normalizes to the reserved primary
])
def test_unmount_rejects_paths_outside_media(
self, test_client, disks_env, mount_point,
):
response = test_client.post(
"/api/disks/unmount",
json={"mount_point": mount_point},
)
assert response.status_code == 400
assert disks_env["fake"].unmount_calls == []
def test_unmount_reserved_mount_point_rejected(self, test_client, disks_env):
# The primary WROLPi drive must never be unmountable from the API.
disks_env["fake"]._live.add("/media/wrolpi")
response = test_client.post(
"/api/disks/unmount",
json={"mount_point": "/media/wrolpi"},
)
assert response.status_code == 400
assert "primary" in response.json()["detail"].lower()
# No umount was attempted, directly or via the reconciler.
assert disks_env["fake"].unmount_calls == []
assert disks_env["fake"].current_mount_points() == {"/media/wrolpi"}
# --- /api/disks/fstab read endpoint ---------------------------------------