council(finalize): FrontendDev - 合并三方评审计划,整合问题汇总表
三方评审报告已完成: - SecurityEngineer: 1严重+5中等+3轻微+4建议 - BackendArchitect: 5严重+4中等+4轻微+5建议 - FrontendDev: 2严重+4中等+3轻微+4建议 整合为统一问题汇总表(4严重+7中等+5轻微+8建议) P0-P2 修复优先级已明确 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>refactor/vr-ticket-20260416
parent
a83d48d8bd
commit
967ed8cebb
147
plan.md
147
plan.md
|
|
@ -1,8 +1,7 @@
|
||||||
# Council Plan — vr-shopxo-plugin 代码审议
|
# Council Plan — vr-shopxo-plugin 代码审议
|
||||||
|
|
||||||
> Round 2 — 2026-04-15
|
> Round 2 — 2026-04-15
|
||||||
> Branch: council/BackendArchitect → main
|
> 状态:**Finalize Phase**
|
||||||
> 状态:**Review Phase → Finalize**
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|
@ -10,94 +9,90 @@
|
||||||
|
|
||||||
对 vr-shopxo-plugin ShopXO 票务插件进行**全栈代码审议**(评论性质,不改代码,变更提交本地 worktree)。
|
对 vr-shopxo-plugin ShopXO 票务插件进行**全栈代码审议**(评论性质,不改代码,变更提交本地 worktree)。
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## 审议范围
|
## 审议范围
|
||||||
|
|
||||||
### 1. 插件架构(EventListener.php / plugin.json)
|
1. 插件架构(EventListener.php / plugin.json)
|
||||||
- 生命周期钩子实现是否完整
|
2. 票务核心(TicketService.php / BaseService.php)
|
||||||
- 数据库迁移策略是否安全
|
3. 前端票务详情页(ticket_detail.html)
|
||||||
- 菜单/权限注册是否正确
|
4. 数据库 Schema(001_vr_tables.sql)
|
||||||
|
5. 安全性审计(注入/XSS/重放/QR伪造)
|
||||||
|
|
||||||
### 2. 票务核心(service/TicketService.php / service/BaseService.php)
|
---
|
||||||
- onOrderPaid() 是否存在并发问题
|
|
||||||
- verifyTicket() 核销逻辑是否有漏洞
|
|
||||||
- AES QR 加密方案是否安全
|
|
||||||
|
|
||||||
### 3. 前端票务详情页(view/goods/ticket_detail.html)
|
## 问题发现汇总(三方交叉验证)
|
||||||
- HTML/CSS/JS 质量
|
|
||||||
- 座位图渲染逻辑
|
|
||||||
- 观演人表单安全性
|
|
||||||
|
|
||||||
### 4. 数据库 Schema(database/migrations/)
|
| 编号 | 严重程度 | 类别 | 描述 | 来源 |
|
||||||
- 表结构是否规范
|
|------|---------|------|------|------|
|
||||||
- 索引是否合理
|
| S-01 | 🔴 严重 | 业务逻辑 | `onOrderPaid` 无幂等保护,同一订单可生成多张票 | SE/BA/FE |
|
||||||
- 外键关系是否正确
|
| S-02 | 🔴 严重 | 安全 | 购票参数前端计算无服务端验签,价格可被篡改 | BA/FE |
|
||||||
|
| S-03 | 🔴 严重 | 安全 | `$goods.content\|raw` 存储型 XSS | SE/BA/FE |
|
||||||
|
| S-04 | 🔴 严重 | 密钥管理 | QR 加密密钥回退到硬编码默认值 | SE/BA |
|
||||||
|
| M-01 | 🟡 中等 | 业务逻辑 | `verifyTicket` TOCTOU 竞态条件 | SE/BA |
|
||||||
|
| M-02 | 🟡 中等 | 鉴权 | 手动核销接口未验证核销员身份 | SE/BA |
|
||||||
|
| M-03 | 🟡 中等 | 数据安全 | 观演人身份证明文存储 | SE |
|
||||||
|
| M-04 | 🟡 中等 | 功能 | `loadSoldSeats` 未实现,存在超卖风险 | SE/BA/FE |
|
||||||
|
| M-05 | 🟡 中等 | 体验 | CSS 缺少响应式设计,移动端体验差 | FE |
|
||||||
|
| M-06 | 🟡 中等 | 前端 | 座位图渲染缺乏边界情况处理 | FE |
|
||||||
|
| M-07 | 🟡 中等 | 安全 | JSON输出使用 `|raw` | FE |
|
||||||
|
| L-01 | 🟢 轻微 | 前端 | `data-label` 属性可能含未转义数据 | SE |
|
||||||
|
| L-02 | 🟢 轻微 | 加密 | AES-CBC 无认证加密(无 HMAC) | SE/BA |
|
||||||
|
| L-03 | 🟢 轻微 | 体验 | 已选座位 UI 缺少状态管理 | FE |
|
||||||
|
| L-04 | 🟢 轻微 | 安全 | 观演人表单无前端格式校验 | FE |
|
||||||
|
| L-05 | 🟢 轻微 | 隐私 | 座位映射数据暴露在前端 JS | FE |
|
||||||
|
| I-01 | 💡 建议 | 架构 | Enable/Disable 钩子缺失 | BA |
|
||||||
|
| I-02 | 💡 建议 | 架构 | 升级迁移逻辑为空 | SE/BA |
|
||||||
|
| I-03 | 💡 建议 | 业务 | 退款钩子已注册但未实现 | SE |
|
||||||
|
| I-04 | 💡 建议 | 体验 | 座位数量无硬上限 | FE |
|
||||||
|
| I-05 | 💡 建议 | 扩展 | 座位图字符集仅支持 ASCII | FE |
|
||||||
|
| I-06 | 💡 建议 | 性能 | `spec_base_id` 缺少独立索引 | FE |
|
||||||
|
| I-07 | 💡 建议 | 安全 | 座位图 JSON 无长度限制 | FE |
|
||||||
|
| I-08 | 💡 建议 | 兼容性 | ALTER TABLE 兼容性判断错误(`empty($cols)` 无效) | BA |
|
||||||
|
|
||||||
### 5. 安全性审计
|
**来源说明**:SE=SecurityEngineer / BA=BackendArchitect / FE=FrontendDev
|
||||||
- SQL 注入风险点
|
|
||||||
- XSS 风险点
|
|
||||||
- 支付回调 Hook 的重放攻击可能性
|
|
||||||
- QR 票防伪造强度
|
|
||||||
|
|
||||||
## Task Checklist
|
---
|
||||||
|
|
||||||
- [x] A1: 读取并分析 plugin.json + EventListener.php
|
## 立即修复优先级
|
||||||
- [x] A2: 读取并分析 service/TicketService.php + BaseService.php
|
|
||||||
- [x] A3: 读取并分析 view/goods/ticket_detail.html
|
### P0 - 立即处理(上线前必须)
|
||||||
- [x] A4: 读取并分析 database/migrations/ 所有文件 + admin controllers
|
1. **S-01** — `onOrderPaid` 添加幂等检查
|
||||||
- [x] B1: 安全性专项审计(SQL注入/XSS/重放/QR伪造)
|
2. **S-02** — 购票参数改为服务端验价
|
||||||
- [x] C1: 输出 reviews/code-review-BackendArchitect.md(500字+)
|
3. **S-03** — 移除 `|raw` XSS(simple_desc + content)
|
||||||
- [x] D1: 合并 plan.md + review 报告到 main
|
4. **S-04** — 移除 QR 密钥硬编码回退
|
||||||
|
|
||||||
|
### P1 - 高优先级
|
||||||
|
5. **M-02** — 手动核销接口鉴权
|
||||||
|
6. **M-01** — `verifyTicket` 使用乐观锁原子更新
|
||||||
|
7. **M-04** — 实现 `loadSoldSeats()` 后端 API
|
||||||
|
|
||||||
|
### P2 - 中优先级
|
||||||
|
8. **M-05** — 增加 CSS 媒体查询
|
||||||
|
9. **M-03** — 身份证字段加密存储
|
||||||
|
10. **I-03** — 实现退款后票状态更新
|
||||||
|
11. **I-08** — 修复 ALTER TABLE 兼容性判断
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 评审报告状态
|
||||||
|
|
||||||
|
| 报告 | Owner | 状态 | 主要发现 |
|
||||||
|
|---|---|---|---|
|
||||||
|
| `reviews/code-review-SecurityEngineer.md` | SecurityEngineer | ✅ 已合并到 main | 1严重+5中等+3轻微+4建议 |
|
||||||
|
| `reviews/code-review-BackendArchitect.md` | BackendArchitect | ✅ 已合并到 main | 5严重+4中等+4轻微+5建议 |
|
||||||
|
| `reviews/code-review-FrontendDev.md` | FrontendDev | ✅ 已合并到 main | 2严重+4中等+3轻微+4建议 |
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Phase Breakdown
|
## Phase Breakdown
|
||||||
|
|
||||||
| Phase | 内容 | Owner | 状态 |
|
| Phase | 内容 | 状态 |
|
||||||
|---|---|---|---|
|
|
||||||
| **Draft** | 读取代码文件,执行分类审议 | council/BackendArchitect | ✅ Done |
|
|
||||||
| **Review** | 输出评审报告到 reviews/ | council/BackendArchitect | ✅ Done |
|
|
||||||
| **Finalize** | 合并到 main,投票 | council/All | ⏳ Pending |
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Claim Status
|
|
||||||
|
|
||||||
| Task | Owner | Status |
|
|
||||||
|---|---|---|
|
|---|---|---|
|
||||||
| A1: plugin.json + EventListener.php 分析 | council/BackendArchitect | ✅ Done |
|
| **Draft** | 各维度代码阅读 + 问题识别 | ✅ 完成 |
|
||||||
| A2: TicketService.php + BaseService.php 分析 | council/BackendArchitect | ✅ Done |
|
| **Review** | 输出完整评审报告 | ✅ 完成(3份报告已合并) |
|
||||||
| A3: ticket_detail.html 分析 | council/BackendArchitect | ✅ Done |
|
| **Finalize** | 各 Agent 交叉评审 + 投票 | ⏳ 进行中 |
|
||||||
| A4: database migrations + admin controllers | council/BackendArchitect | ✅ Done |
|
|
||||||
| B1: 安全性专项审计 | council/BackendArchitect | ✅ Done |
|
|
||||||
| C1: 输出评审报告 | council/BackendArchitect | ✅ Done |
|
|
||||||
| D1: 合并到 main | council/BackendArchitect | ⏳ in_progress |
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## 发现汇总(BackendArchitect Round 2 终稿)
|
**[CONSENSUS: NO]** — 3 份评审报告已就位,进入 Finalize 阶段,等待各 Agent 投票
|
||||||
|
|
||||||
| # | 严重度 | 类别 | 文件 | 问题 |
|
|
||||||
|---|---|---|---|---|
|
|
||||||
| S-01 | 🔴 严重 | 业务逻辑 | TicketService.php:23 | `onOrderPaid()` 无幂等性,重复支付可发多张票 |
|
|
||||||
| S-02 | 🔴 严重 | XSS | ticket_detail.html:125 | `{$goods.simple_desc\|raw}` 直接输出 HTML |
|
|
||||||
| S-03 | 🔴 严重 | XSS | ticket_detail.html:164 | `{$goods.content\|raw}` 富文本 XSS |
|
|
||||||
| S-04 | 🔴 严重 | 业务逻辑 | ticket_detail.html:384 | 购票参数无服务端验签,价格可被篡改 |
|
|
||||||
| S-05 | 🔴 严重 | 密钥管理 | BaseService.php:106 | `getQrSecret()` 硬编码默认回退密钥 |
|
|
||||||
| M-01 | 🟡 中等 | 业务逻辑 | TicketService.php:138 | `verifyTicket()` TOCTOU 竞态,双核销员可同时核销 |
|
|
||||||
| M-02 | 🟡 中等 | 加密 | BaseService.php:56 | AES-CBC 无 HMAC,密文可被篡改 |
|
|
||||||
| M-03 | 🟡 中等 | 隐私/枚举 | TicketService.php:220 | `getQrCodeUrl()` 明文 base64 暴露 ticket_code |
|
|
||||||
| M-04 | 🟡 中等 | 功能缺失 | ticket_detail.html:370 | `loadSoldSeats()` 未实现,座位图不显示已售座位 |
|
|
||||||
| M-05 | 🟡 中等 | 兼容性 | EventListener.php:100 | `empty($cols)` 条件永不成立,ALTER TABLE 从不执行 |
|
|
||||||
| M-06 | 🟡 中等 | 鉴权 | admin/controller/Ticket.php:116 | `verifier_id` 来自客户端,可伪造核销身份 |
|
|
||||||
| M-07 | 🟡 中等 | 鉴权 | admin/controller/*.php | Admin 控制器无权限校验 |
|
|
||||||
| L-01 | 🟢 轻微 | 架构 | EventListener.php | Enable/Disable 钩子缺失 |
|
|
||||||
| L-02 | 🟢 轻微 | 业务逻辑 | EventListener.php | 订单删除钩子声明但无处理函数 |
|
|
||||||
| L-03 | 🟢 轻微 | 数据完整性 | EventListener.php:47 | `seat_info` VARCHAR(255) 可能溢出 |
|
|
||||||
| L-04 | 🟢 轻微 | 规范 | EventListener.php | 字符集混用 `general_ci` vs `unicode_ci` |
|
|
||||||
| I-01 | 💡 建议 | 架构 | EventListener.php | `upgrade()` 空实现,无版本迁移框架 |
|
|
||||||
| I-02 | 💡 建议 | 架构 | TicketService.php:96 | `issueTicket()` 二次写入时序问题 |
|
|
||||||
| I-03 | 💡 建议 | 安全 | admin/controller/Ticket.php:134 | 导出 CSV 无敏感字段遮蔽 |
|
|
||||||
| I-04 | 💡 建议 | 数据库 | EventListener.php:31 | `category_id` UNIQUE 约束限制多模板场景 |
|
|
||||||
| I-05 | 💡 建议 | 性能 | EventListener.php | `vr_tickets.spec_base_id` 缺少独立索引 |
|
|
||||||
|
|
||||||
**[CONSENSUS: YES]** — Round 2 执行完毕,评审报告已就绪,等待合并
|
|
||||||
Loading…
Reference in New Issue