[huang195]

What

Mounts Go's net/http/pprof under /debug/pprof/* on the stats server (:9093 in-cluster, :47602 on a laptop install), enabled only when listener.bind_loopback_only is set.

Why

Nothing in the tree could answer "what is the proxy holding in memory". Measured on the laptop today (2026-10-30): cortex showed 3.4 GB RSS, but ~1.5 GB of that was pages the GC had already freed — macOS counts MADV_FREE pages as resident until the kernel is pressured. The pressure floor was 1.9 GB, and decomposing that further was impossible with no heap endpoint. go tool pprof -inuse_space is the only tool that splits live heap from churn, so the local install now serves it:

curl -s --noproxy '*' http://127.0.0.1:47602/debug/pprof/heap -o /tmp/heap.pb.gz
go tool pprof -top -inuse_space ~/.local/bin/cortex /tmp/heap.pb.gz

The gate

A heap profile is a dump of process memory — request bodies, prompts and tokens included. It rides the same listener.bind_loopback_only gate as sessionapi.WithClearAllowed (the DELETE guards), so:

• laptop install (default config): served
• cluster deployments, where the stats port is not loopback: surface unchanged, no profile data

TestStatServerPprofOffByDefault pins that a server built without the option serves no profile; TestStatServerPprofServesProfiles pins that one built with it serves a parseable heap profile. Verified end-to-end on a scratch instance both ways, including go tool pprof parsing the served profile.

Notes

• No new dependency — net/http/pprof is stdlib.
• The stats index page links the profiles only when they are served.
• CLAUDE.md's "there is no pprof endpoint anywhere in this tree" line is updated, and docs/laptop-service.md documents the laptop usage.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 21 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 4ac62e81-c8c8-4f64-9b87-06dbdad70abf

Commits

Reviewing files that changed from the base of the PR and between 20ffb36 and 3880b7a.

Files selected for processing: 5

• CLAUDE.md
• cmd/cortex/main.go
• core/observe/statserver.go
• core/observe/statserver_test.go
• docs/laptop-service.md

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[clawgenti]

Summary

Clean, well-scoped change: net/http/pprof is mounted on the stats server's own mux (not DefaultServeMux), off by default, and enabled only from cmd/cortex via listener.bind_loopback_only. I traced the gate end to end and found no path that serves pprof on a non-loopback bind today. No blocking issues. The suggestions below are defense-in-depth plus a broken doc command.

Security analysis of the gate

• ApplyPreset runs last and rewrites stats.address with forceLocalhost, so with the flag set every form ends up on 127.0.0.1:<port>: :9093, 0.0.0.0:9093, [::]:9093, [::1]:9093 and localhost:9093 all do. An empty address is defaulted to :9093 in Load before that (an empty address passed to net.Listen would mean every interface on a random port). An address SplitHostPort cannot parse comes back from forceLocalhost unchanged, but net.Listen rejects it too, so it fails closed.
• The listener plan is built from the same cfg that WithPprof reads, and the stats server is built once at boot, so a hot reload cannot flip the flag under a server that is already bound.
• Only cmd/cortex passes WithPprof. cortex-envoy and cortex-praxis import core/observe but never enable it. I found no in-repo cluster/Helm config that sets bind_loopback_only: true; the only one is demos/coding-agent-cpex (a laptop demo).
• One side effect worth knowing: importing net/http/pprof registers handlers on http.DefaultServeMux in all three binaries. I checked every http.Server and ListenAndServe in cmd/ and core/, and none serves the default mux, so nothing is exposed today (see the inline nit).

Author: huang195 (MEMBER, maintainer)
Areas reviewed: Go (core/observe, cmd/cortex, core/config gate path), tests, docs
Agent/IDE config (.claude/.vscode): none (CLAUDE.md is a doc-text edit only)
Commits: 1 commit, all signed-off: yes
CI status: passing (go test ./observe/ and go vet also pass locally)

[clawgenti]

suggestion: The PR describes this as "the same gate as the session API's clear", but clearRefusal makes three checks and pprof adopts only the last one. Clear also rejects a non-loopback Host (loopbackHost) and any request carrying an Origin header, which is the DNS-rebinding and browser defense (see the comment on logClearRefusal). Without those checks:

• A rebinding page can read /debug/pprof/* same-origin.
• Any web page can fire a simple no-cors GET at http://127.0.0.1:47602/debug/pprof/trace?seconds=86400 or /profile?seconds=.... The stats server sets no WriteTimeout, so pprof puts no cap on seconds. The tracer/profiler runs for as long as the browser keeps the connection open, and it holds the process-wide CPU-profile slot. heap?gc=1 in a loop forces repeated GCs.

The data exposed is less sensitive than the PR states (see the comment on line 71), so I'm not treating this as blocking. Still, wrapping the pprof routes in the same Host/Origin check would cost a few lines and make the "same gate" claim true. One way: move loopbackHost into a small shared helper and wrap with func(h http.HandlerFunc) http.HandlerFunc.

[clawgenti]

suggestion (defense in depth): The gate is the config flag, so it relies on an invariant that lives elsewhere: ApplyPreset has to keep running forceLocalhost over Stats.StatsAddress after every default. This tree has already been bitten by that ordering once (#1564, see the comment in presets.go). It would be more robust to also check the socket that was actually bound, e.g.

statsLn := listeners.take("stats")
pprofOK := cfg.Listener.BindLoopbackOnly && isLoopbackAddr(statsLn.Addr())
statOpts = append(statOpts, observe.WithPprof(pprofOK))

Here isLoopbackAddr is addr.(*net.TCPAddr).IP.IsLoopback(). Then a future refactor of the preset ordering, or a new way of setting the stats address, cannot put heap/goroutine/trace endpoints on a wildcard bind. A table test on forceLocalhost covering [::]:p, 0.0.0.0:p, localhost:p and [::1]:p would also pin the half that lives in core/config.

[clawgenti]

suggestion (accuracy): "A heap profile is a dump of process memory — request bodies and tokens included" isn't right for Go's pprof. A heap profile is sampled allocation sites: stack traces with object counts and byte sizes, plus mapping/build-ID metadata. It contains no object contents, so no prompts, bodies or tokens (the tree uses no pprof.Do labels that could carry values). goroutine?debug=2 prints arguments only as hex words. The more sensitive pieces are cmdline (process argv) and the code-structure and timing information.

Keeping the gate conservative is fine. But the stated rationale is repeated in main.go, CLAUDE.md (two places), docs/laptop-service.md and the test comment, and it will steer later decisions such as whether to enable this in-cluster behind port-forward. I'd reword it to something like "exposes internal code structure, argv, and lets a caller start CPU/trace profiling".

[clawgenti]

suggestion: This command fails as written. go tool pprof doesn't read a profile from stdin, so - is treated as a filename (verified with go1.26.10: -: stat -: no such file or directory / failed to fetch any source profiles). The binary argument isn't needed either, because Go heap profiles carry their own symbols. Simplest form:

go tool pprof -top -inuse_space http://127.0.0.1:47602/debug/pprof/heap

or the two-step curl -s --noproxy '*' ... -o /tmp/heap.pb.gz from the PR description. The --noproxy '*' is worth keeping on a laptop whose shell has HTTP_PROXY pointed at cortex.

[clawgenti]

nit: The init() of net/http/pprof also registers /debug/pprof/* on http.DefaultServeMux. Because it's imported here, that now happens in cortex, cortex-envoy and cortex-praxis, whatever WithPprof says. Nothing in the tree serves the default mux today (I checked every http.Server and ListenAndServe under cmd/ and core/), so this is latent. But a future http.ListenAndServe(addr, nil) or an http.Server with no Handler would serve ungated pprof on whatever it binds. (x/net/trace, pulled in through gRPC, already does the same thing for /debug/requests.) A one-line comment here noting the side effect would help, and so would a test asserting that no production server uses a nil handler.

[clawgenti]

nit: The PR description says this test pins "a parseable heap profile", but it only checks for a non-empty body. Checking for the gzip magic (body[0] == 0x1f && body[1] == 0x8b), or Content-Type: application/octet-stream, would actually separate a profile from, say, the HTML index or an error page. The off-by-default test could also assert the index page has no /debug/pprof/ link.

[huang195]

Addressed the review's should-fix: the five /debug/pprof/* handlers now answer 403 unless the Host is exactly localhost, 127.0.0.1 or ::1 (with or without port) — the same DNS-rebinding defense, and the same predicate, as the session API's erase guards (core/sessionapi/clear.go). Copied rather than shared: observe importing sessionapi would be the wrong dependency direction.

Fails closed: an absent Host (HTTP/1.0) is refused too. TestStatServerPprofRefusesReboundHost pins both directions — rebound names (evil.example, 127.0.0.1.evil.example, [::1].evil.example) get 403, loopback names get profiles. Verified on a scratch instance over the wire as well.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

[huang195]

Review round pushed as 3880b7a1 (plus b1b8b1a2, which landed after the review was written). Suggestion by suggestion:

• Host/Origin guard — the loopback-Host half landed in b1b8b1a2; this round adds the Origin refusal (TestStatServerPprofRefusesBrowserOrigin), which is the half the Host check cannot cover: a simple no-cors GET from a page carries a loopback Host and needs no preflight. Not capping ?seconds= — on a loopback port the threat is browsers, not local processes, and a local process needs no pprof to burn CPU.
• Defense in depth on the bound socket — skipped. forceLocalhost's ordering is already pinned by core/config/loopback_bind_test.go (wildcards included), and the repo's rule is that config mistakes stay config problems until they happen often.
• Sensitivity claim — fixed everywhere it was repeated (statserver, main.go, CLAUDE.md, laptop-service.md, test comment). You're right: sampled allocation sites, not object contents. The comments now say stack traces, argv, caller-started CPU/trace profiling.
• Broken doc command — fixed; the two-step curl -o + go tool pprof form, with a note on why the pipe can't work and why --noproxy matters here.
• DefaultServeMux side effect — noted in WithPprof's doc comment: nothing in the tree may serve a nil-Handler http.Server, or these are exposed ungated. Skipped the nil-handler test — one comment states the invariant, and a test that greps for future code doesn't exist yet.
• Test strength — done: the served body must be a gzip stream (magic bytes), and the off-by-default test also asserts the index carries no /debug/pprof/ link.

Full core + cmd/cortex suites, vet, gofmt all green.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com