Bugfix/fix code exec timeout #1209

iceljc · 2025-11-08T06:11:58Z

PR Type

Bug fix, Enhancement

Description

Add timeout handling for code execution with 3-second cancellation token
Implement process-based Python code execution as alternative to in-process
Create CodingPlugin for dependency injection and settings management
Add configurable MaxConcurrency setting for code script execution
Refactor code interpreter with proper error handling and logging

Diagram Walkthrough

flowchart LR
  A["CodeInterpretOptions"] -->|"UseProcess flag"| B["PyCodeInterpreter"]
  B -->|"in-process"| C["CoreRunScript"]
  B -->|"separate process"| D["CoreRunProcess"]
  E["CodingSettings"] -->|"MaxConcurrency"| F["CodeScriptExecutor"]
  G["InstructService"] -->|"3s timeout"| B
  H["CodingPlugin"] -->|"registers"| F

File Walkthrough

Relevant files

Refactoring

2 files

AgentCodeScript.cs `Move ToString implementation to base class`	+6/-1
AgentPlugin.cs `Remove CodeScriptExecutor registration from AgentPlugin`	+0/-3

Formatting

1 files

AgentSettings.cs `Simplify LlmConfig initialization syntax`	+1/-2

Enhancement

5 files

CodeInterpretOptions.cs `Add UseProcess flag for execution mode selection`	+1/-0
CodeInterpretResponse.cs `Add ToString method for response serialization`	+5/-0
CodingSettings.cs `Create new coding configuration settings structure`	+26/-0
CodeScriptExecutor.cs `Add configurable concurrency control via CodingSettings`	+6/-0
CodingPlugin.cs `Create new plugin for coding service registration`	+19/-0

Configuration changes

3 files

Using.cs `Add global using for CodingSettings namespace`	+2/-1
Using.cs `Add global using for CodingSettings namespace`	+1/-0
appsettings.json `Add Coding configuration section with execution settings`	+10/-0

Bug fix

2 files

InstructService.Execute.cs `Add 3-second timeout and error handling for code execution`	+10/-2
PyCodeInterpreter.cs `Implement dual execution modes with timeout and cancellation support`	+159/-23

qodo-merge-pro · 2025-11-08T06:12:30Z

PR Compliance Guide 🔍

(Compliance updated until commit `d911cfc`)

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Arbitrary code execution Description: Passing unvalidated code directly to 'python -c' for execution enables arbitrary code execution if the codeScript can be influenced by user input; this should be sandboxed or restricted and validated. PyCodeInterpreter.cs [219-246] Referred Code var psi = new ProcessStartInfo { FileName = "python", UseShellExecute = false, RedirectStandardOutput = true, RedirectStandardError = true, CreateNoWindow = true, StandardOutputEncoding = Encoding.UTF8, StandardErrorEncoding = Encoding.UTF8 }; // Add raw code script psi.ArgumentList.Add("-c"); psi.ArgumentList.Add(codeScript); // Add arguments (safe—no shared state) if (options?.Arguments?.Any() == true) { foreach (var arg in options.Arguments!) { if (!string.IsNullOrWhiteSpace(arg.Key) && !string.IsNullOrWhiteSpace(arg.Value)) ... (clipped 7 lines)
	Argument injection risk Description: Command-line arguments are concatenated and passed to a subprocess without strict validation or quoting, which could allow argument injection or unexpected behavior if keys/values contain malicious content. PyCodeInterpreter.cs [237-244] Referred Code foreach (var arg in options.Arguments!) { if (!string.IsNullOrWhiteSpace(arg.Key) && !string.IsNullOrWhiteSpace(arg.Value)) { psi.ArgumentList.Add($"--{arg.Key}"); psi.ArgumentList.Add($"{arg.Value}"); } }
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
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Action logging: New critical actions executing code scripts add some logging but do not clearly include user/context identifiers or consistent structured details to reconstruct events. Referred Code _logger.LogWarning($"Begin running python code script in {Provider}: {scriptName}"); if (options?.UseProcess == true) { response = await CoreRunProcess(codeScript, options); } else { response = await CoreRunScript(codeScript, options); } _logger.LogWarning($"End running python code script in {Provider}: {scriptName}"); return response;
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Timeout handling: The 3-second cancellation token aborts execution but upstream error context is reduced to returning the previous response without logging or conveying actionable failure reasons. Referred Code using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(3)); var codeResponse = await codeProcessor.RunAsync(context.CodeScript, options: new() { ScriptName = scriptName, Arguments = context.Arguments, CancellationToken = cts.Token }); if (codeResponse == null \|\| !codeResponse.Success) { return response; } response = new InstructResult { MessageId = message.MessageId, Template = scriptName, Text = codeResponse.Result };
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Error exposure: Errors and stderr from executed scripts are captured and may be propagated (e.g., via ErrorMsg or logs) without clear separation between internal logs and user-facing messages. Referred Code await Task.WhenAll([proc.WaitForExitAsync(token), stdoutTask, stderrTask]); token.ThrowIfCancellationRequested(); return new CodeInterpretResponse { Success = proc.ExitCode == 0, Result = stdoutTask.Result?.TrimEnd('\r', '\n') ?? string.Empty, ErrorMsg = stderrTask.Result };
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Unstructured logs: Logging statements use free-form messages without structured fields and may include dynamic script names and exception messages that could inadvertently contain sensitive data. Referred Code _logger.LogWarning($"Begin running python code script in {Provider}: {scriptName}"); if (options?.UseProcess == true) { response = await CoreRunProcess(codeScript, options); } else { response = await CoreRunScript(codeScript, options); } _logger.LogWarning($"End running python code script in {Provider}: {scriptName}"); return response; } catch (OperationCanceledException oce) { _logger.LogError(oce, $"Operation cancelled in {nameof(InnerRunCode)} in {Provider}."); response.ErrorMsg = oce.Message; return response; } ... (clipped 5 lines)
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Process exec args: When running Python in a separate process, arguments are appended directly to the command line and script content is passed via -c without explicit sanitization or restrictions, which may pose injection/misuse risks depending on upstream inputs. Referred Code var psi = new ProcessStartInfo { FileName = "python", UseShellExecute = false, RedirectStandardOutput = true, RedirectStandardError = true, CreateNoWindow = true, StandardOutputEncoding = Encoding.UTF8, StandardErrorEncoding = Encoding.UTF8 }; // Add raw code script psi.ArgumentList.Add("-c"); psi.ArgumentList.Add(codeScript); // Add arguments (safe—no shared state) if (options?.Arguments?.Any() == true) { foreach (var arg in options.Arguments!) { if (!string.IsNullOrWhiteSpace(arg.Key) && !string.IsNullOrWhiteSpace(arg.Value)) ... (clipped 6 lines)

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit bbb41ff

Security Compliance
⚪	Arbitrary code execution Description: Executing arbitrary Python code via external process with arguments can lead to command misuse and unbounded code execution if upstream inputs are untrusted; ensure scripts and args are strictly controlled and sandboxing is applied. PyCodeInterpreter.cs [219-283] Referred Code var psi = new ProcessStartInfo { FileName = "python", UseShellExecute = false, RedirectStandardOutput = true, RedirectStandardError = true, CreateNoWindow = true, StandardOutputEncoding = Encoding.UTF8, StandardErrorEncoding = Encoding.UTF8 }; // Add raw code script psi.ArgumentList.Add("-c"); psi.ArgumentList.Add(codeScript); // Add arguments (safe—no shared state) if (options?.Arguments?.Any() == true) { foreach (var arg in options.Arguments!) { if (!string.IsNullOrWhiteSpace(arg.Key) && !string.IsNullOrWhiteSpace(arg.Value)) ... (clipped 44 lines)
	In-process RCE risk Description: In-process Python execution (Python.NET) runs arbitrary code within host process, risking RCE, data exfiltration, and process compromise without sandboxing or privilege isolation. PyCodeInterpreter.cs [138-201] Referred Code { _logger.LogWarning($"Begin {nameof(CoreRunScript)} in {Provider}: ${options?.ScriptName}"); var token = options?.CancellationToken ?? CancellationToken.None; token.ThrowIfCancellationRequested(); using (Py.GIL()) { token.ThrowIfCancellationRequested(); // Import necessary Python modules dynamic sys = Py.Import("sys"); dynamic io = Py.Import("io"); try { // Redirect standard output/error to capture it dynamic stringIO = io.StringIO(); sys.stdout = stringIO; sys.stderr = stringIO; ... (clipped 43 lines)
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
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing auditing: Execution of code scripts (a critical action) is performed without emitting structured audit logs that include user/context, timestamp, action description, and outcome. Referred Code #region Private methods private async Task<CodeInterpretResponse> InnerRunCode(string codeScript, CodeInterpretOptions? options = null) { var response = new CodeInterpretResponse(); var scriptName = options?.ScriptName ?? codeScript.SubstringMax(30); try { _logger.LogWarning($"Begin running python code script in {Provider}: {scriptName}"); if (options?.UseProcess == true) { response = await CoreRunProcess(codeScript, options); } else { response = await CoreRunScript(codeScript, options); } _logger.LogWarning($"End running python code script in {Provider}: {scriptName}"); ... (clipped 15 lines)
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Partial handling: New cancellation and process execution paths add exceptions that are logged but not consistently converted into actionable response details or fallbacks, risking silent failure to callers in some branches. Referred Code private async Task<CodeInterpretResponse> CoreRunProcess(string codeScript, CodeInterpretOptions? options = null) { var token = options?.CancellationToken ?? CancellationToken.None; var psi = new ProcessStartInfo { FileName = "python", UseShellExecute = false, RedirectStandardOutput = true, RedirectStandardError = true, CreateNoWindow = true, StandardOutputEncoding = Encoding.UTF8, StandardErrorEncoding = Encoding.UTF8 }; // Add raw code script psi.ArgumentList.Add("-c"); psi.ArgumentList.Add(codeScript); // Add arguments (safe—no shared state) ... (clipped 55 lines)
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Error exposure: Returning raw stderr and exception messages in `CodeInterpretResponse` may expose internal details to callers if propagated to end users. Referred Code await proc.WaitForExitAsync(); var stdout = await stdoutTask; var stderr = await stderrTask; token.ThrowIfCancellationRequested(); return new CodeInterpretResponse { Success = proc.ExitCode == 0, Result = stdout, ErrorMsg = stderr };
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Log sensitivity: Logs include full script names and may include exception details from executed code which can inadvertently contain sensitive data if user-provided. Referred Code _logger.LogWarning($"Begin running python code script in {Provider}: {scriptName}"); if (options?.UseProcess == true) { response = await CoreRunProcess(codeScript, options); } else { response = await CoreRunScript(codeScript, options); } _logger.LogWarning($"End running python code script in {Provider}: {scriptName}"); return response; } catch (OperationCanceledException oce) { _logger.LogError(oce, $"Operation cancelled in {nameof(InnerRunCode)} in {Provider}."); response.ErrorMsg = oce.Message; return response; } ... (clipped 5 lines)
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Unvalidated input: The interpreter passes unvalidated `codeScript` and arguments directly to Python embedding or subprocess which could be unsafe without sandboxing or validation. Referred Code private async Task<CodeInterpretResponse> CoreRunProcess(string codeScript, CodeInterpretOptions? options = null) { var token = options?.CancellationToken ?? CancellationToken.None; var psi = new ProcessStartInfo { FileName = "python", UseShellExecute = false, RedirectStandardOutput = true, RedirectStandardError = true, CreateNoWindow = true, StandardOutputEncoding = Encoding.UTF8, StandardErrorEncoding = Encoding.UTF8 }; // Add raw code script psi.ArgumentList.Add("-c"); psi.ArgumentList.Add(codeScript); // Add arguments (safe—no shared state) if (options?.Arguments?.Any() == true) ... (clipped 13 lines)

qodo-merge-pro · 2025-11-08T06:13:55Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Enable the new process-based execution strategy The newly added process-based execution strategy is never used because the `UseProcess` flag is not set. This should be enabled by default or made configurable to properly handle timeouts. Examples: src/Infrastructure/BotSharp.Core/Instructs/Services/InstructService.Execute.cs [258-263] var codeResponse = await codeProcessor.RunAsync(context.CodeScript, options: new() { ScriptName = scriptName, Arguments = context.Arguments, CancellationToken = cts.Token }); src/Plugins/BotSharp.Plugin.PythonInterpreter/Services/PyCodeInterpreter.cs [110-117] if (options?.UseProcess == true) { response = await CoreRunProcess(codeScript, options); } else { response = await CoreRunScript(codeScript, options); } Solution Walkthrough: Before: // src/Infrastructure/BotSharp.Core/Instructs/Services/InstructService.Execute.cs var codeResponse = await codeProcessor.RunAsync(context.CodeScript, options: new() { ScriptName = scriptName, Arguments = context.Arguments, CancellationToken = cts.Token // UseProcess defaults to false, so the new logic is never called. }); // src/Plugins/BotSharp.Plugin.PythonInterpreter/Services/PyCodeInterpreter.cs private async Task<CodeInterpretResponse> InnerRunCode(...) { if (options?.UseProcess == true) // This is always false { response = await CoreRunProcess(codeScript, options); } else { response = await CoreRunScript(codeScript, options); // Old logic is always used } //... } After: // src/Infrastructure/BotSharp.Abstraction/Coding/Settings/CodingSettings.cs public class CodeScriptExecutionSettings { // ... public bool UseProcess { get; set; } = true; } // src/Infrastructure/BotSharp.Core/Instructs/Services/InstructService.Execute.cs var codeResponse = await codeProcessor.RunAsync(context.CodeScript, options: new() { ScriptName = scriptName, Arguments = context.Arguments, CancellationToken = cts.Token, UseProcess = _codingSettings.CodeExecution.UseProcess // Now configurable }); // src/Plugins/BotSharp.Plugin.PythonInterpreter/Services/PyCodeInterpreter.cs private async Task<CodeInterpretResponse> InnerRunCode(...) { if (options?.UseProcess == true) // Can now be true { response = await CoreRunProcess(codeScript, options); // New logic can be used } //... } Suggestion importance[1-10]: 10 __ Why: This suggestion correctly identifies that the new, more robust process-based execution logic is never used, making the timeout feature ineffective for blocking code and undermining a major goal of the PR.	High
Possible issue	✅ ~~Prevent potential process execution deadlock~~ Suggestion Impact: The commit changed ReadToEndAsync to use the cancellation token and replaced awaiting WaitForExitAsync alone with awaiting Task.WhenAll on WaitForExitAsync(token), stdout, and stderr tasks, implementing the suggested deadlock prevention. code diff: - var stdoutTask = proc.StandardOutput.ReadToEndAsync(); - var stderrTask = proc.StandardError.ReadToEndAsync(); - - await proc.WaitForExitAsync(); - - var stdout = await stdoutTask; - var stderr = await stderrTask; + var stdoutTask = proc.StandardOutput.ReadToEndAsync(token); + var stderrTask = proc.StandardError.ReadToEndAsync(token); + + await Task.WhenAll([proc.WaitForExitAsync(token), stdoutTask, stderrTask]); Prevent a potential process deadlock by reading the standard output and error streams concurrently with waiting for the process to exit using `Task.WhenAll`. src/Plugins/BotSharp.Plugin.PythonInterpreter/Services/PyCodeInterpreter.cs [267-273] -var stdoutTask = proc.StandardOutput.ReadToEndAsync(); -var stderrTask = proc.StandardError.ReadToEndAsync(); +var stdoutTask = proc.StandardOutput.ReadToEndAsync(token); +var stderrTask = proc.StandardError.ReadToEndAsync(token); -await proc.WaitForExitAsync(); +await Task.WhenAll(proc.WaitForExitAsync(token), stdoutTask, stderrTask); var stdout = await stdoutTask; var stderr = await stderrTask; `[Suggestion processed]` Suggestion importance[1-10]: 9 __ Why: This suggestion identifies and fixes a critical potential deadlock in process handling, which could cause the application to hang, making it a high-impact bug fix.	High
General	Make code execution timeout configurable Replace the hardcoded 3-second code execution timeout with a configurable value from `CodeScriptExecutionSettings` to improve flexibility. src/Infrastructure/BotSharp.Core/Instructs/Services/InstructService.Execute.cs [256-263] // Run code script -using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(3)); +var codingSettings = _services.GetRequiredService<CodingSettings>(); +var timeout = codingSettings.CodeExecution.ExecutionTimeoutInSeconds > 0 ? codingSettings.CodeExecution.ExecutionTimeoutInSeconds : 30; +using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(timeout)); var codeResponse = await codeProcessor.RunAsync(context.CodeScript, options: new() { ScriptName = scriptName, Arguments = context.Arguments, CancellationToken = cts.Token }); Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a hardcoded 3-second timeout as a potential issue and proposes making it configurable, which significantly improves the flexibility and robustness of the feature.	Medium
General	Avoid hardcoded LLM model fallbacks Avoid hardcoded LLM provider and model fallbacks in `GetLlmProviderModel` by using the default values from `AgentSettings` for better configuration management. src/Plugins/BotSharp.Plugin.PythonInterpreter/Services/PyCodeInterpreter.cs [303-317] private (string, string) GetLlmProviderModel() { var provider = _settings.CodeGeneration?.Provider; var model = _settings.CodeGeneration?.Model; if (!string.IsNullOrEmpty(provider) && !string.IsNullOrEmpty(model)) { return (provider, model); } - provider = "openai"; - model = "gpt-5-mini"; + var agentSettings = _services.GetRequiredService<AgentSettings>(); + provider = agentSettings.LlmConfig.Provider; + model = agentSettings.LlmConfig.Model; return (provider, model); } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out hardcoded fallback values and proposes a better design by using a centralized configuration from `AgentSettings`, improving maintainability and consistency.	Low
Update

Jicheng Lu and others added 8 commits October 22, 2025 11:36

Merge branch 'master' of https://github.com/SciSharp/BotSharp

ffee953

Merge branch 'master' of https://github.com/SciSharp/BotSharp

c092ff7

Merge branch 'master' of https://github.com/SciSharp/BotSharp

4a02efb

temp save

cfb3c5e

refine

82d51af

add use process and coding settings

7ec4581

minor change

e221167

minor change

bbb41ff

qodo-merge-pro bot added the Review effort 3/5 label Nov 8, 2025

format output

d911cfc

iceljc merged commit 61cad7c into SciSharp:master Nov 8, 2025
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Bugfix/fix code exec timeout #1209

Bugfix/fix code exec timeout #1209

Uh oh!

iceljc commented Nov 8, 2025 •

edited by qodo-merge-pro bot

Loading

Uh oh!

qodo-merge-pro bot commented Nov 8, 2025 •

edited

Loading

Uh oh!

qodo-merge-pro bot commented Nov 8, 2025 •

edited

Loading

Examples:

Solution Walkthrough:

Before:

After:

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Bugfix/fix code exec timeout #1209

Bugfix/fix code exec timeout #1209

Uh oh!

Conversation

iceljc commented Nov 8, 2025 • edited by qodo-merge-pro bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

qodo-merge-pro bot commented Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Compliance Guide 🔍

(Compliance updated until commit d911cfc)

Previous compliance checks

Uh oh!

qodo-merge-pro bot commented Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Examples:

Solution Walkthrough:

Before:

After:

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

iceljc commented Nov 8, 2025 •

edited by qodo-merge-pro bot

Loading

qodo-merge-pro bot commented Nov 8, 2025 •

edited

Loading

(Compliance updated until commit `d911cfc`)

qodo-merge-pro bot commented Nov 8, 2025 •

edited

Loading