.NET: Python: feat(python): Add local-codeact package with AST validation#6091
.NET: Python: feat(python): Add local-codeact package with AST validation#6091eavanvalkenburg wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new alpha Python workspace package, agent-framework-local-codeact, intended to enable CodeAct-style execution of model-generated Python in externally sandboxed environments (e.g., Foundry hosted agents), with subprocess execution, file-mount capture, and AST-based validation.
Changes:
- Registers
agent-framework-local-codeactin the Python workspace (uv/pyproject) and marks it as alpha inPACKAGE_STATUS.md. - Introduces
LocalExecuteCodeTool/LocalCodeActProviderwith subprocess runner + IPC bridge, file-mount capture helpers, and dynamic instructions. - Adds unit tests plus usage samples and package documentation.
Reviewed changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| python/uv.lock | Adds the new editable workspace member and lock metadata. |
| python/pyproject.toml | Registers the new workspace package. |
| python/PACKAGE_STATUS.md | Marks agent-framework-local-codeact as alpha. |
| python/packages/local_codeact/pyproject.toml | New package definition, tooling config, and test tasks. |
| python/packages/local_codeact/README.md | Package docs, security posture, and configuration surface. |
| python/packages/local_codeact/AGENTS.md | Package architecture and contributor notes. |
| python/packages/local_codeact/LICENSE | MIT license for the new package. |
| python/packages/local_codeact/agent_framework_local_codeact/init.py | Public API exports for the package. |
| python/packages/local_codeact/agent_framework_local_codeact/_types.py | Public types for execution mode, mounts, and limits. |
| python/packages/local_codeact/agent_framework_local_codeact/_validator.py | AST-based code validation layer. |
| python/packages/local_codeact/agent_framework_local_codeact/_bridge.py | Parent-side subprocess bridge + tool dispatch. |
| python/packages/local_codeact/agent_framework_local_codeact/_runner.py | Child-process runner implementing the JSON-lines protocol. |
| python/packages/local_codeact/agent_framework_local_codeact/_files.py | Mount normalization + symlink-safe file capture. |
| python/packages/local_codeact/agent_framework_local_codeact/_instructions.py | Dynamic CodeAct instructions and tool descriptions. |
| python/packages/local_codeact/agent_framework_local_codeact/_execute_code_tool.py | Main execute_code tool orchestration and output shaping. |
| python/packages/local_codeact/agent_framework_local_codeact/_provider.py | Context provider that injects the run-scoped tool + instructions. |
| python/packages/local_codeact/tests/local_codeact/test_validator.py | Validator allow/block behavior tests. |
| python/packages/local_codeact/tests/local_codeact/test_local_codeact.py | Tool/provider behavior, subprocess execution, mounts, and limits tests. |
| python/packages/local_codeact/samples/README.md | Sample index and run instructions. |
| python/packages/local_codeact/samples/local_execute_code.py | Local usage sample for direct tool invocation. |
| python/packages/local_codeact/samples/foundry_hosted_agent.py | Foundry hosted-agent wiring sample. |
| python/packages/local_codeact/agent_framework_local_codeact/py.typed | Marks the package as typed. |
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 88%
✓ Correctness
After thorough examination of the local-codeact package implementation, I found no correctness bugs. The code demonstrates excellent engineering practices: proper error handling with early validation, safe resource cleanup using try-finally blocks and context managers, correct subprocess management with timeout handling, secure AST validation with comprehensive allow/block lists, and proper IPC serialization with JSON-safe conversions. All test assertions correctly match the implementation behavior. The Windows environment variable handling (SYSTEMROOT, COMSPEC, PATHEXT) is intentional and necessary for subprocess creation. The validator's permissive approach to user-defined functions is documented and tested. Edge cases like subprocess death, tool call failures, timeout during execution, and symlink handling are all properly managed.
✓ Security Reliability
The local CodeAct package provides defense-in-depth controls for executing LLM-generated Python code, with AST validation, subprocess isolation, and explicit environment control. The implementation is generally sound for its stated purpose (use in external sandboxes like Foundry). However, there are three reliability concerns: (1) the AST validator allows 'open' in ALLOWED_BUILTINS while blocking it in BLOCKED_BUILTINS, creating conflicting policy; (2) subprocess environment building on Windows includes parent environment keys that could leak sensitive data; (3) the validator allows delattr/setattr which could modify object internals unsafely. The package correctly disclaims being a security sandbox and documents required external isolation.
✓ Test Coverage
The test suite provides solid coverage of core functionality (subprocess execution, tool calling, validation, file capture, environment isolation). However, several edge cases and error paths lack coverage: (1) invalid input validation for constructors (empty/invalid paths, negative limits), (2) error handling for subprocess failures (invalid Python executable, runner script errors, malformed bridge responses), (3) boundary conditions for limits (exact limit sizes, total capture limits), (4) file mount edge cases (duplicate mounts, overlapping paths, permission errors), (5) race conditions in async tool calls, and (6) error recovery paths in the bridge protocol. The existing tests are well-structured and verify the happy paths thoroughly.
✓ Design Approach
The design approach is sound for the stated goal of adding AST-validated local code execution for Foundry hosted agents. The validation correctly runs before all execution paths, the subprocess bridge properly serializes concurrent tool calls via async locks, symlink handling prevents directory traversal, and the custom allow/block list semantics are clearly documented. All test cases in the diff are consistent with the implementation.
Automated review by eavanvalkenburg's agents
- Remove 'open', 'getattr', 'setattr', 'hasattr' from ALLOWED_BUILTINS (bypass risk) - Add these to BLOCKED_BUILTINS with explanatory comments - Propagate AST validation settings to create_run_tool snapshot - Terminate subprocess before raising on error messages - Move module docstrings to file start in samples - Remove pointless string statements from samples - Document allowed_builtins behavior in visit_Call Fixes all 8 review comments in PR microsoft#6091
Review Comments AddressedAll 8 review comments have been addressed in commit a38ea7c: Security Fixes
Implementation Fixes
Sample Fixes
All checks passing locally:
|
Python 3.10 Compatibility FixedFixed timeout test that was failing on Python 3.10 due to different TimeoutError string representation. Commit: 4760db8 Verified on:
All 46 tests now pass on both versions. |
Add agent-framework-local-codeact alpha package for running LLM-generated Python code in Foundry hosted agents and other sandboxed environments. Key features: - Subprocess execution by default (isolated process) - Optional unsafe in-process mode for debugging - AST-based allow-list code validation - Customizable allowed/blocked imports and builtins - Host tool bridge with framed JSON-lines IPC - File mount system with capture and limits - .NET portability features (python_executable, runner_script) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove 'open', 'getattr', 'setattr', 'hasattr' from ALLOWED_BUILTINS (bypass risk) - Add these to BLOCKED_BUILTINS with explanatory comments - Propagate AST validation settings to create_run_tool snapshot - Terminate subprocess before raising on error messages - Move module docstrings to file start in samples - Remove pointless string statements from samples - Document allowed_builtins behavior in visit_Call Fixes all 8 review comments in PR microsoft#6091
Python 3.10's TimeoutError has a different string representation than 3.11+. Update test to check for 'TimeoutError' instead of specific message content. Verified on Python 3.10.15 and 3.12.7.
- _validator.py: visit_Call now enforces ALLOWED_BUILTINS for names that match real Python builtins, while still treating unknown names as user-defined functions/registered tools. This makes the allowed_builtins parameter behave as a real allow-list. - _bridge.py / _runner.py: add explicit '# nosec' markers next to the existing '# noqa: S102/S404' so bandit accepts the intentional subprocess import and exec() calls (this package's whole purpose). - test_validator.py: add tests for unknown-builtin rejection, user-defined function acceptance, and custom allow-list expansion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
On Windows a freshly-killed subprocess can briefly hold the temporary workspace directory open. Swallow OSError from temp_dir.cleanup() so the caller still receives the proper error Content from the run and so the timeout test passes on Windows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ruff SIM105 prefers contextlib.suppress over try/except/pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The previous OSError-only suppression missed the RecursionError that Python's TemporaryDirectory cleanup can raise on Windows when a freshly killed subprocess still holds a handle to the workspace. Pass ignore_cleanup_errors=True (Python 3.10+) so the platform stops retrying rmtree, and broaden the outer suppression so unexpected cleanup errors do not mask the actual run result. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
f708ca4 to
9c91299
Compare
Add agent-framework-local-codeact alpha package with AST validation for Foundry hosted agents