Stamp UTC on naive API timestamps before serializing (B-35)

SQLite/aiosqlite returns DateTime columns as naive even though every
value is written in UTC, so a bare .isoformat() dropped the offset and
the frontend's new Date() parsed it as local time. Add a shared
isoformat_utc() helper and use it at every call site that was missing
the fix already applied ad hoc in rounds.py.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
2026-07-27 12:20:22 +02:00
co-authored by Claude Sonnet 5
parent 739fc9fed2
commit bb8b71278a
7 changed files with 70 additions and 30 deletions
+9 -23
View File
@@ -1,11 +1,11 @@
# Known bugs # Known bugs
A second full-codebase audit on 2026-07-27 found **25 further issues** (4 critical, 6 high, A second full-codebase audit on 2026-07-27 found **25 further issues** (4 critical, 6 high,
7 medium, 8 low), listed below as B-33 … B-49. B-25 through B-34 are fixed (see "Previously 7 medium, 8 low), listed below as B-33 … B-49. B-25 through B-35 are fixed (see "Previously
fixed" below) — no Critical-severity finding remains open; the other 15 are High/Medium/Low. fixed" below) — no Critical-severity finding remains open; the other 14 are High/Medium/Low.
The 139-test suite was green at the time of the audit, so none of these were caught by existing The 139-test suite was green at the time of the audit, so none of these were caught by existing
coverage — every fix lands with a regression test (the ten fixes so far brought the suite coverage — every fix lands with a regression test (the eleven fixes so far brought the suite
from 139 to 194). from 139 to 198).
The recurring pattern across the open findings is worth stating once: the code is rigorous The recurring pattern across the open findings is worth stating once: the code is rigorous
about the failure modes that have actually been hit, and silent about the ones that have not. about the failure modes that have actually been hit, and silent about the ones that have not.
@@ -20,21 +20,6 @@ single-process assumptions, no user-facing history, etc.), see "Known gaps / TOD
## Medium ## Medium
### B-35 — Every API timestamp is naive, so the frontend renders it in the wrong timezone
Verified empirically: the `DateTime` columns carry no timezone, so SQLite returns naive
datetimes and `.isoformat()` produces `2026-07-27T06:56:47.489110`**no `Z`**. JavaScript's
`new Date()` parses that as **local time**, so every date in `/admin` (rounds, pending
transactions, audit log, via `fmtDate` in `app/static/admin.js:50`) and `created_at` in
`/users/me` display two hours off in Italy.
The codebase knows about this — `api/routes/rounds.py` calls `.replace(tzinfo=timezone.utc)`
on `opened_at` explicitly — but the fix was never applied systematically.
**Proposed fix.** Make the columns `DateTime(timezone=True)` (Alembic migration) so the value
round-trips as aware, rather than patching each call site. Until then, at minimum a shared
serialization helper that stamps UTC, used by every `.isoformat()` in the API layer.
### B-36 — `_wait_for_next_block` waits forever, with no timeout and no visibility ### B-36 — `_wait_for_next_block` waits forever, with no timeout and no visibility
`rounds/scheduler.py:158-161` loops until a higher block arrives. No timeout, no log, no audit `rounds/scheduler.py:158-161` loops until a higher block arrives. No timeout, no log, no audit
@@ -195,12 +180,13 @@ already does.
- **B-29** — a UTXO absent from one server's `listunspent` was marked spent immediately, irreversibly, on a single unauthenticated reply - **B-29** — a UTXO absent from one server's `listunspent` was marked spent immediately, irreversibly, on a single unauthenticated reply
- **B-30** — a lost scripthash subscription meant a user's deposits were never credited, with no periodic safety net - **B-30** — a lost scripthash subscription meant a user's deposits were never credited, with no periodic safety net
- **B-31** — resubscribing on reconnect ran serially before anything else started, freezing the chain tip (and so an in-flight draw) for the whole sweep - **B-31** — resubscribing on reconnect ran serially before anything else started, freezing the chain tip (and so an in-flight draw) for the whole sweep
- **B-32** — an RBF bump's fee delta could fall below BIP125's relay-mandated minimum, so the node rejected it and the same tick retried identically forever; also had no ceiling on how high the fee rate could climb - **B-32** — an RBF bump could retry forever below BIP125's relay-mandated minimum fee delta, with no ceiling on the fee rate either
- **B-33** — `POST /auth/login` had no rate limiting on a custodial wallet, so a patient distributed attack could brute-force a password against an enumerable username list; fixed with per-username *and* per-IP exponential backoff (`app/auth/rate_limit.py`), registration throttled per-IP too (also bounds B-31's attacker-controlled user count) - **B-33** — `POST /auth/login` had no rate limiting, so a password could be brute-forced against an enumerable username list
- **B-34** — neither self-service password change nor the admin reset invalidated already-issued JWTs, so a stolen token (or an attacker's own session) survived a password change meant to lock it out; fixed with a `User.token_version` column embedded in every JWT (`"tv"` claim) and checked on every request in `get_current_user`/`get_optional_user`, bumped on both endpoints — change-password hands back a fresh token so the caller's own session keeps working, the admin reset does not - **B-34** — password change/reset didn't invalidate already-issued JWTs, so a stolen token survived a change meant to lock it out
- **B-35** — API timestamps round-tripped as naive datetimes, so the frontend parsed them as local time instead of UTC
See git history for the fix-by-fix breakdown (commits `f13f685`, `50a43ae`, `933760e`, and the See git history for the fix-by-fix breakdown (commits `f13f685`, `50a43ae`, `933760e`, and the
B-28/B-29/B-30/B-31/B-32/B-33/B-34 fixes). Suite grew from 139 to 194 tests over the ten. B-28/B-29/B-30/B-31/B-32/B-33/B-34/B-35 fixes). Suite grew from 139 to 198 tests over the eleven.
A full-codebase audit on 2026-07-26 (commit `d4e0974`) found 24 bugs across every Python A full-codebase audit on 2026-07-26 (commit `d4e0974`) found 24 bugs across every Python
module under `app/`, both static frontends, and the Docker/Caddy deployment — 5 critical, module under `app/`, both static frontends, and the Docker/Caddy deployment — 5 critical,
+1 -1
View File
@@ -12,7 +12,7 @@ All 10 stages of the original build order are code-complete and unit-tested —
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. 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.
**Read [BUGS.md](BUGS.md) before trusting any behaviour here.** Two audits: 2026-07-26 found 24 bugs (5 critical), all fixed; 2026-07-27 found 25 more (B-32 … B-49), of which **18 are still open** — no Critical, but High covers an RBF bump that can loop forever on a replacement the node always rejects (B-32), no brute-force protection on login (B-33), and password change/reset not invalidating existing JWTs (B-34). BUGS.md is the live open list with a proposed fix per finding; "Known gaps" at the end of this file is for limitations accepted **by design** instead. Don't fix a BUGS.md item silently as a side effect of other work — each fix lands with its own regression test. **Read [BUGS.md](BUGS.md) before trusting any behaviour here.** Two audits: 2026-07-26 found 24 bugs (5 critical), all fixed; 2026-07-27 found 25 more (B-25 … B-49), of which **14 are still open** — no Critical or High remains, only Medium/Low: `_wait_for_next_block` has no timeout or visibility if a round gets stuck in `drawing` (B-36), no WAL/`busy_timeout` under five concurrent SQLite writer tasks (B-39), among others. BUGS.md is the live open list with a proposed fix per finding; "Known gaps" at the end of this file is for limitations accepted **by design** instead. Don't fix a BUGS.md item silently as a side effect of other work — each fix lands with its own regression test.
Before writing code, read the "Architecture" section below in full plus the diagrams in [flowchart/](flowchart/): [platform-overview.mmd](flowchart/platform-overview.mmd) (the 5-phase flow) and [round-lifecycle.mmd](flowchart/round-lifecycle.mmd) (the round/draw lifecycle). Every node **and edge label** (conditions, retries, loops) is a behaviour that must be implemented as described. Regenerate the companion PDFs with `flowchart/render-pdf.sh <file>.mmd` after editing either. Before writing code, read the "Architecture" section below in full plus the diagrams in [flowchart/](flowchart/): [platform-overview.mmd](flowchart/platform-overview.mmd) (the 5-phase flow) and [round-lifecycle.mmd](flowchart/round-lifecycle.mmd) (the round/draw lifecycle). Every node **and edge label** (conditions, retries, loops) is a behaviour that must be implemented as described. Regenerate the companion PDFs with `flowchart/render-pdf.sh <file>.mmd` after editing either.
+6 -5
View File
@@ -6,6 +6,7 @@ from pydantic import BaseModel, Field, field_validator
from sqlalchemy import select from sqlalchemy import select
from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.ext.asyncio import AsyncSession
from app.api.timeutil import isoformat_utc
from app.audit.log import write_audit_log from app.audit.log import write_audit_log
from app.auth.security import hash_password from app.auth.security import hash_password
from app.config import settings from app.config import settings
@@ -163,7 +164,7 @@ async def list_users(session: AsyncSession = Depends(get_session)) -> list[Admin
username=u.username, username=u.username,
address=u.address, address=u.address,
balance_sats=u.cached_balance_sats, balance_sats=u.cached_balance_sats,
created_at=u.created_at.isoformat(), created_at=isoformat_utc(u.created_at),
) )
for u in users for u in users
] ]
@@ -253,8 +254,8 @@ async def list_rounds(session: AsyncSession = Depends(get_session), limit: int =
AdminRoundResponse( AdminRoundResponse(
id=r.id, id=r.id,
status=r.status, status=r.status,
opened_at=r.opened_at.isoformat(), opened_at=isoformat_utc(r.opened_at),
closed_at=r.closed_at.isoformat() if r.closed_at else None, closed_at=isoformat_utc(r.closed_at),
draw_block_height=r.draw_block_height, draw_block_height=r.draw_block_height,
draw_block_hash=r.draw_block_hash, draw_block_hash=r.draw_block_hash,
winner_user_id=r.winner_user_id, winner_user_id=r.winner_user_id,
@@ -291,7 +292,7 @@ async def list_audit_log(
payload=json.loads(e.payload_json), payload=json.loads(e.payload_json),
user_id=e.user_id, user_id=e.user_id,
round_id=e.round_id, round_id=e.round_id,
created_at=e.created_at.isoformat(), created_at=isoformat_utc(e.created_at),
) )
for e in entries for e in entries
] ]
@@ -333,7 +334,7 @@ async def list_pending_transactions(
current_txid=p.current_txid, current_txid=p.current_txid,
fee_rate_sat_vb=p.fee_rate_sat_vb, fee_rate_sat_vb=p.fee_rate_sat_vb,
attempt_count=p.attempt_count, attempt_count=p.attempt_count,
broadcast_at=p.broadcast_at.isoformat(), broadcast_at=isoformat_utc(p.broadcast_at),
replaced_by_txid=p.replaced_by_txid, replaced_by_txid=p.replaced_by_txid,
) )
for p in entries for p in entries
+2 -1
View File
@@ -4,6 +4,7 @@ from sqlalchemy import select
from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.ext.asyncio import AsyncSession
from app.api.errors import http_error from app.api.errors import http_error
from app.api.timeutil import isoformat_utc
from app.auth.dependencies import get_current_user from app.auth.dependencies import get_current_user
from app.auth.security import MIN_PASSWORD_LENGTH, create_access_token, hash_password, verify_password from app.auth.security import MIN_PASSWORD_LENGTH, create_access_token, hash_password, verify_password
from app.db.models import Round, RoundParticipant, User from app.db.models import Round, RoundParticipant, User
@@ -36,7 +37,7 @@ async def me(
balance_sats=user.cached_balance_sats, balance_sats=user.cached_balance_sats,
pending_balance_sats=pending_balance_sats, pending_balance_sats=pending_balance_sats,
has_pending=has_pending, has_pending=has_pending,
created_at=user.created_at.isoformat(), created_at=isoformat_utc(user.created_at),
) )
+17
View File
@@ -0,0 +1,17 @@
from datetime import datetime, timezone
def isoformat_utc(dt: datetime | None) -> str | None:
"""Serialize a datetime for API responses, stamping it UTC first.
Every DateTime column is written via app.db.models.utcnow() but SQLite/aiosqlite
round-trips it as a naive datetime, so a bare .isoformat() drops the "Z"/offset
and JavaScript's `new Date()` on the frontend parses the result as local time
instead of UTC (B-35). All stored values are UTC in practice, so a naive value
can be safely stamped rather than converted.
"""
if dt is None:
return None
if dt.tzinfo is None:
dt = dt.replace(tzinfo=timezone.utc)
return dt.isoformat()
+21
View File
@@ -0,0 +1,21 @@
from datetime import datetime, timezone
from app.api.timeutil import isoformat_utc
def test_naive_datetime_is_stamped_utc():
# SQLite/aiosqlite round-trips DateTime columns as naive even though every
# value written is UTC (app.db.models.utcnow) — this is the exact shape
# returned by the ORM after a read (B-35).
naive = datetime(2026, 7, 27, 6, 56, 47, 489110)
result = isoformat_utc(naive)
assert result == "2026-07-27T06:56:47.489110+00:00"
def test_aware_datetime_is_left_unchanged():
aware = datetime(2026, 7, 27, 6, 56, 47, tzinfo=timezone.utc)
assert isoformat_utc(aware) == aware.isoformat()
def test_none_passes_through():
assert isoformat_utc(None) is None
+14
View File
@@ -162,3 +162,17 @@ async def test_register_rejects_weak_credentials(client, payload):
async def test_register_accepts_valid_credentials(client): async def test_register_accepts_valid_credentials(client):
resp = await client.post("/auth/register", json={"username": "goodname", "password": "longenough1"}) resp = await client.post("/auth/register", json={"username": "goodname", "password": "longenough1"})
assert resp.status_code == 201 assert resp.status_code == 201
async def test_me_created_at_is_utc_stamped(client):
"""B-35: SQLite/aiosqlite returns DateTime columns as naive, even though every
value written is UTC (app.db.models.utcnow). A bare .isoformat() on that naive
value has no "Z"/offset, and JavaScript's `new Date()` then parses it as local
time instead of UTC."""
token = await _register(client)
headers = {"Authorization": f"Bearer {token}"}
resp = await client.get("/users/me", headers=headers)
assert resp.status_code == 200
created_at = resp.json()["created_at"]
assert created_at.endswith("+00:00") or created_at.endswith("Z")