feat(audit): add legacy code audit workflow - #8403
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Re-run after a major push — the head moved from
Moving on to code review. 🔍 中文说明一次大推送后的重跑——head 从
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRead the full 11,359-line diff statically against the base tip (per triage rules, no PR code was built or executed here). Growth since the last triage head ( The prior round's one finding is fixed. Integration surface verified against the base tree. The lifts are de-duplications with every mirror kept in lockstep: The audit tree held up under a full-depth blocker sweep (walkers, gates, guard, captures, anchors, briefs, parse-args): symlinks Non-blocking, for the record: (1) sequenceDiagram
participant P1 as User
participant P2 as audit skill orchestrator
participant P3 as qwen audit CLI helpers
participant P4 as git and filesystem
participant P5 as Dimension agents
participant P6 as Verifier shards
P1->>P2: audit path with effort flag
P2->>P3: parse-args, then plan-files
P3->>P4: walk, classify, gate, guard probes
P3-->>P2: plan JSON, or refusal exit 3
P2->>P1: roster, estimate, execution opt-ins
P1-->>P2: confirm
P2->>P3: snapshot (sidecar captured)
P2->>P5: roster briefs via agent-prompt
P5-->>P2: findings with return contract
P2->>P3: drift-check, guard-check
P2->>P6: clusters for verification
P6-->>P2: confirmed, confirmed-low, rejected
P2->>P3: check-anchors at write time
P2-->>P1: report in .qwen audits, no verdict
Files changed (30 of 63 shown)
Testing — the PR's own CISingle fetch, no polling. On the reviewed head
Not verified: real-model end-to-end orchestration of the skill at any tier (disclosed by the PR as out of scope for deterministic testing); the macOS/Windows matrix legs and the CLI integration suite (merge-queue-only by design — they run before merge, not on the PR page); the high-tier reverse-audit loop, 1c quotas, and ceiling constants (unmeasured first cuts, disclosed in the report header per the SKILL.md). Sandboxed verification would settle one remaining user-visible slice: 中文说明代码审查按 triage 规则完整静态阅读了 11,359 行 diff（未构建或执行任何 PR 代码）。相比上次 triage 的 head（ 上一轮的唯一发现已修复。 集成面在 base 树上逐项核实。 lift 是真正的去重且所有镜像同步： audit 树经受了全深度阻塞扫描（遍历、门限、守护、捕获、anchor、brief、parse-args）：symlink 被 非阻塞记录：（1）review repository-context 的 （时序图见英文部分；流程为：parse-args → plan-files → 确认（含两类执行 opt-in）→ snapshot → 维度 agent fan-out → drift/guard 检查点 → 验证分片 → check-anchors → 报告落盘，无结论。） 测试——PR 自己的 CI单次读取，不轮询。被审 head 未验证：任何档位的真实模型端到端编排（PR 披露为确定性测试范围之外）；macOS/Windows 矩阵腿与 CLI 集成套件（按设计仅合并队列运行——合并前会跑，PR 页面不跑）；high 档反审循环、1c 配额与上限常量（未测量首版，按 SKILL.md 在报告头披露）。 沙箱验证可以补齐剩余的一个用户可见切面： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — every stage clean on the current head; the remainder is the model-driven orchestration that no deterministic test can exercise, and three merge-queue-only CI legs. Stepping back: this is the third triage pass, and the question this round is whether the near-tripling of the diff since The prior round's one finding (the sidecar's "recorded by name only" comment describing behavior the code didn't have) is fixed properly — Why not 5/5:
Verdict: approve — all stages clean, the approval guardrail doesn't trip (same-repo 中文说明置信度：4/5 —— 当前 head 上各阶段都干净；剩下的是任何确定性测试都无法覆盖的模型驱动编排，以及三条仅合并队列运行的 CI 腿。 退一步看：这是第三次 triage，本轮的问题是 上一轮的唯一发现（sidecar"按名记录"注释描述了代码并不具备的行为）已被正确修复—— 为什么不是 5/5：
结论：批准——各阶段干净，批准护栏不触发（同仓库 — Qwen Code · qwen3.8-max Reviewed at |
|
Qwen Code review timed out. Qwen review timed out after 14400 seconds (of the 240-minute budget). This run already used the maximum 240 minute timeout. See workflow logs. |
1 similar comment
|
Qwen Code review timed out. Qwen review timed out after 14400 seconds (of the 240-minute budget). This run already used the maximum 240 minute timeout. See workflow logs. |
# Conflicts: # docs/design/legacy-code-audit.md
Rework the stacked implementation to match docs/design/legacy-code-audit.md as merged in #8397: - hard topology gates (9,000 subject / 18,000 test lines) refuse at plan time; the above-gate chunk topology, heavy-file invariant triple, and chunk agent prompts are removed from v1 - filesystem-walk enumeration (not git ls-files) with name-excluded directories, vendor/ kept a subject, test-shaped paths under vendor/ classified as test, and uncoverable subjects (binary, over-cap lines, symlinks, non-regular files) recorded by name - two-rate token estimate with the 60M cap enforced at plan time; low tier gets its own 2,000-line gate, a single reader sub-agent, angle rotation minus B, and the 10-finding cap - submodule/gitlink refusal, event-module detection for 1c's event-coverage brief, and reserved-prefix residue surfacing - the local-only guard: consolidated git check-ignore helper in packages/core, index probe for force-added history, exclude remedy via the common-dir exclude file, and an outside-repo Storage fallback - every brief opens with the untrusted-data preamble and carries the substantive-return (whiff check) contract; 1c's N=10 deep-read quotas - snapshot/drift-check/guard-check/check-anchors subcommands keep the sidecar capture, per-file content-drift checkpoints, and write-time anchor resolution deterministic - safeTarget lifts to packages/cli/src/utils for both skills - user docs page (docs/users/features/legacy-audit.md) naming the --effort vocabulary collision with /review Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Triage follow-up — the branch has been reworked to match the design doc as merged in #8397, and the PR is retargeted to
Also from the rework, visible in the new plan output: hard topology gates (9,000 subject / 18,000 test), the two-rate estimate with the 60M cap enforced at plan time, submodule refusal, event-module detection, the local-only guard with the consolidated One deliberate deviation, flagged: the findings-schema lift to |
…cation cwd - tokenizeArgs moves to the CLI-level shared home so /audit consumes it without importing across command groups from commands/review/ - the local-only guard and the exclude remedy now compute probe paths and rule anchors relative to the worktree toplevel, covering an invocation from a repository subdirectory Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The callers file exists only when 1c registered deep-read callers (medium/high); at low it never exists and the flag would fail the read. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
@qwen-code /triage |
|
@qwen-code /takeover |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 146 passed · 0 failed · 146 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：146 通过 · 0 失败 · 146 总计 Verification reportVerification report — PR #8403
|
| cell | build | oracle | result |
|---|---|---|---|
| head · permissions | PR head bundle | exit 0; subjectLines 7,638; testLines 8,640; estimate 32,500,746–42,250,970; event not detected; roster 1a,1c,2,3a,3b,3c,4,5,6a; slug packages_core_src_permissions |
PASS |
| head · hooks | PR head bundle | exit 0; subjectLines 8,516; testLines 16,335; estimate 46,001,307–59,801,699 (< 60M cap); event detected (78 call sites / 8 files) | PASS |
| base · help | base worktree build (core+cli) | exit 0; 0 audit mentions; qwen review still listed (control live) |
PASS (as predicted) |
| base · invocation | same | audit plan-files … --out … → exit 1, yargs Unknown argument: out |
PASS (as predicted) |
| head · help (symmetric control) | PR head bundle | exit 0; lists qwen audit |
PASS |
The estimate brackets match the description's claimed 32.5M–42.3M / 46M–59.8M to the token, and the design doc's pinned rates (2,607 / 1,457 per line) reproduce the measured calibration costs at the merged tree — including the precision case (rounded 2,600/1,500 rates would price hooks' top at 60.6M, over the cap).
Reviewer Test Plan walk-through
| plan step | outcome |
|---|---|
| 1. calibration modules admitted with measured counts, brackets, hooks-but-not-permissions event flag | reproduced exactly (A/B table above) |
2. plan-time refusals: >9,000 subject (exit 3, subject gate); packages/core/dist "only excluded directories"; gitlink refusal naming the submodule rule; >60M corner at the cap; low refuses >2,000 and points at medium |
all reproduced — 02-refusal-gate-matrix.png (27 assertions incl. test-gate, tests-only, low-tier angle/sweep shape) |
3. local-only guard: unprotected → exclude remedy flips probe without touching tracked .gitignore; force-added → tracked; .qwen/*+!** re-include → in-repo landing refused in favor of the outside-repo fallback |
all reproduced — 03-guard-submodule-matrix.png (36 assertions incl. subdirectory invocation, linked worktree, no-worktree, guard-check exit 5/0) |
| 4. medium E2E with real model fan-out | not executable here — the skill is model-driven orchestration; see Not covered. All seven CLI seams the skill drives were exercised instead (parse-args, plan-files, agent-prompt, snapshot, drift-check, guard-check, check-anchors) |
| 5. run-state helpers: path-scoped diff, untracked copies, content hashes; per-file content drift (content-preserving HEAD move stops nothing); check-anchors unique resolution incl. registered out-of-path callers | all reproduced — harness-04, 37 assertions |
Mutation matrix (vacuity)
evidence/04-mutation-matrix.png. Unmutated control green first; each mutant killed by the named test; every restore re-verified green.
| mutant | killed by | restored green |
|---|---|---|
| subject rate 2,607→2,600 | estimate and token cap > the precision case: rounded rates would refuse the hooks module |
yes |
| low-gate compares against the 9,000 subject gate | buildFilesPlan gates > refuses low over its own gate and points at medium |
yes |
ambiguous anchors silently resolved |
resolveAnchors > … flags an ambiguous one |
yes |
isGitIgnored always false |
isGitIgnored > answers git's own verdict… + > is fresh by default… |
yes |
4/4 killed, 0 survivors.
Corrections
- The description's "Before" evidence says an attempted
qwen audit …invocation "falls through to interactive prompt handling". Measured on the base build: with any audit-specific flag present (--out), yargs rejects withUnknown argument: out(exit 1) before any interactive start; only a flag-less invocation would enter the default prompt path. Either way the command is absent — the correction is to the description's characterization of the base cell, not to the code.
Findings
F1 (Suggestion, non-blocking) — unreadable registered callers are silently dropped, and the comment says otherwise. sidecar.ts documents an unreadable caller as "recorded by name only", but both the first-capture and the --callers-extension paths catch and discard, so the caller never appears in callerHashes. Consequence, measured live (harness-04 D8): a caller whose file vanishes between 1c's registration and the snapshot is indistinguishable from never having been registered — drift-check iterates callerHashes, so nothing ever watches it, and the report header's walks record cannot name the loss. This is the audit's own "nothing-found vs could-not-measure" distinction, one level down. Not blocking: the happy path and the drift path for registered-and-readable callers are correct (D7). The fixture that would pin a fix: register a caller, delete the file, snapshot with --callers, and assert the sidecar names the missing caller (and drift-check reports it) instead of dropping it silently.
F2 (completeness, non-blocking) — two lifted/added helpers have no direct unit tests. Storage.getAuditFallbackDir (0700 mode, QWEN_HOME override) and safeTarget carry no collocated tests; both were verified live this round (G1/P5 and the traversal probe: ../../evil→evil, backslashes and dot-runs flattened, dotted slugs preserved). Per house rules a missing test for changed behavior is a Suggestion; the behaviors are correct as measured.
Not covered
- Real-model E2E orchestration (Reviewer Test Plan step 4): the skill body is executed by an interactive model session; this round verified every deterministic seam it drives plus the shipped SKILL.md (including the last commit's conditional
--callersinstruction, present in both shipped copies), but not the model-driven flow itself. The author discloses the same gap. - Per-commit attribution: the checkout is depth 2;
git rev-list HEAD^1..HEAD^2returns 1 against 9 commits in the metadata snapshot, so only the aggregateHEAD^1..HEADdiff was verified. - Windows behavior (author marks
⚠️ ). Linux is covered live — every harness in this round ran on this Linux container (node 22), which the author's "Tested on" table marks untested. - High-tier reverse-audit loop, 1c quotas, ceiling constants — prompt-level constants disclosed by the author as unmeasured first cuts; not re-measured here.
- Repo-wide gates (lint, typecheck, full suites) — covered by the PR's own CI; only affected-workspace suites were re-run from a known-clean state.
- Three harness self-bugs were fixed mid-round (G8 needed a detached worktree; D3 needed a path-scoped
git addso the "elsewhere" commit stays outside the audited path; P5's assertion assumedQWEN_HOMEnests under.qwenwhen it replaces it). All three were my harness errors — the PR code behaved correctly; final numbers come from the corrected harnesses.
Methodology
Environment: node:22 Linux container, merge-ref checkout (HEAD merge commit, HEAD^1 base tip, HEAD^2 PR head), npm ci + npm run build pre-run at head; head cells drove the esbuild bundle dist/cli.js. Base control: git worktree add tmp/base-tree HEAD^1 + npm run build -w packages/core -w packages/cli (~1 m total) after symlinking the root node_modules (plus the nested packages/{core,cli}/node_modules carrying ajv@8/mime) into the worktree; readlink -f node_modules/@qwen-code/qwen-code-core asserted to point at the head tree, so the control's scope is command registration, which lives entirely in packages/cli (config.ts/audit.ts) and is unaffected by the core link. The PR leaves package.json/lockfile untouched, so the dependency tree is identical across arms. Harnesses harness-01-ab.mjs … harness-06-mutations.mjs live in this artifact dir; raw logs in logs/; evidence PNGs rendered by scripts/verify-capture.mjs. Assertion counts in assertions.json are the harness-emitted checks only (146); the 337 suite tests are cited separately above.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-dev-bot
added
the
autofix/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 冲突，直到移除标签或达到轮次上限。移除 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Review Summary
This PR adds the /audit legacy code audit command family to the CLI, implementing design doc #8397. The code quality is high, test coverage is thorough (6652 test diff lines), and the security design is well-thought-out (untrusted-data preamble, local-only guard, drift protection). The PR has been through many review rounds and received an APPROVED review from @qwen-code-ci-bot.
Overall Assessment
The code is well-structured with clear comments and comprehensive tests. Below are the supplementary suggestions from this review.
Suggestions
1. guard-check.ts: fallbackLandingSafe and planRelocated error handling
guard-check.ts lines 90-95 and 67-70: The try/catch blocks around realpathSync and lstatSync treat all errors (including ENOENT) as unsafe or not relocated, causing exit 5 to re-fire. When fallbackRoot contains a path segment that does not yet exist, realpathSync throws ENOENT, but the path may become available on the next check (e.g., during mid-run directory creation). ENOENT should be treated as path not yet available, but safe to retry rather than unsafe.
2. parse-args.ts: readFileSync(0, utf8) blocking risk
parse-args.ts line 129: readFileSync(0, utf8) reads from stdin. When stdin is connected to a pipe, this call blocks until the pipe is closed. If the pipe never closes (e.g., upstream process crashes or hangs), this call blocks indefinitely. Consider using readGuarded or setting a read timeout.
Confirmed
- Object.hasOwn replacement for in operator applied in agent-prompt.ts, fixing the prototype key matching issue.
- isGitIgnored shared helper in packages/core/src/utils/git-ignore.ts with thorough test coverage including environment variable leakage arms.
- safeTarget correctly handles path traversal, long path truncation, and hash suffixing.
- All subcommand exit codes are well-designed (0=success, 3=refusal, 4=anchor unresolved, 5=guard triggered).
- Security design (untrusted-data preamble, guarded read, O_NONBLOCK, size cap, local-only guard) is consistent and thorough.
Not Reviewed
- Build failed for packages/audio-capture (requires Python/node-gyp, not available on this Windows environment) - unrelated to PR changes.
- End-to-end integration tests were not run (requires interactive CLI session).
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Qwen Code Review — Round 1 (deepseek-v4-flash)
This is a consolidated body comment for PR #8403. The automated inline-comment path was not available in this execution environment, so findings are summarized below.
Overall verdict
Issues found — all are medium, suggestion, or low severity; no blockers.
Findings
1. guard-check.ts — fallbackLandingSafe/planRelocated error handling
The try/catch treats every error (including ENOENT) as "unsafe", which causes exit 5 to be raised repeatedly.
2. parse-args.ts — blocking readFileSync(0, 'utf8')
Reading from file descriptor 0 with readFileSync blocks indefinitely if the pipe is never closed.
3. files-plan.ts — stripCommentsAndStrings misses regex literals
Event detection may produce false positives because regex literals are not stripped.
4. files-plan.ts — applyExcludeRemedy in non-standard repos
May create .git/info/exclude in repositories that do not follow the standard layout.
5. audit-agent-briefs.test.ts — missing persona assertions
Personas 6b/6c do not have dedicated test assertions.
6. files-plan.ts — repeated dirHasTrackedFiles calls
Performance concern: dirHasTrackedFiles is invoked for every excluded directory.
7. files-plan.ts — vendor detection over-matches
Any path segment containing "vendor" is treated as vendor code.
8. guard-check.ts — fallbackLandingSafe probes isGitIgnored on non-existent paths
Medium severity: the guard calls isGitIgnored on paths that do not exist.
9. sidecar.ts — captureUntrackedArm omits --exclude-standard
Medium severity: on large repositories this may enumerate ignored files and time out.
10. sidecar.ts — copyGuarded does not protect against FIFO swap
Low severity: target files could be swapped with FIFOs during copy.
11. guard-check.ts — hard-link detection relies on nlink
Low severity: nlink hard-link counting is not reliable on non-NTFS filesystems.
12. sidecar.ts — TOCTOU in writeSidecarJson
Low severity: window exists between lstatSync and writeFileSync.
13. anchors.ts — \r line endings not normalized
Low severity: snippets with \r endings may fail anchor resolution.
14. sidecar.ts — no dedicated tests for re-entry / corruption recovery
Low severity: recovery paths lack targeted coverage.
Positive notes
- Code quality is high and the security design is thoughtful.
- Test coverage is broad; integration paths and edge cases are largely verified.
- No critical or high-severity issues were identified.
Qwen Code Review — Round 1 (deepseek-v4-flash)This is a consolidated body comment for PR #8403. The automated inline-comment path was not available in this execution environment, so findings are summarized below. Overall verdictIssues found — all are medium, suggestion, or low severity; no blockers. Findings1.
|
qwen-code-ci-bot
left a comment
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 (it would exercise the new qwen audit subcommands).
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the Windows suite did not run locally (relevant: R17-3 Windows-only mechanism).
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and the macOS suite did not run locally.
Not reviewed: reverse audit — stopped before round 2 by the review time budget.
Deferred under the convergence posture (round 18, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/audit/guard-check.test.ts:336 — [probe] landing planTs probe arm untested on any credit pathpackages/cli/src/commands/audit/lib/anchors.test.ts:392 — [probe] pendingLocations restart guard unpinned; mutant ships greenpackages/cli/src/commands/audit/lib/anchors.ts:151 — [probe] pair delimiter 'and' case-sensitive against the module's…packages/cli/src/commands/audit/lib/anchors.ts:381 — [probe] deviated-header arm lacks the field arm's…packages/cli/src/commands/audit/lib/audit-agent-briefs.test.ts:166 — [probe] walked-vs-gate fixture cannot discriminate the two totalspackages/cli/src/commands/audit/lib/audit-agent-briefs.ts:59 — [review] recall counterweight dropped; no Confidence field in the…packages/cli/src/commands/audit/lib/files-plan.test.ts:527 (+1 locations) — [probe] EVENT_CALL_MIN and EVENT_SCAN_MAX_CHARS unpinnedpackages/cli/src/commands/audit/lib/files-plan.test.ts:1411 — [probe] written-vs-probed oracle pins only the specialist shapepackages/cli/src/commands/audit/lib/files-plan.test.ts:1574 (+4 locations) — [probe] env-scrub test/coverage gaps across the git probespackages/cli/src/commands/audit/lib/files-plan.ts:284 — [probe] probeFailed vendor flip walks dist/build as subjectspackages/cli/src/commands/audit/lib/files-plan.ts:403 (+2 locations) — [probe] event-scan lexer: regex literals, ${} interpolation,…packages/cli/src/commands/audit/lib/files-plan.ts:1317 — [probe] no file-count gate; oversized self-written plan deadlocks…packages/cli/src/commands/audit/lib/read-json.ts:58 — [review] plan per-entry traversal guard untested though load-bearingpackages/cli/src/commands/audit/lib/safe-read.ts:80 — [probe] readGuarded/readFdCapped cap untested; partial-plan…packages/cli/src/commands/audit/lib/sidecar.ts:128 — [probe] startsWith('..') gate over-matches ..data-style segmentspackages/cli/src/commands/audit/lib/sidecar.ts:193 — [probe] header contract states all-fail; implementation is any-failpackages/cli/src/commands/audit/lib/sidecar.ts:502 — [probe] loadSidecar element types unvalidated; tamper → phantom…packages/cli/src/commands/review/lib/manifest-repository-context.test.ts:232 — [test] inert test: suite green with the paired source change…packages/cli/src/commands/review/lib/manifest-repository-context.test.ts:575 — [review] rationale example 100-pattern list no longer exceeds the…packages/cli/src/config/config.ts:1130 — [probe] audit exiting-subcommand membership untested; fall-through…- …and 2 more (see the run report)
[Critical] R16-19（仍然成立 / still stands — 第 18 轮 body 级 blocker 复核）: the merged design doc's Records ship criterion requires the calibration records under docs/design/assets/ to be checkable BEFORE the token-rate constants are coded; no such records exist at this head (verified again this round: docs/design/assets/ holds only 7 unrelated images), while SUBJECT_TOKENS_PER_LINE=2607 / TEST_TOKENS_PER_LINE=1457 remain coded at files-plan.ts:63-64 and are consumed by the estimate math at :1152. The maintainer decision escalated in comment 3761719334 (land redacted records and re-derive, or explicitly accept the author-reported rates) remains open. 中文：已合入设计文档的 Records 交付准则要求校验记录可检查之前不得编码 token 费率常量；本 head 仍无此类记录，常量仍被估算数学消费；维护者决策仍然开放。
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (it would exercise the new qwen audit subcommands)。
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the Windows suite did not run locally (relevant: R17-3 Windows-only mechanism)。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and the macOS suite did not run locally。
未审查：反向审计——评审时间预算不足，未能开始第 2 轮。
收敛姿态下延后（第 18 轮，非阻断）——已记录，本轮不要求修改：共 22 条（原文未翻译，列表见上方英文部分）。
[Critical] R16-19（仍然成立 / still stands — 第 18 轮 body 级 blocker 复核）: the merged design doc's Records ship criterion requires the calibration records under docs/design/assets/ to be checkable BEFORE the token-rate constants are coded; no such records exist at this head (verified again this round: docs/design/assets/ holds only 7 unrelated images), while SUBJECT_TOKENS_PER_LINE=2607 / TEST_TOKENS_PER_LINE=1457 remain coded at files-plan.ts:63-64 and are consumed by the estimate math at :1152. The maintainer decision escalated in comment 3761719334 (land redacted records and re-derive, or explicitly accept the author-reported rates) remains open. 中文：已合入设计文档的 Records 交付准则要求校验记录可检查之前不得编码 token 费率常量；本 head 仍无此类记录，常量仍被估算数学消费；维护者决策仍然开放。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| inFence = | ||
| value.startsWith('```') && | ||
| !(value.length >= 6 && value.endsWith('```')); |
There was a problem hiding this comment.
[Critical] R18-1: an inline Anchor field value that merely STARTS with three backticks (e.g. - Anchor: ```bash) latches inFence open with no closing fence ever possible — every subsequent line (later fields, entire subsequent findings) is swallowed into this anchor's content, and a false "the draft is truncated" synthetic entry is emitted at EOF — Failure scenario: a finding whose anchor cites a fence language tag (natural when auditing SKILL.md-class files) destroys every later finding in the same draft; remediation then chases a nonexistent truncation.
Witness (probe-verified this round):
parse of [finding1 "- Anchor: ```bash" + fields + complete finding2] →
finding1.anchorHead = "```bash\n- Issue: a..." (finding2's lines, swallowed)
finding2 ABSENT; synthetic "unclosed anchor fence … the draft is truncated" appended
fix (lookahead latch) → anchor stays "```bash", finding2 parses. Flipped.
Note: the self-closing guard only exempts values that also END with ```. A fix must distinguish a single-line fence-opener anchor from a genuinely truncated fence (the naive latch fix breaks the existing truncation test).
中文说明
以三个反引号开头但不闭合的行内 Anchor 字段值（如 - Anchor: ```bash）会把 inFence 永久锁开：其后所有行（后续字段、整个后续 finding）全部被吞进该 anchor，并在文件末尾追加一条虚假的"草稿被截断"合成条目。探针已实证：完整草稿中第二个 finding 被整体吞没；修复后翻转。注意自闭合守卫只豁免同时以 ``` 结尾的值。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| if ( | ||
| hi - lo >= 2 && | ||
| collected[lo].trim().startsWith('```') && | ||
| collected[hi - 1].trim().startsWith('```') | ||
| ) { |
There was a problem hiding this comment.
[Critical] R18-2: the surround-fence strip assumes fence-shaped first/last lines are the author's wrapping, so an UNWRAPPED citation of a real fence block (a snippet from a .md file whose own first/last lines are fence markers) is silently stripped to its interior — flipping anchor verdicts in BOTH directions — Failure scenario: (a) file has only a sh block with `npm run build`; report cites a nonexistent bash block → strip leaves npm run build → resolved matchCount 1, certifying fence lines that do not exist; (b) the ```bash block exists once but its interior repeats in a second block → ambiguous → exit 4 refuses a correct unique anchor, and remediation cannot converge without a double-wrap workaround.
Witness (both arms probe-verified this round):
(a) PR: verdict=resolved matchCount=1 for the fabricated ```bash citation
fix (strip only when a fence opened on the Anchor field line): unresolved. Flipped.
(b) PR: verdict=ambiguous matchCount=2 (correct unique anchor refused)
fix: resolved. Flipped.
中文说明
push() 的围栏剥离仅凭首/尾行的形状假设作者做了包裹，因此对真实围栏块的未包裹引用（.md 文件中首尾即围栏行的片段）会被静默剥成内部内容，两个方向都翻转裁决：(a) 为不存在的 ```bash 块认证 resolved；(b) 拒绝正确的唯一锚点且补救无法收敛。两臂均已探针实证并翻转。建议仅当收集期间确实观察到围栏打开时才剥离。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| hi--; | ||
| } else if (hi - lo === 1) { | ||
| const trimmed = collected[lo].trim(); | ||
| if (/^`+$/.test(trimmed)) { |
There was a problem hiding this comment.
[Critical] R17-4: the residue guard blanks only fence-marker-ONLY lines; a single-line inline-code fence wrapping the whole snippet survives, and - Anchor: peels via slice(1,-1) to a single space that passes the !finding.anchor guard — 第 18 轮复核：仍然成立（still stands at head 580b439）. Round-17 probe: {anchor: " ", verdict: resolved, matchCount: 1} against a file with one trailing space. 完整机制描述见本线程先前评论（full description in the earlier thread comment）.
中文说明
残留守卫只清空"纯围栏标记行"；单行行内代码围栏包裹的片段会残留，且 - Anchor: 经 slice(1,-1) 剥成单个空格后可通过 !finding.anchor 守卫，对含行尾空格的文件裁决 resolved。第 18 轮复核确认机制在 580b439 仍然成立。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| locations: parseLocations(field[2]), | ||
| raw: field[2], |
There was a problem hiding this comment.
[Critical] R17-29: a genuine multi-location citation written as two Location fields mis-binds — the second Location lands in pendingLocations, is committed only by a second Anchor, and is nulled in push(); the second cited file is silently dropped, never scope-checked or matched — 第 18 轮复核：仍然成立（still stands at head 580b439）. Round-17 probe: two Location fields + one Anchor → locations=["sub.ts"], second file dropped, grades resolved. 完整机制描述见本线程先前评论（full description in the earlier thread comment）.
中文说明
以两个 Location 字段书写的多位置引用会错误绑定：第二个 Location 进入 pendingLocations，仅在出现第二个 Anchor 时提交，并在 push() 中被置空——第二个被引用文件被静默丢弃，既不做作用域检查也不做匹配。第 18 轮复核确认机制在 580b439 仍然成立。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| if (line.startsWith('```')) { | ||
| inFence = !inFence; | ||
| if (inFence) anchorFenced = true; |
There was a problem hiding this comment.
[Critical] R17-28: anchorFenced latches on ANY fence pair — an innocent inner fence pair inside a quoted anchor flips the latch, changing how a following Location+Anchor pair commits (fenced pairs keep severity/locations; unfenced pairs downgrade severity to '') — the same input parses differently depending on unrelated inner fences — 第 18 轮复核：仍然成立（still stands at head 580b439）. Round-17 probe: innocent inner fence pair → following pair commits locations=["prior.ts"] severity kept; identical unfenced arm → severity:'' downgrade. 完整机制描述见本线程先前评论（full description in the earlier thread comment）.
中文说明
anchorFenced 会在任意围栏对上锁存：引用锚点内部一个无关的围栏对即可翻转锁存，从而改变后续 Location+Anchor 对的提交语义（围栏对保留 severity/locations，未围栏对降级 severity 为 ''）——相同输入因无关内容而解析出不同结果。第 18 轮复核确认机制在 580b439 仍然成立。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| // of the worktree — probe paths must be toplevel-relative. Both sides are | ||
| // realpath'd: git reports the symlink-resolved toplevel. | ||
| const prefix = toPosix( | ||
| relative(realpathSync(gitRoot), realpathSync(projectRoot)), |
There was a problem hiding this comment.
[Critical] R17-5: guardDir calls realpathSync unguarded in the prefix computation run on every probe (both call sites are bare) — a vanishing/unresolvable path throws out of the guard instead of failing closed with a verdict — 第 18 轮复核：仍然成立（still stands at head 580b439）. Re-check: relative(realpathSync(gitRoot), realpathSync(projectRoot)) (:919) and relative(realpathSync(gitRoot), target) (:884) remain bare; no try/catch in guardDir or its callers. 完整机制描述见本线程先前评论（full description in the earlier thread comment）.
中文说明
guardDir 在每次探测都要执行的前缀计算里裸调用 realpathSync（两处均无保护）：路径消失或不可解析时会直接抛出 guard，而不是给出 fail-closed 裁决。第 18 轮复核确认两处调用在 580b439 仍无 try/catch。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| * would greet every retry of the prescribed remedy. */ | ||
| function writeSidecarJson(path: string, sidecar: Sidecar): void { | ||
| try { | ||
| if (!lstatSync(path).isFile()) { |
There was a problem hiding this comment.
[Critical] R18-4: sidecar destination writes lack the fd-gated discipline the module applies to reads/copies — writeSidecarJson gates lstatSync(path).isFile() THEN writeFileSync(path) (a stat-then-use TOCTOU whose internal open follows symlinks); diff.patch is written with a bare writeFileSync (no gate at all — deterministic); copyGuarded gates only the SOURCE fd, leaving copy destinations ungated (deterministic) — Failure scenario: the audited agent (the module's stated threat model; the sidecar lives inside the audited repo) plants a symlink at diff.patch or a copy destination → repo diff / module-chosen content lands at any user-writable path; a writer-less FIFO at sidecar.json hung in the lstat→open window blocks the prescribed remedy retry forever.
Witness (probe-verified this round):
pre-planted symlink at <sidecar>/diff.patch → victim file contained the repo diff
pre-planted symlink at an untracked-copy destination → victim contained attacker content
fd-gated O_NOFOLLOW destination fix → both victims unchanged. Flipped.
Suggested fix: route all three destination writes through one fd-gated writer (open O_WRONLY|O_CREAT|O_TRUNC|O_NOFOLLOW, fstat regular-file gate, write via fd), mirroring copyGuarded's source discipline.
中文说明
sidecar 目的端写入缺少模块在读取/复制上使用的 fd 门控纪律：writeSidecarJson 先 lstat 后 writeFileSync（check-then-use TOCTOU，且内部 open 跟随符号链接）；diff.patch 完全无门控（确定性可利用）；copyGuarded 只门控源 fd，目的端无门控（确定性可利用）。探针已实证：预置符号链接可把仓库 diff/模块内容写到任意可写路径；修复后翻转。建议三个目的端写入统一走 open+fstat+fd 写入。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| if (top !== null) { | ||
| const headProbe = probeGit(rootAbs, ['rev-parse', 'HEAD'], GIT_TIMEOUT_MS); |
There was a problem hiding this comment.
[Critical] R17-16: a transient rev-parse HEAD failure at capture records NOTHING (neither headSha nor headUnborn nor a failure marker) — driftCheck then reports headUnknown permanently for a born HEAD, with no re-probe path in the capture-time branch — 第 18 轮复核：仍然成立（still stands at head 580b439）. Re-check: the else-if chain records only ok→headSha / unborn→headUnborn; a transient failure falls through with no field set. 完整机制描述见本线程先前评论（full description in the earlier thread comment）.
中文说明
捕获时 rev-parse HEAD 的瞬时失败不记录任何字段（既非 headSha 也非 headUnborn，也无失败标记）：对已出生 HEAD，driftCheck 将永久报告 headUnknown 且捕获分支无重探路径。第 18 轮复核确认该 else-if 链在 580b439 仍只记录 ok/unborn 两态。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
|
||
| const guard = checkLocalOnlyGuard( | ||
| projectRoot, | ||
| `${plan.artifacts.reportSlug}.md`, |
There was a problem hiding this comment.
[Critical] R17-34: the plan-time guard verdict never passes the recovered run-ts through — checkLocalOnlyGuard(projectRoot, \${plan.artifacts.reportSlug}.md`)` is called with two args (no planTs), so plan-time probes ask only their own instant's name shapes; a name-selective re-include keyed to the run-ts name answers differently than the seconds-off probe name, and the plan records ok without offering the remedy — 第 18 轮复核：仍然成立（still stands at head 580b439）. Re-check: guard-check passes the recovered ts as third arg; plan-files never does. 完整机制描述见本线程先前评论（full description in the earlier thread comment）.
中文说明
plan 时刻的 guard 裁决从不传入恢复出的运行 ts：plan-files 只以两个参数调用 checkLocalOnlyGuard（无 planTs），探测只询问探测瞬间的名字形状；按运行 ts 名称做名字选择性 re-include 时，plan 会记录 ok 且不提供补救。第 18 轮复核确认 plan-files 在 580b439 仍不传第三参数。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| (The Step 3 sidecar path is `.qwen/audits/audit-<ts>.sidecar` — or `<fallbackRoot>/audit-<ts>.sidecar` under a fallback landing.) | ||
|
|
||
| - The predicate is per file and keys on **content**, not git state: a file whose content is unchanged is not drifted, whatever HEAD did (a mid-run commit of the run-start dirty state fires the git-state arms and stops nothing). A `headUnknown`/`subtreeUnknown` marker means the git probe failed, so "not moved" is UNKNOWABLE — treat that checkpoint like drift in a walked file: stop and assemble the partial report. Drift in a file already walked **and carrying anchored findings** — walked subjects, test corpus, and registered callers alike — **stops the run**: assemble the partial report through Step 7's anchor resolution before writing it (findings whose anchors no longer resolve against the drifted content are dropped with the refusal recorded — never shipped bound to the changed code), and carry the drift, the phase, and a verification-not-completed mark in the header. Drift in any other file: mark it drifted/uncoverable in the walks record and continue. | ||
| - `guard-check` exits 5 when a module-derived directory became committable mid-run (directories already exposed at plan time do not re-fire ONLY while the plan file itself sits under a fallback root the guard itself verified safe — that is the relocation proof; if exit 5 fires despite a Step 1 relocation, the relocation did not land and must be redone): relocate the intermediates and the sidecar to the plan's `guard.fallbackRoot` immediately, and land the report beside them. Every subsequent command references the relocated paths — `<artifacts-dir>` IS `<fallbackRoot>` from this point (including Step 7's `check-anchors` and the final checkpoints). |
There was a problem hiding this comment.
[Critical] R18-8: the relocation-proof bullet has no terminal branch for the reachable state where the fallback landing itself cannot be certified — QWEN_HOME is user-settable into a worktree without an ignore rule (the author's own comment names this), and a dotfiles-repo home is the same class — Failure scenario (probe-verified this round): with QWEN_HOME=/qwen-home and a plan-time dir exposed, relocation lands at fallbackRoot; fallbackLandingSafe answers false forever (no ignore rule) → exit 5 at every checkpoint; the prescribed remedy ("the relocation did not land and must be redone") is a no-op; Step 7 still lands the module-quoting report beside the sidecar in the committable fallback root.
Witness:
firstRun=5 redoRun=5 checkIgnore=NOT-ignored (committable)
gitStatus='?? qwen-home/' fallbackInsideWorktree=true
applyExcludeRemedy writes rules only for /.qwen/audits/ and /.qwen/tmp/ — never the fallbackRoot. Suggested fix: add a terminal branch — when exit 5 fires although the plan already sits under the fallback root, stop and assemble the partial report (the headUnknown precedent); correct Step 1's "outside the repo" claim.
中文说明
重定位证明条目缺少终态分支：当 fallback 落点本身无法被认证时（QWEN_HOME 可被用户设置到无 ignore 规则的 worktree 内——作者注释也承认此状态；dotfiles 仓库的 home 同类），exit 5 会在每个检查点重复触发，而唯一书面补救（"重做重定位"）是空操作，Step 7 仍会把引用模块内容的报告落在可提交的 fallback 目录中。探针实证：firstRun=5 redoRun=5、check-ignore 未忽略、git status 显示 ?? qwen-home/。建议增加终态分支（计划已在 fallback 下仍 exit 5 时停止并组装部分报告），并更正 Step 1 "仓库之外" 的表述。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: Handoff — PR #8403 is not converging; needs a maintainer's decisionDecision needed: how to proceed with PR #8403 ( Evidence (verified at this head)
Run log: https://github.com/QwenLM/qwen-code/actions/runs/32128372094 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- PF2 inert manifest-repository-context.test.ts:232 fixture — already reported (round-18 deferred list)
Not reviewed: build-and-test — packages/vscode-ide-companion and packages/webui suites not run (whole-call budget).
Not explored to full depth (tool budget reached): chunk 22: none — all checks I intended completed. (I did not execute the vitest file itself because the worktree has no installed node_modules ; the load-bearing runtime….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 19, not a blocker) — recorded, not requested in this round:
packages/cli/src/config/config.ts:1167 — [test] subcommand flush block untested; deadline-mutant ships greenpackages/cli/src/commands/audit/check-anchors.ts:62 — [probe] exit 0 on zero parsed findings — fully-deviated draft certifies vacuouslypackages/cli/src/commands/audit/check-anchors.ts:58 — [probe] --callers wiring untested; dropping the argument ships greenpackages/cli/src/commands/audit/lib/files-plan.ts:949 — [probe] failed tracked-files probe collapses to [] → ok instead of fail-closedpackages/cli/src/commands/audit/guard-check.test.ts:89 — [probe] guardTripped per-directory keying unpinned; find(()=>true) mutant ships greenpackages/cli/src/commands/audit/guard-check.test.ts:122 — [probe] hermeticity pins miss git's XDG default excludes filepackages/cli/src/commands/audit/lib/files-plan.test.ts:395 — [review] SECRET_FILE_RE fixture leaves 8 of 20 clauses unpinnedpackages/cli/src/commands/audit/lib/files-plan.test.ts:411 — [probe] 'never content-reads them' pins only shape; no-read order unpinnedpackages/cli/src/commands/audit/lib/files-plan.test.ts:1727 — [probe] spawn-failure arm (status null) never exercisedpackages/cli/src/commands/audit/lib/anchors.test.ts:1035 — [probe] BOM test inert against the guard it namespackages/cli/src/commands/audit/lib/anchors.test.ts:708 — [probe] CRLF test misses the mid-line raw-scan pathpackages/cli/src/commands/audit/lib/audit-agent-briefs.test.ts:67 — [review] RETURN CONTRACT test iterates medium roster; 6b/6c never checkedpackages/cli/src/commands/audit/lib/audit-agent-briefs.test.ts:388 — [probe] claim-vs-walked validators tested one direction onlypackages/cli/src/commands/audit/lib/audit-agent-briefs.test.ts:313 — [probe] malformed-lowTier angles-array arm untested; raw crash on deletionpackages/cli/src/commands/audit/plan-files.test.ts:98 — [review] handler tests assert nothing about stderr guard warningsscripts/tests/review-source-digest.test.ts:263 — [review] digest policer doesn't walk the lifted utils rootspackages/core/src/utils/git-ignore.test.ts:94 — [probe] GIT_INDEX_FILE/GIT_OBJECT_DIRECTORY scrub arms vacuouspackages/core/src/skills/bundled/audit/SKILL.md:148 — [review] Step-6 drift bullet has no newFiles branchpackages/cli/src/commands/audit/lib/sidecar.test.ts:761 — [probe] glob pathspec test inert against the :(literal) regressionpackages/cli/src/commands/audit/lib/files-plan.ts:415 — [probe] event-detection lexer corners perturb a disclosed heuristic- …and 5 more (see the run report)
[Critical] R18-3 (body-level blocker carried from round 16 via 18; re-verified this round, still stands): the merged design doc's Records ship criterion requires the calibration records under docs/design/assets/ to be checkable BEFORE the spec's rates and caps are coded; at this head docs/design/assets/ holds only 7 unrelated images (dingtalk/git-mode screenshots), while SUBJECT_TOKENS_PER_LINE=2_607 / TEST_TOKENS_PER_LINE=1_457 remain coded at files-plan.ts:63-64 and are consumed by the estimate math at :1152. The maintainer decision escalated in comment 3761719334 (land redacted records and re-derive, or explicitly accept the author-reported rates) remains open. Witness: re-verified at this head (ls docs/design/assets/ → 7 unrelated images; constants at :63-64 consumed at :1152).
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — packages/vscode-ide-companion and packages/webui suites not run (whole-call budget)。
未探索到全部深度（达到工具调用预算）：chunk 22：none — all checks I intended completed. (I did not execute the vitest file itself because the worktree has no installed node_modules ; the load-bearing runtime…。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 19 轮，非阻断）——已记录，本轮不要求修改：共 25 条（原文未翻译，列表见上方英文部分）。
[Critical] R18-3 (body-level blocker carried from round 16 via 18; re-verified this round, still stands): the merged design doc's Records ship criterion requires the calibration records under docs/design/assets/ to be checkable BEFORE the spec's rates and caps are coded; at this head docs/design/assets/ holds only 7 unrelated images (dingtalk/git-mode screenshots), while SUBJECT_TOKENS_PER_LINE=2_607 / TEST_TOKENS_PER_LINE=1_457 remain coded at files-plan.ts:63-64 and are consumed by the estimate math at :1152. The maintainer decision escalated in comment 3761719334 (land redacted records and re-derive, or explicitly accept the author-reported rates) remains open. Witness: re-verified at this head (ls docs/design/assets/ → 7 unrelated images; constants at :63-64 consumed at :1152).
— qwen3.8-max via Qwen Code /review (v0.21.13)
| * and a linked worktree alike, and does not dirty the tracked .gitignore). | ||
| * Returns the exclude file path written. */ | ||
| export function applyExcludeRemedy(projectRoot: string): string { | ||
| const commonDir = git(projectRoot, ['rev-parse', '--git-common-dir']); |
There was a problem hiding this comment.
[Critical] R19-3: gitEnv() strips the ambient env vars that re-home probes to a foreign repository — the GIT_COMMON_DIR scrub comment a few lines above names this exact harm — but an in-tree regular-file .git containing gitdir: /path/to/victim/.git re-homes rev-parse --git-common-dir identically, with no ambient env at all: gitGeometry reports inWorktree (--show-toplevel answers the corpus itself), and applyExcludeRemedy resolves the exclude file into the VICTIM repository and appends the audit's ignore rules there; the re-probe then flips to ignored and certifies ok off foreign state. — Failure scenario (probe-verified on git 2.43): a planted .git gitfile in the audited corpus (a downloaded/vendored tree — the module's stated target class; relative gitdir: targets reach sibling/parent repos) → the remedy write lands at <victim>/.git/info/exclude, and the guard verdict after the remedy is ok/ok where it was unprotected/unprotected before. The whole verdict chain is computed against a repository the user never asked to touch, planted by one file in the audited content.
Witness: probe — exclude file written at <victim>/.git/info/exclude; guard AFTER remedy: [{dir:.qwen/audits,status:ok},{dir:.qwen/tmp,status:ok}].
Suggested fix: treat a non-directory .git at/above the audit root as untrusted indirection — lstatSync the discovered worktree's .git and refuse the remedy (or report probeFailed) when it is a regular gitfile, or verify the resolved common-dir sits under the discovered toplevel's own .git before writing.
中文说明
问题：gitEnv() 剥离了会把探针重定向到外部仓库的环境变量（上方 GIT_COMMON_DIR 剥离注释明确指出了这种危害），但树内的常规文件 .git（内容为 gitdir: /path/to/victim/.git）在完全无环境变量的情况下同样重定向 rev-parse --git-common-dir：gitGeometry 报告 inWorktree（--show-toplevel 返回语料本身），applyExcludeRemedy 把排除规则写入受害仓库的 info/exclude；重新探测随后翻转为 ignored 并据外部状态认证 ok。
失败场景（git 2.43 探针验证）：在被审计语料中植入 .git gitfile（下载/内嵌树——模块明确的目标类别）→ 补救写入落到 <victim>/.git/info/exclude，guard 判定由 unprotected/unprotected 变为 ok/ok。整条判定链在用户从未要求接触的仓库上计算，仅凭被审计内容中的一个文件即可植入。
建议修复：把审计根及以上的非目录 .git 视为不可信间接层——当发现的 worktree 的 .git 是常规 gitfile 时拒绝补救（或报 probeFailed），或在写入前校验解析出的 common-dir 位于所发现 toplevel 自身的 .git 之下。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| expect(() => resolveAuditRoot(' ')).toThrow(/no directory path/); | ||
| }); | ||
|
|
||
| it('resolves a symlinked target to its real path', () => { |
There was a problem hiding this comment.
[Critical] R19-5: 'resolves a symlinked target to its real path' creates a directory symlink via untyped symlinkSync and is NOT skipIf(win32)-guarded, while 11 sibling symlink fixtures in this same file are (lines 249, 269, 295, 1032, 1117, 1490, 1505, 1536, 1726, 1750, 1765). On Windows, untyped symlinkSync needs elevation/Developer Mode (.github/actions/configure-windows-runner enables nothing symlink-related), and Node 22's default type is 'file' even for a directory target — statSync cannot traverse a file-typed link, so resolveAuditRoot throws instead of returning the realpath. — Failure scenario: the merge_group-only test_windows job (ci.yml; its own comment says the name matches the required-status-check context) runs npm run test:ci including this file — on a runner without the privilege the test fails at fixture creation (EPERM), with it at the assertion: red either branch on a merge-queue gate, while PR CI (pull_request) never runs the job so the author sees no red before queueing. Repo convention proves the hazard is known: permission-manager tests pass explicit 'dir'/'file' types; workspace.image-path.test.ts:107 uses 'junction' on win32.
Witness: not run — targets the Windows merge-gate runner, unavailable on this Linux host; triangulated against Node 22's documented win32 default-type semantics, ci.yml's merge_group gating, and this file's own 11 skipIf(win32) sibling guards.
| it('resolves a symlinked target to its real path', () => { | |
| it.skipIf(process.platform === 'win32')('resolves a symlinked target to its real path', () => { |
(或如需 Windows 覆盖：在 win32 上以 symlinkSync(real, link, 'junction') 创建 fixture。)
中文说明
问题：该测试用无类型的 symlinkSync 创建目录符号链接且未加 skipIf(win32)，而同文件另外 11 个符号链接 fixture 均有防护。Windows 上无类型 symlinkSync 需要提权/开发者模式（configure-windows-runner 未启用任何相关配置），且 Node 22 对目录目标默认类型仍为 'file'——statSync 无法遍历文件型链接，resolveAuditRoot 会抛错而不是返回 realpath。
失败场景：仅 merge_group 触发的 test_windows 门禁作业运行含本文件的全量测试——无权限的 runner 上 fixture 创建失败（EPERM），有权限的 runner 上断言失败：合并队列门禁两个分支都红，而 PR CI 不运行该作业，作者入队前看不到红色。
建议修复：与同类测试一致加上 skipIf(win32)，或在 win32 上用 'junction' 类型创建 fixture。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| ); | ||
| const missing = rules.filter((r) => !existingRules.has(r)); | ||
| if (missing.length > 0) { | ||
| mkdirSync(dirname(excludeFile), { recursive: true }); |
There was a problem hiding this comment.
[Critical] R19-9: applyExcludeRemedy writes the common-dir exclude file with bare writeFileSync — follows symlinks, blocks forever opening a writer-less FIFO, no lstat/regular-file gate — while writeSidecarJson a few hundred lines away gates with lstatSync(path).isFile() and measureFile applies O_NONBLOCK+fstat discipline on this same hardened surface. The destination lives inside the audited (untrusted) repository's .git, and SKILL Step 1 offers the remedy whenever the guard reports unprotected — the default state of a fresh checkout. — Failure scenario (probe-verified, both arms): .git/info/exclude → symlink to a user-writable victim file → the victim's content after the remedy: original lines + # qwen audit: keep audit artifacts out of version control + /.qwen/audits/ + /.qwen/tmp/ (O_TRUNC rewrite through the link — format-sensitive targets corrupted); writer-less FIFO at .git/info/exclude → applyExcludeRemedy blocked until killed at 8 s (exit 124). The module's own comment frames the remedy as careful mutation ('does not dirty the tracked .gitignore'), yet a relative symlink .git/info/exclude → ../../.gitignore does exactly that.
Witness: probe — == VICTIM FILE AFTER REMEDY == # user bashrc / export PATH=/usr/bin / # qwen audit… / /.qwen/audits/ / /.qwen/tmp/ · FIFO arm: exit=124 (still blocked when killed).
Suggested fix: gate the destination like writeSidecarJson — lstatSync when it exists and require a regular file; open with O_NOFOLLOW|O_CREAT, fstat the fd, write through it — or refuse the remedy and let the fallback landing engage.
中文说明
问题：applyExcludeRemedy 用裸 writeFileSync 写 common-dir 排除文件——跟随符号链接、在无读端 FIFO 上永久阻塞、无 lstat/常规文件门——而同模块数百行外的 writeSidecarJson 有 lstatSync(path).isFile() 门，measureFile 在同一加固表面应用 O_NONBLOCK+fstat 纪律。目标位于被审计（不可信）仓库的 .git 内，且 SKILL 第 1 步在 guard 报告 unprotected（全新检出的默认状态）时总会提供该补救。
失败场景（探针验证两臂）：.git/info/exclude 符号链接指向用户可写文件 → 补救后受害者内容被追加审计规则（O_TRUNC 透过链接重写，格式敏感目标被破坏）；无读端 FIFO → applyExcludeRemedy 阻塞直至 8 秒被杀（exit 124）。
建议修复：像 writeSidecarJson 一样为目标加门——存在时 lstatSync 并要求常规文件，或以 O_NOFOLLOW|O_CREAT 打开并经 fd 写入——或拒绝补救、让回退落地生效。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| expect(results[0].verdict).toBe('resolved'); | ||
| }); | ||
|
|
||
| it('resolves against registered deep-read callers outside the path', () => { |
There was a problem hiding this comment.
[Critical] R17-3（第 17 轮提出，第 18 轮复核仍然成立，本轮代码未变，仍然成立）: this test feeds a platform-native path into locations (join() — backslashed on Windows) AND the same value into registeredCallers, but resolveAnchors slash-normalizes only the registeredCallers side into callerSet — the locations value keeps backslashes, membership fails, and the resolved assertion flips to out-of-scope on the Windows merge-queue leg, which ci.yml runs and no skipIf guards. — Failure scenario: test_windows runs the full suite on windows-latest: the normalized callerSet ('/') never matches the backslashed locations entry → verdict out-of-scope → red on a required merge gate.
Witness: not run — Windows-only mechanism, unavailable on this Linux host; the round-18 re-check confirmed the one-sided normalization and ci.yml's unguarded Windows leg at the prior head, and the cited code is byte-identical at this head.
Suggested fix: normalize the locations side the same way (backslash → slash before membership), or build the fixture's expected path from the normalized form.
中文说明
R17-3（第 17 轮提出，第 18 轮复核仍然成立；本轮代码未变，仍然成立）：该测试把平台原生路径（join()——Windows 上为反斜杠）同时传入 locations 与 registeredCallers，但 resolveAnchors 只对 registeredCallers 一侧做斜杠归一化进入 callerSet——locations 值保留反斜杠，成员判定失败，resolved 断言在 ci.yml 运行且无 skipIf 防护的 Windows 合并队列分支上翻转为 out-of-scope。
失败场景：test_windows 在 windows-latest 上运行全量测试：归一化后的 callerSet（'/'）永远匹配不上反斜杠的 locations 条目 → 判定 out-of-scope → 必需合并门禁红。
建议修复：对 locations 一侧做同样归一化（成员判定前反斜杠转斜杠），或让 fixture 从归一化形式构造期望路径。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| // of the worktree — probe paths must be toplevel-relative. Both sides are | ||
| // realpath'd: git reports the symlink-resolved toplevel. | ||
| const prefix = toPosix( | ||
| relative(realpathSync(gitRoot), realpathSync(projectRoot)), |
There was a problem hiding this comment.
[Critical] R17-5（第 17 轮提出，第 18 轮复核仍然成立，本轮代码未变，仍然成立）: guardDir computes the toplevel-relative probe prefix with a bare relative(realpathSync(gitRoot), realpathSync(projectRoot)) — no try/catch — while the IDENTICAL computation in fallbackLandingSafe (guard-check.ts) wraps both realpathSync calls and fails closed on throw. — Failure scenario: a symlink-resolution failure or TOCTOU vanish of projectRoot/gitRoot during a hours-long audit throws out of guardDir — a raw stack out of the yargs handler, the failure shape read-json's docstring exists to prevent — instead of degrading to a fail-closed verdict.
Witness: not run — the trigger is a TOCTOU vanish; the asymmetry vs fallbackLandingSafe's wrapped identical computation is visible in the two files at this head.
Suggested fix: wrap the pair in try/catch and fail closed (probeFailed / unprotected), mirroring fallbackLandingSafe.
中文说明
R17-5（第 17 轮提出，第 18 轮复核仍然成立；本轮代码未变，仍然成立）：guardDir 以裸 relative(realpathSync(gitRoot), realpathSync(projectRoot)) 计算 toplevel 相对探测前缀——无 try/catch——而 fallbackLandingSafe（guard-check.ts）中相同的计算对两个 realpathSync 调用都做了包裹并在抛出时失败关闭。
失败场景：数小时审计期间 projectRoot/gitRoot 的符号链接解析失败或 TOCTOU 消失会从 guardDir 抛出——yargs handler 中的裸堆栈（read-json 文档字符串明确要避免的失败形态）——而不是降级为失败关闭的判定。
建议修复：try/catch 包裹并失败关闭（probeFailed / unprotected），与 fallbackLandingSafe 一致。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| reportFileName: string, | ||
| planTs?: string, | ||
| ): boolean { | ||
| const geometry = gitGeometry(fallbackRoot); |
There was a problem hiding this comment.
[Critical] R17-6（第 17 轮提出，第 18 轮复核仍然成立，本轮代码未变，仍然成立）: fallbackLandingSafe keys the landing verdict on gitGeometry(fallbackRoot) — a QWEN_HOME-settable fallback root inside a worktree is probed against THAT repo's ignore state, and the geometry-discovery corners (probeFailed vs outside-any-worktree vs root-undefined) decide whether relocated artifacts are certifiable; the round-17 filing names corners where the verdict mis-fires. — Failure scenario: a fallback root whose geometry probe degrades (transient git failure at a checkpoint) or whose worktree membership flips mid-run re-fires or mis-suppresses exit 5 for relocated artifacts — feeding directly the relocation-proof loop R18-8 names as having no terminal branch.
Witness: not run — mechanism carried from the round-17 filing and re-confirmed present in code at this head (gitGeometry(fallbackRoot) with the probeFailed/inWorktree branch arms).
Suggested fix: probe the landing shapes directly under the fallback root's own repo and treat geometry ambiguity as exposed (fail closed), rather than keying the verdict on geometry classification.
中文说明
R17-6（第 17 轮提出，第 18 轮复核仍然成立；本轮代码未变，仍然成立）：fallbackLandingSafe 以 gitGeometry(fallbackRoot) 作为落地判定依据——QWEN_HOME 可设置的 worktree 内回退根按该仓库的 ignore 状态探测，几何发现的角落（probeFailed、不在任何 worktree、root-undefined）决定重定位产物是否可认证；第 17 轮的 filing 指出了判定误触发的角落。
失败场景：回退根的几何探测降级（检查点时刻的瞬时 git 失败）或 worktree 成员关系中途翻转，会使重定位产物的 exit 5 误触发或误压制——直接喂给 R18-8 所指无终止分支的重定位证明循环。
建议修复：直接在回退根所属仓库下探测落地形状，并把几何歧义视为暴露（失败关闭），而不是以几何分类作为判定依据。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| if (top !== null) { | ||
| const headProbe = probeGit(rootAbs, ['rev-parse', 'HEAD'], GIT_TIMEOUT_MS); |
There was a problem hiding this comment.
[Critical] R17-16（第 17 轮提出，第 18 轮复核仍然成立，本轮代码未变，仍然成立）: the capture-time head probe's else-if chain records only ok → headSha / unborn → headUnborn; a TRANSIENT rev-parse HEAD failure on a born head records NOTHING (no headSha, no failure marker) — so driftCheck reports headUnknown at every subsequent checkpoint, which SKILL Step 6 treats as a stop — permanently, because the capture-time branch has no re-probe path and the extend/repair branch only re-runs on corruption/degradation. One transient git hiccup at Step 3 bricks the run's checkpoint discipline for its entire remainder. — Failure scenario: transient git failure (lock contention, a 5 s timeout kill) during capture → meta lacks headSha with no marker → every later checkpoint reports headUnknown → the skill stops at each one as drift; no arm of captureSidecar ever retries the head baseline.
Witness: not run — the trigger is a transient capture-time failure; the else-if chain (ok/unborn only, no failure arm) is visible at this head; the round-18 re-check confirmed the same at the prior head.
Suggested fix: record a marker on the failure arm (e.g. meta.vcsProbeFailed or a headProbeFailed flag) and let the extend/repair branch re-probe the head baseline the way it re-probes the other arms.
中文说明
R17-16（第 17 轮提出，第 18 轮复核仍然成立；本轮代码未变，仍然成立）：捕获时刻 head 探测的 else-if 链只记录 ok → headSha / unborn → headUnborn；对已出生 head 的瞬时 rev-parse HEAD 失败什么都不记录（无 headSha、无失败标记）——于是 driftCheck 在每个后续检查点报告 headUnknown，而 SKILL 第 6 步将其视为停止——并且是永久性的：捕获分支没有重探路径，extend/repair 分支只在损坏/降级时重跑。第 3 步的一次瞬时 git 抖动即令整个运行的检查点纪律失效。
失败场景：捕获期间瞬时 git 失败（锁争用、5 秒超时被杀）→ meta 缺少 headSha 且无标记 → 之后每个检查点都报 headUnknown → skill 每次都按 drift 停止；captureSidecar 任何分支都不会重试 head 基线。
建议修复：在失败分支记录标记（如 meta.vcsProbeFailed 或 headProbeFailed），并让 extend/repair 分支像重探其他臂一样重探 head 基线。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| const guard = checkLocalOnlyGuard( | ||
| projectRoot, | ||
| `${plan.artifacts.reportSlug}.md`, | ||
| ); |
There was a problem hiding this comment.
[Critical] R17-34（第 17 轮提出，第 18 轮复核仍然成立，本轮代码未变，仍然成立）: plan-files calls checkLocalOnlyGuard(projectRoot, reportFileName) with NO run-ts argument, while the guard-check checkpoint path recovers planTs (PLAN_TS_RE) and probes the ts-keyed shapes — so the plan-time verdict is computed without the very ts-keyed probe shapes the checkpoint path exists to defeat; a re-include keyed to the run ts is invisible at plan time. — Failure scenario: an ignore config re-including audit-<ts> shapes passes the plan-time guard (no ts probes), engages the in-repo landing, and only the later checkpoint sees the exposure — after the plan and early artifacts already landed where the module's own defense says they must not.
Witness: not run — the 2-argument call is visible at this head, vs guard-check.ts's planTs recovery and extraTs plumbing; the round-18 re-check confirmed the same at the prior head.
Suggested fix: plumb the run ts into the plan-time call (checkLocalOnlyGuard(projectRoot, reportFileName, planTs)) so plan-time and checkpoint probes cover the same shape set.
中文说明
R17-34（第 17 轮提出，第 18 轮复核仍然成立；本轮代码未变，仍然成立）：plan-files 调用 checkLocalOnlyGuard(projectRoot, reportFileName) 未传 run-ts 参数，而 guard-check 检查点路径会恢复 planTs（PLAN_TS_RE）并探测 ts 键控形状——因此 plan 时刻判定在缺少检查点路径本要防御的 ts 键控探测形状下计算；run ts 键控的 re-include 在 plan 时刻不可见。
失败场景：re-include audit-<ts> 形状的 ignore 配置通过 plan 时刻 guard（无 ts 探测），启用仓库内落地，直到后续检查点才暴露——而 plan 与早期产物已落在模块自身防御规定不可落之处。
建议修复：把 run ts 传入 plan 时刻调用，使 plan 时刻与检查点探测覆盖同一形状集合。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| (The Step 3 sidecar path is `.qwen/audits/audit-<ts>.sidecar` — or `<fallbackRoot>/audit-<ts>.sidecar` under a fallback landing.) | ||
|
|
||
| - The predicate is per file and keys on **content**, not git state: a file whose content is unchanged is not drifted, whatever HEAD did (a mid-run commit of the run-start dirty state fires the git-state arms and stops nothing). A `headUnknown`/`subtreeUnknown` marker means the git probe failed, so "not moved" is UNKNOWABLE — treat that checkpoint like drift in a walked file: stop and assemble the partial report. Drift in a file already walked **and carrying anchored findings** — walked subjects, test corpus, and registered callers alike — **stops the run**: assemble the partial report through Step 7's anchor resolution before writing it (findings whose anchors no longer resolve against the drifted content are dropped with the refusal recorded — never shipped bound to the changed code), and carry the drift, the phase, and a verification-not-completed mark in the header. Drift in any other file: mark it drifted/uncoverable in the walks record and continue. | ||
| - `guard-check` exits 5 when a module-derived directory became committable mid-run (directories already exposed at plan time do not re-fire ONLY while the plan file itself sits under a fallback root the guard itself verified safe — that is the relocation proof; if exit 5 fires despite a Step 1 relocation, the relocation did not land and must be redone): relocate the intermediates and the sidecar to the plan's `guard.fallbackRoot` immediately, and land the report beside them. Every subsequent command references the relocated paths — `<artifacts-dir>` IS `<fallbackRoot>` from this point (including Step 7's `check-anchors` and the final checkpoints). |
There was a problem hiding this comment.
[Critical] R18-8（第 18 轮提出，本轮被 R19-2 的探针进一步实证，仍然成立）: the relocation-proof bullet's only branch is 'if exit 5 fires despite a Step 1 relocation, the relocation did not land and must be redone' — but R19-2's probe this round demonstrates a reachable state where the relocation IS verified (plan file moved, credit granted) and exit 5 still fires at every checkpoint: sibling residue (audit-args-<ts>.json etc.) keeps .qwen/tmp exposed, and redoing the plan relocation cannot move residue it does not know about. The bullet has no terminal branch for that state — the orchestrator loops forever. — Failure scenario (probe-verified via R19-2): verified relocation + residue siblings → exit 5 at this and every later checkpoint; the skill's instruction ('must be redone') loops with no branch that terminates the run or escalates.
Witness: R19-2 probe — exit 5 re-fires with relocationVerified=true and .qwen/tmp unprotected.
Suggested fix: add the terminal branch — if exit 5 fires despite a verified relocation, enumerate the exposed shapes, move or delete the residue, and if the exposure still persists, stop the run with a partial report rather than redoing.
中文说明
R18-8（第 18 轮提出；本轮被 R19-2 的探针进一步实证，仍然成立）：重定位证明条目唯一的分支是'若第 1 步重定位后仍触发 exit 5，则重定位未落地、必须重做'——但本轮 R19-2 的探针展示了一个可达状态：重定位已验证（plan 文件已移动、信用已授予）而每个检查点仍触发 exit 5：同胞残留（audit-args-<ts>.json 等）使 .qwen/tmp 保持暴露，重做 plan 重定位无法移动它不知道的残留。该条目对此状态没有终止分支——orchestrator 将无限循环。
失败场景（经 R19-2 探针验证）：已验证的重定位 + 同胞残留 → 本检查点及之后每个检查点都 exit 5；skill 指令（'必须重做'）在没有任何终止/升级分支的情况下循环。
建议修复：补上终止分支——若在已验证重定位后仍触发 exit 5，列举暴露的形状、移动或删除残留；若暴露仍在，则以部分报告停止运行，而不是重做。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: Handoff — PR #8403 is not converging and needs a maintainer's decisionBlocker type: defer-to-human (growth brake engaged). This round made no code changes, no commits, and resolved no threads, as the not-converging contract requires. Branch The decision neededHow to land PR #8403 ( Measured state (workflow numbers)
Run log: https://github.com/QwenLM/qwen-code/actions/runs/32160576942 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Unresolved, please confirm:
- [Critical] R17-29 (anchors.ts parseReportFindings, round-18 blocker re-check): the second of two Location fields is silently dropped at parse level — probe at this head confirms locations keep only the first citation (the second is never scope-checked…
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; the same suites ran locally on Linux only, so the Windows leg that would exercise this PR's Windows-sensitive fixtures never ran.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; the same suites ran locally on Linux only.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": resolveAnchors' own binding logic ( whole / fragmentsLined /callerSet membership, lines 546-689) sits just outside my chunk and was read for context only, not a….
Not reviewed: reverse audit — stopped before round 2 by the review time budget.
Not reviewed: "agent verify (round 1)", "agent verify" — pointed at diff lines it never opened: it made tool calls, but none of them read the diff.
Deferred under the convergence posture (round 20, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/audit/lib/sidecar.ts:184 — [probe] diff.patch/untracked/callers artifact writes ungated (FIFO hang + symlink follow) — D20-11packages/cli/src/commands/audit/parse-args.ts:124 — [probe] Step-0 verdict write ungated — planted FIFO freezes the audit before any guard — D20-13packages/core/src/utils/git-ignore.ts:58 — [probe] GIT_CONFIG_COUNT/KEY/VALUE triplet unscrubbed — ambient core.excludesFile defeats the probe — D20-20packages/cli/src/commands/audit/lib/sidecar.ts:534 — [probe] over-cap no-baseline file lands in driftedFiles — permanent phantom drift, unrecoverable — D20-12packages/cli/src/commands/audit/guard-check.ts:235 — [probe] non-PLAN_TS_RE plan silently drops ts probes while keeping the suppression baseline — D20-25packages/cli/src/commands/audit/lib/sidecar.ts:370 — [probe] corruption fall-through catches write failures — baseline reset misdiagnosed as corruption — D20-8packages/cli/src/commands/audit/lib/anchors.ts:530 — [probe] tolerant matcher unbounded by haystack line count — 10MB caller stalls the gate ~1 min/location — D20-2packages/cli/src/commands/audit/lib/anchors.ts:146 — [probe] pair split lacks /i — uppercase AND grades a correct pair out-of-scope — D20-1packages/cli/src/commands/audit/lib/anchors.ts:126 — [probe] leading bold never peeled — bold-wrapped Location grades out-of-scope — D20-26packages/cli/src/commands/audit/lib/anchors.ts:135 — [probe] space after colon not peeled — file.ts: 12 grades out-of-scope — D20-27packages/cli/src/commands/audit/lib/files-plan.ts:949 — [probe] failed tracked-files probe collapses to ok — the one vacuous guard arm — D20-4packages/cli/src/commands/audit/lib/files-plan.ts:869 — [probe] symlink-to-ignored-file certified ok — wrong verdict, mid-run abort after consent — D20-34packages/cli/src/commands/audit/guard-check.test.ts:94 — [probe] exposedAtPlan no-worktree clause unpinned — clause-deletion mutant ships green — D20-23packages/cli/src/commands/audit/guard-check.test.ts:398 — [probe] fallbackLandingSafe planTs arm unreachable by any test — D20-24packages/cli/src/commands/audit/lib/sidecar.ts:179 — [probe] unborn-HEAD diff arm misses staged-but-worktree-clean content — D20-6packages/cli/src/commands/audit/lib/sidecar.test.ts:166 — [probe] extend retry of a name-only caller untested — one-token mutant ships green — D20-5packages/cli/src/commands/audit/lib/sidecar.test.ts:59 — [probe] noVcs drift-check never asserts headUnknown absence — re-arm mutant stops every outside-worktree audit — D20-10packages/cli/src/commands/audit/snapshot.ts:59 — [probe] snapshot subtreeHash/uncoverable fields unasserted — deletion ships green — D20-9packages/cli/src/commands/audit/plan-files.ts:173 — [probe] guard stderr WARNING branches + summary have no test oracle — D20-16packages/cli/src/commands/audit/plan-files.test.ts:55 — [probe] fixture initEnv scrub misses GIT_COMMON_DIR — parity comment false, init breaks under ambient value — D20-14- …and 14 more (see the run report)
[Critical] R18-3 (body-level blocker carried from round 16 via 18/19; re-verified this round, still stands): the merged design doc's Records ship criterion requires the calibration records under docs/design/assets/ to be checkable BEFORE the spec's rates and caps are coded; at this head docs/design/assets/ holds only 7 unrelated images while SUBJECT_TOKENS_PER_LINE=2_607 / TEST_TOKENS_PER_LINE=1_457 remain coded at files-plan.ts:63-64 and are consumed by the estimate math at :1147-1152. The maintainer decision escalated in comment 3761719334 (land redacted records and re-derive, or explicitly accept the author-reported rates) remains open. Witness: re-verified at this head (git ls-files docs/design/assets/ → 7 unrelated PNGs; constants coded and consumed; design doc Provenance :107-122 + Verification :1806-1811 state the criterion). / R18-3（自第 16 轮携带至今的 body 级阻断；本轮重新核实，仍然成立）：合入设计文档的 Records 交付准则要求 docs/design/assets/ 下的校准记录在规格费率与上限被编码之前可核查；当前 head 下 docs/design/assets/ 只有 7 张无关图片，而 SUBJECT_TOKENS_PER_LINE=2_607 / TEST_TOKENS_PER_LINE=1_457 仍编码于 files-plan.ts:63-64 并被 :1147-1152 的估算数学消费。评论 3761719334 中升级给维护者的决定（提交脱敏记录并重新推导，或明确接受作者报告的费率）仍未决。
中文说明
仅完成部分审查，审查缺口已披露。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查：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; the same suites ran locally on Linux only, so the Windows leg that would exercise this PR's Windows-sensitive fixtures never ran。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; the same suites ran locally on Linux only。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：resolveAnchors' own binding logic ( whole / fragmentsLined /callerSet membership, lines 546-689) sits just outside my chunk and was read for context only, not a…。
未审查：反向审计——评审时间预算不足，未能开始第 2 轮。
未审查："agent verify (round 1)"、"agent verify"——启动 prompt 为它指定了 diff 中的行，但它从未打开：有工具调用，却没有一次读取 diff。
收敛姿态下延后（第 20 轮，非阻断）——已记录，本轮不要求修改：共 34 条（原文未翻译，列表见上方英文部分）。
[Critical] R18-3 (body-level blocker carried from round 16 via 18/19; re-verified this round, still stands): the merged design doc's Records ship criterion requires the calibration records under docs/design/assets/ to be checkable BEFORE the spec's rates and caps are coded; at this head docs/design/assets/ holds only 7 unrelated images while SUBJECT_TOKENS_PER_LINE=2_607 / TEST_TOKENS_PER_LINE=1_457 remain coded at files-plan.ts:63-64 and are consumed by the estimate math at :1147-1152. The maintainer decision escalated in comment 3761719334 (land redacted records and re-derive, or explicitly accept the author-reported rates) remains open. Witness: re-verified at this head (git ls-files docs/design/assets/ → 7 unrelated PNGs; constants coded and consumed; design doc Provenance :107-122 + Verification :1806-1811 state the criterion). / R18-3（自第 16 轮携带至今的 body 级阻断；本轮重新核实，仍然成立）：合入设计文档的 Records 交付准则要求 docs/design/assets/ 下的校准记录在规格费率与上限被编码之前可核查；当前 head 下 docs/design/assets/ 只有 7 张无关图片，而 SUBJECT_TOKENS_PER_LINE=2_607 / TEST_TOKENS_PER_LINE=1_457 仍编码于 files-plan.ts:63-64 并被 :1147-1152 的估算数学消费。评论 3761719334 中升级给维护者的决定（提交脱敏记录并重新推导，或明确接受作者报告的费率）仍未决。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| ); | ||
| } | ||
| const reportText = reportContent.toString('utf8'); | ||
| const findings = parseReportFindings(reportText); |
There was a problem hiding this comment.
[Critical] R20-3: check-anchors grades every section-final and report-final finding of the skill's own mandated report template unresolved: parseReportFindings ends anchor collection on - Failure scenario: only when the next content line is field-shaped or a ### [sev] header — but the SKILL Step 7 template ends every block with - Failure scenario: and separates sections with plain ## Name headings invisible to every header net, so the trailing field + heading + prose are absorbed into the needle. — Failure scenario: any template-conformant non-empty report (its last-per-section and last-per-report blocks always break) grades those blocks unresolved (matchCount 0) → exit 4 → the orchestrator downgrades/refuses correctly-anchored findings and records a false refusal. The suite stays green because every canonical fixture carries an - Issue: field after - Anchor: — a shape the template does not have.
中文说明
[Critical] R20-3：check-anchors 会把 skill 自身规定的报告模板中每个"分节末尾/报告末尾"的 finding 判为 unresolved：parseReportFindings 仅当下一内容行是字段形或 ### [sev] 标题时，才在 - Failure scenario: 处结束 anchor 收集——但 SKILL 第 7 步模板以 - Failure scenario: 结束每个块，并用对任何标题网都不可见的普通 ## Name 标题分隔小节，于是尾部字段+标题+正文都被吸入 needle。失败场景：任何符合模板的非空报告（其每节最后一个与全报告最后一个块必然中招）会把这些块判为 unresolved（matchCount 0）→ exit 4 → orchestrator 降级/拒绝锚点正确的 findings 并记录虚假拒绝。测试套件保持绿色，因为每个规范 fixture 都在 - Anchor: 后带有 - Issue: 字段——模板并没有这个形状。
— qwen3.8-max via Qwen Code /review (v0.21.13)
Witness: probe — template-shaped report: first finding anchor "…\n- Failure scenario: y …\n\n## Suggestion", verdict unresolved matchCount 0; EOF block likewise. FLIP (add the fixture-only - Issue: field): both resolved matchCount 1.
Suggested fix: confirm the terminator on a plain markdown section heading as well, and/or end template blocks with a recognized field line; add a regression fixture shaped exactly like the template.
— qwen3.8-max via Qwen Code /review (v0.21.13)
| if (current && inAnchor) { | ||
| if (line.startsWith('```')) { |
There was a problem hiding this comment.
[Critical] R20-5: the anchor fence toggle treats ANY line starting with as a boundary, ignoring the opening fence's marker length and info strings — CommonMark closing fences must be at least as long as the opener and carry no info string. Quoted prior-round content inside an anchor (the exact shape the fence shield exists for) de-shields mid-collection, and the `pendingLocations` commit path re-binds the finding's Location onto quoted field lines: the gate certifies a binding the author never made. — Failure scenario: `- Location: real.ts:5` with an anchor quoting prior-round content that contains a (or ``````ts) line plus quoted - Location: old.ts:10 / `- Anchor: legacy anchor` fields — the inner fence flips `inFence`, the quoted pair parses live, the restart condition commits `locations := ['old.ts']`; with 'legacy anchor' present once in old.ts the verdict is resolved/old.ts where the author bound real.ts — a false certification shipped through the write gate.
中文说明
[Critical] R20-5：anchor 围栏开关把任何以 开头的行都当作边界，忽略起始围栏的标记长度与信息串——CommonMark 要求闭合围栏至少与起始围栏等长且不得携带信息串。anchor 内引用的上一轮内容（围栏屏蔽机制正是为此而设）会在收集中途解除屏蔽，`pendingLocations` 提交路径随后把 finding 的 Location 重新绑定到被引用的字段行上：门禁认证了一个作者从未做出的绑定。失败场景：`- Location: real.ts:5` 的 anchor 引用了包含（或 ``````ts）行的上一轮内容，其中还有被引用的 - Location: old.ts:10 / `- Anchor: legacy anchor` 字段——内部围栏翻转 `inFence`，被引用的字段对被当作活动字段解析，重启条件提交 `locations := ['old.ts']`；只要 'legacy anchor' 在 old.ts 中出现一次，判定即为 resolved/old.ts，而作者绑定的是 real.ts——虚假认证经写入门禁放行。
— qwen3.8-max via Qwen Code /review (v0.21.13)
Witness: probe — locations ['old.ts'], verdict resolved matchCount 1 (both the 4-backtick and ```ts info-string variants). FLIP (CommonMark closing rule): locations ['real.ts'], verdict unresolved.
Suggested fix: track the opening fence's backtick-run length (from both the field-line latch and continuation openers) and close only on a line whose leading run is >= that length with no info string.
— qwen3.8-max via Qwen Code /review (v0.21.13)
| const env: NodeJS.ProcessEnv = { ...process.env }; | ||
| delete env['GIT_DIR']; |
There was a problem hiding this comment.
[Critical] R20-7: the env scrub removes the five repo-selecting vars but none of git's pathspec-interpreter vars (GIT_GLOB_PATHSPECS, GIT_NOGLOB_PATHSPECS, GIT_LITERAL_PATHSPECS, GIT_ICASE_PATHSPECS) — check-ignore rejects env-induced pathspec magic with a fatal, so an ambient value of ANY of them makes every probe answer "not ignored" for paths that ARE ignored. — Failure scenario: a user/CI shell exporting GIT_NOGLOB_PATHSPECS=1 → every isGitIgnored call dies exit 128 → catch → false. The audit guard grades every artifact shape unprotected and the fallback landing never certifies (exit 5 forever on relocated runs); review's Test Plan rules a gitignored build-output path missing → a false contradicted verdict on a correct plan; team memory emits spurious/missed shareability warnings. GIT_LITERAL_PATHSPECS=1 is not a fix — 'literal' magic fatals too.
中文说明
[Critical] R20-7：环境剥离移除了五个仓库选择变量，但没有移除 git 的 pathspec 解释器变量（GIT_GLOB_PATHSPECS、GIT_NOGLOB_PATHSPECS、GIT_LITERAL_PATHSPECS、GIT_ICASE_PATHSPECS）——check-ignore 会对环境引入的 pathspec magic 以 fatal 拒绝，因此其中任何一个环境值都会让每次探测对"确实被忽略"的路径回答"未忽略"。失败场景：用户/CI shell 导出 GIT_NOGLOB_PATHSPECS=1 → 每次 isGitIgnored 调用以 exit 128 失败 → catch → false。审计 guard 把所有产物形状判为 unprotected、回退落地永远无法认证（重定位后的运行永远 exit 5）；review 的 Test Plan 把被 gitignore 的构建输出路径判为 missing → 对正确计划给出虚假的 contradicted；team memory 发出虚假/漏报的可共享性警告。GIT_LITERAL_PATHSPECS=1 也不是解法——'literal' magic 同样 fatal。
— qwen3.8-max via Qwen Code /review (v0.21.13)
Witness: probe (real isGitIgnored, git 2.43) — baseline-ignored: true; under each of the four vars: false. FLIP (four deletes added): true ×4.
Suggested fix: add the four deletes to the scrub, with pinned arms in git-ignore.test.ts (set each to 1, expect the fixture verdict to still hold).
— qwen3.8-max via Qwen Code /review (v0.21.13)
| }); | ||
|
|
||
| it('resolves against registered deep-read callers outside the path', () => { | ||
| const caller = join(dir, '..', `caller-${Date.now()}.ts`); |
There was a problem hiding this comment.
[Critical] R19-1: 第 17 轮提出（R17-3），第 18、19 轮复核仍然成立，本轮代码未变，仍然成立。This test feeds a platform-native path (join() — backslashed on Windows) into locations AND registeredCallers, but resolveAnchors slash-normalizes only the registeredCallers side into callerSet — the locations value keeps backslashes, membership fails, and the resolved assertion flips to out-of-scope on the Windows merge-queue leg, which ci.yml runs and no skipIf guards. — Failure scenario: test_windows (windows-latest, merge_group-only) runs the full suite → normalized callerSet ('/') never matches the backslashed locations entry → out-of-scope → red on a required merge gate, invisible in PR CI. The production citation path is unaffected (normalizeLocation slash-normalizes) — the defect is exactly the shipped test breaking its own merge gate.
中文说明
[Critical] R19-1：第 17 轮提出（R17-3），第 18、19 轮复核仍然成立，本轮代码未变，仍然成立。该测试把平台原生路径（join()——Windows 上为反斜杠）同时传入 locations 与 registeredCallers，但 resolveAnchors 只对 registeredCallers 一侧做斜杠归一化进入 callerSet——locations 值保留反斜杠，成员判定失败，resolved 断言在 ci.yml 运行且无 skipIf 防护的 Windows 合并队列分支上翻转为 out-of-scope。失败场景：test_windows（windows-latest，仅 merge_group 触发）运行全量测试 → 归一化后的 callerSet（'/'）永远匹配不上反斜杠的 locations 条目 → out-of-scope → 必需合并门禁红，而 PR CI 看不到。生产引用路径不受影响（normalizeLocation 会归一化斜杠）——缺陷恰是随 PR 交付的测试打破了自己的合并门禁。
— qwen3.8-max via Qwen Code /review (v0.21.13)
Witness: probe (real resolveAnchors) — slash-path control: resolved; Windows-shape arm (backslashed path in both locations and registeredCallers, as shipped): out-of-scope. Linux baseline: anchors.test.ts 72/72 green.
Suggested fix: normalize the locations side the same way (backslash → slash before membership), or build the fixture's expected path from the normalized form.
— qwen3.8-max via Qwen Code /review (v0.21.13)
| * and a linked worktree alike, and does not dirty the tracked .gitignore). | ||
| * Returns the exclude file path written. */ | ||
| export function applyExcludeRemedy(projectRoot: string): string { | ||
| const commonDir = git(projectRoot, ['rev-parse', '--git-common-dir']); |
There was a problem hiding this comment.
[Critical] R19-3: gitEnv() strips the ambient env vars that re-home probes to a foreign repository — the GIT_COMMON_DIR scrub comment a few lines above names this exact harm — but an in-tree regular-file .git containing gitdir: /path/to/victim/.git re-homes rev-parse --git-common-dir identically, with NO ambient env: gitGeometry reports inWorktree, applyExcludeRemedy resolves the exclude file into the VICTIM repository and appends the audit's ignore rules there; the re-probe then flips to ignored and certifies ok off foreign state. — Failure scenario: a planted .git gitfile in the audited corpus (a downloaded/vendored tree — the module's stated target class; relative gitdir: targets reach sibling/parent repos) → the remedy write lands at <victim>/.git/info/exclude and the guard after the remedy is ok/ok where it was unprotected/unprotected before — the whole verdict chain computed against a repository the user never asked to touch, planted by one file in the audited content.
中文说明
[Critical] R19-3：gitEnv() 剥离了会把探针重定向到外部仓库的环境变量——上方 GIT_COMMON_DIR 剥离注释明确指出了这种危害——但树内的常规文件 .git（内容为 gitdir: /path/to/victim/.git）在完全无环境变量的情况下同样重定向 rev-parse --git-common-dir：gitGeometry 报告 inWorktree，applyExcludeRemedy 把排除规则写入受害仓库的 info/exclude；重新探测随后翻转为 ignored 并据外部状态认证 ok。失败场景：在被审计语料中植入 .git gitfile（下载/内嵌树——模块明确的目标类别；相对 gitdir: 目标可触及兄弟/父仓库）→ 补救写入落到 <victim>/.git/info/exclude，补救后 guard 判定由 unprotected/unprotected 变为 ok/ok——整条判定链在用户从未要求接触的仓库上计算，仅凭被审计内容中的一个文件即可植入。
— qwen3.8-max via Qwen Code /review (v0.21.13)
Witness: probe (git 2.43, zero ambient git env) — remedy wrote /.git/info/exclude (lands in VICTIM repo: true); guard AFTER remedy: ok/ok; check-ignore in corpus: IGNORED.
Suggested fix: treat a non-directory .git at/above the audit root as untrusted indirection — refuse the remedy (or report probeFailed) when the discovered worktree's .git is a regular gitfile, or verify the resolved common-dir sits under the discovered toplevel's own .git before writing.
— qwen3.8-max via Qwen Code /review (v0.21.13)
| const real = join(dir, 'real-root'); | ||
| mkdirSync(real, { recursive: true }); | ||
| const link = join(dir, 'link-root'); | ||
| symlinkSync(real, link); |
There was a problem hiding this comment.
[Critical] R19-5 (location 1 of 2 — :1017 and :326): 第 19 轮提出，本轮代码未变，仍然成立；本轮反向审计发现第二个同款 fixture（:325 'records symlinks and never follows them'）。This test creates a directory symlink via untyped symlinkSync and is NOT skipIf(win32)-guarded, while every other symlink fixture in this file is. On Windows, untyped symlinkSync needs elevation/Developer Mode, and Node 22's default type is 'file' even for a directory target — resolveAuditRoot's statSync/realpathSync cannot traverse it. — Failure scenario: the merge_group-only test_windows job runs npm run test:ci including this file — on a runner without the privilege the fixture throws EPERM; with it, the assertion fails — red either branch on a required merge gate, invisible in PR CI. A fix scoped to one site leaves the other red.
中文说明
[Critical] R19-5（共 2 处——:1017 与 :326）：第 19 轮提出，本轮代码未变，仍然成立；本轮反向审计发现第二个同款 fixture（:325 'records symlinks and never follows them'）。该测试用无类型 symlinkSync 创建目录符号链接且未加 skipIf(win32)，而同文件其余符号链接 fixture 均有防护。Windows 上无类型 symlinkSync 需要提权/开发者模式，且 Node 22 对目录目标默认类型仍为 'file'——resolveAuditRoot 的 statSync/realpathSync 无法遍历。失败场景：仅 merge_group 触发的 test_windows 作业运行含本文件的 npm run test:ci——无权限的 runner 上 fixture 抛 EPERM，有权限的 runner 上断言失败——合并队列门禁两个分支都红，而 PR CI 看不到。只修一处会留下另一处红色。
— qwen3.8-max via Qwen Code /review (v0.21.13)
Witness: not run — Windows-only trigger, unavailable on this Linux host; verdict rests on a sweep (exactly these two unguarded occurrences; all sibling fixtures skipIf'd), ci.yml's merge_group gating, and the repo's own junction/skipIf prior art. Linux baseline for this file: 100/100 green.
Suggested fix: it.skipIf(process.platform === 'win32') on both tests (or create the fixtures with 'junction' type on win32).
— qwen3.8-max via Qwen Code /review (v0.21.13)
| }); | ||
|
|
||
| it('records symlinks and never follows them', () => { | ||
| symlinkSync(join(dir, 'src', 'a.ts'), join(dir, 'src', 'link.ts')); |
There was a problem hiding this comment.
[Critical] R19-5 (location 2 of 2 — see the :1017 comment for the full finding): second unguarded symlink fixture — records symlinks and never follows them calls symlinkSync without skipIf(win32); same Windows merge-gate failure class, distinct test.
中文说明
[Critical] R19-5（第 2 处，完整描述见 :1017 的评论）：第二个未加防护的符号链接 fixture——records symlinks and never follows them 未加 skipIf(win32) 即调用 symlinkSync；同属 Windows 合并门禁失败类别，但为不同测试。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| GIT_TIMEOUT_MS, | ||
| ); | ||
| if (headProbe.ok) { | ||
| existing.meta.headSha = headProbe.out.trim(); |
There was a problem hiding this comment.
[Critical] R19-6 (location 2 of 2 — see the :391 comment for the full finding): the extend re-run re-writes meta.headSha only inside if (probeFailed) — a capture whose toplevel probe succeeded never sets vcsProbeFailed, so a missing headSha is unrecoverable; every checkpoint reports headUnknown and the documented mid-run retry ("this re-run is the only command that can retry it") never re-probes HEAD for this class.
中文说明
[Critical] R19-6（第 2 处，完整描述见 :391 的评论）：extend 重跑只在 if (probeFailed) 内重写 meta.headSha——toplevel 探测成功的捕获从不设置 vcsProbeFailed，因此缺失的 headSha 无法恢复；每个检查点都报 headUnknown，而文档化的中途重试（"该重跑是唯一可以重试它的命令"）对此类别从不重探 HEAD。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
🤖 Could not produce a passing fix for this feedback (round 25/100). This item now needs a human; the loop stays engaged and still picks up new feedback and base conflicts, but will not retry this item on its own. What I found before stopping: PR #8403 (
|
|
🤖 Could not produce a passing fix for this feedback (round 26/100). This item now needs a human; the loop stays engaged and still picks up new feedback and base conflicts, but will not retry this item on its own. What I found before stopping: Needs a maintainer's decision — PR #8403 is not convergingDecision neededWhether to split this PR, redesign the approach, or accept the current state and defer the remaining findings tail. The deterministic growth brake has engaged and this round applied no code changes, per its rules. Why the loop cannot converge on its own
What the PR contains (for scoping the options)
Run log: https://github.com/QwenLM/qwen-code/actions/runs/32198906690 🧠 Handled by Qwen Code · model/模型 |
…100 (QwenLM#9390) * fix(autofix): paginate review threads instead of reaching the oldest 100 `resolve_and_reply_threads` fetched `reviewThreads(first:100)` with no pagination. GitHub returns review threads in ASCENDING creation order, so a single page is the OLDEST hundred — on a long-running PR, precisely not the threads the current round is answering. Both blocks downstream map an inline-comment id to its thread. A thread past the page is absent from `THREADS_JSON`, so an implemented Critical is never resolved and reads as still open, and a declined finding's reply is answered by silence. Those are the two outcomes the function exists to prevent. Live: 8 of the 22 open takeover PRs exceed the cap. QwenLM#8403 carries 1256 threads, so one page reached 8% of them — and all 1256 are unresolved. The code already detected this: it requested `pageInfo{hasNextPage}` and emitted a `::warning::` when true. It just never fetched the next page. Use `gh api graphql --paginate`, which is built for exactly this shape, and slurp its node stream into the flat array both blocks already expect. On QwenLM#8403 that is 13 requests in ~10s. A partial fetch is USED rather than discarded: losing twelve good pages to a rate limit on the thirteenth would resolve nothing at all, so the failure is announced and the threads in hand still map. One residual stays open and is now announced rather than implied: a thread carrying more than 100 comments still truncates, so a comment past that page is unmapped and each block falls back to the id as given. No thread in the live pool comes close. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(autofix): keep the pagination failure's reason, and pin both warnings' absence Round 1's three Suggestions, all on the partial-fetch path this PR adds. R1-1: `2> /dev/null` on the paginated fetch discarded gh's stderr — the only text saying WHY pagination stopped. The warning announced THAT it stopped, so the oncall could not separate a transient rate limit (back off) from an expired PAT (rotate) or a network failure without re-running the ~13-request query by hand. Captured to `${WORKDIR}/threads-fetch.err` with the pattern already used elsewhere in this workflow, and its tail folded into the warning. R1-2: the outer thread pagination silently depends on the inner `comments` pageInfo NOT asking for `endCursor` — gh's paginator adopts the first pageInfo carrying both fields. The `Residual:` note actively invited a maintainer to close that residual by adding it, which would hijack the thread-page cursor and stop after page one at exit 0 with no warning, silently restoring the oldest-hundred bug. Documented as load-bearing, in the comment block above the fetch rather than inside the query literal — a `#` line there is transmitted. R1-3: both new warnings were asserted only in the positive, so a mutation making either unconditional shipped green. Added the clean-run absence assertions this file's own convention calls for (321 `not.toContain` uses), and the gh stub now writes a reason to stderr on failure so the folded-in text is assertable. Verified: qwen-autofix-workflow 178 passed. Mutation-checked — restoring `2> /dev/null` and making the pagination warning unconditional each fail a test. The one remaining failure (`behaviorally replays the stale-duplicate revalidation`, 5s timeout) is identical with these changes stashed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(autofix): keep gh's error body out of the slurped review threads Round 2 of the review on QwenLM#9390 found the paginated review-thread fetch poisons its own output on a partial page, and asked for two clarifications around it. R2-C (Critical) — on a failing page gh skips `--jq` and appends that page's raw response body (a rate-limit message, or a GraphQL error envelope) to stdout after the good nodes. The unfiltered `jq -s '.'` slurped it as an extra element, and both consumers below iterate `.comments.nodes[]` over every element, so the first one exited 5. This step runs under errexit, so that aborted 'Push and report' AFTER a good push had landed — the report and the markers were skipped and the job failed. That contradicts the two invariants the block documents: a resolve failure must never fail a good push, and a partial fetch is used rather than discarded. The slurp now keeps only thread-shaped documents. R2-1 — the comment block warned against adding `endCursor` to the inner `comments` pageInfo, but the outer pageInfo's field ORDER is load-bearing for the same reason: gh's cursor scanner carries its flags across pageInfo objects and breaks at the first one yielding both fields, so alphabetizing to `pageInfo{endCursor hasNextPage}` stops after page one just as silently. Said so at the query, and at the test pin that goes red on a reorder, so the pin is understood rather than bumped. R2-2 — the stderr fold dropped the `tr '\r\n' ' '` that its ten sibling sites apply. Actions parses workflow commands line by line and gh's secondary-rate-limit stderr spans two lines, so the annotation kept only the first — cutting off the words that separate a back-off from a credential rotation. Verification: `scripts/tests/qwen-autofix-workflow.test.js` 179/179; yaml parses; eslint and prettier clean. Mutation-checked all three: reverting the slurp filter fails the resolve arm with exit 5 (expected 5 to be 0), reordering the outer pageInfo fails the field-order pin, and dropping the `tr` fails the folded-reason arm on the second stderr line. * docs(autofix): correct the field-order comment's mechanism (QwenLM#9390 R3-1) The comment explaining why `pageInfo{hasNextPage endCursor}` order is load-bearing described the silent stop as happening with the carried `hasNextPage` "already true" from the last inner page. That cannot produce the symptom: gh's `findEndCursor` returns a cursor only `if hasNextPage`, so a carried true would keep the walk going. The real mechanism is the opposite one. The scanner carries its flags across `pageInfo` objects and breaks at the first point both have been seen; under `pageInfo{endCursor hasNextPage}` that break lands on the outer `endCursor` while `hasNextPage` still holds the last INNER page's value — almost always false, since thread comment pages rarely truncate — and the outer page's own `hasNextPage` is never read. gh returns no cursor and the walk stops after page one, exit 0 and silent. Reworded in both places the clause was copied to: the workflow comment and the field-order pin's comment in the test. No assertion, no shell, and no query text changes; `pageInfo{hasNextPage endCursor}` and the test that pins it are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(autofix): keep gh's thread-fetch stderr off a predictable WORKDIR path R4-1 (Critical, QwenLM#9390): the review-thread pagination wrote gh's stderr to `${WORKDIR}/threads-fetch.err` and read it back with `tail -c 300`, both without a file-type guard. WORKDIR (`/tmp/autofix-review-<pr>`) is bind-mounted read-write into the agent docker sandbox, and the round that just finished ran branch code inside that sandbox, so the name is attacker-chosen by the time this step runs. A planted FIFO makes bash block on the O_WRONLY open before gh even execs, and the only reader is the `tail` that runs strictly after gh returns — so the step hangs to the job timeout with the push already landed, losing the report comment and the round markers. That breaks the invariant this block states for itself: a resolve failure must never fail a good push. A planted symlink instead turns the redirect into a truncate/write against the link target and the tail into a 300-byte arbitrary-file read folded into a public `::warning::`. Route the stderr through a fresh `mktemp` regular file instead, matching the `gh api user` checks elsewhere in this workflow, and remove it afterwards. The diagnostic is unchanged: the warning still carries gh's own reason, which is the only text separating a transient rate limit from an expired PAT. Test: plant a symlink at the old path, run the block through a failing fetch, and assert the target's bytes are neither overwritten nor folded into the annotation; plus assert the named path is not created at all. Mutation- verified — restoring the `${WORKDIR}` redirect turns the canary assertion red (`expected 'threads-fetch stub failure' to be 'CANARY-MUST-SURVIVE'`). The FIFO half cannot be written as a plain assertion because the pre-fix code hangs rather than fails; the same "named path is never opened" property defuses it. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # packages/cli/src/commands/review.ts # packages/cli/src/commands/review/compose-review.ts # packages/cli/src/utils/paths.test.ts # packages/cli/src/utils/paths.ts # packages/core/src/utils/git-ignore.test.ts # packages/core/src/utils/git-ignore.ts # scripts/tests/review-source-digest.test.ts
…lings getAuditFallbackDir hashed the raw project root, so the same directory spelled two ways — macOS /var vs the symlink-resolved /private/var that process.cwd() reports after chdir — produced two different fallback roots. The guard-check relocation credit compares the relocated plan against the live fallback root and failed closed (exit 5 at every checkpoint) on exactly the relocation it is meant to certify. Resolve the root with realpathSync before hashing (falling back to the raw path when unresolvable); pinned by a symlink-stability regression test.
…t/legacy-code-audit
qwen-code-ci-bot
left a comment
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; the same suites ran locally on Linux only, so the Windows leg that would exercise this PR's Windows-sensitive fixtures never ran.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; the same suites ran locally on Linux only.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 21, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/audit/agent-prompt.ts:84 — [probe] yargs .check() arms untested — removing them ships green and silently declines probespackages/cli/src/commands/audit/guard-check.test.ts:116 — [probe] GIT_CONFIG_GLOBAL pin never reaches the check-ignore probes — hostile $HOME fails 14/21 testspackages/cli/src/commands/audit/lib/anchors.test.ts:1117 — [probe] comment misattributes the counting arm (window matcher's first-indent base, not the raw loop)packages/cli/src/commands/audit/lib/anchors.ts:451 — [probe] followRuleOk '#' arm fuses JS private fields / URL fragments — truncated quotes certified resolvedpackages/cli/src/commands/audit/lib/files-plan.test.ts:396 — [review] secret fixture list misses clauses for .netrc/credentials.json/id_ed448/id_xmss/.key/.p12/.pfx/.keystorepackages/cli/src/commands/audit/lib/files-plan.test.ts:470 — [review] over-cap fixtures hardcode 10 MiB literal instead of importing AUDIT_READ_MAX_BYTESpackages/cli/src/commands/audit/lib/files-plan.ts:466 — [probe] measureFile reads to a live EOF with no size bound — appender stalls enumeration (12499ms vs 121ms)packages/cli/src/commands/audit/lib/files-plan.ts:1085 — [probe] exclude remedy anchors logical-path rules while guardDir probes the physical path — remedy can never flip the guardpackages/cli/src/commands/audit/lib/read-json.ts:59 — [probe] readPlanFile element-shape traversal guard untested — mutation ships greenpackages/cli/src/commands/audit/lib/sidecar.ts:192 — [review] captureUntrackedArm docstring says EVERY-copy-fails but code degrades on ANY failed copypackages/cli/src/config/config.ts:1130 — [probe] 'audit' fast-exit membership untested — removing it ships green and boots the interactive promptpackages/cli/src/config/config.ts:1159 — [probe] subcommand exit-flush await + EPIPE swallows untested — >64KiB payloads truncate at pipe bufferpackages/core/src/config/storage.ts:351 — [review] dead defensive branch exists only to tolerate the test file's un-implemented realpathSync mockpackages/core/src/skills/bundled/audit/SKILL.md:193 — [review] role ID '6a' never glossed — no durable artifact maps it after the plan file is deleted
[Critical] R18-3 (body-level blocker carried from round 16 via 18/19/20; re-verified this round, still stands): the merged design doc's Records ship criterion requires the calibration records under docs/design/assets/ to be checkable BEFORE the spec's rates and caps are coded; at this head docs/design/assets/ holds only 7 unrelated images while SUBJECT_TOKENS_PER_LINE=2_607 / TEST_TOKENS_PER_LINE=1_457 remain coded at files-plan.ts:63-64 and are consumed by the estimate math at :1147-1152. The maintainer decision escalated in comment 3761719334 (land redacted records and re-derive, or explicitly accept the author-reported rates) remains open. Witness: re-verified at this head (git ls-files docs/design/assets/ → 7 unrelated PNGs; constants coded and consumed; design doc Provenance :107-122 + Verification :1806-1811 state the criterion). / R18-3（自第 16 轮携带至今的 body 级阻断；本轮重新核实，仍然成立）：合入设计文档的 Records 交付准则要求 docs/design/assets/ 下的校准记录在规格费率与上限被编码之前可核查；当前 head 下 docs/design/assets/ 只有 7 张无关图片，而 SUBJECT_TOKENS_PER_LINE=2_607 / TEST_TOKENS_PER_LINE=1_457 仍编码于 files-plan.ts:63-64 并被 :1147-1152 的估算数学消费。评论 3761719334 中升级给维护者的决定（提交脱敏记录并重新推导，或明确接受作者报告的费率）仍未决。
中文说明
仅完成部分审查，审查缺口已披露。
未审查：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; the same suites ran locally on Linux only, so the Windows leg that would exercise this PR's Windows-sensitive fixtures never ran。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; the same suites ran locally on Linux only。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 21 轮，非阻断）——已记录，本轮不要求修改：共 14 条（原文未翻译，列表见上方英文部分）。
[Critical] R18-3 (body-level blocker carried from round 16 via 18/19/20; re-verified this round, still stands): the merged design doc's Records ship criterion requires the calibration records under docs/design/assets/ to be checkable BEFORE the spec's rates and caps are coded; at this head docs/design/assets/ holds only 7 unrelated images while SUBJECT_TOKENS_PER_LINE=2_607 / TEST_TOKENS_PER_LINE=1_457 remain coded at files-plan.ts:63-64 and are consumed by the estimate math at :1147-1152. The maintainer decision escalated in comment 3761719334 (land redacted records and re-derive, or explicitly accept the author-reported rates) remains open. Witness: re-verified at this head (git ls-files docs/design/assets/ → 7 unrelated PNGs; constants coded and consumed; design doc Provenance :107-122 + Verification :1806-1811 state the criterion). / R18-3（自第 16 轮携带至今的 body 级阻断；本轮重新核实，仍然成立）：合入设计文档的 Records 交付准则要求 docs/design/assets/ 下的校准记录在规格费率与上限被编码之前可核查；当前 head 下 docs/design/assets/ 只有 7 张无关图片，而 SUBJECT_TOKENS_PER_LINE=2_607 / TEST_TOKENS_PER_LINE=1_457 仍编码于 files-plan.ts:63-64 并被 :1147-1152 的估算数学消费。评论 3761719334 中升级给维护者的决定（提交脱敏记录并重新推导，或明确接受作者报告的费率）仍未决。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| artifacts: { | ||
| reportSlug: safeTarget(targetPath), | ||
| }, |
There was a problem hiding this comment.
[Critical] R21-1: slug space mismatch — this plan produces artifacts.reportSlug via safeTarget(targetPath), but the two consumers of that slug accept a narrower space than safeTarget emits. The merge with origin/main reverted safeTarget's leading-dash strip (packages/cli/src/utils/paths.ts now ends with flat.replace(/^[._]+/, '') — the branch-side fix spelling /^[._-]+/ was lost in the merge), so slugs can again start with -; safeTarget also never truncates. Three demonstrated entrances: (1) isSafeReportSlug (guard-check.ts:137-145, whose doc comment claims "The safeTarget output space") rejects dash-leading and >200-char slugs, so slugSafe is false and a fully verified relocation loses credit — guardTripped re-fires exit 5 at every drift checkpoint and SKILL.md re-drives the emergency relocation in an unrecoverable loop; (2) SKILL.md's space-form call guard-check --report-slug <slug> dies in yargs before any handler for a dash-leading slug — the only runtime re-check keeping module-quoting artifacts out of version control is silently disabled for the whole run on exactly the targets the guard exists for; (3) slugs over ~234 chars push the write-time report filename <ts>-<slug>.md past the 255-byte NAME_MAX. Witness (probe at this head): safeTarget('./-foo') → '-foo'; guard-check with dash slug -foo and a fully verified relocation → exit 5, safe slug foo → exit 0; --report-slug -foo → yargs threw Unknown arguments: f, o. Suggested fix: restore the lost leading-dash strip in safeTarget (flat.replace(/^[._-]+/, '') || 'target'), and/or switch SKILL.md's guard-check invocation to the equals form --report-slug=<slug>; add a guard-check test with a plan whose artifacts.reportSlug is -foo / >200 chars crediting a verified relocation.
| artifacts: { | |
| reportSlug: safeTarget(targetPath), | |
| }, | |
| artifacts: { | |
| reportSlug: safeTarget(targetPath).replace(/^[-]+/, '').slice(0, 200) || 'target', | |
| }, |
中文说明
[Critical] R21-1：slug 取值空间不匹配——plan 通过 safeTarget(targetPath) 生成 artifacts.reportSlug，但该 slug 的两个消费方所接受的空间比 safeTarget 实际输出的更窄。与 origin/main 的合并回退了 safeTarget 的前导短横线剥离（packages/cli/src/utils/paths.ts 现在是 flat.replace(/^[._]+/, '')——分支侧 /^[._-]+/ 的修复写法在合并中丢失），因此 slug 又可能以 - 开头；且 safeTarget 从不截断长度。三个已演示的入口：(1) isSafeReportSlug（guard-check.ts:137-145，其文档注释声称"The safeTarget output space"）拒绝短横线开头及超过 200 字符的 slug，于是即便搬迁已完全验证，slugSafe 为 false 也会失去搬迁凭证——guardTripped 在每个 drift 检查点重新触发 exit 5，SKILL.md 陷入不可恢复的紧急搬迁重做循环；(2) SKILL.md 的空格形式调用 guard-check --report-slug <slug> 对短横线开头的 slug 在任何 handler 运行前就死于 yargs 解析——防止模块引用产物进入版本控制的唯一运行时复查在整个运行期间被静默禁用；(3) 超过约 234 字符的 slug 会使写入时的报告文件名 <ts>-<slug>.md 超过 255 字节 NAME_MAX。证据（本 head 探针）：safeTarget('./-foo') → '-foo'；短横线 slug -foo 且搬迁已完全验证 → exit 5，安全 slug foo → exit 0；--report-slug -foo → yargs 抛出 Unknown arguments: f, o。建议修复：恢复 safeTarget 丢失的前导短横线剥离，和/或将 SKILL.md 的调用改为等号形式 --report-slug=<slug>；补充以 -foo/超长 slug 验证搬迁凭证的 guard-check 测试。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const insideArtifacts = | ||
| rootSegments[i] === '.qwen' && | ||
| (rootSegments[i + 1] === 'audits' || rootSegments[i + 1] === 'tmp'); | ||
| if (insideArtifacts || (rootSegments[i] === '.git' && i > 0)) { |
There was a problem hiding this comment.
[Critical] R21-24: the never-walk-git-internals guard matches only the literal name .git (here, in ALWAYS_EXCLUDED_DIRS at :139, and the descent skip at :336), so bare-repository directories (*.git) are enumerated and content-read as source subjects, violating the unconditional invariant stated at :168-170 (".git stays excluded unconditionally: a broken git must not fail-open into walking its internals"). Witness (probe at this head): collectAuditFiles on a fabricated tree containing git init --bare app.git with remote.origin.url = https://user:sekrit-token@example.com/repo.git returned app.git/config (credential-bearing: true), app.git/HEAD, app.git/packed-refs and 14 hook samples classified source with excludedDirs: [] — the deploy credential is measured, priced into the estimate, and quoted into every consumer of module content (agent briefs, sidecar, report). Suggested fix: treat bare-repo shapes as excluded in both guards (name === '.git' || name.endsWith('.git') in the root-segment loop and descent handling, recorded in excludedDirs), or detect the bare-repo shape (HEAD + objects/ + refs/ children) and refuse it like the .git root case.
| if (insideArtifacts || (rootSegments[i] === '.git' && i > 0)) { | |
| if (insideArtifacts || ((rootSegments[i] === '.git' || rootSegments[i].endsWith('.git')) && i > 0)) { |
中文说明
[Critical] R21-24：禁止遍历 git 内部目录的守卫只匹配字面名 .git（此处、:139 的 ALWAYS_EXCLUDED_DIRS 以及 :336 的下行跳过均如此），因此裸仓库目录（*.git）会被枚举并作为 source 主题读取内容，违反 :168-170 声明的无条件不变量（".git stays excluded unconditionally"）。证据（本 head 探针）：对含有 git init --bare app.git（config 中带 url = https://user:sekrit-token@…）的构造目录树运行 collectAuditFiles，返回 app.git/config（含凭据：true）、app.git/HEAD、app.git/packed-refs 及 14 个 hook 样例，均归类为 source，excludedDirs: []——部署凭据被测量、计入估算并流入模块内容的每个消费方。建议修复：在两处守卫中将裸仓库形态一并排除，或检测裸仓库形态并像 .git 根目录一样拒绝。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| 'audits', | ||
| getProjectHash(resolved), | ||
| ); | ||
| fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); |
There was a problem hiding this comment.
[Critical] R21-25: getAuditFallbackDir establishes the relocation landing with a bare mkdirSync(recursive) at a path fully predictable to the audited agent (QWEN_HOME/audits/<sha256(projectRoot)>), never validating a pre-existing leaf. Witness (probe at this head): (1) a leaf pre-planted as a symlink to a directory is silently followed — the write through the returned path landed in the attacker-chosen target with mode 777, voiding the documented 0700 privacy promise; planRelocated realpaths both sides, so containment credits the landing, and if the target were a worktree dir without ignore rules, fallbackLandingSafe would fail at every checkpoint forever (unrecoverable exit-5 loop); (2) a leaf pre-planted as a regular file throws raw EEXIST out of checkLocalOnlyGuard (files-plan.ts:1046 has no try/catch), replacing the designed exit-5 degradation with a generic crash at audit start. The adversary is the module's own declared one (the audited agent, same OS user, knows projectRoot and can compute the leaf path). Suggested fix: lstat the leaf; refuse symlinks and non-directories via the module's designed degradation path, and chmod an adopted real directory to 0o700.
| fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); | |
| fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); | |
| try { | |
| const leaf = fs.lstatSync(dir); | |
| if (leaf.isSymbolicLink() || !leaf.isDirectory()) { | |
| throw new Error('audit: fallback landing is not a real directory'); | |
| } | |
| fs.chmodSync(dir, 0o700); | |
| } catch (err) { | |
| if ((err as NodeJS.ErrnoException).code !== 'ENOENT') throw err; | |
| } | |
| return dir; |
中文说明
[Critical] R21-25：getAuditFallbackDir 用裸 mkdirSync(recursive) 在对被审计 agent 完全可预测的路径（QWEN_HOME/audits/<sha256(projectRoot)>）上建立搬迁落点，从不校验已存在的叶子节点。证据（本 head 探针）：(1) 预先植入的符号链接叶子会被静默跟随——通过返回路径写入的内容落在攻击者选定的目标目录，权限为 777，使文档承诺的 0700 隐私保证失效；planRelocated 两侧都做 realpath，因此包含性检查会给该落点凭证；若目标是某个没有 ignore 规则的 worktree 目录，fallbackLandingSafe 将在每个检查点永远失败（不可恢复的 exit-5 循环）；(2) 预先植入的普通文件叶子会使 checkLocalOnlyGuard（files-plan.ts:1046 无 try/catch）抛出裸 EEXIST，用审计启动时的通用崩溃取代设计好的 exit-5 降级。攻击者正是本模块自己声明的对手（被审计 agent，同一 OS 用户，知道 projectRoot 并可算出叶子路径）。建议修复：对叶子做 lstat，通过设计好的降级路径拒绝符号链接与非目录，并对被接纳的真实目录 chmod 0o700。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| * path, including under vendor/ and the path root: dependency installs, | ||
| * tooling output, and the tool's own artifact class — unless git tracks | ||
| * files inside the directory (see dirHasTrackedFiles). */ | ||
| const ALWAYS_EXCLUDED_DIRS = new Set([ |
There was a problem hiding this comment.
[Critical] R21-28: with QWEN_HOME placed inside the audited tree (QWEN_HOME=<repo>/.qwen-home — user-settable, and guard-check.ts:82 itself names in-worktree fallback roots), a run whose in-repo landing is refused relocates artifacts to <repo>/.qwen-home/audits/<hash>/; the NEXT qwen audit plan-files <repo> enumerates that subtree as source subjects — .qwen-home is absent from ALWAYS_EXCLUDED_DIRS (only .qwen), the descent guard matches only basename .qwen, and the walk deliberately ignores ignore status. Witness (probe at this head): collectAuditFiles(repo) with a populated .qwen-home/audits/<hash>/ returned the prior dated report .md, audit-plan-<ts>.json, sidecar.json, diff.patch and untracked/copied.ts among subjects (excludedDirs: ['.git'] only) — prior-run artifacts that quote the module become subjects of the next audit (self-contaminated findings), and since captureUntrackedArm lists via git ls-files --others without --exclude-standard, they are also copied into the next sidecar. Suggested fix: compute Storage.getAuditFallbackDir for the invocation and skip that subtree in walkAuditTree (extending both the root-segment refusal and the descent guard), or refuse to plan when the fallback root resolves inside the audited root.
中文说明
[Critical] R21-28：当 QWEN_HOME 位于被审计目录树内部（QWEN_HOME=<repo>/.qwen-home——用户可设置，且 guard-check.ts:82 本身就提到 worktree 内的回退根）时，仓库内落点被拒绝的运行会把产物搬迁到 <repo>/.qwen-home/audits/<hash>/；下一次 qwen audit plan-files <repo> 会把该子树枚举为 source 主题——.qwen-home 不在 ALWAYS_EXCLUDED_DIRS（只有 .qwen），下行守卫只匹配 basename .qwen，且遍历刻意忽略 ignore 状态。证据（本 head 探针）：对含已填充 .qwen-home/audits/<hash>/ 的仓库运行 collectAuditFiles，subjects 中出现上一轮的日期报告 .md、audit-plan-<ts>.json、sidecar.json、diff.patch 与 untracked/copied.ts（excludedDirs 仅 ['.git']）——上一轮引用模块内容的产物成为下一次审计的主题（自污染 findings），且 captureUntrackedArm 用不带 --exclude-standard 的 git ls-files --others 列举，它们还会被复制进下一个 sidecar。建议修复：为本次调用计算 Storage.getAuditFallbackDir 并在 walkAuditTree 中跳过该子树，或在回退根落在被审计根内部时拒绝规划。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| // reordered Anchor-before-Location shape arrives with locations | ||
| // still empty). Held as pending instead, committed only by a | ||
| // following second Anchor (see above). | ||
| if (current.locations.length === 0) { |
There was a problem hiding this comment.
[Critical] R17-29: (filed round 17, round 20 could not rule — round-21 probe confirms it still stands at this head) the second of two Location fields is silently dropped at parse level. The first Location binds; a second one is held as pendingLocations and committed only if a SECOND Anchor follows — so the shape Location/Location/Anchor (a pair cited on two lines before the snippet) keeps only the first citation: the second file is never scope-checked, never resolved, and the anchor gate grades the finding against half of its own citation. Witness (tsx probe at this head): a draft with - Location: foo.ts:10 + - Location: bar.ts:20 + one Anchor parses to locations: ['foo.ts'] — bar.ts present: false, in locations, locationRaw, and every downstream verdict. Suggested fix: on a second Location field before any anchor is collected, merge into the existing locations (current.locations.push(...parseLocations(field[2]))), or fail-close the block (current.severity = '') the way the unfenced quoted-pair path does — never silently drop a cited file.
| if (current.locations.length === 0) { | |
| if (current.locations.length === 0 || current.anchorLines.length === 0) { | |
| if (current.anchorLines.length === 0) { | |
| current.locations.push(...parseLocations(field[2])); | |
| current.locationRaw = `${current.locationRaw}, ${field[2]}`; | |
| } else { | |
| current.locations = parseLocations(field[2]); | |
| current.locationRaw = field[2]; | |
| } |
中文说明
[Critical] R17-29：（第 17 轮提出，第 20 轮无法判定——第 21 轮探针确认在本 head 仍然成立）两个 Location 字段中的第二个在解析层被静默丢弃。第一个 Location 绑定；第二个被暂存为 pendingLocations，仅当随后出现第二个 Anchor 时才提交——因此 Location/Location/Anchor 形态（片段之前分两行引用一对位置）只保留第一个引用：第二个文件永远不会被作用域检查、永远不会被解析，anchor 门只用该 finding 自身引用的一半来评级。证据（本 head tsx 探针）：- Location: foo.ts:10 + - Location: bar.ts:20 + 一个 Anchor 的草稿解析为 locations: ['foo.ts']——bar.ts 在 locations、locationRaw 及所有下游 verdict 中均不存在。建议修复：在尚未收集 anchor 时把第二个 Location 合并进已有 locations，或像未围栏引用对路径那样 fail-close（current.severity = ''）——绝不应静默丢弃被引用的文件。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| }); | ||
|
|
||
| it('resolves against registered deep-read callers outside the path', () => { | ||
| const caller = join(dir, '..', `caller-${Date.now()}.ts`); |
There was a problem hiding this comment.
[Critical] R19-1: 第 17 轮提出（R17-3），第 18、19、20 轮复核仍然成立，本轮代码未变，仍然成立 / still stands — code unchanged, re-verified at this head. This test feeds join()-native (backslash) paths into locations and registeredCallers, but resolveAnchors slash-normalizes only callerSet and compares locations verbatim — on Windows the membership check fails and the finding grades out-of-scope instead of resolved. No skipIf(win32) guard; the merge-queue test_windows leg runs it. Suggested fix: normalize both sides through the same slash conversion before membership, or skipIf(process.platform === 'win32') and cover the Windows shape with forward-slash citations.
中文说明
[Critical] R19-1：第 17 轮提出（R17-3），第 18、19、20 轮复核仍然成立，本轮代码未变，仍然成立。该测试把 join() 原生（反斜杠）路径传入 locations 与 registeredCallers，但 resolveAnchors 只对 callerSet 做斜杠归一化，locations 按原样比较——在 Windows 上成员检查失败，finding 被判 out-of-scope 而非 resolved。没有 skipIf(win32) 防护，merge 队列的 test_windows 环节会运行它。建议修复：成员检查前对两侧做同样的斜杠归一化，或 skipIf(win32) 并用正斜杠引用覆盖 Windows 形态。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| * and a linked worktree alike, and does not dirty the tracked .gitignore). | ||
| * Returns the exclude file path written. */ | ||
| export function applyExcludeRemedy(projectRoot: string): string { | ||
| const commonDir = git(projectRoot, ['rev-parse', '--git-common-dir']); |
There was a problem hiding this comment.
[Critical] R19-3: 第 19 轮提出，第 20 轮复核仍然成立，本轮以探针在本 head 重新核实，仍然成立 / still stands — re-probed at this head in round 21. applyExcludeRemedy resolves the exclude file through rev-parse --git-common-dir without checking that the resolved common dir belongs to the audited repository — a planted regular-file .git containing gitdir: <victim>/.git re-homes the remedy: the rules are appended to the VICTIM repo's info/exclude. Witness (round-21 probe): planted-gitfile corpus → applyExcludeRemedy returned <victim>/.git/info/exclude; lands in VICTIM repo: true. Suggested fix: add a containment check between the resolved --git-common-dir and the discovered toplevel, and refuse the remedy with a clear diagnostic when they disagree.
中文说明
[Critical] R19-3：第 19 轮提出，第 20 轮复核仍然成立，本轮以探针在本 head 重新核实，仍然成立。applyExcludeRemedy 通过 rev-parse --git-common-dir 解析 exclude 文件，却不检查解析出的 common dir 是否属于被审计仓库——植入含 gitdir: <victim>/.git 的普通文件 .git 会把补救措施重定向到别处：规则被追加进受害仓库的 info/exclude。证据（第 21 轮探针）：植入 gitfile 的语料 → applyExcludeRemedy 返回 <victim>/.git/info/exclude；落入受害仓库：true。建议修复：在解析出的 --git-common-dir 与发现的 toplevel 之间做包含性检查，不一致时以明确诊断拒绝补救。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const real = join(dir, 'real-root'); | ||
| mkdirSync(real, { recursive: true }); | ||
| const link = join(dir, 'link-root'); | ||
| symlinkSync(real, link); |
There was a problem hiding this comment.
[Critical] R19-5 (location 1 of 2 — :1017 and :326): 第 19 轮提出，第 20 轮复核仍然成立，本轮代码未变，仍然成立 / still stands — code unchanged, re-verified at this head. 'resolves a symlinked target to its real path' creates a directory symlink with symlinkSync but has no skipIf(win32) guard, while every sibling symlink fixture in this file is guarded. On the merge-queue Windows leg, directory-symlink creation requires privileges the runner may lack, turning the test red for environmental reasons. Suggested fix: it.skipIf(process.platform === 'win32'), consistent with every sibling fixture.
中文说明
[Critical] R19-5（两处之一 — :1017 与 :326）：第 19 轮提出，第 20 轮复核仍然成立，本轮代码未变，仍然成立。'resolves a symlinked target to its real path' 用 symlinkSync 创建目录符号链接但没有 skipIf(win32) 防护，而本文件中所有同类符号链接 fixture 都有防护。在 merge 队列的 Windows 环节，创建目录符号链接需要 runner 可能不具备的权限，测试会因环境原因变红。建议修复：与同类 fixture 保持一致，加 it.skipIf(process.platform === 'win32')。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| it('records symlinks and never follows them', () => { | ||
| symlinkSync(join(dir, 'src', 'a.ts'), join(dir, 'src', 'link.ts')); | ||
| mkdirSync(join(dir, 'real'), { recursive: true }); | ||
| symlinkSync(join(dir, 'real'), join(dir, 'dirlink')); |
There was a problem hiding this comment.
[Critical] R20-1 (location 2 of 2 — see the :1017 comment for the full finding): 第 20 轮提出，本轮代码未变，仍然成立 / still stands — code unchanged, re-verified at this head. Second unguarded directory-symlink fixture: 'records symlinks and never follows them' (symlinkSync(join(dir, 'real'), join(dir, 'dirlink'))) while every sibling symlink fixture is skipIf(win32)-guarded. Same Windows-leg environmental-red mechanism as R19-5. Suggested fix: add it.skipIf(process.platform === 'win32').
中文说明
[Critical] R20-1（两处之二——完整论证见 :1017 的评论）：第 20 轮提出，本轮代码未变，仍然成立。第二个未防护的目录符号链接 fixture：'records symlinks and never follows them'（symlinkSync(join(dir, 'real'), join(dir, 'dirlink'))），而所有同类 fixture 均有 skipIf(win32) 防护。与 R19-5 相同的 Windows 环节环境性变红机制。建议修复：加 it.skipIf(process.platform === 'win32')。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| GIT_TIMEOUT_MS, | ||
| ); | ||
| if (headProbe.ok) { | ||
| existing.meta.headSha = headProbe.out.trim(); |
There was a problem hiding this comment.
[Critical] R19-6 (location 2 of 2 — see the :391 comment for the full finding): 第 19 轮提出，第 20 轮复核仍然成立，本轮代码未变，仍然成立 / still stands — code unchanged, re-verified at this head. In captureSidecar's extend branch, meta.headSha is re-probed only inside the probeFailed/vcsProbeFailed === true arm — a capture whose toplevel probe succeeded but whose rev-parse HEAD transiently failed (timeout, load spike) never recovers headSha on any re-run: every checkpoint reports headUnknown, and SKILL.md treats every checkpoint as a stop for the entire remainder of the run — an unrecoverable stop loop from one timeout. Suggested fix: also re-probe headSha on the extend path when existing.meta.headSha is undefined and noVcs is false, mirroring the vcsProbeFailed repair.
中文说明
[Critical] R19-6（两处之二——完整论证见 :391 的评论）：第 19 轮提出，第 20 轮复核仍然成立，本轮代码未变，仍然成立。captureSidecar 的 extend 分支只在 probeFailed/vcsProbeFailed === true 分支内重新探测 meta.headSha——toplevel 探测成功但 rev-parse HEAD 瞬时失败（超时、负载尖峰）的捕获在任何重跑中都无法恢复 headSha：每个检查点都报告 headUnknown，SKILL.md 把每个检查点都当作停止信号，一次超时导致整个运行余下部分陷入不可恢复的停止循环。建议修复：extend 路径上 existing.meta.headSha 为 undefined 且 noVcs 为 false 时也重新探测 headSha，与 vcsProbeFailed 修复对称。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
🤖 Could not produce a passing fix for this feedback (round 27/100). This item now needs a human; the loop stays engaged and still picks up new feedback and base conflicts, but will not retry this item on its own. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/32314895906 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /review — fresh round requested: the branch was reworked on 2026-08-17 to match the merged design doc (hard topology gates replace chunk tiling, filesystem-walk enumeration, budget/estimate machinery, run-state subcommands, SKILL.md rewrite), then the shared-helper lifts left this PR via #9345 (merged). The last review round (round 8, 2026-08-11) predates all of that. |
qwen-code-ci-bot
left a comment
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; the same suites ran locally on Linux only, so the Windows leg that would exercise this PR's Windows-sensitive fixtures never ran.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; the same suites ran locally on Linux only.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 22, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/audit/lib/anchors.ts:643 — [probe] quadratic window scan over uncapped registered callers stalls the gate (measured 152668ms for one resolveAnchors call with a 2MB caller × 2000-line needle)packages/cli/src/commands/audit/lib/audit-agent-briefs.test.ts:159 — [probe] walked-vs-gate-arm fixture cannot distinguish the two arms (fixture.bin records 0 lines; the gate-arm mutation ships green)packages/cli/src/commands/audit/lib/audit-agent-briefs.test.ts:224 — [probe] comment says 31 subject lines; the fixture walks 30 (siblings say 30)packages/cli/src/commands/audit/lib/files-plan.ts:377 — [review] structuralUncoverable returned in readdir order while siblings are sorted — partially nondeterministic plan artifactpackages/cli/src/commands/review/lib/manifest-repository-context.test.ts:576 — [review] rewritten comment's worked example is arithmetically wrong for the 256 cap (2×100=200 ≤ 256) — the regression guard is disarmablepackages/core/src/skills/bundled/audit/SKILL.md:173 — [review] unquoted <artifacts-dir> substitution in every command block breaks relocated runs when QWEN_HOME has spacespackages/cli/src/commands/audit/lib/sidecar.ts:324 — [probe] vcsProbeFailed never cleared by a definitive notRepo re-probe — permanent headUnknown pin, unrecoverable except by deleting sidecar.jsonpackages/core/src/skills/bundled/audit/SKILL.md:180 — [review] exit-4 handling amends the draft but check-anchors is never re-run — the durable report is never gated
[Critical] R18-3 (body-level blocker carried from round 16 via 18/19/20/21; re-verified this round, still stands): the merged design doc's Records ship criterion requires the calibration records under docs/design/assets/ to be checkable BEFORE the spec's rates and caps are coded; at this head docs/design/assets/ holds only 7 unrelated images while SUBJECT_TOKENS_PER_LINE=2_607 / TEST_TOKENS_PER_LINE=1_457 remain coded at files-plan.ts:63-64 and are consumed by the estimate math at :1152. The maintainer decision escalated in comment 3761719334 (land redacted records and re-derive, or explicitly accept the author-reported rates) remains open. Witness: re-verified at this head (git ls-files docs/design/assets/ → 7 unrelated PNGs; constants coded and consumed; design doc Provenance + Verification sections state the criterion). / R18-3（自第 16 轮携带至今的 body 级阻断；本轮重新核实，仍然成立）：合入设计文档的 Records 交付准则要求校准记录在费率与上限被编码之前可核查；当前 docs/design/assets/ 只有 7 张无关图片，而两个费率常量仍编码于 files-plan.ts:63-64 并被估算数学消费。维护者决定（评论 3761719334）仍未决。
[Critical] R22-1 (new class finding this round; the inline site at anchors.ts:66-68 is occupied by prior comments 3745062814 ff.): the hand-rolled report parser and anchor matcher over unbounded LLM-authored report text have no structural fail-closed closure. Probe-verified entrances: a rendering deviation outside the enumerated nets parses to ZERO findings and check-anchors exits 0 (certifying a report whose snippets were never resolved); every template-conformant non-empty report grades its section-final blocks unresolved — under strict template compliance exit 0 is structurally unreachable (R20-3); plus fence-length (R20-5), second-Location drop (R17-29), isolated-second-Anchor merge flipping ambiguous→resolved, callerSet backslash asymmetry (R19-1), bold-peel refusals, two-field lookahead truncation, '#' token fusion, Unicode leading-edge fusion, tab/space indent-kind mismatch in the window matcher (reproduced on gofmt-shaped Go). Supersedes ledger entries R17-29, R20-3, R20-5, R19-1 — close the surface structurally (machine-checkable finding marker/count, symmetric normalization, whitespace-kind-insensitive indent compare), not entrance by entrance. / R22-1（本轮新发现的类级阻断；行内位点被在先评论占用）：针对不受限 LLM 报告文本的手写解析器与锚点匹配器没有结构性 fail-closed 闭合。探针已证入口包括：零 findings 解析仍 exit 0、模板合规报告的分节末块必判 unresolved（严格合规时 exit 0 结构性不可达）等。取代 R17-29、R20-3、R20-5、R19-1——请结构性关闭该表面，而不是逐入口打补丁。
[Critical] R22-2 (new pattern finding this round; all three inline sites occupied by prior comments 3791229750 (R11-28), 3788976686/3799403006 (R7-11/R16-8), 3791229753 (R10-5)): artifact write sites missing writeSidecarJson's regular-file gate — sidecar.ts diff.patch + copyGuarded destinations, plan-files.ts writePlanOut, parse-args.ts verdict write. Probe: planted writer-less FIFO at --out → write blocked forever (exit 124 timeout); the prescribed remedy re-runs the same command reusing the path, so the hang greets every retry. Same mechanism the earlier comments reported; re-confirmed by probe at this head. / R22-2（本轮新发现的模式级阻断；三个行内位点均被在先评论占用）：产物写入点缺少 writeSidecarJson 的常规文件门——探针实证植入无写端 FIFO 后写入永久阻塞，且规定的补救会复用同一路径再次挂起。
[Critical] R22-5 (new this round; inline site at sidecar.ts:179-180 occupied by prior comments 3788976678/3788979455 (R7-9)): captureDiffArm's git diff executes repo-config diff.<driver>.command and textconv drivers from the audited tree — arbitrary code execution against the auditor. Probe on git 2.47.3 with the PR's exact argv and gitEnv scrub: COMMAND DRIVER EXECUTED and TEXTCONV DRIVER EXECUTED markers fired from repo-local config; gitEnv scrubs only repo-selection env vars; every other git call in the pipeline is metadata-only. Fix direction: config isolation per invocation (--no-textconv plus diff.*.command neutralization) or capture dirty state without git diff. / R22-5（本轮新发现；行内位点被在先评论占用）：captureDiffArm 的 git diff 会执行被审计树仓库配置中的 diff 驱动——对审计者的任意代码执行（git 2.47.3 探针双驱动形态均已实测）。
[Critical] R22-6 (new this round; inline site at read-json.ts:91-92 occupied by prior comments 3797112668/3799402948 (R14-35)): the registered-caller channel has no scope or secret policy — readCallersFile accepts ANY absolute path; recordCaller content-copies up to 10MB into the persistent sidecar; resolveAnchors content-reads cited callers; SECRET_FILE_RE is never consulted and there is no repo-containment check; callers land mid-fan-out after consent, so no gate sees the paths. Probe through the real code: in-tree prod.env registered as a caller was copied verbatim into sidecar/callers/… (API_KEY content); id_rsa copied with its OPENSSH header — the walk's NEVER-content-read invariant is bypassed by registration. Same mechanism R14-35 reported; re-confirmed by probe at this head. / R22-6（本轮新发现；行内位点被在先评论 R14-35 占用）：registered-caller 通道没有作用域与机密策略——探针实证注册 .env/id_rsa 即可把凭据内容复制进 sidecar。
[Critical] R22-7 (new this round; inline site at files-plan.ts:455-457 occupied by prior comment 3791229822 (R11-59)): no O_NOFOLLOW on any content open (measureFile/readGuarded/streamSha256/copyGuarded) — the walk-time lstat is the only symlink enforcement, and the fd-based fstat gate sees the symlink's TARGET. Probe: a mid-window symlink swap put an OUT-OF-TREE file's sha256 into the sidecar baseline (byte-identical hash), drift-check silently accepted; adding O_NOFOLLOW flipped the probe to refused. House precedent for this exact race exists in-house (customBanner.ts, skill-args-file.ts). / R22-7（本轮新发现；行内位点被在先评论占用）：任何内容打开都不带 O_NOFOLLOW——遍历期 lstat 是唯一的符号链接防线；探针实证遍历→打开窗口内的符号链接替换可把树外文件内容带进 sidecar 基线。
[Critical] R22-8 (new this round; inline site at guard-check.ts:78 occupied by prior comments 3788976607/3788979377 (RA-5), 3797112701, 3799403018 (R16-10)): relocation credit and the 'original is gone' proof key on the basename of the agent-handed --plan path, not the writer-pinned name audit-plan-.json. Probe (flip on the basename alone, same repo/fallback content): V1-NONPINNED exitCode=undefined originalStillExists=true (copy named plan.json → suppression granted while the stageable original remains under .qwen/tmp); V1-PINNED exitCode=5 (copy named audit-plan-2026-08-13-120000.json → correctly refused). PLAN_TS_RE recovery also yields undefined for non-pinned names, so no plan-ts shape is probed — name-selective re-includes keyed to the plan ts escape. Same mechanism the earlier comments reported; re-probed at this head. / R22-8（本轮新发现；行内位点被在先评论占用）：搬迁凭证以 agent 递交的 --plan 路径的 basename 为键，而非写入方钉死的 audit-plan-.json——探针仅凭 basename 翻转：非钉死名 → exit 0 且原件仍可提交；钉死名 → exit 5。
[Critical] R22-9 (new this round; inline site at safe-read.ts:40-41 occupied by prior comments 3790712466 (R10-51), 3799402901 (R15-4)): streamSha256 has no size bound and is invoked uncapped on agent-nominated callers at capture/extend and at EVERY drift checkpoint (driftCheck re-streams every baselined caller). Probe: 64MB digestMatchesZeroOracle=true (every byte hashed); 4GB sparse hash 2609ms at 1570MiB/s — a 100GB sparse caller (truncate -s 100G, zero disk blocks) costs ≈65s per hash, per checkpoint, synchronously. CALLER_MAX_BYTES bounds only the archive copy, not eligibility. Same mechanism R15-4 reported; re-probed at this head. / R22-9（本轮新发现；行内位点被在先评论 R15-4 占用）：streamSha256 无大小上限，且在每个漂移检查点对 agent 提名的 caller 全量重哈希——100GB 稀疏文件每个检查点约 65 秒同步哈希。
[Critical] R22-11 (new this round; inline site at files-plan.ts:943-948 occupied by prior comment 3737523483 (R4-38)): gitEnv() does not scrub GIT_LITERAL_PATHSPECS (nor its glob/icase siblings) while the tracked arm relies on a :(literal) pathspec — ambient GIT_LITERAL_PATHSPECS=1 (exported profile / inherited launcher env, the same ambient channel the module defends against for GIT_CEILING_DIRECTORIES) makes ls-files answer EMPTY silently: trackedFiles=[] → status degrades to 'ok' → force-added tracked artifacts invisible, no relocation, no warning. The same env reaches sidecar's :(literal) pathspecs (empty diff, zero untracked copies, no captureDegraded marker). Probe on git 2.47.3: BASE lists the force-added file; GIT_LITERAL_PATHSPECS=1 → empty (exit 0) while the plain ls-files control stays healthy. Distinct mechanism from R4-38's raw-pathspec claim. Fix: drop the whole GIT_-prefixed family in gitEnv(), the way core isGitIgnored does. / R22-11（本轮新发现；行内位点被在先评论 R4-38 占用，机制不同）：gitEnv() 未清洗 GIT_LITERAL_PATHSPECS——环境携带 =1 时 :(literal) 探针静默答空，tracked 分支失效，守卫误判 ok（git 2.47.3 探针实证）。
中文说明
仅完成部分审查，审查缺口已披露。
未审查：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; the same suites ran locally on Linux only, so the Windows leg that would exercise this PR's Windows-sensitive fixtures never ran。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; the same suites ran locally on Linux only。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 22 轮，非阻断）——已记录，本轮不要求修改：共 8 条（原文未翻译，列表见上方英文部分）。
[Critical] R18-3 (body-level blocker carried from round 16 via 18/19/20/21; re-verified this round, still stands): the merged design doc's Records ship criterion requires the calibration records under docs/design/assets/ to be checkable BEFORE the spec's rates and caps are coded; at this head docs/design/assets/ holds only 7 unrelated images while SUBJECT_TOKENS_PER_LINE=2_607 / TEST_TOKENS_PER_LINE=1_457 remain coded at files-plan.ts:63-64 and are consumed by the estimate math at :1152. The maintainer decision escalated in comment 3761719334 (land redacted records and re-derive, or explicitly accept the author-reported rates) remains open. Witness: re-verified at this head (git ls-files docs/design/assets/ → 7 unrelated PNGs; constants coded and consumed; design doc Provenance + Verification sections state the criterion). / R18-3（自第 16 轮携带至今的 body 级阻断；本轮重新核实，仍然成立）：合入设计文档的 Records 交付准则要求校准记录在费率与上限被编码之前可核查；当前 docs/design/assets/ 只有 7 张无关图片，而两个费率常量仍编码于 files-plan.ts:63-64 并被估算数学消费。维护者决定（评论 3761719334）仍未决。
[Critical] R22-1 (new class finding this round; the inline site at anchors.ts:66-68 is occupied by prior comments 3745062814 ff.): the hand-rolled report parser and anchor matcher over unbounded LLM-authored report text have no structural fail-closed closure. Probe-verified entrances: a rendering deviation outside the enumerated nets parses to ZERO findings and check-anchors exits 0 (certifying a report whose snippets were never resolved); every template-conformant non-empty report grades its section-final blocks unresolved — under strict template compliance exit 0 is structurally unreachable (R20-3); plus fence-length (R20-5), second-Location drop (R17-29), isolated-second-Anchor merge flipping ambiguous→resolved, callerSet backslash asymmetry (R19-1), bold-peel refusals, two-field lookahead truncation, '#' token fusion, Unicode leading-edge fusion, tab/space indent-kind mismatch in the window matcher (reproduced on gofmt-shaped Go). Supersedes ledger entries R17-29, R20-3, R20-5, R19-1 — close the surface structurally (machine-checkable finding marker/count, symmetric normalization, whitespace-kind-insensitive indent compare), not entrance by entrance. / R22-1（本轮新发现的类级阻断；行内位点被在先评论占用）：针对不受限 LLM 报告文本的手写解析器与锚点匹配器没有结构性 fail-closed 闭合。探针已证入口包括：零 findings 解析仍 exit 0、模板合规报告的分节末块必判 unresolved（严格合规时 exit 0 结构性不可达）等。取代 R17-29、R20-3、R20-5、R19-1——请结构性关闭该表面，而不是逐入口打补丁。
[Critical] R22-2 (new pattern finding this round; all three inline sites occupied by prior comments 3791229750 (R11-28), 3788976686/3799403006 (R7-11/R16-8), 3791229753 (R10-5)): artifact write sites missing writeSidecarJson's regular-file gate — sidecar.ts diff.patch + copyGuarded destinations, plan-files.ts writePlanOut, parse-args.ts verdict write. Probe: planted writer-less FIFO at --out → write blocked forever (exit 124 timeout); the prescribed remedy re-runs the same command reusing the path, so the hang greets every retry. Same mechanism the earlier comments reported; re-confirmed by probe at this head. / R22-2（本轮新发现的模式级阻断；三个行内位点均被在先评论占用）：产物写入点缺少 writeSidecarJson 的常规文件门——探针实证植入无写端 FIFO 后写入永久阻塞，且规定的补救会复用同一路径再次挂起。
[Critical] R22-5 (new this round; inline site at sidecar.ts:179-180 occupied by prior comments 3788976678/3788979455 (R7-9)): captureDiffArm's git diff executes repo-config diff.<driver>.command and textconv drivers from the audited tree — arbitrary code execution against the auditor. Probe on git 2.47.3 with the PR's exact argv and gitEnv scrub: COMMAND DRIVER EXECUTED and TEXTCONV DRIVER EXECUTED markers fired from repo-local config; gitEnv scrubs only repo-selection env vars; every other git call in the pipeline is metadata-only. Fix direction: config isolation per invocation (--no-textconv plus diff.*.command neutralization) or capture dirty state without git diff. / R22-5（本轮新发现；行内位点被在先评论占用）：captureDiffArm 的 git diff 会执行被审计树仓库配置中的 diff 驱动——对审计者的任意代码执行（git 2.47.3 探针双驱动形态均已实测）。
[Critical] R22-6 (new this round; inline site at read-json.ts:91-92 occupied by prior comments 3797112668/3799402948 (R14-35)): the registered-caller channel has no scope or secret policy — readCallersFile accepts ANY absolute path; recordCaller content-copies up to 10MB into the persistent sidecar; resolveAnchors content-reads cited callers; SECRET_FILE_RE is never consulted and there is no repo-containment check; callers land mid-fan-out after consent, so no gate sees the paths. Probe through the real code: in-tree prod.env registered as a caller was copied verbatim into sidecar/callers/… (API_KEY content); id_rsa copied with its OPENSSH header — the walk's NEVER-content-read invariant is bypassed by registration. Same mechanism R14-35 reported; re-confirmed by probe at this head. / R22-6（本轮新发现；行内位点被在先评论 R14-35 占用）：registered-caller 通道没有作用域与机密策略——探针实证注册 .env/id_rsa 即可把凭据内容复制进 sidecar。
[Critical] R22-7 (new this round; inline site at files-plan.ts:455-457 occupied by prior comment 3791229822 (R11-59)): no O_NOFOLLOW on any content open (measureFile/readGuarded/streamSha256/copyGuarded) — the walk-time lstat is the only symlink enforcement, and the fd-based fstat gate sees the symlink's TARGET. Probe: a mid-window symlink swap put an OUT-OF-TREE file's sha256 into the sidecar baseline (byte-identical hash), drift-check silently accepted; adding O_NOFOLLOW flipped the probe to refused. House precedent for this exact race exists in-house (customBanner.ts, skill-args-file.ts). / R22-7（本轮新发现；行内位点被在先评论占用）：任何内容打开都不带 O_NOFOLLOW——遍历期 lstat 是唯一的符号链接防线；探针实证遍历→打开窗口内的符号链接替换可把树外文件内容带进 sidecar 基线。
[Critical] R22-8 (new this round; inline site at guard-check.ts:78 occupied by prior comments 3788976607/3788979377 (RA-5), 3797112701, 3799403018 (R16-10)): relocation credit and the 'original is gone' proof key on the basename of the agent-handed --plan path, not the writer-pinned name audit-plan-.json. Probe (flip on the basename alone, same repo/fallback content): V1-NONPINNED exitCode=undefined originalStillExists=true (copy named plan.json → suppression granted while the stageable original remains under .qwen/tmp); V1-PINNED exitCode=5 (copy named audit-plan-2026-08-13-120000.json → correctly refused). PLAN_TS_RE recovery also yields undefined for non-pinned names, so no plan-ts shape is probed — name-selective re-includes keyed to the plan ts escape. Same mechanism the earlier comments reported; re-probed at this head. / R22-8（本轮新发现；行内位点被在先评论占用）：搬迁凭证以 agent 递交的 --plan 路径的 basename 为键，而非写入方钉死的 audit-plan-.json——探针仅凭 basename 翻转：非钉死名 → exit 0 且原件仍可提交；钉死名 → exit 5。
[Critical] R22-9 (new this round; inline site at safe-read.ts:40-41 occupied by prior comments 3790712466 (R10-51), 3799402901 (R15-4)): streamSha256 has no size bound and is invoked uncapped on agent-nominated callers at capture/extend and at EVERY drift checkpoint (driftCheck re-streams every baselined caller). Probe: 64MB digestMatchesZeroOracle=true (every byte hashed); 4GB sparse hash 2609ms at 1570MiB/s — a 100GB sparse caller (truncate -s 100G, zero disk blocks) costs ≈65s per hash, per checkpoint, synchronously. CALLER_MAX_BYTES bounds only the archive copy, not eligibility. Same mechanism R15-4 reported; re-probed at this head. / R22-9（本轮新发现；行内位点被在先评论 R15-4 占用）：streamSha256 无大小上限，且在每个漂移检查点对 agent 提名的 caller 全量重哈希——100GB 稀疏文件每个检查点约 65 秒同步哈希。
[Critical] R22-11 (new this round; inline site at files-plan.ts:943-948 occupied by prior comment 3737523483 (R4-38)): gitEnv() does not scrub GIT_LITERAL_PATHSPECS (nor its glob/icase siblings) while the tracked arm relies on a :(literal) pathspec — ambient GIT_LITERAL_PATHSPECS=1 (exported profile / inherited launcher env, the same ambient channel the module defends against for GIT_CEILING_DIRECTORIES) makes ls-files answer EMPTY silently: trackedFiles=[] → status degrades to 'ok' → force-added tracked artifacts invisible, no relocation, no warning. The same env reaches sidecar's :(literal) pathspecs (empty diff, zero untracked copies, no captureDegraded marker). Probe on git 2.47.3: BASE lists the force-added file; GIT_LITERAL_PATHSPECS=1 → empty (exit 0) while the plain ls-files control stays healthy. Distinct mechanism from R4-38's raw-pathspec claim. Fix: drop the whole GIT_-prefixed family in gitEnv(), the way core isGitIgnored does. / R22-11（本轮新发现；行内位点被在先评论 R4-38 占用，机制不同）：gitEnv() 未清洗 GIT_LITERAL_PATHSPECS——环境携带 =1 时 :(literal) 探针静默答空，tracked 分支失效，守卫误判 ok（git 2.47.3 探针实证）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| artifacts: { | ||
| reportSlug: safeTarget(targetPath), | ||
| }, |
There was a problem hiding this comment.
[Critical] R21-1: still stands — re-probed at this head in round 22 (code unchanged since round 21; independently rediscovered by this round's territory walk). Slug space mismatch: this plan produces artifacts.reportSlug via safeTarget(targetPath), but the consumers accept a narrower space — the merge with origin/main reverted safeTarget's leading-dash strip (utils/paths.ts ends with flat.replace(/^[._]+/, '')), so slugs can start with -, and safeTarget never truncates. Three demonstrated entrances: (1) isSafeReportSlug (guard-check.ts) rejects dash-leading and >200-char slugs, so a fully verified relocation loses credit and guardTripped re-fires exit 5 at every checkpoint — an unrecoverable redo loop; (2) SKILL.md's space-form guard-check --report-slug <slug> dies in yargs before any handler for a dash-leading slug, silently disabling the runtime re-check; (3) slugs over ~234 chars push <ts>-<slug>.md past NAME_MAX. Witness (rounds 21+22 probes): safeTarget('./-foo') → '-foo'; guard-check with dash slug -foo and a fully verified relocation → exit 5, safe slug foo → exit 0; --report-slug -foo → yargs threw Unknown arguments: f, o. Fix: restore the leading-dash strip in safeTarget (flat.replace(/^[._-]+/, '') || 'target') and/or switch SKILL.md to the equals form --report-slug=<slug>; add a guard-check test crediting a verified relocation with a -foo/over-200-char plan slug.
中文说明
[Critical] R21-1：仍然成立——第 22 轮在本 head 重新探针核实（代码自第 21 轮未变；本轮territory 审查独立重新发现）。slug 取值空间不匹配：plan 通过 safeTarget(targetPath) 生成 artifacts.reportSlug，但消费方接受的空间更窄——与 origin/main 的合并回退了 safeTarget 的前导短横线剥离（utils/paths.ts 现为 flat.replace(/^[._]+/, '')），slug 又可能以 - 开头，且 safeTarget 从不截断长度。三个已演示入口：(1) isSafeReportSlug 拒绝短横线开头及超过 200 字符的 slug，即便搬迁已完全验证也会失去凭证——guardTripped 在每个检查点重新触发 exit 5，形成不可恢复的重做循环；(2) SKILL.md 的空格形式 guard-check --report-slug <slug> 对短横线开头 slug 在任何 handler 运行前就死于 yargs 解析，静默禁用运行时复查；(3) 超过约 234 字符的 slug 使 <ts>-<slug>.md 超过 NAME_MAX。证据（第 21、22 轮探针）：safeTarget('./-foo') → '-foo'；短横线 slug 且搬迁已验证 → exit 5，安全 slug foo → exit 0；--report-slug -foo → yargs 抛出 Unknown arguments: f, o。修复：恢复 safeTarget 的前导短横线剥离，和/或将 SKILL.md 改为等号形式；补充以 -foo/超长 slug 验证搬迁凭证的测试。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const insideArtifacts = | ||
| rootSegments[i] === '.qwen' && | ||
| (rootSegments[i + 1] === 'audits' || rootSegments[i + 1] === 'tmp'); | ||
| if (insideArtifacts || (rootSegments[i] === '.git' && i > 0)) { |
There was a problem hiding this comment.
[Critical] R21-24: still stands — re-verified at this head in round 22. The never-walk-git-internals guard matches only the literal name .git (here, and in ALWAYS_EXCLUDED_DIRS / the descent skip), so bare-repository directories (*.git) are enumerated and content-read as source subjects, violating the unconditional invariant stated at :168-170 ('.git stays excluded unconditionally: a broken git must not fail-open into walking its internals'). Witness (round-21 probe, re-verified): collectAuditFiles on a tree containing git init --bare app.git with remote.origin.url = https://user:sekrit-token@example.com/repo.git returned app.git/config (credential-bearing: true), app.git/HEAD, app.git/packed-refs and hook samples classified source with excludedDirs: [] — the deploy credential is measured, priced into the estimate, and quoted into every consumer of module content. Fix: treat bare-repo shapes as excluded in both guards (name === '.git' || name.endsWith('.git'), recorded in excludedDirs), or detect the bare-repo shape (HEAD + objects/ + refs/) and refuse it like the .git root case.
中文说明
[Critical] R21-24：仍然成立——第 22 轮在本 head 重新核实。禁止遍历 git 内部目录的守卫只匹配字面名 .git（此处以及 ALWAYS_EXCLUDED_DIRS、下行跳过均如此），因此裸仓库目录（*.git）会被枚举并作为 source 主题读取内容，违反 :168-170 声明的无条件不变量。证据（第 21 轮探针，已复核）：对含 git init --bare app.git（config 带凭据 URL）的目录树运行 collectAuditFiles，返回 app.git/config（含凭据）、app.git/HEAD、app.git/packed-refs 及 hook 样例，均归类 source——部署凭据被测量、计价并流入模块内容的每个消费方。修复：在两处守卫中排除裸仓库形态，或检测裸仓库形态并像 .git 根目录一样拒绝。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); | ||
| return dir; |
There was a problem hiding this comment.
[Critical] R21-25: still stands — re-probed at this head in round 22. getAuditFallbackDir establishes the relocation landing with a bare mkdirSync(recursive, mode 0o700) at a path fully predictable to the audited agent (QWEN_HOME/audits/<sha256(projectRoot)>), never validating a pre-existing leaf — and mkdirSync's mode applies only to directories it CREATES. Witness (probe at this head): (1) a leaf pre-planted as a symlink to a directory is silently followed — the write through the returned path lands in the attacker-chosen target with the target's mode (0755), voiding the documented 0700 privacy promise; planRelocated realpaths both sides, so containment credits the landing. (2) a leaf pre-planted as a regular file throws raw EEXIST out of checkLocalOnlyGuard (files-plan.ts:1046 has no try/catch) — a generic crash at audit start replacing the designed exit-5 degradation. The adversary is the module's own (the audited agent, same OS user, can compute the leaf path). Fix: lstat the leaf; refuse symlinks and non-directories via the module's designed degradation path, and chmod an adopted real directory to 0o700.
中文说明
[Critical] R21-25：仍然成立——第 22 轮在本 head 重新探针核实。getAuditFallbackDir 用裸 mkdirSync(recursive, mode 0o700) 在对被审计 agent 完全可预测的路径上建立搬迁落点，从不校验已存在的叶子节点——且 mkdirSync 的 mode 只作用于它创建的目录。证据（本 head 探针）：(1) 预植入的符号链接叶子被静默跟随——写入落在攻击者选定的目标目录（权限 0755），使 0700 隐私承诺失效；planRelocated 两侧 realpath，包含性检查给该落点凭证。(2) 预植入的普通文件叶子使 checkLocalOnlyGuard 抛出裸 EEXIST——审计启动时通用崩溃，取代设计好的 exit-5 降级。修复：对叶子做 lstat，通过设计好的降级路径拒绝符号链接与非目录，并对被接纳的真实目录 chmod 0o700。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const ALWAYS_EXCLUDED_DIRS = new Set([ | ||
| 'node_modules', |
There was a problem hiding this comment.
[Critical] R21-28: still stands — re-probed at this head in round 22. With QWEN_HOME placed inside the audited tree (QWEN_HOME=<repo>/.qwen-home — user-settable, and guard-check.ts itself names in-worktree fallback roots), a run whose in-repo landing is refused relocates artifacts to <repo>/.qwen-home/audits/<hash>/; the NEXT qwen audit plan-files <repo> enumerates that subtree as source subjects — .qwen-home is absent from ALWAYS_EXCLUDED_DIRS (only .qwen), the descent guard matches only basename .qwen, and the walk deliberately ignores ignore status. Witness (probe at this head): collectAuditFiles(repo) with a populated .qwen-home/audits/<hash>/ returned the prior dated report .md, audit-plan-<ts>.json, sidecar.json, diff.patch and untracked/copied.ts among subjects — prior-run artifacts that quote the module become subjects of the next audit (self-contaminated findings), and captureUntrackedArm's git ls-files --others without --exclude-standard also copies them into the next sidecar. Fix: compute Storage.getAuditFallbackDir for the invocation and skip that subtree in walkAuditTree (both the root-segment refusal and the descent guard), or refuse to plan when the fallback root resolves inside the audited root.
中文说明
[Critical] R21-28：仍然成立——第 22 轮在本 head 重新探针核实。当 QWEN_HOME 位于被审计目录树内部时，仓库内落点被拒绝的运行会把产物搬迁到 <repo>/.qwen-home/audits/<hash>/；下一次 qwen audit plan-files <repo> 会把该子树枚举为 source 主题——.qwen-home 不在 ALWAYS_EXCLUDED_DIRS（只有 .qwen），下行守卫只匹配 basename .qwen，且遍历刻意忽略 ignore 状态。证据（本 head 探针）：上一轮的报告、plan、sidecar、diff.patch 与 untracked 拷贝都出现在 subjects 中——上一轮引用模块的产物成为下一次审计的主题（自污染 findings），且会被复制进下一个 sidecar。修复：为本次调用计算 Storage.getAuditFallbackDir 并在 walkAuditTree 中跳过该子树，或在回退根落在被审计根内部时拒绝规划。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| export function applyExcludeRemedy(projectRoot: string): string { | ||
| const commonDir = git(projectRoot, ['rev-parse', '--git-common-dir']); |
There was a problem hiding this comment.
[Critical] R19-3: still stands — re-probed at this head in round 22 (filed round 19, re-verified rounds 20-21). applyExcludeRemedy resolves the exclude file through rev-parse --git-common-dir without checking that the resolved common dir belongs to the audited repository — a planted regular-file .git containing gitdir: <victim>/.git re-homes the remedy: the rules are appended to the VICTIM repo's info/exclude. gitEnv()'s env scrub does not defend against an on-disk .git pointer inside the untrusted audited tree. Witness (round-21 probe, re-verified): planted-gitfile corpus → applyExcludeRemedy returned <victim>/.git/info/exclude; lands in VICTIM repo: true. This is the unvalidated-write sibling of the only-ungated-write-path class, driven by untrusted content. Fix: add a containment check between the resolved --git-common-dir and the discovered toplevel, and refuse the remedy with a clear diagnostic when they disagree.
中文说明
[Critical] R19-3：仍然成立——第 22 轮在本 head 重新探针核实（第 19 轮提出，第 20-21 轮复核）。applyExcludeRemedy 通过 rev-parse --git-common-dir 解析 exclude 文件，却不检查解析出的 common dir 是否属于被审计仓库——植入含 gitdir: <victim>/.git 的普通文件 .git 会把补救措施重定向到受害仓库的 info/exclude。gitEnv() 的环境变量清洗防不住不受信被审计树内的磁盘 .git 指针文件。证据（第 21 轮探针，已复核）：植入 gitfile 的语料 → 返回 <victim>/.git/info/exclude；落入受害仓库：true。修复：在解析出的 common dir 与发现的 toplevel 之间做包含性检查，不一致时以明确诊断拒绝补救。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| mkdirSync(join(dir, 'real'), { recursive: true }); | ||
| symlinkSync(join(dir, 'real'), join(dir, 'dirlink')); |
There was a problem hiding this comment.
[Critical] R20-1: still stands — code unchanged since round 20; re-verified at this head in round 22. (location 2 of 2 — see the :1017 comment R19-5 for the full finding.) Second unguarded directory-symlink fixture: 'records symlinks and never follows them' (symlinkSync(join(dir, 'real'), join(dir, 'dirlink'))) while every sibling symlink fixture is skipIf(win32)-guarded. Same Windows-leg environmental-red mechanism as R19-5; doubly exposed here — on a Windows host without symlink privilege even the file symlink at :326 throws, reddening the whole test. Witness: not run — Windows merge-queue leg not openable from this Linux worktree; deterministic code state at this head. Fix: add it.skipIf(process.platform === 'win32').
中文说明
[Critical] R20-1：仍然成立——代码自第 20 轮未变，第 22 轮在本 head 重新核实。（两处之二——完整论证见 :1017 的 R19-5 评论。）第二个未防护的目录符号链接 fixture：'records symlinks and never follows them'，而所有同类 fixture 均有 skipIf(win32) 防护。与 R19-5 相同的 Windows 环节环境性变红机制；此处双重暴露——在没有符号链接权限的 Windows 主机上，:326 的文件符号链接也会抛错，整个测试变红。修复：加 it.skipIf(process.platform === 'win32')。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| if (probe.ok) { | ||
| if (probeFailed) { | ||
| existing.meta.noVcs = false; |
There was a problem hiding this comment.
[Critical] R19-6: still stands — re-probed at this head in round 22 (filed round 19, re-verified rounds 20-21). (location 2 of 2 — the full finding also covers the :391 headSha write site.) In captureSidecar's extend branch, meta.headSha is re-probed only inside the probeFailed / vcsProbeFailed === true arm — a capture whose toplevel probe succeeded but whose rev-parse HEAD transiently failed (timeout, load spike) never recovers headSha on any re-run: every checkpoint reports headUnknown, and SKILL.md treats every checkpoint as a stop for the entire remainder of the run — an unrecoverable stop loop from one timeout. Witness (round-22 probe, end-to-end through the real module): constructed post-failure sidecar (noVcs:false, no headSha, no vcsProbeFailed) on a repo with a valid HEAD, then the extend re-run → headSha_afterExtend: null, drift_headUnknown: true; control with vcsProbeFailed:true DOES recover headSha — proving the repair exists only on that arm. Fix: also re-probe headSha on the extend path when existing.meta.headSha is undefined and noVcs is false, mirroring the vcsProbeFailed repair.
中文说明
[Critical] R19-6：仍然成立——第 22 轮在本 head 重新探针核实（第 19 轮提出，第 20-21 轮复核）。captureSidecar 的 extend 分支只在 probeFailed/vcsProbeFailed === true 分支内重新探测 meta.headSha——toplevel 探测成功但 rev-parse HEAD 瞬时失败的捕获在任何重跑中都无法恢复 headSha：每个检查点都报告 headUnknown，SKILL.md 把每个检查点都当作停止信号——一次超时导致整个运行余下部分陷入不可恢复的停止循环。证据（第 22 轮端到端探针）：构造故障后 sidecar 并 extend 重跑 → headSha_afterExtend: null、drift_headUnknown: true；对照（vcsProbeFailed:true）能恢复 headSha——证明修复只存在于该分支。修复：extend 路径上 headSha 为 undefined 且 noVcs 为 false 时也重新探测。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const SECRET_FILE_RE = | ||
| /(^|\/)(\.env|\.env\.[^/]+|[^/]+\.env|\.npmrc|\.netrc|credentials\.json|id_(rsa|dsa|ecdsa|ed25519|ed448|xmss)(\.pub)?|[^/]+\.(pem|key|p12|pfx|keystore|tfstate(\.backup)?))$/i; |
There was a problem hiding this comment.
[Critical] R22-3 (new this round, probe-verified): the never-content-read guard for credential-shaped names is escaped by two demonstrated name classes. (1) Any trailing CR/LF in the filename: every clause is $-anchored without the m flag, JS $ matches only end-of-string, and readdirSync names are kept verbatim through the walk — probe: .env, id_rsa land in uncoverable as secret-shaped, while .env\n / id_rsa\n land in subjects (kind source, lines=1), content-read by measureFile and the event scan. (2) Backup/editor suffix variants of every guarded name — regex run: ESCAPES foo.env.bak / .env~ / .npmrc~ / .netrc.old / credentials.json.bak / id_rsa.bak / id_ed25519.old / server.pem.old / deploy.tfstate.bak while all guarded baselines match; the single tfstate-only (\.backup)? shows backup coverage was intended but not generalized. Escaping files land in subjects and their credential content flows into agent model payloads and findings artifacts — against the invariant comment on this block ('NEVER content-read … no content copy, walker read, or model payload ever sees them'), with no adversary required. Fix: close the class at enumeration — fail-closed on names containing control characters (record uncoverable), and generalize the backup-suffix tolerance (e.g. optional (~|.bak|.old|.orig|.swp|.tmp) before the $ anchor).
中文说明
[Critical] R22-3（本轮新发现，探针验证）：凭据形文件名的「永不读取内容」守卫被两类已演示的文件名逃逸。(1) 文件名带尾部 CR/LF：所有分支都是 $ 锚定且无 m 标志，遍历保留 readdir 原名——探针：.env、id_rsa 归入 uncoverable（secret-shaped），而 .env\n/id_rsa\n 进入 subjects（kind source），被内容读取。(2) 每个受保护名字的备份/编辑器后缀变体——正则实测：foo.env.bak/.env~/.npmrc~/.netrc.old/credentials.json.bak/id_rsa.bak 等全部逃逸。逃逸文件进入 subjects，凭据内容流入 agent 模型载荷与审计产物——违反本块注释的不变量，且无需任何对手。修复：在枚举层关闭该类——对含控制字符的文件名 fail-closed，并把备份后缀容忍泛化（如 $ 锚前允许可选 (~|.bak|.old|.orig|.swp|.tmp)）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const nextDate = auditTimestamp(new Date(Date.now() + 24 * 60 * 60 * 1000)) | ||
| .split('-') | ||
| .slice(0, 3) |
There was a problem hiding this comment.
[Critical] R22-4 (new this round, probe-verified): the 'next calendar date' probe computes tomorrow as Date.now() + 24h here and in the duplicated arithmetic at files-plan.ts:1034 — on a DST fall-back day (a 25-hour day) that instant stays inside the current calendar date during the first wall-clock hour, so the true next date is never asked about, in every DST timezone (auditTimestamp uses local-time getters). Witness (probe under TZ=America/Los_Angeles running this exact expression): at local 2026-11-01T00:30 (fall-back day, first hour) nextDate(+24h)=2026-11-01 while the true next calendar date is 2026-11-02 — nextDate === today: true, next date NEVER PROBED; the whole first hour fails (00:59:59 too); normal-day controls are fine. Failure scenario: a guard-check/plan-files run starts in the first hour of a fall-back day; the hours-long audit rolls past midnight; the report lands named with the true next date; a name-selective re-include keyed to that date — the exact adversary the probe set exists for (the suite's own test is named 'a next-date-keyed re-include cannot escape the audits probe') — escapes every probe, and the write-time checkpoint certifies a committable report. The pinned test at files-plan.test.ts:1599 derives tomorrow with the same expression, so the suite shares the error. Fix: derive the next calendar date from date components (e.g. setDate(getDate() + 1) on a local-date construction), not epoch arithmetic, at both sites.
中文说明
[Critical] R22-4（本轮新发现，探针验证）：「下一日历日」探针用 Date.now() + 24h 计算明天（此处与 files-plan.ts:1034 的重复算术）——在 DST 回拨日（25 小时日）的第一个墙钟小时内，该时刻仍落在当天日历日内，于是真正的下一天永远不会被探测（auditTimestamp 用本地时间 getter，适用于所有 DST 时区）。证据（TZ=America/Los_Angeles 探针执行原表达式）：本地 2026-11-01T00:30（回拨日第一小时）nextDate(+24h)=2026-11-01，而真正的下一日历日是 2026-11-02——下一天从未被探测；整个第一小时都失败；正常日对照正常。失败场景：运行始于回拨日第一小时，数小时的审计跨过午夜，报告以真正的下一日期命名落盘；按该日期键入的名称选择性再包含（探针集本要防的对手）逃逸所有探测，写入时检查点认证了可提交的报告。修复：在两处用日期分量推导下一日历日（如 setDate(getDate()+1)），而不是 epoch 算术。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| // Valid JSON is not yet a Sidecar: a wrong-shape file ({} / {"meta":{}}) | ||
| // must hit the same friendly corruption error, not crash driftCheck with | ||
| // a raw TypeError. |
There was a problem hiding this comment.
[Critical] R22-10 (new this round, verified by trace): sidecar.json is trusted as the run-start drift baseline with no integrity mechanism, and its default landing (.qwen/audits/audit-<ts>.sidecar/, SKILL Step 3) is INSIDE the audited tree — writable by the audit's own agents and anything with repo write access. loadSidecar gates only readability and JSON shape; grep across the command group finds no mechanism that hashes or authenticates the sidecar itself; the in-repo landing is gitignored, so git history cannot expose a swap. The module's own comment concedes 'the audited agent can swap the file' — the swap is modeled, and only the read is defended. Failure scenario: a steered subagent (prompt-injection steering suffices per the module's adversary model) edits a walked subject to hide/plant content, then rewrites sidecar.json with shape-conformant JSON carrying recomputed sha256s plus current headSha/subtreeHash/callerHashes: driftCheck reports zero drift, no headUnknown, no recaptured marker; every checkpoint passes and the run ships findings anchored to content that no longer exists — silently voiding exactly the stop this mechanism exists to enforce. The detectable variants (garbage JSON → 'corrupt or truncated'; FIFO → friendly throw) stop the run, so the well-formed forgery is the invisible variant. Fix: move the sidecar landing out of the audited tree (fallback-root-only landing), or record a capture-time hash of sidecar.json in a second orchestrator-held location and have drift-check re-verify the sidecar's own hash before trusting its baselines.
中文说明
[Critical] R22-10（本轮新发现，代码追踪核实）：sidecar.json 被当作运行起点的漂移基线信任，却没有任何完整性机制，且其默认落点（.qwen/audits/audit-<ts>.sidecar/，SKILL 第 3 步）位于被审计树内部——审计自身的 agent 和任何有仓库写权限者都可写。loadSidecar 只校验可读性与 JSON 形状；整个命令组没有任何机制对 sidecar 本身做哈希/认证；仓库内落点被 gitignore，git 历史也无法暴露替换。模块自己的注释承认「被审计 agent 可以替换该文件」——替换已被建模，却只防御了读取。失败场景：被引导的子代理修改已遍历主题后，用形状合规、sha256 重算过的 JSON 覆写 sidecar.json：driftCheck 报告零漂移，每个检查点通过，运行交付锚定在已不存在内容上的 findings——静默废掉了该机制本要强制的停止。可检测的变体（垃圾 JSON、FIFO）会停止运行，形状合规的伪造是唯一不可见的变体。修复：把 sidecar 落点移出被审计树，或在编排器持有的第二位置记录捕获时 sidecar.json 的哈希并在 drift-check 信任基线前重新验证。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: PR #8403 (legacy code audit) is not converging — maintainer decision neededWhy this round stopped without code changesThe workflow's deterministic growth brake has been over budget for 23+ rounds in Current state of the PR (verified at HEAD daa8f6e)
中文说明🤖 AutoFix 更新了一个过期的 base —— 修复未通过验证，但本 PR 落后于 停止前我了解到的情况： PR #8403（legacy code audit）已不收敛——需要维护者决策本轮为何未做任何代码改动即停止工作流的确定性增长刹车在本计数窗口已连续 23 轮以上超出预算，且 diff 仍在 PR 当前状态（已在 HEAD daa8f6e 核实）
为什么继续打补丁不会收敛每一轮的修复都在增加源码与测试行数，使窗口持续超出预算；23 轮逐入口补丁 Run log: https://github.com/QwenLM/qwen-code/actions/runs/32345831409 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- audit-agent-briefs.test.ts:159 walked-vs-gate-arm fixture cannot distinguish the two arms — already recorded in the round-22 deferral list (review 4979543644)
- audit-agent-briefs.test.ts:224 comment says 31 subject lines while the fixture walks 30 — already recorded in the round-22 deferral list (review 4979543644)
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; the Windows leg that would exercise this PR's Windows-sensitive fixtures never ran.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; the same suites ran locally on Linux only.
Not explored to full depth (tool budget reached): chunk 7: none — one check was consciously skipped rather than budget-stopped: executing the suite via vitest was not possible (no node_modules in the review worktree; …; chunk 6: executing anchors.test.ts under vitest (worktree and parent checkout have no node_modules; fresh npm ci + workspace build not attempted).
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 23, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/audit/guard-check.test.ts:122 — [probe] GIT_CONFIG_GLOBAL pin never reaches isGitIgnored's check-ignore probes (strips all GIT_*); hermeticity claim false for that armpackages/cli/src/commands/audit/lib/files-plan.ts:1086 — [probe] exclude remedy anchors logical .qwen name while guardDir probes the physical path; silent no-op for symlinked .qwenpackages/cli/src/commands/audit/lib/safe-read.ts:81 — [review] readGuarded/readFdCapped have no direct unit test; the only over-cap test exercises the collector's earlier stat gatepackages/cli/src/commands/audit/lib/sidecar.ts:179 — [probe] unborn-HEAD git diff fallback misses staged-but-worktree-clean dirt; born arm captures it, unborn arm never can, no markerpackages/cli/src/commands/audit/wiring.test.ts:85 — [review] --callers delivery never asserted for snapshot/check-anchors; a builder-handler rename ships greenpackages/core/src/config/storage.ts:353 — [probe] dead defensive branch in getAuditFallbackDir exists only to tolerate the file's own bare vi.fn() mockpackages/cli/src/commands/audit/lib/read-json.ts:59 — [probe] isFileEntry's absolute/.. escape guard has no test; deleting it ships green and lets plan entries escape the audited rootpackages/cli/src/commands/audit/agent-prompt.ts:84 — [probe] yargs .check() making --probes mandatory exercised by no test; deleting it ships green and silently prints DECLINED briefspackages/cli/src/config/config.ts:1159 — [probe] pre-exit stdout flush pinned by no test; dropping the await re-truncates >pipe-buffer stdout (probe: 146176/204800 vs 204800/204800)packages/cli/src/commands/audit/lib/sidecar.ts:184 — [probe] fresh/re-captured sidecar never cleans the reused dir; stale diff.patch/untracked copies publish as this capture's evidence (archive-only)packages/cli/src/commands/audit/guard-check.test.ts:228 — [probe] fallbackLandingSafe's if (planTs) landing-probe branch exercised by no test; deleting the arm ships greenpackages/cli/src/commands/audit/lib/audit-agent-briefs.ts:293 — [probe] findingCap shape-checked only, never against LOW_FINDING_CAP ground truth; a stale plan can set the low-reader noise cappackages/cli/src/commands/audit/lib/files-plan.test.ts:84 — [review] hermeticity comment claims the opposite of what isGitIgnored does (same mechanism as the confirmed :122 finding)packages/cli/src/commands/audit/lib/files-plan.test.ts:395 — [probe] 8 SECRET_FILE_RE clauses (.netrc, credentials.json, id_ed448, id_xmss, .key, .p12, .pfx, .keystore) have no fixture; deleting any clause ships greenpackages/cli/src/commands/audit/lib/files-plan.test.ts — [probe] whole-content NUL-scan invariant pinned only at byte 16384; a windowed regression >= 16KiB+1 ships greenpackages/cli/src/commands/audit/lib/files-plan.ts:455 — [review] measureFile reads whole content before the size-cap arm; multi-GB subjects block enumeration; live appender drags the loop to a live EOFpackages/cli/src/commands/audit/lib/sidecar.test.ts:166 — [probe] extend-path retry of name-only callers has no positive test; the skip-condition mutation ships green with phantom driftpackages/cli/src/commands/audit/lib/sidecar.ts:124 — [probe] caller-copy key strips the drive letter; C:/D: callers with the same tail collide and the later copy overwrites (Windows)packages/core/src/skills/bundled/audit/SKILL.md:193 — [review] Consumption template tier-blind at low (estimate null at low; unpriced additions never run; only the trailing segment conditional)packages/cli/src/commands/audit/agent-prompt.test.ts:75 — [probe] low-reader happy path through agentPromptCommand untested; a handler-branch regression ships green- …and 6 more (see the run report)
[Critical] R18-3 (body-level blocker carried from round 16 via 18/19/20/21/22; re-verified this round, still stands): the merged design doc's Records ship criterion requires the calibration records under docs/design/assets/ to be checkable BEFORE the spec's rates and caps are coded; at this head docs/design/assets/ holds only 7 unrelated images while SUBJECT_TOKENS_PER_LINE=2_607 / TEST_TOKENS_PER_LINE=1_457 remain coded at files-plan.ts:63-64 and are consumed by the estimate math at :1152. The maintainer decision escalated in comment 3761719334 (land redacted records and re-derive, or explicitly accept the author-reported rates) remains open. Witness: git ls-files docs/design/assets/ → 7 unrelated PNGs; constants coded and consumed.
[Critical] R22-1 (class finding, carried from round 22; still stands; inline site at anchors.ts:66-68 occupied by prior comments): the hand-rolled report parser and anchor matcher over unbounded LLM-authored report text have no structural fail-closed closure. Probe-verified entrances: a rendering deviation outside the enumerated nets parses to ZERO findings and check-anchors exits 0 (certifying a report whose snippets were never resolved); every template-conformant non-empty report grades its section-final blocks unresolved — under strict template compliance exit 0 is structurally unreachable; plus fence-length, second-Location drop, isolated-second-Anchor merge flipping ambiguous→resolved, callerSet backslash asymmetry, bold-peel refusals, two-field lookahead truncation, '#' token fusion, Unicode leading-edge fusion, tab/space indent-kind mismatch. Round-23 folded entrances (re-demonstrated this round): matcher missed line-start occurrence class (probe: ambiguous→resolved flip), trailing-whitespace peel needles, '#[test]'-confirmed terminator truncation, backtick-before-punctuation peel ordering, the SKILL Language section instructing localization while the gate parses English-only tokens (localized block shapes never parse — every finding grades unresolved in non-English runs), and the mandated rejected-findings appendix re-entering the gate as unresolved blocks (no section awareness). Close the surface structurally (machine-checkable finding marker/count, symmetric normalization, whitespace-kind-insensitive indent compare, section awareness), not entrance by entrance.
[Critical] R22-2 (pattern finding, carried from round 22; still stands; all three inline sites occupied by prior comments): artifact write sites missing writeSidecarJson's regular-file gate — sidecar.ts diff.patch + copyGuarded destinations, plan-files.ts writePlanOut, parse-args.ts verdict write. Probe: planted writer-less FIFO at --out blocks the write forever; the prescribed remedy re-runs the same command reusing the path, so the hang greets every retry. Round-23 confirmed a NEW site of the same pattern: applyExcludeRemedy's .git/info/exclude read/write — a symlinked exclude redirects the remedy write into an arbitrary host file (probe: decoy host file rewritten with original content + the remedy's lines; see the occupied inline thread at files-plan.ts:1097, comment 3799402939).
[Critical] R22-5 (carried from round 22; still stands; inline site at sidecar.ts:179-180 occupied by prior comments): captureDiffArm's git diff executes repo-config diff..command and textconv drivers from the audited tree — arbitrary code execution against the auditor. Probe on git 2.47.3 with the PR's exact argv and gitEnv scrub: COMMAND DRIVER EXECUTED and TEXTCONV DRIVER EXECUTED markers fired from repo-local config; gitEnv scrubs only repo-selection env vars. Fix direction: config isolation per invocation (--no-textconv plus diff.*.command neutralization) or capture dirty state without git diff.
[Critical] R22-6 (carried from round 22; still stands; inline site at read-json.ts:91-92 occupied by prior comments): the registered-caller channel has no scope or secret policy — readCallersFile accepts ANY absolute path; recordCaller content-copies up to 10MB into the persistent sidecar; resolveAnchors content-reads cited callers; SECRET_FILE_RE is never consulted and there is no repo-containment check; callers land mid-fan-out after consent, so no gate sees the paths. Probe: in-tree prod.env registered as a caller was copied verbatim into sidecar/callers/… (API_KEY content); id_rsa copied with its OPENSSH header — the walk's NEVER-content-read invariant is bypassed by registration.
[Critical] R22-7 (carried from round 22; still stands; inline site at files-plan.ts:455-457 occupied by prior comments): no O_NOFOLLOW on any content open (measureFile/readGuarded/streamSha256/copyGuarded) — the walk-time lstat is the only symlink enforcement, and the fd-based fstat gate sees the symlink's TARGET. Probe: a mid-window symlink swap put an OUT-OF-TREE file's sha256 into the sidecar baseline (byte-identical hash), drift-check silently accepted; adding O_NOFOLLOW flipped the probe to refused.
[Critical] R22-8 (carried from round 22; still stands; inline site at guard-check.ts:78 occupied by prior comments): relocation credit and the 'original is gone' proof key on the basename of the agent-handed --plan path, not the writer-pinned name audit-plan-.json. Probe (flip on the basename alone, same repo/fallback content): non-pinned name → suppression granted while the stageable original remains under .qwen/tmp; pinned name → correctly refused. PLAN_TS_RE recovery also yields undefined for non-pinned names, so no plan-ts shape is probed — name-selective re-includes keyed to the plan ts escape.
[Critical] R22-9 (carried from round 22; still stands; inline site at safe-read.ts:40-41 occupied by prior comments): streamSha256 has no size bound and is invoked uncapped on agent-nominated callers at capture/extend and at EVERY drift checkpoint (driftCheck re-streams every baselined caller). Probe: 64MB fully hashed; 4GB sparse hash 2609ms at 1570MiB/s — a 100GB sparse caller costs ~65s per hash, per checkpoint, synchronously. CALLER_MAX_BYTES bounds only the archive copy, not eligibility.
[Critical] R22-11 (carried from round 22; still stands; inline site at files-plan.ts:943-948 occupied by prior comments): gitEnv() does not scrub GIT_LITERAL_PATHSPECS (nor its glob/icase siblings) while the tracked arm relies on a :(literal) pathspec — ambient GIT_LITERAL_PATHSPECS=1 (exported profile / inherited launcher env) makes ls-files answer EMPTY silently: trackedFiles=[] → status degrades to 'ok' → force-added tracked artifacts invisible, no relocation, no warning. The same env reaches sidecar's :(literal) pathspecs (empty diff, zero untracked copies, no captureDegraded marker). Probe on git 2.47.3: BASE lists the force-added file; GIT_LITERAL_PATHSPECS=1 → empty (exit 0) while the plain ls-files control stays healthy. Fix: drop the whole GIT_-prefixed family in gitEnv(), the way core isGitIgnored does.
[Critical] R21-1 (ledger re-post; inline site at files-plan.ts:1388 occupied by unrelated prior comments, relocated here): still stands — re-probed at this head; slug space mismatch: buildFilesPlan produces artifacts.reportSlug via safeTarget(targetPath), but the consumers (isSafeReportSlug / probe-name construction) accept a narrower space than safeTarget emits; the merge with origin/main reverted safeTarget's leading-dash strip (utils/paths.ts ends with flat.replace(/^[.]+/, '') — the branch-side fix spelling /^[.-]+/ was lost), so plan slugs starting with '-' fail the consumer gate and relocation credit/probe names break.
[Critical] R19-6 (ledger re-post; inline site at sidecar.ts:325 occupied by prior comments, relocated here): still stands — in captureSidecar's extend branch, meta.headSha is re-probed only inside the probeFailed / vcsProbeFailed === true arm — a capture whose toplevel probe succeeded but whose rev-parse HEAD transiently failed never sets vcsProbeFailed, so a missing headSha is unrecoverable; every checkpoint reports headUnknown and the documented mid-run retry never repairs it — permanent headUnknown over-stop unless sidecar.json is deleted.
[Critical] R23-4 (new this round, probe-verified; inline site at read-json.ts:71 occupied by an unrelated test-gap comment, relocated here): readPlanFile's containment gate forbids element paths from being absolute or carrying '..', but targetPathAbsolute — the base every join resolves against — is only checked for absoluteness, so a root carrying '..' segments passes the gate and re-binds every consumer outside the audited root. Probe: readPlanFile(targetPathAbsolute='/target/..') → no throw; resolveAnchors verdict 'resolved' for sibling/secret.ts OUTSIDE the audited dir; same citation under a correct plan → 'out-of-scope'; element path '../sibling/secret.ts' → throws 'not a plan written by'. The plan lands in-tree and agent-writable (R22-10's premise) — with no plan-hash/root pin anywhere (see the occupied snapshot.ts:97 thread), a tampered plan re-targets check-anchors and drift-check at will. Fix: reject non-normalized roots (plan.targetPathAbsolute !== resolve(plan.targetPathAbsolute)) with the regenerate error.
[Critical] R23-5 (new this round, probe-verified; inline site at sidecar.ts:331-332 occupied by R19-6 threads, relocated here): the vcsProbeFailed repair arm establishes HEAD, subtree, diff-arm and untracked-arm baselines at extend re-run time — mid-run — without setting meta.recaptured or any marker, so drift before the repair is silently invisible and the archive presents mid-run captures as run-start; the fresh-capture path resetting the same kind of baseline IS required to flag it. Probe: after a blind-window commit, repair sets meta.headSha to the post-window HEAD, clears vcsProbeFailed, recaptured=undefined, capturedAt unchanged; driftCheck → headMoved=false, driftedFiles=[]; the corrupt-sidecar sibling correctly sets recaptured. Fix: set a recaptured-semantics marker whenever the repair arm establishes baselines after run start.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：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; the Windows leg that would exercise this PR's Windows-sensitive fixtures never ran。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; the same suites ran locally on Linux only。
未探索到全部深度（达到工具调用预算）：chunk 7：none — one check was consciously skipped rather than budget-stopped: executing the suite via vitest was not possible (no node_modules in the review worktree; …；chunk 6：executing anchors.test.ts under vitest (worktree and parent checkout have no node_modules; fresh npm ci + workspace build not attempted)。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 23 轮，非阻断）——已记录，本轮不要求修改：共 26 条（原文未翻译，列表见上方英文部分）。
[Critical] R18-3 (body-level blocker carried from round 16 via 18/19/20/21/22; re-verified this round, still stands): the merged design doc's Records ship criterion requires the calibration records under docs/design/assets/ to be checkable BEFORE the spec's rates and caps are coded; at this head docs/design/assets/ holds only 7 unrelated images while SUBJECT_TOKENS_PER_LINE=2_607 / TEST_TOKENS_PER_LINE=1_457 remain coded at files-plan.ts:63-64 and are consumed by the estimate math at :1152. The maintainer decision escalated in comment 3761719334 (land redacted records and re-derive, or explicitly accept the author-reported rates) remains open. Witness: git ls-files docs/design/assets/ → 7 unrelated PNGs; constants coded and consumed.
[Critical] R22-1 (class finding, carried from round 22; still stands; inline site at anchors.ts:66-68 occupied by prior comments): the hand-rolled report parser and anchor matcher over unbounded LLM-authored report text have no structural fail-closed closure. Probe-verified entrances: a rendering deviation outside the enumerated nets parses to ZERO findings and check-anchors exits 0 (certifying a report whose snippets were never resolved); every template-conformant non-empty report grades its section-final blocks unresolved — under strict template compliance exit 0 is structurally unreachable; plus fence-length, second-Location drop, isolated-second-Anchor merge flipping ambiguous→resolved, callerSet backslash asymmetry, bold-peel refusals, two-field lookahead truncation, '#' token fusion, Unicode leading-edge fusion, tab/space indent-kind mismatch. Round-23 folded entrances (re-demonstrated this round): matcher missed line-start occurrence class (probe: ambiguous→resolved flip), trailing-whitespace peel needles, '#[test]'-confirmed terminator truncation, backtick-before-punctuation peel ordering, the SKILL Language section instructing localization while the gate parses English-only tokens (localized block shapes never parse — every finding grades unresolved in non-English runs), and the mandated rejected-findings appendix re-entering the gate as unresolved blocks (no section awareness). Close the surface structurally (machine-checkable finding marker/count, symmetric normalization, whitespace-kind-insensitive indent compare, section awareness), not entrance by entrance.
[Critical] R22-2 (pattern finding, carried from round 22; still stands; all three inline sites occupied by prior comments): artifact write sites missing writeSidecarJson's regular-file gate — sidecar.ts diff.patch + copyGuarded destinations, plan-files.ts writePlanOut, parse-args.ts verdict write. Probe: planted writer-less FIFO at --out blocks the write forever; the prescribed remedy re-runs the same command reusing the path, so the hang greets every retry. Round-23 confirmed a NEW site of the same pattern: applyExcludeRemedy's .git/info/exclude read/write — a symlinked exclude redirects the remedy write into an arbitrary host file (probe: decoy host file rewritten with original content + the remedy's lines; see the occupied inline thread at files-plan.ts:1097, comment 3799402939).
[Critical] R22-5 (carried from round 22; still stands; inline site at sidecar.ts:179-180 occupied by prior comments): captureDiffArm's git diff executes repo-config diff..command and textconv drivers from the audited tree — arbitrary code execution against the auditor. Probe on git 2.47.3 with the PR's exact argv and gitEnv scrub: COMMAND DRIVER EXECUTED and TEXTCONV DRIVER EXECUTED markers fired from repo-local config; gitEnv scrubs only repo-selection env vars. Fix direction: config isolation per invocation (--no-textconv plus diff.*.command neutralization) or capture dirty state without git diff.
[Critical] R22-6 (carried from round 22; still stands; inline site at read-json.ts:91-92 occupied by prior comments): the registered-caller channel has no scope or secret policy — readCallersFile accepts ANY absolute path; recordCaller content-copies up to 10MB into the persistent sidecar; resolveAnchors content-reads cited callers; SECRET_FILE_RE is never consulted and there is no repo-containment check; callers land mid-fan-out after consent, so no gate sees the paths. Probe: in-tree prod.env registered as a caller was copied verbatim into sidecar/callers/… (API_KEY content); id_rsa copied with its OPENSSH header — the walk's NEVER-content-read invariant is bypassed by registration.
[Critical] R22-7 (carried from round 22; still stands; inline site at files-plan.ts:455-457 occupied by prior comments): no O_NOFOLLOW on any content open (measureFile/readGuarded/streamSha256/copyGuarded) — the walk-time lstat is the only symlink enforcement, and the fd-based fstat gate sees the symlink's TARGET. Probe: a mid-window symlink swap put an OUT-OF-TREE file's sha256 into the sidecar baseline (byte-identical hash), drift-check silently accepted; adding O_NOFOLLOW flipped the probe to refused.
[Critical] R22-8 (carried from round 22; still stands; inline site at guard-check.ts:78 occupied by prior comments): relocation credit and the 'original is gone' proof key on the basename of the agent-handed --plan path, not the writer-pinned name audit-plan-.json. Probe (flip on the basename alone, same repo/fallback content): non-pinned name → suppression granted while the stageable original remains under .qwen/tmp; pinned name → correctly refused. PLAN_TS_RE recovery also yields undefined for non-pinned names, so no plan-ts shape is probed — name-selective re-includes keyed to the plan ts escape.
[Critical] R22-9 (carried from round 22; still stands; inline site at safe-read.ts:40-41 occupied by prior comments): streamSha256 has no size bound and is invoked uncapped on agent-nominated callers at capture/extend and at EVERY drift checkpoint (driftCheck re-streams every baselined caller). Probe: 64MB fully hashed; 4GB sparse hash 2609ms at 1570MiB/s — a 100GB sparse caller costs ~65s per hash, per checkpoint, synchronously. CALLER_MAX_BYTES bounds only the archive copy, not eligibility.
[Critical] R22-11 (carried from round 22; still stands; inline site at files-plan.ts:943-948 occupied by prior comments): gitEnv() does not scrub GIT_LITERAL_PATHSPECS (nor its glob/icase siblings) while the tracked arm relies on a :(literal) pathspec — ambient GIT_LITERAL_PATHSPECS=1 (exported profile / inherited launcher env) makes ls-files answer EMPTY silently: trackedFiles=[] → status degrades to 'ok' → force-added tracked artifacts invisible, no relocation, no warning. The same env reaches sidecar's :(literal) pathspecs (empty diff, zero untracked copies, no captureDegraded marker). Probe on git 2.47.3: BASE lists the force-added file; GIT_LITERAL_PATHSPECS=1 → empty (exit 0) while the plain ls-files control stays healthy. Fix: drop the whole GIT_-prefixed family in gitEnv(), the way core isGitIgnored does.
[Critical] R21-1 (ledger re-post; inline site at files-plan.ts:1388 occupied by unrelated prior comments, relocated here): still stands — re-probed at this head; slug space mismatch: buildFilesPlan produces artifacts.reportSlug via safeTarget(targetPath), but the consumers (isSafeReportSlug / probe-name construction) accept a narrower space than safeTarget emits; the merge with origin/main reverted safeTarget's leading-dash strip (utils/paths.ts ends with flat.replace(/^[.]+/, '') — the branch-side fix spelling /^[.-]+/ was lost), so plan slugs starting with '-' fail the consumer gate and relocation credit/probe names break.
[Critical] R19-6 (ledger re-post; inline site at sidecar.ts:325 occupied by prior comments, relocated here): still stands — in captureSidecar's extend branch, meta.headSha is re-probed only inside the probeFailed / vcsProbeFailed === true arm — a capture whose toplevel probe succeeded but whose rev-parse HEAD transiently failed never sets vcsProbeFailed, so a missing headSha is unrecoverable; every checkpoint reports headUnknown and the documented mid-run retry never repairs it — permanent headUnknown over-stop unless sidecar.json is deleted.
[Critical] R23-4 (new this round, probe-verified; inline site at read-json.ts:71 occupied by an unrelated test-gap comment, relocated here): readPlanFile's containment gate forbids element paths from being absolute or carrying '..', but targetPathAbsolute — the base every join resolves against — is only checked for absoluteness, so a root carrying '..' segments passes the gate and re-binds every consumer outside the audited root. Probe: readPlanFile(targetPathAbsolute='/target/..') → no throw; resolveAnchors verdict 'resolved' for sibling/secret.ts OUTSIDE the audited dir; same citation under a correct plan → 'out-of-scope'; element path '../sibling/secret.ts' → throws 'not a plan written by'. The plan lands in-tree and agent-writable (R22-10's premise) — with no plan-hash/root pin anywhere (see the occupied snapshot.ts:97 thread), a tampered plan re-targets check-anchors and drift-check at will. Fix: reject non-normalized roots (plan.targetPathAbsolute !== resolve(plan.targetPathAbsolute)) with the regenerate error.
[Critical] R23-5 (new this round, probe-verified; inline site at sidecar.ts:331-332 occupied by R19-6 threads, relocated here): the vcsProbeFailed repair arm establishes HEAD, subtree, diff-arm and untracked-arm baselines at extend re-run time — mid-run — without setting meta.recaptured or any marker, so drift before the repair is silently invisible and the archive presents mid-run captures as run-start; the fresh-capture path resetting the same kind of baseline IS required to flag it. Probe: after a blind-window commit, repair sets meta.headSha to the post-window HEAD, clears vcsProbeFailed, recaptured=undefined, capturedAt unchanged; driftCheck → headMoved=false, driftedFiles=[]; the corrupt-sidecar sibling correctly sets recaptured. Fix: set a recaptured-semantics marker whenever the repair arm establishes baselines after run start.
— qwen3.8-max via Qwen Code /review (v0.21.14)
| writeStdoutLine(JSON.stringify(guard, null, 2)); | ||
| const relocated = | ||
| plan !== undefined && | ||
| slugSafe && |
There was a problem hiding this comment.
[Critical] R23-1: slugSafe applies isSafeReportSlug's 200-char cap to the plan's own reportSlug, but the writer (buildFilesPlan → safeTarget) never truncates — so any target path whose flattened slug exceeds 200 chars pins relocated=false forever, deadlocking the safety gate at exit 5 on every drift checkpoint and the write-time check, in a configuration the gate's own contract promises to clear. Probe (all variables fixed except slug length, repo with both dirs exposed at plan time and a verified relocation): slug len 3/200 → exit undefined; len 201 → exit 5; len 255 (safeTarget output for a deep absolute path) → exit 5; lifting the cap flips 201/255 back to exit undefined. SKILL's exit-5 remedy ("the relocation did not land") cannot fix a slug length. Cap the slug at the writer (buildFilesPlan) so plan and guard agree, rather than denying relocation credit for writer-produced names.
中文说明
[Critical] R23-1：slugSafe 对 plan 自身的 reportSlug 施加了 isSafeReportSlug 的 200 字符上限，但写入方（buildFilesPlan → safeTarget）从不截断——因此任何扁平化后超过 200 字符的目标路径都会把 relocated 永久钉死为 false，使安全门在每个漂移检查点和写入时检查都以 exit 5 死锁，而这正是该门自身契约承诺应当放行的配置。探针（除 slug 长度外全部变量固定）：长度 3/200 → 正常放行；201 → exit 5；255（深层绝对路径的 safeTarget 输出）→ exit 5；放宽上限后 201/255 恢复放行。SKILL 的 exit-5 补救（"搬迁未落盘"）无法修复 slug 长度问题。应在写入方截断 slug，使 plan 与 guard 一致，而不是拒绝承认写入方产出的名字。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| writeSidecarJson(existingPath, existing); | ||
| return existing; |
There was a problem hiding this comment.
[Critical] R23-6: writeSidecarJson's regular-file gate is an lstatSync→writeFileSync check-then-use pair — the write's open follows symlinks and blocks forever on a writer-less FIFO. The extend write runs mid-fan-out with the sidecar's default landing inside the audited tree, where the audited agent can swap the file (the module's own threat statement). Measured race (attacker holding a symlink-to-decoy at sidecar.json ~50% duty, victim looping the extend path): 140 of 1364 writes redirected the full sidecar JSON to the attacker-chosen decoy (~10% in 30s); the static-plant gate throws (337/337) — only the race gets through. Applying the fd-based fix dropped hits to 0. copyGuarded in this same file documents the rule this write violates. Make the gate fd-based: openSync(path, O_WRONLY | O_CREAT | O_TRUNC | O_NOFOLLOW), fstatSync(fd).isFile(), write via the fd — or write a temp regular file and rename it over sidecar.json.
中文说明
[Critical] R23-6：writeSidecarJson 的常规文件门是 lstatSync→writeFileSync 的先检查后使用对——写入打开会跟随符号链接，并在无写端 FIFO 上永久阻塞。extend 写入发生在 fan-out 中途，sidecar 默认落盘位置在被审计树内，被审计 agent 可以替换该文件（模块自己的威胁声明）。实测竞争（攻击者以约 50% 占空比把 sidecar.json 维持为指向诱饵的符号链接，受害者循环执行 extend 路径）：1364 次写入中 140 次把完整 sidecar JSON 重定向到攻击者选定的诱饵（30 秒内约 10%）；静态植入会被门拦下（337/337 抛错）——只有竞争能穿过。采用基于 fd 的修复后命中降为 0。同文件的 copyGuarded 正是这一规则的示范。请把门改为基于 fd：openSync(path, O_WRONLY | O_CREAT | O_TRUNC | O_NOFOLLOW) + fstatSync(fd).isFile() + 经 fd 写入；或写临时常规文件后 rename 覆盖 sidecar.json。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| // 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 = auditTimestamp(new Date(Date.now() + 24 * 60 * 60 * 1000)) |
There was a problem hiding this comment.
[Critical] R22-4: still stands at this head — the audit code is byte-identical to round 22's probes (only config.ts changed via the main merge). The 'next calendar date' probe computes tomorrow as Date.now() + 24h here and in the duplicated arithmetic at files-plan.ts:1034 — on a DST fall-back day (a 25-hour day) that instant stays inside the current calendar date during the first wall-clock hour, so the true next date is never probed, in every DST timezone; a name-selective re-include keyed to the true next date escapes the guard. See the original thread for the full finding and probe.
中文说明
[Critical] R22-4：在本 head 仍然成立——审计代码与第 22 轮探针时逐字节一致（仅 config.ts 因 main 合并变化）。"下一日历日"探针以 Date.now() + 24h 计算明天（此处与 files-plan.ts:1034 的重复算术同）——在 DST 回拨日（25 小时日）该时刻在第一个墙钟小时内仍停留在当前日历日，真正的下一日期永远不会被探测；以真正下一日期为键的名选择性 re-include 会逃过守卫。完整发现与探针见原线程。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| it('records symlinks and never follows them', () => { | ||
| symlinkSync(join(dir, 'src', 'a.ts'), join(dir, 'src', 'link.ts')); | ||
| mkdirSync(join(dir, 'real'), { recursive: true }); | ||
| symlinkSync(join(dir, 'real'), join(dir, 'dirlink')); |
There was a problem hiding this comment.
[Critical] R20-1: still stands — code unchanged since round 20; re-verified at this head. Second unguarded directory-symlink fixture: 'records symlinks and never follows them' (symlinkSync(join(dir, 'real'), join(dir, 'dirlink'))) while every sibling symlink fixture is skipIf(win32)-guarded. On the merge-queue Windows leg, untyped directory-symlink creation requires elevation/Developer Mode and fails the whole file. See the :1017 thread (R19-5) for the full finding.
中文说明
[Critical] R20-1：仍然成立——代码自第 20 轮未变，已在本 head 复核。第二个未加防护的目录符号链接 fixture：'records symlinks and never follows them'（symlinkSync(join(dir, 'real'), join(dir, 'dirlink'))），而同文件所有兄弟符号链接 fixture 均有 skipIf(win32) 防护。在合并队列的 Windows 段，无类型目录符号链接创建需要管理员权限/开发者模式，会导致整个测试文件失败。完整发现见 :1017 线程（R19-5）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const real = join(dir, 'real-root'); | ||
| mkdirSync(real, { recursive: true }); | ||
| const link = join(dir, 'link-root'); | ||
| symlinkSync(real, link); |
There was a problem hiding this comment.
[Critical] R19-5: still stands — code unchanged since round 19; re-verified at this head. 'resolves a symlinked target to its real path' creates a directory symlink via untyped symlinkSync and is NOT skipIf(win32)-guarded, while every other symlink fixture in this file is guarded. On the merge-queue Windows leg, directory-symlink creation requires elevation/Developer Mode, and Node 22's default type is 'file' — the fixture fails the whole file.
中文说明
[Critical] R19-5：仍然成立——代码自第 19 轮未变，已在本 head 复核。'resolves a symlinked target to its real path' 以无类型 symlinkSync 创建目录符号链接且没有 skipIf(win32) 防护，而同文件其他所有符号链接 fixture 均有防护。在合并队列的 Windows 段，目录符号链接创建需要管理员权限/开发者模式，且 Node 22 默认类型为 'file'——该 fixture 会使整个文件失败。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| * path, including under vendor/ and the path root: dependency installs, | ||
| * tooling output, and the tool's own artifact class — unless git tracks | ||
| * files inside the directory (see dirHasTrackedFiles). */ | ||
| const ALWAYS_EXCLUDED_DIRS = new Set([ |
There was a problem hiding this comment.
[Critical] R21-28: still stands — re-probed at this head in round 22; code unchanged since. With QWEN_HOME placed inside the audited tree (user-settable, and guard-check itself names in-worktree fallback roots), a run whose in-repo landing is refused relocates artifacts to <repo>/.qwen-home/audits/<hash>/; the NEXT qwen audit plan-files <repo> enumerates that subtree as source subjects — .qwen-home is absent from ALWAYS_EXCLUDED_DIRS and no root-segment guard covers it.
中文说明
[Critical] R21-28：仍然成立——第 22 轮已在本 head 复测，此时代码未变。当 QWEN_HOME 位于被审计树内（用户可设置，guard-check 自身也会给出树内的 fallback 根）时，树内落盘被拒绝的运行会把产物搬迁到 <repo>/.qwen-home/audits/<hash>/；下一次 qwen audit plan-files <repo> 会把该子树枚举为源码主体——.qwen-home 不在 ALWAYS_EXCLUDED_DIRS 中，也没有根段守卫覆盖它。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const insideArtifacts = | ||
| rootSegments[i] === '.qwen' && | ||
| (rootSegments[i + 1] === 'audits' || rootSegments[i + 1] === 'tmp'); | ||
| if (insideArtifacts || (rootSegments[i] === '.git' && i > 0)) { |
There was a problem hiding this comment.
[Critical] R21-24: still stands — re-verified at this head. The never-walk-git-internals guard matches only the literal name .git (in ALWAYS_EXCLUDED_DIRS and this descent skip), so bare-repository directories (*.git) are enumerated and content-read as source subjects, violating the unconditional invariant stated in the walk's own comment ('.git stays excluded unconditionally').
中文说明
[Critical] R21-24：仍然成立——已在本 head 复核。"永不遍历 git 内部"守卫只匹配字面名 .git（ALWAYS_EXCLUDED_DIRS 与本次下降跳过均如此），因此裸仓库目录（*.git）会被枚举并作为源码主体读取内容，违反遍历自身注释声明的无条件不变量（'.git stays excluded unconditionally'）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| * and a linked worktree alike, and does not dirty the tracked .gitignore). | ||
| * Returns the exclude file path written. */ | ||
| export function applyExcludeRemedy(projectRoot: string): string { | ||
| const commonDir = git(projectRoot, ['rev-parse', '--git-common-dir']); |
There was a problem hiding this comment.
[Critical] R19-3: still stands — re-probed at this head in round 22; code unchanged since. applyExcludeRemedy resolves the exclude file through rev-parse --git-common-dir without checking that the resolved common dir belongs to the audited repository — a planted regular-file .git containing gitdir: <victim>/.git re-homes the remedy: the rules are appended to the VICTIM repo's exclude file while the audited tree stays unremediated.
中文说明
[Critical] R19-3：仍然成立——第 22 轮已在本 head 复测，此时代码未变。applyExcludeRemedy 通过 rev-parse --git-common-dir 解析 exclude 文件，却不校验解析出的 common dir 是否属于被审计仓库——植入含 gitdir: <victim>/.git 的常规文件 .git 即可把补救重定向到受害者仓库：规则被追加进受害者仓库的 exclude 文件，被审计树仍未被补救。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| // Valid JSON is not yet a Sidecar: a wrong-shape file ({} / {"meta":{}}) | ||
| // must hit the same friendly corruption error, not crash driftCheck with | ||
| // a raw TypeError. | ||
| const isPlainRecord = (v: unknown): boolean => |
There was a problem hiding this comment.
[Critical] R22-10: still stands — verified by trace at this head; code unchanged since round 22. sidecar.json is trusted as the run-start drift baseline with no integrity mechanism, and its default landing (.qwen/audits/audit-<ts>.sidecar/) is INSIDE the audited tree — writable by the audit's own agents. loadSidecar gates only readability and JSON shape; the audited agent can rewrite baselines to hide its own drift, and drift-check has nothing to detect the tamper against.
中文说明
[Critical] R22-10：仍然成立——已在本 head 以追踪验证，代码自第 22 轮未变。sidecar.json 被当作运行起始漂移基线信任，却没有任何完整性机制，且其默认落盘位置（.qwen/audits/audit-<ts>.sidecar/）在被审计树内部——审计自身的 agent 可写。loadSidecar 只校验可读性与 JSON 形状；被审计 agent 可以改写基线以掩盖自身漂移，而 drift-check 没有任何依据发现该篡改。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| 'audits', | ||
| getProjectHash(resolved), | ||
| ); | ||
| fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); |
There was a problem hiding this comment.
[Critical] R21-25: still stands — re-probed at this head in round 22; code unchanged since. getAuditFallbackDir establishes the relocation landing with a bare mkdirSync(recursive, mode 0o700) at a path fully predictable to the audited agent (QWEN_HOME/audits/<sha256(projectRoot)>), never validating a pre-existing leaf — and mkdirSync's mode applies only to directories it CREATES. Round-22 probe: a leaf pre-planted as a symlink to a directory is silently followed (writes land in the symlink target); a pre-planted 0777 dir is never tightened.
中文说明
[Critical] R21-25：仍然成立——第 22 轮已在本 head 复测，此时代码未变。getAuditFallbackDir 以裸 mkdirSync(recursive, mode 0o700) 在对被审计 agent 完全可预测的路径（QWEN_HOME/audits/<sha256(projectRoot)>）建立搬迁落点，从不校验已存在的叶子——且 mkdirSync 的 mode 只作用于它新建的目录。第 22 轮探针：预先植入的指向目录的符号链接叶子会被静默跟随（写入落入符号链接目标）；预先植入的 0777 目录永远不会被收紧。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
🤖 AutoFix deferred this item to a human under instruction (round 29/100) — the agent's handoff note below names the decision and the options. The loop stays engaged and still picks up new feedback and base conflicts, but will not retry this item on its own. What I found before stopping: Handoff: PR #8403 is not converging — needs a maintainer decisionThe decisionThis PR has stayed over the diff-growth budget for 24 consecutive rounds (source 575 / test 1511 net lines vs budgets of 400/400) and is still not shrinking. The remaining actionable feedback consists entirely of Critical findings, and fixing them is what drives the growth — each fix adds hardening code plus tests, which repeatedly re-triggers new findings in the same class. Critical-only mode cannot help because the Criticals ARE the growth. Per the convergence rule, no further code fixes are being applied; the way forward is a maintainer call, not another patch round. State of the PR
中文说明🤖 AutoFix 已按指示将此项移交人工处理（第 29/100 轮）—— 下方 agent 的 handoff 说明列出了待决决策与各选项。循环保持在线，仍会拾取新反馈与 base 冲突，但不会自行重试此项。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/32373928290 🧠 Handled by Qwen Code · model/模型 |
|
Superseded by #9749 — closing this one. This PR accumulated 24 review rounds and ~1,400 comments, which made the remaining work impossible to follow: the round-23 ledger carried 24 open Critical findings, most of them re-posts of the same classes, and the autofix loop deferred it as non-converging (each per-entrance fix added a guard plus its tests, which re-triggered the class). #9749 is the same work with every one of those Criticals addressed, as one commit. It is roughly 2,000 lines smaller than this PR's head, because the two largest fixes are subtractive rather than additive:
The remaining Criticals are fixed at their roots: Two findings are accepted rather than closed, by maintainer decision, and every audit report header now says so: R18-3 — the estimate's per-line rates are author-reported and their raw records are not in this repository; R22-10 — the sidecar is not tamper-evident, so it detects accidental drift rather than a module actively hiding it (a run auditing genuinely untrusted code should land artifacts outside the repository, which the local-only guard already offers). Please continue review on #9749. |
wenshao
removed
the
autofix/takeover
|
👋 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（在飞的一轮如有，将完成其有界工作）。重新打上 |
6 participants
What this PR does
This PR implements the legacy-code audit workflow designed in #8397 (now merged). It adds
/audit <directory> [--effort low|medium|high]for auditing existing modules without a diff or pull request, together with deterministic CLI helpers for 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 new bundled skill; the roster, gates, budget estimate, and captures are computed by code so they cannot be shrunk or skipped by the orchestrator.The default medium tier runs the nine dimension agents validated by two A/B experiments (1a, 1c, 2, 3a/3b/3c, 4, 5, 6a) 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 tests, and agent-authored verification probes against scratch copies) are separate opt-ins at an interactive pre-launch confirmation, and non-interactive starts are refused.
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 or author intent to rely on. Two controlled experiments found substantially more confirmed defects with dimension fan-out than with a naive single-agent review, while maintaining 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/), and the two privategit check-ignorecopies consolidated into one fresh-by-default helper inpackages/core/src/utils/(consumed by review's test-plan, team memory, and the audit guard), plus 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 (32.5M–42.3M and 46M–59.8M tokens), and hooks — but not permissions — is flagged as an event/lifecycle module.packages/core) exits 3 naming the subject gate;packages/core/distrefuses with "only excluded directories"; a gitlink under the audited path refuses naming the submodule rule; over-60M-corner plans refuse at the cap. Confirm low refuses above 2,000 subject lines and points at medium.unprotectedwith the exclude remedy offered; applying--apply-exclude-remedyflips the probe to ignored without touching the tracked.gitignore; a force-added file under.qwen/tmp/→tracked; a.qwen/*+!**re-include → the in-repo landing is refused in favor of the outside-repo fallback./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 (probe-backed included) 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.qwen audit snapshotcaptures the path-scoped diff, untracked content copies, and content hashes;drift-checkattributes content drift per file (a content-preserving HEAD move stops nothing);check-anchorsrefuses findings whose snippet does not resolve uniquely, including against registered out-of-path callers.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 bundled skill. Both design-doc calibration modules reproduce their measured line counts and price inside the estimate brackets at plan time; the event-detection fixture pair (permissions / hooks) resolves in the documented directions; the gate, guard, and refusal behaviors above are covered by unit tests, including the precision case where coarsely rounded rates would refuse the hooks module.
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, submodule refusal, and sidecar/drift arms), repository build, TypeScript type checking, ESLint, and Prettier.
Risk & Scope
docs/design/assets/) lands separately from the author's machine — the pinned constants follow the merged doc's quoted rates.Linked Issues
Implements #8397.
中文说明
本 PR 做什么
实现 #8397（已合入）设计的存量代码审计工作流：新增
/audit <directory> [--effort low|medium|high]，在无 diff、无 PR 的情况下审计现有模块，并配套确定性的 CLI helper——参数解析、文件系统规划、agent brief 打印、运行态捕获、drift 检查点、local-only 守护和写入时 anchor 解析。编排在新的 bundled skill 中；roster、门限、预算估算和捕获全部由代码计算，orchestrator 无法缩减或跳过。默认 medium 档运行两轮 A/B 实验验证过的九个维度 agent（1a、1c、2、3a/3b/3c、4、5、6a）加验证分片；high 增加 6b/6c persona 和按文件组分片的迭代反向审计；low 是单个 reader 子代理，产出限 10 条、明确标注未验证的分诊结果。每个模块内容消费者——各 agent、验证者、去重聚类器、low reader、orchestrator 本身——都以 untrusted-data 前置声明开头。walk 全部只读；两类执行（模块自身测试套件的基线运行、针对 scratch 副本的 agent 编写验证探针）在交互式启动前确认中分别选择加入，非交互启动一律拒绝。
为什么需要
现有
/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（review 的 test-plan、team memory 和 audit 守护共同消费），以及 Storage hub 上的 per-user 仓库外产物回退目录。风险与范围
docs/design/assets/下的脱敏实验记录）由作者机器单独提交——代码中的常量遵循合入文档所引用的费率。