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.

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.

4

thanks both

5

  1. Is the variable “mount” set? It might just be that the whole script was not pasted or some other misunderstanding on my part.

  2. This may be a style difference but I prefer to capture the return code and print it if it is non-zero so that the return code can be compared in the documentation to provide more information about what error occurred. If the program does not, or is not able to write an error message to the log you will be without any information.