GET /v0/servers 500 path leaks internal error detail to client (follow-up to #1335)

[rdimitrov]

Follow-up to #1335 (merged as-is per maintainer decision, with this issue tracking the known debt).

1. 🔴 GET /v0/servers 500 path leaks internal error detail to the client (primary)

#1335 refactored the list error handling into ListServersError (internal/api/handlers/v0/list_errors.go) and, in the process, changed the genuine-failure branch from:

return nil, huma.Error500InternalServerError("Failed to get registry list")

to:

return huma.Error500InternalServerError("Failed to get registry list", err)   // err is new

That extra err is not server-side only. Verified against huma v2.38.0:

• Error500InternalServerError(msg, errs...) → NewError (error.go:231) stores each err as &ErrorDetail{Message: err.Error()} in ErrorModel.Errors.
• ErrorModel.Errors is JSON-serialized: `json:"errors,omitempty"` (error.go:91).
• No custom huma.NewError override or response transformer exists in internal/, so the default serialization applies.

So on a real DB failure, the response body on the public, unauthenticated GET /v0/servers endpoint now contains the wrapped error from internal/database/postgres.go:262 (error iterating rows: <pgx error>). pgx/pgconn errors can carry SQLSTATE codes, constraint/table/column names, and internal messages → verbose-error information disclosure (CWE-209).

This also diverges from the rest of the codebase, which logs err server-side and returns only a generic message: servers.go:190, servers.go:220, edit.go:79, status.go:127/234/250.

Fix: drop err from the 500 call, keep the server-side log.

log.Printf("list servers failed: %v", err)
return huma.Error500InternalServerError("Failed to get registry list")

Test gap: list_errors_test.go asserts status codes only. Add an assertion that the 500 response body does not contain the raw error string.

2. 🟡 PR description claim: "Prevents superfluous response.WriteHeader warnings"

This part of #1335 isn't actually achieved. The warning originates inside huma's transformAndWrite (huma.go): ctx.SetStatus(status) at ~line 619 (first WriteHeader), then if the body-marshal write fails and ctx.Err() == context.Canceled, ctx.SetStatus(499) at ~line 627 (second WriteHeader) → the warning. Returning huma.NewError(499, …) flows through the same WriteErr → writeResponse → transformAndWrite path, so the double-SetStatus is unchanged — it's status-code-agnostic.

The 499 status + dropped error-level log from #1335 are real improvements; the WriteHeader suppression is not. Either accept the warning as a benign huma-internal artifact, or pursue an early return that doesn't invoke the error-response writer (as the original issue #1323 suggested).

3. 🟢 Nits

• Reuse statusClientClosed (internal/api/router/router.go:28) instead of the literal 499 in list_errors.go.
• Cancellation is now classified in two places — the handler (list_errors.go) and MetricTelemetryMiddleware (router.go:90). Keep them in sync.
• The 499 branch also passes err into the serialized body — harmless (client has disconnected) but inconsistent; could omit.
• The guard only covers the list handler; the sibling handlers and the second error iterating rows site (postgres.go:479) keep the old behavior. Fine for now since those don't leak err, but the cancellation-logging cleanup is partial vs. what /v0/servers logs benign client cancellations as "list servers failed" and triggers "superfluous response.WriteHeader" warnings #1323 suggested.

---

Generated from a code review of #1335.