fix(agent): make _last_resolved_tool_names thread-local to prevent cross-session tool bleed

[necoweb3]

What

_last_resolved_tool_names in model_tools.py is a process-global List[str] that is overwritten on every call to get_tool_definitions() (line 341 on cache hit, line 536 on fresh compute). When handle_function_call is called for execute_code with enabled_tools=None (line 1307), the sandbox falls back to this global.

The gateway runs 10 concurrent agent sessions via ThreadPoolExecutor. Session A calls get_tool_definitions() (overwriting the global with session A's toolset), and session B simultaneously calls handle_function_call("execute_code", ...) with enabled_tools=None. Session B's sandbox picks up session A's tool list -- granting a restricted session access to tools it should not have (e.g. terminal, web_search, delegate_task).

Fix

Replaced the process-global List[str] with threading.local() storage. Each session's thread gets its own copy via _get_last_resolved_tool_names() / _set_last_resolved_tool_names() helpers.

• model_tools.py: Declaration changed to threading.local(), getter/setter functions added. Cache-hit path (line 341) and fresh-compute path (line 536) now call _set_last_resolved_tool_names(). execute_code fallback (line 1307) calls _get_last_resolved_tool_names().
• tools/delegate_tool.py: All 4 references updated to use getter/setter.
• tests/tools/test_delegate.py: Test assertions updated.
• tests/tools/test_tool_search.py: Test assertions updated.

PR #34451 attempted this fix but was closed without merge.

Why

Cross-session privilege escalation: a restricted user session can inherit tools from another user's session context in the default gateway configuration.

How to Test

python -c "
import model_tools, threading
model_tools._set_last_resolved_tool_names(['tool_a'])
results = {}
def worker(name):
    model_tools._set_last_resolved_tool_names([f'{name}_tool'])
    import time; time.sleep(0.01)
    results[name] = model_tools._get_last_resolved_tool_names()
threads = [threading.Thread(target=worker, args=(f's{i}',)) for i in range(5)]
for t in threads: t.start()
for t in threads: t.join()
for name, val in results.items():
    assert val == [f'{name}_tool']
print('PASS: no cross-session bleed')
"
scripts/run_tests.sh tests/tools/test_delegate.py -q
scripts/run_tests.sh tests/tools/test_tool_search.py -q

[egilewski]

suggesting changes

The implementation replaces the process-global fallback tool-name list with per-thread storage and converts every repository consumer, and focused probes show that it prevents a restricted gateway worker from observing a concurrent privileged worker's tool names. The PR patch also applies cleanly to the current-main tree. However, the committed tests only rename accesses in existing single-thread tests; none reproduces the cross-thread security failure or proves the new isolation property. Because this is a tool-capability boundary and a global-backed getter/setter would pass the current tests while retaining the vulnerability, a deterministic concurrent regression test is required.

• [P2] Add a regression test that exercises concurrent thread isolation (tests/tools/test_delegate.py:554)
  The tests at this class set and read the resolved names on one thread and therefore validate delegation restoration, not the cross-session race this PR claims to fix. Merely wrapping the old process-global list in the new getter/setter API would satisfy these assertions while still allowing a restricted session to read a privileged session's fallback capabilities. This security-boundary change needs a deterministic barrier-controlled test with two worker threads: each thread sets a disjoint capability list, both remain live concurrently, and each must read only its own list. The test should also assert that a third, unset thread receives the empty default.
  Remediation: Add a concurrent regression test using threading.Barrier (or equivalent deterministic synchronization) that demonstrates distinct per-thread values and an empty value in an unset thread. Keep the existing same-thread delegation success and failure restoration cases as the negative/compatibility coverage.

Security evidence:

• trust boundary: Untrusted model-selected execute_code requests cross from a session's advertised tool set into registry.dispatch. The security-sensitive fallback is used when handle_function_call receives no explicit enabled_tools list; it must never inherit a more privileged session's resolved names. Gateway sessions execute blocking agent runs concurrently in a shared ThreadPoolExecutor, while delegated children may use additional worker threads. The privileged sink is the enabled_tools value passed to execute_code dispatch, which controls which sandbox tool imports/calls are exposed.
• source/sink/invariant: Sources are get_tool_definitions results derived from enabled_toolsets, disabled_toolsets, registry checks, and dynamic assembly. The validators are that filtering/assembly and the caller's agent.valid_tool_names determine the granted set. The sink is handle_function_call's execute_code branch and delegate_task's save/restore flow. The invariant is that fallback names read on one active gateway worker equal only the latest definitions resolved on that same worker; another concurrent worker must not widen them. Explicit enabled_tools remains authoritative.
• current-main reproduction: Loaded model_tools.py directly from current main 0fa5e41 and ran a barrier-controlled two-thread probe against its actual module-level _last_resolved_tool_names. After the restricted thread stored [read_file] and the privileged thread stored [terminal, execute_code], the restricted thread read [terminal, execute_code], reproducing the cross-session overwrite. Source inspection also confirmed current main uses this process-global list in get_tool_definitions and execute_code fallback.
• PR-head or patch-replay validation: On reviewed head 67a47b9, a barrier-controlled probe set disjoint lists in two threads and observed restricted=[read_file], privileged=[terminal, execute_code], while the unset main thread returned []. The base-to-head binary patch passed git apply --cached --check against current main; only expected line offsets were reported in model_tools.py, tests/tools/test_tool_search.py, and tools/delegate_tool.py.
• positive/negative cases: Positive case: simultaneous restricted and privileged workers retained distinct tool-name lists on PR head. Negative case: an unset thread returned the empty default rather than either worker's capabilities. Existing committed tests retain same-thread parent names after successful and failed delegation and check that scoped tool-search activity does not introduce terminal. Missing committed case: no test currently creates concurrent threads, so the original vulnerable storage design is not rejected by a behavioral isolation assertion.
• residual bypass search: Repository-wide search found direct resolved-name storage/read consumers only in model_tools.py, tools/delegate_tool.py, tests/tools/test_delegate.py, and tests/tools/test_tool_search.py; the PR converts all production consumers. Normal AIAgent tool dispatch in agent/tool_executor.py and agent/agent_runtime_helpers.py passes agent.valid_tool_names explicitly, preserving the stronger session-owned path. Gateway run_sync work is pinned for its duration to one shared-executor worker, and delegate save/restore copies lists at its boundaries. No residual direct production access to the removed global name was found.
• reviewer validation: Independently inspected the base-to-head diff, current-main and PR-head source, gateway executor scheduling, execute_code dispatch, delegation save/restore, and all symbol references. Reproduced the current-main overwrite and verified PR-head separation with focused local probes. Full pytest execution was attempted but the checkout has no pytest-capable Python environment; /usr/bin/python reported 'No module named pytest'. Import probes emitted missing optional httpx/plugin warnings but completed and exercised the target storage.

Uncertainty: The complete focused pytest suites could not run because the checkout has no pytest-capable environment; no committed concurrent test demonstrates the security invariant, so future refactors would not be protected from reintroducing process-global storage.

Signed: GPT-5.6-sol-xhigh in Codex

[teknium1]

Thanks for isolating the mutable fallback state. Current main still writes _last_resolved_tool_names globally in model_tools.py:341-342 and model_tools.py:536-537, and consumes it for an execute_code call without explicit enabled_tools at model_tools.py:1324-1333.

Problems

• The changed tests only rename same-thread accesses (tests/tools/test_delegate.py:562 in this PR). They would pass if the getter/setter still wrapped a process-global list. Add a deterministic threading.Barrier regression with two disjoint worker values and an unset-thread [] assertion.
• The claimed default-gateway route needs a more precise reproduction: normal agent execution passes agent.valid_tool_names explicitly in both branches of agent/tool_executor.py:1682-1686 and agent/tool_executor.py:1752-1756. Test the real enabled_tools=None fallback path or narrow the claim to that compatibility fallback.

Suggested changes

• Reconcile the conflict against current main and retain the current delegate lifecycle locations in tools/delegate_tool.py:1983-1985, :2566-2568, and :2621-2626.

Automated hermes-sweeper review.

[teknium1]

This remains a same-thread restoration test, so a process-global getter/setter would still pass. Please add a barrier-controlled two-thread regression with disjoint values plus an unset-thread [] assertion to protect the isolation property.