fix(api): POST /sandboxes/{id}/connect accepts non-positive timeout values causing immediate termination
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 1.6k
- Forks
- 438
- PR merge metrics
- No merged PRs in 30d
Description
Problem
In POST /sandboxes/{id}/connect (packages/api/internal/handlers/sandbox_connect.go), the request payload requires a timeout integer field (ConnectSandbox.timeout).
Unlike all other sandbox lifecycle endpoints—POST /sandboxes (sandbox_create.go:L156), POST /sandboxes/{id}/resume (sandbox_resume.go:L63), and POST /sandboxes/{id}/fork (sandbox_fork.go:L70)—which explicitly reject non-positive timeouts with 400 Bad Request ("Timeout must be greater than 0"), sandbox_connect.go only verifies the upper bound:
timeout := time.Duration(body.Timeout) * time.Second
if timeout > time.Duration(teamInfo.Limits.MaxLengthHours)*time.Hour {
a.sendAPIStoreError(c, http.StatusBadRequest, fmt.Sprintf("Timeout cannot be greater than %d hours", teamInfo.Limits.MaxLengthHours))
return
}
If a client sends {"timeout": 0} or a negative value like {"timeout": -60}, this check passes without error. timeout is then calculated as 0s or a negative duration. When startSandbox is subsequently invoked, it sets endTime = startTime.Add(timeout) in the past or immediately at now. When the orchestrator processes the sandbox lifecycle, it sees that the sandbox has already expired and immediately reaps it upon connection, resulting in sudden disconnections for clients without an informative error message.
Root Cause
sandbox_connect.go omitted the lower-bound validation check if body.Timeout <= 0:
| Handler | Validation Check | Response for timeout: 0 / timeout: -1 |
|---|---|---|
POST /sandboxes (sandbox_create.go) |
if *body.Timeout <= 0 |
400 Bad Request ("Timeout must be greater than 0") |
POST /sandboxes/{id}/resume (sandbox_resume.go) |
if *body.Timeout <= 0 |
400 Bad Request ("Timeout must be greater than 0") |
POST /sandboxes/{id}/fork (sandbox_fork.go) |
if *body.Timeout <= 0 |
400 Bad Request ("Timeout must be greater than 0") |
POST /sandboxes/{id}/connect (Current) |
None (Upper-bound only) | 200 OK (Bypassed -> Instant Termination) |
POST /sandboxes/{id}/connect (Expected) |
if body.Timeout <= 0 |
400 Bad Request ("Timeout must be greater than 0") |
Reproduction Steps
- Send
POST /sandboxes/{id}/connectwith body{"timeout": 0}or{"timeout": -1}. - Observed: Request passes validation and attempts connection with zero/negative duration.
- Expected: Handler returns
400 Bad Requestwith payload{"code": 400, "message": "Timeout must be greater than 0"}immediately before any database or orchestrator operations.
Technical Context
- File affected:
packages/api/internal/handlers/sandbox_connect.go - Subsystem: Control Plane API / Sandboxes / Input Validation
- Impact: Medium (Consistency across API endpoints and prevention of premature sandbox termination)
Proposed Changes
| # | Change | File(s) Affected | Complexity |
|---|---|---|---|
| 1 | Add if body.Timeout <= 0 check returning 400 Bad Request in sandbox_connect.go |
packages/api/internal/handlers/sandbox_connect.go |
Trivial |
| 2 | Add TestSandboxConnect_RejectsNonPositiveTimeout test cases in sandbox_timeout_validation_test.go |
packages/api/internal/handlers/sandbox_timeout_validation_test.go |
Low |
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in packages/api/internal/handlers/sandbox_connect.go and compare its timeout validation with sandbox_create.go, sandbox_resume.go, and sandbox_fork.go. Then run or extend packages/api/internal/handlers/sandbox_timeout_validation_test.go for zero and negative values. Done means POST /sandboxes/{id}/connect returns 400 with "Timeout must be greater than 0" before connection work begins.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100