From bb1dd97874d4f9e8334fd8a74011b30694d77bd3 Mon Sep 17 00:00:00 2001 From: WaterRain <109326062+CATMIAOZHI@users.noreply.github.com> Date: Fri, 7 Aug 2026 01:32:43 +0000 Subject: [PATCH] =?UTF-8?q?docs:=20=E5=AE=89=E5=85=A8=E4=BF=AE=E5=A4=8D?= =?UTF-8?q?=E8=AE=A1=E5=88=92=EF=BC=88security-fix-plan=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/security-fix-plan.md | 172 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 docs/security-fix-plan.md diff --git a/docs/security-fix-plan.md b/docs/security-fix-plan.md new file mode 100644 index 0000000..c19e4d4 --- /dev/null +++ b/docs/security-fix-plan.md @@ -0,0 +1,172 @@ +# 安全修复计划 — Codex Security Finding 修复 + +> 基于 `codex-security-findings-2026-08-04T22-22-52.609Z.csv` 报告核实结果制定 +> 日期：2026-08-05 +> 修订：v2 — 2026-08-05（纳入首轮独立审核反馈） + +## 一、背景 + +Codex 安全扫描报告了一条 **high** 级别 finding，涉及 commit `5e25be23` (`ci: Release 自动构建 + 签名配置`)，相关文件为 `.github/workflows/release.yml` 和 `app/build.gradle.kts`。 + +核实结论：报告**半真半假**——action mutable reference 和硬编码 fallback 密码是真实风险，但"workflow 未导出 KEYSTORE_PASSWORD 等环境变量"是事实错误（代码第24-27行已明确导出）。 + +## 二、真实风险点（需要修复） + +| # | 风险 | 严重度 | 涉及文件 | +|---|---|---|---| +| R1 | 所有 workflow 使用 mutable action tag 引用（`@v3`、`@v2`、`@v4`）而非 immutable commit SHA pin | medium | `release.yml`、`ci.yml` | +| R2 | `build.gradle.kts` 第35-37行硬编码了签名 fallback 密码（`"RainyToken2026!"`、`"rainy"`），公开可见 | medium | `app/build.gradle.kts` | +| R3 | `release.jks` 解码后留在 workspace，后续 step（含第三方 action）可读取 | low-medium | `release.yml` | +| R4 | `ci.yml` 缺少显式 `permissions` 声明，违反最小权限原则 | medium | `ci.yml` | + +## 三、报告中的错误（不需要修复的部分） + +| 描述 | 事实 | +|---|---| +| "workflow does not export KEYSTORE_PASSWORD, KEYSTORE_ALIAS, or KEY_PASSWORD" | **错误** — `release.yml` 第24-27行已通过 `env:` 导出全部三个变量 | + +## 四、修复方案 + +### 修复 1：所有 action 引用改为 commit SHA pin + +**原因**：Immutable SHA pin 可防止 action 被恶意 retag 后的供应链攻击。 + +**涉及文件**：`.github/workflows/release.yml`、`.github/workflows/ci.yml` + +**SHA 映射**（通过 GitHub API 获取，2026-08-05 查询）： + +| Action | Tag | Commit SHA | 备注 | +|---|---|---|---| +| `actions/checkout` | v4 | `11d5960a326750d5838078e36cf38b85af677262` | | +| `actions/setup-java` | v4 | `cf277c60eb25467037889841efdb72551f06f6c3` | v4 已弃用，后续可升级 v5（不阻塞本次修复） | +| `android-actions/setup-android` | v3 | `9fc6c4e9069bf8d3d10b2204b1fb8f6ef7065407` | | +| `softprops/action-gh-release` | v2 | `3bb12739c298aeb8a4eeaf626c5b8d85266b0e65` | | +| `actions/upload-artifact` | v4 | `ea165f8d65b6e75b540449e92b4886f43607fa02` | | + +**修改形式**（以 `android-actions/setup-android` 为例）： +```yaml +# Before +- uses: android-actions/setup-android@v3 + +# After +- uses: android-actions/setup-android@9fc6c4e9069bf8d3d10b2204b1fb8f6ef7065407 # v3 +``` + +**影响范围**： +- `release.yml`：4 处 action 引用（checkout、setup-java、setup-android、action-gh-release） +- `ci.yml`：4 种 action 引用（checkout、setup-java、setup-android、upload-artifact），共出现 13 次 + - test job：5 次（checkout、setup-java、setup-android、upload-artifact ×2） + - build-debug job：4 次（checkout、setup-java、setup-android、upload-artifact） + - build-release job：4 次（checkout、setup-java、setup-android、upload-artifact） + +> **修订说明**：v1 计划中误写为 11 次，经逐行核实实际为 13 次，已更正。 + +### 修复 2：移除 build.gradle.kts 中的硬编码签名密码 + +**原因**：硬编码的明文密码公开暴露了签名凭据。应改为：env 变量缺失或为空时直接报错，不提供隐式 fallback。 + +**涉及文件**：`app/build.gradle.kts` 第33-37行 + +**关键设计点 — 空字符串防御**： + +> **首轮审核发现的阻断项**：GitHub Actions 中 `${{ secrets.X }}` 在 secret 未设置时会被替换为**空字符串**（不是 null），经 `env:` 传递后 `System.getenv("X")` 返回空字符串 `""`。Kotlin 的 `?:` (Elvis operator) 只检查 `null`，不检查空字符串——空字符串会绕过 `?: throw`，被直接传给签名工具。 + +因此方案使用 `?.takeIf { it.isNotBlank() }` 同时防御 null 和空白字符串： + +```kotlin +// Before (第33-37行) +if (keystoreFile.exists()) { + storeFile = keystoreFile + storePassword = System.getenv("KEYSTORE_PASSWORD") ?: "RainyToken2026!" + keyAlias = System.getenv("KEYSTORE_ALIAS") ?: "rainy" + keyPassword = System.getenv("KEY_PASSWORD") ?: "RainyToken2026!" + +// After — 使用 takeIf 防御 null + 空字符串 +if (keystoreFile.exists()) { + storeFile = keystoreFile + storePassword = System.getenv("KEYSTORE_PASSWORD") + ?.takeIf { it.isNotBlank() } + ?: throw GradleException("KEYSTORE_PASSWORD env var not set or empty — cannot sign release") + keyAlias = System.getenv("KEYSTORE_ALIAS") + ?.takeIf { it.isNotBlank() } + ?: throw GradleException("KEYSTORE_ALIAS env var not set or empty — cannot sign release") + keyPassword = System.getenv("KEY_PASSWORD") + ?.takeIf { it.isNotBlank() } + ?: throw GradleException("KEY_PASSWORD env var not set or empty — cannot sign release") +``` + +**兼容性分析**： +- Release workflow（`release.yml`）已通过 `env:` 导出这三个变量（secrets 已正确配置）→ ✅ 不受影响 +- CI workflow（`ci.yml`）不涉及 `release.jks`，走 debug keystore fallback → ✅ 不受影响 +- 本地构建：若开发者放置了 `release.jks` 但未设置 env 变量 → 会报错（这是正确行为，避免静默使用错误密码） + +### 修复 3：构建完成后清理 release.jks + +**原因**：减少 keystore 在 workspace 中的暴露窗口。虽然后续 step 不多，但作为深度防御措施值得添加。 + +**涉及文件**：`release.yml` + +**修改方案**：在 "Build Release APK" step 之后、"Rename & Release" step 之前添加清理 step： +```yaml + - name: Cleanup keystore (always run for security) + run: rm -f release.jks + if: always() +``` + +> **修订说明**：step name 加注 `(always run for security)` 说明意图。`if: always()` 确保即使构建失败也清理 keystore——这是正确的安全行为。后续的 `softprops/action-gh-release` 依赖已生成的 APK 文件（通过 `files:` 参数），不依赖 `release.jks`，清理不影响其运行。 + +### 修复 4：ci.yml 补充最小权限声明 + +**原因**：ci.yml 完全没有 `permissions` 块。CI job 只做编译、测试、上传 artifact，不需要 write 权限。显式声明 `contents: read` 遵循最小权限原则，尤其 `pull_request` 触发时（可能来自 fork）更应限制。 + +**涉及文件**：`.github/workflows/ci.yml` + +**修改方案**：在 workflow 顶层添加 `permissions: contents: read`（覆盖所有 job）： +```yaml +on: + push: + branches: [main] + pull_request: + +permissions: + contents: read + +jobs: + ... +``` + +**兼容性分析**： +- ci.yml 中的所有 job 只做 checkout、build、test、upload-artifact → 仅需 `contents: read` +- `actions/upload-artifact` 不需要 `contents: write`，artifact 上传使用独立权限 → ✅ 不受影响 +- `pull_request` 触发时来自 fork 的 PR，GitHub 自动将 token 限制为 read-only → 显式声明与此一致，无冲突 + +## 五、不修改的部分 + +| 项 | 理由 | +|---|---| +| `release.yml` 的 `permissions: contents: write` | Release workflow 需要此权限创建 GitHub Release，无法移除 | +| `KEYSTORE_BASE64` secret 解码方式 | 标准做法，无更安全替代 | +| Debug keystore fallback 逻辑（CI 环境） | CI 中使用 debug keystore 只为验证编译/资源完整性，不用于正式发布 | +| `setup-java@v4` 升级到 v5 | v4 已被标记弃用，但 SHA pin 到 v4 当前 commit 仍是安全的。升级 v5 属后续改进项，不阻塞本次安全修复 | + +## 六、修改顺序 + +1. **`app/build.gradle.kts`** — 移除硬编码密码，使用 `takeIf` 防御空字符串（修复 2） +2. **`.github/workflows/release.yml`** — SHA pin + 清理 keystore（修复 1 + 修复 3） +3. **`.github/workflows/ci.yml`** — SHA pin + 补充 `permissions: contents: read`（修复 1 + 修复 4） + +## 七、验证方法 + +- 检查 `release.yml` 和 `ci.yml` 中所有 `uses:` 行均为 `@<40位SHA> # vN` 格式，无 `@vN` 裸 tag +- 检查 `build.gradle.kts` 中 grep `"RainyToken2026!"` 和 `"rainy"` 返回空 +- 检查 `build.gradle.kts` 中存在 `takeIf { it.isNotBlank() }` 模式 +- 检查 `release.yml` 中存在 `rm -f release.jks` 清理 step，且 step name 含 `always run for security` +- 检查 `ci.yml` 中存在 `permissions: contents: read` 声明 +- 确认 CI workflow 的 build-release job 不受 build.gradle.kts 修改影响（走 debug keystore fallback 路径） + +## 八、修订日志 + +| 版本 | 日期 | 修订内容 | +|---|---|---| +| v1 | 2026-08-05 | 初版计划 | +| v2 | 2026-08-05 | 纳入首轮独立审核反馈：
1. 修复 2：`?: throw` → `?.takeIf { it.isNotBlank() } ?: throw`，防御空字符串绕过（审核问题 #1，高严重度）
2. 新增修复 4：ci.yml 补充 `permissions: contents: read`（审核问题 #3，中严重度）
3. 修复 1：ci.yml action 计数从 11 更正为 13（审核问题 #4，低严重度）
4. 修复 3：step name 加注 `(always run for security)` 说明意图（审核问题 #5，建议性）
5. 新增"不修改的部分"中 `setup-java@v4` 弃用说明（审核问题 #6，信息性）
6. 新增 R4 风险点（ci.yml 缺少 permissions）
7. 新增本修订日志 | \ No newline at end of file