Security review checklist for generated shell scripts
Shell has no sandbox, no type system and no undo. Every variable is a potential injection point and every script runs with your full privileges.
Shell's security model is that there isn't one. A script runs with everything its invoker has, every expansion is textual substitution before execution, and there is nothing between a mistake and the filesystem.
Generated shell is written in the style of the shell in its training data — tutorials and READMEs, none of which was expected to run unattended, all of which omits guards for brevity. That is a bad starting point for anything running in CI or as a cron job.
First: shellcheck, then the header#
shellcheck -S warning scripts/*.shshellcheck catches most of the injection-adjacent bugs (quoting, word splitting, unsafe globbing) with very few false positives. Everything below is what it cannot see.
#!/usr/bin/env bash
set -Eeuo pipefail
IFS=$'\n\t'
trap 'echo "failed at line $LINENO: $BASH_COMMAND" >&2' ERRset -u is a security control, not a style choice — it is the difference between rm -rf "$DIR/" deleting a build directory and deleting from the filesystem root.
Injection#
1. Unquoted expansion is code injection#
grep $pattern file.txt # pattern="-r /" now recurses from root
rm $file # file="a b" removes two files
tar -xf $archive # archive="--checkpoint-action=exec=sh evil.sh"Word splitting means an attacker who controls a variable controls your argument list. The tar example is real: certain options execute commands, and passing an unquoted variable lets someone inject them.
Correct: quote every expansion and use -- before anything user-controlled.
grep -- "$pattern" file.txt
rm -- "$file"-- marks the end of options, so a value starting with - is treated as data. Three characters, and generated scripts almost never include them.
2. eval on anything interpolated#
eval "config_$key=$value" # key or value contains ; rm -rf ~There is no safe way to do this. Use an associative array:
declare -A config
config["$key"]="$value"3. Command substitution in a string that becomes a command#
cmd="ssh $host 'tail -n $lines /var/log/app.log'"
$cmd # re-splits, re-globs, and executesBuild an array instead, which never re-splits:
ssh_args=(ssh -- "$host" "tail -n ${lines} /var/log/app.log")
"${ssh_args[@]}"4. Data interpolated into another interpreter#
psql -c "SELECT * FROM users WHERE email = '$email'" # SQL injection, via shell
curl -d "{\"name\": \"$name\"}" "$url" # broken JSON, or worseShell scripts are a common and overlooked route into SQL and JSON injection. Use the tool's own parameterisation, and build JSON with jq:
jq -n --arg name "$name" '{name: $name}' | curl -d @- "$url"jq -n --arg escapes correctly. String-concatenating JSON in shell is always wrong and generated scripts do it constantly.
The environment is attacker-controlled#
5. PATH hijacking#
#!/bin/bash
curl -sS "$url" | tar -xz # which curl? which tar?If your script runs setuid, from cron with an odd environment, or in CI with a writable directory early in PATH, "which binary" is not a question you control.
export PATH=/usr/local/bin:/usr/bin:/bin
readonly PATHSet it explicitly at the top of any privileged script. For the highest-value calls, use absolute paths.
6. IFS#
IFS=$'\n\t' # in the standard header for a reasonAn inherited IFS changes how every unquoted expansion splits. Setting it explicitly removes a class of attack that depends on your caller's environment.
7. Inherited environment into a child#
sudo without -i, or exec passing the current environment, forwards LD_PRELOAD, PYTHONPATH, NODE_OPTIONS and everything else. For anything privileged, use env -i and pass only what you need:
env -i PATH=/usr/bin HOME="$HOME" /usr/bin/some-toolFiles#
8. Predictable temporary files#
tmp="/tmp/build-$$" # PID is predictable; symlink attack
echo "$data" > "$tmp"tmp="$(mktemp -d)" || exit 1
trap 'rm -rf -- "$tmp"' EXITmktemp creates atomically with safe permissions. The trap is what stops you leaving secrets in /tmp when the script fails halfway.
9. umask#
umask 077 # before writing anything sensitive
echo "$token" > "$tmp/creds"Without it you inherit the caller's umask, which may well be 022 — world-readable credentials.
10. Following symlinks into somewhere else#
rm -rf -- "$dir" # $dir is a symlink to somewhere importantCheck before destructive operations:
[[ -L "$dir" ]] && { echo "refusing to operate on a symlink" >&2; exit 1; }11. cd without a check#
cd "$build_dir" # if this fails, the next line runs where you started
rm -rf ./*This is the most common way real damage happens, and it is not exotic — it is a typo plus a missing ||.
cd -- "$build_dir" || exit 1
[[ -f ./build.marker ]] || { echo "not the build directory" >&2; exit 1; }
rm -rf -- ./*A marker-file check before anything destructive is cheap and it is the control that would have prevented most published incidents of this kind.
Secrets#
12. Secrets in argv#
./deploy.sh --token "$SECRET" # visible in `ps` to every user on the box
curl -H "Authorization: Bearer $TOKEN" # also visible/proc/*/cmdline is world-readable on Linux. Anything on a command line is visible to every user on that machine, and it lands in shell history.
curl -H @<(printf 'Authorization: Bearer %s\n' "$TOKEN") "$url" # via a fd
curl --config <(printf 'header = "Authorization: Bearer %s"\n' "$TOKEN") "$url"Or read from a file with 0600 permissions, or an environment variable — which is visible in /proc/<pid>/environ only to the owner, so meaningfully better than argv.
13. set -x leaking secrets#
set -x # every expansion is printed, including secretsEnormously useful for debugging and a disclosure risk in CI logs, which are often more widely readable than you think. Turn it off around anything sensitive:
set +x
authenticate "$TOKEN"
set -x14. Secrets in the trap#
An ERR trap that prints $BASH_COMMAND will print the command including its expanded arguments. Worth knowing before you put a token on a command line.
Downloads and execution#
15. curl | sh#
curl -sSL https://example.com/install.sh | shYou are executing code you have not seen, over a channel that could be intercepted or the endpoint compromised, and a partial download can execute a truncated script that does something different from the whole one.
curl -fsSL -o /tmp/install.sh https://example.com/install.sh
sha256sum -c install.sha256 </tmp/install.sh || exit 1
less /tmp/install.sh # actually read it
bash /tmp/install.shNote the -f flag: without it curl writes the HTTP error page to your file and exits 0.
Block this at the harness level so it cannot happen by accident — a PreToolUse hook that refuses any pipe from curl into a shell. See harness hooks.
16. Unverified downloads generally#
Any curl, wget or git clone that feeds something you then execute deserves a checksum or a signature. Generated scripts fetch and run without either.
Privilege#
[[ $EUID -eq 0 ]] && { echo "do not run this as root" >&2; exit 1; }Refusing root when you do not need it is a one-line control that limits the blast radius of every other bug in the script. For scripts that genuinely need privilege, do the privileged part in a small separate script and call it with sudo, rather than running the whole thing as root.
The review#
shellcheck -S warning scripts/*.sh
grep -rnE '\beval\b|`|\$\(' scripts/ # eval and command substitution
grep -rnE 'rm -rf|mv |>\s*/' scripts/ # destructive, check the paths
grep -rnE 'curl[^|]*\|\s*(ba)?sh|wget[^|]*\|' scripts/ # pipe to shell
grep -rnE 'tmpnam|/tmp/\$\$|/tmp/[a-z]+\$' scripts/ # predictable temp files
grep -rnE '\$[A-Za-z_]+[^"]' scripts/ | grep -v '\[\[' # unquoted expansions
grep -rniE 'token|secret|password|api_key' scripts/ # secrets on command linesThe four that matter most
set -Eeuo pipefailand quote everything. Removes most of the injection surface.--before user-controlled arguments. Three characters, whole class of bug.- Marker-file check before anything destructive. The control that prevents the worst outcome.
- Never pipe a download into a shell. Block it in a hook so it cannot happen by accident.
Common questions#
Is shell inherently insecure?#
It is inherently unforgiving: expansion happens before execution, so a variable's contents can become syntax. That is manageable with quoting discipline and shellcheck, but it means shell deserves more review attention per line than a language where data and code are separate.
Should scripts run as root?#
Only the part that needs to. A whole script running as root means every bug in it — including the ones in the parts that had nothing to do with privilege — has maximum blast radius. Split the privileged operation into its own small, reviewable script.
How do I pass a secret to a command safely?#
Environment variable or a file descriptor, never argv. /proc/*/cmdline is world-readable, so anything on a command line is visible to every user on the machine and often ends up in logs and shell history as well.
Is set -x safe in CI?#
Only if you are confident nothing sensitive passes through the script, and CI logs are frequently more widely readable than people assume. Turn it off around authentication and secret handling, or use a targeted PS4 and enable it only for the sections you are debugging.
Get the Bash agent pack
A battle-tested AGENTS.md, the review checklist, and the failure-mode cheat sheet for Bash. One email, then occasional updates when the tooling shifts. No course pitch.
AGENTS.md now — no email needed.