English original
Shell Script Safety Reviewer
You review shell scripts the way a careful SRE would before letting them run unattended: deploy scripts, cron jobs, CI steps, installers and Docker entrypoints. You find the lines that can delete the wrong data, leak a secret, run untrusted code or fail silently, explain each one in plain words, and hand back a corrected script that still does the same job.
Files in this skill
scripts/shell_lint.py- line-based linter for bash and POSIX sh that understands quotes, comments, heredocs, [[ ]], (( )), assignments and case words; prints a findings table with fixes, or JSON (Python 3 standard library only)references/shell-pitfalls.md- every rule the linter checks: what goes wrong, a minimal failing example and the fixreferences/safe-patterns.md- a strict-mode header, safe temp files and cleanup, safe loops over files, argument parsing, secrets, downloads and loggingtemplates/shell-review.md- the review to hand backexamples/deploy.sh- a realistic deploy script with 20 findings (3 HIGH)examples/deploy-fixed.sh- the same script after review; lints clean and passes ShellCheck
Workflow
1. Get context
Ask what runs the script (CI, cron, systemd, a human), which shell and OS (bash 5 on Ubuntu, sh on Alpine/busybox, macOS bash 3.2), who can control its inputs (arguments, environment, file names, network data) and whether it runs as root. Ask for the whole script, not a fragment: strict mode and traps live at the top.
2. Run the linter
python3 scripts/shell_lint.py examples/deploy.sh
python3 scripts/shell_lint.py deploy.sh entrypoint.sh --min-severity MEDIUM
python3 scripts/shell_lint.py install.sh --json --ignore BACKTICKS
python3 scripts/shell_lint.py --self-test
Exit 1 means at least one HIGH finding, 2 means a file could not be read. Silence a deliberate line with # shell-lint: disable=CODE. If ShellCheck is available, run it too (shellcheck -f gcc script.sh) and merge the results; the linter adds checks ShellCheck does not do (secrets, curl | sh, chmod 777, fixed /tmp names, disabled TLS).
3. Review by hand
The linter is a first pass. Read the script top to bottom and check what a regex cannot (see references/shell-pitfalls.md):
- Which inputs are untrusted, and do they reach rm, eval, ssh, sql or a path?
- Does every destructive step run only after its preconditions (right directory, non-empty variable, confirmed target host)?
- Is it idempotent: can it run twice, or resume after failing halfway?
- Are errors visible: does a failure in a pipe, subshell or function stop the script and leave a clear message?
- Does it clean up temp files, locks and background jobs on exit and on Ctrl+C?
4. Fix and deliver
Fill templates/shell-review.md: summary verdict, findings table (severity, line, problem, fix), the full fixed script, a diff of the important changes, and how to test it safely (dry run, bash -n, a throwaway directory or container, set -x). Keep the script's behavior; call out every behavior change.
Example result
examples/deploy.sh: 20 findings, 3 HIGH. A Slack token is hardcoded (HARDCODED_SECRET, rotate it), an installer is piped from curl into sudo bash (CURL_PIPE_SHELL), and rm -rf $WEB_ROOT/* wipes /* if WEB_ROOT is ever empty (RM_EMPTY_VAR). There is no set -e, so a failed cd $BUILD_DIR would run the rm and cp in the wrong directory; ssh inside the while read host loop eats hosts.txt, so only the first host restarts (SSH_STDIN_LOOP); local count=$(...) hides failures. examples/deploy-fixed.sh fixes all of them and lints clean.
Rules
- Rank by impact: data loss, code execution and secrets first, then silent failures, then style.
- Quote by default; explain the one place where splitting is intended and use an array there.
- Never suggest
set -eas the only fix: also check pipes (pipefail), subshells,local x=$(...)andif cmdcontexts where errexit is off. - Rotate any secret found in a script, even if the repo is private, and say so.
- Prefer the POSIX form when the shebang is /bin/sh; otherwise make the shebang say bash.
- Do not run a reviewed script against real systems to "test" it; use bash -n, a container or a dry-run flag. FILE:references/shell-pitfalls.md
Shell pitfalls checked by shell_lint.py
Severity: HIGH = can delete data, run untrusted code or leak a secret. MEDIUM = wrong result or silent failure under realistic input. LOW = fragile or legacy style.
HIGH
RM_EMPTY_VAR / RM_ROOT
rm -rf $DIR/* becomes rm -rf /* when DIR is empty or unset (typo, missing argument, failed command substitution). Steam's Linux client shipped exactly this bug in 2015.
rm -rf -- "${DIR:?DIR is empty}"/* # aborts with a message if DIR is empty
set -u alone does not help when the variable is set to an empty string.
CURL_PIPE_SHELL
curl https://x/install.sh | bash runs whatever the server (or anyone in the middle) sends, and a dropped connection can run half a script. Download to a file, check a pinned SHA-256 or a signature, read it, then run it.
EVAL_VAR
eval "$cmd" executes any ;, $(...) or backticks inside the data. Build commands as arrays: cmd=(rsync -a -- "$src" "$dst"); "${cmd[@]}". For dispatch, use case.
HARDCODED_SECRET
A literal token, password or key in a script ends up in git history, CI logs (set -x) and ps output. Read it from the environment or a secret manager and fail early: TOKEN="${TOKEN:?}". Rotate the exposed value; deleting the line does not remove it from history.
MEDIUM
NO_STRICT_MODE / NO_PIPEFAIL
Without set -e a failed command is ignored and the next one runs anyway. Without pipefail, build | tee log succeeds when build fails. Use set -euo pipefail in bash and set -eu in POSIX sh. Know the gaps: errexit is ignored inside if, while, &&/|| lists and functions called from them, and local x=$(cmd) (see below).
UNQUOTED_VAR
Unquoted $var and $(cmd) are split on IFS (spaces, tabs, newlines) and each piece is glob-expanded. cp $src $dst with src="My File.txt" copies two wrong files; rm $f with f='*' removes everything. Quote every expansion; use "$@" for all arguments and "${arr[@]}" for arrays. Safe without quotes: right side of x=$y, inside [[ ]] (except the right side of ==/=~ where it becomes a pattern), inside (( )), and case $x in.
CD_UNCHECKED
If cd build fails, the rest runs in the current directory: cd build; rm -rf *. Use cd -- "$dir" || exit 1 or strict mode. In loops, use a subshell: (cd -- "$d" && make).
LS_PARSING
for f in $(ls *.log) breaks on spaces and newlines and runs the glob twice. Use the glob directly: for f in ./*.log; do [ -e "$f" ] || continue; ...; done, or find . -name '*.log' -print0 | while IFS= read -r -d '' f; do ...; done.
CHMOD_777
World-writable files and directories let any local user or compromised service replace code or data. Use 755/644, 700/600 for private data, or u=rwX,g=rX,o=.
PREDICTABLE_TMP / NO_TRAP_CLEANUP (LOW)
A fixed /tmp/app.log can be pre-created as a symlink by another user (you then overwrite their target as root) and collides between parallel runs. Use mktemp/mktemp -d and remove it with trap '...' EXIT.
LOCAL_MASKS_STATUS
local out=$(cmd) returns the status of local (0), so set -e and $? never see cmd fail. The same goes for export and readonly. Declare first, assign on the next line.
SSH_STDIN_LOOP
while read host; do ssh "$host" cmd; done < hosts.txt stops after the first host because ssh reads the rest of stdin. Use ssh -n or < /dev/null. The same applies to ffmpeg, docker exec -i and anything else that reads stdin.
TLS_DISABLED
curl -k, wget --no-check-certificate and StrictHostKeyChecking=no turn off the check that you are talking to the right server. Fix the CA bundle, pin the host key in known_hosts, or use StrictHostKeyChecking=accept-new only for first contact on trusted networks.
BASHISM_IN_SH
Under #!/bin/sh (dash on Debian/Ubuntu, busybox on Alpine) these fail: [[ ]], == in [ ], function f, arrays, source, <<<, &>, $'...'. Switch the shebang to bash or use [ ], =, ., and positional parameters.
LOW
- NO_SHEBANG: the caller's shell runs it; behavior differs between bash, zsh and sh.
- READ_WITHOUT_R:
readwithout-reats backslashes; withoutIFS=it trims leading and trailing spaces. - BACKTICKS: hard to nest and quote; use
$(...).
Not covered by the linter (check by hand)
- Untrusted input reaching sql, ssh or paths (injection,
../traversal); a leading-in file names (use--). - Race conditions on lock files; use
flock. - Missing
waitfor background jobs, and orphaned jobs on exit. - Locale and tool differences (GNU vs BSD sed -i, date -d, readlink -f on macOS).
- Logging secrets with
set -x; turn xtrace off around secret handling. FILE:references/safe-patterns.md
Safe patterns for bash and sh scripts
Header
#!/usr/bin/env bash
# What it does, how to call it, what it needs (env vars, tools).
set -euo pipefail
IFS=$'\n\t' # optional: no splitting on spaces
readonly SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
POSIX sh: set -eu (pipefail exists in dash 0.5.11+ and busybox, but not everywhere).
Fail with a message
die() { printf 'error: %s\n' "$*" >&2; exit 1; }
command -v jq >/dev/null 2>&1 || die "jq is required"
[ -d "$TARGET" ] || die "no such directory: $TARGET"
Arguments
usage() { echo "usage: $0 [-n] <release>" >&2; exit 2; }
dry_run=0
while getopts ":n" opt; do
case $opt in
n) dry_run=1 ;;
*) usage ;;
esac
done
shift $((OPTIND - 1))
release="${1:-}"; [ -n "$release" ] || usage
Temp files and cleanup
tmpdir="$(mktemp -d)"
cleanup() { rm -rf -- "$tmpdir"; }
trap cleanup EXIT
trap 'exit 130' INT TERM # EXIT trap still runs
Destructive steps
target="${WEB_ROOT:?WEB_ROOT is empty}"
case $target in /|/usr|/etc|"$HOME") die "refusing to clean $target" ;; esac
run() { if [ "$dry_run" -eq 1 ]; then echo "+ $*"; else "$@"; fi; }
run rm -rf -- "$target"/*
Loops over files and lines
for f in "$dir"/*.csv; do [ -e "$f" ] || continue; process "$f"; done
find "$dir" -type f -name '*.log' -print0 | while IFS= read -r -d '' f; do gzip -- "$f"; done
while IFS= read -r line || [ -n "$line" ]; do echo "$line"; done < input.txt # last line without newline
Commands as data
args=(--archive --delete -- "$src/" "$dst/")
[ "$dry_run" -eq 1 ] && args=(--dry-run "${args[@]}")
rsync "${args[@]}"
Secrets
- Read from the environment or a file with 600 permissions:
token="$(<"$CRED_FILE")". - Pass to tools via stdin or env, not command-line arguments (visible in
ps):curl -H @- <<<"Authorization: Bearer $token". set +xaround secret handling.
Downloads
curl -fsSL --retry 3 -o "$tmpdir/tool.tgz" "$URL"
echo "$SHA256 $tmpdir/tool.tgz" | sha256sum -c -
-f makes HTTP errors fail instead of saving the error page.
Locking and idempotency
exec 9>/var/lock/deploy.lock
flock -n 9 || die "another deploy is running"
mkdir -p -- "$dir" # no error if it exists
ln -sfn -- "$release_dir" current # atomic switch of a symlink
Logging
log() { printf '%s %s\n' "$(date -u +%FT%TZ)" "$*" >&2; }
Send logs to stderr so stdout stays clean for data.
Testing a script safely
bash -n script.sh(syntax only),shellcheck script.sh,python3 scripts/shell_lint.py script.sh.- Run with a dry-run flag, then in a throwaway container:
docker run --rm -v "$PWD":/w -w /w bash:5 bash script.sh. - Trace once with
bash -x script.sh(secrets off). FILE:templates/shell-review.md