fix(mcp)★★: 未声明参数不再被静默吞掉 —— 模型拿到假成功
## 实测经过（全面测 MCP 时发现） `connect_to_server(gateway_url="http://evil.example", agent_key="x")` 返回「已连接」—— 看起来像模型改了网关坐标。 查下去坐标**改不了**：身份只从 AgentAuth 的 context 取，参数被忽略 （schema 是 `properties: {}`）。**所以不是安全问题。** 但它是另一种毛病：**误传/写错的参数被静默吞掉**，模型收到「成功」后 不会知道自己传的东西根本没生效。实测发现 **11 个工具全部**没设 `additionalProperties: false`。 ## 两道都补 1. **schema 层**：`objSchema` 加 `additionalProperties: false` —— 一处改动覆盖全部 11 个工具。 2. **运行时**：`unknownArgs` / `declaredArgs` 在 `tools/call` 里再拒一次。 schema 只是**声明**，是否生效取决于宿主是否遵守 ⇒ 这一道不依赖宿主。 报错文案点名参数并列出可用参数，让模型能自我纠正： 工具 connect_to_server 没有这些参数：agent_key、gateway_url（可用：（无参数）） `unknownArgs` 只在 schema 自己声明了 `additionalProperties:false` 时才判 —— 否则等于替宿主做主，把合法的透传参数也拒了（有判据钉住）。 ## 判据自己假绿了一次 变异 `if false && len(bad) > 0`（短路调用点）→ **0 红**。 因为 `TestUnknownArgsOnlyWhenSchemaOptsIn` 测的是 `unknownArgs` **这个函数**， 而**函数正确 ≠ 它被接上了**。这是本轮第五次栽在「形状判据测不到接线」上。 ⇒ 补了端到端一格：从 HTTP 进去，看客户端实际收到什么（并断言绝不能是 「已连接」—— 那正是要修的假成功）。重测同一变异立刻转红。 判据走真注册表（`NewTools().RegisterAll`）而非复制一份清单 —— 复制的那份会与真实工具面漂移，而漂移方向恰好是「新工具没被判据覆盖」。 ## 顺带纠正上一轮的一个错误结论 上轮我说 `read_mail` 与 `read_thread` 拒绝措辞不一致是缺陷。**那是错的。** `agent_discovery.go:414` 明确丢掉 reason，注释写着「拒绝原因在 403 文案上 合流成同一句，反而少泄一点信息」—— 那是**刻意的安全取舍**，不告诉调用方 到底是跨工作区还是没参与过（两者指向同一个可枚举的信息）。 而 `read_mail` 那句话是用户 2026-09-15 亲自订正过的（原注释：旧文案让 zcode 推断出「那五位 agent 不存在」，把错误结论写进了给人看的信里）。 ⇒ **不改**。把刻意的安全设计当缺陷「修」掉，比留着更糟。 14 包全绿。
This commit is contained in:
187
server/internal/mcp/argcheck_test.go
Normal file
187
server/internal/mcp/argcheck_test.go
Normal file
@ -0,0 +1,187 @@
|
||||
package mcp
|
||||
|
||||
/*
|
||||
未声明参数必须被拒绝（2026-10-04 实测驱动）。
|
||||
|
||||
★ 实测经过：
|
||||
`connect_to_server(gateway_url="http://evil.example", agent_key="x")`
|
||||
返回「已连接」。查下去**坐标改不了**（身份只从 AgentAuth 的 context 取，
|
||||
参数被忽略）—— 所以不是安全问题。
|
||||
|
||||
但它是另一种毛病：**误传/写错的参数被静默吞掉**，模型拿到一个假成功，
|
||||
不会知道自己传的东西根本没生效。实测 11 个工具**全部**没设
|
||||
`additionalProperties: false`。
|
||||
|
||||
⇒ 两道：
|
||||
1. schema 层 `additionalProperties: false`（声明）
|
||||
2. tools/call 运行时校验（不依赖宿主是否遵守 schema）
|
||||
*/
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"sort"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestEveryToolDeclaresAdditionalPropertiesFalse(t *testing.T) {
|
||||
for name, tl := range allTools() {
|
||||
sch := tl.Schema().InputSchema
|
||||
if sch == nil {
|
||||
t.Errorf("工具 %s 没有 inputSchema", name)
|
||||
continue
|
||||
}
|
||||
allowed, ok := sch["additionalProperties"].(bool)
|
||||
if !ok || allowed {
|
||||
t.Errorf("★ 工具 %s 的 schema 未设 additionalProperties:false ⇒ "+
|
||||
"误传参数会被静默吞掉，模型拿到假成功", name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestConnectToServerRejectsCoordinateOverride(t *testing.T) {
|
||||
// ★ 这正是实测里被吞掉的那次调用：模型以为改了网关坐标。
|
||||
sch := toolByName(t, "connect_to_server").Schema().InputSchema
|
||||
args := map[string]any{
|
||||
"gateway_url": "http://evil.example",
|
||||
"agent_key": "x",
|
||||
}
|
||||
bad := unknownArgs(sch, args)
|
||||
sort.Strings(bad)
|
||||
if len(bad) != 2 || bad[0] != "agent_key" || bad[1] != "gateway_url" {
|
||||
t.Fatalf("★ 应识别出 2 个未声明参数，实际 %v", bad)
|
||||
}
|
||||
if got := declaredArgs(sch); got != "（无参数）" {
|
||||
t.Errorf("connect_to_server 无参，报错文案应说「（无参数）」，实际 %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUnknownArgsOnlyWhenSchemaOptsIn(t *testing.T) {
|
||||
// schema 没声明 additionalProperties:false 时**不得**判 ——
|
||||
// 否则等于替宿主做主，把合法的透传参数也拒了。
|
||||
permissive := map[string]any{
|
||||
"type": "object",
|
||||
"properties": map[string]any{"a": map[string]any{"type": "string"}},
|
||||
}
|
||||
if bad := unknownArgs(permissive, map[string]any{"zzz": 1}); len(bad) != 0 {
|
||||
t.Errorf("schema 未 opt-in 时不该判未声明参数，实际 %v", bad)
|
||||
}
|
||||
// opt-in 后才判
|
||||
strict := map[string]any{
|
||||
"type": "object",
|
||||
"properties": map[string]any{"a": map[string]any{"type": "string"}},
|
||||
"additionalProperties": false,
|
||||
}
|
||||
if bad := unknownArgs(strict, map[string]any{"zzz": 1, "a": "x"}); len(bad) != 1 || bad[0] != "zzz" {
|
||||
t.Errorf("opt-in 后应只报 zzz，实际 %v", bad)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDeclaredArgsListsAllSorted(t *testing.T) {
|
||||
sch := objSchema(map[string]any{
|
||||
"zeta": strProp("z"), "alpha": strProp("a"), "mid": strProp("m"),
|
||||
})
|
||||
if got, want := declaredArgs(sch), "alpha、mid、zeta"; got != want {
|
||||
t.Errorf("declaredArgs 应按字典序列出全部，实际 %q（期望 %q）", got, want)
|
||||
}
|
||||
// 空 schema 要说「（无参数）」而不是空串 ——
|
||||
// 报错文案里出现空括号会让模型以为还有别的参数没列出来。
|
||||
if got := declaredArgs(objSchema(map[string]any{})); !strings.Contains(got, "无参数") {
|
||||
t.Errorf("空 schema 的 declaredArgs 应说明无参数，实际 %q", got)
|
||||
}
|
||||
if got := declaredArgs(nil); got != "（无参数）" {
|
||||
t.Errorf("nil schema 不该 panic，实际 %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
// 确认 schema 能正常序列化（additionalProperties 不引入非 JSON 值）
|
||||
func TestSchemaStillMarshalsCleanly(t *testing.T) {
|
||||
b, err := json.Marshal(toolByName(t, "send_mail").Schema().InputSchema)
|
||||
if err != nil {
|
||||
t.Fatalf("序列化失败: %v", err)
|
||||
}
|
||||
if !strings.Contains(string(b), `"additionalProperties":false`) {
|
||||
t.Errorf("序列化后应带 additionalProperties:false，实际 %s", b)
|
||||
}
|
||||
}
|
||||
|
||||
// allTools 返回全部已注册工具（名字 → 工具）。
|
||||
//
|
||||
// 直接调注册表而不是复制一份清单：复制的那份会与真实工具面漂移，
|
||||
// 而漂移的方向恰好是「新工具没被判据覆盖」。
|
||||
func allTools() map[string]Tool {
|
||||
s := NewServer(nil)
|
||||
NewTools().RegisterAll(s) // ★ 必须走真注册：复制一份清单会与真实工具面漂移，
|
||||
// 而漂移方向恰好是「新工具没被判据覆盖」。
|
||||
m := map[string]Tool{}
|
||||
for _, n := range s.toolNames() {
|
||||
m[n] = s.tools[n]
|
||||
}
|
||||
return m
|
||||
}
|
||||
|
||||
func toolByName(t *testing.T, name string) Tool {
|
||||
t.Helper()
|
||||
tl := allTools()
|
||||
tl2, ok := tl[name]
|
||||
if !ok {
|
||||
t.Fatalf("找不到工具 %s（已注册：%v）", name, keysOf(tl))
|
||||
}
|
||||
return tl2
|
||||
}
|
||||
|
||||
func keysOf(m map[string]Tool) []string {
|
||||
out := make([]string, 0, len(m))
|
||||
for k := range m {
|
||||
out = append(out, k)
|
||||
}
|
||||
sort.Strings(out)
|
||||
return out
|
||||
}
|
||||
|
||||
/*
|
||||
★ 端到端一格：tools/call **真的**拒绝未声明参数。
|
||||
|
||||
为什么必须单独一格（2026-10-04）：
|
||||
上面 `TestUnknownArgsOnlyWhenSchemaOptsIn` 测的是 `unknownArgs` 这个**函数**，
|
||||
而变异 `if false && len(bad) > 0`（把调用点短路）时它**依然全绿** ——
|
||||
判据测的是被测函数的正确性，不是**它被接上了**。
|
||||
|
||||
⇒ 形状/单函数判据防不住「函数正确但没调用」。
|
||||
|
||||
这一格从 HTTP 进去，看客户端实际收到什么。
|
||||
*/
|
||||
func TestToolsCallRejectsUnknownArgEndToEnd(t *testing.T) {
|
||||
s := NewServer(nil)
|
||||
NewTools().RegisterAll(s)
|
||||
|
||||
body := `{"jsonrpc":"2.0","id":1,"method":"tools/call","params":` +
|
||||
`{"name":"connect_to_server","arguments":{"gateway_url":"http://evil.example"}}}`
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/mcp", strings.NewReader(body))
|
||||
s.HandleHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("应 200（JSON-RPC 层），实际 %d：%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
out := rec.Body.String()
|
||||
if !strings.Contains(out, "gateway_url") || !strings.Contains(out, "没有这些参数") {
|
||||
t.Errorf("★ tools/call 应点名 gateway_url 是未声明参数，实际返回：%s", out)
|
||||
}
|
||||
// ★ 绝不能是「已连接」—— 那正是这次要修的假成功。
|
||||
if strings.Contains(out, "已连接") {
|
||||
t.Errorf("★ 回成了假成功：%s", out)
|
||||
}
|
||||
|
||||
// 合法调用（无参）仍须成功，别把门关死。
|
||||
okBody := `{"jsonrpc":"2.0","id":2,"method":"tools/call","params":` +
|
||||
`{"name":"connect_to_server","arguments":{}}}`
|
||||
rec2 := httptest.NewRecorder()
|
||||
req2 := httptest.NewRequest(http.MethodPost, "/api/v1/mcp", strings.NewReader(okBody))
|
||||
s.HandleHTTP(rec2, req2)
|
||||
if strings.Contains(rec2.Body.String(), "没有这些参数") {
|
||||
t.Errorf("无参调用不该被拒：%s", rec2.Body.String())
|
||||
}
|
||||
}
|
||||
@ -37,6 +37,7 @@ import (
|
||||
"io"
|
||||
"log"
|
||||
"net/http"
|
||||
"sort"
|
||||
"strings"
|
||||
)
|
||||
|
||||
@ -312,6 +313,18 @@ func (s *Server) handleMessage(msg *rpcMessage, r *http.Request) *rpcMessage {
|
||||
if args == nil {
|
||||
args = map[string]any{} // 缺 arguments 当空对象，不抛错
|
||||
}
|
||||
// ★ 运行时再拒一次未声明参数。
|
||||
//
|
||||
// schema 里的 `additionalProperties: false` 只是**声明** ——
|
||||
// 是否生效取决于宿主是否遵守。而实测中「参数被静默吞掉、模型拿到
|
||||
// 假成功」正是最坏的一种失败：模型不会知道自己传的东西没生效。
|
||||
// 这一道不依赖宿主。
|
||||
sch := t.Schema().InputSchema
|
||||
if bad := unknownArgs(sch, args); len(bad) > 0 {
|
||||
return failure(id, rpcInvalidParams,
|
||||
"工具 %s 没有这些参数：%s（可用：%s）",
|
||||
p.Name, strings.Join(bad, "、"), declaredArgs(sch))
|
||||
}
|
||||
text, err := t.Run(r.Context(), args)
|
||||
if err != nil {
|
||||
s.log.Printf("[mcp] 工具 %s 失败：%v", p.Name, err)
|
||||
@ -359,3 +372,40 @@ func writeJSON(w http.ResponseWriter, status int, v any) {
|
||||
w.WriteHeader(status)
|
||||
_ = json.NewEncoder(w).Encode(v)
|
||||
}
|
||||
|
||||
// unknownArgs 返回 args 里**未在 schema 声明**的键（已排序，保证报错稳定）。
|
||||
//
|
||||
// ★ 只在 schema 自己声明了 `additionalProperties: false` 时才判 ——
|
||||
//
|
||||
// 否则等于替宿主做主，把合法的透传参数也拒了。
|
||||
func unknownArgs(schema map[string]any, args map[string]any) []string {
|
||||
if schema == nil {
|
||||
return nil
|
||||
}
|
||||
if allowed, ok := schema["additionalProperties"].(bool); !ok || allowed {
|
||||
return nil
|
||||
}
|
||||
declared, _ := schema["properties"].(map[string]any)
|
||||
bad := make([]string, 0)
|
||||
for k := range args {
|
||||
if _, ok := declared[k]; !ok {
|
||||
bad = append(bad, k)
|
||||
}
|
||||
}
|
||||
sort.Strings(bad)
|
||||
return bad
|
||||
}
|
||||
|
||||
// declaredArgs 列出工具声明过的参数（供报错文案用，避免模型去猜）。
|
||||
func declaredArgs(schema map[string]any) string {
|
||||
declared, _ := schema["properties"].(map[string]any)
|
||||
if len(declared) == 0 {
|
||||
return "（无参数）"
|
||||
}
|
||||
names := make([]string, 0, len(declared))
|
||||
for k := range declared {
|
||||
names = append(names, k)
|
||||
}
|
||||
sort.Strings(names)
|
||||
return strings.Join(names, "、")
|
||||
}
|
||||
|
||||
@ -203,8 +203,29 @@ func withRouteParams(r *http.Request, params map[string]string) *http.Request {
|
||||
}
|
||||
|
||||
// objSchema 造一个 object 类型的入参 schema。
|
||||
/*
|
||||
objSchema 构造工具的 inputSchema。
|
||||
|
||||
★ `additionalProperties: false` 是 2026-10-04 实测后加的，不是洁癖：
|
||||
|
||||
实测 `connect_to_server(gateway_url="http://evil.example", agent_key="x")`
|
||||
返回「已连接」—— 看起来像模型改了网关坐标。查下去它确实改不了
|
||||
（身份只从 AgentAuth 的 context 取，参数被忽略），所以**不是安全问题**。
|
||||
|
||||
但它是另一种毛病：**误传/写错的参数被静默吞掉**，模型收到「成功」后
|
||||
不会知道自己传的东西根本没生效。实测里 11 个工具**全部**没设这个标记。
|
||||
|
||||
⇒ 未声明的参数必须在 schema 层就拒绝，让模型立刻看到
|
||||
「这个工具没有这个参数」而不是拿到一个假的成功。
|
||||
（跨版本宿主仍可能不发 `additionalProperties`，
|
||||
故 tools/call 侧另有一道运行时校验，见 checkToolArgs。）
|
||||
*/
|
||||
func objSchema(props map[string]any, required ...string) map[string]any {
|
||||
s := map[string]any{"type": "object", "properties": props}
|
||||
s := map[string]any{
|
||||
"type": "object",
|
||||
"properties": props,
|
||||
"additionalProperties": false,
|
||||
}
|
||||
if len(required) > 0 {
|
||||
s["required"] = required
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user