Skip to content

Commit 3a2b66d

Browse files
fix(security): validate required config at startup; fail-closed unsigned-session fallback (#174)
config.py read every required secret with an empty-string default and validated none, so the app booted with all secrets unset and failed opaquely later — malformed REST URL on first DB call, mid-request Gemini errors, and worst, an empty SESSION_SECRET silently switched OAuth state to an unsigned in-memory fallback (insecure degradation). - Add validate_config(), called from the FastAPI lifespan: raises one clear error naming every missing key (SUPABASE_URL, SUPABASE_SERVICE_KEY, GEMINI_API_KEY always; SESSION_SECRET outside local). - Add APP_ENV (defaults to production → fail-closed) / IS_LOCAL. The unsigned OAuth-state fallback now raises outside local dev (defense in depth at the use site), so it is unreachable in production. Tests: validate_config names all missing keys and relaxes SESSION_SECRET only in local; the unsigned fallback raises in production (fails pre-fix, which silently used it). Updated the two pre-existing fallback round-trip tests to declare local mode.
1 parent a27b688 commit 3a2b66d

5 files changed

Lines changed: 152 additions & 2 deletions

File tree

backend/config.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@
1616
FRONTEND_URL = os.getenv("FRONTEND_URL", "http://localhost:3000")
1717
SESSION_SECRET = os.getenv("SESSION_SECRET", "")
1818

19+
# Deployment mode (#174). Defaults to "production" so the config is fail-closed:
20+
# a deployment that sets nothing gets the strict checks. Set APP_ENV=local (or
21+
# development/dev/test) to relax SESSION_SECRET for local dev.
22+
APP_ENV = os.getenv("APP_ENV", "production").strip().lower()
23+
IS_LOCAL = APP_ENV in {"local", "development", "dev", "test"}
24+
1925
GOOGLE_SCOPES = [
2026
"https://www.googleapis.com/auth/calendar.events",
2127
"https://www.googleapis.com/auth/calendar.readonly",
@@ -35,6 +41,36 @@
3541
MAX_AVATAR_SIZE: int = 5 * 1024 * 1024 # 5 MB
3642

3743

44+
def validate_config() -> None:
45+
"""Fail loudly at startup if required configuration is missing (#174).
46+
47+
Without this the app boots with empty secrets and fails opaquely later:
48+
a "" SUPABASE_URL builds a malformed REST URL on the first DB call, a
49+
missing GEMINI_API_KEY surfaces only mid-request, and — worst — an empty
50+
SESSION_SECRET silently disables HMAC signing and drops session/OAuth
51+
state into an unsigned in-memory fallback. Raise one clear error naming
52+
every missing key instead.
53+
54+
SESSION_SECRET is required outside local dev (IS_LOCAL). The other three
55+
are always required (CI/tests supply dummy values).
56+
"""
57+
missing = []
58+
if not SUPABASE_URL:
59+
missing.append("SUPABASE_URL")
60+
if not SUPABASE_SERVICE_KEY:
61+
missing.append("SUPABASE_SERVICE_KEY")
62+
if not GEMINI_API_KEY:
63+
missing.append("GEMINI_API_KEY")
64+
if not SESSION_SECRET and not IS_LOCAL:
65+
missing.append("SESSION_SECRET")
66+
if missing:
67+
raise RuntimeError(
68+
"Missing required configuration: "
69+
+ ", ".join(missing)
70+
+ f". (APP_ENV={APP_ENV!r}; set APP_ENV=local to relax SESSION_SECRET for local dev.)"
71+
)
72+
73+
3874
def get_mastery_tier(score: float) -> str:
3975
if score >= 0.75:
4076
return "mastered"

backend/main.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
from fastapi.responses import JSONResponse
1212
from starlette.exceptions import HTTPException as StarletteHTTPException
1313

14-
from config import FRONTEND_URL, MAX_AVATAR_SIZE, PORT, STORAGE_BUCKET
14+
from config import FRONTEND_URL, MAX_AVATAR_SIZE, PORT, STORAGE_BUCKET, validate_config
1515

1616
# App-wide log format. Per-request log lines (with request_id, duration,
1717
# status) are emitted from RequestIDMiddleware; this just sets the
@@ -71,6 +71,9 @@
7171
# that no migration ever made" bugs.
7272
@asynccontextmanager
7373
async def _lifespan(_app: FastAPI):
74+
# #174: fail loudly at startup if required secrets are missing, before
75+
# serving any request, rather than booting and failing opaquely later.
76+
validate_config()
7477
await ensure_bucket_exists(
7578
STORAGE_BUCKET,
7679
public=True, # required for unauthenticated <img src> reads

backend/routes/auth.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
AUTH_SCOPES,
2525
FRONTEND_URL,
2626
SESSION_SECRET,
27+
IS_LOCAL,
2728
)
2829
from db.connection import table
2930
from services.encryption import encrypt, encrypt_if_present, decrypt_if_present
@@ -106,6 +107,15 @@ def _encode_oauth_cookie(payload: dict) -> str:
106107
sig_bytes = _hmac.new(SESSION_SECRET.encode(), payload_b64.encode(), hashlib.sha256).digest()
107108
sig_b64 = base64.urlsafe_b64encode(sig_bytes).decode().rstrip("=")
108109
return f"{payload_b64}.{sig_b64}"
110+
# #174: the unsigned in-memory fallback is local-dev only. Outside local it
111+
# is a silent, insecure degradation (unsigned OAuth state) — fail closed.
112+
# (validate_config already blocks startup without SESSION_SECRET in prod;
113+
# this is defense in depth at the use site.)
114+
if not IS_LOCAL:
115+
raise RuntimeError(
116+
"SESSION_SECRET is required outside local dev; refusing to issue "
117+
"unsigned OAuth state."
118+
)
109119
nonce = payload.get("n", "")
110120
if nonce:
111121
_OAUTH_FALLBACK_STORE[nonce] = (_time.monotonic() + _OAUTH_COOKIE_MAX_AGE, payload)
@@ -133,6 +143,11 @@ def _decode_oauth_cookie(cookie_value: str | None) -> dict | None:
133143
except Exception:
134144
return None
135145
return payload if isinstance(payload, dict) else None
146+
# #174: the unsigned in-memory path is local-dev only. Outside local,
147+
# refuse to accept unsigned OAuth state (symmetric with the encode-side
148+
# guard) so no unsigned cookie is ever honored in production.
149+
if not IS_LOCAL:
150+
return None
136151
try:
137152
padded = cookie_value + "=" * (-len(cookie_value) % 4)
138153
payload = json.loads(base64.urlsafe_b64decode(padded.encode()).decode())

backend/tests/test_auth_state.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,10 @@ def test_malformed_cookie_returns_none(self, monkeypatch):
7171

7272
def test_fallback_in_memory_store_round_trip(self, monkeypatch):
7373
# Without SESSION_SECRET, encode stashes the payload in an in-memory
74-
# dict keyed by nonce. Decode retrieves it.
74+
# dict keyed by nonce. Decode retrieves it. #174: this unsigned
75+
# fallback is local-dev only, so the test must declare local mode.
7576
monkeypatch.setattr(auth_module, "SESSION_SECRET", "")
77+
monkeypatch.setattr(auth_module, "IS_LOCAL", True)
7678
auth_module._OAUTH_FALLBACK_STORE.clear()
7779
payload = {"n": "fallback-nonce", "cv": "verifier", "popup_id": "p"}
7880
cookie = auth_module._encode_oauth_cookie(payload)
@@ -81,6 +83,7 @@ def test_fallback_in_memory_store_round_trip(self, monkeypatch):
8183

8284
def test_fallback_unknown_nonce_returns_none(self, monkeypatch):
8385
monkeypatch.setattr(auth_module, "SESSION_SECRET", "")
86+
monkeypatch.setattr(auth_module, "IS_LOCAL", True)
8487
auth_module._OAUTH_FALLBACK_STORE.clear()
8588
# Build a cookie payload whose nonce was never registered
8689
bogus = base64.urlsafe_b64encode(
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
"""
2+
Regression tests for #174:
3+
- validate_config() fails loudly at startup, naming every missing required key.
4+
- the unsigned in-memory OAuth-state fallback is unreachable outside local dev.
5+
6+
The fail-closed test fails on pre-fix code, where _encode_oauth_cookie silently
7+
fell back to the unsigned in-memory store whenever SESSION_SECRET was empty,
8+
regardless of environment.
9+
"""
10+
import pytest
11+
12+
import config
13+
from routes import auth as auth_mod
14+
15+
16+
def _set_required(monkeypatch, **overrides):
17+
base = {
18+
"SUPABASE_URL": "https://x.supabase.co",
19+
"SUPABASE_SERVICE_KEY": "service-key",
20+
"GEMINI_API_KEY": "gemini-key",
21+
"SESSION_SECRET": "session-secret",
22+
"IS_LOCAL": False,
23+
}
24+
base.update(overrides)
25+
for k, v in base.items():
26+
monkeypatch.setattr(config, k, v)
27+
28+
29+
class TestValidateConfig:
30+
def test_passes_when_all_present(self, monkeypatch):
31+
_set_required(monkeypatch)
32+
config.validate_config() # no raise
33+
34+
def test_raises_naming_every_missing_key(self, monkeypatch):
35+
_set_required(
36+
monkeypatch,
37+
SUPABASE_URL="",
38+
SUPABASE_SERVICE_KEY="",
39+
GEMINI_API_KEY="",
40+
SESSION_SECRET="",
41+
IS_LOCAL=False,
42+
)
43+
with pytest.raises(RuntimeError) as exc:
44+
config.validate_config()
45+
msg = str(exc.value)
46+
for key in ("SUPABASE_URL", "SUPABASE_SERVICE_KEY", "GEMINI_API_KEY", "SESSION_SECRET"):
47+
assert key in msg
48+
49+
def test_session_secret_required_outside_local(self, monkeypatch):
50+
_set_required(monkeypatch, SESSION_SECRET="", IS_LOCAL=False)
51+
with pytest.raises(RuntimeError) as exc:
52+
config.validate_config()
53+
assert "SESSION_SECRET" in str(exc.value)
54+
55+
def test_session_secret_relaxed_in_local(self, monkeypatch):
56+
_set_required(monkeypatch, SESSION_SECRET="", IS_LOCAL=True)
57+
config.validate_config() # no raise — relaxed for local dev
58+
59+
60+
class TestUnsignedOAuthFallbackFailsClosed:
61+
def test_fails_closed_in_production(self, monkeypatch):
62+
# No SESSION_SECRET + not local → must refuse (pre-fix: silently used
63+
# the unsigned in-memory store).
64+
monkeypatch.setattr(auth_mod, "SESSION_SECRET", "")
65+
monkeypatch.setattr(auth_mod, "IS_LOCAL", False)
66+
with pytest.raises(RuntimeError):
67+
auth_mod._encode_oauth_cookie({"n": "nonce-123", "v": "verifier"})
68+
69+
def test_allowed_in_local(self, monkeypatch):
70+
monkeypatch.setattr(auth_mod, "SESSION_SECRET", "")
71+
monkeypatch.setattr(auth_mod, "IS_LOCAL", True)
72+
out = auth_mod._encode_oauth_cookie({"n": "nonce-123", "v": "verifier"})
73+
# Unsigned payload (no "." separator) is permitted only in local dev.
74+
assert isinstance(out, str) and "." not in out
75+
76+
def test_signed_when_secret_present(self, monkeypatch):
77+
monkeypatch.setattr(auth_mod, "SESSION_SECRET", "supersecret-value")
78+
out = auth_mod._encode_oauth_cookie({"n": "nonce-123", "v": "verifier"})
79+
assert "." in out # payload_b64.sig_b64
80+
81+
def test_decode_refuses_unsigned_cookie_in_production(self, monkeypatch):
82+
# Symmetric decode-side guard: even a hand-built unsigned cookie must
83+
# not be honored outside local dev.
84+
import base64
85+
import json
86+
87+
unsigned = base64.urlsafe_b64encode(
88+
json.dumps({"n": "x", "v": "y"}).encode()
89+
).decode().rstrip("=")
90+
# Force the unsigned path (empty secret) in production mode.
91+
monkeypatch.setattr(auth_mod, "SESSION_SECRET", "")
92+
monkeypatch.setattr(auth_mod, "IS_LOCAL", False)
93+
assert auth_mod._decode_oauth_cookie(unsigned) is None

0 commit comments

Comments
 (0)