fix(安全)★★: 停用（status='disabled'）不拦鉴权 —— 两条路径都能绕过
## 缺口怎么被发现的
给公网 MCP 接入做验证时注册了一个一次性探针身份
（`mcp-wan-probe`），收尾执行 `UPDATE agents SET status='disabled'`，
本以为凭证就此失效。实测：
POST /api/v1/mcp（带该凭证） → 200
tools/call send_mail → 已发送（真的发出去了）
## 根因
`SetAgentDisabled` 这个 API 存在、返回成功，但 **status 只在投递方向被检查**
（`EnsureAgentDeliverable`，repo.go:209）。两条鉴权路径都不查：
VerifyAgent(ctx, name, secret) 选了 status 却从不判断 ← 缺口
VerifyAgentKey(ctx, token) 拿到名字直接返回 ← 同一个缺口，更严重
第二条要紧：Bearer key_token 正是 `middleware/auth.go` 注释里标「推荐」的
路径，官方建议用它 —— 它因此成了绕过停用的最短路。
缺口形状是「接口存在、返回成功、但只做了一半」，比没有这个接口更危险：
运维会以为停用已经生效。实测语义完全反了 ——
**停用只挡住了「别人给它发信」，没挡住「它自己发信」**。
## 修法
停用 ⇒ **拿不到 Agent 身份** ⇒ `AgentAuth` 直接 401，后续 handler 根本不执行。
这比「拿到身份后在某个业务分支拒绝」强：后者会让列表类 API 仍泄露身份存在。
两个细节：
* 返回 `sql.ErrNoRows`（而非自定义错误）⇒ 与「凭证不存在」**不可区分**，
否则可用该接口枚举出哪些名字是真实 Agent。判据里有专测这一格。
* status 检查放在 `wsJSON` 解析**之前** ⇒ 停用身份不会被解析出工作区，
那会让调用方以为它还「活着」。
## 判据（4 格）
**其中「反向对照」这格最关键**：`status='online'/'active'/''` 必须照常通过。
没有它，一个「永远返回 ErrNoRows」的 `VerifyAgent` 也是全绿的 ——
而那会让**所有** agent 都登不上，是比原缺口更大的事故。
**变异验证**：
撤掉 VerifyAgent 的 status 检查 → 3 格红 ✓
撤掉 VerifyAgentKey 的 status 检查 → 1 格红 ✓
全量 14 包绿。
This commit is contained in:
168
server/internal/repo/disabled_auth_test.go
Normal file
168
server/internal/repo/disabled_auth_test.go
Normal file
@ -0,0 +1,168 @@
|
||||
package repo
|
||||
|
||||
/*
|
||||
停用必须拦住**鉴权**，不只是拦住「别人给它发信」（2026-10-03）。
|
||||
|
||||
# 这条缺口是怎么被发现的
|
||||
|
||||
要做公网 MCP 接入验证，注册了一个一次性探针身份
|
||||
（`mcp-wan-probe`），收尾时 `UPDATE agents SET status='disabled'`。
|
||||
本以为凭证就此失效，实测：
|
||||
|
||||
POST /api/v1/mcp（带该凭证） → 200
|
||||
tools/call send_mail → 已发送（真的发出去了）
|
||||
|
||||
`SetAgentDisabled` 这个 API 存在、也返回成功，但**只挡住了投递方向**
|
||||
（`EnsureAgentDeliverable` 里查 status），鉴权两条路径都不查：
|
||||
|
||||
VerifyAgent(ctx, name, secret) 选了 status 却从不判断 ← 缺口
|
||||
VerifyAgentKey(ctx, token) 拿到名字直接返回 ← 同一个缺口，更严重
|
||||
|
||||
缺口形状与这轮反复出现的那类一致：**接口存在、返回成功、但只做了一半**。
|
||||
比「没有这个接口」更危险 —— 运维会以为停用已经生效。
|
||||
|
||||
`key_token` 那条尤其要紧：它是 `middleware/auth.go` 注释里标「推荐」的
|
||||
路径，官方建议用它，于是它成了绕过停用的最短路。
|
||||
|
||||
# 判据要点
|
||||
|
||||
停用后必须**拿不到 Agent 身份**（AgentAuth 直接 401），而不是拿到身份后
|
||||
在某个业务分支里拒绝 —— 后者会让「列表类」API 仍然泄露该身份的存在。
|
||||
返回 `sql.ErrNoRows`（而非自定义错误）是为了与「凭证不存在」不可区分。
|
||||
*/
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"errors"
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
"github.com/agentmail/gateway/internal/db"
|
||||
"github.com/agentmail/gateway/internal/models"
|
||||
)
|
||||
|
||||
func setupDisabledAgent(t *testing.T, name string, status string) {
|
||||
t.Helper()
|
||||
db.Close()
|
||||
if err := db.Connect(context.Background(), "sqlite://"+t.TempDir()+"/disabled.db"); err != nil {
|
||||
t.Fatalf("连接测试库: %v", err)
|
||||
}
|
||||
if err := db.Migrate(context.Background()); err != nil {
|
||||
t.Fatalf("迁移测试库: %v", err)
|
||||
}
|
||||
t.Cleanup(db.Close)
|
||||
if _, err := db.DB.ExecContext(context.Background(),
|
||||
`INSERT INTO agents (agent_name, secret, platform, status, default_rounds)
|
||||
VALUES ($1, 'sec', 'test', $2, 50)`, name, status); err != nil {
|
||||
t.Fatalf("建 Agent: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// ★ 主判据：停用 ⇒ 两条鉴权路径都必须失败。
|
||||
|
||||
func TestDisabledAgentFailsAuth(t *testing.T) {
|
||||
setupDisabledAgent(t, "stopped-agent", "disabled")
|
||||
|
||||
// 路径①name/secret
|
||||
if _, err := VerifyAgent(context.Background(), "stopped-agent", "sec"); err == nil {
|
||||
t.Error("★ VerifyAgent 接受了已停用的身份 ⇒ 停用形同虚设，" +
|
||||
"它照样能用凭证调任何 API（含 send_mail）")
|
||||
} else if !errors.Is(err, sql.ErrNoRows) {
|
||||
t.Errorf("应返回 sql.ErrNoRows（与凭证不存在不可区分），实际 %v", err)
|
||||
}
|
||||
|
||||
// 路径② Bearer key_token —— 注释里标「推荐」的那条
|
||||
token := seedKeyFor(t, "stopped-agent")
|
||||
name, err := VerifyAgentKey(context.Background(), token)
|
||||
if err == nil && name != "" {
|
||||
t.Error("★ VerifyAgentKey 接受了已停用身份的令牌 ⇒ Bearer 这条" +
|
||||
"（推荐路径）能绕过停用")
|
||||
}
|
||||
}
|
||||
|
||||
// ★ 反向对照：status='online'（或任何非 disabled 值）必须照常通过。
|
||||
// 没有这一格，一个「永远返回 ErrNoRows」的 VerifyAgent 也是全绿的，
|
||||
// 而那会让**所有** agent 都登不上 —— 一个比原缺口更大的事故。
|
||||
|
||||
func TestNonDisabledAgentStillAuthenticates(t *testing.T) {
|
||||
for _, st := range []string{"online", "active", ""} {
|
||||
setupDisabledAgent(t, "live-agent", st)
|
||||
a, err := VerifyAgent(context.Background(), "live-agent", "sec")
|
||||
if err != nil {
|
||||
t.Errorf("★ status=%q 的在跑身份被拒了 —— 判据只能拦 disabled，%v", st, err)
|
||||
continue
|
||||
}
|
||||
if a.Name != "live-agent" {
|
||||
t.Errorf("返回了错误身份: %q", a.Name)
|
||||
}
|
||||
token := seedKeyFor(t, "live-agent")
|
||||
name, err := VerifyAgentKey(context.Background(), token)
|
||||
if err != nil || name != "live-agent" {
|
||||
t.Errorf("★ status=%q 的在跑身份过不了 Bearer 路径: name=%q err=%v", st, name, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ★ 停用不能泄露「这个身份存在」：错误必须与凭证错配时完全一致。
|
||||
// 若停用返回自定义错误，攻击者可用它枚举出「哪些名字是真实 Agent」。
|
||||
|
||||
func TestDisabledIndistinguishableFromWrongSecret(t *testing.T) {
|
||||
setupDisabledAgent(t, "stopped-agent", "disabled")
|
||||
|
||||
_, errDisabled := VerifyAgent(context.Background(), "stopped-agent", "sec")
|
||||
_, errWrongSecret := VerifyAgent(context.Background(), "stopped-agent", "wrong-secret")
|
||||
_, errNoSuchAgent := VerifyAgent(context.Background(), "nobody", "sec")
|
||||
|
||||
for _, c := range []struct {
|
||||
name string
|
||||
err error
|
||||
}{
|
||||
{"凭证错配", errWrongSecret},
|
||||
{"身份不存在", errNoSuchAgent},
|
||||
} {
|
||||
if !errors.Is(errDisabled, c.err) {
|
||||
t.Errorf("★ 停用(%v) 必须与「%s」(%v) 返回同一个错误，"+
|
||||
"否则可用于枚举真实 Agent 名", errDisabled, c.name, c.err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ★ 停用后不得解析出工作区：那会让调用方以为身份还「活着」。
|
||||
// （实现上把 status 检查放在 wsJSON 解析之前，这里钉住那个顺序。）
|
||||
|
||||
func TestDisabledAgentDoesNotExposeWorkspaces(t *testing.T) {
|
||||
setupDisabledAgent(t, "stopped-agent", "disabled")
|
||||
dbMustExec(t, `UPDATE agents SET workspaces = '[{"path":"/secret/project"}]' WHERE agent_name='stopped-agent'`)
|
||||
|
||||
a, err := VerifyAgent(context.Background(), "stopped-agent", "sec")
|
||||
if err == nil && a != nil && len(a.Workspaces) > 0 {
|
||||
t.Errorf("★ 停用身份仍被解析出工作区 %+v ⇒ 调用方可能以为它还活着",
|
||||
a.Workspaces)
|
||||
}
|
||||
}
|
||||
|
||||
// seedKeyFor 给某 Agent 建一张永久 key_token，返回 token。
|
||||
// 用真实的表结构与 key_type 取值（models.KeyPermanent），别自造。
|
||||
func seedKeyFor(t *testing.T, agentName string) string {
|
||||
t.Helper()
|
||||
token := "tok-" + agentName + "-0123456789abcdef"
|
||||
if _, err := db.DB.ExecContext(context.Background(),
|
||||
`INSERT INTO agent_keys (key_token, agent_name, key_type, label)
|
||||
VALUES ($1, $2, $3, 'test')`,
|
||||
token, agentName, models.KeyPermanent); err != nil {
|
||||
t.Fatalf("建 key_token: %v", err)
|
||||
}
|
||||
return token
|
||||
}
|
||||
|
||||
// dbMustExec 让判据里的 SQL 失败时直接报出来，而不是静默跳过
|
||||
// （踩过：断言语句自己没跑，判据永远绿）。
|
||||
func dbMustExec(t *testing.T, sqlText string) {
|
||||
t.Helper()
|
||||
if _, err := db.DB.ExecContext(context.Background(), sqlText); err != nil {
|
||||
t.Fatalf("执行 %s: %v", sqlText, err)
|
||||
}
|
||||
}
|
||||
|
||||
var _ = fmt.Sprintf
|
||||
@ -223,6 +223,24 @@ func VerifyAgentKey(ctx context.Context, token string) (string, error) {
|
||||
if agentName == nil {
|
||||
return "", nil
|
||||
}
|
||||
// ★ 2026-10-03 修（与 VerifyAgent 同一处缺口）：密钥本身可能没过期，
|
||||
// 但**它所属的 Agent 可能已被停用**。原来这里直接返回名字，
|
||||
// 于是 Bearer 令牌能绕过「停用 = 撤销一切权限」——
|
||||
// 而 key_token 正是注释里标「推荐」的那条鉴权路径。
|
||||
//
|
||||
// 停用后返回空身份（不是错误）：调用方看到的是「这个令牌不是某个
|
||||
// Agent 的令牌」，与令牌不存在不可区分，不泄露身份是否存在。
|
||||
var status string
|
||||
if err := db.DB.QueryRowContext(ctx,
|
||||
`SELECT status FROM agents WHERE agent_name = $1`, *agentName).Scan(&status); err != nil {
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
return "", nil
|
||||
}
|
||||
return "", err
|
||||
}
|
||||
if status == "disabled" {
|
||||
return "", nil
|
||||
}
|
||||
return *agentName, nil
|
||||
}
|
||||
|
||||
|
||||
@ -311,6 +311,19 @@ func ListAgents(ctx context.Context, statusFilter string) ([]models.Agent, error
|
||||
return agents, nil
|
||||
}
|
||||
|
||||
// VerifyAgent 校验 name+secret 并返回该 Agent。
|
||||
//
|
||||
// ★ 2026-10-03 修：停用（status='disabled'）原本**只在投递时**生效
|
||||
// （见 EnsureAgentDeliverable），鉴权这条路径完全不查 —— 于是
|
||||
// `SetAgentDisabled` 看起来在工作，实际只挡住了「别人给它发信」，
|
||||
// 它自己照样能用凭证调任何 API：
|
||||
//
|
||||
// UPDATE agents SET status='disabled' WHERE agent_name='mcp-wan-probe'
|
||||
// POST /api/v1/mcp（带该凭证）→ 200，send_mail 真的发出去了
|
||||
//
|
||||
// 停用的语义是**撤销该身份的一切权限**，不只是「别人不能寄给它」。
|
||||
// 拿不到 Agent 身份 ⇒ AgentAuth 直接 401，后面所有 handler 都不会执行，
|
||||
// 与其它凭证错误的处理完全一致（不泄露「这个身份存在但被停了」）。
|
||||
func VerifyAgent(ctx context.Context, name, secret string) (*models.Agent, error) {
|
||||
var a models.Agent
|
||||
var wsJSON []byte
|
||||
@ -322,6 +335,11 @@ func VerifyAgent(ctx context.Context, name, secret string) (*models.Agent, error
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
// 早于 wsJSON 的解析：停用身份不该被解析出工作区（那会让调用方
|
||||
// 以为它还「活着」）。
|
||||
if a.Status == "disabled" {
|
||||
return nil, sql.ErrNoRows
|
||||
}
|
||||
if wsJSON != nil {
|
||||
json.Unmarshal(wsJSON, &a.Workspaces)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user