diff --git a/workspace-server/internal/provisioner/cp_provisioner.go b/workspace-server/internal/provisioner/cp_provisioner.go index 269816909..ef1f8595c 100644 --- a/workspace-server/internal/provisioner/cp_provisioner.go +++ b/workspace-server/internal/provisioner/cp_provisioner.go @@ -241,9 +241,12 @@ func (p *CPProvisioner) Start(ctx context.Context, cfg WorkspaceConfig) (string, // Cap body read at 64 KiB — the CP only ever returns small JSON // responses; an unbounded read could be weaponized into log-flood // DoS by a compromised upstream. - respBody, _ := io.ReadAll(io.LimitReader(resp.Body, 64<<10)) + respBody, readErr := io.ReadAll(io.LimitReader(resp.Body, 64<<10)) + if readErr != nil { + return "", fmt.Errorf("cp provisioner: read response body: %w", readErr) + } var result cpProvisionResponse - json.Unmarshal(respBody, &result) + unmarshalErr := json.Unmarshal(respBody, &result) if resp.StatusCode != http.StatusCreated { // Prefer the structured {"error":"..."} field. Do NOT fall back @@ -257,6 +260,10 @@ func (p *CPProvisioner) Start(ctx context.Context, cfg WorkspaceConfig) (string, return "", fmt.Errorf("cp provisioner: provision failed (%d): %s", resp.StatusCode, errMsg) } + if unmarshalErr != nil { + return "", fmt.Errorf("cp provisioner: decode 201 response: %w", unmarshalErr) + } + log.Printf("CP provisioner: workspace %s → EC2 instance %s (%s)", cfg.WorkspaceID, result.InstanceID, result.State) provlog.Event("provision.ec2_started", map[string]any{ "workspace_id": cfg.WorkspaceID, @@ -409,7 +416,11 @@ func (p *CPProvisioner) Stop(ctx context.Context, workspaceID string) error { // Read a bounded slice of the body so the error message gives ops // enough to triage without risking a multi-MB log line on a // pathological response. 512 bytes covers any sane error envelope. - body, _ := io.ReadAll(io.LimitReader(resp.Body, 512)) + body, readErr := io.ReadAll(io.LimitReader(resp.Body, 512)) + if readErr != nil { + return fmt.Errorf("cp provisioner: stop %s: unexpected %d (read body failed: %w)", + workspaceID, resp.StatusCode, readErr) + } return fmt.Errorf("cp provisioner: stop %s: unexpected %d: %s", workspaceID, resp.StatusCode, strings.TrimSpace(string(body))) } diff --git a/workspace-server/internal/provisioner/cp_provisioner_test.go b/workspace-server/internal/provisioner/cp_provisioner_test.go index 44e27660a..23c747882 100644 --- a/workspace-server/internal/provisioner/cp_provisioner_test.go +++ b/workspace-server/internal/provisioner/cp_provisioner_test.go @@ -442,6 +442,26 @@ func TestStart_SymlinkTemplatePathError(t *testing.T) { } } +// TestStart_Malformed201SurfacesError — when CP returns 201 Created with +// unparseable JSON, Start must return an error instead of silently +// returning an empty instance_id. CR2 blocker from review #5552. +func TestStart_Malformed201SurfacesError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusCreated) + _, _ = io.WriteString(w, `{"instance_id": broken-json`) + })) + defer srv.Close() + + p := &CPProvisioner{baseURL: srv.URL, orgID: "org-1", httpClient: srv.Client()} + _, err := p.Start(context.Background(), WorkspaceConfig{WorkspaceID: "ws-1", Runtime: "py"}) + if err == nil { + t.Fatal("expected error on malformed 201, got nil") + } + if !strings.Contains(err.Error(), "decode 201 response") { + t.Errorf("error should mention decode 201 response, got %q", err.Error()) + } +} + // TestStop_SendsBothAuthHeaders — verify #118/#130 compliance on the // teardown path. Any call to /cp/workspaces/:id must carry both the // platform-wide shared secret AND the per-tenant admin token, or the