feat(audit): add legacy code audit workflow - #9749
Conversation
Adds `/audit <directory> [--effort low|medium|high]` for auditing existing modules where there is no diff, no PR, and no baseline, together with the deterministic CLI helpers the skill drives: argument parsing, filesystem planning, agent-brief printing, run-state captures, drift checkpoints, the local-only guard, and write-time anchor resolution. The roster, gates, budget estimate, and captures are computed in code so the orchestrator cannot shrink or skip them. Medium runs the nine dimension agents plus verification shards; high adds the 6b/6c personas and iterative reverse-audit rounds; low is a single reader producing capped, explicitly unverified triage. Every consumer of module content opens with an untrusted-data preamble, the walks are read-only, and the two execution classes are separate opt-ins at an interactive pre-launch confirmation. The write-time gate resolves a machine-readable findings manifest rather than parsing the report's markdown: the report is unbounded LLM-authored text, so a parser's nets are open-ended by construction — a rendering it did not anticipate either produced zero findings (certifying snippets that were never resolved) or silently rebound a finding to another file. The manifest arrives typed and verbatim, and the report is held to it by a machine marker per finding, which no rendering, section layout, or output language can defeat. Artifact reads and writes are hardened against the audited tree the module already treats as hostile: content opens refuse symlinks and writer-less FIFOs in the kernel, artifact writes gate on the fd rather than an lstat that a swap can outrun, the capture's `git diff` refuses external diff and textconv drivers, the whole GIT_-prefixed environment is scrubbed the way core's shared ignore probe scrubs it, and the registered-caller channel gets the scope and secret policy the walk already had. Two limitations are accepted rather than closed, and every report says so: the estimate's per-line rates are author-reported, and the sidecar is not tamper-evident.
The content-side half of the secret detector matched a private-key armor string anywhere in a file's first chunk, which classifies a source file that merely QUOTES the armor as credential material and silently drops it from the audit. This repository has several such files — a secret scanner, an MCP client test, a serve test, and the detector's own module — so auditing any of their directories would have lost real subjects. A key file BEGINS with its armor line; a file that discusses one does not. Anchoring to the first non-blank line keeps every real key file detected (the name list is only a pre-filter, so an unguessed name still lands here) with no false positive left in the tree.
The manifest carries every finding's verbatim anchor snippet — the same module content the report draft carries, in the form the gate actually reads — so it is exactly as committable as the draft and needs its own ignore probe. SKILL.md already claimed the guard probed it. The gap was found by generalizing the existing shape-parity invariant from the specialist findings shape to EVERY ts-stamped artifact name SKILL.md instructs a run to write, with the skill as the oracle. Removing the new probe line turns that test red, so the next artifact added to the skill without a probe fails the same way instead of shipping unguarded.
|
Thanks for the PR — this is the clean re-landing of the
Moving on to code review. 🔍 中文说明感谢贡献——这是 #8403 经历 24 轮评审后
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
I reviewed the full diff at Code review findings1. A literal DEL byte (0x7F) in The strip regex is written with a raw 0x7F control byte in the character class (next to an
2. (Suggestion) The stdout-flush change in 3. (Suggestion) The What I checked and found solid: the O_NOFOLLOW/O_NONBLOCK + post-open fstat discipline is applied uniformly across every read, copy, and write; the sequenceDiagram
participant P1 as User
participant P2 as audit skill orchestrator
participant P3 as qwen audit subcommands
participant P4 as Roster agents
participant P5 as check-anchors gate
P1->>P2: directory plus effort
P2->>P3: parse-args, plan-files
P3-->>P2: plan JSON, or refusal exit 3
P2->>P1: pre-launch confirmation, execution opt-ins
P2->>P3: snapshot, run-start sidecar
P2->>P4: launch roster with printed briefs
P4-->>P2: findings per role
P2->>P3: drift-check and guard-check at checkpoints
P2->>P5: findings manifest plus report draft
P5-->>P2: marker and anchor verdicts, exit 4 on mismatch
P2->>P1: report at .qwen/audits, no verdict
Files changed (28 of 36 shown)
Test evidenceThis is an unattended CI run — I do not build or execute PR code. Evidence below is the PR's own CI, read through the API. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 On the original head ( Sandboxed verification would settle the load-bearing claims: 中文说明我在 发现(见英文部分详情):
已核查且可靠的部分:O_NOFOLLOW/O_NONBLOCK + 打开后 fstat 的纪律一致地应用于所有读、拷贝、写; 测试证据:无人值守 CI 运行,不构建或执行 PR 代码。原 head 上 Test 任务在工作流尺寸预检处失败( 沙箱验证可定论关键行为: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 2/5 — the architecture and engineering discipline are exemplary, but a concrete defect in the security path and a file GitHub cannot diff keep this from being approvable as-is. Stepping back: my independent proposal for "audit merged code with no diff" — deterministic planning/gates/captures in code, orchestration in a skill, fan-out roster with verification — is essentially what this PR builds, and the implementation exceeds it in rigor. The fail-closed posture is consistent everywhere I looked: a failed git probe is exposed rather than vacuously passing, an unreadable caller drifts rather than disappearing, a corrupt sidecar re-captures and says so. The manifest gate is the right answer to the fail-open markdown parsing it replaces, and the PR is honest about what it does not close (author-reported token rates, non-tamper-evident sidecar) — in the code, not just the description. The two fixes that landed mid-review (armor detection anchored to the first line, findings manifest probed by the guard) were both caught by this repo's own files, which is the calibration loop working. Why not approve, then:
Everything else — including the shared-path flush change and the review-group test recalibration — is suggestion-level and would not block on its own. If finding 1 lands and the suite is green, this should move quickly on a re-run: the rest of the review stands. Requesting changes on finding 1. 🙏 中文说明置信度:2/5 —— 架构与工程纪律堪称典范,但安全路径上的一个具体缺陷,加上一个 GitHub 无法 diff 的文件,使当前形态不可批准。 退一步看:我对"无 diff 审计已合入代码"的独立方案——确定性的规划/门限/捕获放在代码中、编排放进技能、扇出 roster 加验证——与本 PR 的构建基本一致,而实现的严谨度超出我的方案。失败即关闭的姿态在我看过的每一处都保持一致:git 探测失败会被暴露而非空转通过,不可读的 caller 会记为漂移而非悄悄消失,损坏的 sidecar 会重新捕获并如实声明。清单门是对其所替代的失败即开放式 markdown 解析的正确答案,且 PR 对未关闭项(作者自报的代币费率、无防篡改证据的 sidecar)在代码中而不仅在描述里保持诚实。审查期间落地的两个修复(armor 检测锚定首行、发现清单纳入 guard 探针)都是由本仓库自己的文件触发的——这正是校准循环在起作用。 那为何不批准:
其余一切——包括共享路径的 flush 改动与 review 组测试重校准——都是建议级别,单独不构成阻塞。若发现 1 修复且套件全绿,re-run 时应能快速推进:评审的其余部分维持不变。 就发现 1 请求修改。🙏 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
One concrete defect blocks this as-is — full detail in my review comments above.
packages/cli/src/commands/audit/lib/files-plan.ts, isSecretName (~line 153): the strip regex carries a literal DEL (0x7F) byte in its character class. That (a) makes GitHub render the entire 1,678-line file as binary — it is undiffable in this PR and would stay undiffable for every future change — and (b) contradicts the documented intent of the strip, leaving control-suffixed lowercase secret names (.env\r, .npmrc\r, app.key\r) able to pass the pre-filter and become content-read subjects. The fix is small: strip controls explicitly (/[\u0000-\u001f\u007f]/g), use an escape rather than a literal byte, and add CR/DEL-terminated lowercase cases beside the existing .env\n test.
Also unresolved for approval: no unit-suite CI result exists for this PR yet on either head (the original head failed a pre-existing workflow-size precheck before tests ran; the current head's Test job is still in progress). Green CI on the current head is a precondition; @qwen-code /verify and @qwen-code /tmux would additionally settle the behavioral claims noted in the Stage 2 comment.
Everything else in the review is suggestion-level and non-blocking — the rest of the work is in strong shape.
— Qwen Code · qwen3.8-max
Reviewed at 6c468993c7e31b42bfe3aa3e5406599648735c53 · re-run with @qwen-code /triage
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI; its platform-specific surface (O_NOFOLLOW/O_NONBLOCK degradation, drive-letter archive paths) did not run locally.
Not explored to full depth (tool budget reached): chunk 27: executing storage.test.ts (worktree and parent checkout have no node_modules; full npm ci + build exceeds viable budget) — verification was by code reading and …; chunk 11: none — well, one check was deliberately not done: how the orchestrator *handles* the buildLowReaderPrompt throws (crash vs friendly refusal) lives in a caller….
Not reviewed: "agent verify (round 2)" — pointed at diff lines it never opened: it made tool calls, but none of them read the diff.
[Critical] lib/files-plan.ts is committed as BINARY: a literal NUL byte and raw C0 control bytes sit inside the isSecretName strip regex (line 153 of the new file), so git classifies the entire ~1,700-line module as binary. The PR diff shows only Binary files /dev/null and … differ for it — the size gates, the local-only guard, the secret-name detection and the walk exclusions all ship without ever appearing in GitHub's diff view, git diff, blame, or patch tooling. tsc and prettier accept the file, so nothing in CI catches it, and every future change to these security-relevant guards stays undiffable until the bytes are fixed. Three agents flagged this independently. Suggested fix: write the character class with escapes — relPath.replace(/[\u0000-\u001f\u007f]+/g, '') — runtime-identical, and the file diffs as text again.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查:build-and-test — Test (windows-latest, Node 22.x) was skipped in CI; its platform-specific surface (O_NOFOLLOW/O_NONBLOCK degradation, drive-letter archive paths) did not run locally。
未探索到全部深度(达到工具调用预算):chunk 27:executing storage.test.ts (worktree and parent checkout have no node_modules; full npm ci + build exceeds viable budget) — verification was by code reading and …;chunk 11:none — well, one check was deliberately not done: how the orchestrator *handles* the buildLowReaderPrompt throws (crash vs friendly refusal) lives in a caller…。
未审查:"agent verify (round 2)"——启动 prompt 为它指定了 diff 中的行,但它从未打开:有工具调用,却没有一次读取 diff。
[Critical] lib/files-plan.ts is committed as BINARY: a literal NUL byte and raw C0 control bytes sit inside the isSecretName strip regex (line 153 of the new file), so git classifies the entire ~1,700-line module as binary. The PR diff shows only Binary files /dev/null and … differ for it — the size gates, the local-only guard, the secret-name detection and the walk exclusions all ship without ever appearing in GitHub's diff view, git diff, blame, or patch tooling. tsc and prettier accept the file, so nothing in CI catches it, and every future change to these security-relevant guards stays undiffable until the bytes are fixed. Three agents flagged this independently. Suggested fix: write the character class with escapes — relPath.replace(/[\u0000-\u001f\u007f]+/g, '') — runtime-identical, and the file diffs as text again.
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if (!captureDiffArm(rootAbs, sidecarDir, meta.headUnborn === true)) { | ||
| captureDegraded.push('diff'); | ||
| } |
There was a problem hiding this comment.
[Critical] The capture's git arms leave core.fsmonitor open: captureDiffArm/captureUntrackedArm spawn git diff and git ls-files --others with --no-ext-diff --no-textconv (whose comment says this closes the channel where the audited repo's own config names a program for the auditor to execute), but repo-local core.fsmonitor still executes at capture time. Auditing a hostile checkout is this module's stated purpose — and a vendored nested repo ships its own .git/config — so this is command execution as the auditor during the Step-3 snapshot.
Verified on git 2.43.0: with repo-local core.fsmonitor set to a marker-writing command, both exact audit argv forms fired it; adding -c core.fsmonitor=false stopped it.
Suggested fix: pass -c core.fsmonitor=false on every git spawn against the audited tree.
中文说明
捕获阶段的 git 臂未关闭 core.fsmonitor:captureDiffArm/captureUntrackedArm 以 --no-ext-diff --no-textconv 启动 git diff 与 git ls-files --others(其注释声称这关闭了"被审计仓库自身配置为审计者指定可执行程序"的通道),但仓库本地 core.fsmonitor 仍会在捕获时执行。审计敌意检出正是本模块的既定目标——嵌套引入的仓库自带 .git/config——因此这等于在 Step-3 快照期间以审计者身份执行任意命令。
已在 git 2.43.0 上验证:将仓库本地 core.fsmonitor 设为写标记的命令后,两种审计 argv 形态均触发执行;加上 -c core.fsmonitor=false 后不再触发。
修复建议:对被审计树的每次 git 启动都传 -c core.fsmonitor=false。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const registeredCallers = callers | ||
| ? readCallersFile(callers, 'check-anchors') | ||
| : []; |
There was a problem hiding this comment.
[Critical] check-anchors content-reads callers the sidecar refused. readCallersFile validates only absoluteness and resolveAnchors admits every entry — callerRefusal (secret-shaped / out-of-repo) is applied only inside captureSidecar, never here. SKILL.md passes the SAME unfiltered callers file to both commands, and SKILL.md:123 promises refused callers' content "is never read, so a finding cannot anchor in one". A manifest citing a refused path in locations makes readGuarded open it: the stdout verdict/matchCount is a content-match oracle on exactly the files the policy protects, and a resolved verdict ships a finding anchored in a refused file. Four review agents found this independently; a probe confirmed it end-to-end.
Witness: snapshot refused a planted prod.env caller (reason: secret-shaped); check-anchors on the same file returned verdict resolved, matchCount 1 and exited 0. Applying callerRefusal in check-anchors flips it to out-of-scope / exit 4.
Suggested fix: re-apply callerRefusal at this read site (derive the repo root from the plan) or pass the sidecar's admitted callerNames; grade anchors at refused callers out-of-scope.
中文说明
check-anchors 会读取内容被 sidecar 拒绝的 caller。readCallersFile 只校验绝对路径,resolveAnchors 接纳所有条目——callerRefusal(secret-shaped / 仓库外)只在 captureSidecar 内执行,此处从不执行。SKILL.md 把同一份未过滤的 callers 文件传给两个命令,且 SKILL.md:123 承诺被拒 caller 的内容"永不被读取,发现也不能锚定其中"。清单若在 locations 中引用被拒路径,readGuarded 就会打开它:stdout 的 verdict/matchCount 构成针对策略保护文件的内容匹配预言机;一旦 verdict 为 resolved,就会把锚定在被拒文件中的发现放行。四个评审 agent 独立发现此问题;探针端到端证实。
证据:snapshot 拒绝了植入的 prod.env caller(reason: secret-shaped);check-anchors 对同一文件返回 verdict resolved、matchCount 1 并以退出码 0 结束。在 check-anchors 中应用 callerRefusal 后翻转为 out-of-scope / 退出码 4。
修复建议:在此读取点重新应用 callerRefusal(从 plan 推导仓库根),或传入 sidecar 已接纳的 callerNames;被拒 caller 上的锚定判为 out-of-scope。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred to the next round per the per-round batch cap (Critical/Required first, ~8 findings implemented per round). This round's batch covered the command-execution (core.fsmonitor), artifact-write (hardlink overwrite), sidecar-containment, relocation-departure-proof, and anchor-certification clusters; this finding stays queued first for the next round. The thread remains open.
中文说明
由于每轮批次上限(优先处理 Critical/Required,每轮实现约 8 个发现),本发现推迟到下一轮。本轮批次覆盖了命令执行(core.fsmonitor)、产物写入(硬链接覆盖)、sidecar 收容、迁移离开证明与锚点认证这几个簇;该发现在下一轮优先处理。主题保持打开。
| ); | ||
| } | ||
| try { | ||
| if (!fstatSync(fd).isFile()) { |
There was a problem hiding this comment.
[Critical] writeFileGuarded passes a planted HARD LINK — arbitrary host-file overwrite through the artifact write. O_NOFOLLOW refuses only symlinks; a hard link is a regular file, so the isFile() gate passes, O_TRUNC truncates the linked host inode and the artifact payload lands in it. The audited agent can plant ln ~/<host file> <sidecarDir>/sidecar.json any time before an extend re-run — no race needed — hitting ~/.bashrc or any UID-writable file. fs.protected_hardlinks does not block same-UID plants (verified: the link succeeded).
Probe: planted hard link — writeThrew: null, hostFileOverwritten: true. Refusing nlink > 1 post-open stops content injection, but O_TRUNC fires at open BEFORE the gate, so the complete remedy also defers truncation past the gate (post-gate ftruncate kept the host file intact and all 9 safe-read tests passed).
Suggested fix: if (!st.isFile() || st.nlink > 1) throw … and defer O_TRUNC until after the gate.
中文说明
writeFileGuarded 会放行植入的硬链接——通过产物写入实现任意宿主文件覆盖。O_NOFOLLOW 只拒绝符号链接;硬链接是普通文件,isFile() 门通过,O_TRUNC 截断被链接的宿主 inode,产物内容写入其中。被审计的 agent 可在 extend 重跑前的任意时刻植入 ln ~/<宿主文件> <sidecarDir>/sidecar.json——无需竞态——可命中 ~/.bashrc 或任何 UID 可写文件。fs.protected_hardlinks 不阻止同 UID 植入(已验证:链接成功)。
探针:植入硬链接后——writeThrew: null、hostFileOverwritten: true。打开后拒绝 nlink > 1 可阻止内容注入,但 O_TRUNC 在打开时先于门生效,因此完整修复还需把截断推迟到门之后(门后 ftruncate 的变体保持宿主文件完好,且 9 个 safe-read 测试全部通过)。
修复建议:if (!st.isFile() || st.nlink > 1) throw …,并将 O_TRUNC 推迟到门之后。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| mkdirSync(dirname(dest), { recursive: true }); | ||
| copyGuarded(caller, dest, CALLER_MAX_BYTES); |
There was a problem hiding this comment.
[Critical] Sidecar content copies follow symlinked INTERMEDIATE directories under the sidecar root, escaping the containment the file header claims ("no symlink is followed … no agent-named path escapes its containment"). O_NOFOLLOW in writeFileGuarded checks only the final component. The sidecar lands inside the audited tree by default, and the module's own comment says the adversary can swap paths mid-fan-out: replace <sidecar>/callers with a symlink to $HOME before the extend, and the caller copy is written OUTSIDE the sidecar. The untracked arm (same shape below) has fully attacker-controlled relative paths: an untracked subject named .bashrc plus a planted untracked -> $HOME symlink lands attacker content on $HOME/.bashrc.
Probe: capture → plant callers symlink → extend: files landed outside the sidecar with escaped content; with a realpath containment check before the copy there was no escape and all 38 sidecar tests passed.
Suggested fix: verify realpath containment of dirname(dest) against the sidecar root before every sub-write (re-verify on the extend path); race-free variant: descend with O_DIRECTORY|O_NOFOLLOW-opened directory fds.
中文说明
sidecar 内容拷贝会跟随 sidecar 根目录下的中间目录符号链接,逃逸了文件头声称的封闭域("不跟随任何符号链接……任何 agent 命名的路径都不逃逸")。writeFileGuarded 的 O_NOFOLLOW 只检查末位组件。sidecar 默认落在被审计树内,模块自身注释也承认对手可在 fan-out 中途替换路径:在 extend 前把 <sidecar>/callers 换成指向 $HOME 的符号链接,caller 拷贝就会写到 sidecar 之外。untracked 臂(下方同形态)的相对路径完全由攻击者控制:一个名为 .bashrc 的 untracked 主题文件,加上植入的 untracked -> $HOME 符号链接,可把攻击者内容落到 $HOME/.bashrc。
探针:捕获→植入 callers 符号链接→extend:文件携带逃逸内容落在 sidecar 之外;在拷贝前加入 realpath 封闭域检查后无逃逸,且 38 个 sidecar 测试全部通过。
修复建议:每次子写入前校验 dirname(dest) 的 realpath 位于 sidecar 根之下(extend 路径需重新校验);无竞态变体:用 O_DIRECTORY|O_NOFOLLOW 打开的目录 fd 逐层下行。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const stat = fs.lstatSync(dir); | ||
| if (!stat.isDirectory()) { |
There was a problem hiding this comment.
[Critical] getAuditFallbackDir validates only the LEAF: a symlink planted on the intermediate audits component redirects the private 0700 landing into an attacker-chosen tree while every check passes. mkdirSync({recursive:true}) follows the symlink; lstatSync(dir) then reports on the real leaf (lstat resolves intermediates), so isDirectory and the chmod-tightening both pass. Ahead of the first run, plant ln -s /tmp/shared-dir ~/.qwen/audits — the relocation then moves the report and sidecar (quoting possibly-exploitable module content) into the attacker tree. planRelocated/fallbackLandingSafe realpath both sides, normalizing the redirection away and crediting containment — downstream guards confirm rather than catch.
Probe: planted audits symlink — the function returned the landing as safe while the leaf was physically inside the attacker tree (isDirectory: true, mode 0700); rejecting a symlinked audits component flipped it to a clean refusal.
Suggested fix: after mkdir, compare realpathSync(dir) against realpathSync(getGlobalQwenDir()) + '/audits/' + hash and throw on mismatch (still tolerates a legitimately symlinked QWEN_HOME).
中文说明
getAuditFallbackDir 只校验叶子节点:在中间 audits 组件上植入符号链接,就能把私有的 0700 落点重定向到攻击者选定的目录树,而所有检查都会通过。mkdirSync({recursive:true}) 会跟随符号链接;随后 lstatSync(dir) 报告的是真实叶子(lstat 会解析中间组件),因此 isDirectory 与 chmod 收紧都会通过。在首次运行前植入 ln -s /tmp/shared-dir ~/.qwen/audits——重定位就会把报告与 sidecar(引用可能可利用的模块内容)搬进攻击者的目录树。planRelocated/fallbackLandingSafe 对两侧做 realpath,把重定向归一化掉并判定封闭域通过——下游守卫是在确认而非拦截。
探针:植入 audits 符号链接后——函数返回落点为安全,而叶子实际位于攻击者目录树内(isDirectory: true、mode 0700);拒绝符号链接的 audits 组件后翻转为干净的拒绝。
修复建议:mkdir 之后,将 realpathSync(dir) 与 realpathSync(getGlobalQwenDir()) + '/audits/' + hash 比较,不一致则抛错(仍容忍合法符号链接的 QWEN_HOME)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred to the next round per the per-round batch cap (Critical/Required first, ~8 findings implemented per round). This round's batch covered the command-execution (core.fsmonitor), artifact-write (hardlink overwrite), sidecar-containment, relocation-departure-proof, and anchor-certification clusters; this finding stays queued first for the next round. The thread remains open.
中文说明
由于每轮批次上限(优先处理 Critical/Required,每轮实现约 8 个发现),本发现推迟到下一轮。本轮批次覆盖了命令执行(core.fsmonitor)、产物写入(硬链接覆盖)、sidecar 收容、迁移离开证明与锚点认证这几个簇;该发现在下一轮优先处理。主题保持打开。
| if (token === '--effort') { | ||
| const value = tokens[++i]; | ||
| if (!value || !EFFORT_LEVELS.has(value.toLowerCase())) { |
There was a problem hiding this comment.
[Suggestion] The spaced-form --effort with a missing value (bare --effort as the last token) is the only rejection arm of parseAuditArgs with no fixture; the !value || guard it depends on can be deleted with the suite staying green. Probe: deleting !value || leaves the existing suite 12/12 green and the mutated code throws a raw TypeError on 'X' --effort — the exact harm the guard prevents (the skill and user see an uncaught stack where the actionable diagnostic belongs); deleting the equals-form validation likewise ships green and --effort=nope is silently accepted. Input path: a truncated heredoc write of the skill's stdin payload, or a user typing /audit dir --effort with the level dropped.
Suggested fix: add arms to the existing rejection test: expect(() => parseAuditArgs(`'${dir}' --effort`)).toThrow(/must be low, medium, or high/); and the same for --effort= (empty value).
中文说明
空格形式 --effort 缺值(裸 --effort 作为最后一个词元)是 parseAuditArgs 唯一没有夹具的拒绝臂;它所依赖的 !value || 守卫可以被删除而套件保持绿色。探针:删除 !value || 后既有套件 12/12 为绿,变异代码对 'X' --effort 抛出原始 TypeError——恰是守卫防止的危害(技能与用户在应有可操作诊断处看到未捕获堆栈);删除等号形式校验同样绿灯合入,--effort=nope 被静默接受。输入路径:技能 stdin 载荷的 heredoc 写入被截断,或用户输入 /audit dir --effort 时漏掉档位。
修复建议:在既有拒绝测试中补臂:expect(() => parseAuditArgs(`'${dir}' --effort`)).toThrow(/must be low, medium, or high/); 并对 --effort=(空值)补同样的断言。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| --out .qwen/audits/audit-<ts>.sidecar | ||
| ``` | ||
|
|
||
| (With a fallback landing, use `<fallbackRoot>/audit-<ts>.sidecar` here and for the report.) The capture is unconditional — never gated on a dirty/clean determination, because `git status` never shows the gitignored-untracked class. Record the returned SHA, subtree hash, or `noVcs` for the report header ("no VCS — anchors not alignable" outside a worktree). |
There was a problem hiding this comment.
[Suggestion] Step 3's header vocabulary is a three-way disjunction (SHA / subtree hash / noVcs), but audit snapshot publishes a fourth capture state — vcsProbeFailed: true with null SHA/subtree — that the skill never names (grep: only headUnknown/recaptured appear). The only fallback phrase Step 3 offers for a baseline-less capture is "no VCS — anchors not alignable" — the exact claim sidecar.ts's meta comment forbids for this state ("degrades like noVcs, but the header must not claim 'outside any git worktree'", because the drift arms re-probe at checkpoint time). An orchestrator following Step 3 literally after a transient rev-parse failure at run start records a falsehood about the run. The report template at the Step 7 tail repeats the same disjunction. Run safety is preserved by Step 6's headUnknown stop — this is report accuracy, not a stop-rule breach.
Suggested fix: add the state to Step 3: "with vcsProbeFailed (git present, probe failed), record 'VCS probe failed — baselines re-probed at checkpoints', never 'no VCS'".
中文说明
Step 3 的头部词汇是三分支析取(SHA / 子树哈希 / noVcs),但 audit snapshot 会发布第四种捕获状态——vcsProbeFailed: true 且 SHA/子树为 null——技能从未命名它(grep:只出现 headUnknown/recaptured)。Step 3 为无基线捕获提供的唯一兜底短语是 "no VCS — anchors not alignable"——恰是 sidecar.ts meta 注释禁止在此状态使用的说法("像 noVcs 一样降级,但头部不得声称'在任何 git 工作区之外'",因为漂移臂会在检查点时重新探测)。编排者在运行起始 rev-parse 瞬时失败后按 Step 3 字面执行,会记录一条关于本次运行的虚假陈述。Step 7 尾部的报告模板重复了同一析取。运行安全由 Step 6 的 headUnknown 停止保障——这是报告准确性问题,不是停止规则的破坏。
修复建议:在 Step 3 中加入该状态:"vcsProbeFailed 时(git 存在、探测失败),记录 'VCS probe failed — baselines re-probed at checkpoints',绝不写 'no VCS'"。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| existing.refusedCallers = mergeRefusals( | ||
| existing.refusedCallers, | ||
| refusedCallers, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] mergeRefusals never removes a stale refusal, so a caller refused at capture but admitted by an extend re-run ends up BOTH drift-watched and recorded as refused — violating the module's own documented invariant ("They stay OUT of callerNames — a refused caller is not watched"). Verified in both directions by probe: (1) a dangling-symlink caller refused at capture (out-of-repo) is admitted by the --callers extend re-run once materialized, while refusedCallers still records it; (2) with git down at capture, gitToplevelOf returns undefined so containment is silently skipped — an out-of-repo caller is admitted, hash-baselined and content-copied, then a healthy re-run adds the refusal while the caller stays in callerNames, its archived copy remains, and driftCheck actively watches it. SKILL.md Step 4 tells the orchestrator to disclose refused callers as "NOT watched for drift and their content is never read", so the final report actively misdescribes the archive.
Suggested fix: in the extend path, drop refusal records whose caller was admitted this run (filter the merged map against admittedCallers).
中文说明
mergeRefusals 从不移除过期的拒绝记录,因此在捕获时被拒、但在 extend 重跑时被接纳的 caller,会同时处于"被漂移监视"与"被记录为已拒绝"两种状态——违反模块自身文档化的不变量("它们保持在 callerNames 之外——被拒绝的 caller 不被监视")。探针双向验证:(1)捕获时因仓库外(悬空符号链接)被拒的 caller,在物化后经 --callers extend 重跑被接纳,而 refusedCallers 仍记录着它;(2)捕获时 git 不可用时,gitToplevelOf 返回 undefined,封闭域检查被静默跳过——一个仓库外 caller 被接纳、做了哈希基线并被内容拷贝,随后健康的重跑补记了拒绝,但该 caller 仍在 callerNames 中、归档拷贝仍在、driftCheck 仍在主动监视它。SKILL.md Step 4 要求编排者把被拒 caller 披露为"不做漂移监视、其内容永不被读取",因此最终报告会对归档做出主动的虚假描述。
修复建议:在 extend 路径中,丢弃本次运行已接纳 caller 的拒绝记录(用 admittedCallers 过滤合并后的映射)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const subtree = subtreeHashAt(rootAbs, top.trim()); | ||
| if (subtree) meta.subtreeHash = subtree; |
There was a problem hiding this comment.
[Suggestion] A transient subtreeHashAt failure at capture leaves subtreeHash undefined with no flag and no repair arm — unlike the identical headSha failure class, which gets a dedicated headMissing repair arm precisely because it would otherwise be unrecoverable. Reproduced with a PATH shim failing only rev-parse HEAD:: capture set headSha but left subtreeHash null with vcsProbeFailed false, captureDegraded empty, recaptured null — no marker of any kind; a healthy-git extend re-run left it null (headMissing keys on missing headSha, so it never fires); after a committed mode-only change (chmod +x, invisible to content hashes), drift reported subtreeMoved false. The snapshot's subtreeHash null is byte-identical to the legitimate vendored case, so the report header presents a run that lost an arm as vendored. Two corrections worth noting: subtreeMoved fires no stop (the git-state arms are disclosure/alignment, not stops), and the minimal fix as implied would also fire for legitimate vendored captures — a production fix needs a capture-time flag distinguishing a transient probe failure from "no HEAD entry".
Suggested fix: record a meta.subtreeProbeFailed marker when subtreeHashAt fails while headSha succeeds, and retry the subtree baseline in the extend repair alongside headMissing.
中文说明
捕获时 subtreeHashAt 的瞬时失败会让 subtreeHash 保持 undefined,既无标记也无修复臂——与之相同的 headSha 失败类别却有专门的 headMissing 修复臂,正因为否则将不可恢复。用只让 rev-parse HEAD: 失败的 PATH 垫片复现:捕获设置了 headSha,却把 subtreeHash 留为 null,且 vcsProbeFailed 为 false、captureDegraded 为空、recaptured 为 null——毫无任何标记;健康的 git 下 extend 重跑仍留为 null(headMissing 以 headSha 缺失为键,永不触发);在一次已提交的模式变更(chmod +x,对内容哈希不可见)后,漂移报告 subtreeMoved 为 false。快照中的 subtreeHash null 与合法的 vendored 情形逐字节相同,因此报告头会把一个丢失了该臂的运行呈现为 vendored。两点需要说明:subtreeMoved 不触发停止(git 状态臂是披露/对齐,不是停止),且按现意的最小修复对合法的 vendored 捕获也会误触发——生产修复需要一个捕获期标记来区分"瞬时探测失败"与"没有该 HEAD 条目"。
修复建议:当 subtreeHashAt 失败而 headSha 成功时记录 meta.subtreeProbeFailed 标记,并在 extend 修复中与 headMissing 一道重试子树基线。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
|
||
| An exit code of 4 means at least one of the two needs handling — never ship an anchor that binds arbitrarily. | ||
|
|
||
| **Run the final drift + guard checkpoint** (Step 6 commands), then write the report to `.qwen/audits/<YYYY-MM-DD>-<HHMMSS>-<reportSlug>.md` (or the fallback root when the guard refused the in-repo landing — the report lands beside the sidecar): |
There was a problem hiding this comment.
[Suggestion] The write gate certifies the DRAFT, but the durable report is written afterwards and never checked against the manifest. SKILL.md:195-197 gates audit-draft-.md against the findings manifest; SKILL.md:210 then writes the report to -.md — a separate, later write under a different name — and nowhere says the landed report must BE the gated draft moved/copied. check-anchors is the only command that reads a report file, and nothing ever reads -.md; SKILL.md:266 then deletes the intermediates (the gated draft and the manifest), so any divergence — a dropped marker block, an altered quoted anchor, a gate-refused finding kept — becomes undetectable at run end. This engages the diff's own rationale rather than contradicting it: anchors.ts's module comment exists precisely because unverified LLM-authored transcription was the fail-open class ("each new rendering was a new entrance") that the manifest/marker design eliminated — and this is the one remaining ungated transcription step.
Suggested fix: say explicitly that the landed report is the gated draft moved/copied to the durable path (never re-authored), or re-run check-anchors --report against the landed report before deleting the intermediates.
中文说明
写入门认证的是草稿(draft),但持久报告在其之后才写出,且从不与清单核对。SKILL.md:195-197 让 audit-draft-.md 与发现清单对门;SKILL.md:210 随后把报告写入 -.md——一次独立的、更晚的、名称不同的写入——并且通篇没有说落盘报告必须就是被门认证的草稿移动/拷贝而来。check-anchors 是唯一读取报告文件的命令,而没有任何命令读取 -.md;SKILL.md:266 随后删除中间产物(被门认证的草稿与清单),于是任何分歧——丢失的标记块、被改动的引用锚点、保留了被门拒绝的发现——在运行结束时都变得不可检测。这一点呼应而非违背本 diff 自身的理由:anchors.ts 的模块注释正是为了"未经验证的、由 LLM 撰写的转录是失败开放的类别(每一种新渲染都是一个新入口)"而存在,manifest/marker 设计正是为消除它——而这里是仅剩的一步未设门转录。
修复建议:明确说明落盘报告即被门认证的草稿移动/拷贝到持久路径(绝不重新撰写),或在删除中间产物前对落盘报告重跑 check-anchors --report。
— qwen3.8-max via Qwen Code /review (v0.22.0)
The control-character strip in isSecretName was written with LITERAL control bytes inside its character class — a real NUL, 0x1f, and 0x7f — so git classified the whole ~1,700-line module as binary. The PR diff showed only "Binary files ... differ" for it: the size gates, the local-only guard, the secret-name detection, and the walk exclusions all shipped without ever appearing in a diff, a blame, or any patch tooling, and every future change to those security-relevant guards would have stayed undiffable. tsc and prettier accept the bytes, so nothing in CI caught it. Spelling the same class with escapes restores a text diff. The replacement is runtime-identical: both spellings strip exactly C0 plus DEL, verified against a rebuilt-from-char-codes copy of the old regex over inputs carrying each boundary byte.
The name pre-filter strips all of C0 plus DEL before re-testing, but only the trailing-LF spelling was covered. Reading the class was no help while it was written with literal bytes, so the behaviour was effectively unpinned in both directions. Each boundary spelling now has a case — LF, CR, DEL, NUL — and narrowing the strip to LF alone turns the CR and DEL cases red, so the class cannot be quietly narrowed again.
|
Both round-1 points addressed at The binary-file blocker is real and is fixed (commit The second half of that finding does not reproduce. The claim was that control-suffixed lowercase names ( The suggested test hardening was worth taking regardless, and is in ( On CI: the current head is green on the unit suite ( |
|
🔍 Two-phase code review for #9749 (head:
Final verdict: ERROR — automated review could not be completed. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI; this PR's platform-specific surface (O_NOFOLLOW/O_NONBLOCK degradation, drive-letter archive paths, Windows pathspec normalization) did not run locally.
Not explored to full depth (tool budget reached): chunk 15: could not execute the vitest file itself — the review worktree has no node_modules and no built dist/ prerequisites, so verification was done by tracing the i…; chunk 24: running packages/cli vitest for sidecar.test.ts to confirm green at HEAD — blocked by absent node_modules in the review worktree; assessment rests on tracin…; chunk 13: running files-plan.test.ts under vitest — the review worktree has no node_modules and a full monorepo install + dist build was not viable here; substituted with…; chunk 6: runtime execution of anchors.test.ts (fresh worktree has no node_modules; the vitest globalSetup guard requires multiple workspace packages' dist/ built, and ….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/audit/snapshot.ts:65 — [review] refusedCallers stdout publication untested (deleting the field ships green; SKILL.md Step 4 reads it)packages/cli/src/commands/audit/lib/sidecar.ts:272 — [review] --no-ext-diff/--no-textconv hardening has no test (a refactor dropping the flags ships green and hostile diff drivers execute at capture)packages/cli/src/commands/audit/agent-prompt.test.ts:50 — [review] neither yargs .check() refusal branch tested (dropping .check() ships green; --probes omitted silently prints the declined brief)packages/cli/src/commands/audit/agent-prompt.ts:41 — [review] low-reader success arm untested (mutant printing an empty prompt ships green across all three suites)packages/cli/src/commands/audit/guard-check.test.ts:435 — [review] fallbackLandingSafe's plan-ts arm has no discriminating test (deleting the arm ships green while its primary-directory twin IS pinned)packages/cli/src/commands/audit/lib/anchors.test.ts:331 — [review] no multi-line anchor test against a tab-indented file (space-only dedent mutant ships green; R1-13's fix lands with no regressing fixture)packages/cli/src/commands/audit/lib/anchors.test.ts:268 — [review] Windows backslash-caller normalization contract unpinned (normalize=identity mutant ships green; discriminating probe flips)packages/cli/src/commands/audit/lib/anchors.test.ts:356 — [review] over-long-anchor fixture cannot discriminate the cap (guard removed still unresolved; a matching fixture flips)packages/cli/src/commands/audit/lib/files-plan.test.ts:522 — [review] EVENT_SCAN_MAX_CHARS gate untested (removing it ships green; large subjects pay a 10MB read + strip + regex)packages/cli/src/commands/audit/lib/files-plan.test.ts:455 — [review] late-NUL fixture only disproves an 8KiB window (64KiB-window mutant ships green; re-windowed binaries steer the gate)packages/cli/src/commands/audit/lib/files-plan.test.ts:1582 — [review] artifact-name oracle regex cannot match <role> shapes (roster-expansion mutant missing one role ships green)packages/cli/src/commands/audit/lib/read-json.ts:96 — [review] readCallersFile bounds no count and dedups only at storage (~10^5 agent-authored entries → hours of synchronous hash+copy; extend includes() quadratic)packages/cli/src/commands/audit/lib/sidecar.test.ts:807 — [review] probe-failed repair's recaptured marker unpinned (dropping the ??= assignment ships green; no test references 're-established')packages/cli/src/commands/review/lib/manifest-repository-context.test.ts:576 — [review] rewritten motivating comment no longer discriminates (2×100=200 < 256 cap; the replaced 2×200=400 was correct)packages/core/src/config/storage.ts:351 — [review] mock-driven dead guard: realpathSync non-string branch exists only for the file's own vi.mock (removing it crashes 3 tests)packages/core/src/skills/bundled/audit/SKILL.md:83 — [review] relocated command blocks interpolate <artifacts-dir> unquoted (a home/QWEN_HOME with a space word-splits every relocated command)packages/core/src/skills/bundled/audit/SKILL.md:206 — [review] exit-4 'downgrade the finding' handling can never converge (the gate is severity-blind; only refusal or a fixed quote satisfies it)packages/cli/src/commands/audit/lib/anchors.test.ts:341 — [review] token-fusion fixture exercises only the follow edge (leading-edge mutants ship green; probes flip)packages/cli/src/commands/audit/lib/anchors.test.ts:184 — [review] no fixture exercises any comment-introducer arm of followRuleOk (removing the arms ships green; probes flip)packages/cli/src/commands/audit/lib/anchors.ts:172 — [review] whitespace-only anchors pass the schema and grade resolved (a single-space anchor ships; add a trim() refusal)- …and 8 more (see the run report)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had none either, so the next review re-reads the whole diff and will keep doing so until a round's marker carries an anchor again. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查:build-and-test — Test (windows-latest, Node 22.x) was skipped in CI; this PR's platform-specific surface (O_NOFOLLOW/O_NONBLOCK degradation, drive-letter archive paths, Windows pathspec normalization) did not run locally。
未探索到全部深度(达到工具调用预算):chunk 15:could not execute the vitest file itself — the review worktree has no node_modules and no built dist/ prerequisites, so verification was done by tracing the i…;chunk 24:running packages/cli vitest for sidecar.test.ts to confirm green at HEAD — blocked by absent node_modules in the review worktree; assessment rests on tracin…;chunk 13:running files-plan.test.ts under vitest — the review worktree has no node_modules and a full monorepo install + dist build was not viable here; substituted with…;chunk 6:runtime execution of anchors.test.ts (fresh worktree has no node_modules; the vitest globalSetup guard requires multiple workspace packages' dist/ built, and …。
未审查:反向审计——评审时间预算不足,未能开始第 3 轮。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 28 条(原文未翻译,列表见上方英文部分)。
机制健康:本轮未能干净收尾,因而扣留了增量锚点,而它恢复到的那一轮也没有锚点,因此下一次评审将重读整个 diff——并会一直如此,直到某一轮的标记重新带上锚点。(仅陈述,不据此行动——这不改变本轮发布的任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
| report: string; | ||
| callers?: string; | ||
| }; | ||
| const planJson = readPlanFile(plan, 'check-anchors'); |
There was a problem hiding this comment.
[Critical] R2-4: check-anchors binds all anchor resolution to planJson.targetPathAbsolute without ever validating that the root is the audited tree — readPlanFile checks only absoluteness and lexical normalization, so a stale plan (yesterday's audit of directory A) or a forged plan with a foreign normalized root passes and silently re-binds every verdict: snippets present in the foreign tree grade resolved/exit 0 and ship findings anchored in files the audited tree does not contain; stdout verdict/matchCount becomes a content-match oracle on arbitrary files. plan-files' extend path re-validates its recorded root via resolveAuditRoot; check-anchors has no equivalent. Verified by probe at HEAD.
Witness:
probe: handler run with a well-shaped foreign-root plan → exit 0, verdict resolved against the foreign tree (`RA1-EXIT-CODE undefined`); read-json's root check is only absoluteness + lexical resolve().
Suggested fix: Re-validate before any resolution: call resolveAuditRoot(planJson.targetPathAbsolute) and bind the root to the invocation (refuse a plan whose root is not contained in / equal to the resolved audited tree), mirroring guard-check's distrust of agent-transcribed values.
中文说明
check-anchors 把所有锚点解析绑定到 planJson.targetPathAbsolute,却从不校验该根是否为被审计树——readPlanFile 只检查绝对性与词法归一化,因此陈旧 plan(昨天对目录 A 的审计)或带外部归一化根的伪造 plan 会通过并悄悄重绑定所有裁决:外部树中存在的片段判 resolved/退出码 0,放行锚定在被审计树不存在的文件中的发现;stdout 成为任意文件的内容匹配预言机。plan-files 的 extend 路径会经 resolveAuditRoot 重新校验,check-anchors 没有。探针验证:外部根 plan → 退出码 0、resolved。修复:解析前重新校验并绑定根到本次调用。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const planJson = readPlanFile(plan, 'check-anchors'); | ||
| const registeredCallers = callers | ||
| ? readCallersFile(callers, 'check-anchors') | ||
| : []; |
There was a problem hiding this comment.
[Critical] R1-2: readCallersFile validates only absoluteness; callerRefusal (secret-shaped/out-of-repo) is applied only in captureSidecar. A manifest citing a refused path makes readGuarded open it — stdout verdict/matchCount is a content-match oracle on policy-protected files, and a resolved verdict ships a finding anchored in a refused file, contradicting SKILL.md:123. Round-2 probe re-confirmed at HEAD: snapshot refused a planted prod.env caller; check-anchors on the same file returned resolved/matchCount 1/exit 0; applying callerRefusal flips it to out-of-scope/exit 4.
Witness:
probe: snapshot refusals [{"caller":"…/prod.env","reason":"secret-shaped"}]; check-anchors with same callers file: {"verdict":"resolved","matchCount":1,"exitCode":0}; with callerRefusal applied in handler: {"verdict":"out-of-scope","matchCount":0,"exitCode":4}.
Suggested fix: Re-apply callerRefusal at this read site (derive the repo root from the plan) or pass the sidecar's admitted callerNames; grade anchors at refused callers out-of-scope.
中文说明
check-anchors 会读取内容被 sidecar 拒绝的 caller:readCallersFile 只校验绝对路径,callerRefusal(secret-shaped/仓库外)只在 captureSidecar 内执行。清单若引用被拒路径,readGuarded 会打开它——stdout 的 verdict/matchCount 成为针对策略保护文件的内容匹配预言机,resolved 裁决还会放行锚定在被拒文件中的发现,与 SKILL.md:123 矛盾。第二轮探针复测:snapshot 拒绝植入的 prod.env,check-anchors 对同一文件返回 resolved/1/退出码 0;应用 callerRefusal 后翻转为 out-of-scope/退出码 4。修复:在此读取点重新应用 callerRefusal。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| atPlan.status !== 'ok' && | ||
| atPlan.status !== 'no-worktree'; | ||
| if (!exposedAtPlan) return true; | ||
| return !relocationVerified; |
There was a problem hiding this comment.
[Critical] R1-10: A directory that turns exposed MID-RUN (the .gitignore-entry-drops scenario the file header names) keeps firing exit 5 at every remaining checkpoint even after its relocation lands and is verified by this same command: exposedAtPlan is false, so guardTripped returns true without consulting relocationVerified. SKILL.md's only prescribed exit-5 response is relocate-and-continue → a redo loop over a relocation that landed; the write-time checkpoint fires too, so the run cannot reach the report write. Pinned by guard-check.test.ts:93. Round-2 probe re-confirmed at HEAD.
Witness:
probe: repo ignored at plan time → exit 0; .gitignore deleted mid-run, plan relocated and verified → `R1-10 exit: before-flip=undefined after-relocation=5 redo=5`; control arm (plan-time exposure, same verified relocation) → exit 0.
Suggested fix: Credit a verified relocation for every exposed dir — return !relocationVerified; after the status filter stays fail-closed when there is no plan (relocated is false then); update the pinning test. If mid-run exposure is meant as a hard stop, SKILL.md must say so and name a clearing path.
中文说明
已验证的重定位只为规划期已暴露的目录清除退出码 5:运行中途转为暴露的目录(文件头注释点名的 .gitignore 条目被移除场景),即使重定位已落地并被同一命令验证,其后每个检查点仍反复触发退出码 5——exposedAtPlan 为 false 时 guardTripped 不查询 relocationVerified 就返回 true。SKILL.md 对退出码 5 的唯一处置是重做,于是对已落地的重定位反复重做,写入时检查点同样触发,运行无法到达报告写入。第二轮探针复测。修复:为每个暴露目录信任已验证的重定位(return !relocationVerified;),并更新钉住该行为的测试。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // committable while the guard reports the relocation complete. | ||
| return !existsSync( | ||
| join(process.cwd(), AUDIT_TMP_DIR, `audit-plan-${planTs}.json`), | ||
| ); |
There was a problem hiding this comment.
[Critical] R1-8: The departure proof verifies only that the PLAN file left .qwen/tmp, but the relocated credit suppresses exit 5 globally. audit-args-.json / audit-raw-args-.txt are written BEFORE the guard verdict; if the multi-file move is killed after the plan lands — the exact state planRelocated's own comment anticipates — those artifacts stay committable while every later checkpoint exits 0. Step-8 cleanup deletes only under the post-relocation dir. Round-2 probe re-confirmed at HEAD.
Witness:
probe (real guardCheckCommand handler): {"exitWithPartialRelocationCredit":0,"exitWithoutPlan":5,"argsLeftBehind":true,"rawArgsLeftBehind":true}; extending the departure proof to all guardProbeShapes(...).tmp shapes → {"exitWithPartialRelocationCredit":5,"exitWithCompleteRelocation":0}.
Suggested fix: Void the credit when any pre-relocation tmp shape remains: require none of guardProbeShapes(reportFileName, planTs).tmp exists under .qwen/tmp.
中文说明
部分重定位会让守护失声:离开侧证明只校验 PLAN 文件已离开 .qwen/tmp,但 relocated 信任全局压制退出码 5。audit-args/audit-raw-args 在守卫裁决之前写出;若多文件搬迁在 plan 落地后被中断(正是代码注释预料的状态),这些产物仍可提交而其后每个检查点退出码 0。第二轮探针复测:部分搬迁 → 退出码 0、args 产物可暂存;把离开侧证明扩展到全部 tmp 形态后 → 退出码 5,完整搬迁仍通过。修复:任一重定位前的 tmp 形态仍存在时即撤销信任。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // Mirror checkLocalOnlyGuard's next-date probe: the relocated report is | ||
| // written at write time, so its date can roll past the checkpoint | ||
| // instant — a post-midnight landing must be asked about too. | ||
| const nextDate = nextCalendarDate(new Date()); |
There was a problem hiding this comment.
[Critical] R2-6: The next-calendar-date report probe pins the time component to 000000 — one fixed name of 86,400 — so a name-selective re-include whose time class excludes 000000 evades the probe while re-including whatever name the post-midnight write produces; the same pinned shape exists in checkLocalOnlyGuard (files-plan.ts), so primary dirs and the fallback landing share the hole. Violates the file's own "every shape written is a shape probed" doctrine. Verified with real git at HEAD.
Witness:
probe (real git, fake clock): re-include excluding 000000 → `RA3-AUDITS-STATUS ok`; pinned name ignored true, post-midnight write name ignored false; control (full-date wildcard) → unprotected; mutant probing hourly classes → unprotected.
Suggested fix: Probe the name that will actually be written — the write-time invocation knows it (pass the exact report filename), or probe time representatives spanning every first-digit class; apply the same change to checkLocalOnlyGuard's next-date push.
中文说明
下一日历日报告探针把时间分量钉死为 000000——86,400 个名字中的一个——时间类排除 000000 的按名选择性 re-include 即可逃过探针、同时重新包含午夜后实际写出的名字;checkLocalOnlyGuard 存在同样的钉死形态,主目录与回退落点共享此漏洞。违反本文件"每一种被写出的形态都是被探测的形态"的准则。真实 git 验证:排除 000000 的 re-include → 状态 ok;钉死名被忽略、午夜后写出的名字未被忽略。修复:探测实际将被写出的名字(写入时调用方知道它),或探测覆盖首位数字类的时间代表。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| writeFileSync(b, 'const b = 2;\n'); | ||
| const sidecar = captureSidecar(plan(), sidecarDir, [a, b]); | ||
| expect(sidecar.callerHashes[a]).not.toBe(sidecar.callerHashes[b]); | ||
| expect(driftCheck(plan(), sidecarDir).driftedCallers).toEqual([]); |
There was a problem hiding this comment.
[Suggestion] R1-30: "keeps drive-letter-distinct callers in separate archive paths" pins nothing about archive paths: it never constructs distinct drive letters on any platform, and its assertions are computed from source-content hashes (callerHashes digests the SOURCE files; driftCheck re-hashes the originals), which a copy collision cannot affect. Round-2 mutant: dest = basename(caller) makes the test's own x/same.ts and y/same.ts collide on one archive path with the later copy overwriting — the shipped test still passed (36/38; only two unrelated full-path tests failed). A keying regression ships green on every platform.
Witness:
mutant (collision on basename): shipped test still passed; keying-agnostic archive-preservation probe (two same-basename callers ⇒ two content-preserved copies) fails under the mutant, passes on unmutated code.
Suggested fix: Also assert both archived copies exist at distinct destinations, computing the dests with the implementation's keying helper (existsSync(join(sidecarDir, 'callers', key(a))) and same for b).
中文说明
"让不同盘符的 caller 存于不同归档路径"对归档路径什么都没钉:任何平台都未构造不同盘符,断言由源内容哈希计算(callerHashes 摘要源文件,driftCheck 重新哈希原件),拷贝冲突影响不到。第二轮突变体:dest = basename(caller) 使测试自己的 x/same.ts 与 y/same.ts 冲突于同一路径、后拷贝覆盖——已发布测试仍通过。键控回归在所有平台绿色放行。修复:用实现的键控助手断言两份拷贝存在于不同目标。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const headMissing = | ||
| !existing.meta.noVcs && | ||
| existing.meta.headSha === undefined && | ||
| existing.meta.headUnborn !== true; |
There was a problem hiding this comment.
[Suggestion] R1-31: The extend re-run repair block has three triggers — probeFailed, degraded, headMissing — and sidecar.test.ts exercises only probeFailed through an actual re-run (withBrokenGit → noVcs=true makes headMissing false and degraded false). The headMissing precondition (born HEAD, transient rev-parse failure) is produced by a test that ends at driftCheck — never performing the extend re-run; the degraded-only precondition captures once and never re-runs, so the distinct arms = [...captureDegraded] path is never executed. Re-confirmed by round-2 trace at HEAD.
Witness:
trace: 'repairs a probe-failed capture...' drives only the probeFailed arm; 'does not record headUnborn from a transient rev-parse failure...' stops at driftCheck; 'degrades, not aborts...' never re-runs captureSidecar.
Suggested fix: Add extend re-run tests for the headMissing arm (born HEAD + transient rev-parse failure, then re-run) and the degraded-only arm (one copy failing at capture, then re-run), asserting the arm selection and the recaptured flag.
中文说明
extend 重跑修复块有三个触发器——probeFailed、degraded、headMissing——而 sidecar.test.ts 只通过真实重跑演练 probeFailed(withBrokenGit → noVcs=true 使 headMissing 为 false、degraded 为 false)。headMissing 前置条件的测试止于 driftCheck,从不执行重跑;降级前置条件只捕获一次,独立的 arms = [...captureDegraded] 路径从不执行。第二轮复核。修复:为 headMissing 臂与仅降级臂补 extend 重跑测试。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| process.env['QWEN_HOME'] = join(repo, 'qwen-home'); | ||
| // Process-level git-config hermeticity: the guard's in-process | ||
| // check-ignore probes spawn git with the ambient process.env, so | ||
| // pinning only the `git init` subprocess leaks a host global exclude |
There was a problem hiding this comment.
[Suggestion] R1-26: Third location of the GIT_CONFIG_GLOBAL dead-pin pattern. Round-2 behavioral A/B at HEAD: arm A (HOME pointed at a gitconfig whose core.excludesFile ignores .qwen/, pin set to an empty gitconfig) → exit 0, dirs ok; arm B (identical, HOME carries no gitconfig) → exit 5, dirs unprotected. The only differing variable is the HOME-carried global config: the pin is dead and a host-global exclude participates in probe verdicts despite the hermeticity comment.
Witness:
probe: `R1-26 ARM-A {"exitCode":0,"dirs":[".qwen/audits:ok",".qwen/tmp:ok"]}` vs `ARM-B {"exitCode":5,"dirs":[".qwen/audits:unprotected",".qwen/tmp:unprotected"]}` — only HOME-carried config differs.
Suggested fix: Same as R1-24: correct the comment or pin HOME/XDG_CONFIG_HOME.
中文说明
GIT_CONFIG_GLOBAL 死钉模式的第三处。第二轮行为 A/B:臂 A(HOME 指向 core.excludesFile 忽略 .qwen/ 的 gitconfig,钉指向空 gitconfig)→ 退出码 0;臂 B(HOME 无 gitconfig)→ 退出码 5。唯一差异变量是 HOME 携带的全局配置:钉钉是死的,宿主全局 exclude 参与探针裁决。修复同 R1-24。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| swallowEpipe(process.stderr); | ||
| await Promise.race([ | ||
| new Promise<void>((resolve) => { | ||
| process.stdout.write('', () => { |
There was a problem hiding this comment.
[Suggestion] R1-33: The new pre-exit flush (empty-write-callback drain, EPIPE swallow, 2s SUBCOMMAND_FLUSH_TIMEOUT_MS deadline) sits on the exit path of every registered subcommand (mcp, extensions, auth, hooks, channel, review, audit, sessions, update) and is referenced nowhere else; config.test.ts's 342 tests traverse the exit path but assert nothing about flushing. Round-2 mutant: deleting the entire flush block — 342/342 passed. Both named regressions ship green: dropping the drain (>64KiB plan-JSON truncation at the pipe buffer — empirically demonstrated: 1MiB payload truncated at ~80KiB under immediate exit, delivered whole with the drain) and dropping swallowEpipe (| head EPIPE crash once the event loop is yielded to).
Witness:
mutant (flush block deleted): config.test.ts 342/342 passed; live probe: 1MiB queued write, reader delayed 2s — immediate exit truncated at ~80KiB, the drain delivered the full payload.
Suggested fix: Add a test running a subcommand whose handler writes >64KiB to stdout through a pipe and asserts the consumer receives the complete payload, plus a closed-downstream case asserting no uncaught EPIPE and the designed exit code.
中文说明
新的退出前刷新(空写回调排空、EPIPE 吞掉、2 秒超时)位于每个注册子命令的退出路径上,别处无引用;config.test.ts 的 342 个测试经过退出路径但不断言刷新。第二轮突变体:整体删除刷新块后 342/342 通过。两种回归均绿色放行:丢弃排空(>64KiB plan JSON 在管道缓冲处截断——实测 1MiB 载荷立即退出时截断于 ~80KiB,有排空则完整送达);丢弃 swallowEPIPE(| head 的 EPIPE 崩溃)。修复:补 >64KiB 管道测试与关闭下游用例。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // landing is where the relocation puts artifacts precisely BECAUSE they | ||
| // must stay private. Validate what is actually there, and tighten a | ||
| // permissive one. | ||
| const stat = fs.lstatSync(dir); |
There was a problem hiding this comment.
[Suggestion] R1-32: The planted-leaf defenses (refusing a non-directory/symlink leaf; chmod-tightening a permissive existing leaf) have zero test coverage: the four shipped tests all start from a fresh QWEN_HOME and cover layout, 0700-on-create, separation/idempotency, symlink-spelling stability. Round-2 mutant: disabling both defenses (if (false) on the throw and the chmod) — shipped suite 4/4 passed. Flip probe: loosen an existing leaf to 0755 → expect re-tightening to 0700; plant a symlink leaf → expect /not a directory/ — both fail under the mutant, pass on unmutated code.
Witness:
mutant: shipped suite 4/4 passed; flip probes: 2/2 fail under the mutant, 2/2 pass on unmutated code.
Suggested fix: Add two tests: pre-create the <QWEN_HOME>/audits/ path as a symlink to a directory and assert the "not a directory" throw; pre-create it as a 0o755 directory and assert the returned dir's mode is tightened to 0o700 (skip the mode assertion on win32).
中文说明
植入叶子防御(拒绝非目录/符号链接叶子;收紧过宽的既有叶子权限)零测试覆盖:四个已发布测试都从全新 QWEN_HOME 开始。第二轮突变体:禁用两项防御后 4/4 仍通过;翻转探针(放宽叶子到 0755 期望收紧到 0700;植入符号链接叶子期望抛错)在突变体下失败、在正确代码下通过。修复:补两个测试。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
Closing this in favour of a six-part sequence — the diff is too large to review, and the review runs themselves say so. Every round on this PR (and on #8403 before it) disclosed the same gap: The seams are already there:
Ordering note: the guard (C) landing after the plan writer (B) opens no window, because nothing writes into Round 3's findings carry forward. They are archived and assigned to the PR that owns each file — 9 on
CI on the final head |
|
👋 Takeover released: the autofix loop will no longer engage this PR (an in-flight round, if any, completes its bounded work). Re-apply 中文说明👋 已释放:autofix 循环不再介入此 PR(在飞的一轮如有,将完成其有界工作)。重新打上 |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Autofix review round summary — PR #9749This round triaged the 120 inline findings carried by the two automated review runs (round-1 and round-2 ledgers). CI reported no failed or still-red checks, so the round was feedback-driven. Per the per-round batch cap (~8 findings, Critical/Required first), one coherent batch of 8 fix units covering 10 unique findings (19 inline threads, since each finding was raised in both review runs) is implemented this round; the remaining 101 findings are deferred to the next round with a reply on every thread. Implemented this round (commit 11b2290)
Incidental: one test git shim in Deferred to the next round (101 findings, replied on every thread)All remaining findings — including 39 Criticals (check-anchors caller-refusal oracle R1-2, fallback-root symlink arms R1-5/R1-6/R2-19, relocation-ts binding R1-7, mid-run exposure redo loop R1-10, dedent/whitespace-kind matcher holes R1-12/R1-13, secret-detector coverage R1-17/R2-15, Windows gates R1-18, drive-letter test transforms R1-19/R1-43/R1-30, recaptured-marker arm R1-20, and the R2 cluster R2-1…R2-6, R2-9…R2-13, R2-16…R2-18, R2-20) and 62 Suggestions — are deferred under the batch cap via Review-level items: rv:5000868706's blocking defect (binary-diffed No conflicts ( Verification
中文说明Autofix 评审轮次总结 — PR #9749本轮对两次自动评审运行(第 1 轮与第 2 轮台账)所携带的 120 条行内发现进行了分类处理。CI 没有失败或仍然发红的检查,因此本轮完全由评审反馈驱动。按照每轮批次上限(约 8 个发现、优先 Critical/Required),本轮实现了一个连贯批次:8 个修复单元,覆盖 10 个唯一发现(对应 19 条行内主题,因为每个发现在两次评审中各被提出一次);其余 101 条发现推迟到下一轮,并在每条主题上逐一回复。 本轮已实现(提交 11b2290)
附带改动: 推迟到下一轮(101 条发现,每条主题均已回复)其余全部发现——包括 39 条 Critical(check-anchors 被拒调用者读取预言 R1-2、回退根符号链接各臂 R1-5/R1-6/R2-19、迁移时间戳绑定 R1-7、运行中暴露的重做循环 R1-10、去缩进/空白类型匹配器漏洞 R1-12/R1-13、机密检测器覆盖 R1-17/R2-15、Windows 门控 R1-18、盘符测试转换 R1-19/R1-43/R1-30、recaptured 标记臂 R1-20,以及 R2 簇 R2-1…R2-6、R2-9…R2-13、R2-16…R2-18、R2-20)和 62 条 Suggestion——按批次上限通过 评审级条目:rv:5000868706 的阻塞缺陷(以二进制形式提交的文件 + 无测试的秘密检测器)已在更早一轮由提交 2683a39 与 a659fe7 处理;两次台账运行的发现都映射到上面已处置的行内主题。Issue 级评论 ic:5383303978(第 1 轮各点已处理)与 ic:5383479220(针对上一 head 的两阶段评审以 ERROR 结束、未产生发现)不需要代码改动。 无冲突( 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
What this PR does
This PR adds
/audit <directory> [--effort low|medium|high]— a workflow for auditing existing, merged code where there is no diff, no PR, and no baseline — together with the deterministic CLI helpers the skill drives: argument parsing, filesystem planning, agent-brief printing, run-state captures, drift checkpoints, the local-only guard, and write-time anchor resolution. The orchestration lives in a bundled skill; the roster, gates, budget estimate, and captures are computed in code so the orchestrator cannot shrink or skip them.The default medium tier runs nine dimension agents plus verification shards; high adds the 6b/6c personas and iterative reverse-audit rounds over file-group partitions; low is a single reader sub-agent producing capped, explicitly unverified triage. Every consumer of module content — agents, verifiers, the dedup clusterer, the low reader, the orchestrator itself — opens with an untrusted-data preamble. The walks are read-only; the two execution classes (a baseline run of the module's own test suite, and agent-authored verification probes against scratch copies) are separate opt-ins at an interactive pre-launch confirmation, and non-interactive starts are refused.
This replaces #8403, which accumulated 24 rounds of review history that had become impossible to follow. The code here is that work plus the fixes for every Critical finding that review left open, and it is roughly 2,000 lines smaller than #8403's head — because the two largest fixes are subtractive.
The write-time gate no longer parses the report's markdown. Findings arrive as a machine-readable manifest (JSON, schema-checked), and the human report is held to it by one machine marker per finding. The previous shape hand-parsed unbounded LLM-authored report text, which is fail-open by construction: a rendering the parser's nets did not anticipate either produced zero findings — certifying a report whose snippets were never resolved — or silently rebound a finding to a different file. Every round found new entrances (bold headers, fence lengths, a second Location field, a localized heading, the rejected-findings appendix re-entering as a findings section), because a net per entrance never closes the set. The manifest arrives typed and verbatim, so there is nothing to peel, split, or infer; the marker check is a set comparison that no rendering, section layout, or output language can defeat.
lib/anchors.tsshrank from 688 to 435 lines and its test file from 1,441 to 383.Artifact reads and writes are now hardened against the tree the module already treats as hostile. Content opens refuse symlinks and writer-less FIFOs in the kernel (
O_NOFOLLOW/O_NONBLOCK), so a mid-window swap can no longer put an out-of-tree file's hash into the drift baseline. Artifact writes gate on the fd rather than on anlstata swap can outrun — the sidecar's extend write runs mid-fan-out, inside the audited tree, which is exactly where that race pays. The capture'sgit diffpasses--no-ext-diff --no-textconv, so the audited repository's own config can no longer name a program for the auditor to execute.gitEnv()drops the wholeGIT_-prefixed environment the way core's shared ignore probe already does, instead of enumerating channels —GIT_LITERAL_PATHSPECSalone silently emptied the:(literal)pathspecs the tracked-file arm and the capture both depend on. The registered-caller channel gets the scope and secret policy the walk already had, the plan's root is required to be normalized (not merely absolute), the exclude remedy verifies the common dir belongs to this repository before writing it, the report slug is capped at the writer so the guard's validator and the writer are one rule, and the next-calendar-date probe survives a DST fall-back day.Two limitations are accepted rather than closed, and every report header says so: the estimate's per-line rates are author-reported (their raw records are not in this repository), and the sidecar is not tamper-evident — it detects accidental drift, not a module actively hiding it. A run auditing genuinely untrusted code lands its artifacts outside the repository, which the local-only guard already offers.
Why it's needed
The existing
/reviewworkflow is optimized for incremental diffs and PR decisions. Maintainers also need a high-recall, low-noise way to audit merged legacy modules, where there is no baseline and no author intent to rely on. Two controlled experiments found substantially more confirmed defects with dimension fan-out than with a naive single-agent review, at zero adjudicated false positives.Per the design doc's reuse boundary,
/auditimports nothing across command groups fromcommands/review/: classification, the roster, briefs, budget, coverage, and anchor validation are re-expressed in audit-owned code. Three pieces lift to shared homes instead of being copied: the traversal-safesafeTargetslug (topackages/cli/src/utils/), the two privategit check-ignorecopies consolidated into one fresh-by-default helper inpackages/core/src/utils/, and a per-user outside-repo artifact fallback on the Storage hub.Reviewer Test Plan
How to verify
qwen audit plan-files packages/core/src/permissions --out <tmp>and... packages/core/src/hooks .... Confirm both calibration modules are admitted with their measured line counts (7,638 subject / 8,640 test and 8,516 / 16,335), the estimate brackets the measured costs, and hooks — but not permissions — is flagged as an event/lifecycle module.packages/core/distrefuses with "only excluded directories"; a gitlink under the audited path refuses naming the submodule rule; low refuses above 2,000 subject lines and points at medium.qwen audit check-anchors --plan … --findings … --report …exits 0 for a manifest whose anchors resolve and whose markers match, and exits 4 withmarkerProblemswhen the report ships a block the manifest never listed, when a manifest finding has no block, or when the manifest itself is malformed. Confirm exit 0 is reachable for an empty manifest — a clean audit clears the gate..gitpointer refuses the exclude remedy while a real linked worktree still lands it; a credential-shaped caller and one outside the repository are refused and recorded; a plan root carrying..is refused as stale; a bare*.gitdirectory and an in-treeQWEN_HOMEare never walked; credential material under an unguessed filename records secret-shaped./audit <small-module>at medium effort. Confirm the pre-launch confirmation names the estimate and both execution opt-ins, every agent brief opens with the untrusted-data preamble, whiffed agent returns are relaunched once then recorded not-audited, dedup keeps the max member severity with a completeness receipt, every cluster passes a verification shard, and the report lands at.qwen/audits/<date>-<time>-<slug>.mdwith the run-metadata header, walks record, and unexercised-machinery flags — and no verdict.Evidence (Before & After)
Before: the released CLI has no
auditcommand; an attemptedqwen audit …invocation falls through to interactive prompt handling.After: the built CLI registers the audit command family and the bundled skill. The full audit suite is green (294 tests across the command group), as are the core config and skills suites. Three CLI test files fail on this branch —
AuthDialog,systemController, andrun-qwen-serve-live— and each was confirmed to fail identically on an unmodifiedorigin/maincheckout in the same worktree, so they are pre-existing and unrelated to this change.Tested on
Environment (optional)
Node.js 22 development checkout on macOS. Verified with the targeted CLI and core test suites (including git-backed fixture tests for the guard, the submodule refusal, the exclude-remedy attribution, and the sidecar/drift arms), TypeScript type checking, ESLint, and Prettier.
Risk & Scope
Linked Issues
Implements #8397. Replaces #8403.
中文说明
本 PR 做什么
新增
/audit <directory> [--effort low|medium|high]——在无 diff、无 PR、无基线的情况下审计已合入的存量代码——并配套 skill 驱动的确定性 CLI helper:参数解析、文件系统规划、agent brief 打印、运行态捕获、drift 检查点、local-only 守护和写入时 anchor 解析。编排在 bundled skill 中;roster、门限、预算估算和捕获全部由代码计算,orchestrator 无法缩减或跳过。默认 medium 档运行九个维度 agent 加验证分片;high 增加 6b/6c persona 和按文件组分片的迭代反向审计;low 是单个 reader 子代理,产出限量且明确标注未验证的分诊结果。每个模块内容消费者——各 agent、验证者、去重聚类器、low reader、orchestrator 本身——都以 untrusted-data 前置声明开头。walk 全部只读;两类执行(模块自身测试套件的基线运行、针对 scratch 副本的 agent 编写验证探针)在交互式启动前确认中分别选择加入,非交互启动一律拒绝。
本 PR 取代 #8403——那个 PR 累积了 24 轮评审历史,已无法跟踪。这里的代码是那份工作,加上评审留下的每一条未决 Critical 的修复,且比 #8403 的 head 少约 2000 行——因为两处最大的修复是减法。
写入时的门不再解析报告的 markdown。 发现以机器可读清单(JSON,带 schema 校验)到达,人类可读报告则通过每条发现一个机器标记与之核对。此前的形态手工解析无界的 LLM 撰写报告文本,这在构造上就是 fail-open:解析器的网没有预料到的任何一种渲染,要么产出零条发现——从而认证了一份 snippet 从未被解析过的报告——要么悄悄把发现重新绑定到另一个文件。每一轮都找出新入口(粗体标题、围栏长度、第二个 Location 字段、本地化标题、已驳回发现附录被当成发现区重新进入),因为逐入口打补丁永远关不上这个集合。清单以类型化、逐字的形式到达,因此没有任何东西需要剥离、切分或推断;标记检查是集合比较,任何渲染、章节布局或输出语言都无法绕过。
lib/anchors.ts从 688 行降到 435 行,其测试文件从 1441 行降到 383 行。产物的读写现已针对模块自身早已声明为敌意的树做了加固。 内容打开在内核层拒绝符号链接与无写端 FIFO(
O_NOFOLLOW/O_NONBLOCK),因此窗口期内的替换不再能把树外文件的哈希写进漂移基线。产物写入以 fd 为门,而不是以一个可被替换抢先的lstat为门——sidecar 的 extend 写入发生在 fan-out 中途、在被审计树内部,正是竞争生效之处。捕获的git diff传入--no-ext-diff --no-textconv,被审计仓库自身的配置不再能指定一个程序让审计方执行。gitEnv()像 core 的共享 ignore 探针那样整片丢弃GIT_前缀环境,而不是逐个枚举通道——仅GIT_LITERAL_PATHSPECS一项就会静默清空 tracked 臂与捕获共同依赖的:(literal)pathspec。注册 caller 通道获得 walk 早已具备的作用域与 secret 策略;plan 的根路径必须归一化(不只是绝对路径);exclude 补救在写入前校验 common dir 属于本仓库;报告 slug 在写入方截断,使守护的校验器与写入方成为同一条规则;下一日历日探针在 DST 回拨日仍然正确。有两条限制是接受而非关闭的,且每份报告头都会说明:估算的每行费率是作者自报的(其原始记录不在本仓库),以及 sidecar 不具备防篡改证据——它检测的是意外漂移,而非主动隐藏踪迹的模块。审计真正不可信代码的运行应把产物落到仓库之外,这正是 local-only 守护已经提供的选项。
为什么需要
现有
/review面向增量 diff 和 PR 决策。维护者同样需要面向已合入存量模块的高召回、低噪声审计手段——那里没有基线,也没有作者意图可依赖。两轮对照实验发现维度扇出比朴素单 agent 评审发现显著更多经确认的缺陷,且自我裁决零误报。按设计文档的复用边界,
/audit不跨命令组从commands/review/导入任何东西:分类、roster、brief、预算、覆盖检查和 anchor 验证都在 audit 自有代码中重表达。三个片段 lift 到共享位置而不是复制:遍历安全的safeTargetslug(到packages/cli/src/utils/)、两份私有git check-ignore副本合并为packages/core/src/utils/中一个无缓存的共享 helper,以及 Storage hub 上的 per-user 仓库外产物回退目录。评审测试计划
如何验证
qwen audit plan-files packages/core/src/permissions --out <tmp>与... packages/core/src/hooks ...。确认两个校准模块都被接纳并给出实测行数(7,638 subject / 8,640 test 与 8,516 / 16,335),估算区间覆盖实测成本,且 hooks(而非 permissions)被标记为事件/生命周期模块。packages/core/dist以「只有被排除目录」拒绝;被审计路径下的 gitlink 以子模块规则拒绝;low 档在超过 2,000 subject 行时拒绝并指向 medium。qwen audit check-anchors --plan … --findings … --report …在清单 anchor 全部解析且标记匹配时退出 0;在报告发出清单未列出的块、清单中某条发现没有对应块、或清单本身格式错误时以markerProblems退出 4。确认空清单能够退出 0——干净的审计必须能通过这道门。.git指针会让 exclude 补救被拒,而真实 linked worktree 仍能正常落地;凭据形状的 caller 与仓库外的 caller 被拒绝并记录;带..的 plan 根被判为陈旧;裸仓库*.git目录与树内QWEN_HOME永不被 walk;未被名字列表预料到的凭据内容被记为 secret-shaped。/audit <小模块>。确认启动前确认列出估算与两项执行选择加入,每份 agent brief 以 untrusted-data 前置声明开头,敷衍返回的 agent 会被重启一次、再敷衍则记为未审计,去重保留成员中的最高严重度并给出完整性回执,每个簇都经过验证分片,报告落在.qwen/audits/<date>-<time>-<slug>.md且带有运行元数据头、walk 记录与未行使机制标记——并且没有结论性判决。证据(前后对比)
前:已发布 CLI 没有
audit命令;qwen audit …调用会落入交互式提示处理。后:构建出的 CLI 注册了 audit 命令族与 bundled skill。完整 audit 套件全绿(命令组 294 个测试),core 的 config 与 skills 套件同样全绿。本分支上有三个 CLI 测试文件失败——
AuthDialog、systemController、run-qwen-serve-live——已逐个在同一 worktree 中切到未改动的origin/main复测,失败完全一致,属既有问题且与本改动无关。测试平台
环境(可选)
macOS 上的 Node.js 22 开发检出。已通过目标 CLI 与 core 测试套件(含守护、子模块拒绝、exclude 补救归属、sidecar/drift 各臂的 git 夹具测试)、TypeScript 类型检查、ESLint 与 Prettier 验证。
风险与范围
关联 Issue
实现 #8397。取代 #8403。