JianFeeeee
4e32dd3145
# 漏洞
`POST /permission/request` 的 `to` 字段由 Agent 自由填写，服务端只检查
「这个名字是不是一个合法的人类用户」：
decider := req.To
if isHuman, _ := repo.IsHumanUser(ctx, decider); !isHuman { …回落… }
于是任何 Agent 都能把「是否允许执行 bash」这类危险操作的审批丢给**任意一个
与这条任务无关的人**（例如管理员）。被点名的人看到一封没有上下文的待办，
只能凭猜点头或拒绝。
这跟同一份代码里的另一段注释直接冲突。那段在论证为什么不把权限转给管理员：
管理员对这条 Agent 链的上下文一无所知，既不知道这个 bash 命令在做什么，
也不知道拒绝后 Agent 该怎么绕过去。
这个理由同样适用于「Agent 自己点名一个无关的人」—— 而且更弱：至少管理员还能
查日志，一个随机被点名的用户连从哪查都不知道。两处都指向同一条规则：
**权限应当追溯到最初分配任务的人**，也就是这条线索上的人。
这是静态审计发现的四项之一。当时三桥实测都不传 `to`，所以是潜在面而非活跃
漏洞 —— 但 `to` 是公开的 Agent API 字段，第三方插件照着文档填就会踩上。
# 修法
新增 `repo.IsHumanOnSessionThread(ctx, sessionID, name)`：人类身份 **且**
（会话 owner 或在这条会话的某封邮件里出现过）。
两个来源缺一不可，各有实测场景：
- **只要参与方**会漏掉「会话由 Agent 建立、owner 由平台指派」的会话 ——
那种 owner 可能一封邮件都没收发过，只看邮件会把合法 owner 判成外人，
于是每次审批都回落到线索上随便一个人类。
- **只要 owner** 会漏掉「人在别人的会话里被抄送进来说了话」这种正常协作。
采信与否的处置是**丢弃提示而不是报错**：`to` 只是一个偏好，丢弃后常规解析仍会
给出一个合法人类（owner 或线索上最近的人），实在没有就是既有的 409 —— 无论哪条
分支，都不会把审批送到错的人手上。硬失败则会让 Agent 一次乐观的提示断掉整个
任务，而它并没有做错什么。因为丢弃是静默的，所以**必须留下日志**：
[permission] 忽略不属于本线索的决策人 "jianf"（会话 …, 由 pi 指定）—— 改走常规解析
# 测试
补了这条路径此前**完全缺失**的两层覆盖（审计发现：决策路径
RequestPermission/DecidePermission/ListPendingPermissions 都没有测试）：
- `repo/threadhuman_test.go`：白名单的六种输入（线索上发信/收信的人类、没发过
邮件的 owner、线索外的存在用户、不存在的名字、空串、抄送方），每条都写清
为什么期望这个结果。
- `handler/permission_request_test.go`：**真实 HTTP 层**跑 `RequestPermission`，
断言响应里的 decider 与库里那封权限邮件的 to_name。repo 层 helper 正确但
handler 漏调一次，漏洞就会回来，所以必须有端到端这一层。含纯 Agent 链的
fail-closed 断言（不得退回管理员）。
# 验证
- `go test ./... -count=1` 全绿；`go vet` 干净；`gofmt` 差异行数与改动前完全
相同（8 行，既有的一处空行）—— 即本次改动零新增格式问题
- 真机（workspace 档会话，owner=gui-lab，线索参与者 gui-lab+pi）：
- `to=jianf`（线索外人类）→ decider=gui-lab，库中 to_name=gui-lab，
日志有忽略记录
- `to=gui-lab`（线索内）→ 采纳
- `to=pi`（Agent）→ 忽略，回落 owner
- 已部署（redeploy-gateway.sh 自动项全绿）
107 lines
4.2 KiB
Go
107 lines
4.2 KiB
Go
package repo
|
||
|
||
import (
|
||
"context"
|
||
"testing"
|
||
|
||
"github.com/agentmail/gateway/internal/db"
|
||
"github.com/agentmail/gateway/internal/models"
|
||
"github.com/google/uuid"
|
||
)
|
||
|
||
// 权限询问的决策人白名单。
|
||
//
|
||
// `/permission/request` 的 `to` 字段由 Agent 自由填写，原来只要它是合法的人类
|
||
// 用户名就直接采用 —— 于是一个 Agent 可以把「是否允许执行 bash」丢给任意一个
|
||
// 与这条任务无关的人。这与 handler 里那段 409 的理由直接冲突：既然「管理员对
|
||
// 这条 Agent 链的上下文一无所知」是拒绝转给管理员的依据，那 Agent 自己点名一个
|
||
// 无关的人同样不成立。权限应当追溯到最初分配任务的人，也就是这条线索上的人。
|
||
|
||
func mustHuman(t *testing.T, username string) uuid.UUID {
|
||
t.Helper()
|
||
id := uuid.New()
|
||
if _, err := db.DB.ExecContext(context.Background(),
|
||
`INSERT INTO users (user_id, username, display_name, password_hash, role)
|
||
VALUES ($1, $2, $3, 'x', 'user')`,
|
||
id, username, username); err != nil {
|
||
t.Fatalf("建用户 %s: %v", username, err)
|
||
}
|
||
return id
|
||
}
|
||
|
||
func TestIsHumanOnSessionThread(t *testing.T) {
|
||
setupTestDB(t)
|
||
ctx := context.Background()
|
||
|
||
sid, _ := CreateSession(ctx, nil, "pi", "权限决策人", "/home/program/agentmail")
|
||
ownerID := mustHuman(t, "laoban")
|
||
mustHuman(t, "dispatcher")
|
||
mustHuman(t, "reviewer")
|
||
mustHuman(t, "passerby") // 存在，但与这条线索无关
|
||
|
||
// 会话里有人类 dispatcher / reviewer 往来；owner 由平台指派、从未收发过邮件。
|
||
mustMail(t, sid, "dispatcher", "", "reviewer", "/home/program/agentmail", nil)
|
||
if err := SetSessionOwner(ctx, sid, ownerID); err != nil {
|
||
t.Fatalf("指派 owner: %v", err)
|
||
}
|
||
|
||
cases := []struct {
|
||
name string
|
||
who string
|
||
want bool
|
||
why string
|
||
}{
|
||
{"线索上发过信的人类", "dispatcher", true,
|
||
"发件人就是最初分配任务的人，权限第一该问他"},
|
||
{"线索上收过信的人类", "reviewer", true,
|
||
"人类收件人也是参与者"},
|
||
{"线索上的 Agent 不是决策人", "pi", false,
|
||
"出现在邮件里只是必要条件；只有人类用户才能在 WebUI 决策"},
|
||
{"会话 owner（没发过邮件）", "laoban", true,
|
||
"owner 与参与方是两处来源，缺一不可：会话由 Agent 建立、owner 由平台指派时，只看邮件会把合法 owner 判成外人"},
|
||
{"存在但与线索无关的人", "passerby", false,
|
||
"这就是要堵的那个洞：Agent 不能把审批丢给与任务无关的人"},
|
||
{"根本不存在的人", "nobody", false,
|
||
"白名单从正面判定，不存在的名字自然不通过"},
|
||
{"空串", "", false,
|
||
"空串的语义是「没指定」，走常规解析，不该被当成线索上的人"},
|
||
}
|
||
|
||
for _, c := range cases {
|
||
if got := IsHumanOnSessionThread(ctx, sid, c.who); got != c.want {
|
||
t.Errorf("%s：IsHumanOnSessionThread(%q) = %v，应为 %v —— %s",
|
||
c.name, c.who, got, c.want, c.why)
|
||
}
|
||
}
|
||
}
|
||
|
||
// 抄送方也算线索上的人：一次抄送就把人带进了这条线索。
|
||
func TestIsHumanOnSessionThreadCountsCC(t *testing.T) {
|
||
setupTestDB(t)
|
||
ctx := context.Background()
|
||
|
||
sid, _ := CreateSession(ctx, nil, "pi", "抄送也算", "/home/program/agentmail")
|
||
mustHuman(t, "dispatcher")
|
||
mustHuman(t, "observer")
|
||
mustMail(t, sid, "dispatcher", "", "pi", "/home/program/agentmail", nil)
|
||
|
||
// 抄送方由第二封信带入 —— 一次抄送就把人带进了这条线索。
|
||
cc := []models.Address{{Name: "observer", Path: "/home", Session: "new", Raw: "observer@/home.new"}}
|
||
if _, err := CreateMail(ctx, sid, nil, "dispatcher", "/home/program/agentmail",
|
||
"pi", "/home/program/agentmail", "带抄送", "正文", cc); err != nil {
|
||
t.Fatalf("建带抄送的邮件: %v", err)
|
||
}
|
||
|
||
if !IsHumanOnSessionThread(ctx, sid, "observer") {
|
||
t.Error("人类抄送方应算作线索上的人：一次抄送就把他带进了这条线索")
|
||
}
|
||
}
|
||
|
||
// 会话不存在时不得 panic，且必须返回 false（fail closed）。
|
||
func TestIsHumanOnSessionThreadUnknownSession(t *testing.T) {
|
||
setupTestDB(t)
|
||
if IsHumanOnSessionThread(context.Background(), uuid.New(), "admin") {
|
||
t.Error("会话不存在时应返回 false（fail closed），不能因为查不到就往放行方向倒")
|
||
}
|
||
}
|