fix(api): non-positive timeout on sandbox create/resume/fork bypasses validation and causes instant termination

[chill-czar]

Problem

Passing timeout: 0 or a negative value (e.g. timeout: -10) to the sandbox create, resume, or fork API endpoints is accepted without error. The sandbox is created successfully but immediately expires — the orchestrator's background cleanup loop finds its EndTime already in the past (or at now) and kills it before the caller can do anything with it.

A unit test that demonstrates this returns 404 — not 400:

--- FAIL: TestPostSandboxes_InvalidTimeout (4.14s)
    sandbox_create_test.go:1171:
        Error Trace: sandbox_create_test.go:1171
        Error:       Not equal:
                     expected: 400
                     actual  : 404
        Test:        TestPostSandboxes_InvalidTimeout

The 404 is from the template-not-found path, showing the request advanced past input validation entirely. When the template does exist, the request succeeds with 201 and the sandbox is immediately dead.

Root Cause

All three handlers — PostSandboxes, PostSandboxesSandboxIDResume, and PostSandboxesSandboxIDFork — share the same validation pattern:

// packages/api/internal/handlers/sandbox_create.go:L156
// packages/api/internal/handlers/sandbox_resume.go:L59
// packages/api/internal/handlers/sandbox_fork.go:L65
timeout := sandbox.SandboxTimeoutDefault
if body.Timeout != nil {
    timeout = time.Duration(*body.Timeout) * time.Second

    if timeout > time.Duration(teamInfo.Limits.MaxLengthHours)*time.Hour {
        a.sendAPIStoreError(c, http.StatusBadRequest, ...)
        return
    }
}

The only guard is an upper-bound check (timeout > MaxLengthHours). Since -10s < 24h and 0s < 24h, both pass silently. The resulting time.Duration is zero or negative, so EndTime = now + timeout is in the past or at now by the time the sandbox starts.

Compare to PostSandboxesSandboxIDTimeout (the keepalive endpoint), which correctly guards the other direction:

Handler	Lower-bound check	Upper-bound check
PostSandboxes	❌ missing	✅ present
PostSandboxesSandboxIDResume	❌ missing	✅ present
PostSandboxesSandboxIDFork	❌ missing	✅ present
PostSandboxesSandboxIDTimeout	✅ if body.Timeout < 0	—

The OpenAPI spec already declares minimum: 0 on NewSandbox.timeout, so this is also a spec/implementation mismatch.

Reproduction Steps

1. Start a local API with make local-infra and make run-local from packages/api/.
2. Send a sandbox-create request with a non-positive timeout:

curl -X POST http://localhost:3000/sandboxes \
  -H 'X-API-Key: <key>' \
  -H 'Content-Type: application/json' \
  -d '{"templateID": "<valid-id>", "timeout": -10}'

3. Observe: response is 201 Created. The sandbox is created.
4. Immediately call GET /sandboxes/<id> — the sandbox is already gone (killed by the expiration loop) or has an EndTime in the past.

Unit-test reproduction (no running infra required):

func TestPostSandboxes_InvalidTimeout(t *testing.T) {
    // ... setup store with DB + Redis (see existing test helpers) ...
    invalidTimeout := int32(-10)
    body, _ := json.Marshal(api.PostSandboxesJSONRequestBody{
        TemplateID: "<existing-template-id>",
        Timeout:    &invalidTimeout,
    })
    // ... POST /sandboxes ...
    require.Equal(t, http.StatusBadRequest, recorder.Code) // FAILS: actual is 201 or 404
}

Technical Context

• Files affected:
  • packages/api/internal/handlers/sandbox_create.go L156–L164
  • packages/api/internal/handlers/sandbox_resume.go L59–L67
  • packages/api/internal/handlers/sandbox_fork.go L65–L73
• Subsystem: Control Plane REST API — sandbox lifecycle
• Impact: Medium — silently accepted invalid input produces sandboxes that die immediately; confusing for SDK users and hard to diagnose without knowing to check EndTime

Proposed Changes

#	Change	File(s) Affected	Complexity
1	Add if *body.Timeout <= 0 guard before upper-bound check, return 400 Bad Request with message "Timeout must be greater than 0"	sandbox_create.go, sandbox_resume.go, sandbox_fork.go	Low
2	Add unit test cases asserting 400 for timeout: 0 and timeout: -10	sandbox_create_test.go	Low

[Keesan12]

This boundary should be enforced in one shared timeout parser used by create, resume, fork, and keepalive. Otherwise the handlers will drift again. I would make the contract explicit: either timeout >= 1 everywhere, or define 0 as a named semantic such as “use default”—never accept it and create a sandbox that is already dead. The API response should include the normalized timeout and computed expiry so callers can verify what was authorized.

[chill-czar]

This boundary should be enforced in one shared timeout parser used by create, resume, fork, and keepalive. Otherwise the handlers will drift again. I would make the contract explicit: either timeout >= 1 everywhere, or define 0 as a named semantic such as “use default”—never accept it and create a sandbox that is already dead. The API response should include the normalized timeout and computed expiry so callers can verify what was authorized.

Thanks for suggestion, I have updated the pr according to that