diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6981006..2371f37 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,7 +36,9 @@ jobs: run: .venv/bin/mypy signal_bot fake_signal - name: shellcheck - run: shellcheck run.sh scripts/*.sh + # --severity=style is the floor CI enforces; the local pre-commit hook + # uses the same flag so a version skew cannot pass locally and fail here. + run: shellcheck --severity=style run.sh scripts/*.sh - name: pytest (100% branch coverage) run: .venv/bin/python -m pytest -q diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c93176d..c8aea1a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -77,7 +77,9 @@ repos: - id: shellcheck name: shellcheck - entry: shellcheck + # Same severity floor as CI: a stricter runner version must not be + # the thing that discovers a finding this hook let through. + entry: shellcheck --severity=style language: system types: [shell] diff --git a/scripts/e2e.sh b/scripts/e2e.sh index 9e6e9e2..9a9b5e7 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -28,11 +28,19 @@ BOT_PID="" TEMP_DIR="" cleanup() { - [[ -n "$BOT_PID" ]] && kill "$BOT_PID" 2>/dev/null || true - [[ -n "$FAKE_PID" ]] && kill "$FAKE_PID" 2>/dev/null || true + # Plain ifs, not `A && B || C`: that is not if-then-else, and C runs when + # A is true but B fails -- which is exactly the case here (kill on an + # already-dead pid). + if [[ -n "$BOT_PID" ]]; then + kill "$BOT_PID" 2>/dev/null || true + fi + if [[ -n "$FAKE_PID" ]]; then + kill "$FAKE_PID" 2>/dev/null || true + fi wait 2>/dev/null || true - [[ -n "$TEMP_DIR" && -d "$TEMP_DIR" ]] && rm -rf "$TEMP_DIR" - return 0 + if [[ -n "$TEMP_DIR" && -d "$TEMP_DIR" ]]; then + rm -rf "$TEMP_DIR" + fi } trap cleanup EXIT diff --git a/scripts/live_smoke.sh b/scripts/live_smoke.sh index 264188d..34c75d3 100755 --- a/scripts/live_smoke.sh +++ b/scripts/live_smoke.sh @@ -30,8 +30,9 @@ readonly FALLBACK_IMAGE="${FALLBACK_IMAGE:-bbernhard/signal-cli-rest-api:1787779 TEMP_DIR="" cleanup() { - [[ -n "$TEMP_DIR" && -d "$TEMP_DIR" ]] && rm -rf "$TEMP_DIR" - return 0 + if [[ -n "$TEMP_DIR" && -d "$TEMP_DIR" ]]; then + rm -rf "$TEMP_DIR" + fi } trap cleanup EXIT @@ -106,15 +107,30 @@ wait_for_link() { } choose_group() { - local number="$1" + local number="$1" groups choice id + # Fetched ONCE: two separate calls could return a different order, so the + # index the user picks would resolve against a different list. + groups="$(curl -sf "${API}/v1/groups/${number}")" + + # Every prompt goes to stderr: this function's stdout is captured by $(...) + # and must contain nothing but the chosen group id. echo "== groups on this account:" >&2 - curl -sf "${API}/v1/groups/${number}" \ - | jq -r '.[] | "\(.id)\t\(.name // "(unnamed)")"' \ + printf '%s' "$groups" | jq -r '.[] | "\(.id)\t\(.name // "(unnamed)")"' \ | nl -w3 -s' ' >&2 echo >&2 - read -r -p "Pick the number of the PRIVATE group to use: " choice - curl -sf "${API}/v1/groups/${number}" \ - | jq -r --argjson n "$choice" '.[$n - 1].id' + + read -r -p "Pick the number of the PRIVATE group to use: " choice &2 + if ! [[ "$choice" =~ ^[0-9]+$ ]]; then + echo "Error: '$choice' is not a number" >&2 + return 1 + fi + + id="$(printf '%s' "$groups" | jq -r --argjson n "$choice" '.[$n - 1].id // empty')" + if [[ -z "$id" ]]; then + echo "Error: no group at position $choice" >&2 + return 1 + fi + printf '%s' "$id" } write_env() {