# Shell Safety Review Lens

Review shell scripts for **robustness, error handling, and defensive patterns**.

## What to Look For

### Error Handling
- Missing error strategy: `set -euo pipefail` OR explicit checks per command
- Note: `set -e` has edge cases (conditionals, `||`, subshells) - explicit checks often safer
- Unchecked return codes from critical operations (file ops, network, root commands)
- Missing `trap` for cleanup on exit/error
- Pipes hiding exit codes without `pipefail` or `PIPESTATUS` checks

### Variable & Input Safety
- Unquoted variables in commands (SC2086: word splitting)
- Variables used before assignment or without defaults (`${VAR:-default}`)
- Missing input validation: required args, file existence, numeric checks
- `read` without `IFS= read -r` (SC2162: backslash/whitespace bugs)

### Command Safety
- Unsafe `cd` without checking: use `cd dir || exit 1`
- `rm -rf` with unguarded variables: use `${VAR:?}` or explicit checks
- Dangerous primitives: `eval`, `source` of non-constant paths, `curl | sh`
- Missing `--` to separate options from arguments

### Temp Files & Atomicity
- Hardcoded temp paths (`/tmp/foo`) instead of `mktemp`
- Predictable temp names (`/tmp/script.$$`) - use `mktemp -d`
- Missing cleanup of temp files on exit

## Linter Integration

```bash
shellcheck -x --format=json --severity=style script.sh
```

Key codes for ops safety:
- SC2086: Double quote to prevent splitting
- SC2164: Use `cd ... || exit`
- SC2015: `A && B || C` logic error (C runs if B fails too)
- SC2162: `read` without `-r`
- SC2155: Declare and assign separately (masked return values)

## Output Format

```
[SHELL] <severity:HIGH|MED|LOW> <file:line>
Issue: <what's unsafe and why>
Suggest: <specific fix with code example>
Evidence: <shellcheck code or pattern matched>
```

## Common Fixes

```bash
# Safe rm pattern
: "${TARGET:?TARGET must be set}"
rm -rf -- "$TARGET"

# Safe cd pattern
cd -- "$dir" || { echo "cd failed: $dir" >&2; exit 1; }

# Safe read loop
while IFS= read -r line; do
  ...
done < "$file"
```

## Guidelines

- **HIGH** = data loss risk, silent failure, or injection vector
- **MED** = defensive pattern missing, potential edge-case bugs
- **LOW** = style, portability, maintainability
- Respect shell dialect: `local`, `[[ ]]`, `pipefail` are bash-only
- Prioritize scripts running as root or handling sensitive operations
- Consider: will this break if run twice? With empty input? As cron job?
