From 22e3cfb2beda9802673f9e384770f81fbd1361c5 Mon Sep 17 00:00:00 2001 From: Davide Grilli Date: Mon, 27 Jul 2026 15:34:49 +0200 Subject: [PATCH] Disable Swagger/ReDoc/OpenAPI JSON by default (B-42) They enumerate the entire API surface, admin endpoints included, to anyone who requests them. Gate them behind a new ENABLE_API_DOCS setting (off by default) and update README/docs and BUGS.md/CLAUDE.md open-bug counts accordingly. Co-Authored-By: Claude Sonnet 5 --- .env.example | 5 +++++ BUGS.md | 20 +++++++------------- CLAUDE.md | 4 ++-- README.md | 6 ++++-- app/config.py | 5 +++++ app/main.py | 11 ++++++++++- docs/guida-admin.md | 5 +++-- docs/running-the-server.md | 3 +++ docs/setup.md | 5 +++++ tests/unit/test_docs_disabled.py | 28 ++++++++++++++++++++++++++++ 10 files changed, 72 insertions(+), 20 deletions(-) create mode 100644 tests/unit/test_docs_disabled.py diff --git a/.env.example b/.env.example index 48c8e71..f65bfc3 100644 --- a/.env.example +++ b/.env.example @@ -30,3 +30,8 @@ ADMIN_TOKEN= # Every business/round parameter (bet amount, round duration/cooldown, min # amount, fee rate, RBF timeout, fee address) is configured live from the # admin panel (/admin) instead of here — see docs/guida-admin.md. + +# Swagger/ReDoc/the raw OpenAPI JSON expose the entire API surface — admin +# endpoints included — to anyone who requests them. Off by default; set to +# true only for local development, never in production. +ENABLE_API_DOCS=false diff --git a/BUGS.md b/BUGS.md index 32ef023..ed2b963 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-42 … B-49. B-25 through B-41 are fixed (see "Previously -fixed" below) — no Critical- or Medium-severity finding remains open; the other 8 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 seventeen fixes so far brought the suite -from 139 to 222). +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 +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). 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,13 +18,6 @@ admin auth, single-process assumptions, no user-facing history, etc.) are docume ## Low / hygiene -### B-42 — `/docs` exposed in production - -FastAPI mounts Swagger by default, so the entire API surface — `/admin` included — is publicly -enumerable. The README advertises it. -**Fix:** `docs_url=None, redoc_url=None, openapi_url=None` in production (env-gated), or place -them behind `require_admin`. - ### B-43 — No HTTP security headers The [Caddyfile](Caddyfile) sets no CSP, no `X-Frame-Options`/`frame-ancestors`, and no HSTS @@ -89,6 +82,7 @@ already does. - **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-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-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 @@ -101,7 +95,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 fixes). Suite grew from 139 to 222 tests over the seventeen. +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. 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 16ae980..b916550 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 **12 are still open** — no Critical or High remains, only Medium/Low: no WAL/`busy_timeout` under five concurrent SQLite writer tasks (B-39), a 500-subscriber SSE cap that doubles as a cheap DoS of the realtime feature (B-38), 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 **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. 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. @@ -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) (18 open) — not duplicated here. +Accepted **by design**. For actual bugs see [BUGS.md](BUGS.md) (7 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/README.md b/README.md index f710ec9..bce79a4 100644 --- a/README.md +++ b/README.md @@ -23,8 +23,10 @@ uvicorn app.main:app --reload --port 8123 ``` Open `http://127.0.0.1:8123/` for the test UI, `http://127.0.0.1:8123/admin` -for the admin dashboard, `http://127.0.0.1:8123/docs` for the interactive API -docs. +for the admin dashboard. The interactive API docs at `/docs` are disabled by +default (they'd otherwise expose the whole API surface, admin endpoints +included) — set `ENABLE_API_DOCS=true` in `.env` for local development to +enable them. Or run the whole stack (app + Caddy reverse proxy with automatic TLS) via Docker: diff --git a/app/config.py b/app/config.py index 58e0f78..1314ea1 100644 --- a/app/config.py +++ b/app/config.py @@ -29,6 +29,11 @@ class Settings(BaseSettings): jwt_expire_minutes: int = 60 * 24 admin_token: str = "" + # Swagger/ReDoc/OpenAPI JSON expose the entire API surface (admin endpoints + # included) to anyone who requests them. Off by default (B-42) — set to true + # only for local development, never in production. + enable_api_docs: bool = False + # Every business/round parameter (bet amount, round duration/cooldown, # min amount, fee rate, RBF timeout, fee address) lives in the round_config # DB table instead (app/db/models.py RoundConfig, app/rounds/config.py) — diff --git a/app/main.py b/app/main.py index cc5a5b5..ff547d0 100644 --- a/app/main.py +++ b/app/main.py @@ -84,7 +84,16 @@ async def lifespan(app: FastAPI): await listener.client.close() -app = FastAPI(title="PLM Lottery", lifespan=lifespan) +# Swagger/ReDoc/the raw OpenAPI JSON enumerate the entire API surface, admin +# endpoints included, to anyone who requests them (B-42) — disabled unless +# ENABLE_API_DOCS is explicitly set, which should only happen in development. +app = FastAPI( + title="PLM Lottery", + lifespan=lifespan, + docs_url="/docs" if settings.enable_api_docs else None, + redoc_url="/redoc" if settings.enable_api_docs else None, + openapi_url="/openapi.json" if settings.enable_api_docs else None, +) app.include_router(auth_router) app.include_router(users_router) app.include_router(bets_router) diff --git a/docs/guida-admin.md b/docs/guida-admin.md index be07fc9..abcb212 100644 --- a/docs/guida-admin.md +++ b/docs/guida-admin.md @@ -121,8 +121,9 @@ Eventi a cui vale la pena prestare attenzione: ## Alternative all'interfaccia grafica Le stesse operazioni si possono fare da terminale o da Swagger UI -(`https:///docs`, sezione `admin`), sempre passando `ADMIN_TOKEN` -nell'header `X-Admin-Token`: +(`https:///docs`, sezione `admin` — disponibile solo se `ENABLE_API_DOCS=true` +è impostato in `.env`, disattivata di default perché espone l'intera API), +sempre passando `ADMIN_TOKEN` nell'header `X-Admin-Token`: ```bash # leggere la configurazione diff --git a/docs/running-the-server.md b/docs/running-the-server.md index ad396ed..739dcec 100644 --- a/docs/running-the-server.md +++ b/docs/running-the-server.md @@ -12,6 +12,9 @@ uvicorn app.main:app --reload --port 8123 - App su `http://127.0.0.1:8123/` - Pannello admin su `http://127.0.0.1:8123/admin` +- Docs API interattive su `http://127.0.0.1:8123/docs` solo se `ENABLE_API_DOCS=true` + in `.env` — disattivate di default perché espongono l'intera API, endpoint + admin inclusi (vedi [setup.md](setup.md)) - Log applicativi in `logs/app.log` (rotante, 10MB × 5 backup) - Nessun TLS, nessun reverse proxy — solo per test locali sulla tua macchina. diff --git a/docs/setup.md b/docs/setup.md index 97338d9..373e972 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -38,6 +38,11 @@ cp .env.example .env Le altre chiavi di `.env` (`DATABASE_URL`, `ELECTRUM_HOST`/`PORT`/`USE_SSL`, `MASTER_KEY_PATH`) hanno default sensati in `.env.example`. +`ENABLE_API_DOCS` (default `false`) controlla Swagger/ReDoc/l'OpenAPI JSON grezzo +su `/docs`, `/redoc` e `/openapi.json`: espongono l'intera superficie dell'API, +endpoint admin inclusi, quindi restano disattivati a meno di non impostarlo +esplicitamente a `true` — utile in locale, da evitare in produzione. + `ELECTRUM_FALLBACK_SERVERS` elenca i server di riserva, separati da virgola, nel formato `host:porta` (TLS, il caso normale) oppure `host:porta:notls`. Esempio: diff --git a/tests/unit/test_docs_disabled.py b/tests/unit/test_docs_disabled.py new file mode 100644 index 0000000..c9657a3 --- /dev/null +++ b/tests/unit/test_docs_disabled.py @@ -0,0 +1,28 @@ +"""B-42: Swagger/ReDoc/OpenAPI JSON must not be reachable unless explicitly enabled — +they enumerate the whole API surface, admin endpoints included.""" + +import importlib + +from app.config import settings + + +def _reload_main(): + import app.main + + return importlib.reload(app.main) + + +def test_docs_disabled_by_default(monkeypatch): + monkeypatch.setattr(settings, "enable_api_docs", False) + main = _reload_main() + assert main.app.docs_url is None + assert main.app.redoc_url is None + assert main.app.openapi_url is None + + +def test_docs_enabled_when_configured(monkeypatch): + monkeypatch.setattr(settings, "enable_api_docs", True) + main = _reload_main() + assert main.app.docs_url == "/docs" + assert main.app.redoc_url == "/redoc" + assert main.app.openapi_url == "/openapi.json"