This is an automated email from the ASF dual-hosted git repository. FreeOnePlus pushed a commit to branch agent/sec-016-redact-logs-errors in repository https://gitbox.apache.org/repos/asf/doris-mcp-server.git
commit ac52c52f052cd5f78619af15f03b83abb8976c0b Author: FreeOnePlus <[email protected]> AuthorDate: Thu Jul 30 00:10:09 2026 +0800 fix: redact sensitive logs and errors --- doris_mcp_server/auth/auth_middleware.py | 7 +- doris_mcp_server/auth/doris_oauth_handlers.py | 2 +- doris_mcp_server/auth/jwt_manager.py | 4 +- doris_mcp_server/auth/mcp_auth_middleware.py | 23 ++- doris_mcp_server/auth/oauth_client.py | 2 +- doris_mcp_server/auth/oauth_handlers.py | 37 +++- doris_mcp_server/auth/token_handlers.py | 53 +++-- doris_mcp_server/auth/token_manager.py | 7 +- doris_mcp_server/auth/token_validators.py | 8 +- doris_mcp_server/protocol.py | 60 +++++- doris_mcp_server/tools/resources_manager.py | 18 +- doris_mcp_server/tools/tools_manager.py | 10 +- doris_mcp_server/utils/analysis_tools.py | 32 ++- doris_mcp_server/utils/logger.py | 14 ++ doris_mcp_server/utils/query_executor.py | 10 +- doris_mcp_server/utils/redaction.py | 273 +++++++++++++++++++++++++ doris_mcp_server/utils/schema_extractor.py | 23 ++- doris_mcp_server/utils/security.py | 6 +- test/integration/test_real_doris_transports.py | 17 ++ test/protocol/stdio_capability_server.py | 11 + test/protocol/test_mcp_v2_protocol.py | 44 +++- test/security/test_mcp_auth_middleware.py | 65 ++++++ test/security/test_sensitive_data_redaction.py | 193 +++++++++++++++++ test/tools/test_resources_manager_cache.py | 9 +- test/tools/test_tools_manager.py | 3 +- 25 files changed, 852 insertions(+), 79 deletions(-) diff --git a/doris_mcp_server/auth/auth_middleware.py b/doris_mcp_server/auth/auth_middleware.py index e358063..4e7137e 100644 --- a/doris_mcp_server/auth/auth_middleware.py +++ b/doris_mcp_server/auth/auth_middleware.py @@ -193,9 +193,12 @@ class AuthMiddleware: return await self.app(scope, receive, send_wrapper) - except Exception as e: + except Exception: # Authentication failed, return 401 error - response_body = f'{{"error": "Authentication failed", "message": "{str(e)}"}}' + response_body = ( + '{"error": "Authentication failed", ' + '"message": "Authentication failed"}' + ) await send({ 'type': 'http.response.start', diff --git a/doris_mcp_server/auth/doris_oauth_handlers.py b/doris_mcp_server/auth/doris_oauth_handlers.py index d847152..b6f8bd0 100644 --- a/doris_mcp_server/auth/doris_oauth_handlers.py +++ b/doris_mcp_server/auth/doris_oauth_handlers.py @@ -60,7 +60,7 @@ def protected_resource_error_response(error: Exception, base_url: str) -> JSONRe scope = "tool:list" body = { "error": "authentication_required", - "error_description": str(error) or "Authentication required", + "error_description": "Authentication required", } status_code = 401 return JSONResponse( diff --git a/doris_mcp_server/auth/jwt_manager.py b/doris_mcp_server/auth/jwt_manager.py index fa79d1a..76a080e 100644 --- a/doris_mcp_server/auth/jwt_manager.py +++ b/doris_mcp_server/auth/jwt_manager.py @@ -360,7 +360,7 @@ class JWTManager: # Add to blacklist await self.validator.revoke_token(jti, exp) - logger.info(f"Token {jti} revoked successfully") + logger.info("Token revoked successfully") return True except Exception as e: @@ -468,4 +468,4 @@ class JWTManager: }, 'key_manager': key_info, 'validator': validation_stats - } \ No newline at end of file + } diff --git a/doris_mcp_server/auth/mcp_auth_middleware.py b/doris_mcp_server/auth/mcp_auth_middleware.py index 631480a..67097d0 100644 --- a/doris_mcp_server/auth/mcp_auth_middleware.py +++ b/doris_mcp_server/auth/mcp_auth_middleware.py @@ -25,6 +25,7 @@ from starlette.responses import JSONResponse from ..utils.auth_credentials import BearerCredentials from ..utils.config import EffectiveAuthConfig from ..utils.logger import get_logger +from ..utils.redaction import redact_error_payload from ..utils.security import ( clear_current_auth_context, get_current_auth_context, @@ -90,7 +91,10 @@ class MCPAuthASGIMiddleware: ) else: response = JSONResponse( - {"error": "Authentication required", "message": str(exc)}, + { + "error": "Authentication required", + "message": "Authentication failed", + }, status_code=401, ) await response(scope, receive, send) @@ -109,10 +113,16 @@ class MCPAuthASGIMiddleware: try: reset_auth_context(context_token) except Exception as reset_exc: - logger.error(f"Failed to reset auth context after verification failure: {reset_exc}") + logger.error( + "Failed to reset auth context after verification failure (%s)", + type(reset_exc).__name__, + ) clear_current_auth_context() response = JSONResponse( - {"error": "auth_context_unavailable", "message": str(exc)}, + { + "error": "auth_context_unavailable", + "message": "Authentication context unavailable", + }, status_code=500, ) await response(scope, receive, send) @@ -121,7 +131,7 @@ class MCPAuthASGIMiddleware: try: await self.downstream(scoped_request, receive, send) except OperationAuthorizationError as exc: - body = exc.to_dict() + body = redact_error_payload(exc.to_dict()) if ( self.effective_auth.oauth_discovery_mode == "doris_oauth" and exc.required_scope @@ -149,5 +159,8 @@ class MCPAuthASGIMiddleware: try: reset_auth_context(context_token) except Exception as exc: - logger.error(f"Failed to reset auth context after request: {exc}") + logger.error( + "Failed to reset auth context after request (%s)", + type(exc).__name__, + ) clear_current_auth_context() diff --git a/doris_mcp_server/auth/oauth_client.py b/doris_mcp_server/auth/oauth_client.py index 3e60fc5..a172e76 100644 --- a/doris_mcp_server/auth/oauth_client.py +++ b/doris_mcp_server/auth/oauth_client.py @@ -423,7 +423,7 @@ class OAuthClient: # Build URL authorization_url = f"{self.provider_config.authorization_endpoint}?{urlencode(params)}" - logger.info(f"Built OAuth authorization URL for state: {oauth_state.state}") + logger.info("Built OAuth authorization URL") return authorization_url, oauth_state async def exchange_code_for_tokens(self, code: str, state: str) -> Tuple[OAuthTokens, OAuthState]: diff --git a/doris_mcp_server/auth/oauth_handlers.py b/doris_mcp_server/auth/oauth_handlers.py index 57ce38c..135f6cf 100644 --- a/doris_mcp_server/auth/oauth_handlers.py +++ b/doris_mcp_server/auth/oauth_handlers.py @@ -29,6 +29,7 @@ from starlette.requests import Request from ..utils.config import get_effective_auth_config from ..utils.logger import get_logger +from ..utils.redaction import redact_sensitive_text from .oauth_resource import external_oauth_protected_resource_metadata logger = get_logger(__name__) @@ -71,9 +72,12 @@ class OAuthHandlers: }) except Exception as e: - logger.error(f"OAuth login initiation failed: {e}") + logger.error( + "OAuth login initiation failed (%s)", + type(e).__name__, + ) return JSONResponse( - {"error": f"OAuth login failed: {str(e)}"}, + {"error": "OAuth login failed"}, status_code=500 ) @@ -89,12 +93,19 @@ class OAuthHandlers: # Check for error in callback if "error" in query_params: error_description = query_params.get("error_description", "Unknown error") - logger.warning(f"OAuth callback error: {query_params['error']} - {error_description}") + error_uri = query_params.get("error_uri") + logger.warning("OAuth callback returned an authorization error") return JSONResponse( { - "error": query_params["error"], - "error_description": error_description, - "error_uri": query_params.get("error_uri") + "error": redact_sensitive_text(query_params["error"]), + "error_description": redact_sensitive_text( + error_description + ), + "error_uri": ( + redact_sensitive_text(error_uri) + if error_uri is not None + else None + ) }, status_code=400 ) @@ -124,9 +135,12 @@ class OAuthHandlers: }) except Exception as e: - logger.error(f"OAuth callback handling failed: {e}") + logger.error( + "OAuth callback handling failed (%s)", + type(e).__name__, + ) return JSONResponse( - {"error": f"OAuth callback failed: {str(e)}"}, + {"error": "OAuth callback failed"}, status_code=500 ) @@ -140,9 +154,12 @@ class OAuthHandlers: return JSONResponse(provider_info) except Exception as e: - logger.error(f"Failed to get OAuth provider info: {e}") + logger.error( + "Failed to get OAuth provider info (%s)", + type(e).__name__, + ) return JSONResponse( - {"error": f"Failed to get provider info: {str(e)}"}, + {"error": "Failed to get provider info"}, status_code=500 ) diff --git a/doris_mcp_server/auth/token_handlers.py b/doris_mcp_server/auth/token_handlers.py index 06a4743..45a1407 100644 --- a/doris_mcp_server/auth/token_handlers.py +++ b/doris_mcp_server/auth/token_handlers.py @@ -108,9 +108,9 @@ class TokenHandlers: database=db_data.get("database", "information_schema"), fe_http_port=int(db_data.get("fe_http_port", 8030)) ) - except (ValueError, TypeError) as e: + except (ValueError, TypeError): return JSONResponse({ - "error": f"Invalid database configuration: {str(e)}" + "error": "Invalid database configuration" }, status_code=400) # Validate required fields @@ -149,15 +149,21 @@ class TokenHandlers: }) except Exception as e: - self.logger.error(f"Token creation failed: {e}") + self.logger.error( + "Token creation failed (%s)", + type(e).__name__, + ) return JSONResponse({ - "error": f"Token creation failed: {str(e)}" + "error": "Token creation failed" }, status_code=400) except Exception as e: - self.logger.error(f"Error in handle_create_token: {e}") + self.logger.error( + "Error in handle_create_token (%s)", + type(e).__name__, + ) return JSONResponse({ - "error": f"Internal server error: {str(e)}" + "error": "Internal server error" }, status_code=500) async def handle_revoke_token(self, request: Request) -> JSONResponse: @@ -205,9 +211,12 @@ class TokenHandlers: }, status_code=404) except Exception as e: - self.logger.error(f"Error in handle_revoke_token: {e}") + self.logger.error( + "Error in handle_revoke_token (%s)", + type(e).__name__, + ) return JSONResponse({ - "error": f"Internal server error: {str(e)}" + "error": "Internal server error" }, status_code=500) async def handle_list_tokens(self, request: Request) -> JSONResponse: @@ -235,9 +244,12 @@ class TokenHandlers: }) except Exception as e: - self.logger.error(f"Error in handle_list_tokens: {e}") + self.logger.error( + "Error in handle_list_tokens (%s)", + type(e).__name__, + ) return JSONResponse({ - "error": f"Internal server error: {str(e)}" + "error": "Internal server error" }, status_code=500) async def handle_token_stats(self, request: Request) -> JSONResponse: @@ -264,9 +276,12 @@ class TokenHandlers: }) except Exception as e: - self.logger.error(f"Error in handle_token_stats: {e}") + self.logger.error( + "Error in handle_token_stats (%s)", + type(e).__name__, + ) return JSONResponse({ - "error": f"Internal server error: {str(e)}" + "error": "Internal server error" }, status_code=500) async def handle_cleanup_tokens(self, request: Request) -> JSONResponse: @@ -294,9 +309,12 @@ class TokenHandlers: }) except Exception as e: - self.logger.error(f"Error in handle_cleanup_tokens: {e}") + self.logger.error( + "Error in handle_cleanup_tokens (%s)", + type(e).__name__, + ) return JSONResponse({ - "error": f"Internal server error: {str(e)}" + "error": "Internal server error" }, status_code=500) async def handle_management_page(self, request: Request) -> HTMLResponse: @@ -655,7 +673,10 @@ class TokenHandlers: return HTMLResponse(html_content) except Exception as e: - self.logger.error(f"Error in handle_demo_page: {e}") + self.logger.error( + "Error in handle_demo_page (%s)", + type(e).__name__, + ) error_html = f""" <!DOCTYPE html> <html> @@ -665,7 +686,7 @@ class TokenHandlers: </head> <body> <h1>Token Management Error</h1> - <p>Error loading token management page: {str(e)}</p> + <p>Error loading token management page.</p> </body> </html> """ diff --git a/doris_mcp_server/auth/token_manager.py b/doris_mcp_server/auth/token_manager.py index 2416377..75ff481 100644 --- a/doris_mcp_server/auth/token_manager.py +++ b/doris_mcp_server/auth/token_manager.py @@ -556,10 +556,13 @@ class TokenManager: ) except Exception as e: - self.logger.error(f"Token validation error: {e}") + self.logger.error( + "Token validation error (%s)", + type(e).__name__, + ) return TokenValidationResult( is_valid=False, - error_message=f"Token validation failed: {str(e)}" + error_message="Token validation failed" ) def generate_token(self, length: int = 32) -> str: diff --git a/doris_mcp_server/auth/token_validators.py b/doris_mcp_server/auth/token_validators.py index 43731af..bc680ac 100644 --- a/doris_mcp_server/auth/token_validators.py +++ b/doris_mcp_server/auth/token_validators.py @@ -74,7 +74,7 @@ class TokenBlacklist: exp: Token expiration timestamp """ self._blacklisted_tokens[jti] = exp - logger.info(f"Token {jti} added to blacklist") + logger.info("Token added to blacklist") async def is_blacklisted(self, jti: str) -> bool: """Check if token is blacklisted @@ -98,7 +98,7 @@ class TokenBlacklist: """ if jti in self._blacklisted_tokens: del self._blacklisted_tokens[jti] - logger.info(f"Token {jti} removed from blacklist") + logger.info("Token removed from blacklist") return True return False @@ -341,7 +341,7 @@ class TokenValidator: exp: Token expiration time """ await self.blacklist.add_token(jti, exp) - logger.info(f"Token {jti} has been revoked") + logger.info("Token has been revoked") async def get_validation_stats(self) -> Dict[str, Any]: """Get validation statistics""" @@ -362,4 +362,4 @@ class TokenValidator: async def get_user_rate_limit_info(self, user_id: str) -> Dict[str, Any]: """Get user rate limit information""" - return await self.rate_limiter.get_usage(user_id) \ No newline at end of file + return await self.rate_limiter.get_usage(user_id) diff --git a/doris_mcp_server/protocol.py b/doris_mcp_server/protocol.py index de4cd63..0790d1d 100644 --- a/doris_mcp_server/protocol.py +++ b/doris_mcp_server/protocol.py @@ -52,7 +52,12 @@ from mcp.types import ( Tool, ) -from .auth.operation_policy import authorize_operation +from .auth.operation_policy import OperationAuthorizationError, authorize_operation +from .utils.redaction import ( + redact_error_payload, + redact_sensitive_text, + redact_uri, +) from .utils.security import get_current_auth_context @@ -109,6 +114,20 @@ def _decode_structured_tool_result(payload: str) -> tuple[Any | None, bool]: return decoded, "error" in decoded +def _sanitize_manager_error_payload( + payload: str, +) -> tuple[str, Any | None, bool]: + decoded, is_error = _decode_structured_tool_result(payload) + if not is_error: + return payload, decoded, False + decoded = redact_error_payload(decoded) + return ( + json.dumps(decoded, ensure_ascii=False, indent=2), + decoded, + True, + ) + + _RESOURCE_INVALID_PARAMS_MESSAGES = { "INVALID_RESOURCE_URI": "Invalid resource URI", "RESOURCE_NOT_FOUND": "Resource not found", @@ -168,6 +187,7 @@ def create_doris_mcp_server( ) -> ReadResourceResult: authorize_operation(get_current_auth_context(), "read_resource") content = await resources_manager.read_resource(params.uri) + content, _, _ = _sanitize_manager_error_payload(content) if ctx.protocol_version == LATEST_PROTOCOL_VERSION: request_error = _decode_resource_request_error(content) if request_error is not None: @@ -176,7 +196,7 @@ def create_doris_mcp_server( code=INVALID_PARAMS, message=message, data={ - "uri": str(params.uri), + "uri": redact_uri(str(params.uri)), "resourceErrorCode": error_code, }, ) @@ -206,8 +226,21 @@ def create_doris_mcp_server( ) -> CallToolResult: del ctx arguments = params.arguments or {} - payload = await tools_manager.call_tool(params.name, arguments) - structured_content, is_error = _decode_structured_tool_result(payload) + try: + payload = await tools_manager.call_tool(params.name, arguments) + except OperationAuthorizationError: + raise + except Exception: + logger.exception("Tool execution failed") + payload = json.dumps( + { + "error": "Tool execution failed", + "error_code": "TOOL_EXECUTION_FAILED", + } + ) + payload, structured_content, is_error = _sanitize_manager_error_payload( + payload + ) return CallToolResult( content=[TextContent(type="text", text=payload)], structured_content=structured_content, @@ -240,7 +273,7 @@ def create_doris_mcp_server( message = _PROMPT_INVALID_PARAMS_MESSAGES.get(prompt_error_code) if message is not None: data = { - "name": params.name, + "name": redact_sensitive_text(params.name), "promptErrorCode": prompt_error_code, } argument = getattr(exc, "argument", None) @@ -286,6 +319,23 @@ def create_doris_mcp_server( on_get_prompt=get_prompt, ) + async def hide_unhandled_errors( + ctx: ServerRequestContext, + call_next: CallNext, + ) -> Any: + try: + return await call_next(ctx) + except (MCPError, OperationAuthorizationError): + raise + except Exception as exc: + logger.exception("Unhandled MCP request failure for %s", ctx.method) + raise MCPError( + code=INTERNAL_ERROR, + message="Internal server error", + ) from exc + + server.middleware.append(hide_unhandled_errors) + if required_client_capabilities or required_tool_capabilities: requirements = dict(required_client_capabilities or {}) tool_requirements = dict(required_tool_capabilities or {}) diff --git a/doris_mcp_server/tools/resources_manager.py b/doris_mcp_server/tools/resources_manager.py index 1f615bb..b481c36 100644 --- a/doris_mcp_server/tools/resources_manager.py +++ b/doris_mcp_server/tools/resources_manager.py @@ -28,9 +28,14 @@ from urllib.parse import quote, unquote from mcp.types import Resource from ..utils.db import DorisConnectionManager +from ..utils.logger import get_logger +from ..utils.redaction import redact_uri from ..utils.sql_security_utils import get_auth_context +logger = get_logger(__name__) + + class TableMetadata: """Data table metadata""" @@ -268,7 +273,7 @@ class DorisResourcesManager: except Exception as e: self._reraise_if_doris_oauth_resource_error(e) - print(f"Failed to get resource list: {e}") + logger.exception("Failed to get resource list") return resources @@ -334,9 +339,16 @@ class DorisResourcesManager: except Exception as e: self._reraise_if_doris_oauth_resource_error(e) + if isinstance(e, InvalidResourceURIError): + message = "Invalid resource URI" + elif isinstance(e, ResourceNotFoundError): + message = "Resource not found" + else: + logger.exception("Failed to read resource") + message = "Resource read failed" payload = { - "error": f"Failed to read resource: {str(e)}", - "uri": uri, + "error": message, + "uri": redact_uri(uri), } if isinstance(e, InvalidResourceURIError | ResourceNotFoundError): payload["error_code"] = e.error_code diff --git a/doris_mcp_server/tools/tools_manager.py b/doris_mcp_server/tools/tools_manager.py index 89e5e30..c352797 100644 --- a/doris_mcp_server/tools/tools_manager.py +++ b/doris_mcp_server/tools/tools_manager.py @@ -1459,11 +1459,13 @@ No parameters required. Returns connection status, configuration, and diagnostic except OperationAuthorizationError: raise except Exception as e: - logger.error(f"Tool call failed {name}: {str(e)}") + logger.error( + "Tool call failed (%s)", + type(e).__name__, + ) error_result = { - "error": str(e), - "tool_name": name, - "arguments": arguments, + "error": "Tool execution failed", + "error_code": "TOOL_EXECUTION_FAILED", "timestamp": datetime.now().isoformat(), } return json.dumps(error_result, ensure_ascii=False, indent=2) diff --git a/doris_mcp_server/utils/analysis_tools.py b/doris_mcp_server/utils/analysis_tools.py index d50f3d3..ecf33a7 100644 --- a/doris_mcp_server/utils/analysis_tools.py +++ b/doris_mcp_server/utils/analysis_tools.py @@ -428,7 +428,7 @@ class SQLAnalyzer: explain_type = "EXPLAIN VERBOSE" if verbose else "EXPLAIN" explain_sql = f"{explain_type} {sql.strip().rstrip(';')}" - logger.info(f"Executing explain query: {explain_sql}") + logger.info("Executing explain query") # Execute context switching and explain query on one routed connection # whenever a database/catalog context is requested. @@ -618,7 +618,7 @@ class SQLAnalyzer: logger.info(f"Enabled profile") # Execute the SQL statement - logger.info(f"Executing SQL with trace ID: {sql}") + logger.info("Executing SQL with trace ID %s", trace_id) start_time = time.time() sql_result = await connection.execute(sql, auth_context=auth_context) execution_time = time.time() - start_time @@ -871,14 +871,17 @@ class SQLAnalyzer: content_type = response.headers.get('content-type', '') response_text = await response.text() logger.info(f"Response content type: {content_type}") - logger.info(f"Response body: {response_text}") + logger.info( + "Query ID response body length: %s", + len(response_text), + ) # Parse JSON response (regardless of content-type) if response_text.strip(): try: import json result = json.loads(response_text) - logger.info(f"Query ID API response: {result}") + logger.info("Query ID API returned JSON") # Parse response according to Doris API format if result.get("code") == 0 and result.get("data"): @@ -911,7 +914,10 @@ class SQLAnalyzer: else: logger.error(f"HTTP request failed with status {response.status}") response_text = await response.text() - logger.error(f"Response body: {response_text}") + logger.error( + "Query ID response body omitted (length=%s)", + len(response_text), + ) return None @@ -957,7 +963,7 @@ class SQLAnalyzer: if 'application/json' in content_type: try: result = await response.json() - logger.info(f"Profile JSON response: {result}") + logger.info("Profile API returned JSON") if result.get("code") == 0 and result.get("data"): profile_text = result["data"].get("profile", "") @@ -987,7 +993,9 @@ class SQLAnalyzer: "api_endpoint": url } else: - logger.warning(f"Profile not found or empty: {response_text}") + logger.warning( + "Profile not found or empty" + ) continue # Try next URL elif response.status == 404: @@ -996,7 +1004,10 @@ class SQLAnalyzer: else: logger.error(f"Profile HTTP request failed with status {response.status} at {url}") response_text = await response.text() - logger.error(f"Response body: {response_text}") + logger.error( + "Profile response body omitted (length=%s)", + len(response_text), + ) continue # Try next URL return None @@ -1093,7 +1104,10 @@ class SQLAnalyzer: else: logger.error(f"HTTP request failed with status {response.status}") response_text = await response.text() - logger.error(f"Response body: {response_text}") + logger.error( + "Table size response body omitted (length=%s)", + len(response_text), + ) return { "success": False, "error": f"HTTP request failed with status {response.status}", diff --git a/doris_mcp_server/utils/logger.py b/doris_mcp_server/utils/logger.py index 9dc612d..eb3c68e 100644 --- a/doris_mcp_server/utils/logger.py +++ b/doris_mcp_server/utils/logger.py @@ -35,6 +35,11 @@ from typing import Any, Optional from datetime import datetime, timedelta import threading +from .redaction import SensitiveDataFilter + + +_sensitive_data_filter = SensitiveDataFilter() + class TimestampedFormatter(logging.Formatter): """Custom formatter with enhanced timestamp and structured format""" @@ -348,6 +353,7 @@ class DorisLoggerManager: if enable_console: console_handler = logging.StreamHandler(sys.stdout) console_handler.setLevel(getattr(logging, level.upper())) + console_handler.addFilter(_sensitive_data_filter) console_formatter = TimestampedFormatter( fmt="%(asctime)s.%(msecs)03d %(level_aligned)s %(name)s - %(message)s", datefmt="%Y-%m-%d %H:%M:%S" @@ -364,6 +370,7 @@ class DorisLoggerManager: backup_count=backup_count ) level_handler.setLevel(logging.DEBUG) # Accept all levels + level_handler.addFilter(_sensitive_data_filter) handlers.append(level_handler) # Combined application log (all levels in one file) @@ -376,6 +383,7 @@ class DorisLoggerManager: encoding='utf-8' ) app_handler.setLevel(getattr(logging, level.upper())) + app_handler.addFilter(_sensitive_data_filter) app_formatter = TimestampedFormatter() app_handler.setFormatter(app_formatter) handlers.append(app_handler) @@ -396,12 +404,14 @@ class DorisLoggerManager: backupCount=backup_count, encoding='utf-8' ) + audit_handler.addFilter(_sensitive_data_filter) audit_formatter = TimestampedFormatter( fmt="%(asctime)s.%(msecs)03d [AUDIT] %(name)s - %(message)s", datefmt="%Y-%m-%d %H:%M:%S" ) audit_handler.setFormatter(audit_formatter) audit_logger.addHandler(audit_handler) + audit_logger.addFilter(_sensitive_data_filter) audit_logger.propagate = False # Don't propagate to root logger # Add all handlers to root logger @@ -455,6 +465,8 @@ class DorisLoggerManager: for logger_name in package_loggers: logger = logging.getLogger(logger_name) logger.setLevel(getattr(logging, level.upper())) + if _sensitive_data_filter not in logger.filters: + logger.addFilter(_sensitive_data_filter) # Don't add handlers here - they inherit from root logger def get_logger(self, name: str) -> logging.Logger: @@ -469,6 +481,8 @@ class DorisLoggerManager: """ if name not in self.loggers: logger = logging.getLogger(name) + if _sensitive_data_filter not in logger.filters: + logger.addFilter(_sensitive_data_filter) self.loggers[name] = logger return self.loggers[name] diff --git a/doris_mcp_server/utils/query_executor.py b/doris_mcp_server/utils/query_executor.py index 269cc9d..744e387 100644 --- a/doris_mcp_server/utils/query_executor.py +++ b/doris_mcp_server/utils/query_executor.py @@ -417,7 +417,7 @@ class DorisQueryExecutor: ) if cached_result: self.metrics.cache_hits += 1 - self.logger.debug(f"Cache hit for query: {query_request.sql[:50]}...") + self.logger.debug("Query cache hit") return cached_result.result self.metrics.cache_misses += 1 @@ -802,7 +802,9 @@ class DorisQueryExecutor: validation_result = await security_manager.validate_sql_security(sql, auth_context) if not validation_result.is_valid: - self.logger.warning(f"SQL security validation failed for query: {sql[:100]}...") + self.logger.warning( + "SQL security validation rejected a query" + ) return { "success": False, "error": f"SQL security validation failed: {validation_result.error_message}", @@ -819,7 +821,9 @@ class DorisQueryExecutor: } } else: - self.logger.debug(f"SQL security validation passed for query: {sql[:100]}...") + self.logger.debug( + "SQL security validation passed" + ) except Exception as security_error: self.logger.error(f"Security validation error: {str(security_error)}") # In case of security validation error, fail safe diff --git a/doris_mcp_server/utils/redaction.py b/doris_mcp_server/utils/redaction.py new file mode 100644 index 0000000..86e2b19 --- /dev/null +++ b/doris_mcp_server/utils/redaction.py @@ -0,0 +1,273 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Secret redaction helpers for logs and public error payloads.""" + +from __future__ import annotations + +import logging +import re +import traceback +from collections.abc import Mapping +from types import TracebackType +from typing import Any + +REDACTED = "[REDACTED]" + +_SENSITIVE_KEYS = frozenset( + { + "authorization", + "proxyauthorization", + "password", + "passwd", + "pwd", + "dbpassword", + "admintoken", + "customtoken", + "token", + "authtoken", + "sessiontoken", + "accesstoken", + "refreshtoken", + "idtoken", + "bearertoken", + "secret", + "secretkey", + "clientsecret", + "tokensecret", + "apikey", + "privatekey", + "credential", + "credentials", + "cookie", + "setcookie", + } +) +_SQL_KEYS = frozenset({"sql", "query", "statement", "sqltext", "querytext"}) +_ERROR_DROP_KEYS = frozenset( + { + "arguments", + "headers", + "payload", + "rawrequest", + "request", + "requestbody", + } +) + +_TEXT_SECRET_KEY = ( + r"(?:authorization|proxy[-_ ]authorization|password|passwd|pwd|" + r"db[-_ ]password|admin[-_ ]token|custom[-_ ]token|auth[-_ ]token|" + r"session[-_ ]token|token|secret|secret[-_ ]key|" + r"access[-_ ]token|refresh[-_ ]token|id[-_ ]token|bearer[-_ ]token|" + r"client[-_ ]secret|token[-_ ]secret|api[-_ ]key|private[-_ ]key|" + r"cookie|set[-_ ]cookie)" +) +_QUOTED_KEY_VALUE_RE = re.compile( + rf"(?i)(?P<prefix>[\"']?{_TEXT_SECRET_KEY}[\"']?\s*[:=]\s*)" + r"(?P<quote>[\"'])(?P<value>.*?)(?P=quote)" +) +_AUTHORIZATION_VALUE_RE = re.compile( + r"(?i)(?P<prefix>\b(?:authorization|proxy[-_ ]authorization)" + r"\s*[:=]\s*)(?:(?:bearer|basic|token)\s+)?" + r"(?P<value>[^\s,;\"'}]+)" +) +_UNQUOTED_KEY_VALUE_RE = re.compile( + rf"(?i)(?P<prefix>\b{_TEXT_SECRET_KEY}\b\s*[:=]\s*)" + r"(?P<value>(?![\"'\[])[^\s,;&}\]]+)" +) +_AUTH_SCHEME_RE = re.compile( + r"(?i)\b(?P<scheme>bearer|basic|token)\s+" + r"(?P<value>[A-Za-z0-9._~+/=-]+)" +) +_QUERY_SECRET_RE = re.compile( + rf"(?i)(?P<prefix>[?&]{_TEXT_SECRET_KEY}=)(?P<value>[^&#\s]*)" +) +_DSN_PASSWORD_RE = re.compile( + r"(?i)(?P<prefix>[a-z][a-z0-9+.-]*://[^:/@\s]+:)" + r"(?P<password>[^@\s/]+)(?P<suffix>@)" +) + +_SQL_KEYWORD_RE = re.compile( + r"(?i)\b(?:select|insert|update|delete|replace|merge|create|alter|drop|" + r"truncate|explain|with)\b" +) +_SQL_BLOCK_COMMENT_RE = re.compile(r"/\*.*?\*/", re.DOTALL) +_SQL_LINE_COMMENT_RE = re.compile(r"--[^\r\n]*") +_SQL_SINGLE_QUOTED_RE = re.compile(r"'(?:''|\\.|[^'])*'") +_SQL_DOUBLE_QUOTED_RE = re.compile(r'"(?:""|\\.|[^"])*"') +_SQL_HEX_RE = re.compile(r"(?i)\b(?:0x[0-9a-f]+|x'[0-9a-f]+')\b") +_SQL_NUMBER_RE = re.compile(r"(?<![\w.])[-+]?\d+(?:\.\d+)?(?![\w.])") + +_ERROR_DETAIL_RE = re.compile( + r"(?is)^(?P<prefix>.*?\b(?:failed|failure|error|exception)\b" + r"[^:\n]{0,160}:)\s*.+$" +) + + +def _normalized_key(key: object) -> str: + return re.sub(r"[^a-z0-9]", "", str(key).casefold()) + + +def redact_sql_literals(value: str) -> str: + """Remove values and comments from SQL while retaining diagnostic shape.""" + if not _SQL_KEYWORD_RE.search(value): + return value + value = _SQL_BLOCK_COMMENT_RE.sub("/* [REDACTED] */", value) + value = _SQL_LINE_COMMENT_RE.sub("-- [REDACTED]", value) + value = _SQL_HEX_RE.sub(REDACTED, value) + value = _SQL_SINGLE_QUOTED_RE.sub(f"'{REDACTED}'", value) + value = _SQL_DOUBLE_QUOTED_RE.sub(f'"{REDACTED}"', value) + return _SQL_NUMBER_RE.sub("?", value) + + +def redact_sensitive_text(value: str) -> str: + """Redact recognized credentials, secret fields, URIs, and SQL literals.""" + + def quoted_replacement(match: re.Match[str]) -> str: + return f"{match.group('prefix')}{match.group('quote')}{REDACTED}{match.group('quote')}" + + value = _AUTHORIZATION_VALUE_RE.sub( + lambda match: f"{match.group('prefix')}{REDACTED}", + value, + ) + value = _QUOTED_KEY_VALUE_RE.sub(quoted_replacement, value) + value = _UNQUOTED_KEY_VALUE_RE.sub( + lambda match: f"{match.group('prefix')}{REDACTED}", + value, + ) + value = _AUTH_SCHEME_RE.sub( + lambda match: f"{match.group('scheme')} {REDACTED}", + value, + ) + value = _QUERY_SECRET_RE.sub( + lambda match: f"{match.group('prefix')}{REDACTED}", + value, + ) + value = _DSN_PASSWORD_RE.sub( + lambda match: (f"{match.group('prefix')}{REDACTED}{match.group('suffix')}"), + value, + ) + return redact_sql_literals(value) + + +def redact_log_message(value: str) -> str: + """Redact a log message and suppress untrusted exception detail suffixes.""" + redacted = redact_sensitive_text(value) + match = _ERROR_DETAIL_RE.match(redacted) + if match is None: + return redacted + return f"{match.group('prefix')} {REDACTED}" + + +def redact_sensitive_data(value: Any, *, key: object | None = None) -> Any: + """Recursively redact data by semantic key and recognizable string format.""" + normalized_key = _normalized_key(key) if key is not None else "" + if normalized_key in _SENSITIVE_KEYS or normalized_key in _SQL_KEYS: + return REDACTED + if isinstance(value, Mapping): + return { + item_key: redact_sensitive_data(item_value, key=item_key) + for item_key, item_value in value.items() + } + if isinstance(value, list): + return [redact_sensitive_data(item) for item in value] + if isinstance(value, tuple): + return tuple(redact_sensitive_data(item) for item in value) + if isinstance(value, set): + return {redact_sensitive_data(item) for item in value} + if isinstance(value, BaseException): + return value.__class__.__name__ + if isinstance(value, str): + return redact_sensitive_text(value) + if isinstance(value, bytes): + return redact_sensitive_text(value.decode("utf-8", errors="replace")).encode() + return value + + +def redact_error_payload(value: Any) -> Any: + """Sanitize an error-only response without touching successful results.""" + if isinstance(value, Mapping): + sanitized = {} + for key, item_value in value.items(): + normalized_key = _normalized_key(key) + if normalized_key in _ERROR_DROP_KEYS: + continue + sanitized[key] = redact_error_payload( + REDACTED + if normalized_key in _SENSITIVE_KEYS or normalized_key in _SQL_KEYS + else item_value + ) + return sanitized + if isinstance(value, list): + return [redact_error_payload(item) for item in value] + if isinstance(value, tuple): + return tuple(redact_error_payload(item) for item in value) + if isinstance(value, str): + return redact_sensitive_text(value) + return value + + +def redact_uri(value: str) -> str: + """Redact credential-bearing URI components without changing safe URIs.""" + return redact_sensitive_text(value) + + +def _safe_exception_text( + exc_info: tuple[ + type[BaseException], + BaseException, + TracebackType | None, + ], +) -> str: + exception_type, _exception, traceback_value = exc_info + frames = traceback.format_list(traceback.extract_tb(traceback_value)) + frames.append(f"{exception_type.__name__}: {REDACTED}\n") + return "".join(frames) + + +class SensitiveDataFilter(logging.Filter): + """Apply defense-in-depth redaction before any configured log handler.""" + + def filter(self, record: logging.LogRecord) -> bool: + dropped_error_details = False + if isinstance(record.msg, str): + sanitized_message = redact_sensitive_text(record.msg) + dropped_error_details = ( + _ERROR_DETAIL_RE.match(sanitized_message) is not None + ) + record.msg = redact_log_message(record.msg) + else: + record.msg = redact_sensitive_data(record.msg) + + if dropped_error_details: + record.args = () + elif isinstance(record.args, Mapping): + record.args = redact_sensitive_data(record.args) + elif isinstance(record.args, tuple): + record.args = tuple( + redact_sensitive_data(argument) for argument in record.args + ) + + if ( + record.exc_info + and record.exc_info[0] is not None + and record.exc_info[1] is not None + ): + record.exc_text = _safe_exception_text(record.exc_info) + if record.stack_info: + record.stack_info = redact_sensitive_text(record.stack_info) + return True diff --git a/doris_mcp_server/utils/schema_extractor.py b/doris_mcp_server/utils/schema_extractor.py index 22f57a3..e6da0bb 100644 --- a/doris_mcp_server/utils/schema_extractor.py +++ b/doris_mcp_server/utils/schema_extractor.py @@ -386,7 +386,11 @@ class MetadataExtractor: """ result = self._execute_query_with_catalog(query, db_name, effective_catalog) - logger.info(f"{effective_catalog or 'default'}.{db_name}.information_schema.tables query result: {result}") + logger.info( + "%s.%s.information_schema.tables query completed", + effective_catalog or "default", + db_name, + ) if not result: tables = [] @@ -815,7 +819,7 @@ class MetadataExtractor: if effective_catalog: safe_catalog = quote_identifier(effective_catalog, "catalog name") query = f"SHOW INDEX FROM {safe_catalog}.{safe_db}.{safe_table}" - logger.info(f"Using three-part naming for index query: {query}") + logger.info("Using three-part naming for index query") else: query = f"SHOW INDEX FROM {safe_db}.{safe_table}" @@ -1288,7 +1292,10 @@ class MetadataExtractor: try: if catalog_name and 'information_schema' in query.lower(): modified_query = query.replace('information_schema', f'{catalog_name}.information_schema') - logger.info(f"Modified query for catalog {catalog_name}: {modified_query}") + logger.info( + "Prepared catalog-qualified query for %s", + catalog_name, + ) return await self._execute_query_async(modified_query, db_name) else: return await self._execute_query_async(query, db_name) @@ -1633,7 +1640,7 @@ class MetadataExtractor: if effective_catalog: safe_catalog = quote_identifier(effective_catalog, "catalog name") query = f"SHOW INDEX FROM {safe_catalog}.{safe_db}.{safe_table}" - logger.info(f"Using three-part naming for async index query: {query}") + logger.info("Using three-part naming for async index query") else: query = f"SHOW INDEX FROM {safe_db}.{safe_table}" @@ -1753,7 +1760,13 @@ class MetadataExtractor: FIX for Issue #62 Bug 1: Now retrieves auth_context from context variable to support token-bound database configuration FIX for Issue #62 Bug 3: Now uses db_name and catalog_name parameters to switch database context """ - logger.info(f"Executing SQL query: {sql}, DB: {db_name}, Catalog: {catalog_name}, MaxRows: {max_rows}, Timeout: {timeout}") + logger.info( + "Executing SQL query for DB=%s, catalog=%s, max_rows=%s, timeout=%s", + db_name, + catalog_name, + max_rows, + timeout, + ) try: if not sql: diff --git a/doris_mcp_server/utils/security.py b/doris_mcp_server/utils/security.py index d6acb27..e1e6915 100644 --- a/doris_mcp_server/utils/security.py +++ b/doris_mcp_server/utils/security.py @@ -1087,7 +1087,11 @@ class SQLSecurityValidator: if not parsed.tokens or str(parsed).strip() == '': continue - self.logger.debug(f"Validating SQL statement {idx + 1}/{len(all_statements)}: {str(parsed)[:100]}...") + self.logger.debug( + "Validating SQL statement %s/%s", + idx + 1, + len(all_statements), + ) # Check blocked operations first (more specific) keyword_result = await self._check_blocked_keywords(parsed) diff --git a/test/integration/test_real_doris_transports.py b/test/integration/test_real_doris_transports.py index 28afb28..df9099a 100644 --- a/test/integration/test_real_doris_transports.py +++ b/test/integration/test_real_doris_transports.py @@ -25,6 +25,7 @@ to 9030 and ``DORIS_REAL_PASSWORD`` may be empty. from __future__ import annotations import asyncio +import json import os import re import secrets @@ -369,6 +370,22 @@ async def test_real_doris_read_write_permission_timeout_and_recovery( assert verify_result.is_error is False assert verify_payload["data"] == [{"marker": doris_sandbox.marker}] + response_secret = f"sec016-{secrets.token_hex(12)}" + sensitive_error_result, sensitive_error_payload = await _exec_query( + client, + ( + f"SELECT '{response_secret}' AS marker " + f"FROM `{doris_sandbox.table}_missing`" + ), + ) + assert sensitive_error_result.is_error is True + assert sensitive_error_payload["success"] is False + serialized_error = json.dumps( + sensitive_error_result.model_dump(by_alias=True, mode="json"), + ensure_ascii=False, + ) + assert response_secret not in serialized_error + timeout_result, timeout_payload = await _exec_query( client, "SELECT SLEEP(2) AS slept", diff --git a/test/protocol/stdio_capability_server.py b/test/protocol/stdio_capability_server.py index d0d222e..ad588fc 100644 --- a/test/protocol/stdio_capability_server.py +++ b/test/protocol/stdio_capability_server.py @@ -231,6 +231,17 @@ class OneToolManager: ] async def call_tool(self, name: str, arguments: dict) -> str: + if name == "echo" and arguments.get("fail"): + return json.dumps( + { + "error": ( + f"query failed: password={arguments['password']}; " + f"token={arguments['token']}; {arguments['sql']}" + ), + "arguments": arguments, + "token": arguments["token"], + } + ) if name == "get_sql_profile": return json.dumps( await self.profile_analyzer.get_sql_profile( diff --git a/test/protocol/test_mcp_v2_protocol.py b/test/protocol/test_mcp_v2_protocol.py index a050239..aa816e6 100644 --- a/test/protocol/test_mcp_v2_protocol.py +++ b/test/protocol/test_mcp_v2_protocol.py @@ -94,6 +94,17 @@ class StubToolsManager: async def call_tool(self, name: str, arguments: dict) -> str: if name == "fail": + if arguments: + return json.dumps( + { + "error": ( + f"query failed: password={arguments['password']}; " + f"token={arguments['token']}; {arguments['sql']}" + ), + "arguments": arguments, + "token": arguments["token"], + } + ) return json.dumps({"error": "expected failure"}) return json.dumps({"name": name, "arguments": arguments}) @@ -574,13 +585,26 @@ async def test_http_enforces_tool_specific_client_capabilities_and_recovers(): "arguments": {"value": "capable"}, } + secret = "http-secret-sec-016" recovered = await client.post( "/mcp", - json=modern_tool_request(3, "fail"), + json=modern_tool_request( + 3, + "fail", + { + "password": secret, + "token": secret, + "sql": f"SELECT '{secret}'", + }, + ), headers=modern_tool_headers("fail"), ) assert recovered.status_code == 200 assert recovered.json()["result"]["isError"] is True + assert secret not in recovered.text + structured = recovered.json()["result"]["structuredContent"] + assert "arguments" not in structured + assert structured["token"] == "[REDACTED]" @pytest.mark.asyncio @@ -863,6 +887,24 @@ async def test_stdio_validates_capabilities_versions_and_process_survival(): "monitor_data_freshness", "analyze_data_access_patterns", ] + secret = "stdio-secret-sec-016" + error_result = await capable.call_tool( + "echo", + { + "fail": True, + "password": secret, + "token": secret, + "sql": f"SELECT '{secret}'", + }, + ) + assert error_result.is_error is True + serialized = json.dumps( + error_result.model_dump(by_alias=True, mode="json"), + ensure_ascii=False, + ) + assert secret not in serialized + assert "arguments" not in error_result.structured_content + assert error_result.structured_content["token"] == "[REDACTED]" async with Client(stdio_client(server_params), mode="legacy") as legacy: assert [tool.name for tool in (await legacy.list_tools()).tools] == [ diff --git a/test/security/test_mcp_auth_middleware.py b/test/security/test_mcp_auth_middleware.py index 7aec4b5..ad9b9c4 100644 --- a/test/security/test_mcp_auth_middleware.py +++ b/test/security/test_mcp_auth_middleware.py @@ -118,6 +118,71 @@ async def test_mcp_auth_middleware_rejects_query_string_token(): assert get_current_auth_context() is None [email protected] [email protected]( + ("auth_methods", "discovery_mode", "base_url"), + [ + (("token",), "none", ""), + ( + ("doris_oauth",), + "doris_oauth", + "https://mcp.example.test";, + ), + ], +) +async def test_mcp_auth_middleware_does_not_reflect_unexpected_auth_details( + auth_methods, + discovery_mode, + base_url, +): + secret = "auth-provider-secret-sec-016" + + class SecurityManager: + async def authenticate_request(self, credentials): + del credentials + raise RuntimeError( + f"Authorization: Bearer {secret}; password={secret}" + ) + + async def downstream(scope, receive, send): + raise AssertionError("downstream must not be called") + + messages = [] + middleware = MCPAuthASGIMiddleware( + SecurityManager(), + downstream, + _effective( + auth_methods=auth_methods, + discovery_mode=discovery_mode, + base_url=base_url, + ), + ) + await middleware( + { + "type": "http", + "path": "/mcp", + "headers": [(b"authorization", f"Bearer {secret}".encode())], + "client": ("127.0.0.1", 1), + }, + _receive, + _send_collector(messages), + ) + + assert messages[0]["status"] == 401 + assert secret.encode() not in messages[1]["body"] + body = json.loads(messages[1]["body"]) + if discovery_mode == "doris_oauth": + assert body == { + "error": "authentication_required", + "error_description": "Authentication required", + } + else: + assert body == { + "error": "Authentication required", + "message": "Authentication failed", + } + + @pytest.mark.asyncio async def test_mcp_auth_middleware_returns_doris_oauth_challenge_on_401(): class SecurityManager: diff --git a/test/security/test_sensitive_data_redaction.py b/test/security/test_sensitive_data_redaction.py new file mode 100644 index 0000000..289b987 --- /dev/null +++ b/test/security/test_sensitive_data_redaction.py @@ -0,0 +1,193 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import io +import json +import logging +from unittest.mock import Mock + +import pytest + +from doris_mcp_server.tools.resources_manager import DorisResourcesManager +from doris_mcp_server.tools.tools_manager import DorisToolsManager +from doris_mcp_server.utils.redaction import ( + REDACTED, + SensitiveDataFilter, + redact_error_payload, + redact_sensitive_data, + redact_sensitive_text, +) + +AUTH_SECRET = "auth-secret-sec-016" +PASSWORD_SECRET = "password-secret-sec-016" +TOKEN_SECRET = "token-secret-sec-016" +URI_SECRET = "uri-secret-sec-016" +SQL_SECRET = "customer-secret-sec-016" + + +def assert_secrets_absent(value) -> None: + serialized = ( + value + if isinstance(value, str) + else json.dumps(value, ensure_ascii=False, default=str) + ) + for secret in ( + AUTH_SECRET, + PASSWORD_SECRET, + TOKEN_SECRET, + URI_SECRET, + SQL_SECRET, + ): + assert secret not in serialized + + +def test_recursive_redaction_uses_keys_without_hiding_safe_token_ids(): + payload = { + "Authorization": f"Bearer {AUTH_SECRET}", + "database": {"user": "alice", "password": PASSWORD_SECRET}, + "access_token": TOKEN_SECRET, + "secret_key": TOKEN_SECRET, + "token_id": "public-token-id", + "sql": f"SELECT * FROM customer WHERE email = '{SQL_SECRET}'", + } + + redacted = redact_sensitive_data(payload) + + assert redacted["Authorization"] == REDACTED + assert redacted["database"]["password"] == REDACTED + assert redacted["access_token"] == REDACTED + assert redacted["secret_key"] == REDACTED + assert redacted["token_id"] == "public-token-id" + assert redacted["sql"] == REDACTED + assert_secrets_absent(redacted) + + +def test_text_redaction_covers_headers_dsn_query_parameters_and_sql_literals(): + message = ( + f"Authorization: Bearer {AUTH_SECRET}; " + f"password='{PASSWORD_SECRET}'; " + f"mysql://alice:{URI_SECRET}@db.example.test/analytics" + f"?access_token={TOKEN_SECRET}; " + f"SELECT * FROM customer WHERE email = '{SQL_SECRET}' AND id = 42" + ) + + redacted = redact_sensitive_text(message) + + assert redacted.count(REDACTED) >= 5 + assert "id = ?" in redacted + assert_secrets_absent(redacted) + + +def test_error_payload_drops_request_material_and_preserves_safe_diagnostics(): + payload = { + "error": ( + f"Access denied password={PASSWORD_SECRET}; " + f"SELECT * FROM customer WHERE email='{SQL_SECRET}'" + ), + "error_code": "QUERY_FAILED", + "arguments": { + "Authorization": f"Bearer {AUTH_SECRET}", + "token": TOKEN_SECRET, + }, + "details": {"access_token": TOKEN_SECRET}, + } + + redacted = redact_error_payload(payload) + + assert redacted["error_code"] == "QUERY_FAILED" + assert "arguments" not in redacted + assert redacted["details"]["access_token"] == REDACTED + assert_secrets_absent(redacted) + + +def test_logging_filter_redacts_arguments_and_exception_tracebacks(): + output = io.StringIO() + handler = logging.StreamHandler(output) + handler.addFilter(SensitiveDataFilter()) + handler.setFormatter(logging.Formatter("%(levelname)s %(message)s")) + logger = logging.Logger("sec-016-redaction-test") + logger.addHandler(handler) + logger.setLevel(logging.DEBUG) + + logger.info( + "request headers=%s payload=%s", + {"Authorization": f"Bearer {AUTH_SECRET}"}, + { + "password": PASSWORD_SECRET, + "token": TOKEN_SECRET, + "sql": f"SELECT '{SQL_SECRET}'", + }, + ) + try: + raise RuntimeError( + f"backend echoed password={PASSWORD_SECRET} and token={TOKEN_SECRET}" + ) + except RuntimeError: + logger.exception("Backend operation failed") + + rendered = output.getvalue() + assert REDACTED in rendered + assert "RuntimeError" in rendered + assert_secrets_absent(rendered) + + [email protected] +async def test_tool_manager_never_returns_exception_or_arguments(): + manager = object.__new__(DorisToolsManager) + + async def fail_with_secrets(arguments): + del arguments + raise RuntimeError( + f"password={PASSWORD_SECRET}; token={TOKEN_SECRET}; SELECT '{SQL_SECRET}'" + ) + + manager._exec_query_tool = fail_with_secrets + result = await manager.call_tool( + "exec_query", + { + "sql": f"SELECT '{SQL_SECRET}'", + "password": PASSWORD_SECRET, + "token": TOKEN_SECRET, + }, + ) + payload = json.loads(result) + + assert payload["error"] == "Tool execution failed" + assert payload["error_code"] == "TOOL_EXECUTION_FAILED" + assert "arguments" not in payload + assert_secrets_absent(payload) + + [email protected] +async def test_resource_error_redacts_uri_and_backend_exception(): + manager = DorisResourcesManager(Mock()) + + def fail_to_parse(uri): + del uri + raise RuntimeError( + f"password={PASSWORD_SECRET}; token={TOKEN_SECRET}; SELECT '{SQL_SECRET}'" + ) + + manager._parse_resource_uri = fail_to_parse + result = await manager.read_resource( + f"doris://table/orders?access_token={URI_SECRET}" + ) + payload = json.loads(result) + + assert payload["error"] == "Resource read failed" + assert payload["uri"].endswith(f"access_token={REDACTED}") + assert_secrets_absent(payload) diff --git a/test/tools/test_resources_manager_cache.py b/test/tools/test_resources_manager_cache.py index 77596cd..4dcff8c 100644 --- a/test/tools/test_resources_manager_cache.py +++ b/test/tools/test_resources_manager_cache.py @@ -320,14 +320,15 @@ def test_parse_stats_resource_distinguishes_legacy_current_database_from_literal @pytest.mark.asyncio -async def test_legacy_read_resource_keeps_json_error_body_compatibility(): +async def test_legacy_read_resource_hides_backend_error_details(): manager = DorisResourcesManager(RaisingConnectionManager()) result = await manager.read_resource("doris://table/orders") payload = json.loads(result) assert payload["uri"] == "doris://table/orders" - assert "metadata backend failed" in payload["error"] + assert payload["error"] == "Resource read failed" + assert "metadata backend failed" not in payload["error"] assert "error_code" not in payload @@ -339,7 +340,7 @@ async def test_read_resource_marks_invalid_uri_for_protocol_boundary(): payload = json.loads(result) assert payload == { - "error": "Failed to read resource: Invalid resource URI format", + "error": "Invalid resource URI", "error_code": "INVALID_RESOURCE_URI", "uri": "https://example.com/orders";, } @@ -362,7 +363,7 @@ async def test_read_resource_marks_missing_table_for_protocol_boundary(): payload = json.loads(result) assert payload == { - "error": "Failed to read resource: Table missing does not exist", + "error": "Resource not found", "error_code": "RESOURCE_NOT_FOUND", "uri": "doris://table/missing", } diff --git a/test/tools/test_tools_manager.py b/test/tools/test_tools_manager.py index 83b29ac..9c94ab6 100644 --- a/test/tools/test_tools_manager.py +++ b/test/tools/test_tools_manager.py @@ -240,7 +240,8 @@ class TestDorisToolsManager: assert "error" in result_data or "success" in result_data if "error" in result_data: - assert "Unknown tool" in result_data["error"] + assert result_data["error"] == "Tool execution failed" + assert result_data["error_code"] == "TOOL_EXECUTION_FAILED" @pytest.mark.asyncio async def test_missing_required_arguments(self, tools_manager): --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
