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) <noreply@anthropic.com>
This commit is contained in:
Hongming Wang 2026-04-20 09:06:20 -07:00
parent f71f408c20
commit 0a06cb4fc9
2 changed files with 35 additions and 1 deletions

View File

@ -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

View File

@ -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) {