feat: use separate python venv for bash tool #1087
Conversation
Signed-off-by: Jet Chiang <jetjiang.ez@gmail.com>
Signed-off-by: Jet Chiang <jetjiang.ez@gmail.com>
Signed-off-by: Jet Chiang <jetjiang.ez@gmail.com>
Signed-off-by: Jet Chiang <jetjiang.ez@gmail.com>
Signed-off-by: Jet Chiang <jetjiang.ez@gmail.com>
Signed-off-by: Jet Chiang <jetjiang.ez@gmail.com>
Signed-off-by: Jet Chiang <jetjiang.ez@gmail.com>
Signed-off-by: Jet Chiang <jetjiang.ez@gmail.com>
Signed-off-by: Jet Chiang <jetjiang.ez@gmail.com>
Signed-off-by: Jet Chiang <jetjiang.ez@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR creates a separate Python virtual environment for bash tool commands to isolate them from the main application environment. The bash tool now uses a dedicated sandbox venv at /.kagent/sandbox-venv instead of the app's venv.
Key changes:
- Created a new sandbox virtual environment in the Dockerfile for bash tool isolation
- Modified bash_tool.py to use the sandbox venv by prepending it to PATH and setting VIRTUAL_ENV
- Added BASH_VENV_PATH environment variable to configure the sandbox venv path
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| python/Dockerfile | Creates the bash tool sandbox venv during Docker build and sets BASH_VENV_PATH environment variable |
| python/packages/kagent-adk/src/kagent/adk/tools/bash_tool.py | Modifies environment variables to use the bash tool sandbox venv instead of the app's venv |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Use the separate bash tool venv instead of the app's venv | ||
| bash_venv_path = os.environ.get("BASH_VENV_PATH", "/.kagent/sandbox-venv") | ||
| bash_venv_bin = os.path.join(bash_venv_path, "bin") | ||
| # Prepend bash venv to PATH so its python and pip are used | ||
| env["PATH"] = f"{bash_venv_bin}:{env.get('PATH', '')}" | ||
| env["VIRTUAL_ENV"] = bash_venv_path |
There was a problem hiding this comment.
The bash_venv_path is read from an environment variable without validation. A malicious value could potentially be used to point to an attacker-controlled directory. Consider validating that the path exists and is a directory before using it, or use the hardcoded default value directly since it's set in the Dockerfile.
Creates a separate venv for ADK container and prepends its path into
PATHso that this is used by bash tool instead of the application venv.