Skip to content

fix(executors): add best-effort POSIX timeout for Python skill scripts, enforce timeout_seconds in ContainerCodeExecutor#5083

Draft
caohy1988 wants to merge 1 commit intogoogle:mainfrom
caohy1988:fix/code-executor-timeout-gaps
Draft

fix(executors): add best-effort POSIX timeout for Python skill scripts, enforce timeout_seconds in ContainerCodeExecutor#5083
caohy1988 wants to merge 1 commit intogoogle:mainfrom
caohy1988:fix/code-executor-timeout-gaps

Conversation

@caohy1988
Copy link
Copy Markdown

Summary

Fixes #5079 — timeout enforcement gaps in code executors.

  • Gap 1 (Python skill scripts): RunSkillScriptTool's script_timeout now applies to Python scripts via signal.SIGALRM on POSIX. Guarded by hasattr(signal, 'SIGALRM') so Windows is unaffected (no regression). This is cooperative/best-effort — scripts that catch TimeoutError can continue.
  • Gap 2 (ContainerCodeExecutor): execute_code() now uses exec_create/exec_start in a daemon thread with thread.join(timeout=...) to honor timeout_seconds. On timeout, best-effort kill -9 of the exec PID inside the container. When timeout_seconds=None, waits indefinitely (backward compatible).
  • Gap 3 (UnsafeLocalCodeExecutor): Expanded docstring with security guidance (no sandboxing warning, recommended use cases, timeout mechanism).

Limitations

  • Gap 1: SIGALRM timeout is cooperative on POSIX and absent on Windows. Hard enforcement depends on the underlying code executor's own process-level timeout.
  • Gap 2: The daemon thread may linger on the Docker socket after timeout until the container is cleaned up. A stronger approach (container restart) is out of scope.

Test plan

  • test_execute_script_python_includes_timeout — verifies generated wrapper contains signal.alarm and hasattr guard
  • test_python_script_timeout_fires_with_real_executor — POSIX integration test proving SIGALRM kills a sleeping script (skipped on Windows)
  • TestContainerTimeout::test_timeout_returns_error — exec_start that blocks is killed after timeout
  • TestContainerTimeout::test_no_timeout_waits_indefinitely — None timeout = backward compat
  • TestContainerTimeout::test_normal_execution_within_timeout — fast command within timeout
  • TestContainerTimeout::test_exec_start_exception_propagated — Docker API errors re-raised
  • Full regression: 142 tests pass (test_skill_toolset.py + code_executors/)

🤖 Generated with Claude Code

…rCodeExecutor

- Add best-effort SIGALRM-based timeout for Python skill scripts on POSIX
  (guarded by hasattr so Windows is unaffected)
- Replace ContainerCodeExecutor.exec_run() with thread-based exec_create/
  exec_start to honor timeout_seconds, with best-effort kill on timeout
- Expand UnsafeLocalCodeExecutor docstring with security guidance

Closes google#5079

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@google-cla
Copy link
Copy Markdown

google-cla bot commented Mar 31, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Production-Readiness for Run Skill Script Tool

1 participant