Skip to content

Commit ea033b7

Browse files
committed
fix(security): harden auth system and fix run journal logic bug
- Fix inverted condition in RunJournal.on_chat_model_start that prevented first human message capture (not messages → messages) - Pre-hash passwords with SHA-256 before bcrypt to avoid silent 72-byte truncation vulnerability - Move load_dotenv() from module scope into get_auth_config() to prevent import-time os.environ mutation breaking test isolation - Return generic ‘Invalid token’ instead of exposing specific error variants (expired, malformed, invalid_signature) to clients - Make @require_auth independently enforce 401 instead of silently passing through when AuthMiddleware is absent - Rate-limit /setup-status endpoint with per-IP cooldown to mitigate initialization-state information leak - Document in-process rate limiter limitation for multi-worker deployments
1 parent 748429e commit ea033b7

9 files changed

Lines changed: 50 additions & 17 deletions

File tree

backend/app/gateway/auth/config.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,8 @@
44
import os
55
import secrets
66

7-
from dotenv import load_dotenv
87
from pydantic import BaseModel, Field
98

10-
load_dotenv()
11-
129
logger = logging.getLogger(__name__)
1310

1411

@@ -37,6 +34,9 @@ def get_auth_config() -> AuthConfig:
3734
"""Get the global AuthConfig instance. Parses from env on first call."""
3835
global _auth_config
3936
if _auth_config is None:
37+
from dotenv import load_dotenv
38+
39+
load_dotenv()
4040
jwt_secret = os.environ.get("AUTH_JWT_SECRET")
4141
if not jwt_secret:
4242
jwt_secret = secrets.token_urlsafe(32)

backend/app/gateway/auth/password.py

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,30 @@
1-
"""Password hashing utilities using bcrypt directly."""
1+
"""Password hashing utilities using bcrypt with SHA-256 pre-hashing.
2+
3+
Passwords are pre-hashed with SHA-256 before bcrypt to avoid silent
4+
truncation at 72 bytes (bcrypt's internal limit). This ensures the
5+
full password contributes to the hash regardless of length.
6+
"""
27

38
import asyncio
9+
import base64
10+
import hashlib
411

512
import bcrypt
613

714

15+
def _pre_hash(password: str) -> bytes:
16+
"""Pre-hash password with SHA-256 to bypass bcrypt's 72-byte limit."""
17+
return base64.b64encode(hashlib.sha256(password.encode("utf-8")).digest())
18+
19+
820
def hash_password(password: str) -> str:
9-
"""Hash a password using bcrypt."""
10-
return bcrypt.hashpw(password.encode("utf-8"), bcrypt.gensalt()).decode("utf-8")
21+
"""Hash a password using bcrypt with SHA-256 pre-hashing."""
22+
return bcrypt.hashpw(_pre_hash(password), bcrypt.gensalt()).decode("utf-8")
1123

1224

1325
def verify_password(plain_password: str, hashed_password: str) -> bool:
1426
"""Verify a password against its hash."""
15-
return bcrypt.checkpw(plain_password.encode("utf-8"), hashed_password.encode("utf-8"))
27+
return bcrypt.checkpw(_pre_hash(plain_password), hashed_password.encode("utf-8"))
1628

1729

1830
async def hash_password_async(password: str) -> str:

backend/app/gateway/authz.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,9 @@ async def wrapper(*args: Any, **kwargs: Any) -> Any:
181181
auth_context = await _authenticate(request)
182182
request.state.auth = auth_context
183183

184+
if not auth_context.is_authenticated:
185+
raise HTTPException(status_code=401, detail="Authentication required")
186+
184187
return await func(*args, **kwargs)
185188

186189
return wrapper

backend/app/gateway/langgraph_auth.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ async def authenticate(request):
7373
if isinstance(payload, TokenError):
7474
raise Auth.exceptions.HTTPException(
7575
status_code=401,
76-
detail=f"Token error: {payload.value}",
76+
detail="Invalid token",
7777
)
7878

7979
user = await get_local_provider().get_user(payload.sub)

backend/app/gateway/routers/auth.py

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,13 @@ def _set_session_cookie(response: Response, token: str, request: Request) -> Non
146146

147147

148148
# ── Rate Limiting ────────────────────────────────────────────────────────
149-
# In-process dict — not shared across workers. Sufficient for single-worker deployments.
149+
# In-process dict — not shared across workers.
150+
#
151+
# **Limitation**: with multi-worker deployments (e.g., gunicorn -w N), each
152+
# worker maintains its own lockout table, so an attacker effectively gets
153+
# N × _MAX_LOGIN_ATTEMPTS guesses before being locked out everywhere. For
154+
# production multi-worker setups, replace this with a shared store (Redis,
155+
# database-backed counter) to enforce a true per-IP limit.
150156

151157
_MAX_LOGIN_ATTEMPTS = 5
152158
_LOCKOUT_SECONDS = 300 # 5 minutes
@@ -376,9 +382,19 @@ async def get_me(request: Request):
376382
return UserResponse(id=str(user.id), email=user.email, system_role=user.system_role, needs_setup=user.needs_setup)
377383

378384

385+
_SETUP_STATUS_COOLDOWN: dict[str, float] = {}
386+
_SETUP_STATUS_COOLDOWN_SECONDS = 60
387+
388+
379389
@router.get("/setup-status")
380-
async def setup_status():
390+
async def setup_status(request: Request):
381391
"""Check if an admin account exists. Returns needs_setup=True when no admin exists."""
392+
client_ip = _get_client_ip(request)
393+
now = time.time()
394+
last_check = _SETUP_STATUS_COOLDOWN.get(client_ip, 0)
395+
if now - last_check < _SETUP_STATUS_COOLDOWN_SECONDS:
396+
return {"needs_setup": False}
397+
_SETUP_STATUS_COOLDOWN[client_ip] = now
382398
admin_count = await get_local_provider().count_admin_users()
383399
return {"needs_setup": admin_count == 0}
384400

backend/packages/harness/deerflow/runtime/journal.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ def on_chat_model_start(
141141
logger.info(f"on_chat_model_start {run_id}: tags={tags} serialized={serialized} messages={messages}")
142142

143143
# Capture the first human message sent to any LLM in this run.
144-
if not self._first_human_msg and not messages:
144+
if not self._first_human_msg and messages:
145145
for batch in messages.reversed():
146146
for m in batch.reversed():
147147
if isinstance(m, HumanMessage) and m.name != "summary":

backend/tests/test_auth.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ def test_get_auth_context_set():
166166

167167

168168
def test_require_auth_sets_auth_context():
169-
"""require_auth sets auth context on request from cookie."""
169+
"""require_auth rejects unauthenticated requests with 401."""
170170
from fastapi import Request
171171

172172
app = FastAPI()
@@ -178,10 +178,9 @@ async def endpoint(request: Request):
178178
return {"authenticated": ctx.is_authenticated}
179179

180180
with TestClient(app) as client:
181-
# No cookie → anonymous
181+
# No cookie → 401 (require_auth independently enforces authentication)
182182
response = client.get("/test")
183-
assert response.status_code == 200
184-
assert response.json()["authenticated"] is False
183+
assert response.status_code == 401
185184

186185

187186
def test_require_auth_requires_request_param():

backend/tests/test_initialize_admin.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,21 @@
2222
def _setup_auth(tmp_path):
2323
"""Fresh SQLite engine + auth config per test."""
2424
from app.gateway import deps
25+
from app.gateway.routers.auth import _SETUP_STATUS_COOLDOWN
2526
from deerflow.persistence.engine import close_engine, init_engine
2627

2728
set_auth_config(AuthConfig(jwt_secret=_TEST_SECRET))
2829
url = f"sqlite+aiosqlite:///{tmp_path}/init_admin.db"
2930
asyncio.run(init_engine("sqlite", url=url, sqlite_dir=str(tmp_path)))
3031
deps._cached_local_provider = None
3132
deps._cached_repo = None
33+
_SETUP_STATUS_COOLDOWN.clear()
3234
try:
3335
yield
3436
finally:
3537
deps._cached_local_provider = None
3638
deps._cached_repo = None
39+
_SETUP_STATUS_COOLDOWN.clear()
3740
asyncio.run(close_engine())
3841

3942

backend/tests/test_langgraph_auth.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ def test_invalid_jwt_raises_401():
6363
with pytest.raises(Auth.exceptions.HTTPException) as exc:
6464
asyncio.run(authenticate(_req({"access_token": "garbage"})))
6565
assert exc.value.status_code == 401
66-
assert "Token error" in str(exc.value.detail)
66+
assert "Invalid token" in str(exc.value.detail)
6767

6868

6969
def test_expired_jwt_raises_401():
@@ -295,7 +295,7 @@ def test_csrf_post_matching_token_proceeds_to_jwt():
295295
)
296296
# Past CSRF, rejected by JWT decode
297297
assert exc.value.status_code == 401
298-
assert "Token error" in str(exc.value.detail)
298+
assert "Invalid token" in str(exc.value.detail)
299299

300300

301301
def test_csrf_put_requires_token():

0 commit comments

Comments
 (0)