mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-19 07:54:38 +00:00
feat(cli): add configurable plansDirectory for Plan Mode (#4062)
* feat(cli): add configurable plansDirectory for Plan Mode Add a plansDirectory setting that allows users to define a custom directory for approved Plan Mode files. Relative paths are resolved against the project root and validated to prevent path traversal. - Storage: add isPathWithinDirectory() with realpathSync-based symlink resolution to prevent traversal bypass attacks (direct, intermediate, and cross-drive) - Config: cache plansDir at construction time, use atomic write (write-temp then rename) to prevent corrupted plan files on crash - CLI: respect bareMode by clearing plansDirectory in minimal mode - Docs: document plansDirectory with requiresRestart and gitignore hint - Tests: 26 new tests covering path validation, symlink attacks (direct and intermediate), Windows cross-drive paths, mixed separators, and configuration integration Closes #3548 * fix(core): align symlink test with return value * fix(core): harden plans directory handling * fix(config): address PR #4062 review findings for plansDirectory - Handle EXDEV during atomic plan writes (cross-device rename fallback) - Sanitize session IDs to prevent path traversal in plan filenames - Expand tilde (~) in configured plansDirectory paths - Preserve plansDirectory in bare mode - Add EACCES/EPERM handling to getPlanFileNames with user-visible warnings - Close TOCTOU gap with post-write path containment validation - Fix docs to clarify plansDirectory is a top-level key - Add happy-path I/O tests for configured plansDirectory
This commit is contained in:
parent
d2d426fad0
commit
9985d91e08
10 changed files with 853 additions and 35 deletions
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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 ||
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
|
|
@ -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<string, string>();
|
||||
(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', () => {
|
||||
|
|
|
|||
|
|
@ -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<string, unknown> | 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) {
|
||||
|
|
|
|||
|
|
@ -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<typeof import('node:fs')>();
|
||||
const mocked = {
|
||||
...actual,
|
||||
realpathSync: mockRealpathSync,
|
||||
};
|
||||
return {
|
||||
...mocked,
|
||||
default: mocked,
|
||||
};
|
||||
});
|
||||
|
||||
const actualFs = await vi.importActual<typeof import('node:fs')>('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<string, string>,
|
||||
missingPaths = new Set<string>(),
|
||||
): 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'];
|
||||
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue