diff --git a/containers/libreportal/frontend/core/setup/css/setup-wizard.css b/containers/libreportal/frontend/core/setup/css/setup-wizard.css index 93f8d4d..9f9e5b9 100755 --- a/containers/libreportal/frontend/core/setup/css/setup-wizard.css +++ b/containers/libreportal/frontend/core/setup/css/setup-wizard.css @@ -1248,3 +1248,82 @@ body.setup-wizard-open { .setup-storage-disabled input { cursor: not-allowed; } + +/* Card: a Details button sits at the right of the row, outside the label's + click target so pressing it opens the modal instead of toggling the tick. */ +.setup-storage-card { position: relative; padding-right: 96px; } +.setup-storage-details { + position: absolute; + right: 12px; + top: 50%; + transform: translateY(-50%); + padding: 5px 12px; + font-size: 0.82em; + border-radius: 8px; + border: 1px solid rgba(255, 255, 255, 0.28); + background: rgba(255, 255, 255, 0.08); + color: inherit; + cursor: pointer; +} +.setup-storage-details:hover { background: rgba(255, 255, 255, 0.16); } +.setup-storage-flags { opacity: 0.9; } + +/* Modal */ +.setup-modal-backdrop { + position: fixed; inset: 0; z-index: 10000; + background: rgba(4, 20, 38, 0.62); + backdrop-filter: blur(3px); + display: flex; align-items: center; justify-content: center; + padding: 24px; +} +.setup-modal { + width: min(680px, 100%); + max-height: 82vh; + overflow: auto; + border-radius: 14px; + border: 1px solid rgba(255, 255, 255, 0.18); + background: rgba(12, 48, 84, 0.97); + box-shadow: 0 24px 64px rgba(0, 0, 0, 0.45); +} +.setup-modal-head { + display: flex; align-items: center; justify-content: space-between; + gap: 12px; padding: 16px 20px; + border-bottom: 1px solid rgba(255, 255, 255, 0.14); +} +.setup-modal-title { font-weight: 700; font-size: 1.05em; word-break: break-all; } +.setup-modal-close { + background: none; border: none; color: inherit; + font-size: 1.6em; line-height: 1; cursor: pointer; opacity: 0.75; +} +.setup-modal-close:hover { opacity: 1; } +.setup-modal-body { padding: 18px 20px 22px; } + +.setup-storage-spec { width: 100%; border-collapse: collapse; margin-bottom: 18px; } +.setup-storage-spec th { + text-align: left; font-weight: 500; opacity: 0.72; + padding: 5px 12px 5px 0; white-space: nowrap; vertical-align: top; +} +.setup-storage-spec td { padding: 5px 0; word-break: break-all; } +.setup-storage-spec code { font-size: 0.9em; } + +.setup-storage-checks { list-style: none; margin: 0 0 4px; padding: 0; } +.setup-storage-check { + display: flex; gap: 10px; align-items: flex-start; + padding: 9px 0; border-top: 1px solid rgba(255, 255, 255, 0.1); +} +.setup-storage-check-icon { flex: 0 0 auto; } +.setup-storage-check-detail { opacity: 0.8; font-size: 0.92em; } + +.setup-storage-fstab { + margin-top: 18px; padding: 14px 16px; border-radius: 10px; + border: 1px solid rgba(224, 168, 0, 0.32); + background: rgba(224, 168, 0, 0.09); +} +.setup-storage-fstab-title { font-weight: 700; margin-bottom: 6px; } +.setup-storage-fstab p { margin: 0 0 10px; font-size: 0.93em; opacity: 0.9; } +.setup-storage-fstab pre { + margin: 0 0 10px; padding: 9px 11px; overflow-x: auto; + border-radius: 7px; background: rgba(0, 0, 0, 0.3); font-size: 0.85em; +} +.setup-storage-fstab-opt { display: flex; gap: 9px; align-items: center; cursor: pointer; } +.setup-storage-fstab-note { margin-top: 10px !important; font-size: 0.85em !important; opacity: 0.75 !important; } diff --git a/containers/libreportal/frontend/core/setup/js/setup-wizard.js b/containers/libreportal/frontend/core/setup/js/setup-wizard.js index 8180a41..d408710 100755 --- a/containers/libreportal/frontend/core/setup/js/setup-wizard.js +++ b/containers/libreportal/frontend/core/setup/js/setup-wizard.js @@ -29,6 +29,8 @@ class SetupWizard { this.hasStorageCandidates = false; this.storageCandidates = []; this.selectedStorage = []; + // Paths the user asked us to make permanent in /etc/fstab. + this.fstabWanted = []; this.installLevel = 'beginner'; this.totalSteps = this._effectiveTotalSteps(); this.domainCount = 0; // tracked dynamically as the user adds rows @@ -407,6 +409,44 @@ class SetupWizard { this.showStep(this.currentStep); } + // Short, human summary for a check. The full sentence lives in the details + // modal; a card that dumps every message becomes a wall of prose nobody + // reads, which is exactly what this replaces. + // + // Severity-aware, and it has to be: keying on the check id alone printed the + // FAILURE wording next to a green tick — "This drive's format can't store + // file ownership" above "Filesystem: ext4" — which says the opposite of what + // the check found. + _storageCheckSummary(c) { + if (c.severity === 'info') { + const ok = { + 'fstype': 'Filesystem is supported', + 'ownership': 'Ownership and permissions work', + 'persistence': 'Mounts automatically at boot', + 'space': 'Plenty of space', + 'encryption': 'Encrypted at rest', + 'mount-nosuid': 'Mounted nosuid', + }; + return ok[c.id] || c.message; + } + const map = { + 'fstype': "This drive's format can't store file ownership", + 'mount-ro': 'Mounted read-only', + 'mount-noexec': 'Mounted noexec', + 'ownership': "Can't set file ownership here", + 'subuid': "Can't store the IDs containers need", + 'write': 'Write test failed', + 'readback': 'Read-back test failed', + 'writable': "Can't create files here", + 'space': c.severity === 'refuse' ? 'Not enough space' : 'Low on space', + 'persistence': "Won't be mounted after a reboot", + 'removable': 'Removable drive — apps pause when unplugged', + 'same-device': 'Same disk as the system', + 'shared-fate': 'Shares a disk with your backups', + }; + return map[c.id] || c.message; + } + renderStorage() { const list = this.container.querySelector('#sw-storage-list'); const note = this.container.querySelector('#sw-storage-note'); @@ -422,14 +462,18 @@ class SetupWizard { const refused = c.verdict === 'refuse'; const warned = c.verdict === 'warn'; const id = `sw-storage-${i}`; - // Checks are joined with "; " on the shell side. Split them back out: - // a drive can trip several at once, and one run-on paragraph buries the - // fstab line the user is meant to copy. - const detailParts = String((refused ? c.refusals : c.warnings) || '') - .split(/;\s+/).map(s => s.trim()).filter(Boolean); const badge = refused - ? 'unusable' + ? 'can\u2019t be used' : (warned ? 'needs care' : ''); + + // One line of plain facts, then at most two short flags. Everything else + // is a click away rather than in the user's face. + const flags = (c.checks || []) + .filter(k => k.severity === 'refuse' || k.severity === 'warn') + .map(k => this._storageCheckSummary(k)); + const shown = flags.slice(0, 2); + const more = flags.length - shown.length; + return ` `; }).join(''); + list.querySelectorAll('[data-storage-details]').forEach((btn) => { + btn.addEventListener('click', (e) => { + e.preventDefault(); + e.stopPropagation(); + this.showStorageDetails(Number(btn.dataset.storageDetails)); + }); + }); + if (note) { - note.innerHTML = 'A drive you tick is registered as a storage location during install. ' - + 'You can add or remove locations later from the CLI (libreportal storage). ' - + 'Unusable drives are shown greyed with the reason rather than hidden.'; + note.innerHTML = 'A drive you tick becomes a storage location during install. ' + + 'Drives that can\u2019t hold app data are shown greyed with the reason.'; } } + // Details modal: the technical spec, every check with its full explanation, + // and — when the drive isn\u2019t in fstab — the offer to make it permanent. + showStorageDetails(idx) { + const c = this.storageCandidates[idx]; + if (!c) return; + const esc = (s) => this.escapeHtml(s); + + const specRows = [ + ['Mount point', c.path], + ['Device', c.device], + ['Filesystem', c.fstype], + ['Size', c.size], + ['Free', c.free], + ['UUID', c.uuid || '—'], + ['Mount options', c.options || '—'], + ['Removable', c.removable ? 'Yes' : 'No'], + ].map(([k, v]) => `${esc(k)}${esc(v)}`).join(''); + + const icon = { refuse: '\u26d4', warn: '\u26a0\ufe0f', info: '\u2705' }; + const checkRows = (c.checks || []).map(k => ` +
  • + ${icon[k.severity] || ''} + + ${esc(this._storageCheckSummary(k))}
    + ${esc(k.message)} +     +
    • `).join(''); + + const fstab = c.fstab_line ? ` +
    +
    Make this drive mount automatically
    +

    + The system doesn\u2019t mount this drive at boot, so apps stored on it won\u2019t start + until it is mounted again. LibrePortal can add it to /etc/fstab for you. +

    + 
    ${esc(c.fstab_line)}
    + +

    + Written with nofail, so if the drive is missing the machine still boots + normally. Your current /etc/fstab is backed up first. +

    +
    ` : ''; + + const modal = document.createElement('div'); + modal.className = 'setup-modal-backdrop'; + modal.innerHTML = ` + `; + + const close = () => modal.remove(); + modal.addEventListener('click', (e) => { if (e.target === modal) close(); }); + modal.querySelector('.setup-modal-close').addEventListener('click', close); + document.addEventListener('keydown', function onKey(e) { + if (e.key === 'Escape') { close(); document.removeEventListener('keydown', onKey); } + }); + + // Remember the fstab choice on the wizard, not in the DOM: the modal is + // destroyed on close and the choice has to survive to submit(). + const fstabBox = modal.querySelector('[data-storage-fstab]'); + if (fstabBox) { + fstabBox.addEventListener('change', () => { + const path = fstabBox.dataset.storageFstab; + this.fstabWanted = this.fstabWanted.filter(p => p !== path); + if (fstabBox.checked) { + this.fstabWanted.push(path); + // Adding it to fstab only makes sense if the drive is actually used. + const own = this.container.querySelector(`[data-storage-path="${CSS.escape(path)}"]`); + if (own && !own.disabled) own.checked = true; + } + }); + } + + this.container.appendChild(modal); + } + escapeHtml(s) { return String(s == null ? '' : s).replace(/[&<>"']/g, (ch) => ( { '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' }[ch] @@ -864,7 +1004,9 @@ class SetupWizard { // Paths ticked on the Storage step. Registering one is a privileged // action, so this is only a request — the host applier runs it through // the CLI and the root helper, which validate independently. - storage: this.collectStorage() + storage: this.collectStorage(), + // Only for drives that are actually being registered. + storage_fstab: this.fstabWanted.filter(p => this.collectStorage().includes(p)) }; // Apply the experience choice to the WebUI immediately so the next diff --git a/docs/roadmap/storage-locations.md b/docs/roadmap/storage-locations.md index 2a1980f..22afc21 100644 --- a/docs/roadmap/storage-locations.md +++ b/docs/roadmap/storage-locations.md @@ -1,6 +1,6 @@ # LibrePortal — Storage Locations (per-app data placement) -**Status:** Phases 0-2, 4 and 5 **built**; the two WebUI surfaces (setup-wizard step, Disks page) are not. · **Audience:** us, future-self · **Scope:** register more than one filesystem root for live app data, choose one per app, move an app between them, and resolve the right one on restore/migrate · **Origin:** "add different locations to set up LibrePortal on, with control per app" (2026-08-24) +**Status:** Phases 0-5 **built** (incl. the setup-wizard Storage step); the Disks WebUI page is not. · **Audience:** us, future-self · **Scope:** register more than one filesystem root for live app data, choose one per app, move an app between them, and resolve the right one on restore/migrate · **Origin:** "add different locations to set up LibrePortal on, with control per app" (2026-08-24) --- @@ -15,7 +15,8 @@ Nextcloud's 4 TB of photos go on the spinning disk. Vaultwarden and the control ## 1. Non-goals - ❌ Relocating the **system** root (configs/db/logs) per-app. It stays one place, chosen at install. Same for the WebUI's own container dir. -- ❌ A general volume manager. We don't format, partition, mount, or write `/etc/fstab`. The drive must already be mounted; we validate and use it. +- ❌ A general volume manager. We don't format, partition or mount anything — the drive must already be mounted; we validate and use it. +- ⚠️ **One exception, added deliberately:** on explicit request we append a single marked `/etc/fstab` entry so a registered drive comes back after a reboot (§6.3). Telling a non-expert "add this line to fstab yourself" is a wall, and the most likely outcome is a reboot where nothing starts. - ❌ Striping/tiering/RAID-alikes. One app's data lives on exactly one location. No splitting an app across two. - ❌ Per-*volume* placement inside an app (`./data` here, `./db` there). Location granularity is the app directory. Revisit only if a real need shows up. - ❌ Merging the backup-location and storage-location **registries**. They differ in trust, lifecycle and ownership, so they stay separate — but they may freely share a *drive*, which is a supported and expected setup (§6.2). @@ -209,6 +210,46 @@ Worth noting the compounding case explicitly, because it's the one people don't **Consequence for naming.** Sharing makes the collision worse, not better: the user now genuinely sees "bigdisk" in two places meaning two things. That's what the **Disks view** (§7.1) answers — one row per device, with *app data* and *backups* as roles on it rather than as competing top-level nouns. It's also where this section's shared-fate badge and per-device space accounting naturally live. +### 6.3 — Writing `/etc/fstab`, and why that is allowed here + +§1 originally ruled this out, and reversing that deserves an argument rather +than a shrug. + +The case for: the persistence warning (§6, check 6) is useless to the audience +this product is for. "Add `UUID=… /mnt/disk ext4 defaults,nofail 0 2` to +/etc/fstab" assumes SSH, root, an editor, and knowing what fstab is. Someone who +registers a drive, doesn't act on the warning, and reboots gets apps that refuse +to start — the exact failure we were trying to prevent, arrived at by a longer +road. LibrePortal already writes sysctl drop-ins, `modules-load.d`, systemd +units, sudoers and firewall rules; fstab is not a different category of file. + +The case against is real and specific: **a bad fstab entry can leave a machine +unbootable**, needing rescue media. That is worse than any other failure mode in +this product. + +What makes it defensible is `nofail`, plus `x-systemd.device-timeout=10s`. +Together they mean a missing device can never block boot — which is precisely +the failure the objection is about. Without that pair this would stay a non-goal. + +The rules, all enforced in the root helper (`libreportal-storage fstab-add`): + +| Rule | Why | +|---|---| +| `nofail,x-systemd.device-timeout=10s` always | a missing drive can't block boot or strand the box at a systemd timeout | +| `UUID=`, never `/dev/sdX` | device names reorder between boots; a stale one mounts the wrong disk or nothing | +| append inside a marked block, never rewrite | anything the user or another tool manages is untouched | +| refuse if the target or UUID is already described | we don't get to be the second opinion on someone else's mount | +| refuse the root filesystem outright | never our business | +| must currently be a real mount | we describe reality, we don't invent it | +| timestamped backup to `/etc/fstab.libreportal-*.bak` | recoverable | +| `findmnt --verify` before install; discard on failure | a file that doesn't parse never reaches `/etc` | +| opt-in only | nothing calls it unless a person ticked the box | + +Verified against a real filesystem: the entry is added and verifies, the +persistence warning then disappears on the next scan, and a duplicate, the root +filesystem, a non-mountpoint and a relative path are each refused with the +reason. + ## 7. Surfaces — first-run wizard, config panels, CLI **First-run wizard.** The setup wizard (`core/setup/js/setup-wizard.js`) currently runs `Experience → Identity → Domains → Recommended → Metrics`, and already has the pattern for a step that isn't always shown: Metrics is advanced-only, and `_effectiveTotalSteps()` makes the count dynamic. A **Storage** step slots in **before Recommended** — locations must exist before apps get placed on them — and follows the same conditional rule, with a better trigger: @@ -401,7 +442,7 @@ Each phase is independently shippable and independently verifiable. Phase 0 carr | **0** ✅ | `appDir` / `storageRoots` / `pathIsContainerData` / `webuiDir`; ~260 call sites swept. `scripts/dev/lp-storage-test`. | No | | **1** ✅ | Root-owned registry + `libreportal-storage` + fitness checks + `libreportal storage {list,add,remove,check,scan,apps,disks}`. | CLI only | | **2** ✅ | `CFG__STORAGE` in 37 templates + the `**READONLY**` marker + resolved path in the comment (§5.2) + per-location config panel emit. | Yes | -| **3** ⬜ | Setup-wizard **Storage** step (§7) + `--storage-dir=`. **Not built.** The headless path it would drive already works, so this is UI on top of a working feature, not a prerequisite. | Yes | +| **3** ✅ | Setup-wizard **Storage** step (§7), with a details modal and the opt-in `/etc/fstab` offer (§6.3). `--storage-dir=` for unattended installs is still open. | Yes | | **4** ✅ | `libreportal app move` — stop, snapshot, copy as root, verify, then delete the source. | Yes | | **5** ◐ | Manifest `storage` block and the staged restore + path rewrite are done (§9's latent bug is fixed). The "unknown location" **prompt** is not — an unresolvable location currently falls back to this host's default rather than asking. | Yes | | **6** ◐ | The Disks **data layer** is built and `libreportal storage disks` renders it (including not-attached rows). The WebUI page at `/admin/system/storage` and the periodic `storage check` cron are **not**. | Yes | diff --git a/scripts/cli/commands/storage/cli_storage_commands.sh b/scripts/cli/commands/storage/cli_storage_commands.sh index 68a4d97..15a6cd8 100644 --- a/scripts/cli/commands/storage/cli_storage_commands.sh +++ b/scripts/cli/commands/storage/cli_storage_commands.sh @@ -102,6 +102,23 @@ cliHandleStorageCommands() storageDisks ;; + fstab-add) + # Make a mount permanent. The root helper does the writing and all + # the validating; this is only the front door. + if [[ -z "$arg1" ]]; then + isNotice "Usage: storage fstab-add " + return 1 + fi + local line + if line=$(runStorage fstab-add "$arg1" 2>&1); then + isSuccessful "Added to /etc/fstab: $line" + isNotice "A backup of the previous file is in /etc/fstab.libreportal-*.bak" + else + isError "$line" + return 1 + fi + ;; + *) isNotice "Invalid storage action: $action" cliShowStorageHelp diff --git a/scripts/cli/commands/storage/cli_storage_header.sh b/scripts/cli/commands/storage/cli_storage_header.sh index ad7f720..f5ea7ee 100644 --- a/scripts/cli/commands/storage/cli_storage_header.sh +++ b/scripts/cli/commands/storage/cli_storage_header.sh @@ -25,6 +25,11 @@ cliShowStorageHelp() echo "storage apps [id]" echo " Which apps live on a location." echo "" + echo "storage fstab-add " + echo " Make a mounted drive permanent by adding one line to /etc/fstab, so" + echo " it comes back after a reboot. Written with nofail, so a missing drive" + echo " can never block boot; the previous file is backed up first." + echo "" echo "storage disks" echo " One row per filesystem, showing what LibrePortal uses it for." echo "" diff --git a/scripts/setup/setup_apply.sh b/scripts/setup/setup_apply.sh index 6d84fc6..d36a8f6 100644 --- a/scripts/setup/setup_apply.sh +++ b/scripts/setup/setup_apply.sh @@ -25,6 +25,7 @@ setupApplyConfig() local traefik_email=$(echo "$payload" | jq -r '.traefik_email // empty') local domains_json=$(echo "$payload" | jq -c '.domains // []') local storage_json=$(echo "$payload" | jq -c '.storage // []') + local storage_fstab_json=$(echo "$payload" | jq -c '.storage_fstab // []') if [[ -n "$install_name" ]]; then updateConfigOption "CFG_INSTALL_NAME" "$install_name" @@ -68,6 +69,17 @@ setupApplyConfig() [[ -z "$sname" ]] && sname="disk$((i+1))" if storageAdd "$s" "$sname" >/dev/null; then isSuccessful "Storage location '$sname' registered at $s" + # Only when explicitly asked on the Storage step. The helper + # validates independently and writes with nofail, so a + # missing drive can never block boot. + if echo "$storage_fstab_json" | jq -e --arg p "$s" 'index($p) != null' >/dev/null 2>&1; then + local fline + if fline=$(runStorage fstab-add "$s" 2>&1); then + isSuccessful "Added to /etc/fstab so it mounts at boot: $fline" + else + isNotice "Could not update /etc/fstab for $s: $fline" + fi + fi else isNotice "Could not register '$s' as a storage location — continuing without it." fi diff --git a/scripts/source/files/arrays/function_manifest.sh b/scripts/source/files/arrays/function_manifest.sh index 2018055..5a29078 100644 --- a/scripts/source/files/arrays/function_manifest.sh +++ b/scripts/source/files/arrays/function_manifest.sh @@ -1020,6 +1020,7 @@ declare -gA LP_FN_MAP=( [storageDisksData]="storage/storage_disks.sh" [_storageEmit]="storage/storage_checks.sh" [_storageFreeHuman]="storage/storage_locations.sh" + [storageFstabLine]="storage/storage_checks.sh" [storageList]="storage/storage_locations.sh" [storageLocationConfig]="storage/storage_locations.sh" [storageLocationDir]="storage/storage_locations.sh" @@ -2243,6 +2244,7 @@ declare -gA LP_FN_ROOT=( [storageDisksData]="scripts" [_storageEmit]="scripts" [_storageFreeHuman]="scripts" + [storageFstabLine]="scripts" [storageList]="scripts" [storageLocationConfig]="scripts" [storageLocationDir]="scripts" @@ -3503,6 +3505,7 @@ storageDisks() { unset -f storageDisks; __lpAutoload "${install_scripts_dir}stor storageDisksData() { unset -f storageDisksData; __lpAutoload "${install_scripts_dir}storage/storage_disks.sh"; storageDisksData "$@"; } _storageEmit() { unset -f _storageEmit; __lpAutoload "${install_scripts_dir}storage/storage_checks.sh"; _storageEmit "$@"; } _storageFreeHuman() { unset -f _storageFreeHuman; __lpAutoload "${install_scripts_dir}storage/storage_locations.sh"; _storageFreeHuman "$@"; } +storageFstabLine() { unset -f storageFstabLine; __lpAutoload "${install_scripts_dir}storage/storage_checks.sh"; storageFstabLine "$@"; } storageList() { unset -f storageList; __lpAutoload "${install_scripts_dir}storage/storage_locations.sh"; storageList "$@"; } storageLocationConfig() { unset -f storageLocationConfig; __lpAutoload "${install_scripts_dir}storage/storage_locations.sh"; storageLocationConfig "$@"; } storageLocationDir() { unset -f storageLocationDir; __lpAutoload "${install_scripts_dir}storage/storage_locations.sh"; storageLocationDir "$@"; } diff --git a/scripts/storage/storage_checks.sh b/scripts/storage/storage_checks.sh index f97168a..8708129 100644 --- a/scripts/storage/storage_checks.sh +++ b/scripts/storage/storage_checks.sh @@ -114,14 +114,38 @@ _storageCheckPersistence() return 0 fi - local src fstype + _storageEmit warn persistence "This drive is not listed in /etc/fstab, so the system will not mount it automatically after a reboot. Apps stored here will refuse to start until it is mounted again." + # The exact line, as its own record: the UI offers to add it, and a person + # reading the CLI can copy it. Kept separate from the prose so neither has + # to be parsed out of the other. + local line; line=$(storageFstabLine "$probe") + [[ -n "$line" ]] && _storageEmit info fstab-line "$line" + return 0 +} + +# The /etc/fstab line that would make this mount permanent. +# +# UUID= rather than /dev/sdX because device names reorder across reboots, and a +# stale one either mounts the wrong disk or nothing at all. +# +# nofail + x-systemd.device-timeout are what make this safe to add at all: a +# missing device then cannot block boot. Without them an absent drive leaves the +# machine sitting at a systemd timeout, or worse in emergency mode — a far worse +# failure than the app-not-starting one we are trying to prevent. +storageFstabLine() +{ + local probe="$1" target src fstype uuid + command -v findmnt >/dev/null 2>&1 || return 1 + target=$(findmnt -no TARGET --target "$probe" 2>/dev/null | tail -1) + [[ -z "$target" || "$target" == "/" ]] && return 1 src=$(findmnt -no SOURCE --target "$probe" 2>/dev/null | tail -1) fstype=$(findmnt -no FSTYPE --target "$probe" 2>/dev/null | tail -1) - local uuid; uuid=$(findmnt -no UUID --target "$probe" 2>/dev/null | tail -1) - local line="${src:-} $target ${fstype:-auto} defaults,nofail 0 2" - [[ -n "$uuid" ]] && line="UUID=$uuid $target ${fstype:-auto} defaults,nofail 0 2" - _storageEmit warn persistence "Not in /etc/fstab: after a reboot this drive will not be mounted, and apps stored here will not start until it is. To make it permanent, add: $line" - return 0 + uuid=$(findmnt -no UUID --target "$probe" 2>/dev/null | tail -1) + local dev="${src:-}" + [[ -n "$uuid" ]] && dev="UUID=$uuid" + [[ -z "$dev" ]] && return 1 + printf '%s %s %s defaults,nofail,x-systemd.device-timeout=10s 0 2' \ + "$dev" "$target" "${fstype:-auto}" } # 7. Removable / hot-plug. WARNS — an external drive is a supported setup. diff --git a/scripts/system/libreportal-storage b/scripts/system/libreportal-storage index 349d538..ab2062c 100755 --- a/scripts/system/libreportal-storage +++ b/scripts/system/libreportal-storage @@ -216,6 +216,103 @@ probe() { return $rc } +# Make a mount permanent by adding one line to /etc/fstab. +# +# fstab is the most dangerous file LibrePortal touches: a bad entry does not +# break an app, it can leave the machine unbootable and needing rescue media. +# Everything below exists to make that impossible: +# +# * nofail + x-systemd.device-timeout=10s — a missing device can then never +# block boot. This single pair is what makes writing fstab defensible at +# all; without it an unplugged drive strands the box at a systemd timeout +# or drops it to emergency mode. +# * UUID=, never /dev/sdX — device names reorder between boots. +# * append only, inside a marked block. Existing lines are never rewritten, +# so anything the user or another tool manages is untouched. +# * refuse when a line for that target or device already exists — we do not +# get to be the second opinion on a mount someone else configured. +# * a timestamped backup, and `findmnt --verify` before the new file is put +# in place. A file that does not verify is discarded, not installed. +# +# Opt-in only: nothing calls this unless a person asked for it. +fstab_add() { + local target="${1:-}" + [[ -n "$target" && "$target" == /* ]] \ + || { _err "fstab-add requires an absolute mount point"; return 2; } + target="${target%/}" + [[ "$target" == *..* ]] && { _err "invalid mount point"; return 2; } + + command -v findmnt >/dev/null 2>&1 || { _err "findmnt unavailable"; return 1; } + + # Refuse the root filesystem before anything else, so the reason given is the + # real one rather than a confusing "not a mount point". + [[ "$target" == "/" || -z "$target" ]] \ + && { _err "refusing to touch the root filesystem's fstab entry"; return 1; } + + # Must currently be a real mount — we describe reality, we do not invent it. + local now_target + now_target=$(findmnt -no TARGET --target "$target" 2>/dev/null | tail -1) + if [[ "$now_target" != "$target" ]]; then + _err "'$target' is not a mount point right now. Mount it first, then add it." + return 1 + fi + + local uuid fstype + uuid=$(findmnt -no UUID --target "$target" 2>/dev/null | tail -1) + fstype=$(findmnt -no FSTYPE --target "$target" 2>/dev/null | tail -1) + if [[ -z "$uuid" ]]; then + _err "'$target' has no filesystem UUID, so no stable fstab entry can be written for it." + return 1 + fi + [[ -z "$fstype" ]] && fstype="auto" + + # Already described? Leave it alone — do not add a second opinion. + local existing + existing=$(grep -vE '^[[:space:]]*#' /etc/fstab 2>/dev/null \ + | awk -v t="$target" -v u="UUID=$uuid" '$2==t || $1==u {print; exit}') + if [[ -n "$existing" ]]; then + _err "/etc/fstab already has an entry for this mount: $existing" + return 1 + fi + + local line="UUID=$uuid $target $fstype defaults,nofail,x-systemd.device-timeout=10s 0 2" + + local stamp backup tmp + stamp=$(date +%Y%m%d-%H%M%S) + backup="/etc/fstab.libreportal-$stamp.bak" + cp -a /etc/fstab "$backup" || { _err "could not back up /etc/fstab"; return 1; } + + tmp=$(mktemp) || return 1 + cat /etc/fstab > "$tmp" + # Guarantee the file ends with a newline before appending, or the new entry + # would be glued onto whatever the last line was. + [[ -s "$tmp" && -n "$(tail -c1 "$tmp")" ]] && printf '\n' >> "$tmp" + { + printf '\n# Added by LibrePortal (%s) — storage location %s\n' "$stamp" "$target" + printf '# nofail: a missing device must never block boot.\n' + printf '%s\n' "$line" + } >> "$tmp" + + # Verify BEFORE installing. findmnt --verify parses fstab and reports + # structural problems; a file that does not pass is thrown away. + if ! findmnt --verify --tab-file "$tmp" >/dev/null 2>&1; then + local why; why=$(findmnt --verify --tab-file "$tmp" 2>&1 | head -5) + rm -f "$tmp" + _err "the resulting /etc/fstab did not verify, so it was NOT installed: $why" + return 1 + fi + + install -m 0644 -o root -g root "$tmp" /etc/fstab || { rm -f "$tmp"; return 1; } + rm -f "$tmp" + + # Let systemd pick up the new unit now, so the entry is live rather than + # only true after the next boot. + systemctl daemon-reload >/dev/null 2>&1 || true + + echo "$line" + return 0 +} + add() { local raw="" name="" allow_home=0 a for a in "$@"; do @@ -384,9 +481,10 @@ action="${1:-}"; shift 2>/dev/null || true case "$action" in add) add "$@" ;; probe) probe "${1:-}" "${2:-}" ;; + fstab-add) fstab_add "${1:-}" ;; remove) remove "${1:-}" ;; list) list ;; path) path "${1:-}" ;; verify) verify "${1:-}" ;; - *) echo "usage: libreportal-storage {add [--name=NAME] [--allow-home]|probe [--allow-home]|remove |list|path |verify [id]}" >&2; exit 2 ;; + *) echo "usage: libreportal-storage {add [--name=NAME] [--allow-home]|probe [--allow-home]|fstab-add |remove |list|path |verify [id]}" >&2; exit 2 ;; esac diff --git a/scripts/webui/data/generators/system/webui_storage_candidates.sh b/scripts/webui/data/generators/system/webui_storage_candidates.sh index 7074393..d2d9bdf 100644 --- a/scripts/webui/data/generators/system/webui_storage_candidates.sh +++ b/scripts/webui/data/generators/system/webui_storage_candidates.sh @@ -42,17 +42,26 @@ webuiGenerateStorageCandidates() # Only offer filesystems LibrePortal isn't already using. [[ "$role" == "free" ]] || continue - local sev check msg refusals="" warnings="" + # One record per check rather than two joined strings: the card shows a + # short summary, the details modal shows the full text, and neither has + # to parse anything back out of the other. + local sev check msg checks="[" kfirst=1 verdict="ok" fstab_line="" while IFS=$'\t' read -r sev check msg; do - case "$sev" in - refuse) refusals+="${refusals:+; }$msg" ;; - warn) warnings+="${warnings:+; }$msg" ;; - esac + [[ -z "$sev" ]] && continue + if [[ "$check" == "fstab-line" ]]; then + fstab_line="$msg" + continue + fi + [[ "$sev" == "warn" && "$verdict" == "ok" ]] && verdict="warn" + [[ "$sev" == "refuse" ]] && verdict="refuse" + (( kfirst )) || checks+="," + kfirst=0 + checks+="{\"severity\":\"$(_lpJsonEsc "$sev")\",\"id\":\"$(_lpJsonEsc "$check")\",\"message\":\"$(_lpJsonEsc "$msg")\"}" done < <(storageCheckPath "$target" 2>/dev/null) + checks+="]" - local verdict="ok" - [[ -n "$warnings" ]] && verdict="warn" - [[ -n "$refusals" ]] && verdict="refuse" + local uuid_val; uuid_val=$(findmnt -no UUID --target "$target" 2>/dev/null | tail -1) + local opts_val; opts_val=$(findmnt -no OPTIONS --target "$target" 2>/dev/null | tail -1) (( cfirst )) || candidates+="," cfirst=0 @@ -61,10 +70,12 @@ webuiGenerateStorageCandidates() candidates+=",\"fstype\":\"$(_lpJsonEsc "$fstype")\"" candidates+=",\"size\":\"$(_lpJsonEsc "$size")\"" candidates+=",\"free\":\"$(_lpJsonEsc "$avail")\"" + candidates+=",\"uuid\":\"$(_lpJsonEsc "$uuid_val")\"" + candidates+=",\"options\":\"$(_lpJsonEsc "$opts_val")\"" candidates+=",\"removable\":$([[ "$rm_flag" == "1" ]] && echo true || echo false)" candidates+=",\"verdict\":\"$verdict\"" - candidates+=",\"refusals\":\"$(_lpJsonEsc "$refusals")\"" - candidates+=",\"warnings\":\"$(_lpJsonEsc "$warnings")\"}" + candidates+=",\"fstab_line\":\"$(_lpJsonEsc "$fstab_line")\"" + candidates+=",\"checks\":$checks}" done < <(storageScanCandidates 2>/dev/null) candidates+="]"