mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-23 15:45:13 +00:00
* refactor(cli): keep acp-integration off serve internals (#8084) The dependency direction set in #8084 regressed: native Live Voice (a5c637b749) added four acp-integration imports of serve/live modules, because nothing in the repo enforces the boundary the issue defines. Ownership, measured by consumer rather than by directory: - capture-screen-context, live-task-tools, live-speak-to-user and live-backend-instructions each have exactly one production consumer, acp-integration/session/Session.ts, and import nothing from serve/. They move to acp-integration/live/ with their tests. - conversations/session-source is shared by acpAgent and four serve modules, has no imports, and takes its reader as a parameter, so it moves to runtime/live-session-source.ts alongside the other neutral contracts. Renamed because every symbol in it is Live-specific. Adds a no-restricted-imports rule for acp-integration/** so the next feature spanning both surfaces gets a lint error pointing at runtime/, rather than silently reopening the criterion. No behavior change: moves, import rewrites, and the lint block. * fix(cli): harden the acp/serve boundary guard (round 2) - Flag the bare '../serve' directory specifier, which resolves to the serve/ barrel and skipped the trailing-segment group patterns (also added to the utils/ guard for symmetry). - Extend the same boundary to runtime/, the layer the rule directs authors to, so the #8084 coupling cannot reform one hop away. - Cover dynamic imports: no-restricted-imports never visits ImportExpression, so a no-restricted-syntax selector now enforces the boundary for await import('../serve/...') too. The acp-integration block moves after the general TS block (flat config lets the last matching block win per rule) and restates its no-restricted-syntax selectors so the override drops nothing. - Document that CI lint is the enforcement point; no fixture test pins the block. Verified: synthetic fixtures for all three violation shapes are rejected; full npm run lint passes with no live violations. * test(cli): pin serve boundary lint rules * test(cli): close serve boundary lint gaps * fix(lint): close serve-boundary entrances and harden the guard - reject computed dynamic-import sources (concatenation, new URL) and type-level imports fail-closed; rounds 2-5 each demonstrated a new per-spelling regex entrance, so non-literal forms are blocked outright (R4-1) - rewrite the boundary patterns without nested quantifiers; the previous shape backtracked exponentially (~4x per two ../ segments, lint-time ReDoS) (R5-2) - build the three guarded override blocks no-restricted-syntax arrays from one shared helper so flat config last-wins cannot silently drop selectors (R5-3) - pin the bare-directory barrel specifier in fixtures (R5-4) and add a string-throw probe pinning the restated selectors in the overrides (R5-5) - replace the **/serve* static globs with enumerated relative depths so third-party serve-named packages are never flagged (R5-7) * fix(lint): correct TSImportType selector path and computed-template handling - read the type-import specifier at argument.literal.value: @typescript-eslint wraps it in a TSLiteralType, so argument.value was dead code and the old fail-closed TSImportType selector over-matched every type-level import (37 errors in files this PR never touches) (round-6 Critical) - reject computed template literals (templates containing expressions) fail-closed; pure-literal templates stay covered by the quasis pattern selectors — the old blanket TemplateLiteral exemption contradicted the fail-closed comment above it (round-6 Critical) - give the fail-closed selectors a distinct message: computed sources cannot be checked against the boundary, which is not the same policy as importing serve/ (round-6 suggestion) - pin the depth-enumeration loop beyond depth 1 with a depth-2 fixture, pin the fixed type-import selector with a negative typeof-import control, and pin the computed-template fail-closed path (round-6 suggestion) * fix(lint): close the remaining round-6 serve-boundary entrances Complements the previous commit (which fixed the TSImportType selector path and computed-template fail-closed) with the R4-1 entrances it left open, each pinned by a fixture: - percent-encoded segments (`../%73erve/index.js`): Node percent-decodes segments when mapping the resolved URL to the filesystem, so raw-text patterns cannot see through them — any `%` in a guarded-tree specifier is now rejected with a dedicated message. - static traversal twins: the pattern regexes now run over static ImportDeclaration/ExportNamedDeclaration/ExportAllDeclaration sources too, closing `import './../serve/x'`, `import '../runtime/../serve/x'`, and `import '..//serve/x'`, whose dynamic twins were already blocked. - leading literal segment: a traversal-anywhere pattern catches `import('foo/../../../serve/x')` past the dot-slash anchor. - vitest module-loading calls (vi.mock/doMock/importActual/importMock) resolve and load the real module, so they get the same patterns plus fail-closed coverage for computed arguments. * fix(lint): cover vitest serve-boundary calls * fix(lint): close the round-7 serve-boundary entrance classes R4-1 round-7 interim hardening (the durable specifier-resolving custom rule remains tracked separately): - case-variant spellings (../Serve/...): every pattern, percent and quasis attribute regex now carries the i flag, covering the dynamic, static, vi.*/vitest.* and TSImportType arms. - ?query/#fragment suffixes: rejected alongside % in all eight specifier shapes (bundlers/Node strip them when resolving, so '../serve?x' reaches the same module as '../serve'). - percent-encoded pure-template vitest calls: added the missing arguments.0.quasis.0.value.cooked twin to the reject list. - root-absolute and file: literal specifiers: fail-closed rejected in every literal shape (guarded trees sweep verified clean of both). - createRequire: its source modules ('module'/'node:module') are flagged in guarded trees, since the alias escapes the callee-name="require" arm and Node >=22 require(esm) loads serve/. Each entrance class is pinned by a fixture case (18/18 green through the real ESLint API); the three guarded trees lint clean with the new arms. * refactor(lint): resolve the serve boundary by resolution, not text (#8084) R4-1 round-8 decision (maintainer-approved option a): replace the spelling-by-spelling regex/glob matrix with a local resolution-based ESLint rule (eslint-rules/no-serve-boundary-cross.js). Eight review rounds each demonstrated a new spelling escaping the text matrix (data: URLs, percent-encoding, traversal through a leading literal segment, baseUrl bare specifiers, createRequire/getBuiltinModule, TSImportType, aliased vitest loaders, Worker/fork), because every spelling is just another way to NAME the same target. The new rule resolves each import-like specifier against the importing file and reports anything landing inside packages/cli/src/serve/: - relative specifiers resolved against the importing file - baseUrl bare specifiers resolved against packages/cli (tsconfig baseUrl makes `src/serve/...` reachable — the round-8 entrance text never saw) - file: URLs resolved to concrete paths (case-insensitive, whitespace-trimmed scheme detection, since the URL parser normalizes both) - vitest loaders matched alias-proof (v.mock / destructured importActual); only specifiers resolving INTO serve/ report - child_process.fork checked; spawn deliberately not (first arg is an executable, not a module) - fail-closed on statically-unresolvable sources: computed sources, data: URLs, traversal-bearing bare specifiers, node:module imports, process.getBuiltinModule - case-insensitive path comparison (Serve/ loads serve/ on case-insensitive filesystems) Fixture suite reworked to resolution semantics: several round-4..7 fixture depths corrected to spellings that genuinely resolve into src/serve (the old depths resolved to packages/cli/serve, outside src/serve, and were only caught by text matching); new pins for every round-8 entrance and for the Codex self-review Criticals (aliased loaders, uppercase/whitespace URL schemes, spawn not an import source). 26/26 pass; guarded trees and the full cli src lint clean (zero false positives). Removed: relativeServeImportPatterns, restrictedServeImports, serveDynamicImportPatterns, serveGuardSyntaxRules and the per-spelling selector/percent/absolute/createRequire special cases. * fix(lint): drop the dead serveGuardSyntaxRules helper The resolution-rule commit removed the mechanism but left the serveGuardSyntaxRules helper behind — unused (no-unused-vars) and referencing the already-deleted restrictedServeDynamicImports (no-undef), which failed CI's repo-wide eslint. The guarded trees inherit restrictedRequire + restrictedStringThrow from the general TS block, so nothing is lost. * fix(lint): close serve boundary resolver gaps * fix(lint): address serve boundary review suggestions - R9-2: move the new-URL-with-import.meta check into the NewExpression visitor with the real MemberExpression base shape; the CallExpression placement was unreachable and standalone new URL(...) reported nothing - R9-3: report module-builtin entrances via the moduleBuiltin messageId instead of the self-contradicting failClosed remediation text - R9-4: match the destructured fork(...) spelling, not just child_process.fork(...) - R9-5/R8-2: fixture pins for re-exports, Worker, fork, require, vi.doMock and vi.importMock entrances - R9-7: pin the false branch of static-template concatenation (pure template literals resolving outside serve stay allowed) - R10-3: filter the third-party serve-named package pin by ruleId so a failClosed false positive also turns it red - R13-2: pin resolution detections on the serveBoundary messageId so inside-detection degrading to blanket fail-closed cannot ship green * fix(lint): close round-11 serve boundary gaps Critical fixes: - '#name' package-imports specifiers sailed through: stripUrlSuffixes splits on '#' before the fail-closed check saw it, so the branch was dead code and '#s' classified outside. Check '#' before suffix stripping (fixture pins both entrances). - scheme detection used JS trim(), which keeps non-whitespace C0 controls — '\x01data:…' slipped past while Node's URL parser strips C0-or-space at the edges and loaded it. Detect schemes on the WHATWG-normalized form (fixtures added). - eval("(0,eval)"/globalThis.eval spellings included) and new Function can embed import('…') the rule cannot resolve — fail closed like computed sources; no-eval/no-new-func are not enabled in the shared config and the guarded trees contain no such calls. - backslashes normalize to '/' under Node's URL-based ESM resolution (file: URLs are special), so '..\\serve\\x.js' loaded serve/ on posix while the rule saw a bare specifier. Normalize backslashes before classification (fixture added). Hardening + pins: - fork/Worker arms match object-agnostically (namespace/default-import spellings no longer evade); Worker skips new URL(spec, import.meta.url) arguments so the URL arm owns them (no more fail-closed false positive on the canonical construct, no double report on serve targets). - new TSImportEqualsDeclaration visitor: import x = require('../serve/…') emits a working createRequire shim under tsc NodeNext. - isProcessObject accepts computed properties (globalThis['process']) and the Reflect.apply arm accepts computed getBuiltinModule; the three getBuiltinModule arms collapse into one via a shared property matcher. - vitest loader names lifted into a module-level constant; corrected two stale comments (fail-closed branches; R5-5 probe description). - fixtures: root-absolute/file: inside verdicts (repoRoot, messageId), fork member arm, template cooked values, bare vitest loaders, dynamic bare-'module' entrances, outside-serve negatives for URL/Worker/fork/ require. Suite 44/44. * fix(cli): restore live session source import * fix(lint): clear the two lint errors breaking CI on the boundary rule Follow-up to the round-11 batch, which landed without running the repo lint: - the C0-edge-strip regex legitimately contains control-character ranges (it mirrors the WHATWG URL parser), so disable no-control-regex on that line with a rationale comment instead of rewriting the range. - drop the unused UTILS_FIXTURE constant from the boundary tests (no fixture lints a utils/ file). eslint clean on both files, boundary suite 44/44, prettier clean. * fix(lint): close bounded serve boundary gaps * fix(lint): complete the round-12 boundary escape closures Extends the previous commit (which canonicalized the serve/baseUrl comparison sides, added staticMemberPropertyName, and closed the Function-call and Worker-eval-option shapes) with the remaining round-12 review surface — every demonstrated spelling probed before and after: - Callee identity is now shape-tolerant end to end: rightmost-segment object matching (nested member objects like globalThis.vi / x.cp no longer evade the object-agnostic arms), renamed loader bindings resolved from the import declarations (fork-as-f, Worker-as-W), Reflect.apply/construct unwrapped for guarded targets (fork included), Function.prototype.call/apply/bind indirection handled (.call unwraps with shifted args; .apply/.bind fail closed), and the SequenceExpression unwrap applied uniformly instead of eval-only. - The string-code execution class fails closed beyond Function/eval direct calls: .constructor property chains (({}).constructor.constructor, (function(){}).constructor, AsyncFunction variants), eval.call/apply, and the node:vm surface (runInThisContext / runInNewContext / runInContext / compileFunction / new vm.Script, scoped to vm imports). - The Worker eval option fails closed unless eval is statically false (a dynamic option or non-object second argument is unverifiable), and the URL arm's import.meta base restriction reports failClosed on the construct. - Fixtures pin each class: shape variants, renamed bindings, Reflect indirection, call/apply/bind, the string-code family (incl. messageId- specific failClosed pins for the eval:true Worker and non-url import.meta bases), bare-directory/baseUrl query-suffix spellings, and outside-serve allow pins for the export/import-equals arms. - expectServeBoundaryError now filters on the rule id (all three messageIds contain 'serve'); the divergent substring negative pins move to expectNoBoundaryHits. Suite 52/52; guarded trees lint clean (no false positives from the new arms); eslint + prettier clean. * fix(lint): close the round-12 reviewer escape classes (#8084) Ten Criticals plus five hardenings from the round-12 review, every class probe-verified before and after: - R12-1: Worker eval-option analysis now matches runtime object-literal semantics — the LAST eval key wins (duplicates included), an options object without eval defaults to false (specifier path, no over-block), and a spread after the last literal eval is unverifiable — fail closed. - R12-2: sequence unwrapping is now a uniform invariant — recursive on callees in both visitors and applied to object expressions (rightmostObjectName/isProcessObject), closing (0, require).call, (0, (0, require)), (0, process).getBuiltinModule, (0, vm).* and new (0, vm).Script. - R12-3: call/apply/bind indirection is complete — Function/constructor forward code (unconditional fail-closed), chained indirection (x.call.call) fails closed instead of falling through, and vm exec names plus the fork/vitest alias sets resolve. - R12-4: Reflect.apply/construct target lists mirror the direct-call arms — Function (incl. member spellings), the vm exec/Script surface, and the vitest loaders (member and identifier targets). - R12-5: alias sets populate in a pre-pass over the module body — ESM imports are hoisted, so use-before-import now resolves like the import-first direction. - R12-6: renamed destructured vitest imports resolve through a new vitestLoaderAliases set. - R12-7: a named guarded global (process/globalThis/global) carrying an opaque computed key fails closed (process-family keeps the dedicated moduleBuiltin message); object-agnostic arms keep their documented residue. - R12-8: .constructor fails closed on variable bodies and expression templates; statically non-string literals keep the pass-through. - R12-9: inline lazy vm imports ((await import('node:vm')).*) count as vm objects in the exec and Script arms. - R12-11/12/13: checkSource skips statically non-specifier arguments (no unactionable advice for env objects), the URL arm owns new URL(spec, import.meta.url) on every entrance (no over-block, one report on the serve form), and the module builtin reports moduleBuiltin on every entrance. Test hardening: R12-10 normalizes the repoRoot pin to forward slashes (Windows merge-gate determinism), R12-14 pins the four mutation survivors, R12-15 pre-cleans and catch-cleans the baseUrl symlink links. Suite 65/65; guarded trees lint clean. * fix(lint): repair corrupted files from the git-API blob upload The previous commit (552bc7c8f) was pushed via the GitHub git API with `-f content=@file` blob payloads that GitHub stored corrupted (9-byte binary blobs), breaking the eslint config load (SyntaxError) and the CI Test gate. Re-upload both files with JSON --input payloads whose blob SHAs match the local git objects byte-for-byte (8270a0d2/5a0ebaa2). No content change beyond restoring the intended files; suite 65/65. * fix(lint): close the round-13 serve-boundary escape classes (#8084) - re-normalize backslashes AFTER percent-decoding so %5c/%5C cannot reintroduce a traversal the pre-decode normalization missed - on realpath ENOENT canonicalize the deepest existing ancestor and re-append the missing tail, so symlinked-ancestor checkouts fail closed instead of open - Worker eval-option scan: treat an unresolvable computed key like a spread (unknown), fail closed on a non-computed __proto__ prototype unless it is statically null, and resolve quoted string-literal keys - carry the opaque-key fail-closed check through composed callees: one hop below .call/.apply/.bind, as a Reflect target, and on the getBuiltinModule object side, with the process-family message Pins all four classes with executed fixtures plus negative controls; the guarded trees stay lint-clean. * fix(lint): close the round-13 binding-hop and callee-opacity escapes (#8084) * refactor(cli): simplify ACP serve boundary guard * fix(lint): close the round-21 contract pins and bare-barrel escape (#8084) * fix(lint): close dynamic-import and js-file holes in the acp/serve guard (#8084) * fix(lint): make acp/serve dynamic-import guard case-insensitive The no-restricted-syntax selector for dynamic import() of serve/ was case-sensitive, so a macOS case-variant specifier (`../Serve/...`) would resolve to the daemon barrel without tripping the guard. Add the /i flag and cover case-variant plus computed-specifier behavior. --------- Co-authored-by: yiliang114 <yiliang114@users.noreply.github.com>
114 lines
3.2 KiB
JavaScript
114 lines
3.2 KiB
JavaScript
/**
|
|
* @license
|
|
* Copyright 2026 Qwen Team
|
|
* SPDX-License-Identifier: Apache-2.0
|
|
*/
|
|
|
|
import { execFileSync } from 'node:child_process';
|
|
import { readFileSync } from 'node:fs';
|
|
import { dirname, join } from 'node:path';
|
|
import { fileURLToPath } from 'node:url';
|
|
import { expect, it } from 'vitest';
|
|
|
|
const root = join(dirname(fileURLToPath(import.meta.url)), '../..');
|
|
|
|
function definitionFiles(pattern) {
|
|
return execFileSync(
|
|
'git',
|
|
['grep', '--untracked', '-l', '-E', pattern, '--', 'packages'],
|
|
{ cwd: root, encoding: 'utf8' },
|
|
)
|
|
.trim()
|
|
.split(/\r?\n/)
|
|
.filter(Boolean);
|
|
}
|
|
|
|
const definitions = [
|
|
{
|
|
symbol: 'LIVE_TASK_TOOL_NAMES',
|
|
pattern: '^(export )?(const|let|var) LIVE_TASK_TOOL_NAMES[[:space:]]*[:=]',
|
|
owner: 'packages/acp-bridge/src/bridgeOptions.ts',
|
|
},
|
|
{
|
|
symbol: 'LiveTaskToolName',
|
|
pattern:
|
|
'^(export )?type LiveTaskToolName[[:space:]]*(<[^>]+>)?[[:space:]]*=',
|
|
owner: 'packages/acp-bridge/src/bridgeOptions.ts',
|
|
},
|
|
{
|
|
symbol: 'MAX_SUB_SESSION_PROMPT_CHARS',
|
|
pattern:
|
|
'^(export )?(const|let|var) MAX_SUB_SESSION_PROMPT_CHARS[[:space:]]*[:=]',
|
|
owner: 'packages/core/src/tools/sub-session-constants.ts',
|
|
},
|
|
];
|
|
|
|
it.each(definitions)('$symbol has one owner', ({ pattern, owner }) => {
|
|
expect(definitionFiles(pattern)).toEqual([owner]);
|
|
});
|
|
|
|
const imports = [
|
|
[
|
|
'LIVE_TASK_TOOL_NAMES',
|
|
'packages/acp-bridge/src/bridgeClient.ts',
|
|
'./bridgeOptions.js',
|
|
],
|
|
[
|
|
'LIVE_TASK_TOOL_NAMES',
|
|
'packages/cli/src/acp-integration/live/live-task-tools.ts',
|
|
'@qwen-code/acp-bridge/bridgeOptions',
|
|
],
|
|
[
|
|
'LIVE_TASK_TOOL_NAMES',
|
|
'packages/cli/src/serve/live/live-task-service.ts',
|
|
'@qwen-code/acp-bridge/bridgeOptions',
|
|
],
|
|
[
|
|
'LiveTaskToolName',
|
|
'packages/cli/src/acp-integration/live/live-task-tools.ts',
|
|
'@qwen-code/acp-bridge/bridgeOptions',
|
|
],
|
|
[
|
|
'LiveTaskToolName',
|
|
'packages/cli/src/serve/live/live-task-service.ts',
|
|
'@qwen-code/acp-bridge/bridgeOptions',
|
|
],
|
|
[
|
|
'MAX_SUB_SESSION_PROMPT_CHARS',
|
|
'packages/core/src/tools/create-sub-session.ts',
|
|
'./sub-session-constants.js',
|
|
],
|
|
[
|
|
'MAX_SUB_SESSION_PROMPT_CHARS',
|
|
'packages/acp-bridge/src/bridgeOptions.ts',
|
|
'@qwen-code/qwen-code-core/subSessionConstants',
|
|
],
|
|
[
|
|
'MAX_SUB_SESSION_PROMPT_CHARS',
|
|
'packages/acp-bridge/src/bridgeClient.ts',
|
|
'./bridgeOptions.js',
|
|
],
|
|
];
|
|
|
|
it.each(imports)('%s is imported by %s', (symbol, path, source) => {
|
|
const text = readFileSync(join(root, path), 'utf8');
|
|
const escapedSource = source.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
|
const statements =
|
|
text.replace(/\/\/.*$/gm, '').match(/^import[\s\S]*?;$/gm) ?? [];
|
|
expect(
|
|
statements.some(
|
|
(statement) =>
|
|
new RegExp(`\\b${symbol}\\b`).test(statement) &&
|
|
new RegExp(`from ['"]${escapedSource}['"]`).test(statement),
|
|
),
|
|
).toBe(true);
|
|
});
|
|
|
|
it('distinguishes locale resolution from prompt sanitization', () => {
|
|
expect(
|
|
definitionFiles('^export function getExtensionDisplayName[(]'),
|
|
).toEqual(['packages/core/src/extension/i18n.ts']);
|
|
expect(
|
|
definitionFiles('^export function getSanitizedExtensionDisplayName[(]'),
|
|
).toEqual(['packages/cli/src/utils/extension-mention.ts']);
|
|
});
|