docs(review): record residual review findings
This commit is contained in:
Chiguyong
parent
3cf29f4633
commit
444667e2f0
|
|
@ -0,0 +1,182 @@
|
|||
# Residual Review Findings — feat/enterprise-agent-platform-p0
|
||||
|
||||
**Source**: ce-code-review run on commit range `8633f60..3cf29f4`
|
||||
**Branch**: `feat/enterprise-agent-platform-p0`
|
||||
**HEAD**: `3cf29f4`
|
||||
**Tracker availability**: no named tracker documented; `gh` CLI not installed;
|
||||
`any_sink_available = false` — all findings routed to `no_sink` bucket per
|
||||
lfg `references/tracker-defer.md` non-interactive mode.
|
||||
|
||||
These findings were surfaced by the multi-agent code review (5 reviewer
|
||||
subagents: security + adversarial, performance + reliability,
|
||||
maintainability + standards, api-contract + data-migration, deployment
|
||||
verification). They are **actionable** (`autofix_class: manual`) but were
|
||||
**not applied** in Step 5 because they need design input or app+chart
|
||||
coordinated changes. They are durably recorded here per lfg SKILL.md Step 6.
|
||||
|
||||
## P1 — High-impact defects
|
||||
|
||||
- **P1 / `src/agentkit/llm/pii_filter.py` + `src/agentkit/llm/gateway.py`** —
|
||||
PERF-1: PII 脱敏模块未接入生产 LLM 调用路径（死代码）。`filter_messages_pii`
|
||||
仅被单元测试引用，gateway.py 的 chat()/chat_stream() 直接透传原始 messages。
|
||||
等保合规声称的 PII 保护实际未生效。**Fix**: 在 gateway chat() 接入
|
||||
`filter_messages_pii(messages, fail_closed=True)`，从 `agentkit.yaml` 读
|
||||
PII 配置开关和 ner_hook 路径。
|
||||
|
||||
- **P1 / `src/agentkit/server/auth/scim/router.py:233-238`** —
|
||||
API-1: SCIM GET /Users 的 `totalResults` 返回当前页条数而非全量总数
|
||||
（违反 RFC 7644 3.4.2）。当用户总数 > count 时，SCIM 客户端无法得知真实
|
||||
总数，分页逻辑错误。**Fix**: 在列表查询前先执行 `SELECT COUNT(*) FROM
|
||||
users {where}` 复用同一 where 子句和参数。
|
||||
|
||||
- **P1 / `src/agentkit/server/auth/scim/router.py:50-64,107-151,154-197,200-238`** —
|
||||
API-2: SCIM 错误响应使用 FastAPI 默认 `{"detail": "..."}` 而非 RFC 7644
|
||||
3.12 规定的 `{"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
|
||||
"status": "409", "detail": "..."}`。同文件中定义的 `SCIMErrorResponse` 是
|
||||
死代码。**Fix**: 添加 SCIM 异常处理器转换所有 HTTPException 为 SCIM
|
||||
ErrorResponse 格式。
|
||||
|
||||
- **P1 / `src/agentkit/server/routes/auth.py:804-815,823-827`** —
|
||||
API-4: `_sso_issue_token_pair` 用占位符 `refresh_token="__pending_sso__"`
|
||||
创建 session，并发 SSO 登录会触发 `refresh_token_hash` UNIQUE 约束冲突
|
||||
抛 IntegrityError → 500。即使不考虑并发，两步写入也不原子，第二步失败
|
||||
会留下孤儿 session。**Fix**: 先 `secrets.token_urlsafe(32)` 生成真实
|
||||
refresh token 再 `SessionCreate` 一步创建。
|
||||
|
||||
- **P1 / `src/agentkit/server/auth/scim/router.py:107-151`** —
|
||||
API-5: SCIM `create_user` 不写入 `user_idp_links`，导致 SCIM 预配的用户
|
||||
无法 SSO 登录（OIDC 会 JIT 创建重复账户）。SCIM 的核心价值就是预配 + SSO
|
||||
联动，此处断裂使 SCIM 预配失效。**Fix**: SCIM create_user 接受
|
||||
`externalId` 作为 idp_subject，INSERT users 后 INSERT user_idp_links。
|
||||
|
||||
- **P1 / `deploy/offline/scripts/install.sh:58` + docs** —
|
||||
DEPLOY-1: 部署名引用错误。fullname helper 在 release=agentkit 时渲染
|
||||
Deployment 名为 `agentkit`，但 install.sh:58 + 11 处文档引用
|
||||
`agentkit-agentkit`（不存在）。**Fix**: 改用标签选择器
|
||||
`kubectl rollout restart deployment -l app.kubernetes.io/instance=${RELEASE}`。
|
||||
|
||||
- **P1 / `deploy/helm/agentkit/templates/deployment.yaml:88-92`** —
|
||||
DEPLOY-2: Redis 鉴权断裂。`REDIS_PASSWORD` env 注入但应用从不消费
|
||||
（`src/agentkit/**/*.py` 零命中），Redis 客户端通过 `redis_url` 构造
|
||||
（`redis://redis:6379/0`，无密码）。若 Redis 启用密码，pod 全部 Redis
|
||||
操作 NOAUTH 失败。**Fix**: 在 redis 客户端构造处读取 `REDIS_PASSWORD`
|
||||
并以 `password=` 参数传入 `from_url`，或将密码拼入 redis_url。
|
||||
|
||||
## P2 — Moderate issues with meaningful downside
|
||||
|
||||
- **P2 / `src/agentkit/server/auth/scim/router.py:107-151`** —
|
||||
API-6: SCIM POST /Users 缺少 `Location` 响应头（RFC 7644 3.14 强制要求）。
|
||||
**Fix**: 用 `JSONResponse` + `response.headers["Location"] = ...`。
|
||||
|
||||
- **P2 / `src/agentkit/server/auth/scim/router.py:183-187`** —
|
||||
API-7: SCIM PATCH userName 更新未处理 UNIQUE 约束冲突，返回 500 而非
|
||||
409。**Fix**: 包在 try/except `aiosqlite.IntegrityError` 中返回 409。
|
||||
|
||||
- **P2 / `src/agentkit/server/routes/auth.py:812`** —
|
||||
API-8: `_sso_issue_token_pair` 硬编码 `auth_provider="sso"`，丢失 OIDC/SAML
|
||||
区分。admin UI 无法按 IDP 筛选 session。**Fix**: 增加 `provider_name` +
|
||||
`provider_type` 参数。
|
||||
|
||||
- **P2 / `src/agentkit/server/auth/scim/router.py:216-225`** —
|
||||
API-9: SCIM filter 语法不匹配时静默返回全量用户，可能导致数据泄露。
|
||||
**Fix**: 不支持的 filter 返回 400 + `scimType=invalidFilter`。
|
||||
|
||||
- **P2 / `src/agentkit/server/routes/auth.py:1040-1085,1097-1143`** —
|
||||
API-10: `deprovision_user` / `change_user_role` 返回 untyped `dict[str, Any]`，
|
||||
OpenAPI schema 无法文档化。**Fix**: 定义 `DeprovisionResponse` /
|
||||
`RoleChangeResponse` Pydantic 模型。
|
||||
|
||||
- **P2 / `src/agentkit/server/auth/scim/router.py:94-104`** —
|
||||
API-11: SCIM `_user_to_scim` 用 `username` 作为 `displayName`，语义错误。
|
||||
**Fix**: 改为 `None` 让客户端 fallback。
|
||||
|
||||
- **P2 / `src/agentkit/server/auth/models.py:612-620`** —
|
||||
MIG-1: `user_idp_links` 表无 ON DELETE 行为且无 FK 约束到 users。当前无
|
||||
删除路径，但未来引入 GDPR 物理删除时会产生孤儿链接。**Fix**: 引入物理
|
||||
删除时加 `REFERENCES users(id) ON DELETE CASCADE`。
|
||||
|
||||
- **P2 / `deploy/helm/agentkit/templates/deployment.yaml:58-62,93-97`** —
|
||||
DEPLOY-4: PG 密码双源不一致。`DATABASE_URL`（被消费） 与 `POSTGRES_PASSWORD`
|
||||
（死 env）并存。runbook Slot 10 轮换 `postgres-password` 但应用实际用
|
||||
`DATABASE_URL`。**Fix**: 明确单一可信源 — 若只用 `DATABASE_URL`，移除
|
||||
`POSTGRES_PASSWORD` env 与 `postgresPassword` slot。
|
||||
|
||||
- **P2 / `deploy/helm/agentkit/templates/deployment.yaml:83-87`** —
|
||||
DEPLOY-5: OTel exporter token 值格式与 `OTEL_EXPORTER_OTLP_HEADERS` 语义
|
||||
不符。期望 `key=value` 格式，但存储值为 `Bearer <token>`。**Fix**: 改为
|
||||
`Authorization=Bearer <token>` 格式。
|
||||
|
||||
- **P2 / `src/agentkit/server/auth/scim/router.py` (7 处) + `scim/models.py`** —
|
||||
STAND-1: SCIM router 使用 `typing.Any` 7 处，违反 AGENTS.md 禁用 any 规则。
|
||||
**Fix**: 定义 `SCIMUserResponse` / `SCIMUserListResponse` Pydantic 模型。
|
||||
|
||||
## P3 — Low-impact / narrow scope
|
||||
|
||||
- **P3 / `src/agentkit/server/auth/providers/oidc.py:185-197`** —
|
||||
SEC-2: OIDC id_token 未按规范验签（key=None + 关闭 iss/exp）。当前
|
||||
authlib 1.6.12 fail-closed，非直接绕过，但规范违反且脆弱。**Fix**: 用
|
||||
IDP JWKS 公钥验签，校验 iss/aud/exp。
|
||||
|
||||
- **P3 / `src/agentkit/server/auth/scim/router.py:170-185`** —
|
||||
SEC-3: SCIM PATCH deprovisioning（`active=false`）不写审计。admin 同等
|
||||
操作有审计，SCIM 没有。**Fix**: 检测到 active true→false 时调用
|
||||
`record_deprovision(source=SOURCE_SCIM)`。
|
||||
|
||||
- **P3 / `src/agentkit/llm/gateway.py:401-414`** —
|
||||
PERF-6: 流式路径在无 usage 数据时记录"cache miss"，指标误导。**Fix**:
|
||||
仅当 `final_usage` 非 None 时才记录 cache metric。
|
||||
|
||||
- **P3 / `src/agentkit/llm/gateway.py:450-457`** —
|
||||
PERF-7: 非 Anthropic provider 调用膨胀 `prompt_cache.miss` 计数器。
|
||||
**Fix**: 仅对 Anthropic provider 记录 cache metric。
|
||||
|
||||
- **P3 / `src/agentkit/server/auth/scim/router.py:171-187`** —
|
||||
API-12: SCIM PATCH 静默忽略不支持的 op/path，无错误反馈。**Fix**:
|
||||
收集 skipped ops 并返回 400 + `scimType=noTarget`。
|
||||
|
||||
- **P3 / `src/agentkit/server/auth/scim/router.py:200-238`** —
|
||||
API-13: SCIM filter 解析对属性名和操作符大小写敏感，违反 RFC 7644
|
||||
3.4.2.2。**Fix**: 用 `filter.lower()` + 正则 `re.IGNORECASE`。
|
||||
|
||||
- **P3 / `src/agentkit/server/auth/scim/router.py:154-197`** —
|
||||
API-14: SCIM PATCH `op.path==None` 时仅更新 active，丢弃 value 中的其他
|
||||
字段（部分实现静默丢数据）。**Fix**: 遍历 value 的 keys 对每个已知属性
|
||||
执行对应 UPDATE。
|
||||
|
||||
- **P3 / `src/agentkit/server/auth/models.py:609-639`** —
|
||||
MIG-2: schema 迁移幂等但缺少 V4→V5 的显式迁移记录。**Fix**: 未来新增列
|
||||
时按 schema_version 差值执行 ALTER TABLE。
|
||||
|
||||
- **P3 / `deploy/helm/agentkit/values.yaml:8-11`** —
|
||||
DEPLOY-9: 镜像仅 tag 固定（0.1.0），未做 digest pinning。政企生产场景
|
||||
要求 digest pinning 防篡改。**Fix**: 支持可选 `image.digest` 字段。
|
||||
|
||||
- **P3 / `docs/deployment/key-rotation-runbook.md:254-283`** —
|
||||
DEPLOY-8: runbook 仅 Slot 1-10，缺独立 Slot 11（PG 密码）段落。**Fix**:
|
||||
拆为两节，Slot 11 内注明须同步更新 `database-url`（Slot 3）。
|
||||
|
||||
- **P3 / `src/agentkit/bitable/service.py:564-568`** —
|
||||
MAINT-2: `delete_view` 将两种不同失败原因统一抛 `LastViewDeletionError`，
|
||||
误导性错误信息。**Fix**: 区分 `DELETED`/`LAST_VIEW`/`NOT_FOUND` 三态。
|
||||
|
||||
- **P3 / `src/agentkit/server/frontend/src/stores/chatStream.ts:881-887,923-929`** —
|
||||
MAINT-3: `team_synthesis_chunk` 与 `team_synthesis` 重复同一 `findLastMessage`
|
||||
谓词。**Fix**: 提取 `findStreamingSynthesisMilestone(conv, sid)` helper。
|
||||
|
||||
- **P3 / `src/agentkit/server/frontend/tests/unit/bitable-tokens.test.ts:75,81,85`** —
|
||||
MAINT-4: token 声明正则转义重复 3 次。**Fix**: 提取 `tokenDeclPattern(token)`
|
||||
helper。
|
||||
|
||||
- **P3 / `src/agentkit/server/frontend/tests/unit/bitable-tokens.test.ts:92`** —
|
||||
STAND-2: test "outside root" 用 replace 仅替换首个 `:root` 块。**Fix**:
|
||||
加 `g` 标志 `replace(/:root\s*\{[^}]*\}/g, '')`。
|
||||
|
||||
- **P3 / `src/agentkit/server/app.py:1401`** —
|
||||
STAND-3: SCIM router 挂载在 `/api/v1` 前缀下，偏离 RFC 7644 标准 SCIM
|
||||
路径 `/scim/v2`。**Fix**: SCIM router 单独挂载不加 `/api/v1` 前缀。
|
||||
|
||||
---
|
||||
|
||||
**Total residual findings**: 27 (P1=7, P2=10, P3=10)
|
||||
**Applied in Step 5**: 9 fixes (commit `3cf29f4`)
|
||||
**Deferred to follow-up**: 27 (this file)
|
||||
Loading…
Reference in New Issue