fix(skills): 增强安全性与交互体验

- 添加 skill slug 格式校验,防止路径遍历攻击
- 优化导出文件清理逻辑,添加异常处理
- 前端依赖管理表单添加禁用状态联动
- 补充相关单元测试

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
肖泽涛 2026-02-23 20:52:26 +08:00
parent 2666b27fcb
commit 0f7c0d3017
7 changed files with 70 additions and 7 deletions

View File

@ -52,6 +52,13 @@ def _raise_from_value_error(e: ValueError) -> None:
raise HTTPException(status_code=status_code, detail=message) raise HTTPException(status_code=status_code, detail=message)
def _cleanup_export_file(path: str) -> None:
try:
Path(path).unlink(missing_ok=True)
except Exception as e:
logger.warning(f"Failed to cleanup exported skill archive '{path}': {e}")
@skills.get("") @skills.get("")
async def list_skills_route( async def list_skills_route(
_current_user: User = Depends(get_admin_user), _current_user: User = Depends(get_admin_user),
@ -252,7 +259,7 @@ async def export_skill_route(
"""导出技能压缩包(仅超级管理员)。""" """导出技能压缩包(仅超级管理员)。"""
try: try:
export_path, download_name = await export_skill_zip(db, slug) export_path, download_name = await export_skill_zip(db, slug)
background_tasks.add_task(lambda p: Path(p).unlink(missing_ok=True), export_path) background_tasks.add_task(_cleanup_export_file, export_path)
return FileResponse( return FileResponse(
path=export_path, path=export_path,
media_type="application/zip", media_type="application/zip",

View File

@ -6,7 +6,7 @@ from typing import Any
from deepagents.backends import CompositeBackend, FilesystemBackend, StateBackend from deepagents.backends import CompositeBackend, FilesystemBackend, StateBackend
from deepagents.backends.protocol import EditResult, FileDownloadResponse, FileUploadResponse, WriteResult from deepagents.backends.protocol import EditResult, FileDownloadResponse, FileUploadResponse, WriteResult
from src.services.skill_service import get_expanded_visible_skill_slugs, get_skills_root_dir from src.services.skill_service import get_expanded_visible_skill_slugs, get_skills_root_dir, is_valid_skill_slug
class SelectedSkillsReadonlyBackend(FilesystemBackend): class SelectedSkillsReadonlyBackend(FilesystemBackend):
@ -15,7 +15,9 @@ class SelectedSkillsReadonlyBackend(FilesystemBackend):
def __init__(self, *, selected_slugs: list[str] | None): def __init__(self, *, selected_slugs: list[str] | None):
super().__init__(root_dir=get_skills_root_dir(), virtual_mode=True) super().__init__(root_dir=get_skills_root_dir(), virtual_mode=True)
self._selected_slugs = { self._selected_slugs = {
str(slug).strip() for slug in (selected_slugs or []) if isinstance(slug, str) and str(slug).strip() str(slug).strip()
for slug in (selected_slugs or [])
if isinstance(slug, str) and is_valid_skill_slug(str(slug))
} }
def _extract_slug(self, path: str | None) -> str | None: def _extract_slug(self, path: str | None) -> str | None:

View File

@ -14,7 +14,11 @@ from langgraph.types import Command
from src.agents.common import load_chat_model from src.agents.common import load_chat_model
from src.agents.common.tools import get_buildin_tools, get_kb_based_tools from src.agents.common.tools import get_buildin_tools, get_kb_based_tools
from src.services.mcp_service import get_enabled_mcp_tools from src.services.mcp_service import get_enabled_mcp_tools
from src.services.skill_service import get_dependency_bundle_for_activated_skills, get_skill_prompt_metadata_by_slugs from src.services.skill_service import (
get_dependency_bundle_for_activated_skills,
get_skill_prompt_metadata_by_slugs,
is_valid_skill_slug,
)
from src.utils.datetime_utils import shanghai_now from src.utils.datetime_utils import shanghai_now
from src.utils.logging_config import logger from src.utils.logging_config import logger
@ -283,7 +287,10 @@ class RuntimeConfigMiddleware(AgentMiddleware):
return None return None
if parts[0] != "skills" or parts[2] != "SKILL.md": if parts[0] != "skills" or parts[2] != "SKILL.md":
return None return None
return parts[1] slug = parts[1]
if not is_valid_skill_slug(slug):
return None
return slug
def _merge_activated_skill_update(self, result: Any, slug: str): def _merge_activated_skill_update(self, result: Any, slug: str):
if isinstance(result, Command): if isinstance(result, Command):

View File

@ -18,7 +18,8 @@ from src.storage.postgres.manager import pg_manager
from src.storage.postgres.models_business import Skill from src.storage.postgres.models_business import Skill
from src.utils.logging_config import logger from src.utils.logging_config import logger
SKILL_NAME_PATTERN = re.compile(r"^[a-z0-9]+(-[a-z0-9]+)*$") SKILL_SLUG_PATTERN = re.compile(r"^[a-z0-9]+(-[a-z0-9]+)*$")
SKILL_NAME_PATTERN = SKILL_SLUG_PATTERN
FRONTMATTER_PATTERN = re.compile(r"^---\s*\n(.*?)\n---\s*\n", re.DOTALL) FRONTMATTER_PATTERN = re.compile(r"^---\s*\n(.*?)\n---\s*\n", re.DOTALL)
TEXT_FILE_EXTENSIONS = { TEXT_FILE_EXTENSIONS = {
@ -72,6 +73,19 @@ def _normalize_string_list(values: list[str] | None) -> list[str]:
return normalized return normalized
def is_valid_skill_slug(slug: str) -> bool:
if not isinstance(slug, str):
return False
return bool(SKILL_SLUG_PATTERN.match(slug.strip()))
def validate_skill_slug(slug: str) -> str:
normalized = slug.strip() if isinstance(slug, str) else ""
if not is_valid_skill_slug(normalized):
raise ValueError("无效 skill slug")
return normalized
def _get_buildin_tool_names() -> list[str]: def _get_buildin_tool_names() -> list[str]:
from src.agents.common.tools import get_buildin_tools from src.agents.common.tools import get_buildin_tools
@ -491,6 +505,7 @@ async def import_skill_zip(
async def get_skill_or_raise(db: AsyncSession, slug: str) -> Skill: async def get_skill_or_raise(db: AsyncSession, slug: str) -> Skill:
slug = validate_skill_slug(slug)
repo = SkillRepository(db) repo = SkillRepository(db)
item = await repo.get_by_slug(slug) item = await repo.get_by_slug(slug)
if not item: if not item:

View File

@ -184,6 +184,23 @@ async def test_awrap_tool_call_activates_skill_when_read_skill_md():
assert len(result.update["messages"]) == 1 assert len(result.update["messages"]) == 1
@pytest.mark.asyncio
async def test_awrap_tool_call_skips_invalid_skill_slug_path():
middleware = _build_middleware()
request = _FakeToolCallRequest(
tool_call={
"name": "read_file",
"args": {"file_path": "/skills/../SKILL.md"},
}
)
async def _handler(_request):
return ToolMessage(content="ok", tool_call_id="tc-1")
result = await middleware.awrap_tool_call(request, _handler)
assert isinstance(result, ToolMessage)
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_awrap_tool_call_merges_with_existing_command_update(): async def test_awrap_tool_call_merges_with_existing_command_update():
middleware = _build_middleware() middleware = _build_middleware()

View File

@ -37,6 +37,12 @@ def test_parse_skill_markdown_requires_frontmatter():
svc._parse_skill_markdown("# missing") svc._parse_skill_markdown("# missing")
def test_validate_skill_slug():
assert svc.validate_skill_slug("demo-skill") == "demo-skill"
with pytest.raises(ValueError, match="无效 skill slug"):
svc.validate_skill_slug("../bad")
def test_get_skill_prompt_metadata_by_slugs_dedup_and_skip_missing(monkeypatch: pytest.MonkeyPatch): def test_get_skill_prompt_metadata_by_slugs_dedup_and_skip_missing(monkeypatch: pytest.MonkeyPatch):
monkeypatch.setattr( monkeypatch.setattr(
svc, svc,

View File

@ -59,7 +59,13 @@
<div class="dependency-panel"> <div class="dependency-panel">
<div class="dependency-header"> <div class="dependency-header">
<span class="dependency-title">依赖管理</span> <span class="dependency-title">依赖管理</span>
<a-button type="primary" size="small" :loading="savingDependencies" @click="saveDependencies"> <a-button
type="primary"
size="small"
:loading="savingDependencies"
:disabled="loading || savingDependencies"
@click="saveDependencies"
>
保存依赖 保存依赖
</a-button> </a-button>
</div> </div>
@ -70,6 +76,7 @@
mode="multiple" mode="multiple"
:options="toolDependencyOptions" :options="toolDependencyOptions"
placeholder="选择工具依赖" placeholder="选择工具依赖"
:disabled="loading || savingDependencies"
allow-clear allow-clear
/> />
</a-form-item> </a-form-item>
@ -79,6 +86,7 @@
mode="multiple" mode="multiple"
:options="mcpDependencyOptions" :options="mcpDependencyOptions"
placeholder="选择 MCP 服务依赖" placeholder="选择 MCP 服务依赖"
:disabled="loading || savingDependencies"
allow-clear allow-clear
/> />
</a-form-item> </a-form-item>
@ -88,6 +96,7 @@
mode="multiple" mode="multiple"
:options="skillDependencyOptions" :options="skillDependencyOptions"
placeholder="选择 Skill 依赖" placeholder="选择 Skill 依赖"
:disabled="loading || savingDependencies"
allow-clear allow-clear
/> />
</a-form-item> </a-form-item>