From cf2754f3024d1aad6eb2af673e4f17ff22cb497b Mon Sep 17 00:00:00 2001
From: Johan Lundberg <lundberg@sunet.se>
Date: Fri, 5 Jun 2026 14:12:54 +0200
Subject: [PATCH] fix(security): require a configured session secret in
 production

session_secret defaulted to a random per-process value, which silently
invalidates all sessions on restart and rotates the management client secret.
Add _resolve_session_secret(): use the configured secret; allow a generated
one only in debug or for a localhost issuer; otherwise fail startup. The
management client secret is now tied to the resolved session secret.

Refs: porchlight-wvx

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/porchlight/app.py             | 22 +++++++++++++++++++---
 tests/test_app.py                 | 19 +++++++++++++++++++
 tests/test_client_registration.py |  4 ++--
 3 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/src/porchlight/app.py b/src/porchlight/app.py
index 38f801f..e07d51f 100644
--- a/src/porchlight/app.py
+++ b/src/porchlight/app.py
@@ -82,8 +82,8 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]:
                 }
                 oidc_server.keyjar.add_symmetric(client_id, client_cfg.client_secret)
 
-            # Register management client
-            manage_secret = settings.session_secret or secrets.token_hex(32)
+            # Register management client (stable secret tied to the session secret)
+            manage_secret = app.state.session_secret
             oidc_server.context.cdb[settings.manage_client_id] = {
                 "client_id": settings.manage_client_id,
                 "client_secret": manage_secret,
@@ -101,6 +101,21 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]:
         raise NotImplementedError("MongoDB backend not yet implemented")
 
 
+def _resolve_session_secret(settings: Settings) -> str:
+    """Return the session signing secret, requiring one in production.
+
+    A random per-process secret silently invalidates sessions on restart and
+    rotates the management client secret, so it is only acceptable for local
+    development (debug or a localhost issuer).
+    """
+    if settings.session_secret:
+        return settings.session_secret
+    host = urlparse(settings.issuer).hostname or ""
+    if settings.debug or host in ("localhost", "127.0.0.1", "::1"):
+        return secrets.token_hex(32)
+    raise RuntimeError("OIDC_OP_SESSION_SECRET must be set in production (non-debug, non-localhost issuer).")
+
+
 def create_app(settings: Settings | None = None) -> FastAPI:
     if settings is None:
         settings = Settings()
@@ -116,7 +131,8 @@ def create_app(settings: Settings | None = None) -> FastAPI:
     app.state.settings = settings
 
     # Session middleware
-    session_secret = settings.session_secret or secrets.token_hex(32)
+    session_secret = _resolve_session_secret(settings)
+    app.state.session_secret = session_secret
     app.add_middleware(
         CSRFMiddleware,  # ty: ignore[invalid-argument-type]
         exempt_paths={"/token", "/userinfo"},
diff --git a/tests/test_app.py b/tests/test_app.py
index 82f1358..14c3aca 100644
--- a/tests/test_app.py
+++ b/tests/test_app.py
@@ -1,7 +1,10 @@
 from unittest.mock import MagicMock
 
+import pytest
 from httpx import AsyncClient
 
+from porchlight.app import create_app
+from porchlight.config import Settings
 from porchlight.dependencies import (
     get_credential_repo,
     get_magic_link_repo,
@@ -56,3 +59,19 @@ async def test_dependency_functions() -> None:
     assert get_user_repo(request) == "user_repo_sentinel"
     assert get_credential_repo(request) == "credential_repo_sentinel"
     assert get_magic_link_repo(request) == "magic_link_repo_sentinel"
+
+
+def test_create_app_requires_session_secret_in_production() -> None:
+    settings = Settings(issuer="https://op.example.com", sqlite_path=":memory:", debug=False)
+    with pytest.raises(RuntimeError, match="SESSION_SECRET"):
+        create_app(settings)
+
+
+def test_create_app_allows_missing_secret_on_localhost() -> None:
+    settings = Settings(issuer="http://localhost:8000", sqlite_path=":memory:")
+    assert create_app(settings) is not None
+
+
+def test_create_app_allows_missing_secret_in_debug() -> None:
+    settings = Settings(issuer="https://op.example.com", sqlite_path=":memory:", debug=True)
+    assert create_app(settings) is not None
diff --git a/tests/test_client_registration.py b/tests/test_client_registration.py
index d1d7e2b..ea6ba37 100644
--- a/tests/test_client_registration.py
+++ b/tests/test_client_registration.py
@@ -19,7 +19,7 @@ scope = ["openid", "profile"]
     toml_file = tmp_path / "test.toml"
     toml_file.write_text(toml_content)
 
-    settings = Settings(_toml_file=str(toml_file))
+    settings = Settings(_toml_file=str(toml_file), session_secret="x" * 32)
     app = create_app(settings)
 
     async with (
@@ -41,7 +41,7 @@ scope = ["openid", "profile"]
 
 async def test_manage_app_always_registered() -> None:
     """The internal manage-app client is always registered, even without config file clients."""
-    settings = Settings(issuer="https://test.example.com")
+    settings = Settings(issuer="https://test.example.com", session_secret="x" * 32)
     app = create_app(settings)
 
     async with (