<<<<<<< HEAD # Council Plan — vr-shopxo-plugin 安全审议 > Round 1 — 2026-04-15 > Branch: council/SecurityEngineer → main > 状态:**Draft Phase 完成,进入 Review** ======= # Council Plan — vr-shopxo-plugin 代码审议 <<<<<<< HEAD > Round 1 — 2026-04-15 > Branch: council/FrontendDev → main > 状态:**Draft Phase** >>>>>>> council/FrontendDev ======= > Round 2 — 2026-04-15 > Branch: council/BackendArchitect → main > 状态:**Review Phase → Finalize** >>>>>>> council/BackendArchitect --- ## Task Summary <<<<<<< HEAD 对 vr-shopxo-plugin 票务插件进行完整代码安全审议,输出独立评审报告(≥500字),列出所有发现的问题(严重/中等/轻微/建议),给出具体修复建议。**仅评论不改代码,变更提交本地 worktree。** ======= 对 vr-shopxo-plugin 插件进行全面的代码审议,覆盖插件架构、票务核心、前端页面、数据库 Schema、安全性 5 个维度。**仅评论不改代码**,输出独立评审报告到 `reviews/code-review-FrontendDev.md`。 --- ## Review Scope ### 1. 插件架构 - `app/plugins/vr_ticket/EventListener.php` - `app/plugins/vr_ticket/plugin.json` - 生命周期钩子实现、数据库迁移策略、菜单/权限注册 ### 2. 票务核心 - `app/plugins/vr_ticket/service/TicketService.php` - `app/plugins/vr_ticket/service/BaseService.php` - `onOrderPaid()` 并发问题、`verifyTicket()` 核销漏洞、AES QR 加密强度 ### 3. 前端票务详情页 - `app/plugins/vr_ticket/view/goods/ticket_detail.html` - HTML/CSS/JS 质量、座位图渲染逻辑、观演人表单安全性 ### 4. 数据库 Schema - `app/plugins/vr_ticket/database/migrations/001_vr_tables.sql` - 表结构规范、索引合理性、外键关系 ### 5. 安全性审计 - SQL 注入、XSS、支付回调重放攻击、QR 票防伪造 >>>>>>> council/FrontendDev --- ## Task Checklist <<<<<<< HEAD <<<<<<< HEAD - [x] 1. 插件架构审计(EventListener.php / plugin.json) - [x] 2. 票务核心审计(TicketService.php / BaseService.php) - [x] 3. 前端票务详情页审计(ticket_detail.html) - [x] 4. 数据库 Schema 审计 - [x] 5. 安全性综合审计(SQL注入/XSS/重放攻击/QR伪造) - [x] 6. 输出评审报告到 reviews/code-review-SecurityEngineer.md - [x] 7. 提交 plan.md 到 main --- ## 审计发现汇总 | 编号 | 严重程度 | 类别 | 描述 | |------|---------|------|------| | S-01 | 🔴 严重 | 业务逻辑 | `onOrderPaid` 无幂等保护,同一订单可生成多张票 | | M-01 | 🟡 中等 | 业务逻辑 | `verifyTicket` 存在 TOCTOU 竞态条件 | | M-02 | 🟡 中等 | 鉴权 | 手动核销接口未验证核销员身份 | | M-03 | 🟡 中等 | 数据安全 | 观演人身份证明文存储 | | M-04 | 🟡 中等 | 前端安全 | `simple_desc` 使用 `|raw` 导致存储型 XSS | | M-05 | 🟡 中等 | 加密 | QR 加密密钥回退到硬编码默认值 | | L-01 | 🟢 轻微 | 前端安全 | `data-label` 属性可能含未转义数据 | | L-02 | 🟢 轻微 | 数据完整性 | AES-CBC 无认证加密 | | L-03 | 🟢 轻微 | 业务逻辑 | `loadSoldSeats` 未实现,存在超卖风险 | | I-01~04 | 💡 建议 | 架构/业务 | 升级迁移、退款钩子、购买上限、表单校验缺失 | ======= - [x] R1: 评审插件架构 (EventListener.php / plugin.json) - [x] R2: 评审票务核心 (TicketService.php / BaseService.php) - [x] R3: 评审前端页面 (ticket_detail.html) - [x] R4: 评审数据库 Schema (001_vr_tables.sql) - [x] R5: 安全性综合审计(注入/XSS/重放/QR伪造) - [x] R6: 汇总评审报告 (reviews/code-review-FrontendDev.md) >>>>>>> council/FrontendDev ======= - [x] A1: 读取并分析 plugin.json + EventListener.php - [x] A2: 读取并分析 service/TicketService.php + BaseService.php - [x] A3: 读取并分析 view/goods/ticket_detail.html - [x] A4: 读取并分析 database/migrations/ 所有文件 + admin controllers - [x] B1: 安全性专项审计(SQL注入/XSS/重放/QR伪造) - [x] C1: 输出 reviews/code-review-BackendArchitect.md(500字+) - [x] D1: 合并 plan.md + review 报告到 main >>>>>>> council/BackendArchitect --- ## Phase Breakdown <<<<<<< HEAD | Phase | 内容 | 状态 | |---|---|---| <<<<<<< HEAD | **Draft** | 各模块代码审计 + 报告撰写 | ✅ Done | | **Review** | 评审其他成员报告 | ⏳ Pending | | **Finalize** | 合并到 main,投票 | ⏳ Pending | ======= | **Draft** | 各维度代码阅读 + 问题识别 | ✅ 完成 | | **Review** | 输出完整评审报告 | ✅ 完成 | | **Finalize** | 提交报告到 main | ⏳ 待合并 | --- ## 问题发现汇总 | 严重度 | 数量 | 典型问题 | |--------|------|---------| | 🔴 严重 | 2 | 购票参数前端计算无验签 / `$goods.content\|raw` XSS | | 🟡 中等 | 4 | loadSoldSeats 未实现 / CSS 无响应式 / 座位图渲染边界 / JSON输出XSS | | 🟢 轻微 | 3 | 已选座位 UI 无状态管理 / 观演人表单无校验 / 座位映射数据泄露 | | 💡 建议 | 4 | 座位字符集ASCII限制 / 座位数无上限 / spec_base_id缺索引 / 地图JSON无长度限制 | >>>>>>> council/FrontendDev ======= | Phase | 内容 | Owner | 状态 | |---|---|---|---| | **Draft** | 读取代码文件,执行分类审议 | council/BackendArchitect | ✅ Done | | **Review** | 输出评审报告到 reviews/ | council/BackendArchitect | ✅ Done | | **Finalize** | 合并到 main,投票 | council/All | ⏳ Pending | >>>>>>> council/BackendArchitect --- ## Claim Status | Task | Owner | Status | |---|---|---| <<<<<<< HEAD <<<<<<< HEAD | 插件架构审计 | council/SecurityEngineer | `[Done]` | | 票务核心审计 | council/SecurityEngineer | `[Done]` | | 前端票务页审计 | council/SecurityEngineer | `[Done]` | | 数据库Schema审计 | council/SecurityEngineer | `[Done]` | | 安全综合审计 | council/SecurityEngineer | `[Done]` | | 输出评审报告 | council/SecurityEngineer | `[Done]` | --- ## 立即修复优先级(上线前必须) 1. **S-01** — `onOrderPaid` 添加幂等检查 2. **M-02** — 手动核销接口鉴权 3. **M-04** — 移除 `|raw` XSS 4. **M-05** — 移除 QR 密钥硬编码回退 --- **[CONSENSUS: NO]** — Draft 完成,等待其他成员评审 ======= | R1-R6: 完整评审 | council/FrontendDev | `[Done: council/FrontendDev]` | --- **[CONSENSUS: NO]** — Round 1 规划完成,待执行审议 >>>>>>> council/FrontendDev ======= | A1: plugin.json + EventListener.php 分析 | council/BackendArchitect | ✅ Done | | A2: TicketService.php + BaseService.php 分析 | council/BackendArchitect | ✅ Done | | A3: ticket_detail.html 分析 | council/BackendArchitect | ✅ Done | | 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 终稿) | # | 严重度 | 类别 | 文件 | 问题 | |---|---|---|---|---| | 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 执行完毕,评审报告已就绪,等待合并 >>>>>>> council/BackendArchitect