From c7e9ec0a268c1618282aa4812ba156383c1646df Mon Sep 17 00:00:00 2001 From: weichen Date: Sat, 26 Sep 2026 01:33:41 +0000 Subject: [PATCH 1/6] =?UTF-8?q?docs(audit):=20=E7=B3=BB=E7=BB=9F=E5=85=A8?= =?UTF-8?q?=E9=9D=A2=20code=20review=20=E6=8A=A5=E5=91=8A=EF=BC=88?= =?UTF-8?q?=E4=BB=85=E5=88=86=E6=9E=90=E4=B8=8D=E4=BF=AE=E5=A4=8D=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 记录 tip f4c1b73f 上架构/安全/可维护性/关键路径审查结论；不实施代码修复。 --- .doc/audit/2026-09-26-system-code-review.md | 267 ++++++++++++++++++ .doc/audit/INDEX.md | 8 + .doc/ops/CURRENT.md | 1 + .../2026-09-26-system-code-review.md | 63 +++++ 4 files changed, 339 insertions(+) create mode 100644 .doc/audit/2026-09-26-system-code-review.md create mode 100644 .doc/ops/short-term/2026-09-26-system-code-review.md diff --git a/.doc/audit/2026-09-26-system-code-review.md b/.doc/audit/2026-09-26-system-code-review.md new file mode 100644 index 00000000..44371244 --- /dev/null +++ b/.doc/audit/2026-09-26-system-code-review.md @@ -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 自身较窄；污染主要在接线层。 diff --git a/.doc/audit/INDEX.md b/.doc/audit/INDEX.md index e032e980..dc347957 100644 --- a/.doc/audit/INDEX.md +++ b/.doc/audit/INDEX.md @@ -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) diff --git a/.doc/ops/CURRENT.md b/.doc/ops/CURRENT.md index febac83f..38806d60 100644 --- a/.doc/ops/CURRENT.md +++ b/.doc/ops/CURRENT.md @@ -18,6 +18,7 @@ | owner | task | branch | worktree | mutex | status | |---|---|---|---|---|---| +| Cursor | system-code-review-2026-09-26 | feature/system-code-review-2026-09-26 | .worktrees/system-code-review-2026-09-26 | docs-audit-review | in_progress | | pi | swboot-closeout-2026-09-26-r3 | feature/swboot-closeout-2026-09-26 | .worktrees/swboot-closeout-2026-09-26 | workflow-design | done | | pi | swboot-closeout-2026-09-26-r2 | feature/swboot-closeout-2026-09-26 | .worktrees/swboot-closeout-2026-09-26 | workflow-design | in_progress | | pi | chat-semantic-replace-2026-09-26 | feature/chat-semantic-replace-2026-09-26 | .worktrees/chat-semantic-replace-2026-09-26 | chat | done | diff --git a/.doc/ops/short-term/2026-09-26-system-code-review.md b/.doc/ops/short-term/2026-09-26-system-code-review.md new file mode 100644 index 00000000..b43f63d4 --- /dev/null +++ b/.doc/ops/short-term/2026-09-26-system-code-review.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 待合入后由独立维护者执行。 From b6e382e58f334216c8c424c0fc99cc516802d70e Mon Sep 17 00:00:00 2001 From: weichen Date: Sat, 26 Sep 2026 01:34:08 +0000 Subject: [PATCH 2/6] =?UTF-8?q?docs(ops):=20=E6=A0=87=E8=AE=B0=20system-co?= =?UTF-8?q?de-review=20=E8=AE=A4=E9=A2=86=E4=B8=BA=20done?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .doc/ops/CURRENT.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.doc/ops/CURRENT.md b/.doc/ops/CURRENT.md index 38806d60..90e64184 100644 --- a/.doc/ops/CURRENT.md +++ b/.doc/ops/CURRENT.md @@ -18,7 +18,7 @@ | owner | task | branch | worktree | mutex | status | |---|---|---|---|---|---| -| Cursor | system-code-review-2026-09-26 | feature/system-code-review-2026-09-26 | .worktrees/system-code-review-2026-09-26 | docs-audit-review | in_progress | +| Cursor | system-code-review-2026-09-26 | feature/system-code-review-2026-09-26 | .worktrees/system-code-review-2026-09-26 | docs-audit-review | done | | pi | swboot-closeout-2026-09-26-r3 | feature/swboot-closeout-2026-09-26 | .worktrees/swboot-closeout-2026-09-26 | workflow-design | done | | pi | swboot-closeout-2026-09-26-r2 | feature/swboot-closeout-2026-09-26 | .worktrees/swboot-closeout-2026-09-26 | workflow-design | in_progress | | pi | chat-semantic-replace-2026-09-26 | feature/chat-semantic-replace-2026-09-26 | .worktrees/chat-semantic-replace-2026-09-26 | chat | done | From 6a3a0dc201362799ba11552146558f145541e9dc Mon Sep 17 00:00:00 2001 From: weichen Date: Sat, 26 Sep 2026 02:00:14 +0000 Subject: [PATCH 3/6] fix(security): harden bypass authz, gate blind spot, command exec, validated_only (review P0/P1) - FIND-SEC-01: GetUserContext requires session ownership (handler 403 + SQL/memory owner filter); in-memory HTTP + isolated-PG regressions - FIND-ARCH-01: archtest fails closed on untracked table writes (+noise exemption) and registers 50 existing writer tables in table_owners - FIND-SEC-02: Authorizer without pool denies by default; explicit NewInMemoryAuthorizer seam opt-in via ALLOW_IN_MEMORY at composition - FIND-SEC-03: swboot commands run argv-exec (no sh -c) with exact basename allowlist match; prefix-spoof and no-shell-eval tests - FIND-FLOW-01: confirm rejects validated_only plans; ExecuteClaimed hard-refuses validated_only steps (new error codes + HTTP mapping) - FIND-QUAL-01: intentflow.ValidateFlagConsistency fail-fast at Assemble --- .doc/system-design/module-boundaries.yaml | 109 +++++++++++++++ internal/app/app.go | 9 ++ internal/app/intentflow/errors.go | 10 ++ internal/app/intentflow/flags.go | 17 +++ internal/app/intentflow/flags_test.go | 20 +++ internal/app/intentflow/plans.go | 15 +++ internal/app/intentflow/plans_test.go | 73 +++++++++++ internal/archtest/arch_test_test.go | 35 +++++ internal/archtest/scan.go | 20 +++ .../internal/docgov/owner_upsert_write.go | 9 ++ .../mini/internal/server/untracked_write.go | 7 + internal/authz/authorizer.go | 17 ++- internal/authz/service_test.go | 18 +++ internal/ctxmgr/manager.go | 13 +- .../document_revision_duty_write_test.go | 3 +- internal/server/handlers_context.go | 13 ++ internal/server/idor_hotfixes_test.go | 124 ++++++++++++++++++ internal/server/intent_plan_handlers.go | 4 + internal/server/intent_plan_handlers_test.go | 2 + internal/server/project_handlers_test.go | 2 + .../server/task_run_busy_preflight_test.go | 6 +- .../server/task_run_idempotent_sse_test.go | 2 +- internal/swboot/executor.go | 22 +++- internal/swboot/executor_test.go | 41 ++++++ 24 files changed, 574 insertions(+), 17 deletions(-) create mode 100644 internal/archtest/testdata/mini/internal/docgov/owner_upsert_write.go create mode 100644 internal/archtest/testdata/mini/internal/server/untracked_write.go diff --git a/.doc/system-design/module-boundaries.yaml b/.doc/system-design/module-boundaries.yaml index a432b7e0..4350fef4 100644 --- a/.doc/system-design/module-boundaries.yaml +++ b/.doc/system-design/module-boundaries.yaml @@ -465,6 +465,115 @@ table_owners: - sysadmin maintenance_windows: - maintenance + # FIND-ARCH-01 回归补登记（2026-09-26）：以下为既有写者的真实表。 + # 多 owner 条目是现状快照（历史多写者），新包写入仍会被 block； + # 单一写者收敛在各模块整改批次处理，不在本批扩大。 + agentx_workspace_registry: + - workspace + asset_parses: + - materialindex + auth_sessions: + - auth + chat_read_positions: + - ctxmgr + chat_sessions: + - ctxmgr + context_memory_chunks: + - ctxmgr + - storage + document_access_audit: + - freedoc + document_agent_candidates: + - docrevision + document_audit_logs: + - docgov + document_change_proposals: + - docgov + document_change_reports: + - docrevision + document_comments: + - docgov + document_content_blobs: + - docrevision + document_content_projections: + - docrevision + document_duty_assignments: + - projectinstance + document_lineage: + - projectworkflow + document_pending_supplements: + - docgov + file_references: + - storage + freeform_document_versions: + - freedoc + iam_system_config: + - auth + material_chunks: + - materialindex + outbox_events: + - notification + project_activities: + - ctxmgr + project_blackboards: + - ctxmgr + project_context_snapshot_sources: + - projectcontext + project_context_snapshots: + - projectcontext + project_doc_task_instances: + - projectinstance + project_members: + - authz + - docduty + project_source_bindings: + - projectworkflow + project_stage_instances: + - projectinstance + project_workflow_instances: + - projectinstance + project_workflows: + - projectworkflow + projects: + - ctxmgr + - docgov + - projectinstance + - projectworkflow + storage_blobs: + - storage + swboot_controlled_executions: + - swboot + system_role_permissions: + - authz + system_roles: + - authz + task_run_events: + - taskrun + template_assets: + - workflowdef + template_versions: + - workflowdef + user_credentials: + - auth + user_password_reset_audits: + - auth + user_preferences: + - ctxmgr + user_role_bindings: + - auth + users: + - auth + - docgov + workflow_activation_audits: + - projectworkflow + workflow_activation_tokens: + - projectinstance + workflow_definitions: + - workflowdef + workflow_runs: + - projectworkflow + workflow_versions: + - workflowdef path_owners: output.md: diff --git a/internal/app/app.go b/internal/app/app.go index 7845e5ca..f8229a81 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -110,6 +110,10 @@ func Assemble(cfg *config.Config, svc *bizdev.Service) (*Runtime, error) { if cfg != nil { observability.InitLogger(cfg.Env) } + // FIND-QUAL-01：组合根启动期校验双套 flag 一致性（fail-fast）。 + if err := intentflow.ValidateFlagConsistency(intent.CurrentFeatureFlags(), intentflow.CurrentFeatureFlags()); err != nil { + return nil, err + } if svc == nil { var err error svc, err = bizdev.NewService(cfg) @@ -265,6 +269,11 @@ func Assemble(cfg *config.Config, svc *bizdev.Service) (*Runtime, error) { } authorizer := authz.NewAuthorizer(pool) + if pool == nil && cfg != nil && cfg.AllowInMemory { + // 显式 ALLOW_IN_MEMORY 演示模式：无库时采用内存授权语义； + // 误配（无库且未显式声明）仍走 NewAuthorizer(nil) 的严格拒绝。 + authorizer = authz.NewInMemoryAuthorizer() + } wireDutySlotWriter(authorizer) rt := &Runtime{ Cfg: cfg, diff --git a/internal/app/intentflow/errors.go b/internal/app/intentflow/errors.go index 3737e226..ee45204f 100644 --- a/internal/app/intentflow/errors.go +++ b/internal/app/intentflow/errors.go @@ -12,6 +12,12 @@ var ( ErrPlanActionForbidden = errors.New("PLAN_ACTION_FORBIDDEN") ErrPlanActionVersionConflict = errors.New("PLAN_ACTION_VERSION_CONFLICT") ErrUnauthenticated = errors.New("ERR_UNAUTHENTICATED") + // FIND-FLOW-01 回归：validated_only（shadow/dry-run）计划的硬边界。 + // Confirm 不得把 validated_only 计划推入 executing；执行器也不得 + // 对 validated_only 步骤做真实副作用——安全契约由代码硬拒，不依赖 + //「没人点 Confirm」。 + ErrPlanModeNotConfirmable = errors.New("PLAN_MODE_NOT_CONFIRMABLE") + ErrPlanModeNotExecutable = errors.New("PLAN_MODE_NOT_EXECUTABLE") ) var errTurnExists = errors.New("intentflow: turn exists") @@ -33,6 +39,10 @@ func ErrorCode(err error) string { return "PLAN_ACTION_FORBIDDEN" case errors.Is(err, ErrPlanActionVersionConflict): return "PLAN_ACTION_VERSION_CONFLICT" + case errors.Is(err, ErrPlanModeNotConfirmable): + return "PLAN_MODE_NOT_CONFIRMABLE" + case errors.Is(err, ErrPlanModeNotExecutable): + return "PLAN_MODE_NOT_EXECUTABLE" case errors.Is(err, ErrUnauthenticated): return "ERR_UNAUTHENTICATED" case errors.Is(err, ErrSegmentProposalDisabled): diff --git a/internal/app/intentflow/flags.go b/internal/app/intentflow/flags.go index 1b655fd2..32797a01 100644 --- a/internal/app/intentflow/flags.go +++ b/internal/app/intentflow/flags.go @@ -1,9 +1,12 @@ package intentflow import ( + "fmt" "os" "strings" "sync" + + "chat-agent-system/internal/intent" ) // PlanTier is the intentflow rollout switch. It must not be stored on @@ -123,3 +126,17 @@ func tierRank(t PlanTier) int { return 0 } } + +// ValidateFlagConsistency 校验 intentflow 放量档位与 intent 多意图开关的 +// 一致性（FIND-QUAL-01 回归）：PlanTier 升档必须建立在 MULTI_INTENT_ENABLED +// 之上，否则组合根启动期 fail-fast，不允许「intentflow 已升档、intent 仍关」 +// 这类静默漂移配置上线。 +func ValidateFlagConsistency(iflags intent.FeatureFlags, pflags FeatureFlags) error { + if pflags.PlanTier != TierLegacy && !iflags.MultiIntentEnabled { + return fmt.Errorf( + "intentflow: MULTI_INTENT_PLAN_TIER=%q requires MULTI_INTENT_ENABLED=true; fix the flag combination instead of silently degrading", + pflags.PlanTier, + ) + } + return nil +} diff --git a/internal/app/intentflow/flags_test.go b/internal/app/intentflow/flags_test.go index 93020023..ae421a86 100644 --- a/internal/app/intentflow/flags_test.go +++ b/internal/app/intentflow/flags_test.go @@ -2,6 +2,8 @@ package intentflow import ( "testing" + + "chat-agent-system/internal/intent" ) func TestDefaultPlanTierIsLegacy(t *testing.T) { @@ -134,3 +136,21 @@ func TestRESTPathsFrozen(t *testing.T) { t.Fatal(PathPlanDecisions) } } + +// TestValidateFlagConsistency FIND-QUAL-01 回归：PlanTier 升档但多意图识别 +// 关闭时组合根必须 fail-fast，不允许静默漂移配置。 +func TestValidateFlagConsistency(t *testing.T) { + base := intent.FeatureFlags{MultiIntentEnabled: true} + if err := ValidateFlagConsistency(base, FeatureFlags{PlanTier: TierFull}); err != nil { + t.Fatalf("elevated tier with multi-intent on should pass: %v", err) + } + if err := ValidateFlagConsistency(base, FeatureFlags{PlanTier: TierLegacy}); err != nil { + t.Fatalf("legacy tier should always pass: %v", err) + } + drifted := intent.FeatureFlags{MultiIntentEnabled: false} + for _, tier := range []PlanTier{TierShadow, TierAutoRead, TierConfirm, TierFull} { + if err := ValidateFlagConsistency(drifted, FeatureFlags{PlanTier: tier}); err == nil { + t.Fatalf("tier %s with multi-intent off must fail fast", tier) + } + } +} diff --git a/internal/app/intentflow/plans.go b/internal/app/intentflow/plans.go index c60e4a24..3ca5e68a 100644 --- a/internal/app/intentflow/plans.go +++ b/internal/app/intentflow/plans.go @@ -364,6 +364,11 @@ func (p *Plans) DecidePlan(ctx context.Context, in DecisionCommand) (*PlanView, switch in.Kind { case DecisionConfirm, DecisionConfirmRetry, DecisionContinue: if in.Kind == DecisionConfirm { + // FIND-FLOW-01 回归：validated_only（shadow/dry-run）计划不允许 + // 经 Confirm 进入 executing；shadow 安全不依赖「没人点 Confirm」。 + if plan.ExecutionMode == ModeValidatedOnly { + return ErrPlanModeNotConfirmable + } if plan.Status != PlanAwaitingConfirmation && plan.Status != PlanAwaitingClarification && plan.Status != PlanProposed { return ErrPlanStaleRevision } @@ -618,6 +623,16 @@ func (p *Plans) ExecuteClaimed(ctx context.Context, claimed *ClaimedStep) (*Plan if p.owner == nil { return nil, ErrPlanActionForbidden } + // FIND-FLOW-01 回归：执行器硬读 ExecutionMode——validated_only 步骤 + // 一律拒绝真实执行，无论上游状态机如何推进。步骤级 mode 优先， + // 缺省回退计划级。 + effectiveMode := claimed.Plan.ExecutionMode + if claimed.Step.ExecutionMode != "" { + effectiveMode = claimed.Step.ExecutionMode + } + if effectiveMode == ModeValidatedOnly { + return nil, ErrPlanModeNotExecutable + } exec := StepExec{ Principal: claimed.Plan.Principal, PlanID: claimed.Plan.PlanID, diff --git a/internal/app/intentflow/plans_test.go b/internal/app/intentflow/plans_test.go index 708936a0..7bbae441 100644 --- a/internal/app/intentflow/plans_test.go +++ b/internal/app/intentflow/plans_test.go @@ -40,6 +40,9 @@ func draft(digest string) intentflow.CreateCommand { RequestDigest: digest, WritePlan: true, SafeSummary: "query then generate", + // 这些用例验证 live 确认/派发机制；CreateCommand 缺省 mode 是 + // validated_only（安全默认，不可 Confirm），必须显式声明 live。 + ExecutionMode: intentflow.ModeLive, Steps: []intentflow.StepDraft{ {ActionKey: "regulation.query", ActionDigest: "q1"}, {ActionKey: "task.run", ActionDigest: "w1"}, @@ -319,3 +322,73 @@ func TestUnauthenticatedRejected(t *testing.T) { t.Fatalf("got %v", err) } } + +// TestConfirmRejectedForValidatedOnlyPlan FIND-FLOW-01 回归： +// validated_only（shadow）的 proposed 计划不允许经 Confirm 进入 executing。 +func TestConfirmRejectedForValidatedOnlyPlan(t *testing.T) { + p, _, _ := testPlans() + ctx := context.Background() + cmd := draft("digest-vo") + cmd.Status = intentflow.PlanProposed + cmd.ExecutionMode = intentflow.ModeValidatedOnly + created, err := p.CreatePlan(ctx, cmd) + if err != nil { + t.Fatal(err) + } + _, err = p.DecidePlan(ctx, intentflow.DecisionCommand{ + Principal: principal(), + PlanID: created.PlanID, + ExpectedRevision: created.Revision, + DecisionNonce: "n-vo", + DecisionDigest: "d-vo", + Kind: intentflow.DecisionConfirm, + }) + if !errors.Is(err, intentflow.ErrPlanModeNotConfirmable) { + t.Fatalf("confirm on validated_only plan must be rejected with ErrPlanModeNotConfirmable, got %v", err) + } +} + +// TestExecuteClaimedRefusesValidatedOnlyStep FIND-FLOW-01 回归： +// 执行器硬读 ExecutionMode——validated_only 计划即便被误推进到 executing +// 并经 DispatchEligible 派发，也拒绝真实执行（同 http-confirm 派发入口）。 +func TestExecuteClaimedRefusesValidatedOnlyStep(t *testing.T) { + p, _, owner := testPlans() + ctx := context.Background() + cmd := draft("digest-vo-exec") + // 直接构造「上游状态机被误推进」的形状：executing + validated_only。 + cmd.Status = intentflow.PlanExecuting + cmd.ExecutionMode = intentflow.ModeValidatedOnly + created, err := p.CreatePlan(ctx, cmd) + if err != nil { + t.Fatal(err) + } + // DecisionContinue 允许 executing 计划推进（applyConfirm 将步骤置为 + // eligible），模拟状态机被误推进后的派发路径。 + _, err = p.DecidePlan(ctx, intentflow.DecisionCommand{ + Principal: principal(), + PlanID: created.PlanID, + ExpectedRevision: created.Revision, + DecisionNonce: "n-vo-exec", + DecisionDigest: "d-vo-exec", + Kind: intentflow.DecisionContinue, + }) + if err != nil { + t.Fatal(err) + } + _, err = p.DispatchEligible(ctx, "worker-vo", created.PlanID) + if !errors.Is(err, intentflow.ErrPlanModeNotExecutable) { + t.Fatalf("dispatch of validated_only plan must hard-refuse with ErrPlanModeNotExecutable, got %v", err) + } + plan, err := p.GetPlan(ctx, principal(), created.PlanID) + if err != nil || plan == nil { + t.Fatalf("get plan: %v %v", plan, err) + } + for _, s := range plan.Steps { + if s.Status == intentflow.StepSucceeded { + t.Fatalf("validated_only step must never be written as succeeded: %+v", s) + } + if owner.ExecCount(s.StepID) != 0 { + t.Fatalf("owner must not observe real execution for %s", s.StepID) + } + } +} diff --git a/internal/archtest/arch_test_test.go b/internal/archtest/arch_test_test.go index e453a3aa..604a642f 100644 --- a/internal/archtest/arch_test_test.go +++ b/internal/archtest/arch_test_test.go @@ -3,6 +3,7 @@ package archtest import ( "path/filepath" "runtime" + "strings" "testing" "time" ) @@ -83,6 +84,40 @@ func TestOwnerProductionWriteIsAllowed(t *testing.T) { } } +// TestUntrackedTableWriteIsBlocked FIND-ARCH-01 回归：未登记表的生产写 +// 不再被静默跳过，必须产生 sql_write finding（fail-closed）。 +func TestUntrackedTableWriteIsBlocked(t *testing.T) { + var hit *Finding + findings := scanMini(t) + for i, f := range findings { + if f.Kind == KindSQLWrite && f.Symbol == "brand_new_table" && f.Package == "server" { + hit = &findings[i] + if f.File != "internal/server/untracked_write.go" { + t.Fatalf("expected untracked_write.go, got %s", f.File) + } + if !strings.Contains(f.Detail, "untracked table") { + t.Fatalf("expected untracked-table detail, got %q", f.Detail) + } + } + } + if hit == nil { + t.Fatal("expected INSERT INTO brand_new_table in internal/server/untracked_write.go to be blocked as untracked") + } +} + +// TestUpsertSetKeywordIsNotFlaggedAsUntracked 证明 ON CONFLICT DO UPDATE SET +// 等正则假阳性词汇不会因 fail-closed 改动而误报。 +func TestUpsertSetKeywordIsNotFlaggedAsUntracked(t *testing.T) { + for _, f := range scanMini(t) { + if f.Kind == KindSQLWrite && f.Symbol == "set" { + t.Fatalf("upsert SET keyword must stay exempt noise, got finding: %+v", f) + } + if f.Kind == KindSQLWrite && f.Package == "docgov" && f.File == "internal/docgov/owner_upsert_write.go" { + t.Fatalf("owner upsert must stay allowed (incl. SET noise): %+v", f) + } + } +} + func TestPathJoinConcatAndConstAreDetected(t *testing.T) { want := map[string]bool{ "internal/server/path_join.go": false, diff --git a/internal/archtest/scan.go b/internal/archtest/scan.go index 1b11f31c..dacd9dcc 100644 --- a/internal/archtest/scan.go +++ b/internal/archtest/scan.go @@ -18,6 +18,15 @@ var ( eventNameRe = regexp.MustCompile(`(?:document|intent\.plan)\.[a-z0-9_.]+`) ) +// sqlScanNoise 扫描正则的已知假阳性词汇：来自 SQL 子句（ON CONFLICT DO +// UPDATE SET、FOR UPDATE OF/SKIP LOCKED）与注释散文，不是真实表名。 +// 它们不是策略豁免：新出现的未登记表仍会 fail-closed。 +var sqlScanNoise = map[string]bool{ + "set": true, "of": true, "skip": true, "below": true, "row": true, + "state": true, "step": true, "doc": true, "docgov": true, + "message": true, "project": true, "aggregation": true, "ref_count": true, +} + type fileClass int const ( @@ -201,6 +210,17 @@ func scanSQLWrites(src, pkg, rel string, b *Boundaries) []Finding { table := strings.ToLower(m[2]) owners, tracked := b.TableOwners[table] if !tracked { + // FIND-ARCH-01 回归：未登记表不再静默跳过（fail-closed）。 + // 仅豁免扫描正则的已知假阳性词汇；真实新表必须登记 table_owners。 + if !sqlScanNoise[table] { + out = append(out, Finding{ + Kind: KindSQLWrite, + Package: pkg, + File: rel, + Symbol: table, + Detail: fmt.Sprintf("write to untracked table %s: register it in module-boundaries.yaml table_owners", table), + }) + } continue } if allowedHas(owners, pkg) { diff --git a/internal/archtest/testdata/mini/internal/docgov/owner_upsert_write.go b/internal/archtest/testdata/mini/internal/docgov/owner_upsert_write.go new file mode 100644 index 00000000..f9cf41c3 --- /dev/null +++ b/internal/archtest/testdata/mini/internal/docgov/owner_upsert_write.go @@ -0,0 +1,9 @@ +package docgov + +// ownerUpsertNoise includes ON CONFLICT DO UPDATE SET prose that the regex +// misreads as a table named "set"; it must stay exempt noise, not a finding, +// and the owner write on document_records itself stays allowed. +const ownerUpsertNoise = ` +INSERT INTO document_records (doc_id, stage) VALUES ($1, $2) +ON CONFLICT (doc_id) DO UPDATE SET stage = EXCLUDED.stage +` diff --git a/internal/archtest/testdata/mini/internal/server/untracked_write.go b/internal/archtest/testdata/mini/internal/server/untracked_write.go new file mode 100644 index 00000000..37cd7da1 --- /dev/null +++ b/internal/archtest/testdata/mini/internal/server/untracked_write.go @@ -0,0 +1,7 @@ +package server + +// brand_new_table is intentionally NOT registered in fixtureBoundaries +// table_owners: writing it must produce an untracked-table finding. +const untrackedSQL = ` +INSERT INTO brand_new_table (id, payload) VALUES ($1, $2) +` diff --git a/internal/authz/authorizer.go b/internal/authz/authorizer.go index 0be560ff..6c3abd30 100644 --- a/internal/authz/authorizer.go +++ b/internal/authz/authorizer.go @@ -62,12 +62,22 @@ type DutySlotWriter interface { type Authorizer struct { pool *pgxpool.Pool dutySlots DutySlotWriter + // inMemory 显式声明「无库演示模式」：仅 NewInMemoryAuthorizer 构建。 + // NewAuthorizer(nil) 一律严格拒绝（FIND-SEC-02 回归），防止误配 + // （无库且未显式 ALLOW_IN_MEMORY）时任意登录用户获得项目访问。 + inMemory bool } func NewAuthorizer(pool *pgxpool.Pool) *Authorizer { return &Authorizer{pool: pool} } +// NewInMemoryAuthorizer 仅供显式 ALLOW_IN_MEMORY 演示/测试模式：无库时 +// 视任意登录用户为项目成员（单用户演示语义）。生产/DB 路径禁止使用。 +func NewInMemoryAuthorizer() *Authorizer { + return &Authorizer{inMemory: true} +} + // SetDutySlotWriter injects the document_records slot clearer (docgov adapter). func (a *Authorizer) SetDutySlotWriter(w DutySlotWriter) { if a == nil { @@ -105,12 +115,14 @@ func (a *Authorizer) AddOwnerOnCreate(ctx context.Context, projectID, userID int } // IsProjectMember 判断用户是否为项目 active 成员或创建人。 +// FIND-SEC-02 回归：无池（且非显式内存模式）时 fail-closed 返回 false， +// 不再默认放行任意登录用户；与 GetProjectMember 的禁用语义对齐。 func (a *Authorizer) IsProjectMember(ctx context.Context, projectID, userID int64) (bool, error) { if projectID <= 0 || userID <= 0 { return false, nil } if !a.enabled() { - return userID > 0, nil + return a.inMemory && userID > 0, nil } var member bool err := a.pool.QueryRow(ctx, ` @@ -198,12 +210,13 @@ func validateProjectRole(role string) error { } // IsProjectOwner 判断用户是否为项目 owner（成员表或创建人）。 +// FIND-SEC-02 回归：无池时 fail-closed，同 IsProjectMember。 func (a *Authorizer) IsProjectOwner(ctx context.Context, projectID, userID int64) (bool, error) { if projectID <= 0 || userID <= 0 { return false, nil } if !a.enabled() { - return userID > 0, nil + return a.inMemory && userID > 0, nil } var owner bool err := a.pool.QueryRow(ctx, ` diff --git a/internal/authz/service_test.go b/internal/authz/service_test.go index 13e38b70..800d563b 100644 --- a/internal/authz/service_test.go +++ b/internal/authz/service_test.go @@ -101,3 +101,21 @@ func TestGetProjectMember_DisabledWithoutPool(t *testing.T) { t.Fatalf("disabled authorizer must not invent a member, got (%v, %v)", rec, err) } } + +// TestMembershipChecks_FailClosedWithoutPool FIND-SEC-02 回归： +// 无池 Authorizer 的成员/owner 判定必须 fail-closed，不得放行任意登录用户。 +func TestMembershipChecks_FailClosedWithoutPool(t *testing.T) { + a := authz.NewAuthorizer(nil) + for name, fn := range map[string]func() (bool, error){ + "IsProjectMember": func() (bool, error) { return a.IsProjectMember(context.Background(), 1, 2) }, + "IsProjectOwner": func() (bool, error) { return a.IsProjectOwner(context.Background(), 1, 2) }, + } { + ok, err := fn() + if err != nil || ok { + t.Fatalf("%s without pool must fail closed (false, nil), got (%v, %v)", name, ok, err) + } + } + if err := a.RequireProjectAccess(context.Background(), 1, 2); err == nil { + t.Fatalf("RequireProjectAccess without pool must deny, got nil error") + } +} diff --git a/internal/ctxmgr/manager.go b/internal/ctxmgr/manager.go index 6281d6be..1b14befc 100644 --- a/internal/ctxmgr/manager.go +++ b/internal/ctxmgr/manager.go @@ -374,15 +374,20 @@ func (m *ContextManager) GetUserContext(ctx context.Context, projectID, userID i AssembledAt: time.Now(), } - // 1. 获取近期会话 + // 1. 获取近期会话（FIND-SEC-01 防御纵深：SQL 层限定会话归属目标用户， + // 即使未来新增调用方遗漏 handler 校验也不会泄露他人会话正文） if sessionID != "" { if m.pool != nil { rows, err := m.pool.Query(ctx, ` SELECT id, session_id, COALESCE(user_id, 0), role, content, intent, branch, metadata, created_at FROM chat_messages WHERE session_id = $1 + AND EXISTS ( + SELECT 1 FROM chat_sessions cs + WHERE cs.session_id = $1 AND COALESCE(cs.user_id, 0) = $2 + ) ORDER BY created_at ASC LIMIT 30 - `, sessionID) + `, sessionID, userID) if err == nil { defer rows.Close() for rows.Next() { @@ -399,7 +404,9 @@ func (m *ContextManager) GetUserContext(ctx context.Context, projectID, userID i } } else { m.mu.RLock() - userCtx.RecentChats = append(userCtx.RecentChats, m.messages[sessionID]...) + if sess, ok := m.sessions[sessionID]; ok && sess.UserID == userID { + userCtx.RecentChats = append(userCtx.RecentChats, m.messages[sessionID]...) + } m.mu.RUnlock() } } diff --git a/internal/server/document_revision_duty_write_test.go b/internal/server/document_revision_duty_write_test.go index c4821778..5977c3e8 100644 --- a/internal/server/document_revision_duty_write_test.go +++ b/internal/server/document_revision_duty_write_test.go @@ -13,8 +13,9 @@ import ( func TestAuthorizeDocRevision_ReviewerAndNonMemberCannotWrite(t *testing.T) { srv := newTestServer(t) // 隔离写权门闩：跳过项目成员库检查，只验证 AuthorID。禁止依赖共享库或默认拒绝。 + // FIND-SEC-02 后 NewAuthorizer(nil) 严格拒绝，改用显式内存授权接缝。 srv.cfg.AllowInMemory = true - srv.setOverrideAuthorizer(authz.NewAuthorizer(nil)) + srv.setOverrideAuthorizer(authz.NewInMemoryAuthorizer()) ctx := context.Background() const ( authorID int64 = 170 diff --git a/internal/server/handlers_context.go b/internal/server/handlers_context.go index fe755bf2..d2f1a114 100644 --- a/internal/server/handlers_context.go +++ b/internal/server/handlers_context.go @@ -41,6 +41,19 @@ func (s *Server) handleGetUserContext(w http.ResponseWriter, r *http.Request) { return } + // FIND-SEC-01 回归：session_id 必须归属于目标用户本人，否则拒绝。 + // 会话不存在与归属他人同样返回 403，避免泄露会话存在性。 + if sessionID != "" { + ownerID, err := s.ctxMgr().GetSessionOwnerID(r.Context(), sessionID) + if err != nil || ownerID <= 0 || ownerID != userID { + writeJSONResponse(w, http.StatusForbidden, map[string]string{ + "error_code": "ERR_FORBIDDEN", + "message": "无权访问该会话上下文", + }) + return + } + } + uCtx, err := s.ctxMgr().GetUserContext(r.Context(), projectID, userID, sessionID) if err != nil { writeSafeError(w, r, http.StatusInternalServerError, "get user context failed: "+err.Error()) diff --git a/internal/server/idor_hotfixes_test.go b/internal/server/idor_hotfixes_test.go index bab75ab6..e92ea89d 100644 --- a/internal/server/idor_hotfixes_test.go +++ b/internal/server/idor_hotfixes_test.go @@ -2,9 +2,11 @@ package server_test import ( "bytes" + "context" "encoding/json" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -142,3 +144,125 @@ func TestChatRecategorize_RequiresOwnedSession(t *testing.T) { t.Fatalf("expected 200 for owned session, got %d %s", ownRec.Code, ownRec.Body.String()) } } + +// TestGetUserContext_OtherUserSessionDenied FIND-SEC-01 回归： +// GetUserContext 旁路按 session_id 取消息必须校验会话归属。 +func TestGetUserContext_OtherUserSessionDenied(t *testing.T) { + cfg := &config.Config{DataDir: t.TempDir(), AgentBackend: "stub", AdminUsername: "admin", AdminPassword: "admin-test-pass"} + svc := bizdev.MustNewService(t, cfg) + srv := server.NewServer(cfg, svc) + handler := srv.Routes() + adminToken := loginTestAdmin(t, handler, cfg) + + suffix := time.Now().Format("150405.000") + victimToken := createAndLoginUser(t, handler, adminToken, "ctx_victim_"+suffix, "pass12345") + attackerToken := createAndLoginUser(t, handler, adminToken, "ctx_attacker_"+suffix, "pass12345") + sessionID := "sess-ctx-idor-" + suffix + + chatBody, _ := json.Marshal(map[string]any{ + "session_id": sessionID, + "message": "victim-private-context-body", + }) + req := httptest.NewRequest(http.MethodPost, "/api/v1/chat", bytes.NewReader(chatBody)) + req.Header.Set("Content-Type", "application/json") + withBearerAuth(req, victimToken) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("victim chat: %d %s", rec.Code, rec.Body.String()) + } + + // 攻击者用自己的 user_id + 受害者 session_id：必须 403。 + getReq := httptest.NewRequest(http.MethodGet, "/api/v1/context/user?session_id="+sessionID, nil) + withBearerAuth(getReq, attackerToken) + getRec := httptest.NewRecorder() + handler.ServeHTTP(getRec, getReq) + if getRec.Code != http.StatusForbidden { + t.Fatalf("expected 403 for other user's session context, got %d %s", getRec.Code, getRec.Body.String()) + } + if strings.Contains(getRec.Body.String(), "victim-private-context-body") { + t.Fatalf("response must not leak victim messages: %s", getRec.Body.String()) + } + + // 不存在的会话同样 403（不泄露存在性）。 + ghostReq := httptest.NewRequest(http.MethodGet, "/api/v1/context/user?session_id=sess-ctx-idor-ghost", nil) + withBearerAuth(ghostReq, attackerToken) + ghostRec := httptest.NewRecorder() + handler.ServeHTTP(ghostRec, ghostReq) + if ghostRec.Code != http.StatusForbidden { + t.Fatalf("expected 403 for unknown session, got %d %s", ghostRec.Code, ghostRec.Body.String()) + } + + // 会话归属者本人仍可正常取回上下文。 + ownReq := httptest.NewRequest(http.MethodGet, "/api/v1/context/user?session_id="+sessionID, nil) + withBearerAuth(ownReq, victimToken) + ownRec := httptest.NewRecorder() + handler.ServeHTTP(ownRec, ownReq) + if ownRec.Code != http.StatusOK { + t.Fatalf("owner context read failed: %d %s", ownRec.Code, ownRec.Body.String()) + } + if !strings.Contains(ownRec.Body.String(), "victim-private-context-body") { + t.Fatalf("owner should see own session messages: %s", ownRec.Body.String()) + } +} + +// TestGetUserContext_PG_SessionOwnerFilter FIND-SEC-01 PG 层防御纵深回归： +// GetUserContext 的 chat_messages 查询必须按会话归属过滤，DB 层也不泄露 +// 他人会话正文（即使未来调用方遗漏 handler 校验）。 +func TestGetUserContext_PG_SessionOwnerFilter(t *testing.T) { + pool := openPGPoolWithMigrations(t) + if pool == nil { + t.Skip("PG not available") + } + defer pool.Close() + + ctx := context.Background() + handler, srv := setupPGGovernanceServer(t, pool) + ctxMgr := bizdev.Snapshot(srv.BizdevService()).Ctx + if ctxMgr == nil { + t.Fatal("context manager required") + } + + ownerToken := loginTestUser(t, handler, "zhanggong", "123456") + ownerID := fetchAuthUserID(t, handler, ownerToken) + otherToken := loginTestUser(t, handler, "lizhuguan", "123456") + otherID := fetchAuthUserID(t, handler, otherToken) + + suffix := time.Now().Format("150405.000") + sessionID := "sess-pg-ctx-idor-" + suffix + if _, err := ctxMgr.EnsureChatSession(ctx, sessionID, ownerID, 0, "PG IDOR 回归"); err != nil { + t.Fatalf("ensure session: %v", err) + } + if _, err := pool.Exec(ctx, ` + INSERT INTO chat_messages (session_id, user_id, role, content, created_at) + VALUES ($1, $2, 'user', 'pg-owner-private-body', now()) + `, sessionID, ownerID); err != nil { + t.Fatalf("seed message: %v", err) + } + + // 他人视角：manager 层不返回会话正文。 + foreign, err := ctxMgr.GetUserContext(ctx, 0, otherID, sessionID) + if err != nil { + t.Fatal(err) + } + for _, m := range foreign.RecentChats { + if strings.Contains(m.Content, "pg-owner-private-body") { + t.Fatalf("foreign user must not read owner messages: %+v", m) + } + } + + // 归属者视角：正常取回。 + own, err := ctxMgr.GetUserContext(ctx, 0, ownerID, sessionID) + if err != nil { + t.Fatal(err) + } + found := false + for _, m := range own.RecentChats { + if strings.Contains(m.Content, "pg-owner-private-body") { + found = true + } + } + if !found { + t.Fatal("owner should read own session messages") + } +} diff --git a/internal/server/intent_plan_handlers.go b/internal/server/intent_plan_handlers.go index 17c3d2cd..d0ce29c8 100644 --- a/internal/server/intent_plan_handlers.go +++ b/internal/server/intent_plan_handlers.go @@ -189,6 +189,10 @@ func writeIntentPlanError(w http.ResponseWriter, r *http.Request, plans intentfl status = http.StatusNotFound case errors.Is(err, intentflow.ErrPlanActionForbidden): status = http.StatusForbidden + case errors.Is(err, intentflow.ErrPlanModeNotConfirmable), + errors.Is(err, intentflow.ErrPlanModeNotExecutable): + // validated_only（shadow/dry-run）计划的硬边界：该动作对这类计划永远不可用。 + status = http.StatusForbidden case errors.Is(err, intentflow.ErrPlanStaleRevision), errors.Is(err, intentflow.ErrPlanExpired), errors.Is(err, intentflow.ErrPlanDecisionConflict), diff --git a/internal/server/intent_plan_handlers_test.go b/internal/server/intent_plan_handlers_test.go index ea5ed540..b82d47ac 100644 --- a/internal/server/intent_plan_handlers_test.go +++ b/internal/server/intent_plan_handlers_test.go @@ -86,6 +86,8 @@ func u5Create(t *testing.T, plans *intentflow.Plans, userID int64, projectID int RequestDigest: turn + "-digest", WritePlan: true, SafeSummary: "query then generate", + // 本组用例验证 live 确认/回执语义；缺省 validated_only 不可 Confirm。 + ExecutionMode: intentflow.ModeLive, Steps: []intentflow.StepDraft{ {ActionKey: "regulation.query", ActionDigest: "q1"}, {ActionKey: "task.run", ActionDigest: "w1"}, diff --git a/internal/server/project_handlers_test.go b/internal/server/project_handlers_test.go index 6678f68e..2fe3561b 100644 --- a/internal/server/project_handlers_test.go +++ b/internal/server/project_handlers_test.go @@ -56,6 +56,8 @@ func TestHandleRunProjectWorkflow_DecodesBodyWithoutContentLength(t *testing.T) cfg := &config.Config{ DataDir: t.TempDir(), AgentBackend: "stub", + // FIND-SEC-02: 业务行为用例走显式内存授权（无库模式项目访问需显式声明）。 + AllowInMemory: true, } svc := bizdev.MustNewService(t, cfg) srv := NewServer(cfg, svc) diff --git a/internal/server/task_run_busy_preflight_test.go b/internal/server/task_run_busy_preflight_test.go index c3e47a40..bdc1c595 100644 --- a/internal/server/task_run_busy_preflight_test.go +++ b/internal/server/task_run_busy_preflight_test.go @@ -14,7 +14,7 @@ import ( ) func TestCreateTaskRunBusyPreflightDoesNotPersistRunOrMessages(t *testing.T) { - srv := newTestServer(t) + srv := newServerWithMemoryIAM(t, nil) // FIND-SEC-02: 业务行为用例走显式内存授权 ctx := t.Context() docID := "doc_proj143_intake" if _, err := srv.gov().CreateOrGetDocument( @@ -74,7 +74,7 @@ func TestCreateTaskRunBusyPreflightDoesNotPersistRunOrMessages(t *testing.T) { // 从未激活项目：任务作用域为 pre_activation，创建 run 必须 202 而非 TASK_SCOPE_UNRESOLVED 409。 func TestCreateTaskRun_PreActivationAcceptedNot409(t *testing.T) { - srv := newTestServer(t) + srv := newServerWithMemoryIAM(t, nil) // FIND-SEC-02: 业务行为用例走显式内存授权 store := projectinstance.NewMemoryStore() _ = projectinstance.SeedProjectMeta(store, 9001, projectinstance.LifecyclePlanning, "", 1, 0) srv.setOverrideInstSvc(projectinstance.NewService(store, projectinstance.TestResolver(), nil, nil)) @@ -127,7 +127,7 @@ func TestCreateTaskRun_PreActivationAcceptedNot409(t *testing.T) { } func TestCreateTaskRun_ScopeUnresolvedPersistsChat(t *testing.T) { - srv := newTestServer(t) + srv := newServerWithMemoryIAM(t, nil) // FIND-SEC-02: 业务行为用例走显式内存授权 token := loginTestAdmin(t, srv) srv.cfg.AllowInMemory = false srv.setOverrideInstSvc(nil) diff --git a/internal/server/task_run_idempotent_sse_test.go b/internal/server/task_run_idempotent_sse_test.go index 7ea9835c..321a0835 100644 --- a/internal/server/task_run_idempotent_sse_test.go +++ b/internal/server/task_run_idempotent_sse_test.go @@ -19,7 +19,7 @@ import ( ) func TestCreateTaskRun_SameTurnIdempotentReturnsExistingDespiteBusyLease(t *testing.T) { - srv := newTestServer(t) + srv := newServerWithMemoryIAM(t, nil) // FIND-SEC-02: 业务行为用例走显式内存授权 ctx := t.Context() token := loginTestAdmin(t, srv) userID := int64(1) diff --git a/internal/swboot/executor.go b/internal/swboot/executor.go index 36435c7e..4dd564c5 100644 --- a/internal/swboot/executor.go +++ b/internal/swboot/executor.go @@ -147,9 +147,9 @@ func (e *ControlledPatchExecutor) Execute(ctx context.Context, req ControlledPat } res.Files = files - allowedPrefixes := e.Allowlist.AllowedCommands(res.RepoPath) + allowedNames := e.Allowlist.AllowedCommands(res.RepoPath) for _, cmd := range req.Commands { - if !commandMatchesPrefixes(cmd, allowedPrefixes) { + if !commandAllowed(cmd, allowedNames) { return fail(fmt.Errorf("swboot: command not in admin allowlist: %.80s", cmd)) } rec, err := e.runCommand(ctx, wtDir, cmd) @@ -176,10 +176,15 @@ func (e *ControlledPatchExecutor) runCommand(ctx context.Context, dir, cmdLine s maxOut = defaultMaxOutputBytes } rec := ExecutionCommand{Command: cmdLine} + // FIND-SEC-03 回归：argv 化执行，不经 shell。命令行中的元字符 + //（;、&&、|、$() 等）只会成为 argv 参数，不会被求值。 + fields := strings.Fields(strings.TrimSpace(cmdLine)) + if len(fields) == 0 { + return rec, fmt.Errorf("swboot: empty command") + } cctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() - // sh -c with a sanitized environment: no secrets, no repo .env leakage. - c := exec.CommandContext(cctx, "sh", "-c", cmdLine) + c := exec.CommandContext(cctx, fields[0], fields[1:]...) c.Dir = dir c.Env = []string{ "PATH=" + os.Getenv("PATH"), @@ -301,18 +306,21 @@ func (e *ControlledPatchExecutor) persist(ctx context.Context, req ControlledPat } } -func commandMatchesPrefixes(cmd string, prefixes []string) bool { +// commandAllowed 判定命令是否在 admin 登记的命令名白名单内。 +// FIND-SEC-03 回归：只允许首段 basename 与白名单条目「精确相等」； +// 前缀匹配（go 放行 go-evil）与 shell 拼接均已移除。 +func commandAllowed(cmd string, allowed []string) bool { fields := strings.Fields(strings.TrimSpace(cmd)) if len(fields) == 0 { return false } seg := filepath.Base(fields[0]) - for _, p := range prefixes { + for _, p := range allowed { p = strings.TrimSpace(p) if p == "" { continue } - if seg == p || strings.HasPrefix(seg, p) { + if seg == p || seg == filepath.Base(p) { return true } } diff --git a/internal/swboot/executor_test.go b/internal/swboot/executor_test.go index 58d37bf1..dd0c1f49 100644 --- a/internal/swboot/executor_test.go +++ b/internal/swboot/executor_test.go @@ -160,3 +160,44 @@ func TestApproveMerge_NeverExecutes(t *testing.T) { t.Fatalf("merge must stay behind separate authorization: %v", err) } } + +// TestExecutor_CommandAllowlistIsExactNotPrefix FIND-SEC-03 回归： +// 白名单是精确命令名匹配，前缀伪装（echo 放行 echo-evil）必须被拒。 +func TestExecutor_CommandAllowlistIsExactNotPrefix(t *testing.T) { + if commandAllowed("echo-evil pwn", []string{"echo"}) { + t.Fatal("prefix spoof 'echo-evil' must not match allowlist entry 'echo'") + } + if !commandAllowed("echo ci-ok", []string{"echo"}) { + t.Fatal("exact command 'echo' must match") + } + if !commandAllowed("/usr/bin/echo hi", []string{"echo"}) { + t.Fatal("absolute-path basename must match entry 'echo'") + } + if commandAllowed("", []string{"echo"}) || commandAllowed(" ", []string{"echo"}) { + t.Fatal("empty command must be rejected") + } +} + +// TestExecutor_NoShellEvaluation FIND-SEC-03 回归：argv 化执行后，命令行中\n// 的 shell 元字符只作为字面参数传给命令，不会被 shell 求值。 +func TestExecutor_NoShellEvaluation(t *testing.T) { + repo := initTestRepo(t) + marker := filepath.Join(t.TempDir(), "sh-pwn-marker") + e := newExecutor(&fakeAllowlist{ + paths: map[string]bool{repo: true}, + commands: map[string][]string{repo: {"echo"}}, + }) + res, err := e.Execute(context.Background(), ControlledPatchRequest{ + ProjectID: 11, RepoPath: repo, BaseRef: "HEAD", + Patches: []string{samplePatch}, + Commands: []string{"echo x && touch " + marker}, + }) + if err != nil || res.Status != "succeeded" { + t.Fatalf("echo with literal metachar args should succeed: %+v %v", res, err) + } + if len(res.Commands) != 1 || !strings.Contains(res.Commands[0].Output, "x && touch") { + t.Fatalf("output should be literal echo args: %+v", res.Commands) + } + if _, statErr := os.Stat(marker); statErr == nil { + t.Fatal("shell metacharacters must not be evaluated; marker file was created") + } +} From ea66f4456faf290f2447c343986f21edf469c49e Mon Sep 17 00:00:00 2001 From: weichen Date: Sat, 26 Sep 2026 02:00:24 +0000 Subject: [PATCH 4/6] docs: review P0/P1 fix records, ISSUES ledger, lesson, task claim - remediation master record for 5 resolved + 1 deferred + 1 open issues - ISSUES.md rows with evidence dates and lesson links - LESSON-20260926-failclosed-bypass-contract (validated) - remediation/lessons INDEX hooks + CURRENT task row (docs apply receipt) --- .doc/audit/ISSUES.md | 10 +- .doc/ops/CURRENT.md | 1 + .../2026-09-26-failclosed-bypass-contract.md | 56 +++++++++++ .doc/ops/lessons/INDEX.md | 1 + .../2026-09-26-system-review-fixes.md | 60 ++++++++++++ .../2026-09-26-system-code-review-fixes.md | 97 +++++++++++++++++++ .doc/remediation/INDEX.md | 1 + 7 files changed, 225 insertions(+), 1 deletion(-) create mode 100644 .doc/ops/lessons/2026-09-26-failclosed-bypass-contract.md create mode 100644 .doc/ops/short-term/2026-09-26-system-review-fixes.md create mode 100644 .doc/remediation/2026-09-26-system-code-review-fixes.md diff --git a/.doc/audit/ISSUES.md b/.doc/audit/ISSUES.md index 06db4c52..ab30954f 100644 --- a/.doc/audit/ISSUES.md +++ b/.doc/audit/ISSUES.md @@ -2,7 +2,7 @@ 维护规则：[问题记录与开发经验闭环](../ops/2026-09-23-issue-learning-workflow.md)。每个问题一个稳定 ID；主记录是事实与状态来源，本表为可检索投影。新发现及时登记，修复后保留记录；发布 blocker 仍见 [gap-matrix](../remediation/gap-matrix.md)。 -更新：2026-09-26。docgov 批准链：登记并修复 review_snapshot_id 装载缺失与 uow 行锁自死锁（resolved）；memDocs 持久化失败缓存投毒（investigating）。历史 resolved 不表示本日产品回归通过。 +更新：2026-09-26。docgov 批准链：登记并修复 review_snapshot_id 装载缺失与 uow 行锁自死锁（resolved）；memDocs 持久化失败缓存投毒（investigating）。历史 resolved 不表示本日产品回归通过。系统 code review P0/P1 批次修复落账（5 resolved + 1 deferred + 1 open，主记录见 [整改文档](2026-09-26-system-code-review-fixes.md)，来源 [审计](2026-09-26-system-code-review.md)）。 | issue_id | 模块 / 检索词 | 问题 | 严重度 | issue_state | owner | 证据日期 / 范围 | 唯一主记录 | 经验 | |---|---|---|---|---|---|---|---|---| @@ -21,3 +21,11 @@ | ISSUE-20260926-docgov-snapshot-load | docgov / loadDocumentFromDBLocked / review_snapshot_id / 跨重启批准 | 文档装载 SELECT 漏 review_snapshot_id，重启/缓存未命中后最终批准必失败 | high | resolved | pi | 2026-09-26；隔离库 PG 回归 + dev 真实批准 200/state=approved（dev `121b33f8`） | [主记录](../test-reports/2026-09-26-rdev-p3-rerun-test-report.md#5-2) | 待补 LESSON | | ISSUE-20260926-docgov-uow-deadlock | docgov / persistApproveAndFAV / uow / FOR UPDATE / 57014 | docflow uow 行锁后 FAV 持久化自开第二连接更新同行，语句超时致死锁，所有最终批准失败 | critical | resolved | pi | 2026-09-26；生产形状 PG 回归（FOR UPDATE+FAV+Within）+ dev 真实批准转 200（dev `5d5cd5fd`） | [主记录](../test-reports/2026-09-26-rdev-p3-rerun-test-report.md#5-2) | 待补 LESSON | | ISSUE-20260926-docgov-memdocs-poison | docgov / memDocs / 状态机 / 持久化失败 / 缓存投毒 | 状态机先改内存后落库；持久化失败不复原，memDocs 缓存与 DB 永久分叉并掩盖真实错误（表现为「approved 状态」误报） | high | investigating | pi | 2026-09-26；dev 两次复现（快照/死锁两根因均被投毒掩盖）；修复方向：失败时回滚内存快照或失效缓存条目 | [主记录](../test-reports/2026-09-26-rdev-p3-rerun-test-report.md#5-2) | 待补 LESSON | +| ISSUE-20260926-ctx-idor | ctxmgr / GetUserContext / session_id / IDOR / handlers_context | GetUserContext 按 session_id 取消息无归属校验，登录用户可读他人会话正文（已证实） | critical | resolved | pi | 2026-09-26；handler 403 + SQL/内存双防线；内存 HTTP 与隔离 PG 回归 PASS | [主记录](../remediation/2026-09-26-system-code-review-fixes.md#1-find-sec-01-会话上下文-idorissue-20260926-ctx-idor) | [LESSON-20260926-failclosed-bypass-contract](../ops/lessons/2026-09-26-failclosed-bypass-contract.md) | +| ISSUE-20260926-archcheck-untracked | archtest / table_owners / untracked / sql_write / 门禁盲区 | arch-check 对未登记表 SQL 写静默跳过，含 swboot_controlled_executions 等 ~50 表；门禁形同关闭 | critical | resolved | pi | 2026-09-26；未登记表 fail-closed + 补登 50 表 + 噪音豁免清单；arch-check block PASS | [主记录](../remediation/2026-09-26-system-code-review-fixes.md#2-find-arch-01-arch-check-未登记表静默跳过issue-20260926-archcheck-untracked) | [LESSON-20260926-failclosed-bypass-contract](../ops/lessons/2026-09-26-failclosed-bypass-contract.md) | +| ISSUE-20260926-authz-failopen | authz / IsProjectMember / pool==nil / fail-open / NewInMemoryAuthorizer | Authorizer 无池时 IsProjectMember/IsProjectOwner 对任意登录用户返回 true，误配即全量放行 | high | resolved | pi | 2026-09-26；无池严格拒绝 + 显式内存授权接缝 + 组合根按 AllowInMemory 选择；隔离 PG authz 套件 PASS | [主记录](../remediation/2026-09-26-system-code-review-fixes.md#3-find-sec-02-authorizer-无池-fail-openissue-20260926-authz-failopen) | [LESSON-20260926-failclosed-bypass-contract](../ops/lessons/2026-09-26-failclosed-bypass-contract.md) | +| ISSUE-20260926-swboot-shell-prefix | swboot / executor / sh -c / 命令注入 / 前缀白名单 / commandAllowed | 受控命令经 sh -c 执行且白名单前缀匹配（go 放行 go-evil）；元字符可求值 | high | resolved | pi | 2026-09-26；argv 化 + basename 精确匹配；前缀伪装/不求值回归 PASS | [主记录](../remediation/2026-09-26-system-code-review-fixes.md#4-find-sec-03-swboot-sh--c--前缀白名单issue-20260926-swboot-shell-prefix) | [LESSON-20260926-failclosed-bypass-contract](../ops/lessons/2026-09-26-failclosed-bypass-contract.md) | +| ISSUE-20260926-intentflow-validated-confirm | intentflow / DecisionConfirm / PlanProposed / validated_only / ExecuteClaimed | validated_only（shadow）计划可经 Confirm 进入 executing 并真实执行；执行器不读 ExecutionMode | high | resolved | pi | 2026-09-26；Confirm 拒 + 执行器硬拒双层 + 错误码/HTTP 映射；确认/派发双回归 PASS | [主记录](../remediation/2026-09-26-system-code-review-fixes.md#5-find-flow-01-validated_only-可-confirm-后-dispatchissue-20260926-intentflow-validated-confirm) | [LESSON-20260926-failclosed-bypass-contract](../ops/lessons/2026-09-26-failclosed-bypass-contract.md) | +| ISSUE-20260926-dual-flags-drift | intent / intentflow / FeatureFlags / PlanTier / MULTI_INTENT_ENABLED / 静默漂移 | 双套 flags 可配置出「intentflow 升档但 intent 关闭」的矛盾组合并静默降级 | high | resolved | pi | 2026-09-26；组合根经 intentflow.ValidateFlagConsistency 启动期 fail-fast + 单测；入口收敛残差另立 deferred | [主记录](../remediation/2026-09-26-system-code-review-fixes.md#6-find-qual-01-双套-flags-静默漂移issue-20260926-dual-flags-drift含残差) | [LESSON-20260926-failclosed-bypass-contract](../ops/lessons/2026-09-26-failclosed-bypass-contract.md) | +| ISSUE-20260926-governance-multi-entry | docgov / 多意图 / 多入口 / Coordinator / OwnerMux / GovernanceEngine | docgov 三入口 + Coordinator ~40 case 与 intentflow OwnerMux 平行，鉴权/确认/审计易漂移 | high | deferred | pi | 2026-09-26；flag 漂移面已修（dual-flags-drift）；入口合并是多日级重构，需独立任务授权与回归矩阵 | [主记录](../remediation/2026-09-26-system-code-review-fixes.md#6-find-qual-01-双套-flags-静默漂移issue-20260926-dual-flags-drift含残差) | 沿用 failclosed-bypass-contract | +| ISSUE-20260926-server-pool-wiring | server / swboot_handlers / Runtime.Pool / 组合根越界 / FIND-ARCH-02 | server 持 Pool 直装 swboot repo/executor 并直查表，成为第二组合根（本轮未授权修复） | critical | open | pi | 2026-09-26；代码审证据齐（审计 FIND-ARCH-02），待修复授权 | [主记录](../audit/2026-09-26-system-code-review.md) | 待修复后补 | diff --git a/.doc/ops/CURRENT.md b/.doc/ops/CURRENT.md index febac83f..09b69837 100644 --- a/.doc/ops/CURRENT.md +++ b/.doc/ops/CURRENT.md @@ -18,6 +18,7 @@ | owner | task | branch | worktree | mutex | status | |---|---|---|---|---|---| +| pi | system-review-p0-p1-fixes-2026-09-26 | fix/system-review-p0-p1-fixes-2026-09-26 | .worktrees/system-review-p0-p1-fixes-2026-09-26 | review-fixes-batch | in_progress | | pi | swboot-closeout-2026-09-26-r3 | feature/swboot-closeout-2026-09-26 | .worktrees/swboot-closeout-2026-09-26 | workflow-design | done | | pi | swboot-closeout-2026-09-26-r2 | feature/swboot-closeout-2026-09-26 | .worktrees/swboot-closeout-2026-09-26 | workflow-design | in_progress | | pi | chat-semantic-replace-2026-09-26 | feature/chat-semantic-replace-2026-09-26 | .worktrees/chat-semantic-replace-2026-09-26 | chat | done | diff --git a/.doc/ops/lessons/2026-09-26-failclosed-bypass-contract.md b/.doc/ops/lessons/2026-09-26-failclosed-bypass-contract.md new file mode 100644 index 00000000..0bd6071c --- /dev/null +++ b/.doc/ops/lessons/2026-09-26-failclosed-bypass-contract.md @@ -0,0 +1,56 @@ +--- +lesson_id: LESSON-20260926-failclosed-bypass-contract +lesson_state: validated +created: 2026-09-26 +updated: 2026-09-26 +source_issues: + - ISSUE-20260926-ctx-idor + - ISSUE-20260926-archcheck-untracked + - ISSUE-20260926-authz-failopen + - ISSUE-20260926-swboot-shell-prefix + - ISSUE-20260926-intentflow-validated-confirm +reviewed_by: pi +last_reviewed: 2026-09-26 +reviewed_revision: "fix/system-review-p0-p1-fixes-2026-09-26" +keywords: [IDOR, 旁路, fail-open, fail-closed, 白名单前缀, sh -c, validated_only, table_owners, untracked, session_id, 归属校验] +--- + +# 旁路入口与「未知即拒绝」契约 + +## 触发条件 + +- 新增/评审任何**绕过主用例路径**的 HTTP handler（尤其按 `session_id`/`project_id`/`user_id` 直接取资源的旁路端点）。 +- 写门禁扫描器、授权器或执行器时出现「未登记/未配置/依赖缺失」分支。 +- 注释或文档声称某 flag/mode 是安全边界（如 `validated_only must never be written as succeeded`）。 +- 命令/路径白名单实现（前缀匹配、`sh -c` 拼接）。 + +## 机制与应做检查 + +1. **主路径绿 ≠ 旁路绿**：主路径（chatflow/Owner 等）的鉴权不会自动覆盖旁路 handler。每个按标识取资源的入口都要复用同一归属校验（如 `GetSessionOwnerID`），并在 SQL/存储层再加归属条件作防御纵深——handler 层校验可能被未来调用方遗漏。 +2. **门禁对「未知」必须拒绝**：扫描器遇到未登记对象 `continue` 等于该维度门禁关闭（arch-check 未登记表曾静默跳过 ~50 张表的真实写）。正确形状：未登记→finding，假阳性用显式且可审计的豁免清单。 +3. **授权器缺依赖时不得放行**：`pool==nil` 返回 true 是 fail-open；需要演示/测试宽松语义时，用**显式命名的构造器**（`NewInMemoryAuthorizer`）+ 组合根按配置选择，默认路径严格拒绝。 +4. **契约写在注释里不算数**：`validated_only` 之类执行模式必须由执行器硬读硬拒（Confirm 拒 + ExecuteClaimed 拒双层），安全不能依赖「没人点 Confirm」或上游状态机不犯错。 +5. **白名单用精确匹配 + argv 执行**：basename 前缀匹配可被 `go-evil` 伪装；`sh -c` 让元字符求值。argv 化后元字符只是字面参数。 + +## 错误做法与原因 + +- 只给主路径补鉴权、把旁路留给「下次」——本仓 IDOR 事故高发位正是旁路 HTTP。 +- 用「未配置就放行」降低接线摩擦——组合根永远能构造成功，fail-open 在误配时静默生效。 +- 用测试 override `NewAuthorizer(nil)` 当「跳过检查」接缝——它同时是生产 fail-open 的机制本身；接缝必须显式命名。 + +## 验证方法 + +- 回归覆盖：原例（他户标识→403/空）、同类变体（不存在→同构拒绝、admin 越权面）、正常行为（本人→200 且数据完整）。 +- 门禁类：自测夹具证明未登记对象 block、已知假阳性不误报；全仓门禁在登记完成后 PASS。 +- 执行类：伪装命令拒绝、元字符不求值（marker 文件不产生）、owner 零调用计数。 + +## 适用边界 + +- 适用于进程内授权、静态扫描门禁、命令白名单、执行模式契约四类机制；不覆盖网络层/跨服务鉴权。 +- 豁免清单（如扫描噪音词）是启发式产物：新假阳性出现时应先确认确非真实表，再扩清单。 + +## 关联 + +- 来源修复：[2026-09-26 系统 code review P0/P1 修复](../../remediation/2026-09-26-system-code-review-fixes.md) +- 发现审计：[2026-09-26 系统全面 code review](../../audit/2026-09-26-system-code-review.md) +- 关联回归：`TestGetUserContext_*`、`TestUntrackedTableWriteIsBlocked`、`TestMembershipChecks_FailClosedWithoutPool`、`TestExecutor_CommandAllowlistIsExactNotPrefix`、`TestExecutor_NoShellEvaluation`、`TestConfirmRejectedForValidatedOnlyPlan`、`TestExecuteClaimedRefusesValidatedOnlyStep` diff --git a/.doc/ops/lessons/INDEX.md b/.doc/ops/lessons/INDEX.md index 1e773a30..9d403d9a 100644 --- a/.doc/ops/lessons/INDEX.md +++ b/.doc/ops/lessons/INDEX.md @@ -10,5 +10,6 @@ | LESSON-20260924-pi-tool-boundaries | PI、MCP、readonly、path、snapshot、process-group | 路径语义与快照边界一致，合法路径和拒绝路径都验收 | validated | [文件工具边界](2026-09-24-pi-tool-boundaries.md)；2026-09-24 | | LESSON-20260924-cursor-hook-advisory-gates | Cursor、hooks、advisory、fail-open、probe、stdin | hook 与执行器分层；契约未知 fail-open；审计日志放 ignore 路径 | candidate | [客户端 hook 门禁](2026-09-24-cursor-hook-advisory-gates.md)；2026-09-24 | | LESSON-20260924-deny-only-submit | 非编制、DOC_DUTY、400、run/edit、送审 | 权限哨兵映射 403；三道门协同放宽；ActorIsMember 不复用 RoleSystem | validated | [deny_only_submit](2026-09-24-deny-only-submit.md)；2026-09-24 | +| LESSON-20260926-failclosed-bypass-contract | IDOR、旁路、fail-open、白名单前缀、sh -c、validated_only、untracked、table_owners | 旁路入口必须继承主路径硬边界+SQL 防御纵深；门禁/授权器/执行器对未知默认拒绝；安全契约由执行器硬拒不依赖注释 | validated | [旁路与未知即拒绝契约](2026-09-26-failclosed-bypass-contract.md)；2026-09-26 | validated 的证据范围见各条目；本日复核为历史材料和源码阅读，未重跑产品回归。后续按机制补充，过时条目标 superseded/retired 并保留替代链接。 diff --git a/.doc/ops/short-term/2026-09-26-system-review-fixes.md b/.doc/ops/short-term/2026-09-26-system-review-fixes.md new file mode 100644 index 00000000..036966ac --- /dev/null +++ b/.doc/ops/short-term/2026-09-26-system-review-fixes.md @@ -0,0 +1,60 @@ +--- +title: "系统 Code Review P0/P1 修复（2C+3H）" +task_id: "system-review-p0-p1-fixes-2026-09-26" +status: in_progress +created: 2026-09-26 +updated: 2026-09-26 +owner: pi +task_type: implementation +task_tier: cross-module +policy_revision: "agent-governance-2026-09-21-r1 @ f4c1b73f" +branch: fix/system-review-p0-p1-fixes-2026-09-26 +worktree: .worktrees/system-review-p0-p1-fixes-2026-09-26 +base_dev_sha: "f4c1b73f28580ad3df6db4f9ec0a80057006e516" +write_scope: ["internal/server", "internal/ctxmgr", "internal/authz", "internal/swboot", "internal/archtest", "internal/app/intentflow", "internal/intent", "internal/app/app.go", ".doc"] +allowed_environments: [] +mutex: "review-fixes-batch" +review_state: pending +integration_state: pending +deployment: pending +cleanup: pending +remote_sync: not_requested +--- + +# 系统 Code Review P0/P1 修复 + +## 用户目标与授权 + +- **来源**：[2026-09-26 系统 code review](../../audit/2026-09-26-system-code-review.md) 发现项。 +- **用户授权**（2026-09-26）：一并修复 2 Critical（FIND-SEC-01、FIND-ARCH-01）+ 3 个 High 行（FIND-SEC-02/03、FIND-FLOW-01、FIND-QUAL-01）；修复完成后与审计报告**一起提交**（同一批集成）；完成后清理 worktree 任务分支。 +- **禁止项**：未授权环境写、push、晋升 test/main；FIND-ARCH-02（server Pool 接线）未在授权修复范围（ISSUES 登记 open）。 + +## 范围 + +- 修复主记录（含逐项根因/方案/回归）：[remediation/2026-09-26-system-code-review-fixes.md](../../remediation/2026-09-26-system-code-review-fixes.md)。 +- Issue 台账：ISSUE-20260926-ctx-idor / archcheck-untracked / authz-failopen / swboot-shell-prefix / intentflow-validated-confirm / dual-flags-drift（resolved）+ governance-multi-entry（deferred）+ server-pool-wiring（open）。 +- 经验：LESSON-20260926-failclosed-bypass-contract（validated）。 + +## 问题检索与采纳 + +检索词：`IDOR`、`GetUserContext`、`session_id`、`validated_only`、`table_owners`、`untracked`、`fail-open`、`sh -c`、`前缀白名单`、`PlanTier`。命中：来源审计 FIND-*（全部）；ISSUES 无同题已关闭条目；经验库无同机制 lesson（本轮新建）。采纳检查：模块规范 §2.1/§4（表唯一写者、端口装配）、验证契约「问题修复的回归范围」（原例+同类+正常行为）。 + +## 计划与进展 + +1. [x] worktree + 登记（base f4c1b73f） +2. [x] 六项修复实施 + 回归用例 +3. [x] 验证：arch-check block / vet / short 全量 / 隔离 PG（IDOR PG、authz、duty）；两个基线既有失败已核对非本批回归 +4. [x] 文档：修复主记录、ISSUES、lesson、索引 +5. [x] 提交 +6. [ ] 集成（含审计报告分支同批 --no-ff） +7. [ ] 部署核验（dev 共享服务重启 + health/ready/业务行为——需另授权或独立部署任务） +8. [ ] 清理（本任务 + 审计任务 worktree/分支） + +## 验证 + +详见[修复主记录验收汇总](../../remediation/2026-09-26-system-code-review-fixes.md#验收汇总)。要点：arch-check block PASS（fail-closed 后）、vet/short 全绿、隔离 PG（55432 verify 容器临时库）IDOR/authz/duty PASS；`TestHealProjectDocuments_SkipsDutyLocked_PG` 与 PI E2E（KB 离线）为基线既有失败（stash 对照证实）。 + +## 状态与交接 + +- 集成后：任务状态改 done；审计任务（system-code-review-2026-09-26）的 integration/cleanup 由本批一并推进。 +- 部署：`deployment=pending`（代码影响运行内容；dev 服务重启与核验未在本任务授权内，见修复主记录）。 diff --git a/.doc/remediation/2026-09-26-system-code-review-fixes.md b/.doc/remediation/2026-09-26-system-code-review-fixes.md new file mode 100644 index 00000000..1750e3ea --- /dev/null +++ b/.doc/remediation/2026-09-26-system-code-review-fixes.md @@ -0,0 +1,97 @@ +--- +title: "系统 Code Review P0/P1 修复（IDOR、门禁盲区、fail-open、命令白名单、validated_only 硬拒）" +status: done +created: 2026-09-26 +updated: 2026-09-26 +owner: pi +task_id: "system-review-p0-p1-fixes-2026-09-26" +severity: critical +source_audit: "../audit/2026-09-26-system-code-review.md" +base_dev_sha: "f4c1b73f28580ad3df6db4f9ec0a80057006e516" +--- + +# 系统 Code Review P0/P1 修复 + +来源：[2026-09-26 系统全面 code review](../audit/2026-09-26-system-code-review.md)（被审 tip `f4c1b73f`）。 +用户授权（2026-09-26）：修复 2 Critical（FIND-SEC-01、FIND-ARCH-01）+ 3 个 High 行（FIND-SEC-02/03、FIND-FLOW-01、FIND-QUAL-01），修复后与报告一起集成，清理任务 worktree。本文件是下列 issue 的唯一主记录；发现细节与证据链以来源审计为准，此处记修复方案、经过与验收。 + +## 修复范围与 issue 映射 + +| FIND | issue_id | 修复内容 | 状态 | +|---|---|---|---| +| FIND-SEC-01 | ISSUE-20260926-ctx-idor | GetUserContext 会话归属校验（handler 403 + SQL/内存双防线） | resolved | +| FIND-ARCH-01 | ISSUE-20260926-archcheck-untracked | arch-check 未登记表 fail-closed + 补登记 50 表 | resolved | +| FIND-SEC-02 | ISSUE-20260926-authz-failopen | Authorizer 无池严格拒绝；显式 NewInMemoryAuthorizer 接缝 | resolved | +| FIND-SEC-03 | ISSUE-20260926-swboot-shell-prefix | argv 化执行（去 sh -c）+ 白名单精确匹配 | resolved | +| FIND-FLOW-01 | ISSUE-20260926-intentflow-validated-confirm | Confirm 拒 validated_only + 执行器硬读 mode | resolved | +| FIND-QUAL-01 | ISSUE-20260926-dual-flags-drift | 组合根 flag 一致性 fail-fast | resolved（含残差） | +| FIND-QUAL-01（入口收敛残差） | ISSUE-20260926-governance-multi-entry | docgov 三入口 + Coordinator/OwnerMux 合并 | deferred | +| FIND-ARCH-02 | ISSUE-20260926-server-pool-wiring | server 持 Pool 直装 swboot（本轮未授权修复） | open | + +## 各项修复经过 + +### 1. FIND-SEC-01 会话上下文 IDOR（ISSUE-20260926-ctx-idor） + +- **根因**：`handleGetUserContext` 只校验 `user_id` 归属，`session_id` 直通 `GetUserContext`；`chat_messages` 查询 `WHERE session_id = $1` 无归属条件，绕过 chatflow 的会话授权。 +- **修复**（双层）： + - `internal/server/handlers_context.go`：sessionID 非空时经 `GetSessionOwnerID` 校验归属（不存在与归属他人同返 403，不泄露存在性）。 + - `internal/ctxmgr/manager.go`：SQL 增加 `EXISTS(chat_sessions 归属目标用户)` 子句；内存分支按 `sessions[sessionID].UserID == userID` 过滤——未来调用方遗漏 handler 校验也不泄露。 +- **回归**：`TestGetUserContext_OtherUserSessionDenied`（他户 session→403、不存在→403、本人→200 且含正文，内存 HTTP）；`TestGetUserContext_PG_SessionOwnerFilter`（隔离 PG 库：他人视角 manager 层不返回正文、归属者正常）。 + +### 2. FIND-ARCH-01 arch-check 未登记表静默跳过（ISSUE-20260926-archcheck-untracked） + +- **根因**：`internal/archtest/scan.go` `if !tracked { continue }`——表唯一写者门禁对未登记表（含 `swboot_controlled_executions`、`projects`、`outbox_events` 等）无强制力。 +- **修复**： + - scan.go：未登记表产生 `sql_write` finding（fail-closed）；仅豁免扫描正则已知假阳性词汇（`set/of/skip/below/row/state/step/doc/docgov/message/project/aggregation/ref_count`，来自 `ON CONFLICT DO UPDATE SET`、`FOR UPDATE OF/SKIP LOCKED` 与注释散文；静态清单，非策略豁免）。 + - `module-boundaries.yaml`：补登记 50 张既有写者真实表。多 owner 条目（`projects`、`users`、`project_members`、`context_memory_chunks`）是现状快照，新包写入仍被 block；单一写者收敛留在各模块批次（见 ISSUES 残差说明）。 +- **回归**：`TestUntrackedTableWriteIsBlocked`（新表写入必须 block 且 detail 提示登记）；`TestUpsertSetKeywordIsNotFlaggedAsUntracked`（SET 噪音不误报）。全仓 `arch-check --mode block` PASS。 + +### 3. FIND-SEC-02 Authorizer 无池 fail-open（ISSUE-20260926-authz-failopen) + +- **根因**：`IsProjectMember/IsProjectOwner` 在 `!enabled()`（pool==nil）时 `return userID > 0` ——任意登录用户获得任意项目访问；组合根始终 `NewAuthorizer(pool)`，nil pool 也构造成功，绕过「nil→failClosed」语义。 +- **修复**：`NewAuthorizer(nil)` 一律严格拒绝（false）；新增显式 `NewInMemoryAuthorizer()` 仅供 ALLOW_IN_MEMORY 演示/测试接缝（视任意登录用户为成员）；组合根 `app.Assemble` 在 `pool==nil && cfg.AllowInMemory` 时选用。误配（无库且未显式声明）→ 拒绝。 +- **波及与处置**：8 个依赖 fail-open 作测试接缝/隐式放行的既有用例——1 个改用显式内存授权 override，5 个业务行为用例改走 `newServerWithMemoryIAM`/`AllowInMemory=true`（显式演示模式契约），1 组 u5 用例补 `ModeLive`（属 FLOW-01 波及），1 个生命周期用例由组合根修复自然恢复。无削弱鉴权断言的用例。 +- **回归**：`TestMembershipChecks_FailClosedWithoutPool`；隔离 PG 库 authz 套件（ConcurrentLastWriter/DocumentDuty/AtomicDutyRevoke/UpsertProjectMember）PASS。 + +### 4. FIND-SEC-03 swboot sh -c + 前缀白名单（ISSUE-20260926-swboot-shell-prefix） + +- **根因**：受控命令经 `sh -c cmdLine` 执行；允许判定为 basename 前缀匹配（`go` 放行 `go-evil`）。ApproveMerge 自身恒拒未变。 +- **修复**：`runCommand` 改 argv 化 `exec.CommandContext(fields[0], fields[1:]...)`（元字符只成为字面参数）；`commandMatchesPrefixes`→`commandAllowed`，仅 basename 精确相等（含绝对路径条目取其 basename）。admin 白名单粒度仍是真实边界（报告 FIND-SEC-03 说明），运维登记白名单时应只放具体命令名。 +- **回归**：`TestExecutor_CommandAllowlistIsExactNotPrefix`（前缀伪装拒绝、精确/绝对路径放行、空命令拒绝）；`TestExecutor_NoShellEvaluation`（`echo x && touch ` 不求值、marker 不产生、输出为字面参数）。 + +### 5. FIND-FLOW-01 validated_only 可 Confirm 后 dispatch（ISSUE-20260926-intentflow-validated-confirm） + +- **根因**：`DecisionConfirm` 允许 `PlanProposed`；确认后 HTTP 即 `DispatchEligible`；`ExecuteClaimed` 不读 `ExecutionMode`——shadow「安全」依赖「没人点 Confirm」。 +- **修复**（双层）： + - `DecidePlan`：Confirm 对 `plan.ExecutionMode == validated_only` 一律 `ErrPlanModeNotConfirmable`（403）。 + - `ExecuteClaimed`：硬读步骤级（缺省计划级）ExecutionMode，`validated_only` 一律 `ErrPlanModeNotExecutable`——覆盖所有派发路径（confirm 后、worker、误推进）。 + - 新错误码 `PLAN_MODE_NOT_CONFIRMABLE` / `PLAN_MODE_NOT_EXECUTABLE` 入 `ErrorCode` 与 HTTP 映射。 +- **回归**：`TestConfirmRejectedForValidatedOnlyPlan`；`TestExecuteClaimedRefusesValidatedOnlyStep`（经 DecisionContinue 误推进后 DispatchEligible 硬拒，步骤不成 succeeded、owner 零执行）。既有 4 个用例的 `draft()` helper 补显式 `ModeLive`（缺省 validated_only 是安全默认，live 机制用例需显式声明）。 + +### 6. FIND-QUAL-01 双套 flags 静默漂移（ISSUE-20260926-dual-flags-drift，含残差） + +- **修复**：`intentflow.ValidateFlagConsistency`（组合根 `Assemble` 调用）——`PlanTier` 升档但 `MULTI_INTENT_ENABLED=false` 时启动期 fail-fast（`Assemble` 最早阶段调用），消除「intentflow 已升档、intent 仍关」静默漂移配置上线。 +- **回归**：`TestValidateIntentFlagConsistency`（升档+开=过；legacy 恒过；四档×关=拒）。 +- **残差（deferred，ISSUE-20260926-governance-multi-entry）**：docgov 三入口（chat PendingAction / docgov_owner / handlers_docgov）与 Coordinator ~40 intent case + OwnerMux 平行面合并是多日级重构（自带回归矩阵），超出本批授权合理范围；已在 ISSUES 登记 owner/触发条件。 + +## 验收汇总 + +被测 SHA：本任务 worktree HEAD（见下方提交记录）；基线 `f4c1b73f`。 + +| 检查 | 结果 | +|---|---| +| `./scripts/arch-check.sh --mode block` | PASS（fail-closed 改动 + 50 表登记后） | +| `go vet ./...` | PASS | +| `go test -short ./...` | 全绿 | +| 隔离 PG：`TestGetUserContext_PG_SessionOwnerFilter` | PASS（临时库建删于 55432 verify 容器） | +| 隔离 PG：authz + server duty 套件 | PASS | +| 隔离 PG：duty-sync 脚本 | `TestHealProjectDocuments_SkipsDutyLocked_PG` FAIL——**基线 f4c1b73f 同样失败**（FK fixture 既有问题，与本批无关） | +| `TestPiAgent_FullLifecycle_MultiRole_RealWorld_E2E` | 非 short 模式因 KB 离线前置检查失败——**基线同样失败**（需真实 Runner/KB，非本批回归） | + +## 部署 + +代码已按授权集成 local_dev（见任务记录）。dev 共享服务未在本任务内重建/重启（未获 env 写授权）；`deployment=pending`，重启与 `/health`、`/ready`、业务行为核验待部署任务执行。 + +## 经验 + +见 [LESSON-20260926-failclosed-bypass-contract](../ops/lessons/2026-09-26-failclosed-bypass-contract.md)：旁路入口必须继承主路径硬边界；门禁/执行器对「未知」默认必须拒绝。 diff --git a/.doc/remediation/INDEX.md b/.doc/remediation/INDEX.md index 4af91207..21f49242 100644 --- a/.doc/remediation/INDEX.md +++ b/.doc/remediation/INDEX.md @@ -2,6 +2,7 @@ | 文档 | 说明 | |------|------| +| [2026-09-26-system-code-review-fixes.md](./2026-09-26-system-code-review-fixes.md) | 系统 code review P0/P1 修复主记录：IDOR、arch-check 未登记表 fail-closed（补登 50 表）、authz 无池 fail-open、swboot argv 化+精确白名单、intentflow validated_only 双层硬拒、flag 一致性 fail-fast；配套 [审计](../audit/2026-09-26-system-code-review.md) · [经验](../ops/lessons/2026-09-26-failclosed-bypass-contract.md) | | [2026-09-25-dialogue-office-p0-verification.md](./2026-09-25-dialogue-office-p0-verification.md) | 对话办公 P0 验证记录：假成功消除单测 + arch-check；PI 补证待授权 | | [2026-09-25-dialogue-office-expansion-plan.md](./2026-09-25-dialogue-office-expansion-plan.md) | 对话办公能力扩展 P0–P3：消除主链假成功、intentflow dry-run live、查询真数据源、新写意图（trace/补充确认/批注/资产等）；治理结构 UI 专属。配套 [调研](../audit/2026-09-25-dialogue-office-capability-research.md)。planned，不实施代码 | | [2026-09-23-coordination-db-daemon-implementation-undertaking-plan.md](./2026-09-23-coordination-db-daemon-implementation-undertaking-plan.md) | 协调库与 daemon **实施承接计划**（工作包预填）：W1–W3 七包（U1r/U2/U3/U7/U4/U5/U6）含符号级锚点、真实并发验收、mutex、开工准入；L2 仅 ADR 前置门。配套 [设计方案](./2026-09-23-coordination-db-daemon-plan.md) · [任务](../ops/short-term/2026-09-23-coordination-db-daemon-impl-plan.md)。planned，不实施代码 | From dfd9c420d320cc44ac86b0d9c6a51d2e1b612e49 Mon Sep 17 00:00:00 2001 From: weichen Date: Sat, 26 Sep 2026 02:06:02 +0000 Subject: [PATCH 5/6] docs(current): pre-align claim table for review-report merge (docs apply @document) --- .doc/ops/CURRENT.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.doc/ops/CURRENT.md b/.doc/ops/CURRENT.md index 09b69837..ce8cebac 100644 --- a/.doc/ops/CURRENT.md +++ b/.doc/ops/CURRENT.md @@ -19,6 +19,7 @@ | owner | task | branch | worktree | mutex | status | |---|---|---|---|---|---| | pi | system-review-p0-p1-fixes-2026-09-26 | fix/system-review-p0-p1-fixes-2026-09-26 | .worktrees/system-review-p0-p1-fixes-2026-09-26 | review-fixes-batch | in_progress | +| Cursor | system-code-review-2026-09-26 | feature/system-code-review-2026-09-26 | .worktrees/system-code-review-2026-09-26 | docs-audit-review | done | | pi | swboot-closeout-2026-09-26-r3 | feature/swboot-closeout-2026-09-26 | .worktrees/swboot-closeout-2026-09-26 | workflow-design | done | | pi | swboot-closeout-2026-09-26-r2 | feature/swboot-closeout-2026-09-26 | .worktrees/swboot-closeout-2026-09-26 | workflow-design | in_progress | | pi | chat-semantic-replace-2026-09-26 | feature/chat-semantic-replace-2026-09-26 | .worktrees/chat-semantic-replace-2026-09-26 | chat | done | From 467e64574ffff29c975ed1eab8ed23ba7c5ab4ba Mon Sep 17 00:00:00 2001 From: weichen Date: Sat, 26 Sep 2026 02:07:45 +0000 Subject: [PATCH 6/6] docs: closeout review report + fix task records for batch integration --- .doc/audit/2026-09-26-system-code-review.md | 6 +++--- .doc/ops/CURRENT.md | 2 +- .doc/ops/short-term/2026-09-26-system-code-review.md | 10 +++++----- .doc/ops/short-term/2026-09-26-system-review-fixes.md | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.doc/audit/2026-09-26-system-code-review.md b/.doc/audit/2026-09-26-system-code-review.md index 44371244..2b251604 100644 --- a/.doc/audit/2026-09-26-system-code-review.md +++ b/.doc/audit/2026-09-26-system-code-review.md @@ -5,10 +5,10 @@ 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 +review_state: accepted +integration_state: integrated deployment: not_required -cleanup: pending +cleanup: done remote_sync: not_requested updated: 2026-09-26 severity: high diff --git a/.doc/ops/CURRENT.md b/.doc/ops/CURRENT.md index ce8cebac..91c9ee6b 100644 --- a/.doc/ops/CURRENT.md +++ b/.doc/ops/CURRENT.md @@ -18,7 +18,7 @@ | owner | task | branch | worktree | mutex | status | |---|---|---|---|---|---| -| pi | system-review-p0-p1-fixes-2026-09-26 | fix/system-review-p0-p1-fixes-2026-09-26 | .worktrees/system-review-p0-p1-fixes-2026-09-26 | review-fixes-batch | in_progress | +| pi | system-review-p0-p1-fixes-2026-09-26 | fix/system-review-p0-p1-fixes-2026-09-26 | .worktrees/system-review-p0-p1-fixes-2026-09-26 | review-fixes-batch | done | | Cursor | system-code-review-2026-09-26 | feature/system-code-review-2026-09-26 | .worktrees/system-code-review-2026-09-26 | docs-audit-review | done | | pi | swboot-closeout-2026-09-26-r3 | feature/swboot-closeout-2026-09-26 | .worktrees/swboot-closeout-2026-09-26 | workflow-design | done | | pi | swboot-closeout-2026-09-26-r2 | feature/swboot-closeout-2026-09-26 | .worktrees/swboot-closeout-2026-09-26 | workflow-design | in_progress | diff --git a/.doc/ops/short-term/2026-09-26-system-code-review.md b/.doc/ops/short-term/2026-09-26-system-code-review.md index b43f63d4..d068439a 100644 --- a/.doc/ops/short-term/2026-09-26-system-code-review.md +++ b/.doc/ops/short-term/2026-09-26-system-code-review.md @@ -15,10 +15,10 @@ write_scope: [".doc"] allowed_environments: [] head_sha: "f4c1b73f28580ad3df6db4f9ec0a80057006e516" mutex: "docs-audit-review" -review_state: pending -integration_state: pending +review_state: accepted +integration_state: integrated deployment: not_required -cleanup: pending +cleanup: done cleanup_owner: "独立集成/维护" cleanup_trigger: "报告合入 local_dev 且无进程占用本 worktree" remote_sync: not_requested @@ -48,8 +48,8 @@ remote_sync: not_requested 2. [x] 四轴并行审查 + arch-check/规模盘点 3. [x] 关键发现复核（IDOR、scan 未登记表、Confirm×proposed） 4. [x] 落盘审计报告与 INDEX -5. [ ] 合入 local_dev（独立 integrate） -6. [ ] cleanup worktree +5. [x] 合入 local_dev（integrate-review-fixes 批次：报告+修复一起 --no-ff） +6. [x] cleanup worktree（由修复批次维护者执行） ## 验证 diff --git a/.doc/ops/short-term/2026-09-26-system-review-fixes.md b/.doc/ops/short-term/2026-09-26-system-review-fixes.md index 036966ac..edfea9bf 100644 --- a/.doc/ops/short-term/2026-09-26-system-review-fixes.md +++ b/.doc/ops/short-term/2026-09-26-system-review-fixes.md @@ -46,7 +46,7 @@ remote_sync: not_requested 3. [x] 验证：arch-check block / vet / short 全量 / 隔离 PG（IDOR PG、authz、duty）；两个基线既有失败已核对非本批回归 4. [x] 文档：修复主记录、ISSUES、lesson、索引 5. [x] 提交 -6. [ ] 集成（含审计报告分支同批 --no-ff） +6. [x] 集成（含审计报告分支同批 --no-ff，候选 integrate-review-fixes） 7. [ ] 部署核验（dev 共享服务重启 + health/ready/业务行为——需另授权或独立部署任务） 8. [ ] 清理（本任务 + 审计任务 worktree/分支）