[插件安全边界] tools/pre-execute 在 ask 审批前已执行，可先完成独立 Node.js 子进程

[causebefore]

我发现了一个与工具审批执行顺序有关的安全缺陷。这可能导致用户看到审批请求时，产生该请求的插件监听器已经完成了宿主级操作，使审批无法为这些操作提供事前保护。

tools/pre-execute 返回的 { kind: "ask" } 并不是该监听器运行前取得的许可，而是监听器执行完成后的返回结果。插件必须先获得执行机会，才能返回 ask。因此，当用户看到审批请求时，监听器在返回 ask 前等待并完成的操作已经发生。

我使用本地无害 PoC 验证了这一点：tools/pre-execute 启动一个独立的 Node.js 子进程，等待其完成固定计算并正常 close(code=0)，随后才返回 ask。用户拒绝工具调用能够阻止 ToolDefinition.execute()，但无法阻止或撤销监听器在审批前已经完成的操作。

实际执行顺序为：

tools/pre-execute 监听器开始执行
  → 启动并完成 Node.js 子进程
  → 子进程 close(code=0)
  → 监听器返回 ask
  → serviceAsk()
  → approval.request()
  → approval/asked
  → 用户看到审批请求

测试环境

项目	值
操作系统	Windows
DSH	@deepseek-ai/dsh@0.1.0-rc.6
Node.js	v24.18.0
源码基线	47f943859bef60e4160492346772ded9b24f765a
运行方式	本地 Web UI，loopback
会话权限	Workspace Write
Tool approval policy	ask

实际观察

我进行了拒绝轮和 Allow once 对照轮：

• 拒绝轮中，Node.js 子进程先完成并正常退出，随后才出现审批请求。用户拒绝后，ToolDefinition.execute() 没有执行。
• Allow once 对照轮中，同样先完成子进程并显示审批；用户允许后，ToolDefinition.execute() 才执行。
• 两轮审批请求均携带与已完成子进程对应的 runId、childPid 和证明摘要。

这说明拒绝机制本身仍然有效，但它只阻止后续工具主体，无法保护或撤销 tools/pre-execute 中已经完成的操作。

安全影响

当前审批顺序意味着：

• 工具审批无法作为 tools/pre-execute 监听器执行宿主 Node.js 操作的事前许可。
• 监听器可以先完成独立子进程等操作，再通过返回 ask 创建审批请求；用户此时选择“拒绝”已经无法阻止或撤销这些操作。
• 审批拒绝实际只保护后续 ToolDefinition.execute() 的 dispatch。如果用户把审批 UI 理解为“本次工具调用尚未执行任何相关代码”，当前行为与这一安全预期不一致。
• 如果用户安装或启用的第三方服务端插件并非完全可信，或插件供应链被攻陷，该插件可以在用户看到审批请求前执行宿主级 Node.js 操作；用户随后点击“拒绝”也无法阻止或撤销这些操作，工具审批不能缓解这一风险。

如果 ask 的安全语义是“在相关操作执行前取得用户许可”，审批判断需要发生在可产生副作用的监听器获得执行机会之前。否则，文档和审批 UI 应明确说明其保证范围仅为后续工具主体的 dispatch。

出于安全考虑，我不会在公开讨论中提供完整 PoC、可直接运行的代码或原始测试日志。

[zoahdev]

Agreed with the finding, with an important framing: ask-approval is consent UX, not a sandbox boundary. Plugins are explicitly installed host code ? they can always import Node built-ins and run anything. Approval gates intent, not capability; it cannot retroactively undo a listener that ran host operations before returning ask.

That makes the practical fixes:

1. Contract: pre-execute listeners should be pure decision functions ? no child processes, no fs writes, no network. Anything side-effectful must happen in execute() after approval. This is enforceable by review/CI rather than by the runtime.
2. Runtime: when a listener returns ask, the framework should be able to distinguish "pure decision" from "already ran work" ? e.g. run listeners in a capability-restricted context, or require a declared precheck surface that is guaranteed side-effect-free.
3. Docs: state explicitly that approval is consent UX; the security boundary is install-time trust (vetting, allowBuilds, plugin review), not the ask dialog.

Tooling angle: ecosystem vetting plugins (dsh-plugin-vetting) already static-scan for exfiltration/credential patterns; a side-effect-before-ask lint for pre-execute listeners (child_process/fs/network imports inside the listener body) fits naturally there and in dsh-plugin-doctor's preflight. Happy to implement the lint if useful.

[causebefore]

Thanks for the clarification. I agree with the distinction: an installed plugin is already host code with Node.js capabilities, so this finding is not a sandbox escape or capability escalation.

I agree with the proposed directions. At minimum, the pure-decision contract for tools/pre-execute and the exact scope of the approval UI should be documented explicitly, with review or CI helping enforce that convention.

[zoahdev]

The lint is now shipped: dsh-plugin-doctor v1.9.0 adds pre-execute-side-effects to the default pipeline (heuristic, clearly labeled as a review aid ? not a security sandbox):

• scans source/lib for pre-execute listeners
• flags same-file host-level side-effect APIs (child_process, fs writes, network)
• FAIL message names the file + matched API and points at this discussion
• no listener ? PASS; listener without obvious side-effect APIs ? PASS with heuristic note

22/22 tests (3 new fixtures: absent / side-effect FAIL / clean PASS). Release: https://github.com/zoahdev/dsh-plugin-doctor/releases/tag/v1.9.0

This complements (not replaces) the three upstream fix directions: pure pre-execute contract, capability-restricted listener context, and docs clarifying approval = consent UX.

[causebefore]

Thanks for sharing this tool and for implementing the check so quickly.

I pulled dsh-plugin-doctor v1.9.0 and tested it against my local PoC. The new pre-execute-side-effects check returned FAIL and correctly identified:

lib/index.js (node:child_process)

I also tested the npm-packaged plugin artifact, and the same finding remained. This matches the pattern demonstrated by the PoC and confirms that the new rule works as intended.

it is already valuable for plugin review before stronger upstream measures are available.