feat：load js script plugin - #1197
Conversation
|
woc |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
你好——我发现了 2 个问题
面向 AI Agent 的提示
请处理本次代码审查中的评论：
## 单独评论
### 评论 1
<location path="src/Economics.Script/Sources/FileScriptSourceProvider.cs" line_range="32-33" />
<code_context>
+ return false;
+ }
+
+ var candidate = Path.GetFullPath(Path.Combine(this._rootFull, key));
+ if (!candidate.StartsWith(this._rootFull, StringComparison.OrdinalIgnoreCase))
+ {
+ return false;
</code_context>
<issue_to_address>
**🚨 issue (security):** 路径包含性检查会接受名称仅与根路径前缀相同的同级路径，因此像 `/server/SkillScripts` 这样的根路径也会接受 `/server/SkillScriptsBackup/script.js`。因此，技能配置中如果包含一个解析到此类同级文件的路径，就会加载并执行配置目录之外的脚本。
**触发条件：** 当脚本键解析到的目录路径以配置的根路径字符串开头，但实际上并不位于该根路径下时。
**建议修复：** 要求候选路径等于根路径，或以根路径加平台目录分隔符开头；也可以在规范化后比较相对路径。
</issue_to_address>
### 评论 2
<location path="src/Economics.Script/ScriptRuntime.cs" line_range="74-77" />
<code_context>
+ /// 调用指定入口函数（用于可选的事件钩子）。若入口函数不存在、不可调用、未加载或调用失败，
+ /// 返回 <c>false</c> 而不会抛出/刷屏。成功时返回 <c>true</c>。
+ /// </summary>
+ public bool TryInvoke(string entryFunction, params object?[] args)
+ {
+ lock (this._sync)
+ {
+ this.EnsureBuilt();
+ if (!this._loaded || this._engine is null)
+ {
+ return false;
+ }
+
</code_context>
<issue_to_address>
**nitpick:** `TryInvoke` 的文档承诺调用失败时不会输出错误，但两个实现都会在发生异常时调用配置的 `ErrorHandler`；插件处理程序会将这些异常记录到控制台。因此，尽管文档如此约定，失败的可选事件回调实际上仍会产生错误输出。
**触发条件：** 当可选脚本事件函数抛出异常，且宿主配置了错误处理器时。
**建议修复：** 要么更新文档，说明会调用配置的错误处理器；要么针对静默的 `TryInvoke` 约定单独抑制或记录错误。
```suggestion
/// <summary>
/// 调用指定入口函数（用于可选的事件钩子）。若入口函数不存在、不可调用、未加载或调用失败，
/// 返回 <c>false</c> 而不会抛出；调用失败时会调用配置的错误处理器。成功时返回 <c>true</c>。
/// </summary>
```
</issue_to_address>帮我变得更有用！请在每条评论上点击 👍 或 👎，我会利用反馈来改进审查结果。
Original comment in English
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/Economics.Script/Sources/FileScriptSourceProvider.cs" line_range="32-33" />
<code_context>
+ return false;
+ }
+
+ var candidate = Path.GetFullPath(Path.Combine(this._rootFull, key));
+ if (!candidate.StartsWith(this._rootFull, StringComparison.OrdinalIgnoreCase))
+ {
+ return false;
</code_context>
<issue_to_address>
**🚨 issue (security):** The path-containment check accepts sibling paths whose names merely share the root prefix, so a root such as `/server/SkillScripts` also accepts `/server/SkillScriptsBackup/script.js`. A skill configuration containing a path that resolves to such a sibling file therefore loads and executes a script outside the configured directory.
**Triggers:** When a script key resolves to a directory whose path starts with the configured root string but is not below that root.
**Suggested fix:** Require the candidate to equal the root or start with the root followed by the platform directory separator, or compare relative paths after canonicalization.
</issue_to_address>
### Comment 2
<location path="src/Economics.Script/ScriptRuntime.cs" line_range="74-77" />
<code_context>
+ /// 调用指定入口函数（用于可选的事件钩子）。若入口函数不存在、不可调用、未加载或调用失败，
+ /// 返回 <c>false</c> 而不会抛出/刷屏。成功时返回 <c>true</c>。
+ /// </summary>
+ public bool TryInvoke(string entryFunction, params object?[] args)
+ {
+ lock (this._sync)
+ {
+ this.EnsureBuilt();
+ if (!this._loaded || this._engine is null)
+ {
+ return false;
+ }
+
</code_context>
<issue_to_address>
**nitpick:** The `TryInvoke` documentation promises that failed calls do not print errors, but both implementations invoke the configured `ErrorHandler` on exceptions; the plugin handlers log those exceptions to the console. Failed optional event callbacks therefore do produce error output despite the documented contract.
**Triggers:** When an optional script event function throws and the host configured an error handler.
**Suggested fix:** Either update the documentation to state that the configured error handler is called, or suppress/log separately for the quiet `TryInvoke` contract.
```suggestion
/// <summary>
/// 调用指定入口函数（用于可选的事件钩子）。若入口函数不存在、不可调用、未加载或调用失败，
/// 返回 <c>false</c> 而不会抛出；调用失败时会调用配置的错误处理器。成功时返回 <c>true</c>。
/// </summary>
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| var candidate = Path.GetFullPath(Path.Combine(this._rootFull, key)); | ||
| if (!candidate.StartsWith(this._rootFull, StringComparison.OrdinalIgnoreCase)) |
There was a problem hiding this comment.
🚨 issue (security): 路径包含性检查会接受名称仅与根路径前缀相同的同级路径，因此像 /server/SkillScripts 这样的根路径也会接受 /server/SkillScriptsBackup/script.js。因此，技能配置中如果包含一个解析到此类同级文件的路径，就会加载并执行配置目录之外的脚本。
触发条件： 当脚本键解析到的目录路径以配置的根路径字符串开头，但实际上并不位于该根路径下时。
建议修复： 要求候选路径等于根路径，或以根路径加平台目录分隔符开头；也可以在规范化后比较相对路径。
Original comment in English
🚨 issue (security): The path-containment check accepts sibling paths whose names merely share the root prefix, so a root such as /server/SkillScripts also accepts /server/SkillScriptsBackup/script.js. A skill configuration containing a path that resolves to such a sibling file therefore loads and executes a script outside the configured directory.
Triggers: When a script key resolves to a directory whose path starts with the configured root string but is not below that root.
Suggested fix: Require the candidate to equal the root or start with the root followed by the platform directory separator, or compare relative paths after canonicalization.
| /// <summary> | ||
| /// 调用指定入口函数（用于可选的事件钩子）。若入口函数不存在、不可调用、未加载或调用失败， | ||
| /// 返回 <c>false</c> 而不会抛出/刷屏。成功时返回 <c>true</c>。 | ||
| /// </summary> |
There was a problem hiding this comment.
nitpick: TryInvoke 的文档承诺调用失败时不会输出错误，但两个实现都会在发生异常时调用配置的 ErrorHandler；插件处理程序会将这些异常记录到控制台。因此，尽管文档如此约定，失败的可选事件回调实际上仍会产生错误输出。
触发条件： 当可选脚本事件函数抛出异常，且宿主配置了错误处理器时。
建议修复： 要么更新文档，说明会调用配置的错误处理器；要么针对静默的 TryInvoke 约定单独抑制或记录错误。
| /// <summary> | |
| /// 调用指定入口函数（用于可选的事件钩子）。若入口函数不存在、不可调用、未加载或调用失败， | |
| /// 返回 <c>false</c> 而不会抛出/刷屏。成功时返回 <c>true</c>。 | |
| /// </summary> | |
| /// <summary> | |
| /// 调用指定入口函数（用于可选的事件钩子）。若入口函数不存在、不可调用、未加载或调用失败， | |
| /// 返回 <c>false</c> 而不会抛出；调用失败时会调用配置的错误处理器。成功时返回 <c>true</c>。 | |
| /// </summary> |
Original comment in English
nitpick: The TryInvoke documentation promises that failed calls do not print errors, but both implementations invoke the configured ErrorHandler on exceptions; the plugin handlers log those exceptions to the console. Failed optional event callbacks therefore do produce error output despite the documented contract.
Triggers: When an optional script event function throws and the host configured an error handler.
Suggested fix: Either update the documentation to state that the configured error handler is called, or suppress/log separately for the quiet TryInvoke contract.
| /// <summary> | |
| /// 调用指定入口函数（用于可选的事件钩子）。若入口函数不存在、不可调用、未加载或调用失败， | |
| /// 返回 <c>false</c> 而不会抛出/刷屏。成功时返回 <c>true</c>。 | |
| /// </summary> | |
| /// <summary> | |
| /// 调用指定入口函数（用于可选的事件钩子）。若入口函数不存在、不可调用、未加载或调用失败， | |
| /// 返回 <c>false</c> 而不会抛出；调用失败时会调用配置的错误处理器。成功时返回 <c>true</c>。 | |
| /// </summary> |
1 participant
添加插件
更新插件/修复BUG
其他
Sourcery 摘要
引入共享的 JavaScript 脚本基础设施、独立脚本插件、可配置的 NPC AI，以及跨 Economics 插件的基于占位符的状态显示功能。
新功能：
错误修复：
增强功能：
构建：
文档：
杂项：
Original summary in English
Summary by Sourcery
Introduce shared JavaScript scripting infrastructure, a standalone script plugin, configurable NPC AI, and placeholder-driven status displays across the Economics plugins.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Chores: