首页 全部提示词 Agent Skill

Shell Script Safety Reviewer

技能 @f 40,139 字符 在 prompts.chat 查看 译文有问题?

这条还没有中文译文,当前显示英文原文。译文由定时任务补齐,也可以到仓库提 issue 催更。

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 fix
  • references/safe-patterns.md - a strict-mode header, safe temp files and cleanup, safe loops over files, argument parsing, secrets, downloads and logging
  • templates/shell-review.md - the review to hand back
  • examples/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 -e as the only fix: also check pipes (pipefail), subshells, local x=$(...) and if cmd contexts 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: read without -r eats backslashes; without IFS= 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 wait for 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 +x around 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

  1. bash -n script.sh (syntax only), shellcheck script.sh, python3 scripts/shell_lint.py script.sh.
  2. Run with a dry-run flag, then in a throwaway container: docker run --rm -v "$PWD":/w -w /w bash:5 bash script.sh.
  3. Trace once with bash -x script.sh (secrets off). FILE:templates/shell-review.md

Shell script review: