diff --git a/docs/users/configuration/settings.md b/docs/users/configuration/settings.md index 7fd53083c..f6b71a076 100644 --- a/docs/users/configuration/settings.md +++ b/docs/users/configuration/settings.md @@ -73,13 +73,7 @@ When both legacy settings are present with different values, the migration follo ### Available settings in `settings.json` -Settings are organized into categories. Most settings should be placed within their corresponding top-level category object in your `settings.json` file. A few compatibility settings, such as `proxy`, are top-level keys. - -#### top-level - -| Setting | Type | Description | Default | -| ------- | ------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- | -| `proxy` | string | Proxy URL for CLI HTTP requests. Precedence is `--proxy` > `proxy` in `settings.json` > `HTTPS_PROXY` / `https_proxy` / `HTTP_PROXY` / `http_proxy` environment variables. | `undefined` | +Settings are organized into categories. Most settings should be placed within their corresponding top-level category object in your `settings.json` file. A few top-level settings like `proxy` and `plansDirectory` remain direct root keys for compatibility. #### general @@ -145,17 +139,17 @@ Settings are organized into categories. Most settings should be placed within th #### model -| Setting | Type | Description | Default | -| -------------------------------------------------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------- | -| `model.name` | string | The Qwen model to use for conversations. | `undefined` | -| `model.maxSessionTurns` | number | Maximum number of user/model/tool turns to keep in a session. -1 means unlimited. | `-1` | -| `model.generationConfig` | object | Advanced overrides passed to the underlying content generator. Supports request controls such as `timeout`, `maxRetries`, `enableCacheControl`, `splitToolMedia` (set `true` for strict OpenAI-compatible servers like LM Studio that reject non-text content on `role: "tool"` messages — splits media into a follow-up user message), `contextWindowSize` (override model's context window size), `modalities` (override auto-detected input modalities), `customHeaders` (custom HTTP headers for API requests), `extra_body` (additional body parameters for OpenAI-compatible API requests only), and `reasoning` (`{ effort: 'low' \| 'medium' \| 'high' \| 'max', budget_tokens?: number }` to control thinking intensity, or `false` to disable; `'max'` is a DeepSeek extension — see [Reasoning / thinking configuration](./model-providers.md#reasoning--thinking-configuration) for per-provider behavior. **Note:** when `samplingParams` is set on an OpenAI-compatible provider, the pipeline ships those keys verbatim and the separate top-level `reasoning` field is dropped — put `reasoning_effort` inside `samplingParams` (or `extra_body`) instead in that case), along with fine-tuning knobs under `samplingParams` (for example `temperature`, `top_p`, `max_tokens`). Leave unset to rely on provider defaults. | `undefined` | -| `model.chatCompression.contextPercentageThreshold` | number | Sets the threshold for chat history compression as a percentage of the model's total token limit. This is a value between 0 and 1 that applies to both automatic compression and the manual `/compress` command. For example, a value of `0.6` will trigger compression when the chat history exceeds 60% of the token limit. Use `0` to disable compression entirely. | `0.7` | -| `model.skipNextSpeakerCheck` | boolean | Skip the next speaker check. | `false` | -| `model.skipLoopDetection` | boolean | Disables loop detection checks. Loop detection prevents infinite loops in AI responses but can generate false positives that interrupt legitimate workflows. Enable this option if you experience frequent false positive loop detection interruptions. | `false` | -| `model.skipStartupContext` | boolean | Skips sending the startup workspace context (environment summary and acknowledgement) at the beginning of each session. Enable this if you prefer to provide context manually or want to save tokens on startup. | `false` | -| `model.enableOpenAILogging` | boolean | Enables logging of OpenAI API calls for debugging and analysis. When enabled, API requests and responses are logged to JSON files. | `false` | -| `model.openAILoggingDir` | string | Custom directory path for OpenAI API logs. If not specified, defaults to `logs/openai` in the current working directory. Supports absolute paths, relative paths (resolved from current working directory), and `~` expansion (home directory). | `undefined` | +| Setting | Type | Description | Default | +| -------------------------------------------------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------- | +| `model.name` | string | The Qwen model to use for conversations. | `undefined` | +| `model.maxSessionTurns` | number | Maximum number of user/model/tool turns to keep in a session. -1 means unlimited. | `-1` | +| `model.generationConfig` | object | Advanced overrides passed to the underlying content generator. Supports request controls such as `timeout`, `maxRetries`, `enableCacheControl`, `splitToolMedia` (set `true` for strict OpenAI-compatible servers like LM Studio that reject non-text content on `role: "tool"` messages — splits media into a follow-up user message), `contextWindowSize` (override model's context window size), `modalities` (override auto-detected input modalities), `customHeaders` (custom HTTP headers for API requests), and `extra_body` (additional body parameters for OpenAI-compatible API requests only), along with fine-tuning knobs under `samplingParams` (for example `temperature`, `top_p`, `max_tokens`). Leave unset to rely on provider defaults. | `undefined` | +| `model.chatCompression.contextPercentageThreshold` | number | Sets the threshold for chat history compression as a percentage of the model's total token limit. This is a value between 0 and 1 that applies to both automatic compression and the manual `/compress` command. For example, a value of `0.6` will trigger compression when the chat history exceeds 60% of the token limit. Use `0` to disable compression entirely. | `0.7` | +| `model.skipNextSpeakerCheck` | boolean | Skip the next speaker check. | `false` | +| `model.skipLoopDetection` | boolean | Disables loop detection checks. Loop detection prevents infinite loops in AI responses but can generate false positives that interrupt legitimate workflows. Enable this option if you experience frequent false positive loop detection interruptions. | `false` | +| `model.skipStartupContext` | boolean | Skips sending the startup workspace context (environment summary and acknowledgement) at the beginning of each session. Enable this if you prefer to provide context manually or want to save tokens on startup. | `false` | +| `model.enableOpenAILogging` | boolean | Enables logging of OpenAI API calls for debugging and analysis. When enabled, API requests and responses are logged to JSON files. | `false` | +| `model.openAILoggingDir` | string | Custom directory path for OpenAI API logs. If not specified, defaults to `logs/openai` in the current working directory. Supports absolute paths, relative paths (resolved from current working directory), and `~` expansion (home directory). | `undefined` | **Example model.generationConfig:** @@ -440,12 +434,13 @@ LSP server configuration is done through `.lsp.json` files in your project root #### advanced -| Setting | Type | Description | Default | -| ------------------------------ | ---------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------ | -| `advanced.autoConfigureMemory` | boolean | Automatically configure Node.js memory limits. | `false` | -| `advanced.dnsResolutionOrder` | string | The DNS resolution order. | `undefined` | -| `advanced.excludedEnvVars` | array of strings | Environment variables to exclude from project context. Specifies environment variables that should be excluded from being loaded from project `.env` files. This prevents project-specific environment variables (like `DEBUG=true`) from interfering with the CLI behavior. Variables from `.qwen/.env` files are never excluded. | `["DEBUG","DEBUG_MODE"]` | -| `advanced.bugCommand` | object | Configuration for the bug report command. Overrides the default URL for the `/bug` command. Properties: `urlTemplate` (string): A URL that can contain `{title}` and `{info}` placeholders. Example: `"bugCommand": { "urlTemplate": "https://bug.example.com/new?title={title}&info={info}" }` | `undefined` | +| Setting | Type | Description | Default | +| ------------------------------ | ---------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------ | +| `advanced.autoConfigureMemory` | boolean | Automatically configure Node.js memory limits. | `false` | +| `advanced.dnsResolutionOrder` | string | The DNS resolution order. | `undefined` | +| `advanced.excludedEnvVars` | array of strings | Environment variables to exclude from project context. Specifies environment variables that should be excluded from being loaded from project `.env` files. This prevents project-specific environment variables (like `DEBUG=true`) from interfering with the CLI behavior. Variables from `.qwen/.env` files are never excluded. | `["DEBUG","DEBUG_MODE"]` | +| `advanced.bugCommand` | object | Configuration for the bug report command. Overrides the default URL for the `/bug` command. Properties: `urlTemplate` (string): A URL that can contain `{title}` and `{info}` placeholders. Example: `"bugCommand": { "urlTemplate": "https://bug.example.com/new?title={title}&info={info}" }` | `undefined` | +| `plansDirectory` | string | Custom directory for approved Plan Mode files. Relative paths are resolved from the project root, and the resolved path must stay within the project root. If unset, plan files are stored in `~/.qwen/plans`. **Requires restart.** If the directory is inside the project root, add it to `.gitignore` to avoid committing plan files. | `undefined` | #### mcpServers @@ -487,6 +482,7 @@ Here is an example of a `settings.json` file with the nested structure, new as o ``` { "proxy": "http://localhost:7890", + "plansDirectory": "./.qwen/plans", "general": { "vimMode": true, "preferredEditor": "code" diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index 6e4eec27a..6d2e283c9 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -2409,6 +2409,17 @@ describe('loadCliConfig with includeDirectories', () => { ToolNames.SHELL, ]); }); + + it('should preserve plansDirectory in bare mode', async () => { + process.argv = ['node', 'script.js', '--bare']; + const argv = await parseArguments(); + const settings: Settings = { + plansDirectory: './project-plans', + }; + const config = await loadCliConfig(settings, argv, undefined, []); + + expect(config.getPlansDir()).toContain('project-plans'); + }); }); describe('loadCliConfig chatCompression', () => { @@ -3393,3 +3404,41 @@ describe('loadCliConfig runtimeOutputDir', () => { expect(Storage.getRuntimeBaseDir()).toBe(Storage.getGlobalQwenDir()); }); }); + +describe('loadCliConfig plansDirectory', () => { + const originalArgv = process.argv; + + beforeEach(() => { + process.argv = ['node', 'script.js']; + }); + + afterEach(() => { + process.argv = originalArgv; + vi.restoreAllMocks(); + }); + + it('should resolve relative plansDirectory against cwd', async () => { + const argv = await parseArguments(); + const cwd = path.resolve('workspace', 'my-project'); + const settings: Settings = { + plansDirectory: './project-plans', + }; + + const config = await loadCliConfig(settings, argv, cwd); + + expect(config.getPlansDir()).toBe(path.join(cwd, 'project-plans')); + expect(config.getPlanFilePath()).toContain(path.join(cwd, 'project-plans')); + }); + + it('should reject plansDirectory values outside cwd', async () => { + const argv = await parseArguments(); + const cwd = path.resolve('workspace', 'my-project'); + const settings: Settings = { + plansDirectory: '../plans', + }; + + await expect(loadCliConfig(settings, argv, cwd)).rejects.toThrow( + 'plansDirectory must resolve within the project root', + ); + }); +}); diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 84dd7d2e4..a21a2ed03 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -1685,6 +1685,7 @@ export async function loadCliConfig( fileFiltering: settings.context?.fileFiltering, checkpointing: argv.checkpointing || settings.general?.checkpointing?.enabled, + plansDirectory: settings.plansDirectory, proxy: argv.proxy || settings.proxy || diff --git a/packages/cli/src/config/settingsSchema.test.ts b/packages/cli/src/config/settingsSchema.test.ts index 865328814..fbf4fe073 100644 --- a/packages/cli/src/config/settingsSchema.test.ts +++ b/packages/cli/src/config/settingsSchema.test.ts @@ -22,13 +22,13 @@ describe('SettingsSchema', () => { 'ide', 'privacy', 'telemetry', - 'proxy', 'model', 'context', 'tools', 'mcp', 'security', 'advanced', + 'plansDirectory', ]; expectedSettings.forEach((setting) => { @@ -129,6 +129,15 @@ describe('SettingsSchema', () => { expect(getSettingsSchema().proxy.showInDialog).toBe(false); }); + it('should have plansDirectory setting in schema', () => { + expect(getSettingsSchema().plansDirectory).toBeDefined(); + expect(getSettingsSchema().plansDirectory.type).toBe('string'); + expect(getSettingsSchema().plansDirectory.category).toBe('Advanced'); + expect(getSettingsSchema().plansDirectory.default).toBe(undefined); + expect(getSettingsSchema().plansDirectory.requiresRestart).toBe(true); + expect(getSettingsSchema().plansDirectory.showInDialog).toBe(false); + }); + it('should have unique categories', () => { const categories = new Set(); @@ -251,7 +260,6 @@ describe('SettingsSchema', () => { includeDirectories: ['/path/to/dir'], loadFromIncludeDirectories: true, }, - proxy: 'http://localhost:7890', }; // TypeScript should not complain about these properties @@ -259,7 +267,6 @@ describe('SettingsSchema', () => { expect(settings.ui?.renderMode).toBe('raw'); expect(settings.context?.includeDirectories).toEqual(['/path/to/dir']); expect(settings.context?.loadFromIncludeDirectories).toBe(true); - expect(settings.proxy).toBe('http://localhost:7890'); }); it('should have includeDirectories setting in schema', () => { diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index df02d02d3..f206200c1 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -300,6 +300,17 @@ const SETTINGS_SCHEMA = { mergeStrategy: MergeStrategy.REPLACE, }, + plansDirectory: { + type: 'string', + label: 'Plans Directory', + category: 'Advanced', + requiresRestart: true, + default: undefined as string | undefined, + description: + 'Custom directory for approved Plan Mode files. Relative paths are resolved from the project root, and the resolved path must stay within the project root. Defaults to ~/.qwen/plans.', + showInDialog: false, + }, + // Environment variables fallback env: { type: 'object', diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 876228b26..172b3983d 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -8,6 +8,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import type { Mock } from 'vitest'; import type { ConfigParameters, SandboxConfig } from './config.js'; import { Config, ApprovalMode, MCPServerConfig } from './config.js'; +import { Storage } from './storage.js'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { setGeminiMdFilename as mockSetGeminiMdFilename } from '../memory/const.js'; @@ -60,12 +61,16 @@ vi.mock('node:fs', async (importOriginal) => { const mocked = { ...actual, existsSync: vi.fn().mockReturnValue(true), + readdirSync: vi.fn().mockReturnValue([]), statSync: vi.fn().mockReturnValue({ isDirectory: vi.fn().mockReturnValue(true), }), realpathSync: vi.fn((path) => path), mkdirSync: vi.fn(), writeFileSync: vi.fn(), + renameSync: vi.fn(), + copyFileSync: vi.fn(), + unlinkSync: vi.fn(), readFileSync: vi.fn(), }; return { @@ -289,6 +294,18 @@ describe('Server Config (config.ts)', () => { beforeEach(() => { // Reset mocks if necessary vi.clearAllMocks(); + (fs.existsSync as Mock).mockReturnValue(true); + (fs.readdirSync as Mock).mockReturnValue([]); + (fs.statSync as Mock).mockReturnValue({ + isDirectory: vi.fn().mockReturnValue(true), + }); + vi.mocked(fs.realpathSync).mockImplementation((path) => path.toString()); + (fs.mkdirSync as Mock).mockImplementation(() => undefined); + (fs.writeFileSync as Mock).mockImplementation(() => undefined); + (fs.renameSync as Mock).mockImplementation(() => undefined); + (fs.copyFileSync as Mock).mockImplementation(() => undefined); + (fs.unlinkSync as Mock).mockImplementation(() => undefined); + (fs.readFileSync as Mock).mockImplementation(() => undefined); vi.mocked(isTelemetrySdkInitialized).mockReturnValue(false); vi.spyOn(QwenLogger.prototype, 'logStartSessionEvent').mockImplementation( async () => undefined, @@ -2161,7 +2178,7 @@ describe('setApprovalMode with folder trust', () => { }); describe('plan file persistence', () => { - it('should save plan to disk', () => { + it('should save plan to disk atomically', () => { const config = new Config(baseParams); config.savePlan('# My Plan\n1. Step one\n2. Step two'); @@ -2170,11 +2187,17 @@ describe('setApprovalMode with folder trust', () => { expect.stringContaining('plans'), { recursive: true }, ); + // Writes to temp file first expect(fs.writeFileSync).toHaveBeenCalledWith( - expect.stringContaining('.md'), + expect.stringContaining('.tmp'), '# My Plan\n1. Step one\n2. Step two', 'utf-8', ); + // Then atomically renames to final path + expect(fs.renameSync).toHaveBeenCalledWith( + expect.stringContaining('.tmp'), + expect.stringContaining('.md'), + ); }); it('should load plan from disk', () => { @@ -2218,6 +2241,323 @@ describe('setApprovalMode with folder trust', () => { expect(filePath).toContain('test-session-123'); expect(filePath).toMatch(/\.md$/); }); + + it('should sanitize session ID when building plan file path', () => { + const config = new Config({ + ...baseParams, + sessionId: '../../../escape', + plansDirectory: './project-plans', + }); + + expect(config.getPlanFilePath()).toBe( + path.join( + path.resolve(baseParams.targetDir), + 'project-plans', + 'escape.md', + ), + ); + }); + + it('should use configured plansDirectory for plan file path', () => { + const config = new Config({ + ...baseParams, + sessionId: 'test-session-123', + plansDirectory: './project-plans', + }); + + expect(config.getPlansDir()).toBe( + path.join(path.resolve(baseParams.targetDir), 'project-plans'), + ); + expect(config.getPlanFilePath()).toBe( + path.join( + path.resolve(baseParams.targetDir), + 'project-plans', + 'test-session-123.md', + ), + ); + }); + + it('should save and load plan from configured plansDirectory', () => { + const config = new Config({ + ...baseParams, + sessionId: 'test-session-123', + plansDirectory: './project-plans', + }); + const targetDir = path.resolve(baseParams.targetDir); + const plansDir = path.join(targetDir, 'project-plans'); + const filePath = path.join(plansDir, 'test-session-123.md'); + const tmpPath = `${filePath}.tmp`; + const storedFiles = new Map(); + (fs.writeFileSync as Mock).mockImplementation((pathToWrite, contents) => { + storedFiles.set(pathToWrite.toString(), contents.toString()); + }); + (fs.renameSync as Mock).mockImplementation((fromPath, toPath) => { + const contents = storedFiles.get(fromPath.toString()); + if (contents === undefined) { + throw new Error(`missing temp file: ${fromPath.toString()}`); + } + storedFiles.set(toPath.toString(), contents); + storedFiles.delete(fromPath.toString()); + }); + (fs.readFileSync as Mock).mockImplementation((pathToRead) => { + const contents = storedFiles.get(pathToRead.toString()); + if (contents === undefined) { + const enoent = new Error('ENOENT') as NodeJS.ErrnoException; + enoent.code = 'ENOENT'; + throw enoent; + } + return contents; + }); + + config.savePlan('# My Plan'); + + expect(fs.mkdirSync).toHaveBeenCalledWith(plansDir, { recursive: true }); + expect(fs.writeFileSync).toHaveBeenCalledWith( + tmpPath, + '# My Plan', + 'utf-8', + ); + expect(fs.renameSync).toHaveBeenCalledWith(tmpPath, filePath); + expect(config.loadPlan()).toBe('# My Plan'); + expect(fs.readFileSync).toHaveBeenCalledWith(filePath, 'utf-8'); + }); + + it('should fall back to copyFileSync when renameSync hits EXDEV', () => { + const config = new Config({ + ...baseParams, + sessionId: 'test-session-123', + plansDirectory: './project-plans', + }); + const exdevError = new Error('EXDEV') as NodeJS.ErrnoException; + exdevError.code = 'EXDEV'; + (fs.renameSync as Mock).mockImplementation(() => { + throw exdevError; + }); + + config.savePlan('# My Plan'); + + expect(fs.copyFileSync).toHaveBeenCalledWith( + expect.stringContaining('.tmp'), + expect.stringContaining('project-plans'), + ); + expect(fs.unlinkSync).toHaveBeenCalledWith( + expect.stringContaining('.tmp'), + ); + }); + + it('should remove plan file when post-write containment check fails', () => { + const config = new Config({ + ...baseParams, + sessionId: 'test-session-123', + plansDirectory: './project-plans', + }); + const targetDir = path.resolve(baseParams.targetDir); + const plansDir = path.join(targetDir, 'project-plans'); + const filePath = path.join(plansDir, 'test-session-123.md'); + const outsideFilePath = path.resolve( + path.dirname(targetDir), + 'outside-plans', + 'test-session-123.md', + ); + vi.mocked(fs.realpathSync).mockImplementation((pathToResolve) => { + const resolvedPath = pathToResolve.toString(); + if (resolvedPath === targetDir || resolvedPath === plansDir) { + return resolvedPath; + } + if (resolvedPath === filePath) { + return outsideFilePath; + } + return resolvedPath; + }); + + try { + expect(() => config.savePlan('# My Plan')).toThrow( + 'plansDirectory must resolve within the project root', + ); + expect(fs.unlinkSync).toHaveBeenCalledWith(filePath); + } finally { + vi.mocked(fs.realpathSync).mockImplementation((pathToResolve) => + pathToResolve.toString(), + ); + } + }); + + it('should reject loading a plan when final file path escapes targetDir', () => { + const config = new Config({ + ...baseParams, + sessionId: 'test-session-123', + plansDirectory: './project-plans', + }); + const targetDir = path.resolve(baseParams.targetDir); + const plansDir = path.join(targetDir, 'project-plans'); + const filePath = path.join(plansDir, 'test-session-123.md'); + const outsideFilePath = path.resolve( + path.dirname(targetDir), + 'outside-plans', + 'test-session-123.md', + ); + vi.mocked(fs.readFileSync).mockClear(); + vi.mocked(fs.realpathSync).mockImplementation((pathToResolve) => { + const resolvedPath = pathToResolve.toString(); + if (resolvedPath === targetDir || resolvedPath === plansDir) { + return resolvedPath; + } + if (resolvedPath === filePath) { + return outsideFilePath; + } + return resolvedPath; + }); + + try { + expect(() => config.loadPlan()).toThrow( + 'plansDirectory must resolve within the project root', + ); + expect(fs.readFileSync).not.toHaveBeenCalled(); + } finally { + vi.mocked(fs.realpathSync).mockImplementation((pathToResolve) => + pathToResolve.toString(), + ); + } + }); + + it('should warn when configured plansDirectory hides a legacy plan file', () => { + const targetDir = path.resolve(baseParams.targetDir); + const currentPlansDir = path.join(targetDir, 'project-plans'); + const legacyPlansDir = Storage.getPlansDir(); + (fs.readdirSync as Mock).mockImplementation((pathToCheck) => { + const resolvedPath = pathToCheck.toString(); + if (resolvedPath === currentPlansDir) { + return []; + } + if (resolvedPath === legacyPlansDir) { + return ['other-session.md']; + } + return []; + }); + + try { + const config = new Config({ + ...baseParams, + plansDirectory: './project-plans', + }); + + expect(config.getWarnings()).toContainEqual( + expect.stringContaining(legacyPlansDir), + ); + expect(config.getWarnings()).toContainEqual( + expect.stringContaining('plansDirectory is configured'), + ); + } finally { + (fs.readdirSync as Mock).mockReturnValue([]); + } + }); + + it('should warn when configured plansDirectory has only some legacy plan files', () => { + const targetDir = path.resolve(baseParams.targetDir); + const currentPlansDir = path.join(targetDir, 'project-plans'); + const legacyPlansDir = Storage.getPlansDir(); + (fs.readdirSync as Mock).mockImplementation((pathToCheck) => { + const resolvedPath = pathToCheck.toString(); + if (resolvedPath === currentPlansDir) { + return ['migrated-session.md']; + } + if (resolvedPath === legacyPlansDir) { + return ['migrated-session.md', 'hidden-session.md']; + } + return []; + }); + + try { + const config = new Config({ + ...baseParams, + plansDirectory: './project-plans', + }); + + expect(config.getWarnings()).toContainEqual( + expect.stringContaining(legacyPlansDir), + ); + } finally { + (fs.readdirSync as Mock).mockReturnValue([]); + } + }); + + it('should surface legacy plan directory read failures as warnings', () => { + const legacyError = new Error('EACCES') as NodeJS.ErrnoException; + legacyError.code = 'EACCES'; + (fs.readdirSync as Mock).mockImplementation((pathToCheck) => { + const resolvedPath = pathToCheck.toString(); + if ( + resolvedPath === + path.join(path.resolve(baseParams.targetDir), 'project-plans') + ) { + return []; + } + throw legacyError; + }); + + try { + const config = new Config({ + ...baseParams, + plansDirectory: './project-plans', + }); + + expect(config.getWarnings()).toContainEqual( + expect.stringContaining('Failed to read plan directory'), + ); + } finally { + (fs.readdirSync as Mock).mockReturnValue([]); + } + }); + + it('should reject configured plansDirectory outside targetDir', () => { + expect( + () => + new Config({ + ...baseParams, + plansDirectory: '../project-plans', + }), + ).toThrow('plansDirectory must resolve within the project root'); + }); + + it('should revalidate configured plansDirectory before plan I/O', () => { + const config = new Config({ + ...baseParams, + plansDirectory: './project-plans', + }); + vi.mocked(fs.mkdirSync).mockClear(); + vi.mocked(fs.readFileSync).mockClear(); + const targetDir = path.resolve(baseParams.targetDir); + const plansDir = path.join(targetDir, 'project-plans'); + const outsidePlansDir = path.resolve( + path.dirname(targetDir), + 'outside-plans', + ); + vi.mocked(fs.realpathSync).mockImplementation((pathToResolve) => { + const resolvedPath = pathToResolve.toString(); + if (resolvedPath === targetDir) { + return targetDir; + } + if (resolvedPath === plansDir) { + return outsidePlansDir; + } + return resolvedPath; + }); + + try { + expect(() => config.savePlan('# My Plan')).toThrow( + 'plansDirectory must resolve within the project root', + ); + expect(() => config.loadPlan()).toThrow( + 'plansDirectory must resolve within the project root', + ); + expect(fs.mkdirSync).not.toHaveBeenCalled(); + expect(fs.readFileSync).not.toHaveBeenCalled(); + } finally { + vi.mocked(fs.realpathSync).mockImplementation((pathToResolve) => + pathToResolve.toString(), + ); + } + }); }); describe('registerCoreTools', () => { diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 94e769a55..a8e539692 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -491,6 +491,8 @@ export interface ConfigParameters { }; checkpointing?: boolean; fileCheckpointingEnabled?: boolean; + /** Directory where approved plan files are stored. Must resolve inside targetDir. */ + plansDirectory?: string; proxy?: string; cwd: string; fileDiscoveryService?: FileDiscoveryService; @@ -823,6 +825,8 @@ export class Config { private readonly jsonFile: string | undefined; private readonly jsonSchema: Record | undefined; private readonly inputFile: string | undefined; + private readonly plansDir: string; + private readonly plansDirectoryConfigured: boolean; private readonly defaultFileEncoding: FileEncodingType | undefined; private readonly enableManagedAutoMemory: boolean; private readonly enableManagedAutoDream: boolean; @@ -850,6 +854,8 @@ export class Config { this.fileSystemService = new StandardFileSystemService(); this.sandbox = params.sandbox; this.targetDir = path.resolve(params.targetDir); + this.plansDirectoryConfigured = Boolean(params.plansDirectory?.trim()); + this.plansDir = Storage.getPlansDir(this.targetDir, params.plansDirectory); this.explicitIncludeDirectories = Array.from( new Set(params.includeDirectories ?? []), ); @@ -961,6 +967,7 @@ export class Config { this.skipStartupContext = params.skipStartupContext ?? false; this.bareMode = params.bareMode ?? false; this.warnings = params.warnings ?? []; + this.addLegacyPlanLocationWarning(); this.allowedHttpHookUrls = params.allowedHttpHookUrls ?? []; this.onPersistPermissionRuleCallback = params.onPersistPermissionRule; @@ -2483,28 +2490,143 @@ export class Config { this.approvalMode = mode; } + /** + * Returns the directory where this session's plan file is stored. + */ + getPlansDir(): string { + return this.plansDir; + } + + private assertPlansDirWithinTargetDir(): void { + if (!this.plansDirectoryConfigured) { + return; + } + + Storage.assertPathWithinDirectory( + this.plansDir, + this.targetDir, + `plansDirectory must resolve within the project root.`, + ); + } + + private assertPlanFilePathWithinTargetDir(filePath: string): void { + if (!this.plansDirectoryConfigured) { + return; + } + + Storage.assertPathWithinDirectory( + filePath, + this.targetDir, + `plansDirectory must resolve within the project root.`, + ); + } + + private addLegacyPlanLocationWarning(): void { + try { + if (!this.plansDirectoryConfigured) { + return; + } + + const legacyPlansDir = Storage.getPlansDir(); + const legacyPlanFiles = this.getPlanFileNames(legacyPlansDir); + if (legacyPlanFiles.length === 0) { + return; + } + + const configuredPlanFiles = new Set(this.getPlanFileNames(this.plansDir)); + const hiddenLegacyPlanFiles = legacyPlanFiles.filter( + (fileName) => !configuredPlanFiles.has(fileName), + ); + if (hiddenLegacyPlanFiles.length === 0) { + return; + } + + this.warnings.push( + `Warning: Saved plan files exist at ${legacyPlansDir}, but ` + + `plansDirectory is configured to use ${this.plansDir}. Move ` + + `existing plan files to ${this.plansDir} if you want to keep ` + + `using them.`, + ); + } catch (err: unknown) { + const message = `Failed to check legacy plan directory migration warning: ${ + err instanceof Error ? err.message : String(err) + }`; + this.warnings.push(message); + this.debugLogger.warn(message, err); + } + } + + private getPlanFileNames(plansDir: string): string[] { + try { + return fs.readdirSync(plansDir).filter((entry) => entry.endsWith('.md')); + } catch (err: unknown) { + const code = (err as NodeJS.ErrnoException).code; + if (code === 'ENOENT') { + return []; + } + if (code === 'EACCES' || code === 'EPERM') { + const message = `Failed to read plan directory ${plansDir}: ${ + err instanceof Error ? err.message : String(err) + }`; + this.warnings.push(message); + this.debugLogger.warn(message, err); + return []; + } + throw err; + } + } + /** * Returns the file path for this session's plan file. */ getPlanFilePath(): string { - return Storage.getPlanFilePath(this.sessionId); + return path.join( + this.plansDir, + `${Storage.sanitizePlanSessionId(this.sessionId)}.md`, + ); } /** * Saves a plan to disk for the current session. */ savePlan(plan: string): void { + this.assertPlansDirWithinTargetDir(); const filePath = this.getPlanFilePath(); const dir = path.dirname(filePath); fs.mkdirSync(dir, { recursive: true }); - fs.writeFileSync(filePath, plan, 'utf-8'); + // Write to a temp file first, then atomically rename to avoid + // leaving a corrupted file if the process crashes mid-write. + const tmpPath = filePath + '.tmp'; + fs.writeFileSync(tmpPath, plan, 'utf-8'); + try { + fs.renameSync(tmpPath, filePath); + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code !== 'EXDEV') { + throw err; + } + + fs.copyFileSync(tmpPath, filePath); + fs.unlinkSync(tmpPath); + } + try { + this.assertPlanFilePathWithinTargetDir(filePath); + } catch (err) { + try { + fs.unlinkSync(filePath); + } catch { + // Ignore rollback errors; the containment check already failed. + } + throw err; + } } /** * Loads the plan for the current session, or returns undefined if none exists. */ loadPlan(): string | undefined { + this.assertPlansDirWithinTargetDir(); const filePath = this.getPlanFilePath(); + this.assertPlanFilePathWithinTargetDir(filePath); try { return fs.readFileSync(filePath, 'utf-8'); } catch (error: unknown) { diff --git a/packages/core/src/config/storage.test.ts b/packages/core/src/config/storage.test.ts index 0c7623d20..419480d10 100644 --- a/packages/core/src/config/storage.test.ts +++ b/packages/core/src/config/storage.test.ts @@ -4,11 +4,48 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import * as os from 'node:os'; import * as path from 'node:path'; import { Storage } from './storage.js'; +const mockRealpathSync = vi.hoisted(() => vi.fn()); + +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal(); + const mocked = { + ...actual, + realpathSync: mockRealpathSync, + }; + return { + ...mocked, + default: mocked, + }; +}); + +const actualFs = await vi.importActual('node:fs'); + +function createEnoent(pathToResolve: string): NodeJS.ErrnoException { + const error = new Error( + `ENOENT: no such file or directory, realpath '${pathToResolve}'`, + ) as NodeJS.ErrnoException; + error.code = 'ENOENT'; + return error; +} + +function mockRealpath( + resolutions: Map, + missingPaths = new Set(), +): void { + mockRealpathSync.mockImplementation((pathToResolve) => { + const resolvedPath = pathToResolve.toString(); + if (missingPaths.has(resolvedPath)) { + throw createEnoent(resolvedPath); + } + return resolutions.get(resolvedPath) ?? resolvedPath; + }); +} + describe('Storage – getGlobalSettingsPath', () => { it('returns path to ~/.qwen/settings.json', () => { const expected = path.join(os.homedir(), '.qwen', 'settings.json'); @@ -157,6 +194,152 @@ describe('Storage – getRuntimeBaseDir / setRuntimeBaseDir', () => { }); }); +describe('Storage – getPlansDir', () => { + const projectRoot = path.resolve('workspace', 'project'); + + beforeEach(() => { + mockRealpathSync.mockImplementation((pathToResolve) => + actualFs.realpathSync(pathToResolve), + ); + }); + + afterEach(() => { + mockRealpathSync.mockReset(); + }); + + it('defaults to ~/.qwen/plans when plansDirectory is not configured', () => { + expect(Storage.getPlansDir(projectRoot)).toBe( + path.join(Storage.getGlobalQwenDir(), 'plans'), + ); + }); + + it('resolves relative plansDirectory values against the project root', () => { + expect(Storage.getPlansDir(projectRoot, './project-plans')).toBe( + path.join(projectRoot, 'project-plans'), + ); + }); + + it('expands tilde in configured plansDirectory values', () => { + const projectInHome = path.join(os.homedir(), 'workspace', 'project'); + expect( + Storage.getPlansDir(projectInHome, '~/workspace/project/plans'), + ).toBe(path.join(projectInHome, 'plans')); + }); + + it('allows absolute plansDirectory values inside the project root', () => { + const plansDir = path.join(projectRoot, 'nested', 'plans'); + expect(Storage.getPlansDir(projectRoot, plansDir)).toBe(plansDir); + }); + + it('rejects relative plansDirectory values that escape the project root', () => { + expect(() => Storage.getPlansDir(projectRoot, '../plans')).toThrow( + 'plansDirectory must resolve within the project root', + ); + }); + + it('rejects absolute plansDirectory values outside the project root', () => { + const outsideProject = path.join(path.dirname(projectRoot), 'plans'); + expect(() => Storage.getPlansDir(projectRoot, outsideProject)).toThrow( + 'plansDirectory must resolve within the project root', + ); + }); + + it('requires projectRoot when plansDirectory is configured', () => { + expect(() => Storage.getPlansDir(undefined, './plans')).toThrow( + 'projectRoot is required when plansDirectory is configured', + ); + expect(() => Storage.getPlansDir(null, './plans')).toThrow( + 'projectRoot is required when plansDirectory is configured', + ); + }); + + it('rejects Windows-style absolute path outside the project root', () => { + // Simulate project root on C: drive and plansDirectory on D: drive + const projectOnC = path.resolve('C:', 'work', 'project'); + const plansOnD = path.resolve('D:', 'plans'); + expect(() => Storage.getPlansDir(projectOnC, plansOnD)).toThrow( + 'plansDirectory must resolve within the project root', + ); + }); + + it('rejects path with mixed separators that escapes project root', () => { + // On Windows, path.resolve normalizes backslashes as path separators. + // On POSIX, backslashes are literal characters, so this traversal + // is inherently Windows-specific and should be guarded. + if (process.platform !== 'win32') { + return; + } + const tricky = '..\\..\\plans'; // backslashes with traversal + expect(() => Storage.getPlansDir(projectRoot, tricky)).toThrow( + 'plansDirectory must resolve within the project root', + ); + }); + + it('rejects symlink pointing outside the project root', () => { + const project = path.resolve('tmp', 'project'); + const outside = path.resolve('tmp', 'outside'); + const symlink = path.join(project, 'escape-link'); + mockRealpath( + new Map([ + [project, project], + [symlink, outside], + ]), + ); + + expect(() => Storage.getPlansDir(project, './escape-link')).toThrow( + 'plansDirectory must resolve within the project root', + ); + }); + + it('allows legitimate symlink that stays within project root', () => { + const project = path.resolve('tmp', 'project'); + const target = path.join(project, 'plans-target'); + const symlink = path.join(project, 'plans-link'); + mockRealpath( + new Map([ + [project, project], + [symlink, target], + ]), + ); + + const result = Storage.getPlansDir(project, './plans-link'); + // The configured symlink path is accepted as long as it stays inside + // the project root. + expect(result).toBe(symlink); + }); + + it('rejects missing nested path under symlink that escapes project root', () => { + const project = path.resolve('tmp', 'project'); + const outside = path.resolve('tmp', 'outside'); + const dataSymlink = path.join(project, 'data'); + const missingSubdir = path.join(dataSymlink, 'subdir'); + const missingPlans = path.join(missingSubdir, 'plans'); + mockRealpath( + new Map([ + [project, project], + [dataSymlink, outside], + ]), + new Set([missingPlans, missingSubdir]), + ); + + expect(() => Storage.getPlansDir(project, './data/subdir/plans')).toThrow( + 'plansDirectory must resolve within the project root', + ); + }); + + it('uses configured plansDirectory when building plan file paths', () => { + expect(Storage.getPlanFilePath('session-123', projectRoot, './plans')).toBe( + path.join(projectRoot, 'plans', 'session-123.md'), + ); + }); + + it('sanitizes session IDs when building plan file paths', () => { + expect( + Storage.getPlanFilePath('../../../escape', projectRoot, './plans'), + ).toBe(path.join(projectRoot, 'plans', 'escape.md')); + }); +}); + describe('Storage – runtime path methods use getRuntimeBaseDir', () => { const originalEnv = process.env['QWEN_RUNTIME_DIR']; diff --git a/packages/core/src/config/storage.ts b/packages/core/src/config/storage.ts index 1ee97592a..71ccc7faa 100644 --- a/packages/core/src/config/storage.ts +++ b/packages/core/src/config/storage.ts @@ -9,6 +9,7 @@ import * as os from 'node:os'; import * as fs from 'node:fs'; import { AsyncLocalStorage } from 'node:async_hooks'; import { getProjectHash, QWEN_DIR, sanitizeCwd } from '../utils/paths.js'; +import { FatalConfigError } from '../utils/errors.js'; export { QWEN_DIR } from '../utils/paths.js'; export const GOOGLE_ACCOUNTS_FILENAME = 'google_accounts.json'; @@ -63,6 +64,22 @@ export class Storage { return resolved; } + /** + * Sanitizes a session id for use as a plan filename. + * + * Plan files are keyed by session id, but the raw id is public SDK input. + * Strip directory separators and Windows-invalid filename characters so a + * hostile value cannot escape the plans directory. + */ + static sanitizePlanSessionId(sessionId: string): string { + const safeName = path + .basename(sessionId.replace(/\\/g, '/')) + .replace(/^\.+/g, '_') + // eslint-disable-next-line no-control-regex + .replace(/[<>:"|?*\x00-\x1F]/g, '_'); + return safeName || '_'; + } + private static resolveRuntimeBaseDir( dir: string | null | undefined, cwd?: string, @@ -179,12 +196,100 @@ export class Storage { return path.join(Storage.getGlobalQwenDir(), IDE_DIR_NAME); } - static getPlansDir(): string { + /** + * Resolves pathToResolve by realpathing its deepest existing ancestor and + * appending the not-yet-created remainder. + */ + private static resolvePathThroughExistingAncestor( + pathToResolve: string, + ): string { + let candidate = pathToResolve; + while (true) { + try { + const realCandidate = fs.realpathSync(candidate); + const remainder = path.relative(candidate, pathToResolve); + return path.join(realCandidate, remainder); + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code !== 'ENOENT') { + throw err; + } + const parent = path.dirname(candidate); + if (parent === candidate) { + return pathToResolve; + } + candidate = parent; + } + } + } + + /** + * Checks whether {@link childPath} resides within {@link parentPath}, + * resolving symbolic links to prevent traversal bypass attacks. + */ + private static isPathWithinDirectory( + childPath: string, + parentPath: string, + ): boolean { + const realParent = Storage.resolvePathThroughExistingAncestor(parentPath); + const realChild = Storage.resolvePathThroughExistingAncestor(childPath); + + const relativePath = path.relative(realParent, realChild); + return ( + relativePath === '' || + (!relativePath.startsWith('..') && !path.isAbsolute(relativePath)) + ); + } + + static assertPathWithinDirectory( + childPath: string, + parentPath: string, + errorMessage: string, + ): void { + if (!Storage.isPathWithinDirectory(childPath, parentPath)) { + throw new FatalConfigError(errorMessage); + } + } + + static getPlansDir( + projectRoot?: string | null, + plansDirectory?: string | null, + ): string { + const configuredPlansDirectory = plansDirectory?.trim(); + if (configuredPlansDirectory) { + if (!projectRoot) { + throw new FatalConfigError( + 'projectRoot is required when plansDirectory is configured.', + ); + } + + const resolvedProjectRoot = path.resolve(projectRoot); + const resolvedPlansDirectory = Storage.resolvePath( + configuredPlansDirectory, + resolvedProjectRoot, + ); + + Storage.assertPathWithinDirectory( + resolvedPlansDirectory, + resolvedProjectRoot, + `plansDirectory must resolve within the project root.`, + ); + + return resolvedPlansDirectory; + } + return path.join(Storage.getGlobalQwenDir(), PLANS_DIR_NAME); } - static getPlanFilePath(sessionId: string): string { - return path.join(Storage.getPlansDir(), `${sessionId}.md`); + static getPlanFilePath( + sessionId: string, + projectRoot?: string | null, + plansDirectory?: string | null, + ): string { + // Kept for tests and SDK callers that still use Storage helpers directly. + return path.join( + Storage.getPlansDir(projectRoot, plansDirectory), + `${Storage.sanitizePlanSessionId(sessionId)}.md`, + ); } static getGlobalBinDir(): string { diff --git a/packages/vscode-ide-companion/schemas/settings.schema.json b/packages/vscode-ide-companion/schemas/settings.schema.json index 1e3c7be0c..32ad0fe77 100644 --- a/packages/vscode-ide-companion/schemas/settings.schema.json +++ b/packages/vscode-ide-companion/schemas/settings.schema.json @@ -18,6 +18,10 @@ "type": "object", "additionalProperties": true }, + "plansDirectory": { + "description": "Custom directory for approved Plan Mode files. Relative paths are resolved from the project root, and the resolved path must stay within the project root. Defaults to ~/.qwen/plans.", + "type": "string" + }, "env": { "description": "Environment variables to set as fallback defaults. These are loaded with the lowest priority: system environment variables > .env files > settings.json env field.", "type": "object",