From 15e4fe9203d0f0b5d2249a9951758223a69b312a Mon Sep 17 00:00:00 2001 From: JianFeeeee Date: Fri, 2 Oct 2026 00:34:13 +0800 Subject: [PATCH] =?UTF-8?q?fix(=E5=AE=89=E5=85=A8):=20=E6=9C=AA=E5=A3=B0?= =?UTF-8?q?=E6=98=8E=20session=5Fid=20=E4=B8=8D=E5=86=8D=E6=94=BE=E8=A1=8C?= =?UTF-8?q?=20=E2=80=94=E2=80=94=20=E5=AE=9E=E6=B5=8B=E4=BB=BB=E6=84=8F=20?= =?UTF-8?q?agent=20=E5=8F=AF=E8=AF=BB=E5=85=A8=E9=83=A8=E9=82=AE=E4=BB=B6?= =?UTF-8?q?=E6=AD=A3=E6=96=87?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 漏洞（亲自实测，不是读码推断） 用 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` 干净。 --- server/internal/repo/session_workspace.go | 49 +++++++- .../internal/repo/session_workspace_test.go | 116 +++++++++++++----- 2 files changed, 131 insertions(+), 34 deletions(-) diff --git a/server/internal/repo/session_workspace.go b/server/internal/repo/session_workspace.go index 507e73f..d346389 100644 --- a/server/internal/repo/session_workspace.go +++ b/server/internal/repo/session_workspace.go @@ -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 为空时它已经能自然表达 + // 「你还没声明自己在哪条会话」—— 不必新增文案就不会漏译。 + // （新增 reason 的话，还要同步改 handler 的 switch，而漏改就是 500。） + return false, "not-your-session", nil } if *scope != target { return false, "not-your-session", nil diff --git a/server/internal/repo/session_workspace_test.go b/server/internal/repo/session_workspace_test.go index c60bb2f..bef68cb 100644 --- a/server/internal/repo/session_workspace_test.go +++ b/server/internal/repo/session_workspace_test.go @@ -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("跨会话仍应拒绝") + } +} \ No newline at end of file