diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml new file mode 100644 index 0000000..1a5ed64 --- /dev/null +++ b/.github/workflows/security.yml @@ -0,0 +1,28 @@ +name: security + +on: + pull_request: + push: + +jobs: + security-tests: + runs-on: ubuntu-latest + defaults: + run: + working-directory: backend + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.11" + + - name: Install backend dependencies + run: | + python -m pip install --upgrade pip + python -m pip install -r requirements.txt + + - name: Run security regression tests + run: python -m pytest -m security diff --git a/.gitignore b/.gitignore index 1ab50fc..a678e8b 100644 --- a/.gitignore +++ b/.gitignore @@ -32,7 +32,6 @@ htmlcov/ .nox/ coverage/ .hypothesis/ -backend/tests # SQLite database files *.sqlite diff --git a/backend/app/api/v1/endpoints/auth.py b/backend/app/api/v1/endpoints/auth.py index 6ad7fd8..6bcd627 100644 --- a/backend/app/api/v1/endpoints/auth.py +++ b/backend/app/api/v1/endpoints/auth.py @@ -28,6 +28,27 @@ logger = logging.getLogger(__name__) +def _token_debug_enabled() -> bool: + return settings.APP_ENV.lower() in {"dev", "development", "test", "local"} + + +def _token_preview(token: Optional[str], keep: int = 6) -> str: + if not token: + return "None" + if len(token) <= keep * 2: + return "***" + return f"{token[:keep]}...{token[-keep:]}" + + +def _require_dev_mode() -> None: + env = settings.APP_ENV.lower() + if env not in {"dev", "development", "test", "local"}: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail="Endpoint not available", + ) + + def _log_auth_event_background(request: Request, event_type: str, success: bool, user_id: str = None, reason: str = None): """Schedule audit log write in background.""" import asyncio @@ -57,6 +78,8 @@ async def _write(): def log_token_info(token, token_type="access", mask=True): """Log token information for debugging purposes.""" try: + if not _token_debug_enabled(): + return # Decode without verification to extract payload for logging parts = token.split(".") if len(parts) != 3: @@ -89,10 +112,10 @@ def log_token_info(token, token_type="access", mask=True): ) # Mask the token for security - if mask and token: - token_preview = f"{token[:10]}...{token[-5:]}" if len(token) > 15 else "***" + if mask: + token_preview = _token_preview(token, keep=10) else: - token_preview = token + token_preview = "***" logger.info( f"{token_type.capitalize()} token info: " @@ -106,15 +129,17 @@ def log_token_info(token, token_type="access", mask=True): def get_cookie_options(path: str = "/") -> dict: """Get cookie options based on environment.""" # Determine if we're in development or production - is_dev = settings.APP_ENV.lower() == "dev" + is_dev = settings.APP_ENV.lower() in {"dev", "development", "test", "local"} + secure = settings.COOKIE_SECURE if settings.COOKIE_SECURE is not None else not is_dev + samesite = (settings.COOKIE_SAMESITE or "lax").lower() + if samesite == "none": + secure = True return { "key": "refresh_token", "httponly": True, # Prevent JavaScript access - "secure": not is_dev, # Only set secure=True in production (HTTPS) - "samesite": ( - "lax" if is_dev else "none" - ), # Use 'lax' for dev, 'none' for production + "secure": secure, + "samesite": samesite, "max_age": settings.REFRESH_TOKEN_EXPIRE_DAYS * 24 * 60 @@ -190,6 +215,8 @@ def clear_refresh_token_systematically(response: Response, request: Request): def log_token_validation(token: str, user_id: str, source: str, result: str): """Enhanced logging for token validation""" try: + if not _token_debug_enabled(): + return # Import json and base64 here to ensure they're available in this scope import json import base64 @@ -362,8 +389,14 @@ async def login( # Log detailed information about the token being stored logger.info(f"Storing refresh token in database for user {user.email}") - logger.info(f"Token (first 10 chars): {refresh_token[:10]}") - logger.info(f"Token length: {len(refresh_token)}") + if _token_debug_enabled(): + logger.debug( + "Refresh token stored", + extra={ + "token_preview": _token_preview(refresh_token), + "token_length": len(refresh_token), + }, + ) logger.info(f"Token expires: {user.refresh_token_expires}") # Save the user with the new refresh token @@ -375,12 +408,10 @@ async def login( logger.info(f"Refresh token successfully stored in database") else: logger.error(f"Failed to store refresh token in database") - if updated_user: - logger.error( - f"Stored token (first 10 chars): {updated_user.refresh_token[:10] if updated_user.refresh_token else 'None'}" - ) - logger.error( - f"Stored token length: {len(updated_user.refresh_token) if updated_user.refresh_token else 0}" + if updated_user and _token_debug_enabled(): + logger.debug( + "Stored refresh token mismatch", + extra={"token_length": len(updated_user.refresh_token) if updated_user.refresh_token else 0}, ) # Set refresh token cookie @@ -461,21 +492,22 @@ async def refresh_token( """Refresh access token using a valid refresh token from HTTP-only cookie.""" # EMERGENCY FIX: Wrap the entire function in a try-except block to ensure it always returns a response try: - # Log all cookies for debugging - logger.info(f"Cookies in request: {request.cookies}") + if _token_debug_enabled(): + logger.debug("Cookies received", extra={"cookie_keys": list(request.cookies.keys())}) # Get refresh token from cookie refresh_token = request.cookies.get("refresh_token") - # DETAILED TOKEN LOGGING - if refresh_token: - logger.info(f"๐Ÿ” REFRESH TOKEN RECEIVED:") - logger.info(f" Token (first 50 chars): {refresh_token[:50]}...") - logger.info(f" Token (last 50 chars): ...{refresh_token[-50:]}") - logger.info(f" Token length: {len(refresh_token)}") - logger.info(f" Full token: {refresh_token}") - else: - logger.info("โŒ NO REFRESH TOKEN IN REQUEST") + if refresh_token and _token_debug_enabled(): + logger.debug( + "Refresh token received", + extra={ + "token_preview": _token_preview(refresh_token, keep=8), + "token_length": len(refresh_token), + }, + ) + elif not refresh_token: + logger.info("No refresh token in request") # NUCLEAR OPTION: Block the specific problematic token immediately BLOCKED_TOKEN = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJiOTExODgwMTMyNWQ0MDdmYWIzM2E2Zjc5YmQyNGRhZCIsInJlZnJlc2giOnRydWUsImlhdCI6MTc1NTQ2NjQxMC41MzM1MDEsImV4cCI6MTc1ODA0NDAxMH0.0lVRx8qHILYv3IaaaMWNLDdKx_5ANTp4vMiAGuC_Hzg" @@ -744,14 +776,7 @@ async def refresh_token( ) # Verify that the refresh token matches what's in the database - # Log detailed token information for debugging - logger.info(f"Comparing tokens:") - logger.info( - f"DB token (first 10 chars): {user.refresh_token[:10] if user.refresh_token else 'None'}" - ) - logger.info( - f"Request token (first 10 chars): {refresh_token[:10] if refresh_token else 'None'}" - ) + logger.info("Comparing refresh token against stored value") # Check if the user has a refresh token in the database if not user.refresh_token: @@ -767,9 +792,6 @@ async def refresh_token( logger.info(f"Updated user record with token from request") elif user.refresh_token != refresh_token: logger.warning(f"Token mismatch between database and request") - logger.warning( - f"DB token length: {len(user.refresh_token)}, Request token length: {len(refresh_token)}" - ) # For now, update the database with the token from the request # This is a temporary fix to help diagnose the issue @@ -867,8 +889,14 @@ async def refresh_token( logger.info( f"New refresh token generated and about to be stored in database" ) - logger.info(f"New token (first 10 chars): {new_refresh_token[:10]}") - logger.info(f"New token length: {len(new_refresh_token)}") + if _token_debug_enabled(): + logger.debug( + "New refresh token metadata", + extra={ + "token_preview": _token_preview(new_refresh_token), + "token_length": len(new_refresh_token), + }, + ) # Save the user with the new refresh token await user.save(db) @@ -879,7 +907,6 @@ async def refresh_token( logger.info( "Continuing despite database error (tokens will still be returned to client)" ) - logger.info(f"New token length: {len(new_refresh_token)}") logger.info(f"Token expires at: {expiry_time.isoformat()}") logger.info(f"Token lifetime: {settings.REFRESH_TOKEN_EXPIRE_DAYS} days") @@ -892,12 +919,10 @@ async def refresh_token( logger.info(f"New refresh token successfully stored in database") else: logger.error(f"Failed to store new refresh token in database") - if updated_user: - logger.error( - f"Stored token (first 10 chars): {updated_user.refresh_token[:10] if updated_user.refresh_token else 'None'}" - ) - logger.error( - f"Stored token length: {len(updated_user.refresh_token) if updated_user.refresh_token else 0}" + if updated_user and _token_debug_enabled(): + logger.debug( + "Stored refresh token mismatch after rotation", + extra={"token_length": len(updated_user.refresh_token) if updated_user.refresh_token else 0}, ) # Set new refresh token cookie - use root path to ensure cookie is sent with all requests @@ -1195,6 +1220,7 @@ async def update_password( @router.post("/nuclear-clear-cookies") async def nuclear_clear_cookies(request: Request, response: Response): """ + _require_dev_mode() Nuclear option: Clear ALL cookies with every possible domain/path combination. This addresses shared development environment cookie pollution issues. """ @@ -1338,6 +1364,7 @@ async def force_logout_clear(request: Request, response: Response): Force logout and aggressive cookie clearing with cache-busting headers. This is the nuclear option for persistent cookie issues. """ + _require_dev_mode() logger.info("๐Ÿšจ FORCE LOGOUT AND CLEAR INITIATED") @@ -1488,6 +1515,7 @@ async def force_logout_clear(request: Request, response: Response): @router.post("/test-cookie-setting") async def test_cookie_setting(response: Response): """Test endpoint to verify cookie setting works""" + _require_dev_mode() logger.info("๐Ÿงช Testing cookie setting...") diff --git a/backend/app/api/v1/endpoints/navigation_routes.py b/backend/app/api/v1/endpoints/navigation_routes.py index 99f9ec1..88763da 100644 --- a/backend/app/api/v1/endpoints/navigation_routes.py +++ b/backend/app/api/v1/endpoints/navigation_routes.py @@ -565,8 +565,8 @@ async def delete_navigation_route( print(f"Attempting to delete route with ID: {route_id_str}") # Get existing route using raw SQL to avoid ORM issues - query = text(f"SELECT * FROM navigation_routes WHERE id = '{route_id_str}'") - result = await db.execute(query) + query = text("SELECT * FROM navigation_routes WHERE id = :route_id") + result = await db.execute(query, {"route_id": route_id_str}) route_data = result.fetchone() if not route_data: @@ -584,8 +584,8 @@ async def delete_navigation_route( ) # Check if route has associated pages using raw SQL - pages_query = text(f"SELECT COUNT(*) FROM pages WHERE navigation_route_id = '{route_id_str}'") - pages_result = await db.execute(pages_query) + pages_query = text("SELECT COUNT(*) FROM pages WHERE navigation_route_id = :route_id") + pages_result = await db.execute(pages_query, {"route_id": route_id_str}) page_count = pages_result.scalar() if page_count > 0: @@ -595,8 +595,8 @@ async def delete_navigation_route( ) # Delete route using raw SQL - delete_query = text(f"DELETE FROM navigation_routes WHERE id = '{route_id_str}'") - await db.execute(delete_query) + delete_query = text("DELETE FROM navigation_routes WHERE id = :route_id") + await db.execute(delete_query, {"route_id": route_id_str}) await db.commit() print(f"Successfully deleted route with ID: {route_id_str}") @@ -612,4 +612,3 @@ async def delete_navigation_route( detail=f"Failed to delete navigation route: {str(e)}" ) - diff --git a/backend/app/api/v1/endpoints/settings.py b/backend/app/api/v1/endpoints/settings.py index 820bd2d..0909cba 100644 --- a/backend/app/api/v1/endpoints/settings.py +++ b/backend/app/api/v1/endpoints/settings.py @@ -116,6 +116,29 @@ def _mask_key(payload: dict, field_name: str, api_key: str, *, prefix: str, min_ return value +def _normalize_scope(scope_value): + if isinstance(scope_value, SettingScope): + return scope_value + if isinstance(scope_value, str): + scope_value = scope_value.strip() + if not scope_value: + return None + try: + return SettingScope(scope_value) + except ValueError: + return None + return None + + +def _enforce_instance_ownership(instance: dict, auth: Optional[AuthContext]) -> None: + if instance.get("user_id"): + if not auth: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Authentication required for user settings" + ) + ensure_setting_instance_belongs_to_user(instance, auth) + async def get_definition_by_id(db, definition_id: str): """Helper function to get a setting definition by ID using direct SQL.""" query = text(""" @@ -1258,10 +1281,13 @@ async def get_setting_instance( "created_at": row[7], "updated_at": row[8] } + + _enforce_instance_ownership(instance, auth) # Check access permission scope_value = instance["scope"] - if scope_value in [SettingScope.USER.value, SettingScope.USER_PAGE.value]: + scope_enum = _normalize_scope(scope_value) + if scope_enum in [SettingScope.USER, SettingScope.USER_PAGE]: if not auth: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, @@ -1269,7 +1295,7 @@ async def get_setting_instance( ) # Verify ownership using service helper ensure_setting_instance_belongs_to_user(instance, auth) - elif scope_value == SettingScope.SYSTEM.value: + elif scope_enum == SettingScope.SYSTEM: if not auth or not auth.is_admin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, @@ -1321,17 +1347,20 @@ async def update_setting_instance( "id": row[0], "definition_id": row[1], "name": row[2], - "value": json.loads(row[3]) if row[3] else None, + "value": row[3], "scope": row[4], "user_id": row[5], "page_id": row[6], "created_at": row[7], "updated_at": row[8] } + + _enforce_instance_ownership(instance, auth) # Check access permission scope_value = instance["scope"] - if scope_value in [SettingScope.USER.value, SettingScope.USER_PAGE.value]: + scope_enum = _normalize_scope(scope_value) + if scope_enum in [SettingScope.USER, SettingScope.USER_PAGE]: if not auth: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, @@ -1339,7 +1368,7 @@ async def update_setting_instance( ) # Verify ownership using service helper ensure_setting_instance_belongs_to_user(instance, auth) - elif scope_value == SettingScope.SYSTEM.value: + elif scope_enum == SettingScope.SYSTEM: if not auth or not auth.is_admin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, @@ -1449,17 +1478,20 @@ async def put_setting_instance( "id": row[0], "definition_id": row[1], "name": row[2], - "value": json.loads(row[3]) if row[3] else None, + "value": row[3], "scope": row[4], "user_id": row[5], "page_id": row[6], "created_at": row[7], "updated_at": row[8] } + + _enforce_instance_ownership(instance, auth) # Check access permission scope_value = instance["scope"] - if scope_value in [SettingScope.USER.value, SettingScope.USER_PAGE.value]: + scope_enum = _normalize_scope(scope_value) + if scope_enum in [SettingScope.USER, SettingScope.USER_PAGE]: if not auth: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, @@ -1467,7 +1499,7 @@ async def put_setting_instance( ) # Verify ownership using service helper ensure_setting_instance_belongs_to_user(instance, auth) - elif scope_value == SettingScope.SYSTEM.value: + elif scope_enum == SettingScope.SYSTEM: if not auth or not auth.is_admin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, @@ -1588,17 +1620,20 @@ async def delete_setting_instance( "id": row[0], "definition_id": row[1], "name": row[2], - "value": json.loads(row[3]) if row[3] else None, + "value": row[3], "scope": row[4], "user_id": row[5], "page_id": row[6], "created_at": row[7], "updated_at": row[8] } + + _enforce_instance_ownership(instance, auth) # Check access permission scope_value = instance["scope"] - if scope_value in [SettingScope.USER.value, SettingScope.USER_PAGE.value]: + scope_enum = _normalize_scope(scope_value) + if scope_enum in [SettingScope.USER, SettingScope.USER_PAGE]: if not auth: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, @@ -1606,7 +1641,7 @@ async def delete_setting_instance( ) # Verify ownership using service helper ensure_setting_instance_belongs_to_user(instance, auth) - elif scope_value == SettingScope.SYSTEM.value: + elif scope_enum == SettingScope.SYSTEM: if not auth or not auth.is_admin: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 3942ef2..accd043 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -3,7 +3,7 @@ from pathlib import Path from typing import List, Optional, Tuple, Union from pydantic_settings import BaseSettings -from pydantic import field_validator +from pydantic import field_validator, model_validator def _env_file_candidates() -> Tuple[Union[str, Path], ...]: """Build a prioritized list of .env files for cross-platform support.""" @@ -53,6 +53,8 @@ class Settings(BaseSettings): ACCESS_TOKEN_EXPIRE_MINUTES: int = 30 REFRESH_TOKEN_EXPIRE_DAYS: int = 30 ALGORITHM: str = "HS256" + COOKIE_SAMESITE: str = "lax" + COOKIE_SECURE: Optional[bool] = None # Rate Limiting & Request Size MAX_REQUEST_SIZE: int = 5 * 1024 * 1024 # 5MB for JSON bodies @@ -121,6 +123,22 @@ def parse_hosts(cls, v): return [p.strip() for p in s.split(",") if p.strip()] return v + @model_validator(mode="after") + def enforce_production_security(self): + env = (self.APP_ENV or "").lower() + if env in {"prod", "production", "staging"}: + if self.SECRET_KEY == "your-secret-key-here": + raise ValueError("SECRET_KEY must be set for production/staging.") + if not self.ENCRYPTION_MASTER_KEY: + raise ValueError("ENCRYPTION_MASTER_KEY must be set for production/staging.") + if any(host == "*" for host in (self.ALLOWED_HOSTS or [])): + raise ValueError("ALLOWED_HOSTS cannot include '*' in production/staging.") + if (self.FORWARDED_ALLOW_IPS or "").strip() == "*": + raise ValueError("FORWARDED_ALLOW_IPS cannot be '*' in production/staging.") + if not self.PLUGIN_RUNTIME_TOKEN or not self.JOB_WORKER_TOKEN or not self.PLUGIN_LIFECYCLE_TOKEN: + raise ValueError("Service tokens must be set in production/staging.") + return self + model_config = { "env_file": _env_file_candidates(), "env_file_encoding": "utf-8", diff --git a/backend/app/core/rate_limit_deps.py b/backend/app/core/rate_limit_deps.py index bddc9fe..9af4d47 100644 --- a/backend/app/core/rate_limit_deps.py +++ b/backend/app/core/rate_limit_deps.py @@ -6,12 +6,49 @@ - IP address (for unauthenticated endpoints like login) """ from fastapi import Request, Depends -from typing import Optional, Callable +from typing import Optional, Callable, Iterable import functools +import ipaddress from app.core.rate_limit import rate_limiter from app.core.auth_context import AuthContext from app.core.auth_deps import optional_user +from app.core.config import settings + + +def _parse_forwarded_allow_ips(value: str) -> Iterable[str]: + if not value: + return [] + if value.strip() == "*": + return ["*"] + return [part.strip() for part in value.split(",") if part.strip()] + + +def _is_trusted_proxy(client_host: Optional[str]) -> bool: + if not client_host: + return False + + allow_ips = _parse_forwarded_allow_ips(settings.FORWARDED_ALLOW_IPS) + if not allow_ips: + return False + if "*" in allow_ips: + return True + + try: + client_ip = ipaddress.ip_address(client_host) + except ValueError: + return False + + for entry in allow_ips: + try: + if "/" in entry: + if client_ip in ipaddress.ip_network(entry, strict=False): + return True + elif client_ip == ipaddress.ip_address(entry): + return True + except ValueError: + continue + return False def _get_client_ip(request: Request) -> str: @@ -27,16 +64,14 @@ def _get_client_ip(request: Request) -> str: Returns: Client IP address as string """ - # Check X-Forwarded-For header (for reverse proxies) - forwarded_for = request.headers.get("X-Forwarded-For") - if forwarded_for: - # X-Forwarded-For can be a comma-separated list, take the first (original client) - return forwarded_for.split(",")[0].strip() - - # Check X-Real-IP header (alternative proxy header) - real_ip = request.headers.get("X-Real-IP") - if real_ip: - return real_ip.strip() + if _is_trusted_proxy(request.client.host if request.client else None): + forwarded_for = request.headers.get("X-Forwarded-For") + if forwarded_for: + return forwarded_for.split(",")[0].strip() + + real_ip = request.headers.get("X-Real-IP") + if real_ip: + return real_ip.strip() # Fall back to direct client host if request.client: @@ -148,4 +183,3 @@ async def dependency( return None return dependency - diff --git a/backend/app/core/security.py b/backend/app/core/security.py index 614ee31..096a87c 100644 --- a/backend/app/core/security.py +++ b/backend/app/core/security.py @@ -21,6 +21,9 @@ # Using bcrypt with a work factor of 12 (2^12 iterations) pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto", bcrypt__rounds=12) +def _token_debug_enabled() -> bool: + return settings.APP_ENV.lower() in {"dev", "development", "test", "local"} or settings.DEBUG + def verify_password(plain_password: str, hashed_password: str) -> bool: """Verify a password against its hash using Passlib.""" try: @@ -95,16 +98,17 @@ async def get_current_user(token: str = Depends(oauth2_scheme), db: AsyncSession """Get the current user from the JWT token.""" try: - # Log the token being used (first 10 chars only for security) - token_preview = token[:10] + "..." if token else "None" - logger.info(f"Authenticating with token: {token_preview}") + if _token_debug_enabled(): + token_preview = token[:10] + "..." if token else "None" + logger.debug(f"Authenticating with token: {token_preview}") # Decode the token payload = decode_access_token(token) # Log the payload (excluding sensitive data) user_id = payload.get("sub") - logger.info(f"Token payload contains user_id: {user_id}") + if _token_debug_enabled(): + logger.debug(f"Token payload contains user_id: {user_id}") if user_id is None: logger.error("Token payload does not contain 'sub' field") diff --git a/backend/app/core/service_auth.py b/backend/app/core/service_auth.py index f633ebb..1cc6ab5 100644 --- a/backend/app/core/service_auth.py +++ b/backend/app/core/service_auth.py @@ -109,70 +109,80 @@ async def get_service_context(request: Request) -> ServiceContext: Raises: HTTPException: 401 if token invalid, 403 if service not recognized """ - # Extract token - token = _extract_service_token(request) - - if not token: - logger.warning( - "Service auth failed - no token", - path=request.url.path, - method=request.method - ) - _log_service_auth_background(request, "No service token provided", success=False) - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Service authentication required. Provide Bearer token.", - headers={"WWW-Authenticate": "Bearer"}, + try: + # Extract token + token = _extract_service_token(request) + + if not token: + logger.warning( + "Service auth failed - no token", + extra={"path": request.url.path, "method": request.method} + ) + _log_service_auth_background(request, "No service token provided", success=False) + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Service authentication required. Provide Bearer token.", + headers={"WWW-Authenticate": "Bearer"}, + ) + + # Validate token and get service name + service_name = _validate_service_token(token) + + if not service_name: + logger.warning( + "Service auth failed - invalid token", + extra={"path": request.url.path, "method": request.method} + ) + _log_service_auth_background(request, "Invalid service token", success=False) + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Invalid service token", + headers={"WWW-Authenticate": "Bearer"}, + ) + + # Get service definition + service_def = SERVICE_DEFINITIONS.get(service_name) + + if not service_def: + logger.error( + "Service auth failed - unknown service", + extra={"service_name": service_name, "path": request.url.path} + ) + _log_service_auth_background(request, f"Unknown service: {service_name}", success=False, service_name=service_name) + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail=f"Service '{service_name}' not recognized" + ) + + # Create service context + context = ServiceContext( + service_name=service_name, + scopes=service_def["scopes"] ) - - # Validate token and get service name - service_name = _validate_service_token(token) - - if not service_name: - logger.warning( - "Service auth failed - invalid token", - path=request.url.path, - method=request.method + + # Log successful authentication + _log_service_auth_background(request, "Service authenticated", success=True, service_name=service_name) + + logger.info( + "Service authenticated", + extra={ + "service_name": service_name, + "scopes": list(context.scopes), + "path": request.url.path, + } ) - _log_service_auth_background(request, "Invalid service token", success=False) + + return context + except HTTPException: + raise + except Exception as e: + logger.error("Service auth error", exc_info=True) + _log_service_auth_background(request, "Service auth error", success=False) raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, - detail="Invalid service token", + detail="Service authentication failed", headers={"WWW-Authenticate": "Bearer"}, ) - - # Get service definition - service_def = SERVICE_DEFINITIONS.get(service_name) - - if not service_def: - logger.error( - "Service auth failed - unknown service", - service_name=service_name, - path=request.url.path - ) - _log_service_auth_background(request, f"Unknown service: {service_name}", success=False, service_name=service_name) - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail=f"Service '{service_name}' not recognized" - ) - - # Create service context - context = ServiceContext( - service_name=service_name, - scopes=service_def["scopes"] - ) - - # Log successful authentication - _log_service_auth_background(request, "Service authenticated", success=True, service_name=service_name) - - logger.info( - "Service authenticated", - service_name=service_name, - scopes=list(context.scopes), - path=request.url.path - ) - - return context async def require_service( @@ -223,9 +233,11 @@ async def require_service_scope( if not service_context.has_scope(required_scope): logger.warning( "Service missing required scope", - service_name=service_context.service_name, - required_scope=required_scope, - has_scopes=list(service_context.scopes) + extra={ + "service_name": service_context.service_name, + "required_scope": required_scope, + "has_scopes": list(service_context.scopes), + } ) raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, @@ -265,4 +277,3 @@ async def dependency( require_plugin_execution = create_scope_dependency("execute_plugin") require_job_execution = create_scope_dependency("execute_job") require_plugin_lifecycle = create_scope_dependency("install_plugin") - diff --git a/backend/app/main.py b/backend/app/main.py index cd00c46..7a9a4af 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -7,6 +7,9 @@ from app.routers.plugins import plugin_manager from app.plugins.service_installler.start_stop_plugin_services import start_plugin_services_on_startup, stop_all_plugin_services_on_shutdown from app.core.job_manager_provider import initialize_job_manager, shutdown_job_manager +from app.middleware.request_size import RequestSizeMiddleware +from app.middleware.request_id import RequestIdMiddleware +from app.core.audit.redaction import redact_sensitive_data import logging import time import structlog @@ -24,6 +27,12 @@ max_age=settings.CORS_MAX_AGE, ) +app.add_middleware( + RequestSizeMiddleware, + max_size=settings.MAX_REQUEST_SIZE +) +app.add_middleware(RequestIdMiddleware) + # Add startup event to initialize settings @app.on_event("startup") async def startup_event(): @@ -62,9 +71,9 @@ async def log_requests(request: Request, call_next): method=request.method, url=str(request.url), path=request.url.path, - query_params=str(request.query_params), + query_params=redact_sensitive_data(dict(request.query_params)), client=request.client.host if request.client else None, - headers=dict(request.headers), + headers=redact_sensitive_data(dict(request.headers)), ) try: diff --git a/backend/app/middleware/request_size.py b/backend/app/middleware/request_size.py index 3d31dce..ab1014a 100644 --- a/backend/app/middleware/request_size.py +++ b/backend/app/middleware/request_size.py @@ -88,7 +88,25 @@ async def dispatch(self, request: Request, call_next): except ValueError: # Invalid Content-Length header - let it through, will fail downstream pass + else: + # Fall back to reading the body when Content-Length is missing. + body = await request.body() + if len(body) > self.max_size: + limit_mb = self.max_size / (1024 * 1024) + size_mb = len(body) / (1024 * 1024) + logger.warning( + "Request size exceeded (no Content-Length)", + path=request_path, + size_mb=f"{size_mb:.2f}", + limit_mb=f"{limit_mb:.2f}", + client=request.client.host if request.client else "unknown" + ) + return JSONResponse( + status_code=413, + content={ + "detail": f"Request body too large. Maximum size is {limit_mb:.1f}MB, received {size_mb:.2f}MB" + } + ) # Request is within size limit, continue processing return await call_next(request) - diff --git a/backend/app/routers/plugins.py b/backend/app/routers/plugins.py index 09c99c2..81f3768 100644 --- a/backend/app/routers/plugins.py +++ b/backend/app/routers/plugins.py @@ -6,6 +6,7 @@ from ..plugins import PluginManager from ..plugins.repository import PluginRepository from ..core.database import get_db +from ..core.config import settings from ..models.plugin import Plugin, Module from ..models.user import User from pathlib import Path @@ -22,8 +23,22 @@ PLUGINS_DIR = Path(__file__).parent.parent.parent / "plugins" plugin_manager = PluginManager(str(PLUGINS_DIR)) + +def _safe_join(base_dir: Path, relative_path: str) -> Optional[Path]: + if not relative_path: + return None + if Path(relative_path).is_absolute(): + return None + base_dir = base_dir.resolve() + candidate = (base_dir / relative_path).resolve() + try: + candidate.relative_to(base_dir) + except ValueError: + return None + return candidate + # Import new auth dependencies -from ..core.auth_deps import require_user +from ..core.auth_deps import require_user, optional_user from ..core.auth_context import AuthContext # Create a router for plugin management endpoints WITHOUT a prefix @@ -1088,6 +1103,11 @@ async def serve_plugin_static( # Try multiple possible locations for the file possible_paths = [] + + def add_candidate(base_dir: Path) -> None: + candidate = _safe_join(base_dir, path) + if candidate: + possible_paths.append(candidate) # 1. New architecture: Shared storage with version if plugin.plugin_slug and plugin.version: @@ -1095,34 +1115,34 @@ async def serve_plugin_static( # PLUGINS_DIR is already /path/to/project/plugins, so PLUGINS_DIR.parent is /path/to/project # We need to go to /path/to/project/backend/backend/plugins/shared/ shared_plugin_dir = PLUGINS_DIR.parent / "backend" / "plugins" / "shared" / plugin.plugin_slug / f"v{plugin.version}" - possible_paths.append(shared_plugin_dir / path) + add_candidate(shared_plugin_dir) # 2. New architecture: Shared storage without version (fallback) if plugin.plugin_slug: shared_plugin_dir = PLUGINS_DIR.parent / "backend" / "plugins" / "shared" / plugin.plugin_slug - possible_paths.append(shared_plugin_dir / path) + add_candidate(shared_plugin_dir) # 3. Backend plugins directory (where webpack builds to) if plugin.plugin_slug: backend_plugin_dir = PLUGINS_DIR.parent / "backend" / "plugins" / plugin.plugin_slug - possible_paths.append(backend_plugin_dir / path) + add_candidate(backend_plugin_dir) # 4. User-specific directory with plugin_slug if plugin.user_id and plugin.plugin_slug: user_plugin_dir = PLUGINS_DIR / plugin.user_id / plugin.plugin_slug - possible_paths.append(user_plugin_dir / path) + add_candidate(user_plugin_dir) # 5. User-specific directory with plugin ID if plugin.user_id: user_plugin_dir = PLUGINS_DIR / plugin.user_id / plugin.id - possible_paths.append(user_plugin_dir / path) + add_candidate(user_plugin_dir) # 6. Legacy path directly under plugins directory with plugin_slug if plugin.plugin_slug: - possible_paths.append(PLUGINS_DIR / plugin.plugin_slug / path) + add_candidate(PLUGINS_DIR / plugin.plugin_slug) # 7. Legacy path directly under plugins directory with plugin ID - possible_paths.append(PLUGINS_DIR / plugin.id / path) + add_candidate(PLUGINS_DIR / plugin.id) # Try each path for plugin_path in possible_paths: @@ -1139,7 +1159,8 @@ async def serve_plugin_static( async def serve_plugin_static_public( plugin_id: str, path: str, - db: AsyncSession = Depends(get_db) + db: AsyncSession = Depends(get_db), + auth: Optional[AuthContext] = Depends(optional_user), ): """Serve static files from plugin directory without authentication. This endpoint is specifically for serving JavaScript bundles and other @@ -1152,6 +1173,9 @@ async def serve_plugin_static_public( if file_ext not in allowed_extensions: logger.warning(f"Attempted to access disallowed file type: {file_ext}") raise HTTPException(status_code=403, detail="File type not allowed") + + if settings.APP_ENV.lower() not in {"dev", "development", "test", "local"} and auth is None: + raise HTTPException(status_code=401, detail="Authentication required") # Skip if the path starts with "modules/" to avoid catching module endpoints if path.startswith("modules/"): @@ -1160,12 +1184,18 @@ async def serve_plugin_static_public( logger.debug(f"Serving public static file for plugin_id: {plugin_id}, path: {path}") # Try to find the plugin by ID first - plugin_query = await db.execute(select(Plugin).where(Plugin.id == plugin_id)) + plugin_filters = [Plugin.id == plugin_id] + if auth: + plugin_filters.append(Plugin.user_id == auth.user_id) + plugin_query = await db.execute(select(Plugin).where(*plugin_filters)) plugin = plugin_query.scalars().first() # If not found by ID, try to find by plugin_slug if not plugin: - plugin_query = await db.execute(select(Plugin).where(Plugin.plugin_slug == plugin_id)) + plugin_filters = [Plugin.plugin_slug == plugin_id] + if auth: + plugin_filters.append(Plugin.user_id == auth.user_id) + plugin_query = await db.execute(select(Plugin).where(*plugin_filters)) plugin = plugin_query.scalars().first() if not plugin: @@ -1176,6 +1206,11 @@ async def serve_plugin_static_public( # Try multiple possible locations for the file possible_paths = [] + + def add_candidate(base_dir: Path) -> None: + candidate = _safe_join(base_dir, path) + if candidate: + possible_paths.append(candidate) # 1. New architecture: Shared storage with version if plugin.plugin_slug and plugin.version: @@ -1183,37 +1218,37 @@ async def serve_plugin_static_public( # PLUGINS_DIR is already /path/to/project/plugins, so PLUGINS_DIR.parent is /path/to/project # We need to go to /path/to/project/backend/backend/plugins/shared/ shared_plugin_dir = PLUGINS_DIR.parent / "plugins" / "shared" / plugin.plugin_slug / f"v{plugin.version}" - possible_paths.append(shared_plugin_dir / path) + add_candidate(shared_plugin_dir) logger.debug(f"Added new architecture path with version: {shared_plugin_dir / path}") # 2. New architecture: Shared storage without version (fallback) if plugin.plugin_slug: shared_plugin_dir = PLUGINS_DIR.parent / "plugins" / "shared" / plugin.plugin_slug - possible_paths.append(shared_plugin_dir / path) + add_candidate(shared_plugin_dir) logger.debug(f"Added new architecture path without version: {shared_plugin_dir / path}") # 3. Backend plugins directory (where webpack builds to) if plugin.plugin_slug: backend_plugin_dir = PLUGINS_DIR.parent / "backend" / "plugins" / plugin.plugin_slug - possible_paths.append(backend_plugin_dir / path) + add_candidate(backend_plugin_dir) logger.debug(f"Added backend plugins path: {backend_plugin_dir / path}") # 4. User-specific directory with plugin_slug if plugin.user_id and plugin.plugin_slug: user_plugin_dir = PLUGINS_DIR / plugin.user_id / plugin.plugin_slug - possible_paths.append(user_plugin_dir / path) + add_candidate(user_plugin_dir) # 5. User-specific directory with plugin ID if plugin.user_id: user_plugin_dir = PLUGINS_DIR / plugin.user_id / plugin.id - possible_paths.append(user_plugin_dir / path) + add_candidate(user_plugin_dir) # 6. Legacy path directly under plugins directory with plugin_slug if plugin.plugin_slug: - possible_paths.append(PLUGINS_DIR / plugin.plugin_slug / path) + add_candidate(PLUGINS_DIR / plugin.plugin_slug) # 7. Legacy path directly under plugins directory with plugin ID - possible_paths.append(PLUGINS_DIR / plugin.id / path) + add_candidate(PLUGINS_DIR / plugin.id) # Try each path for plugin_path in possible_paths: diff --git a/backend/main.py b/backend/main.py index 3b2ccbc..eb187a0 100644 --- a/backend/main.py +++ b/backend/main.py @@ -239,9 +239,11 @@ async def dispatch(self, request: Request, call_next): return response # โœ… 3. Allow All Hosts for Development (Fix 403 Issues) +env = settings.APP_ENV.lower() +allowed_hosts = ["*"] if env in {"dev", "development", "test", "local"} else settings.ALLOWED_HOSTS app.add_middleware( - TrustedHostMiddleware, - allowed_hosts=["*"] # Allow all hosts (change in production) + TrustedHostMiddleware, + allowed_hosts=allowed_hosts ) # โœ… 3.5. Request ID for correlation and audit logging diff --git a/backend/pytest.ini b/backend/pytest.ini new file mode 100644 index 0000000..2795745 --- /dev/null +++ b/backend/pytest.ini @@ -0,0 +1,3 @@ +[pytest] +markers = + security: security regression tests diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index c22c1f4..a5c5009 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -48,6 +48,9 @@ def __init__(self, *args, **kwargs): # Override settings for testing os.environ["DATABASE_URL"] = "sqlite+aiosqlite:///./test.db" os.environ["ENVIRONMENT"] = "test" +os.environ["PLUGIN_RUNTIME_TOKEN"] = "test-plugin-runtime-token" +os.environ["JOB_WORKER_TOKEN"] = "test-job-worker-token" +os.environ["PLUGIN_LIFECYCLE_TOKEN"] = "test-plugin-lifecycle-token" from app.core.config import settings diff --git a/backend/tests/test_security_regressions.py b/backend/tests/test_security_regressions.py new file mode 100644 index 0000000..93ad4d5 --- /dev/null +++ b/backend/tests/test_security_regressions.py @@ -0,0 +1,182 @@ +import pytest +from fastapi.testclient import TestClient + +from app.core.config import settings +from app.core.rate_limit import rate_limiter + + +pytestmark = pytest.mark.security + + +@pytest.fixture(autouse=True) +def reset_rate_limiter(): + rate_limiter.buckets.clear() + yield + rate_limiter.buckets.clear() + + +def _register_user(client: TestClient, email: str, username: str, password: str) -> None: + response = client.post( + "/api/v1/auth/register", + json={"email": email, "username": username, "password": password}, + ) + assert response.status_code == 200 + + +def _login_user(client: TestClient, email: str, password: str) -> str: + response = client.post( + "/api/v1/auth/login", + json={"email": email, "password": password}, + ) + assert response.status_code == 200 + return response.json()["access_token"] + +def _create_user_setting_instance(client: TestClient, token: str, instance_id: str = None) -> dict: + payload = { + "definition_id": "security_user_setting", + "name": "security-user-setting", + "value": {"flag": True}, + "scope": "user", + "user_id": "current", + } + if instance_id: + payload["id"] = instance_id + response = client.post( + "/api/v1/settings/instances", + json=payload, + headers={"Authorization": f"Bearer {token}"}, + ) + assert response.status_code == 200 + return response.json() + + +def test_protected_endpoint_requires_auth(client: TestClient) -> None: + response = client.get("/api/v1/settings/instances") + assert response.status_code == 401 + + +def test_admin_endpoint_denied_for_non_admin(client: TestClient) -> None: + _register_user(client, "user@example.com", "user", "password123") + token = _login_user(client, "user@example.com", "password123") + + response = client.post( + "/api/v1/settings/definitions", + json={ + "id": "testdef", + "name": "test-definition", + "description": "test definition", + "category": "system", + "type": "string", + "default_value": "value", + "allowed_scopes": ["system"], + "validation": None, + "is_multiple": False, + "tags": ["security"], + }, + headers={"Authorization": f"Bearer {token}"}, + ) + + assert response.status_code == 403 + + +def test_internal_endpoint_rejects_missing_service_token(client: TestClient) -> None: + response = client.get("/api/v1/_internal/health") + assert response.status_code == 401 + + +def test_internal_endpoint_rejects_user_jwt(client: TestClient) -> None: + _register_user(client, "internal@example.com", "internal", "password123") + token = _login_user(client, "internal@example.com", "password123") + + response = client.get( + "/api/v1/_internal/health", + headers={"Authorization": f"Bearer {token}"}, + ) + + assert response.status_code == 401 + + +def test_internal_endpoint_accepts_service_token(client: TestClient) -> None: + assert settings.PLUGIN_RUNTIME_TOKEN + + response = client.get( + "/api/v1/_internal/health", + headers={"Authorization": f"Bearer {settings.PLUGIN_RUNTIME_TOKEN}"}, + ) + + assert response.status_code == 200 + data = response.json() + assert data["service_name"] == "plugin_runtime" + + +def test_internal_job_endpoint_requires_job_worker_scope(client: TestClient) -> None: + assert settings.JOB_WORKER_TOKEN + + response = client.post( + "/api/v1/_internal/job/progress", + params={"job_id": "job-123"}, + headers={"Authorization": f"Bearer {settings.JOB_WORKER_TOKEN}"}, + ) + + assert response.status_code == 200 + data = response.json() + assert data["job_id"] == "job-123" + +def test_user_cannot_read_other_users_setting_instance(client: TestClient) -> None: + _register_user(client, "owner@example.com", "owner", "password123") + _register_user(client, "other@example.com", "other", "password123") + + owner_token = _login_user(client, "owner@example.com", "password123") + other_token = _login_user(client, "other@example.com", "password123") + + created = _create_user_setting_instance(client, owner_token) + instance_id = created["id"] + + response = client.get( + f"/api/v1/settings/instances/{instance_id}", + headers={"Authorization": f"Bearer {other_token}"}, + ) + + assert response.status_code == 404 + +def test_user_can_read_own_setting_instance(client: TestClient) -> None: + _register_user(client, "self@example.com", "self", "password123") + token = _login_user(client, "self@example.com", "password123") + + created = _create_user_setting_instance(client, token) + instance_id = created["id"] + + response = client.get( + f"/api/v1/settings/instances/{instance_id}", + headers={"Authorization": f"Bearer {token}"}, + ) + + assert response.status_code == 200 + data = response.json() + assert data["id"] == instance_id + + +def test_login_rate_limit_enforced(client: TestClient) -> None: + for _ in range(5): + response = client.post( + "/api/v1/auth/login", + json={"email": "missing@example.com", "password": "wrong"}, + ) + assert response.status_code == 401 + + response = client.post( + "/api/v1/auth/login", + json={"email": "missing@example.com", "password": "wrong"}, + ) + assert response.status_code == 429 + + +def test_request_size_limit_enforced(client: TestClient) -> None: + oversized_body = b"a" * (settings.MAX_REQUEST_SIZE + 1) + content_length = str(len(oversized_body)) + response = client.post( + "/api/v1/auth/login", + content=oversized_body, + headers={"Content-Type": "application/json", "Content-Length": content_length}, + ) + assert response.status_code == 413 diff --git a/docs/reference/security-final-report.md b/docs/reference/security-final-report.md new file mode 100644 index 0000000..7d462da --- /dev/null +++ b/docs/reference/security-final-report.md @@ -0,0 +1,59 @@ +# Security Audit Final Report - BrainDrive Core + +## Overview +This report summarizes the security verification and remediation work completed for BrainDrive Core. +The security roadmap phases were verified, fixes were implemented, and penetration-style tests were executed and retested. + +## Scope +- Auth context and authorization enforcement +- Rate limiting and request size enforcement +- Internal service authentication +- Audit logging and request correlation +- Plugin static asset serving +- Configuration hardening for production +- Targeted code review for critical files + +## Key Fixes Delivered +- Enforced production safety guards for SECRET_KEY, ENCRYPTION_MASTER_KEY, service tokens, and allowed hosts +- TrustedHostMiddleware restricted outside dev environments +- Rate limit IP trust is restricted to allowlisted proxies +- Sanitized token and cookie logging; debug-gated token previews +- Plugin static assets protected against path traversal; public assets gated outside dev +- Request ID middleware added and sensitive headers redacted in request logs +- Settings instance ownership enforced to prevent cross-user access (IDOR fix) +- Internal service auth hardened to return 401 instead of 500 on invalid tokens +- Raw SQL f-strings parameterized in navigation routes +- Dev-only cookie endpoints gated to dev/test/local + +## Penetration Test Results (Retest) +- Protected route without token: 401 (pass) +- Tampered token: 401 (pass) +- Admin endpoint as user: 403 (pass) +- Path traversal: 404 (pass) +- Rate limiting login: 429 (pass) +- Cross-user settings read: 404 (pass) +- Cross-user delete: 404 (pass) +- Internal endpoint with user JWT: 401 (pass) +- X-Forwarded-For spoofing: config-dependent + - If behind trusted proxy, set FORWARDED_ALLOW_IPS to proxy IPs/CIDRs + - If direct client access, set FORWARDED_ALLOW_IPS to empty/strict list to ignore XFF + +## Tests Run +- pytest -m security (latest run after fixes) + +## Tooling Not Run +- Bandit and pip-audit were not executed in this environment (tools not installed) + +## Configuration Requirements (Production) +- SECRET_KEY (non-default) +- ENCRYPTION_MASTER_KEY (non-empty, >= 32 chars) +- FORWARDED_ALLOW_IPS (proxy allowlist, not "*") +- PLUGIN_RUNTIME_TOKEN, JOB_WORKER_TOKEN, PLUGIN_LIFECYCLE_TOKEN +- COOKIE_SAMESITE / COOKIE_SECURE as appropriate for cross-site cookies + +## Residual Risk +- Only remaining risk is misconfiguration of FORWARDED_ALLOW_IPS in production. + This is a deployment decision, not a code issue. + +## Branch +- security/waring-regression-suite diff --git a/docs/reference/security-pr-description.md b/docs/reference/security-pr-description.md new file mode 100644 index 0000000..fb2db12 --- /dev/null +++ b/docs/reference/security-pr-description.md @@ -0,0 +1,25 @@ +# Draft PR Description (Final Status) + +## Summary +This PR completes the security hardening work from the audit plan and verifies fixes with automated tests and pen-test retests. + +Key changes: +- Production safety guards for SECRET_KEY, ENCRYPTION_MASTER_KEY, service tokens, and allowed hosts +- Trusted proxy handling for rate limiting (no XFF spoofing from untrusted clients) +- TrustedHostMiddleware restricted outside dev +- Sanitized token/cookie logging; debug-gated token previews +- Plugin static asset path traversal fixed; public assets gated outside dev +- Request ID middleware and redacted request logs +- Settings instance ownership enforcement (IDOR fix) +- Internal service auth hardened to return 401 instead of 500 +- Navigation routes SQL parameterized +- Dev-only cookie endpoints gated + +## Testing +- pytest -m security (latest run after fixes) +- Pen-test matrix rerun: all pass; XFF behavior is config-dependent + +## Notes +- Production requires SECRET_KEY, ENCRYPTION_MASTER_KEY, service tokens, and non-* FORWARDED_ALLOW_IPS +- If cross-site cookies are required, set COOKIE_SAMESITE=none with COOKIE_SECURE=true +- XFF behavior depends on FORWARDED_ALLOW_IPS (proxy allowlist vs direct access) diff --git a/docs/reference/security-testing-plan.md b/docs/reference/security-testing-plan.md new file mode 100644 index 0000000..b716587 --- /dev/null +++ b/docs/reference/security-testing-plan.md @@ -0,0 +1,151 @@ +# Security Testing Plan - BrainDrive + +This plan verifies the security roadmap implementation and guards against regressions in BrainDrive-Core. It layers automated checks, targeted manual testing, and repeatable evidence capture so results are auditable. + +## 1) Scope + +In scope: +- Backend API (`backend/app`, `backend/main.py`, `backend/scripts`) +- Frontend client (`frontend/src`, auth/session handling) +- Plugin lifecycle and file handling (`backend/app/routers/plugins.py`, `backend/plugins`) +- Internal service endpoints (`backend/app/api/v1/internal`) +- Configuration defaults and environment enforcement (`backend/app/core/config.py`) + +Out of scope (unless explicitly requested): +- Third-party hosted services +- External SaaS integrations beyond auth/token handling + +## 2) Environments and Data + +Environments: +- Local dev (baseline behavior) +- Staging or prod-like (required for final sign-off) + +Test data: +- Admin user, standard user, suspended user (if supported) +- Service tokens for internal endpoints (plugin/runtime/worker) +- Seeded content (documents, plugins, jobs) for authz tests +- Clean state: reset database, clear caches/rate limiter, and seed a known dataset before each run + +## 3) Execution Phases + +### Phase A - Preflight +- Verify environment variables set (no default secrets). +- Ensure service auth tokens are non-empty (e.g., `PLUGIN_RUNTIME_TOKEN`, `JOB_WORKER_TOKEN`). +- Record git commit hash being tested. +- Reset/seed the database and confirm test accounts exist and are isolated from prod data. + +### Phase B - Automated Static Checks (CI-friendly) + +Backend (Python): +- SAST: `bandit -r backend/app` +- Lint for unsafe patterns: `python backend/scripts/check_security_patterns.py` +- SCA: `pip-audit -r backend/requirements.txt` +- Secrets: `gitleaks detect --source backend --no-git` + +Frontend (Node/TS): +- SCA: `npm audit --omit=dev` (or `--production`) +- Secrets: `gitleaks detect --source frontend --no-git` + +Config/IaC (if present): +- Dockerfiles, compose, k8s manifests: `trivy config .` or `checkov -d .` + +### Phase C - Automated Dynamic Checks + +API tests (local or staging): +- Authn/Authz regression suite (see matrix below) +- Rate limiting and request size tests +- Internal endpoint access tests +- Convert curl scenarios into pytest regression tests to prevent drift + +DAST (staging only): +- Run OWASP ZAP against the exposed API surface and capture report + +### Phase D - Manual Targeted Review + +- Auth dependency usage and router-level protection +- Plugin file handling and path traversal guards +- Audit logging redaction and request ID correlation +- TrustedHost, CORS, cookie flags, and proxy header trust + +### Phase E - Reporting and Sign-off + +- PASS/FAIL per roadmap item +- Prioritized findings list with severity +- Evidence bundle (command output, logs, test results) + +## 4) Test Matrix (Core) + +Authn: +- No token -> 401 +- Expired token -> 401 +- Tampered token -> 401 +- Refresh flow -> 200 only with valid refresh token + +Authz: +- User access admin endpoint -> 403 +- User access another user's resource -> 403/404 +- Service endpoint with user JWT -> 401 +- Internal endpoint with service token -> 200 + +Rate Limiting: +- Login brute force (6+ attempts in window) -> 429 +- Authenticated abuse (user_id key) -> 429 +- X-Forwarded-For spoofing -> still rate limited + +Request Size: +- Oversized JSON body -> 413 +- Oversized upload -> 413 (and no partial write) + +Audit Logging: +- Auth failure emits audit event (redacted) +- Admin actions emit audit event (redacted) +- X-Request-ID present in responses and logs + +Plugin and File Handling: +- Path traversal attempts -> 404 +- Zip slip or unsafe extraction -> blocked +- Public plugin endpoints enforce auth where required + +Configuration Hardening: +- `SECRET_KEY` non-default enforced in prod-like +- `ENCRYPTION_MASTER_KEY` enforced +- `TrustedHostMiddleware` not `["*"]` in prod-like +- Cookie flags: `HttpOnly`, `Secure`, `SameSite` align to env + +Injection: +- SQL injection probes in search/filter params -> no leak +- Log injection via headers -> sanitized + +## 5) Evidence and Exit Criteria + +Evidence to capture: +- Test command outputs +- DAST reports +- API response logs for each test case +- Audit log samples showing redaction + +Exit criteria: +- All roadmap items PASS +- No CRITICAL/HIGH findings open +- MEDIUM findings triaged with a plan and owner + +## 6) CI Gating Recommendations + +Add a security job that runs on PRs: +- `backend/scripts/check_security_patterns.py` +- `bandit -r backend/app` +- `pip-audit -r backend/requirements.txt` +- `npm audit --omit=dev` +- `gitleaks detect --source . --no-git` +- Security pytest suite (e.g., `pytest -m security`) + +Add a nightly job (staging): +- OWASP ZAP report +- Full authn/authz matrix + +## 7) Open Questions + +- Which environments are in scope for sign-off? +- Do you want these checks to block merges or only generate reports? +- Any compliance targets that require additional controls or evidence?