diff --git a/.env.sample b/.env.sample index 9fd2507..a6da578 100644 --- a/.env.sample +++ b/.env.sample @@ -4,18 +4,24 @@ # REQUIRED. The account signal-cli is linked to, in +E.164 form. PHONE_NUMBER= -# REQUIRED. The groups the bot listens to, comma-separated. One entry is -# fine; several let a test group and the production group share one process. +# OPTIONAL, and normally left empty. Comma-separated group ids. +# +# Empty means "every group this account is a member of", discovered at +# runtime: adding the bot to a group is then the whole procedure, with no +# edit here and no restart. That is safe when the bot has its OWN number, +# because such an account has no other chats. +# +# Set it when the bot runs as a LINKED DEVICE on a personal account, where +# it would otherwise see every group on that account. scripts/live_smoke.sh +# writes it for you in that case; scripts/register_bot.sh deliberately does +# not. Startup fails if a listed group is not one the bot has joined. # -# A linked Signal device receives every direct message and every group on the -# account. This value is the ONLY thing scoping the bot to its conversations, -# which is why it has no default and startup fails without it. # Find the ids with: curl -s localhost:9922/v1/groups/$PHONE_NUMBER | jq # -# Replies, rate limits and the daily tally are all per group: a command is -# answered in the group that asked and nowhere else, so there is no separate -# send target to configure. GROUP_ID is still accepted as the one-group -# spelling; GROUP_ID_SEND has been removed and now fails startup if set. +# Replies, rate limits and the daily tally are per group either way: a +# command is answered in the group that asked and nowhere else, so there is +# no send target to configure. GROUP_ID is still accepted as the one-group +# spelling; GROUP_ID_SEND was removed and now fails startup if set. GROUP_IDS= # Optional. The signal-cli-rest-api container. Point this at the fake server diff --git a/README.md b/README.md index 73b15af..314ccc2 100644 --- a/README.md +++ b/README.md @@ -2,17 +2,23 @@ A Signal group bot: `!cat` / `!dog` image commands and a daily message tally. -Runs as a **linked device** on an existing Signal account via +Runs on its **own registered Signal number** (or, if you prefer, as a linked +device on an existing account) via [signal-cli-rest-api](https://github.com/bbernhard/signal-cli-rest-api). ## Scoping, first -A linked device receives **every direct message and every group** on the -account it is linked to. `GROUP_IDS` is the only thing that keeps this bot -inside its own conversations, so: +How wide the bot's reach is depends entirely on **whose account it runs as**, +and the setup scripts pick the safe answer for you rather than leaving it to +be remembered: + +| Run as | Written to `.env` | Reach | +|---|---|---| +| its own number (`register_bot.sh`) | no `GROUP_IDS` | every group somebody adds it to — and nothing else, because that account has no other chats | +| a linked device (`live_smoke.sh`) | `GROUP_IDS=` | only that group. A linked device receives every DM and group on the **personal** account it rides, so it is pinned | + +In both cases: -- the bot **refuses to start** without `GROUP_IDS` (`GROUP_ID` is still - accepted as the one-group spelling); - direct messages are dropped explicitly, never by a falsy check; - the `scoping` scenario asserts that DMs and other groups produce **zero** outbound messages, and `scripts/e2e.sh` fails the build if that stops holding. @@ -136,7 +142,8 @@ 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_IDS`, a taken port — from hiding inside a permanent restart loop. +a pinned group it cannot join, 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 @@ -156,9 +163,9 @@ cannot bind 127.0.0.1:8000 -- held by pid 82385: /usr/bin/python -m ... Starts the container, opens the QR code, waits for the link, then picks a group. **Caveat:** a linked device rides your account — it sends *as you* and -receives every DM and group you are in. `GROUP_IDS` is the only thing scoping -it, which is why that scoping is a tested, CI-enforced invariant rather than a -convention. +receives every DM and group you are in, so `signal_write_env` pins it to the +single group you picked. Prefer `scripts/register_bot.sh`, which gives the bot +its own number and no access to your chats at all. ### Why not Shelter / a second Signal app on the phone @@ -214,7 +221,9 @@ groups inside a single cooldown window and must get two images, one in each. Every non-sticker, non-reaction message in a watched group increments its sender's count **for that group**. At `REPORT_HOUR:REPORT_MINUTE` (default 21:37) each group is posted its own tally, in its own chat, and reset. A group -that refuses the report keeps its counts and does not cost the others theirs. +where nobody spoke is logged and **not** posted into — otherwise every quiet +group the bot has ever been added to would get a nightly ping. A group that +refuses the report keeps its counts and does not cost the others theirs. Counts live in SQLite and survive a restart. ## Develop diff --git a/SECURITY.md b/SECURITY.md index d536539..cc4bc55 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -1,15 +1,29 @@ # Security Policy -## Scope of a linked device +## Scope depends on whose account it runs as -This bot runs as a **linked device** on an existing Signal account. That means -it can read every direct message and every group on that account, and anything -it sends is sent **as the account owner**. It is not a separate identity. +Two deployments, two very different blast radii: -`GROUP_IDS` is the only thing that narrows this. The bot therefore: +- **Its own registered number** (`scripts/register_bot.sh`). A separate + identity with no DMs and no history of its own. The groups it can see are + exactly the groups somebody added it to, so it watches all of them and + discovers new ones by itself. `.env` gets no `GROUP_IDS`. +- **A linked device** (`scripts/live_smoke.sh`). It rides a **personal** + account: it can read every DM and every group on it, and anything it sends + is sent as the account owner. `.env` therefore gets `GROUP_IDS` pinned to + the single group chosen during setup, and `signal_write_env` writes that + automatically — the mode decides, not the operator's memory. -- refuses to start when `GROUP_IDS` is missing or blank, rather than - defaulting to an empty string that matches nothing predictably; +Anyone who can add the bot's number to a group can make it active there. That +is the intended trade for zero-step setup on a dedicated account; pin +`GROUP_IDS` if you want the old behaviour back. + +The bot therefore: + +- writes the pinning decision at setup time, from the mode, so an unpinned + linked device is not something you can arrive at by forgetting a step; +- refuses to start when `GROUP_IDS` names a group it is not a joined member + of, naming the group; - drops messages with no `groupInfo` (direct messages) explicitly, in `counting.watched_group`, rather than relying on a falsy comparison; - resolves a message's group **once**, and uses that one answer as the reply diff --git a/fake_signal/cli.py b/fake_signal/cli.py index 2d8c0a6..2709308 100644 --- a/fake_signal/cli.py +++ b/fake_signal/cli.py @@ -57,6 +57,13 @@ async def run_scenario( """Inject every frame, wait for the bot to react, then compare.""" async with httpx.AsyncClient(base_url=base_url, timeout=10.0) as client: await client.post("/_control/reset") + if scenario.arrive: + # Additive: the groups already on the account stay, exactly as + # they would when somebody adds the bot to one more. + await client.post( + "/_control/groups/add", + json=[arrival.to_entry(groups) for arrival in scenario.arrive], + ) for injection in scenario.inject: await client.post("/_control/inject", json={"raw": injection.to_frame(groups)}) await asyncio.sleep(settle) diff --git a/fake_signal/routes_account.py b/fake_signal/routes_account.py index a1b7913..353081e 100644 --- a/fake_signal/routes_account.py +++ b/fake_signal/routes_account.py @@ -70,11 +70,4 @@ def build_account_router() -> APIRouter: state.accounts.append(number) return JSONResponse({}, status_code=201) - @router.get("/v1/groups/{number}") - async def groups(request: Request, number: str) -> JSONResponse: - state = _state(request) - if number not in state.accounts: - return JSONResponse({"error": f"{number} is not registered"}, status_code=400) - return JSONResponse(state.groups, status_code=200) - return router diff --git a/fake_signal/routes_control.py b/fake_signal/routes_control.py index 7dc5c54..9357b5a 100644 --- a/fake_signal/routes_control.py +++ b/fake_signal/routes_control.py @@ -31,12 +31,26 @@ def build_control_router() -> APIRouter: @router.post("/groups") 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.""" + """Replace 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("/groups/add") + async def add_groups(request: Request, groups: list[dict[str, Any]]) -> dict[str, int]: + """Add groups without disturbing the existing ones. + + Additive on purpose. A scenario that declares only the group it + cares about would otherwise silently remove the others -- and with + the bot dropping a departed group's rows, that would delete state + every later scenario in the same process depends on. + """ + state = _state(request) + known = {group.get("id") for group in state.groups} + state.groups.extend(group for group in groups if group.get("id") not in known) + 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.""" diff --git a/fake_signal/routes_groups.py b/fake_signal/routes_groups.py new file mode 100644 index 0000000..1dd5c42 --- /dev/null +++ b/fake_signal/routes_groups.py @@ -0,0 +1,49 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""The group endpoints, including the join the bot self-heals with.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from fastapi import APIRouter, Request +from fastapi.responses import JSONResponse + +if TYPE_CHECKING: + from fake_signal.state import State + + +def _state(request: Request) -> State: + """Return the State attached to the running app.""" + state: State = request.app.state.fake + return state + + +def build_group_router() -> APIRouter: + """Build the router covering group listing and joining.""" + router = APIRouter() + + @router.get("/v1/groups/{number}") + async def groups(request: Request, number: str) -> JSONResponse: + state = _state(request) + if number not in state.accounts: + return JSONResponse({"error": f"{number} is not registered"}, status_code=400) + return JSONResponse(state.groups, status_code=200) + + @router.post("/v1/groups/{number}/{group_id}/join") + async def join(request: Request, number: str, group_id: str) -> JSONResponse: + """Accept a pending invitation, the way signal-cli sometimes can. + + The real endpoint answers 201 whether or not the account ended up a + member, which is why the bot re-reads the listing instead of + trusting this. Modelling that faithfully is the point: a fake that + only ever succeeds would hide the re-check. + """ + state = _state(request) + if number not in state.accounts: + return JSONResponse({"error": f"{number} is not registered"}, status_code=400) + for group in state.groups: + if group_id in {group.get("id"), group.get("internal_id")} and group.get("joinable"): + group["member"] = True + return JSONResponse({}, status_code=201) + + return router diff --git a/fake_signal/scenario.py b/fake_signal/scenario.py index 4260a43..485ee43 100644 --- a/fake_signal/scenario.py +++ b/fake_signal/scenario.py @@ -52,6 +52,47 @@ class Injection: return json.dumps({"envelope": body, "account": "+15550000000"}) +@dataclass(frozen=True, slots=True) +class GroupArrival: + """A group that appears on the account partway through a scenario. + + This is what makes "somebody added the bot to a group" expressible + offline. ``id`` and ``internal_id`` are scenario alias names, resolved + through the same table the injections use, so a scenario never contains + a real group id. + """ + + id: str + internal_id: str + name: str = "added-later" + member: bool = True + joinable: bool = True + + def to_entry(self, groups: dict[str, str]) -> dict[str, Any]: + """Render this as a /v1/groups row. + + Args: + groups: Scenario alias to real group id. + + Returns: + The listing row the fake server should start returning. + + Raises: + ScenarioError: An alias is not in the table. + """ + for alias in (self.id, self.internal_id): + if alias not in groups: + msg = f"unknown group alias {alias!r}; known: {sorted(groups)}" + raise ScenarioError(msg) + return { + "id": groups[self.id], + "internal_id": groups[self.internal_id], + "name": self.name, + "member": self.member, + "joinable": self.joinable, + } + + @dataclass(frozen=True, slots=True) class Expectation: """One message the bot is expected to have sent.""" @@ -66,6 +107,7 @@ class Scenario: """A named list of injections and the sends they must produce.""" name: str + arrive: list[GroupArrival] = field(default_factory=list) inject: list[Injection] = field(default_factory=list) expect: list[Expectation] = field(default_factory=list) @@ -98,6 +140,7 @@ def load_scenario(path: Path) -> Scenario: raise ScenarioError(msg) return Scenario( name=str(document["name"]), + arrive=[_build(GroupArrival, e, "arrive") for e in document.get("arrive", [])], inject=[_build(Injection, e, "inject") for e in document.get("inject", [])], expect=[_build(Expectation, e, "expect") for e in document.get("expect", [])], ) diff --git a/fake_signal/scenarios/discovers_a_new_group.yaml b/fake_signal/scenarios/discovers_a_new_group.yaml new file mode 100644 index 0000000..808738c --- /dev/null +++ b/fake_signal/scenarios/discovers_a_new_group.yaml @@ -0,0 +1,11 @@ +# The whole feature, in one file: a group appears on the account while the +# bot is running, and the very next message in it is answered. No entry in +# .env, no restart, and not even a wait for the 30-second sweep -- the +# dispatcher refreshes the moment it sees an id it does not recognise. +name: "a group added while the bot is running is answered immediately" +arrive: + - { id: third_reply, internal_id: third, name: added-later } +inject: + - { text: "!cat", group: third, uuid: uuid-hana, name: Hana } +expect: + - { kind: image, to: third_reply } diff --git a/fake_signal/scenarios/joins_a_pending_invitation.yaml b/fake_signal/scenarios/joins_a_pending_invitation.yaml new file mode 100644 index 0000000..434ec81 --- /dev/null +++ b/fake_signal/scenarios/joins_a_pending_invitation.yaml @@ -0,0 +1,10 @@ +# Invited is not joined. The bot must accept the invitation itself rather +# than sitting in the group receiving nothing until a human notices; a +# non-member looks perfectly healthy from outside. +name: "a pending invitation is joined, then answered in" +arrive: + - { id: fourth_reply, internal_id: fourth, name: invited-later, member: false, joinable: true } +inject: + - { text: "!cat", group: fourth, uuid: uuid-ivan, name: Ivan } +expect: + - { kind: image, to: fourth_reply } diff --git a/fake_signal/scenarios/scoping.yaml b/fake_signal/scenarios/scoping.yaml index 891b073..263cf5c 100644 --- a/fake_signal/scenarios/scoping.yaml +++ b/fake_signal/scenarios/scoping.yaml @@ -1,6 +1,8 @@ -# The security invariant. A linked Signal device receives every direct -# conversation and every group on the account; GROUP_ID is the only thing -# scoping the bot. Nothing here may produce a single outbound message. +# The security invariant that survives discovery: the bot answers in groups +# it is a MEMBER of, and in nothing else. A direct message must never reach a +# handler, and a group the account is not in must not be adopted just because +# a frame claiming it arrived. Nothing here may produce a single outbound +# message. name: "direct messages and other groups are never answered" inject: - { text: "!cat", group: null, uuid: uuid-stranger, name: Stranger } diff --git a/fake_signal/server.py b/fake_signal/server.py index d29d65f..d4d68e4 100644 --- a/fake_signal/server.py +++ b/fake_signal/server.py @@ -18,6 +18,7 @@ from fake_signal.routes_account import build_account_router from fake_signal.routes_api import build_api_router from fake_signal.routes_control import build_control_router from fake_signal.routes_fixtures import build_fixture_router +from fake_signal.routes_groups import build_group_router from fake_signal.state import State @@ -27,6 +28,7 @@ def create_app(state: State | None = None) -> FastAPI: app.state.fake = state if state is not None else State() app.include_router(build_api_router()) app.include_router(build_account_router()) + app.include_router(build_group_router()) app.include_router(build_fixture_router()) app.include_router(build_control_router()) return app diff --git a/scripts/e2e.sh b/scripts/e2e.sh index 84d2222..dbe219e 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -30,6 +30,11 @@ readonly WATCHED_GROUP_INTERNAL="ZTJlLXdhdGNoZWQtZ3JvdXAtaWQtMzItYnl0ZXMhISE=" # in the group that asked instead of leaking into the other one. readonly SECOND_GROUP="group.WlRKbExYTmxZMjl1WkMxbmNtOTFjQzFwWkMwek1pMWllWFJsY3lFaElTRT0=" readonly SECOND_GROUP_INTERNAL="ZTJlLXNlY29uZC1ncm91cC1pZC0zMi1ieXRlcyEhISE=" +# Not seeded at boot: these arrive mid-run, which is the point. +readonly THIRD_GROUP="group.WlRKbExYUm9hWEprTFdkeWIzVndMV2xrTFRNeUxXSjVkR1Z6SVNFaElRPT0=" +readonly THIRD_GROUP_INTERNAL="ZTJlLXRoaXJkLWdyb3VwLWlkLTMyLWJ5dGVzISEhIQ==" +readonly FOURTH_GROUP="group.WlRKbExXWnZkWEowYUMxbmNtOTFjQzFwWkMwek1pMWllWFJsY3lFaElTRT0=" +readonly FOURTH_GROUP_INTERNAL="ZTJlLWZvdXJ0aC1ncm91cC1pZC0zMi1ieXRlcyEhISE=" readonly OTHER_GROUP="group.e2e.other==" FAKE_PID="" @@ -102,12 +107,16 @@ main() { -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},{\"id\":\"${SECOND_GROUP}\",\"internal_id\":\"${SECOND_GROUP_INTERNAL}\",\"name\":\"e2e-second\",\"member\":true}]" >/dev/null + -d "[{\"id\":\"${WATCHED_GROUP}\",\"internal_id\":\"${WATCHED_GROUP_INTERNAL}\",\"name\":\"e2e\",\"member\":true,\"joinable\":true},{\"id\":\"${SECOND_GROUP}\",\"internal_id\":\"${SECOND_GROUP_INTERNAL}\",\"name\":\"e2e-second\",\"member\":true,\"joinable\":true}]" >/dev/null + # GROUP_IDS deliberately empty: the bot discovers its groups from the + # account, which is the configuration real deployments now use. Set + # rather than merely unset, because load_dotenv() would otherwise let + # the developer's own .env decide what this test run watches. echo "== booting the bot on :${BOT_PORT}, pointed at the fake" env -u CAT_API \ PHONE_NUMBER="+15550000000" \ - GROUP_IDS="${WATCHED_GROUP},${SECOND_GROUP}" \ + GROUP_IDS="" \ API_BASE_URL="$FAKE_URL" \ DATABASE_PATH="${TEMP_DIR}/e2e.sqlite3" \ CAT_API_URL="${FAKE_URL}/_fixtures/cat-api" \ @@ -126,6 +135,10 @@ main() { --group "reply=${WATCHED_GROUP}" \ --group "second=${SECOND_GROUP_INTERNAL}" \ --group "second_reply=${SECOND_GROUP}" \ + --group "third=${THIRD_GROUP_INTERNAL}" \ + --group "third_reply=${THIRD_GROUP}" \ + --group "fourth=${FOURTH_GROUP_INTERNAL}" \ + --group "fourth_reply=${FOURTH_GROUP}" \ --group "other=${OTHER_GROUP}" } diff --git a/scripts/lib_signal.sh b/scripts/lib_signal.sh index 570cafa..55d0f14 100644 --- a/scripts/lib_signal.sh +++ b/scripts/lib_signal.sh @@ -135,17 +135,40 @@ signal_free_port() { return 1 } -# Write .env for the bot. GROUP_IDS is a comma-separated list; replies always -# go back to whichever of those groups the message came from. +# Write .env for the bot. +# +# The third argument decides whether GROUP_IDS is written at all, and it is +# not a style choice: +# +# own account (register_bot.sh) -> no GROUP_IDS. The bot is its own Signal +# identity, so "every group it is in" is exactly the groups somebody +# added it to. Adding it to a group is then the entire procedure. +# linked device (live_smoke.sh) -> GROUP_IDS pinned to the chosen group. +# A linked device rides a PERSONAL account and receives every group on +# it, so an unpinned bot there would read and answer in all of them. +# +# Getting this backwards is the one way discovery becomes a privacy problem, +# which is why the mode picks the value rather than the operator remembering. signal_write_env() { - local repo_root="$1" number="$2" group="$3" port + local repo_root="$1" number="$2" group="$3" mode="${4:-linked}" port port="$(signal_free_port "${BOT_PORT_BASE:-8137}")" cat > "${repo_root}/.env" <> "${repo_root}/.env" <> "${repo_root}/.env" <&2; exit 1; } - signal_write_env "$REPO_ROOT" "$number" "$group" + # "linked": this is somebody's personal account, so the bot stays pinned + # to the one group instead of discovering every chat on it. + signal_write_env "$REPO_ROOT" "$number" "$group" linked echo "== starting the bot. Send '!cat' in that group; you should get a cat." exec "$REPO_ROOT/run.sh" diff --git a/scripts/register_bot.sh b/scripts/register_bot.sh index 3d351dc..3c11215 100755 --- a/scripts/register_bot.sh +++ b/scripts/register_bot.sh @@ -203,13 +203,16 @@ main() { local group group="$(signal_choose_group "$NUMBER")" signal_require_membership "$NUMBER" "$group" - signal_write_env "$REPO_ROOT" "$NUMBER" "$group" + # "own": this number is the bot's own account, so it may watch every + # group it is added to. See signal_write_env for why the mode decides. + signal_write_env "$REPO_ROOT" "$NUMBER" "$group" own # 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 "== to use it in another group later, just add ${NUMBER} to it." echo "== following the log; Ctrl-C stops watching, not the bot." exec journalctl --user -u signal-bot.service -f } diff --git a/signal_bot/app.py b/signal_bot/app.py index b5c149a..606ae06 100644 --- a/signal_bot/app.py +++ b/signal_bot/app.py @@ -22,6 +22,7 @@ from signal_bot.animals import register_animal_commands from signal_bot.config import Config, load_config from signal_bot.counting import format_tally from signal_bot.decorators import cooldown +from signal_bot.discovery import GroupDirectory from signal_bot.dispatcher import Dispatcher from signal_bot.http_client import ImageFetcher from signal_bot.registry import CommandRegistry @@ -47,6 +48,7 @@ class Bot: config: Config store: Store transport: Transport + directory: GroupDirectory dispatcher: Dispatcher async def publish_report(self) -> None: @@ -55,10 +57,17 @@ class Bot: Each group is isolated on purpose. One group refusing the report -- the bot removed from it, a transient send failure -- must not cost every other group its report, and a tally is cleared only after its - report has actually gone out, so a failed send loses nothing. + report has actually gone out, so a failed send loses nothing. A + group where nobody spoke is skipped entirely. """ - for group_id in self.config.group_ids: + for group_id in self.directory.groups: rows = [(t.display_name or t.uuid, t.count) for t in self.store.tallies(group_id)] + if not rows: + # Logged, not posted. The bot is in every group it was ever + # added to, so posting "nobody spoke" would mean a nightly + # ping into every quiet group, forever. + logger.info("no messages in %s today; not posting a report", group_id) + continue try: await self.transport.send_text(format_tally(rows), group_id) except Exception: @@ -72,7 +81,13 @@ class Bot: await self.dispatcher.handle(frame) -def build_bot(config: Config, client: httpx.AsyncClient, store: Store, transport: Transport) -> Bot: +def build_bot( + config: Config, + client: httpx.AsyncClient, + store: Store, + transport: Transport, + directory: GroupDirectory, +) -> Bot: """Assemble the object graph from its injected dependencies.""" registry = CommandRegistry() register_animal_commands( @@ -83,12 +98,19 @@ def build_bot(config: Config, client: httpx.AsyncClient, store: Store, transport ) dispatcher = Dispatcher( config=config, + directory=directory, registry=registry, store=store, transport=transport, clock=now_local, ) - return Bot(config=config, store=store, transport=transport, dispatcher=dispatcher) + return Bot( + config=config, + store=store, + transport=transport, + directory=directory, + dispatcher=dispatcher, + ) @dataclass @@ -112,6 +134,9 @@ class Runtime: self.tasks = [ asyncio.create_task(self.bot.listen()), asyncio.create_task(scheduler.run()), + # Third loop: without it, a group added later is only noticed + # when somebody happens to speak in it. + asyncio.create_task(self.bot.directory.run()), ] async def stop(self) -> None: @@ -130,8 +155,14 @@ 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) + directory = GroupDirectory(config, client, store) + await directory.refresh() + await directory.verify_allowlist() + return Runtime( + bot=build_bot(config, client, store, transport, directory), + client=client, + store=store, + ) @contextlib.asynccontextmanager @@ -149,8 +180,8 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: logger.info( "listening as %s, watching %d group(s): %r", config.phone_number, - len(config.group_ids), - list(config.group_ids), + len(runtime.bot.directory.groups), + list(runtime.bot.directory.groups), ) app.state.runtime = runtime try: diff --git a/signal_bot/config.py b/signal_bot/config.py index 910146c..63d6462 100644 --- a/signal_bot/config.py +++ b/signal_bot/config.py @@ -55,7 +55,9 @@ class Config: """Everything the bot needs to know, resolved and validated.""" phone_number: str - group_ids: tuple[str, ...] + # The optional allowlist. Empty means every group the account is a + # full member of, discovered at runtime -- see signal_bot.discovery. + group_ids: tuple[str, ...] = () api_base_url: str = DEFAULT_API_BASE_URL database_path: str = DEFAULT_DATABASE_PATH report_hour: int = DEFAULT_REPORT_HOUR @@ -77,34 +79,27 @@ class Config: log_level: str = DEFAULT_LOG_LEVEL @property - def group_alias_map(self) -> dict[str, str]: - """Every spelling of every watched group, mapped to its canonical id.""" - return build_alias_map(self.group_ids) + def is_pinned(self) -> bool: + """True when GROUP_IDS restricts the bot to a fixed set of groups.""" + return bool(self.group_ids) - @property - def watched_group_ids(self) -> frozenset[str]: - """Group ids that count as "a watched group" in an envelope.""" - return frozenset(self.group_alias_map) + def allows(self, group_id: str) -> bool: + """True when a discovered group is permitted by the allowlist. - def resolve_group(self, group_id: str | None) -> str | None: - """Return the canonical id of a watched group, or None. - - This is the single place an envelope's group id becomes a group the - bot acts on. The scoping gate, the reply address and the per-group - state key are all derived from this one answer, so the gate and the - router cannot disagree about which group a message belongs to. + An empty allowlist permits everything: the bot answers wherever it + has been added. This is only the *permission* check -- whether the + account is actually a member is decided by the live listing, in + signal_bot.discovery. Args: - group_id: The group id as it arrived, or None for a direct - message. + group_id: A canonical group id from the group listing. Returns: - The canonical ``group.`` id, or None when the message - came from a direct conversation or an unwatched group. + True when the group may be watched. """ - if group_id is None: - return None - return self.group_alias_map.get(group_id) + if not self.group_ids: + return True + return group_id in build_alias_map(self.group_ids) @property def receive_url(self) -> str: diff --git a/signal_bot/counting.py b/signal_bot/counting.py index 9904ce6..7fcbb1d 100644 --- a/signal_bot/counting.py +++ b/signal_bot/counting.py @@ -6,18 +6,17 @@ from __future__ import annotations from typing import TYPE_CHECKING if TYPE_CHECKING: - from signal_bot.config import Config + from signal_bot.discovery import GroupDirectory from signal_bot.models import IncomingMessage -def watched_group(message: IncomingMessage, config: Config) -> str | None: +def watched_group(message: IncomingMessage, directory: GroupDirectory) -> str | None: """Return the canonical id of the watched group a message came from. - A linked Signal device receives every direct conversation and every group - on the account. This function is the entire boundary between "a bot in - its groups" and "a bot reading all of your chats", so it is deliberately - written as an explicit group-id lookup rather than a falsy check: a - direct message has no group id at all and must never reach a handler. + The bot answers in every group it has been added to, and in nothing + else. Direct messages are rejected here by an explicit lookup rather + than a falsy check: a DM carries no group id at all, and one must never + reach a handler no matter how the surrounding code is later rearranged. It returns the id rather than a bool so that the gate and the reply router are the same decision. Answering "may this run?" and "where does @@ -26,7 +25,7 @@ def watched_group(message: IncomingMessage, config: Config) -> str | None: Args: message: The parsed inbound message. - config: Resolved runtime configuration. + directory: The live set of groups the bot is a member of. Returns: The canonical group id, or None for a direct message or any group @@ -34,7 +33,7 @@ def watched_group(message: IncomingMessage, config: Config) -> str | None: """ if message.group_id is None: return None - return config.resolve_group(message.group_id) + return directory.resolve(message.group_id) def should_count(message: IncomingMessage) -> bool: diff --git a/signal_bot/discovery.py b/signal_bot/discovery.py new file mode 100644 index 0000000..01166fa --- /dev/null +++ b/signal_bot/discovery.py @@ -0,0 +1,238 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""Which groups the bot is in, decided at runtime rather than in .env. + +The bot is its own registered Signal account, so the groups it can see are +exactly the groups somebody put it in. Making the operator also write those +ids into .env and restart added a second step to "add the bot" that could +only ever be forgotten. This module keeps the watched set in sync with the +account itself. + +The one rule that matters: a *failed* refresh never narrows the set. If the +listing cannot be fetched, or comes back malformed, the previous set stands. +Rebuilding from a partial answer would make the bot conclude it had been +removed from everything -- deaf, and with every group's counts purged. +""" + +from __future__ import annotations + +import asyncio +import logging +from collections.abc import Awaitable, Callable +from typing import TYPE_CHECKING, Any, Final + +import httpx + +from signal_bot.groups import build_alias_map, canonical_group_id +from signal_bot.membership import JoinAttempts, MembershipError, attempt_join + +if TYPE_CHECKING: + from signal_bot.config import Config + from signal_bot.store import Store + +logger = logging.getLogger(__name__) + +REFRESH_SECONDS: Final = 30.0 + +Sleeper = Callable[[float], Awaitable[None]] + + +class GroupDirectory: + """The live set of groups the bot answers in.""" + + def __init__( + self, + config: Config, + client: httpx.AsyncClient, + store: Store, + *, + sleeper: Sleeper | None = None, + ) -> None: + """Wire the directory to its config, HTTP client and store. + + Args: + config: Resolved runtime configuration. + client: HTTP client for the REST API. + store: Where per-group state lives, so a group the bot is + removed from can have its rows dropped. + sleeper: Async sleep used by run(). Injected by the tests. + """ + self._config = config + self._client = client + self._store = store + self._sleep = sleeper if sleeper is not None else asyncio.sleep + self._aliases: dict[str, str] = {} + self._groups: tuple[str, ...] = () + self._joins = JoinAttempts() + # Group ids seen in an envelope that were not in the listing yet. One + # refresh per *id*, not per unit of time: a global cooldown would + # make a stranger's message a second earlier swallow the first + # command in a group that was just added. + self._refreshed_for: set[str] = set() + + @property + def groups(self) -> tuple[str, ...]: + """Every group currently watched, canonical and in listing order.""" + return self._groups + + def resolve(self, group_id: str | None) -> str | None: + """Return the canonical id of a watched group, or None. + + This is the single place an envelope's group id becomes a group the + bot acts on: the scoping gate, the reply address and the per-group + state key all come from this one answer. + + Args: + group_id: The id as it arrived, or None for a direct message. + + Returns: + The canonical id, or None for a direct message or a group the + bot does not watch. + """ + if group_id is None: + return None + return self._aliases.get(group_id) + + async def refresh(self) -> None: + """Rebuild the watched set from the account's own group listing. + + Joins anything the bot was merely invited to, then re-reads the + listing so membership is decided by the API rather than by the join + endpoint's own optimistic answer. + """ + entries = await self._fetch() + if entries is None: + return + if await self._join_pending(entries): + entries = await self._fetch() + if entries is None: + return + self._adopt(entries) + + async def refresh_for(self, group_id: str) -> str | None: + """Refresh once because a message arrived from an unknown group. + + Args: + group_id: The unrecognised id from the envelope. + + Returns: + The canonical id if the refresh adopted that group, else None. + """ + if group_id in self._refreshed_for: + return None + self._refreshed_for.add(group_id) + logger.info("message from unknown group %r; refreshing the listing", group_id) + await self.refresh() + return self.resolve(group_id) + + async def verify_allowlist(self) -> None: + """Refuse to start when GROUP_IDS names a group the bot cannot use. + + Only applies when the operator pinned the bot to specific groups. An + unpinned bot watches whatever it has been added to, including + nothing at all, and that is not an error -- it is a bot waiting to + be added. + + Raises: + MembershipError: A pinned group is missing or unjoinable. + """ + if not self._config.is_pinned: + logger.info("watching every group this account is a member of") + return + missing = [group_id for group_id in self._config.group_ids if group_id not in self._groups] + if missing: + details = ", ".join(f"{gid} ({self._joins.reason(gid)})" for gid in missing) + msg = ( + f"GROUP_IDS pins the bot to {len(self._config.group_ids)} group(s) but it is " + f"not a joined member of: {details}. Have an admin add the bot again now that " + "it has a profile, or drop the id from GROUP_IDS." + ) + raise MembershipError(msg) + + async def run(self, iterations: int | None = None) -> None: + """Refresh forever, so a group added later needs no restart. + + Args: + iterations: Stop after this many refreshes. None means forever. + """ + completed = 0 + while iterations is None or completed < iterations: + await self._sleep(REFRESH_SECONDS) + await self.refresh() + completed += 1 + + async def _fetch(self) -> list[dict[str, Any]] | None: + """Return the group listing, or None when it could not be read. + + None is the "keep what we have" signal. Every failure mode of this + call -- timeout, 500, a body that is not a list -- must leave the + watched set alone rather than emptying it. + """ + try: + response = await self._client.get(self._config.groups_url) + response.raise_for_status() + body = response.json() + except (httpx.HTTPError, ValueError) as exc: + logger.warning( + "group listing unavailable, keeping the %d group(s) already known: %s", + len(self._groups), + exc, + ) + return None + if not isinstance(body, list): + logger.warning("group listing was %s, not a list; keeping what we have", type(body)) + return None + return [entry for entry in body if isinstance(entry, dict)] + + def _permitted(self, entry: dict[str, Any]) -> str | None: + """Return the canonical id of a listing row the bot may watch.""" + raw = entry.get("id") or entry.get("internal_id") + if not isinstance(raw, str) or not raw: + return None + group_id = canonical_group_id(raw) + return group_id if self._config.allows(group_id) else None + + async def _join_pending(self, entries: list[dict[str, Any]]) -> bool: + """Try to accept every invitation worth retrying. + + Args: + entries: The current group listing. + + Returns: + True when at least one join was attempted, so the caller knows + the listing needs re-reading. + """ + attempted = False + for entry in entries: + group_id = self._permitted(entry) + if group_id is None or entry.get("member") or self._joins.has_failed(group_id): + continue + logger.warning("invited to %r but not a member; trying to join", entry.get("name")) + detail = await attempt_join(self._client, self._config, group_id) + self._joins.record_failure(group_id, detail) + attempted = True + return attempted + + def _adopt(self, entries: list[dict[str, Any]]) -> None: + """Replace the watched set from a listing known to be good.""" + groups = tuple( + group_id + for entry in entries + if entry.get("member") and (group_id := self._permitted(entry)) is not None + ) + if groups == self._groups: + return + added = [group_id for group_id in groups if group_id not in self._groups] + removed = [group_id for group_id in self._groups if group_id not in groups] + # Built whole and assigned in one statement: the dispatcher reads + # this map between awaits, and must never see it half-built. + self._aliases = build_alias_map(groups) + self._groups = groups + for group_id in added: + self._joins.forget(group_id) + self._refreshed_for.discard(group_id) + logger.info("now watching %s", group_id) + for group_id in removed: + # Dropped, not kept: a re-add should start a fresh tally rather + # than resurrect counts from whenever the bot was last in there. + self._store.forget_group(group_id) + logger.info("no longer a member of %s; its counts were dropped", group_id) diff --git a/signal_bot/dispatcher.py b/signal_bot/dispatcher.py index 44ed4c7..30fa018 100644 --- a/signal_bot/dispatcher.py +++ b/signal_bot/dispatcher.py @@ -15,6 +15,7 @@ from signal_bot.registry import CommandContext, CommandRegistry if TYPE_CHECKING: from signal_bot.config import Config + from signal_bot.discovery import GroupDirectory from signal_bot.store import Store from signal_bot.transport_base import Transport @@ -28,6 +29,7 @@ class Dispatcher: """Applies the scoping rules and dispatches commands.""" config: Config + directory: GroupDirectory registry: CommandRegistry store: Store transport: Transport @@ -44,12 +46,18 @@ class Dispatcher: # The canonical id it yields is the reply address and the key for # every piece of per-group state, so a message can only ever be # counted, rate-limited and answered in the group it came from. - group = watched_group(message, self.config) + group = watched_group(message, self.directory) + if group is None and message.group_id is not None: + # The bot was probably just added: Signal is already delivering + # this group's messages while our listing still predates it. + # Refreshing here is what makes the *first* command after an add + # work, instead of the one after the next 30-second sweep. + group = await self.directory.refresh_for(message.group_id) if group is None: logger.debug( "ignoring message from group %r; watching %r", message.group_id, - sorted(self.config.group_ids), + list(self.directory.groups), ) return diff --git a/signal_bot/groups.py b/signal_bot/groups.py index 1ea6c51..2bf6618 100644 --- a/signal_bot/groups.py +++ b/signal_bot/groups.py @@ -93,21 +93,30 @@ def build_alias_map(group_ids: Iterable[str]) -> dict[str, str]: def parse_group_ids(env: dict[str, str]) -> tuple[str, ...]: - """Return the watched groups, canonical and de-duplicated. + """Return the optional allowlist of groups, canonical and de-duplicated. - ``GROUP_IDS`` is the list form; ``GROUP_ID`` remains accepted as a - one-element spelling so an existing .env keeps working. Setting both is - an error rather than a precedence rule, because a precedence rule means - silently ignoring one of two values the operator clearly meant. + Empty is the normal case and means "every group this account is a full + member of". The bot is its own registered Signal account, not a linked + device on a personal one, so the groups it can see are exactly the + groups somebody put it in -- there is nothing to scope it away from, + and requiring a list here is what made adding the bot a two-step + procedure with a restart in the middle. + + Set it to pin the bot to specific groups anyway. ``GROUP_IDS`` is the + list form; ``GROUP_ID`` remains accepted as a one-element spelling. + Setting both is an error rather than a precedence rule, because a + precedence rule means silently ignoring one of two values the operator + clearly meant. Args: env: The environment mapping to read. Returns: - Canonical group ids, in the order they were listed. + Canonical group ids in the order listed, or empty for "all of them". Raises: - ConfigError: Neither key is set, both are set, or a removed key is. + ConfigError: Both keys are set, a removed key is set, or the value + is nothing but separators. """ if env.get("GROUP_ID_SEND", "").strip(): msg = ( @@ -126,13 +135,7 @@ def parse_group_ids(env: dict[str, str]) -> tuple[str, ...]: raw = listed or single if not raw: - msg = ( - "GROUP_IDS is required and must not be empty. A linked Signal " - "device sees every chat on the account, so the bot refuses to " - "start without an explicit list of groups to scope itself to. " - "Find them with: curl -s $API_BASE_URL/v1/groups/$PHONE_NUMBER" - ) - raise ConfigError(msg) + return () group_ids: list[str] = [] seen: dict[str, str] = {} diff --git a/signal_bot/membership.py b/signal_bot/membership.py index 763dba9..2c8c555 100644 --- a/signal_bot/membership.py +++ b/signal_bot/membership.py @@ -1,16 +1,18 @@ # Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. -"""Is the account actually *in* the groups it is configured to watch? +"""Invited is not joined. -Split out of transport_rest.py because it answers a different question from -"how do I send a message": a pending invitation is indistinguishable from -membership from the outside, and the whole point of this module is that the -difference is checked before the bot is allowed to start. +Signal only adds an account to a group outright when whoever adds it already +holds its profile key; otherwise the add becomes a pending invitation. A +non-member looks perfectly healthy from outside -- the group is listed, the +websocket is up, /healthz answers -- and not one message ever arrives. This +module is the part that tells the two states apart and tries to fix the +second one. """ from __future__ import annotations import logging -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, Final from urllib.parse import quote import httpx @@ -22,116 +24,83 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) +MEMBER_KEY: Final = "member" + class MembershipError(RuntimeError): - """Raised when the account is not a full member of a watched group.""" + """Raised when the account is not a full member of a required group.""" -class MembershipVerifier: - """Checks each watched group, joining one it was only invited to.""" +def entry_matches(entry: dict[str, Any], group_id: str) -> bool: + """True when a /v1/groups row is the group with this id, in any spelling. - def __init__(self, config: Config, client: httpx.AsyncClient) -> None: - """Wire the verifier to its config and HTTP client.""" - self._config = config - self._client = client + Args: + entry: One row of the group listing. + group_id: The canonical id being looked for. - async def _fetch_groups(self) -> list[dict[str, Any]]: - """Return the groups the account knows about, as raw dicts.""" - response = await self._client.get(self._config.groups_url) - response.raise_for_status() - return [group for group in response.json() if isinstance(group, dict)] + Returns: + True when the row describes that group. + """ + return bool({entry.get("id"), entry.get("internal_id")} & group_id_aliases(group_id)) - @staticmethod - def _find_group(groups: list[dict[str, Any]], group_id: str) -> dict[str, Any] | None: - """Return the entry for one configured group, in either spelling.""" - aliases = group_id_aliases(group_id) - for group in groups: - if {group.get("id"), group.get("internal_id")} & aliases: - return group - return None - async def _attempt_join(self, group_id: str) -> str: - """Try to accept a pending invitation, reporting what the API said. +def find_entry(entries: list[dict[str, Any]], group_id: str) -> dict[str, Any] | None: + """Return the listing row for one group, or None when it is absent.""" + return next((entry for entry in entries if entry_matches(entry, group_id)), None) - This never raises: the join endpoint is a best-effort self-heal and - the authority on whether it worked is the membership re-check by the - caller, not this response. Returning the outcome as text keeps it in - the error message when the re-check still says non-member. - Args: - group_id: The canonical group id to join. +class JoinAttempts: + """Remembers which invitations could not be accepted. - Returns: - A short description of the attempt, for the error message. - """ - # The prefixed spelling, percent-encoded: the internal form contains - # '/' and '+', which do not survive being a path segment. - quoted = quote(group_id, safe="") - url = f"{self._config.api_base_url}/v1/groups/{self._config.phone_number}/{quoted}/join" - try: - response = await self._client.post(url) - except httpx.HTTPError as exc: - return f"join request failed: {exc}" - return f"join returned HTTP {response.status_code}: {response.text.strip()[:200]}" + signal-cli cannot accept an invite addressed to a phone number at all + ("Cannot find service ID for self"). Without this, the refresh loop would + retry that same impossible join every 30 seconds forever, logging a + warning each time and burying anything worth reading. + """ - async def verify(self) -> None: - """Raise unless the account is a full member of every watched group. + def __init__(self) -> None: + """Start with nothing known to have failed.""" + self._failed: dict[str, str] = {} - 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. 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. + def has_failed(self, group_id: str) -> bool: + """True when joining this group has already been tried and refused.""" + return group_id in self._failed - Each configured group is checked on its own. One joined group - satisfying the check for another is exactly how a bot ends up - silently deaf in the group nobody was watching. + def reason(self, group_id: str) -> str: + """Return why the last join attempt for this group failed.""" + return self._failed.get(group_id, "not attempted") - Raises: - MembershipError: A watched group is missing, or the account is - not a full member of it and could not join. - """ - groups = await self._fetch_groups() - for group_id in self._config.group_ids: - await self._verify_one(groups, group_id) + def record_failure(self, group_id: str, detail: str) -> None: + """Remember that this group could not be joined, and why.""" + self._failed[group_id] = detail - async def _verify_one(self, groups: list[dict[str, Any]], group_id: str) -> None: - """Check one configured group, joining it if it is only an invite. + def forget(self, group_id: str) -> None: + """Drop the memory of a failure, once the group is joined.""" + self._failed.pop(group_id, None) - Args: - groups: The current /v1/groups listing. - group_id: The canonical group id to check. - Raises: - MembershipError: The group is unknown, or still not joined after - the join attempt. - """ - state = self._find_group(groups, group_id) - if state is None: - msg = ( - f"the account is in no group matching {group_id!r}. Check GROUP_IDS " - "in .env against: curl -s $API_BASE_URL/v1/groups/$PHONE_NUMBER" - ) - raise MembershipError(msg) - if state.get("member"): - return +async def attempt_join(client: httpx.AsyncClient, config: Config, group_id: str) -> str: + """Try to accept a pending invitation, reporting what the API said. - name = state.get("name") - logger.warning("invited to %r but not a member; attempting to join", name) - detail = await self._attempt_join(group_id) - # The join endpoint's own response is not trusted: only the listing - # decides, so the gate cannot degrade into an optimistic warning. - rechecked = self._find_group(await self._fetch_groups(), group_id) - if rechecked is not None and rechecked.get("member"): - logger.info("joined %r on startup (%s)", name, detail) - return + This never raises. The join endpoint is a best-effort self-heal, and the + authority on whether it worked is the caller's membership re-check, not + this response -- returning the outcome as text only keeps it available + for the error message when the re-check still says non-member. - msg = ( - f"the account is invited to {name!r} ({group_id}) but has not joined " - f"it, so it will receive nothing there, and joining did not work: " - f"{detail}. Have an admin remove the pending invite and add the bot " - "again now that it has a profile, or have the bot create the group." - ) - raise MembershipError(msg) + Args: + client: HTTP client for the REST API. + config: Resolved runtime configuration. + group_id: The canonical group id to join. + + Returns: + A short description of the attempt. + """ + # Percent-encoded: the internal spelling contains '/' and '+', which do + # not survive being a path segment. + quoted = quote(group_id, safe="") + url = f"{config.api_base_url}/v1/groups/{config.phone_number}/{quoted}/join" + try: + response = await client.post(url) + except httpx.HTTPError as exc: + return f"join request failed: {exc}" + return f"join returned HTTP {response.status_code}: {response.text.strip()[:200]}" diff --git a/signal_bot/store.py b/signal_bot/store.py index e82ff8d..f5b9b2d 100644 --- a/signal_bot/store.py +++ b/signal_bot/store.py @@ -116,6 +116,23 @@ class Store: """Close the underlying connection.""" self._conn.close() + def forget_group(self, group_id: str) -> None: + """Drop every row belonging to one group. + + Called when the bot is removed from a group, so that being added + back starts a fresh tally and a clean cooldown rather than + resurrecting whatever was true whenever it was last in there. + """ + # Spelled out rather than looped over _TABLES: an f-string here is a + # SQL-injection shape even when the input is a module constant, and + # the repo does not take lint suppressions. test_forget_group_clears + # _every_table is what keeps this list in step with the schema. + self._conn.execute("DELETE FROM message_counts WHERE group_id = ?", (group_id,)) + self._conn.execute("DELETE FROM command_uses WHERE group_id = ?", (group_id,)) + self._conn.execute("DELETE FROM daily_uses WHERE group_id = ?", (group_id,)) + self._conn.execute("DELETE FROM votes WHERE group_id = ?", (group_id,)) + self._conn.commit() + # -- message tallies --------------------------------------------------- def increment(self, group_id: str, uuid: str, display_name: str) -> None: diff --git a/signal_bot/transport_rest.py b/signal_bot/transport_rest.py index aa7d9c8..b9268dd 100644 --- a/signal_bot/transport_rest.py +++ b/signal_bot/transport_rest.py @@ -16,8 +16,6 @@ from typing import TYPE_CHECKING, Any, Final from websockets.asyncio.client import connect as ws_connect from websockets.exceptions import WebSocketException -from signal_bot.membership import MembershipError, MembershipVerifier - if TYPE_CHECKING: import httpx @@ -89,22 +87,6 @@ class RestTransport: raise TransportError(msg) return mode - async def verify_membership(self) -> None: - """Raise unless the account is a full member of every watched group. - - Delegated to signal_bot.membership, which owns the invite-versus-join - distinction; kept as a method here because it is a startup precondition - of this transport, not of the bot in general. - - Raises: - TransportError: A watched group is missing, or the account is not - a full member of it and could not join. - """ - try: - await MembershipVerifier(self._config, self._client).verify() - except MembershipError as exc: - raise TransportError(str(exc)) from exc - async def receive(self) -> AsyncIterator[str]: """Yield raw envelope frames, reconnecting with backoff when dropped.""" backoff = INITIAL_BACKOFF diff --git a/tests/conftest.py b/tests/conftest.py index 81ea7ee..4362f52 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,28 +7,56 @@ import datetime as dt import json from typing import Any +import httpx import pytest from signal_bot.config import Config +from signal_bot.discovery import GroupDirectory from signal_bot.store import Store from signal_bot.transport_fake import FakeTransport -WATCHED_GROUP = "group-watched==" -SECOND_GROUP = "group-second==" -OTHER_GROUP = "group-other==" +# Canonical ``group.`` ids, the spelling /v1/groups reports and the +# only one /v2/send accepts. The *_INTERNAL forms are what an envelope +# carries: exactly what the canonical form decodes to. +WATCHED_GROUP = "group.ZDJGMFkyaGxaQzFuY205MWNDMXBaQzB6TWkxaWVYUmxjeUVoSVNFaElRPT0=" +WATCHED_INTERNAL = "d2F0Y2hlZC1ncm91cC1pZC0zMi1ieXRlcyEhISEhIQ==" +SECOND_GROUP = "group.YzJWamIyNWtMV2R5YjNWd0xXbGtMVE15TFdKNWRHVnpJU0VoSVNFaA==" +SECOND_INTERNAL = "c2Vjb25kLWdyb3VwLWlkLTMyLWJ5dGVzISEhISEh" +OTHER_GROUP = "group.YjNSb1pYSXRaM0p2ZFhBdGFXUXRNekl0WW5sMFpYTWhJU0VoSVNFPQ==" @pytest.fixture def config() -> Config: + """The production shape: no allowlist, groups discovered from the account.""" return Config( phone_number="+15550000000", - group_ids=(WATCHED_GROUP, SECOND_GROUP), api_base_url="http://localhost:9922", database_path=":memory:", cooldown_seconds=10, ) +def group_entry(group_id: str, *, member: bool = True, name: str = "e2e") -> dict[str, Any]: + """One /v1/groups row, in both spellings the real API uses.""" + return {"id": group_id, "internal_id": group_id, "name": name, "member": member} + + +def listing_client(entries: list[dict[str, Any]]) -> httpx.AsyncClient: + """A client whose /v1/groups always answers with these rows.""" + return httpx.AsyncClient( + transport=httpx.MockTransport(lambda request: httpx.Response(200, json=entries)) + ) + + +@pytest.fixture +async def directory(config: Config, store: Store) -> GroupDirectory: + """A directory that has already discovered both test groups.""" + client = listing_client([group_entry(WATCHED_GROUP), group_entry(SECOND_GROUP)]) + found = GroupDirectory(config, client, store) + await found.refresh() + return found + + @pytest.fixture def store() -> Store: with Store(":memory:") as opened: diff --git a/tests/test_allowlist.py b/tests/test_allowlist.py new file mode 100644 index 0000000..2ab4f56 --- /dev/null +++ b/tests/test_allowlist.py @@ -0,0 +1,117 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""GROUP_IDS as a restriction: pinning the bot, and refusing to start unpinned.""" + +from __future__ import annotations + +from dataclasses import replace +from typing import TYPE_CHECKING + +import httpx +import pytest + +from signal_bot.discovery import GroupDirectory +from signal_bot.membership import MembershipError +from tests.conftest import SECOND_GROUP, WATCHED_GROUP, group_entry + +if TYPE_CHECKING: + from signal_bot.config import Config + from signal_bot.store import Store + + +def _directory(config: Config, store: Store, handler: object) -> GroupDirectory: + return GroupDirectory(config, httpx.AsyncClient(transport=httpx.MockTransport(handler)), store) + + +async def test_the_allowlist_restricts_what_is_adopted(config: Config, store: Store) -> None: + """GROUP_IDS still pins the bot when somebody wants it pinned.""" + pinned = replace(config, group_ids=(WATCHED_GROUP,)) + found = _directory( + pinned, + store, + lambda r: httpx.Response(200, json=[group_entry(WATCHED_GROUP), group_entry(SECOND_GROUP)]), + ) + await found.refresh() + assert found.groups == (WATCHED_GROUP,) + await found.verify_allowlist() + + +async def test_verify_allowlist_names_a_pinned_group_that_is_missing( + config: Config, store: Store +) -> None: + pinned = replace(config, group_ids=(WATCHED_GROUP, SECOND_GROUP)) + found = _directory( + pinned, store, lambda r: httpx.Response(200, json=[group_entry(WATCHED_GROUP)]) + ) + await found.refresh() + with pytest.raises(MembershipError, match="not a joined member") as raised: + await found.verify_allowlist() + assert SECOND_GROUP in str(raised.value) + + +async def test_verify_allowlist_reports_why_the_join_failed(config: Config, store: Store) -> None: + """The refusal carries the API's own answer, not a generic complaint.""" + pinned = replace(config, group_ids=(WATCHED_GROUP,)) + + def handler(request: httpx.Request) -> httpx.Response: + if request.method == "POST": + return httpx.Response(400, text="Cannot find service ID for self") + return httpx.Response(200, json=[group_entry(WATCHED_GROUP, member=False)]) + + found = _directory(pinned, store, handler) + await found.refresh() + with pytest.raises(MembershipError, match="Cannot find service ID"): + await found.verify_allowlist() + + +async def test_an_unpinned_bot_in_no_groups_is_allowed_to_start( + config: Config, store: Store +) -> None: + found = _directory(config, store, lambda r: httpx.Response(200, json=[])) + await found.refresh() + await found.verify_allowlist() + + +async def test_run_refreshes_on_a_schedule(config: Config, store: Store) -> None: + """A group added while nobody is talking is still picked up.""" + slept: list[float] = [] + listings = iter([[], [group_entry(WATCHED_GROUP)]]) + + async def sleeper(seconds: float) -> None: + slept.append(seconds) + + found = GroupDirectory( + config, + httpx.AsyncClient( + transport=httpx.MockTransport(lambda r: httpx.Response(200, json=next(listings))) + ), + store, + sleeper=sleeper, + ) + await found.run(iterations=2) + assert slept == [30.0, 30.0] + assert found.groups == (WATCHED_GROUP,) + + +async def test_a_failed_re_read_after_joining_keeps_the_old_set( + config: Config, store: Store +) -> None: + """Fail closed: an unverified join must not be treated as membership.""" + responses = iter( + [ + httpx.Response(200, json=[group_entry(WATCHED_GROUP)]), + httpx.Response( + 200, json=[group_entry(WATCHED_GROUP), group_entry(SECOND_GROUP, member=False)] + ), + httpx.Response(201, text=""), + httpx.Response(503, text="gone"), + ] + ) + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(201, text="") if request.method == "POST" else next(responses) + + found = _directory(config, store, handler) + await found.refresh() + assert found.groups == (WATCHED_GROUP,) + await found.refresh() + assert found.groups == (WATCHED_GROUP,) diff --git a/tests/test_app.py b/tests/test_app.py index 7e8b342..012b71c 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -4,7 +4,7 @@ from __future__ import annotations import asyncio -from dataclasses import dataclass +from dataclasses import dataclass, replace from typing import TYPE_CHECKING import httpx @@ -13,9 +13,11 @@ from fastapi.testclient import TestClient from signal_bot import app as app_module from signal_bot.app import Bot, Runtime, build_bot, create_runtime, lifespan, now_local +from signal_bot.discovery import GroupDirectory +from signal_bot.membership import MembershipError from signal_bot.transport_fake import FakeTransport from signal_bot.transport_rest import TransportError -from tests.conftest import SECOND_GROUP, WATCHED_GROUP, envelope +from tests.conftest import SECOND_GROUP, WATCHED_GROUP, envelope, group_entry, listing_client if TYPE_CHECKING: from signal_bot.config import Config @@ -24,7 +26,7 @@ if TYPE_CHECKING: BASE_ENV = { "PHONE_NUMBER": "+15550000000", - "GROUP_IDS": "group.watched==", + "GROUP_IDS": "", "DATABASE_PATH": ":memory:", } @@ -39,29 +41,18 @@ def _client(handler: Handler) -> httpx.AsyncClient: def _signal(*group_ids: str, mode: str = "json-rpc", member: bool = True) -> Handler: - """Answer both startup probes: the mode check and the membership check.""" + """Answer both startup probes: the mode check and the group listing.""" 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": f"group {index}", - "member": member, - } - for index, group_id in enumerate(group_ids) - ], - ) + return httpx.Response(200, json=[group_entry(gid, member=member) for gid in group_ids]) 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) +def _bot(config: Config, store: Store, transport: FakeTransport, directory: GroupDirectory) -> Bot: + return build_bot(config, _client(lambda r: httpx.Response(200)), store, transport, directory) def test_now_local_is_timezone_aware() -> None: @@ -69,21 +60,21 @@ def test_now_local_is_timezone_aware() -> None: def test_build_bot_registers_the_animal_commands( - config: Config, store: Store, transport: FakeTransport + config: Config, store: Store, transport: FakeTransport, directory: GroupDirectory ) -> None: - bot = _bot(config, store, transport) + bot = _bot(config, store, transport, directory) assert bot.dispatcher.registry.resolve("!cat") is not None assert bot.dispatcher.registry.resolve("!pies") is not None async def test_publish_report_posts_each_group_its_own_tally( - config: Config, store: Store, transport: FakeTransport + config: Config, store: Store, transport: FakeTransport, directory: GroupDirectory ) -> None: """Every group gets its own counts, in its own chat, and only its own.""" store.increment(WATCHED_GROUP, "u1", "Alice") store.increment(SECOND_GROUP, "u2", "Bob") store.increment(SECOND_GROUP, "u2", "Bob") - await _bot(config, store, transport).publish_report() + await _bot(config, store, transport, directory).publish_report() assert [(m.recipient, m.text) for m in transport.sent] == [ (WATCHED_GROUP, "Alice 1"), (SECOND_GROUP, "Bob 2"), @@ -93,23 +84,23 @@ async def test_publish_report_posts_each_group_its_own_tally( async def test_publish_report_falls_back_to_uuid_when_unnamed( - config: Config, store: Store, transport: FakeTransport + config: Config, store: Store, transport: FakeTransport, directory: GroupDirectory ) -> None: store.increment(WATCHED_GROUP, "u1", "") - await _bot(config, store, transport).publish_report() + await _bot(config, store, transport, directory).publish_report() assert transport.texts[0] == "u1 1" async def test_listen_dispatches_every_frame( - config: Config, store: Store, transport: FakeTransport + config: Config, store: Store, transport: FakeTransport, directory: GroupDirectory ) -> None: transport.queue([envelope(text="hello", group_id=WATCHED_GROUP)]) - await _bot(config, store, transport).listen() + await _bot(config, store, transport, directory).listen() assert store.tallies(WATCHED_GROUP)[0].count == 1 async def test_create_runtime_verifies_the_container_mode(config: Config) -> None: - runtime = await create_runtime(config, _client(_signal(*config.group_ids))) + runtime = await create_runtime(config, _client(_signal(WATCHED_GROUP, SECOND_GROUP))) assert isinstance(runtime, Runtime) await runtime.stop() @@ -122,12 +113,13 @@ async def test_create_runtime_refuses_a_polling_container(config: Config) -> Non async def test_runtime_start_and_stop_cancels_tasks( - config: Config, store: Store, transport: FakeTransport + config: Config, store: Store, transport: FakeTransport, directory: GroupDirectory ) -> None: client = _client(lambda r: httpx.Response(200)) - runtime = Runtime(bot=_bot(config, store, transport), client=client, store=store) + runtime = Runtime(bot=_bot(config, store, transport, directory), client=client, store=store) runtime.start() - assert len(runtime.tasks) == 2 + # listen, the daily report, and the group refresh loop. + assert len(runtime.tasks) == 3 await asyncio.sleep(0) await runtime.stop() assert all(task.done() for task in runtime.tasks) @@ -139,7 +131,7 @@ def test_healthz_serves_and_startup_completes(monkeypatch: pytest.MonkeyPatch) - monkeypatch.setenv(key, value) def fake_client(**kwargs: object) -> httpx.AsyncClient: - return _client(_signal(BASE_ENV["GROUP_IDS"])) + return _client(_signal(WATCHED_GROUP)) monkeypatch.setattr(app_module.httpx, "AsyncClient", fake_client) with TestClient(app_module.app) as client: @@ -154,18 +146,28 @@ async def test_lifespan_is_an_async_context_manager( monkeypatch.setattr( app_module.httpx, "AsyncClient", - lambda **kwargs: _client(_signal(BASE_ENV["GROUP_IDS"])), + lambda **kwargs: _client(_signal(WATCHED_GROUP)), ) 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( +async def test_create_runtime_refuses_a_pinned_group_it_cannot_join( 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_ids, member=False))) + """A pending invite looks healthy from outside; a pinned startup must not.""" + pinned = replace(config, group_ids=(WATCHED_GROUP,)) + with pytest.raises(MembershipError, match="not a joined member"): + await create_runtime(pinned, _client(_signal(WATCHED_GROUP, member=False))) + + +async def test_create_runtime_accepts_an_account_in_no_groups_at_all( + config: Config, +) -> None: + """An unpinned bot with no groups is not broken; it is waiting to be added.""" + runtime = await create_runtime(config, _client(_signal())) + assert runtime.bot.directory.groups == () + await runtime.stop() @dataclass @@ -183,16 +185,38 @@ class RefusingTransport(FakeTransport): async def test_one_group_failing_its_report_does_not_cost_the_others( - config: Config, store: Store + config: Config, store: Store, directory: GroupDirectory ) -> None: """A failed send must not clear that group's day, nor skip the next group.""" store.increment(WATCHED_GROUP, "u1", "Alice") store.increment(SECOND_GROUP, "u2", "Bob") transport = RefusingTransport(refuses=WATCHED_GROUP) - await _bot(config, store, transport).publish_report() + await _bot(config, store, transport, directory).publish_report() assert [(m.recipient, m.text) for m in transport.sent] == [(SECOND_GROUP, "Bob 1")] # The group that failed keeps its counts; the one that succeeded is reset. assert store.tallies(WATCHED_GROUP)[0].count == 1 assert store.tallies(SECOND_GROUP) == [] + + +async def test_a_quiet_group_is_logged_rather_than_posted_into( + config: Config, store: Store, transport: FakeTransport, directory: GroupDirectory +) -> None: + """The bot sits in every group it was added to; a nightly ping is noise.""" + store.increment(SECOND_GROUP, "u2", "Bob") + await _bot(config, store, transport, directory).publish_report() + assert [(m.recipient, m.text) for m in transport.sent] == [(SECOND_GROUP, "Bob 1")] + + +async def test_the_report_follows_the_live_group_set( + config: Config, store: Store, transport: FakeTransport +) -> None: + """Groups come from the directory, not from .env, so a new one is included.""" + client = listing_client([group_entry(WATCHED_GROUP), group_entry(SECOND_GROUP)]) + directory = GroupDirectory(config, client, store) + await directory.refresh() + store.increment(WATCHED_GROUP, "u1", "Alice") + store.increment(SECOND_GROUP, "u2", "Bob") + await _bot(config, store, transport, directory).publish_report() + assert [m.recipient for m in transport.sent] == [WATCHED_GROUP, SECOND_GROUP] diff --git a/tests/test_cli.py b/tests/test_cli.py index c09612b..098fba3 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -3,6 +3,7 @@ from __future__ import annotations +import json import runpy import sys from pathlib import Path @@ -11,7 +12,7 @@ import httpx import pytest from fake_signal import cli -from fake_signal.scenario import Expectation, Injection, Scenario +from fake_signal.scenario import Expectation, GroupArrival, Injection, Scenario GROUPS = {"watched": "W==", "reply": "R==", "other": "O=="} @@ -22,12 +23,16 @@ class Recorder: def __init__(self, sent: list[dict]) -> None: self.sent = sent self.injected: list[str] = [] + self.added: list[list[dict]] = [] def __call__(self, request: httpx.Request) -> httpx.Response: path = request.url.path if path == "/_control/inject": self.injected.append(request.content.decode()) return httpx.Response(200, json={"delivered": 1}) + if path == "/_control/groups/add": + self.added.append(json.loads(request.content)) + return httpx.Response(200, json={"groups": 1}) if path == "/_control/sent": return httpx.Response(200, json=self.sent) return httpx.Response(200, json={"status": "ok"}) @@ -55,6 +60,30 @@ async def test_run_scenario_injects_and_passes(monkeypatch: pytest.MonkeyPatch) assert "!cat" in recorder.injected[0] +async def test_run_scenario_announces_arriving_groups(monkeypatch: pytest.MonkeyPatch) -> None: + """A scenario can add a group to the account before it injects anything.""" + recorder = Recorder([{"base64_attachments": ["A"], "recipients": ["R=="]}]) + _patch_client(monkeypatch, recorder) + scenario = Scenario( + "s", + arrive=[GroupArrival(id="reply", internal_id="watched")], + inject=[Injection(text="!cat")], + expect=[Expectation(kind="image")], + ) + assert await cli.run_scenario("http://x", scenario, GROUPS, settle=0) == [] + assert recorder.added == [ + [ + { + "id": "R==", + "internal_id": "W==", + "name": "added-later", + "member": True, + "joinable": True, + } + ] + ] + + async def test_run_scenario_reports_a_mismatch(monkeypatch: pytest.MonkeyPatch) -> None: _patch_client(monkeypatch, Recorder([])) scenario = Scenario("s", expect=[Expectation(kind="image")]) diff --git a/tests/test_config.py b/tests/test_config.py index 8917e99..18d39dc 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -31,20 +31,25 @@ def test_loads_minimal_environment() -> None: assert config.report_minute == 37 -@pytest.mark.parametrize("missing", ["PHONE_NUMBER", "GROUP_IDS"]) -def test_refuses_to_start_without_required_value(missing: str) -> None: +def test_refuses_to_start_without_a_phone_number() -> None: env = dict(BASE_ENV) - del env[missing] - with pytest.raises(ConfigError, match=missing): + del env["PHONE_NUMBER"] + with pytest.raises(ConfigError, match="PHONE_NUMBER"): load_config(env) -@pytest.mark.parametrize("missing", ["PHONE_NUMBER", "GROUP_IDS"]) -def test_refuses_to_start_on_blank_value(missing: str) -> None: - """A linked device sees every chat, so an unscoped bot must not start.""" - env = dict(BASE_ENV) | {missing: " "} - with pytest.raises(ConfigError, match=missing): - load_config(env) +def test_refuses_to_start_on_a_blank_phone_number() -> None: + with pytest.raises(ConfigError, match="PHONE_NUMBER"): + load_config(dict(BASE_ENV) | {"PHONE_NUMBER": " "}) + + +@pytest.mark.parametrize("env", [{}, {"GROUP_IDS": " "}]) +def test_no_allowlist_is_the_normal_case(env: dict[str, str]) -> None: + """Empty means "every group I am in": adding the bot is the whole setup.""" + config = load_config({"PHONE_NUMBER": "+15550000000"} | env) + assert config.group_ids == () + assert not config.is_pinned + assert config.allows("group.anything==") def test_overrides_are_honoured() -> None: @@ -135,9 +140,13 @@ def test_group_id_aliases_tolerates_undecodable_ids() -> None: assert group_id_aliases("group.not!valid!base64") == {"group.not!valid!base64"} -def test_watched_group_ids_exposes_both_spellings() -> None: +def test_the_allowlist_accepts_both_spellings() -> None: + """An envelope's internal id must satisfy an allowlist written the other way.""" config = load_config({"PHONE_NUMBER": "+15550000000", "GROUP_IDS": PREFIXED}) - assert INTERNAL in config.watched_group_ids + assert config.is_pinned + assert config.allows(PREFIXED) + assert config.allows(INTERNAL) + assert not config.allows("group.someone-elses==") def test_groups_url_is_scoped_to_the_account() -> None: @@ -178,8 +187,8 @@ def test_several_groups_are_parsed_in_order() -> None: """The point of the change: one process, a test group and a real one.""" config = load_config(dict(BASE_ENV) | {"GROUP_IDS": f"{PREFIXED}, group.other=="}) assert config.group_ids == (PREFIXED, "group.other==") - assert config.resolve_group(INTERNAL) == PREFIXED - assert config.resolve_group("group.other==") == "group.other==" + assert config.allows(INTERNAL) + assert config.allows("group.other==") def test_group_id_is_still_accepted_as_the_one_group_spelling() -> None: @@ -220,7 +229,7 @@ def test_an_internal_id_is_stored_in_its_addressable_form() -> None: """Replies are sent to the canonical id, and /v2/send rejects the other.""" config = load_config(dict(BASE_ENV) | {"GROUP_IDS": INTERNAL}) assert config.group_ids == (PREFIXED,) - assert config.resolve_group(INTERNAL) == PREFIXED + assert config.allows(PREFIXED) def test_canonical_group_id_is_idempotent() -> None: @@ -228,7 +237,6 @@ def test_canonical_group_id_is_idempotent() -> None: assert canonical_group_id(canonical_group_id(INTERNAL)) == PREFIXED -def test_resolve_group_rejects_direct_messages_and_strangers() -> None: - config = load_config(dict(BASE_ENV)) - assert config.resolve_group(None) is None - assert config.resolve_group("group.stranger==") is None +def test_a_pinned_bot_refuses_a_stranger_group() -> None: + config = load_config(dict(BASE_ENV) | {"GROUP_IDS": PREFIXED}) + assert not config.allows("group.stranger==") diff --git a/tests/test_counting.py b/tests/test_counting.py index a3ccafa..a7ddd70 100644 --- a/tests/test_counting.py +++ b/tests/test_counting.py @@ -12,7 +12,7 @@ from signal_bot.models import parse_envelope from tests.conftest import OTHER_GROUP, SECOND_GROUP, WATCHED_GROUP, envelope if TYPE_CHECKING: - from signal_bot.config import Config + from signal_bot.discovery import GroupDirectory def _message(**kwargs: object) -> object: @@ -22,18 +22,20 @@ def _message(**kwargs: object) -> object: @pytest.mark.parametrize("group", [WATCHED_GROUP, SECOND_GROUP]) -def test_every_watched_group_is_accepted_as_itself(config: Config, group: str) -> None: +async def test_every_watched_group_is_accepted_as_itself( + directory: GroupDirectory, group: str +) -> None: """Each watched group resolves to itself, never to the first one listed.""" - assert watched_group(_message(group_id=group), config) == group + assert watched_group(_message(group_id=group), directory) == group -def test_other_group_is_rejected(config: Config) -> None: - assert watched_group(_message(group_id=OTHER_GROUP), config) is None +async def test_other_group_is_rejected(directory: GroupDirectory) -> None: + assert watched_group(_message(group_id=OTHER_GROUP), directory) is None -def test_direct_message_is_rejected(config: Config) -> None: - """The dangerous case: a linked device receives every DM on the account.""" - assert watched_group(_message(group_id=None), config) is None +async def test_direct_message_is_rejected(directory: GroupDirectory) -> None: + """The dangerous case: a DM must never reach a handler.""" + assert watched_group(_message(group_id=None), directory) is None def test_plain_text_counts() -> None: diff --git a/tests/test_discovery.py b/tests/test_discovery.py new file mode 100644 index 0000000..ee642ae --- /dev/null +++ b/tests/test_discovery.py @@ -0,0 +1,206 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""The live group set: discovery, the allowlist, and never narrowing on failure.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +import httpx +import pytest + +from signal_bot.discovery import GroupDirectory +from tests.conftest import ( + OTHER_GROUP, + SECOND_GROUP, + WATCHED_GROUP, + WATCHED_INTERNAL, + group_entry, + listing_client, +) + +if TYPE_CHECKING: + from signal_bot.config import Config + from signal_bot.store import Store + +Listing = list[dict[str, Any]] + + +def _directory(config: Config, store: Store, handler: object) -> GroupDirectory: + return GroupDirectory(config, httpx.AsyncClient(transport=httpx.MockTransport(handler)), store) + + +async def test_discovers_every_group_the_account_is_in(config: Config, store: Store) -> None: + """No GROUP_IDS at all: whatever the account is a member of is watched.""" + found = GroupDirectory( + config, listing_client([group_entry(WATCHED_GROUP), group_entry(SECOND_GROUP)]), store + ) + await found.refresh() + assert found.groups == (WATCHED_GROUP, SECOND_GROUP) + assert found.resolve(WATCHED_INTERNAL) == WATCHED_GROUP + assert found.resolve(OTHER_GROUP) is None + assert found.resolve(None) is None + + +async def test_a_group_that_is_only_an_invite_is_not_watched(config: Config, store: Store) -> None: + """Joining is attempted, but the listing decides whether it worked.""" + found = _directory( + config, + store, + lambda r: httpx.Response(200, json=[group_entry(WATCHED_GROUP, member=False)]), + ) + await found.refresh() + assert found.groups == () + + +async def test_a_pending_invitation_is_joined(config: Config, store: Store) -> None: + # Two GETs per refresh when a join is attempted: read, join, re-read. + # The re-read is the authority -- the join endpoint answers 201 either way. + listings = iter( + [ + [group_entry(WATCHED_GROUP, member=False)], + [group_entry(WATCHED_GROUP)], + ] + ) + + def handler(request: httpx.Request) -> httpx.Response: + if request.method == "POST": + return httpx.Response(201, text="") + return httpx.Response(200, json=next(listings)) + + found = _directory(config, store, handler) + await found.refresh() + assert found.groups == (WATCHED_GROUP,) + + +async def test_an_impossible_invitation_is_not_retried_forever( + config: Config, store: Store +) -> None: + """signal-cli cannot accept an invite addressed to a phone number.""" + posts: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + if request.method == "POST": + posts.append(str(request.url)) + return httpx.Response(400, text="Cannot find service ID for self") + return httpx.Response(200, json=[group_entry(WATCHED_GROUP, member=False)]) + + found = _directory(config, store, handler) + await found.refresh() + await found.refresh() + await found.refresh() + assert len(posts) == 1 + + +async def test_a_failed_listing_never_narrows_the_watched_set(config: Config, store: Store) -> None: + """The worst bug available: conclude you were removed from everything.""" + responses = iter( + [ + httpx.Response(200, json=[group_entry(WATCHED_GROUP)]), + httpx.Response(500, text="upstream is down"), + ] + ) + found = _directory(config, store, lambda r: next(responses)) + await found.refresh() + assert found.groups == (WATCHED_GROUP,) + await found.refresh() + assert found.groups == (WATCHED_GROUP,) + assert found.resolve(WATCHED_INTERNAL) == WATCHED_GROUP + + +async def test_a_transport_error_never_narrows_the_watched_set( + config: Config, store: Store +) -> None: + calls = {"n": 0} + + def handler(request: httpx.Request) -> httpx.Response: + calls["n"] += 1 + if calls["n"] == 1: + return httpx.Response(200, json=[group_entry(WATCHED_GROUP)]) + msg = "signal-cli went away" + raise httpx.ConnectError(msg) + + found = _directory(config, store, handler) + await found.refresh() + await found.refresh() + assert found.groups == (WATCHED_GROUP,) + + +@pytest.mark.parametrize("body", [{"error": "nope"}, "a string"]) +async def test_a_listing_that_is_not_a_list_is_ignored( + config: Config, store: Store, body: object +) -> None: + responses = iter( + [httpx.Response(200, json=[group_entry(WATCHED_GROUP)]), httpx.Response(200, json=body)] + ) + found = _directory(config, store, lambda r: next(responses)) + await found.refresh() + await found.refresh() + assert found.groups == (WATCHED_GROUP,) + + +async def test_rows_that_are_not_objects_are_skipped(config: Config, store: Store) -> None: + found = _directory( + config, store, lambda r: httpx.Response(200, json=["nonsense", group_entry(WATCHED_GROUP)]) + ) + await found.refresh() + assert found.groups == (WATCHED_GROUP,) + + +@pytest.mark.parametrize("entry", [{"member": True}, {"id": "", "member": True}, {"id": 7}]) +async def test_a_row_without_a_usable_id_is_skipped( + config: Config, store: Store, entry: dict[str, Any] +) -> None: + found = _directory(config, store, lambda r: httpx.Response(200, json=[entry])) + await found.refresh() + assert found.groups == () + + +async def test_a_removed_group_is_dropped_and_its_counts_purged( + config: Config, store: Store +) -> None: + """A re-add should start a fresh tally, not resurrect an old one.""" + responses = iter( + [ + httpx.Response(200, json=[group_entry(WATCHED_GROUP), group_entry(SECOND_GROUP)]), + httpx.Response(200, json=[group_entry(SECOND_GROUP)]), + ] + ) + found = _directory(config, store, lambda r: next(responses)) + await found.refresh() + store.increment(WATCHED_GROUP, "u1", "Alice") + store.increment(SECOND_GROUP, "u2", "Bob") + + await found.refresh() + + assert found.groups == (SECOND_GROUP,) + assert found.resolve(WATCHED_INTERNAL) is None + assert store.tallies(WATCHED_GROUP) == [] + assert store.tallies(SECOND_GROUP)[0].count == 1 + + +async def test_refresh_for_looks_again_once_per_unknown_group(config: Config, store: Store) -> None: + """The first command after an add must work, without hammering the API.""" + listings: list[Any] = [ + [], + [group_entry(WATCHED_GROUP)], + [group_entry(WATCHED_GROUP)], + ] + calls = {"n": 0} + + def handler(request: httpx.Request) -> httpx.Response: + index = min(calls["n"], len(listings) - 1) + calls["n"] += 1 + return httpx.Response(200, json=listings[index]) + + found = _directory(config, store, handler) + await found.refresh() + assert found.groups == () + + assert await found.refresh_for(WATCHED_INTERNAL) == WATCHED_GROUP + before = calls["n"] + # A second unknown id refreshes; the same one again does not. + assert await found.refresh_for(OTHER_GROUP) is None + assert calls["n"] > before + after = calls["n"] + assert await found.refresh_for(OTHER_GROUP) is None + assert calls["n"] == after diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py index 3f315a2..205b344 100644 --- a/tests/test_dispatcher.py +++ b/tests/test_dispatcher.py @@ -14,6 +14,7 @@ from tests.conftest import OTHER_GROUP, SECOND_GROUP, WATCHED_GROUP, envelope if TYPE_CHECKING: from signal_bot.config import Config + from signal_bot.discovery import GroupDirectory from signal_bot.store import Store from signal_bot.transport_fake import FakeTransport @@ -37,10 +38,15 @@ def registry(runs: list[str]) -> CommandRegistry: @pytest.fixture def dispatcher( - config: Config, registry: CommandRegistry, store: Store, transport: FakeTransport + config: Config, + directory: GroupDirectory, + registry: CommandRegistry, + store: Store, + transport: FakeTransport, ) -> Dispatcher: return Dispatcher( config=config, + directory=directory, registry=registry, store=store, transport=transport, @@ -101,7 +107,7 @@ async def test_non_command_text_runs_nothing(dispatcher: Dispatcher, runs: list[ async def test_a_failing_command_does_not_kill_the_loop( - config: Config, store: Store, transport: FakeTransport + config: Config, directory: GroupDirectory, store: Store, transport: FakeTransport ) -> None: async def boom(ctx: CommandContext) -> None: msg = "upstream is down" @@ -111,6 +117,7 @@ async def test_a_failing_command_does_not_kill_the_loop( registry.register("cat", ["!cat"], boom) dispatcher = Dispatcher( config=config, + directory=directory, registry=registry, store=store, transport=transport, @@ -127,7 +134,7 @@ async def test_command_message_is_also_counted(dispatcher: Dispatcher, store: St async def test_a_reply_goes_back_to_the_group_that_asked( - config: Config, store: Store, transport: FakeTransport + config: Config, directory: GroupDirectory, store: Store, transport: FakeTransport ) -> None: """The multi-group invariant: never answer in the other watched group.""" @@ -137,7 +144,12 @@ async def test_a_reply_goes_back_to_the_group_that_asked( registry = CommandRegistry() registry.register("cat", ["!cat"], answer) dispatcher = Dispatcher( - config=config, registry=registry, store=store, transport=transport, clock=lambda: NOW + config=config, + directory=directory, + registry=registry, + store=store, + transport=transport, + clock=lambda: NOW, ) await dispatcher.handle(envelope(text="!cat", group_id=SECOND_GROUP)) assert [m.recipient for m in transport.sent] == [SECOND_GROUP] diff --git a/tests/test_fake_groups.py b/tests/test_fake_groups.py new file mode 100644 index 0000000..495d932 --- /dev/null +++ b/tests/test_fake_groups.py @@ -0,0 +1,68 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""The fake server's group endpoints, including the join the bot self-heals with.""" + +from __future__ import annotations + +import pytest +from fastapi.testclient import TestClient + +from fake_signal.server import create_app +from fake_signal.state import State + +NUMBER = "+15550001234" +GROUP = "group.first==" +INTERNAL = "first==" + + +@pytest.fixture +def state() -> State: + return State(base_url="http://testserver", accounts=[NUMBER]) + + +@pytest.fixture +def client(state: State) -> TestClient: + return TestClient(create_app(state)) + + +def _entry(group_id: str, *, member: bool = True, joinable: bool = True) -> dict[str, object]: + return { + "id": group_id, + "internal_id": INTERNAL, + "name": "g", + "member": member, + "joinable": joinable, + } + + +def test_groups_require_a_registered_account(client: TestClient) -> None: + assert client.get("/v1/groups/+15559999999").status_code == 400 + + +def test_adding_groups_keeps_the_existing_ones(client: TestClient) -> None: + """Replacing instead of adding would delete state a later scenario needs.""" + client.post("/_control/groups", json=[_entry(GROUP)]) + client.post("/_control/groups/add", json=[_entry("group.second==")]) + listed = client.get(f"/v1/groups/{NUMBER}").json() + assert [g["id"] for g in listed] == [GROUP, "group.second=="] + + +def test_adding_a_group_twice_does_not_duplicate_it(client: TestClient) -> None: + client.post("/_control/groups", json=[_entry(GROUP)]) + assert client.post("/_control/groups/add", json=[_entry(GROUP)]).json() == {"groups": 1} + + +def test_join_requires_a_registered_account(client: TestClient) -> None: + assert client.post(f"/v1/groups/+15559999999/{GROUP}/join").status_code == 400 + + +def test_join_turns_an_invitation_into_membership(client: TestClient) -> None: + client.post("/_control/groups", json=[_entry(GROUP, member=False)]) + assert client.post(f"/v1/groups/{NUMBER}/{GROUP}/join").status_code == 201 + assert client.get(f"/v1/groups/{NUMBER}").json()[0]["member"] is True + + +def test_join_answers_201_even_when_it_changed_nothing(client: TestClient) -> None: + """The real endpoint does too, which is why the bot re-reads the listing.""" + client.post("/_control/groups", json=[_entry(GROUP, member=False, joinable=False)]) + assert client.post(f"/v1/groups/{NUMBER}/{GROUP}/join").status_code == 201 + assert client.get(f"/v1/groups/{NUMBER}").json()[0]["member"] is False diff --git a/tests/test_membership.py b/tests/test_membership.py index 371ec2d..5095b2f 100644 --- a/tests/test_membership.py +++ b/tests/test_membership.py @@ -1,155 +1,91 @@ # Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. -"""The startup membership gate: invited is not joined, and each group counts.""" +"""Invited is not joined: the join attempt and its memory of failures.""" from __future__ import annotations -from collections.abc import Callable -from dataclasses import replace from typing import TYPE_CHECKING from urllib.parse import quote import httpx import pytest -from signal_bot.transport_rest import RestTransport, TransportError +from signal_bot.membership import ( + JoinAttempts, + attempt_join, + entry_matches, + find_entry, +) +from tests.conftest import SECOND_GROUP, WATCHED_GROUP, WATCHED_INTERNAL, group_entry if TYPE_CHECKING: from signal_bot.config import Config -# The callable httpx.MockTransport expects -- not a bot command handler. -MockHandler = Callable[[httpx.Request], httpx.Response] + +def _client(handler: object) -> httpx.AsyncClient: + return httpx.AsyncClient(transport=httpx.MockTransport(handler)) -def _transport(config: Config, handler: MockHandler) -> RestTransport: - """A transport whose HTTP calls are answered by handler.""" - return RestTransport(config, httpx.AsyncClient(transport=httpx.MockTransport(handler))) +def test_entry_matches_either_spelling() -> None: + """Envelopes carry internal_id; the listing reports the prefixed form.""" + assert entry_matches({"internal_id": WATCHED_INTERNAL}, WATCHED_GROUP) + assert entry_matches({"id": WATCHED_GROUP}, WATCHED_GROUP) + assert not entry_matches({"id": SECOND_GROUP}, WATCHED_GROUP) -def _groups(*entries: object) -> MockHandler: - """Answer /v1/groups with an arbitrary payload.""" - return lambda r: httpx.Response(200, json=list(entries)) +def test_find_entry_returns_none_when_absent() -> None: + assert find_entry([group_entry(SECOND_GROUP)], WATCHED_GROUP) is None + assert find_entry([group_entry(WATCHED_GROUP)], WATCHED_GROUP) is not None -def _member(group_id: str, *, member: bool = True, name: str = "testbot") -> dict[str, object]: - """One /v1/groups row for a configured group.""" - return {"id": group_id, "internal_id": group_id, "name": name, "member": member} +def test_join_attempts_remembers_and_forgets() -> None: + """Without this, an impossible invite is retried every 30s forever.""" + attempts = JoinAttempts() + assert not attempts.has_failed(WATCHED_GROUP) + assert attempts.reason(WATCHED_GROUP) == "not attempted" + attempts.record_failure(WATCHED_GROUP, "no service ID") + assert attempts.has_failed(WATCHED_GROUP) + assert attempts.reason(WATCHED_GROUP) == "no service ID" + attempts.forget(WATCHED_GROUP) + assert not attempts.has_failed(WATCHED_GROUP) -def _all_joined(config: Config) -> MockHandler: - """Answer /v1/groups with every configured group, fully joined.""" - return _groups(*(_member(group_id) for group_id in config.group_ids)) +async def test_attempt_join_percent_encodes_the_group_id(config: Config) -> None: + """The internal spelling carries '/' and '+', which are not path-safe.""" + group_id = "Y29uZmlnLWdyb3VwLWlkLTMy" + seen: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append(request.url.raw_path.decode()) + return httpx.Response(201, text="") + + detail = await attempt_join(_client(handler), config, group_id) + assert seen == [f"/v1/groups/{config.phone_number}/{quote(group_id, safe='')}/join"] + assert "201" in detail -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_ids[0], "member": True}, - _member(config.group_ids[1]), +async def test_attempt_join_reports_a_refusal_instead_of_raising(config: Config) -> None: + """The caller's membership re-check is the authority, not this response.""" + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(400, text="Cannot find service ID for self") + + detail = await attempt_join(_client(handler), config, WATCHED_GROUP) + assert "400" in detail + assert "Cannot find service ID" in detail + + +async def test_attempt_join_survives_an_endpoint_that_is_not_there(config: Config) -> None: + def handler(request: httpx.Request) -> httpx.Response: + msg = "connection refused" + raise httpx.ConnectError(msg) + + assert "join request failed" in await attempt_join(_client(handler), config, WATCHED_GROUP) + + +@pytest.mark.parametrize("body", ["", "x" * 400]) +async def test_attempt_join_truncates_a_long_body(config: Config, body: str) -> None: + """A whole HTML error page in a log line helps nobody.""" + detail = await attempt_join( + _client(lambda r: httpx.Response(500, text=body)), config, WATCHED_GROUP ) - assert await _transport(config, handler).verify_membership() is None - - -async def test_membership_checks_every_configured_group(config: Config) -> None: - """One joined group must not vouch for another: that is a silent deafness.""" - handler = _groups(_member(config.group_ids[0])) - with pytest.raises(TransportError, match="in no group matching"): - await _transport(config, handler).verify_membership() - - -async def test_membership_names_the_group_it_is_only_invited_to( - config: Config, -) -> None: - """The join self-heal fails too, so the gate must still refuse loudly.""" - joined, invited = config.group_ids - - def handler(request: httpx.Request) -> httpx.Response: - if request.method == "POST": - return httpx.Response(400, text="Cannot find service ID for self") - return httpx.Response( - 200, json=[_member(joined), _member(invited, member=False, name="prod")] - ) - - with pytest.raises(TransportError, match="invited to 'prod'") as raised: - await _transport(config, handler).verify_membership() - # The join attempt's own answer travels into the error, so the reason is - # in the journal rather than one debugging session away. - assert "Cannot find service ID" in str(raised.value) - assert invited in str(raised.value) - - -async def test_membership_joins_a_pending_invitation(config: Config) -> None: - """The self-heal: accept the invite at startup instead of demanding a re-add.""" - joined = config.group_ids[0] - # A real internal id, slashes and pluses included: unencoded, they would - # not match the join route at all, and the self-heal would never fire. - invited = "Y29uZmlnLWdyb3VwLWlkLTMy" - config = replace(config, group_ids=(joined, invited)) - posts: list[str] = [] - listings = iter( - [ - [_member(joined), _member(invited, member=False)], - [_member(joined), _member(invited)], - ] - ) - - def handler(request: httpx.Request) -> httpx.Response: - if request.method == "POST": - posts.append(request.url.raw_path.decode()) - return httpx.Response(201, text="") - return httpx.Response(200, json=next(listings)) - - assert await _transport(config, handler).verify_membership() is None - # The id is percent-encoded: the internal spelling carries '/' and '+', - # which would otherwise not match the route at all. - expected = f"/v1/groups/{config.phone_number}/{quote(invited, safe='')}/join" - assert posts == [expected] - - -async def test_membership_survives_a_join_endpoint_that_is_not_there( - config: Config, -) -> None: - """A transport error on join must become the loud refusal, not a crash.""" - joined, invited = config.group_ids - - def handler(request: httpx.Request) -> httpx.Response: - if request.method == "POST": - msg = "connection refused" - raise httpx.ConnectError(msg) - return httpx.Response(200, json=[_member(joined), _member(invited, member=False)]) - - with pytest.raises(TransportError, match="join request failed"): - await _transport(config, handler).verify_membership() - - -async def test_membership_refuses_when_the_group_vanishes_after_joining( - config: Config, -) -> None: - """The re-check is the authority, so a group gone from the listing fails.""" - joined, invited = config.group_ids - listings = iter([[_member(joined), _member(invited, member=False)], [_member(joined)]]) - - def handler(request: httpx.Request) -> httpx.Response: - if request.method == "POST": - return httpx.Response(201, text="") - return httpx.Response(200, json=next(listings)) - - 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", *(_member(g) for g in config.group_ids)) - assert await _transport(config, handler).verify_membership() is None + assert len(detail) < 260 diff --git a/tests/test_scenario.py b/tests/test_scenario.py index 724595c..9621c1e 100644 --- a/tests/test_scenario.py +++ b/tests/test_scenario.py @@ -10,6 +10,7 @@ import pytest from fake_signal.scenario import ( Expectation, + GroupArrival, Injection, Scenario, ScenarioError, @@ -130,3 +131,30 @@ def test_unexpected_sends_are_a_failure() -> None: def test_frame_is_json() -> None: assert json.loads(Injection().to_frame(GROUPS))["account"] == "+15550000000" + + +def test_group_arrival_renders_a_listing_row() -> None: + """A scenario names aliases; the fake needs both real spellings.""" + entry = GroupArrival(id="reply", internal_id="watched", name="added").to_entry(GROUPS) + assert entry == { + "id": GROUPS["reply"], + "internal_id": GROUPS["watched"], + "name": "added", + "member": True, + "joinable": True, + } + + +@pytest.mark.parametrize("field", ["id", "internal_id"]) +def test_group_arrival_rejects_an_unknown_alias(field: str) -> None: + arrival = GroupArrival(**{"id": "reply", "internal_id": "watched", field: "nope"}) + with pytest.raises(ScenarioError, match="unknown group alias"): + arrival.to_entry(GROUPS) + + +def test_arrivals_are_parsed_from_a_file(tmp_path: Path) -> None: + path = tmp_path / "s.yaml" + path.write_text( + "name: demo\narrive:\n - {id: reply, internal_id: watched}\n", encoding="utf-8" + ) + assert load_scenario(path).arrive[0].internal_id == "watched"