feat(formatters/sh): surface missing shfmt/shellcheck + add SETE/LEAK checks (U-H2)
This commit is contained in:
110
formatters/sh
110
formatters/sh
@@ -1,31 +1,127 @@
|
|||||||
#!/usr/bin/env bash
|
#!/usr/bin/env bash
|
||||||
# Formatter: Shell — shfmt + shellcheck
|
# Formatter: Shell — shfmt + shellcheck + SETE/LEAK checks
|
||||||
# Contract: $1 = absolute file path, format in place, lint, exit 0 if clean / 1 if errors
|
# Contract: $1 = absolute file path, format in place, lint, exit 0 if clean / 1 if errors
|
||||||
# Missing tools: exit 0 silently
|
#
|
||||||
|
# SETE-1: shebanged script with no `set -e` / `set -euo pipefail` anywhere
|
||||||
|
# SETE-2: sh/bash -c string with 2+ ;- or newline-separated commands and no set -e inside
|
||||||
|
# LEAK-1: command substitution reading a secret-named file interpolated into
|
||||||
|
# echo/printf/die/error/fail argument
|
||||||
|
#
|
||||||
|
# Missing shfmt AND shellcheck: emit one warning to stderr, still exit 0
|
||||||
|
# (a missing tool must not block an edit). Any SETE/LEAK finding: print
|
||||||
|
# "path:line: ID message" to stderr, exit non-zero.
|
||||||
|
|
||||||
|
set -uo pipefail
|
||||||
|
|
||||||
FILE="$1"
|
FILE="$1"
|
||||||
[ -z "$FILE" ] && exit 0
|
[ -z "$FILE" ] && exit 0
|
||||||
[ -f "$FILE" ] || exit 0
|
[ -f "$FILE" ] || exit 0
|
||||||
|
|
||||||
HAD_ERRORS=0
|
HAD_ERRORS=0
|
||||||
|
HAS_SHFMT=0
|
||||||
|
HAS_SHELLCHECK=0
|
||||||
|
|
||||||
# Format with shfmt if available
|
# Format with shfmt if available
|
||||||
if command -v shfmt >/dev/null 2>&1; then
|
if command -v shfmt >/dev/null 2>&1; then
|
||||||
|
HAS_SHFMT=1
|
||||||
shfmt -w "$FILE" 2>/dev/null
|
shfmt -w "$FILE" 2>/dev/null
|
||||||
fi
|
fi
|
||||||
|
|
||||||
# Lint with shellcheck if available
|
# Lint with shellcheck if available
|
||||||
if command -v shellcheck >/dev/null 2>&1; then
|
if command -v shellcheck >/dev/null 2>&1; then
|
||||||
ERRORS=$(shellcheck -f gcc "$FILE" 2>&1)
|
HAS_SHELLCHECK=1
|
||||||
if [ $? -ne 0 ]; then
|
if ! ERRORS=$(shellcheck -f gcc "$FILE" 2>&1); then
|
||||||
echo "$ERRORS" >&2
|
echo "$ERRORS" >&2
|
||||||
HAD_ERRORS=1
|
HAD_ERRORS=1
|
||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
|
|
||||||
# If neither tool is installed, silently pass
|
# --- Custom checks: SETE-1, SETE-2, LEAK-1 ---
|
||||||
if ! command -v shfmt >/dev/null 2>&1 && ! command -v shellcheck >/dev/null 2>&1; then
|
# Always run, regardless of shfmt/shellcheck presence. Reads $FILE, emits
|
||||||
exit 0
|
# one "path:line: ID message" line per finding to stdout, exits non-zero on
|
||||||
|
# any finding.
|
||||||
|
|
||||||
|
if ! CUSTOM_OUT=$(
|
||||||
|
python3 - "$FILE" <<'PYEOF'
|
||||||
|
import re
|
||||||
|
import sys
|
||||||
|
|
||||||
|
filepath = sys.argv[1]
|
||||||
|
findings = []
|
||||||
|
|
||||||
|
try:
|
||||||
|
with open(filepath, 'r', encoding='utf-8', errors='replace') as f:
|
||||||
|
content = f.read()
|
||||||
|
lines = content.splitlines()
|
||||||
|
except OSError:
|
||||||
|
sys.exit(0)
|
||||||
|
|
||||||
|
# SETE-1: shebanged script with no `set -e` / `set -euo pipefail` anywhere
|
||||||
|
first = lines[0] if lines else ''
|
||||||
|
if first.startswith('#!'):
|
||||||
|
has_sete = False
|
||||||
|
for line in lines:
|
||||||
|
if re.search(r'(^|[^A-Za-z0-9_])set\s+-[A-Za-z]*[eE]', line):
|
||||||
|
has_sete = True
|
||||||
|
break
|
||||||
|
if not has_sete:
|
||||||
|
findings.append((1, "SETE-1 executable shell script without 'set -e'; failed earlier commands will not abort"))
|
||||||
|
|
||||||
|
SECRET_RE = re.compile(r'TOKEN|SECRET|KEY|PASSWORD|_JSON_FILE|CREDENTIAL', re.IGNORECASE)
|
||||||
|
SINK_RE = re.compile(r'\b(echo|printf|die|error|fail)(?=\s|\()')
|
||||||
|
FILE_READ_RE = re.compile(
|
||||||
|
r'(\$\(|`)\s*(cat|<)\s*"\$\{?([A-Za-z_][A-Za-z0-9_]*)\}?"\s*(\)|`)'
|
||||||
|
)
|
||||||
|
|
||||||
|
def count_top_level_commands(s):
|
||||||
|
"""Count ;- or newline-separated top-level commands in a shell string,
|
||||||
|
ignoring separators inside single or double quotes."""
|
||||||
|
in_single = False
|
||||||
|
in_double = False
|
||||||
|
separators = 0
|
||||||
|
for c in s:
|
||||||
|
if not in_double and c == "'":
|
||||||
|
in_single = not in_single
|
||||||
|
elif not in_single and c == '"':
|
||||||
|
in_double = not in_double
|
||||||
|
elif c in ';\n' and not in_single and not in_double:
|
||||||
|
separators += 1
|
||||||
|
return separators + 1
|
||||||
|
|
||||||
|
for i, line in enumerate(lines, 1):
|
||||||
|
# SETE-2: sh/bash -c with 2+ ;- or newline-separated commands and no set -e inside
|
||||||
|
m = re.search(r'\b(?:sh|bash)\s+-c\s+(["\'])([^"\']*)\1', line)
|
||||||
|
if m:
|
||||||
|
s = m.group(2)
|
||||||
|
if not re.search(r'(^|[^A-Za-z0-9_])set\s+-[A-Za-z]*[eE]', s):
|
||||||
|
n = count_top_level_commands(s)
|
||||||
|
if n >= 2:
|
||||||
|
findings.append((i, "SETE-2 sh/bash -c with " + str(n) + " ;-separated commands and no 'set -e' inside; container exit reflects only the last command"))
|
||||||
|
|
||||||
|
# LEAK-1: command substitution reading a secret-named file, inside echo/printf/die/error/fail
|
||||||
|
sink_match = SINK_RE.search(line)
|
||||||
|
if sink_match:
|
||||||
|
cmd = sink_match.group(1)
|
||||||
|
for cmdsub in FILE_READ_RE.finditer(line):
|
||||||
|
var = cmdsub.group(3)
|
||||||
|
if SECRET_RE.search(var):
|
||||||
|
findings.append((i, "LEAK-1 secret variable '$" + var + "' read into string passed to '" + cmd + "'; could print credential document"))
|
||||||
|
|
||||||
|
for line_no, msg in findings:
|
||||||
|
print(filepath + ":" + str(line_no) + ": " + msg)
|
||||||
|
|
||||||
|
sys.exit(1 if findings else 0)
|
||||||
|
PYEOF
|
||||||
|
); then
|
||||||
|
echo "$CUSTOM_OUT" >&2
|
||||||
|
HAD_ERRORS=1
|
||||||
|
fi
|
||||||
|
|
||||||
|
# If neither shfmt nor shellcheck is installed, warn ONCE so the silent pass
|
||||||
|
# is no longer silent. A missing tool must not block an edit (would be a worse
|
||||||
|
# bug than the one this warning surfaces).
|
||||||
|
if [ "$HAS_SHFMT" -eq 0 ] && [ "$HAS_SHELLCHECK" -eq 0 ]; then
|
||||||
|
echo "formatters/sh: shfmt and shellcheck not installed; install via 'apt install shfmt shellcheck' (or equivalent). Custom checks still ran." >&2
|
||||||
fi
|
fi
|
||||||
|
|
||||||
exit $HAD_ERRORS
|
exit $HAD_ERRORS
|
||||||
|
|||||||
245
formatters/tests/test-sh-formatter.sh
Executable file
245
formatters/tests/test-sh-formatter.sh
Executable file
@@ -0,0 +1,245 @@
|
|||||||
|
#!/usr/bin/env bash
|
||||||
|
# tests/test-sh-formatter.sh — assert formatters/sh runs SETE-1/2 and LEAK-1
|
||||||
|
#
|
||||||
|
# House convention: bash, set -uo pipefail, SKIP-and-exit-0 when a tool is
|
||||||
|
# missing, one PASS:/FAIL: line per assertion, FAIL=1 aggregate, ALL_OK on
|
||||||
|
# full pass.
|
||||||
|
|
||||||
|
set -uo pipefail
|
||||||
|
|
||||||
|
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||||
|
FORMATTER="$SCRIPT_DIR/../sh"
|
||||||
|
|
||||||
|
# --- SKIP gates: missing tool → exit 0 silently ---
|
||||||
|
if ! command -v python3 >/dev/null 2>&1; then
|
||||||
|
echo "SKIP: python3 not on PATH — formatters/sh custom checks need it"
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
if [ ! -x "$FORMATTER" ]; then
|
||||||
|
echo "SKIP: formatters/sh not found or not executable at $FORMATTER"
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
PASS=0
|
||||||
|
FAIL=0
|
||||||
|
|
||||||
|
pass() {
|
||||||
|
printf "PASS: %s\n" "$1"
|
||||||
|
PASS=$((PASS + 1))
|
||||||
|
}
|
||||||
|
|
||||||
|
fail() {
|
||||||
|
printf "FAIL: %s\n" "$1"
|
||||||
|
[[ -n "${2:-}" ]] && printf " %s\n" "$2"
|
||||||
|
FAIL=$((FAIL + 1))
|
||||||
|
}
|
||||||
|
|
||||||
|
TMPDIR="$(mktemp -d)"
|
||||||
|
trap 'rm -rf "$TMPDIR"' EXIT
|
||||||
|
|
||||||
|
# Write a fixture and run the formatter. Assert:
|
||||||
|
# - exit code matches expected_exit
|
||||||
|
# - stderr contains the pattern (when expect_pattern is set)
|
||||||
|
assert_flagged() {
|
||||||
|
local label="$1"
|
||||||
|
local expected_exit="$2"
|
||||||
|
local pattern="$3"
|
||||||
|
local content="$4"
|
||||||
|
local f="$TMPDIR/$(printf '%s' "$label" | tr ' /' '__').sh"
|
||||||
|
|
||||||
|
printf '%s\n' "$content" > "$f"
|
||||||
|
|
||||||
|
local stderr_file="$TMPDIR/$(basename "$f").stderr"
|
||||||
|
"$FORMATTER" "$f" >/dev/null 2>"$stderr_file"
|
||||||
|
local actual_exit=$?
|
||||||
|
local stderr_content
|
||||||
|
stderr_content="$(cat "$stderr_file")"
|
||||||
|
|
||||||
|
if [ "$actual_exit" -ne "$expected_exit" ]; then
|
||||||
|
fail "$label (exit)" "expected $expected_exit, got $actual_exit. stderr: $stderr_content"
|
||||||
|
return
|
||||||
|
fi
|
||||||
|
|
||||||
|
if [ -n "$pattern" ] && ! echo "$stderr_content" | grep -qE "$pattern"; then
|
||||||
|
fail "$label (pattern)" "expected pattern '$pattern', got: $stderr_content"
|
||||||
|
return
|
||||||
|
fi
|
||||||
|
|
||||||
|
pass "$label"
|
||||||
|
}
|
||||||
|
|
||||||
|
# Write a fixture and run the formatter. Assert:
|
||||||
|
# - exit code is 0
|
||||||
|
# - stderr does NOT contain the forbidden pattern
|
||||||
|
assert_clean() {
|
||||||
|
local label="$1"
|
||||||
|
local forbidden="$2"
|
||||||
|
local content="$3"
|
||||||
|
local f="$TMPDIR/$(printf '%s' "$label" | tr ' /' '__').sh"
|
||||||
|
|
||||||
|
printf '%s\n' "$content" > "$f"
|
||||||
|
|
||||||
|
local stderr_file="$TMPDIR/$(basename "$f").stderr"
|
||||||
|
"$FORMATTER" "$f" >/dev/null 2>"$stderr_file"
|
||||||
|
local actual_exit=$?
|
||||||
|
local stderr_content
|
||||||
|
stderr_content="$(cat "$stderr_file")"
|
||||||
|
|
||||||
|
if [ "$actual_exit" -ne 0 ]; then
|
||||||
|
fail "$label (exit)" "expected 0, got $actual_exit. stderr: $stderr_content"
|
||||||
|
return
|
||||||
|
fi
|
||||||
|
|
||||||
|
if [ -n "$forbidden" ] && echo "$stderr_content" | grep -qE "$forbidden"; then
|
||||||
|
fail "$label (forbidden pattern)" "should NOT match '$forbidden', got: $stderr_content"
|
||||||
|
return
|
||||||
|
fi
|
||||||
|
|
||||||
|
pass "$label"
|
||||||
|
}
|
||||||
|
|
||||||
|
# ===========================================================================
|
||||||
|
echo "=== SETE-1: shebanged script without set -e ==="
|
||||||
|
# ===========================================================================
|
||||||
|
|
||||||
|
# Positive: shebang, no set -e anywhere → must flag SETE-1, exit 1
|
||||||
|
assert_flagged "SETE-1 positive" 1 "SETE-1" \
|
||||||
|
"#!/usr/bin/env bash
|
||||||
|
echo hello
|
||||||
|
ls /nonexistent
|
||||||
|
echo world"
|
||||||
|
|
||||||
|
# Negative: shebang + set -euo pipefail → no SETE-1 finding, exit 0
|
||||||
|
assert_clean "SETE-1 negative (has set -euo pipefail)" "SETE-1" \
|
||||||
|
"#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
echo hello
|
||||||
|
ls /nonexistent || true"
|
||||||
|
|
||||||
|
# Negative: shebang + set -e (short form) → no SETE-1 finding, exit 0
|
||||||
|
assert_clean "SETE-1 negative (has set -e)" "SETE-1" \
|
||||||
|
"#!/usr/bin/env bash
|
||||||
|
set -e
|
||||||
|
echo hello"
|
||||||
|
|
||||||
|
# Negative: sourced fragment (no shebang) without set -e → no SETE-1 finding
|
||||||
|
# `shell=bash` directive satisfies shellcheck's SC2148 ("add a shebang or shell directive")
|
||||||
|
assert_clean "SETE-1 negative (sourced fragment, no shebang)" "SETE-1" \
|
||||||
|
"# shellcheck shell=bash
|
||||||
|
echo hello
|
||||||
|
ls /nonexistent || true
|
||||||
|
echo world"
|
||||||
|
|
||||||
|
# ===========================================================================
|
||||||
|
echo ""
|
||||||
|
echo "=== SETE-2: sh/bash -c with multi-command string and no set -e ==="
|
||||||
|
# ===========================================================================
|
||||||
|
|
||||||
|
# Positive: sh -c with two ;-separated commands and no set -e → flag SETE-2
|
||||||
|
assert_flagged "SETE-2 positive (hugo mod get; mod tidy)" 1 "SETE-2" \
|
||||||
|
"#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
sh -c \"hugo mod get github.com/foo/bar; hugo mod tidy\""
|
||||||
|
|
||||||
|
# Positive: bash -c with multiple ; commands, no set -e
|
||||||
|
assert_flagged "SETE-2 positive (bash -c cmd1; cmd2)" 1 "SETE-2" \
|
||||||
|
"#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
bash -c \"echo a; echo b; echo c\""
|
||||||
|
|
||||||
|
# Negative: sh -c with single command → no SETE-2 finding
|
||||||
|
assert_clean "SETE-2 negative (single command)" "SETE-2" \
|
||||||
|
"#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
sh -c \"hugo mod tidy\""
|
||||||
|
|
||||||
|
# Negative: sh -c with multi commands BUT set -e inside → no SETE-2 finding
|
||||||
|
assert_clean "SETE-2 negative (has set -e inside)" "SETE-2" \
|
||||||
|
"#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
sh -c \"set -e; hugo mod get; hugo mod tidy\""
|
||||||
|
|
||||||
|
# ===========================================================================
|
||||||
|
echo ""
|
||||||
|
echo "=== LEAK-1: secret-named file read into echo/printf/die/error/fail ==="
|
||||||
|
# ===========================================================================
|
||||||
|
|
||||||
|
# Positive: die "$(cat "$KV_JSON_FILE")" — the canonical real case from the brief
|
||||||
|
assert_flagged "LEAK-1 positive (die with cat \$KV_JSON_FILE)" 1 "LEAK-1" \
|
||||||
|
"#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
die \"kv put failed: \$(cat \"\$KV_JSON_FILE\")\""
|
||||||
|
|
||||||
|
# Positive: echo with TOKEN-named file read
|
||||||
|
assert_flagged "LEAK-1 positive (echo with \$GITHUB_TOKEN_FILE)" 1 "LEAK-1" \
|
||||||
|
"#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
echo \"got: \$(cat \"\$GITHUB_TOKEN_FILE\")\""
|
||||||
|
|
||||||
|
# Positive: printf with PASSWORD-named file read
|
||||||
|
assert_flagged "LEAK-1 positive (printf with \$DB_PASSWORD_FILE)" 1 "LEAK-1" \
|
||||||
|
"#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
printf 'db password was: %s\n' \"\$(< \"\$DB_PASSWORD_FILE\")\""
|
||||||
|
|
||||||
|
# Negative: die with no secret-named variable
|
||||||
|
assert_clean "LEAK-1 negative (regular error)" "LEAK-1" \
|
||||||
|
"#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
die \"kv put failed: connection refused\""
|
||||||
|
|
||||||
|
# Negative: die with secret-named var but NOT read via cat/$() — just interpolated
|
||||||
|
assert_clean "LEAK-1 negative (var interpolated, not cat-substituted)" "LEAK-1" \
|
||||||
|
"#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
RESULT_VAR=foo
|
||||||
|
die \"error code: \$RESULT_VAR\""
|
||||||
|
|
||||||
|
# Negative: secret-named var read via cat but NOT in echo/die/etc.
|
||||||
|
# Mark vars as used via export so shellcheck doesn't flag SC2034.
|
||||||
|
assert_clean "LEAK-1 negative (secret file read, not in echo)" "LEAK-1" \
|
||||||
|
"#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
contents=\$(cat \"\$API_TOKEN_FILE\")
|
||||||
|
export contents"
|
||||||
|
|
||||||
|
# Negative: echo with non-secret variable name
|
||||||
|
assert_clean "LEAK-1 negative (echo with non-secret var)" "LEAK-1" \
|
||||||
|
"#!/usr/bin/env bash
|
||||||
|
set -euo pipefail
|
||||||
|
echo \"result file content: \$(cat \"\$RESULT_FILE\")\""
|
||||||
|
|
||||||
|
# ===========================================================================
|
||||||
|
echo ""
|
||||||
|
echo "=== Missing-tool warning (formatters/sh must NOT silently pass) ==="
|
||||||
|
# ===========================================================================
|
||||||
|
|
||||||
|
# When neither tool is on PATH, formatter should emit ONE warning line.
|
||||||
|
# Build a PATH that has bash + python3 but no shfmt or shellcheck, then run.
|
||||||
|
PATH_CLEAN="$(mktemp -d)"
|
||||||
|
ln -sf /usr/bin/bash "$PATH_CLEAN/bash"
|
||||||
|
ln -sf /usr/bin/python3 "$PATH_CLEAN/python3"
|
||||||
|
stderr_file="$TMPDIR/no-tools.stderr"
|
||||||
|
# Use a clean fixture — must not flag any SETE/LEAK.
|
||||||
|
printf '#!/usr/bin/env bash\nset -euo pipefail\necho hello\n' > "$TMPDIR/clean-fixture.sh"
|
||||||
|
PATH="$PATH_CLEAN" bash "$FORMATTER" "$TMPDIR/clean-fixture.sh" >/dev/null 2>"$stderr_file"
|
||||||
|
rc=$?
|
||||||
|
stderr_content="$(cat "$stderr_file")"
|
||||||
|
rm -rf "$PATH_CLEAN"
|
||||||
|
|
||||||
|
if [ "$rc" -eq 0 ] && echo "$stderr_content" | grep -qE 'formatters/sh: shfmt and shellcheck not installed'; then
|
||||||
|
pass "missing-tool warning emitted, exit 0"
|
||||||
|
else
|
||||||
|
fail "missing-tool warning" "expected exit 0 + warning line, got exit=$rc stderr=$stderr_content"
|
||||||
|
fi
|
||||||
|
|
||||||
|
# ===========================================================================
|
||||||
|
echo ""
|
||||||
|
echo "Results: $PASS passed, $FAIL failed"
|
||||||
|
|
||||||
|
if [ "$FAIL" -eq 0 ]; then
|
||||||
|
echo "ALL_OK"
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
exit 1
|
||||||
Reference in New Issue
Block a user