docs: record security + observability hardening (ADR-050, ADR-051)
swadhinbiswas committed 3 months ago
d09fe00
1 parent f20419c
commit d09fe00
2 files changed
+85 -0
Showing 2 changed files+85-0
▼DECISIONS.md
+60-0
| @@ -1746,3 +1746,63 @@ committed). PLAN.md Phase 53 status updated to "reverted — dead code removed". | ||
1746 | 1746 | |
1747 | 1747 | |
1748 | 1748 | |
1749 | + | |
1750 | +## ADR-050 — 2026-07-15 — Secure-by-default CORS + opt-in security headers | |
1751 | + | |
1752 | +**Context:** The production-readiness audit (MED#9) flagged two security | |
1753 | +defaults. (1) `Cors::new()`/`Default` were `permissive()` → emitted | |
1754 | +`Access-Control-Allow-Origin: *` for every request, and the credential branch | |
1755 | +could reflect `*` with `Access-Control-Allow-Credentials: true` (an invalid but | |
1756 | +leaky combination). (2) `SecurityHeaders` was only opt-in and always emitted HSTS | |
1757 | +even on plaintext. | |
1758 | + | |
1759 | +**Decision:** | |
1760 | +- `Cors::new()` and `Default` now start with an **empty** `allow_origins` (no | |
1761 | + ACAO emitted) — fail-closed. Explicit `allow_origin(...)` / `allow_origins(...)` | |
1762 | + is required to enable CORS. `permissive()` still exists for dev. | |
1763 | +- The `*` + `allow_credentials` bug is fixed: when the matched origin is `*`, | |
1764 | + credentials are force-disabled and the concrete request origin is reflected | |
1765 | + instead of `*`. | |
1766 | +- `SecurityHeaders` gains `without_hsts()`; `Default` keeps HSTS but the Python | |
1767 | + `enable_secure_headers()` applies the **non-HSTS** default (plaintext server). | |
1768 | + HSTS is only added on explicit `enable_secure_headers(with_hsts=True)`. | |
1769 | +- Security headers are **not** auto-applied to every app: forcing a CSP | |
1770 | + (`default-src 'self'`) by default would break CDN-loaded UIs such as the | |
1771 | + `/docs` Swagger UI added in the same audit. They are a one-call opt-in. | |
1772 | + | |
1773 | +**Rationale:** secure defaults must not leak cross-origin or pin plaintext to | |
1774 | +HTTPS; but forcing CSP by default is a breaking change for legitimate apps. | |
1775 | +**Evidence:** `cargo test --workspace` (CORS unit tests updated to configure an | |
1776 | +origin), `clippy -D warnings`, `cargo fmt --check` all clean; live server | |
1777 | +verified that `enable_secure_headers()` emits `X-Content-Type-Options`, | |
1778 | +`X-Frame-Options: DENY`, `Content-Security-Policy`, `X-XSS-Protection` and | |
1779 | +**no** `Strict-Transport-Security`. | |
1780 | + | |
1781 | +## ADR-051 — 2026-07-15 — Real metrics + genuine readiness for Python apps | |
1782 | + | |
1783 | +**Context:** The Python app's builtin `/metrics` returned a static | |
1784 | +`justapi_up 1` string and `/ready` returned a static `{"status":"ready"}`, so | |
1785 | +Kubernetes probes and Prometheus scrapes saw no real signal. The Rust | |
1786 | +`Server` already collects `Metrics` and runs a `HealthRegistry`, but for Python | |
1787 | +apps routing is handled by the Python handler, which bypassed those builtins. | |
1788 | + | |
1789 | +**Decision:** | |
1790 | +- The live `Metrics` and `Arc<HealthRegistry>` are captured from the `Server` | |
1791 | + just before `server.run()` (via `slf` + `Python::attach`, because the | |
1792 | + `PyRefMut` borrow is `!Send` and cannot cross the detached thread) and stored | |
1793 | + on the `_JustAPIApp`. | |
1794 | +- `/metrics` now calls `Metrics::prometheus()` → real Prometheus exposition | |
1795 | + (request counters, status breakdown, latency histogram, bytes, connections). | |
1796 | +- `/ready` now consults the `HealthRegistry`: returns 200 with a component | |
1797 | + report when healthy/degraded, 503 when any registered check is unhealthy. | |
1798 | +- Added `JustAPIApp.register_health_check(name, callable)` (Rust | |
1799 | + `PyHealthCheck` wrapping a Python callable, invoked under the GIL) so users can | |
1800 | + wire DB/dependency probes into readiness. | |
1801 | + | |
1802 | +**Rationale:** observability and liveness/readiness are core production | |
1803 | +requirements; a static 200 / stub defeats autoscaling and alerting. | |
1804 | +**Evidence:** live server shows `/metrics` reporting `justapi_requests_total` | |
1805 | +incrementing on real traffic; `/ready` returns 200 healthy with a component | |
1806 | +report, and 503 (with report) when a registered check returns falsy. | |
1807 | +`cargo test --workspace` and `pytest` (minus two pre-existing, unrelated | |
1808 | +failures in `test_websocket`/`test_openapi_parity`) pass. | |
1749 | 1809 | |
▼PLAN.md
+25-0
| @@ -1079,3 +1079,28 @@ k8s guides, OpenAPI. | ||
1079 | 1079 | disaggregated P/D structural benchmark). |
1080 | 1080 | |
1081 | 1081 | |
1082 | + | |
1083 | +--- | |
1084 | + | |
1085 | +## Production-readiness hardening (post-audit, 2026-07-15) | |
1086 | + | |
1087 | +Status of the P0 sprint driven by the MED#9 audit verdict (see BENCHMARKS.md | |
1088 | +"Production-readiness verdict"). | |
1089 | + | |
1090 | +- [x] **Security defaults** (ADR-050): CORS secure-by-default (empty origins), | |
1091 | + `*`+credentials bug fixed; `SecurityHeaders` opt-in via | |
1092 | + `enable_secure_headers()` (non-HSTS default; HSTS on `with_hsts=True`). | |
1093 | +- [x] **Observability** (ADR-051): Python `/metrics` exports real Prometheus | |
1094 | + data; `/ready` reflects the `HealthRegistry` (503 on unhealthy dep); | |
1095 | + `register_health_check(name, callable)` wires Python probes. | |
1096 | +- [x] **API footguns** (prior session, in save-point): `with_default_routes`/ | |
1097 | + `with_handler` clobber is a hard panic; OpenAPI UI (`/openapi.json`, | |
1098 | + `/docs`, `/redoc`) served for Python apps. | |
1099 | +- [x] **Dead code / CI** (prior session): HTTP/3 removed (ADR-049); fuzz CI | |
1100 | + fixed to run all 7 real targets; orphan root `.rs` removed; `rustfmt.toml` | |
1101 | + stable-only so `cargo fmt --check` passes. | |
1102 | +- [ ] **Error-contract unification** (P1): three inconsistent shapes | |
1103 | + (`{"error"}` vs `{"detail"}` vs RFC-7807) should collapse to one. | |
1104 | +- [ ] **Panic isolation** (P1/P2): `panic = "abort"` means one bad request can | |
1105 | + kill the process; a GIL-path `catch_unwind` around handlers + an unwrap | |
1106 | + audit on the hot path (server/ + native/) is still TODO. | |
1082 | 1107 | |