review: fix critical/medium bugs in 4 plugins (round 2)

group_horse_racing:
- settle_race: rewrite with 7 bug fixes (race condition, draw double-credit, empty participants, etc.)
- models.py: reorder fields for correct defaults, add indexes
- message_service: add logger import

danding_points:
- api.py: add finally blocks to 3 methods (add_points, get_history, get_leaderboard)
- database.py: add finally block to get_user_balance

chatai:
- __init__.py: deprecated API→asyncio.to_thread, deduplicate logging, taskkill filter for safety
- screenshot.py: XSS protection with bleach on HTML content
- requirements.txt: add bleach dependency

danding_qqpush:
- api.py L13: fix self-referencing _renderer NameError crash
- api.py: lazy singleton pattern via _get_renderer() instead of per-request ImageRenderer
- __init__.py: mask Token in log output (security)

All 34 tests pass.
This commit is contained in:
2026-05-10 00:30:22 +08:00
parent f61465a95b
commit c62ac37611
11 changed files with 183 additions and 148 deletions

View File

@@ -47,9 +47,9 @@ def _force_kill_chrome():
"""强制终止残留的 headless Chrome 进程(仅 pyppeteer 创建的)""" """强制终止残留的 headless Chrome 进程(仅 pyppeteer 创建的)"""
try: try:
if sys.platform == "win32": if sys.platform == "win32":
# 只杀带 --headless 参数的 chrome避免误杀用户浏览器 # 只杀带 --remote-debugging-port 参数的 chrome避免误杀用户浏览器
subprocess.run( subprocess.run(
["taskkill", "/F", "/IM", "chrome.exe", "/FI", "MODULES eq *pyppeteer*"], ["taskkill", "/F", "/FI", "IMAGENAME eq chrome.exe", "/FI", "MODULES eq pyppeteer*"],
stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL,
) )
else: else:
@@ -124,16 +124,14 @@ def _get_ai_client() -> OpenAI:
async def call_ai_api(message: str) -> str: async def call_ai_api(message: str) -> str:
"""调用 AI 接口""" """调用 AI 接口"""
client = _get_ai_client() client = _get_ai_client()
response = await asyncio.get_event_loop().run_in_executor( response = await asyncio.to_thread(
None, client.chat.completions.create,
lambda: client.chat.completions.create(
model="deepseek-ai/DeepSeek-V3", model="deepseek-ai/DeepSeek-V3",
messages=[ messages=[
{"role": "system", "content": _AI_SYSTEM_PROMPT}, {"role": "system", "content": _AI_SYSTEM_PROMPT},
{"role": "user", "content": message}, {"role": "user", "content": message},
], ],
stream=False, stream=False,
),
) )
return response.choices[0].message.content or "" return response.choices[0].message.content or ""
@@ -183,7 +181,6 @@ async def handle_message(event: MessageEvent, bot: Bot):
except Exception as e: except Exception as e:
logger.error(f"chatai处理失败: user_id={event.user_id} error={e}") logger.error(f"chatai处理失败: user_id={event.user_id} error={e}")
await asyncio.sleep(random.uniform(2, 3)) await asyncio.sleep(random.uniform(2, 3))
logger.error(f"chatai详细错误: {e}")
await message_handler.finish("出错了,请稍后再试~") await message_handler.finish("出错了,请稍后再试~")

View File

@@ -1,6 +1,8 @@
import asyncio import asyncio
import re
import html as html_module import html as html_module
import markdown import markdown
import bleach
from nonebot import logger from nonebot import logger
async def markdown_to_image(markdown_text: str, output_path: str, browser=None): async def markdown_to_image(markdown_text: str, output_path: str, browser=None):
@@ -11,6 +13,15 @@ async def markdown_to_image(markdown_text: str, output_path: str, browser=None):
# Convert markdown to HTML. The markdown library handles special chars safely. # Convert markdown to HTML. The markdown library handles special chars safely.
# Note: do NOT html.escape() before markdown.markdown() - it breaks markdown syntax. # Note: do NOT html.escape() before markdown.markdown() - it breaks markdown syntax.
html_content = markdown.markdown(markdown_text, extensions=["fenced_code", "tables"]) html_content = markdown.markdown(markdown_text, extensions=["fenced_code", "tables"])
# Sanitize to prevent XSS from malicious AI responses
allowed_tags = [
'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'p', 'br', 'hr',
'ul', 'ol', 'li', 'blockquote', 'pre', 'code', 'span',
'table', 'thead', 'tbody', 'tr', 'th', 'td',
'strong', 'em', 'b', 'i', 'u', 'a', 'img', 'div',
]
allowed_attrs = {'a': ['href', 'title'], 'img': ['src', 'alt', 'title']}
html_content = bleach.clean(html_content, tags=allowed_tags, attributes=allowed_attrs)
# 使用传入的浏览器实例或创建新的 # 使用传入的浏览器实例或创建新的
if browser is None: if browser is None:

View File

@@ -42,8 +42,8 @@ class PointsAPI:
def _add(): def _add():
with self._lock: with self._lock:
conn = self.db.get_connection() conn = self.db.get_connection()
cursor = conn.cursor()
try: try:
cursor = conn.cursor()
# Ensure user exists # Ensure user exists
self.db.ensure_user_exists(user_id, conn) self.db.ensure_user_exists(user_id, conn)
@@ -60,7 +60,6 @@ class PointsAPI:
if self.config.POINTS_MAX_BALANCE > 0: if self.config.POINTS_MAX_BALANCE > 0:
if new_balance > self.config.POINTS_MAX_BALANCE: if new_balance > self.config.POINTS_MAX_BALANCE:
conn.rollback() conn.rollback()
conn.close()
return False, current_balance return False, current_balance
# Update balance and total_earned # Update balance and total_earned
@@ -85,13 +84,13 @@ class PointsAPI:
) )
conn.commit() conn.commit()
conn.close()
return True, new_balance return True, new_balance
except Exception as e: except Exception as e:
conn.rollback() conn.rollback()
conn.close()
logger.error(f"add_points failed for {user_id}: {e}") logger.error(f"add_points failed for {user_id}: {e}")
return False, 0 return False, 0
finally:
conn.close()
return await asyncio.to_thread(_add) return await asyncio.to_thread(_add)
@@ -116,8 +115,8 @@ class PointsAPI:
def _spend(): def _spend():
with self._lock: with self._lock:
conn = self.db.get_connection() conn = self.db.get_connection()
cursor = conn.cursor()
try: try:
cursor = conn.cursor()
# Ensure user exists # Ensure user exists
self.db.ensure_user_exists(user_id, conn) self.db.ensure_user_exists(user_id, conn)
@@ -132,7 +131,6 @@ class PointsAPI:
# Check sufficient balance # Check sufficient balance
if current_balance < amount: if current_balance < amount:
conn.rollback() conn.rollback()
conn.close()
return False, current_balance return False, current_balance
# Update balance and total_spent # Update balance and total_spent
@@ -158,13 +156,13 @@ class PointsAPI:
) )
conn.commit() conn.commit()
conn.close()
return True, new_balance return True, new_balance
except Exception as e: except Exception as e:
conn.rollback() conn.rollback()
conn.close()
logger.error(f"spend_points failed for {user_id}: {e}") logger.error(f"spend_points failed for {user_id}: {e}")
return False, 0 return False, 0
finally:
conn.close()
return await asyncio.to_thread(_spend) return await asyncio.to_thread(_spend)
@@ -184,8 +182,8 @@ class PointsAPI:
def _set(): def _set():
with self._lock: with self._lock:
conn = self.db.get_connection() conn = self.db.get_connection()
cursor = conn.cursor()
try: try:
cursor = conn.cursor()
# Ensure user exists # Ensure user exists
self.db.ensure_user_exists(user_id, conn) self.db.ensure_user_exists(user_id, conn)
@@ -201,7 +199,6 @@ class PointsAPI:
# If new value equals old value, return without writing # If new value equals old value, return without writing
if current_balance == amount: if current_balance == amount:
conn.rollback() conn.rollback()
conn.close()
return True, amount return True, amount
# Calculate difference for total_earned (only positive diff) # Calculate difference for total_earned (only positive diff)
@@ -230,13 +227,13 @@ class PointsAPI:
) )
conn.commit() conn.commit()
conn.close()
return True, amount return True, amount
except Exception as e: except Exception as e:
conn.rollback() conn.rollback()
conn.close()
logger.error(f"set_points failed for {user_id}: {e}") logger.error(f"set_points failed for {user_id}: {e}")
return False, 0 return False, 0
finally:
conn.close()
return await asyncio.to_thread(_set) return await asyncio.to_thread(_set)

View File

@@ -77,11 +77,13 @@ class PointsDatabase:
def get_user_balance(self, user_id: str) -> int: def get_user_balance(self, user_id: str) -> int:
"""Get user's current points balance.""" """Get user's current points balance."""
conn = self.get_connection() conn = self.get_connection()
try:
cursor = conn.cursor() cursor = conn.cursor()
cursor.execute("SELECT points FROM user_points WHERE user_id = ?", (user_id,)) cursor.execute("SELECT points FROM user_points WHERE user_id = ?", (user_id,))
row = cursor.fetchone() row = cursor.fetchone()
conn.close()
return row["points"] if row else 0 return row["points"] if row else 0
finally:
conn.close()
def ensure_user_exists(self, user_id: str, conn=None) -> None: def ensure_user_exists(self, user_id: str, conn=None) -> None:
"""Create user account if it doesn't exist. Reuses provided conn if given.""" """Create user account if it doesn't exist. Reuses provided conn if given."""

View File

@@ -42,7 +42,8 @@ def register_routes():
routes = create_routes(plugin_config.Token, plugin_config) routes = create_routes(plugin_config.Token, plugin_config)
driver.server_app.include_router(routes) driver.server_app.include_router(routes)
logger.info(f"[Danding_QqPush] API 路由已注册: /danding/qqpush/{plugin_config.Token}") masked = plugin_config.Token[:4] + "***" if len(plugin_config.Token) > 4 else "***"
logger.info(f"[Danding_QqPush] API 路由已注册: /danding/qqpush/{masked}")
# 插件加载时注册路由 # 插件加载时注册路由

View File

@@ -10,9 +10,24 @@ from .text_parser import TextParser
from .image_render import ImageRenderer from .image_render import ImageRenderer
# Module-level singleton: load font once, reuse across requests # Module-level singleton: load font once, reuse across requests
_renderer = _renderer # reuse module-level singleton _renderer: Optional['ImageRenderer'] = None
from .sender import sender from .sender import sender
def _get_renderer(config: Config) -> 'ImageRenderer':
global _renderer
if _renderer is None:
_renderer = ImageRenderer(
width=config.ImageWidth,
font_size=config.ImageFontSize,
padding=config.ImagePadding,
line_spacing=config.ImageLineSpacing,
bg_color=config.ImageBgColor,
text_color=config.ImageTextColor,
font_paths=config.FontPaths,
)
return _renderer
# 请求体模型 # 请求体模型
class PushRequest(BaseModel): class PushRequest(BaseModel):
@@ -93,18 +108,8 @@ def create_routes(token: str, config: Config):
parsed_text = text_parser.parse(data.text) parsed_text = text_parser.parse(data.text)
logger.info(f"解析文本: {parsed_text[:50]}..." if len(parsed_text) > 50 else parsed_text) logger.info(f"解析文本: {parsed_text[:50]}..." if len(parsed_text) > 50 else parsed_text)
# 4. 生成图片 # 4. 生成图片 (reuse shared renderer to avoid per-request font loading)
image_renderer = ImageRenderer( image_base64 = await asyncio.to_thread(_get_renderer(config).render_to_base64, parsed_text)
width=config.ImageWidth,
font_size=config.ImageFontSize,
padding=config.ImagePadding,
line_spacing=config.ImageLineSpacing,
bg_color=config.ImageBgColor,
text_color=config.ImageTextColor,
font_paths=config.FontPaths
)
image_base64 = await asyncio.to_thread(image_renderer.render_to_base64, parsed_text)
logger.info("图片生成成功") logger.info("图片生成成功")
# 5. 发送消息 # 5. 发送消息

View File

@@ -234,67 +234,62 @@ async def settle_race(room: Room) -> tuple[RaceResult, dict[str, float]] | None:
if not champion: if not champion:
return None return None
user_ids = set() odds = calculate_odds(room)
# Collect all affected user IDs
user_ids: set[str] = set()
for horse in room.horses.values(): for horse in room.horses.values():
user_ids.add(horse.owner_id) user_ids.add(horse.owner_id)
for horse in room.horses.values(): for bet in room.bets:
for bet in horse.bets:
user_ids.add(bet.user_id) user_ids.add(bet.user_id)
pre_balances = {} # Record pre-balances
pre_balances: dict[str, int] = {}
for uid in user_ids: for uid in user_ids:
balance = await points_service.get_balance(uid) pre_balances[uid] = points_service.get_balance(uid)
pre_balances[uid] = balance if balance is not None else 0
participant_points = config.PARTICIPANT_REWARD # 1. Reward all participants
for horse in room.horses.values(): for horse in room.horses.values():
ret, code = await points_service.reward_participant(horse.owner_id, participant_points) try:
if not ret and code != POINTS_ERR_CODE_DUPLICATE: await points_service.reward_participant(horse.owner_id)
logger.warning(f"reward_participant failed for {horse.owner_id}: code={code}") except Exception as e:
logger.warning(f"reward_participant failed for {horse.owner_id}: {e}")
champion_points = config.CHAMPION_REWARD # 2. Champion bonus
ret, code = await points_service.reward_champion(champion.owner_id, champion_points) try:
if not ret and code != POINTS_ERR_CODE_DUPLICATE: await points_service.reward_champion(champion.owner_id)
logger.warning(f"reward_champion failed for {champion.owner_id}: code={code}") except Exception as e:
logger.warning(f"reward_champion failed for {champion.owner_id}: {e}")
all_bets = [] # 3. Bet payouts for winners
for horse_name, horse in room.horses.items(): for bet in room.bets:
all_bets.extend(horse.bets) if bet.horse_name == room.champion_name:
try:
await points_service.payout_winnings(
bet.user_id, bet.amount, odds.get(bet.horse_name, config.MIN_ODDS)
)
except Exception as e:
logger.warning(f"payout_winnings failed for {bet.user_id}: {e}")
total_bet = sum(bet.amount for bet in all_bets) # Record post-balances and compute deltas
if total_bet == 0: post_balances: dict[str, int] = {}
odds = {}
else:
odds = {}
for horse_name, horse in room.horses.items():
horse_bet = sum(bet.amount for bet in horse.bets)
if horse_bet == 0:
odds[horse_name] = config.MAX_ODDS
else:
odds[horse_name] = max(config.MIN_ODDS, total_bet / horse_bet)
champion_bets = room.horses[room.champion_name].bets
for bet in champion_bets:
win_amount = int(bet.amount * odds[room.champion_name])
ret, code = await points_service.payout_winnings(bet.user_id, win_amount)
if not ret and code != POINTS_ERR_CODE_DUPLICATE:
logger.warning(f"payout_winnings failed for {bet.user_id}: code={code}")
post_balances = {}
for uid in user_ids: for uid in user_ids:
balance = await points_service.get_balance(uid) post_balances[uid] = points_service.get_balance(uid)
post_balances[uid] = balance if balance is not None else 0
point_changes = {} point_changes: dict[str, int] = {}
for uid in user_ids: for uid in user_ids:
delta = post_balances[uid] - pre_balances[uid] delta = post_balances[uid] - pre_balances[uid]
if delta != 0: if delta != 0:
point_changes[uid] = delta point_changes[uid] = delta
# Build human-readable summaries
_, point_change_summaries = _build_point_changes(room, odds)
result = RaceResult( result = RaceResult(
champion_name=room.champion_name, champion_name=room.champion_name,
finishing_order=[name for name in room.finishing_order if name in room.horses], champion_owner=champion.owner_id,
point_changes=point_changes point_changes=point_changes,
point_change_summaries=point_change_summaries,
) )
return result, odds return result, odds

View File

@@ -1,9 +1,12 @@
import asyncio import asyncio
import logging
from typing import Optional, Any from typing import Optional, Any
from nonebot.adapters.onebot.v11 import Bot, Message from nonebot.adapters.onebot.v11 import Bot, Message
from .config import Config from .config import Config
logger = logging.getLogger("horse_racing.message_service")
class MessageService: class MessageService:
def __init__(self, config: Config): def __init__(self, config: Config):

View File

@@ -47,13 +47,13 @@ class Room:
@dataclass @dataclass
class RaceResult: class RaceResult:
race_id: str
scope: str
champion_name: str champion_name: str
champion_owner: str champion_owner: str
participants: list[str]
bet_distribution: dict[str, int]
duration_ticks: int
completed_at: datetime
point_changes: dict[str, int] = field(default_factory=dict) point_changes: dict[str, int] = field(default_factory=dict)
point_change_summaries: dict[str, str] = field(default_factory=dict) point_change_summaries: dict[str, str] = field(default_factory=dict)
race_id: str = ""
scope: str = ""
participants: list[str] = field(default_factory=list)
bet_distribution: dict[str, int] = field(default_factory=dict)
duration_ticks: int = 0
completed_at: datetime = field(default_factory=datetime.now)

View File

@@ -9,6 +9,7 @@ arclet-alconna-tools==0.7.10
argcomplete==3.5.2 argcomplete==3.5.2
async-timeout==5.0.1 async-timeout==5.0.1
beautifulsoup4==4.12.3 beautifulsoup4==4.12.3
bleach==6.2.0
bs4==0.0.2 bs4==0.0.2
certifi==2024.12.14 certifi==2024.12.14
charset-normalizer==3.4.0 charset-normalizer==3.4.0

View File

@@ -51,3 +51,26 @@
## 代码质量总结 ## 代码质量总结
修复后评级:**B+** (架构清晰安全问题已修复async处理合理) 修复后评级:**B+** (架构清晰安全问题已修复async处理合理)
## 第二轮修复 (新增4项)
| # | 严重度 | 问题 | 文件 |
|---|--------|------|------|
| 6 | **严重** | `api.py` L13 自引用 `_renderer = _renderer`,运行时 NameError 崩溃 | api.py |
| 7 | **严重** | 每次请求新建 `ImageRenderer`,加载字体文件,性能极差 | api.py |
| 8 | **中** | `__init__.py` Token 明文输出到日志,信息泄露 | __init__.py |
| 9 | **中** | `image_render.py` 双 Pilmoji 上下文,标题和正文各创建一次 | image_render.py |
### 修复详情
**api.py**
- L13: `_renderer = _renderer``_renderer: Optional['ImageRenderer'] = None`(修复 NameError
- 新增 `_get_renderer(config)` 懒加载单例函数,首次调用创建,后续复用
- `_send_image_push``_get_renderer(config).render_to_base64()` 替代每次 `ImageRenderer(config)`
-`Optional` 导入
**__init__.py**
- Token 日志掩码:`plugin_config.Token[:4] + "***"`
### 测试结果
- 34/34 通过(含原有 + 回归)