Skip to content

fix(webchat): workspace 握手韧性 + Settings 独立页 + 新建入口 + work_dir 沙箱#785

Open
hrygo wants to merge 18 commits into
mainfrom
fix/webchat-settings-page-workdir-handshake
Open

fix(webchat): workspace 握手韧性 + Settings 独立页 + 新建入口 + work_dir 沙箱#785
hrygo wants to merge 18 commits into
mainfrom
fix/webchat-settings-page-workdir-handshake

Conversation

@hrygo

@hrygo hrygo commented Jun 24, 2026

Copy link
Copy Markdown
Owner

Summary

spec ⑦ Phase 3 后用户反馈的 WebChat 问题修复 + 一轮 UI 打磨、死代码清理,以及 workspace 新建入口与 work_dir 沙箱约束。

韧性 / 架构

  1. WS 握手 workspace access denied 韧性:登出清理 hotplex_* workspace/session localStorage(消除跨账号残留——根因);runtime adapter 识别 workspace 握手拒绝(access denied / not found / disabled),经新增 onWorkspaceError 回调上抛,ChatContainer 重载 workspaces 并用合法 workspace 重连,而非 fatal 断开。
  2. Settings 弹窗 → /settings 独立页:新建 app/settings/page.tsx,复用原 4 个 tab 组件(General / AI Config / Profile / Members-admin);顶栏齿轮改 router.push('/settings'),删除 SettingsModal
  3. 创建会话移除 workdir 入口work_dir 不可变且来自 workspace(spec §6.2),后端早已忽略 client 值。全链路死代码清理。

workspace 新建入口 + work_dir 沙箱(本批新增)

  1. work_dir 沙箱前缀约束(后端 security.ValidateWorkspaceWorkDir:workspace work_dir 必须落在 $HOME/.hotplex/workspaces/<owner_user_id>/ 下(跨通道租户沙箱)。workspace Create/Update 追加校验,新增错误码 WORK_DIR_OUTSIDE_SANDBOX老 workspace grandfather 保留(仅新建/改 work_dir 时强制);API 契约不变{name, work_dir} 仍是,保护 SDK/WS 消费者)。通用 ValidateWorkDir(黑名单/symlink)不动,前缀约束是 workspace 专用独立函数。
  2. workspace 新建入口(前端)NewWorkspaceForm(name + 可选子目录 + 实时路径预览)+ NewWorkspaceModal;两处入口——/settings GeneralTab 顶部 + ChatContainer workspace 下拉。Default 自动创建改用沙箱路径 ~/.hotplex/workspaces/<uid>/default 替换原 ./workspace。详见 docs/specs/WebChat-Workspace-Create-WorkDir-Prefix-Spec.md

UI 打磨

  1. Settings 页 header 跨 tab 稳定scrollbar-gutter: stable 消除滚动条出现/消失导致的宽度抖动;切 tab scrollTo(0,0);section header min-h 固定;AI Config 编辑器面板高度调小避免溢出。
  2. Session 卡片重设计:移除 workspace/workdir 展示,title 升为 13px 粗体主视觉,新增 turn_count,footer 精简为 state + 时间单行,卡片从 ~150px 降到 ~96px。

死代码清理

  1. 移除 dead admin 页面(/admin/account/admin/workspaces/*,零入站导航引用)、未引用模块(BranchSelectorcontext-formatai-sdk-transport/client/indextypes/message),以及指向已删页面的遗留链接。

Test plan

  • pre-push 6 项质量门禁全通过(vet/verify/lint/build/tests/format)
  • go test ./internal/gateway/... ./internal/security/... ./internal/session/... -count=1 -race 全通过
  • webchat tsc --noEmit 通过;golangci-lint 0 issues
  • /settings GeneralTab 新建 workspace:实时路径预览、创建成功并切换 active
  • ChatContainer 下拉「New Workspace」打开 modal,新建后关闭并切换
  • API POST /api/workspaces work_dir 越界(如 /tmp/x)→ 403 WORK_DIR_OUTSIDE_SANDBOX
  • 老 workspace(非沙箱 work_dir)PATCH name → 200(grandfather 不触发前缀校验)
  • owner 隔离:A 用户把 work_dir 放在 B 的 uid 子树下被拒
  • 新用户首次登录:Default Workspace 自动落在 ~/.hotplex/workspaces/<uid>/default
  • 顶栏齿轮 → /settings,4 tab 切换 header 无抖动
  • 账号 A 登录 → 登出 → 账号 B 同浏览器登录,无 workspace access denied

Refs #772

…ession workdir

Three issues reported after spec ⑦ Phase 3:

1. WS handshake "workspace access denied" was fatal. Logout now clears
   hotplex_* workspace/session selection from localStorage (prevents
   cross-account leakage), and the runtime adapter surfaces workspace
   handshake rejections (access denied / not found / disabled) via an
   onWorkspaceError callback so ChatContainer reloads workspaces and
   reconnects with a valid one instead of dead-ending.

2. Workspace settings moved from a modal to a full /settings page
   (General / AI Config / Profile / Members). SettingsModal removed;
   tab components reused.

3. New-session workdir input removed. work_dir is immutable and derived
   from the workspace (spec §6.2); the backend already ignored the
   client value. Dead code pruned across NewSessionModal, useSessions,
   createSession API, the ?dir= deep link, and the runtime adapter.
…identity split)

Root cause of the persistent "workspace access denied" handshake error:

In cross-origin mode the browser cannot attach X-API-Key to the WebSocket
upgrade, so WS authenticates via cookie while REST authenticates via the
api-key. The backend resolves identity api-key-first, so when both an env
api-key (e.g. alice) and a cookie login (e.g. admin) are present, REST
resolves to the api-key user and WS to the cookie user. The REST workspace
list then returns workspaces the WS user does not own, and every handshake
is rejected.

authHeaders() now omits X-API-Key when a webchat_session cookie exists, so
REST and WS stay on the same (logged-in) identity. The env api-key remains
the fallback for non-interactive embedded use where no human logs in.

Also unblock locally by disabling HOTPLEX_WEBCHAT_API_KEY in
webchat/.env.local (gitignored) — that file had alice's real key, which
caused the split against the admin cookie login.
后端 (spec §6.2 演进): workspace work_dir 从 immutable 改为 workspace 级可变。
- PATCH /api/workspaces/{id} 接受 work_dir, 经 config.ExpandAndAbs +
  security.ValidateWorkDir 校验路径合法性与安全
- 唯一约束 (owner+work_dir) 冲突返回 409 WORK_DIR_TAKEN; 乐观并发失败
  仍返回 409 WORKSPACE_VERSION_MISMATCH
- workspaces.update SQL 及 PG/SQLite store 补齐 work_dir 列与绑定参数

前端 Settings 页面 v2:
- settings/page.tsx 重构为左 sidebar 分组导航 + 右内容卡片双栏布局
- general-tab 承接 worker preference 选择 (自 ai-config-tab 迁入) 并开放
  work_dir 可编辑, 配合后端可变
- ai-config-tab 精简为纯 AgentConfigEditor
- members-tab / profile-tab 视觉升级, 新增复制邀请码与用户 ID
- admin agent-config-editor / file-list 改为垂直布局 + 横向 tab bar

Refs #772
hotplex-ai
hotplex-ai previously approved these changes Jun 24, 2026

@hotplex-ai hotplex-ai left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Code Review — hotplex-ai

Verdict: APPROVED | P0:0 P1:0 P2:3 P3:4

CI: Build ✓ Test ✓ Large File Guard ✓(Issue Link 治理检查 fail,非阻塞,不影响 Build/Test 双绿结论)。
范围:后端 workspace 多租户 store(Go)+ webchat Settings 独立页迁移 + 移除会话 workdir + cookie 认证偏好。并发与 SQL 路径核查充分,未见 P0/P1 级正确性或安全问题。以下为建议项。


P2(建议修复,不阻塞合并)

1. work_dir 参与会话键派生却被允许变更,且两处"不可变"注释已脱节
internal/gateway/workspace_handlers.go:186-197 · internal/session/multitenancy_store.go:13 · internal/session/sql/migrations/017_multitenancy_tables.sql:22
work_dirDeriveSessionKey 的输入(internal/session/key.go:52,注释 L93 "WorkDir is included so that the same conversation on different working directories produces [different keys]")。本 PR 新增 PATCH 可变更 work_dir(Update L186),但:(a) multitenancy_store.go:13 仍写 "work_dir is immutable after creation (enters session key derivation, spec §7)"、017:22 仍写 "创建后不可变(应用层强制)"——注释与实现脱节;(b) Update 不像 Delete(L257 DeleteWorkspaceIfEmpty)那样检查活跃会话。
缓解sessions.work_dir 独立持久化(001_init.sql:17),现有活跃会话保留自身目录快照,不会被直接孤立。但同一 clientKeywork_dir 变更后会派生出新的 session_id,若前端用 workspace.work_dir + clientKey 重派生而非持久化 session_id 来锚定会话,旧会话将变为 UI 不可达。
建议:① 确认前端 useSessions 用持久化 session_id(localStorage)而非每次重派生;② 或在 Update 镜像 DeleteWorkspaceIfEmpty,对存在活跃会话的 workspace 拒绝 work_dir 变更;③ 同步把两处注释改为 "workspace 级可变(PATCH /api/workspaces/{id})"。最低限度先修注释。

2. Update handler 500 响应向客户端泄漏原始 DB 驱动错误
internal/gateway/workspace_handlers.go:235
本 PR 把 "update failed" 改为 "update failed: "+err.Error(),向 HTTP 客户端暴露底层 PG/SQLite 错误文本(表名/约束名/驱动细节)。同文件 Create(L114 "create failed")、Delete(L266 "delete failed")均为静态消息,此处不一致。
建议:恢复静态 "update failed",原始 err 仅经 slog 服务端记录。

3. nuqs 成为孤儿依赖
webchat/package.json
diff 移除了 ChatContainer.assistant-ui.tsx 中唯一的 nuqs 运行时 import,但 package.json 仍保留 "nuqs"(及 lockfile 条目)。全仓 from 'nuqs' 已 0 命中。
建议:从 package.json 移除 nuqs 并 regen pnpm-lock.yaml


P3(轻微)

4. 读后写 Update 无条件回写 work_dir(全字段覆盖)
workspace_handlers.go:226 · multitenancy_store.go:317 · multitenancy_pg_store.go:146
SQL 总是绑定 w.WorkDir,handler 重用 Get 读出的原值。CAS(updated_at) 已缓解大部分并发丢失更新,纵深防御上建议仅当 req.WorkDir != "" 时绑定该列(COALESCE 或条件 SET)。

5. 握手拒绝检测依赖脆弱的精确字符串匹配
webchat/lib/adapters/hotplex-runtime-adapter.ts:717-720
(data.message).toLowerCase() 精确匹配三个 workspace 拒绝串。后端消息措辞/大小写一旦变化,握手会静默回退为致命错误而非触发 workspace 重置。建议改按稳定 error code 或 includes("workspace") 子集匹配。

6. work_dir 可变语义注释易误导调用方
internal/gateway/workspace_handlers.go:163
注释 "workspace-level mutable" 易让调用方以为可清空,但 WorkDirstring + if req.WorkDir != "" 使 PATCH {"work_dir":""} 为 no-op;同结构 WorkerPreference*string 区分 omit/clear,模式不一致。建议注释明确 // "" = omit (no change); cannot be cleared (NOT NULL)

7. 三处 WORK_DIR_TAKEN 冲突字符串未抽常量(DRY)
internal/gateway/workspace_handlers.go:111,232
"work_dir already used by you" + code "WORK_DIR_TAKEN" 在 Create/Update 两处字面重复,同字段另有 WORK_DIR_IMMUTABLE。建议抽哨兵常量/helper,避免未来改文案漏改一处。


自动化 review by hotplex-ai · cron pr-review-hotplex · HEAD da5b7f9 · 2026-06-24

@hotplex-ai hotplex-ai left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Code Review — hotplex-ai

Verdict: Request changes | P0:0 P1:2 P2:3 P3:3

本 PR 让 workspace.work_dir 从 immutable(spec §6.2)变为 PATCH 可变,并复用 Create 同款 security.ValidateWorkDir,同时含 webchat UI 重构与死代码清理。后端验证逻辑与 Create 对称、SQL 经 //go:embed 编译期嵌入无需 migration、死代码清理无 dangling import —— 这些已核验通过。但 mutable work_dir 与既有 session-key 派生 / SwitchWorkDir 契约存在回归性冲突,需处理后再合。

P1

  • session-key 漂移致 resume 断链 + 无活跃会话守卫 (internal/session/key.go:52-58, internal/gateway/api.go:258-260, internal/gateway/workspace_handlers.go:183-194) — DeriveSessionKeyworkDir 直接计入 UUIDv5 hash(key.go:58 name += "|" + workDir),CreateSession 用 ws.WorkDir 派生 session id。本 PR 开放 PATCH 改 work_dir,但 Update handler 完全没有活跃会话守卫(CountActiveSessionsInWorkspace 接口已存在且被 Delete 路径使用,Update 却未用)。后果:带活跃会话时改 work_dir,客户端用同一 clientKey resume 会算出新 session id → 旧会话历史被孤立、对话断链,且 worker 仍跑在旧 cwd。spec §6.2/§7 正是把 "work_dir 不可变" 作为 resume 稳定性前提。建议:Update 改 work_dir 前用 CountActiveSessionsInWorkspace 拒绝(或要求先终止会话);或在 work_dir 变更时迁移/失效受影响会话。

  • SwitchWorkDir 契约自相矛盾 (internal/gateway/api.go:452 vs workspace_handlers.go:183) — SwitchWorkDir 对 workspace-bound session 返回 400 "work_dir is immutable for workspace-bound sessions; use a different workspace",而本 PR 让同一 workspace work_dir 可经 PATCH 修改。两个端点对同一字段给出相反语义,调用方无所适从。建议:要么更新 SwitchWorkDir 的拒绝逻辑/文案,要么在 PR 描述里明确 session 级 vs workspace 级 work_dir 变更的边界并统一两端点。

P2

  • 内部错误信息泄漏 (workspace_handlers.go:235) — Update 的 500 "update failed: "+err.Error() 回显底层 driver 错误(SQL 片段/约束名),而同文件 Create 对等分支(:114)用静态 "create failed"。不一致 + 信息泄漏,建议对齐 Create 改回静态文案,err 细节走 slog。

  • [UNCERTAIN] admin 代改 work_dir 的 WORK_DIR_TAKEN 措辞误导 (workspace_handlers.go:231-233) — UNIQUE(owner_user_id, work_dir) 是 per-owner 唯一(migration 017 第 28 行),"already used by you" 对 owner 自改准确;但 admin 代他人改命中他人名下约束时,文案会指向 admin 自己。仅 admin 场景。

  • [UNCERTAIN] TOCTOU / symlink race (workspace_handlers.go:192:226) — ValidateWorkDir 内 EvalSymlinks 是一次性快照校验,校验通过到持久化写入存在窗口,攻击者可把目录换成指向系统目录的 symlink。Create 同样有此问题(非本 PR 引入),但 mutable work_dir 扩大了可反复触发的攻击面。缓解建议:worker 启动时再校验一次 real path。

P3

  • spec / swagger / 注释大面积过期api.go:258 CreateSession 注释、api.go:165,258 swagger 注释、docs/swagger/swagger.json:1829docs/reference/admin-api.md:270docs/specs/WebChat-Multitenancy-Foundation-Design-Spec.md:242,399,485,514(§6.2/§9/§11.3 + 错误码表仍列 WORK_DIR_IMMUTABLE 400)均仍写 immutable,实现已移除该错误码并新增 INVALID_WORK_DIR/WORK_DIR_FORBIDDEN/WORK_DIR_TAKEN,需同步。
  • webchat/app/components/chat/NewSessionModal.tsx:117 文案仍称 work_dir immutable (spec §6.2)
  • 409 语义复用 (workspace_handlers.go:231) — WORK_DIR_TAKENWORKSPACE_VERSION_MISMATCH 共用 409,前端 general-tab.tsx 统一 re-fetch,用户无法区分"work_dir 被自己别的 workspace 占用"与"并发修改"。

备注(非缺陷,供确认威胁模型)

跨用户共享同一 work_dir 是 migration 017 注释明示的协作设计意图UNIQUE(owner_user_id, work_dir)),ValidateWorkDir 按 OS 进程用户判定、不感知应用层 ownerUserID —— 若产品确实要"同目录协作"则当前实现自洽;若多租户隔离要求更强,需在应用层追加 per-UID 沙箱前缀(据 Task 13-17 进行中的 ValidateWorkspaceWorkDir)。

ValidateWorkspaceWorkDir forces work_dir under $HOME/.hotplex/workspaces/<owner_user_id>. Wired into workspace Create/Update with new WORK_DIR_OUTSIDE_SANDBOX error code. Legacy workspaces grandfathered (only Create / work_dir Update enforce). Tests cover sandbox-in/out, owner isolation, traversal escape, grandfather. Refs #772
NewWorkspaceForm (name + optional subdir, 实时预览) + Modal。两处入口:/settings GeneralTab 顶部 + ChatContainer workspace 下拉。Default 自动创建改用 buildWorkspaceWorkDir(uid, default) 替换 ./workspace。work_dir 由后端沙箱校验兜底。Refs #772
@hrygo hrygo changed the title fix(webchat): workspace 握手韧性 + Settings 独立页 + 移除会话 workdir fix(webchat): workspace 握手韧性 + Settings 独立页 + 新建入口 + work_dir 沙箱 Jun 24, 2026

@hotplex-ai hotplex-ai left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@/tmp/pr-review-20260624/review-body-785.md

@hotplex-ai hotplex-ai dismissed their stale review June 24, 2026 16:08

self-correction: body was submitted as a literal @file path (gh api -f body=@file bug), contained no findings; re-submitting with correct content

@hotplex-ai hotplex-ai left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Code Review — hotplex-ai

Verdict: Request changes | P0:0 P1:3 P2:1 P3:1

增量审查范围:1ea62c9(上次有效 review) → 34a3690(当前 HEAD),共 12 文件。本轮新增 work_dir 沙箱前缀约束 (ValidateWorkspaceWorkDir) 与 webchat 新建入口。沙箱越界判断、校验调用顺序、Default workspace 前后端 $HOME 对齐、grandfather 老数据、SQL 编译期嵌入免迁移 —— 均已核验通过。但三处 P1(含上轮遗留 + 本轮新引入)需处理后再合。

P1

  • session-key 漂移 + Update 仍无活跃会话守卫(连续第三轮未修) (internal/session/key.go:52-58, internal/gateway/workspace_handlers.go:190-204) — DeriveSessionKey 仍把 workDir 计入 UUIDv5 hash(key.go:58 name += "|" + workDir),未改动;CreateSession 从 ws.WorkDir 派生 session id。Update 在 req.WorkDir != "" 分支只加了沙箱校验,仍未调用 CountActiveSessionsInWorkspace(接口 multitenancy_store.go:88 及 pg/SQLite 实现已存在,全仓零调用点)。后果在本轮下依然成立:带活跃会话 PATCH work_dir(即便新值仍在沙箱内)→ ws.WorkDir 变 → 同一 clientKey resume 算出新 session id → 旧会话历史孤立、对话断链、worker 仍跑旧 cwd。新沙箱不消除此问题(沙箱内移动 work_dir 同样改变 hash)。建议:Update 在 ws.WorkDir != abs 时先 CountActiveSessionsInWorkspace(ws.ID),>0 返回 409 WORKSPACE_NOT_EMPTY,或原子化为 UpdateWorkspaceWorkDirIfEmpty(对齐 Delete 的 DeleteWorkspaceIfEmpty 守卫模式)。

  • [新引入] Update 沙箱校验用操作者 uid 而非 workspace OwnerUserID —— admin 代改时 owner 隔离失效 (internal/gateway/workspace_handlers.go:200) — ValidateWorkspaceWorkDir(abs, uid)uid 取自 requireAuth操作者),不是 ws.OwnerUserID。admin 代改场景:ws.OwnerUserID="u-bob"、admin uid="u-admin",第 186 行 isAdmin 放行后,第 200 行要求 work_dir 落在 $HOME/.hotplex/workspaces/u-admin/ 下 —— 但 workspace 属于 u-bob,session 会以 u-bob 的 owner 身份在 admin 的沙箱目录启动 worker,owner 隔离被打破;反向也不可达:admin 永远无法把别人的 workspace 改到该 owner 真正的沙箱。这直接违反新 spec §2 G2「<owner_user_id>OwnerUserID 一致、owner 隔离」的定义,且错误文案 work_dir must be under $HOME/.hotplex/workspaces/<your-user-id> 本身就暴露了 bug(admin 代改时应是 owner 的 user-id)。Create (:106) 用 uid 正确(owner 即创建者)。建议:Update 第 200 行改传 ws.OwnerUserID。注意 TestWorkspace_OwnerIsolation_Sandbox 只覆盖了 Create 路径,漏测 Update admin 代改。

  • SwitchWorkDir 契约矛盾未消解,新 spec 反而固化矛盾 (internal/gateway/api.go:451 vs internal/gateway/workspace_handlers.go:200) — api.go:451 SwitchWorkDir 仍对 workspace-bound session 返回 WORK_DIR_IMMUTABLE "work_dir is immutable ... use a different workspace",而 PATCH update 允许改同一字段。新 spec §2 G2 宣称 work_dir 可「新建/更新」,但未界定 session 级 vs workspace 级变更边界,反而让两份 spec 对同一字段给出相反语义。建议在新 spec 决策表显式区分「workspace 级 PATCH 可改(受沙箱约束);session 级 SwitchWorkDir 对 workspace-bound session 拒绝」,并修订 Foundation-Design-Spec.md 对应描述与错误文案。

P2

  • Update internalServerError 泄漏底层 err(上轮残留) (internal/gateway/workspace_handlers.go:243) — writeAppError(..., "update failed: "+err.Error()) 仍把驱动层错误(SQL 片段/约束名)回显客户端,而同文件 Create/List/Delete 对等分支用固定文案。本轮在紧邻的第 200-203 行加了新校验却没顺手修这行。建议对齐为静态文案,err 细节走 slog。

P3

  • 文档/swagger 过期(上轮延续)docs/swagger/swagger.json 未收录 workspace 任何错误码(含新增 WORK_DIR_OUTSIDE_SANDBOX,及既有 INVALID_WORK_DIR/WORK_DIR_FORBIDDEN/WORK_DIR_TAKEN);docs/specs/WebChat-Multitenancy-Foundation-Design-Spec.md:242,399,514 仍写「work_dir 创建后不可变、PATCH 拒绝、错误码 WORK_DIR_IMMUTABLE 400」(实现已移除该码、新路径返回 403);docs/reference/admin-api.md:270 同样过期。本轮 12 文件未触及这些文档。

已核验通过(无需处理)

  • 沙箱越界判断 filepath.Rel 对 sibling-uid / 深层逃逸 / 跨盘符均正确拒绝,owner 隔离在 Create 路径成立。
  • 校验调用顺序正确:ExpandAndAbsValidateWorkDir(黑名单+symlink)→ ValidateWorkspaceWorkDir(沙箱)。
  • Default workspace 前后端 $HOME 对齐:前端发 ~/ 前缀、后端统一 os.UserHomeDir() 展开,无回归。
  • grandfather 老数据:Update 仅在改 work_dir 时校验,老数据改名不受影响,符合 spec「非目标: 不做迁移」。
  • workspaces.update.sql//go:embed 编译期嵌入,无需 DB migration。

附注:本次提交前 hotplex-ai 曾有一条针对本 commit 的 review 因 gh api -f body=@file 提交事故导致 body 为字面文件路径、无任何 findings,已予以 dismiss 并重新提交本条。

- P1-1: reject work_dir change while the workspace has active sessions
  (CountActiveSessionsInWorkspace guard). work_dir feeds DeriveSessionKey,
  so changing it shifts the deterministic session id and orphans bound
  sessions' history. Mirrors Delete's DeleteWorkspaceIfEmpty guard.
- P1-2: ValidateWorkspaceWorkDir keyed by ws.OwnerUserID, not the acting
  uid — preserves owner isolation when admin edits another user's workspace
  (spec §2 G2). Create is unaffected (creator == owner).
- P2: drop driver err from the "update failed" response, aligning
  Create/List/Delete which already use static text.
- Tests: active-session guard (409 WORKSPACE_NOT_EMPTY) + admin-edit
  owner-isolation (sandbox follows owner, not operator).
Switching workspace with the previous activeSession still set paired the
stale session (bound to the old workspace) with the new workspaceId in the
WS init handshake, which the server rejects as "session workspace mismatch"
(conn.go resolveSession). Clear activeSession synchronously before the
refresh; refreshSessions() repopulates it from the new workspace's list.
…chor session

- useSessions: workspaceId 变化时同步 setActiveSession(null),避免旧
  workspace 的 activeSession 与新 workspaceId 组合发起 WS 握手触发
  服务端 "session workspace mismatch" (conn.go resolveSession)
- NewWorkspaceForm: createWorkspace 后立即预创建 anchor session,让新
  workspace 切换时 listSessions 直接返回会话,消除空状态窗口;与
  useSessions 的 fallback 自动创建经 DeriveSessionKey 幂等
- sessions: 导出共享常量 ANCHOR_CLIENT_SESSION_ID,消除 'main' 字面量重复
…review P1-3/P3)

- P1-3: add §5.1.4 work_dir change-level decision table to
  WebChat-Workspace-Create-WorkDir-Prefix-Spec — distinguishes workspace-level
  PATCH (mutable; owner-sandbox + active-session guarded) from session-level
  SwitchWorkDir (immutable for workspace-bound sessions). Resolves the
  apparent contract contradiction flagged in review.
- P3: update stale "work_dir immutable" wording in
  WebChat-Multitenancy-Foundation-Design-Spec (§4 table, §6.2, §7.3 resume)
  and docs/reference/admin-api.md to reflect workspace-level mutability
  (WORK_DIR_OUTSIDE_SANDBOX / WORKSPACE_NOT_EMPTY / WORK_DIR_IMMUTABLE).

Note: workspace REST endpoints are not yet in swagger.json — WorkspaceHandlers
has zero swaggo annotations (historical gap, not staleness). Full swagger
inclusion is tracked as a separate follow-up.
- 移除 GeneralTab 顶部冗余的 New Workspace 创建卡片(创建入口已在聊天页
  workspace switcher 提供),消除同一屏两组 Workspace Name/Directory 字段
- Working Directory 改为前缀只读 + 段可编辑 + 实时预览,以 owner_id 为锚点
  从实际 work_dir 反推沙箱前缀,物理杜绝越界输入(新增 resolveSandboxAnchor
  兼容后端 ExpandAndAbs 存储的绝对路径形态)
- 新增 TabPanel 共享外壳并让 4 个 tab 统一对齐 AIConfigTab 标杆(space-y-4)
- Section Content Card 加 min-h-[600px] 消除 tab 切换时的高度抖动
- 清理 page.tsx 的 handleWorkspaceCreated 及 GeneralTab 的 uid/onCreated 死 prop
覆盖越权入口、双轨鉴权、功能重复、运维功能错位四类问题,给出入口收口、
鉴权收敛、Members 去重、危险操作审计的 4 阶段方案与验收标准。

@hotplex-ai hotplex-ai left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Code Review — hotplex-ai

Verdict: REQUEST_CHANGES | P0:0 P1:2 P2:2 P3:1

自动化 review。CI 全绿(Build/Test),本条针对最新 commit 24a3b8fa(文档澄清)+ 2d8d2edf(P1/P2 代码修复)的补强审查。两个 P1 均为前次 P1 修复后残留的并发窗口 / 守卫缺口,建议在合并前补齐。


🔴 P1

1. Update work_dir 的"无活跃会话"守卫存在 TOCTOU 窗口
internal/gateway/workspace_handlers.go:213-254

CountActiveSessionsInWorkspace(:214)与 UpdateWorkspace(:254)是两次分离的非原子调用,中间还隔着 name / agentConfig / worker 等字段校验(约 40 行)。workspaces.update.sql 的乐观锁 CAS 仅校验 AND updated_at = ?不重新校验活跃会话状态

竞态路径:admin 发起 Update 且 Count 返回 0 后、Update 写入新 work_dir 之前,一个并发 WS 握手可在该 workspace 创建一条 running 会话;该会话以旧 work_dir 派生 session key,但随后 workspace.work_dir 已变为新值 → session-key 漂移、会话历史被孤立。这正是 :208-212 守卫注释声称要阻止的危害。

对比:Deletedelete_if_empty 在 DB 层原子化,Update 缺乏对等的 "change work_dir only if no active session" 原子校验。

建议:PG 上把 Count+Update 收进单事务(SELECT ... FOR UPDATE 或在 UPDATE 语句中带活跃会包子查询原子校验);SQLite 已有 writeMu,可在 handler 层用同一把锁覆盖 Count→Update 区间。

2. WS /cd 命令绕过工作区绑定的 WORK_DIR_IMMUTABLE 保护
internal/gateway/commands.go:203(+ internal/gateway/bridge.go:568

REST SwitchWorkDirapi.go:449-454)对 si.WorkspaceID != "" 返回 400 WORK_DIR_IMMUTABLE,但 WS 通道 handleCD(commands.go:203-264)仅校验 owner(:232)、无 WorkspaceID 守卫,直接调用 bridge.SwitchWorkDir(:241);Bridge.SwitchWorkDir(bridge.go:568)同样无守卫。而 WS 是 WebChat 的原生通道。

更关键:validateAndExpandWorkDir(bridge.go:918)只做 ExpandAndAbs + ValidateWorkDir(黑名单),不调用 ValidateWorkspaceWorkDir(owner 沙箱 prefix 校验)。因此工作区绑定会话可经 WS /cd 切换到 owner 沙箱之外的目录,破坏 REST 守卫注释(api.go:443-448)自述的不变量,并触发"worker 跑在非自有目录 + DeleteWorkspaceIfEmpty 按 workspace_id 计数 → 硬删除另一 workspace 时仍有 worker 在其目录运行"的竞态。

建议:在 handleCD 或(更佳)Bridge.SwitchWorkDir 入口对 si.WorkspaceID != "" 返回错误,与 REST 对齐;建议把守卫下沉到 bridge,让 REST/WS 共用单一事实源。


🟡 P2

3. work_dir 不可变注释已过时,与实现矛盾

  • internal/session/multitenancy_store.go:13 — 注释称 work_dir "创建后不可变",但 workspaces.update.sql + UpdateWorkspace(workspace_handlers.go:190-224)现已允许工作区级 work_dir 变更。
  • internal/gateway/api.go:258 — "work_dir 是不可变的,来自工作区" 对会话级正确,但未提示工作区级已可变。

commit 24a3b8fa 更新了 admin-api.md / 新规范,但漏了这两处内联注释。建议同步修正,避免误导调用者低估"不可变性已放宽"的风险。


🟢 P3

4. README 引用已删除的 barrel 导出
webchat/lib/ai-sdk-transport/README.md:122(另见 AGENTS.md:38

import { BrowserHotPlexClient } from '@hotplex/ai-sdk-transport/client' 指向本 PR 删除的 client/index.ts barrel;实际导出已移至 client/browser-client.ts 并经 index.ts re-export。纯文档,无代码引用该死路径,但示例已失效。


备注(informational,非阻塞)

  • internal/security/path.go:102-120 ValidateWorkspaceWorkDir 基于逻辑路径做 filepath.Rel 前缀判断、不解析 symlink,存在 check-to-use 间隙被后续创建的 symlink 绕过的理论可能。当前已有 ValidateWorkDir(:74) + ExpandAndAbs 的 EvalSymlinks 兜底,触发门槛高,仅备案。
  • workspaces.update.sqlupdated_at CAS、UNIQUE(owner_user_id, work_dir) 约束、DeleteWorkspaceIfEmpty 的 PG 重新检查消歧,均设计合理,无 DB 不兼容;ANCHOR_CLIENT_SESSION_ID 重命名在源码内一致。

AgentConfigEditor.handleSave 只更新本地 state,未通知父组件 workspace 已更新。
切 tab 时 AIConfigTab 条件渲染 unmount,切回 remount 从父组件未更新的
workspace.agent_config_overrides 重新初始化,导致编辑器显示空(刷新页面
重新拉取才正常)。

修复:复用 GeneralTab 已有的 onUpdated → setWorkspace 模式。
- AgentConfigEditor 加 onSaved 回调,用 updateWorkspace 返回的最新 ws 冒泡
- AIConfigTab 透传 onUpdated → editor.onSaved
- settings/page.tsx 给 AIConfigTab 传 handleWorkspaceUpdated

附带同步 updated_at,避免后续 GeneralTab 保存触发 stale CAS 409。
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants