| content |
## 自动代码审查报告
**分支**: app-260616
**提交**: `697aa9f19 ## 自动代码审查报告
**分支**: app-260616
**提交**: `697aa9f1918d12c5fb5d69b56908c15e2658e2b1`
**提交人**: LITTLEMAIDI (11833999+littlemaidi@user.noreply.gitee.com)
**时间**: 2026-06-05 16:03:21
---
## 1. 审查摘要
- **代码质量评分**：5.5 / 10 分
- **总体评价**：该模型文件承载了大量核心业务逻辑（包厢状态流转、权限过滤、计费差价计算等），整体结构具备一定分层意识。但存在多处**高危 SQL 注入风险**、**Token 解析缺乏安全校验**、**返回值类型严重不一致**以及**全局函数/变量滥用**等问题。代码规范与 PSR-12 存在较大偏差，且末尾代码被截断，部分逻辑无法完整评估。
- **风险等级**：🔴 高（存在直接拼接 SQL 的注入点、未校验的 Token 解析、逻辑缺陷可能导致资损或越权）
> ⚠️ **局限性说明**：您提供的代码在 `get_room_detail` 方法末尾（`$priv_where['_role_i`）被截断。本次审查仅基于已提供的完整代码片段，未覆盖截断后的逻辑。
---
## 2. 问题详情
| 严重程度 | 文件/行号 | 问题描述 | 建议修改方案 | 代码示例 (可选) |
| :--- | :--- | :--- | :--- | :--- |
| 🔴 严重 | `list_rooms_info`<br>`list_nearby_family_servers_info`<br>`get_nearby_ids`<br>`get_family_server_ids_by_shop_ids` | **SQL 注入漏洞**：多处使用 `$addsql` 或 `$shop_ids` 直接拼接 SQL 字符串，未做任何过滤或参数绑定。攻击者可构造恶意参数执行任意 SQL。 | 废弃字符串拼接，全面改用 CI 查询构建器（Query Builder）或参数绑定 `$this->db->query($sql, $binds)`。 | ```php<br>// 修复前<br>$sql = "... WHERE _shop_id in " . $shop_ids;<br><br>// 修复后<br>$ids = explode(',', $shop_ids);<br>$this->db->where_in('_shop_id', $ids);<br>$result = $this->db->get('ahead_family_servers')->result_array();<br>``` |
| 🔴 严重 | `get_count_status_room`<br>`get_count_room_by_shop` | **Token 解析无校验且忽略签名**：`explode("_", $token)` 未检查数组长度，越界会触发 Warning；`$sign` 参数完全未参与验签，存在伪造身份越权风险。 | 增加格式校验、长度检查，并调用框架签名验证函数。 | ```php<br>$parts = explode('_', trim($request['token']));<br>if (count($parts) !== 4) {<br> return ['status' => -1, 'msg' => 'Token格式错误'];<br>}<br>[$merchant_id, $uid, $utype, $sign] = $parts;<br>// 必须调用签名验证逻辑<br>if (!$this->verify_token_sign($merchant_id, $uid, $utype, $sign)) {<br> return ['status' => -2, 'msg' => '签名验证失败'];<br>}<br>``` |
| 🔴 严重 | `get_is_valid_family_server_id` | **逻辑缺陷：参数未生效**：方法接收 `$id` 和 `$shop_id`，但 SQL 查询中完全未使用这两个条件，导致返回结果与预期不符。 | 将参数加入 WHERE 条件。 | ```php<br>$sql = "SELECT count(1) as num FROM ahead_family_servers WHERE _family_server_id=? AND _merchant_id=? AND _shop_id=?";<br>$result['data'] = $this->db->query($sql, [$family_server_id, $id, $shop_id])->row_array();<br>``` |
| 🟠 警告 | 全局多处 | **返回值类型不一致**：部分方法返回 `array('status'=>1)`，部分返回 `bool`，部分返回 `string`（如 `last_query()`）。调用方难以统一处理，易引发 `Undefined index` 或类型错误。 | 统一模型层返回规范，建议始终返回结构化数组：`['success' => bool, 'data' => mixed, 'msg' => string]`。 | ```php<br>// 统一规范示例<br>public function add_room(...): array {<br> if ($exists) {<br> $this->update(...);<br> return ['success' => true, 'msg' => '更新成功'];<br> }<br> $res = $this->insert(...);<br> return $res ? ['success' => true, 'msg' => '新增成功'] : ['success' => false, 'msg' => '数据库写入失败'];<br>}<br>``` |
| 🟠 警告 | `get_room_detail` (约 L950) | **字符串拼接含双逗号**：`$functions .= "32,2,5,6,12,18,,20,24,58";` 中 `18,,20` 存在连续逗号，下游解析时会产生空元素或报错。 | 使用数组管理功能码，最后统一 `implode(',', $functions)`。 | ```php<br>$functions = [32, 2, 5, 6, 12, 18, 20, 24, 58];<br>// 后续动态添加<br>$functions[] = 41;<br>$func_str = implode(',', array_unique($functions));<br>``` |
| 🟠 警告 | 文件顶部 & 方法内 | **`$CI =& get_instance()` 滥用**：在类外部调用一次，内部方法又重复调用。违反 CI 框架设计，增加内存开销且破坏封装性。 | 移除文件顶部调用。在 CI 模型中，`$this` 已继承自 `CI_Model`，可直接使用 `$this->load`、`$this->db` 等。 | ```php<br>// 删除文件顶部的 $CI =& get_instance();<br>// 方法内直接使用 $this->load->helper('common'); 即可<br>``` |
| 🟡 建议 | 类属性定义 (L15-L30) | **属性可见性过高**：`$tableName`、`$redis_key`、`$room_info` 等均为 `public`，外部可随意篡改，破坏对象状态。 | 改为 `protected` 或 `private`，通过 Getter/Setter 或受保护方法访问。 | `protected $tableName = 'ahead_family_servers';` |
| 🟡 建议 | 全局函数调用 | **强依赖全局函数**：大量使用 `get_aliyun_redis_conn()`、`smallChangeFormat()`、`preMinute()` 等全局函数。不利于单元测试、依赖注入及框架迁移。 | 将全局函数封装为 Helper 类或服务类，通过 `$this->load->helper()` 或 DI 容器注入。 | 建议逐步重构为 `RedisService::getInstance()->hGet(...)` |
| 🟡 建议 | 代码规范 | **短 `if` 语句未使用大括号**：如 `if ($room_area) $where['_room_area'] = $room_area;` 违反 PSR-12 规范，易引发后续维护时的逻辑错误。 | 严格遵循 PSR-12，所有控制结构必须使用大括号。 | ```php<br>if ($room_area) {<br> $where['_room_area'] = $room_area;<br>}<br>``` |
---
## 3. 总结与行动建议
### 🔑 优先修复的关键问题（P0/P1）
1. **彻底修复 SQL 注入**：立即替换 `list_rooms_info`、`list_nearby_family_servers_info`、`get_nearby_ids` 等 4 个方法中的字符串拼接 SQL。这是最高危的安全漏洞，必须优先处理。
2. **统一返回值契约**：制定团队级模型返回规范（如 `['code' => int, 'data' => mixed, 'message' => string]`），并在 `update_room_status`、`add_room` 等方法中强制对齐，避免调用层崩溃。
3. **修复 Token 解析逻辑**：补充 `explode` 后的 `count()` 校验，并接入签名验证机制。当前逻辑等同于“裸奔”，极易被伪造请求绕过权限。
### 🛠 后续重构与优化方向
1. **状态机与魔法数字治理**：代码中大量使用 `-1, 0, 1, 2, 3` 及 `32,2,5...` 等硬编码。建议提取为类常量或枚举（PHP 8.1+），例如：
```php
const STATUS_FREE = 0;
const STATUS_CONSUMING = 1;
const STATUS_REPAIRING = 2;
```
2. **Redis 连接生命周期管理**：当前 `get_aliyun_redis_conn()` 在多处被调用且部分未显式 `close()`。建议封装为单例或连接池，或在 CI 的 `__destruct()` 中统一释放，防止连接泄漏。
3. **解耦全局函数**：将 `smallChangeFormat`、`getNewSmallChange` 等计费相关函数抽离至独立的 `PriceCalculator` 服务类，便于后续进行单元测试和价格策略扩展。
4. **框架适配说明**：基于代码特征，`phpci` 高度兼容 CodeIgniter 3。若该框架有自定义的 `DB_driver` 或 `Session` 实现，请确保 `$this->db->query()` 的底层预处理机制未被覆盖。建议查阅 `phpci` 官方文档中关于 **Query Builder 安全过滤** 与 **模型自动加载** 的章节。
> 💡 **下一步建议**：修复上述 P0 问题后，建议引入静态分析工具（如 `PHPStan` 或 `Psalm`）配置 CI 流水线，自动拦截类型不一致与未定义变量问题。如需对截断的 `get_room_detail` 方法或特定计费逻辑进行深度审查，请提供完整代码片段。
---
*此 Issue 由代码审查服务自动创建*... |