compute: exec hardening — no auth-replay of mutations, wall-clock deadlines, structured errors

pkg/cli/compute.go

@@ -5,6 +5,7 @@ import (
 	"crypto/rand"
 	"encoding/hex"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"strconv"
 	"strings"
@@ -559,10 +560,40 @@ var computeSandboxesExecCmd = &cobra.Command{
 		opArgs["command"] = command
 		setIfNotEmpty(opArgs, "timeout", computeExecTimeout)
 
-		return runComputeRaw(cmd, "compute.exec_sandbox", opArgs)
+		budget := computeExecDeadline(computeExecTimeout)
+
+		ctx, cancel := context.WithTimeout(commandContext(cmd), budget)
+		defer cancel()
+
+		err := runComputeRawContext(ctx, "compute.exec_sandbox", opArgs)
+		if err != nil && errors.Is(ctx.Err(), context.DeadlineExceeded) {
+			return fmt.Errorf(
+				"no response after %s — the guest command may still be running; check the sandbox with 'panda compute sandboxes get %s'",
+				budget, args[0],
+			)
+		}
+
+		return err
 	},
 }
 
+// computeExecDeadline is the CLI wall-clock budget for one exec call: the
+// guest timeout (or the 30s server default) plus transport headroom. Invalid
+// --timeout values keep the default budget; the server still rejects them.
+func computeExecDeadline(timeout string) time.Duration {
+	const (
+		defaultGuestTimeout = 30 * time.Second
+		transportHeadroom   = time.Minute
+	)
+
+	guest := defaultGuestTimeout
+	if parsed, err := time.ParseDuration(timeout); err == nil && parsed > 0 {
+		guest = parsed
+	}
+
+	return guest + transportHeadroom
+}
+
 var computeSandboxesMetricsCmd = &cobra.Command{
 	Use:   "metrics <id>",
 	Short: "Fetch guest resource metrics for a sandbox",
@@ -969,7 +1000,11 @@ func computeIdemOrGenerated() string {
 // single object) and raw JSON when --output json is set. List results honour
 // --filter, which the server applies before pagination.
 func runComputeRaw(cmd *cobra.Command, operationID string, args map[string]any) error {
-	response, err := runServerOperationRaw(cmd, operationID, args)
+	return runComputeRawContext(commandContext(cmd), operationID, args)
+}
+
+func runComputeRawContext(ctx context.Context, operationID string, args map[string]any) error {
+	response, err := serverOperationRaw(ctx, operationID, args)
 	if err != nil {
 		return err
 	}

pkg/cli/compute_test.go

@@ -0,0 +1,32 @@
+package cli
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestComputeExecDeadline(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name    string
+		timeout string
+		want    time.Duration
+	}{
+		{name: "unset uses the server default budget", timeout: "", want: 90 * time.Second},
+		{name: "explicit timeout adds headroom", timeout: "2m", want: 3 * time.Minute},
+		{name: "max guest timeout adds headroom", timeout: "5m", want: 6 * time.Minute},
+		{name: "unparseable falls back to the default budget", timeout: "bogus", want: 90 * time.Second},
+		{name: "non-positive falls back to the default budget", timeout: "-10s", want: 90 * time.Second},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			assert.Equal(t, tt.want, computeExecDeadline(tt.timeout))
+		})
+	}
+}

pkg/cli/serverclient.go

@@ -230,22 +230,39 @@ func decodeAPIError(status int, data []byte) error {
 // apiErrorMessage extracts a human-readable message from an error response body.
 // It tries the common JSON error fields in order — panda's `error`, then RFC
 // 7807 problem+json `detail` and `title` — before falling back to the raw body.
-// The raw-body fallback covers non-JSON responses (e.g. an unknown route's
-// `text/plain` "404 page not found"), so decoding never panics or loses the
-// message.
+// Structured `code` and `request_id` fields are appended when present so the
+// failure is actionable and traceable. The raw-body fallback covers non-JSON
+// responses (e.g. an unknown route's `text/plain` "404 page not found"), so
+// decoding never panics or loses the message.
 func apiErrorMessage(data []byte) string {
 	var payload map[string]any
 	if err := json.Unmarshal(data, &payload); err == nil {
 		for _, key := range []string{"error", "detail", "title"} {
 			if msg, ok := payload[key].(string); ok && msg != "" {
-				return msg
+				return msg + apiErrorContext(payload)
 			}
 		}
 	}
 
 	return strings.TrimSpace(string(data))
 }
 
+func apiErrorContext(payload map[string]any) string {
+	parts := make([]string, 0, 2)
+
+	for _, key := range []string{"code", "request_id"} {
+		if value, ok := payload[key].(string); ok && value != "" {
+			parts = append(parts, key+"="+value)
+		}
+	}
+
+	if len(parts) == 0 {
+		return ""
+	}
+
+	return " (" + strings.Join(parts, ", ") + ")"
+}
+
 func serverErrorHint(status int, message string) string {
 	// Error classification is the ClickHouse module's knowledge; the CLI only
 	// owns the command-idiom wording per class.
@@ -298,6 +315,8 @@ func serverErrorHint(status int, message string) string {
 	switch status {
 	case http.StatusNotFound:
 		return "the requested module, operation, datasource, or resource is not available on this server; check 'panda datasources' and 'panda resources'"
+	case http.StatusInternalServerError:
+		return "the backend hit an internal error; for compute operations check the sandbox with 'panda compute sandboxes get <id>' — the guest may be unreachable"
 	case http.StatusBadGateway:
 		return "an upstream datasource or node returned a gateway error; the target may be temporarily unreachable — retry, or confirm it is still advertised with 'panda datasources'"
 	case http.StatusServiceUnavailable:

pkg/cli/serverclient_test.go

@@ -0,0 +1,61 @@
+package cli
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestAPIErrorMessage(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		body string
+		want string
+	}{
+		{
+			name: "plain error field",
+			body: `{"error":"sandbox not found"}`,
+			want: "sandbox not found",
+		},
+		{
+			name: "error with code and request id",
+			body: `{"error":"guest unreachable","code":"guest_unreachable","request_id":"req-123"}`,
+			want: "guest unreachable (code=guest_unreachable, request_id=req-123)",
+		},
+		{
+			name: "error with code only",
+			body: `{"error":"guest unreachable","code":"guest_unreachable"}`,
+			want: "guest unreachable (code=guest_unreachable)",
+		},
+		{
+			name: "error with request id only",
+			body: `{"error":"internal error","request_id":"req-456"}`,
+			want: "internal error (request_id=req-456)",
+		},
+		{
+			name: "problem json detail",
+			body: `{"detail":"upstream failed","title":"Bad Gateway"}`,
+			want: "upstream failed",
+		},
+		{
+			name: "non-string structured fields are ignored",
+			body: `{"error":"boom","code":42}`,
+			want: "boom",
+		},
+		{
+			name: "non-json falls back to raw body",
+			body: "404 page not found\n",
+			want: "404 page not found",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			assert.Equal(t, tt.want, apiErrorMessage([]byte(tt.body)))
+		})
+	}
+}

pkg/proxy/handlers/transport.go

@@ -14,6 +14,11 @@ const (
 	defaultMaxIdleConns        = 100
 	defaultMaxIdleConnsPerHost = 10
 	defaultIdleConnTimeout     = 90 * time.Second
+
+	// Above the 5m compute exec ceiling (sync exec sends headers only after
+	// the command completes) and below the 6m server write timeout, so a hung
+	// backend fails fast instead of holding the connection open.
+	defaultResponseHeaderTimeout = 5*time.Minute + 30*time.Second
 )
 
 // newProxyTransport returns an *http.Transport with sensible defaults for
@@ -24,10 +29,11 @@ func newProxyTransport(skipVerify bool) *http.Transport {
 		TLSClientConfig: &tls.Config{
 			InsecureSkipVerify: skipVerify, //nolint:gosec // User-configured per datasource
 		},
-		DialContext:         (&net.Dialer{Timeout: defaultDialTimeout}).DialContext,
-		TLSHandshakeTimeout: defaultTLSHandshakeTimeout,
-		MaxIdleConns:        defaultMaxIdleConns,
-		MaxIdleConnsPerHost: defaultMaxIdleConnsPerHost,
-		IdleConnTimeout:     defaultIdleConnTimeout,
+		DialContext:           (&net.Dialer{Timeout: defaultDialTimeout}).DialContext,
+		TLSHandshakeTimeout:   defaultTLSHandshakeTimeout,
+		MaxIdleConns:          defaultMaxIdleConns,
+		MaxIdleConnsPerHost:   defaultMaxIdleConnsPerHost,
+		IdleConnTimeout:       defaultIdleConnTimeout,
+		ResponseHeaderTimeout: defaultResponseHeaderTimeout,
 	}
 }

pkg/proxy/server_config.go

@@ -482,8 +482,10 @@ func (c *ServerConfig) ApplyDefaults() {
 		c.Server.ReadTimeout = 30 * time.Second
 	}
 
+	// Above the 5m compute exec ceiling, so a near-max guest command cannot
+	// race the server deadline.
 	if c.Server.WriteTimeout == 0 {
-		c.Server.WriteTimeout = 5 * time.Minute
+		c.Server.WriteTimeout = 6 * time.Minute
 	}
 
 	if c.Server.IdleTimeout == 0 {

pkg/server/api.go

@@ -760,6 +760,13 @@ func (s *service) handleAPIBuildStatus(w http.ResponseWriter, r *http.Request) {
 	writeJSON(w, http.StatusOK, proxyResp)
 }
 
+// Replay policy for proxied requests: a replayable request may be re-sent once
+// after an auth rejection; a non-replayable one must never run twice upstream.
+const (
+	proxyReplayable = true
+	proxyNoReplay   = false
+)
+
 func (s *service) proxyRequest(
 	ctx context.Context,
 	method string,
@@ -772,7 +779,7 @@ func (s *service) proxyRequest(
 		return nil, http.StatusServiceUnavailable, nil, err
 	}
 
-	return s.proxyRequestWithService(ctx, proxySvc, method, requestPath, body, headers)
+	return s.proxyRequestWithService(ctx, proxySvc, method, requestPath, body, headers, proxyReplayable)
 }
 
 func (s *service) proxyDatasourceRequest(
@@ -783,13 +790,14 @@ func (s *service) proxyDatasourceRequest(
 	requestPath string,
 	body io.Reader,
 	headers http.Header,
+	replayable bool,
 ) ([]byte, int, http.Header, error) {
 	proxySvc, status, err := s.proxyServiceForDatasource(datasourceType, datasourceName)
 	if err != nil {
 		return nil, status, nil, err
 	}
 
-	return s.proxyRequestWithService(ctx, proxySvc, method, requestPath, body, headers)
+	return s.proxyRequestWithService(ctx, proxySvc, method, requestPath, body, headers, replayable)
 }
 
 func (s *service) proxyRequestWithService(
@@ -799,6 +807,7 @@ func (s *service) proxyRequestWithService(
 	requestPath string,
 	body io.Reader,
 	headers http.Header,
+	replayable bool,
 ) ([]byte, int, http.Header, error) {
 	if s.proxyService == nil {
 		return nil, http.StatusServiceUnavailable, nil, fmt.Errorf("proxy service is unavailable")
@@ -874,11 +883,17 @@ func (s *service) proxyRequestWithService(
 	}
 
 	// One invalidate-and-retry on auth rejection covers a token revoked
-	// server-side before the local refresh buffer kicks in.
+	// server-side before the local refresh buffer kicks in. Non-replayable
+	// requests still refresh the token but must not run twice upstream.
 	if status == http.StatusUnauthorized || status == http.StatusForbidden {
 		proxySvc.Invalidate()
 
-		return send()
+		if replayable {
+			return send()
+		}
+
+		return nil, status, header, fmt.Errorf(
+			"authentication expired mid-request; token refreshed — retry the command")
 	}
 
 	return data, status, header, nil

pkg/server/operations_benchmarkoor.go

@@ -185,6 +185,7 @@ func (s *service) handleBenchmarkoorGetRun(w http.ResponseWriter, r *http.Reques
 		benchmarkoorAPIPrefix+"/index/query/runs?"+params.Encode(),
 		nil,
 		http.Header{handlers.DatasourceHeader: []string{datasource}},
+		proxyReplayable,
 	)
 	if err != nil {
 		writeAPIError(w, status, err.Error())

pkg/server/operations_clickhouse.go

@@ -108,6 +108,7 @@ func (s *service) handleClickHouseQuery(w http.ResponseWriter, r *http.Request)
 			handlers.DatasourceHeader: []string{datasource},
 			"Content-Type":            []string{"text/plain"},
 		},
+		proxyReplayable,
 	)
 	if err != nil {
 		writeAPIError(w, status, err.Error())

pkg/server/operations_compute.go

@@ -57,6 +57,13 @@ func (d *computeProxyDoer) Do(req *http.Request) (*http.Response, error) {
 
 	headers.Set(handlers.DatasourceHeader, d.datasource)
 
+	// Compute mutations (exec above all) must never run twice, so only
+	// read-only methods keep the proxy's invalidate-and-retry on auth rejection.
+	replayable := proxyNoReplay
+	if req.Method == http.MethodGet || req.Method == http.MethodHead {
+		replayable = proxyReplayable
+	}
+
 	respBody, status, respHeaders, err := d.svc.proxyDatasourceRequest(
 		req.Context(),
 		"compute",
@@ -65,6 +72,7 @@ func (d *computeProxyDoer) Do(req *http.Request) (*http.Response, error) {
 		requestPath,
 		body,
 		headers,
+		replayable,
 	)
 	if err != nil {
 		return nil, err
@@ -685,6 +693,14 @@ func (s *service) writeComputeResult(w http.ResponseWriter, resp *http.Response,
 	}
 
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		// Forward JSON error objects verbatim so structured fields (code,
+		// request_id) survive to the CLI instead of being nested in a wrapper.
+		if trimmed := bytes.TrimSpace(body); jsonObjectBody(trimmed) {
+			writePassthroughResponse(w, resp.StatusCode, "application/json", trimmed)
+
+			return
+		}
+
 		writeAPIError(w, resp.StatusCode, strings.TrimSpace(string(body)))
 
 		return
@@ -700,6 +716,17 @@ func (s *service) writeComputeResult(w http.ResponseWriter, resp *http.Response,
 	writePassthroughResponse(w, resp.StatusCode, contentType, body)
 }
 
+// jsonObjectBody reports whether body is a JSON object that can be forwarded
+// to the caller verbatim.
+func jsonObjectBody(body []byte) bool {
+	var payload map[string]any
+	if err := json.Unmarshal(body, &payload); err != nil {
+		return false
+	}
+
+	return payload != nil
+}
+
 func (s *service) computeDatasources() ([]types.DatasourceInfo, error) {
 	if s.proxyService == nil {
 		return nil, fmt.Errorf("compute is unavailable")