diff --git a/README.md b/README.md index 8c4a741..0a6a2c8 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,108 @@ currently active, the box's LAN IP, and the exact command to run — `ssh @`. Handy since the box normally only has a touchscreen attached, not a keyboard. +## Remote storage sync (CIFS/SMB) + +The box can mount an institutional SMB/CIFS share and copy experiment images to +it automatically as they are captured, replacing the legacy hand-run +`sudo mount -t cifs //ds.asuch.cas.cz/ueb/lhr /mnt/Shared -o user=…,pass=…`. + +Settings → General → **Remote Sync** card: + +- **On/off toggle**: arms syncing for the researcher currently set on the home + screen. +- **Server / share**: pre-filled with `//ds.asuch.cas.cz/ueb/lhr` and freely + editable. The value is checked against a strict allowlist + (`//host/share[/folder]`, letters, digits, dots, hyphens and underscores + only) before it can reach the mount command; anything else is rejected with + a clear message. +- **Username** and **Password** for the share account. The password field is + masked, and when a password is already set the field shows a fixed-width + placeholder — the UI is never told the real password or its length. +- **Check Connection**: enabled only once a username and password are both + present. It mounts the share (if it isn't already), proves the destination + folder is actually writable, and reports the real error text if not — + "wrong password" and "host unreachable" need different fixes. +- **Sync Entire Folder**: a one-shot bulk copy of *every* local experiment + belonging to the current researcher, not just newly captured images. +- **Status**: mounted / not mounted / credentials needed, the destination path, + the last successful sync time, and a count of files still waiting to be + copied. + +**Remote layout** mirrors the legacy convention: the mounted share, then a +subfolder named after the researcher (created if missing), then one folder per +experiment: + +``` +/mnt/rapidboxes-remote//__/ + dark_00000.jpg + metadata.json + .xml +``` + +Thumbnails are not copied — they are regenerated locally on demand. + +**Sync stops** when the toggle is switched off, or when the researcher name +changes (starting an experiment under a different name switches sync off and +says so, rather than quietly writing into someone else's folder). + +**The local experiment always wins.** Copying happens on a background queue, +never on the capture path, so a slow, hung or dead share cannot delay the +capture schedule. A failed copy is logged, counted as pending, and retried on +the next capture; it never aborts or errors a running experiment. + +### The password is deliberately never written to disk + +This is a design decision, not an oversight: the password is held in memory for +the lifetime of the backend process and is written nowhere — not to +`settings.json`, not to `remote_sync.json`, not to logs, and not to any +credentials file that outlives the mount call itself. It is also never returned +by any API endpoint (this box has no authentication and binds `0.0.0.0`, so +anything it serves is readable by anyone on the LAN); the API exposes only a +`passwordSet` boolean. + +**The operational consequence: after any restart the password is gone and must +be re-entered.** That includes a reboot, a power blip, and the monthly +`rapidboxes-update.timer` OTA restart. In that state sync does not quietly +pretend to work — the Remote Sync card turns orange and reads **"Inactive — +credentials needed after restart"** until someone re-enters the password and +presses Check Connection. The same tradeoff is stated as helper text next to +the password field and confirmed by a toast when credentials are accepted, so +it is known *before* anyone leaves the box on a long unattended run. Server, +username and the on/off setting all persist normally; only the password does +not. + +### What the sudoers entry grants + +Mounting needs root, so `deploy/install.sh` installs +`/etc/sudoers.d/rapidboxes` (mode 0440, validated with `visudo -c` **before** +installation — a malformed sudoers file can lock the account out of `sudo` +entirely). It grants the service account exactly two commands and nothing else: + +``` +Cmnd_Alias RAPIDBOXES_CIFS = \ + /usr/bin/mount -t cifs //* /mnt/rapidboxes-remote -o credentials=/run/rapidboxes-cifs/cred-*\,nosuid\,nodev\,noexec\,uid=1000\,gid=1000\,file_mode=0664\,dir_mode=0775, \ + /usr/bin/umount /mnt/rapidboxes-remote + ALL=(root) NOPASSWD: RAPIDBOXES_CIFS +``` + +That is: one fixed mount point, one fixed trailing option string, and no +blanket `ALL`. The hardening options (`nosuid,nodev,noexec` and the +unprivileged uid/gid) come last on purpose — mount options are last-one-wins. +`deploy/uninstall.sh` removes the rule, the mount point and the mount. + +The password reaches `mount` through a `credentials=` file created 0600 with +`tempfile.mkstemp` in the service's private `/run/rapidboxes-cifs` directory +(systemd `RuntimeDirectory=`, on tmpfs), and that file is unlinked in a +`finally` the instant `mount` returns, success or failure. It is never passed +as `-o pass=…`, because `ps aux` is world-readable. Every subprocess call uses +a fixed argument list; `shell=True` is never used anywhere in the backend, and +a test enforces that. + +**Simulation mode** (`RAPIDBOXES_SIMULATION=1`, i.e. laptop development) never +attempts a real mount: the share is emulated by a local directory so the whole +sync path stays exercisable with no CIFS server present. + ## What the programs do ### Tropism program @@ -227,9 +329,11 @@ The settings menu has two tabs: - **Camera**: opens the full camera settings panel. - **General**: system info (hostname, version, disk space), LED strip segment - editor, IR pin display, software update / rollback controls, and SSH access - info. See [Software updates & version rollback](#software-updates--version-rollback) - and [SSH access](#ssh-access) below. + editor, IR pin display, remote CIFS sync configuration, software update / + rollback controls, and SSH access info. See + [Software updates & version rollback](#software-updates--version-rollback), + [SSH access](#ssh-access) and + [Remote storage sync](#remote-storage-sync-cifssmb) below. The **X** button closes the settings menu. @@ -392,3 +496,8 @@ Compared with the old single-purpose UI flow, the current system now includes: - **one-click rollback** to the previously-running version, with how-long-it-ran tracked automatically - an in-app **SSH access** panel (username, status, IP, ready-to-run command) +- **remote CIFS/SMB sync**: images copied to an institutional share as they are + captured, plus a one-shot bulk copy of a researcher's whole back catalogue — + off the capture path, so a dead share can never stall or fail a running + experiment. The share password is session-only and never written to disk, and + the UI says so plainly both while it is typed and after a restart clears it. diff --git a/back/rapidboxes/api/deps.py b/back/rapidboxes/api/deps.py index 9a29fc1..be719b7 100644 --- a/back/rapidboxes/api/deps.py +++ b/back/rapidboxes/api/deps.py @@ -9,6 +9,7 @@ from ..engine.runner import ExperimentRunner from ..hardware.manager import HardwareManager, build_hardware from ..models import DeviceSettings +from ..remote_sync import RemoteSyncService from ..storage import Storage @@ -19,6 +20,9 @@ class AppState: storage: Storage hw: HardwareManager runner: ExperimentRunner + # Remote CIFS sync. Holds the session-only password in memory; see + # rapidboxes/remote_sync.py for why it lives here and nowhere else. + sync: RemoteSyncService async def rebuild_hardware(self, settings: DeviceSettings) -> None: """Swap in fresh hardware after a settings change (idle only). diff --git a/back/rapidboxes/api/experiments.py b/back/rapidboxes/api/experiments.py index 24869ea..5cd472a 100644 --- a/back/rapidboxes/api/experiments.py +++ b/back/rapidboxes/api/experiments.py @@ -14,6 +14,11 @@ @router.post("", response_model=StartResponse) async def start_experiment(config: ExperimentConfig, state: AppState = Depends(get_state)): + # This is how the backend learns who the active researcher is: the name is + # client-side state (localStorage, see client/lib/session.ts) that arrives + # with every experiment config. Remote sync uses it as the destination + # subfolder, and switches itself off if it changes mid-stream. + state.sync.note_active_researcher(config.username) return await state.runner.start(config, state.settings.camera) diff --git a/back/rapidboxes/api/remote_sync.py b/back/rapidboxes/api/remote_sync.py new file mode 100644 index 0000000..2ed9836 --- /dev/null +++ b/back/rapidboxes/api/remote_sync.py @@ -0,0 +1,127 @@ +"""Remote CIFS sync configuration + actions (Settings -> General -> Remote Sync). + +Deliberately a separate router from /api/settings: the remote-sync config is +not a "how the image was taken" device setting (it must not travel into the +per-experiment config XML), and separating it keeps the password well away +from the DeviceSettings object that GET /api/settings serialises wholesale. + +The password is accepted here on PUT and nowhere else. No route in this file -- +or any other -- ever returns it: RemoteSyncStatus has no field for it, only +`passwordSet`. +""" +from __future__ import annotations + +from typing import Optional + +from fastapi import APIRouter, Depends, HTTPException +from pydantic import BaseModel + +from ..models import ( + RemoteSyncStatus, + RemoteSyncUpdate, + validate_remote_server, + validate_remote_username, +) +from .deps import AppState, get_state + +router = APIRouter(prefix="/api/settings/remote-sync", tags=["remote-sync"]) + + +class CheckConnectionResult(BaseModel): + ok: bool + message: str + status: RemoteSyncStatus + + +class SyncAllRequest(BaseModel): + # The researcher whose experiments to bulk-copy. Defaults to whoever sync + # is currently armed for. + researcher: Optional[str] = None + + +@router.get("", response_model=RemoteSyncStatus) +async def get_remote_sync(state: AppState = Depends(get_state)): + return state.sync.status() + + +@router.put("", response_model=RemoteSyncStatus) +async def put_remote_sync(update: RemoteSyncUpdate, state: AppState = Depends(get_state)): + """Patch the config. Only fields actually sent are touched. + + `password` is write-only: it goes into process memory and is not echoed + back, not persisted, and not logged. + """ + sync = state.sync + settings = sync.settings + + if update.server is not None: + try: + settings.server = validate_remote_server(update.server) + except ValueError as e: + raise HTTPException(400, str(e)) + if update.username is not None: + # An empty username is how the UI clears the field mid-edit; only + # validate something that is actually being set. + if update.username.strip(): + try: + settings.username = validate_remote_username(update.username) + except ValueError as e: + raise HTTPException(400, str(e)) + else: + settings.username = "" + if update.researcher is not None and update.researcher.strip(): + settings.researcher = update.researcher.strip() + if update.password is not None: + sync.set_password(update.password) + + if update.enabled is not None: + if update.enabled: + if not settings.username or not sync.password_set: + raise HTTPException(400, "a username and password are required to switch sync on") + if not settings.researcher: + raise HTTPException(400, "no active researcher — set a user name on the home screen first") + settings.enabled = True + else: + settings.enabled = False + # Drop the session password with the toggle: leaving it in memory + # after the user has explicitly turned sync off serves no purpose. + sync.clear_password() + await sync.unmount() + + sync.persist() + return sync.status() + + +@router.post("/check", response_model=CheckConnectionResult) +async def check_connection(state: AppState = Depends(get_state)): + """Mount (if needed) and prove the destination is actually writable. + + Reports the real error text from mount rather than a generic failure -- + "wrong password" and "host unreachable" need different fixes. + """ + sync = state.sync + if not sync.settings.username or not sync.password_set: + raise HTTPException(400, "a username and password are required") + ok, message = await sync.check_connection() + return CheckConnectionResult(ok=ok, message=message, status=sync.status()) + + +@router.post("/sync-all", response_model=RemoteSyncStatus) +async def sync_all(request: SyncAllRequest, state: AppState = Depends(get_state)): + """One-shot bulk copy of every local experiment belonging to this researcher. + + Returns immediately; the copy runs on the same background worker as + per-capture syncing, so it can never block an experiment. + """ + sync = state.sync + researcher = (request.researcher or sync.settings.researcher or "").strip() + if not researcher: + raise HTTPException(400, "no researcher given") + if not sync.settings.enabled or not sync.password_set: + raise HTTPException( + 400, + "remote sync is not active — switch it on and enter the password " + "(it is not stored and must be re-entered after a restart)", + ) + sync.enqueue_bulk(researcher) + return sync.status() diff --git a/back/rapidboxes/config.py b/back/rapidboxes/config.py index c65124d..9d96ce2 100644 --- a/back/rapidboxes/config.py +++ b/back/rapidboxes/config.py @@ -73,6 +73,12 @@ class AppConfig(BaseSettings): # convention as settings_path. update_history_path: Path = Path.home() / "rapidboxes" / "update_history.json" + # Remote CIFS sync (Settings -> General -> Remote Sync). Only the + # non-secret half lives here -- server, CIFS username, on/off, researcher. + # The password is session-only and is never written to this (or any) file; + # see rapidboxes/remote_sync.py. + remote_sync_path: Path = Path.home() / "rapidboxes" / "remote_sync.json" + def ensure_dirs(self) -> None: self.storage_root.mkdir(parents=True, exist_ok=True) self.settings_path.parent.mkdir(parents=True, exist_ok=True) diff --git a/back/rapidboxes/engine/runner.py b/back/rapidboxes/engine/runner.py index 505024a..deb1816 100644 --- a/back/rapidboxes/engine/runner.py +++ b/back/rapidboxes/engine/runner.py @@ -11,6 +11,7 @@ import logging from dataclasses import dataclass from datetime import datetime +from pathlib import Path from typing import Awaitable, Callable, List, Optional, Set, Union from .. import config_xml @@ -112,12 +113,19 @@ def __init__( now: Optional[Callable[[], float]] = None, sleep: Optional[Callable[[float], Awaitable[None]]] = None, tick_seconds: float = 1.0, + on_image_captured: Optional[Callable[[Path, str, str], None]] = None, ): self._hw = hw self._storage = storage self._now = now or (lambda: asyncio.get_event_loop().time()) self._sleep = sleep or asyncio.sleep self._tick = tick_seconds + # Notified (path, experiment_id, username) right after each capture, so + # remote sync can queue a copy. MUST be synchronous, non-blocking and + # non-throwing -- see _capture, where its failure is swallowed: the + # local experiment's schedule is paramount, the remote copy is + # best-effort. + self._on_image_captured = on_image_captured self.status = ExperimentStatus() self._task: Optional[asyncio.Task] = None @@ -396,6 +404,17 @@ async def _capture( self.status.imagesCaptured = idx + 1 self.status.lastImageId = image_id self._write_metadata(exp) + + # Hand the new image to remote sync (if configured). This only drops a + # job on an in-memory queue -- no I/O, no await, no exception escapes: + # a hung or dead network share must never delay the next capture or + # fail the run. + if self._on_image_captured is not None: + try: + self._on_image_captured(path, exp.experiment_id, config.username) + except Exception: + log.warning("remote sync notification failed; experiment continues", exc_info=True) + await self._broadcast() def _write_metadata(self, exp: ExperimentDir) -> None: diff --git a/back/rapidboxes/main.py b/back/rapidboxes/main.py index 31ca695..64c15b4 100644 --- a/back/rapidboxes/main.py +++ b/back/rapidboxes/main.py @@ -18,9 +18,20 @@ from .config import AppConfig, get_config from .engine.runner import ExperimentRunner from .hardware.manager import build_hardware +from .remote_sync import RemoteSyncService, load_remote_sync_settings from .settings_store import load_device_settings_for_new_session from .storage import Storage -from .api import experiments, health, images, preview, settings as settings_api, system, update, ws +from .api import ( + experiments, + health, + images, + preview, + remote_sync as remote_sync_api, + settings as settings_api, + system, + update, + ws, +) from .api.deps import AppState logging.basicConfig(level=logging.INFO, format="%(asctime)s %(levelname)s %(name)s: %(message)s") @@ -33,14 +44,33 @@ async def lifespan(app: FastAPI): device_settings = load_device_settings_for_new_session(config.settings_path) storage = Storage(config.storage_root) hw = build_hardware(config, device_settings) - runner = ExperimentRunner(hw, storage) + + # Remote CIFS sync. The persisted half (server/user/on-off) survives a + # restart; the password deliberately does not, so a box that comes back up + # with `enabled: true` reports credentialsRequired until a human re-enters + # it -- see remote_sync.py and the "Remote Sync" card in the UI. + sync = RemoteSyncService( + load_remote_sync_settings(config.remote_sync_path), + storage_root=config.storage_root, + simulation=config.simulation, + settings_path=config.remote_sync_path, + ) + sync.start() + if sync.credentials_required: + log.warning( + "remote sync is switched on but has no password after this restart; " + "it stays inactive until the password is re-entered in Settings" + ) + + runner = ExperimentRunner(hw, storage, on_image_captured=sync.enqueue_image) runner.recover() - app.state.app = AppState(config, device_settings, storage, hw, runner) + app.state.app = AppState(config, device_settings, storage, hw, runner, sync) log.info("RaPiD-boxes started (simulation=%s, storage=%s)", config.simulation, config.storage_root) try: yield finally: await runner.shutdown() + await sync.shutdown() log.info("RaPiD-boxes stopped; hardware released") @@ -57,7 +87,16 @@ def create_app(config: Optional[AppConfig] = None) -> FastAPI: allow_headers=["*"], ) - for module in (experiments, images, settings_api, system, update, preview, health): + for module in ( + experiments, + images, + settings_api, + remote_sync_api, + system, + update, + preview, + health, + ): app.include_router(module.router) app.include_router(ws.router) diff --git a/back/rapidboxes/models.py b/back/rapidboxes/models.py index f1ba5de..397c654 100644 --- a/back/rapidboxes/models.py +++ b/back/rapidboxes/models.py @@ -4,6 +4,7 @@ """ from __future__ import annotations +import re from datetime import datetime from enum import Enum from typing import Annotated, List, Literal, Optional, Tuple, Union @@ -363,3 +364,133 @@ class VersionStatus(BaseModel): class StartResponse(BaseModel): status: str # "started" | "busy" | "no_camera" experimentId: Optional[str] = None + + +# --------------------------------------------------------------------------- +# Remote CIFS/SMB sync (Settings -> General -> Remote Sync). +# See rapidboxes/remote_sync.py for the mount + copy plumbing. +# +# SECURITY: there is deliberately NO password field on any model below that is +# persisted or returned by the API. The password is accepted only on +# RemoteSyncUpdate (write-only, PUT body) and lives in process memory for the +# lifetime of the process -- never in settings files, never in a response, +# never in a process argument list. `passwordSet` is the only thing the UI +# learns about it. +# --------------------------------------------------------------------------- + +# Pre-filled default, from the institutional share the legacy script mounted. +DEFAULT_REMOTE_SERVER = "//ds.asuch.cas.cz/ueb/lhr" + +# Strict allowlist for the //host/share[/path] string. This value is passed to +# a *sudo* mount command, so anything outside this pattern is rejected outright +# rather than escaped: no spaces, no commas, no leading "-" (which mount would +# read as an option), no shell metacharacters, no relative segments. +REMOTE_SERVER_PATTERN = r"^//[A-Za-z0-9][A-Za-z0-9._-]*(?:/[A-Za-z0-9][A-Za-z0-9._-]*)+$" +_REMOTE_SERVER_RE = re.compile(REMOTE_SERVER_PATTERN) +REMOTE_SERVER_MAX_LEN = 255 + +# The CIFS account name also reaches the (root) mount, inside the credentials +# file rather than on the command line -- but a newline there would let a +# crafted username inject extra credential directives, so it is constrained too. +REMOTE_USERNAME_PATTERN = r"^[A-Za-z0-9][A-Za-z0-9._@\\-]{0,63}$" +_REMOTE_USERNAME_RE = re.compile(REMOTE_USERNAME_PATTERN) + + +def validate_remote_server(server: str) -> str: + """Return `server` if it is a safe //host/share[/path], else raise ValueError.""" + value = (server or "").strip() + if not value: + raise ValueError("server/share path is required") + if len(value) > REMOTE_SERVER_MAX_LEN: + raise ValueError(f"server/share path is too long (max {REMOTE_SERVER_MAX_LEN} characters)") + if not _REMOTE_SERVER_RE.match(value): + raise ValueError( + "server/share path must look like //host/share or //host/share/folder " + "(letters, digits, dots, hyphens and underscores only)" + ) + if any(segment == ".." for segment in value.split("/")): + raise ValueError("server/share path must not contain '..' segments") + return value + + +def validate_remote_username(username: str) -> str: + """Return `username` if it is a safe CIFS account name, else raise ValueError.""" + value = (username or "").strip() + if not value: + raise ValueError("username is required") + if not _REMOTE_USERNAME_RE.match(value): + raise ValueError( + "username may only contain letters, digits, dots, underscores, " + "hyphens, '@' and '\\'" + ) + return value + + +class RemoteSyncSettings(BaseModel): + """The persisted (non-secret) half of the remote-sync configuration. + + Written to its own JSON file rather than into DeviceSettings: it is not a + property of how an image was taken, so it must not travel into the + per-experiment config XML — and keeping it separate means the password can + never be swept into a settings snapshot by accident. + """ + + enabled: bool = False + server: str = DEFAULT_REMOTE_SERVER + # The CIFS/SMB account used to mount the share. + username: str = "" + # The researcher whose experiments sync (the destination subfolder on the + # share). Captured when sync is switched on; sync stops if it changes. + researcher: str = "" + + @model_validator(mode="after") + def _check(self) -> "RemoteSyncSettings": + if self.server: + validate_remote_server(self.server) + if self.username: + validate_remote_username(self.username) + return self + + +class RemoteSyncUpdate(BaseModel): + """PUT /api/settings/remote-sync body — the only model carrying a password. + + Every field is optional so the UI can patch one thing at a time (e.g. flip + the toggle without resending credentials). `password` is WRITE-ONLY: it is + never echoed back by any endpoint and never written to disk. + """ + + enabled: Optional[bool] = None + server: Optional[str] = None + username: Optional[str] = None + password: Optional[str] = Field(default=None, max_length=256) + researcher: Optional[str] = None + + +class RemoteSyncStatus(BaseModel): + """GET /api/settings/remote-sync — everything the UI shows. No password.""" + + enabled: bool = False + server: str = DEFAULT_REMOTE_SERVER + username: str = "" + # Whether a password is held in memory. Never the password, never its length. + passwordSet: bool = False + mounted: bool = False + # The loud state: sync is switched on but the in-memory password is gone + # (fresh process after a restart/reboot/OTA update), so nothing can sync + # until a human re-enters it. + credentialsRequired: bool = False + # Destination subfolder on the share = the researcher this sync is armed for. + researcher: str = "" + remotePath: Optional[str] = None + # Images captured but not yet copied (queued + failed-and-awaiting-retry). + pendingCount: int = 0 + lastSyncAt: Optional[datetime] = None + lastResult: Optional[str] = None # "ok" | "error" + lastError: Optional[str] = None + # Progress/outcome text for the last "Sync entire folder now" run. + bulkInProgress: bool = False + bulkMessage: Optional[str] = None + # True on a dev laptop (RAPIDBOXES_SIMULATION=1): no real CIFS mount is + # attempted; the share is emulated by a local directory. + simulation: bool = False diff --git a/back/rapidboxes/remote_sync.py b/back/rapidboxes/remote_sync.py new file mode 100644 index 0000000..0415c6f --- /dev/null +++ b/back/rapidboxes/remote_sync.py @@ -0,0 +1,754 @@ +"""Remote CIFS/SMB sync: mount an institutional share and copy images to it. + +Replaces the legacy hardcoded + + sudo mount -t cifs //ds.asuch.cas.cz/ueb/lhr /mnt/Shared -o user=...,pass=... + +with something configurable from the UI and, crucially, safe on a box that has +no API authentication and binds 0.0.0.0. + +Remote layout (mirrors the legacy convention): + + /// + +SECURITY NOTES (all four are load-bearing, please keep them): + +1. **The password is session-only.** It lives in `_password` on this object and + nowhere else: not in remote_sync.json, not in settings.json, not in a + credentials file that outlives the mount call, not in a log line. A restart + (including the monthly OTA one) loses it by design, which the API surfaces + as `credentialsRequired` so the UI can say so loudly. + +2. **The password never reaches a process argument list.** `ps aux` is + world-readable, so `-o pass=...` is not an option. It is written to a + 0600 file created with `tempfile.mkstemp` and passed as `-o credentials=`; + the file is unlinked in a `finally` the moment `mount` returns, success or + failure. (The stdin alternative was rejected: `mount.cifs` only prompts + under conditions we would have to guess at, and a mis-guess hangs the + mount; the credentials file is the documented, deterministic route.) + +3. **No `shell=True`, ever.** Every subprocess is a fixed argument list. The + server string is additionally validated against a strict allowlist + (`validate_remote_server` in models.py) before it can reach the command, so + a value like `//h/s -o suid` or `-oremount` cannot smuggle mount options. + +4. **The sudo grant is narrow.** deploy/install.sh writes an /etc/sudoers.d + entry allowing exactly this mount (fixed mount point, fixed trailing option + string) and the matching umount -- never a blanket ALL. The hardening + options are deliberately the *last* thing on the option string: mount + options are last-one-wins, so even if something unexpected were injected + earlier, `nosuid,nodev,noexec` and the unprivileged uid/gid still win. +""" +from __future__ import annotations + +import asyncio +import logging +import os +import re +import shutil +import subprocess +import tempfile +from collections import OrderedDict +from dataclasses import dataclass, field +from datetime import datetime +from pathlib import Path +from typing import List, Optional, Sequence, Tuple + +from .models import ( + RemoteSyncSettings, + RemoteSyncStatus, + validate_remote_server, + validate_remote_username, +) + +log = logging.getLogger("rapidboxes.remote_sync") + +# Where the share is mounted on the Pi. Fixed (not user-settable) precisely so +# the sudoers rule can pin it literally. +MOUNT_POINT = Path("/mnt/rapidboxes-remote") + +# Credentials files are created here when it exists: the systemd unit declares +# `RuntimeDirectory=rapidboxes-cifs` (mode 0700, owned by the service user), and +# the sudoers rule pins this directory in the allowed `credentials=` path. +# /run is tmpfs, so nothing survives a reboot even if a cleanup were missed. +CREDENTIALS_DIR = Path("/run/rapidboxes-cifs") +CREDENTIALS_PREFIX = "cred-" + +# Absolute paths, preferred in order: sudoers matches the literal command we +# invoke, so this must agree with what deploy/install.sh writes. +_SUDO_CANDIDATES = ("/usr/bin/sudo", "/bin/sudo") +_MOUNT_CANDIDATES = ("/usr/bin/mount", "/bin/mount") +_UMOUNT_CANDIDATES = ("/usr/bin/umount", "/bin/umount") + +MOUNT_TIMEOUT_S = 25.0 +UMOUNT_TIMEOUT_S = 15.0 +COPY_TIMEOUT_S = 120.0 + +# A failed item is retried on the next capture (see _flush_failed); this caps +# how many attempts one file gets before it is dropped from the retry set, so a +# permanently unreadable file cannot wedge the queue forever. +MAX_ATTEMPTS = 5 +# Bound on the backlog we keep in memory, so a week-long run against a dead +# share cannot grow without limit. +MAX_PENDING = 2000 + +_SAFE = re.compile(r"[^A-Za-z0-9._-]+") + + +def _slug(text: str) -> str: + return _SAFE.sub("-", (text or "").strip()) or "x" + + +def _first_existing(candidates: Sequence[str], name: str) -> Optional[str]: + for path in candidates: + if os.path.exists(path): + return path + found = shutil.which(name) + return found + + +def sudo_binary() -> Optional[str]: + return _first_existing(_SUDO_CANDIDATES, "sudo") + + +def mount_binary() -> Optional[str]: + return _first_existing(_MOUNT_CANDIDATES, "mount") + + +def umount_binary() -> Optional[str]: + return _first_existing(_UMOUNT_CANDIDATES, "umount") + + +def fixed_mount_options() -> str: + """The literal, non-negotiable tail of the mount option string. + + Kept last on purpose (mount options are last-one-wins) and mirrored + verbatim by the sudoers rule deploy/install.sh installs. + """ + return ( + "nosuid,nodev,noexec," + f"uid={os.getuid()},gid={os.getgid()}," + "file_mode=0664,dir_mode=0775" + ) + + +def build_mount_argv(server: str, mount_point: Path, credentials_path: str) -> List[str]: + """The exact argument list handed to subprocess.run (never a shell string). + + `server` must already have passed validate_remote_server(). + """ + sudo = sudo_binary() + mount = mount_binary() + if sudo is None or mount is None: + raise FileNotFoundError("sudo/mount not available on this system") + return [ + sudo, + "-n", # never prompt: the sudoers grant is NOPASSWD or we fail loudly + mount, + "-t", + "cifs", + server, + str(mount_point), + "-o", + f"credentials={credentials_path},{fixed_mount_options()}", + ] + + +def build_umount_argv(mount_point: Path) -> List[str]: + sudo = sudo_binary() + umount = umount_binary() + if sudo is None or umount is None: + raise FileNotFoundError("sudo/umount not available on this system") + return [sudo, "-n", umount, str(mount_point)] + + +def _write_credentials_file(username: str, password: str) -> str: + """Create a 0600 credentials file and return its path. + + The caller MUST unlink it in a `finally` as soon as mount returns. + """ + directory = CREDENTIALS_DIR if os.path.isdir(CREDENTIALS_DIR) else None + fd, path = tempfile.mkstemp(prefix=CREDENTIALS_PREFIX, dir=str(directory) if directory else None) + try: + os.fchmod(fd, 0o600) # mkstemp is already 0600; make it explicit and robust + with os.fdopen(fd, "w") as f: + f.write("username=%s\npassword=%s\n" % (username, password)) + except Exception: + try: + os.unlink(path) + except OSError: + pass + raise + return path + + +def is_mount_point(path: Path) -> bool: + try: + return path.is_mount() # py3.7+: Path.is_mount + except Exception: + return False + + +@dataclass +class _Job: + kind: str # "image" | "bulk" + researcher: str + experiment_id: str = "" + files: List[Path] = field(default_factory=list) + + +@dataclass +class _Pending: + src: Path + researcher: str + experiment_id: str + attempts: int = 0 + + +class RemoteSyncService: + """Owns the mount, the session password, and the background copy queue. + + Every public coroutine is safe to call from an API handler. The only entry + point used from the capture path is `enqueue_image`, which is synchronous, + non-blocking and swallows everything: a dead share must never delay or fail + an experiment. + """ + + def __init__( + self, + settings: RemoteSyncSettings, + *, + storage_root: Path, + simulation: bool = False, + settings_path: Optional[Path] = None, + mount_point: Path = MOUNT_POINT, + ): + self.settings = settings + self._storage_root = storage_root + self._settings_path = settings_path + self._mount_point = mount_point + # Simulation covers both "dev laptop" and "no mount binary here". + self.simulation = simulation or mount_binary() is None + self._sim_root = storage_root.parent / "remote-sim" + + # Session-only. Never persisted, never serialised, never logged. + self._password: Optional[str] = None + + self._mounted = False + self._queue: "asyncio.Queue[_Job]" = asyncio.Queue() + self._worker: Optional[asyncio.Task] = None + self._pending: "OrderedDict[str, _Pending]" = OrderedDict() + self._last_sync_at: Optional[datetime] = None + self._last_result: Optional[str] = None + self._last_error: Optional[str] = None + self._bulk_in_progress = False + self._bulk_message: Optional[str] = None + self._mount_lock = asyncio.Lock() + self._unmount_task: Optional[asyncio.Future] = None + + # --- lifecycle ------------------------------------------------------- + def start(self) -> None: + if self._worker is None or self._worker.done(): + self._worker = asyncio.create_task(self._run_worker()) + + async def shutdown(self) -> None: + if self._worker is not None: + self._worker.cancel() + try: + await self._worker + except asyncio.CancelledError: + pass + except Exception: # pragma: no cover - best effort + log.debug("remote sync worker raised on shutdown", exc_info=True) + self._worker = None + # The password dies with the process, so a mount left behind could + # never be re-established anyway -- take it down cleanly. + await self.unmount() + self._password = None + + # --- configuration --------------------------------------------------- + @property + def password_set(self) -> bool: + return bool(self._password) + + @property + def credentials_required(self) -> bool: + """Switched on, but with no password in memory: the post-restart state.""" + return self.settings.enabled and not self._password + + def set_password(self, password: str) -> None: + self._password = password or None + + def clear_password(self) -> None: + self._password = None + + def note_active_researcher(self, researcher: str) -> None: + """Called when an experiment starts. + + Per spec, sync runs "until synchro is off, or until the user changes": + a different researcher means the destination folder would change under + someone's feet, so sync switches itself off and says why. + """ + name = (researcher or "").strip() + if not name or not self.settings.enabled: + return + if self.settings.researcher and name != self.settings.researcher: + log.warning( + "remote sync stopped: researcher changed from %r to %r", + self.settings.researcher, + name, + ) + previous = self.settings.researcher + self.settings.enabled = False + self._last_result = "error" + self._last_error = ( + "Sync stopped: the researcher changed from '%s' to '%s'. " + "Switch sync back on to sync as '%s'." % (previous, name, name) + ) + self.persist() + # Fire-and-forget, but keep a reference: a bare ensure_future can + # be garbage-collected mid-flight. Never awaited here, because this + # runs on the experiment-start path. + try: + self._unmount_task = asyncio.ensure_future(self.unmount()) + except RuntimeError: # no running loop (unit tests / sync callers) + pass + + def persist(self) -> None: + """Write the non-secret settings. There is no password to leak here: + RemoteSyncSettings has no such field at all.""" + if self._settings_path is None: + return + try: + save_remote_sync_settings(self._settings_path, self.settings) + except Exception: + log.exception("failed to persist remote sync settings") + + # --- mounting -------------------------------------------------------- + @property + def mounted(self) -> bool: + """The cached flag, deliberately NOT a fresh stat(). + + Statting a hung CIFS mount blocks -- uninterruptibly, for however long + the kernel takes to give up. This property is read by GET + /api/settings/remote-sync, which the settings panel polls every 5s, so + touching the filesystem here would let a dead share freeze the whole + (single-process) API. Every real filesystem check happens on a worker + thread inside mount/unmount and refreshes this flag. + """ + return self._mounted + + def remote_root(self) -> Path: + """Where the share is (or would be) reachable in the filesystem.""" + if self.simulation: + return self._sim_root / _slug(self.settings.server) + return self._mount_point + + def remote_path_for(self, researcher: str) -> Path: + return self.remote_root() / _slug(researcher or self.settings.researcher) + + async def mount(self) -> Tuple[bool, str]: + """Mount the share (idempotent). Returns (ok, message).""" + async with self._mount_lock: + return await self._mount_locked() + + async def _mount_locked(self) -> Tuple[bool, str]: + try: + server = validate_remote_server(self.settings.server) + username = validate_remote_username(self.settings.username) + except ValueError as e: + return False, str(e) + if not self._password: + return False, "Password required — it is not stored and must be re-entered after a restart." + + if self.simulation: + # No CIFS server on a dev laptop: emulate the share with a local + # directory so the whole sync path stays exercisable. + try: + self.remote_root().mkdir(parents=True, exist_ok=True) + except Exception as e: + return False, "Simulated share unavailable: %s" % e + self._mounted = True + return True, "Connected (simulation — no real CIFS mount was made)." + + # Everything below touches the filesystem, so it all happens on a + # worker thread with a timeout: a hung share must not block the loop. + try: + ok, message = await asyncio.wait_for( + asyncio.to_thread(self._mount_sync, server, username, self._password), + timeout=MOUNT_TIMEOUT_S + 10, + ) + except asyncio.TimeoutError: + return False, "Mount did not complete in time — is the share reachable?" + self._mounted = ok + return ok, message + + def _mount_sync(self, server: str, username: str, password: str) -> Tuple[bool, str]: + """Blocking mount, run in a worker thread. + + The credentials file exists only for the duration of the mount call. + """ + if is_mount_point(self._mount_point): + return True, "Already mounted at %s." % self._mount_point + + try: + self._mount_point.mkdir(parents=True, exist_ok=True) + except PermissionError: + return False, ( + "Mount point %s does not exist and could not be created — " + "re-run deploy/install.sh." % self._mount_point + ) + except Exception as e: + return False, "Could not prepare %s: %s" % (self._mount_point, e) + + try: + credentials_path = _write_credentials_file(username, password) + except Exception as e: + return False, "Could not create the credentials file: %s" % e + try: + argv = build_mount_argv(server, self._mount_point, credentials_path) + except FileNotFoundError as e: + self._unlink_quietly(credentials_path) + return False, str(e) + + try: + result = subprocess.run( # noqa: S603 - fixed argv, never shell=True + argv, + capture_output=True, + text=True, + timeout=MOUNT_TIMEOUT_S, + check=False, + ) + except subprocess.TimeoutExpired: + return False, "Mount timed out after %ds — is the share reachable?" % int(MOUNT_TIMEOUT_S) + except Exception as e: + return False, "Mount failed to run: %s" % e + finally: + # Unconditionally, immediately: the password must not survive on + # disk past this call under any outcome. + self._unlink_quietly(credentials_path) + + if result.returncode == 0: + return True, "Connected — share mounted at %s." % self._mount_point + detail = (result.stderr or result.stdout or "").strip() or ( + "mount exited with status %d" % result.returncode + ) + if "sudo:" in detail and "password" in detail.lower(): + detail += ( + " (the sudoers entry is missing — re-run deploy/install.sh on this box)" + ) + return False, detail + + @staticmethod + def _unlink_quietly(path: str) -> None: + try: + os.unlink(path) + except OSError: + pass + + async def unmount(self) -> Tuple[bool, str]: + if self.simulation: + self._mounted = False + return True, "Disconnected (simulation)." + try: + ok, message = await asyncio.wait_for( + asyncio.to_thread(self._umount_sync), timeout=UMOUNT_TIMEOUT_S + 10 + ) + except asyncio.TimeoutError: + return False, "umount did not complete in time." + self._mounted = not ok + return ok, message + + def _umount_sync(self) -> Tuple[bool, str]: + if not is_mount_point(self._mount_point): + return True, "Not mounted." + try: + argv = build_umount_argv(self._mount_point) + except FileNotFoundError as e: + return False, str(e) + try: + result = subprocess.run( # noqa: S603 - fixed argv, never shell=True + argv, capture_output=True, text=True, timeout=UMOUNT_TIMEOUT_S, check=False + ) + except Exception as e: + return False, "umount failed to run: %s" % e + if result.returncode == 0: + return True, "Disconnected." + return False, (result.stderr or result.stdout or "umount failed").strip() + + async def check_connection(self) -> Tuple[bool, str]: + """Settings -> "Check connection": mount if needed and verify writability.""" + ok, message = await self.mount() + if not ok: + self._last_result = "error" + self._last_error = message + return False, message + # A mount that is read-only (or points somewhere unexpected) would fail + # every later copy silently, so prove we can actually write now. + target = self.remote_path_for(self.settings.researcher) + try: + await asyncio.wait_for(asyncio.to_thread(self._probe_writable, target), timeout=30.0) + except asyncio.TimeoutError: + self._last_result = "error" + self._last_error = "The share did not respond within 30s." + return False, self._last_error + except Exception as e: + self._last_result = "error" + self._last_error = "Mounted, but the destination folder is not writable: %s" % e + return False, self._last_error + self._last_result = "ok" + self._last_error = None + return True, message + + @staticmethod + def _probe_writable(target: Path) -> None: + target.mkdir(parents=True, exist_ok=True) + probe = target / ".rapidboxes-write-test" + probe.write_text("ok") + probe.unlink() + + # --- the capture path (must never block or raise) -------------------- + def enqueue_image(self, image_path: Path, experiment_id: str, researcher: str) -> None: + """Queue one just-captured image for copying. Fire-and-forget. + + Called from the experiment runner immediately after a capture. It does + no I/O, never awaits and never raises: a slow, hung or dead share must + not delay the deadline scheduler by even a millisecond. + """ + try: + if not self.settings.enabled or not self._password: + return + files = [image_path] + metadata = image_path.parent / "metadata.json" + if metadata.exists(): + files.append(metadata) + self._queue.put_nowait( + _Job(kind="image", researcher=researcher, experiment_id=experiment_id, files=files) + ) + except Exception: # pragma: no cover - defensive + log.debug("could not queue image for remote sync", exc_info=True) + + def enqueue_bulk(self, researcher: str) -> None: + self._bulk_in_progress = True + self._bulk_message = "Queued…" + self._queue.put_nowait(_Job(kind="bulk", researcher=researcher)) + + # --- background worker ---------------------------------------------- + async def _run_worker(self) -> None: + while True: + job = await self._queue.get() + try: + if job.kind == "image": + await self._handle_image_job(job) + elif job.kind == "bulk": + await self._handle_bulk_job(job) + except asyncio.CancelledError: + raise + except Exception: # pragma: no cover - defensive + log.exception("remote sync worker error") + finally: + self._queue.task_done() + + async def _handle_image_job(self, job: _Job) -> None: + if not self.settings.enabled or not self._password: + self._remember_pending(job) + return + # Retry whatever failed earlier before adding more, so the remote + # catches up in capture order rather than leaving holes. + await self._flush_failed() + for src in job.files: + await self._copy_or_defer(src, job.researcher, job.experiment_id) + + async def _flush_failed(self) -> None: + if not self._pending: + return + for key, item in list(self._pending.items()): + if not self.settings.enabled or not self._password: + return + ok = await self._copy_one(item.src, item.researcher, item.experiment_id) + if ok: + self._pending.pop(key, None) + else: + item.attempts += 1 + if item.attempts >= MAX_ATTEMPTS: + log.warning("giving up on remote copy of %s after %d attempts", item.src, item.attempts) + self._pending.pop(key, None) + # One failure means the share is unhappy; don't hammer the rest + # of the backlog on this pass. + return + + async def _copy_or_defer(self, src: Path, researcher: str, experiment_id: str) -> None: + ok = await self._copy_one(src, researcher, experiment_id) + if not ok: + self._defer(src, researcher, experiment_id) + + def _defer(self, src: Path, researcher: str, experiment_id: str) -> None: + key = str(src) + if key in self._pending: + return + if len(self._pending) >= MAX_PENDING: + self._pending.popitem(last=False) + self._pending[key] = _Pending(src=src, researcher=researcher, experiment_id=experiment_id) + + def _remember_pending(self, job: _Job) -> None: + for src in job.files: + self._defer(src, job.researcher, job.experiment_id) + + async def _copy_one(self, src: Path, researcher: str, experiment_id: str) -> bool: + """Copy one file to the share. Returns success; never raises.""" + try: + if not self.mounted: + ok, message = await self.mount() + if not ok: + self._last_result = "error" + self._last_error = message + return False + dest_dir = self.remote_path_for(researcher) / _slug(experiment_id) + await asyncio.wait_for( + asyncio.to_thread(self._copy_sync, src, dest_dir), + timeout=COPY_TIMEOUT_S, + ) + except asyncio.CancelledError: + raise + except asyncio.TimeoutError: + self._last_result = "error" + self._last_error = "Copy of %s timed out after %ds." % (src.name, int(COPY_TIMEOUT_S)) + log.warning("remote sync: %s", self._last_error) + return False + except Exception as e: + self._last_result = "error" + self._last_error = "Copy of %s failed: %s" % (src.name, e) + log.warning("remote sync: %s", self._last_error) + return False + self._last_result = "ok" + self._last_error = None + self._last_sync_at = datetime.now() + return True + + @staticmethod + def _copy_sync(src: Path, dest_dir: Path) -> None: + """Blocking copy, via a .part file so a half-written image is never + visible to whoever is watching the share.""" + dest_dir.mkdir(parents=True, exist_ok=True) + final = dest_dir / src.name + tmp = dest_dir / (src.name + ".part") + shutil.copyfile(src, tmp) + os.replace(tmp, final) + + # --- bulk "sync entire folder now" ----------------------------------- + async def _handle_bulk_job(self, job: _Job) -> None: + self._bulk_in_progress = True + try: + if not self.settings.enabled or not self._password: + self._bulk_message = "Cannot sync: credentials are needed after a restart." + return + ok, message = await self.mount() + if not ok: + self._bulk_message = "Could not connect: %s" % message + self._last_result = "error" + self._last_error = message + return + experiments = self._experiments_for(job.researcher) + if not experiments: + self._bulk_message = "No local experiments found for '%s'." % job.researcher + return + copied = 0 + failed = 0 + for index, exp_dir in enumerate(experiments, start=1): + self._bulk_message = "Copying %d/%d: %s" % (index, len(experiments), exp_dir.name) + for src in self._files_to_sync(exp_dir): + if await self._copy_one(src, job.researcher, exp_dir.name): + copied += 1 + else: + failed += 1 + self._defer(src, job.researcher, exp_dir.name) + self._bulk_message = "Copied %d file%s from %d experiment%s%s." % ( + copied, + "" if copied == 1 else "s", + len(experiments), + "" if len(experiments) == 1 else "s", + "" if not failed else " (%d failed — will retry)" % failed, + ) + finally: + self._bulk_in_progress = False + + def _experiments_for(self, researcher: str) -> List[Path]: + """This researcher's local experiment folders. + + Folders are named `__`, but metadata.json carries + the authoritative username, so prefer that and fall back to the name. + """ + if not self._storage_root.exists(): + return [] + wanted = _slug(researcher) + out: List[Path] = [] + for path in sorted(p for p in self._storage_root.iterdir() if p.is_dir()): + owner = self._experiment_owner(path) + if owner is not None and _slug(owner) == wanted: + out.append(path) + return out + + @staticmethod + def _experiment_owner(path: Path) -> Optional[str]: + meta = path / "metadata.json" + if meta.exists(): + try: + import json + + data = json.loads(meta.read_text()) + username = data.get("username") + if username: + return str(username) + except Exception: + pass + parts = path.name.split("_") + return parts[1] if len(parts) >= 3 else None + + @staticmethod + def _files_to_sync(exp_dir: Path) -> List[Path]: + """Everything worth having on the share: images, metadata, config XML. + Locally-regenerable thumbnails are skipped.""" + files = sorted(p for p in exp_dir.iterdir() if p.is_file() and not p.name.endswith(".part")) + return files + + # --- status ---------------------------------------------------------- + def status(self) -> RemoteSyncStatus: + researcher = self.settings.researcher + return RemoteSyncStatus( + enabled=self.settings.enabled, + server=self.settings.server, + username=self.settings.username, + passwordSet=self.password_set, + mounted=self.mounted, + credentialsRequired=self.credentials_required, + researcher=researcher, + remotePath=str(self.remote_path_for(researcher)) if researcher else None, + pendingCount=len(self._pending) + self._queue.qsize(), + lastSyncAt=self._last_sync_at, + lastResult=self._last_result, + lastError=self._last_error, + bulkInProgress=self._bulk_in_progress, + bulkMessage=self._bulk_message, + simulation=self.simulation, + ) + + +# --------------------------------------------------------------------------- +# Persistence of the non-secret settings +# --------------------------------------------------------------------------- + + +def load_remote_sync_settings(path: Path) -> RemoteSyncSettings: + if path.exists(): + try: + return RemoteSyncSettings.model_validate_json(path.read_text()) + except Exception: + log.exception("invalid remote sync settings %s; using defaults", path) + return RemoteSyncSettings() + + +def save_remote_sync_settings(path: Path, settings: RemoteSyncSettings) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + tmp = path.with_suffix(".tmp") + tmp.write_text(settings.model_dump_json(indent=2)) + os.replace(tmp, path) diff --git a/back/tests/test_api.py b/back/tests/test_api.py index 91e5e6c..f0b8d55 100644 --- a/back/tests/test_api.py +++ b/back/tests/test_api.py @@ -25,6 +25,9 @@ def app_config(tmp_path: Path) -> AppConfig: simulation=True, storage_root=tmp_path / "experiments", settings_path=tmp_path / "settings.json", + # Keep remote-sync state in the tmpdir too, so tests never read or + # write the developer's real ~/rapidboxes/remote_sync.json. + remote_sync_path=tmp_path / "remote_sync.json", spa_dir=None, ) diff --git a/back/tests/test_remote_sync.py b/back/tests/test_remote_sync.py new file mode 100644 index 0000000..3a6b09a --- /dev/null +++ b/back/tests/test_remote_sync.py @@ -0,0 +1,670 @@ +"""Remote CIFS sync: input validation, password secrecy, and failure isolation. + +Nothing here mounts anything for real -- every subprocess call is mocked. The +security properties under test are the ones the feature lives or dies by: + + * a malicious server string can never reach the sudo mount command + * the password never appears in an API response, on disk, or in an argv + * the credentials file is 0600 and is gone the moment mount returns + * a broken share never breaks a running experiment +""" +from __future__ import annotations + +import ast +import asyncio +import json +import os +import subprocess +from pathlib import Path +from typing import List, Optional + +import pytest +from httpx import ASGITransport, AsyncClient + +from rapidboxes import remote_sync as rs +from rapidboxes.config import AppConfig +from rapidboxes.main import create_app +from rapidboxes.models import ( + RemoteSyncSettings, + RemoteSyncStatus, + RemoteSyncUpdate, + TropismConfig, + validate_remote_server, + validate_remote_username, +) + +# A value distinctive enough that finding it anywhere is unambiguous. +SECRET = "correct-horse-battery-staple-9271" + + +@pytest.fixture +def app_config(tmp_path: Path) -> AppConfig: + return AppConfig( + simulation=True, + storage_root=tmp_path / "experiments", + settings_path=tmp_path / "settings.json", + remote_sync_path=tmp_path / "remote_sync.json", + spa_dir=None, + ) + + +@pytest.fixture +async def client(app_config: AppConfig): + app = create_app(app_config) + async with app.router.lifespan_context(app): + transport = ASGITransport(app=app) + async with AsyncClient(transport=transport, base_url="http://test") as ac: + ac._app = app # type: ignore[attr-defined] - tests reach for app.state.app + yield ac + + +def make_service(tmp_path: Path, **kwargs) -> rs.RemoteSyncService: + settings = kwargs.pop( + "settings", + RemoteSyncSettings( + enabled=True, server="//host.example.org/share/sub", username="LHR", researcher="alice" + ), + ) + service = rs.RemoteSyncService( + settings, + storage_root=kwargs.pop("storage_root", tmp_path / "experiments"), + simulation=kwargs.pop("simulation", True), + settings_path=kwargs.pop("settings_path", tmp_path / "remote_sync.json"), + mount_point=kwargs.pop("mount_point", tmp_path / "mnt"), + ) + service.set_password(kwargs.pop("password", SECRET)) + return service + + +# --------------------------------------------------------------------------- +# 1. Server/username validation -- the string that reaches a sudo command +# --------------------------------------------------------------------------- + +MALICIOUS_SERVERS = [ + "-o", # bare option + "-oremount,suid", # smuggled option, no space needed + "--bind", + "//host/share,suid", # comma splits the option list + "//host/share -o suid", # extra argv words + "//host/share\t-o\tsuid", + "//host/share\n//other/share", # newline injection + "//host/share;rm -rf /", # shell metacharacters (defence in depth) + "//host/share`id`", + "//host/share$(id)", + "//host/share|nc evil 1234", + "//host/share&&reboot", + "//host/../../etc/shadow", # traversal + "//host/share/../..", + "/host/share", # single leading slash + "///host/share", + "//host", # no share component + "//", + "", + " ", + "\\\\host\\share", # UNC backslash form is not accepted + "//host/share/'", + '//host/share/"', + "//ho st/share", # embedded space + "//host:2049/share", # colon is not in the allowlist + "//host/share#frag", + "//" + "a" * 300 + "/share", # over the length cap +] + +LEGITIMATE_SERVERS = [ + "//ds.asuch.cas.cz/ueb/lhr", # the pre-filled default + "//nas1/data", + "//192.168.1.20/share/Pictures/Raps_pi", + "//file-server_2/my.share/sub_folder-3", +] + + +@pytest.mark.parametrize("value", MALICIOUS_SERVERS) +def test_validate_remote_server_rejects_malicious_values(value: str): + with pytest.raises(ValueError): + validate_remote_server(value) + + +@pytest.mark.parametrize("value", LEGITIMATE_SERVERS) +def test_validate_remote_server_accepts_legitimate_values(value: str): + assert validate_remote_server(value) == value + + +@pytest.mark.parametrize( + "value", + ["a b", "user\nname", "user,name", "-user", "user;id", "u" * 65, "", "user`id`"], +) +def test_validate_remote_username_rejects_malicious_values(value: str): + with pytest.raises(ValueError): + validate_remote_username(value) + + +@pytest.mark.parametrize("value", ["LHR", "domain\\user", "first.last", "user_1@example.org"]) +def test_validate_remote_username_accepts_legitimate_values(value: str): + assert validate_remote_username(value) == value + + +@pytest.mark.asyncio +@pytest.mark.parametrize("value", MALICIOUS_SERVERS[:12]) +async def test_api_rejects_malicious_server_strings(client: AsyncClient, value: str): + res = await client.put("/api/settings/remote-sync", json={"server": value}) + assert res.status_code == 400, f"{value!r} was accepted" + # And the bad value was not retained. + current = (await client.get("/api/settings/remote-sync")).json() + assert current["server"] != value + + +@pytest.mark.asyncio +async def test_api_rejects_malicious_server_even_when_sync_is_being_enabled(client: AsyncClient): + res = await client.put( + "/api/settings/remote-sync", + json={ + "server": "//host/share -o suid", + "username": "LHR", + "password": SECRET, + "researcher": "alice", + "enabled": True, + }, + ) + assert res.status_code == 400 + status = (await client.get("/api/settings/remote-sync")).json() + assert status["enabled"] is False + + +def test_settings_model_rejects_malicious_server_on_load(): + """A hand-edited remote_sync.json cannot smuggle a bad server in either.""" + with pytest.raises(Exception): + RemoteSyncSettings(server="//host/share -o suid") + + +# --------------------------------------------------------------------------- +# 2. The password must never be readable back out +# --------------------------------------------------------------------------- + +# Every GET the box exposes that could plausibly carry configuration. +GET_ENDPOINTS = [ + "/api/health", + "/api/system", + "/api/settings", + "/api/settings/remote-sync", + "/api/experiments/current", + "/api/experiments/history", + "/api/images", + "/api/system/update/version", + "/openapi.json", +] + + +@pytest.mark.asyncio +async def test_password_never_appears_in_any_get_response(client: AsyncClient): + """This box has no auth and binds 0.0.0.0 -- anyone on the LAN can curl it.""" + res = await client.put( + "/api/settings/remote-sync", + json={ + "server": "//ds.asuch.cas.cz/ueb/lhr", + "username": "LHR", + "password": SECRET, + "researcher": "alice", + "enabled": True, + }, + ) + assert res.status_code == 200 + # Not even the write that accepted it echoes it back. + assert SECRET not in res.text + assert res.json()["passwordSet"] is True + assert "password" not in res.json() + + for endpoint in GET_ENDPOINTS: + response = await client.get(endpoint) + assert SECRET not in response.text, f"password leaked from GET {endpoint}" + assert "correct-horse" not in response.text + + # And the same after a check-connection round trip, which handles the + # password most directly. + res = await client.post("/api/settings/remote-sync/check") + assert SECRET not in res.text + for endpoint in GET_ENDPOINTS: + response = await client.get(endpoint) + assert SECRET not in response.text, f"password leaked from GET {endpoint} after check" + + +def test_status_model_has_no_password_field(): + """Structural guarantee: there is no field for a password to be put in.""" + assert "password" not in RemoteSyncStatus.model_fields + assert "password" not in RemoteSyncSettings.model_fields + # ...and exactly one model accepts it, on the way in only. + assert "password" in RemoteSyncUpdate.model_fields + + +@pytest.mark.asyncio +async def test_password_is_never_written_to_disk(client: AsyncClient, app_config: AppConfig, tmp_path: Path): + await client.put( + "/api/settings/remote-sync", + json={ + "server": "//ds.asuch.cas.cz/ueb/lhr", + "username": "LHR", + "password": SECRET, + "researcher": "alice", + "enabled": True, + }, + ) + await client.post("/api/settings/remote-sync/check") + + assert app_config.remote_sync_path.exists(), "the non-secret half should persist" + persisted = json.loads(app_config.remote_sync_path.read_text()) + assert persisted["server"] == "//ds.asuch.cas.cz/ueb/lhr" + assert persisted["username"] == "LHR" + assert "password" not in persisted + + # Nothing anywhere under the data root may contain it. + for path in tmp_path.rglob("*"): + if path.is_file(): + try: + content = path.read_text(errors="ignore") + except OSError: + continue + assert SECRET not in content, f"password written to {path}" + + +@pytest.mark.asyncio +async def test_password_is_lost_on_restart_and_surfaces_as_credentials_required( + app_config: AppConfig, +): + """The session-only design's headline consequence, asserted end to end.""" + app = create_app(app_config) + async with app.router.lifespan_context(app): + transport = ASGITransport(app=app) + async with AsyncClient(transport=transport, base_url="http://test") as ac: + await ac.put( + "/api/settings/remote-sync", + json={ + "server": "//host.example.org/share", + "username": "LHR", + "password": SECRET, + "researcher": "alice", + "enabled": True, + }, + ) + status = (await ac.get("/api/settings/remote-sync")).json() + assert status["enabled"] is True + assert status["passwordSet"] is True + assert status["credentialsRequired"] is False + + # A fresh process over the same files -- exactly what a reboot or the + # monthly OTA restart produces. + app2 = create_app(app_config) + async with app2.router.lifespan_context(app2): + transport = ASGITransport(app=app2) + async with AsyncClient(transport=transport, base_url="http://test") as ac: + status = (await ac.get("/api/settings/remote-sync")).json() + assert status["enabled"] is True, "the switch setting itself survives" + assert status["passwordSet"] is False, "the password does not" + assert status["credentialsRequired"] is True, ( + "sync must report itself inactive rather than appear on-but-silent" + ) + assert status["mounted"] is False + + # And it refuses to pretend it can do anything in that state. + res = await ac.post("/api/settings/remote-sync/sync-all", json={"researcher": "alice"}) + assert res.status_code == 400 + assert "restart" in res.json()["detail"].lower() + + +# --------------------------------------------------------------------------- +# 3. The mount invocation: no password in argv, credentials file cleaned up +# --------------------------------------------------------------------------- + + +class FakeRun: + """Stands in for subprocess.run, recording what the mount was asked to do.""" + + def __init__(self, returncode: int = 0, raises: Optional[Exception] = None): + self.returncode = returncode + self.raises = raises + self.argv: List[str] = [] + self.kwargs: dict = {} + self.credentials_path: Optional[str] = None + self.credentials_content: Optional[str] = None + self.credentials_mode: Optional[int] = None + self.credentials_existed_during_call = False + + def __call__(self, argv, **kwargs): + self.argv = list(argv) + self.kwargs = kwargs + for arg in self.argv: + if arg.startswith("credentials=") or ",credentials=" in arg: + for option in arg.split(","): + if option.startswith("credentials="): + path = option.split("=", 1)[1] + self.credentials_path = path + self.credentials_existed_during_call = os.path.exists(path) + if self.credentials_existed_during_call: + self.credentials_content = Path(path).read_text() + self.credentials_mode = os.stat(path).st_mode & 0o777 + if self.raises is not None: + raise self.raises + return subprocess.CompletedProcess(self.argv, self.returncode, stdout="", stderr="mount error text") + + +@pytest.mark.asyncio +async def test_mount_passes_password_via_0600_credentials_file_not_argv( + tmp_path: Path, monkeypatch +): + fake = FakeRun(returncode=0) + monkeypatch.setattr(rs.subprocess, "run", fake) + + service = make_service(tmp_path, simulation=False) + ok, message = await service.mount() + assert ok, message + + # `ps aux` is world-readable: the password must not be anywhere in argv. + joined = " ".join(fake.argv) + assert SECRET not in joined + assert "pass=" not in joined + assert "password=" not in joined + + # It went through a credentials file instead... + assert fake.credentials_existed_during_call + assert fake.credentials_content == "username=LHR\npassword=%s\n" % SECRET + assert fake.credentials_mode == 0o600, "credentials file must not be readable by others" + + # ...which is gone the moment mount returned. + assert fake.credentials_path is not None + assert not os.path.exists(fake.credentials_path), "credentials file outlived the mount call" + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "fake", + [ + FakeRun(returncode=1), + FakeRun(raises=subprocess.TimeoutExpired(cmd="mount", timeout=25)), + FakeRun(raises=OSError("boom")), + ], + ids=["nonzero-exit", "timeout", "oserror"], +) +async def test_credentials_file_is_removed_even_when_mount_fails( + tmp_path: Path, monkeypatch, fake: FakeRun +): + monkeypatch.setattr(rs.subprocess, "run", fake) + + service = make_service(tmp_path, simulation=False) + ok, message = await service.mount() + assert ok is False + assert message # the real error text is surfaced, not a generic failure + + assert fake.credentials_path is not None + assert not os.path.exists(fake.credentials_path), "credentials file leaked on the failure path" + + +@pytest.mark.asyncio +async def test_mount_argv_shape_is_safe(tmp_path: Path, monkeypatch): + fake = FakeRun(returncode=0) + monkeypatch.setattr(rs.subprocess, "run", fake) + + service = make_service(tmp_path, simulation=False) + await service.mount() + + # A fixed argument list, never a shell string. + assert isinstance(fake.argv, list) + assert fake.kwargs.get("shell") in (None, False) + assert fake.kwargs.get("timeout") == rs.MOUNT_TIMEOUT_S + + assert fake.argv[1] == "-n", "sudo must never prompt" + assert fake.argv[3:5] == ["-t", "cifs"] + assert fake.argv[5] == "//host.example.org/share/sub" + assert fake.argv[6] == str(tmp_path / "mnt") + assert fake.argv[7] == "-o" + + # The hardening options come LAST, because mount options are last-one-wins. + options = fake.argv[8] + assert options.startswith("credentials=") + assert options.endswith(rs.fixed_mount_options()) + for hardening in ("nosuid", "nodev", "noexec", "uid=%d" % os.getuid()): + assert hardening in options + + +@pytest.mark.asyncio +async def test_mount_refuses_without_a_password(tmp_path: Path, monkeypatch): + fake = FakeRun(returncode=0) + monkeypatch.setattr(rs.subprocess, "run", fake) + + service = make_service(tmp_path, simulation=False) + service.clear_password() + ok, message = await service.mount() + assert ok is False + assert "re-entered after a restart" in message + assert fake.argv == [], "mount must not be invoked at all without credentials" + + +def test_no_shell_true_anywhere_in_the_backend(): + """A blunt guard: the server string flows into a *sudo* command, and with + `shell=True` that would be local privilege escalation to root. + + Parsed rather than grepped, so the prose in the module docstrings that says + "never shell=True" doesn't trip it -- only a real keyword argument does. + """ + package_root = Path(rs.__file__).parent + offenders = [] + for path in sorted(package_root.rglob("*.py")): + tree = ast.parse(path.read_text(), filename=str(path)) + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + for keyword in node.keywords: + if keyword.arg == "shell" and not ( + isinstance(keyword.value, ast.Constant) and keyword.value.value is False + ): + offenders.append("%s:%d" % (path.name, node.lineno)) + assert offenders == [], f"shell= passed to a subprocess call at {offenders}" + + +# --------------------------------------------------------------------------- +# 4. A broken share must never break an experiment +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_sync_failure_does_not_break_a_running_experiment( + client: AsyncClient, app_config: AppConfig, monkeypatch +): + """Data safety of the local run is paramount; the remote copy is best-effort.""" + + def exploding_copy(src: Path, dest_dir: Path) -> None: + raise OSError("Host is down") + + monkeypatch.setattr(rs.RemoteSyncService, "_copy_sync", staticmethod(exploding_copy)) + + res = await client.put( + "/api/settings/remote-sync", + json={ + "server": "//host.example.org/share", + "username": "LHR", + "password": SECRET, + "researcher": "alice", + "enabled": True, + }, + ) + assert res.status_code == 200 + + config = TropismConfig( + experimentName="sync-failure", + username="alice", + darkPhaseEnabled=True, + darkPhaseHours=0.05, + lateralIlluminationHours=0, + intervalMinutes=1, + ) + res = await client.post("/api/experiments", json=config.model_dump()) + assert res.status_code == 200 + assert res.json()["status"] == "started" + experiment_id = res.json()["experimentId"] + + # Wait for the first capture (taken immediately at phase start). + for _ in range(200): + status = (await client.get("/api/experiments/current")).json() + if status["imagesCaptured"] >= 1: + break + await asyncio.sleep(0.05) + + status = (await client.get("/api/experiments/current")).json() + assert status["imagesCaptured"] >= 1 + assert status["state"] == "running", "a dead share must not stop the run" + + res = await client.post("/api/experiments/current/stop") + final = res.json() + assert final["state"] == "done" + assert final["message"] == "stopped by user" + + # The image is safely on local disk regardless of the remote failure. + local = app_config.storage_root / experiment_id + assert list(local.glob("*.jpg")), "local images must survive a sync failure" + + # Let the background worker finish, then confirm the failure was recorded + # as pending rather than raised. + sync = client._app.state.app.sync # type: ignore[attr-defined] + await asyncio.wait_for(sync._queue.join(), timeout=10) + remote = (await client.get("/api/settings/remote-sync")).json() + assert remote["pendingCount"] >= 1 + assert remote["lastResult"] == "error" + assert "Host is down" in (remote["lastError"] or "") + + +@pytest.mark.asyncio +async def test_enqueue_image_never_raises_on_the_capture_path(tmp_path: Path): + """The one method the runner calls: synchronous, non-blocking, total.""" + service = make_service(tmp_path) + # Nonsense inputs, a full queue, a missing file -- none may propagate. + service.enqueue_image(tmp_path / "does-not-exist.jpg", "exp", "alice") + service.enqueue_image(Path("/definitely/not/here.jpg"), "", "") + service._queue = None # type: ignore[assignment] - simulate a broken queue + service.enqueue_image(tmp_path / "x.jpg", "exp", "alice") # must not raise + + +@pytest.mark.asyncio +async def test_disabled_sync_queues_nothing(tmp_path: Path): + service = make_service(tmp_path, settings=RemoteSyncSettings(enabled=False, researcher="alice")) + service.enqueue_image(tmp_path / "a.jpg", "exp", "alice") + assert service._queue.qsize() == 0 + + +# --------------------------------------------------------------------------- +# 5. Researcher changes, bulk sync, and the simulated share +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_sync_stops_when_the_researcher_changes(tmp_path: Path): + service = make_service(tmp_path) + assert service.settings.enabled is True + + service.note_active_researcher("alice") # same person: no change + assert service.settings.enabled is True + + service.note_active_researcher("bob") + assert service.settings.enabled is False + assert "researcher changed" in (service.status().lastError or "") + assert "alice" in service.status().lastError and "bob" in service.status().lastError + await asyncio.sleep(0) # let the best-effort unmount task run + + +@pytest.mark.asyncio +async def test_bulk_sync_copies_only_this_researchers_experiments(client: AsyncClient, app_config: AppConfig): + root = app_config.storage_root + root.mkdir(parents=True, exist_ok=True) + for name, owner in [ + ("2026-01-01_alice_run-a", "alice"), + ("2026-01-02_alice_run-b", "alice"), + ("2026-01-03_bob_run-c", "bob"), + ]: + exp = root / name + (exp / "thumbs").mkdir(parents=True) + (exp / "dark_00000.jpg").write_bytes(b"jpeg-bytes") + (exp / "thumbs" / "dark_00000.jpg").write_bytes(b"thumb") + (exp / "metadata.json").write_text(json.dumps({"username": owner})) + + await client.put( + "/api/settings/remote-sync", + json={ + "server": "//host.example.org/share", + "username": "LHR", + "password": SECRET, + "researcher": "alice", + "enabled": True, + }, + ) + res = await client.post("/api/settings/remote-sync/sync-all", json={"researcher": "alice"}) + assert res.status_code == 200 + + sync = client._app.state.app.sync # type: ignore[attr-defined] + await asyncio.wait_for(sync._queue.join(), timeout=20) + + # In simulation the "share" is a local directory, so the layout is checkable: + # ///... + destination = sync.remote_path_for("alice") + assert (destination / "2026-01-01_alice_run-a" / "dark_00000.jpg").exists() + assert (destination / "2026-01-02_alice_run-b" / "metadata.json").exists() + assert not (destination / "2026-01-03_bob_run-c").exists(), "another user's data must not be copied" + # Locally-regenerable thumbnails are not shipped over the network. + assert not (destination / "2026-01-01_alice_run-a" / "thumbs").exists() + + status = (await client.get("/api/settings/remote-sync")).json() + assert status["lastResult"] == "ok" + assert "Copied" in (status["bulkMessage"] or "") + + +@pytest.mark.asyncio +async def test_simulation_mode_degrades_gracefully_without_a_cifs_server(client: AsyncClient): + """The whole stack must stay usable on a dev laptop with no share.""" + status = (await client.get("/api/settings/remote-sync")).json() + assert status["simulation"] is True + assert status["server"] == "//ds.asuch.cas.cz/ueb/lhr", "the default is pre-filled" + assert status["enabled"] is False + + await client.put( + "/api/settings/remote-sync", + json={"username": "LHR", "password": SECRET, "researcher": "alice", "enabled": True}, + ) + res = await client.post("/api/settings/remote-sync/check") + assert res.status_code == 200 + body = res.json() + assert body["ok"] is True + assert "simulation" in body["message"].lower() + assert body["status"]["mounted"] is True + + +@pytest.mark.asyncio +async def test_check_connection_requires_both_credentials(client: AsyncClient): + res = await client.post("/api/settings/remote-sync/check") + assert res.status_code == 400 + + await client.put("/api/settings/remote-sync", json={"username": "LHR"}) + res = await client.post("/api/settings/remote-sync/check") + assert res.status_code == 400, "username alone is not enough" + + +@pytest.mark.asyncio +async def test_enabling_sync_requires_credentials_and_a_researcher(client: AsyncClient): + res = await client.put("/api/settings/remote-sync", json={"enabled": True}) + assert res.status_code == 400 + + res = await client.put( + "/api/settings/remote-sync", + json={"username": "LHR", "password": SECRET, "enabled": True}, + ) + assert res.status_code == 400, "a destination researcher folder is required" + + +@pytest.mark.asyncio +async def test_turning_sync_off_drops_the_session_password(client: AsyncClient): + await client.put( + "/api/settings/remote-sync", + json={"username": "LHR", "password": SECRET, "researcher": "alice", "enabled": True}, + ) + assert (await client.get("/api/settings/remote-sync")).json()["passwordSet"] is True + + res = await client.put("/api/settings/remote-sync", json={"enabled": False}) + assert res.status_code == 200 + body = res.json() + assert body["enabled"] is False + assert body["passwordSet"] is False + assert body["mounted"] is False diff --git a/deploy/install.sh b/deploy/install.sh index 22d350d..6d314b1 100755 --- a/deploy/install.sh +++ b/deploy/install.sh @@ -32,7 +32,8 @@ sudo apt-get update sudo apt-get install -y \ python3-venv python3-picamera2 python3-lgpio \ chromium nodejs npm curl \ - swayidle wlopm + swayidle wlopm \ + cifs-utils echo "==> Enabling SPI + camera (and pinning core clock on Pi 4)..." CONFIG=/boot/firmware/config.txt @@ -77,6 +78,53 @@ RAPIDBOXES_SETTINGS_PATH=$HOME_DIR/rapidboxes/settings.json RAPIDBOXES_UPDATE_BRANCH=$UPDATE_BRANCH EOF +echo "==> Installing sudoers rule for remote CIFS sync..." +# Remote sync (Settings -> General) mounts an institutional SMB share, which +# needs root. The grant below is deliberately as narrow as it can be made: +# +# * exactly two commands, mount -t cifs and umount -- never a blanket ALL +# * a FIXED mount point, so no other path can be mounted or unmounted +# * a FIXED trailing option string; the only wildcards are the //host/share +# (validated against a strict allowlist server-side before it can get here) +# and the random credentials filename inside the service's own private +# /run directory +# * the hardening options come LAST in the option string. mount options are +# last-one-wins, so even if something unexpected slipped in earlier, +# nosuid/nodev/noexec and the unprivileged uid/gid still take effect. +# +# Commas inside a sudoers command argument must be escaped (an unescaped comma +# would end the command and start a new list entry). +SUDOERS_FILE=/etc/sudoers.d/rapidboxes +MOUNT_BIN="$( [ -x /usr/bin/mount ] && echo /usr/bin/mount || command -v mount )" +UMOUNT_BIN="$( [ -x /usr/bin/umount ] && echo /usr/bin/umount || command -v umount )" +RUN_UID="$(id -u "$RUN_USER")" +RUN_GID="$(id -g "$RUN_USER")" +MOUNT_OPTS="nosuid\,nodev\,noexec\,uid=$RUN_UID\,gid=$RUN_GID\,file_mode=0664\,dir_mode=0775" + +sudo install -d -m 0755 /mnt/rapidboxes-remote + +SUDOERS_TMP="$(mktemp)" +cat > "$SUDOERS_TMP" </dev/null; then + sudo install -m 0440 -o root -g root "$SUDOERS_TMP" "$SUDOERS_FILE" + echo " installed $SUDOERS_FILE (validated with visudo -c)" +else + echo " !! generated sudoers file failed validation -- NOT installed." >&2 + echo " !! Remote CIFS sync will be unavailable; everything else still works." >&2 + sudo visudo -c -f "$SUDOERS_TMP" >&2 || true +fi +rm -f "$SUDOERS_TMP" + echo "==> Installing systemd service..." sed -e "s|@USER@|$RUN_USER|g" \ -e "s|@BACK_DIR@|$BACK_DIR|g" \ diff --git a/deploy/rapidboxes.service b/deploy/rapidboxes.service index 90219da..d4ad8a7 100644 --- a/deploy/rapidboxes.service +++ b/deploy/rapidboxes.service @@ -8,6 +8,12 @@ User=@USER@ WorkingDirectory=@BACK_DIR@ EnvironmentFile=/etc/rapidboxes.env ExecStart=@VENV@/bin/python -m rapidboxes +# Private 0700 directory under /run for the transient CIFS credentials file +# (remote sync). systemd creates it on start and removes it on stop; /run is +# tmpfs, so nothing can survive a reboot. The path is pinned by the +# /etc/sudoers.d/rapidboxes rule -- keep the two in step. +RuntimeDirectory=rapidboxes-cifs +RuntimeDirectoryMode=0700 Restart=always RestartSec=1 # Hard-kill promptly: graceful stop can hang on open WebSockets / MJPEG. diff --git a/deploy/uninstall.sh b/deploy/uninstall.sh index b6e5928..61f2de3 100755 --- a/deploy/uninstall.sh +++ b/deploy/uninstall.sh @@ -10,8 +10,14 @@ sudo systemctl disable --now rapidboxes-update.timer 2>/dev/null || true sudo rm -f /etc/systemd/system/rapidboxes.service /etc/rapidboxes.env sudo rm -f /etc/systemd/system/rapidboxes-update.service /etc/systemd/system/rapidboxes-update.timer sudo systemctl daemon-reload + +# Remote CIFS sync: drop the mount, the narrow sudo grant, and the mount point. +sudo umount /mnt/rapidboxes-remote 2>/dev/null || true +sudo rm -f /etc/sudoers.d/rapidboxes +sudo rmdir /mnt/rapidboxes-remote 2>/dev/null || true +sudo rm -rf /run/rapidboxes-cifs rm -f "$HOME_DIR/.config/autostart/rapidboxes-kiosk.desktop" rm -f "$HOME_DIR/.config/autostart/rapidboxes-idle.desktop" -echo "Removed service + kiosk/idle-screen autostart." +echo "Removed service + kiosk/idle-screen autostart, and the remote-sync sudoers rule." echo "Experiment data under ~/rapidboxes was kept; delete it manually if desired." diff --git a/front/plant-imaging-controller-faa-main/client/components/GeneralSettingsMenu.tsx b/front/plant-imaging-controller-faa-main/client/components/GeneralSettingsMenu.tsx index 04fd385..f49e1fa 100644 --- a/front/plant-imaging-controller-faa-main/client/components/GeneralSettingsMenu.tsx +++ b/front/plant-imaging-controller-faa-main/client/components/GeneralSettingsMenu.tsx @@ -1,6 +1,7 @@ import { Copy } from "lucide-react"; import { toast } from "sonner"; import { useSystemInfo } from "@/hooks/useSystemInfo"; +import RemoteSyncPanel from "@/components/RemoteSyncPanel"; import UpdatePanel from "@/components/UpdatePanel"; const LOW_DISK_BYTES = 2 * 1024 * 1024 * 1024; @@ -138,6 +139,8 @@ export default function GeneralSettingsMenu() { )} + + diff --git a/front/plant-imaging-controller-faa-main/client/components/OnScreenKeyboard.tsx b/front/plant-imaging-controller-faa-main/client/components/OnScreenKeyboard.tsx index 413a899..22fde5a 100644 --- a/front/plant-imaging-controller-faa-main/client/components/OnScreenKeyboard.tsx +++ b/front/plant-imaging-controller-faa-main/client/components/OnScreenKeyboard.tsx @@ -17,11 +17,16 @@ export default function OnScreenKeyboard({ initialValue, onConfirm, onCancel, + masked = false, }: { title: string; initialValue: string; onConfirm: (value: string) => void; onCancel: () => void; + /** Render the entry field as a password box (remote-sync credentials). + * The kiosk screen is in a shared lab, so a password must not sit on it + * in plain text while it is being typed. */ + masked?: boolean; }) { const [value, setValue] = useState(initialValue); const [shift, setShift] = useState(false); @@ -41,6 +46,7 @@ export default function OnScreenKeyboard({ setValue(e.target.value)} className="mb-3 w-full rounded-lg border border-app-border-primary bg-app-bg-primary px-3 py-2 text-lg text-white outline-none focus:border-app-green" diff --git a/front/plant-imaging-controller-faa-main/client/components/RemoteSyncPanel.tsx b/front/plant-imaging-controller-faa-main/client/components/RemoteSyncPanel.tsx new file mode 100644 index 0000000..6b13ab9 --- /dev/null +++ b/front/plant-imaging-controller-faa-main/client/components/RemoteSyncPanel.tsx @@ -0,0 +1,402 @@ +import { useCallback, useEffect, useRef, useState } from "react"; +import { AlertTriangle, FolderSync, KeyRound, Loader2, Plug, RefreshCw } from "lucide-react"; +import { toast } from "sonner"; +import OnScreenKeyboard from "@/components/OnScreenKeyboard"; +import { api } from "@/lib/api"; +import { getUsername } from "@/lib/session"; +import type { RemoteSyncStatus } from "@shared/api"; + +/** + * Remote CIFS/SMB sync card for Settings -> General. + * + * Two things here are deliberate and easy to break by accident: + * + * 1. **The password's length is never revealed.** The backend does not return + * the password at all (there is no field for it on RemoteSyncStatus), so + * when one is already set the field renders EMPTY with a fixed-width + * placeholder — never a string sized to the real password. + * + * 2. **"Credentials needed after restart" is a loud state, not a footnote.** + * The password is session-only by design, so any restart (a reboot, a + * power blip, or the monthly OTA update) leaves sync switched on but + * unable to copy anything. That must never look like a working sync, so + * it takes over the card with an orange banner. + * + * The same tradeoff is stated twice on purpose: as static helper text next to + * the fields (so the expectation is set *while* someone types the password and + * before they walk away trusting a long unattended run), and as a confirmation + * toast the moment credentials are accepted. + */ + +// Fixed width, always 8 — NOT the real password's length. +const MASKED_PLACEHOLDER = "••••••••"; + +const CREDENTIALS_NOTICE = + "For better security, credentials are not stored on disk and must be entered again after every system restart."; + +type Field = "server" | "username" | "password"; + +function formatTime(iso: string): string { + try { + return new Date(iso).toLocaleString(); + } catch { + return iso; + } +} + +export default function RemoteSyncPanel() { + const [status, setStatus] = useState(null); + const [server, setServer] = useState(""); + const [username, setUsername] = useState(""); + // Only ever holds a password the operator is entering right now. It is + // cleared as soon as it has been sent, and is never populated from the API. + const [password, setPassword] = useState(""); + const [editing, setEditing] = useState(null); + const [checking, setChecking] = useState(false); + const [saving, setSaving] = useState(false); + const [syncingAll, setSyncingAll] = useState(false); + const dirtyRef = useRef(false); + + const researcher = getUsername(); + + const refresh = useCallback(async () => { + try { + const next = await api.remoteSync(); + setStatus(next); + // Don't clobber half-typed edits with polled values. + if (!dirtyRef.current) { + setServer(next.server); + setUsername(next.username); + } + } catch { + /* best-effort: keep whatever we last had */ + } + }, []); + + useEffect(() => { + refresh(); + const id = window.setInterval(refresh, 5000); + return () => window.clearInterval(id); + }, [refresh]); + + const hasPassword = password.length > 0 || (status?.passwordSet ?? false); + const canCheck = username.trim().length > 0 && hasPassword; + + /** PUT the current field values. Fires the credentials notice when a + * password was actually part of what got accepted. */ + const saveCredentials = async (extra: { enabled?: boolean } = {}) => { + const sentPassword = password.length > 0; + const next = await api.saveRemoteSync({ + server: server.trim() || undefined, + username: username.trim() || undefined, + researcher, + ...(sentPassword ? { password } : {}), + ...extra, + }); + setStatus(next); + setPassword(""); + dirtyRef.current = false; + if (sentPassword) { + // Informational, not a warning: this is a deliberate design property. + // Longer than the default because it is a sentence worth reading (same + // precedent as the important toast in UpdatePanel.tsx). + toast.success(CREDENTIALS_NOTICE, { duration: 15000 }); + } + return next; + }; + + const handleToggle = async () => { + if (!status || saving) return; + setSaving(true); + try { + if (status.enabled) { + const next = await api.saveRemoteSync({ enabled: false }); + setStatus(next); + setPassword(""); + toast.success("Remote sync switched off."); + } else { + await saveCredentials({ enabled: true }); + toast.success(`Remote sync on — copying to ${server.trim()}/${researcher}`); + } + } catch (e) { + toast.error((e as Error).message); + await refresh(); + } finally { + setSaving(false); + } + }; + + const handleCheck = async () => { + if (!canCheck || checking) return; + setChecking(true); + try { + await saveCredentials(); + const result = await api.checkRemoteSync(); + setStatus(result.status); + if (result.ok) { + toast.success(result.message); + } else { + // Report the real error — "wrong password" and "host unreachable" + // need very different fixes. + toast.error(result.message, { duration: 12000 }); + } + } catch (e) { + toast.error((e as Error).message, { duration: 12000 }); + } finally { + setChecking(false); + } + }; + + const handleSyncAll = async () => { + if (syncingAll) return; + setSyncingAll(true); + try { + const next = await api.syncAllRemote(researcher); + setStatus(next); + toast.success(`Copying all of ${researcher}'s experiments in the background…`); + } catch (e) { + toast.error((e as Error).message, { duration: 12000 }); + } finally { + setSyncingAll(false); + } + }; + + const openEditor = (field: Field) => setEditing(field); + + const editorTitle = + editing === "server" + ? "Server / share path" + : editing === "username" + ? "Share username" + : "Share password"; + + const editorValue = editing === "server" ? server : editing === "username" ? username : ""; + + const applyEditor = (value: string) => { + dirtyRef.current = true; + if (editing === "server") setServer(value); + else if (editing === "username") setUsername(value); + else if (editing === "password") setPassword(value); + setEditing(null); + }; + + const credentialsRequired = status?.credentialsRequired ?? false; + + return ( + <> +
+
+
+ Remote Sync +
+ +
+ + {/* The loud post-restart state: on, but unable to do anything. */} + {credentialsRequired && ( +
+ +
+

+ Inactive — credentials needed after restart +

+

+ Sync is switched on but nothing is being copied. The password is never saved + to disk, so it was lost when the box restarted. Re-enter it below and press + Check Connection to resume. +

+
+
+ )} + +
+
Status
+
+ {status == null + ? "—" + : credentialsRequired + ? "Credentials needed" + : status.mounted + ? "Mounted" + : status.enabled + ? "On — not mounted yet" + : "Off"} +
+ +
Destination
+
+ {status?.remotePath ?? `${server || "—"}/${researcher}`} +
+ +
Last sync
+
+ {status?.lastSyncAt ? formatTime(status.lastSyncAt) : "Never"} +
+ + {status != null && status.pendingCount > 0 && ( + <> +
Pending
+
+ {status.pendingCount} file{status.pendingCount === 1 ? "" : "s"} not yet copied +
+ + )} +
+ + {status?.lastResult === "error" && status.lastError && !credentialsRequired && ( +

+ {status.lastError} +

+ )} + + {status?.simulation && ( +

+ Simulation mode: no real network share is mounted; copies go to a local folder. +

+ )} + + {/* --- credentials ------------------------------------------------ */} +
+ + { + dirtyRef.current = true; + setServer(e.target.value); + }} + onClick={() => openEditor("server")} + placeholder="//server/share/folder" + className="w-full rounded-md border border-app-border-primary bg-app-bg-primary px-2.5 py-1.5 font-mono text-[11px] text-white outline-none focus:border-app-green" + /> + + + { + dirtyRef.current = true; + setUsername(e.target.value); + }} + onClick={() => openEditor("username")} + placeholder="share account" + className="w-full rounded-md border border-app-border-primary bg-app-bg-primary px-2.5 py-1.5 text-[11px] text-white outline-none focus:border-app-green" + /> + + + { + dirtyRef.current = true; + setPassword(e.target.value); + }} + onClick={() => openEditor("password")} + // Fixed-width placeholder. This is NOT the stored password's + // length — the backend never tells us that, and it must not. + placeholder={status?.passwordSet ? MASKED_PLACEHOLDER : "share password"} + className="w-full rounded-md border border-app-border-primary bg-app-bg-primary px-2.5 py-1.5 text-[11px] text-white outline-none focus:border-app-green" + /> + + {/* Always visible, whatever the current state: sets the expectation + while the password is being typed, not after sync has broken. */} +

{CREDENTIALS_NOTICE}

+
+ +
+ + + +
+ + {status?.bulkMessage && ( +

{status.bulkMessage}

+ )} +
+ + {editing && ( + setEditing(null)} + onConfirm={applyEditor} + /> + )} + + ); +} diff --git a/front/plant-imaging-controller-faa-main/client/lib/api.ts b/front/plant-imaging-controller-faa-main/client/lib/api.ts index 59c14c5..b4b117c 100644 --- a/front/plant-imaging-controller-faa-main/client/lib/api.ts +++ b/front/plant-imaging-controller-faa-main/client/lib/api.ts @@ -6,6 +6,9 @@ import type { ExperimentStatus, HistoryEntry, ImageListResponse, + RemoteSyncCheckResult, + RemoteSyncStatus, + RemoteSyncUpdate, SavedExperimentConfig, StartResponse, SystemInfo, @@ -54,6 +57,20 @@ export const api = { settings: () => jsonFetch("/api/settings"), saveSettings: (s: DeviceSettings) => jsonFetch("/api/settings", { method: "PUT", body: JSON.stringify(s) }), + remoteSync: () => jsonFetch("/api/settings/remote-sync"), + /** Patch the remote-sync config. `password` is write-only and never comes back. */ + saveRemoteSync: (update: RemoteSyncUpdate) => + jsonFetch("/api/settings/remote-sync", { + method: "PUT", + body: JSON.stringify(update), + }), + checkRemoteSync: () => + jsonFetch("/api/settings/remote-sync/check", { method: "POST" }), + syncAllRemote: (researcher: string) => + jsonFetch("/api/settings/remote-sync/sync-all", { + method: "POST", + body: JSON.stringify({ researcher }), + }), health: () => jsonFetch<{ ok: boolean; version: string }>("/api/health"), system: () => jsonFetch("/api/system"), recheckCamera: () => jsonFetch("/api/system/recheck-camera", { method: "POST" }), diff --git a/front/plant-imaging-controller-faa-main/shared/api.ts b/front/plant-imaging-controller-faa-main/shared/api.ts index 891698d..a9fd3f3 100644 --- a/front/plant-imaging-controller-faa-main/shared/api.ts +++ b/front/plant-imaging-controller-faa-main/shared/api.ts @@ -217,6 +217,64 @@ export interface DeviceSettings { photoIlluminationSource: PhotoIlluminationSource; } +// --------------------------------------------------------------------------- +// Remote CIFS/SMB sync (Settings -> General -> Remote Sync). +// Mirrors RemoteSyncStatus / RemoteSyncUpdate in back/rapidboxes/models.py. +// +// SECURITY: there is deliberately no `password` field on RemoteSyncStatus. The +// backend never returns it, and the UI must never try to display or infer it — +// including its length. `passwordSet` is all there is. +// --------------------------------------------------------------------------- + +/** Pre-filled default, from the institutional share the legacy script mounted. */ +export const DEFAULT_REMOTE_SERVER = "//ds.asuch.cas.cz/ueb/lhr"; + +export interface RemoteSyncStatus { + enabled: boolean; + /** //host/share[/path] — strictly validated server-side before it can reach mount. */ + server: string; + /** The CIFS/SMB account used to mount the share. */ + username: string; + /** Whether a password is held in the backend's memory. Never the password itself. */ + passwordSet: boolean; + mounted: boolean; + /** + * Switched on, but the in-memory password is gone — i.e. the backend has + * restarted (reboot, power blip, or the monthly OTA update). Sync can do + * nothing at all in this state, so the UI must say so loudly rather than + * showing an "on" toggle that silently copies nothing. + */ + credentialsRequired: boolean; + /** Destination subfolder on the share; sync stops if the researcher changes. */ + researcher: string; + remotePath: string | null; + /** Images captured but not yet copied (queued + failed-and-awaiting-retry). */ + pendingCount: number; + lastSyncAt: string | null; // ISO 8601 + lastResult: "ok" | "error" | null; + lastError: string | null; + bulkInProgress: boolean; + bulkMessage: string | null; + /** True on a dev laptop: no real CIFS mount is attempted. */ + simulation: boolean; +} + +/** PUT body. Every field optional so the UI can patch one thing at a time. + * `password` is write-only — it is never returned by any endpoint. */ +export interface RemoteSyncUpdate { + enabled?: boolean; + server?: string; + username?: string; + password?: string; + researcher?: string; +} + +export interface RemoteSyncCheckResult { + ok: boolean; + message: string; + status: RemoteSyncStatus; +} + /** The saved/loaded per-experiment .xml: phases + light + illumination + camera, no identity fields. */ export interface SavedExperimentConfig { protocol: "tropism" | "growth";