diff --git a/docs/users/features/commands.md b/docs/users/features/commands.md index 6ac20b12a16..cb3ef100cc7 100644 --- a/docs/users/features/commands.md +++ b/docs/users/features/commands.md @@ -21,7 +21,7 @@ These commands help you save, restore, and summarize work progress. | Command | Description | Usage Examples | | ---------------- | ------------------------------------------------------------------------ | ------------------------------------------------------------- | | `/init` | Analyze current directory and create initial context file | `/init` | -| `/summary` | Generate project summary based on conversation history | `/summary` | +| `/summary` | Generate project summary based on conversation history | `/summary` or `/summary docs/my-summary.md` | | `/compress` | Replace chat history with summary to save Tokens | `/compress` or `/summarize` | | `/compress-fast` | Fast compression without AI — strips old tool outputs and thinking parts | `/compress-fast` | | `/resume` | Resume a previous conversation session | `/resume` or `/continue` | @@ -38,6 +38,10 @@ These commands help you save, restore, and summarize work progress. > > `/summarize` is an alias for `/compress` (it compresses chat history — a destructive operation). To generate a non-destructive project summary instead, use `/summary`. +> [!note] +> +> `/summary` accepts an optional `[path]` argument to save the summary to a custom location within the project root. Without an argument, it saves to `.qwen/PROJECT_SUMMARY.md`. Custom-path summaries are not detected by the welcome-back flow (`ui.enableWelcomeBack`), which only reads the default `.qwen/PROJECT_SUMMARY.md` location. + ### 1.2 Interface and Workspace Control Commands for adjusting interface appearance and work environment. diff --git a/packages/cli/src/i18n/locales/ca.js b/packages/cli/src/i18n/locales/ca.js index 3f12283ad3e..b2a187ab95c 100644 --- a/packages/cli/src/i18n/locales/ca.js +++ b/packages/cli/src/i18n/locales/ca.js @@ -1117,6 +1117,14 @@ export default { "Ja s'està generant el resum, espereu que acabi la sol·licitud anterior", 'No conversation found to summarize.': "No s'ha trobat cap conversa per resumir.", + 'Summary path already exists and is not a generated summary: {{path}}': + 'El camí del resum ja existeix i no és un resum generat: {{path}}', + 'Summary path must be within the project root.': + 'El camí del resum ha de ser dins de la arrel del projecte.', + 'Summary path resolves to an existing directory: {{path}}': + 'El camí del resum es resol a un directori existent: {{path}}', + 'Summary path ends with a separator but is an existing file: {{path}}': + 'El camí del resum acaba amb un separador però és un fitxer existent: {{path}}', 'Failed to generate project context summary: {{error}}': 'Error en generar el resum del context del projecte: {{error}}', 'Saved project summary to {{filePathForDisplay}}.': diff --git a/packages/cli/src/i18n/locales/de.js b/packages/cli/src/i18n/locales/de.js index 49c706e84f4..42db5e6e59e 100644 --- a/packages/cli/src/i18n/locales/de.js +++ b/packages/cli/src/i18n/locales/de.js @@ -992,6 +992,14 @@ export default { 'Zusammenfassung wird bereits generiert, warten Sie auf Abschluss der vorherigen Anfrage', 'No conversation found to summarize.': 'Kein Gespräch zum Zusammenfassen gefunden.', + 'Summary path already exists and is not a generated summary: {{path}}': + 'Der Zusammenfassungspfad existiert bereits und ist keine generierte Zusammenfassung: {{path}}', + 'Summary path must be within the project root.': + 'Der Zusammenfassungspfad muss sich im Projektstammverzeichnis befinden.', + 'Summary path resolves to an existing directory: {{path}}': + 'Der Zusammenfassungspfad verweist auf ein vorhandenes Verzeichnis: {{path}}', + 'Summary path ends with a separator but is an existing file: {{path}}': + 'Der Zusammenfassungspfad endet mit einem Trennzeichen, ist aber eine vorhandene Datei: {{path}}', 'Failed to generate project context summary: {{error}}': 'Fehler beim Generieren der Projektkontextzusammenfassung: {{error}}', 'Saved project summary to {{filePathForDisplay}}.': diff --git a/packages/cli/src/i18n/locales/en.js b/packages/cli/src/i18n/locales/en.js index ce366717732..a478ee36853 100644 --- a/packages/cli/src/i18n/locales/en.js +++ b/packages/cli/src/i18n/locales/en.js @@ -1470,6 +1470,14 @@ export default { 'Already generating summary, wait for previous request to complete': 'Already generating summary, wait for previous request to complete', 'No conversation found to summarize.': 'No conversation found to summarize.', + 'Summary path already exists and is not a generated summary: {{path}}': + 'Summary path already exists and is not a generated summary: {{path}}', + 'Summary path must be within the project root.': + 'Summary path must be within the project root.', + 'Summary path resolves to an existing directory: {{path}}': + 'Summary path resolves to an existing directory: {{path}}', + 'Summary path ends with a separator but is an existing file: {{path}}': + 'Summary path ends with a separator but is an existing file: {{path}}', 'Failed to generate project context summary: {{error}}': 'Failed to generate project context summary: {{error}}', 'Saved project summary to {{filePathForDisplay}}.': diff --git a/packages/cli/src/i18n/locales/fr.js b/packages/cli/src/i18n/locales/fr.js index f06aa98a2db..737ce36151b 100644 --- a/packages/cli/src/i18n/locales/fr.js +++ b/packages/cli/src/i18n/locales/fr.js @@ -1123,6 +1123,14 @@ export default { 'Génération de résumé déjà en cours, attendez que la demande précédente se termine', 'No conversation found to summarize.': 'Aucune conversation trouvée à résumer.', + 'Summary path already exists and is not a generated summary: {{path}}': + "Le chemin du résumé existe déjà et n'est pas un résumé généré : {{path}}", + 'Summary path must be within the project root.': + 'Le chemin du résumé doit se trouver dans la racine du projet.', + 'Summary path resolves to an existing directory: {{path}}': + 'Le chemin du résumé correspond à un répertoire existant : {{path}}', + 'Summary path ends with a separator but is an existing file: {{path}}': + 'Le chemin du résumé se termine par un séparateur mais est un fichier existant : {{path}}', 'Failed to generate project context summary: {{error}}': 'Échec de la génération du résumé du contexte du projet : {{error}}', 'Saved project summary to {{filePathForDisplay}}.': diff --git a/packages/cli/src/i18n/locales/ja.js b/packages/cli/src/i18n/locales/ja.js index c2d8439b4e5..a1ab99889ad 100644 --- a/packages/cli/src/i18n/locales/ja.js +++ b/packages/cli/src/i18n/locales/ja.js @@ -763,6 +763,14 @@ export default { 'Already generating summary, wait for previous request to complete': 'サマリー生成中です。前のリクエストの完了をお待ちください', 'No conversation found to summarize.': '要約する会話が見つかりません', + 'Summary path already exists and is not a generated summary: {{path}}': + 'サマリーパスは既に存在し、生成されたサマリーではありません: {{path}}', + 'Summary path must be within the project root.': + 'サマリーパスはプロジェクトルート内にある必要があります', + 'Summary path resolves to an existing directory: {{path}}': + 'サマリーパスは既存のディレクトリに解決されます: {{path}}', + 'Summary path ends with a separator but is an existing file: {{path}}': + 'サマリーパスは区切り文字で終わっていますが、既存のファイルです: {{path}}', 'Failed to generate project context summary: {{error}}': 'プロジェクトコンテキストサマリーの生成に失敗: {{error}}', 'Saved project summary to {{filePathForDisplay}}.': diff --git a/packages/cli/src/i18n/locales/pt.js b/packages/cli/src/i18n/locales/pt.js index 4d25ca0783c..1cca737f082 100644 --- a/packages/cli/src/i18n/locales/pt.js +++ b/packages/cli/src/i18n/locales/pt.js @@ -997,6 +997,14 @@ export default { 'Já gerando resumo, aguarde a conclusão da solicitação anterior', 'No conversation found to summarize.': 'Nenhuma conversa encontrada para resumir.', + 'Summary path already exists and is not a generated summary: {{path}}': + 'O caminho do resumo já existe e não é um resumo gerado: {{path}}', + 'Summary path must be within the project root.': + 'O caminho do resumo deve estar dentro da raiz do projeto.', + 'Summary path resolves to an existing directory: {{path}}': + 'O caminho do resumo resolve para um diretório existente: {{path}}', + 'Summary path ends with a separator but is an existing file: {{path}}': + 'O caminho do resumo termina com um separador, mas é um arquivo existente: {{path}}', 'Failed to generate project context summary: {{error}}': 'Falha ao gerar resumo do contexto do projeto: {{error}}', 'Saved project summary to {{filePathForDisplay}}.': diff --git a/packages/cli/src/i18n/locales/ru.js b/packages/cli/src/i18n/locales/ru.js index 4ffbe37705b..d2c524fc46d 100644 --- a/packages/cli/src/i18n/locales/ru.js +++ b/packages/cli/src/i18n/locales/ru.js @@ -1005,6 +1005,14 @@ export default { 'Генерация сводки уже выполняется, дождитесь завершения предыдущего запроса', 'No conversation found to summarize.': 'Не найдено диалогов для создания сводки.', + 'Summary path already exists and is not a generated summary: {{path}}': + 'Путь сводки уже существует и не является сгенерированной сводкой: {{path}}', + 'Summary path must be within the project root.': + 'Путь сводки должен находиться в корне проекта.', + 'Summary path resolves to an existing directory: {{path}}': + 'Путь сводки указывает на существующий каталог: {{path}}', + 'Summary path ends with a separator but is an existing file: {{path}}': + 'Путь сводки заканчивается разделителем, но является существующим файлом: {{path}}', 'Failed to generate project context summary: {{error}}': 'Не удалось сгенерировать сводку контекста проекта: {{error}}', 'Saved project summary to {{filePathForDisplay}}.': diff --git a/packages/cli/src/i18n/locales/zh-TW.js b/packages/cli/src/i18n/locales/zh-TW.js index bbb2b00bf75..be8a5d24461 100644 --- a/packages/cli/src/i18n/locales/zh-TW.js +++ b/packages/cli/src/i18n/locales/zh-TW.js @@ -1300,6 +1300,13 @@ export default { 'Already generating summary, wait for previous request to complete': '正在生成摘要，請等待上一個請求完成', 'No conversation found to summarize.': '未找到要總結的對話', + 'Summary path already exists and is not a generated summary: {{path}}': + '摘要路徑已存在且非產生的摘要：{{path}}', + 'Summary path must be within the project root.': '摘要路徑必須在專案根目錄內', + 'Summary path resolves to an existing directory: {{path}}': + '摘要路徑解析為一個已存在的目錄：{{path}}', + 'Summary path ends with a separator but is an existing file: {{path}}': + '摘要路徑以分隔符結尾，但是一個已存在的檔案：{{path}}', 'Failed to generate project context summary: {{error}}': '生成項目上下文摘要失敗：{{error}}', 'Saved project summary to {{filePathForDisplay}}.': diff --git a/packages/cli/src/i18n/locales/zh.js b/packages/cli/src/i18n/locales/zh.js index a0d8668faed..fc3dcab0e16 100644 --- a/packages/cli/src/i18n/locales/zh.js +++ b/packages/cli/src/i18n/locales/zh.js @@ -1410,6 +1410,13 @@ export default { 'Already generating summary, wait for previous request to complete': '正在生成摘要，请等待上一个请求完成', 'No conversation found to summarize.': '未找到要总结的对话', + 'Summary path already exists and is not a generated summary: {{path}}': + '摘要路径已存在且不是生成的摘要：{{path}}', + 'Summary path must be within the project root.': '摘要路径必须在项目根目录内', + 'Summary path resolves to an existing directory: {{path}}': + '摘要路径解析为一个已存在的目录：{{path}}', + 'Summary path ends with a separator but is an existing file: {{path}}': + '摘要路径以分隔符结尾，但是一个已存在的文件：{{path}}', 'Failed to generate project context summary: {{error}}': '生成项目上下文摘要失败：{{error}}', 'Saved project summary to {{filePathForDisplay}}.': diff --git a/packages/cli/src/ui/commands/summaryCommand.test.ts b/packages/cli/src/ui/commands/summaryCommand.test.ts new file mode 100644 index 00000000000..beebc4f4e01 --- /dev/null +++ b/packages/cli/src/ui/commands/summaryCommand.test.ts @@ -0,0 +1,602 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import path from 'node:path'; +import { summaryCommand } from './summaryCommand.js'; +import { createMockCommandContext } from '../../test-utils/mockCommandContext.js'; +import type { CommandContext } from './types.js'; +import { runSideQuery } from '@qwen-code/qwen-code-core'; + +vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + getProjectSummaryPrompt: () => 'summary prompt', + runSideQuery: vi.fn(async () => ({ text: 'SUMMARY BODY' })), + }; +}); + +const makeContext = (projectRoot: string): CommandContext => { + const chat = { + getHistoryShallow: () => [ + { role: 'user', parts: [{ text: 'a' }] }, + { role: 'model', parts: [{ text: 'b' }] }, + { role: 'user', parts: [{ text: 'c' }] }, + ], + getGenerationConfig: () => ({ systemInstruction: 'sys' }), + }; + const config = { + getProjectRoot: () => projectRoot, + getGeminiClient: () => ({ getChat: () => chat }), + getModel: () => 'test-model', + }; + return createMockCommandContext({ + executionMode: 'non_interactive', + services: { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + config: config as any, + }, + }); +}; + +describe('summaryCommand custom export path', () => { + let projectRoot: string; + + beforeEach(async () => { + vi.mocked(runSideQuery).mockClear(); + projectRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'summary-cmd-')); + }); + + afterEach(async () => { + await fs.rm(projectRoot, { recursive: true, force: true }); + }); + + interface MessageResult { + type: string; + messageType: string; + content: string; + } + + const run = async (args: string): Promise => + (await summaryCommand.action?.( + makeContext(projectRoot), + args, + )) as MessageResult; + + const fileExists = async (p: string): Promise => { + try { + return (await fs.stat(p)).isFile(); + } catch { + return false; + } + }; + + const dirExists = async (p: string): Promise => { + try { + return (await fs.stat(p)).isDirectory(); + } catch { + return false; + } + }; + + it('defaults to .qwen/PROJECT_SUMMARY.md with no argument', async () => { + const result = await run(''); + const fullPath = path.join(projectRoot, '.qwen', 'PROJECT_SUMMARY.md'); + expect(await fileExists(fullPath)).toBe(true); + const written = await fs.readFile(fullPath, 'utf8'); + expect(written).toContain('SUMMARY BODY'); + expect(written).toContain('## Summary Metadata'); + expect(result).toMatchObject({ type: 'message', messageType: 'info' }); + expect(result.content).toContain('.qwen/PROJECT_SUMMARY.md'); + if (process.platform !== 'win32') { + const stat = await fs.stat(path.dirname(fullPath)); + expect(stat.mode & 0o777).toBe(0o700); + } + }); + + it('overwrites a hand-written file at the default path', async () => { + const qwenDir = path.join(projectRoot, '.qwen'); + await fs.mkdir(qwenDir, { recursive: true }); + await fs.writeFile( + path.join(qwenDir, 'PROJECT_SUMMARY.md'), + 'hand-written notes', + 'utf8', + ); + const result = await run(''); + expect(result).toMatchObject({ type: 'message', messageType: 'info' }); + const written = await fs.readFile( + path.join(qwenDir, 'PROJECT_SUMMARY.md'), + 'utf8', + ); + expect(written).toContain('SUMMARY BODY'); + expect(written).not.toContain('hand-written notes'); + }); + + it('writes a relative file path as-is', async () => { + const result = await run('notes.md'); + expect(await fileExists(path.join(projectRoot, 'notes.md'))).toBe(true); + expect(result.content).toContain('notes.md'); + expect(result.content).not.toContain(projectRoot); + }); + + it('treats a relative path with a trailing separator as a directory', async () => { + // Regression: path.resolve strips the trailing separator, so the directory + // must be detected from the raw argument, not the resolved path. + const result = await run('docs/'); + expect( + await fileExists(path.join(projectRoot, 'docs', 'PROJECT_SUMMARY.md')), + ).toBe(true); + expect(await fileExists(path.join(projectRoot, 'docs'))).toBe(false); + expect(result.content).toContain('docs/PROJECT_SUMMARY.md'); + expect(result.content).not.toContain(projectRoot); + }); + + it('appends the default filename for an existing directory', async () => { + await fs.mkdir(path.join(projectRoot, 'existingdir')); + const result = await run('existingdir'); + expect( + await fileExists( + path.join(projectRoot, 'existingdir', 'PROJECT_SUMMARY.md'), + ), + ).toBe(true); + expect(result.content).toContain('existingdir/PROJECT_SUMMARY.md'); + expect(result.content).not.toContain(projectRoot); + }); + + it('writes an absolute path as-is and reports it absolutely', async () => { + const target = path.join(projectRoot, 'abs', 'out.md'); + const result = await run(target); + expect(await fileExists(target)).toBe(true); + expect(result.content).toContain(target.replaceAll(path.sep, '/')); + }); + + it('rejects a relative path that escapes the project root', async () => { + const result = await run('../outside/leak.md'); + expect(result).toMatchObject({ type: 'message', messageType: 'error' }); + expect(result.content).toContain('within the project root'); + expect(runSideQuery).not.toHaveBeenCalled(); + }); + + it('rejects an absolute path outside the project root', async () => { + const result = await run('/tmp/summary-escape/leak.md'); + expect(result).toMatchObject({ type: 'message', messageType: 'error' }); + expect(result.content).toContain('within the project root'); + expect(runSideQuery).not.toHaveBeenCalled(); + }); + + it.skipIf(process.platform === 'win32')( + 'allows a symlink that resolves inside the project root', + async () => { + await fs.mkdir(path.join(projectRoot, 'real-dir')); + await fs.symlink( + path.join(projectRoot, 'real-dir'), + path.join(projectRoot, 'internal-link'), + ); + const result = await run('internal-link/summary.md'); + expect(result).toMatchObject({ type: 'message', messageType: 'info' }); + expect( + await fileExists(path.join(projectRoot, 'real-dir', 'summary.md')), + ).toBe(true); + }, + ); + + it.skipIf(process.platform === 'win32')( + 'rejects a path that escapes the project root via a symlink', + async () => { + const outside = await fs.mkdtemp( + path.join(os.tmpdir(), 'summary-outside-'), + ); + try { + await fs.symlink(outside, path.join(projectRoot, 'link')); + const result = await run('link/leak.md'); + expect(result).toMatchObject({ + type: 'message', + messageType: 'error', + }); + expect(result.content).toContain('within the project root'); + expect(await fileExists(path.join(outside, 'leak.md'))).toBe(false); + expect(runSideQuery).not.toHaveBeenCalled(); + } finally { + await fs.rm(outside, { recursive: true, force: true }); + } + }, + ); + + it.skipIf(process.platform === 'win32')( + 'rejects a broken symlink whose target is outside the project root', + async () => { + const outsideTarget = path.join( + os.tmpdir(), + `summary-broken-${Date.now()}`, + 'leak.md', + ); + await fs.symlink(outsideTarget, path.join(projectRoot, 'broken-link')); + const result = await run('broken-link'); + expect(result).toMatchObject({ type: 'message', messageType: 'error' }); + expect(result.content).toContain('within the project root'); + expect(await fileExists(outsideTarget)).toBe(false); + expect(runSideQuery).not.toHaveBeenCalled(); + }, + ); + + it.skipIf(process.platform === 'win32')( + 'rejects a multi-link symlink chain that escapes the project root', + async () => { + const outsideTarget = path.join( + os.tmpdir(), + `summary-chain-${Date.now()}`, + 'evil.md', + ); + // link1 -> link2 (relative, inside root), link2 -> outside (absolute, + // broken). The old single-readlink check saw only the inside-root link2 + // and passed containment; the full-chain walk reaches the outside target. + await fs.symlink('link2', path.join(projectRoot, 'link1')); + await fs.symlink(outsideTarget, path.join(projectRoot, 'link2')); + const result = await run('link1'); + expect(result).toMatchObject({ type: 'message', messageType: 'error' }); + expect(result.content).toContain('within the project root'); + expect(await fileExists(outsideTarget)).toBe(false); + expect(runSideQuery).not.toHaveBeenCalled(); + }, + ); + + it.skipIf(process.platform === 'win32')( + 'rejects a symlink with a relative target escaping the project root', + async () => { + const outsideDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'summary-outside-'), + ); + try { + // Git stores symlink targets verbatim, so a committed + // `ln -s ../outside/leak.md` recreates a relative target on checkout. + await fs.symlink( + path.join('..', path.basename(outsideDir), 'leak.md'), + path.join(projectRoot, 'rel-link'), + ); + const result = await run('rel-link'); + expect(result).toMatchObject({ + type: 'message', + messageType: 'error', + }); + expect(result.content).toContain('within the project root'); + expect(runSideQuery).not.toHaveBeenCalled(); + } finally { + await fs.rm(outsideDir, { recursive: true, force: true }); + } + }, + ); + + it.skipIf(process.platform === 'win32')( + 'rejects a directory whose appended default filename is a symlink escaping the project root', + async () => { + const outside = await fs.mkdtemp( + path.join(os.tmpdir(), 'summary-outside-'), + ); + try { + const docsDir = path.join(projectRoot, 'docs'); + await fs.mkdir(docsDir); + await fs.symlink( + path.join(outside, 'evil-target.md'), + path.join(docsDir, 'PROJECT_SUMMARY.md'), + ); + const result = await run('docs'); + expect(result).toMatchObject({ + type: 'message', + messageType: 'error', + }); + expect(result.content).toContain('within the project root'); + expect(await fileExists(path.join(outside, 'evil-target.md'))).toBe( + false, + ); + expect(runSideQuery).not.toHaveBeenCalled(); + } finally { + await fs.rm(outside, { recursive: true, force: true }); + } + }, + ); + + it.skipIf(process.platform === 'win32')( + 'rejects a symlink cycle', + async () => { + await fs.symlink('link-b', path.join(projectRoot, 'link-a')); + await fs.symlink('link-a', path.join(projectRoot, 'link-b')); + const result = await run('link-a'); + expect(result).toMatchObject({ type: 'message', messageType: 'error' }); + expect(result.content).toContain('within the project root'); + expect(runSideQuery).not.toHaveBeenCalled(); + }, + ); + + it('does not create the target directory when generation fails', async () => { + vi.mocked(runSideQuery).mockRejectedValueOnce(new Error('rate limit')); + const result = await run('reports/2026/summary.md'); + expect(result).toMatchObject({ type: 'message', messageType: 'error' }); + expect(await dirExists(path.join(projectRoot, 'reports'))).toBe(false); + }); + + it('expands a leading ~ and rejects it when outside the project root', async () => { + const result = await run('~/summary-tilde-leak.md'); + expect(result).toMatchObject({ type: 'message', messageType: 'error' }); + expect(result.content).toContain('within the project root'); + expect(runSideQuery).not.toHaveBeenCalled(); + // The unexpanded argument must not create a literal "~" directory. + expect(await dirExists(path.join(projectRoot, '~'))).toBe(false); + }); + + it('refuses to overwrite an existing file that is not a generated summary', async () => { + const target = path.join(projectRoot, 'IMPORTANT.md'); + await fs.writeFile(target, 'precious content', 'utf8'); + const result = await run('IMPORTANT.md'); + expect(result).toMatchObject({ type: 'message', messageType: 'error' }); + expect(result.content).toContain('already exists'); + expect(await fs.readFile(target, 'utf8')).toBe('precious content'); + expect(runSideQuery).not.toHaveBeenCalled(); + }); + + it('refuses to overwrite a file that merely mentions Summary Metadata in prose', async () => { + const target = path.join(projectRoot, 'DESIGN.md'); + await fs.writeFile( + target, + 'The summary file ends with a `## Summary Metadata` footer.\n', + 'utf8', + ); + const result = await run('DESIGN.md'); + expect(result).toMatchObject({ type: 'message', messageType: 'error' }); + expect(result.content).toContain('already exists'); + expect(await fs.readFile(target, 'utf8')).toContain( + '`## Summary Metadata`', + ); + expect(runSideQuery).not.toHaveBeenCalled(); + }); + + it('refuses to overwrite a file with a Summary Metadata heading but no Update time', async () => { + const target = path.join(projectRoot, 'DESIGN.md'); + await fs.writeFile( + target, + 'Some content\n\n---\n\n## Summary Metadata\n\nThis is a design doc.\n', + 'utf8', + ); + const result = await run('DESIGN.md'); + expect(result).toMatchObject({ type: 'message', messageType: 'error' }); + expect(result.content).toContain('already exists'); + expect(runSideQuery).not.toHaveBeenCalled(); + }); + + it('overwrites a previously generated summary', async () => { + const target = path.join(projectRoot, 'summary.md'); + await fs.writeFile( + target, + 'old body\n\n---\n\n## Summary Metadata\n**Update time**: old\n', + 'utf8', + ); + const result = await run('summary.md'); + expect(result).toMatchObject({ type: 'message', messageType: 'info' }); + const written = await fs.readFile(target, 'utf8'); + expect(written).toContain('SUMMARY BODY'); + expect(written).not.toContain('old body'); + }); + + it.skipIf(process.platform === 'win32')( + 'creates a custom-path summary with mode 0o600', + async () => { + await run('private-notes.md'); + const stat = await fs.stat(path.join(projectRoot, 'private-notes.md')); + expect(stat.mode & 0o777).toBe(0o600); + }, + ); + + it('overwrites a previously generated summary with CRLF line endings', async () => { + const target = path.join(projectRoot, 'crlf-summary.md'); + await fs.writeFile( + target, + 'old body\r\n\r\n---\r\n\r\n## Summary Metadata\r\n**Update time**: old\r\n', + 'utf8', + ); + const result = await run('crlf-summary.md'); + expect(result).toMatchObject({ type: 'message', messageType: 'info' }); + const written = await fs.readFile(target, 'utf8'); + expect(written).toContain('SUMMARY BODY'); + expect(written).not.toContain('old body'); + }); + + it('overwrites an empty pre-created file', async () => { + const target = path.join(projectRoot, 'empty.md'); + await fs.writeFile(target, '', 'utf8'); + const result = await run('empty.md'); + expect(result).toMatchObject({ type: 'message', messageType: 'info' }); + const written = await fs.readFile(target, 'utf8'); + expect(written).toContain('SUMMARY BODY'); + }); + + it('rejects a trailing separator on an existing file', async () => { + await fs.writeFile(path.join(projectRoot, 'notes.md'), 'content', 'utf8'); + const result = await run('notes.md/'); + expect(result).toMatchObject({ type: 'message', messageType: 'error' }); + expect(result.content).toContain('ends with a separator'); + expect(runSideQuery).not.toHaveBeenCalled(); + }); + + it.skipIf(process.platform === 'win32')( + 'allows the default target when .qwen is a symlink', + async () => { + const outside = await fs.mkdtemp( + path.join(os.tmpdir(), 'summary-shared-'), + ); + try { + await fs.symlink(outside, path.join(projectRoot, '.qwen')); + const result = await run(''); + expect(result).toMatchObject({ + type: 'message', + messageType: 'info', + }); + expect(await fileExists(path.join(outside, 'PROJECT_SUMMARY.md'))).toBe( + true, + ); + } finally { + await fs.rm(outside, { recursive: true, force: true }); + } + }, + ); + + it('rejects a non-summary file created during generation', async () => { + vi.mocked(runSideQuery).mockImplementationOnce(async () => { + await fs.writeFile( + path.join(projectRoot, 'race.md'), + 'precious content', + 'utf8', + ); + return { text: 'SUMMARY BODY' }; + }); + const result = await run('race.md'); + expect(result).toMatchObject({ type: 'message', messageType: 'error' }); + expect(result.content).toContain('already exists'); + expect(await fs.readFile(path.join(projectRoot, 'race.md'), 'utf8')).toBe( + 'precious content', + ); + }); + + it.skipIf(process.platform === 'win32')( + 'rejects a symlink planted at the target during generation', + async () => { + const outside = await fs.mkdtemp( + path.join(os.tmpdir(), 'summary-toctou-'), + ); + try { + vi.mocked(runSideQuery).mockImplementationOnce(async () => { + await fs.symlink(outside, path.join(projectRoot, 'race-link.md')); + return { text: 'SUMMARY BODY' }; + }); + const result = await run('race-link.md'); + expect(result).toMatchObject({ + type: 'message', + messageType: 'error', + }); + expect(result.content).toContain('within the project root'); + } finally { + await fs.rm(outside, { recursive: true, force: true }); + } + }, + ); + + it.skipIf(process.platform === 'win32')( + 'applies 0o700 to .qwen/ when spelled explicitly', + async () => { + const result = await run('.qwen/'); + expect(result).toMatchObject({ type: 'message', messageType: 'info' }); + const stat = await fs.stat(path.join(projectRoot, '.qwen')); + expect(stat.mode & 0o777).toBe(0o700); + }, + ); + + it.skipIf(process.platform === 'win32')( + 'preserves existing file permissions on regeneration', + async () => { + const target = path.join(projectRoot, 'summary.md'); + await fs.writeFile( + target, + 'old body\n\n---\n\n## Summary Metadata\n**Update time**: old\n', + 'utf8', + ); + await fs.chmod(target, 0o644); + await run('summary.md'); + const stat = await fs.stat(target); + expect(stat.mode & 0o777).toBe(0o644); + }, + ); + + it('overwrites a hand-written file when the default path is spelled explicitly', async () => { + const qwenDir = path.join(projectRoot, '.qwen'); + await fs.mkdir(qwenDir, { recursive: true }); + await fs.writeFile( + path.join(qwenDir, 'PROJECT_SUMMARY.md'), + 'hand-written notes', + 'utf8', + ); + const result = await run('.qwen/PROJECT_SUMMARY.md'); + expect(result).toMatchObject({ type: 'message', messageType: 'info' }); + const written = await fs.readFile( + path.join(qwenDir, 'PROJECT_SUMMARY.md'), + 'utf8', + ); + expect(written).toContain('SUMMARY BODY'); + expect(written).not.toContain('hand-written notes'); + }); + + it('rejects an existing directory at the leaf before generating', async () => { + await fs.mkdir(path.join(projectRoot, 'docs', 'PROJECT_SUMMARY.md'), { + recursive: true, + }); + const result = await run('docs'); + expect(result).toMatchObject({ type: 'message', messageType: 'error' }); + expect(result.content).toContain('existing directory'); + expect(runSideQuery).not.toHaveBeenCalled(); + }); + + it('refuses to overwrite a file that embeds a full footer mid-document', async () => { + const target = path.join(projectRoot, 'ARCHIVE.md'); + await fs.writeFile( + target, + 'intro\n\n---\n\n## Summary Metadata\n**Update time**: old\n\ntrailing prose\n', + 'utf8', + ); + const result = await run('ARCHIVE.md'); + expect(result).toMatchObject({ type: 'message', messageType: 'error' }); + expect(result.content).toContain('already exists'); + expect(runSideQuery).not.toHaveBeenCalled(); + }); + + it('detects the footer in a file larger than the tail window', async () => { + const target = path.join(projectRoot, 'big-summary.md'); + const padding = 'x'.repeat(8192); + await fs.writeFile( + target, + `${padding}\n\n---\n\n## Summary Metadata\n**Update time**: old\n`, + 'utf8', + ); + const result = await run('big-summary.md'); + expect(result).toMatchObject({ type: 'message', messageType: 'info' }); + const written = await fs.readFile(target, 'utf8'); + expect(written).toContain('SUMMARY BODY'); + expect(written).not.toContain(padding); + }); + + it('returns empty content in interactive mode errors to avoid double rendering', async () => { + const chat = { + getHistoryShallow: () => [ + { role: 'user', parts: [{ text: 'a' }] }, + { role: 'model', parts: [{ text: 'b' }] }, + { role: 'user', parts: [{ text: 'c' }] }, + ], + getGenerationConfig: () => ({ systemInstruction: 'sys' }), + }; + const config = { + getProjectRoot: () => projectRoot, + getGeminiClient: () => ({ getChat: () => chat }), + getModel: () => 'test-model', + }; + const context = createMockCommandContext({ + executionMode: 'interactive', + services: { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + config: config as any, + }, + }); + const result = (await summaryCommand.action?.( + context, + '../outside/leak.md', + )) as MessageResult; + expect(result).toMatchObject({ type: 'message', messageType: 'error' }); + expect(result.content).toBe(''); + expect(vi.mocked(context.ui.addItem)).toHaveBeenCalledWith( + expect.objectContaining({ type: 'error' }), + expect.any(Number), + ); + }); +}); diff --git a/packages/cli/src/ui/commands/summaryCommand.ts b/packages/cli/src/ui/commands/summaryCommand.ts index 5a1b5e67403..603d0c69567 100644 --- a/packages/cli/src/ui/commands/summaryCommand.ts +++ b/packages/cli/src/ui/commands/summaryCommand.ts @@ -13,11 +13,97 @@ import { } from './types.js'; import { getProjectSummaryPrompt, + isSubpath, + resolvePath, runSideQuery, } from '@qwen-code/qwen-code-core'; import type { HistoryItemSummary } from '../types.js'; import { t } from '../../i18n/index.js'; +// Resolves the real path of the nearest existing ancestor of targetPath. The +// target file itself usually does not exist yet, but a symlinked parent +// directory can still point outside the project root, so the ancestor must be +// resolved to detect that. Unlike the realpathNearestExisting copies in +// exportCommand.ts/statsCommand.ts, this one does not guard the upward walk +// itself (they loop while isSubpath(cwd, currentPath) and throw on escape); +// every call site here must check containment on the returned path. +const realpathNearestExisting = async (targetPath: string): Promise => { + let currentPath = targetPath; + for (;;) { + try { + return await fsPromises.realpath(currentPath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + throw error; + } + const parentPath = path.dirname(currentPath); + if (parentPath === currentPath) { + return currentPath; + } + currentPath = parentPath; + } + } +}; + +// Follows the full symlink chain at filePath (with cycle detection) and +// verifies the final target is contained within realProjectRoot. A broken +// chain whose terminal target is absent still resolves via +// realpathNearestExisting, catching multi-hop escapes. +const assertLeafNotSymlinkEscape = async ( + filePath: string, + realProjectRoot: string, +): Promise => { + let current = filePath; + const seen = new Set(); + for (;;) { + const linkStat = await fsPromises.lstat(current).catch(() => null); + if (!linkStat?.isSymbolicLink()) break; + if (seen.has(current)) { + throw new Error(t('Summary path must be within the project root.')); + } + seen.add(current); + const linkTarget = await fsPromises.readlink(current); + current = path.isAbsolute(linkTarget) + ? linkTarget + : path.resolve(path.dirname(current), linkTarget); + } + const realTarget = await realpathNearestExisting(current); + if (!isSubpath(realProjectRoot, realTarget)) { + throw new Error(t('Summary path must be within the project root.')); + } +}; + +// Static footer prefix shared by the writer (saveSummaryToDisk) and the +// detector below, kept in one place so the two cannot drift apart. +const SUMMARY_FOOTER_PREFIX = '\n---\n\n## Summary Metadata\n**Update time**: '; + +const buildSummaryFooter = (timestamp: string): string => + `${SUMMARY_FOOTER_PREFIX}${timestamp}\n`; + +// Reads only the file tail: the footer always lives in the last ~90 bytes, so +// a mistyped multi-GB path is not loaded fully into memory just to test for it. +const readSummaryTail = async (p: string, bytes = 4096): Promise => { + const handle = await fsPromises.open(p, 'r'); + try { + const { size } = await handle.stat(); + const length = Math.min(size, bytes); + const buffer = Buffer.alloc(length); + await handle.read(buffer, 0, length, Math.max(0, size - length)); + return buffer.toString('utf8'); + } finally { + await handle.close(); + } +}; + +// Built from the same prefix the writer emits (escaped so the literal `**` is +// not read as a quantifier) and anchored to end-of-file, so a document that +// merely embeds a sample footer is not mistaken for a generated summary and +// clobbered. +const isGeneratedSummary = (content: string): boolean => + new RegExp( + `${SUMMARY_FOOTER_PREFIX.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}[^\\n]*\\n?$`, + ).test(content.replace(/\r\n/g, '\n')); + export const summaryCommand: SlashCommand = { name: 'summary', get description() { @@ -25,9 +111,10 @@ export const summaryCommand: SlashCommand = { 'Generate a project summary and save it to .qwen/PROJECT_SUMMARY.md', ); }, + argumentHint: '[path]', kind: CommandKind.BUILT_IN, supportedModes: ['interactive', 'non_interactive', 'acp'] as const, - action: async (context): Promise => { + action: async (context, args): Promise => { const { config } = context.services; const { ui } = context; const executionMode = context.executionMode ?? 'interactive'; @@ -134,36 +221,190 @@ export const summaryCommand: SlashCommand = { return result.text; }; + const resolveSummaryTarget = async (): Promise<{ + summaryPath: string; + filePathForDisplay: string; + isDefaultTarget: boolean; + realProjectRoot: string; + }> => { + const projectRoot = config.getProjectRoot(); + const defaultSummaryPath = path.join( + projectRoot, + '.qwen', + 'PROJECT_SUMMARY.md', + ); + const customPath = args?.trim(); + + if (!customPath) { + // The default target always overwrites: regenerating the summary is + // the command's purpose, and .qwen/PROJECT_SUMMARY.md is a generated + // artifact — not user prose the overwrite guard protects. + return { + summaryPath: defaultSummaryPath, + filePathForDisplay: '.qwen/PROJECT_SUMMARY.md', + isDefaultTarget: true, + realProjectRoot: await fsPromises.realpath(projectRoot), + }; + } + + // resolvePath expands a leading ~ so the path is honest before the + // containment check rejects it, instead of creating a literal "~" dir. + const resolved = resolvePath(projectRoot, customPath); + + if (!isSubpath(projectRoot, resolved)) { + throw new Error(t('Summary path must be within the project root.')); + } + + // A lexical check cannot see through symlinks: a link inside the project + // root may point outside it. Re-check containment on the real paths. + const realProjectRoot = await fsPromises.realpath(projectRoot); + + await assertLeafNotSymlinkEscape(resolved, realProjectRoot); + + const realResolved = await realpathNearestExisting(resolved); + if (!isSubpath(realProjectRoot, realResolved)) { + throw new Error(t('Summary path must be within the project root.')); + } + + const hasTrailingSep = + customPath.endsWith('/') || customPath.endsWith(path.sep); + const resolvedStat = await fsPromises.stat(resolved).catch(() => null); + if (hasTrailingSep && resolvedStat?.isFile()) { + throw new Error( + t( + 'Summary path ends with a separator but is an existing file: {{path}}', + { path: customPath }, + ), + ); + } + const isDir = hasTrailingSep || (resolvedStat?.isDirectory() ?? false); + + const summaryPath = isDir + ? path.join(resolved, 'PROJECT_SUMMARY.md') + : resolved; + + // The appended filename may itself be a symlink escaping the project + // root (e.g. a malicious repo tracks docs/PROJECT_SUMMARY.md -> /etc/ + // passwd). Re-check the leaf after the join. + if (isDir) { + await assertLeafNotSymlinkEscape(summaryPath, realProjectRoot); + } + + const filePathForDisplay = ( + path.isAbsolute(customPath) + ? summaryPath + : path.relative(projectRoot, summaryPath) + ).replaceAll(path.sep, '/'); + + // Compute the default-target flag before the guards so an explicitly + // spelled default path (`.qwen/PROJECT_SUMMARY.md` or `.qwen/`) is + // treated as the default target for the overwrite guard and always + // overwrites. Unlike the no-arg command, a spelled path still runs the + // custom-path symlink containment checks. + const isDefaultTarget = summaryPath === defaultSummaryPath; + + const existingStat = await fsPromises.stat(summaryPath).catch(() => null); + + // Reject an existing directory at the leaf (e.g. `/summary docs` where + // docs/PROJECT_SUMMARY.md is itself a directory) before the LLM call, + // instead of surfacing a raw EISDIR only at write time. + if (existingStat?.isDirectory()) { + throw new Error( + t('Summary path resolves to an existing directory: {{path}}', { + path: filePathForDisplay, + }), + ); + } + + // For any non-default path, refuse to clobber a pre-existing file that is + // not a generated summary (e.g. a mistyped `package.json`). A generated + // summary carries a `## Summary Metadata` footer, so regenerating one is + // allowed. + if (!isDefaultTarget && existingStat?.isFile() && existingStat.size > 0) { + const existing = await readSummaryTail(summaryPath).catch(() => ''); + if (!isGeneratedSummary(existing)) { + throw new Error( + t( + 'Summary path already exists and is not a generated summary: {{path}}', + { path: filePathForDisplay }, + ), + ); + } + } + + return { + summaryPath, + filePathForDisplay, + isDefaultTarget, + realProjectRoot, + }; + }; + const saveSummaryToDisk = async ( markdownSummary: string, + target: { + summaryPath: string; + filePathForDisplay: string; + isDefaultTarget: boolean; + realProjectRoot: string; + }, ): Promise<{ filePathForDisplay: string; fullPath: string; }> => { - // Ensure .qwen directory exists - const projectRoot = config.getProjectRoot(); - const qwenDir = path.join(projectRoot, '.qwen'); - try { - await fsPromises.mkdir(qwenDir, { recursive: true }); - } catch (_err) { - // Directory might already exist, ignore error + const summaryContent = `${markdownSummary}\n${buildSummaryFooter( + new Date().toISOString(), + )}`; + + // Re-check the leaf right before writing to narrow the TOCTOU window + // between resolveSummaryTarget (pre-LLM) and the write (post-LLM). + // The default target is the user's own .qwen/ directory — a symlinked + // .qwen/ is a deliberate setup, not an attack vector. + if (!target.isDefaultTarget) { + await assertLeafNotSymlinkEscape( + target.summaryPath, + target.realProjectRoot, + ); } - // Save the summary to PROJECT_SUMMARY.md - const summaryPath = path.join(qwenDir, 'PROJECT_SUMMARY.md'); - const summaryContent = `${markdownSummary} - ---- - -## Summary Metadata -**Update time**: ${new Date().toISOString()} -`; + const preWriteStat = await fsPromises + .stat(target.summaryPath) + .catch(() => null); + + // Re-run the overwrite guard: a file created during generation (the + // slow step) would otherwise be silently destroyed. + if ( + !target.isDefaultTarget && + preWriteStat?.isFile() && + preWriteStat.size > 0 + ) { + const existing = await readSummaryTail(target.summaryPath).catch( + () => '', + ); + if (!isGeneratedSummary(existing)) { + throw new Error( + t( + 'Summary path already exists and is not a generated summary: {{path}}', + { path: target.filePathForDisplay }, + ), + ); + } + } - await fsPromises.writeFile(summaryPath, summaryContent, 'utf8'); + await fsPromises.mkdir(path.dirname(target.summaryPath), { + recursive: true, + ...(target.isDefaultTarget ? { mode: 0o700 } : {}), + }); + // writeFile's mode applies at creation; for an existing file the user may + // have relaxed, the mode argument is ignored and permissions are kept. + await fsPromises.writeFile(target.summaryPath, summaryContent, { + encoding: 'utf8', + mode: 0o600, + }); return { - filePathForDisplay: '.qwen/PROJECT_SUMMARY.md', - fullPath: summaryPath, + filePathForDisplay: target.filePathForDisplay, + fullPath: target.summaryPath, }; }; @@ -252,13 +493,17 @@ export const summaryCommand: SlashCommand = { markdownSummary: string; filePathForDisplay: string; }> => { + const target = await resolveSummaryTarget(); emitInteractivePending('generating'); const markdownSummary = await generateSummaryMarkdown(history); if (abortSignal?.aborted) { throw new DOMException('Summary generation cancelled.', 'AbortError'); } emitInteractivePending('saving'); - const { filePathForDisplay } = await saveSummaryToDisk(markdownSummary); + const { filePathForDisplay } = await saveSummaryToDisk( + markdownSummary, + target, + ); completeInteractive(filePathForDisplay); return { markdownSummary, filePathForDisplay }; }; @@ -324,7 +569,8 @@ export const summaryCommand: SlashCommand = { return { type: 'message', messageType: 'error', - content: formatErrorMessage(error), + content: + executionMode === 'interactive' ? '' : formatErrorMessage(error), }; } },