fix(security): gate pprof behind ARC_DEBUG_PPROF on a localhost listener + anchor + normalise PublicPrefixes match

[xe-nvdk]

Summary

Closes audit finding #2 from the 2026-05-19 external report (@NeuroWinter): pre-26.06.1 Arc mounted /debug/pprof/* unconditionally on the public Fiber app AND added /debug/pprof to the auth middleware's PublicPrefixes list. Any network-reachable caller — no token, no auth — could fetch heap dumps (leaking in-flight SQL strings, msgpack records, cached *TokenInfo), leak goroutine stacks, or pin a CPU core via /debug/pprof/profile?seconds=N.

This PR removes pprof from the public Fiber app entirely. The endpoints are now opt-in via ARC_DEBUG_PPROF=1 on a separate listener bound to 127.0.0.1:6060 by default. Non-loopback binds require a second explicit opt-in (ARC_DEBUG_PPROF_ALLOW_NON_LOOPBACK=1) so a single typo in the address knob cannot expose pprof to the network.

Tied to GHSA-j93g-rp6m-j32m (private draft).

What's in this PR

pprof hardening:

• internal/api/server.go: drop the unconditional app.Use(pprof.New()).
• cmd/arc/main.go: drop /debug/pprof from PublicPrefixes, wire startDebugPprofIfEnabled after the shutdown coordinator.
• cmd/arc/debug_pprof.go (NEW): env-gated localhost listener on a private *http.ServeMux. Force-closes via srv.Close() (not srv.Shutdown(ctx)) so a long /debug/pprof/profile?seconds=N capture cannot starve the coordinator's shared 30s shutdown budget. Server-side timeouts (ReadHeaderTimeout=5s, WriteTimeout=10m, IdleTimeout=60s) bound slow-client attacks.

Auth middleware defence-in-depth (same shape as the deniedRoots gap gemini caught on PR #442):

• Anchored prefix match: path == prefix || strings.HasPrefix(path, prefix + \"/\"). Sibling paths like /metricsX, /metrics-secret no longer slip through.
• path.Clean normalisation before match: non-canonical shapes (/metrics//foo, /metrics/./x, /metrics/../sensitive) are normalised before the bypass branch checks them.
• Empty-prefix guard: skips empty PublicPrefixes entries so a future config bug can't open the whole API.

With /debug/pprof removed from the public prefix list, the middleware changes are not currently reachable for any production route — they're guards for any prefix added in the future.

Operator-facing changes

• Default behavior changed: curl http://arc:8000/debug/pprof/heap now returns 404. Existing deployments that relied on this for production debugging must set ARC_DEBUG_PPROF=1 and reach pprof on 127.0.0.1:6060.
• /metrics unchanged: Prometheus scrapers continue to work as before.
• Two env vars for the non-loopback case: ARC_DEBUG_PPROF_ADDR=0.0.0.0:6060 alone refuses to start; pair with ARC_DEBUG_PPROF_ALLOW_NON_LOOPBACK=1 for the explicit opt-in.

Test plan

• go build ./... clean.
• go test ./internal/auth/... ./internal/api/... ./cmd/arc/... — all green.
• go test -count=1 -short ./... -timeout 300s — all packages green (pre-existing benchmarks/fts_experiment vet failure unaffected).
• TestServer_PprofNotRegisteredOnPublicApp exercises the 12 pprof paths the gofiber middleware used to register and asserts each returns 404.
• TestMiddleware_PublicPrefixes_AnchoredMatch covers 10 subtests: exact / trailing-slash / true subdir / deep subdir bypass + sibling-byte-prefix shapes (/metricsX, /metrics-secret, /metricsadmin) + parent-traversal escape shapes (/metrics/../sensitive, /metrics/sub/../../etc) + empty-prefix guard.
• TestStartDebugPprofIfEnabled_NoopWhenDisabled — no listener when env unset.
• TestStartDebugPprofIfEnabled_BindsAndServes — env-gated listener binds, serves pprof, 404s non-pprof paths (proving private mux not DefaultServeMux), force-closes cleanly.
• TestIsTruthy + TestIsLoopbackBindAddr — env-var contract + loopback detection (incl. fail-closed on unresolvable hosts).

Internal review

Three rounds of multi-agent review per .claude/CLAUDE.md caught:

• 2 HIGH: shutdown DoS chain via long pprof captures starving downstream hooks; un-normalised c.Path() admitting //, /./, /../ shapes into the bypass branch.
• 6 MED: factually wrong DefaultServeMux comment, duplicate component logger field, listener-vs-Serve race window, missing WriteTimeout/IdleTimeout, non-loopback bind needing a second opt-in, empty-string PublicPrefixes guard.
• 5 style/nits: builtin min vs reinvented minInt, duplicate /metrics append, dead srv.Addr field, stale 30-second http-server comment, unnecessary handler registrations in test.

All 13 addressed before opening this PR (single squashed commit on the branch).

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request enhances security by moving Go pprof endpoints to an opt-in localhost listener and implementing path normalization and anchored prefix matching in the authentication middleware. Reviewer feedback identifies discrepancies in the release notes regarding log levels and test counts, and suggests simplifying the pprof listener by removing redundant handler registrations.

[gemini-code-assist]

The release note states that Arc logs a Warn for non-loopback pprof binds. However, the implementation in cmd/arc/debug_pprof.go upgrades the log level to Error for non-loopback addresses to increase visibility. Please update the release note to reflect that an Error is logged in this case for accuracy.

[gemini-code-assist]

The description of TestMiddleware_PublicPrefixes_AnchoredMatch mentions that it covers 7 paths. The test implementation is more comprehensive and includes 10 sub-tests, covering important cases like path traversal (/metrics/../sensitive) and the empty-prefix guard. To accurately reflect the test's thoroughness, consider updating this description.

[gemini-code-assist]

The pprof.Index handler, registered on line 100 for the /debug/pprof/ path, is designed to handle all standard pprof sub-paths, including cmdline, profile, symbol, and trace. These explicit HandleFunc registrations are therefore redundant and can be removed to simplify the code.

[xe-nvdk]

Verified empirically that pprof.Index does NOT cover cmdline/profile/symbol/trace. Index dispatches via runtime/pprof.Lookup, which contains only the runtime profiles (heap, goroutine, allocs, mutex, block, threadcreate). The four special endpoints are separate functions in net/http/pprof and require their own registrations.

Test with just pprof.Index registered (Gemini's suggestion):

/debug/pprof/cmdline       -> 404 body: Unknown profile
/debug/pprof/profile?...   -> 404 body: Unknown profile
/debug/pprof/symbol        -> 404 body: Unknown profile
/debug/pprof/trace?...     -> 404 body: Unknown profile
/debug/pprof/heap          -> 200
/debug/pprof/goroutine     -> 200

Source: net/http/pprof/pprof.go — Index extracts name after /debug/pprof/ and calls handler(name).ServeHTTP, which calls pprof.Lookup(name) and 404s on the four non-runtime-profile names.

Keeping the explicit registrations. Strengthened TestStartDebugPprofIfEnabled_BindsAndServes in commit fd781a0 to hit /debug/pprof/cmdline and /debug/pprof/symbol so a future contributor accepting the same suggestion would catch the regression in CI.

[xe-nvdk]

@gemini-code-assist please review

[gemini-code-assist]

Code Review

This pull request enhances security by removing pprof endpoints from the public API and relocating them to an opt-in, localhost-bound listener. It also strengthens the authentication middleware by introducing path normalization and anchored prefix matching to mitigate risks from path traversal and prefix-based auth bypasses. Feedback was provided regarding a logic flaw in the anchored matching implementation where prefixes with trailing slashes would fail to match subdirectories, along with a suggested fix to handle these cases correctly.

[gemini-code-assist]

The current logic for anchored prefix matching doesn't correctly handle public prefixes that have a trailing slash (e.g., /metrics/). This could lead to unexpected behavior where subpaths are not considered public.

For example, if PublicPrefixes contains /metrics/, a request to /metrics/prometheus would not be matched as a public path because the check becomes strings.HasPrefix("/metrics/prometheus", "/metrics//"), which is false.

To make the logic more robust and handle this case, I suggest differentiating between prefixes with and without a trailing slash.

			// Anchor the match: require exact path or true subdirectory.
			// This logic handles prefixes with and without trailing slashes
			// to prevent unintended auth bypasses for sibling paths.
			if strings.HasSuffix(prefix, "/") {
				if strings.HasPrefix(reqPath, prefix) {
					return c.Next()
				}
			} else {
				if reqPath == prefix || strings.HasPrefix(reqPath, prefix+"/") {
					return c.Next()
				}
			}

[xe-nvdk]

@gemini-code-assist please review

[gemini-code-assist]

Code Review

This pull request significantly improves the security posture of the application by removing Go pprof endpoints from the public API port and replacing them with an opt-in, separate listener that defaults to localhost. Additionally, it implements a defense-in-depth fix for the authentication middleware's public prefix matching logic, which now utilizes anchored matching and path normalization to prevent bypasses via path traversal or sibling-prefix attacks. Comprehensive regression tests and documentation updates are included for both changes. I have no feedback to provide as there were no review comments.