From a6d699ac03720f48feff7313c2f0b624cec69914 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 30 Oct 2026 09:14:38 -0400 Subject: [PATCH 1/3] feat: Serve Go pprof profiles on the stats server when loopback-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The proxy's memory figures were unreadable: RSS on macOS counts pages the GC has freed but the kernel has not reclaimed, and nothing in the tree could decompose the heap. The stats server now mounts net/http/ pprof under /debug/pprof/ when listener.bind_loopback_only is set — the same gate as the session API's clear — because a heap profile dumps process memory, prompts and tokens included. Cluster deployments, where the stats port is not loopback, keep today's surface unchanged. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- CLAUDE.md | 4 +-- cmd/cortex/main.go | 3 ++ core/observe/statserver.go | 55 +++++++++++++++++++++++++-------- core/observe/statserver_test.go | 38 +++++++++++++++++++++++ docs/laptop-service.md | 7 +++++ 5 files changed, 92 insertions(+), 15 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 226beb1f3..efd2e317e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -509,7 +509,7 @@ Envoy config lives in the `envoy-config` ConfigMap rendered by the [rossoctl Hel | 15123 | Envoy | TCP | Outbound listener (iptables redirects app traffic here) | | 15124 | Envoy | TCP | Inbound listener (iptables redirects incoming traffic here) | | 9090 | authbridge | gRPC | Ext-proc server (called by Envoy) | -| 9093 | authbridge | HTTP | Stats + config inspection (`/stats`, `/config`, `/reload/status`, `/tls-bridge/unread`) | +| 9093 | authbridge | HTTP | Stats + config inspection (`/stats`, `/config`, `/reload/status`, `/tls-bridge/unread`, and `/debug/pprof/*` where `listener.bind_loopback_only` is set) | | 9094 | authbridge | HTTP | Session events API (JSON snapshots + SSE stream) | | 9901 | Envoy | HTTP | Admin interface (bound to 127.0.0.1) | @@ -1209,7 +1209,7 @@ resulting `/shared/client-id.txt` and `/shared/client-secret.txt`. The schemas were the last big duplicate and needed a type change to reach. `InferenceTool.Parameters` was `map[string]any`, which cost **4.1x its JSON text** to hold and could not be interned without a recursive walk that rewrites map values — a walk cannot lean on string immutability the way sharing a string can. Measured on a live session: 84KB per event, ~172MB across one 2050-event session. So the field became `pipeline.RawJSON`, a named string type that keeps the schema exactly as the client sent it, and it interns like any other string. Two consequences worth knowing: the API now shows schemas in the client's own key order (a map round-trip silently sorted them), and the type must stay a NAMED string with a `MarshalJSON` method — OPA's `ast.InterfaceToValue` treats a plain or aliased string as a JSON string, which would leave every policy indexing `input.inference.tools[_].parameters` undefined with no error (`core/plugins/opa/tool_parameters_rego_test.go` guards it). MCP `Params`/`Result` still do not intern, being `map[string]any`; they are unmeasured on this workload and the same field-type change is available if that changes. - **Retention was only half of it, and reading the store was the more expensive half.** A memory figure that keeps climbing while traffic is idle is usually not retention: `GET /v1/sessions/{id}` used to encode the whole response into a single buffer before writing a byte, so serving one snapshot cost heap proportional to the *response* — ~246MB of heap growth for a 105MB response — and Go keeps that as idle heap rather than returning it promptly. agentop requests a snapshot on every Enter into a session, so RSS ratcheted a couple hundred megabytes per keystroke: four requests took a live proxy from 731MB to 1447MB, still 1447MB two minutes later. Interning could not have helped there, and the two are not alternatives — interning makes stored events *share* one copy of a repeated message, and an encoder expands every share back into its own bytes on the way out. The response is now written one event at a time (`core/sessionapi/snapshot.go`, `BenchmarkSnapshotHeap`). When judging any memory change here, note that **RSS is not heap** and the proxy exposes no heap metric — there is no `pprof` endpoint anywhere in this tree — so an RSS-per-event figure mixes retention with read-path churn and cannot settle either one. **`memory.limit_mb` (default 2048 on a local install, #1541) is the soft cap on exactly that gap**: `debug.SetMemoryLimit`, applied once at boot and refused on reload. Soft by the runtime's contract — near it the GC trades CPU for memory, and a live heap that genuinely exceeds it still grows rather than failing anything; the failure mode of too low a limit is slowness, never a crash. It counts Go-managed memory only, so the CPEX build's cgo allocations sit outside it. + **Retention was only half of it, and reading the store was the more expensive half.** A memory figure that keeps climbing while traffic is idle is usually not retention: `GET /v1/sessions/{id}` used to encode the whole response into a single buffer before writing a byte, so serving one snapshot cost heap proportional to the *response* — ~246MB of heap growth for a 105MB response — and Go keeps that as idle heap rather than returning it promptly. agentop requests a snapshot on every Enter into a session, so RSS ratcheted a couple hundred megabytes per keystroke: four requests took a live proxy from 731MB to 1447MB, still 1447MB two minutes later. Interning could not have helped there, and the two are not alternatives — interning makes stored events *share* one copy of a repeated message, and an encoder expands every share back into its own bytes on the way out. The response is now written one event at a time (`core/sessionapi/snapshot.go`, `BenchmarkSnapshotHeap`). When judging any memory change here, note that **RSS is not heap**: the stats server serves `/debug/pprof/*` only where `listener.bind_loopback_only` is set (a heap profile dumps process memory), so on a cluster deployment an RSS-per-event figure mixes retention with read-path churn and nothing on the box can settle either one. **`memory.limit_mb` (default 2048 on a local install, #1541) is the soft cap on exactly that gap**: `debug.SetMemoryLimit`, applied once at boot and refused on reload. Soft by the runtime's contract — near it the GC trades CPU for memory, and a live heap that genuinely exceeds it still grows rather than failing anything; the failure mode of too low a limit is slowness, never a crash. It counts Go-managed memory only, so the CPEX build's cgo allocations sit outside it. **And the client is the other end of the same problem.** Measured on a laptop, `agentop` sat at 1.64GB against the proxy's 1.03GB — larger than the process it was watching, and the only one of the two still climbing. It made both mistakes the server had just stopped making: it decoded each snapshot as one document (`json.Decoder` only bounds its buffer between *values*, and a whole snapshot is one value), and it kept every string the server's encoder had expanded back out of the store's sharing. Both are fixed in `cmd/agentop/apiclient/snapshot.go`: the response is decoded one event at a time and fed through the same `session.Interner` the store uses, exported for the purpose rather than reimplemented. On a 23.5MB document, `+32.0MB HeapSys` and 26.49MB retained became +0.0MB and 2.31MB (`BenchmarkDecodeSessionViewHeap`). When a memory figure here looks wrong, measure **both** processes — the proxy has not been the larger one for a while. **The stream was the client's remaining leak (#1542):** `handleStreamEvent` appended every `/v1/events` arrival to `m.events[sessionID]` for EVERY session — never interned, never released — so an idle agentop grew at the wire rate (~18 MB/min; 1.5GB after 13h). Streamed events are now retained only for the session being watched (`watchingSession`: `selectedSess` — pane-agnostic, so pane walks don't punch a gap in the live tail — or the timeline's session); everything else updates its summary and is dropped, and opening a session fetches a fresh interned snapshot. The pipeline pane's per-plugin counts are folded into `model.pluginRuns` at stream arrival for every session — watched or not — so they still describe all traffic since attach (a snapshot's history contributes nothing, and a pod switch resets them). diff --git a/cmd/cortex/main.go b/cmd/cortex/main.go index 2e613ada4..70aff4234 100644 --- a/cmd/cortex/main.go +++ b/cmd/cortex/main.go @@ -1143,6 +1143,9 @@ func main() { if bridge != nil { statOpts = append(statOpts, observe.WithTLSBridgeUnread(bridge.UnreadHandler())) } + // A heap profile dumps process memory — prompts and tokens included — so it + // rides the same loopback-only gate as the session API's clear. + statOpts = append(statOpts, observe.WithPprof(cfg.Listener.BindLoopbackOnly)) statSrv := bootstrap.ServeStatServer(rld.ConfigProvider(), statsProvider, rld.Handler(), pricingRegistry.Handler(), listeners.take("stats"), statOpts...) diff --git a/core/observe/statserver.go b/core/observe/statserver.go index 985a75d75..5c1f9c8f9 100644 --- a/core/observe/statserver.go +++ b/core/observe/statserver.go @@ -7,6 +7,8 @@ import ( "log/slog" "net" "net/http" + "net/http/pprof" + "strings" "time" "github.com/rossoctl/cortex/core/auth" @@ -38,6 +40,7 @@ type statServerOpts struct { reloadStatus http.Handler pricingTable http.Handler tlsBridgeUnread http.Handler + pprof bool } // WithReloadStatus registers a /reload/status handler (typically the @@ -64,6 +67,15 @@ func WithTLSBridgeUnread(h http.Handler) Option { return func(o *statServerOpts) { o.tlsBridgeUnread = h } } +// WithPprof mounts Go's net/http/pprof handlers under /debug/pprof/. Off unless +// asked for: a heap profile is a dump of process memory — request bodies and +// tokens included — so the host enables it only where the stats port is +// loopback-only (cmd/cortex reads listener.bind_loopback_only, the same gate +// as the session API's clear). +func WithPprof(enabled bool) Option { + return func(o *statServerOpts) { o.pprof = enabled } +} + // NewStatServer builds the stat HTTP server. configProvider is // invoked per /config request and statsProvider per /stats request, // so both reflect current state rather than a snapshot captured at @@ -87,22 +99,24 @@ func NewStatServer(addr string, configProvider ConfigProvider, statsProvider Sta if o.tlsBridgeUnread != nil { mux.Handle("/tls-bridge/unread", o.tlsBridgeUnread) } + if o.pprof { + mux.HandleFunc("/debug/pprof/", pprof.Index) + mux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline) + mux.HandleFunc("/debug/pprof/profile", pprof.Profile) + mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol) + mux.HandleFunc("/debug/pprof/trace", pprof.Trace) + } + indexPage := statServerIndex + if o.pprof { + indexPage = strings.Replace(statServerIndex, "{{pprof}}", + `
  • Go pprof profiles
    • `, 1) + } else { + indexPage = strings.Replace(statServerIndex, "{{pprof}}", "", 1) + } mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "text/html; charset=utf-8") - fmt.Fprint(w, ` - - - - - -`) + fmt.Fprint(w, indexPage) }) return &StatServer{ @@ -165,3 +179,18 @@ func (s *StatServer) Shutdown(ctx context.Context) error { func (s *StatServer) Close() error { return s.server.Close() } + +// statServerIndex is the / body, with {{pprof}} a slot WithPprof fills. +const statServerIndex = ` + + + + +` diff --git a/core/observe/statserver_test.go b/core/observe/statserver_test.go index 06087318c..2cf99f391 100644 --- a/core/observe/statserver_test.go +++ b/core/observe/statserver_test.go @@ -196,3 +196,41 @@ func TestStatServer_TLSBridgeUnread(t *testing.T) { t.Error("a server built without the option served the report") } } + +func serveMuxOpts(cfg *config.Config, stats *auth.Stats, opts ...Option) http.Handler { + s := NewStatServer(":0", func() *config.Config { return cfg }, func() *auth.Stats { return stats }, opts...) + return s.server.Handler +} + +// WithPprof mounts Go's net/http/pprof handlers. A heap profile is a dump of +// process memory — request bodies and tokens included — so it must be possible +// to build the server without them, which is the default. +func TestStatServerPprofOffByDefault(t *testing.T) { + handler := serveMux(newTestConfig(), auth.NewStats()) + req := httptest.NewRequest(http.MethodGet, "/debug/pprof/heap", nil) + w := httptest.NewRecorder() + + handler.ServeHTTP(w, req) + + resp := w.Result() + if ct := resp.Header.Get("Content-Type"); !strings.HasPrefix(ct, "text/html") { + t.Errorf("GET /debug/pprof/heap without WithPprof served %q, want the HTML index", ct) + } +} + +func TestStatServerPprofServesProfiles(t *testing.T) { + handler := serveMuxOpts(newTestConfig(), auth.NewStats(), WithPprof(true)) + req := httptest.NewRequest(http.MethodGet, "/debug/pprof/heap", nil) + w := httptest.NewRecorder() + + handler.ServeHTTP(w, req) + + resp := w.Result() + body, _ := io.ReadAll(resp.Body) + if resp.StatusCode != http.StatusOK { + t.Fatalf("GET /debug/pprof/heap status = %d, want 200", resp.StatusCode) + } + if len(body) == 0 { + t.Error("GET /debug/pprof/heap returned an empty profile") + } +} diff --git a/docs/laptop-service.md b/docs/laptop-service.md index 364adc620..0594a025e 100644 --- a/docs/laptop-service.md +++ b/docs/laptop-service.md @@ -828,6 +828,13 @@ also keeps them as a list, which `agentop doctor` prints and the stats listener curl -s http://127.0.0.1:47602/tls-bridge/unread ``` +The stats port also serves Go's pprof profiles under `/debug/pprof/` — +`curl -s http://127.0.0.1:47602/debug/pprof/heap | go tool pprof -top -inuse_space cortex -` +answers "what is the proxy holding in memory", which `ps` RSS cannot (freed pages the +kernel has not reclaimed still count as resident on macOS). A heap profile is a dump of +process memory — prompts and tokens included — so it is served only when +`listener.bind_loopback_only` is true, the local install's default. + `programs` has one entry per program Cortex is not reading: `program` and `agent` (the executables), `reason`, `connections` (how many of the program's connections Cortex has passed through unread since Cortex started), `failures` and `stopped` (on `program-refused`), From b1b8b1a2ea1161bfcefef01a1d6c6083a74b41f6 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 30 Oct 2026 09:35:55 -0400 Subject: [PATCH 2/3] fix: Refuse non-loopback Hosts on the pprof endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A page can rebind a DNS name to 127.0.0.1 and have the victim's browser read a loopback port, and a heap profile is process memory. The five /debug/pprof handlers now answer 403 unless the Host is exactly localhost, 127.0.0.1 or ::1 — the same defense, and the same predicate, as the session API's erase guards, copied rather than shared because observe must not import sessionapi. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- core/observe/statserver.go | 37 ++++++++++++++++++++++++++++----- core/observe/statserver_test.go | 31 +++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/core/observe/statserver.go b/core/observe/statserver.go index 5c1f9c8f9..670b917aa 100644 --- a/core/observe/statserver.go +++ b/core/observe/statserver.go @@ -100,11 +100,20 @@ func NewStatServer(addr string, configProvider ConfigProvider, statsProvider Sta mux.Handle("/tls-bridge/unread", o.tlsBridgeUnread) } if o.pprof { - mux.HandleFunc("/debug/pprof/", pprof.Index) - mux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline) - mux.HandleFunc("/debug/pprof/profile", pprof.Profile) - mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol) - mux.HandleFunc("/debug/pprof/trace", pprof.Trace) + pprofGuard := func(h http.HandlerFunc) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + if !loopbackHost(r.Host) { + http.Error(w, "pprof is served only for a loopback Host", http.StatusForbidden) + return + } + h(w, r) + } + } + mux.HandleFunc("/debug/pprof/", pprofGuard(pprof.Index)) + mux.HandleFunc("/debug/pprof/cmdline", pprofGuard(pprof.Cmdline)) + mux.HandleFunc("/debug/pprof/profile", pprofGuard(pprof.Profile)) + mux.HandleFunc("/debug/pprof/symbol", pprofGuard(pprof.Symbol)) + mux.HandleFunc("/debug/pprof/trace", pprofGuard(pprof.Trace)) } indexPage := statServerIndex @@ -194,3 +203,21 @@ const statServerIndex = ` ` + +// loopbackHost reports whether hostport names exactly localhost, 127.0.0.1 or +// ::1, with or without a port. A twin of sessionapi's unexported loopbackHost +// (the erase guards): copied rather than shared because observe must not import +// the session API. A short allowlist, not 127.0.0.0/8 or a localhost suffix — +// the point is to refuse a name an attacker controls. +func loopbackHost(hostport string) bool { + host := hostport + if h, _, err := net.SplitHostPort(hostport); err == nil { + host = h + } + host = strings.TrimSuffix(strings.TrimPrefix(host, "["), "]") + switch strings.ToLower(host) { + case "localhost", "127.0.0.1", "::1": + return true + } + return false +} diff --git a/core/observe/statserver_test.go b/core/observe/statserver_test.go index 2cf99f391..a939056b1 100644 --- a/core/observe/statserver_test.go +++ b/core/observe/statserver_test.go @@ -221,6 +221,7 @@ func TestStatServerPprofOffByDefault(t *testing.T) { func TestStatServerPprofServesProfiles(t *testing.T) { handler := serveMuxOpts(newTestConfig(), auth.NewStats(), WithPprof(true)) req := httptest.NewRequest(http.MethodGet, "/debug/pprof/heap", nil) + req.Host = "localhost:47802" // a loopback Host, as a local client sends w := httptest.NewRecorder() handler.ServeHTTP(w, req) @@ -234,3 +235,33 @@ func TestStatServerPprofServesProfiles(t *testing.T) { t.Error("GET /debug/pprof/heap returned an empty profile") } } + +// A page can rebind a DNS name to 127.0.0.1 and have a browser read a loopback +// port, so the profiles refuse any Host that is not exactly loopback — the same +// defense as the session API's erase guards. +func TestStatServerPprofRefusesReboundHost(t *testing.T) { + handler := serveMuxOpts(newTestConfig(), auth.NewStats(), WithPprof(true)) + for _, host := range []string{"evil.example:47802", "127.0.0.1.evil.example", "[::1].evil.example"} { + req := httptest.NewRequest(http.MethodGet, "/debug/pprof/heap", nil) + req.Host = host + w := httptest.NewRecorder() + + handler.ServeHTTP(w, req) + + if w.Result().StatusCode != http.StatusForbidden { + t.Errorf("GET /debug/pprof/heap with Host %q = %d, want 403", host, w.Result().StatusCode) + } + } + // The loopback names themselves still pass. + for _, host := range []string{"localhost:47802", "127.0.0.1:47802", "[::1]:47802"} { + req := httptest.NewRequest(http.MethodGet, "/debug/pprof/heap", nil) + req.Host = host + w := httptest.NewRecorder() + + handler.ServeHTTP(w, req) + + if w.Result().StatusCode != http.StatusOK { + t.Errorf("GET /debug/pprof/heap with Host %q = %d, want 200", host, w.Result().StatusCode) + } + } +} From 3880b7a1258f0e17b471cb654283ea4e574d75ff Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 30 Oct 2026 09:39:31 -0400 Subject: [PATCH 3/3] fix: Finish the pprof browser defenses and correct the sensitivity claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round on #1603: the Host check alone does not stop a browser, because a simple no-cors GET carries a loopback Host and needs no preflight — browsers always send Origin, so its presence is now a refusal, the erase guards' third check. And the profiles expose less than the first round claimed: a heap profile is sampled allocation sites, not object contents, so the rationale now says what they do expose (stack traces, argv, caller-started CPU/trace profiling). Also fixes a laptop-service command that piped a profile into go tool pprof, which cannot read stdin, and pins the served body as a gzip stream rather than merely non-empty. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- CLAUDE.md | 2 +- cmd/cortex/main.go | 5 +++-- core/observe/statserver.go | 19 +++++++++++++++---- core/observe/statserver_test.go | 32 +++++++++++++++++++++++++++----- docs/laptop-service.md | 20 ++++++++++++++------ 5 files changed, 60 insertions(+), 18 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index efd2e317e..7a76242df 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1209,7 +1209,7 @@ resulting `/shared/client-id.txt` and `/shared/client-secret.txt`. The schemas were the last big duplicate and needed a type change to reach. `InferenceTool.Parameters` was `map[string]any`, which cost **4.1x its JSON text** to hold and could not be interned without a recursive walk that rewrites map values — a walk cannot lean on string immutability the way sharing a string can. Measured on a live session: 84KB per event, ~172MB across one 2050-event session. So the field became `pipeline.RawJSON`, a named string type that keeps the schema exactly as the client sent it, and it interns like any other string. Two consequences worth knowing: the API now shows schemas in the client's own key order (a map round-trip silently sorted them), and the type must stay a NAMED string with a `MarshalJSON` method — OPA's `ast.InterfaceToValue` treats a plain or aliased string as a JSON string, which would leave every policy indexing `input.inference.tools[_].parameters` undefined with no error (`core/plugins/opa/tool_parameters_rego_test.go` guards it). MCP `Params`/`Result` still do not intern, being `map[string]any`; they are unmeasured on this workload and the same field-type change is available if that changes. - **Retention was only half of it, and reading the store was the more expensive half.** A memory figure that keeps climbing while traffic is idle is usually not retention: `GET /v1/sessions/{id}` used to encode the whole response into a single buffer before writing a byte, so serving one snapshot cost heap proportional to the *response* — ~246MB of heap growth for a 105MB response — and Go keeps that as idle heap rather than returning it promptly. agentop requests a snapshot on every Enter into a session, so RSS ratcheted a couple hundred megabytes per keystroke: four requests took a live proxy from 731MB to 1447MB, still 1447MB two minutes later. Interning could not have helped there, and the two are not alternatives — interning makes stored events *share* one copy of a repeated message, and an encoder expands every share back into its own bytes on the way out. The response is now written one event at a time (`core/sessionapi/snapshot.go`, `BenchmarkSnapshotHeap`). When judging any memory change here, note that **RSS is not heap**: the stats server serves `/debug/pprof/*` only where `listener.bind_loopback_only` is set (a heap profile dumps process memory), so on a cluster deployment an RSS-per-event figure mixes retention with read-path churn and nothing on the box can settle either one. **`memory.limit_mb` (default 2048 on a local install, #1541) is the soft cap on exactly that gap**: `debug.SetMemoryLimit`, applied once at boot and refused on reload. Soft by the runtime's contract — near it the GC trades CPU for memory, and a live heap that genuinely exceeds it still grows rather than failing anything; the failure mode of too low a limit is slowness, never a crash. It counts Go-managed memory only, so the CPEX build's cgo allocations sit outside it. + **Retention was only half of it, and reading the store was the more expensive half.** A memory figure that keeps climbing while traffic is idle is usually not retention: `GET /v1/sessions/{id}` used to encode the whole response into a single buffer before writing a byte, so serving one snapshot cost heap proportional to the *response* — ~246MB of heap growth for a 105MB response — and Go keeps that as idle heap rather than returning it promptly. agentop requests a snapshot on every Enter into a session, so RSS ratcheted a couple hundred megabytes per keystroke: four requests took a live proxy from 731MB to 1447MB, still 1447MB two minutes later. Interning could not have helped there, and the two are not alternatives — interning makes stored events *share* one copy of a repeated message, and an encoder expands every share back into its own bytes on the way out. The response is now written one event at a time (`core/sessionapi/snapshot.go`, `BenchmarkSnapshotHeap`). When judging any memory change here, note that **RSS is not heap**: the stats server serves `/debug/pprof/*` only where `listener.bind_loopback_only` is set (the profiles expose stack traces and argv and let a caller start CPU/trace profiling), so on a cluster deployment an RSS-per-event figure mixes retention with read-path churn and nothing on the box can settle either one. **`memory.limit_mb` (default 2048 on a local install, #1541) is the soft cap on exactly that gap**: `debug.SetMemoryLimit`, applied once at boot and refused on reload. Soft by the runtime's contract — near it the GC trades CPU for memory, and a live heap that genuinely exceeds it still grows rather than failing anything; the failure mode of too low a limit is slowness, never a crash. It counts Go-managed memory only, so the CPEX build's cgo allocations sit outside it. **And the client is the other end of the same problem.** Measured on a laptop, `agentop` sat at 1.64GB against the proxy's 1.03GB — larger than the process it was watching, and the only one of the two still climbing. It made both mistakes the server had just stopped making: it decoded each snapshot as one document (`json.Decoder` only bounds its buffer between *values*, and a whole snapshot is one value), and it kept every string the server's encoder had expanded back out of the store's sharing. Both are fixed in `cmd/agentop/apiclient/snapshot.go`: the response is decoded one event at a time and fed through the same `session.Interner` the store uses, exported for the purpose rather than reimplemented. On a 23.5MB document, `+32.0MB HeapSys` and 26.49MB retained became +0.0MB and 2.31MB (`BenchmarkDecodeSessionViewHeap`). When a memory figure here looks wrong, measure **both** processes — the proxy has not been the larger one for a while. **The stream was the client's remaining leak (#1542):** `handleStreamEvent` appended every `/v1/events` arrival to `m.events[sessionID]` for EVERY session — never interned, never released — so an idle agentop grew at the wire rate (~18 MB/min; 1.5GB after 13h). Streamed events are now retained only for the session being watched (`watchingSession`: `selectedSess` — pane-agnostic, so pane walks don't punch a gap in the live tail — or the timeline's session); everything else updates its summary and is dropped, and opening a session fetches a fresh interned snapshot. The pipeline pane's per-plugin counts are folded into `model.pluginRuns` at stream arrival for every session — watched or not — so they still describe all traffic since attach (a snapshot's history contributes nothing, and a pod switch resets them). diff --git a/cmd/cortex/main.go b/cmd/cortex/main.go index 70aff4234..7ba024d89 100644 --- a/cmd/cortex/main.go +++ b/cmd/cortex/main.go @@ -1143,8 +1143,9 @@ func main() { if bridge != nil { statOpts = append(statOpts, observe.WithTLSBridgeUnread(bridge.UnreadHandler())) } - // A heap profile dumps process memory — prompts and tokens included — so it - // rides the same loopback-only gate as the session API's clear. + // pprof exposes the process's internals (allocation stack traces, argv, + // caller-started CPU/trace profiling), so it rides the same loopback-only + // gate as the session API's clear. statOpts = append(statOpts, observe.WithPprof(cfg.Listener.BindLoopbackOnly)) statSrv := bootstrap.ServeStatServer(rld.ConfigProvider(), statsProvider, rld.Handler(), pricingRegistry.Handler(), listeners.take("stats"), statOpts...) diff --git a/core/observe/statserver.go b/core/observe/statserver.go index 670b917aa..5962d959a 100644 --- a/core/observe/statserver.go +++ b/core/observe/statserver.go @@ -68,10 +68,17 @@ func WithTLSBridgeUnread(h http.Handler) Option { } // WithPprof mounts Go's net/http/pprof handlers under /debug/pprof/. Off unless -// asked for: a heap profile is a dump of process memory — request bodies and -// tokens included — so the host enables it only where the stats port is -// loopback-only (cmd/cortex reads listener.bind_loopback_only, the same gate -// as the session API's clear). +// asked for: the profiles expose the process's internals — allocation stack +// traces, argv (cmdline), and a caller's ability to start CPU profiling and +// tracing — so the host enables it only where the stats port is loopback-only +// (cmd/cortex reads listener.bind_loopback_only, the same gate as the session +// API's clear). The handlers also refuse a non-loopback Host and any request +// carrying Origin, the erase guards' browser defenses. +// +// Side effect of the import: net/http/pprof's init registers /debug/pprof/* on +// http.DefaultServeMux in every binary linking this package. Nothing in this +// tree serves that mux — an http.Server built with a nil Handler would, and +// would expose these ungated, so none may be. func WithPprof(enabled bool) Option { return func(o *statServerOpts) { o.pprof = enabled } } @@ -106,6 +113,10 @@ func NewStatServer(addr string, configProvider ConfigProvider, statsProvider Sta http.Error(w, "pprof is served only for a loopback Host", http.StatusForbidden) return } + if r.Header.Get("Origin") != "" { + http.Error(w, "pprof is refused from a browser: the request carries an Origin header", http.StatusForbidden) + return + } h(w, r) } } diff --git a/core/observe/statserver_test.go b/core/observe/statserver_test.go index a939056b1..a83115af7 100644 --- a/core/observe/statserver_test.go +++ b/core/observe/statserver_test.go @@ -202,9 +202,9 @@ func serveMuxOpts(cfg *config.Config, stats *auth.Stats, opts ...Option) http.Ha return s.server.Handler } -// WithPprof mounts Go's net/http/pprof handlers. A heap profile is a dump of -// process memory — request bodies and tokens included — so it must be possible -// to build the server without them, which is the default. +// WithPprof mounts Go's net/http/pprof handlers. The profiles expose the +// process's internals, so it must be possible to build the server without +// them, which is the default. func TestStatServerPprofOffByDefault(t *testing.T) { handler := serveMux(newTestConfig(), auth.NewStats()) req := httptest.NewRequest(http.MethodGet, "/debug/pprof/heap", nil) @@ -213,9 +213,13 @@ func TestStatServerPprofOffByDefault(t *testing.T) { handler.ServeHTTP(w, req) resp := w.Result() + body, _ := io.ReadAll(resp.Body) if ct := resp.Header.Get("Content-Type"); !strings.HasPrefix(ct, "text/html") { t.Errorf("GET /debug/pprof/heap without WithPprof served %q, want the HTML index", ct) } + if strings.Contains(string(body), "/debug/pprof/") { + t.Error("index page links /debug/pprof/ without WithPprof") + } } func TestStatServerPprofServesProfiles(t *testing.T) { @@ -231,8 +235,9 @@ func TestStatServerPprofServesProfiles(t *testing.T) { if resp.StatusCode != http.StatusOK { t.Fatalf("GET /debug/pprof/heap status = %d, want 200", resp.StatusCode) } - if len(body) == 0 { - t.Error("GET /debug/pprof/heap returned an empty profile") + // A gzip stream is what pprof serves; the HTML index or an error page is not. + if len(body) < 2 || body[0] != 0x1f || body[1] != 0x8b { + t.Error("GET /debug/pprof/heap did not return a gzip profile stream") } } @@ -265,3 +270,20 @@ func TestStatServerPprofRefusesReboundHost(t *testing.T) { } } } + +// The Host check alone does not stop a browser: a simple no-cors GET from a +// page carries a loopback Host and no preflight. Browsers always send Origin, +// so its presence is the refusal — the erase guards' third check. +func TestStatServerPprofRefusesBrowserOrigin(t *testing.T) { + handler := serveMuxOpts(newTestConfig(), auth.NewStats(), WithPprof(true)) + req := httptest.NewRequest(http.MethodGet, "/debug/pprof/heap", nil) + req.Host = "127.0.0.1:47802" + req.Header.Set("Origin", "https://evil.example") + w := httptest.NewRecorder() + + handler.ServeHTTP(w, req) + + if w.Result().StatusCode != http.StatusForbidden { + t.Errorf("GET /debug/pprof/heap with Origin = %d, want 403", w.Result().StatusCode) + } +} diff --git a/docs/laptop-service.md b/docs/laptop-service.md index 0594a025e..b58020d72 100644 --- a/docs/laptop-service.md +++ b/docs/laptop-service.md @@ -828,12 +828,20 @@ also keeps them as a list, which `agentop doctor` prints and the stats listener curl -s http://127.0.0.1:47602/tls-bridge/unread ``` -The stats port also serves Go's pprof profiles under `/debug/pprof/` — -`curl -s http://127.0.0.1:47602/debug/pprof/heap | go tool pprof -top -inuse_space cortex -` -answers "what is the proxy holding in memory", which `ps` RSS cannot (freed pages the -kernel has not reclaimed still count as resident on macOS). A heap profile is a dump of -process memory — prompts and tokens included — so it is served only when -`listener.bind_loopback_only` is true, the local install's default. +The stats port also serves Go's pprof profiles under `/debug/pprof/` — this answers +"what is the proxy holding in memory", which `ps` RSS cannot (freed pages the kernel +has not reclaimed still count as resident on macOS): + +```sh +curl -s --noproxy '*' http://127.0.0.1:47602/debug/pprof/heap -o /tmp/heap.pb.gz +go tool pprof -top -inuse_space /tmp/heap.pb.gz +``` + +`go tool pprof` cannot read the profile from a pipe, so the two-step form it is. The +`--noproxy '*'` matters on this machine: the shell's proxy env points at cortex itself. +The profiles expose stack traces and argv rather than object contents, but are served +only when `listener.bind_loopback_only` is true (the local install's default), and the +endpoints refuse a non-loopback Host or any browser Origin. `programs` has one entry per program Cortex is not reading: `program` and `agent` (the executables), `reason`, `connections` (how many of the program's connections Cortex has