fix(core): allow scoped Git credential config - #9870
Conversation
Credentialed HTTPS Extension Git E2E ReportScopeValidate one-time credential installation, stored credential installation, and the update check performed after restart without exposing a real credential. Baseline
AfterOne-time credential installation path
Stored credential installation and update path
Anonymous public path
Commands
Result
|
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: observed bug, not theoretical. Credentialed HTTPS extension installs and update checks currently die before Git even spawns — Direction: aligned. This repairs qwen-code's own recently added extension credential handling rather than adding new surface — the security posture is unchanged: same explicit child-env whitelist, same header scoped to the exact repository URL, and the new unsafe category is only enabled when Qwen Code itself constructs the credential environment. CHANGELOG has no direct reference, but the area is obviously relevant. Size: touches core ( Approach: scope feels right. Both call sites (install clone and post-restart update check) carried the same latent bug, so centralizing client construction into one helper is the minimal fix, not a drive-by refactor; I don't see a hunk that could be cut. One question the new test file answers well: it drives the real Risk: no elevated risk signals — no revert-correlated paths are touched. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：是已观测到的 bug，不是理论性问题。带凭证的 HTTPS Extension 安装与更新检查目前会在 Git 启动前就失败—— 方向：对齐。这是修复 qwen-code 自己新近加入的 Extension 凭证处理，而不是引入新面——安全姿态不变：子进程环境仍是显式白名单，Header 仍精确限定在仓库 URL，新的 unsafe 类别只在 Qwen Code 自己构造凭证环境时才启用。CHANGELOG 无直接引用，但该领域显然相关。 规模：触及核心（ 方案：范围合理。两个调用点（安装 clone 与重启后更新检查）都带有同一个潜在 bug，因此把客户端构造集中到一个 helper 是最小修复，不是顺手重构；没有看到可以砍掉的改动块。新测试文件很好地回答了一个问题：它用真实 风险：无升级风险信号——未触及与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewBefore reading the diff I sketched my own fix: compute the union of What I verified in detail:
One honest limitation of the test suite, non-blocking: the "does not inherit Git config count" case asserts only that the command succeeds — a stray inherited config trio would not break Test evidence (PR's own CI — I did not run any PR code)The main CI run is still in flight on the reviewed commit; nothing has failed so far. The macOS/Windows unit jobs and the CLI integration job report skipped rather than failed. Details in the table below — the finalize job will refresh it once the run settles. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Not verified: live credentialed install/update against a real private repository — nobody has run that, since it needs a disposable credential (the author states none was available; that is their claim, and it is inherent to the test environment, not a shortcut). The author's local testing covered macOS only, per the PR body. Sandboxed verification would settle the remaining gap: 中文说明代码审查在看 diff 之前我先独立勾画了自己的修复方案：根据客户端自身配置的内容计算 逐项核实：
测试套件一个诚实的局限（非阻塞）："不继承 Git config count" 用例只断言命令成功——偶然继承的凭证三件套并不会让 测试证据（PR 自身 CI——未运行任何 PR 代码）主 CI 在受审提交上仍在进行中，目前没有失败。macOS/Windows 单元测试与 CLI 集成任务显示为 skipped 而非 failed。表格由 finalize 任务在 CI 结束后刷新。未验证：针对真实私有仓库的带凭证安装/更新——需要有可丢弃的凭证，任何人都没有跑（作者声明没有可用凭证，这是其自述，属于测试环境的固有限制）。作者本地仅测试了 macOS（PR 正文自述）。沙箱验证可补齐剩余缺口： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — clean across every stage; a small, correct fix for a hard failure in our own extension credential path. My independent proposal for this bug was exactly what the PR does — union the The one thing keeping this from an immediate approval is CI: the main run is still in flight on the reviewed commit (nothing red so far). Approval deferred until CI lands green on 中文说明置信度：5/5 —— 各阶段均干净；这是对自有 Extension 凭证路径硬故障的小而正确的修复。 我对这个 bug 的独立方案与 PR 完全一致——根据客户端自身配置的内容取 唯一阻止立即批准的是 CI：主运行在受审提交上仍在进行（目前没有红色）。批准推迟到 CI 在 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
One blocking correctness issue remains on 34fce2d8: valid credentialed repository URLs can still be rejected by the simple-git safety plugin before Git starts. See the inline finding for the reproduction and requested regression coverage.
|
@doudouOUC The blocking review on 34fce2d has been addressed on the current head 0699929. The valid-URL false positives now have real-process regressions for both alias and credential.helper text, plus a negative regression proving the scoped alias allowance does not permit unrelated credential.helper configuration. The original thread is resolved and all current CI checks are green; please submit a replacement review so the stale CHANGES_REQUESTED decision can be cleared. |
Dismissed as stale after the blocking finding on 34fce2d was fixed in 0699929. The current head adds real-process regressions for alias and credential.helper URL false positives plus a negative test that unrelated unsafe Git configuration remains rejected; the review thread is resolved and no unresolved findings remain.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 6b": none — no check was cut short..
Test Plan (not a blocker): tests: 3 passed — this review observed 21297, 1688, 23616, 1654, 496, 4226, 626 passed; tests: 150 passed — this review observed 21297, 1688, 23616, 1654, 496, 4226, 626 passed.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 6b"：none — no check was cut short.。
Test Plan（非阻断）：tests: 3 passed — this review observed 21297, 1688, 23616, 1654, 496, 4226, 626 passed; tests: 150 passed — this review observed 21297, 1688, 23616, 1654, 496, 4226, 626 passed。
— qwen3.8-max via Qwen Code /review (v0.22.0)
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 — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the unit suite including the new extension-git-client tests ran only on ubuntu-latest and locally on Linux.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/core/src/extension/extension-git-client.test.ts:113 — [review] 'does not inherit Git config count' test never observes the child environment; the isolation guarantee it is named after is unpinned (a ||→&& mutation survives the test…
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the unit suite including the new extension-git-client tests ran only on ubuntu-latest and locally on Linux。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
Dismissed as stale after Critical R2-1 on c9fd421 was fixed in 2224d72. Credentialed sources now require a valid HTTPS URL, and option-shaped refs are rejected before simple-git/client creation in both clone and update paths. Four new regressions failed before the fix and now pass; 160 focused tests, Core typecheck/build, ESLint, and diff checks are green. The repeated non-blocking dependency-drift suggestion remains explicitly deferred and both threads are resolved.
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. Suggestions are inline.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the unit suite including the new extension-git-client tests ran only on ubuntu-latest and locally on Linux.
Test Plan (not a blocker): tests: 3 passed — this review observed 1688, 23666, 1658, 601, 4227, 626 passed; tests: 150 passed — this review observed 1688, 23666, 1658, 601, 4227, 626 passed.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/core/src/extension/extension-git-client.test.ts:51 — [probe] no test pins that allowUnsafeProtocolOverride is absent without networkConfig — the unconditional-assignment mutation survives the whole suite (D3-1, deferred under the r…packages/core/src/extension/extension-git-client.ts:111 — [probe] no test pins that anonymous non-public clients skip the restricted env — deleting the early return survives the whole suite (D3-2, deferred under the round-3 code-age rule)
Convergence: round 3 posted 2 inline comment(s), 1 of them reported for the first time; the previous round posted 2 (1 new). The rate of new findings is not falling. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the unit suite including the new extension-git-client tests ran only on ubuntu-latest and locally on Linux。
Test Plan（非阻断）：tests: 3 passed — this review observed 1688, 23666, 1658, 601, 4227, 626 passed; tests: 150 passed — this review observed 1688, 23666, 1658, 601, 4227, 626 passed。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 3 轮发布了 2 条行内评论，其中 1 条是首次提出；上一轮发布了 2 条（其中 1 条首次提出）。新发现的产出速度没有下降。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。本轮没有未决的 Critical，因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Round 4 review
This round confirms the same findings as the prior rounds: no Criticals, 1 Suggestion.
What changed since round 3 (HEAD 2224d72)
The head is unchanged since round 3. The two open Suggestions from round 3 (R3-1 and R1-1) remain standing. One new finding follows.
Findings
Suggestion (R4-1): The JSDoc comment for cloneFromGit is attached to resolveGitRef instead.
The JSDoc block /** Clones a Git repository ... @param installMetadata ... @param destination */ at line 217 is placed directly above resolveGitRef (line 221), but its text and parameter list describe cloneFromGit (which has installMetadata and destination parameters). The JSDoc should either be moved to above cloneFromGit (line 230), or removed — per the codebase's "comments default to none" convention, the helper is simple enough to not need one.
Convergence
Round 4 posts 1 new finding (R4-1). The previous round posted 2 (1 new). The rate of new findings continues to fall. No Critical finding is open on this round, so merging is available.
Verification
- Build:
packages/corebuilds successfully. Theaudio-capturebuild failure on this Windows machine is pre-existing (missing Python for node-gyp). - Real-process tests:
extension-git-client.test.tsspawns real Git through all three option shapes — the fix is pinned by an actual spawn, not a mock. - Credential isolation: byte-for-byte identical to the removed
restrictGitEnvironment— the child-env whitelist and header construction are unchanged.
|
@qwen-code /takeover from 3 |
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. This window's round counter starts at 3 (the rounds this PR spent in review before takeover), so the Critical-only brake engages after 2 more change-producing round(s) instead of a full fresh 5. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本窗口轮次计数从 3 起算（即本 PR 托管前已进行的评审轮数），因此再经过 2 个产生改动的轮次即进入 Critical-only，而非重新计满 5 轮。移除 |
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Review round summary — PR #9870This round addresses the two open Suggestion threads from rounds 3–4. Both fixes are doc-level (a declaration move and one provenance comment); no behavior changed, so the existing focused suite is the witness and no new guards or tests were added. Feedback triage[rc:3844415488] R3-1 / [rv:5009283411] R4-1 — [rc:3844415497] R1-1 re-post — hand-maintained mirror of the argv-parser blocklist (Suggestion) — Addressed, resolved. Implemented the minimal action this re-post proposes: a provenance comment above Informational, no action: rv:5005453982, rv:5006217349, rv:5006218451, rv:5007551447, rv:5007552227 carry no new actionable text; ic:5391616909 is the credentialed E2E report; ic:5392438560 requests a replacement review. The round-2 Critical (R2-1, self-approval through the blocklist) was already fixed in 2224d72 and the comment-invariant finding (R1-2) in bd39137; both remain fixed on this head. Changes
No conflicts ( Verification
中文说明评审轮次总结 — PR #9870本轮处理第 3–4 轮遗留的两条 Suggestion 线程。两处修复均为文档层面（移动函数声明位置 + 一条来源注释），不改变任何行为，因此既有的聚焦测试套件即为见证，未新增任何守卫或测试。 反馈分类处理[rc:3844415488] R3-1 / [rv:5009283411] R4-1 — [rc:3844415497] R1-1 重发 — 手工维护的 argv-parser 黑名单镜像（Suggestion）— 已处理，已解决。 实现了本次重发所提出的最小动作：在 信息性条目，无需处理： rv:5005453982、rv:5006217349、rv:5006218451、rv:5007551447、rv:5007552227 无新的可操作内容；ic:5391616909 为带凭证 E2E 报告；ic:5392438560 为请求重新评审。第 2 轮的 Critical（R2-1，经由黑名单自我批准）已在 2224d72 修复，注释不变量发现（R1-2）已在 bd39137 修复，二者在当前 head 上仍保持已修复状态。 变更内容
无冲突（ 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
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 — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the unit suite including the extension-git-client tests ran only on ubuntu-latest and locally on Linux.
Test Plan (not a blocker): tests: 3 passed — this review observed 21321, 1688, 23663, 1658, 601, 4227, 626 passed; tests: 150 passed — this review observed 21321, 1688, 23663, 1658, 601, 4227, 626 passed.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/core/src/extension/github.ts:217 — [probe] single-dash half of the resolveGitRef dash guard is pinned by no test — the startsWith('-') -> startsWith('--') mutant survives the whole suite (D4-1, deferred under the round-4 code-age r…
Convergence: round 4 posted 2 inline comment(s), 1 of them reported for the first time; the previous round posted 2 (1 new). Findings keep coming back to the same files: packages/core/src/extension/extension-git-client.ts (findings in round 1; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the unit suite including the extension-git-client tests ran only on ubuntu-latest and locally on Linux。
Test Plan（非阻断）：tests: 3 passed — this review observed 21321, 1688, 23663, 1658, 601, 4227, 626 passed; tests: 150 passed — this review observed 21321, 1688, 23663, 1658, 601, 4227, 626 passed。
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 4 轮发布了 2 条行内评论，其中 1 条是首次提出；上一轮发布了 2 条（其中 1 条首次提出）。发现反复回到同一批文件：packages/core/src/extension/extension-git-client.ts（第 1 轮已出过发现，本轮又有 1 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
) The restricted child environment replaced the spawn environment wholesale with a small allowlist, so a PATH-resolved git wrapper that needs a non-allowlisted indirection variable failed every spawned command ("exec: : not found"), breaking both the unit suite and real credentialed installs in such environments. Inherit the parent environment minus Git behavior variables (git_*), home locations, proxies, TLS trust overrides, loader-injection keys, and the non-GIT_* keys @simple-git/argv-parser flags as unsafe. This keeps wrapper indirection variables flowing while ambient Git configuration, program overrides, and network redirection still never reach the spawned client. Adds a regression test that spawns git through a PATH wrapper requiring a non-allowlisted variable.
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #9870Addressed the one Critical finding (R4-1) in code and deferred the repeated Suggestion (R1-1) with evidence. No base-conflict merge was needed ( Feedback points and dispositions[rv:5011653726] [rc:3846685081]
[rc:3846685088] Changes
Net: Conflict notesNone. VerificationCommands actually run this round, and their results:
中文说明Autofix 评审轮次 — PR #9870本轮在代码中解决了一个 Critical 发现（R4-1），并带证据延后处理了重复出现的 Suggestion（R1-1）。无需合并 base 冲突（ 反馈点及处理[rv:5011653726] [rc:3846685081]
[rc:3846685088] 变更
净变更： 冲突说明无。 验证本轮实际执行的命令及结果：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
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 — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the unit suite including the new extension-git-client tests ran only on ubuntu-latest and locally on Linux.
Test Plan (not a blocker): tests: 3 passed — this review observed 21327, 1688, 23663, 1658, 601, 4227, 626 passed; tests: 150 passed — this review observed 21327, 1688, 23663, 1658, 601, 4227, 626 passed.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/core/src/extension/extension-git-client.ts:145 — [review] factory comment condemns its only callers — the derived '-c' entries warning reads as stale because both call sites pass extension-derived positional argumentspackages/core/src/extension/extension-git-client.ts:155 — [review] no test pins that unrestricted (legacy) clients skip the restricted env — deleting the early return ships green (round-3 D3-2 rediscovery)packages/core/src/extension/github.ts:219 — [review] resolveGitRef centralizes the ref default for two of three occurrences; downloadPublicGitHubArchiveFallback still spells 'installMetadata.ref || HEAD' inline
Convergence: round 5 posted 3 inline comment(s), 2 of them reported for the first time; the previous round posted 2 (1 new). Findings keep coming back to the same files: packages/core/src/extension/extension-git-client.ts (findings in rounds 1, 4; 2 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the unit suite including the new extension-git-client tests ran only on ubuntu-latest and locally on Linux。
Test Plan（非阻断）：tests: 3 passed — this review observed 21327, 1688, 23663, 1658, 601, 4227, 626 passed; tests: 150 passed — this review observed 21327, 1688, 23663, 1658, 601, 4227, 626 passed。
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 5 轮发布了 3 条行内评论，其中 2 条是首次提出；上一轮发布了 2 条（其中 1 条首次提出）。发现反复回到同一批文件：packages/core/src/extension/extension-git-client.ts（第 1、4 轮已出过发现，本轮又有 2 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 6/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 6/100 轮）。改动内容与我反驳保留之处如下： PR #9870 — review round summaryAll three inline findings in this round were verified against the code and resolved in one commit ( Feedback dispositions[rc:3847985560] R5-1 — Critical — RESOLVED (fixed in code)Claim: Reproduced independently on
Fix (as suggested): the assert now requires [rc:3847985575] R5-2 — Suggestion — RESOLVED (fixed in code)Claim: the allowlist→blocklist switch lets ambient Reproduced: with Fix: added [rc:3847985578] R1-1 — Suggestion (re-posted from round 4) — RESOLVED (mechanical guard implemented)Claim: the Implemented the parity-test form of the requested guard (deriving the allowances at runtime was rejected: it would require declaring a new direct dependency and touching the lockfile, outside this PR's footprint). New parity tests compare, for a corpus covering all 23 matcher literals plus dash-form variants and a negative control, the unsafe categories the factory enables (captured via a stub [rv:5013176082] Review body (CHANGES_REQUESTED, "Partially reviewed — gaps disclosed") — no action requestedThe review's actionable content is the three inline comments above, all resolved. The remaining disclosures require no code change this round: the macOS/Windows CI test skips are CI scheduling (the unit suite ran on ubuntu-latest and locally on Linux — this round also ran it on Linux), the Test Plan observations are informational ("not a blocker"), and the three items deferred under the convergence posture (factory comment wording at line 145, missing unrestricted-client env test, Changed files
Verification
中文说明PR #9870 — 评审轮次总结本轮的三条行内发现均已对照代码核实，并在一个提交（ 反馈处理结果[rc:3847985560] R5-1 — Critical — 已解决（代码修复）论断： 已在
修复（按建议实施）：断言现在要求 [rc:3847985575] R5-2 — Suggestion — 已解决（代码修复）论断：白名单改黑名单的切换让环境中的 已复现：设置 修复：向 [rc:3847985578] R1-1 — Suggestion（自第 4 轮重发）— 已解决（已实现机械化防护）论断： 实现了所请求防护中的奇偶校验测试形式（在运行时从 parser 推导放行类别的方案被否决：那需要声明新的直接依赖并改动 lockfile，超出本 PR 的范围）。新的奇偶校验测试针对覆盖全部 23 个匹配器字面量、外加破折号变体与一个阴性对照的语料库，比较工厂启用的 unsafe 类别（通过桩 [rv:5013176082] 评审正文（CHANGES_REQUESTED，“Partially reviewed — gaps disclosed”）— 本轮无需处理该评审可执行的内容即上方三条行内评论，均已解决。其余披露本轮不需要代码改动：macOS/Windows 的 CI 测试跳过属于 CI 调度（单元测试套件已在 ubuntu-latest 和本地 Linux 上运行——本轮同样在 Linux 上运行）；Test Plan 观察为信息性（“非阻断”）；收敛姿态下延后的三项（第 145 行工厂注释措辞、缺失的非受限客户端环境测试、 变更文件
验证
Deferred non-Critical feedbackCritical-only mode is active: the round counter reached 5 (this window was seeded at round 3 by 中文说明已进入仅处理 Critical 的模式：轮次计数已达 5（本窗口由 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
Dismissed as stale on current head 56eef4d. The blocking CI-wrapper finding is addressed without broadening production inheritance: restricted Git children retain the explicit environment allowlist, while real-process tests use QWEN_CI_REAL_GIT only as an absolute test binary. The focused extension suite passes normally and with QWEN_CI_REAL_GIT=/usr/bin/git, and the associated thread is resolved.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
Test Plan (not a blocker): tests: 3 passed — this review observed 21264, 1688, 23666, 1658, 601, 4227, 626 passed; tests: 150 passed — this review observed 21264, 1688, 23666, 1658, 601, 4227, 626 passed.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/core/src/extension/extension-git-client.ts:33 — [probe] href round-trip rejects case/port-variant HTTPS URLs that getGitHubCredential still attaches credentials topackages/core/src/extension/extension-git-client.test.ts:185 — [probe] env oracle never pins the restricted child environment's key set; an allowlist-addition mutant survives the suite
中文说明
无阻断问题。LGTM！✅
Test Plan（非阻断）：tests: 3 passed — this review observed 21264, 1688, 23666, 1658, 601, 4227, 626 passed; tests: 150 passed — this review observed 21264, 1688, 23666, 1658, 601, 4227, 626 passed。
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix address-review — no code changes this roundCritical-only mode is active (round counter at 5), and this round's actionable feedback contained no new change requests: all 7 inline items and the 7 accompanying review submissions are status records from @callmeYe referencing head Re-verified as resolved in code (listed in
|
Local verification — real credentialed HTTPS extension installsI built a real end-to-end environment for this change and ran the PR's whole Reviewer Test Plan on Linux, plus a fuzz/negative-control pass on the new blocklist mirror table. Everything the PR claims reproduces. I recommend merging. Two non-blocking observations at the bottom. Verified at head Environment
Results
The blocklist mirror table is exactly right, and it is load-bearing
Real clones through 23 colliding repository names.
Differential fuzz against the shipped parser. I recovered all 23 matcher literals from Residual surface, measured. The code comment warns that enabled categories relax the blocklist for the whole client. That is accurate and minimal — the relaxation is per-client and per-category, with no cross-category bleed:
And the invariant that comment relies on holds: the only arguments reaching these clients are
Also ran
Two notes, neither blocking1. A second failure mode is fixed that the description does not mention. Credentialed installs without pinned network config ( 2. 中文说明本地验证 —— 真实带凭证 HTTPS Extension 安装我为这个改动搭了一套真实的端到端环境，在 Linux 上跑完了 PR 的整个 Reviewer 测试计划，另外对新增的 blocklist 镜像表做了 fuzz 和反向对照。PR 声称的每一条都能复现，建议合并。 两条不阻塞的观察放在最后。 验证对象为 head 环境
结果
镜像表完全正确，而且确实在起作用
用 23 个会撞上匹配器的仓库名跑真实 clone。
针对实际依赖做差分 fuzz。 我用机械方式从 残余面，实测。 代码注释警告"被启用的类别会对该 client 的所有任务放宽 blocklist"。这个说法准确且是最小的 —— 放宽是按 client、按类别的，没有跨类别外溢：
而且该注释依赖的不变量成立：这些 client 上只有 另外跑了
两条观察，都不阻塞1. 顺带修掉了描述里没提的第二种失败形态。 没有 pinned network config 的带凭证安装（ 2. Verified with Claude Code (Claude Opus 5). |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round: no code changesFeedback dispositionThis round's feedback contains no reviews, no inline comments, and no failed or
Critical-only mode is active for this window (round counter reached 5), so No code changes were made and no commits were created. No verification 中文说明Autofix 轮次：无代码改动反馈处理本轮反馈中没有 review、没有行内评论，也没有失败或持续失败的检查。唯一的
本窗口已进入仅处理 Critical 的模式（轮次计数已达 5），本轮只会实施 Critical 本轮没有做任何代码改动，也没有创建提交。由于工作区没有任何变化，无需运行 Deferred non-Critical feedbackCritical-only mode is active: the round counter reached 5 (this window was seeded at round 3 by 中文说明已进入仅处理 Critical 的模式：轮次计数已达 5（本窗口由 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
Released in v0.22.2. |
6 participants
What this PR does
This change centralizes construction of the Git client used by HTTPS extension clone and update operations. It authorizes Git configuration paths for restricted public or authenticated operations, protocol overrides for pinned network configuration, and Git config-count environment variables only when Qwen Code itself creates the scoped credential header.
The credential header remains scoped to the exact clean repository URL. The child Git process still receives only the existing environment whitelist, and credentials remain outside repository URLs, command arguments, logs, errors, and extension metadata.
Why it's needed
Credentialed HTTPS extension installs and update checks currently create
GIT_CONFIG_COUNT=1for an exact-scope authorization header, butsimple-gitrejects that environment before spawning Git unless the corresponding safety category is explicitly enabled. Users therefore see an install failure even though Qwen Code generated and isolated the configuration itself.Reviewer Test Plan
How to verify
GIT_CONFIG_COUNTsafety error while the clone URL remains credential-free.GITHUB_TOKEN; confirm it follows the same scoped configuration path and the token is absent from the clone URL and arguments.GIT_CONFIG_COUNT; confirm the child process ignores that external configuration and does not enable the config-count safety category.Evidence (Before & After)
Before:
After:
Tested on
Environment (optional)
macOS 15.1.1, Node.js 22.17.0, Git 2.53.0. The real-process regression uses the installed Git binary and no network access.
Risk & Scope
Linked Issues
N/A
中文说明
这个 PR 做了什么
这个改动集中构造 HTTPS Extension clone 和更新操作使用的 Git 客户端。受限公网或认证操作会放行 Git 配置路径，固定网络配置会放行协议覆盖，并且只有在 Qwen Code 自己生成作用域凭证 Header 时才放行 Git config-count 环境变量。
凭证 Header 仍然精确限定在干净的仓库 URL。Git 子进程仍只接收现有环境变量白名单，凭证不会进入仓库 URL、命令参数、日志、错误或 Extension 元数据。
为什么需要
带凭证的 HTTPS Extension 安装和更新检查当前会为精确作用域的 Authorization Header 创建
GIT_CONFIG_COUNT=1，但如果未显式启用对应安全类别，simple-git会在启动 Git 之前拒绝该环境。因此，即使配置由 Qwen Code 自己生成并隔离，用户仍会看到安装失败。Reviewer 测试计划
如何验证
GIT_CONFIG_COUNT安全错误，同时 clone URL 不包含凭证。GITHUB_TOKEN安装私有 GitHub Extension；确认它经过相同的作用域配置路径，Token 不出现在 clone URL 和参数中。GIT_CONFIG_COUNT时运行匿名公网 Git 操作；确认子进程忽略外部配置，并且不会启用 config-count 安全类别。证据（Before & After）
Before:
After:
测试平台
环境（可选）
macOS 15.1.1、Node.js 22.17.0、Git 2.53.0。真实进程回归测试使用本机已安装的 Git，不访问网络。
风险与范围
关联 Issue
N/A