promise.md names Connect as a paid service for "keeping off-site backups", with
two constraints that are load-bearing rather than marketing: it never sees your
data, and every hosted service has a free equivalent in the open code. Nothing
was implemented — no endpoint, no account, no client support.
The client half turns out to be almost entirely there, because a Connect
destination is not a new kind of thing: it is a restic REST repository whose
password never leaves the machine. So `connect` resolves exactly like `rest` in
resticLocationUri — it IS one. It is a separate TYPE only so the UI can tell it
apart from a REST server someone runs themselves, which are identical on disk.
Availability is data, not code: CFG_BACKUP_CONNECT_ENDPOINT (empty) is reported
through the locations feed as connect:{available,endpoint}, and the wizard
renders from that — the option present but disabled, its panel saying what it
will be and that SFTP and S3 do the same job today. The day the service exists,
setting that one value turns it on with no release. Verified both ways.
The device code is a credential, so it goes through the secret channel as a
reference rather than travelling in the wizard payload, which is base64'd into a
world-readable task file.
Design, and what the service still has to provide, in
docs/roadmap/connect-backup-destination.md.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reported: the dropdowns in Add destination don't work. They rendered correctly,
reported as enhanced, and did nothing when clicked.
custom-select portals its popup into <body> at z-index 1200, chosen — as
forms.css says in as many words — to clear eo-modal at 1100. The wizard raises
its modal to 10000, because at 1100 a modal opened from inside the wizard
rendered behind the wizard itself. That fix silently broke the other invariant:
the popup then opened behind the dialog that owns it. Raise the popup with it,
scoped to the wizard so nothing else's stacking moves.
The test already asserted the select was enhanced, which was true and useless —
the control was enhanced, it just could not be reached. So it now hit-tests:
open the popup and ask what is actually on top at its own centre, then click an
option and check the value, the button label and the field group all follow.
Verified by removing the rule again: two checks fail.
That is the second time this pair has bitten (the modal itself did the same
thing earlier), so the rule and the reason now sit together in one comment.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reported after looking at the step: the add button unstyled, the dialog missing
the fields a backup location actually has, and its dropdowns not working. Three
real faults, and one reason all three shipped.
* "+ Add destination" carried class .setup-add-domain, which I invented. The
real one is .setup-domain-add, so no rule matched and it rendered as a bare
browser button in the middle of a styled form.
* The dialog asked for name / type / host / user / path / password. A backup
location has SSH port and auth method (key or password — key is the default
and needs nothing typed), S3 access and secret keys, B2 account id and key,
and a path mode. It now asks for what each backend needs, with the wording
taken from the location config so the wizard and the Backup page describe
the same thing the same way.
* .setup-field styled input[type=text] and [type=email] but not [type=password]
or [type=number], so a credential field and the SSH port rendered unstyled
even inside a correct container.
Only the credentials go through the secret channel — SSH password, S3 secret
key, B2 account key. The rest is ordinary configuration and travels as itself.
The reason all three shipped is that I checked the step by querying the DOM and
never looked at it. Structural checks cannot see an unstyled control, and a
dialog is behind a click so a screenshot cannot reach it either. So:
lp-shot --eval <route> <js> run JS in the page and print the result
LP_SHOT_EVAL=<js> run JS before a capture — open a dialog, then shoot
and scripts/dev/lp-backup-dialog-test drives the whole thing in a real browser:
opens it, swaps every backend and asserts only that backend's fields show,
toggles SSH auth and asserts the password field follows, submits, and asserts
the credential is not left in the DOM.
Its styling check needed two attempts, which is the point of mutation-testing
it: "is the background transparent" passes for an unstyled button, because a
native button is grey rather than transparent. It now compares the control
against a bare <button> in the same parent, so "no rule matched" is what fails.
Verified: reintroducing the wrong class fails the test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The step asked one question — pick a destination, or "not now" — while the
system underneath already had a full location model: eight backend types, per
location engine, path mode, credentials and retention, and a generated
locations.json carrying all of it. None of that was reachable during setup, so a
second destination, or even seeing where the first one points, meant finding the
Backup page afterwards.
Now it mirrors the Storage step — the choice above, the list below:
Backups Automatic — daily, on a schedule | Manual
Destinations Local disk [default] /libreportal-backups/1 [Edit]
+ Add destination
Automatic/Manual needed a setting, because there was no off switch:
crontabSetupBackupScheduler installed the entry unconditionally. CFG_BACKUP_MODE
is explicit rather than overloading "empty schedule", so it reads properly in
the config editor too, and Manual REMOVES an entry that is already installed
rather than merely declining to add one — otherwise answering Manual changes
nothing. The schedule itself is left alone, so switching back restores the time
the user picked.
Destinations are seeded from locations.json, so the default one is shown and
editable instead of being discovered later, and only entries the user actually
added or changed are submitted. A destination on the same disk as the app data
says so on the card rather than in a paragraph under the step.
Remote destinations are what the secret channel was for. The wizard payload is
base64'd into a task's command string and tasks are recorded world-readable, so
a password is POSTed to /api/setup/secret, which writes it where only the
manager can read it and returns an opaque reference; the reference travels in
the payload and setup_apply redeems it once, at the write. A reference that
cannot be redeemed leaves the password alone and says so, rather than blanking
it.
Verified in the browser on a clean install: the step renders both modes, lists
the existing destination at its resolved path, and the add dialog swaps between
local and remote fields. scripts/dev/lp-backup-setup-test covers the apply side,
including that what reaches the config is the secret and never the reference.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A password typed in the WebUI has to reach the host, and both existing routes
leak it. As part of a task's command string it lands in
frontend/data/tasks/*.json — 0644, inside a world-readable directory — and is
visible in `ps` while the task runs; as a plain file there it is either
world-readable at 0644 or unreadable by the manager at 0640. Verified still true
on a clean install. A backup repository password sent that way is the key to
every backup the user has, readable by any local account.
libreportal-ownership gains `secret-dir`: the mirror of _webui_bind_access.
That one makes manager-owned config readable by the container; this makes a
container-written file readable by the MANAGER. The directory is
<container>:<manager> mode 2730 — setgid so each file inherits the manager's
group, the container writes it 0640, and 0730 leaves the directory unlistable
because the manager is handed a filename rather than going looking. Group rwx
is what lets it unlink after reading.
The WebUI then sends a REFERENCE ("secret:<id>") wherever it used to send the
value, and configUpdateBatch redeems it at the last moment before the write.
That is the single point every config write from the WebUI passes through, so
this covers every password field rather than only the backup ones — which is
what docs/roadmap/first-run-restore.md §4.1 asked for. A reference that cannot
be redeemed leaves the field unchanged rather than blanking it.
Verified on a live install: the container drops a secret, the manager applies it
by reference, the file is unlinked, `nobody` can neither read nor list it, and a
second redemption of the same reference fails.
footprint_version 9 -> 10 (root-owned helper changed).
Also fixes a block of constructor initialisations I spliced into the middle of
renderStorageChoices in aa44e0b: on a single-drive box — the case in the
screenshot that prompted this — rendering the Storage step silently reset
backupDest and cleared the import selections.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dockerComposeUp and dockerComposeDown derive compose_file from $compose_setup,
which setupBasicScanVariables reads from CFG_<APP>_COMPOSE_FILE — a variable
that is only set once the app's config has been sourced, and is not always. A
restore wipes and re-creates the app folder around those calls.
setupBasicScanVariables already handles that, falling back to the standard file.
Neither compose function did: with compose_setup empty, neither branch ran,
compose_file stayed UNSET, and the guard
[ ! -f "$(appDir "$app")/$compose_file" ]
then tested the app DIRECTORY, which is never a regular file. So the app was
reported as having no compose file and quietly not started.
Found restoring an app on a clean install: the restore ran to completion and
reported success, having neither stopped nor started the app —
---- 3. Shutting down container(s) for restoration
! Notice Unable to find the compose file to docker compose down this application.
---- 10. Starting up the linkding docker service(s)
! Notice Unable to find the compose file to docker compose up this application.
— while docker-compose.yml sat in the app directory the whole time. Verified:
with CFG_LINKDING_COMPOSE_FILE unset, dockerComposeUp now starts the app.
scripts/dev/lp-compose-file-test covers the empty case, the normal one, and a
genuinely missing file so the guard still fires when it should.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Choosing a drive worked from the CLI but was invisible in the WebUI, for three
separate reasons, each of which hid the next:
* the config editor only renders fields listed in apps-field-mappings.json,
and STORAGE was not one — so no amount of correct data made it appear. Added
there, in General, with its choices built from the locations registered at
generate time (unlike every other select here, they are not knowable
statically).
* app TEMPLATES ship "[default:Primary]", and templates are what the install
form reads for an app that is not installed yet — precisely the app whose
form needs to show which drives exist. storageSyncAllAppComments now covers
templates, and is finally called from a regen path: it was written for one
and never wired in, so every option list was frozen at install time and
adding a drive made it selectable nowhere.
* storageLocationName resolved a name only from an in-scope
CFG_STORAGE_LOC_<id>_NAME and otherwise fell back to the bare id. That name
is the value CFG_<APP>_STORAGE takes, so the generated dropdown offered
"location-1" as both label and value — a choice that does not resolve. Read
it from the location's own config when the variable is not in scope.
Then the control rendered but sat blank. Config values are the raw right-hand
side of "KEY=value # comment"; almost all are stored without a comment, but a
field whose comment is regenerated keeps one — CFG_<APP>_STORAGE records the
location it currently resolves to. updateConfigForm assigned that whole string
to the field, which for a <select> matches no option, sets selectedIndex to -1
and renders empty: an app on a second disk read as "nothing configured", or
after a partial fix as "Primary". Normalise once where the config enters the
form, and never assign a select a value none of its options carry.
Verified in the App Center: authelia, installed on disk1, shows
"disk1 (/mnt/lptest1/apps)" selected, with Primary/disk1/disk2 offered.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The resolver already supported per-app placement — CFG_<APP>_STORAGE names a
location and appDir sends data, compose and config there — and 37 of 39 app
templates ship the field. What was missing was choosing AT INSTALL TIME. The
only routes were editing a config by hand before installing, or installing onto
the default disk and then `app move`ing it, which copies the data twice.
libreportal app install <app> --storage=<location>
and the App Center's existing storage dropdown, which travels inside
config_variables. Both resolve to one answer in storageChoiceFor, so there is a
single code path.
Ordering is the whole difficulty, and getting it wrong is quiet. installApp
copies the app template into appDir(), sources it, and later applies the form
overrides. The choice has to be live before the copy (or the directory is
created on the wrong disk), written into the config before the source (or the
template's "default" wins and every later appDir in that process returns the
primary root), and folded into config_variables (or the override pass writes
"default" back). Miss any one and the directory and its config disagree — which
resolves correctly only until something sources the config.
Refuses an unknown or unmounted location, an existing directory, and an app
whose template marks the field **READONLY** (fixed to the primary root because
other apps reach it by literal path — storageMoveApp already refuses to move
those, and installing one elsewhere is the same violation from the other end).
Three shipped bugs found making this work:
* updateConfigOption chose its write helper by comparing the path against
$containers_dir — the PRIMARY root only — so an app on any other registered
location took the manager branch and `sed -i` failed with exactly the
permission error the comment above that code describes. `app move` writes
the new location with `|| true`, so it reported a successful move while
leaving the config naming the old disk.
* storageLocationName resolved a location's name only from an in-scope
CFG_STORAGE_LOC_<id>_NAME, falling back to the bare id. That name is the
value CFG_<APP>_STORAGE is set to, so the generated dropdown offered
"location-1" as both label and value — a choice that does not resolve. Read
it from the location's config when the variable is not in scope.
* storageSyncAllAppComments was written for "the regen path" and never wired
into one. Every CFG_<APP>_STORAGE option list was frozen at install time, so
adding a drive did not make it selectable anywhere. Called from the storage
generator now, which runs exactly when those lists go stale — and extended
to app TEMPLATES, since an app not installed yet is precisely the one whose
install form needs to show which drives exist.
Verified on a live install with three locations: linkding and authelia on disk1,
ipinfo on disk2, fourteen on the default root, each config naming its own drive.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
initPickRoots can only offer MOUNTED filesystems, so on a box whose second drive
is new — unmounted, often unformatted — it returned in silence and the "where
should LibrePortal keep things?" prompt never appeared. The users most likely to
want a separate disk were the ones told nothing.
Add a notice listing what is attached and how to use it, deliberately only a
notice: mounting or formatting someone's disk is not something an installer
should do unasked, and `storage add` does it properly later with the
empty-directory admission rule and the fitness checks behind it.
Two things it has to get right, and both bit during development:
* "has no mount point" is not "is free" — the disk holding root has no mount
point of its own, its partition does, so the naive check offered the user
the disk they booted from. Walk the parent chain and mark holders in use.
* lsblk -r renders an empty mount point as a run of spaces, which `read`
collapses, shifting every later column left; an LVM member then parsed as a
mount point and was offered as free space. Use -P.
scripts/dev/lp-installer-disks-test runs the real function against a stubbed
lsblk that honours the output flag it is passed — without that the stub answers
-P to everything and cannot tell the two parsers apart, which is the bug being
guarded against. Verified both regressions fail it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
libreportal-relocate re-bakes the helpers, the systemd unit and the WebUI
compose, but never touched the manager's crontab — and the @reboot entry embeds
an absolute path under the SYSTEM dir:
@reboot /libreportal-system/install/scripts/crontab/system/crontab_boot_app_reconcile.sh
Move that root and the path stops existing. The entry is redirected to
/dev/null, so nothing reports it; the only symptom is that apps quietly do not
come back after a reboot, which is easy to blame on anything else.
Rewrite it with the same old->new substitution already applied to the unit and
compose, keeping a copy in the relocation's backup dir first.
(A fresh install is unaffected — it generates the entry with the resolved
install_scripts_dir. Verified on a matrix case-4 install with all three roots on
test disks: the entry points at /mnt/lptest1/... and the target exists.)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The wizard's Storage step builds its first entry from primaryRoot() — the
app-data root — and labelled it "System disk". On a default install those are
the same drive and the name is honest. Installed with --containers-dir on its
own disk they are not, and the step then showed the DATA drive's size under the
system disk's name while the actual system disk never appeared in the list.
Seen on a matrix case-2 install (apps on a 29.4G test disk, system on a 912G
root): "System disk — 26.7G free of 29.4G".
The generator now reports whether that root is really on the OS disk
(is_os_disk, by st_dev against /), and the wizard labels it from that: "System
disk" when they coincide, otherwise the mount point. The "system" badge stays —
it marks the default location, which is still what it is.
Also add lp-shot --token / --cookie-js. A screenshot answers "does it render";
"does this wizard step work" needs clicking, which needs a real browser, which
needs the session lp-shot already knows how to mint from the stored jwtSecret.
This bug was found that way.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The three roots reach running code three ways — the CLI wrapper exports them,
the task-processor unit carries them as Environment=, and anything those start
inherits them. An @reboot crontab entry is started by none of the three: it runs
a script by absolute path, so paths.sh fell through to
: "${LP_CONTAINERS_DIR:=/libreportal-containers}"
That entry is crontab_boot_app_reconcile.sh, which brings every installed app up
at boot. On a relocated install it therefore reconciled against the DEFAULT
root, and that does not fail — docker creates the bind-mount directories it does
not find, so every app comes back empty while the real data sits untouched on
the other disk. Nothing logs an error; the only symptom is opening an app and
finding it blank (storage-locations §10.1 calls this the top data-integrity
risk).
Verified on a case-2 install (apps on /mnt/lptest2): a bare environment resolved
containers_dir to /libreportal-containers/ where the real root was
/mnt/lptest2/libreportal-containers.
Recover them from the systemd unit, which is the authoritative baked record —
init.sh already reads it back the same way, libreportal-relocate rewrites it,
and it is root-owned, so this is not the manager reading a config it can edit.
An explicit environment still wins; with no unit the defaults are unchanged.
scripts/dev/lp-paths-roots-test pins all four cases; verified it fails when the
recovery is removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The three roots are independently relocatable, and the failures that matter are
the ones where only ONE of them moves: paths are baked into root-owned helpers,
the systemd unit and the CLI wrapper at install time, so anything that resolves
a root at runtime instead works on a default install and points at the wrong
disk on a relocated one. Testing "all default" or "all moved" misses that.
scripts/dev/lp-testdisk loopback ext4 disks — a real superblock, its own
st_dev and free space, thrown away between runs
scripts/dev/lp-install-matrix installs across the four root combinations and
checks each landed on the intended DEVICE, that
the helpers were baked (no __PLACEHOLDER__ left)
and that the WebUI answers
First thing the harness turned up: lp-shot hardcoded /libreportal-containers for
both the compose file it reads the published port from and the .auth.json it
signs a session with. On an install whose app data is on another disk it fell
back to a default port and a missing auth file — which looks exactly like a
WebUI that failed to boot. It now reads the baked LP_CONTAINERS_DIR back out of
the CLI wrapper.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The user-namespace prefix that lets an unprivileged restore put back a file's
original owner was only wired into restic. borg extract and kopia snapshot
restore run as the same backup user with the same lack of CAP_CHOWN, so both
lost <container-uid>:<backup-user> exactly the way restic did — an app whose
data comes back owned by the backup user cannot write it, which is how grafana
kept dying with "attempt to write a readonly database".
borg is quieter about it than restic: it does not print an "ignoring error"
line at all, so there was nothing to notice.
Move the prefix to engine_dispatch.sh as backupUsernsPrefix — it was never
restic-specific — and use it from all three engines.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
grafana restored and then died with "attempt to write a readonly database",
repeatedly. Its database is recorded in the snapshot as 231543:1002 and landed
as 1002:1002 — the owner was lost, so grafana, running as 231543, could not
write it at mode 0640.
Restore runs as the backup user with no CAP_CHOWN, so it reinstates ownership
inside a user namespace. The prefix was
unshare --map-root-user --map-users=SUB:SUB:N --map-groups=SUB:SUB:N
and unshare accepts ONE range per option, so the backup user's own GID was never
mapped — while app data is written as <container-uid>:<backup-user>. The group
half of every such chown referred to an unmapped id, lchown returned EINVAL, and
the file kept the restoring user's ownership. restic reports those as "ignoring
error ..." and still exits 0, so nothing failed: 1626 of one 13-app restore's
2086 failed chowns were grafana's, under a restore that reported success.
restic-userns-exec uses newuidmap/newgidmap, which write the multi-range maps
unshare cannot express:
uid: 0 <- caller inner root, or caps are dropped at exec
SUB.. <- SUB.. identity, so restic can name the stored uid
gid: caller <- caller identity: the group half of app-data chowns
SUB.. <- SUB.. identity
The caller's own UID is deliberately not identity-mapped — that slot is spent on
inner root — and a file stored as <caller>:<caller> lands owned by the caller
anyway, because that is who inner root is outside. So the one case this cannot
map is the one case needing no mapping. `unshare --map-auto --map-current-user`
is not a shortcut: it maps the subuid range to low inner ids while restic needs
identity. Tested.
Measured live, restoring grafana: failed chowns 1626 -> 12 (the 12 being the
caller's own files, correct), grafana.db back to 231543:1002, grafana up and
writing. Falls back to running the command plainly when there is no subuid
range, no newuidmap, or the namespace will not start.
scripts/dev/lp-userns-ownership-test pins all three ownership cases; verified
the old prefix fails it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
resticRestoreSnapshot forgives the un-mappable-uid lchown failures so a restore
is not aborted by them, and reported: "expected, they are already owned
correctly". That is true only for LibrePortal's own files, whose owner is the
backup user restic already runs as. It is false for container-owned data.
_resticUsernsPrefix maps the subuid range and root, but unshare takes one range
per option so the backup user's own GID is never mapped — and app data is
written as <container-uid>:<backup-user>. Every such chown fails with EINVAL and
the file falls back to <backup-user>:<backup-user>. Verified directly:
231543:231543 applies, 231543:1002 does not.
Observed on a 13-app restore: grafana's grafana.db is recorded as 231543:1002
and landed as 1002:1002, so grafana (running as 231543) could not write it at
mode 0640 and died with "attempt to write a readonly database" — under a restore
that reported success. 1626 of that run's 2086 failed chowns were grafana's.
This commit does not fix the mapping — that is the backup engine's ownership
handling rather than the first-run restore path, and the candidate fixes
(newuidmap multi-range maps, or restoring as root via a path-validated helper)
want a decision first. See docs/roadmap/first-run-restore.md §3.5. What it fixes
is the reporting: count the files and say plainly that container-owned data was
not reinstated and the app may fail to write.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`restore system` reported
✓ Success System config restored to: /libreportal-system/restore/system-config
for a directory that did not exist. Nothing had been written — on the step the
whole restore ordering depends on, since the system config carries every other
backup location's credentials.
Restore stages through $SYSTEM_DIR, which the manager owns, but the thing that
writes into the staging tree is restic, and runBackupOp runs it as the container
user. Both call sites created the directory as the wrong principal, in opposite
directions:
backupRestoreSystemConfig runFileOp mkdir -> container user; denied on the
0751 manager-owned restore_dir, and unchecked
storageRestoreAppTo runInstallOp mkdir -> manager; restic could then
not create anything beneath it
Restic reports a permission denial as "ignoring error ..." and still exits 0, so
the callers' success checks were satisfied either way.
libreportal-ownership gains restore-stage (creates it cowner:MANAGER 0750 —
owner writes, manager traverses to confirm and review) and restore-unstage
(removes it; neither principal can, so staging trees simply accumulated). Both
confine the path to one component directly under the restore/migrate area.
footprint_version 8 -> 9.
backupRestoreSystemConfig now verifies the tree landed as the user that wrote
it, because the manager cannot read inside its own staging directory.
Verified on a live install: system config stages 57 real files, and the
relocation branch of storageRestoreAppTo ran for the first time — speedtest
restored from a snapshot taken at /libreportal-containers/speedtest into
/libreportal-alt/speedtest via stage-and-move, staging cleaned up afterwards.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
portFindNextAvailablePort consulted LibrePortal's own network_resources table
plus a hardcoded list (8080, 6060) and CFG_RESERVED_PORTS_EXTRA — while the
comment above it claimed a picked port "can never collide with a host service at
compose-up time". It can: the list only covers what someone thought to write
down. Ask the kernel instead, via ss, read once per allocation rather than per
candidate. No ss => empty set => exactly the old behaviour.
Found while restoring 13 apps onto a desktop, though not the cause there:
stoat's livekit publishes a FIXED udp range (50000-50100, which it advertises to
clients and so cannot be re-rolled), and kdeconnectd held 50016. That collision
needs its own answer; this fixes the randomly-allocated ports, which had the
same exposure with no reason to.
Also make the bulk restore stop reporting a half-running app as a clean success.
continue-on-error lets a failed compose-up log and carry on, so restoreAppStart
returns 0 either way — which is how that run printed "13 apps restored" while
four of stoat's containers had exited 101. checkSuccess already appends every
failure to error_report.log, so watch it grow across each app and name the ones
that were noisy.
scripts/dev/lp-port-host-test binds a real socket and asserts the allocator
refuses that port; verified it fails when the check is removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
scripts/dev/lp-cli-argv-test builds stubs from the real invocation line in
init.sh and the real LP_CLI_ARGS line in start.sh, then pushes thirteen app
names through them — so editing either file is what makes it fail. Verified
against both regressions: dropping "$@" from the wrapper, and reading "$@"
instead of "${@:10}" in start.sh.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An end-to-end run restored 4 of 13 apps and reported
"First-run restore complete — 4 apps restored" as a success.
Two truncations, and fixing the first had hidden the second:
* the CLI dispatcher calls handlers with no arguments, so "$@"/shift inside
one operate on an empty list. Fixed earlier with LP_CLI_ARGS.
* LP_CLI_ARGS was built from start.sh's "$@" — but the root wrapper invokes
start.sh with exactly nine hardcoded positional slots. So the array could
never hold more than nine entries, and `${LP_CLI_ARGS[@]:5}` yielded at
most four app names.
The wrapper now forwards the real argv after those nine slots (they stay
untouched: every dispatcher reads them, and unset ones must keep arriving as
the literal "empty"), and start.sh reads it back as "${@:10}". Verified: a
preflight given six apps checks six, where five was the previous ceiling.
footprint_version 7 -> 8, since the wrapper is root-owned and baked at install.
Two further fixes so a truncation cannot pass as success again:
* restoreFirstRunBulk with no app list is now a whole-host restore — it
discovers the host's apps and re-applies the preflight. The installer's
report runs in its own process, so without this an app the user was told
would be skipped got restored anyway. init.sh now passes no list, so a
whole-host restore builds nothing that can be truncated.
* it counts what actually landed and returns non-zero naming the failures,
instead of reporting the length of the list it was handed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
storageSnapshotSourcePath resolved a snapshot's source path with
engineSnapshotsJson "$idx" "$snapshot_id"
but that function's second parameter is an app TAG filter. So it ran
`restic snapshots --tag app=<snapshot-id>`, matched nothing, and returned 1 —
every time, for every snapshot, since the file was written.
Nothing broke loudly, because both callers have a fallback:
* storageRestoreAppTo fell through to "restoring in place", reinstating the
exact cross-root bug the file exists to fix — restoring onto a host whose
containers root differs from the source's matched no include path and
restored nothing, silently
* the first-run preflight never read a manifest, so every app reported size
"?" and its fit and location checks passed unconditionally. Thirteen green
ticks that had checked nothing.
Add engineSnapshotPaths: restic answers it with a positional snapshot id, kopia
by filtering its list. borg has no adapter on purpose — it rebuilds its listing
from archive metadata that carries no paths — so a missing adapter is a quiet
"no" and those callers keep their in-place fallback.
Add scripts/dev/lp-preflight-test, which pins the cases that must say NO: an
app too big for the disk, one this version no longer ships, one whose storage
location is gone, and a resolver that reaches for the app-tag filter again.
Verified against both historical bugs — reintroducing either fails the test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
backupLocationEnsureDir and the config write both went through runFileOp /
runFileWrite, which run as the container user. Backup location configs live
under the system tree, which is owned by the manager — so the mkdir was denied,
the write then failed with "No such file or directory", and locationAdd still
printed "Location N added".
The result was a location that existed in name only: every later command that
sourced its config found nothing. It surfaced in the first-run restore path,
where the installer adds the location it is about to read from and then fails
with "Backup location 2 has no config".
Use runInstallOp/runInstallWrite, which run as the manager and can write there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ran export -> uninstall -> import on trivy against the live install. It
worked end to end (1.3G app, marker file byte-identical afterwards,
container running, database status correct, tree owned by the container
user) but only after three real bugs, none of which syntax checks or
isolated tests would have caught.
Export wrote the tarball as the CONTAINER user, because tar has to read
app data holding sub-UIDs the manager cannot. That meant the container
user also had to be able to create the destination file, which fails for
any normal destination. Now tar writes to stdout and the caller's shell
creates the file: reading uses the privileges that need it, writing uses
the caller's. Import had the mirror-image bug — tar extracted as the
container user and so could not READ a manager-owned .lpapp; the caller
now opens it and tar reads stdin.
Export also failed at tar time with no hint that the destination was the
problem, so it checks the directory exists and is writable up front.
The third one was quiet and worse. The manifest is pretty-printed, so it
reads `"size_bytes": 1324973614` — with a space that a `"key":[0-9]*`
pattern does not match. Both size_bytes and storage.location came back
empty everywhere they were read, which turned "will it fit" and "does
that location still exist" into checks that always passed. That is the
failure mode preflight exists to prevent, hiding inside preflight itself.
Fixed in app_portable.sh and restore_preflight.sh.
Verified afterwards with crafted manifests: an app claiming 8 TB is now
refused on an 800 GB disk ("Needs 8192G, 806G free"), and one naming a
location this machine lacks warns and names the fallback.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds the wizard step for importing existing apps, so the common case is
answerable in the WebUI rather than only from a terminal.
Path-based, not upload, and that is the design rather than a shortcut. A
.lpapp is a plain tarball and the file is already on the server, so
nothing secret crosses into the browser — which is exactly why this can
live in the WebUI when the encrypted-repository restore cannot (§4.1).
Accepts a single file or a folder of them.
Check first, then accept: the step enqueues `app import-check --publish`,
polls the document it writes, and renders one row per file with its
verdict — ready, a warning (its old storage location is gone, so it will
land on the default), or a refusal (already installed, no longer shipped,
will not fit). Refused rows are shown greyed with the reason rather than
hidden, and cannot be selected.
setupApplyConfig re-runs appImport's own checks rather than trusting the
payload: the machine can change between the check and the apply, and the
list arrives from a browser.
The backend route shell-quotes the path — it reaches a command line and
is user input.
Verified: the step renders as "Step 6 of 7", and the underlying check was
proven against real .lpapp files (correct app name from the tar, size
from the manifest, warning for a missing storage location, refusals for
an already-installed app and a non-export).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`libreportal app import-check <file-or-directory>` reports what an import
would do, as one JSON object per file, without touching anything. It runs
the same checks appImport makes — app still shipped, not already
installed, fits on the target, storage location still exists — so a UI can
show them and ask for acceptance before acting.
This is what makes a path-based import safe to drive from the WebUI when
the repository restore is not. A .lpapp is a plain tarball, not encrypted,
so there is no password to collect and nothing secret crosses from the
browser to the host — the blocker recorded in first-run-restore.md §4.1
simply does not apply.
Accepts a single file or a directory of them, so "point at this folder"
works as well as "point at this file".
Verified against real .lpapp files built for the purpose: app name read
from the tar's top-level directory rather than the filename, size read
from the manifest, a location the machine does not have downgraded to a
warning naming the fallback, and refusals for an already-installed app
and a file that is not an export at all.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`libreportal app export <app>` writes one app to a single file;
`libreportal app import <file>` installs it here. This is the thing the
original request described as "upload or navigate to the backup file" — a
restic repository is not a file, but the want behind the phrasing is real.
The format is deliberately boring: gzipped tar of the app directory with
its .libreportal-manifest.json at the root. That manifest already records
size, images, volumes, databases and storage location, so import reuses
the phase-3 checks for free — refusing an app this version no longer
ships, or one that will not fit, before unpacking anything.
Export stops the app first. A tar of a running Postgres is a corrupt
Postgres, and a file that looks fine until you restore it is worse than a
refusal. tar runs as the owning user with --numeric-owner so container
sub-UIDs survive the round trip instead of being remapped through this
machine's /etc/passwd.
Import re-runs the normal install pipeline after unpacking, because the
compose still carries the SOURCE machine's ports, IPs and domains — that
pipeline is what re-allocates them here, and migrateUrlRewrite fixes the
host-bound CFG_* fields.
Documented throughout as a courier format, not a backup: no history, no
retention, no encryption. Importing under a different name is refused
outright rather than half-working — the CFG_<APP>_* namespace and compose
identities would all need rewriting, and `instance create` already
answers "a second copy".
Fixes a bug this surfaced: _appDirIntended did an indirect expansion on
CFG_<SLUG>_STORAGE without checking <SLUG> can be a variable name, so a
hyphenated or mistyped app name emitted "invalid variable name" and then
reported the misleading "storage location is not mounted" for an app that
simply did not exist.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`restore preflight <loc_idx> <host>` reads every app's manifest out of its
own snapshot (engineDumpFile pulls a single file without restoring) and
checks it against this machine before anything is written:
* an app this version no longer ships is skipped — restoring one
produces a directory that can never start, and looks like success
until someone opens it
* an app that will not fit is skipped individually, because filling the
disk part-way through takes the apps that already landed with it
* a manifest naming a storage location this machine lacks falls back to
the default, and says which app moved where
The installer's restore path runs it and asks once before continuing.
Two bugs found by running it against the live repository rather than
reading it:
The CLI dispatcher calls handlers with NO arguments, so `shift 4; "$@"`
inside one operates on an empty list. `restore first-run bulk` has always
had this — a bulk restore silently received zero apps. Fixed at the entry
point: start.sh now captures LP_CLI_ARGS from "$@", and both call sites
use it.
And the wrapper fills unset argv slots with the literal string "empty"
(${5:-empty} … ${9:-empty}), so a trailing slot arrives as a five-
character app name rather than a blank. Filtering on -n alone let five
phantom apps through and reported each as "no longer shipped". Both call
sites now drop the sentinel. That also caps any explicit list at five, so
preflight discovers the host's apps itself when given none.
Verified against the live repository: 13 apps discovered and checked.
Sizes read "?" there because those snapshots predate manifests carrying
size_bytes — the intended graceful fallback, not a refusal.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing prompted anyone to configure backups, so the people most likely
to need a restore were the least likely to have one. The wizard now asks,
once, with the drives it already scanned as the options.
Three messages, because the honest answer differs by choice:
declined nothing is protected until you set it up
same drive still covers deletion, a bad update and ransomware — not
this disk failing, since the data and its only copy go
together
another drive the repository is encrypted; write the password down
somewhere other than this machine
That last one matters more than it reads. An encrypted repository cannot
be opened with anything stored inside itself, and the location password
lives in the system config, which is inside the backup. On a rebuilt
machine the user must supply it by hand — so the wizard says so up front
rather than letting them discover it during a restore.
The password is deliberately NOT echoed by setupApplyConfig: task output
is logged, and a secret in a log is a secret you have to treat as leaked.
It is shown on the Backup page, which is what the wizard tells the user.
locationAdd creates a location disabled, so the applier enables it and
runs engineInitLocation — an un-initialised destination silently backs up
nothing, which is the worst possible way to have "configured backups".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Root-run command to move the system and/or containers root to another
disk after install, re-baking everything that carries those paths: the
nine root-owned helpers, the CLI wrapper, the systemd unit and the
WebUI's own compose bind-mounts.
Deliberately NOT in the manager's scoped sudoers, and symlinked into PATH
like the uninstaller. Moving a root re-bakes the very helpers the sudoers
allowlist trusts, so a helper that did it from a caller-supplied path
would hand the manager the entire trust boundary those helpers exist to
defend. A human with real root runs this; the WebUI can only print the
command, which is what the Storage step now does.
Copy-verify-then-leave, never move: the source tree is not removed at all
— the command tells you to delete it once you have confirmed the WebUI
works. An interrupted run therefore leaves a working install behind
rather than half of one, and the pre-relocation copies of every
root-owned file are kept under $lp_lib_dir/.relocate-<timestamp>/.
Admission mirrors libreportal-storage: absolute, no "..", not a protected
system path, not already in use, must be an empty directory, roots must
not nest, and space checked with 10% headroom.
One bug worth recording, caught on the first test run against a live
install: _validate_target was called inside $(...), and `die` runs `exit`
— which inside a command substitution kills only the subshell. Every
refusal silently became "proceed" and the relocation ran. No damage (the
copy steps were guarded on a now-empty variable, so the re-bake wrote
identical values and only the service bounced), but it is exactly the
difference between a refusal and an unintended relocation. It now sets a
global and returns, so `die` exits the script it is meant to.
footprint_version -> 7 for the new root-owned executable.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Per-app placement worked but had no default: a box with a big second disk
meant setting CFG_<APP>_STORAGE on every app individually. CFG_STORAGE_DEFAULT
fixes that, and the wizard asks for it in one line.
CFG_<APP>_STORAGE now has three states rather than two, and the third is
the point:
<name> this app goes there, whatever the default says
primary this app goes on the install-time root, explicitly
default no opinion — follow CFG_STORAGE_DEFAULT
Templates ship "default", so the setting reaches every app without
touching 37 configs, while an app that was deliberately placed keeps its
placement. "primary" is new, and needed: without it there was no way to
say "keep this one on the system disk" once the global default moved.
A default naming a location that has since been removed falls back to the
primary root rather than refusing — a disk that got unregistered must not
make apps un-installable.
The wizard asks only once a second drive is ticked; with nothing ticked
there is one possible answer and a control would be furniture. It sets a
default, not a placement, and the value stored is the location NAME, so it
survives the disk being remounted elsewhere.
scripts/dev/lp-storage-default-test covers all three states plus the
removed-location fallback.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The card said "911.9G · 808.4G free · ext4". The filesystem type is a
Details row, not something you choose a drive on, so it goes.
On percentage vs size: which one matters depends on the question. This
step asks "will my data fit?", and absolute free space is what decides
that — a 4 GB disk that is 89% free is still useless for a media library.
Percentage answers "is this filling up?", a health signal rather than a
placement one. So the text carries the magnitude ("808.4G free of 911.9G")
and a thin bar carries the proportion, which is what the eye reads
fastest, with no second number competing with the first.
The bar fills with FREE space, not used. Filling by usage made a healthy
7%-full disk render as an almost-empty track that read as a broken widget
— and it pointed the opposite way to the text beside it. Filled = room to
spare, draining = filling up, matching the words. It turns amber below
25% free and red below 10%.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three changes to the Storage step.
The system disk is now a first-class entry — pinned first, ticked, and
locked, since apps fall back to it and it therefore cannot be deselected.
Its Details work like any other drive's, which is the whole point on a
single-disk box: the step now answers "where does my data actually go?"
instead of being skipped and answering nothing. The step is consequently
unconditional; the note changes to explain that no other drives were
found rather than the step vanishing.
The system entry is excluded from the submitted payload — it is already
the primary root, and asking the helper to register it would (correctly)
be refused for nesting.
Cards are one line again. Listing every warning under each drive pushed
them to three lines and made the step tall for no gain: the badge already
carries severity and Details carries the explanation. The note now says
to open Details for the reason rather than claiming it is on the card.
Badge colours were dark-on-light, which against the wizard's mid-blue
glass read as muddy grey — the "needs care" pill in particular. Switched
to light-on-dark, legible without shouting over the drive name.
Verified with lp-shot in both states: system disk alone, and system disk
plus a second drive.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Storage step was a technical dump: every check's full sentence
concatenated onto the card, so the fstab line the user is meant to act on
was buried in prose nobody reads.
The card now shows plain facts and at most two short flags — "Low on
space · Won't be mounted after a reboot" — with everything else behind a
Details button. The modal carries the technical spec (device, UUID, mount
options, removable), every check with its full explanation, and the
fstab offer.
That needed the shell to stop joining checks into one string: the
generator emits a record per check, plus the fstab line as its own field,
so neither the card nor the modal has to parse anything back out of the
other.
The screenshot caught a bug this restructure introduced: summaries keyed
on check id alone, so a PASSING check printed the failure wording next to
a green tick — "This drive's format can't store file ownership" above
"Filesystem: ext4". Now severity-aware.
On writing /etc/fstab — §1 ruled it out and §6.3 now records why that
reverses. The warning is useless to the audience this is for: "add this
line to fstab" assumes SSH, root, an editor, and knowing what fstab is,
and the likely outcome is a reboot where nothing starts. What makes it
defensible is nofail + x-systemd.device-timeout, which mean a missing
device can never block boot — without that pair it would stay a non-goal,
because the failure being risked (an unbootable machine) is worse than
the one being fixed.
Enforced in the root helper: UUID never /dev/sdX, append inside a marked
block, refuse a target or UUID already described, refuse the root
filesystem, require a live mount, timestamped backup, and
`findmnt --verify` before the file is installed — a file that doesn't
parse never reaches /etc. Opt-in only.
Verified against a real filesystem: entry added and verifies, the
persistence warning then disappears on the next scan, and duplicate /
root-fs / non-mountpoint / relative are each refused with the reason.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
webuiGenerateStorageCandidates now runs as part of webuiSystemUpdate, so
frontend/data/storage.json exists without anyone remembering to generate
it — the wizard reads it to decide whether its Storage step appears, and
the Disks view reads the same file, so the two can never disagree.
Warnings arrive from the shell joined with "; ". Rendering that verbatim
produced one run-on paragraph that buried the fstab line the user is
supposed to copy, so the card splits them back onto separate lines.
Verified on the live install with lp-shot: with one filesystem the wizard
shows "Step 1 of 4" and the Storage step is correctly absent; with a
second filesystem attached it becomes "Step 4 of 5" with the drive
carrying a "needs care" badge and both warnings legible. That also
exercises the visible-step mapping in both directions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 3 of docs/roadmap/storage-locations.md.
The step appears only when the candidate scan finds a filesystem
LibrePortal isn't already using, so the single-disk case — which is most
boxes — is completely unchanged. It sits before Recommended because a
location has to exist before an app can be placed on it.
Supporting two conditional steps meant the wizard could no longer treat
'position in the DOM' and 'step index' as the same number: Metrics was
advanced-only and got away with 'length minus one', but a step hidden in
the MIDDLE leaves a gap. Navigation, progress, validation and submit now
all run off _visibleSteps(), and section matching is by data-step rather
than DOM position.
Unusable candidates render greyed WITH the reason rather than being
filtered out — 'why isn't my drive listed?' is a support burden, and
'exFAT can't store file ownership' is actionable. The step is skipped
only when nothing usable was found at all.
What the wizard sends is a request, not an instruction: setupApplyConfig
feeds each path through storageAdd, so the fitness checks and the root
helper's admission rules both re-run regardless of what arrived in the
payload.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Restore built its restic include filter from THIS host's containers root:
engineRestoreSnapshot "$idx" "$id" "/" "$containers_dir$app"
restic reproduces a snapshot's absolute paths, so that only works when
both sides agree byte-for-byte. LibrePortal has shipped configurable
roots for a while, so restoring a snapshot taken on a host installed with
--containers-dir=/mnt/ssd/apps onto a default host matched no include
path and restored NOTHING — with no error, because an include filter that
matches nothing is not a failure. Storage locations turn that from a rare
cross-host case into an ordinary one.
storageSnapshotSourcePath asks the repository where the app actually
lived. storageRestoreAppTo restores in place when that agrees with where
the app belongs here, and stages-then-moves when it does not — which is
also what makes "restore this app onto a different disk" possible at all.
Both restore_app_start.sh and resticRestoreAppLatest go through it, and
both fall back to the old behaviour when a snapshot does not report its
paths, so older snapshots restore exactly as before.
The move into place runs as root (app-adopt) for the same reason app-move
does: a restored tree carries container sub-UIDs the manager cannot
recreate. Staging is constrained to the restore/migrate area and the
destination is validated against the root-owned registry, so neither end
is taken on trust from the caller.
The manifest now records where an app lived — location name, path and fs
uuid. The name is what travels, since a path means nothing on the other
host; the rest is for diagnostics and for answering "is this the same
disk?" during a migrate.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 2 and 4 of docs/roadmap/storage-locations.md. Apps can now be
placed on a location and moved between them.
CFG_<APP>_STORAGE lands in all 37 app templates, holding a location NAME
rather than a path: names survive a migrate to a host with different
disks, paths do not. The 11 infrastructure apps that other apps reach by
literal path (traefik, prometheus, grafana, adguard, gluetun, crowdsec,
headscale, dashy, pihole, unbound, wireguard) are pinned. libreportal
itself never gets the key — it is pinned structurally by webuiDir.
Pinning needed no second config key. "Pinned" is not a fact about a value,
it is a statement about whether the field may be edited, so it goes in the
comment beside **ADVANCED** and **DEV** as **READONLY**, and the field
factory renders those disabled. That marker earns its keep beyond this
feature: derived fields already warned in prose that editing them does
nothing (crowdsec.config:72) next to a perfectly editable input.
storage_app_config.sh keeps the comment honest — it carries the resolved
path for hand-recovery and regenerates the dropdown from the registry, but
only writes when something actually changed, since the app .config is
user-editable and lives in the container-owned tree.
app move stops the app (a live copy of a running Postgres is a corrupt
copy), snapshots it, copies, verifies, and only then removes the source.
The copy runs in libreportal-ownership because it must: app data holds
rootless sub-UID files the manager can neither read nor recreate.
Verified against two real ext4 filesystems that a cross-device move
preserves uid 231141 and the payload, and that the source survives every
refusal path — unregistered destination, the WebUI app, a traversal in the
app name, and an occupied destination.
Task titles registered in both tables, with a specific rule so a move
renders as "Nextcloud - Move to bigdisk" rather than the generic fallback
dropping the destination. lp-task-names could not be run to confirm — it
borrows the WebUI container's node and no containers are running.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 1 of docs/roadmap/storage-locations.md — locations can now exist.
Nothing places an app on one yet; that is phase 2.
libreportal-storage is the only writer of the root-owned registry, and
its admission rules are what make adding a location safe: absolute and
canonical (a symlinked path is refused), outside the protected system
set, non-nesting with any known root in either direction, and EMPTY — or
already carrying our marker, which is the adopt case for a drive that
already holds app data. Root only ever chowns an empty directory, so
acceptance cannot hand away anything that existed. The parent must also
not be manager-writable, which is what closes the validate-then-chown
race; /mnt and /srv qualify, a path inside the manager's home does not.
The fitness checks answer a different question — "will app data actually
work here" — and grade rather than refuse. Only checks 1-5 (filesystem
type, mount options, ownership, sub-UID range, write/read-back) can block.
Reboot persistence and removability warn, because both describe supported
setups and start-up is already gated by the marker test.
The ownership probe had to move into the root helper. For a candidate the
directory is not ours yet — a fresh /mnt/disk is root-owned 0755 — so an
unprivileged probe could only ever report "cannot create a directory
here", which says nothing about the filesystem. Verified against a real
loopback ext4: it now reports ownership, sub-UID and read-back cleanly.
The Disks view unions the registry with attached hardware, registry
first. Verified on the live box that pulling a drive leaves its row in
place as not-attached, naming the app stranded on it, rather than the row
silently disappearing at exactly the moment someone needs it.
Also registers storage_scripts with both loaders, adds the CLI category
(auto-dispatched by cli_initialize.sh), and bumps footprint_version to 6
for the new root helper and the widened sudoers allowlist.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to 928e244, which stopped configs/ subdirectories being sourced
without a .category marker. That closed the hole; this removes the thing
that fell into it.
storageIndexFile pointed at configs/storage/app_locations. The file's
requirements are only "manager-owned" and "not on a removable disk" —
configs/ satisfies both, which is why I put it there, and it was still
wrong: that tree carries a third property the file violates. sourceScanFiles
SOURCES what it finds under configs/, and sourcing means executing.
The index is a TSV of "<slug><TAB><root>", which bash reads as a command
and its argument. Harmless while no slug matched a real executable. The
row for the app named `libreportal` armed it, because that IS the CLI on
PATH: sourcing ran `libreportal /libreportal-containers`, which re-entered
the scan, which sourced the file again — one process pair per level until
the host OOMed and took the desktop session with it.
It now lives at $system_dir/storage/app_locations, with a one-shot
migration so an install that already has an index keeps knowing where its
apps live rather than silently forgetting. libreportal-ownership
reconciles the new directory, and scan_files.sh gained a note that
configs/storage/ carries no .category on purpose.
scripts/dev/lp-configs-guard-test covers both ends: the index never lands
in configs/, a legacy one migrates, and a file of the exact detonating
shape placed in an unmarked configs/ subdirectory is not executed while a
marked category still loads.
Also wires sourceStorageLocations into the config scan beside
sourceBackupLocations — per-location configs sit at depth 3, below the
generic scan, and need their own walker like the backup ones do.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sourceScanFiles sourced every file two levels deep under configs/, and
sourcing means executing. A directory used there as ordinary storage
therefore turned its contents into a script.
storageIndexSet caches an app -> root TSV at configs/storage/app_locations,
with no .category marker alongside it. Every line is `<slug><TAB><path>`,
which bash reads as a command and its argument. That stayed invisible while
no slug matched a real executable — and became a fork bomb the moment the
index recorded the app named `libreportal`, because that IS the CLI on PATH:
sourcing ran `libreportal /libreportal-containers`, which re-entered the same
scan, which sourced the file again, one process pair per level until the host
died of OOM. Every CLI invocation on the box detonated it, the task
processor's own poll included, so the machine black-screened out of memory
minutes after each boot.
Files in a SUBDIRECTORY are now sourced only when that directory carries
.category — the contract commandReloadConfigs already enforces in the CLI
wrapper, and one every real config category (webui, general, security,
backup, network) already satisfies. Files directly in configs/ are unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The main sweep — ~260 call sites across ~100 files move from string
concatenation on a single root to appDir/storageAppDirs/storageAppConfigs.
On a single-root install the resolved paths are identical, so this is a
no-op until a location is registered.
Enumerators were the interesting half. `for d in "$containers_dir"/*/`
appears in the menus, the registry/artifact scanners and the DNS setup —
and a shell glob cannot list a rootless 751 tree at all, which is the
same bug config_find_file.sh already documents in a comment. Routing them
through storageAppDirs (which enumerates as the owning user) fixes that
alongside the multi-root work.
Three places needed judgement rather than substitution:
db_app_scan.sh deletes database rows and port allocations for apps whose
folder is missing, and reaps "empty" app dirs. With a storage location
unmounted, every app on it looks exactly like that. Each of those
branches now gates on appStorageAvailable first — an app on an unplugged
drive is skipped with a notice, never deleted.
instance_create.sh rewrites cloned hooks so an instance touches its own
directory instead of the base app's. Its sed matched ${containers_dir}<type>,
which this sweep just replaced with $(appDir <type>) — so it would have
silently stopped redirecting, and an instance would have written to the
original's files (the adguard auth adapter case its own comment warns
about). Now matches both appDir forms, verified against bare, quoted,
unrelated-app, legacy and prose cases.
peer_shell/peer_pull streamed and extracted relative to the primary root.
Both now use the app's own root, and peer_shell keeps a single-root
fallback since it runs as a restricted SSH shell with no LibrePortal env.
Also fixes a pre-existing bug found on the way: webui_app_config.sh
tested "$containers_dir/frontend/data/last_update", one level short of the
real tree under the libreportal app dir, so the WebUI refresh trigger
after a config update has never once fired.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two mechanical sweeps, no behaviour change on a single-root install.
The 14 `[[ "$p" == "$containers_dir"* ]]` prefix tests that decide
manager-vs-container-user elevation become pathIsContainerData, so a file
on a second storage root is no longer misclassified as manager-owned —
which would have written it with the wrong owner and failed later, far
from the cause. The 65 references to the WebUI's own tree become
webuiDir(), which is pinned to the primary root by design.
Two traps found while doing it:
run_privileged.sh is sourced directly by init.sh without paths.sh, so it
needs a fallback. Defining one named pathIsContainerData was wrong:
generate_function_manifest.sh indexes top-level definitions, and the
resulting autoload stub would have shadowed the real multi-root
implementation with the primary-only fallback — silently classifying
every file on a second disk as manager-owned, which is exactly the bug
this sweep exists to prevent. Renamed to _runCfgIsContainerPath, which
delegates when the real one is loaded.
setup_lock.sh built its path in a top-level assignment, so it was
evaluated at source time and needed the file flagged eager. Made it a
function instead: the path resolves on call, and the file drops off
LP_EAGER_FILES entirely.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 0 of docs/roadmap/storage-locations.md — the resolver layer. No
behaviour change yet: with no registry present, every function here
returns exactly what the old single-root code did, which is what makes
the ~200-site sweep that follows safe to land incrementally.
primaryRoot / webuiDir the install-time root, and the one tree that
never moves
storageRoots every registered root, primary first
storageRootAvailable marker present == drive mounted
pathIsContainerData replaces the `== "$containers_dir"*` idiom
that picks manager vs container-user elevation
appDir / appDirSlash THE resolver, memoised
storageLocationPath/Name name <-> path, via the root-owned registry
storageIndexGet/Set app -> location cache
appDir resolves discovery-first: whichever root actually holds
<slug>/<slug>.config wins, so a hand-move or half-finished migration
self-heals rather than corrupting.
The index exists because of a bug the unit test caught immediately.
Discovery cannot see an unmounted disk, so an installed app on an
unplugged drive looked identical to a brand-new app — and the fallback
handed back the PRIMARY root. Docker would then have created the bind
mounts there and booted the app empty on the wrong disk, which is the
precise failure the availability design exists to prevent. The index is
manager-owned (deliberately not on the removable disk: it must be
readable exactly when that disk is absent), consulted only when the scan
comes up empty, and rewritten by every successful scan so the disk stays
authoritative whenever it is actually present.
Availability is gated in appDir alone rather than at each caller: every
site reaches it by construction. It returns non-zero AND prints an
unusable sentinel path, so the many callers that will never check $?
still fail loudly on something harmless.
scripts/dev/lp-storage-test covers all of it against a throwaway tree.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fixing _instanceRewriteTools does nothing for an instance already on disk, and
a clone from the old code is broken in ways that never announce themselves:
every Tools action answers "App '<slug>' has no tool '<id>'" because
dockerAppRunTool wants app<Ucfirst><Pascal>; `authPersistCfg <type>` writes the
instance's new admin credential into the BASE app's config; and the clone
defines the base app's adapter and tool names while its bodies exec against the
instance's container, so the loader keeps whichever it sourced last and the
base app's user tools can end up administering the instance — decided by
nothing but find(1) order. Seen on a live install: the generated manifest
resolved [appBookstackListUsers] to bookstack_test's copy.
libreportal instance repair [slug] [--dry-run]
Rewrites the template dir only — no container is touched, nothing reinstalled,
so it does not route through the task system the way create/remove do.
Idempotent by construction. Two of the three renames match their own output
(appMattermost_teest… still starts with appMattermost), and a clone from the
old code is only PARTLY wrong — its suffix hooks were always correct and end at
the slug with no trailing underscore, which the infix rule would otherwise read
as type + id + () and append the id twice
(appSetupComposeTags_nextcloud_family_family). Three sentinels park the
already-correct spellings before the rewrite and restore them after, so a
healthy instance is a no-op and an interrupted run can just be re-run.
Verified against fixtures built with the old rule set for all five
multi-instance apps that ship tools: after repair each tree is byte-identical
to a fresh clone from the fixed cloner, a second pass reports "already
correct", and --dry-run leaves checksums untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four defects that all reduce to "a function an instance defines is invisible
to the code that dispatches it". Reported as `mattermost_teest has no upgrade
verifier`, for an app whose verifier was on disk the whole time.
* generate_function_manifest.sh shipped 0664. lpRegenArrays invokes it as an
executable, so it died rc=126 on every call and `|| true` swallowed it — the
manifest was never rebuilt on any live system, only laid down at deploy.
Its sibling generate_arrays.sh is 0775, which is why the files_*.sh arrays
looked current while the manifest was byte-identical to the shipped copy.
* lpRegenArrays now runs both generators through bash rather than depending on
the exec bit, reports a manifest failure instead of hiding it, and treats a
new containers/<app> dir as stale — the one event on a live box that adds
functions was the one the scripts/-only mtime check could not see.
* updaterHasVerifier consults the disk before answering no. The CLI runs
LP_LAZY=1, where the container scan is skipped and every function must come
from the build-time manifest, so an app created after the build reads as
having no verifier. GATE 1 then refuses an upgrade that is fully verifiable,
and updaterUpgradeAuto's `|| continue` drops the app in silence for good.
Self-healing regardless of manifest staleness, which matters because a
self-update restores the shipped manifest and drops instance entries again.
* _instanceRewriteTools gains three renames. authAdapter_<type>_<method>() was
caught by neither the prefix rule (no word boundary before _<type>) nor the
suffix rule (needs () right after the type), so the clone defined the base
app's adapter name while pointing at its own container — every instance user
tool answered "does not implement", and which definition survived came down
to find(1) order. Bare-app arguments to authAdapterCall/authPersistCfg went
unrewritten too, so an instance's password reset wrote the credential into
the base app's config. And dockerAppRunTool wants app<Ucfirst><Pascal>, which
no rule produced, so every tool on every instance was unreachable. The infix
rename runs before the suffix rename: the reverse order appends the id half
twice (appSetupComposeTags_nextcloud_work_work).
Verified on a live install: the upgrade ladder now plans mattermost_teest
11.9 -> 11.10, and `regen arrays --force` indexes the instance hooks.
Also carries in-flight instance-removal regen work from a concurrent session
on the same worktree (_lpRegenOrphanedApp, instanceRemove's WebUI refresh).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two discoveries from exercising the capture path live:
- docker exec prints its 'executable file not found' OCI error to STDOUT
(a docker quirk), i.e. into the tar pipe — so the exit code (126/127)
is the only trustworthy no-tar signal, and the host-side 'not a tar
archive' noise is a symptom, not the cause. Detect on the code and say
plainly that the image has no tar.
- The two pipe halves shared one stderr file through an O_TRUNC fd and
an O_APPEND fd, racing and overwriting each other's lines — one file
per half, concatenated after.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The /tasks page's right-side tick + dynamic Select all / Clear All ⇄
Delete Selected layout now covers the other three management surfaces:
- App detail → Tasks tab: filter bar gains the Clear All button and
master tick; Clear All there scopes to that app's tasks only. The
selection set is resolved through window.tasksManager everywhere —
TasksManager is constructed in several places, and ticks previously
landed on one instance while Delete Selected read another's empty set.
- Apps overview → Updates: the header's Update all button now morphs to
Update Selected (N) + Clear in place as rows are ticked, replacing the
separate selection bar between toolbar and list.
- App detail → Backups: each snapshot row gains Delete + a right-side
tick; a toolbar atop the list morphs Delete All ⇄ Delete Selected (N).
The whole selection rides in ONE task (delete <app> 1:a,2:b,…) since
the backup surfaces hold one task per subject at a time.
- CLI: backup app delete accepts comma-separated <idx>:<snap> pairs, and
both delete and delete_all now regenerate the WebUI backup JSON so
deleted snapshots leave the screen instead of lingering until the next
backup.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Matrix was uninstalled and the Updates tab kept listing it as up to
date. Not an instance problem — updates.json and cves.json are
scan-time snapshots on a 30-minute cadence, and nothing rewrote them at
uninstall, so any removed app haunted every updater surface until the
next scan happened to run. The backend was never wrong: the DB, the
apps data and the app's own page all said uninstalled within seconds.
Fixed at both ends. Uninstall now deletes the app's rows from both
generated files, surgically — a full rescan re-runs CVE checks against
every image and has no place inside an uninstall. And the updater's
merge drops any row whose app window.apps does not list as installed,
which covers every other way the snapshot can go stale (a crashed
uninstall, a hand-edited file, the next bug). The filter only applies
when the installed list has actually loaded, preserving the page's
degrade-gracefully contract when it has not.
The stale Matrix rows on this install were purged the same surgical
way; the tab now shows 14 rows with the merge still intact.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Prometheus kept being found stopped after boots, always Exited(0),
always alone. The journal settles it: both stops sit seconds before a
host shutdown boundary — container stopped 05:45:22, boot ended
05:45:30; stopped 04:41:59, boot ended 04:42:05. This is a laptop-class
host that gets shut down, and under ROOTLESS docker the containers are
ordinary processes in the user session, torn down by systemd in
parallel with dockerd's own exit.
That parallelism is the race. An app that handles SIGTERM promptly
exits while dockerd is still alive to record "stopped" — and
unless-stopped then means what it says: not restarted at the next
boot. Apps that exit slower, or die only when dockerd does, are
recorded as running and come back. Prometheus loses reliably because it
is the best-behaved process on the box ("See you next time!"), but
which app loses is a scheduling accident — changing Prometheus's
restart policy would treat the sample, not the race.
So an @reboot crontab entry now waits for the rootless daemon (up to
five minutes, then gives up rather than hang) and `compose up -d`s
every installed app via the existing dockerComposeUpAllApps. Idempotent:
running apps see no diff, stopped ones start, ordering is compose's
problem. Registered through crontabRefresh like the other entries, and
installed on this box.
The accepted trade, stated rather than hidden: an app deliberately
stopped before a reboot comes back after it. On a self-hosting box "the
fleet is up after boot" is the promise unless-stopped was already trying
to make; a stop that must survive reboots is what uninstall is for.
Verified by direct execution: daemon answered immediately, all
installed apps reconciled, running containers untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- backup_files.sh / backup_db.sh: every docker exec/run in the capture,
sidecar-discovery, rehydrate and DB-import paths now goes through
runFileOp — bare docker can't reach the rootless daemon socket, which
made live capture fail (and silently bounce containers) on every
rootless install, and would have broken DB restores the same way.
- capture/rehydrate stderr is kept and printed on failure instead of
being discarded, with a clear message when the image has no tar.
- backup_app_start.sh: when no location produced a complete snapshot the
backup now returns 1 — the task is marked failed instead of logging a
nonexistent/incomplete backup as a success and skipping verification.
- restic engine: on restic exit 3 the orphan incomplete snapshot is
called out explicitly so nobody restores it believing it is whole.
- speedtest: capture /config through the container (root-owned TLS key
and logrotate state are unreadable from the host), which also flips
its auto strategy to live — no more container stop per backup.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
librelad