fix(安全): 未声明 session_id 不再放行 —— 实测任意 agent 可读全部邮件正文

## 漏洞(亲自实测,不是读码推断)

用 dsh 的密钥、不带任何 session_id,逐个 GET `/api/v1/agent/mail/{id}`:

    20 封别人的信(收件方 pi / homeagent / opencode,分属
    /home/program/TrueAgent 等不同工作区)⇒ **20 封全部 200,拿到完整正文**,0 拒绝。

对照(证明闸本身没坏,只有一个缺口):

    带自己参与的 session_id 读别人的信 ⇒ 403   ← 闸有效
    不带 session_id                    ⇒ 200   ← 漏洞

根因是 `AgentMayReadSession` 的一个分支:`if scope == nil { return true }`。
该函数 2026-09-15 的注释写明「首次接线前的旧语义(未声明 scope)保持放行」——
那是**迁移期妥协**,不是设计。

## 为什么「迁移期」已经结束(实测数据推翻了当初的假设)

当初假设「未接线的桥/脚本/浏览器会走这里,等接完就收口」。而
`[agent-scope]` 警告日志累计 203 次,按调用方拆开:

    homeagent 125 / dsh 40 / pi 37 / opencode 1 / 其它 0

⇒ **202/203 来自四个桥自己**,集中在 `/api/v1/agent/mail/{id}`(pi 24 次)、
读会话参与者、`/mail/{id}/forward`。
不是「少数旧客户端没接线」,而是**主力客户端在裸奔**,而放行恰好把它们全漏过去。

日志抓手已完成使命:它精确指出了「谁还没带」,答案就是所有人。

## 为什么不能靠「补齐调用方」收口

要同时改四个桥(pi 的 `getMailSessionId` 有 `= () => ''` 的默认值,
忘注入就是静默空串 ⇒ 回到裸奔)。**默认放行与默认拒绝的差别就在这里:
前者的失败模式是沉默的。** 任何一处漏了 = 静默越权,且没有任何东西会红。

## 判据

`grep -rln AgentMayReadSession --include=*_test.go` ⇒ 修改前**零覆盖**。
一个决定安全边界的函数没有任何判据,这就是妥协能活到今天的原因。
新增 4 格:未声明须拒 / 声明且相等须放行 / 跨会话须拒 / 判定不随 agentName 变
(后者钉住 2026-09-15 裁定「每个 session 是独立『用户』」,防有人顺手加按 agent 的仲裁)。
两个变异都经得起:恢复放行、reason 改成 handler 不认识的值,各红一格。

reason 复用既有的 `not-your-session`:新增 reason 不同步改 handler 的 switch
就会把 403 变成 500;复用后 canReadSession 的 self 为空时,文案自然表达
「你还没声明自己在哪条会话」。

验证:13 包全绿 + `-race` 干净。
This commit is contained in:
2026-10-02 00:34:13 +08:00
parent b2a0a42c91
commit 15e4fe9203
2 changed files with 131 additions and 34 deletions

View File

@ -54,12 +54,57 @@ func SessionWorkspaceOf(ctx context.Context, id uuid.UUID) string {
//
// 信任边界在**桥**:`session_id` 由 worker 闭包注入(模型改不了),桥是我们的可信组件。
// 这与「一个邮件客户端替它持有的每个账号收发信」是同一种信任:声明自己是哪条会话,
// 就以那条会话的身份行事。首次接线前的旧语义(未声明 scope)保持放行并记警告日志。
// 就以那条会话的身份行事。
//
// ★★ 2026-10-02:未声明 scope **不再放行**(2026-09-15 那句「保持放行」到此作废)。
//
// # 那个妥协的代价(实测,不是推演)
//
// 用 dsh 的密钥、不带任何 session_id,逐个 GET `/api/v1/agent/mail/{id}`:
//
// 20 封别人的信(收件方 pi / homeagent / opencode,跨 /home/program/TrueAgent
// 等不同工作区)⇒ **20 封全部 200,拿到完整正文**,0 拒绝。
//
// 而带上 session_id 时闸是好的(对照实测):
//
// 带自己参与的 session_id 读别人的信 ⇒ 403
// 不带 session_id ⇒ 200(漏洞)
//
// ⇒ 缺口就是下面那个 `if scope == nil { return true }`。
//
// # 为什么「迁移期」已经结束(当初的理由已不成立)
//
// 当时的假设是「未接线的桥/脚本/浏览器会走这里,等接完就收口」。实测:
//
// [agent-scope] 警告日志累计 203 次,其中 **202 次来自四个桥自己**
// (homeagent 125 / dsh 40 / pi 37 / opencode 1),
// 集中在 `/api/v1/agent/mail/{id}`、读会话参与者、`/mail/{id}/forward`。
//
// 也就是说:不是「少数旧客户端还没接线」,而是**主力客户端在裸奔**,
// 而放行恰好把它们全部漏了过去。日志抓手已经完成了它的使命 ——
// 它精确地指出了「谁还没带」,而答案就是所有人。
//
// # 为什么不能靠「补齐调用方」来收口
//
// 那要同时改四个桥(其中 pi 的 `getMailSessionId` 有 `= () => ''` 的默认值,
// 忘了注入就是静默空串 ⇒ 又回到裸奔),任何一处漏了 = 静默越权。
// **默认放行**与**默认拒绝**的差别就在这里:前者的失败模式是沉默的。
//
// 收口后调用方拿到的错误文案见 canReadSession 的 not-your-session 分支。
func AgentMayReadSession(ctx context.Context, agentName string, scope *uuid.UUID, target uuid.UUID) (bool, string, error) {
_ = ctx
_ = agentName
if scope == nil {
return true, "", nil // 迁移期:未声明当前会话者按旧语义放行(服务端另记警告)
// ★ 2026-10-02:由 `return true`(旧语义放行)改为**拒绝**。
//
// 理由见函数头:实测 dsh 不带 session_id 能读 20/20 封别人的信,
// 而那 202 次裸奔里 202 次来自四个桥自己 ⇒ 「迁移期」早已结束。
//
// reason 用 "not-your-session" 而不是新造一个:canReadSession 那个分支的
// 文案是「你当前在会话 <self>」,而 self 为空时它已经能自然表达
// 「你还没声明自己在哪条会话」—— 不必新增文案就不会漏译。
// (新增 reason 的话,还要同步改 handler 的 switch,而漏改就是 500。)
return false, "not-your-session", nil
}
if *scope != target {
return false, "not-your-session", nil

View File

@ -3,44 +3,96 @@ package repo
import (
"context"
"testing"
"github.com/google/uuid"
)
// SessionWorkspaceOf 必须真的读得到 `sessions.workspace`。
//
// 为什么值得单独立一条测试:这个查询是**列名对不上就静默返回空串**的形态
// (读不到时按设计返回 "",不报错)。空串在这里的语义是「这条会话没有工作
// 目录」,与「我查错列了」无法区分 —— 于是列名写错的表现是:功能看起来还在
// 跑,只是工作目录永远继承不到,Agent 继续落在一次性空目录里。
//
// 本轮演练里我刚因为猜错表名(`mail_attachments` 不存在)而崩过一次,
// 这类错误只能靠对着真实 schema 跑一次来发现。
func TestSessionWorkspaceOf(t *testing.T) {
ctx := context.Background()
/*
`AgentMayReadSession` —— 决定「能不能读这条会话」的那道闸(2026-10-02 补判据)。
# 为什么它此前零覆盖
`grep -rln AgentMayReadSession --include=*_test.go` ⇒ **没有任何测试碰过它**。
一个决定安全边界的函数没有任何判据,于是 2026-09-15 那个
「未声明 scope 就放行」的迁移期妥协一直活着,直到今天被实测打出来。
# 那个妥协的代价(实测数字,见函数头注释)
dsh 密钥 + 不带 session_id ⇒ 20/20 封别人的信全部 200(完整正文)。
带上 session_id ⇒ 闸是好的(带自己会话读别人的 = 403)。
⇒ 缺口就是 `scope == nil` 那一个分支。
*/
// ★ 核心:未声明 session_id 必须**拒绝**(旧语义是放行)。
func TestAgentMayReadSessionRejectsUndeclaredScope(t *testing.T) {
setupTestDB(t)
ctx := context.Background()
target := uuid.New()
sid, err := CreateSession(ctx, nil, "pi", "工作目录继承", "/root/projects/demo")
ok, reason, err := AgentMayReadSession(ctx, "dsh", nil, target)
if err != nil {
t.Fatalf("建会话: %v", err)
t.Fatal(err)
}
got := SessionWorkspaceOf(ctx, sid)
if got != "/root/projects/demo" {
t.Fatalf("SessionWorkspaceOf = %q,期望 /root/projects/demo —— "+
"空串说明列名或表名对不上(本函数读不到时静默返回空串)", got)
if ok {
t.Fatal("★ 未声明 session_id 必须拒绝 —— 实测旧语义下 dsh 不带 session_id " +
"读到了 20/20 封别人的信(收件方 pi/homeagent/opencode,跨工作区),全部 200")
}
// 反向对照:另一条没有 workspace 的会话必须返回空,而不是串上一条的值。
empty, err := CreateSession(ctx, nil, "pi", "无工作目录", "")
if err != nil {
t.Fatalf("建第二条会话: %v", err)
}
if got := SessionWorkspaceOf(ctx, empty); got != "" {
t.Errorf("没有 workspace 的会话应返回空串,实得 %q —— "+
"这可能是查询漏了 WHERE session_id 条件", got)
}
// 不存在的会话:按设计返回空串而不是 panic/报错(投递路径不能因它中断)。
if got := SessionWorkspaceOf(ctx, sid); got == "" {
t.Errorf("已存在的会话不应返回空串")
// reason 必须是 canReadSession 认识的既有值:新增 reason 而不同步改
// handler 的 switch,会让那个端点返回 500 而不是 403。
if reason != "not-your-session" {
t.Fatalf("★ reason 必须是 canReadSession 已处理的 %q(否则 handler 落到 default 分支或 500),实际 %q",
"not-your-session", reason)
}
}
// 闸本身没坏:声明了且等于目标就放行。
func TestAgentMayReadSessionAllowsOwnSession(t *testing.T) {
setupTestDB(t)
ctx := context.Background()
id := uuid.New()
ok, _, err := AgentMayReadSession(ctx, "dsh", &id, id)
if err != nil {
t.Fatal(err)
}
if !ok {
t.Fatal("声明的会话等于目标时必须放行(否则修复会把正常读信也堵死)")
}
}
// 声明了但不是目标 ⇒ 拒绝(跨会话隔离,这是「每个 session 是独立用户」的落点)。
func TestAgentMayReadSessionRejectsOtherSession(t *testing.T) {
setupTestDB(t)
ctx := context.Background()
mine, theirs := uuid.New(), uuid.New()
ok, reason, err := AgentMayReadSession(ctx, "dsh", &mine, theirs)
if err != nil {
t.Fatal(err)
}
if ok {
t.Fatal("★ 跨会话读必须拒绝")
}
if reason != "not-your-session" {
t.Fatalf("reason=%q", reason)
}
}
// agentName 不参与判断 —— 这是**刻意**的(2026-09-15 用户裁定:
// 「每个 session 概念上是一个独立的『用户』」,不按 agent 身份仲裁)。
// 这一格钉住那个裁定,防止将来有人"顺手"加一层按 agent 的判断。
func TestAgentMayReadSessionIgnoresAgentName(t *testing.T) {
setupTestDB(t)
ctx := context.Background()
mine, theirs := uuid.New(), uuid.New()
asDSH, _, _ := AgentMayReadSession(ctx, "dsh", &mine, theirs)
asNobody, _, _ := AgentMayReadSession(ctx, "完全不相干的 agent", &mine, theirs)
if asDSH != asNobody {
t.Fatalf("判定不应随 agentName 改变(2026-09-15 裁定:会话才是私有单位):dsh=%v other=%v",
asDSH, asNobody)
}
if asDSH {
t.Fatal("跨会话仍应拒绝")
}
}