first commit
This commit is contained in:
@@ -0,0 +1,307 @@
|
||||
# KIOSK 工程代码审查报告
|
||||
|
||||
审查日期：2026-08-04
|
||||
工程包名：`com.yzx.kiosk`
|
||||
审查范围：工程配置、应用入口、网络、存储、WebSocket、打印、上传下载、版本更新、权限、测试与构建质量。
|
||||
|
||||
## 总体结论
|
||||
|
||||
工程当前能够完成 Debug 和 Release 编译，也能安装到真机运行，但暂不建议直接作为正式生产包发布。主要风险集中在：
|
||||
|
||||
1. 正式包签名与密钥管理。
|
||||
2. 打印结果和服务端通知的准确性。
|
||||
3. 隐私合规及本地敏感数据保护。
|
||||
4. WebSocket、上传下载等异步任务的并发可靠性。
|
||||
5. 自动化测试和静态检查缺失。
|
||||
|
||||
## 严重问题
|
||||
|
||||
### 1. Release 包实际使用 Debug 签名
|
||||
|
||||
位置：`app/build.gradle.kts:112-151`
|
||||
|
||||
- Debug 和 Release keystore 及其密码直接写在工程配置中。
|
||||
- Release 构建在 `signingConfig` 中选择了 `debug` 签名，而非已声明的 `release` 签名。
|
||||
- 实际构建并验证后，Debug 和 Release APK 的签名证书 SHA-256 完全相同。
|
||||
- 获取仓库内容的人可以使用现有密钥签出能够覆盖正式应用的 APK。
|
||||
|
||||
建议：
|
||||
|
||||
- 立即停止在仓库中保存 keystore 和密码。
|
||||
- 将签名信息改为从安全的 CI Secret、本机环境变量或未纳入版本控制的属性文件读取。
|
||||
- 评估现有正式设备是否已经使用该证书，再制定密钥轮换和升级迁移方案。
|
||||
|
||||
### 2. 设置页密码硬编码在客户端
|
||||
|
||||
位置：
|
||||
|
||||
- `app/src/main/java/com/yzx/kiosk/ui/home/viewmodel/HomeViewModel.kt:162-164`
|
||||
- `app/src/main/java/com/yzx/kiosk/ui/home/viewmodel/HomeViewModel.kt:322-324`
|
||||
|
||||
设置入口使用固定常量密码，并在客户端直接进行字符串比较。APK 被反编译后即可得到密码，进而进入设置页并修改设备密钥等敏感配置。
|
||||
|
||||
建议：
|
||||
|
||||
- 不在 APK 中保存通用管理密码。
|
||||
- 使用服务端校验、每台设备独立凭证或短期动态口令。
|
||||
- 增加失败次数限制、审计和必要的锁定机制。
|
||||
|
||||
### 3. 高德 SDK 被无条件标记为“用户已同意隐私政策”
|
||||
|
||||
位置：`app/src/main/java/com/yzx/kiosk/App.kt:50-54`
|
||||
|
||||
Application 启动时直接调用：
|
||||
|
||||
```kotlin
|
||||
AMapLocationClient.updatePrivacyShow(this, true, true)
|
||||
AMapLocationClient.updatePrivacyAgree(this, true)
|
||||
```
|
||||
|
||||
工程虽然包含 `PrivacyPolicyDialogFragment`，但当前没有业务代码展示它，也没有持久化和校验用户真实同意状态。
|
||||
|
||||
建议：
|
||||
|
||||
- 首次启动先展示隐私政策并记录同意状态。
|
||||
- 只有用户主动同意后才能通知第三方 SDK 并初始化相关能力。
|
||||
- 用户拒绝时不得初始化非必要 SDK，并提供可用的退出或受限模式。
|
||||
|
||||
### 4. 打印成功判定不可信
|
||||
|
||||
位置：`app/src/main/java/com/yzx/kiosk/priter/PrinterService.kt:253-296`
|
||||
|
||||
- RX1 将任务加入 SDK 队列后立即报告成功。
|
||||
- QW410 固定等待 18 秒后报告成功。
|
||||
- 两种情况都没有读取打印任务的最终完成或失败状态。
|
||||
|
||||
缺纸、卡纸、断连或 SDK 后续执行失败时，业务仍可能记录为打印成功，并进一步扣减纸张和通知服务端。
|
||||
|
||||
建议：
|
||||
|
||||
- 监听打印 SDK 的任务状态或结果回调。
|
||||
- 只在收到最终成功状态后更新本地记录、纸张数量及服务端状态。
|
||||
- 为超时、离线、卡纸、缺纸和用户取消建立明确的错误映射。
|
||||
|
||||
### 5. 打印状态通知没有真正串行执行
|
||||
|
||||
位置：
|
||||
|
||||
- `app/src/main/java/com/yzx/kiosk/ui/upload/viewmodel/PrintingViewModel.kt:107-143`
|
||||
- `app/src/main/java/com/yzx/kiosk/ui/upload/viewmodel/PrintingViewModel.kt:367-410`
|
||||
- `app/src/main/java/com/yzx/kiosk/network/result/ResultHandler.kt:27-73`
|
||||
|
||||
`PrintingViewModel` 使用 Channel 设计了通知队列，但 `executePrintNotify` 调用的 `handleResultWithData` 会再启动一个协程并立即返回。因此队列只保证发起顺序，不保证网络请求完成顺序。
|
||||
|
||||
可能产生：
|
||||
|
||||
- `print-complete` 先于部分 `print-notify` 到达服务端。
|
||||
- 网络失败后没有持久化重试。
|
||||
- 本地纸张数在服务端确认前已扣减，失败后不会回滚。
|
||||
|
||||
建议：
|
||||
|
||||
- 让单次通知成为真正可等待的挂起调用。
|
||||
- 只有接口确认成功后再消费下一条通知。
|
||||
- 将待通知状态持久化到 Room，并增加幂等键、重试和恢复机制。
|
||||
|
||||
### 6. 启动后可能错误报告打印机在线
|
||||
|
||||
位置：
|
||||
|
||||
- `app/src/main/java/com/yzx/kiosk/priter/PrintStatusManager.kt:24-30`
|
||||
- `app/src/main/java/com/yzx/kiosk/MainActivity.kt:93-100`
|
||||
- `app/src/main/java/com/yzx/kiosk/websocket/WebSocketService.kt:565-575`
|
||||
|
||||
`PrintStatusManager` 默认打印机在线，而 MainActivity 启动后会立即连接 WebSocket。打印机只在进入打印或管理流程后才初始化，因此没有连接打印机时，心跳仍可能报告“空闲可接单”。
|
||||
|
||||
建议：
|
||||
|
||||
- 默认状态改为离线或未知。
|
||||
- 应用启动时主动初始化并探测打印机。
|
||||
- 只有获得可靠设备状态后才报告可接单。
|
||||
|
||||
## 高优先级问题
|
||||
|
||||
### 7. 敏感数据使用未加密 MMKV 保存
|
||||
|
||||
位置：
|
||||
|
||||
- `app/src/main/java/com/yzx/kiosk/utils/MMKVUtils.kt`
|
||||
- `app/src/main/java/com/yzx/kiosk/datastore/AppStoreDataSource.kt:48-68`
|
||||
- `app/src/main/AndroidManifest.xml:52-64`
|
||||
|
||||
设备密钥、Socket Token、盒子 API Token 等数据均存入默认 MMKV，没有加密。同时应用开启 `allowBackup`，备份规则仍是空模板，存在敏感数据被备份或迁移的风险。
|
||||
|
||||
建议：
|
||||
|
||||
- 使用 Android Keystore 保护的密钥加密敏感字段。
|
||||
- 明确排除 MMKV、数据库和凭证文件的云备份与设备迁移。
|
||||
- 对 Token 设置有效期，并支持服务端吊销。
|
||||
|
||||
### 8. APK 更新只校验文件长度
|
||||
|
||||
位置：`app/src/main/java/com/yzx/kiosk/utils/VersionUpdateManager.kt:312-384, 535-557`
|
||||
|
||||
- 已存在 APK 只比较 Content-Length。
|
||||
- HEAD 请求无法获取文件大小时会直接使用本地文件。
|
||||
- 新下载文件同样只验证长度，没有 SHA-256 或可信证书白名单校验。
|
||||
|
||||
建议：
|
||||
|
||||
- 版本接口下发 APK 的 SHA-256，并在安装前严格比对。
|
||||
- 校验 APK 包名、版本号和签名证书摘要。
|
||||
- 下载到临时文件，校验通过后再原子替换正式文件。
|
||||
|
||||
### 9. 下载任务的并发模型互相冲突
|
||||
|
||||
位置：
|
||||
|
||||
- `app/src/main/java/com/yzx/kiosk/utils/cloud/DownloadRepository.kt:26-167`
|
||||
- `app/src/main/java/com/yzx/kiosk/utils/cloud/CloudOssUtils.kt:422-509`
|
||||
|
||||
`DownloadRepository` 用 Map 表示可以同时运行多个下载任务，但 `CloudOssUtils` 只保存一个全局 `currentDownloadTask`。暂停或取消某个任务时，可能取消最后启动的另一个任务。
|
||||
|
||||
建议：
|
||||
|
||||
- 每个任务持有独立 OSS 请求对象和取消句柄。
|
||||
- 或显式限制为单任务队列，不暴露并发能力。
|
||||
- 使用互斥锁或单线程调度器保护任务状态。
|
||||
|
||||
### 10. ResponseInterceptor 篡改所有错误响应的数据结构
|
||||
|
||||
位置：`app/src/main/java/com/yzx/kiosk/network/interceptor/ResponseInterceptor.kt:27-44`
|
||||
|
||||
所有业务失败响应都会被强制修改为：
|
||||
|
||||
```json
|
||||
{"data": {}}
|
||||
```
|
||||
|
||||
当接口声明的 `data` 类型是字符串、列表或其他非对象类型时，Retrofit/Gson 可能在业务错误处理之前反序列化失败，最终把业务错误误判为网络异常。同时原始错误数据也会丢失。
|
||||
|
||||
建议：
|
||||
|
||||
- 不在拦截器中修改业务 JSON。
|
||||
- 让 `data` 保持可空，并在统一结果层根据 `code` 处理。
|
||||
- 为不同错误响应建立兼容测试样例。
|
||||
|
||||
### 11. Room 数据库可能破坏性清库
|
||||
|
||||
位置：`app/src/main/java/com/yzx/kiosk/network/db/DatabaseModule.kt:20-28`
|
||||
|
||||
数据库当前只提供 `7 -> 8` 迁移，其他版本使用 `fallbackToDestructiveMigration()`。老版本升级时可能直接清除未完成上传任务。
|
||||
|
||||
此外两个数据库都声明了 `exportSchema = true`，但未配置 `room.schemaLocation`，构建时已出现警告，也没有生成可纳入版本管理的 schema。
|
||||
|
||||
建议：
|
||||
|
||||
- 补齐所有已发布版本的迁移链。
|
||||
- 移除生产数据库的破坏性迁移回退。
|
||||
- 导出 schema 并编写 MigrationTestHelper 测试。
|
||||
|
||||
### 12. WebSocket 状态存在多线程竞争
|
||||
|
||||
位置：`app/src/main/java/com/yzx/kiosk/websocket/WebSocketService.kt`
|
||||
|
||||
OkHttp 回调线程和主线程会同时读写 `webSocket`、`isBound`、`isConnecting`、重连计数和普通 `mutableListOf` 监听器。当前缺少统一协程作用域、线程封闭或锁保护。
|
||||
|
||||
可能造成重复重连、旧连接回调覆盖新连接状态、遍历监听器时被修改等问题。
|
||||
|
||||
建议：
|
||||
|
||||
- 建立一个 Application 级 SupervisorJob。
|
||||
- 将所有连接状态归一到单线程调度器或 Mutex 保护的状态机。
|
||||
- 使用线程安全监听器集合，或统一切换到主线程通知。
|
||||
|
||||
## 工程质量与维护问题
|
||||
|
||||
### Lint 当前失败
|
||||
|
||||
执行 `:app:lintDebug` 的结果：
|
||||
|
||||
- 4 errors
|
||||
- 345 warnings
|
||||
- 8 hints
|
||||
|
||||
4 个错误包括：
|
||||
|
||||
1. Target Android 13+ 时缺少 `POST_NOTIFICATIONS` 权限处理。
|
||||
2. `AppNavHost.kt` 使用 Media3 不稳定 API 但没有 opt-in。
|
||||
3. `PosterScreenWithLiveOrSlideshow.kt` 两处使用 Media3 不稳定 API但没有 opt-in。
|
||||
|
||||
完整报告：`app/build/reports/lint-results-debug.html`
|
||||
|
||||
### 自动化测试基本为空
|
||||
|
||||
- 本地单元测试只有 `2 + 2 = 4` 示例。
|
||||
- Instrumentation 测试仍断言旧包名 `com.zhifly.follow`，当前包名为 `com.yzx.kiosk`，运行后会失败。
|
||||
- 打印状态、通知顺序、数据库迁移、WebSocket 重连、版本校验等核心逻辑均无测试覆盖。
|
||||
|
||||
### 权限申请过多
|
||||
|
||||
位置：`app/src/main/AndroidManifest.xml`
|
||||
|
||||
Manifest 申请了后台定位、电话状态、忽略电池优化、旧版外部存储等权限，但工程中未找到对应业务使用。应用同时全局允许明文流量，扩大了安全与合规风险。
|
||||
|
||||
建议按真实业务最小化权限，并用 `networkSecurityConfig` 只对白名单局域网地址放开明文流量。
|
||||
|
||||
### Release 包体积偏大
|
||||
|
||||
实际构建的 Release APK 约为 144 MB。
|
||||
|
||||
主要原因包括：
|
||||
|
||||
- Release 未开启代码压缩和资源压缩。
|
||||
- 同时配置多个 ABI。
|
||||
- 本地库约 67 MB，高德 native 库占比较高。
|
||||
- 音频资源约 24 MB。
|
||||
- 存在重复依赖和不应作为运行时依赖的构建工具依赖。
|
||||
|
||||
建议使用 ABI Split/App Bundle、开启 R8 和资源压缩，并清理重复或无效依赖。
|
||||
|
||||
### 核心类过大
|
||||
|
||||
部分文件行数：
|
||||
|
||||
| 文件 | 行数 |
|
||||
| --- | ---: |
|
||||
| `DeviceConfigViewModel.kt` | 1574 |
|
||||
| `DeviceConfigScreen.kt` | 1024 |
|
||||
| `HomeScreen.kt` | 996 |
|
||||
| `WebSocketService.kt` | 974 |
|
||||
| `VersionUpdateManager.kt` | 811 |
|
||||
| `PrivacyPrintService.kt` | 673 |
|
||||
| `UploadManager.kt` | 589 |
|
||||
|
||||
建议按设备配置、上传、播放器、连接状态机、打印订单等职责拆分，并将业务流程从 Composable 和 ViewModel 中下沉到可测试的 UseCase/Repository。
|
||||
|
||||
### Gradle Wrapper 不可直接运行
|
||||
|
||||
根目录的 `gradlew` 使用 CRLF 换行且没有执行权限，在 macOS/Linux 上会出现解析错误或 Permission denied。
|
||||
|
||||
建议恢复标准 Wrapper 文件、使用 LF 换行并设置可执行权限，同时在 CI 中验证 `./gradlew tasks`。
|
||||
|
||||
## 验证结果
|
||||
|
||||
| 项目 | 结果 |
|
||||
| --- | --- |
|
||||
| Debug 编译 | 通过 |
|
||||
| Debug 真机安装启动 | 通过 |
|
||||
| Release 编译 | 通过 |
|
||||
| Release APK 大小 | 约 144 MB |
|
||||
| Debug/Release 签名对比 | 相同证书 |
|
||||
| 单元测试 | 1 个示例测试通过 |
|
||||
| Lint | 失败，4 errors / 345 warnings / 8 hints |
|
||||
| Instrumentation 测试 | 未执行；代码中的包名断言确定错误 |
|
||||
|
||||
## 建议处理顺序
|
||||
|
||||
1. 移除仓库签名密钥和密码，修正 Release 签名，评估密钥轮换。
|
||||
2. 移除固定管理密码，改为可靠的设备管理认证。
|
||||
3. 修复打印完成判定、通知顺序、纸张扣减和失败重试。
|
||||
4. 启动时真实探测打印机，避免错误报告可接单。
|
||||
5. 补齐隐私授权流程，收敛权限和明文网络范围。
|
||||
6. 加密设备密钥及 Token，关闭敏感数据备份。
|
||||
7. 增加 APK 哈希与签名校验。
|
||||
8. 修复上传下载及 WebSocket 并发状态管理。
|
||||
9. 修复 Lint、数据库迁移和测试基础设施。
|
||||
10. 拆分大类、清理依赖并优化 APK 体积。
|
||||
Reference in New Issue
Block a user