fix(e2b): support e2b api url and CubeSandbox#1765
Conversation
📝 WalkthroughChange overview This PR adds explicit E2B management API URL support and CubeSandbox-friendly behavior to the e2b executor, plus TLS/HTTP-client improvements and test coverage:
Compatibility and behavioral risks
Recommended validation steps
Priority fixes if validations fail
中文变更概览 此 PR 为 e2b 执行器引入显式的 E2B 管理 API URL 支持并增强对 CubeSandbox 的兼容性,同时改进了 TLS/HTTP 客户端初始化并增加了测试覆盖:
兼容性与行为风险
推荐验证步骤 1)API URL 行为
2)Sandbox 域与迁移行为
3)TLS 与 HTTP 客户端处理
4)端到端 CubeSandbox 检查
5)向后兼容与运维检查
若验证失败应优先处理
WalkthroughThis PR adds an explicit APIURL override to the E2B executor and connection types, builds a TLS-aware default HTTP client that loads roots from SSL_CERT_FILE/SSL_CERT_DIR, switches sandbox hostname construction to prefer an API-resolved cached domain, and wires the APIURL through executor options and sandbox lifecycle; tests and README updates included. ChangesE2B Sandbox API URL and TLS Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
中文摘要(折叠): 此 PR 为 E2B 沙箱执行器添加可配置的 API URL 与 TLS 证书支持:在 ConnectionConfig/SandboxOpts 中添加 APIURL,支持从 E2B_API_URL 环境变量回退;构建可从 SSL_CERT_FILE/SSL_CERT_DIR 加载根证书的默认 HTTP 客户端;将沙箱主机名生成从 fullID 改为 API 返回的 domain 并缓存;通过 CodeExecutor 的 WithAPIURL 将配置传入 Create/Connect/List/GetInfo;新增测试用例与 README 示例。 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| // getHost returns the public host for a port exposed by the sandbox. | ||
| func (s *Sandbox) getHost(port int) string { | ||
| return fmt.Sprintf("%d-%s.%s", port, s.fullID(), s.connection.Domain) | ||
| return fmt.Sprintf("%d-%s.%s", port, s.id, s.sandboxHostDomain()) |
There was a problem hiding this comment.
When domain is absent, this drops clientID from sandbox hostnames and changes existing GetHost and Jupyter URLs. Keep the old sandboxID-clientID fallback.
中文
当 `domain` 缺失时,这里会从 sandbox 主机名中丢掉 `clientID`,并改变现有 `GetHost` 和 Jupyter URL。请保留旧的 `sandboxID-clientID` 回退。| // this keeps the jupyter/envd URLs correct even if the sandbox was | ||
| // relocated to a different host. | ||
| if info.Domain != "" { | ||
| s.sandboxDomain = info.Domain |
There was a problem hiding this comment.
GetInfo now writes sandboxDomain while Jupyter requests read it, so concurrent info refresh and execution races. Guard the cached domain or avoid mutating it.
中文
`GetInfo` 现在会在 Jupyter 请求读取 `sandboxDomain` 的同时写入它,因此并发刷新信息和执行代码会产生数据竞争。请为缓存域名加锁,或避免修改它。There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
codeexecutor/e2b/internal/codeinterpreter/sandbox_test.go (1)
443-472: ⚡ Quick win
TestGetInfo_ThroughMockAPIdoes not exercise the newsandboxDomainrefresh logic.The mock response omits the
domainfield, so theif info.Domain != ""branch inGetInfois never taken. The primary new behavior — thatGetHost/jupyterURLuses the API-returned domain after aGetInfocall — is unverified.🧪 Suggested test extension
func TestGetInfo_ThroughMockAPI(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ... - _, _ = w.Write([]byte(`{"sandboxID":"abc","clientID":"cid","state":"running","metadata":{"k":"v"}}`)) + _, _ = w.Write([]byte(`{"sandboxID":"abc","clientID":"cid","state":"running","metadata":{"k":"v"},"domain":"custom.host.local"}`)) })) ... info, err := sbx.GetInfo(context.Background()) ... + // Verify that sandboxDomain is refreshed from the API response. + if sbx.sandboxDomain != "custom.host.local" { + t.Errorf("sandboxDomain not refreshed: %q", sbx.sandboxDomain) + } + if got := sbx.GetHost(3000); got != "3000-abc.custom.host.local" { + t.Errorf("GetHost after domain refresh: %q", got) + } }中文
TestGetInfo_ThroughMockAPI的 mock 响应中未包含domain字段,导致GetInfo中的sandboxDomain刷新分支从未被执行。GetInfo返回 domain 后GetHost/jupyterURL使用新域名这一核心新行为缺乏测试覆盖。建议在 mock 响应中添加"domain"字段并断言sandboxDomain及GetHost的结果。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeexecutor/e2b/internal/codeinterpreter/sandbox_test.go` around lines 443 - 472, Update the TestGetInfo_ThroughMockAPI test to include a "domain" field in the mock JSON response so the sandboxDomain refresh branch in Sandbox.GetInfo is exercised; after calling sbx.GetInfo assert that info.Domain (or sbx.sandboxDomain) equals the returned domain and that sbx.GetHost (and/or jupyterURL) now uses that domain (e.g., returns host/jupyter URL containing the new domain) to verify the new behavior is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@codeexecutor/e2b/internal/codeinterpreter/client.go`:
- Around line 162-163: The TLS config returned currently only sets RootCAs which
leaves the minimum TLS version implicit; update the returned tls.Config (in the
function that currently returns &tls.Config{RootCAs: pool}) to explicitly set
MinVersion: tls.VersionTLS12 so the client enforces a TLS 1.2+ floor (i.e.,
return &tls.Config{RootCAs: pool, MinVersion: tls.VersionTLS12}).
In `@codeexecutor/e2b/internal/codeinterpreter/sandbox.go`:
- Around line 262-267: GetInfo currently writes s.sandboxDomain without
synchronization while sandboxHostDomain (used by jupyterURL → RunCode) reads it,
causing a data race; add a sync.RWMutex field (e.g., domainMu) to the Sandbox
struct and use domainMu.Lock()/Unlock() around writes in GetInfo and
domainMu.RLock()/RUnlock() around reads in sandboxHostDomain (and any other
readers like jupyterURL/RunCode) to ensure safe concurrent access to
sandboxDomain.
In `@examples/tool/codeexec/README.md`:
- Around line 52-59: Add the missing working-directory step to the CubeSandbox
snippet in the README so the commands run reproducibly: insert a `cd
tool/codeexec` step immediately before exporting E2B_ENV vars and running
`./codeexec-demo` in the CubeSandbox example block (the same placement used in
the other executor snippets) so users copying the Quick Start from `examples/`
land in the correct directory before invoking `codeexec-demo` with the `-model
deepseek-v4-flash -executor e2b` flags.
- Around line 55-57: Update the example env block in README.md to clearly mark
E2B_API_URL and SSL_CERT_FILE as optional overrides: change the comments or
variable descriptions around E2B_API_URL and SSL_CERT_FILE (while keeping
E2B_API_KEY shown as required) so the snippet communicates that E2B_API_URL and
SSL_CERT_FILE are optional overrides rather than mandatory configuration values
(reference the E2B_API_URL and SSL_CERT_FILE identifiers in the example and add
“(optional)” or similar wording next to them).
---
Nitpick comments:
In `@codeexecutor/e2b/internal/codeinterpreter/sandbox_test.go`:
- Around line 443-472: Update the TestGetInfo_ThroughMockAPI test to include a
"domain" field in the mock JSON response so the sandboxDomain refresh branch in
Sandbox.GetInfo is exercised; after calling sbx.GetInfo assert that info.Domain
(or sbx.sandboxDomain) equals the returned domain and that sbx.GetHost (and/or
jupyterURL) now uses that domain (e.g., returns host/jupyter URL containing the
new domain) to verify the new behavior is covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 16922e9b-7b87-41e7-8464-3b2e95a23df5
📒 Files selected for processing (6)
codeexecutor/e2b/e2b.gocodeexecutor/e2b/internal/codeinterpreter/client.gocodeexecutor/e2b/internal/codeinterpreter/sandbox.gocodeexecutor/e2b/internal/codeinterpreter/sandbox_test.gocodeexecutor/e2b/internal/codeinterpreter/testutil_test.goexamples/tool/codeexec/README.md
| Run with CubeSandbox: | ||
|
|
||
| ```bash | ||
| export E2B_API_KEY=dummy | ||
| export E2B_API_URL=http://<your-sandbox-server>:3000 | ||
| export SSL_CERT_FILE=<caPATH>/rootCA.pem | ||
| ./codeexec-demo -model deepseek-v4-flash -executor e2b | ||
| ``` |
There was a problem hiding this comment.
Add the missing working-directory step for reproducible run commands.
The CubeSandbox snippet omits cd tool/codeexec, while the other executor snippets include it. Copy-pasting from the Quick Start flow can fail if the user is still in examples/.
Proposed doc patch
Run with CubeSandbox:
```bash
+cd tool/codeexec
export E2B_API_KEY=dummy
export E2B_API_URL=http://<your-sandbox-server>:3000
export SSL_CERT_FILE=<caPATH>/rootCA.pem
./codeexec-demo -model deepseek-v4-flash -executor e2b</details>
<details><summary>中文</summary>
请补充工作目录切换步骤以保证可复现性。当前 CubeSandbox 片段缺少 `cd tool/codeexec`,而其他执行器都有;用户按 Quick Start 直接复制时,若仍在 `examples/` 目录可能执行失败。
</details>
As per coding guidelines, “Examples represent the recommended usage and the user's first impression. Review them for reproducibility, clarity, and safe execution,” and “Whether runtime steps, dependencies, environment variables, and side effects are clear.”
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
Run with CubeSandbox:
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/tool/codeexec/README.md` around lines 52 - 59, Add the missing
working-directory step to the CubeSandbox snippet in the README so the commands
run reproducibly: insert a `cd tool/codeexec` step immediately before exporting
E2B_ENV vars and running `./codeexec-demo` in the CubeSandbox example block (the
same placement used in the other executor snippets) so users copying the Quick
Start from `examples/` land in the correct directory before invoking
`codeexec-demo` with the `-model deepseek-v4-flash -executor e2b` flags.
| export E2B_API_KEY=dummy | ||
| export E2B_API_URL=http://<your-sandbox-server>:3000 | ||
| export SSL_CERT_FILE=<caPATH>/rootCA.pem |
There was a problem hiding this comment.
Mark E2B_API_URL and SSL_CERT_FILE as optional in the example block.
The PR objective and code path indicate these are overrides, but the snippet currently makes them look mandatory. Please label them optional to match the public contract and reduce setup friction.
Proposed doc patch
export E2B_API_KEY=dummy
-export E2B_API_URL=http://<your-sandbox-server>:3000
-export SSL_CERT_FILE=<caPATH>/rootCA.pem
+# Optional: override API endpoint (if not using the default E2B endpoint)
+export E2B_API_URL=http://<your-sandbox-server>:3000
+# Optional: custom CA bundle for self-hosted TLS
+export SSL_CERT_FILE=<caPATH>/rootCA.pem
./codeexec-demo -model deepseek-v4-flash -executor e2b</details>
<details><summary>中文</summary>
建议在示例中明确 `E2B_API_URL` 和 `SSL_CERT_FILE` 为可选项。当前写法看起来像必填,会与实际“覆盖默认值”的语义不一致,也会增加上手门槛。
</details>
As per coding guidelines, “The goal of documentation is to accurately communicate the current public contract,” and “Whether commands, import paths, configuration keys, defaults, and return behavior match the current code.”
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
export E2B_API_KEY=dummy
# Optional: override API endpoint (if not using the default E2B endpoint)
export E2B_API_URL=http://<your-sandbox-server>:3000
# Optional: custom CA bundle for self-hosted TLS
export SSL_CERT_FILE=<caPATH>/rootCA.pem
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/tool/codeexec/README.md` around lines 55 - 57, Update the example
env block in README.md to clearly mark E2B_API_URL and SSL_CERT_FILE as optional
overrides: change the comments or variable descriptions around E2B_API_URL and
SSL_CERT_FILE (while keeping E2B_API_KEY shown as required) so the snippet
communicates that E2B_API_URL and SSL_CERT_FILE are optional overrides rather
than mandatory configuration values (reference the E2B_API_URL and SSL_CERT_FILE
identifiers in the example and add “(optional)” or similar wording next to
them).
There was a problem hiding this comment.
♻️ Duplicate comments (1)
codeexecutor/e2b/internal/codeinterpreter/client.go (1)
162-163:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSet explicit
MinVersion: tls.VersionTLS12on the returnedtls.Config.Static analysis (CWE-327) still flags this line, and the prior review thread marked the same finding as addressed in commit 495760e, but the current state shows
&tls.Config{RootCAs: pool}without an explicit floor. For a framework-level default client, please make the policy explicit so it doesn't silently drift with future Go releases or config reuse.🔒 Proposed fix
- return &tls.Config{RootCAs: pool} + return &tls.Config{ + RootCAs: pool, + MinVersion: tls.VersionTLS12, + }中文
静态分析(CWE-327)仍在此处告警;上一轮评审标记已在 495760e 中修复,但当前代码仍为
&tls.Config{RootCAs: pool},未显式设置最低 TLS 版本。框架级默认客户端建议显式指定MinVersion: tls.VersionTLS12,避免随 Go 版本或配置复用产生隐式策略漂移。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeexecutor/e2b/internal/codeinterpreter/client.go` around lines 162 - 163, The returned TLS configuration created at the return statement "return &tls.Config{RootCAs: pool}" must explicitly set a minimum TLS version; update the returned value to include MinVersion: tls.VersionTLS12 so the config becomes &tls.Config{RootCAs: pool, MinVersion: tls.VersionTLS12} ensuring the framework-level client enforces TLS1.2 as a floor and avoids future default drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@codeexecutor/e2b/internal/codeinterpreter/client.go`:
- Around line 162-163: The returned TLS configuration created at the return
statement "return &tls.Config{RootCAs: pool}" must explicitly set a minimum TLS
version; update the returned value to include MinVersion: tls.VersionTLS12 so
the config becomes &tls.Config{RootCAs: pool, MinVersion: tls.VersionTLS12}
ensuring the framework-level client enforces TLS1.2 as a floor and avoids future
default drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4eb5c0fc-9756-45e1-92ee-841e14a8722a
📒 Files selected for processing (6)
codeexecutor/e2b/e2b.gocodeexecutor/e2b/internal/codeinterpreter/client.gocodeexecutor/e2b/internal/codeinterpreter/sandbox.gocodeexecutor/e2b/internal/codeinterpreter/sandbox_test.gocodeexecutor/e2b/internal/codeinterpreter/testutil_test.goexamples/tool/codeexec/README.md
✅ Files skipped from review due to trivial changes (1)
- examples/tool/codeexec/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- codeexecutor/e2b/e2b.go
- codeexecutor/e2b/internal/codeinterpreter/testutil_test.go
- codeexecutor/e2b/internal/codeinterpreter/sandbox_test.go
- codeexecutor/e2b/internal/codeinterpreter/sandbox.go
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (83.51648%) is below the target coverage (85.00000%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1765 +/- ##
===================================================
- Coverage 89.80970% 89.80029% -0.00942%
===================================================
Files 935 935
Lines 151291 151367 +76
===================================================
+ Hits 135874 135928 +54
- Misses 9703 9718 +15
- Partials 5714 5721 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: joey <zchengjoey@gmail.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
codeexecutor/e2b/internal/codeinterpreter/client_test.go (2)
150-161: ⚡ Quick winAdd RootCAs assertion for consistency.
Unlike
TestBuildTLSConfigFromEnv_MissingCertFile(lines 145-147), this test doesn't verify theRootCAsfield. For consistency and completeness, consider adding an assertion thatRootCAsis non-nil, similar to the missing cert file test.Suggested addition
if cfg.MinVersion != tls.VersionTLS12 { t.Errorf("MinVersion: got %d, want TLS1.2", cfg.MinVersion) } + if cfg.RootCAs == nil { + t.Error("RootCAs should fall back to the system pool, not be nil") + } }中文
为保持一致性添加 RootCAs 断言。
与
TestBuildTLSConfigFromEnv_MissingCertFile(第 145-147 行)不同,此测试未验证RootCAs字段。为保持一致性和完整性,考虑添加断言验证RootCAs非空,类似于缺失证书文件测试。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeexecutor/e2b/internal/codeinterpreter/client_test.go` around lines 150 - 161, The test TestBuildTLSConfigFromEnv_MissingCertDir calls buildTLSConfigFromEnv but doesn't assert RootCAs; add a check to ensure cfg.RootCAs is non-nil (same style as TestBuildTLSConfigFromEnv_MissingCertFile) so the test verifies that RootCAs is populated even when SSL_CERT_DIR is missing.
134-148: ⚡ Quick winStrengthen the system pool fallback verification.
The test expects the TLS config to use the system certificate pool when SSL_CERT_FILE is missing, but only asserts that
RootCAs != nil. While this ensures graceful degradation, it doesn't distinguish between the system pool and an empty fallback pool created whenx509.SystemCertPool()fails (line 122–124 in the implementation).Consider verifying that the pool contains certificates—for example, by attempting to append a test certificate or checking that system pool population succeeded—to confirm the actual fallback behavior matches the test's intent.
中文
加强系统证书池回退的验证。
测试期望在 SSL_CERT_FILE 缺失时使用系统证书池,但只断言了
RootCAs != nil。虽然这确保了优雅降级,但未能区分系统池和x509.SystemCertPool()失败时创建的空回退池(实现第 122–124 行)。考虑验证池中确实包含证书——例如,通过尝试附加测试证书或检查系统池是否成功加载——以确认实际回退行为与测试意图相符。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@codeexecutor/e2b/internal/codeinterpreter/client_test.go` around lines 134 - 148, The test TestBuildTLSConfigFromEnv_MissingCertFile currently only checks cfg.RootCAs != nil which doesn't prove the system cert pool was used; update the test to verify the pool actually contains certificates (e.g., assert len(cfg.RootCAs.Subjects()) > 0 or otherwise confirm Subjects() returns a non-empty slice) so it fails if x509.SystemCertPool() fell back to an empty pool; reference buildTLSConfigFromEnv and the TestBuildTLSConfigFromEnv_MissingCertFile test to locate where to add this stronger assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@codeexecutor/e2b/internal/codeinterpreter/client_test.go`:
- Around line 150-161: The test TestBuildTLSConfigFromEnv_MissingCertDir calls
buildTLSConfigFromEnv but doesn't assert RootCAs; add a check to ensure
cfg.RootCAs is non-nil (same style as TestBuildTLSConfigFromEnv_MissingCertFile)
so the test verifies that RootCAs is populated even when SSL_CERT_DIR is
missing.
- Around line 134-148: The test TestBuildTLSConfigFromEnv_MissingCertFile
currently only checks cfg.RootCAs != nil which doesn't prove the system cert
pool was used; update the test to verify the pool actually contains certificates
(e.g., assert len(cfg.RootCAs.Subjects()) > 0 or otherwise confirm Subjects()
returns a non-empty slice) so it fails if x509.SystemCertPool() fell back to an
empty pool; reference buildTLSConfigFromEnv and the
TestBuildTLSConfigFromEnv_MissingCertFile test to locate where to add this
stronger assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8d595ed1-7bfb-453e-b522-092fce4123d9
📒 Files selected for processing (7)
codeexecutor/e2b/e2b.gocodeexecutor/e2b/internal/codeinterpreter/client.gocodeexecutor/e2b/internal/codeinterpreter/client_test.gocodeexecutor/e2b/internal/codeinterpreter/sandbox.gocodeexecutor/e2b/internal/codeinterpreter/sandbox_test.gocodeexecutor/e2b/internal/codeinterpreter/testutil_test.goexamples/tool/codeexec/README.md
✅ Files skipped from review due to trivial changes (1)
- examples/tool/codeexec/README.md
🚧 Files skipped from review as they are similar to previous changes (5)
- codeexecutor/e2b/internal/codeinterpreter/testutil_test.go
- codeexecutor/e2b/e2b.go
- codeexecutor/e2b/internal/codeinterpreter/sandbox.go
- codeexecutor/e2b/internal/codeinterpreter/client.go
- codeexecutor/e2b/internal/codeinterpreter/sandbox_test.go
run e2b with cubesandbox:

cubesandbox log:
