Skip to content

fix(security): honor explicit distrust over inherited trust - #8628

Merged
yiliang114 merged 6 commits into
QwenLM:mainfrom
daleselaji-dev:agent/most-specific-folder-trust-8627
Aug 8, 2026
Merged

fix(security): honor explicit distrust over inherited trust#8628
yiliang114 merged 6 commits into
QwenLM:mainfrom
daleselaji-dev:agent/most-specific-folder-trust-8627

Conversation

@daleselaji-dev

@daleselaji-dev daleselaji-dev commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

What this PR does

This PR makes an explicitly distrusted workspace take precedence over an inherited trusted parent when Qwen Code decides whether to load workspace settings or environment files. The same rule is applied in the regular trusted-folder path and the fast-path settings path.

Why it's needed

Problem

When a parent directory is trusted but a nested workspace is explicitly marked DO_NOT_TRUST, the nested workspace can still be treated as trusted. That can allow workspace-provided settings or environment values to be loaded unexpectedly.

Root Cause

Both trust-resolution paths checked ancestor containment before checking the exact workspace's explicit distrust marker. The exact DO_NOT_TRUST rule therefore lost to inherited trust.

Reviewer Test Plan

How to verify

  1. Mark a parent directory trusted and a nested workspace explicitly DO_NOT_TRUST.
  2. Ask the trusted-folder resolver and the fast-path settings loader about the nested workspace.
  3. Confirm that the nested workspace is not treated as trusted and its environment file is not loaded, while an explicitly trusted workspace still behaves as before.

Evidence (Before & After)

N/A — this is a non-UI security behavior change. Before the patch, the nested-workspace regression test expected inherited trust; after the patch, the same scenario returns no trust and does not load the environment file. The focused config suite passed 56 tests; the risk-matched config/daemon/fast-path run passed 129 tests with one Windows symlink EPERM environment failure in an existing canonical-path test.

Tested on

OS Status
macOS ⚠️ not tested locally
Windows ✅ tested locally; one existing symlink test is blocked by EPERM
Linux ⚠️ not tested locally

Environment (optional)

Windows PowerShell, blobless sparse checkout, Node/npm workspace tests. ESLint and the CLI workspace typecheck were not verified because the sparse environment lacks required generated SDK/channel/template artifacts and an ESLint plugin.

Solution

Check the exact DO_NOT_TRUST marker before ancestor containment trust in both trusted-folder resolution and fast-path settings resolution.

Changes

  • Updated trustedFolders.ts and fast-path-settings.ts to apply exact distrust first.
  • Updated the existing trust-order regression expectations.
  • Added fast-path coverage for an explicitly untrusted nested workspace.
  • No unrelated credential, token-exclusion, or descendant-containment policy changes are included.

Testing

  • Baseline: npm test --workspace=packages/cli -- src/config/trustedFolders.test.ts src/config/daemon-trust-policy.test.ts --run — 55 passed.
  • Focused after change: same config suites — 56 passed.
  • Targeted fast-path regression — 1 passed.
  • Risk-matched config/daemon/fast-path suites — 129 passed; 1 failed only because Windows cannot create the existing directory symlink (EPERM).
  • npx prettier --check and git diff --check passed.
  • npm run build --workspace=packages/channels/base passed.
  • ESLint and npm run typecheck --workspace=packages/cli were not verified for the environment limitations stated above.

Risk & Scope

The change is intentionally limited to exact-workspace distrust precedence; broader descendant containment is outside this PR.

Compatibility/Risk

This is a narrow security precedence fix. Explicitly untrusted nested workspaces stop inheriting trust from a trusted parent; ordinary explicit trust and unrelated descendant policy remain unchanged. A broader distrust-by-containment rule may be considered separately, but is intentionally outside this PR.

Notes for Reviewer

Please review the two resolution paths together: the regular trusted-folder path and the fast-path settings path must agree on the exact-workspace precedence rule. The Windows symlink failure is an environment limitation in an existing test, not a changed assertion. GitHub prechecks passed; repository triage/review checks are the remaining external gates.

Linked Issues

Linked Issue

Fixes #8627

中文说明

本 PR 做了什么

当 Qwen Code 判断是否加载工作区设置或环境文件时,如果嵌套工作区被明确标记为 DO_NOT_TRUST,本 PR 让该明确不信任规则优先于父目录继承的信任状态。普通可信目录路径和快速设置路径使用相同规则。

为什么需要它

问题

当父目录可信、但嵌套工作区明确标记为 DO_NOT_TRUST 时,嵌套工作区仍可能被当作可信目录,从而意外加载工作区提供的设置或环境值。

根因

两条信任解析路径都先检查祖先目录包含关系,再检查当前工作区的明确不信任标记,因此精确的 DO_NOT_TRUST 规则被继承的信任状态覆盖。

评审测试计划

如何验证

  1. 将父目录标记为可信,并将嵌套工作区明确标记为 DO_NOT_TRUST
  2. 分别检查普通可信目录解析器和快速设置加载器对该嵌套工作区的判断。
  3. 确认嵌套工作区不再被视为可信且不会加载其环境文件,同时明确可信的工作区保持原有行为。

前后证据

这是非 UI 的安全行为变更,因此不适用截图。修改前嵌套工作区回归测试预期继承父目录信任;修改后同一场景返回不可信且不加载环境文件。配置聚焦测试通过 56 项;配置、守护进程和快速路径风险匹配测试通过 129 项,另有一个既有规范路径测试因 Windows 无法创建目录符号链接而出现 EPERM 环境失败。

测试平台

Windows 本地测试通过;macOS 和 Linux 未在本地运行。Windows 的既有符号链接测试受 EPERM 环境限制。

环境

Windows PowerShell、blobless sparse checkout 和 Node/npm workspace 测试。由于缺少生成的 SDK、channel、模板产物和 ESLint 插件,ESLint 与 CLI workspace typecheck 未验证。

解决方案

在可信目录解析和快速设置解析中,都先检查当前工作区的精确 DO_NOT_TRUST 标记,再检查祖先目录的包含式信任。

修改内容

  • 更新 trustedFolders.tsfast-path-settings.ts,让精确不信任优先。
  • 更新既有信任顺序回归预期。
  • 增加明确不信任的嵌套工作区快速路径覆盖。
  • 未加入无关的凭据、token 排除或后代目录策略变更。

测试

已运行配置基线 55 项、修改后配置 56 项、快速路径回归 1 项,以及风险匹配测试 129 项;Prettier 检查、diff check 和 base channel build 通过。ESLint 与 CLI typecheck 因环境限制未验证。

兼容性/风险

这是范围很小的安全优先级修复。明确不可信的嵌套工作区不再从可信父目录继承信任;普通明确可信行为和无关的后代目录策略保持不变。更广泛的“按包含关系不信任”策略可另行讨论,本 PR 有意不扩大范围。

给评审者的说明

请同时检查普通可信目录路径和快速设置路径,确认两者对当前工作区的精确优先级规则一致。Windows 符号链接失败是既有测试的环境限制,不是断言变化。GitHub 预检查已通过,剩余是仓库分诊和评审外部门禁。

关联 Issue

Fixes #8627

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, @daleselaji-dev! We can't take this into review yet: the description doesn't follow the pull request template — none of the required headings are present.

What's missing:

  • ## What this PR does and ## Why it's needed
  • ## Reviewer Test Plan with ### How to verify, ### Evidence (Before & After), and the ### Tested on OS table
  • ## Risk & Scope
  • ## Linked Issues
  • The Chinese <details> translation of the description

Your current description (Problem / Solution / Testing / ...) already covers similar ground, so this is mostly reorganizing content you have: port it into the template, fill the Tested-on table with the environments you actually ran, and keep the note that ESLint/typecheck could not run locally. Then push again (or a maintainer can re-run triage with @qwen-code /triage) and the review continues from there.

中文说明

感谢提交 PR,@daleselaji-dev!目前还无法进入评审:PR 描述未遵循 PR 模板——全部必需标题均缺失。

缺失内容:

  • ## What this PR does## Why it's needed
  • ## Reviewer Test Plan,含 ### How to verify### Evidence (Before & After)### Tested on 操作系统表格
  • ## Risk & Scope
  • ## Linked Issues
  • 描述内容的中文 <details> 翻译

现有描述(Problem / Solution / Testing 等)已涵盖类似内容,主要是把已有内容按模板重新组织:请填入模板,在 Tested-on 表格中标注实际测试过的环境,并保留"本地未能运行 ESLint/typecheck"的说明。完成后重新推送(或请维护者使用 @qwen-code /triage 重新运行分诊),评审将继续进行。

Qwen Code · qwen3.8-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not reviewed: build-and-test — tmux-testing was skipped in CI and its suite did not run locally.

Not reviewed: build-and-test — verify was skipped in CI and its suite did not run locally.

Not reviewed: reverse audit — stopped before round 3 by the review time budget.

— qwen3.8-max via Qwen Code /review (v0.21.6)

Comment on lines +151 to +153
for (const trustedPath of trustedPaths) {
for (const locationVariant of locationVariants) {
for (const trustedVariant of getPathComparisonVariants(trustedPath)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] This fix only wins for a DO_NOT_TRUST rule on the exact workspace path. Any workspace inside a distrusted folder is still evaluated as trusted via the ancestor TRUST_FOLDER rule, because the distrust check matches only by exact variant equality (unchanged here and in isPathTrustedFastPath) — so the vulnerability reported in the linked issue remains reachable one directory deeper. Verified end-to-end with a live probe: with rules TRUST_FOLDER /projects + DO_NOT_TRUST /projects/evil-repo, isPathTrusted('/projects/evil-repo/packages/app') returns true, and bootstrapServeFastPathEnvironment run from that descendant workspace loads the distrusted repo's .env and adopts QWEN_SERVER_TOKEN (the exact distrusted path itself is correctly denied). The issue's stated principle is "most-specific rule wins", and the issue thread's containment argument (distrust must match by containment — a prerequisite, not a follow-up) is unrebutted. The PR description scopes the change to exact-workspace distrust, which documents the limitation but does not remove the traced harm. — Failure scenario: rules TRUST_FOLDER on /projects and DO_NOT_TRUST on /projects/evil-repo; user runs qwen serve from /projects/evil-repo/packages/app (a common monorepo shape) → the exact-equality distrust check misses, this ancestor-trust loop matches via isWithinRoot, the repo-controlled .env is loaded and QWEN_SERVER_TOKEN is injected — the exact outcome the issue describes.

Suggested fix: match DO_NOT_TRUST by containment (or resolve by most-specific/deepest matching rule) in isPathTrusted, isPathTrustedFastPath, and getExplicitTrustLevel together, with regression tests for a descendant workspace both with and without a covering ancestor trust rule. If exact-path-only semantics are the deliberate maintainer decision, please state that in the PR and the issue thread, since it contradicts the thread's prerequisite analysis.

— qwen3.8-max via Qwen Code /review (v0.21.6)

const locationVariants = getPathComparisonVariants(location);
for (const rule of rules) {
if (rule.level !== 'trusted') continue;
if (rule.level !== 'untrusted') continue;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The trust precedence policy is hand-encoded as loop order in three places — isPathTrustedFastPath here, LoadedTrustedFolders.isPathTrusted, and getExplicitTrustLevel — so this precedence change had to be hand-repeated in each, and every future policy change must be too. This PR is evidence the policy does change (the old order was a security bug). — Concrete cost: a future precedence change applied to trustedFolders.ts but missed in this file (whose name advertises settings plumbing, not trust evaluation) means qwen serve decides whether to load a workspace's .env and settings.json under a different policy than the interactive CLI reports — loading environment from a workspace the updated rules consider untrusted. The duplicated scenario tests in fast-path.test.ts catch a one-site miss today; the standing cost is the repetition plus relying on remembering the second site exists.

Suggested fix: extract the pure decision procedure into one dependency-light function beside path-comparison.ts (both files already import from there, so the fast path keeps its cheap startup), or at minimum add a parity test that runs both resolvers over a shared table of precedence scenarios so drift fails mechanically rather than by convention.

— qwen3.8-max via Qwen Code /review (v0.21.6)

Comment on lines +341 to +342
trustLevel === TrustLevel.DO_NOT_TRUST &&
arePathsEquivalent(workspaceCwd, rulePath)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This precedence flip makes the source === 'file' branch of the "This folder is still trusted because a parent folder is trusted." warning in useTrustModify.updateTrustLevel (packages/cli/src/ui/hooks/useTrustModify.ts:86-89) unreachable, and its test (useTrustModify.test.ts, 'still trusted by parent') now passes only by mocking { isTrusted: true, source: 'file' } — a state the implementation can no longer produce. Probe-verified: a file-source preview with [cwd]: DO_NOT_TRUST can now only return untrusted; isTrusted: true is reachable only with source: 'ide' or source: undefined. — Concrete cost: a maintainer reading the hook or its test concludes an exact DO_NOT_TRUST can be overridden by an ancestor's trust — the opposite of the security semantics this PR establishes; under a future parent-grant-priority regression this stale branch and test would mask the bug instead of catching it.

Suggested fix: drop the if (source === 'file') override (the IDE message is the only remaining reachable case) and update/remove the 'still trusted by parent' test to assert the new behavior (no warning; pending change + restart).

— qwen3.8-max via Qwen Code /review (v0.21.6)

Comment on lines +151 to +153
for (const trustedPath of trustedPaths) {
for (const locationVariant of locationVariants) {
for (const trustedVariant of getPathComparisonVariants(trustedPath)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The diff also changes the DO_NOT_TRUST-vs-TRUST_PARENT precedence winner (here, in getExplicitTrustLevel, and in the fast path), but no test pins that combination — only the DO_NOT_TRUST-vs-TRUST_FOLDER shape is tested. Verified empirically: a mutant restoring parent-grant priority passes all 126 trust-related tests while isPathTrusted('/a/b') with rules {'/a/b': DO_NOT_TRUST, '/a/b/sub': TRUST_PARENT} flips from false to true. — Failure scenario: such a regression sails through the whole suite green, and on the qwen serve fast path it loads the .env/settings of the explicitly distrusted workspace into the server process — the exposure this PR fixes.

Suggested fix: add one case per implementation, e.g. in trustedFolders.test.ts: mockRules['/home/user/projectA'] = TrustLevel.DO_NOT_TRUST; mockRules['/home/user/projectA/child/marker'] = TrustLevel.TRUST_PARENT; with mockCwd = '/home/user/projectA' → expect {isTrusted: false, source: 'file'}; mirror in fast-path.test.ts by adding a TRUST_PARENT rule pointing inside the distrusted nested workspace and keeping the QWEN_SERVER_TOKEN undefined expectation.

— qwen3.8-max via Qwen Code /review (v0.21.6)

Comment on lines +341 to +342
trustLevel === TrustLevel.DO_NOT_TRUST &&
arePathsEquivalent(workspaceCwd, rulePath)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The /trust dialog's inherited-trust note (packages/cli/src/ui/components/TrustDialog.tsx:88-95) still asserts the precedence this PR removes: it tells the user the folder "will remain trusted even if you set a different trust level here" and to modify the parent instead — no longer true for the "Don't trust" choice. A/B probe-verified against the merge base: pre-PR the note's claim held (a DO_NOT_TRUST preview stayed trusted); on this PR the same preview returns untrusted while the note still renders and contradicts the shipped behavior. (The IDE variant of the note remains accurate — IDE trust still overrides file rules.) — Failure scenario: with TRUST_FOLDER on /home/user/projects, a user opens /home/user/projects/myapp and runs /trust wanting to distrust only myapp; the note says that is impossible, so they set the parent to DO_NOT_TRUST — stripping trust from every sibling project — or give up, even though selecting "Don't trust" in the dialog would in fact work under the new precedence.

Suggested fix: reword the parent-inheritance note to carve out the new behavior, e.g. "This folder behaves as trusted because a parent folder is trusted. Setting 'Don't trust' here overrides that for this folder only; other trust levels keep inheriting from the parent."

— qwen3.8-max via Qwen Code /review (v0.21.6)

@yiliang114

Copy link
Copy Markdown
Collaborator

Thanks for picking this up, @daleselaji-dev — the direction is exactly what #8627 asked for, and splitting getExplicitTrustLevel into two ordered passes is a clean way to express it. But I don't think this can land as-is: the reported attack still works after the patch, and I'd rather keep the issue open than close it with the exploit one cd away.

The gap

The PR reorders the checks but doesn't change how they match. Distrust still matches by exact path equality (arePathsEquivalent in trustedFolders.ts, locationVariant === untrustedVariant in fast-path-settings.ts), while trust matches by containment (isWithinRoot). So the fix only holds when the launch directory is exactly the distrusted path.

Real repos have subdirectories, and users cd into them. I applied this PR locally and added the #8627 scenario one level deeper:

it('POC: subdir of an explicitly untrusted workspace still loads its env', async () => {
  delete process.env['QWEN_SERVER_TOKEN'];
  const qwenHome = useTempQwenHome();
  tempWorkspace = realpathSync(
    mkdtempSync(join(os.tmpdir(), 'qws-fast-path-trust-poc-')),
  );
  const evilRepo = join(tempWorkspace, 'evil-repo');
  const subDir = join(evilRepo, 'packages', 'foo');
  mkdirSync(subDir, { recursive: true });
  writeFileSync(
    join(qwenHome, 'settings.json'),
    JSON.stringify({ security: { folderTrust: { enabled: true } } }),
  );
  process.env['QWEN_CODE_TRUSTED_FOLDERS_PATH'] = join(
    qwenHome,
    'trustedFolders.json',
  );
  writeFileSync(
    process.env['QWEN_CODE_TRUSTED_FOLDERS_PATH'],
    JSON.stringify({
      [tempWorkspace]: TrustLevel.TRUST_FOLDER,
      [evilRepo]: TrustLevel.DO_NOT_TRUST,
    }),
  );
  writeFileSync(join(evilRepo, '.env'), 'QWEN_SERVER_TOKEN=attacker\n');

  await bootstrapServeFastPathEnvironment(subDir);

  expect(process.env['QWEN_SERVER_TOKEN']).toBeUndefined();
});

With this PR applied:

× POC: subdir of an explicitly untrusted workspace still loads its env
  → expected 'attacker' to be undefined

isPathTrustedFastPath('…/evil-repo/packages/foo') finds no exact distrust match, falls through to isWithinRoot(…, tempWorkspace)trusted. findEnvFilesFastPath then walks up and loads evil-repo/.env. Same token adoption, same non-loopback boot-guard bypass. This is the same thing the review bot flagged as R1-1.

The PR description names this ("broader descendant containment is outside this PR") — I just don't think that line can be drawn here, because descendant containment is the vulnerability.

Please don't just widen distrust to isWithinRoot

That's the obvious next step and it breaks a legitimate configuration in the other direction:

  • /projectsDO_NOT_TRUST, /projects/goodTRUST_FOLDER

Blanket-distrust-with-opt-in-exceptions would stop working, and that's a reasonable way to use this feature.

What we actually want is most-specific-rule-wins: the longest matching rule path decides, with distrust winning exact ties. Both directions fall out of that for free, and it's the semantics #8627 asked for ("最具体规则优先").

Suggested shape

  1. One shared resolver — something like resolveTrustDecision(rules, path) — that picks the longest matching rule path, distrust winning ties. Have LoadedTrustedFolders.isPathTrusted, getExplicitTrustLevel, and isPathTrustedFastPath all delegate to it. Right now the policy is hand-encoded as loop order in three places, which is what let the two paths drift into this bug in the first place; this PR keeps all three copies (also review-bot R1-2).
  2. Regression tests for both directions: the subdir POC above, and the DO_NOT_TRUST parent + TRUST_FOLDER child case.
  3. Update TrustDialog.tsx:92-94 — "It will remain trusted even if you set a different trust level here" becomes false once explicit distrust wins, and it's the exact sentence that would talk a user out of protecting themselves (review-bot R1-5).

A note on ordering while you're in there: getExplicitTrustLevel returns the first match from Object.entries(trustConfig), so when several rules match, the winner depends on key insertion order in trustedFolders.json. Longest-prefix resolution makes that deterministic as a side effect.

Separate issue, not yours to fix here

findEnvFilesFastPath computes isTrusted once for the start dir (fast-path-settings.ts:156) and applies that single boolean to every candidate found while walking up (canUseEnvFile, line 166). So a trusted workspace nested under a distrusted parent still loads the parent's .env. Fixing precedence doesn't fix that — the check needs to be per-candidate-directory. I'll file it separately so it doesn't grow this PR.

Happy to help with the shared-resolver refactor if you'd like to keep authorship here.

中文

感谢 @daleselaji-dev 接手这个问题 —— 方向正是 #8627 所期望的,把 getExplicitTrustLevel 拆成两轮有序检查的写法也很清晰。但我认为目前还不能合入:打上这个补丁后,issue 中描述的攻击依然可以复现。与其在漏洞仅差一次 cd 就能触发的情况下关闭 issue,我更倾向于让它继续开着。

问题所在

本 PR 只调整了检查顺序,并未改变匹配方式。不信任规则仍然是精确路径相等匹配(trustedFolders.ts 中的 arePathsEquivalentfast-path-settings.ts 中的 locationVariant === untrustedVariant),而信任规则是包含匹配(isWithinRoot)。因此该修复仅在启动目录恰好等于被标记为不信任的路径时才生效。

真实仓库都有子目录,用户也会 cd 进去。我在本地应用了本 PR,并把 #8627 的场景往下挪了一层(测试代码见上文英文部分):

× POC: subdir of an explicitly untrusted workspace still loads its env
  → expected 'attacker' to be undefined

isPathTrustedFastPath('…/evil-repo/packages/foo') 找不到精确的不信任匹配,继而落到 isWithinRoot(…, tempWorkspace)判为可信。随后 findEnvFilesFastPath遍历并加载了 evil-repo/.env。token 被采用的路径完全相同,非 loopback 启动守卫同样被绕过。这与 review bot 标记的 R1-1 是同一个问题。

PR 描述中提到了这一点("broader descendant containment is outside this PR"),但我认为这条边界在这里划不出来 —— 后代目录的包含关系本身就是这个漏洞。

请不要简单地把不信任也改成 isWithinRoot

这是最直观的下一步,但它会在反方向上破坏一种合理配置:

  • /projectsDO_NOT_TRUST,/projects/goodTRUST_FOLDER

"整体不信任 + 局部显式放行"将无法工作,而这是该功能的一种合理用法。

我们真正需要的是"最具体规则优先":由最长匹配的规则路径决定,精确相等时不信任优先。 这样两个方向都能自然成立,也正是 #8627 所要求的语义。

建议的实现形态

  1. 抽出统一的解析函数,例如 resolveTrustDecision(rules, path),选取最长匹配的规则路径、平局时不信任优先。让 LoadedTrustedFolders.isPathTrustedgetExplicitTrustLevelisPathTrustedFastPath 全部委托给它。目前这套优先级是以循环顺序的形式在三处手工编码的 —— 这正是两条代码路径当初会出现分歧的根源;本 PR 保留了全部三份副本(即 review bot 的 R1-2)。
  2. 双向回归测试:上面的子目录 POC,以及"父目录 DO_NOT_TRUST + 子目录 TRUST_FOLDER"的场景。
  3. 更新 TrustDialog.tsx:92-94 —— 一旦显式不信任优先,"即使你在此处设置不同的信任级别,它仍将保持受信任"这句话就不再成立;而恰恰是这句话会劝退一个本想保护自己的用户(review bot 的 R1-5)。

顺带一提:getExplicitTrustLevel 返回的是 Object.entries(trustConfig) 中的首个匹配项,因此当多条规则同时命中时,胜出者取决于 trustedFolders.json 中的键插入顺序。改为最长前缀匹配后,这一行为会顺带变得确定。

另一个独立问题,不必在本 PR 中处理

findEnvFilesFastPath 只针对起始目录计算了一次 isTrusted(fast-path-settings.ts:156),然后把这个布尔值应用到向上遍历过程中找到的每一个候选文件(canUseEnvFile,第 166 行)。因此,一个嵌套在不受信任父目录下的受信任工作区,仍会加载父目录的 .env。修复优先级并不能解决这个问题 —— 该检查需要按候选文件所在目录逐个进行。我会另开 issue 跟踪,以免撑大本 PR。

如果你希望保留本 PR 的作者身份,我很乐意协助完成这次共享解析函数的重构。

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

🩺 serve daemon A/B

Built the PR base vs this PR head f20816b, drove a fixed endpoint set against each, and diffed the JSON responses. Only fields that changed are shown.

No response changes against the PR base across 4 scenario(s).

Qwen Code · serve A/B

@yiliang114 yiliang114 self-assigned this Aug 6, 2026
@yiliang114
yiliang114 self-requested a review August 6, 2026 13:35
@yiliang114

Copy link
Copy Markdown
Collaborator

Following up on my earlier review with a concrete proposal, @daleselaji-dev — I prototyped the "most-specific rule wins" direction locally to check it actually holds up before asking you to build it. I'm deliberately not opening a competing PR: this is yours, and I'd rather you fold whatever is useful into it than have two PRs racing. Sketch below, take or leave any of it.

The shape that worked

One resolver, in a new leaf module packages/cli/src/config/trust-precedence.ts:

export type TrustRuleLevel = 'trusted' | 'untrusted';

export interface TrustPrecedenceRule<TPayload = undefined> {
  readonly level: TrustRuleLevel;
  readonly variants: ReadonlySet<string>;  // pre-computed by getPathComparisonVariants
  readonly payload?: TPayload;             // e.g. the original TrustLevel
}

export function resolveTrustRule<TPayload>(
  rules: Iterable<TrustPrecedenceRule<TPayload>>,
  locationVariants: ReadonlySet<string>,
): TrustPrecedenceRule<TPayload> | undefined;

export function resolveTrustDecision(
  rules: Iterable<TrustPrecedenceRule<unknown>>,
  locationVariants: ReadonlySet<string>,
): boolean | undefined;

Of every rule whose path contains the location, the deepest one decides; distrust wins exact ties. TRUST_PARENT is collapsed onto its parent directory before it reaches the resolver.

Two details that turned out to matter:

  • It imports only path-comparison.js. That keeps the serve fast path able to share it without pulling the settings stack into startup — which was my worry about "just put it in trustedFolders.ts".
  • variants comes in pre-computed. The fast path already caches these to avoid repeated realpathSync during boot; taking a raw path string in the signature would have quietly given that up. CachedTrustRule then collapses to type CachedTrustRule = TrustPrecedenceRule.

Four call sites delegate to it: LoadedTrustedFolders.isPathTrusted, getExplicitTrustLevel, isPathTrustedFastPath, and — the one I'd missed in my first review — daemon-trust-policy.ts's explicitTrustLevel, which was probing each rule in isolation and returning the first whose verdict happened to match the effective one. That's a fourth copy of the policy, and it can just ask the resolver which rule actually won.

The consequence to decide on deliberately

Descendants of a DO_NOT_TRUST folder now resolve to false rather than undefined. This is the fix, not a side effectcanUseEnvFile tests isTrusted !== false, so undefined loaded the workspace env. That's the bypass.

It does mean two existing tests need updating, and they're worth updating with a comment rather than quietly:

  • trustedFolders.test.ts:153/secret/bankaccounts.json under a DO_NOT_TRUST /secret
  • should return undefined for a child of an untrusted folder

It also changes UX: useFolderTrust.ts:31 opens the trust dialog on trusted === undefined, so those subdirectories stop prompting and are simply treated as untrusted. I think that's right — the user already said "don't trust this repo", and re-asking per subdirectory would be a downgrade of that decision — but it's a visible change and reviewers should see it argued rather than discover it.

Note that useTrustModify.ts needs no change: it derives isInheritedTrust from isTrusted, so the misleading TrustDialog note stops rendering for the explicit-distrust case on its own. The copy still needs the edit I mentioned before, for the remaining inherited-trust case.

Tests worth pinning

Both directions, so nobody later "simplifies" this back into a one-directional fix:

Where I got to

Locally: 135/135 on fast-path + trustedFolders + daemon-trust-policy, plus 10 unit tests on the resolver; ESLint and prettier clean. Full tsc OOMs on my box, so typecheck is CI's to confirm. I have this on a local branch — say the word and I'll push it as a branch you can cherry-pick from, or paste any individual piece inline here. If you'd rather not take this on, no hard feelings; tell me and I'll open it as a separate PR crediting your groundwork.

中文

接着之前的评审补充一个具体方案,@daleselaji-dev —— 我在本地把"最具体规则优先"的方向做了原型验证,确认可行后再来请你实现。我特意没有另开一个竞争性的 PR:这个 PR 是你的,我更希望你把其中有用的部分吸收进去,而不是两个 PR 并行。以下是草案,采纳与否随你。

可行的形态

一个统一的解析器,放在新的叶子模块 packages/cli/src/config/trust-precedence.ts 中(接口签名见上文英文部分)。

在所有路径包含该 location 的规则中,最深的那条决定结果;精确相等时不信任优先。TRUST_PARENT 在进入解析器之前先折叠到其父目录。

有两个细节事后证明很关键:

  • 它只依赖 path-comparison.js 这样 serve fast path 就能共用它,而不会把 settings 那一整套拉进启动路径 —— 这正是我对"直接放进 trustedFolders.ts"的顾虑。
  • variants 由调用方预先计算好传入。 fast path 已经缓存了这些值,以避免启动过程中重复调用 realpathSync;如果签名里直接收原始路径字符串,这个优化就悄悄丢了。之后 CachedTrustRule 可以直接简化为 type CachedTrustRule = TrustPrecedenceRule

有四处调用点委托给它:LoadedTrustedFolders.isPathTrustedgetExplicitTrustLevelisPathTrustedFastPath,以及我第一轮评审时漏掉的 daemon-trust-policy.ts 中的 explicitTrustLevel —— 它当时是逐条规则单独试探,返回第一条其判定结果恰好与最终结果相同的规则。那其实是这套优先级策略的第四份副本,现在可以直接询问解析器究竟是哪条规则胜出。

需要明确决策的一个后果

DO_NOT_TRUST 目录的后代现在解析为 false 而不是 undefined这正是修复本身,而非副作用 —— canUseEnvFile 判断的是 isTrusted !== false,因此 undefined 会导致工作区 env 被加载。这就是那个绕过点。

这意味着有两个现有测试需要更新,并且值得加注释说明,而不是悄悄改掉:

  • trustedFolders.test.ts:153 —— DO_NOT_TRUST/secret 之下的 /secret/bankaccounts.json
  • should return undefined for a child of an untrusted folder

它同时也改变了交互行为:useFolderTrust.ts:31 是在 trusted === undefined 时弹出信任对话框,因此这些子目录不再询问用户,而是直接按不可信处理。我认为这是对的 —— 用户已经表示"不信任这个仓库",再逐个子目录追问相当于削弱了他这个决定 —— 但这是一处用户可见的变更,应该让评审者看到明确的论证,而不是自己去发现。

另外 useTrustModify.ts 无需改动:它是从 isTrusted 推导 isInheritedTrust 的,因此在显式不信任的场景下,那条有误导性的 TrustDialog 提示会自动不再渲染。但对于仍然存在的继承信任场景,那段文案依然需要按我之前提到的那样修改。

值得固化的测试

覆盖两个方向,以免日后有人把它"简化"回单向修复:

我这边的进展

本地结果:fast-path + trustedFolders + daemon-trust-policy 135/135 通过,解析器另有 10 个单元测试;ESLint 与 prettier 均通过。完整 tsc 在我的机器上 OOM,因此类型检查需要交给 CI 确认。我把这些放在一个本地分支上 —— 你说一声,我可以把它推上来供你 cherry-pick,或者把其中任意片段贴在本讨论串里。如果你不方便继续跟进,也完全没关系,告诉我一声,我会另开 PR 并注明你的前期工作。

qqqys
qqqys previously requested changes Aug 8, 2026

@qqqys qqqys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

本 PR 引入的 CI 失败:Test (ubuntu-latest, Node 22.x) 红,阻塞合并。

位置

  • packages/cli/src/ui/components/TrustDialog.tsx:88-95 —— 本 PR 把 isInheritedTrustFromParent 分支的提示文案整段改写为「Note: This folder currently inherits trust from a parent folder. A more-specific trust rule here can override that decision.」
  • packages/cli/src/ui/components/TrustDialog.test.tsx:96 —— 该断言仍是旧文案「Note: This folder behaves as a trusted folder because one of the parent folders is trusted.」,本 PR 未改到这个文件(gh pr diff 8628 --name-only 里没有它)。

触发条件

TrustDialog.test.tsx > should display the inherited trust note from parent 渲染对话框后 waitFor 断言 lastFrame() 包含旧文案,实际渲染出的是新文案,waitFor 超时后失败。

影响

Test (ubuntu-latest, Node 22.x) 失败,阻塞合并。不是既有失败也不是偶发:

FAIL  src/ui/components/TrustDialog.test.tsx > TrustDialog > should display the inherited trust note from parent
AssertionError: expected '╭───…' to contain 'Note: This folder behaves as a truste…'
Tests  1 failed | 18357 passed | 22 skipped (18380)

当前 head 49e4095 的 run 31236322379 里全仓库仅此一个失败用例;mainTrustDialog.tsx:91-95 仍是旧文案,与该断言一致,所以这个失败只来自本 PR。

修复方向

TrustDialog.test.tsx:96 的断言同步成新文案(建议只断言稳定片段,例如 'currently inherits trust from a parent folder',避免 Ink 换行把整句拆开导致 toContain 再次误伤)。同一文件第 118 行的 IDE 分支断言不受影响,isInheritedTrustFromIde 的文案本 PR 没改,不用动。

@wenshao

wenshao commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /review

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor
_Qwen Code review request accepted. Review is queued in [workflow run](https://github.com/QwenLM/qwen-code/actions/runs/31246158860)._

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the current head (49e4095). The original Critical is fixed: the regular resolver, serve fast path, daemon effective decision, and daemon explicitTrustLevel now share the same deepest-rule policy; descendant DO_NOT_TRUST stays untrusted, while a more-specific trusted child can opt back in. I did not find another reachable code-path blocker in this revision.

The remaining merge blocker is the TrustDialog assertion mismatch already reported by @qqqys: CI is red with 18,357 passed / 1 failed / 22 skipped. One additional cleanup before merge: the PR body is now stale—it still describes an exact-workspace-only fix and says descendant containment is out of scope, while the current head deliberately changes descendant semantics, centralizes the policy, updates daemon reporting, and changes the /trust copy. Could you update the description and test evidence when fixing that assertion? #8643 remains a separate issue.

@yiliang114

Copy link
Copy Markdown
Collaborator

@qwen-code /review

yiliang114 and others added 2 commits August 8, 2026 15:33
…ording

This PR changed the isInheritedTrustFromParent note in TrustDialog.tsx
but the test still asserted the old copy, failing Test (ubuntu-latest,
Node 22.x). Assert the stable fragment instead of the full sentence so
Ink line wrapping cannot break toContain.

Suggested-by: @qqqys in QwenLM#8628 (review)

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I verified at 49e4095 that the original critical is fixed: the regular resolver, the serve fast path, the daemon's effective decision, and the daemon's explicitTrustLevel all share the same deepest-rule policy — a descendant DO_NOT_TRUST stays untrusted, while a more-specific trusted child can opt back in, and I found no other reachable code-path blocker. The only merge blocker left was the TrustDialog assertion still expecting the old wording; f20816b syncs it — that one is a maintainer commit I pushed (branch allows maintainer edits) following @qqqys's suggestion to assert the stable fragment, since the branch had been quiet for days. This run is green. Nothing blocks merge.

@yiliang114
yiliang114 enabled auto-merge August 8, 2026 09:24
@qqqys
qqqys dismissed their stale review August 8, 2026 09:54

原问题已在 f20816b 修复:packages/cli/src/ui/components/TrustDialog.test.tsx:96 的断言已由整句旧文案改为稳定片段 'currently inherits trust from a parent folder',与 TrustDialog.tsx:88-95 的新文案一致;当前 head 上 Test (ubuntu-latest, Node 22.x) 已为 SUCCESS。解除本条 CHANGES_REQUESTED,不代表对本 PR 的整体批准。

@wenshao

wenshao commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Verification report — built and ran this locally, real artifacts, no mocks

Verdict: the fix does what it claims, in both directions, on real binaries. I found nothing that blocks the merge. Three things below are worth a deliberate reviewer decision rather than discovery-after-merge, and one follow-up needs its scope widened by one file.

I built dist/cli.js twice — once from the PR head f20816b, once from 59b750f (the main tip this branch last merged, so the diff between the two builds is exactly the 10 files of this PR) — and drove the same scenarios through both. Every row below is a real qwen serve / qwen mcp list / real TUI run against a real trustedFolders.json under a fresh HOME. No LLM is involved anywhere; every observable is a boot guard, an HTTP response, or a rendered frame.

Artifact-level sanity check that the two builds really differ as intended: resolveTrustDecision appears in 3 chunks of the PR bundle and in 0 chunks of the base bundle.

What I confirmed

A/B matrix

# Layout Expected Result
S1 <ws> TRUST_FOLDER + <ws>/evil-repo DO_NOT_TRUST, launched from evil-repo/packages/foo untrusted trusted → untrusted
S2 <ws> DO_NOT_TRUST + <ws>/good-repo TRUST_FOLDER stays trusted trusted → trusted
S3 S1 with the trustedFolders.json keys written in reverse order untrusted trusted → untrusted
S4 alternating T/D/T/D down a 4-deep chain untrusted trusted → untrusted
S5 DO_NOT_TRUST vs a TRUST_PARENT resolving to the same folder untrusted (distrust wins the tie) trusted → untrusted

S2 is the one that matters most for "don't just widen distrust to isWithinRoot": blanket-distrust-with-opt-in-exceptions still works, and it works identically on base and PR, so it is a genuine no-regression guard rather than a behaviour change.

The #8627 bypass, before and after

attack A/B

Two independent consequences, both on real binaries:

  1. Non-loopback boot guard. The only QWEN_SERVER_TOKEN on disk is the one in the distrusted repo's .env. On base the daemon adopts it and happily binds 0.0.0.0; the daemon then answers only to the attacker's token (I confirmed that by having to send Authorization: Bearer attacker-supplied-token to get a 200 out of GET /workspace/trust). On the PR it refuses to boot.
  2. Workspace mcpServers. qwen mcp list picks up the repo-provided MCP server on base and drops it on the PR. This is the full loadSettings() path, not the fast path — settings.ts:1048 does isWorkspaceTrusted(...).isTrusted ?? true, so the old undefined verdict meant trusted. That ?? true is exactly why the undefined → false change is the fix and not a side effect.

The daemon agrees with both loaders: GET /workspace/trust reports effective.state: untrusted, source: file at both evil-repo and evil-repo/packages/foo on the PR, and trusted on base. That is the fourth policy copy (daemon-trust-policy.ts) now landing on the same answer as the other three.

The TrustDialog copy, in the real TUI

trust dialog A/B

Driven through tmux against the real TUI at <ws>/evil-repo. Base tells the user their DO_NOT_TRUST setting is inert and then proves it — selecting "Don't trust" emits Note: This folder is still trusted because a parent folder is trusted. and the session stays in Auto mode. On the PR the note is gone, the selection is silently effective, and the footer sits at ⏸ Ask permissions. useTrustModify reads folders.user.config[cwd] by raw key, so Current Level: DO_NOT_TRUST is correctly unchanged on both sides.

Checks the PR description lists as unverified

The description says ESLint and the CLI typecheck could not be run in the author's sparse checkout. I ran them on a full npm ci checkout of the head:

  • npm run typecheck --workspace=packages/cli (tsc --noEmit) — clean
  • npx eslint over all 6 changed source files — clean
  • npm run check:serve-fast-path-bundle"Startup bundle closure checks passed." This is the one I most wanted to see: trust-precedence.ts is now a static import of fast-path-settings.ts, and the guard confirms it doesn't drag anything new into the pre-listen closure. The leaf-module discipline held.
  • Targeted suites on the head: trust-precedence + trustedFolders + daemon-trust-policy + fast-path + TrustDialog + useTrustModify + useFolderTrust = 169/169 pass.
  • Same test files against base source: 8 fail across 3 suites (the two fast-path env tests, the daemon-trust-policy descendant test, and 5 trustedFolders assertions). So the new tests genuinely pin the new behaviour. Notably allows a trusted child rule to override an untrusted parent passes on base too — correct, it is a guard, not a change.
  • Startup cost with a pathological 500-rule trustedFolders.json (the resolver no longer early-returns on the first trusted match): base 321 ms vs PR 328 ms, mean of 5 qwen mcp list runs. Noise.

I also checked the case the depth metric could plausibly get wrong: a symlink inside a trusted subtree pointing into an explicitly distrusted repo (<ws>/sub/deep trusted at depth 3, <ws>/evil distrusted at depth 1, <ws>/sub/deep/x → <ws>/evil, entered via the symlink). The PR resolves it untrusted, because both env loaders realpathSync the start dir before asking, so the literal symlink path never reaches the depth comparison. Base treats it as trusted. Good outcome, but it is load-bearing on that upstream realpathSync rather than on the resolver — worth a comment in trust-precedence.ts so nobody removes it later.


Findings

1. The deferred .env hole exists in two files, not one — the follow-up issue needs both

@yiliang114's review flagged that findEnvFilesFastPath computes isTrusted once for the start dir and applies that single boolean to every candidate found walking up (fast-path-settings.ts:152canUseEnvFile at :162), and correctly kept it out of this PR. The identical shape is in the full loader too: environment.ts:215canUseEnvFile at :225. Fixing only the fast path would leave the interactive CLI exposed.

Reproduced on real binaries, identical on base and PR (so: pre-existing, not a regression, not a merge blocker):

<ws>            DO_NOT_TRUST
<ws>/good-repo  TRUST_FOLDER
<ws>/.env       QWEN_SERVER_TOKEN=...   OPENAI_MODEL=model-from-DISTRUSTED-parent
cwd = <ws>/good-repo/src
  • fast path: qwen serve --hostname 0.0.0.0 boots, adopting the distrusted parent's token — base and PR alike.
  • full loader: the TUI header renders API Key | model-from-DISTRUSTED-parent — base and PR alike.

Worth calling out that this PR makes the shape more reachable, not less: S2 (DO_NOT_TRUST parent + TRUST_FOLDER child) is now a first-class supported configuration with a test of its own, and that is precisely the configuration where this bites.

2. Descendants of a DO_NOT_TRUST folder stop prompting — please sign off on this explicitly

useFolderTrust.ts:31 opens the folder-trust dialog on trusted === undefined. Descendants now resolve false, so those directories no longer ask; they are silently treated as untrusted. @yiliang114 already argued this is correct ("the user already said don't trust this repo") and I agree — I am flagging it only because it is user-visible and easy to discover as a bug report later. I verified it is what actually happens, and that a folder matched by no rule still returns undefined and still prompts.

3. explicitTrustLevel now reports an inherited level (low, naming/doc only)

getExplicitTrustLevel returns the winning rule's payload, which may come from an ancestor. At <ws>/evil-repo/packages/foo — a path with no rule of its own — the daemon now reports explicitTrustLevel: "DO_NOT_TRUST". This is not newly wrong: base already leaked ancestor rules here, reporting "TRUST_FOLDER" for the same path, because the old trusted pass matched by containment. The PR makes it symmetric, which is an improvement. But the field is named explicit, it is public surface (GET /workspace/trust, configured.explicitTrustLevel in the v2 status, and DaemonWorkspaceTrustLevel in packages/sdk-typescript), and a UI could reasonably read it as "there is a rule on this exact folder". A doc comment on getExplicitTrustLevel — or renaming to something like decidingTrustLevel — would close that off. The TUI is unaffected: useTrustModify reads the raw config key.


Repro

git fetch origin pull/8628/head:pr-8628 && git worktree add wt-8628 pr-8628
cd wt-8628 && npm ci && npm run build && npm run bundle   # PR build -> dist/

mkdir -p /tmp/h/.qwen /tmp/ws/evil-repo/packages/foo
echo '{"security":{"folderTrust":{"enabled":true}}}' > /tmp/h/.qwen/settings.json
cat > /tmp/h/.qwen/trustedFolders.json <<'JSON'
{ "/tmp/ws": "TRUST_FOLDER", "/tmp/ws/evil-repo": "DO_NOT_TRUST" }
JSON
echo 'QWEN_SERVER_TOKEN=attacker-supplied-token' > /tmp/ws/evil-repo/.env

cd /tmp/ws/evil-repo/packages/foo
HOME=/tmp/h node <wt>/dist/cli.js serve --hostname 0.0.0.0 --port 4171 --no-web
# base: "listening on http://0.0.0.0:4171"   PR: "Refusing to bind 0.0.0.0:4171 ..."

For the base side, revert the 10 PR files to 59b750f and rebuild — that keeps everything else in the tree identical, which is what makes the A/B trustworthy.

Two harness notes for anyone repeating this: once the daemon adopts a token from the workspace .env, unauthenticated GET /workspace/trust returns 401, which reads as "daemon never came up" if you only poll for res.ok — retry with the planted token, and which token it accepts is itself the evidence. And workspace settings are read from <cwd>/.qwen/settings.json with no upward walk, while .env discovery does walk up — so the settings probe and the env probe have to be run from different directories to exercise both the exact-path and the descendant path.

Overall

The refactor does what the review asked: one resolver, four call sites, both directions covered, order-independent, and the misleading dialog copy fixed. It holds up on real binaries across the fast path, the full settings loader, the daemon HTTP surface, and the TUI. The three items above are a scope note on the follow-up, a UX decision worth recording, and a naming nit — none of them a reason to hold this.

中文版(合并参考)

验证报告 —— 本地真实构建,无 mock

结论:这个修复在两个方向上都达成了它声明的目标,用真实二进制验证通过,没有发现阻塞合并的问题。 下面有三点建议评审者主动确认,另有一个后续 issue 的范围需要扩大一个文件。

我构建了两份 dist/cli.js:一份来自 PR head f20816b,一份来自 59b750f(本分支最后合入的 main 提交,因此两份构建之间的差异恰好就是本 PR 的 10 个文件),然后用同一组场景分别驱动。下面每一行都是在全新 HOME 下、针对真实 trustedFolders.json 运行的真实 qwen serve / qwen mcp list / 真实 TUI。全程不涉及 LLM,所有观测点都是启动守卫、HTTP 响应或渲染帧。

构建差异的产物级校验:resolveTrustDecision 出现在 PR bundle 的 3 个 chunk 中,在 base bundle 中为 0 个。

已确认的行为

# 布局 期望 结果
S1 <ws> TRUST_FOLDER + <ws>/evil-repo DO_NOT_TRUST,从 evil-repo/packages/foo 启动 不可信 trusted → untrusted
S2 <ws> DO_NOT_TRUST + <ws>/good-repo TRUST_FOLDER 保持可信 trusted → trusted
S3 与 S1 相同,但 trustedFolders.json 键顺序反转 不可信 trusted → untrusted
S4 4 层深度上 T/D/T/D 交替 不可信 trusted → untrusted
S5 DO_NOT_TRUST 对上解析到同一目录的 TRUST_PARENT 不可信(平局时不信任优先) trusted → untrusted

S2 是"不要简单把不信任改成 isWithinRoot"这一点上最关键的场景:整体不信任 + 局部显式放行仍然可用,且在 base 与 PR 上表现完全一致,因此它确实是一个无回归护栏,而不是行为变更。

#8627 绕过:修复前后

两个相互独立的后果,均在真实二进制上验证:

  1. 非 loopback 启动守卫。 磁盘上唯一的 QWEN_SERVER_TOKEN 就是不可信仓库 .env 里的那一个。base 会采用它并顺利绑定 0.0.0.0;此后守护进程只认攻击者的 token —— 我必须发送 Authorization: Bearer attacker-supplied-token 才能从 GET /workspace/trust 拿到 200,这一点本身就是证据。PR 上守护进程拒绝启动。
  2. 工作区 mcpServers qwen mcp list 在 base 上加载了仓库提供的 MCP server,在 PR 上被丢弃。这走的是完整的 loadSettings() 路径而非 fast path —— settings.ts:1048isWorkspaceTrusted(...).isTrusted ?? true,所以旧的 undefined 判定等价于可信。这个 ?? true 正说明 undefined → false 是修复本身,而不是副作用。

守护进程与两个加载器的判定一致:PR 上 GET /workspace/trustevil-repoevil-repo/packages/foo 都返回 effective.state: untrusted, source: file,base 上返回 trusted。这说明第四份策略副本(daemon-trust-policy.ts)现在与另外三处得出了相同答案。

真实 TUI 中的 TrustDialog 文案

通过 tmux 在 <ws>/evil-repo 驱动真实 TUI。base 告诉用户他的 DO_NOT_TRUST 设置无效,并且随即证明了这一点 —— 选择"Don't trust"后弹出 Note: This folder is still trusted because a parent folder is trusted.,会话仍停留在 Auto mode。PR 上该提示消失,选择静默生效,底栏变为 ⏸ Ask permissionsuseTrustModify 按原始 key 读取 folders.user.config[cwd],因此两侧的 Current Level: DO_NOT_TRUST 都正确未变。

PR 描述中标注为未验证的检查项

描述里说作者的 sparse checkout 无法运行 ESLint 与 CLI typecheck。我在完整 npm ci 的 head 检出上跑了:

  • npm run typecheck --workspace=packages/clitsc --noEmit)—— 通过
  • 对全部 6 个改动源文件执行 npx eslint —— 通过
  • npm run check:serve-fast-path-bundle —— "Startup bundle closure checks passed." 这是我最想看到的一项:trust-precedence.ts 现在是 fast-path-settings.ts 的静态导入,该守卫确认它没有把任何新东西拖进 pre-listen 闭包。叶子模块的约束成立。
  • head 上的定向测试套件:trust-precedence + trustedFolders + daemon-trust-policy + fast-path + TrustDialog + useTrustModify + useFolderTrust = 169/169 通过
  • 同样这些测试文件跑在 base 源码上:3 个套件共 8 个失败。说明新增测试确实钉住了新行为。值得一提的是 allows a trusted child rule to override an untrusted parent 在 base 上也通过 —— 这是对的,它是护栏而非变更。
  • 500 条规则的极端 trustedFolders.json 下的启动开销(解析器不再在首个可信匹配处提前返回):base 321 ms vs PR 328 ms,5 次 qwen mcp list 的均值。属于噪声。

我还验证了深度度量最可能出错的场景:可信子树内的一个符号链接指向被显式不信任的仓库(<ws>/sub/deep 在深度 3 可信,<ws>/evil 在深度 1 不可信,<ws>/sub/deep/x → <ws>/evil,从符号链接进入)。PR 判定为不可信,因为两个 env 加载器都会先对起始目录做 realpathSync,字面符号链接路径根本到不了深度比较那一步。base 判定为可信。结果是好的,但它依赖的是上游那次 realpathSync 而非解析器本身 —— 建议在 trust-precedence.ts 里加一句注释,以免日后被人移除。


发现的问题

1. 被推迟的 .env 漏洞存在于两个文件中,后续 issue 需要覆盖两处

@yiliang114 的评审指出 findEnvFilesFastPath 只针对起始目录计算一次 isTrusted,然后把这个布尔值应用到向上遍历中找到的每个候选(fast-path-settings.ts:152:162canUseEnvFile),并正确地把它排除在本 PR 之外。完整加载器里存在形状完全相同的代码: environment.ts:215:225canUseEnvFile。只修 fast path 会让交互式 CLI 继续暴露。

在真实二进制上复现,base 与 PR 表现完全一致(因此属于既有问题,不是回归,也不阻塞合并):

<ws>            DO_NOT_TRUST
<ws>/good-repo  TRUST_FOLDER
<ws>/.env       QWEN_SERVER_TOKEN=...   OPENAI_MODEL=model-from-DISTRUSTED-parent
cwd = <ws>/good-repo/src
  • fast path:qwen serve --hostname 0.0.0.0 成功启动,采用了不可信父目录的 token —— base 与 PR 相同。
  • 完整加载器:TUI 头部渲染出 API Key | model-from-DISTRUSTED-parent —— base 与 PR 相同。

值得指出的是,本 PR 让这个形状变得更常见而非更少见:S2(DO_NOT_TRUST 父目录 + TRUST_FOLDER 子目录)现在是有专门测试的一等支持配置,而这恰恰就是该漏洞会命中的配置。

2. DO_NOT_TRUST 目录的后代不再弹窗询问 —— 建议明确签署这一点

useFolderTrust.ts:31 是在 trusted === undefined 时打开信任对话框。后代现在解析为 false,因此这些目录不再询问,而是被静默按不可信处理。@yiliang114 已经论证过这是正确的("用户已经说了不信任这个仓库"),我同意 —— 我提出来只是因为它对用户可见,日后很容易被当成 bug 报上来。我验证了实际行为确实如此,并且没有任何规则命中的目录仍然返回 undefined、仍然会弹窗。

3. explicitTrustLevel 现在会报告继承来的级别(低优先级,仅命名/文档)

getExplicitTrustLevel 返回胜出规则的 payload,而它可能来自祖先目录。在 <ws>/evil-repo/packages/foo(本身没有任何规则)上,守护进程现在报告 explicitTrustLevel: "DO_NOT_TRUST"。这并非新引入的错误:base 在同一路径上就已经泄漏祖先规则,报告的是 "TRUST_FOLDER",因为旧的可信轮次本来就是包含匹配。本 PR 让它变得对称,是个改进。但这个字段名叫 explicit,而且是公开接口(GET /workspace/trust、v2 状态里的 configured.explicitTrustLevel、以及 packages/sdk-typescript 中的 DaemonWorkspaceTrustLevel),UI 完全可能把它理解成"这个目录上有一条规则"。给 getExplicitTrustLevel 加一段文档注释,或改名为 decidingTrustLevel 之类,就能堵住这个歧义。TUI 不受影响:useTrustModify 读的是原始配置 key。


复现方式

git fetch origin pull/8628/head:pr-8628 && git worktree add wt-8628 pr-8628
cd wt-8628 && npm ci && npm run build && npm run bundle   # PR 构建 -> dist/

mkdir -p /tmp/h/.qwen /tmp/ws/evil-repo/packages/foo
echo '{"security":{"folderTrust":{"enabled":true}}}' > /tmp/h/.qwen/settings.json
cat > /tmp/h/.qwen/trustedFolders.json <<'JSON'
{ "/tmp/ws": "TRUST_FOLDER", "/tmp/ws/evil-repo": "DO_NOT_TRUST" }
JSON
echo 'QWEN_SERVER_TOKEN=attacker-supplied-token' > /tmp/ws/evil-repo/.env

cd /tmp/ws/evil-repo/packages/foo
HOME=/tmp/h node <wt>/dist/cli.js serve --hostname 0.0.0.0 --port 4171 --no-web
# base: "listening on http://0.0.0.0:4171"   PR: "Refusing to bind 0.0.0.0:4171 ..."

base 一侧把这 10 个文件回退到 59b750f 后重新构建即可 —— 树里其他内容保持完全一致,这正是 A/B 可信的原因。

给复现者两个 harness 提示:一旦守护进程采用了工作区 .env 里的 token,未认证的 GET /workspace/trust 会返回 401;如果只轮询 res.ok,看起来就像"守护进程没起来" —— 应该带上预埋的 token 重试,而它接受哪个 token 本身就是证据。另外,工作区设置是从 <cwd>/.qwen/settings.json 读取的、不向上查找,而 .env 发现向上遍历 —— 所以设置探针和 env 探针必须从不同目录运行,才能分别覆盖精确路径与后代路径。

总体评价

这次重构达成了评审的要求:一个解析器、四处调用点、双向覆盖、与顺序无关,误导性的对话框文案也修好了。它在 fast path、完整设置加载器、守护进程 HTTP 接口和 TUI 上都经受住了真实二进制的检验。上面三点分别是后续 issue 的范围提示、一个值得记录的 UX 决策,以及一个命名建议 —— 都不构成阻拦本 PR 的理由。

@yiliang114
yiliang114 dismissed stale reviews from qwen-code-ci-bot and qwen-code-ci-bot August 8, 2026 11:26

Dismissed: the PR body has carried all required template headings (What this PR does / Why it is needed / Reviewer Test Plan) since the same morning; verified on current head f20816b.

@yiliang114
yiliang114 added this pull request to the merge queue Aug 8, 2026
Merged via the queue into QwenLM:main with commit 9e87453 Aug 8, 2026
61 of 62 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(security): explicit DO_NOT_TRUST loses to ancestor TRUST_FOLDER, letting an untrusted workspace inject the qwen serve bearer token

5 participants