Code Review：fix(webui): support PPT preview and shell open in server mode (#1679)

变更概述

本 PR 为 server（非 Electron）模式新增了两项能力：(1) shellBridgeStandalone 通过 Node.js child_process 实现 shell 操作（打开文件/外链/文件夹）；(2) /api/ppt-proxy/:port 反向代理路由将 officecli watch server 的 PPT 预览通过 Web 服务器暴露给浏览器 iframe。渲染层 PptViewer 根据运行环境分别使用 <webview>（Electron）或 <iframe>（server 模式）。变更涉及 src/process/bridge/、src/process/webserver/routes/apiRoutes.ts 和 src/renderer/ 共 5 个文件。

方案评估

结论：✅ 方案合理

整体方案思路正确：通过 port 白名单校验防止 SSRF、用 Location 重写保证代理透明性、注入 guard script 限制 iframe 导航逸出——三层防护设计合理。shellBridgeStandalone 使用 execFile（而非 exec）避免了 shell 注入，是正确选择。方案与项目已有的 process/renderer 分层架构完全一致。注入 guard script 采用模板字符串拼接 port（已验证为整数），不存在 XSS 风险。主要待完善点为：proxy 请求缺少超时保护、非 HTML 流式响应路径缺少 proxyRes 错误处理。

问题清单

🟠 HIGH — 非 HTML 流式响应路径缺少 proxyRes 错误处理

文件：src/process/webserver/routes/apiRoutes.ts，第 538–542 行

问题代码：

} else {
  res.removeHeader('X-Frame-Options');
  res.writeHead(statusCode, responseHeaders);
  proxyRes.pipe(res, { end: true });
}

问题说明：proxyRes 是一个 IncomingMessage 可读流。Node.js 中 pipe() 不会自动将 source 的 error 事件转发给 destination；如果 proxyRes 在 pipe 过程中触发 error（例如 officecli server 中途崩溃、连接被重置），且没有注册 error 监听器，该错误将成为 未处理的 EventEmitter error，在 Node.js 中会导致进程崩溃。对比同文件 HTML 路径（第 535–537 行）已经正确处理了 proxyRes.on('error', ...)，非 HTML 路径遗漏了这一保护。

修复建议：

} else {
  res.removeHeader('X-Frame-Options');
  res.writeHead(statusCode, responseHeaders);
  proxyRes.on('error', () => {
    // headers already sent via writeHead — can't change status, just destroy
    res.destroy();
  });
  proxyRes.pipe(res, { end: true });
}

🟡 MEDIUM — proxy 请求缺少超时配置，请求可能永久挂起

文件：src/process/webserver/routes/apiRoutes.ts，第 467–473 行

问题代码：

const options: http.RequestOptions = {
  hostname: '127.0.0.1',
  port,
  path: subPath + query,
  method: req.method,
  headers: proxyHeaders,
};

问题说明：未设置 timeout 字段。如果 officecli watch server 在接收请求后迟迟不返回响应（如处理大型 PPT 卡住），Express 请求会一直挂起，最终耗尽服务器连接资源。proxyReq.on('error') 只处理连接建立失败的情况，无法覆盖"连接成功但响应超时"的场景。

修复建议：

const PROXY_TIMEOUT_MS = 30_000;

const options: http.RequestOptions = {
  hostname: '127.0.0.1',
  port,
  path: subPath + query,
  method: req.method,
  headers: proxyHeaders,
  timeout: PROXY_TIMEOUT_MS,
};

// 在创建请求后追加：
proxyReq.on('timeout', () => {
  proxyReq.destroy();
  if (!res.headersSent) res.status(504).json({ message: 'PPT preview proxy timeout' });
});

🔵 LOW — guard script 中 location.href getter 返回相对路径，可能破坏 officecli 页面代码

文件：src/process/webserver/routes/apiRoutes.ts，第 515 行

问题代码：

try{Object.defineProperty(location,'href',{get:function(){return location.pathname+location.search+location.hash;},set:function(v){_a(rw(v));},configurable:true});}catch(e){}

问题说明：覆盖 location.href 的 getter 使其返回 pathname + search + hash（相对路径，如 /api/ppt-proxy/55555/index.html），而非浏览器标准的完整 URL（如 http://app-host/api/ppt-proxy/55555/index.html）。officecli 页面若读取 window.location.href 做 URL 构造（例如 new URL(path, location.href)），将因缺少 origin 而报错或得到错误结果。guard script 的核心目标是拦截导航，setter 拦截已足够——getter 覆盖是多余且有副作用的。

修复建议：移除 getter 覆盖，仅保留 setter 以拦截直接赋值导航：

try{Object.defineProperty(location,'href',{set:function(v){_a(rw(v));},configurable:true});}catch(e){}

🔵 LOW — isActivePreviewPort 无测试覆盖

文件：src/process/bridge/pptPreviewBridge.ts，第 277–284 行

问题说明：新导出的 isActivePreviewPort 函数是 proxy 路由 SSRF 防护的核心逻辑，但现有 tests/unit/pptPreviewBridge.test.ts 中未包含对此函数的任何测试用例（包括"有效端口返回 true"、"已终止 session 返回 false"、"不存在端口返回 false"等场景）。按 testing skill 要求，新增功能必须有对应测试。

修复建议：在 pptPreviewBridge.test.ts 中补充：

describe('isActivePreviewPort', () => {
  it('returns true for an active session port', async () => { ... });
  it('returns false for an aborted session port', async () => { ... });
  it('returns false for an unknown port', () => {
    expect(isActivePreviewPort(9999)).toBe(false);
  });
});

🔵 LOW — shellBridgeStandalone.ts 无测试覆盖

文件：src/process/bridge/shellBridgeStandalone.ts

问题说明：新文件没有对应的单元测试。runOpen 的三平台分支逻辑（win32/darwin/linux）和各 IPC provider 注册均未被测试。

修复建议：新建 tests/unit/shellBridgeStandalone.test.ts，mock execFile 和 ipcBridge.shell.*，验证三个平台下 runOpen 调用的命令和参数正确。

汇总

结论

⚠️ 有条件批准 — 存在 1 个 HIGH 问题（非 HTML pipe 路径的进程崩溃风险）需处理后合并；MEDIUM 超时问题和其余 LOW 问题建议同步修复。

本报告由本地 pr-review skill 生成，包含完整项目上下文，无截断限制。

PR Fix 验证报告

原始 PR: #1679
修复方式: 直接推送到 zynx/fix/server-mode-ppt-preview

总结： ✅ 已修复 5 个 | ❌ 未能修复 0 个 | ⏭️ 跳过 0 个