Harden user_manage, update_system, and the Zimbra pair
user_manage.sh ran useradd/userdel/usermod with no privilege check at
all, so an ordinary user got "Failed to create user." with no hint that
root was the missing piece. Mutating subcommands now require root while
listusers/listgroups stay open, and the check runs *after* the
subcommand is recognised so a bare invocation still prints usage instead
of complaining about privileges. Account names are validated before
reaching useradd, and `deluser` -- which removes the home directory
irrecoverably -- prints what it will delete and confirms first.
zimbra_backup.sh reported success on failed backups. getRestURL can
write an HTTP error body and still exit zero, so a "✅ Backup completed"
could sit over a file containing an error page. Size and gzip -t checks
now gate that, and a suspect file is renamed .suspect rather than
deleted, so it can be looked at. zimbra_restore.sh likewise validates
the archive before starting a restore from it.
Both Zimbra scripts had `cmd` followed by `if [ $? -eq 0 ]`. Adding
set -e to those would have made the error branches unreachable -- set -e
exits before the check -- so the tests are inline instead. That would
have been a silent regression rather than a visible one.
update_system.sh gained strict mode, and a note on the pacman branch:
Arch has no supported partial-upgrade path, and --noconfirm answers away
the prompts that would otherwise warn.
Integrity checks verified against an error page, a truncated archive, a
non-gzip file and a real one.
This commit is contained in:
+25
-6
@@ -14,6 +14,8 @@
|
||||
# You should have received a copy of the GNU General Public License along
|
||||
# with this program. If not, see <https://www.gnu.org/licenses/>.
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
# Ensure script is run as root or sudo
|
||||
if [ "$EUID" -ne 0 ]; then
|
||||
echo "❌ This script must be run as root or with sudo."
|
||||
@@ -63,14 +65,31 @@ echo "📦 Starting backup..."
|
||||
# a zmmailbox failure), but it means the file lands root-owned inside a
|
||||
# directory chowned to zimbra, so ownership is handed over below.
|
||||
# shellcheck disable=SC2024
|
||||
sudo -u zimbra bash -c '/opt/zimbra/bin/zmmailbox -z -m "$1" getRestURL "//?fmt=tgz"' _ "$EMAIL" > "$BACKUP_FILE"
|
||||
|
||||
# Verify success
|
||||
if [ $? -eq 0 ]; then
|
||||
# Tested inline rather than via `$?` on the next line: under set -e a
|
||||
# failure would exit before any check ran, making the error branch below
|
||||
# dead code.
|
||||
if sudo -u zimbra bash -c '/opt/zimbra/bin/zmmailbox -z -m "$1" getRestURL "//?fmt=tgz"' _ "$EMAIL" > "$BACKUP_FILE"; then
|
||||
# A zero exit is not sufficient: getRestURL can write an HTTP error
|
||||
# body and still succeed, which previously produced a cheerful "✅"
|
||||
# over a file containing an error page. Require a plausible size and
|
||||
# a readable gzip container.
|
||||
size="$(stat -c %s -- "$BACKUP_FILE" 2>/dev/null || echo 0)"
|
||||
if [ "$size" -lt 1024 ]; then
|
||||
echo "❌ Backup is only ${size} bytes -- almost certainly an error response, not a mailbox."
|
||||
echo " Leaving it at ${BACKUP_FILE}.suspect for inspection."
|
||||
mv -- "$BACKUP_FILE" "${BACKUP_FILE}.suspect"
|
||||
exit 1
|
||||
fi
|
||||
if ! gzip -t -- "$BACKUP_FILE" 2>/dev/null; then
|
||||
echo "❌ Backup is not a valid gzip stream -- treating as failed."
|
||||
echo " Leaving it at ${BACKUP_FILE}.suspect for inspection."
|
||||
mv -- "$BACKUP_FILE" "${BACKUP_FILE}.suspect"
|
||||
exit 1
|
||||
fi
|
||||
chown zimbra:zimbra "$BACKUP_FILE" 2>/dev/null || true
|
||||
echo "✅ Backup completed: $BACKUP_FILE"
|
||||
echo "✅ Backup completed: $BACKUP_FILE ($(numfmt --to=iec "$size" 2>/dev/null || echo "$size bytes"))"
|
||||
else
|
||||
echo "❌ Backup failed. Check if the user exists or zmmailbox is working."
|
||||
rm -f "$BACKUP_FILE"
|
||||
rm -f -- "$BACKUP_FILE"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
Reference in New Issue
Block a user