From 646622aefd39f80ed6efe02cb647bd14ebd14c90 Mon Sep 17 00:00:00 2001 From: Mish Ushakov <10400064+mishushakov@users.noreply.github.com> Date: Wed, 19 Aug 2026 19:44:13 +0200 Subject: [PATCH] fix(sdk): match JS and Python on malformed and null egress proxy input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The BYOP surface from #1688 diverged for callers that bypass the types. Python raised InvalidArgumentException on a proxy without a string address; JS rebuilt the body from the known fields, so `egressProxy` passed as a bare string sent `{}` and the caller got an API error naming a field they never left out. Mirror the guard in buildEgressProxyBody, the way buildIamBody already does for untyped token maps. Both SDKs also forwarded a null/None username or password as a JSON null, which the API rejects — `{"username": os.environ.get(...)}` on an unset variable is the way that happens. Read it as "no credentials", the same reading both already gave `egressProxy: null` itself, and normalize a null username coming back out of getInfo so SandboxEgressProxyInfo.username cannot be a null its type forbids. The get_info example published in both CHANGELOGs for 2.41.0 subscripts `info.network["egress_proxy"]`, which KeyErrors on every sandbox without a proxy — SandboxNetworkInfo is total=False and the key is only set when one is configured. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/egress-proxy-untyped-callers.md | 43 +++++++++++ packages/js-sdk/CHANGELOG.md | 2 +- packages/js-sdk/src/sandbox/sandboxApi.ts | 35 +++++---- .../js-sdk/tests/sandbox/egressProxy.test.ts | 74 +++++++++++++++++-- packages/python-sdk/CHANGELOG.md | 2 +- .../python-sdk/e2b/sandbox/sandbox_api.py | 16 ++-- .../tests/shared/sandbox/test_egress_proxy.py | 35 +++++++++ 7 files changed, 181 insertions(+), 26 deletions(-) create mode 100644 .changeset/egress-proxy-untyped-callers.md diff --git a/.changeset/egress-proxy-untyped-callers.md b/.changeset/egress-proxy-untyped-callers.md new file mode 100644 index 0000000000..efd7eaab53 --- /dev/null +++ b/.changeset/egress-proxy-untyped-callers.md @@ -0,0 +1,43 @@ +--- +'e2b': patch +'@e2b/python-sdk': patch +--- + +Bring the JS and Python halves of `network.egressProxy` / `network["egress_proxy"]` back in line for callers that bypass the types, and stop `null` credentials from reaching the wire. + +A malformed proxy now raises `InvalidArgumentError` / `InvalidArgumentException` in both SDKs. Before, only Python did; JS rebuilt the body from the known fields, so a proxy passed as a bare string sent `{}` and the caller got an API error about a field they never left out: + +```ts +// Now: InvalidArgumentError, naming the option you typed. +// Before: sent `"egressProxy": {}` and failed at the API. +await Sandbox.create({ + network: { egressProxy: 'proxy.example.com:1080' as never }, +}) +``` + +A `username` or `password` that is `null` / `None` is treated as "no credentials" rather than serialized as a JSON null the API rejects — the same reading both SDKs already gave `egressProxy: null` itself: + +```ts +await Sandbox.create({ + network: { + egressProxy: { + address: 'proxy.example.com:1080', + // Unset in the environment; the proxy takes no credentials. + username: process.env.PROXY_USER, + }, + }, +}) +``` + +```python +Sandbox.create( + network={ + "egress_proxy": { + "address": "proxy.example.com:1080", + "username": os.environ.get("PROXY_USER"), + }, + }, +) +``` + +`getInfo` / `get_info` normalizes a `null` `username` the same way, so `SandboxEgressProxyInfo.username` is `undefined` / an absent key rather than a null that its type says cannot be there. diff --git a/packages/js-sdk/CHANGELOG.md b/packages/js-sdk/CHANGELOG.md index 8c1166b0d0..241216a801 100644 --- a/packages/js-sdk/CHANGELOG.md +++ b/packages/js-sdk/CHANGELOG.md @@ -92,7 +92,7 @@ ```python info = sandbox.get_info() - print(info.network["egress_proxy"]) + print(info.network.get("egress_proxy")) # {'address': 'proxy.example.com:1080', 'username': 'proxy-user'} ``` diff --git a/packages/js-sdk/src/sandbox/sandboxApi.ts b/packages/js-sdk/src/sandbox/sandboxApi.ts index 5e6f31177d..9b84312af4 100644 --- a/packages/js-sdk/src/sandbox/sandboxApi.ts +++ b/packages/js-sdk/src/sandbox/sandboxApi.ts @@ -997,20 +997,30 @@ function resolveRulesForBody( /** * Rebuild the proxy config from the known fields so stray properties on the * caller's object never reach the wire and a later mutation of it cannot alter - * the in-flight request. Validation is the server's — it is the only side that - * can tell whether the address resolves, and to where. + * the in-flight request. Address reachability is the server's — it is the only + * side that can tell whether the address resolves, and to where. */ function buildEgressProxyBody( egressProxy: SandboxEgressProxyOpts ): components['schemas']['SandboxEgressProxyConfig'] { + // Re-check at runtime for callers that bypass the type — rebuilding from the + // known fields drops an address that isn't there, and the API error for the + // resulting `{}` names neither the option the caller typed nor the mistake. + // Python raises `InvalidArgumentException` on the same input. + if (typeof egressProxy.address !== 'string') { + throw new InvalidArgumentError( + "network egressProxy must be an object with a string 'address' " + + "(e.g. 'proxy.example.com:1080')." + ) + } + return { address: egressProxy.address, - ...(egressProxy.username !== undefined - ? { username: egressProxy.username } - : {}), - ...(egressProxy.password !== undefined - ? { password: egressProxy.password } - : {}), + // `!= null` so a credential read out of an unset environment variable + // reads as "no credentials" rather than reaching the wire as JSON null, + // which the API rejects. Same reasoning as `egressProxy: null` itself. + ...(egressProxy.username != null ? { username: egressProxy.username } : {}), + ...(egressProxy.password != null ? { password: egressProxy.password } : {}), } } @@ -1054,8 +1064,9 @@ function buildNetworkEgress( /** * Map the wire proxy config into the SDK-owned shape: `password` is dropped - * because the API never returns it, and the wire's `null` for "no proxy" is - * normalized so the union never reaches a consumer. + * because the API never returns it, and the wire's `null` — for "no proxy" and + * for an anonymous proxy's `username` alike — is normalized so it never reaches + * a consumer typed to see `undefined`. */ function fromApiEgressProxy( egressProxy: components['schemas']['SandboxEgressProxyConfig'] | undefined @@ -1066,9 +1077,7 @@ function fromApiEgressProxy( return { address: egressProxy.address, - ...(egressProxy.username !== undefined - ? { username: egressProxy.username } - : {}), + ...(egressProxy.username != null ? { username: egressProxy.username } : {}), } } diff --git a/packages/js-sdk/tests/sandbox/egressProxy.test.ts b/packages/js-sdk/tests/sandbox/egressProxy.test.ts index b861fd2df7..d6087bc77e 100644 --- a/packages/js-sdk/tests/sandbox/egressProxy.test.ts +++ b/packages/js-sdk/tests/sandbox/egressProxy.test.ts @@ -2,7 +2,7 @@ import { afterAll, afterEach, beforeAll, expect, test } from 'vitest' import { http, HttpResponse } from 'msw' import { setupServer } from 'msw/node' -import { Sandbox } from '../../src' +import { InvalidArgumentError, Sandbox } from '../../src' import { TEST_API_KEY, apiUrl } from '../setup' const sandboxId = 'test-sandbox-id' @@ -101,14 +101,64 @@ test('Sandbox.create combines the egress proxy with allow and deny lists', async }) }) -test('Sandbox.create omits the egress proxy when not provided', async () => { +test.for([ + ['omitted', { allowOut: ['api.example.com'] }], + // Untyped callers spell "no proxy" as null; Python treats an explicit None + // the same way. + ['null', { egressProxy: null }], +])( + 'Sandbox.create omits the egress proxy when it is %s', + async ([, network]: [string, Record]) => { + await Sandbox.create('base', { + apiKey: TEST_API_KEY, + network, + }) + + expect(lastCreateBody?.network).toBeDefined() + expect(lastCreateBody?.network).not.toHaveProperty('egressProxy') + } +) + +test.for([ + // An empty object is falsy but present — it must not silently disable + // tunneling. Match Python: fail loudly. + ['empty', {}], + ['missing-address', { username: 'proxy-user' }], + ['non-string-address', { address: 1080 }], + ['string', 'proxy.example.com:1080'], +])( + 'Sandbox.create rejects a %s egress proxy', + async ([, egressProxy]: [string, unknown]) => { + // Rebuilding from the known fields drops an address that isn't there, so + // without this the caller gets an API error about a `{}` they never wrote. + await expect( + Sandbox.create('base', { + apiKey: TEST_API_KEY, + network: { egressProxy } as never, + }) + ).rejects.toThrow(InvalidArgumentError) + + expect(lastCreateBody).toBeUndefined() + } +) + +test('Sandbox.create omits credentials that are null', async () => { + // `{ username: process.env.PROXY_USER }` on an unset variable is the way + // this happens; a JSON null is rejected by the API. await Sandbox.create('base', { apiKey: TEST_API_KEY, - network: { allowOut: ['api.example.com'] }, + network: { + egressProxy: { + address: 'proxy.example.com:1080', + username: null, + password: undefined, + } as never, + }, }) - expect(lastCreateBody?.network).toBeDefined() - expect(lastCreateBody?.network).not.toHaveProperty('egressProxy') + expect(lastCreateBody?.network.egressProxy).toEqual({ + address: 'proxy.example.com:1080', + }) }) test('Sandbox.create strips unknown egress proxy properties', async () => { @@ -199,6 +249,20 @@ test('getInfo drops a password the API unexpectedly returns', async () => { }) }) +test('getInfo drops a null username', async () => { + // `username?: string` says absence is `undefined`, so a null from the wire + // has to be normalized rather than handed to a consumer. + sandboxNetwork = { + egressProxy: { address: 'proxy.example.com:1080', username: null }, + } + + const info = await Sandbox.getInfo(sandboxId, { apiKey: TEST_API_KEY }) + + expect(info.network?.egressProxy).toEqual({ + address: 'proxy.example.com:1080', + }) +}) + test.for([ ['omitted', {}], ['null', { egressProxy: null }], diff --git a/packages/python-sdk/CHANGELOG.md b/packages/python-sdk/CHANGELOG.md index fe15466fab..1b58b63c76 100644 --- a/packages/python-sdk/CHANGELOG.md +++ b/packages/python-sdk/CHANGELOG.md @@ -92,7 +92,7 @@ ```python info = sandbox.get_info() - print(info.network["egress_proxy"]) + print(info.network.get("egress_proxy")) # {'address': 'proxy.example.com:1080', 'username': 'proxy-user'} ``` diff --git a/packages/python-sdk/e2b/sandbox/sandbox_api.py b/packages/python-sdk/e2b/sandbox/sandbox_api.py index c13d0d28b0..daf38c79b5 100644 --- a/packages/python-sdk/e2b/sandbox/sandbox_api.py +++ b/packages/python-sdk/e2b/sandbox/sandbox_api.py @@ -671,9 +671,12 @@ def _build_egress_proxy( ) body = ClientSandboxEgressProxyConfig(address=egress_proxy["address"]) - if "username" in egress_proxy: + # `is not None` so a credential read out of an unset environment variable + # reads as "no credentials" rather than reaching the wire as JSON null, + # which the API rejects. Same reasoning as ``"egress_proxy": None`` itself. + if egress_proxy.get("username") is not None: body.username = egress_proxy["username"] - if "password" in egress_proxy: + if egress_proxy.get("password") is not None: body.password = egress_proxy["password"] return body @@ -899,15 +902,16 @@ def _from_client_egress_proxy( ) -> Optional[SandboxEgressProxyInfo]: """ Map the wire proxy config into the SDK-owned shape: ``password`` is dropped - because the API never returns it, and the wire's ``None`` for "no proxy" - becomes an absent key. + because the API never returns it, and the wire's ``None`` — for "no proxy" + and for an anonymous proxy's ``username`` alike — becomes an absent key. """ if not isinstance(egress_proxy, ClientSandboxEgressProxyConfig): return None result: SandboxEgressProxyInfo = {"address": egress_proxy.address} - if not isinstance(egress_proxy.username, Unset): - result["username"] = egress_proxy.username + username = egress_proxy.username + if not isinstance(username, Unset) and username is not None: + result["username"] = username return result diff --git a/packages/python-sdk/tests/shared/sandbox/test_egress_proxy.py b/packages/python-sdk/tests/shared/sandbox/test_egress_proxy.py index e097ee1711..182167699b 100644 --- a/packages/python-sdk/tests/shared/sandbox/test_egress_proxy.py +++ b/packages/python-sdk/tests/shared/sandbox/test_egress_proxy.py @@ -85,6 +85,25 @@ def test_create_rejects_a_malformed_egress_proxy(egress_proxy): build_network_config(cast(Any, {"egress_proxy": egress_proxy})) +def test_create_omits_credentials_that_are_none(): + # ``{"username": os.environ.get("PROXY_USER")}`` on an unset variable is the + # way this happens; a JSON null is rejected by the API. + body = build_network_config( + cast( + Any, + { + "egress_proxy": { + "address": "proxy.example.com:1080", + "username": None, + "password": None, + }, + }, + ) + ) + assert body is not None + assert body["egress_proxy"].to_dict() == {"address": "proxy.example.com:1080"} + + def test_create_strips_unknown_egress_proxy_keys(): # An untyped caller can copy an extra key out of a config file; the API # rejects unknown properties. @@ -145,6 +164,22 @@ def test_get_info_reports_the_active_egress_proxy_without_the_password(): } +def test_get_info_drops_a_none_username(): + # ``username`` is ``NotRequired[str]``, so absence is a missing key — a None + # from the wire has to be normalized rather than handed to a caller. + info = from_client_network_config( + SandboxNetworkConfig( + egress_proxy=ClientSandboxEgressProxyConfig( + address="proxy.example.com:1080", + username=cast(Any, None), + ) + ) + ) + + assert info is not None + assert info["egress_proxy"] == {"address": "proxy.example.com:1080"} + + @pytest.mark.parametrize( "egress_proxy", [