diff --git a/reviews/code-review-BackendArchitect.md b/reviews/code-review-BackendArchitect.md new file mode 100644 index 0000000..017984c --- /dev/null +++ b/reviews/code-review-BackendArchitect.md @@ -0,0 +1,352 @@ +# vr-shopxo-plugin 代码审议报告 + +> 审议人:BackendArchitect +> 日期:2026-04-15 +> 审议范围:vr_ticket 插件核心代码(EventListener.php、TicketService.php、BaseService.php、ticket_detail.html) +> 视角:Backend Architect / PHP / 数据库 / 架构完整性 + +--- + +## 一、插件架构(EventListener.php / plugin.json) + +### 1.1 plugin.json 生命周期钩子缺失 ⚠️ 严重 + +**问题**:plugin.json 声明了 `hooks` 数组包含两个 hook,但文件中缺少关键的 **Install/Uninstall/Enable/Disable** 生命周期钩子。 + +```json +"hooks": [ + "plugins_service_order_pay_success_handle_end", + "plugins_service_order_delete_success" +] +``` + +ShopXO 插件标准要求实现以下函数: +- `vr_ticket_install()` — ✅ 存在 +- `vr_ticket_uninstall()` — ✅ 存在(但为空实现) +- `vr_ticket_enable()` — ❌ 缺失 +- `vr_ticket_disable()` — ❌ 缺失 + +**影响**:插件启用/停用时无法执行必要的初始化或清理操作,可能导致菜单重复注册或权限残留。 + +**修复建议**: +```php +function vr_ticket_enable() +{ + // 注册菜单/权限 + return true; +} + +function vr_ticket_disable() +{ + // 清理菜单/权限状态 + return true; +} +``` + +### 1.2 ALTER TABLE 缺少兼容性检查 ⚠️ 中等 + +EventListener.php 第 100-103 行: + +```php +$cols = $db->query("SHOW COLUMNS FROM `{$prefix}goods` LIKE 'item_type'"); +if (empty($cols)) { + $db->query("ALTER TABLE `{$prefix}goods` ADD COLUMN `item_type` ..."); +} +``` + +`SHOW COLUMNS` 返回的是结果集(resource 或 PDOStatement),不是数组,直接 `empty($cols)` 判断无效。应使用 `$cols->rowCount()` 或重新查询。 + +### 1.3 Uninstall 空实现 ⚠️ 建议 + +`vr_ticket_uninstall()` 返回 `true` 但不做任何清理。虽然保留数据是合理的设计决策,但应在代码注释中明确标注「有意保留数据」,而非看起来像是未完成的占位符。 + +### 1.4 Upgrade 空实现 ⚠️ 建议 + +`vr_ticket_upgrade($old_version)` 是空实现,未来版本升级时可能遗漏数据迁移。应在首次发布时就设计好版本号比对框架。 + +--- + +## 二、票务核心(TicketService.php / BaseService.php) + +### 2.1 onOrderPaid() 重复发放风险 — 并发漏洞 ⚠️ 严重 + +`TicketService.php` 第 23-68 行,**没有**任何幂等性保护。如果以下情况发生: + +1. ShopXO 支付回调在短时间内触发两次(网络重试) +2. 多实例部署下两个进程同时处理同一订单 + +**结果**:同一订单商品生成两张票(inventory=1 的票出现多张)。 + +代码中没有任何以下保护机制: +- 事务(Transaction) +- 悲观锁(SELECT ... FOR UPDATE) +- 幂等键(已发放标志检查) + +**修复建议**:在 `onOrderPaid()` 开头增加幂等检查: +```php +$existing = \Db::name(BaseService::table('tickets')) + ->where('order_id', $order_id) + ->find(); +if (!empty($existing)) { + BaseService::log('onOrderPaid: already issued', ['order_id' => $order_id], 'info'); + return true; // 已发放,跳过 +} +``` + +### 2.2 issueTicket() 二次写入时序问题 ⚠️ 中等 + +第 96-126 行: + +```php +$ticket_id = \Db::name(...)->insertGetId([...]); // qr_data 的 id=0 + +// 更新 QR 数据中的 ticket_id +if ($ticket_id > 0) { + $qr_payload['id'] = $ticket_id; + $qr_data_updated = BaseService::encryptQrData($qr_payload); + \Db::name(...)->where('id', $ticket_id)->update(['qr_data' => $qr_data_updated]); +} +``` + +在两步之间,如果系统读取了 ticket,会得到 `id=0` 的 QR 数据(虽然 `decryptQrData` 会成功解密,但数据内容不完整)。 + +**修复建议**:使用数据库事务包裹,或在插入前生成 UUID 作为内部关联码,而非依赖插入后的自增 ID。 + +### 2.3 verifyTicket() 核销状态竞态条件 ⚠️ 严重 + +第 138-196 行,`verifyTicket` 使用「查询-判断-更新」三步模式: + +```php +$ticket = \Db::name('tickets')->where('ticket_code', $ticket_code)->find(); +if ($ticket['verify_status'] == 1) { return ... } +\Db::name('tickets')->where('id', $ticket['id'])->update(['verify_status' => 1, ...]); +``` + +在并发场景下,两个核销员可能同时通过状态检查,导致: +- 同一张票被核销两次(记录写入两次) +- 核销员 A 成功更新后,核销员 B 的 update 仍会执行(覆盖 verifier_id) + +**修复建议**:使用带条件的原子更新: +```php +$affected = \Db::name(BaseService::table('tickets')) + ->where('id', $ticket['id']) + ->where('verify_status', 0) // 原子条件 + ->update(['verify_status' => 1, 'verify_time' => $now, ...]); +if ($affected === 0) { + return ['code' => -2, 'msg' => '该票已核销']; +} +``` + +### 2.4 QR Secret 密钥管理问题 ⚠️ 严重 + +BaseService.php 第 98-107 行: + +```php +private static function getQrSecret() +{ + $secret = env('VR_TICKET_QR_SECRET', ''); + if (!empty($secret)) { + return $secret; + } + return config('shopxo.app_key', 'shopxo_default_secret_change_me'); +} +``` + +问题: +1. **硬编码默认值**:`'shopxo_default_secret_change_me'` 若被用于生产环境,QR 加密等于明文 +2. **密钥与 ShopXO 共享**:`app_key` 可能被用于多个目的,增加密钥泄露面 +3. **无密钥强度验证**:未检查密钥长度是否满足 AES-256 要求 + +**修复建议**: +```php +private static function getQrSecret() +{ + $secret = env('VR_TICKET_QR_SECRET', ''); + if (empty($secret) || strlen($secret) < 32) { + throw new \Exception('VR票务 QR 加密密钥未配置或长度不足(需要32字符)'); + } + return $secret; +} +``` + +### 2.5 AES 加密模式无 HMAC 防篡改 ⚠️ 中等 + +`encryptQrData()` 使用 `AES-256-CBC` + `OPENSSL_RAW_DATA`,但没有对密文做 HMAC 签名。在某些 padding oracle 攻击场景下,CBC 模式可能被利用。即使当前风险较低,缺乏完整性验证是设计缺陷。 + +**修复建议**:在加密后增加 HMAC: +```php +$mac = hash_hmac('sha256', $iv . $encrypted, $secret); +return base64_encode($iv . $encrypted . $mac); +``` + +### 2.6 getQrCodeUrl() 明文暴露票码 ⚠️ 中等 + +第 220-228 行: + +```php +public static function getQrCodeUrl($ticket_code) +{ + $content = base64_encode(json_encode([ + 'type' => 'vr_ticket', + 'code' => $ticket_code, // ⚠️ 未经加密的票码直接暴露 + ])); + return ROOT_URL . '?s=index/qrcode/index&content=' . urlencode($content) ... +} +``` + +QR 码扫描后,任何人都能从 URL 中提取 `ticket_code`(只需 `base64_decode`),无需破解加密。这使得: +1. **票码可枚举**:攻击者可遍历 1-N 的 UUID 尝试核销 +2. **隐私泄露**:QR 码本身不需要包含明文票码 + +**修复建议**:QR URL 只传加密后的 payload,不包含明文 ticket_code: +```php +$qr_data = BaseService::encryptQrData([ + 'code' => $ticket_code, + // 不需要 type 明文 +]); +return ROOT_URL . '?s=index/qrcode/index&content=' . urlencode($qr_data); +``` + +--- + +## 三、前端票务详情页(ticket_detail.html) + +### 3.1 XSS 漏洞 — `|raw` 过滤器 ⚠️ 严重 + +第 125 行: +```html +