feat(sandbox): reuse e2b container across requests when metadata.session_id is set - #31688
Conversation
…ion_id is set When a client passes `metadata.session_id` in a /chat/completions request alongside a code_interpreter tool, the proxy now routes all requests sharing that session_id to the same sandbox container. State (variables, imports, installed packages) persists across requests within the session. Without a session_id the existing ephemeral behavior is unchanged: one container per agentic loop, deleted immediately after. The sandbox key is derived from session_id rather than a per-request UUID. The cleanup and post-loop hooks skip deletion for session-scoped containers. TTL-based pruning (15 min idle) still applies and refreshes on every use, so an active session never expires mid-use. The session_id-scoped key is registered in all_litellm_params and the proxy strip-list so it never leaks to the upstream LLM provider.
|
|
Greptile SummaryThis PR adds sticky E2B code-interpreter sandboxes keyed by
Confidence Score: 4/5The sticky-session implementation needs tenant-scoped cache keys before it is safe to merge. The changed code is small and covered by targeted tests, but the session cache key currently omits caller context, allowing unrelated callers with the same session identifier to share one sandbox. litellm/integrations/code_interpreter_interception/handler.py
 T-Rex LogsWhat T-Rex did
Reviews (1): Last reviewed commit: "feat(sandbox): reuse e2b container acros..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| kwargs[_SANDBOX_KEY] = session_id | ||
| kwargs[_SESSION_SCOPED_KEY] = True |
There was a problem hiding this comment.
Using the raw client-supplied metadata.session_id as the process-wide sandbox cache key lets any caller who guesses or reuses the same string attach to another caller's E2B container. The cache at self._container_cache is shared for the logger instance, and there is no user/team/key component in this key, so requests with session_id="demo-A" from different tenants share code state and files.
Artifacts
Repro: focused pytest harness for cross-tenant session cache reuse
- Contains supporting evidence from the run (text/x-python; charset=utf-8).
Repro: verbose pytest output showing shared raw cache key and shared container
- Keeps the command output available without making the summary code-heavy.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| kwargs[_SANDBOX_KEY] = uuid.uuid4().hex | ||
| session_id = _extract_session_id(kwargs) | ||
| if session_id: | ||
| kwargs[_SANDBOX_KEY] = session_id |
There was a problem hiding this comment.
High: Cross-user sandbox reuse
metadata.session_id is client-controlled, but it becomes the process-wide sandbox cache key without being scoped to the authenticated key, user, or team. A logged-in user who knows or collides with another caller's session ID can run code in that caller's long-lived sandbox and read files, variables, or other state left there; derive the cache key from server-side identity plus the session ID instead of using the raw metadata value.
There was a problem hiding this comment.
Fixed in commit 659c9c9. The cache key is now f"{user_api_key_hash}:{session_id}" when a proxy API key is present (_extract_identity reads kwargs["user_api_key_hash"], which is the server-minted hash set by proxy auth middleware). Two tenants supplying the same session_id therefore get separate cache entries and separate sandboxes. For non-proxy use where no hash is available, the bare session_id is used as before (single-tenant context, no cross-user risk). A dedicated test test_sandbox_key_scoped_to_api_key_hash_isolates_users verifies that two callers with different hashes but the same session_id get distinct sandbox keys.
PR overviewThis pull request adds session-aware reuse of E2B code interpreter sandboxes, allowing requests with There are still two open security issues in the session-scoped sandbox caching logic. The most significant concern is that a client-controlled session ID is used as the cache key without server-side identity scoping, which could let one authenticated caller attach to another caller’s reused sandbox if the session ID is known or collides. A second open issue allows concurrent requests for the same new session to bypass the intended per-identity sandbox cap and leave overwritten containers untracked. One issue has already been addressed, but these remaining items leave the PR with a clear cross-user isolation risk. Open issues (2)
Fixed/addressed: 1 · PR risk: 7/10 |
…identity LRU cap
Two security issues addressed:
1. Cross-user sandbox isolation: the session_id supplied by the client is now
combined with the server-minted user_api_key_hash to form the cache key
(format: "{hash}:{session_id}" when authenticated, bare session_id for
non-proxy use). Two tenants sharing the same session_id no longer share a
sandbox.
2. Bounded session allocation: each API key identity is capped at
_SESSION_SCOPED_PER_IDENTITY_CAP (10) live session-scoped containers. When
a new session is opened beyond the cap, the least-recently-used entry for
that identity is evicted and its sandbox deleted, preventing unbounded
accumulation via rotating session IDs.
The container cache tuple gains a fourth element (identity: str | None) so
eviction can filter by identity without parsing key formats. Tests added for
both properties.
|
@veria-ai |
1 similar comment
|
@veria-ai |
| @@ -153,6 +153,7 @@ def parse_cache_control(cache_control): | |||
| "_code_interpreter_interception_active", | |||
There was a problem hiding this comment.
can we not group these variables code_interpreter_variables that way we can re-use the enum across diff places
| self._container_cache[cache_key] = (cached[0], cached[1], time.time(), cached[3]) | ||
| return cached[0], cached[1] | ||
|
|
||
| container, params = await self._create_container() |
There was a problem hiding this comment.
High: Concurrent sandbox allocation bypasses the session cap
metadata.session_id is client-controlled, and concurrent requests for the same new session can all pass the cache miss check before any of them stores an entry here. Each request creates a sandbox, later writes over the same cache key, and because session-scoped plans skip both cleanup hooks, the overwritten containers are no longer tracked or deleted; an authenticated caller can repeat this to run far more sandboxes than _SESSION_SCOPED_PER_IDENTITY_CAP. Add a per-cache-key creation lock or reserve the key before awaiting _create_container(), and make sure any displaced container is explicitly deleted.
6c21029
into
litellm_internal_staging
Covers the new sticky-session support in the code interpreter interceptor (BerriAI/litellm#31688): curl + OpenAI SDK examples, per-key hash isolation, 10-session LRU cap, TTL reset on access, and a Notes update so the "one sandbox per request" line no longer contradicts the feature. Co-authored-by: Krrish Dholakia <krrishdholakia@berri.ai>
…ion_id is set (BerriAI#31688) * feat(sandbox): reuse e2b container across requests when metadata.session_id is set When a client passes `metadata.session_id` in a /chat/completions request alongside a code_interpreter tool, the proxy now routes all requests sharing that session_id to the same sandbox container. State (variables, imports, installed packages) persists across requests within the session. Without a session_id the existing ephemeral behavior is unchanged: one container per agentic loop, deleted immediately after. The sandbox key is derived from session_id rather than a per-request UUID. The cleanup and post-loop hooks skip deletion for session-scoped containers. TTL-based pruning (15 min idle) still applies and refreshes on every use, so an active session never expires mid-use. The session_id-scoped key is registered in all_litellm_params and the proxy strip-list so it never leaks to the upstream LLM provider. * fix(sandbox): scope session sandbox key to API key identity; add per-identity LRU cap Two security issues addressed: 1. Cross-user sandbox isolation: the session_id supplied by the client is now combined with the server-minted user_api_key_hash to form the cache key (format: "{hash}:{session_id}" when authenticated, bare session_id for non-proxy use). Two tenants sharing the same session_id no longer share a sandbox. 2. Bounded session allocation: each API key identity is capped at _SESSION_SCOPED_PER_IDENTITY_CAP (10) live session-scoped containers. When a new session is opened beyond the cap, the least-recently-used entry for that identity is evicted and its sandbox deleted, preventing unbounded accumulation via rotating session IDs. The container cache tuple gains a fourth element (identity: str | None) so eviction can filter by identity without parsing key formats. Tests added for both properties.
3 participants
Relevant issues
Linear ticket
Pre-Submission checklist
Screenshots / Proof of Fix
See QA runbook in `qa_sticky_session.sh`. Run the proxy with:
Then run the script:
The script verifies that: (1) a variable set in one request is visible in the next when session_id is stable, (2) changing session_id starts a fresh sandbox, (3) omitting session_id still works as an ephemeral per-request sandbox.
Type
🆕 New Feature
Changes
Adds sticky session support to the code interpreter interception layer: when a client passes
metadata.session_id, the same e2b sandbox container is reused across sequential HTTP requests for that session instead of being torn down after each agentic loop.Two security properties are enforced:
Cross-tenant isolation. The
session_idsupplied by the client is combined with the server-minteduser_api_key_hashto form the process-wide cache key ({hash}:{session_id}when authenticated via proxy, baresession_idfor non-proxy use). Two API keys that happen to send the samesession_idtherefore get separate sandboxes.Bounded allocation per identity. Each API key is capped at 10 live session-scoped containers (
_SESSION_SCOPED_PER_IDENTITY_CAP). When a new session would exceed the cap, the least-recently-used session for that identity is evicted and its sandbox deleted. This prevents a caller from rotating session IDs to accumulate an unbounded number of live e2b containers.Implementation details:
async_pre_call_deployment_hook: whensession_idis present, sets_SANDBOX_KEY = "{identity}:{session_id}"and marks_SESSION_SCOPED_KEY = True; otherwise mints a random UUID as before_get_or_create_container: accepts an optionalidentityarg; refresheslast_accessedon cache hit (so active sessions do not expire); calls_evict_lru_session_if_over_capbefore inserting a new session-scoped entry_evict_lru_session_if_over_cap: filters cache entries by identity (stored as the 4th element of the cache tuple), evicts the one with the oldestlast_accessedis_session_scopedis set in the plan metadata_prune_expired_cache) resets on each access, so an actively-used session does not expire mid-conversation_SESSION_SCOPED_KEYis stripped from the upstream LLM request viaagentic_loop_internal_litellm_paramsand from the inbound proxy request body