-
Notifications
You must be signed in to change notification settings - Fork 902
feat(mcp/sse): support passthourgh the query parameter in sse server to the rest api server #2460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
新增SSE服务器对查询参数的透传支持及脚本健壮性优化变更概述新功能
问题修复
配置调整
变更文件
时序图sequenceDiagram
participant Client as 客户端
participant SSE as SSEServer
participant Filter as Filter处理器
participant REST as REST API服务器
Client->>SSE: 发起含查询参数的SSE请求
SSE->>Filter: 触发过滤器处理
Filter->>Filter: 提取原始查询参数
Filter->>SSE: 传递完整URL路径(含查询参数)
SSE->>REST: 发起REST调用时携带所有查询参数
REST-->>SSE: 返回处理结果
SSE-->>Client: 通过SSE通道推送结果
💡 小贴士与 lingma-agents 交流的方式📜 直接回复评论
📜 在代码行处标记
📜 在讨论中提问
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔎 代码评审报告
🎯 评审意见概览
| 严重度 | 数量 | 说明 |
|---|---|---|
| 🔴 Blocker | 0 | 阻断性问题,需立即修复。例如:系统崩溃、关键功能不可用或严重安全漏洞。 |
| 🟠 Critical | 1 | 严重问题,高优先级修复。例如:核心功能异常或性能瓶颈影响用户体验。 |
| 🟡 Major | 1 | 主要问题,建议修复。例如:非核心功能缺陷或代码维护性较差。 |
| 🟢 Minor | 0 | 次要问题,酬情优化。例如:代码格式不规范或注释缺失。 |
总计: 2 个问题
📋 评审意见详情
💡 代码实现建议
以下是文件级别的代码建议,聚焦于代码的可读性、可维护性和潜在问题。
🔹 plugins/golang-filter/mcp-session/common/sse.go (2 💬)
- 拼接URL时未对sessionID进行URL编码,可能导致特殊字符问题 (L102-L115)
- 存在重复的URL拼接代码,建议提取为函数 (L102-L115)
🚀 架构设计建议
以下是对代码架构和设计的综合分析,聚焦于跨文件交互、系统一致性和潜在优化空间。
🔍1. 跨文件参数处理不一致可能导致查询参数丢失或注入风险
在filter.go中将原始查询参数直接拼接到URL路径(第134-136行),而在sse.go中处理参数时未对sessionID进行URL编码(虽文件级已指出,但系统层面需确保两端一致)。这种跨文件的参数处理方式可能导致:
- 未验证的用户输入直接拼接到URL,存在注入风险(如恶意构造的查询参数)
- 当
messageEndpoint已包含查询参数时,参数追加逻辑可能导致格式错误(如重复问号) - 前后端参数传递格式不一致导致业务功能失效
📌 关键代码
trimmed += "?" + rqif strings.Contains(s.messageEndpoint, "?") {
...
} else {
...
}-
恶意用户可能通过构造特殊查询参数绕过安全验证
-
参数格式错误导致API调用失败,影响核心功能
-
后续维护时需同时修改多处逻辑,增加维护成本
🔍2. URL拼接逻辑分散导致技术债务
URL拼接逻辑在sse.go(第102-115行)和filter.go(第132-136行)均存在重复实现,建议:
- 将URL构造抽象为公共函数,统一处理参数编码、连接符添加等逻辑
- 建立参数白名单机制,过滤非法查询参数
- 在
common包中提供标准化的URL组合工具类
📌 关键代码
if strings.Contains(s.messageEndpoint, "?") {
messageEndpoint = fmt.Sprintf(...)
} else {
messageEndpoint = fmt.Sprintf(...)
}trimmed += "?" + rq-
重复代码导致维护成本增加,修改一处需同步修改其他位置
-
参数处理逻辑不一致可能引发隐蔽的边界条件错误
-
新增参数需求时需多处修改,违反DRY原则
🔍3. 未验证用户输入可能导致注入攻击
在filter.go的第134行直接将requestUrl.RawQuery拼接到URL路径,未进行任何输入验证或编码处理。攻击者可通过构造包含特殊字符(如%00或#)的查询参数,导致:
URL解析异常
后端API接收到意外参数
路径遍历等攻击
📌 关键代码
trimmed += "?" + rq- 恶意构造的查询参数破坏URL结构
- 触发后端服务未预期的行为
- 违反OWASP安全编码规范
审查详情
📒 文件清单 (3 个文件)
📝 变更: 3 个文件
📝 变更文件:
plugins/golang-filter/mcp-session/common/sse.goplugins/golang-filter/mcp-session/filter.gotools/hack/get-hgctl.sh
💡 小贴士
与 lingma-agents 交流的方式
📜 直接回复评论
直接回复本条评论,lingma-agents 将自动处理您的请求。例如:
-
在当前代码中添加详细的注释说明。
-
请详细介绍一下你说的 LRU 改造方案,并使用伪代码加以说明。
📜 在代码行处标记
在文件的特定位置创建评论并 @lingma-agents。例如:
-
@lingma-agents 分析这个方法的性能瓶颈并提供优化建议。
-
@lingma-agents 对这个方法生成优化代码。
📜 在讨论中提问
在任何讨论中 @lingma-agents 来获取帮助。例如:
-
@lingma-agents 请总结上述讨论并提出解决方案。
-
@lingma-agents 请根据讨论内容生成优化代码。
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2460 +/- ##
===========================================
+ Coverage 35.91% 46.04% +10.13%
===========================================
Files 69 81 +12
Lines 11576 13020 +1444
===========================================
+ Hits 4157 5995 +1838
+ Misses 7104 6680 -424
- Partials 315 345 +30 🚀 New features to boost your workflow:
|
johnlanni
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…to the rest api server (alibaba#2460)
Ⅰ. Describe what this PR did
Modify the
message-endpoint-sending logic in the mcp-session plugin under the go-filter module.Ⅱ. Does this pull request fix one issue?
fixes #2368 fixes #2421
Ⅲ. Why don't you add test cases (unit test/integration test)?
I've done the test case below:
Started a sse server in the backend
It will print the request url to help us see the test result.
Then I start a simple mcp server:
So the final sse url should be
http://localhost/hello/sse.Test Case
with no query param

with one query param

with plenty query params

Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews