fix(auto-review): 保留宿主审批并用当前模型静默兜底 - #1227
Conversation
Signed-off-by: zqchris <chrisz83@gmail.com>
There was a problem hiding this comment.
Pull request overview
本 PR 调整 Maker Core / Desktop Host 的 Auto-review 行为接线：在 Claude Code / Codex 的 官方 OAuth 路由优先使用原生 reviewer，而在第三方/网关路由或原生 reviewer 运行期失效时，改用 当前会话已选 provider+model 进行轻量三态裁决（allow/block/ask），并确保 故障/超时/畸形输出静默 block、不把会话模式从 Auto 反向持久化为 Ask。
Changes:
- 重构 auto-review 核心策略为“两层判定”：确定性放行/红线必问 + 灰区交轻量 reviewer 三态裁决，并引入单轮裁决缓存。
- Claude Code / Codex 接线改为“原生优先 + 当前模型 fallback”，并在原生 reviewer 故障时保持产品模式为 Auto，仅切换 reviewer 路由。
- Desktop Host 增加轻量 reviewer（最小 prompt + 严格解析/上限），并把 reasoningEffort/max_output_tokens 兼容细节下沉到 utility-model one-shot 请求层。
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/maker-core/src/agents/shared/auto-review.ts | 调整 shell/动作的确定性规则分层，引入灰区交给轻量 reviewer 的语义。 |
| packages/maker-core/src/agents/shared/auto-review.test.ts | 更新 auto-review 核心单测以匹配“极高风险必问，其余灰区交 reviewer”。 |
| packages/maker-core/src/agents/shared/auto-review-decision.ts | 新增统一三态裁决入口与 userIntent 抽取，负责本地规则 + delegate 兜底。 |
| packages/maker-core/src/agents/shared/auto-review-decision.test.ts | 覆盖 deterministic 不调用模型、灰区委托、故障静默 block、intent 截断等行为。 |
| packages/maker-core/src/agents/index.ts | 导出 AutoReviewDecision/Delegate/Request 与 ReviewableAction 类型供 host 侧使用。 |
| packages/maker-core/src/agents/codex/index.ts | Codex Auto 接线：仅官方 OAuth 用 Guardian；第三方路由用当前模型 reviewer；原生故障保持 Auto 并切 fallback。 |
| packages/maker-core/src/agents/codex/index.test.ts | 更新 Codex 侧接线测试，覆盖第三方路由当前模型审查、Guardian 故障不降 Ask、仅 ask 才弹 UI 等。 |
| packages/maker-core/src/agents/claude-code/index.ts | Claude Code Auto 接线：官方 OAuth 保留 SDK auto；第三方/故障走 SDK default 以触发 canUseTool 并用 Cindy fallback。 |
| packages/maker-core/src/agents/claude-code/auto-review-policy.ts | 抽出 normalizeBuiltinToolForAutoReview，使内置工具动作归一化结果可被本地规则与 fallback 共用。 |
| packages/maker-core/src/agents/claude-code/tests/auto-review-wiring.test.ts | 更新 Claude 接线集成测试：原生优先、fallback 后保持 Auto、灰区 allow/block 静默处理等。 |
| packages/maker-core/src/agents/claude-code/tests/auto-review-policy.test.ts | 更新 Claude 内置工具策略测试以匹配新分层语义。 |
| packages/maker-core/src/agents/base-agent.ts | AgentDeps 改为注入 reviewAutoPermissionAction（轻量 reviewer delegate），并在 SessionHandle 增加 useCindyAutoReviewFallback。 |
| apps/desktop/src/main/utility-model/oneShotCandidates.ts | one-shot 文本请求增加 reasoningEffort，并对部分私有 Responses 端点禁用 max_output_tokens。 |
| apps/desktop/src/main/utility-model/tests/oneShotCandidates.test.ts | 覆盖 reasoning.effort 传递与 max_output_tokens 禁用行为。 |
| apps/desktop/src/main/maker-ipc/register.ts | Claude 原生分类器不可用时不再持久化/广播 Auto→Ask，改为运行期切到 Cindy fallback。 |
| apps/desktop/src/main/maker-host/index.ts | 注入 createAutoPermissionReviewer + requestUtilityText，作为 host 侧 reviewAutoPermissionAction 实现。 |
| apps/desktop/src/main/maker-host/claude-auto-permission-fallback.ts | fallback coordinator 语义改为“保持 Auto，仅切换 reviewer”，并移除持久化/广播。 |
| apps/desktop/src/main/maker-host/auto-permission-reviewer.ts | 新增轻量 reviewer：最小 prompt、输入压缩、输出严格解析与日志脱敏。 |
| apps/desktop/src/main/maker-host/tests/claudeAutoPermissionFallback.test.ts | 更新 fallback 行为单测：不改持久态，仅触发 useCindyAutoReviewFallback。 |
| apps/desktop/src/main/maker-host/tests/autoPermissionReviewer.test.ts | 新增 host 侧 reviewer prompt/parse/故障兜底与日志不泄漏动作内容的测试。 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: zqchris <chrisz83@gmail.com>
Signed-off-by: zqchris <chrisz83@gmail.com>
173d91c to
0529d63
Compare
Signed-off-by: zqchris <chrisz83@gmail.com>
0529d63 to
e824b0f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/maker-core/src/agents/codex/index.ts:2803
- autoReviewDecisionCache 的 key 直接 JSON.stringify(request) 会把 action.command/path、workspaceRoots 等原始字符串完整塞进 Map key；当命令/路径很长或 roots 很多时会造成不必要的大字符串分配与内存占用。建议对 key 使用截断/裁剪后的“等价”请求（与 host reviewer 的 compact 上限一致），避免缓存本身成为热点。
action,
workspaceRoots: runtimeWorkspaceRoots().filter(
(dir): dir is string => typeof dir === 'string' && dir.length > 0,
),
platform: sessionReviewPlatform,
};
const key = JSON.stringify(request);
const cached = autoReviewDecisionCache.get(key);
if (cached) return cached;
packages/maker-core/src/agents/claude-code/index.ts:1588
- autoReviewDecisionCache 的 key 直接 JSON.stringify(request) 会把 action.command/path、workspaceRoots 等原始字符串完整塞进 Map key；在 Claude 侧 action 还可能包含 URL/query 等较长文本，导致大量大字符串分配。建议用与轻量 reviewer prompt 同步的截断/裁剪 key（不需要包含 sessionId 等短字段），避免缓存 key 过大。
const request = {
sessionId: opts.sessionId,
agentKind: 'claude-code' as const,
providerId: mutableProviderId,
model: mutableModel,
userIntent: currentAutoReviewIntent,
action,
workspaceRoots,
platform,
};
const key = JSON.stringify(request);
const cached = autoReviewDecisionCache.get(key);
if (cached) return cached;
Signed-off-by: zqchris <chrisz83@gmail.com>
Signed-off-by: zqchris <chrisz83@gmail.com>
Signed-off-by: zqchris <chrisz83@gmail.com>
e824b0f to
b707fda
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/maker-core/src/agents/shared/auto-review.ts:172
- 当前把执行影响型环境变量注入（如 LD_PRELOAD / DYLD_* / BASH_ENV / PROMPT_COMMAND / PS4）仅归为灰区走轻量 reviewer，这意味着 reviewer 一旦误判为 allow，会在无用户确认的情况下放行典型 RCE 注入。建议将这些“明确的本地代码执行注入”提升为确定性红线（prompt-each-time），只把相对常见的 PATH/PAGER 等留在灰区。
/\bchmod\b[^|;&]*\s[ugoa]*[oa][ugoa]*[-+=][^\s]*w/, // chmod 符号型对 other/all 开放写(a+w / o+w / a+rwx)
...CREDENTIAL_PATH_PATTERNS, // 凭证/密钥路径(见上)
/\bsecurity\s+(?:find|dump|export|add)-/, // macOS keychain
/\$\{?[A-Za-z0-9_]*(?:KEY|TOKEN|SECRET|PASSWORD|PASSWD|CREDENTIAL|APIKEY|_PAT)[A-Za-z0-9_]*\}?/i, // 敏感环境变量展开(echo "$API_KEY" 等)
];
packages/maker-core/src/agents/shared/auto-review.test.ts:515
- 这里的“执行影响型环境变量赋值”用例现在全部断言为 prompt（灰区）。如果按上面建议把 LD_PRELOAD / DYLD_* / BASH_ENV / PROMPT_COMMAND / PS4 这类明确 RCE 注入提升为 prompt-each-time，这个测试需要拆分：高危变量应为 prompt-each-time，其余（如 PATH/PAGER）仍可保持 prompt。
it('执行影响型环境变量赋值(LD_PRELOAD/PAGER/PATH/DYLD)→ AI 灰区', () => {
for (const c of [
'env LD_PRELOAD=/repo/payload.so /usr/bin/true',
'env PAGER=./payload git --paginate log',
'env GIT_PAGER=./p git -p log',
Signed-off-by: zqchris <chrisz83@gmail.com>
Signed-off-by: zqchris <chrisz83@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/maker-core/src/agents/shared/auto-review-decision.ts:138
resolveAutoReviewDecision在接受 delegate 返回值时直接原样返回 decision；如果某个 host delegate（或未来实现）返回了超长/非字符串的 reason，可能导致后续日志/UI/LLM 反馈携带过大文本。建议在这里做一次防御性规范化：仅接受 string reason，并裁剪到固定上限。
try {
const decision = await delegate(request);
if (
decision?.verdict === 'allow'
|| decision?.verdict === 'block'
|| decision?.verdict === 'ask'
) {
return decision;
}
…routing Signed-off-by: zqchris <chrisz83@gmail.com> # Conflicts: # apps/desktop/src/main/utility-model/oneShotCandidates.ts # packages/maker-core/src/agents/base-agent.ts # packages/maker-core/src/agents/claude-code/index.ts # packages/maker-core/src/agents/codex/index.ts # packages/maker-core/src/agents/shared/auto-review-decision.test.ts # packages/maker-core/src/agents/shared/auto-review-decision.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f0215283d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@zqchris 👋 这个 PR 还有 3 条 review conversation 没 resolve(packages/maker-core/src/agents/claude-code/index.ts / packages/maker-core/src/agents/shared/auto-review.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
三条 bot 反馈: 1) SSH 远端 `ask_user_question` 回调返回答案前没有并入审查意图 —— 同一个远端回调只为 plan_review 更新了意图(第三十八批修的),ask 分支漏了。用户在远端把范围从 src/ 收窄到 build/ 后,后续工具 仍按澄清前的意图裁决,越界操作可能被轻量 reviewer 静默允许。补 composeAutoReviewIntentWithClarification (顺带清空裁决缓存),与本地 AskUserQuestion 分支对称。 2) 归档/下载的落地选项在**短选项簇**里解析不到:原正则只认以 `-C`/`-o`/`-O`/`-d`/`-P` 开头的 token, `tar -xC /etc -f p.tar`、`unzip -oqd /etc p.zip`、`curl -so/etc/hosts URL`、`wget -qO/etc/hosts URL` 全部只落灰区。新增 shortClusterOption 按 getopt 语义解析:簇内**第一个**带值字母之后的字符即其值, 在簇尾则吃下一个 argv;要传该命令**全部**带值短选项字母,否则 `curl -do out URL` 会把 `-d` 的值 误当输出文件。cp/mv/install/ln 的 `-t` 目标目录同样改走簇语义。 3) chroot 既不在包装器集合也不在红线,`chroot / rm -rf /outside` 的内层命令完全没被看见。chroot 与 sudo/su 同族(需 CAP_SYS_CHROOT),且**换根后绝对路径也重新指向新根下**(`chroot /mnt rm -rf /repo` 删的是 /mnt/repo)→ 目标作用域静态不可证,按确定性同意处理;与 `su` 一样只在命令位匹配, `git commit -m "fix chroot"` 不误升。 自审顺带修/补的两处(非 bot 报): - **rsync 的 `-t` 是 --times、不是目标目录**:原 `-t` 判定对 rsync 生效,会把 `rsync -avt /etc/nginx/ backup/` 的**读源**当写目标而误拦;改成只对 coreutils 的 cp/mv/install/ln 生效(修一处误拦)。 - 下载工具**不带落地选项**时按远端文件名写进当前目录(`curl -O URL`、`wget URL`),cwd 落系统目录即写 系统文件(与第四十三批"解压落 cwd"同类)→ 以 `.` 为写目标交有效-cwd 解析;curl 默认写 stdout, 只有 -O/--remote-name 系才算落盘,`curl -sSL URL | sh` 不受影响。 验证:双向语料 —— 22 条危险(六种簇形态 × tar/unzip/curl/wget、`cp -ft /etc`、`wget -o` 日志落盘、 `curl -O`/`wget` 在 /etc 下、`cd /etc && wget`、chroot 三形态)全部必问,改前 17 条只落灰区; 22 条良性(区内落地目录、`wget -qO-`、`curl -sSL`、`curl -d @body.json`、`tar -czf`、`rsync -avt /etc/nginx/ backup/`、`git commit -m "fix chroot …"`、`rg chroot src`)零误拦。均固化为回归测试。 maker-core 1403 单测 + typecheck 全绿。适配器那条(远端 ask)与本地分支对称,compose 函数本身已有单测; 远端回调深在闭包内,未加针对性单测。 Signed-off-by: zqchris <chrisz83@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated no new comments.
Suppressed comments (1)
apps/desktop/src/main/maker-host/claude-auto-permission-fallback.ts:219
- 此处注释仍提到“持久态 CAS”，但当前 coordinator 已不再做条件持久化/compare-and-swap（只读 getSessionMeta + runtime fallback）。建议更新注释，避免误导后续维护者对并发/幂等性的理解。
// 任何一次通知(瞬时升级或确定性 4xx 立即切 fallback)都清零该会话的瞬时记账:
// 用户重开 Auto 时从零累计,不因残账被单次偶发失败提前推过阈值。协调器自身有
// in-flight 去重 + 持久态 CAS,重复信号安全。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 940694892c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@zqchris 👋 这个 PR 还有 1 条 review conversation 没 resolve(packages/maker-core/src/agents/shared/auto-review.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
…令纳入判定(第四十六批评审) bot 报的:`script -q -c 'rm -rf /outside' /dev/null` 会真的执行该命令,但 script 不在包装器集合里, 目标级分析只看到外层可执行文件,区外递归删除只落灰区。 script 有两种形态,都会跑命令,一并解析: - util-linux `script [opts] -c '<命令串>' [file]`:值经 shell 执行(含 `-c'…'` 附着与 `--command=` 形态); 带独立值的日志/管道选项(-T/-I/-B/-O/-m/-F 及长名)必须消费其值,否则解析会停在文件名而看不到 -c; `-t`(util-linux 的 --timing 可无值)刻意不消费 —— 少吃只会让它当 file 操作数被跳过,多吃可能把真正 的命令吞掉。 - BSD/macOS `script [opts] [file [command ...]]`:跳过选项与 typescript 文件后即内层 argv。 `-c` 缺值或没有内层命令(纯记录交互会话)时留壳 fail-closed,不解包成空。 顺带把同族的启动器一次补齐(不逐条等报):sg(`sg GROUP -c '<命令串>'`,缺 -c 时末位操作数同样是命令串)、 unbuffer(expect 的透明包装,唯一选项 -p 不带值,复用 setsid 分支)、busybox(applet 多路复用器)、 macOS 的 arch(`-arch/-e` 带值)与 caffeinate(`-t/-w` 带值)。选项处理统一遵循既有的**只少吃不多吃** policy:少吃会让选项值当命令名 → 未知 bin → 灰区 fail-closed,多吃会把真正的 rm 吞掉 → 漏红线。 验证:双向语料 —— 18 条危险(script 七形态含叠加 env、sg 两形态、unbuffer/busybox/arch/caffeinate) 全部必问,改前 **18 条全部只落灰区**;16 条良性(区内命令、`script -q /tmp/typescript` 无内层命令、 裸 arch/caffeinate、`rg "script -c" src`、`git commit -m "add script -c wrapper"`)零误拦。 均固化为回归测试。maker-core 1406 单测 + typecheck 全绿。 Signed-off-by: zqchris <chrisz83@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated no new comments.
Suppressed comments (1)
apps/desktop/src/renderer/i18n/locales/zh-CN/common.json:4687
- 该中文描述里“自动审批提权请求”语义更偏向“自动批准”，与同一条目的英文/日文/韩文“reviews/レビュー/검토”(自动审查)不一致，也与本 PR 的三态裁决语义（高风险可能被拒绝或要求确认、灰区可 block）有冲突。建议把“自动审批”改为“自动审查/自动评审”以避免用户误解为必然放行。
"auto": {
"label": "自动审批",
"description": "允许在工作区内读写，并自动审批提权请求；高风险操作可能被拒绝或要求确认。能减少打断，但存在误判风险。"
},
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9517b43a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@zqchris 👋 这个 PR 还有 1 条 review conversation 没 resolve(packages/maker-core/src/agents/shared/auto-review.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
两条 bot 反馈: 1) `tar xCf /etc payload.tar` —— GNU/BSD tar 都接受传统无横线选项词,原先只认 `-` 开头的 token, 既判不出解压模式也取不到 `/etc`。关键是这种写法的**带值字母按出现顺序依次取后面的操作数** (`xCf /etc p.tar` → C=/etc、f=p.tar),与 getopt 簇的"附着值"语义(`-Cf DIR FILE` 里 C 的值是 字面 `f`)完全不同,不能复用 shortClusterOption。新增 tarOldStyleOptionWord/tarOldStyleValues, 只把**首个**参数按传统选项词解析,且要求含功能字母(x/c/t/r/u/A/d),避免 `tar dist` 这类目录名 被当成选项词。isArchiveExtraction 与 `P`(--absolute-names)判定同步认传统写法。 2) `chmod 000 /etc/passwd` / `chown attacker /etc/passwd` —— 改的是**访问控制**,与改内容同等危险; 既有红线只覆盖 chmod 777 / 全局开放写这类"放宽"形态,收紧与换属主完全没覆盖,提取器对 chmod/chown/chgrp 返回空目标。现在把 FILE 操作数当写目标复用系统路径判定,同族的 chflags/chattr/setfacl 一并纳入。 解析上的坎:chmod 的符号模式与 chattr 的属性词可以 `-`/`+`/`=` 起头(`chmod -w f`、`chmod +x f`、 `chattr +i f`),当成选项跳过会把**真实目标**误当规格操作数吃掉 → 先正面识别规格词;大小写敏感, `-R`(递归)不落进 `-[rwxXstugo]+` 仍按选项跳过。`--reference=RFILE` 从参考文件取模式 → 没有规格 操作数,首个操作数就是目标;setfacl 的 ACL 由 -m/-x/-M/-X 给出,同样无规格操作数。 验证:双向语料 —— 20 条危险(tar 四种传统写法 + cwd 落 /etc 的传统解压、chmod 数字/符号/收紧/ --reference、chown/chgrp/chflags/chattr/setfacl 写系统路径、`chmod 600 /usr/bin/node`,以及与 -exec 递归、`cd /etc &&` 有效-cwd 的组合)全部必问,改前 **20 条全部只落灰区**;17 条良性 (区内解压与落地目录、`tar cf`/`tar tvf` 打包列出、`tar dist`、区内 chmod/chown、`chmod +x scripts/…`、 `chmod 755 /usr/local/bin/tool`、`rg "chmod 000" docs`)零误拦。均固化为回归测试。 maker-core 1409 单测 + typecheck 全绿。 Signed-off-by: zqchris <chrisz83@gmail.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/maker-core/src/agents/claude-code/auto-review-policy.ts:125
- 这里把 WebSearch 的 query 原样作为 network.target 传给 core。core 的 internal/metadata 判定（isInternalFetchTarget）会把任意字符串当“类似 URL 的目标”来剥 scheme/端口/空白，因此像
localhost:3000/169.254.169.254这类搜索词可能被误判为“内网/metadata 抓取”而直接升级到prompt-each-time，与注释里“WebSearch 查询词仍走灰区”的语义不一致。建议：要么让 core 仅对 WebFetch 做 internal host 判定（按 operation 分支），要么在这里对 WebSearch 的 target 做不可被 URL 解析的包裹编码（例如search:<query>），并同步更新对应单测。
if (toolName === 'WebFetch' || toolName === 'WebSearch') {
return {
kind: 'network',
operation: toolName,
target: extractNetworkTarget(toolName, input),
apps/desktop/src/renderer/i18n/locales/zh-CN/common.json:4686
- zh-CN 文案里“自动审批提权请求”与 en/ja/ko 的“自动 review/검토/レビュー 昇格请求”语义不一致；同时本 PR 新增的后半句已说明高风险可能被拒绝或要求确认，因此“自动审批”容易造成误解。建议改为“自动审核提权请求”以与其它语言及当前行为对齐。
"description": "允许在工作区内读写，并自动审批提权请求；高风险操作可能被拒绝或要求确认。能减少打断，但存在误判风险。"
There was a problem hiding this comment.
💡 Codex Review
将 sort 的输出文件纳入系统路径红线
当 Auto 执行 sort -o /etc/passwd /tmp/input 时，本机 sort --help 明确将 -o, --output=FILE 定义为把结果写入 FILE；这里虽然识别该选项并取消只读放行，却只返回模型可审查的 prompt，而 argumentWriteTargets 没有提取这个静态可见的输出目标，因此覆盖系统文件时仍可能被 reviewer 静默允许。请提取 -o FILE、-oFILE 与 --output=FILE 并复用受保护系统路径判定。
AGENTS.md reference: AGENTS.md:L69-L71
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@zqchris 👋 这个 PR 目前与 请在本地 merge 最新的 |
main 上已并行落了同一份设计的 auto-review 核心(ed4dc8b「keep native-first fallback quiet」 + pi 线的 6cfd122/cacd1cf0),与本支 53 个提交在同一批文件上语义冲突。解法按"谁在该维度是超集" 分文件定: - **适配器(claude-code / codex / base-agent / maker-host / 三个适配器测试)取 main**: main 在**路由维度**是超集 —— 官方 Claude OAuth 保留 CC 原生 Auto classifier、第三方/网关与原生 失效后才落 Cindy 兜底(usesNativeClaudeAutoReview),另有本支没有的热切换收口(审查期间 setPermissionMode 收紧/放宽按最新档位决策)与 setModel 后的 auto 审查重配 best-effort。 - **确定性分类器(shared/auto-review.ts 及其测试)取本支**:main 版 759 行,本支 2488 行,含 第十六~四十七批评审的全部加固(系统路径写红线、包装器/启动器解包、短选项簇与 tar 传统选项词、 find -exec 递归、权限属主变更、云 metadata、伪设备白名单等),main 一项都没有。 同时采纳 main 的结构化改动:凭证正则抽到 shared/sensitive-credential-paths.ts(改用 SENSITIVE_CREDENTIAL_PATH_PATTERNS,已核对 10 条正则无丢失)、ReviewableAction.other 增 description。 - **本支独有的「审查意图同步」族重新贴回 main 的适配器**:澄清答案并入意图(cc 本地 + cc 远端 + codex requestUserInput)、计划获批/修订 turn 携带"原始意图 + 获批计划"(cc 两处 + codex,含 CODEX_AUTO_REVIEW_INTENT sendOptions 透传与计划请求时的意图快照)、Guardian 失效时同 turn 立即 pushThreadSettings({approvalsReviewer:'user'})、exec 审查动作的 cwd 三态。 **一处刻意保留的档位分歧(需 Chris 拍板是否回退)**:`curl x | sh` / `| bash` / `eval`、区外 `rm -rf /tmp/x`、`find . -delete`(遍历根=工作区根)、`git push --force origin main`、写系统目录 在本支是确定性红线(prompt-each-time),在 main 是灰区(prompt,可被轻量 reviewer 静默 allow)。 本支这些红线是第十六批起多轮 bot 评审的直接产物(理由:静态可证的任意代码执行/区外破坏, reviewer 看不到载荷内容,不能静默放行)。因此把 main 侧这些断言按本支档位更新;若要回退成 main 的"全交 reviewer",改动点集中在 highImpactExecutionNeedsConsent / scopedDestructionNeedsConsent 与对应测试。main 用系统路径当"灰区 fixture"的 wiring/dispatch 测试改用 /tmp/... 非系统路径, 保持原测试意图(灰区由 reviewer 裁决)。 验证:submodule 对齐 main 的 cindy-protocol ef3a90d2 + pnpm install(main 新增 @cindy/model-access-protocol workspace 包);maker-core 1507 单测全绿、typecheck 绿; apps/desktop 定向 maker-host + maker-ipc 155 文件 2469 测试全绿、desktop typecheck 绿。 Signed-off-by: zqchris <chrisz83@gmail.com>
bot 报的:`rm -- /etc/passwd` 不带 `-rf`,原先只有出现 rRfF/--recursive/--force/--dir 才提取目标, 普通单文件删除完全取不到目标 → 只落灰区。删除本身就是写通道,现在把删除目标纳入写通道提取,复用 受保护系统路径判定;**区外批量破坏**仍由 destructiveRmTargets 的递归/强制条件负责,故 `rm -rf build` 这类区内删除档位不变。 覆盖 rm / unlink / shred / srm(shred 的 `-n` 次数、`-s` 字节、`--random-source` 是带值选项,不能 当删除目标)与 cmd.exe 的 del / erase(开关形如 `/f` `/s` `/q` `/a:-h`,Windows 路径不会以单个 `/`+ 字母起头)。 自审顺带补的同族缺口:**mv 的源操作数同样被销毁** —— `mv /usr/bin/node /tmp/` 等于删掉系统程序, 原先只把末位目标当写目标。cp/install/ln/rsync 的源是只读的,不在此列。 验证:双向语料 —— 11 条危险(rm/unlink/shred 删系统文件、mv 搬走系统文件、`cd /etc && rm passwd`、 `find . -exec rm /etc/passwd`)全部必问,改前 **11 条全部只落灰区**;11 条良性(区内删除、`rm -rf build`、 `rm -- build/x`、`mv src/a.ts src/b.ts`、`mv build/x /usr/local/lib/`、`rm /tmp/scratch.txt`、 `rm >/dev/null`)零误拦。均固化为回归测试。maker-core 1534 单测 + typecheck 全绿。 Signed-off-by: zqchris <chrisz83@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
packages/maker-core/src/agents/claude-code/index.ts:2442
- 计划获批后这里用
currentAutoReviewIntent组装实施阶段的审查意图，但该值可能已在 plan_review 等待期间被后续消息覆盖；应改用进入 plan_review 分支时快照的planRequestAutoReviewIntent，与本地 ExitPlanMode 分支的处理保持一致。
setAutoReviewIntent(composeAutoReviewIntentWithApprovedPlan(
currentAutoReviewIntent,
decision.editedPlan ?? plan,
copilot 报:远端 plan_review 分支用的是 await 之后的 currentAutoReviewIntent。审批等待期间用户可以 继续发消息(send 会 setAutoReviewIntent 覆盖它),于是实施阶段的审查意图会丢掉原始请求、掺进审批期间 的内部跟进。改成进入分支时先快照,与本地 ExitPlanMode 分支的 planRequestAutoReviewIntent 同款。 这是我上一轮把"意图同步"族重新贴回 main 适配器时留下的不一致:本地分支带了快照,远端分支没带。 顺带把两个适配器里所有"await 后拼装意图"的点都过了一遍,确认剩下的锚点是对的: - cc 本地 ExitPlanMode / codex runPlanReviewFlow:已有快照。 - 澄清(cc 本地 + cc 远端 + codex requestUserInput)**刻意仍用 await 后的当前意图** —— 这里不能快照: 若审问期间用户发了新消息,新消息才是 agent 正在执行的请求,用快照会把它覆盖回旧意图,反而是回退。 计划审批不同:计划是为原始请求起草的,实施 turn 实施的是那份计划。 maker-core 1534 单测 + typecheck 全绿。远端回调深在 session 闭包内,与前几轮同样未加针对性单测。 Signed-off-by: zqchris <chrisz83@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (3)
packages/maker-core/src/agents/claude-code/index.ts:2414
- 远端 approval 回调里 ask_user_question 分支同样用
Object.entries(decision.answers)生成澄清列表：这会把 header/id 等重复键或 resolver 返回的额外键并入 intent，且顺序不稳定，影响裁决缓存与安全边界。建议复用本次实际下发的 questions 列表（params/questions 或 input/questions）按顺序生成澄清，并按 (question→header) 回退取值，只合并“真的问过的问题”。
setAutoReviewIntent(composeAutoReviewIntentWithClarification(
currentAutoReviewIntent,
Object.entries(decision.answers ?? {}).map(([question, answer]) => ({ question, answer })),
));
packages/maker-core/src/agents/codex/index.ts:4972
- 这里把澄清问答并入 auto-review intent 时使用了
Object.entries(decision.answers)。answers的 key 允许是 question/header/id（types/events.ts 里有注明），直接遍历会带来：1) 顺序不稳定，影响同轮裁决缓存命中；2) 可能把 header/id 等重复键都写进 intent；3) 若 resolver 返回了额外键，会把未展示给用户的问题文本也并入 intent，扩大 prompt 注入/误导裁决的面。建议按原始questions顺序生成澄清列表，并用与responseFromAskUserAnswers一致的 key 回退规则取答案，从而只合并“本次真正问过的问题”。
setAutoReviewIntent(composeAutoReviewIntentWithClarification(
currentAutoReviewIntent,
Object.entries(decision.answers ?? {}).map(([question, answer]) => ({ question, answer })),
));
packages/maker-core/src/agents/claude-code/index.ts:1498
- 本地 AskUserQuestion 分支在把澄清问答并入 auto-review intent 时直接
Object.entries(decision.answers)，会把 header/id 等重复键或 resolver 返回的额外键一并写入 intent，且顺序不稳定，可能影响裁决缓存命中，并扩大 prompt 注入/误导裁决的面。建议按本次实际展示的questions顺序生成澄清列表，并只从 decision.answers 中按 (question→header) 回退取值。
This issue also appears on line 2411 of the same file.
setAutoReviewIntent(composeAutoReviewIntentWithClarification(
currentAutoReviewIntent,
Object.entries(decision.answers ?? {}).map(([question, answer]) => ({ question, answer })),
));
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6250cea372
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!cluster) continue; | ||
| if (cluster.consumedNext) i++; | ||
| if (cluster.letter !== 't') continue; | ||
| return cluster.value ? [cluster.value] : [UNPROVABLE_WRITE_TARGET]; |
There was a problem hiding this comment.
当使用 mv -t /tmp /etc/passwd 或 mv --target-directory=/tmp /etc/passwd 时，这里命中目标目录后立即返回，只检查安全的 /tmp，跳过了同样会被移走的系统源文件；本机 mv --help 明确其第三种用法为 mv ... -t DIRECTORY SOURCE...，实测分类结果也从普通 mv /etc/passwd /tmp/ 的 prompt-each-time 降为可被 reviewer 静默允许的 prompt。与已覆盖的普通 SOURCE DEST 形态不同，新证据是 target-directory 分支的提前返回；请在 mv 情况下把源操作数与目标目录一起纳入系统路径检查。
AGENTS.md reference: AGENTS.md:L149-L150
Useful? React with 👍 / 👎.
| ...(params.cwd?.trim() | ||
| ? { cwd: params.cwd } | ||
| : params.cwd === undefined | ||
| ? { cwd: opts.workingDir } | ||
| : { cwdUnknown: true }), |
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
审查通过（heavy tier，含安全扫描）。零 P0/P1，代码实现正确、测试覆盖充分。
唯一 P2：auto-review.ts:63 注释 typo 'copidot' → 'copilot'，不阻断。
structural-check 未上报但 CI 全绿、review 通过，admin bypass 合入。
|
把 auto-review 从「宿主审批或什么都不审」升级到「宿主优先 + 当前模型静默兜底」,同时把系统路径写入从灰区提到逐次确认——既没砍掉 Guardian/canUseTool 的原生保护力,又补上了它们不在场时的安全网。测试覆盖也很扎实 🛡️ |
这次改了什么
摘要
修正 Auto-review 的产品语义：用户选择 Auto 后，日常开发动作不应不断弹人工确认。
default + canUseTool，保留宿主 MCP、逐次确认与 AskUserQuestion 语义；Codex 官方 OpenAI OAuth 保留 Guardian。auto。两端都不持久化为 Ask，也不广播 Auto→Ask。allow/block/ask。只有ask才能弹用户，模型故障、超时或畸形输出都静默block，让主 Agent 换安全做法。变更类型
feat新功能fix缺陷修复refactor/perf重构或性能优化docs/test/chore文档、测试或工程维护范围
UI 变化
仅 UI 文案：权限档选择器里 Claude Code「Auto-review」档的说明补一句「高风险操作可能被拒绝或要求确认」（en / zh-CN / ja / ko 四语）。没有新增或修改界面、布局、样式、动效或交互控件。
改动理由：本 PR 让 Claude 与 Codex 的 auto 档共用同一套确定性核心，而 claude-code 的原说明只讲「自动审批提权请求…能减少打断」，没提确定性红线仍会逐次征求确认（codex 档的说明本就有这句）。用户据此会以为 Auto 完全不打扰，实际偶发弹窗，预期不符。
common.json全部更新且各自符合本语言的大小写／标点约定（en 句式大小写、zh-CN 全角标点无句末句号约束不适用于正文说明、ja。、ko.）；无「成功／successfully」填充词；非按钮／非错误／非进行中文案，对应条目不适用。术语门禁pnpm check:i18n-glossary与scripts/check-i18n.mjs（6228 key 四语一致）均通过。怎么验证的
自动验证
手工验证
用代表性的当前 Claude 与 Codex 会话模型发送生产形状的极小 reviewer prompt：
allow；allow；ask；max_output_tokens，请求成功返回紧凑 JSON。reviewer 请求使用低推理强度、8 秒超时；输入有 2,000 字符意图 / 4,096 字符动作 / 8 个工作区根上限，超长灰区动作不会截断后送审，而是在调用模型前静默 block；响应超过 1,024 字符或不符合三态 schema 时同样按失败处理。
未执行的验证
pnpm test:unit：该机器曾被全仓高并发测试卡死，本地按定向单 worker 验证，完整门禁交由 CI。main已存在的 unused-variable 项，本次 diff 未新增 lint finding。最终以 CI 为准。风险
风险分类
影响与回滚
提交前检查
git commit -s，见 DCO）