merge: system code review report into dev

# Conflicts:
#	.doc/ops/CURRENT.md
This commit is contained in:
weichen
2026-09-26 02:06:58 +00:00
3 changed files with 338 additions and 0 deletions
+267
View File
@@ -0,0 +1,267 @@
---
title: "系统全面 Code Review(local_dev tip)"
status: done
created: 2026-09-26
owner: Cursor
task_id: "system-code-review-2026-09-26"
policy_revision: "agent-governance-2026-09-21-r1 + cursor-governance-2026-09-24-r1 @ f4c1b73f28580ad3df6db4f9ec0a80057006e516"
review_state: pending
integration_state: pending
deployment: not_required
cleanup: pending
remote_sync: not_requested
updated: 2026-09-26
severity: high
---
# 系统全面 Code Review(local_dev tip)
## 范围与授权
- **用户目标**:对系统做一次全面 code review,并按规范形成报告;**不允许实施修复**,只形成报告。
- **被审对象**:本地 `dev` tip `f4c1b73f28580ad3df6db4f9ec0a80057006e516`(merge: swboot-closeout-2026-09-26-r3);运行态参考 no-docker local_dev(`bin/agent-server`,schema 75),本报告以代码与门禁证据为主。
- **允许写路径**:`.doc/`(本审计主文档、INDEX、short-term 任务记录)。
- **交付终点**:可审审计报告 + 索引挂接;不改业务代码、不改 gap-matrix / FEATURE-MATRIX 状态值、不部署。
- **非目标**:逐文件全量行审、依赖 CVE 扫描、前端 XSS 系统化、动态渗透、实施整改。
- **方法**:并行四轴审查(架构边界 / 安全 / 可维护性 / 关键执行路径)+ 本机 `./scripts/arch-check.sh --mode block`、规模盘点与关键片段复核。
## 问题登记与经验检索
- **issue_id**:本轮不新建 ISSUES 行;发现项以本报告 FIND-* 编号跟踪。若后续授权修复,再按 [闭环规范](../ops/2026-09-23-issue-learning-workflow.md) 升 issue。
- **检索词**:`IDOR`、`GetUserContext`、`session_id`、`validated_only`、`fencing`、`arch-check`、`table_owners`、`ALLOW_IN_MEMORY`、`sh -c`、`swboot`。
- **命中**:历史审计见 [INDEX](./INDEX.md)(模块耦合 2026-09-20、对话办公盘点 2026-09-25、gap-matrix);经验库无与本轮 FIND-SEC-01 完全同题的已关闭 lesson。**采纳检查**:对照模块开发规范 §2/§4、authz fail-closed 惯例、intentflow 注释契约。
## 总判(Verdict)
`arch-check --mode block` **PASS**,且多项生产门禁(任务路径 stub 拒绝、FAV outbox consumer 多层禁用、docgov 角色不信任 body)仍然有效。但系统存在三类结构性风险:
1. **安全旁路**:会话上下文 IDOR(已证实);Authorizer 无池 fail-open;swboot `sh -c` + 前缀白名单。
2. **门禁空心化**:未登记表的 SQL 写被 arch-check **静默跳过**;server 仍持 `Runtime.Pool` 并直查/直写。
3. **多入口编排**:docgov / 多意图 / flag 面平行三套,关闭开关时存在静默降级。
本轮 **不改变** gap-matrix 的 `production_ready` 叙事;下列 critical/high 建议作为独立 remediation 候选,待授权后再立项。
## 现象
只读审查与门禁核验,无用户报障个案。审查期望:合入 swboot / dialogue-ctx / jevdecide / semantic-replace 后的 tip 在模块边界、鉴权与关键执行路径上仍满足生效规范。实际:门禁绿,但多处规范语义与实现分叉(见 Findings)。
## Findings(按严重度)
置信度说明:`verified` = 父会话已打开源码复核;`agent` = 子代理报告、抽样一致;`inferred` = 机制推断未跑动态用例。
### Critical
#### FIND-SEC-01 — 会话消息 IDOR(`session_id` 无归属校验)
| 项 | 内容 |
|---|---|
| 类别 | 授权 / IDOR |
| 位置 | `internal/server/handlers_context.go:17-50`;`internal/ctxmgr/manager.go:377-385` |
| 证据 | `handleGetUserContext` 仅校验 `user_id` 为本用户/管理员;`GetUserContext` 对 `chat_messages` 使用 `WHERE session_id = $1` **无 owner 条件**。对照 `chatflow` 读路径会校验会话归属。 |
| 影响 | 已登录用户若获知他人 `session_id`,可读取对方最近聊天正文。 |
| 置信度 | verified · high |
| 修复方向(不实施) | 复用 chatflow 会话授权;或按 `session.owner_id`/`user_id` 过滤后再返回。 |
#### FIND-ARCH-01 — arch-check 对未登记表静默跳过(表唯一写者门禁盲区)
| 项 | 内容 |
|---|---|
| 类别 | 门禁 / 模块边界 |
| 位置 | `internal/archtest/scan.go:198-205`;`module-boundaries.yaml` `table_owners`(约 59 表) |
| 证据 | `if !tracked { continue }`;本机 tip:`swboot_controlled_executions` / `projects` / `project_source_bindings` / `outbox_events` **UNTRACKED**;`arch-check --mode block` 仍 PASS。 |
| 影响 | 新表(含 swboot)可任意包写入而不触发 block;「表唯一写者」对未入库表无强制力。 |
| 置信度 | verified · high |
| 修复方向 | 未登记表的生产写应变 fail-closed,或强制登记。 |
#### FIND-ARCH-02 — server 持 Pool 直装 swboot 并 SQL 旁路
| 项 | 内容 |
|---|---|
| 类别 | 组合根越界 |
| 位置 | `internal/server/swboot_handlers.go`(NewPGRepoAllowlist / ControlledPatchExecutor / 直查 bindings & executions) |
| 证据 | 与模块规范 §2.1「server 只拿用例或端口」冲突;`app.Build` 已另有 allowlist 装配。 |
| 影响 | HTTP 层成为第二组合根;边界回归。 |
| 置信度 | agent · high(与规范条文一致) |
| 修复方向 | 收成 app 用例端口;禁止 handler 使用 `rt.Pool`。 |
### High
#### FIND-SEC-02 — Authorizer 无池时成员判定 fail-open
| 项 | 内容 |
|---|---|
| 类别 | 鉴权 fail-open |
| 位置 | `internal/authz/authorizer.go:112-114`、`205-207` |
| 证据 | `!a.enabled()`(pool==nil)时 `IsProjectMember`/`IsProjectOwner` 对 `userID>0` 返回 true;组合根始终 `NewAuthorizer(pool)`,nil pool 时 `authorizer()!=nil`,绕过「nil → failClosed」分支。 |
| 影响 | AllowInMemory / 无库误配时,任意登录用户可访任意 `project_id`。生产有 `ALLOW_IN_MEMORY` 门禁则触发面缩小,**机制本身仍 fail-open**。 |
| 置信度 | verified · high |
#### FIND-SEC-03 — swboot 受控执行:`sh -c` + basename 前缀白名单
| 项 | 内容 |
|---|---|
| 类别 | 命令注入面 / 边界语义 |
| 位置 | `internal/swboot/executor.go`(`sh -c`、前缀匹配);`ApproveMerge` 恒拒 |
| 证据 | 允许判定为字段首段 basename 前缀;命令经 shell 执行。执行器自身不做 merge/push,但若 admin allowlist 含 `git`/`bash` 等,客户端可间接 push/merge/deploy。 |
| 影响 | 在 creator + 宽松 allowlist 前提下,隔离 worktree 内可越出「测试命令」意图。 |
| 置信度 | agent · high |
| 说明 | 与 MATRIX「永不 merge/push/deploy」在「执行器 API」语义下仍成立;真正边界在 **admin 命令白名单粒度**。 |
#### FIND-FLOW-01 — intentflow `validated_only`/`proposed` 可经 confirm 进入 dispatch
| 项 | 内容 |
|---|---|
| 类别 | 执行模式门禁 |
| 位置 | `internal/app/intentflow/plans.go:365-376`;`intent_plan_handlers.go:86-88`;`ExecuteClaimed` 不读 `ExecutionMode` |
| 证据 | `DecisionConfirm` **允许** `PlanProposed`;确认后 HTTP 调用 `DispatchEligible`;`AllowedModes` 策略字段几乎未被执行路径使用。 |
| 影响 | shadow「安全」依赖「无人 Confirm」,而非执行器硬拒绝;U10/U11 live 已接线时可升级为真实写。 |
| 置信度 | verified · high(静态);未做动态 confirm 用例 |
#### FIND-QUAL-01 — docgov / 多意图多入口并行(职责扩散)
| 项 | 内容 |
|---|---|
| 类别 | 可维护性 / 双写路径 |
| 位置 | chat `docgov.*` PendingAction;`app/docgov_owner.go` `document.*`;`server/handlers_docgov.go`;intent vs intentflow 双套 flags |
| 证据 | 同一 `GovernanceEngine` 三入口;`intent.FeatureFlags` 与 `intentflow.FeatureFlags` 并存;Coordinator ~40 intent case + intentflow OwnerMux 平行。 |
| 影响 | 鉴权/确认/审计易漂移;回归矩阵笛卡尔积;开关关闭时 Mux→Query 静默降级(FIND-FLOW-02)。 |
| 置信度 | agent · high |
#### FIND-ARCH-03 — bizdev / Runtime / 万能 Engine 门面残留
| 项 | 内容 |
|---|---|
| 类别 | 架构债 |
| 位置 | `internal/bizdev/service.go`;`internal/app/app.go` Runtime;`docgov`/`coordinator` Engine |
| 证据 | bizdev 自开池并装配多引擎;Runtime 暴露 Pool+数十服务;Engine 方法面极大。arch-check allowlist 放行上帝包,PASS ≠ 模块化健康。 |
| 影响 | 新功能默认堆 handler/Runtime;与解耦批次目标持续摩擦。 |
| 置信度 | agent · high |
### Medium
| ID | 摘要 | 关键位置 | 置信度 |
|---|---|---|---|
| FIND-SEC-04 | 授权拒绝 404/403 区分,泄露资源存在性 | `task_run_auth.go`、`auth_helpers.go` | agent |
| FIND-SEC-05 | Chat SSE:阶段拒绝在 Flush 之后;`user_role` 可写入会话上下文 | `sse.go` | agent |
| FIND-SEC-06 | 对话 `agent.NewBackend` 对未知名默认 stub;生产密钥校验不禁 stub | `agent/backend.go`、`config.go` | agent |
| FIND-SEC-07 | 未认证 `/api/v1/vault/status` 暴露 FAV 开关 | `auth_middleware.go`、`vault.go` | agent |
| FIND-SEC-08 | 会话 token 存 `localStorage`(XSS 放大) | `web/src/api/auth.ts` | agent |
| FIND-DOC-01 | docgov 省略 fencing token 仍可写;heartbeat persist / squad reclaim fail-open | `docgov/lease.go` | agent |
| FIND-DOC-02 | `CommitAgentVersion` 与 edit lease 解耦竞态面 | `docrevision/service_agent.go` | agent |
| FIND-FLOW-02 | `OwnerMux` 在 U10/U11/U12 关闭时静默降级到 Query | `intentflow/mux.go` | agent |
| FIND-ARCH-04 | notification handler 端口失败后回落 Pool SQL | `notification_handlers.go` | agent |
| FIND-ARCH-05 | 提案批准后 handler 裸 goroutine 副作用 | `handlers_docgov.go` | agent |
| FIND-ARCH-06 | 错误信封拼接 `err.Error()` | 多 handler | agent |
| FIND-QUAL-02 | 全局 flag/metrics 损害并行测试隔离 | `intent/flags.go`、`intentflow/*` | agent |
| FIND-QUAL-03 | 巨石文件逼近/超过规范阈值(见规模盘点) | squad/taskrun/server/web | verified |
### Low / 契约说明
| ID | 摘要 |
|---|---|
| FIND-JEV-01 | jevdecide assist 失败为 **fail-open 回规则**(不编造);若产品要求「jev 挂则拒答」则不满足——属契约澄清,非实现疏忽。 |
| FIND-DOC-03 | summarizer 注释仍写「REPLACE 不接线」,与已合入 `applySemanticReplace` 矛盾。 |
| FIND-EVT-01 | 事件 payload 密钥过滤面窄于 taskrun `StripSecrets`(子串级)。 |
## 同类问题与影响范围
| 关联场景 | 共用机制 | 检查与证据 | 判定 |
|---|---|---|---|
| chatflow 读会话 | 会话 owner 校验 | chatlog Adapter authorize | 主路径健全;**旁路** `GetUserContext` 缺校验 |
| 任务 SSE 鉴权 | Flush 前 RequireProjectAccess | gap-matrix AUTH-TASK-SSE PASS | 项目门健全;阶段拒绝在流后属中危差异 |
| FAV outbox | Enabled:false + Register/Claim 硬拒 | registry/dispatcher/eventsconsumer | **未见旁路打开** |
| swboot ApproveMerge | 恒拒 | repo_allowlist | 自身 API 健全;命令白名单是真边界 |
| 生产 ALLOW_IN_MEMORY | validateProductionSecrets | config.go | 非空 Env 的 prod 有防护;空 Env+内存组合仍危险 |
**未覆盖(明确边界)**:govulncheck/npm audit;MinerU/Embedding SSRF 动态验证;FAV degraded 跨项目读 E2E;Markdown XSS 系统化;全量 `BeginTx` 持锁外呼追踪。
## 方案与验收(仅建议,本轮不实施)
### 建议优先级
1. **P0**:FIND-SEC-01(IDOR)— 补会话归属;加回归(他用户 session_id → 403/空)。
2. **P0**:FIND-ARCH-01 — arch-check 未登记表 fail-closed;补登记 swboot/projects 等属主。
3. **P1**:FIND-SEC-02 — Authorizer `!enabled()` 改 fail-closed 或与 `failClosedAuthz` 对齐。
4. **P1**:FIND-SEC-03 / FIND-FLOW-01 — argv 化命令 + 收紧白名单;Confirm 拒绝 `validated_only`/`proposed` 或执行器硬读 mode。
5. **P2**:收敛 docgov/多意图单一写路径与单一 flag 面;收回 server Pool。
### 验收提示(未来修复任务)
按 [验证契约](../ops/2026-09-21-validation-contract.md)「问题修复的回归范围」:原例 + 同类旁路 + 正常授权路径;工程侧补 `archtest` 用例证明未登记表 block。
## 严重度
报告整体:`high`(存在已证实 IDOR + 门禁盲区;未做动态利用)。
## 关联整改
- 既有: [模块解耦分批](../remediation/2026-09-20-module-decoupling-batch-plan.md)、[对话办公扩展](../remediation/2026-09-25-dialogue-office-expansion-plan.md)、[gap-matrix](../remediation/gap-matrix.md)
- **本轮未新建 remediation 文件**(用户禁止实施;方案仅建议)。授权后建议拆:`remediation/2026-09-26-system-code-review-followups.md`。
## 证据
### 门禁与规模(本机)
```text
被审 SHA: f4c1b73f28580ad3df6db4f9ec0a80057006e516
arch-check --mode block → production gate passed
tracked table_owners ≈ 59;swboot_controlled_executions/projects/project_source_bindings/outbox_events = UNTRACKED
Go 生产文件 LOC 前列(非测试):
cmd/bizdev-cli/main.go 1017
internal/squad/workflow.go 963
internal/server/task_run_handlers.go 858
internal/taskrun/manager.go 813
…
Web:
web/src/App.tsx 1748
web/src/engine/docxRenderer.ts 1263
…
```
### 子代理
| 轴 | 类型 | 用途 |
|---|---|---|
| 架构边界 | architecture-critic | 模块规范对抗审 |
| 安全 | security-auditor | OWASP 向只读审(无 PoC) |
| 可维护性 | thermo-nuclear-code-quality-review | 上帝对象/双路径/开关丛林 |
| 关键路径 | code-explorer | intentflow/docgov/swboot/FAV/jev |
### 运行态(旁证,非本报告验收对象)
此前会话:no-docker `bin/agent-server` `vcs.revision` = tip;`/api/v1/health`+`/ready` ok,schema 75。
## 开放问题
1. FIND-FLOW-01:产品是否故意允许对 shadow `proposed` 计划一键 Confirm 升 live?若否,属缺陷;若是,应改文档/MATRIX 与执行器注释。
2. FIND-JEV-01:assist「失败 fail-closed」的产品定义是否等于「回退规则」?
3. 是否授权将 FIND-SEC-01/ARCH-01 升 ISSUES 并开 remediation?
## 修复经过与经验
- **本轮未实施任何代码修复**(用户硬约束)。
- 不新增 lesson:发现尚未经验证修复闭环。
- 复发提醒:旁路 HTTP(绕过 chatflow/Owner)是鉴权回归的高发点。
## 状态与交接
| 字段 | 值 |
|---|---|
| 任务状态 | `done`(研究产物可审) |
| review_state | pending(待人工审阅报告) |
| integration_state | pending(报告合入 local_dev 另走 integrate) |
| deployment | not_required |
| cleanup | pending |
| remote_sync | not_requested |
| 剩余工作 | 授权后开 P0 修复任务;本 worktree 合入/清理由独立维护者执行 |
| 禁止续作 | 在本任务下直接改业务代码冒充「顺便修」 |
## Positive observations
1. `arch-check` 对**已登记** import/sql/path/event 仍 enforced。
2. FAV outbox consumer `Enabled:false` 多层硬拒,未见旁路打开。
3. 任务 SSE 项目鉴权在 Flush 前;docgov 角色取 principal 不信任 body。
4. 语义摘要 shadow:lease 提交后再 Summarize,符合持锁不做外部 IO。
5. swboot/jevdecide 领域包 import allowlist 自身较窄;污染主要在接线层。
+8
View File
@@ -13,6 +13,14 @@
## 核心文档(按阅读顺序推荐)
### 系统全面 Code Review(2026-09-26)
**文件**: [2026-09-26-system-code-review.md](./2026-09-26-system-code-review.md)
**结论**: tip `f4c1b73f` 上 `arch-check --mode block` PASS,但存在已证实会话上下文 IDOR、arch-check 未登记表静默跳过、Authorizer 无池 fail-open、swboot `sh -c` 前缀白名单、intentflow `proposed` 可 Confirm 升 dispatch 等 critical/high 项;FAV outbox 禁用路径未见旁路。**本轮不实施修复**。
**任务**: [short-term](../ops/short-term/2026-09-26-system-code-review.md) · mutex `docs-audit-review`
### 对话办公能力全景盘点(2026-09-25)
**文件**: [2026-09-25-dialogue-office-capability-research.md](./2026-09-25-dialogue-office-capability-research.md)
@@ -0,0 +1,63 @@
---
title: "系统全面 Code Review(仅报告)"
task_id: "system-code-review-2026-09-26"
status: done
created: 2026-09-26
updated: 2026-09-26
owner: Cursor
task_type: research
task_tier: small
policy_revision: "agent-governance-2026-09-21-r1 + cursor-governance-2026-09-24-r1 @ f4c1b73f"
branch: feature/system-code-review-2026-09-26
worktree: .worktrees/system-code-review-2026-09-26
base_dev_sha: "f4c1b73f28580ad3df6db4f9ec0a80057006e516"
write_scope: [".doc"]
allowed_environments: []
head_sha: "f4c1b73f28580ad3df6db4f9ec0a80057006e516"
mutex: "docs-audit-review"
review_state: pending
integration_state: pending
deployment: not_required
cleanup: pending
cleanup_owner: "独立集成/维护"
cleanup_trigger: "报告合入 local_dev 且无进程占用本 worktree"
remote_sync: not_requested
---
# 系统全面 Code Review(仅报告)
## 用户目标与授权
- **用户目标**:对系统做一次全面 code review,并按规范形成报告;不允许实施修复,只形成报告。
- **交付终点**:审计主文档可审;INDEX 挂接;本任务记录收口。
- **禁止项**:业务代码修复、gap-matrix/MATRIX 状态冒充发布、未授权环境写、push。
## 范围与非目标
- 允许写入:`.doc/audit`、`.doc/ops/short-term`、审计 INDEX。
- 非目标:实施 FIND-* 修复、动态渗透、依赖 CVE。
## 相关问题与经验
- 检索词与结论见 [审计主文档](../../audit/2026-09-26-system-code-review.md)。
- 无新增 lesson(未修复验证)。
## 计划与进展
1. [x] catalog overlap(none)+ worktree/register/claim
2. [x] 四轴并行审查 + arch-check/规模盘点
3. [x] 关键发现复核(IDOR、scan 未登记表、Confirm×proposed)
4. [x] 落盘审计报告与 INDEX
5. [ ] 合入 local_dev(独立 integrate)
6. [ ] cleanup worktree
## 验证
- 研究验收 = 报告有据可审(FIND 带路径与置信度)。
- 不适用产品 PI 全链路;**NOT REQUIRED** 产品测试 PASS。
## 状态与交接
- `status=done`(研究完成 ≠ 缺陷已修)。
- 后续 P0 候选:FIND-SEC-01、FIND-ARCH-01(须新授权/新 task)。
- cleanup 待合入后由独立维护者执行。