From 49e462c021a52818f84ed3d1ecab3e9769ba22ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=98=93=E8=89=AF?= <1204183885@qq.com> Date: Thu, 30 Apr 2026 15:24:18 +0800 Subject: [PATCH] =?UTF-8?q?fix(lsp):=20=E4=BF=AE=E5=A4=8D=20LSP=20?= =?UTF-8?q?=E6=96=87=E6=A1=A3=E3=80=81isPathSafe=20=E9=99=90=E5=88=B6?= =?UTF-8?q?=EF=BC=8C=E5=B9=B6=E6=8F=90=E5=8D=87=20LSP=20=E5=B7=A5=E5=85=B7?= =?UTF-8?q?=E8=B0=83=E7=94=A8=E7=8E=87=20(#3615)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(docs): correct outdated and inaccurate LSP documentation - Remove reference to non-existent `packages/cli/LSP_DEBUGGING_GUIDE.md` - Remove reference to unimplemented `/lsp status` slash command - Replace incorrect `DEBUG=lsp*` env var with actual debug log location (`~/.qwen/debug/` session files with `[LSP]` tag) - Remove external Claude Code documentation links (`code.claude.com`) - Document `isPathSafe` constraint: absolute paths outside workspace are blocked, users must add server binary directory to PATH - Add practical troubleshooting: `ps aux | grep ` to check if the server process is actually running - Add clangd-specific guidance: `--background-index`, `compile_commands.json` location, and `--compile-commands-dir` usage - Simplify trust documentation (remove vague "configure in settings") * fix(lsp): allow absolute paths in LSP server command configuration Previously, `isPathSafe` rejected any command containing a path separator that resolved outside the workspace directory. This blocked legitimate use cases where users specify absolute paths to language server binaries (e.g. `/usr/bin/clangd`, `/opt/tools/jdtls/bin/jdtls`). The fix allows: - Bare command names resolved via PATH (unchanged) - Absolute paths (explicit user intent, already gated by trust checks) - Relative paths within the workspace (unchanged) Only relative paths that traverse outside the workspace (e.g. `../../malicious-binary`) are still blocked. Closes: server silently fails to start when users configure absolute paths in `.lsp.json`, with only a debug log warning visible. * feat(lsp): inject LSP priority instruction into system prompt when enabled The model was not using the LSP tool because the system prompt's "Tool Usage" section never mentioned it. The tool description alone ("ALWAYS use LSP as the PRIMARY tool") was insufficient — models follow system prompt instructions more reliably than tool descriptions. Changes: - getCoreSystemPrompt() accepts `options.lspEnabled` parameter - When LSP is enabled, injects an instruction in the Tool Usage section telling the model to ALWAYS use the LSP tool FIRST for code intelligence queries (definitions, references, hover, symbols, etc.) instead of falling back to grep/readfile - Updated client.ts to pass config.isLspEnabled() to the prompt builder - Updated test mocks and snapshots * feat(lsp): add symbolName parameter for position-free LSP queries The model avoided calling LSP for findReferences, hover, etc. because these operations required filePath + line + character which the user rarely provides. The model would read files directly instead. Changes: - Add `symbolName` optional parameter to LspTool - When symbolName is provided without line/character, auto-resolve the symbol's position via workspaceSymbol before executing the actual operation (findReferences, hover, goToImplementation, etc.) - Update tool description with examples showing symbolName usage - Move LSP priority instruction to top of system prompt for visibility - Add debug logging for LSP prompt injection This enables natural queries like: {operation: "findReferences", symbolName: "Calculator"} {operation: "hover", symbolName: "addShape"} without requiring the user to know exact file positions. * feat(lsp): add LSP reminder to grep/readfile tool descriptions When LSP is enabled, the model often chose grep or readfile instead of LSP for code intelligence queries. Now the competing tools' descriptions include a note reminding the model to use the LSP tool for definitions, references, symbols, hover, diagnostics, etc. This "push-pull" approach: - System prompt pushes toward LSP (top-level priority instruction) - Grep/ReadFile descriptions pull away from code intelligence usage * fix(docs): align LSP doc with isPathSafe change — absolute paths now supported The doc still said "absolute paths outside the workspace are not supported" but the code was changed to allow them. Updated all three places (Required Fields table, Troubleshooting, Debugging) to reflect that absolute paths are now accepted. * fix(lsp): improve symbol-based tool resolution * fix(lsp): normalize display paths across platforms * fix(lsp): narrow docs and path safety changes * fix(lsp): add edge-case tests for isPathSafe and fix Chinese comment - Add test for intermediate path traversal (./a/../../../etc/passwd) - Add test for forward-slash relative paths (tools/clangd) - Replace Chinese JSDoc with English on requestUserConsent * fix(lsp): rename requestUserConsent to checkWorkspaceTrust The method only checks workspace trust level and does not actually prompt the user for consent. Rename the method and update the JSDoc and call-site log message to accurately reflect the behavior. --- docs/users/features/lsp.md | 64 ++++++------- .../core/src/lsp/LspServerManager.test.ts | 94 +++++++++++++++++++ packages/core/src/lsp/LspServerManager.ts | 41 +++++--- 3 files changed, 154 insertions(+), 45 deletions(-) create mode 100644 packages/core/src/lsp/LspServerManager.test.ts diff --git a/docs/users/features/lsp.md b/docs/users/features/lsp.md index 2af14ed01..5a0c61b2e 100644 --- a/docs/users/features/lsp.md +++ b/docs/users/features/lsp.md @@ -40,7 +40,7 @@ You need to have the language server for your programming language installed: ### .lsp.json File -You can configure language servers using a `.lsp.json` file in your project root. This uses the language-keyed format described in the [Claude Code plugin LSP configuration reference](https://code.claude.com/docs/en/plugins-reference#lsp-servers). +You can configure language servers using a `.lsp.json` file in your project root. Each top-level key is a language identifier, and its value is the server configuration object. **Basic format:** @@ -104,9 +104,9 @@ Example: #### Required Fields -| Option | Type | Description | -| --------- | ------ | ------------------------------------------------- | -| `command` | string | Command to start the LSP server (must be in PATH) | +| Option | Type | Description | +| --------- | ------ | ------------------------------------------------------------------------------------------------------------------------------------------------- | +| `command` | string | Command to start the LSP server. Supports bare command names resolved via `PATH` (e.g. `clangd`) and absolute paths (e.g. `/opt/llvm/bin/clangd`) | #### Optional Fields @@ -148,6 +148,8 @@ For servers that use TCP or Unix socket transport: Qwen Code exposes LSP functionality through the unified `lsp` tool. Here are the available operations: +Location-based operations (`goToDefinition`, `findReferences`, `hover`, `goToImplementation`, and `prepareCallHierarchy`) require an exact `filePath` + `line` + `character` position. If you do not know the exact position, use `workspaceSymbol` or `documentSymbol` first to locate the symbol. + ### Code Navigation #### Go to Definition @@ -315,7 +317,7 @@ LSP servers are only started in trusted workspaces by default. This is because l - **Trusted Workspace**: LSP servers start if configured - **Untrusted Workspace**: LSP servers won't start unless `trustRequired: false` is set in the server configuration -To mark a workspace as trusted, use the `/trust` command or configure trusted folders in settings. +To mark a workspace as trusted, use the `/trust` command. ### Per-Server Trust Override @@ -338,11 +340,12 @@ You can override trust requirements for specific servers in their configuration: ### Server Not Starting -1. **Check if the server is installed**: Run the command manually to verify -2. **Check the PATH**: Ensure the server binary is in your system PATH -3. **Check workspace trust**: The workspace must be trusted for LSP -4. **Check logs**: Look for error messages in the console output -5. **Verify --experimental-lsp flag**: Make sure you're using the flag when starting Qwen Code +1. **Verify `--experimental-lsp` flag**: Make sure you're using the flag when starting Qwen Code +2. **Check if the server is installed**: Run the command manually (e.g. `clangd --version`) to verify +3. **Check the command**: The server binary must be in your system `PATH`, or specified as an absolute path (e.g. `/opt/llvm/bin/clangd`). Relative paths that escape the workspace are blocked +4. **Check workspace trust**: The workspace must be trusted for LSP (use `/trust`) +5. **Check logs**: Look for `[LSP]` entries in the debug log (see Debugging section below) +6. **Check the process**: Run `ps aux | grep ` to verify the server process is running ### Slow Performance @@ -351,41 +354,34 @@ You can override trust requirements for specific servers in their configuration: ### No Results -1. **Server not ready**: The server may still be indexing +1. **Server not ready**: The server may still be indexing. For C/C++ projects with clangd, ensure `--background-index` is in the args and a `compile_commands.json` (or `compile_flags.txt`) exists in the project root or a parent directory. Use `--compile-commands-dir=` if it is in a build subdirectory 2. **File not saved**: Save your file for the server to pick up changes 3. **Wrong language**: Check if the correct server is running for your language +4. **Check the process**: Run `ps aux | grep ` to verify the server is actually running ### Debugging -Enable debug logging to see LSP communication: +LSP debug logs are automatically written to session log files in `~/.qwen/debug/`. To check LSP-related entries: ```bash -DEBUG=lsp* qwen --experimental-lsp +# View the latest session log +grep '\[LSP\]' ~/.qwen/debug/latest + +# Common error messages to look for: +# "command path is unsafe" → relative path escapes workspace, use absolute path or add to PATH +# "command not found" → server binary not installed or not in PATH +# "requires trusted workspace" → run /trust first ``` -Or check the LSP debugging guide at `packages/cli/LSP_DEBUGGING_GUIDE.md`. +You can also verify the server process is running: -## Claude Code Compatibility - -Qwen Code supports Claude Code-style `.lsp.json` configuration files in the language-keyed format defined in the [Claude Code plugins reference](https://code.claude.com/docs/en/plugins-reference#lsp-servers). If you're migrating from Claude Code, use the language-as-key layout in your configuration. - -### Configuration Format - -The recommended format follows Claude Code's specification: - -```json -{ - "go": { - "command": "gopls", - "args": ["serve"], - "extensionToLanguage": { - ".go": "go" - } - } -} +```bash +ps aux | grep clangd # or typescript-language-server, jdtls, etc. ``` -Claude Code LSP plugins can also supply `lspServers` in `plugin.json` (or a referenced `.lsp.json`). Qwen Code loads those configs when the extension is enabled, and they must use the same language-keyed format. +## Extension LSP Configuration + +Extensions can provide LSP server configurations through the `lspServers` field in their `plugin.json`. This can be either an inline object or a path to a `.lsp.json` file. Qwen Code loads these configs when the extension is enabled. The format is the same language-keyed layout used in project `.lsp.json` files. ## Best Practices @@ -406,7 +402,7 @@ qwen --experimental-lsp ### Q: How do I know which language servers are running? -Use the `/lsp status` command to see all configured and running language servers. +Check the debug log for `[LSP]` entries (`grep '\[LSP\]' ~/.qwen/debug/latest`), or verify the process directly with `ps aux | grep `. ### Q: Can I use multiple language servers for the same file type? diff --git a/packages/core/src/lsp/LspServerManager.test.ts b/packages/core/src/lsp/LspServerManager.test.ts new file mode 100644 index 000000000..2135bde0c --- /dev/null +++ b/packages/core/src/lsp/LspServerManager.test.ts @@ -0,0 +1,94 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import path from 'node:path'; +import { describe, expect, it } from 'vitest'; +import type { Config as CoreConfig } from '../config/config.js'; +import type { FileDiscoveryService } from '../services/fileDiscoveryService.js'; +import type { WorkspaceContext } from '../utils/workspaceContext.js'; +import { LspServerManager } from './LspServerManager.js'; + +type PathSafeManager = { + isPathSafe(command: string, workspacePath: string, cwd?: string): boolean; +}; + +function createManager(workspaceRoot: string): PathSafeManager { + return new LspServerManager( + {} as CoreConfig, + {} as WorkspaceContext, + {} as FileDiscoveryService, + { + requireTrustedWorkspace: false, + workspaceRoot, + }, + ) as unknown as PathSafeManager; +} + +describe('LspServerManager', () => { + describe('isPathSafe', () => { + it('allows bare commands resolved through PATH', () => { + const workspaceRoot = path.resolve('/workspace/project'); + const manager = createManager(workspaceRoot); + + expect(manager.isPathSafe('clangd', workspaceRoot)).toBe(true); + }); + + it('allows explicit absolute command paths', () => { + const workspaceRoot = path.resolve('/workspace/project'); + const absoluteCommand = path.join( + path.parse(workspaceRoot).root, + 'usr', + 'bin', + 'clangd', + ); + const manager = createManager(workspaceRoot); + + expect(manager.isPathSafe(absoluteCommand, workspaceRoot)).toBe(true); + }); + + it('allows relative paths that resolve inside the workspace', () => { + const workspaceRoot = path.resolve('/workspace/project'); + const manager = createManager(workspaceRoot); + + expect( + manager.isPathSafe('./tools/clangd', workspaceRoot, workspaceRoot), + ).toBe(true); + }); + + it('blocks relative paths that escape the workspace', () => { + const workspaceRoot = path.resolve('/workspace/project'); + const manager = createManager(workspaceRoot); + + expect( + manager.isPathSafe('../bin/clangd', workspaceRoot, workspaceRoot), + ).toBe(false); + }); + + it('blocks relative paths that use intermediate traversal to escape', () => { + const workspaceRoot = path.resolve('/workspace/project'); + const manager = createManager(workspaceRoot); + + expect( + manager.isPathSafe( + './tools/../../../etc/passwd', + workspaceRoot, + workspaceRoot, + ), + ).toBe(false); + }); + + it('treats commands with forward slash but no path.sep on Windows as relative', () => { + const workspaceRoot = path.resolve('/workspace/project'); + const manager = createManager(workspaceRoot); + + // A command like "subdir/server" is relative; if it resolves inside + // the workspace it should be allowed. + expect( + manager.isPathSafe('tools/clangd', workspaceRoot, workspaceRoot), + ).toBe(true); + }); + }); +}); diff --git a/packages/core/src/lsp/LspServerManager.ts b/packages/core/src/lsp/LspServerManager.ts index 544dcd6ef..f73d07faf 100644 --- a/packages/core/src/lsp/LspServerManager.ts +++ b/packages/core/src/lsp/LspServerManager.ts @@ -213,14 +213,16 @@ export class LspServerManager { return; } - // Request user confirmation - const consent = await this.requestUserConsent( + // Check workspace trust before starting the server + const trusted = await this.checkWorkspaceTrust( name, handle.config, workspaceTrusted, ); - if (!consent) { - debugLogger.info(`User declined to start LSP server ${name}`); + if (!trusted) { + debugLogger.info( + `Workspace trust check failed, not starting LSP server ${name}`, + ); handle.status = 'FAILED'; return; } @@ -623,7 +625,14 @@ export class LspServerManager { } /** - * Check path safety + * Check path safety. + * + * Allows: + * - Bare command names (resolved via PATH, e.g. "clangd") + * - Absolute paths (explicit user intent, e.g. "/usr/bin/clangd") + * + * Blocks: + * - Relative paths that escape the workspace (e.g. "../../bin/evil") */ private isPathSafe( command: string, @@ -636,12 +645,18 @@ export class LspServerManager { return true; } - // For explicit paths (absolute or relative), verify they're within workspace + // Allow absolute paths — the user explicitly specified a full path to + // the server binary (e.g. /usr/bin/clangd, /opt/tools/jdtls/bin/jdtls). + // Trust checks (workspace trust + user consent) already gate server startup. + if (path.isAbsolute(command)) { + return true; + } + + // For relative paths, verify they resolve within the workspace to prevent + // path traversal attacks (e.g. "../../malicious-binary"). const resolvedWorkspacePath = path.resolve(workspacePath); const basePath = cwd ? path.resolve(cwd) : resolvedWorkspacePath; - const resolvedPath = path.isAbsolute(command) - ? path.resolve(command) - : path.resolve(basePath, command); + const resolvedPath = path.resolve(basePath, command); return ( resolvedPath.startsWith(resolvedWorkspacePath + path.sep) || @@ -650,9 +665,13 @@ export class LspServerManager { } /** - * 请求用户确认启动 LSP 服务器 + * Check whether the workspace trust level allows starting an LSP server. + * + * Auto-allows in trusted workspaces. In untrusted workspaces, blocks + * servers that require trust (`trustRequired` or global + * `requireTrustedWorkspace`), and cautiously allows the rest. */ - private async requestUserConsent( + private async checkWorkspaceTrust( serverName: string, serverConfig: LspServerConfig, workspaceTrusted: boolean,