fix(sandbox): stop sending preview token as URL query param in from_session#186
Conversation
…ession The session preview token was attached both as the X-Blaxel-Preview-Token header and a redundant bl_preview_token entry in SandboxConfiguration.params. The header alone authenticates this programmatic client, so the query-param copy only risks leaking a ~24h full-sandbox-control credential into access logs, Referer headers, and browser history. Remove it from from_session() in both the async and sync variants and add regression tests asserting from_session() produces no credential-bearing params. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
🧪 Testing GuideWhat this PR addresses
This PR removes the Steps to reproduce the original issue
What to verify (expected behavior)
Note Posted by PR Testing Guide · Tag @mendral-app with feedback. |
There was a problem hiding this comment.
LGTM
Clean, minimal security hardening. The removal is safe because SandboxConfiguration.params defaults to {} when omitted, and the tests correctly verify this. No correctness, concurrency, or data risks introduced.
Tag @mendral-app with feedback or questions. View session
🔀 Interaction Flow DiagramHere's how the sequenceDiagram
participant Caller as Caller Code
participant SF as SandboxInstance.from_session()
participant Cfg as SandboxConfiguration
participant HTTP as httpx Client
participant API as Sandbox API
Caller->>SF: from_session(session)
SF->>Cfg: Create config (host, scheme)
Note over SF,Cfg: ❌ No longer sets params={"bl_preview_token": token}
SF->>HTTP: Build client with headers
Note over HTTP: X-Blaxel-Preview-Token: session.token
HTTP->>API: Request (token in header only)
Note right of API: ✅ Token not leaked in URL/query string
API-->>HTTP: Response
HTTP-->>SF: Result
SF-->>Caller: SandboxInstance
SummaryBefore: The preview token was sent in both the After: The token is transmitted exclusively via the HTTP header, which is the correct channel for programmatic SDK clients. The query-param path exists only for browser navigation (which can't set custom headers) and is not needed here. Components affected:
Note Posted by PR Sequence Diagram · Tag @mendral-app with feedback. |
|
📋 Created Linear issue ENG-3840 — status: In Progress
Auto-created because no Linear reference was found in the PR title, description, or branch name. Note Posted by Linear Issue Enforcer · Tag @mendral-app with feedback. |
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Impact summary & bot-feedback triageAutomated feedback: Mendral LGTM (safe minimal hardening); Testing Guide, Sequence Diagram, Linear Enforcer (ENG-3840), Observability = informational/pass. Supply Chain Review correctly skipped (no dep changes). Cursor Bugbot not run (team free-tier disabled); Cursor Security Agent + CodeQL pass. No actionable code finding attributable to this PR — nothing to change on the branch. Devin Review: posts No Issues Found but the badge notes "1 additional finding" only visible in the web review. Flagging for a human to confirm it's not a blocker: https://app.devin.ai/review/blaxel-ai/sdk-python/pull/186 CI — 2 failures verified NOT from this diff (preexisting/environmental):
Neither test references Impact: behavior-preserving — removes a dead, credential-bearing Out of scope (separate): devbox Not merging — this is analysis only. |
Fixes ENG-3840
Summary
Fixes Tolmo finding 5a4bcd81 (LOW).
SandboxInstance.from_session()attached the session preview token in two places — theX-Blaxel-Preview-Tokenheader and abl_preview_tokenentry inSandboxConfiguration.params. On this programmatic client the header alone authenticates, so the query-param copy only risked leaking a ~24h full-sandbox-control credential (filesystem CRUD,process.exec, network proxy) into access logs,Refererheaders, and browser history. The query-param transport exists for browser navigation (which can't set custom headers), not for the SDK.This removes the
paramskwarg fromfrom_session()in both the async and sync variants:return cls( sandbox=sandbox, force_url=session.url, headers={"X-Blaxel-Preview-Token": session.token}, - params={"bl_preview_token": session.token}, )Behavior-preserving:
SandboxConfiguration.paramswas never actually attached to any outgoing request in the current code (both request paths —get_client()and the generatedClient— send headers only; theparams=...calls insidefilesystem.pyare unrelated method-local query dicts). So this deletes dead, credential-bearing config and hardens against a future change accidentally wiring it onto requests. Header-based auth is already exercised by the session integration tests (fs.ls,process.exec,stream_logs,watch).Changes
src/blaxel/core/sandbox/default/sandbox.py— dropparams={"bl_preview_token": ...}from asyncfrom_session().src/blaxel/core/sandbox/sync/sandbox.py— same for syncfrom_session().tests/core/test_sandbox.py— regression tests (async + sync) assertingfrom_session().config.params == {}, the header is still set, and the token never appears in the persistent HTTP client's default params.Out of scope (separate repo)
The report also flags the devbox launcher printing the token-bearing preview URL to stdout (
launch.pyin blaxel-ai/blaxel-devbox), which is the realistic CI-log leak path. That lives in another repository and is not addressed here — it should be tracked/fixed separately (redact the token before printing). The report's longer-term recommendation (token-exchange → HttpOnly cookie for the browser path, shorter session TTL) is likewise a control-plane/proxy change, not an SDK change.Link to Devin session: https://app.devin.ai/sessions/1599b0c6fe27498ba14d6e691b4b406e
Requested by: @drappier-charles
Note
Removes the redundant
bl_preview_tokenquery parameter fromfrom_session()in both async and sync sandbox variants, keeping only theX-Blaxel-Preview-Tokenheader for authentication. Adds regression tests to prevent reintroduction.Written by Mendral for commit 870b6d7.