From 0a06cb4fc970e342b5e914fe9f18fed01bd8c73f Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Mon, 20 Apr 2026 09:06:20 -0700 Subject: [PATCH] fix(cp_provisioner): cap IsRunning body read at 64 KiB IsRunning used an unbounded json.NewDecoder(resp.Body).Decode on CP status responses. Start already caps its body read at 64 KiB (cp_provisioner.go:137) to defend against a misconfigured or compromised CP streaming a huge body and exhausting memory. IsRunning is called reactively per-request from a2a_proxy and periodically from healthsweep, so it's a hotter path than Start and arguably deserves the same defense more. Adds TestIsRunning_BoundedBodyRead that serves a body padded past the cap and asserts the decode still succeeds on the JSON prefix. Follow-up to code-review Nit-2 on #1073. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../internal/provisioner/cp_provisioner.go | 6 +++- .../provisioner/cp_provisioner_test.go | 30 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/workspace-server/internal/provisioner/cp_provisioner.go b/workspace-server/internal/provisioner/cp_provisioner.go index a13e03e6..a081e6a9 100644 --- a/workspace-server/internal/provisioner/cp_provisioner.go +++ b/workspace-server/internal/provisioner/cp_provisioner.go @@ -205,7 +205,11 @@ func (p *CPProvisioner) IsRunning(ctx context.Context, workspaceID string) (bool return true, fmt.Errorf("cp provisioner: status: unexpected %d", resp.StatusCode) } var result struct{ State string `json:"state"` } - if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + // Cap body read at 64 KiB for parity with Start — a misconfigured + // or compromised CP streaming a huge body could otherwise exhaust + // memory in this hot path (called reactively per-request from + // a2a_proxy and periodically from healthsweep). + if err := json.NewDecoder(io.LimitReader(resp.Body, 64<<10)).Decode(&result); err != nil { return true, fmt.Errorf("cp provisioner: status decode: %w", err) } return result.State == "running", nil diff --git a/workspace-server/internal/provisioner/cp_provisioner_test.go b/workspace-server/internal/provisioner/cp_provisioner_test.go index 2500bcf6..162e8717 100644 --- a/workspace-server/internal/provisioner/cp_provisioner_test.go +++ b/workspace-server/internal/provisioner/cp_provisioner_test.go @@ -478,6 +478,36 @@ func TestIsRunning_ContractCompat_A2AProxy(t *testing.T) { }) } +// TestIsRunning_BoundedBodyRead — IsRunning caps the decoded body at +// 64 KiB so a misconfigured/compromised CP streaming a huge body can't +// exhaust memory in this hot path. We serve a body larger than the cap +// and assert the decode either succeeds (json terminated within cap) or +// fails with a decode error (json truncated) — what MUST NOT happen is +// the whole body getting buffered. +// +// Implementation note: the test server writes a valid JSON object +// whose trailing whitespace pads the body past 64 KiB. The decoder +// only needs the prefix to produce a value, so the decode succeeds — +// and the LimitReader enforces the cap regardless. +func TestIsRunning_BoundedBodyRead(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(200) + // Valid JSON prefix, then pad well past the 64 KiB cap. + _, _ = io.WriteString(w, `{"state":"running"}`) + _, _ = io.WriteString(w, strings.Repeat(" ", 128<<10)) + })) + defer srv.Close() + + p := &CPProvisioner{baseURL: srv.URL, orgID: "org-1", httpClient: srv.Client()} + got, err := p.IsRunning(context.Background(), "ws-1") + if err != nil { + t.Fatalf("decode of prefix should succeed, got err %v", err) + } + if !got { + t.Errorf("state=running in prefix should decode to true, got false") + } +} + // TestClose_Noop — explicit contract: Close has no side effects and // no error. Exists for the Provisioner interface; compliance guard. func TestClose_Noop(t *testing.T) {