fix(security): 2 improvements across 2 files#1412
Conversation
- Security: Path Traversal in Plugin Node Entry Execution - Security: Arbitrary File Write via Feedback Archive Path Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
- Security: Path Traversal in Plugin Node Entry Execution - Security: Arbitrary File Write via Feedback Archive Path Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45d46c100f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const ext = path.extname(entryReal).toLowerCase(); | ||
| if (!ALLOWED_EXTENSIONS.includes(ext)) { | ||
| throw new Error(`Plugin node entry must have a valid extension (${ALLOWED_EXTENSIONS.join(', ')}): ${entry}`); |
There was a problem hiding this comment.
When the native binary rewrites plugin MCP servers with command: "node", it passes the manifest's original args directly to __plugin_run_node (packages/agent-core/src/plugin/manager.ts:501-506), so this helper needs to accept the same script paths users can pass to Node. A valid plugin config such as args: ["./server"] or a direct package bin without a .js/.mjs/.cjs suffix previously ran from the plugin root, but now path.extname(entryReal) is "" and the MCP server fails before import. Please allow extensionless in-root scripts or otherwise emulate Node's accepted entrypoint behavior.
Useful? React with 👍 / 👎.
1 participant
Summary
fix(security): 2 improvements across 2 files
Problem
Severity:
High| File:apps/kimi-code/src/cli/sub/plugin-run-node.ts:L5The
runPluginNodeEntryfunction inplugin-run-node.tsattempts to validate that an plugin entry point is within theKIMI_PLUGIN_ROOTdirectory usingisWithin(). However, the validation is performed on resolved real paths after callingrealpath()on both the entry and root. While this prevents some symlink-based traversal, theisWithinfunction usespath.relative()which can be bypassed on case-insensitive filesystems (Windows, macOS) or with certain Unicode normalization attacks. More critically, the function then directly imports the entry file viaimport(pathToFileURL(entryReal).href)with no additional sandboxing, allowing arbitrary code execution within the Node.js process. An attacker who can control theentryparameter or influenceKIMI_PLUGIN_ROOTcould execute arbitrary code.Solution
Add additional validation: (1) ensure the resolved entry path ends with expected extensions (.js, .mjs, .cjs), (2) add a second check after
realpaththat the entry is still within root using a more robust check likeentryReal.startsWith(rootReal + path.sep), (3) consider running plugin entries in a worker thread or subprocess with limited privileges, (4) validate thatKIMI_PLUGIN_ROOTis not writable by untrusted users.Changes
apps/kimi-code/src/cli/sub/plugin-run-node.ts(modified)apps/kimi-code/src/feedback/archive.ts(modified)