让 CLI 按 UUID 安全维护访谈选项 - #2
Conversation
mizzenbot
left a comment
There was a problem hiding this comment.
🔴 Taste Rating:需要改进
[CRITICAL ISSUES]
- 两处问题会掩盖无效 ID,或让 PR 分支取得并外传持久凭证,详情见行内评论。
[TESTING GAPS]
- PR 描述仅列出测试与帮助命令;请补充一次针对已部署后端的真实 CLI add/update/delete/reorder 命令及输出,证明完整读改写路径。
[RISK ASSESSMENT]
- [Overall PR]
⚠️ 风险评估:🔴 HIGH。选项写入涉及身份稳定性,新增工作流又使高权限 PAT 和 LLM 密钥进入会执行 PR 内容的环境。
VERDICT:
❌ Needs rework:先修复凭证暴露边界和无效 ID 被静默重写的问题。
KEY INSIGHT:
稳定 ID 的迁移必须保留用户提供的无效值以便后端拒绝,而自动审查不能以持久密钥执行不受信任的 PR 工作区。
Improve this review? 如果任何反馈不适用于本仓库,请在
.agents/skills/custom-codereview-guide.md补充/codereview触发的仓库上下文后重新请求审查。你也可以使用 iterate skill 自动推进修复、CI 与 QA。此审查由 AI 代理(OpenHands)代表用户生成。
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/MizzenAI/mizzen-cli/actions/runs/30710469748
| options: options.map((option) => { | ||
| if (typeof option === "string") return { id: randomUUID(), text: option } | ||
| if (typeof option !== "object" || option === null || Array.isArray(option)) return option | ||
| if (option["id"]) return option |
There was a problem hiding this comment.
🔴 Critical:这里以 truthiness 判断 ID,会把 {"id":"","text":"…"}(以及其他 falsy 值)静默改成新 UUID。用户本来提交的是无效 ID,后端本应返回包含 INVALID_OPTION_ID 与 path 的结构化错误;现在 CLI 却可能成功写入,既掩盖输入错误,也违反“已有 ID 保持不变/畸形 payload 明确失败”的契约。请按属性是否存在来区分缺失与已提供的无效值(或先明确校验并报错),不要用 falsy 值触发自动补齐。
| use-sub-agents: false | ||
| enable-uv-cache: false | ||
| llm-api-key: ${{ secrets.LLM_API_KEY }} | ||
| github-token: ${{ secrets.MIZZENBOT_GITHUB_PAT }} |
There was a problem hiding this comment.
🔴 Critical:该工作流由 pull_request 触发,却把可写的持久 MIZZENBOT_GITHUB_PAT 和 LLM 密钥传给 Action;该 Action 会 checkout PR head,并在该工作区运行 agent。来自同仓库分支的贡献者可在 PR 中修改 workflow/agent 指令或仓库内容以读取并外传这些秘密,且可滥用 PAT。不要在会执行不受信任 PR 内容的 pull_request 工作流中注入持久秘密:应改为受信任的基线工作流与隔离审查架构,并使用权限最小、短期且不可外传的令牌;同时将第三方 Action 固定为完整 commit SHA。
There was a problem hiding this comment.
🔴 Taste Rating:需要改进
Linus-Style Analysis: 审查动作只需要读取仓库并写入 PR review;额外的 issue 写权限没有对应功能。
[CRITICAL ISSUES]
- 详见行内评论:移除不需要的 issue 写权限。
[RISK ASSESSMENT]
- [Overall PR]
⚠️ 风险评估:🟡 MEDIUM。选项身份写入和自动审查工作流均有风险面;当前已固定 Action SHA 并改用临时GITHUB_TOKEN,但仍应收紧多余的写权限。
VERDICT:
❌ Needs rework:先移除无须的 issues: write。
KEY INSIGHT:
自动审查令牌应只拥有提交 PR review 所必需的权限。
Improve this review? 如果任何反馈不适用于本仓库,请在
.agents/skills/custom-codereview-guide.md补充/codereview触发的仓库上下文后重新请求审查。你也可以使用 iterate skill 自动推进修复、CI 与 QA。此审查由 AI 代理(OpenHands)代表用户生成。
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/MizzenAI/mizzen-cli/actions/runs/30710878306
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| issues: write |
There was a problem hiding this comment.
🟠 Important: 固定版本的 pr-review Action 仅通过 Pull Request review API 创建/删除并提交 review,不需要写入 issue。这里多授予的 issues: write 会让被提示注入的审查 agent 持有额外写能力;请删除该权限,仅保留 contents: read 与 pull-requests: write。
There was a problem hiding this comment.
🔴 Taste Rating:需要改进
Linus-Style Analysis: UUID 读改写本身直接,但默认展示给出的 ID 不能驱动新增命令,审查工作流也仍把可外传的 LLM 凭证交给了会读取 PR 内容的 agent。
[CRITICAL ISSUES]
- 详见两条行内评论:修复默认
outline show的 question ID 可用性,并隔离 PR 内容与 LLM 密钥。
[RISK ASSESSMENT]
- [Overall PR]
⚠️ 风险评估:🔴 HIGH。选项管理命令当前会对用户显示的 ID 失败;工作流让具备写权限的贡献者可借由 PR 内容影响持有 LLM 密钥的 agent。
VERDICT:
❌ Needs rework:先保证默认输出提供可执行的稳定 question ID,并建立不向不受信任 PR 工作区暴露 LLM 密钥的审查边界。
KEY INSIGHT:
稳定 ID 的价值取决于 CLI 实际输出的值可直接作为后续命令参数,而自动审查的密钥边界必须独立于被审查的内容。
Improve this review? 如果任何反馈不适用于本仓库,请在
.agents/skills/custom-codereview-guide.md补充/codereview触发的仓库上下文后重新请求审查。你也可以使用 iterate skill 自动推进修复、CI 与 QA。此审查由 AI 代理(OpenHands)代表用户生成。
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/MizzenAI/mizzen-cli/actions/runs/30711058643
| extensions-version: 279ece7e6e678dc1edee98da75a2cf05df2dbfa3 | ||
| llm-model: openrouter/openai/gpt-5.6-terra | ||
| llm-base-url: ${{ secrets.LLM_BASE_URL }} | ||
| llm-api-key: ${{ secrets.LLM_API_KEY }} |
There was a problem hiding this comment.
🔴 Critical: 虽然持久 PAT 已移除,但这里仍把 LLM_API_KEY 交给会 checkout PR head、在该工作区加载仓库 skill/指令并运行 agent 的 Action。任何拥有仓库写权限的贡献者都能提交恶意 PR 内容后请求 mizzenbot 审查,从而通过提示注入诱导 agent 外传该密钥;固定 Action SHA 和短期 GITHUB_TOKEN 并不能保护这个第三方 LLM 凭证。请将审查放入只执行受信任基线代码、与 PR 工作区隔离的架构,或确保 agent 执行环境绝不获得可外传的长效 LLM 密钥。
| const value = optionValue(option) | ||
| rows.push([ | ||
| "", | ||
| value?.id ?? "-", |
There was a problem hiding this comment.
🟠 Important: 默认表格的题目行仍在上方输出 item.readableId(例如 Q1),但新增命令的 getQuestionOptions() 只以 item.id 查找。用户按文档执行 outline show 后复制表格中的 question ID,再运行任一 question option ... <question-id> 命令会稳定得到 Question 'Q1' not found;虽然 option UUID 已展示,却没有可用的父 question UUID。请在默认输出中显示 item.id,或同时清楚显示两者并把稳定 UUID 标为命令参数。
There was a problem hiding this comment.
🟢 Taste Rating:良好
Linus-Style Analysis: 选项身份现在从创建、原始 payload 补齐、回读展示到按 UUID 读改写形成一致链路;已有 ID 与运行时元数据在单项编辑和重排时保留。此前指出的空 ID、可操作 question UUID 与最小 GitHub 权限问题已在当前提交中修复。
[RISK ASSESSMENT]
- [Overall PR]
⚠️ 风险评估:🟡 MEDIUM。该变更会整组写回选项,且依赖后端先发布完整 option identity 回读;当前代码保留未知对象字段,并对缺少稳定 ID 的重排明确失败,风险已得到控制。
VERDICT:
✅ Worth merging:当前提交未发现需要处理的新问题。
KEY INSIGHT:
稳定 UUID 只有在展示、读取和写回同一份选项对象时才真正可用于安全维护。
此审查由 AI 代理(OpenHands)代表用户生成。
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/MizzenAI/mizzen-cli/actions/runs/30711263511
改动
--payload中的字符串选项或缺 ID 选项同样自动补 UUID,已有 ID 和元数据保持不变outline show展示 option ID,并支持--json完整输出验证
bun test(7 passed)bun run typecheckbun run bundlegit diff --checkoutline question option --help、outline show --help依赖与发布顺序
依赖后端 #833 提供 multiple_choice option ID 回读以及 option 元数据往返支持。后端先合并/发布,再发布本 PR 对应 CLI 版本。