From aaa5d5f44706b63d55743fdad293da2c001893c9 Mon Sep 17 00:00:00 2001 From: Nicholas023 Date: Tue, 20 Oct 2026 14:30:57 +0800 Subject: [PATCH 1/4] =?UTF-8?q?fix(#21):=20js-yaml=20=E5=8A=A0=E8=BD=BD?= =?UTF-8?q?=E5=A4=B1=E8=B4=A5=E8=BE=93=E5=87=BA=E5=8F=AF=E8=AF=8A=E6=96=AD?= =?UTF-8?q?=E4=BF=A1=E6=81=AF=EF=BC=8C=E4=BF=9D=E6=8C=81=E8=BF=94=E5=9B=9E?= =?UTF-8?q?=20null=20=E5=A5=91=E7=BA=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/model-provider.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/model-provider.js b/src/model-provider.js index ff0692c..224e491 100644 --- a/src/model-provider.js +++ b/src/model-provider.js @@ -64,7 +64,13 @@ function loadProfileLlmConfig() { let yaml; try { yaml = require('js-yaml'); - } catch { + } catch (error) { + // issue #21：js-yaml 依赖损坏（缺包/版本不兼容/加载抛错）此前被静默吞掉—— + // 路由表为空 → 评审 0.0s 失败且失败文案不指向原因。这里只输出可诊断信息 + // （真实错误原因），绝不抛出、绝不改变返回值契约：scripts/switch-model-e2e.mjs + // 的 check('解析出端点', !!ep) + ep?.keyEnv ?? '' 安全链与运行态降级链都 + // 依赖「解析失败 = 返回 null 不抛」。 + console.error('[dsh-code-review] 警告：加载 js-yaml 失败（依赖损坏），模型路由表将为空，评审可能失败。原因：', String(error?.message ?? error)); return null; // 开发环境若 junction 未建也能继续（路由表可能为空，resolveProvider 返回 null） } // 根目录统一走 resolveDshRoot：优先 DSH_HOME，回退 ~/.dsh（issue #13）。 From a12bd2fabf2af7ed0e81842c71de9c273ebcfc2e Mon Sep 17 00:00:00 2001 From: Nicholas023 Date: Tue, 20 Oct 2026 14:30:58 +0800 Subject: [PATCH 2/4] =?UTF-8?q?fix(#22):=20e2e=20=E6=97=A0=20key=20?= =?UTF-8?q?=E7=9F=AD=E8=B7=AF=E8=B7=B3=E8=BF=87=E5=BC=95=E6=93=8E=E8=B0=83?= =?UTF-8?q?=E7=94=A8=EF=BC=9B=E6=95=8F=E6=84=9F=E6=96=AD=E8=A8=80=E6=8F=90?= =?UTF-8?q?=E5=87=BA=E5=B5=8C=E5=A5=97=E6=9D=9C=E7=BB=9D=E5=81=87=E7=BB=BF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scripts/engine-e2e-smoke.mjs | 6 ++++++ scripts/sensitive-e2e-smoke.mjs | 18 ++++++++++++++---- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/scripts/engine-e2e-smoke.mjs b/scripts/engine-e2e-smoke.mjs index e3dab4d..5af8fe9 100644 --- a/scripts/engine-e2e-smoke.mjs +++ b/scripts/engine-e2e-smoke.mjs @@ -111,6 +111,12 @@ try { // 提示不再硬编码 codecheck.exe——平台化路径由 // BIN 计算得出，提示直接引用它（Linux/macOS 上出现 .exe 字面会误导）。 check(`引擎存在（${BIN}）`, false, `请先把引擎二进制放到插件包 bin/（预期 ${BIN}）`); + } else if (typeof key !== 'string' || key.length === 0) { + // issue #22：key 未解析（无 .credentials.yaml / 未配置 DEEPSEEK_API_KEY）时， + // 此前仍把 undefined 塞进 OCR_LLM_TOKEN → spawn 成字面量 'undefined'，引擎 + // 拿它空等 180s 必败且无明确归因。显式短路跳过引擎调用，并输出明确 check + // （无 key 环境下跑 e2e 是「预期跳过」形态，不是挂死也不是假失败）。 + check('codecheck review（需 DEEPSEEK_API_KEY）', false, '跳过引擎调用（key 缺失）——本机未配置评审凭据，引擎实证无法执行'); } else { const env = { ...process.env, diff --git a/scripts/sensitive-e2e-smoke.mjs b/scripts/sensitive-e2e-smoke.mjs index d55aad9..4ea2cb9 100644 --- a/scripts/sensitive-e2e-smoke.mjs +++ b/scripts/sensitive-e2e-smoke.mjs @@ -112,11 +112,21 @@ try { 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)); - check('引擎输出未含 .env 内容（送审链路也未触碰）', leakedOut.length === 0, leakedOut.join(',') || '无泄露'); + } else { + // issue #22：报告断言不再静默跳过——评审失败（报告未落盘）时逐条记 FAIL + // 并归因到「评审未产出报告」，杜绝零失败项假绿。 + check('报告含「敏感文件点名」节', false, '报告缺失——评审未产出报告，点名断言无法通过'); + check('报告点名 .env（内容未送审的兜底人眼检查点）', false, '报告缺失——评审未产出报告，点名断言无法通过'); + check('报告未含 .env 内容（密钥值不出现 = 内容未送审）', false, '报告缺失——评审未产出报告，泄露断言无法通过'); } + // 引擎评审输出泄露断言：从 candidates>0 嵌套提出（issue #22）——无论报告是否 + // 落盘，只要 reviewOnce 有返回就检查其不泄露 .env 内容；评审静默失败（text 空） + // 时也记录失败归因，而非整段跳过。 + const engineOut = text; + const envProbesOut = ['EXAMPLE_PLACEHOLDER', 'EXAMPLE_NOT_A_REAL_KEY_12345']; + const leakedOut = envProbesOut.filter((s) => engineOut.includes(s)); + check('引擎输出未含 .env 内容（送审链路也未触碰）', engineOut.length > 0 && leakedOut.length === 0, + engineOut.length === 0 ? '评审输出为空（评审失败/跳过），泄露断言视同未通过' : (leakedOut.join(',') || '无泄露')); } finally { rmSync(work, { recursive: true, force: true }); console.log('临时仓库/报告目录已清理。'); From 692b0ecc5387a6d216fc67f05c7b0fbf99ec1129 Mon Sep 17 00:00:00 2001 From: Nicholas023 Date: Tue, 20 Oct 2026 14:30:59 +0800 Subject: [PATCH 3/4] =?UTF-8?q?fix(#23):=20=E6=9D=83=E9=99=90=E5=A3=B0?= =?UTF-8?q?=E6=98=8E=E8=A1=A5=E5=AE=89=E5=85=A8=E6=89=BF=E8=AF=BA=EF=BC=9B?= =?UTF-8?q?report=5Fdir/engine=5Fbin=20=E7=A0=81=E7=BA=A7=E5=86=99?= =?UTF-8?q?=E5=89=8D=E6=A0=A1=E9=AA=8C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- dsh-plugin.json | 8 ++--- src/set-config-tool.js | 81 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 80 insertions(+), 9 deletions(-) diff --git a/dsh-plugin.json b/dsh-plugin.json index 1ed878e..77533ca 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 配置的报告目录", - "reason": "落盘任务簿（tasks/ 任务文件）/ 插件运行配置（plugin-config.json）/ 评审报告留档" + "scope": "~/.dsh/dsh-code-review/**、默认报告目录（~ 展开后的 ~/dsh-code-review-reports/**）及用户经 report_dir 配置的报告目录。写入前校验：路径解析为绝对路径后必须位于用户 home 下，且 realpath 解析符号链接后无目录遍历逃逸（issue #23）", + "reason": "落盘任务簿（tasks/ 任务文件）/ 插件运行配置（plugin-config.json）/ 评审报告留档（写入路径按 scope 声明校验，防提示注入把报告写到任意目录）" }, { "name": "process.spawn", - "scope": "插件包 bin/ 或 PATH 中的 codecheck（含用户经 engine_bin 显式配置、写入前做本地文件校验的路径）；辅助命令 git / where|which / taskkill（平台子进程树终止）；以及以 process.execPath 拉起的本插件后台 serve 进程", + "scope": "插件包 bin/ 或 PATH 中的 codecheck（含用户经 engine_bin 显式配置的路径（写入前校验：绝对路径、文件存在、非符号链接、非 setuid 位，issue #23））；辅助命令 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 调用，绝不写入日志或评审报告（issue #23）" } ], "contributes": { diff --git a/src/set-config-tool.js b/src/set-config-tool.js index 7a1a525..8330e4a 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, lstatSync, realpathSync, constants } from 'node:fs'; +import { resolve, join, isAbsolute, basename, dirname } from 'node:path'; import { homedir } from 'node:os'; import { cwdOfExec } from './exec-context.js'; @@ -189,6 +189,26 @@ export function createSetConfigTool(ctx, getConfig, io = {}) { engineBinRejected = `engine_bin 校验失败：\`${candidate}\` 存在但不可执行（权限不足或非可执行文件，传空串可恢复自动探测）。`; } } + // issue #23：补「绝对路径 / 非符号链接 / 非 setuid」校验——候选由 + // resolve/join 构造通常已是绝对路径，仍显式校验一次（防御未来改动）； + // statSync 跟随符号链接，链接指向任意可执行文件也能通过 exists+isFile+ + // X_OK 三层，需 lstat 单独判定；setuid 位会让引擎进程以文件属主权限 + // 运行。与 dsh-plugin.json process.spawn scope 承诺逐字对齐。 + if (engineBinRejected === null && !isAbsolute(candidate)) { + engineBinRejected = `engine_bin 校验失败：\`${candidate}\` 非绝对路径（该字段未写入）。`; + } + if (engineBinRejected === null) { + try { + const lst = lstatSync(candidate); + if (lst.isSymbolicLink()) { + engineBinRejected = `engine_bin 校验失败：\`${candidate}\` 是符号链接（防引擎路径被重定向到任意可执行文件，不接受符号链接，传空串可恢复自动探测）。`; + } else if (!IS_WIN && (lst.mode & 0o4000) !== 0) { + engineBinRejected = `engine_bin 校验失败：\`${candidate}\` 带 setuid 位（拒绝以文件属主权限拉起引擎，传空串可恢复自动探测）。`; + } + } catch (error) { + engineBinRejected = `engine_bin 校验异常：${String(error?.message ?? error)}（该字段未写入，其余字段不受影响）`; + } + } } catch (error) { engineBinRejected = `engine_bin 校验异常：${String(error?.message ?? error)}（该字段未写入，其余字段不受影响）`; } @@ -199,12 +219,62 @@ export function createSetConfigTool(ctx, getConfig, io = {}) { patch.engine_bin = candidate; } } - // 整单只剩被拒的 engine_bin（其余字段时照写，拒绝原因进回显）。 - if (Object.keys(patch).length === 0 && engineBinRejected !== null) { + // issue #23（深度方案）：report_dir 写前校验——此前直接落盘无校验， + // 恶意/误导路径（提示注入）会让后续评审把报告写到任意目录；码级校验 + // 与 dsh-plugin.json fs.write scope 承诺一致：展开 ~ → 绝对路径 → 必须 + // 位于用户 home 下 → 已存在段经 realpath 解析符号链接后无目录遍历逃逸。 + // 与 engine_bin 同款「部分拒绝」语义：只剔除该字段，其余字段照常落盘。 + let reportDirRejected = null; + if (patch.report_dir !== null && patch.report_dir !== undefined) { + let rd; + try { + const home = homedir(); + if (patch.report_dir === '~') { + rd = home; + } else if (patch.report_dir.startsWith('~/') || patch.report_dir.startsWith('~\\')) { + rd = join(home, patch.report_dir.slice(2)); + } else { + rd = resolve(cwdOfExec(_exec) ?? process.cwd(), patch.report_dir); + } + if (!isAbsolute(rd)) throw new Error(`解析结果 \`${rd}\` 非绝对路径`); + // home 前缀比对：Windows 卷/盘符与目录名大小写不敏感，小写归一后比较。 + const norm = (p) => (IS_WIN ? String(p).toLowerCase() : String(p)); + const homeReal = realpathSync(home); + let rdReal = rd; + if (existsSync(rd)) { + rdReal = realpathSync(rd); // 已存在：全量解析（含符号链接跳转） + } else { + // 目录尚未创建：逐级向上找最近存在祖先做 realpath，再原序拼接缺失 + // 段——防「home 下看似合法、前缀段却是符号链接指向 home 外」（遍历逃逸）。 + const missing = []; + let probe = rd; + while (!existsSync(probe)) { + const parent = dirname(probe); + if (parent === probe) break; + missing.push(basename(probe)); + probe = parent; + } + rdReal = join(realpathSync(probe), ...missing.reverse()); + } + if (!norm(rdReal).startsWith(norm(homeReal))) { + reportDirRejected = `report_dir 校验失败：\`${patch.report_dir}\` 解析为 \`${rd}\`（realpath \`${rdReal}\`），不在用户 home（\`${home}\`）下（传空串可恢复默认）。`; + } + } catch (error) { + reportDirRejected = `report_dir 校验异常：${String(error?.message ?? error)}（该字段未写入，其余字段不受影响）`; + } + if (reportDirRejected !== null) { + delete patch.report_dir; + } else { + // 落盘规范化绝对路径（与 engine_bin 同款口径：校验与消费端同一视角）。 + patch.report_dir = rd; + } + } + // 整单只剩被拒的字段（其余字段时照写，拒绝原因进回显）。 + if (Object.keys(patch).length === 0 && (engineBinRejected !== null || reportDirRejected !== null)) { return { ok: false, config_path: configFile, - text: `## 代码审查插件配置（写入被拒）\n\n- ${engineBinRejected}\n- 已拒绝写入，现有配置未改动。`, + text: `## 代码审查插件配置（写入被拒）\n\n- ${engineBinRejected ?? reportDirRejected}\n- 已拒绝写入，现有配置未改动。`, }; } let wrote = false; @@ -237,6 +307,7 @@ export function createSetConfigTool(ctx, getConfig, io = {}) { // 成功回显必须带拒绝说明——「写入成功」只是其余字段成功，被拒字段 // 不可静默消失。 ...(engineBinRejected !== null ? ['', `- ⚠ ${engineBinRejected}（该字段未写入，其余字段已生效）`] : []), + ...(reportDirRejected !== null ? ['', `- ⚠ ${reportDirRejected}（该字段未写入，其余字段已生效）`] : []), '- 提示：面板「设置」里的同名字段在 0.2.0 宿主对第三方插件的回传通道修复前暂不生效，以本工具/本配置文件为准。', ]; return { ok: true, config_path: configFile, text: lines.join('\n') }; From 72e5f6920fea02a5f7156c7b99dc596f7130cadf Mon Sep 17 00:00:00 2001 From: Nicholas023 Date: Tue, 20 Oct 2026 14:31:00 +0800 Subject: [PATCH 4/4] =?UTF-8?q?fix(#24):=20=E6=95=8F=E6=84=9F=E5=90=8D?= =?UTF-8?q?=E5=8D=95=E6=94=B6=E6=95=9B=20token=20=E8=AF=8D=E8=BE=B9?= =?UTF-8?q?=E7=95=8C/=E6=B8=85=E6=B4=97=E5=AD=97=E7=AC=A6/=E6=AD=A3?= =?UTF-8?q?=E5=88=99=E9=87=8D=E5=8F=A0/=E5=87=AD=E6=8D=AE=E9=94=9A?= =?UTF-8?q?=E5=AE=9A?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/sensitive-list.js | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/sensitive-list.js b/src/sensitive-list.js index 9f96c47..e1fb767 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：加尾锚 ([.\-/]|$) —— secret 只认「隐藏文件名词段结尾」形态 + // （.secret / .my.secret / .a-secret.bak），不再命中 .secretive 这类词干扩展； + // 独立词段 secrets/secret+x 形态统一由下方 (secrets?|credentials?) 负责，两规则不重叠。 + /(^|\/)\.[^/]*secret([.\-/]|$)/i, /(^|\/)\.env(\.[^/]+)?$/i, // .env / .env.production 等 // 凭据扩展名必须作为「独立扩展名」出现（my.key、cert.pfx、server.crt.bak 等）： // 敏感词前必须由点号分隔（[^/]*\.），后跟行尾或再一个点（如 .key.bak 的 .bak）。 @@ -38,14 +41,16 @@ export const SENSITIVE_PATH_PATTERNS = [ // key/cert/pem（sessionkey.go、keycmd.go 等）也不再命中；id_rsa/credentials/.env // 等由各自独立规则覆盖，不受此行影响。 /(^|\/)[^/]*\.(pem|p12|pfx|keystore|jks|cert|crt|key|kdbx|otpkey)(\.|$)/i, - /(^|\/)credentials[^/]*$/i, // token 模式收紧（形态：独立词段 token(s) + **凭据类扩展名或路径结尾**）—— // 原 `.*token[^/]*$` 命中路径任意位置含 "token" 的文件（src/tokenizer.js、 // token-utils/index.js 等业务文件也被点名），噪声淹没「逐个人眼确认」信号。 // 扩展名收敛到凭据类白名单（与下方 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：tokens? 补词边界 (?=[._-]|$) —— token 后必须是分隔符或行尾， + // 否则 tokenizer-decoder.yml、tokenization.js 这类「token 作词干」的普通 + // 命名会被误收（原正则部分匹配 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 +62,10 @@ export const SENSITIVE_PATH_PATTERNS = [ // secrets/credentials 命名（原 credentials 行只覆盖含字面量 credentials 的 // 基名、\.secret 要求点开头）。 /(^|\/)\.(npmrc|netrc|git-credentials)$/i, - /(^|\/)(secrets?|credentials?)(\.|\/|$)/i, + // issue #24：凭据扩展名锚定与 .env 模式（(\.[^/]+)?$）对齐——词段后只认 + // 「点扩展名 + 行尾」或「目录斜杠」，扩展名白名单不在这里重复判定（.env 同款 + // 口径：任何点后缀都视为凭据文件候选，误报面由词段本身收窄）。 + /(^|\/)(secrets?|credentials?)(\.[^/]*)?($|\/)/i, // 个人/隐私信息。 /(^|\/)(.*\.)?(id_rsa|id_dsa|id_ecdsa|id_ed25519)$/i, // 常见凭据兜底（作业本/测试台账类命名）。 @@ -117,7 +125,9 @@ export function buildSensitiveBlock(pendingPaths) { // 路径未经转义直接进反引号代码块——git 文件名可含 // 反引号/换行（git 允许），会闭合代码块并向报告注入任意 markdown。拼接前 // 剔除反引号与换行（点名目的是人眼确认路径，替换为空格不影响辨识）。 - const safe = String(h.path).replace(/[`\r\n*_[\]<>|]+/g, ' '); + // issue #24：清洗字符类补 ~ # &——GitHub 文件名可含这些字符，落到报告 + // markdown 里 # 会截断标题、& 触发 HTML 实体、~ 与删除线/下标语法冲突。 + const safe = String(h.path).replace(/[`\r\n*_[\]<>|~#&]+/g, ' '); return `${i + 1}. \`${safe}\``; }); return [