已合并
【安全排查】【重明问题修复】收口配置驱动动态导入与表达式求值风险 #388
ChaseChe77创建于 6月27日
【安全排查】【重明问题修复】收口配置驱动动态导入与表达式求值风险 #388
已合并
共 14 个文件变更+550-190
| @@ -66,6 +66,7 @@ from typing import Callable, Dict, List, Optional, Tuple, ContextManager | |||
| 66 | 66 | ||
| 67 | from ms_service_metric.utils.logger import get_logger | 67 | from ms_service_metric.utils.logger import get_logger |
| 68 | from ms_service_metric.utils.exceptions import HandlerError | 68 | from ms_service_metric.utils.exceptions import HandlerError |
| 69 | +from ms_service_metric.utils.import_security import is_allowed_handler_module | ||
| 69 | from ms_service_metric.metrics.metrics_manager import MetricConfig, MetricType | 70 | from ms_service_metric.metrics.metrics_manager import MetricConfig, MetricType |
| 70 | 71 | ||
| 71 | logger = get_logger("handler") | 72 | logger = get_logger("handler") |
| @@ -473,6 +474,8 @@ class MetricHandler(Handler): | |||
| 473 | 474 | ||
| 474 | module_path, func_name = handler_path.rsplit(':', 1) | 475 | module_path, func_name = handler_path.rsplit(':', 1) |
| 475 | logger.debug("Importing handler: %s.%s", module_path, func_name) | 476 | logger.debug("Importing handler: %s.%s", module_path, func_name) |
| 477 | + if not is_allowed_handler_module(module_path): | ||
| 478 | + raise HandlerError(f"Handler module is not allowed: {module_path}") | ||
| 476 | 479 | ||
| 477 | # 导入模块 | 480 | # 导入模块 |
| 478 | module = importlib.import_module(module_path) | 481 | module = importlib.import_module(module_path) |
| @@ -38,6 +38,7 @@ from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional | |||
| 38 | 38 | ||
| 39 | from ms_service_metric.utils.exceptions import SymbolError | 39 | from ms_service_metric.utils.exceptions import SymbolError |
| 40 | from ms_service_metric.utils.logger import get_logger | 40 | from ms_service_metric.utils.logger import get_logger |
| 41 | +from ms_service_metric.utils.import_security import is_allowed_symbol_module | ||
| 41 | from ms_service_metric.core.handler import MetricHandler, HandlerType | 42 | from ms_service_metric.core.handler import MetricHandler, HandlerType |
| 42 | from ms_service_metric.core.hook.hook_chain import HookChain | 43 | from ms_service_metric.core.hook.hook_chain import HookChain |
| 43 | 44 | ||
| @@ -723,6 +724,9 @@ class Symbol: | |||
| 723 | """ | 724 | """ |
| 724 | try: | 725 | try: |
| 725 | # 导入模块 | 726 | # 导入模块 |
| 727 | + if not is_allowed_symbol_module(self._module_path): | ||
| 728 | + logger.error("Symbol module is not allowed: %s", self._module_path) | ||
| 729 | + return None | ||
| 726 | module = importlib.import_module(self._module_path) | 730 | module = importlib.import_module(self._module_path) |
| 727 | 731 | ||
| 728 | # 解析属性路径 | 732 | # 解析属性路径 |
| @@ -0,0 +1,38 @@ | |||
| 1 | +# ------------------------------------------------------------------------- | ||
| 2 | +# This file is part of the MindStudio project. | ||
| 3 | +# Copyright (c) 2025 Huawei Technologies Co.,Ltd. | ||
| 4 | +# | ||
| 5 | +# MindStudio is licensed under Mulan PSL v2. | ||
| 6 | +# You can use this software according to the terms and conditions of the Mulan PSL v2. | ||
| 7 | +# You may obtain a copy of Mulan PSL v2 at: | ||
| 8 | +# | ||
| 9 | +# http://license.coscl.org.cn/MulanPSL2 | ||
| 10 | +# | ||
| 11 | +# THIS SOFTWARE IS PROVIDED ON AN "AS IS" BASIS, WITHOUT WARRANTIES OF ANY KIND, | ||
| 12 | +# MERCHANTABILITY OR CONDITIONS OF ANY KIND. | ||
| 13 | +# See the Mulan PSL v2 for more details. | ||
| 14 | +# ------------------------------------------------------------------------- | ||
| 15 | +# pylint: disable=duplicate-code | ||
| 16 | + | ||
| 17 | +"""Allow-list checks for ms_service_metric configuration-driven imports.""" | ||
| 18 | + | ||
| 19 | +ALLOWED_HANDLER_MODULE_PREFIXES = ( | ||
| 20 | + "ms_service_profiler.", | ||
| 21 | + "ms_service_metric.", | ||
| 22 | +) | ||
| 23 | + | ||
| 24 | +ALLOWED_SYMBOL_MODULE_PREFIXES = ( | ||
| 25 | + "vllm.", | ||
| 26 | + "vllm_ascend.", | ||
| 27 | + "sglang.", | ||
| 28 | + "ms_service_profiler.", | ||
| 29 | + "ms_service_metric.", | ||
| 30 | +) | ||
| 31 | + | ||
| 32 | + | ||
| 33 | +def is_allowed_handler_module(module_path: str) -> bool: | ||
| 34 | + return isinstance(module_path, str) and module_path.startswith(ALLOWED_HANDLER_MODULE_PREFIXES) | ||
| 35 | + | ||
| 36 | + | ||
| 37 | +def is_allowed_symbol_module(module_path: str) -> bool: | ||
| 38 | + return isinstance(module_path, str) and module_path.startswith(ALLOWED_SYMBOL_MODULE_PREFIXES) | ||
| @@ -13,9 +13,10 @@ | |||
| 13 | # - min_version/max_version: 可选版本约束 | 13 | # - min_version/max_version: 可选版本约束 |
| 14 | # - caller_filter: 可选,仅当由特定调用者触发时才生效 | 14 | # - caller_filter: 可选,仅当由特定调用者触发时才生效 |
| 15 | # - attributes: 可选,自定义属性采集配置 | 15 | # - attributes: 可选,自定义属性采集配置 |
| 16 | -# - expr 中直接使用参数名或 return | 16 | +# - expr 中使用 args[index]、kwargs['name'] 或 return |
| 17 | # - 支持管道操作 |:len(return) | str 等价于 str(len(return)) | 17 | # - 支持管道操作 |:len(return) | str 等价于 str(len(return)) |
| 18 | -# - 支持 attr 操作:return[0] | attr input_ids 获取对象的 input_ids 属性 | 18 | +# - 支持直接属性访问:return[0].input_ids 获取对象的 input_ids 属性 |
| 19 | +# - 也支持在管道中使用 attr:return[0] | attr input_ids | ||
| 19 | # | 20 | # |
| 20 | # 三、示例 | 21 | # 三、示例 |
| 21 | # 示例 A:自定义 handler(单一 symbol) | 22 | # 示例 A:自定义 handler(单一 symbol) |
| @@ -40,8 +41,8 @@ | |||
| 40 | name: computing_logits | 41 | name: computing_logits |
| 41 | attributes: | 42 | attributes: |
| 42 | - name: input_ids_len | 43 | - name: input_ids_len |
| 43 | - expr: len(input_ids) | 44 | + expr: len(kwargs['input_ids']) |
| 44 | - name: logits_shape_str | 45 | - name: logits_shape_str |
| 45 | expr: len(return) | str | 46 | expr: len(return) | str |
| 46 | - name: first_output_input_ids_len | 47 | - name: first_output_input_ids_len |
| 47 | - expr: return[0] | attr input_ids | len | 48 | + expr: len(return[0].input_ids) |
| @@ -1,5 +1,4 @@ | |||
| 1 | # ------------------------------------------------------------------------- | 1 | # ------------------------------------------------------------------------- |
| 2 | -# pylint: disable=logging-fstring-interpolation | ||
| 3 | # This file is part of the MindStudio project. | 2 | # This file is part of the MindStudio project. |
| 4 | # Copyright (c) 2025 Huawei Technologies Co.,Ltd. | 3 | # Copyright (c) 2025 Huawei Technologies Co.,Ltd. |
| 5 | # | 4 | # |
| @@ -14,6 +13,7 @@ | |||
| 14 | # MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. | 13 | # MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. |
| 15 | # See the Mulan PSL v2 for more details. | 14 | # See the Mulan PSL v2 for more details. |
| 16 | # ------------------------------------------------------------------------- | 15 | # ------------------------------------------------------------------------- |
| 16 | +# pylint: disable=logging-fstring-interpolation | ||
| 17 | 17 | ||
| 18 | """ | 18 | """ |
| 19 | ConfigLoader: 加载 YAML 配置并解析为 Handler 列表。 | 19 | ConfigLoader: 加载 YAML 配置并解析为 Handler 列表。 |
| @@ -34,6 +34,7 @@ from .utils import load_yaml_config | |||
| 34 | from .logger import logger | 34 | from .logger import logger |
| 35 | from .dynamic_hook import make_default_time_hook, ConfigHooker | 35 | from .dynamic_hook import make_default_time_hook, ConfigHooker |
| 36 | from .metric_hook import wrap_handler_with_metrics | 36 | from .metric_hook import wrap_handler_with_metrics |
| 37 | +from .import_security import is_allowed_handler_module | ||
| 37 | 38 | ||
| 38 | 39 | ||
| 39 | def _is_pattern_symbol(symbol_path: str) -> bool: | 40 | def _is_pattern_symbol(symbol_path: str) -> bool: |
| @@ -97,6 +98,9 @@ def _resolve_handler_func(symbol_info: dict, method_name: str) -> Callable: | |||
| 97 | if isinstance(handler_path, str) and ':' in handler_path: | 98 | if isinstance(handler_path, str) and ':' in handler_path: |
| 98 | try: | 99 | try: |
| 99 | mod_str, func_name = handler_path.split(':', 1) | 100 | mod_str, func_name = handler_path.split(':', 1) |
| 101 | + if not is_allowed_handler_module(mod_str): | ||
| 102 | + logger.warning("Handler module '%s' is not allowed, using default", mod_str) | ||
| 103 | + raise ImportError(f"Handler module is not allowed: {mod_str}") | ||
| 100 | mod_obj = importlib.import_module(mod_str) | 104 | mod_obj = importlib.import_module(mod_str) |
| 101 | func = getattr(mod_obj, func_name, None) | 105 | func = getattr(mod_obj, func_name, None) |
| 102 | if callable(func): | 106 | if callable(func): |
| @@ -126,6 +130,9 @@ def _resolve_metrics_handler_func(symbol_info: dict, method_name: str) -> Callab | |||
| 126 | if isinstance(handler_path, str) and ':' in handler_path: | 130 | if isinstance(handler_path, str) and ':' in handler_path: |
| 127 | try: | 131 | try: |
| 128 | mod_str, func_name = handler_path.split(':', 1) | 132 | mod_str, func_name = handler_path.split(':', 1) |
| 133 | + if not is_allowed_handler_module(mod_str): | ||
| 134 | + logger.warning("Metrics handler module '%s' is not allowed, using wrap_handler_with_metrics", mod_str) | ||
| 135 | + raise ImportError(f"Metrics handler module is not allowed: {mod_str}") | ||
| 129 | mod_obj = importlib.import_module(mod_str) | 136 | mod_obj = importlib.import_module(mod_str) |
| 130 | func = getattr(mod_obj, func_name, None) | 137 | func = getattr(mod_obj, func_name, None) |
| 131 | if callable(func): | 138 | if callable(func): |
| @@ -1,5 +1,4 @@ | |||
| 1 | # ------------------------------------------------------------------------- | 1 | # ------------------------------------------------------------------------- |
| 2 | -# pylint: disable=comparison-with-callable,eval-used,global-variable-not-assigned,logging-fstring-interpolation,ungrouped-imports | ||
| 3 | # This file is part of the MindStudio project. | 2 | # This file is part of the MindStudio project. |
| 4 | # Copyright (c) 2025 Huawei Technologies Co.,Ltd. | 3 | # Copyright (c) 2025 Huawei Technologies Co.,Ltd. |
| 5 | # | 4 | # |
| @@ -14,9 +13,14 @@ | |||
| 14 | # MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. | 13 | # MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. |
| 15 | # See the Mulan PSL v2 for more details. | 14 | # See the Mulan PSL v2 for more details. |
| 16 | # ------------------------------------------------------------------------- | 15 | # ------------------------------------------------------------------------- |
| 16 | +# pylint: disable=logging-fstring-interpolation,global-variable-not-assigned | ||
| 17 | +# pylint: disable=comparison-with-callable,ungrouped-imports | ||
| 17 | 18 | ||
| 19 | +import ast | ||
| 18 | import importlib | 20 | import importlib |
| 19 | import inspect | 21 | import inspect |
| 22 | +import io | ||
| 23 | +import tokenize | ||
| 20 | from dataclasses import dataclass | 24 | from dataclasses import dataclass |
| 21 | from typing import Tuple, List, Optional, Callable, Dict, Any | 25 | from typing import Tuple, List, Optional, Callable, Dict, Any |
| 22 | 26 | ||
| @@ -34,6 +38,7 @@ except Exception: | |||
| 34 | Profiler = None # type: ignore | 38 | Profiler = None # type: ignore |
| 35 | Level = None # type: ignore | 39 | Level = None # type: ignore |
| 36 | from .module_hook import VLLMHookerBase, import_object_from_string | 40 | from .module_hook import VLLMHookerBase, import_object_from_string |
| 41 | +from .import_security import is_allowed_handler_module | ||
| 37 | 42 | ||
| 38 | 43 | ||
| 39 | 44 | ||
| @@ -416,102 +421,193 @@ def _build_safe_locals(ctx: FuncCallContext): | |||
| 416 | "args": ctx.args, | 421 | "args": ctx.args, |
| 417 | "kwargs": ctx.kwargs, | 422 | "kwargs": ctx.kwargs, |
| 418 | "return": ctx.ret_val, | 423 | "return": ctx.ret_val, |
| 424 | + "ret": ctx.ret_val, | ||
| 419 | "self": self_obj, | 425 | "self": self_obj, |
| 420 | - "len": len, | ||
| 421 | - "str": str, | ||
| 422 | - "attr": _get_object_attribute, | ||
| 423 | } | 426 | } |
| 424 | - try: | ||
| 425 | - safe_locals.update({k: v for k, v in named_params.items() if k != "self"}) | ||
| 426 | - except Exception as e: | ||
| 427 | - logger.warning(f"Failed to update safe locals: {e}") | ||
| 428 | return safe_locals | 427 | return safe_locals |
| 429 | 428 | ||
| 430 | 429 | ||
| 430 | +_SAFE_EXPR_FUNCTIONS = { | ||
| 431 | + "len": len, | ||
| 432 | + "str": str, | ||
| 433 | + "int": int, | ||
| 434 | + "float": float, | ||
| 435 | + "bool": bool, | ||
| 436 | +} | ||
| 437 | + | ||
| 438 | + | ||
| 439 | +def _normalize_expr(expr_str: str) -> str: | ||
| 440 | + """Keep YAML compatibility for the 'return' pseudo variable. | ||
| 441 | + | ||
| 442 | + Replace only standalone NAME tokens so string literals like "return" | ||
| 443 | + keep their original meaning. | ||
| 444 | + """ | ||
| 445 | + try: | ||
| 446 | + tokens = [] | ||
| 447 | + for token in tokenize.generate_tokens(io.StringIO(expr_str).readline): | ||
| 448 | + if token.type == tokenize.NAME and token.string == "return": | ||
| 449 | + token = tokenize.TokenInfo(token.type, "ret", token.start, token.end, token.line) | ||
| 450 | + tokens.append(token) | ||
| 451 | + return tokenize.untokenize(tokens) | ||
| 452 | + except tokenize.TokenError: | ||
| 453 | + return expr_str | ||
| 454 | + | ||
| 455 | + | ||
| 456 | +def _is_safe_attribute_name(attr_name: str) -> bool: | ||
| 457 | + return isinstance(attr_name, str) and attr_name and not attr_name.startswith("_") and "__" not in attr_name | ||
| 458 | + | ||
| 459 | + | ||
| 460 | +class _SafeExpressionEvaluator: | ||
| 461 | + """AST evaluator for profiler attribute expressions.""" | ||
| 462 | + | ||
| 463 | + def __init__(self, safe_locals: Dict[str, Any]): | ||
| 464 | + self.safe_locals = safe_locals | ||
| 465 | + | ||
| 466 | + def validate(self, node: ast.AST) -> None: | ||
| 467 | + if isinstance(node, ast.Expression): | ||
| 468 | + self.validate(node.body) | ||
| 469 | + return | ||
| 470 | + if isinstance(node, (ast.Constant, ast.Name)): | ||
| 471 | + return | ||
| 472 | + if isinstance(node, (ast.Tuple, ast.List)): | ||
| 473 | + for item in node.elts: | ||
| 474 | + self.validate(item) | ||
| 475 | + return | ||
| 476 | + if isinstance(node, ast.Subscript): | ||
| 477 | + self.validate(node.value) | ||
| 478 | + self.validate(node.slice) | ||
| 479 | + return | ||
| 480 | + if isinstance(node, ast.Attribute): | ||
| 481 | + if not _is_safe_attribute_name(node.attr): | ||
| 482 | + raise ValueError(f"Unsafe attribute access: {node.attr}") | ||
| 483 | + self.validate(node.value) | ||
| 484 | + return | ||
| 485 | + if isinstance(node, ast.Call): | ||
| 486 | + if not isinstance(node.func, ast.Name) or node.func.id not in _SAFE_EXPR_FUNCTIONS: | ||
| 487 | + raise ValueError("Only simple allow-listed function calls are supported") | ||
| 488 | + if node.keywords: | ||
| 489 | + raise ValueError("Keyword arguments are not supported") | ||
| 490 | + for arg in node.args: | ||
| 491 | + if isinstance(arg, ast.Call): | ||
| 492 | + raise ValueError("Nested function calls are not supported") | ||
| 493 | + self.validate(arg) | ||
| 494 | + return | ||
| 495 | + raise ValueError(f"Unsupported expression node: {type(node).__name__}") | ||
| 496 | + | ||
| 497 | + def evaluate(self, node: ast.AST) -> Any: | ||
| 498 | + if isinstance(node, ast.Expression): | ||
| 499 | + return self.evaluate(node.body) | ||
| 500 | + if isinstance(node, ast.Constant): | ||
| 501 | + return node.value | ||
| 502 | + if isinstance(node, ast.Name): | ||
| 503 | + if node.id in _SAFE_EXPR_FUNCTIONS: | ||
| 504 | + raise ValueError(f"Function name cannot be used as a value: {node.id}") | ||
| 505 | + if node.id not in self.safe_locals: | ||
| 506 | + raise NameError(f"Undefined params: {node.id}") | ||
| 507 | + value = self.safe_locals[node.id] | ||
| 508 | + if callable(value): | ||
| 509 | + raise ValueError(f"Callable value is not allowed: {node.id}") | ||
| 510 | + return value | ||
| 511 | + if isinstance(node, ast.Tuple): | ||
| 512 | + return tuple(self.evaluate(item) for item in node.elts) | ||
| 513 | + if isinstance(node, ast.List): | ||
| 514 | + return [self.evaluate(item) for item in node.elts] | ||
| 515 | + if isinstance(node, ast.Subscript): | ||
| 516 | + return self.evaluate(node.value)[self.evaluate(node.slice)] | ||
| 517 | + if isinstance(node, ast.Attribute): | ||
| 518 | + value = getattr(self.evaluate(node.value), node.attr) | ||
| 519 | + if callable(value): | ||
| 520 | + raise ValueError(f"Callable attribute is not allowed: {node.attr}") | ||
| 521 | + return value | ||
| 522 | + if isinstance(node, ast.Call): | ||
| 523 | + func = _SAFE_EXPR_FUNCTIONS[node.func.id] | ||
| 524 | + return func(*(self.evaluate(arg) for arg in node.args)) | ||
| 525 | + raise ValueError(f"Unsupported expression node: {type(node).__name__}") | ||
| 526 | + | ||
| 527 | + | ||
| 528 | +def _parse_safe_expression(expr_str: str) -> ast.Expression: | ||
| 529 | + return ast.parse(_normalize_expr(expr_str), mode="eval") | ||
| 530 | + | ||
| 531 | + | ||
| 532 | +def _validate_direct_expression(expr_str: str) -> bool: | ||
| 533 | + try: | ||
| 534 | + tree = _parse_safe_expression(expr_str) | ||
| 535 | + _SafeExpressionEvaluator({}).validate(tree) | ||
| 536 | + return True | ||
| 537 | + except Exception as e: | ||
| 538 | + logger.warning(f"Expression validation failed: {expr_str}, err={e}") | ||
| 539 | + return False | ||
| 540 | + | ||
| 541 | + | ||
| 431 | def _validate_expression_safety(expr_str): | 542 | def _validate_expression_safety(expr_str): |
| 432 | """验证表达式安全性,只允许预定义的安全操作。""" | 543 | """验证表达式安全性,只允许预定义的安全操作。""" |
| 433 | - dangerous_chars = ['import', 'exec', 'eval', '__', 'open', 'file', 'input', 'raw_input'] | 544 | + if '|' not in expr_str: |
| 434 | - # 允许管道符 '|' | 545 | + return _validate_direct_expression(expr_str) |
| 435 | - dangerous_ops = ['+', '-', '*', '/', '%', '**', '//', '&', '^', '~', '<<', '>>'] | 546 | + parts = [part.strip() for part in expr_str.split('|')] |
| 436 | - expr_lower = expr_str.lower() | 547 | + if len(parts) < 2 or not parts[0]: |
| 437 | - for dangerous in dangerous_chars: | 548 | + return _validate_direct_expression(expr_str) |
| 438 | - if dangerous in expr_lower: | 549 | + if not _validate_direct_expression(parts[0]): |
| 439 | - logger.warning(f"Expression contains dangerous keyword: {dangerous}") | 550 | + return False |
| 440 | - return False | 551 | + for operation in parts[1:]: |
| 441 | - for op in dangerous_ops: | 552 | + if operation in ("len", "str"): |
| 442 | - if op in expr_str: | 553 | + continue |
| 443 | - logger.warning(f"Expression contains dangerous operator: {op}") | 554 | + if operation.startswith("attr "): |
| 444 | - return False | 555 | + attr_name = operation[5:].strip() |
| 445 | - if expr_str.count('(') != expr_str.count(')'): | 556 | + if _is_safe_attribute_name(attr_name): |
| 446 | - logger.warning(f"Unmatched parentheses in expression: {expr_str}") | 557 | + continue |
| 558 | + logger.warning(f"Pipe operation not allowed: {operation}") | ||
| 447 | return False | 559 | return False |
| 448 | - if '(' in expr_str and ')' in expr_str: | ||
| 449 | - func_name = expr_str.split('(')[0].strip() | ||
| 450 | - allowed_functions = ['len', 'str', 'int', 'float', 'bool', 'attr'] | ||
| 451 | - if func_name not in allowed_functions: | ||
| 452 | - logger.warning(f"Function call not allowed: {func_name}") | ||
| 453 | - return False | ||
| 454 | return True | 560 | return True |
| 455 | 561 | ||
| 456 | 562 | ||
| 457 | -def _execute_direct_expression(expr_str, safe_locals): | 563 | +def _execute_safe_expression(expr_str, safe_locals): |
| 458 | - """安全执行表达式,严格控制输入参数。""" | 564 | + """Execute a safe AST expression with optional len/str pipe operations.""" |
| 459 | - try: | 565 | + if not _validate_expression_safety(expr_str): |
| 460 | - if not _validate_expression_safety(expr_str): | ||
| 461 | - return None | ||
| 462 | - # 特殊处理关键名 'return'(不是合法标识符) | ||
| 463 | - trimmed = expr_str.strip() | ||
| 464 | - if trimmed == 'return': | ||
| 465 | - return safe_locals.get('return') | ||
| 466 | - safe_globals = { | ||
| 467 | - "__builtins__": { | ||
| 468 | - "len": len, | ||
| 469 | - "str": str, | ||
| 470 | - "int": int, | ||
| 471 | - "float": float, | ||
| 472 | - "bool": bool, | ||
| 473 | - } | ||
| 474 | - } | ||
| 475 | - return eval(expr_str, safe_globals, safe_locals) # nosec B307 | ||
| 476 | - except Exception as e: | ||
| 477 | - logger.warning(f"Safe eval failed: {expr_str}, err={e}") | ||
| 478 | return None | 566 | return None |
| 479 | - | ||
| 480 | - | ||
| 481 | -def _apply_pipe_operation(result, operation): | ||
| 482 | - """应用单个管道操作。""" | ||
| 483 | - if operation == 'str': | ||
| 484 | - return str(result) | ||
| 485 | - elif operation == 'len': | ||
| 486 | - return len(result) if result is not None else None | ||
| 487 | - elif operation.startswith('attr '): | ||
| 488 | - attr_name = operation[5:].strip() | ||
| 489 | - return _get_object_attribute(result, attr_name) | ||
| 490 | - else: | ||
| 491 | - logger.warning(f"Unknown pipe operation: {operation}") | ||
| 492 | - return None | ||
| 493 | - | ||
| 494 | - | ||
| 495 | -def _execute_pipe_expression(expr_str, safe_locals): | ||
| 496 | - """执行管道表达式。""" | ||
| 497 | if '|' not in expr_str: | 567 | if '|' not in expr_str: |
| 498 | - return _execute_direct_expression(expr_str, safe_locals) | 568 | + return _execute_direct_expression_ast(expr_str, safe_locals) |
| 499 | parts = [part.strip() for part in expr_str.split('|')] | 569 | parts = [part.strip() for part in expr_str.split('|')] |
| 500 | - if len(parts) < 2: | 570 | + if len(parts) < 2 or not parts[0]: |
| 501 | - return _execute_direct_expression(expr_str, safe_locals) | 571 | + return _execute_direct_expression_ast(expr_str, safe_locals) |
| 502 | - result = _execute_direct_expression(parts[0], safe_locals) | 572 | + result = _execute_direct_expression_ast(parts[0], safe_locals) |
| 503 | for operation in parts[1:]: | 573 | for operation in parts[1:]: |
| 504 | - result = _apply_pipe_operation(result, operation) | 574 | + if operation == "len": |
| 575 | + result = len(result) if result is not None else None | ||
| 576 | + elif operation == "str": | ||
| 577 | + result = str(result) | ||
| 578 | + elif operation.startswith("attr "): | ||
| 579 | + attr_name = operation[5:].strip() | ||
| 580 | + if not _is_safe_attribute_name(attr_name): | ||
| 581 | + logger.warning(f"Pipe operation not allowed: {operation}") | ||
| 582 | + return None | ||
| 583 | + result = _get_object_attribute(result, attr_name) | ||
| 584 | + else: | ||
| 585 | + logger.warning(f"Pipe operation not allowed: {operation}") | ||
| 586 | + return None | ||
| 505 | if result is None: | 587 | if result is None: |
| 506 | break | 588 | break |
| 507 | return result | 589 | return result |
| 508 | 590 | ||
| 509 | 591 | ||
| 592 | +def _execute_direct_expression_ast(expr_str, safe_locals): | ||
| 593 | + try: | ||
| 594 | + trimmed = expr_str.strip() | ||
| 595 | + if trimmed == 'return': | ||
| 596 | + return safe_locals.get('return') | ||
| 597 | + tree = _parse_safe_expression(expr_str) | ||
| 598 | + evaluator = _SafeExpressionEvaluator(safe_locals) | ||
| 599 | + evaluator.validate(tree) | ||
| 600 | + return evaluator.evaluate(tree) | ||
| 601 | + except Exception as e: | ||
| 602 | + logger.warning(f"Safe eval failed: {expr_str}, err={e}") | ||
| 603 | + return None | ||
| 604 | + | ||
| 605 | + | ||
| 510 | def _safe_eval_expr(expr: str, ctx: FuncCallContext): | 606 | def _safe_eval_expr(expr: str, ctx: FuncCallContext): |
| 511 | - """安全执行表达式,支持管道操作和 attr 操作。""" | 607 | + """安全执行表达式,支持 len/str 管道操作。""" |
| 512 | try: | 608 | try: |
| 513 | safe_locals = _build_safe_locals(ctx) | 609 | safe_locals = _build_safe_locals(ctx) |
| 514 | - return _execute_pipe_expression(expr, safe_locals) | 610 | + return _execute_safe_expression(expr, safe_locals) |
| 515 | except Exception as e: | 611 | except Exception as e: |
| 516 | logger.warning(f"Pipe eval failed: {expr}, err={e}") | 612 | logger.warning(f"Pipe eval failed: {expr}, err={e}") |
| 517 | return None | 613 | return None |
| @@ -571,7 +667,7 @@ def make_default_time_hook(domain: str, name: str, attributes: Optional[List[Dic | |||
| 571 | expr = item.get("expr") | 667 | expr = item.get("expr") |
| 572 | if not attr_name or not expr: | 668 | if not attr_name or not expr: |
| 573 | continue | 669 | continue |
| 574 | - # 在 expr 中直接使用参数名或 return 来表示数据来源 | 670 | + # Use args/kwargs/return in expr to identify the data source. |
| 575 | ctx = FuncCallContext( | 671 | ctx = FuncCallContext( |
| 576 | func_obj=original_func, | 672 | func_obj=original_func, |
| 577 | this_obj=args[0] if len(args) > 0 else None, | 673 | this_obj=args[0] if len(args) > 0 else None, |
| @@ -653,6 +749,9 @@ class HandlerResolver: | |||
| 653 | """ | 749 | """ |
| 654 | try: | 750 | try: |
| 655 | mod, func_name = handler_val.split(":", 1) | 751 | mod, func_name = handler_val.split(":", 1) |
| 752 | + if not is_allowed_handler_module(mod): | ||
| 753 | + logger.warning("Handler module '%s' is not allowed", mod) | ||
| 754 | + return None | ||
| 656 | mod_obj = importlib.import_module(mod) | 755 | mod_obj = importlib.import_module(mod) |
| 657 | # Avoid Mock auto-creation: inspect module dict directly | 756 | # Avoid Mock auto-creation: inspect module dict directly |
| 658 | value = getattr(mod_obj, "__dict__", {}).get(func_name, None) | 757 | value = getattr(mod_obj, "__dict__", {}).get(func_name, None) |
| @@ -0,0 +1,41 @@ | |||
| 1 | +# ------------------------------------------------------------------------- | ||
| 2 | +# This file is part of the MindStudio project. | ||
| 3 | +# Copyright (c) 2025 Huawei Technologies Co.,Ltd. | ||
| 4 | +# | ||
| 5 | +# MindStudio is licensed under Mulan PSL v2. | ||
| 6 | +# You can use this software according to the terms and conditions of the Mulan PSL v2. | ||
| 7 | +# You may obtain a copy of Mulan PSL v2 at: | ||
| 8 | +# | ||
| 9 | +# http://license.coscl.org.cn/MulanPSL2 | ||
| 10 | +# | ||
| 11 | +# THIS SOFTWARE IS PROVIDED ON AN "AS IS" BASIS, WITHOUT WARRANTIES OF ANY KIND, | ||
| 12 | +# EITHER EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO NON-INFRINGEMENT, | ||
| 13 | +# MERCHANTABILITY OR CONDITIONS OF ANY KIND. | ||
| 14 | +# See the Mulan PSL v2 for more details. | ||
| 15 | +# ------------------------------------------------------------------------- | ||
| 16 | +# pylint: disable=duplicate-code | ||
| 17 | + | ||
| 18 | +"""Shared allow-list checks for configuration-driven imports.""" | ||
| 19 | + | ||
| 20 | +ALLOWED_HANDLER_MODULE_PREFIXES = ( | ||
| 21 | + "ms_service_profiler.", | ||
| 22 | + "ms_service_metric.", | ||
| 23 | +) | ||
| 24 | + | ||
| 25 | +ALLOWED_SYMBOL_MODULE_PREFIXES = ( | ||
| 26 | + "vllm.", | ||
| 27 | + "vllm_ascend.", | ||
| 28 | + "sglang.", | ||
| 29 | + "ms_service_profiler.", | ||
| 30 | + "ms_service_metric.", | ||
| 31 | +) | ||
| 32 | + | ||
| 33 | + | ||
| 34 | +def is_allowed_handler_module(module_path: str) -> bool: | ||
| 35 | + """Return True when a configured handler module is in a trusted namespace.""" | ||
| 36 | + return isinstance(module_path, str) and module_path.startswith(ALLOWED_HANDLER_MODULE_PREFIXES) | ||
| 37 | + | ||
| 38 | + | ||
| 39 | +def is_allowed_symbol_module(module_path: str) -> bool: | ||
| 40 | + """Return True when a configured hook target module is in an expected namespace.""" | ||
| 41 | + return isinstance(module_path, str) and module_path.startswith(ALLOWED_SYMBOL_MODULE_PREFIXES) | ||
| @@ -1,5 +1,5 @@ | |||
| 1 | # ------------------------------------------------------------------------- | 1 | # ------------------------------------------------------------------------- |
| 2 | -# pylint: disable=attribute-defined-outside-init,comparison-with-callable,logging-fstring-interpolation,no-name-in-module,unnecessary-dunder-call | 2 | +# pylint: disable=attribute-defined-outside-init,comparison-with-callable,logging-fstring-interpolation,unnecessary-dunder-call |
| 3 | # This file is part of the MindStudio project. | 3 | # This file is part of the MindStudio project. |
| 4 | # Copyright (c) 2025 Huawei Technologies Co.,Ltd. | 4 | # Copyright (c) 2025 Huawei Technologies Co.,Ltd. |
| 5 | # | 5 | # |
| @@ -27,6 +27,7 @@ from packaging.version import Version | |||
| 27 | from .logger import logger | 27 | from .logger import logger |
| 28 | from .registry import add_to_hook_registry | 28 | from .registry import add_to_hook_registry |
| 29 | from .utils import FunctionContext | 29 | from .utils import FunctionContext |
| 30 | +from .import_security import is_allowed_symbol_module | ||
| 30 | 31 | ||
| 31 | MAX_HOOK_FAILURES = 5 | 32 | MAX_HOOK_FAILURES = 5 |
| 32 | 33 | ||
| @@ -87,6 +88,9 @@ def import_object_from_string(import_path: str, module_path: str) -> Any: | |||
| 87 | if not import_path: | 88 | if not import_path: |
| 88 | logger.error("Module import_path is empty") | 89 | logger.error("Module import_path is empty") |
| 89 | return None | 90 | return None |
| 91 | + if not is_allowed_symbol_module(import_path): | ||
| 92 | + logger.error("Module import_path is not allowed: %s", import_path) | ||
| 93 | + return None | ||
| 90 | 94 | ||
| 91 | try: | 95 | try: |
| 92 | module = importlib.import_module(import_path) | 96 | module = importlib.import_module(import_path) |
| @@ -441,13 +445,13 @@ class VLLMHookerBase(ABC): | |||
| 441 | hook_func (Optional[Callable]): hook 处理函数 | 445 | hook_func (Optional[Callable]): hook 处理函数 |
| 442 | """ | 446 | """ |
| 443 | 447 | ||
| 444 | - vllm_version = (None, None) # (min_version, max_version) | ||
| 445 | - applied_hook_func_name = "" | ||
| 446 | - | ||
| 447 | 448 | ||
| 448 | def default_hook_func(ori_func, *args, **kwargs): | 449 | def default_hook_func(ori_func, *args, **kwargs): |
| 449 | return ori_func(*args, **kwargs) | 450 | return ori_func(*args, **kwargs) |
| 450 | 451 | ||
| 452 | + vllm_version = (None, None) # (min_version, max_version) | ||
| 453 | + applied_hook_func_name = "" | ||
| 454 | + | ||
| 451 | def __init__(self): | 455 | def __init__(self): |
| 452 | """初始化 VLLMHookerBase。""" | 456 | """初始化 VLLMHookerBase。""" |
| 453 | self.hooks = [] | 457 | self.hooks = [] |
| @@ -153,6 +153,11 @@ def test_given_invalid_handler_path_when_from_config_then_raises_handler_error() | |||
| 153 | MetricHandler.from_config(config, "module:func") | 153 | MetricHandler.from_config(config, "module:func") |
| 154 | 154 | ||
| 155 | 155 | ||
| 156 | +def test_given_disallowed_handler_module_when_import_then_raises_handler_error(): | ||
| 157 | + with pytest.raises(HandlerError, match="not allowed"): | ||
| 158 | + MetricHandler._import_handler("evil.module:payload") | ||
| 159 | + | ||
| 160 | + | ||
| 156 | def test_given_empty_config_when_from_config_then_uses_default_handler(): | 161 | def test_given_empty_config_when_from_config_then_uses_default_handler(): |
| 157 | config = {} | 162 | config = {} |
| 158 | handler = MetricHandler.from_config(config, "module:func") | 163 | handler = MetricHandler.from_config(config, "module:func") |
| @@ -13,12 +13,13 @@ | |||
| 13 | # MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. | 13 | # MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. |
| 14 | # See the Mulan PSL v2 for more details. | 14 | # See the Mulan PSL v2 for more details. |
| 15 | # ------------------------------------------------------------------------- | 15 | # ------------------------------------------------------------------------- |
| 16 | +# pylint: disable=disallowed-name | ||
| 16 | 17 | ||
| 17 | import types | 18 | import types |
| 19 | +from unittest.mock import patch | ||
| 18 | 20 | ||
| 19 | from ms_service_metric.core.handler import MetricHandler, HandlerType | 21 | from ms_service_metric.core.handler import MetricHandler, HandlerType |
| 20 | from ms_service_metric.core.symbol import Symbol | 22 | from ms_service_metric.core.symbol import Symbol |
| 21 | -from ms_service_metric.metrics.metrics_manager import MetricType | ||
| 22 | 23 | ||
| 23 | 24 | ||
| 24 | class DummyWatcher: | 25 | class DummyWatcher: |
| @@ -67,11 +68,11 @@ def test_given_wrap_handler_when_module_loaded_then_wrapped_result_unhook_restor | |||
| 67 | 68 | ||
| 68 | # Simulate module loaded event -> should apply hook. | 69 | # Simulate module loaded event -> should apply hook. |
| 69 | event = types.SimpleNamespace(module_name=module_path) | 70 | event = types.SimpleNamespace(module_name=module_path) |
| 70 | - symbol._on_module_loaded(event) | 71 | + with patch("ms_service_metric.core.symbol.is_allowed_symbol_module", return_value=True): |
| 72 | + symbol._on_module_loaded(event) | ||
| 71 | 73 | ||
| 72 | assert DummyTarget().foo(5) == 11 # (5*2)+1 | 74 | assert DummyTarget().foo(5) == 11 # (5*2)+1 |
| 73 | 75 | ||
| 74 | # Unhook should restore original behavior. | 76 | # Unhook should restore original behavior. |
| 75 | symbol.unhook() | 77 | symbol.unhook() |
| 76 | assert DummyTarget().foo(5) == 10 | 78 | assert DummyTarget().foo(5) == 10 |
| 77 | - | ||
| @@ -1,5 +1,4 @@ | |||
| 1 | # ------------------------------------------------------------------------- | 1 | # ------------------------------------------------------------------------- |
| 2 | -# pylint: disable=no-name-in-module,redefined-outer-name | ||
| 3 | # This file is part of the MindStudio project. | 2 | # This file is part of the MindStudio project. |
| 4 | # Copyright (c) 2025 Huawei Technologies Co.,Ltd. | 3 | # Copyright (c) 2025 Huawei Technologies Co.,Ltd. |
| 5 | # | 4 | # |
| @@ -14,6 +13,7 @@ | |||
| 14 | # MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. | 13 | # MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. |
| 15 | # See the Mulan PSL v2 for more details. | 14 | # See the Mulan PSL v2 for more details. |
| 16 | # ------------------------------------------------------------------------- | 15 | # ------------------------------------------------------------------------- |
| 16 | +# pylint: disable=redefined-outer-name | ||
| 17 | 17 | ||
| 18 | """Lightweight Symbol tests with mocked watcher / manager.""" | 18 | """Lightweight Symbol tests with mocked watcher / manager.""" |
| 19 | 19 | ||
| @@ -80,6 +80,15 @@ def test_given_target_module_not_loaded_when_hook_then_no_apply(no_watch_symbol) | |||
| 80 | sym.hook() | 80 | sym.hook() |
| 81 | 81 | ||
| 82 | 82 | ||
| 83 | +def test_given_disallowed_symbol_module_when_import_target_then_returns_none(no_watch_symbol): | ||
| 84 | + sym, _, _ = no_watch_symbol | ||
| 85 | + sym._module_path = "evil.module" | ||
| 86 | + sym._attr_path = "Payload.run" | ||
| 87 | + with patch("importlib.import_module") as mock_import: | ||
| 88 | + assert sym._import_target() is None | ||
| 89 | + mock_import.assert_not_called() | ||
| 90 | + | ||
| 91 | + | ||
| 83 | def test_given_hook_not_applied_when_unhook_then_noop(no_watch_symbol): | 92 | def test_given_hook_not_applied_when_unhook_then_noop(no_watch_symbol): |
| 84 | sym, _, _ = no_watch_symbol | 93 | sym, _, _ = no_watch_symbol |
| 85 | sym.unhook() | 94 | sym.unhook() |
| @@ -1,5 +1,4 @@ | |||
| 1 | # ------------------------------------------------------------------------- | 1 | # ------------------------------------------------------------------------- |
| 2 | -# pylint: disable=attribute-defined-outside-init,no-member,unspecified-encoding | ||
| 3 | # This file is part of the MindStudio project. | 2 | # This file is part of the MindStudio project. |
| 4 | # Copyright (c) 2025 Huawei Technologies Co.,Ltd. | 3 | # Copyright (c) 2025 Huawei Technologies Co.,Ltd. |
| 5 | # | 4 | # |
| @@ -14,6 +13,7 @@ | |||
| 14 | # MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. | 13 | # MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. |
| 15 | # See the Mulan PSL v2 for more details. | 14 | # See the Mulan PSL v2 for more details. |
| 16 | # ------------------------------------------------------------------------- | 15 | # ------------------------------------------------------------------------- |
| 16 | +# pylint: disable=no-member,unspecified-encoding,attribute-defined-outside-init | ||
| 17 | 17 | ||
| 18 | import importlib | 18 | import importlib |
| 19 | import os | 19 | import os |
| @@ -127,15 +127,17 @@ class TestResolveHandlerFunc: | |||
| 127 | mock_handler = MagicMock() | 127 | mock_handler = MagicMock() |
| 128 | mock_module.my_handler = mock_handler | 128 | mock_module.my_handler = mock_handler |
| 129 | with patch("importlib.import_module", return_value=mock_module): | 129 | with patch("importlib.import_module", return_value=mock_module): |
| 130 | - result = _resolve_handler_func({"handler": "my_module.handlers:my_handler"}, "my_method") | 130 | + result = _resolve_handler_func({"handler": "ms_service_profiler.handlers:my_handler"}, "my_method") |
| 131 | assert result == mock_handler | 131 | assert result == mock_handler |
| 132 | - importlib.import_module.assert_called_once_with("my_module.handlers") | 132 | + importlib.import_module.assert_called_once_with("ms_service_profiler.handlers") |
| 133 | 133 | ||
| 134 | def test_resolve_handler_func_import_fails_returns_default(self): | 134 | def test_resolve_handler_func_import_fails_returns_default(self): |
| 135 | - with patch("importlib.import_module", side_effect=ImportError("No module")): | 135 | + # make_default_time_hook 必须先 patch(外层),否则 patch 字符串目标解析会调用 |
| 136 | - with patch("ms_service_profiler.patcher.core.config_loader.make_default_time_hook") as m: | 136 | + # 已被 mock 的 importlib.import_module 而失败 |
| 137 | - default_h = MagicMock() | 137 | + with patch("ms_service_profiler.patcher.core.config_loader.make_default_time_hook") as m: |
| 138 | - m.return_value = default_h | 138 | + default_h = MagicMock() |
| 139 | + m.return_value = default_h | ||
| 140 | + with patch("importlib.import_module", side_effect=ImportError("No module")): | ||
| 139 | result = _resolve_handler_func( | 141 | result = _resolve_handler_func( |
| 140 | {"handler": "x:y", "domain": "TestDomain", "name": "custom_name"}, "my_method" | 142 | {"handler": "x:y", "domain": "TestDomain", "name": "custom_name"}, "my_method" |
| 141 | ) | 143 | ) |
| @@ -145,16 +147,26 @@ class TestResolveHandlerFunc: | |||
| 145 | def test_resolve_handler_func_not_callable_returns_default(self): | 147 | def test_resolve_handler_func_not_callable_returns_default(self): |
| 146 | mock_module = MagicMock() | 148 | mock_module = MagicMock() |
| 147 | mock_module.my_handler = "not_a_function" | 149 | mock_module.my_handler = "not_a_function" |
| 148 | - with patch("importlib.import_module", return_value=mock_module): | 150 | + with patch("ms_service_profiler.patcher.core.config_loader.make_default_time_hook") as m: |
| 149 | - with patch("ms_service_profiler.patcher.core.config_loader.make_default_time_hook") as m: | 151 | + default_h = MagicMock() |
| 150 | - default_h = MagicMock() | 152 | + m.return_value = default_h |
| 151 | - m.return_value = default_h | 153 | + with patch("importlib.import_module", return_value=mock_module): |
| 152 | result = _resolve_handler_func( | 154 | result = _resolve_handler_func( |
| 153 | - {"handler": "my_module.handlers:my_handler", "domain": "TestDomain"}, "my_method" | 155 | + {"handler": "ms_service_profiler.handlers:my_handler", "domain": "TestDomain"}, "my_method" |
| 154 | ) | 156 | ) |
| 155 | assert result == default_h | 157 | assert result == default_h |
| 156 | m.assert_called_once_with(domain="TestDomain", name="my_method", attributes=None) | 158 | m.assert_called_once_with(domain="TestDomain", name="my_method", attributes=None) |
| 157 | 159 | ||
| 160 | + def test_resolve_handler_func_disallowed_module_returns_default(self): | ||
| 161 | + with patch("ms_service_profiler.patcher.core.config_loader.make_default_time_hook") as m: | ||
| 162 | + default_h = MagicMock() | ||
| 163 | + m.return_value = default_h | ||
| 164 | + with patch("importlib.import_module") as mock_import: | ||
| 165 | + result = _resolve_handler_func({"handler": "evil.module:payload", "domain": "TestDomain"}, "my_method") | ||
| 166 | + assert result == default_h | ||
| 167 | + mock_import.assert_not_called() | ||
| 168 | + m.assert_called_once_with(domain="TestDomain", name="my_method", attributes=None) | ||
| 169 | + | ||
| 158 | def test_resolve_handler_func_no_handler_returns_default(self): | 170 | def test_resolve_handler_func_no_handler_returns_default(self): |
| 159 | with patch("ms_service_profiler.patcher.core.config_loader.make_default_time_hook") as m: | 171 | with patch("ms_service_profiler.patcher.core.config_loader.make_default_time_hook") as m: |
| 160 | default_h = MagicMock() | 172 | default_h = MagicMock() |
| @@ -326,11 +338,24 @@ class TestResolveMetricsHandlerFunc: | |||
| 326 | mock_mod = MagicMock() | 338 | mock_mod = MagicMock() |
| 327 | mock_mod.my_func = imported_func | 339 | mock_mod.my_func = imported_func |
| 328 | mock_import.return_value = mock_mod | 340 | mock_import.return_value = mock_mod |
| 329 | - symbol_info = {"handler": "some.module:my_func", "metrics": []} | 341 | + symbol_info = {"handler": "ms_service_metric.handlers:my_func", "metrics": []} |
| 330 | result = _resolve_metrics_handler_func(symbol_info, "my_method") | 342 | result = _resolve_metrics_handler_func(symbol_info, "my_method") |
| 331 | - mock_import.assert_called_once_with("some.module") | 343 | + mock_import.assert_called_once_with("ms_service_metric.handlers") |
| 332 | assert result is imported_func | 344 | assert result is imported_func |
| 333 | 345 | ||
| 346 | + def test_resolve_metrics_handler_func_disallowed_module_wraps_noop(self): | ||
| 347 | + # wrap_handler_with_metrics 必须先 patch(外层),避免其字符串目标解析被 | ||
| 348 | + # 已 mock 的 importlib.import_module 干扰 | ||
| 349 | + with patch("ms_service_profiler.patcher.core.config_loader.wrap_handler_with_metrics") as mock_wrap: | ||
| 350 | + wrapped = MagicMock() | ||
| 351 | + mock_wrap.return_value = wrapped | ||
| 352 | + with patch("ms_service_profiler.patcher.core.config_loader.importlib.import_module") as mock_import: | ||
| 353 | + result = _resolve_metrics_handler_func({"handler": "evil.module:payload", "metrics": []}, "my_method") | ||
| 354 | + mock_import.assert_not_called() | ||
| 355 | + mock_wrap.assert_called_once() | ||
| 356 | + assert mock_wrap.call_args[0][0].__name__ == "_metrics_noop_handler" | ||
| 357 | + assert result == wrapped | ||
| 358 | + | ||
| 334 | def test_resolve_metrics_handler_func_no_handler_wraps_noop(self): | 359 | def test_resolve_metrics_handler_func_no_handler_wraps_noop(self): |
| 335 | """无 handler 时用 wrap_handler_with_metrics 封装透传函数""" | 360 | """无 handler 时用 wrap_handler_with_metrics 封装透传函数""" |
| 336 | with patch("ms_service_profiler.patcher.core.config_loader.wrap_handler_with_metrics") as mock_wrap: | 361 | with patch("ms_service_profiler.patcher.core.config_loader.wrap_handler_with_metrics") as mock_wrap: |
| @@ -1,5 +1,4 @@ | |||
| 1 | # ------------------------------------------------------------------------- | 1 | # ------------------------------------------------------------------------- |
| 2 | -# pylint: disable=comparison-with-callable,redefined-outer-name,use-implicit-booleaness-not-comparison | ||
| 3 | # This file is part of the MindStudio project. | 2 | # This file is part of the MindStudio project. |
| 4 | # Copyright (c) 2025 Huawei Technologies Co.,Ltd. | 3 | # Copyright (c) 2025 Huawei Technologies Co.,Ltd. |
| 5 | # | 4 | # |
| @@ -14,6 +13,8 @@ | |||
| 14 | # MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. | 13 | # MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. |
| 15 | # See the Mulan PSL v2 for more details. | 14 | # See the Mulan PSL v2 for more details. |
| 16 | # ------------------------------------------------------------------------- | 15 | # ------------------------------------------------------------------------- |
| 16 | +# pylint: disable=too-many-lines,redefined-outer-name | ||
| 17 | +# pylint: disable=comparison-with-callable,use-implicit-booleaness-not-comparison | ||
| 17 | 18 | ||
| 18 | from unittest.mock import Mock, patch, call | 19 | from unittest.mock import Mock, patch, call |
| 19 | from unittest.mock import ANY | 20 | from unittest.mock import ANY |
| @@ -187,7 +188,7 @@ class TestRegisterDynamicHook: | |||
| 187 | mock_hooker_instance = Mock() | 188 | mock_hooker_instance = Mock() |
| 188 | mock_dynamic_hooker.return_value = mock_hooker_instance | 189 | mock_dynamic_hooker.return_value = mock_hooker_instance |
| 189 | 190 | ||
| 190 | - register_dynamic_hook(hook_list=sample_hook_list, hook_func=mock_hook_func) | 191 | + result = register_dynamic_hook(hook_list=sample_hook_list, hook_func=mock_hook_func) |
| 191 | 192 | ||
| 192 | mock_dynamic_hooker.assert_called_once_with( | 193 | mock_dynamic_hooker.assert_called_once_with( |
| 193 | hook_list=sample_hook_list, | 194 | hook_list=sample_hook_list, |
| @@ -197,6 +198,7 @@ class TestRegisterDynamicHook: | |||
| 197 | caller_filter=None, | 198 | caller_filter=None, |
| 198 | need_locals=False, | 199 | need_locals=False, |
| 199 | ) | 200 | ) |
| 201 | + assert result == mock_hooker_instance | ||
| 200 | 202 | ||
| 201 | 203 | ||
| 202 | class TestMakeDefaultTimeHook: | 204 | class TestMakeDefaultTimeHook: |
| @@ -205,14 +207,8 @@ class TestMakeDefaultTimeHook: | |||
| 205 | 207 | ||
| 206 | def test_make_default_time_hook_no_profiler(): | 208 | def test_make_default_time_hook_no_profiler(): |
| 207 | """测试没有 ms_service_profiler 的情况""" | 209 | """测试没有 ms_service_profiler 的情况""" |
| 208 | - with patch.dict('sys.modules', {'ms_service_profiler': None}): | 210 | + # 直接 patch 模块级 Profiler 为 None,避免 reload 模块污染全局状态 |
| 209 | - # 重新导入以应用模拟 | 211 | + with patch('ms_service_profiler.patcher.core.dynamic_hook.Profiler', None): |
| 210 | - import importlib | ||
| 211 | - import sys | ||
| 212 | - | ||
| 213 | - if 'ms_service_profiler.patcher.core.dynamic_hook' in sys.modules: | ||
| 214 | - importlib.reload(sys.modules['ms_service_profiler.patcher.core.dynamic_hook']) | ||
| 215 | - | ||
| 216 | result_func = make_default_time_hook("test_domain", "test_name") | 212 | result_func = make_default_time_hook("test_domain", "test_name") |
| 217 | 213 | ||
| 218 | # 测试返回的函数 | 214 | # 测试返回的函数 |
| @@ -263,10 +259,11 @@ class TestMakeDefaultTimeHook: | |||
| 263 | with patch('ms_service_profiler.patcher.core.dynamic_hook._safe_eval_expr') as mock_safe_eval: | 259 | with patch('ms_service_profiler.patcher.core.dynamic_hook._safe_eval_expr') as mock_safe_eval: |
| 264 | mock_safe_eval.side_effect = [6, 3, "test_model"] # 模拟三个属性的返回值 | 260 | mock_safe_eval.side_effect = [6, 3, "test_model"] # 模拟三个属性的返回值 |
| 265 | 261 | ||
| 266 | - result_func(mock_original, *mock_args, **mock_kwargs) | 262 | + result = result_func(mock_original, *mock_args, **mock_kwargs) |
| 267 | 263 | ||
| 268 | # 验证属性设置 | 264 | # 验证属性设置 |
| 269 | assert mock_profiler_instance.attr.call_count == 3 | 265 | assert mock_profiler_instance.attr.call_count == 3 |
| 266 | + assert result == "result" | ||
| 270 | mock_profiler_instance.attr.assert_has_calls( | 267 | mock_profiler_instance.attr.assert_has_calls( |
| 271 | [call("input_length", 6), call("output_length", 3), call("model_name", "test_model")] | 268 | [call("input_length", 6), call("output_length", 3), call("model_name", "test_model")] |
| 272 | ) | 269 | ) |
| @@ -287,10 +284,11 @@ class TestMakeDefaultTimeHook: | |||
| 287 | with patch('ms_service_profiler.patcher.core.dynamic_hook._safe_eval_expr') as mock_safe_eval: | 284 | with patch('ms_service_profiler.patcher.core.dynamic_hook._safe_eval_expr') as mock_safe_eval: |
| 288 | mock_safe_eval.return_value = None # 所有表达式执行失败 | 285 | mock_safe_eval.return_value = None # 所有表达式执行失败 |
| 289 | 286 | ||
| 290 | - result_func(mock_original, *mock_args, **mock_kwargs) | 287 | + result = result_func(mock_original, *mock_args, **mock_kwargs) |
| 291 | 288 | ||
| 292 | # 验证没有属性被设置 | 289 | # 验证没有属性被设置 |
| 293 | mock_profiler_instance.attr.assert_not_called() | 290 | mock_profiler_instance.attr.assert_not_called() |
| 291 | + assert result == "result" | ||
| 294 | 292 | ||
| 295 | 293 | ||
| 296 | 294 | ||
| @@ -313,11 +311,12 @@ class TestMakeDefaultTimeHook: | |||
| 313 | with patch('ms_service_profiler.patcher.core.dynamic_hook._safe_eval_expr') as mock_safe_eval: | 311 | with patch('ms_service_profiler.patcher.core.dynamic_hook._safe_eval_expr') as mock_safe_eval: |
| 314 | mock_safe_eval.return_value = 5 | 312 | mock_safe_eval.return_value = 5 |
| 315 | 313 | ||
| 316 | - result_func(mock_original, 1, 2, 3) | 314 | + result = result_func(mock_original, 1, 2, 3) |
| 317 | 315 | ||
| 318 | # 只有第一个有效属性被处理 | 316 | # 只有第一个有效属性被处理 |
| 319 | mock_safe_eval.assert_called_once_with("len(args)", ANY) | 317 | mock_safe_eval.assert_called_once_with("len(args)", ANY) |
| 320 | mock_profiler_instance.attr.assert_called_once_with("valid", 5) | 318 | mock_profiler_instance.attr.assert_called_once_with("valid", 5) |
| 319 | + assert result == "result" | ||
| 321 | 320 | ||
| 322 | 321 | ||
| 323 | class TestHandlerResolver: | 322 | class TestHandlerResolver: |
| @@ -341,9 +340,9 @@ class TestHandlerResolver: | |||
| 341 | mock_import_module.return_value = mock_module | 340 | mock_import_module.return_value = mock_module |
| 342 | mock_module.test_handler = mock_handler | 341 | mock_module.test_handler = mock_handler |
| 343 | 342 | ||
| 344 | - result = HandlerResolver._try_import("some.module:test_handler") | 343 | + result = HandlerResolver._try_import("ms_service_profiler.handlers:test_handler") |
| 345 | 344 | ||
| 346 | - mock_import_module.assert_called_once_with("some.module") | 345 | + mock_import_module.assert_called_once_with("ms_service_profiler.handlers") |
| 347 | assert result == mock_handler | 346 | assert result == mock_handler |
| 348 | 347 | ||
| 349 | 348 | ||
| @@ -352,7 +351,7 @@ class TestHandlerResolver: | |||
| 352 | """测试导入模块失败""" | 351 | """测试导入模块失败""" |
| 353 | mock_import_module.side_effect = ImportError("Module not found") | 352 | mock_import_module.side_effect = ImportError("Module not found") |
| 354 | 353 | ||
| 355 | - result = HandlerResolver._try_import("nonexistent.module:handler") | 354 | + result = HandlerResolver._try_import("ms_service_profiler.nonexistent:handler") |
| 356 | 355 | ||
| 357 | assert result is None | 356 | assert result is None |
| 358 | 357 | ||
| @@ -364,10 +363,19 @@ class TestHandlerResolver: | |||
| 364 | mock_module.test_handler = None # 函数不存在 | 363 | mock_module.test_handler = None # 函数不存在 |
| 365 | mock_import_module.return_value = mock_module | 364 | mock_import_module.return_value = mock_module |
| 366 | 365 | ||
| 367 | - result = HandlerResolver._try_import("some.module:nonexistent_handler") | 366 | + result = HandlerResolver._try_import("ms_service_profiler.handlers:nonexistent_handler") |
| 368 | 367 | ||
| 369 | assert result is None | 368 | assert result is None |
| 370 | 369 | ||
| 370 | + | ||
| 371 | + | ||
| 372 | + def test_try_import_disallowed_module(mock_import_module): | ||
| 373 | + """测试非白名单 handler 模块不触发 import""" | ||
| 374 | + result = HandlerResolver._try_import("evil.module:test_handler") | ||
| 375 | + | ||
| 376 | + assert result is None | ||
| 377 | + mock_import_module.assert_not_called() | ||
| 378 | + | ||
| 371 | 379 | ||
| 372 | 380 | ||
| 373 | def test_resolve_explicit_timer(mock_make_default): | 381 | def test_resolve_explicit_timer(mock_make_default): |
| @@ -589,11 +597,13 @@ class TestInternalFunctions: | |||
| 589 | assert safe_locals['args'] == (mock_self, "arg1_value", "arg2_value") | 597 | assert safe_locals['args'] == (mock_self, "arg1_value", "arg2_value") |
| 590 | assert safe_locals['kwargs'] == {} | 598 | assert safe_locals['kwargs'] == {} |
| 591 | assert safe_locals['return'] == "result_value" | 599 | assert safe_locals['return'] == "result_value" |
| 600 | + assert safe_locals['ret'] == "result_value" | ||
| 592 | 601 | ||
| 593 | # 验证具名参数 | 602 | # 验证具名参数 |
| 594 | assert 'self' in safe_locals | 603 | assert 'self' in safe_locals |
| 595 | - assert 'arg1' in safe_locals | 604 | + assert 'arg1' not in safe_locals |
| 596 | - assert 'arg2' in safe_locals | 605 | + assert 'arg2' not in safe_locals |
| 606 | + assert 'attr' not in safe_locals | ||
| 597 | 607 | ||
| 598 | 608 | ||
| 599 | 609 | ||
| @@ -604,10 +614,16 @@ class TestInternalFunctions: | |||
| 604 | ("__import__('os')", False), # 危险函数 | 614 | ("__import__('os')", False), # 危险函数 |
| 605 | ("eval('1+1')", False), # 危险函数 | 615 | ("eval('1+1')", False), # 危险函数 |
| 606 | ("args[0] + args[1]", False), # 危险操作符 | 616 | ("args[0] + args[1]", False), # 危险操作符 |
| 607 | - ("len(kwargs.get('key', []))", True), # 安全函数调用 | 617 | + ("len(kwargs['key'])", True), # 安全函数调用 |
| 618 | + ("len(kwargs.get('key', []))", False), # 不允许对象方法调用 | ||
| 608 | ("unknown_func()", False), # 未知函数 | 619 | ("unknown_func()", False), # 未知函数 |
| 609 | ("(1 + 2) * 3", False), # 算术运算 | 620 | ("(1 + 2) * 3", False), # 算术运算 |
| 610 | ("args[0] | len", True), # 管道操作(在后续验证) | 621 | ("args[0] | len", True), # 管道操作(在后续验证) |
| 622 | + ("args[0] | attr safe_attr", True), | ||
| 623 | + ("args[0] | attr __class__", False), | ||
| 624 | + ("len(this.shutdown())", False), | ||
| 625 | + ("this.__class__", False), | ||
| 626 | + ("kwargs['x'].dangerous()", False), | ||
| 611 | ], | 627 | ], |
| 612 | ) | 628 | ) |
| 613 | def test_validate_expression_safety(expr, expected): | 629 | def test_validate_expression_safety(expr, expected): |
| @@ -619,73 +635,70 @@ class TestInternalFunctions: | |||
| 619 | assert result == expected | 635 | assert result == expected |
| 620 | 636 | ||
| 621 | 637 | ||
| 622 | - def test_execute_direct_expression(sample_func_call_context): | 638 | + def test_normalize_expr_keeps_string_literals(): |
| 623 | - """测试 _execute_direct_expression 函数""" | 639 | + """测试 _normalize_expr 仅替换伪变量 return,不修改字符串字面量。""" |
| 624 | 640 | ||
| 625 | hook_func = make_default_time_hook("test", "test") | 641 | hook_func = make_default_time_hook("test", "test") |
| 642 | + normalize_expr = hook_func.__globals__['_normalize_expr'] | ||
| 626 | 643 | ||
| 627 | - safe_locals = {'args': (1, 2, 3), 'kwargs': {'key': 'value'}, 'return': "result", 'len': len, 'str': str} | 644 | + assert normalize_expr("return") == "ret" |
| 628 | - | 645 | + assert normalize_expr("'return'") == "'return'" |
| 629 | - # 测试安全表达式 | 646 | + assert normalize_expr('str("return")') == 'str("return")' |
| 630 | - result = hook_func.__globals__['_execute_direct_expression']("len(args)", safe_locals) | 647 | + assert normalize_expr('["return", return]') == '["return", ret]' |
| 631 | - assert result == 3 | ||
| 632 | - | ||
| 633 | - # 测试危险表达式(应该返回 None) | ||
| 634 | - result = hook_func.__globals__['_execute_direct_expression']("import os", safe_locals) | ||
| 635 | - assert result is None | ||
| 636 | - | ||
| 637 | - # 测试无效表达式 | ||
| 638 | - result = hook_func.__globals__['_execute_direct_expression']("invalid_syntax", safe_locals) | ||
| 639 | - assert result is None | ||
| 640 | 648 | ||
| 641 | 649 | ||
| 642 | 650 | ||
| 643 | - "input_val,operation,expected", | 651 | + "expr,expected", |
| 644 | [ | 652 | [ |
| 645 | - ([1, 2, 3], 'len', 3), # len 操作 | 653 | + ("len(args[0])", 3), |
| 646 | - ("hello", 'str', "hello"), # str 操作 | 654 | + ("args[0] | len", 3), |
| 647 | - (Mock(test_attr="value"), 'attr test_attr', "value"), # attr 操作 | 655 | + ("return | len | str", "11"), |
| 648 | - ([1, 2, 3], 'unknown', None), # 未知操作 | 656 | + ("args[1] | attr safe_attr", "ok"), |
| 649 | - (None, 'len', None), # None 输入 | 657 | + ("args[0] | attr __class__", None), |
| 658 | + ("args[0] | unknown", None), | ||
| 650 | ], | 659 | ], |
| 651 | ) | 660 | ) |
| 652 | - def test_apply_pipe_operation(input_val, operation, expected): | 661 | + def test_execute_safe_expression(expr, expected): |
| 653 | - """测试 _apply_pipe_operation 函数""" | 662 | + """测试 _execute_safe_expression 函数""" |
| 654 | - | ||
| 655 | - hook_func = make_default_time_hook("test", "test") | ||
| 656 | - | ||
| 657 | - result = hook_func.__globals__['_apply_pipe_operation'](input_val, operation) | ||
| 658 | - | ||
| 659 | - if expected is None: | ||
| 660 | - assert result is None | ||
| 661 | - else: | ||
| 662 | - assert result == expected | ||
| 663 | - | ||
| 664 | - | ||
| 665 | - def test_execute_pipe_expression(sample_func_call_context): | ||
| 666 | - """测试 _execute_pipe_expression 函数""" | ||
| 667 | 663 | ||
| 668 | hook_func = make_default_time_hook("test", "test") | 664 | hook_func = make_default_time_hook("test", "test") |
| 669 | 665 | ||
| 670 | safe_locals = { | 666 | safe_locals = { |
| 671 | - 'args': ([1, 2, 3],), | 667 | + 'args': ([1, 2, 3], Mock(safe_attr="ok")), |
| 672 | 'kwargs': {'key': 'value'}, | 668 | 'kwargs': {'key': 'value'}, |
| 673 | 'return': "hello world", | 669 | 'return': "hello world", |
| 674 | - 'len': len, | 670 | + 'ret': "hello world", |
| 675 | - 'str': str, | ||
| 676 | } | 671 | } |
| 677 | 672 | ||
| 678 | - # 测试简单表达式 | 673 | + result = hook_func.__globals__['_execute_safe_expression'](expr, safe_locals) |
| 679 | - result = hook_func.__globals__['_execute_pipe_expression']("len(args[0])", safe_locals) | 674 | + assert result == expected |
| 680 | - assert result == 3 | ||
| 681 | 675 | ||
| 682 | - # 测试管道表达式 | 676 | + @staticmethod |
| 683 | - result = hook_func.__globals__['_execute_pipe_expression']("args[0] | len", safe_locals) | 677 | + def test_execute_safe_expression_validates_before_execution(): |
| 684 | - assert result == 3 | 678 | + """测试 _execute_safe_expression 会先走前置安全校验。""" |
| 685 | 679 | ||
| 686 | - # 测试多步管道 | 680 | + hook_func = make_default_time_hook("test", "test") |
| 687 | - result = hook_func.__globals__['_execute_pipe_expression']("return | len | str", safe_locals) | 681 | + safe_locals = {'args': ([1, 2, 3],), 'kwargs': {}, 'return': "hello", 'ret': "hello"} |
| 688 | - assert result == "11" # len("hello world") = 11, then str(11) = "11" | 682 | + |
| 683 | + with patch('ms_service_profiler.patcher.core.dynamic_hook._validate_expression_safety') as mock_validate: | ||
| 684 | + mock_validate.return_value = False | ||
| 685 | + result = hook_func.__globals__['_execute_safe_expression']("len(args[0])", safe_locals) | ||
| 686 | + | ||
| 687 | + mock_validate.assert_called_once_with("len(args[0])") | ||
| 688 | + assert result is None | ||
| 689 | + | ||
| 690 | + | ||
| 691 | + def test_execute_direct_expression_ast(): | ||
| 692 | + """测试 AST 求值链路。""" | ||
| 693 | + | ||
| 694 | + hook_func = make_default_time_hook("test", "test") | ||
| 695 | + safe_locals = {'args': (1, 2, 3), 'kwargs': {'key': 'value'}, 'return': "result", 'ret': "result"} | ||
| 696 | + execute_direct = hook_func.__globals__['_execute_direct_expression_ast'] | ||
| 697 | + | ||
| 698 | + assert execute_direct("len(args)", safe_locals) == 3 | ||
| 699 | + assert execute_direct("str('return')", safe_locals) == "return" | ||
| 700 | + assert execute_direct('["return", return]', safe_locals) == ["return", "result"] | ||
| 701 | + assert execute_direct("len(this.shutdown())", safe_locals) is None | ||
| 689 | 702 | ||
| 690 | 703 | ||
| 691 | def test_safe_eval_expr(sample_func_call_context): | 704 | def test_safe_eval_expr(sample_func_call_context): |
| @@ -696,29 +709,76 @@ class TestInternalFunctions: | |||
| 696 | # 模拟成功的表达式执行 | 709 | # 模拟成功的表达式执行 |
| 697 | with ( | 710 | with ( |
| 698 | patch('ms_service_profiler.patcher.core.dynamic_hook._build_safe_locals') as mock_build_locals, | 711 | patch('ms_service_profiler.patcher.core.dynamic_hook._build_safe_locals') as mock_build_locals, |
| 699 | - patch('ms_service_profiler.patcher.core.dynamic_hook._execute_pipe_expression') as mock_execute, | 712 | + patch('ms_service_profiler.patcher.core.dynamic_hook._execute_safe_expression') as mock_execute, |
| 700 | ): | 713 | ): |
| 701 | - mock_build_locals.return_value = {'args': (1, 2, 3), 'len': len} | 714 | + mock_build_locals.return_value = {'args': (1, 2, 3)} |
| 702 | mock_execute.return_value = 3 | 715 | mock_execute.return_value = 3 |
| 703 | 716 | ||
| 704 | result = hook_func.__globals__['_safe_eval_expr']("len(args)", sample_func_call_context) | 717 | result = hook_func.__globals__['_safe_eval_expr']("len(args)", sample_func_call_context) |
| 705 | 718 | ||
| 706 | mock_build_locals.assert_called_once_with(sample_func_call_context) | 719 | mock_build_locals.assert_called_once_with(sample_func_call_context) |
| 707 | - mock_execute.assert_called_once_with("len(args)", {'args': (1, 2, 3), 'len': len}) | 720 | + mock_execute.assert_called_once_with("len(args)", {'args': (1, 2, 3)}) |
| 708 | assert result == 3 | 721 | assert result == 3 |
| 709 | 722 | ||
| 710 | # 模拟表达式执行失败 | 723 | # 模拟表达式执行失败 |
| 711 | with ( | 724 | with ( |
| 712 | patch('ms_service_profiler.patcher.core.dynamic_hook._build_safe_locals') as mock_build_locals, | 725 | patch('ms_service_profiler.patcher.core.dynamic_hook._build_safe_locals') as mock_build_locals, |
| 713 | - patch('ms_service_profiler.patcher.core.dynamic_hook._execute_pipe_expression') as mock_execute, | 726 | + patch('ms_service_profiler.patcher.core.dynamic_hook._execute_safe_expression') as mock_execute, |
| 714 | ): | 727 | ): |
| 715 | - mock_build_locals.return_value = {'args': (1, 2, 3), 'len': len} | 728 | + mock_build_locals.return_value = {'args': (1, 2, 3)} |
| 716 | mock_execute.side_effect = Exception("Test error") | 729 | mock_execute.side_effect = Exception("Test error") |
| 717 | 730 | ||
| 718 | result = hook_func.__globals__['_safe_eval_expr']("len(args)", sample_func_call_context) | 731 | result = hook_func.__globals__['_safe_eval_expr']("len(args)", sample_func_call_context) |
| 719 | 732 | ||
| 720 | assert result is None | 733 | assert result is None |
| 721 | 734 | ||
| 735 | + | ||
| 736 | + | ||
| 737 | + "expr", | ||
| 738 | + [ | ||
| 739 | + "len(this.shutdown())", | ||
| 740 | + "this.__class__", | ||
| 741 | + "kwargs['x'].dangerous()", | ||
| 742 | + "args[0] | attr __class__", | ||
| 743 | + ], | ||
| 744 | + ) | ||
| 745 | + def test_safe_eval_expr_rejects_dangerous_expressions(expr): | ||
| 746 | + hook_func = make_default_time_hook("test", "test") | ||
| 747 | + | ||
| 748 | + class Dangerous: | ||
| 749 | + safe_value = "visible" | ||
| 750 | + | ||
| 751 | + def shutdown(self): | ||
| 752 | + raise AssertionError("shutdown should not be called") | ||
| 753 | + | ||
| 754 | + def dangerous(self): | ||
| 755 | + raise AssertionError("dangerous should not be called") | ||
| 756 | + | ||
| 757 | + ctx = FuncCallContext( | ||
| 758 | + func_obj=lambda *args, **kwargs: None, | ||
| 759 | + this_obj=Dangerous(), | ||
| 760 | + args=([1, 2, 3],), | ||
| 761 | + kwargs={"x": Dangerous()}, | ||
| 762 | + ret_val="result", | ||
| 763 | + ) | ||
| 764 | + | ||
| 765 | + assert hook_func.__globals__['_safe_eval_expr'](expr, ctx) is None | ||
| 766 | + | ||
| 767 | + | ||
| 768 | + def test_safe_eval_expr_keeps_simple_supported_expressions(): | ||
| 769 | + hook_func = make_default_time_hook("test", "test") | ||
| 770 | + ctx = FuncCallContext( | ||
| 771 | + func_obj=lambda *args, **kwargs: None, | ||
| 772 | + this_obj=None, | ||
| 773 | + args=([1, 2, 3],), | ||
| 774 | + kwargs={"input_ids": [1, 2, 3, 4]}, | ||
| 775 | + ret_val="ok", | ||
| 776 | + ) | ||
| 777 | + | ||
| 778 | + assert hook_func.__globals__['_safe_eval_expr']("len(args[0])", ctx) == 3 | ||
| 779 | + assert hook_func.__globals__['_safe_eval_expr']("len(kwargs['input_ids'])", ctx) == 4 | ||
| 780 | + assert hook_func.__globals__['_safe_eval_expr']("str(return)", ctx) == "ok" | ||
| 781 | + | ||
| 722 | 782 | ||
| 723 | 783 | ||
| 724 | def reset_global_manager(): | 784 | def reset_global_manager(): |
| @@ -897,8 +957,8 @@ class TestConfigHooker: | |||
| 897 | """正例:ConfigHooker 初始化应正确设置属性""" | 957 | """正例:ConfigHooker 初始化应正确设置属性""" |
| 898 | hook_list = [("mod1", "func1"), ("mod2", "func2")] | 958 | hook_list = [("mod1", "func1"), ("mod2", "func2")] |
| 899 | 959 | ||
| 900 | - def hook_func(x): | 960 | + def hook_func(value): |
| 901 | - return x | 961 | + return value |
| 902 | 962 | ||
| 903 | symbol_path = "test.symbol" | 963 | symbol_path = "test.symbol" |
| 904 | min_v = "1.0" | 964 | min_v = "1.0" |
| @@ -986,13 +1046,21 @@ class TestConfigHooker: | |||
| 986 | 1046 | ||
| 987 | def test_given_manager_exists_when_init_then_adds_handler(self, mock_import_object): | 1047 | def test_given_manager_exists_when_init_then_adds_handler(self, mock_import_object): |
| 988 | """正例:管理器已存在时 init 直接添加自身""" | 1048 | """正例:管理器已存在时 init 直接添加自身""" |
| 1049 | + | ||
| 989 | # 预先创建管理器 | 1050 | # 预先创建管理器 |
| 990 | - hooker = ConfigHooker([], lambda x: x, "sym", None, None, None, False) | 1051 | + def identity(value): |
| 1052 | + return value | ||
| 1053 | + | ||
| 1054 | + hooker = ConfigHooker([], identity, "sym", None, None, None, False) | ||
| 991 | hooker.init() | 1055 | hooker.init() |
| 992 | 1056 | ||
| 993 | def test_given_handler_exists_when_recover_then_removes_from_manager(self): | 1057 | def test_given_handler_exists_when_recover_then_removes_from_manager(self): |
| 994 | """正例:recover 从管理器中移除自身""" | 1058 | """正例:recover 从管理器中移除自身""" |
| 995 | manager = MagicMock() | 1059 | manager = MagicMock() |
| 996 | manager.recover_handler.return_value = 1 | 1060 | manager.recover_handler.return_value = 1 |
| 997 | - hooker = ConfigHooker([], lambda x: x, "sym", None, None, None, False) | 1061 | + |
| 1062 | + def identity(value): | ||
| 1063 | + return value | ||
| 1064 | + | ||
| 1065 | + hooker = ConfigHooker([], identity, "sym", None, None, None, False) | ||
| 998 | hooker.recover() | 1066 | hooker.recover() |
| @@ -1,5 +1,4 @@ | |||
| 1 | # ------------------------------------------------------------------------- | 1 | # ------------------------------------------------------------------------- |
| 2 | -# pylint: disable=redefined-outer-name | ||
| 3 | # This file is part of the MindStudio project. | 2 | # This file is part of the MindStudio project. |
| 4 | # Copyright (c) 2025 Huawei Technologies Co.,Ltd. | 3 | # Copyright (c) 2025 Huawei Technologies Co.,Ltd. |
| 5 | # | 4 | # |
| @@ -14,9 +13,11 @@ | |||
| 14 | # MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. | 13 | # MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. |
| 15 | # See the Mulan PSL v2 for more details. | 14 | # See the Mulan PSL v2 for more details. |
| 16 | # ------------------------------------------------------------------------- | 15 | # ------------------------------------------------------------------------- |
| 16 | +# pylint: disable=no-member,redefined-outer-name | ||
| 17 | 17 | ||
| 18 | -import importlib | 18 | +import sys |
| 19 | import asyncio | 19 | import asyncio |
| 20 | +import types | ||
| 20 | from unittest.mock import patch, MagicMock | 21 | from unittest.mock import patch, MagicMock |
| 21 | import pytest | 22 | import pytest |
| 22 | 23 | ||
| @@ -38,33 +39,81 @@ def cleanup_hook_registry(): | |||
| 38 | clear_hook_registry() | 39 | clear_hook_registry() |
| 39 | 40 | ||
| 40 | 41 | ||
| 42 | +def _install_fake_vllm_module(module_name, module): | ||
| 43 | + old_parent = sys.modules.get("vllm") | ||
| 44 | + if old_parent is None: | ||
| 45 | + parent = types.ModuleType("vllm") | ||
| 46 | + parent.__path__ = [] | ||
| 47 | + sys.modules["vllm"] = parent | ||
| 48 | + sys.modules[module_name] = module | ||
| 49 | + return old_parent | ||
| 50 | + | ||
| 51 | + | ||
| 52 | +def _cleanup_fake_vllm_module(module_name, old_parent): | ||
| 53 | + sys.modules.pop(module_name, None) | ||
| 54 | + if old_parent is None: | ||
| 55 | + sys.modules.pop("vllm", None) | ||
| 56 | + else: | ||
| 57 | + sys.modules["vllm"] = old_parent | ||
| 58 | + | ||
| 59 | + | ||
| 41 | # Test cases for import_object_from_string | 60 | # Test cases for import_object_from_string |
| 42 | def test_import_object_from_string_given_valid_path_when_importing_module_then_returns_object(): | 61 | def test_import_object_from_string_given_valid_path_when_importing_module_then_returns_object(): |
| 43 | """Test importing a valid module-level function""" | 62 | """Test importing a valid module-level function""" |
| 44 | - result = import_object_from_string("os", "path") | 63 | + module = types.ModuleType("vllm.fake_module") |
| 45 | - assert result == importlib.import_module("os").path | 64 | + module.target = object() |
| 65 | + old_parent = _install_fake_vllm_module("vllm.fake_module", module) | ||
| 66 | + try: | ||
| 67 | + result = import_object_from_string("vllm.fake_module", "target") | ||
| 68 | + finally: | ||
| 69 | + _cleanup_fake_vllm_module("vllm.fake_module", old_parent) | ||
| 70 | + | ||
| 71 | + assert result == module.target | ||
| 46 | 72 | ||
| 47 | 73 | ||
| 48 | def test_import_object_from_string_given_nested_attribute_when_importing_then_returns_object(): | 74 | def test_import_object_from_string_given_nested_attribute_when_importing_then_returns_object(): |
| 49 | """Test importing nested attributes""" | 75 | """Test importing nested attributes""" |
| 50 | - result = import_object_from_string("collections", "defaultdict.__class__") | 76 | + module = types.ModuleType("vllm.fake_nested") |
| 51 | - from collections import defaultdict | ||
| 52 | 77 | ||
| 53 | - assert result == defaultdict.__class__ | 78 | + class Target: |
| 79 | + value = "nested" | ||
| 80 | + | ||
| 81 | + module.Target = Target | ||
| 82 | + old_parent = _install_fake_vllm_module("vllm.fake_nested", module) | ||
| 83 | + try: | ||
| 84 | + result = import_object_from_string("vllm.fake_nested", "Target.value") | ||
| 85 | + finally: | ||
| 86 | + _cleanup_fake_vllm_module("vllm.fake_nested", old_parent) | ||
| 87 | + | ||
| 88 | + assert result == "nested" | ||
| 54 | 89 | ||
| 55 | 90 | ||
| 56 | def test_import_object_from_string_given_invalid_module_when_importing_then_returns_none(): | 91 | def test_import_object_from_string_given_invalid_module_when_importing_then_returns_none(): |
| 57 | """Test handling of non-existent module""" | 92 | """Test handling of non-existent module""" |
| 58 | - result = import_object_from_string("nonexistent_module", "anything") | 93 | + result = import_object_from_string("vllm.nonexistent_module", "anything") |
| 59 | assert result is None | 94 | assert result is None |
| 60 | 95 | ||
| 61 | 96 | ||
| 62 | def test_import_object_from_string_given_invalid_attribute_when_importing_then_returns_none(): | 97 | def test_import_object_from_string_given_invalid_attribute_when_importing_then_returns_none(): |
| 63 | """Test handling of non-existent attribute""" | 98 | """Test handling of non-existent attribute""" |
| 64 | - result = import_object_from_string("os", "nonexistent_attr") | 99 | + module = types.ModuleType("vllm.fake_missing_attr") |
| 100 | + old_parent = _install_fake_vllm_module("vllm.fake_missing_attr", module) | ||
| 101 | + try: | ||
| 102 | + result = import_object_from_string("vllm.fake_missing_attr", "nonexistent_attr") | ||
| 103 | + finally: | ||
| 104 | + _cleanup_fake_vllm_module("vllm.fake_missing_attr", old_parent) | ||
| 65 | assert result is None | 105 | assert result is None |
| 66 | 106 | ||
| 67 | 107 | ||
| 108 | +def test_import_object_from_string_given_disallowed_module_when_importing_then_returns_none(): | ||
| 109 | + """Test that non allow-listed symbol modules are rejected before import""" | ||
| 110 | + with patch("importlib.import_module") as mock_import: | ||
| 111 | + result = import_object_from_string("evil.module", "Payload.run") | ||
| 112 | + | ||
| 113 | + assert result is None | ||
| 114 | + mock_import.assert_not_called() | ||
| 115 | + | ||
| 116 | + | ||
| 68 | def test_import_object_from_string_given_empty_path_when_importing_then_returns_none(): | 117 | def test_import_object_from_string_given_empty_path_when_importing_then_returns_none(): |
| 69 | """Test handling of empty path""" | 118 | """Test handling of empty path""" |
| 70 | result = import_object_from_string("", "") | 119 | result = import_object_from_string("", "") |
| @@ -512,7 +561,10 @@ class TestHookFuncNotNeedLocals: | |||
| 512 | def __exit__(self, *args): | 561 | def __exit__(self, *args): |
| 513 | pass | 562 | pass |
| 514 | 563 | ||
| 515 | - return lambda ctx: FailingHook() | 564 | + def failing_hook(ctx): |
| 565 | + return FailingHook() | ||
| 566 | + | ||
| 567 | + return failing_hook | ||
| 516 | 568 | ||
| 517 | def create_normal_hook(): | 569 | def create_normal_hook(): |
| 518 | mock_enter = MagicMock() | 570 | mock_enter = MagicMock() |
| @@ -525,7 +577,10 @@ class TestHookFuncNotNeedLocals: | |||
| 525 | def __exit__(self, *args): | 577 | def __exit__(self, *args): |
| 526 | pass | 578 | pass |
| 527 | 579 | ||
| 528 | - return lambda ctx: NormalHook() | 580 | + def normal_hook(ctx): |
| 581 | + return NormalHook() | ||
| 582 | + | ||
| 583 | + return normal_hook | ||
| 529 | 584 | ||
| 530 | context_hook_funcs = [create_failing_hook(), create_normal_hook()] | 585 | context_hook_funcs = [create_failing_hook(), create_normal_hook()] |
| 531 | 586 | ||
🟡 Medium Priority
变更后的
_validate_expression_safety(第542行)和它内部调用的_validate_direct_expression(第532行)在整个生产代码路径中没有任何调用点。旧的_execute_direct_expression函数(已被删除)原来会调用_validate_expression_safety做执行前的安全校验,但新代码路径_safe_eval_expr→_execute_safe_expression→_execute_direct_expression_ast仅在_execute_direct_expression_ast内部通过evaluator.validate(tree)做结构校验,完全绕过了这两个函数。这使得这两个安全校验函数成为死代码——它们仅在 UT 中通过
__globals__访问进行测试,但在生产环境中永远不会被执行。如果未来有人在新的调用路径中忘记做 AST 结构校验,可能会被误导以为已有安全网关。建议:在
_execute_safe_expression开头增加_validate_expression_safety(expr_str)作为前置安全过滤,或者删除这两个无调用者的函数。