From 0c42a6367c11494737e9c6e0345c67a6370d7e58 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Sat, 29 Aug 2026 14:37:27 +0200 Subject: [PATCH] feat: make the bot actually reply on Signal, and gate every way it went silent First live reply, as its own registered account. `!cat` and `!dog` from the phone return images -- verified in the production shape (the bot *invited to an already-existing group*) and under a systemd user unit. Four failures, each of which left the bot looking perfectly healthy: - Invited is not joined. Signal only adds an account outright if the adder holds its profile key; with no profile the add becomes a pending invite, and signal-cli cannot accept one addressed to a phone number ("Cannot find service ID for self"). Group listed, websocket up, /healthz OK, zero messages. register_bot.sh now sets the profile *before* the add, and startup refuses on `member: false`. - MODE=json-rpc-native is a GraalVM build with no AWT: text sends work, every attachment 400s with "Can't load library: awt". Both mode guards now demand exactly json-rpc, and start_container replaces a wrong-mode container instead of reusing it. - The API spells one group two ways -- /v1/groups returns `group.`, envelopes carry what that decodes to -- so the scoping gate rejected the bot's own group. e2e.sh now models both spellings; reverting the fix fails 4 scenarios, which is why the offline suite missed it before. - BIND_PORT in .env was dead: __main__ read os.getenv before anything loaded a .env. The bind address comes from Config now. A taken port used to be uvicorn logging the collision and exiting 0, which every supervisor reads as success. portcheck.py refuses to start, exits non-zero, and names the holder by reading /proc directly -- no subprocess, no PATH assumptions, testable against a fake proc tree. Default port is 8137: not 8000, and below the ephemeral range so the kernel cannot take it. The bot now runs as signal-bot.service (Restart=always, linger checked rather than assumed, StartLimitBurst so a config error fails visibly instead of looping). Verified by SIGKILL and by re-running a command under the unit. 100% branch coverage, 230 tests, all gates green. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_013yaBT45KhHqxTKVCTA87iy --- .env.sample | 4 +- .gitignore | 3 + README.md | 65 ++++++++++++++- fake_signal/routes_control.py | 10 ++- fake_signal/state.py | 4 +- scripts/e2e.sh | 18 ++++- scripts/install_service.sh | 123 ++++++++++++++++++++++++++++ scripts/lib_signal.sh | 75 ++++++++++++++++-- scripts/live_smoke.sh | 10 ++- scripts/register_bot.sh | 33 ++++++-- scripts/signal-bot.service | 38 +++++++++ signal_bot/__main__.py | 30 ++++--- signal_bot/app.py | 6 ++ signal_bot/config.py | 73 +++++++++++++++++ signal_bot/counting.py | 8 +- signal_bot/dispatcher.py | 7 +- signal_bot/portcheck.py | 119 ++++++++++++++++++++++++++++ signal_bot/transport_rest.py | 53 ++++++++++++- tests/test_app.py | 37 +++++++-- tests/test_config.py | 70 +++++++++++++++- tests/test_fake_account.py | 13 +++ tests/test_fake_server.py | 2 +- tests/test_main.py | 49 ++++++++++-- tests/test_portcheck.py | 145 ++++++++++++++++++++++++++++++++++ tests/test_transport_rest.py | 45 ++++++++++- 25 files changed, 984 insertions(+), 56 deletions(-) create mode 100755 scripts/install_service.sh create mode 100644 scripts/signal-bot.service create mode 100644 signal_bot/portcheck.py create mode 100644 tests/test_portcheck.py diff --git a/.env.sample b/.env.sample index 300c0c4..c4f62b3 100644 --- a/.env.sample +++ b/.env.sample @@ -34,6 +34,8 @@ COOLDOWN_SECONDS= CAT_API_URL= DOG_API_URL= -# Optional. Where the /healthz probe binds. Default: 127.0.0.1:8000 +# Optional. Where the /healthz readiness probe binds. +# Default: 127.0.0.1:8137 -- deliberately not 8000, which unrelated dev +# servers grab, and below the ephemeral range so the kernel cannot take it. BIND_HOST= BIND_PORT= diff --git a/.gitignore b/.gitignore index 9ca4be5..cd0cf62 100644 --- a/.gitignore +++ b/.gitignore @@ -60,6 +60,9 @@ cover/ local_settings.py db.sqlite3 db.sqlite3-journal +# The bot's live tally database, created next to the code by default. +signal_bot.sqlite3 +signal_bot.sqlite3-journal # Flask stuff: instance/ diff --git a/README.md b/README.md index 67386c4..961b498 100644 --- a/README.md +++ b/README.md @@ -79,10 +79,73 @@ ratchet mandatory and **de-registered every account whose client lacked it**, so a stale client is how a bot dies silently months later. The script checks the bundled version and refuses rather than warning. +The container runs `MODE=json-rpc`, not `json-rpc-native`. Both expose the +receive WebSocket, but `-native` is a GraalVM native-image build with no AWT, +so every attachment send fails with `Can't load library: awt` — text replies +work and `!cat` returns nothing, which reads as a bug in this repo and is not. + The script solves everything except two steps only a human can do: pasting a captcha token (the captcha exists to stop exactly this automation) and reading the SMS code. Then add the bot's number to your private group from your -personal Signal, and it picks the group and starts. +personal Signal, and it picks the group, verifies it really joined, installs +the service and tails the log. + +#### Adding the bot to a group: invited is not joined + +Signal adds an account to a group outright **only if the person adding it +already holds that account's profile key**. With no profile there is no key, +so the add silently becomes a pending *invitation* — and signal-cli cannot +accept one addressed to a phone number: + +``` +POST /v1/groups///join → 400 +{"error":"Cannot find service ID for self to accept invite"} +``` + +This is why `register_bot.sh` sets the bot's profile **before** telling you to +add it. A group in that state is indistinguishable from a working one from the +outside — `/v1/groups` lists it, the WebSocket connects, `/healthz` answers, +and not one message arrives — so the bot checks `member` at startup and +refuses to run rather than looking healthy and staying deaf: + +``` +TransportError: the account is invited to 'testbot' but has not joined it, +so it will receive nothing. +``` + +If you hit it on an existing group: add the bot's number again from Signal. +Verified on 2026-08-29 against a group that already held an unacceptable +invite — re-adding turned `member: false` into `member: true, members: 2`, +and the bot answered `!cat` in that same group. (Signal keeps the dead +invite listed in `pending_invites`; `member` is the field that matters, +which is what the startup gate reads.) + +### It runs as a service, not in a terminal + +`register_bot.sh` finishes by installing a **user** systemd unit, so the bot +outlives the terminal that started it: + +```sh +./scripts/install_service.sh # install, start, verify +./scripts/install_service.sh --verify-only # re-check without reinstalling +systemctl --user status signal-bot.service +journalctl --user -u signal-bot.service -f +``` + +`Restart=always` brings it back from a crash; lingering (checked by the +installer, because without it the promise is false) starts it at boot with +nobody logged in. `StartLimitBurst=5` stops a config error — a missing +`GROUP_ID`, a taken port — from hiding inside a permanent restart loop. + +The health probe binds `127.0.0.1:8137` (`BIND_PORT`). Not 8000: that port +attracts unrelated dev servers, and 8137 sits below the ephemeral range so +the kernel cannot hand it out. If it is taken anyway the bot names the +offender and exits non-zero, because uvicorn logs the collision and exits +**0**, which every supervisor reads as success: + +``` +cannot bind 127.0.0.1:8000 -- held by pid 82385: /usr/bin/python -m ... +``` ### A linked device on your own account diff --git a/fake_signal/routes_control.py b/fake_signal/routes_control.py index 47ffa74..7dc5c54 100644 --- a/fake_signal/routes_control.py +++ b/fake_signal/routes_control.py @@ -30,13 +30,21 @@ def build_control_router() -> APIRouter: return {"delivered": delivered} @router.post("/groups") - async def set_groups(request: Request, groups: list[dict[str, str]]) -> dict[str, int]: + async def set_groups(request: Request, groups: list[dict[str, Any]]) -> dict[str, int]: """Seed the group list, so the group picker can be rehearsed offline.""" state = _state(request) state.groups.clear() state.groups.extend(groups) return {"groups": len(state.groups)} + @router.post("/accounts") + async def set_accounts(request: Request, accounts: list[str]) -> dict[str, int]: + """Seed the registered accounts, so /v1/groups answers at all.""" + state = _state(request) + state.accounts.clear() + state.accounts.extend(accounts) + return {"accounts": len(state.accounts)} + @router.get("/sent") async def captured(request: Request) -> list[dict[str, Any]]: return list(_state(request).sent) diff --git a/fake_signal/state.py b/fake_signal/state.py index 93f3ee0..7724a43 100644 --- a/fake_signal/state.py +++ b/fake_signal/state.py @@ -24,13 +24,13 @@ class State: sockets: list[WebSocket] = field(default_factory=list) sent: list[dict[str, Any]] = field(default_factory=list) attachments: list[str] = field(default_factory=list) - mode: str = "json-rpc-native" + mode: str = "json-rpc" base_url: str = "http://127.0.0.1:8099" # Registration and group state, so the registration runbook can be # rehearsed offline instead of first executing against real Signal. accounts: list[str] = field(default_factory=list) pending: dict[str, str] = field(default_factory=dict) - groups: list[dict[str, str]] = field(default_factory=list) + groups: list[dict[str, Any]] = field(default_factory=list) def reset(self) -> None: """Forget captured sends and attachments.""" diff --git a/scripts/e2e.sh b/scripts/e2e.sh index 9a9b5e7..860be61 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -19,7 +19,13 @@ readonly FAKE_PORT="${FAKE_PORT:-8099}" readonly BOT_PORT="${BOT_PORT:-8098}" readonly FAKE_URL="http://127.0.0.1:${FAKE_PORT}" -readonly WATCHED_GROUP="group.e2e.watched==" +# Both spellings of one group, exactly as the real API reports them: +# /v1/groups returns the `group.` form (what .env holds), while an +# incoming envelope carries the id that base64 decodes to. Modelling only +# one is why the offline suite once passed against a bot that ignored every +# message in its own group. +readonly WATCHED_GROUP="group.WlRKbExYZGhkR05vWldRdFozSnZkWEF0YVdRdE16SXRZbmwwWlhNaElTRT0=" +readonly WATCHED_GROUP_INTERNAL="ZTJlLXdhdGNoZWQtZ3JvdXAtaWQtMzItYnl0ZXMhISE=" readonly REPLY_GROUP="group.e2e.reply==" readonly OTHER_GROUP="group.e2e.other==" @@ -87,6 +93,14 @@ main() { FAKE_PID=$! wait_for_url "${FAKE_URL}/v1/about" "fake signal server" + echo "== seeding the fake with a registered, joined account" + curl -sf -X POST "${FAKE_URL}/_control/accounts" \ + -H 'Content-Type: application/json' \ + -d '["+15550000000"]' >/dev/null + curl -sf -X POST "${FAKE_URL}/_control/groups" \ + -H 'Content-Type: application/json' \ + -d "[{\"id\":\"${WATCHED_GROUP}\",\"internal_id\":\"${WATCHED_GROUP_INTERNAL}\",\"name\":\"e2e\",\"member\":true}]" >/dev/null + echo "== booting the bot on :${BOT_PORT}, pointed at the fake" env -u CAT_API \ PHONE_NUMBER="+15550000000" \ @@ -106,7 +120,7 @@ main() { "$PYTHON" -m fake_signal.cli \ "$REPO_ROOT"/fake_signal/scenarios/*.yaml \ --base-url "$FAKE_URL" \ - --group-id "$WATCHED_GROUP" \ + --group-id "$WATCHED_GROUP_INTERNAL" \ --group-id-send "$REPLY_GROUP" \ --other-group "$OTHER_GROUP" } diff --git a/scripts/install_service.sh b/scripts/install_service.sh new file mode 100755 index 0000000..75661b4 --- /dev/null +++ b/scripts/install_service.sh @@ -0,0 +1,123 @@ +#!/bin/bash + +# ============================================================================ +# Install, start and verify the signal-bot user service. +# +# A *user* unit, not a system one: the bot runs as whoever registered the +# Signal account and reads that user's .env. Lingering is what makes it start +# at boot with nobody logged in -- without it, "survives reboots" is a lie the +# unit file cannot tell you about, so this script checks and fails loudly. +# ============================================================================ + +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +readonly REPO_ROOT +SCRIPT_NAME="$(basename "$0")" +readonly SCRIPT_NAME +readonly UNIT_NAME="signal-bot.service" +readonly UNIT_DIR="${XDG_CONFIG_HOME:-$HOME/.config}/systemd/user" +readonly TEMPLATE="${REPO_ROOT}/scripts/${UNIT_NAME}" +readonly DEFAULT_PORT=8137 + +VERIFY_ONLY="false" + +# shellcheck source=scripts/lib_signal.sh +source "${REPO_ROOT}/scripts/lib_signal.sh" + +usage() { + echo "Usage: $SCRIPT_NAME [--verify-only]" + echo " --verify-only Check the running service without reinstalling it." + exit 0 +} + +die() { + echo "Error: $1" >&2 + exit 1 +} + +# The port the bot will actually bind: .env wins, code default otherwise. +bot_port() { + local port="" + if [[ -f "${REPO_ROOT}/.env" ]]; then + port="$(sed -n 's/^BIND_PORT=[[:space:]]*//p' "${REPO_ROOT}/.env" | tail -1)" + fi + printf '%s' "${port:-$DEFAULT_PORT}" +} + +validate_requirements() { + command -v systemctl >/dev/null || die "systemctl not found; this needs systemd" + [[ -f "${REPO_ROOT}/.env" ]] || + die "no ${REPO_ROOT}/.env -- run scripts/register_bot.sh first" + [[ -x "${REPO_ROOT}/run.sh" ]] || die "no executable ${REPO_ROOT}/run.sh" + [[ -f "$TEMPLATE" ]] || die "no unit template at $TEMPLATE" +} + +# Without linger, a user unit dies at logout and never starts at boot. +require_linger() { + if [[ "$(loginctl show-user "$USER" --property=Linger --value)" == "yes" ]]; then + echo "== linger enabled: the bot starts at boot with nobody logged in" + return 0 + fi + echo "== enabling linger for $USER" + if ! sudo -n loginctl enable-linger "$USER" 2>/dev/null; then + echo "Error: linger is off and sudo needs a password here." >&2 + echo " Without it the bot will NOT come back after a reboot." >&2 + echo " Run this, then re-run with --verify-only:" >&2 + echo >&2 + echo " sudo loginctl enable-linger $USER" >&2 + return 1 + fi +} + +install_unit() { + mkdir -p "$UNIT_DIR" + sed "s|@REPO_ROOT@|${REPO_ROOT}|g" "$TEMPLATE" > "${UNIT_DIR}/${UNIT_NAME}" + systemctl --user daemon-reload + systemctl --user enable "$UNIT_NAME" >/dev/null + systemctl --user restart "$UNIT_NAME" + echo "== installed and started ${UNIT_DIR}/${UNIT_NAME}" +} + +# Report what actually happened, not that the commands returned 0. +verify() { + local port="$1" + if ! systemctl --user is-active --quiet "$UNIT_NAME"; then + echo "Error: ${UNIT_NAME} is not active." >&2 + systemctl --user --no-pager --lines=20 status "$UNIT_NAME" >&2 || true + return 1 + fi + signal_wait_for_url "http://127.0.0.1:${port}/healthz" "signal-bot" 60 + echo "== active, and answering on http://127.0.0.1:${port}/healthz" + echo "== enabled at boot: $(systemctl --user is-enabled "$UNIT_NAME")" +} + +main() { + validate_requirements + local port + port="$(bot_port)" + + if [[ "$VERIFY_ONLY" != "true" ]]; then + require_linger + install_unit + fi + + verify "$port" + echo "== logs: journalctl --user -u ${UNIT_NAME} -f" +} + +while [[ $# -gt 0 ]]; do + case $1 in + --verify-only) + VERIFY_ONLY="true" + shift + ;; + -h | --help) usage ;; + *) + echo "Unknown option: $1" >&2 + exit 1 + ;; + esac +done + +main "$@" diff --git a/scripts/lib_signal.sh b/scripts/lib_signal.sh index cea90fd..e1d7fd8 100644 --- a/scripts/lib_signal.sh +++ b/scripts/lib_signal.sh @@ -27,13 +27,18 @@ signal_wait_for_url() { done } -# Refuse to continue unless the container exposes a receive WebSocket. +# Refuse to continue unless the container is in exactly the right mode. +# +# Not a *json-rpc* glob: 'json-rpc-native' also matches one, exposes the +# same WebSocket, and is a GraalVM build with no AWT -- so every attachment +# send fails with "Can't load library: awt" while text replies work, which +# looks like a bug in the bot. Accepting it is how that returns. signal_verify_mode() { local mode mode="$(curl -sf "${API}/v1/about" | jq -r '.mode // "unknown"')" - if [[ "$mode" != *json-rpc* ]]; then - echo "Error: container mode is '$mode'." >&2 - echo " /v1/receive is only a WebSocket in a json-rpc mode." >&2 + if [[ "$mode" != "json-rpc" ]]; then + echo "Error: container mode is '$mode', not 'json-rpc'." >&2 + echo " Recreate the container with -e MODE=json-rpc." >&2 return 1 fi echo "== container mode: $mode" @@ -72,16 +77,76 @@ signal_choose_group() { printf '%s' "$id" } +# Give the account a profile before anyone adds it to a group. +# +# Signal only adds an account to a group outright when the inviter already +# holds its profile key; with no profile there is no key to hold, so the +# add silently degrades into a pending *invitation* -- and signal-cli +# cannot accept one addressed to a phone number ("Cannot find service ID +# for self to accept invite"). That dead end cost a whole session on +# 2026-08-29: the bot looked healthy and received nothing, for hours. +signal_set_profile() { + local number="$1" name="$2" status + status="$(curl -sS -o /dev/null -w '%{http_code}' -X PUT \ + "${API}/v1/profiles/${number}" \ + -H 'Content-Type: application/json' \ + -d "{\"name\":\"${name}\"}")" + if [[ "$status" != "2"* ]]; then + echo "Error: could not set the bot's profile (HTTP $status)." >&2 + echo " Without a profile, adding the bot to a group creates an" >&2 + echo " invitation it can never accept." >&2 + return 1 + fi + echo "== profile set to '${name}' (required before joining a group)" +} + +# Refuse to finish on a group the bot was only *invited* to. +signal_require_membership() { + local number="$1" group="$2" detail + detail="$(curl -sf "${API}/v1/groups/${number}/${group}")" + if [[ "$(printf '%s' "$detail" | jq -r '.member')" == "true" ]]; then + echo "== the bot is a full member of $(printf '%s' "$detail" | jq -r '.name')" + return 0 + fi + echo "Error: the bot is only INVITED to this group, not a member." >&2 + echo " It would receive nothing. signal-cli cannot accept an" >&2 + echo " invite addressed to a phone number." >&2 + echo " Fix: in Signal, remove the pending invite for ${number}," >&2 + echo " then add it again -- now that it has a profile, Signal" >&2 + echo " adds it as a real member instead of inviting it." >&2 + return 1 +} + +# Print the first free TCP port at or after $1. +# +# The bot's default is 8137, but writing whatever is actually free means a +# machine that already runs something there gets a working .env instead of +# a bot that refuses to start. +signal_free_port() { + local port="$1" limit=$(($1 + 50)) + while [[ $port -lt $limit ]]; do + if ! ss -ltn "sport = :${port}" 2>/dev/null | grep -q LISTEN; then + printf '%s' "$port" + return 0 + fi + port=$((port + 1)) + done + echo "Error: no free port between $1 and ${limit}" >&2 + return 1 +} + # Write .env for the bot. GROUP_ID == GROUP_ID_SEND so replies land in the # same group that triggered them. signal_write_env() { - local repo_root="$1" number="$2" group="$3" + local repo_root="$1" number="$2" group="$3" port + port="$(signal_free_port "${BOT_PORT_BASE:-8137}")" cat > "${repo_root}/.env" </dev/null 2>&1 || true mkdir -p "$DATA_DIR" - echo "== starting $image on :${PORT} (MODE=json-rpc-native)" + # MODE=json-rpc, not json-rpc-native. Both expose the receive + # WebSocket, but the -native mode runs a GraalVM native-image build + # with no AWT, so every attachment send fails with + # "Can't load library: awt (UnsatisfiedLinkError)" -- meaning !cat + # and !dog return nothing while text replies work. Verified live on + # 2026-08-29: identical request, HTTP 400 native vs 201 on the JVM. + echo "== starting $image on :${PORT} (MODE=json-rpc)" docker run -d --name "$CONTAINER" --restart=always -p "${PORT}:8080" \ -v "${DATA_DIR}:/home/.local/share/signal-cli" \ - -e 'MODE=json-rpc-native' "$image" >/dev/null + -e 'MODE=json-rpc' "$image" >/dev/null if ! signal_wait_for_url "${API}/v1/about" "signal-cli-rest-api"; then docker logs --tail 30 "$CONTAINER" >&2 diff --git a/scripts/register_bot.sh b/scripts/register_bot.sh index 5f26fff..3d351dc 100755 --- a/scripts/register_bot.sh +++ b/scripts/register_bot.sh @@ -34,6 +34,7 @@ readonly CAPTCHA_URL="https://signalcaptchas.org/registration/generate.html" # carries "Update to new registration/provisioning API". The stable # container tag lags, so this is checked rather than hoped for. readonly MIN_SIGNAL_CLI="0.14.7" +readonly BOT_NAME="${BOT_NAME:-signal-bot}" readonly CI_IMAGE="bbernhard/signal-cli-rest-api:1787779383-ci" NUMBER="" @@ -66,16 +67,28 @@ validate_requirements() { } start_container() { + # Only reuse a container that is in the mode we need. A leftover + # json-rpc-native container answers /v1/about perfectly well and then + # fails every image send, so reusing it silently undoes the fix below. if curl -sf "${API}/v1/about" >/dev/null 2>&1; then - echo "== reusing the container already listening on :${PORT}" - return 0 + if signal_verify_mode >/dev/null 2>&1; then + echo "== reusing the container already listening on :${PORT}" + return 0 + fi + echo "== replacing the container on :${PORT}: wrong mode for attachments" fi mkdir -p "$DATA_DIR" docker rm -f "$CONTAINER" >/dev/null 2>&1 || true - echo "== starting $IMAGE on :${PORT} (MODE=json-rpc-native)" + # MODE=json-rpc, not json-rpc-native. Both expose the receive + # WebSocket, but the -native mode runs a GraalVM native-image build + # with no AWT, so every attachment send fails with + # "Can't load library: awt (UnsatisfiedLinkError)" -- meaning !cat + # and !dog return nothing while text replies work. Verified live on + # 2026-08-29: identical request, HTTP 400 native vs 201 on the JVM. + echo "== starting $IMAGE on :${PORT} (MODE=json-rpc)" docker run -d --name "$CONTAINER" --restart=always -p "${PORT}:8080" \ -v "${DATA_DIR}:/home/.local/share/signal-cli" \ - -e 'MODE=json-rpc-native' "$IMAGE" >/dev/null + -e 'MODE=json-rpc' "$IMAGE" >/dev/null signal_wait_for_url "${API}/v1/about" "signal-cli-rest-api" } @@ -180,6 +193,8 @@ main() { confirm_registered fi + signal_set_profile "$NUMBER" "$BOT_NAME" + echo echo "== NEXT: from your personal Signal, add ${NUMBER} to the private group," echo " then press Enter. The bot must be a member before it can post." @@ -187,10 +202,16 @@ main() { local group group="$(signal_choose_group "$NUMBER")" + signal_require_membership "$NUMBER" "$group" signal_write_env "$REPO_ROOT" "$NUMBER" "$group" - echo "== starting the bot. Send '!cat' in that group." - exec "$REPO_ROOT/run.sh" + # Install the service rather than running in the foreground: a bot that + # only lives as long as this terminal is a demo, not a bot. + "$REPO_ROOT/scripts/install_service.sh" + echo + echo "== running. Send '!cat' in that group." + echo "== following the log; Ctrl-C stops watching, not the bot." + exec journalctl --user -u signal-bot.service -f } while [[ $# -gt 0 ]]; do diff --git a/scripts/signal-bot.service b/scripts/signal-bot.service new file mode 100644 index 0000000..923dd3d --- /dev/null +++ b/scripts/signal-bot.service @@ -0,0 +1,38 @@ +# Installed to ~/.config/systemd/user/ by scripts/install_service.sh, which +# substitutes @REPO_ROOT@. Edit this template, never the installed copy. + +[Unit] +Description=Signal group bot +Documentation=file://@REPO_ROOT@/README.md +# The bot's only outbound dependency is signal-cli-rest-api in Docker. That +# container runs --restart=always, so it comes back by itself, but Docker has +# to exist before the bot's first connection attempt. +After=network-online.target docker.service +Wants=network-online.target +# A config error -- missing GROUP_ID, a port that is taken -- exits non-zero +# instantly, and would otherwise spin forever at RestartSec. Five failures in +# a minute stops the unit in `failed` state, which is visible in `systemctl +# --user status`, instead of hiding as a permanent restart loop. +StartLimitIntervalSec=60 +StartLimitBurst=5 + +[Service] +Type=exec +WorkingDirectory=@REPO_ROOT@ +# run.sh execs the interpreter, so this PID is the bot itself. +ExecStart=@REPO_ROOT@/run.sh +# `always`, not `on-failure`: this process has no correct reason to exit 0. +Restart=always +RestartSec=5 +# The bot writes only its SQLite tally database, inside the repo. +NoNewPrivileges=true +PrivateTmp=true +ProtectSystem=strict +ProtectHome=read-only +ReadWritePaths=@REPO_ROOT@ +ProtectKernelTunables=true +ProtectControlGroups=true +RestrictSUIDSGID=true + +[Install] +WantedBy=default.target diff --git a/signal_bot/__main__.py b/signal_bot/__main__.py index f75217b..ca0b222 100644 --- a/signal_bot/__main__.py +++ b/signal_bot/__main__.py @@ -3,27 +3,37 @@ The old readme told people to run `python main.py`, which had no __main__ block and no uvicorn.run() call, so it silently did nothing at all. + +The bind address comes from Config rather than os.getenv() here: this module +runs *before* uvicorn imports the app, and therefore before anything has +called load_dotenv(), so reading the environment directly would ignore +BIND_HOST/BIND_PORT whenever they live in .env -- which is where they live. """ from __future__ import annotations -import os +import logging import uvicorn -# Loopback by default: the only inbound surface is /healthz, and the link to -# signal-cli-rest-api is outbound. Override with BIND_HOST when containerised. -DEFAULT_HOST = "127.0.0.1" -DEFAULT_PORT = 8000 +from signal_bot.config import load_config +from signal_bot.portcheck import PortInUseError, assert_port_free def main() -> None: - """Run the ASGI app under uvicorn.""" - uvicorn.run( - "signal_bot.app:app", - host=os.getenv("BIND_HOST", DEFAULT_HOST), - port=int(os.getenv("BIND_PORT", str(DEFAULT_PORT))), + """Run the ASGI app under uvicorn, refusing to start on a taken port.""" + config = load_config() + # Before anything else: uvicorn only configures its own loggers, so + # without this the bot's own log lines go to logging.lastResort and + # anything below WARNING is thrown away. + logging.basicConfig( + level=config.log_level, format="%(asctime)s %(levelname)s %(name)s: %(message)s" ) + try: + assert_port_free(config.bind_host, config.bind_port) + except PortInUseError as exc: + raise SystemExit(str(exc)) from exc + uvicorn.run("signal_bot.app:app", host=config.bind_host, port=config.bind_port) if __name__ == "__main__": diff --git a/signal_bot/app.py b/signal_bot/app.py index 71dd98c..7f94f47 100644 --- a/signal_bot/app.py +++ b/signal_bot/app.py @@ -119,6 +119,7 @@ async def create_runtime(config: Config, client: httpx.AsyncClient) -> Runtime: store = Store(config.database_path) transport = RestTransport(config, client) await transport.verify_mode() + await transport.verify_membership() return Runtime(bot=build_bot(config, client, store, transport), client=client, store=store) @@ -134,6 +135,11 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: client = httpx.AsyncClient(timeout=HTTP_TIMEOUT_SECONDS) runtime = await create_runtime(config, client) runtime.start() + logger.info( + "listening as %s, watching group ids %r", + config.phone_number, + sorted(config.watched_group_ids), + ) app.state.runtime = runtime try: yield diff --git a/signal_bot/config.py b/signal_bot/config.py index 6f2a415..ec83a11 100644 --- a/signal_bot/config.py +++ b/signal_bot/config.py @@ -3,6 +3,9 @@ from __future__ import annotations +import base64 +import binascii +import logging import os from dataclasses import dataclass from typing import Final @@ -17,12 +20,49 @@ DEFAULT_REPORT_MINUTE: Final = 37 DEFAULT_COOLDOWN_SECONDS: Final = 10 DEFAULT_CAT_API_URL: Final = "https://api.thecatapi.com/v1/images/search" DEFAULT_DOG_API_URL: Final = "https://dog.ceo/api/breeds/image/random" +DEFAULT_BIND_HOST: Final = "127.0.0.1" +# Not 8000: that port is a magnet for unrelated dev servers and was already +# taken here by a long-running enforcer. 8137 sits below the ephemeral range +# (/proc/sys/net/ipv4/ip_local_port_range, 32768-60999 on Arch), so the kernel +# will never hand it out to an outbound connection and steal it back. +DEFAULT_BIND_PORT: Final = 8137 MAX_HOUR: Final = 23 MAX_MINUTE: Final = 59 +MAX_PORT: Final = 65535 +GROUP_ID_PREFIX: Final = "group." +DEFAULT_LOG_LEVEL: Final = "INFO" _WS_SCHEMES: Final = {"http": "ws", "https": "wss"} +def group_id_aliases(group_id: str) -> frozenset[str]: + """Every spelling of one group id that can arrive in an envelope. + + The REST API reports group ids two different ways and never says so: + ``/v1/groups`` returns ``group.`` (what the setup script writes + into .env), while an incoming envelope's ``groupInfo.groupId`` carries + the *internal* id -- which is exactly what that base64 decodes to. The + two never compare equal, so a bot configured from the API's own output + silently ignores every message in its own group. Verified live on + 2026-08-29: ``group.Ni9nRXdS...`` decodes to ``6/gEwRkqs/...``, the id + the same endpoint reports as ``internal_id``. + + Args: + group_id: The configured group id, in either spelling. + + Returns: + The configured id plus its decoded internal form, when it has one. + """ + aliases = {group_id} + if group_id.startswith(GROUP_ID_PREFIX): + try: + decoded = base64.b64decode(group_id.removeprefix(GROUP_ID_PREFIX), validate=True) + except binascii.Error, ValueError: + return frozenset(aliases) + aliases.add(decoded.decode("utf-8", errors="replace")) + return frozenset(aliases) + + class ConfigError(RuntimeError): """Raised when the environment does not describe a runnable bot.""" @@ -43,6 +83,21 @@ class Config: # without the bot knowing it is not talking to the real APIs. cat_api_url: str = DEFAULT_CAT_API_URL dog_api_url: str = DEFAULT_DOG_API_URL + # Where the /healthz readiness probe binds. Read here rather than in + # __main__ so that .env governs it: __main__ runs before anything has + # loaded a .env file, so an os.getenv() there would silently ignore it. + bind_host: str = DEFAULT_BIND_HOST + bind_port: int = DEFAULT_BIND_PORT + # Set LOG_LEVEL=DEBUG to see why a frame was ignored. Without a knob, + # the one log line that explains a silently-dropped message is + # unreachable in production, which is how a scoping mismatch turns + # into a bot that looks healthy and answers nothing. + log_level: str = DEFAULT_LOG_LEVEL + + @property + def watched_group_ids(self) -> frozenset[str]: + """Group ids that count as "the watched group" in an envelope.""" + return group_id_aliases(self.group_id) @property def receive_url(self) -> str: @@ -68,6 +123,11 @@ class Config: """REST endpoint reporting the container build and its mode.""" return f"{self.api_base_url}/v1/about" + @property + def groups_url(self) -> str: + """REST endpoint listing the groups this account knows about.""" + return f"{self.api_base_url}/v1/groups/{self.phone_number}" + def _require(env: dict[str, str], key: str) -> str: """Return a non-empty environment value or raise ConfigError.""" @@ -119,6 +179,16 @@ def load_config(env: dict[str, str] | None = None, *, use_dotenv: bool = True) - msg = f"REPORT_HOUR/REPORT_MINUTE out of range: {hour:02d}:{minute:02d}" raise ConfigError(msg) + log_level = env.get("LOG_LEVEL", "").strip().upper() or DEFAULT_LOG_LEVEL + if log_level not in logging.getLevelNamesMapping(): + msg = f"LOG_LEVEL must be a logging level name, got {log_level!r}" + raise ConfigError(msg) + + bind_port = _positive_int(env, "BIND_PORT", DEFAULT_BIND_PORT) + if not 1 <= bind_port <= MAX_PORT: + msg = f"BIND_PORT must be between 1 and {MAX_PORT}, got {bind_port}" + raise ConfigError(msg) + return Config( phone_number=_require(env, "PHONE_NUMBER"), group_id=_require(env, "GROUP_ID"), @@ -130,4 +200,7 @@ def load_config(env: dict[str, str] | None = None, *, use_dotenv: bool = True) - cooldown_seconds=_positive_int(env, "COOLDOWN_SECONDS", DEFAULT_COOLDOWN_SECONDS), cat_api_url=env.get("CAT_API_URL", "").strip() or DEFAULT_CAT_API_URL, dog_api_url=env.get("DOG_API_URL", "").strip() or DEFAULT_DOG_API_URL, + bind_host=env.get("BIND_HOST", "").strip() or DEFAULT_BIND_HOST, + bind_port=bind_port, + log_level=log_level, ) diff --git a/signal_bot/counting.py b/signal_bot/counting.py index 3eed7b5..122ced8 100644 --- a/signal_bot/counting.py +++ b/signal_bot/counting.py @@ -16,12 +16,14 @@ def is_watched_group(message: IncomingMessage, config: Config) -> bool: A linked Signal device receives every direct conversation and every group on the account. This predicate is the entire boundary between "a bot in one group" and "a bot reading all of your chats", so it is deliberately - written as an explicit group-id equality rather than a falsy check: a - direct message has no group id at all and must never reach a handler. + written as an explicit group-id membership test rather than a falsy + check: a direct message has no group id at all and must never reach a + handler. The set, rather than a single ==, is because the REST API + spells the same group two ways -- see Config.watched_group_ids. """ if message.group_id is None: return False - return message.group_id == config.group_id + return message.group_id in config.watched_group_ids def should_count(message: IncomingMessage) -> bool: diff --git a/signal_bot/dispatcher.py b/signal_bot/dispatcher.py index f32e50b..a3eed95 100644 --- a/signal_bot/dispatcher.py +++ b/signal_bot/dispatcher.py @@ -42,7 +42,11 @@ class Dispatcher: # The scoping gate. Direct messages and every other group the account # belongs to stop here, before counting and before any handler runs. if not is_watched_group(message, self.config): - logger.debug("ignoring message outside the watched group") + logger.debug( + "ignoring message from group %r; watching %r", + message.group_id, + sorted(self.config.watched_group_ids), + ) return if should_count(message): @@ -52,6 +56,7 @@ class Dispatcher: if command is None: return + logger.info("running command %s for %s", command.name, message.source_name) now = self.clock() ctx = CommandContext( message=message, diff --git a/signal_bot/portcheck.py b/signal_bot/portcheck.py new file mode 100644 index 0000000..faef743 --- /dev/null +++ b/signal_bot/portcheck.py @@ -0,0 +1,119 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""Refuse to start on a taken port, and name whatever is already holding it. + +uvicorn logs "address already in use" and then shuts down *cleanly*: the +process exits 0, so a supervisor records a successful run and a human reads a +tidy "Application shutdown complete" as if nothing were wrong. That is a +fail-open, and it is exactly how this bot silently failed to start. + +Attribution reads /proc directly rather than forking `ss` or `lsof`: no +subprocess, no PATH assumptions, and the whole thing is testable against a +fake proc tree passed in as `proc_root`. +""" + +from __future__ import annotations + +import socket +from pathlib import Path +from typing import Final + +PROC_ROOT: Final = Path("/proc") +_TCP_TABLES: Final = ("net/tcp", "net/tcp6") +_LISTEN_STATE: Final = "0A" +_INODE_FIELD: Final = 9 + + +class PortInUseError(RuntimeError): + """Raised when the bot's bind address is already taken.""" + + +def _listening_inodes(port: int, proc_root: Path) -> set[str]: + """Return the socket inodes of every LISTEN socket on the given port.""" + inodes: set[str] = set() + for table in _TCP_TABLES: + try: + # Skip the header row; every remaining line is one socket. + lines = (proc_root / table).read_text().splitlines()[1:] + except OSError: + continue + for line in lines: + fields = line.split() + if len(fields) <= _INODE_FIELD or fields[3] != _LISTEN_STATE: + continue + _, _, hex_port = fields[1].rpartition(":") + try: + listening_on = int(hex_port, 16) + except ValueError: + continue + if listening_on == port: + inodes.add(fields[_INODE_FIELD]) + return inodes + + +def _command_of(pid_dir: Path) -> str: + """Return the full command line of a process, or a placeholder.""" + try: + raw = (pid_dir / "cmdline").read_bytes() + except OSError: + return "(command unavailable)" + argv = raw.decode(errors="replace").rstrip("\0").split("\0") + return " ".join(argv).strip() or "(command unavailable)" + + +def _owner_of(inodes: set[str], proc_root: Path) -> tuple[int, str] | None: + """Find the pid holding any of these socket inodes, with its command line.""" + targets = {f"socket:[{inode}]" for inode in inodes} + for pid_dir in sorted(proc_root.iterdir()): + if not pid_dir.name.isdigit(): + continue + try: + descriptors = sorted((pid_dir / "fd").iterdir()) + except OSError: + # Another user's process, or one that exited mid-scan. + continue + for descriptor in descriptors: + try: + link = str(descriptor.readlink()) + except OSError: + continue + if link in targets: + return int(pid_dir.name), _command_of(pid_dir) + return None + + +def describe_listener(port: int, proc_root: Path = PROC_ROOT) -> str: + """Describe whoever is listening on a port, or "" if nobody identifiable is.""" + inodes = _listening_inodes(port, proc_root) + if not inodes: + return "" + owner = _owner_of(inodes, proc_root) + if owner is None: + return "held by a process belonging to another user" + pid, command = owner + return f"held by pid {pid}: {command}" + + +def assert_port_free(host: str, port: int, proc_root: Path = PROC_ROOT) -> None: + """Raise PortInUseError unless host:port can actually be bound. + + Args: + host: Address the server will bind. + port: Port the server will bind. + proc_root: Root of the proc filesystem. Replaced in tests. + + Raises: + PortInUseError: The address is already in use. + """ + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as probe: + probe.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + try: + probe.bind((host, port)) + except OSError as exc: + holder = describe_listener(port, proc_root) + detail = f" -- {holder}" if holder else "" + msg = ( + f"cannot bind {host}:{port}{detail}. " + f"Set BIND_PORT in .env to a free port, or stop that process. " + f"Refusing to start, because uvicorn would log this and exit 0." + ) + raise PortInUseError(msg) from exc diff --git a/signal_bot/transport_rest.py b/signal_bot/transport_rest.py index 57d2ff6..d13fbc5 100644 --- a/signal_bot/transport_rest.py +++ b/signal_bot/transport_rest.py @@ -25,6 +25,8 @@ logger = logging.getLogger(__name__) INITIAL_BACKOFF: Final = 1.0 MAX_BACKOFF: Final = 30.0 +# Exactly this mode. See verify_mode for why "json-rpc-native" is not enough. +REQUIRED_MODE: Final = "json-rpc" BACKOFF_FACTOR: Final = 2.0 _OK_STATUSES: Final = frozenset({200, 201, 204}) @@ -73,15 +75,58 @@ class RestTransport: response.raise_for_status() body = response.json() mode = str(body.get("mode", "")) if isinstance(body, dict) else "" - if "json-rpc" not in mode: + if mode != REQUIRED_MODE: msg = ( - f"signal-cli-rest-api is running in mode {mode!r}; the receive " - "endpoint is only a WebSocket in a json-rpc mode. Start the " - "container with MODE=json-rpc-native." + f"signal-cli-rest-api is running in mode {mode!r}, not " + f"{REQUIRED_MODE!r}. 'normal' exposes no receive WebSocket at " + "all, and 'json-rpc-native' is a GraalVM build with no AWT, so " + 'every attachment send fails with "Can\'t load library: awt" ' + "while text replies keep working. Recreate the container with " + f"MODE={REQUIRED_MODE}." ) raise TransportError(msg) return mode + async def verify_membership(self) -> None: + """Raise unless the account is a full member of the watched group. + + A pending invitation is indistinguishable from membership from the + outside: /v1/groups lists the group, the websocket connects, + /healthz answers, and not one message ever arrives. Signal only + adds an account to a group outright when the inviter already holds + its profile key; otherwise it creates an invite, and signal-cli + cannot accept one addressed to a phone number ("Cannot find + service ID for self to accept invite"). Observed live on + 2026-08-29, where the bot looked perfectly healthy for hours while + being a non-member. Startup is the only honest place to catch it. + + Raises: + TransportError: The account is not a full member of the group. + """ + response = await self._client.get(self._config.groups_url) + response.raise_for_status() + wanted = self._config.watched_group_ids + for group in response.json(): + if not isinstance(group, dict): + continue + if not {group.get("id"), group.get("internal_id")} & wanted: + continue + if group.get("member"): + return + msg = ( + f"the account is invited to {group.get('name')!r} but has not " + "joined it, so it will receive nothing. signal-cli cannot " + "accept an invite addressed to a phone number: have an admin " + "revoke the invite and add the bot again now that it has a " + "profile, or have the bot create the group itself." + ) + raise TransportError(msg) + msg = ( + f"the account is in no group matching {sorted(wanted)}. Check GROUP_ID " + "in .env against: curl -s $API_BASE_URL/v1/groups/$PHONE_NUMBER" + ) + raise TransportError(msg) + async def receive(self) -> AsyncIterator[str]: """Yield raw envelope frames, reconnecting with backoff when dropped.""" backoff = INITIAL_BACKOFF diff --git a/tests/test_app.py b/tests/test_app.py index 2721308..282ef5a 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -38,6 +38,27 @@ def _client(handler: Handler) -> httpx.AsyncClient: return _REAL_ASYNC_CLIENT(transport=httpx.MockTransport(handler)) +def _signal(group_id: str, *, mode: str = "json-rpc", member: bool = True) -> Handler: + """Answer both startup probes: the mode check and the membership check.""" + + def handler(request: httpx.Request) -> httpx.Response: + if "/v1/groups" in request.url.path: + return httpx.Response( + 200, + json=[ + { + "id": group_id, + "internal_id": group_id, + "name": "watched", + "member": member, + } + ], + ) + return httpx.Response(200, json={"mode": mode}) + + return handler + + def _bot(config: Config, store: Store, transport: FakeTransport) -> Bot: return build_bot(config, _client(lambda r: httpx.Response(200)), store, transport) @@ -81,9 +102,7 @@ async def test_listen_dispatches_every_frame( async def test_create_runtime_verifies_the_container_mode(config: Config) -> None: - runtime = await create_runtime( - config, _client(lambda r: httpx.Response(200, json={"mode": "json-rpc-native"})) - ) + runtime = await create_runtime(config, _client(_signal(config.group_id))) assert isinstance(runtime, Runtime) await runtime.stop() @@ -113,7 +132,7 @@ def test_healthz_serves_and_startup_completes(monkeypatch: pytest.MonkeyPatch) - monkeypatch.setenv(key, value) def fake_client(**kwargs: object) -> httpx.AsyncClient: - return _client(lambda r: httpx.Response(200, json={"mode": "json-rpc-native"})) + return _client(_signal(BASE_ENV["GROUP_ID"])) monkeypatch.setattr(app_module.httpx, "AsyncClient", fake_client) with TestClient(app_module.app) as client: @@ -128,7 +147,15 @@ async def test_lifespan_is_an_async_context_manager( monkeypatch.setattr( app_module.httpx, "AsyncClient", - lambda **kwargs: _client(lambda r: httpx.Response(200, json={"mode": "json-rpc"})), + lambda **kwargs: _client(_signal(BASE_ENV["GROUP_ID"])), ) async with lifespan(app_module.app): assert app_module.app.state.runtime is not None + + +async def test_create_runtime_refuses_a_group_it_only_got_invited_to( + config: Config, +) -> None: + """A pending invite looks healthy from outside; startup must not.""" + with pytest.raises(TransportError, match="has not joined"): + await create_runtime(config, _client(_signal(config.group_id, member=False))) diff --git a/tests/test_config.py b/tests/test_config.py index 862cdd8..984eafd 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -5,7 +5,7 @@ from __future__ import annotations import pytest -from signal_bot.config import Config, ConfigError, load_config +from signal_bot.config import Config, ConfigError, group_id_aliases, load_config BASE_ENV = { "PHONE_NUMBER": "+15550000000", @@ -103,3 +103,71 @@ def test_https_base_url_yields_wss() -> None: api_base_url="https://signal.example", ) assert config.receive_url.startswith("wss://signal.example/") + + +def test_group_id_aliases_adds_the_internal_form() -> None: + """The API spells one group two ways; the bot must answer to both.""" + # Real value observed live: the base64 decodes to the internal_id the + # same endpoint reports, and to what envelopes actually carry. + configured = "group.Ni9nRXdSa3FzLzI1V1F4MkpPMkVlUlc4RUIwSXJBdXVPKzhEMGh1K09wST0=" + assert group_id_aliases(configured) == { + configured, + "6/gEwRkqs/25WQx2JO2EeRW8EB0IrAuuO+8D0hu+OpI=", + } + + +def test_group_id_aliases_leaves_a_bare_internal_id_alone() -> None: + assert group_id_aliases("6/gEwRkqs=") == {"6/gEwRkqs="} + + +def test_group_id_aliases_tolerates_undecodable_ids() -> None: + """A hand-edited GROUP_ID must not crash startup before the real check.""" + assert group_id_aliases("group.not!valid!base64") == {"group.not!valid!base64"} + + +def test_watched_group_ids_exposes_both_spellings() -> None: + config = load_config( + { + "PHONE_NUMBER": "+15550000000", + "GROUP_ID": "group.Ni9nRXdSa3FzLzI1V1F4MkpPMkVlUlc4RUIwSXJBdXVPKzhEMGh1K09wST0=", + "GROUP_ID_SEND": "reply==", + } + ) + assert "6/gEwRkqs/25WQx2JO2EeRW8EB0IrAuuO+8D0hu+OpI=" in config.watched_group_ids + + +def test_groups_url_is_scoped_to_the_account() -> None: + config = load_config( + { + "PHONE_NUMBER": "+15550000000", + "GROUP_ID": "g==", + "GROUP_ID_SEND": "g==", + "API_BASE_URL": "http://localhost:9922", + } + ) + assert config.groups_url == "http://localhost:9922/v1/groups/+15550000000" + + +def test_an_unknown_log_level_is_rejected() -> None: + with pytest.raises(ConfigError, match="LOG_LEVEL"): + load_config( + { + "PHONE_NUMBER": "+15550000000", + "GROUP_ID": "g==", + "GROUP_ID_SEND": "g==", + "LOG_LEVEL": "chatty", + } + ) + + +@pytest.mark.parametrize("port", ["0", "65536"]) +def test_a_port_outside_the_valid_range_is_rejected(port: str) -> None: + with pytest.raises(ConfigError, match="BIND_PORT"): + load_config( + { + "PHONE_NUMBER": "+15550000000", + "GROUP_ID": "g==", + "GROUP_ID_SEND": "g==", + "BIND_PORT": port, + } + ) diff --git a/tests/test_fake_account.py b/tests/test_fake_account.py index 64cbdcd..73af42c 100644 --- a/tests/test_fake_account.py +++ b/tests/test_fake_account.py @@ -98,3 +98,16 @@ def test_seeding_groups_replaces_the_previous_list(client: TestClient, state: St client.post("/_control/groups", json=[{"id": "old==", "name": "Old"}]) client.post("/_control/groups", json=[{"id": "new==", "name": "New"}]) assert [g["id"] for g in client.get(f"/v1/groups/{NUMBER}").json()] == ["new=="] + + +def test_control_seeds_registered_accounts(client: TestClient) -> None: + """/v1/groups answers 400 for an unknown account, so the fake needs this.""" + assert client.get("/v1/groups/+15550000000").status_code == 400 + assert client.post("/_control/accounts", json=["+15550000000"]).json() == {"accounts": 1} + assert client.get("/v1/groups/+15550000000").status_code == 200 + + +def test_control_replaces_accounts_rather_than_appending(client: TestClient) -> None: + client.post("/_control/accounts", json=["+15550000001"]) + client.post("/_control/accounts", json=["+15550000002"]) + assert client.get("/v1/accounts").json() == ["+15550000002"] diff --git a/tests/test_fake_server.py b/tests/test_fake_server.py index ff6f405..7ed4d15 100644 --- a/tests/test_fake_server.py +++ b/tests/test_fake_server.py @@ -22,7 +22,7 @@ def client(state: State) -> TestClient: def test_about_reports_a_websocket_mode(client: TestClient) -> None: body = client.get("/v1/about").json() - assert body["mode"] == "json-rpc-native" + assert body["mode"] == "json-rpc" def test_qrcodelink_returns_a_png(client: TestClient) -> None: diff --git a/tests/test_main.py b/tests/test_main.py index 7b5af20..13978d7 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -13,6 +13,28 @@ import pytest import uvicorn import signal_bot.__main__ as entrypoint +from signal_bot.portcheck import PortInUseError + +BASE_ENV = { + "PHONE_NUMBER": "+15550000000", + "GROUP_ID": "watched==", + "GROUP_ID_SEND": "reply==", +} + + +@pytest.fixture(autouse=True) +def _isolated_env(monkeypatch: pytest.MonkeyPatch) -> None: + """Keep the developer's own .env out of the entrypoint's decisions. + + load_dotenv() does not overwrite variables that are already set, but it + happily *adds* ones that are not -- so a real .env carrying BIND_PORT + would silently decide the outcome of the default-port test. + """ + monkeypatch.setattr("signal_bot.config.load_dotenv", lambda *a, **k: None) + for key, value in BASE_ENV.items(): + monkeypatch.setenv(key, value) + for key in ("BIND_HOST", "BIND_PORT"): + monkeypatch.delenv(key, raising=False) @pytest.fixture @@ -24,18 +46,15 @@ def captured(monkeypatch: pytest.MonkeyPatch) -> dict[str, object]: seen.update(kwargs) monkeypatch.setattr(entrypoint.uvicorn, "run", fake_run) + monkeypatch.setattr(entrypoint, "assert_port_free", lambda host, port: None) return seen -def test_runs_the_asgi_app_on_loopback_by_default( - captured: dict[str, object], monkeypatch: pytest.MonkeyPatch -) -> None: - monkeypatch.delenv("BIND_HOST", raising=False) - monkeypatch.delenv("BIND_PORT", raising=False) +def test_runs_the_asgi_app_on_loopback_by_default(captured: dict[str, object]) -> None: entrypoint.main() assert captured["target"] == "signal_bot.app:app" assert captured["host"] == "127.0.0.1" - assert captured["port"] == 8000 + assert captured["port"] == 8137 def test_bind_overrides_are_honoured( @@ -49,11 +68,25 @@ def test_bind_overrides_are_honoured( assert captured["port"] == 9001 +def test_a_taken_port_exits_non_zero_instead_of_starting( + captured: dict[str, object], monkeypatch: pytest.MonkeyPatch +) -> None: + """uvicorn would log the collision and exit 0; the bot must not.""" + + def refuse(host: str, port: int) -> None: + msg = "cannot bind 127.0.0.1:8137 -- held by pid 1: /sbin/init" + raise PortInUseError(msg) + + monkeypatch.setattr(entrypoint, "assert_port_free", refuse) + with pytest.raises(SystemExit, match="held by pid 1"): + entrypoint.main() + assert "target" not in captured + + def test_module_runs_as_a_script(monkeypatch: pytest.MonkeyPatch) -> None: """`python -m signal_bot` must actually start the server.""" seen: dict[str, object] = {} monkeypatch.setattr(uvicorn, "run", lambda target, **kw: seen.update(target=target)) - monkeypatch.delenv("BIND_HOST", raising=False) - monkeypatch.delenv("BIND_PORT", raising=False) + monkeypatch.setattr("signal_bot.portcheck.assert_port_free", lambda host, port: None) runpy.run_module("signal_bot", run_name="__main__", alter_sys=True) assert seen["target"] == "signal_bot.app:app" diff --git a/tests/test_portcheck.py b/tests/test_portcheck.py new file mode 100644 index 0000000..68781e3 --- /dev/null +++ b/tests/test_portcheck.py @@ -0,0 +1,145 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""Refusing to start on a taken port, and naming what holds it.""" + +from __future__ import annotations + +import socket +from typing import TYPE_CHECKING + +import pytest + +from signal_bot.portcheck import PortInUseError, assert_port_free, describe_listener + +if TYPE_CHECKING: + from pathlib import Path + +LISTEN = "0A" +ESTABLISHED = "01" + + +def _tcp_line(port: int, inode: str, state: str = LISTEN) -> str: + """One /proc/net/tcp row for a socket on the given port.""" + return ( + f" 0: 0100007F:{port:04X} 00000000:0000 {state} " + f"00000000:00000000 00:00000000 00000000 1000 0 {inode} 1" + ) + + +def _proc(tmp_path: Path, rows: list[str]) -> Path: + """Build a fake /proc containing just a net/tcp table.""" + net = tmp_path / "net" + net.mkdir(parents=True, exist_ok=True) + (net / "tcp").write_text(" sl local_address ...\n" + "\n".join(rows) + "\n") + return tmp_path + + +def _process( + root: Path, pid: str, inode: str, cmdline: bytes = b"/usr/bin/python\0-m\0bot\0" +) -> Path: + """Add a process to the fake /proc holding the given socket inode.""" + pid_dir = root / pid + (pid_dir / "fd").mkdir(parents=True) + (pid_dir / "fd" / "3").symlink_to(f"socket:[{inode}]") + (pid_dir / "cmdline").write_bytes(cmdline) + return pid_dir + + +def test_names_the_pid_and_command_holding_the_port(tmp_path: Path) -> None: + root = _proc(tmp_path, [_tcp_line(8137, "25863")]) + _process(root, "4242", "25863") + assert describe_listener(8137, root) == "held by pid 4242: /usr/bin/python -m bot" + + +def test_reports_nothing_when_no_socket_listens_on_the_port(tmp_path: Path) -> None: + root = _proc(tmp_path, [_tcp_line(9999, "25863")]) + assert describe_listener(8137, root) == "" + + +def test_ignores_sockets_that_are_not_listening(tmp_path: Path) -> None: + root = _proc(tmp_path, [_tcp_line(8137, "25863", state=ESTABLISHED)]) + assert describe_listener(8137, root) == "" + + +def test_survives_a_proc_with_no_tcp_tables_at_all(tmp_path: Path) -> None: + assert describe_listener(8137, tmp_path) == "" + + +def test_skips_short_and_unparseable_rows(tmp_path: Path) -> None: + root = _proc( + tmp_path, + [ + " 1: truncated row", + " 2: NOTHEX:NOTHEX 00000000:0000 0A a b c 1000 0 99 1", + _tcp_line(8137, "25863"), + ], + ) + _process(root, "4242", "25863") + assert "pid 4242" in describe_listener(8137, root) + + +def test_reports_another_user_when_the_owner_is_invisible(tmp_path: Path) -> None: + """Scanning /proc//fd fails for processes that are not ours.""" + root = _proc(tmp_path, [_tcp_line(8137, "25863")]) + (root / "4242").mkdir() # no fd/ directory at all + (root / "not-a-pid").mkdir() + assert describe_listener(8137, root) == "held by a process belonging to another user" + + +def test_skips_descriptors_that_are_not_symlinks(tmp_path: Path) -> None: + root = _proc(tmp_path, [_tcp_line(8137, "25863")]) + pid_dir = root / "4242" + (pid_dir / "fd").mkdir(parents=True) + (pid_dir / "fd" / "3").write_text("not a link") + assert describe_listener(8137, root) == "held by a process belonging to another user" + + +def test_falls_back_when_the_command_line_is_unreadable(tmp_path: Path) -> None: + root = _proc(tmp_path, [_tcp_line(8137, "25863")]) + pid_dir = root / "4242" + (pid_dir / "fd").mkdir(parents=True) + (pid_dir / "fd" / "3").symlink_to("socket:[25863]") + # No cmdline file: a process that exited between the two reads. + assert describe_listener(8137, root) == "held by pid 4242: (command unavailable)" + + +def test_falls_back_when_the_command_line_is_empty(tmp_path: Path) -> None: + root = _proc(tmp_path, [_tcp_line(8137, "25863")]) + _process(root, "4242", "25863", cmdline=b"") + assert describe_listener(8137, root) == "held by pid 4242: (command unavailable)" + + +def test_a_free_port_is_accepted(tmp_path: Path) -> None: + with socket.socket() as probe: + probe.bind(("127.0.0.1", 0)) + free = probe.getsockname()[1] + # Must simply not raise; a bare comparison here would assert nothing. + assert_port_free("127.0.0.1", free, tmp_path) + + +def test_a_taken_port_raises_and_names_the_holder(tmp_path: Path) -> None: + with socket.socket() as held: + held.bind(("127.0.0.1", 0)) + held.listen(1) + port = held.getsockname()[1] + root = _proc(tmp_path, [_tcp_line(port, "25863")]) + _process(root, "4242", "25863") + with pytest.raises(PortInUseError, match="held by pid 4242") as caught: + assert_port_free("127.0.0.1", port, root) + assert "Set BIND_PORT" in str(caught.value) + + +def test_a_taken_port_still_raises_when_the_holder_is_unknown(tmp_path: Path) -> None: + with socket.socket() as held: + held.bind(("127.0.0.1", 0)) + held.listen(1) + port = held.getsockname()[1] + with pytest.raises(PortInUseError, match=f"cannot bind 127.0.0.1:{port}"): + assert_port_free("127.0.0.1", port, tmp_path) + + +def test_scans_past_processes_holding_other_sockets(tmp_path: Path) -> None: + """The first process examined rarely owns the port; the scan must go on.""" + root = _proc(tmp_path, [_tcp_line(8137, "25863")]) + _process(root, "1111", "99999") # a different socket entirely + _process(root, "4242", "25863") + assert describe_listener(8137, root) == "held by pid 4242: /usr/bin/python -m bot" diff --git a/tests/test_transport_rest.py b/tests/test_transport_rest.py index 602e7fb..432f507 100644 --- a/tests/test_transport_rest.py +++ b/tests/test_transport_rest.py @@ -103,16 +103,18 @@ async def test_verify_mode_accepts_json_rpc(config: Config) -> None: assert await transport.verify_mode() == "json-rpc" -async def test_verify_mode_accepts_json_rpc_native(config: Config) -> None: +async def test_verify_mode_rejects_the_native_build(config: Config) -> None: + """It serves the same WebSocket and then fails every attachment send.""" transport = _transport(config, lambda r: httpx.Response(200, json={"mode": "json-rpc-native"})) - assert await transport.verify_mode() == "json-rpc-native" + with pytest.raises(TransportError, match="awt"): + await transport.verify_mode() @pytest.mark.parametrize("body", [{"mode": "normal"}, {"mode": "native"}, {}, ["nope"]]) async def test_verify_mode_rejects_non_websocket_modes(config: Config, body: object) -> None: """In normal/native mode /v1/receive silently degrades to a polling GET.""" transport = _transport(config, lambda r: httpx.Response(200, json=body)) - with pytest.raises(TransportError, match="json-rpc-native"): + with pytest.raises(TransportError, match="not 'json-rpc'"): await transport.verify_mode() @@ -206,3 +208,40 @@ async def test_receive_backs_off_exponentially(config: Config) -> None: async def _no_sleep(seconds: float) -> None: return None + + +def _groups(*entries: object) -> MockHandler: + """Answer /v1/groups with an arbitrary payload.""" + return lambda r: httpx.Response(200, json=list(entries)) + + +async def test_membership_accepts_a_group_matched_by_its_internal_id( + config: Config, +) -> None: + """Envelopes carry internal_id, so that spelling must satisfy the gate.""" + handler = _groups({"id": "group.other", "internal_id": config.group_id, "member": True}) + assert await _transport(config, handler).verify_membership() is None + + +async def test_membership_rejects_a_group_the_bot_was_only_invited_to( + config: Config, +) -> None: + handler = _groups({"id": config.group_id, "name": "testbot", "member": False}) + with pytest.raises(TransportError, match="has not joined"): + await _transport(config, handler).verify_membership() + + +async def test_membership_rejects_an_account_in_no_matching_group( + config: Config, +) -> None: + handler = _groups({"id": "group.someone-elses", "member": True}) + with pytest.raises(TransportError, match="in no group matching"): + await _transport(config, handler).verify_membership() + + +async def test_membership_ignores_entries_that_are_not_objects( + config: Config, +) -> None: + """A malformed row must not mask the real reason the bot is deaf.""" + handler = _groups("unexpected", {"id": config.group_id, "member": True}) + assert await _transport(config, handler).verify_membership() is None