Reviewing Shell Scripts: What Teams Should Standardize

1. Why reviewing shell scripts often fails

Reviewing shell scripts in a team context suffers from a structural problem: most developers have no shared idea of what makes a shell script good. In languages like Python or TypeScript there are linters, formatters and established style guides; for shell scripts, that culture is often entirely absent. The result is reviews where fundamental problems such as missing error handling, unquoted variables or silent pipe failures go unnoticed, simply because the reviewer does not know what to look for.

A second problem is the heterogeneity of existing shell codebases. Scripts accumulate over years, written by different people, and each script reflects the knowledge level and style of its author. New team members orient themselves on existing scripts, and in doing so reproduce both the good and the bad patterns. Without explicit standards and automated checks, every new file becomes part of a growing tangle that is barely maintainable anymore. The goal of this article is to establish a clear framework for reviewing shell scripts.

A third factor is underestimating shell scripts as critical infrastructure. Deployment scripts, backup routines, data migration scripts and monitoring alerters are often written in Bash and run with root privileges on production systems. A bug in these scripts can cause data loss, downtime or security holes. That justifies the same quality standard as application code, including review, tests and automated checks.

2. The review checklist for shell scripts

A structured checklist is the most important first step toward making shell script reviews systematic. The checklist does not replace thinking, but it ensures that basic checkpoints are never forgotten in any review. The first block covers the foundation: does the script start with the correct shebang (#!/usr/bin/env bash)? Are set -euo pipefail and IFS=$'\n\t' set? Is there a trap cleanup EXIT registration? Without these three points, the safety net for every subsequent operation is missing.

The second block of the checklist covers variables and quoting: are all variable references quoted? Are arrays used for file lists and command lists instead of strings? Are path variables protected with readonly or declare -r? The third block covers error handling: are exit codes of critical commands checked explicitly? Are there clear error messages written to stderr? Is pipefail used correctly? Consistently ticking off these points while reviewing shell scripts catches the most common bugs before the script ever reaches production.

The fourth block of the review checklist is security: is external input validated? Are temporary files created with mktemp and cleaned up with trap? Are there race conditions on file access? Are commands with user input executed in eval contexts? A fifth block covers testability: does the script have a main() function? Can it be included via source without triggering side effects? Are functions small enough to be tested individually? This structure turns reviewing shell scripts into a repeatable process.

#!/usr/bin/env bash
# review-check.sh - Automated pre-review checks for shell scripts
# Run: bash review-check.sh path/to/script.sh

set -euo pipefail
IFS=$'\n\t'

readonly SCRIPT="${1:?Usage: $0 <script.sh>}"
declare -i errors=0

check() {
  local desc="$1" pattern="$2"
  if ! grep -qE "$pattern" "$SCRIPT"; then
    echo "[FAIL] $desc" >&2
    (( errors++ )) || true
  else
    echo "[OK]   $desc"
  fi
}

echo "=== Reviewing: $SCRIPT ==="
check "Shebang: #!/usr/bin/env bash"    '^#!/usr/bin/env bash'
check "set -e present"                  'set -[a-z]*e|set -euo'
check "set -u present"                  'set -[a-z]*u|set -euo'
check "pipefail set"                    'pipefail'
check "IFS hardened"                    "IFS=\$'\\\\n\\\\t'"
check "trap EXIT registered"            'trap .* EXIT'
check "SCRIPT_DIR defined"              'SCRIPT_DIR'

echo ""
if (( errors > 0 )); then
  echo "[RESULT] $errors check(s) failed, review required" >&2
  exit 1
fi
echo "[RESULT] All basic checks passed"

3. Style rules: what the team should agree on

Style rules for shell scripts are not an end in themselves. They reduce the cognitive load of reviewing shell scripts, because the reviewer does not have to judge content and form at the same time. The first style rule concerns naming: functions are written in snake_case, global constants in UPPER_SNAKE_CASE, local variables in lower_snake_case. A team that applies this convention consistently can tell at a glance whether a variable is local or global, and whether it is allowed to be changed.

The second style rule: every function gets a single, clearly defined purpose. Functions with more than 30 lines are usually too big and should be split into smaller units. The third rule: comments explain the why, not the what. A comment like # delete temp file above rm -f "$tmpfile" is useless. A comment like # trap ensures cleanup even on SIGPIPE from downstream consumer explains a non-obvious design decision. Pointing out missing "why" comments while reviewing shell scripts is one of the most valuable reviewer activities.

The fourth style rule concerns the shebang line: always #!/usr/bin/env bash instead of #!/bin/bash, because the path to Bash differs between macOS and various Linux distributions. The fifth style rule: no parsing of ls output, no for f in $(cat filelist) constructs, no line processing with sed or awk for tasks that Bash builtins can handle. Writing these rules down and keeping them in the repository as SHELL_STYLE.md gives the review process a stable reference point.

4. EditorConfig for consistent script files

An .editorconfig file at the repository root ensures that all developers use the same baseline settings for shell script files, regardless of editor. The most important settings for shell scripts: indent_style = space with indent_size = 2 (shell convention, not 4 as in Python), end_of_line = lf (CRLF in shell scripts on Windows causes hard-to-diagnose errors), trim_trailing_whitespace = true and insert_final_newline = true. These settings are trivial to configure, but they prevent an entire category of diff noise when reviewing shell scripts.

A commonly overlooked problem: shell scripts edited on Windows systems and then executed on Linux can break due to CRLF line endings. The \r character is not interpreted as whitespace and ends up inside variable values, leading to cryptic error messages. end_of_line = lf in EditorConfig prevents that, and file -i script.sh shows whether a file already contains CRLF line endings. When reviewing shell scripts, it is worth including file -i as the first step in the review script.

# .editorconfig - consistent settings for all shell scripts in the repo
root = true

[*.sh]
charset = utf-8
indent_style = space
indent_size = 2
end_of_line = lf
trim_trailing_whitespace = true
insert_final_newline = true

[*.bash]
charset = utf-8
indent_style = space
indent_size = 2
end_of_line = lf
trim_trailing_whitespace = true
insert_final_newline = true

# Prevent CRLF contamination in CI helper scripts
[bin/*]
end_of_line = lf
trim_trailing_whitespace = true
insert_final_newline = true

5. ShellCheck: static analysis in detail

ShellCheck is the most important tool for automating shell script reviews. It statically analyzes Bash scripts and detects over 500 different classes of errors, from unquoted variables (SC2086) to lost exit codes (SC2155) to portability problems between Bash versions. For every warning, ShellCheck outputs a link to shellcheck.net that explains the problem and shows the correct fix. That makes it the ideal teaching tool for teams that are just starting to take reviewing shell scripts seriously.

The most important ShellCheck warning categories are: SC2086 (missing quotes on variable expansion), SC2034 (unused variable, often a typo in the variable name), SC2155 (declaration and assignment combined, exit code gets lost), SC2046 (unquoted command substitution in command arguments) and SC1090/SC1091 (source file cannot be statically analyzed, ShellCheck cannot check dynamic source paths). The option shellcheck -S warning filters to only warnings and errors, no style hints, a good starting point for legacy codebases where style hints would generate too much noise.

ShellCheck supports directives for selectively suppressing warnings: # shellcheck disable=SC2086 above a line disables the warning for exactly that line. That is useful when a warning is a false positive, but these directives should always come with a comment explaining why the warning does not apply here. A common case: intentionally unquoted variables when word splitting is desired. When reviewing shell scripts, unexplained ShellCheck directives are a clear piece of review feedback.

6. Integrating ShellCheck into the CI pipeline

Integrating ShellCheck as a CI gate is the most effective way to enforce standards while reviewing shell scripts, without reviewers having to manually hunt for known bugs. In GitHub Actions, ShellCheck is available as a ready-made action and can be integrated in a few lines. Alternatively, ShellCheck can be installed directly on the runner and invoked as a shell command, which gives more control over parameters and makes the pipeline less dependent on external actions. The key decision: should ShellCheck run as a hard gate (pipeline fails) or a soft gate (warning without blocking)? For new codebases, a hard gate from the start is recommended.

For existing codebases with many legacy scripts, a step-by-step approach is recommended: first check only new and changed files (git diff --name-only HEAD~1 | grep '\.sh$'), then gradually lower the threshold of allowed warnings. A .shellcheckrc at the repository root allows project-wide configuration without command-line parameters and keeps the CI configuration cleaner. In human shell script reviews, ShellCheck then only leaves the cases that static analysis cannot detect: logic errors, faulty assumptions about system state and missing business logic validation.

# .github/workflows/shellcheck.yml - ShellCheck CI gate for all shell scripts

name: ShellCheck
on:
  push:
    paths: ['**.sh', 'bin/**']
  pull_request:
    paths: ['**.sh', 'bin/**']

jobs:
  shellcheck:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4

      - name: Install ShellCheck
        run: sudo apt-get install -y shellcheck

      - name: Run ShellCheck on all scripts
        run: |
          # Find all shell scripts (including those without .sh extension)
          mapfile -t scripts < <(
            find . -name "*.sh" -not -path "./.git/*" -print
            find bin/ -type f -not -name "*.*" -exec grep -l '#!/.*bash' {} \;
          )
          echo "Checking ${#scripts[@]} scripts..."
          shellcheck --severity=warning --shell=bash "${scripts[@]}"

      - name: Check EditorConfig compliance
        run: |
          # Detect CRLF line endings, fatal for shell scripts on Linux
          if grep -rlU $'\r' --include="*.sh" .; then
            echo "ERROR: CRLF line endings found in shell scripts" >&2
            exit 1
          fi

7. Automated testing with BATS

BATS (Bash Automated Testing System) enables unit tests for shell scripts with a syntax reminiscent of pytest or RSpec. For reviewing shell scripts, BATS is especially valuable because it distinguishes testable scripts from untestable ones: a script with a main() function and a clean separation of functions from the main logic can be tested with BATS. A script that immediately executes commands when sourced cannot. That distinction becomes visible in review: missing testability is a concrete criticism, not just a style issue.

A complete BATS test checks the behavior of a function in isolation, mocks external commands via function shadowing, and verifies both stdout and stderr output as well as exit codes. The test file lives in a tests/ directory next to the script files and runs in the CI pipeline after ShellCheck. When reviewing shell scripts, it is worth requiring at least three tests per new function: the happy path, the error case and the edge case (empty input, whitespace in the argument). Requiring tests during review is the most sustainable measure for long-term shell codebase quality.

8. Anchoring the review process in the team

Technical tools alone are not enough. Reviewing shell scripts must also be anchored culturally. That means: pull requests for every script change, even small hotfixes. A minimum of one reviewer with Bash knowledge. A clear definition of which points are covered by automated checks and which points should be checked in manual review. And a review template as a pull request description that mirrors the checklist from section 2.

A frequently underestimated aspect of reviewing shell scripts is the documentation inside the script itself. Every script should have a usage comment at the top showing how it is invoked, which environment variables it expects and what it returns. A usage() function invoked by -h or --help is not a luxury, it is the first piece of documentation a new team member sees. Requiring during review that this documentation stays current costs little, but saves considerable time during every later debugging session.

9. Tool comparison for shell reviews

Several tools are available for reviewing shell scripts, differing in scope, integration and depth of analysis. The right combination depends on team size, repository complexity and the existing CI stack.

The optimal combination for reviewing shell scripts in teams: ShellCheck as a mandatory CI gate, shfmt for automatic formatting in a pre-commit hook, BATS for critical production-relevant scripts, and the manual checklist for human review. This layering separates what can be checked automatically from what requires human judgment.

10. Summary

Reviewing shell scripts as a team becomes effective when it rests on three pillars: automated checking through ShellCheck as a CI gate, consistent formatting through EditorConfig and shfmt, and a structured manual review process with a checklist. ShellCheck automatically handles the well-known error classes and gives reviewers more time for logic and design. EditorConfig prevents the most common formatting debates and protects against CRLF problems. The manual checklist ensures that foundation, error handling and security are checked systematically.

The cultural aspect matters just as much as the tools: pull requests for every script change, a reviewer with Bash knowledge, and the expectation that scripts are documented, tested and developed to standards just like application code. That pays off especially for deployment scripts, backup routines and other automations that run with elevated privileges on production systems, where bugs are not an academic question but have direct operational consequences.

11. FAQ: Reviewing Shell Scripts, What Teams Should Standardize