Skip to content

feat(api): trusted-proxy-aware client IP in traces & logs (+ gate trace-context propagation) #333

Closed

Description

@EricAndrechek

Area: api · observability — edge integration · split out of #332 / #281

Context

PR #332 dropped chi's deprecated middleware.RealIP (#281) because it rewrote r.RemoteAddr from spoofable forwarded headers (X-Forwarded-For/X-Real-IP/True-Client-IP) on every request, whether or not a trusted proxy set them, and nothing in WaveHouse read r.RemoteAddr (no per-IP logic — rate limiting is the reverse proxy's job). That removed the IP-spoofing vector (GHSA-3fxj-6jh8-hvhx / GHSA-rjr7-jggh-pgcp / GHSA-9g5q-2w5x-hmxf) and unblocked the go-deps dependabot bump (#209).

The trade-off: WaveHouse now has no real client IP in its own traces/logs — behind a proxy, r.RemoteAddr (and OTel's client.address) is the proxy's IP. We want the real client IP back, captured safely, plus to formalize trace continuation from the proxy.

Scope

  1. Trusted-proxy config. Declare trusted proxies — a CIDR list (e.g. WH_TRUSTED_PROXIES=10.0.0.0/8,172.16.0.0/12) is the correct model. Default empty ⇒ trust nothing (use the immediate peer).
  2. Client-IP extraction (trusted-proxy-aware). When the immediate peer is a trusted proxy, walk X-Forwarded-For right-to-left and take the first hop that is not itself a trusted proxy — the real client. ⚠️ Do not take the leftmost / True-Client-IP value blindly: that's client-controlled and would reintroduce the exact spoofing the RealIP drop just fixed.
  3. Surface it. Attach the resolved client IP as a span attribute (client.address) and a structured-log field, alongside the existing trace_id/span_id (internal/observability/logger.go).
  4. Trace-context propagation (already works — formalize + gate). WaveHouse already sets the W3C TraceContext propagator (internal/observability/provider.go:110) and uses otelhttp without WithPublicEndpoint, so an incoming traceparent from an nginx/Caddy OTel module is already adopted as the parent — the request joins that trace and logs already carry the continued trace_id. Two gaps: (a) it's undocumented and has no test for the incoming-traceparent case; (b) it trusts any client's traceparent (trace pollution / a direct client injecting itself into your traces). Gate continuation on the same trusted-proxy boundary (e.g. otelhttp.WithPublicEndpoint() or a custom check, so an untrusted peer's incoming trace is linked, not adopted).
  5. Docs. Add a "trusted proxy / client IP" section to the reverse-proxy page, and document that a proxy-supplied traceparent is continued.

Security note

The XFF-parsing logic is a classic footgun — getting the trusted-hop walk wrong rebuilds a spoofable-IP feature. Warrants careful review + tests: spoof attempts, multiple chained proxies, IPv6, and missing/empty headers.

Related: #281 (dropped RealIP), #332 (the PR that dropped it), #241 (reverse-proxy docs), #209 (go-deps dependabot, unblocked by the drop).

Activity

  1. coderabbitai commented on Jun 10, 2026

    @coderabbitai
    🔗 Related PRs

    #123 - fix(api): drop CORS credentials + skip same-origin decoration [merged]


    📝 Issue Planner

    Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.

    • Create Plan

    🧪 Issue enrichment is currently in open beta.

    You can configure auto-planning by selecting labels in the issue_enrichment configuration.

    To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

    issue_enrichment:
      auto_enrich:
        enabled: false

    💬 Have feedback or questions? Drop into our discord!

  2. EricAndrechek commented on Jul 7, 2026

    @EricAndrechek
    MemberAuthor

    Folding in a related item from #378 (the operator-key PR): the operator-key audit log (internal/auth/auth.go) wants a request-scoped correlation ID (chi's request_id). I intentionally did not stamp it per-call-site there — it belongs here, in the global TraceHandler (internal/observability/logger.go), alongside the trusted-proxy client IP this issue already covers.

    Concretely, to add here:

    • TraceHandler.Handle should also pull request_id from context (chi's middleware.GetReqID, always populated via the middleware.RequestID at router.go) and stamp it on every record, next to trace_id/span_id.
    • The handler should wrap the base handler in all modes, not just when OTel logs are enabled. Today it's only installed in the OTel-logs path (main.go); the plain slog.JSONHandler fallback gets no context-derived fields, so a non-OTel deployment currently has no correlation ID at all.

    Net once this lands: every log line — including the operator-key audit line — uniformly carries request_id (OTel-independent) and the trusted-proxy client IP, wired once instead of per call site.

    — Claude Code

  3. EricAndrechek commented on Oct 8, 2026

    @EricAndrechek
    MemberAuthor

    The request-id item folded in from #378 (stamp chi's request_id from context in TraceHandler, next to trace_id/span_id) is now tracked in #771, together with the resolved tenant, so it can ship without the trusted-proxy design. This issue keeps the trusted-proxy client IP and the trace-context gating. Both are part of the observability epic #777.

  4. moved this from Backlog to In progress in WaveHouse Task Boardon Oct 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/apiHTTP handlers, routing, middlewarearea/observabilityMetrics, logs, traces, health, profilingdocumentationImprovements or additions to documentationenhancementNew feature or requestsecuritySecurity-sensitive issue or fix

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions