diff --git a/dsh-plugin.json b/dsh-plugin.json index 1ed878e..f7c7e85 100644 --- a/dsh-plugin.json +++ b/dsh-plugin.json @@ -16,18 +16,18 @@ "permissions": [ { "name": "fs.write", - "scope": "~/.dsh/dsh-code-review/**、默认报告目录（~ 展开后的 ~/dsh-code-review-reports/**）及用户经 report_dir 配置的报告目录", + "scope": "~/.dsh/dsh-code-review/**、默认报告目录（~ 展开后的 ~/dsh-code-review-reports/**）及用户经 report_dir 配置的报告目录（写入前校验：须为用户 home 下绝对路径，解析符号链接后确认无目录遍历逃逸）", "reason": "落盘任务簿（tasks/ 任务文件）/ 插件运行配置（plugin-config.json）/ 评审报告留档" }, { "name": "process.spawn", - "scope": "插件包 bin/ 或 PATH 中的 codecheck（含用户经 engine_bin 显式配置、写入前做本地文件校验的路径）；辅助命令 git / where|which / taskkill（平台子进程树终止）；以及以 process.execPath 拉起的本插件后台 serve 进程", + "scope": "插件包 bin/ 或 PATH 中的 codecheck（含用户经 engine_bin 显式配置的路径——engine_bin 须为绝对路径，且写入前校验文件存在、非符号链接、非 setuid 位）；辅助命令 git / where|which / taskkill（平台子进程树终止）；以及以 process.execPath 拉起的本插件后台 serve 进程", "reason": "评审引擎与后台 serve 均为外部子进程（由 runner / task-book 以子进程方式拉起执行），辅助命令服务于待提交清单（git）、引擎路径探测（where/which）与超时终止（taskkill）" }, { "name": "credentials.read", "scope": "~/.dsh/.credentials.yaml 与环境变量中的模型凭据引用", - "reason": "调用评审 LLM 所需的 API key，复用宿主既有凭据通道" + "reason": "调用评审 LLM 所需的 API key，复用宿主既有凭据通道；凭据仅用于 API 调用，绝不写入日志或评审报告" } ], "contributes": { diff --git a/scripts/engine-e2e-smoke.mjs b/scripts/engine-e2e-smoke.mjs index e3dab4d..a4551e7 100644 --- a/scripts/engine-e2e-smoke.mjs +++ b/scripts/engine-e2e-smoke.mjs @@ -111,6 +111,14 @@ try { // 提示不再硬编码 codecheck.exe——平台化路径由 // BIN 计算得出，提示直接引用它（Linux/macOS 上出现 .exe 字面会误导）。 check(`引擎存在（${BIN}）`, false, `请先把引擎二进制放到插件包 bin/（预期 ${BIN}）`); + } else if (typeof key !== 'string' || key.length < 8) { + // issue #22：key 未解析（undefined）时旧代码仍构造 OCR_LLM_TOKEN: key + // （成字面量 'undefined'）并 spawn 引擎空等 180s 必败——无 key 环境把 + // e2e 误报成「引擎失败」。短路跳过引擎调用：每次只注入自己的 key 判定 + // （与上方 check 同一口径 `typeof key === 'string' && key.length >= 8` 取反）， + // 明确输出跳过原因，不再空等、不误报失败。 + check('跳过引擎调用（key 缺失）', true, + '无 DEEPSEEK_API_KEY：不构造 OCR_LLM_TOKEN=undefined 的 spawn（避免空等 180s 必败），这是无 key 环境的预期跳过形态'); } else { const env = { ...process.env, diff --git a/scripts/sensitive-e2e-smoke.mjs b/scripts/sensitive-e2e-smoke.mjs index d55aad9..43114cd 100644 --- a/scripts/sensitive-e2e-smoke.mjs +++ b/scripts/sensitive-e2e-smoke.mjs @@ -102,6 +102,9 @@ try { }; if (existsSync(reportDir)) walk(reportDir); check('报告文件已落盘（按日期/仓库分目录）', candidates.length >= 1, candidates[0] ?? '无'); + // envProbes 提前定义（报告与引擎两条泄露断言共用同一夹具串—— + // 改夹具必须同步两处断言，防「改夹具不改断言 = 恒真假绿」）。 + const envProbes = ['EXAMPLE_PLACEHOLDER', 'EXAMPLE_NOT_A_REAL_KEY_12345']; if (candidates.length > 0) { reportPath = candidates[0]; const report = readFileSync(reportPath, 'utf8'); @@ -109,13 +112,18 @@ try { check('报告点名 .env（内容未送审的兜底人眼检查点）', report.includes('.env'), '点名项出现'); // 内容不送审：报告正文不得出现 .env 内容里的占位值。 // 断言值必须与上方写入 .env 的夹具**同一串**（改夹具不改断言 = 断言恒真假绿）。 - const envProbes = ['EXAMPLE_PLACEHOLDER', 'EXAMPLE_NOT_A_REAL_KEY_12345']; const leaked = envProbes.filter((s) => report.includes(s)); check('报告未含 .env 内容（密钥值不出现 = 内容未送审）', leaked.length === 0, leaked.join(',') || '无泄露'); - // 引擎评审输出同样不得含密钥值（送审对象只有 app.py diff）。 - const engineOut = text; - const leakedOut = envProbes.filter((s) => engineOut.includes(s)); + } + // issue #22：引擎输出泄露断言**不得嵌套在报告分支内**——reviewOnce 静默失败 + // 且报告未落盘时整段跳过 = 假绿。提出为独立断言：评审失败（text 空）也 + // 显式失败归因（不许空输出借「不含泄露样本」恒过）；空输出本身由上方 + // 「评审返回非空文本」check 钉住失败，两条失败信息叠加让排查指向同一根因。 + if (typeof text === 'string' && text.length > 0) { + const leakedOut = envProbes.filter((s) => text.includes(s)); check('引擎输出未含 .env 内容（送审链路也未触碰）', leakedOut.length === 0, leakedOut.join(',') || '无泄露'); + } else { + check('引擎输出未含 .env 内容（送审链路也未触碰）——评审无输出', false, '评审返回空文本，无法验证送审链路（根因见上方「评审返回非空文本」失败项）'); } } finally { rmSync(work, { recursive: true, force: true }); diff --git a/src/model-provider.js b/src/model-provider.js index ff0692c..adecd87 100644 --- a/src/model-provider.js +++ b/src/model-provider.js @@ -64,8 +64,18 @@ function loadProfileLlmConfig() { let yaml; try { yaml = require('js-yaml'); - } catch { - return null; // 开发环境若 junction 未建也能继续（路由表可能为空，resolveProvider 返回 null） + } catch (error) { + // issue #21：js-yaml 依赖损坏/缺失时此前静默 return null——路由表为空、 + // 评审 0.0s 失败且失败文案不指向原因，排查者无从下手。输出可诊断信息 + // （含真实错误原因）后**仍保持 return null 契约**：运行态降级链与 + // scripts/switch-model-e2e.mjs 的 check('解析出端点', !!ep) + ep?.keyEnv + // 安全链都依赖「解析失败 = 返回 null 不抛」，此分支只允许加诊断，绝不 + // 能改成抛异常/改返回值。 + console.error( + '[dsh-code-review] js-yaml 加载失败：评审模型配置（profile 的 cordis.patch.yml）无法解析，' + + '路由表将为空（评审会因缺少 LLM 端点而失败）。原因：' + String(error?.message ?? error), + ); + return null; // 契约不变：解析失败 = 返回 null 不抛（路由表空时由 resolveProvider 返回 null，调用方明确报错） } // 根目录统一走 resolveDshRoot：优先 DSH_HOME，回退 ~/.dsh（issue #13）。 const profilesRoot = join(resolveDshRoot(), 'profiles'); diff --git a/src/sensitive-list.js b/src/sensitive-list.js index 9f96c47..c1cfac7 100644 --- a/src/sensitive-list.js +++ b/src/sensitive-list.js @@ -28,7 +28,10 @@ export const SENSITIVE_PATH_PATTERNS = [ // 下方 id_* 行覆盖、*.pem 由凭据扩展名行覆盖）；.ssh 目录段单独无覆盖属 // 已知盲区（引擎侧路径保护另有 .ssh 排除），勿据此删除下方覆盖行。 /(^|\/)\.[^/]*key[^/]*$/i, - /(^|\/)\.[^/]*secret/i, + // issue #24：r2 与 r9 覆盖重叠归一——r2 只管「点开头的 secret 隐藏文件」（.secret/.secrets/.x.secret.yml）, + // r9 只管「非点开头词段」（(?!\.) 与 r2 互斥，见 r9 注释）；锚定收紧为 .env 式 + // （后缀段可选且必须结尾），去掉旧无锚定写法（.anythingsecretanything 乱接形态不再命中）。 + /(^|\/)\.[^/]*secrets?(\.[^/]+)?$/i, /(^|\/)\.env(\.[^/]+)?$/i, // .env / .env.production 等 // 凭据扩展名必须作为「独立扩展名」出现（my.key、cert.pfx、server.crt.bak 等）： // 敏感词前必须由点号分隔（[^/]*\.），后跟行尾或再一个点（如 .key.bak 的 .bak）。 @@ -37,7 +40,10 @@ export const SENSITIVE_PATH_PATTERNS = [ // 敏感文件，噪声稀释真正凭据命中的信号（issue #11）。修后路径段内嵌的 // key/cert/pem（sessionkey.go、keycmd.go 等）也不再命中；id_rsa/credentials/.env // 等由各自独立规则覆盖，不受此行影响。 - /(^|\/)[^/]*\.(pem|p12|pfx|keystore|jks|cert|crt|key|kdbx|otpkey)(\.|$)/i, + // issue #24：凭据扩展名锚定对齐 .env 模式（(\.[^/]+)?$）——扩展名后要么结尾、 + // 要么跟**一个后缀段**再结尾，且整段必须到行尾（旧 (\\.|$) 无尾锚，扩展名后 + // 跟随任意内容也命中，与 .env 的锚定口径不一致）。 + /(^|\/)[^/]*\.(pem|p12|pfx|keystore|jks|cert|crt|key|kdbx|otpkey)(\.[^/]+)?$/i, /(^|\/)credentials[^/]*$/i, // token 模式收紧（形态：独立词段 token(s) + **凭据类扩展名或路径结尾**）—— // 原 `.*token[^/]*$` 命中路径任意位置含 "token" 的文件（src/tokenizer.js、 @@ -45,7 +51,10 @@ export const SENSITIVE_PATH_PATTERNS = [ // 扩展名收敛到凭据类白名单（与下方 token+分隔符规则同一白名单）：token.json / // api_token / my-tokens 命中；token.js（令牌类型定义模块）、tokens.ts、 // design-tokens.md 等业务源码放出——避免把「代码文件」当凭据文件点名。 - /(^|\/)(.*[.\-_])?tokens?(\.(json|yml|yaml|txt|ini|conf|cfg|config|log|bak|old|tmp|env))?$/i, + // issue #24：token(s) 显式词边界（(?=[._-]|$)）——tokens? 之后必须紧跟分隔符 + // 或路径结尾，杜绝 tokenizer-decoder.yml 这类「token 词段后接普通字符」的形态 + // 在未来正则结构调整时被误收（本次在结构上锁死「独立词段」语义）。 + /(^|\/)(.*[.\-_])?tokens?(?=[._-]|$)(\.(json|yml|yaml|txt|ini|conf|cfg|config|log|bak|old|tmp|env))?$/i, // token 后跟分隔符、且带凭据类扩展名的形态（refresh_token_backup.json、 // token_store.yaml、my_token.txt）——上一行的「token 作为独立词段 + 点号 // 扩展名或路径结尾」覆盖不到它们：token 后不是点而是下划线/横线。此处不写 @@ -57,7 +66,10 @@ export const SENSITIVE_PATH_PATTERNS = [ // secrets/credentials 命名（原 credentials 行只覆盖含字面量 credentials 的 // 基名、\.secret 要求点开头）。 /(^|\/)\.(npmrc|netrc|git-credentials)$/i, - /(^|\/)(secrets?|credentials?)(\.|\/|$)/i, + // issue #24：r9 与 r2 覆盖重叠归一——本行只管**非点开头**的 secrets/credentials + // 词段（(?!\.) 负向前瞻：点开头形态由 r2 负责，两行职责互斥不重叠， + // 改一处不会留下另一处没同步的隐式覆盖）。 + /(^|\/)(?!\.)(secrets?|credentials?)(\.|\/|$)/i, // 个人/隐私信息。 /(^|\/)(.*\.)?(id_rsa|id_dsa|id_ecdsa|id_ed25519)$/i, // 常见凭据兜底（作业本/测试台账类命名）。 @@ -117,7 +129,10 @@ export function buildSensitiveBlock(pendingPaths) { // 路径未经转义直接进反引号代码块——git 文件名可含 // 反引号/换行（git 允许），会闭合代码块并向报告注入任意 markdown。拼接前 // 剔除反引号与换行（点名目的是人眼确认路径，替换为空格不影响辨识）。 - const safe = String(h.path).replace(/[`\r\n*_[\]<>|]+/g, ' '); + // issue #24：清洗字符类补 ~ # &——文件名可含删除线 ~、标题/锚点 #、HTML 实体 + // 起始 &、代码块反引号等，均会破坏报告 markdown 结构（文件名注入报告格式）； + // 点名目的是人眼确认路径，替换为空格不影响辨识。 + const safe = String(h.path).replace(/[`\r\n*_[\]<>|~#&]+/g, ' '); return `${i + 1}. \`${safe}\``; }); return [ diff --git a/src/set-config-tool.js b/src/set-config-tool.js index 7a1a525..05dcf24 100644 --- a/src/set-config-tool.js +++ b/src/set-config-tool.js @@ -22,8 +22,8 @@ * @module dsh-code-review/set-config-tool */ import { writeFileConfig, PLUGIN_CONFIG_FILE } from './file-config.js'; -import { existsSync, statSync, accessSync, constants } from 'node:fs'; -import { resolve, join } from 'node:path'; +import { existsSync, statSync, accessSync, constants, realpathSync } from 'node:fs'; +import { resolve, join, basename, dirname, sep } from 'node:path'; import { homedir } from 'node:os'; import { cwdOfExec } from './exec-context.js'; @@ -56,7 +56,7 @@ export function createSetConfigTool(ctx, getConfig, io = {}) { '【写入即生效】本工具把配置写入插件自有配置文件（用户目录下），后续评审任务立即按新值执行——', '不需要改客户端设置面板、不需要重启（0.2.0 宿主「面板保存」对第三方插件暂不回传运行层，面板改值会落盘但不到运行时，以本工具为准）。', '参数：review_model（评审模型标识：留空/省略 = 沿用会话模型；传其他值 = 显式另选，如 DSH 模型配置里已有的模型名）；', - 'report_dir（评审报告落盘目录，支持 ~；传空串恢复默认 ~/dsh-code-review-reports）；', + 'report_dir（评审报告落盘目录，支持 ~；写入前校验须为 home 下绝对路径、解析符号链接后无目录遍历逃逸；传空串恢复默认 ~/dsh-code-review-reports）；', 'engine_bin（引擎 codecheck 路径；留空 = 自动探测。写入路径前请先确认文件存在——不存在/目录会被拒绝写入）；', '一个参数都不传 = 只查询当前生效配置（含默认值补齐），返回全量字段。', ].join(' '), @@ -68,7 +68,7 @@ export function createSetConfigTool(ctx, getConfig, io = {}) { additionalProperties: false, properties: { review_model: { type: ['string', 'null'], description: '评审模型标识。省略/空串/null = 沿用会话模型；其他值 = 显式另选的模型。' }, - report_dir: { type: ['string', 'null'], description: '报告目录（支持 ~）。空串/null 恢复默认。' }, + report_dir: { type: ['string', 'null'], description: '报告目录（支持 ~；须位于 home 下，写入前校验路径无遍历逃逸）。空串/null 恢复默认。' }, engine_bin: { type: ['string', 'null'], description: '引擎二进制路径。空串/null 恢复自动探测。传路径前请先确认文件存在——不存在/目录会被拒绝写入。' }, }, required: [], @@ -199,12 +199,75 @@ export function createSetConfigTool(ctx, getConfig, io = {}) { patch.engine_bin = candidate; } } - // 整单只剩被拒的 engine_bin（其余字段时照写，拒绝原因进回显）。 - if (Object.keys(patch).length === 0 && engineBinRejected !== null) { + // ---- issue #23：report_dir 写前校验（与 engine_bin 同风格；落实 dsh-plugin.json + // fs.write 声明的承诺「写入前校验为用户 home 下绝对路径、解析符号链接后无目录 + // 遍历逃逸」）。report_dir 落盘后作为报告目录被 report.js 消费（mkdir/写文件）， + // 用户可控路径不加校验 = 提示注入可把后续评审报告写到系统目录/任意位置； + // 校验失败只剔除该字段（与 engine_bin 同一「逐字段合并」语义），原因进回显。 ---- + let reportDirRejected = null; + if (patch.report_dir !== null && patch.report_dir !== undefined) { + try { + const home = resolve(homedir()); + // underHome：规范化前缀判断（win32 忽略大小写；home 自身也算合法）。 + const underHome = (p) => { + const target = resolve(p); + const homeWithSep = home.endsWith(sep) ? home : home + sep; + return IS_WIN + ? target.toLowerCase() === home.toLowerCase() || target.toLowerCase().startsWith(homeWithSep.toLowerCase()) + : target === home || target.startsWith(homeWithSep); + }; + // 1) 展开 ~（与 report.js expandHome 同口径：'~' 与 '~/'、'~\\' 前缀）。 + let expanded; + if (patch.report_dir === '~') { + expanded = homedir(); + } else if (patch.report_dir.startsWith('~/') || patch.report_dir.startsWith('~\\')) { + expanded = join(homedir(), patch.report_dir.slice(2)); + } else { + // 相对路径以**调用方会话目录**为基准解析（与 engine_bin 同基准）； + // 解析结果不在 home 下会被下方前缀校验拦下（相对路径几乎必错，显式拒绝）。 + expanded = patch.report_dir; + } + const normAbs = resolve(cwdOfExec(_exec) ?? process.cwd(), expanded); + // 2) 绝对路径 + home 前缀（resolve 已规范化 .. 段，目录遍历逃逸在此层面失效）。 + if (!underHome(normAbs)) { + reportDirRejected = `report_dir 校验失败：\`${patch.report_dir}\` 解析为 \`${normAbs}\`，不在用户 home 目录下（报告目录须为 home 下绝对路径，传空串可恢复默认）。`; + } else { + // 3) 符号链接逃逸：对**最近存在的祖先**做 realpath（目录可能尚未创建， + // realpath 对不存在路径直接调用会抛 ENOENT），把不存在的后续段拼回， + // 再判断解析后仍在 home 内——防 home 下某父级是符号链接指向外部目录。 + let probe = normAbs; + const tail = []; + while (!existsSync(probe)) { + const parent = dirname(probe); + if (parent === probe) break; // 到盘根，不再上溯 + tail.unshift(basename(probe)); + probe = parent; + } + const realFull = resolve(join(realpathSync(probe), ...tail)); + if (!underHome(realFull)) { + reportDirRejected = `report_dir 校验失败：\`${patch.report_dir}\` 解析为 \`${normAbs}\`，解析符号链接后为 \`${realFull}\`，逃逸出用户 home 目录（报告目录须位于 home 下，传空串可恢复默认）。`; + } else if (existsSync(normAbs) && !statSync(normAbs).isDirectory()) { + reportDirRejected = `report_dir 校验失败：\`${normAbs}\` 已存在但不是目录（传空串可恢复默认）。`; + } else { + // 落盘规范化绝对路径（校验与消费端同一视角；expandHome 对绝对路径幂等，行为不变）。 + patch.report_dir = normAbs; + } + } + } catch (error) { + reportDirRejected = `report_dir 校验异常：${String(error?.message ?? error)}（该字段未写入，其余字段不受影响）`; + } + if (reportDirRejected !== null) { + delete patch.report_dir; + } + } + // 整单只剩被拒字段（engine_bin / report_dir，其余字段时照写，拒绝原因进回显）。 + const anyRejected = engineBinRejected !== null || reportDirRejected !== null; + if (Object.keys(patch).length === 0 && anyRejected) { + const rejected = [engineBinRejected, reportDirRejected].filter(Boolean).join('\n- '); return { ok: false, config_path: configFile, - text: `## 代码审查插件配置（写入被拒）\n\n- ${engineBinRejected}\n- 已拒绝写入，现有配置未改动。`, + text: `## 代码审查插件配置（写入被拒）\n\n- ${rejected}\n- 已拒绝写入，现有配置未改动。`, }; } let wrote = false; @@ -233,10 +296,11 @@ export function createSetConfigTool(ctx, getConfig, io = {}) { `- 评审模型：\`${active.review_model}\``, `- 报告目录：\`${active.report_dir}\``, `- 引擎路径：\`${active.engine_bin || '（自动探测）'}\``, - // 部分字段被拒（engine_bin 校验失败）时 + // 部分字段被拒（engine_bin / report_dir 校验失败）时 // 成功回显必须带拒绝说明——「写入成功」只是其余字段成功，被拒字段 // 不可静默消失。 ...(engineBinRejected !== null ? ['', `- ⚠ ${engineBinRejected}（该字段未写入，其余字段已生效）`] : []), + ...(reportDirRejected !== null ? ['', `- ⚠ ${reportDirRejected}（该字段未写入，其余字段已生效）`] : []), '- 提示：面板「设置」里的同名字段在 0.2.0 宿主对第三方插件的回传通道修复前暂不生效，以本工具/本配置文件为准。', ]; return { ok: true, config_path: configFile, text: lines.join('\n') };