feat: Add Kubernetes Agent Sandbox tools - #6708
Open
ArthurKamalov wants to merge 45 commits into
Open
Conversation
📝 WalkthroughWalkthroughAdds a Kubernetes Agent Sandbox integration for CrewAI, including configurable ephemeral, persistent, and attach lifecycle modes, shared toolsets, command execution, Python execution, filesystem operations, package exports, documentation, and tests. ChangesKubernetes Agent Sandbox Integration
Sequence Diagram(s)sequenceDiagram
participant CrewAI as CrewAI agent
participant Toolset as K8sAgentSandboxToolset
participant Lifecycle as K8sAgentSandboxLifecycleManager
participant Sandbox as Kubernetes Sandbox
CrewAI->>Toolset: invoke sandbox tool
Toolset->>Lifecycle: acquire_sandbox()
Lifecycle->>Sandbox: create or attach
Sandbox-->>Lifecycle: sandbox execution context
Lifecycle-->>Toolset: acquired sandbox
Toolset-->>CrewAI: command, Python, or file result
Toolset->>Lifecycle: release or close
Lifecycle->>Sandbox: terminate when configured
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/python_tool.py (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse CapWords for the schema class.
Rename
k8sAgentSandboxPythonToolSchematoK8sAgentSandboxPythonToolSchemaand update theargs_schemaassignment in the tool class.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/python_tool.py` at line 18, Rename the schema class k8sAgentSandboxPythonToolSchema to K8sAgentSandboxPythonToolSchema using CapWords, then update the tool class’s args_schema assignment to reference the renamed class.Source: Coding guidelines
lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/utils.py (1)
8-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocstring overstates capability; drop unused
globals()injection.The docstring says this supports
'math.sqrt'-style object paths, but the implementation only imports submodules viaimportlib.import_module— it can't resolve an attribute. Also,globals()[short_name] = obj(line 35) injects the imported module into this file's namespace, but neither call site (settings.py,lifecycle_manager.py) uses the injected global — both use the returned value directly, making this dead/confusing side effect.♻️ Proposed cleanup
def lazy_import_k8s_agent_sandbox(target: str) -> types.ModuleType: """ - Lazily imports a module or attribute by string name, caches it globally, + Lazily imports a `k8s_agent_sandbox` submodule by name, caches it globally, and returns it. Subsequent calls return the object directly from the cache. - :param target: Module name (e.g., 'json') or object path (e.g., 'math.sqrt') + :param target: Submodule name relative to `k8s_agent_sandbox` (e.g., 'sandbox_client') :return: The imported module or attribute object """ full_target = f"k8s_agent_sandbox.{target}" module = _MODULE_CACHE.get(full_target) if module is not None: return module try: obj = importlib.import_module(full_target) except ModuleNotFoundError as e: raise ImportError( "The 'k8s-agent-sandbox' package is required for K8s Agent Sandbox tools. " ) from e - # 3. Store in global cache collection _MODULE_CACHE[full_target] = obj - - # 4. Inject into the module's global space (using short name) - short_name = full_target.split(".")[-1] - globals()[short_name] = obj - return obj🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/utils.py` around lines 8 - 37, Update lazy_import_k8s_agent_sandbox to document only module-name imports, removing the unsupported object-path example from its docstring. Remove the globals()[short_name] injection and related comments, while preserving caching and returning the imported object to callers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/crewai-tools/pyproject.toml`:
- Around line 151-153: Update the test/dev dependency configuration in
pyproject.toml to include the existing k8s_agent_sandbox extra, ensuring tests
that import k8s_agent_sandbox have the package installed in the test
environment; alternatively, convert the affected test imports to lazy imports
consistent with the production tools.
In `@lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/file_tool.py`:
- Around line 51-54: Update the path validation around the FileTool path field
to reject absolute paths and paths that normalize to “..” or start with “../”
before dispatching any action; also place “--” before user-controlled operands
in shell-backed rm and mkdir commands. In
lib/crewai-tools/tests/tools/k8s_agent_sandbox/test_file_tool.py lines 249-274,
change the /app/some-path case to expect failure and add coverage for traversal
paths.
- Around line 78-85: The _validate_action_args validator must reject malformed
base64 payloads before any file write or append occurs. Validate provided
content using base64.b64decode with validate=True for both supported actions,
preserve the existing append-content requirement, and add invalid-payload tests
covering write and append.
In
`@lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/lifecycle_manager.py`:
- Around line 44-55: Update acquire_sandbox so failures from _acquire_sandbox()
release self._lock and restore the unacquired state before propagating the
exception. Keep the existing closed-helper handling and successful acquisition
behavior unchanged, ensuring every lock acquired by acquire_sandbox is either
retained for a successful sandbox or released on failure.
In `@lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/README.md`:
- Around line 105-108: Update each lifecycle example import block in
lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/README.md at lines
105-108, 124-127, and 144-147 to import K8sAgentSandboxExecTool and
K8sAgentSandboxFileTool alongside the existing K8sAgentSandboxToolset and
SandboxSettings imports, so every snippet can instantiate the tools it uses.
- Around line 163-175: Update the K8sAgentSandboxFileTool action list to remove
the unsupported "info" value, and synchronize timeout documentation for
K8sAgentSandboxExecTool and K8sAgentSandboxPythonTool with the schemas as int
values defaulting to 60. Add the file tool’s timeout argument and document its
matching schema type and default.
In `@lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/settings.py`:
- Around line 31-45: Make the lazy initialization in the client property
thread-safe by adding a lock to the settings object and guarding both the
cached-client check and SandboxClient construction/assignment with it. Preserve
the fast return for an already initialized client, and ensure concurrent callers
share the single cached instance without creating discarded connections.
In `@lib/crewai-tools/tests/tools/k8s_agent_sandbox/test_base_tool.py`:
- Around line 93-107: Update the exception test around tool.run to assert the
wrapped lifecycle manager directly: after pytest.raises completes, call
m.release_sandbox.assert_called_once_with(). Keep the existing termination
assertions unchanged so the test verifies release behavior across persistent and
attach modes.
In `@lib/crewai-tools/tests/tools/k8s_agent_sandbox/test_python_tool.py`:
- Around line 33-36: Update the test around mock_sandbox.files.write and
mock_sandbox.commands.run to assert that files.write receives the code payload
b"some-code". Remove the command prefix assertion tied to the temporary path,
while preserving the timeout validation and other behavior-focused checks.
---
Nitpick comments:
In `@lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/python_tool.py`:
- Line 18: Rename the schema class k8sAgentSandboxPythonToolSchema to
K8sAgentSandboxPythonToolSchema using CapWords, then update the tool class’s
args_schema assignment to reference the renamed class.
In `@lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/utils.py`:
- Around line 8-37: Update lazy_import_k8s_agent_sandbox to document only
module-name imports, removing the unsupported object-path example from its
docstring. Remove the globals()[short_name] injection and related comments,
while preserving caching and returning the imported object to callers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bfaade49-7c06-47e9-9a8d-221da1aa2fc4
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
lib/crewai-tools/pyproject.tomllib/crewai-tools/src/crewai_tools/__init__.pylib/crewai-tools/src/crewai_tools/tools/__init__.pylib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/README.mdlib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/__init__.pylib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/base_tool.pylib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/exec_tool.pylib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/file_tool.pylib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/lifecycle_manager.pylib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/python_tool.pylib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/settings.pylib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/toolset.pylib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/utils.pylib/crewai-tools/tests/tools/k8s_agent_sandbox/conftest.pylib/crewai-tools/tests/tools/k8s_agent_sandbox/test_base_tool.pylib/crewai-tools/tests/tools/k8s_agent_sandbox/test_exec_tool.pylib/crewai-tools/tests/tools/k8s_agent_sandbox/test_file_tool.pylib/crewai-tools/tests/tools/k8s_agent_sandbox/test_lifecycle_manager.pylib/crewai-tools/tests/tools/k8s_agent_sandbox/test_python_tool.pylib/crewai-tools/tests/tools/k8s_agent_sandbox/test_toolset.py
Comment on lines
+151
to
+153
| k8s_agent_sandbox = [ | ||
| "k8s-agent-sandbox~=0.5.0", | ||
| ] |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the k8s_agent_sandbox extra is wired into test/dev dependency groups.
rg -n -A5 'k8s_agent_sandbox' lib/crewai-tools/pyproject.toml
rg -n -B2 -A10 '\[dependency-groups\]|\[tool\.uv\]|extra ==' lib/crewai-tools/pyproject.tomlRepository: crewAIInc/crewAI
Length of output: 487
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== pyproject relevant sections =="
sed -n '1,220p' lib/crewai-tools/pyproject.toml | nl -ba | sed -n '130,220p'
echo
echo "== candidate test files importing k8s_agent_sandbox =="
rg -n "k8s_agent_sandbox|k8s-agent-sandbox|SandboxNotFoundError" lib/crewai-tools/src lib/crewai-tools/tests lib/crewai-tools -g '*.py' | head -200 || true
echo
echo "== dependency groups / pytest uv scripts =="
rg -n "dependency-groups|pytest|k8s" lib/crewai-tools/pyproject.toml -C 3 || trueRepository: crewAIInc/crewAI
Length of output: 228
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== pyproject relevant sections =="
python3 - <<'PY'
from pathlib import Path
p = Path("lib/crewai-tools/pyproject.toml")
lines = p.read_text().splitlines()
for i in range(130, min(220, len(lines))+1):
print(f"{i:4}: {lines[i-1]}")
PY
echo
echo "== candidate test files importing k8s_agent_sandbox =="
rg -n "k8s_agent_sandbox|k8s-agent-sandbox|SandboxNotFoundError" lib/crewai-tools/src lib/crewai-tools/tests lib/crewai-tools -g '*.py' | head -200 || true
echo
echo "== dependency groups / pytest uv scripts =="
rg -n "dependency-groups|pytest|k8s" lib/crewai-tools/pyproject.toml -C 3 || trueRepository: crewAIInc/crewAI
Length of output: 14424
Wire k8s_agent_sandbox into the test environment.
The package extra is only defined, so these new K8s tool tests that directly import k8s_agent_sandbox can fail with ModuleNotFoundError when installed without k8s_agent_sandbox. Add the extra to the test/dev dependency group and/or use lazy imports like the production tools.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/crewai-tools/pyproject.toml` around lines 151 - 153, Update the test/dev
dependency configuration in pyproject.toml to include the existing
k8s_agent_sandbox extra, ensuring tests that import k8s_agent_sandbox have the
package installed in the test environment; alternatively, convert the affected
test imports to lazy imports consistent with the production tools.
Comment on lines
+51
to
+54
| path: str = Field( | ||
| ..., | ||
| description="The path inside the sandbox which is relative to a sandbox's working directory.", | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Enforce the documented sandbox-relative path boundary. Absolute paths and normalized ../ traversal are accepted, reaching shell-backed rm and mkdir outside the sandbox working directory. Validate paths centrally; also use -- before user-controlled rm/mkdir operands.
lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/file_tool.py#L51-L54: reject absolute paths and paths normalizing to..or beginning with../before dispatching any action.lib/crewai-tools/tests/tools/k8s_agent_sandbox/test_file_tool.py#L249-L274: change/app/some-pathto an expected failure and add traversal-path coverage.
📍 Affects 2 files
lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/file_tool.py#L51-L54(this comment)lib/crewai-tools/tests/tools/k8s_agent_sandbox/test_file_tool.py#L249-L274
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/file_tool.py`
around lines 51 - 54, Update the path validation around the FileTool path field
to reject absolute paths and paths that normalize to “..” or start with “../”
before dispatching any action; also place “--” before user-controlled operands
in shell-backed rm and mkdir commands. In
lib/crewai-tools/tests/tools/k8s_agent_sandbox/test_file_tool.py lines 249-274,
change the /app/some-path case to expect failure and add coverage for traversal
paths.
Comment on lines
+78
to
+85
| @model_validator(mode="after") | ||
| def _validate_action_args(self) -> "K8sAgentSandboxFileToolSchema": | ||
| if self.action == "append" and self.content is None: | ||
| raise ValueError( | ||
| "action='append' requires 'content'. Pass the chunk to append " | ||
| "in the 'content' field." | ||
| ) | ||
| return self |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject malformed base64 before modifying files.
base64.b64decode() silently discards invalid characters by default, so binary writes/appends can persist corrupted or empty content. Validate with validate=True in this schema validator and add invalid-payload tests for both actions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/file_tool.py`
around lines 78 - 85, The _validate_action_args validator must reject malformed
base64 payloads before any file write or append occurs. Validate provided
content using base64.b64decode with validate=True for both supported actions,
preserve the existing append-content requirement, and add invalid-payload tests
covering write and append.
Comment on lines
+44
to
+55
| def acquire_sandbox(self) -> "Sandbox": # type: ignore[no-any-unimported] | ||
| """ | ||
| Acquires a sandbox based on this implementation and returns it. | ||
| In order to be acquired again by someone else, it has to be | ||
| released first by the :meth:`release_sandbox` method. | ||
| """ | ||
| self._lock.acquire() | ||
| if self._closed: | ||
| self._lock.release() | ||
| raise RuntimeError("Attempt to acquire a sandbox from a closed helper.") | ||
| self._sandbox_acquired = True | ||
| return self._acquire_sandbox() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Lock is leaked if _acquire_sandbox() raises, unless the caller wraps the acquire call itself in try/finally.
self._sandbox_acquired = True is set, and the lock stays held, before self._acquire_sandbox() runs. If it raises, nothing here releases self._lock; the lock is only released because today's sole caller (base_tool._run) happens to wrap acquire_sandbox() inside its own try/finally with release_sandbox(). This isn't documented as a required contract, so any future caller that acquires outside a try block (the more natural pattern) will deadlock all subsequent acquire_sandbox() calls.
🔒 Proposed fix: release the lock on failure inside `acquire_sandbox()`
def acquire_sandbox(self) -> "Sandbox": # type: ignore[no-any-unimported]
self._lock.acquire()
if self._closed:
self._lock.release()
raise RuntimeError("Attempt to acquire a sandbox from a closed helper.")
- self._sandbox_acquired = True
- return self._acquire_sandbox()
+ try:
+ sandbox = self._acquire_sandbox()
+ except Exception:
+ self._lock.release()
+ raise
+ self._sandbox_acquired = True
+ return sandbox📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def acquire_sandbox(self) -> "Sandbox": # type: ignore[no-any-unimported] | |
| """ | |
| Acquires a sandbox based on this implementation and returns it. | |
| In order to be acquired again by someone else, it has to be | |
| released first by the :meth:`release_sandbox` method. | |
| """ | |
| self._lock.acquire() | |
| if self._closed: | |
| self._lock.release() | |
| raise RuntimeError("Attempt to acquire a sandbox from a closed helper.") | |
| self._sandbox_acquired = True | |
| return self._acquire_sandbox() | |
| def acquire_sandbox(self) -> "Sandbox": # type: ignore[no-any-unimported] | |
| """ | |
| Acquires a sandbox based on this implementation and returns it. | |
| In order to be acquired again by someone else, it has to be | |
| released first by the :meth:`release_sandbox` method. | |
| """ | |
| self._lock.acquire() | |
| if self._closed: | |
| self._lock.release() | |
| raise RuntimeError("Attempt to acquire a sandbox from a closed helper.") | |
| try: | |
| sandbox = self._acquire_sandbox() | |
| except Exception: | |
| self._lock.release() | |
| raise | |
| self._sandbox_acquired = True | |
| return sandbox |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/lifecycle_manager.py`
around lines 44 - 55, Update acquire_sandbox so failures from _acquire_sandbox()
release self._lock and restore the unacquired state before propagating the
exception. Keep the existing closed-helper handling and successful acquisition
behavior unchanged, ensuring every lock acquired by acquire_sandbox is either
retained for a successful sandbox or released on failure.
Comment on lines
+105
to
+108
| from crewai_tools import ( | ||
| K8sAgentSandboxToolSandboxSettings as SandboxSettings, | ||
| K8sAgentSandboxToolset, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Import the tools used by each lifecycle example. Each snippet instantiates K8sAgentSandboxExecTool and K8sAgentSandboxFileTool without importing them.
lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/README.md#L105-L108: importK8sAgentSandboxExecToolandK8sAgentSandboxFileTool.lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/README.md#L124-L127: importK8sAgentSandboxExecToolandK8sAgentSandboxFileTool.lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/README.md#L144-L147: importK8sAgentSandboxExecToolandK8sAgentSandboxFileTool.
📍 Affects 1 file
lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/README.md#L105-L108(this comment)lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/README.md#L124-L127lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/README.md#L144-L147
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/README.md` around
lines 105 - 108, Update each lifecycle example import block in
lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/README.md at lines
105-108, 124-127, and 144-147 to import K8sAgentSandboxExecTool and
K8sAgentSandboxFileTool alongside the existing K8sAgentSandboxToolset and
SandboxSettings imports, so every snippet can instantiate the tools it uses.
Comment on lines
+163
to
+175
| ### `K8sAgentSandboxExecTool` | ||
| - `command: str` — shell command to run. | ||
| - `timeout: float | None` — seconds. | ||
|
|
||
| ### `K8sAgentSandboxFileTool` | ||
| - `action: "read" | "write" | "append" | "list" | "delete" | "mkdir" | "info" | "exists"` | ||
| - `path: str` — relative path inside the sandbox. | ||
| - `content: str | None` — required for `append`; optional for `write`. | ||
| - `binary: bool` — if `True`, `content` is base64 on write / returned as base64 on read. | ||
|
|
||
| ### `K8sAgentSandboxPythonTool` | ||
| - `code: str` — source to execute. | ||
| - `timeout: float | None` — seconds. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Synchronize the argument reference with the schemas.
info is not a supported file action. Also, the execution and Python timeouts are int fields with a default of 60, not float | None; document the file tool timeout as well.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/README.md` around
lines 163 - 175, Update the K8sAgentSandboxFileTool action list to remove the
unsupported "info" value, and synchronize timeout documentation for
K8sAgentSandboxExecTool and K8sAgentSandboxPythonTool with the schemas as int
values defaulting to 60. Add the file tool’s timeout argument and document its
matching schema type and default.
Comment on lines
+31
to
+45
| @property | ||
| def client(self) -> "SandboxClient": # type: ignore[no-any-unimported] | ||
| if self._client is not None: | ||
| return self._client | ||
|
|
||
| # from k8s_agent_sandbox.sandbox_client import SandboxClient | ||
| kas_client_sandbox_module = lazy_import_k8s_agent_sandbox("sandbox_client") | ||
|
|
||
| client: "SandboxClient" = kas_client_sandbox_module.SandboxClient( # type: ignore[no-any-unimported] | ||
| connection_config=self.connection_config, | ||
| tracer_config=self.tracer_config, | ||
| cleanup=self.cleanup, | ||
| ) | ||
| self._client = client | ||
| return self._client |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Race condition (TOCTOU) in lazy SandboxClient initialization.
The check-then-set pattern (if self._client is not None: return ... then later self._client = client) is not synchronized. If two threads call .client concurrently before it's cached (e.g., two K8sAgentSandboxLifecycleManagers sharing one K8sAgentSandboxToolClientSettings are constructed in parallel), both will construct a separate SandboxClient, and one gets silently discarded — potentially leaking an underlying tunnel/connection the client opened.
🔒 Proposed fix: guard the lazy init with a lock
+from threading import Lock
+
`@dataclass`
class K8sAgentSandboxToolClientSettings: # type: ignore[no-any-unimported]
...
_client: "SandboxClient | None" = field(default=None, init=False, repr=False) # type: ignore[no-any-unimported]
+ _client_lock: Lock = field(default_factory=Lock, init=False, repr=False)
`@property`
def client(self) -> "SandboxClient": # type: ignore[no-any-unimported]
- if self._client is not None:
- return self._client
-
- # from k8s_agent_sandbox.sandbox_client import SandboxClient
- kas_client_sandbox_module = lazy_import_k8s_agent_sandbox("sandbox_client")
-
- client: "SandboxClient" = kas_client_sandbox_module.SandboxClient( # type: ignore[no-any-unimported]
- connection_config=self.connection_config,
- tracer_config=self.tracer_config,
- cleanup=self.cleanup,
- )
- self._client = client
- return self._client
+ with self._client_lock:
+ if self._client is not None:
+ return self._client
+
+ kas_client_sandbox_module = lazy_import_k8s_agent_sandbox("sandbox_client")
+ self._client = kas_client_sandbox_module.SandboxClient( # type: ignore[no-any-unimported]
+ connection_config=self.connection_config,
+ tracer_config=self.tracer_config,
+ cleanup=self.cleanup,
+ )
+ return self._client🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/crewai-tools/src/crewai_tools/tools/k8s_agent_sandbox/settings.py` around
lines 31 - 45, Make the lazy initialization in the client property thread-safe
by adding a lock to the settings object and guarding both the cached-client
check and SandboxClient construction/assignment with it. Preserve the fast
return for an already initialized client, and ensure concurrent callers share
the single cached instance without creating discarded connections.
Comment on lines
+93
to
+107
| with patch.object( | ||
| sample_toolset, | ||
| "lifecycle_manager", | ||
| MagicMock(wraps=sample_toolset.lifecycle_manager), | ||
| ) as m: | ||
| with pytest.raises(Exception, match="some error"): | ||
| tool.run( | ||
| "some-arg", | ||
| test_kwarg="some-kwarg", | ||
| ) | ||
|
|
||
| if lifecycle_mode_sandbox_termination_expected: | ||
| assert mock_sandbox.terminate.called | ||
| else: | ||
| assert not mock_sandbox.terminate.called |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert the lifecycle release directly.
The wrapped manager is never asserted. Persistent and attach modes can pass without proving release_sandbox() ran after the exception. Add m.release_sandbox.assert_called_once_with() after tool.run() raises.
As per coding guidelines, **/tests/**/*.py must “Write unit tests for new functionality that focus on behavior rather than implementation details.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/crewai-tools/tests/tools/k8s_agent_sandbox/test_base_tool.py` around
lines 93 - 107, Update the exception test around tool.run to assert the wrapped
lifecycle manager directly: after pytest.raises completes, call
m.release_sandbox.assert_called_once_with(). Keep the existing termination
assertions unchanged so the test verifies release behavior across persistent and
attach modes.
Source: Coding guidelines
Comment on lines
+33
to
+36
| assert mock_sandbox.files.write.called | ||
|
|
||
| assert mock_sandbox.commands.run.call_args.args[0].startswith("python3 /tmp",) | ||
| assert 0 <= mock_sandbox.commands.run.call_args.kwargs["timeout"] <= 120 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert the code payload, not the temporary-path detail.
Verify files.write receives b"some-code"; the current /tmp prefix assertion does not prove the requested code is executed.
As per coding guidelines, **/tests/**/*.py must “Write unit tests for new functionality that focus on behavior rather than implementation details.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/crewai-tools/tests/tools/k8s_agent_sandbox/test_python_tool.py` around
lines 33 - 36, Update the test around mock_sandbox.files.write and
mock_sandbox.commands.run to assert that files.write receives the code payload
b"some-code". Remove the command prefix assertion tied to the temporary path,
while preserving the timeout validation and other behavior-focused checks.
Source: Coding guidelines
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
This PR introduces set of tools that allows interaction with the Kubernetes Agent Sandbox .
The API and overall architecture was inspired by already existing sandbox tools, like E2B, Daytona and etc. All information about all tools and their usage can be found in the README file.
Introduced changes
K8sAgentSandboxExecToolfor executing shell commands within a sandbox.K8sAgentSandboxFileToolfor managing files (read, write, delete, etc.) in a sandbox.K8sAgentSandboxPythonToolfor running Python code in a sandbox environment.pyproject.tomlto include thek8s-agent-sandboxas a dependency.uv.lock.