Skip to content

feat(webchat): spec ⑥ 多租户前端一等公民化 — 登录/workspace/设置 + 后端 A1-A5 接线#762

Open
hrygo wants to merge 22 commits into
mainfrom
feat/webchat-spec6-frontend
Open

feat(webchat): spec ⑥ 多租户前端一等公民化 — 登录/workspace/设置 + 后端 A1-A5 接线#762
hrygo wants to merge 22 commits into
mainfrom
feat/webchat-spec6-frontend

Conversation

@hrygo

@hrygo hrygo commented Jun 18, 2026

Copy link
Copy Markdown
Owner

Summary

WebChat 多租户线路图最后一个 spec(集大成)。后端 spec ①-⑤ 已合入,本 PR 交付前端一等公民化 + 解除后端潜伏接线(A1-A5),端到端跑通多租户。

交付

前端

  • 强制登录:废除匿名 webchat_user,未登录重定向 /login(内建账号 + OAuth SSO 入口)
  • workspace 隔离:双侧边栏(workspace + session),按 workspace_id 过滤会话
  • 设置 modal:General / AI Config(worker_preference + agent_config_overrides)/ Members(admin: 用户启停 + 邀请码)/ Profile

后端 A1-A5 接线

  • A1 注册 /api/workspaces* CRUD + /api/admin/{invitations,users} 路由(含 OPTIONS preflight)
  • A2 WS init 绑 workspace(归属校验 + DeriveSessionKey 分叉修复)
  • A3 disable 用户 per-request 校验(AuthenticateRequest 三路径 + SetIdentityProvider)
  • A4 cookie HMAC secret 持久化(~/.hotplex/data/cookie_secret.key,0600)
  • A5 前端 init 发送 workspace_id 对齐 REST 强制

设计 / 跟踪

  • 设计:docs/superpowers/specs/2026-06-18-webchat-spec6-frontend-design.md
  • 后端已审查(≤50 人规模):无 P0/P1;P2-3 已修;其余因规模撤销或低风险不动作

Closes #760

Test plan

  • make check(quality + build + test)
  • 登录流(内建账号 + OAuth SSO)
  • workspace CRUD + 切换隔离
  • disable 用户即时拦截
  • cookie secret 重启持久
  • WS init workspace 归属拒绝

spec ⑥ brainstorm 输入基线。经代码核实区分"已就绪/潜伏/待对齐":
- A 组后端潜伏接线(A1 workspace 路由未注册阻塞前端 / A2 WS init 不绑
  workspace / A3 disable 用户无 per-request 校验 / A4 cookie secret 内存
  生成 / A5 前端未对齐 workspace_id 强制)
- B 组前端主体(登录页/workspace 切换/worker 选择/agent-configs 编辑/
  OAuth post-login)
- C 组文档同步 / D 组遗留升级
- 6 个 brainstorm 待决策点

跟踪 issue #760;spec ①-⑤ 已合入(#746/#748/#753/#757/#755)。
P0-1: Mitigate login timing attack — always execute bcrypt comparison
      using a dummy hash for non-existent/API-key-only users to equalize
      response times (~200ms regardless of user existence).

P0-2: Replace O(N) API key validation with O(1) SHA256 hash map lookup.
      Keys are pre-hashed at startup/reload/CRUD; authenticateKey now
      performs a single SHA256 + map lookup per request.

P1-3: Add 10s timeout to OIDC discovery via oidc.ClientContext to
      prevent config hot-reload from hanging on unreachable IdPs.

P1-4: Cache negative DB results (sql.ErrNoRows) with 5s TTL in
      DBResolver to prevent cache-penetration DoS attacks.

P2-5: Add background cleanup goroutine to DBResolver that sweeps
      expired cache entries every 2 minutes, with proper Close()
      wired into gateway shutdown sequence.

Closes #761
写 cookie_secret.key 失败时记录 slog.Warn(含"重启将使所有登录 cookie 失效"
后果说明),消除"重启后全员静默登出"的无日志故障。

spec⑥ 后端审查 P2-3。基于 ≤50 人规模重评估,这是唯一值得做的代码改动:
P2-1(DB Lookup 无缓存)/P2-4(多实例非原子写)/双轨性能/P3-4(fail-closed)
均因规模不成立而撤销。

验证:gofmt + go build + go vet + security/gateway test 全绿。
Delete architecture.svg, banner.png, and bot_avatar_white.png from
the repo-root assets/ directory. None are referenced in code or docs;
the README points to docs/assets/architecture.svg which remains.
make dev 之前走 dev-start → gateway-start → dev.sh,链路无 build 依赖,
首次或二进制缺失时才靠 dev.sh 兜底提示。新增 dev-build 目标:
- 复用 webchat-embed 缓存 + docs 存在性检查(跳过 swagger)
- 仅 go build,实测增量 1.7s
- dev-start 依赖 dev-build,make dev 每次自动产出最新二进制
CookieAuth was gated solely on cfg.WebChat.Enabled, which only covers
the embedded same-origin SPA. External dev/production frontends and
local devMode runs were left without browser-session cookies, breaking
authenticated webchat access in those setups.

Broaden the condition to also activate when cfg.WebChat.Addr is
configured (external frontend) or in devMode, so the cookie-based auth
path is available wherever webchat traffic is expected.
The Next dev server (127.0.0.1:3000) could not authenticate against the
gateway (localhost:8888): login set a cookie, but cross-origin requests
neither sent it (credentials defaulted to same-origin) nor were allowed
to receive it (no Access-Control-Allow-Credentials, and wildcard ACAO
forbids credentials per the CORS spec).

- security/cors.go: emit Allow-Credentials in echo-back (pinned-origin)
  mode; wildcard mode unchanged (credentials are spec-forbidden with "*")
- config-dev.yaml: pin allowed_origins to the dev frontend hosts instead
  of inheriting wildcard "*" so echo-back + credentials engage
- webchat api clients (auth/workspaces/sessions): cross-origin branch
  sends credentials:'include' so the login cookie travels with requests

@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:4 P3:4

A3(disabled 用户拦截)在 REST 三路径(AuthenticateRequest)上已正确修复,但 cookie 鉴权路径漏了两处;A2 workspace 绑定在两个分支补了 WorkspaceID,唯独 session-not-found 创建分支漏了。这两个 P1 建议阻断合并。架构层面整体良好(DeriveSessionKey 向后兼容、DBResolver 生命周期接线正确、无 N+1)。


🔴 P1

[P1] disabled 用户认证绕过 — cookie 路径未查 user status

  • WS upgrade: internal/gateway/hub.go:430-437 — 无 API key 时 cookieAuth.Authenticate(r) 成功即 userID=uidpendingAuth=falsec.userID 在 upgrade 时直接赋值(hub.go:457);conn.go 全程无 idp.Lookup(仅 workspace disabled 校验 @345)。
  • workspace REST CRUD: internal/gateway/workspace_handlers.go:33-45requireAuthcookieAuth.Authenticate,不查 disabled。
  • 对比已修复的 REST 三路径 internal/security/auth.go:98-103AuthenticateRequestidp.Lookupu.Status=="disabled")。
  • 后果:admin disable 用户后,cookie 7 天有效期内仍可建立 WS 会话 + 操作 workspace CRUD,disable 形同虚设。
  • 建议:抽取公共 cookieAuth.AuthenticateActive(r) → (uid, ok)(内部查 idp 拒绝 disabled),WS upgrade 与 requireAuth 统一复用;或在 upgrade cookie 分支同样 idp.Lookup 拒绝 disabled。

[P1] handleSessionNotFound 新建 session 丢失 WorkspaceID

  • internal/gateway/conn.go:458-468 — 生产 StartSessionSessionStartParamsWorkspaceID;而 startCreatedSession(508) 与 recreateDeletedSession(543) 均带 WorkspaceID: c.workspaceID
  • sessionID 虽由 DeriveSessionKey(...,workspaceID,...) 派生(386),但持久化的 SessionInfo.WorkspaceID="" → 该会话逃出 ListSessions?workspace_id=X 过滤与 pool.ReleaseForWorkspace(uid,"") 配额,多租户隔离漂移(首次创建即丢,影响面最大)。
  • 建议:补 WorkspaceID: c.workspaceID

🟡 P2

[P2] 前后端响应契约不一致

  • webchat/lib/api/auth.ts:60,89 声明 login/acceptInvite 返回 Promise<User>Userusername/role/status/display_name/created_at/updated_at),后端 internal/gateway/auth_handlers.go:85,211 仅返回 {"user_id": uid}(字段名也不匹配)。
  • auth.ts:47 getMe 声明 Promise<User>,后端 auth_handlers.go:111-113 仅回 {id,username,role,status},缺 display_name/created_at/updated_at(TS 必填)→ Profile tab 渲染 undefined;若 login 响应被直接当 currentUserrole undefined 导致 admin tab currentUser?.role==='admin' 恒 false。
  • 建议:login/accept-invite 返回完整 User(或前端统一 login 后 getMe 拉取并修正类型),me 端点补齐缺失字段。

[P2] conn.gocontext.Background() 替代请求/连接 ctx

  • internal/gateway/conn.go:339,380,421,458,474,498,515,533GetWorkspaceByID/sm.Get/StartSession 均用 context.Background(),client 断开或 graceful shutdown 时这些 DB 查询/worker 启动不取消,长查询可能泄漏 goroutine 或挂起关闭。属既有技术债,本 PR 新增 GetWorkspaceByID(339) 放大。
  • 建议:引入 conn 级 ctx(conn 关闭时 cancel),至少给新增调用传入可取消 ctx。

[P2] NewCookieAuthflag.Lookup("test.v") 探测测试环境

  • internal/security/cookie.go:58 — 查全局 flag 表判断是否落盘,脆弱(任何同名 flag 注册即误判)且生产代码不应依赖 testing flag 做行为分支。建议改为显式注入(构造参数/env),或测试用 t.TempDir()+HOTPLEX_HOME 隔离。

[P2] CookieAuth godoc 过期

  • internal/security/cookie.go:42,45 — 注释称 "never stored on disk or embedded in the binary" / "24h Max-Age",但本 PR 新增 ~/.hotplex/data/cookie_secret.key 文件持久化(@74-96) 且 cookieMaxAge=7*24h。注释与实现矛盾,易误导维护者。建议同步更新注释。

🔵 P3

[P3] docs/security/TODO-security-arch-review.md:49-50 — 验证 checkbox(golangci-lint run ./internal/security/...make test)未勾选,但正文 P0/P1/P2 全标 [x]。读者会误以为"实现完成但质量门禁未过"。跑完勾上或删除该清单。

[P3] cmd/hotplex/routes.go:228-246 — 显式注册的 OPTIONS handler 与 CORSMiddleware 内部 OPTIONS 短路重复,属死代码(CORS preflight 规范要求不鉴权,当前实现正确)。删除可减维护噪声。

[P3] 首屏 3 次 /api/auth/me 往返webchat/app/page.tsx:44app/login/page.tsx:60ChatContainer.assistant-ui.tsx:174 各调一次 getMe()。非热路径,建议用 SWR/cache 或把 user 状态提升到 layout 共享。

[P3] [UNCERTAIN] 删除 assets/architecture.svg|banner.png|bot_avatar_white.png 可能留 dangling 引用 — 删图后未在同 PR 更新 README/文档引用,建议 grep -r 全仓确认无 broken image。


Reviewed at HEAD 4752ef4 · automated by hotplex-ai

hotplex-ai
hotplex-ai previously approved these changes Jun 18, 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: APPROVE | P0:0 P1:0 P2:0 P3:3

增量 review(基准 4752ef42,新增 9 commits:首次部署引导功能)。两路并行核查(正确性/安全/并发 + 历史/架构/性能/文档),无阻塞项

P3 建议(非阻塞,可选采纳)

  • [P3] firstLogin TOCTOU 竞态 (internal/gateway/auth_handlers.go:84) — 同一用户并发登录,两请求均可能在 TouchUserLastLogin 前读到 LastLoginAt==0,都返回 first_login:true,导致 onboarding 卡重复弹出。仅 UX 影响,无数据完整性问题,可接受。
  • [P3] AcceptInvite first_login 硬编码一致性 (internal/gateway/auth_handlers.go:232) — Login 路径查询 LastLoginAt 判定,AcceptInvite 直接硬编码 true。当前 AcceptInvite 总是 CreateUser(LastLoginAt 必为 0),语义正确;但若未来支持"已有账号接受邀请"复用路径会误报。建议统一为查询逻辑以保持两条路径一致。
  • [P3] bootstrap-status 公开端点信息泄露 (internal/gateway/auth_handlers.go:104) — 无认证返回 {bootstrapped:bool},可探测系统是否已初始化。属引导功能固有代价,状态低敏感,且 corsMw 对 * 不设 Allow-Credentials 无 CSRF 风险,可接受。

已核查无问题(确认项)

  • 错误码契约:前端 mapAuthError 新增 10 个 code(INVALID_CREDENTIALS / USER_DISABLED / NO_IDP / INVALID_USERNAME / INVALID_PASSWORD / USERNAME_TAKEN / INVITATION_NOT_FOUND / INVITATION_USED / INVITATION_EXPIRED / BAD_REQUEST)与后端 writeAppError 逐个核对,大小写/拼写全匹配,无死代码 case。
  • login/acceptInvite 返回类型 User→LoginResult:仅 page.tsx 消费,无其他调用方依赖 User 字段,无破坏性。
  • 接口扩展:UserWorkspaceStore 新增 HasAdmin,仅 pgStore/SQLiteStore 两实现(编译断言强制),均已实现;测试用真实 SQLiteStore 非 mock,无编译破坏。
  • SQL 加载:users.has_admin.sql 经 embed 自动加载,PG/SQLite query map key 一致。
  • 路由注册:bootstrap-status 在 CookieAuth 块外,Go 1.22+ ServeMux pattern 精确匹配,无冲突;deps.WorkspaceStore 非 webchat-enabled 门控,引导场景可达。
  • 硬编码 role='admin':符合全库既有约定(identity_provider 等均用字面量),非新增 DRY 违规。
  • 文档一致性:前端展示的 hotplex admin create 命令字符串与真实子命令 + --username/--config flags 一致。

Cookie: relax session cookie to SameSite=None and treat loopback
origins as a secure context so the login cookie travels between the
dev frontend (127.0.0.1:3000) and the gateway (localhost:8888).
Browsers treat 127.0.0.1 and localhost as different sites, so the
previous SameSite=Strict cookie was dropped on the post-login getMe,
yielding a 401 that redirected back to /login. Chrome/Firefox allow
Secure cookies over http loopback, and production is same-origin
(embedded SPA) where SameSite is ignored, so None adds no CSRF risk.

Routes: always register GET /api/auth/oauth/providers (returns [] when
SSO is unconfigured) so a cross-origin browser gets 200 + CORS headers
instead of a CORS-masked 404 that spammed the login console.
Update TestCookieAuthSignVerify to expect SameSiteNoneMode (was Strict)
and add TestCookieSecureFlagLoopback covering the new loopback-as-secure-
context behavior that lets the dev frontend (127.0.0.1:3000) exchange
Secure cookies with the gateway (localhost:8888) over plain http.
session.Workspace had no JSON tags, so encoding/json emitted PascalCase
keys (Name, WorkDir, OwnerUserID, ...) while webchat reads snake_case
(ws.name, ws.work_dir). ws.name was undefined, crashing ChatContainer at
ws.name.slice(0, 2).

Existing tests decoded responses back into the same tagless struct, so
PascalCase round-tripped symmetrically and hid the wire-format bug. Add
TestWorkspace_JSONWireContract that decodes into a raw map to assert the
actual on-wire keys.
hotplex-ai
hotplex-ai previously approved these changes Jun 18, 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: ✅ APPROVE | P0:0 P1:0 P2:2 P3:3

增量 review(4752ef42..34bbfa01,11 commits)。本次新增 webchat 登录 onboarding、bootstrap-status 公共端点、first_login flag、cookie SameSite/Secure 对齐。无 P0/P1 阻断项,CI 全绿,予以通过。以下为改进建议。


P2

1. bootstrap-status 公共端点无缓存,登录页无节流轮询打库 [UNCERTAIN]
cmd/hotplex/routes.go:206 · internal/session/sql/queries/users.has_admin.sql:3
新增的 public、无需认证端点执行 SELECT 1 FROM users WHERE role='admin' LIMIT 1。前端登录页(webchat/app/login/page.tsxuseEffect 加载即轮询,diff 未见节流/缓存。查询本身轻量,但端点完全无防护,脚本化/爬虫可放大 DB 负载;017 migration 也未对 role 建索引。
建议:响应加 Cache-Control: public, max-age=30 抑制重复轮询;bootstrap 状态极少变化,store 层加短 TTL 缓存更佳。

2. OAuth/SSO 登录路径未返回 first_login,onboarding 引导对 SSO 用户缺失 [UNCERTAIN]
internal/gateway/oauth_handlers.go:171
password login 与 accept-invite 都新增了 first_login 语义,前端据此 localStorage('hotplex.onboarding','1') 触发欢迎卡片;但 OAuth Callback 直接 302 → "/",不经前端 login(),SSO 首次登录用户看不到 onboarding。
建议:SSO 回调重定向带 query(如 /?first_login=1),由 page.tsx 消费触发引导,保持三条登录路径契约一致。

P3

3. first_login 检测存在 TOCTOU 竞态
internal/gateway/auth_handlers.go:84-90
Login 在 LookupLastLoginAt==0TouchUserLastLogin 写入之间无事务/锁。同一账号两端并发首次登录,可能两次都返回 first_login:true(重复显示 onboarding),或第二次读到首次 touch 后的结果返回 false。UX 级边缘场景,非安全缺陷。
建议:可接受 best-effort 语义,加一行注释说明;若需消除,用条件 UPDATE ... WHERE last_login_at IS NULL 并检查 rows affected。

4. SameSite=Strict → None 放宽了跨站 Cookie,注释仅述开发场景未记 CSRF 取舍
internal/security/cookie.go:125,170,247
blame 显示原设计为 Strict。Strict→None 使会话 Cookie 随跨站请求发送。变更对跨 origin webchat 前端是必需的,且已用 Secure 缓解;但 cookie.go 注释只解释 loopback dev 场景,未记录生产 CSRF 评估(密码/accept-invite 的 POST 不再有 SameSite 默认防护,后端也无额外 CSRF token)。
建议:补充注释说明生产依赖 SameSite=None + Secure 的取舍,并在设计文档记录 CSRF 评估;确认所有状态变更端点通过要求 Content-Type: application/json 等方式防御 CSRF。

5. BootstrapStatus 入参用全量 UserWorkspaceStore,违反 ISP
internal/gateway/auth_handlers.go:104
函数仅需 HasAdmin,却接收暴露 ListUsers/DeleteUser 等能力的 UserWorkspaceStore
建议:抽最小接口 AdminChecker interface { HasAdmin(ctx) (bool, error) }。设计洁癖,非阻断。


确认无问题的项

  • role='admin' 与 017 migration CHECK 一致,schema 兼容,无需新 migration;
  • first_login 为新增响应键,旧前端忽略,向后兼容;
  • writeAppError envelope {"error":{code,message}} 与前端 errData?.error?.code 映射一致;
  • bootstrap-status 注册在 CookieAuth-gated 块外(CookieAuth 可能为 nil 的未 bootstrap 态)属刻意设计,注释充分;
  • HasAdmin 接口新增有编译期断言(multitenancy_store.go:97-98),PG/SQLite 两 store 均实现,无破坏性;
  • error 处理用 fmt.Errorf("has admin: %w", err) 符合项目 %w 约定。

@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:2

本次为增量 review(4752ef4 → d742cab3),聚焦 login bootstrap guidance 新功能。上次 CHANGES_REQUESTED 提出的 2 个 P1 在本增量中仍未修复——这两项是多租户安全/隔离的核心缺陷,建议先处理再合并。增量功能本身(bootstrap 引导卡 / first_login / onboarding / 错误码中文映射)实现合理。


🔴 P1(上次未修复,阻断合并)

[P1] disabled 用户 cookie 路径认证绕过(上次 P1,本次未触及)

  • internal/gateway/hub.go:430-434 — WS upgrade cookie 分支 cookieAuth.Authenticate(r) 成功即 userID=uid 放行, idp.Lookupu.Status=="disabled"。对比同包 auth_handlers.gorequireAdmin/resolveCookieAdmin(:384-425)与 REST AuthenticateRequest(internal/security/auth.go:101-105)均查 status。
  • internal/gateway/workspace_handlers.go requireAuthcurrentUsercookieAuth.Authenticate 同样不查 disabled。
  • 后果:admin disable 用户后,其 cookie(本次 cookie.go 已将有效期从 24h 延长到 7d,进一步放大窗口)在有效期内仍可建 WS 会话 + 操作 workspace CRUD,disable 形同虚设。
  • 建议:抽公共 AuthenticateActive(r)(内部 idp.Lookup 拒绝 disabled),WS upgrade cookie 分支与 requireAuth 统一复用。

[P1] handleSessionNotFound 新建 session 丢 WorkspaceID(上次 P1,本次未触及)

  • internal/gateway/conn.go:458-468StartSessionSessionStartParamsWorkspaceID(对比同文件 :508/:543 均带 WorkspaceID: c.workspaceID)。
  • 后果:该路径创建的会话 SessionInfo.WorkspaceID="",逃出 ListSessions?workspace_id=X 过滤与 per-workspace 配额,多租户隔离漂移(首次创建即丢,影响面最大)。
  • 建议:补 WorkspaceID: c.workspaceID

🟡 P2

[P2] HasAdmin 全表扫描 + users.role 无索引,公开端点轻量 DoS

  • internal/session/sql/queries/users.has_admin.sql:3 + migration 017_multitenancy_tables.sql(仅 users(status) 建索引,role 无索引)。
  • GET /api/auth/bootstrap-status 公开无鉴权(routes.go:206),每次请求 SELECT 1 FROM users WHERE role='admin' LIMIT 1。无索引 + 未认证外部可高频打 → 放大 DB 负载。
  • 建议:加 idx_users_role,或缓存 bootstrap 结果(只增不减,首次 true 后不再查库)。

[P2] getMe 响应契约不一致(上次 P2 子项,未修复)

  • 前端 webchat/lib/api/auth.ts:21-30 User 声明 display_name?/created_at/updated_at(部分非 optional),后端 Me(auth_handlers.go:116-133)仅返回 {id,username,role,status}。依赖 created_at/updated_at 的 UI 取 undefined 静默退化。
  • 建议:me 端点补齐字段,或前端类型改 optional 并处理缺失。
  • (注:login/acceptInvite 契约 + cookie godoc 两项上次 P2 本次已修复 ✅)

[P2] workspace 响应缺 created_at/updated_at/limit/offset

  • internal/session/multitenancy_store.go:14-21 Workspace struct 无 CreatedAt/UpdatedAt;前端 webchat/lib/api/workspaces.ts:26-28 声明 created_at:number; updated_at:number(非 optional)。snake_case wire contract 修复方向正确,但 marshal 后这两个 key 不存在 → 前端恒 undefined;ListWorkspacesResponse.limit/offset 后端同样未返回。
  • 建议:后端补字段 + 序列化,或前端类型改 optional。

🔵 P3

[P3] 非 HTTPS 非 loopback 明文部署登录静默失败

  • internal/security/cookie.go:124SameSite=None 必须配 Secure;cookieSecure(r)=isHTTPS(r)||isLocalhost(r) 在非环回明文 HTTP(内网 IP 或反代漏 X-Forwarded-Proto)下返回 false → 浏览器丢弃 cookie,登录 200 但后续 401,无明确错误。dev loopback 注释已覆盖,生产场景需文档强提示必须 HTTPS / 正确反代头。

[P3][UNCERTAIN] getBootstrapStatus 5xx 降级为已 bootstrap

  • webchat/lib/api/auth.ts:55-57!res.ok(含 5xx)统一 return true,后端 DB 故障时前端显示正常登录表单,把后端故障静默化。低危健壮性提示。

✅ 增量确认无问题

  • BootstrapStatus 无鉴权设计正确(引导前 CookieAuth 可能 nil,路由注释已声明)
  • OAuth providers discovery 的 else 分支(无 provider 返回 []+CORS)合理,消除登录页 CORS-masked 404 噪声
  • first_loginLookup→Touch TOCTOU 无安全后果(最坏并发首次登录两次都判 true,仅多弹一次 onboarding),且 lerr == nil && u.LastLoginAt == 0 短路保护 nil 解引用
  • AcceptInvite 硬编码 first_login:true 语义正确(新建用户必首次)
  • Workspace snake_case json tag 修复方向正确(对齐前端期望)

Reviewed at HEAD d742cab3(增量 4752ef4→d742cab3)· automated by hotplex-ai

P1-1 (auth bypass): cookie 路径 disabled 用户绕过 — 新增
  Authenticator.AuthenticateActiveCookie,WS upgrade / workspace requireAuth /
  AuthenticateRequest cookie 分支统一复用(拒绝 disabled,对齐 REST 三路径)
P1-2 (tenant isolation): handleSessionNotFound 补 WorkspaceID
  (对比 startCreatedSession/recreateDeletedSession)
P2-1: bootstrap-status 公开端点加 idx_users_role 索引 + Cache-Control
P2-2: getMe 返回完整 User(display_name/created_at/updated_at)
P2-3: workspace 响应补 created_at/updated_at + List 内存分页回显 limit/offset
P2-4: OAuth/SSO 回调补 first_login(getOrCreateUser 返回,重定向 /?first_login=1)
P3: cookie Secure 部署/CSRF 注释 + getBootstrapStatus 5xx 可观测

新增测试:TestAuthenticateActiveCookie(6 路径)+ 扩展 wire contract /
Me / BootstrapStatus 断言。

Co-Authored-By: Claude <noreply@anthropic.com>
hotplex-ai
hotplex-ai previously approved these changes Jun 19, 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-20260619/review_body.md

P2-1: AuthenticateActiveCookie 加 RLock 快照 cookieAuth/idp(消除热重载
       cookie/idp 时的 data race,-race 可检出)
P2-3: idx_users_role 改 partial index WHERE role='admin'(精确匹配 HasAdmin
       谓词,避免 2 值列全 B-tree 选择性差)
P3-6: 确认前端 last_login_at truthy 守卫正确处理 NULL→0(不显示,语义正确)

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

@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: ✅ Approve(可合并) | P0:0 P1:0 P2:0 P3:2

本次为第 3 轮增量 review36425005 → 29354cc9,3 文件 / 70 行)。上轮 CHANGES_REQUESTED 的 2 个 P1 阻断项已在 36425005 修复并验证;本轮增量(auth.go RLock snapshot + migration 021 partial index)正确性 / 并发 / migration 均无阻断问题。收敛,建议合并。


✅ 上轮 P1 阻断项收敛确认(在 3642500 修复)

  • P1-1 disabled 用户 cookie 认证绕过 — 已修复
    • hub.go:434 WS upgrade cookie 分支改用 auth.AuthenticateActiveCookie(r)(内部 idp.Lookup 拒绝 disabled,见 auth.go:171
    • workspace_handlers.go:36 currentUser 委托 AuthenticateActiveCookierequireAuth 统一复用
    • 三条 cookie 路径(REST / WS upgrade / workspace CRUD)现共享同一 disabled 拒绝定义,7d cookie 有效期下 disable 真正生效
  • P1-2 handleSessionNotFound 丢 WorkspaceID — 已修复
    • conn.go:468 StartSession 已带 WorkspaceID: c.workspaceID,与同文件 :509/:544 一致,不再逃出 per-workspace 过滤与配额

✅ 本轮增量确认无问题

  • auth.go:158-161 AuthenticateActiveCookie RLock snapshot — 正确
    • SetCookieAuth(auth.go:61)/SetIdentityProvider(auth.go:69) 均在写锁下整体替换指针;snapshot 旧指针后锁外做 IO 安全(旧对象不被原地改)
    • 与既有 AuthenticateKey(auth.go:120-123) 同模式,race 真消除;锁外 Authenticate/Lookup 避免持锁 IO;nil 判空充分(:162/:169 短路),ctx 用 r.Context() 正确
  • migration 021 partial index(SQLite + PG) — 正确
    • 021 是本 PR 新增(不在 origin/main,未发布),修改 migration 文件有效
    • 谓词 WHERE role='admin'users.has_admin.sql 查询精确匹配;admin 行极少 → 索引极小、查询近 O(1);SQLite(modernc 3.46+) / PG 均原生支持 partial index
    • DROP INDEX IF EXISTS idx_users_role 两方言按名删除,Down 兼容
    • 顺带修正旧注释对 "spec §8.6" 的误标引用(仓库内 §8.6 实指邀请制/防枚举,与 bootstrap 无关)

🔵 P3(非阻断,建议后续清理)

[P3] 残留同源 race 未覆盖(既有代码,非本轮引入)internal/security/auth.go:97
AuthenticateRequest 在 RUnlock 后仍无快照直接读 a.cookieAuth,与本次修复的 AuthenticateActiveCookie 同字段同模式。本轮目标即消除 -race,建议顺带将 :97 也 snapshot 化,彻底清除该字段的裸读。

[P3] migration 注释引用上轮 review 编号021_users_role_index.sql:5 / 021_users_role_index.pg.sql:5
(review P2-3) 是跨 review 会话的内部编号,对未来维护者无意义。建议改为中性表述,如 avoids poor selectivity of a full B-tree on a low-cardinality column


Reviewed at HEAD 29354cc9(增量 36425005→29354cc9)· automated by hotplex-ai

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.

[spec ⑥] WebChat 多租户收尾 — 前端一等公民化 + 后端潜伏接线 + 文档同步

2 participants