mirror of
https://github.com/kuhyx/signal-bot.git
synced 2026-11-11 16:34:08 +01:00
Krzysztof kuhy Rudnicki
8500a1add1
The first pass shipped the tagging checkboxes and nothing else, which was a regression dressed as an absorption: an album with no people.json has an empty roster, so the checkboxes rendered an empty list and there was no way to add anybody. The old page gated a setup form on `configured` for exactly that reason. So does this one now. Restored, behaviour-for-behaviour: - The roster grammar, in src/api/setup.ts. One person per line, aliases by comma, `=` for a combination with `+` meaning both and `|` meaning either. Keys are reused for anyone already listed and otherwise handed out one past the highest in use -- never the count, or a key deleted by hand gets reissued and inherits somebody else's photos. Mirrors pack_people.next_key. - The add-person box, for the usual case of noticing a new face in the photo already on screen. - `--untagged`, which was a no-op: it is a view filter, and anything seen this session stays navigable, or tagging the photo you are looking at removes it from the list you are walking. DOCS-web-ui.md claimed group ids were "picked, never typed" while settings rendered a plain text box. That was the mitigation cited for accepting an .env editor at all, so the claim is now backed by a <select> filled from /api/groups -- and a failed listing leaves it empty rather than unhandled. parseSetup returns a result instead of throwing, and readCombos has one return type: a try/catch in the view carried an "and if it was some other error" branch that could not happen, and an untestable branch is an untested one. Same reason the album view splits into roster.tsx and tagger.tsx. Still 100% on all four metrics with no exclude list, and verified against the real album: 185 items, 4 groups in the picker, guard still answering 421. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01416TegJ3sa8xL5qGGgrygP
177 lines
9.1 KiB
Markdown
177 lines
9.1 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.
|
|
|
|
## 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**. |
|
|
|
|
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.
|