diff --git a/BUGS.md b/BUGS.md index ed2b963..d4cdf42 100644 --- a/BUGS.md +++ b/BUGS.md @@ -1,11 +1,11 @@ # Known bugs 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-43 … B-49. B-25 through B-42 are fixed (see "Previously -fixed" below) — no Critical-, High- or Medium-severity finding remains open; the remaining 7 are +7 medium, 8 low), listed below as B-44 … B-49. B-25 through B-43 are fixed (see "Previously +fixed" below) — no Critical-, High- or Medium-severity finding remains open; the remaining 6 are Low/hygiene. 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 eighteen fixes so far brought -the suite from 139 to 224). +by existing coverage — every fix lands with a regression test (the nineteen fixes so far brought +the suite from 139 to 230). 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. @@ -18,15 +18,6 @@ admin auth, single-process assumptions, no user-facing history, etc.) are docume ## Low / hygiene -### B-43 — No HTTP security headers - -The [Caddyfile](Caddyfile) sets no CSP, no `X-Frame-Options`/`frame-ancestors`, and no HSTS -(Caddy does not add it on its own). The JWT lives in `localStorage`, so any XSS exfiltrates -it, and the page is iframeable. -**Fix:** a `header` block in the Caddyfile with `Strict-Transport-Security`, -`X-Content-Type-Options: nosniff`, `Referrer-Policy` and a CSP tight enough for two static -pages with no external assets (`default-src 'self'`). - ### B-44 — README and CLAUDE.md contradict each other The README says to run `uvicorn --reload` directly and @@ -83,6 +74,7 @@ already does. - **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-42** — Swagger/ReDoc/the raw OpenAPI JSON enumerated the entire API surface, admin endpoints included, to anyone who requested them; now off by default and gated behind `ENABLE_API_DOCS` +- **B-43** — the Caddyfile sent no CSP, no `X-Frame-Options`/`frame-ancestors`, and no HSTS, on a page whose JWT lives in `localStorage` - **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, so a password could be brute-forced against an enumerable username list - **B-34** — password change/reset didn't invalidate already-issued JWTs, so a stolen token survived a change meant to lock it out @@ -95,7 +87,7 @@ already does. - **B-41** — confirmation/reconciliation depended on a verbose `blockchain.transaction.get` reply many Electrum servers reject, and abandonment relied on fragile substring-matching of an error message 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/B-35/B-36/B-37/B-38/B-39/B-40/B-41/B-42 fixes). Suite grew from 139 to 224 tests over the eighteen. +B-28/B-29/B-30/B-31/B-32/B-33/B-34/B-35/B-36/B-37/B-38/B-39/B-40/B-41/B-42/B-43 fixes). Suite grew from 139 to 230 tests over the nineteen. 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, diff --git a/CLAUDE.md b/CLAUDE.md index b916550..2a66989 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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. -**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 **7 are still open** — no Critical, High or Medium remains, only Low/hygiene: an admin list endpoint with no pagination bound (B-45), unbounded `String` columns for large text (B-47), 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. +**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 **6 are still open** — no Critical, High or Medium remains, only Low/hygiene: an admin list endpoint with no pagination bound (B-45), unbounded `String` columns for large text (B-47), 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 .mmd` after editing either. @@ -55,7 +55,7 @@ docker compose down `SITE_ADDRESS` unset → `localhost`, Caddy issues a self-signed cert from its internal CA (browser warning on first visit is expected; `curl -k`). `SITE_ADDRESS=lottery.example.com docker compose up -d` → real Let's Encrypt cert, automatically renewed (needs DNS pointing here and ports 80+443 reachable). -The `Caddyfile` sets **no** security headers — no CSP, HSTS or `X-Frame-Options` (B-43). `restart: unless-stopped` on `app` means a mid-round crash auto-restarts: `closing` and `paying_out` resume on their own, `drawing` does not (see Known gaps). +The `Caddyfile` sends baseline security headers — HSTS, `X-Content-Type-Options: nosniff`, `X-Frame-Options: DENY`, `Referrer-Policy`, and a CSP scoped to `default-src 'self'` plus the Google Fonts `@import` in `style.css`/`admin.css`. `script-src`/`style-src` need `'unsafe-inline'` because both SPAs use inline `onclick` handlers and `style=""` attributes throughout — removing those is a separate, larger refactor, not a header change. `restart: unless-stopped` on `app` means a mid-round crash auto-restarts: `closing` and `paying_out` resume on their own, `drawing` does not (see Known gaps). ## Tech stack @@ -233,7 +233,7 @@ Explicit design choices, not derivable from any single file — respect them: ## Known gaps / TODO -Accepted **by design**. For actual bugs see [BUGS.md](BUGS.md) (7 open) — not duplicated here. +Accepted **by design**. For actual bugs see [BUGS.md](BUGS.md) (6 open) — not duplicated here. - **`drawing` doesn't resume after a restart.** `_tick()` handles `open`, `closing` and `paying_out` (the last via `_retry_payout_if_due`); nothing re-enters `_wait_for_next_block` after a crash. That wait is unbounded by design (the draw's entropy genuinely depends on a future block) but no longer silent — past `_DRAW_STALL_THRESHOLD_SECONDS` it logs progress and writes a `draw_stalled` audit entry, and `GET /rounds/current`'s `draw_waiting_since` surfaces it live (B-36). Restart-resumption itself remains the last prerequisite for running unattended. - **RBF handles one shape only**: a single change output, back to the tx's own sender, big enough to absorb the increase. No extra-input fallback — an exact-amount tx or too-small change raises `RbfError`. Not permanent, though: an unbumpable tx that never confirms is eventually abandoned and its UTXOs released. diff --git a/Caddyfile b/Caddyfile index b51229c..f6db413 100644 --- a/Caddyfile +++ b/Caddyfile @@ -13,5 +13,21 @@ not path /rounds/stream } encode @not_sse gzip + + # B-43: Caddy adds none of these on its own. The JWT lives in + # localStorage, so any XSS exfiltrates it — CSP is the main mitigation. + # script-src/style-src need 'unsafe-inline' because both SPAs + # (app/static/index.html, admin.html) use inline onclick handlers and + # style="" attributes throughout; removing those is a separate, + # larger refactor, not a header change. fonts.googleapis.com/gstatic.com + # are the one external asset (the Google Fonts @import in style.css/admin.css). + header { + Strict-Transport-Security "max-age=31536000; includeSubDomains" + X-Content-Type-Options "nosniff" + X-Frame-Options "DENY" + Referrer-Policy "strict-origin-when-cross-origin" + Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; font-src 'self' https://fonts.gstatic.com; img-src 'self'; connect-src 'self'; base-uri 'self'; form-action 'self'; frame-ancestors 'none'; object-src 'none'" + } + reverse_proxy app:8123 } diff --git a/tests/unit/test_caddyfile_security_headers.py b/tests/unit/test_caddyfile_security_headers.py new file mode 100644 index 0000000..ed12a9a --- /dev/null +++ b/tests/unit/test_caddyfile_security_headers.py @@ -0,0 +1,35 @@ +"""B-43: the Caddyfile must keep sending baseline security headers. Caddy adds +none of these on its own, and the JWT lives in localStorage, so a regression +here silently reopens an XSS/clickjacking exposure with no test ever failing +in the Python suite (the Caddyfile isn't imported/exercised by anything else).""" + +from pathlib import Path + +CADDYFILE = (Path(__file__).parent.parent.parent / "Caddyfile").read_text() + + +def test_header_block_present(): + assert "header {" in CADDYFILE + + +def test_hsts_is_set(): + assert "Strict-Transport-Security" in CADDYFILE + assert "max-age=" in CADDYFILE + + +def test_nosniff_is_set(): + assert 'X-Content-Type-Options "nosniff"' in CADDYFILE + + +def test_frame_ancestors_are_blocked(): + assert 'X-Frame-Options "DENY"' in CADDYFILE + assert "frame-ancestors 'none'" in CADDYFILE + + +def test_referrer_policy_is_set(): + assert "Referrer-Policy" in CADDYFILE + + +def test_csp_default_src_is_self(): + assert "Content-Security-Policy" in CADDYFILE + assert "default-src 'self'" in CADDYFILE