From da340a00b3fa87df2a78c0b0dcbdb574d1ce5542 Mon Sep 17 00:00:00 2001 From: Roland Date: Thu, 14 May 2026 15:51:32 -0600 Subject: [PATCH] Displaying warning when no drive is mounted. --- app/src/DashboardPage.js | 13 +++- app/src/DashboardPage.test.js | 36 ++++++++++- app/src/api.js | 1 + app/src/components/admin/Settings.js | 14 +++++ main.py | 13 ++++ wrolpi/common.py | 10 ++++ wrolpi/downloader.py | 4 ++ wrolpi/flags.py | 70 +++++++++++++++++++++- wrolpi/root_api.py | 1 + wrolpi/schema.py | 3 + wrolpi/test/test_downloader.py | 50 ++++++++++++++++ wrolpi/test/test_flags.py | 90 ++++++++++++++++++++++++++++ 12 files changed, 301 insertions(+), 4 deletions(-) diff --git a/app/src/DashboardPage.js b/app/src/DashboardPage.js index 681444a1..012080d4 100644 --- a/app/src/DashboardPage.js +++ b/app/src/DashboardPage.js @@ -38,7 +38,7 @@ import {DateSelectorButton} from "./components/DatesSelector"; import {useCalculators} from "./components/Calculators"; import {classifyAndEvaluate} from "./components/calculators/mathConfig"; -function FlagsMessages() { +export function FlagsMessages() { const {settings, fetchSettings} = React.useContext(SettingsContext); const {status} = useContext(StatusContext); @@ -88,9 +88,20 @@ function FlagsMessages() { ; } + let mediaUnmountedMessage; + if (flags.media_mounted === false) { + mediaUnmountedMessage = + No drive mounted + WROLPi has no media drive mounted for one or more storage locations. + Downloads would write to the root filesystem and can fill it. + {' '}Open the Controller to mount a drive. + ; + } + return <> {refreshingMessage} {dbDownMessage || refreshRequiredMessage} + {mediaUnmountedMessage} {settings && settings['ignore_outdated_zims'] === false && flags.outdated_zims ? : null} {internetDownMessage} diff --git a/app/src/DashboardPage.test.js b/app/src/DashboardPage.test.js index d6b21623..c86d2021 100644 --- a/app/src/DashboardPage.test.js +++ b/app/src/DashboardPage.test.js @@ -1,8 +1,8 @@ import React from 'react'; import {render, screen} from '@testing-library/react'; import {MemoryRouter} from 'react-router'; -import {Getters} from './DashboardPage'; -import {StatusContext, ThemeContext} from './contexts/contexts'; +import {Getters, FlagsMessages} from './DashboardPage'; +import {SettingsContext, StatusContext, ThemeContext} from './contexts/contexts'; // Replace DownloadMenu with a stub that records its props. The deep-link // behavior under test is just "Getters reads URL params and forwards them @@ -76,3 +76,35 @@ describe('DashboardPage Getters deep-link', () => { // newlines into the `download` prop's `urls` field) is exercised in the // browser via manual verification, not unit-tested here — reaching into the // per-form trees pulls in heavy fetch-driven components. + +function renderFlagsMessages({flags = {}, settings = {}} = {}) { + const status = {flags: {refresh_complete: true, db_up: true, have_internet: true, ...flags}}; + return render( + + {}}}> + + + + + + ); +} + +describe('DashboardPage no-drive-mounted banner', () => { + it('shows the banner when media_mounted is false', () => { + renderFlagsMessages({flags: {media_mounted: false}}); + expect(screen.getByText(/No drive mounted/i)).toBeInTheDocument(); + const link = screen.getByText(/Open the Controller/i); + expect(link).toHaveAttribute('href', '/admin/controller'); + }); + + it('hides the banner when media_mounted is true', () => { + renderFlagsMessages({flags: {media_mounted: true}}); + expect(screen.queryByText(/No drive mounted/i)).toBeNull(); + }); + + it('hides the banner when media_mounted is undefined (status not yet loaded)', () => { + renderFlagsMessages(); + expect(screen.queryByText(/No drive mounted/i)).toBeNull(); + }); +}); diff --git a/app/src/api.js b/app/src/api.js index a91248c0..07c0264a 100644 --- a/app/src/api.js +++ b/app/src/api.js @@ -332,6 +332,7 @@ export async function getSettings() { nav_color: content.nav_color, media_directory: content.media_directory, require_cookies_unlocked: content.require_cookies_unlocked, + require_media_mounted: content.require_media_mounted, save_ffprobe_json: content.save_ffprobe_json, tags_directory: content.tags_directory, throttle_on_startup: content.throttle_on_startup, diff --git a/app/src/components/admin/Settings.js b/app/src/components/admin/Settings.js index 2337556c..d8b4231e 100644 --- a/app/src/components/admin/Settings.js +++ b/app/src/components/admin/Settings.js @@ -453,6 +453,7 @@ export function SettingsPage() { archive_destination: settings.archive_destination, download_on_startup: settings.download_on_startup, require_cookies_unlocked: settings.require_cookies_unlocked, + require_media_mounted: settings.require_media_mounted, download_timeout: settings.download_timeout, download_wait: settings.download_wait, download_window_start: settings.download_window_start || '', @@ -686,6 +687,19 @@ export function SettingsPage() { /> +
+ + Pause downloads when no drive is mounted + + } + disabled={disabled || state.require_media_mounted === null} + checked={state.require_media_mounted === true} + onChange={checked => handleInputChange(null, 'require_media_mounted', checked)} + /> +
+ diff --git a/main.py b/main.py index d2b72d39..979e53f1 100755 --- a/main.py +++ b/main.py @@ -346,6 +346,19 @@ async def perpetual_have_internet_worker(): logger.error('Failed to check if internet is up', exc_info=e) +@perpetual_signal(sleep=30) +async def perpetual_media_mounted_worker(): + try: + if flags._destinations_have_mounted_storage(): + flags.media_mounted.set() + # Re-check less often once all destinations are covered. + await asyncio.sleep(float(Seconds.minute * 5)) + else: + flags.media_mounted.clear() + except Exception as e: + logger.error('Failed to check media mount status', exc_info=e) + + @perpetual_signal(sleep=30) async def perpetual_start_video_missing_comments_download(): from modules.videos.video.lib import get_missing_videos_comments diff --git a/wrolpi/common.py b/wrolpi/common.py index c6fa9d24..355a7466 100644 --- a/wrolpi/common.py +++ b/wrolpi/common.py @@ -810,6 +810,7 @@ class WROLPiConfigValidator: map_destination: str = None nav_color: str = None require_cookies_unlocked: bool = None + require_media_mounted: bool = None save_ffprobe_json: bool = None tags_directory: bool = None throttle_on_startup: bool = None @@ -989,6 +990,7 @@ class WROLPiConfig(ConfigFile): map_destination='map', nav_color='violet', require_cookies_unlocked=True, + require_media_mounted=True, save_ffprobe_json=True, tags_directory=True, throttle_on_startup=False, @@ -1058,6 +1060,14 @@ class WROLPiConfig(ConfigFile): def require_cookies_unlocked(self, value: bool): self.update({'require_cookies_unlocked': value}) + @property + def require_media_mounted(self) -> bool: + return self._config.get('require_media_mounted', True) + + @require_media_mounted.setter + def require_media_mounted(self, value: bool): + self.update({'require_media_mounted': value}) + @property def download_timeout(self) -> int: return self._config['download_timeout'] diff --git a/wrolpi/downloader.py b/wrolpi/downloader.py index 1e80bd9c..2d0cd2a2 100644 --- a/wrolpi/downloader.py +++ b/wrolpi/downloader.py @@ -768,6 +768,10 @@ class DownloadManager: if wrol_mode_enabled(): # Do not download with WROL Mode enabled. return False + if get_wrolpi_config().require_media_mounted \ + and not flags.media_mounted.is_set(): + # No mounted drive backs every destination; refuse to fill the root disk. + return False if get_wrolpi_config().require_cookies_unlocked \ and flags.cookies_exist.is_set() \ and not flags.cookies_unlocked.is_set(): diff --git a/wrolpi/flags.py b/wrolpi/flags.py index d1b0479b..26aca9c1 100644 --- a/wrolpi/flags.py +++ b/wrolpi/flags.py @@ -3,10 +3,12 @@ import contextlib import multiprocessing import subprocess from datetime import datetime +from pathlib import Path from sqlalchemy import Column, Boolean, Integer -from wrolpi.common import logger, Base +from wrolpi.common import logger, Base, get_media_directory, get_wrolpi_config +from wrolpi.vars import DOCKERIZED __all__ = [ 'get_flags', @@ -110,6 +112,9 @@ db_up = Flag('db_up') outdated_zims = Flag('outdated_zims', store_db=True) # Used to disable or enable downloading when Internet is down. have_internet = Flag('have_internet') +# Every configured destination (videos/archive/zims/map) has a mounted drive at or above it. +# Cleared when at least one destination would write to the unmounted root filesystem. +media_mounted = Flag('media_mounted') # The FileWorker is busy (refreshing, moving, tagging, or renaming files). file_worker_busy = Flag('file_worker_busy') @@ -149,6 +154,7 @@ def get_flags() -> dict: global_refresh_active=global_refresh_active.is_set(), have_internet=have_internet.is_set(), map_search_building=map_search_building.is_set(), + media_mounted=media_mounted.is_set(), outdated_zims=outdated_zims.is_set(), refresh_complete=refresh_complete.is_set(), ) @@ -167,6 +173,68 @@ class WROLPiFlag(Base): f' outdated_zims={self.outdated_zims}>' +def _destinations_have_mounted_storage() -> bool: + """True when every configured destination is backed by a mounted drive. + + Walks each destination (videos/archive/zims/map) from its static path + prefix up through the media directory; if any ancestor in that range is + a mount point, the destination is considered covered. All destinations + must be covered for this to return True — otherwise a download could + silently write to the SD card on a Raspberry Pi. + + Docker mode is exempt: the media directory is always a bind mount and + the user cannot remount drives from inside the container. + """ + if DOCKERIZED: + return True + + media_dir = get_media_directory().resolve() + + config = get_wrolpi_config() + destinations = [ + config.videos_destination, + config.archive_destination, + config.zims_destination, + config.map_destination, + ] + + for dest in destinations: + if not dest: + continue + # Templated paths like 'videos/%(channel_tag)s/...' can't be fully + # resolved without a real channel; use the static prefix. + static_prefix = dest.split('%', 1)[0].rstrip('/') + candidate = (media_dir / static_prefix) if static_prefix else media_dir + # Resolve to normalize '..' segments before the walk; otherwise a + # destination like '../outside' would still walk back through + # media_dir and falsely report covered. + candidate = candidate.resolve() + + # If the destination escapes the media directory entirely, the + # download would land outside any drive we control — treat as + # uncovered so the warning fires. + if candidate != media_dir and media_dir not in candidate.parents: + return False + + covered = False + cursor = candidate + while True: + try: + if cursor.is_mount(): + covered = True + break + except OSError: + # Path doesn't exist yet (e.g. fresh install); keep walking. + pass + if cursor == media_dir: + break + cursor = cursor.parent + if not covered: + return False + + return True + + def check_db_is_up(): """Attempts to connect to the database, sets flags.db_up if successful.""" from wrolpi.db import get_db_curs, get_db_args diff --git a/wrolpi/root_api.py b/wrolpi/root_api.py index 14a8cf56..cd634119 100644 --- a/wrolpi/root_api.py +++ b/wrolpi/root_api.py @@ -183,6 +183,7 @@ def get_settings(_: Request): 'wrol_mode': wrolpi_config.wrol_mode, 'zims_destination': wrolpi_config.zims_destination, 'require_cookies_unlocked': wrolpi_config.require_cookies_unlocked, + 'require_media_mounted': wrolpi_config.require_media_mounted, 'save_ffprobe_json': wrolpi_config.save_ffprobe_json, 'timezone': wrolpi_config.timezone, } diff --git a/wrolpi/schema.py b/wrolpi/schema.py index 03939b6b..64ea1708 100644 --- a/wrolpi/schema.py +++ b/wrolpi/schema.py @@ -32,6 +32,7 @@ class SettingsResponse: nav_color: str media_directory: str require_cookies_unlocked: bool + require_media_mounted: bool tags_directory: bool throttle_on_startup: bool throttle_status: str @@ -84,6 +85,7 @@ class SettingsRequest: wrol_mode: Optional[bool] = None zims_destination: Optional[str] = None require_cookies_unlocked: Optional[bool] = None + require_media_mounted: Optional[bool] = None save_ffprobe_json: Optional[bool] = None timezone: Optional[str] = None @@ -104,6 +106,7 @@ class FlagsStatusResponse: file_worker_modeling: bool global_refresh_active: bool have_internet: bool + media_mounted: bool outdated_zims: bool refresh_complete: bool diff --git a/wrolpi/test/test_downloader.py b/wrolpi/test/test_downloader.py index c4258daf..0e9b020e 100644 --- a/wrolpi/test/test_downloader.py +++ b/wrolpi/test/test_downloader.py @@ -1415,3 +1415,53 @@ def test_get_local_time_without_configured_timezone(test_download_manager): # Should return the system's local time conversion. expected = mock_utc.astimezone().time() assert result == expected + + +def test_require_media_mounted_config_default(test_download_manager, test_wrolpi_config): + """require_media_mounted defaults to True (safe default for fresh installs).""" + assert get_wrolpi_config().require_media_mounted is True + + +def test_require_media_mounted_gate_blocks_when_flag_clear(test_download_manager, test_wrolpi_config, flags_lock): + """can_download returns False when the gate is on and the media_mounted flag is clear. + + Bypasses the PYTEST short-circuit at the top of can_download so the gate logic runs. + """ + from wrolpi import flags + config = get_wrolpi_config() + config.require_media_mounted = True + flags.media_mounted.clear() + flags.have_internet.set() # other gates must pass + + with mock.patch('wrolpi.downloader.PYTEST', False), \ + mock.patch('wrolpi.downloader.wrol_mode_enabled', return_value=False): + assert test_download_manager.can_download is False + + +def test_require_media_mounted_gate_allows_when_flag_set(test_download_manager, test_wrolpi_config, flags_lock): + """The gate stops blocking once media_mounted is set.""" + from wrolpi import flags + config = get_wrolpi_config() + config.require_media_mounted = True + flags.media_mounted.set() + flags.have_internet.set() + # Make successful_import True so the function reaches the final return True. + get_download_manager_config().successful_import = True + + with mock.patch('wrolpi.downloader.PYTEST', False), \ + mock.patch('wrolpi.downloader.wrol_mode_enabled', return_value=False): + assert test_download_manager.can_download is True + + +def test_require_media_mounted_gate_opt_out(test_download_manager, test_wrolpi_config, flags_lock): + """When the user opts out via the config, the gate is skipped even with the flag clear.""" + from wrolpi import flags + config = get_wrolpi_config() + config.require_media_mounted = False + flags.media_mounted.clear() + flags.have_internet.set() + get_download_manager_config().successful_import = True + + with mock.patch('wrolpi.downloader.PYTEST', False), \ + mock.patch('wrolpi.downloader.wrol_mode_enabled', return_value=False): + assert test_download_manager.can_download is True diff --git a/wrolpi/test/test_flags.py b/wrolpi/test/test_flags.py index 2e13a334..33a52c74 100644 --- a/wrolpi/test/test_flags.py +++ b/wrolpi/test/test_flags.py @@ -1,4 +1,6 @@ import asyncio +from pathlib import Path +from unittest import mock import pytest from sqlalchemy.orm import Session @@ -53,3 +55,91 @@ async def test_flag_wait_for(async_client, flags_lock): with pytest.raises(TimeoutError): async with flags.file_worker_busy.wait_for(timeout=1): await asyncio.sleep(2) + + +def _patch_mounts(mounted_paths: set, media_dir: Path): + """Make Path.is_mount return True only for paths in `mounted_paths`. + + Resolves all paths so test inputs match what the helper sees. + """ + mounted = {Path(p).resolve() for p in mounted_paths} + + def fake_is_mount(self): + try: + return self.resolve() in mounted + except OSError: + return False + + return mock.patch.object(Path, 'is_mount', fake_is_mount) + + +def test_destinations_have_mounted_storage_all_covered_by_root_mount(test_directory, test_wrolpi_config): + """The single common case: /media/wrolpi itself is mounted, so all destinations are covered.""" + with _patch_mounts({test_directory}, test_directory): + assert flags._destinations_have_mounted_storage() is True + + +def test_destinations_have_mounted_storage_nothing_mounted(test_directory, test_wrolpi_config): + """Fresh install: no mounts at all, warning should fire (returns False).""" + with _patch_mounts(set(), test_directory): + assert flags._destinations_have_mounted_storage() is False + + +def test_destinations_have_mounted_storage_partial_coverage(async_client, test_directory, test_wrolpi_config): + """If videos is on a drive but archive/zims/map are not, return False. + + This is the SD-card-fill case: archive downloads would still hit root. + """ + from wrolpi.common import get_wrolpi_config + # Point videos at a subdirectory that has its own mount. + get_wrolpi_config().videos_destination = 'one/videos/%(channel_tag)s/%(channel_name)s' + with _patch_mounts({test_directory / 'one'}, test_directory): + # videos covered, but archive/zims/map default destinations are not. + assert flags._destinations_have_mounted_storage() is False + + +def test_destinations_have_mounted_storage_each_destination_on_its_own_mount(async_client, test_directory, test_wrolpi_config): + """Multi-drive setup: every destination has its own mount above it. Pass.""" + from wrolpi.common import get_wrolpi_config + config = get_wrolpi_config() + config.videos_destination = 'one/videos/%(channel_tag)s/%(channel_name)s' + config.archive_destination = 'two/archive/%(domain_tag)s/%(domain)s' + config.zims_destination = 'three/zims' + config.map_destination = 'four/map' + + mounts = { + test_directory / 'one', + test_directory / 'two', + test_directory / 'three', + test_directory / 'four', + } + with _patch_mounts(mounts, test_directory): + assert flags._destinations_have_mounted_storage() is True + + +def test_destinations_have_mounted_storage_template_only_destination(async_client, test_directory, test_wrolpi_config): + """A destination with no static prefix (just '%(...)s') walks straight up to media_dir.""" + from wrolpi.common import get_wrolpi_config + get_wrolpi_config().videos_destination = '%(channel_name)s' + with _patch_mounts({test_directory}, test_directory): + # Static prefix is empty -> candidate is media_dir itself, which is mounted. + assert flags._destinations_have_mounted_storage() is True + + +def test_destinations_have_mounted_storage_docker_mode(test_directory, test_wrolpi_config): + """In Docker mode, the helper short-circuits to True.""" + with mock.patch('wrolpi.flags.DOCKERIZED', True), _patch_mounts(set(), test_directory): + assert flags._destinations_have_mounted_storage() is True + + +def test_destinations_have_mounted_storage_dotdot_escape(async_client, test_directory, test_wrolpi_config): + """A '..'-relative destination escapes media_dir and must be treated as uncovered. + + Without the resolve+escape check, the walk would terminate at media_dir + (which is mounted) and incorrectly return True, even though the real + destination resolves outside the mount and writes to the root filesystem. + """ + from wrolpi.common import get_wrolpi_config + get_wrolpi_config().videos_destination = '../outside' + with _patch_mounts({test_directory}, test_directory): + assert flags._destinations_have_mounted_storage() is False