fix(tools): close execute_code sandbox allow-list fail-open

[petrichor-op]

What does this PR do?

The execute_code sandbox computes the set of Hermes tools its child
process may reach over RPC as the intersection of SANDBOX_ALLOWED_TOOLS
with the session's enabled tools. Both execution paths then applied the
fallback if not sandbox_tools: sandbox_tools = SANDBOX_ALLOWED_TOOLS,
which conflated two distinct cases: the caller passing enabled_tools=None
(the legacy "allow everything" contract) and the caller passing a non-None
list whose intersection with the sandbox allow-list happens to be empty.

The second case is a real, reachable configuration: a session that enables
execute_code but deliberately disables terminal, file, and web tools. In
that situation the intersection is empty, so the fallback re-granted the
full set (terminal, write_file, patch, read_file, search_files, web_search,
web_extract) to the sandbox. The schema layer in model_tools.py already
reports zero sandbox tools to the model in this case, so the runtime
silently disagreed with the advertised contract: a script could simply
import and call terminal() despite the session forbidding it, defeating the
enabled_tools guard that exists specifically so a restricted or subagent
session cannot widen its own tool set.

This change makes enabled_tools is None the only path that falls back to
the full allow-list. Any explicit list, including an empty or disjoint one,
is treated as authoritative and the sandbox receives only the intersection.

Related Issue

N/A

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• 🔒 Security fix

Changes Made

• tools/code_execution_tool.py: replace the fail-open if not sandbox_tools fallback in both _execute_remote and the local path of execute_code with an explicit enabled_tools is None check, so a specified-but-disjoint (or empty) allow-list yields no sandbox tools instead of all of them.
• tests/tools/test_code_execution_modes.py: add test_disjoint_enabled_tools_grants_no_sandbox_tools and test_empty_enabled_tools_grants_no_sandbox_tools to the cross-mode security-invariant suite; extend its _run helper to accept an explicit enabled_tools argument.
• tests/tools/test_code_execution.py: update the two edge-case tests that asserted the old fail-open behavior to assert the corrected fail-closed semantics (empty and non-overlapping lists expose no sandbox tools; enabled_tools=None still grants all).

How to Test

1. Reproduce the regression by reverting the enabled_tools is None guard back to if not sandbox_tools: sandbox_tools = SANDBOX_ALLOWED_TOOLS; the new tests fail with terminal: True, demonstrating that a restricted session still receives terminal access.
2. Restore the fix and run the targeted suites:
   scripts/run_tests.sh tests/tools/test_code_execution.py tests/tools/test_code_execution_modes.py -q
3. Confirm that execute_code with enabled_tools=["execute_code", "todo"] (or []) exposes none of the sandbox tools, while enabled_tools=None continues to expose the full allow-list.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS 15 (Darwin 25.5.0)

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

No UI changes. The corrected behavior is captured by the regression tests described above.

[liuhao1024]

✅ Verification — clean security fix (fail-open sandbox allow-list)

The fix correctly distinguishes enabled_tools is None (legacy unscoped, grant all) from an explicit non-None list (even empty or disjoint). The old code treated an empty or non-overlapping list as "grant all" — a classic fail-open that silently re-enabled terminal(), write_file(), patch() in sessions that deliberately scoped them out.

Code change: Both _execute_remote() and execute_code() in tools/code_execution_tool.py now use if enabled_tools is None to guard the legacy fallback, and the else branch computes SANDBOX_ALLOWED_TOOLS & set(enabled_tools) without the previous if not sandbox_tools bail-out that triggered the fail-open.

Test coverage:

• test_empty_enabled_tools_grants_no_tools — explicit empty list → no tools
• test_nonoverlapping_tools_grants_no_tools — disjoint list → no tools
• test_disjoint_enabled_tools_grants_no_sandbox_tools — cross-mode regression (strict + project)
• test_empty_enabled_tools_grants_no_sandbox_tools — empty list in strict mode

The schema layer already advertises zero sandbox tools to the model when the list is disjoint, so this brings the runtime enforcement in line with what the model was told it has. Clean fix — no issues found.

[teknium1]

Thanks for the focused authorization-boundary fix. Current main still restores the full sandbox allow-list after an empty intersection in both _execute_remote() (tools/code_execution_tool.py:929-932) and the local path (tools/code_execution_tool.py:1193-1197). This conflicts with the per-session schema, which advertises only SANDBOX_ALLOWED_TOOLS & available_tool_names (model_tools.py:453-463), and with the executor's propagation of the session's valid tools (agent/tool_executor.py:1481).

Suggested changes

• Please add a focused remote/file-RPC regression test for the changed _execute_remote() selector. Existing remote tests in tests/tools/test_code_execution.py:143-217 exercise transport setup but do not inspect the generated hermes_tools.py; the new mode tests exercise local strict/project execution. Capture the payload sent through _ship_file_to_remote and assert explicit empty/disjoint scopes omit sandbox stubs while None preserves the legacy full allow-list.

Automated hermes-sweeper review.

[andrexibiza]

Interlock with #82243

#82243 now centralizes local and remote sandbox selection in _resolve_sandbox_tools(), but its current helper preserves this PR's exact fail-open: any explicit empty or disjoint list returns all seven direct tools. A normal enabled_toolsets=["code_execution"] session therefore advertises only execute_code yet receives terminal, file-write, patch, and web helpers inside the sandbox.

This PR remains the canonical fail-closed implementation, although its branch now conflicts with the newer helper. Please carry the semantic fix into #82243 rather than treating the new helper as resolving this report. The #47494/#6614 maintainer objection still needs an explicit policy decision: either None alone gets the compatibility fallback, or execute_code must be documented and modeled as an umbrella grant for every direct helper.