refactor(security): 针对普通用户采用安全白名单投影过滤技能与MCP元数据
1. 技能列表接口：普通用户返回数据通过白名单字段投影，屏蔽 dir_path 等绝对物理路径。 2. MCP 服务器列表接口：废除原 pop() 黑名单模式，升级为显式白名单准入映射。 3. 单元测试：同步升级 test_skill_router.py 和 test_mcp_router.py，强化字段脱敏性测试断言。
This commit is contained in:
supreme0597
parent
261ea88b11
commit
9406463849
@ -89,16 +89,19 @@ async def get_mcp_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:
|
||||
d = s.to_dict()
|
||||
d.pop("url", None)
|
||||
d.pop("command", None)
|
||||
d.pop("args", None)
|
||||
d.pop("env", None)
|
||||
d.pop("headers", None)
|
||||
data.append(d)
|
||||
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}")
|
||||
|
||||
@ -10,7 +10,11 @@ from pydantic import BaseModel, Field
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
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.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_required_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 失败")
|
||||
|
||||
|
||||
|
||||
@ -18,6 +18,7 @@ def _build_app(*, allow_admin: bool = True) -> FastAPI:
|
||||
async def fake_admin_user():
|
||||
if not allow_admin:
|
||||
from fastapi import HTTPException
|
||||
|
||||
raise HTTPException(status_code=403, detail="需要管理员权限")
|
||||
return User(
|
||||
username="admin",
|
||||
@ -119,7 +120,7 @@ def test_get_mcp_servers_normal_user_is_stripped(monkeypatch):
|
||||
assert data_admin["command"] == "python"
|
||||
assert data_admin["env"] == {"API_KEY": "secret"}
|
||||
|
||||
# 2. 普通用户请求，敏感字段应该被脱敏（剔除）
|
||||
# 2. 普通用户请求，敏感字段及一切非安全白名单字段应该被彻底脱敏
|
||||
client_user = TestClient(_build_app(allow_admin=False))
|
||||
resp_user = client_user.get("/api/system/mcp-servers")
|
||||
assert resp_user.status_code == 200
|
||||
@ -128,5 +129,7 @@ def test_get_mcp_servers_normal_user_is_stripped(monkeypatch):
|
||||
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
|
||||
|
||||
@ -264,12 +264,18 @@ def test_list_skills_route_normal_user_success(monkeypatch):
|
||||
|
||||
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
|
||||
assert payload["data"][0]["slug"] == "test-skill"
|
||||
assert payload["data"][0]["name"] == "test-skill-name"
|
||||
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
|
||||
|
||||
Loading…
Reference in New Issue
Block a user