-
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 1 commit
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 no longer preset a 5-minute timeout, fork no longer presets `count: 1`, create no longer presets `secure` and `allow_internet_access`, pause no longer presets keeping memory, and template builds no longer preset CPU/memory. Explicitly provided values are still sent unchanged. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -514,7 +514,7 @@ export interface SandboxPauseOpts extends SandboxApiOpts { | |
| * persisted (a filesystem-only snapshot); resuming such a sandbox cold-boots | ||
| * (reboots) it from disk, losing running processes and open connections. | ||
| * | ||
| * @default true | ||
| * When not set, the API default (currently a full memory snapshot) applies. | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
Outdated
|
||
| */ | ||
| keepMemory?: boolean | ||
| } | ||
|
|
@@ -530,15 +530,15 @@ export interface SandboxForkOpts extends ConnectionOpts { | |
| * regardless of count. Each fork succeeds or fails independently; the | ||
| * outcome of each is reported in its entry of the returned array. | ||
| * | ||
| * @default 1 | ||
| * When not set, the API default (currently 1) applies. | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
Outdated
|
||
| */ | ||
| 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 | ||
| * When not set, the API default timeout applies. | ||
| */ | ||
| timeoutMs?: number | ||
| } | ||
|
|
@@ -591,21 +591,21 @@ 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 | ||
| * When not set, the API default timeout applies. | ||
| */ | ||
| timeoutMs?: number | ||
|
|
||
| /** | ||
| * Secure all traffic coming to the sandbox controller with auth token | ||
| * | ||
| * @default true | ||
| * When not set, the API default (currently enabled) applies. | ||
| */ | ||
| 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 | ||
| * When not set, the API default (currently allowed) applies. | ||
| */ | ||
| allowInternetAccess?: boolean | ||
|
|
||
|
|
@@ -714,8 +714,7 @@ 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' | ||
| * When not set, the API default (currently `'desc'`, newest first) applies. | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
Outdated
|
||
| */ | ||
| order?: SandboxListOrder | ||
|
|
||
|
|
@@ -1475,7 +1474,7 @@ export class SandboxApi extends ClientFactory { | |
| }, | ||
| }, | ||
| body: { | ||
| memory: apiOpts?.keepMemory ?? true, | ||
| memory: apiOpts?.keepMemory, | ||
| }, | ||
| signal: config.getSignal(apiOpts?.requestTimeoutMs, apiOpts?.signal), | ||
| }) | ||
|
|
@@ -1602,7 +1601,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 +1655,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,11 +1705,11 @@ 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) { | ||
| if (count !== undefined && count < 1) { | ||
| throw new InvalidArgumentError('count must be at least 1') | ||
| } | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
|
|
@@ -1724,7 +1724,8 @@ export class SandboxApi extends ClientFactory { | |
| }, | ||
| }, | ||
| body: { | ||
| timeout: timeoutToSeconds(timeoutMs), | ||
| timeout: | ||
| timeoutMs === undefined ? undefined : timeoutToSeconds(timeoutMs), | ||
| count, | ||
| }, | ||
| signal: config.getSignal(apiOpts?.requestTimeoutMs, apiOpts?.signal), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| 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 | ||
|
|
||
| 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/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 | ||
| 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) | ||
| }) |


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