From 3ec09470ffbd6bd81a8cd9cb25fd44797c90f387 Mon Sep 17 00:00:00 2001 From: "Mumuni (Syslog Code Agent)" Date: Wed, 2 Sep 2026 23:26:44 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20P0=20hardening=20=E2=80=94=20fail-closed?= =?UTF-8?q?=20SECRET=5FKEY,=20locked=20CORS,=20role-safe=20registration=20?= =?UTF-8?q?(HARDENING.md=20P0.1/P0.2/P0.3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P0.1 — fail-closed secrets: - config.py: no default SECRET_KEY; refuses to boot when unset, a known placeholder, or <32 chars. Generate with: openssl rand -hex 32. - docker-compose.yml: literal secrets removed; runtime env now comes from a git-ignored .env via env_file. .env.example added as template. - .gitignore already covers .env (verified). P0.2 — locked CORS: - main.py: CORS_ORIGINS must be an explicit comma-separated allow-list. '*' or an empty value refuses to boot (was: silently ['*'] with allow_credentials=True). P0.3 — role-safe registration: - services/auth.py: client-supplied 'role' is IGNORED on POST /api/auth/register; self-registered users always get the least-privilege 'CS Rep' role. Unauthenticated callers can no longer mint Admin/Jerome, Admin/Wahab, or Director accounts. Tests: - conftest.py sets test SECRET_KEY/CORS_ORIGINS before app import. - New tests/test_p0_hardening.py (8 tests): role-escalation blocked for Admin/Jerome and Admin/Wahab, duplicate-email 409, and subprocess boot-validation for placeholder/short/missing secret + wildcard CORS. - Full suite: 44 passed. Redeploy note (per research): seed_units/seed_categories are insert-only, so the Aug-26 redeploy does NOT orphan historical tickets referencing units 103E/103W/105E/105W or the legacy 34-category tree. Pending Wahab: are 103E/103W/105E/105W real apartments dropped from the Excel regeneration? Optional follow-up: floor-number backfill for already- seeded units (mapping corrected floors; existing rows keep old values). Checks per HARDENING.md acceptance: - [x] starting without a real key fails loudly (subprocess-verified) - [x] compose carries no literal secret; secrets come from .env - [x] CORS_ORIGINS explicit allow-list, '*' rejected - [x] unauthenticated register cannot mint Admin/* or Director --- .env.example | 15 +++++ app/core/config.py | 20 +++++- app/main.py | 11 +++- app/schemas/auth.py | 5 +- app/services/auth.py | 13 +++- docker-compose.yml | 5 +- tests/conftest.py | 4 ++ tests/test_p0_hardening.py | 132 +++++++++++++++++++++++++++++++++++++ 8 files changed, 196 insertions(+), 9 deletions(-) create mode 100644 .env.example create mode 100644 tests/test_p0_hardening.py diff --git a/.env.example b/.env.example new file mode 100644 index 0000000..7b9110f --- /dev/null +++ b/.env.example @@ -0,0 +1,15 @@ +# Denya OneCare — runtime environment template (HARDENING.md P0.1/P1.1) +# Copy to .env and fill in real values. NEVER commit .env. +# Generate the secret with: openssl rand -hex 32 + +# ── Required ───────────────────────────────────────────── +SECRET_KEY= +DATABASE_URL=sqlite+aiosqlite:///./data/denya_onecare.db +# Explicit origin allow-list — "*" is rejected at startup (P0.2) +CORS_ORIGINS=http://localhost:8000 + +# ── Optional (WhatsApp; needed before wiring Meta) ─────── +WHATSAPP_PHONE_NUMBER_ID= +WHATSAPP_ACCESS_TOKEN= +WHATSAPP_VERIFY_TOKEN= +META_GRAPH_BASE=https://graph.facebook.com/v18.0 diff --git a/app/core/config.py b/app/core/config.py index 78b2e17..5a8cc51 100644 --- a/app/core/config.py +++ b/app/core/config.py @@ -23,7 +23,7 @@ class Settings(BaseSettings): DATABASE_URL: str = "sqlite+aiosqlite:///./denya_onecare.db" # ── Auth ───────────────────────────────────────────────────────── - SECRET_KEY: str = "change-me-in-production-use-a-real-secret" + SECRET_KEY: str = "" ALGORITHM: str = "HS256" ACCESS_TOKEN_EXPIRE_MINUTES: int = 60 # Phase 1: raised 30 -> 60 for fewer re-logins REFRESH_TOKEN_EXPIRE_MINUTES: int = 60 * 24 * 7 # 7 days @@ -42,3 +42,21 @@ class Settings(BaseSettings): settings = Settings() + +# ── Fail-closed secret validation (HARDENING.md P0.1) ───────────────── +# Refuse to boot without a real SECRET_KEY. Devs must create a local .env +# (see .env.example); production injects it via docker-compose env_file. +_KNOWN_PLACEHOLDER_SECRETS = { + "", + "change-me-in-production", + "change-me-in-production-use-a-real-secret", + "changeme", + "secret", +} + +if settings.SECRET_KEY in _KNOWN_PLACEHOLDER_SECRETS or len(settings.SECRET_KEY) < 32: + raise RuntimeError( + "SECRET_KEY is missing, a known placeholder, or shorter than 32 chars. " + "Generate one with: openssl rand -hex 32 — and set it in .env " + "(dev) or the runtime environment (prod). Refusing to start." + ) diff --git a/app/main.py b/app/main.py index d45289d..18800e7 100644 --- a/app/main.py +++ b/app/main.py @@ -83,10 +83,17 @@ app = FastAPI( lifespan=lifespan, ) -# ── CORS ───────────────────────────────────────────────────────────── +# ── CORS (HARDENING.md P0.2 — explicit origin allow-list, never "*") ── +_origins = [o.strip() for o in settings.CORS_ORIGINS.split(",") if o.strip()] +if "*" in _origins or not _origins: + raise RuntimeError( + "CORS_ORIGINS must be an explicit comma-separated origin allow-list " + "(e.g. 'https://denya.sysloggh.net,http://localhost:8000'). " + "'*' with allow_credentials=True is invalid and unsafe. Refusing to start." + ) app.add_middleware( CORSMiddleware, - allow_origins=settings.CORS_ORIGINS.split(",") if settings.CORS_ORIGINS != "*" else ["*"], + allow_origins=_origins, allow_credentials=True, allow_methods=["*"], allow_headers=["*"], diff --git a/app/schemas/auth.py b/app/schemas/auth.py index 3febf54..ac55166 100644 --- a/app/schemas/auth.py +++ b/app/schemas/auth.py @@ -10,7 +10,10 @@ class RegisterRequest(BaseModel): password: str full_name: str phone: str | None = None - role: str = "CS Rep" + # HARDENING.md P0.3: role is NOT client-controllable. Self-registration + # always creates the least-privilege role; privileged roles are assigned + # by an admin directly in the DB (or a future admin-gated endpoint). + role: str = "CS Rep" # kept for backward compat; ignored by the service class LoginRequest(BaseModel): diff --git a/app/services/auth.py b/app/services/auth.py index 9bed864..fd82810 100644 --- a/app/services/auth.py +++ b/app/services/auth.py @@ -16,9 +16,18 @@ from app.core.security import ( from app.models.user import User from app.schemas.auth import RegisterRequest +# HARDENING.md P0.3 — least-privilege default for self-registered users. +_SELF_REGISTER_ROLE = "CS Rep" + async def register(db: AsyncSession, body: RegisterRequest) -> User: - """Create a new user. Raises 409 if email already exists.""" + """Create a new user. Raises 409 if email already exists. + + HARDENING.md P0.3: unauthenticated self-registration must never mint a + privileged role. The client-supplied ``role`` field is IGNORED — new + self-registered users always land on the least-privilege role. + Admins assign elevated roles directly (DB seed / admin endpoint). + """ result = await db.execute(select(User).where(User.email == body.email)) if result.scalar_one_or_none(): raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="Email already registered") @@ -28,7 +37,7 @@ async def register(db: AsyncSession, body: RegisterRequest) -> User: password_hash=hash_password(body.password), full_name=body.full_name, phone=body.phone, - role=body.role, + role=_SELF_REGISTER_ROLE, ) db.add(user) await db.flush() diff --git a/docker-compose.yml b/docker-compose.yml index 1fe37ba..bfacdba 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -4,10 +4,9 @@ services: container_name: denya-onecare ports: - "8000:8000" + env_file: + - .env # git-ignored; see .env.example for required keys environment: - - DATABASE_URL=sqlite+aiosqlite:///./data/denya_onecare.db - - SECRET_KEY=change-me-in-production - - CORS_ORIGINS=* - DEBUG=false volumes: - app-data:/app/data diff --git a/tests/conftest.py b/tests/conftest.py index 23c8bd1..b8fbdf6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,6 +12,10 @@ import tempfile _TMP_DIR = tempfile.mkdtemp(prefix="denya-test-") os.environ["DATABASE_URL"] = f"sqlite+aiosqlite:///{_TMP_DIR}/test.db" +# HARDENING.md P0.1/P0.2: the app now fails closed without a real SECRET_KEY +# and an explicit CORS allow-list — tests must satisfy both. +os.environ.setdefault("SECRET_KEY", "test-secret-key-not-for-production-0123456789abcdef") +os.environ.setdefault("CORS_ORIGINS", "http://test") import pytest_asyncio # noqa: E402 (DATABASE_URL must be set before app imports) from httpx import ASGITransport, AsyncClient # noqa: E402 diff --git a/tests/test_p0_hardening.py b/tests/test_p0_hardening.py new file mode 100644 index 0000000..71d3b0c --- /dev/null +++ b/tests/test_p0_hardening.py @@ -0,0 +1,132 @@ +"""P0 hardening regression tests (HARDENING.md P0.1 / P0.2 / P0.3). + +Covers: +- P0.3: self-registration CANNOT mint a privileged role (role field ignored) +- P0.3: duplicate email still 409s +- P0.1: app fails to import/boot with placeholder or missing SECRET_KEY +- P0.2: app fails to boot with CORS_ORIGINS="*" +""" + +from __future__ import annotations + +import os +import subprocess +import sys +from pathlib import Path + +import pytest +from httpx import ASGITransport, AsyncClient + +REPO_ROOT = Path(__file__).resolve().parent.parent + +pytestmark = pytest.mark.asyncio + + + +# ── P0.3: registration role-escalation ──────────────────────────────── + +async def test_register_cannot_mint_admin_role(client: AsyncClient): + """A raw unauthenticated register call must NOT be able to mint Admin/*.""" + resp = await client.post( + "/api/auth/register", + json={ + "email": "attacker@example.com", + "password": "Sup3rSecret!", + "full_name": "Attacker", + "role": "Admin/Jerome", + }, + ) + assert resp.status_code == 201, resp.text + created = resp.json() + assert created["role"] == "CS Rep", ( + f"self-registration minted privileged role: {created['role']}" + ) + + +async def test_register_role_wahab_also_blocked(client: AsyncClient): + resp = await client.post( + "/api/auth/register", + json={ + "email": "attacker2@example.com", + "password": "Sup3rSecret!", + "full_name": "Attacker Two", + "role": "Admin/Wahab", + }, + ) + assert resp.status_code == 201 + assert resp.json()["role"] == "CS Rep" + + +async def test_register_duplicate_email_conflict(client: AsyncClient): + payload = { + "email": "dupe@example.com", + "password": "Sup3rSecret!", + "full_name": "Dupe", + } + r1 = await client.post("/api/auth/register", json=payload) + assert r1.status_code == 201 + r2 = await client.post("/api/auth/register", json=payload) + assert r2.status_code == 409 + + +# ── P0.1 / P0.2: fail-closed boot validation ────────────────────────── + +def _boot_with_env(env_overrides: dict[str, str]) -> subprocess.CompletedProcess: + """Try importing app.main in a subprocess with the given env; the import + must fail (non-zero) when fail-closed validation trips.""" + env = os.environ.copy() + env["DATABASE_URL"] = "sqlite+aiosqlite:///:memory:" + env.pop("SECRET_KEY", None) + env.pop("CORS_ORIGINS", None) + env.update(env_overrides) + script = ( + "import sys; sys.path.insert(0, ''); " + "import app.main" # noqa + ) + return subprocess.run( + [sys.executable, "-c", script], + cwd=str(REPO_ROOT), + env=env, + capture_output=True, + text=True, + timeout=60, + ) + + +def test_boot_fails_with_placeholder_secret(): + result = _boot_with_env({"SECRET_KEY": "change-me-in-production"}) + assert result.returncode != 0, "app booted with placeholder SECRET_KEY!" + assert "SECRET_KEY" in result.stderr + + +def test_boot_fails_with_short_secret(): + result = _boot_with_env({"SECRET_KEY": "tooshort"}) + assert result.returncode != 0, "app booted with a <32-char SECRET_KEY!" + assert "SECRET_KEY" in result.stderr + + +def test_boot_fails_without_secret(): + result = _boot_with_env({"SECRET_KEY": ""}) + assert result.returncode != 0, "app booted without a SECRET_KEY!" + assert "SECRET_KEY" in result.stderr + + +def test_boot_fails_with_wildcard_cors(): + result = _boot_with_env( + { + "SECRET_KEY": "test-secret-key-not-for-production-0123456789abcdef", + "CORS_ORIGINS": "*", + } + ) + assert result.returncode != 0, "app booted with CORS_ORIGINS=* !" + assert "CORS_ORIGINS" in result.stderr + + +def test_boot_succeeds_with_valid_env(): + result = _boot_with_env( + { + "SECRET_KEY": "test-secret-key-not-for-production-0123456789abcdef", + "CORS_ORIGINS": "http://test", + } + ) + assert result.returncode == 0, result.stderr -- 2.54.0