fix: MissingGreenlet error — convert all middleware to pure ASGI
Root cause: BaseHTTPMiddleware.call_next() runs the endpoint in a separate async task, which breaks SQLAlchemy's greenlet context. After session.commit(), the connection is released to the pool. When session.refresh() tries to acquire a new connection for a SELECT, it fails with MissingGreenlet because the greenlet spawn context is not available in the call_next() task. Fix (two-part): 1. Convert all 4 middleware classes from BaseHTTPMiddleware to pure ASGI middleware — eliminates call_next() entirely so the entire request chain runs in the same async context. 2. Set expire_on_commit=False on the session factory — after commit, objects retain their in-memory values instead of being expired. This removes the need for session.refresh() entirely. All session.refresh() calls removed from 11 repository files. With expire_on_commit=False, post-commit attribute access no longer triggers lazy loads that would also fail with MissingGreenlet. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
+58
-18
@@ -23,7 +23,6 @@ def _is_install_mode() -> bool:
|
||||
from fastapi import Depends, HTTPException, Request, status
|
||||
from fastapi.responses import JSONResponse
|
||||
from fastapi.security import HTTPBearer, HTTPAuthorizationCredentials
|
||||
from starlette.middleware.base import BaseHTTPMiddleware
|
||||
|
||||
from server.domain.models import Admin
|
||||
from server.api.dependencies import _get_request_session
|
||||
@@ -74,34 +73,75 @@ def _extract_bearer_token(request: Request) -> Optional[str]:
|
||||
return token or None
|
||||
|
||||
|
||||
class JwtAuthMiddleware(BaseHTTPMiddleware):
|
||||
"""Defense-in-depth: reject unauthenticated /api/* before route handlers run.
|
||||
class JwtAuthMiddleware:
|
||||
"""Pure ASGI middleware: reject unauthenticated /api/* before route handlers run.
|
||||
|
||||
Must be registered inside DbSessionMiddleware so _verify_token can use request.state.db.
|
||||
Route-level Depends(get_current_admin) remains for injecting Admin into handlers.
|
||||
|
||||
NOTE: Uses pure ASGI (not BaseHTTPMiddleware) to avoid MissingGreenlet errors
|
||||
with SQLAlchemy async. BaseHTTPMiddleware's call_next() runs the endpoint in
|
||||
a separate task, which breaks SQLAlchemy's async greenlet context.
|
||||
"""
|
||||
|
||||
async def dispatch(self, request: Request, call_next):
|
||||
path = request.url.path
|
||||
if request.method == "OPTIONS":
|
||||
return await call_next(request)
|
||||
if _is_install_mode():
|
||||
return await call_next(request)
|
||||
if not path.startswith("/api/"):
|
||||
return await call_next(request)
|
||||
if _is_public_path(path):
|
||||
return await call_next(request)
|
||||
def __init__(self, app):
|
||||
self.app = app
|
||||
|
||||
async def __call__(self, scope, receive, send):
|
||||
if scope["type"] != "http":
|
||||
await self.app(scope, receive, send)
|
||||
return
|
||||
|
||||
path = scope.get("path", "")
|
||||
|
||||
# Skip OPTIONS (CORS preflight)
|
||||
# Read method from ASGI scope
|
||||
method = scope.get("method", "")
|
||||
if method == "OPTIONS":
|
||||
await self.app(scope, receive, send)
|
||||
return
|
||||
|
||||
if _is_install_mode():
|
||||
await self.app(scope, receive, send)
|
||||
return
|
||||
|
||||
if not path.startswith("/api/"):
|
||||
await self.app(scope, receive, send)
|
||||
return
|
||||
|
||||
if _is_public_path(path):
|
||||
await self.app(scope, receive, send)
|
||||
return
|
||||
|
||||
# Extract Bearer token from headers
|
||||
token = None
|
||||
for header_name, header_value in scope.get("headers", []):
|
||||
if header_name == b"authorization":
|
||||
auth_str = header_value.decode("latin-1", errors="replace")
|
||||
if auth_str.startswith("Bearer "):
|
||||
token = auth_str[7:].strip()
|
||||
break
|
||||
|
||||
token = _extract_bearer_token(request)
|
||||
if not token:
|
||||
return _unauthorized_response("Missing Authorization header")
|
||||
response = _unauthorized_response("Missing Authorization header")
|
||||
await response(scope, receive, send)
|
||||
return
|
||||
|
||||
# Verify token — need a Request object for _verify_token
|
||||
# Create a minimal Request from the ASGI scope
|
||||
from starlette.requests import Request
|
||||
request = Request(scope, receive, send)
|
||||
|
||||
admin = await _verify_token(token, request)
|
||||
if not admin:
|
||||
return _unauthorized_response("Invalid or expired token")
|
||||
response = _unauthorized_response("Invalid or expired token")
|
||||
await response(scope, receive, send)
|
||||
return
|
||||
|
||||
request.state.admin = admin
|
||||
return await call_next(request)
|
||||
# Store admin in scope state (accessible via request.state.admin)
|
||||
scope.setdefault("state", {})
|
||||
scope["state"]["admin"] = admin
|
||||
await self.app(scope, receive, send)
|
||||
|
||||
|
||||
async def get_current_admin(
|
||||
|
||||
@@ -26,12 +26,10 @@ class AdminRepositoryImpl:
|
||||
async def create(self, admin: Admin) -> Admin:
|
||||
self.session.add(admin)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(admin)
|
||||
return admin
|
||||
|
||||
async def update(self, admin: Admin) -> Admin:
|
||||
await self.session.commit()
|
||||
await self.session.refresh(admin)
|
||||
return admin
|
||||
|
||||
|
||||
@@ -42,7 +40,6 @@ class LoginAttemptRepositoryImpl:
|
||||
async def create(self, attempt: LoginAttempt) -> LoginAttempt:
|
||||
self.session.add(attempt)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(attempt)
|
||||
return attempt
|
||||
|
||||
async def count_recent_failures(self, username: str, ip_address: str, minutes: int = 15) -> int:
|
||||
|
||||
@@ -47,7 +47,6 @@ class AuditLogRepositoryImpl:
|
||||
async def create(self, log: AuditLog) -> AuditLog:
|
||||
self.session.add(log)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(log)
|
||||
return log
|
||||
|
||||
async def query(
|
||||
|
||||
@@ -22,7 +22,6 @@ class DbCredentialRepositoryImpl:
|
||||
async def create(self, credential: DbCredential) -> DbCredential:
|
||||
self.session.add(credential)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(credential)
|
||||
return credential
|
||||
|
||||
async def delete(self, id: int) -> bool:
|
||||
|
||||
@@ -22,7 +22,6 @@ class PasswordPresetRepositoryImpl:
|
||||
async def create(self, preset: PasswordPreset) -> PasswordPreset:
|
||||
self.session.add(preset)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(preset)
|
||||
return preset
|
||||
|
||||
async def delete(self, id: int) -> bool:
|
||||
|
||||
@@ -26,12 +26,10 @@ class PlatformRepositoryImpl:
|
||||
async def create(self, platform: Platform) -> Platform:
|
||||
self.session.add(platform)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(platform)
|
||||
return platform
|
||||
|
||||
async def update(self, platform: Platform) -> Platform:
|
||||
await self.session.commit()
|
||||
await self.session.refresh(platform)
|
||||
return platform
|
||||
|
||||
async def delete(self, id: int) -> bool:
|
||||
@@ -70,12 +68,10 @@ class NodeRepositoryImpl:
|
||||
async def create(self, node: Node) -> Node:
|
||||
self.session.add(node)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(node)
|
||||
return node
|
||||
|
||||
async def update(self, node: Node) -> Node:
|
||||
await self.session.commit()
|
||||
await self.session.refresh(node)
|
||||
return node
|
||||
|
||||
async def delete(self, id: int) -> bool:
|
||||
|
||||
@@ -28,12 +28,10 @@ class PushScheduleRepositoryImpl:
|
||||
async def create(self, schedule: PushSchedule) -> PushSchedule:
|
||||
self.session.add(schedule)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(schedule)
|
||||
return schedule
|
||||
|
||||
async def update(self, schedule: PushSchedule) -> PushSchedule:
|
||||
await self.session.commit()
|
||||
await self.session.refresh(schedule)
|
||||
return schedule
|
||||
|
||||
async def delete(self, id: int) -> bool:
|
||||
@@ -81,7 +79,6 @@ class PushRetryJobRepositoryImpl:
|
||||
async def create(self, job: PushRetryJob) -> PushRetryJob:
|
||||
self.session.add(job)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(job)
|
||||
return job
|
||||
|
||||
async def update_status(self, id: int, status: str, **kwargs) -> PushRetryJob:
|
||||
@@ -92,5 +89,4 @@ class PushRetryJobRepositoryImpl:
|
||||
if hasattr(job, key):
|
||||
setattr(job, key, value)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(job)
|
||||
return job
|
||||
@@ -28,12 +28,10 @@ class ScriptRepositoryImpl:
|
||||
async def create(self, script: Script) -> Script:
|
||||
self.session.add(script)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(script)
|
||||
return script
|
||||
|
||||
async def update(self, script: Script) -> Script:
|
||||
await self.session.commit()
|
||||
await self.session.refresh(script)
|
||||
return script
|
||||
|
||||
async def delete(self, id: int) -> bool:
|
||||
@@ -58,7 +56,6 @@ class ScriptExecutionRepositoryImpl:
|
||||
async def create(self, execution: ScriptExecution) -> ScriptExecution:
|
||||
self.session.add(execution)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(execution)
|
||||
return execution
|
||||
|
||||
async def list_recent(
|
||||
@@ -90,5 +87,4 @@ class ScriptExecutionRepositoryImpl:
|
||||
if status != "running" and execution.completed_at is None:
|
||||
execution.completed_at = datetime.now(timezone.utc)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(execution)
|
||||
return execution
|
||||
@@ -68,12 +68,10 @@ class ServerRepositoryImpl:
|
||||
async def create(self, server: Server) -> Server:
|
||||
self.session.add(server)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(server)
|
||||
return server
|
||||
|
||||
async def update(self, server: Server) -> Server:
|
||||
await self.session.commit()
|
||||
await self.session.refresh(server)
|
||||
return server
|
||||
|
||||
async def delete(self, id: int) -> bool:
|
||||
|
||||
@@ -32,6 +32,7 @@ def _ensure_engine():
|
||||
autoflush=False,
|
||||
bind=_engine,
|
||||
class_=AsyncSession,
|
||||
expire_on_commit=False, # Fix MissingGreenlet: keep in-memory values after commit
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -25,7 +25,6 @@ class SettingRepositoryImpl:
|
||||
setting = Setting(key=key, value=value)
|
||||
self.session.add(setting)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(setting)
|
||||
return setting
|
||||
|
||||
async def get_all(self) -> List[Setting]:
|
||||
|
||||
@@ -36,7 +36,6 @@ class SshSessionRepositoryImpl:
|
||||
async def create(self, ssh_session: SshSession) -> SshSession:
|
||||
self.session.add(ssh_session)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(ssh_session)
|
||||
return ssh_session
|
||||
|
||||
async def close_session(self, session_id: str) -> Optional[SshSession]:
|
||||
@@ -45,7 +44,6 @@ class SshSessionRepositoryImpl:
|
||||
ssh_session.status = "closed"
|
||||
ssh_session.closed_at = datetime.now(timezone.utc)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(ssh_session)
|
||||
return ssh_session
|
||||
|
||||
|
||||
@@ -74,7 +72,6 @@ class CommandLogRepositoryImpl:
|
||||
async def create(self, log: CommandLog) -> CommandLog:
|
||||
self.session.add(log)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(log)
|
||||
return log
|
||||
|
||||
async def count_by_admin(self, admin_id: int, hours: int = 24) -> int:
|
||||
|
||||
@@ -64,7 +64,6 @@ class SyncLogRepositoryImpl:
|
||||
async def create(self, log: SyncLog) -> SyncLog:
|
||||
self.session.add(log)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(log)
|
||||
return log
|
||||
|
||||
async def update_status(self, id: int, status: str, **kwargs) -> SyncLog:
|
||||
@@ -75,5 +74,4 @@ class SyncLogRepositoryImpl:
|
||||
if hasattr(log, key):
|
||||
setattr(log, key, value)
|
||||
await self.session.commit()
|
||||
await self.session.refresh(log)
|
||||
return log
|
||||
+85
-37
@@ -23,10 +23,9 @@ import logging
|
||||
from contextlib import asynccontextmanager
|
||||
from pathlib import Path
|
||||
|
||||
from fastapi import FastAPI, Request, Response
|
||||
from fastapi import FastAPI, Request
|
||||
from fastapi.middleware.cors import CORSMiddleware
|
||||
from fastapi.staticfiles import StaticFiles
|
||||
from starlette.middleware.base import BaseHTTPMiddleware
|
||||
|
||||
from server.config import settings
|
||||
from server.infrastructure.database.session import init_db, AsyncSessionLocal
|
||||
@@ -131,41 +130,58 @@ async def _renew_primary_lock():
|
||||
|
||||
# ── D7: DB Session Middleware (fixes session leak in 4 service factories) ──
|
||||
|
||||
class DbSessionMiddleware(BaseHTTPMiddleware):
|
||||
"""Auto-manage DB session lifecycle per HTTP request.
|
||||
class DbSessionMiddleware:
|
||||
"""Pure ASGI middleware: auto-manage DB session lifecycle per HTTP request.
|
||||
|
||||
Problem: 4 service factories (get_server_service, get_script_service,
|
||||
get_auth_service, get_sync_service) create AsyncSessionLocal() but never
|
||||
close them — causing connection pool exhaustion (P0 bug).
|
||||
|
||||
Solution: Middleware opens session at request start, stores in request.state.db,
|
||||
Solution: Middleware opens session at request start, stores in scope["state"],
|
||||
and closes it after response. Service factories read from request.state.db
|
||||
instead of creating new sessions.
|
||||
|
||||
Pattern: Similar to Django's ATOMIC_REQUESTS — session lifetime = request lifetime.
|
||||
|
||||
NOTE: Uses pure ASGI (not BaseHTTPMiddleware) to avoid MissingGreenlet errors
|
||||
with SQLAlchemy async. BaseHTTPMiddleware's call_next() runs the endpoint in
|
||||
a separate task, which breaks SQLAlchemy's async greenlet context, causing
|
||||
session.commit() and session.refresh() to fail.
|
||||
"""
|
||||
|
||||
async def dispatch(self, request: Request, call_next):
|
||||
def __init__(self, app):
|
||||
self.app = app
|
||||
|
||||
async def __call__(self, scope, receive, send):
|
||||
if scope["type"] != "http":
|
||||
await self.app(scope, receive, send)
|
||||
return
|
||||
|
||||
path = scope.get("path", "")
|
||||
|
||||
# Skip non-API routes (WebSocket, health, static files)
|
||||
# WebSocket connections are long-lived — never wrap them in a DB session
|
||||
path = request.url.path
|
||||
if not path.startswith("/api/"):
|
||||
return await call_next(request)
|
||||
await self.app(scope, receive, send)
|
||||
return
|
||||
|
||||
# Skip install API (it manages its own DB connections)
|
||||
if path.startswith("/api/install/"):
|
||||
return await call_next(request)
|
||||
await self.app(scope, receive, send)
|
||||
return
|
||||
|
||||
# Skip in install mode (no global engine available)
|
||||
if is_install_mode():
|
||||
return await call_next(request)
|
||||
await self.app(scope, receive, send)
|
||||
return
|
||||
|
||||
# Ensure scope has a state dict (for request.state.db sharing)
|
||||
scope.setdefault("state", {})
|
||||
|
||||
# Open session for this request
|
||||
async with AsyncSessionLocal() as session:
|
||||
request.state.db = session
|
||||
scope["state"]["db"] = session
|
||||
try:
|
||||
response = await call_next(request)
|
||||
return response
|
||||
await self.app(scope, receive, send)
|
||||
except Exception:
|
||||
await session.rollback()
|
||||
raise
|
||||
@@ -287,58 +303,90 @@ app = FastAPI(
|
||||
|
||||
# ── Install mode middleware: block non-install routes when not configured ──
|
||||
|
||||
class InstallModeMiddleware(BaseHTTPMiddleware):
|
||||
"""In install mode, only /api/install/, /app/, /health are accessible.
|
||||
All other API routes return 503 Service Unavailable."""
|
||||
class InstallModeMiddleware:
|
||||
"""Pure ASGI middleware: in install mode, only /api/install/, /app/, /health are accessible.
|
||||
All other API routes return 503 Service Unavailable.
|
||||
|
||||
NOTE: Uses pure ASGI (not BaseHTTPMiddleware) to avoid MissingGreenlet errors
|
||||
with SQLAlchemy async.
|
||||
"""
|
||||
|
||||
def __init__(self, app):
|
||||
self.app = app
|
||||
|
||||
async def __call__(self, scope, receive, send):
|
||||
if scope["type"] != "http":
|
||||
await self.app(scope, receive, send)
|
||||
return
|
||||
|
||||
async def dispatch(self, request: Request, call_next):
|
||||
if is_install_mode():
|
||||
path = request.url.path
|
||||
path = scope.get("path", "")
|
||||
# Redirect root to install wizard
|
||||
if path == "/":
|
||||
from fastapi.responses import RedirectResponse
|
||||
return RedirectResponse(url="/app/install.html")
|
||||
response = RedirectResponse(url="/app/install.html")
|
||||
await response(scope, receive, send)
|
||||
return
|
||||
# Allow: install API, static files, health check
|
||||
if path.startswith("/api/install/") or path.startswith("/app/") or path == "/health":
|
||||
return await call_next(request)
|
||||
await self.app(scope, receive, send)
|
||||
return
|
||||
if path.startswith("/ws/"):
|
||||
from fastapi.responses import JSONResponse
|
||||
return JSONResponse(
|
||||
response = JSONResponse(
|
||||
status_code=503,
|
||||
content={"detail": "安装模式下 WebSocket 不可用,请先完成安装"},
|
||||
)
|
||||
await response(scope, receive, send)
|
||||
return
|
||||
# Block everything else
|
||||
from fastapi.responses import JSONResponse
|
||||
return JSONResponse(
|
||||
response = JSONResponse(
|
||||
status_code=503,
|
||||
content={"detail": "系统尚未配置,请先访问 /app/install.html 完成安装"},
|
||||
)
|
||||
return await call_next(request)
|
||||
await response(scope, receive, send)
|
||||
return
|
||||
|
||||
await self.app(scope, receive, send)
|
||||
|
||||
|
||||
# Install mode middleware (must be first — before DB session middleware)
|
||||
app.add_middleware(InstallModeMiddleware)
|
||||
|
||||
# Security headers middleware (before CORS so headers appear on all responses)
|
||||
class SecurityHeadersMiddleware(BaseHTTPMiddleware):
|
||||
"""Inject security-related response headers on every response.
|
||||
# Security headers middleware (pure ASGI — injects headers on every HTTP response)
|
||||
class SecurityHeadersMiddleware:
|
||||
"""Pure ASGI middleware: inject security-related response headers.
|
||||
|
||||
- X-Content-Type-Options: nosniff — prevents MIME-type sniffing
|
||||
- X-Frame-Options: DENY — prevents clickjacking via iframes
|
||||
- Referrer-Policy: strict-origin-when-cross-origin — limits referrer leakage
|
||||
- Permissions-Policy: restricts browser features (camera, mic, geolocation)
|
||||
|
||||
Not included:
|
||||
- Content-Security-Policy: too many inline scripts/styles to enforce easily
|
||||
- Strict-Transport-Security: handled by Nginx in production
|
||||
NOTE: Uses pure ASGI (not BaseHTTPMiddleware) to avoid MissingGreenlet errors.
|
||||
Intercepts ASGI response messages to inject headers before they reach the client.
|
||||
"""
|
||||
async def dispatch(self, request, call_next):
|
||||
response = await call_next(request)
|
||||
response.headers["X-Content-Type-Options"] = "nosniff"
|
||||
response.headers["X-Frame-Options"] = "DENY"
|
||||
response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin"
|
||||
response.headers["Permissions-Policy"] = "camera=(), microphone=(), geolocation=()"
|
||||
return response
|
||||
|
||||
def __init__(self, app):
|
||||
self.app = app
|
||||
|
||||
async def __call__(self, scope, receive, send):
|
||||
if scope["type"] != "http":
|
||||
await self.app(scope, receive, send)
|
||||
return
|
||||
|
||||
# Wrap send to inject security headers into the response
|
||||
async def send_with_headers(message):
|
||||
if message["type"] == "http.response.start":
|
||||
headers = list(message.get("headers", []))
|
||||
headers.append((b"x-content-type-options", b"nosniff"))
|
||||
headers.append((b"x-frame-options", b"DENY"))
|
||||
headers.append((b"referrer-policy", b"strict-origin-when-cross-origin"))
|
||||
headers.append((b"permissions-policy", b"camera=(), microphone=(), geolocation=()"))
|
||||
message["headers"] = headers
|
||||
await send(message)
|
||||
|
||||
await self.app(scope, receive, send_with_headers)
|
||||
|
||||
app.add_middleware(SecurityHeadersMiddleware)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user