Run wget and other commands in shell script

Question

I'm trying to create a shell script that I will download the latest Atomic gotroot rules to my server, unpack them, copy them to the correct folder, etc.,

I've been reading shell tutorials and forum posts for most of the day and the syntax escapes me for some of these. I have run all these commands and I know they work if I manually run them.

I know I need to develop some error checking, but I'm just trying to get the commands to run correctly. The main problem at the moment is the syntax of the wget commands, i've got errors about missing semi-colons, divide by zero, unsupported schemes - I've tried various quoting (single and double) and escaping - / " characters in various combinations.

Thanks for any help. The raw wget command is

wget --user="jim" --password="xxx-yyy-zzz" "http://updates.atomicorp.com/channels/rules/subscription/VERSION"

#!/bin/sh
update_modsec_rules(){

wget=/usr/bin/wget
tar=/bin/tar
apachectl=/usr/bin/apache2ctl


TXT="Script Run Finished"
WORKING_DIR="/var/asl/updates"
TARGET_DIR="/usr/local/apache/conf/modsec_rules/"
EXISTING_FILES="/var/asl/updates/modsec/*"
EXISTING_ARCH="/var/asl/updates/modsec-*"
WGET_OPTS='--user=jim --password=xxx-yyy-zzz'
URL_BASE="http://updates.atomicorp.com/channels/rules/subscription"


# change to working directory and cleanup any downloaded files and extracted rules in modsec/ directory
cd $WORKING_DIR
rm -f $EXISTING_ARCH
rm -f $EXISTING_FILES
rm -f VERSION*

# wget to download VERSION file
$wget ${WGET_OPTS} "${URL_BASE}/VERSION"

# get current MODSEC_VERSION from VERSION file and save as variable
source VERSION
TARGET_DATE=$MODSEC_VERSION
echo $TARGET_DATE

# wget to download current archive
$wget ${WGET_OPTS} "${URL_BASE}/modsec-${TARGET_DATE}.tar.gz"

# extract archive
echo "extracting files . . . "
tar zxvf $WORKING_DIR/modsec-${TARGET_DATE}.tar.gz

echo "copying files . . . "
cp -uv $EXISTING_FILES $TARGET_DIR

echo $TXT
}

update_modsec_rules $@ 2>&1 | tee -a /var/asl/modsec_update.log

RESTART_APACHE="/usr/local/cpanel/scripts/restartsrv httpd"
$RESTART_APACHE

Answer 1

Here are some guidelines to use when writing shell scripts.

1. Always quote variables when you use them. This helps avoid the possibility of misinterpretation. (What if a filename contains a space?)
2. Don't trust fileglobbing on commands like rm. Use for loops instead. (What if a filename starts with a hyphen?)
3. Avoid subshells when possible. Your lines with backquotes make me itchy.
4. Don't exec if you can help it. And especially don't expect any parts of your script after your exec to actually get run.

I should point out that while your shell may be bash, you've specified /bin/sh for execution of this script, so it is NOT a bash script.

Here's a rewrite with some error checking. Add salt to taste.

#!/bin/sh

# Linux
wget=/usr/bin/wget
tar=/bin/tar
apachectl=/usr/sbin/apache2ctl

# FreeBSD
#wget=/usr/local/bin/wget
#tar=/usr/bin/tar
#apachectl=/usr/local/sbin/apachectl

TXT="GOT TO THE END, YEAH"
WORKING_DIR="/var/asl/updates"
TARGET_DIR="/usr/local/apache/conf/modsec_rules/"
EXISTING_FILES_DIR="/var/asl/updates/modsec/"
EXISTING_ARCH="/var/asl/updates/"

URL_BASE="http://updates.atomicorp.com/channels/rules/subscription"
WGET_OPTS='--user="jim" --password="xxx-yyy-zzz"'

if [ ! -x "$wget" ]; then
  echo "ERROR: No wget." >&2
  exit 1
elif [ ! -x "$apachectl" ]; then
  echo "ERROR: No apachectl." >&2
  exit 1
elif [ ! -x "$tar" ]; then
  echo "ERROR: Not in Kansas anymore, Toto." >&2
  exit 1
fi

# change to working directory and cleanup any downloaded files
# and extracted rules in modsec/ directory
if ! cd "$WORKING_DIR"; then
  echo "ERROR: can't access working directory ($WORKING_DIR)" >&2
  exit 1
fi

# Delete each file in a loop.
for file in "$EXISTING_FILES_DIR"/* "$EXISTING_ARCH_DIR"/modsec-*; do
  rm -f "$file"
done

# Move old VERSION out of the way.
mv VERSION VERSION-$$

# wget1 to download VERSION file (replaces WGET1)
if ! $wget $WGET_OPTS $URL_BASE}/VERSION; then
  echo "ERROR: can't get VERSION" >&2
  mv VERSION-$$ VERSION
  exit 1
fi

# get current MODSEC_VERSION from VERSION file and save as variable,
# but DON'T blindly trust and run scripts from an external source.
if grep -q '^MODSEC_VERSION=' VERSION; then
  TARGET_DATE="`sed -ne '/^MODSEC_VERSION=/{s/^[^=]*=//p;q;}' VERSION`"
  echo "Target date: $TARGET_DATE"
fi

# Download current archive (replaces WGET2)
if ! $wget ${WGET_OPTS} "${URL_BASE}/modsec-$TARGET_DATE.tar.gz"; then
  echo "ERROR: can't get archive" >&2
  mv VERSION-$$ VERSION         # Do this, don't do this, I don't know your needs.
  exit 1
fi

# extract archive
if [ ! -f "$WORKING_DIR/modsec-${TARGET_DATE}.tar.gz" ]; then
  echo "ERROR: I'm confused, where's my archive?" >&2
  mv VERSION-$$ VERSION         # Do this, don't do this, I don't know your needs.
  exit 1
fi
tar zxvf "$WORKING_DIR/modsec-${TARGET_DATE}.tar.gz"

for file in "$EXISTING_FILES_DIR"/*; do
  cp "$file" "$TARGET_DIR/"
done

# So far so good, so let's restart apache.
if $apachectl configtest; then
  if $apachectl restart; then
    # Success!
    rm -f VERSION-$$
    echo "$TXT"
  else
    echo "ERROR: PANIC! Apache didn't restart.  Notify the authorities!" >&2
    exit 3
  fi
else
  echo "ERROR: Apache configs are broken.  We're still running, but you'd better fix this ASAP." >&2
  exit 2
fi

Note that while I've rewritten this to be more sensible, there is certainly still a lot of room for improvement.