From aea1798592c4067053c8ee03e9d6aaa4b54c9424 Mon Sep 17 00:00:00 2001 From: Dennis Jakobsen Date: Wed, 23 Sep 2026 16:52:04 +0200 Subject: [PATCH 1/8] fix(qa): harden blacklist test process and runner boundaries (#83) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Keep the pubkey, SQL and URLs out of argv on both ends: the config edit reads the pubkey from ssh stdin and passes it to jq/python3 via the environment, curl reads its URL from a -K - config on stdin, and grep reads its patterns from mode-600 files in the run's mode-700 temp dir. - Remove the ADMIN_API_TOKEN path: /api/admin/transmissions never existed, so §10.2 is SQLite-only. Setting the variable now refuses to start. - Split TARGET_DB_PATH into TARGET_CONTAINER_DB_PATH and TARGET_HOST_DB_PATH, each used only by its own runner. The path is checked as a non-empty file in the runner's environment before any query, and sqlite3 runs with -readonly, so a wrong path can no longer leave an empty database behind. The legacy variable refuses to start. - Trap SIGPIPE like INT/TERM (teardown, 141, +1 on teardown failure) and ignore it inside teardown, so a closed stdout/stderr cannot cut the restore short. - Route all messages through say/warn: bash 3.2 replays text it failed to write into the next subshell's output, which with a dead stdout put the pubkey and log lines into the remote config-edit script. A dead stream is now sent to /dev/null and its buffer flushed there. - Build remote command lines with printf -v %q instead of $(...). - Correct the RESTART_WAIT_S and second-signal-in-teardown wording. - Tests: a fake target (ssh/docker/curl + argv-logging shims) runs the whole script through success, failure, signal and broken-stream cases. - CI: run shellcheck -x on both scripts. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/deploy.yml | 5 +- qa/plans/v3.6.0-rc.md | 2 +- qa/scripts/blacklist-test.sh | 404 ++++++++++++++----- qa/scripts/test-blacklist-sql.sh | 662 +++++++++++++++++++++++++++---- 4 files changed, 902 insertions(+), 171 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 108d2b970..4fabea9a4 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -91,9 +91,12 @@ jobs: - name: Staging disk-monitor unit tests (issue #1684) run: bash scripts/staging/test-disk-monitor.sh - - name: QA SQL parameter-binding unit tests (issue #1977) + - name: QA blacklist script unit tests (issues #1977, #83) run: bash qa/scripts/test-blacklist-sql.sh + - name: ShellCheck QA blacklist scripts (issue #83) + run: shellcheck -x -P SCRIPTDIR qa/scripts/blacklist-test.sh qa/scripts/test-blacklist-sql.sh + - name: Lint CSS variables (issue #1128) run: | set -e diff --git a/qa/plans/v3.6.0-rc.md b/qa/plans/v3.6.0-rc.md index 77bcb8a72..7b981e929 100644 --- a/qa/plans/v3.6.0-rc.md +++ b/qa/plans/v3.6.0-rc.md @@ -95,7 +95,7 @@ Run `scripts/api-contract-diff.sh BASELINE_URL TARGET_URL` once. Report the scri | 10.1 | Add node pubkey to nodeBlacklist config; restart | Hidden from listings/map/neighbor graph | #742 | auto: blacklist-test.sh | | 10.2 | Packets still in DB | Yes (filter not delete) | #742 | auto: blacklist-test.sh | -`blacklist-test.sh` covers both 10.1 and 10.2 in one run. Required env: `TEST_NODE_PUBKEY` (hex, of a real visible node on TARGET), `TARGET_SSH_HOST`, `TARGET_CONFIG_PATH`, `TARGET_CONTAINER`. Optional: `TARGET_DB_PATH` or `ADMIN_API_TOKEN` for §10.2 probe; `TARGET_SSH_KEY` (default `/root/.ssh/id_ed25519`). Mandatory teardown removes the pubkey and verifies the node returns to listings. +`blacklist-test.sh` covers both 10.1 and 10.2 in one run. Required env: `TEST_NODE_PUBKEY` (hex, of a real visible node on TARGET), `TARGET_SSH_HOST`, `TARGET_CONFIG_PATH`, `TARGET_CONTAINER`. §10.2 is SQLite-only and needs at least one db path, each used only by its own runner: `TARGET_CONTAINER_DB_PATH` (path inside the container, for `docker exec … sqlite3`) and/or `TARGET_HOST_DB_PATH` (path on the target host, for its own `sqlite3`). The two may differ; a path that is not a non-empty file in its runner's environment fails §10.2 without creating a file. `TARGET_DB_PATH` and `ADMIN_API_TOKEN` are no longer read and make the run refuse to start. Optional: `TARGET_SSH_KEY` (default `/root/.ssh/id_ed25519`), `CURL_TIMEOUT`, `RESTART_WAIT_S`. Mandatory teardown removes the pubkey and verifies the node returns to listings. The pubkey, SQL and URLs never appear in process arguments (stdin / mode-600 files instead). ### 11. Deploy/ops | # | Step | Pass criteria | Source | Mode | diff --git a/qa/scripts/blacklist-test.sh b/qa/scripts/blacklist-test.sh index 195cdd5b5..efe52d5c7 100755 --- a/qa/scripts/blacklist-test.sh +++ b/qa/scripts/blacklist-test.sh @@ -15,27 +15,67 @@ # TARGET_CONFIG_PATH — absolute path to config.json on the target # TARGET_CONTAINER — docker container name on the target # Optional env: -# TARGET_DB_PATH — sqlite db path on the target (for §10.2 sqlite probe) -# ADMIN_API_TOKEN — if /api/admin/transmissions exists, use it instead of ssh+sqlite -# (read from env, not argv — never appears in ps) +# TARGET_CONTAINER_DB_PATH — sqlite db path INSIDE TARGET_CONTAINER. Used only +# by the container runner (docker exec … sqlite3). +# TARGET_HOST_DB_PATH — sqlite db path on TARGET_SSH_HOST's own +# filesystem. Used only by the host runner. +# §10.2 needs at least one of the two; they may +# differ, and neither is ever tried in the other +# environment. # CURL_TIMEOUT — per-request curl timeout, seconds (default 60) -# RESTART_WAIT_S — max wait for /api/stats after restart (default 120) +# RESTART_WAIT_S — seconds after a restart during which a new +# /api/stats poll may still start (default 120). A poll +# in flight at the deadline runs to completion, so the +# wait can last up to RESTART_WAIT_S + CURL_TIMEOUT + 3. +# +# Removed env — the run refuses to start (exit 2) if either is set, rather than +# silently doing something other than what the operator asked for: +# TARGET_DB_PATH — named one path for two filesystems. The host runner +# used the container's path, and the host's sqlite3 +# could create an empty database there. +# ADMIN_API_TOKEN — queried /api/admin/transmissions, which CoreScope +# has never had; every run fell through to SQLite. +# §10.2 is SQLite-only. +# +# Process arguments: the pubkey, the SQL and every URL stay out of argv on both +# ends. The config edit reads the pubkey from ssh stdin, curl reads its URL from +# a config on stdin (-K -), grep reads its pattern from a mode-600 file under +# the run's mode-700 temp dir, and SQL crosses to sqlite3 on stdin. # # Distinguishes: # ssh-failed → cannot reach/control target -# restart-stuck → /api/stats not 200 within RESTART_WAIT_S +# restart-stuck → no /api/stats poll started within RESTART_WAIT_S returned 200 # hide-failed → blacklisted pubkey still surfaced via API (§10.1 fail) # retain-failed → no transmissions.from_pubkey rows (ADVERTs) for the # blacklisted pubkey in the DB (§10.2 fail), or the -# §10.2 probe could not run at all — no sqlite3 on the target -# able to bind a parameter. The message names what is needed; -# there is no fallback to interpolated SQL. +# §10.2 probe could not run at all — no db path configured, +# no sqlite3 able to bind a parameter, or the configured path +# is not a non-empty file in its runner's environment. The +# message names what is needed; there is no fallback to +# interpolated SQL, and no path is tried in the wrong place. # teardown-failed→ post-test removal did not restore listing # # Exit code = number of failures (0 = pass). An interrupted run still tears -# down, then exits 130 (SIGINT) or 143 (SIGTERM), plus 1 if teardown failed. +# down, then exits 130 (SIGINT), 143 (SIGTERM) or 141 (SIGPIPE), plus 1 if +# teardown failed. +# +# SIGPIPE: the script's own writes go to stdout/stderr, so if either is a pipe +# whose reader has gone, the shell gets SIGPIPE. It is handled like INT/TERM: +# teardown, then 141, plus 1 if teardown failed. Unhandled, bash would still +# run the EXIT trap but then re-raise the signal, so the status would be 141 +# whatever teardown found, and teardown's own next write to the dead stream +# would raise SIGPIPE again and cut the restore short. Inside teardown SIGPIPE +# is ignored, so a lost output channel cannot do that. Child processes keep +# the default disposition; one killed by SIGPIPE fails its step, which is +# classified as usual. A shell started with SIGPIPE already ignored cannot +# trap it; its writes then fail with EPIPE and the run continues to its normal +# exit status. Either way a dead stream is sent to /dev/null (see say/warn), +# never replayed. +# # A further INT/TERM does not abort teardown: it stops only the step in -# progress (e.g. a hung ssh), which is then reported as teardown-failed. +# progress. An interrupted ssh step fails and is reported as teardown-failed; +# an interrupted /api/stats poll only counts as one failed poll, so the wait +# goes on until RESTART_WAIT_S and teardown can still succeed. # PUBLIC repo: zero PII — no real pubkeys, IPs, or hostnames as defaults. # # Structure: helpers live at top level and the imperative body lives in main(), @@ -45,8 +85,29 @@ set -uo pipefail SSH_OPTS=() # populated by main() from TARGET_SSH_KEY +# Callers build the remote command line here on purpose, quoting every value +# with printf -v %q; nothing else is meant to reach the remote shell. +# shellcheck disable=SC2029 ssh_t() { ssh "${SSH_OPTS[@]}" "$TARGET_SSH_HOST" "$@"; } +# Output. All of the script's own messages go through say (stdout) and warn +# (stderr), never a bare echo, because of how bash 3.2 — still macOS's +# /bin/bash — handles a write that fails: it keeps the unwritten text in its +# stdout buffer, and the next subshell flushes that text into ITS output. The +# next subshell might be a $(...) capture, or the group that pipes the config +# edit into ssh, whose remote bash would then run the leftover lines as +# commands. (bash 5 purges the buffer; the failure mode is 3.2's.) So a stream +# found dead is pointed at /dev/null for the rest of the run and the leftover +# buffer is flushed there at once; the run's exit status carries the result. +# A write can only fail this way once SIGPIPE is not killing the shell: inside +# teardown, where it is ignored, or when the shell started with it ignored. +stream_dead() { + if [[ "$1" == 1 ]]; then exec 1>/dev/null; else exec 2>/dev/null; fi + echo -n '' >/dev/null # flush 3.2's leftover buffer into /dev/null +} +say() { printf '%s\n' "$*" 2>/dev/null || stream_dead 1; } +warn() { printf '%s\n' "$*" >&2 2>/dev/null || stream_dead 2; } + # ----------------------------------------------------------------------------- # Teardown — MANDATORY in all exit paths. # ----------------------------------------------------------------------------- @@ -58,22 +119,27 @@ teardown() { if [[ -n "${1:-}" ]]; then rc=$1; fi if [[ "$TEARDOWN_DONE" == "1" ]]; then rm -rf "$TMP"; exit "$rc"; fi TEARDOWN_DONE=1 + # A closed stdout/stderr must not end the restore: every message below, and + # the INT/TERM notice, would otherwise raise SIGPIPE again with no handler + # left to finish the job. Ignored rather than handled, so a write just fails + # (EPIPE) and say/warn retire the dead stream. + trap '' PIPE # A second Ctrl-C/TERM must not abort the restore, or the node would stay # blacklisted with no warning. Note the signal rather than ignoring it: an # ignored disposition is inherited by ssh/curl, so a hung step could no longer # be interrupted. With a handler, a terminal Ctrl-C still stops the running # step; that step fails and teardown reports teardown-failed. - trap 'echo " (signal received — teardown continues restoring the target)" >&2' INT TERM - echo "=== teardown: removing $TEST_PUBKEY from nodeBlacklist ===" + trap 'warn " (signal received — teardown continues restoring the target)"' INT TERM + say "=== teardown: removing $TEST_PUBKEY from nodeBlacklist ===" if remove_from_blacklist && restart_target && wait_for_stats; then if node_visible; then - echo " ✅ teardown ok — node returned to listings" + say " ✅ teardown ok — node returned to listings" else - echo " ❌ teardown-failed: node still hidden after removal" + say " ❌ teardown-failed: node still hidden after removal" rc=$((rc + 1)) fi else - echo " ❌ teardown-failed: could not restore config / restart / stats" + say " ❌ teardown-failed: could not restore config / restart / stats" rc=$((rc + 1)) fi rm -rf "$TMP" @@ -84,57 +150,103 @@ install_teardown_traps() { trap teardown EXIT trap 'teardown 130' INT trap 'teardown 143' TERM + trap 'teardown 141' PIPE } # ----------------------------------------------------------------------------- # Helpers # ----------------------------------------------------------------------------- +# curl config line for a URL. main() rejects URLs with quotes, backslashes, +# whitespace or control characters, so the escaping here is defence in depth. +curl_url_config() { + local u="$1" + u=${u//\\/\\\\} + u=${u//\"/\\\"} + printf 'url = "%s"\n' "$u" +} + +# The URL reaches curl as a config on stdin (-K -), never as an argument: +# /api/nodes/ would otherwise put the pubkey in curl's argv. fetch_code() { local url="$1" out="$2" - curl -s -m "$CURL_TIMEOUT" -o "$out" -w "%{http_code}" "$url" 2>/dev/null || echo "000" + curl_url_config "$url" | curl -s -m "$CURL_TIMEOUT" -K - -o "$out" -w "%{http_code}" 2>/dev/null || echo "000" +} + +# Pattern files for grep -f, written once by main() under the mode-700 $TMP. +# Each holds exactly one non-empty line (the pubkey is validated non-empty hex), +# which matters: an empty pattern line would make grep -F match everything. +PK_PATTERN="" # bare pubkey +PK_QUOTED_PATTERN="" # the pubkey as a JSON string, "…" +write_pubkey_patterns() { + PK_PATTERN="$TMP/pubkey.pat" + PK_QUOTED_PATTERN="$TMP/pubkey-quoted.pat" + ( umask 077 + printf '%s\n' "$TEST_PUBKEY" >"$PK_PATTERN" + printf '"%s"\n' "$TEST_PUBKEY" >"$PK_QUOTED_PATTERN" ) +} + +# grep FILE for a pattern file. Status is grep's: 0 found, 1 not found, 2 error. +# Callers must not read 2 as "not found" — for a hide check that is a pass. +file_has_pattern() { + local pattern_file="$1" file="$2" + grep -qF -f "$pattern_file" -- "$file" } wait_for_stats() { local deadline code - echo " waiting up to ${RESTART_WAIT_S}s for $TARGET_URL/api/stats ..." + say " polling $TARGET_URL/api/stats; no new poll starts after ${RESTART_WAIT_S}s ..." deadline=$(( $(date +%s) + RESTART_WAIT_S )) while (( $(date +%s) < deadline )); do code=$(fetch_code "$TARGET_URL/api/stats" "$TMP/stats.json") - if [[ "$code" == "200" ]]; then echo " stats OK"; return 0; fi + if [[ "$code" == "200" ]]; then say " stats OK"; return 0; fi sleep 3 done - echo " ❌ restart-stuck: /api/stats never returned 200" + say " ❌ restart-stuck: no /api/stats poll started within ${RESTART_WAIT_S}s returned 200" return 1 } restart_target() { - echo " restarting container $TARGET_CONTAINER ..." + say " restarting container $TARGET_CONTAINER ..." # TARGET_CONTAINER is validated above; still quote defensively. - if ! ssh_t "docker restart $(printf %q "$TARGET_CONTAINER")" >/dev/null; then - echo " ❌ ssh-failed: docker restart failed" + local q_container + printf -v q_container '%q' "$TARGET_CONTAINER" + if ! ssh_t "docker restart $q_container" /dev/null; then + say " ❌ ssh-failed: docker restart failed" return 1 fi return 0 } -# Mutate config.json on target. Values pass via env (printf %q + single-quoted -# heredoc) so $TEST_PUBKEY etc. never enter the remote shell as code. +# Mutate config.json on target. The pubkey travels on ssh's stdin, in the line +# right after the remote script's first command, which reads it: bash -s reads +# its script from stdin without reading ahead, so `read` gets exactly that line +# (POSIX requires this of a shell reading commands from stdin). From there it +# reaches jq/python3 through the environment. It is therefore in neither the +# local ssh argv nor any remote argv; only the config path and the constant +# mode are on the command line. set_blacklist_state() { - local mode="$1" # add | remove - ssh_t "CFG=$(printf %q "$TARGET_CONFIG_PATH") PK=$(printf %q "$TEST_PUBKEY") MODE=$(printf %q "$mode") bash -s" <<'REMOTE' + local mode="$1" q_cfg q_mode # mode: add | remove + printf -v q_cfg '%q' "$TARGET_CONFIG_PATH" + printf -v q_mode '%q' "$mode" + { + printf 'IFS= read -r PK || exit 3\n%s\n' "$TEST_PUBKEY" + cat <<'REMOTE' set -euo pipefail +case "$PK" in ""|*[!0-9a-fA-F]*) echo "blacklist-test: pubkey on stdin is not hex" >&2; exit 3 ;; esac +export PK TMP="$(mktemp)" trap 'rm -f "$TMP"' EXIT if command -v jq >/dev/null; then if [ "$MODE" = "add" ]; then - jq --arg pk "$PK" '.nodeBlacklist = ((.nodeBlacklist // []) + [$pk] | unique)' "$CFG" > "$TMP" + jq '.nodeBlacklist = ((.nodeBlacklist // []) + [env.PK] | unique)' "$CFG" > "$TMP" else - jq --arg pk "$PK" '.nodeBlacklist = ((.nodeBlacklist // []) - [$pk])' "$CFG" > "$TMP" + jq '.nodeBlacklist = ((.nodeBlacklist // []) - [env.PK])' "$CFG" > "$TMP" fi else - python3 - "$CFG" "$PK" "$MODE" "$TMP" <<'PY' -import json, sys -cfg, pk, mode, out = sys.argv[1:] + python3 - "$CFG" "$MODE" "$TMP" <<'PY' +import json, os, sys +cfg, mode, out = sys.argv[1:] +pk = os.environ["PK"] with open(cfg) as f: d = json.load(f) bl = list(dict.fromkeys(d.get("nodeBlacklist") or [])) if mode == "add": @@ -151,9 +263,10 @@ chown --reference="$CFG" "$TMP" 2>/dev/null || true mv "$TMP" "$CFG" trap - EXIT REMOTE + } | ssh_t "CFG=$q_cfg MODE=$q_mode bash -s" local rc=$? if (( rc != 0 )); then - echo " ❌ ssh-failed: could not edit $TARGET_CONFIG_PATH ($mode)" + say " ❌ ssh-failed: could not edit $TARGET_CONFIG_PATH ($mode)" return 1 fi return 0 @@ -168,7 +281,7 @@ node_visible() { code=$(fetch_code "$TARGET_URL/api/nodes/$TEST_PUBKEY" "$TMP/node.json") if [[ "$code" == "200" ]]; then return 0; fi fetch_code "$TARGET_URL/api/nodes?limit=10000" "$TMP/nodes.json" >/dev/null - if grep -qF -- "\"$TEST_PUBKEY\"" "$TMP/nodes.json" 2>/dev/null; then + if file_has_pattern "$PK_QUOTED_PATTERN" "$TMP/nodes.json" 2>/dev/null; then return 0 fi return 1 @@ -182,7 +295,11 @@ node_visible() { # -init /dev/null ignore the operator's ~/.sqliterc — a stray .mode there # would make the count unparseable # -noheader -list stdout is exactly the number, nothing else -SQLITE_ARGS=(-batch -bail -init /dev/null -noheader -list) +# -readonly never create or modify the database. A path that does not +# exist fails "unable to open database" instead of leaving an +# empty file behind. db_path_ok checks the path first; this is +# the second, independent guard. +SQLITE_ARGS=(-batch -bail -init /dev/null -noheader -list -readonly) # Round-trip probe token. The value is arbitrary; it only has to come back intact. SQLITE_PROBE_TOKEN="corescope-probe-ok" SQLITE_RUNNER="" # "container" | "host", set by resolve_sqlite_runner @@ -231,70 +348,127 @@ sqlite_probe_sql() { printf 'SELECT :probe;\n' } +# Probe one runner. Stderr is collected rather than discarded, but only printed +# if no runner qualifies. +probe_runner() { + local runner="$1" probe out q_container + probe=$(sqlite_probe_sql) + printf -v q_container '%q' "$TARGET_CONTAINER" + case "$runner" in + container) out=$(ssh_t "docker exec -i $q_container sqlite3 ${SQLITE_ARGS[*]} :memory:" \ + <<<"$probe" 2>>"$TMP/sqlite-probe.err") ;; + host) out=$(ssh_t "sqlite3 ${SQLITE_ARGS[*]} :memory:" <<<"$probe" 2>>"$TMP/sqlite-probe.err") ;; + *) return 1 ;; + esac + [[ "$out" == "$SQLITE_PROBE_TOKEN" ]] +} + # Find a sqlite3 that can bind a parameter — in the container first, then on the # host. Sets SQLITE_RUNNER; returns 1 if neither qualifies. There is deliberately # no interpolating fallback: that would leave the vulnerable path in place under # a nicer name. # -# Probe stderr is collected rather than discarded, but only printed if BOTH -# probes fail. The container miss is the known-normal case — the app image has -# no sqlite3 (pure-Go driver, no CGO; Dockerfile:15) — so surfacing it on every -# run would be noise. +# A runner is only a candidate if its own path is configured: the container is +# tried only with TARGET_CONTAINER_DB_PATH set, the host only with +# TARGET_HOST_DB_PATH. The container miss is the known-normal case — the app +# image has no sqlite3 (pure-Go driver, no CGO; Dockerfile:15) — so it falls +# back to the host when a host path is configured, and is not surfaced unless +# nothing qualifies. resolve_sqlite_runner() { - local probe out - probe=$(sqlite_probe_sql) SQLITE_RUNNER="" - out=$(ssh_t "docker exec -i $(printf %q "$TARGET_CONTAINER") sqlite3 ${SQLITE_ARGS[*]} :memory:" \ - <<<"$probe" 2>>"$TMP/sqlite-probe.err") - if [[ "$out" == "$SQLITE_PROBE_TOKEN" ]]; then SQLITE_RUNNER="container"; return 0; fi - out=$(ssh_t "sqlite3 ${SQLITE_ARGS[*]} :memory:" <<<"$probe" 2>>"$TMP/sqlite-probe.err") - if [[ "$out" == "$SQLITE_PROBE_TOKEN" ]]; then SQLITE_RUNNER="host"; return 0; fi + if [[ -n "$TARGET_CONTAINER_DB_PATH" ]] && probe_runner container; then + SQLITE_RUNNER="container"; return 0 + fi + if [[ -n "$TARGET_HOST_DB_PATH" ]] && probe_runner host; then + SQLITE_RUNNER="host"; return 0 + fi return 1 } -# Run SQL from stdin against TARGET_DB_PATH via the resolved runner. Stderr is -# left alone so the caller can capture it, and the exit status is sqlite3's. +# The db path for the resolved runner, and the env var it came from, for +# messages. Each runner has exactly one path; there is no fallback from one to +# the other. +runner_db_path() { + case "$SQLITE_RUNNER" in + container) printf '%s' "$TARGET_CONTAINER_DB_PATH" ;; + host) printf '%s' "$TARGET_HOST_DB_PATH" ;; + *) return 1 ;; + esac +} +runner_db_var() { + case "$SQLITE_RUNNER" in + container) printf 'TARGET_CONTAINER_DB_PATH' ;; + host) printf 'TARGET_HOST_DB_PATH' ;; + *) return 1 ;; + esac +} + +# Is the runner's db path a non-empty regular file in the runner's own +# environment? Checked before any query, so a wrong path is named as such rather +# than surfacing as a sqlite error — and so sqlite3 is never pointed at it. +# Stdin is closed: nothing crosses for a test. +db_path_ok() { + local q_container q_path + printf -v q_container '%q' "$TARGET_CONTAINER" + case "$SQLITE_RUNNER" in + container) printf -v q_path '%q' "$TARGET_CONTAINER_DB_PATH" + ssh_t "docker exec $q_container sh -c 'test -f \"\$1\" && test -s \"\$1\"' sh $q_path" &2; return 127 ;; + container) printf -v q_path '%q' "$TARGET_CONTAINER_DB_PATH" + ssh_t "docker exec -i $q_container sqlite3 ${SQLITE_ARGS[*]} $q_path" ;; + host) printf -v q_path '%q' "$TARGET_HOST_DB_PATH" + ssh_t "sqlite3 ${SQLITE_ARGS[*]} $q_path" ;; + *) warn "run_sqlite: no runner resolved"; return 127 ;; esac } # Read the retained-transmission count into RETAIN_COUNT. Prints a classified # "retain-failed" line and returns 1 on failure, so §10.2 has exactly one place # that increments $fails. +# +# SQLite is the only source. CoreScope has no HTTP endpoint that counts a +# blacklisted node's transmissions, and adding one for a QA script would be a +# production change; the ADMIN_API_TOKEN path this replaced called an endpoint +# that never existed. read_retain_count() { RETAIN_COUNT="" - local code - if [[ -n "$ADMIN_API_TOKEN" ]]; then - # Read auth header from stdin so the token never enters argv (ps-safe). - code=$(printf 'header = "Authorization: Bearer %s"\n' "$ADMIN_API_TOKEN" | \ - curl -s -m "$CURL_TIMEOUT" -K - -o "$TMP/admin.json" -w "%{http_code}" \ - "$TARGET_URL/api/admin/transmissions?from_node=$TEST_PUBKEY&count=1" 2>/dev/null || echo "000") - if [[ "$code" == "200" ]]; then - RETAIN_COUNT=$(jq -r '.count // ((.transmissions // []) | length)' "$TMP/admin.json" 2>/dev/null || echo "") - fi - if [[ -n "$RETAIN_COUNT" ]]; then return 0; fi - fi - - if [[ -z "$TARGET_DB_PATH" ]]; then - echo " ❌ retain-failed: TARGET_DB_PATH unset and no ADMIN_API_TOKEN — cannot probe" + if [[ -z "$TARGET_CONTAINER_DB_PATH" && -z "$TARGET_HOST_DB_PATH" ]]; then + say " ❌ retain-failed: no db path — set TARGET_CONTAINER_DB_PATH and/or TARGET_HOST_DB_PATH" return 1 fi if ! resolve_sqlite_runner; then - echo " ❌ retain-failed: no sqlite3 able to bind a parameter on the target" - echo " tried: docker exec -i $TARGET_CONTAINER sqlite3, then sqlite3 on $TARGET_SSH_HOST" - echo " need: the sqlite3 CLI reachable over ssh, supporting '.parameter set'" + say " ❌ retain-failed: no sqlite3 able to bind a parameter on the target" + [[ -n "$TARGET_CONTAINER_DB_PATH" ]] && say " tried: docker exec -i $TARGET_CONTAINER sqlite3 (TARGET_CONTAINER_DB_PATH set)" + [[ -n "$TARGET_HOST_DB_PATH" ]] && say " tried: sqlite3 on $TARGET_SSH_HOST (TARGET_HOST_DB_PATH set)" + say " need: the sqlite3 CLI reachable over ssh, supporting '.parameter set' and -readonly" cat "$TMP/sqlite-probe.err" >&2 return 1 fi - echo " sqlite3 runner: $SQLITE_RUNNER" + say " sqlite3 runner: $SQLITE_RUNNER ($(runner_db_var))" + local path_rc=0 + db_path_ok || path_rc=$? + if (( path_rc == 255 )); then + say " ❌ retain-failed: ssh failed while checking $(runner_db_var) (exit 255)" + return 1 + elif (( path_rc != 0 )); then + say " ❌ retain-failed: $(runner_db_var)=$(runner_db_path) is not a non-empty file in the $SQLITE_RUNNER" + return 1 + fi if ! RETAIN_COUNT=$(run_sqlite <<<"$(transmission_count_sql "$TEST_PUBKEY")" 2>"$TMP/sqlite.err"); then - echo " ❌ retain-failed: sqlite3 query failed via $SQLITE_RUNNER" + say " ❌ retain-failed: sqlite3 query failed via $SQLITE_RUNNER" cat "$TMP/sqlite.err" >&2 RETAIN_COUNT="" return 1 @@ -309,7 +483,7 @@ main() { BASELINE_URL="${1:-}" TARGET_URL="${2:-}" if [[ -z "$BASELINE_URL" || -z "$TARGET_URL" ]]; then - echo "usage: $0 BASELINE_URL TARGET_URL (TEST_NODE_PUBKEY+TARGET_* via env)" >&2 + warn "usage: $0 BASELINE_URL TARGET_URL (TEST_NODE_PUBKEY+TARGET_* via env)" exit 2 fi @@ -318,11 +492,26 @@ main() { TARGET_SSH_KEY="${TARGET_SSH_KEY:-/root/.ssh/id_ed25519}" TARGET_CONFIG_PATH="${TARGET_CONFIG_PATH:-}" TARGET_CONTAINER="${TARGET_CONTAINER:-}" - TARGET_DB_PATH="${TARGET_DB_PATH:-}" - ADMIN_API_TOKEN="${ADMIN_API_TOKEN:-}" + TARGET_CONTAINER_DB_PATH="${TARGET_CONTAINER_DB_PATH:-}" + TARGET_HOST_DB_PATH="${TARGET_HOST_DB_PATH:-}" if [[ -z "$TEST_PUBKEY" || -z "$TARGET_SSH_HOST" || -z "$TARGET_CONFIG_PATH" || -z "$TARGET_CONTAINER" ]]; then - echo "error: TEST_NODE_PUBKEY, TARGET_SSH_HOST, TARGET_CONFIG_PATH, TARGET_CONTAINER are required" >&2 + warn "error: TEST_NODE_PUBKEY, TARGET_SSH_HOST, TARGET_CONFIG_PATH, TARGET_CONTAINER are required" + exit 2 + fi + + # Removed variables fail the run before anything touches the target. Ignoring + # them would run a different probe from the one the operator configured; the + # values themselves are never printed. + if [[ -n "${TARGET_DB_PATH:-}" ]]; then + warn "error: TARGET_DB_PATH is no longer read — it named one path for two filesystems." + warn " Set TARGET_CONTAINER_DB_PATH (inside TARGET_CONTAINER) and/or" + warn " TARGET_HOST_DB_PATH (on TARGET_SSH_HOST)." + exit 2 + fi + if [[ -n "${ADMIN_API_TOKEN:-}" ]]; then + warn "error: ADMIN_API_TOKEN is no longer read — /api/admin/transmissions never existed;" + warn " §10.2 is SQLite-only (TARGET_CONTAINER_DB_PATH / TARGET_HOST_DB_PATH)." exit 2 fi @@ -332,21 +521,30 @@ main() { # (issue #1977). Keep it: redundant is not the same as wrong. # Pubkey must be hex (MeshCore pubkeys are hex-encoded ed25519 prefixes). if ! [[ "$TEST_PUBKEY" =~ ^[0-9a-fA-F]+$ ]]; then - echo "error: TEST_NODE_PUBKEY must be hex (got: redacted)" >&2 + warn "error: TEST_NODE_PUBKEY must be hex (got: redacted)" exit 2 fi # Container name must match docker's allowed chars: [a-zA-Z0-9][a-zA-Z0-9_.-]* if ! [[ "$TARGET_CONTAINER" =~ ^[a-zA-Z0-9][a-zA-Z0-9_.-]*$ ]]; then - echo "error: TARGET_CONTAINER has illegal chars" >&2 + warn "error: TARGET_CONTAINER has illegal chars" exit 2 fi # Config path must be an absolute, sane path (no spaces, quotes, $, ;, etc.). if ! [[ "$TARGET_CONFIG_PATH" =~ ^/[A-Za-z0-9_./-]+$ ]]; then - echo "error: TARGET_CONFIG_PATH must be a sane absolute path" >&2 + warn "error: TARGET_CONFIG_PATH must be a sane absolute path" exit 2 fi - if [[ -n "$TARGET_DB_PATH" ]] && ! [[ "$TARGET_DB_PATH" =~ ^/[A-Za-z0-9_./-]+$ ]]; then - echo "error: TARGET_DB_PATH must be a sane absolute path" >&2 + local var + for var in TARGET_CONTAINER_DB_PATH TARGET_HOST_DB_PATH; do + if [[ -n "${!var}" ]] && ! [[ "${!var}" =~ ^/[A-Za-z0-9_./-]+$ ]]; then + warn "error: $var must be a sane absolute path" + exit 2 + fi + done + # TARGET_URL reaches curl inside a quoted config line (-K -); keep it plain. + local bad_url_re='[[:space:][:cntrl:]"\\]' + if ! [[ "$TARGET_URL" =~ ^https?:// ]] || [[ "$TARGET_URL" =~ $bad_url_re ]]; then + warn "error: TARGET_URL must be an http(s) URL without whitespace, quotes or backslashes" exit 2 fi @@ -358,15 +556,19 @@ main() { SSH_OPTS=(-i "$TARGET_SSH_KEY" -o StrictHostKeyChecking=accept-new -o ConnectTimeout=15 -o BatchMode=yes -o ServerAliveInterval=15 -o ServerAliveCountMax=4) + # Everything the run writes locally is private to it: $TMP is mode 700, and + # the pattern files and responses inside are mode 600. + umask 077 TMP=$(mktemp -d) fails=0 TEARDOWN_DONE=0 install_teardown_traps + write_pubkey_patterns # --------------------------------------------------------------------------- # §10.1 — hide # --------------------------------------------------------------------------- - echo "=== §10.1 add $TEST_PUBKEY to nodeBlacklist ===" + say "=== §10.1 add $TEST_PUBKEY to nodeBlacklist ===" if ! add_to_blacklist; then fails=$((fails+1)); exit "$fails"; fi if ! restart_target; then fails=$((fails+1)); exit "$fails"; fi if ! wait_for_stats; then fails=$((fails+1)); exit "$fails"; fi @@ -374,43 +576,55 @@ main() { detail_code=$(fetch_code "$TARGET_URL/api/nodes/$TEST_PUBKEY" "$TMP/detail.json") list_code=$(fetch_code "$TARGET_URL/api/nodes?limit=10000" "$TMP/list.json") in_list=0 - if [[ "$list_code" == "200" ]] && grep -qF -- "\"$TEST_PUBKEY\"" "$TMP/list.json"; then - in_list=1 + if [[ "$list_code" == "200" ]]; then + file_has_pattern "$PK_QUOTED_PATTERN" "$TMP/list.json" + case $? in + 0) in_list=1 ;; + 1) in_list=0 ;; + *) in_list=error ;; + esac fi - if [[ "$detail_code" == "404" || "$in_list" == "0" ]]; then - echo " ✅ hide ok: detail=$detail_code in_list=$in_list" + if [[ "$in_list" == "error" ]]; then + say " ❌ hide-failed: could not search /api/nodes response (grep error)" + fails=$((fails+1)) + elif [[ "$detail_code" == "404" || "$in_list" == "0" ]]; then + say " ✅ hide ok: detail=$detail_code in_list=$in_list" else - echo " ❌ hide-failed: detail=$detail_code in_list=$in_list — pubkey still surfaced" + say " ❌ hide-failed: detail=$detail_code in_list=$in_list — pubkey still surfaced" fails=$((fails+1)) fi topo_code=$(fetch_code "$TARGET_URL/api/topology" "$TMP/topo.json") if [[ "$topo_code" != "200" ]]; then - echo " ⚠️ /api/topology HTTP $topo_code — skipping topology assertion" - elif grep -qF -- "$TEST_PUBKEY" "$TMP/topo.json"; then - echo " ❌ hide-failed: /api/topology references blacklisted pubkey" - fails=$((fails+1)) + say " ⚠️ /api/topology HTTP $topo_code — skipping topology assertion" else - echo " ✅ topology clean" + file_has_pattern "$PK_PATTERN" "$TMP/topo.json" + case $? in + 0) say " ❌ hide-failed: /api/topology references blacklisted pubkey" + fails=$((fails+1)) ;; + 1) say " ✅ topology clean" ;; + *) say " ❌ hide-failed: could not search /api/topology response (grep error)" + fails=$((fails+1)) ;; + esac fi # --------------------------------------------------------------------------- # §10.2 — DB retain # --------------------------------------------------------------------------- - echo "=== §10.2 verify packets retained in DB ===" + say "=== §10.2 verify packets retained in DB ===" if ! read_retain_count; then # read_retain_count already printed the classified reason. Counting here and - # nowhere else: the old code incremented $fails for the "TARGET_DB_PATH - # unset" case and then again for the empty count it left behind. + # nowhere else: an older version incremented $fails for the "no db path" + # case and then again for the empty count it left behind. fails=$((fails+1)) elif [[ "$RETAIN_COUNT" =~ ^[0-9]+$ ]] && (( RETAIN_COUNT > 0 )); then - echo " ✅ DB retains $RETAIN_COUNT packets from $TEST_PUBKEY" + say " ✅ DB retains $RETAIN_COUNT packets from $TEST_PUBKEY" else - echo " ❌ retain-failed: count=$RETAIN_COUNT (expected > 0)" + say " ❌ retain-failed: count=$RETAIN_COUNT (expected > 0)" fails=$((fails+1)) fi - echo "=== summary: $fails failure(s) before teardown ===" + say "=== summary: $fails failure(s) before teardown ===" # trap handles teardown + exit exit "$fails" } diff --git a/qa/scripts/test-blacklist-sql.sh b/qa/scripts/test-blacklist-sql.sh index de24fe1f0..0716bd978 100755 --- a/qa/scripts/test-blacklist-sql.sh +++ b/qa/scripts/test-blacklist-sql.sh @@ -16,15 +16,47 @@ # committed staging-captured test-fixtures/e2e-fixture.db. An earlier version # invented a `from_node` column that no CoreScope database has, and so passed # while the real probe could never succeed. +# +# The fake-target group (issue #83) runs the whole script, unmodified, against +# fake ssh/docker/curl with every other command behind an argv-logging shim, +# and checks the runner contract end to end: no pubkey, SQL, URL or token in +# any process's argv; each sqlite runner reads only its own db path, and a +# wrong path is refused without creating a file; removed variables refuse to +# start; and every success, failure, signal and broken-stream run restores +# the config, leaves the databases and filesystem as they were, and exits with +# its classified status. It needs sqlite3, jq and (for the broken-stream +# cases) perl. +# ShellCheck (run with -x so the sourced script's globals are known): +# SC2016 — single-quoted $… is literal on purpose: injection payloads, and +# code for a child bash/perl to expand. +# SC2030/SC2031 — the fake-target runs set PATH and FAKE_* inside ( … ) on +# purpose, so nothing leaks into the next run. +# shellcheck disable=SC2016,SC2030,SC2031 set -uo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" REPO_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" INGESTOR_DB_GO="$REPO_ROOT/cmd/ingestor/db.go" REAL_FIXTURE="$REPO_ROOT/test-fixtures/e2e-fixture.db" +# BLACKLIST_TEST_SH points the suite at another copy of the script (used to +# check that deliberately broken variants fail it). +BLACKLIST_SH="${BLACKLIST_TEST_SH:-$SCRIPT_DIR/blacklist-test.sh}" # shellcheck source=blacklist-test.sh -. "$SCRIPT_DIR/blacklist-test.sh" +. "$BLACKLIST_SH" + +# Stream breaker: `perl -e "$PERL_BREAK" FD cmd...` runs cmd with FD (1 or 2) +# on a pipe whose reader is already closed, so the first write to it raises +# SIGPIPE — deterministically, unlike a pipe into a process that exits "soon". +# SIGPIPE is reset to default first, in case this suite itself was started with +# it ignored. `perl -e "$PERL_IGNORE" FD cmd...` does the same but starts cmd +# with SIGPIPE ignored, which bash cannot trap. +PERL_BREAK='my $fd = shift; $SIG{PIPE} = "DEFAULT"; pipe(my $r, my $w) or die "pipe: $!"; close $r; + if ($fd == 1) { open(STDOUT, ">&", $w) or die "dup: $!" } else { open(STDERR, ">&", $w) or die "dup: $!" } + close $w; exec { $ARGV[0] } @ARGV or die "exec: $!"' +PERL_IGNORE='my $fd = shift; $SIG{PIPE} = "IGNORE"; pipe(my $r, my $w) or die "pipe: $!"; close $r; + if ($fd == 1) { open(STDOUT, ">&", $w) or die "dup: $!" } else { open(STDERR, ">&", $w) or die "dup: $!" } + close $w; exec { $ARGV[0] } @ARGV or die "exec: $!"' PASS=0 FAIL=0 @@ -207,78 +239,505 @@ SQL assert_eq "$want → no count on stdout" "" "$out" done - # ----- runner selection and transport, with ssh stubbed --------------------- - # ssh_t is replaced by a stub that logs its argv, captures stdin, and runs the - # remote command string through a real `bash -c`, so the script's own quoting - # is what gets parsed. "docker exec -i NAME" is peeled off when the stub - # container is said to have sqlite3; otherwise it fails like the app image. - STUB_ARGV="$FIXTURE_DIR/ssh.argv"; STUB_STDIN="$FIXTURE_DIR/ssh.stdin" - STUB_CONTAINER_SQLITE=0 - STUB_PATH="$PATH" - ssh_t() { - local cmd="$1" in - printf '%s\n' "$*" >>"$STUB_ARGV" - in=$(cat); printf '%s\n' "$in" >>"$STUB_STDIN" - case "$cmd" in - "docker exec -i "*) - if [ "$STUB_CONTAINER_SQLITE" != 1 ]; then - echo 'exec: "sqlite3": executable file not found in $PATH' >&2; return 126 - fi - cmd=${cmd#docker exec -i }; cmd=${cmd#* } ;; - esac - printf '%s\n' "$in" | PATH="$STUB_PATH" bash -c "$cmd" - } - reset_stub() { : >"$STUB_ARGV"; : >"$STUB_STDIN"; } - TMP="$FIXTURE_DIR"; ADMIN_API_TOKEN=""; TARGET_CONTAINER="corescope-stub" - TARGET_SSH_HOST="stub-host"; TARGET_DB_PATH="$DB"; TEST_PUBKEY="$PK_A" - - # Container has no sqlite3 → host runner, correct count. - reset_stub - read_retain_count >/dev/null 2>&1; rc=$? - assert_eq "fallback: rc" "0" "$rc" - assert_eq "fallback: runner" "host" "$SQLITE_RUNNER" - assert_eq "fallback: count" "3" "$RETAIN_COUNT" - argv=$(cat "$STUB_ARGV"); stdin=$(cat "$STUB_STDIN") - assert_true "transport: container tried first" contains "$(head -1 "$STUB_ARGV")" "docker exec -i corescope-stub sqlite3" - assert_true "transport: pubkey not in remote argv" lacks "$argv" "$PK_A" - assert_true "transport: SQL not in remote argv" lacks "$argv" "SELECT" - assert_true "transport: column not in remote argv" lacks "$argv" "from_pubkey" - assert_true "transport: SQL arrives on stdin" contains "$stdin" "SELECT COUNT(*) FROM transmissions WHERE from_pubkey = lower(:pubkey);" - assert_true "transport: pubkey arrives hex-encoded on stdin" contains "$stdin" "$(sql_hex_literal "$PK_A")" - assert_true "transport: raw pubkey never on stdin" lacks "$stdin" "$PK_A" - - # Container has sqlite3 → container runner. - reset_stub; STUB_CONTAINER_SQLITE=1 - read_retain_count >/dev/null 2>&1; rc=$? - assert_eq "container: rc" "0" "$rc" - assert_eq "container: runner" "container" "$SQLITE_RUNNER" - assert_eq "container: count" "3" "$RETAIN_COUNT" - STUB_CONTAINER_SQLITE=0 - - # A sqlite3 that cannot bind (.parameter prints help to stdout, exit 0, - # parameter left NULL) must be rejected by the probe, not trusted. - FAKE_BIN="$FIXTURE_DIR/fakebin"; mkdir -p "$FAKE_BIN" - cat >"$FAKE_BIN/sqlite3" <<'FAKE' + # ----- a fake target: argv, transport and the db-path contract -------------- + # The script runs unmodified against fake `ssh`, `docker` and `curl` placed + # first on PATH. Every other command the run (or the "remote" side) executes + # goes through a logging shim that records its argv and then execs the real + # binary, so the argv log is what `ps` would have shown. The fake ssh runs + # the remote command through a real `bash -c`, so the script's own quoting is + # what gets parsed; the fake docker gives the container its own filesystem + # root, so a container path and a host path really are different files. + FAKE="$FIXTURE_DIR/fake" + SHIM_DIR="$FAKE/shim"; SHIM_NOJQ_DIR="$FAKE/shim-nojq" + FAKE_STATE="$FAKE/state"; FAKE_CROOT="$FAKE/container-root" + HOST_DIR="$FAKE/host/var/lib/corescope"; FAKE_TMPDIR="$FAKE/tmp" + mkdir -p "$SHIM_DIR" "$SHIM_NOJQ_DIR" "$FAKE_STATE" "$FAKE_CROOT/srv/corescope/data" \ + "$HOST_DIR" "$FAKE_TMPDIR" "$FAKE/target" + ORIG_PATH="$PATH" + + cat >"$SHIM_DIR/logexec" <<'SHIM' +#!/bin/bash +# Record this exec's argv, then run the real command from FAKE_REAL_PATH. +name=${0##*/} +{ printf '%s' "$name"; printf ' %q' "$@"; printf '\n'; } >>"$FAKE_ARGV_LOG" +set -f; IFS=: +for d in $FAKE_REAL_PATH; do + if [ -x "$d/$name" ] && [ ! -d "$d/$name" ]; then exec "$d/$name" "$@"; fi +done +echo "logexec: $name not found" >&2; exit 127 +SHIM + chmod +x "$SHIM_DIR/logexec" + for c in bash sh cat grep od tr mktemp rm mv chmod chown date sleep jq python3 \ + sqlite3 head sed awk tee env ls cp; do + ln -s logexec "$SHIM_DIR/$c" + [ "$c" = jq ] || ln -s "$SHIM_DIR/logexec" "$SHIM_NOJQ_DIR/$c" + done + + cat >"$SHIM_DIR/ssh" <<'FAKESSH' +#!/bin/bash +# Fake ssh: log argv, keep this call's command and stdin, run the command with +# a real `bash -c` as sshd would. Remote PATH is FAKE_REMOTE_PATH. +{ printf 'ssh'; printf ' %q' "$@"; printf '\n'; } >>"$FAKE_ARGV_LOG" +while [ $# -gt 0 ]; do + case $1 in -i|-o|-p|-l|-F) shift 2 ;; -*) shift ;; *) break ;; esac +done +shift # host +n=0; [ -f "$FAKE_STATE/ssh.n" ] && n=$(<"$FAKE_STATE/ssh.n"); n=$((n + 1)); echo "$n" >"$FAKE_STATE/ssh.n" +printf '%s' "$*" >"$FAKE_STATE/ssh.$n.cmd" +if [ "${FAKE_SSH_FAIL_CMD:-}" != "" ] && [[ "$*" == *"$FAKE_SSH_FAIL_CMD"* ]]; then + fails=0; [ -f "$FAKE_STATE/ssh.failcount" ] && fails=$(<"$FAKE_STATE/ssh.failcount") + fails=$((fails + 1)); echo "$fails" >"$FAKE_STATE/ssh.failcount" + if [ "$fails" -ge "${FAKE_SSH_FAIL_FROM:-1}" ]; then + echo "ssh: connect to host stub-host port 22: Connection refused" >&2; exit 255 + fi +fi +"$FAKE_REAL_TEE" "$FAKE_STATE/ssh.$n.stdin" | PATH="$FAKE_REMOTE_PATH" bash -c "$*" +exit "${PIPESTATUS[1]}" +FAKESSH + + cat >"$SHIM_DIR/docker" <<'FAKEDOCKER' +#!/bin/bash +# Fake docker: `restart` snapshots the config's blacklist as the app's live +# state; `exec` runs the command with absolute paths mapped into the container +# root. Without FAKE_CONTAINER_SQLITE=1 the container has no sqlite3, like the +# production image. +{ printf 'docker'; printf ' %q' "$@"; printf '\n'; } >>"$FAKE_ARGV_LOG" +case $1 in + restart) + [ "$2" = "$FAKE_CONTAINER" ] || { echo "Error: No such container: $2" >&2; exit 1; } + "$FAKE_REAL_JQ" -r '.nodeBlacklist // [] | .[] | ascii_downcase' "$FAKE_CONFIG" \ + >"$FAKE_STATE/live-blacklist" || exit 1 + { echo "restart"; cat "$FAKE_STATE/live-blacklist"; } >>"$FAKE_STATE/restarts" 2>/dev/null + echo "$2" ;; + exec) + shift; [ "$1" = -i ] && shift + [ "$1" = "$FAKE_CONTAINER" ] || { echo "Error: No such container: $1" >&2; exit 1; } + shift + if [ "$1" = sqlite3 ] && [ "${FAKE_CONTAINER_SQLITE:-0}" != 1 ]; then + echo 'OCI runtime exec failed: exec: "sqlite3": executable file not found in $PATH' >&2 + exit 126 + fi + args=() + for a in "$@"; do + case $a in /dev/*) args+=("$a") ;; /*) args+=("$FAKE_CROOT$a") ;; *) args+=("$a") ;; esac + done + exec "${args[@]}" ;; + *) echo "fake docker: unsupported: $1" >&2; exit 1 ;; +esac +FAKEDOCKER + + cat >"$SHIM_DIR/curl" <<'FAKECURL' +#!/bin/bash +# Fake curl + CoreScope API, in pure bash so the fake's own work adds nothing to +# the argv log. Takes the URL from argv or from a -K - config on stdin. Can +# send a signal to the script when a given path is hit for the Nth time. +{ printf 'curl'; printf ' %q' "$@"; printf '\n'; } >>"$FAKE_ARGV_LOG" +out=/dev/stdout; fmt=""; url=""; cfg="" +while [ $# -gt 0 ]; do + case $1 in + -s) shift ;; + -m|-H) shift 2 ;; + -o) out=$2; shift 2 ;; + -w) fmt=$2; shift 2 ;; + -K) cfg=$2; shift 2 ;; + -*) shift ;; + *) url=$1; shift ;; + esac +done +if [ "$cfg" = - ]; then + while IFS= read -r line; do + case $line in 'url = "'*'"') url=${line#'url = "'}; url=${url%'"'} ;; esac + done +fi +path=${url#"$FAKE_URL"} +if [ "$path" = "$url" ]; then printf '000'; exit 7; fi + +hits_file="$FAKE_STATE/hits.${path//[^a-z]/_}" +hits=0; [ -f "$hits_file" ] && hits=$(<"$hits_file"); hits=$((hits + 1)); echo "$hits" >"$hits_file" +if [ -n "${FAKE_SIGNAL_ON:-}" ] && [ "$path" = "$FAKE_SIGNAL_ON" ] && [ "$hits" = "${FAKE_SIGNAL_NTH:-1}" ]; then + kill -s "$FAKE_SIGNAL" "$(<"$FAKE_STATE/script.pid")" +fi + +BL=(); while IFS= read -r b; do BL+=("$b"); done <"$FAKE_STATE/live-blacklist" +hidden() { + [ "${FAKE_LEAK:-0}" = 1 ] && return 1 + local b; for b in ${BL[@]+"${BL[@]}"}; do [ "$b" = "$1" ] && return 0; done; return 1 +} +code=404; body='{"error":"not found"}' +case $path in + /api/stats) code=200; body='{"ok":true}' ;; + /api/nodes/*) + want=${path#/api/nodes/} + for n in $FAKE_NODES; do + if [ "$n" = "$want" ] && ! hidden "$n"; then code=200; body="{\"public_key\":\"$n\"}"; fi + done ;; + '/api/nodes?limit=10000') + code=200; body='{"nodes":['; sep='' + for n in $FAKE_NODES; do hidden "$n" && continue; body+="$sep{\"public_key\":\"$n\"}"; sep=','; done + body+=']}' ;; + /api/topology) + code=200; body='{"nodes":['; sep='' + for n in $FAKE_NODES; do hidden "$n" && continue; body+="$sep\"$n\""; sep=','; done + body+=']}' ;; +esac +printf '%s' "$body" >"$out" +[ "$fmt" = '%{http_code}' ] && printf '%s' "$code" +exit 0 +FAKECURL + chmod +x "$SHIM_DIR/ssh" "$SHIM_DIR/docker" "$SHIM_DIR/curl" + for c in ssh docker curl; do ln -s "$SHIM_DIR/$c" "$SHIM_NOJQ_DIR/$c"; done + + # Two databases with DIFFERENT counts for PK_A, so the count names the file + # that was read: the container's holds 2, the host's holds 3. + CONTAINER_DB_PATH="/srv/corescope/data/meshcore.db" # a path inside the container + HOST_DB_PATH="$HOST_DIR/meshcore.db" # a path on the host + printf '%s\n' "$DDL" | sqlite3 "$FAKE_CROOT$CONTAINER_DB_PATH" + sqlite3 "$FAKE_CROOT$CONTAINER_DB_PATH" "INSERT INTO transmissions(raw_hex,hash,first_seen,payload_type,from_pubkey) VALUES + ('00','c1','t',4,'$PK_A'),('00','c2','t',4,'$PK_A'),('00','c3','t',4,'$PK_B');" + cp "$DB" "$HOST_DB_PATH" + cp "$LEGACY_DB" "$HOST_DIR/legacy.db" + UNBINDABLE_BIN="$FIXTURE_DIR/unbindable-bin"; mkdir -p "$UNBINDABLE_BIN" + cat >"$UNBINDABLE_BIN/sqlite3" <<'FAKE' #!/usr/bin/env bash +# A sqlite3 whose .parameter cannot bind: help text on stdout, exit 0. cat >/dev/null echo ".parameter CMD ... Manage SQL parameter bindings" echo "0" FAKE - chmod +x "$FAKE_BIN/sqlite3" - reset_stub; STUB_PATH="$FAKE_BIN:$PATH" - read_retain_count >/dev/null 2>&1; rc=$? - assert_eq "unbindable sqlite3: rejected" "1" "$rc" - assert_eq "unbindable sqlite3: no runner" "" "$SQLITE_RUNNER" - assert_eq "unbindable sqlite3: no count" "" "$RETAIN_COUNT" - STUB_PATH="$PATH" - - # A query error on the target is a failure with no count, never "0". - reset_stub; TARGET_DB_PATH="$LEGACY_DB" - read_retain_count >/dev/null 2>&1; rc=$? - assert_eq "remote schema error: rc" "1" "$rc" - assert_eq "remote schema error: count" "" "$RETAIN_COUNT" - assert_true "remote schema error: stderr kept" contains "$(cat "$TMP/sqlite.err")" "no such column: from_pubkey" - TARGET_DB_PATH="$DB" + chmod +x "$UNBINDABLE_BIN/sqlite3" + + FAKE_CONFIG="$FAKE/target/config.json" + ORIG_CONFIG='{"port":3000,"nodeBlacklist":["aa00aa00"],"mqtt":{"sources":[]}}' + SYNTH_TOKEN="synthetic-admin-token-not-a-secret-83" + export FAKE_ARGV_LOG="$FAKE/argv.log" FAKE_STATE FAKE_CROOT FAKE_CONFIG + export FAKE_REAL_TEE; FAKE_REAL_TEE=$(command -v tee) + export FAKE_REAL_JQ; FAKE_REAL_JQ=$(command -v jq) + export FAKE_CONTAINER="corescope-stub" FAKE_URL="http://target.invalid" + export FAKE_NODES="$PK_A $PK_B" + ALL_ARGV="$FAKE/all-argv.log"; : >"$ALL_ARGV" + + snapshot_files() { (cd "$FAKE" && find host container-root tmp target -print | LC_ALL=C sort); } + db_sums() { cksum "$FAKE_CROOT$CONTAINER_DB_PATH" "$HOST_DB_PATH" | awk '{print $1, $2}'; } + DB_SUMS=$(db_sums) + + # Run the whole script against the fake target. Settings come as NAME=VALUE + # arguments; `--` then an optional wrapper command (e.g. a stream breaker). + # Sets RUN_RC, RUN_OUT, RUN_ERR. + run_full() { + local kv wrap=() + rm -rf "$FAKE_STATE"; mkdir -p "$FAKE_STATE"; : >"$FAKE_STATE/live-blacklist" + printf '%s\n' "$ORIG_CONFIG" >"$FAKE_CONFIG" + : >"$FAKE_ARGV_LOG" + FILES_BEFORE=$(snapshot_files) + ( + unset TARGET_DB_PATH ADMIN_API_TOKEN TARGET_CONTAINER_DB_PATH TARGET_HOST_DB_PATH \ + FAKE_CONTAINER_SQLITE FAKE_SIGNAL_ON FAKE_SIGNAL FAKE_SIGNAL_NTH FAKE_LEAK \ + FAKE_SSH_FAIL_CMD FAKE_SSH_FAIL_FROM + export TEST_NODE_PUBKEY="$PK_A" TARGET_SSH_HOST="stub-host" TARGET_SSH_KEY="/nonexistent/key" \ + TARGET_CONFIG_PATH="$FAKE_CONFIG" TARGET_CONTAINER="$FAKE_CONTAINER" \ + CURL_TIMEOUT=2 RESTART_WAIT_S=4 TMPDIR="$FAKE_TMPDIR" \ + FAKE_REAL_PATH="$ORIG_PATH" FAKE_REMOTE_PATH="$SHIM_DIR" + while [ $# -gt 0 ] && [ "$1" != -- ]; do kv=$1; export "${kv?}"; shift; done + [ "${1:-}" = -- ] && shift + wrap=("$@") + export PATH="$SHIM_DIR:$ORIG_PATH" + # Foreground, so SIGINT is not ignored the way it is for a background + # job; the pid is recorded for the fake curl to signal. + ${wrap[@]+"${wrap[@]}"} bash -c 'echo $$ >"$FAKE_STATE/script.pid"; exec bash "$@"' _ \ + "$BLACKLIST_SH" "http://baseline.invalid" "$FAKE_URL" \ + >"$FAKE/out" 2>"$FAKE/err" + ) + RUN_RC=$? + RUN_OUT=$(cat "$FAKE/out"); RUN_ERR=$(cat "$FAKE/err") + cat "$FAKE_ARGV_LOG" >>"$ALL_ARGV" + } + restarts() { + if [ -f "$FAKE_STATE/restarts" ]; then awk '$0 == "restart" { n++ } END { print n + 0 }' "$FAKE_STATE/restarts" + else echo 0; fi + } + config_restored() { [ "$(jq -S . "$FAKE_CONFIG")" = "$(printf '%s' "$ORIG_CONFIG" | jq -S .)" ]; } + files_unchanged() { [ "$(snapshot_files)" = "$FILES_BEFORE" ]; } + argv_has() { grep -qF -- "$1" "$FAKE_ARGV_LOG"; } + # The ssh call whose command contains $1: prints its number. + ssh_call() { + local n=1 + while [ -f "$FAKE_STATE/ssh.$n.cmd" ]; do + if [[ "$(cat "$FAKE_STATE/ssh.$n.cmd")" == *"$1"* ]]; then echo "$n"; return 0; fi + n=$((n + 1)) + done + return 1 + } + # Assertions every run must meet: target restored exactly, nothing left + # behind, and nothing sensitive in any process's argv. + common_after() { + local label="$1" + assert_true "$label: config restored" config_restored + assert_eq "$label: databases unchanged" "$DB_SUMS" "$(db_sums)" + assert_true "$label: no new or leftover files" files_unchanged + assert_true "$label: argv log recorded the run" test -s "$FAKE_ARGV_LOG" + assert_true "$label: pubkey not in any argv" lacks "$(cat "$FAKE_ARGV_LOG")" "$PK_A" + assert_true "$label: SQL not in any argv" lacks "$(cat "$FAKE_ARGV_LOG")" "SELECT" + assert_true "$label: token not in any argv" lacks "$(cat "$FAKE_ARGV_LOG")" "$SYNTH_TOKEN" + assert_true "$label: remote input exactly as built" remote_input_clean + } + # Every ssh call carries only what the script built: config edits have the + # fixed command line and start their stdin with the read line; sqlite3 + # calls start their stdin with .parameter init. Stray output (bash 3.2's + # leftover stdout buffer) shows up here as extra leading lines. + remote_input_clean() { + local n=1 cmd first + while [ -f "$FAKE_STATE/ssh.$n.cmd" ]; do + cmd=$(cat "$FAKE_STATE/ssh.$n.cmd"); first=$(head -n 1 "$FAKE_STATE/ssh.$n.stdin") + case $cmd in + *"bash -s") + [[ "$cmd" =~ ^CFG=[^[:space:]]+\ MODE=(add|remove)\ bash\ -s$ ]] || { echo " unexpected edit cmd: $cmd" >&2; return 1; } + [ "$first" = "IFS= read -r PK || exit 3" ] || { echo " unexpected edit stdin: $first" >&2; return 1; } ;; + *sqlite3*) + [ "$first" = ".parameter init" ] || { echo " unexpected sqlite stdin: $first" >&2; return 1; } ;; + *) + [ ! -s "$FAKE_STATE/ssh.$n.stdin" ] || { echo " unexpected stdin for: $cmd" >&2; return 1; } ;; + esac + n=$((n + 1)) + done + } + + # 1. Container has no sqlite3 → host fallback with the HOST path (count 3). + # Container and host paths differ. Also the stdin-transport evidence. + run_full TARGET_CONTAINER_DB_PATH="$CONTAINER_DB_PATH" TARGET_HOST_DB_PATH="$HOST_DB_PATH" + assert_eq "host fallback: exit 0" "0" "$RUN_RC" + assert_true "host fallback: runner named" contains "$RUN_OUT" "sqlite3 runner: host (TARGET_HOST_DB_PATH)" + assert_true "host fallback: host db count" contains "$RUN_OUT" "DB retains 3 packets" + assert_true "host fallback: hidden" contains "$RUN_OUT" "hide ok: detail=404 in_list=0" + assert_true "host fallback: topology" contains "$RUN_OUT" "topology clean" + assert_true "host fallback: teardown ok" contains "$RUN_OUT" "teardown ok" + assert_eq "host fallback: two restarts" "2" "$(restarts)" + # The pubkey reached the app through the stdin edit: the first restart saw + # it in the blacklist, the second (teardown) did not. + assert_eq "stdin edit: added, then removed" "restart $PK_A aa00aa00 restart aa00aa00" \ + "$(tr '\n' ' ' <"$FAKE_STATE/restarts" | sed 's/ $//')" + common_after "host fallback" + assert_true "host fallback: container path never used" lacks "$(cat "$FAKE_ARGV_LOG")" "$CONTAINER_DB_PATH" + assert_true "host fallback: host path used" argv_has "$HOST_DB_PATH" + assert_true "host fallback: container tried first" contains "$(cat "$FAKE_STATE/ssh.$(ssh_call ':memory:').cmd")" "docker exec -i corescope-stub sqlite3" + # Coverage of the argv log itself: every kind of call is in it. + for c in 'ssh ' 'curl ' 'grep ' 'jq ' 'sqlite3 ' 'docker ' 'bash -c'; do + assert_true "argv log covers: $c" grep -q "^$c" "$FAKE_ARGV_LOG" + done + assert_true "grep reads its pattern from a file" grep -q '^grep -qF -f .*pubkey' "$FAKE_ARGV_LOG" + assert_true "curl reads its URL from stdin" grep -q '^curl .*-K -' "$FAKE_ARGV_LOG" + assert_true "no URL in curl argv" lacks "$(grep '^curl ' "$FAKE_ARGV_LOG")" "$FAKE_URL" + edit=$(ssh_call "bash -s") + assert_eq "config-edit command carries no value" \ + "CFG=$FAKE_CONFIG MODE=add bash -s" "$(cat "$FAKE_STATE/ssh.$edit.cmd")" + assert_eq "config-edit stdin: pubkey is line 2" "$PK_A" "$(sed -n 2p "$FAKE_STATE/ssh.$edit.stdin")" + query=$(ssh_call "-readonly $HOST_DB_PATH") + qin=$(cat "$FAKE_STATE/ssh.$query.stdin") + assert_true "sql stdin: the count query" contains "$qin" "SELECT COUNT(*) FROM transmissions WHERE from_pubkey = lower(:pubkey);" + assert_true "sql stdin: pubkey hex-encoded" contains "$qin" "$(sql_hex_literal "$PK_A")" + assert_true "sql stdin: raw pubkey absent" lacks "$qin" "$PK_A" + assert_true "path check before query" test "$(ssh_call "test -f $HOST_DB_PATH")" -lt "$query" + + # 2. Container has sqlite3 → container runner with the CONTAINER path + # (count 2). A valid host db is configured too and must not be read. + run_full TARGET_CONTAINER_DB_PATH="$CONTAINER_DB_PATH" TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_CONTAINER_SQLITE=1 + assert_eq "container runner: exit 0" "0" "$RUN_RC" + assert_true "container runner: named" contains "$RUN_OUT" "sqlite3 runner: container (TARGET_CONTAINER_DB_PATH)" + assert_true "container runner: container db count" contains "$RUN_OUT" "DB retains 2 packets" + assert_true "container runner: host path never used" lacks "$(cat "$FAKE_ARGV_LOG")" "$HOST_DB_PATH" + assert_true "container runner: path checked in the container" \ + argv_has "docker exec corescope-stub sh -c" + common_after "container runner" + + # 3. Only a container path, container has no sqlite3 → fail; the host is + # never tried with the container's path. + run_full TARGET_CONTAINER_DB_PATH="$CONTAINER_DB_PATH" + assert_eq "container path only: exit 1" "1" "$RUN_RC" + assert_true "container path only: classified" contains "$RUN_OUT" "retain-failed: no sqlite3 able to bind" + assert_true "container path only: probe error shown" contains "$RUN_ERR" '"sqlite3": executable file not found' + assert_true "container path only: host not probed" lacks "$(grep '^bash -c sqlite3' "$FAKE_ARGV_LOG")" "sqlite3" + common_after "container path only" + + # 4. Host path that does not exist → named as such before any query, and + # no empty database appears. + run_full TARGET_HOST_DB_PATH="$HOST_DIR/missing.db" + assert_eq "wrong host path: exit 1" "1" "$RUN_RC" + assert_true "wrong host path: classified" contains "$RUN_OUT" "TARGET_HOST_DB_PATH=$HOST_DIR/missing.db is not a non-empty file in the host" + assert_true "wrong host path: no file created" test ! -e "$HOST_DIR/missing.db" + assert_true "wrong host path: sqlite3 never opened it" lacks "$(grep '^sqlite3 ' "$FAKE_ARGV_LOG")" "missing.db" + common_after "wrong host path" + + # 5. Wrong container path with a sqlite3-capable container → no fallback to + # the host, no file created in the container. + run_full TARGET_CONTAINER_DB_PATH="/srv/corescope/data/missing.db" TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_CONTAINER_SQLITE=1 + assert_eq "wrong container path: exit 1" "1" "$RUN_RC" + assert_true "wrong container path: classified" contains "$RUN_OUT" "TARGET_CONTAINER_DB_PATH=/srv/corescope/data/missing.db is not a non-empty file in the container" + assert_true "wrong container path: no file created" test ! -e "$FAKE_CROOT/srv/corescope/data/missing.db" + assert_true "wrong container path: host db not read instead" lacks "$(cat "$FAKE_ARGV_LOG")" "$HOST_DB_PATH" + common_after "wrong container path" + + # 6. No path at all → classified, no sqlite3 anywhere. + run_full + assert_eq "no db path: exit 1" "1" "$RUN_RC" + assert_true "no db path: classified" contains "$RUN_OUT" "retain-failed: no db path" + assert_true "no db path: sqlite3 never run" lacks "$(cat "$FAKE_ARGV_LOG")" "sqlite3" + common_after "no db path" + + # 7. Query error (a db without from_pubkey) → failure with no count. + run_full TARGET_HOST_DB_PATH="$HOST_DIR/legacy.db" + assert_eq "query error: exit 1" "1" "$RUN_RC" + assert_true "query error: classified" contains "$RUN_OUT" "retain-failed: sqlite3 query failed via host" + assert_true "query error: sqlite error kept" contains "$RUN_ERR" "no such column: from_pubkey" + assert_true "query error: no count claimed" lacks "$RUN_OUT" "DB retains" + common_after "query error" + + # 8. A sqlite3 that cannot bind → rejected by the probe, not trusted. + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_REAL_PATH="$UNBINDABLE_BIN:$ORIG_PATH" + assert_eq "unbindable sqlite3: exit 1" "1" "$RUN_RC" + assert_true "unbindable sqlite3: classified" contains "$RUN_OUT" "retain-failed: no sqlite3 able to bind" + common_after "unbindable sqlite3" + + # 9. No jq on the target → python3 edit path, pubkey via environment. + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_REMOTE_PATH="$SHIM_NOJQ_DIR" + assert_eq "python3 edit: exit 0" "0" "$RUN_RC" + assert_true "python3 edit: python3 did the edit" grep -q '^python3 - ' "$FAKE_ARGV_LOG" + assert_true "python3 edit: jq not used remotely" lacks "$(cat "$FAKE_ARGV_LOG")" "jq " + common_after "python3 edit" + + # 10. Removed variables refuse to start: nothing runs against the target. + for removed in "TARGET_DB_PATH=$HOST_DB_PATH" "ADMIN_API_TOKEN=$SYNTH_TOKEN"; do + name=${removed%%=*} + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" "$removed" + assert_eq "$name: exit 2" "2" "$RUN_RC" + assert_true "$name: named as removed" contains "$RUN_ERR" "$name is no longer read" + assert_true "$name: no ssh" lacks "$(cat "$FAKE_ARGV_LOG")" "ssh " + assert_true "$name: no curl" lacks "$(cat "$FAKE_ARGV_LOG")" "curl " + assert_eq "$name: no restart" "0" "$(restarts)" + assert_true "$name: value never printed" lacks "$RUN_OUT$RUN_ERR" "${removed#*=}" + common_after "$name" + done + # Only the refusal message may still name the endpoint; nothing calls it. + assert_true "admin API path gone: no Authorization header" lacks "$(cat "$BLACKLIST_SH")" "Authorization" + assert_true "admin API path gone: no admin query" lacks "$(cat "$BLACKLIST_SH")" "api/admin/transmissions?" + assert_true "admin API path gone: token never read" lacks "$(grep -v 'ADMIN_API_TOKEN:-}" ]]; then' "$BLACKLIST_SH" | grep -v '^ *#' | grep -v 'warn "')" 'ADMIN_API_TOKEN' + + + # 11. Hide failure on the server side is reported and still torn down. + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_LEAK=1 + assert_eq "server leaks: exit 2" "2" "$RUN_RC" + assert_true "server leaks: hide-failed" contains "$RUN_OUT" "hide-failed: detail=200 in_list=1" + assert_true "server leaks: topology hide-failed" contains "$RUN_OUT" "/api/topology references blacklisted pubkey" + common_after "server leaks" + + # 11b. grep itself failing is a failure, never "pubkey absent" (which, for + # a hide check, is a pass). + GREP_ERR_BIN="$FIXTURE_DIR/grep-err-bin"; mkdir -p "$GREP_ERR_BIN" + printf '#!/bin/sh\necho "grep: simulated I/O error" >&2\nexit 2\n' >"$GREP_ERR_BIN/grep" + chmod +x "$GREP_ERR_BIN/grep" + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_REAL_PATH="$GREP_ERR_BIN:$ORIG_PATH" + assert_eq "grep error: both searches fail" "2" "$RUN_RC" + assert_true "grep error: list classified" contains "$RUN_OUT" "could not search /api/nodes response" + assert_true "grep error: topology classified" contains "$RUN_OUT" "could not search /api/topology response" + assert_true "grep error: no hide ok claimed" lacks "$RUN_OUT" "hide ok" + common_after "grep error" + + # 12. Signals mid-run: classified exit, full teardown. + for sig in TERM:143 INT:130; do + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_SIGNAL_ON=/api/topology FAKE_SIGNAL="${sig%%:*}" + assert_eq "SIG${sig%%:*} mid-run: exit ${sig#*:}" "${sig#*:}" "$RUN_RC" + assert_true "SIG${sig%%:*} mid-run: teardown ok" contains "$RUN_OUT" "teardown ok" + assert_eq "SIG${sig%%:*} mid-run: two restarts" "2" "$(restarts)" + common_after "SIG${sig%%:*} mid-run" + done + + # 13. SIGTERM mid-run and a failing teardown restart → 143 + 1. + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_SIGNAL_ON=/api/topology FAKE_SIGNAL=TERM \ + FAKE_SSH_FAIL_CMD="docker restart" FAKE_SSH_FAIL_FROM=2 + assert_eq "SIGTERM + failed teardown: exit 144" "144" "$RUN_RC" + assert_true "SIGTERM + failed teardown: classified" contains "$RUN_OUT" "teardown-failed" + + # 14. A second signal during teardown's stats wait: the restore finishes + # and the run's status is kept. + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_SIGNAL_ON=/api/stats FAKE_SIGNAL_NTH=2 FAKE_SIGNAL=INT + assert_eq "INT in teardown: status kept" "0" "$RUN_RC" + assert_true "INT in teardown: noted" contains "$RUN_ERR" "teardown continues" + assert_true "INT in teardown: teardown ok" contains "$RUN_OUT" "teardown ok" + common_after "INT in teardown" + + # 15. Broken output streams (see the stream-breaker below). + if command -v perl >/dev/null 2>&1; then + # Only stderr broken, failing run: diagnostics are lost, the status and + # the restore are not. + run_full TARGET_HOST_DB_PATH="$HOST_DIR/legacy.db" -- perl -e "$PERL_BREAK" 2 + assert_eq "stderr broken, query error: exit 1" "1" "$RUN_RC" + assert_true "stderr broken, query error: teardown ok" contains "$RUN_OUT" "teardown ok" + common_after "stderr broken, query error" + # Only stderr broken and a signal in teardown: the notice goes to the + # broken stderr. Before #83 that SIGPIPE killed the shell mid-restore. + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_SIGNAL_ON=/api/stats FAKE_SIGNAL_NTH=2 FAKE_SIGNAL=INT \ + -- perl -e "$PERL_BREAK" 2 + assert_eq "stderr broken, INT in teardown: status kept" "0" "$RUN_RC" + assert_true "stderr broken, INT in teardown: teardown ok" contains "$RUN_OUT" "teardown ok" + common_after "stderr broken, INT in teardown" + # stdout broken from the start: the first echo raises SIGPIPE → 141, + # after a teardown that still restores (one restart: teardown's). + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" -- perl -e "$PERL_BREAK" 1 + assert_eq "stdout broken: exit 141" "141" "$RUN_RC" + assert_eq "stdout broken: teardown restarted" "1" "$(restarts)" + common_after "stdout broken" + # stdout broken AND SIGPIPE ignored from the start (not trappable): the + # run cannot stop, so every message fails. On bash 3.2 the unwritten + # text used to be replayed into the next subshell — the config-edit + # group piped to ssh, whose remote bash then ran it. The run must keep + # its real result and send nothing but what it built. + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" -- perl -e "$PERL_IGNORE" 1 + assert_eq "stdout dead, SIGPIPE ignored: real result" "0" "$RUN_RC" + assert_eq "stdout dead, SIGPIPE ignored: two restarts" "2" "$(restarts)" + common_after "stdout dead, SIGPIPE ignored" + run_full TARGET_HOST_DB_PATH="$HOST_DIR/legacy.db" -- perl -e "$PERL_IGNORE" 2 + assert_eq "stderr dead, SIGPIPE ignored: real result" "1" "$RUN_RC" + assert_true "stderr dead, SIGPIPE ignored: stdout intact" contains "$RUN_OUT" "teardown ok" + common_after "stderr dead, SIGPIPE ignored" + elif [ -n "${CI:-}" ]; then + FAIL=$((FAIL + 1)); echo "FAIL: perl not on PATH in CI — the broken-stream group cannot run" >&2 + else + echo "SKIP: perl not on PATH — skipping the full-run broken-stream group" >&2 + fi + + # Across every run above. + assert_true "all runs: pubkey never in argv" lacks "$(cat "$ALL_ARGV")" "$PK_A" + assert_true "all runs: token never in argv" lacks "$(cat "$ALL_ARGV")" "$SYNTH_TOKEN" + assert_true "all runs: SQL never in argv" lacks "$(cat "$ALL_ARGV")" "from_pubkey" + + # ----- the two empty-file guards, each on its own --------------------------- + # db_path_ok and -readonly are independent: each alone must stop sqlite3 + # from creating a file at a wrong path. + with_fake() { + ( export PATH="$SHIM_DIR:$ORIG_PATH" FAKE_REAL_PATH="$ORIG_PATH" FAKE_REMOTE_PATH="$SHIM_DIR" + rm -rf "$FAKE_STATE"; mkdir -p "$FAKE_STATE"; "$@" ) + } + TMP="$FIXTURE_DIR"; TARGET_CONTAINER="$FAKE_CONTAINER"; TARGET_SSH_HOST="stub-host" + TEST_PUBKEY="$PK_A"; SSH_OPTS=(-o BatchMode=yes); TARGET_CONTAINER_DB_PATH="" + SQLITE_RUNNER=host; TARGET_HOST_DB_PATH="$HOST_DIR/direct.db" + with_fake run_sqlite <<<"SELECT 1;" >/dev/null 2>&1; rc=$? + assert_true "-readonly: query on a missing path fails" test "$rc" -ne 0 + assert_true "-readonly: no file created" test ! -e "$HOST_DIR/direct.db" + assert_true "path check: missing path rejected" test "$(with_fake db_path_ok; echo $?)" -ne 0 + : >"$HOST_DIR/empty.db"; TARGET_HOST_DB_PATH="$HOST_DIR/empty.db" + assert_true "path check: empty file rejected" test "$(with_fake db_path_ok; echo $?)" -ne 0 + rm -f "$HOST_DIR/empty.db" + TARGET_HOST_DB_PATH="$HOST_DB_PATH" + assert_eq "path check: real db accepted" "0" "$(with_fake db_path_ok; echo $?)" + + # read_retain_count's own contract: a failed query leaves no count and + # returns 1 — never a count of 0 that a caller could read as a result. + rrc() { read_retain_count >/dev/null 2>&1; echo "$?:$RETAIN_COUNT"; } + TARGET_HOST_DB_PATH="$HOST_DIR/legacy.db" + assert_eq "read_retain_count: query error → 1, no count" "1:" "$(with_fake rrc)" + TARGET_HOST_DB_PATH="$HOST_DB_PATH" + assert_eq "read_retain_count: host db → 0, count 3" "0:3" "$(with_fake rrc)" + TARGET_HOST_DB_PATH="" + assert_eq "read_retain_count: no path → 1, no count" "1:" "$(with_fake rrc)" + TARGET_HOST_DB_PATH="$HOST_DB_PATH" # run_sqlite with no resolved runner must refuse rather than guess. SQLITE_RUNNER="" @@ -287,6 +746,25 @@ FAKE else PASS=$((PASS + 1)) fi + + # Pattern files: private, one non-empty line each. + PT=$(mktemp -d); TMP="$PT"; TEST_PUBKEY="$PK_A" + write_pubkey_patterns + assert_match "pattern file mode 600" '^-rw-------' "$(ls -l "$PK_PATTERN")" + assert_match "quoted pattern file mode 600" '^-rw-------' "$(ls -l "$PK_QUOTED_PATTERN")" + assert_eq "pattern file content" "$PK_A" "$(cat "$PK_PATTERN")" + assert_eq "quoted pattern file content" "\"$PK_A\"" "$(cat "$PK_QUOTED_PATTERN")" + printf '{"x":"%s"}' "$PK_B" >"$PT/other.json" + file_has_pattern "$PK_PATTERN" "$PT/other.json"; assert_eq "grep: absent → 1" "1" "$?" + printf '{"x":"%s"}' "$PK_A" >"$PT/hit.json" + file_has_pattern "$PK_QUOTED_PATTERN" "$PT/hit.json"; assert_eq "grep: present → 0" "0" "$?" + file_has_pattern "$PT/no-such.pat" "$PT/hit.json" 2>/dev/null + assert_eq "grep: error is 2, not 'absent'" "2" "$?" + rm -rf "$PT" + + # curl config: quotes and backslashes cannot end the url string. + assert_eq "curl config: plain" 'url = "http://h/api/nodes/ab"' "$(curl_url_config 'http://h/api/nodes/ab')" + assert_eq "curl config: escaped" 'url = "a\"b\\c"' "$(curl_url_config 'a"b\c')" fi # ----- teardown exit status ---------------------------------------------------- @@ -294,11 +772,16 @@ fi # effects stubbed. The run must always tear down, and an interrupted run must # never exit 0: before the fix, SIGTERM tore down and then exited as a pass. TD_DIR=$(mktemp -d) -# MODE: exit | term | int | exit-sig-in-teardown. The last one signals the run -# while teardown is restoring the target; teardown must still finish. -teardown_case() { # MODE FAILS NODE_VISIBLE_RC → prints exit status +# MODE: exit | term | int | pipe | exit-sig-in-teardown | stderr-write | +# stdout-write. exit-sig-in-teardown signals the run while teardown is +# restoring the target; teardown must still finish. The *-write modes make one +# plain write from the shell itself — combined with a broken stream (below) +# that is how the shell, rather than a child, meets SIGPIPE. +# Anything after the three arguments is a wrapper command (a stream breaker). +teardown_case() { # MODE FAILS NODE_VISIBLE_RC [WRAPPER...] → prints exit status + local mode=$1 fails=$2 visible=$3; shift 3 : >"$TD_DIR/calls" - bash -c ' + "$@" bash -c ' calls="$2/calls"; visible_rc=$3; fails=$4; mode=$5 . "$1" remove_from_blacklist() { echo remove >>"$calls"; } @@ -307,7 +790,7 @@ teardown_case() { # MODE FAILS NODE_VISIBLE_RC → prints exit status # disposition was inherited as ignored (trap -p cannot show that). restart_target() { echo "child-int:$(sh -c "kill -INT \$\$; echo survived" 2>/dev/null)" >>"$calls"; } wait_for_stats() { - if [ "$mode" = exit-sig-in-teardown ]; then kill -TERM $$; kill -INT $$; fi + if [ "$mode" = exit-sig-in-teardown ]; then kill -TERM $$; kill -INT $$; kill -PIPE $$; fi } node_visible() { echo visible-checked >>"$calls"; return "$visible_rc"; } TMP=$(mktemp -d); TEST_PUBKEY=synthetic; TEARDOWN_DONE=0 @@ -315,11 +798,15 @@ teardown_case() { # MODE FAILS NODE_VISIBLE_RC → prints exit status case "$mode" in term) kill -TERM $$; sleep 5; exit 0 ;; int) kill -INT $$; sleep 5; exit 0 ;; + pipe) kill -PIPE $$; sleep 5; exit 0 ;; + stderr-write) echo "diagnostic" >&2; exit "$fails" ;; + stdout-write) echo "progress"; exit "$fails" ;; exit|exit-sig-in-teardown) exit "$fails" ;; esac - ' _ "$SCRIPT_DIR/blacklist-test.sh" "$TD_DIR" "$3" "$2" "$1" >/dev/null 2>&1 + ' _ "$BLACKLIST_SH" "$TD_DIR" "$visible" "$fails" "$mode" >/dev/null 2>&1 echo $? } +tore_down() { [ "$(grep -c remove "$TD_DIR/calls")" = 1 ] && grep -q visible-checked "$TD_DIR/calls"; } assert_eq "clean run exits 0" "0" "$(teardown_case exit 0 0)" assert_eq "clean run tore down once" "1" "$(grep -c remove "$TD_DIR/calls")" assert_eq "two failures exit 2" "2" "$(teardown_case exit 2 0)" @@ -333,6 +820,33 @@ assert_eq "SIGINT + failed teardown" "131" "$(teardown_case int 0 1)" assert_eq "signal during teardown: status kept" "2" "$(teardown_case exit-sig-in-teardown 2 0)" assert_eq "signal during teardown: teardown finished" "1" "$(grep -c visible-checked "$TD_DIR/calls")" assert_eq "teardown children still die on SIGINT" "child-int:" "$(grep child-int "$TD_DIR/calls")" + +# SIGPIPE (issue #83). kill -PIPE is the plain case; the stream breaker makes +# the shell's own write raise it for real. A shell killed by SIGPIPE also exits +# 141, so every case also checks that teardown actually ran. +assert_eq "SIGPIPE mid-run exits 141" "141" "$(teardown_case pipe 0 0)" +assert_true "SIGPIPE mid-run tore down" tore_down +assert_eq "SIGPIPE + failed teardown" "142" "$(teardown_case pipe 0 1)" +if command -v perl >/dev/null 2>&1; then + assert_eq "broken stderr, shell writes: 141" "141" "$(teardown_case stderr-write 0 0 perl -e "$PERL_BREAK" 2)" + assert_true "broken stderr, shell writes: tore down" tore_down + assert_eq "broken stdout, shell writes: 141" "141" "$(teardown_case stdout-write 2 0 perl -e "$PERL_BREAK" 1)" + assert_true "broken stdout, shell writes: tore down" tore_down + assert_eq "broken stderr, failed teardown: 142" "142" "$(teardown_case stderr-write 0 1 perl -e "$PERL_BREAK" 2)" + # The INT/TERM notice in teardown goes to the broken stderr. Before #83 + # that SIGPIPE killed the shell mid-restore. + assert_eq "broken stderr, signal in teardown: status kept" "2" \ + "$(teardown_case exit-sig-in-teardown 2 0 perl -e "$PERL_BREAK" 2)" + assert_true "broken stderr, signal in teardown: teardown finished" tore_down + # Started with SIGPIPE ignored: not trappable, so writes fail with EPIPE and + # the run keeps its real status — never a pass it did not earn. + assert_eq "SIGPIPE ignored on entry: status kept" "2" "$(teardown_case stderr-write 2 0 perl -e "$PERL_IGNORE" 2)" + assert_true "SIGPIPE ignored on entry: tore down" tore_down +elif [ -n "${CI:-}" ]; then + FAIL=$((FAIL + 1)); echo "FAIL: perl not on PATH in CI — the SIGPIPE stream group cannot run" >&2 +else + echo "SKIP: perl not on PATH — skipping the SIGPIPE stream group" >&2 +fi rm -rf "$TD_DIR" echo "test-blacklist-sql.sh: $PASS passed, $FAIL failed" From 8a7bed26662fa715778bbdbcb922ef4c9b581867 Mon Sep 17 00:00:00 2001 From: Dennis Jakobsen Date: Wed, 23 Sep 2026 17:09:24 +0200 Subject: [PATCH 2/8] reviewfix(qa): tighten hide check, trap SIGHUP, close test gaps (#83) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Independent review of the #83 change found no blockers; this addresses its should-fix items and surviving mutants: - §10.1 now requires BOTH a 404 from the detail endpoint and absence from the listing, and a listing that cannot be fetched fails instead of counting as "not listed" (it previously passed with detail=200). - SIGHUP is handled like INT/TERM (teardown, 129, +1 on teardown failure). - Document the WAL requirement of the -readonly open, the inherited SIGPIPE ignore inside teardown, and include the path-check exit code. - Tests: listing 500 and detail-only leak, empty file at the container path, a sqlite3 that echoes the probe token among other output, the remote hex re-check, SIGHUP, and bash 3.2's buffer replay after say on a dead stdout. Optional strace execve evidence per full run on Linux (BLACKLIST_TEST_STRACE_DIR). Co-Authored-By: Claude Opus 5.5 --- qa/scripts/blacklist-test.sh | 33 +++++++++----- qa/scripts/test-blacklist-sql.sh | 78 ++++++++++++++++++++++++++++++-- 2 files changed, 97 insertions(+), 14 deletions(-) diff --git a/qa/scripts/blacklist-test.sh b/qa/scripts/blacklist-test.sh index efe52d5c7..35c9b5878 100755 --- a/qa/scripts/blacklist-test.sh +++ b/qa/scripts/blacklist-test.sh @@ -56,8 +56,8 @@ # teardown-failed→ post-test removal did not restore listing # # Exit code = number of failures (0 = pass). An interrupted run still tears -# down, then exits 130 (SIGINT), 143 (SIGTERM) or 141 (SIGPIPE), plus 1 if -# teardown failed. +# down, then exits 130 (SIGINT), 143 (SIGTERM), 129 (SIGHUP) or 141 (SIGPIPE), +# plus 1 if teardown failed. # # SIGPIPE: the script's own writes go to stdout/stderr, so if either is a pipe # whose reader has gone, the shell gets SIGPIPE. It is handled like INT/TERM: @@ -65,14 +65,15 @@ # run the EXIT trap but then re-raise the signal, so the status would be 141 # whatever teardown found, and teardown's own next write to the dead stream # would raise SIGPIPE again and cut the restore short. Inside teardown SIGPIPE -# is ignored, so a lost output channel cannot do that. Child processes keep -# the default disposition; one killed by SIGPIPE fails its step, which is -# classified as usual. A shell started with SIGPIPE already ignored cannot +# is ignored, so a lost output channel cannot do that. Outside teardown child +# processes keep the default disposition, and one killed by SIGPIPE fails its +# step, which is classified as usual; inside it they inherit the ignore and +# see EPIPE instead. A shell started with SIGPIPE already ignored cannot # trap it; its writes then fail with EPIPE and the run continues to its normal # exit status. Either way a dead stream is sent to /dev/null (see say/warn), # never replayed. # -# A further INT/TERM does not abort teardown: it stops only the step in +# A further INT/TERM/HUP does not abort teardown: it stops only the step in # progress. An interrupted ssh step fails and is reported as teardown-failed; # an interrupted /api/stats poll only counts as one failed poll, so the wait # goes on until RESTART_WAIT_S and teardown can still succeed. @@ -129,7 +130,7 @@ teardown() { # ignored disposition is inherited by ssh/curl, so a hung step could no longer # be interrupted. With a handler, a terminal Ctrl-C still stops the running # step; that step fails and teardown reports teardown-failed. - trap 'warn " (signal received — teardown continues restoring the target)"' INT TERM + trap 'warn " (signal received — teardown continues restoring the target)"' INT TERM HUP say "=== teardown: removing $TEST_PUBKEY from nodeBlacklist ===" if remove_from_blacklist && restart_target && wait_for_stats; then if node_visible; then @@ -150,6 +151,7 @@ install_teardown_traps() { trap teardown EXIT trap 'teardown 130' INT trap 'teardown 143' TERM + trap 'teardown 129' HUP trap 'teardown 141' PIPE } @@ -299,6 +301,9 @@ node_visible() { # exist fails "unable to open database" instead of leaving an # empty file behind. db_path_ok checks the path first; this is # the second, independent guard. +# CoreScope's database is in WAL mode: a read-only open +# needs the -wal/-shm files a running app keeps; §10.2 +# runs after /api/stats has shown the restarted app is up. SQLITE_ARGS=(-batch -bail -init /dev/null -noheader -list -readonly) # Round-trip probe token. The value is arbitrary; it only has to come back intact. SQLITE_PROBE_TOKEN="corescope-probe-ok" @@ -464,7 +469,7 @@ read_retain_count() { say " ❌ retain-failed: ssh failed while checking $(runner_db_var) (exit 255)" return 1 elif (( path_rc != 0 )); then - say " ❌ retain-failed: $(runner_db_var)=$(runner_db_path) is not a non-empty file in the $SQLITE_RUNNER" + say " ❌ retain-failed: $(runner_db_var)=$(runner_db_path) is not a non-empty file in the $SQLITE_RUNNER (check exit $path_rc)" return 1 fi if ! RETAIN_COUNT=$(run_sqlite <<<"$(transmission_count_sql "$TEST_PUBKEY")" 2>"$TMP/sqlite.err"); then @@ -575,7 +580,10 @@ main() { detail_code=$(fetch_code "$TARGET_URL/api/nodes/$TEST_PUBKEY" "$TMP/detail.json") list_code=$(fetch_code "$TARGET_URL/api/nodes?limit=10000" "$TMP/list.json") - in_list=0 + # Hidden means BOTH: the detail endpoint 404s and the listing omits it. A + # listing that could not be fetched or searched proves nothing, so it fails + # rather than counting as "not in the list". + in_list=unknown if [[ "$list_code" == "200" ]]; then file_has_pattern "$PK_QUOTED_PATTERN" "$TMP/list.json" case $? in @@ -584,10 +592,13 @@ main() { *) in_list=error ;; esac fi - if [[ "$in_list" == "error" ]]; then + if [[ "$in_list" == "unknown" ]]; then + say " ❌ hide-failed: /api/nodes HTTP $list_code — listing not checked" + fails=$((fails+1)) + elif [[ "$in_list" == "error" ]]; then say " ❌ hide-failed: could not search /api/nodes response (grep error)" fails=$((fails+1)) - elif [[ "$detail_code" == "404" || "$in_list" == "0" ]]; then + elif [[ "$detail_code" == "404" && "$in_list" == "0" ]]; then say " ✅ hide ok: detail=$detail_code in_list=$in_list" else say " ❌ hide-failed: detail=$detail_code in_list=$in_list — pubkey still surfaced" diff --git a/qa/scripts/test-blacklist-sql.sh b/qa/scripts/test-blacklist-sql.sh index 0716bd978..247b5353b 100755 --- a/qa/scripts/test-blacklist-sql.sh +++ b/qa/scripts/test-blacklist-sql.sh @@ -361,6 +361,7 @@ fi BL=(); while IFS= read -r b; do BL+=("$b"); done <"$FAKE_STATE/live-blacklist" hidden() { [ "${FAKE_LEAK:-0}" = 1 ] && return 1 + [ "${FAKE_DETAIL_LEAK:-0}" = 1 ] && [ "${path#/api/nodes/}" != "$path" ] && return 1 local b; for b in ${BL[@]+"${BL[@]}"}; do [ "$b" = "$1" ] && return 0; done; return 1 } code=404; body='{"error":"not found"}' @@ -372,7 +373,7 @@ case $path in if [ "$n" = "$want" ] && ! hidden "$n"; then code=200; body="{\"public_key\":\"$n\"}"; fi done ;; '/api/nodes?limit=10000') - code=200; body='{"nodes":['; sep='' + code=${FAKE_LIST_CODE:-200}; body='{"nodes":['; sep='' for n in $FAKE_NODES; do hidden "$n" && continue; body+="$sep{\"public_key\":\"$n\"}"; sep=','; done body+=']}' ;; /api/topology) @@ -395,6 +396,7 @@ FAKECURL sqlite3 "$FAKE_CROOT$CONTAINER_DB_PATH" "INSERT INTO transmissions(raw_hex,hash,first_seen,payload_type,from_pubkey) VALUES ('00','c1','t',4,'$PK_A'),('00','c2','t',4,'$PK_A'),('00','c3','t',4,'$PK_B');" cp "$DB" "$HOST_DB_PATH" + : >"$FAKE_CROOT/srv/corescope/data/empty.db" cp "$LEGACY_DB" "$HOST_DIR/legacy.db" UNBINDABLE_BIN="$FIXTURE_DIR/unbindable-bin"; mkdir -p "$UNBINDABLE_BIN" cat >"$UNBINDABLE_BIN/sqlite3" <<'FAKE' @@ -405,6 +407,15 @@ echo ".parameter CMD ... Manage SQL parameter bindings" echo "0" FAKE chmod +x "$UNBINDABLE_BIN/sqlite3" + NOISY_BIN="$FIXTURE_DIR/noisy-bin"; mkdir -p "$NOISY_BIN" + cat >"$NOISY_BIN/sqlite3" <<'FAKE' +#!/usr/bin/env bash +# Echoes the probe token among other output without having bound anything. +cat >/dev/null +echo ".parameter CMD ... Manage SQL parameter bindings" +echo "corescope-probe-ok" +FAKE + chmod +x "$NOISY_BIN/sqlite3" FAKE_CONFIG="$FAKE/target/config.json" ORIG_CONFIG='{"port":3000,"nodeBlacklist":["aa00aa00"],"mqtt":{"sources":[]}}' @@ -415,6 +426,7 @@ FAKE export FAKE_CONTAINER="corescope-stub" FAKE_URL="http://target.invalid" export FAKE_NODES="$PK_A $PK_B" ALL_ARGV="$FAKE/all-argv.log"; : >"$ALL_ARGV" + RUN_N=0 snapshot_files() { (cd "$FAKE" && find host container-root tmp target -print | LC_ALL=C sort); } db_sums() { cksum "$FAKE_CROOT$CONTAINER_DB_PATH" "$HOST_DB_PATH" | awk '{print $1, $2}'; } @@ -429,9 +441,11 @@ FAKE printf '%s\n' "$ORIG_CONFIG" >"$FAKE_CONFIG" : >"$FAKE_ARGV_LOG" FILES_BEFORE=$(snapshot_files) + RUN_N=$((RUN_N + 1)) ( unset TARGET_DB_PATH ADMIN_API_TOKEN TARGET_CONTAINER_DB_PATH TARGET_HOST_DB_PATH \ FAKE_CONTAINER_SQLITE FAKE_SIGNAL_ON FAKE_SIGNAL FAKE_SIGNAL_NTH FAKE_LEAK \ + FAKE_DETAIL_LEAK FAKE_LIST_CODE \ FAKE_SSH_FAIL_CMD FAKE_SSH_FAIL_FROM export TEST_NODE_PUBKEY="$PK_A" TARGET_SSH_HOST="stub-host" TARGET_SSH_KEY="/nonexistent/key" \ TARGET_CONFIG_PATH="$FAKE_CONFIG" TARGET_CONTAINER="$FAKE_CONTAINER" \ @@ -441,9 +455,15 @@ FAKE [ "${1:-}" = -- ] && shift wrap=("$@") export PATH="$SHIM_DIR:$ORIG_PATH" + # Optional kernel-level evidence (Linux): strace every execve of the + # run, including anything the PATH shims cannot see. + local tracer=() + if [ -n "${BLACKLIST_TEST_STRACE_DIR:-}" ]; then + tracer=(strace -f -qq -e trace=execve -s 65535 -o "$BLACKLIST_TEST_STRACE_DIR/run.$RUN_N") + fi # Foreground, so SIGINT is not ignored the way it is for a background # job; the pid is recorded for the fake curl to signal. - ${wrap[@]+"${wrap[@]}"} bash -c 'echo $$ >"$FAKE_STATE/script.pid"; exec bash "$@"' _ \ + ${tracer[@]+"${tracer[@]}"} ${wrap[@]+"${wrap[@]}"} bash -c 'echo $$ >"$FAKE_STATE/script.pid"; exec bash "$@"' _ \ "$BLACKLIST_SH" "http://baseline.invalid" "$FAKE_URL" \ >"$FAKE/out" 2>"$FAKE/err" ) @@ -640,8 +660,34 @@ FAKE assert_true "grep error: no hide ok claimed" lacks "$RUN_OUT" "hide ok" common_after "grep error" + # 11c. Hidden means detail 404 AND absent from the listing; a listing that + # could not be fetched proves nothing. + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_LIST_CODE=500 + assert_eq "listing HTTP 500: fails" "1" "$RUN_RC" + assert_true "listing HTTP 500: classified" contains "$RUN_OUT" "hide-failed: /api/nodes HTTP 500" + common_after "listing HTTP 500" + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_DETAIL_LEAK=1 + assert_eq "detail leaks, listing hides: fails" "1" "$RUN_RC" + assert_true "detail leaks, listing hides: classified" contains "$RUN_OUT" "hide-failed: detail=200 in_list=0" + common_after "detail leaks, listing hides" + + # 11d. Empty file at the container path, sqlite3-capable container → + # refused before any query; the file stays empty. + run_full TARGET_CONTAINER_DB_PATH="/srv/corescope/data/empty.db" FAKE_CONTAINER_SQLITE=1 + assert_eq "empty container db: exit 1" "1" "$RUN_RC" + assert_true "empty container db: classified" contains "$RUN_OUT" "TARGET_CONTAINER_DB_PATH=/srv/corescope/data/empty.db is not a non-empty file in the container" + assert_true "empty container db: still empty" test ! -s "$FAKE_CROOT/srv/corescope/data/empty.db" + assert_true "empty container db: sqlite3 never opened it" lacks "$(grep '^sqlite3 ' "$FAKE_ARGV_LOG")" "empty.db" + common_after "empty container db" + + # 11e. The probe wants the token back and nothing else. + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_REAL_PATH="$NOISY_BIN:$ORIG_PATH" + assert_eq "noisy sqlite3: rejected" "1" "$RUN_RC" + assert_true "noisy sqlite3: classified" contains "$RUN_OUT" "retain-failed: no sqlite3 able to bind" + common_after "noisy sqlite3" + # 12. Signals mid-run: classified exit, full teardown. - for sig in TERM:143 INT:130; do + for sig in TERM:143 INT:130 HUP:129; do run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_SIGNAL_ON=/api/topology FAKE_SIGNAL="${sig%%:*}" assert_eq "SIG${sig%%:*} mid-run: exit ${sig#*:}" "${sig#*:}" "$RUN_RC" assert_true "SIG${sig%%:*} mid-run: teardown ok" contains "$RUN_OUT" "teardown ok" @@ -707,6 +753,15 @@ FAKE assert_true "all runs: pubkey never in argv" lacks "$(cat "$ALL_ARGV")" "$PK_A" assert_true "all runs: token never in argv" lacks "$(cat "$ALL_ARGV")" "$SYNTH_TOKEN" assert_true "all runs: SQL never in argv" lacks "$(cat "$ALL_ARGV")" "from_pubkey" + if [ -n "${BLACKLIST_TEST_STRACE_DIR:-}" ]; then + traces=$(cat "$BLACKLIST_TEST_STRACE_DIR"/run.* 2>/dev/null) + assert_eq "strace: one trace per run" "$RUN_N" "$(find "$BLACKLIST_TEST_STRACE_DIR" -name 'run.*' | wc -l | tr -d ' ')" + assert_true "strace: traced ssh/curl/sqlite3/jq execs" contains "$traces" 'execve("' + for c in /ssh /curl /sqlite3 /jq /grep; do assert_true "strace: saw $c" contains "$traces" "$c\""; done + assert_true "strace: pubkey in no execve" lacks "$traces" "$PK_A" + assert_true "strace: SQL in no execve" lacks "$traces" "from_pubkey" + assert_true "strace: token in no execve" lacks "$traces" "$SYNTH_TOKEN" + fi # ----- the two empty-file guards, each on its own --------------------------- # db_path_ok and -readonly are independent: each alone must stop sqlite3 @@ -739,6 +794,15 @@ FAKE assert_eq "read_retain_count: no path → 1, no count" "1:" "$(with_fake rrc)" TARGET_HOST_DB_PATH="$HOST_DB_PATH" + # The remote edit re-checks the pubkey it reads from stdin: a non-hex value + # (main() never lets one through) is refused and the config left alone. + printf '%s\n' "$ORIG_CONFIG" >"$FAKE_CONFIG"; before_cfg=$(cat "$FAKE_CONFIG") + TARGET_CONFIG_PATH="$FAKE_CONFIG"; TEST_PUBKEY='zz"; touch /tmp/pr83-never' + with_fake set_blacklist_state add >/dev/null 2>&1; rc=$? + assert_eq "remote hex check: refused" "1" "$rc" + assert_eq "remote hex check: config untouched" "$before_cfg" "$(cat "$FAKE_CONFIG")" + TEST_PUBKEY="$PK_A" + # run_sqlite with no resolved runner must refuse rather than guess. SQLITE_RUNNER="" if run_sqlite /dev/null 2>&1; then @@ -799,6 +863,7 @@ teardown_case() { # MODE FAILS NODE_VISIBLE_RC [WRAPPER...] → prints exit sta term) kill -TERM $$; sleep 5; exit 0 ;; int) kill -INT $$; sleep 5; exit 0 ;; pipe) kill -PIPE $$; sleep 5; exit 0 ;; + hup) kill -HUP $$; sleep 5; exit 0 ;; stderr-write) echo "diagnostic" >&2; exit "$fails" ;; stdout-write) echo "progress"; exit "$fails" ;; exit|exit-sig-in-teardown) exit "$fails" ;; @@ -817,6 +882,9 @@ assert_eq "SIGTERM + failed teardown" "144" "$(teardown_case term 0 1)" assert_eq "SIGINT mid-run exits 130" "130" "$(teardown_case int 0 0)" assert_eq "SIGINT still tore down (once)" "1" "$(grep -c remove "$TD_DIR/calls")" assert_eq "SIGINT + failed teardown" "131" "$(teardown_case int 0 1)" +assert_eq "SIGHUP mid-run exits 129" "129" "$(teardown_case hup 0 0)" +assert_true "SIGHUP mid-run tore down" tore_down +assert_eq "SIGHUP + failed teardown" "130" "$(teardown_case hup 0 1)" assert_eq "signal during teardown: status kept" "2" "$(teardown_case exit-sig-in-teardown 2 0)" assert_eq "signal during teardown: teardown finished" "1" "$(grep -c visible-checked "$TD_DIR/calls")" assert_eq "teardown children still die on SIGINT" "child-int:" "$(grep child-int "$TD_DIR/calls")" @@ -838,6 +906,10 @@ if command -v perl >/dev/null 2>&1; then assert_eq "broken stderr, signal in teardown: status kept" "2" \ "$(teardown_case exit-sig-in-teardown 2 0 perl -e "$PERL_BREAK" 2)" assert_true "broken stderr, signal in teardown: teardown finished" tore_down + # say on a dead stdout must not leave text for the next subshell to + # replay (bash 3.2 keeps it buffered; bash 5 purges it). + replay=$(perl -e "$PERL_IGNORE" 1 bash -c '. "$1"; say "STALE-MARK"; x=$(printf clean); printf "%s" "$x" >&3' _ "$BLACKLIST_SH" 3>&1 2>/dev/null) + assert_eq "dead stdout: nothing replayed into a capture" "clean" "$replay" # Started with SIGPIPE ignored: not trappable, so writes fail with EPIPE and # the run keeps its real status — never a pass it did not earn. assert_eq "SIGPIPE ignored on entry: status kept" "2" "$(teardown_case stderr-write 2 0 perl -e "$PERL_IGNORE" 2)" From f97d494970d40782653333290839e033aa7dda29 Mon Sep 17 00:00:00 2001 From: Dennis Jakobsen Date: Wed, 23 Sep 2026 17:31:50 +0200 Subject: [PATCH 3/8] reviewfix(qa): test HUP during teardown, guard strace evidence dir (#83) Second review pass: dropping HUP from teardown's notice trap survived the suite, although a HUP during the stats wait would then re-enter teardown and cut the restore short. The signal-during-teardown case now sends HUP too. BLACKLIST_TEST_STRACE_DIR must be an existing empty directory and strace must be present, so stale traces cannot mix into a run's checks. Co-Authored-By: Claude Opus 5.5 --- qa/scripts/test-blacklist-sql.sh | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/qa/scripts/test-blacklist-sql.sh b/qa/scripts/test-blacklist-sql.sh index 247b5353b..d67c7f3bb 100755 --- a/qa/scripts/test-blacklist-sql.sh +++ b/qa/scripts/test-blacklist-sql.sh @@ -427,6 +427,16 @@ FAKE export FAKE_NODES="$PK_A $PK_B" ALL_ARGV="$FAKE/all-argv.log"; : >"$ALL_ARGV" RUN_N=0 + # Optional strace evidence (Linux): needs strace and an EMPTY directory, so + # no earlier run's traces can mix into this run's checks. + if [ -n "${BLACKLIST_TEST_STRACE_DIR:-}" ]; then + if ! command -v strace >/dev/null 2>&1; then + echo "BLACKLIST_TEST_STRACE_DIR is set but strace is not on PATH (Linux only)" >&2; exit 2 + fi + if [ -n "$(ls -A "$BLACKLIST_TEST_STRACE_DIR" 2>/dev/null)" ] || [ ! -d "$BLACKLIST_TEST_STRACE_DIR" ]; then + echo "BLACKLIST_TEST_STRACE_DIR must be an existing, empty directory" >&2; exit 2 + fi + fi snapshot_files() { (cd "$FAKE" && find host container-root tmp target -print | LC_ALL=C sort); } db_sums() { cksum "$FAKE_CROOT$CONTAINER_DB_PATH" "$HOST_DB_PATH" | awk '{print $1, $2}'; } @@ -854,7 +864,7 @@ teardown_case() { # MODE FAILS NODE_VISIBLE_RC [WRAPPER...] → prints exit sta # disposition was inherited as ignored (trap -p cannot show that). restart_target() { echo "child-int:$(sh -c "kill -INT \$\$; echo survived" 2>/dev/null)" >>"$calls"; } wait_for_stats() { - if [ "$mode" = exit-sig-in-teardown ]; then kill -TERM $$; kill -INT $$; kill -PIPE $$; fi + if [ "$mode" = exit-sig-in-teardown ]; then kill -TERM $$; kill -INT $$; kill -HUP $$; kill -PIPE $$; fi } node_visible() { echo visible-checked >>"$calls"; return "$visible_rc"; } TMP=$(mktemp -d); TEST_PUBKEY=synthetic; TEARDOWN_DONE=0 From 5b2891ea64c41aa5f9df406e7aa55f011b1cdd7c Mon Sep 17 00:00:00 2001 From: Dennis Jakobsen Date: Wed, 23 Sep 2026 18:00:30 +0200 Subject: [PATCH 4/8] fix(qa-test): start the SIGPIPE trap cases from the default disposition (#83) GitHub Actions starts steps with SIGPIPE ignored, which bash cannot trap, so `kill -PIPE $$` did nothing there and the two trap cases saw exit 0/1 instead of 141/142 (the script's documented ignored-on-entry behaviour). Run them through a perl wrapper that resets SIGPIPE to default, as the stream-breaker cases already do. The suite now passes both with SIGPIPE at default and with it ignored on entry. Co-Authored-By: Claude Opus 5.5 --- qa/scripts/test-blacklist-sql.sh | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/qa/scripts/test-blacklist-sql.sh b/qa/scripts/test-blacklist-sql.sh index d67c7f3bb..890fbdd4a 100755 --- a/qa/scripts/test-blacklist-sql.sh +++ b/qa/scripts/test-blacklist-sql.sh @@ -54,6 +54,10 @@ BLACKLIST_SH="${BLACKLIST_TEST_SH:-$SCRIPT_DIR/blacklist-test.sh}" PERL_BREAK='my $fd = shift; $SIG{PIPE} = "DEFAULT"; pipe(my $r, my $w) or die "pipe: $!"; close $r; if ($fd == 1) { open(STDOUT, ">&", $w) or die "dup: $!" } else { open(STDERR, ">&", $w) or die "dup: $!" } close $w; exec { $ARGV[0] } @ARGV or die "exec: $!"' +# `perl -e "$PERL_DEFAULT" cmd...` only resets SIGPIPE to default. CI runners +# (GitHub Actions) start steps with SIGPIPE ignored, which bash cannot trap, so +# every case that tests the PIPE trap itself must start from the default. +PERL_DEFAULT='$SIG{PIPE} = "DEFAULT"; exec { $ARGV[0] } @ARGV or die "exec: $!"' PERL_IGNORE='my $fd = shift; $SIG{PIPE} = "IGNORE"; pipe(my $r, my $w) or die "pipe: $!"; close $r; if ($fd == 1) { open(STDOUT, ">&", $w) or die "dup: $!" } else { open(STDERR, ">&", $w) or die "dup: $!" } close $w; exec { $ARGV[0] } @ARGV or die "exec: $!"' @@ -902,10 +906,10 @@ assert_eq "teardown children still die on SIGINT" "child-int:" "$(grep child-int # SIGPIPE (issue #83). kill -PIPE is the plain case; the stream breaker makes # the shell's own write raise it for real. A shell killed by SIGPIPE also exits # 141, so every case also checks that teardown actually ran. -assert_eq "SIGPIPE mid-run exits 141" "141" "$(teardown_case pipe 0 0)" -assert_true "SIGPIPE mid-run tore down" tore_down -assert_eq "SIGPIPE + failed teardown" "142" "$(teardown_case pipe 0 1)" if command -v perl >/dev/null 2>&1; then + assert_eq "SIGPIPE mid-run exits 141" "141" "$(teardown_case pipe 0 0 perl -e "$PERL_DEFAULT")" + assert_true "SIGPIPE mid-run tore down" tore_down + assert_eq "SIGPIPE + failed teardown" "142" "$(teardown_case pipe 0 1 perl -e "$PERL_DEFAULT")" assert_eq "broken stderr, shell writes: 141" "141" "$(teardown_case stderr-write 0 0 perl -e "$PERL_BREAK" 2)" assert_true "broken stderr, shell writes: tore down" tore_down assert_eq "broken stdout, shell writes: 141" "141" "$(teardown_case stdout-write 2 0 perl -e "$PERL_BREAK" 1)" From 41b2e0e3f78358f5cfa9000b71800a974c9b8b91 Mon Sep 17 00:00:00 2001 From: Dennis Jakobsen Date: Thu, 24 Sep 2026 07:45:32 +0200 Subject: [PATCH 5/8] reviewfix(qa): keep existing blacklist rules, real topology route, canonical pubkey (#83) Follow-up review of PR #87 found three problems: 1. Existing privacy rule removed. Teardown always removed the pubkey, so a node that was already in nodeBlacklist lost its rule. A read-only preflight (remote MODE=check, case-insensitive, pubkey on stdin) now refuses the run with exit 2 before any side effect and before the traps, without printing the pubkey. add now appends and remove drops only the exact canonical value, so order, duplicates and other spellings in nodeBlacklist are restored exactly (jq `unique` used to sort and de-duplicate the whole list; python de-duplicated it too). 2. Vacuous topology pass. /api/topology does not exist; the SPA fallback answered 200 HTML and it was reported clean. The check now uses /api/analytics/topology, waits out warm-up 503s for up to RESTART_WAIT_S, requires HTTP 200 and the TopologyResponse shape, and compares the pubkey fields the server filters (topRepeaters, topPairs A/B, bestPathList, multiObsNodes, perObserverReach rings) lower-cased and whole-line. Anything else fails; nothing is skipped. jq is now required on the runner. 3. Case. TEST_NODE_PUBKEY is validated as before, then lower-cased once; config entry, API paths, pattern files and the SQLite binding all use that canonical value. Tests were written first (123 failures against 5b2891ea): pre-blacklisted lower/upper/python3, check unreachable, topology HTML/bad shape/not JSON/ 500/stuck 503/warm-up/null arrays/leak in each part in both cases, an upper-case end-to-end run, and exact restore of an unsorted list with a duplicate and an upper-case entry. The fake API no longer serves /api/topology. Co-Authored-By: Claude Opus 5.5 --- qa/scripts/blacklist-test.sh | 156 ++++++++++++++++++++++++---- qa/scripts/test-blacklist-sql.sh | 173 ++++++++++++++++++++++++++++--- 2 files changed, 292 insertions(+), 37 deletions(-) diff --git a/qa/scripts/blacklist-test.sh b/qa/scripts/blacklist-test.sh index 35c9b5878..8e4b41706 100755 --- a/qa/scripts/blacklist-test.sh +++ b/qa/scripts/blacklist-test.sh @@ -187,6 +187,12 @@ write_pubkey_patterns() { printf '"%s"\n' "$TEST_PUBKEY" >"$PK_QUOTED_PATTERN" ) } +# Whole-line variant, for lists of extracted values (one per line). +file_has_line() { + local pattern_file="$1" file="$2" + grep -qxF -f "$pattern_file" -- "$file" +} + # grep FILE for a pattern file. Status is grep's: 0 found, 1 not found, 2 error. # Callers must not read 2 as "not found" — for a hide check that is a pass. file_has_pattern() { @@ -194,6 +200,45 @@ file_has_pattern() { grep -qF -f "$pattern_file" -- "$file" } +# §10.1 topology. The route is /api/analytics/topology (cmd/server/routes.go); +# /api/topology does not exist and the SPA fallback answers it 200 with HTML, +# which used to pass as "clean". Only a 200 whose body has the TopologyResponse +# shape can pass. The pubkeys it carries are the fields the server's +# filterBlacklistedFromTopology filters: topRepeaters[].pubkey, +# topPairs[].pubkeyA/pubkeyB, bestPathList[].pubkey, multiObsNodes[].pubkey and +# perObserverReach{}.rings[].nodes[].pubkey. Those are extracted, lower-cased, +# one per line, and compared whole-line with the canonical pubkey — not a text +# grep over the body. Arrays the server filtered empty come back as null. +# A shape mismatch makes jq exit non-zero ("not the expected topology JSON"). +TOPOLOGY_PUBKEYS_JQ=' + def list(k): .[k] // [] | if type == "array" then . else error("\(k) is not an array") end; + if type != "object" then error("not an object") else . end + | . as $r + | if (["uniqueNodes","topRepeaters","topPairs","bestPathList","multiObsNodes","perObserverReach"] + | all(. as $k | $r | has($k))) and ((.uniqueNodes | type) == "number") + then . else error("missing topology fields") end + | [ (list("topRepeaters")[] | .pubkey), + (list("topPairs")[] | .pubkeyA, .pubkeyB), + (list("bestPathList")[] | .pubkey), + (list("multiObsNodes")[] | .pubkey), + (.perObserverReach // {} + | if type == "object" then . else error("perObserverReach is not an object") end + | .[] | (.rings // [])[] | (.nodes // [])[] | .pubkey) ] + | .[] | select(type == "string") | ascii_downcase' + +# Fetch the topology into $TMP/topo.json, setting TOPO_CODE. The analytics +# recomputer answers 503 while it warms up after a restart; that is waited out +# for up to RESTART_WAIT_S. Any other code is final. +fetch_topology() { + local deadline=$(( $(date +%s) + RESTART_WAIT_S )) + while :; do + TOPO_CODE=$(fetch_code "$TARGET_URL/api/analytics/topology" "$TMP/topo.json") + [[ "$TOPO_CODE" == "503" ]] || return 0 + (( $(date +%s) < deadline )) || return 0 + sleep 3 + done +} + wait_for_stats() { local deadline code say " polling $TARGET_URL/api/stats; no new poll starts after ${RESTART_WAIT_S}s ..." @@ -219,28 +264,59 @@ restart_target() { return 0 } -# Mutate config.json on target. The pubkey travels on ssh's stdin, in the line -# right after the remote script's first command, which reads it: bash -s reads -# its script from stdin without reading ahead, so `read` gets exactly that line -# (POSIX requires this of a shell reading commands from stdin). From there it -# reaches jq/python3 through the environment. It is therefore in neither the -# local ssh argv nor any remote argv; only the config path and the constant -# mode are on the command line. -set_blacklist_state() { - local mode="$1" q_cfg q_mode # mode: add | remove +# Run the remote config script on the target. MODE is check | add | remove. +# The pubkey travels on ssh's stdin, in the line right after the remote +# script's first command, which reads it: bash -s reads its script from stdin +# without reading ahead, so `read` gets exactly that line (POSIX requires this +# of a shell reading commands from stdin). From there it reaches jq/python3 +# through the environment. It is therefore in neither the local ssh argv nor +# any remote argv; only the config path and the constant mode are on the +# command line. Returns the remote status; for check: 0 = not blacklisted, +# 10 = already blacklisted (any case), anything else = could not tell. +# +# add appends and remove drops only the exact canonical value, so the rest of +# nodeBlacklist — order, duplicates, other spellings — is left as it was. add +# only ever runs after check has shown the value absent, which is what makes +# teardown's remove an exact undo. +remote_config() { + local q_cfg q_mode printf -v q_cfg '%q' "$TARGET_CONFIG_PATH" - printf -v q_mode '%q' "$mode" + printf -v q_mode '%q' "$1" { printf 'IFS= read -r PK || exit 3\n%s\n' "$TEST_PUBKEY" cat <<'REMOTE' set -euo pipefail case "$PK" in ""|*[!0-9a-fA-F]*) echo "blacklist-test: pubkey on stdin is not hex" >&2; exit 3 ;; esac +case "$MODE" in check|add|remove) ;; *) echo "blacklist-test: bad mode" >&2; exit 3 ;; esac export PK +if [ "$MODE" = check ]; then + if command -v jq >/dev/null; then + if jq -e '(.nodeBlacklist // []) + | if type == "array" then . else error("nodeBlacklist is not an array") end + | any(.[]; type == "string" and ascii_downcase == (env.PK | ascii_downcase)) | not' \ + "$CFG" >/dev/null; then + exit 0 + else + rc=$?; [ "$rc" = 1 ] && exit 10; exit 4 + fi + fi + python3 - "$CFG" <<'PY' +import json, os, sys +pk = os.environ["PK"].lower() +try: + with open(sys.argv[1]) as f: bl = json.load(f).get("nodeBlacklist") or [] +except Exception: + sys.exit(4) +if not isinstance(bl, list): sys.exit(4) +sys.exit(10 if any(isinstance(x, str) and x.lower() == pk for x in bl) else 0) +PY + exit 0 +fi TMP="$(mktemp)" trap 'rm -f "$TMP"' EXIT if command -v jq >/dev/null; then if [ "$MODE" = "add" ]; then - jq '.nodeBlacklist = ((.nodeBlacklist // []) + [env.PK] | unique)' "$CFG" > "$TMP" + jq '.nodeBlacklist = ((.nodeBlacklist // []) | if any(.[]; . == env.PK) then . else . + [env.PK] end)' "$CFG" > "$TMP" else jq '.nodeBlacklist = ((.nodeBlacklist // []) - [env.PK])' "$CFG" > "$TMP" fi @@ -250,7 +326,7 @@ import json, os, sys cfg, mode, out = sys.argv[1:] pk = os.environ["PK"] with open(cfg) as f: d = json.load(f) -bl = list(dict.fromkeys(d.get("nodeBlacklist") or [])) +bl = list(d.get("nodeBlacklist") or []) if mode == "add": if pk not in bl: bl.append(pk) else: @@ -266,8 +342,11 @@ mv "$TMP" "$CFG" trap - EXIT REMOTE } | ssh_t "CFG=$q_cfg MODE=$q_mode bash -s" - local rc=$? - if (( rc != 0 )); then +} + +set_blacklist_state() { + local mode="$1" # add | remove + if ! remote_config "$mode"; then say " ❌ ssh-failed: could not edit $TARGET_CONFIG_PATH ($mode)" return 1 fi @@ -529,6 +608,11 @@ main() { warn "error: TEST_NODE_PUBKEY must be hex (got: redacted)" exit 2 fi + # One canonical form from here on: lower case, as the ingestor stores it + # (hex.EncodeToString) and the API returns it. The config entry, API paths, + # pattern files and the SQLite binding all use this value. (tr reads the + # value on stdin; only the character classes are arguments.) + TEST_PUBKEY=$(printf '%s' "$TEST_PUBKEY" | tr 'A-F' 'a-f') # Container name must match docker's allowed chars: [a-zA-Z0-9][a-zA-Z0-9_.-]* if ! [[ "$TARGET_CONTAINER" =~ ^[a-zA-Z0-9][a-zA-Z0-9_.-]*$ ]]; then warn "error: TARGET_CONTAINER has illegal chars" @@ -552,6 +636,11 @@ main() { warn "error: TARGET_URL must be an http(s) URL without whitespace, quotes or backslashes" exit 2 fi + # The topology check parses JSON locally. + if ! command -v jq >/dev/null 2>&1; then + warn "error: jq is required on the machine running this script" + exit 2 + fi CURL_TIMEOUT="${CURL_TIMEOUT:-60}" RESTART_WAIT_S="${RESTART_WAIT_S:-120}" @@ -561,6 +650,24 @@ main() { SSH_OPTS=(-i "$TARGET_SSH_KEY" -o StrictHostKeyChecking=accept-new -o ConnectTimeout=15 -o BatchMode=yes -o ServerAliveInterval=15 -o ServerAliveCountMax=4) + # Is the pubkey already a privacy rule on the target? If so, refuse before + # any side effect: teardown removes the pubkey, and would take the + # operator's rule with it. Case-insensitive, like the server's blacklist. + # Runs before the traps, so a refusal or a failure here tears nothing down, + # and the message does not name the pubkey. It prints nothing on success + # until the traps are in place, so a dead stdout still meets the trap. + local pre_rc=0 + remote_config check || pre_rc=$? + if (( pre_rc == 10 )); then + say " ❌ refused: TEST_NODE_PUBKEY is already in nodeBlacklist on the target." + say " Teardown would remove that existing rule, so nothing was changed." + say " Use a node that is not blacklisted." + exit 2 + elif (( pre_rc != 0 )); then + say " ❌ ssh-failed: could not read nodeBlacklist from $TARGET_CONFIG_PATH (exit $pre_rc) — nothing was changed" + exit 1 + fi + # Everything the run writes locally is private to it: $TMP is mode 700, and # the pattern files and responses inside are mode 600. umask 077 @@ -569,6 +676,7 @@ main() { TEARDOWN_DONE=0 install_teardown_traps write_pubkey_patterns + say "=== preflight: TEST_NODE_PUBKEY is not in the target's nodeBlacklist — ok ===" # --------------------------------------------------------------------------- # §10.1 — hide @@ -605,16 +713,22 @@ main() { fails=$((fails+1)) fi - topo_code=$(fetch_code "$TARGET_URL/api/topology" "$TMP/topo.json") - if [[ "$topo_code" != "200" ]]; then - say " ⚠️ /api/topology HTTP $topo_code — skipping topology assertion" + # Anything short of a well-formed topology fails: absence is only proven + # by a response that could have contained the pubkey. + fetch_topology + if [[ "$TOPO_CODE" != "200" ]]; then + say " ❌ hide-failed: /api/analytics/topology HTTP $TOPO_CODE — topology not checked" + fails=$((fails+1)) + elif ! jq -r "$TOPOLOGY_PUBKEYS_JQ" "$TMP/topo.json" >"$TMP/topo.pubkeys" 2>"$TMP/topo.err"; then + say " ❌ hide-failed: /api/analytics/topology is not the expected topology JSON — topology not checked" + fails=$((fails+1)) else - file_has_pattern "$PK_PATTERN" "$TMP/topo.json" + file_has_line "$PK_PATTERN" "$TMP/topo.pubkeys" case $? in - 0) say " ❌ hide-failed: /api/topology references blacklisted pubkey" + 0) say " ❌ hide-failed: /api/analytics/topology lists the blacklisted pubkey" fails=$((fails+1)) ;; - 1) say " ✅ topology clean" ;; - *) say " ❌ hide-failed: could not search /api/topology response (grep error)" + 1) say " ✅ topology clean ($(grep -c . "$TMP/topo.pubkeys") pubkey fields checked)" ;; + *) say " ❌ hide-failed: could not search /api/analytics/topology response (grep error)" fails=$((fails+1)) ;; esac fi diff --git a/qa/scripts/test-blacklist-sql.sh b/qa/scripts/test-blacklist-sql.sh index 890fbdd4a..5e1e1e335 100755 --- a/qa/scripts/test-blacklist-sql.sh +++ b/qa/scripts/test-blacklist-sql.sh @@ -312,6 +312,7 @@ case $1 in "$FAKE_REAL_JQ" -r '.nodeBlacklist // [] | .[] | ascii_downcase' "$FAKE_CONFIG" \ >"$FAKE_STATE/live-blacklist" || exit 1 { echo "restart"; cat "$FAKE_STATE/live-blacklist"; } >>"$FAKE_STATE/restarts" 2>/dev/null + "$FAKE_REAL_JQ" -c '.nodeBlacklist' "$FAKE_CONFIG" >>"$FAKE_STATE/restarts-raw" echo "$2" ;; exec) shift; [ "$1" = -i ] && shift @@ -368,6 +369,39 @@ hidden() { [ "${FAKE_DETAIL_LEAK:-0}" = 1 ] && [ "${path#/api/nodes/}" != "$path" ] && return 1 local b; for b in ${BL[@]+"${BL[@]}"}; do [ "$b" = "$1" ] && return 0; done; return 1 } +topo_el() { # PART PUBKEY → one element of that part + local h=${2:0:2} + case $1 in + topRepeaters) printf '{"hop":"%s","count":3,"name":"n","pubkey":"%s"}' "$h" "$2" ;; + topPairsA) printf '{"hopA":"%s","hopB":"ee","count":1,"nameA":"n","nameB":null,"pubkeyA":"%s","pubkeyB":"%s"}' "$h" "$2" "$FAKE_THIRD" ;; + topPairsB) printf '{"hopA":"ee","hopB":"%s","count":1,"nameA":null,"nameB":"n","pubkeyA":"%s","pubkeyB":"%s"}' "$h" "$FAKE_THIRD" "$2" ;; + bestPathList) printf '{"hop":"%s","name":"n","pubkey":"%s","minDist":1,"observer_id":"obs1","observer_name":"o"}' "$h" "$2" ;; + multiObsNodes) printf '{"hop":"%s","name":"n","pubkey":"%s","observers":[]}' "$h" "$2" ;; + perObserverReach) printf '{"hop":"%s","name":"n","pubkey":"%s","count":1}' "$h" "$2" ;; + esac +} +topo_part() { # PART... → JSON array of the visible nodes (plus a leak), or null + local part n out='' sep='' + for part in "$@"; do + for n in $FAKE_NODES; do + hidden "$n" && continue + out+="$sep$(topo_el "$part" "$n")"; sep=',' + done + if [ "${FAKE_TOPO_LEAK:-}" = "$part" ]; then + out+="$sep$(topo_el "$part" "$FAKE_TOPO_LEAK_PK")"; sep=',' + fi + done + if [ -z "$out" ] || [ "${FAKE_TOPO:-}" = nulls ]; then printf 'null'; else printf '[%s]' "$out"; fi +} +topo_body() { + body='{"uniqueNodes":2,"avgHops":1.5,"medianHops":1,"maxHops":2,"hopDistribution":[{"hops":1,"count":2}],' + body+="\"topRepeaters\":$(topo_part topRepeaters)," + body+="\"topPairs\":$(topo_part topPairsA topPairsB)," + body+='"hopsVsSnr":[],"observers":[{"id":"obs1","name":"o"}],' + body+="\"perObserverReach\":{\"obs1\":{\"observer_name\":\"o\",\"rings\":[{\"hops\":1,\"nodes\":$(topo_part perObserverReach)}]}}," + body+="\"multiObsNodes\":$(topo_part multiObsNodes)," + body+="\"bestPathList\":$(topo_part bestPathList)}" +} code=404; body='{"error":"not found"}' case $path in /api/stats) code=200; body='{"ok":true}' ;; @@ -380,10 +414,22 @@ case $path in code=${FAKE_LIST_CODE:-200}; body='{"nodes":['; sep='' for n in $FAKE_NODES; do hidden "$n" && continue; body+="$sep{\"public_key\":\"$n\"}"; sep=','; done body+=']}' ;; - /api/topology) - code=200; body='{"nodes":['; sep='' - for n in $FAKE_NODES; do hidden "$n" && continue; body+="$sep\"$n\""; sep=','; done - body+=']}' ;; + /api/analytics/topology) + # cmd/server TopologyResponse. Pubkeys sit in topRepeaters[].pubkey, + # topPairs[].pubkeyA/B, bestPathList[].pubkey, multiObsNodes[].pubkey + # and perObserverReach{}.rings[].nodes[].pubkey. FAKE_TOPO picks a + # broken variant; FAKE_TOPO_LEAK puts the hidden node back into one part. + if [ "$hits" -le "${FAKE_TOPO_503:-0}" ]; then + code=503; body='{"error":"analytics warming up","retry_after_s":5}' + else + case ${FAKE_TOPO:-json} in + html) code=200; body='CoreScope
' ;; + badshape) code=200; body='{"nodes":[],"edges":[]}' ;; + notjson) code=200; body='{"uniqueNodes": 2, "topRepeaters": [' ;; + 500) code=500; body='{"error":"internal"}' ;; + *) code=200; topo_body ;; + esac + fi ;; esac printf '%s' "$body" >"$out" [ "$fmt" = '%{http_code}' ] && printf '%s' "$code" @@ -422,13 +468,17 @@ FAKE chmod +x "$NOISY_BIN/sqlite3" FAKE_CONFIG="$FAKE/target/config.json" - ORIG_CONFIG='{"port":3000,"nodeBlacklist":["aa00aa00"],"mqtt":{"sources":[]}}' + # Unsorted, with a duplicate and an upper-case entry: the run must restore + # this array exactly — not sorted, not de-duplicated, not case-folded. + DEFAULT_CONFIG='{"port":3000,"nodeBlacklist":["ff00ff00","AA00AA00","ff00ff00"],"mqtt":{"sources":[]}}' + ORIG_CONFIG=$DEFAULT_CONFIG + PK_C="7777777777777777777777777777777777777777777777777777777777777777" SYNTH_TOKEN="synthetic-admin-token-not-a-secret-83" export FAKE_ARGV_LOG="$FAKE/argv.log" FAKE_STATE FAKE_CROOT FAKE_CONFIG export FAKE_REAL_TEE; FAKE_REAL_TEE=$(command -v tee) export FAKE_REAL_JQ; FAKE_REAL_JQ=$(command -v jq) export FAKE_CONTAINER="corescope-stub" FAKE_URL="http://target.invalid" - export FAKE_NODES="$PK_A $PK_B" + export FAKE_NODES="$PK_A $PK_B" FAKE_THIRD="$PK_C" FAKE_TOPO_LEAK_PK="$PK_A" ALL_ARGV="$FAKE/all-argv.log"; : >"$ALL_ARGV" RUN_N=0 # Optional strace evidence (Linux): needs strace and an EMPTY directory, so @@ -452,14 +502,15 @@ FAKE run_full() { local kv wrap=() rm -rf "$FAKE_STATE"; mkdir -p "$FAKE_STATE"; : >"$FAKE_STATE/live-blacklist" - printf '%s\n' "$ORIG_CONFIG" >"$FAKE_CONFIG" + printf '%s\n' "$ORIG_CONFIG" >"$FAKE_CONFIG"; chmod 640 "$FAKE_CONFIG" + CONFIG_SUM=$(cksum <"$FAKE_CONFIG"); CONFIG_PERM=$(config_perm) : >"$FAKE_ARGV_LOG" FILES_BEFORE=$(snapshot_files) RUN_N=$((RUN_N + 1)) ( unset TARGET_DB_PATH ADMIN_API_TOKEN TARGET_CONTAINER_DB_PATH TARGET_HOST_DB_PATH \ FAKE_CONTAINER_SQLITE FAKE_SIGNAL_ON FAKE_SIGNAL FAKE_SIGNAL_NTH FAKE_LEAK \ - FAKE_DETAIL_LEAK FAKE_LIST_CODE \ + FAKE_DETAIL_LEAK FAKE_LIST_CODE FAKE_TOPO FAKE_TOPO_503 FAKE_TOPO_LEAK \ FAKE_SSH_FAIL_CMD FAKE_SSH_FAIL_FROM export TEST_NODE_PUBKEY="$PK_A" TARGET_SSH_HOST="stub-host" TARGET_SSH_KEY="/nonexistent/key" \ TARGET_CONFIG_PATH="$FAKE_CONFIG" TARGET_CONTAINER="$FAKE_CONTAINER" \ @@ -485,6 +536,10 @@ FAKE RUN_OUT=$(cat "$FAKE/out"); RUN_ERR=$(cat "$FAKE/err") cat "$FAKE_ARGV_LOG" >>"$ALL_ARGV" } + config_perm() { ls -l "$FAKE_CONFIG" | awk '{print $1, $3, $4}'; } + # GNU chmod --reference is what the remote edit uses to keep the mode; the + # macOS fake "remote" lacks it, so mode is only compared where it exists. + GNU_CHMOD=0; chmod --reference="$FIXTURE_DIR" "$FIXTURE_DIR" 2>/dev/null && GNU_CHMOD=1 restarts() { if [ -f "$FAKE_STATE/restarts" ]; then awk '$0 == "restart" { n++ } END { print n + 0 }' "$FAKE_STATE/restarts" else echo 0; fi @@ -510,6 +565,8 @@ FAKE assert_true "$label: no new or leftover files" files_unchanged assert_true "$label: argv log recorded the run" test -s "$FAKE_ARGV_LOG" assert_true "$label: pubkey not in any argv" lacks "$(cat "$FAKE_ARGV_LOG")" "$PK_A" + assert_true "$label: upper-case pubkey not in any argv" lacks "$(cat "$FAKE_ARGV_LOG")" "$PK_A_UPPER" + if [ "$GNU_CHMOD" = 1 ]; then assert_eq "$label: config mode/owner kept" "$CONFIG_PERM" "$(config_perm)"; fi assert_true "$label: SQL not in any argv" lacks "$(cat "$FAKE_ARGV_LOG")" "SELECT" assert_true "$label: token not in any argv" lacks "$(cat "$FAKE_ARGV_LOG")" "$SYNTH_TOKEN" assert_true "$label: remote input exactly as built" remote_input_clean @@ -524,7 +581,7 @@ FAKE cmd=$(cat "$FAKE_STATE/ssh.$n.cmd"); first=$(head -n 1 "$FAKE_STATE/ssh.$n.stdin") case $cmd in *"bash -s") - [[ "$cmd" =~ ^CFG=[^[:space:]]+\ MODE=(add|remove)\ bash\ -s$ ]] || { echo " unexpected edit cmd: $cmd" >&2; return 1; } + [[ "$cmd" =~ ^CFG=[^[:space:]]+\ MODE=(check|add|remove)\ bash\ -s$ ]] || { echo " unexpected edit cmd: $cmd" >&2; return 1; } [ "$first" = "IFS= read -r PK || exit 3" ] || { echo " unexpected edit stdin: $first" >&2; return 1; } ;; *sqlite3*) [ "$first" = ".parameter init" ] || { echo " unexpected sqlite stdin: $first" >&2; return 1; } ;; @@ -542,13 +599,15 @@ FAKE assert_true "host fallback: runner named" contains "$RUN_OUT" "sqlite3 runner: host (TARGET_HOST_DB_PATH)" assert_true "host fallback: host db count" contains "$RUN_OUT" "DB retains 3 packets" assert_true "host fallback: hidden" contains "$RUN_OUT" "hide ok: detail=404 in_list=0" - assert_true "host fallback: topology" contains "$RUN_OUT" "topology clean" + assert_true "host fallback: topology" contains "$RUN_OUT" "✅ topology clean" assert_true "host fallback: teardown ok" contains "$RUN_OUT" "teardown ok" assert_eq "host fallback: two restarts" "2" "$(restarts)" # The pubkey reached the app through the stdin edit: the first restart saw # it in the blacklist, the second (teardown) did not. - assert_eq "stdin edit: added, then removed" "restart $PK_A aa00aa00 restart aa00aa00" \ + assert_eq "stdin edit: appended, then removed" "restart ff00ff00 aa00aa00 ff00ff00 $PK_A restart ff00ff00 aa00aa00 ff00ff00" \ "$(tr '\n' ' ' <"$FAKE_STATE/restarts" | sed 's/ $//')" + assert_eq "blacklist order, duplicate and case kept while added" \ + "[\"ff00ff00\",\"AA00AA00\",\"ff00ff00\",\"$PK_A\"]" "$(head -n 1 "$FAKE_STATE/restarts-raw")" common_after "host fallback" assert_true "host fallback: container path never used" lacks "$(cat "$FAKE_ARGV_LOG")" "$CONTAINER_DB_PATH" assert_true "host fallback: host path used" argv_has "$HOST_DB_PATH" @@ -561,6 +620,9 @@ FAKE assert_true "curl reads its URL from stdin" grep -q '^curl .*-K -' "$FAKE_ARGV_LOG" assert_true "no URL in curl argv" lacks "$(grep '^curl ' "$FAKE_ARGV_LOG")" "$FAKE_URL" edit=$(ssh_call "bash -s") + assert_eq "first remote call is the read-only check" \ + "CFG=$FAKE_CONFIG MODE=check bash -s" "$(cat "$FAKE_STATE/ssh.1.cmd")" + edit=$(ssh_call "MODE=add bash -s") assert_eq "config-edit command carries no value" \ "CFG=$FAKE_CONFIG MODE=add bash -s" "$(cat "$FAKE_STATE/ssh.$edit.cmd")" assert_eq "config-edit stdin: pubkey is line 2" "$PK_A" "$(sed -n 2p "$FAKE_STATE/ssh.$edit.stdin")" @@ -634,7 +696,8 @@ FAKE run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_REMOTE_PATH="$SHIM_NOJQ_DIR" assert_eq "python3 edit: exit 0" "0" "$RUN_RC" assert_true "python3 edit: python3 did the edit" grep -q '^python3 - ' "$FAKE_ARGV_LOG" - assert_true "python3 edit: jq not used remotely" lacks "$(cat "$FAKE_ARGV_LOG")" "jq " + assert_true "python3 edit: jq not used remotely" lacks "$(grep '^jq ' "$FAKE_ARGV_LOG")" "nodeBlacklist" + assert_true "python3 edit: python3 did the check" grep -q "^python3 - $FAKE_CONFIG\$" "$FAKE_ARGV_LOG" common_after "python3 edit" # 10. Removed variables refuse to start: nothing runs against the target. @@ -659,7 +722,7 @@ FAKE run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_LEAK=1 assert_eq "server leaks: exit 2" "2" "$RUN_RC" assert_true "server leaks: hide-failed" contains "$RUN_OUT" "hide-failed: detail=200 in_list=1" - assert_true "server leaks: topology hide-failed" contains "$RUN_OUT" "/api/topology references blacklisted pubkey" + assert_true "server leaks: topology hide-failed" contains "$RUN_OUT" "/api/analytics/topology lists the blacklisted pubkey" common_after "server leaks" # 11b. grep itself failing is a failure, never "pubkey absent" (which, for @@ -670,7 +733,7 @@ FAKE run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_REAL_PATH="$GREP_ERR_BIN:$ORIG_PATH" assert_eq "grep error: both searches fail" "2" "$RUN_RC" assert_true "grep error: list classified" contains "$RUN_OUT" "could not search /api/nodes response" - assert_true "grep error: topology classified" contains "$RUN_OUT" "could not search /api/topology response" + assert_true "grep error: topology classified" contains "$RUN_OUT" "could not search /api/analytics/topology response" assert_true "grep error: no hide ok claimed" lacks "$RUN_OUT" "hide ok" common_after "grep error" @@ -700,9 +763,87 @@ FAKE assert_true "noisy sqlite3: classified" contains "$RUN_OUT" "retain-failed: no sqlite3 able to bind" common_after "noisy sqlite3" + # 11f. PRE-BLACKLISTED (issue #83 review): the pubkey is already a privacy + # rule on the target. The run must refuse before any side effect — + # teardown would otherwise remove the operator's rule — and must not + # print the pubkey. Lower- and upper-case entries, jq and python3. + for pre in "lower:$PK_A:$SHIM_DIR" "upper:$PK_A_UPPER:$SHIM_DIR" "python3:$PK_A_UPPER:$SHIM_NOJQ_DIR"; do + label="pre-blacklisted ${pre%%:*}"; entry=${pre#*:}; entry=${entry%%:*}; rpath=${pre##*:} + ORIG_CONFIG="{\"nodeBlacklist\":[\"ff00ff00\",\"$entry\"],\"port\":3000}" + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_REMOTE_PATH="$rpath" + assert_eq "$label: refused, exit 2" "2" "$RUN_RC" + assert_true "$label: says why" contains "$RUN_OUT$RUN_ERR" "already in nodeBlacklist" + assert_eq "$label: no restart" "0" "$(restarts)" + assert_eq "$label: config bytes unchanged" "$CONFIG_SUM" "$(cksum <"$FAKE_CONFIG")" + assert_eq "$label: config mode/owner unchanged" "$CONFIG_PERM" "$(config_perm)" + assert_eq "$label: entry still blacklisted" "$entry" "$(jq -r '.nodeBlacklist[1]' "$FAKE_CONFIG")" + assert_true "$label: no add/remove sent" lacks "$(cat "$FAKE_STATE"/ssh.*.cmd)" "MODE=add" + assert_true "$label: no teardown remove" lacks "$(cat "$FAKE_STATE"/ssh.*.cmd)" "MODE=remove" + assert_true "$label: no API call" lacks "$(cat "$FAKE_ARGV_LOG")" "curl " + assert_true "$label: pubkey not printed" lacks "$RUN_OUT$RUN_ERR" "$PK_A" + assert_true "$label: upper pubkey not printed" lacks "$RUN_OUT$RUN_ERR" "$PK_A_UPPER" + common_after "$label" + done + ORIG_CONFIG=$DEFAULT_CONFIG + # The check itself failing (ssh down) is a failure before any side effect. + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_SSH_FAIL_CMD="MODE=check" FAKE_SSH_FAIL_FROM=1 + assert_eq "blacklist check unreachable: exit 1" "1" "$RUN_RC" + assert_true "blacklist check unreachable: classified" contains "$RUN_OUT" "ssh-failed: could not read nodeBlacklist" + assert_eq "blacklist check unreachable: no restart" "0" "$(restarts)" + assert_eq "blacklist check unreachable: config bytes unchanged" "$CONFIG_SUM" "$(cksum <"$FAKE_CONFIG")" + + # 11g. Topology is /api/analytics/topology, and only a 200 with the expected + # JSON shape can pass. Everything else fails — never skipped. + for bad in "html:not the expected topology JSON" "badshape:not the expected topology JSON" \ + "notjson:not the expected topology JSON" "500:/api/analytics/topology HTTP 500"; do + v=${bad%%:*}; want=${bad#*:} + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_TOPO="$v" + assert_eq "topology $v: fails" "1" "$RUN_RC" + assert_true "topology $v: classified" contains "$RUN_OUT" "$want" + assert_true "topology $v: not clean" lacks "$RUN_OUT" "topology clean" + assert_true "topology $v: not skipped" lacks "$RUN_OUT" "skipping topology" + common_after "topology $v" + done + # Warm-up 503s after the restart are waited out; a 503 that never ends fails. + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_TOPO_503=2 + assert_eq "topology warm-up then 200: passes" "0" "$RUN_RC" + assert_true "topology warm-up then 200: clean" contains "$RUN_OUT" "✅ topology clean" + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_TOPO_503=999 + assert_eq "topology stuck warming up: fails" "1" "$RUN_RC" + assert_true "topology stuck warming up: classified" contains "$RUN_OUT" "/api/analytics/topology HTTP 503" + common_after "topology stuck warming up" + # Arrays the server nulls out after filtering are a valid, clean shape. + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_TOPO=nulls + assert_eq "topology null arrays: clean" "0" "$RUN_RC" + assert_true "topology null arrays: reported clean" contains "$RUN_OUT" "✅ topology clean" + # The blacklisted pubkey back in each part, in both cases. + for part in topRepeaters topPairsA topPairsB bestPathList multiObsNodes perObserverReach; do + for leak in "$PK_A" "$PK_A_UPPER"; do + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_TOPO_LEAK="$part" FAKE_TOPO_LEAK_PK="$leak" + assert_eq "topology leak in $part (${leak:0:4}): fails" "1" "$RUN_RC" + assert_true "topology leak in $part (${leak:0:4}): classified" contains "$RUN_OUT" "/api/analytics/topology lists the blacklisted pubkey" + done + done + common_after "topology leak" + # The old route must not be what is asked for. + assert_true "old /api/topology route never requested" lacks "$(cat "$FAKE_STATE"/hits.* 2>/dev/null; ls "$FAKE_STATE")" "hits._api_topology" + + # 11h. Upper-case TEST_NODE_PUBKEY while the API and the database hold the + # canonical lower-case form: one canonical value everywhere. + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" TEST_NODE_PUBKEY="$PK_A_UPPER" + assert_eq "upper-case input: passes" "0" "$RUN_RC" + assert_true "upper-case input: hidden" contains "$RUN_OUT" "hide ok: detail=404 in_list=0" + assert_true "upper-case input: topology" contains "$RUN_OUT" "✅ topology clean" + assert_true "upper-case input: db count" contains "$RUN_OUT" "DB retains 3 packets" + assert_true "upper-case input: teardown ok" contains "$RUN_OUT" "teardown ok" + assert_eq "upper-case input: config got the canonical form" \ + "[\"ff00ff00\",\"AA00AA00\",\"ff00ff00\",\"$PK_A\"]" "$(head -n 1 "$FAKE_STATE/restarts-raw")" + assert_true "upper-case input: detail path canonical" test -f "$FAKE_STATE/hits._api_nodes_$(printf '%s' "$PK_A" | tr -c 'a-z\n' '_')" + common_after "upper-case input" + # 12. Signals mid-run: classified exit, full teardown. for sig in TERM:143 INT:130 HUP:129; do - run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_SIGNAL_ON=/api/topology FAKE_SIGNAL="${sig%%:*}" + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_SIGNAL_ON=/api/analytics/topology FAKE_SIGNAL="${sig%%:*}" assert_eq "SIG${sig%%:*} mid-run: exit ${sig#*:}" "${sig#*:}" "$RUN_RC" assert_true "SIG${sig%%:*} mid-run: teardown ok" contains "$RUN_OUT" "teardown ok" assert_eq "SIG${sig%%:*} mid-run: two restarts" "2" "$(restarts)" @@ -710,7 +851,7 @@ FAKE done # 13. SIGTERM mid-run and a failing teardown restart → 143 + 1. - run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_SIGNAL_ON=/api/topology FAKE_SIGNAL=TERM \ + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_SIGNAL_ON=/api/analytics/topology FAKE_SIGNAL=TERM \ FAKE_SSH_FAIL_CMD="docker restart" FAKE_SSH_FAIL_FROM=2 assert_eq "SIGTERM + failed teardown: exit 144" "144" "$RUN_RC" assert_true "SIGTERM + failed teardown: classified" contains "$RUN_OUT" "teardown-failed" From 350f6dae115e1c334db833b8d3e5c7487a703df2 Mon Sep 17 00:00:00 2001 From: Dennis Jakobsen Date: Thu, 24 Sep 2026 07:58:48 +0200 Subject: [PATCH 6/8] test(qa): mixed-case and python3 pre-blacklisted entries (#83) A python3 check that compared against the upper-cased pubkey survived: the only python3 case used an upper-case entry. Mixed-case entries, which only a case-insensitive comparison matches, and a lower-case python3 case now cover both check implementations. Co-Authored-By: Claude Opus 5.5 --- qa/scripts/test-blacklist-sql.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/qa/scripts/test-blacklist-sql.sh b/qa/scripts/test-blacklist-sql.sh index 5e1e1e335..bfcecb7c8 100755 --- a/qa/scripts/test-blacklist-sql.sh +++ b/qa/scripts/test-blacklist-sql.sh @@ -767,7 +767,11 @@ FAKE # rule on the target. The run must refuse before any side effect — # teardown would otherwise remove the operator's rule — and must not # print the pubkey. Lower- and upper-case entries, jq and python3. - for pre in "lower:$PK_A:$SHIM_DIR" "upper:$PK_A_UPPER:$SHIM_DIR" "python3:$PK_A_UPPER:$SHIM_NOJQ_DIR"; do + # Mixed case only matches a genuinely case-insensitive comparison. + PK_A_MIXED="$(printf '%s' "${PK_A:0:32}" | tr 'a-f' 'A-F')${PK_A:32}" + for pre in "lower:$PK_A:$SHIM_DIR" "upper:$PK_A_UPPER:$SHIM_DIR" "mixed:$PK_A_MIXED:$SHIM_DIR" \ + "python3-lower:$PK_A:$SHIM_NOJQ_DIR" "python3-upper:$PK_A_UPPER:$SHIM_NOJQ_DIR" \ + "python3-mixed:$PK_A_MIXED:$SHIM_NOJQ_DIR"; do label="pre-blacklisted ${pre%%:*}"; entry=${pre#*:}; entry=${entry%%:*}; rpath=${pre##*:} ORIG_CONFIG="{\"nodeBlacklist\":[\"ff00ff00\",\"$entry\"],\"port\":3000}" run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_REMOTE_PATH="$rpath" From 877e0938c8867d30aa1277527c866bcfb41a4b6f Mon Sep 17 00:00:00 2001 From: Dennis Jakobsen Date: Thu, 24 Sep 2026 08:09:55 +0200 Subject: [PATCH 7/8] reviewfix(qa): one-document topology parse, server-equal blacklist check (#83) Independent review of 41b2e0e3/350f6dae found no blockers and these gaps: - An empty or blank 200 body from /api/analytics/topology still printed "topology clean": plain `jq FILE` accepts no input, prints nothing and exits 0. jq now reads the body itself (-n, [inputs]) and requires exactly one JSON document; two concatenated documents fail as well. - The preflight compared lower-cased only, while the server trims and lower-cases (buildBlacklistSet); a padded entry was not recognised. Both remote implementations now trim. - The preflight's error branches were untested. Tests now cover a non-array, object-valued or non-JSON nodeBlacklist/config (exit 1 before any side effect, jq and python3), a missing or null list (runs, comes back as []), padded entries, and topology bodies that are empty, blank, two documents, of the wrong field type or with a non-numeric uniqueNodes. Co-Authored-By: Claude Opus 5.5 --- qa/scripts/blacklist-test.sh | 17 ++++++++--- qa/scripts/test-blacklist-sql.sh | 52 ++++++++++++++++++++++++++++++-- 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/qa/scripts/blacklist-test.sh b/qa/scripts/blacklist-test.sh index 8e4b41706..bdc5e2b12 100755 --- a/qa/scripts/blacklist-test.sh +++ b/qa/scripts/blacklist-test.sh @@ -210,9 +210,13 @@ file_has_pattern() { # one per line, and compared whole-line with the canonical pubkey — not a text # grep over the body. Arrays the server filtered empty come back as null. # A shape mismatch makes jq exit non-zero ("not the expected topology JSON"). +# jq runs with -n and reads the body itself: exactly one JSON document, so an +# empty or blank body (which plain `jq FILE` accepts silently, printing +# nothing — "clean") or two concatenated documents fail too. TOPOLOGY_PUBKEYS_JQ=' def list(k): .[k] // [] | if type == "array" then . else error("\(k) is not an array") end; - if type != "object" then error("not an object") else . end + [inputs] | if length == 1 then .[0] else error("expected one JSON document, got \(length)") end + | if type != "object" then error("not an object") else . end | . as $r | if (["uniqueNodes","topRepeaters","topPairs","bestPathList","multiObsNodes","perObserverReach"] | all(. as $k | $r | has($k))) and ((.uniqueNodes | type) == "number") @@ -272,7 +276,10 @@ restart_target() { # through the environment. It is therefore in neither the local ssh argv nor # any remote argv; only the config path and the constant mode are on the # command line. Returns the remote status; for check: 0 = not blacklisted, -# 10 = already blacklisted (any case), anything else = could not tell. +# 10 = already blacklisted, anything else = could not tell. check compares the +# way the server does (cmd/server/config.go buildBlacklistSet): trimmed and +# lower-cased. A missing or null nodeBlacklist means none; a non-array one, or +# a config that is not JSON, is "could not tell" — never "not blacklisted". # # add appends and remove drops only the exact canonical value, so the rest of # nodeBlacklist — order, duplicates, other spellings — is left as it was. add @@ -293,7 +300,7 @@ if [ "$MODE" = check ]; then if command -v jq >/dev/null; then if jq -e '(.nodeBlacklist // []) | if type == "array" then . else error("nodeBlacklist is not an array") end - | any(.[]; type == "string" and ascii_downcase == (env.PK | ascii_downcase)) | not' \ + | any(.[]; type == "string" and (ascii_downcase | gsub("^\\s+|\\s+$"; "")) == (env.PK | ascii_downcase)) | not' \ "$CFG" >/dev/null; then exit 0 else @@ -308,7 +315,7 @@ try: except Exception: sys.exit(4) if not isinstance(bl, list): sys.exit(4) -sys.exit(10 if any(isinstance(x, str) and x.lower() == pk for x in bl) else 0) +sys.exit(10 if any(isinstance(x, str) and x.strip().lower() == pk for x in bl) else 0) PY exit 0 fi @@ -719,7 +726,7 @@ main() { if [[ "$TOPO_CODE" != "200" ]]; then say " ❌ hide-failed: /api/analytics/topology HTTP $TOPO_CODE — topology not checked" fails=$((fails+1)) - elif ! jq -r "$TOPOLOGY_PUBKEYS_JQ" "$TMP/topo.json" >"$TMP/topo.pubkeys" 2>"$TMP/topo.err"; then + elif ! jq -r -n "$TOPOLOGY_PUBKEYS_JQ" "$TMP/topo.json" >"$TMP/topo.pubkeys" 2>"$TMP/topo.err"; then say " ❌ hide-failed: /api/analytics/topology is not the expected topology JSON — topology not checked" fails=$((fails+1)) else diff --git a/qa/scripts/test-blacklist-sql.sh b/qa/scripts/test-blacklist-sql.sh index bfcecb7c8..b8de537e7 100755 --- a/qa/scripts/test-blacklist-sql.sh +++ b/qa/scripts/test-blacklist-sql.sh @@ -426,6 +426,11 @@ case $path in html) code=200; body='CoreScope
' ;; badshape) code=200; body='{"nodes":[],"edges":[]}' ;; notjson) code=200; body='{"uniqueNodes": 2, "topRepeaters": [' ;; + empty) code=200; body='' ;; + blank) code=200; body=' ' ;; + twodocs) code=200; topo_body; body="$body$body" ;; + wrongtype) code=200; topo_body; body=${body/\"topRepeaters\":\[/\"topRepeaters\":\{\"x\":\[}; body=${body/\],\"topPairs\"/\]\},\"topPairs\"} ;; + badunique) code=200; topo_body; body=${body/\"uniqueNodes\":2/\"uniqueNodes\":\"2\"} ;; 500) code=500; body='{"error":"internal"}' ;; *) code=200; topo_body ;; esac @@ -544,7 +549,11 @@ FAKE if [ -f "$FAKE_STATE/restarts" ]; then awk '$0 == "restart" { n++ } END { print n + 0 }' "$FAKE_STATE/restarts" else echo 0; fi } - config_restored() { [ "$(jq -S . "$FAKE_CONFIG")" = "$(printf '%s' "$ORIG_CONFIG" | jq -S .)" ]; } + # A missing or null nodeBlacklist comes back as [] — the same to the server + # (documented); everything else, the array's order included, must match. + config_restored() { + [ "$(jq -S '.nodeBlacklist //= []' "$FAKE_CONFIG")" = "$(printf '%s' "$ORIG_CONFIG" | jq -S '.nodeBlacklist //= []')" ] + } files_unchanged() { [ "$(snapshot_files)" = "$FILES_BEFORE" ]; } argv_has() { grep -qF -- "$1" "$FAKE_ARGV_LOG"; } # The ssh call whose command contains $1: prints its number. @@ -788,6 +797,42 @@ FAKE assert_true "$label: upper pubkey not printed" lacks "$RUN_OUT$RUN_ERR" "$PK_A_UPPER" common_after "$label" done + # The server trims entries before comparing (cmd/server/config.go + # buildBlacklistSet), so a padded entry is the same rule. + for pre in "padded:$SHIM_DIR" "python3-padded:$SHIM_NOJQ_DIR"; do + label="pre-blacklisted ${pre%%:*}" + ORIG_CONFIG="{\"nodeBlacklist\":[\" $PK_A_UPPER \"]}" + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_REMOTE_PATH="${pre#*:}" + assert_eq "$label: refused, exit 2" "2" "$RUN_RC" + assert_eq "$label: no restart" "0" "$(restarts)" + assert_eq "$label: config bytes unchanged" "$CONFIG_SUM" "$(cksum <"$FAKE_CONFIG")" + done + # A config the check cannot read is a failure before any side effect — + # never "not blacklisted". Both remote implementations. + for cfgcase in 'nonarray:{"nodeBlacklist":"ff00ff00"}' 'object:{"nodeBlacklist":{"a":1}}' 'notjson:{"nodeBlacklist":[' 'toplevel:[1,2]'; do + for rpath in "jq:$SHIM_DIR" "python3:$SHIM_NOJQ_DIR"; do + label="unreadable config ${cfgcase%%:*} (${rpath%%:*})" + ORIG_CONFIG=${cfgcase#*:} + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_REMOTE_PATH="${rpath#*:}" + assert_eq "$label: exit 1" "1" "$RUN_RC" + assert_true "$label: classified" contains "$RUN_OUT" "could not read nodeBlacklist" + assert_eq "$label: no restart" "0" "$(restarts)" + assert_eq "$label: config bytes unchanged" "$CONFIG_SUM" "$(cksum <"$FAKE_CONFIG")" + assert_true "$label: no add sent" lacks "$(cat "$FAKE_STATE"/ssh.*.cmd)" "MODE=add" + done + done + # No nodeBlacklist key, or null: nothing is blacklisted, the run proceeds, + # and the list comes back empty. + for cfgcase in 'missing:{"port":3000}' 'null:{"port":3000,"nodeBlacklist":null}'; do + for rpath in "jq:$SHIM_DIR" "python3:$SHIM_NOJQ_DIR"; do + label="no blacklist ${cfgcase%%:*} (${rpath%%:*})" + ORIG_CONFIG=${cfgcase#*:} + run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_REMOTE_PATH="${rpath#*:}" + assert_eq "$label: passes" "0" "$RUN_RC" + assert_eq "$label: list back to empty" "[]" "$(jq -c '.nodeBlacklist' "$FAKE_CONFIG")" + common_after "$label" + done + done ORIG_CONFIG=$DEFAULT_CONFIG # The check itself failing (ssh down) is a failure before any side effect. run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_SSH_FAIL_CMD="MODE=check" FAKE_SSH_FAIL_FROM=1 @@ -799,7 +844,10 @@ FAKE # 11g. Topology is /api/analytics/topology, and only a 200 with the expected # JSON shape can pass. Everything else fails — never skipped. for bad in "html:not the expected topology JSON" "badshape:not the expected topology JSON" \ - "notjson:not the expected topology JSON" "500:/api/analytics/topology HTTP 500"; do + "notjson:not the expected topology JSON" "500:/api/analytics/topology HTTP 500" \ + "empty:not the expected topology JSON" "blank:not the expected topology JSON" \ + "twodocs:not the expected topology JSON" "wrongtype:not the expected topology JSON" \ + "badunique:not the expected topology JSON"; do v=${bad%%:*}; want=${bad#*:} run_full TARGET_HOST_DB_PATH="$HOST_DB_PATH" FAKE_TOPO="$v" assert_eq "topology $v: fails" "1" "$RUN_RC" From ec90ea24fb7f74bf795386f48e2cc8d9f07de4aa Mon Sep 17 00:00:00 2001 From: Dennis Jakobsen Date: Thu, 24 Sep 2026 08:59:03 +0200 Subject: [PATCH 8/8] chore(qa): justify two shellcheck info findings (#83) SC2016 on the topology jq program ($r/$k are jq variables) and SC2012 on the test's ls -l mode/owner read (portable across macOS and Linux). The CI shellcheck step fails on info-level findings. Co-Authored-By: Claude Opus 5.5 --- qa/scripts/blacklist-test.sh | 1 + qa/scripts/test-blacklist-sql.sh | 3 +++ 2 files changed, 4 insertions(+) diff --git a/qa/scripts/blacklist-test.sh b/qa/scripts/blacklist-test.sh index bdc5e2b12..b21947e77 100755 --- a/qa/scripts/blacklist-test.sh +++ b/qa/scripts/blacklist-test.sh @@ -213,6 +213,7 @@ file_has_pattern() { # jq runs with -n and reads the body itself: exactly one JSON document, so an # empty or blank body (which plain `jq FILE` accepts silently, printing # nothing — "clean") or two concatenated documents fail too. +# shellcheck disable=SC2016 # a jq program: $r and $k are jq variables TOPOLOGY_PUBKEYS_JQ=' def list(k): .[k] // [] | if type == "array" then . else error("\(k) is not an array") end; [inputs] | if length == 1 then .[0] else error("expected one JSON document, got \(length)") end diff --git a/qa/scripts/test-blacklist-sql.sh b/qa/scripts/test-blacklist-sql.sh index b8de537e7..7b5ec9edf 100755 --- a/qa/scripts/test-blacklist-sql.sh +++ b/qa/scripts/test-blacklist-sql.sh @@ -541,6 +541,9 @@ FAKE RUN_OUT=$(cat "$FAKE/out"); RUN_ERR=$(cat "$FAKE/err") cat "$FAKE_ARGV_LOG" >>"$ALL_ARGV" } + # ls -l is the one way to read mode and owner that is the same on macOS and + # Linux (stat's flags differ); the path is a fixed test fixture. + # shellcheck disable=SC2012 config_perm() { ls -l "$FAKE_CONFIG" | awk '{print $1, $3, $4}'; } # GNU chmod --reference is what the remote edit uses to keep the mode; the # macOS fake "remote" lacks it, so mode is only compared where it exists.