You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Add Python interpreter plugin with code execution capabilities
Implement agent code script management system
Enhance instruct service with code template execution
Add realtime options support for audio format configuration
Diagram Walkthrough
flowchart LR
A["User Request"] --> B["InstructService"]
B --> C["Code Template Check"]
C --> D["PyInterpretService"]
D --> E["Python Code Execution"]
E --> F["Result Output"]
B --> G["LLM Completion"]
G --> F
H["Agent Code Scripts"] --> C
I["Repository Layer"] --> H
Loading
File Walkthrough
Relevant files
Enhancement
18 files
PythonInterpreterPlugin.cs
Main plugin registration and Python engine initialization
Here are some key observations to aid the review process:
⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 Security concerns
Code execution sandboxing: The Python interpreter executes arbitrary LLM-generated code (PyProgrammerFn and PyInterpretService) with full process permissions and access to sys modules. There are no resource limits, filesystem/network restrictions, or timeouts. This could allow remote code execution impacting the host. Consider sandboxing (separate process/container, restricted working directory, timeout, memory/CPU limits), validating/whitelisting imports, and disabling dangerous modules. Also, pip install via PyPackageHelper can fetch arbitrary packages at runtime without verification, which may introduce supply-chain risks.
Missing consideration for installing/importing required Python packages returned by the model; ret.ImportedPackages is parsed but never used. Generated code that relies on external packages may fail at runtime without pre-install checks or sandboxing.
varresponse=awaitGetChatCompletion(innerAgent,dialogs);varret=response.JsonContent<LlmContextOut>();try{using(Py.GIL()){// Import necessary Python modulesdynamicsys=Py.Import("sys");dynamicio=Py.Import("io");// Redirect standard output/error to capture itdynamicstringIO=io.StringIO();sys.stdout=stringIO;sys.stderr=stringIO;// Set global itemsusingvarglobals=newPyDict();if(ret.PythonCode?.Contains("__main__")==true){globals.SetItem("__name__",newPyString("__main__"));}// Execute Python scriptPythonEngine.Exec(ret.PythonCode,globals);// Get resultvarresult=stringIO.getvalue()?.ToString()asstring;message.Content=result?.TrimEnd('\r','\n')??string.Empty;message.RichContent=newRichContent<IRichMessage>{Recipient=newRecipient{Id=convService.ConversationId},Message=newProgramCodeTemplateMessage{Text=ret.PythonCode??string.Empty,Language="python"}};message.StopCompletion=true;// Restore the original stdout/stderrsys.stdout=sys.__stdout__;sys.stderr=sys.__stderr__;}}catch(Exceptionex){varerrorMsg=$"Error when executing python code.";message.Content=$"{errorMsg}{ex.Message}";_logger.LogError(ex,errorMsg);}
In GetAgentCodeScripts, the condition if (scriptNames != null || !scriptNames.Contains(fileName)) will skip all files whenever scriptNames is not null. Likely intended to be && to filter only when a non-null list is provided.
Python runtime is initialized and thread state stored, but shutdown relies on ApplicationStopping only. If initialization partially fails or multiple initializations occur across scopes, ensure idempotency and guard against double shutdown; also validate InstallLocation path to a DLL vs directory on non-Windows.
publicvoidConfigure(IApplicationBuilderapp){varsp=app.ApplicationServices;varsettings=sp.GetRequiredService<PythonInterpreterSettings>();varlogger=sp.GetRequiredService<ILogger<PyProgrammerFn>>();varpyLoc=settings.InstallLocation;try{if(File.Exists(pyLoc)){Runtime.PythonDLL=pyLoc;PythonEngine.Initialize();_pyState=PythonEngine.BeginAllowThreads();varlifetime=app.ApplicationServices.GetRequiredService<IHostApplicationLifetime>();lifetime.ApplicationStopping.Register(()=>{PythonEngine.EndAllowThreads(_pyState);PythonEngine.Shutdown();});}else{logger.LogError($"Python dll not found at {pyLoc}");}}catch(Exceptionex){logger.LogError(ex,$"Error when loading python dll {pyLoc}");}}
The current direct execution of LLM-generated Python code is a major security risk. To fix this, implement a sandboxed environment using tools like Docker to isolate the code execution and restrict its access to system resources.
// In PyInterpretService.cs and PyProgrammerFn.cspublicclassPyInterpretService{publicasyncTask<CodeInterpretResult>RunCode(stringcodeScript, ...){try{using(Py.GIL()){// ... setup environment// Direct execution of arbitrary code in the same processPythonEngine.Exec(codeScript,globals);// ... capture and return result}}catch(Exceptionex){/* ... */}}}
After:
// A new sandboxed execution servicepublicclassSandboxedPythonExecutor{publicasyncTask<ExecutionResult>Execute(stringcode){// 1. Spin up an isolated environment (e.g., Docker container).// 2. Set strict resource limits (no network, read-only filesystem).// 3. Execute the code inside the isolated environment.// 4. Capture stdout/stderr.// 5. Tear down the environment.// 6. Return the result.varresult=awaitRunInContainerAsync(code);returnresult;}}// In PyInterpretService.cspublicasyncTask<CodeInterpretResult>RunCode(stringcodeScript, ...){varsandboxedExecutor=_services.GetRequiredService<SandboxedPythonExecutor>();varresult=awaitsandboxedExecutor.Execute(codeScript);// ... return result}
Suggestion importance[1-10]: 10
__
Why: This suggestion addresses a critical security vulnerability by pointing out that executing LLM-generated code directly is unsafe and proposes a robust, standard solution.
High
Possible issue
Ensure agent-specific data deletion
Fix a critical bug in DeleteAgentCodeScripts by adding a filter for agentId to prevent deleting code scripts from unintended agents.
public bool DeleteAgentCodeScripts(string agentId, List<string>? scriptNames)
{
if (string.IsNullOrWhiteSpace(agentId))
{
return false;
}
- var filterDef = Builders<AgentCodeDocument>.Filter.Empty;+ var builder = Builders<AgentCodeDocument>.Filter;+ var filterDef = builder.Eq(x => x.AgentId, agentId);+
if (scriptNames != null)
{
- var builder = Builders<AgentCodeDocument>.Filter;- var filters = new List<FilterDefinition<AgentCodeDocument>>- {- builder.In(x => x.Name, scriptNames)- };- filterDef = builder.And(filters);+ var scriptFilter = builder.In(x => x.Name, scriptNames);+ filterDef = builder.And(filterDef, scriptFilter);
}
var deleted = _dc.AgentCodes.DeleteMany(filterDef);
return deleted.DeletedCount > 0;
}
Apply / Chat
Suggestion importance[1-10]: 10
__
Why: This suggestion points out a critical data-loss bug where agentId is ignored during deletion, potentially deleting scripts from all agents. The fix is crucial for data integrity.
High
Fix incorrect filtering logic
Fix the incorrect filtering logic in GetAgentCodeScripts by changing the || operator to && to correctly filter files when scriptNames is provided.
var fileName = Path.GetFileName(file);
- if (scriptNames != null || !scriptNames.Contains(fileName))+ if (scriptNames != null && !scriptNames.Contains(fileName))
{
continue;
}
Apply / Chat
Suggestion importance[1-10]: 9
__
Why: The suggestion correctly identifies a critical bug where the filtering logic is flawed, causing the method to either return an empty list or throw a NullReferenceException.
High
Ensure Python streams are always restored
Move the restoration logic for Python's sys.stdout and sys.stderr into a finally block to ensure they are always restored, even if an exception occurs.
var result = stringIO.getvalue()?.ToString() as string;
message.Content = result?.TrimEnd('\r', '\n') ?? string.Empty;
message.RichContent = new RichContent<IRichMessage>
{
Recipient = new Recipient { Id = convService.ConversationId },
Message = new ProgramCodeTemplateMessage
{
Text = ret.PythonCode ?? string.Empty,
Language = "python"
}
};
message.StopCompletion = true;
+ }+ finally+ {+ using (Py.GIL())+ {+ // Restore the original stdout/stderr+ dynamic sys = Py.Import("sys");+ sys.stdout = sys.__stdout__;+ sys.stderr = sys.__stderr__;+ }+ }- // Restore the original stdout/stderr- sys.stdout = sys.__stdout__;- sys.stderr = sys.__stderr__;-
[To ensure code accuracy, apply this suggestion manually]
Suggestion importance[1-10]: 8
__
Why: The suggestion correctly identifies a potential issue where Python's standard streams are not restored on error and provides a robust fix using a finally block, improving application stability.
Medium
Learned best practice
Remove throwing default implementation
Remove the throwing default implementation and require implementers to provide a concrete method, or provide a safe base class if needed.
Below is a summary of compliance checks for this PR:
Security Compliance
🔴
Arbitrary code execution
Description: Arbitrary Python code execution via Python.NET (PythonEngine.Exec) without sandboxing, resource limits, or process isolation allows code injection to access filesystem, network, or host process memory. PyInterpretService.cs [22-90]
Referred Code
publicasyncTask<CodeInterpretResult>RunCode(stringcodeScript,CodeInterpretOptions?options=null){try{using(Py.GIL()){// Import necessary Python modulesdynamicsys=Py.Import("sys");dynamicio=Py.Import("io");// Redirect standard output/error to capture itdynamicstringIO=io.StringIO();sys.stdout=stringIO;sys.stderr=stringIO;// Set global itemsusingvarglobals=newPyDict();if(codeScript.Contains("__main__")==true){globals.SetItem("__name__",newPyString("__main__"));}
...(clipped 48lines)
LLM code execution risk
Description: Executes LLM-generated Python code directly inside app process (PythonEngine.Exec) capturing stdout with no isolation or validation, enabling remote code execution if LLM is manipulated. PyProgrammerFn.cs [67-116]
Referred Code
try{using(Py.GIL()){// Import necessary Python modulesdynamicsys=Py.Import("sys");dynamicio=Py.Import("io");// Redirect standard output/error to capture itdynamicstringIO=io.StringIO();sys.stdout=stringIO;sys.stderr=stringIO;// Set global itemsusingvarglobals=newPyDict();if(ret.PythonCode?.Contains("__main__")==true){globals.SetItem("__name__",newPyString("__main__"));}// Execute Python script
...(clipped 29lines)
⚪
Supply-chain risk
Description: Invokes pip install with unvalidated package names from inputs which may allow dependency confusion or supply-chain compromise and executes external process without strict path or allowlist. PyPackageHelper.cs [21-69]
Referred Code
{varpackageList=string.Join(" ",packages);varstartInfo=newProcessStartInfo{FileName="pip",Arguments=$"install {packageList}",RedirectStandardOutput=true,RedirectStandardError=true,UseShellExecute=false,CreateNoWindow=true};usingvarprocess=Process.Start(startInfo);if(process==null){returnnewPackageInstallResult{Success=false,ErrorMsg="Failed to start pip process"};}
...(clipped 28lines)
Untrusted library load
Description: Loads Python runtime DLL path from configuration without validation; incorrect or tampered path could lead to loading untrusted native library (DLL preloading risk). PythonInterpreterPlugin.cs [35-58]
Referred Code
try{if(File.Exists(pyLoc)){Runtime.PythonDLL=pyLoc;PythonEngine.Initialize();_pyState=PythonEngine.BeginAllowThreads();varlifetime=app.ApplicationServices.GetRequiredService<IHostApplicationLifetime>();lifetime.ApplicationStopping.Register(()=>{PythonEngine.EndAllowThreads(_pyState);PythonEngine.Shutdown();});}else{logger.LogError($"Python dll not found at {pyLoc}");}}catch(Exceptionex){
...(clipped 3lines)
Ticket Compliance
⚪
🎫 No ticket provided
Create ticket/issue
Codebase Duplication Compliance
⚪
Codebase context is not defined
Follow the guide to enable codebase context checks.
Custom Compliance
⚪
No custom compliance provided
Follow the guide to enable custom compliance check.
Compliance status legend
🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label
The current implementation of Python code execution is insecure because it runs within the main application process. It should be moved to a sandboxed environment, like a Docker container, to prevent potential server compromise.
// In PyInterpretService.cspublicasyncTask<CodeInterpretResult>RunCode(stringcodeScript, ...){try{using(Py.GIL()){// ... setup stdout/stderr redirection ...// Execute Python script directly in the current processPythonEngine.Exec(codeScript,globals);// ... get result and restore stdout/stderr ...returnnewCodeInterpretResult{Success=true, ...};}}catch(Exceptionex){ ...}}
After:
// In a new SandboxedPyInterpretService.cspublicasyncTask<CodeInterpretResult>RunCode(stringcodeScript, ...){try{// 1. Create a new isolated environment (e.g., Docker container)varcontainerId=await_sandboxManager.CreateContainerAsync("python:3.11-slim");// 2. Execute the script inside the isolated environmentvarexecResult=await_sandboxManager.ExecuteInContainerAsync(containerId,codeScript);// 3. Clean up the environmentawait_sandboxManager.RemoveContainerAsync(containerId);// 4. Return the result from the sandboxed executionreturnnewCodeInterpretResult{Result=execResult.StdOut,Success=execResult.ExitCode==0};}catch(Exceptionex){ ...}}
Suggestion importance[1-10]: 10
__
Why: The suggestion correctly identifies a critical security vulnerability by executing LLM-generated Python code in-process, which could lead to server compromise, and proposes the correct mitigation strategy.
High
Possible issue
Prevent unintended deletion of agent scripts
Fix a critical bug in DeleteAgentCodeScripts where passing null for scripts would delete all scripts from the database instead of just those for the specified agent.
public bool DeleteAgentCodeScripts(string agentId, List<AgentCodeScript>? scripts = null)
{
if (string.IsNullOrWhiteSpace(agentId))
{
return false;
}
++ var builder = Builders<AgentCodeScriptDocument>.Filter;+ var agentFilter = builder.Eq(x => x.AgentId, agentId);
DeleteResult deleted;
if (scripts != null)
{
var scriptPaths = scripts.Select(x => x.CodePath);
var exprFilter = new BsonDocument("$expr", new BsonDocument("$in", new BsonArray
{
new BsonDocument("$concat", new BsonArray { "$ScriptType", "/", "$Name" }),
new BsonArray(scriptPaths)
}));
var filterDef = new BsonDocumentFilterDefinition<AgentCodeScriptDocument>(exprFilter);
- deleted = _dc.AgentCodeScripts.DeleteMany(filterDef);+ deleted = _dc.AgentCodeScripts.DeleteMany(builder.And(agentFilter, filterDef));
}
else
{
- deleted = _dc.AgentCodeScripts.DeleteMany(Builders<AgentCodeScriptDocument>.Filter.Empty);+ deleted = _dc.AgentCodeScripts.DeleteMany(agentFilter);
}
return deleted.DeletedCount > 0;
}
[To ensure code accuracy, apply this suggestion manually]
Suggestion importance[1-10]: 10
__
Why: This suggestion identifies a critical bug where deleting all scripts for one agent would delete all scripts for all agents, leading to major data loss.
High
Implement empty method to prevent data loss
Implement the empty BulkInsertAgentTasks method to iterate through tasks and save them individually, preventing data loss.
public void BulkInsertAgentTasks(string agentId, List<AgentTask> tasks)
{
-+ if (tasks.IsNullOrEmpty()) return;++ foreach (var task in tasks)+ {+ InsertAgentTask(task);+ }
}
Apply / Chat
Suggestion importance[1-10]: 8
__
Why: The suggestion correctly identifies an empty method body for BulkInsertAgentTasks which was introduced in the PR, and its implementation is crucial to prevent data loss.
Medium
Fix bulk insert logic for code scripts
Fix the BulkInsertAgentCodeScripts method to correctly handle file and directory creation for new scripts, instead of incorrectly reusing the update logic.
public bool BulkInsertAgentCodeScripts(string agentId, List<AgentCodeScript> scripts)
{
- return UpdateAgentCodeScripts(agentId, scripts);+ if (string.IsNullOrWhiteSpace(agentId) || scripts.IsNullOrEmpty())+ {+ return false;+ }++ foreach (var script in scripts)+ {+ if (string.IsNullOrWhiteSpace(script.Name) || string.IsNullOrWhiteSpace(script.ScriptType))+ {+ continue;+ }++ var dir = BuildAgentCodeScriptDir(agentId, script.ScriptType);+ if (!Directory.Exists(dir))+ {+ Directory.CreateDirectory(dir);+ }++ var file = Path.Combine(dir, script.Name);+ File.WriteAllText(file, script.Content);+ }++ return true;
}
Apply / Chat
Suggestion importance[1-10]: 8
__
Why: The suggestion correctly identifies a bug where BulkInsertAgentCodeScripts reuses UpdateAgentCodeScripts, which doesn't create new directories, causing insertions to fail silently.
Medium
General
Improve Python exception handling and logging
Enhance exception handling for Python code execution by specifically catching PythonException to log the full Python traceback for better debugging.
+ catch (PythonException pyEx)+ {+ var errorMsg = $"An error occurred while executing the Python code.";+ using (Py.GIL())+ {+ dynamic traceback = Py.Import("traceback");+ string pyTraceback = traceback.format_exc();+ message.Content = $"{errorMsg}\n\nTraceback:\n{pyTraceback}";+ _logger.LogError(pyEx, $"{errorMsg}\n{pyTraceback}");+ }+ }
catch (Exception ex)
{
var errorMsg = $"Error when executing python code.";
message.Content = $"{errorMsg} {ex.Message}";
_logger.LogError(ex, errorMsg);
}
[To ensure code accuracy, apply this suggestion manually]
Suggestion importance[1-10]: 6
__
Why: This suggestion improves error handling by catching a more specific PythonException and logging the full traceback, which significantly aids in debugging Python code execution failures.
Low
Ensure logging for missing interpreter
Remove the #if DEBUG condition for logging a missing code interpreter to ensure warnings are always logged, aiding diagnostics in all build configurations.
Why: The suggestion correctly points out that logging a missing code interpreter only in DEBUG mode can hide configuration issues in RELEASE builds, and recommends unconditional logging.
Low
Learned best practice
Validate script input and defaults
Validate inputs and return safe defaults: check codeScript for null/empty before execution and avoid calling .Contains on a potentially null string.
public async Task<CodeInterpretResult> RunCode(string codeScript, CodeInterpretOptions? options = null)
{
+ if (string.IsNullOrWhiteSpace(codeScript))+ {+ return new CodeInterpretResult+ {+ Result = string.Empty,+ Success = false,+ ErrorMsg = "Empty code script."+ };+ }+
try
{
using (Py.GIL())
{
- ...- if (codeScript.Contains("__main__") == true)+ dynamic sys = Py.Import("sys");+ dynamic io = Py.Import("io");++ dynamic stringIO = io.StringIO();+ sys.stdout = stringIO;+ sys.stderr = stringIO;++ using var globals = new PyDict();+ if (!string.IsNullOrEmpty(codeScript) && codeScript.Contains("__main__"))
{
globals.SetItem("__name__", new PyString("__main__"));
}
- ...++ var list = new PyList();+ if (options?.Arguments?.Any() == true)+ {+ list.Append(new PyString("code.py"));+ foreach (var arg in options.Arguments)+ {+ if (!string.IsNullOrWhiteSpace(arg.Key) && !string.IsNullOrWhiteSpace(arg.Value))+ {+ list.Append(new PyString($"--{arg.Key}"));+ list.Append(new PyString($"{arg.Value}"));+ }+ }+ }+ sys.argv = list;+
PythonEngine.Exec(codeScript, globals);
- ...++ var result = stringIO.getvalue()?.ToString() as string;++ sys.stdout = sys.__stdout__;+ sys.stderr = sys.__stderr__;+ sys.argv = new PyList();+
return new CodeInterpretResult
{
- Result = result?.TrimEnd('\r', '\n'),+ Result = (result ?? string.Empty).TrimEnd('\r', '\n'),
Success = true
};
}
}
catch (Exception ex)
{
var errorMsg = $"Error when executing python code in {nameof(PyInterpretService)}: {Provider}. {ex.Message}";
_logger.LogError(ex, errorMsg);
return new CodeInterpretResult
{
+ Result = string.Empty,
Success = false,
ErrorMsg = errorMsg
};
}
}
[To ensure code accuracy, apply this suggestion manually]
Suggestion importance[1-10]: 6
__
Why:
Relevant best practice - Validate external/system state and asynchronous usage to avoid deadlocks and invalid blocking calls; also ensure null-safe defaults.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Type
Enhancement
Description
Add Python interpreter plugin with code execution capabilities
Implement agent code script management system
Enhance instruct service with code template execution
Add realtime options support for audio format configuration
Diagram Walkthrough
File Walkthrough
18 files
Main plugin registration and Python engine initializationCore Python code execution service implementationFunction callback for Python code generation and executionEnhanced instruct service with code template executionAdd agent code script repository interface methodsMongoDB implementation for agent code script storageFile system implementation for agent code script storageDomain model for agent code scriptsInterface definition for code interpretation servicesOptions model for code instruction executionAudio format configuration options for realtimeAdd `RoleContent` property for unified content accessPython package installation and validation utilitiesAdd code script loading from file systemInclude code scripts in agent refresh processAdd code execution hooks to instruct interfaceAdd options setter for realtime completion providersImplement realtime options and use `RoleContent` property1 files
Configuration settings for Python interpreter75 files