-
Notifications
You must be signed in to change notification settings - Fork 1k
Remove SDK-side defaults from API request payloads #1749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
1c4dc53
b92ce2b
996c679
ef24b64
b984e34
6d991cb
aa59e1f
e4d7b95
a06670b
e9963a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| 'e2b': minor | ||
| '@e2b/python-sdk': minor | ||
| --- | ||
|
|
||
| Remove SDK-side defaults from API request payloads so the API defaults apply when options are omitted. Sandbox create/fork/connect no longer preset a 5-minute timeout, fork no longer presets `count: 1`, create no longer presets `secure: true` or `allow_internet_access`, pause no longer presets keeping memory, and template builds no longer preset CPU/memory. Explicitly provided values are still sent unchanged. Note: until the API-side defaults for `secure` and connect `timeout` are deployed, omitting them changes behavior (sandboxes are created unsecured and connect requests without a timeout are rejected). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| 'e2b': patch | ||
| '@e2b/python-sdk': patch | ||
| --- | ||
|
|
||
| Remove client-side validation of the fork `count` argument. The API validates the requested fork count and rejects invalid values. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,6 @@ import { | |
| ClientFactory, | ||
| ConnectionConfig, | ||
| ConnectionOpts, | ||
| DEFAULT_SANDBOX_TIMEOUT_MS, | ||
| } from '../connectionConfig' | ||
| import { compareVersions } from 'compare-versions' | ||
| import { ALL_TRAFFIC } from './network' | ||
|
|
@@ -513,8 +512,6 @@ export interface SandboxPauseOpts extends SandboxApiOpts { | |
| * When `false`, the in-memory state is dropped and only the filesystem is | ||
| * persisted (a filesystem-only snapshot); resuming such a sandbox cold-boots | ||
| * (reboots) it from disk, losing running processes and open connections. | ||
| * | ||
| * @default true | ||
| */ | ||
| keepMemory?: boolean | ||
| } | ||
|
|
@@ -529,16 +526,12 @@ export interface SandboxForkOpts extends ConnectionOpts { | |
| * All forks boot from the same snapshot — the snapshot is captured once | ||
| * regardless of count. Each fork succeeds or fails independently; the | ||
| * outcome of each is reported in its entry of the returned array. | ||
| * | ||
| * @default 1 | ||
| */ | ||
| count?: number | ||
|
|
||
| /** | ||
| * Timeout for the forked sandboxes in **milliseconds**. | ||
| * Maximum time a sandbox can be kept alive is 24 hours (86_400_000 milliseconds) for Pro users and 1 hour (3_600_000 milliseconds) for Hobby users. | ||
| * | ||
| * @default 300_000 // 5 minutes | ||
| */ | ||
| timeoutMs?: number | ||
| } | ||
|
|
@@ -590,22 +583,16 @@ export interface SandboxOpts extends ConnectionOpts { | |
| /** | ||
| * Timeout for the sandbox in **milliseconds**. | ||
| * Maximum time a sandbox can be kept alive is 24 hours (86_400_000 milliseconds) for Pro users and 1 hour (3_600_000 milliseconds) for Hobby users. | ||
| * | ||
| * @default 300_000 // 5 minutes | ||
| */ | ||
| timeoutMs?: number | ||
|
|
||
| /** | ||
| * Secure all traffic coming to the sandbox controller with auth token | ||
| * | ||
| * @default true | ||
| */ | ||
| secure?: boolean | ||
|
|
||
| /** | ||
| * Allow sandbox to access the internet. If set to `False`, it works the same as setting network `denyOut` to `[0.0.0.0/0]`. | ||
| * | ||
| * @default true | ||
| */ | ||
| allowInternetAccess?: boolean | ||
|
|
||
|
|
@@ -673,8 +660,6 @@ export type SandboxConnectOpts = ConnectionOpts & { | |
| * Timeout for the sandbox in **milliseconds**. | ||
| * For running sandboxes, the timeout will update only if the new timeout is longer than the existing one. | ||
| * Maximum time a sandbox can be kept alive is 24 hours (86_400_000 milliseconds) for Pro users and 1 hour (3_600_000 milliseconds) for Hobby users. | ||
| * | ||
| * @default 300_000 // 5 minutes | ||
| */ | ||
| timeoutMs?: number | ||
| } | ||
|
|
@@ -714,8 +699,6 @@ export interface SandboxListOpts extends Omit<SandboxApiOpts, 'signal'> { | |
| /** | ||
| * Sort order of the list of sandboxes by start time, applied across the | ||
| * whole result set before pagination (not within a page). | ||
| * | ||
| * @default 'desc' | ||
| */ | ||
| order?: SandboxListOrder | ||
|
|
||
|
|
@@ -1475,7 +1458,7 @@ export class SandboxApi extends ClientFactory { | |
| }, | ||
| }, | ||
| body: { | ||
| memory: apiOpts?.keepMemory ?? true, | ||
| memory: apiOpts?.keepMemory, | ||
| }, | ||
| signal: config.getSignal(apiOpts?.requestTimeoutMs, apiOpts?.signal), | ||
| }) | ||
|
|
@@ -1602,7 +1585,7 @@ export class SandboxApi extends ClientFactory { | |
|
|
||
| protected static async createSandbox( | ||
| template: string, | ||
| timeoutMs: number, | ||
| timeoutMs?: number, | ||
| opts?: SandboxOpts | ||
| ) { | ||
| const apiOpts = this.resolveOpts(opts) | ||
|
|
@@ -1656,9 +1639,10 @@ export class SandboxApi extends ClientFactory { | |
| metadata: opts?.metadata, | ||
| mcp: opts?.mcp as Record<string, unknown> | undefined, | ||
| envVars: opts?.envs, | ||
| timeout: timeoutToSeconds(timeoutMs), | ||
| secure: opts?.secure ?? true, | ||
| allow_internet_access: opts?.allowInternetAccess ?? true, | ||
| timeout: | ||
| timeoutMs === undefined ? undefined : timeoutToSeconds(timeoutMs), | ||
| secure: opts?.secure, | ||
|
mishushakov marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Agentic Security Review The SDK now omits Impact: Callers that rely on defaults can unintentionally create sandboxes with weaker controller access protection while backend defaulting is not universally guaranteed, allowing unauthorized controller interaction when the endpoint is reachable. Reviewed by Cursor Security Reviewer for commit e9963a5. Configure here. |
||
| allow_internet_access: opts?.allowInternetAccess, | ||
| network: buildNetworkBody(opts?.network, iam), | ||
| iam, | ||
| autoPause: onTimeoutConfigured ? action === 'pause' : undefined, | ||
|
|
@@ -1705,14 +1689,10 @@ export class SandboxApi extends ClientFactory { | |
|
|
||
| protected static async forkSandbox( | ||
| sandboxId: string, | ||
| timeoutMs: number, | ||
| count: number, | ||
| timeoutMs?: number, | ||
| count?: number, | ||
| opts?: SandboxApiOpts | ||
| ): Promise<SandboxForkResponse[]> { | ||
| if (count < 1) { | ||
| throw new InvalidArgumentError('count must be at least 1') | ||
| } | ||
|
|
||
| const apiOpts = this.resolveOpts(opts) | ||
| const config = new ConnectionConfig(apiOpts) | ||
| const client = new ApiClient(config) | ||
|
|
@@ -1724,7 +1704,8 @@ export class SandboxApi extends ClientFactory { | |
| }, | ||
| }, | ||
| body: { | ||
| timeout: timeoutToSeconds(timeoutMs), | ||
| timeout: | ||
| timeoutMs === undefined ? undefined : timeoutToSeconds(timeoutMs), | ||
| count, | ||
| }, | ||
| signal: config.getSignal(apiOpts?.requestTimeoutMs, apiOpts?.signal), | ||
|
|
@@ -1777,7 +1758,7 @@ export class SandboxApi extends ClientFactory { | |
| opts?: SandboxConnectOpts | ||
| ) { | ||
| const apiOpts = this.resolveOpts(opts) | ||
| const timeoutMs = apiOpts?.timeoutMs ?? DEFAULT_SANDBOX_TIMEOUT_MS | ||
| const timeoutMs = apiOpts?.timeoutMs | ||
|
|
||
| const config = new ConnectionConfig(apiOpts) | ||
| const client = new ApiClient(config) | ||
|
|
@@ -1788,9 +1769,11 @@ export class SandboxApi extends ClientFactory { | |
| sandboxID: sandboxId, | ||
| }, | ||
| }, | ||
| // TODO: drop the cast once the API spec makes `timeout` optional | ||
| body: { | ||
| timeout: timeoutToSeconds(timeoutMs), | ||
| }, | ||
| timeout: | ||
| timeoutMs === undefined ? undefined : timeoutToSeconds(timeoutMs), | ||
| } as components['schemas']['ConnectSandbox'], | ||
| signal: config.getSignal(apiOpts?.requestTimeoutMs, apiOpts?.signal), | ||
| }) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| import { afterAll, afterEach, beforeAll, expect, test } from 'vitest' | ||
| import { http, HttpResponse } from 'msw' | ||
| import { setupServer } from 'msw/node' | ||
|
|
||
| import { Sandbox } from '../../src' | ||
| import { TEST_API_KEY, apiUrl } from '../setup' | ||
|
|
||
| let lastCreateBody: Record<string, unknown> | undefined | ||
| let lastForkBody: Record<string, unknown> | undefined | ||
| let lastPauseBody: Record<string, unknown> | undefined | ||
| let lastConnectBody: Record<string, unknown> | undefined | ||
|
|
||
| const server = setupServer( | ||
| http.post(apiUrl('/sandboxes'), async ({ request }) => { | ||
| lastCreateBody = (await request.json()) as Record<string, unknown> | ||
| return HttpResponse.json({ | ||
| sandboxID: 'test-sandbox-id', | ||
| templateID: 'base', | ||
| envdVersion: '0.2.4', | ||
| }) | ||
| }), | ||
| http.post(apiUrl('/sandboxes/:sandboxID/connect'), async ({ request }) => { | ||
| lastConnectBody = (await request.json()) as Record<string, unknown> | ||
| return HttpResponse.json({ | ||
| sandboxID: 'test-sandbox-id', | ||
| templateID: 'base', | ||
| envdVersion: '0.2.4', | ||
| }) | ||
| }), | ||
| http.post(apiUrl('/sandboxes/:sandboxID/fork'), async ({ request }) => { | ||
| lastForkBody = (await request.json()) as Record<string, unknown> | ||
| return HttpResponse.json([ | ||
| { | ||
| sandbox: { | ||
| sandboxID: 'forked-sandbox-id', | ||
| templateID: 'base', | ||
| envdVersion: '0.2.4', | ||
| }, | ||
| }, | ||
| ]) | ||
| }), | ||
| http.post(apiUrl('/sandboxes/:sandboxID/pause'), async ({ request }) => { | ||
| lastPauseBody = (await request.json()) as Record<string, unknown> | ||
| return new HttpResponse(null, { status: 204 }) | ||
| }) | ||
| ) | ||
|
|
||
| beforeAll(() => server.listen({ onUnhandledRequest: 'error' })) | ||
|
|
||
| afterAll(() => server.close()) | ||
|
|
||
| afterEach(() => { | ||
| lastCreateBody = undefined | ||
| lastForkBody = undefined | ||
| lastPauseBody = undefined | ||
| lastConnectBody = undefined | ||
| server.resetHandlers() | ||
| }) | ||
|
|
||
| test('Sandbox.create omits timeout, secure and allow_internet_access when unset', async () => { | ||
| await Sandbox.create('base', { apiKey: TEST_API_KEY }) | ||
|
|
||
| expect(lastCreateBody).toBeDefined() | ||
| expect(lastCreateBody).not.toHaveProperty('timeout') | ||
| expect(lastCreateBody).not.toHaveProperty('secure') | ||
| expect(lastCreateBody).not.toHaveProperty('allow_internet_access') | ||
| }) | ||
|
|
||
| test('Sandbox.create sends explicit timeout, secure and allow_internet_access', async () => { | ||
| await Sandbox.create('base', { | ||
| apiKey: TEST_API_KEY, | ||
| timeoutMs: 60_000, | ||
| secure: false, | ||
| allowInternetAccess: false, | ||
| }) | ||
|
|
||
| expect(lastCreateBody?.timeout).toBe(60) | ||
| expect(lastCreateBody?.secure).toBe(false) | ||
| expect(lastCreateBody?.allow_internet_access).toBe(false) | ||
| }) | ||
|
|
||
| test('Sandbox.fork omits timeout and count when unset', async () => { | ||
| await Sandbox.fork('test-sandbox-id', { apiKey: TEST_API_KEY }) | ||
|
|
||
| expect(lastForkBody).toBeDefined() | ||
| expect(lastForkBody).not.toHaveProperty('timeout') | ||
| expect(lastForkBody).not.toHaveProperty('count') | ||
| }) | ||
|
|
||
| test('Sandbox.fork sends explicit timeout and count', async () => { | ||
| await Sandbox.fork('test-sandbox-id', { | ||
| apiKey: TEST_API_KEY, | ||
| timeoutMs: 60_000, | ||
| count: 2, | ||
| }) | ||
|
|
||
| expect(lastForkBody?.timeout).toBe(60) | ||
| expect(lastForkBody?.count).toBe(2) | ||
| }) | ||
|
|
||
| test('Sandbox.pause omits memory when keepMemory is unset', async () => { | ||
| await Sandbox.pause('test-sandbox-id', { apiKey: TEST_API_KEY }) | ||
|
|
||
| expect(lastPauseBody).toBeDefined() | ||
| expect(lastPauseBody).not.toHaveProperty('memory') | ||
| }) | ||
|
|
||
| test('Sandbox.pause sends an explicit keepMemory', async () => { | ||
| await Sandbox.pause('test-sandbox-id', { | ||
| apiKey: TEST_API_KEY, | ||
| keepMemory: false, | ||
| }) | ||
|
|
||
| expect(lastPauseBody?.memory).toBe(false) | ||
| }) | ||
|
|
||
| test('Sandbox.connect omits timeout when unset', async () => { | ||
| await Sandbox.connect('test-sandbox-id', { apiKey: TEST_API_KEY }) | ||
|
|
||
| expect(lastConnectBody).toBeDefined() | ||
| expect(lastConnectBody).not.toHaveProperty('timeout') | ||
| }) | ||
|
|
||
| test('Sandbox.connect sends an explicit timeout', async () => { | ||
| await Sandbox.connect('test-sandbox-id', { | ||
| apiKey: TEST_API_KEY, | ||
| timeoutMs: 60_000, | ||
| }) | ||
|
|
||
| expect(lastConnectBody?.timeout).toBe(60) | ||
| }) |


Uh oh!
There was an error while loading. Please reload this page.