Please, peer-review my shell script for restic backups

Getting Help
1

Hi

please, peer-review my shell script for restic backups

#!/bin/bash
set -e

# --- Check if required network volumes are mounted ---
REQUIRED_MOUNTS=("/Volumes/docs" "/Volumes/siri")

for MOUNT_POINT in "${REQUIRED_MOUNTS[@]}"; do
    if ! mount | grep -q "on ${MOUNT_POINT} "; then
        echo "ERROR: ${MOUNT_POINT} is not mounted. Please connect and try again."
        exit 1
    fi
done

echo "All required volumes are mounted. Proceeding with backup..."

# Set Restic repository and SSH user
REPO="sftp:john@192.168.x.x:/mnt/restic/restic-dst/restic-repo"

PASSWORD=$(cat "$HOME/.restic")
export RESTIC_PASSWORD="$PASSWORD"

# Define the directories to backup
DIRS=(
    "/Volumes/docs/docs"
    "/Volumes/siri/EMP"
    "$HOME/Pictures/Photos Library.photoslibrary"
    "$HOME/Music"
    "$HOME/Library/Thunderbird/Profiles/"
    "$HOME/Documents/git/personal"
)

# Create a timestamp for the log file
TIMESTAMP=$(date +"%Y%m%d%H%M%S")
LOG_FILE="backup_log_${TIMESTAMP}.log"

# Perform backup and redirect stderr to the log file
restic -r "$REPO" --verbose backup "${DIRS[@]}" --skip-if-unchanged 2>"$LOG_FILE" || {
    echo "Backup failed. Check $LOG_FILE for details."
    exit 1
}

# List all snapshots and redirect stderr to the log file
restic -r "$REPO" snapshots 2>>"$LOG_FILE" || {
    echo "Failed to list snapshots. Check $LOG_FILE for details."
    exit 1
}

# Remove any existing locks and redirect stderr to the log file
restic -r "$REPO" unlock --remove-all 2>>"$LOG_FILE" || {
    echo "Failed to remove locks. Check $LOG_FILE for details."
    exit 1
}

# Define the backup policy to only keep the last 3 backups and redirect stderr to the log file
restic -r "$REPO" forget --keep-last 3 --prune 2>>"$LOG_FILE" || {
    echo "Failed to define backup policy. Check $LOG_FILE for details."
    exit 1
}
2

One thing I would change is the locking part: don’t run restic unlock --remove-all as a normal step after every backup. It is meant for stale locks and can remove a real lock if another restic process is active. Also consider using RESTIC_PASSWORD_FILE instead of exporting the password in the shell, and log stdout as well as stderr if you want to review what was actually backed up.

1 Like
3

A few things I’d change before trusting it unattended. First, don’t run restic unlock --remove-all on every normal backup; only use that after you have checked there is no other restic process still running, otherwise you can remove a valid lock. Second, use a password file or keychain/env file instead of putting the password in the script, and make the log path absolute so cron/launchd doesn’t write it somewhere unexpected. I’d also add set -euo pipefail and run restic check on a slower schedule than every backup/prune run. The mount check is a good idea.

1 Like
4

thanks both

1 Like