Files
MailUI4Agents/server/internal/mcp/argcheck_test.go
JianFeeeee 11654f88b5 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 包全绿。
2026-10-04 17:08:15 +08:00

188 lines
6.6 KiB
Go
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

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())
}
}