Skip to content

feat(#461,#462): hub 观察者事件流 + 节点头像(含授权修复与 aggregate 隔离) - #474

Merged
vansin merged 4 commits into
mainfrom
feat/461-462-observer-events-avatar
Jul 29, 2026
Merged

feat(#461,#462): hub 观察者事件流 + 节点头像(含授权修复与 aggregate 隔离)#474
vansin merged 4 commits into
mainfrom
feat/461-462-observer-events-avatar

Conversation

@vansin

@vansin vansin commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Closes #461, closes #462.

Hub 观察者事件流(#461)+ 节点头像(#462)。已通过通信龙独立复审(第二轮)。

复审结论:PASS

复审方式:detached worktree,COMMHUB_DB 全程指向 scratch 路径(未触碰生产库),主工作树未改动,测试端口跑完全部释放(按精确 PID 清理)。复审者亲自重跑,非采信作者自述。

硬门 1 — observer 授权(第一轮 blocker)

第一轮的守卫是「或」:if (!observerRole && authCtx.networkId !== observedNetId),而既有 /events/:session 的 ntok 路径是「与」。成员关系正是吊销机制——removeNetworkMember 删成员行但不吊销该用户的 ntok。实测证伪:用户加入网络 A → 发 ntok → 移出网络 A → 同一活 token 打两个端点,旧端点 403、新端点仍 200,被移出者保住整网实时流。

现已改为无条件成员校验,networkId 逃生通道删除。

witnessed-red:把守卫改回旧「或」写法,精确命中一条红——
(fail) #461 GET /events/network/:id — auth > membership removal revokes observer access — ntok exploit repro → 11 pass / 1 fail;还原后 12 pass / 0 fail。该用例能红且红在正确位置,不是假门。

硬门 2 — aggregate 隔离(第一轮 blocker)

第一轮 9 个 HTTP 安全测试在 aggregate 跑法下完全不执行await import("./index.js") 在别的文件先启过 server 时是模块缓存 no-op,所有 fetch ConnectionRefused),死掉的正是本 PR 的整个 HTTP 层安全面。

现引入 bootServer({port:0}) seam,测试用私有实例并读回实际端口。双向验证:

正序 bun test src/uploads-http.test.ts src/observer-avatar-http.test.ts → 22 pass / 0 fail
反序 bun test src/observer-avatar-http.test.ts src/uploads-http.test.ts → 22 pass / 0 fail

(第一轮为 10 pass / 9 fail。)

关键:本 PR 刻意不使用 import.meta.main 守卫。 该守卫会让已发布包完全不监听端口——package.jsonbin 指向 bin/commhub.ts,它通过 import("../src/index.js") 动态加载 src/index.ts,因此那里的 import.meta.main 恒为 false。复审独立重做打包验收:

npm pack → 干净目录安装 → 真跑 node_modules/.bin/commhub-server → curl /health = 200

硬门 3 — avatar 规范化存储

第一轮校验通过后存的是原始串而非 parsed.hrefhttps://e.com/a".png 原样落库。现返回规范化值并拒绝残留危险字符。复审独立喂 12 个恶意 URL:

输入 结果
a".png 落库 a%22.png
<img> 落库 %3Cimg%3E
</script><script>… 全部百分号编码
反引号 落库 %60…%60
javascript: / java\tscript: / java\nscript: 拒绝
data: / vbscript: / file: 拒绝
user:pass@ / user@ 拒绝(本 PR 追加)

落库值中 " ' < > 反引号 零残留。

回归:零新增失败

两棵树以完全相同方式跑 aggregate:

基线 origin/main : 515 pass / 8 fail
本 PR            : 546 pass / 8 fail
本 PR 独有的失败  : 0 条

两边 8 条同名,均为既有的 #380 /api/host-supervisors(属 #434 范围,不在本 PR)。净增 31 个通过用例,无任何既有测试因本 PR 改变状态

其他

  • observer 事件仅含路由元数据,零内容字段;e2e 断言任务正文与回复正文均不出现在观察帧中。
  • observer key 中的裸 NUL 已净化:日志裸 NUL 字节数 0(第一轮为 17),file 判定 UTF-8 text 而非二进制。
  • send_task 按目标在线与否给 delivered/queued,不再无条件 delivered
  • /health 未鉴权暴露 observer key 的既有问题另开 [security][server] /health 未鉴权且暴露 SSE session/observer key 清单 #473 跟踪。

后续(不阻塞本 PR)

export const server = bootServer() 为模块级副作用——import 即绑端口。它正确避开了 import.meta.main 的坑,但更干净的终态是导出显式启动函数、由 bin 入口显式调用,两个坑都不踩。已与 #438 corrective 约定一起收敛,避免测试基础设施分叉。

SmartFlowAITeam and others added 3 commits July 29, 2026 08:15
…e persistence

#461 — hub 向观察者推事件(方案 B:独立观察流)
- push.ts: createNetworkObserverStream + pushNetworkObserverEvent;观察者
  client 走与 session 流完全相同的 backpressure/liveness 机制,key 用
  "\u0000netobs:" 前缀与 session key 空间隔离
- index.ts: GET /events/network/:network_id 路由(置于 /events/(.+) 之前),
  鉴权三路径对齐 /events/:session(master token / ntok_ / utok_ 按网络成员)
- 5 个 new_task/new_reply 推送点(MCP send_task/send_reply/retry_task/
  reassign + REST /api/task)补发网络级摘要事件:仅 task_id/from/to/
  status/priority/message_id,函数内统一盖 network_id + scope:"network",
  不含任何正文 → dashboard 可退役 15s 轮询
- network_id 为 null 的 legacy 流量无观察流(路由要求显式网络 id),
  pushNetworkObserverEvent 对 null 静默 no-op

#462 — 自定义头像跨设备持久化(方案 B:nodes 列,不动 RFC-024 白名单)
- avatar 是纯展示属性:不进 config-apply 管道(无 agent-node 消费方、
  无 revision/ack/restart),因此不触碰 ALLOWED_FLAGS 安全边界
- db.ts: nodes 表 additive 迁移 avatar_url TEXT(try/catch duplicate column
  样式与既有迁移一致)
- avatar-validate.ts: XSS 边界校验 — 仅 http/https、≤2048、先拒空白/控制符
  再 URL() 解析(防 "java\tscript:" 规范化走私)、拒 javascript:/data:/
  vbscript:/file:/blob:
- index.ts: PUT /api/nodes/:ref/avatar(网络作用域查找 + canRestWriteNetwork
  写权限门,同 node delete);GET /api/nodes 投影加 avatar_url

测试(已过"改坏报红"自检:分别 stub pushNetworkObserverEvent /
validateAvatarUrl 后对应用例全红)
- observer-push.test.ts: 6 用例,真实读流字节(摘要字段、无正文、网络隔离、
  与 session 通道双向隔离、null no-op、多观察者扇出)
- avatar-validate.test.ts: 11 用例(伪协议、控制符走私、长度、清除语义)
- observer-avatar-http.test.ts: 9 用例,真实 Bun.serve + fetch(401/403 门、
  第三方派发摘要不泄漏正文、avatar 设置/清除/拒收/越网隔离)
- scripts/qa.sh: 三个新文件登记进 L0_TESTS(L0 按文件独立 DB 运行)
- 全量 bun test src/ 的 8 个既有 fail 为 origin/main 基线原有
  (多 HTTP 测试文件同进程端口冲突),非本次引入;受影响既有套件
  (push/api-nodes-shape/uploads-http/api-host-supervisors/config-apply)
  按 L0 方式逐文件运行全绿

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… live HTTP gate, href normalization

通信龙独立审查(CHANGES_REQUIRED)三条 + 非阻塞项全部处理:

硬门 1 — observer 鉴权「或」改「与」:
- /events/network/:id 现在无条件要求 getUserNetworkRole 命中,删除
  authCtx.networkId 逃生通道。network_members 行即吊销机制
  (removeNetworkMember 不吊销 ntok),被移出网络的 ntok 立即 403
- 新增 ntok 精确复现用例:add member → mint ntok → remove member →
  同一活 token 打 /events/network/A 必须 403。已验证是真门:把「或」
  stub 回去该用例红(utok 变体不覆盖此洞——utok 的 authCtx.networkId
  为 null,只有 ntok 走得到漏洞分支,注释已说明)

硬门 2 — aggregate 下 9 个 HTTP 测试假活:
- index.ts 引入与 PR #438 同签名的 bootServer({port,hostname}) seam,
  但【不采用 import.meta.main 守卫】——已发布包入口是 bin/commhub.ts
  动态 import src/index.js,import.meta.main 恒 false 会导致进程不监听
  (通信龙 #438 审查结论)。保留模块级 export const server = bootServer()
  (import 即启动,pre-#434 行为),bunx 生产路径不变
- observer-avatar-http.test.ts 改为 bootServer({port:0}) 私有实例 +
  从返回值读实际端口,彻底顺序无关。门禁命令双向 0 fail:
    bun test src/uploads-http.test.ts src/observer-avatar-http.test.ts → 22 pass/0 fail
    (反序同样 22/0;三文件组合中仅 api-host-supervisors 的 8 个
    #434 既有 fail,属 #438 范围)
- 打包验收(通信龙要求):npm pack → 干净目录安装 → 真跑
  node_modules/.bin/commhub-server → curl /health 200 ok,
  新路由 /events/network/:id 与 PUT /api/nodes/:ref/avatar 401 如预期

硬门 3 — avatar 存 href 规范化串:
- validateAvatarUrl 返回 parsed.href(" < > ` → %22 %3C %3E %60),
  再拒 href 未编码的残留危险字符(路径中的 ');e2e 断言
  a"b.png 落库为 a%22b.png 且不含引号

非阻塞项:
- push.ts printableKey():observer key 的裸 NUL 在 console.log 与
  getSSEStats(/health 数据源)中显示为 \0,日志不再二进制化
- send_task 两处(MCP/REST)observer 摘要按 target.state 给
  delivered/queued,不再对离线目标谎报 delivered
- 补 send_reply e2e:registerTools 驱动真 MCP handler + 真 HTTP 观察流,
  断言回复正文不出现在 observer 帧
- /health 未鉴权暴露 observer key 属既有问题,另开 issue 跟进

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
通信龙复审 PASS 时留的小瑕疵:https://user:pass@host/a.png 会原样落库,
凭证既泄给头像宿主、也可能在界面上伪装来源。选择直接拒绝而非静默剥离
——调用方应当知道自己的输入带了秘密。+3 个测试用例。
门禁复跑:avatar-validate 14/0,双文件 gate 22/0。

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: abf7ff78f6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread server/src/index.ts
Comment on lines +630 to +634
const observerRole = getUserNetworkRole(authCtx.userId, observedNetId);
if (!observerRole) {
return withCors(req, Response.json({ ok: false, error: "not a member of this network" }, { status: 403 }));
}
return createNetworkObserverStream(observedNetId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Confine network tokens to their bound network

When a user belongs to networks A and B, an ntok_ minted for an agent in A can subscribe to /events/network/B, because this check only verifies the token owner's membership and ignores authCtx.networkId. This exposes B's network-wide task and reply routing metadata to a credential delegated specifically for A; the existing REST scope and session SSE paths explicitly force network tokens to their bound network, so this route should likewise reject requests where the bound network differs from observedNetId.

Useful? React with 👍 / 👎.

Comment thread server/src/index.ts
Comment on lines +630 to +634
const observerRole = getUserNetworkRole(authCtx.userId, observedNetId);
if (!observerRole) {
return withCors(req, Response.json({ ok: false, error: "not a member of this network" }, { status: 403 }));
}
return createNetworkObserverStream(observedNetId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Revoke already-open observer streams on member removal

If a member opens this SSE stream and is then removed from the network, the established connection remains registered and continues receiving every subsequent network summary indefinitely; authorization is checked only while creating the stream. The revocation test cancels its reader before removing the member and therefore covers only reconnect attempts, not the live stream described as revoked. Associate observer clients with their authenticated user/token and close or revalidate them when membership disappears.

Useful? React with 👍 / 👎.

Comment thread server/src/index.ts
Comment on lines +2109 to +2112
let sql = "SELECT node_id, alias, network_id FROM nodes WHERE (node_id = ?1 OR node_name = ?2 OR alias = ?3)";
sql = addNetworkScope(sql, params, restScope);
sql += " ORDER BY updated_at DESC LIMIT 1";
const node = db.get<any>(sql, ...params);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require a network for non-unique avatar references

When a non-admin user belongs to multiple networks and two nodes share the supplied alias or node name, restScope contains both networks and this query silently selects whichever row has the newest updated_at. Because node heartbeats continuously change that ordering, PUT /api/nodes/<alias>/avatar can update a different network's node from the one the caller intended. Require an explicit network for alias/name references, reject ambiguous matches, or restrict this write route to the globally unique node_id.

Useful? React with 👍 / 👎.

Comment thread scripts/qa.sh Outdated
Comment on lines +53 to +55
"observer-push:server/src/observer-push.test.ts"
"avatar-validate:server/src/avatar-validate.test.ts"
"observer-avatar-http:server/src/observer-avatar-http.test.ts"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Run the new suites in isolated Docker images

These new suites are registered as L0 tests, whose runner invokes bun test directly in the host checkout, and there are no independent Dockerfiles for them. That makes the new HTTP/security coverage depend on and mutate the local environment instead of the required isolated execution path; package each suite in its own Docker test directory and run it through the Docker stage.

AGENTS.md reference: AGENTS.md:L7-L9

Useful? React with 👍 / 👎.

Comment on lines +39 to +42
beforeAll(async () => {
// Kept for per-file runs launched without an external COMMHUB_DB (the
// documented contract still is to set it externally; see db-adapter.ts).
process.env.COMMHUB_DB = process.env.COMMHUB_DB || SERVER_DB;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Set the scratch database before importing DB modules

When this suite is run directly without an externally supplied COMMHUB_DB, the advertised fallback never takes effect: static imports of auth.ts, tools.ts, and db.ts are evaluated before beforeAll, so createAdapter() sees NODE_ENV=test with no database path and throws during module loading. Set the scratch path in a preload/bootstrap module before these imports, or dynamically import the DB-backed modules after setting the environment, so the documented standalone and aggregate test commands can actually start.

Useful? React with 👍 / 👎.

CI 的 qa.yml L0 层只 checkout+setup-bun,不安装依赖(ms 级预算),
而 observer-avatar-http 启真 HTTP server、import 链需要 MCP SDK,
进 L0 必然红。保留 observer-push / avatar-validate 两个纯单测。
per 通信龙 PR #474 CI 根因分析:一条永远红的门等于没有门。

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vansin
vansin merged commit 7fa3ad3 into main Jul 29, 2026
4 checks passed
@vansin
vansin deleted the feat/461-462-observer-events-avatar branch July 29, 2026 02:46
vansin added a commit that referenced this pull request Jul 29, 2026
- PUT /api/nodes/:ref/avatar:权限门、校验规则表、规范化存储(存 URL.href
  而非原始串)、内嵌凭证拒绝而非静默剥离、以及明确记录的残余风险
  (服务端不 fetch 故无 SSRF,但可让访问者浏览器请求任意主机)。
- GET /events/network/:network_id:成员关系是唯一放行条件,并说明为什么
  不能用「token 的 network_id 匹配」当逃生通道——ntok 在成员被移出时不会
  被吊销,那样等于给已移出的人保留整网实时流。
- 事件表只列元数据字段,并写明 new_task 的 status 反映在线/入队。

两节均按已合入 main 的实现核对(#474),vitepress build 通过。

Co-authored-by: vansin <smartflowaiteam@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants