diff --git a/.github/workflows/shell-tests.yml b/.github/workflows/shell-tests.yml new file mode 100644 index 0000000..355eef7 --- /dev/null +++ b/.github/workflows/shell-tests.yml @@ -0,0 +1,35 @@ +name: shell-tests + +# Behavioral (bats) tests for the scripts/connect*.sh installers. These drive the +# byte-identical shipped files end-to-end through a stub-first PATH (fake docker/ +# sudo/ss/curl/...) and assert on what each installer *tried* to do — so a silent +# regression like #70 (a bare `port_in_use` tripping `set -e` when the WS port is +# free, aborting the whole installer) is caught before it ships over the CDN. +# +# Runs unconditionally on every PR rather than filtering on `paths:`. A +# `paths`-filtered *required* check is skipped-not-reported on PRs that don't touch +# the filtered paths, leaving the PR stuck on "Expected — Waiting for status to be +# reported" (see .github/workflows/actionlint.yml for the same reasoning). bats is +# fast, so mirror rust.yml and always run. +on: + push: + branches: [main, 'hotfix/**'] + pull_request: + +# Least-privilege default: this workflow only reads the repo. +permissions: + contents: read + +jobs: + bats: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + # bats-core from apt; python3 is preinstalled on ubuntu-latest (the access-pass + # heredoc tests use it — the regression suite stubs it out). + - name: Install bats + run: | + sudo apt-get update + sudo apt-get install -y bats + - name: Run installer regression tests + run: bats tests/scripts/ diff --git a/CHANGELOG.md b/CHANGELOG.md index cd43910..c782473 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 against a live edge+public capture (2026-06-30): Phoenix uses the same bare symbol on both feeds (edge `instrument_id == public assetId`) and `trade_id == tradeSequenceNumber` on shared fills. No `FEEDS` row depends on it. +- Behavioral regression tests for the `scripts/connect*.sh` installers (`tests/scripts/`, bats-core), + run in CI by a new `shell-tests` workflow. The suite drives the **byte-identical shipped scripts** + end-to-end through a stub-first `PATH` (fake `docker`/`sudo`/`ss`/`curl`/...) and asserts on what + each installer tried to do — no source guard or refactor added to the files served over the CDN. It + iterates all three installers (so per-script drift is caught) and pins the #70 fix: with the WS port + free the installer must survive preflight and reach `docker run` (the pre-#70 code aborted here under + `set -e`); with the port busy, preflight must actually detect the conflict (warn) and, non-interactively, + continue to `docker run` anyway. - **Multi-publisher dedup for Market-by-Order `depth`** (#28, the MBO half of #3 — TOB shipped earlier). `MboProcessor` now reconstructs an **independent L3 book per `(publisher, instrument)`** (keyed on the datagram source IP), since two publishers' instance-scoped per-instrument delta diff --git a/CLAUDE.md b/CLAUDE.md index 94278bd..6bad16a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -15,6 +15,13 @@ an implementation detail; the *only* external contract is the WebSocket output, **PROTOCOL.md** (v1). Any engine that speaks WebSocket + JSON consumes it via a thin adapter; the consumer is not part of the protocol. +## Commits & authorship + +- **Never attribute commits or code to Claude/Anthropic (or any AI).** No + `Co-Authored-By: Claude`, no "Generated with Claude Code" trailers in commit messages or + PR bodies, and no AI-attribution comments in source. Commits and code read as the human + author's own work. + ## Commands ```bash diff --git a/docs/superpowers/specs/2026-07-06-connect-script-unit-tests-design.md b/docs/superpowers/specs/2026-07-06-connect-script-unit-tests-design.md new file mode 100644 index 0000000..e5ef8a2 --- /dev/null +++ b/docs/superpowers/specs/2026-07-06-connect-script-unit-tests-design.md @@ -0,0 +1,159 @@ +# Unit tests for the `connect*.sh` installers — design + +**Date:** 2026-07-06 +**Status:** SUPERSEDED — implemented as a black-box suite in PR #73, not as designed here. +**Scope:** `scripts/connect.sh`, `scripts/connect-testnet.sh`, `scripts/connect-devnet.sh` + +> **Superseded (PR #73).** The shipped tests deliberately reject this design's two +> central decisions. There is **no source guard and no function extraction**: editing a +> file that users fetch via `curl | bash` to make it testable was judged too risky (a +> misfiring guard aborts the installer for every user), so the installers stay +> **byte-identical**. Instead each script is driven end-to-end through a stub-first +> `PATH` and asserted on its observable behavior (argv handed to the `docker` stub, exit +> status). The delivered suite is `tests/scripts/preflight_ws_port.bats` + +> `tests/scripts/_helpers.bash` — the `set -e`/#70 regression net and the WS-port +> preflight path — not the six files / four coverage buckets below. Treat the sections +> that follow (source guard, `dz_token_to_json`/`build_env_args` extraction, `guard.bats`, +> etc.) as a rejected earlier proposal kept for history; **do not** reintroduce the +> CDN-served source guard. + +## Motivation + +The three installers carry non-trivial logic (`DZ_` token decode, an embedded +Python access-pass/PDA check, WS-port preflight, env passthrough) and ship to users +verbatim over a CDN (`get.doublezero.xyz/connect*`). A silent regression recently +shipped undetected: `port_in_use "$p"; local rc=$?` exits the whole script under +`set -e` when the port is free (the common case), because the bare non-zero return +trips `errexit` before `rc` is captured. Nothing caught it — there are no shell +tests, and `shellcheck` does not reliably flag this specific `set -e` footgun. + +Goal: behavioral unit tests around the pure logic and the exact failure mode we hit, +runnable in CI, without changing the runtime contract (each installer stays a single +self-contained file served over `curl | bash`). + +## Design decisions (approved) + +1. **Testability = source guard**, not a shared lib. Each script stays an independent + file; a guard lets tests `source` it to get the functions without running `main`. +2. **Framework = bats-core.** +3. **Coverage buckets:** (a) `set -e` regression, (b) `DZ_` token parsing, + (c) WS-port logic + env passthrough, (d) the embedded Python access-pass checker. +4. **The guard must not break `curl | bash`** — use behavioral source-detection, not + `BASH_SOURCE == $0` (approved point 1). +5. **Extract `dz_token_to_json` and `build_env_args`** into functions (approved point 2). +6. **Mock RPC** for the Python network exit codes 0/2/3 (approved point 3). + +## Script changes (all three, identical) + +### The source guard — critical detail + +The idiomatic `[ "${BASH_SOURCE[0]}" = "$0" ]` guard is **unsafe here**: under +`curl -fsSL … | bash` the script is read from stdin, `BASH_SOURCE[0]` is empty and +`$0` is `"bash"`, so the guard would misfire and abort the installer immediately — +worse than the bug we fixed. Use behavioral detection instead: + +```bash +# after all function definitions, before section 1: +if (return 0 2>/dev/null); then return 0; fi # sourced -> only funcs defined, stop +``` + +`return` succeeds only when sourced; executed (including via stdin from `curl|bash`) +it fails, so `main` runs. This is verified by a dedicated regression test +(`guard.bats`): executing via stdin proceeds past the guard; `source` stops before it. + +### Reorganization (minimal) + +- Move the `ws_disabled` / `ws_port` / `port_in_use` / `preflight_ws_port` block up + next to `info`/`warn`/`ask`/… so all functions precede the guard. +- Add the two extractions below, also above the guard. +- Place the guard immediately before section 1 (the first executable statement). +- Sections 1–8 stay line-for-line the same except the two extracted blocks become + function calls. + +On `source`: `set` flags run, config defaults run (harmless var assignments), color +detection runs, all functions get defined, guard returns. **No `sudo`, `docker`, or +network side effects.** + +### Two extractions (to make them testable) + +- `dz_token_to_json ` — emits the 64-int JSON array, returns non-zero on invalid + base64url length / wrong byte count. **Pure: no `die`/`info`** (the main flow keeps + the messaging). Replaces the inline block (current `connect.sh:157–175`). +- `build_env_args` — prints the `-e VAR=val` arguments from the environment (the + `PASSTHROUGH` loop plus the special "forward `WS_BIND` even when empty" rule). + Replaces the inline block (current `connect.sh:487–502`). + +## Test architecture + +``` +tests/scripts/ + _helpers.bash # SCRIPTS list, stub-PATH builder, Python-heredoc extractor, mock-RPC starter + guard.bats # source stops at guard; stdin-exec runs main (blinds the curl|bash guard) + set_e_regression.bats # preflight_ws_port with a free port -> script survives (the bug) + token_parse.bats # dz_token_to_json: valid vectors + rejections + ws_port.bats # ws_disabled/ws_port/port_in_use + preflight decision table + build_env_args + accesspass.bats # embedded Python checker (extract heredoc + mock RPC) +``` + +`cargo` compiles only `tests/*.rs`; `tests/scripts/*.bats|*.bash` are ignored — they +coexist without collision. + +### Isolation pattern + +Each assertion runs in a fresh subshell with a stub-first `PATH` (fake `ss`, `docker`, +`sudo`, `netstat`, `curl`): + +```bash +run bash -c 'source "$SCRIPT"; ws_port' +``` + +This keeps the script's `set -euo pipefail` from contaminating bats and exercises the +**real file** through its **real guard**. + +### Drift protection across the three scripts + +Because the files stay independent (no shared lib), the behavioral suite **iterates +over all three** (`SCRIPTS=(connect connect-testnet connect-devnet)`). A function that +diverges and breaks in one script is caught — this is the net against the drift we +already observed (the log-cap block present only in `connect.sh`). + +## Coverage detail + +### (a) `set -e` regression — `set_e_regression.bats` +Run `preflight_ws_port` with a stubbed `ss` reporting the port **free**; assert the +sourced script survives (status 0, execution continues). Fails against the old +`cmd; local rc=$?`, passes with `local rc=0; port_in_use || rc=$?`. Run for all three. + +### (b) Token parse — `token_parse.bats` +`dz_token_to_json`: a known `DZ_` token → exact 64-int JSON; padding variants +(len %4 == 2 and == 3); reject len %4 == 1; reject a token that decodes to ≠ 64 bytes. + +### (c) WS-port logic + env passthrough — `ws_port.bats` +- `ws_disabled`: unset / non-empty / set-empty (`WS_BIND=""`). +- `ws_port`: default 8081 / port from `WS_BIND`. +- `port_in_use`: anchoring (8081 must **not** match 18081), `ss` present, `netstat` + fallback, neither → rc 2. +- `preflight_ws_port` decision table: free / in-use / no-tooling / disabled, plus the + non-interactive continue and the interactive `p`/`d`/`c` branches (feed answers on a + fake TTY; `p` re-runs the recursion on the new port). +- `build_env_args`: only non-empty vars forwarded; `WS_BIND` forwarded even when empty. + +### (d) Embedded Python checker — `accesspass.bats` +The script stays single-file. The test helper extracts the `<<'PY' … PY` heredoc into a +temp `.py` **at test time** (real code, not a maintained copy) and runs it: +- **No network:** exit 5 (stdin not a 64-int array); exit 4 (RPC not http(s)); + "ignoring malformed public IP"; and an **identity vector** (known 64-byte keypair → + expected `IDENTITY `, validating `b58encode` + the identity slice). +- **With network:** a local `http.server` mock returns a canned `getAccountInfo` — + `data[0]==11` ⇒ exit 0; `value:null` ⇒ exit 2 (pub_ip set) / exit 3 (no pub_ip). + +## CI + +New `.github/workflows/shell-tests.yml` (sibling of `actionlint.yml`), triggered on PRs +touching `scripts/**` or `tests/scripts/**`: installs `bats` + `python3`, runs +`bats tests/scripts/`. + +## Out of scope (YAGNI) + +`detect_cloud` / `detect_public_ip` (network), the real `docker run`, sudo priming, and +`ask`/`confirm` TTY interaction beyond the non-interactive preflight path. diff --git a/tests/scripts/_helpers.bash b/tests/scripts/_helpers.bash new file mode 100755 index 0000000..ab51e2a --- /dev/null +++ b/tests/scripts/_helpers.bash @@ -0,0 +1,134 @@ +# Shared helpers for the connect*.sh black-box tests. +# +# These tests never modify the shipped installer. Each script is run end-to-end +# through a stub-first PATH: fake `docker`, `sudo`, `ss`, `curl`, `sleep`, ... +# shadow the real tools, so the byte-identical file users get via `curl | bash` +# is exercised, and we assert on what it *tried* to do (the argv it handed the +# `docker` stub, its exit status) rather than on any test-only seam. + +# Repo layout: this file lives at /tests/scripts/_helpers.bash +HELPERS_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "$HELPERS_DIR/../.." && pwd)" +SCRIPTS_DIR="$REPO_ROOT/scripts" + +# The three installers stay independent files (no shared lib), so every behavioral +# test iterates this list — a function that drifts and breaks in one is caught. +SCRIPTS=(connect connect-testnet connect-devnet) + +# make_stubs +# Populate with the default stub set that lets the installer reach the +# `docker run` step without any privilege, network, or real container. Individual +# tests overwrite a single stub (e.g. `ss`) to script the case under test. +make_stubs() { + local bin="$1" + mkdir -p "$bin" + + # sudo: strip its own leading options and exec the rest unprivileged, so + # `sudo -n true`, `sudo -v`, and `sudo docker ...` all behave. + cat >"$bin/sudo" <<'EOF' +#!/usr/bin/env bash +while [ $# -gt 0 ]; do case "$1" in -*) shift;; *) break;; esac; done +[ $# -eq 0 ] && exit 0 +exec "$@" +EOF + + # docker: record every invocation to $DOCKER_LOG; answer the few subcommands + # the script reads back. `logs` emits the readiness line so the 30x wait loop + # breaks on the first iteration. + cat >"$bin/docker" <<'EOF' +#!/usr/bin/env bash +printf 'docker %s\n' "$*" >>"$DOCKER_LOG" +case "$1" in + info) exit 0 ;; + logs) echo "doublezerod ready"; exit 0 ;; + ps) echo "stubcontainerid"; exit 0 ;; + *) exit 0 ;; +esac +EOF + + # Fully hermetic, deterministic host — never touch the network or the real box. + cat >"$bin/curl" <<'EOF' +#!/usr/bin/env bash +exit 1 +EOF + # python3 stub: skip the onchain access-pass pre-check with an inconclusive + # code (the script warns and continues). Keeps the run offline regardless of + # whether real python3 is installed. (accesspass.bats uses the REAL python3.) + cat >"$bin/python3" <<'EOF' +#!/usr/bin/env bash +exit 4 +EOF + cat >"$bin/uname" <<'EOF' +#!/usr/bin/env bash +case "$1" in -s) echo Linux ;; -m) echo x86_64 ;; *) echo Linux ;; esac +EOF + cat >"$bin/sleep" <<'EOF' +#!/usr/bin/env bash +exit 0 +EOF + cat >"$bin/modprobe" <<'EOF' +#!/usr/bin/env bash +exit 0 +EOF + cat >"$bin/sysctl" <<'EOF' +#!/usr/bin/env bash +[ "$1" = -n ] && { echo 268435456; exit 0; } +exit 0 +EOF + cat >"$bin/getenforce" <<'EOF' +#!/usr/bin/env bash +echo Disabled +EOF + cat >"$bin/ufw" <<'EOF' +#!/usr/bin/env bash +echo "Status: inactive" +EOF + cat >"$bin/firewall-cmd" <<'EOF' +#!/usr/bin/env bash +echo "offline" +exit 1 +EOF + + # Default ss: report the WS port FREE (one unrelated listening socket). + ss_reports_free "$bin" + + chmod +x "$bin"/* +} + +# ss_reports_free : `ss -ltn` lists a socket that never matches a WS port. +ss_reports_free() { + cat >"$1/ss" <<'EOF' +#!/usr/bin/env bash +echo "LISTEN 0 128 127.0.0.1:22 0.0.0.0:*" +EOF + chmod +x "$1/ss" +} + +# ss_reports_busy : `ss -ltn` lists as already bound. The +# heredoc is quoted so the stub body stays literal (nothing expands here even if +# it grows); the port is supplied at stub runtime via the exported env var. +ss_reports_busy() { + export DZ_TEST_BUSY_PORT="$2" + cat >"$1/ss" <<'EOF' +#!/usr/bin/env bash +echo "LISTEN 0 128 0.0.0.0:${DZ_TEST_BUSY_PORT} 0.0.0.0:*" +EOF + chmod +x "$1/ss" +} + +# common_env: env every run needs to be non-interactive and offline. +# - DZ_SECRET points at a keyfile (KEY_SRC=file) so we skip token decode here. +# - DZ_CLIENT_IP short-circuits public-IP detection. +# Call after $STUB_BIN and $KEYFILE are set. Prepends the stub dir to PATH. +common_env() { + export PATH="$STUB_BIN:$PATH" + export DZ_SECRET="$KEYFILE" + export DZ_ASSUME_YES=1 + export DZ_CLIENT_IP="203.0.113.7" + # connect-devnet.sh pulls a private image and requires a ghcr token; the docker + # stub makes `docker login` a no-op, so any non-empty value gets us past the gate. + # Harmless for connect.sh / connect-testnet.sh, which ignore it. + export DZ_GHCR_TOKEN="${DZ_GHCR_TOKEN:-stub-ghcr-token}" + # DZ_ENV intentionally left unset — each installer picks its own default + # (connect.sh -> mainnet-beta, connect-testnet.sh -> testnet, etc.). +} diff --git a/tests/scripts/preflight_ws_port.bats b/tests/scripts/preflight_ws_port.bats new file mode 100755 index 0000000..5a69c05 --- /dev/null +++ b/tests/scripts/preflight_ws_port.bats @@ -0,0 +1,80 @@ +#!/usr/bin/env bats +# +# WS-port preflight coverage for the connect*.sh installers, including the +# regression net for #70: under `set -e`, a bare `port_in_use "$p"` returning +# non-zero (the common case — the WS port is FREE) tripped errexit and aborted +# the whole installer before it ever started the container. The fix captures the +# status without letting the bare non-zero command trip errexit +# (`local rc=0; port_in_use || rc=$?`). +# +# Both tests drive the REAL, unmodified script end-to-end through a stub-first +# PATH — no change to the shipped file required. +# - free port : asserts the installer survives preflight and reaches `docker +# run` (fails against pre-#70 code, passes against the fix). +# - busy port : asserts preflight actually DETECTS the conflict (warns) and, +# non-interactively, continues to `docker run` anyway. + +load _helpers + +setup() { + STUB_BIN="$BATS_TEST_TMPDIR/bin" + DOCKER_LOG="$BATS_TEST_TMPDIR/docker.log" + KEYFILE="$BATS_TEST_TMPDIR/id.json" + export DOCKER_LOG + : >"$DOCKER_LOG" + # A plausible keypair file (contents don't matter: KEY_SRC=file skips token + # decode, and the access-pass check is stubbed out). + printf '[%s]' "$(seq -s, 64 | sed 's/[0-9]*/0/g')" >"$KEYFILE" + make_stubs "$STUB_BIN" +} + +@test "free WS port: installer survives preflight and reaches docker run (all scripts)" { + local fails=0 + for s in "${SCRIPTS[@]}"; do + : >"$DOCKER_LOG" + ss_reports_free "$STUB_BIN" + ( + common_env + bash "$SCRIPTS_DIR/$s.sh" + ) + status=$? + if [ "$status" -ne 0 ]; then + echo "# $s.sh exited $status with a free WS port (the #70 abort)"; fails=1; continue + fi + if ! grep -q '^docker run ' "$DOCKER_LOG"; then + echo "# $s.sh never reached 'docker run' — preflight aborted early" + echo "# docker calls seen:"; sed 's/^/# /' "$DOCKER_LOG"; fails=1; continue + fi + done + [ "$fails" -eq 0 ] +} + +@test "busy WS port, non-interactive: preflight detects it and still reaches docker run (all scripts)" { + local fails=0 + for s in "${SCRIPTS[@]}"; do + : >"$DOCKER_LOG" + ss_reports_busy "$STUB_BIN" 8081 + local err="$BATS_TEST_TMPDIR/$s.err" + ( + common_env + bash "$SCRIPTS_DIR/$s.sh" + ) 2>"$err" + status=$? + if [ "$status" -ne 0 ]; then + echo "# $s.sh exited $status on a busy port"; fails=1; continue + fi + # Prove the busy branch actually fired. Without this, the test is a + # near-duplicate of the free-port case: with DZ_ASSUME_YES=1 both paths reach + # `docker run`, so it would still pass even if port_in_use wrongly reported + # the busy port free. The warn ("already in use") comes only from the + # conflict branch, so it pins that preflight detected the bound port. + if ! grep -qi 'already in use' "$err"; then + echo "# $s.sh never warned the WS port was busy — preflight didn't detect the conflict:" + sed 's/^/# /' "$err"; fails=1; continue + fi + if ! grep -q '^docker run ' "$DOCKER_LOG"; then + echo "# $s.sh did not reach 'docker run' on a busy port"; fails=1; continue + fi + done + [ "$fails" -eq 0 ] +}