Merge Pisces Kernel variant#2
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the project to TypeScript, changes the license to BSD 3-Clause, and implements a Discord bot using a LangGraph-powered ReAct agent. It introduces a command registration system, persistent storage via LowDB, and AI integration with OpenAI and Tavily. Feedback identifies a critical security vulnerability in the shell execution command, a memory leak in the timeout utility, and suggests optimizations for agent initialization and input validation.
| export const runShellCommand: CommandConfig = { | ||
| description: "Execute a shell command and return the result", | ||
| options: [ | ||
| { | ||
| name: "command", | ||
| description: "Shell command to execute", | ||
| type: 3, // STRING | ||
| required: true, | ||
| }, | ||
| ], | ||
| action: async (interaction: ChatInputCommandInteraction): Promise<void> => { | ||
| const command = interaction.options.getString("command", true); | ||
|
|
||
| // Security check: verify the executor's identity | ||
| const botOwnerId = process.env.DISCORD_BOT_OWNER_ID as Snowflake; | ||
| if (interaction.user.id !== botOwnerId) { | ||
| await interaction.reply({ | ||
| content: "You do not have permission to execute this command.", | ||
| ephemeral: true, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const execAsync = promisify(exec); | ||
|
|
||
| await interaction.deferReply(); | ||
| const {stdout, stderr} = | ||
| await execAsync(command, { | ||
| timeout: 10000, | ||
| maxBuffer: 1024 * 1024, | ||
| }); | ||
|
|
||
| await interaction.editReply("Execution complete"); | ||
|
|
||
| if (stdout) { | ||
| const partialStdout = sliceContent(stdout, 1990); | ||
| for (const snippet of partialStdout) { | ||
| const message = `Standard Output:\n\`\`\`\n${snippet}\n\`\`\`\n`; | ||
| await interaction.followUp(message); | ||
| } | ||
| } | ||
| if (stderr) { | ||
| const partialStderr = sliceContent(stderr, 1990); | ||
| for (const snippet of partialStderr) { | ||
| const message = `Error Output:\n\`\`\`\n${snippet}\n\`\`\`\n`; | ||
| await interaction.followUp(message); | ||
| } | ||
| } | ||
| } catch (error) { | ||
| const content = (error as Error).message; | ||
| const message = `Execution failed:\n\`\`\`\n${content}\n\`\`\``; | ||
| await interaction.editReply(message); | ||
| } | ||
| }, | ||
| }; |
There was a problem hiding this comment.
This command allows for arbitrary shell execution on the host machine. Even with the bot owner check, this is a critical security risk (Remote Code Execution). If the bot token or the owner's Discord account is compromised, the entire server is exposed. It is strongly recommended to remove this command or replace it with a strictly whitelisted set of safe operations.
| function withTimeout<T>(promise: Promise<T>, ms: number, label: string) { | ||
| return Promise.race([ | ||
| promise, | ||
| new Promise<never>((_, reject) => | ||
| setTimeout(() => reject(new Error(`${label} timed out`)), ms), | ||
| ), | ||
| ]); | ||
| } |
There was a problem hiding this comment.
The withTimeout function creates a setTimeout that is never cleared if the main promise resolves first. This results in a memory leak and keeps the event loop active longer than necessary. Consider using clearTimeout in a .finally() block to ensure resources are released.
function withTimeout<T>(promise: Promise<T>, ms: number, label: string) {
let timeoutId: ReturnType<typeof setTimeout>;
const timeoutPromise = new Promise<never>((_, reject) => {
timeoutId = setTimeout(() => reject(new Error(`${label} timed out`)), ms);
});
return Promise.race([promise, timeoutPromise]).finally(() => clearTimeout(timeoutId));
}| const agent = createReActAgent({ | ||
| model: resolvedChatModel, | ||
| apiKey: resolvedApiKey, | ||
| baseURL: baseUrl, | ||
| temperature, | ||
| systemPrompt: avatarSettingsContent, | ||
| }); |
There was a problem hiding this comment.
The ReAct agent is being recreated on every call to chatWithAI. Since the configuration (model, API key, system prompt) is static for the lifetime of the application, it is much more efficient to initialize the agent once at the module level or use a memoization pattern to avoid unnecessary overhead.
| @@ -0,0 +1,225 @@ | |||
| import {REST} from "@discordjs/rest"; | |||
| import {Routes} from "discord-api-types/v9"; | |||
There was a problem hiding this comment.
The project is using discord.js v14, which targets Discord API v10. Importing Routes from discord-api-types/v9 may lead to compatibility issues or missing features. It is recommended to use the version that matches the library's expectations.
| import {Routes} from "discord-api-types/v9"; | |
| import {Routes} from "discord-api-types/v10"; |
|
|
||
| try { | ||
| const args = argsStr.split(/\s+/).filter(Boolean); | ||
| const env = JSON.parse(envJson); |
There was a problem hiding this comment.
JSON.parse on user-provided input should be validated to ensure it returns an object. If a user provides a JSON array or a primitive, it might cause unexpected behavior in the MCP manager or when merging configurations.
const env = JSON.parse(envJson);
if (typeof env !== "object" || env === null || Array.isArray(env)) {
throw new Error("env_json must be a valid JSON object");
}1 participant
No description provided.