Fix snapper /home config creation on chroot installations#5419
Fix snapper /home config creation on chroot installations#5419kuro-toji wants to merge 2 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
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent silent snapper misconfiguration after chroot installations (reported to cause unmanaged snapshot subvolumes and disk exhaustion) by adding scripts intended to ensure snapper configs exist, and also adds a /boot permission hardening fix.
Changes:
- Add a migration to create a snapper
homeconfig (and ensurerootexists) for affected systems. - Add an installer script intended to ensure snapper
/homeconfig creation during install. - Add a migration + installer script to tighten
/bootand random-seed permissions.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
| migrations/1777007501.sh | Migration attempting to create missing snapper configs for /home and /. |
| install/config/snapper-home-config.sh | New installer-time script intended to ensure snapper configs exist (especially for chroot installs). |
| migrations/1777007500.sh | Migration tightening /boot and random-seed permissions. |
| install/config/boot-permissions-fix.sh | New installer-time script intended to apply /boot permission hardening. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Check if /home is on btrfs and has .snapshots | ||
| if [[ -d /home/.snapshots ]] || mountpoint -q /home 2>/dev/null; then |
There was a problem hiding this comment.
The /home gating condition here (mountpoint or existing /home/.snapshots dir) can still skip the chroot scenario where /home is a btrfs subvolume but not mounted separately and has no .snapshots yet. This means the migration may not create the config even when it should.
| # Check if /home is on btrfs and has .snapshots | |
| if [[ -d /home/.snapshots ]] || mountpoint -q /home 2>/dev/null; then | |
| home_has_snapper_target() { | |
| [[ -d /home/.snapshots ]] && return 0 | |
| mountpoint -q /home 2>/dev/null && return 0 | |
| if command -v btrfs >/dev/null 2>&1; then | |
| btrfs subvolume show /home >/dev/null 2>&1 && return 0 | |
| fi | |
| return 1 | |
| } | |
| # Check if /home is already initialized for snapper or is a btrfs subvolume | |
| if home_has_snapper_target; then |
| sudo snapper -c home create-config /home 2>/dev/null || echo "Warning: Could not create /home snapper config" | ||
|
|
||
| # Copy default config | ||
| if [[ -f /etc/snapper/configs/root ]]; then | ||
| sudo cp /etc/snapper/configs/root /etc/snapper/configs/home 2>/dev/null || true | ||
| # Modify for /home - don't create timeline snapshots | ||
| sudo sed -i 's|SUBVOLUME="/"|SUBVOLUME="/home"|' /etc/snapper/configs/home 2>/dev/null || true | ||
| sudo sed -i 's|TIMELINE_CREATE="yes"|TIMELINE_CREATE="no"|' /etc/snapper/configs/home 2>/dev/null || true | ||
| fi | ||
|
|
||
| echo "✓ Created snapper /home config" |
There was a problem hiding this comment.
The script prints "✓ Created snapper /home config" unconditionally, even if snapper -c home create-config fails (it only echoes a warning but doesn’t change control flow). This can mislead users and hides the fact that the fix didn’t apply.
| sudo snapper -c home create-config /home 2>/dev/null || echo "Warning: Could not create /home snapper config" | |
| # Copy default config | |
| if [[ -f /etc/snapper/configs/root ]]; then | |
| sudo cp /etc/snapper/configs/root /etc/snapper/configs/home 2>/dev/null || true | |
| # Modify for /home - don't create timeline snapshots | |
| sudo sed -i 's|SUBVOLUME="/"|SUBVOLUME="/home"|' /etc/snapper/configs/home 2>/dev/null || true | |
| sudo sed -i 's|TIMELINE_CREATE="yes"|TIMELINE_CREATE="no"|' /etc/snapper/configs/home 2>/dev/null || true | |
| fi | |
| echo "✓ Created snapper /home config" | |
| if sudo snapper -c home create-config /home 2>/dev/null; then | |
| # Copy default config | |
| if [[ -f /etc/snapper/configs/root ]]; then | |
| sudo cp /etc/snapper/configs/root /etc/snapper/configs/home 2>/dev/null || true | |
| # Modify for /home - don't create timeline snapshots | |
| sudo sed -i 's|SUBVOLUME="/"|SUBVOLUME="/home"|' /etc/snapper/configs/home 2>/dev/null || true | |
| sudo sed -i 's|TIMELINE_CREATE="yes"|TIMELINE_CREATE="no"|' /etc/snapper/configs/home 2>/dev/null || true | |
| fi | |
| echo "✓ Created snapper /home config" | |
| else | |
| echo "Warning: Could not create /home snapper config" | |
| fi |
| 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.
Using notify-send in a migration is likely to fail in non-graphical contexts (e.g., running updates from TTY/SSH or without a notification daemon). This can add noisy errors or cause confusion; consider removing it or gating it behind a DISPLAY/DBUS session check and ignoring failures.
| 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" >/dev/null 2>&1 || true | |
| fi |
| #!/bin/bash | ||
|
|
||
| # Fix /boot permissions security issue | ||
| # The random seed file and /boot mount should not be world accessible | ||
| # See: https://github.com/basecamp/omarchy/issues/5377 | ||
|
|
||
| echo "Fixing /boot permissions for better security..." |
There was a problem hiding this comment.
This installer script is not referenced by the install pipeline (it is not called from install/config/all.sh, and no other scripts source/execute it), so it won’t actually run during installation.
| # 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.
The fstab check is incorrect: /etc/fstab lines typically start with the device/spec, not the mountpoint, so grep -q "^/boot" will usually miss an existing /boot entry and print a false warning. Also, the comment says “Add to fstab…” but the code only warns and doesn’t add anything.
| # Add to fstab if not already present with correct options | |
| if ! grep -q "^/boot" /etc/fstab 2>/dev/null; then | |
| # Warn if /boot is not present in /etc/fstab; this script does not modify fstab | |
| if ! awk '!/^[[:space:]]*#/ && NF >= 2 && $2 == "/boot" { found=1; exit } END { exit !found }' /etc/fstab 2>/dev/null; then |
| #!/bin/bash | ||
|
|
||
| # Fix snapper /home config creation for chroot installations | ||
| # See: https://github.com/basecamp/omarchy/issues/5344 | ||
|
|
||
| echo "Ensuring snapper /home config is created..." |
There was a problem hiding this comment.
This installer script is not referenced by the install pipeline (e.g., it is not called from install/config/all.sh, and no other scripts source/execute it), so it won’t run during installation and won’t fix chroot installs as described.
| # 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 | ||
| echo "Creating snapper config for /home..." | ||
| sudo snapper -c home create-config /home 2>/dev/null || echo "Warning: Could not create /home snapper config" | ||
| 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 | ||
| echo "Creating snapper config for /home subvolume..." | ||
| sudo snapper -c home create-config /home 2>/dev/null || echo "Warning: Could not create /home snapper config" | ||
| fi | ||
| else | ||
| echo "/home is not on a separate subvolume, skipping /home snapper config" |
There was a problem hiding this comment.
The /home detection logic here is likely to miss the chroot case: in chroot installs /home may not be a separate mountpoint yet, and /home/.snapshots won’t exist until after create-config succeeds. Consider detecting whether /home is on btrfs (and preferably a btrfs subvolume) via findmnt/stat/btrfs subvolume show rather than mountpoint/.snapshots existence.
| # 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 | |
| echo "Creating snapper config for /home..." | |
| sudo snapper -c home create-config /home 2>/dev/null || echo "Warning: Could not create /home snapper config" | |
| 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 | |
| echo "Creating snapper config for /home subvolume..." | |
| sudo snapper -c home create-config /home 2>/dev/null || echo "Warning: Could not create /home snapper config" | |
| fi | |
| else | |
| echo "/home is not on a separate subvolume, skipping /home snapper config" | |
| home_uses_btrfs_subvolume() { | |
| local home_fstype | |
| local home_mount_root | |
| home_fstype="$(findmnt -n -o FSTYPE --target /home 2>/dev/null || true)" | |
| [[ "$home_fstype" == "btrfs" ]] || return 1 | |
| # Prefer detecting an actual btrfs subvolume so chroot installs work | |
| # even when /home is not mounted separately yet. | |
| if btrfs subvolume show /home >/dev/null 2>&1; then | |
| return 0 | |
| fi | |
| # Fall back to checking whether /home is the root of its own mount. | |
| home_mount_root="$(stat -c %m /home 2>/dev/null || true)" | |
| [[ "$home_mount_root" == "/home" ]] | |
| } | |
| # Check if /home is backed by btrfs and is either a subvolume or a separate mount. | |
| if home_uses_btrfs_subvolume; then | |
| if ! sudo snapper list-configs 2>/dev/null | grep -q "home"; then | |
| echo "Creating snapper config for /home..." | |
| sudo snapper -c home create-config /home 2>/dev/null || echo "Warning: Could not create /home snapper config" | |
| fi | |
| else | |
| echo "/home is not on a separate btrfs subvolume, skipping /home snapper config" |
| # Also ensure root snapper config exists | ||
| 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 | ||
| fi | ||
|
|
||
| echo "Snapper config check complete!" |
There was a problem hiding this comment.
This script calls snapper unconditionally. Other parts of the repo guard snapper usage with a command presence check; without that, a missing snapper binary will cause this to silently do nothing (stderr is redirected) and still print a successful completion message.
| # Fix snapper /home config for chroot installations | ||
| # See: https://github.com/basecamp/omarchy/issues/5344 | ||
|
|
||
| echo "Fixing snapper /home config..." | ||
|
|
||
| # Check if /home is on btrfs and has .snapshots | ||
| if [[ -d /home/.snapshots ]] || mountpoint -q /home 2>/dev/null; then | ||
| # Check if /home snapper config exists | ||
| if ! sudo snapper list-configs 2>/dev/null | grep -q "^home"; then | ||
| echo "Creating snapper config for /home..." | ||
| sudo snapper -c home create-config /home 2>/dev/null || echo "Warning: Could not create /home snapper config" | ||
|
|
||
| # Copy default config | ||
| if [[ -f /etc/snapper/configs/root ]]; then | ||
| sudo cp /etc/snapper/configs/root /etc/snapper/configs/home 2>/dev/null || true | ||
| # Modify for /home - don't create timeline snapshots | ||
| sudo sed -i 's|SUBVOLUME="/"|SUBVOLUME="/home"|' /etc/snapper/configs/home 2>/dev/null || true | ||
| sudo sed -i 's|TIMELINE_CREATE="yes"|TIMELINE_CREATE="no"|' /etc/snapper/configs/home 2>/dev/null || true | ||
| fi | ||
|
|
||
| echo "✓ Created snapper /home config" | ||
| else | ||
| echo "Snapper /home config already exists" | ||
| fi | ||
| else | ||
| echo "/home is not on btrfs or separate subvolume, skipping" | ||
| fi | ||
|
|
There was a problem hiding this comment.
This introduces (or re-introduces) a /home snapper config, but the current codebase explicitly avoids /home snapshots (install/login/limine-snapper.sh comment) and migration 1776927490 removes the home config and .snapshots subvolume. Because migrations run in filename order, this new migration would effectively undo 1776927490 on the next update.
| # Fix snapper /home config for chroot installations | |
| # See: https://github.com/basecamp/omarchy/issues/5344 | |
| echo "Fixing snapper /home config..." | |
| # Check if /home is on btrfs and has .snapshots | |
| if [[ -d /home/.snapshots ]] || mountpoint -q /home 2>/dev/null; then | |
| # Check if /home snapper config exists | |
| if ! sudo snapper list-configs 2>/dev/null | grep -q "^home"; then | |
| echo "Creating snapper config for /home..." | |
| sudo snapper -c home create-config /home 2>/dev/null || echo "Warning: Could not create /home snapper config" | |
| # Copy default config | |
| if [[ -f /etc/snapper/configs/root ]]; then | |
| sudo cp /etc/snapper/configs/root /etc/snapper/configs/home 2>/dev/null || true | |
| # Modify for /home - don't create timeline snapshots | |
| sudo sed -i 's|SUBVOLUME="/"|SUBVOLUME="/home"|' /etc/snapper/configs/home 2>/dev/null || true | |
| sudo sed -i 's|TIMELINE_CREATE="yes"|TIMELINE_CREATE="no"|' /etc/snapper/configs/home 2>/dev/null || true | |
| fi | |
| echo "✓ Created snapper /home config" | |
| else | |
| echo "Snapper /home config already exists" | |
| fi | |
| else | |
| echo "/home is not on btrfs or separate subvolume, skipping" | |
| fi | |
| # Ensure snapper root config exists for chroot installations | |
| # See: https://github.com/basecamp/omarchy/issues/5344 | |
| echo "Fixing snapper root config..." |
| # Ensure root config exists | ||
| 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 | ||
| fi |
There was a problem hiding this comment.
This migration uses snapper without checking it exists. If snapper isn’t installed (or fails), most errors are redirected to /dev/null and the migration will appear to succeed while not applying the intended fix. Consider guarding with a command presence check (as done in other migrations).
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
1f3f406 to
c305cba
Compare
Summary
On chroot installations, the snapper /home config wasn't being created, leading to silent failures. This caused snapshot subvolumes to grow unbounded and fill up the disk.
Issue
Users reported that after a chroot installation, snapper configs weren't created properly:
Fix
Files Changed
install/config/snapper-home-config.sh- New installer scriptmigrations/1777007501.sh- Migration to fix existing installationsTesting
sudo snapper list-configsFixes: #5344