mirror of
https://github.com/kuhyx/signal-bot.git
synced 2026-11-11 14:37:32 +01:00
Krzysztof kuhy Rudnicki
405541be9b
One row per command attempt, not two aggregates written only on success. command_uses holds last-use only and daily_uses counts successes; both are rate-limiter state and both are written by store.record_use, which the dispatcher calls only when a handler returns cleanly. A gate refusal and a crash returned before it, so a command that failed every time looked exactly like one nobody ran, and nothing knew which command a counted message was. command_events now takes every attempt with its outcome, its duration and the item it served; upstream_calls takes every outbound request made while serving one. The two writes stay separate on purpose -- recording a refusal as a use is what slid the cooldown window forward every time somebody was told to wait -- and test_dispatcher_outcomes asserts last_used does not move. Backfilled from journald joined against message_events, with daily_uses filling the residual so the two sources cannot double-count. Two things were wrong on the first attempt and both were found by measuring rather than by reasoning: the window has to be symmetric, because sent_at_ms comes from the sender's phone and the journal stamp from this host (194 of 299 lines landed *before* their own message, by up to 3.2 s); and matching has to be a global closest-pair assignment, because per-row greedy cannot separate two commands seconds apart. Together those took the dropped rows from 137 to zero, and recovered 418 invocations back to 29 August. Also: !analityka in the group, twelve Polish lines, naming at most three people and only ones whose display name has been seen; an Analytics tab with Nivo charts; and continuous latency measurement, which answers the open question in DOCS-latency.md -- a text send is 235 ms and an image send about 2.2 s, so the upload really is where the time goes. EPHEMERAL_TABLES was doing two unrelated jobs and is now two tuples: what a schema bump rebuilds, and what leaving a group clears. daily_uses left the first (it is a backfill source now) and stayed in the second, and _drop_reshaped took over the repair a bump used to do for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T4QTp3h5Q3MvfrwGV8xqPf
215 lines
11 KiB
Markdown
215 lines
11 KiB
Markdown
# The management page
|
|
|
|
`./web.sh` serves one page on `127.0.0.1:8140`: whether the bot is alive, what
|
|
it is configured with, and the photo album's tagger. It replaces
|
|
`tool/tagger.py`, which served the album half of this on the same port.
|
|
|
|
```sh
|
|
./web.sh # the configured pack
|
|
./web.sh --dir ~/pack --port 8140
|
|
./web.sh --import ~/photos # dedup, strip and import, then serve
|
|
./web.sh --import ~/photos --dry-run # the report, changing nothing
|
|
./web.sh --untagged # only items with no tags yet
|
|
```
|
|
|
|
## Why it is started by hand
|
|
|
|
A systemd unit would be more convenient and is the wrong shape. This page can
|
|
rewrite `.env` — the file that decides which groups the bot answers in and who
|
|
may add it to one — and a unit would keep that writable on loopback all day
|
|
for the sake of a bookmark. Started by hand, the window is the few minutes
|
|
somebody is actually looking at it.
|
|
|
|
It is also a **second process**, not more routes on the bot. The bot's own
|
|
port already serves `POST /_selftest`, which posts for real into a real group;
|
|
putting a configuration writer beside it would put both behind one mistake.
|
|
|
|
## Why loopback is not the whole defence
|
|
|
|
Binding to `127.0.0.1` stops the network. It does not stop the browser already
|
|
running on this machine, and two attacks reach a loopback port from an
|
|
ordinary web page. `signal_bot/manage/guard.py` closes both, in about forty
|
|
lines, with nothing to configure and no session to keep:
|
|
|
|
- **Cross-origin form posts.** Any site can POST a `<form>` at
|
|
`127.0.0.1:8140` without reading the reply. A form can only send
|
|
`application/x-www-form-urlencoded`, `multipart/form-data` or `text/plain`,
|
|
so every mutating request must declare `application/json` — which a form
|
|
cannot. Anything else is **415**.
|
|
- **DNS rebinding.** A hostname the attacker controls is re-pointed at
|
|
`127.0.0.1` after the page loads, which makes the browser treat this app as
|
|
same-origin. The rebound request still carries the attacker's hostname, so
|
|
a `Host` that is not loopback is **421**.
|
|
|
|
Neither is authentication, and neither pretends to be. They are the two
|
|
cheapest things that make "only this machine" mean it.
|
|
|
|
## Editing .env
|
|
|
|
Three refusals, in this order, and the order is the point.
|
|
|
|
1. **A key that may not be written is refused before anything is parsed**, so
|
|
a read-only key cannot be smuggled past by making the rest of the file
|
|
invalid. `403`, naming the key.
|
|
2. **The whole candidate file is validated by the function startup uses** —
|
|
`load_config(candidate, use_dotenv=False)`. An edit this bot would refuse
|
|
at boot is refused at the form instead, with the journal's own sentence.
|
|
`400`.
|
|
3. **Only then is anything written.** A rejected edit leaves `.env`
|
|
byte-identical.
|
|
|
|
The write itself is line-by-line: only the `KEY=` line of a key that actually
|
|
changed is replaced, so the paragraphs explaining each setting survive an edit
|
|
made in a browser. A key the file never had is appended under a marker rather
|
|
than dropped among comments that explain something else. Temp-then-replace
|
|
with mode `0600` reapplied before the swap, because this file holds the
|
|
account's number and is edited from a tab that can be closed mid-save.
|
|
|
|
### What is read-only, and why
|
|
|
|
| Key | Why the page will not write it |
|
|
|---|---|
|
|
| `PHONE_NUMBER` | Identifies the account; the container holds the matching registration. |
|
|
| `API_BASE_URL` | Repointing it sends every outgoing message and attachment fetch elsewhere at the next restart. Strictly worse than changing the number. |
|
|
| `DATABASE_PATH` | Repointing it silently orphans the whole statistical history. |
|
|
| `PHOTO_PACK_DIR` | **Cannot be validated.** `_photo_pack_dirs()` reads `os.environ` directly rather than the mapping it is handed — deliberately, since that is what keeps the test suite away from a real album. |
|
|
| `BIND_HOST` / `BIND_PORT` | Moving the bot's port from a page that proxies to it. |
|
|
|
|
### Group settings need a group listing
|
|
|
|
`GROUP_IDS` and `SELFTEST_GROUPS` are editable, but only while `/api/groups`
|
|
has actually returned something. `parse_group_ids` checks *syntax*; startup
|
|
additionally requires each listed group to be one the bot really joined,
|
|
judged against a listing that was really read. So with the container down, a
|
|
syntactically valid write can name a group the next startup rejects — turning
|
|
a scoping mistake into a unit that will not start. The page says so and leaves
|
|
the other keys editable.
|
|
|
|
Group ids are **picked, never typed** — `GROUP_IDS` and `SELFTEST_GROUPS`
|
|
render as a `<select>` filled from `/api/groups`, for the same reason the
|
|
tagger picked them: an id is 44 characters of base64.
|
|
|
|
> `SELFTEST_GROUPS` is editable and worth understanding before you widen it.
|
|
> It is what keeps `POST /_selftest` fail-closed; every group named in it is a
|
|
> group the bot can be made to post arbitrary text into.
|
|
|
|
## The album, in two halves
|
|
|
|
Which half you get is the album's own `configured` flag, exactly as the
|
|
tagger decided it.
|
|
|
|
- **No `people.json` yet** → the roster form. One person per line, aliases
|
|
separated by commas; a line with an `=` is a combination, `+` for "both of
|
|
them" and `|` for "either of them". Keys are reused for anyone already on
|
|
the roster and otherwise handed out one past the highest in use — never the
|
|
count, because a key deleted by hand must not be reissued and inherit
|
|
somebody else's photos. That mirrors `pack_people.next_key`, which is the
|
|
authority. Names are typed into the page and never passed as arguments: an
|
|
argument lands in the shell history.
|
|
- **Configured** → the tagger: the item, a checkbox per person, saved on the
|
|
click rather than on a "done" button, and an "add somebody" box for the
|
|
usual case of noticing a new face in the photo already on screen.
|
|
|
|
`--untagged` is a *view filter*, never a shorter list — `/api/album/media` and
|
|
the tag route both index the full album, so filtering server-side would show
|
|
item *i* of the album while tagging item *i* of the filtered set. Anything
|
|
seen this session stays navigable even once it has tags, or tagging the photo
|
|
you are looking at would remove it from the list you are walking.
|
|
|
|
## Analytics
|
|
|
|
The first tab: what the group actually asks the bot for. Read-only, and the
|
|
only thing on this page that is worth nothing to an attacker and everything
|
|
to a maintainer -- so there is deliberately no write route under it.
|
|
|
|
`?group=` is optional on every route. Absent means the all-groups roll-up,
|
|
and it runs the *same SQL* rather than a second query that would drift.
|
|
|
|
Every panel carries the window it covers and the three source counts, because
|
|
two of the three classes of row were reconstructed rather than observed.
|
|
`DOCS-analytics.md` says what each class knows and what it does not.
|
|
|
|
Charts are Nivo, always the fixed-size components: the `Responsive*` ones
|
|
measure their container, and a jsdom container has none, so the whole test
|
|
suite would assert against empty divs.
|
|
|
|
## Ideas
|
|
|
|
The fourth tab lists what the group has asked for: `!zaproponuj` notes with
|
|
the text people wrote, and every unrecognised `!word` with a count and the
|
|
messages it came in.
|
|
|
|
The **Export** button writes that to a file, and the file goes to the state
|
|
directory — `$STATE_DIRECTORY`, else `$XDG_DATA_HOME/signal-bot/` — never
|
|
into this repository. Two reasons, and both are about a public git tree that
|
|
has leaked real names once. A file the group can write into would wedge
|
|
`check_personal_data` the moment somebody's name landed in it, refusing every
|
|
future commit until a human edited it by hand; and the gate learns the names
|
|
it refuses from the live database, so it catches `anna` and cannot possibly
|
|
catch `!annna`, which is exactly what an unrecognised-command corpus is full
|
|
of. The export therefore carries deliberate notes in full and unrecognised
|
|
words as counts only. Copy what is worth keeping into the tree yourself.
|
|
|
|
## Routes
|
|
|
|
Everything under `/api`. The page itself is served from `web/dist` at every
|
|
other path, so a reload on a sub-view works.
|
|
|
|
| Method | Path | What it does |
|
|
|---|---|---|
|
|
| GET | `/api/status` | Proxies the bot's `/healthz`. Unreachable is a state, never an error. |
|
|
| GET | `/api/groups` | The live listing, or `[]` when the container is down. |
|
|
| GET | `/api/config` | The resolved config. The number is cut to its last four digits. |
|
|
| GET / PUT | `/api/env` | Read and write `.env`, per the rules above. |
|
|
| POST | `/api/selftest` | Proxied to the bot verbatim, including its 403. |
|
|
| POST | `/api/restart-container` | `docker restart`, through `signal_bot/external.py`. |
|
|
| GET | `/api/album/state` | The album, the roster and the groups to pick from. |
|
|
| POST | `/api/album/tag` | Record who is in one file. |
|
|
| GET | `/api/album/summary` | What each command would answer with. |
|
|
| POST | `/api/album/setup` | Write the roster, then prove the bot could load it. |
|
|
| POST | `/api/album/person` | Add one person mid-session. |
|
|
| GET | `/api/album/media/{index}` | One item, **by position**. |
|
|
| GET | `/api/suggestions` | Filed ideas and unrecognised-word tallies, bodies included. |
|
|
| GET | `/api/analytics/groups` | Every group with a command history, for the picker. |
|
|
| GET | `/api/analytics/{overview,time,health,demand,behaviour,content}` | One per panel, all read-only, all taking an optional `?group=`. |
|
|
| POST | `/api/suggestions/export` | Write those to a file **outside the repository**. |
|
|
|
|
That last one is not a style choice. A filename supplied by a client is a
|
|
path, and this directory holds private photographs; the route takes an `int`,
|
|
so FastAPI refuses a name with **422** before any handler code runs.
|
|
|
|
## Building it
|
|
|
|
`dist/` is a build output and this repository does not commit those.
|
|
`scripts/install_service.sh` builds it, and verifies the artefact rather than
|
|
trusting the exit code. Until it has been built the page answers **503** with
|
|
the command to run, because `StaticFiles` raises at construction on a missing
|
|
directory — mounting it unconditionally would take the app down on a fresh
|
|
clone, and in CI, where pytest never runs `pnpm build`.
|
|
|
|
```sh
|
|
pnpm --dir web install --frozen-lockfile
|
|
pnpm --dir web build
|
|
pnpm --dir web lint # tsc --noEmit + eslint strictTypeChecked
|
|
pnpm --dir web coverage # vitest, 100% with no exclude list
|
|
```
|
|
|
|
React 19, Vite 8, TypeScript **6.0.3**, ESLint 10 with `strictTypeChecked` +
|
|
`stylisticTypeChecked` + unicorn + sonarjs + perfectionist, Vitest 4 at 100%
|
|
on all four metrics. TypeScript stays on 6.0.3 because
|
|
`typescript-eslint@8.68.0` declares `typescript: ">=4.8.4 <6.1.0"`; the pin is
|
|
fleet-wide in `~/utils/dependency-freshness.allowlist.yaml` and clears itself
|
|
when the linter catches up. The compiler waits for the linter.
|
|
|
|
`web/tsconfig.json` is strict JSON rather than the usual JSONC: this
|
|
repository's `check-json` hook is generic and tsconfig being commentable is a
|
|
convention, not a requirement. The two things the comments used to say: `lib`
|
|
is `ESNext` while `target` stays `ES2023`, so iterator helpers are declared
|
|
without being emitted for; and every strictness flag is on, including
|
|
`noUncheckedIndexedAccess` and `exactOptionalPropertyTypes`.
|
|
|
|
There is **no coverage exclude list**, including for `src/main.tsx`. A
|
|
coverage omit is a suppression, and "the DOM bootstrap is awkward to assert"
|
|
is the same sentence as "this code is untested" — so the bootstrap is a shim
|
|
thin enough to test in jsdom instead.
|