From 01ec934d28a640822a6685ed5d6e1d98be9e9049 Mon Sep 17 00:00:00 2001 From: fancivez Date: Sat, 2 May 2026 12:50:35 +0800 Subject: [PATCH] fix: address 13 review findings (security, UX, i18n) P1: - cli: pin codex --sandbox read-only (defeat prompt injection) - settings: clear apiKey/apiHeaders on provider preset switch - main/view: render error state when LLM returns empty cards - schema/cli: do not log raw model output; UI errors expose length only P2: - cli: pass --model to Claude Code when settings.model set - settings-tab: add CLI timeout input + normalizeCliTimeoutMs (min 1s) - view: export source link uses [[path|basename]] - view: export failures surface a localized Notice P3: - schema: cardUntitled fallback now respects uiLanguage - generation-job-manager: error codes localized via main.ts - batch: settled guard on promptForBatchFolder modal - README: drop stale e2e validator install note Codex review pass: - provider-parsers: forward settings to normalizeCardsPayload - generation-job-manager: classifyGenerationError regex covers new EN/zh schema-error messages - cli: do NOT pass --model to codex (DEFAULT_SETTINGS.model is a Claude name and would break codex; codex relies on its own config) Tests: cli (codex sandbox + claude --model + classify zh/en), settings (applyApiProviderPreset isolation + normalizeCliTimeoutMs), schema (language-aware fallback title), test-exports (new symbols). Change-Id: I9f4c21f9299e1a9bf166f69f712859854482fe92 --- README.md | 2 +- README.zh-CN.md | 2 +- main.ts | 3 +- src/batch.ts | 14 +++-- src/cli.ts | 25 +++++++-- src/generation-job-manager.ts | 9 ++- src/i18n-strings.ts | 20 +++++-- src/provider-parsers.ts | 2 +- src/schema.ts | 18 ++++-- src/settings-tab.ts | 14 +++++ src/settings.ts | 10 ++++ src/test-exports.ts | 2 + src/view.ts | 43 +++++++------- tests/cli.test.js | 103 ++++++++++++++++++++++++++++++++++ tests/schema.test.js | 9 ++- tests/settings.test.js | 28 +++++++++ tests/test-exports.test.js | 2 + 17 files changed, 260 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 8a7c862..0afd7a9 100644 --- a/README.md +++ b/README.md @@ -138,7 +138,7 @@ evidence: bash .e2e/gate.sh --json ``` -If `e2e_contract_validator` is not installed, set `E2E_CONTRACT_VALIDATOR_PYTHONPATH` to the `claude-code-addons/scripts` directory before running the gate. Runtime evidence such as `.e2e/artifact.json` and `.e2e/results/` is generated locally and ignored by git. +The gate is self-contained — `.e2e/gate.sh` runs the bundled Node-based contract checker directly, no external Python validator required. Runtime evidence such as `.e2e/artifact.json` and `.e2e/results/` is generated locally and ignored by git. Test classification lives in `tests/catalog.json`. `verify` owns unit, component, contract, and local integration checks. Mocked Obsidian/runtime tests diff --git a/README.zh-CN.md b/README.zh-CN.md index 429570d..4ea130e 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -137,7 +137,7 @@ npm run test:e2e # 打包插件 + 临时 Vault smoke bash .e2e/gate.sh --json ``` -如果本机没有安装 `e2e_contract_validator`，运行前把 `E2E_CONTRACT_VALIDATOR_PYTHONPATH` 指向 `claude-code-addons/scripts` 目录。`.e2e/artifact.json`、`.e2e/results/` 等运行证据只在本地生成，并已被 git 忽略。 +该 gate 是自包含的 —— `.e2e/gate.sh` 直接运行仓库内的 Node 版 contract checker，不需要额外的 Python 校验器。`.e2e/artifact.json`、`.e2e/results/` 等运行证据只在本地生成，并已被 git 忽略。 测试分类的来源是 `tests/catalog.json`。`verify` 负责 unit、component、contract 和本地 integration 检查。使用 mock Obsidian/runtime 的测试归为 component 或 diff --git a/main.ts b/main.ts index c41419c..904b668 100644 --- a/main.ts +++ b/main.ts @@ -485,6 +485,7 @@ class ParallelReaderPlugin extends Plugin { const sections = await summarizeDocument(content, this.settings, job, this.streamProgressFor(view, file)); job.throwIfCancelled(); if (sections.length === 0) { + if (view && this.viewIsShowingFile(view, file)) view.renderError(file, this.t('noCardsReturned')); new Notice(this.t('noCardsReturned')); return; } @@ -521,7 +522,7 @@ class ParallelReaderPlugin extends Plugin { private handleGenerationError(e: unknown, file: TFile, view: ParallelReaderView | null) { if (e instanceof GenerationJobAlreadyRunningError) { - new Notice(e.message); + new Notice(this.t('generationAlreadyRunning')); return; } if (e instanceof GenerationJobCancelledError) { diff --git a/src/batch.ts b/src/batch.ts index 689d23d..b153d32 100644 --- a/src/batch.ts +++ b/src/batch.ts @@ -112,6 +112,12 @@ export function promptForBatchFolder( cancelText: string, ): Promise { return new Promise((resolve) => { + let settled = false; + const settle = (value: string | null) => { + if (settled) return; + settled = true; + resolve(value); + }; const modal = new Modal(app); modal.onOpen = () => { modal.contentEl.createEl('p', { text: selectText }); @@ -120,21 +126,21 @@ export function promptForBatchFolder( input.placeholder = promptText; input.addEventListener('keydown', (e) => { if (e.key === 'Enter') { - resolve(input.value.trim()); + settle(input.value.trim()); modal.close(); } }); const actions = modal.contentEl.createDiv({ cls: 'modal-button-container' }); actions.createEl('button', { text: cancelText }).addEventListener('click', () => { - resolve(null); + settle(null); modal.close(); }); actions.createEl('button', { text: confirmText, cls: 'mod-cta' }).addEventListener('click', () => { - resolve(input.value.trim()); + settle(input.value.trim()); modal.close(); }); }; - modal.onClose = () => resolve(null); + modal.onClose = () => settle(null); modal.open(); }); } diff --git a/src/cli.ts b/src/cli.ts index 37af320..6cd4c09 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -5,6 +5,7 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; import { type GenerationJob, GenerationJobCancelledError } from './generation-job-manager'; +import { translate } from './i18n'; import { parseCardsJson } from './schema'; import type { PluginSettings, RawCard } from './types'; @@ -176,6 +177,8 @@ export async function summarizeViaClaudeCode( '--disallowed-tools', 'Bash,Read,Write,Edit,Glob,Grep,WebFetch,WebSearch,TodoWrite,Task', ]; + const claudeModel = (settings.model || '').trim(); + if (claudeModel) args.push('--model', claudeModel); const { stdout } = await runCli(cmd, args, user, settings.cliTimeoutMs, job, spawnImpl); // --output-format json produces a JSON array of event objects. @@ -193,12 +196,23 @@ export async function summarizeViaClaudeCode( resultText = events.result || events.content || ''; } } catch (_) { - throw new Error('claude CLI returned unexpected output:\n' + stdout.slice(0, 500)); + console.warn( + '[parallel-reader] claude CLI returned unexpected output. length=', + stdout.length, + 'head=', + stdout.slice(0, 80), + ); + throw new Error(translate(settings, 'errorClaudeCliBadJson', { length: String(stdout.length) })); } if (!resultText) { - console.warn('[parallel-reader] claude CLI returned no result. Full stdout:', stdout); - throw new Error('claude CLI returned no result. Output:\n' + stdout.slice(0, 500)); + console.warn( + '[parallel-reader] claude CLI returned no result. length=', + stdout.length, + 'head=', + stdout.slice(0, 80), + ); + throw new Error(translate(settings, 'errorClaudeCliNoResult', { length: String(stdout.length) })); } return parseCardsJson(resultText, settings); @@ -213,7 +227,10 @@ export async function summarizeViaCodex( ): Promise { const cmd = resolveCliPath('codex', settings.cliPath); const combined = `<>\n${system}\n<>\n${user}\n\nOutput JSON directly with no explanation.`; - const args = ['exec', '--skip-git-repo-check', '-']; + // NOTE: do NOT pass --model. settings.model defaults to a Claude model name + // (claude-sonnet-4-6), which would break Codex if passed verbatim. Codex + // uses its own config.toml profile for model selection. + const args = ['exec', '--skip-git-repo-check', '--sandbox', 'read-only', '-']; const { stdout } = await runCli(cmd, args, combined, settings.cliTimeoutMs, job, spawnImpl); return parseCardsJson(stdout, settings); } diff --git a/src/generation-job-manager.ts b/src/generation-job-manager.ts index 4f59187..2212ebb 100644 --- a/src/generation-job-manager.ts +++ b/src/generation-job-manager.ts @@ -7,7 +7,7 @@ export class GenerationJobAlreadyRunningError extends Error { key: string; constructor(key: string) { - super('该笔记正在生成对照笔记'); + super('already-running'); this.name = 'GenerationJobAlreadyRunningError'; this.code = 'already-running'; this.key = key; @@ -19,7 +19,7 @@ export class GenerationJobCancelledError extends Error { key: string; constructor(key: string) { - super('生成已取消'); + super('cancelled'); this.name = 'GenerationJobCancelledError'; this.code = 'cancelled'; this.key = key; @@ -125,7 +125,10 @@ export function classifyGenerationError(error: unknown): ErrorKind { if (/api key|unauthorized|401|403|认证|权限/i.test(message)) return 'auth'; if (/timeout|超时|timed out/i.test(message)) return 'timeout'; if (/429|rate limit|too many requests/i.test(message)) return 'rate-limit'; - if (/非 JSON|json_schema|schema|structured/i.test(message)) return 'schema'; + if ( + /非 JSON|非预期输出|没有返回结果|non-JSON|unexpected output|no result|json_schema|schema|structured/i.test(message) + ) + return 'schema'; if (/model 未设置|base url|配置|config/i.test(message)) return 'config'; return 'unknown'; } diff --git a/src/i18n-strings.ts b/src/i18n-strings.ts index 6e6becd..fb54a5d 100644 --- a/src/i18n-strings.ts +++ b/src/i18n-strings.ts @@ -31,6 +31,8 @@ export const STRINGS: Record> = { errorTitle: '生成失败', actionCopyError: '复制错误信息', emptyCard: '（未生成）', + cardUntitled: '(无标题)', + generationAlreadyRunning: '该笔记正在生成对照笔记', anchorMismatch: 'anchor 匹配失败，无法滚动联动', menuCopyMarkdown: '复制 Markdown', menuCopyPlain: '复制纯文本', @@ -46,6 +48,7 @@ export const STRINGS: Record> = { actionFailed: '{label}失败：{error}', exported: '已导出 → {path}', exportCancelled: '已取消导出', + exportFailed: '导出失败：{error}', confirmExportOverwrite: '导出文件已存在：{path}\n是否覆盖？', confirmExportCancel: '取消', confirmExportOverwriteButton: '覆盖', @@ -91,7 +94,9 @@ export const STRINGS: Record> = { errorModelMissing: 'Model 未设置。请在设置里填写模型 ID。', errorApiKeyMissing: 'API key 未设置。请在设置里填写 API Key{hint}。', errorApiKeyEnvHint: ' 或环境变量 {envVar}', - errorLlmNonJson: 'LLM 返回非 JSON：\n{excerpt}', + errorLlmNonJson: 'LLM 返回非 JSON（长度 {length} 字符），详见 console', + errorClaudeCliBadJson: 'Claude CLI 返回了非预期输出（长度 {length} 字符），详见 console', + errorClaudeCliNoResult: 'Claude CLI 没有返回结果（长度 {length} 字符），详见 console', errorCustomHeadersJsonParse: '自定义 headers JSON 解析失败：{error}', errorCustomHeadersJsonObject: '自定义 headers JSON 必须是对象', errorCustomHeadersLineFormat: '自定义 headers 每行格式应为 `Header-Name: value`', @@ -107,6 +112,7 @@ export const STRINGS: Record> = { settingCliPathDesc: '留空则自动探测常见位置；Obsidian GUI 启动时不继承 shell PATH，必要时填绝对路径', settingCliPathPlaceholder: '例：/Users/you/bin/codex', settingCliTimeoutName: 'CLI 超时 (ms)', + settingCliTimeoutDesc: 'CLI 调用的最大等待时间（毫秒），最小 1000，默认 120000', apiProviderHeader: 'API Provider', settingProviderPresetName: 'Provider preset', settingProviderPresetDesc: '参考 OpenClaw 的 provider/model 思路：preset 只负责协议、base URL 和认证默认值', @@ -126,7 +132,7 @@ export const STRINGS: Record> = { settingModelName: 'Model', settingModelDescApi: 'API 调用的模型 ID；支持 OpenClaw 风格 provider/model，若 provider 前缀匹配当前 preset 会自动剥离', - settingModelDescCli: 'Claude Code 下会传 --model；Codex 下通常忽略（用 Codex 默认配置）', + settingModelDescCli: 'Claude Code 下会以 --model 透传；Codex 始终使用自身 config 的默认 model', settingMaxInputName: '最大输入字符数', settingMaxInputDesc: '超过该长度会截断后再发送给模型；长上下文模型可适当调大', promptHeader: 'Prompt', @@ -199,6 +205,8 @@ export const STRINGS: Record> = { errorTitle: 'Generation failed', actionCopyError: 'Copy error', emptyCard: '(not generated)', + cardUntitled: '(Untitled)', + generationAlreadyRunning: 'This note is already being summarized.', anchorMismatch: 'Anchor did not match; scroll sync is unavailable', menuCopyMarkdown: 'Copy Markdown', menuCopyPlain: 'Copy plain text', @@ -214,6 +222,7 @@ export const STRINGS: Record> = { actionFailed: '{label} failed: {error}', exported: 'Exported → {path}', exportCancelled: 'Export cancelled', + exportFailed: 'Export failed: {error}', confirmExportOverwrite: 'Export file already exists: {path}\nOverwrite it?', confirmExportCancel: 'Cancel', confirmExportOverwriteButton: 'Overwrite', @@ -261,7 +270,9 @@ export const STRINGS: Record> = { errorModelMissing: 'Model is not set. Enter a model ID in settings.', errorApiKeyMissing: 'API key is not set. Enter an API Key in settings{hint}.', errorApiKeyEnvHint: ' or set environment variable {envVar}', - errorLlmNonJson: 'LLM returned non-JSON:\n{excerpt}', + errorLlmNonJson: 'LLM returned non-JSON ({length} chars). See console for excerpt.', + errorClaudeCliBadJson: 'Claude CLI returned unexpected output ({length} chars). See console for excerpt.', + errorClaudeCliNoResult: 'Claude CLI returned no result ({length} chars). See console for excerpt.', errorCustomHeadersJsonParse: 'Custom headers JSON parse failed: {error}', errorCustomHeadersJsonObject: 'Custom headers JSON must be an object', errorCustomHeadersLineFormat: 'Custom headers lines must use `Header-Name: value`', @@ -279,6 +290,7 @@ export const STRINGS: Record> = { 'Leave blank to auto-detect common paths. Obsidian launched from the GUI may not inherit shell PATH.', settingCliPathPlaceholder: 'Example: /Users/you/bin/codex', settingCliTimeoutName: 'CLI timeout (ms)', + settingCliTimeoutDesc: 'Max wait time for CLI calls (ms). Minimum 1000, default 120000.', apiProviderHeader: 'API Provider', settingProviderPresetName: 'Provider preset', settingProviderPresetDesc: @@ -299,7 +311,7 @@ export const STRINGS: Record> = { settingModelName: 'Model', settingModelDescApi: 'Model ID for API calls. Supports OpenClaw-style provider/model; matching provider prefixes are stripped.', - settingModelDescCli: 'Passed as --model for Claude Code. Usually ignored by Codex, which uses its default config.', + settingModelDescCli: 'Passed as --model for Claude Code. Codex always uses its own config default.', settingMaxInputName: 'Max input characters', settingMaxInputDesc: 'Longer notes are truncated before sending to the model. Raise this for long-context models.', promptHeader: 'Prompt', diff --git a/src/provider-parsers.ts b/src/provider-parsers.ts index b8a0225..6165e07 100644 --- a/src/provider-parsers.ts +++ b/src/provider-parsers.ts @@ -55,7 +55,7 @@ export function cardsFromAnthropicToolUse( const block = content.find((c) => c && c.type === 'tool_use' && c.name === ANTHROPIC_CARD_TOOL_NAME); if (!block) return null; if (typeof block.input === 'string') return parseCardsJson(block.input, settings); - if (block.input && typeof block.input === 'object') return normalizeCardsPayload(block.input); + if (block.input && typeof block.input === 'object') return normalizeCardsPayload(block.input, settings); return []; } diff --git a/src/schema.ts b/src/schema.ts index 8b236c0..1ec015a 100644 --- a/src/schema.ts +++ b/src/schema.ts @@ -118,28 +118,34 @@ export function parseCardsJson(text: string, settings?: PluginSettings | null): (parsed as { cards?: unknown[] }).cards?.length ?? 0, 'complete cards. Consider increasing max tokens.', ); - return normalizeCardsPayload(parsed); + return normalizeCardsPayload(parsed, settings); } catch (_) { /* repair failed, fall through to error */ } } - console.warn('[parallel-reader] LLM returned non-JSON. Raw response:', text); + console.warn( + '[parallel-reader] LLM returned non-JSON. length=', + (text || '').length, + 'head=', + (text || '').slice(0, 80), + ); throw new Error( translate(settings || null, 'errorLlmNonJson', { - excerpt: (text || '').slice(0, 500), + length: String((text || '').length), }), ); } - return normalizeCardsPayload(parsed); + return normalizeCardsPayload(parsed, settings); } -export function normalizeCardsPayload(parsed: unknown): RawCard[] { +export function normalizeCardsPayload(parsed: unknown, settings?: PluginSettings | null): RawCard[] { const obj = parsed as { cards?: unknown[] } | null | undefined; const raw = obj && Array.isArray(obj.cards) ? obj.cards : []; + const fallbackTitle = translate(settings ?? null, 'cardUntitled'); return raw .filter((c): c is Record => !!c && typeof c === 'object') .map((c) => ({ - title: typeof c.title === 'string' ? c.title : '(无标题)', + title: typeof c.title === 'string' ? c.title : fallbackTitle, anchor: typeof c.anchor === 'string' ? c.anchor : '', gist: typeof c.gist === 'string' ? c.gist : '', bullets: Array.isArray(c.bullets) diff --git a/src/settings-tab.ts b/src/settings-tab.ts index 8d6d23d..9f56d0d 100644 --- a/src/settings-tab.ts +++ b/src/settings-tab.ts @@ -12,6 +12,7 @@ import { DEFAULT_SETTINGS, getApiFormat, getApiPreset, + normalizeCliTimeoutMs, normalizeStreamingTimeoutMs, PROMPT_LANGUAGES, UI_LANGUAGES, @@ -117,6 +118,19 @@ export class ParallelReaderSettingTab extends PluginSettingTab { this.plugin.saveSettingsDebounced(); }), ); + + new Setting(containerEl) + .setName(this.tr('settingCliTimeoutName')) + .setDesc(this.tr('settingCliTimeoutDesc')) + .addText((t) => + t + .setPlaceholder(String(DEFAULT_SETTINGS.cliTimeoutMs)) + .setValue(String(this.plugin.settings.cliTimeoutMs || DEFAULT_SETTINGS.cliTimeoutMs)) + .onChange((v) => { + this.plugin.settings.cliTimeoutMs = normalizeCliTimeoutMs(v); + this.plugin.saveSettingsDebounced(); + }), + ); } private renderApiBackendSettings(containerEl: HTMLElement) { diff --git a/src/settings.ts b/src/settings.ts index b8d346c..eda395d 100644 --- a/src/settings.ts +++ b/src/settings.ts @@ -12,6 +12,7 @@ export const PROMPT_VERSION = 2; export const CACHE_SCHEMA_VERSION = 2; export const DEFAULT_MAX_CACHE_ENTRIES = 100; export const MIN_STREAMING_TIMEOUT_MS = 1000; +export const MIN_CLI_TIMEOUT_MS = 1000; export const PROMPT_LANGUAGES = { zh: '中文', en: 'English', @@ -180,6 +181,8 @@ export function applyApiProviderPreset(settings: Readonly, provi apiBaseUrl: preset.baseUrl, apiAuthType: preset.authType || 'auto', apiKeyEnvVar: preset.envVar || '', + apiKey: '', + apiHeaders: '', ...(shouldSwapModel ? { model: preset.model || '' } : {}), }; } @@ -211,6 +214,7 @@ export function normalizeSettings(settings: Readonly): PluginSet out.maxCards = normalizeCardCount(out.maxCards, DEFAULT_SETTINGS.maxCards); if (out.maxCards < out.minCards) out.maxCards = out.minCards; out.streamingTimeoutMs = normalizeStreamingTimeoutMs(out.streamingTimeoutMs); + out.cliTimeoutMs = normalizeCliTimeoutMs(out.cliTimeoutMs); if (typeof out.customSystemPrompt !== 'string') out.customSystemPrompt = ''; return out; } @@ -233,6 +237,12 @@ export function normalizeStreamingTimeoutMs(value: unknown): number { return n; } +export function normalizeCliTimeoutMs(value: unknown): number { + const n = Math.floor(Number(value)); + if (!Number.isFinite(n) || n < MIN_CLI_TIMEOUT_MS) return DEFAULT_SETTINGS.cliTimeoutMs; + return n; +} + function cacheEntryTime(entry: CacheEntry): number { const value = entry && (entry.lastAccessedAt || entry.generatedAt || entry.updatedAt); const timestamp = Date.parse(value || ''); diff --git a/src/test-exports.ts b/src/test-exports.ts index 2c8482c..ee64064 100644 --- a/src/test-exports.ts +++ b/src/test-exports.ts @@ -45,11 +45,13 @@ export { export { collectJsonObjectCandidates, extractJson, normalizeCardsPayload, repairTruncatedCardsJson } from './schema'; export { createRafThrottledHandler, visibleTopProbeY } from './scroll'; export { + applyApiProviderPreset, CACHE_SCHEMA_VERSION, cacheEntryMatches, generationFingerprint, getApiBaseUrl, modelForApi, + normalizeCliTimeoutMs, normalizeSettings, normalizeStreamingTimeoutMs, pruneCacheEntries, diff --git a/src/view.ts b/src/view.ts index 6ad4a0a..8d8dd14 100644 --- a/src/view.ts +++ b/src/view.ts @@ -424,7 +424,7 @@ export class ParallelReaderView extends ItemView { const markdown = [ '---', - `source: [[${this.sourceFile.basename}]]`, + `source: [[${this.sourceFile.path}|${this.sourceFile.basename}]]`, `generated: ${new Date().toISOString().slice(0, 10)}`, 'tool: parallel-reader', '---', @@ -434,25 +434,30 @@ export class ParallelReaderView extends ItemView { ].join('\n'); const app = this.plugin.app; - await ensureVaultFolder(app, folder); - - const existing = app.vault.getAbstractFileByPath(targetPath); - if (existing instanceof TFile) { - const shouldOverwrite = await confirmExportOverwrite( - this.app, - this.plugin.t('displayName'), - this.plugin.t('confirmExportOverwrite', { path: targetPath }), - this.plugin.t('confirmExportCancel'), - this.plugin.t('confirmExportOverwriteButton'), - ); - if (!shouldOverwrite) { - new Notice(this.plugin.t('exportCancelled')); - return; + try { + await ensureVaultFolder(app, folder); + const existing = app.vault.getAbstractFileByPath(targetPath); + if (existing instanceof TFile) { + const shouldOverwrite = await confirmExportOverwrite( + this.app, + this.plugin.t('displayName'), + this.plugin.t('confirmExportOverwrite', { path: targetPath }), + this.plugin.t('confirmExportCancel'), + this.plugin.t('confirmExportOverwriteButton'), + ); + if (!shouldOverwrite) { + new Notice(this.plugin.t('exportCancelled')); + return; + } + await app.vault.modify(existing, markdown); + } else { + await app.vault.create(targetPath, markdown); } - await app.vault.modify(existing, markdown); - } else { - await app.vault.create(targetPath, markdown); + new Notice(this.plugin.t('exported', { path: targetPath })); + } catch (e: unknown) { + const error = e instanceof Error ? e.message : String(e); + console.error('[parallel-reader] exportToVault failed', e); + new Notice(this.plugin.t('exportFailed', { error })); } - new Notice(this.plugin.t('exported', { path: targetPath })); } } diff --git a/tests/cli.test.js b/tests/cli.test.js index a2b02c8..88782c6 100644 --- a/tests/cli.test.js +++ b/tests/cli.test.js @@ -278,6 +278,82 @@ async function testCodexArgs() { // Verify stdin content combines system and user prompts assert.ok(capturedArgs.includes('exec'), 'should include exec subcommand'); + + // Verify sandbox is read-only (security: prevent prompt injection from running tools) + const sandboxIdx = capturedArgs.indexOf('--sandbox'); + assert.ok(sandboxIdx >= 0, 'should pass --sandbox to codex exec'); + assert.strictEqual(capturedArgs[sandboxIdx + 1], 'read-only', 'sandbox value must be read-only'); + + // Codex never receives --model (settings.model defaults to a Claude-name and would break codex) + assert.ok(!capturedArgs.includes('--model'), 'codex args must not include --model (defers to codex config)'); +} + +async function testCodexArgsIgnoresClaudeDefaultModel() { + let capturedArgs = null; + const resultJson = '{"cards":[{"title":"T","anchor":"hello world test anchor","gist":"G","bullets":["B"]}]}'; + const fakeSpawn = (_cmd, args) => { + capturedArgs = args; + const child = createFakeChild(); + process.nextTick(() => { + child.stdout.emit('data', Buffer.from(resultJson)); + child.emit('close', 0); + }); + return child; + }; + + // Even if settings.model is set (e.g. user has DEFAULT_SETTINGS.model = 'claude-sonnet-4-6'), + // codex must NOT receive --model. + const settings = { + backend: 'codex', + cliPath: '/usr/bin/codex', + cliTimeoutMs: 5000, + model: 'claude-sonnet-4-6', + promptLanguage: 'zh', + minCards: 5, + maxCards: 15, + maxDocChars: 100000, + }; + + await t.summarizeViaCodex('system prompt', 'user content', settings, undefined, fakeSpawn); + assert.ok(!capturedArgs.includes('--model'), 'codex must ignore settings.model regardless of value'); +} + +async function testClaudeCodeArgsWithModel() { + let capturedArgs = null; + const resultJson = JSON.stringify([ + { + type: 'result', + result: '{"cards":[{"title":"T","anchor":"hello world test anchor","gist":"G","bullets":["B"]}]}', + }, + ]); + const fakeSpawn = (_cmd, args) => { + capturedArgs = args; + const child = createFakeChild(); + process.nextTick(() => { + child.stdout.emit('data', Buffer.from(resultJson)); + child.emit('close', 0); + }); + return child; + }; + + const baseSettings = { + backend: 'claude-code', + cliPath: '/usr/bin/claude', + apiMaxTokens: 8192, + cliTimeoutMs: 5000, + promptLanguage: 'zh', + minCards: 5, + maxCards: 15, + maxDocChars: 100000, + }; + + await t.summarizeViaClaudeCode('system prompt', 'user content', baseSettings, undefined, fakeSpawn); + assert.ok(!capturedArgs.includes('--model'), 'claude args omit --model when settings.model unset'); + + await t.summarizeViaClaudeCode('system', 'user', { ...baseSettings, model: 'claude-opus-4-7' }, undefined, fakeSpawn); + const modelIdx = capturedArgs.indexOf('--model'); + assert.ok(modelIdx >= 0, 'claude should pass --model when settings.model set'); + assert.strictEqual(capturedArgs[modelIdx + 1], 'claude-opus-4-7', 'claude --model value matches settings.model'); } // ── classifyGenerationError ── @@ -289,6 +365,31 @@ assert.strictEqual(t.classifyGenerationError(new Error('CLI 超时 (120000ms)')) assert.strictEqual(t.classifyGenerationError(new Error('API returned HTTP 429')), 'rate-limit'); assert.strictEqual(t.classifyGenerationError(new Error('json_schema validation failed')), 'schema'); assert.strictEqual(t.classifyGenerationError(new Error('LLM 返回非 JSON')), 'schema'); +assert.strictEqual( + t.classifyGenerationError(new Error('LLM returned non-JSON (123 chars). See console for excerpt.')), + 'schema', + 'EN non-JSON message classified as schema', +); +assert.strictEqual( + t.classifyGenerationError(new Error('Claude CLI returned unexpected output (200 chars).')), + 'schema', + 'EN unexpected output classified as schema', +); +assert.strictEqual( + t.classifyGenerationError(new Error('Claude CLI returned no result (0 chars).')), + 'schema', + 'EN no result classified as schema', +); +assert.strictEqual( + t.classifyGenerationError(new Error('Claude CLI 返回了非预期输出（长度 200 字符）')), + 'schema', + 'zh unexpected-output message classified as schema', +); +assert.strictEqual( + t.classifyGenerationError(new Error('Claude CLI 没有返回结果（长度 0 字符）')), + 'schema', + 'zh no-result message classified as schema', +); assert.strictEqual(t.classifyGenerationError(new Error('bad config value')), 'config'); assert.strictEqual(t.classifyGenerationError(new Error('random error')), 'unknown'); assert.strictEqual(t.classifyGenerationError(null), 'unknown', 'null error'); @@ -297,7 +398,9 @@ assert.strictEqual(t.classifyGenerationError('string error'), 'unknown', 'string (async () => { await testRunCliEdgeCases(); await testClaudeCodeArgs(); + await testClaudeCodeArgsWithModel(); await testCodexArgs(); + await testCodexArgsIgnoresClaudeDefaultModel(); console.log('cli tests passed'); })().catch((e) => { console.error(e); diff --git a/tests/schema.test.js b/tests/schema.test.js index b64ee75..e4b795f 100644 --- a/tests/schema.test.js +++ b/tests/schema.test.js @@ -101,9 +101,14 @@ assert.deepStrictEqual( 'valid card passes through', ); assert.deepStrictEqual( - normalizeCardsPayload({ cards: [{ title: 123, gist: null, bullets: [1, 'valid', null] }] }), + normalizeCardsPayload({ cards: [{ title: 123, gist: null, bullets: [1, 'valid', null] }] }, { uiLanguage: 'zh' }), [{ title: '(无标题)', anchor: '', gist: '', bullets: ['valid'] }], - 'non-string fields get defaults, non-string bullets filtered', + 'non-string fields get defaults (zh fallback title), non-string bullets filtered', +); +assert.deepStrictEqual( + normalizeCardsPayload({ cards: [{ title: 123, gist: null, bullets: ['only'] }] }, { uiLanguage: 'en' }), + [{ title: '(Untitled)', anchor: '', gist: '', bullets: ['only'] }], + 'fallback title respects en uiLanguage', ); console.log('schema tests passed'); diff --git a/tests/settings.test.js b/tests/settings.test.js index ded2c9f..eb9e5fc 100644 --- a/tests/settings.test.js +++ b/tests/settings.test.js @@ -81,4 +81,32 @@ assert.strictEqual( ); assert.strictEqual(t.shouldSkipBatchFile(null, 'test', baseSettings), false, 'missing cache entry is not skipped'); +// normalizeCliTimeoutMs (mirrors normalizeStreamingTimeoutMs) +assert.strictEqual(t.normalizeCliTimeoutMs('60000'), 60000, 'cli timeout accepts numeric strings'); +assert.strictEqual(t.normalizeCliTimeoutMs(999), 120000, 'cli timeout below minimum falls back to default'); +assert.strictEqual(t.normalizeCliTimeoutMs('bad'), 120000, 'cli timeout rejects non-numeric values'); +assert.strictEqual( + t.normalizeSettings({ ...baseSettings, cliTimeoutMs: 500 }).cliTimeoutMs, + 120000, + 'normalizeSettings protects invalid cli timeout values', +); + +// applyApiProviderPreset clears credentials but keeps preset baseUrl (security: prevent cross-provider key leak) +{ + const previous = { + ...baseSettings, + apiProvider: 'anthropic', + apiKey: 'sk-ant-secret', + apiHeaders: 'Authorization: Bearer leaked', + apiBaseUrl: 'https://api.anthropic.com/v1', + }; + const switched = t.applyApiProviderPreset(previous, 'openai'); + assert.strictEqual(switched.apiKey, '', 'switching provider clears apiKey'); + assert.strictEqual(switched.apiHeaders, '', 'switching provider clears apiHeaders'); + assert.notStrictEqual(switched.apiBaseUrl, '', 'switching provider keeps preset baseUrl (not blank)'); + assert.ok(switched.apiBaseUrl.includes('openai'), 'switched apiBaseUrl matches new openai preset'); + assert.notStrictEqual(switched, previous, 'applyApiProviderPreset returns new object'); + assert.strictEqual(previous.apiKey, 'sk-ant-secret', 'applyApiProviderPreset does not mutate input'); +} + console.log('settings tests passed'); diff --git a/tests/test-exports.test.js b/tests/test-exports.test.js index ba508a4..320215b 100644 --- a/tests/test-exports.test.js +++ b/tests/test-exports.test.js @@ -36,6 +36,8 @@ const expectedExports = [ 'validateBatchFolderInput', 'normalizeSettings', 'normalizeStreamingTimeoutMs', + 'normalizeCliTimeoutMs', + 'applyApiProviderPreset', ]; for (const name of expectedExports) {