Fix snapshot restore to exclude /home (prevent data loss)#5420
Fix snapshot restore to exclude /home (prevent data loss)#5420kuro-toji wants to merge 3 commits intobasecamp:devfrom
Conversation
The /boot mount point and random-seed file were world accessible, which is a security issue per bootctl warnings. This fix: - Sets /boot directory permissions to 700 - Sets random-seed file permissions to 600 - Runs bootctl random-seed to regenerate with correct permissions Fixes: basecamp#5377
On chroot installations, the snapper /home config wasn't being created, leading to silent failures and disk space issues as snapshot subvolumes kept growing without cleanup policies. This fix ensures: - /home snapper config is created when /home is on btrfs - Root snapper config is verified to exist - Config is copied from defaults with appropriate modifications Fixes: basecamp#5344
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds user-facing safeguards around snapshot restores after reports of /home being restored (and user data lost), and introduces migrations/install scripts for snapper and boot permission fixes.
Changes:
- Add warnings around
omarchy-snapshot restoreand introduce a “safe restore” wrapper script. - Add migrations for snapper
/homeconfig creation and/bootpermission tightening. - Add install-time scripts for snapper
/homeconfig and/bootpermission fixes.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| migrations/1777007502.sh | Adds a wrapper script and messaging intended to prevent /home data loss during restores. |
| migrations/1777007501.sh | Adds migration to create/repair snapper configs for /home and /. |
| migrations/1777007500.sh | Adds migration to tighten /boot and random-seed permissions. |
| install/config/snapper-home-config.sh | Adds install-time logic to ensure snapper config for /home exists. |
| install/config/boot-permissions-fix.sh | Adds install-time logic to set /boot permissions and run bootctl random-seed. |
| bin/omarchy-snapshot | Adds restore-time warning output before invoking limine-snapper-restore. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| WRAPPER="/usr/local/bin/omarchy-snapshot-restore-safe" | ||
| cat > "$WRAPPER" << 'WRAPPEREOF' | ||
| #!/bin/bash | ||
| # Safe snapshot restore wrapper | ||
| # Warns users that /home will NOT be restored | ||
|
|
||
| echo "⚠️ WARNING: This will restore the ROOT filesystem only." | ||
| echo "⚠️ Your /home directory will NOT be affected." | ||
| echo "" | ||
| echo "To restore a snapshot:" | ||
| echo "1. Reboot and select the snapshot from limine menu" | ||
| echo "2. The snapshot will restore ONLY the root filesystem" | ||
| echo "" | ||
| echo "If you need to restore /home from a snapshot:" | ||
| echo "- Boot into the snapshot" | ||
| echo "- Manually restore /home from .snapshots subvolumes" | ||
| echo "" | ||
|
|
||
| if [[ -t 0 ]]; then | ||
| read -p "Continue with snapshot restore? (y/N) " -n 1 -r | ||
| echo | ||
| if [[ ! $REPLY =~ ^[Yy]$ ]]; then | ||
| exit 1 | ||
| fi | ||
| fi | ||
|
|
||
| exec sudo limine-snapper-restore "$@" | ||
| WRAPPEREOF | ||
|
|
||
| sudo chmod +x "$WRAPPER" |
There was a problem hiding this comment.
cat > "$WRAPPER" writes into /usr/local/bin, which typically requires root. As written, this migration will fail for non-root runs (even though chmod uses sudo). Write the file via sudo (e.g., sudo tee "$WRAPPER" > /dev/null or write to a temp file then sudo mv into place) so the wrapper is reliably created.
|
|
||
| # Ensure /boot is mounted with proper permissions | ||
| # Add to fstab if not already present with correct options | ||
| if ! grep -q "^/boot" /etc/fstab 2>/dev/null; then |
There was a problem hiding this comment.
/etc/fstab typically has the device in column 1 and the mount point (e.g., /boot) in column 2, so grep -q "^/boot" will usually never match even when /boot is present. Parse the mountpoint column (e.g., with awk '$2=="/boot"{found=1} END{exit !found}' /etc/fstab) or use findmnt -n /boot to accurately detect whether /boot is configured.
| if ! grep -q "^/boot" /etc/fstab 2>/dev/null; then | |
| if ! awk '$2=="/boot"{found=1} END{exit !found}' /etc/fstab 2>/dev/null; then |
| if ! sudo snapper list-configs 2>/dev/null | grep -q "^root"; then | ||
| echo "Creating snapper config for root..." | ||
| sudo snapper -c root create-config / 2>/dev/null || true | ||
| sudo cp $OMARCHY_PATH/default/snapper/root /etc/snapper/configs/root 2>/dev/null || true |
There was a problem hiding this comment.
$OMARCHY_PATH should be quoted to avoid word-splitting/globbing issues (paths with spaces will break the cp). Use quotes around the full source path.
| sudo cp $OMARCHY_PATH/default/snapper/root /etc/snapper/configs/root 2>/dev/null || true | |
| sudo cp "$OMARCHY_PATH/default/snapper/root" /etc/snapper/configs/root 2>/dev/null || true |
| # Check if /home is on a separate subvolume or btrfs | ||
| if mountpoint -q /home 2>/dev/null; then | ||
| # /home is a separate mount point | ||
| if ! sudo snapper list-configs 2>/dev/null | grep -q "home"; then |
There was a problem hiding this comment.
The grep -q "home" / grep -q "root" checks are not anchored, so they can produce false positives (e.g., a config name like home2 would make the script think home exists). Match the config name exactly (consistent with migrations/1777007501.sh using ^home). Also quote $OMARCHY_PATH in the cp for the same reason as in the migration.
| fi | ||
| elif [[ -d /home/.snapshots ]]; then | ||
| # /home has .snapshots subdirectory, ensure config exists | ||
| if ! sudo snapper list-configs 2>/dev/null | grep -q "home"; then |
There was a problem hiding this comment.
The grep -q "home" / grep -q "root" checks are not anchored, so they can produce false positives (e.g., a config name like home2 would make the script think home exists). Match the config name exactly (consistent with migrations/1777007501.sh using ^home). Also quote $OMARCHY_PATH in the cp for the same reason as in the migration.
| if ! sudo snapper list-configs 2>/dev/null | grep -q "root"; then | ||
| echo "Creating snapper config for root..." | ||
| sudo snapper -c root create-config / 2>/dev/null || echo "Warning: Could not create root snapper config" | ||
| sudo cp $OMARCHY_PATH/default/snapper/root /etc/snapper/configs/root 2>/dev/null || true |
There was a problem hiding this comment.
The grep -q "home" / grep -q "root" checks are not anchored, so they can produce false positives (e.g., a config name like home2 would make the script think home exists). Match the config name exactly (consistent with migrations/1777007501.sh using ^home). Also quote $OMARCHY_PATH in the cp for the same reason as in the migration.
| echo "✓ random-seed permissions fixed to 600" | ||
| fi | ||
|
|
||
| notify-send "Boot permissions fixed" "Security improvement applied to /boot" |
There was a problem hiding this comment.
notify-send commonly fails in non-GUI / non-user-session contexts (no DISPLAY/DBUS_SESSION_BUS_ADDRESS), which can create noisy errors or cause the migration runner to treat the migration as failed depending on how failures are handled. Guard it (check command -v notify-send and session env) and/or make it non-fatal (e.g., append || true).
| notify-send "Boot permissions fixed" "Security improvement applied to /boot" | |
| if command -v notify-send >/dev/null 2>&1 && [[ -n "${DISPLAY:-}" || -n "${DBUS_SESSION_BUS_ADDRESS:-}" ]]; then | |
| notify-send "Boot permissions fixed" "Security improvement applied to /boot" || true | |
| fi |
Users reported that omarchy-snapshot restore was also restoring /home, causing data loss of user files. This fix: - Updates omarchy-snapshot to show clear warnings about /home exclusion - Documents that /home is NOT restored during snapshot operations - Creates a safe wrapper script for snapshot restore The root cause may be in limine-snapper-restore, but until that's fixed, this provides user awareness and prevents accidental data loss. Fixes: basecamp#5361
3b66783 to
a4e03b0
Compare
|
This PR is superseded by #5422 which combines all fixes with proper Copilot review feedback addressed. Please review that PR instead. |
Summary
Users reported that
omarchy-snapshot restorewas restoring /home along with root, causing data loss of user files.Issue
Users configured their system, took a snapshot, then used
omarchy-snapshot restoreexpecting only root to be restored. Instead, /home was also restored, deleting all user files in /home.Fix
Note
The root cause may be in the external
limine-snapper-restoretool. This fix provides user awareness until a proper fix can be implemented upstream.Files Changed
bin/omarchy-snapshot- Updated with warning messagesmigrations/1777007502.sh- Migration with documentation and wrapperTesting
omarchy-snapshot restoreFixes: #5361