Merge pull request #710 from supreme0597/fix/mention-skill-mcp
fix(mention): 修复普通用户无法在聊天中唤出技能与MCP提及列表的Bug
This commit is contained in:
commit
6b16c41604
@ -17,7 +17,7 @@ from yuxi.services.mcp_service import (
|
||||
)
|
||||
from yuxi.storage.postgres.models_business import User
|
||||
from yuxi.utils import logger
|
||||
from server.utils.auth_middleware import get_admin_user, get_db
|
||||
from server.utils.auth_middleware import get_admin_user, get_db, get_required_user
|
||||
|
||||
mcp = APIRouter(prefix="/system/mcp-servers", tags=["mcp"])
|
||||
|
||||
@ -80,13 +80,29 @@ async def get_server_or_404(db: AsyncSession, name: str):
|
||||
|
||||
@mcp.get("")
|
||||
async def get_mcp_servers(
|
||||
current_user: User = Depends(get_admin_user),
|
||||
current_user: User = Depends(get_required_user),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
"""获取所有 MCP 服务器配置"""
|
||||
"""获取所有 MCP 服务器配置(普通用户仅获取脱敏的基础信息)"""
|
||||
try:
|
||||
servers = await get_all_mcp_servers(db)
|
||||
return {"success": True, "data": [s.to_dict() for s in servers]}
|
||||
if current_user.role in ["admin", "superadmin"]:
|
||||
return {"success": True, "data": [s.to_dict() for s in servers]}
|
||||
else:
|
||||
# NOTE: 针对普通用户采用高安全显式白名单字段准入投影,使用 getattr 兼容 Mock
|
||||
# 仿真对象和历史数据,避免未来新增敏感字段或审计信息越权泄露
|
||||
data = []
|
||||
for s in servers:
|
||||
data.append(
|
||||
{
|
||||
"name": getattr(s, "name", ""),
|
||||
"description": getattr(s, "description", None),
|
||||
"icon": getattr(s, "icon", None),
|
||||
"enabled": bool(getattr(s, "enabled", True)),
|
||||
"tags": getattr(s, "tags", None) or [],
|
||||
}
|
||||
)
|
||||
return {"success": True, "data": data}
|
||||
except Exception as e:
|
||||
logger.error(f"Failed to get MCP servers: {e}")
|
||||
raise HTTPException(status_code=500, detail=str(e))
|
||||
|
||||
@ -9,8 +9,12 @@ from fastapi.responses import FileResponse
|
||||
from pydantic import BaseModel, Field
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from server.utils.auth_middleware import get_admin_user, get_db
|
||||
from yuxi.services.remote_skill_install_service import install_remote_skill, install_remote_skills_batch, list_remote_skills
|
||||
from server.utils.auth_middleware import get_admin_user, get_db, get_required_user
|
||||
from yuxi.services.remote_skill_install_service import (
|
||||
install_remote_skill,
|
||||
install_remote_skills_batch,
|
||||
list_remote_skills,
|
||||
)
|
||||
from yuxi.services.skill_service import (
|
||||
BuiltinSkillUpdateConflictError,
|
||||
create_skill_node,
|
||||
@ -82,13 +86,29 @@ def _cleanup_export_file(path: str) -> None:
|
||||
|
||||
@skills.get("")
|
||||
async def list_skills_route(
|
||||
_current_user: User = Depends(get_admin_user),
|
||||
current_user: User = Depends(get_required_user),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
"""获取技能列表(管理员可读)。"""
|
||||
"""获取技能列表(普通用户仅获取白名单脱敏数据,管理员可读完整元数据)。"""
|
||||
try:
|
||||
items = await list_skills(db)
|
||||
return {"success": True, "data": [item.to_dict() for item in items]}
|
||||
|
||||
# NOTE: 针对管理员与常规登录用户分流返回,防止物理目录结构(dir_path)与系统审计信息越权暴露给常规用户
|
||||
if current_user.role in ["admin", "superadmin"]:
|
||||
return {"success": True, "data": [item.to_dict() for item in items]}
|
||||
|
||||
safe_data = []
|
||||
for item in items:
|
||||
safe_data.append(
|
||||
{
|
||||
"slug": item.slug,
|
||||
"name": item.name,
|
||||
"description": item.description,
|
||||
"version": item.version,
|
||||
"is_builtin": item.is_builtin,
|
||||
}
|
||||
)
|
||||
return {"success": True, "data": safe_data}
|
||||
except Exception as e:
|
||||
logger.error(f"Failed to list skills: {e}")
|
||||
raise HTTPException(status_code=500, detail="获取技能列表失败")
|
||||
@ -249,9 +269,7 @@ async def install_remote_skill_route(
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
logger.error(
|
||||
f"Failed to install remote skill '{payload.skill}' from '{payload.source}': {e}"
|
||||
)
|
||||
logger.error(f"Failed to install remote skill '{payload.skill}' from '{payload.source}': {e}")
|
||||
raise HTTPException(status_code=500, detail="安装远程 skill 失败")
|
||||
|
||||
|
||||
@ -281,9 +299,7 @@ async def install_remote_skills_batch_route(
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
logger.error(
|
||||
f"Failed to install remote skills batch from '{payload.source}': {e}"
|
||||
)
|
||||
logger.error(f"Failed to install remote skills batch from '{payload.source}': {e}")
|
||||
raise HTTPException(status_code=500, detail="批量安装远程 skills 失败")
|
||||
|
||||
|
||||
|
||||
@ -4,11 +4,11 @@ from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from server.routers.mcp_router import mcp
|
||||
from server.utils.auth_middleware import get_admin_user, get_db
|
||||
from server.utils.auth_middleware import get_admin_user, get_db, get_required_user
|
||||
from yuxi.storage.postgres.models_business import User
|
||||
|
||||
|
||||
def _build_app() -> FastAPI:
|
||||
def _build_app(*, allow_admin: bool = True) -> FastAPI:
|
||||
app = FastAPI()
|
||||
app.include_router(mcp, prefix="/api")
|
||||
|
||||
@ -16,6 +16,10 @@ def _build_app() -> FastAPI:
|
||||
return None
|
||||
|
||||
async def fake_admin_user():
|
||||
if not allow_admin:
|
||||
from fastapi import HTTPException
|
||||
|
||||
raise HTTPException(status_code=403, detail="需要管理员权限")
|
||||
return User(
|
||||
username="admin",
|
||||
user_id="admin",
|
||||
@ -23,8 +27,17 @@ def _build_app() -> FastAPI:
|
||||
role="admin",
|
||||
)
|
||||
|
||||
async def fake_required_user():
|
||||
return User(
|
||||
username="admin" if allow_admin else "user",
|
||||
user_id="admin" if allow_admin else "user",
|
||||
password_hash="x",
|
||||
role="admin" if allow_admin else "user",
|
||||
)
|
||||
|
||||
app.dependency_overrides[get_db] = fake_db
|
||||
app.dependency_overrides[get_admin_user] = fake_admin_user
|
||||
app.dependency_overrides[get_required_user] = fake_required_user
|
||||
return app
|
||||
|
||||
|
||||
@ -65,3 +78,58 @@ def test_update_mcp_server_status_not_found(monkeypatch):
|
||||
client = TestClient(_build_app())
|
||||
resp = client.put("/api/system/mcp-servers/missing/status", json={"enabled": True})
|
||||
assert resp.status_code == 404, resp.text
|
||||
|
||||
|
||||
def test_get_mcp_servers_normal_user_is_stripped(monkeypatch):
|
||||
class DummyServer:
|
||||
def __init__(self):
|
||||
self.name = "test-mcp"
|
||||
self.description = "test mcp description"
|
||||
self.transport = "stdio"
|
||||
self.url = "http://localhost:8000"
|
||||
self.command = "python"
|
||||
self.args = ["-m", "mcp"]
|
||||
self.env = {"API_KEY": "secret"}
|
||||
self.headers = {"Auth": "Bearer secret"}
|
||||
self.enabled = 1
|
||||
|
||||
def to_dict(self):
|
||||
return {
|
||||
"name": self.name,
|
||||
"description": self.description,
|
||||
"transport": self.transport,
|
||||
"url": self.url,
|
||||
"command": self.command,
|
||||
"args": self.args,
|
||||
"env": self.env,
|
||||
"headers": self.headers,
|
||||
"enabled": bool(self.enabled),
|
||||
}
|
||||
|
||||
async def fake_get_all_mcp_servers(db):
|
||||
return [DummyServer()]
|
||||
|
||||
monkeypatch.setattr("server.routers.mcp_router.get_all_mcp_servers", fake_get_all_mcp_servers)
|
||||
|
||||
# 1. 管理员请求,应该返回全部字段
|
||||
client_admin = TestClient(_build_app(allow_admin=True))
|
||||
resp_admin = client_admin.get("/api/system/mcp-servers")
|
||||
assert resp_admin.status_code == 200
|
||||
data_admin = resp_admin.json()["data"][0]
|
||||
assert data_admin["url"] == "http://localhost:8000"
|
||||
assert data_admin["command"] == "python"
|
||||
assert data_admin["env"] == {"API_KEY": "secret"}
|
||||
|
||||
# 2. 普通用户请求,敏感字段及一切非安全白名单字段应该被彻底脱敏
|
||||
client_user = TestClient(_build_app(allow_admin=False))
|
||||
resp_user = client_user.get("/api/system/mcp-servers")
|
||||
assert resp_user.status_code == 200
|
||||
data_user = resp_user.json()["data"][0]
|
||||
assert "url" not in data_user
|
||||
assert "command" not in data_user
|
||||
assert "env" not in data_user
|
||||
assert "headers" not in data_user
|
||||
assert "transport" not in data_user # NOTE: 进一步验证连 transport 等配置层元数据也一并过滤
|
||||
assert data_user["name"] == "test-mcp"
|
||||
assert data_user["description"] == "test mcp description"
|
||||
assert data_user["enabled"] is True
|
||||
|
||||
@ -4,7 +4,7 @@ from fastapi import FastAPI, HTTPException
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from server.routers.skill_router import skills
|
||||
from server.utils.auth_middleware import get_admin_user, get_db
|
||||
from server.utils.auth_middleware import get_admin_user, get_db, get_required_user
|
||||
from yuxi.storage.postgres.models_business import Skill, User
|
||||
|
||||
|
||||
@ -25,8 +25,17 @@ def _build_app(*, allow_admin: bool = True) -> FastAPI:
|
||||
role="admin",
|
||||
)
|
||||
|
||||
async def fake_required_user():
|
||||
return User(
|
||||
username="admin" if allow_admin else "user",
|
||||
user_id="admin" if allow_admin else "user",
|
||||
password_hash="x",
|
||||
role="admin" if allow_admin else "user",
|
||||
)
|
||||
|
||||
app.dependency_overrides[get_db] = fake_db
|
||||
app.dependency_overrides[get_admin_user] = fake_admin_user
|
||||
app.dependency_overrides[get_required_user] = fake_required_user
|
||||
return app
|
||||
|
||||
|
||||
@ -240,3 +249,33 @@ def test_install_remote_skill_route(monkeypatch):
|
||||
assert captured["source"] == "anthropics/skills"
|
||||
assert captured["skill"] == "frontend-design"
|
||||
assert captured["created_by"] == "admin"
|
||||
|
||||
|
||||
def test_list_skills_route_normal_user_success(monkeypatch):
|
||||
async def fake_list_skills(_db):
|
||||
return [
|
||||
Skill(
|
||||
slug="test-skill",
|
||||
name="test-skill-name",
|
||||
description="test skill description",
|
||||
dir_path="skills/test-skill",
|
||||
)
|
||||
]
|
||||
|
||||
monkeypatch.setattr("server.routers.skill_router.list_skills", fake_list_skills)
|
||||
|
||||
# 普通用户应该也能成功获取列表,但返回的字段应被安全白名单投影过滤
|
||||
app = _build_app(allow_admin=False)
|
||||
client = TestClient(app)
|
||||
resp = client.get("/api/system/skills")
|
||||
assert resp.status_code == 200, resp.text
|
||||
payload = resp.json()
|
||||
assert payload["success"] is True
|
||||
skill_data = payload["data"][0]
|
||||
assert skill_data["slug"] == "test-skill"
|
||||
assert skill_data["name"] == "test-skill-name"
|
||||
# NOTE: 验证敏感字段如 dir_path、created_by 以及其它元数据已全部被白名单机制过滤,不发生越权泄露
|
||||
assert "dir_path" not in skill_data
|
||||
assert "created_by" not in skill_data
|
||||
assert "updated_by" not in skill_data
|
||||
assert "content_hash" not in skill_data
|
||||
|
||||
@ -39,6 +39,7 @@
|
||||
|
||||
<!-- 0.6.2 的内容请放在这里 -->
|
||||
- 下放扩展管理权限:普通管理员现在可进入扩展管理并完整管理 Tools、MCP、SubAgent、Skills;同步放开 Skill 管理接口权限并补充权限测试。
|
||||
- 修复聊天中普通用户 `@` 提及出不来技能和 MCP 列表的 Bug:将获取技能列表 `GET /api/system/skills` 与获取 MCP 服务器列表 `GET /api/system/mcp-servers` 的鉴权要求放宽至已登录的普通用户(`get_required_user`);同时为了确保敏感配置的安全性,当普通用户请求 MCP 列表时,后端会自动进行脱敏,剔除 `url`、`command`、`args`、`env` 和 `headers` 等敏感连接参数,并新增了对应的角色鉴权及脱敏剔除测试用例。
|
||||
- 调整 Agent 知识库默认选择:未显式配置知识库时默认启用当前用户可访问的全部知识库,显式保存空列表仍表示不启用知识库。
|
||||
- 优化评估基准自动生成:仅支持 commonrag/Milvus 知识库,默认参考 chunks 数量改为 1;多 chunk 场景复用知识库向量检索选择相似 chunks,不再对全量 chunks 重新计算 embedding,并移除前端 Embedding 模型选择。
|
||||
- 修复知识库文档入库状态回退:当已解析文件缺失 `markdown_file` 解析产物时,索引流程会将文件状态恢复为未解析,便于重新解析而不是停留在索引失败。
|
||||
|
||||
@ -1,4 +1,4 @@
|
||||
import { apiAdminGet, apiAdminPost, apiAdminPut, apiAdminDelete } from './base'
|
||||
import { apiGet, apiAdminGet, apiAdminPost, apiAdminPut, apiAdminDelete } from './base'
|
||||
|
||||
/**
|
||||
* MCP 服务器管理 API 模块
|
||||
@ -16,7 +16,7 @@ const BASE_URL = '/api/system/mcp-servers'
|
||||
* @returns {Promise} - 服务器列表
|
||||
*/
|
||||
export const getMcpServers = async () => {
|
||||
return apiAdminGet(BASE_URL)
|
||||
return apiGet(BASE_URL)
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@ -1,9 +1,9 @@
|
||||
import { apiAdminGet, apiAdminPost, apiAdminPut, apiAdminDelete } from './base'
|
||||
import { apiGet, apiAdminGet, apiAdminPost, apiAdminPut, apiAdminDelete } from './base'
|
||||
|
||||
const BASE_URL = '/api/system/skills'
|
||||
|
||||
export const listSkills = async () => {
|
||||
return apiAdminGet(BASE_URL)
|
||||
return apiGet(BASE_URL)
|
||||
}
|
||||
|
||||
export const importSkillZip = async (file) => {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user