Audit-log bug report status changes (B-60)

Every other admin mutation — config edit, pause/resume, privkey export, password
reset — leaves a trace; this one could silently mark a report resolved. With one
shared ADMIN_TOKEN and no per-admin identity, the audit log is the only
accountability there is.

bug_report_status_changed records the report id and the before/after status, and
carries the report's author as user_id so the entry is traceable from either
side. Nothing is written when the status doesn't actually change, matching
config_updated: an edit that changes nothing isn't an event, and noise hides the
real changes.

Also documents the new event in docs/guida-admin.md's audit table.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-03 22:59:00 +02:00
co-authored by Claude Opus 5
parent 6246b13247
commit 77e07e87dc
5 changed files with 70 additions and 16 deletions
-13
View File
@@ -38,19 +38,6 @@ remains the last prerequisite for running unattended.
---
## High — security
### B-60 — `POST /admin/bug-reports/{id}/status` writes no audit entry
`app/api/routes/admin.py:404-419`.
Every other admin mutation (config edit, pause/resume, privkey export, password
reset) is audit-logged. This one is not, so a report can be silently marked
`resolved` with no trace — and with one shared `ADMIN_TOKEN` and no per-admin
identity, the audit log is the only accountability there is.
---
## Medium — correctness and robustness
### B-61 — config edits apply retroactively to the round already in progress
+2 -2
View File
@@ -8,7 +8,7 @@ The user communicates in Italian in chat — reply to them in Italian. Everythin
## Project status
All 10 stages of the original build order are code-complete and unit-tested — 302 tests, all under `tests/unit/` (`tests/integration/` is an empty package). Beyond them: Docker + Caddy deployment, admin dashboard (`/admin`), static test UI (`/`), pending-inclusive balance display, an SSE push channel layered over the original polling, self-service password change + admin password reset, and the reconciliation/corroboration machinery below.
All 10 stages of the original build order are code-complete and unit-tested — 305 tests, all under `tests/unit/` (`tests/integration/` is an empty package). Beyond them: Docker + Caddy deployment, admin dashboard (`/admin`), static test UI (`/`), pending-inclusive balance display, an SSE push channel layered over the original polling, self-service password change + admin password reset, and the reconciliation/corroboration machinery below.
Verified on mainnet with real money: registration + address derivation, deposit crediting (1-conf), a real 10 PLM bet (broadcast → confirmed → change credited back), and one full round cycle (close → draw on a real block hash → 70/30 payout with sat math checked against the broadcast tx → confirmation → close → next round auto-opened). **Withdrawal and the RBF bump path have never been exercised against a live broadcast** — unit-tested only.
@@ -33,7 +33,7 @@ PYTHONPATH=. python scripts/decrypt_master_key.py # ops recovery: decrypt+pr
PYTHONPATH=. python scripts/encrypt_master_key.py # ops bootstrap: import an externally-generated xprv (--overwrite to replace)
PYTHONPATH=. python scripts/electrum_smoke_test.py # manual check: connect, handshake, subscribe to headers, print the tip
python -m pytest # all 302 tests
python -m pytest # all 305 tests
python -m pytest tests/unit/test_hd.py # one file
python -m pytest tests/unit/test_hd.py::test_derivation_is_deterministic # one test
```
+14 -1
View File
@@ -408,7 +408,20 @@ async def update_bug_report_status(
if report is None:
raise HTTPException(status.HTTP_404_NOT_FOUND, "bug report not found")
report.status = body.status
# B-60: every other admin mutation (config edit, pause/resume, privkey export,
# password reset) leaves a trace; this one silently marked a report `resolved`.
# With one shared ADMIN_TOKEN and no per-admin identity, the audit log is the
# only accountability there is. Before/after like config_updated, and nothing
# written when the status doesn't actually change — re-clicking the status a
# report already has isn't an event.
if report.status != body.status:
await write_audit_log(
session,
"bug_report_status_changed",
{"report_id": report_id, "from": report.status, "to": body.status},
user_id=report.user_id,
)
report.status = body.status
await session.commit()
username = None
+1
View File
@@ -116,6 +116,7 @@ Eventi a cui vale la pena prestare attenzione:
| `bet_broadcast_failed` / `withdrawal_broadcast_failed` | La rete ha rifiutato la transazione. Non è stato speso nulla: gli UTXO sono stati liberati e il saldo dell'utente è tornato come prima. |
| `pending_tx_abandoned` | Una transazione trasmessa è scomparsa dalla catena e il sistema l'ha dichiarata persa: UTXO liberati, bet rimossa o prelievo segnato `failed`. Se capita spesso, la fee rate configurata è probabilmente troppo bassa. |
| `pending_tx_recovered` | Una transazione che si credeva incompleta è invece finita in catena (tipicamente dopo un riavvio a metà invio) e il sistema l'ha ripresa da sé. |
| `bug_report_status_changed` | Un admin ha cambiato lo stato di una segnalazione (payload: `report_id`, stato precedente e nuovo). Con un token admin unico e condiviso, questa riga è l'unica traccia di chi tocca le segnalazioni. |
| `payout_failed` | Il payout di un round non è partito. Il round resta in `paying_out` e **richiede intervento manuale**: non esiste un retry automatico. Controlla `fee_address`, il saldo dell'indirizzo pool e la connessione Electrum. |
## Alternative all'interfaccia grafica
+53
View File
@@ -179,3 +179,56 @@ async def test_update_status_unknown_report_is_404(client):
"/admin/bug-reports/999/status", headers=_ADMIN_HEADERS, json={"status": "read"}
)
assert resp.status_code == 404
# --- B-60: the status change is an admin mutation, so it leaves a trace ----------
async def _audit_entries(event_type: str) -> list:
from sqlalchemy import select
from app.db.base import AsyncSessionLocal
from app.db.models import AuditLog
async with AsyncSessionLocal() as session:
return (
await session.scalars(select(AuditLog).where(AuditLog.event_type == event_type))
).all()
async def test_status_change_is_audit_logged(client): # B-60
"""One shared ADMIN_TOKEN and no per-admin identity means the audit log is the
only accountability there is — a report could be silently marked resolved."""
token = await _register(client, username="reporter")
resp = await client.post(
"/bug-reports",
json={"description": "some bug"},
headers={"Authorization": f"Bearer {token}"},
)
report_id = resp.json()["id"]
await client.post(
f"/admin/bug-reports/{report_id}/status", headers=_ADMIN_HEADERS, json={"status": "resolved"}
)
entries = await _audit_entries("bug_report_status_changed")
assert len(entries) == 1
import json
payload = json.loads(entries[0].payload_json)
assert payload == {"report_id": report_id, "from": "open", "to": "resolved"}
assert entries[0].user_id is not None # the report's author, so it's traceable both ways
async def test_setting_the_status_it_already_has_logs_nothing(client): # B-60
"""Same rule as config_updated: an edit that changes nothing isn't an event, or
the log fills with noise that hides the real changes."""
resp = await client.post("/bug-reports", json={"description": "some bug"})
report_id = resp.json()["id"]
for _ in range(3):
await client.post(
f"/admin/bug-reports/{report_id}/status", headers=_ADMIN_HEADERS, json={"status": "open"}
)
assert await _audit_entries("bug_report_status_changed") == []