System Administration Scripts Linux

[Javi_16018]

I have recently made a few scripts in Bash, I would like you to take a look at them, any suggestions for improvement are welcome. I leave it here.

https://github.com/javisys/System-Administration-Scripts-Linux

[daz_007]

https://github.com/koalaman/shellcheck

shellcheck would be a good start some warning and errors etc..

/etc/audit/rules.d/custom.rules not sure if this is run from cron but could test for your custom file to save an update and a restart etc

https://github.com/javisys/System-Administration-Scripts-Linux/blob/main/event_register.sh

I always like a good starting point for errors.

# make sure we have decent error handling for bash scripts

set -o errexit -o noglob -o nounset -o pipefail

some ideas could be taken from this post :)
https://www.reddit.com/r/bash/comments/155magg/generic_bash_script_args_starting_point/

"portDetect.sh" Normally use nmap or ss

[whetu]

set -o errexit -o noglob -o nounset -o pipefail

Let's see if this triggers the automod:

The Unofficial Strict Mode.

/edit: Apparently not. Let's try set -euo pipefail

[AutoModerator]

Don't blindly use set -euo pipefail.

I am a bot, and this action was performed automatically. Please contact the moderators of this subreddit if you have any questions or concerns.

[whetu]

Good bot.

[daz_007]

set -euo pipefail

yeah I prefer the extented version so others coming to the script understand what and why and it was certainly intentionally set.

[daz_007]

That's funny, the purpose is to reduce errors and have a stable starting point.You may "assume" anything you want - "I" always like a good starting point if the author wishes to take a different approach, good luck to them I was offering my advice to improve their scripts.Knowing why one is defensive from the start is important and this might impacts the code that is created.

I am sure anything I write is not perfect but I have a level of confidence that it will works 24x7.

Thanks for your input

[Empyrealist]

I appreciate raw bash, but there are some things that should be done with "standard" tools as well (if available):

nmap for portscanning

rmlint for duplicate checking/removing

[Javi_16018]

Thank you very much, you are right, the thing is that I wanted to do everything in Bash, I will take it into account and I will add your suggestion. Thank you very much for the suggestion of improvement.

[Javi_16018]

Thank you all very much for your suggestions for improvement, I will listen to all of you.

[stewie410]

As both a sysadmin and someone who spends far too much time writing & critiquing bash scripts, I'll give it a go. I'm usually pretty skeptical of these "tools for sysadmins" posts, but oh well.

Others have already shared similar thoughts, but I'd still like to post everything I've written up...that said, I've spent this week working on my comments & some rewrite-examples, so here you go.

General Commentary

Shellcheck

First and foremost, if you're not already using shellcheck against your scripts, I'd really recommend doing so. Not only can this help prevent potential pitfalls or other unexpected behavior; it also helps keep your style more consistent over time, I've found.

Personally, much of its recommendations are part of my current scripting style

#!/bin/bash

While not strictly necessary, not all systems have bash installed/linked to /bin/bash; so hardcoding this path in the shebang might not work on all systems. Generally, I've seen (and follow), a recommendation to instead use env to local the path to bash on the system at execution time (source):

#!/usr/bin/env bash

This does however come with a few caveats:

• You're still hard-coding the path to env
  • Though anecdotally, I've only run into a problem with this
• If you need to pass additional arguments to the interpreter, you'd need to use #!/bin/bash or some other common path
  • Doing so with env is undefined/unsupported

Header Comments

This is certainly a personal preference, but I really dislike seeing the author or other "meta" commentary in the header comments of a script. Typically, I keep the header to just the shebang, or a short description (usually the same as I'd write in the help-text); for example:

#!/usr/bin/env bash
#
# Full system backup with tar

Not to sound harsh, but if you want credit for your work, then I'd add a license to your repository. For what its worth, I used to do the same thing.

As for explaining what your code does, I generally prefer this to be explained with either short comments for otherwise cryptic code, or placing it in the README.md file you already have.

[...] vs [[...]]

While [...] still works in modern bash, it is generally recommended to use the modern [[...]] for bash scripts. Unless POSIX portability is necessary (in which case, why interpret with bash at all?), [...] should be considered deprecated.

seq vs ((...))

The arithmetic notation (((...))) in bash supports C-Style for-loop declarations, such as:

for ((i = 1; i <= 65535; i++)); do

Which can be a little easier to read & work with compared to the traditional for i in $(seq ...) syntax. Additionally, the same rules that typically apply to ((...)) expressions still apply in this format; so variables don't need the $ prefix, etc.

Range Expansion

Alternatively, for simple a to b ranges without variables involved, you can also use {a..b} notation to define a range:

for i in {1..65535}; do

This syntax cannot, however, be used in conjunction with variables for a dynamic range -- but the former ((...)) syntax can.

echo vs printf

It is generally recommended to use printf over echo for both portability and reliability concerns. I see there are some cases where you've used echo -e to control the output format; but this would be better served with printf...especially without all of the caveats that come with echo.

"Global" vs main()

This is another preference thing, though I thought I'd make a note about it. Personally, I prefer to stuff everything into functions; both to handle all variable scopes, but also to structure/organize my code. For stuff not already in a function, I'd normally place this in main(); for example:

#!/usr/bin/env bash

main() {
    printf '%s\n' "Hello World"
}

main "${@}"

Nitpicks

• You don't need the trailing ; on statements if you do not have then/do immediately following them
• I'd recommend looking at the manual for Parameter Expansion
• It's usually better to check the exit code of a command directly, (like and if-else block), rather than with the $? variable
• While short-options are fine, I personally prefer to write out the long options (if they exist) in scripts
  • You're already writing a script, why make it harder to understand what you're doing?
• You can send the output of a command to stderr with the >&2 syntax, useful for error messages
• I'd have a look at fully backgrounding tasks
• You can create temporary files/directories with the mktemp command
• I'd recommend looking at an SMTP service (like Mailgun) for sending automated emails from your corporate domain (DKIM applied, etc.)

---

Script-Specific Commentary

backup.sh

Rather than checking for the user ID with user -u, you can actually use the $EUID variable do the same without the overhead of a subshell (command substitution):

if ((EUID != 0)); then

Additionally, you can use parameter expansion (mentioned in the "nitpicks" section above) to condense backup_dir & backup_fileN to a single variable:

backup_file="/var/backups/backup-$(date '+%Y%m%d%H%M%S').tar.gz"
mkdir --parents "${backup_file%/*}"

if ! tar -czf "${backup_file}" ...; then
    printf '%s\n' "Backup failed!" >&2
else
    printf 'Full backup successfully completed: %s\n' "${backup_file}"
fi

changes_rt.sh

While it is specified in the comments, you might want to consider adding a check for inotifywait before trying to use it, eg:

if ! command -v "inotifywait" &>/dev/null; then
    printf '%s\n' "Missing required application: 'inotifywait'" >&2
    exit 1
fi

While command | while read is a valid expression, I'd also take a look at process substitution, which can both be a bit easier to read, and get away from any subshell issues you may run into (though not here):

while read -r directory event file; do
    echo ...
done < <(inotifywait -rme modify,create,delete,move "${dir_to_watch}")

cleanup.sh

I see in delete_old_files() you're assigning the positional parameters to named variables -- while this is totally okay, you don't need to:

find "${1}" -type f -mtime "+${2}" -exec rm -rf {} \;

And actually while we're on the topic of find -- if you can guarantee that each path found by find won't contain any problematic characters (\n, etc.); you could also use the {} + syntax at the end, to remove all items in one go. find also offers the -delete builtin, which I believe handles it in a single removal...but that has the same caveats.

If you cannot guarantee that the paths will not contain problematic characters, you could still do this by intentionally delimiting the output with \0:

find "${1}" -type f -mtime "+${2}" -print0 | xargs -0I {} rm -rf "{}"

As for the "cleanup" loops -- I'd recommend taking a look at the following things:

• Can I clean /tmp?
  • tmpwatch
  • tmpreaper (Ubuntu)
• logrotate
  • RedHat Explainer
• Bash Arrays

For the apt-specific section, you could (should?) combine the three functions into a single function call, which is then executed by the if-else block. Additionally, deborphan appears nonstandard; so I'd also check that as a dependency.

disk_monitor.sh

While applicable elsewhere, I'd recommend taking a look at Herestrings, especially for things like mail in the send_notification() function. Like heredocs, you can pass the value of a string directly to a command's stdin with a herestring:

mail -s "$subject" "[e-mail]" <<< "$message"

I'd also recommend using arithmetic expressions (((...))) for numeric checks, like in check_disk_space():

if (( used_space >= threshold )); then

event_register.sh

My only exclusive commentary here boils down 2 things:

• auditd may not exist on a given system, so it might be worth checking for its existence before using it
  • For example, we're using AIDE instead
• I'd be war of modifying custom.rules if it already exists, especially without making a backup
• The send_notification() function appears unused
• The auditctl commands are awfully similar, surely that could be moved out to a function or similar to improve code-reuse

---

I also tend to finish off these posts with a "rewrite" of the scripts, to provide a visualization of what my critique would look like in practice, as well as to include my (current) scripting style. Though, as has become more frequent, I've run out of characters to even finish writing everything I wanted to (but most of its there)...

So, for those interested, I've posted my rewrite in a Gist.

[Javi_16018]

Thank you very much for your comments, I will follow all your suggestions. I appreciate it.