Files
plm-lottery/BUGS.md
T

80 lines
3.9 KiB
Markdown
Raw Normal View History

# BUGS.md — audit of 2026-08-03
Third full-codebase audit, opened after the 2026-07-26 (B-01 … B-24) and
2026-07-27 (B-25 … B-49) lists were emptied. Numbering continues from the last
fixed finding, B-51.
The list opened at B-52 … B-72 and holds only what is still **open**: a finding is
removed from this file once it is fixed, and is not listed here afterwards. Per
CLAUDE.md's convention each entry gets its own commit with its own regression test,
and the `B-nn` marker goes in a comment next to the fix, so
`git log --all --grep 'B-nn'` is the record of how any closed finding was closed.
State of the tree at audit time: 264 unit tests, all passing; `tests/integration/`
still empty; withdrawal and the RBF bump path still never live-broadcast.
Verified as *not* broken while looking for these: i18n key parity (146 identical
keys across all 7 languages), HTML escaping of every user-controlled value in
`admin.js`, the Alembic chain (linear, single head, matching `models.py`),
strictly-integer satoshi arithmetic everywhere, and `.gitignore` coverage of
secrets/DB/logs (nothing sensitive is tracked in git).
Severity is about consequence, not likelihood:
**critical** = money stuck or lost, or the lottery stops;
**high** = a security control that does not hold;
**medium** = wrong behaviour with a bounded blast radius;
**low** = drift between documentation and code.
---
## Critical
### (not new) `drawing` does not resume after a restart
Already tracked as an accepted gap in CLAUDE.md's "Known gaps", not re-numbered
here. Worth restating in context: with `restart: unless-stopped` on the app
container, this is the one state that gets stuck with money in play, and it
remains the last prerequisite for running unattended.
---
## Low — documentation and consistency drift
### B-70 — the flowchart's WITHDRAW precondition is not implemented as written
`flowchart/platform-overview.mmd:39` (node E1) states a withdrawal cannot happen
together with a bet in progress. The code only serializes the *builds* through the
per-user lock (`app/tx/locks.py`): a withdrawal is accepted while a bet is still
unconfirmed, as long as confirmed UTXOs cover it.
CLAUDE.md declares every node and edge label of the diagrams a behaviour that must
be implemented as described, so one of the two has to move — most likely the
diagram, since the lock already prevents the actual double-spend hazard, but that
is a decision, not a cleanup.
### B-71 — `.env` points `MASTER_KEY_PATH` at a second copy of the master key
CLAUDE.md's deployment section prescribes pointing `MASTER_KEY_PATH` at the
host-side `./data/keys/master.xprv.enc` so the venv scripts and the container read
one file. `.env` instead sets `./master.xprv.enc`, and both files now exist in the
working tree (both gitignored). They were verified during this audit to decrypt to
the *same* xprv, so nothing has diverged yet — but `scripts/decrypt_master_key.py`
reads a different file from the one the container uses, and a future
`generate_master_key.py --overwrite` would split them silently, with an ops
recovery path that then reports the wrong key.
The same duplication exists for the database (`./plm_lottery.db` next to
`data/db/plm_lottery.db`), which is less dangerous but equally confusing.
Fix: set `MASTER_KEY_PATH=./data/keys/master.xprv.enc` in `.env`, delete the stray
root copy once confirmed redundant, and state the same for `DATABASE_URL`.
### B-72 — `docs/setup.md` still frames setup as "locally or via Docker"
`docs/setup.md:1-10` lists Python as a prerequisite "for the local/venv workflow"
and describes the master-key step as local *or* Docker, while B-44 made Docker the
only supported way to run the server (`docs/running-the-server.md` and README were
updated, this file was not). The venv genuinely is needed for tests, migrations and
the key scripts — the wording just needs to say that instead of implying a second
way to run the server.