fix: move E2b-Sandbox-Id/Port headers from control plane connect to data plane ConnectionConfig#1272
fix: move E2b-Sandbox-Id/Port headers from control plane connect to data plane ConnectionConfig#1272WoodenStone wants to merge 3 commits intoe2b-dev:mainfrom
Conversation
…ata plane ConnectionConfig
WoodenStone
requested review from
ValentaTomas,
jakubno and
mishushakov
as code owners
🦋 Changeset detectedLatest commit: 65c45bc The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if envd_access_token is not None and not isinstance(envd_access_token, Unset): | ||
| sandbox_headers["X-Access-Token"] = envd_access_token | ||
|
|
||
| sandbox_headers["E2b-Sandbox-Id"] = sandbox.sandbox_id | ||
| sandbox_headers["E2b-Sandbox-Port"] = str(ConnectionConfig.envd_port) | ||
|
|
||
| connection_config = ConnectionConfig( | ||
| extra_sandbox_headers=sandbox_headers, | ||
| **opts, |
There was a problem hiding this comment.
🟣 In _cls_connect_sandbox (both async sandbox_async/main.py:865-873 and sync sandbox_sync/main.py:860-868), envd_access_token and traffic_access_token are passed directly from the raw Sandbox API model to cls() without sanitizing the UNSET sentinel, unlike the _create_sandbox path which applies isinstance(token, str) else None. When a sandbox is connected with secure=False, UNSET gets stored as self.__envd_access_token, causing download_url()/upload_url() to raise ValueError because use_signature = self._envd_access_token is not None evaluates to True for the UNSET sentinel. Apply the same sanitization pattern from _create_sandbox to fix this: envd_access_token if isinstance(envd_access_token, str) else None.
Extended reasoning...
The bug: In _cls_connect_sandbox, the envd_access_token (typed Union[Unset, str]) and traffic_access_token (typed Union[None, Unset, str]) are taken directly from the raw Sandbox API response model and forwarded unsanitized to cls(...) as keyword arguments. The _create_sandbox path correctly sanitizes these via isinstance(res.parsed.envd_access_token, str) else None before constructing the response object, but _cls_connect_sandbox skips this step.
Code path that triggers it: When Sandbox.connect() or AsyncSandbox.connect() is called for a sandbox created with secure=False, the API response omits the envdAccessToken field. The generated Sandbox model sets sandbox.envd_access_token = UNSET by default (Union[Unset, str] = UNSET per the model definition). This sentinel is then passed to SandboxBase.init which stores it as self.__envd_access_token = UNSET.
Why existing code does not prevent it: The code in _cls_connect_sandbox correctly guards the header assignment — if envd_access_token is not None and not isinstance(envd_access_token, Unset) — but the same raw value is still forwarded unsanitized to cls() on the very next line.
Impact: Once UNSET is stored in SandboxBase, calls to download_url() or upload_url() fail. The check use_signature = self._envd_access_token is not None evaluates to True because UNSET is not None. This causes get_signature() to be invoked with UNSET as the token. Inside get_signature(), the guard "if not envd_access_token:" evaluates to True because Unset.bool returns False, so it raises ValueError("Access token is not set and signature cannot be generated!"). Sandboxes connected via connect() with secure=False cannot use file download or upload URL generation, while the equivalent create(secure=False) path works correctly.
Step-by-step proof:
- User calls Sandbox.connect(sandbox_id) where sandbox was created with secure=False.
- _cls_connect_sandbox receives sandbox where sandbox.envd_access_token is UNSET.
- Header guard works correctly: isinstance check prevents X-Access-Token from being set.
- BUT cls(..., envd_access_token=envd_access_token, ...) is called with envd_access_token=UNSET.
- SandboxBase.init stores self.__envd_access_token = UNSET.
- User calls sandbox.files.download_url("/file") — use_signature = UNSET is not None — True.
- get_signature(UNSET) is called — if not UNSET: — if not False: — True — raises ValueError.
Fix: Apply the same isinstance-str sanitization used in _create_sandbox:
envd_access_token = sandbox.envd_access_token if isinstance(sandbox.envd_access_token, str) else None
traffic_access_token = sandbox.traffic_access_token if isinstance(sandbox.traffic_access_token, str) else None
This is a pre-existing issue not introduced by this PR, but since the PR directly modifies _cls_connect_sandbox in both sync and async paths, this is the right moment to address the gap.
In _cls_connect_sandbox, envd_access_token, traffic_access_token, and domain from the API response were passed directly to the constructor without isinstance(str) cleaning. This could cause UNSET sentinel values to leak into SandboxBase, leading to invalid signatures in download_url()/upload_url() and incorrect domain URL construction. Apply the same isinstance(str) sanitization pattern used in _create_sandbox to ensure UNSET values are converted to None.
2 participants
Problem
The Python SDK's
Sandbox.connect()currently injectsE2b-Sandbox-IdandE2b-Sandbox-Portheaders into the control plane request (POST /sandboxes/{sandboxID}/connect).This causes issues for any proxy/gateway that uses these headers to distinguish between control plane and data plane traffic. Since these headers are the canonical signal for "this request targets a specific sandbox instance (data plane)", their presence on a control plane API call causes the proxy to misroute the request to the sandbox's envd endpoint instead of the API server — resulting in authentication failures (401).
The JS/TS SDK does not have this problem:
connectSandbox()sends a clean control plane request without these headers, and only injects them on data plane requests via the sandbox instance's connection config.Root Cause
Introduced in #1013 (Support overriding envd API URL), the
_cls_connectmethod was updated to passE2b-Sandbox-Id/E2b-Sandbox-Portdirectly toget_api_client()as extra headers. This means they end up on the control plane HTTP client, polluting thePOST /sandboxes/{id}/connectrequest with data plane routing signals.Fix
Move
E2b-Sandbox-IdandE2b-Sandbox-Portfrom the control plane API client (get_api_client()insandbox_api.py) toextra_sandbox_headersin theConnectionConfig(set inmain.py's_cls_connect_sandbox).This ensures:
config.headers→ApiClient) do not carry these headersconfig.sandbox_headers→ filesystem / commands / pty / health) do carry them, as intendedThe fix applies to both sync and async SDK paths (4 files).
Why this matters
These two headers serve as a data plane routing signal in the E2B protocol. Any transparent proxy, API gateway, or load balancer that multiplexes control plane and data plane traffic on a single endpoint will use their presence/absence to decide where to forward the request.
When a control plane API like
connectcarries these headers, it creates an ambiguous signal — the path says "control plane" but the headers say "data plane". This is not specific to any single proxy implementation; it affects any gateway that follows the protocol semantics.Verification
connectrequests no longer carryE2b-Sandbox-Id/E2b-Sandbox-Portsandbox_headers