#!/usr/bin/env python3 """ Pre-commit hook to detect resources (services, database sessions) that are instantiated without context managers. Services like DownloadService, LibraryRAGService, and LocalEmbeddingManager hold resources (file handles, connections, models) that need to be released. They should be used with context managers (with statements) to ensure cleanup. Functions like get_auth_db_session() return raw sessions that require manual cleanup. Use the auth_db_session() context manager instead. Exceptions: - Factory functions that return services (caller is responsible for cleanup) - Services passed to other objects (dependency injection) - try/finally with explicit close() """ import ast import sys from typing import List, Optional, Set, Tuple # Services (classes) that require context manager usage SERVICES_REQUIRING_CONTEXT = { "DownloadService", "LibraryRAGService", "LocalEmbeddingManager", } # Functions returning resources that need cleanup # Maps function name -> suggested context manager replacement FUNCTIONS_REQUIRING_CONTEXT = { "get_auth_db_session": "auth_db_session()", "get_db_session": "get_user_db_session(username)", } # Files/patterns where direct instantiation is allowed ALLOWED_PATTERNS = { "tests/", # Test files often mock or need special handling "test_", # Test files "_test.py", # Test files # The service implementations themselves "download_service.py", "library_rag_service.py", "search_engine_local.py", # Database implementation files "auth_db.py", "encrypted_db.py", "db_utils.py", # Defines get_db_session itself "session_context.py", # Stores session in g.db_session (cleaned by teardown) "web/auth/decorators.py", # inject_current_user stores in g.db_session } def is_file_allowed(filename: str) -> bool: """Check if this file is allowed to use services without context managers.""" for pattern in ALLOWED_PATTERNS: if pattern in filename: return True return False def get_resource_name(node: ast.expr) -> Optional[Tuple[str, Optional[str]]]: """ Check if an expression is a resource that requires cleanup. Returns: Tuple of (resource_name, suggested_replacement) or None if not a resource. For services, suggested_replacement is None (use 'with ServiceName(...) as var:'). For functions, suggested_replacement is the context manager to use instead. """ if not isinstance(node, ast.Call): return None func_name = None # Check for Name() pattern (e.g., ServiceName() or get_auth_db_session()) if isinstance(node.func, ast.Name): func_name = node.func.id # Check for module.Name() pattern (e.g., module.ServiceName()) elif isinstance(node.func, ast.Attribute): func_name = node.func.attr if func_name is None: return None # Check if it's a service class if func_name in SERVICES_REQUIRING_CONTEXT: return (func_name, None) # Check if it's a function returning a resource if func_name in FUNCTIONS_REQUIRING_CONTEXT: return (func_name, FUNCTIONS_REQUIRING_CONTEXT[func_name]) return None class FunctionScopeAnalyzer(ast.NodeVisitor): """Analyze patterns within a function scope.""" def __init__(self): self.close_vars: Set[str] = set() # Variables with try/finally close() self.safe_with_lines: Set[int] = set() # Lines in with context self.returned_vars: Set[str] = set() # Variables that are returned self.passed_to_args_vars: Set[str] = ( set() ) # Variables passed as arguments def visit_Try(self, node: ast.Try) -> None: """Find variables that are closed in finally blocks within this scope.""" if node.finalbody: self._find_close_calls(node.finalbody) self.generic_visit(node) def _find_close_calls(self, stmts: List) -> None: """Recursively find .close() calls in a list of statements.""" for stmt in stmts: if isinstance(stmt, ast.Expr) and isinstance(stmt.value, ast.Call): call = stmt.value if ( isinstance(call.func, ast.Attribute) and call.func.attr == "close" and isinstance(call.func.value, ast.Name) ): self.close_vars.add(call.func.value.id) # Recursively check nested try blocks in finally elif isinstance(stmt, ast.Try): self._find_close_calls(stmt.body) for handler in stmt.handlers: self._find_close_calls(handler.body) self._find_close_calls(stmt.finalbody) elif isinstance(stmt, ast.If): self._find_close_calls(stmt.body) self._find_close_calls(stmt.orelse) def visit_With(self, node: ast.With) -> None: """Mark with statements that use resources as context managers.""" for item in node.items: resource_info = get_resource_name(item.context_expr) if resource_info: self.safe_with_lines.add(item.context_expr.lineno) self.generic_visit(node) def visit_AsyncWith(self, node: ast.AsyncWith) -> None: """Mark async with statements that use resources as context managers.""" for item in node.items: resource_info = get_resource_name(item.context_expr) if resource_info: self.safe_with_lines.add(item.context_expr.lineno) self.generic_visit(node) def visit_Return(self, node: ast.Return) -> None: """Track variables that are returned (factory function pattern).""" if node.value and isinstance(node.value, ast.Name): self.returned_vars.add(node.value.id) # Also handle direct resource returns if node.value and get_resource_name(node.value): # This is a direct return of a resource - mark the line as safe self.safe_with_lines.add(node.value.lineno) self.generic_visit(node) def visit_Call(self, node: ast.Call) -> None: """Track variables passed as arguments to other functions.""" # Check all arguments for arg in node.args: if isinstance(arg, ast.Name): self.passed_to_args_vars.add(arg.id) # Check keyword arguments for kwarg in node.keywords: if isinstance(kwarg.value, ast.Name): self.passed_to_args_vars.add(kwarg.value.id) self.generic_visit(node) class ServiceContextChecker(ast.NodeVisitor): """Check for service instantiations that aren't properly managed.""" def __init__(self): self.errors: List[Tuple[int, str]] = [] def visit_FunctionDef(self, node: ast.FunctionDef) -> None: """Analyze a function for improper service usage.""" self._check_function_body(node) self.generic_visit(node) def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: """Analyze an async function for improper service usage.""" self._check_function_body(node) self.generic_visit(node) def visit_Module(self, node: ast.Module) -> None: """Also check module-level code.""" # Create a fake function body from module-level statements # that aren't inside functions module_stmts = [] for stmt in node.body: if not isinstance( stmt, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef) ): module_stmts.append(stmt) if module_stmts: self._check_statements(module_stmts) self.generic_visit(node) def _check_function_body(self, func_node) -> None: """Check a function body for improper service usage.""" self._check_statements(func_node.body) def _check_statements(self, statements: List[ast.stmt]) -> None: """Check a list of statements for improper service usage.""" # First, analyze this scope for patterns analyzer = FunctionScopeAnalyzer() for stmt in statements: analyzer.visit(stmt) # Now check each statement for service assignments self._check_statements_recursive( statements, analyzer.close_vars, analyzer.safe_with_lines, analyzer.returned_vars, analyzer.passed_to_args_vars, ) def _check_statements_recursive( self, statements: List[ast.stmt], close_vars: Set[str], safe_with_lines: Set[int], returned_vars: Set[str], passed_to_args_vars: Set[str], ) -> None: """Recursively check statements for service assignments.""" for stmt in statements: if isinstance(stmt, ast.Assign): self._check_assign( stmt, close_vars, safe_with_lines, returned_vars, passed_to_args_vars, ) elif isinstance(stmt, ast.With): self._check_statements_recursive( stmt.body, close_vars, safe_with_lines, returned_vars, passed_to_args_vars, ) elif isinstance(stmt, ast.AsyncWith): self._check_statements_recursive( stmt.body, close_vars, safe_with_lines, returned_vars, passed_to_args_vars, ) elif isinstance(stmt, ast.If): self._check_statements_recursive( stmt.body, close_vars, safe_with_lines, returned_vars, passed_to_args_vars, ) self._check_statements_recursive( stmt.orelse, close_vars, safe_with_lines, returned_vars, passed_to_args_vars, ) elif isinstance(stmt, ast.For): self._check_statements_recursive( stmt.body, close_vars, safe_with_lines, returned_vars, passed_to_args_vars, ) self._check_statements_recursive( stmt.orelse, close_vars, safe_with_lines, returned_vars, passed_to_args_vars, ) elif isinstance(stmt, ast.While): self._check_statements_recursive( stmt.body, close_vars, safe_with_lines, returned_vars, passed_to_args_vars, ) self._check_statements_recursive( stmt.orelse, close_vars, safe_with_lines, returned_vars, passed_to_args_vars, ) elif isinstance(stmt, ast.Try): self._check_statements_recursive( stmt.body, close_vars, safe_with_lines, returned_vars, passed_to_args_vars, ) for handler in stmt.handlers: self._check_statements_recursive( handler.body, close_vars, safe_with_lines, returned_vars, passed_to_args_vars, ) self._check_statements_recursive( stmt.orelse, close_vars, safe_with_lines, returned_vars, passed_to_args_vars, ) self._check_statements_recursive( stmt.finalbody, close_vars, safe_with_lines, returned_vars, passed_to_args_vars, ) def _check_assign( self, node: ast.Assign, close_vars: Set[str], safe_with_lines: Set[int], returned_vars: Set[str], passed_to_args_vars: Set[str], ) -> None: """Check an assignment for improper resource usage.""" resource_info = get_resource_name(node.value) if not resource_info: return resource_name, suggested_replacement = resource_info # Get the variable name being assigned var_name = None if node.targets and isinstance(node.targets[0], ast.Name): var_name = node.targets[0].id # Check if this is a safe instantiation if node.value.lineno in safe_with_lines: # Inside a with statement as context manager - safe return if var_name and var_name in close_vars: # Has explicit try/finally with close() in the same function - acceptable return if var_name and var_name in returned_vars: # Variable is returned (factory function pattern) - caller responsible return if var_name and var_name in passed_to_args_vars: # Variable is passed to another function (dependency injection) - receiver responsible return # Not safe - flag this if suggested_replacement: # Function with a known replacement context manager self.errors.append( ( node.lineno, f"{resource_name}() returns a resource that needs cleanup. " f"Use 'with {suggested_replacement} as var:' instead.", ) ) else: # Service class self.errors.append( ( node.lineno, f"{resource_name} should be used with a context manager " f"('with {resource_name}(...) as var:') to ensure proper cleanup.", ) ) def check_file(filename: str) -> bool: """Check a single Python file for service context manager usage.""" if not filename.endswith(".py"): return True if is_file_allowed(filename): return True try: with open(filename, "r", encoding="utf-8") as f: content = f.read() except Exception as e: print(f"Error reading {filename}: {e}") return False try: tree = ast.parse(content, filename=filename) # Check for service instantiations checker = ServiceContextChecker() checker.visit(tree) if checker.errors: print(f"\n{filename}:") for line_num, error in checker.errors: print(f" Line {line_num}: {error}") return False except SyntaxError: # Skip files with syntax errors (let other tools handle that) pass except Exception as e: print(f"Error parsing {filename}: {e}") return False return True def main(): """Main function to check all provided Python files.""" if len(sys.argv) < 2: print("Usage: check-service-context-managers.py ...") sys.exit(1) files_to_check = sys.argv[1:] has_errors = False for filename in files_to_check: if not check_file(filename): has_errors = True if has_errors: print("\n" + "=" * 70) print("Resource Leak Prevention: Use context managers for cleanup!") print("=" * 70) print("\n--- Services ---") print("Services like DownloadService, LibraryRAGService, and") print("LocalEmbeddingManager hold resources that need cleanup.") print("\nTo fix, use context managers:") print(" # Before (leaks resources):") print(" service = DownloadService()") print(" result = service.download(...)") print() print(" # After (proper cleanup):") print(" with DownloadService() as service:") print(" result = service.download(...)") print("\n--- Database Sessions ---") print("get_auth_db_session() returns a raw session that needs cleanup.") print("Use auth_db_session() context manager instead:") print() print(" # Before (may leak on exception):") print(" session = get_auth_db_session()") print(" user = session.query(User).first()") print(" session.close()") print() print(" # After (proper cleanup):") print(" with auth_db_session() as session:") print(" user = session.query(User).first()") print() print("get_db_session() is deprecated and leaks QueuePool connections.") print("Use get_user_db_session(username) context manager instead:") print() print(" # Before (leaks pool connection — never closed):") print(" session = get_db_session(username=username)") print() print(" # After (proper cleanup):") print(" with get_user_db_session(username) as session:") print(" settings = SettingsManager(session)") print("\n--- Alternative: try/finally ---") print(" service = DownloadService()") print(" try:") print(" result = service.download(...)") print(" finally:") print(" service.close()") print("\nNote: Factory functions that return resources are allowed,") print("as the caller is responsible for cleanup.") sys.exit(1) else: sys.exit(0) if __name__ == "__main__": main()