refactor(sandbox): move sandbox state management server-side (ENG-4287)#381
Draft
huv1k wants to merge 20 commits into
Draft
refactor(sandbox): move sandbox state management server-side (ENG-4287)#381huv1k wants to merge 20 commits into
huv1k wants to merge 20 commits into
Conversation
Stop exposing the user's account-level access token to browser JS. The filesystem inspector and dashboard terminal previously embedded the account token into client-side Sandbox.connect/create calls via sandboxManagementAuth. Now only the sandbox-scoped envdAccessToken reaches the client: - Add createEnvdSandbox helper that builds an envd-only Sandbox client from sandbox-scoped credentials (no control-plane call, no account token). - Filesystem inspector builds its client from the envd creds already returned by the sandbox.details query. - Terminal create/connect moves into the openTerminalSandboxAction server action; the client builds an envd-only client from the result. - Delete the now-unused sandbox-management-auth modules.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…4287 Resolved conflicts in the terminal feature, integrating main's terminal refactor (attach-retry, useTerminalInstance, launchTarget, killTerminalPty) with this branch's server-side sandbox state management: - Terminal acquisition still runs through openTerminalSandboxAction + createEnvdSandbox (no account token in the browser), now forwarding main's requestTimeoutMs and preserving TimeoutError so attach-retry works. - Dropped sandboxManagementAuth from DashboardTerminal in favor of teamSlug + userId; kept main's launchTarget/getSandbox/sandboxScoped props. - Merged the unit tests (action/createEnvdSandbox mocks + attach-retry suite).
…287 variant)
Alternative to the next-safe-action server action: terminal create/connect
now runs through a `sandbox.openTerminal` tRPC mutation (protectedTeamProcedure,
alongside killTerminalPty). The vanilla tRPC client is injected into
openTerminalSandbox from the component, the return type is inferred from the
router, and timeouts are signaled via a TRPCError('TIMEOUT') the client maps
back to a TimeoutError so attach-retry still works.
This branch exists to compare the two transports; only the terminal-open path
differs from the server-action branch (the filesystem inspector is unchanged).
…4287 Only conflict was tests/unit/dashboard-terminal.test.ts: kept this branch's tRPC-mutation test setup (injected openTerminal mock + createEnvdSandbox), dropping main's stale server-action/SDK-mock assertions and the sandboxManagementAuth fixture. Picked up main's auth refactor (auth.getAuthContext -> getAuthContext) in the terminal/filesystem pages via auto-merge, and installed the new @launchdarkly/node-server-sdk dependency.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aee0b2cd40
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…+ inspector Addresses Codex P2 feedback on #381. The defensive guards required an envdAccessToken (and non-null domain), but those are legitimately absent for secure:false sandboxes (envd is reachable without X-Access-Token) and domain can be null (the SDK falls back to the configured E2B domain) — regressing support that worked before this PR. - create-envd-sandbox: envdAccessToken is now optional. - sandbox.openTerminal: stop throwing when getFullInfo has no envdAccessToken; pass it through as-is. - inspect context: drop the !envdAccessToken || !domain early-return (keep the killed-state narrowing guard). - test: cover connecting to a tokenless sandbox.
Member
Author
|
Addressed both Codex P2 comments in 37592de: @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37592def2a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The follow-up getFullInfo call ignored requestTimeoutMs and sat outside the timeout-to-sentinel catch, so a stalled info GET after a successful connect would wait for the SDK default timeout and fail without the attach-retry path recognizing it. Move getFullInfo inside the same try block and pass the same requestTimeoutMs, so its TimeoutError is normalized to the TIMEOUT sentinel and retried like the connect timeout.
…4287 Reconciles main's terminal/inspect rework with this PR's no-account-token security model (full reconcile per review decision): - main added an explicit "Resume sandbox" feature to the inspector and a sandbox-details terminal tab, both implemented with client-side Sandbox.connect + sandboxManagementAuth (account token in the browser). - Kept main's resume UX and details terminal tab, but routed every control-plane call server-side: normal inspect connect stays envd-only via createEnvdSandbox; inspect resume now calls a new `sandbox.resume` tRPC mutation; the terminal (standalone + details tab) stays on the `sandbox.openTerminal` tRPC mutation. No account token reaches the browser. - Added `sandbox.resume` mutation + shared SANDBOX_RESUME_TIMEOUT_MS constant. - Accepted main's deletion of the client-side attach-retry; dropped the now unused timeout sentinel. DashboardTerminal/inspect take `userId` instead of sandboxManagementAuth; deleted sandbox-management-auth modules.
…4287 Conflicts in the terminal files only (dashboard-terminal.tsx, the details terminal view + page). Kept this branch's no-account-token model — tRPC openTerminal injection + userId instead of sandboxManagementAuth — while adopting main's additions: useEffectEvent-based handlers, resizePty error guard, terminal-size tweaks, and the `command` search param plumbed through the details terminal tab. Picked up main's new input-otp dependency.
…pect view Locks in the lifecycle invariant for the debug/inspect views: - Behavioral: sandbox.resume and sandbox.openTerminal are the only procedures that hit the control plane, and they pass an explicit bounded timeoutMs (SANDBOX_RESUME_TIMEOUT_MS / TERMINAL_SANDBOX_TIMEOUT_MS); create vs connect is chosen correctly. - Structural: the inspect/terminal client modules never call Sandbox.connect/create (they go through createEnvdSandbox / the tRPC mutation), so normal inspect can't resume a paused sandbox or extend its TTL.
Adds a toggle button in the sandbox detail header that pauses a running sandbox (preserving state via memory snapshot) and resumes a paused one, mirroring the existing kill-button confirmation pattern.
auto-merge was automatically disabled
June 24, 2026 17:49
Pull request was converted to draft
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Moves sandbox state management server-side so the user's account-level access token is never exposed to browser JS (ENG-4287). Previously the sandbox-details surfaces drove the E2B SDK client-side with
sandboxManagementAuth(the account token); now the browser only ever receives sandbox-scoped envd credentials.How it works
createEnvdSandboxhelper from theenvdAccessTokenalready returned by thesandbox.detailsquery — no control-plane call, so normal inspect never resumes a paused sandbox or extends its TTL./dashboard/terminaland the sandbox-details terminal tab) runs create/connect through asandbox.openTerminaltRPC mutation; the client receives only scoped envd creds.sandbox.resumetRPC mutation server-side (shortSANDBOX_RESUME_TIMEOUT_MSwindow), then the client rebuilds the envd-only client from the returned creds. Resuming/extending TTL only happens on this explicit user action — never implicitly.secure: false) sandboxes are supported end-to-end (envdAccessTokenoptional throughout).sandbox-management-authmodules are deleted;getFullInfo/sandbox.detailsremain read-only.Notes
origin/mainmerged in — reconciled main's resume UX and details terminal tab onto the no-token model, and adopted main's removal of the client-side attach-retry.Tests: 289 passing; typecheck/lint clean on changed files.
Not yet verified live: open the filesystem inspector + terminal (incl. an explicit Resume, and a
secure: falsesandbox) on the preview deploy and confirm via DevTools that envd requests carry onlyX-Access-Token— no accountAuthorization: Bearer/X-Supabase-Token.Known follow-up (pre-existing, from main):
sandbox.killTerminalPtystill does a control-planeSandbox.connect(resume + extend TTL) just to kill a PTY on terminal close; it should become envd-only to avoid that side effect.